Re: [Xen-devel] [PATCH v2 1/5] IOMMU: iommu_intremap is x86-only

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: 09 March 2020 10:43
> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Kevin Tian <kevin.tian@xxxxxxxxx>; Stefano Stabellini 
> <sstabellini@xxxxxxxxxx>; Julien Grall
> <julien@xxxxxxx>; Wei Liu <wl@xxxxxxx>; Konrad Wilk <konrad.wilk@xxxxxxxxxx>; 
> George Dunlap
> <George.Dunlap@xxxxxxxxxxxxx>; Andrew Cooper <andrew.cooper3@xxxxxxxxxx>; Ian 
> Jackson
> <ian.jackson@xxxxxxxxxx>; Paul Durrant <paul@xxxxxxx>
> Subject: [PATCH v2 1/5] IOMMU: iommu_intremap is x86-only
> 
> Provide a #define for other cases; it didn't seem worthwhile to me to
> introduce an IOMMU_INTREMAP Kconfig option at this point.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Reviewed-by: Paul Durrant <paul@xxxxxxx>

> ---
> v2: Refine doc adjustment.
> 
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -1299,6 +1299,8 @@ boolean (e.g. `iommu=no`) can override t
>      generation of IOMMUs only supported DMA remapping, and Interrupt 
> Remapping
>      appeared in the second generation.
> 
> +    This option is only valid on x86.
> +
>  *   The `intpost` boolean controls the Posted Interrupt sub-feature.  In
>      combination with APIC acceleration (VT-x APICV, SVM AVIC), the IOMMU can
>      be configured to deliver interrupts from assigned PCI devices directly
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -35,7 +35,6 @@ bool __read_mostly iommu_quarantine = tr
>  bool_t __read_mostly iommu_igfx = 1;
>  bool_t __read_mostly iommu_snoop = 1;
>  bool_t __read_mostly iommu_qinval = 1;
> -enum iommu_intremap __read_mostly iommu_intremap = iommu_intremap_full;
>  bool_t __read_mostly iommu_crash_disable;
> 
>  static bool __hwdom_initdata iommu_hwdom_none;
> @@ -90,8 +89,10 @@ static int __init parse_iommu_param(cons
>              iommu_snoop = val;
>          else if ( (val = parse_boolean("qinval", s, ss)) >= 0 )
>              iommu_qinval = val;
> +#ifndef iommu_intremap
>          else if ( (val = parse_boolean("intremap", s, ss)) >= 0 )
>              iommu_intremap = val ? iommu_intremap_full : iommu_intremap_off;
> +#endif
>          else if ( (val = parse_boolean("intpost", s, ss)) >= 0 )
>              iommu_intpost = val;
>  #ifdef CONFIG_KEXEC
> @@ -474,8 +475,11 @@ int __init iommu_setup(void)
>          rc = iommu_hardware_setup();
>          iommu_enabled = (rc == 0);
>      }
> +
> +#ifndef iommu_intremap
>      if ( !iommu_enabled )
>          iommu_intremap = iommu_intremap_off;
> +#endif
> 
>      if ( (force_iommu && !iommu_enabled) ||
>           (force_intremap && !iommu_intremap) )
> @@ -500,7 +504,9 @@ int __init iommu_setup(void)
>          printk(" - Dom0 mode: %s\n",
>                 iommu_hwdom_passthrough ? "Passthrough" :
>                 iommu_hwdom_strict ? "Strict" : "Relaxed");
> +#ifndef iommu_intremap
>          printk("Interrupt remapping %sabled\n", iommu_intremap ? "en" : 
> "dis");
> +#endif
>          tasklet_init(&iommu_pt_cleanup_tasklet, iommu_free_pagetables, NULL);
>      }
> 
> @@ -558,7 +564,9 @@ void iommu_crash_shutdown(void)
>      if ( iommu_enabled )
>          iommu_get_ops()->crash_shutdown();
>      iommu_enabled = iommu_intpost = 0;
> +#ifndef iommu_intremap
>      iommu_intremap = iommu_intremap_off;
> +#endif
>  }
> 
>  int iommu_get_reserved_device_memory(iommu_grdm_t *func, void *ctxt)
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -27,6 +27,8 @@
>  const struct iommu_init_ops *__initdata iommu_init_ops;
>  struct iommu_ops __read_mostly iommu_ops;
> 
> +enum iommu_intremap __read_mostly iommu_intremap = iommu_intremap_full;
> +
>  int __init iommu_hardware_setup(void)
>  {
>      struct IO_APIC_route_entry **ioapic_entries = NULL;
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -55,21 +55,24 @@ static inline bool_t dfn_eq(dfn_t x, dfn
>  extern bool_t iommu_enable, iommu_enabled;
>  extern bool force_iommu, iommu_quarantine, iommu_verbose, iommu_igfx;
>  extern bool_t iommu_snoop, iommu_qinval, iommu_intpost;
> +
> +#ifdef CONFIG_X86
>  extern enum __packed iommu_intremap {
>     /*
>      * In order to allow traditional boolean uses of the iommu_intremap
>      * variable, the "off" value has to come first (yielding a value of zero).
>      */
>     iommu_intremap_off,
> -#ifdef CONFIG_X86
>     /*
>      * Interrupt remapping enabled, but only able to generate interrupts
>      * with an 8-bit APIC ID.
>      */
>     iommu_intremap_restricted,
> -#endif
>     iommu_intremap_full,
>  } iommu_intremap;
> +#else
> +# define iommu_intremap false
> +#endif
> 
>  #if defined(CONFIG_IOMMU_FORCE_PT_SHARE)
>  #define iommu_hap_pt_share true



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel