Re: [Qemu-devel] [PATCH v1 1/7] hmat acpi: Build Memory Subsystem Address Range Structre(s) in ACPI HMAT

[Liu, Jingqi]

On Tue, May 15, 2018 10:36 PM, Igor Mammedov <address@hidden> wrote:

> On Wed,  9 May 2018 16:34:29 +0800
> Liu Jingqi <address@hidden> wrote:
> 
> > HMAT is defined in ACPI 6.2: 5.2.27 Heterogeneous Memory Attribute Table
> (HMAT).
> > The specification references below link:
> > http://www.uefi.org/sites/default/files/resources/ACPI_6_2.pdf
> >
> > It describes the memory attributes, such as memory side cache
> > attributes and bandwidth and latency details, related to the System
> > Physical Address (SPA) Memory Ranges. The software is expected to use
> > this information as hint for optimization.
> >
> > This structure describes the System Physical Address(SPA) range
> > occupied by memory subsystem and its associativity with processor
> > proximity domain as well as hint for memory usage.
> >
> > Signed-off-by: Liu Jingqi <address@hidden>
> > ---
> >  default-configs/x86_64-softmmu.mak |   1 +
> >  hw/acpi/Makefile.objs              |   1 +
> >  hw/acpi/hmat.c                     | 174
> +++++++++++++++++++++++++++++++++++++
> >  hw/acpi/hmat.h                     |  75 ++++++++++++++++
> >  hw/i386/acpi-build.c               |   3 +
> >  5 files changed, 254 insertions(+)
> >  create mode 100644 hw/acpi/hmat.c
> >  create mode 100644 hw/acpi/hmat.h
> >
> > diff --git a/default-configs/x86_64-softmmu.mak
> > b/default-configs/x86_64-softmmu.mak
> > index 0390b43..3b4a37d 100644
> > --- a/default-configs/x86_64-softmmu.mak
> > +++ b/default-configs/x86_64-softmmu.mak
> > @@ -66,3 +66,4 @@ CONFIG_I2C=y
> >  CONFIG_SEV=$(CONFIG_KVM)
> >  CONFIG_VTD=y
> >  CONFIG_AMD_IOMMU=y
> > +CONFIG_ACPI_HMAT=y
> > diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs index
> > 11c35bc..21889fd 100644
> > --- a/hw/acpi/Makefile.objs
> > +++ b/hw/acpi/Makefile.objs
> > @@ -6,6 +6,7 @@ common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) +=
> > memory_hotplug.o
> >  common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu.o
> >  common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
> >  common-obj-$(CONFIG_ACPI_VMGENID) += vmgenid.o
> > +common-obj-$(CONFIG_ACPI_HMAT) += hmat.o
> >  common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o
> >
> >  common-obj-y += acpi_interface.o
> > diff --git a/hw/acpi/hmat.c b/hw/acpi/hmat.c new file mode 100644
> > index 0000000..bca1fbb
> > --- /dev/null
> > +++ b/hw/acpi/hmat.c
> > @@ -0,0 +1,174 @@
> > +/*
> > + * HMAT ACPI Implementation
> > + *
> > + * Copyright(C) 2018 Intel Corporation.
> > + *
> > + * Author:
> > + *  Liu jingqi <address@hidden>
> > + *
> > + * HMAT is defined in ACPI 6.2.
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see
> > +<http://www.gnu.org/licenses/>  */
> > +
> > +#include "unistd.h"
> > +#include "fcntl.h"
> > +#include "qemu/osdep.h"
> > +#include "sysemu/numa.h"
> > +#include "hw/i386/pc.h"
> > +#include "hw/acpi/acpi.h"
> > +#include "hw/acpi/hmat.h"
> > +#include "hw/acpi/aml-build.h"
> > +#include "hw/nvram/fw_cfg.h"
> > +#include "hw/acpi/bios-linker-loader.h"
> > +
> > +#define HOLE_640K_START  (640 * 1024)
> > +#define HOLE_640K_END    (1024 * 1024)
> > +
> > +uint32_t initiator_pxm[MAX_NODES], target_pxm[MAX_NODES]; uint32_t
> > +num_initiator = 0, num_target = 0;
> > +
> > +static void hmat_build_spa_info(AcpiHmatSpaRange *spa,
> > +                                uint64_t base, uint64_t length, int
> > +node) {
> > +    int i;
> > +
> > +    spa->type       = ACPI_HMAT_SPA;
> > +    spa->length     = sizeof(*spa);
> > +    spa->spa_base   = base;
> > +    spa->spa_length = length;
> all of above will break on big-endian host, that's one of the reasons we 
> prefer
> new code/tables to use build_append_foo() API.
> 
> Pls rewrite it using preferred build_append_int_noprefix() API,
> build_amd_iommu() can serve as an example.
> 
> As comments above fields use exact field names from spec tables so one could
> easily match spec vs code.
> 
Thanks for your reviewing.
I will use preferred build_append_int_noprefix() API in next version.
Jingqi Liu