[PATCH] x86/mm: Alter is_xen_fixed_mfn() to use mfn_t

[PATCH] x86/mm: Alter is_xen_fixed_mfn() to use mfn_t

[Andrew Cooper]

The predicate is common between x86 and ARM, and is unlikley to be different
on other architectures.  Drop the arch declarations and introduce a common
static inline in xen/mm.h instead.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
CC: Tim Deegan <tim@xen.org>
CC: George Dunlap <george.dunlap@eu.citrix.com>
---
 xen/common/page_alloc.c  | 2 +-
 xen/include/asm-arm/mm.h | 4 ----
 xen/include/asm-x86/mm.h | 3 ---
 xen/include/xen/mm.h     | 6 ++++++
 4 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 915e1f7..e2c31fe 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -1093,7 +1093,7 @@ int offline_page(unsigned long mfn, int broken, uint32_t *status)
     *status = 0;
     pg = mfn_to_page(mfn);
 
-    if ( is_xen_fixed_mfn(mfn) )
+    if ( is_xen_fixed_mfn(_mfn(mfn)) )
     {
         *status = PG_OFFLINE_XENPAGE | PG_OFFLINE_FAILED |
           (DOMID_XEN << PG_OFFLINE_OWNER_SHIFT);
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index 19eadd2..3b73935 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -132,10 +132,6 @@ extern vaddr_t xenheap_virt_start;
     (mfn_valid(mfn) && is_xen_heap_page(__mfn_to_page(mfn)))
 #endif
 
-#define is_xen_fixed_mfn(mfn)                                   \
-    ((pfn_to_paddr(mfn) >= virt_to_maddr(&_start)) &&       \
-     (pfn_to_paddr(mfn) <= virt_to_maddr(&_end)))
-
 #define page_get_owner(_p)    (_p)->v.inuse.domain
 #define page_set_owner(_p,_d) ((_p)->v.inuse.domain = (_d))
 
diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h
index f0efacb..f4de840 100644
--- a/xen/include/asm-x86/mm.h
+++ b/xen/include/asm-x86/mm.h
@@ -252,9 +252,6 @@ struct spage_info
 #define is_xen_heap_page(page) ((page)->count_info & PGC_xen_heap)
 #define is_xen_heap_mfn(mfn) \
     (__mfn_valid(mfn) && is_xen_heap_page(__mfn_to_page(mfn)))
-#define is_xen_fixed_mfn(mfn)                     \
-    ((((mfn) << PAGE_SHIFT) >= __pa(&_start)) &&  \
-     (((mfn) << PAGE_SHIFT) <= __pa(&_end)))
 
 #define PRtype_info "016lx"/* should only be used for printk's */
 
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 88de3c1..7a2b0d0 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -599,4 +599,10 @@ static inline void filtered_flush_tlb_mask(uint32_t tlbflush_timestamp)
     }
 }
 
+static inline bool is_xen_fixed_mfn(mfn_t mfn)
+{
+    return (pfn_to_paddr(mfn_x(mfn)) >= __pa(&_start) &&
+            pfn_to_paddr(mfn_x(mfn)) <= __pa(&_end));
+}
+
 #endif /* __XEN_MM_H__ */
-- 
2.1.4


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

Re: [PATCH] x86/mm: Alter is_xen_fixed_mfn() to use mfn_t

[Jan Beulich]

>>> On 07.02.17 at 19:57, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/include/xen/mm.h
> +++ b/xen/include/xen/mm.h
> @@ -599,4 +599,10 @@ static inline void filtered_flush_tlb_mask(uint32_t tlbflush_timestamp)
>      }
>  }
>  
> +static inline bool is_xen_fixed_mfn(mfn_t mfn)
> +{
> +    return (pfn_to_paddr(mfn_x(mfn)) >= __pa(&_start) &&
> +            pfn_to_paddr(mfn_x(mfn)) <= __pa(&_end));
> +}

Even if you only morph what's been there before, I do think the
latter wants to be < instead of <= . With that adjusted
Reviewed-by: Jan Beulich <jbeulich@suse.com>
but I'm also unconvinced it wouldn't be better to compare PFNs
instead of physical addresses here.

Jan


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

Re: [PATCH] x86/mm: Alter is_xen_fixed_mfn() to use mfn_t

[George Dunlap]

On 08/02/17 13:07, Jan Beulich wrote:
>>>> On 07.02.17 at 19:57, <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/include/xen/mm.h
>> +++ b/xen/include/xen/mm.h
>> @@ -599,4 +599,10 @@ static inline void filtered_flush_tlb_mask(uint32_t tlbflush_timestamp)
>>      }
>>  }
>>  
>> +static inline bool is_xen_fixed_mfn(mfn_t mfn)
>> +{
>> +    return (pfn_to_paddr(mfn_x(mfn)) >= __pa(&_start) &&
>> +            pfn_to_paddr(mfn_x(mfn)) <= __pa(&_end));
>> +}
> 
> Even if you only morph what's been there before, I do think the
> latter wants to be < instead of <= . With that adjusted
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> but I'm also unconvinced it wouldn't be better to compare PFNs
> instead of physical addresses here.

Not positive this falls under my remit but just in case:

Acked-by: George Dunlap <george.dunlap@citrix.com>

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

Re: [PATCH] x86/mm: Alter is_xen_fixed_mfn() to use mfn_t

[Julien Grall]

Hi Andrew,

On 02/07/2017 06:57 PM, Andrew Cooper wrote:
> The predicate is common between x86 and ARM, and is unlikley to be different/

NIT: s/unlikley/unlikely/

> on other architectures.  Drop the arch declarations and introduce a common
> static inline in xen/mm.h instead.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Julien Grall <julien.grall@arm.com>

Regards,

-- 
Julien Grall

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