[PATCH v3 0/3] virtio: support requiring restricted access per device

[PATCH v3 0/3] virtio: support requiring restricted access per device

[Juergen Gross]

Instead of an all or nothing approach add support for requiring
restricted memory access per device.

Changes in V3:
- new patches 1 + 2
- basically complete rework of patch 3

Juergen Gross (3):
  virtio: replace restricted mem access flag with callback
  kernel: remove platform_has() infrastructure
  xen: don't require virtio with grants for non-PV guests

 MAINTAINERS                            |  8 --------
 arch/arm/xen/enlighten.c               |  4 +++-
 arch/s390/mm/init.c                    |  4 ++--
 arch/x86/mm/mem_encrypt_amd.c          |  4 ++--
 arch/x86/xen/enlighten_hvm.c           |  4 +++-
 arch/x86/xen/enlighten_pv.c            |  5 ++++-
 drivers/virtio/Kconfig                 |  4 ++++
 drivers/virtio/Makefile                |  1 +
 drivers/virtio/virtio.c                |  4 ++--
 drivers/virtio/virtio_anchor.c         | 18 +++++++++++++++++
 drivers/xen/Kconfig                    |  9 +++++++++
 drivers/xen/grant-dma-ops.c            | 10 ++++++++++
 include/asm-generic/Kbuild             |  1 -
 include/asm-generic/platform-feature.h |  8 --------
 include/linux/platform-feature.h       | 19 ------------------
 include/linux/virtio_anchor.h          | 19 ++++++++++++++++++
 include/xen/xen-ops.h                  |  6 ++++++
 include/xen/xen.h                      |  8 --------
 kernel/Makefile                        |  2 +-
 kernel/platform-feature.c              | 27 --------------------------
 20 files changed, 84 insertions(+), 81 deletions(-)
 create mode 100644 drivers/virtio/virtio_anchor.c
 delete mode 100644 include/asm-generic/platform-feature.h
 delete mode 100644 include/linux/platform-feature.h
 create mode 100644 include/linux/virtio_anchor.h
 delete mode 100644 kernel/platform-feature.c

-- 
2.35.3

[PATCH v3 1/3] virtio: replace restricted mem access flag with callback

[Juergen Gross]

Instead of having a global flag to require restricted memory access
for all virtio devices, introduce a callback which can select that
requirement on a per-device basis.

For convenience add a common function returning always true, which can
be used for use cases like SEV.

Per default use a callback always returning false.

As the callback needs to be set in early init code already, add a
virtio anchor which is builtin in case virtio is enabled.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/s390/mm/init.c              |  4 ++--
 arch/x86/mm/mem_encrypt_amd.c    |  4 ++--
 drivers/virtio/Kconfig           |  4 ++++
 drivers/virtio/Makefile          |  1 +
 drivers/virtio/virtio.c          |  4 ++--
 drivers/virtio/virtio_anchor.c   | 18 ++++++++++++++++++
 include/linux/platform-feature.h |  6 +-----
 include/linux/virtio_anchor.h    | 19 +++++++++++++++++++
 include/xen/xen.h                |  4 ++--
 9 files changed, 51 insertions(+), 13 deletions(-)
 create mode 100644 drivers/virtio/virtio_anchor.c
 create mode 100644 include/linux/virtio_anchor.h

diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index 6a0ac00d5a42..4a154a084966 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -31,7 +31,6 @@
 #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>
@@ -48,6 +47,7 @@
 #include <asm/kasan.h>
 #include <asm/dma-mapping.h>
 #include <asm/uv.h>
+#include <linux/virtio_anchor.h>
 #include <linux/virtio_config.h>
 
 pgd_t swapper_pg_dir[PTRS_PER_PGD] __section(".bss..swapper_pg_dir");
@@ -175,7 +175,7 @@ static void pv_init(void)
 	if (!is_prot_virt_guest())
 		return;
 
-	platform_set(PLATFORM_VIRTIO_RESTRICTED_MEM_ACCESS);
+	virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc);
 
 	/* make sure bounce buffers are shared */
 	swiotlb_init(true, SWIOTLB_FORCE | SWIOTLB_VERBOSE);
