Re: [Xen-devel] [Patch v6 04/11] IOMMU: propagate IOMMU Device-TLB flush error up to IOMMU mapping (top level ones)

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 31.05.16 at 15:57, <quan.xu@xxxxxxxxx> wrote:
> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> @@ -295,12 +297,22 @@ static void __hwdom_init amd_iommu_hwdom_init(struct 
> domain *d)
>               * a pfn_valid() check would seem desirable here.
>               */
>              if ( mfn_valid(pfn) )
> -                amd_iommu_map_page(d, pfn, pfn, 
> -                                   IOMMUF_readable|IOMMUF_writable);
> +            {
> +                int ret;
> +
> +                ret = amd_iommu_map_page(d, pfn, pfn,
> +                                         IOMMUF_readable|IOMMUF_writable);

Same here as for the earlier patch regarding assignment vs initializer.

But overall the entire change to this function seems to rather belong
into patch 2. As would a respective change to
vtd_set_hwdom_mapping(), which I'm not sure which patch you've
put that in.

> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -166,8 +166,8 @@ struct iommu_ops {
>  #endif /* HAS_PCI */
>  
>      void (*teardown)(struct domain *d);
> -    int (*map_page)(struct domain *d, unsigned long gfn, unsigned long mfn,
> -                    unsigned int flags);
> +    int __must_check (*map_page)(struct domain *d, unsigned long gfn,
> +                                 unsigned long mfn, unsigned int flags);

With this and with the rule set forth in the context of the discussion
of v5, iommu_map_page() (as well as any other caller of this hook
that do not themselves _consume_ the error [e.g. hwdom init ones])
should become or already be __must_check, which afaict isn't the
case. The same then, btw., applies to patch 3, and hence I have to
withdraw the R-b that you've got there.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel