>-----Original Message----- >From: Jean-Philippe Brucker <jean-phili...@linaro.org> >Sent: Thursday, July 6, 2023 10:36 PM >Subject: Re: [PATCH 2/2] virtio-iommu: Rework the trace in >virtio_iommu_set_page_size_mask() > >On Wed, Jul 05, 2023 at 03:16:31PM +0200, Eric Auger wrote: >> >>> 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? >> I think this should work but this is not straightforward to test. >> virtio-iommu would use the current granule for map/unmap. In >map/unmap >> notifiers, this is split into pow2 ranges and cascaded to VFIO through >> vfio_dma_map/unmap. The iova and size are aligned with the smaller >> supported granule. >> >> Jean, do you share this understanding or do I miss something. > >Yes, I also think that would work. The guest would only issue mappings with >the larger granularity, which can be applied by VFIO with a finer granule. >However I doubt we're going to encounter this case, because seeing a >cur_granule larger than 4k here means that a VFIO device has already been >assigned with a large granule like 64k, and we're trying to add a new device >with 4k. This indicates two HW IOMMUs supporting different granules in the >same system, which seems unlikely.
Indeed. Another scenario I can think of is migration from src with 64k granule to dest with 4k, then hotplug a new device with 4k. Not clear if it's allowed to migrate between host with different granule. Thanks Zhenzhong