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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.12.2022 11:59, Andrew Cooper wrote:
> 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

I think so, yes - at the time the goal was to disallow cachability other
than the one used by Xen for those pages where we know Xen has a mapping.

Jan

> - 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