Re: [PULL v4 15/47] intel_iommu: support snoop control

[Peter Maydell]

On Mon, 7 Mar 2022 at 22:45, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> From: Jason Wang <jasowang@redhat.com>
>
> SC is required for some kernel features like vhost-vDPA. So this patch
> implements basic SC feature. The idea is pretty simple, for software
> emulated DMA it would be always coherent. In this case we can simple
> advertise ECAP_SC bit. For VFIO and vhost, thing will be more much
> complicated, so this patch simply fail the IOMMU notifier
> registration.
>
> In the future, we may want to have a dedicated notifiers flag or
> similar mechanism to demonstrate the coherency so VFIO could advertise
> that if it has VFIO_DMA_CC_IOMMU, for vhost kernel backend we don't
> need that since it's a software backend.

Hi; Coverity points out (CID 1487174) an error in this change:

> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -3030,6 +3030,13 @@ static int 
> vtd_iommu_notify_flag_changed(IOMMUMemoryRegion *iommu,
>      VTDAddressSpace *vtd_as = container_of(iommu, VTDAddressSpace, iommu);
>      IntelIOMMUState *s = vtd_as->iommu_state;
>
> +    /* TODO: add support for VFIO and vhost users */
> +    if (s->snoop_control) {
> +        error_setg_errno(errp, -ENOTSUP,
> +                         "Snoop Control with vhost or VFIO is not 
> supported");
> +        return -ENOTSUP;
> +    }

error_setg_errno() expects a normal errno value, not a negated
one, so we should be passing it "ENOTSUP" here, not "-ENOTSUP".

thanks
-- PMM