[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH 2/6] virtio: add option to restrict memory access under Xen
- To: Juergen Gross <jgross@xxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>
- From: Oleksandr <olekstysh@xxxxxxxxx>
- Date: Tue, 19 Apr 2022 09:37:32 +0300
- Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx, x86@xxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>, Andy Lutomirski <luto@xxxxxxxxxx>, Peter Zijlstra <peterz@xxxxxxxxxxxxx>, Thomas Gleixner <tglx@xxxxxxxxxxxxx>, Ingo Molnar <mingo@xxxxxxxxxx>, Borislav Petkov <bp@xxxxxxxxx>, "H. Peter Anvin" <hpa@xxxxxxxxx>, Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>, linux-arm-kernel@xxxxxxxxxxxxxxxxxxx, Christoph Hellwig <hch@xxxxxxxxxxxxx>, "Michael S. Tsirkin" <mst@xxxxxxxxxx>
- Delivery-date: Tue, 19 Apr 2022 06:37:42 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
Hello Stefano, Juergen
On 19.04.22 09:21, Juergen Gross wrote:
On 18.04.22 21:11, Stefano Stabellini wrote:
On Sun, 17 Apr 2022, Oleksandr wrote:
On 16.04.22 01:01, Stefano Stabellini wrote:
On Thu, 14 Apr 2022, Oleksandr Tyshchenko wrote:
From: Juergen Gross <jgross@xxxxxxxx>
In order to support virtio in Xen guests add a config option enabling
the user to specify whether in all Xen guests virtio should be
able to
access memory via Xen grant mappings only on the host side.
This applies to fully virtualized guests only, as for paravirtualized
guests this is mandatory.
This requires to switch arch_has_restricted_virtio_memory_access()
from a pure stub to a real function on x86 systems (Arm systems are
not covered by now).
Add the needed functionality by providing a special set of DMA ops
handling the needed grant operations for the I/O pages.
Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
---
arch/x86/mm/init.c | 15 ++++
arch/x86/mm/mem_encrypt.c | 5 --
arch/x86/xen/Kconfig | 9 +++
drivers/xen/Kconfig | 20 ++++++
drivers/xen/Makefile | 1 +
drivers/xen/xen-virtio.c | 177
++++++++++++++++++++++++++++++++++++++++++++++
include/xen/xen-ops.h | 8 +++
7 files changed, 230 insertions(+), 5 deletions(-)
create mode 100644 drivers/xen/xen-virtio.c
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index d8cfce2..526a3b2 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -8,6 +8,8 @@
#include <linux/kmemleak.h>
#include <linux/sched/task.h>
+#include <xen/xen.h>
+
#include <asm/set_memory.h>
#include <asm/e820/api.h>
#include <asm/init.h>
@@ -1065,3 +1067,16 @@ unsigned long max_swapfile_size(void)
return pages;
}
#endif
+
+#ifdef CONFIG_ARCH_HAS_RESTRICTED_VIRTIO_MEMORY_ACCESS
+int arch_has_restricted_virtio_memory_access(void)
+{
+ if (IS_ENABLED(CONFIG_XEN_PV_VIRTIO) && xen_pv_domain())
+ return 1;
+ if (IS_ENABLED(CONFIG_XEN_HVM_VIRTIO_GRANT) && xen_hvm_domain())
+ return 1;
I think these two checks could be moved to a separate function in a
Xen
header, e.g. xen_restricted_virtio_memory_access, and here you could
just
if (xen_restricted_virtio_memory_access())
return 1;
Agree, will do
+ return cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT);
+}
+EXPORT_SYMBOL_GPL(arch_has_restricted_virtio_memory_access);
+#endif
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index 50d2099..dda020f 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -77,8 +77,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/xen/Kconfig b/arch/x86/xen/Kconfig
index 85246dd..dffdffd 100644
--- a/arch/x86/xen/Kconfig
+++ b/arch/x86/xen/Kconfig
@@ -92,3 +92,12 @@ config XEN_DOM0
select X86_X2APIC if XEN_PVH && X86_64
help
Support running as a Xen Dom0 guest.
+
+config XEN_PV_VIRTIO
+ bool "Xen virtio support for PV guests"
+ depends on XEN_VIRTIO && XEN_PV
+ default y
+ help
+ Support virtio for running as a paravirtualized guest. This
will
+ need support on the backend side (qemu or kernel, depending
on the
+ virtio device types used).
diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index 120d32f..fc61f7a 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -335,4 +335,24 @@ config XEN_UNPOPULATED_ALLOC
having to balloon out RAM regions in order to obtain
physical memory
space to create such mappings.
+config XEN_VIRTIO
+ bool "Xen virtio support"
+ default n
+ depends on VIRTIO && DMA_OPS
+ select ARCH_HAS_RESTRICTED_VIRTIO_MEMORY_ACCESS
+ help
+ Enable virtio support for running as Xen guest. Depending
on the
+ guest type this will require special support on the backend
side
+ (qemu or kernel, depending on the virtio device types used).
+
+config XEN_HVM_VIRTIO_GRANT
+ bool "Require virtio for fully virtualized guests to use grant
mappings"
+ depends on XEN_VIRTIO && X86_64
+ default y
+ help
+ Require virtio for fully virtualized 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
+ (qemu or kernel, depending on the virtio device types used).
I don't think we need 3 visible kconfig options for this.
In fact, I would only add one: XEN_VIRTIO. We can have any X86 (or
ARM)
specific dependencies in the "depends" line under XEN_VIRTIO. And I
don't think we need XEN_HVM_VIRTIO_GRANT as a kconfig option
necessarely. It doesn't seem like some we want as build time
option. At
most, it could be a runtime option (like a command line) or a debug
option (like an #define at the top of the source file.)
I don't know what was the initial idea of having and extra
XEN_HVM_VIRTIO and
XEN_PV_VIRTIO options, but taking into the account that
they are only used in arch_has_restricted_virtio_memory_access()
currently, I
share your opinion regarding a single XEN_VIRTIO option.
Looking ahead (including changes in the commit #4), we can imagine the
resulting option:
config XEN_VIRTIO
bool "Xen virtio support"
default n
depends on VIRTIO && DMA_OPS
depends on (X86_64 || ARM || ARM64)
select ARCH_HAS_RESTRICTED_VIRTIO_MEMORY_ACCESS
help
Enable virtio support for running as Xen guest. Depending on the
guest type this will require special support on the backend side
(qemu or kernel, depending on the virtio device types used).
and then arch_has_restricted_virtio_memory_access() per arch:
1. x86:
int arch_has_restricted_virtio_memory_access(void)
{
return (xen_has_restricted_virtio_memory_access() ||
cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT));
}
2. Arm:
int arch_has_restricted_virtio_memory_access(void)
{
return xen_has_restricted_virtio_memory_access();
}
3. xen.h:
static inline int xen_has_restricted_virtio_memory_access(void)
{
if (IS_ENABLED(CONFIG_XEN_VIRTIO) && (xen_pv_domain() ||
xen_hvm_domain()))
return 1;
return 0;
}
Actually, as domain type on Arm is always XEN_HVM_DOMAIN, we could
probably
have the following on Arm:
int arch_has_restricted_virtio_memory_access(void)
{
return IS_ENABLED(CONFIG_XEN_VIRTIO);
}
but I would prefer not to diverge and use common
xen_has_restricted_virtio_memory_access().
Any thoughts?
Yes, I would also prefer not to diverge between the x86 and arm versions
of xen_has_restricted_virtio_memory_access. But what case are we trying
to catch with (xen_pv_domain() || xen_hvm_domain()) ? Even on x86, it is
not going to leave much out. Is it really meant only to exclude pvh
domains?
Good question. By leaving (xen_pv_domain() || xen_hvm_domain()) here I
tried to retain what the *initial* version of
arch_has_restricted_virtio_memory_access() covered.
It wouldn't exclude pvh domains.
ok
I have the feeling that we could turn this check into:
static inline int xen_has_restricted_virtio_memory_access(void)
{
return IS_ENABLED(CONFIG_XEN_VIRTIO) && xen_domain();
}
even on x86, but one of the xen/x86 maintainers should confirm.
I do confirm this is better and functionally equivalent.
Perfect, thank you for confirming. Will use that check.
Juergen
--
Regards,
Oleksandr Tyshchenko
|
