Re: [Xen-devel] [PATCH] IOMMU: don't BUG() on exotic hardware

["Xu, Quan" <quan.xu@xxxxxxxxx>]

On May 06, 2016 10:24 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> On x86, iommu_get_ops() BUG()s when running on non-Intel, non-AMD
> hardware. While, with our current code, that's a correct prerequisite
> assumption for IOMMU presence, this is wrong on systems without IOMMU.
> Hence iommu_enabled (and alike) checks should be done prior to calling that
> function, not after.
> 
> Also move iommu_suspend() next to iommu_resume() - it escapes me why
> iommu_do_domctl() had got put between the two.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -337,11 +337,16 @@ int __init iommu_setup(void)
>      return rc;
>  }
> 
> +void iommu_suspend()
> +{
> +    if ( iommu_enabled )
> +        iommu_get_ops()->suspend();
> +}
> +


What about this style:

+void iommu_suspend()
+{
+    if ( iommu_enabled &&
+         iommu_get_ops()->suspend )
+        iommu_get_ops()->suspend();
+}
+

At least for AMD, not all of the .callback are  initialized.

>  void iommu_crash_shutdown(void)
>  {
> -    const struct iommu_ops *ops = iommu_get_ops();
>      if ( iommu_enabled )
> -        ops->crash_shutdown();
> +        iommu_get_ops()->crash_shutdown();
>      iommu_enabled = iommu_intremap = iommu_intpost = 0;

btw, is this line still a code style issue?

}


Quan

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