| Message ID | 20190218113600.9540-4-julien.grall@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | xen/arm: Properly disable M2P on Arm. | expand |
>>> On 18.02.19 at 12:35, <julien.grall@arm.com> wrote: > mfn_to_gfn and mfn_to_gmfn are doing exactly the same except the former > is using mfn_t. > > Furthermore, the naming of the former is more consistent with the > current naming scheme (GFN/MFN). So use replace mfn_to_gmfn with > mfn_to_gfn in x86 code. > > No functional changes. > > Signed-off-by: Julien Grall <julien.grall@arm.com> Fundamentally I'm fine with this change, but before making its use more wide-spread, wouldn't it be better to make mfn_to_gfn() fully type-safe, i.e. have it also return gfn_t? There aren't that many uses of the function just yet, and doing the conversion now would save us from having to touch all places you now change yet another time. Jan
Hi Jan, On 13/03/2019 14:45, Jan Beulich wrote: >>>> On 18.02.19 at 12:35, <julien.grall@arm.com> wrote: >> mfn_to_gfn and mfn_to_gmfn are doing exactly the same except the former >> is using mfn_t. >> >> Furthermore, the naming of the former is more consistent with the >> current naming scheme (GFN/MFN). So use replace mfn_to_gmfn with >> mfn_to_gfn in x86 code. >> >> No functional changes. >> >> Signed-off-by: Julien Grall <julien.grall@arm.com> > > Fundamentally I'm fine with this change, but before making its use > more wide-spread, wouldn't it be better to make mfn_to_gfn() > fully type-safe, i.e. have it also return gfn_t? There aren't that > many uses of the function just yet, and doing the conversion now > would save us from having to touch all places you now change > yet another time. I vaguely recall some problems when trying to use typesafe GFN. Maybe it is because I was trying to cleanup the code at the same time. Let me have another try. Cheers,
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index 32dc4253ff..ab1f25f49d 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -714,7 +714,7 @@ int arch_domain_soft_reset(struct domain *d) ASSERT( owner == d ); mfn = page_to_mfn(page); - gfn = mfn_to_gmfn(d, mfn_x(mfn)); + gfn = mfn_to_gfn(d, mfn); /* * gfn == INVALID_GFN indicates that the shared_info page was never mapped diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 7ec5954b03..df6e5bdd31 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -2518,7 +2518,7 @@ int free_page_type(struct page_info *page, unsigned long type, ASSERT(!shadow_mode_refcounts(owner)); - gmfn = mfn_to_gmfn(owner, mfn_x(page_to_mfn(page))); + gmfn = mfn_to_gfn(owner, page_to_mfn(page)); if ( VALID_M2P(gmfn) ) shadow_remove_all_shadows(owner, _mfn(gmfn)); } diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c index 942ece2ca0..0da1e29782 100644 --- a/xen/arch/x86/pv/emul-priv-op.c +++ b/xen/arch/x86/pv/emul-priv-op.c @@ -711,7 +711,7 @@ static int read_cr(unsigned int reg, unsigned long *val, if ( !is_pv_32bit_domain(currd) ) { mfn = pagetable_get_mfn(curr->arch.guest_table); - *val = xen_pfn_to_cr3(mfn_to_gmfn(currd, mfn_x(mfn))); + *val = xen_pfn_to_cr3(mfn_to_gfn(currd, mfn)); } else { @@ -720,7 +720,7 @@ static int read_cr(unsigned int reg, unsigned long *val, mfn = l4e_get_mfn(*pl4e); unmap_domain_page(pl4e); - *val = compat_pfn_to_cr3(mfn_to_gmfn(currd, mfn_x(mfn))); + *val = compat_pfn_to_cr3(mfn_to_gfn(currd, mfn)); } /* PTs should not be shared */ BUG_ON(page_get_owner(mfn_to_page(mfn)) == dom_cow); diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c index a88ef9b189..2b1915548a 100644 --- a/xen/drivers/passthrough/x86/iommu.c +++ b/xen/drivers/passthrough/x86/iommu.c @@ -59,15 +59,15 @@ int arch_iommu_populate_page_table(struct domain *d) if ( is_hvm_domain(d) || (page->u.inuse.type_info & PGT_type_mask) == PGT_writable_page ) { - unsigned long mfn = mfn_x(page_to_mfn(page)); - unsigned long gfn = mfn_to_gmfn(d, mfn); + mfn_t mfn = page_to_mfn(page); + unsigned long gfn = mfn_to_gfn(d, mfn); unsigned int flush_flags = 0; if ( gfn != gfn_x(INVALID_GFN) ) { ASSERT(!(gfn >> DEFAULT_DOMAIN_ADDRESS_WIDTH)); BUG_ON(SHARED_M2P(gfn)); - rc = iommu_map(d, _dfn(gfn), _mfn(mfn), PAGE_ORDER_4K, + rc = iommu_map(d, _dfn(gfn), mfn, PAGE_ORDER_4K, IOMMUF_readable | IOMMUF_writable, &flush_flags); }
mfn_to_gfn and mfn_to_gmfn are doing exactly the same except the former is using mfn_t. Furthermore, the naming of the former is more consistent with the current naming scheme (GFN/MFN). So use replace mfn_to_gmfn with mfn_to_gfn in x86 code. No functional changes. Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/x86/domain.c | 2 +- xen/arch/x86/mm.c | 2 +- xen/arch/x86/pv/emul-priv-op.c | 4 ++-- xen/drivers/passthrough/x86/iommu.c | 6 +++--- 4 files changed, 7 insertions(+), 7 deletions(-)