Re: [QEMU][PATCH v3 3/7] softmmu: let qemu_map_ram_ptr() use qemu_ram_ptr_length()
On Tue, Feb 27, 2024 at 11:37 PM Vikram Garhwal
wrote:
> From: Juergen Gross
>
> qemu_map_ram_ptr() and qemu_ram_ptr_length() share quite some code, so
> modify qemu_ram_ptr_length() a little bit and use it for
> qemu_map_ram_ptr(), too.
>
> Signed-off-by: Juergen Gross
> Signed-off-by: Vikram Garhwal
> Reviewed-by: Stefano Stabellini
>
Reviewed-by: Edgar E. Iglesias
> ---
> system/physmem.c | 56
> 1 file changed, 23 insertions(+), 33 deletions(-)
>
> diff --git a/system/physmem.c b/system/physmem.c
> index 84f3022099..949dcb20ba 100644
> --- a/system/physmem.c
> +++ b/system/physmem.c
> @@ -2163,43 +2163,17 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t
> length)
> }
> #endif /* !_WIN32 */
>
> -/* Return a host pointer to ram allocated with qemu_ram_alloc.
> - * This should not be used for general purpose DMA. Use address_space_map
> - * or address_space_rw instead. For local memory (e.g. video ram) that the
> - * device owns, use memory_region_get_ram_ptr.
> - *
> - * Called within RCU critical section.
> - */
> -void *qemu_map_ram_ptr(RAMBlock *block, ram_addr_t addr)
> -{
> -if (block == NULL) {
> -block = qemu_get_ram_block(addr);
> -addr -= block->offset;
> -}
> -
> -if (xen_enabled() && block->host == NULL) {
> -/* We need to check if the requested address is in the RAM
> - * because we don't want to map the entire memory in QEMU.
> - * In that case just map until the end of the page.
> - */
> -if (block->offset == 0) {
> -return xen_map_cache(addr, 0, 0, false);
> -}
> -
> -block->host = xen_map_cache(block->offset, block->max_length, 1,
> false);
> -}
> -return ramblock_ptr(block, addr);
> -}
> -
> -/* Return a host pointer to guest's ram. Similar to qemu_map_ram_ptr
> - * but takes a size argument.
> +/*
> + * Return a host pointer to guest's ram.
> *
> * Called within RCU critical section.
> */
> static void *qemu_ram_ptr_length(RAMBlock *block, ram_addr_t addr,
> hwaddr *size, bool lock)
> {
> -if (*size == 0) {
> +hwaddr len = 0;
> +
> +if (size && *size == 0) {
> return NULL;
> }
>
> @@ -2207,7 +2181,10 @@ static void *qemu_ram_ptr_length(RAMBlock *block,
> ram_addr_t addr,
> block = qemu_get_ram_block(addr);
> addr -= block->offset;
> }
> -*size = MIN(*size, block->max_length - addr);
> +if (size) {
> +*size = MIN(*size, block->max_length - addr);
> +len = *size;
> +}
>
> if (xen_enabled() && block->host == NULL) {
> /* We need to check if the requested address is in the RAM
> @@ -2215,7 +2192,7 @@ static void *qemu_ram_ptr_length(RAMBlock *block,
> ram_addr_t addr,
> * In that case just map the requested area.
> */
> if (block->offset == 0) {
> -return xen_map_cache(addr, *size, lock, lock);
> +return xen_map_cache(addr, len, lock, lock);
> }
>
> block->host = xen_map_cache(block->offset, block->max_length, 1,
> lock);
> @@ -2224,6 +2201,19 @@ static void *qemu_ram_ptr_length(RAMBlock *block,
> ram_addr_t addr,
> return ramblock_ptr(block, addr);
> }
>
> +/*
> + * Return a host pointer to ram allocated with qemu_ram_alloc.
> + * This should not be used for general purpose DMA. Use address_space_map
> + * or address_space_rw instead. For local memory (e.g. video ram) that the
> + * device owns, use memory_region_get_ram_ptr.
> + *
> + * Called within RCU critical section.
> + */
> +void *qemu_map_ram_ptr(RAMBlock *ram_block, ram_addr_t addr)
> +{
> +return qemu_ram_ptr_length(ram_block, addr, NULL, false);
> +}
> +
> /* Return the offset of a hostpointer within a ramblock */
> ram_addr_t qemu_ram_block_host_offset(RAMBlock *rb, void *host)
> {
> --
> 2.17.1
>
>
>
Re: [QEMU][PATCH v3 3/7] softmmu: let qemu_map_ram_ptr() use qemu_ram_ptr_length()
On Fri, Mar 01, 2024 at 05:04:54PM +, Alex Bennée wrote:
> Vikram Garhwal writes:
>
> > From: Juergen Gross
> >
> > qemu_map_ram_ptr() and qemu_ram_ptr_length() share quite some code, so
> > modify qemu_ram_ptr_length() a little bit and use it for
> > qemu_map_ram_ptr(), too.
> >
> > Signed-off-by: Juergen Gross
> > Signed-off-by: Vikram Garhwal
> > Reviewed-by: Stefano Stabellini
> > ---
> > system/physmem.c | 56
> > 1 file changed, 23 insertions(+), 33 deletions(-)
> >
>
> > -
> > -/* Return a host pointer to guest's ram. Similar to qemu_map_ram_ptr
> > - * but takes a size argument.
> > +/*
> > + * Return a host pointer to guest's ram.
> > *
> > * Called within RCU critical section.
> > */
>
> If you end up re-spinning it would be nice to kdoc this function and at
> least call out size as a return by ref and optional.
Will do if re-spinning is needed.
>
> > static void *qemu_ram_ptr_length(RAMBlock *block, ram_addr_t addr,
> > hwaddr *size, bool lock)
> > {
> > -if (*size == 0) {
> > +hwaddr len = 0;
> > +
> > +if (size && *size == 0) {
> > return NULL;
> > }
> >
> > @@ -2207,7 +2181,10 @@ static void *qemu_ram_ptr_length(RAMBlock *block,
> > ram_addr_t addr,
> > block = qemu_get_ram_block(addr);
> > addr -= block->offset;
> > }
> > -*size = MIN(*size, block->max_length - addr);
> > +if (size) {
> > +*size = MIN(*size, block->max_length - addr);
> > +len = *size;
> > +}
> >
> > if (xen_enabled() && block->host == NULL) {
> > /* We need to check if the requested address is in the RAM
> > @@ -2215,7 +2192,7 @@ static void *qemu_ram_ptr_length(RAMBlock *block,
> > ram_addr_t addr,
> > * In that case just map the requested area.
> > */
> > if (block->offset == 0) {
> > -return xen_map_cache(addr, *size, lock, lock);
> > +return xen_map_cache(addr, len, lock, lock);
>
> I did wonder if len == 0 will confuse things but it seems xen_map_cache
> will default to XC_PAGE_SIZE in that case.
>
> Anyway:
>
> Reviewed-by: Alex Bennée
>
> --
> Alex Bennée
> Virtualisation Tech Lead @ Linaro
Re: [QEMU][PATCH v3 3/7] softmmu: let qemu_map_ram_ptr() use qemu_ram_ptr_length()
Vikram Garhwal writes:
> From: Juergen Gross
>
> qemu_map_ram_ptr() and qemu_ram_ptr_length() share quite some code, so
> modify qemu_ram_ptr_length() a little bit and use it for
> qemu_map_ram_ptr(), too.
>
> Signed-off-by: Juergen Gross
> Signed-off-by: Vikram Garhwal
> Reviewed-by: Stefano Stabellini
> ---
> system/physmem.c | 56
> 1 file changed, 23 insertions(+), 33 deletions(-)
>
> -
> -/* Return a host pointer to guest's ram. Similar to qemu_map_ram_ptr
> - * but takes a size argument.
> +/*
> + * Return a host pointer to guest's ram.
> *
> * Called within RCU critical section.
> */
If you end up re-spinning it would be nice to kdoc this function and at
least call out size as a return by ref and optional.
> static void *qemu_ram_ptr_length(RAMBlock *block, ram_addr_t addr,
> hwaddr *size, bool lock)
> {
> -if (*size == 0) {
> +hwaddr len = 0;
> +
> +if (size && *size == 0) {
> return NULL;
> }
>
> @@ -2207,7 +2181,10 @@ static void *qemu_ram_ptr_length(RAMBlock *block,
> ram_addr_t addr,
> block = qemu_get_ram_block(addr);
> addr -= block->offset;
> }
> -*size = MIN(*size, block->max_length - addr);
> +if (size) {
> +*size = MIN(*size, block->max_length - addr);
> +len = *size;
> +}
>
> if (xen_enabled() && block->host == NULL) {
> /* We need to check if the requested address is in the RAM
> @@ -2215,7 +2192,7 @@ static void *qemu_ram_ptr_length(RAMBlock *block,
> ram_addr_t addr,
> * In that case just map the requested area.
> */
> if (block->offset == 0) {
> -return xen_map_cache(addr, *size, lock, lock);
> +return xen_map_cache(addr, len, lock, lock);
I did wonder if len == 0 will confuse things but it seems xen_map_cache
will default to XC_PAGE_SIZE in that case.
Anyway:
Reviewed-by: Alex Bennée
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
[QEMU][PATCH v3 3/7] softmmu: let qemu_map_ram_ptr() use qemu_ram_ptr_length()
From: Juergen Gross
qemu_map_ram_ptr() and qemu_ram_ptr_length() share quite some code, so
modify qemu_ram_ptr_length() a little bit and use it for
qemu_map_ram_ptr(), too.
Signed-off-by: Juergen Gross
Signed-off-by: Vikram Garhwal
Reviewed-by: Stefano Stabellini
---
system/physmem.c | 56
1 file changed, 23 insertions(+), 33 deletions(-)
diff --git a/system/physmem.c b/system/physmem.c
index 84f3022099..949dcb20ba 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -2163,43 +2163,17 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
}
#endif /* !_WIN32 */
-/* Return a host pointer to ram allocated with qemu_ram_alloc.
- * This should not be used for general purpose DMA. Use address_space_map
- * or address_space_rw instead. For local memory (e.g. video ram) that the
- * device owns, use memory_region_get_ram_ptr.
- *
- * Called within RCU critical section.
- */
-void *qemu_map_ram_ptr(RAMBlock *block, ram_addr_t addr)
-{
-if (block == NULL) {
-block = qemu_get_ram_block(addr);
-addr -= block->offset;
-}
-
-if (xen_enabled() && block->host == NULL) {
-/* We need to check if the requested address is in the RAM
- * because we don't want to map the entire memory in QEMU.
- * In that case just map until the end of the page.
- */
-if (block->offset == 0) {
-return xen_map_cache(addr, 0, 0, false);
-}
-
-block->host = xen_map_cache(block->offset, block->max_length, 1,
false);
-}
-return ramblock_ptr(block, addr);
-}
-
-/* Return a host pointer to guest's ram. Similar to qemu_map_ram_ptr
- * but takes a size argument.
+/*
+ * Return a host pointer to guest's ram.
*
* Called within RCU critical section.
*/
static void *qemu_ram_ptr_length(RAMBlock *block, ram_addr_t addr,
hwaddr *size, bool lock)
{
-if (*size == 0) {
+hwaddr len = 0;
+
+if (size && *size == 0) {
return NULL;
}
@@ -2207,7 +2181,10 @@ static void *qemu_ram_ptr_length(RAMBlock *block,
ram_addr_t addr,
block = qemu_get_ram_block(addr);
addr -= block->offset;
}
-*size = MIN(*size, block->max_length - addr);
+if (size) {
+*size = MIN(*size, block->max_length - addr);
+len = *size;
+}
if (xen_enabled() && block->host == NULL) {
/* We need to check if the requested address is in the RAM
@@ -2215,7 +2192,7 @@ static void *qemu_ram_ptr_length(RAMBlock *block,
ram_addr_t addr,
* In that case just map the requested area.
*/
if (block->offset == 0) {
-return xen_map_cache(addr, *size, lock, lock);
+return xen_map_cache(addr, len, lock, lock);
}
block->host = xen_map_cache(block->offset, block->max_length, 1, lock);
@@ -2224,6 +2201,19 @@ static void *qemu_ram_ptr_length(RAMBlock *block,
ram_addr_t addr,
return ramblock_ptr(block, addr);
}
+/*
+ * Return a host pointer to ram allocated with qemu_ram_alloc.
+ * This should not be used for general purpose DMA. Use address_space_map
+ * or address_space_rw instead. For local memory (e.g. video ram) that the
+ * device owns, use memory_region_get_ram_ptr.
+ *
+ * Called within RCU critical section.
+ */
+void *qemu_map_ram_ptr(RAMBlock *ram_block, ram_addr_t addr)
+{
+return qemu_ram_ptr_length(ram_block, addr, NULL, false);
+}
+
/* Return the offset of a hostpointer within a ramblock */
ram_addr_t qemu_ram_block_host_offset(RAMBlock *rb, void *host)
{
--
2.17.1
