Re: [Xen-devel] [PATCH v3] x86/arm/mm: use gfn instead of pfn in p2m_get_mem_access/p2m_set_mem_access

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.06.15 at 17:17, <vkuznets@xxxxxxxxxx> 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.

This is insufficient.

> ---
>  xen/arch/arm/p2m.c           | 12 ++++++------
>  xen/arch/x86/mm/p2m.c        | 24 ++++++++++++------------
>  xen/include/xen/p2m-common.h | 12 ++++++------

Please make sure you Cc maintainers (the x86/mm one recently
changed; now added).

Patch looks okay to me now.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel