[SeaBIOS] [PATCH 1/2] fw/acpi: Build MADT the way commodity BIOSes do

[Laszlo Ersek]

On 01/30/17 13:39, Igor Mammedov wrote:
> On Fri, 27 Jan 2017 15:57:19 +0100
> Laszlo Ersek <lersek at redhat.com> wrote:
> 
>> On 01/27/17 15:27, Kevin O'Connor wrote:
>>> On Fri, Jan 27, 2017 at 02:18:39AM +0100, Filippo Sironi wrote:
>>>> When running Linux on an Intel machine with a commodity BIOS, CPUs are
>>>> numbered starting from 0 and the first half refers to the first logical
>>>> CPU on each core while the second half refers to the second logical CPU
>>>> on each core.
>>>> As an example, on machine with 2 sockets, 4 cores per socket, and 2
>>>> logical CPUs per core, CPUs would be numbered:
>>>>
>>>> *  0  1  2  3 - first logical CPU on each core of socket 0
>>>> *  4  5  6  7 - first logical CPU on each core of socket 1
>>>> *  8  9 10 11 - second logical CPU on each core of socket 0
>>>> * 12 13 14 15 - second logical CPU on each core of socket 1
> 
> ACPI spec says
> "
>  platform firmware should list the first logical processor of each of
>  the individual multi-threaded processors in the MADT before listing
>  any of the second logical processors.
> "
> 
> socket order is not mandated there and for example I have a system
> where apic id's re listed as following:
>   0x0  - first logical CPU on each core 0 of socket 0
>   0x10 - first logical CPU on each core 0 of socket 1
>   0x2  - first logical CPU on each core 1 of socket 0 
>   0x12 - first logical CPU on each core 1 of socket 1
> ...
> 
> the main point is that first threads are listed first
> (which socket/core order is not important except of boot CPU)
> and only after that goes list of secondary threads.
> 
> QEMU currently doesn't conform to above guideline as MADT is 1.0 revision
> doesn't have such requirement.
> However patches are welcome to make it adhering to the later ACPI specs.

Ah I didn't realize that. Thank you for the analysis!

> See build_madt() function to see how it's generated now.

Hopefully Filippo can look into that.

Thanks!
Laszlo

> 
> Also does patch fix any particular issue/problem or just making MADT
> conform to spec?
> 
>>>>
>>>> With seabios (prior to this patch), CPU 0 would be the first logical CPU
>>>> on core 0 of socket 0, CPU 1 would be the second logical CPU on core 0
>>>> of socket 0, and so on.
>>>> This is due to the fact that processor_id and local_apic_id are assigned
>>>> with the same value when building MADT.
>>>>
>>>> Exhaust the most-significant 7 bits of the 8-bit APIC ID and then come
>>>> back to the least-significant bit when assigning local_apic_id, since
>>>> the least-significant bit identifies the second logical CPU on a core.
>>>>
>>>> If HTT isn't available, keep the previous behavior.
>>>>
>>>> Signed-off-by: Filippo Sironi <sironi at amazon.de>
>>>> ---
>>>>  src/fw/acpi.c | 42 +++++++++++++++++++++++++++++++++++++++++-
>>>>  1 file changed, 41 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/fw/acpi.c b/src/fw/acpi.c
>>>> index 8bc2ca6..864c247 100644
>>>> --- a/src/fw/acpi.c
>>>> +++ b/src/fw/acpi.c
>>>
>>> The acpi.c code in seabios is deprecated.  With all recent version of
>>> qemu/kvm the acpi tables are obtained from qemu.  So, if there is a
>>> desired change to the tables then a patch should be made for qemu.
>>
>> QEMU constructs minimal / condensed APIC IDs so that they precisely
>> correspond to the socket / core / thread hierarchy requested on the
>> command line. If I remember correctly, the algorithm was implemented
>> directly from Intel's MP spec. If the APIC IDs in the guest don't look
>> as desired, I think the -smp option might have to be tweaked.
>>
>> I also seem to recall that this spec-adherent behavior could be machine
>> type dependent (i.e., unavailable before a specific machine type). The
>> machtypes not conforming to it should be ancient IMO.
>>
>> CC'ing Eduardo and Igor.
>>
>> Thanks
>> Laszlo
>>
>