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

[Pranavkumar Sawargaonkar <psawargaonkar@xxxxxxx>]

On Tue, Dec 3, 2013 at 8:34 PM, Ian Campbell <ian.campbell@xxxxxxxxxx> 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@xxxxxxx>

>
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> Cc: Julien.Grall@xxxxxxxxxx
> Cc: Stefano.Stabellini@xxxxxxxxxxxxx
> Cc: Andre Przywara <andre.przywara@xxxxxxxxxx>
> Cc: Chen Baozi <baozich@xxxxxxxxx>
> Cc: Pranavkumar Sawargaonkar <psawargaonkar@xxxxxxx>
> Cc: Anup Patel <apatel@xxxxxxx>
> ---
>  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
>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel