| Message ID | 1392913301-25524-1-git-send-email-julien.grall@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Thu, 2014-02-20 at 16:21 +0000, Julien Grall wrote: > Only Xen is able to know if a device can safely avoid to use xen-swiotlb. > This patch introduce a new property "protected-devices" for the hypervisor > node which list device which the IOMMU are been correctly programmed by Xen. > > During Linux boot, Xen specific code will create an hash table which > contains all these devices. The hash table will be used in need_xen_dma_ops > to check if the Xen DMA ops needs to be used for the current device. Is it out of the question to find a field within struct device itself to store this e.g. in struct device_dma_parameters perhaps and avoid the need for a hashtable lookup. device->iommu_group might be another option, if we can create our own group? Ian. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Adding the -spec list for the generic IOMMU binding question. On Thu, 2014-02-20 at 16:21 +0000, Julien Grall wrote: > diff --git a/Documentation/devicetree/bindings/arm/xen.txt b/Documentation/devicetree/bindings/arm/xen.txt > index 0f7b9c2..ee25a57 100644 > --- a/Documentation/devicetree/bindings/arm/xen.txt > +++ b/Documentation/devicetree/bindings/arm/xen.txt > @@ -15,6 +15,8 @@ the following properties: > - interrupts: the interrupt used by Xen to inject event notifications. > A GIC node is also required. > > +- protected-devices: (optional) List of phandles to device node where the > +IOMMU has been programmed by Xen. Is there some common/generic IOMMU binding which we can reuse here? Although this isn't exactly an IOMMU it certainly has some similarities -- it is providing IOMMU like functionality (albeit a very inflexible IOMMU which you don't need to/can't actually program). I'd far rather we followed existing patterns rather than invent our own things. I'm wondering if perhaps we didn't ought to integrate this as an actual IOMMU driver, although I'm not convinced this would make sense. I'm also not sure about shovelling everything as properties under a single Xen node, should this not be its own node with compatible = "xen,(pv)iommu"? Ian. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
CC'ing Greg. On Thu, 20 Feb 2014, Ian Campbell wrote: > On Thu, 2014-02-20 at 16:21 +0000, Julien Grall wrote: > > Only Xen is able to know if a device can safely avoid to use xen-swiotlb. > > This patch introduce a new property "protected-devices" for the hypervisor > > node which list device which the IOMMU are been correctly programmed by Xen. > > > > During Linux boot, Xen specific code will create an hash table which > > contains all these devices. The hash table will be used in need_xen_dma_ops > > to check if the Xen DMA ops needs to be used for the current device. > > Is it out of the question to find a field within struct device itself to > store this e.g. in struct device_dma_parameters perhaps and avoid the > need for a hashtable lookup. > > device->iommu_group might be another option, if we can create our own > group? I agree that a field in struct device would be ideal. Greg, get_maintainer.pl points at you as main maintainer of device.h, do you have an opinion on this? -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 24 Feb 2014, gregkh@linuxfoundation.org wrote: > On Mon, Feb 24, 2014 at 12:19:11PM +0000, Stefano Stabellini wrote: > > CC'ing Greg. > > > > On Thu, 20 Feb 2014, Ian Campbell wrote: > > > On Thu, 2014-02-20 at 16:21 +0000, Julien Grall wrote: > > > > Only Xen is able to know if a device can safely avoid to use xen-swiotlb. > > > > This patch introduce a new property "protected-devices" for the hypervisor > > > > node which list device which the IOMMU are been correctly programmed by Xen. > > > > > > > > During Linux boot, Xen specific code will create an hash table which > > > > contains all these devices. The hash table will be used in need_xen_dma_ops > > > > to check if the Xen DMA ops needs to be used for the current device. > > > > > > Is it out of the question to find a field within struct device itself to > > > store this e.g. in struct device_dma_parameters perhaps and avoid the > > > need for a hashtable lookup. > > > > > > device->iommu_group might be another option, if we can create our own > > > group? > > > > I agree that a field in struct device would be ideal. > > Greg, get_maintainer.pl points at you as main maintainer of device.h, do > > you have an opinion on this? > > I need a whole lot more context here please. With a patch would be even > better so that I know exactly what you are referring to... The Xen hypervisor tells Linux which devices are protected by an SMMU, preprogrammed by Xen, so that Linux can avoid the swiotlb and bounce buffers for DMA requests involving them. The information is present on device tree and parsed at boot time by Linux. Julien is proposing to store the list of "safe" devices on an hash table in the Xen specific code (in arch/arm/xen/enlighten.c, see http://marc.info/?l=linux-kernel&m=139291370526082&w=2). Whenever Linux is about to do DMA, we would check in the hashtable to figure out whether we need to go through the swiotlb or we can simply use the native dma_ops. Ian and I were thinking that it would be much easier and faster to have a "xen_safe_device" parameter in struct device and just check for that. It doesn't actually need to be in struct device, it could simply be a flag in struct device_dma_parameters as Ian was suggesting. Julien, could you please come up with a simple patch to demonstrate the concept? -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Stefano, On 25/02/14 04:49, Stefano Stabellini wrote: > Julien, could you please come up with a simple patch to demonstrate the > concept? > Sure. I won't have time to write the patch next week. I will try to send it as soon as possible. Cheers,
diff --git a/Documentation/devicetree/bindings/arm/xen.txt b/Documentation/devicetree/bindings/arm/xen.txt index 0f7b9c2..ee25a57 100644 --- a/Documentation/devicetree/bindings/arm/xen.txt +++ b/Documentation/devicetree/bindings/arm/xen.txt @@ -15,6 +15,8 @@ the following properties: - interrupts: the interrupt used by Xen to inject event notifications. A GIC node is also required. +- protected-devices: (optional) List of phandles to device node where the +IOMMU has been programmed by Xen. Example (assuming #address-cells = <2> and #size-cells = <2>): diff --git a/arch/arm/include/asm/xen/dma-mapping.h b/arch/arm/include/asm/xen/dma-mapping.h index 002fc57..da8e4fe 100644 --- a/arch/arm/include/asm/xen/dma-mapping.h +++ b/arch/arm/include/asm/xen/dma-mapping.h @@ -5,9 +5,18 @@ extern struct dma_map_ops *xen_dma_ops; +#ifdef CONFIG_XEN +bool xen_is_protected_device(const struct device *dev); +#else +static inline bool xen_is_protected_device(const struct device *dev) +{ + return 0; +} +#endif + static inline bool need_xen_dma_ops(struct device *dev) { - return xen_initial_domain(); + return xen_initial_domain() && !xen_is_protected_device(dev); } #endif /* _ASM_ARM_XEN_DMA_MAPPING_H */ diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c index b96723e..f124c8c 100644 --- a/arch/arm/xen/enlighten.c +++ b/arch/arm/xen/enlighten.c @@ -24,6 +24,7 @@ #include <linux/cpuidle.h> #include <linux/cpufreq.h> #include <linux/cpu.h> +#include <linux/hashtable.h> #include <linux/mm.h> @@ -53,6 +54,42 @@ EXPORT_SYMBOL_GPL(xen_platform_pci_unplug); static __read_mostly int xen_events_irq = -1; +/* Hash table for list of devices protected by an IOMMU in Xen */ +#define DEV_HASH_BITS 4 +#define DEV_HASH_SIZE (1 << DEV_HASH_BITS) + +static struct hlist_head *protected_devices; + +struct protected_device +{ + struct hlist_node hlist; + struct device_node *node; +}; + +static unsigned long pdev_hash(const struct device_node *node) +{ + return (node->phandle % DEV_HASH_SIZE); +} + +bool xen_is_protected_device(const struct device *dev) +{ + const struct device_node *node = dev->of_node; + unsigned long hash; + const struct protected_device *pdev; + + if (!node->phandle) + return 0; + + hash = pdev_hash(node); + + hlist_for_each_entry(pdev, &protected_devices[hash], hlist) { + if (node == pdev->node) + return 1; + } + + return 0; +} + /* map fgmfn of domid to lpfn in the current domain */ static int map_foreign_page(unsigned long lpfn, unsigned long fgmfn, unsigned int domid) @@ -235,6 +272,8 @@ static int __init xen_guest_init(void) const char *xen_prefix = "xen,xen-"; struct resource res; phys_addr_t grant_frames; + int i = 0; + struct device_node *dev; node = of_find_compatible_node(NULL, NULL, "xen,xen"); if (!node) { @@ -259,6 +298,31 @@ static int __init xen_guest_init(void) if (xen_events_irq < 0) return -ENODEV; + protected_devices = kmalloc(DEV_HASH_SIZE * sizeof (*protected_devices), + GFP_KERNEL); + if (!protected_devices) + return -ENOMEM; + + for (i = 0; i < DEV_HASH_SIZE; i++) + INIT_HLIST_HEAD(&protected_devices[i]); + + pr_info("List of protected devices:\n"); + i = 0; + while ((dev = of_parse_phandle(node, "protected-devices", i))) { + struct protected_device *pdev; + unsigned long hash; + + pr_info(" - %s\n", of_node_full_name(dev)); + pdev = kmalloc(sizeof (*pdev), GFP_KERNEL); + if (!pdev) + goto free_hash; + + pdev->node = dev; + hash = pdev_hash(dev); + hlist_add_head(&pdev->hlist, &protected_devices[hash]); + i++; + } + xen_domain_type = XEN_HVM_DOMAIN; xen_setup_features(); @@ -324,6 +388,17 @@ static int __init xen_guest_init(void) register_cpu_notifier(&xen_cpu_notifier); return 0; +free_hash: + for (i = 0; i < DEV_HASH_SIZE; i++) { + struct protected_device *pdev; + struct hlist_node *next; + + hlist_for_each_entry_safe(pdev, next, &protected_devices[i], + hlist) + kfree(pdev); + } + kfree(protected_devices); + return -ENOMEM; } early_initcall(xen_guest_init);
Only Xen is able to know if a device can safely avoid to use xen-swiotlb. This patch introduce a new property "protected-devices" for the hypervisor node which list device which the IOMMU are been correctly programmed by Xen. During Linux boot, Xen specific code will create an hash table which contains all these devices. The hash table will be used in need_xen_dma_ops to check if the Xen DMA ops needs to be used for the current device. Signed-off-by: Julien Grall <julien.grall@linaro.org> Cc: Rob Herring <robh+dt@kernel.org> Cc: Pawel Moll <pawel.moll@arm.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk> Cc: Kumar Gala <galak@codeaurora.org> Cc: Rob Landley <rob@landley.net> Cc: Russell King <linux@arm.linux.org.uk> Cc: devicetree@vger.kernel.org --- Documentation/devicetree/bindings/arm/xen.txt | 2 + arch/arm/include/asm/xen/dma-mapping.h | 11 +++- arch/arm/xen/enlighten.c | 75 +++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 1 deletion(-)