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

[Igor Mammedov]

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.
See build_madt() function to see how it's generated now.

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
>