Re: [PATCH] hw/loongarch: Fix ACPI processor id off-by-one error

[Jiajie Chen]

On 2023/8/21 09:24, bibo mao wrote:

> + Add xianglai
>
> Good catch.
>
> In theory, it is logical id, and it can be not equal to physical id.
> However it must be equal to _UID in cpu dsdt table which is missing
> now.
>

Yes, the logical id can be different from index. The spec says:

If the processor structure represents an actual processor, this field must match the value of ACPI processor ID field in the processor’s entry in the MADT. If the processor structure represents a group of associated processors, the structure might match a processor container in the name space. In that case this entry will match the value of the _UID method of the associated processor container. Where there is a match it must be represented. The flags field, described in Processor Structure Flags, includes a bit to describe whether the ACPI processor ID is valid.

I believe PPTT, MADT and DSDT should all adhere to the same logical id mapping.

> Can pptt table parse error be fixed if cpu dsdt table is added?
>
> Regards
> Bibo Mao
>
>
> 在 2023/8/20 18:56, Jiajie Chen 写道:
> > In hw/acpi/aml-build.c:build_pptt() function, the code assumes that the
> > ACPI processor id equals to the cpu index, for example if we have 8
> > cpus, then the ACPI processor id should be in range 0-7.
> >
> > However, in hw/loongarch/acpi-build.c:build_madt() function we broke the
> > assumption. If we have 8 cpus again, the ACPI processor id in MADT table
> > would be in range 1-8. It violates the following description taken from
> > ACPI spec 6.4 table 5.138:
> >
> > If the processor structure represents an actual processor, this field
> > must match the value of ACPI processor ID field in the processor’s entry
> > in the MADT.
> >
> > It will break the latest Linux 6.5-rc6 with the
> > following error message:
> >
> > ACPI PPTT: PPTT table found, but unable to locate core 7 (8)
> > Invalid BIOS PPTT
> >
> > Here 7 is the last cpu index, 8 is the ACPI processor id learned from
> > MADT.
> >
> > With this patch, Linux can properly detect SMT threads when "-smp
> > 8,sockets=1,cores=4,threads=2" is passed:
> >
> > Thread(s) per core:  2
> > Core(s) per socket:  2
> > Socket(s):           2
> >
> > The detection of number of sockets is still wrong, but that is out of
> > scope of the commit.
> >
> > Signed-off-by: Jiajie Chen <c@jia.je>
> > ---
> >  hw/loongarch/acpi-build.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
> > index 0b62c3a2f7..ae292fc543 100644
> > --- a/hw/loongarch/acpi-build.c
> > +++ b/hw/loongarch/acpi-build.c
> > @@ -127,7 +127,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, LoongArchMachineState *lams)
> >          build_append_int_noprefix(table_data, 17, 1);    /* Type */
> >          build_append_int_noprefix(table_data, 15, 1);    /* Length */
> >          build_append_int_noprefix(table_data, 1, 1);     /* Version */
> > -        build_append_int_noprefix(table_data, i + 1, 4); /* ACPI Processor ID */
> > +        build_append_int_noprefix(table_data, i, 4);     /* ACPI Processor ID */
> >          build_append_int_noprefix(table_data, arch_id, 4); /* Core ID */
> >          build_append_int_noprefix(table_data, 1, 4);     /* Flags */
> >      }
> >       
>