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

[Andre Przywara <andre.przywara@linaro.org>]

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   \
>