SeaBIOS

seabios@seabios.org

4 participants
3801 discussions

Re: [SeaBIOS] [Qemu-devel] [QEMU RFC PATCH 6/7] i386: topology & APIC ID utility functions
by Eduardo Habkost 24 Jul '12

24 Jul '12

On Mon, Jul 23, 2012 at 07:44:44PM +0000, Blue Swirl wrote: > On Mon, Jul 23, 2012 at 7:28 PM, Eduardo Habkost <ehabkost(a)redhat.com> wrote: > > On Mon, Jul 23, 2012 at 07:11:11PM +0000, Blue Swirl wrote: > >> On Mon, Jul 23, 2012 at 6:59 PM, Eduardo Habkost <ehabkost(a)redhat.com> wrote: > >> > On Mon, Jul 23, 2012 at 04:49:07PM +0000, Blue Swirl wrote: > >> >> On Mon, Jul 16, 2012 at 5:42 PM, Eduardo Habkost <ehabkost(a)redhat.com> wrote: > >> >> > On Sat, Jul 14, 2012 at 09:14:30AM +0000, Blue Swirl wrote: > >> >> > [...] > >> >> >> >> > diff --git a/tests/Makefile b/tests/Makefile > >> >> >> >> > index b605e14..89bd890 100644 > >> >> >> >> > --- a/tests/Makefile > >> >> >> >> > +++ b/tests/Makefile > >> >> >> >> > @@ -15,6 +15,7 @@ check-unit-y += tests/test-string-output-visitor$(EXESUF) > >> >> >> >> > check-unit-y += tests/test-coroutine$(EXESUF) > >> >> >> >> > check-unit-y += tests/test-visitor-serialization$(EXESUF) > >> >> >> >> > check-unit-y += tests/test-iov$(EXESUF) > >> >> >> >> > +check-unit-y += tests/test-x86-cpuid$(EXESUF) > >> >> >> >> > >> >> >> >> This probably tries to build the cpuid test also for non-x86 targets > >> >> >> >> and break them all. > >> >> >> > > >> >> >> > I don't think there's any concept of "targets" for the check-unit tests. > >> >> >> > >> >> >> How about: > >> >> >> check-qtest-i386-y = tests/test-x86-cpuid$(EXESUF) > >> >> > > >> >> > test-x86-cpuid is not a qtest test case. > >> >> > >> >> Why not? I don't think it is a unit test either, judging from what the > >> >> other unit tests do. > >> > > >> > It is absolutely a unit test. I don't know why you don't think so. It > >> > simply checks the results of the APIC ID calculation functions. > >> > >> Yes, but the other 'unit tests' (the names used here are very > >> confusing, btw) check supporting functions like coroutines, not > >> hardware. Hardware tests (qtest) need to bind to an architecture, in > >> this case x86. > > > > test-x86-cpuid doesn't check hardware, either. It just checks the simple > > functions that calculate APIC IDs (to make sure the math is correct). > > Those functions aren't even used by any hardware emulation code until > > later in the patch series (when the CPU initialization code gets changed > > to use the new function). > > By that time the function is clearly x86 HW and the check is an x86 > hardware check. QEMU as whole consists of simple functions that > calculate something. It's useful onily for a specific architecture, yes, but it surely doesn't make libqtest necessary. That's the difference between the unit tests and qtest test cases: unit tests simply test small parts of the code (that may or may not be hardware-specific), and qtest tests hardware emulation after starting up a whole qemu process. It's a different kind of testing, with different sets of goals. I suppose you are not arguing that no function inside target-* would be ever allowed to have a unit test written for it. That would be a very silly constraint for people writing tests. -- Eduardo

2 1

[Seabois] BSOD during start of Win2003 installation
by Alexey Korolev 24 Jul '12

24 Jul '12

Hi, Win2003 falls to BSOD during early init stage of installer. It is ACPI related issue and it appears in any configuration. RAM size is 6GB Error code is : 0x000000A5 (0x02, 0xFFFFfADF6A446880, 0x1, 0xFFFFFADFAA34690) The things get broken after commit 2062f2bab81915c5c1f7af12a49ad8d4b3fd23fb (update dsdt resources at runtime) I've tried to debug this issue but haven't succeed yet. One strange thing here is that the code doesn't reach the _CRS method. We have added a debug output but nothing related to CRS was printed. May be you have any ideas why it may fail. I would much appreciate any help on this. Thanks, Alexey

1 0

[seabios PATCH 0/2] Allow non-contiguous APIC IDs
by Eduardo Habkost 24 Jul '12

24 Jul '12

Now this is an actual proposal I would like to get included on Seabios. It's much simpler than the previous RFCs. It simply changes the ACPI and mptable code to not require APIC IDs to be contiguous, eliminates references to the CountCPUs variable, using a 256-bit APIC ID bitmap to find out which CPUs are present on boot. Eduardo Habkost (2): acpi: set I/O APIC ID to 0 by default allow CPUs to have non-contiguous Local APIC IDs src/acpi-dsdt.dsl | 4 +++- src/acpi.c | 11 ++++++----- src/config.h | 2 ++ src/mptable.c | 2 +- src/smp.c | 17 +++++++++++++++++ src/util.h | 1 + 6 files changed, 30 insertions(+), 7 deletions(-) -- 1.7.10.4

