|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 06/14] xen: Convert hotplug page function to use typesafe MFN
On Tue, 7 May 2019, Julien Grall wrote:
> Convert online_page, offline_page and query_page_offline to use
> typesafe MFN.
I would like to have a statement in the commit message mentioning the
changes below to mci_action_add_pageoffline and mc_memerr_dhandler.
From an ARM point of view, it is fine.
> Note, for clarity, the words have been re-ordered in the error message
> updated by this patch.
>
> No functional changes.
>
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> ---
> Changes:
> - Update error message
> - Add Jan's acked-by
> ---
> 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(-)
>
> 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 be44158033..f445f7daec 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 out of range page %"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);
> --
> 2.11.0
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |