| Message ID | 1387215452-10951-8-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Mon, 2013-12-16 at 17:37 +0000, Julien Grall wrote: > Modify get_page_from_gfn to take reference on foreign mapping. This will avoid > specific handling in the common code. > > Signed-off-by: Julien Grall <julien.grall@linaro.org> > > --- > Changes in v4.2: > - get_page_from_gfn: move foreign checking before get_page > - add assert fdom != dom > Changes in v4.1: > - Remove specific p2m handling in common code > - Handle foreign mapping in get_page_from_gfn > Changes in v4: > - Split patch #6 from dom0 pvh series v6.2 to retrieve only common > code. > - Rework commit title > - Rename xen_rem_foreign_from_p2m in p2m_remove_foreign > - Get the mfn from the pte. We are not sure that maddr given in > parameters is valid > Changes in v3: > - Move put_page in create_p2m_entries > - Move xenmem_rem_foreign_from_p2m in arch/arm/p2m.c > Changes in v2: > - Introduce the patch > --- > xen/arch/arm/p2m.c | 15 +++++++++++++-- > xen/include/asm-arm/p2m.h | 12 ++++++++++++ > 2 files changed, 25 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > index 9bdcacd..d05fdff 100644 > --- a/xen/arch/arm/p2m.c > +++ b/xen/arch/arm/p2m.c > @@ -317,10 +317,21 @@ static int create_p2m_entries(struct domain *d, > break; > case REMOVE: > { > - lpae_t pte; > + lpae_t pte = third[third_table_offset(addr)]; > + unsigned long mfn; > + > + maddr = (pte.bits & PADDR_MASK & PAGE_MASK); > + mfn = paddr_to_pfn(maddr); FWIW mfn = pte.p2m.base, I think. I'm not sure we use this everywhere we could. > + > + /* TODO: Handle other p2m type */ > + if ( pte.p2m.valid && p2m_is_foreign(pte.p2m.type) ) How useful is p2m_is_foreign now that this stuff doesn't need to be in common code? Both of the above are really just observations rather than requests for change, so: Acked-by: Ian Campbell <ian.campbell@citrix.com>
On 12/17/2013 11:18 AM, Ian Campbell wrote: > On Mon, 2013-12-16 at 17:37 +0000, Julien Grall wrote: >> Modify get_page_from_gfn to take reference on foreign mapping. This will avoid >> specific handling in the common code. >> >> Signed-off-by: Julien Grall <julien.grall@linaro.org> >> >> --- >> Changes in v4.2: >> - get_page_from_gfn: move foreign checking before get_page >> - add assert fdom != dom >> Changes in v4.1: >> - Remove specific p2m handling in common code >> - Handle foreign mapping in get_page_from_gfn >> Changes in v4: >> - Split patch #6 from dom0 pvh series v6.2 to retrieve only common >> code. >> - Rework commit title >> - Rename xen_rem_foreign_from_p2m in p2m_remove_foreign >> - Get the mfn from the pte. We are not sure that maddr given in >> parameters is valid >> Changes in v3: >> - Move put_page in create_p2m_entries >> - Move xenmem_rem_foreign_from_p2m in arch/arm/p2m.c >> Changes in v2: >> - Introduce the patch >> --- >> xen/arch/arm/p2m.c | 15 +++++++++++++-- >> xen/include/asm-arm/p2m.h | 12 ++++++++++++ >> 2 files changed, 25 insertions(+), 2 deletions(-) >> >> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c >> index 9bdcacd..d05fdff 100644 >> --- a/xen/arch/arm/p2m.c >> +++ b/xen/arch/arm/p2m.c >> @@ -317,10 +317,21 @@ static int create_p2m_entries(struct domain *d, >> break; >> case REMOVE: >> { >> - lpae_t pte; >> + lpae_t pte = third[third_table_offset(addr)]; >> + unsigned long mfn; >> + >> + maddr = (pte.bits & PADDR_MASK & PAGE_MASK); >> + mfn = paddr_to_pfn(maddr); > > FWIW mfn = pte.p2m.base, I think. I'm not sure we use this everywhere we > could. The patch #8 which relinquish memory doesn't give the right mfn in parameters. It's because, you will need to look for each gfn the translated mfn (you can't assumed the mfn are contiguous). So the loop will go back to the version 3. > >> + >> + /* TODO: Handle other p2m type */ >> + if ( pte.p2m.valid && p2m_is_foreign(pte.p2m.type) ) > > How useful is p2m_is_foreign now that this stuff doesn't need to be in > common code? I'm not sure to understand. Do you mean the macro? If so it's only for convenience. > Both of the above are really just observations rather than requests for > change, so: > Acked-by: Ian Campbell <ian.campbell@citrix.com> Thanks,
On Tue, 2013-12-17 at 15:06 +0000, Julien Grall wrote: > On 12/17/2013 11:18 AM, Ian Campbell wrote: > > On Mon, 2013-12-16 at 17:37 +0000, Julien Grall wrote: > >> Modify get_page_from_gfn to take reference on foreign mapping. This will avoid > >> specific handling in the common code. > >> > >> Signed-off-by: Julien Grall <julien.grall@linaro.org> > >> > >> --- > >> Changes in v4.2: > >> - get_page_from_gfn: move foreign checking before get_page > >> - add assert fdom != dom > >> Changes in v4.1: > >> - Remove specific p2m handling in common code > >> - Handle foreign mapping in get_page_from_gfn > >> Changes in v4: > >> - Split patch #6 from dom0 pvh series v6.2 to retrieve only common > >> code. > >> - Rework commit title > >> - Rename xen_rem_foreign_from_p2m in p2m_remove_foreign > >> - Get the mfn from the pte. We are not sure that maddr given in > >> parameters is valid > >> Changes in v3: > >> - Move put_page in create_p2m_entries > >> - Move xenmem_rem_foreign_from_p2m in arch/arm/p2m.c > >> Changes in v2: > >> - Introduce the patch > >> --- > >> xen/arch/arm/p2m.c | 15 +++++++++++++-- > >> xen/include/asm-arm/p2m.h | 12 ++++++++++++ > >> 2 files changed, 25 insertions(+), 2 deletions(-) > >> > >> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > >> index 9bdcacd..d05fdff 100644 > >> --- a/xen/arch/arm/p2m.c > >> +++ b/xen/arch/arm/p2m.c > >> @@ -317,10 +317,21 @@ static int create_p2m_entries(struct domain *d, > >> break; > >> case REMOVE: > >> { > >> - lpae_t pte; > >> + lpae_t pte = third[third_table_offset(addr)]; > >> + unsigned long mfn; > >> + > >> + maddr = (pte.bits & PADDR_MASK & PAGE_MASK); > >> + mfn = paddr_to_pfn(maddr); > > > > FWIW mfn = pte.p2m.base, I think. I'm not sure we use this everywhere we > > could. > > The patch #8 which relinquish memory doesn't give the right mfn in > parameters. > It's because, you will need to look for each gfn the translated mfn (you > can't assumed the mfn are contiguous). So the loop will go back to the > version 3. I'm not sure what you mean. All I was trying to say is that: mfn = paddr_to_pfn(pte.bits & PADDR_MASK & PAGE_MASK) is (probably) equivalent to: mfn = pte.p2m.base > >> + > >> + /* TODO: Handle other p2m type */ > >> + if ( pte.p2m.valid && p2m_is_foreign(pte.p2m.type) ) > > > > How useful is p2m_is_foreign now that this stuff doesn't need to be in > > common code? > > I'm not sure to understand. Do you mean the macro? Yes > If so it's only for convenience. Convenience is fair enoguh, especially once I noticed p2m_is_ram etc. > > > Both of the above are really just observations rather than requests for > > change, so: > > Acked-by: Ian Campbell <ian.campbell@citrix.com> > > Thanks, >
On 12/17/2013 03:21 PM, Ian Campbell wrote: > On Tue, 2013-12-17 at 15:06 +0000, Julien Grall wrote: >> On 12/17/2013 11:18 AM, Ian Campbell wrote: >> The patch #8 which relinquish memory doesn't give the right mfn in >> parameters. >> It's because, you will need to look for each gfn the translated mfn (you >> can't assumed the mfn are contiguous). So the loop will go back to the >> version 3. > > I'm not sure what you mean. > > All I was trying to say is that: > mfn = paddr_to_pfn(pte.bits & PADDR_MASK & PAGE_MASK) > is (probably) equivalent to: > mfn = pte.p2m.base We didn't talk about the same thing. I will use pte.p2m.base for the next version.
On Tue, 2013-12-17 at 15:44 +0000, Julien Grall wrote: > On 12/17/2013 03:21 PM, Ian Campbell wrote: > > On Tue, 2013-12-17 at 15:06 +0000, Julien Grall wrote: > >> On 12/17/2013 11:18 AM, Ian Campbell wrote: > >> The patch #8 which relinquish memory doesn't give the right mfn in > >> parameters. > >> It's because, you will need to look for each gfn the translated mfn (you > >> can't assumed the mfn are contiguous). So the loop will go back to the > >> version 3. > > > > I'm not sure what you mean. > > > > All I was trying to say is that: > > mfn = paddr_to_pfn(pte.bits & PADDR_MASK & PAGE_MASK) > > is (probably) equivalent to: > > mfn = pte.p2m.base > > We didn't talk about the same thing. > > I will use pte.p2m.base for the next version. Make sure to check that I am right about them being the same ;-) Ian.
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 9bdcacd..d05fdff 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -317,10 +317,21 @@ static int create_p2m_entries(struct domain *d, break; case REMOVE: { - lpae_t pte; + lpae_t pte = third[third_table_offset(addr)]; + unsigned long mfn; + + maddr = (pte.bits & PADDR_MASK & PAGE_MASK); + mfn = paddr_to_pfn(maddr); + + /* TODO: Handle other p2m type */ + if ( pte.p2m.valid && p2m_is_foreign(pte.p2m.type) ) + { + ASSERT(mfn_valid(mfn)); + put_page(mfn_to_page(mfn)); + } + memset(&pte, 0x00, sizeof(pte)); write_pte(&third[third_table_offset(addr)], pte); - maddr += PAGE_SIZE; } break; } diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h index 0eb07a8..5ccfa7f 100644 --- a/xen/include/asm-arm/p2m.h +++ b/xen/include/asm-arm/p2m.h @@ -122,6 +122,18 @@ static inline struct page_info *get_page_from_gfn( if ( !mfn_valid(mfn) ) return NULL; page = mfn_to_page(mfn); + + /* get_page won't work on foreign mapping because the page doesn't + * belong to the current domain. + */ + if ( p2mt == p2m_map_foreign ) + { + struct domain *fdom = page_get_owner_and_reference(page); + ASSERT(fdom != NULL); + ASSERT(fdom != d); + return page; + } + if ( !get_page(page, d) ) return NULL; return page;
Modify get_page_from_gfn to take reference on foreign mapping. This will avoid specific handling in the common code. Signed-off-by: Julien Grall <julien.grall@linaro.org> --- Changes in v4.2: - get_page_from_gfn: move foreign checking before get_page - add assert fdom != dom Changes in v4.1: - Remove specific p2m handling in common code - Handle foreign mapping in get_page_from_gfn Changes in v4: - Split patch #6 from dom0 pvh series v6.2 to retrieve only common code. - Rework commit title - Rename xen_rem_foreign_from_p2m in p2m_remove_foreign - Get the mfn from the pte. We are not sure that maddr given in parameters is valid Changes in v3: - Move put_page in create_p2m_entries - Move xenmem_rem_foreign_from_p2m in arch/arm/p2m.c Changes in v2: - Introduce the patch --- xen/arch/arm/p2m.c | 15 +++++++++++++-- xen/include/asm-arm/p2m.h | 12 ++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-)