> -----Original Message----- > From: David Hildenbrand [mailto:da...@redhat.com] > Sent: 12 February 2020 18:21 > To: Shameerali Kolothum Thodi <shameerali.kolothum.th...@huawei.com>; > Igor Mammedov <imamm...@redhat.com> > Cc: peter.mayd...@linaro.org; xiaoguangrong.e...@gmail.com; > m...@redhat.com; shannon.zha...@gmail.com; qemu-devel@nongnu.org; > xuwei (O) <xuw...@huawei.com>; Linuxarm <linux...@huawei.com>; > eric.au...@redhat.com; qemu-...@nongnu.org; ler...@redhat.com; > dgilb...@redhat.com; Juan Jose Quintela Carreira <quint...@redhat.com> > Subject: Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback
[...] > > Hmm..it breaks x86 + seabios boot. The issue is seabios expects RSDP in FSEG > > memory. With the above proposed change, RSDP will be aligned to PAGE_SIZE > and > > seabios mem allocation for RSDP fails at, > > > > > https://github.com/coreboot/seabios/blob/master/src/fw/romfile_loader.c#L8 > 5 > > > > To get pass the above, I changed "alloc_fseg" flag to false in > > build_rsdp(), but > > seabios seems to make the assumption that RSDP has to be placed in FSEG > memory > > here, > > https://github.com/coreboot/seabios/blob/master/src/fw/biostables.c#L126 > > > > So doesn’t look like there is an easy fix for this without changing the > > seabios > code. > > > > Between, OVMF works fine with the aligned size on x86. > > > > One thing we can do is treat the RSDP case separately or only use the > > aligned > > page size for "etc/acpi/tables" as below, [...] > > > > Thoughts? > > I don't think introducing memory_region_get_used_length() is a > good idea. I also dislike, that the ram block size can differ > to the memory region size. I wasn't aware of that condition, sorry! Right. They can differ in size and is the case here. > Making the memory region always store an aligned size might break other use > cases. > > Summarizing the issue: > 1. Memory regions contain ram blocks with a different size, if the size is > not properly aligned. While memory regions can have an unaligned size, > ram blocks can't. This is true when creating resizable memory region with > an unaligned size. > 2. When resizing a ram block/memory region, the size of the memory region > is set to the aligned size. The callback is called with the aligned size. > The unaligned piece is lost. > 3. When migrating, we migrate the aligned size. > > > What about something like this: (untested) Thanks for that. I had a go with the below patch and it indeed fixes the issue of callback not being called on resize. But the migration fails with the below error, For x86 --------- qemu-system-x86_64: Unknown combination of migration flags: 0x14 qemu-system-x86_64: error while loading state for instance 0x0 of device 'ram' qemu-system-x86_64: load of migration failed: Invalid argument For arm64 -------------- qemu-system-aarch64: Received an unexpected compressed page qemu-system-aarch64: error while loading state for instance 0x0 of device 'ram' qemu-system-aarch64: load of migration failed: Invalid argument I haven’t debugged this further but looks like there is a corruption happening. Please let me know if you have any clue. Thanks, Shameer > > From d84c21bc67e15acdac2f6265cd1576d8dd920211 Mon Sep 17 00:00:00 > 2001 > From: David Hildenbrand <da...@redhat.com> > Date: Wed, 12 Feb 2020 19:16:34 +0100 > Subject: [PATCH v1] tmp > > Signed-off-by: David Hildenbrand <da...@redhat.com> > --- > exec.c | 14 ++++++++++++-- > migration/ram.c | 44 ++++++++++++++++++++++++++++++++------------ > 2 files changed, 44 insertions(+), 14 deletions(-) > > diff --git a/exec.c b/exec.c > index 05cfe868ab..d41a1e11b5 100644 > --- a/exec.c > +++ b/exec.c > @@ -2130,11 +2130,21 @@ static int memory_try_enable_merging(void > *addr, size_t len) > */ > int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp) > { > + const ram_addr_t unaligned_size = newsize; > + > assert(block); > > newsize = HOST_PAGE_ALIGN(newsize); > > if (block->used_length == newsize) { > + /* > + * We don't have to resize the ram block (which only knows aligned > + * sizes), however, we have to notify if the unaligned size changed. > + */ > + if (block->resized && unaligned_size != > memory_region_size(block->mr)) { > + block->resized(block->idstr, unaligned_size, block->host); > + memory_region_set_size(block->mr, unaligned_size); > + } > return 0; > } > > @@ -2158,9 +2168,9 @@ int qemu_ram_resize(RAMBlock *block, > ram_addr_t newsize, Error **errp) > block->used_length = newsize; > cpu_physical_memory_set_dirty_range(block->offset, > block->used_length, > DIRTY_CLIENTS_ALL); > - memory_region_set_size(block->mr, newsize); > + memory_region_set_size(block->mr, unaligned_size); > if (block->resized) { > - block->resized(block->idstr, newsize, block->host); > + block->resized(block->idstr, unaligned_size, block->host); > } > return 0; > } > diff --git a/migration/ram.c b/migration/ram.c > index ed23ed1c7c..2acc4b85ca 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -1761,28 +1761,43 @@ void acct_update_position(QEMUFile *f, size_t > size, bool zero) > } > } > > -static uint64_t ram_bytes_total_common(bool count_ignored) > +static uint64_t ramblock_ram_bytes_migration(RAMBlock *block) > +{ > + /* > + * When resizing on the target, we need the unaligned size, > + * otherwise, we lose the unaligned size during migration. > + */ > + if (block->mr && (block->flags & RAM_RESIZEABLE)) { > + return memory_region_size(block->mr); > + } else { > + return block->used_length; > + } > +} > + > +static uint64_t ram_bytes_total_migration(void) > { > RAMBlock *block; > uint64_t total = 0; > > RCU_READ_LOCK_GUARD(); > > - if (count_ignored) { > - RAMBLOCK_FOREACH_MIGRATABLE(block) { > - total += block->used_length; > - } > - } else { > - RAMBLOCK_FOREACH_NOT_IGNORED(block) { > - total += block->used_length; > - } > + RAMBLOCK_FOREACH_MIGRATABLE(block) { > + total += ramblock_ram_bytes_migration(block); > } > return total; > } > > uint64_t ram_bytes_total(void) > { > - return ram_bytes_total_common(false); > + RAMBlock *block; > + uint64_t total = 0; > + > + RCU_READ_LOCK_GUARD(); > + > + RAMBLOCK_FOREACH_NOT_IGNORED(block) { > + total += block->used_length; > + } > + return total; > } > > static void xbzrle_load_setup(void) > @@ -2432,12 +2447,17 @@ static int ram_save_setup(QEMUFile *f, void > *opaque) > (*rsp)->f = f; > > WITH_RCU_READ_LOCK_GUARD() { > - qemu_put_be64(f, ram_bytes_total_common(true) | > RAM_SAVE_FLAG_MEM_SIZE); > + qemu_put_be64(f, ram_bytes_total_migration() | > RAM_SAVE_FLAG_MEM_SIZE); > > RAMBLOCK_FOREACH_MIGRATABLE(block) { > qemu_put_byte(f, strlen(block->idstr)); > qemu_put_buffer(f, (uint8_t *)block->idstr, > strlen(block->idstr)); > - qemu_put_be64(f, block->used_length); > + /* > + * When resizing on the target, we need the unaligned size, > + * otherwise we lose the unaligned sise during migration. > + */ > + qemu_put_be64(f, ramblock_ram_bytes_migration(block)); > + > if (migrate_postcopy_ram() && block->page_size != > qemu_host_page_size) { > qemu_put_be64(f, block->page_size); > -- > 2.24.1 > > > -- > Thanks, > > David / dhildenb