3 8

Re: [SeaBIOS] [PATCH] geodevga: make use of readl and writel for mmio access
by Kevin O'Connor 24 Jul '12

24 Jul '12

On Wed, Jul 18, 2012 at 07:10:33PM +0200, Christian Gmeiner wrote: > On the other hand I will try to find someone how uses the vgabios on a > geode with a vga connector. Nils was able to run tests last time. -Kevin

3 3

Re: [SeaBIOS] [Qemu-devel] [QEMU RFC PATCH 6/7] i386: topology & APIC ID utility functions
by Eduardo Habkost 23 Jul '12

23 Jul '12

On Mon, Jul 23, 2012 at 07:11:11PM +0000, Blue Swirl wrote: > On Mon, Jul 23, 2012 at 6:59 PM, Eduardo Habkost <ehabkost(a)redhat.com> wrote: > > On Mon, Jul 23, 2012 at 04:49:07PM +0000, Blue Swirl wrote: > >> On Mon, Jul 16, 2012 at 5:42 PM, Eduardo Habkost <ehabkost(a)redhat.com> wrote: > >> > On Sat, Jul 14, 2012 at 09:14:30AM +0000, Blue Swirl wrote: > >> > [...] > >> >> >> > diff --git a/tests/Makefile b/tests/Makefile > >> >> >> > index b605e14..89bd890 100644 > >> >> >> > --- a/tests/Makefile > >> >> >> > +++ b/tests/Makefile > >> >> >> > @@ -15,6 +15,7 @@ check-unit-y += tests/test-string-output-visitor$(EXESUF) > >> >> >> > check-unit-y += tests/test-coroutine$(EXESUF) > >> >> >> > check-unit-y += tests/test-visitor-serialization$(EXESUF) > >> >> >> > check-unit-y += tests/test-iov$(EXESUF) > >> >> >> > +check-unit-y += tests/test-x86-cpuid$(EXESUF) > >> >> >> > >> >> >> This probably tries to build the cpuid test also for non-x86 targets > >> >> >> and break them all. > >> >> > > >> >> > I don't think there's any concept of "targets" for the check-unit tests. > >> >> > >> >> How about: > >> >> check-qtest-i386-y = tests/test-x86-cpuid$(EXESUF) > >> > > >> > test-x86-cpuid is not a qtest test case. > >> > >> Why not? I don't think it is a unit test either, judging from what the > >> other unit tests do. > > > > It is absolutely a unit test. I don't know why you don't think so. It > > simply checks the results of the APIC ID calculation functions. > > Yes, but the other 'unit tests' (the names used here are very > confusing, btw) check supporting functions like coroutines, not > hardware. Hardware tests (qtest) need to bind to an architecture, in > this case x86. test-x86-cpuid doesn't check hardware, either. It just checks the simple functions that calculate APIC IDs (to make sure the math is correct). Those functions aren't even used by any hardware emulation code until later in the patch series (when the CPU initialization code gets changed to use the new function). -- Eduardo

2 1

Re: [SeaBIOS] [Qemu-devel] SeaBIOS, FW_CFG_NUMA, and FW_CFG_MAX_CPUS
by Eduardo Habkost 23 Jul '12

23 Jul '12

