flatview_translate()'s @plen argument is output-only and can be NULL.
When Xen is enabled, only update @plen_out when non-NULL.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
include/system/memory.h | 5 +++--
system/physmem.c | 9 +++++----
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/include/system/memory.h b/include/system/memory.h
index aa85fc27a10..3e5bf3ef05e 100644
--- a/include/system/memory.h
+++ b/include/system/memory.h
@@ -2992,13 +2992,14 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
* @addr: address within that address space
* @xlat: pointer to address within the returned memory region section's
* #MemoryRegion.
- * @len: pointer to length
+ * @plen_out: pointer to valid read/write length of the translated address.
+ * It can be @NULL when we don't care about it.
* @is_write: indicates the transfer direction
* @attrs: memory attributes
*/
MemoryRegion *flatview_translate(FlatView *fv,
hwaddr addr, hwaddr *xlat,
- hwaddr *len, bool is_write,
+ hwaddr *plen_out, bool is_write,
MemTxAttrs attrs);
static inline MemoryRegion *address_space_translate(AddressSpace *as,
diff --git a/system/physmem.c b/system/physmem.c
index 8a8be3a80e2..86422f294e2 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -566,7 +566,7 @@ iotlb_fail:
/* Called from RCU critical section */
MemoryRegion *flatview_translate(FlatView *fv, hwaddr addr, hwaddr *xlat,
- hwaddr *plen, bool is_write,
+ hwaddr *plen_out, bool is_write,
MemTxAttrs attrs)
{
MemoryRegion *mr;
@@ -574,13 +574,14 @@ MemoryRegion *flatview_translate(FlatView *fv, hwaddr addr, hwaddr *xlat,
AddressSpace *as = NULL;
/* This can be MMIO, so setup MMIO bit. */
- section = flatview_do_translate(fv, addr, xlat, plen, NULL,
+ section = flatview_do_translate(fv, addr, xlat, plen_out, NULL,
is_write, true, &as, attrs);
mr = section.mr;
- if (xen_enabled() && memory_access_is_direct(mr, is_write, attrs)) {
+ if (xen_enabled() && plen_out && memory_access_is_direct(mr, is_write,
+ attrs)) {
hwaddr page = ((addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE) - addr;
- *plen = MIN(page, *plen);
+ *plen_out = MIN(page, *plen_out);
}
return mr;
--
2.51.0
On 9/30/25 01:21, Philippe Mathieu-Daudé wrote: > flatview_translate()'s @plen argument is output-only and can be NULL. > > When Xen is enabled, only update @plen_out when non-NULL. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > include/system/memory.h | 5 +++-- > system/physmem.c | 9 +++++---- > 2 files changed, 8 insertions(+), 6 deletions(-) > > diff --git a/include/system/memory.h b/include/system/memory.h > index aa85fc27a10..3e5bf3ef05e 100644 > --- a/include/system/memory.h > +++ b/include/system/memory.h > @@ -2992,13 +2992,14 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr, > * @addr: address within that address space > * @xlat: pointer to address within the returned memory region section's > * #MemoryRegion. > - * @len: pointer to length > + * @plen_out: pointer to valid read/write length of the translated address. > + * It can be @NULL when we don't care about it. > * @is_write: indicates the transfer direction > * @attrs: memory attributes > */ > MemoryRegion *flatview_translate(FlatView *fv, > hwaddr addr, hwaddr *xlat, > - hwaddr *len, bool is_write, > + hwaddr *plen_out, bool is_write, > MemTxAttrs attrs); Renaming to plen_out is misleading, because it is used as an input as well: > - *plen = MIN(page, *plen); > + *plen_out = MIN(page, *plen_out); right here. If non-null, the input should be the access length of the caller. r~
On 30/09/2025 10.21, Philippe Mathieu-Daudé wrote:
> flatview_translate()'s @plen argument is output-only and can be NULL.
>
> When Xen is enabled, only update @plen_out when non-NULL.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> include/system/memory.h | 5 +++--
> system/physmem.c | 9 +++++----
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/include/system/memory.h b/include/system/memory.h
> index aa85fc27a10..3e5bf3ef05e 100644
> --- a/include/system/memory.h
> +++ b/include/system/memory.h
> @@ -2992,13 +2992,14 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
> * @addr: address within that address space
> * @xlat: pointer to address within the returned memory region section's
> * #MemoryRegion.
> - * @len: pointer to length
> + * @plen_out: pointer to valid read/write length of the translated address.
> + * It can be @NULL when we don't care about it.
> * @is_write: indicates the transfer direction
> * @attrs: memory attributes
> */
> MemoryRegion *flatview_translate(FlatView *fv,
> hwaddr addr, hwaddr *xlat,
> - hwaddr *len, bool is_write,
> + hwaddr *plen_out, bool is_write,
> MemTxAttrs attrs);
>
> static inline MemoryRegion *address_space_translate(AddressSpace *as,
> diff --git a/system/physmem.c b/system/physmem.c
> index 8a8be3a80e2..86422f294e2 100644
> --- a/system/physmem.c
> +++ b/system/physmem.c
> @@ -566,7 +566,7 @@ iotlb_fail:
>
> /* Called from RCU critical section */
> MemoryRegion *flatview_translate(FlatView *fv, hwaddr addr, hwaddr *xlat,
> - hwaddr *plen, bool is_write,
> + hwaddr *plen_out, bool is_write,
> MemTxAttrs attrs)
> {
> MemoryRegion *mr;
> @@ -574,13 +574,14 @@ MemoryRegion *flatview_translate(FlatView *fv, hwaddr addr, hwaddr *xlat,
> AddressSpace *as = NULL;
>
> /* This can be MMIO, so setup MMIO bit. */
> - section = flatview_do_translate(fv, addr, xlat, plen, NULL,
> + section = flatview_do_translate(fv, addr, xlat, plen_out, NULL,
> is_write, true, &as, attrs);
> mr = section.mr;
>
> - if (xen_enabled() && memory_access_is_direct(mr, is_write, attrs)) {
> + if (xen_enabled() && plen_out && memory_access_is_direct(mr, is_write,
> + attrs)) {
> hwaddr page = ((addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE) - addr;
> - *plen = MIN(page, *plen);
> + *plen_out = MIN(page, *plen_out);
> }
My question from the previous version is still unanswered:
https://lore.kernel.org/qemu-devel/22ff756a-51a2-43f4-8fe1-05f17ff4a371@redhat.com/
Thomas
Hi Thomas,
On 30/9/25 10:24, Thomas Huth wrote:
> On 30/09/2025 10.21, Philippe Mathieu-Daudé wrote:
>> flatview_translate()'s @plen argument is output-only and can be NULL.
>>
>> When Xen is enabled, only update @plen_out when non-NULL.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> include/system/memory.h | 5 +++--
>> system/physmem.c | 9 +++++----
>> 2 files changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/system/memory.h b/include/system/memory.h
>> index aa85fc27a10..3e5bf3ef05e 100644
>> --- a/include/system/memory.h
>> +++ b/include/system/memory.h
>> @@ -2992,13 +2992,14 @@ IOMMUTLBEntry
>> address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
>> * @addr: address within that address space
>> * @xlat: pointer to address within the returned memory region
>> section's
>> * #MemoryRegion.
>> - * @len: pointer to length
>> + * @plen_out: pointer to valid read/write length of the translated
>> address.
>> + * It can be @NULL when we don't care about it.
>> * @is_write: indicates the transfer direction
>> * @attrs: memory attributes
>> */
>> MemoryRegion *flatview_translate(FlatView *fv,
>> hwaddr addr, hwaddr *xlat,
>> - hwaddr *len, bool is_write,
>> + hwaddr *plen_out, bool is_write,
>> MemTxAttrs attrs);
>> static inline MemoryRegion *address_space_translate(AddressSpace *as,
>> diff --git a/system/physmem.c b/system/physmem.c
>> index 8a8be3a80e2..86422f294e2 100644
>> --- a/system/physmem.c
>> +++ b/system/physmem.c
>> @@ -566,7 +566,7 @@ iotlb_fail:
>> /* Called from RCU critical section */
>> MemoryRegion *flatview_translate(FlatView *fv, hwaddr addr, hwaddr
>> *xlat,
>> - hwaddr *plen, bool is_write,
>> + hwaddr *plen_out, bool is_write,
>> MemTxAttrs attrs)
>> {
>> MemoryRegion *mr;
>> @@ -574,13 +574,14 @@ MemoryRegion *flatview_translate(FlatView *fv,
>> hwaddr addr, hwaddr *xlat,
>> AddressSpace *as = NULL;
>> /* This can be MMIO, so setup MMIO bit. */
>> - section = flatview_do_translate(fv, addr, xlat, plen, NULL,
>> + section = flatview_do_translate(fv, addr, xlat, plen_out, NULL,
>> is_write, true, &as, attrs);
>> mr = section.mr;
>> - if (xen_enabled() && memory_access_is_direct(mr, is_write, attrs)) {
>> + if (xen_enabled() && plen_out && memory_access_is_direct(mr,
>> is_write,
>> + attrs)) {
>> hwaddr page = ((addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE)
>> - addr;
>> - *plen = MIN(page, *plen);
>> + *plen_out = MIN(page, *plen_out);
>> }
>
> My question from the previous version is still unanswered:
>
> https://lore.kernel.org/qemu-
> devel/22ff756a-51a2-43f4-8fe1-05f17ff4a371@redhat.com/
This patches
- checks for plen not being NULL
- describes it as
"When Xen is enabled, only update @plen_out when non-NULL."
- mention that in the updated flatview_translate() documentation
"It can be @NULL when we don't care about it." as documented for
the flatview_do_translate() callee in commit d5e5fafd11b ("exec:
add page_mask for flatview_do_translate")
before:
it was not clear whether we can pass plen=NULL without having
to look at the code.
after:
no change when plen is not NULL, we can pass plen=NULL safely
(it is documented).
I shouldn't be understanding your original question, do you mind
rewording it?
Thanks,
Phil.
On 30/09/2025 10.31, Philippe Mathieu-Daudé wrote:
> Hi Thomas,
>
> On 30/9/25 10:24, Thomas Huth wrote:
>> On 30/09/2025 10.21, Philippe Mathieu-Daudé wrote:
>>> flatview_translate()'s @plen argument is output-only and can be NULL.
>>>
>>> When Xen is enabled, only update @plen_out when non-NULL.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>> include/system/memory.h | 5 +++--
>>> system/physmem.c | 9 +++++----
>>> 2 files changed, 8 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/include/system/memory.h b/include/system/memory.h
>>> index aa85fc27a10..3e5bf3ef05e 100644
>>> --- a/include/system/memory.h
>>> +++ b/include/system/memory.h
>>> @@ -2992,13 +2992,14 @@ IOMMUTLBEntry
>>> address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
>>> * @addr: address within that address space
>>> * @xlat: pointer to address within the returned memory region section's
>>> * #MemoryRegion.
>>> - * @len: pointer to length
>>> + * @plen_out: pointer to valid read/write length of the translated address.
>>> + * It can be @NULL when we don't care about it.
>>> * @is_write: indicates the transfer direction
>>> * @attrs: memory attributes
>>> */
>>> MemoryRegion *flatview_translate(FlatView *fv,
>>> hwaddr addr, hwaddr *xlat,
>>> - hwaddr *len, bool is_write,
>>> + hwaddr *plen_out, bool is_write,
>>> MemTxAttrs attrs);
>>> static inline MemoryRegion *address_space_translate(AddressSpace *as,
>>> diff --git a/system/physmem.c b/system/physmem.c
>>> index 8a8be3a80e2..86422f294e2 100644
>>> --- a/system/physmem.c
>>> +++ b/system/physmem.c
>>> @@ -566,7 +566,7 @@ iotlb_fail:
>>> /* Called from RCU critical section */
>>> MemoryRegion *flatview_translate(FlatView *fv, hwaddr addr, hwaddr *xlat,
>>> - hwaddr *plen, bool is_write,
>>> + hwaddr *plen_out, bool is_write,
>>> MemTxAttrs attrs)
>>> {
>>> MemoryRegion *mr;
>>> @@ -574,13 +574,14 @@ MemoryRegion *flatview_translate(FlatView *fv,
>>> hwaddr addr, hwaddr *xlat,
>>> AddressSpace *as = NULL;
>>> /* This can be MMIO, so setup MMIO bit. */
>>> - section = flatview_do_translate(fv, addr, xlat, plen, NULL,
>>> + section = flatview_do_translate(fv, addr, xlat, plen_out, NULL,
>>> is_write, true, &as, attrs);
>>> mr = section.mr;
>>> - if (xen_enabled() && memory_access_is_direct(mr, is_write, attrs)) {
>>> + if (xen_enabled() && plen_out && memory_access_is_direct(mr, is_write,
>>> + attrs)) {
>>> hwaddr page = ((addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE) -
>>> addr;
>>> - *plen = MIN(page, *plen);
>>> + *plen_out = MIN(page, *plen_out);
>>> }
>>
>> My question from the previous version is still unanswered:
>>
>> https://lore.kernel.org/qemu-
>> devel/22ff756a-51a2-43f4-8fe1-05f17ff4a371@redhat.com/
>
> This patches
> - checks for plen not being NULL
> - describes it as
> "When Xen is enabled, only update @plen_out when non-NULL."
> - mention that in the updated flatview_translate() documentation
> "It can be @NULL when we don't care about it." as documented for
> the flatview_do_translate() callee in commit d5e5fafd11b ("exec:
> add page_mask for flatview_do_translate")
>
> before:
>
> it was not clear whether we can pass plen=NULL without having
> to look at the code.
>
> after:
>
> no change when plen is not NULL, we can pass plen=NULL safely
> (it is documented).
>
> I shouldn't be understanding your original question, do you mind
> rewording it?
Ah, you've updated the patch in v3 to include a check for plen_out in the
if-statement! It was not there in v2. Ok, this should be fine now:
Reviewed-by: Thomas Huth <thuth@redhat.com>
I just re-complained since you did not respond to my mail in v2, and when I
looked at the changelog in your v3 cover letter, you did not mention the
modification here, so I blindly assumed that this patch was unchanged.
Thomas
On 30/9/25 11:18, Thomas Huth wrote:
> On 30/09/2025 10.31, Philippe Mathieu-Daudé wrote:
>> Hi Thomas,
>>
>> On 30/9/25 10:24, Thomas Huth wrote:
>>> On 30/09/2025 10.21, Philippe Mathieu-Daudé wrote:
>>>> flatview_translate()'s @plen argument is output-only and can be NULL.
>>>>
>>>> When Xen is enabled, only update @plen_out when non-NULL.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> ---
>>>> include/system/memory.h | 5 +++--
>>>> system/physmem.c | 9 +++++----
>>>> 2 files changed, 8 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/include/system/memory.h b/include/system/memory.h
>>>> index aa85fc27a10..3e5bf3ef05e 100644
>>>> --- a/include/system/memory.h
>>>> +++ b/include/system/memory.h
>>>> @@ -2992,13 +2992,14 @@ IOMMUTLBEntry
>>>> address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
>>>> * @addr: address within that address space
>>>> * @xlat: pointer to address within the returned memory region
>>>> section's
>>>> * #MemoryRegion.
>>>> - * @len: pointer to length
>>>> + * @plen_out: pointer to valid read/write length of the translated
>>>> address.
>>>> + * It can be @NULL when we don't care about it.
>>>> * @is_write: indicates the transfer direction
>>>> * @attrs: memory attributes
>>>> */
>>>> MemoryRegion *flatview_translate(FlatView *fv,
>>>> hwaddr addr, hwaddr *xlat,
>>>> - hwaddr *len, bool is_write,
>>>> + hwaddr *plen_out, bool is_write,
>>>> MemTxAttrs attrs);
>>>> static inline MemoryRegion *address_space_translate(AddressSpace *as,
>>>> diff --git a/system/physmem.c b/system/physmem.c
>>>> index 8a8be3a80e2..86422f294e2 100644
>>>> --- a/system/physmem.c
>>>> +++ b/system/physmem.c
>>>> @@ -566,7 +566,7 @@ iotlb_fail:
>>>> /* Called from RCU critical section */
>>>> MemoryRegion *flatview_translate(FlatView *fv, hwaddr addr, hwaddr
>>>> *xlat,
>>>> - hwaddr *plen, bool is_write,
>>>> + hwaddr *plen_out, bool is_write,
>>>> MemTxAttrs attrs)
>>>> {
>>>> MemoryRegion *mr;
>>>> @@ -574,13 +574,14 @@ MemoryRegion *flatview_translate(FlatView *fv,
>>>> hwaddr addr, hwaddr *xlat,
>>>> AddressSpace *as = NULL;
>>>> /* This can be MMIO, so setup MMIO bit. */
>>>> - section = flatview_do_translate(fv, addr, xlat, plen, NULL,
>>>> + section = flatview_do_translate(fv, addr, xlat, plen_out, NULL,
>>>> is_write, true, &as, attrs);
>>>> mr = section.mr;
>>>> - if (xen_enabled() && memory_access_is_direct(mr, is_write,
>>>> attrs)) {
>>>> + if (xen_enabled() && plen_out && memory_access_is_direct(mr,
>>>> is_write,
>>>> + attrs)) {
>>>> hwaddr page = ((addr & TARGET_PAGE_MASK) +
>>>> TARGET_PAGE_SIZE) - addr;
>>>> - *plen = MIN(page, *plen);
>>>> + *plen_out = MIN(page, *plen_out);
>>>> }
>>>
>>> My question from the previous version is still unanswered:
>>>
>>> https://lore.kernel.org/qemu-
>>> devel/22ff756a-51a2-43f4-8fe1-05f17ff4a371@redhat.com/
>>
>> This patches
>> - checks for plen not being NULL
>> - describes it as
>> "When Xen is enabled, only update @plen_out when non-NULL."
>> - mention that in the updated flatview_translate() documentation
>> "It can be @NULL when we don't care about it." as documented for
>> the flatview_do_translate() callee in commit d5e5fafd11b ("exec:
>> add page_mask for flatview_do_translate")
>>
>> before:
>>
>> it was not clear whether we can pass plen=NULL without having
>> to look at the code.
>>
>> after:
>>
>> no change when plen is not NULL, we can pass plen=NULL safely
>> (it is documented).
>>
>> I shouldn't be understanding your original question, do you mind
>> rewording it?
>
> Ah, you've updated the patch in v3 to include a check for plen_out in
> the if-statement! It was not there in v2. Ok, this should be fine now:
>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
>
> I just re-complained since you did not respond to my mail in v2, and
> when I looked at the changelog in your v3 cover letter, you did not
> mention the modification here, so I blindly assumed that this patch was
> unchanged.
Ah I see... OK I'll try to be more explicit in my respins.
Thanks for your review!
Phil.
© 2016 - 2025 Red Hat, Inc.