Date:   Thu, 14 Apr 2022 22:19:31 +0300
From:   Oleksandr Tyshchenko <olekstysh@...il.com>
To:     xen-devel@...ts.xenproject.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Cc:     Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>,
        Stefano Stabellini <sstabellini@...nel.org>,
        Russell King <linux@...linux.org.uk>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Juergen Gross <jgross@...e.com>, Julien Grall <julien@....org>
Subject: [RFC PATCH 4/6] virtio: Various updates to xen-virtio DMA ops layer

From: Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>

In the context of current patch do the following:
1. Update code to support virtio-mmio devices
2. Introduce struct xen_virtio_data and account passed virtio devices
   (using list) as we need to store some per-device data
3. Add multi-page support for xen_virtio_dma_map(unmap)_page callbacks
4. Harden code against malicious backend
5. Change to use alloc_pages_exact() instead of __get_free_pages()
6. Introduce locking scheme to protect mappings (I am not 100% sure
   whether per-device lock is really needed)
7. Handle virtio device's DMA mask
8. Retrieve the ID of backend domain from DT for virtio-mmio device
   instead of hardcoding it.

Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>
---
 arch/arm/xen/enlighten.c |  11 +++
 drivers/xen/Kconfig      |   2 +-
 drivers/xen/xen-virtio.c | 200 ++++++++++++++++++++++++++++++++++++++++++-----
 include/xen/xen-ops.h    |   5 ++
 4 files changed, 196 insertions(+), 22 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index ec5b082..870d92f 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -409,6 +409,17 @@ int __init arch_xen_unpopulated_init(struct resource **res)
 }
 #endif
 
