Message-ID: <aAetTj1U4BWSmqQV@google.com>
Date: Tue, 22 Apr 2025 07:53:02 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Oliver Upton <oliver.upton@...ux.dev>
Cc: Ankit Agrawal <ankita@...dia.com>, Jason Gunthorpe <jgg@...dia.com>, Marc Zyngier <maz@...nel.org>, 
	Catalin Marinas <catalin.marinas@....com>, "joey.gouly@....com" <joey.gouly@....com>, 
	"suzuki.poulose@....com" <suzuki.poulose@....com>, "yuzenghui@...wei.com" <yuzenghui@...wei.com>, 
	"will@...nel.org" <will@...nel.org>, "ryan.roberts@....com" <ryan.roberts@....com>, 
	"shahuang@...hat.com" <shahuang@...hat.com>, "lpieralisi@...nel.org" <lpieralisi@...nel.org>, 
	"david@...hat.com" <david@...hat.com>, Aniket Agashe <aniketa@...dia.com>, Neo Jia <cjia@...dia.com>, 
	Kirti Wankhede <kwankhede@...dia.com>, "Tarun Gupta (SW-GPU)" <targupta@...dia.com>, 
	Vikram Sethi <vsethi@...dia.com>, Andy Currid <acurrid@...dia.com>, 
	Alistair Popple <apopple@...dia.com>, John Hubbard <jhubbard@...dia.com>, Dan Williams <danw@...dia.com>, 
	Zhi Wang <zhiw@...dia.com>, Matt Ochs <mochs@...dia.com>, Uday Dhoke <udhoke@...dia.com>, 
	Dheeraj Nigam <dnigam@...dia.com>, Krishnakant Jaju <kjaju@...dia.com>, 
	"alex.williamson@...hat.com" <alex.williamson@...hat.com>, 
	"sebastianene@...gle.com" <sebastianene@...gle.com>, "coltonlewis@...gle.com" <coltonlewis@...gle.com>, 
	"kevin.tian@...el.com" <kevin.tian@...el.com>, "yi.l.liu@...el.com" <yi.l.liu@...el.com>, 
	"ardb@...nel.org" <ardb@...nel.org>, "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>, 
	"gshan@...hat.com" <gshan@...hat.com>, "linux-mm@...ck.org" <linux-mm@...ck.org>, 
	"ddutile@...hat.com" <ddutile@...hat.com>, "tabba@...gle.com" <tabba@...gle.com>, 
	"qperret@...gle.com" <qperret@...gle.com>, "kvmarm@...ts.linux.dev" <kvmarm@...ts.linux.dev>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, 
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v3 1/1] KVM: arm64: Allow cacheable stage 2 mapping using
 VMA flags

On Tue, Apr 22, 2025, Oliver Upton wrote:
> Hi,
> 
> On Wed, Apr 16, 2025 at 08:51:05AM +0000, Ankit Agrawal wrote:
> > Hi, summarizing the discussion so far and outlining the next steps. The key points
> > are as follows:
> > 1. KVM cap to expose whether the kernel supports mapping cacheable PFNMAP:
> > If the host doesn't have FWB, then the capability doesn't exist. Jason, Oliver, Caitlin
> > and Sean points that this may not be required as userspace do not have
> > much choice anyways. KVM has to follow the PTEs and userspace cannot ask
> > for something different. However, Marc points that enumerating FWB support
> > would allow userspace to discover the support and prevent live-migration
> > across FWB and non-FWB hosts. Jason suggested that this may still be fine as
> > we have already built in VFIO side protection where a live migration can be
> > attempted and then fail because of late-detected HW incompatibilities.
> > 
> > 2. New memslot flag that VMM passes at memslot registration:
> > Discussion point that this is not necessary and KVM should just follow the
> > VMA pgprot.
> > 
> > 3. Fallback path handling for PFNMAP when the FWB is not set:
> > Discussion points that there shouldn't be any fallback path and the memslot
> > should just fail. i.e. KVM should not allow degrading cachable to non-cachable
> > when it can't do flushing. This is to prevent the potential security issue
> > pointed by Jason (S1 cacheable, S2 noncacheable).
> > 
> > 
> > So AIU, the next step is to send out the updated series with the following patches:
> > 1. Block cacheable PFN map in memslot creation (kvm_arch_prepare_memory_region)
> > and during fault handling (user_mem_abort()).
> 
> Yes, we need to prevent the creation of stage-2 mappings to PFNMAP memory
> that uses cacheable attributes in the host stage-1. I believe we have alignment
> that this is a bugfix.
> 
> > 2. Enable support for cacheable PFN maps if S2FWB is enabled by following
> > the vma pgprot (this patch).
> > 
> > 3. Add and expose the new KVM cap to expose cacheable PFNMAP (set to false
> > for !FWB), pending maintainers' feedback on the necessity of this capability.
> 
> Regarding UAPI: I'm still convinced that we need the VMM to buy in to this
> behavior. And no, it doesn't matter if this is some VFIO-based mapping
> or kernel-managed memory.
> 
> The reality is that userspace is an equal participant in remaining coherent with
> the guest. Whether or not FWB is employed for a particular region of IPA
> space is useful information for userspace deciding what it needs to do to access guest
> memory. Ignoring the Nvidia widget for a second, userspace also needs to know this for
> 'normal', kernel-managed memory so it understands what CMOs may be necessary when (for
> example) doing live migration of the VM.
> 
> So this KVM CAP needs to be paired with a memslot flag.
>
>  - The capability says KVM is able to enforce Write-Back at stage-2
> 
>  - The memslot flag says userspace expects a particular GFN range to guarantee
>    Write-Back semantics. This can be applied to 'normal', kernel-managed memory
>    and PFNMAP thingies that have cacheable attributes at host stage-1.

I am very strongly opposed to adding a memslot flag.  IMO, it sets a terrible
precedent, and I am struggling to understand why a per-VM CAP isn't sufficient
protection for the VMM.

>  - Under no situation do we allow userspace to create non-cacheable mapping at
>    stage-2 for something PFNMAP cacheable at stage-1.
> 
> No matter what, my understanding is that we all agree the driver which provided the
> host stage-1 mapping is the authoritative source for memory attributes compatible
> with a given device. The accompanying UAPI is necessary for the VMM to understand how
> to handle arbitrary cacheable mappings provided to the VM.
> 
> Thanks,
> Oliver

Powered by blists - more mailing lists