[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
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; > + 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.) > endmenu > diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile > index 5aae66e..767009c 100644 > --- a/drivers/xen/Makefile > +++ b/drivers/xen/Makefile > @@ -39,3 +39,4 @@ xen-gntalloc-y := gntalloc.o > xen-privcmd-y := privcmd.o privcmd-buf.o > obj-$(CONFIG_XEN_FRONT_PGDIR_SHBUF) += xen-front-pgdir-shbuf.o > obj-$(CONFIG_XEN_UNPOPULATED_ALLOC) += unpopulated-alloc.o > +obj-$(CONFIG_XEN_VIRTIO) += xen-virtio.o > diff --git a/drivers/xen/xen-virtio.c b/drivers/xen/xen-virtio.c > new file mode 100644 > index 00000000..cfd5eda > --- /dev/null > +++ b/drivers/xen/xen-virtio.c > @@ -0,0 +1,177 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/****************************************************************************** > + * Xen virtio driver - enables using virtio devices in Xen guests. > + * > + * Copyright (c) 2021, Juergen Gross <jgross@xxxxxxxx> > + */ > + > +#include <linux/module.h> > +#include <linux/dma-map-ops.h> > +#include <linux/pci.h> > +#include <linux/pfn.h> > +#include <linux/virtio_config.h> > +#include <xen/xen.h> > +#include <xen/grant_table.h> > + > +#define XEN_GRANT_ADDR_OFF 0x8000000000000000ULL NIT: (1ULL << 31) > +static inline dma_addr_t grant_to_dma(grant_ref_t grant) > +{ > + return XEN_GRANT_ADDR_OFF | ((dma_addr_t)grant << PAGE_SHIFT); > +} > + > +static inline grant_ref_t dma_to_grant(dma_addr_t dma) > +{ > + return (grant_ref_t)((dma & ~XEN_GRANT_ADDR_OFF) >> PAGE_SHIFT); > +} > + > +/* > + * DMA ops for Xen virtio frontends. > + * > + * Used to act as a kind of software IOMMU for Xen guests by using grants as > + * DMA addresses. > + * Such a DMA address is formed by using the grant reference as a frame > + * number and setting the highest address bit (this bit is for the backend > + * to be able to distinguish it from e.g. a mmio address). > + * > + * Note that for now we hard wire dom0 to be the backend domain. In order to > + * support any domain as backend we'd need to add a way to communicate the > + * domid of this backend, e.g. via Xenstore or via the PCI-device's config > + * space. I would add device tree as possible way of domid communication > + */ > +static void *xen_virtio_dma_alloc(struct device *dev, size_t size, > + dma_addr_t *dma_handle, gfp_t gfp, > + unsigned long attrs) > +{ > + unsigned int n_pages = PFN_UP(size); > + unsigned int i; > + unsigned long pfn; > + grant_ref_t grant; > + void *ret; > + > + ret = (void *)__get_free_pages(gfp, get_order(size)); > + if (!ret) > + return NULL; > + > + pfn = virt_to_pfn(ret); > + > + if (gnttab_alloc_grant_reference_seq(n_pages, &grant)) { > + free_pages((unsigned long)ret, get_order(size)); > + return NULL; > + } > + > + for (i = 0; i < n_pages; i++) { > + gnttab_grant_foreign_access_ref(grant + i, 0, > + pfn_to_gfn(pfn + i), 0); > + } > + > + *dma_handle = grant_to_dma(grant); > + > + return ret; > +} > + > +static void xen_virtio_dma_free(struct device *dev, size_t size, void *vaddr, > + dma_addr_t dma_handle, unsigned long attrs) > +{ > + unsigned int n_pages = PFN_UP(size); > + unsigned int i; > + grant_ref_t grant; > + > + grant = dma_to_grant(dma_handle); > + > + for (i = 0; i < n_pages; i++) > + gnttab_end_foreign_access_ref(grant + i); > + > + gnttab_free_grant_reference_seq(grant, n_pages); > + > + free_pages((unsigned long)vaddr, get_order(size)); > +} > + > +static struct page *xen_virtio_dma_alloc_pages(struct device *dev, size_t > size, > + dma_addr_t *dma_handle, > + enum dma_data_direction dir, > + gfp_t gfp) > +{ > + WARN_ONCE(1, "xen_virtio_dma_alloc_pages size %ld\n", size); > + return NULL; > +} > + > +static void xen_virtio_dma_free_pages(struct device *dev, size_t size, > + struct page *vaddr, dma_addr_t dma_handle, > + enum dma_data_direction dir) > +{ > + WARN_ONCE(1, "xen_virtio_dma_free_pages size %ld\n", size); > +} > + > +static dma_addr_t xen_virtio_dma_map_page(struct device *dev, struct page > *page, > + unsigned long offset, size_t size, > + enum dma_data_direction dir, > + unsigned long attrs) > +{ > + grant_ref_t grant; > + > + if (gnttab_alloc_grant_references(1, &grant)) > + return 0; > + > + gnttab_grant_foreign_access_ref(grant, 0, xen_page_to_gfn(page), > + dir == DMA_TO_DEVICE); > + return grant_to_dma(grant) + offset; > +} > + > +static void xen_virtio_dma_unmap_page(struct device *dev, dma_addr_t > dma_handle, > + size_t size, enum dma_data_direction dir, > + unsigned long attrs) > +{ > + grant_ref_t grant; > + > + grant = dma_to_grant(dma_handle); > + > + gnttab_end_foreign_access_ref(grant); > + > + gnttab_free_grant_reference(grant); > +} > + > +static int xen_virtio_dma_map_sg(struct device *dev, struct scatterlist *sg, > + int nents, enum dma_data_direction dir, > + unsigned long attrs) > +{ > + WARN_ONCE(1, "xen_virtio_dma_map_sg nents %d\n", nents); > + return -EINVAL; > +} > + > +static void xen_virtio_dma_unmap_sg(struct device *dev, struct scatterlist > *sg, > + int nents, enum dma_data_direction dir, > + unsigned long attrs) > +{ > + WARN_ONCE(1, "xen_virtio_dma_unmap_sg nents %d\n", nents); > +} You can implement xen_virtio_dma_map_sg and xen_virtio_dma_unmap_sg based on xen_virtio_dma_map_page and xen_virtio_dma_unmap_page, like we do in drivers/xen/swiotlb-xen.c. > +static int xen_virtio_dma_dma_supported(struct device *dev, u64 mask) > +{ > + return 1; > +} > + > +static const struct dma_map_ops xen_virtio_dma_ops = { > + .alloc = xen_virtio_dma_alloc, > + .free = xen_virtio_dma_free, > + .alloc_pages = xen_virtio_dma_alloc_pages, > + .free_pages = xen_virtio_dma_free_pages, > + .mmap = dma_common_mmap, > + .get_sgtable = dma_common_get_sgtable, > + .map_page = xen_virtio_dma_map_page, > + .unmap_page = xen_virtio_dma_unmap_page, > + .map_sg = xen_virtio_dma_map_sg, > + .unmap_sg = xen_virtio_dma_unmap_sg, > + .dma_supported = xen_virtio_dma_dma_supported, > +}; > + > +void xen_virtio_setup_dma_ops(struct device *dev) > +{ > + dev->dma_ops = &xen_virtio_dma_ops; > +} > +EXPORT_SYMBOL_GPL(xen_virtio_setup_dma_ops); > + > +MODULE_DESCRIPTION("Xen virtio support driver"); > +MODULE_AUTHOR("Juergen Gross <jgross@xxxxxxxx>"); > +MODULE_LICENSE("GPL"); > diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h > index a3584a3..ae3c1bc 100644 > --- a/include/xen/xen-ops.h > +++ b/include/xen/xen-ops.h > @@ -221,4 +221,12 @@ static inline void xen_preemptible_hcall_end(void) { } > > #endif /* CONFIG_XEN_PV && !CONFIG_PREEMPTION */ > > +#ifdef CONFIG_XEN_VIRTIO > +void xen_virtio_setup_dma_ops(struct device *dev); > +#else > +static inline void xen_virtio_setup_dma_ops(struct device *dev) > +{ > +} > +#endif /* CONFIG_XEN_VIRTIO */ > + > #endif /* INCLUDE_XEN_OPS_H */ > -- > 2.7.4 >
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |