[PATCH] xen: arm: Enable 1:1 workaround by default

[PATCH] xen: arm: Enable 1:1 workaround by default

[Ian Campbell]

I was just about to send out patches adding the 1:1 workaround to vexpress (the
foundation model is a vexpress platfrom with DMA) and sunxi.

That would have meant that all platforms now implement the quirk. Instead lets
just make it the default and remove the quirk.

In the future this will likely be set based on the presence absence of an
IOMMU, perhaps with additional overrides by the platform.

A command line option no-dom0_11_mapping is provided.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/domain_build.c      | 5 ++++-
 xen/arch/arm/platforms/exynos5.c | 6 ------
 xen/arch/arm/platforms/midway.c  | 6 ------
 xen/arch/arm/platforms/omap5.c   | 6 ------
 xen/include/asm-arm/platform.h   | 6 ------
 5 files changed, 4 insertions(+), 25 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index e9bb01f..ed22613 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -22,6 +22,9 @@
 static unsigned int __initdata opt_dom0_max_vcpus;
 integer_param("dom0_max_vcpus", opt_dom0_max_vcpus);
 
+static int opt_dom0_11_mapping = 1;
+boolean_param("dom0_11_mapping", opt_dom0_11_mapping);
+
 #define DOM0_MEM_DEFAULT 0x8000000 /* 128 MiB */
 static u64 __initdata dom0_mem = DOM0_MEM_DEFAULT;
 
@@ -110,7 +113,7 @@ static void allocate_memory(struct domain *d, struct kernel_info *kinfo)
     u32 reg_len, reg_size;
     unsigned int bank = 0;
 
-    if ( platform_has_quirk(PLATFORM_QUIRK_DOM0_MAPPING_11) )
+    if ( opt_dom0_11_mapping )
         return allocate_memory_11(d, kinfo);
 
     while ( (memory = dt_find_node_by_type(memory, "memory")) )
diff --git a/xen/arch/arm/platforms/exynos5.c b/xen/arch/arm/platforms/exynos5.c
index 0e76cac..fa6b465 100644
--- a/xen/arch/arm/platforms/exynos5.c
+++ b/xen/arch/arm/platforms/exynos5.c
@@ -111,11 +111,6 @@ static void exynos5_reset(void)
     iounmap(pmu);
 }
 
-static uint32_t exynos5_quirks(void)
-{
-    return PLATFORM_QUIRK_DOM0_MAPPING_11;
-}
-
 static const char * const exynos5_dt_compat[] __initconst =
 {
     "samsung,exynos5250",
@@ -139,7 +134,6 @@ PLATFORM_START(exynos5, "SAMSUNG EXYNOS5")
     .smp_init = exynos5_smp_init,
     .cpu_up = exynos5_cpu_up,
     .reset = exynos5_reset,
-    .quirks = exynos5_quirks,
     .blacklist_dev = exynos5_blacklist_dev,
 PLATFORM_END
 
diff --git a/xen/arch/arm/platforms/midway.c b/xen/arch/arm/platforms/midway.c
index 399056b..b221279 100644
--- a/xen/arch/arm/platforms/midway.c
+++ b/xen/arch/arm/platforms/midway.c
@@ -42,11 +42,6 @@ static void midway_reset(void)
     iounmap(pmu);
 }
 
