Received-SPF: pass (google.com: domain of linux-kernel+bounces-62097-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Mon, 12 Feb 2024 10:27:18 -0700
From: Alex Williamson <alex.williamson@redhat.com>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: ankita@nvidia.com, maz@kernel.org, oliver.upton@linux.dev,
 james.morse@arm.com, suzuki.poulose@arm.com, yuzenghui@huawei.com,
 reinette.chatre@intel.com, surenb@google.com, stefanha@redhat.com,
 brauner@kernel.org, catalin.marinas@arm.com, will@kernel.org,
 mark.rutland@arm.com, kevin.tian@intel.com, yi.l.liu@intel.com,
 ardb@kernel.org, akpm@linux-foundation.org, andreyknvl@gmail.com,
 wangjinchao@xfusion.com, gshan@redhat.com, shahuang@redhat.com,
 ricarkol@google.com, linux-mm@kvack.org, lpieralisi@kernel.org,
 rananta@google.com, ryan.roberts@arm.com, david@redhat.com,
 linus.walleij@linaro.org, bhe@redhat.com, aniketa@nvidia.com,
 cjia@nvidia.com, kwankhede@nvidia.com, targupta@nvidia.com,
 vsethi@nvidia.com, acurrid@nvidia.com, apopple@nvidia.com,
 jhubbard@nvidia.com, danw@nvidia.com, kvmarm@lists.linux.dev,
 mochs@nvidia.com, zhiw@nvidia.com, kvm@vger.kernel.org,
 linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v7 4/4] vfio: convey kvm that the vfio-pci device is wc
 safe
Message-ID: <20240212102718.07543659.alex.williamson@redhat.com>
In-Reply-To: <20240212172001.GE4048826@nvidia.com>
References: <20240211174705.31992-1-ankita@nvidia.com>
	<20240211174705.31992-5-ankita@nvidia.com>
	<20240212100502.2b5009e4.alex.williamson@redhat.com>
	<20240212172001.GE4048826@nvidia.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Mon, 12 Feb 2024 13:20:01 -0400
Jason Gunthorpe <jgg@nvidia.com> wrote:

> On Mon, Feb 12, 2024 at 10:05:02AM -0700, Alex Williamson wrote:
> 
> > > --- a/drivers/vfio/pci/vfio_pci_core.c
> > > +++ b/drivers/vfio/pci/vfio_pci_core.c
> > > @@ -1862,8 +1862,12 @@ int vfio_pci_core_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma
> > >  	/*
> > >  	 * See remap_pfn_range(), called from vfio_pci_fault() but we can't
> > >  	 * change vm_flags within the fault handler.  Set them now.
> > > +	 *
> > > +	 * Set an additional flag VM_ALLOW_ANY_UNCACHED to convey kvm that
> > > +	 * the device is wc safe.
> > >  	 */  
> > 
> > That's a pretty superficial comment.  Check that this is accurate, but
> > maybe something like:
> > 
> > 	The VM_ALLOW_ANY_UNCACHED flag is implemented for ARM64,
> > 	allowing stage 2 device mapping attributes to use Normal-NC  
>                ^^^^ 
> 
> > 	rather than DEVICE_nGnRE, which allows guest mappings
> > 	supporting combining attributes (WC).  This attribute has
> > 	potential risks with the GICv2 VCPU interface, but is expected
> > 	to be safe for vfio-pci use cases.  
> 
> Sure, if you want to elaborate more
> 
>   The VM_ALLOW_ANY_UNCACHED flag is implemented for ARM64,
>   allowing KVM stage 2 device mapping attributes to use Normal-NC
>   rather than DEVICE_nGnRE, which allows guest mappings
>   supporting combining attributes (WC). ARM does not architecturally
>   guarentee this is safe, and indeed some MMIO regions like the GICv2
>   VCPU interface can trigger uncontained faults if Normal-NC is used.
> 
>   Even worse we expect there are platforms where even DEVICE_nGnRE can
>   allow uncontained faults in conercases. Unfortunately existing ARM
                                ^^^^^^^^^^

*corner cases


>   IP requires platform integration to take responsibility to prevent
>   this.
> 
>   To safely use VFIO in KVM the platform must guarantee full safety
>   in the guest where no action taken against a MMIO mapping can
>   trigger an uncontainer failure. We belive that most VFIO PCI
>   platforms support this for both mapping types, at least in common
>   flows, based on some expectations of how PCI IP is integrated. This
>   can be enabled more broadly, for instance into vfio-platform
>   drivers, but only after the platform vendor completes auditing for
>   safety.

I like it, please incorporate into the next version.
  
> > And specifically, I think these other devices that may be problematic
> > as described in the cover letter is a warning against use for
> > vfio-platform, is that correct?  
> 
> Maybe more like "we have a general consensus that vfio-pci is likely
> safe due to how PCI IP is typically integrated, but it is much less
> obvious for other VFIO bus types. As there is no known WC user for
> vfio-platform drivers be conservative and do not enable it."

Ok.  Thanks for the clarification.

Alex