Julien Grall
2013-Oct-24 09:02 UTC
[PATCH 0/3] ARM: Support read-only mapping in the grant table
Hi, This small patch series implements read-only mapping for the grant table. The read-only mapping is used by block-front when persistent grants is not used. It''s the case when a freebsd domU is booting on Xen on ARM. The first two patches extend the p2m interface to support read-only mapping. The third patch add support to read-only mapping in the grant table. Cheers, Julien Grall (3): xen/arm: p2m: extend create_p2m_entries to support read-only mapping xen/arm: p2m: add guest_physmap_add_page_rw xen/arm: grant-table: Support read-only mapping xen/arch/arm/mm.c | 12 ++++-------- xen/arch/arm/p2m.c | 30 ++++++++++++++++++++++-------- xen/include/asm-arm/p2m.h | 5 +++++ xen/include/asm-arm/page.h | 5 +++-- 4 files changed, 34 insertions(+), 18 deletions(-) -- 1.8.3.1
Julien Grall
2013-Oct-24 09:02 UTC
[PATCH 1/3] xen/arm: p2m: extend create_p2m_entries to support read-only mapping
Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
xen/arch/arm/p2m.c | 19 +++++++++++--------
xen/include/asm-arm/page.h | 5 +++--
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 2d09fef..fdbb07b 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -111,7 +111,7 @@ static int p2m_create_table(struct domain *d,
clear_page(p);
unmap_domain_page(p);
- pte = mfn_to_p2m_entry(page_to_mfn(page), MATTR_MEM);
+ pte = mfn_to_p2m_entry(page_to_mfn(page), MATTR_MEM, 1);
write_pte(entry, pte);
@@ -129,7 +129,8 @@ static int create_p2m_entries(struct domain *d,
paddr_t start_gpaddr,
paddr_t end_gpaddr,
paddr_t maddr,
- int mattr)
+ int mattr,
+ bool_t rw)
{
int rc, flush;
struct p2m_domain *p2m = &d->arch.p2m;
@@ -201,14 +202,15 @@ static int create_p2m_entries(struct domain *d,
goto out;
}
- pte = mfn_to_p2m_entry(page_to_mfn(page), mattr);
+ pte = mfn_to_p2m_entry(page_to_mfn(page), mattr, rw);
write_pte(&third[third_table_offset(addr)], pte);
}
break;
case INSERT:
{
- lpae_t pte = mfn_to_p2m_entry(maddr >> PAGE_SHIFT,
mattr);
+ lpae_t pte = mfn_to_p2m_entry(maddr >> PAGE_SHIFT,
+ mattr, rw);
write_pte(&third[third_table_offset(addr)], pte);
maddr += PAGE_SIZE;
}
@@ -243,7 +245,7 @@ int p2m_populate_ram(struct domain *d,
paddr_t start,
paddr_t end)
{
- return create_p2m_entries(d, ALLOCATE, start, end, 0, MATTR_MEM);
+ return create_p2m_entries(d, ALLOCATE, start, end, 0, MATTR_MEM, 1);
}
int map_mmio_regions(struct domain *d,
@@ -251,7 +253,8 @@ int map_mmio_regions(struct domain *d,
paddr_t end_gaddr,
paddr_t maddr)
{
- return create_p2m_entries(d, INSERT, start_gaddr, end_gaddr, maddr,
MATTR_DEV);
+ return create_p2m_entries(d, INSERT, start_gaddr, end_gaddr,
+ maddr, MATTR_DEV, 1);
}
int guest_physmap_add_page(struct domain *d,
@@ -261,7 +264,7 @@ int guest_physmap_add_page(struct domain *d,
{
return create_p2m_entries(d, INSERT, gpfn << PAGE_SHIFT,
(gpfn + (1<<page_order)) <<
PAGE_SHIFT,
- mfn << PAGE_SHIFT, MATTR_MEM);
+ mfn << PAGE_SHIFT, MATTR_MEM, 1);
}
void guest_physmap_remove_page(struct domain *d,
@@ -270,7 +273,7 @@ void guest_physmap_remove_page(struct domain *d,
{
create_p2m_entries(d, REMOVE, gpfn << PAGE_SHIFT,
(gpfn + (1<<page_order)) << PAGE_SHIFT,
- mfn << PAGE_SHIFT, MATTR_MEM);
+ mfn << PAGE_SHIFT, MATTR_MEM, 0);
}
int p2m_alloc_table(struct domain *d)
diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index 3d0f8a9..3e9dda7 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -213,14 +213,15 @@ static inline lpae_t mfn_to_xen_entry(unsigned long mfn)
return e;
}
-static inline lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr)
+static inline lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr,
+ bool_t rw)
{
paddr_t pa = ((paddr_t) mfn) << PAGE_SHIFT;
lpae_t e = (lpae_t) {
.p2m.xn = 0,
.p2m.af = 1,
.p2m.sh = LPAE_SH_OUTER,
- .p2m.write = 1,
+ .p2m.write = !!rw,
.p2m.read = 1,
.p2m.mattr = mattr,
.p2m.table = 1,
--
1.8.3.1
Julien Grall
2013-Oct-24 09:02 UTC
[PATCH 2/3] xen/arm: p2m: add guest_physmap_add_page_rw
This function allows Xen to map memory read/write or read-only to the
guest memory.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
xen/arch/arm/p2m.c | 11 +++++++++++
xen/include/asm-arm/p2m.h | 5 +++++
2 files changed, 16 insertions(+)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index fdbb07b..0c515e1 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -267,6 +267,17 @@ int guest_physmap_add_page(struct domain *d,
mfn << PAGE_SHIFT, MATTR_MEM, 1);
}
+int guest_physmap_add_page_rw(struct domain *d,
+ unsigned long gpfn,
+ unsigned long mfn,
+ unsigned int page_order,
+ bool_t rw)
+{
+ return create_p2m_entries(d, INSERT, gpfn << PAGE_SHIFT,
+ (gpfn + (1 << page_order)) <<
PAGE_SHIFT,
+ mfn << PAGE_SHIFT, MATTR_MEM, rw);
+}
+
void guest_physmap_remove_page(struct domain *d,
unsigned long gpfn,
unsigned long mfn, unsigned int page_order)
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index c660820..250f916 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -54,6 +54,11 @@ int guest_physmap_add_page(struct domain *d,
unsigned long gfn,
unsigned long mfn,
unsigned int page_order);
+int guest_physmap_add_page_rw(struct domain *d,
+ unsigned long gfn,
+ unsigned long mfn,
+ unsigned int page_order,
+ bool_t rw);
void guest_physmap_remove_page(struct domain *d,
unsigned long gpfn,
unsigned long mfn, unsigned int page_order);
--
1.8.3.1
Julien Grall
2013-Oct-24 09:02 UTC
[PATCH 3/3] xen/arm: grant-table: Support read-only mapping
Use guest_physmap_add_page_rw to map the page in the guest with a
specific read-write attribute.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
xen/arch/arm/mm.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index eaeb0c3..e45c6aa 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1262,19 +1262,15 @@ int create_grant_host_mapping(unsigned long addr,
unsigned long frame,
unsigned int flags, unsigned int cache_flags)
{
int rc;
+ bool_t rw;
if ( cache_flags || (flags & ~GNTMAP_readonly) != GNTMAP_host_map )
return GNTST_general_error;
- /* XXX: read only mappings */
- if ( flags & GNTMAP_readonly )
- {
- gdprintk(XENLOG_WARNING, "read only mappings not implemented
yet\n");
- return GNTST_general_error;
- }
+ rw = !(flags & GNTMAP_readonly);
- rc = guest_physmap_add_page(current->domain,
- addr >> PAGE_SHIFT, frame, 0);
+ rc = guest_physmap_add_page_rw(current->domain,
+ addr >> PAGE_SHIFT, frame, 0, rw);
if ( rc )
return GNTST_general_error;
else
--
1.8.3.1
Ian Campbell
2013-Nov-11 13:23 UTC
Re: [PATCH 1/3] xen/arm: p2m: extend create_p2m_entries to support read-only mapping
On Thu, 2013-10-24 at 10:02 +0100, Julien Grall wrote:> Signed-off-by: Julien Grall <julien.grall@linaro.org> > --- > xen/arch/arm/p2m.c | 19 +++++++++++-------- > xen/include/asm-arm/page.h | 5 +++-- > 2 files changed, 14 insertions(+), 10 deletions(-) > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > index 2d09fef..fdbb07b 100644 > --- a/xen/arch/arm/p2m.c > +++ b/xen/arch/arm/p2m.c > @@ -111,7 +111,7 @@ static int p2m_create_table(struct domain *d, > clear_page(p); > unmap_domain_page(p); > > - pte = mfn_to_p2m_entry(page_to_mfn(page), MATTR_MEM); > + pte = mfn_to_p2m_entry(page_to_mfn(page), MATTR_MEM, 1);If this is a bool_t then true/false might be more appropriate? (and all the other such too) I have a weak preference for avoiding such boolean parameters, since it is never obvious what a given 1/0/true/false is actually is without having to look at the prototype. At least there is only 1 such param here (now). We don''t need full p2m typing yet but perhaps we can make a start by defining "typdef enum { p2m_ram_rw = N; p2m_ram_ro = M } p2m_type_t" for this?> @@ -213,14 +213,15 @@ static inline lpae_t mfn_to_xen_entry(unsigned long mfn) > return e; > } > > -static inline lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr) > +static inline lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr, > + bool_t rw) > { > paddr_t pa = ((paddr_t) mfn) << PAGE_SHIFT; > lpae_t e = (lpae_t) { > .p2m.xn = 0, > .p2m.af = 1, > .p2m.sh = LPAE_SH_OUTER, > - .p2m.write = 1, > + .p2m.write = !!rw,I don''t think the !! is strictly necessary for either a bool_t nor a single bit bitfield. Ian.
Ian Campbell
2013-Nov-11 13:30 UTC
Re: [PATCH 2/3] xen/arm: p2m: add guest_physmap_add_page_rw
On Thu, 2013-10-24 at 10:02 +0100, Julien Grall wrote:> This function allows Xen to map memory read/write or read-only to the > guest memory.x86''s equivalent seems to be guest_physmap_add_entry() which is used by guest_physmap_add_page. Unless there is a reason to diverge it would be nice to keep the arch API similar I think. x86 also passes a p2m type as I suggested in my previous reply. Ian.
Julien Grall
2013-Nov-12 13:13 UTC
Re: [PATCH 1/3] xen/arm: p2m: extend create_p2m_entries to support read-only mapping
On 11/11/2013 01:23 PM, Ian Campbell wrote:> On Thu, 2013-10-24 at 10:02 +0100, Julien Grall wrote: >> Signed-off-by: Julien Grall <julien.grall@linaro.org> >> --- >> xen/arch/arm/p2m.c | 19 +++++++++++-------- >> xen/include/asm-arm/page.h | 5 +++-- >> 2 files changed, 14 insertions(+), 10 deletions(-) >> >> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c >> index 2d09fef..fdbb07b 100644 >> --- a/xen/arch/arm/p2m.c >> +++ b/xen/arch/arm/p2m.c >> @@ -111,7 +111,7 @@ static int p2m_create_table(struct domain *d, >> clear_page(p); >> unmap_domain_page(p); >> >> - pte = mfn_to_p2m_entry(page_to_mfn(page), MATTR_MEM); >> + pte = mfn_to_p2m_entry(page_to_mfn(page), MATTR_MEM, 1); > > If this is a bool_t then true/false might be more appropriate? (and all > the other such too) > > I have a weak preference for avoiding such boolean parameters, since it > is never obvious what a given 1/0/true/false is actually is without > having to look at the prototype. At least there is only 1 such param > here (now). > > We don''t need full p2m typing yet but perhaps we can make a start by > defining "typdef enum { p2m_ram_rw = N; p2m_ram_ro = M } p2m_type_t" for > this?Sounds good. I will update the patch series with that.>> @@ -213,14 +213,15 @@ static inline lpae_t mfn_to_xen_entry(unsigned long mfn) >> return e; >> } >> >> -static inline lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr) >> +static inline lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr, >> + bool_t rw) >> { >> paddr_t pa = ((paddr_t) mfn) << PAGE_SHIFT; >> lpae_t e = (lpae_t) { >> .p2m.xn = 0, >> .p2m.af = 1, >> .p2m.sh = LPAE_SH_OUTER, >> - .p2m.write = 1, >> + .p2m.write = !!rw, > > I don''t think the !! is strictly necessary for either a bool_t nor a > single bit bitfield.I will use p2m_type_t here and check if the type is equal to p2m_ram_rw. -- Julien Grall