-static uint32_t midway_quirks(void)
-{
-    return PLATFORM_QUIRK_DOM0_MAPPING_11;
-}
-
 static const char * const midway_dt_compat[] __initconst =
 {
     "calxeda,ecx-2000",
@@ -56,7 +51,6 @@ static const char * const midway_dt_compat[] __initconst =
 PLATFORM_START(midway, "CALXEDA MIDWAY")
     .compatible = midway_dt_compat,
     .reset = midway_reset,
-    .quirks = midway_quirks,
 PLATFORM_END
 
 /*
diff --git a/xen/arch/arm/platforms/omap5.c b/xen/arch/arm/platforms/omap5.c
index 54fa5ff..3a3b16b 100644
--- a/xen/arch/arm/platforms/omap5.c
+++ b/xen/arch/arm/platforms/omap5.c
@@ -153,11 +153,6 @@ static int __init omap5_cpu_up(int cpu)
     return 0;
 }
 
-static uint32_t omap5_quirks(void)
-{
-    return PLATFORM_QUIRK_DOM0_MAPPING_11;
-}
-
 static const char const *omap5_dt_compat[] __initconst =
 {
     "ti,omap5",
@@ -170,7 +165,6 @@ PLATFORM_START(omap5, "TI OMAP5")
     .specific_mapping = omap5_specific_mapping,
     .smp_init = omap5_smp_init,
     .cpu_up = omap5_cpu_up,
-    .quirks = omap5_quirks,
 PLATFORM_END
 
 /*
diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h
index c282b30..6d108e7 100644
--- a/xen/include/asm-arm/platform.h
+++ b/xen/include/asm-arm/platform.h
@@ -39,12 +39,6 @@ struct platform_desc {
     const struct dt_device_match *blacklist_dev;
 };
 
-/*
- * Quirk to map dom0 memory in 1:1
- * Useful on platform where System MMU is not yet implemented
- */
-#define PLATFORM_QUIRK_DOM0_MAPPING_11 (1 << 0)
-
 void __init platform_init(void);
 int __init platform_init_time(void);
 int __init platform_specific_mapping(struct domain *d);
-- 
1.8.4.rc3

Re: [PATCH] xen: arm: Enable 1:1 workaround by default

[Andrew Cooper]

On 27/11/13 12:19, Ian Campbell wrote:
> I was just about to send out patches adding the 1:1 workaround to vexpress (the
> foundation model is a vexpress platfrom with DMA) and sunxi.
>
> That would have meant that all platforms now implement the quirk. Instead lets
> just make it the default and remove the quirk.
>
> In the future this will likely be set based on the presence absence of an
> IOMMU, perhaps with additional overrides by the platform.
>
> A command line option no-dom0_11_mapping is provided.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

Can you patch docs/misc/xen-command-line.markdown as well please

~Andrew

> ---
>  xen/arch/arm/domain_build.c      | 5 ++++-
>  xen/arch/arm/platforms/exynos5.c | 6 ------
>  xen/arch/arm/platforms/midway.c  | 6 ------
>  xen/arch/arm/platforms/omap5.c   | 6 ------
>  xen/include/asm-arm/platform.h   | 6 ------
>  5 files changed, 4 insertions(+), 25 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index e9bb01f..ed22613 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -22,6 +22,9 @@
>  static unsigned int __initdata opt_dom0_max_vcpus;
>  integer_param("dom0_max_vcpus", opt_dom0_max_vcpus);
>  
> +static int opt_dom0_11_mapping = 1;
> +boolean_param("dom0_11_mapping", opt_dom0_11_mapping);
> +
>  #define DOM0_MEM_DEFAULT 0x8000000 /* 128 MiB */
>  static u64 __initdata dom0_mem = DOM0_MEM_DEFAULT;
>  
> @@ -110,7 +113,7 @@ static void allocate_memory(struct domain *d, struct kernel_info *kinfo)
>      u32 reg_len, reg_size;
>      unsigned int bank = 0;
>  
> -    if ( platform_has_quirk(PLATFORM_QUIRK_DOM0_MAPPING_11) )
> +    if ( opt_dom0_11_mapping )
>          return allocate_memory_11(d, kinfo);
>  
>      while ( (memory = dt_find_node_by_type(memory, "memory")) )
> diff --git a/xen/arch/arm/platforms/exynos5.c b/xen/arch/arm/platforms/exynos5.c
> index 0e76cac..fa6b465 100644
> --- a/xen/arch/arm/platforms/exynos5.c
> +++ b/xen/arch/arm/platforms/exynos5.c
> @@ -111,11 +111,6 @@ static void exynos5_reset(void)
>      iounmap(pmu);
>  }
>  
> -static uint32_t exynos5_quirks(void)
> -{
> -    return PLATFORM_QUIRK_DOM0_MAPPING_11;
> -}
> -
>  static const char * const exynos5_dt_compat[] __initconst =
>  {
>      "samsung,exynos5250",
> @@ -139,7 +134,6 @@ PLATFORM_START(exynos5, "SAMSUNG EXYNOS5")
>      .smp_init = exynos5_smp_init,
>      .cpu_up = exynos5_cpu_up,
>      .reset = exynos5_reset,
> -    .quirks = exynos5_quirks,
>      .blacklist_dev = exynos5_blacklist_dev,
>  PLATFORM_END
>  
> diff --git a/xen/arch/arm/platforms/midway.c b/xen/arch/arm/platforms/midway.c
> index 399056b..b221279 100644
> --- a/xen/arch/arm/platforms/midway.c
> +++ b/xen/arch/arm/platforms/midway.c
> @@ -42,11 +42,6 @@ static void midway_reset(void)
>      iounmap(pmu);
>  }
>  
> -static uint32_t midway_quirks(void)
> -{
> -    return PLATFORM_QUIRK_DOM0_MAPPING_11;
> -}
> -
>  static const char * const midway_dt_compat[] __initconst =
>  {
>      "calxeda,ecx-2000",
> @@ -56,7 +51,6 @@ static const char * const midway_dt_compat[] __initconst =
>  PLATFORM_START(midway, "CALXEDA MIDWAY")
>      .compatible = midway_dt_compat,
>      .reset = midway_reset,
> -    .quirks = midway_quirks,
>  PLATFORM_END
>  
>  /*
> diff --git a/xen/arch/arm/platforms/omap5.c b/xen/arch/arm/platforms/omap5.c
> index 54fa5ff..3a3b16b 100644
> --- a/xen/arch/arm/platforms/omap5.c
> +++ b/xen/arch/arm/platforms/omap5.c
> @@ -153,11 +153,6 @@ static int __init omap5_cpu_up(int cpu)
>      return 0;
>  }
>  
> -static uint32_t omap5_quirks(void)
> -{
> -    return PLATFORM_QUIRK_DOM0_MAPPING_11;
> -}
> -
>  static const char const *omap5_dt_compat[] __initconst =
>  {
>      "ti,omap5",
> @@ -170,7 +165,6 @@ PLATFORM_START(omap5, "TI OMAP5")
>      .specific_mapping = omap5_specific_mapping,
>      .smp_init = omap5_smp_init,
>      .cpu_up = omap5_cpu_up,
> -    .quirks = omap5_quirks,
>  PLATFORM_END
>  
>  /*
> diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h
> index c282b30..6d108e7 100644
> --- a/xen/include/asm-arm/platform.h
> +++ b/xen/include/asm-arm/platform.h
> @@ -39,12 +39,6 @@ struct platform_desc {
>      const struct dt_device_match *blacklist_dev;
>  };
>  
> -/*
> - * Quirk to map dom0 memory in 1:1
> - * Useful on platform where System MMU is not yet implemented
> - */
> -#define PLATFORM_QUIRK_DOM0_MAPPING_11 (1 << 0)
> -
>  void __init platform_init(void);
>  int __init platform_init_time(void);
>  int __init platform_specific_mapping(struct domain *d);

Re: [PATCH] xen: arm: Enable 1:1 workaround by default

[Ian Campbell]

On Wed, 2013-11-27 at 12:20 +0000, Andrew Cooper wrote:
> On 27/11/13 12:19, Ian Campbell wrote:
> > I was just about to send out patches adding the 1:1 workaround to vexpress (the
> > foundation model is a vexpress platfrom with DMA) and sunxi.
> >
> > That would have meant that all platforms now implement the quirk. Instead lets
> > just make it the default and remove the quirk.
> >
> > In the future this will likely be set based on the presence absence of an
> > IOMMU, perhaps with additional overrides by the platform.
> >
> > A command line option no-dom0_11_mapping is provided.
> >
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> 
> Can you patch docs/misc/xen-command-line.markdown as well please

I left it out on purpose, given the implications of using it I actually
want people to have to talk to us before using it.

I'd be more inclined to remove the option that to document it.

Ian.

Re: [PATCH] xen: arm: Enable 1:1 workaround by default

[Andrew Cooper]

On 27/11/13 12:23, Ian Campbell wrote:
> On Wed, 2013-11-27 at 12:20 +0000, Andrew Cooper wrote:
>> On 27/11/13 12:19, Ian Campbell wrote:
>>> I was just about to send out patches adding the 1:1 workaround to vexpress (the
>>> foundation model is a vexpress platfrom with DMA) and sunxi.
>>>
>>> That would have meant that all platforms now implement the quirk. Instead lets
>>> just make it the default and remove the quirk.
>>>
>>> In the future this will likely be set based on the presence absence of an
>>> IOMMU, perhaps with additional overrides by the platform.
>>>
>>> A command line option no-dom0_11_mapping is provided.
>>>
>>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>> Can you patch docs/misc/xen-command-line.markdown as well please
> I left it out on purpose, given the implications of using it I actually
> want people to have to talk to us before using it.
>
> I'd be more inclined to remove the option that to document it.
>
> Ian.
>

Hmm ok - perhaps a comment beside boolean_param() indicating that this
is a temporary hack and is expected to disappear completely in due course?

~Andrew

Re: [PATCH] xen: arm: Enable 1:1 workaround by default

[Stefano Stabellini]

On Wed, 27 Nov 2013, Ian Campbell wrote:
> On Wed, 2013-11-27 at 12:20 +0000, Andrew Cooper wrote:
> > On 27/11/13 12:19, Ian Campbell wrote:
> > > I was just about to send out patches adding the 1:1 workaround to vexpress (the
> > > foundation model is a vexpress platfrom with DMA) and sunxi.
> > >
> > > That would have meant that all platforms now implement the quirk. Instead lets
> > > just make it the default and remove the quirk.
> > >
> > > In the future this will likely be set based on the presence absence of an
> > > IOMMU, perhaps with additional overrides by the platform.
> > >
> > > A command line option no-dom0_11_mapping is provided.
> > >
> > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > 
> > Can you patch docs/misc/xen-command-line.markdown as well please
> 
> I left it out on purpose, given the implications of using it I actually
> want people to have to talk to us before using it.
> 
> I'd be more inclined to remove the option that to document it.

Given that there is no way of using Xen on any ARM hardware without that
option right now (unless the hardware doesn't do DMA at all, but we can
ignore that case), I think that making it a global variable, not exposed
to the user, would be fine. xen/x86 can set the option to false.

Re: [PATCH] xen: arm: Enable 1:1 workaround by default

[Julien Grall]

On 11/27/2013 12:19 PM, Ian Campbell wrote:
> I was just about to send out patches adding the 1:1 workaround to vexpress (the
> foundation model is a vexpress platfrom with DMA) and sunxi.
>
> That would have meant that all platforms now implement the quirk. Instead lets
> just make it the default and remove the quirk.
>
> In the future this will likely be set based on the presence absence of an
> IOMMU, perhaps with additional overrides by the platform.
>
> A command line option no-dom0_11_mapping is provided.
>

I plan to use the variable in other place, for instance in 
common/memory.c to solve issue between ballooning and 1:1 workaround.

It would be great if we have a function (platform_has_iommu) that we can 
use everywhere. What do you think?


> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>   xen/arch/arm/domain_build.c      | 5 ++++-
>   xen/arch/arm/platforms/exynos5.c | 6 ------
>   xen/arch/arm/platforms/midway.c  | 6 ------
>   xen/arch/arm/platforms/omap5.c   | 6 ------
>   xen/include/asm-arm/platform.h   | 6 ------
>   5 files changed, 4 insertions(+), 25 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index e9bb01f..ed22613 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -22,6 +22,9 @@
>   static unsigned int __initdata opt_dom0_max_vcpus;
>   integer_param("dom0_max_vcpus", opt_dom0_max_vcpus);
>
> +static int opt_dom0_11_mapping = 1;
> +boolean_param("dom0_11_mapping", opt_dom0_11_mapping);
> +
>   #define DOM0_MEM_DEFAULT 0x8000000 /* 128 MiB */
>   static u64 __initdata dom0_mem = DOM0_MEM_DEFAULT;
>
> @@ -110,7 +113,7 @@ static void allocate_memory(struct domain *d, struct kernel_info *kinfo)
>       u32 reg_len, reg_size;
>       unsigned int bank = 0;
>
> -    if ( platform_has_quirk(PLATFORM_QUIRK_DOM0_MAPPING_11) )
> +    if ( opt_dom0_11_mapping )
>           return allocate_memory_11(d, kinfo);
>
>       while ( (memory = dt_find_node_by_type(memory, "memory")) )
> diff --git a/xen/arch/arm/platforms/exynos5.c b/xen/arch/arm/platforms/exynos5.c
> index 0e76cac..fa6b465 100644
> --- a/xen/arch/arm/platforms/exynos5.c
> +++ b/xen/arch/arm/platforms/exynos5.c
> @@ -111,11 +111,6 @@ static void exynos5_reset(void)
>       iounmap(pmu);
>   }
>
> -static uint32_t exynos5_quirks(void)
> -{
> -    return PLATFORM_QUIRK_DOM0_MAPPING_11;
> -}
> -
>   static const char * const exynos5_dt_compat[] __initconst =
>   {
>       "samsung,exynos5250",
> @@ -139,7 +134,6 @@ PLATFORM_START(exynos5, "SAMSUNG EXYNOS5")
>       .smp_init = exynos5_smp_init,
>       .cpu_up = exynos5_cpu_up,
>       .reset = exynos5_reset,
> -    .quirks = exynos5_quirks,
>       .blacklist_dev = exynos5_blacklist_dev,
>   PLATFORM_END
>
> diff --git a/xen/arch/arm/platforms/midway.c b/xen/arch/arm/platforms/midway.c
> index 399056b..b221279 100644
> --- a/xen/arch/arm/platforms/midway.c
> +++ b/xen/arch/arm/platforms/midway.c
> @@ -42,11 +42,6 @@ static void midway_reset(void)
>       iounmap(pmu);
>   }
>
> -static uint32_t midway_quirks(void)
> -{
> -    return PLATFORM_QUIRK_DOM0_MAPPING_11;
> -}
> -
>   static const char * const midway_dt_compat[] __initconst =
>   {
>       "calxeda,ecx-2000",
> @@ -56,7 +51,6 @@ static const char * const midway_dt_compat[] __initconst =
>   PLATFORM_START(midway, "CALXEDA MIDWAY")
>       .compatible = midway_dt_compat,
>       .reset = midway_reset,
> -    .quirks = midway_quirks,
>   PLATFORM_END
>
>   /*
> diff --git a/xen/arch/arm/platforms/omap5.c b/xen/arch/arm/platforms/omap5.c
> index 54fa5ff..3a3b16b 100644
> --- a/xen/arch/arm/platforms/omap5.c
> +++ b/xen/arch/arm/platforms/omap5.c
> @@ -153,11 +153,6 @@ static int __init omap5_cpu_up(int cpu)
>       return 0;
>   }
>
> -static uint32_t omap5_quirks(void)
> -{
> -    return PLATFORM_QUIRK_DOM0_MAPPING_11;
> -}
> -
>   static const char const *omap5_dt_compat[] __initconst =
>   {
>       "ti,omap5",
> @@ -170,7 +165,6 @@ PLATFORM_START(omap5, "TI OMAP5")
>       .specific_mapping = omap5_specific_mapping,
>       .smp_init = omap5_smp_init,
>       .cpu_up = omap5_cpu_up,
> -    .quirks = omap5_quirks,
>   PLATFORM_END
>
>   /*
> diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h
> index c282b30..6d108e7 100644
> --- a/xen/include/asm-arm/platform.h
> +++ b/xen/include/asm-arm/platform.h
> @@ -39,12 +39,6 @@ struct platform_desc {
>       const struct dt_device_match *blacklist_dev;
>   };
>
> -/*
> - * Quirk to map dom0 memory in 1:1
> - * Useful on platform where System MMU is not yet implemented
> - */
> -#define PLATFORM_QUIRK_DOM0_MAPPING_11 (1 << 0)
> -
>   void __init platform_init(void);
>   int __init platform_init_time(void);
>   int __init platform_specific_mapping(struct domain *d);
>

-- 
Julien Grall

Re: [PATCH] xen: arm: Enable 1:1 workaround by default

[Ian Campbell]

On Wed, 2013-11-27 at 13:23 +0000, Julien Grall wrote:
> 
> On 11/27/2013 12:19 PM, Ian Campbell wrote:
> > I was just about to send out patches adding the 1:1 workaround to vexpress (the
> > foundation model is a vexpress platfrom with DMA) and sunxi.
> >
> > That would have meant that all platforms now implement the quirk. Instead lets
> > just make it the default and remove the quirk.
> >
> > In the future this will likely be set based on the presence absence of an
> > IOMMU, perhaps with additional overrides by the platform.
> >
> > A command line option no-dom0_11_mapping is provided.
> >
> 
> I plan to use the variable in other place, for instance in 
> common/memory.c to solve issue between ballooning and 1:1 workaround.
> 
> It would be great if we have a function (platform_has_iommu) that we can 
> use everywhere. What do you think?

I think in reality once iommu's arrive it will be a bit more subtle than
their presence or absence, e.g. something (perhaps the platform code)
will need to decide if all the write devices are behind an iommu such
that the 11 can be disabled.

I can unstatic opt_dom0_11_mapping and stick it in a header though. Or
provide an accessor method.

Ian.

Re: [PATCH] xen: arm: Enable 1:1 workaround by default

[Julien Grall]

On 11/27/2013 01:26 PM, Ian Campbell wrote:
> On Wed, 2013-11-27 at 13:23 +0000, Julien Grall wrote:
>>
>> On 11/27/2013 12:19 PM, Ian Campbell wrote:
>>> I was just about to send out patches adding the 1:1 workaround to vexpress (the
>>> foundation model is a vexpress platfrom with DMA) and sunxi.
>>>
>>> That would have meant that all platforms now implement the quirk. Instead lets
>>> just make it the default and remove the quirk.
>>>
>>> In the future this will likely be set based on the presence absence of an
>>> IOMMU, perhaps with additional overrides by the platform.
>>>
>>> A command line option no-dom0_11_mapping is provided.
>>>
>>
>> I plan to use the variable in other place, for instance in
>> common/memory.c to solve issue between ballooning and 1:1 workaround.
>>
>> It would be great if we have a function (platform_has_iommu) that we can
>> use everywhere. What do you think?
>
> I think in reality once iommu's arrive it will be a bit more subtle than
> their presence or absence, e.g. something (perhaps the platform code)
> will need to decide if all the write devices are behind an iommu such
> that the 11 can be disabled.
>
> I can unstatic opt_dom0_11_mapping and stick it in a header though. Or
> provide an accessor method.

An accessor method sounds better.

-- 
Julien Grall