Juergen Gross
2022-Apr-26 13:40 UTC
[PATCH 2/2] virtio: replace arch_has_restricted_virtio_memory_access()
Instead of using arch_has_restricted_virtio_memory_access() together
with CONFIG_ARCH_HAS_RESTRICTED_VIRTIO_MEMORY_ACCESS, replace those
with platform_has() and a new platform feature
PLATFORM_VIRTIO_RESTRICTED_MEM_ACCESS.
Signed-off-by: Juergen Gross <jgross at suse.com>
---
I've only done a compile test on x86 for now, as I can't test these
changes easily (SEV might be doable for me, but s390 isn't).
---
arch/s390/Kconfig | 1 -
arch/s390/mm/init.c | 13 +++----------
arch/x86/Kconfig | 1 -
arch/x86/kernel/cpu/mshyperv.c | 5 ++++-
arch/x86/mm/mem_encrypt.c | 6 ------
arch/x86/mm/mem_encrypt_identity.c | 5 +++++
drivers/virtio/Kconfig | 6 ------
drivers/virtio/virtio.c | 5 ++---
include/linux/platform-feature.h | 3 ++-
include/linux/virtio_config.h | 9 ---------
10 files changed, 16 insertions(+), 38 deletions(-)
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index e084c72104f8..f97a22ae69a8 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -772,7 +772,6 @@ menu "Virtualization"
config PROTECTED_VIRTUALIZATION_GUEST
def_bool n
prompt "Protected virtualization guest support"
- select ARCH_HAS_RESTRICTED_VIRTIO_MEMORY_ACCESS
help
Select this option, if you want to be able to run this
kernel as a protected virtualization KVM guest.
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index 86ffd0d51fd5..8e4fa10c6b12 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -31,6 +31,7 @@
#include <linux/cma.h>
#include <linux/gfp.h>
#include <linux/dma-direct.h>
+#include <linux/platform-feature.h>
#include <asm/processor.h>
#include <linux/uaccess.h>
#include <asm/pgalloc.h>
@@ -168,22 +169,14 @@ bool force_dma_unencrypted(struct device *dev)
return is_prot_virt_guest();
}
-#ifdef CONFIG_ARCH_HAS_RESTRICTED_VIRTIO_MEMORY_ACCESS
-
-int arch_has_restricted_virtio_memory_access(void)
-{
- return is_prot_virt_guest();
-}
-EXPORT_SYMBOL(arch_has_restricted_virtio_memory_access);
-
-#endif
-
/* protected virtualization */
static void pv_init(void)
{
if (!is_prot_virt_guest())
return;
+ platform_set_feature(PLATFORM_VIRTIO_RESTRICTED_MEM_ACCESS);
+
/* make sure bounce buffers are shared */
swiotlb_force = SWIOTLB_FORCE;
swiotlb_init(1);
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index b0142e01002e..20ac72546ae4 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1515,7 +1515,6 @@ config X86_CPA_STATISTICS
config X86_MEM_ENCRYPT
select ARCH_HAS_FORCE_DMA_UNENCRYPTED
select DYNAMIC_PHYSICAL_MASK
- select ARCH_HAS_RESTRICTED_VIRTIO_MEMORY_ACCESS
def_bool n
config AMD_MEM_ENCRYPT
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index 4b67094215bb..435611d83895 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -19,6 +19,7 @@
#include <linux/i8253.h>
#include <linux/random.h>
#include <linux/swiotlb.h>
+#include <linux/platform-feature.h>
#include <asm/processor.h>
#include <asm/hypervisor.h>
#include <asm/hyperv-tlfs.h>
@@ -347,8 +348,10 @@ static void __init ms_hyperv_init_platform(void)
#endif
/* Isolation VMs are unenlightened SEV-based VMs, thus this check: */
if (IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT)) {
- if (hv_get_isolation_type() != HV_ISOLATION_TYPE_NONE)
+ if (hv_get_isolation_type() != HV_ISOLATION_TYPE_NONE) {
cc_set_vendor(CC_VENDOR_HYPERV);
+ platform_set_feature(PLATFORM_VIRTIO_RESTRICTED_MEM_ACCESS);
+ }
}
}
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index 50d209939c66..9b6a7c98b2b1 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -76,9 +76,3 @@ void __init mem_encrypt_init(void)
print_mem_encrypt_feature_info();
}
-
-int arch_has_restricted_virtio_memory_access(void)
-{
- return cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT);
-}
-EXPORT_SYMBOL_GPL(arch_has_restricted_virtio_memory_access);
diff --git a/arch/x86/mm/mem_encrypt_identity.c
b/arch/x86/mm/mem_encrypt_identity.c
index b43bc24d2bb6..6043ba6cd17d 100644
--- a/arch/x86/mm/mem_encrypt_identity.c
+++ b/arch/x86/mm/mem_encrypt_identity.c
@@ -40,6 +40,7 @@
#include <linux/mm.h>
#include <linux/mem_encrypt.h>
#include <linux/cc_platform.h>
+#include <linux/platform-feature.h>
#include <asm/setup.h>
#include <asm/sections.h>
@@ -566,6 +567,10 @@ void __init sme_enable(struct boot_params *bp)
} else {
/* SEV state cannot be controlled by a command line option */
sme_me_mask = me_mask;
+
+ /* Set restricted memory access for virtio. */
+ platform_set_feature(PLATFORM_VIRTIO_RESTRICTED_MEM_ACCESS);
+
goto out;
}
diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
index b5adf6abd241..a6dc8b5846fe 100644
--- a/drivers/virtio/Kconfig
+++ b/drivers/virtio/Kconfig
@@ -6,12 +6,6 @@ config VIRTIO
bus, such as CONFIG_VIRTIO_PCI, CONFIG_VIRTIO_MMIO, CONFIG_RPMSG
or CONFIG_S390_GUEST.
-config ARCH_HAS_RESTRICTED_VIRTIO_MEMORY_ACCESS
- bool
- help
- This option is selected if the architecture may need to enforce
- VIRTIO_F_ACCESS_PLATFORM
-
config VIRTIO_PCI_LIB
tristate
help
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 22f15f444f75..371e16b18381 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -5,6 +5,7 @@
#include <linux/module.h>
#include <linux/idr.h>
#include <linux/of.h>
+#include <linux/platform-feature.h>
#include <uapi/linux/virtio_ids.h>
/* Unique numbering for virtio devices. */
@@ -170,12 +171,10 @@ EXPORT_SYMBOL_GPL(virtio_add_status);
static int virtio_features_ok(struct virtio_device *dev)
{
unsigned status;
- int ret;
might_sleep();
- ret = arch_has_restricted_virtio_memory_access();
- if (ret) {
+ if (platform_has(PLATFORM_VIRTIO_RESTRICTED_MEM_ACCESS)) {
if (!virtio_has_feature(dev, VIRTIO_F_VERSION_1)) {
dev_warn(&dev->dev,
"device must provide VIRTIO_F_VERSION_1\n");
diff --git a/include/linux/platform-feature.h b/include/linux/platform-feature.h
index df393d502a4f..34b649aaa1da 100644
--- a/include/linux/platform-feature.h
+++ b/include/linux/platform-feature.h
@@ -6,7 +6,8 @@
#include <asm/platform-feature.h>
/* The platform features are starting with the architecture specific ones. */
-#define PLATFORM_FEAT_N (0 + PLATFORM_ARCH_FEAT_N)
+#define PLATFORM_VIRTIO_RESTRICTED_MEM_ACCESS 0
+#define PLATFORM_FEAT_N (1 + PLATFORM_ARCH_FEAT_N)
#define PLATFORM_FEAT_ARRAY_SZ BITS_TO_LONGS(PLATFORM_FEAT_N)
extern unsigned long platform_features[PLATFORM_FEAT_ARRAY_SZ];
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index b341dd62aa4d..79498298519d 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -559,13 +559,4 @@ static inline void virtio_cwrite64(struct virtio_device
*vdev,
_r; \
})
-#ifdef CONFIG_ARCH_HAS_RESTRICTED_VIRTIO_MEMORY_ACCESS
-int arch_has_restricted_virtio_memory_access(void);
-#else
-static inline int arch_has_restricted_virtio_memory_access(void)
-{
- return 0;
-}
-#endif /* CONFIG_ARCH_HAS_RESTRICTED_VIRTIO_MEMORY_ACCESS */
-
#endif /* _LINUX_VIRTIO_CONFIG_H */
--
2.34.1
Borislav Petkov
2022-Apr-26 17:35 UTC
[PATCH 2/2] virtio: replace arch_has_restricted_virtio_memory_access()
On Tue, Apr 26, 2022 at 03:40:21PM +0200, Juergen Gross wrote:> /* protected virtualization */ > static void pv_init(void) > { > if (!is_prot_virt_guest()) > return; > > + platform_set_feature(PLATFORM_VIRTIO_RESTRICTED_MEM_ACCESS);Kinda long-ish for my taste. I'll probably call it: platform_set() as it is implicit that it sets a feature bit.> diff --git a/arch/x86/mm/mem_encrypt_identity.c b/arch/x86/mm/mem_encrypt_identity.c > index b43bc24d2bb6..6043ba6cd17d 100644 > --- a/arch/x86/mm/mem_encrypt_identity.c > +++ b/arch/x86/mm/mem_encrypt_identity.c > @@ -40,6 +40,7 @@ > #include <linux/mm.h> > #include <linux/mem_encrypt.h> > #include <linux/cc_platform.h> > +#include <linux/platform-feature.h> > > #include <asm/setup.h> > #include <asm/sections.h> > @@ -566,6 +567,10 @@ void __init sme_enable(struct boot_params *bp) > } else { > /* SEV state cannot be controlled by a command line option */ > sme_me_mask = me_mask; > + > + /* Set restricted memory access for virtio. */ > + platform_set_feature(PLATFORM_VIRTIO_RESTRICTED_MEM_ACCESS);Huh, what does that have to do with SME? In any case, yeah, looks ok at a quick glance. It would obviously need for more people to look at it and say whether it makes sense to them and whether that's fine to have in generic code but so far, the experience with cc_platform_* says that it seems to work ok in generic code. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette