>-----Original Message-----
>From: Eric Auger <eric.au...@redhat.com>
>Sent: Tuesday, July 4, 2023 7:15 PM
>To: eric.auger....@gmail.com; eric.au...@redhat.com; qemu-
>de...@nongnu.org; qemu-...@nongnu.org; m...@redhat.com; jean-
>phili...@linaro.org; Duan, Zhenzhong <zhenzhong.d...@intel.com>
>Cc: alex.william...@redhat.com; c...@redhap.com;
>bharat.bhus...@nxp.com; peter.mayd...@linaro.org
>Subject: [PATCH 2/2] virtio-iommu: Rework the trace in
>virtio_iommu_set_page_size_mask()
>
>The current error messages in virtio_iommu_set_page_size_mask()
>sound quite similar for different situations and miss the IOMMU
>memory region that causes the issue.
>
>Clarify them and rework the comment.
>
>Also remove the trace when the new page_size_mask is not applied as
>the current frozen granule is kept. This message is rather confusing
>for the end user and anyway the current granule would have been used
>by the driver
>
>Signed-off-by: Eric Auger <eric.au...@redhat.com>
>---
> hw/virtio/virtio-iommu.c | 19 +++++++------------
> 1 file changed, 7 insertions(+), 12 deletions(-)
>
>diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>index 1eaf81bab5..0d9f7196fe 100644
>--- a/hw/virtio/virtio-iommu.c
>+++ b/hw/virtio/virtio-iommu.c
>@@ -1101,29 +1101,24 @@ static int
>virtio_iommu_set_page_size_mask(IOMMUMemoryRegion *mr,
>                                           new_mask);
>
>     if ((cur_mask & new_mask) == 0) {
>-        error_setg(errp, "virtio-iommu page mask 0x%"PRIx64
>-                   " is incompatible with mask 0x%"PRIx64, cur_mask, 
>new_mask);
>+        error_setg(errp, "virtio-iommu %s reports a page size mask 0x%"PRIx64
>+                   " incompatible with currently supported mask 0x%"PRIx64,
>+                   mr->parent_obj.name, new_mask, cur_mask);
>         return -1;
>     }
>
>     /*
>      * Once the granule is frozen we can't change the mask anymore. If by
>      * chance the hotplugged device supports the same granule, we can still
>-     * accept it. Having a different masks is possible but the guest will use
>-     * sub-optimal block sizes, so warn about it.
>+     * accept it.
>      */
>     if (s->granule_frozen) {
>-        int new_granule = ctz64(new_mask);
>         int cur_granule = ctz64(cur_mask);
>
>-        if (new_granule != cur_granule) {
>-            error_setg(errp, "virtio-iommu page mask 0x%"PRIx64
>-                       " is incompatible with mask 0x%"PRIx64, cur_mask,
>-                       new_mask);
>+        if (!(BIT(cur_granule) & new_mask)) {

Good catch.

Reviewed-by: Zhenzhong Duan <zhenzhong.d...@intel.com>

Thanks
Zhenzhong

>+            error_setg(errp, "virtio-iommu %s does not support frozen granule
>0x%"PRIx64,
>+                       mr->parent_obj.name, BIT(cur_granule));
>             return -1;
>-        } else if (new_mask != cur_mask) {
>-            warn_report("virtio-iommu page mask 0x%"PRIx64
>-                        " does not match 0x%"PRIx64, cur_mask, new_mask);
>         }
>         return 0;
>     }
>--
>2.38.1


Reply via email to