On Mon, Jul 1, 2024 at 2:55 PM Anthony PERARD <anthony.perard@vates.tech> wrote:
Hi all,
Following this commit, a test which install Debian in a guest with OVMF
as firmware started to fail. QEMU exit with an error when GRUB is
running on the freshly installed Debian (I don't know if GRUB is
starting Linux or not).
The error is:
Bad ram offset ffffffffffffffff
Some logs:
http://logs.test-lab.xenproject.org/osstest/logs/186611/test-amd64-amd64-xl-qemuu-ovmf-amd64/info.html
Any idea? Something is trying to do something with the address "-1" when
it shouldn't?
Hi Anothny,
Yes, it looks like something is calling qemu_get_ram_block() on something that isn't mapped.
One possible path is in qemu_ram_block_from_host() but there may be others.
The following patch may help.
Any chance you could try to get a backtrace from QEMU when it failed?
diff --git a/system/physmem.c b/system/physmem.c
index 33d09f7571..2669c4dbbb 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -2277,6 +2277,9 @@ RAMBlock *qemu_ram_block_from_host(void *ptr, bool round_offset,
ram_addr_t ram_addr;
RCU_READ_LOCK_GUARD();
ram_addr = xen_ram_addr_from_mapcache(ptr);
+ if (ram_addr == RAM_ADDR_INVALID) {
+ return NULL;
+ }
block = qemu_get_ram_block(ram_addr);
if (block) {
*offset = ram_addr - block->offset;
One more thing, regarding this specific patch. I don't think we should clear the
entire entry, the next field should be kept, otherwise we'll disconnect following
mappings that will never be found again. IIUC, this could very well be causing the problem you see.
Does the following make sense?
diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
index 5f23b0adbe..e9df53c19d 100644
--- a/hw/xen/xen-mapcache.c
+++ b/hw/xen/xen-mapcache.c
@@ -597,7 +597,14 @@ static void xen_invalidate_map_cache_entry_unlocked(MapCache *mc,
pentry->next = entry->next;
g_free(entry);
} else {
- memset(entry, 0, sizeof *entry);
+ /* Invalidate mapping. */
+ entry->paddr_index = 0;
+ entry->vaddr_base = NULL;
+ entry->size = 0;
+ g_free(entry->valid_mapping);
+ entry->valid_mapping = NULL;
+ entry->flags = 0;
+ /* Keep entry->next pointing to the rest of the list. */
}
}
Cheers,
Anthony
On Wed, May 29, 2024 at 04:07:33PM +0200, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
>
> When invalidating memory ranges, if we happen to hit the first
> entry in a bucket we were never unmapping it. This was harmless
> for foreign mappings but now that we're looking to reuse the
> mapcache for transient grant mappings, we must unmap entries
> when invalidated.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
> hw/xen/xen-mapcache.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
> index bc860f4373..ec95445696 100644
> --- a/hw/xen/xen-mapcache.c
> +++ b/hw/xen/xen-mapcache.c
> @@ -491,18 +491,23 @@ static void xen_invalidate_map_cache_entry_unlocked(MapCache *mc,
> return;
> }
> entry->lock--;
> - if (entry->lock > 0 || pentry == NULL) {
> + if (entry->lock > 0) {
> return;
> }
>
> - pentry->next = entry->next;
> ram_block_notify_remove(entry->vaddr_base, entry->size, entry->size);
> if (munmap(entry->vaddr_base, entry->size) != 0) {
> perror("unmap fails");
> exit(-1);
> }
> +
> g_free(entry->valid_mapping);
> - g_free(entry);
> + if (pentry) {
> + pentry->next = entry->next;
> + g_free(entry);
> + } else {
> + memset(entry, 0, sizeof *entry);
> + }
> }
>
> typedef struct XenMapCacheData {
> --
> 2.40.1
>
>
--
Anthony Perard | Vates XCP-ng Developer
XCP-ng & Xen Orchestra - Vates solutions
web: https://vates.tech