| Message ID | 1387290499-3970-1-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Tue, 17 Dec 2013, Julien Grall wrote: > With the lack of iommu, dom0 must have a 1:1 memory mapping for all > these guest physical address. When the ballon decides to give back a > page to the kernel, this page must have the same address as previously. > Otherwise, we will loose the 1:1 mapping and will break DMA-capable > devices. > > Signed-off-by: Julien Grall <julien.grall@linaro.org> > Reviewed-by: Jan Beulich <jbeulich@suse.com> > Cc: Keir Fraser <keir@xen.org> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Release: This is a bug that prevents DMA-capable devices to work after > a guest has started. > > Changes in v6: > - Use # instead of 0x > - Make sure that d is evaluated in is_domain_direct_mapped > Changes in v5: > - Export dom0_11_mapping and transform is_domain_direct_mapped in macro > - Add ifndef in common code to define is_domain_direct_mapped > Changes in v4: > - Fix typoes > - Update comment in the code > Changes in v3: > - Remove spurious page = NULL > - Rename is_dom0_mapped_11 to is_domain_direct_mapped > - Coding style > Changes in v2: > - Drop CONFIG_ARM and add is_dom0_mapped_11 > --- > xen/arch/arm/domain_build.c | 2 +- > xen/common/memory.c | 35 ++++++++++++++++++++++++++++++++++- > xen/include/asm-arm/domain.h | 3 +++ > 3 files changed, 38 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index faff88e..47b781b 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -22,7 +22,7 @@ > static unsigned int __initdata opt_dom0_max_vcpus; > integer_param("dom0_max_vcpus", opt_dom0_max_vcpus); > > -static int dom0_11_mapping = 1; > +int dom0_11_mapping = 1; > > #define DOM0_MEM_DEFAULT 0x8000000 /* 128 MiB */ > static u64 __initdata dom0_mem = DOM0_MEM_DEFAULT; > diff --git a/xen/common/memory.c b/xen/common/memory.c > index eb7b72b..51f11ff 100644 > --- a/xen/common/memory.c > +++ b/xen/common/memory.c > @@ -29,6 +29,10 @@ > #include <xsm/xsm.h> > #include <xen/trace.h> > > +#ifndef is_domain_direct_mapped > +# define is_domain_direct_mapped(d) ((void)(d), 0) > +#endif > + > struct memop_args { > /* INPUT */ > struct domain *domain; /* Domain to be affected. */ > @@ -122,7 +126,29 @@ static void populate_physmap(struct memop_args *a) > } > else > { > - page = alloc_domheap_pages(d, a->extent_order, a->memflags); > + if ( is_domain_direct_mapped(d) ) > + { > + mfn = gpfn; > + if ( !mfn_valid(mfn) ) > + { > + gdprintk(XENLOG_INFO, "Invalid mfn %#"PRI_xen_pfn"\n", > + mfn); > + goto out; > + } > + > + page = mfn_to_page(mfn); > + if ( !get_page(page, d) ) > + { > + gdprintk(XENLOG_INFO, > + "mfn %#"PRI_xen_pfn" doesn't belong to the" > + " domain\n", mfn); > + goto out; > + } > + put_page(page); > + } > + else > + page = alloc_domheap_pages(d, a->extent_order, a->memflags); > + > if ( unlikely(page == NULL) ) > { > if ( !opt_tmem || (a->extent_order != 0) ) > @@ -270,6 +296,13 @@ static void decrease_reservation(struct memop_args *a) > && p2m_pod_decrease_reservation(a->domain, gmfn, a->extent_order) ) > continue; > > + /* With the lack for iommu on some ARM platform, domain with DMA-capable > + * device must retrieve the same pfn when the hypercall > + * populate_physmap is called. > + */ > + if ( is_domain_direct_mapped(a->domain) ) > + continue; > + > for ( j = 0; j < (1 << a->extent_order); j++ ) > if ( !guest_remove_page(a->domain, gmfn + j) ) > goto out; > diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h > index 28d39a0..e2202a6 100644 > --- a/xen/include/asm-arm/domain.h > +++ b/xen/include/asm-arm/domain.h > @@ -86,6 +86,9 @@ enum domain_type { > #define is_pv64_domain(d) (0) > #endif > > +extern int dom0_11_mapping; > +#define is_domain_direct_mapped(d) ((d) == dom0 && dom0_11_mapping) > + > struct vtimer { > struct vcpu *v; > int irq; > -- > 1.7.10.4 >
On Tue, 2013-12-17 at 14:55 +0000, Stefano Stabellini wrote: > On Tue, 17 Dec 2013, Julien Grall wrote: > > With the lack of iommu, dom0 must have a 1:1 memory mapping for all > > these guest physical address. When the ballon decides to give back a > > page to the kernel, this page must have the same address as previously. > > Otherwise, we will loose the 1:1 mapping and will break DMA-capable > > devices. > > > > Signed-off-by: Julien Grall <julien.grall@linaro.org> > > Reviewed-by: Jan Beulich <jbeulich@suse.com> > > Cc: Keir Fraser <keir@xen.org> > > Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Keir, any objections to this patch? > > > > Release: This is a bug that prevents DMA-capable devices to work after > > a guest has started. > > > > Changes in v6: > > - Use # instead of 0x > > - Make sure that d is evaluated in is_domain_direct_mapped > > Changes in v5: > > - Export dom0_11_mapping and transform is_domain_direct_mapped in macro > > - Add ifndef in common code to define is_domain_direct_mapped > > Changes in v4: > > - Fix typoes > > - Update comment in the code > > Changes in v3: > > - Remove spurious page = NULL > > - Rename is_dom0_mapped_11 to is_domain_direct_mapped > > - Coding style > > Changes in v2: > > - Drop CONFIG_ARM and add is_dom0_mapped_11 > > --- > > xen/arch/arm/domain_build.c | 2 +- > > xen/common/memory.c | 35 ++++++++++++++++++++++++++++++++++- > > xen/include/asm-arm/domain.h | 3 +++ > > 3 files changed, 38 insertions(+), 2 deletions(-) > > > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > > index faff88e..47b781b 100644 > > --- a/xen/arch/arm/domain_build.c > > +++ b/xen/arch/arm/domain_build.c > > @@ -22,7 +22,7 @@ > > static unsigned int __initdata opt_dom0_max_vcpus; > > integer_param("dom0_max_vcpus", opt_dom0_max_vcpus); > > > > -static int dom0_11_mapping = 1; > > +int dom0_11_mapping = 1; > > > > #define DOM0_MEM_DEFAULT 0x8000000 /* 128 MiB */ > > static u64 __initdata dom0_mem = DOM0_MEM_DEFAULT; > > diff --git a/xen/common/memory.c b/xen/common/memory.c > > index eb7b72b..51f11ff 100644 > > --- a/xen/common/memory.c > > +++ b/xen/common/memory.c > > @@ -29,6 +29,10 @@ > > #include <xsm/xsm.h> > > #include <xen/trace.h> > > > > +#ifndef is_domain_direct_mapped > > +# define is_domain_direct_mapped(d) ((void)(d), 0) > > +#endif > > + > > struct memop_args { > > /* INPUT */ > > struct domain *domain; /* Domain to be affected. */ > > @@ -122,7 +126,29 @@ static void populate_physmap(struct memop_args *a) > > } > > else > > { > > - page = alloc_domheap_pages(d, a->extent_order, a->memflags); > > + if ( is_domain_direct_mapped(d) ) > > + { > > + mfn = gpfn; > > + if ( !mfn_valid(mfn) ) > > + { > > + gdprintk(XENLOG_INFO, "Invalid mfn %#"PRI_xen_pfn"\n", > > + mfn); > > + goto out; > > + } > > + > > + page = mfn_to_page(mfn); > > + if ( !get_page(page, d) ) > > + { > > + gdprintk(XENLOG_INFO, > > + "mfn %#"PRI_xen_pfn" doesn't belong to the" > > + " domain\n", mfn); > > + goto out; > > + } > > + put_page(page); > > + } > > + else > > + page = alloc_domheap_pages(d, a->extent_order, a->memflags); > > + > > if ( unlikely(page == NULL) ) > > { > > if ( !opt_tmem || (a->extent_order != 0) ) > > @@ -270,6 +296,13 @@ static void decrease_reservation(struct memop_args *a) > > && p2m_pod_decrease_reservation(a->domain, gmfn, a->extent_order) ) > > continue; > > > > + /* With the lack for iommu on some ARM platform, domain with DMA-capable > > + * device must retrieve the same pfn when the hypercall > > + * populate_physmap is called. > > + */ > > + if ( is_domain_direct_mapped(a->domain) ) > > + continue; > > + > > for ( j = 0; j < (1 << a->extent_order); j++ ) > > if ( !guest_remove_page(a->domain, gmfn + j) ) > > goto out; > > diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h > > index 28d39a0..e2202a6 100644 > > --- a/xen/include/asm-arm/domain.h > > +++ b/xen/include/asm-arm/domain.h > > @@ -86,6 +86,9 @@ enum domain_type { > > #define is_pv64_domain(d) (0) > > #endif > > > > +extern int dom0_11_mapping; > > +#define is_domain_direct_mapped(d) ((d) == dom0 && dom0_11_mapping) > > + > > struct vtimer { > > struct vcpu *v; > > int irq; > > -- > > 1.7.10.4 > >
On Mon, 2014-01-06 at 11:48 +0000, Ian Campbell wrote: > On Mon, 2014-01-06 at 11:44 +0000, Julien Grall wrote: > > > > On 12/19/2013 01:51 PM, Ian Campbell wrote: > > > On Tue, 2013-12-17 at 14:55 +0000, Stefano Stabellini wrote: > > >> On Tue, 17 Dec 2013, Julien Grall wrote: > > >>> With the lack of iommu, dom0 must have a 1:1 memory mapping for all > > >>> these guest physical address. When the ballon decides to give back a > > >>> page to the kernel, this page must have the same address as previously. > > >>> Otherwise, we will loose the 1:1 mapping and will break DMA-capable > > >>> devices. > > >>> > > >>> Signed-off-by: Julien Grall <julien.grall@linaro.org> > > >>> Reviewed-by: Jan Beulich <jbeulich@suse.com> > > >>> Cc: Keir Fraser <keir@xen.org> > > >> > > >> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > > > > Keir, any objections to this patch? > > > > Hi Ian, > > > > Do we need to wait Keir Ack? > > I think he's had ample opportunity to object (last call!) and since this > code is explicitly dead on everything apart from ARM I intend to commit > it next time I go through my queue. Which I've now done.
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index faff88e..47b781b 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -22,7 +22,7 @@ static unsigned int __initdata opt_dom0_max_vcpus; integer_param("dom0_max_vcpus", opt_dom0_max_vcpus); -static int dom0_11_mapping = 1; +int dom0_11_mapping = 1; #define DOM0_MEM_DEFAULT 0x8000000 /* 128 MiB */ static u64 __initdata dom0_mem = DOM0_MEM_DEFAULT; diff --git a/xen/common/memory.c b/xen/common/memory.c index eb7b72b..51f11ff 100644 --- a/xen/common/memory.c +++ b/xen/common/memory.c @@ -29,6 +29,10 @@ #include <xsm/xsm.h> #include <xen/trace.h> +#ifndef is_domain_direct_mapped +# define is_domain_direct_mapped(d) ((void)(d), 0) +#endif + struct memop_args { /* INPUT */ struct domain *domain; /* Domain to be affected. */ @@ -122,7 +126,29 @@ static void populate_physmap(struct memop_args *a) } else { - page = alloc_domheap_pages(d, a->extent_order, a->memflags); + if ( is_domain_direct_mapped(d) ) + { + mfn = gpfn; + if ( !mfn_valid(mfn) ) + { + gdprintk(XENLOG_INFO, "Invalid mfn %#"PRI_xen_pfn"\n", + mfn); + goto out; + } + + page = mfn_to_page(mfn); + if ( !get_page(page, d) ) + { + gdprintk(XENLOG_INFO, + "mfn %#"PRI_xen_pfn" doesn't belong to the" + " domain\n", mfn); + goto out; + } + put_page(page); + } + else + page = alloc_domheap_pages(d, a->extent_order, a->memflags); + if ( unlikely(page == NULL) ) { if ( !opt_tmem || (a->extent_order != 0) ) @@ -270,6 +296,13 @@ static void decrease_reservation(struct memop_args *a) && p2m_pod_decrease_reservation(a->domain, gmfn, a->extent_order) ) continue; + /* With the lack for iommu on some ARM platform, domain with DMA-capable + * device must retrieve the same pfn when the hypercall + * populate_physmap is called. + */ + if ( is_domain_direct_mapped(a->domain) ) + continue; + for ( j = 0; j < (1 << a->extent_order); j++ ) if ( !guest_remove_page(a->domain, gmfn + j) ) goto out; diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h index 28d39a0..e2202a6 100644 --- a/xen/include/asm-arm/domain.h +++ b/xen/include/asm-arm/domain.h @@ -86,6 +86,9 @@ enum domain_type { #define is_pv64_domain(d) (0) #endif +extern int dom0_11_mapping; +#define is_domain_direct_mapped(d) ((d) == dom0 && dom0_11_mapping) + struct vtimer { struct vcpu *v; int irq;