* [Qemu-devel] [RFC PATCH v2] i386: Add _PXM ACPI method to CPU objects
@ 2013-11-27 13:02 Vasilis Liaskovitis
2013-11-27 15:53 ` Igor Mammedov
0 siblings, 1 reply; 4+ messages in thread
From: Vasilis Liaskovitis @ 2013-11-27 13:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Vasilis Liaskovitis, imammedo, thilo.fromm, seabios, mst
This patch adds a _PXM method to ACPI CPU objects for the pc machine. The _PXM
value is derived from the passed in guest info, same way as CPU SRAT entries.
Currently, CPU SRAT entries are only enabled for cpus that are already present
in the system. The SRAT entries for hotpluggable processors are disabled (flags
bit 0 set to 0 in hw/i385/acpi-build.c:build_srat). Section 5.2.16.1 of ACPI
spec mentions "If the Local APIC ID of a dynamically added processor is not
present in the SRAT, a _PXM object must exist for the processor’s device or one
of its ancestors in the ACPI Namespace." Since SRAT entries are not available
for the hot-pluggable processors, a _PXM method must exist for them. Otherwise,
the CPU is hot-added in the wrong NUMA node (default node 0).
Even if CPU SRAT entries are enabled, _PXM method is what the linux kernel
consults on hot-add time. Section 17.2.1 of ACPI spec mentions " OSPM will
consume the SRAT only at boot time. OSPM should use _PXM for any devices that
are hot-added into the system after boot up." To be more precise if SRAT
information is available to the guest kernel, it is used. However, parsed SRAT
info is reset and lost after hot-remove operations, see kernel commit c4c60524.
This means that on a hot-unplug / hot-replug scenario, and without a _PXM
method, the kernel may put a CPU on different nodes because SRAT info has been
reset by a previous hot-remove operation.
The above hot-remove/hot-add scenario has been tested on master, plus cpu-del
patches from:
https://lists.gnu.org/archive/html/qemu-devel/2013-10/msg01085.html
With the curret _PXM patch, hot-added CPUs are always placed into the correct
NUMA node, regardless of kernel behaviour.
v1->v2:
Make method return a DWORD integer
Tested on qemu master + cpu-del patches
Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com>
Reviewed-by: Thilo Fromm <t-lo@thilo-fromm.de>
---
hw/i386/acpi-build.c | 5 +++++
hw/i386/ssdt-proc.dsl | 5 +++++
2 files changed, 10 insertions(+)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index d089e1e..3c11ddc 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -603,6 +603,7 @@ static inline char acpi_get_hex(uint32_t val)
#define ACPI_PROC_OFFSET_CPUHEX (*ssdt_proc_name - *ssdt_proc_start + 2)
#define ACPI_PROC_OFFSET_CPUID1 (*ssdt_proc_name - *ssdt_proc_start + 4)
#define ACPI_PROC_OFFSET_CPUID2 (*ssdt_proc_id - *ssdt_proc_start)
+#define ACPI_PROC_OFFSET_CPUPXM (*ssdt_proc_pxm - *ssdt_proc_start)
#define ACPI_PROC_SIZEOF (*ssdt_proc_end - *ssdt_proc_start)
#define ACPI_PROC_AML (ssdp_proc_aml + *ssdt_proc_start)
@@ -724,6 +725,10 @@ build_ssdt(GArray *table_data, GArray *linker,
proc[ACPI_PROC_OFFSET_CPUHEX+1] = acpi_get_hex(i);
proc[ACPI_PROC_OFFSET_CPUID1] = i;
proc[ACPI_PROC_OFFSET_CPUID2] = i;
+ proc[ACPI_PROC_OFFSET_CPUPXM] = guest_info->node_cpu[i];
+ proc[ACPI_PROC_OFFSET_CPUPXM + 1] = 0;
+ proc[ACPI_PROC_OFFSET_CPUPXM + 2] = 0;
+ proc[ACPI_PROC_OFFSET_CPUPXM + 3] = 0;
}
/* build this code:
diff --git a/hw/i386/ssdt-proc.dsl b/hw/i386/ssdt-proc.dsl
index 8229bfd..8d4c5bf 100644
--- a/hw/i386/ssdt-proc.dsl
+++ b/hw/i386/ssdt-proc.dsl
@@ -47,6 +47,8 @@ DefinitionBlock ("ssdt-proc.aml", "SSDT", 0x01, "BXPC", "BXSSDT", 0x1)
* also updating the C code.
*/
Name(_HID, "ACPI0007")
+ ACPI_EXTRACT_NAME_DWORD_CONST ssdt_proc_pxm
+ Name(PXM, 0xAAAAAAAA)
External(CPMA, MethodObj)
External(CPST, MethodObj)
External(CPEJ, MethodObj)
@@ -59,5 +61,8 @@ DefinitionBlock ("ssdt-proc.aml", "SSDT", 0x01, "BXPC", "BXSSDT", 0x1)
Method(_EJ0, 1, NotSerialized) {
CPEJ(ID, Arg0)
}
+ Method(_PXM, 0) {
+ Return (PXM)
+ }
}
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [RFC PATCH v2] i386: Add _PXM ACPI method to CPU objects
2013-11-27 13:02 [Qemu-devel] [RFC PATCH v2] i386: Add _PXM ACPI method to CPU objects Vasilis Liaskovitis
@ 2013-11-27 15:53 ` Igor Mammedov
2013-11-27 15:58 ` Paolo Bonzini
0 siblings, 1 reply; 4+ messages in thread
From: Igor Mammedov @ 2013-11-27 15:53 UTC (permalink / raw)
To: Vasilis Liaskovitis; +Cc: thilo.fromm, seabios, qemu-devel, mst
On Wed, 27 Nov 2013 14:02:43 +0100
Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com> wrote:
> This patch adds a _PXM method to ACPI CPU objects for the pc machine. The _PXM
> value is derived from the passed in guest info, same way as CPU SRAT entries.
>
> Currently, CPU SRAT entries are only enabled for cpus that are already present
> in the system. The SRAT entries for hotpluggable processors are disabled (flags
> bit 0 set to 0 in hw/i385/acpi-build.c:build_srat). Section 5.2.16.1 of ACPI
> spec mentions "If the Local APIC ID of a dynamically added processor is not
> present in the SRAT, a _PXM object must exist for the processor’s device or one
> of its ancestors in the ACPI Namespace." Since SRAT entries are not available
> for the hot-pluggable processors, a _PXM method must exist for them. Otherwise,
> the CPU is hot-added in the wrong NUMA node (default node 0).
>
> Even if CPU SRAT entries are enabled, _PXM method is what the linux kernel
> consults on hot-add time. Section 17.2.1 of ACPI spec mentions " OSPM will
> consume the SRAT only at boot time. OSPM should use _PXM for any devices that
> are hot-added into the system after boot up." To be more precise if SRAT
> information is available to the guest kernel, it is used. However, parsed SRAT
> info is reset and lost after hot-remove operations, see kernel commit c4c60524.
> This means that on a hot-unplug / hot-replug scenario, and without a _PXM
> method, the kernel may put a CPU on different nodes because SRAT info has been
> reset by a previous hot-remove operation.
>
> The above hot-remove/hot-add scenario has been tested on master, plus cpu-del
> patches from:
> https://lists.gnu.org/archive/html/qemu-devel/2013-10/msg01085.html
> With the curret _PXM patch, hot-added CPUs are always placed into the correct
> NUMA node, regardless of kernel behaviour.
>
> v1->v2:
> Make method return a DWORD integer
> Tested on qemu master + cpu-del patches
>
> Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com>
> Reviewed-by: Thilo Fromm <t-lo@thilo-fromm.de>
Patch looks good,
Please add patch to update hw/i386/ssdt-proc.hex.generated for hosts without iasl
for completness
>
> ---
> hw/i386/acpi-build.c | 5 +++++
> hw/i386/ssdt-proc.dsl | 5 +++++
> 2 files changed, 10 insertions(+)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index d089e1e..3c11ddc 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -603,6 +603,7 @@ static inline char acpi_get_hex(uint32_t val)
> #define ACPI_PROC_OFFSET_CPUHEX (*ssdt_proc_name - *ssdt_proc_start + 2)
> #define ACPI_PROC_OFFSET_CPUID1 (*ssdt_proc_name - *ssdt_proc_start + 4)
> #define ACPI_PROC_OFFSET_CPUID2 (*ssdt_proc_id - *ssdt_proc_start)
> +#define ACPI_PROC_OFFSET_CPUPXM (*ssdt_proc_pxm - *ssdt_proc_start)
> #define ACPI_PROC_SIZEOF (*ssdt_proc_end - *ssdt_proc_start)
> #define ACPI_PROC_AML (ssdp_proc_aml + *ssdt_proc_start)
>
> @@ -724,6 +725,10 @@ build_ssdt(GArray *table_data, GArray *linker,
> proc[ACPI_PROC_OFFSET_CPUHEX+1] = acpi_get_hex(i);
> proc[ACPI_PROC_OFFSET_CPUID1] = i;
> proc[ACPI_PROC_OFFSET_CPUID2] = i;
> + proc[ACPI_PROC_OFFSET_CPUPXM] = guest_info->node_cpu[i];
> + proc[ACPI_PROC_OFFSET_CPUPXM + 1] = 0;
> + proc[ACPI_PROC_OFFSET_CPUPXM + 2] = 0;
> + proc[ACPI_PROC_OFFSET_CPUPXM + 3] = 0;
> }
>
> /* build this code:
> diff --git a/hw/i386/ssdt-proc.dsl b/hw/i386/ssdt-proc.dsl
> index 8229bfd..8d4c5bf 100644
> --- a/hw/i386/ssdt-proc.dsl
> +++ b/hw/i386/ssdt-proc.dsl
> @@ -47,6 +47,8 @@ DefinitionBlock ("ssdt-proc.aml", "SSDT", 0x01, "BXPC", "BXSSDT", 0x1)
> * also updating the C code.
> */
> Name(_HID, "ACPI0007")
> + ACPI_EXTRACT_NAME_DWORD_CONST ssdt_proc_pxm
> + Name(PXM, 0xAAAAAAAA)
> External(CPMA, MethodObj)
> External(CPST, MethodObj)
> External(CPEJ, MethodObj)
> @@ -59,5 +61,8 @@ DefinitionBlock ("ssdt-proc.aml", "SSDT", 0x01, "BXPC", "BXSSDT", 0x1)
> Method(_EJ0, 1, NotSerialized) {
> CPEJ(ID, Arg0)
> }
> + Method(_PXM, 0) {
> + Return (PXM)
> + }
> }
> }
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [RFC PATCH v2] i386: Add _PXM ACPI method to CPU objects
2013-11-27 15:53 ` Igor Mammedov
@ 2013-11-27 15:58 ` Paolo Bonzini
2013-11-29 14:44 ` Vasilis Liaskovitis
0 siblings, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2013-11-27 15:58 UTC (permalink / raw)
To: Igor Mammedov; +Cc: Vasilis Liaskovitis, thilo.fromm, seabios, qemu-devel, mst
Il 27/11/2013 16:53, Igor Mammedov ha scritto:
> Patch looks good,
> Please add patch to update hw/i386/ssdt-proc.hex.generated for hosts without iasl
> for completness
Also please rename PXM to CPXM or CPPX for consistency.
Paolo
>> >
>> > ---
>> > hw/i386/acpi-build.c | 5 +++++
>> > hw/i386/ssdt-proc.dsl | 5 +++++
>> > 2 files changed, 10 insertions(+)
>> >
>> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> > index d089e1e..3c11ddc 100644
>> > --- a/hw/i386/acpi-build.c
>> > +++ b/hw/i386/acpi-build.c
>> > @@ -603,6 +603,7 @@ static inline char acpi_get_hex(uint32_t val)
>> > #define ACPI_PROC_OFFSET_CPUHEX (*ssdt_proc_name - *ssdt_proc_start + 2)
>> > #define ACPI_PROC_OFFSET_CPUID1 (*ssdt_proc_name - *ssdt_proc_start + 4)
>> > #define ACPI_PROC_OFFSET_CPUID2 (*ssdt_proc_id - *ssdt_proc_start)
>> > +#define ACPI_PROC_OFFSET_CPUPXM (*ssdt_proc_pxm - *ssdt_proc_start)
>> > #define ACPI_PROC_SIZEOF (*ssdt_proc_end - *ssdt_proc_start)
>> > #define ACPI_PROC_AML (ssdp_proc_aml + *ssdt_proc_start)
>> >
>> > @@ -724,6 +725,10 @@ build_ssdt(GArray *table_data, GArray *linker,
>> > proc[ACPI_PROC_OFFSET_CPUHEX+1] = acpi_get_hex(i);
>> > proc[ACPI_PROC_OFFSET_CPUID1] = i;
>> > proc[ACPI_PROC_OFFSET_CPUID2] = i;
>> > + proc[ACPI_PROC_OFFSET_CPUPXM] = guest_info->node_cpu[i];
>> > + proc[ACPI_PROC_OFFSET_CPUPXM + 1] = 0;
>> > + proc[ACPI_PROC_OFFSET_CPUPXM + 2] = 0;
>> > + proc[ACPI_PROC_OFFSET_CPUPXM + 3] = 0;
>> > }
>> >
>> > /* build this code:
>> > diff --git a/hw/i386/ssdt-proc.dsl b/hw/i386/ssdt-proc.dsl
>> > index 8229bfd..8d4c5bf 100644
>> > --- a/hw/i386/ssdt-proc.dsl
>> > +++ b/hw/i386/ssdt-proc.dsl
>> > @@ -47,6 +47,8 @@ DefinitionBlock ("ssdt-proc.aml", "SSDT", 0x01, "BXPC", "BXSSDT", 0x1)
>> > * also updating the C code.
>> > */
>> > Name(_HID, "ACPI0007")
>> > + ACPI_EXTRACT_NAME_DWORD_CONST ssdt_proc_pxm
>> > + Name(PXM, 0xAAAAAAAA)
>> > External(CPMA, MethodObj)
>> > External(CPST, MethodObj)
>> > External(CPEJ, MethodObj)
>> > @@ -59,5 +61,8 @@ DefinitionBlock ("ssdt-proc.aml", "SSDT", 0x01, "BXPC", "BXSSDT", 0x1)
>> > Method(_EJ0, 1, NotSerialized) {
>> > CPEJ(ID, Arg0)
>> > }
>> > + Method(_PXM, 0) {
>> > + Return (PXM)
>> > + }
>> > }
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [RFC PATCH v2] i386: Add _PXM ACPI method to CPU objects
2013-11-27 15:58 ` Paolo Bonzini
@ 2013-11-29 14:44 ` Vasilis Liaskovitis
0 siblings, 0 replies; 4+ messages in thread
From: Vasilis Liaskovitis @ 2013-11-29 14:44 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Igor Mammedov, thilo.fromm, seabios, qemu-devel, mst
On Wed, Nov 27, 2013 at 04:58:51PM +0100, Paolo Bonzini wrote:
> Il 27/11/2013 16:53, Igor Mammedov ha scritto:
> > Patch looks good,
> > Please add patch to update hw/i386/ssdt-proc.hex.generated for hosts without iasl
> > for completness
>
> Also please rename PXM to CPXM or CPPX for consistency.
I posted an updated version of the patch on the list, also here:
http://patchwork.ozlabs.org/patch/294636/
thanks,
- Vasilis
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-11-29 14:44 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-27 13:02 [Qemu-devel] [RFC PATCH v2] i386: Add _PXM ACPI method to CPU objects Vasilis Liaskovitis
2013-11-27 15:53 ` Igor Mammedov
2013-11-27 15:58 ` Paolo Bonzini
2013-11-29 14:44 ` Vasilis Liaskovitis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).