[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3] x86/arm/mm: use gfn instead of pfn in p2m_get_mem_access/p2m_set_mem_access
On 26/06/15 16:17, Vitaly Kuznetsov wrote: > 'pfn' and 'start_pfn' are ambiguous, both these functions expect GFNs as > input. > > On x86 the interface of p2m_set_mem_access() in p2m.c doesn't match the > declaration in p2m-common.h as 'pfn' is being used instead of 'start_pfn'. > > On ARM both p2m_set_mem_access and p2m_get_mem_access interfaces don't match > declarations from p2m-common.h: p2m_set_mem_access uses 'pfn' instead of > 'start_pfn' and p2m_get_mem_access uses 'gpfn' instead of 'pfn'. > > There is also an issue in p2m_get_mem_access on x86: 'gfn' parameter passed to > gfn_lock/gfn_unlock is not defined. This code compiles only because of a > coincidence: gfn_lock/gfn_unlock are currently macros which don't use their > second argument. > > Signed-off-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> > --- > Changes since v2: > - Instead of adding start_ prefix on ARM remove it on x86 [Jan Beulich, > Ian Campbell, Razvan Cojocaru] > > Changes since v1: > - This patch is a successor of '[PATCH] x86/mm: use existing 'pfn' in > p2m_get_mem_access', instead of fixing gfn_lock/gfn_unlock arguments we do > s/pfn/gfn/g for both p2m_get_mem_access/p2m_set_mem_access [Andrew Cooper, > Jan Beulich] > > P.S. > - The patch was compile-tested on x86 only. > --- > xen/arch/arm/p2m.c | 12 ++++++------ > xen/arch/x86/mm/p2m.c | 24 ++++++++++++------------ > xen/include/xen/p2m-common.h | 12 ++++++------ > 3 files changed, 24 insertions(+), 24 deletions(-) > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > index 903fa3f..54c238c 100644 > --- a/xen/arch/arm/p2m.c > +++ b/xen/arch/arm/p2m.c > @@ -1709,9 +1709,9 @@ bool_t p2m_mem_access_check(paddr_t gpa, vaddr_t gla, > const struct npfec npfec) > > /* > * Set access type for a region of pfns. > - * If start_pfn == -1ul, sets the default access type. > + * If gfn == -1ul, sets the default access type. > */ > -long p2m_set_mem_access(struct domain *d, unsigned long pfn, uint32_t nr, > +long p2m_set_mem_access(struct domain *d, unsigned long gfn, uint32_t nr, > uint32_t start, uint32_t mask, xenmem_access_t > access) > { > struct p2m_domain *p2m = p2m_get_hostp2m(d); > @@ -1752,14 +1752,14 @@ long p2m_set_mem_access(struct domain *d, unsigned > long pfn, uint32_t nr, > p2m->mem_access_enabled = true; > > /* If request to set default access. */ > - if ( pfn == ~0ul ) > + if ( gfn == ~0ul ) Please here and everywhere else in the patch, use INVALID_GFN instead of ~0 or -1. With those changes and the comment style from Razvan, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> However, if you feel up to it, it would be fantastic if you could substitute unsigned long for gfn_t, per the justification in e758ed1 and 177bd5f. c/s 24036a5 is an example of a different API I fixed up in a similar way. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |