Skip to content

Commit cb0009f

Browse files
shiqingglijinxia
authored andcommitted
hv: cpu: fix 'Pointer arithmetic is not on array'
Use the array for lapic_id directly to avoid the unnecessary pointer arithmetic. With current implementation, lapic_id_base is always a byte array with CPU_PAGE_SIZE elements What this patch does: - replace 'uint8_t *lapic_id_base' with 'uint8_t lapic_id_array[CPU_PAGE_SIZE]' to make the boundary explicit - add a range check to ensure that there is no overflow v2 -> v3: * update the array size of lapic_id_array per discussion with Fengwei v1 -> v2: * remove the unnecessary range check in parse_madt in cpu.c Signed-off-by: Shiqing Gao <shiqing.gao@intel.com> Reviewed-by: Junjie Mao <junjie.mao@intel.com> Acked-by: Eddie Dong <eddie.dong@intel.com>
1 parent 44a175e commit cb0009f

File tree

3 files changed

+30
-26
lines changed

3 files changed

+30
-26
lines changed

hypervisor/arch/x86/cpu.c

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ volatile uint16_t up_count = 0U;
2828
uint64_t pcpu_active_bitmap = 0UL;
2929

3030
/* TODO: add more capability per requirement */
31-
/*APICv features*/
31+
/* APICv features */
3232
#define VAPIC_FEATURE_VIRT_ACCESS (1U << 0)
3333
#define VAPIC_FEATURE_VIRT_REG (1U << 1)
3434
#define VAPIC_FEATURE_INTR_DELIVERY (1U << 2)
@@ -251,45 +251,39 @@ static void alloc_phy_cpu_data(uint16_t pcpu_num)
251251
ASSERT(per_cpu_data_base_ptr != NULL, "");
252252
}
253253

254-
uint16_t __attribute__((weak)) parse_madt(uint8_t *lapic_id_base)
254+
uint16_t __attribute__((weak)) parse_madt(uint8_t lapic_id_array[MAX_PCPU_NUM])
255255
{
256256
static const uint8_t lapic_id[] = {0U, 2U, 4U, 6U};
257257
uint32_t i;
258258

259259
for (i = 0U; i < ARRAY_SIZE(lapic_id); i++) {
260-
*lapic_id_base = lapic_id[i];
261-
lapic_id_base++;
260+
lapic_id_array[i] = lapic_id[i];
262261
}
263262

264263
return ((uint16_t)ARRAY_SIZE(lapic_id));
265264
}
266265

267-
static void init_phy_cpu_storage(void)
266+
static void init_percpu_data_area(void)
268267
{
269268
uint16_t i;
270-
uint16_t pcpu_num=0U;
269+
uint16_t pcpu_num = 0U;
271270
uint16_t bsp_cpu_id;
272271
uint8_t bsp_lapic_id = 0U;
273-
uint8_t *lapic_id_base;
272+
uint8_t lapic_id_array[MAX_PCPU_NUM];
274273

275-
/*
276-
* allocate memory to save all lapic_id detected in parse_mdt.
277-
* We allocate 4K size which could save 4K CPUs lapic_id info.
278-
*/
279-
lapic_id_base = alloc_page();
280-
ASSERT(lapic_id_base != NULL, "fail to alloc page");
274+
/* Save all lapic_id detected via parse_mdt in lapic_id_array */
275+
pcpu_num = parse_madt(lapic_id_array);
276+
if (pcpu_num == 0U) {
277+
/* failed to get the physcial cpu number */
278+
ASSERT(false);
279+
}
281280

282-
pcpu_num = parse_madt(lapic_id_base);
283281
alloc_phy_cpu_data(pcpu_num);
284282

285283
for (i = 0U; i < pcpu_num; i++) {
286-
per_cpu(lapic_id, i) = *lapic_id_base;
287-
lapic_id_base++;
284+
per_cpu(lapic_id, i) = lapic_id_array[i];
288285
}
289286

290-
/* free memory after lapic_id are saved in per_cpu data */
291-
free((void *)lapic_id_base);
292-
293287
bsp_lapic_id = get_cur_lapic_id();
294288

295289
bsp_cpu_id = get_cpu_id_from_lapic_id(bsp_lapic_id);
@@ -458,7 +452,7 @@ void bsp_boot_init(void)
458452

459453
early_init_lapic();
460454

461-
init_phy_cpu_storage();
455+
init_percpu_data_area();
462456

463457
load_gdtr_and_tr();
464458

hypervisor/boot/acpi.c

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ void *get_acpi_tbl(const char *sig)
221221
return HPA2HVA(addr);
222222
}
223223

224-
static uint16_t _parse_madt(void *madt, uint8_t *lapic_id_base)
224+
static uint16_t _parse_madt(void *madt, uint8_t lapic_id_array[MAX_PCPU_NUM])
225225
{
226226
uint16_t pcpu_id = 0;
227227
struct acpi_madt_local_apic *processor;
@@ -243,9 +243,16 @@ static uint16_t _parse_madt(void *madt, uint8_t *lapic_id_base)
243243
if (entry->type == ACPI_MADT_TYPE_LOCAL_APIC) {
244244
processor = (struct acpi_madt_local_apic *)entry;
245245
if ((processor->lapic_flags & ACPI_MADT_ENABLED) != 0U) {
246-
*lapic_id_base = processor->id;
247-
lapic_id_base++;
246+
lapic_id_array[pcpu_id] = processor->id;
248247
pcpu_id++;
248+
/*
249+
* set the pcpu_num as 0U to indicate the
250+
* potential overflow
251+
*/
252+
if (pcpu_id >= MAX_PCPU_NUM) {
253+
pcpu_id = 0U;
254+
break;
255+
}
249256
}
250257
}
251258

@@ -256,8 +263,8 @@ static uint16_t _parse_madt(void *madt, uint8_t *lapic_id_base)
256263
return pcpu_id;
257264
}
258265

259-
/* The lapic_id info gotten from madt will be returned in lapic_id_base */
260-
uint16_t parse_madt(uint8_t *lapic_id_base)
266+
/* The lapic_id info gotten from madt will be returned in lapic_id_array */
267+
uint16_t parse_madt(uint8_t lapic_id_array[MAX_PCPU_NUM])
261268
{
262269
void *madt;
263270

@@ -267,7 +274,7 @@ uint16_t parse_madt(uint8_t *lapic_id_base)
267274
madt = get_acpi_tbl(ACPI_SIG_MADT);
268275
ASSERT(madt != NULL, "fail to get madt");
269276

270-
return _parse_madt(madt, lapic_id_base);
277+
return _parse_madt(madt, lapic_id_array);
271278
}
272279

273280
void *get_dmar_table(void)

hypervisor/include/arch/x86/cpu.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@
4343
#define CPU_PAGE_SIZE 0x1000U
4444
#define CPU_PAGE_MASK 0xFFFFFFFFFFFFF000UL
4545

46+
/* Assume the max physcial cpu number is 128 */
47+
#define MAX_PCPU_NUM 128U
48+
4649
#define MMU_PTE_PAGE_SHIFT CPU_PAGE_SHIFT
4750
#define MMU_PDE_PAGE_SHIFT 21
4851

0 commit comments

Comments
 (0)