thanks, Rafael. Accepted all suggestions. Sent out v3 patch
regards
James
On 12/19/22 09:24, Rafael J. Wysocki wrote:
> On Fri, Dec 16, 2022 at 7:36 PM James Puthukattukaran
> <james.puthukattukaran@xxxxxxxxxx> wrote:
>>
>> Extending commit aa06e20f1be6 ("x86/ACPI: Don't add CPUs that are not
>> online capable") to include acpi_parse_x2apic as well. There is a check
>> for invalid apicid; however, there are BIOS FW with madt version >= 5
>> support that do not bother setting apic id to an invalid value since they
>> assume the OS will check the enabled and online capable flags.
>>
>> Signed-off-by: James Puthukattukaran<james.puthukattukaran@xxxxxxxxxx>
>> Reported-by: Benjamin Fuller<ben.fuller@xxxxxxxxxx>
>>
>> v2 : use 'enabled' local variable. Also fix checkpatch.pl catches
>> ---
>> arch/x86/kernel/acpi/boot.c | 11 ++++++++++-
>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
>> index 907cc98b1938..35d8c8654b42 100644
>> --- a/arch/x86/kernel/acpi/boot.c
>> +++ b/arch/x86/kernel/acpi/boot.c
>> @@ -208,7 +208,16 @@ acpi_parse_x2apic(union acpi_subtable_headers *header, const unsigned long end)
>> apic_id = processor->local_apic_id;
>> enabled = processor->lapic_flags & ACPI_MADT_ENABLED;
>>
>> - /* Ignore invalid ID */
>> +
>> + /* don't register processors that can not be onlined */
>> + if (acpi_support_online_capable &&
>> + !enabled &&
>
> Is the line break before the "enabled" check necessary?
>
> I think it would be better to check "enabled" first anyway.
>
>> + !(processor->lapic_flags & ACPI_MADT_ONLINE_CAPABLE))
>> + return 0;
>> +
>> + /* for systems older than madt version 5 (does not have
>> + * ACPI_MADT_ONLINE_CAPABLE defined), ignore invalid ID
>> + */
>
> The formatting of the above comment doesn't follow the kernel coding
> style for multi-line comments.
>
>> if (apic_id == 0xffffffff)
>> return 0;
>>
>> --
