> From: Roger Pau Monne <roger....@citrix.com>
> Sent: Monday, June 27, 2022 6:01 PM
>
> The current logic in epte_get_entry_emt() will split any page marked
> as special with order greater than zero, without checking whether the
> super page is all special.
>
> Fix this by only splitting the page only if it's not all marked as
> special, in order to prevent unneeded super page shuttering.
>
> Fixes: ca24b2ffdb ('x86/hvm: set 'ipat' in EPT for special pages')
> Signed-off-by: Roger Pau Monné <roger....@citrix.com>
> ---
> Cc: Paul Durrant <p...@xen.org>
> ---
> It would seem weird to me to have a super page entry in EPT with
> ranges marked as special and not the full page. I guess it's better
> to be safe, but I don't see an scenario where we could end up in that
> situation.
>
> I've been trying to find a clarification in the original patch
> submission about how it's possible to have such super page EPT entry,
> but haven't been able to find any justification.
>
> Might be nice to expand the commit message as to why it's possible to
> have such mixed super page entries that would need splitting.
Here is what I dig out.
When reviewing v1 of adding special page check, Jan suggested
that APIC access page was also covered hence the old logic for APIC
access page can be removed. [1]
Then when reviewing v2 he found that the order check in removed
logic was not carried to the new check on special page. [2]
The original order check in old APIC access logic came from:
commit 126018f2acd5416434747423e61a4690108b9dc9
Author: Jan Beulich <jbeul...@suse.com>
Date: Fri May 2 10:48:48 2014 +0200
x86/EPT: consider page order when checking for APIC MFN
This was overlooked in 3d90d6e6 ("x86/EPT: split super pages upon
mismatching memory types").
Signed-off-by: Jan Beulich <jbeul...@suse.com>
Acked-by: Kevin Tian <kevin.t...@intel.com>
Reviewed-by: Tim Deegan <t...@xen.org>
I suppose Jan may actually find such mixed super page entry case
to bring this fix in.
Not sure whether Jan still remember the history.
[1] https://lists.xenproject.org/archives/html/xen-devel/2020-07/msg01648.html
[2] https://lore.kernel.org/all/a4856c33-8bb0-4afa-cc71-3af4c229b...@suse.com/
> ---
> xen/arch/x86/mm/p2m-ept.c | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
> index b04ca6dbe8..b4919bad51 100644
> --- a/xen/arch/x86/mm/p2m-ept.c
> +++ b/xen/arch/x86/mm/p2m-ept.c
> @@ -491,7 +491,7 @@ int epte_get_entry_emt(struct domain *d, gfn_t gfn,
> mfn_t mfn,
> {
> int gmtrr_mtype, hmtrr_mtype;
> struct vcpu *v = current;
> - unsigned long i;
> + unsigned long i, special_pgs;
>
> *ipat = false;
>
> @@ -525,15 +525,17 @@ int epte_get_entry_emt(struct domain *d, gfn_t
> gfn, mfn_t mfn,
> return MTRR_TYPE_WRBACK;
> }
>
> - for ( i = 0; i < (1ul << order); i++ )
> - {
> + for ( special_pgs = 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;
> - }
> + special_pgs++;
> +
> + if ( special_pgs )
> + {
> + if ( special_pgs != (1ul << order) )
> + return -1;
> +
> + *ipat = true;
> + return MTRR_TYPE_WRBACK;
Did you actually observe an impact w/o this fix?
> }
>
> switch ( type )
> --
> 2.36.1