On Mon, Jul 23, 2012 at 07:25:01PM +0000, Blue Swirl wrote: > On Mon, Jul 23, 2012 at 7:09 PM, Eduardo Habkost <ehabkost(a)redhat.com> wrote: > > On Mon, Jul 23, 2012 at 06:40:51PM +0000, Blue Swirl wrote: > >> On Fri, Jul 20, 2012 at 8:00 PM, Eduardo Habkost <ehabkost(a)redhat.com> wrote: > >> > Hi, > >> > > >> > While working at the CPU index vs APIC ID changes, I stumbled upon > >> > another not-very-well-defined interface between SeaBIOS and QEMU, and I > >> > would like to clarify the semantics and constraints of some FW_CFG > >> > entries. > >> > > >> > First, the facts/assumptions: > >> > > >> > - There's no concept of "CPU index" or "CPU identifier" that SeaBIOS and > >> > QEMU agree upon, except for the APIC ID. All SeaBIOS can really see > >> > are the CPU APIC IDs, on boot or on CPU hotplug. > >> > - The APIC ID is already a perfectly good CPU identifier, that is > >> > present on bare metal hardware too. > >> > - Adding a new kind of "CPU identifier" in addition to the APIC ID > >> > would just make things more complex. > >> > - The only problem with APIC IDs is that they may not be contiguous. > >> > > >> > That said, I would like to clarify the meaning of: > >> > > >> > - FW_CFG_MAX_CPUS > >> > > >> > What are the basic semantics and expectations about FW_CFG_MAX_CPUS? > >> > >> FYI: This originates from Sparc and PPC, it says how many SMP CPUs > >> there are in the system. There we don't have (at least now) any CPU > >> IDs and of course no APIC. > > > > Aren't you describing FW_CFG_NB_CPUS? If not, what's the difference > > between FS_CFG_NB_CPUS and FW_CFG_MAX_CPUS on those architectures? > > Yes, sorry. There's no difference. > > > > > Until now, the only purpose I see for max_cpus/FW_CFG_MAX_CPUS is to > > allow CPU hotplug. I don't know if there are other use cases where > > max_cpus/FW_CFG_MAX_CPUS is useful. > > > > > >> > >> But I have no idea what x86 should use. As a general rule, what would > >> happen on a real machine should be emulated, but QEMU can also assist > >> BIOS (for example to skip some complex HW probes). > > > > Right now I am divided between two approaches: > > > > - In case FW_CFG_MAX_CPUS' only purpose is to allow CPU hotplug, make it > > really mean "upper limit to APIC ID values" in x86; > > - Otherwise, I am inclined to add a FW_CFG_MAX_APIC_ID entry to x86, so > > the firmware can (optionally) choose appropriate sizes for its > > internal APIC-ID-based data structures. > > One integer does not tell very much. It tells a lot: it contains exactly 32 perfectly good bits of information. ;-) Jokes aside: one integer like that would be very helpful to SeaBIOS: by knowing what's the maximum APIC ID value it would ever see, it can build ACPI tables and support CPU hotplug without having to maintain and update Processor ID => APIC ID tables inside the ACPI ASL code (that's very hard to debug). > > > > >> > >> > Considering that the APIC IDs may not be contiguous, is it supposed to > >> > be: > >> > > >> > a) the maximum number of CPUs that will be ever online, doesn't matter > >> > their APIC IDs, or > >> > b) a value so that every CPU has APIC ID < MAX_CPUS. > >> > > >> > A practical example: suppose we have a machine with 18 CPUs with the > >> > following APIC IDs: 0x00, 0x01, 0x02, 0x04, 0x05, 0x06, 0x08, 0x09, > >> > 0x0a, 0x10, 0x11, 0x12, 0x14, 0x15, 0x16, 0x18, 0x19, 0x1a. > >> > > >> > (That's the expected result for a machine with 2 sockets, 3 cores per > >> > socket, 3 threads per core.) > >> > > >> > In that case, should FW_CFG_MAX_CPUS be: a) 18, or b) 27 (0x1b)? > >> > > >> > If it should be 18, it will require additional work on SeaBIOS to make: > >> > - CPU hotplug work > >> > - SRAT/MADT/SSDT tables be built with Processor ID != APIC ID > >> > - SRAT/MADT/SSDT tables be kept stable if the system is hibernated and > >> > resumed after a CPU is hot-plugged. > >> > > >> > (Probably in that case I would suggest introducing a FW_CFG_MAX_APIC_ID > >> > entry, so that SeaBIOS can still build the ACPI tables more easily). > >> > > >> > > >> > - FW_CFG_NUMA > >> > > >> > The first problem with FW_CFG_NUMA is that it depends on FW_CFG_MAX_CPUS > >> > (so it inherits the same questions above). The second is that > >> > FW_CFG_NUMA is a CPU-based table, but there's nothing SeaBIOS can use to > >> > know what CPUs FW_CFG_NUMA is refering to, except for the APIC IDs. So, > >> > should FW_CFG_NUMA be indexed by APIC IDs? > >> > > >> > > >> > - My proposal: > >> > > >> > My proposal is to try to keep things simple, and just use the following > >> > rule: > >> > > >> > - Never have a CPU with APIC ID > FW_CFG_MAX_CPUS. > >> > > >> > This way: > >> > - The SeaBIOS ACPI code can be kept simple. > >> > - The current CPU hotplug interface can work as-is (up to 256 CPUs), > >> > based on APIC IDs. > >> > - The current FW_CFG_NUMA interface can work as-is, indexed by APIC IDs. > >> > - The ACPI tables can be easily kept stable between hibernate and > >> > resume, after CPU hotplug. > >> > > >> > This is the direction I am trying to go, and I am sending this just to > >> > make sure nobody is against it, and to not surprise anybody when I send > >> > a QEMU patch to make FW_CFG_MAX_CPUS be based on APIC IDs. > >> > > >> > > >> > My second proposal would be to introduce a FW_CFG_MAX_APIC_ID entry, so > >> > the SeaBIOS ACPI code can be kept simple. > >> > > >> > My third proposal would be to introduce a FW_CFG CPU Index => APIC ID > >> > table, but I really wouldn't like to introduce a new type of CPU > >> > identifier to be used between QEMU and SeaBIOS, when the APIC ID is a > >> > perfectly good unique CPU identifier that already exists in bare metal > >> > hardware. > >> > > >> > -- > >> > Eduardo > >> > > >> > > > > -- > > Eduardo > -- Eduardo

1 0

Re: [SeaBIOS] [Qemu-devel] SeaBIOS, FW_CFG_NUMA, and FW_CFG_MAX_CPUS
by Eduardo Habkost 23 Jul '12

23 Jul '12

On Mon, Jul 23, 2012 at 06:40:51PM +0000, Blue Swirl wrote: > On Fri, Jul 20, 2012 at 8:00 PM, Eduardo Habkost <ehabkost(a)redhat.com> wrote: > > Hi, > > > > While working at the CPU index vs APIC ID changes, I stumbled upon > > another not-very-well-defined interface between SeaBIOS and QEMU, and I > > would like to clarify the semantics and constraints of some FW_CFG > > entries. > > > > First, the facts/assumptions: > > > > - There's no concept of "CPU index" or "CPU identifier" that SeaBIOS and > > QEMU agree upon, except for the APIC ID. All SeaBIOS can really see > > are the CPU APIC IDs, on boot or on CPU hotplug. > > - The APIC ID is already a perfectly good CPU identifier, that is > > present on bare metal hardware too. > > - Adding a new kind of "CPU identifier" in addition to the APIC ID > > would just make things more complex. > > - The only problem with APIC IDs is that they may not be contiguous. > > > > That said, I would like to clarify the meaning of: > > > > - FW_CFG_MAX_CPUS > > > > What are the basic semantics and expectations about FW_CFG_MAX_CPUS? > > FYI: This originates from Sparc and PPC, it says how many SMP CPUs > there are in the system. There we don't have (at least now) any CPU > IDs and of course no APIC. Aren't you describing FW_CFG_NB_CPUS? If not, what's the difference between FS_CFG_NB_CPUS and FW_CFG_MAX_CPUS on those architectures? Until now, the only purpose I see for max_cpus/FW_CFG_MAX_CPUS is to allow CPU hotplug. I don't know if there are other use cases where max_cpus/FW_CFG_MAX_CPUS is useful. > > But I have no idea what x86 should use. As a general rule, what would > happen on a real machine should be emulated, but QEMU can also assist > BIOS (for example to skip some complex HW probes). Right now I am divided between two approaches: - In case FW_CFG_MAX_CPUS' only purpose is to allow CPU hotplug, make it really mean "upper limit to APIC ID values" in x86; - Otherwise, I am inclined to add a FW_CFG_MAX_APIC_ID entry to x86, so the firmware can (optionally) choose appropriate sizes for its internal APIC-ID-based data structures. > > > Considering that the APIC IDs may not be contiguous, is it supposed to > > be: > > > > a) the maximum number of CPUs that will be ever online, doesn't matter > > their APIC IDs, or > > b) a value so that every CPU has APIC ID < MAX_CPUS. > > > > A practical example: suppose we have a machine with 18 CPUs with the > > following APIC IDs: 0x00, 0x01, 0x02, 0x04, 0x05, 0x06, 0x08, 0x09, > > 0x0a, 0x10, 0x11, 0x12, 0x14, 0x15, 0x16, 0x18, 0x19, 0x1a. > > > > (That's the expected result for a machine with 2 sockets, 3 cores per > > socket, 3 threads per core.) > > > > In that case, should FW_CFG_MAX_CPUS be: a) 18, or b) 27 (0x1b)? > > > > If it should be 18, it will require additional work on SeaBIOS to make: > > - CPU hotplug work > > - SRAT/MADT/SSDT tables be built with Processor ID != APIC ID > > - SRAT/MADT/SSDT tables be kept stable if the system is hibernated and > > resumed after a CPU is hot-plugged. > > > > (Probably in that case I would suggest introducing a FW_CFG_MAX_APIC_ID > > entry, so that SeaBIOS can still build the ACPI tables more easily). > > > > > > - FW_CFG_NUMA > > > > The first problem with FW_CFG_NUMA is that it depends on FW_CFG_MAX_CPUS > > (so it inherits the same questions above). The second is that > > FW_CFG_NUMA is a CPU-based table, but there's nothing SeaBIOS can use to > > know what CPUs FW_CFG_NUMA is refering to, except for the APIC IDs. So, > > should FW_CFG_NUMA be indexed by APIC IDs? > > > > > > - My proposal: > > > > My proposal is to try to keep things simple, and just use the following > > rule: > > > > - Never have a CPU with APIC ID > FW_CFG_MAX_CPUS. > > > > This way: > > - The SeaBIOS ACPI code can be kept simple. > > - The current CPU hotplug interface can work as-is (up to 256 CPUs), > > based on APIC IDs. > > - The current FW_CFG_NUMA interface can work as-is, indexed by APIC IDs. > > - The ACPI tables can be easily kept stable between hibernate and > > resume, after CPU hotplug. > > > > This is the direction I am trying to go, and I am sending this just to > > make sure nobody is against it, and to not surprise anybody when I send > > a QEMU patch to make FW_CFG_MAX_CPUS be based on APIC IDs. > > > > > > My second proposal would be to introduce a FW_CFG_MAX_APIC_ID entry, so > > the SeaBIOS ACPI code can be kept simple. > > > > My third proposal would be to introduce a FW_CFG CPU Index => APIC ID > > table, but I really wouldn't like to introduce a new type of CPU > > identifier to be used between QEMU and SeaBIOS, when the APIC ID is a > > perfectly good unique CPU identifier that already exists in bare metal > > hardware. > > > > -- > > Eduardo > > > -- Eduardo

