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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.03.17 at 09:10, <vijay.kilari@xxxxxxxxx> wrote:
> @@ -254,7 +262,6 @@ static inline int gvirt_to_maddr(vaddr_t va, paddr_t *pa, 
> unsigned int flags)
>  #define virt_to_mfn(va)   (virt_to_maddr(va) >> PAGE_SHIFT)
>  #define mfn_to_virt(mfn)  (maddr_to_virt((paddr_t)(mfn) << PAGE_SHIFT))
>  
> -
>  /* Convert between Xen-heap virtual addresses and page-info structures. */

If I was an ARM maintainer, I'd object to such a stray change (even
if generally it looks good to me to remove double blank lines).

> @@ -374,6 +375,17 @@ perms_strictly_increased(uint32_t old_flags, uint32_t 
> new_flags)
>      return ((of | (of ^ nf)) == nf);
>  }
>  
> +/*
> + * x86 maps DIRECTMAP_VIRT to physical memory. Get the mfn for directmap
> + * memory region.
> + */
> +static inline bool_t arch_mfn_below_directmap_max_mfn(unsigned long mfn)

I'm pretty convinced it has been pointed out to you that we use
plain bool nowadays. Also the function name looks overly long to
me. How about arch_mfn_in_directmap()?

> +{
> +    unsigned long eva = min(DIRECTMAP_VIRT_END, HYPERVISOR_VIRT_END);
> +
> +    return (mfn <= (virt_to_mfn(eva - 1) + 1)) ? true : false;

There's absolutely no need for conditional expressions like this. The
result of the comparison is fine as is for a function with a boolean
result (and that was already the case back when we were still using
bool_t).

Jan


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