thr3ads.net - Xen devel - [PATCH] xen: arm: remove hardcoded gnttab location from dom0 [Dec 2013]

If this information is useful, please help other people find it:
Share via:

Ian Campbell

2013-Dec-03 15:04 UTC

[PATCH] xen: arm: remove hardcoded gnttab location from dom0

The DT provided to guests (including dom0) includes a Xen node which, among
other things, describes an MMIO region which can be safely used for grant
table mappings (i.e. it is a hole in the physical address space). For domU we
provide a hardcoded values based on our hardcoded guest virtual machine
layout. However for dom0 we need to fit in with the underlying platform.
Leaving this hardcoded was an oversight which on some platforms could result
in the grant table overlaying RAM or MMIO regions which are in use by domain
0.

For the 4.4 release do as we did with the dom0 evtchn PPI and provide a hook
for the platform code to supply a suitable hardcoded address for the platform
(derived from reading the data sheet). Platforms which do not provide the hook
get the existing address as a default.

After 4.4 we should switch to selecting a region of host RAM which is not RAM
in the guest address map. This should be more flexible and safer but the patch
was looking too complex for 4.4.

Platform        Gnttab Address
========        =============exynos5.c       0xb0000000, confirmed with Julien.
sunxi.c         0x01d00000, confirmed in data sheet.
midway.c        0xff800000, confirmed with Andre, boot tested.
vexpress.c      0xb0000000, existing hardcoded value was selected for vexpress.
omap5.c         0xb0000000, no datasheet, looks safe in DTB.
xgene-storm.c   0x1f800000, reserved according to datasheet, boot tested.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Julien.Grall@linaro.org
Cc: Stefano.Stabellini@eu.citrix.com
Cc: Andre Przywara <andre.przywara@linaro.org>
Cc: Chen Baozi <baozich@gmail.com>
Cc: Pranavkumar Sawargaonkar <psawargaonkar@apm.com>
Cc: Anup Patel <apatel@apm.com>
---
 xen/arch/arm/domain_build.c          |    7 +++++--
 xen/arch/arm/platform.c              |   14 ++++++++++++++
 xen/arch/arm/platforms/midway.c      |    3 +++
 xen/arch/arm/platforms/sunxi.c       |    3 +++
 xen/arch/arm/platforms/xgene-storm.c |    3 +++
 xen/include/asm-arm/platform.h       |    7 +++++++
 6 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index be39686..332e937 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -329,6 +329,7 @@ static int make_hypervisor_node(struct domain *d,
     int res;
     int addrcells = dt_n_addr_cells(parent);
     int sizecells = dt_n_size_cells(parent);
+    paddr_t gnttab_start, gnttab_size;
 
     DPRINT("Create hypervisor node\n");
 
@@ -350,10 +351,12 @@ static int make_hypervisor_node(struct domain *d,
     if ( res )
         return res;
 
-    DPRINT("  Grant table range: 0xb0000000-0x20000\n");
+    platform_dom0_gnttab(&gnttab_start, &gnttab_size);
+    DPRINT("  Grant table range:
%#"PRIpaddr"-%#"PRIpaddr"\n",
+           gnttab_start, gnttab_start + gnttab_size);
     /* reg 0 is grant table space */
     cells = &reg[0];
-    dt_set_range(&cells, parent, 0xb0000000, 0x20000);
+    dt_set_range(&cells, parent, gnttab_start, gnttab_size);
     res = fdt_property(fdt, "reg", reg,
                        dt_cells_to_size(addrcells + sizecells));
     if ( res )
diff --git a/xen/arch/arm/platform.c b/xen/arch/arm/platform.c
index a7f9ee4..87ceff1 100644
--- a/xen/arch/arm/platform.c
+++ b/xen/arch/arm/platform.c
@@ -163,6 +163,20 @@ unsigned int platform_dom0_evtchn_ppi(void)
     return GUEST_EVTCHN_PPI;
 }
 
+void platform_dom0_gnttab(paddr_t *start, paddr_t *size)
+{
+    if ( platform && platform->dom0_gnttab_size )
+    {
+        *start = platform->dom0_gnttab_start;
+        *size = platform->dom0_gnttab_size;
+    }
+    else
+    {
+        *start = 0xb0000000;
+        *size = 0x20000;
+    }
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/arm/platforms/midway.c b/xen/arch/arm/platforms/midway.c
index 399056b..27f46f8 100644
--- a/xen/arch/arm/platforms/midway.c
+++ b/xen/arch/arm/platforms/midway.c
@@ -57,6 +57,9 @@ PLATFORM_START(midway, "CALXEDA MIDWAY")
     .compatible = midway_dt_compat,
     .reset = midway_reset,
     .quirks = midway_quirks,
+
+    .dom0_gnttab_start = 0xff800000,
+    .dom0_gnttab_size = 0x20000,
 PLATFORM_END
 
 /*
diff --git a/xen/arch/arm/platforms/sunxi.c b/xen/arch/arm/platforms/sunxi.c
index b466518..fb12801 100644
--- a/xen/arch/arm/platforms/sunxi.c
+++ b/xen/arch/arm/platforms/sunxi.c
@@ -37,6 +37,9 @@ static const struct dt_device_match sunxi_blacklist_dev[]
__initconst  PLATFORM_START(sunxi, "Allwinner A20")
     .compatible = sunxi_dt_compat,
     .blacklist_dev = sunxi_blacklist_dev,
+
+    .dom0_gnttab_start = 0x01d00000,
+    .dom0_gnttab_size = 0x20000,
 PLATFORM_END
 
 /*
diff --git a/xen/arch/arm/platforms/xgene-storm.c
b/xen/arch/arm/platforms/xgene-storm.c
index ea9b0f4..c0c6857 100644
--- a/xen/arch/arm/platforms/xgene-storm.c
+++ b/xen/arch/arm/platforms/xgene-storm.c
@@ -118,7 +118,10 @@ PLATFORM_START(xgene_storm, "APM X-GENE STORM")
     .compatible = xgene_storm_dt_compat,
     .quirks = xgene_storm_quirks,
     .specific_mapping = xgene_storm_specific_mapping,
+
     .dom0_evtchn_ppi = 24,
+    .dom0_gnttab_start = 0x1f800000,
+    .dom0_gnttab_size = 0x20000,
 PLATFORM_END
 
 /*
diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h
index 5900ee4..bda4e41 100644
--- a/xen/include/asm-arm/platform.h
+++ b/xen/include/asm-arm/platform.h
@@ -41,6 +41,12 @@ struct platform_desc {
      * The IRQ (PPI) to use to inject event channels to dom0.
      */
     unsigned int dom0_evtchn_ppi;
+    /*
+     * The location of a region of physical address space which dom0
+     * can use for grant table mappings. If size is zero defaults to
+     * 0xb0000000-0xb0020000.
+     */
+    paddr_t dom0_gnttab_start, dom0_gnttab_size;
 };
 
 /*
@@ -66,6 +72,7 @@ void platform_poweroff(void);
 bool_t platform_has_quirk(uint32_t quirk);
 bool_t platform_device_is_blacklisted(const struct dt_device_node *node);
 unsigned int platform_dom0_evtchn_ppi(void);
+void platform_dom0_gnttab(paddr_t *start, paddr_t *size);
 
 #define PLATFORM_START(_name, _namestr)                         \
 static const struct platform_desc  __plat_desc_##_name __used   \
-- 
1.7.10.4

Julien Grall

2013-Dec-03 15:20 UTC

head link

Re: [PATCH] xen: arm: remove hardcoded gnttab location from dom0

On 12/03/2013 03:04 PM, Ian Campbell wrote:> The DT provided to guests (including dom0) includes a Xen node which, among
> other things, describes an MMIO region which can be safely used for grant
> table mappings (i.e. it is a hole in the physical address space). For domU
we
> provide a hardcoded values based on our hardcoded guest virtual machine
> layout. However for dom0 we need to fit in with the underlying platform.
> Leaving this hardcoded was an oversight which on some platforms could
result
> in the grant table overlaying RAM or MMIO regions which are in use by
domain
> 0.
> 
> For the 4.4 release do as we did with the dom0 evtchn PPI and provide a
hook
> for the platform code to supply a suitable hardcoded address for the
platform
> (derived from reading the data sheet). Platforms which do not provide the
hook
> get the existing address as a default.
> 
> After 4.4 we should switch to selecting a region of host RAM which is not
RAM
> in the guest address map. This should be more flexible and safer but the
patch
> was looking too complex for 4.4.
> 
> Platform        Gnttab Address
> ========        =============> exynos5.c       0xb0000000, confirmed
with Julien.
> sunxi.c         0x01d00000, confirmed in data sheet.
> midway.c        0xff800000, confirmed with Andre, boot tested.
> vexpress.c      0xb0000000, existing hardcoded value was selected for
vexpress.
> omap5.c         0xb0000000, no datasheet, looks safe in DTB.
> xgene-storm.c   0x1f800000, reserved according to datasheet, boot tested.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Julien.Grall@linaro.org
> Cc: Stefano.Stabellini@eu.citrix.com
> Cc: Andre Przywara <andre.przywara@linaro.org>
> Cc: Chen Baozi <baozich@gmail.com>
> Cc: Pranavkumar Sawargaonkar <psawargaonkar@apm.com>
> Cc: Anup Patel <apatel@apm.com>
> ---
>  xen/arch/arm/domain_build.c          |    7 +++++--
>  xen/arch/arm/platform.c              |   14 ++++++++++++++
>  xen/arch/arm/platforms/midway.c      |    3 +++
>  xen/arch/arm/platforms/sunxi.c       |    3 +++
>  xen/arch/arm/platforms/xgene-storm.c |    3 +++
>  xen/include/asm-arm/platform.h       |    7 +++++++
>  6 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index be39686..332e937 100644
>  
>  /*
> diff --git a/xen/include/asm-arm/platform.h
b/xen/include/asm-arm/platform.h
> index 5900ee4..bda4e41 100644
> --- a/xen/include/asm-arm/platform.h
> +++ b/xen/include/asm-arm/platform.h
> @@ -41,6 +41,12 @@ struct platform_desc {
>       * The IRQ (PPI) to use to inject event channels to dom0.
>       */
>      unsigned int dom0_evtchn_ppi;
> +    /*
> +     * The location of a region of physical address space which dom0
> +     * can use for grant table mappings. If size is zero defaults to
> +     * 0xb0000000-0xb0020000.
> +     */
> +    paddr_t dom0_gnttab_start, dom0_gnttab_size;
The size looks the same everywhere, can we just hardcode the base
address per platform?

-- 
Julien Grall

Ian Campbell

2013-Dec-03 15:22 UTC

head link

Re: [PATCH] xen: arm: remove hardcoded gnttab location from dom0

On Tue, 2013-12-03 at 15:20 +0000, Julien Grall wrote:
> The size looks the same everywhere, can we just hardcode the base
> address per platform?
It''s not implausible that a platform might want a different size in the
future. e.g. if they can only spare 64K of space. I don''t think there
is
any reason to restrict things, start and size are naturally paired
together.

Ian.

Andre Przywara

2013-Dec-03 15:32 UTC

head link

Re: [PATCH] xen: arm: remove hardcoded gnttab location from dom0

On 12/03/2013 04:04 PM, Ian Campbell wrote:> The DT provided to guests (including dom0) includes a Xen node which, among
> other things, describes an MMIO region which can be safely used for grant
> table mappings (i.e. it is a hole in the physical address space). For domU
we
> provide a hardcoded values based on our hardcoded guest virtual machine
> layout. However for dom0 we need to fit in with the underlying platform.
> Leaving this hardcoded was an oversight which on some platforms could
result
> in the grant table overlaying RAM or MMIO regions which are in use by
domain
> 0.
>
> For the 4.4 release do as we did with the dom0 evtchn PPI and provide a
hook
> for the platform code to supply a suitable hardcoded address for the
platform
> (derived from reading the data sheet). Platforms which do not provide the
hook
> get the existing address as a default.
>
> After 4.4 we should switch to selecting a region of host RAM which is not
RAM
> in the guest address map. This should be more flexible and safer but the
patch
> was looking too complex for 4.4.
>
> Platform        Gnttab Address
> ========        =============> exynos5.c       0xb0000000, confirmed
with Julien.
> sunxi.c         0x01d00000, confirmed in data sheet.
> midway.c        0xff800000, confirmed with Andre, boot tested.
For this Midway specific address choice:
Acked-by: Andre Przywara <andre.przywara@calxeda.com>

But don''t use more than the 128K, there seems to be debug MMIO mapping 
behind this.

Haven''t had a chance to actually test that, though.

Regards,
Andre.
> vexpress.c      0xb0000000, existing hardcoded value was selected for
vexpress.
> omap5.c         0xb0000000, no datasheet, looks safe in DTB.
> xgene-storm.c   0x1f800000, reserved according to datasheet, boot tested.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Julien.Grall@linaro.org
> Cc: Stefano.Stabellini@eu.citrix.com
> Cc: Andre Przywara <andre.przywara@linaro.org>
> Cc: Chen Baozi <baozich@gmail.com>
> Cc: Pranavkumar Sawargaonkar <psawargaonkar@apm.com>
> Cc: Anup Patel <apatel@apm.com>
> ---
>   xen/arch/arm/domain_build.c          |    7 +++++--
>   xen/arch/arm/platform.c              |   14 ++++++++++++++
>   xen/arch/arm/platforms/midway.c      |    3 +++
>   xen/arch/arm/platforms/sunxi.c       |    3 +++
>   xen/arch/arm/platforms/xgene-storm.c |    3 +++
>   xen/include/asm-arm/platform.h       |    7 +++++++
>   6 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index be39686..332e937 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -329,6 +329,7 @@ static int make_hypervisor_node(struct domain *d,
>       int res;
>       int addrcells = dt_n_addr_cells(parent);
>       int sizecells = dt_n_size_cells(parent);
> +    paddr_t gnttab_start, gnttab_size;
>
>       DPRINT("Create hypervisor node\n");
>
> @@ -350,10 +351,12 @@ static int make_hypervisor_node(struct domain *d,
>       if ( res )
>           return res;
>
> -    DPRINT("  Grant table range: 0xb0000000-0x20000\n");
> +    platform_dom0_gnttab(&gnttab_start, &gnttab_size);
> +    DPRINT("  Grant table range:
%#"PRIpaddr"-%#"PRIpaddr"\n",
> +           gnttab_start, gnttab_start + gnttab_size);
>       /* reg 0 is grant table space */
>       cells = &reg[0];
> -    dt_set_range(&cells, parent, 0xb0000000, 0x20000);
> +    dt_set_range(&cells, parent, gnttab_start, gnttab_size);
>       res = fdt_property(fdt, "reg", reg,
>                          dt_cells_to_size(addrcells + sizecells));
>       if ( res )
> diff --git a/xen/arch/arm/platform.c b/xen/arch/arm/platform.c
> index a7f9ee4..87ceff1 100644
> --- a/xen/arch/arm/platform.c
> +++ b/xen/arch/arm/platform.c
> @@ -163,6 +163,20 @@ unsigned int platform_dom0_evtchn_ppi(void)
>       return GUEST_EVTCHN_PPI;
>   }
>
> +void platform_dom0_gnttab(paddr_t *start, paddr_t *size)
> +{
> +    if ( platform && platform->dom0_gnttab_size )
> +    {
> +        *start = platform->dom0_gnttab_start;
> +        *size = platform->dom0_gnttab_size;
> +    }
> +    else
> +    {
> +        *start = 0xb0000000;
> +        *size = 0x20000;
> +    }
> +}
> +
>   /*
>    * Local variables:
>    * mode: C
> diff --git a/xen/arch/arm/platforms/midway.c
b/xen/arch/arm/platforms/midway.c
> index 399056b..27f46f8 100644
> --- a/xen/arch/arm/platforms/midway.c
> +++ b/xen/arch/arm/platforms/midway.c
> @@ -57,6 +57,9 @@ PLATFORM_START(midway, "CALXEDA MIDWAY")
>       .compatible = midway_dt_compat,
>       .reset = midway_reset,
>       .quirks = midway_quirks,
> +
> +    .dom0_gnttab_start = 0xff800000,
> +    .dom0_gnttab_size = 0x20000,
>   PLATFORM_END
>
>   /*
> diff --git a/xen/arch/arm/platforms/sunxi.c
b/xen/arch/arm/platforms/sunxi.c
> index b466518..fb12801 100644
> --- a/xen/arch/arm/platforms/sunxi.c
> +++ b/xen/arch/arm/platforms/sunxi.c
> @@ -37,6 +37,9 @@ static const struct dt_device_match sunxi_blacklist_dev[]
__initconst >   PLATFORM_START(sunxi, "Allwinner A20")
>       .compatible = sunxi_dt_compat,
>       .blacklist_dev = sunxi_blacklist_dev,
> +
> +    .dom0_gnttab_start = 0x01d00000,
> +    .dom0_gnttab_size = 0x20000,
>   PLATFORM_END
>
>   /*
> diff --git a/xen/arch/arm/platforms/xgene-storm.c
b/xen/arch/arm/platforms/xgene-storm.c
> index ea9b0f4..c0c6857 100644
> --- a/xen/arch/arm/platforms/xgene-storm.c
> +++ b/xen/arch/arm/platforms/xgene-storm.c
> @@ -118,7 +118,10 @@ PLATFORM_START(xgene_storm, "APM X-GENE
STORM")
>       .compatible = xgene_storm_dt_compat,
>       .quirks = xgene_storm_quirks,
>       .specific_mapping = xgene_storm_specific_mapping,
> +
>       .dom0_evtchn_ppi = 24,
> +    .dom0_gnttab_start = 0x1f800000,
> +    .dom0_gnttab_size = 0x20000,
>   PLATFORM_END
>
>   /*
> diff --git a/xen/include/asm-arm/platform.h
b/xen/include/asm-arm/platform.h
> index 5900ee4..bda4e41 100644
> --- a/xen/include/asm-arm/platform.h
> +++ b/xen/include/asm-arm/platform.h
> @@ -41,6 +41,12 @@ struct platform_desc {
>        * The IRQ (PPI) to use to inject event channels to dom0.
>        */
>       unsigned int dom0_evtchn_ppi;
> +    /*
> +     * The location of a region of physical address space which dom0
> +     * can use for grant table mappings. If size is zero defaults to
> +     * 0xb0000000-0xb0020000.
> +     */
> +    paddr_t dom0_gnttab_start, dom0_gnttab_size;
>   };
>
>   /*
> @@ -66,6 +72,7 @@ void platform_poweroff(void);
>   bool_t platform_has_quirk(uint32_t quirk);
>   bool_t platform_device_is_blacklisted(const struct dt_device_node *node);
>   unsigned int platform_dom0_evtchn_ppi(void);
> +void platform_dom0_gnttab(paddr_t *start, paddr_t *size);
>
>   #define PLATFORM_START(_name, _namestr)                         \
>   static const struct platform_desc  __plat_desc_##_name __used   \
>

Julien Grall

2013-Dec-03 15:52 UTC

head link

Re: [PATCH] xen: arm: remove hardcoded gnttab location from dom0

On 12/03/2013 03:22 PM, Ian Campbell wrote:> On Tue, 2013-12-03 at 15:20 +0000, Julien Grall wrote:
> 
>> The size looks the same everywhere, can we just hardcode the base
>> address per platform?
> 
> It''s not implausible that a platform might want a different size
in the
> future. e.g. if they can only spare 64K of space. I don''t think
there is
> any reason to restrict things, start and size are naturally paired
> together.
Ok. I gave a try on the Arndale and it works.

So:
Tested-by: Julien Grall <julien.grall@linaro.org>

And for the patch:
Acked-by: Julien Grall <julien.grall@linaro.org>

-- 
Julien Grall

Chen Baozi

2013-Dec-03 18:18 UTC

head link

Re: [PATCH] xen: arm: remove hardcoded gnttab location from dom0

On Dec 3, 2013, at 23:04, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> The DT provided to guests (including dom0) includes a Xen node which, among
> other things, describes an MMIO region which can be safely used for grant
> table mappings (i.e. it is a hole in the physical address space). For domU
we
> provide a hardcoded values based on our hardcoded guest virtual machine
> layout. However for dom0 we need to fit in with the underlying platform.
> Leaving this hardcoded was an oversight which on some platforms could
result
> in the grant table overlaying RAM or MMIO regions which are in use by
domain
> 0.
> 
> For the 4.4 release do as we did with the dom0 evtchn PPI and provide a
hook
> for the platform code to supply a suitable hardcoded address for the
platform
> (derived from reading the data sheet). Platforms which do not provide the
hook
> get the existing address as a default.
> 
> After 4.4 we should switch to selecting a region of host RAM which is not
RAM
> in the guest address map. This should be more flexible and safer but the
patch
> was looking too complex for 4.4.
> 
> Platform        Gnttab Address
> ========        =============> exynos5.c       0xb0000000, confirmed
with Julien.
> sunxi.c         0x01d00000, confirmed in data sheet.
> midway.c        0xff800000, confirmed with Andre, boot tested.
> vexpress.c      0xb0000000, existing hardcoded value was selected for
vexpress.
> omap5.c         0xb0000000, no datasheet, looks safe in DTB.According to OMAP5432 data sheet, 0x80000000~0xbfffffff is the DRAM 
address space. So this is Ok.

Acked-by: Chen Baozi <baozich@gmail.com>

Cheers,

Baozi

Pranavkumar Sawargaonkar

2013-Dec-04 07:27 UTC

head link

Re: [PATCH] xen: arm: remove hardcoded gnttab location from dom0

On Tue, Dec 3, 2013 at 8:34 PM, Ian Campbell <ian.campbell@citrix.com>
wrote:> The DT provided to guests (including dom0) includes a Xen node which, among
> other things, describes an MMIO region which can be safely used for grant
> table mappings (i.e. it is a hole in the physical address space). For domU
we
> provide a hardcoded values based on our hardcoded guest virtual machine
> layout. However for dom0 we need to fit in with the underlying platform.
> Leaving this hardcoded was an oversight which on some platforms could
result
> in the grant table overlaying RAM or MMIO regions which are in use by
domain
> 0.
>
> For the 4.4 release do as we did with the dom0 evtchn PPI and provide a
hook
> for the platform code to supply a suitable hardcoded address for the
platform
> (derived from reading the data sheet). Platforms which do not provide the
hook
> get the existing address as a default.
>
> After 4.4 we should switch to selecting a region of host RAM which is not
RAM
> in the guest address map. This should be more flexible and safer but the
patch
> was looking too complex for 4.4.
>
> Platform        Gnttab Address
> ========        =============> exynos5.c       0xb0000000, confirmed
with Julien.
> sunxi.c         0x01d00000, confirmed in data sheet.
> midway.c        0xff800000, confirmed with Andre, boot tested.
> vexpress.c      0xb0000000, existing hardcoded value was selected for
vexpress.
> omap5.c         0xb0000000, no datasheet, looks safe in DTB.
> xgene-storm.c   0x1f800000, reserved according to datasheet, boot tested.
Acked-by: Pranavkumar Sawargaonkar <psawargaonkar@apm.com>
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Julien.Grall@linaro.org
> Cc: Stefano.Stabellini@eu.citrix.com
> Cc: Andre Przywara <andre.przywara@linaro.org>
> Cc: Chen Baozi <baozich@gmail.com>
> Cc: Pranavkumar Sawargaonkar <psawargaonkar@apm.com>
> Cc: Anup Patel <apatel@apm.com>
> ---
>  xen/arch/arm/domain_build.c          |    7 +++++--
>  xen/arch/arm/platform.c              |   14 ++++++++++++++
>  xen/arch/arm/platforms/midway.c      |    3 +++
>  xen/arch/arm/platforms/sunxi.c       |    3 +++
>  xen/arch/arm/platforms/xgene-storm.c |    3 +++
>  xen/include/asm-arm/platform.h       |    7 +++++++
>  6 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index be39686..332e937 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -329,6 +329,7 @@ static int make_hypervisor_node(struct domain *d,
>      int res;
>      int addrcells = dt_n_addr_cells(parent);
>      int sizecells = dt_n_size_cells(parent);
> +    paddr_t gnttab_start, gnttab_size;
>
>      DPRINT("Create hypervisor node\n");
>
> @@ -350,10 +351,12 @@ static int make_hypervisor_node(struct domain *d,
>      if ( res )
>          return res;
>
> -    DPRINT("  Grant table range: 0xb0000000-0x20000\n");
> +    platform_dom0_gnttab(&gnttab_start, &gnttab_size);
> +    DPRINT("  Grant table range:
%#"PRIpaddr"-%#"PRIpaddr"\n",
> +           gnttab_start, gnttab_start + gnttab_size);
>      /* reg 0 is grant table space */
>      cells = &reg[0];
> -    dt_set_range(&cells, parent, 0xb0000000, 0x20000);
> +    dt_set_range(&cells, parent, gnttab_start, gnttab_size);
>      res = fdt_property(fdt, "reg", reg,
>                         dt_cells_to_size(addrcells + sizecells));
>      if ( res )
> diff --git a/xen/arch/arm/platform.c b/xen/arch/arm/platform.c
> index a7f9ee4..87ceff1 100644
> --- a/xen/arch/arm/platform.c
> +++ b/xen/arch/arm/platform.c
> @@ -163,6 +163,20 @@ unsigned int platform_dom0_evtchn_ppi(void)
>      return GUEST_EVTCHN_PPI;
>  }
>
> +void platform_dom0_gnttab(paddr_t *start, paddr_t *size)
> +{
> +    if ( platform && platform->dom0_gnttab_size )
> +    {
> +        *start = platform->dom0_gnttab_start;
> +        *size = platform->dom0_gnttab_size;
> +    }
> +    else
> +    {
> +        *start = 0xb0000000;
> +        *size = 0x20000;
> +    }
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/arch/arm/platforms/midway.c
b/xen/arch/arm/platforms/midway.c
> index 399056b..27f46f8 100644
> --- a/xen/arch/arm/platforms/midway.c
> +++ b/xen/arch/arm/platforms/midway.c
> @@ -57,6 +57,9 @@ PLATFORM_START(midway, "CALXEDA MIDWAY")
>      .compatible = midway_dt_compat,
>      .reset = midway_reset,
>      .quirks = midway_quirks,
> +
> +    .dom0_gnttab_start = 0xff800000,
> +    .dom0_gnttab_size = 0x20000,
>  PLATFORM_END
>
>  /*
> diff --git a/xen/arch/arm/platforms/sunxi.c
b/xen/arch/arm/platforms/sunxi.c
> index b466518..fb12801 100644
> --- a/xen/arch/arm/platforms/sunxi.c
> +++ b/xen/arch/arm/platforms/sunxi.c
> @@ -37,6 +37,9 @@ static const struct dt_device_match sunxi_blacklist_dev[]
__initconst >  PLATFORM_START(sunxi, "Allwinner A20")
>      .compatible = sunxi_dt_compat,
>      .blacklist_dev = sunxi_blacklist_dev,
> +
> +    .dom0_gnttab_start = 0x01d00000,
> +    .dom0_gnttab_size = 0x20000,
>  PLATFORM_END
>
>  /*
> diff --git a/xen/arch/arm/platforms/xgene-storm.c
b/xen/arch/arm/platforms/xgene-storm.c
> index ea9b0f4..c0c6857 100644
> --- a/xen/arch/arm/platforms/xgene-storm.c
> +++ b/xen/arch/arm/platforms/xgene-storm.c
> @@ -118,7 +118,10 @@ PLATFORM_START(xgene_storm, "APM X-GENE
STORM")
>      .compatible = xgene_storm_dt_compat,
>      .quirks = xgene_storm_quirks,
>      .specific_mapping = xgene_storm_specific_mapping,
> +
>      .dom0_evtchn_ppi = 24,
> +    .dom0_gnttab_start = 0x1f800000,
> +    .dom0_gnttab_size = 0x20000,
>  PLATFORM_END
>
>  /*
> diff --git a/xen/include/asm-arm/platform.h
b/xen/include/asm-arm/platform.h
> index 5900ee4..bda4e41 100644
> --- a/xen/include/asm-arm/platform.h
> +++ b/xen/include/asm-arm/platform.h
> @@ -41,6 +41,12 @@ struct platform_desc {
>       * The IRQ (PPI) to use to inject event channels to dom0.
>       */
>      unsigned int dom0_evtchn_ppi;
> +    /*
> +     * The location of a region of physical address space which dom0
> +     * can use for grant table mappings. If size is zero defaults to
> +     * 0xb0000000-0xb0020000.
> +     */
> +    paddr_t dom0_gnttab_start, dom0_gnttab_size;
>  };
>
>  /*
> @@ -66,6 +72,7 @@ void platform_poweroff(void);
>  bool_t platform_has_quirk(uint32_t quirk);
>  bool_t platform_device_is_blacklisted(const struct dt_device_node *node);
>  unsigned int platform_dom0_evtchn_ppi(void);
> +void platform_dom0_gnttab(paddr_t *start, paddr_t *size);
>
>  #define PLATFORM_START(_name, _namestr)                         \
>  static const struct platform_desc  __plat_desc_##_name __used   \
> --
> 1.7.10.4
>

Ian Campbell

2013-Dec-04 09:58 UTC

head link

Re: [PATCH] xen: arm: remove hardcoded gnttab location from dom0

On Wed, 2013-12-04 at 02:18 +0800, Chen Baozi wrote:> On Dec 3, 2013, at 23:04, Ian Campbell <Ian.Campbell@citrix.com>
wrote:
> 
> > The DT provided to guests (including dom0) includes a Xen node which,
among
> > other things, describes an MMIO region which can be safely used for
grant
> > table mappings (i.e. it is a hole in the physical address space). For
domU we
> > provide a hardcoded values based on our hardcoded guest virtual
machine
> > layout. However for dom0 we need to fit in with the underlying
platform.
> > Leaving this hardcoded was an oversight which on some platforms could
result
> > in the grant table overlaying RAM or MMIO regions which are in use by
domain
> > 0.
> > 
> > For the 4.4 release do as we did with the dom0 evtchn PPI and provide
a hook
> > for the platform code to supply a suitable hardcoded address for the
platform
> > (derived from reading the data sheet). Platforms which do not provide
the hook
> > get the existing address as a default.
> > 
> > After 4.4 we should switch to selecting a region of host RAM which is
not RAM
> > in the guest address map. This should be more flexible and safer but
the patch
> > was looking too complex for 4.4.
> > 
> > Platform        Gnttab Address
> > ========        =============> > exynos5.c       0xb0000000,
confirmed with Julien.
> > sunxi.c         0x01d00000, confirmed in data sheet.
> > midway.c        0xff800000, confirmed with Andre, boot tested.
> > vexpress.c      0xb0000000, existing hardcoded value was selected for
vexpress.
> > omap5.c         0xb0000000, no datasheet, looks safe in DTB.
> According to OMAP5432 data sheet, 0x80000000~0xbfffffff is the DRAM 
> address space. So this is Ok.
If this is DRAM address space then it''s not OK -- what if the guest has
RAM mapped there?

Looks like there are reserved regions at 0x4b000000 or 0x58800000, which
ought to be suitable I think.

> Acked-by: Chen Baozi <baozich@gmail.com>
> 
> Cheers,
> 
> Baozi

Chen Baozi

2013-Dec-04 14:49 UTC

head link

Re: [PATCH] xen: arm: remove hardcoded gnttab location from dom0

On Wed, Dec 4, 2013 at 5:58 PM, Ian Campbell <Ian.Campbell@citrix.com>
wrote:> On Wed, 2013-12-04 at 02:18 +0800, Chen Baozi wrote:
>> On Dec 3, 2013, at 23:04, Ian Campbell <Ian.Campbell@citrix.com>
wrote:
>>
>> > The DT provided to guests (including dom0) includes a Xen node
which, among
>> > other things, describes an MMIO region which can be safely used
for grant
>> > table mappings (i.e. it is a hole in the physical address space).
For domU we
>> > provide a hardcoded values based on our hardcoded guest virtual
machine
>> > layout. However for dom0 we need to fit in with the underlying
platform.
>> > Leaving this hardcoded was an oversight which on some platforms
could result
>> > in the grant table overlaying RAM or MMIO regions which are in use
by domain
>> > 0.
>> >
>> > For the 4.4 release do as we did with the dom0 evtchn PPI and
provide a hook
>> > for the platform code to supply a suitable hardcoded address for
the platform
>> > (derived from reading the data sheet). Platforms which do not
provide the hook
>> > get the existing address as a default.
>> >
>> > After 4.4 we should switch to selecting a region of host RAM which
is not RAM
>> > in the guest address map. This should be more flexible and safer
but the patch
>> > was looking too complex for 4.4.
>> >
>> > Platform        Gnttab Address
>> > ========        =============>> > exynos5.c      
0xb0000000, confirmed with Julien.
>> > sunxi.c         0x01d00000, confirmed in data sheet.
>> > midway.c        0xff800000, confirmed with Andre, boot tested.
>> > vexpress.c      0xb0000000, existing hardcoded value was selected
for vexpress.
>> > omap5.c         0xb0000000, no datasheet, looks safe in DTB.
>> According to OMAP5432 data sheet, 0x80000000~0xbfffffff is the DRAM
>> address space. So this is Ok.
>
> If this is DRAM address space then it''s not OK -- what if the
guest has
> RAM mapped there?
>
> Looks like there are reserved regions at 0x4b000000 or 0x58800000, which
> ought to be suitable I think.
Sorry, I misunderstood it.

Yes, 0x4b000000 ~ 0x4bffffff and 0x58800000 ~ 0x58ffffff are reserved
area. Besides, there are also other reserved regions of memory space,
such as 0x30000000 ~ 0x3fffffff.

Thanks,

Baozi
>
>
>> Acked-by: Chen Baozi <baozich@gmail.com>
>>
>> Cheers,
>>
>> Baozi
>
>

Xen devel - Dec 2013 - [PATCH] xen: arm: remove hardcoded gnttab location from dom0

[PATCH] xen: arm: remove hardcoded gnttab location from dom0

Re: [PATCH] xen: arm: remove hardcoded gnttab location from dom0

Re: [PATCH] xen: arm: remove hardcoded gnttab location from dom0

Re: [PATCH] xen: arm: remove hardcoded gnttab location from dom0

Re: [PATCH] xen: arm: remove hardcoded gnttab location from dom0

Re: [PATCH] xen: arm: remove hardcoded gnttab location from dom0

Re: [PATCH] xen: arm: remove hardcoded gnttab location from dom0

Re: [PATCH] xen: arm: remove hardcoded gnttab location from dom0

Re: [PATCH] xen: arm: remove hardcoded gnttab location from dom0