[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH 2/2] virtio-iommu: Rework the trace in virtio_iommu_set_page_
From: |
Duan, Zhenzhong |
Subject: |
RE: [PATCH 2/2] virtio-iommu: Rework the trace in virtio_iommu_set_page_size_mask() |
Date: |
Fri, 7 Jul 2023 02:34:47 +0000 |
>-----Original Message-----
>From: Jean-Philippe Brucker <jean-philippe@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
- RE: [PATCH 1/2] virtio-iommu: Fix 64kB host page size VFIO device assignment, (continued)
[PATCH 2/2] virtio-iommu: Rework the trace in virtio_iommu_set_page_size_mask(), Eric Auger, 2023/07/04
- RE: [PATCH 2/2] virtio-iommu: Rework the trace in virtio_iommu_set_page_size_mask(), Duan, Zhenzhong, 2023/07/05
- RE: [PATCH 2/2] virtio-iommu: Rework the trace in virtio_iommu_set_page_size_mask(), Duan, Zhenzhong, 2023/07/05
- Re: [PATCH 2/2] virtio-iommu: Rework the trace in virtio_iommu_set_page_size_mask(), Eric Auger, 2023/07/05
- RE: [PATCH 2/2] virtio-iommu: Rework the trace in virtio_iommu_set_page_size_mask(), Duan, Zhenzhong, 2023/07/06
- Re: [PATCH 2/2] virtio-iommu: Rework the trace in virtio_iommu_set_page_size_mask(), Jean-Philippe Brucker, 2023/07/06
- Re: [PATCH 2/2] virtio-iommu: Rework the trace in virtio_iommu_set_page_size_mask(), Eric Auger, 2023/07/06
- RE: [PATCH 2/2] virtio-iommu: Rework the trace in virtio_iommu_set_page_size_mask(),
Duan, Zhenzhong <=
- Re: [PATCH 2/2] virtio-iommu: Rework the trace in virtio_iommu_set_page_size_mask(), Michael S. Tsirkin, 2023/07/06