2022-04-27 16:02:52

by Juergen Gross

[permalink] [raw]
Subject: [PATCH v2 0/2] kernel: add new infrastructure for platform_has() support

In another patch series [1] the need has come up to have support for
a generic feature flag infrastructure.

This patch series is introducing that infrastructure and adds the first
use case.

I have decided to use a similar interface as the already known x86
cpu_has() function. As the new infrastructure is meant to be usable for
general and arch-specific feature flags, the flags are being spread
between a general bitmap and an arch specific one.

The bitmaps start all being zero, single features can be set or reset
at any time by using the related platform_[re]set_feature() functions.

The platform_has() function is using a simple test_bit() call for now,
further optimization might be added when needed.

[1]: https://lore.kernel.org/lkml/[email protected]/T/#t

Juergen Gross (2):
kernel: add platform_has() infrastructure
virtio: replace arch_has_restricted_virtio_memory_access()

MAINTAINERS | 8 ++++++++
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_amd.c | 4 ++++
drivers/virtio/Kconfig | 6 ------
drivers/virtio/virtio.c | 5 ++---
include/asm-generic/Kbuild | 1 +
include/asm-generic/platform-feature.h | 8 ++++++++
include/linux/platform-feature.h | 16 +++++++++++++++
include/linux/virtio_config.h | 9 ---------
kernel/Makefile | 2 +-
kernel/platform-feature.c | 27 ++++++++++++++++++++++++++
15 files changed, 74 insertions(+), 38 deletions(-)
create mode 100644 include/asm-generic/platform-feature.h
create mode 100644 include/linux/platform-feature.h
create mode 100644 kernel/platform-feature.c

--
2.34.1


2022-04-27 16:02:57

by Juergen Gross

[permalink] [raw]
Subject: [PATCH v2 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 <[email protected]>
---
V2:
- move setting of PLATFORM_VIRTIO_RESTRICTED_MEM_ACCESS in SEV case
to sev_setup_arch().
---
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_amd.c | 4 ++++
drivers/virtio/Kconfig | 6 ------
drivers/virtio/virtio.c | 5 ++---
include/linux/platform-feature.h | 3 ++-
include/linux/virtio_config.h | 9 ---------
10 files changed, 15 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..2c3b451813ed 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(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..965518b9d14b 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(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_amd.c b/arch/x86/mm/mem_encrypt_amd.c
index 6169053c2854..39b71084d36b 100644
--- a/arch/x86/mm/mem_encrypt_amd.c
+++ b/arch/x86/mm/mem_encrypt_amd.c
@@ -21,6 +21,7 @@
#include <linux/dma-mapping.h>
#include <linux/virtio_config.h>
#include <linux/cc_platform.h>
+#include <linux/platform-feature.h>

#include <asm/tlbflush.h>
#include <asm/fixmap.h>
@@ -206,6 +207,9 @@ void __init sev_setup_arch(void)
size = total_mem * 6 / 100;
size = clamp_val(size, IO_TLB_DEFAULT_SIZE, SZ_1G);
swiotlb_adjust_size(size);
+
+ /* Set restricted memory access for virtio. */
+ platform_set(PLATFORM_VIRTIO_RESTRICTED_MEM_ACCESS);
}

static unsigned long pg_level_to_pfn(int level, pte_t *kpte, pgprot_t *ret_prot)
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 6ed859928b97..5e2f08554b38 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 + PLATFORM_ARCH_FEAT_N)
+#define PLATFORM_FEAT_N (1 + PLATFORM_ARCH_FEAT_N)

void platform_set(unsigned int feature);
void platform_clear(unsigned int feature);
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

2022-04-27 17:02:03

by Michael Kelley (LINUX)

[permalink] [raw]
Subject: RE: [PATCH v2 2/2] virtio: replace arch_has_restricted_virtio_memory_access()

From: Juergen Gross <[email protected]> Sent: Wednesday, April 27, 2022 8:34 AM
>
> 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 <[email protected]>
> ---
> V2:
> - move setting of PLATFORM_VIRTIO_RESTRICTED_MEM_ACCESS in SEV case
> to sev_setup_arch().
> ---
> 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_amd.c | 4 ++++
> drivers/virtio/Kconfig | 6 ------
> drivers/virtio/virtio.c | 5 ++---
> include/linux/platform-feature.h | 3 ++-
> include/linux/virtio_config.h | 9 ---------
> 10 files changed, 15 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..2c3b451813ed 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(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..965518b9d14b 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(PLATFORM_VIRTIO_RESTRICTED_MEM_ACCESS);
> + }
> }
> }
>

Unless I'm misunderstanding something, the Hyper-V specific change isn't
needed. Hyper-V doesn't support virtio in the first place, so it's a bit
unexpected be setting a virtio-related flag in Hyper-V code. Also, Hyper-V
guests call sev_setup_arch() with CC_ATTR_GUEST_MEM_ENCRYPT set,
so this virtio-related flag will get set anyway via that path.

Michael

> 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_amd.c b/arch/x86/mm/mem_encrypt_amd.c
> index 6169053c2854..39b71084d36b 100644
> --- a/arch/x86/mm/mem_encrypt_amd.c
> +++ b/arch/x86/mm/mem_encrypt_amd.c
> @@ -21,6 +21,7 @@
> #include <linux/dma-mapping.h>
> #include <linux/virtio_config.h>
> #include <linux/cc_platform.h>
> +#include <linux/platform-feature.h>
>
> #include <asm/tlbflush.h>
> #include <asm/fixmap.h>
> @@ -206,6 +207,9 @@ void __init sev_setup_arch(void)
> size = total_mem * 6 / 100;
> size = clamp_val(size, IO_TLB_DEFAULT_SIZE, SZ_1G);
> swiotlb_adjust_size(size);
> +
> + /* Set restricted memory access for virtio. */
> + platform_set(PLATFORM_VIRTIO_RESTRICTED_MEM_ACCESS);
> }
>
> static unsigned long pg_level_to_pfn(int level, pte_t *kpte, pgprot_t *ret_prot)
> 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 6ed859928b97..5e2f08554b38 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 +
> PLATFORM_ARCH_FEAT_N)
> +#define PLATFORM_FEAT_N (1 +
> PLATFORM_ARCH_FEAT_N)
>
> void platform_set(unsigned int feature);
> void platform_clear(unsigned int feature);
> 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

2022-04-27 18:53:40

by Juergen Gross

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] virtio: replace arch_has_restricted_virtio_memory_access()

On 27.04.22 18:30, Michael Kelley (LINUX) wrote:
> From: Juergen Gross <[email protected]> Sent: Wednesday, April 27, 2022 8:34 AM
>>
>> 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 <[email protected]>
>> ---
>> V2:
>> - move setting of PLATFORM_VIRTIO_RESTRICTED_MEM_ACCESS in SEV case
>> to sev_setup_arch().
>> ---
>> 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_amd.c | 4 ++++
>> drivers/virtio/Kconfig | 6 ------
>> drivers/virtio/virtio.c | 5 ++---
>> include/linux/platform-feature.h | 3 ++-
>> include/linux/virtio_config.h | 9 ---------
>> 10 files changed, 15 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..2c3b451813ed 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(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..965518b9d14b 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(PLATFORM_VIRTIO_RESTRICTED_MEM_ACCESS);
>> + }
>> }
>> }
>>
>
> Unless I'm misunderstanding something, the Hyper-V specific change isn't
> needed. Hyper-V doesn't support virtio in the first place, so it's a bit
> unexpected be setting a virtio-related flag in Hyper-V code. Also, Hyper-V
> guests call sev_setup_arch() with CC_ATTR_GUEST_MEM_ENCRYPT set,
> so this virtio-related flag will get set anyway via that path.

Okay, thanks. Will drop that chunk then.


Juergen


Attachments:
OpenPGP_0xB0DE9DD628BF132F.asc (3.08 kB)
OpenPGP public key
OpenPGP_signature (505.00 B)
OpenPGP digital signature
Download all attachments

2022-04-27 20:42:57

by Oleksandr Tyshchenko

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] virtio: replace arch_has_restricted_virtio_memory_access()


On 27.04.22 18:33, Juergen Gross wrote:


Hello Juergen, all

> 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 <[email protected]>
> ---
> V2:
> - move setting of PLATFORM_VIRTIO_RESTRICTED_MEM_ACCESS in SEV case
> to sev_setup_arch().


V2 works as fine as V1 did. I have tested on Arm64 in the context of
xen-virtio enabling work [1]. Thank you!


Just small NIT below.


> ---
> 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_amd.c | 4 ++++
> drivers/virtio/Kconfig | 6 ------
> drivers/virtio/virtio.c | 5 ++---
> include/linux/platform-feature.h | 3 ++-
> include/linux/virtio_config.h | 9 ---------
> 10 files changed, 15 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..2c3b451813ed 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(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..965518b9d14b 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(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);

I assume, everywhere where <linux/virtio_config.h> was specifically
included only for sake of arch_has_restricted_virtio_memory_access(),
the inclusion of <linux/virtio_config.h>

could be removed now.


> diff --git a/arch/x86/mm/mem_encrypt_amd.c b/arch/x86/mm/mem_encrypt_amd.c
> index 6169053c2854..39b71084d36b 100644
> --- a/arch/x86/mm/mem_encrypt_amd.c
> +++ b/arch/x86/mm/mem_encrypt_amd.c
> @@ -21,6 +21,7 @@
> #include <linux/dma-mapping.h>
> #include <linux/virtio_config.h>
> #include <linux/cc_platform.h>
> +#include <linux/platform-feature.h>
>
> #include <asm/tlbflush.h>
> #include <asm/fixmap.h>
> @@ -206,6 +207,9 @@ void __init sev_setup_arch(void)
> size = total_mem * 6 / 100;
> size = clamp_val(size, IO_TLB_DEFAULT_SIZE, SZ_1G);
> swiotlb_adjust_size(size);
> +
> + /* Set restricted memory access for virtio. */
> + platform_set(PLATFORM_VIRTIO_RESTRICTED_MEM_ACCESS);
> }
>
> static unsigned long pg_level_to_pfn(int level, pte_t *kpte, pgprot_t *ret_prot)
> 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 6ed859928b97..5e2f08554b38 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 + PLATFORM_ARCH_FEAT_N)

I would add a sentence describing the purpose of that "common" feature.


> +#define PLATFORM_FEAT_N (1 + PLATFORM_ARCH_FEAT_N)
>
> void platform_set(unsigned int feature);
> void platform_clear(unsigned int feature);
> 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 */


[1]
https://lore.kernel.org/lkml/[email protected]/T/#mf3eaee90da6bb2c52a4c4b36b9989dacc4e9c597


--
Regards,

Oleksandr Tyshchenko