[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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 3/3] xen: don't require virtio with grants for non-PV guests

[Juergen Gross]

Commit fa1f57421e0b ("xen/virtio: Enable restricted memory access using
Xen grant mappings") introduced a new requirement for using virtio
devices: the backend now needs to support the VIRTIO_F_ACCESS_PLATFORM
feature.

This is an undue requirement for non-PV guests, as those can be operated
with existing backends without any problem, as long as those backends
are running in dom0.

Per default allow virtio devices without grant support for non-PV
guests.

On Arm require VIRTIO_F_ACCESS_PLATFORM for devices having been listed
in the device tree to use grants.

Add a new config item to always force use of grants for virtio.

Fixes: fa1f57421e0b ("xen/virtio: Enable restricted memory access using Xen grant mappings")
Reported-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- remove command line parameter (Christoph Hellwig)
V3:
- rebase to callback method
---
 arch/arm/xen/enlighten.c     |  4 +++-
 arch/x86/xen/enlighten_hvm.c |  4 +++-
 arch/x86/xen/enlighten_pv.c  |  5 ++++-
 drivers/xen/Kconfig          |  9 +++++++++
 drivers/xen/grant-dma-ops.c  | 10 ++++++++++
 include/xen/xen-ops.h        |  6 ++++++
 include/xen/xen.h            |  8 --------
 7 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 1f9c3ba32833..93c8ccbf2982 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -34,6 +34,7 @@
 #include <linux/timekeeping.h>
 #include <linux/timekeeper_internal.h>
 #include <linux/acpi.h>
+#include <linux/virtio_anchor.h>
 
 #include <linux/mm.h>
 
@@ -443,7 +444,8 @@ static int __init xen_guest_init(void)
 	if (!xen_domain())
 		return 0;
 
-	xen_set_restricted_virtio_memory_access();
+	if (IS_ENABLED(CONFIG_XEN_VIRTIO))
+		virtio_set_mem_acc_cb(xen_virtio_mem_acc);
 
 	if (!acpi_disabled)
 		xen_acpi_guest_init();
diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
index 8b71b1dd7639..28762f800596 100644
--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -4,6 +4,7 @@
 #include <linux/cpu.h>
 #include <linux/kexec.h>
 #include <linux/memblock.h>
+#include <linux/virtio_anchor.h>
 
 #include <xen/features.h>
 #include <xen/events.h>
@@ -195,7 +196,8 @@ static void __init xen_hvm_guest_init(void)
 	if (xen_pv_domain())
 		return;
 
-	xen_set_restricted_virtio_memory_access();
+	if (IS_ENABLED(CONFIG_XEN_VIRTIO_FORCE_GRANT))
+		virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc);
 
 	init_hvm_pv_info();
 
diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index e3297b15701c..5aaae8a77f55 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -31,6 +31,7 @@
 #include <linux/gfp.h>
 #include <linux/edd.h>
 #include <linux/reboot.h>
+#include <linux/virtio_anchor.h>
 
 #include <xen/xen.h>
 #include <xen/events.h>
@@ -109,7 +110,9 @@ static DEFINE_PER_CPU(struct tls_descs, shadow_tls_desc);
 
 static void __init xen_pv_init_platform(void)
 {
-	xen_set_restricted_virtio_memory_access();
+	/* PV guests can't operate virtio devices without grants. */
+	if (IS_ENABLED(CONFIG_XEN_VIRTIO))
+		virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc);
 
 	populate_extra_pte(fix_to_virt(FIX_PARAVIRT_BOOTMAP));
 
diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index bfd5f4f706bc..a65bd92121a5 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -355,4 +355,13 @@ config XEN_VIRTIO
 
 	  If in doubt, say n.
 
+config XEN_VIRTIO_FORCE_GRANT
+	bool "Require Xen virtio support to use grants"
+	depends on XEN_VIRTIO
+	help
+	  Require virtio for Xen guests to use grant mappings.
+	  This will avoid the need to give the backend the right to map all
+	  of the guest memory. This will need support on the backend side
+	  (e.g. qemu or kernel, depending on the virtio device types used).
+
 endmenu
diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c
index fc0142484001..8973fc1e9ccc 100644
--- a/drivers/xen/grant-dma-ops.c
+++ b/drivers/xen/grant-dma-ops.c
@@ -12,6 +12,8 @@
 #include <linux/of.h>
 #include <linux/pfn.h>
 #include <linux/xarray.h>
+#include <linux/virtio_anchor.h>
+#include <linux/virtio.h>
 #include <xen/xen.h>
 #include <xen/xen-ops.h>
 #include <xen/grant_table.h>
@@ -287,6 +289,14 @@ bool xen_is_grant_dma_device(struct device *dev)
 	return has_iommu;
 }
 
+bool xen_virtio_mem_acc(struct virtio_device *dev)
+{
+	if (IS_ENABLED(CONFIG_XEN_VIRTIO_FORCE_GRANT))
+		return true;
+
+	return xen_is_grant_dma_device(dev->dev.parent);
+}
+
 void xen_grant_setup_dma_ops(struct device *dev)
 {
 	struct xen_grant_dma_data *data;
diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
index 80546960f8b7..98c399a960a3 100644
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -5,6 +5,7 @@
 #include <linux/percpu.h>
 #include <linux/notifier.h>
 #include <linux/efi.h>
+#include <linux/virtio_anchor.h>
 #include <xen/features.h>
 #include <asm/xen/interface.h>
 #include <xen/interface/vcpu.h>
@@ -217,6 +218,7 @@ static inline void xen_preemptible_hcall_end(void) { }
 #ifdef CONFIG_XEN_GRANT_DMA_OPS
 void xen_grant_setup_dma_ops(struct device *dev);
 bool xen_is_grant_dma_device(struct device *dev);
+bool xen_virtio_mem_acc(struct virtio_device *dev);
 #else
 static inline void xen_grant_setup_dma_ops(struct device *dev)
 {
@@ -225,6 +227,10 @@ static inline bool xen_is_grant_dma_device(struct device *dev)
 {
 	return false;
 }
+static inline bool xen_virtio_mem_acc(struct virtio_device *dev)
+{
+	return false;
+}
 #endif /* CONFIG_XEN_GRANT_DMA_OPS */
 
 #endif /* INCLUDE_XEN_OPS_H */
diff --git a/include/xen/xen.h b/include/xen/xen.h
index ac5a144c6a65..a99bab817523 100644
--- a/include/xen/xen.h
+++ b/include/xen/xen.h
@@ -52,14 +52,6 @@ bool xen_biovec_phys_mergeable(const struct bio_vec *vec1,
 extern u64 xen_saved_max_mem_size;
 #endif
 
-#include <linux/virtio_anchor.h>
-
-static inline void xen_set_restricted_virtio_memory_access(void)
-{
-	if (IS_ENABLED(CONFIG_XEN_VIRTIO) && xen_domain())
-		virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc);
-}
-
 #ifdef CONFIG_XEN_UNPOPULATED_ALLOC
 int xen_alloc_unpopulated_pages(unsigned int nr_pages, struct page **pages);
 void xen_free_unpopulated_pages(unsigned int nr_pages, struct page **pages);
-- 
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 3/3] xen: don't require virtio with grants for non-PV guests

[Oleksandr]

On 22.06.22 09:38, Juergen Gross wrote:

Hello Juergen

> Commit fa1f57421e0b ("xen/virtio: Enable restricted memory access using
> Xen grant mappings") introduced a new requirement for using virtio
> devices: the backend now needs to support the VIRTIO_F_ACCESS_PLATFORM
> feature.
>
> This is an undue requirement for non-PV guests, as those can be operated
> with existing backends without any problem, as long as those backends
> are running in dom0.
>
> Per default allow virtio devices without grant support for non-PV
> guests.
>
> On Arm require VIRTIO_F_ACCESS_PLATFORM for devices having been listed
> in the device tree to use grants.
>
> Add a new config item to always force use of grants for virtio.
>
> Fixes: fa1f57421e0b ("xen/virtio: Enable restricted memory access using Xen grant mappings")
> Reported-by: Viresh Kumar <viresh.kumar@linaro.org>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> V2:
> - remove command line parameter (Christoph Hellwig)
> V3:
> - rebase to callback method


Patch looks good, just one NIT ...


> ---
>   arch/arm/xen/enlighten.c     |  4 +++-
>   arch/x86/xen/enlighten_hvm.c |  4 +++-
>   arch/x86/xen/enlighten_pv.c  |  5 ++++-
>   drivers/xen/Kconfig          |  9 +++++++++
>   drivers/xen/grant-dma-ops.c  | 10 ++++++++++
>   include/xen/xen-ops.h        |  6 ++++++
>   include/xen/xen.h            |  8 --------
>   7 files changed, 35 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 1f9c3ba32833..93c8ccbf2982 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -34,6 +34,7 @@
>   #include <linux/timekeeping.h>
>   #include <linux/timekeeper_internal.h>
>   #include <linux/acpi.h>
> +#include <linux/virtio_anchor.h>
>   
>   #include <linux/mm.h>
>   
> @@ -443,7 +444,8 @@ static int __init xen_guest_init(void)
>   	if (!xen_domain())
>   		return 0;
>   
> -	xen_set_restricted_virtio_memory_access();
> +	if (IS_ENABLED(CONFIG_XEN_VIRTIO))
> +		virtio_set_mem_acc_cb(xen_virtio_mem_acc);
>   
>   	if (!acpi_disabled)
>   		xen_acpi_guest_init();
> diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
> index 8b71b1dd7639..28762f800596 100644
> --- a/arch/x86/xen/enlighten_hvm.c
> +++ b/arch/x86/xen/enlighten_hvm.c
> @@ -4,6 +4,7 @@
>   #include <linux/cpu.h>
>   #include <linux/kexec.h>
>   #include <linux/memblock.h>
> +#include <linux/virtio_anchor.h>
>   
>   #include <xen/features.h>
>   #include <xen/events.h>
> @@ -195,7 +196,8 @@ static void __init xen_hvm_guest_init(void)
>   	if (xen_pv_domain())
>   		return;
>   
> -	xen_set_restricted_virtio_memory_access();
> +	if (IS_ENABLED(CONFIG_XEN_VIRTIO_FORCE_GRANT))
> +		virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc);
>   
>   	init_hvm_pv_info();
>   
> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
> index e3297b15701c..5aaae8a77f55 100644
> --- a/arch/x86/xen/enlighten_pv.c
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -31,6 +31,7 @@
>   #include <linux/gfp.h>
>   #include <linux/edd.h>
>   #include <linux/reboot.h>
> +#include <linux/virtio_anchor.h>
>   
>   #include <xen/xen.h>
>   #include <xen/events.h>
> @@ -109,7 +110,9 @@ static DEFINE_PER_CPU(struct tls_descs, shadow_tls_desc);
>   
>   static void __init xen_pv_init_platform(void)
>   {
> -	xen_set_restricted_virtio_memory_access();
> +	/* PV guests can't operate virtio devices without grants. */
> +	if (IS_ENABLED(CONFIG_XEN_VIRTIO))
> +		virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc);
>   
>   	populate_extra_pte(fix_to_virt(FIX_PARAVIRT_BOOTMAP));
>   
> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> index bfd5f4f706bc..a65bd92121a5 100644
> --- a/drivers/xen/Kconfig
> +++ b/drivers/xen/Kconfig
> @@ -355,4 +355,13 @@ config XEN_VIRTIO
>   
>   	  If in doubt, say n.
>   
> +config XEN_VIRTIO_FORCE_GRANT
> +	bool "Require Xen virtio support to use grants"
> +	depends on XEN_VIRTIO
> +	help
> +	  Require virtio for Xen guests to use grant mappings.
> +	  This will avoid the need to give the backend the right to map all
> +	  of the guest memory. This will need support on the backend side
> +	  (e.g. qemu or kernel, depending on the virtio device types used).
> +
>   endmenu
> diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c
> index fc0142484001..8973fc1e9ccc 100644
> --- a/drivers/xen/grant-dma-ops.c
> +++ b/drivers/xen/grant-dma-ops.c
> @@ -12,6 +12,8 @@
>   #include <linux/of.h>
>   #include <linux/pfn.h>
>   #include <linux/xarray.h>
> +#include <linux/virtio_anchor.h>
> +#include <linux/virtio.h>
>   #include <xen/xen.h>
>   #include <xen/xen-ops.h>
>   #include <xen/grant_table.h>
> @@ -287,6 +289,14 @@ bool xen_is_grant_dma_device(struct device *dev)
>   	return has_iommu;
>   }
>   
> +bool xen_virtio_mem_acc(struct virtio_device *dev)
> +{
> +	if (IS_ENABLED(CONFIG_XEN_VIRTIO_FORCE_GRANT))
> +		return true;
> +
> +	return xen_is_grant_dma_device(dev->dev.parent);
> +}


    ... I am thinking would it be better to move this to xen/xen-ops.h 
