[v6,4/5] ACPI: move acpi_build_srat_memory to common place

From: Shannon Zhao <shannon.zhao@linaro.org>

Move acpi_build_srat_memory to common place so that it could be reused
by ARM.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 hw/acpi/aml-build.c         | 12 ++++++++++++
 hw/i386/acpi-build.c        | 20 --------------------
 include/hw/acpi/aml-build.h | 10 ++++++++++
 3 files changed, 22 insertions(+), 20 deletions(-)

On Sat, Apr 23, 2016 at 05:06:35PM +0800, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Move acpi_build_srat_memory to common place so that it could be reused
> by ARM.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  hw/acpi/aml-build.c         | 12 ++++++++++++
>  hw/i386/acpi-build.c        | 20 --------------------
>  include/hw/acpi/aml-build.h | 10 ++++++++++
>  3 files changed, 22 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index ab89ca6..d167003 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1563,3 +1563,15 @@ build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets,
>      build_header(linker, table_data,
>                   (void *)rsdt, "RSDT", rsdt_len, 1, oem_id, oem_table_id);
>  }
> +
> +void acpi_build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
> +                            uint64_t len, int node, MemoryAffinityFlags flags)

It looks like functions like these in hw/acpi/aml-build.c usually start
with 'build_' not 'acpi_'

> +{
> +    numamem->type = ACPI_SRAT_MEMORY;
> +    numamem->length = sizeof(*numamem);
> +    memset(numamem->proximity, 0, 4);

This memset thing is still weird...

> +    numamem->proximity[0] = node;
> +    numamem->flags = cpu_to_le32(flags);
> +    numamem->base_addr = cpu_to_le64(base);
> +    numamem->range_length = cpu_to_le64(len);
> +}
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 9ae4c0d..cd93825 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2427,26 +2427,6 @@ build_tpm2(GArray *table_data, GArray *linker)
>                   (void *)tpm2_ptr, "TPM2", sizeof(*tpm2_ptr), 4, NULL, NULL);
>  }
>  
> -typedef enum {
> -    MEM_AFFINITY_NOFLAGS      = 0,
> -    MEM_AFFINITY_ENABLED      = (1 << 0),
> -    MEM_AFFINITY_HOTPLUGGABLE = (1 << 1),
> -    MEM_AFFINITY_NON_VOLATILE = (1 << 2),
> -} MemoryAffinityFlags;
> -
> -static void
> -acpi_build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
> -                       uint64_t len, int node, MemoryAffinityFlags flags)
> -{
> -    numamem->type = ACPI_SRAT_MEMORY;
> -    numamem->length = sizeof(*numamem);
> -    memset(numamem->proximity, 0, 4);
> -    numamem->proximity[0] = node;
> -    numamem->flags = cpu_to_le32(flags);
> -    numamem->base_addr = cpu_to_le64(base);
> -    numamem->range_length = cpu_to_le64(len);
> -}
> -
>  static void
>  build_srat(GArray *table_data, GArray *linker, MachineState *machine)
>  {
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 2c994b3..d8f9fca 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -198,6 +198,13 @@ typedef enum {
>      AML_PULL_NONE = 3,
>  } AmlPinConfig;
>  
> +typedef enum {
> +    MEM_AFFINITY_NOFLAGS      = 0,
> +    MEM_AFFINITY_ENABLED      = (1 << 0),
> +    MEM_AFFINITY_HOTPLUGGABLE = (1 << 1),
> +    MEM_AFFINITY_NON_VOLATILE = (1 << 2),
> +} MemoryAffinityFlags;
> +
>  typedef
>  struct AcpiBuildTables {
>      GArray *table_data;
> @@ -372,4 +379,7 @@ int
>  build_append_named_dword(GArray *array, const char *name_format, ...)
>  GCC_FMT_ATTR(2, 3);
>  
> +void acpi_build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
> +                            uint64_t len, int node, MemoryAffinityFlags flags);
> +
>  #endif
> -- 
> 2.0.4
> 
> 
>

Otherwise looks good to me.

drew

On 2016/4/25 1:00, Andrew Jones wrote:
> On Sat, Apr 23, 2016 at 05:06:35PM +0800, Shannon Zhao wrote:
>> > From: Shannon Zhao <shannon.zhao@linaro.org>
>> > 
>> > Move acpi_build_srat_memory to common place so that it could be reused
>> > by ARM.
>> > 
>> > Cc: Michael S. Tsirkin <mst@redhat.com>
>> > Cc: Igor Mammedov <imammedo@redhat.com>
>> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> > ---
>> >  hw/acpi/aml-build.c         | 12 ++++++++++++
>> >  hw/i386/acpi-build.c        | 20 --------------------
>> >  include/hw/acpi/aml-build.h | 10 ++++++++++
>> >  3 files changed, 22 insertions(+), 20 deletions(-)
>> > 
>> > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>> > index ab89ca6..d167003 100644
>> > --- a/hw/acpi/aml-build.c
>> > +++ b/hw/acpi/aml-build.c
>> > @@ -1563,3 +1563,15 @@ build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets,
>> >      build_header(linker, table_data,
>> >                   (void *)rsdt, "RSDT", rsdt_len, 1, oem_id, oem_table_id);
>> >  }
>> > +
>> > +void acpi_build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
>> > +                            uint64_t len, int node, MemoryAffinityFlags flags)
> It looks like functions like these in hw/acpi/aml-build.c usually start
> with 'build_' not 'acpi_'
> 
Ok, I will rename it to build_acpi_srat_memory.

>> > +{
>> > +    numamem->type = ACPI_SRAT_MEMORY;
>> > +    numamem->length = sizeof(*numamem);
>> > +    memset(numamem->proximity, 0, 4);
> This memset thing is still weird...
> 
I will add a patch before this one to fix the definition of proximity
and use uint32_t as you said before.

Thanks,