2 1

Re: [SeaBIOS] [Qemu-devel] [QEMU RFC PATCH 6/7] i386: topology & APIC ID utility functions
by Eduardo Habkost 23 Jul '12

23 Jul '12

On Mon, Jul 23, 2012 at 04:49:07PM +0000, Blue Swirl wrote: > On Mon, Jul 16, 2012 at 5:42 PM, Eduardo Habkost <ehabkost(a)redhat.com> wrote: > > On Sat, Jul 14, 2012 at 09:14:30AM +0000, Blue Swirl wrote: > > [...] > >> >> > diff --git a/tests/Makefile b/tests/Makefile > >> >> > index b605e14..89bd890 100644 > >> >> > --- a/tests/Makefile > >> >> > +++ b/tests/Makefile > >> >> > @@ -15,6 +15,7 @@ check-unit-y += tests/test-string-output-visitor$(EXESUF) > >> >> > check-unit-y += tests/test-coroutine$(EXESUF) > >> >> > check-unit-y += tests/test-visitor-serialization$(EXESUF) > >> >> > check-unit-y += tests/test-iov$(EXESUF) > >> >> > +check-unit-y += tests/test-x86-cpuid$(EXESUF) > >> >> > >> >> This probably tries to build the cpuid test also for non-x86 targets > >> >> and break them all. > >> > > >> > I don't think there's any concept of "targets" for the check-unit tests. > >> > >> How about: > >> check-qtest-i386-y = tests/test-x86-cpuid$(EXESUF) > > > > test-x86-cpuid is not a qtest test case. > > Why not? I don't think it is a unit test either, judging from what the > other unit tests do. It is absolutely a unit test. I don't know why you don't think so. It simply checks the results of the APIC ID calculation functions. > > > > >> > >> > I had to do the following, to be able to make a test that uses the > >> > target-i386 code: > >> > > >> >> > +tests/test-x86-cpuid.o: QEMU_INCLUDES += -Itarget-i386 > >> > > >> > Any suggestions to avoid this hack would be welcome. > >> > >> Maybe it would be simpler to adjust #include path in the file. > > > > Using the full path on the #include line would break in case > > target-i386/topology.h include other files from the target-i386 > > directory. > > That's fragile. Maybe the target-xyz files should use the full path. Yes, it would be fragile. That's why I used -Itarget-i386 (not a perfect solution, but more likely to stay working and not break easily). I don't know if I understand what you propose. You mean to change the 256 target-specific #include lines inside qemu? $ git grep -f <(find target-* -name '*.h' | sed -e 's@target-[^/]*/@#include "@' | sort -u) target-* | wc -l 256 -- Eduardo