diff --git a/arch/x86/mm/mem_encrypt_amd.c b/arch/x86/mm/mem_encrypt_amd.c
index f6d038e2cd8e..97452688f99f 100644
--- a/arch/x86/mm/mem_encrypt_amd.c
+++ b/arch/x86/mm/mem_encrypt_amd.c
@@ -20,8 +20,8 @@
 #include <linux/bitops.h>
 #include <linux/dma-mapping.h>
 #include <linux/virtio_config.h>
+#include <linux/virtio_anchor.h>
 #include <linux/cc_platform.h>
-#include <linux/platform-feature.h>
 
 #include <asm/tlbflush.h>
 #include <asm/fixmap.h>
@@ -245,7 +245,7 @@ void __init sev_setup_arch(void)
 	swiotlb_adjust_size(size);
 
 	/* Set restricted memory access for virtio. */
-	platform_set(PLATFORM_VIRTIO_RESTRICTED_MEM_ACCESS);
+	virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc);
 }
 
 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 a6dc8b5846fe..ce93966575a1 100644
--- a/drivers/virtio/Kconfig
+++ b/drivers/virtio/Kconfig
@@ -1,6 +1,10 @@
 # SPDX-License-Identifier: GPL-2.0-only
+config VIRTIO_ANCHOR
+	bool
+
 config VIRTIO
 	tristate
+	select VIRTIO_ANCHOR
 	help
 	  This option is selected by any driver which implements the virtio
 	  bus, such as CONFIG_VIRTIO_PCI, CONFIG_VIRTIO_MMIO, CONFIG_RPMSG
diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
index 0a82d0873248..8e98d24917cc 100644
--- a/drivers/virtio/Makefile
+++ b/drivers/virtio/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_VIRTIO) += virtio.o virtio_ring.o
+obj-$(CONFIG_VIRTIO_ANCHOR) += virtio_anchor.o
 obj-$(CONFIG_VIRTIO_PCI_LIB) += virtio_pci_modern_dev.o
 obj-$(CONFIG_VIRTIO_PCI_LIB_LEGACY) += virtio_pci_legacy_dev.o
 obj-$(CONFIG_VIRTIO_MMIO) += virtio_mmio.o
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 6bace84ae37e..21e753fe1b50 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -2,10 +2,10 @@
 #include <linux/virtio.h>
 #include <linux/spinlock.h>
 #include <linux/virtio_config.h>
+#include <linux/virtio_anchor.h>
 #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. */
@@ -174,7 +174,7 @@ static int virtio_features_ok(struct virtio_device *dev)
 
 	might_sleep();
 
