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