linux-kernel - Re: 答复: [RFC PATCH] x86/acpi: Prevent x2apic id -1 from being accounted

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <148150f5-4b33-c064-a877-88c20f63e96e@cn.fujitsu.com>
Date:   Mon, 9 Apr 2018 16:46:55 +0800
From:   Dou Liyang <douly.fnst@...fujitsu.com>
To:     "Li,Rongqing" <lirongqing@...du.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "hpa@...or.com" <hpa@...or.com>,
        "jgross@...e.com" <jgross@...e.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "peterz@...radead.org" <peterz@...radead.org>
Subject: Re: 答复: [RFC PATCH] x86/acpi: Prevent x2apic id -1 from being accounted

RongQing,

At 04/09/2018 02:38 PM, Li,Rongqing wrote:
> 
> 
>> -----邮件原件-----
>> 发件人: Dou Liyang [mailto:douly.fnst@...fujitsu.com]
>> 发送时间: 2018年4月9日 13:38
>> 收件人: Li,Rongqing <lirongqing@...du.com>; linux-kernel@...r.kernel.org;
>> tglx@...utronix.de; mingo@...hat.com; hpa@...or.com; jgross@...e.com;
>> x86@...nel.org; peterz@...radead.org
>> 主题: 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@...du.com>
>>> Cc: Peter Zijlstra <peterz@...radead.org>
>>> Cc: Thomas Gleixner <tglx@...utronix.de>
>>> Cc: Dou Liyang <douly.fnst@...fujitsu.com>
>>> ---
>>>    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