From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andre Przywara Subject: Re: [PATCH] xen: arm: remove hardcoded gnttab location from dom0 Date: Tue, 03 Dec 2013 16:32:37 +0100 Message-ID: <529DF995.4060604@linaro.org> References: <1386083062-30104-1-git-send-email-ian.campbell@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1386083062-30104-1-git-send-email-ian.campbell@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: stefano.stabellini@eu.citrix.com, julien.grall@linaro.org, tim@xen.org, xen-devel@lists.xen.org, Chen Baozi , Anup Patel , Pranavkumar Sawargaonkar List-Id: xen-devel@lists.xenproject.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 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 > Cc: Julien.Grall@linaro.org > Cc: Stefano.Stabellini@eu.citrix.com > Cc: Andre Przywara > Cc: Chen Baozi > Cc: Pranavkumar Sawargaonkar > Cc: Anup Patel > --- > 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 = ®[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 \ >