Re: [PATCH v4 07/14] vfio/common: Add helper to consolidate iova/end calculation
On 07/03/2023 09:52, Avihai Horon wrote:
>
> On 07/03/2023 4:02, Joao Martins wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> In preparation to be used in device dirty tracking, move the code that
>> calculates a iova/end range from the container/section. This avoids
>> duplication on the common checks across listener callbacks.
>>
>> Signed-off-by: Joao Martins
>> ---
>> hw/vfio/common.c | 37 ++---
>> 1 file changed, 30 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index 54b4a4fc7daf..3a6491dbc523 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -961,6 +961,35 @@ static bool
>> vfio_listener_valid_section(MemoryRegionSection *section)
>> return true;
>> }
>>
>> +/*
>> + * Called for the dirty tracking memory listener to calculate the iova/end
>> + * for a given memory region section.
>> + */
>
> Should we just delete this comment? The function name is pretty clear.
> Besides that, the comment is not completely accurate -- in this patch we are
> not
> using it yet for dirty tracking and it's also used for region_{add,del}.
>
Yes, let me delete it.
> Thanks.
>
>> +static bool vfio_get_section_iova_range(VFIOContainer *container,
>> + MemoryRegionSection *section,
>> + hwaddr *out_iova, hwaddr *out_end,
>> + Int128 *out_llend)
>> +{
>> + Int128 llend;
>> + hwaddr iova;
>> +
>> + iova = REAL_HOST_PAGE_ALIGN(section->offset_within_address_space);
>> + llend = int128_make64(section->offset_within_address_space);
>> + llend = int128_add(llend, section->size);
>> + llend = int128_and(llend, int128_exts64(qemu_real_host_page_mask()));
>> +
>> + if (int128_ge(int128_make64(iova), llend)) {
>> + return false;
>> + }
>> +
>> + *out_iova = iova;
>> + *out_end = int128_get64(int128_sub(llend, int128_one()));
>> + if (out_llend) {
>> + *out_llend = llend;
>> + }
>> + return true;
>> +}
>> +
>> static void vfio_listener_region_add(MemoryListener *listener,
>> MemoryRegionSection *section)
>> {
>> @@ -976,12 +1005,7 @@ static void vfio_listener_region_add(MemoryListener
>> *listener,
>> return;
>> }
>>
>> - iova = REAL_HOST_PAGE_ALIGN(section->offset_within_address_space);
>> - llend = int128_make64(section->offset_within_address_space);
>> - llend = int128_add(llend, section->size);
>> - llend = int128_and(llend, int128_exts64(qemu_real_host_page_mask()));
>> -
>> - if (int128_ge(int128_make64(iova), llend)) {
>> + if (!vfio_get_section_iova_range(container, section, , ,
>> )) {
>> if (memory_region_is_ram_device(section->mr)) {
>> trace_vfio_listener_region_add_no_dma_map(
>> memory_region_name(section->mr),
>> @@ -991,7 +1015,6 @@ static void vfio_listener_region_add(MemoryListener
>> *listener,
>> }
>> return;
>> }
>> - end = int128_get64(int128_sub(llend, int128_one()));
>>
>> if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
>> hwaddr pgsize = 0;
>> --
>> 2.17.2
>>
Re: [PATCH v4 07/14] vfio/common: Add helper to consolidate iova/end calculation
On 07/03/2023 02:40, Alex Williamson wrote:
> On Tue, 7 Mar 2023 02:02:51 +
> Joao Martins wrote:
>
>> In preparation to be used in device dirty tracking, move the code that
>> calculates a iova/end range from the container/section. This avoids
>> duplication on the common checks across listener callbacks.
>>
>> Signed-off-by: Joao Martins
>> ---
>> hw/vfio/common.c | 37 ++---
>> 1 file changed, 30 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index 54b4a4fc7daf..3a6491dbc523 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -961,6 +961,35 @@ static bool
>> vfio_listener_valid_section(MemoryRegionSection *section)
>> return true;
>> }
>>
>> +/*
>> + * Called for the dirty tracking memory listener to calculate the iova/end
>> + * for a given memory region section.
>> + */
>> +static bool vfio_get_section_iova_range(VFIOContainer *container,
>> +MemoryRegionSection *section,
>> +hwaddr *out_iova, hwaddr *out_end,
>> +Int128 *out_llend)
>> +{
>> +Int128 llend;
>> +hwaddr iova;
>> +
>> +iova = REAL_HOST_PAGE_ALIGN(section->offset_within_address_space);
>> +llend = int128_make64(section->offset_within_address_space);
>> +llend = int128_add(llend, section->size);
>> +llend = int128_and(llend, int128_exts64(qemu_real_host_page_mask()));
>> +
>> +if (int128_ge(int128_make64(iova), llend)) {
>> +return false;
>> +}
>> +
>> +*out_iova = iova;
>> +*out_end = int128_get64(int128_sub(llend, int128_one()));
>> +if (out_llend) {
>> +*out_llend = llend;
>> +}
>> +return true;
>> +}
>> +
>> static void vfio_listener_region_add(MemoryListener *listener,
>> MemoryRegionSection *section)
>> {
>> @@ -976,12 +1005,7 @@ static void vfio_listener_region_add(MemoryListener
>> *listener,
>> return;
>> }
>>
>> -iova = REAL_HOST_PAGE_ALIGN(section->offset_within_address_space);
>> -llend = int128_make64(section->offset_within_address_space);
>> -llend = int128_add(llend, section->size);
>> -llend = int128_and(llend, int128_exts64(qemu_real_host_page_mask()));
>> -
>> -if (int128_ge(int128_make64(iova), llend)) {
>> +if (!vfio_get_section_iova_range(container, section, , ,
>> )) {
>> if (memory_region_is_ram_device(section->mr)) {
>> trace_vfio_listener_region_add_no_dma_map(
>> memory_region_name(section->mr),
>> @@ -991,7 +1015,6 @@ static void vfio_listener_region_add(MemoryListener
>> *listener,
>> }
>> return;
>> }
>> -end = int128_get64(int128_sub(llend, int128_one()));
>>
>> if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
>> hwaddr pgsize = 0;
>
> Shouldn't this convert vfio_listener_region_del() too? Thanks,
Yeap.
Re: [PATCH v4 07/14] vfio/common: Add helper to consolidate iova/end calculation
On 07/03/2023 4:02, Joao Martins wrote:
External email: Use caution opening links or attachments
In preparation to be used in device dirty tracking, move the code that
calculates a iova/end range from the container/section. This avoids
duplication on the common checks across listener callbacks.
Signed-off-by: Joao Martins
---
hw/vfio/common.c | 37 ++---
1 file changed, 30 insertions(+), 7 deletions(-)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 54b4a4fc7daf..3a6491dbc523 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -961,6 +961,35 @@ static bool
vfio_listener_valid_section(MemoryRegionSection *section)
return true;
}
+/*
+ * Called for the dirty tracking memory listener to calculate the iova/end
+ * for a given memory region section.
+ */
Should we just delete this comment? The function name is pretty clear.
Besides that, the comment is not completely accurate -- in this patch we
are not using it yet for dirty tracking and it's also used for
region_{add,del}.
Thanks.
+static bool vfio_get_section_iova_range(VFIOContainer *container,
+MemoryRegionSection *section,
+hwaddr *out_iova, hwaddr *out_end,
+Int128 *out_llend)
+{
+Int128 llend;
+hwaddr iova;
+
+iova = REAL_HOST_PAGE_ALIGN(section->offset_within_address_space);
+llend = int128_make64(section->offset_within_address_space);
+llend = int128_add(llend, section->size);
+llend = int128_and(llend, int128_exts64(qemu_real_host_page_mask()));
+
+if (int128_ge(int128_make64(iova), llend)) {
+return false;
+}
+
+*out_iova = iova;
+*out_end = int128_get64(int128_sub(llend, int128_one()));
+if (out_llend) {
+*out_llend = llend;
+}
+return true;
+}
+
static void vfio_listener_region_add(MemoryListener *listener,
MemoryRegionSection *section)
{
@@ -976,12 +1005,7 @@ static void vfio_listener_region_add(MemoryListener
*listener,
return;
}
-iova = REAL_HOST_PAGE_ALIGN(section->offset_within_address_space);
-llend = int128_make64(section->offset_within_address_space);
-llend = int128_add(llend, section->size);
-llend = int128_and(llend, int128_exts64(qemu_real_host_page_mask()));
-
-if (int128_ge(int128_make64(iova), llend)) {
+if (!vfio_get_section_iova_range(container, section, , , ))
{
if (memory_region_is_ram_device(section->mr)) {
trace_vfio_listener_region_add_no_dma_map(
memory_region_name(section->mr),
@@ -991,7 +1015,6 @@ static void vfio_listener_region_add(MemoryListener
*listener,
}
return;
}
-end = int128_get64(int128_sub(llend, int128_one()));
if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
hwaddr pgsize = 0;
--
2.17.2
Re: [PATCH v4 07/14] vfio/common: Add helper to consolidate iova/end calculation
On Tue, 7 Mar 2023 02:02:51 +
Joao Martins wrote:
> In preparation to be used in device dirty tracking, move the code that
> calculates a iova/end range from the container/section. This avoids
> duplication on the common checks across listener callbacks.
>
> Signed-off-by: Joao Martins
> ---
> hw/vfio/common.c | 37 ++---
> 1 file changed, 30 insertions(+), 7 deletions(-)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 54b4a4fc7daf..3a6491dbc523 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -961,6 +961,35 @@ static bool
> vfio_listener_valid_section(MemoryRegionSection *section)
> return true;
> }
>
> +/*
> + * Called for the dirty tracking memory listener to calculate the iova/end
> + * for a given memory region section.
> + */
> +static bool vfio_get_section_iova_range(VFIOContainer *container,
> +MemoryRegionSection *section,
> +hwaddr *out_iova, hwaddr *out_end,
> +Int128 *out_llend)
> +{
> +Int128 llend;
> +hwaddr iova;
> +
> +iova = REAL_HOST_PAGE_ALIGN(section->offset_within_address_space);
> +llend = int128_make64(section->offset_within_address_space);
> +llend = int128_add(llend, section->size);
> +llend = int128_and(llend, int128_exts64(qemu_real_host_page_mask()));
> +
> +if (int128_ge(int128_make64(iova), llend)) {
> +return false;
> +}
> +
> +*out_iova = iova;
> +*out_end = int128_get64(int128_sub(llend, int128_one()));
> +if (out_llend) {
> +*out_llend = llend;
> +}
> +return true;
> +}
> +
> static void vfio_listener_region_add(MemoryListener *listener,
> MemoryRegionSection *section)
> {
> @@ -976,12 +1005,7 @@ static void vfio_listener_region_add(MemoryListener
> *listener,
> return;
> }
>
> -iova = REAL_HOST_PAGE_ALIGN(section->offset_within_address_space);
> -llend = int128_make64(section->offset_within_address_space);
> -llend = int128_add(llend, section->size);
> -llend = int128_and(llend, int128_exts64(qemu_real_host_page_mask()));
> -
> -if (int128_ge(int128_make64(iova), llend)) {
> +if (!vfio_get_section_iova_range(container, section, , ,
> )) {
> if (memory_region_is_ram_device(section->mr)) {
> trace_vfio_listener_region_add_no_dma_map(
> memory_region_name(section->mr),
> @@ -991,7 +1015,6 @@ static void vfio_listener_region_add(MemoryListener
> *listener,
> }
> return;
> }
> -end = int128_get64(int128_sub(llend, int128_one()));
>
> if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
> hwaddr pgsize = 0;
Shouldn't this convert vfio_listener_region_del() too? Thanks,
Alex
