>-----Original Message-----
>From: Duan, Zhenzhong
>Sent: Wednesday, July 5, 2023 12:56 PM
>Subject: RE: [PATCH 2/2] virtio-iommu: Rework the trace in
>virtio_iommu_set_page_size_mask()
>
>
>
>>-----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)) {
Sorry, I read this piece code again and got a question, if new_mask has finer
granularity than cur_granule, should we allow it to pass even though
BIT(cur_granule) is not set?
Thanks
Zhenzhong
>
>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
