Re: [PATCH v2 for-4.19 2/3] x86/EPT: avoid marking non-present entries for re-configuring

["Oleksii K." <oleksii.kurochko@xxxxxxxxx>]

On Wed, 2024-06-12 at 15:16 +0200, Jan Beulich wrote:
> For non-present entries EMT, like most other fields, is meaningless
> to
> hardware. Make the logic in ept_set_entry() setting the field (and
> iPAT)
> conditional upon dealing with a present entry, leaving the value at 0
> otherwise. This has two effects for epte_get_entry_emt() which we'll
> want to leverage subsequently:
> 1) The call moved here now won't be issued with INVALID_MFN anymore
> (a
>    respective BUG_ON() is being added).
> 2) Neither of the other two calls could now be issued with a
> truncated
>    form of INVALID_MFN anymore (as long as there's no bug anywhere
>    marking an entry present when that was populated using
> INVALID_MFN).
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
Release-Acked-By: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>

~ Oleksii
> ---
> v2: New.
> 
> --- a/xen/arch/x86/mm/p2m-ept.c
> +++ b/xen/arch/x86/mm/p2m-ept.c
> @@ -650,6 +650,8 @@ static int cf_check resolve_misconfig(st
>              if ( e.emt != MTRR_NUM_TYPES )
>                  break;
>  
> +            ASSERT(is_epte_present(&e));
> +
>              if ( level == 0 )
>              {
>                  for ( gfn -= i, i = 0; i < EPT_PAGETABLE_ENTRIES;
> ++i )
> @@ -915,17 +917,6 @@ ept_set_entry(struct p2m_domain *p2m, gf
>  
>      if ( mfn_valid(mfn) || p2m_allows_invalid_mfn(p2mt) )
>      {
> -        bool ipat;
> -        int emt = epte_get_entry_emt(p2m->domain, _gfn(gfn), mfn,
> -                                     i * EPT_TABLE_ORDER, &ipat,
> -                                     p2mt);
> -
> -        if ( emt >= 0 )
> -            new_entry.emt = emt;
> -        else /* ept_handle_misconfig() will need to take care of
> this. */
> -            new_entry.emt = MTRR_NUM_TYPES;
> -
> -        new_entry.ipat = ipat;
>          new_entry.sp = !!i;
>          new_entry.sa_p2mt = p2mt;
>          new_entry.access = p2ma;
> @@ -941,6 +932,22 @@ ept_set_entry(struct p2m_domain *p2m, gf
>              need_modify_vtd_table = 0;
>  
>          ept_p2m_type_to_flags(p2m, &new_entry);
> +
> +        if ( is_epte_present(&new_entry) )
> +        {
> +            bool ipat;
> +            int emt = epte_get_entry_emt(p2m->domain, _gfn(gfn),
> mfn,
> +                                         i * EPT_TABLE_ORDER, &ipat,
> +                                         p2mt);
> +
> +            BUG_ON(mfn_eq(mfn, INVALID_MFN));
> +
> +            if ( emt >= 0 )
> +                new_entry.emt = emt;
> +            else /* ept_handle_misconfig() will need to take care of
> this. */
> +                new_entry.emt = MTRR_NUM_TYPES;
> +            new_entry.ipat = ipat;
> +        }
>      }
>  
>      if ( sve != -1 )
>