* [PATCH RFC 0/1] MemoryListener address_space_map callback @ 2023-04-05 12:57 Antonio Caggiano 2023-04-05 12:57 ` [PATCH RFC 1/1] memory: Address space map listener Antonio Caggiano 0 siblings, 1 reply; 6+ messages in thread From: Antonio Caggiano @ 2023-04-05 12:57 UTC (permalink / raw) To: qemu-devel Hi! This is the RFC about the memory issue I mentioned in our last KVM call. In our use case, QEMU is used as a library, where RAM and Alias MemoryRegions are created by listening to read/write events through MemoryRegionOps callbacks. In this case, no read/write happened yet, so we did not have a chance to create those memory regions yet. The callstack looks like this: - virtio_blk_get_request - virtqueue_pop -> virtqueue_split_pop -> virtqueue_map_desc - dma_memory_map -> address_space_map The address_space_map function calls flatview_translate to get the memory region for a certain address. If the memory region is not directly accessible, the bounce buffer is used which only allows one mapping at a time, forcing to unmap before mapping again. The virtqueue_map_desc function calls iteratively address_space_map for a region of 4KB but address_space_map is only mapping 1KB using the bounce buffer. Then virtqueue_map_desc calls address_space_map again for mapping the missing 3KB, but address_space_map returns NULL as the bounce is in use now. With this patch a MemoryListener callback is introduced for listening to address space map events, before calling flatview_translate, so that listeners might have a chance to create any needed alias or RAM memory region for that address space. Effectively making flatview_translate return a directly accessible memory region, and avoiding address_space_map to use the bounce buffer. This will require a change to the memory listener callbacks: while it currently uses "self" as first argument for the callbacks, this new approach is going to use an "opaque" member, effectively following the model used for MemoryRegion and MemoryRegionOps. Antonio Caggiano (1): memory: Address space map listener include/exec/memory.h | 19 +++++++++++++++++++ softmmu/physmem.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) -- 2.40.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH RFC 1/1] memory: Address space map listener 2023-04-05 12:57 [PATCH RFC 0/1] MemoryListener address_space_map callback Antonio Caggiano @ 2023-04-05 12:57 ` Antonio Caggiano 2023-04-05 13:23 ` David Hildenbrand 0 siblings, 1 reply; 6+ messages in thread From: Antonio Caggiano @ 2023-04-05 12:57 UTC (permalink / raw) To: qemu-devel Cc: Paolo Bonzini, Peter Xu, David Hildenbrand, Philippe Mathieu-Daudé Introduce a MemoryListener callback for address space map events. This will require a change to the memory listener callbacks: while it currently uses "self" as first argument for the callbacks, this new approach is going to use an "opaque" member, effectively following the model used for MemoryRegion and MemoryRegionOps. Signed-off-by: Antonio Caggiano <quic_acaggian@quicinc.com> --- include/exec/memory.h | 19 +++++++++++++++++++ softmmu/physmem.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/include/exec/memory.h b/include/exec/memory.h index 7ec6df3289..f959d53a12 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -1045,6 +1045,18 @@ struct MemoryListener { */ void (*coalesced_io_del)(MemoryListener *listener, MemoryRegionSection *section, hwaddr addr, hwaddr len); + + /** + * @map: + * + * Called during an address space map. + * + * @opaque: User data opaque object + * @addr: address within that address space + * @len: length of buffer + */ + void (*map)(void *opaque, hwaddr addr, hwaddr len); + /** * @priority: * @@ -1054,6 +1066,13 @@ struct MemoryListener { */ unsigned priority; + /** + * @opaque: + * + * Opaque pointer to user data + */ + void *opaque; + /** * @name: * diff --git a/softmmu/physmem.c b/softmmu/physmem.c index 9486a1ebdf..0f8bad6b40 100644 --- a/softmmu/physmem.c +++ b/softmmu/physmem.c @@ -3246,6 +3246,38 @@ flatview_extend_translation(FlatView *fv, hwaddr addr, } } +enum ListenerDirection { Forward, Reverse }; + +/* + * This will require a change to the memory listener callbacks: + * while it currently uses "self" as first argument for the callbacks, this new + * approach is going to use an "opaque" member, effectively following the model + * used for MemoryRegion and MemoryRegionOps. + */ +#define MEMORY_LISTENER_CALL(_as, _callback, _direction, _args...) \ + do { \ + MemoryListener *_listener; \ + \ + switch (_direction) { \ + case Forward: \ + QTAILQ_FOREACH(_listener, &(_as)->listeners, link_as) { \ + if (_listener->_callback) { \ + _listener->_callback(_listener->opaque, ##_args); \ + } \ + } \ + break; \ + case Reverse: \ + QTAILQ_FOREACH_REVERSE(_listener, &(_as)->listeners, link_as) { \ + if (_listener->_callback) { \ + _listener->_callback(_listener->opaque, ##_args); \ + } \ + } \ + break; \ + default: \ + abort(); \ + } \ + } while (0) + /* Map a physical memory region into a host virtual address. * May map a subset of the requested range, given by and returned in *plen. * May return NULL if resources needed to perform the mapping are exhausted. @@ -3268,6 +3300,8 @@ void *address_space_map(AddressSpace *as, return NULL; } + MEMORY_LISTENER_CALL(as, map, Reverse, addr, len); + l = len; RCU_READ_LOCK_GUARD(); fv = address_space_to_flatview(as); -- 2.40.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH RFC 1/1] memory: Address space map listener 2023-04-05 12:57 ` [PATCH RFC 1/1] memory: Address space map listener Antonio Caggiano @ 2023-04-05 13:23 ` David Hildenbrand 2023-04-05 14:25 ` Antonio Caggiano 0 siblings, 1 reply; 6+ messages in thread From: David Hildenbrand @ 2023-04-05 13:23 UTC (permalink / raw) To: Antonio Caggiano, qemu-devel Cc: Paolo Bonzini, Peter Xu, Philippe Mathieu-Daudé On 05.04.23 14:57, Antonio Caggiano wrote: > Introduce a MemoryListener callback for address space map events. > Why? -- Thanks, David / dhildenb ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC 1/1] memory: Address space map listener 2023-04-05 13:23 ` David Hildenbrand @ 2023-04-05 14:25 ` Antonio Caggiano 2023-04-05 14:26 ` David Hildenbrand 0 siblings, 1 reply; 6+ messages in thread From: Antonio Caggiano @ 2023-04-05 14:25 UTC (permalink / raw) To: David Hildenbrand, qemu-devel Cc: Paolo Bonzini, Peter Xu, Philippe Mathieu-Daudé Hi David, On 05/04/23 15:23, David Hildenbrand wrote: > On 05.04.23 14:57, Antonio Caggiano wrote: >> Introduce a MemoryListener callback for address space map events. >> > > Why? > Please, have a look at the cover letter "[PATCH RFC 0/1] MemoryListener address_space_map callback" with a detail explanation of the issue and the reason behind this. While I think it solves the issue for my use case, it might not be the best or even the right solution for it. Cheers, Antonio ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC 1/1] memory: Address space map listener 2023-04-05 14:25 ` Antonio Caggiano @ 2023-04-05 14:26 ` David Hildenbrand 0 siblings, 0 replies; 6+ messages in thread From: David Hildenbrand @ 2023-04-05 14:26 UTC (permalink / raw) To: Antonio Caggiano, qemu-devel Cc: Paolo Bonzini, Peter Xu, Philippe Mathieu-Daudé On 05.04.23 16:25, Antonio Caggiano wrote: > Hi David, > > On 05/04/23 15:23, David Hildenbrand wrote: >> On 05.04.23 14:57, Antonio Caggiano wrote: >>> Introduce a MemoryListener callback for address space map events. >>> >> >> Why? >> > > Please, have a look at the cover letter "[PATCH RFC 0/1] MemoryListener > address_space_map callback" with a detail explanation of the issue and > the reason behind this. While I think it solves the issue for my use > case, it might not be the best or even the right solution for it. Oh, you did not CC me on the cover letter, so I missed it ... -- Thanks, David / dhildenb ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <20230405111515.62137-1-quic_acaggian@quicinc.com>]
* [PATCH RFC 1/1] memory: Address space map listener [not found] <20230405111515.62137-1-quic_acaggian@quicinc.com> @ 2023-04-05 11:15 ` Antonio Caggiano 0 siblings, 0 replies; 6+ messages in thread From: Antonio Caggiano @ 2023-04-05 11:15 UTC (permalink / raw) To: mburton, quic_acaggian Cc: Paolo Bonzini, Peter Xu, David Hildenbrand, Philippe Mathieu-Daudé, open list:All patches CC here Introduce a MemoryListener callback for address space map events. This will require a change to the memory listener callbacks: while it currently uses "self" as first argument for the callbacks, this new approach is going to use an "opaque" member, effectively following the model used for MemoryRegion and MemoryRegionOps. Signed-off-by: Antonio Caggiano <quic_acaggian@quicinc.com> --- include/exec/memory.h | 19 +++++++++++++++++++ softmmu/physmem.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/include/exec/memory.h b/include/exec/memory.h index 7ec6df3289..f959d53a12 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -1045,6 +1045,18 @@ struct MemoryListener { */ void (*coalesced_io_del)(MemoryListener *listener, MemoryRegionSection *section, hwaddr addr, hwaddr len); + + /** + * @map: + * + * Called during an address space map. + * + * @opaque: User data opaque object + * @addr: address within that address space + * @len: length of buffer + */ + void (*map)(void *opaque, hwaddr addr, hwaddr len); + /** * @priority: * @@ -1054,6 +1066,13 @@ struct MemoryListener { */ unsigned priority; + /** + * @opaque: + * + * Opaque pointer to user data + */ + void *opaque; + /** * @name: * diff --git a/softmmu/physmem.c b/softmmu/physmem.c index 9486a1ebdf..0f8bad6b40 100644 --- a/softmmu/physmem.c +++ b/softmmu/physmem.c @@ -3246,6 +3246,38 @@ flatview_extend_translation(FlatView *fv, hwaddr addr, } } +enum ListenerDirection { Forward, Reverse }; + +/* + * This will require a change to the memory listener callbacks: + * while it currently uses "self" as first argument for the callbacks, this new + * approach is going to use an "opaque" member, effectively following the model + * used for MemoryRegion and MemoryRegionOps. + */ +#define MEMORY_LISTENER_CALL(_as, _callback, _direction, _args...) \ + do { \ + MemoryListener *_listener; \ + \ + switch (_direction) { \ + case Forward: \ + QTAILQ_FOREACH(_listener, &(_as)->listeners, link_as) { \ + if (_listener->_callback) { \ + _listener->_callback(_listener->opaque, ##_args); \ + } \ + } \ + break; \ + case Reverse: \ + QTAILQ_FOREACH_REVERSE(_listener, &(_as)->listeners, link_as) { \ + if (_listener->_callback) { \ + _listener->_callback(_listener->opaque, ##_args); \ + } \ + } \ + break; \ + default: \ + abort(); \ + } \ + } while (0) + /* Map a physical memory region into a host virtual address. * May map a subset of the requested range, given by and returned in *plen. * May return NULL if resources needed to perform the mapping are exhausted. @@ -3268,6 +3300,8 @@ void *address_space_map(AddressSpace *as, return NULL; } + MEMORY_LISTENER_CALL(as, map, Reverse, addr, len); + l = len; RCU_READ_LOCK_GUARD(); fv = address_space_to_flatview(as); -- 2.40.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-04-05 14:27 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-04-05 12:57 [PATCH RFC 0/1] MemoryListener address_space_map callback Antonio Caggiano 2023-04-05 12:57 ` [PATCH RFC 1/1] memory: Address space map listener Antonio Caggiano 2023-04-05 13:23 ` David Hildenbrand 2023-04-05 14:25 ` Antonio Caggiano 2023-04-05 14:26 ` David Hildenbrand [not found] <20230405111515.62137-1-quic_acaggian@quicinc.com> 2023-04-05 11:15 ` Antonio Caggiano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).