| Message ID | 1386258131-755-5-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, 2013-12-05 at 15:42 +0000, Julien Grall wrote: > Use the field 'avail' to store the type of the page. This information will be > retrieved in a future patch to change the behaviour when the page is removed. > > Also introduce guest_physmap_add_entry to map and set a specific p2m type for > a page. > > Signed-off-by: Julien Grall <julien.grall@linaro.org> > --- > xen/arch/arm/p2m.c | 49 +++++++++++++++++++++++++++++++-------------- > xen/include/asm-arm/p2m.h | 18 +++++++++++++---- > 2 files changed, 48 insertions(+), 19 deletions(-) > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > index 8f8b47e..5449a35 100644 > --- a/xen/arch/arm/p2m.c > +++ b/xen/arch/arm/p2m.c > @@ -124,7 +124,8 @@ int p2m_pod_decrease_reservation(struct domain *d, > return -ENOSYS; > } > > -static lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr) > +static lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr, > + p2m_type_t t) > { > paddr_t pa = ((paddr_t) mfn) << PAGE_SHIFT; > lpae_t e = (lpae_t) { > @@ -132,12 +133,25 @@ static lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr) > .p2m.af = 1, > .p2m.sh = LPAE_SH_OUTER, > .p2m.read = 1, > - .p2m.write = 1, > .p2m.mattr = mattr, > .p2m.table = 1, > .p2m.valid = 1, > + .p2m.avail = t, /* Use avail to store p2m type */ Can we change the name in the struct instead and have a comment "using avail bits for type" there instead please. We only have 5 types so could only steal 3 but we may as well take all 4. A BUILD_BUG_ON to check that the last p2m entry is < 2^4 would be good too. > }; > > + switch (t) > + { > + case p2m_ram_rw: > + case p2m_mmio_direct: > + case p2m_map_foreign: > + e.p2m.write = 1; > + break; > + case p2m_invalid: > + case p2m_ram_ro: > + default: > + e.p2m.write = 0; > + } > + > ASSERT(!(pa & ~PAGE_MASK)); > ASSERT(!(pa & ~PADDR_MASK)); > > @@ -167,7 +181,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, p2m_invalid); > > write_pte(entry, pte); > > @@ -185,7 +199,8 @@ static int create_p2m_entries(struct domain *d, > paddr_t start_gpaddr, > paddr_t end_gpaddr, > paddr_t maddr, > - int mattr) > + int mattr, > + p2m_type_t t) > { > int rc, flush; > struct p2m_domain *p2m = &d->arch.p2m; > @@ -260,14 +275,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, t); > > 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, t); > write_pte(&third[third_table_offset(addr)], pte); > maddr += PAGE_SIZE; > } > @@ -302,7 +318,8 @@ 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, p2m_ram_rw); > } > > int map_mmio_regions(struct domain *d, > @@ -310,18 +327,20 @@ 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, p2m_mmio_direct); > } > > -int guest_physmap_add_page(struct domain *d, > - unsigned long gpfn, > - unsigned long mfn, > - unsigned int page_order) > +int guest_physmap_add_entry(struct domain *d, > + unsigned long gpfn, > + unsigned long mfn, > + unsigned long page_order, > + p2m_type_t t) > { > return create_p2m_entries(d, INSERT, > pfn_to_paddr(gpfn), > - pfn_to_paddr(gpfn + (1<<page_order)), > - pfn_to_paddr(mfn), MATTR_MEM); > + pfn_to_paddr(gpfn + (1 << page_order)), > + pfn_to_paddr(mfn), MATTR_MEM, t); > } > > void guest_physmap_remove_page(struct domain *d, > @@ -331,7 +350,7 @@ void guest_physmap_remove_page(struct domain *d, > create_p2m_entries(d, REMOVE, > pfn_to_paddr(gpfn), > pfn_to_paddr(gpfn + (1<<page_order)), > - pfn_to_paddr(mfn), MATTR_MEM); > + pfn_to_paddr(mfn), MATTR_MEM, p2m_invalid); > } > > int p2m_alloc_table(struct domain *d) > diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h > index b24f94a..1cf7d36 100644 > --- a/xen/include/asm-arm/p2m.h > +++ b/xen/include/asm-arm/p2m.h > @@ -59,11 +59,21 @@ int p2m_populate_ram(struct domain *d, paddr_t start, paddr_t end); > int map_mmio_regions(struct domain *d, paddr_t start_gaddr, > paddr_t end_gaddr, paddr_t maddr); > > +int guest_physmap_add_entry(struct domain *d, > + unsigned long gfn, > + unsigned long mfn, > + unsigned long page_order, > + p2m_type_t t); > + > /* Untyped version for RAM only, for compatibility */ > -int guest_physmap_add_page(struct domain *d, > - unsigned long gfn, > - unsigned long mfn, > - unsigned int page_order); > +static inline int guest_physmap_add_page(struct domain *d, > + unsigned long gfn, > + unsigned long mfn, > + unsigned int page_order) > +{ > + return guest_physmap_add_entry(d, gfn, mfn, page_order, p2m_ram_rw); > +} > + > void guest_physmap_remove_page(struct domain *d, > unsigned long gpfn, > unsigned long mfn, unsigned int page_order);
On Thu, 2013-12-05 at 15:55 +0000, Ian Campbell wrote: [....] oops, hit send before I was done ;-) > > }; > > > > + switch (t) > > + { > > + case p2m_ram_rw: > > + case p2m_mmio_direct: > > + case p2m_map_foreign: > > + e.p2m.write = 1; > > + break; > > + case p2m_invalid: > > + case p2m_ram_ro: > > + default: > > + e.p2m.write = 0; > > + } > > + > > ASSERT(!(pa & ~PAGE_MASK)); > > ASSERT(!(pa & ~PADDR_MASK)); > > > > @@ -167,7 +181,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, p2m_invalid); Is invalid really the right type here? Are you using invalid for non-leaf pages? I think the table bit suffices to identify them, doesn't it? Really the type field is only valid for the leafs ptes (including superpage leafs). If it is useful to have "pseudo-types" which don't directly correspond to the 4 available bits perhaps make them >= 16, although then you would need a function to take a pte and extract its type taking that into account. > > > > -int guest_physmap_add_page(struct domain *d, > > - unsigned long gpfn, > > - unsigned long mfn, > > - unsigned int page_order) > > +int guest_physmap_add_entry(struct domain *d, > > + unsigned long gpfn, > > + unsigned long mfn, > > + unsigned long page_order, > > + p2m_type_t t) > > { > > return create_p2m_entries(d, INSERT, > > pfn_to_paddr(gpfn), > > - pfn_to_paddr(gpfn + (1<<page_order)), > > - pfn_to_paddr(mfn), MATTR_MEM); > > + pfn_to_paddr(gpfn + (1 << page_order)), > > + pfn_to_paddr(mfn), MATTR_MEM, t); > > } > > > > void guest_physmap_remove_page(struct domain *d, > > @@ -331,7 +350,7 @@ void guest_physmap_remove_page(struct domain *d, > > create_p2m_entries(d, REMOVE, > > pfn_to_paddr(gpfn), > > pfn_to_paddr(gpfn + (1<<page_order)), > > - pfn_to_paddr(mfn), MATTR_MEM); > > + pfn_to_paddr(mfn), MATTR_MEM, p2m_invalid); This looks odd to me, although I understand it is calling a common helper function which requires a type elsewhere. Perhpas p2m_invalid here really needs a placeholder. p2m_none == INT_MAX perhaps? Ian.
On 12/05/2013 03:55 PM, Ian Campbell wrote: > On Thu, 2013-12-05 at 15:42 +0000, Julien Grall wrote: >> Use the field 'avail' to store the type of the page. This information will be >> retrieved in a future patch to change the behaviour when the page is removed. >> >> Also introduce guest_physmap_add_entry to map and set a specific p2m type for >> a page. >> >> Signed-off-by: Julien Grall <julien.grall@linaro.org> >> --- >> xen/arch/arm/p2m.c | 49 +++++++++++++++++++++++++++++++-------------- >> xen/include/asm-arm/p2m.h | 18 +++++++++++++---- >> 2 files changed, 48 insertions(+), 19 deletions(-) >> >> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c >> index 8f8b47e..5449a35 100644 >> --- a/xen/arch/arm/p2m.c >> +++ b/xen/arch/arm/p2m.c >> @@ -124,7 +124,8 @@ int p2m_pod_decrease_reservation(struct domain *d, >> return -ENOSYS; >> } >> >> -static lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr) >> +static lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr, >> + p2m_type_t t) >> { >> paddr_t pa = ((paddr_t) mfn) << PAGE_SHIFT; >> lpae_t e = (lpae_t) { >> @@ -132,12 +133,25 @@ static lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr) >> .p2m.af = 1, >> .p2m.sh = LPAE_SH_OUTER, >> .p2m.read = 1, >> - .p2m.write = 1, >> .p2m.mattr = mattr, >> .p2m.table = 1, >> .p2m.valid = 1, >> + .p2m.avail = t, /* Use avail to store p2m type */ > > Can we change the name in the struct instead and have a comment "using > avail bits for type" there instead please. We only have 5 types so could > only steal 3 but we may as well take all 4. > > A BUILD_BUG_ON to check that the last p2m entry is < 2^4 would be good > too. I will do both modification for the next version.
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 8f8b47e..5449a35 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -124,7 +124,8 @@ int p2m_pod_decrease_reservation(struct domain *d, return -ENOSYS; } -static lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr) +static lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr, + p2m_type_t t) { paddr_t pa = ((paddr_t) mfn) << PAGE_SHIFT; lpae_t e = (lpae_t) { @@ -132,12 +133,25 @@ static lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr) .p2m.af = 1, .p2m.sh = LPAE_SH_OUTER, .p2m.read = 1, - .p2m.write = 1, .p2m.mattr = mattr, .p2m.table = 1, .p2m.valid = 1, + .p2m.avail = t, /* Use avail to store p2m type */ }; + switch (t) + { + case p2m_ram_rw: + case p2m_mmio_direct: + case p2m_map_foreign: + e.p2m.write = 1; + break; + case p2m_invalid: + case p2m_ram_ro: + default: + e.p2m.write = 0; + } + ASSERT(!(pa & ~PAGE_MASK)); ASSERT(!(pa & ~PADDR_MASK)); @@ -167,7 +181,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, p2m_invalid); write_pte(entry, pte); @@ -185,7 +199,8 @@ static int create_p2m_entries(struct domain *d, paddr_t start_gpaddr, paddr_t end_gpaddr, paddr_t maddr, - int mattr) + int mattr, + p2m_type_t t) { int rc, flush; struct p2m_domain *p2m = &d->arch.p2m; @@ -260,14 +275,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, t); 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, t); write_pte(&third[third_table_offset(addr)], pte); maddr += PAGE_SIZE; } @@ -302,7 +318,8 @@ 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, p2m_ram_rw); } int map_mmio_regions(struct domain *d, @@ -310,18 +327,20 @@ 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, p2m_mmio_direct); } -int guest_physmap_add_page(struct domain *d, - unsigned long gpfn, - unsigned long mfn, - unsigned int page_order) +int guest_physmap_add_entry(struct domain *d, + unsigned long gpfn, + unsigned long mfn, + unsigned long page_order, + p2m_type_t t) { return create_p2m_entries(d, INSERT, pfn_to_paddr(gpfn), - pfn_to_paddr(gpfn + (1<<page_order)), - pfn_to_paddr(mfn), MATTR_MEM); + pfn_to_paddr(gpfn + (1 << page_order)), + pfn_to_paddr(mfn), MATTR_MEM, t); } void guest_physmap_remove_page(struct domain *d, @@ -331,7 +350,7 @@ void guest_physmap_remove_page(struct domain *d, create_p2m_entries(d, REMOVE, pfn_to_paddr(gpfn), pfn_to_paddr(gpfn + (1<<page_order)), - pfn_to_paddr(mfn), MATTR_MEM); + pfn_to_paddr(mfn), MATTR_MEM, p2m_invalid); } int p2m_alloc_table(struct domain *d) diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h index b24f94a..1cf7d36 100644 --- a/xen/include/asm-arm/p2m.h +++ b/xen/include/asm-arm/p2m.h @@ -59,11 +59,21 @@ int p2m_populate_ram(struct domain *d, paddr_t start, paddr_t end); int map_mmio_regions(struct domain *d, paddr_t start_gaddr, paddr_t end_gaddr, paddr_t maddr); +int guest_physmap_add_entry(struct domain *d, + unsigned long gfn, + unsigned long mfn, + unsigned long page_order, + p2m_type_t t); + /* Untyped version for RAM only, for compatibility */ -int guest_physmap_add_page(struct domain *d, - unsigned long gfn, - unsigned long mfn, - unsigned int page_order); +static inline int guest_physmap_add_page(struct domain *d, + unsigned long gfn, + unsigned long mfn, + unsigned int page_order) +{ + return guest_physmap_add_entry(d, gfn, mfn, page_order, p2m_ram_rw); +} + void guest_physmap_remove_page(struct domain *d, unsigned long gpfn, unsigned long mfn, unsigned int page_order);
Use the field 'avail' to store the type of the page. This information will be retrieved in a future patch to change the behaviour when the page is removed. Also introduce guest_physmap_add_entry to map and set a specific p2m type for a page. Signed-off-by: Julien Grall <julien.grall@linaro.org> --- xen/arch/arm/p2m.c | 49 +++++++++++++++++++++++++++++++-------------- xen/include/asm-arm/p2m.h | 18 +++++++++++++---- 2 files changed, 48 insertions(+), 19 deletions(-)