| Message ID | 1386560047-17500-7-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, 2013-12-09 at 03:34 +0000, Julien Grall wrote: > Signed-off-by: Julien Grall <julien.grall@linaro.org> > > --- > Changes in v2: > - Use p2m_lookup as p2m_get_entry was removed > --- > xen/include/asm-arm/p2m.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h > index f4bcd7d..b63204d 100644 > --- a/xen/include/asm-arm/p2m.h > +++ b/xen/include/asm-arm/p2m.h > @@ -109,7 +109,8 @@ static inline struct page_info *get_page_from_gfn( > struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q) > { > struct page_info *page; > - unsigned long mfn = gmfn_to_mfn(d, gfn); > + paddr_t maddr = p2m_lookup(d, pfn_to_paddr(gfn), t); > + unsigned long mfn = maddr >> PAGE_SHIFT; I think this resend happened before I replied to the original a second time, (in <1386583029.13126.13.camel@kazak.uk.xensource.com>) so you hadn't seen it. TL;DR: INVALID_MADDR >> PAGE_SHIFT != INVALID_MFN on arm64. Ian.
On 12/09/2013 04:06 PM, Ian Campbell wrote: > On Mon, 2013-12-09 at 03:34 +0000, Julien Grall wrote: >> Signed-off-by: Julien Grall <julien.grall@linaro.org> >> >> --- >> Changes in v2: >> - Use p2m_lookup as p2m_get_entry was removed >> --- >> xen/include/asm-arm/p2m.h | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h >> index f4bcd7d..b63204d 100644 >> --- a/xen/include/asm-arm/p2m.h >> +++ b/xen/include/asm-arm/p2m.h >> @@ -109,7 +109,8 @@ static inline struct page_info *get_page_from_gfn( >> struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q) >> { >> struct page_info *page; >> - unsigned long mfn = gmfn_to_mfn(d, gfn); >> + paddr_t maddr = p2m_lookup(d, pfn_to_paddr(gfn), t); >> + unsigned long mfn = maddr >> PAGE_SHIFT; > > I think this resend happened before I replied to the original a second > time, (in <1386583029.13126.13.camel@kazak.uk.xensource.com>) so you > hadn't seen it. Indeed, I will fix it. > TL;DR: INVALID_MADDR >> PAGE_SHIFT != INVALID_MFN on arm64. Does it mean that INVALID_MFN can be a valid mfn on arm64? If so, we have some place in common code where this constant is used (see common/domain.c for instance).
On Mon, 2013-12-09 at 16:50 +0000, Julien Grall wrote: > On 12/09/2013 04:06 PM, Ian Campbell wrote: > > On Mon, 2013-12-09 at 03:34 +0000, Julien Grall wrote: > >> Signed-off-by: Julien Grall <julien.grall@linaro.org> > >> > >> --- > >> Changes in v2: > >> - Use p2m_lookup as p2m_get_entry was removed > >> --- > >> xen/include/asm-arm/p2m.h | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h > >> index f4bcd7d..b63204d 100644 > >> --- a/xen/include/asm-arm/p2m.h > >> +++ b/xen/include/asm-arm/p2m.h > >> @@ -109,7 +109,8 @@ static inline struct page_info *get_page_from_gfn( > >> struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q) > >> { > >> struct page_info *page; > >> - unsigned long mfn = gmfn_to_mfn(d, gfn); > >> + paddr_t maddr = p2m_lookup(d, pfn_to_paddr(gfn), t); > >> + unsigned long mfn = maddr >> PAGE_SHIFT; > > > > I think this resend happened before I replied to the original a second > > time, (in <1386583029.13126.13.camel@kazak.uk.xensource.com>) so you > > hadn't seen it. > > Indeed, I will fix it. > > > TL;DR: INVALID_MADDR >> PAGE_SHIFT != INVALID_MFN on arm64. > > Does it mean that INVALID_MFN can be a valid mfn on arm64? If so, we > have some place in common code where this constant is used (see > common/domain.c for instance). No, INVALID_MFN is 0xffffffffffffffff which is fine[0]. But given INVALID_MADDR of 0xffffffffffffffff, (INVALID_MADDR>>PAGE_SHIFT) is 0x000fffffffffffff. Ian. [0] actually, the page at -1 could in theory be valid on any platform, either arm32 or x86, bit a) it's unlikely and b) there isn't really any better value to use as this sentinal. Lets not worry about it right now.
On 12/09/2013 04:58 PM, Ian Campbell wrote: > On Mon, 2013-12-09 at 16:50 +0000, Julien Grall wrote: >> On 12/09/2013 04:06 PM, Ian Campbell wrote: >>> On Mon, 2013-12-09 at 03:34 +0000, Julien Grall wrote: >>>> Signed-off-by: Julien Grall <julien.grall@linaro.org> >>>> >>>> --- >>>> Changes in v2: >>>> - Use p2m_lookup as p2m_get_entry was removed >>>> --- >>>> xen/include/asm-arm/p2m.h | 3 ++- >>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h >>>> index f4bcd7d..b63204d 100644 >>>> --- a/xen/include/asm-arm/p2m.h >>>> +++ b/xen/include/asm-arm/p2m.h >>>> @@ -109,7 +109,8 @@ static inline struct page_info *get_page_from_gfn( >>>> struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q) >>>> { >>>> struct page_info *page; >>>> - unsigned long mfn = gmfn_to_mfn(d, gfn); >>>> + paddr_t maddr = p2m_lookup(d, pfn_to_paddr(gfn), t); >>>> + unsigned long mfn = maddr >> PAGE_SHIFT; >>> >>> I think this resend happened before I replied to the original a second >>> time, (in <1386583029.13126.13.camel@kazak.uk.xensource.com>) so you >>> hadn't seen it. >> >> Indeed, I will fix it. >> >>> TL;DR: INVALID_MADDR >> PAGE_SHIFT != INVALID_MFN on arm64. >> >> Does it mean that INVALID_MFN can be a valid mfn on arm64? If so, we >> have some place in common code where this constant is used (see >> common/domain.c for instance). > > No, INVALID_MFN is 0xffffffffffffffff which is fine[0]. > > But given INVALID_MADDR of 0xffffffffffffffff, > (INVALID_MADDR>>PAGE_SHIFT) is 0x000fffffffffffff. oh right, I didn't pay attention for that. The patch is now fixed.
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h index f4bcd7d..b63204d 100644 --- a/xen/include/asm-arm/p2m.h +++ b/xen/include/asm-arm/p2m.h @@ -109,7 +109,8 @@ static inline struct page_info *get_page_from_gfn( struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q) { struct page_info *page; - unsigned long mfn = gmfn_to_mfn(d, gfn); + paddr_t maddr = p2m_lookup(d, pfn_to_paddr(gfn), t); + unsigned long mfn = maddr >> PAGE_SHIFT; if (!mfn_valid(mfn)) return NULL;
Signed-off-by: Julien Grall <julien.grall@linaro.org> --- Changes in v2: - Use p2m_lookup as p2m_get_entry was removed --- xen/include/asm-arm/p2m.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)