diff mbox series

[v8,2/4] mm: introduce new flag to indicate wc safe

Message ID 20240220072926.6466-3-ankita@nvidia.com (mailing list archive)
State New, archived
Headers show
Series kvm: arm64: allow the VM to select DEVICE_* and NORMAL_NC for IO memory | expand

Commit Message

Ankit Agrawal Feb. 20, 2024, 7:29 a.m. UTC
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
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(+)

Comments

Zhi Wang Feb. 20, 2024, 8:26 a.m. UTC | #1
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)
>
Ankit Agrawal Feb. 20, 2024, 8:51 a.m. UTC | #2
>> 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.
Zhi Wang Feb. 20, 2024, 9:07 a.m. UTC | #3
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.
Ankit Agrawal Feb. 20, 2024, 9:08 a.m. UTC | #4
>>>> 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.
David Hildenbrand Feb. 20, 2024, 9:09 a.m. UTC | #5
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 :) ).
Zhi Wang Feb. 20, 2024, 9:33 a.m. UTC | #6
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
>
Ankit Agrawal Feb. 20, 2024, 9:40 a.m. UTC | #7
>>>>> 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 mbox series

Patch

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)