|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 3/3] Convert map_domain_page() to use the new mfn_t type
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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
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
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |