diff mbox series

[v1,2/3] memory-device: Factor out device memory initialization into memory_devices_init()

Message ID 20230523185144.533592-3-david@redhat.com
State New
Headers show
Series memory-device: Some cleanups | expand

Commit Message

David Hildenbrand May 23, 2023, 6:51 p.m. UTC
Let's factor the common setup out, to prepare for further changes.

On arm64, we'll add the subregion to system RAM now earlier -- which
shouldn't matter, because the system RAM memory region should already be
alive at that point.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 hw/arm/virt.c                  |  9 +--------
 hw/i386/pc.c                   | 17 ++++++-----------
 hw/loongarch/virt.c            | 14 ++++----------
 hw/mem/memory-device.c         | 20 ++++++++++++++++++++
 hw/ppc/spapr.c                 | 15 ++++++---------
 include/hw/mem/memory-device.h |  2 ++
 6 files changed, 39 insertions(+), 38 deletions(-)

Comments

gaosong May 25, 2023, 12:32 p.m. UTC | #1
在 2023/5/24 上午2:51, David Hildenbrand 写道:
> Let's factor the common setup out, to prepare for further changes.
>
> On arm64, we'll add the subregion to system RAM now earlier -- which
> shouldn't matter, because the system RAM memory region should already be
> alive at that point.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>   hw/arm/virt.c                  |  9 +--------
>   hw/i386/pc.c                   | 17 ++++++-----------
>   hw/loongarch/virt.c            | 14 ++++----------
>   hw/mem/memory-device.c         | 20 ++++++++++++++++++++
>   hw/ppc/spapr.c                 | 15 ++++++---------
>   include/hw/mem/memory-device.h |  2 ++
>   6 files changed, 39 insertions(+), 38 deletions(-)
For LoongArch

Tested-by: Song Gao <gaosong@loongson.cn>

