[v2,01/11] hw/acpi: Move constant definitions to header files

From: Sebastien Boeuf <sebastien.boeuf@intel.com>

By moving the definition of memory hotplug related constants used
by ACPI for both CPU and memory, this commits allows those to be
used from  other parts of the code.

Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
 hw/acpi/memory_hotplug.c         | 26 --------------------------
 include/hw/acpi/memory_hotplug.h | 26 ++++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 26 deletions(-)

Hi,

On 3/8/19 12:42 PM, Shameer Kolothum wrote:
> From: Sebastien Boeuf <sebastien.boeuf@intel.com>
> 
> By moving the definition of memory hotplug related constants used
> by ACPI for both CPU and memory, this commits allows those to be
> used from  other parts of the code.

Maybe elaborate on where you intend to use them.

> 
> Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
>  hw/acpi/memory_hotplug.c         | 26 --------------------------
>  include/hw/acpi/memory_hotplug.h | 26 ++++++++++++++++++++++++++
>  2 files changed, 26 insertions(+), 26 deletions(-)
> 
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index 921cad2..a6beb10 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -8,32 +8,6 @@
>  #include "qapi/error.h"
>  #include "qapi/qapi-events-misc.h"
>  
> -#define MEMORY_SLOTS_NUMBER          "MDNR"
> -#define MEMORY_HOTPLUG_IO_REGION     "HPMR"
> -#define MEMORY_SLOT_ADDR_LOW         "MRBL"
> -#define MEMORY_SLOT_ADDR_HIGH        "MRBH"
> -#define MEMORY_SLOT_SIZE_LOW         "MRLL"
> -#define MEMORY_SLOT_SIZE_HIGH        "MRLH"
> -#define MEMORY_SLOT_PROXIMITY        "MPX"
> -#define MEMORY_SLOT_ENABLED          "MES"
> -#define MEMORY_SLOT_INSERT_EVENT     "MINS"
> -#define MEMORY_SLOT_REMOVE_EVENT     "MRMV"
> -#define MEMORY_SLOT_EJECT            "MEJ"
> -#define MEMORY_SLOT_SLECTOR          "MSEL"
> -#define MEMORY_SLOT_OST_EVENT        "MOEV"
> -#define MEMORY_SLOT_OST_STATUS       "MOSC"
> -#define MEMORY_SLOT_LOCK             "MLCK"
> -#define MEMORY_SLOT_STATUS_METHOD    "MRST"
> -#define MEMORY_SLOT_CRS_METHOD       "MCRS"
> -#define MEMORY_SLOT_OST_METHOD       "MOST"
> -#define MEMORY_SLOT_PROXIMITY_METHOD "MPXM"
> -#define MEMORY_SLOT_EJECT_METHOD     "MEJ0"
> -#define MEMORY_SLOT_NOTIFY_METHOD    "MTFY"
> -#define MEMORY_SLOT_SCAN_METHOD      "MSCN"
> -#define MEMORY_HOTPLUG_DEVICE        "MHPD"
> -#define MEMORY_HOTPLUG_IO_LEN         24
> -#define MEMORY_DEVICES_CONTAINER     "\\_SB.MHPC"

Do we really need to expose all of them. I just can see
MEMORY_SLOT_SCAN_METHOD used in hw/acpi/ged.c? Maybe I missed some though?

Also at the beginning I tried to find some specification details for
those stuff but I failed. It would be helpful for a non specialist
reader to add a comment that helps to understand what is part of a
specification (link?) and what is arbitrarily defined.

Thanks

Eric
> -
>  static uint16_t memhp_io_base;
>  
>  static ACPIOSTInfo *acpi_memory_device_status(int slot, MemStatus *mdev)
> diff --git a/include/hw/acpi/memory_hotplug.h b/include/hw/acpi/memory_hotplug.h
> index 77c6576..fbfcbe6 100644
> --- a/include/hw/acpi/memory_hotplug.h
> +++ b/include/hw/acpi/memory_hotplug.h
> @@ -5,6 +5,32 @@
>  #include "hw/acpi/acpi.h"
>  #include "hw/acpi/aml-build.h"
>  
> +#define MEMORY_SLOTS_NUMBER          "MDNR"
> +#define MEMORY_HOTPLUG_IO_REGION     "HPMR"
> +#define MEMORY_SLOT_ADDR_LOW         "MRBL"
> +#define MEMORY_SLOT_ADDR_HIGH        "MRBH"
> +#define MEMORY_SLOT_SIZE_LOW         "MRLL"
> +#define MEMORY_SLOT_SIZE_HIGH        "MRLH"
> +#define MEMORY_SLOT_PROXIMITY        "MPX"
> +#define MEMORY_SLOT_ENABLED          "MES"
> +#define MEMORY_SLOT_INSERT_EVENT     "MINS"
> +#define MEMORY_SLOT_REMOVE_EVENT     "MRMV"
> +#define MEMORY_SLOT_EJECT            "MEJ"
> +#define MEMORY_SLOT_SLECTOR          "MSEL"
> +#define MEMORY_SLOT_OST_EVENT        "MOEV"
> +#define MEMORY_SLOT_OST_STATUS       "MOSC"
> +#define MEMORY_SLOT_LOCK             "MLCK"
> +#define MEMORY_SLOT_STATUS_METHOD    "MRST"
> +#define MEMORY_SLOT_CRS_METHOD       "MCRS"
> +#define MEMORY_SLOT_OST_METHOD       "MOST"
> +#define MEMORY_SLOT_PROXIMITY_METHOD "MPXM"
> +#define MEMORY_SLOT_EJECT_METHOD     "MEJ0"
> +#define MEMORY_SLOT_NOTIFY_METHOD    "MTFY"
> +#define MEMORY_SLOT_SCAN_METHOD      "MSCN"
> +#define MEMORY_HOTPLUG_DEVICE        "MHPD"
> +#define MEMORY_HOTPLUG_IO_LEN         24
> +#define MEMORY_DEVICES_CONTAINER     "\\_SB.MHPC"
> +
>  /**
>   * MemStatus:
>   * @is_removing: the memory device in slot has been requested to be ejected.
>

> -----Original Message-----
> From: Auger Eric [mailto:eric.auger@redhat.com]
> Sent: 08 March 2019 16:09
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
> qemu-devel@nongnu.org; qemu-arm@nongnu.org; imammedo@redhat.com;
> peter.maydell@linaro.org; shannon.zhaosl@gmail.com;
> sameo@linux.intel.com; sebastien.boeuf@intel.com
> Cc: Linuxarm <linuxarm@huawei.com>; xuwei (O) <xuwei5@huawei.com>
> Subject: Re: [PATCH v2 01/11] hw/acpi: Move constant definitions to header
> files
> 
> Hi,
> 
> On 3/8/19 12:42 PM, Shameer Kolothum wrote:
> > From: Sebastien Boeuf <sebastien.boeuf@intel.com>
> >
> > By moving the definition of memory hotplug related constants used
> > by ACPI for both CPU and memory, this commits allows those to be
> > used from  other parts of the code.
> 
> Maybe elaborate on where you intend to use them.

Ok.
 
> >
> > Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
> > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> > ---
> >  hw/acpi/memory_hotplug.c         | 26 --------------------------
> >  include/hw/acpi/memory_hotplug.h | 26 ++++++++++++++++++++++++++
> >  2 files changed, 26 insertions(+), 26 deletions(-)
> >
> > diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> > index 921cad2..a6beb10 100644
> > --- a/hw/acpi/memory_hotplug.c
> > +++ b/hw/acpi/memory_hotplug.c
> > @@ -8,32 +8,6 @@
> >  #include "qapi/error.h"
> >  #include "qapi/qapi-events-misc.h"
> >
> > -#define MEMORY_SLOTS_NUMBER          "MDNR"
> > -#define MEMORY_HOTPLUG_IO_REGION     "HPMR"
> > -#define MEMORY_SLOT_ADDR_LOW         "MRBL"
> > -#define MEMORY_SLOT_ADDR_HIGH        "MRBH"
> > -#define MEMORY_SLOT_SIZE_LOW         "MRLL"
> > -#define MEMORY_SLOT_SIZE_HIGH        "MRLH"
> > -#define MEMORY_SLOT_PROXIMITY        "MPX"
> > -#define MEMORY_SLOT_ENABLED          "MES"
> > -#define MEMORY_SLOT_INSERT_EVENT     "MINS"
> > -#define MEMORY_SLOT_REMOVE_EVENT     "MRMV"
> > -#define MEMORY_SLOT_EJECT            "MEJ"
> > -#define MEMORY_SLOT_SLECTOR          "MSEL"
> > -#define MEMORY_SLOT_OST_EVENT        "MOEV"
> > -#define MEMORY_SLOT_OST_STATUS       "MOSC"
> > -#define MEMORY_SLOT_LOCK             "MLCK"
> > -#define MEMORY_SLOT_STATUS_METHOD    "MRST"
> > -#define MEMORY_SLOT_CRS_METHOD       "MCRS"
> > -#define MEMORY_SLOT_OST_METHOD       "MOST"
> > -#define MEMORY_SLOT_PROXIMITY_METHOD "MPXM"
> > -#define MEMORY_SLOT_EJECT_METHOD     "MEJ0"
> > -#define MEMORY_SLOT_NOTIFY_METHOD    "MTFY"
> > -#define MEMORY_SLOT_SCAN_METHOD      "MSCN"
> > -#define MEMORY_HOTPLUG_DEVICE        "MHPD"
> > -#define MEMORY_HOTPLUG_IO_LEN         24
> > -#define MEMORY_DEVICES_CONTAINER     "\\_SB.MHPC"
> 
> Do we really need to expose all of them. I just can see
> MEMORY_SLOT_SCAN_METHOD used in hw/acpi/ged.c? Maybe I missed some
> though?

That is right. This was taken from the Nemu approach and I thought it might be
useful in future changes. But if there is no major motivation in keeping this in header,
I may skip this patch and just move the MEMORY_SLOT_SCAN_METHOD.

> Also at the beginning I tried to find some specification details for
> those stuff but I failed. It would be helpful for a non specialist
> reader to add a comment that helps to understand what is part of a
> specification (link?) and what is arbitrarily defined.

Ok. I could only find this Nemu wiki description here
https://github.com/intel/nemu/wiki/Memory-hotplug

Not sure Qemu or ACPI has a similar documentation elsewhere.

Thanks,
Shameer

> Thanks
> 
> Eric
> > -
> >  static uint16_t memhp_io_base;
> >
> >  static ACPIOSTInfo *acpi_memory_device_status(int slot, MemStatus
> *mdev)
> > diff --git a/include/hw/acpi/memory_hotplug.h
> b/include/hw/acpi/memory_hotplug.h
> > index 77c6576..fbfcbe6 100644
> > --- a/include/hw/acpi/memory_hotplug.h
> > +++ b/include/hw/acpi/memory_hotplug.h
> > @@ -5,6 +5,32 @@
> >  #include "hw/acpi/acpi.h"
> >  #include "hw/acpi/aml-build.h"
> >
> > +#define MEMORY_SLOTS_NUMBER          "MDNR"
> > +#define MEMORY_HOTPLUG_IO_REGION     "HPMR"
> > +#define MEMORY_SLOT_ADDR_LOW         "MRBL"
> > +#define MEMORY_SLOT_ADDR_HIGH        "MRBH"
> > +#define MEMORY_SLOT_SIZE_LOW         "MRLL"
> > +#define MEMORY_SLOT_SIZE_HIGH        "MRLH"
> > +#define MEMORY_SLOT_PROXIMITY        "MPX"
> > +#define MEMORY_SLOT_ENABLED          "MES"
> > +#define MEMORY_SLOT_INSERT_EVENT     "MINS"
> > +#define MEMORY_SLOT_REMOVE_EVENT     "MRMV"
> > +#define MEMORY_SLOT_EJECT            "MEJ"
> > +#define MEMORY_SLOT_SLECTOR          "MSEL"
> > +#define MEMORY_SLOT_OST_EVENT        "MOEV"
> > +#define MEMORY_SLOT_OST_STATUS       "MOSC"
> > +#define MEMORY_SLOT_LOCK             "MLCK"
> > +#define MEMORY_SLOT_STATUS_METHOD    "MRST"
> > +#define MEMORY_SLOT_CRS_METHOD       "MCRS"
> > +#define MEMORY_SLOT_OST_METHOD       "MOST"
> > +#define MEMORY_SLOT_PROXIMITY_METHOD "MPXM"
> > +#define MEMORY_SLOT_EJECT_METHOD     "MEJ0"
> > +#define MEMORY_SLOT_NOTIFY_METHOD    "MTFY"
> > +#define MEMORY_SLOT_SCAN_METHOD      "MSCN"
> > +#define MEMORY_HOTPLUG_DEVICE        "MHPD"
> > +#define MEMORY_HOTPLUG_IO_LEN         24
> > +#define MEMORY_DEVICES_CONTAINER     "\\_SB.MHPC"
> > +
> >  /**
> >   * MemStatus:
> >   * @is_removing: the memory device in slot has been requested to be
> ejected.
> >