Re: [Xen-devel] [PATCH v4] boot allocator: Use arch helper for virt_to_mfn on DIRECTMAP

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.03.17 at 14:35, <vijay.kilari@xxxxxxxxx> wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxx>
> 
> On ARM64, virt_to_mfn uses the hardware for address
> translation. So if the virtual address is not mapped translation
> fault is raised. On ARM64, DIRECTMAP_VIRT region is direct mapped.
> 
> On ARM platforms with NUMA, While initializing second memory node,
> panic is triggered from init_node_heap() when virt_to_mfn()
> is called for DIRECTMAP_VIRT region address.
> Here the check is made to ensure that MFN less than max MFN mapped.
> The max MFN is found by calling virt_to_mfn of DIRECTMAP_VIRT_END
> region. Since DIRECMAP_VIRT region is not mapped to any virtual address
> on ARM, it fails.
> 
> In this patch, instead of calling virt_to_mfn(), arch helper
> arch_mfn_in_directmap() is introduced. On ARM64 this arch helper
> will return true, whereas on ARM DIRECTMAP_VIRT region is not directly 
> mapped
> only xenheap region is directly mapped. So on ARM return false always.
> For x86 this helper does virt_to_mfn.
> 
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxx>

Non-ARM parts
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
with two remarks:

> ---

Please get used to add a brief change summary here.

> --- a/xen/include/asm-x86/mm.h
> +++ b/xen/include/asm-x86/mm.h
> @@ -602,4 +602,15 @@ extern const char zero_page[];
>  /* Build a 32bit PSE page table using 4MB pages. */
>  void write_32bit_pse_identmap(uint32_t *l2);
>  
> +/*
> + * x86 maps DIRECTMAP_VIRT to physical memory. Get the mfn for directmap
> + * memory region.

The comment does not describe what the function does. The ARM64
one doesn't look right, either. I'd say "x86 maps part of physical
memory via the directmap region. Return whether the input MFN
falls in that range."

> + */
> +static inline bool arch_mfn_in_directmap(unsigned long mfn)
> +{
> +    unsigned long eva = min(DIRECTMAP_VIRT_END, HYPERVISOR_VIRT_END);
> +
> +    return (mfn <= (virt_to_mfn(eva - 1) + 1));

Stray parentheses (at least the outer ones).

Both issues could be taken care of while committing, unless v5
becomes necessary for another reason.

Jan


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