RE: [PULL 2/3] xen: Drop out of coroutine context xen_invalidate_map_cache_entry

2024-03-13 Thread Peng Fan 
> Subject: Re: [PULL 2/3] xen: Drop out of coroutine context
> xen_invalidate_map_cache_entry
> 
> 13.03.2024 20:21, Michael Tokarev:
> > 12.03.2024 17:27, Anthony PERARD wrote:
> >> From: Peng Fan 
> >>
> >> xen_invalidate_map_cache_entry is not expected to run in a coroutine.
> >> Without this, there is crash:
> >
> > Hi!  Is this a stable material? (It applies cleanly and builds on 8.2
> > and 7.2)
> 
> Actually for 7.2 it needed a minor tweak:
> 
> -void coroutine_mixed_fn xen_invalidate_map_cache_entry(uint8_t *buffer)
> +void xen_invalidate_map_cache_entry(uint8_t *buffer)

I only tested 8.2 with xen virtio enabled. Not sure whether 7.2 has the issue
or not.

Thanks,
Peng.

> 
> but the rest is okay.
> 
> /mjt

Re: [PULL 2/3] xen: Drop out of coroutine context xen_invalidate_map_cache_entry

2024-03-13 Thread Michael Tokarev 

13.03.2024 20:21, Michael Tokarev:

12.03.2024 17:27, Anthony PERARD wrote:

From: Peng Fan 

xen_invalidate_map_cache_entry is not expected to run in a
coroutine. Without this, there is crash:


Hi!  Is this a stable material? (It applies cleanly and builds on 8.2 and 7.2)


Actually for 7.2 it needed a minor tweak:

-void coroutine_mixed_fn xen_invalidate_map_cache_entry(uint8_t *buffer)
+void xen_invalidate_map_cache_entry(uint8_t *buffer)

but the rest is okay.

/mjt

Re: [PULL 2/3] xen: Drop out of coroutine context xen_invalidate_map_cache_entry

2024-03-13 Thread Michael Tokarev 

12.03.2024 17:27, Anthony PERARD wrote:

From: Peng Fan 

xen_invalidate_map_cache_entry is not expected to run in a
coroutine. Without this, there is crash:


Hi!  Is this a stable material? (It applies cleanly and builds on 8.2 and 7.2)

Thanks,

/mjt

[PULL 2/3] xen: Drop out of coroutine context xen_invalidate_map_cache_entry

2024-03-12 Thread Anthony PERARD 
From: Peng Fan 

xen_invalidate_map_cache_entry is not expected to run in a
coroutine. Without this, there is crash:

signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
threadid=) at pthread_kill.c:78
at /usr/src/debug/glibc/2.38+git-r0/sysdeps/posix/raise.c:26
fmt=0x9e1ca8a8 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n",
assertion=assertion@entry=0xe0d25740 "!qemu_in_coroutine()",
file=file@entry=0xe0d301a8 "../qemu-xen-dir-remote/block/graph-lock.c", 
line=line@entry=260,
function=function@entry=0xe0e522c0 <__PRETTY_FUNCTION__.3> 
"bdrv_graph_rdlock_main_loop") at assert.c:92
assertion=assertion@entry=0xe0d25740 "!qemu_in_coroutine()",
file=file@entry=0xe0d301a8 "../qemu-xen-dir-remote/block/graph-lock.c", 
line=line@entry=260,
function=function@entry=0xe0e522c0 <__PRETTY_FUNCTION__.3> 
"bdrv_graph_rdlock_main_loop") at assert.c:101
at ../qemu-xen-dir-remote/block/graph-lock.c:260
at 
/home/Freenix/work/sw-stash/xen/upstream/tools/qemu-xen-dir-remote/include/block/graph-lock.h:259
host=host@entry=0x742c8000, size=size@entry=2097152)
at ../qemu-xen-dir-remote/block/io.c:3362
host=0x742c8000, size=2097152)
at ../qemu-xen-dir-remote/block/block-backend.c:2859
host=, size=, max_size=)
at ../qemu-xen-dir-remote/block/block-ram-registrar.c:33
size=2097152, max_size=2097152)
at ../qemu-xen-dir-remote/hw/core/numa.c:883
buffer=buffer@entry=0x743c5000 "")
at ../qemu-xen-dir-remote/hw/xen/xen-mapcache.c:475
buffer=buffer@entry=0x743c5000 "")
at ../qemu-xen-dir-remote/hw/xen/xen-mapcache.c:487
as=as@entry=0xe1ca3ae8 , buffer=0x743c5000,
len=, is_write=is_write@entry=true,
access_len=access_len@entry=32768)
at ../qemu-xen-dir-remote/system/physmem.c:3199
dir=DMA_DIRECTION_FROM_DEVICE, len=,
buffer=, as=0xe1ca3ae8 )
at 
/home/Freenix/work/sw-stash/xen/upstream/tools/qemu-xen-dir-remote/include/sysemu/dma.h:236
elem=elem@entry=0xf620aa30, len=len@entry=32769)
at ../qemu-xen-dir-remote/hw/virtio/virtio.c:758
elem=elem@entry=0xf620aa30, len=len@entry=32769, idx=idx@entry=0)
at ../qemu-xen-dir-remote/hw/virtio/virtio.c:919
elem=elem@entry=0xf620aa30, len=32769)
at ../qemu-xen-dir-remote/hw/virtio/virtio.c:994
req=req@entry=0xf620aa30, status=status@entry=0 '\000')
at ../qemu-xen-dir-remote/hw/block/virtio-blk.c:67
ret=0) at ../qemu-xen-dir-remote/hw/block/virtio-blk.c:136
at ../qemu-xen-dir-remote/block/block-backend.c:1559
--Type  for more, q to quit, c to continue without paging--
at ../qemu-xen-dir-remote/block/block-backend.c:1614
i1=) at ../qemu-xen-dir-remote/util/coroutine-ucontext.c:177
at ../sysdeps/unix/sysv/linux/aarch64/setcontext.S:123

Signed-off-by: Peng Fan 
Reviewed-by: Stefano Stabellini 
Message-Id: <20240124021450.21656-1-peng@oss.nxp.com>
Signed-off-by: Anthony PERARD 
---
 hw/xen/xen-mapcache.c | 30 --
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
index 4f956d048e..7f59080ba7 100644
--- a/hw/xen/xen-mapcache.c
+++ b/hw/xen/xen-mapcache.c
@@ -476,11 +476,37 @@ static void 
xen_invalidate_map_cache_entry_unlocked(uint8_t *buffer)
 g_free(entry);
 }
 
-void xen_invalidate_map_cache_entry(uint8_t *buffer)
+typedef struct XenMapCacheData {
+Coroutine *co;
+uint8_t *buffer;
+} XenMapCacheData;
+
+static void xen_invalidate_map_cache_entry_bh(void *opaque)
 {
+XenMapCacheData *data = opaque;
+
 mapcache_lock();
-xen_invalidate_map_cache_entry_unlocked(buffer);
+xen_invalidate_map_cache_entry_unlocked(data->buffer);
 mapcache_unlock();
+
+aio_co_wake(data->co);
+}
+
+void coroutine_mixed_fn xen_invalidate_map_cache_entry(uint8_t *buffer)
+{
+if (qemu_in_coroutine()) {
+XenMapCacheData data = {
+.co = qemu_coroutine_self(),
+.buffer = buffer,
+};
+aio_bh_schedule_oneshot(qemu_get_current_aio_context(),
+xen_invalidate_map_cache_entry_bh, );
+qemu_coroutine_yield();
+} else {
+mapcache_lock();
+xen_invalidate_map_cache_entry_unlocked(buffer);
+mapcache_unlock();
+}
 }
 
 void xen_invalidate_map_cache(void)
-- 
Anthony PERARD