diff mbox series

[v5,15/24] hw: i386: Export the i386 ACPI SRAT build method

Message ID 20181105014047.26447-16-sameo@linux.intel.com
State New
Headers show
Series ACPI reorganization for hardware-reduced API addition | expand

Commit Message

Samuel Ortiz Nov. 5, 2018, 1:40 a.m. UTC
This is the standard way of building SRAT on x86 platfoms. But future
machine types could decide to define their own custom SRAT build method
through the ACPI builder methods.
Moreover, we will also need to reach build_srat() from outside of
acpi-build in order to use it as the ACPI builder SRAT build method.

Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
---
 hw/i386/acpi-build.h | 5 +++++
 hw/i386/acpi-build.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

Comments

Igor Mammedov Nov. 15, 2018, 1:28 p.m. UTC | #1
On Mon,  5 Nov 2018 02:40:38 +0100
Samuel Ortiz <sameo@linux.intel.com> wrote:

> This is the standard way of building SRAT on x86 platfoms. But future
> machine types could decide to define their own custom SRAT build method
> through the ACPI builder methods.
> Moreover, we will also need to reach build_srat() from outside of
> acpi-build in order to use it as the ACPI builder SRAT build method.
SRAT is usually highly machine specific (memory holes, layout, guest OS
specific quirks) so it's hard to generalize it.

I'd  drop SRAT related patches from this series and introduce
i386/virt specific SRAT when you post patches for it.

What we could generalize here is building blocks used to
create entries into acpi/aml-build.c
   build_srat_memory -> build_srat_memory_entry()
   build_apic_entry()
   build_x2apic_entry()
and please switch these blocks to build_append_int_noprefix() API
before moving them to acpi/aml-build.c

> Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
> ---
>  hw/i386/acpi-build.h | 5 +++++
>  hw/i386/acpi-build.c | 2 +-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h
> index 065a1d8250..d73c41fe8f 100644
> --- a/hw/i386/acpi-build.h
> +++ b/hw/i386/acpi-build.h
> @@ -4,6 +4,11 @@
>  
>  #include "hw/acpi/acpi.h"
>  
> +/* ACPI SRAT (Static Resource Affinity Table) build method for x86 */
> +void
> +build_srat(GArray *table_data, BIOSLinker *linker,
> +           MachineState *machine, AcpiConfiguration *acpi_conf);
> +
>  void acpi_setup(MachineState *machine, AcpiConfiguration *acpi_conf);
>  
>  #endif
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 1ef1a38441..673c5dfafc 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1615,7 +1615,7 @@ build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog)
>  #define HOLE_640K_START  (640 * KiB)
>  #define HOLE_640K_END   (1 * MiB)
>  
> -static void
> +void
>  build_srat(GArray *table_data, BIOSLinker *linker,
>             MachineState *machine, AcpiConfiguration *acpi_conf)
>  {
Samuel Ortiz Nov. 21, 2018, 11:27 p.m. UTC | #2
On Thu, Nov 15, 2018 at 02:28:54PM +0100, Igor Mammedov wrote:
> On Mon,  5 Nov 2018 02:40:38 +0100
> Samuel Ortiz <sameo@linux.intel.com> wrote:
> 
> > This is the standard way of building SRAT on x86 platfoms. But future
> > machine types could decide to define their own custom SRAT build method
> > through the ACPI builder methods.
> > Moreover, we will also need to reach build_srat() from outside of
> > acpi-build in order to use it as the ACPI builder SRAT build method.
> SRAT is usually highly machine specific (memory holes, layout, guest OS
> specific quirks) so it's hard to generalize it.
Hence the need for an SRAT builder interface.

> I'd  drop SRAT related patches from this series and introduce
> i386/virt specific SRAT when you post patches for it.
virt uses the existing i386 build_srat() routine, there's nothing
special about it. So this would be purely duplicated code.

Cheers,
Samuel.
Igor Mammedov Nov. 26, 2018, 3:47 p.m. UTC | #3
On Thu, 22 Nov 2018 00:27:33 +0100
Samuel Ortiz <sameo@linux.intel.com> wrote:

> On Thu, Nov 15, 2018 at 02:28:54PM +0100, Igor Mammedov wrote:
> > On Mon,  5 Nov 2018 02:40:38 +0100
> > Samuel Ortiz <sameo@linux.intel.com> wrote:
> >   
> > > This is the standard way of building SRAT on x86 platfoms. But future
> > > machine types could decide to define their own custom SRAT build method
> > > through the ACPI builder methods.
> > > Moreover, we will also need to reach build_srat() from outside of
> > > acpi-build in order to use it as the ACPI builder SRAT build method.  
> > SRAT is usually highly machine specific (memory holes, layout, guest OS
> > specific quirks) so it's hard to generalize it.  
> Hence the need for an SRAT builder interface.
so far builder interface (trying to generalize acpi_build()) looks
not necessary.
I'd suggest to drop and call build_start() directly.

> > I'd  drop SRAT related patches from this series and introduce
> > i386/virt specific SRAT when you post patches for it.  
> virt uses the existing i386 build_srat() routine, there's nothing
> special about it. So this would be purely duplicated code.
Looking at build_srat(), it has a bunch of code to handle legacy
PC layout. You probably don't need any of it for new is86/virt
machine and can make simpler version of it.

In addition (probably repeating question I've asked elsewhere),
Do you have to use split initial memory model for new machine?
Is it possible to use only pc-dimms both for initial and hotplugged memory
at some address (4Gb?) without cutting out PCI hole or any toher holes in RAM layout?

> Cheers,
> Samuel.
>
diff mbox series

Patch

diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h
index 065a1d8250..d73c41fe8f 100644
--- a/hw/i386/acpi-build.h
+++ b/hw/i386/acpi-build.h
@@ -4,6 +4,11 @@ 
 
 #include "hw/acpi/acpi.h"
 
+/* ACPI SRAT (Static Resource Affinity Table) build method for x86 */
+void
+build_srat(GArray *table_data, BIOSLinker *linker,
+           MachineState *machine, AcpiConfiguration *acpi_conf);
+
 void acpi_setup(MachineState *machine, AcpiConfiguration *acpi_conf);
 
 #endif
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 1ef1a38441..673c5dfafc 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1615,7 +1615,7 @@  build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog)
 #define HOLE_640K_START  (640 * KiB)
 #define HOLE_640K_END   (1 * MiB)
 
-static void
+void
 build_srat(GArray *table_data, BIOSLinker *linker,
            MachineState *machine, AcpiConfiguration *acpi_conf)
 {