Re: [Qemu-devel] [RFC seabios PATCH] enumerate APIC IDs directly from CPUs

2012-07-23 Thread Gleb Natapov
On Thu, Jul 19, 2012 at 04:46:35PM -0300, Eduardo Habkost wrote:
 On Thu, Jul 19, 2012 at 11:28:54AM -0300, Eduardo Habkost wrote:
  On Thu, Jul 19, 2012 at 12:58:46PM +0300, Gleb Natapov wrote:
   On Tue, Jul 17, 2012 at 06:56:30PM -0300, Eduardo Habkost wrote:
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.

   We need to be prepared to support more than 255 cpus. With  255 cpus
   comes x2apic and x2apic has 32bit apic ids. HW does not have to support
   all of the bits though, but potentially all the bitmasks can grow
   prohibitedly large.
  
  I see only two solutions:
  
  - Specify an interface/convention for QEMU and Seabios agree upon a CPU
identifier = x2apic = LAPIC ID mapping for all CPUs.
  - Specify new NUMA-information and CPU hotplug interfaces (or extend the
existing ones) based on x2apic ID, when Seabios start supporting
x2apic.
  
  I am not particularly inclined towards any of those two solutions. I
  dislike them equally.  :-)
 
 Oh, it is simpler than I have expected. x2APIC specification:
 
 The local APIC ID is initialized by hardware with a 32 bit ID (x2APIC
 ID). The lowest 8 bits of the x2APIC ID is the legacy local xAPIC ID,
 and is stored in the upper 8 bits of the APIC register for access in
 xAPIC mode.
 
 And the ACPI specification:
 
 Logical processors with APIC ID values of 255 and greater are required
 to have a Processor Device object and must convey the processor’s APIC
 information to OSPM using the Processor Local X2APIC structure. Logical
 processors with APIC ID values less than 255 must use the Processor
 Local APIC structure to convey their APIC information to OSPM.
 
 That means the x2APIC ID and the xAPIC ID are interchangeable, for
 values = 255. That means the QEMU=Seabios communication can be safely
 based on APIC IDs without any ambiguity.
 
Yes for = 255 they interchangeable. That's why we can add +x2apic to
our cpu models without changes to the BIOS.

 The CPU hotplug interface is a bit of a problem because it is based on a
 256-bit bitmap. But on the day it gets extended to support more than 256
 CPUs, it can safely be based on APIC IDs and still keep compatibility
 with systems without x2APIC.
The bitmap will have to be extended if we will go beyond 256 cpus. Using
apic-id to index the bitmap means that the size of the bitmap is a
function of max apic-id we want to support, not max number of cpus.

 
 So, now I am strongly inclined towards the second option from the list
 above: just use APIC IDs everywhere to identify CPUs when QEMU and
 Seabios communicate with each other, and QEMU can completely ignore the
 Processor ID used by Seabios.
I agree with making Processor ID Seabios internal thing.

 
  
  Note that I am more worried about the QEMU-Seabios interfaces. The
  APIC ID bitmap on smp.c, for example, is just an implementation detail:
  if we make Seabios support x2apic, that code can be changed to use a
  different data structure instead.
  
 [...]
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 am curious what will happen if cpu will be hot plugged, than hibernate
   and resume is done. After resume hot plugged cpu will have different
   Processor ID in ACPI. This may or may not be a problem.
  
  True. Keeping those tables stable after hotplug and hibernate may be a
  challenge.
  
  Maybe it would be easier to just leave holes on the MADT and SSDT tables
  (making APIC ID and Processor ID always match), and hope no OS will be
  confused by the holes.
 
 I am inclined to try this approach first (keep APIC ID == ACPI Processor
 ID), to keep things simple in Seabios. I am hoping no OS will have
 problems with the holes in the list of enabled Processor IDs.
 
They shouldn't.

--
Gleb.



Re: [Qemu-devel] [RFC seabios PATCH] enumerate APIC IDs directly from CPUs

2012-07-19 Thread Gleb Natapov
On Tue, Jul 17, 2012 at 06:56:30PM -0300, Eduardo Habkost wrote:
 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.
 
We need to be prepared to support more than 255 cpus. With  255 cpus
comes x2apic and x2apic has 32bit apic ids. HW does not have to support
all of the bits though, but potentially all the bitmasks can grow
prohibitedly large.


 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 am curious what will happen if cpu will be hot plugged, than hibernate
and resume is done. After resume hot plugged cpu will have different
Processor ID in ACPI. This may or may not be a problem.

Some comment below.

 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 ehabk...@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 

Re: [Qemu-devel] [RFC seabios PATCH] enumerate APIC IDs directly from CPUs