2 1

Re: [SeaBIOS] [Qemu-devel] [QEMU RFC PATCH 6/7] i386: topology & APIC ID utility functions
by Eduardo Habkost 23 Jul '12

23 Jul '12

On Sat, Jul 14, 2012 at 09:14:30AM +0000, Blue Swirl wrote: [...] > >> > diff --git a/tests/Makefile b/tests/Makefile > >> > index b605e14..89bd890 100644 > >> > --- a/tests/Makefile > >> > +++ b/tests/Makefile > >> > @@ -15,6 +15,7 @@ check-unit-y += tests/test-string-output-visitor$(EXESUF) > >> > check-unit-y += tests/test-coroutine$(EXESUF) > >> > check-unit-y += tests/test-visitor-serialization$(EXESUF) > >> > check-unit-y += tests/test-iov$(EXESUF) > >> > +check-unit-y += tests/test-x86-cpuid$(EXESUF) > >> > >> This probably tries to build the cpuid test also for non-x86 targets > >> and break them all. > > > > I don't think there's any concept of "targets" for the check-unit tests. > > How about: > check-qtest-i386-y = tests/test-x86-cpuid$(EXESUF) test-x86-cpuid is not a qtest test case. > > > I had to do the following, to be able to make a test that uses the > > target-i386 code: > > > >> > +tests/test-x86-cpuid.o: QEMU_INCLUDES += -Itarget-i386 > > > > Any suggestions to avoid this hack would be welcome. > > Maybe it would be simpler to adjust #include path in the file. Using the full path on the #include line would break in case target-i386/topology.h include other files from the target-i386 directory. -- Eduardo

2 1

[RFC seabios PATCH] enumerate APIC IDs directly from CPUs
by Eduardo Habkost 23 Jul '12

23 Jul '12