+#ifdef CONFIG_ARCH_HAS_RESTRICTED_VIRTIO_MEMORY_ACCESS
+int arch_has_restricted_virtio_memory_access(void)
+{
+	if (IS_ENABLED(CONFIG_XEN_HVM_VIRTIO_GRANT) && xen_hvm_domain())
+		return 1;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(arch_has_restricted_virtio_memory_access);
+#endif
+
 static void __init xen_dt_guest_init(void)
 {
 	struct device_node *xen_node;
diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index fc61f7a..56afe6a 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -347,7 +347,7 @@ config XEN_VIRTIO
 
 config XEN_HVM_VIRTIO_GRANT
 	bool "Require virtio for fully virtualized guests to use grant mappings"
-	depends on XEN_VIRTIO && X86_64
+	depends on XEN_VIRTIO && (X86_64 || ARM || ARM64)
 	default y
 	help
 	  Require virtio for fully virtualized guests to use grant mappings.
diff --git a/drivers/xen/xen-virtio.c b/drivers/xen/xen-virtio.c
index cfd5eda..c5b2ec9 100644
--- a/drivers/xen/xen-virtio.c
+++ b/drivers/xen/xen-virtio.c
@@ -7,12 +7,26 @@
 
 #include <linux/module.h>
 #include <linux/dma-map-ops.h>
+#include <linux/of.h>
 #include <linux/pci.h>
 #include <linux/pfn.h>
 #include <linux/virtio_config.h>
 #include <xen/xen.h>
 #include <xen/grant_table.h>
 
+struct xen_virtio_data {
+	/* The ID of backend domain */
+	domid_t dev_domid;
+	struct device *dev;
+	struct list_head list;
+	spinlock_t lock;
+	/* Is device behaving sane? */
+	bool broken;
+};
+
+static LIST_HEAD(xen_virtio_devices);
+static DEFINE_SPINLOCK(xen_virtio_lock);
+
 #define XEN_GRANT_ADDR_OFF	0x8000000000000000ULL
 
 static inline dma_addr_t grant_to_dma(grant_ref_t grant)
@@ -25,6 +39,25 @@ static inline grant_ref_t dma_to_grant(dma_addr_t dma)
 	return (grant_ref_t)((dma & ~XEN_GRANT_ADDR_OFF) >> PAGE_SHIFT);
 }
 
+static struct xen_virtio_data *find_xen_virtio_data(struct device *dev)
+{
+	struct xen_virtio_data *data = NULL;
+	bool found = false;
+
+	spin_lock(&xen_virtio_lock);
+
+	list_for_each_entry( data, &xen_virtio_devices, list) {
+		if (data->dev == dev) {
+			found = true;
+			break;
+		}
+	}
+
+	spin_unlock(&xen_virtio_lock);
+
+	return found ? data : NULL;
+}
+
 /*
  * DMA ops for Xen virtio frontends.
  *
@@ -43,48 +76,78 @@ 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;
+	struct xen_virtio_data *data;
+	unsigned int i, n_pages = PFN_UP(size);
 	unsigned long pfn;
 	grant_ref_t grant;
-	void *ret;
+	void *ret = NULL;
 
-	ret = (void *)__get_free_pages(gfp, get_order(size));
-	if (!ret)
+	data = find_xen_virtio_data(dev);
+	if (!data)
 		return NULL;
 
+	spin_lock(&data->lock);
+
+	if (unlikely(data->broken))
+		goto out;
+
+	ret = alloc_pages_exact(n_pages * PAGE_SIZE, gfp);
+	if (!ret)
+		goto out;
+
 	pfn = virt_to_pfn(ret);
 
 	if (gnttab_alloc_grant_reference_seq(n_pages, &grant)) {
-		free_pages((unsigned long)ret, get_order(size));
-		return NULL;
+		free_pages_exact(ret, n_pages * PAGE_SIZE);
+		ret = NULL;
+		goto out;
 	}
 
 	for (i = 0; i < n_pages; i++) {
-		gnttab_grant_foreign_access_ref(grant + i, 0,
+		gnttab_grant_foreign_access_ref(grant + i, data->dev_domid,
 						pfn_to_gfn(pfn + i), 0);
 	}
 
 	*dma_handle = grant_to_dma(grant);
 
+out:
+	spin_unlock(&data->lock);
+
 	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;
+	struct xen_virtio_data *data;
+	unsigned int i, n_pages = PFN_UP(size);
 	grant_ref_t grant;
 
+	data = find_xen_virtio_data(dev);
+	if (!data)
+		return;
+
+	spin_lock(&data->lock);
+
+	if (unlikely(data->broken))
+		goto out;
+
 	grant = dma_to_grant(dma_handle);
 
-	for (i = 0; i < n_pages; i++)
-		gnttab_end_foreign_access_ref(grant + i);
+	for (i = 0; i < n_pages; i++) {
+		if (unlikely(!gnttab_end_foreign_access_ref(grant + i))) {
+			dev_alert(dev, "Grant still in use by backend domain, disabled for further use\n");
+			data->broken = true;
+			goto out;
+		}
+	}
 
 	gnttab_free_grant_reference_seq(grant, n_pages);
 
-	free_pages((unsigned long)vaddr, get_order(size));
+	free_pages_exact(vaddr, n_pages * PAGE_SIZE);
+
+out:
+	spin_unlock(&data->lock);
 }
 
 static struct page *xen_virtio_dma_alloc_pages(struct device *dev, size_t size,
@@ -108,28 +171,71 @@ static dma_addr_t xen_virtio_dma_map_page(struct device *dev, struct page *page,
 					  enum dma_data_direction dir,
 					  unsigned long attrs)
 {
+	struct xen_virtio_data *data;
+	unsigned int i, n_pages = PFN_UP(size);
 	grant_ref_t grant;
+	dma_addr_t dma_handle = DMA_MAPPING_ERROR;
+
+	BUG_ON(dir == DMA_NONE);
+
+	data = find_xen_virtio_data(dev);
+	if (!data)
+		return DMA_MAPPING_ERROR;
+
+	spin_lock(&data->lock);
 
-	if (gnttab_alloc_grant_references(1, &grant))
-		return 0;
+	if (unlikely(data->broken))
+		goto out;
 
-	gnttab_grant_foreign_access_ref(grant, 0, xen_page_to_gfn(page),
-					dir == DMA_TO_DEVICE);
+	if (gnttab_alloc_grant_reference_seq(n_pages, &grant))
+		goto out;
 
-	return grant_to_dma(grant) + offset;
+	for (i = 0; i < n_pages; i++) {
+		gnttab_grant_foreign_access_ref(grant + i, data->dev_domid,
+				xen_page_to_gfn(page) + i, dir == DMA_TO_DEVICE);
+	}
+
+	dma_handle = grant_to_dma(grant) + offset;
+
+out:
+	spin_unlock(&data->lock);
+
+	return dma_handle;
 }
 
 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)
 {
+	struct xen_virtio_data *data;
+	unsigned int i, n_pages = PFN_UP(size);
 	grant_ref_t grant;
 
+	BUG_ON(dir == DMA_NONE);
+
+	data = find_xen_virtio_data(dev);
+	if (!data)
+		return;
+
+	spin_lock(&data->lock);
+
+	if (unlikely(data->broken))
+		goto out;
+
 	grant = dma_to_grant(dma_handle);
 
-	gnttab_end_foreign_access_ref(grant);
+	for (i = 0; i < n_pages; i++) {
+		if (unlikely(!gnttab_end_foreign_access_ref(grant + i))) {
+			dev_alert(dev, "Grant still in use by backend domain, disabled for further use\n");
+			data->broken = true;
+			goto out;
+		}
+	}
+
+	gnttab_free_grant_reference_seq(grant, n_pages);
 
-	gnttab_free_grant_reference(grant);
+out:
+	spin_unlock(&data->lock);
 }
 
 static int xen_virtio_dma_map_sg(struct device *dev, struct scatterlist *sg,
@@ -149,7 +255,7 @@ static void xen_virtio_dma_unmap_sg(struct device *dev, struct scatterlist *sg,
 
 static int xen_virtio_dma_dma_supported(struct device *dev, u64 mask)
 {
-	return 1;
+	return mask == DMA_BIT_MASK(64);
 }
 
 static const struct dma_map_ops xen_virtio_dma_ops = {
@@ -166,9 +272,61 @@ static const struct dma_map_ops xen_virtio_dma_ops = {
 	.dma_supported = xen_virtio_dma_dma_supported,
 };
 
+bool xen_is_virtio_device(struct device *dev)
+{
+	/* XXX Handle only DT devices for now */
+	if (!dev->of_node)
+		return false;
+
+	if (!of_device_is_compatible(dev->of_node, "virtio,mmio"))
+		return false;
+
+	return of_property_read_bool(dev->of_node, "xen,dev-domid");
+}
+EXPORT_SYMBOL_GPL(xen_is_virtio_device);
+
 void xen_virtio_setup_dma_ops(struct device *dev)
 {
+	struct xen_virtio_data *data;
+	uint32_t dev_domid;
+
+	data = find_xen_virtio_data(dev);
+	if (data) {
+		dev_err(dev, "xen_virtio data is already created\n");
+		return;
+	}
+
+	if (dev_is_pci(dev)) {
+		/* XXX Leave it hard wired to dom0 for now */
+		dev_domid = 0;
+	} else if (dev->of_node) {
+		if (of_property_read_u32(dev->of_node, "xen,dev-domid", &dev_domid)) {
+			dev_err(dev, "xen,dev-domid property is not present\n");
+			goto err;
+		}
+	} else
+		/* The ACPI case is not supported */
+		goto err;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data) {
+		dev_err(dev, "Сannot allocate xen_virtio data\n");
+		goto err;
+	}
+	data->dev_domid = dev_domid;
+	data->dev = dev;
+	spin_lock_init(&data->lock);
+
+	spin_lock(&xen_virtio_lock);
+	list_add(&data->list, &xen_virtio_devices);
+	spin_unlock(&xen_virtio_lock);
+
 	dev->dma_ops = &xen_virtio_dma_ops;
+
+	return;
+
+err:
+	dev_err(dev, "Сannot set up xen_virtio DMA ops, retain platform DMA ops\n");
 }
 EXPORT_SYMBOL_GPL(xen_virtio_setup_dma_ops);
 
diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
index ae3c1bc..fdbcb99 100644
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -223,10 +223,15 @@ static inline void xen_preemptible_hcall_end(void) { }
 
 #ifdef CONFIG_XEN_VIRTIO
 void xen_virtio_setup_dma_ops(struct device *dev);
+bool xen_is_virtio_device(struct device *dev);
 #else
 static inline void xen_virtio_setup_dma_ops(struct device *dev)
 {
 }
+static inline bool xen_is_virtio_device(struct device *dev)
+{
+	return false;
+}
 #endif /* CONFIG_XEN_VIRTIO */
 
 #endif /* INCLUDE_XEN_OPS_H */
-- 
2.7.4

Powered by blists - more mailing lists