Re: çå: [RFC PATCH] x86/acpi: Prevent x2apic id -1 from being accounted

From: Dou Liyang
Date: Mon Apr 09 2018 - 04:47:04 EST


RongQing,

At 04/09/2018 02:38 PM, Li,Rongqing wrote:


-----éäåä-----
åää: Dou Liyang [mailto:douly.fnst@xxxxxxxxxxxxxx]
åéæé: 2018å4æ9æ 13:38
æää: Li,Rongqing <lirongqing@xxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx;
tglx@xxxxxxxxxxxxx; mingo@xxxxxxxxxx; hpa@xxxxxxxxx; jgross@xxxxxxxx;
x86@xxxxxxxxxx; peterz@xxxxxxxxxxxxx
äé: Re: [RFC PATCH] x86/acpi: Prevent x2apic id -1 from being accounted

Hi RongQing,

Is there an local x2apic whose ID is in your machine?


I think no

[...]

[ 0.000000] ACPI: X2APIC (apic_id[0xffffffff] uid[0x00] disabled)
[ 0.000000] ACPI: X2APIC (apic_id[0xffffffff] uid[0x01] disabled)
[ 0.000000] ACPI: X2APIC (apic_id[0xffffffff] uid[0x02] disabled)

Ah, sure enough!

[...]


At 04/08/2018 07:38 PM, Li RongQing wrote:
local_apic_id of acpi_madt_local_x2apic is u32, it is converted to int
when checked by default_apic_id_valid() and return true if it is
larger than 0x7fffffff, this is wrong


For x2apic enabled systems,

- the byte length of X2APIC ID is 4, and it can be larger than
0x7fffffff in theory


Yes

- the ->apic_id_valid points to x2apic_apic_id_valid(), which always
return _ture_ , not default_apic_id_valid().


To this machine, the apic is apic_flat

I see, I am sorry the title and changelog make me misunderstand.

 Here, actually, we prevent all the X2APIC Id from being parsed in
non-x2apic mode, not just 0xFFFFFFFF. because the values of x2APIC ID
must be 255 and greater in ACPI MADT.


-RongQing

Thanks,
dou

and if local_apic_id is invalid, we should prevent it from being
accounted > This fixes a bug that Purley platform displays too many
possible cpu

Signed-off-by: Li RongQing <lirongqing@xxxxxxxxx>
Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
Cc: Dou Liyang <douly.fnst@xxxxxxxxxxxxxx>
---
arch/x86/include/asm/apic.h | 4 ++--
arch/x86/kernel/acpi/boot.c | 10 ++++++----
arch/x86/kernel/apic/apic_common.c | 2 +-
arch/x86/kernel/apic/apic_numachip.c | 2 +-
arch/x86/kernel/apic/x2apic.h | 2 +-
arch/x86/kernel/apic/x2apic_phys.c | 2 +-
arch/x86/kernel/apic/x2apic_uv_x.c | 2 +-
arch/x86/xen/apic.c | 2 +-
8 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 40a3d3642f3a..08acd954f00e 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -313,7 +313,7 @@ struct apic {
/* Probe, setup and smpboot functions */
int (*probe)(void);
int (*acpi_madt_oem_check)(char *oem_id, char
*oem_table_id);
- int (*apic_id_valid)(int apicid);
+ int (*apic_id_valid)(u32 apicid);
int (*apic_id_registered)(void);

bool (*check_apicid_used)(physid_mask_t *map, int apicid);
@@ -486,7 +486,7 @@ static inline unsigned int read_apic_id(void)
return apic->get_apic_id(reg);
}

-extern int default_apic_id_valid(int apicid);
+extern int default_apic_id_valid(u32 apicid);
extern int default_acpi_madt_oem_check(char *, char *);
extern void default_setup_apic_routing(void);

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 7a37d9357bc4..7412564dc2a7 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -200,7 +200,7 @@ acpi_parse_x2apic(struct acpi_subtable_header
*header, const unsigned long end)
{
struct acpi_madt_local_x2apic *processor = NULL;
#ifdef CONFIG_X86_X2APIC
- int apic_id;
+ u32 apic_id;
u8 enabled;
#endif

@@ -222,10 +222,12 @@ acpi_parse_x2apic(struct acpi_subtable_header
*header, const unsigned long end)
* to not preallocating memory for all NR_CPUS
* when we use CPU hotplug.
*/
- if (!apic->apic_id_valid(apic_id) && enabled)
+ if (!apic->apic_id_valid(apic_id)) {
printk(KERN_WARNING PREFIX "x2apic entry ignored\n");

Due to the disable APIC entries may not exist physically, please just
printk the warning if the APIC ID is enabled.

if (!apic->apic_id_valid(apic_id)) {
if(enabled)
printk...

return 0;
}



Thanks,
dou