Thanks.
Song Gao
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index b99ae18501..284f45d998 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1816,10 +1816,7 @@ static void virt_set_memmap(VirtMachineState *vms, int pa_bits)
>       virt_set_high_memmap(vms, base, pa_bits);
>   
>       if (device_memory_size > 0) {
> -        ms->device_memory = g_malloc0(sizeof(*ms->device_memory));
> -        ms->device_memory->base = device_memory_base;
> -        memory_region_init(&ms->device_memory->mr, OBJECT(vms),
> -                           "device-memory", device_memory_size);
> +        memory_devices_init(ms, device_memory_base, device_memory_size);
>       }
>   }
>   
> @@ -2260,10 +2257,6 @@ static void machvirt_init(MachineState *machine)
>   
>       memory_region_add_subregion(sysmem, vms->memmap[VIRT_MEM].base,
>                                   machine->ram);
> -    if (machine->device_memory) {
> -        memory_region_add_subregion(sysmem, machine->device_memory->base,
> -                                    &machine->device_memory->mr);
> -    }
>   
>       virt_flash_fdt(vms, sysmem, secure_sysmem ?: sysmem);
>   
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index bb62c994fa..9d215df92e 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1039,13 +1039,11 @@ void pc_memory_init(PCMachineState *pcms,
>           exit(EXIT_FAILURE);
>       }
>   
> -    /* always allocate the device memory information */
> -    machine->device_memory = g_malloc0(sizeof(*machine->device_memory));
> -
>       /* initialize device memory address space */
>       if (pcmc->has_reserved_memory &&
>           (machine->ram_size < machine->maxram_size)) {
>           ram_addr_t device_mem_size;
> +        hwaddr device_mem_base;
>   
>           if (machine->ram_slots > ACPI_MAX_RAM_SLOTS) {
>               error_report("unsupported amount of memory slots: %"PRIu64,
> @@ -1060,19 +1058,16 @@ void pc_memory_init(PCMachineState *pcms,
>               exit(EXIT_FAILURE);
>           }
>   
> -        pc_get_device_memory_range(pcms, &machine->device_memory->base, &device_mem_size);
> +        pc_get_device_memory_range(pcms, &device_mem_base, &device_mem_size);
>   
> -        if ((machine->device_memory->base + device_mem_size) <
> -            device_mem_size) {
> +        if (device_mem_base + device_mem_size < device_mem_size) {
>               error_report("unsupported amount of maximum memory: " RAM_ADDR_FMT,
>                            machine->maxram_size);
>               exit(EXIT_FAILURE);
>           }
> -
> -        memory_region_init(&machine->device_memory->mr, OBJECT(pcms),
> -                           "device-memory", device_mem_size);
> -        memory_region_add_subregion(system_memory, machine->device_memory->base,
> -                                    &machine->device_memory->mr);
> +        memory_devices_init(machine, device_mem_base, device_mem_size);
> +    } else {
> +        memory_devices_init(machine, 0, 0);
>       }
>   
>       if (pcms->cxl_devices_state.is_enabled) {
> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> index 2b7588e32a..2ccc90feb4 100644
> --- a/hw/loongarch/virt.c
> +++ b/hw/loongarch/virt.c
> @@ -45,7 +45,7 @@
>   #include "sysemu/block-backend.h"
>   #include "hw/block/flash.h"
>   #include "qemu/error-report.h"
> -
> +#include "hw/mem/memory-device.h"
>   
>   static void virt_flash_create(LoongArchMachineState *lams)
>   {
> @@ -804,8 +804,8 @@ static void loongarch_init(MachineState *machine)
>   
>       /* initialize device memory address space */
>       if (machine->ram_size < machine->maxram_size) {
> -        machine->device_memory = g_malloc0(sizeof(*machine->device_memory));
>           ram_addr_t device_mem_size = machine->maxram_size - machine->ram_size;
> +        hwaddr device_mem_base;
>   
>           if (machine->ram_slots > ACPI_MAX_RAM_SLOTS) {
>               error_report("unsupported amount of memory slots: %"PRIu64,
> @@ -820,14 +820,8 @@ static void loongarch_init(MachineState *machine)
>               exit(EXIT_FAILURE);
>           }
>           /* device memory base is the top of high memory address. */
> -        machine->device_memory->base = 0x90000000 + highram_size;
> -        machine->device_memory->base =
> -            ROUND_UP(machine->device_memory->base, 1 * GiB);
> -
> -        memory_region_init(&machine->device_memory->mr, OBJECT(lams),
> -                           "device-memory", device_mem_size);
> -        memory_region_add_subregion(address_space_mem, machine->device_memory->base,
> -                                    &machine->device_memory->mr);
> +        device_mem_base = ROUND_UP(0x90000000 + highram_size, 1 * GiB);
> +        memory_devices_init(machine, device_mem_base, device_mem_size);
>       }
>   
>       /* Add isa io region */
> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
> index 6c025b02c1..d99ceb621a 100644
> --- a/hw/mem/memory-device.c
> +++ b/hw/mem/memory-device.c
> @@ -17,6 +17,7 @@
>   #include "qemu/range.h"
>   #include "hw/virtio/vhost.h"
>   #include "sysemu/kvm.h"
> +#include "exec/address-spaces.h"
>   #include "trace.h"
>   
>   static gint memory_device_addr_sort(gconstpointer a, gconstpointer b)
> @@ -333,6 +334,25 @@ uint64_t memory_device_get_region_size(const MemoryDeviceState *md,
>       return memory_region_size(mr);
>   }
>   
> +void memory_devices_init(MachineState *ms, hwaddr base, uint64_t size)
> +{
> +    g_assert(!ms->device_memory);
> +    ms->device_memory = g_new0(DeviceMemoryState, 1);
> +    ms->device_memory->base = base;
> +
> +    /*
> +     * See memory_device_get_free_addr(): An empty device memory region
> +     * means "this machine supports memory devices, but they are not enabled".
> +     */
> +    if (size > 0) {
> +        memory_region_init(&ms->device_memory->mr, OBJECT(ms), "device-memory",
> +                           size);
> +        memory_region_add_subregion(get_system_memory(),
> +                                    ms->device_memory->base,
> +                                    &ms->device_memory->mr);
> +    }
> +}
> +
>   static const TypeInfo memory_device_info = {
>       .name          = TYPE_MEMORY_DEVICE,
>       .parent        = TYPE_INTERFACE,
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 1baea16c96..d66dc00ea5 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2866,12 +2866,11 @@ static void spapr_machine_init(MachineState *machine)
>       /* map RAM */
>       memory_region_add_subregion(sysmem, 0, machine->ram);
>   
> -    /* always allocate the device memory information */
> -    machine->device_memory = g_malloc0(sizeof(*machine->device_memory));
> -
>       /* initialize hotplug memory address space */
>       if (machine->ram_size < machine->maxram_size) {
>           ram_addr_t device_mem_size = machine->maxram_size - machine->ram_size;
> +        hwaddr device_mem_base;
> +
>           /*
>            * Limit the number of hotpluggable memory slots to half the number
>            * slots that KVM supports, leaving the other half for PCI and other
> @@ -2890,12 +2889,10 @@ static void spapr_machine_init(MachineState *machine)
>               exit(1);
>           }
>   
> -        machine->device_memory->base = ROUND_UP(machine->ram_size,
> -                                                SPAPR_DEVICE_MEM_ALIGN);
> -        memory_region_init(&machine->device_memory->mr, OBJECT(spapr),
> -                           "device-memory", device_mem_size);
> -        memory_region_add_subregion(sysmem, machine->device_memory->base,
> -                                    &machine->device_memory->mr);
> +        device_mem_base = ROUND_UP(machine->ram_size, SPAPR_DEVICE_MEM_ALIGN);
> +        memory_devices_init(machine, device_mem_base, device_mem_size);
> +    } else {
> +        memory_devices_init(machine, 0, 0);
>       }
>   
>       if (smc->dr_lmb_enabled) {
> diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h
> index 48d2611fc5..6e8a10e2f5 100644
> --- a/include/hw/mem/memory-device.h
> +++ b/include/hw/mem/memory-device.h
> @@ -16,6 +16,7 @@
>   #include "hw/qdev-core.h"
>   #include "qapi/qapi-types-machine.h"
>   #include "qom/object.h"
> +#include "exec/hwaddr.h"
>   
>   #define TYPE_MEMORY_DEVICE "memory-device"
>   
> @@ -113,5 +114,6 @@ void memory_device_plug(MemoryDeviceState *md, MachineState *ms);
>   void memory_device_unplug(MemoryDeviceState *md, MachineState *ms);
>   uint64_t memory_device_get_region_size(const MemoryDeviceState *md,
>                                          Error **errp);
> +void memory_devices_init(MachineState *ms, hwaddr base, uint64_t size);
>   
>   #endif
Philippe Mathieu-Daudé May 25, 2023, 1:30 p.m. UTC | #2
Hi David,

On 23/5/23 20:51, David Hildenbrand wrote:
> Let's factor the common setup out, to prepare for further changes.
> 
> On arm64, we'll add the subregion to system RAM now earlier -- which
> shouldn't matter, because the system RAM memory region should already be
> alive at that point.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>   hw/arm/virt.c                  |  9 +--------
>   hw/i386/pc.c                   | 17 ++++++-----------
>   hw/loongarch/virt.c            | 14 ++++----------
>   hw/mem/memory-device.c         | 20 ++++++++++++++++++++
>   hw/ppc/spapr.c                 | 15 ++++++---------
>   include/hw/mem/memory-device.h |  2 ++
>   6 files changed, 39 insertions(+), 38 deletions(-)

Split in boring 'first add method then use it for each arch'
would be easier to review.

> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
> index 6c025b02c1..d99ceb621a 100644
> --- a/hw/mem/memory-device.c
> +++ b/hw/mem/memory-device.c
> @@ -17,6 +17,7 @@
>   #include "qemu/range.h"
>   #include "hw/virtio/vhost.h"
>   #include "sysemu/kvm.h"
> +#include "exec/address-spaces.h"
>   #include "trace.h"
>   
>   static gint memory_device_addr_sort(gconstpointer a, gconstpointer b)
> @@ -333,6 +334,25 @@ uint64_t memory_device_get_region_size(const MemoryDeviceState *md,
>       return memory_region_size(mr);
>   }
>   
> +void memory_devices_init(MachineState *ms, hwaddr base, uint64_t size)
> +{
> +    g_assert(!ms->device_memory);
> +    ms->device_memory = g_new0(DeviceMemoryState, 1);
> +    ms->device_memory->base = base;
> +
> +    /*
> +     * See memory_device_get_free_addr(): An empty device memory region
> +     * means "this machine supports memory devices, but they are not enabled".
> +     */
> +    if (size > 0) {
> +        memory_region_init(&ms->device_memory->mr, OBJECT(ms), "device-memory",
> +                           size);
> +        memory_region_add_subregion(get_system_memory(),
> +                                    ms->device_memory->base,
> +                                    &ms->device_memory->mr);

What about always init/register and set if enabled?

   memory_region_set_enabled(&ms->device_memory->mr, size > 0);

Otherwise why allocate ms->device_memory?

> +    }
> +}
David Hildenbrand May 25, 2023, 1:40 p.m. UTC | #3
On 25.05.23 15:30, Philippe Mathieu-Daudé wrote:
> Hi David,
> 
> On 23/5/23 20:51, David Hildenbrand wrote:
>> Let's factor the common setup out, to prepare for further changes.
>>
>> On arm64, we'll add the subregion to system RAM now earlier -- which
>> shouldn't matter, because the system RAM memory region should already be
>> alive at that point.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>    hw/arm/virt.c                  |  9 +--------
>>    hw/i386/pc.c                   | 17 ++++++-----------
>>    hw/loongarch/virt.c            | 14 ++++----------
>>    hw/mem/memory-device.c         | 20 ++++++++++++++++++++
>>    hw/ppc/spapr.c                 | 15 ++++++---------
>>    include/hw/mem/memory-device.h |  2 ++
>>    6 files changed, 39 insertions(+), 38 deletions(-)
> 
> Split in boring 'first add method then use it for each arch'
> would be easier to review.

Right, I agree if I'd be touching more code (I consider this fairly 
minimal and straight-forward refactoring).

> 
>> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
>> index 6c025b02c1..d99ceb621a 100644
>> --- a/hw/mem/memory-device.c
>> +++ b/hw/mem/memory-device.c
>> @@ -17,6 +17,7 @@
>>    #include "qemu/range.h"
>>    #include "hw/virtio/vhost.h"
>>    #include "sysemu/kvm.h"
>> +#include "exec/address-spaces.h"
>>    #include "trace.h"
>>    
>>    static gint memory_device_addr_sort(gconstpointer a, gconstpointer b)
>> @@ -333,6 +334,25 @@ uint64_t memory_device_get_region_size(const MemoryDeviceState *md,
>>        return memory_region_size(mr);
>>    }
>>    
>> +void memory_devices_init(MachineState *ms, hwaddr base, uint64_t size)
>> +{
>> +    g_assert(!ms->device_memory);
>> +    ms->device_memory = g_new0(DeviceMemoryState, 1);
>> +    ms->device_memory->base = base;
>> +
>> +    /*
>> +     * See memory_device_get_free_addr(): An empty device memory region
>> +     * means "this machine supports memory devices, but they are not enabled".
>> +     */
>> +    if (size > 0) {
>> +        memory_region_init(&ms->device_memory->mr, OBJECT(ms), "device-memory",
>> +                           size);
>> +        memory_region_add_subregion(get_system_memory(),
>> +                                    ms->device_memory->base,
>> +                                    &ms->device_memory->mr);
> 
> What about always init/register and set if enabled?
> 
>     memory_region_set_enabled(&ms->device_memory->mr, size > 0);
> 
> Otherwise why allocate ms->device_memory?

We have right now:

ms->device_memory not allocated: no support
ms->device_memory->mr with size 0: not enabled
ms->device_memory->mr with size > 0: enabled

Let me see if initializing a memory region with size 0 (and adding a 
subregion with size 0) works.

Thanks!
David Hildenbrand May 26, 2023, 9:33 a.m. UTC | #4
On 25.05.23 15:30, Philippe Mathieu-Daudé wrote:
> Hi David,
> 
> On 23/5/23 20:51, David Hildenbrand wrote:
>> Let's factor the common setup out, to prepare for further changes.
>>
>> On arm64, we'll add the subregion to system RAM now earlier -- which
>> shouldn't matter, because the system RAM memory region should already be
>> alive at that point.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>    hw/arm/virt.c                  |  9 +--------
>>    hw/i386/pc.c                   | 17 ++++++-----------
>>    hw/loongarch/virt.c            | 14 ++++----------
>>    hw/mem/memory-device.c         | 20 ++++++++++++++++++++
>>    hw/ppc/spapr.c                 | 15 ++++++---------
>>    include/hw/mem/memory-device.h |  2 ++
>>    6 files changed, 39 insertions(+), 38 deletions(-)
> 
> Split in boring 'first add method then use it for each arch'
> would be easier to review.
> 
>> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
>> index 6c025b02c1..d99ceb621a 100644
>> --- a/hw/mem/memory-device.c
>> +++ b/hw/mem/memory-device.c
>> @@ -17,6 +17,7 @@
>>    #include "qemu/range.h"
>>    #include "hw/virtio/vhost.h"
>>    #include "sysemu/kvm.h"
>> +#include "exec/address-spaces.h"
>>    #include "trace.h"
>>    
>>    static gint memory_device_addr_sort(gconstpointer a, gconstpointer b)
>> @@ -333,6 +334,25 @@ uint64_t memory_device_get_region_size(const MemoryDeviceState *md,
>>        return memory_region_size(mr);
>>    }
>>    
>> +void memory_devices_init(MachineState *ms, hwaddr base, uint64_t size)
>> +{
>> +    g_assert(!ms->device_memory);
>> +    ms->device_memory = g_new0(DeviceMemoryState, 1);
>> +    ms->device_memory->base = base;
>> +
>> +    /*
>> +     * See memory_device_get_free_addr(): An empty device memory region
>> +     * means "this machine supports memory devices, but they are not enabled".
>> +     */
>> +    if (size > 0) {
>> +        memory_region_init(&ms->device_memory->mr, OBJECT(ms), "device-memory",
>> +                           size);
>> +        memory_region_add_subregion(get_system_memory(),
>> +                                    ms->device_memory->base,
>> +                                    &ms->device_memory->mr);
> 
> What about always init/register and set if enabled?
> 
>     memory_region_set_enabled(&ms->device_memory->mr, size > 0);
> 
> Otherwise why allocate ms->device_memory?

With

void memory_devices_init(MachineState *ms, hwaddr base, uint64_t size)
{
     g_assert(!ms->device_memory);
     ms->device_memory = g_new0(DeviceMemoryState, 1);
     ms->device_memory->base = base;

     /*
      * An empty region (size == 0) indicates that memory devices are supported
      * by the machine, but they are not enabled (see memory_device_pre_plug()).
      */
     memory_region_init(&ms->device_memory->mr, OBJECT(ms), "device-memory",
                        size);
     memory_region_set_enabled(&ms->device_memory->mr, !!size);
     memory_region_add_subregion(get_system_memory(), ms->device_memory->base,
                                 &ms->device_memory->mr);
}

"info mtree -D" on x86-64 with only "-m 2G" (no maxmem) will show that the
region is placed at address 0 and disabled:

memory-region: system
   0000000000000000-ffffffffffffffff (prio 0, i/o): system
     0000000000000000-0000000000000000 (prio 0, i/o): device-memory [disabled]
     0000000000000000-000000007fffffff (prio 0, ram): alias ram-below-4g @pc.ram 0000000000000000-000000007fffffff
     0000000000000000-ffffffffffffffff (prio -1, i/o): pci


Good enough for me.

However, I think we could just stop allocating ms->device_memory with size == 0 and
not care about the "not supported" case in memory_device_pre_plug(): this function should
only get called by a machine, and if the machine does not support memory devices, it is
to blame for calling that function after all. (and it will only affect the error message
after all)
diff mbox series

Patch

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index b99ae18501..284f45d998 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1816,10 +1816,7 @@  static void virt_set_memmap(VirtMachineState *vms, int pa_bits)
     virt_set_high_memmap(vms, base, pa_bits);
 
     if (device_memory_size > 0) {
-        ms->device_memory = g_malloc0(sizeof(*ms->device_memory));
-        ms->device_memory->base = device_memory_base;
-        memory_region_init(&ms->device_memory->mr, OBJECT(vms),
-                           "device-memory", device_memory_size);
+        memory_devices_init(ms, device_memory_base, device_memory_size);
     }
 }
 
@@ -2260,10 +2257,6 @@  static void machvirt_init(MachineState *machine)
 
     memory_region_add_subregion(sysmem, vms->memmap[VIRT_MEM].base,
                                 machine->ram);
-    if (machine->device_memory) {
-        memory_region_add_subregion(sysmem, machine->device_memory->base,
-                                    &machine->device_memory->mr);
-    }
 
     virt_flash_fdt(vms, sysmem, secure_sysmem ?: sysmem);
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index bb62c994fa..9d215df92e 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1039,13 +1039,11 @@  void pc_memory_init(PCMachineState *pcms,
         exit(EXIT_FAILURE);
     }
 
-    /* always allocate the device memory information */
-    machine->device_memory = g_malloc0(sizeof(*machine->device_memory));
-
     /* initialize device memory address space */
     if (pcmc->has_reserved_memory &&
         (machine->ram_size < machine->maxram_size)) {
         ram_addr_t device_mem_size;
+        hwaddr device_mem_base;
 
         if (machine->ram_slots > ACPI_MAX_RAM_SLOTS) {
             error_report("unsupported amount of memory slots: %"PRIu64,
@@ -1060,19 +1058,16 @@  void pc_memory_init(PCMachineState *pcms,
             exit(EXIT_FAILURE);
         }
 
-        pc_get_device_memory_range(pcms, &machine->device_memory->base, &device_mem_size);
+        pc_get_device_memory_range(pcms, &device_mem_base, &device_mem_size);
 
-        if ((machine->device_memory->base + device_mem_size) <
-            device_mem_size) {
+        if (device_mem_base + device_mem_size < device_mem_size) {
             error_report("unsupported amount of maximum memory: " RAM_ADDR_FMT,
                          machine->maxram_size);
             exit(EXIT_FAILURE);
         }
-
-        memory_region_init(&machine->device_memory->mr, OBJECT(pcms),
-                           "device-memory", device_mem_size);
-        memory_region_add_subregion(system_memory, machine->device_memory->base,
-                                    &machine->device_memory->mr);
+        memory_devices_init(machine, device_mem_base, device_mem_size);
+    } else {
+        memory_devices_init(machine, 0, 0);
     }
 
     if (pcms->cxl_devices_state.is_enabled) {
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index 2b7588e32a..2ccc90feb4 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -45,7 +45,7 @@ 
 #include "sysemu/block-backend.h"
 #include "hw/block/flash.h"
 #include "qemu/error-report.h"
-
+#include "hw/mem/memory-device.h"
 
 static void virt_flash_create(LoongArchMachineState *lams)
 {
@@ -804,8 +804,8 @@  static void loongarch_init(MachineState *machine)
 
     /* initialize device memory address space */
     if (machine->ram_size < machine->maxram_size) {
-        machine->device_memory = g_malloc0(sizeof(*machine->device_memory));
         ram_addr_t device_mem_size = machine->maxram_size - machine->ram_size;
+        hwaddr device_mem_base;
 
         if (machine->ram_slots > ACPI_MAX_RAM_SLOTS) {
             error_report("unsupported amount of memory slots: %"PRIu64,
@@ -820,14 +820,8 @@  static void loongarch_init(MachineState *machine)
             exit(EXIT_FAILURE);
         }
         /* device memory base is the top of high memory address. */
-        machine->device_memory->base = 0x90000000 + highram_size;
-        machine->device_memory->base =
-            ROUND_UP(machine->device_memory->base, 1 * GiB);
-
-        memory_region_init(&machine->device_memory->mr, OBJECT(lams),
-                           "device-memory", device_mem_size);
-        memory_region_add_subregion(address_space_mem, machine->device_memory->base,
-                                    &machine->device_memory->mr);
+        device_mem_base = ROUND_UP(0x90000000 + highram_size, 1 * GiB);
+        memory_devices_init(machine, device_mem_base, device_mem_size);
     }
 
     /* Add isa io region */
diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
index 6c025b02c1..d99ceb621a 100644
--- a/hw/mem/memory-device.c
+++ b/hw/mem/memory-device.c
@@ -17,6 +17,7 @@ 
 #include "qemu/range.h"
 #include "hw/virtio/vhost.h"
 #include "sysemu/kvm.h"
+#include "exec/address-spaces.h"
 #include "trace.h"
 
 static gint memory_device_addr_sort(gconstpointer a, gconstpointer b)
@@ -333,6 +334,25 @@  uint64_t memory_device_get_region_size(const MemoryDeviceState *md,
     return memory_region_size(mr);
 }
 
+void memory_devices_init(MachineState *ms, hwaddr base, uint64_t size)
+{
+    g_assert(!ms->device_memory);
+    ms->device_memory = g_new0(DeviceMemoryState, 1);
+    ms->device_memory->base = base;
+
+    /*
+     * See memory_device_get_free_addr(): An empty device memory region
+     * means "this machine supports memory devices, but they are not enabled".
+     */
+    if (size > 0) {
+        memory_region_init(&ms->device_memory->mr, OBJECT(ms), "device-memory",
+                           size);
+        memory_region_add_subregion(get_system_memory(),
+                                    ms->device_memory->base,
+                                    &ms->device_memory->mr);
+    }
+}
+
 static const TypeInfo memory_device_info = {
     .name          = TYPE_MEMORY_DEVICE,
     .parent        = TYPE_INTERFACE,
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 1baea16c96..d66dc00ea5 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2866,12 +2866,11 @@  static void spapr_machine_init(MachineState *machine)
     /* map RAM */
     memory_region_add_subregion(sysmem, 0, machine->ram);
 
-    /* always allocate the device memory information */
-    machine->device_memory = g_malloc0(sizeof(*machine->device_memory));
-
     /* initialize hotplug memory address space */
     if (machine->ram_size < machine->maxram_size) {
         ram_addr_t device_mem_size = machine->maxram_size - machine->ram_size;
+        hwaddr device_mem_base;
+
         /*
          * Limit the number of hotpluggable memory slots to half the number
          * slots that KVM supports, leaving the other half for PCI and other
@@ -2890,12 +2889,10 @@  static void spapr_machine_init(MachineState *machine)
             exit(1);
         }
 
-        machine->device_memory->base = ROUND_UP(machine->ram_size,
-                                                SPAPR_DEVICE_MEM_ALIGN);
-        memory_region_init(&machine->device_memory->mr, OBJECT(spapr),
-                           "device-memory", device_mem_size);
-        memory_region_add_subregion(sysmem, machine->device_memory->base,
-                                    &machine->device_memory->mr);
+        device_mem_base = ROUND_UP(machine->ram_size, SPAPR_DEVICE_MEM_ALIGN);
+        memory_devices_init(machine, device_mem_base, device_mem_size);
+    } else {
+        memory_devices_init(machine, 0, 0);
     }
 
     if (smc->dr_lmb_enabled) {
diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h
index 48d2611fc5..6e8a10e2f5 100644
--- a/include/hw/mem/memory-device.h
+++ b/include/hw/mem/memory-device.h
@@ -16,6 +16,7 @@ 
 #include "hw/qdev-core.h"
 #include "qapi/qapi-types-machine.h"
 #include "qom/object.h"
+#include "exec/hwaddr.h"
 
 #define TYPE_MEMORY_DEVICE "memory-device"
 
@@ -113,5 +114,6 @@  void memory_device_plug(MemoryDeviceState *md, MachineState *ms);
 void memory_device_unplug(MemoryDeviceState *md, MachineState *ms);
 uint64_t memory_device_get_region_size(const MemoryDeviceState *md,
                                        Error **errp);
+void memory_devices_init(MachineState *ms, hwaddr base, uint64_t size);
 
 #endif