2012-07-19 Thread Eduardo Habkost
On Thu, Jul 19, 2012 at 12:58:46PM +0300, Gleb Natapov wrote:
 On Tue, Jul 17, 2012 at 06:56:30PM -0300, Eduardo Habkost wrote:
  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.
  
 We need to be prepared to support more than 255 cpus. With  255 cpus
 comes x2apic and x2apic has 32bit apic ids. HW does not have to support
 all of the bits though, but potentially all the bitmasks can grow
 prohibitedly large.

I see only two solutions:

- Specify an interface/convention for QEMU and Seabios agree upon a CPU
  identifier = x2apic = LAPIC ID mapping for all CPUs.
- Specify new NUMA-information and CPU hotplug interfaces (or extend the
  existing ones) based on x2apic ID, when Seabios start supporting
  x2apic.

I am not particularly inclined towards any of those two solutions. I
dislike them equally.  :-)

Note that I am more worried about the QEMU-Seabios interfaces. The
APIC ID bitmap on smp.c, for example, is just an implementation detail:
if we make Seabios support x2apic, that code can be changed to use a
different data structure instead.



 
 
  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 am curious what will happen if cpu will be hot plugged, than hibernate
 and resume is done. After resume hot plugged cpu will have different
 Processor ID in ACPI. This may or may not be a problem.

True. Keeping those tables stable after hotplug and hibernate may be a
challenge.

Maybe it would be easier to just leave holes on the MADT and SSDT tables
(making APIC ID and Processor ID always match), and hope no OS will be
confused by the holes.

 
 Some comment below.
 
  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 ehabk...@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, 

Re: [Qemu-devel] [RFC seabios PATCH] enumerate APIC IDs directly from CPUs

2012-07-19 Thread Eduardo Habkost
On Thu, Jul 19, 2012 at 11:28:54AM -0300, Eduardo Habkost wrote:
 On Thu, Jul 19, 2012 at 12:58:46PM +0300, Gleb Natapov wrote:
  On Tue, Jul 17, 2012 at 06:56:30PM -0300, Eduardo Habkost wrote:
   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.
   
  We need to be prepared to support more than 255 cpus. With  255 cpus
  comes x2apic and x2apic has 32bit apic ids. HW does not have to support
  all of the bits though, but potentially all the bitmasks can grow
  prohibitedly large.
 
 I see only two solutions:
 
 - Specify an interface/convention for QEMU and Seabios agree upon a CPU
   identifier = x2apic = LAPIC ID mapping for all CPUs.
 - Specify new NUMA-information and CPU hotplug interfaces (or extend the
   existing ones) based on x2apic ID, when Seabios start supporting
   x2apic.
 
 I am not particularly inclined towards any of those two solutions. I
 dislike them equally.  :-)

Oh, it is simpler than I have expected. x2APIC specification:

The local APIC ID is initialized by hardware with a 32 bit ID (x2APIC
ID). The lowest 8 bits of the x2APIC ID is the legacy local xAPIC ID,
and is stored in the upper 8 bits of the APIC register for access in
xAPIC mode.

And the ACPI specification:

Logical processors with APIC ID values of 255 and greater are required
to have a Processor Device object and must convey the processor’s APIC
information to OSPM using the Processor Local X2APIC structure. Logical
processors with APIC ID values less than 255 must use the Processor
Local APIC structure to convey their APIC information to OSPM.

That means the x2APIC ID and the xAPIC ID are interchangeable, for
values = 255. That means the QEMU=Seabios communication can be safely
based on APIC IDs without any ambiguity.

The CPU hotplug interface is a bit of a problem because it is based on a
256-bit bitmap. But on the day it gets extended to support more than 256
CPUs, it can safely be based on APIC IDs and still keep compatibility
with systems without x2APIC.

So, now I am strongly inclined towards the second option from the list
above: just use APIC IDs everywhere to identify CPUs when QEMU and
Seabios communicate with each other, and QEMU can completely ignore the
Processor ID used by Seabios.

 
 Note that I am more worried about the QEMU-Seabios interfaces. The
 APIC ID bitmap on smp.c, for example, is just an implementation detail:
 if we make Seabios support x2apic, that code can be changed to use a
 different data structure instead.
 
[...]
   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 am curious what will happen if cpu will be hot plugged, than hibernate
  and resume is done. After resume hot plugged cpu will have different
  Processor ID in ACPI. This may or may not be a problem.
 
 True. Keeping those tables stable after hotplug and hibernate may be a
 challenge.
 
 Maybe it would be easier to just leave holes on the MADT and SSDT tables
 (making APIC ID and Processor ID always match), and hope no OS will be
 confused by the holes.

I am inclined to try this approach first (keep APIC ID == ACPI Processor
ID), to keep things simple in Seabios. I am hoping no OS will have
problems with the holes in the list of enabled Processor IDs.

-- 
Eduardo