[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
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:I think these two checks could be moved to a separate function in a XenFrom: Juergen Gross <jgross@xxxxxxxx> In order to support virtio in Xen guests add a config option enablingthe user to specify whether in all Xen guests virtio should be able toaccess 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;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_ALLOChaving to balloon out RAM regions in order to obtain physical memoryspace 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 optionnecessarely. It doesn't seem like some we want as build time option. Atmost, 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 andXEN_PV_VIRTIO options, but taking into the account thatthey are only used in arch_has_restricted_virtio_memory_access() currently, Ishare 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 probablyhave 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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |