Newer Xen versions expose two Xen feature flags to tell us if the domain
is directly mapped or not. Only when a domain is directly mapped it
makes sense to enable swiotlb-xen on ARM.

Introduce a function on ARM to check the new Xen feature flags and also
to deal with the legacy case. Call the function xen_swiotlb_detect.

Also rename the existing pci_xen_swiotlb_detect on x86 to
xen_swiotlb_detect so that we can share a common function declaration.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
---

This is the corresponding Xen patch under review:
https://marc.info/?l=xen-devel&m=161421618217686

We don't *have to* make the x86 function and the ARM function exactly
the same, but I thought it would be much nicer if we did. However, we
can't really call it pci_* on ARM as there is no PCI necessarily.

---
 arch/arm/xen/mm.c                      | 14 +++++++++++++-
 arch/arm64/mm/dma-mapping.c            |  2 +-
 arch/x86/include/asm/xen/swiotlb-xen.h |  4 ++--
 arch/x86/kernel/pci-swiotlb.c          |  2 +-
 arch/x86/xen/pci-swiotlb-xen.c         |  6 +++---
 include/xen/interface/features.h       |  7 +++++++
 include/xen/swiotlb-xen.h              |  6 ++++++
 7 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
index 467fa225c3d0..f8e5acbef05d 100644
--- a/arch/arm/xen/mm.c
+++ b/arch/arm/xen/mm.c
@@ -135,10 +135,22 @@ void xen_destroy_contiguous_region(phys_addr_t pstart, unsigned int order)
 	return;
 }
 
+int __init xen_swiotlb_detect(void)
+{
+	if (!xen_domain())
+		return 0;
+	if (xen_feature(XENFEAT_direct_mapped))
+		return 1;
+	/* legacy case */
+	if (!xen_feature(XENFEAT_not_direct_mapped) && xen_initial_domain())
+		return 1;
+	return 0;
+}
+
 static int __init xen_mm_init(void)
 {
 	struct gnttab_cache_flush cflush;
-	if (!xen_initial_domain())
+	if (!xen_swiotlb_detect())
 		return 0;
 	xen_swiotlb_init(1, false);
 
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 93e87b287556..4bf1dd3eb041 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -53,7 +53,7 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
 		iommu_setup_dma_ops(dev, dma_base, size);
 
 #ifdef CONFIG_XEN
-	if (xen_initial_domain())
+	if (xen_swiotlb_detect())
 		dev->dma_ops = &xen_swiotlb_dma_ops;
 #endif
 }
diff --git a/arch/x86/include/asm/xen/swiotlb-xen.h b/arch/x86/include/asm/xen/swiotlb-xen.h
index 6b56d0d45d15..494694744844 100644
--- a/arch/x86/include/asm/xen/swiotlb-xen.h
+++ b/arch/x86/include/asm/xen/swiotlb-xen.h
@@ -2,14 +2,14 @@
 #ifndef _ASM_X86_SWIOTLB_XEN_H
 #define _ASM_X86_SWIOTLB_XEN_H
 
+#include <xen/swiotlb-xen.h>
+
 #ifdef CONFIG_SWIOTLB_XEN
 extern int xen_swiotlb;
-extern int __init pci_xen_swiotlb_detect(void);
 extern void __init pci_xen_swiotlb_init(void);
 extern int pci_xen_swiotlb_init_late(void);
 #else
 #define xen_swiotlb (0)
-static inline int __init pci_xen_swiotlb_detect(void) { return 0; }
 static inline void __init pci_xen_swiotlb_init(void) { }
 static inline int pci_xen_swiotlb_init_late(void) { return -ENXIO; }
 #endif
diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c
index c2cfa5e7c152..c18eb6629326 100644
--- a/arch/x86/kernel/pci-swiotlb.c
+++ b/arch/x86/kernel/pci-swiotlb.c
@@ -30,7 +30,7 @@ int __init pci_swiotlb_detect_override(void)
 	return swiotlb;
 }
 IOMMU_INIT_FINISH(pci_swiotlb_detect_override,
-		  pci_xen_swiotlb_detect,
+		  xen_swiotlb_detect,
 		  pci_swiotlb_init,
 		  pci_swiotlb_late_init);
 
