Re: [PATCH v10 3/4] hw/i386: hook sp-mem into the pc machine plug path

[Igor Mammedov <imammedo@redhat.com>]

On Fri, 5 Jun 2026 18:46:08 +0800
fanhuang <FangSheng.Huang@amd.com> wrote:

> Add the pc machine hookup for TYPE_SP_MEM so each sp-mem instance is
> placed by the memory-device framework and reported to the guest as
> E820_SOFT_RESERVED.
> 
> Signed-off-by: FangSheng Huang <FangSheng.Huang@amd.com>
> ---
>  hw/i386/e820_memory_layout.h | 11 +++++-----
>  hw/i386/pc.c                 | 42 ++++++++++++++++++++++++++++++++++++
>  hw/i386/Kconfig              |  2 ++
>  3 files changed, 50 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/i386/e820_memory_layout.h b/hw/i386/e820_memory_layout.h
> index b50acfa201..6ef169db9c 100644
> --- a/hw/i386/e820_memory_layout.h
> +++ b/hw/i386/e820_memory_layout.h
> @@ -10,11 +10,12 @@
>  #define HW_I386_E820_MEMORY_LAYOUT_H
>  
>  /* e820 types */
> -#define E820_RAM        1
> -#define E820_RESERVED   2
> -#define E820_ACPI       3
> -#define E820_NVS        4
> -#define E820_UNUSABLE   5
> +#define E820_RAM           1
> +#define E820_RESERVED      2
> +#define E820_ACPI          3
> +#define E820_NVS           4
> +#define E820_UNUSABLE      5
> +#define E820_SOFT_RESERVED 0xefffffff
>  
>  struct e820_entry {
>      uint64_t address;
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 2ecad3c503..b538f49f23 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -63,6 +63,7 @@
>  #include "hw/i386/kvm/xen_gnttab.h"
>  #include "hw/i386/kvm/xen_xenstore.h"
>  #include "hw/mem/memory-device.h"
> +#include "hw/mem/sp-mem.h"
>  #include "e820_memory_layout.h"
>  #include "trace.h"
>  #include "sev.h"
> @@ -1283,11 +1284,49 @@ static void pc_hv_balloon_plug(HotplugHandler *hotplug_dev,
>      memory_device_plug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev));
>  }
>  
> +static void pc_sp_mem_pre_plug(HotplugHandler *hotplug_dev,
> +                               DeviceState *dev, Error **errp)
> +{
> +    MachineState *ms = MACHINE(hotplug_dev);
> +    SpMemDevice *spm = SP_MEM(dev);
> +

> +    if (host_memory_backend_is_mapped(spm->hostmem)) {
> +        error_setg(errp, "memory backend '%s' is already in use",
> +                   object_get_canonical_path_component(OBJECT(spm->hostmem)));
> +        return;
> +    }
this belongs to sp_mem realize

> +    if (ms->numa_state && spm->node >= ms->numa_state->num_nodes) {
> +        error_setg(errp,
> +                   "'node' property value %" PRIu32
> +                   " exceeds the number of NUMA nodes (%d)",
> +                   spm->node, ms->numa_state->num_nodes);
> +        return;
> +    }

I think this the right place for this, despite what other mems do.
Perhaps it is worth to consolidate/cleanup 1st. 

I'd put it in memory_device_pre_plug():
       if has_node_prop:
          .....

> +    memory_device_pre_plug(MEMORY_DEVICE(dev), ms, errp);
> +}
> +
> +static void pc_sp_mem_plug(HotplugHandler *hotplug_dev,
> +                               DeviceState *dev, Error **errp)
> +{
> +    SpMemDevice *spm = SP_MEM(dev);
> +    MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(MEMORY_DEVICE(dev));
> +    uint64_t addr, size;
> +
> +    host_memory_backend_set_mapped(spm->hostmem, true);
dup? see above.

> +    memory_device_plug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev));
> +
> +    addr = mdc->get_addr(MEMORY_DEVICE(dev));
> +    size = memory_region_size(host_memory_backend_get_memory(spm->hostmem));
> +    e820_add_entry(addr, size, E820_SOFT_RESERVED);
> +}
> +
>  static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>                                            DeviceState *dev, Error **errp)
>  {
>      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>          pc_memory_pre_plug(hotplug_dev, dev, errp);
> +    } else if (object_dynamic_cast(OBJECT(dev), TYPE_SP_MEM)) {
> +        pc_sp_mem_pre_plug(hotplug_dev, dev, errp);
>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
>          x86_cpu_pre_plug(hotplug_dev, dev, errp);
>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
> @@ -1324,6 +1363,8 @@ static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev,
>  {
>      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>          pc_memory_plug(hotplug_dev, dev, errp);
> +    } else if (object_dynamic_cast(OBJECT(dev), TYPE_SP_MEM)) {
> +        pc_sp_mem_plug(hotplug_dev, dev, errp);
>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
>          x86_cpu_plug(hotplug_dev, dev, errp);
>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
> @@ -1368,6 +1409,7 @@ static HotplugHandler *pc_get_hotplug_handler(MachineState *machine,
>                                               DeviceState *dev)
>  {
>      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
> +        object_dynamic_cast(OBJECT(dev), TYPE_SP_MEM) ||
>          object_dynamic_cast(OBJECT(dev), TYPE_CPU) ||
>          object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI) ||
>          object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI) ||
> diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
> index 12473acaa7..e27d8816e5 100644
> --- a/hw/i386/Kconfig
> +++ b/hw/i386/Kconfig
> @@ -84,6 +84,7 @@ config I440FX
>      select PCI_I440FX
>      select PIIX
>      select DIMM
> +    select SP_MEM
>      select SMBIOS
>      select SMBIOS_LEGACY
>      select FW_CFG_DMA
> @@ -113,6 +114,7 @@ config Q35
>      select LPC_ICH9
>      select AHCI_ICH9
>      select DIMM
> +    select SP_MEM
>      select SMBIOS
>      select FW_CFG_DMA
>