Re: [PATCH v4] KVM: arm64: Record whether pKVM stage 2 mapping is cacheable

[Bradley Morgan]

On July 5, 2026 9:13:31 PM GMT+01:00, Marc Zyngier <maz at kernel.org> wrote:
>On Sun, 05 Jul 2026 20:29:20 +0100,
>Bradley Morgan <include at grrlz.net> wrote:
>> 
>> On July 5, 2026 8:27:34 PM GMT+01:00, Marc Zyngier <maz at kernel.org>
>wrote:
>> >On Sun, 05 Jul 2026 15:08:58 +0100,
>> >Dev Jain <dev.jain at arm.com> wrote:
>> >> 
>> >> 
>> >> 
>> >> On 02/07/26 12:54 am, Bradley Morgan wrote:
>> >> > pKVM keeps its own mapping list for stage 2 operations. Its flush
>path
>> >> > uses that list directly, so it lost the PTE attribute check done by
>> >the
>> >> > generic stage 2 walker.
>> >> > 
>> >> > Record whether a mapping is cacheable and skip cache maintenance
>for
>> >> > mappings that are not cacheable.
>> >> > 
>> >> > Fixes: e912efed485a ("KVM: arm64: Introduce the EL1 pKVM MMU")
>> >> 
>> >> Is Fixes tag required? If I am reading correctly, Arm ARM says this:
>> >> 
>> >> "For VA-based cache maintenance instructions, the instruction
>operates
>> >on the
>> >> caches regardless of the memory type and cacheability attributes
>marked
>> >for
>> >> the memory address in the VMSA translation table entries. This means
>> >that
>> >> the effects of the cache maintenance instructions can apply
>regardless
>> >of:
>> >>   Whether the address accessed:
>> >>     Is Normal memory or Device memory.
>> >>     Has the Cacheable attribute or the Non-cacheable attribute."
>> >> 
>> >> So nothing goes wrong if we do dcache clean for non-cacheable
>> >> memory.
>> >
>> >Two things:
>> >
>> >- having to perform CMOs for something that is not *expected* to be
>> >  cacheable is both pointless and a contradiction of the intent
>> >
>> >- what you quote is about the nature of the *mapping*, and not the
>> >  memory that is being mapped. Cleaning a dirty cache line on an
>> >  unsuspecting MMIO endpoint is never going to end nicely. Just have a
>> >  try.
>> >
>> >My reading of all this is that a fix indeed is required, and therefore
>> >a Fixes tag *must* be present.
>> >
>> >	M.
>> >
>> >
>> 
>> 
>> Well, fair enough.
>> 
>> It's kind of two things.
>> 
>> 1: A fix
>> 2: a optimization 
>> 
>> I say it's a fix, because it's not fun, no.
>>
>> And I Also say it's a optimization because wasting cycles isn't a good
>> idea, trust me.
>
>This is getting tiresome. Really. And when it comes to wasting cycles,
>you strike me as the expert here.
>
>It's not an optimisation at all. If anything, the test is going to
>cost cycles because there is statistically never a case that you map a
>device with upstream pKVM.
>
>If you call this an optimisation, I'll ask for numbers *proving* this
>is actually one. Do you really want to get down that road?

yeah, lesson learned..

before I embarrass myself more, I'll only respond if I have to...

sorry.

>	M.
>
>

Thanks!