-	if (platform_has(PLATFORM_VIRTIO_RESTRICTED_MEM_ACCESS)) {
+	if (virtio_check_mem_acc_cb(dev)) {
 		if (!virtio_has_feature(dev, VIRTIO_F_VERSION_1)) {
 			dev_warn(&dev->dev,
 				 "device must provide VIRTIO_F_VERSION_1\n");
diff --git a/drivers/virtio/virtio_anchor.c b/drivers/virtio/virtio_anchor.c
new file mode 100644
index 000000000000..4d6a5d269b55
--- /dev/null
+++ b/drivers/virtio/virtio_anchor.c
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/virtio.h>
+#include <linux/virtio_anchor.h>
+
+bool virtio_require_restricted_mem_acc(struct virtio_device *dev)
+{
+	return true;
+}
+EXPORT_SYMBOL_GPL(virtio_require_restricted_mem_acc);
+
+static bool virtio_no_restricted_mem_acc(struct virtio_device *dev)
+{
+	return false;
+}
+
+bool (*virtio_check_mem_acc_cb)(struct virtio_device *dev) =
+	virtio_no_restricted_mem_acc;
+EXPORT_SYMBOL_GPL(virtio_check_mem_acc_cb);
diff --git a/include/linux/platform-feature.h b/include/linux/platform-feature.h
index b2f48be999fa..6ed859928b97 100644
--- a/include/linux/platform-feature.h
+++ b/include/linux/platform-feature.h
@@ -6,11 +6,7 @@
 #include <asm/platform-feature.h>
 
 /* The platform features are starting with the architecture specific ones. */
-
-/* Used to enable platform specific DMA handling for virtio devices. */
-#define PLATFORM_VIRTIO_RESTRICTED_MEM_ACCESS	(0 + PLATFORM_ARCH_FEAT_N)
-
-#define PLATFORM_FEAT_N				(1 + PLATFORM_ARCH_FEAT_N)
+#define PLATFORM_FEAT_N				(0 + PLATFORM_ARCH_FEAT_N)
 
 void platform_set(unsigned int feature);
 void platform_clear(unsigned int feature);
diff --git a/include/linux/virtio_anchor.h b/include/linux/virtio_anchor.h
new file mode 100644
index 000000000000..432e6c00b3ca
--- /dev/null
+++ b/include/linux/virtio_anchor.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_VIRTIO_ANCHOR_H
+#define _LINUX_VIRTIO_ANCHOR_H
+
+#ifdef CONFIG_VIRTIO_ANCHOR
+struct virtio_device;
+
+bool virtio_require_restricted_mem_acc(struct virtio_device *dev);
+extern bool (*virtio_check_mem_acc_cb)(struct virtio_device *dev);
+
+static inline void virtio_set_mem_acc_cb(bool (*func)(struct virtio_device *))
+{
+	virtio_check_mem_acc_cb = func;
+}
+#else
+#define virtio_set_mem_acc_cb(func) do { } while (0)
+#endif
+
+#endif /* _LINUX_VIRTIO_ANCHOR_H */
diff --git a/include/xen/xen.h b/include/xen/xen.h
index 0780a81e140d..ac5a144c6a65 100644
--- a/include/xen/xen.h
+++ b/include/xen/xen.h
@@ -52,12 +52,12 @@ bool xen_biovec_phys_mergeable(const struct bio_vec *vec1,
 extern u64 xen_saved_max_mem_size;
 #endif
 
-#include <linux/platform-feature.h>
+#include <linux/virtio_anchor.h>
 
 static inline void xen_set_restricted_virtio_memory_access(void)
 {
 	if (IS_ENABLED(CONFIG_XEN_VIRTIO) && xen_domain())
-		platform_set(PLATFORM_VIRTIO_RESTRICTED_MEM_ACCESS);
+		virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc);
 }
 
 #ifdef CONFIG_XEN_UNPOPULATED_ALLOC
-- 
2.35.3

Re: [PATCH v3 0/3] virtio: support requiring restricted access per device

[Oleksandr]

On 22.06.22 09:38, Juergen Gross wrote:

Hello Juergen

> Instead of an all or nothing approach add support for requiring
> restricted memory access per device.
>
> Changes in V3:
> - new patches 1 + 2
> - basically complete rework of patch 3
>
> Juergen Gross (3):
>    virtio: replace restricted mem access flag with callback
>    kernel: remove platform_has() infrastructure
>    xen: don't require virtio with grants for non-PV guests
>
>   MAINTAINERS                            |  8 --------
>   arch/arm/xen/enlighten.c               |  4 +++-
>   arch/s390/mm/init.c                    |  4 ++--
>   arch/x86/mm/mem_encrypt_amd.c          |  4 ++--
>   arch/x86/xen/enlighten_hvm.c           |  4 +++-
>   arch/x86/xen/enlighten_pv.c            |  5 ++++-
>   drivers/virtio/Kconfig                 |  4 ++++
>   drivers/virtio/Makefile                |  1 +
>   drivers/virtio/virtio.c                |  4 ++--
>   drivers/virtio/virtio_anchor.c         | 18 +++++++++++++++++
>   drivers/xen/Kconfig                    |  9 +++++++++
>   drivers/xen/grant-dma-ops.c            | 10 ++++++++++
>   include/asm-generic/Kbuild             |  1 -
>   include/asm-generic/platform-feature.h |  8 --------
>   include/linux/platform-feature.h       | 19 ------------------
>   include/linux/virtio_anchor.h          | 19 ++++++++++++++++++
>   include/xen/xen-ops.h                  |  6 ++++++
>   include/xen/xen.h                      |  8 --------
>   kernel/Makefile                        |  2 +-
>   kernel/platform-feature.c              | 27 --------------------------
>   20 files changed, 84 insertions(+), 81 deletions(-)
>   create mode 100644 drivers/virtio/virtio_anchor.c
>   delete mode 100644 include/asm-generic/platform-feature.h
>   delete mode 100644 include/linux/platform-feature.h
>   create mode 100644 include/linux/virtio_anchor.h
>   delete mode 100644 kernel/platform-feature.c

I have tested the series on Arm64 guest using Xen hypervisor and didn't 
notice any issues.


I assigned two virtio-mmio devices to the guest:
#1 - grant dma device (required DT binding is present, so 
xen_is_grant_dma_device() returns true), virtio-mmio modern transport 
(backend offers VIRTIO_F_VERSION_1, VIRTIO_F_ACCESS_PLATFORM)
#2 - non grant dma device (required DT binding is absent, so 
xen_is_grant_dma_device() returns false), virtio-mmio legacy transport 
(backend does not offer these flags)


# CONFIG_XEN_VIRTIO is not set

both works, and both do not use grant mappings for virtio


CONFIG_XEN_VIRTIO=y
# CONFIG_XEN_VIRTIO_FORCE_GRANT is not set

both works, #1 uses grant mappings for virtio, #2 does not use it


CONFIG_XEN_VIRTIO=y
CONFIG_XEN_VIRTIO_FORCE_GRANT=y

only #1 works and uses grant mappings for virtio, #2 was rejected by 
validation in virtio_features_ok()


You can add my:
Tested-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> # Arm64 
guest using Xen


>
-- 
Regards,

Oleksandr Tyshchenko

Re: [PATCH v3 0/3] virtio: support requiring restricted access per device

[Stefano Stabellini]

On Wed, 22 Jun 2022, Juergen Gross wrote:
> Instead of an all or nothing approach add support for requiring
> restricted memory access per device.
> 
> Changes in V3:
> - new patches 1 + 2
> - basically complete rework of patch 3
> 
> Juergen Gross (3):
>   virtio: replace restricted mem access flag with callback
>   kernel: remove platform_has() infrastructure
>   xen: don't require virtio with grants for non-PV guests


On the whole series:

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


>  MAINTAINERS                            |  8 --------
>  arch/arm/xen/enlighten.c               |  4 +++-
>  arch/s390/mm/init.c                    |  4 ++--
>  arch/x86/mm/mem_encrypt_amd.c          |  4 ++--
>  arch/x86/xen/enlighten_hvm.c           |  4 +++-
>  arch/x86/xen/enlighten_pv.c            |  5 ++++-
>  drivers/virtio/Kconfig                 |  4 ++++
>  drivers/virtio/Makefile                |  1 +
>  drivers/virtio/virtio.c                |  4 ++--
>  drivers/virtio/virtio_anchor.c         | 18 +++++++++++++++++
>  drivers/xen/Kconfig                    |  9 +++++++++
>  drivers/xen/grant-dma-ops.c            | 10 ++++++++++
>  include/asm-generic/Kbuild             |  1 -
>  include/asm-generic/platform-feature.h |  8 --------
>  include/linux/platform-feature.h       | 19 ------------------
>  include/linux/virtio_anchor.h          | 19 ++++++++++++++++++
>  include/xen/xen-ops.h                  |  6 ++++++
>  include/xen/xen.h                      |  8 --------
>  kernel/Makefile                        |  2 +-
>  kernel/platform-feature.c              | 27 --------------------------
>  20 files changed, 84 insertions(+), 81 deletions(-)
>  create mode 100644 drivers/virtio/virtio_anchor.c
>  delete mode 100644 include/asm-generic/platform-feature.h
>  delete mode 100644 include/linux/platform-feature.h
>  create mode 100644 include/linux/virtio_anchor.h
>  delete mode 100644 kernel/platform-feature.c
> 
> -- 
> 2.35.3
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

Re: [PATCH v3 0/3] virtio: support requiring restricted access per device

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 475 bytes --]

On 22.06.22 08:38, Juergen Gross wrote:
> Instead of an all or nothing approach add support for requiring
> restricted memory access per device.
> 
> Changes in V3:
> - new patches 1 + 2
> - basically complete rework of patch 3
> 
> Juergen Gross (3):
>    virtio: replace restricted mem access flag with callback
>    kernel: remove platform_has() infrastructure
>    xen: don't require virtio with grants for non-PV guests

Any further comments?


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]