diff --git a/arch/x86/xen/pci-swiotlb-xen.c b/arch/x86/xen/pci-swiotlb-xen.c
index 19ae3e4fe4e9..0a35657eeb85 100644
--- a/arch/x86/xen/pci-swiotlb-xen.c
+++ b/arch/x86/xen/pci-swiotlb-xen.c
@@ -21,12 +21,12 @@
 int xen_swiotlb __read_mostly;
 
 /*
- * pci_xen_swiotlb_detect - set xen_swiotlb to 1 if necessary
+ * xen_swiotlb_detect - set xen_swiotlb to 1 if necessary
  *
  * This returns non-zero if we are forced to use xen_swiotlb (by the boot
  * option).
  */
-int __init pci_xen_swiotlb_detect(void)
+int __init xen_swiotlb_detect(void)
 {
 
 	if (!xen_pv_domain())
@@ -90,7 +90,7 @@ int pci_xen_swiotlb_init_late(void)
 }
 EXPORT_SYMBOL_GPL(pci_xen_swiotlb_init_late);
 
-IOMMU_INIT_FINISH(pci_xen_swiotlb_detect,
+IOMMU_INIT_FINISH(xen_swiotlb_detect,
 		  NULL,
 		  pci_xen_swiotlb_init,
 		  NULL);
diff --git a/include/xen/interface/features.h b/include/xen/interface/features.h
index 6d1384abfbdf..f0d00bb0ac63 100644
--- a/include/xen/interface/features.h
+++ b/include/xen/interface/features.h
@@ -83,6 +83,13 @@
  */
 #define XENFEAT_linux_rsdp_unrestricted   15
 
+/*
+ * A direct-mapped (or 1:1 mapped) domain is a domain for which its
+ * local pages have gfn == mfn.
+ */
+#define XENFEAT_not_direct_mapped         16
+#define XENFEAT_direct_mapped             17
+
 #define XENFEAT_NR_SUBMAPS 1
 
 #endif /* __XEN_PUBLIC_FEATURES_H__ */
diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
index d5eaf9d682b8..6a2fc4e4b838 100644
--- a/include/xen/swiotlb-xen.h
+++ b/include/xen/swiotlb-xen.h
@@ -12,4 +12,10 @@ void xen_dma_sync_for_device(struct device *dev, dma_addr_t handle,
 extern int xen_swiotlb_init(int verbose, bool early);
 extern const struct dma_map_ops xen_swiotlb_dma_ops;
 
+#ifdef CONFIG_SWIOTLB_XEN
+extern int __init xen_swiotlb_detect(void);
+#else
+static inline int __init xen_swiotlb_detect(void) { return 0; }
+#endif
+
 #endif /* __LINUX_SWIOTLB_XEN_H */
-- 
2.17.1

Gentle ping, now that the Xen side is acked (not committed due to the
imminent release):

https://marc.info/?l=xen-devel&m=161559099506456


On Thu, 25 Feb 2021, Stefano Stabellini wrote:

> Newer Xen versions expose two Xen feature flags to tell us if the domain
> is directly mapped or not. Only when a domain is directly mapped it
> makes sense to enable swiotlb-xen on ARM.
> 
> Introduce a function on ARM to check the new Xen feature flags and also
> to deal with the legacy case. Call the function xen_swiotlb_detect.
> 
> Also rename the existing pci_xen_swiotlb_detect on x86 to
> xen_swiotlb_detect so that we can share a common function declaration.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> ---
> 
> This is the corresponding Xen patch under review:
> https://marc.info/?l=xen-devel&m=161421618217686
> 
> We don't *have to* make the x86 function and the ARM function exactly
> the same, but I thought it would be much nicer if we did. However, we
> can't really call it pci_* on ARM as there is no PCI necessarily.
> 
> ---
>  arch/arm/xen/mm.c                      | 14 +++++++++++++-
>  arch/arm64/mm/dma-mapping.c            |  2 +-
>  arch/x86/include/asm/xen/swiotlb-xen.h |  4 ++--
>  arch/x86/kernel/pci-swiotlb.c          |  2 +-
>  arch/x86/xen/pci-swiotlb-xen.c         |  6 +++---
>  include/xen/interface/features.h       |  7 +++++++
>  include/xen/swiotlb-xen.h              |  6 ++++++
>  7 files changed, 33 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
> index 467fa225c3d0..f8e5acbef05d 100644
> --- a/arch/arm/xen/mm.c
> +++ b/arch/arm/xen/mm.c
> @@ -135,10 +135,22 @@ void xen_destroy_contiguous_region(phys_addr_t pstart, unsigned int order)
>  	return;
>  }
>  
> +int __init xen_swiotlb_detect(void)
> +{
> +	if (!xen_domain())
> +		return 0;
> +	if (xen_feature(XENFEAT_direct_mapped))
> +		return 1;
> +	/* legacy case */
> +	if (!xen_feature(XENFEAT_not_direct_mapped) && xen_initial_domain())
> +		return 1;
> +	return 0;
> +}
> +
>  static int __init xen_mm_init(void)
>  {
>  	struct gnttab_cache_flush cflush;
> -	if (!xen_initial_domain())
> +	if (!xen_swiotlb_detect())
>  		return 0;
>  	xen_swiotlb_init(1, false);
>  
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index 93e87b287556..4bf1dd3eb041 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -53,7 +53,7 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
>  		iommu_setup_dma_ops(dev, dma_base, size);
>  
>  #ifdef CONFIG_XEN
> -	if (xen_initial_domain())
> +	if (xen_swiotlb_detect())
>  		dev->dma_ops = &xen_swiotlb_dma_ops;
>  #endif
>  }
> diff --git a/arch/x86/include/asm/xen/swiotlb-xen.h b/arch/x86/include/asm/xen/swiotlb-xen.h
> index 6b56d0d45d15..494694744844 100644
> --- a/arch/x86/include/asm/xen/swiotlb-xen.h
> +++ b/arch/x86/include/asm/xen/swiotlb-xen.h
> @@ -2,14 +2,14 @@
>  #ifndef _ASM_X86_SWIOTLB_XEN_H
>  #define _ASM_X86_SWIOTLB_XEN_H
>  
> +#include <xen/swiotlb-xen.h>
> +
>  #ifdef CONFIG_SWIOTLB_XEN
>  extern int xen_swiotlb;
> -extern int __init pci_xen_swiotlb_detect(void);
>  extern void __init pci_xen_swiotlb_init(void);
>  extern int pci_xen_swiotlb_init_late(void);
>  #else
>  #define xen_swiotlb (0)
> -static inline int __init pci_xen_swiotlb_detect(void) { return 0; }
>  static inline void __init pci_xen_swiotlb_init(void) { }
>  static inline int pci_xen_swiotlb_init_late(void) { return -ENXIO; }
>  #endif
> diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c
> index c2cfa5e7c152..c18eb6629326 100644
> --- a/arch/x86/kernel/pci-swiotlb.c
> +++ b/arch/x86/kernel/pci-swiotlb.c
> @@ -30,7 +30,7 @@ int __init pci_swiotlb_detect_override(void)
>  	return swiotlb;
>  }
>  IOMMU_INIT_FINISH(pci_swiotlb_detect_override,
> -		  pci_xen_swiotlb_detect,
> +		  xen_swiotlb_detect,
>  		  pci_swiotlb_init,
>  		  pci_swiotlb_late_init);
>  
> diff --git a/arch/x86/xen/pci-swiotlb-xen.c b/arch/x86/xen/pci-swiotlb-xen.c
> index 19ae3e4fe4e9..0a35657eeb85 100644
> --- a/arch/x86/xen/pci-swiotlb-xen.c
> +++ b/arch/x86/xen/pci-swiotlb-xen.c
> @@ -21,12 +21,12 @@
>  int xen_swiotlb __read_mostly;
>  
>  /*
> - * pci_xen_swiotlb_detect - set xen_swiotlb to 1 if necessary
> + * xen_swiotlb_detect - set xen_swiotlb to 1 if necessary
>   *
>   * This returns non-zero if we are forced to use xen_swiotlb (by the boot
>   * option).
>   */
> -int __init pci_xen_swiotlb_detect(void)
> +int __init xen_swiotlb_detect(void)
>  {
>  
>  	if (!xen_pv_domain())
> @@ -90,7 +90,7 @@ int pci_xen_swiotlb_init_late(void)
>  }
>  EXPORT_SYMBOL_GPL(pci_xen_swiotlb_init_late);
>  
> -IOMMU_INIT_FINISH(pci_xen_swiotlb_detect,
> +IOMMU_INIT_FINISH(xen_swiotlb_detect,
>  		  NULL,
>  		  pci_xen_swiotlb_init,
>  		  NULL);
> diff --git a/include/xen/interface/features.h b/include/xen/interface/features.h
> index 6d1384abfbdf..f0d00bb0ac63 100644
> --- a/include/xen/interface/features.h
> +++ b/include/xen/interface/features.h
> @@ -83,6 +83,13 @@
>   */
>  #define XENFEAT_linux_rsdp_unrestricted   15
>  
> +/*
> + * A direct-mapped (or 1:1 mapped) domain is a domain for which its
> + * local pages have gfn == mfn.
> + */
> +#define XENFEAT_not_direct_mapped         16
> +#define XENFEAT_direct_mapped             17
> +
>  #define XENFEAT_NR_SUBMAPS 1
>  
>  #endif /* __XEN_PUBLIC_FEATURES_H__ */
> diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
> index d5eaf9d682b8..6a2fc4e4b838 100644
> --- a/include/xen/swiotlb-xen.h
> +++ b/include/xen/swiotlb-xen.h
> @@ -12,4 +12,10 @@ void xen_dma_sync_for_device(struct device *dev, dma_addr_t handle,
>  extern int xen_swiotlb_init(int verbose, bool early);
>  extern const struct dma_map_ops xen_swiotlb_dma_ops;
>  
> +#ifdef CONFIG_SWIOTLB_XEN
> +extern int __init xen_swiotlb_detect(void);
> +#else
> +static inline int __init xen_swiotlb_detect(void) { return 0; }
> +#endif
> +
>  #endif /* __LINUX_SWIOTLB_XEN_H */
> -- 
> 2.17.1
> 

On 2/25/21 6:51 PM, Stefano Stabellini wrote:
> Newer Xen versions expose two Xen feature flags to tell us if the domain
> is directly mapped or not. Only when a domain is directly mapped it
> makes sense to enable swiotlb-xen on ARM.
>
> Introduce a function on ARM to check the new Xen feature flags and also
> to deal with the legacy case. Call the function xen_swiotlb_detect.
>
> Also rename the existing pci_xen_swiotlb_detect on x86 to
> xen_swiotlb_detect so that we can share a common function declaration.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> ---
>
> This is the corresponding Xen patch under review:
> https://marc.info/?l=xen-devel&m=161421618217686
>
> We don't *have to* make the x86 function and the ARM function exactly
> the same, but I thought it would be much nicer if we did. However, we
> can't really call it pci_* on ARM as there is no PCI necessarily.


I would prefer to keep existing names for consistency on x86 side (but making that inconsistent with ARM, as you point out).  But if you feel strongly about making the change you would have to have x86 maintainers agree to this (arch/x86/kernel/pci-swiotlb.c).


-boris

On Wed, 17 Mar 2021, Boris Ostrovsky wrote:
> On 2/25/21 6:51 PM, Stefano Stabellini wrote:
> > Newer Xen versions expose two Xen feature flags to tell us if the domain
> > is directly mapped or not. Only when a domain is directly mapped it
> > makes sense to enable swiotlb-xen on ARM.
> >
> > Introduce a function on ARM to check the new Xen feature flags and also
> > to deal with the legacy case. Call the function xen_swiotlb_detect.
> >
> > Also rename the existing pci_xen_swiotlb_detect on x86 to
> > xen_swiotlb_detect so that we can share a common function declaration.
> >
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> > ---
> >
> > This is the corresponding Xen patch under review:
> > https://marc.info/?l=xen-devel&m=161421618217686
> >
> > We don't *have to* make the x86 function and the ARM function exactly
> > the same, but I thought it would be much nicer if we did. However, we
> > can't really call it pci_* on ARM as there is no PCI necessarily.
> 
> 
> I would prefer to keep existing names for consistency on x86 side (but making that inconsistent with ARM, as you point out).  But if you feel strongly about making the change you would have to have x86 maintainers agree to this (arch/x86/kernel/pci-swiotlb.c).

OK. And from an x86 point of view, it makes sense for it to be called
pci_xen_swiotlb_detect given that it is passed to IOMMU_INIT_FINISH in
arch/x86/kernel/pci-swiotlb.c.

So, I'll follow you suggestion, keep the x86 side named as it is today,
and provide a tiny wrapper so that we can still have an arch-neutral
xen_swiotlb_detect function (on x86 just calls pci_xen_swiotlb_detect.)

See below.


From 5f6c91990df1e834c78501799905b04916105836 Mon Sep 17 00:00:00 2001
From: Stefano Stabellini <stefano.stabellini@xilinx.com>
Date: Thu, 18 Mar 2021 16:17:21 -0700
Subject: [PATCH] xen/arm: introduce XENFEAT_direct_mapped and
 XENFEAT_not_direct_mapped

Newer Xen versions expose two Xen feature flags to tell us if the domain
is directly mapped or not. Only when a domain is directly mapped it
makes sense to enable swiotlb-xen on ARM.

Introduce a function on ARM to check the new Xen feature flags and also
to deal with the legacy case. Call the function xen_swiotlb_detect.

Also implement xen_swiotlb_detect on x88 using the existing
pci_xen_swiotlb_detect() so that we can share a common function
declaration.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
---
 arch/arm/xen/mm.c                      | 14 +++++++++++++-
 arch/arm64/mm/dma-mapping.c            |  2 +-
 arch/x86/include/asm/xen/swiotlb-xen.h |  4 ++--
 arch/x86/xen/pci-swiotlb-xen.c         |  7 ++++++-
 include/xen/interface/features.h       | 14 ++++++++++++++
 include/xen/swiotlb-xen.h              |  6 ++++++
 6 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
index 467fa225c3d0..e1b12b242a32 100644
--- a/arch/arm/xen/mm.c
+++ b/arch/arm/xen/mm.c
@@ -135,10 +135,22 @@ void xen_destroy_contiguous_region(phys_addr_t pstart, unsigned int order)
 	return;
 }
 
+int xen_swiotlb_detect(void)
+{
+	if (!xen_domain())
+		return 0;
+	if (xen_feature(XENFEAT_direct_mapped))
+		return 1;
+	/* legacy case */
+	if (!xen_feature(XENFEAT_not_direct_mapped) && xen_initial_domain())
+		return 1;
+	return 0;
+}
+
 static int __init xen_mm_init(void)
 {
 	struct gnttab_cache_flush cflush;
-	if (!xen_initial_domain())
+	if (!xen_swiotlb_detect())
 		return 0;
 	xen_swiotlb_init(1, false);
 
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 93e87b287556..4bf1dd3eb041 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -53,7 +53,7 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
 		iommu_setup_dma_ops(dev, dma_base, size);
 
 #ifdef CONFIG_XEN
-	if (xen_initial_domain())
+	if (xen_swiotlb_detect())
 		dev->dma_ops = &xen_swiotlb_dma_ops;
 #endif
 }
diff --git a/arch/x86/include/asm/xen/swiotlb-xen.h b/arch/x86/include/asm/xen/swiotlb-xen.h
index 6b56d0d45d15..66d576486509 100644
--- a/arch/x86/include/asm/xen/swiotlb-xen.h
+++ b/arch/x86/include/asm/xen/swiotlb-xen.h
@@ -4,12 +4,12 @@
 
 #ifdef CONFIG_SWIOTLB_XEN
 extern int xen_swiotlb;
-extern int __init pci_xen_swiotlb_detect(void);
+extern int pci_xen_swiotlb_detect(void);
 extern void __init pci_xen_swiotlb_init(void);
 extern int pci_xen_swiotlb_init_late(void);
 #else
 #define xen_swiotlb (0)
-static inline int __init pci_xen_swiotlb_detect(void) { return 0; }
+static inline int pci_xen_swiotlb_detect(void) { return 0; }
 static inline void __init pci_xen_swiotlb_init(void) { }
 static inline int pci_xen_swiotlb_init_late(void) { return -ENXIO; }
 #endif
diff --git a/arch/x86/xen/pci-swiotlb-xen.c b/arch/x86/xen/pci-swiotlb-xen.c
index 19ae3e4fe4e9..0555f68a773f 100644
--- a/arch/x86/xen/pci-swiotlb-xen.c
+++ b/arch/x86/xen/pci-swiotlb-xen.c
@@ -26,7 +26,7 @@ int xen_swiotlb __read_mostly;
  * This returns non-zero if we are forced to use xen_swiotlb (by the boot
  * option).
  */
-int __init pci_xen_swiotlb_detect(void)
+int pci_xen_swiotlb_detect(void)
 {
 
 	if (!xen_pv_domain())
@@ -56,6 +56,11 @@ int __init pci_xen_swiotlb_detect(void)
 	return xen_swiotlb;
 }
 
+int xen_swiotlb_detect(void)
+{
+	return pci_xen_swiotlb_detect();
+}
+
 void __init pci_xen_swiotlb_init(void)
 {
 	if (xen_swiotlb) {
diff --git a/include/xen/interface/features.h b/include/xen/interface/features.h
index 6d1384abfbdf..5a7bdefa06a8 100644
--- a/include/xen/interface/features.h
+++ b/include/xen/interface/features.h
@@ -83,6 +83,20 @@
  */
 #define XENFEAT_linux_rsdp_unrestricted   15
 
+/*
+ * A direct-mapped (or 1:1 mapped) domain is a domain for which its
+ * local pages have gfn == mfn. If a domain is direct-mapped,
+ * XENFEAT_direct_mapped is set; otherwise XENFEAT_not_direct_mapped
+ * is set.
+ *
+ * If neither flag is set (e.g. older Xen releases) the assumptions are:
+ * - not auto_translated domains (x86 only) are always direct-mapped
+ * - on x86, auto_translated domains are not direct-mapped
+ * - on ARM, Dom0 is direct-mapped, DomUs are not
+ */
+#define XENFEAT_not_direct_mapped         16
+#define XENFEAT_direct_mapped             17
+
 #define XENFEAT_NR_SUBMAPS 1
 
 #endif /* __XEN_PUBLIC_FEATURES_H__ */
diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
index d5eaf9d682b8..336356b61738 100644
--- a/include/xen/swiotlb-xen.h
+++ b/include/xen/swiotlb-xen.h
@@ -12,4 +12,10 @@ void xen_dma_sync_for_device(struct device *dev, dma_addr_t handle,
 extern int xen_swiotlb_init(int verbose, bool early);
 extern const struct dma_map_ops xen_swiotlb_dma_ops;
 
+#ifdef CONFIG_SWIOTLB_XEN
+extern int xen_swiotlb_detect(void);
+#else
+static inline int xen_swiotlb_detect(void) { return 0; }
+#endif
+
 #endif /* __LINUX_SWIOTLB_XEN_H */
-- 
2.17.1

On 3/18/21 7:28 PM, Stefano Stabellini wrote:
>
> So, I'll follow you suggestion, keep the x86 side named as it is today,
> and provide a tiny wrapper so that we can still have an arch-neutral
> xen_swiotlb_detect function (on x86 just calls pci_xen_swiotlb_detect.)


But now on x86 side we end up with a routine that noone calls. And pci_xen_swiotlb_detect() becomes not __init and so it will have to stick around after boot for no good reason. (You could have made it __ref btw).


I think we should leave x86 alone. And if there is a declaration in include/xen/swiotlb-xen.h that's only relevant to ARM --- well, so be it. Or perhaps you can create arch/arm/include/asm/xen/swiotlb-xen.h


-boris

On Fri, 19 Mar 2021, Boris Ostrovsky wrote:
> On 3/18/21 7:28 PM, Stefano Stabellini wrote:
> >
> > So, I'll follow you suggestion, keep the x86 side named as it is today,
> > and provide a tiny wrapper so that we can still have an arch-neutral
> > xen_swiotlb_detect function (on x86 just calls pci_xen_swiotlb_detect.)
> 
> 
> But now on x86 side we end up with a routine that noone calls. And pci_xen_swiotlb_detect() becomes not __init and so it will have to stick around after boot for no good reason. (You could have made it __ref btw).
> 
> 
> I think we should leave x86 alone. And if there is a declaration in include/xen/swiotlb-xen.h that's only relevant to ARM --- well, so be it. Or perhaps you can create arch/arm/include/asm/xen/swiotlb-xen.h

Yeah I wanted to avoid creating arch/arm/include/asm/xen/swiotlb-xen.h
because I would have to do one of the following:

1) add one more #include <asm/xen/swiotlb-xen.h> to arch/arm64/mm/dma-mapping.c
   (#include <xen/swiotlb-xen.h> is there already)
2) add #include <asm/xen/swiotlb-xen.h> to include/xen/swiotlb-xen.h

What's your preference? If I have to create
arch/arm/include/asm/xen/swiotlb-xen.h, I would go with 2).

On 3/19/21 1:53 PM, Stefano Stabellini wrote:
> On Fri, 19 Mar 2021, Boris Ostrovsky wrote:
>> On 3/18/21 7:28 PM, Stefano Stabellini wrote:
>>> So, I'll follow you suggestion, keep the x86 side named as it is today,
>>> and provide a tiny wrapper so that we can still have an arch-neutral
>>> xen_swiotlb_detect function (on x86 just calls pci_xen_swiotlb_detect.)
>>
>> But now on x86 side we end up with a routine that noone calls. And pci_xen_swiotlb_detect() becomes not __init and so it will have to stick around after boot for no good reason. (You could have made it __ref btw).
>>
>>
>> I think we should leave x86 alone. And if there is a declaration in include/xen/swiotlb-xen.h that's only relevant to ARM --- well, so be it. Or perhaps you can create arch/arm/include/asm/xen/swiotlb-xen.h
> Yeah I wanted to avoid creating arch/arm/include/asm/xen/swiotlb-xen.h
> because I would have to do one of the following:
>
> 1) add one more #include <asm/xen/swiotlb-xen.h> to arch/arm64/mm/dma-mapping.c
>    (#include <xen/swiotlb-xen.h> is there already)
> 2) add #include <asm/xen/swiotlb-xen.h> to include/xen/swiotlb-xen.h
>
> What's your preference? If I have to create
> arch/arm/include/asm/xen/swiotlb-xen.h, I would go with 2).


Yes, I agree --- #2 is the better choice.


-boris

Hi Stefano,

On 19/03/2021 17:53, Stefano Stabellini wrote:
> On Fri, 19 Mar 2021, Boris Ostrovsky wrote:
>> On 3/18/21 7:28 PM, Stefano Stabellini wrote:
>>>
>>> So, I'll follow you suggestion, keep the x86 side named as it is today,
>>> and provide a tiny wrapper so that we can still have an arch-neutral
>>> xen_swiotlb_detect function (on x86 just calls pci_xen_swiotlb_detect.)
>>
>>
>> But now on x86 side we end up with a routine that noone calls. And pci_xen_swiotlb_detect() becomes not __init and so it will have to stick around after boot for no good reason. (You could have made it __ref btw).
>>
>>
>> I think we should leave x86 alone. And if there is a declaration in include/xen/swiotlb-xen.h that's only relevant to ARM --- well, so be it. Or perhaps you can create arch/arm/include/asm/xen/swiotlb-xen.h
> 
> Yeah I wanted to avoid creating arch/arm/include/asm/xen/swiotlb-xen.h

If the header is going to be useed by arch/arm and arch/arm64, then it 
should be defined in include/xen/arm/ to avoid any packaging issue.

Cheers,

-- 
Julien Grall