From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Ben Catterall <Ben.Catterall@citrix.com>, xen-devel@lists.xenproject.org
Cc: keir@xen.org, ian.campbell@citrix.com, tim@xen.org,
ian.jackson@eu.citrix.com, stefano.stabellini@citrix.com,
jbeulich@suse.com
Subject: Re: [PATCH 3/3] Convert map_domain_page() to use the new mfn_t type
Date: Wed, 1 Jul 2015 17:22:06 +0100 [thread overview]
Message-ID: <559413AE.4020405@citrix.com> (raw)
In-Reply-To: <1435758095-20821-4-git-send-email-Ben.Catterall@citrix.com>
On 01/07/15 14:41, Ben Catterall wrote:
> Reworked the internals and declaration, applying (un)boxing
> where needed. Converted calls to map_domain_page() to
> provide mfn_t types, boxing where needed.
>
> Signed-off-by: Ben Catterall <Ben.Catterall@citrix.com>
> ---
> xen/arch/arm/domain_build.c | 2 +-
> xen/arch/arm/kernel.c | 2 +-
> xen/arch/arm/mm.c | 12 +++++-----
> xen/arch/arm/p2m.c | 4 ++--
> xen/arch/arm/traps.c | 4 ++--
> xen/arch/x86/debug.c | 10 ++++----
> xen/arch/x86/domain.c | 4 ++--
> xen/arch/x86/domain_build.c | 10 ++++----
> xen/arch/x86/domain_page.c | 22 ++++++++---------
> xen/arch/x86/domctl.c | 2 +-
> xen/arch/x86/mm.c | 40 +++++++++++++++----------------
> xen/arch/x86/mm/guest_walk.c | 2 +-
> xen/arch/x86/mm/hap/guest_walk.c | 2 +-
> xen/arch/x86/mm/mem_sharing.c | 4 ++--
> xen/arch/x86/mm/p2m-ept.c | 22 ++++++++---------
> xen/arch/x86/mm/p2m-pod.c | 8 +++----
> xen/arch/x86/mm/p2m-pt.c | 28 +++++++++++-----------
> xen/arch/x86/mm/p2m.c | 2 +-
> xen/arch/x86/mm/paging.c | 32 ++++++++++++-------------
> xen/arch/x86/mm/shadow/common.c | 2 +-
> xen/arch/x86/mm/shadow/multi.c | 4 ++--
> xen/arch/x86/mm/shadow/private.h | 2 +-
> xen/arch/x86/smpboot.c | 2 +-
> xen/arch/x86/tboot.c | 4 ++--
> xen/arch/x86/traps.c | 12 +++++-----
> xen/arch/x86/x86_64/mm.c | 14 +++++------
> xen/arch/x86/x86_64/traps.c | 10 ++++----
> xen/arch/x86/x86_emulate.c | 10 ++++----
> xen/common/grant_table.c | 4 ++--
> xen/common/kexec.c | 4 ++--
> xen/common/kimage.c | 10 ++++----
> xen/common/memory.c | 6 ++---
> xen/common/tmem_xen.c | 6 ++---
> xen/drivers/passthrough/amd/iommu_guest.c | 10 ++++----
> xen/drivers/passthrough/amd/iommu_map.c | 14 +++++------
> xen/drivers/passthrough/vtd/x86/vtd.c | 2 +-
> xen/include/asm-x86/hap.h | 2 +-
> xen/include/asm-x86/page.h | 6 ++---
> xen/include/asm-x86/paging.h | 2 +-
> xen/include/xen/domain_page.h | 8 +++----
> 40 files changed, 173 insertions(+), 173 deletions(-)
Wow - this is a rather larger patch than I would have guessed. Good work!
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
As this is a cleanup patch, I have a few further suggestions.
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index e9cb8a9..01add40 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1322,7 +1322,7 @@ static void initrd_load(struct kernel_info *kinfo)
> return;
> }
>
> - dst = map_domain_page(ma>>PAGE_SHIFT);
> + dst = map_domain_page(_mfn(ma>>PAGE_SHIFT));
>
> copy_from_paddr(dst + s, paddr + offs, l);
>
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index 209c3dd..8b2bf9b 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -182,7 +182,7 @@ static void kernel_zimage_load(struct kernel_info *info)
> return;
> }
>
> - dst = map_domain_page(ma>>PAGE_SHIFT);
> + dst = map_domain_page(_mfn(ma>>PAGE_SHIFT));
These two hunks should use _mfn(paddr_to_mfn(ma)) rather than
open-coding paddr_to_mfn()
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 258d4c5..71c0fd9 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2263,7 +2263,7 @@ void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
> printk("Failed TTBR0 maddr lookup\n");
> goto done;
> }
> - first = map_domain_page(paddr>>PAGE_SHIFT);
> + first = map_domain_page(_mfn(paddr>>PAGE_SHIFT));
>
> offset = addr >> (12+10);
> printk("1ST[0x%"PRIx32"] (0x%"PRIpaddr") = 0x%08"PRIx32"\n",
> @@ -2279,7 +2279,7 @@ void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
> printk("Failed L1 entry maddr lookup\n");
> goto done;
> }
> - second = map_domain_page(paddr>>PAGE_SHIFT);
> + second = map_domain_page(_mfn(paddr>>PAGE_SHIFT));
Similarly, these should be _mfn(paddr_to_mfn(paddr))
> diff --git a/xen/arch/x86/mm/shadow/private.h b/xen/arch/x86/mm/shadow/private.h
> index eff39dc..31b36ef 100644
> --- a/xen/arch/x86/mm/shadow/private.h
> +++ b/xen/arch/x86/mm/shadow/private.h
> @@ -508,7 +508,7 @@ sh_mfn_is_a_page_table(mfn_t gmfn)
> static inline void *
> sh_map_domain_page(mfn_t mfn)
> {
> - return map_domain_page(mfn_x(mfn));
> + return map_domain_page(mfn);
> }
As a smaller further patch, you should drop sh_{,un}map_domain_page().
They were just wrappers to make a map_domain_page()-like-thing which
took an mfn_t.
> diff --git a/xen/arch/x86/tboot.c b/xen/arch/x86/tboot.c
> index 01b9530..af8bbf6 100644
> --- a/xen/arch/x86/tboot.c
> +++ b/xen/arch/x86/tboot.c
> @@ -161,7 +161,7 @@ static void update_iommu_mac(vmac_ctx_t *ctx, uint64_t pt_maddr, int level)
> if ( pt_maddr == 0 )
> return;
>
> - pt_vaddr = (struct dma_pte *)map_domain_page(pt_maddr >> PAGE_SHIFT_4K);
> + pt_vaddr = (struct dma_pte *)map_domain_page(_mfn(pt_maddr >> PAGE_SHIFT_4K));
Again, _mfn(paddr_to_mfn(pt_maddr)).
> diff --git a/xen/common/kimage.c b/xen/common/kimage.c
> index 742e4e8..91f7299 100644
> --- a/xen/common/kimage.c
> +++ b/xen/common/kimage.c
> @@ -495,10 +495,10 @@ static void kimage_terminate(struct kexec_image *image)
> * Call unmap_domain_page(ptr) after the loop exits.
> */
> #define for_each_kimage_entry(image, ptr, entry) \
> - for ( ptr = map_domain_page(image->head >> PAGE_SHIFT); \
> + for ( ptr = map_domain_page(_mfn(image->head >> PAGE_SHIFT)); \
> (entry = *ptr) && !(entry & IND_DONE); \
> ptr = (entry & IND_INDIRECTION) ? \
> - (unmap_domain_page(ptr), map_domain_page(entry >> PAGE_SHIFT)) \
> + (unmap_domain_page(ptr), map_domain_page(_mfn(entry >> PAGE_SHIFT))) \
> : ptr + 1 )
>
And here.
> diff --git a/xen/drivers/passthrough/amd/iommu_map.c b/xen/drivers/passthrough/amd/iommu_map.c
> index 64c5225..a4d43b2 100644
> --- a/xen/drivers/passthrough/amd/iommu_map.c
> +++ b/xen/drivers/passthrough/amd/iommu_map.c
> @@ -349,12 +349,12 @@ static int iommu_update_pde_count(struct domain *d, unsigned long pt_mfn,
> next_level = merge_level - 1;
>
> /* get pde at merge level */
> - table = map_domain_page(pt_mfn);
> + table = map_domain_page(_mfn(pt_mfn));
> pde = table + pfn_to_pde_idx(gfn, merge_level);
>
> /* get page table of next level */
> ntable_maddr = amd_iommu_get_next_table_from_pte((u32*)pde);
> - ntable = map_domain_page(ntable_maddr >> PAGE_SHIFT);
> + ntable = map_domain_page(_mfn(ntable_maddr >> PAGE_SHIFT));
And here.
> diff --git a/xen/drivers/passthrough/vtd/x86/vtd.c b/xen/drivers/passthrough/vtd/x86/vtd.c
> index 109234e..8a79e08 100644
> --- a/xen/drivers/passthrough/vtd/x86/vtd.c
> +++ b/xen/drivers/passthrough/vtd/x86/vtd.c
> @@ -41,7 +41,7 @@ boolean_param("iommu_inclusive_mapping", iommu_inclusive_mapping);
>
> void *map_vtd_domain_page(u64 maddr)
> {
> - return map_domain_page(maddr >> PAGE_SHIFT_4K);
> + return map_domain_page(_mfn(maddr >> PAGE_SHIFT_4K));
> }
And here.
>
> void unmap_vtd_domain_page(void *va)
> diff --git a/xen/include/asm-x86/hap.h b/xen/include/asm-x86/hap.h
> index 7876527..ca590f3 100644
> --- a/xen/include/asm-x86/hap.h
> +++ b/xen/include/asm-x86/hap.h
> @@ -37,7 +37,7 @@
> static inline void *
> hap_map_domain_page(mfn_t mfn)
> {
> - return map_domain_page(mfn_x(mfn));
> + return map_domain_page(mfn);
> }
Also worth removing hap_{,un}map_domain_page() when you remove the
shadow variants.
~Andrew
prev parent reply other threads:[~2015-07-01 16:22 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-01 13:41 [PATCH 0/3] Ben Catterall
2015-07-01 13:41 ` [PATCH 1/3] xen/domain_page: Convert map_domain_page_global() to using mfn_t Ben Catterall
2015-07-01 13:53 ` Andrew Cooper
2015-07-01 13:41 ` [PATCH 2/3] xen/domain_page: Convert copy/clear_domain_page() " Ben Catterall
2015-07-01 13:57 ` Andrew Cooper
2015-07-01 16:07 ` David Vrabel
2015-07-01 16:09 ` Andrew Cooper
2015-07-01 13:41 ` [PATCH 3/3] Convert map_domain_page() to use the new mfn_t type Ben Catterall
2015-07-01 16:22 ` Andrew Cooper [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=559413AE.4020405@citrix.com \
--to=andrew.cooper3@citrix.com \
--cc=Ben.Catterall@citrix.com \
--cc=ian.campbell@citrix.com \
--cc=ian.jackson@eu.citrix.com \
--cc=jbeulich@suse.com \
--cc=keir@xen.org \
--cc=stefano.stabellini@citrix.com \
--cc=tim@xen.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.