diff mbox series

[Xen-devel,for-next,5/9] xen: Convert hotplug page function to use typesafe MFN

Message ID 20190218113600.9540-6-julien.grall@arm.com
State Superseded
Headers show
Series xen/arm: Properly disable M2P on Arm. | expand

Commit Message

Julien Grall Feb. 18, 2019, 11:35 a.m. UTC 
Convert online_page, offline_page and query_page_offline to use
typesafe MFN.

No functional changes.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/x86/cpu/mcheck/mcaction.c | 18 ++++++++++--------
 xen/common/page_alloc.c            | 24 ++++++++++++------------
 xen/common/sysctl.c                | 14 +++++++-------
 xen/include/xen/mm.h               |  6 +++---
 4 files changed, 32 insertions(+), 30 deletions(-)

Comments

Jan Beulich March 13, 2019, 2:57 p.m. UTC | #1 
>>> On 18.02.19 at 12:35, <julien.grall@arm.com> wrote:
> Convert online_page, offline_page and query_page_offline to use
> typesafe MFN.
> 
> No functional changes.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
But ...

> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -1568,23 +1568,23 @@ static int reserve_heap_page(struct page_info *pg)
>  
>  }
>  
> -int offline_page(unsigned long mfn, int broken, uint32_t *status)
> +int offline_page(mfn_t mfn, int broken, uint32_t *status)
>  {
>      unsigned long old_info = 0;
>      struct domain *owner;
>      struct page_info *pg;
>  
> -    if ( !mfn_valid(_mfn(mfn)) )
> +    if ( !mfn_valid(mfn) )
>      {
>          dprintk(XENLOG_WARNING,
> -                "try to offline page out of range %lx\n", mfn);
> +                "try to offline page out of range %"PRI_mfn"\n", mfn_x(mfn));

... would you mind adjusting the wording here as well:
"attempt to offline out of range page %"PRI_mfn"\n" or
some such?

Of course the usefulness of the message as a whole is
questionable, so I'd also be happy to see it deleted altogether.

Jan
Julien Grall March 13, 2019, 4:26 p.m. UTC | #2 
Hi,

On 13/03/2019 14:57, Jan Beulich wrote:
>>>> On 18.02.19 at 12:35, <julien.grall@arm.com> wrote:
>> Convert online_page, offline_page and query_page_offline to use
>> typesafe MFN.
>>
>> No functional changes.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>
> But ...
> 
>> --- a/xen/common/page_alloc.c
>> +++ b/xen/common/page_alloc.c
>> @@ -1568,23 +1568,23 @@ static int reserve_heap_page(struct page_info *pg)
>>   
>>   }
>>   
>> -int offline_page(unsigned long mfn, int broken, uint32_t *status)
>> +int offline_page(mfn_t mfn, int broken, uint32_t *status)
>>   {
>>       unsigned long old_info = 0;
>>       struct domain *owner;
>>       struct page_info *pg;
>>   
>> -    if ( !mfn_valid(_mfn(mfn)) )
>> +    if ( !mfn_valid(mfn) )
>>       {
>>           dprintk(XENLOG_WARNING,
>> -                "try to offline page out of range %lx\n", mfn);
>> +                "try to offline page out of range %"PRI_mfn"\n", mfn_x(mfn));
> 
> ... would you mind adjusting the wording here as well:
> "attempt to offline out of range page %"PRI_mfn"\n" or > some such?

Sure.

> 
> Of course the usefulness of the message as a whole is
> questionable, so I'd also be happy to see it deleted altogether.

While I am happy to do formatting, I would rather leave code removal in separate 
patch.

Cheers,
diff mbox series

Patch

diff --git a/xen/arch/x86/cpu/mcheck/mcaction.c b/xen/arch/x86/cpu/mcheck/mcaction.c
index e42267414e..69332fb84d 100644
--- a/xen/arch/x86/cpu/mcheck/mcaction.c
+++ b/xen/arch/x86/cpu/mcheck/mcaction.c
@@ -6,7 +6,7 @@ 
 
 static struct mcinfo_recovery *
 mci_action_add_pageoffline(int bank, struct mc_info *mi,
-                           uint64_t mfn, uint32_t status)
+                           mfn_t mfn, uint32_t status)
 {
     struct mcinfo_recovery *rec;
 
@@ -22,7 +22,7 @@  mci_action_add_pageoffline(int bank, struct mc_info *mi,
 
     rec->mc_bank = bank;
     rec->action_types = MC_ACTION_PAGE_OFFLINE;
-    rec->action_info.page_retire.mfn = mfn;
+    rec->action_info.page_retire.mfn = mfn_x(mfn);
     rec->action_info.page_retire.status = status;
     return rec;
 }
@@ -42,7 +42,8 @@  mc_memerr_dhandler(struct mca_binfo *binfo,
     struct mcinfo_bank *bank = binfo->mib;
     struct mcinfo_global *global = binfo->mig;
     struct domain *d;
-    unsigned long mfn, gfn;
+    mfn_t mfn;
+    unsigned long gfn;
     uint32_t status;
     int vmce_vcpuid;
     unsigned int mc_vcpuid;
@@ -54,11 +55,12 @@  mc_memerr_dhandler(struct mca_binfo *binfo,
         return;
     }
 
-    mfn = bank->mc_addr >> PAGE_SHIFT;
+    mfn = maddr_to_mfn(bank->mc_addr);
     if ( offline_page(mfn, 1, &status) )
     {
         dprintk(XENLOG_WARNING,
-                "Failed to offline page %lx for MCE error\n", mfn);
+                "Failed to offline page %"PRI_mfn" for MCE error\n",
+                mfn_x(mfn));
         return;
     }
 
@@ -89,10 +91,10 @@  mc_memerr_dhandler(struct mca_binfo *binfo,
                 ASSERT(d);
                 gfn = get_gpfn_from_mfn((bank->mc_addr) >> PAGE_SHIFT);
 
-                if ( unmmap_broken_page(d, _mfn(mfn), gfn) )
+                if ( unmmap_broken_page(d, mfn, gfn) )
                 {
-                    printk("Unmap broken memory %lx for DOM%d failed\n",
-                           mfn, d->domain_id);
+                    printk("Unmap broken memory %"PRI_mfn" for DOM%d failed\n",
+                           mfn_x(mfn), d->domain_id);
                     goto vmce_failed;
                 }
 
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index f71d3bb7a1..5684a13557 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -1568,23 +1568,23 @@  static int reserve_heap_page(struct page_info *pg)
 
 }
 
-int offline_page(unsigned long mfn, int broken, uint32_t *status)
+int offline_page(mfn_t mfn, int broken, uint32_t *status)
 {
     unsigned long old_info = 0;
     struct domain *owner;
     struct page_info *pg;
 
-    if ( !mfn_valid(_mfn(mfn)) )
+    if ( !mfn_valid(mfn) )
     {
         dprintk(XENLOG_WARNING,
-                "try to offline page out of range %lx\n", mfn);
+                "try to offline page out of range %"PRI_mfn"\n", mfn_x(mfn));
         return -EINVAL;
     }
 
     *status = 0;
-    pg = mfn_to_page(_mfn(mfn));
+    pg = mfn_to_page(mfn);
 
-    if ( is_xen_fixed_mfn(mfn) )
+    if ( is_xen_fixed_mfn(mfn_x(mfn)) )
     {
         *status = PG_OFFLINE_XENPAGE | PG_OFFLINE_FAILED |
           (DOMID_XEN << PG_OFFLINE_OWNER_SHIFT);
@@ -1595,7 +1595,7 @@  int offline_page(unsigned long mfn, int broken, uint32_t *status)
      * N.B. xen's txt in x86_64 is marked reserved and handled already.
      * Also kexec range is reserved.
      */
-    if ( !page_is_ram_type(mfn, RAM_TYPE_CONVENTIONAL) )
+    if ( !page_is_ram_type(mfn_x(mfn), RAM_TYPE_CONVENTIONAL) )
     {
         *status = PG_OFFLINE_FAILED | PG_OFFLINE_NOT_CONV_RAM;
         return -EINVAL;
@@ -1677,19 +1677,19 @@  int offline_page(unsigned long mfn, int broken, uint32_t *status)
  *   The caller should make sure end_pfn <= max_page,
  *   if not, expand_pages() should be called prior to online_page().
  */
-unsigned int online_page(unsigned long mfn, uint32_t *status)
+unsigned int online_page(mfn_t mfn, uint32_t *status)
 {
     unsigned long x, nx, y;
     struct page_info *pg;
     int ret;
 
-    if ( !mfn_valid(_mfn(mfn)) )
+    if ( !mfn_valid(mfn) )
     {
         dprintk(XENLOG_WARNING, "call expand_pages() first\n");
         return -EINVAL;
     }
 
-    pg = mfn_to_page(_mfn(mfn));
+    pg = mfn_to_page(mfn);
 
     spin_lock(&heap_lock);
 
@@ -1730,11 +1730,11 @@  unsigned int online_page(unsigned long mfn, uint32_t *status)
     return ret;
 }
 
-int query_page_offline(unsigned long mfn, uint32_t *status)
+int query_page_offline(mfn_t mfn, uint32_t *status)
 {
     struct page_info *pg;
 
-    if ( !mfn_valid(_mfn(mfn)) || !page_is_ram_type(mfn, RAM_TYPE_CONVENTIONAL) )
+    if ( !mfn_valid(mfn) || !page_is_ram_type(mfn_x(mfn), RAM_TYPE_CONVENTIONAL) )
     {
         dprintk(XENLOG_WARNING, "call expand_pages() first\n");
         return -EINVAL;
@@ -1743,7 +1743,7 @@  int query_page_offline(unsigned long mfn, uint32_t *status)
     *status = 0;
     spin_lock(&heap_lock);
 
-    pg = mfn_to_page(_mfn(mfn));
+    pg = mfn_to_page(mfn);
 
     if ( page_state_is(pg, offlining) )
         *status |= PG_OFFLINE_STATUS_OFFLINE_PENDING;
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index c0aa6bde4e..ab161793e5 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -186,7 +186,7 @@  long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
     case XEN_SYSCTL_page_offline_op:
     {
         uint32_t *status, *ptr;
-        unsigned long pfn;
+        mfn_t mfn;
 
         ret = xsm_page_offline(XSM_HOOK, op->u.page_offline.cmd);
         if ( ret )
@@ -205,21 +205,21 @@  long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
         memset(status, PG_OFFLINE_INVALID, sizeof(uint32_t) *
                       (op->u.page_offline.end - op->u.page_offline.start + 1));
 
-        for ( pfn = op->u.page_offline.start;
-              pfn <= op->u.page_offline.end;
-              pfn ++ )
+        for ( mfn = _mfn(op->u.page_offline.start);
+              mfn_x(mfn) <= op->u.page_offline.end;
+              mfn = mfn_add(mfn, 1) )
         {
             switch ( op->u.page_offline.cmd )
             {
                 /* Shall revert her if failed, or leave caller do it? */
                 case sysctl_page_offline:
-                    ret = offline_page(pfn, 0, ptr++);
+                    ret = offline_page(mfn, 0, ptr++);
                     break;
                 case sysctl_page_online:
-                    ret = online_page(pfn, ptr++);
+                    ret = online_page(mfn, ptr++);
                     break;
                 case sysctl_query_page_offline:
-                    ret = query_page_offline(pfn, ptr++);
+                    ret = query_page_offline(mfn, ptr++);
                     break;
                 default:
                     ret = -EINVAL;
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index e971147234..3ba7168cc9 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -206,9 +206,9 @@  unsigned long avail_domheap_pages(void);
 unsigned long avail_node_heap_pages(unsigned int);
 #define alloc_domheap_page(d,f) (alloc_domheap_pages(d,0,f))
 #define free_domheap_page(p)  (free_domheap_pages(p,0))
-unsigned int online_page(unsigned long mfn, uint32_t *status);
-int offline_page(unsigned long mfn, int broken, uint32_t *status);
-int query_page_offline(unsigned long mfn, uint32_t *status);
+unsigned int online_page(mfn_t mfn, uint32_t *status);
+int offline_page(mfn_t mfn, int broken, uint32_t *status);
+int query_page_offline(mfn_t mfn, uint32_t *status);
 unsigned long total_free_pages(void);
 
 void heap_init_late(void);