Re: [PULL 08/18] memory: Factor out common ram region initialization

Thu, 12 Mar 2026 15:28:13 -0700 

On 3/9/26 3:39 PM, Peter Xu wrote:

From: BALATON Zoltan <[email protected]>

Introduce internal helper function to remove duplicated code from
different memory_region_init_*ram functions. Remove local err and
error_propagate and pass errp and check return value instead.
Also shorten some function prototypes while at it.

Signed-off-by: BALATON Zoltan <[email protected]>
Reviewed-by: Akihiko Odaki <[email protected]>
Link: 
https://lore.kernel.org/r/d6db01c283149b46023ffeb9c5b368c67f6acc8d.1772924151.git.bala...@eik.bme.hu
Signed-off-by: Peter Xu <[email protected]>
---
Hi, I bisected master (fff352b9b6) to this patch as the first bad commit that results in this very early qemu error:
ERROR:../system/memory.c:2042:memory_region_set_ram_discard_manager: assertion failed: (memory_region_is_ram(mr)) 

It's pointing to this line of code:

int memory_region_set_ram_discard_manager(MemoryRegion *mr,
                                          RamDiscardManager *rdm)
{
    g_assert(memory_region_is_ram(mr));   <<<<<

Qemu then bails out. The command line is:

sudo /usr/bin/qemu-system-x86_64 -enable-kvm \
 -drive file=/home/amd/ubuntu-22.04-server-cloudimg-amd64-kim.qcow2,if=none,id=disk0,format=qcow2 \ 
 -device virtio-scsi-pci,id=scsi0,disable-legacy=on,iommu_platform=true \
 -device scsi-hd,drive=disk0 \
 -cpu EPYC-v4 \
 -bios /home/amd/OVMF.fd \
 -machine memory-encryption=sev0,vmport=off \
 -object memory-backend-memfd,id=ram1,size=8G,share=true,prealloc=false \
 -machine memory-backend=ram1  \
 -object sev-snp-guest,id=sev0,policy=0xb0000,cbitpos=51,reduced-phys-bits=1 \ 
 -monitor unix:qemu-monitor-socket,server,nowait \
 -smp 2 -m 8G -nographic -s \
 --kernel /home/amd/git/linux/arch/x86/boot/bzImage \
 --append "verbose debug console=ttyS0,115200n8 earlyprintk=ttyS0,115200 log_level=8 keep_bootcon root=/dev/sda1" \ 
 --initrd /boot/initrd.img \
 -no-reboot
The host (& guest) kernel version is a vanilla v6.19, with an Ubuntu based config.
Let me know if there's something you'd like me to test on my AMD EPYC system. 
AFAICT, it fails on all Zens that support SEV-SNP (Zen3+).

Thanks,

Kim


  system/memory.c | 167 ++++++++++++++++++------------------------------
  1 file changed, 61 insertions(+), 106 deletions(-)

diff --git a/system/memory.c b/system/memory.c
index 9a12224555..2cda814bd6 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1568,39 +1568,39 @@ static void memory_region_set_ops(MemoryRegion *mr,
      mr->terminates = true;
  }
-void memory_region_init_io(MemoryRegion *mr, 
-                           Object *owner,
-                           const MemoryRegionOps *ops,
-                           void *opaque,
-                           const char *name,
-                           uint64_t size)
+void memory_region_init_io(MemoryRegion *mr, Object *owner,
+                           const MemoryRegionOps *ops, void *opaque,
+                           const char *name, uint64_t size)
  {
      memory_region_init(mr, owner, name, size);
      memory_region_set_ops(mr, ops, opaque);
  }
-bool memory_region_init_ram_flags_nomigrate(MemoryRegion *mr, 
-                                            Object *owner,
-                                            const char *name,
-                                            uint64_t size,
-                                            uint32_t ram_flags,
-                                            Error **errp)
+static bool memory_region_set_ram_block(MemoryRegion *mr, RAMBlock *rb)
  {
-    Error *err = NULL;
-    memory_region_init(mr, owner, name, size);
      mr->ram = true;
      mr->terminates = true;
      mr->destructor = memory_region_destructor_ram;
-    mr->ram_block = qemu_ram_alloc(size, ram_flags, mr, &err);
-    if (err) {
+    mr->ram_block = rb;
+    if (!rb) {
          mr->size = int128_zero();
          object_unparent(OBJECT(mr));
-        error_propagate(errp, err);
          return false;
      }
      return true;
  }
+bool memory_region_init_ram_flags_nomigrate(MemoryRegion *mr, Object *owner, 
+                                            const char *name, uint64_t size,
+                                            uint32_t ram_flags, Error **errp)
+{
+    RAMBlock *rb;
+
+    memory_region_init(mr, owner, name, size);
+    rb = qemu_ram_alloc(size, ram_flags, mr, errp);
+    return memory_region_set_ram_block(mr, rb);
+}
+
  bool memory_region_init_resizeable_ram(MemoryRegion *mr,
                                         Object *owner,
                                         const char *name,
@@ -1611,116 +1611,74 @@ bool memory_region_init_resizeable_ram(MemoryRegion 
*mr,
                                                         void *host),
                                         Error **errp)
  {
-    Error *err = NULL;
+    RAMBlock *rb;
+
      memory_region_init(mr, owner, name, size);
-    mr->ram = true;
-    mr->terminates = true;
-    mr->destructor = memory_region_destructor_ram;
-    mr->ram_block = qemu_ram_alloc_resizeable(size, max_size, resized,
-                                              mr, &err);
-    if (err) {
-        mr->size = int128_zero();
-        object_unparent(OBJECT(mr));
-        error_propagate(errp, err);
-        return false;
-    }
-    return true;
+    rb = qemu_ram_alloc_resizeable(size, max_size, resized, mr, errp);
+    return memory_region_set_ram_block(mr, rb);
  }
#if defined(CONFIG_POSIX) && !defined(EMSCRIPTEN) 
-bool memory_region_init_ram_from_file(MemoryRegion *mr,
-                                      Object *owner,
-                                      const char *name,
-                                      uint64_t size,
-                                      uint64_t align,
-                                      uint32_t ram_flags,
-                                      const char *path,
-                                      ram_addr_t offset,
+bool memory_region_init_ram_from_file(MemoryRegion *mr, Object *owner,
+                                      const char *name, uint64_t size,
+                                      uint64_t align, uint32_t ram_flags,
+                                      const char *path, ram_addr_t offset,
                                        Error **errp)
  {
-    Error *err = NULL;
+    RAMBlock *rb;
+
      memory_region_init(mr, owner, name, size);
-    mr->ram = true;
      mr->readonly = !!(ram_flags & RAM_READONLY);
-    mr->terminates = true;
-    mr->destructor = memory_region_destructor_ram;
      mr->align = align;
-    mr->ram_block = qemu_ram_alloc_from_file(size, mr, ram_flags, path,
-                                             offset, &err);
-    if (err) {
-        mr->size = int128_zero();
-        object_unparent(OBJECT(mr));
-        error_propagate(errp, err);
-        return false;
-    }
-    return true;
+    rb = qemu_ram_alloc_from_file(size, mr, ram_flags, path, offset, errp);
+    return memory_region_set_ram_block(mr, rb);
  }
-bool memory_region_init_ram_from_fd(MemoryRegion *mr, 
-                                    Object *owner,
-                                    const char *name,
-                                    uint64_t size,
-                                    uint32_t ram_flags,
-                                    int fd,
-                                    ram_addr_t offset,
-                                    Error **errp)
+bool memory_region_init_ram_from_fd(MemoryRegion *mr, Object *owner,
+                                    const char *name, uint64_t size,
+                                    uint32_t ram_flags, int fd,
+                                    ram_addr_t offset, Error **errp)
  {
-    Error *err = NULL;
+    RAMBlock *rb;
+
      memory_region_init(mr, owner, name, size);
-    mr->ram = true;
      mr->readonly = !!(ram_flags & RAM_READONLY);
-    mr->terminates = true;
-    mr->destructor = memory_region_destructor_ram;
-    mr->ram_block = qemu_ram_alloc_from_fd(size, size, NULL, mr, ram_flags, fd,
-                                           offset, false, &err);
-    if (err) {
-        mr->size = int128_zero();
-        object_unparent(OBJECT(mr));
-        error_propagate(errp, err);
-        return false;
-    }
-    return true;
+    rb = qemu_ram_alloc_from_fd(size, size, NULL, mr, ram_flags, fd, offset,
+                                false, errp);
+    return memory_region_set_ram_block(mr, rb);
  }
  #endif
-void memory_region_init_ram_ptr(MemoryRegion *mr, 
-                                Object *owner,
-                                const char *name,
-                                uint64_t size,
+void memory_region_init_ram_ptr(MemoryRegion *mr, Object *owner,
+                                const char *name, uint64_t size,
                                  void *ptr)
  {
-    memory_region_init(mr, owner, name, size);
-    mr->ram = true;
-    mr->terminates = true;
-    mr->destructor = memory_region_destructor_ram;
+    RAMBlock *rb;
+ memory_region_init(mr, owner, name, size); 
      /* qemu_ram_alloc_from_ptr cannot fail with ptr != NULL.  */
      assert(ptr != NULL);
-    mr->ram_block = qemu_ram_alloc_from_ptr(size, ptr, mr, &error_abort);
+    rb = qemu_ram_alloc_from_ptr(size, ptr, mr, &error_abort);
+    memory_region_set_ram_block(mr, rb);
  }
-void memory_region_init_ram_device_ptr(MemoryRegion *mr, 
-                                       Object *owner,
-                                       const char *name,
-                                       uint64_t size,
+void memory_region_init_ram_device_ptr(MemoryRegion *mr, Object *owner,
+                                       const char *name, uint64_t size,
                                         void *ptr)
  {
+    RAMBlock *rb;
+
      memory_region_init_io(mr, owner, &ram_device_mem_ops, mr, name, size);
-    mr->ram = true;
      mr->ram_device = true;
-    mr->destructor = memory_region_destructor_ram;
-
      /* qemu_ram_alloc_from_ptr cannot fail with ptr != NULL.  */
      assert(ptr != NULL);
-    mr->ram_block = qemu_ram_alloc_from_ptr(size, ptr, mr, &error_abort);
+    rb = qemu_ram_alloc_from_ptr(size, ptr, mr, &error_abort);
+    memory_region_set_ram_block(mr, rb);
  }
-void memory_region_init_alias(MemoryRegion *mr, 
-                              Object *owner,
-                              const char *name,
-                              MemoryRegion *orig,
-                              hwaddr offset,
-                              uint64_t size)
+void memory_region_init_alias(MemoryRegion *mr, Object *owner,
+                              const char *name, MemoryRegion *orig,
+                              hwaddr offset, uint64_t size)
  {
      memory_region_init(mr, owner, name, size);
      mr->alias = orig;
@@ -3732,21 +3690,18 @@ bool memory_region_init_rom_device(MemoryRegion *mr, 
Object *owner,
                                     const char *name, uint64_t size,
                                     Error **errp)
  {
-    Error *err = NULL;
+    RAMBlock *rb;
assert(ops); 
      memory_region_init_io(mr, owner, ops, opaque, name, size);
-    mr->rom_device = true;
-    mr->destructor = memory_region_destructor_ram;
-    mr->ram_block = qemu_ram_alloc(size, 0, mr, &err);
-    if (err) {
-        mr->size = int128_zero();
-        object_unparent(OBJECT(mr));
-        error_propagate(errp, err);
-        return false;
+    rb = qemu_ram_alloc(size, 0, mr, errp);
+    if (memory_region_set_ram_block(mr, rb)) {
+        mr->ram = false;
+        mr->rom_device = true;
+        memory_region_register_ram(mr, owner);
+        return true;
      }
-    memory_region_register_ram(mr, owner);
-    return true;
+    return false;
  }
/*

Reply via email to