Re: [PATCH 2/8] p2m-pt: Avoid hard-coding Xen's PAT

[Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>]

On 06/12/2022 04:33, Demi Marie Obenour wrote:
> This makes the code much easier to understand.
>
> Signed-off-by: Demi Marie Obenour <demi@xxxxxxxxxxxxxxxxxxxxxx>
> ---
>  xen/arch/x86/mm/p2m-pt.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
> index 
> eaba2b0fb4e6830f52b7d112fba8175dfe6d2770..cd1af33b6772ab1016e8d4c3284a6bc5d282869d
>  100644
> --- a/xen/arch/x86/mm/p2m-pt.c
> +++ b/xen/arch/x86/mm/p2m-pt.c
> @@ -99,13 +99,13 @@ static unsigned long p2m_type_to_flags(const struct 
> p2m_domain *p2m,
>          return flags | P2M_BASE_FLAGS | _PAGE_RW | _PAGE_NX_BIT;
>      case p2m_mmio_direct:
>          if ( !rangeset_contains_singleton(mmio_ro_ranges, mfn_x(mfn)) )
> -            flags |= _PAGE_RW;
> +            flags |= _PAGE_RW | _PAGE_UCM;
>          else
>          {
> -            flags |= _PAGE_PWT;
> +            flags |= _PAGE_UC;
>              ASSERT(!level);
>          }
> -        return flags | P2M_BASE_FLAGS | _PAGE_PCD;
> +        return flags | P2M_BASE_FLAGS;

I agree that this is a correct transformation of the logic, but the
logic cannot possibly be correct in the first place.

The read-only-ness of the MMIO range has no legitimate bearing on UC vs
UC-.  I have a feeling this is another attempt to control mixed
cacheability - the HVM side cleanup from XSA-402 is still very much pending.

I'm tempted to R-by and commit the patch, with a note in the commit
message saying that the transformation is correct but that it highlights
that the pre-existing logic is suspect.

~Andrew