This patch is an attempt to fix the non-continguous-APIC-ID problem without the FW_CFG_LAPIC_INFO approach I have sent proposed last week. Basically, this changes Seabios to probe for APIC IDs directly from the CPUs on boot, instead of getting it using fw_cfg, store the found APIC IDs on a bitmap, and use that information whe building the MADT, SRAT, and SSDT ACPI tables. To do this properly, we have to decide the meaning of "CPU IDs" in the QEMU<->Seabios interfaces, too. I see two possible approaches: 1) Have Seabios and QEMU agree on a a "CPU identifier", that is independent from the APIC ID. 2) Always use the Initial APIC ID on all communication between QEMU and Seabios. This patch implements the second approach. That means: - QEMU won't know anything about the ACPI Processor IDs chosen by Seabios; - The NUMA CPU affinity table in QEMU_CFG_NUMA is APIC-ID-based, not Processor-ID-based; - The CPU hotplug bitmaps on I/O port 0xaf00 will be APIC-ID-based, too. Internally, the following changes were made on the ACPI code: - The CPON array is now APIC-ID-based - The NTFY method now gets the CPU APIC ID as parameter - The CPMA method now gets two parameters: the Processor ID and the APIC ID - The CPST method now gets the CPU APIC ID as parameter To try to make things less likely to break on the common, non-hotplug case, this patch makes the Processor IDs be chosen this way: - The CPUs present on boot get contiguous Processor IDs (even if the APIC IDs are not contiguous); - The remaining Processor declarations are going to associated to the remaining APIC IDs (immediately after the last present APIC ID), sequentially. This means that hotplugged CPUs may not get contiguous Processor declarations if their APIC IDs are not contiguous. I would like to know what others think about this approach. I think it is a much simpler approach than relying on some APIC ID <=> CPU ID translation to make Seabios and QEMU agree. This way, Seabios has absolute freedom to choose the ACPI Processor IDs. Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com> --- src/acpi-dsdt.dsl | 11 ++++-- src/acpi.c | 109 ++++++++++++++++++++++++++++++++++++++++++++--------- src/smp.c | 17 +++++++++ src/ssdt-proc.dsl | 12 ++++-- src/util.h | 3 ++ 5 files changed, 127 insertions(+), 25 deletions(-) diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl index 2060686..51ca037 100644 --- a/src/acpi-dsdt.dsl +++ b/src/acpi-dsdt.dsl @@ -674,20 +674,23 @@ DefinitionBlock ( External(CPON, PkgObj) /* Methods called by run-time generated SSDT Processor objects */ - Method (CPMA, 1, NotSerialized) { + Method (CPMA, 2, NotSerialized) { // _MAT method - create an madt apic buffer + // Arg0 = Processor ID + // Arg1 = Local APIC ID // Local0 = CPON flag for this cpu - Store(DerefOf(Index(CPON, Arg0)), Local0) + Store(DerefOf(Index(CPON, Arg1)), Local0) // Local1 = Buffer (in madt apic form) to return Store(Buffer(8) {0x00, 0x08, 0x00, 0x00, 0x00, 0, 0, 0}, Local1) // Update the processor id, lapic id, and enable/disable status Store(Arg0, Index(Local1, 2)) - Store(Arg0, Index(Local1, 3)) + Store(Arg1, Index(Local1, 3)) Store(Local0, Index(Local1, 4)) Return (Local1) } Method (CPST, 1, NotSerialized) { // _STA method - return ON status of cpu + // Arg0 = Local APIC ID // Local0 = CPON flag for this cpu Store(DerefOf(Index(CPON, Arg0)), Local0) If (Local0) { Return(0xF) } Else { Return(0x0) } @@ -708,7 +711,7 @@ DefinitionBlock ( Store (PRS, Local5) // Local2 = last read byte from bitmap Store (Zero, Local2) - // Local0 = cpuid iterator + // Local0 = APIC ID iterator Store (Zero, Local0) While (LLess(Local0, SizeOf(CPON))) { // Local1 = CPON flag for this cpu diff --git a/src/acpi.c b/src/acpi.c index 55e4607..149ff42 100644 --- a/src/acpi.c +++ b/src/acpi.c @@ -302,6 +302,63 @@ build_fadt(struct pci_device *pci) return fadt; } + +/* APIC IDs of each Processor indexed by ACPI Processor ID. + * Used to make the MADT entries and SSDT Processor declarations + * match each other. + */ +static u8 *processor_apic_ids; + +/* Number of Processor IDs that have a valid 8-bit APIC ID */ +static int count_valid_apic_ids; + +/* Build a ACPI Processor ID -> Local APIC ID table, used for the MADT + * and SSDT Processor declarations. + * The CPUs present at boot will be assigned to the first entries. + * The remaining entries will get sequential APIC IDs. + */ +static void +check_apic_ids(void) +{ + processor_apic_ids = malloc_high(MaxCountCPUs); + if (!processor_apic_ids) { + warn_noalloc(); + count_valid_apic_ids = 0; + return; + } + + int i; + int apic_id = 0; + int last_present_apic_id = 0; + + // the first entries are for the already-present CPUs: + for (i = 0; i < MaxCountCPUs; i++) { + // look for next present APIC ID: + while (apic_id <= MAX_APIC_ID && !apic_id_is_present(apic_id)) + apic_id++; + + if (apic_id > MAX_APIC_ID) + break; // no more valid APIC IDs + + processor_apic_ids[i] = apic_id; + last_present_apic_id = apic_id; + apic_id++; + } + + // the remaining entries will get sequential APIC IDs assigned, up to + // MAX_APIC_ID + apic_id = last_present_apic_id; + for ( ; i < MaxCountCPUs; i++) { + apic_id++; + if (apic_id > MAX_APIC_ID) + break; + + processor_apic_ids[i] = apic_id; + } + + count_valid_apic_ids = i; +} + static void* build_madt(void) { @@ -321,15 +378,16 @@ build_madt(void) madt->flags = cpu_to_le32(1); struct madt_processor_apic *apic = (void*)&madt[1]; int i; - for (i=0; i<MaxCountCPUs; i++) { + for (i = 0; i < count_valid_apic_ids; i++) { apic->type = APIC_PROCESSOR; apic->length = sizeof(*apic); apic->processor_id = i; - apic->local_apic_id = i; - if (i < CountCPUs) + apic->local_apic_id = processor_apic_ids[i]; + if (apic_id_is_present(apic->local_apic_id)) { apic->flags = cpu_to_le32(1); - else + } else { apic->flags = cpu_to_le32(0); + } apic++; } struct madt_io_apic *io_apic = (void*)apic; @@ -402,6 +460,7 @@ encodeLen(u8 *ssdt_ptr, int length, int bytes) #define SD_OFFSET_CPUHEX (*ssdt_proc_name - *ssdt_proc_start + 2) #define SD_OFFSET_CPUID1 (*ssdt_proc_name - *ssdt_proc_start + 4) #define SD_OFFSET_CPUID2 (*ssdt_proc_id - *ssdt_proc_start) +#define SD_OFFSET_APICID (*ssdt_proc_apic_id - *ssdt_proc_start) #define SD_SIZEOF (*ssdt_proc_end - *ssdt_proc_start) #define SD_PROC (ssdp_proc_aml + *ssdt_proc_start) @@ -410,12 +469,13 @@ encodeLen(u8 *ssdt_ptr, int length, int bytes) static void* build_ssdt(void) { - int acpi_cpus = MaxCountCPUs > 0xff ? 0xff : MaxCountCPUs; + int acpi_cpus = count_valid_apic_ids; + int cpon_size = count_valid_apic_ids ? processor_apic_ids[count_valid_apic_ids - 1] + 1 : 0; // length = ScopeOp + procs + NTYF method + CPON package int length = ((1+3+4) + (acpi_cpus * SD_SIZEOF) + (1+2+5+(12*acpi_cpus)) - + (6+2+1+(1*acpi_cpus)) + + (6+2+1+(1*cpon_size)) + 17); u8 *ssdt = malloc_high(sizeof(struct acpi_table_header) + length); if (! ssdt) { @@ -440,10 +500,12 @@ build_ssdt(void) ssdt_ptr[SD_OFFSET_CPUHEX+1] = getHex(i); ssdt_ptr[SD_OFFSET_CPUID1] = i; ssdt_ptr[SD_OFFSET_CPUID2] = i; + ssdt_ptr[SD_OFFSET_APICID] = processor_apic_ids[i]; ssdt_ptr += SD_SIZEOF; } // build "Method(NTFY, 2) {If (LEqual(Arg0, 0x00)) {Notify(CP00, Arg1)} ...}" + // Arg0 = APIC ID *(ssdt_ptr++) = 0x14; // MethodOp ssdt_ptr = encodeLen(ssdt_ptr, 2+5+(12*acpi_cpus), 2); *(ssdt_ptr++) = 'N'; @@ -457,7 +519,7 @@ build_ssdt(void) *(ssdt_ptr++) = 0x93; // LEqualOp *(ssdt_ptr++) = 0x68; // Arg0Op *(ssdt_ptr++) = 0x0A; // BytePrefix - *(ssdt_ptr++) = i; + *(ssdt_ptr++) = processor_apic_ids[i]; *(ssdt_ptr++) = 0x86; // NotifyOp *(ssdt_ptr++) = 'C'; *(ssdt_ptr++) = 'P'; @@ -467,6 +529,7 @@ build_ssdt(void) } // build "Name(CPON, Package() { One, One, ..., Zero, Zero, ... })" + // CPON array is APIC-ID based, not Processor ID based *(ssdt_ptr++) = 0x08; // NameOp *(ssdt_ptr++) = 'C'; *(ssdt_ptr++) = 'P'; @@ -474,9 +537,9 @@ build_ssdt(void) *(ssdt_ptr++) = 'N'; *(ssdt_ptr++) = 0x12; // PackageOp ssdt_ptr = encodeLen(ssdt_ptr, 2+1+(1*acpi_cpus), 2); - *(ssdt_ptr++) = acpi_cpus; - for (i=0; i<acpi_cpus; i++) - *(ssdt_ptr++) = (i < CountCPUs) ? 0x01 : 0x00; + *(ssdt_ptr++) = cpon_size; + for (i=0; i<cpon_size; i++) + *(ssdt_ptr++) = (apic_id_is_present(i)) ? 0x01 : 0x00; // store pci io windows: start, end, length // this way we don't have to do the math in the dsdt @@ -644,30 +707,40 @@ build_srat(void) memset(srat, 0, srat_size); srat->reserved1=1; struct srat_processor_affinity *core = (void*)(srat + 1); - int i; + int apic_id; u64 curnode; + u64 *cpudata = numadata; - for (i = 0; i < MaxCountCPUs; ++i) { + /* Note that the fw_cfg NUMA CPU affinity data is APIC-ID-based, not + * Processor ID based, but the size of the tablesble is based on MaxCountCPUs + */ + for (apic_id = 0; apic_id < MaxCountCPUs; ++apic_id) { + if (apic_id > MAX_APIC_ID) + break; + curnode = *cpudata++; core->type = SRAT_PROCESSOR; core->length = sizeof(*core); - core->local_apic_id = i; - curnode = *numadata++; core->proximity_lo = curnode; memset(core->proximity_hi, 0, 3); core->local_sapic_eid = 0; - if (i < CountCPUs) + core->local_apic_id = apic_id; + if (apic_id_is_present(apic_id)) { core->flags = cpu_to_le32(1); - else - core->flags = 0; + } else { + core->flags = cpu_to_le32(0); + } core++; } + numadata += MaxCountCPUs; + /* the memory map is a bit tricky, it contains at least one hole * from 640k-1M and possibly another one from 3.5G-4G. */ struct srat_memory_affinity *numamem = (void*)core; int slots = 0; + int i; u64 mem_len, mem_base, next_base = 0; acpi_build_srat_memory(numamem, 0, 640*1024, 0, 1); @@ -742,6 +815,8 @@ acpi_bios_init(void) tbl_idx++; \ } while(0) + check_apic_ids(); + struct fadt_descriptor_rev1 *fadt = build_fadt(pci); ACPI_INIT_TABLE(fadt); ACPI_INIT_TABLE(build_ssdt()); diff --git a/src/smp.c b/src/smp.c index 8c077a1..af92e4a 100644 --- a/src/smp.c +++ b/src/smp.c @@ -36,6 +36,8 @@ wrmsr_smp(u32 index, u64 val) u32 CountCPUs VAR16VISIBLE; u32 MaxCountCPUs VAR16VISIBLE; +// 256 bits for the found APIC IDs +u32 FoundAPICIDs[256/32] VAR16VISIBLE; extern void smp_ap_boot_code(void); ASM16( " .global smp_ap_boot_code\n" @@ -59,6 +61,12 @@ ASM16( " jmp 1b\n" "2:\n" + // get apic ID on EBX, set bit on FoundAPICIDs + " mov $1, %eax\n" + " cpuid\n" + " shrl $24, %ebx\n" + " lock bts %ebx, FoundAPICIDs\n" + // Increment the cpu counter " lock incl CountCPUs\n" @@ -67,6 +75,11 @@ ASM16( " jmp 1b\n" ); +int apic_id_is_present(u8 apic_id) +{ + return FoundAPICIDs[apic_id/32] & (1 << (apic_id % 32)); +} + // find and initialize the CPUs by launching a SIPI to them void smp_probe(void) @@ -82,6 +95,10 @@ smp_probe(void) return; } + // mark the BSP initial APIC ID as found, too: + u8 apic_id = ebx>>24; + FoundAPICIDs[apic_id/32] |= (1 << (apic_id % 32)); + // Init the counter. writel(&CountCPUs, 1); diff --git a/src/ssdt-proc.dsl b/src/ssdt-proc.dsl index 0339422..7e7e855 100644 --- a/src/ssdt-proc.dsl +++ b/src/ssdt-proc.dsl @@ -5,13 +5,15 @@ * be placed in the \_SB_ namespace. * * In addition to the aml code generated from this file, the - * src/acpi.c file creates a NTFY method with an entry for each cpu: + * src/acpi.c file creates a NTFY method with an entry for each cpu, + * that checks for the APIC ID: * Method(NTFY, 2) { * If (LEqual(Arg0, 0x00)) { Notify(CP00, Arg1) } * If (LEqual(Arg0, 0x01)) { Notify(CP01, Arg1) } * ... * } - * and a CPON array with the list of active and inactive cpus: + * and a CPON array with the list of active and inactive cpus, + * indexed by the APIC ID: * Name(CPON, Package() { One, One, ..., Zero, Zero, ... }) */ @@ -25,6 +27,8 @@ DefinitionBlock ("ssdt-proc.aml", "SSDT", 0x01, "BXPC", "BXSSDT", 0x1) Processor (CPAA, 0xAA, 0x0000b010, 0x06) { ACPI_EXTRACT_NAME_BYTE_CONST ssdt_proc_id Name (ID, 0xAA) + ACPI_EXTRACT_NAME_BYTE_CONST ssdt_proc_apic_id + Name (APIC, 0xAA) /* * The src/acpi.c code requires the above ACP_EXTRACT tags so that it can update * CPAA and 0xAA with the appropriate CPU id (see @@ -36,10 +40,10 @@ DefinitionBlock ("ssdt-proc.aml", "SSDT", 0x01, "BXPC", "BXSSDT", 0x1) External(CPST, MethodObj) External(CPEJ, MethodObj) Method(_MAT, 0) { - Return(CPMA(ID)) + Return(CPMA(ID, APIC)) } Method (_STA, 0) { - Return(CPST(ID)) + Return(CPST(APIC)) } Method (_EJ0, 1, NotSerialized) { CPEJ(ID, Arg0) diff --git a/src/util.h b/src/util.h index ef8ec7c..a68f56b 100644 --- a/src/util.h +++ b/src/util.h @@ -351,10 +351,13 @@ void pci_setup(void); void smm_init(void); // smp.c +#define MAX_APIC_ID 0xFE + extern u32 CountCPUs; extern u32 MaxCountCPUs; void wrmsr_smp(u32 index, u64 val); void smp_probe(void); +int apic_id_is_present(u8 apic_id); // coreboot.c extern const char *CBvendor, *CBpart; -- 1.7.10.4

2 4

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

SeaBIOS