On Tue, Jul 18, 2023 at 08:21:36PM +0200, Eric Auger wrote:
> At several locations we compute the granule from the config
> page_size_mask using ctz() and then format it in traces using
> BIT(). As the page_size_mask is 64b we should use ctz64 and
> BIT_ULL() for formatting. We failed to be consistent.
>
> Note the page_size_mask is garanteed to be non null. The spec
> mandates the device to set at least one bit, so ctz64 cannot
> return 64. This is garanteed by the fact the device
> initializes the page_size_mask to qemu_target_page_mask()
> and then the page_size_mask is further constrained by
> virtio_iommu_set_page_size_mask() callback which can't
> result in a new mask being null. So if Coverity complains
> round those ctz64/BIT_ULL with CID 1517772 this is a false
> positive
>
> Signed-off-by: Eric Auger <eric.au...@redhat.com>
> Fixes: 94df5b2180 ("virtio-iommu: Fix 64kB host page size VFIO device
> assignment")
Reviewed-by: Jean-Philippe Brucker <jean-phili...@linaro.org>
> ---
> hw/virtio/virtio-iommu.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 201127c488..c6ee4d7a3c 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -852,17 +852,19 @@ static IOMMUTLBEntry
> virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
> VirtIOIOMMUEndpoint *ep;
> uint32_t sid, flags;
> bool bypass_allowed;
> + int granule;
> bool found;
> int i;
>
> interval.low = addr;
> interval.high = addr + 1;
> + granule = ctz64(s->config.page_size_mask);
>
> IOMMUTLBEntry entry = {
> .target_as = &address_space_memory,
> .iova = addr,
> .translated_addr = addr,
> - .addr_mask = (1 << ctz32(s->config.page_size_mask)) - 1,
> + .addr_mask = BIT_ULL(granule) - 1,
> .perm = IOMMU_NONE,
> };
>
> @@ -1115,7 +1117,7 @@ static int
> virtio_iommu_set_page_size_mask(IOMMUMemoryRegion *mr,
> if (s->granule_frozen) {
> int cur_granule = ctz64(cur_mask);
>
> - if (!(BIT(cur_granule) & new_mask)) {
> + if (!(BIT_ULL(cur_granule) & new_mask)) {
> error_setg(errp, "virtio-iommu %s does not support frozen
> granule 0x%llx",
> mr->parent_obj.name, BIT_ULL(cur_granule));
> return -1;
> @@ -1161,7 +1163,7 @@ static void virtio_iommu_freeze_granule(Notifier
> *notifier, void *data)
> }
> s->granule_frozen = true;
> granule = ctz64(s->config.page_size_mask);
> - trace_virtio_iommu_freeze_granule(BIT(granule));
> + trace_virtio_iommu_freeze_granule(BIT_ULL(granule));
> }
>
> static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
> --
> 2.38.1
>
