On 06/12/2022 04:33, Demi Marie Obenour wrote: > This makes the code much easier to understand. > > Signed-off-by: Demi Marie Obenour <[email protected]> > --- > 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