as grant-dma-ops.c is generic (not only for virtio, although the virtio 
is the first use-case)


diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c
index 8973fc1..fc01424 100644
--- a/drivers/xen/grant-dma-ops.c
+++ b/drivers/xen/grant-dma-ops.c
@@ -12,8 +12,6 @@
  #include <linux/of.h>
  #include <linux/pfn.h>
  #include <linux/xarray.h>
-#include <linux/virtio_anchor.h>
-#include <linux/virtio.h>
  #include <xen/xen.h>
  #include <xen/xen-ops.h>
  #include <xen/grant_table.h>
@@ -289,14 +287,6 @@ bool xen_is_grant_dma_device(struct device *dev)
         return has_iommu;
  }

-bool xen_virtio_mem_acc(struct virtio_device *dev)
-{
-       if (IS_ENABLED(CONFIG_XEN_VIRTIO_FORCE_GRANT))
-               return true;
-
-       return xen_is_grant_dma_device(dev->dev.parent);
-}
-
  void xen_grant_setup_dma_ops(struct device *dev)
  {
         struct xen_grant_dma_data *data;
diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
index 98c399a..a9ae51b 100644
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -6,6 +6,7 @@
  #include <linux/notifier.h>
  #include <linux/efi.h>
  #include <linux/virtio_anchor.h>
+#include <linux/virtio.h>
  #include <xen/features.h>
  #include <asm/xen/interface.h>
  #include <xen/interface/vcpu.h>
@@ -218,7 +219,13 @@ static inline void xen_preemptible_hcall_end(void) { }
  #ifdef CONFIG_XEN_GRANT_DMA_OPS
  void xen_grant_setup_dma_ops(struct device *dev);
  bool xen_is_grant_dma_device(struct device *dev);
-bool xen_virtio_mem_acc(struct virtio_device *dev);
+static inline bool xen_virtio_mem_acc(struct virtio_device *dev)
+{
+       if (IS_ENABLED(CONFIG_XEN_VIRTIO_FORCE_GRANT))
+               return true;
+
+       return xen_is_grant_dma_device(dev->dev.parent);
+}
  #else
  static inline void xen_grant_setup_dma_ops(struct device *dev)
  {


> +
>   void xen_grant_setup_dma_ops(struct device *dev)
>   {
>   	struct xen_grant_dma_data *data;
> diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
> index 80546960f8b7..98c399a960a3 100644
> --- a/include/xen/xen-ops.h
> +++ b/include/xen/xen-ops.h
> @@ -5,6 +5,7 @@
>   #include <linux/percpu.h>
>   #include <linux/notifier.h>
>   #include <linux/efi.h>
> +#include <linux/virtio_anchor.h>
>   #include <xen/features.h>
>   #include <asm/xen/interface.h>
>   #include <xen/interface/vcpu.h>
> @@ -217,6 +218,7 @@ static inline void xen_preemptible_hcall_end(void) { }
>   #ifdef CONFIG_XEN_GRANT_DMA_OPS
>   void xen_grant_setup_dma_ops(struct device *dev);
>   bool xen_is_grant_dma_device(struct device *dev);
> +bool xen_virtio_mem_acc(struct virtio_device *dev);
>   #else
>   static inline void xen_grant_setup_dma_ops(struct device *dev)
>   {
> @@ -225,6 +227,10 @@ static inline bool xen_is_grant_dma_device(struct device *dev)
>   {
>   	return false;
>   }
> +static inline bool xen_virtio_mem_acc(struct virtio_device *dev)
> +{
> +	return false;
> +}
>   #endif /* CONFIG_XEN_GRANT_DMA_OPS */
>   
>   #endif /* INCLUDE_XEN_OPS_H */
> diff --git a/include/xen/xen.h b/include/xen/xen.h
> index ac5a144c6a65..a99bab817523 100644
> --- a/include/xen/xen.h
> +++ b/include/xen/xen.h
> @@ -52,14 +52,6 @@ bool xen_biovec_phys_mergeable(const struct bio_vec *vec1,
>   extern u64 xen_saved_max_mem_size;
>   #endif
>   
> -#include <linux/virtio_anchor.h>
> -
> -static inline void xen_set_restricted_virtio_memory_access(void)
> -{
> -	if (IS_ENABLED(CONFIG_XEN_VIRTIO) && xen_domain())
> -		virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc);
> -}
> -
>   #ifdef CONFIG_XEN_UNPOPULATED_ALLOC
>   int xen_alloc_unpopulated_pages(unsigned int nr_pages, struct page **pages);
>   void xen_free_unpopulated_pages(unsigned int nr_pages, struct page **pages);

-- 
Regards,

Oleksandr Tyshchenko


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 3/3] xen: don't require virtio with grants for non-PV guests

[Juergen Gross]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 6067 bytes --]

On 22.06.22 11:03, Oleksandr wrote:
> 
> On 22.06.22 09:38, Juergen Gross wrote:
> 
> Hello Juergen
> 
>> Commit fa1f57421e0b ("xen/virtio: Enable restricted memory access using
>> Xen grant mappings") introduced a new requirement for using virtio
>> devices: the backend now needs to support the VIRTIO_F_ACCESS_PLATFORM
>> feature.
>>
>> This is an undue requirement for non-PV guests, as those can be operated
>> with existing backends without any problem, as long as those backends
>> are running in dom0.
>>
>> Per default allow virtio devices without grant support for non-PV
>> guests.
>>
>> On Arm require VIRTIO_F_ACCESS_PLATFORM for devices having been listed
>> in the device tree to use grants.
>>
>> Add a new config item to always force use of grants for virtio.
>>
>> Fixes: fa1f57421e0b ("xen/virtio: Enable restricted memory access using Xen 
>> grant mappings")
>> Reported-by: Viresh Kumar <viresh.kumar@linaro.org>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>> V2:
>> - remove command line parameter (Christoph Hellwig)
>> V3:
>> - rebase to callback method
> 
> 
> Patch looks good, just one NIT ...
> 
> 
>> ---
>>   arch/arm/xen/enlighten.c     |  4 +++-
>>   arch/x86/xen/enlighten_hvm.c |  4 +++-
>>   arch/x86/xen/enlighten_pv.c  |  5 ++++-
>>   drivers/xen/Kconfig          |  9 +++++++++
>>   drivers/xen/grant-dma-ops.c  | 10 ++++++++++
>>   include/xen/xen-ops.h        |  6 ++++++
>>   include/xen/xen.h            |  8 --------
>>   7 files changed, 35 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
>> index 1f9c3ba32833..93c8ccbf2982 100644
>> --- a/arch/arm/xen/enlighten.c
>> +++ b/arch/arm/xen/enlighten.c
>> @@ -34,6 +34,7 @@
>>   #include <linux/timekeeping.h>
>>   #include <linux/timekeeper_internal.h>
>>   #include <linux/acpi.h>
>> +#include <linux/virtio_anchor.h>
>>   #include <linux/mm.h>
>> @@ -443,7 +444,8 @@ static int __init xen_guest_init(void)
>>       if (!xen_domain())
>>           return 0;
>> -    xen_set_restricted_virtio_memory_access();
>> +    if (IS_ENABLED(CONFIG_XEN_VIRTIO))
>> +        virtio_set_mem_acc_cb(xen_virtio_mem_acc);
>>       if (!acpi_disabled)
>>           xen_acpi_guest_init();
>> diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
>> index 8b71b1dd7639..28762f800596 100644
>> --- a/arch/x86/xen/enlighten_hvm.c
>> +++ b/arch/x86/xen/enlighten_hvm.c
>> @@ -4,6 +4,7 @@
>>   #include <linux/cpu.h>
>>   #include <linux/kexec.h>
>>   #include <linux/memblock.h>
>> +#include <linux/virtio_anchor.h>
>>   #include <xen/features.h>
>>   #include <xen/events.h>
>> @@ -195,7 +196,8 @@ static void __init xen_hvm_guest_init(void)
>>       if (xen_pv_domain())
>>           return;
>> -    xen_set_restricted_virtio_memory_access();
>> +    if (IS_ENABLED(CONFIG_XEN_VIRTIO_FORCE_GRANT))
>> +        virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc);
>>       init_hvm_pv_info();
>> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
>> index e3297b15701c..5aaae8a77f55 100644
>> --- a/arch/x86/xen/enlighten_pv.c
>> +++ b/arch/x86/xen/enlighten_pv.c
>> @@ -31,6 +31,7 @@
>>   #include <linux/gfp.h>
>>   #include <linux/edd.h>
>>   #include <linux/reboot.h>
>> +#include <linux/virtio_anchor.h>
>>   #include <xen/xen.h>
>>   #include <xen/events.h>
>> @@ -109,7 +110,9 @@ static DEFINE_PER_CPU(struct tls_descs, shadow_tls_desc);
>>   static void __init xen_pv_init_platform(void)
>>   {
>> -    xen_set_restricted_virtio_memory_access();
>> +    /* PV guests can't operate virtio devices without grants. */
>> +    if (IS_ENABLED(CONFIG_XEN_VIRTIO))
>> +        virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc);
>>       populate_extra_pte(fix_to_virt(FIX_PARAVIRT_BOOTMAP));
>> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
>> index bfd5f4f706bc..a65bd92121a5 100644
>> --- a/drivers/xen/Kconfig
>> +++ b/drivers/xen/Kconfig
>> @@ -355,4 +355,13 @@ config XEN_VIRTIO
>>         If in doubt, say n.
>> +config XEN_VIRTIO_FORCE_GRANT
>> +    bool "Require Xen virtio support to use grants"
>> +    depends on XEN_VIRTIO
>> +    help
>> +      Require virtio for Xen guests to use grant mappings.
>> +      This will avoid the need to give the backend the right to map all
>> +      of the guest memory. This will need support on the backend side
>> +      (e.g. qemu or kernel, depending on the virtio device types used).
>> +
>>   endmenu
>> diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c
>> index fc0142484001..8973fc1e9ccc 100644
>> --- a/drivers/xen/grant-dma-ops.c
>> +++ b/drivers/xen/grant-dma-ops.c
>> @@ -12,6 +12,8 @@
>>   #include <linux/of.h>
>>   #include <linux/pfn.h>
>>   #include <linux/xarray.h>
>> +#include <linux/virtio_anchor.h>
>> +#include <linux/virtio.h>
>>   #include <xen/xen.h>
>>   #include <xen/xen-ops.h>
>>   #include <xen/grant_table.h>
>> @@ -287,6 +289,14 @@ bool xen_is_grant_dma_device(struct device *dev)
>>       return has_iommu;
>>   }
>> +bool xen_virtio_mem_acc(struct virtio_device *dev)
>> +{
>> +    if (IS_ENABLED(CONFIG_XEN_VIRTIO_FORCE_GRANT))
>> +        return true;
>> +
>> +    return xen_is_grant_dma_device(dev->dev.parent);
>> +}
> 
> 
>     ... I am thinking would it be better to move this to xen/xen-ops.h as 
> grant-dma-ops.c is generic (not only for virtio, although the virtio is the 
> first use-case)

I dislike using a function marked as inline in a function vector.

We could add another module "xen-virtio" for this purpose, but this seems
to be overkill.

I think we should just leave it here and move it later in case more real
virtio dependent stuff is being added.


Juergen

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

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 3/3] xen: don't require virtio with grants for non-PV guests

[Oleksandr]

On 22.06.22 17:35, Juergen Gross wrote:


Hello Juergen

> On 22.06.22 11:03, Oleksandr wrote:
>>
>> On 22.06.22 09:38, Juergen Gross wrote:
>>
>> Hello Juergen
>>
>>> Commit fa1f57421e0b ("xen/virtio: Enable restricted memory access using
>>> Xen grant mappings") introduced a new requirement for using virtio
>>> devices: the backend now needs to support the VIRTIO_F_ACCESS_PLATFORM
>>> feature.
>>>
>>> This is an undue requirement for non-PV guests, as those can be 
>>> operated
>>> with existing backends without any problem, as long as those backends
>>> are running in dom0.
>>>
>>> Per default allow virtio devices without grant support for non-PV
>>> guests.
>>>
>>> On Arm require VIRTIO_F_ACCESS_PLATFORM for devices having been listed
>>> in the device tree to use grants.
>>>
>>> Add a new config item to always force use of grants for virtio.
>>>
>>> Fixes: fa1f57421e0b ("xen/virtio: Enable restricted memory access 
>>> using Xen grant mappings")
>>> Reported-by: Viresh Kumar <viresh.kumar@linaro.org>
>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>> ---
>>> V2:
>>> - remove command line parameter (Christoph Hellwig)
>>> V3:
>>> - rebase to callback method
>>
>>
>> Patch looks good, just one NIT ...
>>
>>
>>> ---
>>>   arch/arm/xen/enlighten.c     |  4 +++-
>>>   arch/x86/xen/enlighten_hvm.c |  4 +++-
>>>   arch/x86/xen/enlighten_pv.c  |  5 ++++-
>>>   drivers/xen/Kconfig          |  9 +++++++++
>>>   drivers/xen/grant-dma-ops.c  | 10 ++++++++++
>>>   include/xen/xen-ops.h        |  6 ++++++
>>>   include/xen/xen.h            |  8 --------
>>>   7 files changed, 35 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
>>> index 1f9c3ba32833..93c8ccbf2982 100644
>>> --- a/arch/arm/xen/enlighten.c
>>> +++ b/arch/arm/xen/enlighten.c
>>> @@ -34,6 +34,7 @@
>>>   #include <linux/timekeeping.h>
>>>   #include <linux/timekeeper_internal.h>
>>>   #include <linux/acpi.h>
>>> +#include <linux/virtio_anchor.h>
>>>   #include <linux/mm.h>
>>> @@ -443,7 +444,8 @@ static int __init xen_guest_init(void)
>>>       if (!xen_domain())
>>>           return 0;
>>> -    xen_set_restricted_virtio_memory_access();
>>> +    if (IS_ENABLED(CONFIG_XEN_VIRTIO))
>>> +        virtio_set_mem_acc_cb(xen_virtio_mem_acc);
>>>       if (!acpi_disabled)
>>>           xen_acpi_guest_init();
>>> diff --git a/arch/x86/xen/enlighten_hvm.c 
>>> b/arch/x86/xen/enlighten_hvm.c
>>> index 8b71b1dd7639..28762f800596 100644
>>> --- a/arch/x86/xen/enlighten_hvm.c
>>> +++ b/arch/x86/xen/enlighten_hvm.c
>>> @@ -4,6 +4,7 @@
>>>   #include <linux/cpu.h>
>>>   #include <linux/kexec.h>
>>>   #include <linux/memblock.h>
>>> +#include <linux/virtio_anchor.h>
>>>   #include <xen/features.h>
>>>   #include <xen/events.h>
>>> @@ -195,7 +196,8 @@ static void __init xen_hvm_guest_init(void)
>>>       if (xen_pv_domain())
>>>           return;
>>> -    xen_set_restricted_virtio_memory_access();
>>> +    if (IS_ENABLED(CONFIG_XEN_VIRTIO_FORCE_GRANT))
>>> + virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc);
>>>       init_hvm_pv_info();
>>> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
>>> index e3297b15701c..5aaae8a77f55 100644
>>> --- a/arch/x86/xen/enlighten_pv.c
>>> +++ b/arch/x86/xen/enlighten_pv.c
>>> @@ -31,6 +31,7 @@
>>>   #include <linux/gfp.h>
>>>   #include <linux/edd.h>
>>>   #include <linux/reboot.h>
>>> +#include <linux/virtio_anchor.h>
>>>   #include <xen/xen.h>
>>>   #include <xen/events.h>
>>> @@ -109,7 +110,9 @@ static DEFINE_PER_CPU(struct tls_descs, 
>>> shadow_tls_desc);
>>>   static void __init xen_pv_init_platform(void)
>>>   {
>>> -    xen_set_restricted_virtio_memory_access();
>>> +    /* PV guests can't operate virtio devices without grants. */
>>> +    if (IS_ENABLED(CONFIG_XEN_VIRTIO))
>>> + virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc);
>>>       populate_extra_pte(fix_to_virt(FIX_PARAVIRT_BOOTMAP));
>>> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
>>> index bfd5f4f706bc..a65bd92121a5 100644
>>> --- a/drivers/xen/Kconfig
>>> +++ b/drivers/xen/Kconfig
>>> @@ -355,4 +355,13 @@ config XEN_VIRTIO
>>>         If in doubt, say n.
>>> +config XEN_VIRTIO_FORCE_GRANT
>>> +    bool "Require Xen virtio support to use grants"
>>> +    depends on XEN_VIRTIO
>>> +    help
>>> +      Require virtio for Xen guests to use grant mappings.
>>> +      This will avoid the need to give the backend the right to map 
>>> all
>>> +      of the guest memory. This will need support on the backend side
>>> +      (e.g. qemu or kernel, depending on the virtio device types 
>>> used).
>>> +
>>>   endmenu
>>> diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c
>>> index fc0142484001..8973fc1e9ccc 100644
>>> --- a/drivers/xen/grant-dma-ops.c
>>> +++ b/drivers/xen/grant-dma-ops.c
>>> @@ -12,6 +12,8 @@
>>>   #include <linux/of.h>
>>>   #include <linux/pfn.h>
>>>   #include <linux/xarray.h>
>>> +#include <linux/virtio_anchor.h>
>>> +#include <linux/virtio.h>
>>>   #include <xen/xen.h>
>>>   #include <xen/xen-ops.h>
>>>   #include <xen/grant_table.h>
>>> @@ -287,6 +289,14 @@ bool xen_is_grant_dma_device(struct device *dev)
>>>       return has_iommu;
>>>   }
>>> +bool xen_virtio_mem_acc(struct virtio_device *dev)
>>> +{
>>> +    if (IS_ENABLED(CONFIG_XEN_VIRTIO_FORCE_GRANT))
>>> +        return true;
>>> +
>>> +    return xen_is_grant_dma_device(dev->dev.parent);
>>> +}
>>
>>
>>     ... I am thinking would it be better to move this to 
>> xen/xen-ops.h as grant-dma-ops.c is generic (not only for virtio, 
>> although the virtio is the first use-case)
>
> I dislike using a function marked as inline in a function vector.
>
> We could add another module "xen-virtio" for this purpose, but this seems
> to be overkill.
>
> I think we should just leave it here and move it later in case more real
> virtio dependent stuff is being added.

I am happy with that explanation.

Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>


>
>
>
> Juergen

-- 
Regards,

Oleksandr Tyshchenko


_______________________________________________
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

[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


_______________________________________________
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

[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
> 

_______________________________________________
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.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.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel