From: "Tian, Kevin" <kevin.tian@intel.com>
To: Roger Pau Monne <roger.pau@citrix.com>,
"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: Jan Beulich <jbeulich@suse.com>,
"Cooper, Andrew" <andrew.cooper3@citrix.com>,
Wei Liu <wl@xen.org>, "Nakajima, Jun" <jun.nakajima@intel.com>,
George Dunlap <george.dunlap@citrix.com>
Subject: RE: [PATCH 2/3] x86/mtrr: move epte_get_entry_emt to p2m-ept.c
Date: Thu, 17 Jun 2021 09:15:33 +0000 [thread overview]
Message-ID: <MWHPR11MB1886DF80D81BAD5050A7DD8C8C0E9@MWHPR11MB1886.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20210528173935.29919-3-roger.pau@citrix.com>
> From: Roger Pau Monne <roger.pau@citrix.com>
> Sent: Saturday, May 29, 2021 1:40 AM
>
> This is an EPT specific function, so it shouldn't live in the generic
> mtrr file. Such movement is also needed for future work that will
> require passing a p2m_type_t parameter to epte_get_entry_emt, and
> making that type visible to the mtrr users is cumbersome and
> unneeded.
>
> Moving epte_get_entry_emt out of mtrr.c requires making the helper to
> get the MTRR type of an address from the mtrr state public. While
> there rename the function to start with the mtrr prefix, like other
> mtrr related functions.
>
> While there fix some of the types of the function parameters.
>
> No functional change intended.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> ---
> xen/arch/x86/hvm/mtrr.c | 107 +---------------------------
> xen/arch/x86/hvm/vmx/vmx.c | 4 +-
> xen/arch/x86/mm/p2m-ept.c | 114 ++++++++++++++++++++++++++++--
> xen/include/asm-x86/hvm/vmx/vmx.h | 2 +
> xen/include/asm-x86/mtrr.h | 5 +-
> 5 files changed, 118 insertions(+), 114 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/mtrr.c b/xen/arch/x86/hvm/mtrr.c
> index 82ded1635c..4a9f3177ed 100644
> --- a/xen/arch/x86/hvm/mtrr.c
> +++ b/xen/arch/x86/hvm/mtrr.c
> @@ -194,8 +194,7 @@ void hvm_vcpu_cacheattr_destroy(struct vcpu *v)
> * May return a negative value when order > 0, indicating to the caller
> * that the respective mapping needs splitting.
> */
> -static int get_mtrr_type(const struct mtrr_state *m,
> - paddr_t pa, unsigned int order)
> +int mtrr_get_type(const struct mtrr_state *m, paddr_t pa, unsigned int
> order)
> {
> uint8_t overlap_mtrr = 0;
> uint8_t overlap_mtrr_pos = 0;
> @@ -323,7 +322,7 @@ static uint8_t effective_mm_type(struct mtrr_state
> *m,
> * just use it
> */
> if ( gmtrr_mtype == NO_HARDCODE_MEM_TYPE )
> - mtrr_mtype = get_mtrr_type(m, gpa, 0);
> + mtrr_mtype = mtrr_get_type(m, gpa, 0);
> else
> mtrr_mtype = gmtrr_mtype;
>
> @@ -350,7 +349,7 @@ uint32_t get_pat_flags(struct vcpu *v,
> guest_eff_mm_type = effective_mm_type(g, pat, gpaddr,
> gl1e_flags, gmtrr_mtype);
> /* 2. Get the memory type of host physical address, with MTRR */
> - shadow_mtrr_type = get_mtrr_type(&mtrr_state, spaddr, 0);
> + shadow_mtrr_type = mtrr_get_type(&mtrr_state, spaddr, 0);
>
> /* 3. Find the memory type in PAT, with host MTRR memory type
> * and guest effective memory type.
> @@ -789,106 +788,6 @@ void memory_type_changed(struct domain *d)
> }
> }
>
> -int epte_get_entry_emt(struct domain *d, unsigned long gfn, mfn_t mfn,
> - unsigned int order, uint8_t *ipat, bool_t direct_mmio)
> -{
> - int gmtrr_mtype, hmtrr_mtype;
> - struct vcpu *v = current;
> - unsigned long i;
> -
> - *ipat = 0;
> -
> - if ( v->domain != d )
> - v = d->vcpu ? d->vcpu[0] : NULL;
> -
> - /* Mask, not add, for order so it works with INVALID_MFN on unmapping
> */
> - if ( rangeset_overlaps_range(mmio_ro_ranges, mfn_x(mfn),
> - mfn_x(mfn) | ((1UL << order) - 1)) )
> - {
> - if ( !order || rangeset_contains_range(mmio_ro_ranges, mfn_x(mfn),
> - mfn_x(mfn) | ((1UL << order) - 1)) )
> - {
> - *ipat = 1;
> - return MTRR_TYPE_UNCACHABLE;
> - }
> - /* Force invalid memory type so resolve_misconfig() will split it */
> - return -1;
> - }
> -
> - if ( !mfn_valid(mfn) )
> - {
> - *ipat = 1;
> - return MTRR_TYPE_UNCACHABLE;
> - }
> -
> - if ( !direct_mmio && !is_iommu_enabled(d)
> && !cache_flush_permitted(d) )
> - {
> - *ipat = 1;
> - return MTRR_TYPE_WRBACK;
> - }
> -
> - for ( i = 0; i < (1ul << order); i++ )
> - {
> - if ( is_special_page(mfn_to_page(mfn_add(mfn, i))) )
> - {
> - if ( order )
> - return -1;
> - *ipat = 1;
> - return MTRR_TYPE_WRBACK;
> - }
> - }
> -
> - if ( direct_mmio )
> - return MTRR_TYPE_UNCACHABLE;
> -
> - gmtrr_mtype = hvm_get_mem_pinned_cacheattr(d, _gfn(gfn), order);
> - if ( gmtrr_mtype >= 0 )
> - {
> - *ipat = 1;
> - return gmtrr_mtype != PAT_TYPE_UC_MINUS ? gmtrr_mtype
> - : MTRR_TYPE_UNCACHABLE;
> - }
> - if ( gmtrr_mtype == -EADDRNOTAVAIL )
> - return -1;
> -
> - gmtrr_mtype = v ? get_mtrr_type(&v->arch.hvm.mtrr, gfn << PAGE_SHIFT,
> order)
> - : MTRR_TYPE_WRBACK;
> - hmtrr_mtype = get_mtrr_type(&mtrr_state, mfn_x(mfn) << PAGE_SHIFT,
> order);
> - if ( gmtrr_mtype < 0 || hmtrr_mtype < 0 )
> - return -1;
> -
> - /* If both types match we're fine. */
> - if ( likely(gmtrr_mtype == hmtrr_mtype) )
> - return hmtrr_mtype;
> -
> - /* If either type is UC, we have to go with that one. */
> - if ( gmtrr_mtype == MTRR_TYPE_UNCACHABLE ||
> - hmtrr_mtype == MTRR_TYPE_UNCACHABLE )
> - return MTRR_TYPE_UNCACHABLE;
> -
> - /* If either type is WB, we have to go with the other one. */
> - if ( gmtrr_mtype == MTRR_TYPE_WRBACK )
> - return hmtrr_mtype;
> - if ( hmtrr_mtype == MTRR_TYPE_WRBACK )
> - return gmtrr_mtype;
> -
> - /*
> - * At this point we have disagreeing WC, WT, or WP types. The only
> - * combination that can be cleanly resolved is WT:WP. The ones involving
> - * WC need to be converted to UC, both due to the memory ordering
> - * differences and because WC disallows reads to be cached (WT and WP
> - * permit this), while WT and WP require writes to go straight to memory
> - * (WC can buffer them).
> - */
> - if ( (gmtrr_mtype == MTRR_TYPE_WRTHROUGH &&
> - hmtrr_mtype == MTRR_TYPE_WRPROT) ||
> - (gmtrr_mtype == MTRR_TYPE_WRPROT &&
> - hmtrr_mtype == MTRR_TYPE_WRTHROUGH) )
> - return MTRR_TYPE_WRPROT;
> -
> - return MTRR_TYPE_UNCACHABLE;
> -}
> -
> /*
> * Local variables:
> * mode: C
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 7e3e67fdc3..0d4b47681b 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -417,12 +417,12 @@ static int vmx_domain_initialise(struct domain *d)
> static void domain_creation_finished(struct domain *d)
> {
> gfn_t gfn = gaddr_to_gfn(APIC_DEFAULT_PHYS_BASE);
> - uint8_t ipat;
> + bool ipat;
>
> if ( !has_vlapic(d) || mfn_eq(apic_access_mfn, INVALID_MFN) )
> return;
>
> - ASSERT(epte_get_entry_emt(d, gfn_x(gfn), apic_access_mfn, 0, &ipat,
> + ASSERT(epte_get_entry_emt(d, gfn, apic_access_mfn, 0, &ipat,
> true) == MTRR_TYPE_WRBACK);
> ASSERT(ipat);
>
> diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
> index a3beaf91e2..f1d1d07e92 100644
> --- a/xen/arch/x86/mm/p2m-ept.c
> +++ b/xen/arch/x86/mm/p2m-ept.c
> @@ -20,6 +20,7 @@
> #include <public/hvm/dm_op.h>
> #include <asm/altp2m.h>
> #include <asm/current.h>
> +#include <asm/iocap.h>
> #include <asm/paging.h>
> #include <asm/types.h>
> #include <asm/domain.h>
> @@ -485,6 +486,108 @@ static int ept_invalidate_emt_range(struct
> p2m_domain *p2m,
> return rc;
> }
>
> +int epte_get_entry_emt(struct domain *d, gfn_t gfn, mfn_t mfn,
> + unsigned int order, bool *ipat, bool direct_mmio)
> +{
> + int gmtrr_mtype, hmtrr_mtype;
> + struct vcpu *v = current;
> + unsigned long i;
> +
> + *ipat = false;
> +
> + if ( v->domain != d )
> + v = d->vcpu ? d->vcpu[0] : NULL;
> +
> + /* Mask, not add, for order so it works with INVALID_MFN on unmapping
> */
> + if ( rangeset_overlaps_range(mmio_ro_ranges, mfn_x(mfn),
> + mfn_x(mfn) | ((1UL << order) - 1)) )
> + {
> + if ( !order || rangeset_contains_range(mmio_ro_ranges, mfn_x(mfn),
> + mfn_x(mfn) | ((1UL << order) - 1)) )
> + {
> + *ipat = true;
> + return MTRR_TYPE_UNCACHABLE;
> + }
> + /* Force invalid memory type so resolve_misconfig() will split it */
> + return -1;
> + }
> +
> + if ( !mfn_valid(mfn) )
> + {
> + *ipat = true;
> + return MTRR_TYPE_UNCACHABLE;
> + }
> +
> + if ( !direct_mmio && !is_iommu_enabled(d)
> && !cache_flush_permitted(d) )
> + {
> + *ipat = true;
> + return MTRR_TYPE_WRBACK;
> + }
> +
> + for ( i = 0; i < (1ul << order); i++ )
> + {
> + if ( is_special_page(mfn_to_page(mfn_add(mfn, i))) )
> + {
> + if ( order )
> + return -1;
> + *ipat = true;
> + return MTRR_TYPE_WRBACK;
> + }
> + }
> +
> + if ( direct_mmio )
> + return MTRR_TYPE_UNCACHABLE;
> +
> + gmtrr_mtype = hvm_get_mem_pinned_cacheattr(d, gfn, order);
> + if ( gmtrr_mtype >= 0 )
> + {
> + *ipat = true;
> + return gmtrr_mtype != PAT_TYPE_UC_MINUS ? gmtrr_mtype
> + : MTRR_TYPE_UNCACHABLE;
> + }
> + if ( gmtrr_mtype == -EADDRNOTAVAIL )
> + return -1;
> +
> + gmtrr_mtype = v ? mtrr_get_type(&v->arch.hvm.mtrr,
> + gfn_x(gfn) << PAGE_SHIFT, order)
> + : MTRR_TYPE_WRBACK;
> + hmtrr_mtype = mtrr_get_type(&mtrr_state, mfn_x(mfn) << PAGE_SHIFT,
> + order);
> + if ( gmtrr_mtype < 0 || hmtrr_mtype < 0 )
> + return -1;
> +
> + /* If both types match we're fine. */
> + if ( likely(gmtrr_mtype == hmtrr_mtype) )
> + return hmtrr_mtype;
> +
> + /* If either type is UC, we have to go with that one. */
> + if ( gmtrr_mtype == MTRR_TYPE_UNCACHABLE ||
> + hmtrr_mtype == MTRR_TYPE_UNCACHABLE )
> + return MTRR_TYPE_UNCACHABLE;
> +
> + /* If either type is WB, we have to go with the other one. */
> + if ( gmtrr_mtype == MTRR_TYPE_WRBACK )
> + return hmtrr_mtype;
> + if ( hmtrr_mtype == MTRR_TYPE_WRBACK )
> + return gmtrr_mtype;
> +
> + /*
> + * At this point we have disagreeing WC, WT, or WP types. The only
> + * combination that can be cleanly resolved is WT:WP. The ones involving
> + * WC need to be converted to UC, both due to the memory ordering
> + * differences and because WC disallows reads to be cached (WT and WP
> + * permit this), while WT and WP require writes to go straight to memory
> + * (WC can buffer them).
> + */
> + if ( (gmtrr_mtype == MTRR_TYPE_WRTHROUGH &&
> + hmtrr_mtype == MTRR_TYPE_WRPROT) ||
> + (gmtrr_mtype == MTRR_TYPE_WRPROT &&
> + hmtrr_mtype == MTRR_TYPE_WRTHROUGH) )
> + return MTRR_TYPE_WRPROT;
> +
> + return MTRR_TYPE_UNCACHABLE;
> +}
> +
> /*
> * Resolve deliberately mis-configured (EMT field set to an invalid value)
> * entries in the page table hierarchy for the given GFN:
> @@ -519,7 +622,7 @@ static int resolve_misconfig(struct p2m_domain
> *p2m, unsigned long gfn)
>
> if ( level == 0 || is_epte_superpage(&e) )
> {
> - uint8_t ipat = 0;
> + bool ipat;
>
> if ( e.emt != MTRR_NUM_TYPES )
> break;
> @@ -535,7 +638,7 @@ static int resolve_misconfig(struct p2m_domain
> *p2m, unsigned long gfn)
> e.emt = 0;
> if ( !is_epte_valid(&e) || !is_epte_present(&e) )
> continue;
> - e.emt = epte_get_entry_emt(p2m->domain, gfn + i,
> + e.emt = epte_get_entry_emt(p2m->domain, _gfn(gfn + i),
> _mfn(e.mfn), 0, &ipat,
> e.sa_p2mt == p2m_mmio_direct);
> e.ipat = ipat;
> @@ -553,7 +656,8 @@ static int resolve_misconfig(struct p2m_domain
> *p2m, unsigned long gfn)
> }
> else
> {
> - int emt = epte_get_entry_emt(p2m->domain, gfn, _mfn(e.mfn),
> + int emt = epte_get_entry_emt(p2m->domain, _gfn(gfn),
> + _mfn(e.mfn),
> level * EPT_TABLE_ORDER, &ipat,
> e.sa_p2mt == p2m_mmio_direct);
> bool_t recalc = e.recalc;
> @@ -788,8 +892,8 @@ ept_set_entry(struct p2m_domain *p2m, gfn_t gfn_,
> mfn_t mfn,
>
> if ( mfn_valid(mfn) || p2m_allows_invalid_mfn(p2mt) )
> {
> - uint8_t ipat = 0;
> - int emt = epte_get_entry_emt(p2m->domain, gfn, mfn,
> + bool ipat;
> + int emt = epte_get_entry_emt(p2m->domain, _gfn(gfn), mfn,
> i * EPT_TABLE_ORDER, &ipat,
> p2mt == p2m_mmio_direct);
>
> diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h b/xen/include/asm-
> x86/hvm/vmx/vmx.h
> index 534e9fc221..f668ee1f09 100644
> --- a/xen/include/asm-x86/hvm/vmx/vmx.h
> +++ b/xen/include/asm-x86/hvm/vmx/vmx.h
> @@ -599,6 +599,8 @@ void ept_p2m_uninit(struct p2m_domain *p2m);
>
> void ept_walk_table(struct domain *d, unsigned long gfn);
> bool_t ept_handle_misconfig(uint64_t gpa);
> +int epte_get_entry_emt(struct domain *d, gfn_t gfn, mfn_t mfn,
> + unsigned int order, bool *ipat, bool direct_mmio);
> void setup_ept_dump(void);
> void p2m_init_altp2m_ept(struct domain *d, unsigned int i);
> /* Locate an alternate p2m by its EPTP */
> diff --git a/xen/include/asm-x86/mtrr.h b/xen/include/asm-x86/mtrr.h
> index 24e5de5c22..e0fd1005ce 100644
> --- a/xen/include/asm-x86/mtrr.h
> +++ b/xen/include/asm-x86/mtrr.h
> @@ -72,12 +72,11 @@ extern int mtrr_add_page(unsigned long base,
> unsigned long size,
> unsigned int type, char increment);
> extern int mtrr_del(int reg, unsigned long base, unsigned long size);
> extern int mtrr_del_page(int reg, unsigned long base, unsigned long size);
> +extern int mtrr_get_type(const struct mtrr_state *m, paddr_t pa,
> + unsigned int order);
> extern void mtrr_centaur_report_mcr(int mcr, u32 lo, u32 hi);
> extern u32 get_pat_flags(struct vcpu *v, u32 gl1e_flags, paddr_t gpaddr,
> paddr_t spaddr, uint8_t gmtrr_mtype);
> -extern int epte_get_entry_emt(struct domain *, unsigned long gfn, mfn_t
> mfn,
> - unsigned int order, uint8_t *ipat,
> - bool_t direct_mmio);
> extern unsigned char pat_type_2_pte_flags(unsigned char pat_type);
> extern int hold_mtrr_updates_on_aps;
> extern void mtrr_aps_sync_begin(void);
> --
> 2.31.1
next prev parent reply other threads:[~2021-06-17 9:16 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-28 17:39 [PATCH 0/3] x86/ept: force WB to foreign and grant mappings Roger Pau Monne
2021-05-28 17:39 ` [PATCH 1/3] x86/mtrr: remove stale function prototype Roger Pau Monne
2021-05-31 6:48 ` Jan Beulich
2021-05-28 17:39 ` [PATCH 2/3] x86/mtrr: move epte_get_entry_emt to p2m-ept.c Roger Pau Monne
2021-05-31 7:00 ` Jan Beulich
2021-06-17 9:15 ` Tian, Kevin [this message]
2021-05-28 17:39 ` [PATCH 3/3] x86/ept: force WB cache attributes for grant and foreign maps Roger Pau Monne
2021-05-31 7:21 ` Jan Beulich
2021-06-02 9:36 ` Roger Pau Monné
2021-06-17 9:31 ` Tian, Kevin
2021-06-17 11:40 ` Roger Pau Monné
2021-06-17 11:57 ` Jan Beulich
2021-06-15 13:37 ` [PATCH 0/3] x86/ept: force WB to foreign and grant mappings Roger Pau Monné
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=MWHPR11MB1886DF80D81BAD5050A7DD8C8C0E9@MWHPR11MB1886.namprd11.prod.outlook.com \
--to=kevin.tian@intel.com \
--cc=andrew.cooper3@citrix.com \
--cc=george.dunlap@citrix.com \
--cc=jbeulich@suse.com \
--cc=jun.nakajima@intel.com \
--cc=roger.pau@citrix.com \
--cc=wl@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).