| Message ID | 20240220072926.6466-3-ankita@nvidia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | kvm: arm64: allow the VM to select DEVICE_* and NORMAL_NC for IO memory | expand |
On 2/20/24 07:29, Ankit Agrawal wrote: > From: Ankit Agrawal <ankita@nvidia.com> > > The VM_ALLOW_ANY_UNCACHED flag is implemented for ARM64, allowing KVM > stage 2 device mapping attributes to use NormalNC rather than > DEVICE_nGnRE, which allows guest mappings supporting combining > attributes (WC). ARM does not architecturally guarantee this is safe, > and indeed some MMIO regions like the GICv2 VCPU interface can trigger > uncontained faults if NormalNC is used. > > Even worse we expect there are platforms where even DEVICE_nGnRE can > allow uncontained faults in corner cases. Unfortunately existing ARM 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 > uncontained failure. We belive that most VFIO PCI platforms support this A nit, let's use passive voice in the patch comment. Also belive is mostly a typo. > 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. > > The VMA flag VM_ALLOW_ANY_UNCACHED was found to be the simplest and > cleanest way to communicate the information from VFIO to KVM that > mapping the region in S2 as NormalNC is safe. KVM consumes it to > activate the code that does the S2 mapping as NormalNC. > > Suggested-by: Catalin Marinas <catalin.marinas@arm.com> > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> > Acked-by: David Hildenbrand <david@redhat.com> > Signed-off-by: Ankit Agrawal <ankita@nvidia.com> > --- > include/linux/mm.h | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/include/linux/mm.h b/include/linux/mm.h > index f5a97dec5169..59576e56c58b 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -391,6 +391,20 @@ extern unsigned int kobjsize(const void *objp); > # define VM_UFFD_MINOR VM_NONE > #endif /* CONFIG_HAVE_ARCH_USERFAULTFD_MINOR */ > > +/* > + * This flag is used to connect VFIO to arch specific KVM code. It > + * indicates that the memory under this VMA is safe for use with any > + * non-cachable memory type inside KVM. Some VFIO devices, on some > + * platforms, are thought to be unsafe and can cause machine crashes > + * if KVM does not lock down the memory type. > + */ > +#ifdef CONFIG_64BIT > +#define VM_ALLOW_ANY_UNCACHED_BIT 39 > +#define VM_ALLOW_ANY_UNCACHED BIT(VM_ALLOW_ANY_UNCACHED_BIT) > +#else > +#define VM_ALLOW_ANY_UNCACHED VM_NONE > +#endif > + > /* Bits set in the VMA until the stack is in its final location */ > #define VM_STACK_INCOMPLETE_SETUP (VM_RAND_READ | VM_SEQ_READ | VM_STACK_EARLY) >
>> 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 >> uncontained failure. We belive that most VFIO PCI platforms support this > > A nit, let's use passive voice in the patch comment. Also belive is mostly > a typo. Sure, will do.
On 2/20/24 08:51, Ankit Agrawal wrote: >>> 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 >>> uncontained failure. We belive that most VFIO PCI platforms support this >> >> A nit, let's use passive voice in the patch comment. Also belive is mostly >> a typo. > > Sure, will do. Also patch 4 has the same nit. It should be fixed as well.
>>>> 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 >>>> uncontained failure. We belive that most VFIO PCI platforms support this >>> >>> A nit, let's use passive voice in the patch comment. Also belive is mostly >>> a typo. >> >> Sure, will do. >Also patch 4 has the same nit. It should be fixed as well. Yes.
On 20.02.24 09:51, Ankit Agrawal wrote: >>> 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 >>> uncontained failure. We belive that most VFIO PCI platforms support this >> >> A nit, let's use passive voice in the patch comment. Also belive is mostly >> a typo. > > Sure, will do. > s/we expect/the expectation is that/ s/We belive/The assumption is/ If it's just that, likely no need to resend. Maintainers usually can fix that up when applying (otherwise, they'll let you know :) ).
On 2/20/24 09:09, David Hildenbrand wrote: > External email: Use caution opening links or attachments > > > On 20.02.24 09:51, Ankit Agrawal wrote: >>>> 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 >>>> uncontained failure. We belive that most VFIO PCI platforms support >>>> this >>> >>> A nit, let's use passive voice in the patch comment. Also belive is >>> mostly >>> a typo. >> >> Sure, will do. >> > > s/we expect/the expectation is that/ > s/We belive/The assumption is/ > > If it's just that, likely no need to resend. Maintainers usually can fix > that up when applying (otherwise, they'll let you know :) ). > Many thanks! :) > -- > Cheers, > > David / dhildenb >
>>>>> 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 >>>>> uncontained failure. We belive that most VFIO PCI platforms support >>>>> this >>>> >>>> A nit, let's use passive voice in the patch comment. Also belive is >>>> mostly >>>> a typo. >>> >>> Sure, will do. >>> >> >> s/we expect/the expectation is that/ >> s/We belive/The assumption is/ >> >> If it's just that, likely no need to resend. Maintainers usually can fix >> that up when applying (otherwise, they'll let you know :) ). >> > Many thanks! :) Good to know, thanks David.
diff --git a/include/linux/mm.h b/include/linux/mm.h index f5a97dec5169..59576e56c58b 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -391,6 +391,20 @@ extern unsigned int kobjsize(const void *objp); # define VM_UFFD_MINOR VM_NONE #endif /* CONFIG_HAVE_ARCH_USERFAULTFD_MINOR */ +/* + * This flag is used to connect VFIO to arch specific KVM code. It + * indicates that the memory under this VMA is safe for use with any + * non-cachable memory type inside KVM. Some VFIO devices, on some + * platforms, are thought to be unsafe and can cause machine crashes + * if KVM does not lock down the memory type. + */ +#ifdef CONFIG_64BIT +#define VM_ALLOW_ANY_UNCACHED_BIT 39 +#define VM_ALLOW_ANY_UNCACHED BIT(VM_ALLOW_ANY_UNCACHED_BIT) +#else +#define VM_ALLOW_ANY_UNCACHED VM_NONE +#endif + /* Bits set in the VMA until the stack is in its final location */ #define VM_STACK_INCOMPLETE_SETUP (VM_RAND_READ | VM_SEQ_READ | VM_STACK_EARLY)