| Message ID | 1394552999-14171-9-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
>>> On 11.03.14 at 16:49, Julien Grall <julien.grall@linaro.org> wrote: > Add IOMMU helpers to support device tree assignment/deassignment. This patch > introduces 2 new fields in the dt_device_node: > - is_protected: Does the device is protected by an IOMMU > - next_assigned: Pointer to the next device assigned to the same > domain > > Signed-off-by: Julien Grall <julien.grall@linaro.org> > Cc: Xiantao Zhang <xiantao.zhang@intel.com> For the modifications to existing files: Acked-by: Jan Beulich <jbeulich@suse.com> > > --- > Changes in v3: > - Remove iommu_dt_domain_{init,destroy} call in common code. Let > architecture code to call them > - Fix indentation in xen/include/xen/hvm/iommu.h > Changes in v2: > - Patch added > --- > xen/common/device_tree.c | 4 ++ > xen/drivers/passthrough/Makefile | 1 + > xen/drivers/passthrough/device_tree.c | 106 > +++++++++++++++++++++++++++++++++ > xen/include/xen/device_tree.h | 14 +++++ > xen/include/xen/hvm/iommu.h | 6 ++ > xen/include/xen/iommu.h | 16 +++++ > 6 files changed, 147 insertions(+) > create mode 100644 xen/drivers/passthrough/device_tree.c > > diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c > index 564f2bb..7c6b683 100644 > --- a/xen/common/device_tree.c > +++ b/xen/common/device_tree.c > @@ -1695,6 +1695,10 @@ static unsigned long __init unflatten_dt_node(const > void *fdt, > np->full_name = ((char *)np) + sizeof(struct dt_device_node); > /* By default dom0 owns the device */ > np->used_by = 0; > + /* By default the device is not protected */ > + np->is_protected = false; > + INIT_LIST_HEAD(&np->next_assigned); > + > if ( new_format ) > { > char *fn = np->full_name; > diff --git a/xen/drivers/passthrough/Makefile > b/xen/drivers/passthrough/Makefile > index 6e08f89..5a0a35e 100644 > --- a/xen/drivers/passthrough/Makefile > +++ b/xen/drivers/passthrough/Makefile > @@ -5,3 +5,4 @@ subdir-$(x86_64) += x86 > obj-y += iommu.o > obj-$(x86) += io.o > obj-$(HAS_PCI) += pci.o > +obj-$(HAS_DEVICE_TREE) += device_tree.o > diff --git a/xen/drivers/passthrough/device_tree.c > b/xen/drivers/passthrough/device_tree.c > new file mode 100644 > index 0000000..7384e73 > --- /dev/null > +++ b/xen/drivers/passthrough/device_tree.c > @@ -0,0 +1,106 @@ > +/* > + * xen/drivers/passthrough/arm/device_tree.c > + * > + * Code to passthrough device tree node to a guest > + * > + * Julien Grall <julien.grall@linaro.org> > + * Copyright (c) 2014 Linaro Limited. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include <xen/lib.h> > +#include <xen/sched.h> > +#include <xen/iommu.h> > +#include <xen/device_tree.h> > + > +static spinlock_t dtdevs_lock = SPIN_LOCK_UNLOCKED; > + > +int iommu_assign_dt_device(struct domain *d, struct dt_device_node *dev) > +{ > + int rc = -EBUSY; > + struct hvm_iommu *hd = domain_hvm_iommu(d); > + > + if ( !iommu_enabled || !hd->platform_ops ) > + return -EINVAL; > + > + if ( !dt_device_is_protected(dev) ) > + return -EINVAL; > + > + spin_lock(&dtdevs_lock); > + > + if ( !list_empty(&dev->next_assigned) ) > + goto fail; > + > + rc = hd->platform_ops->assign_dt_device(d, dev); > + > + if ( rc ) > + goto fail; > + > + list_add(&dev->next_assigned, &hd->dt_devices); > + dt_device_set_used_by(dev, d->domain_id); > + > +fail: > + spin_unlock(&dtdevs_lock); > + > + return rc; > +} > + > +int iommu_deassign_dt_device(struct domain *d, struct dt_device_node *dev) > +{ > + struct hvm_iommu *hd = domain_hvm_iommu(d); > + int rc; > + > + if ( !iommu_enabled || !hd->platform_ops ) > + return -EINVAL; > + > + if ( !dt_device_is_protected(dev) ) > + return -EINVAL; > + > + spin_lock(&dtdevs_lock); > + > + rc = hd->platform_ops->reassign_dt_device(d, dom0, dev); > + if ( rc ) > + goto fail; > + > + dt_device_set_used_by(dev, dom0->domain_id); > + > + list_del(&dev->next_assigned); > + > +fail: > + spin_unlock(&dtdevs_lock); > + > + return rc; > +} > + > +int iommu_dt_domain_init(struct domain *d) > +{ > + struct hvm_iommu *hd = domain_hvm_iommu(d); > + > + INIT_LIST_HEAD(&hd->dt_devices); > + > + return 0; > +} > + > +void iommu_dt_domain_destroy(struct domain *d) > +{ > + struct hvm_iommu *hd = domain_hvm_iommu(d); > + struct dt_device_node *dev, *_dev; > + int rc; > + > + list_for_each_entry_safe(dev, _dev, &hd->dt_devices, next_assigned) > + { > + rc = iommu_deassign_dt_device(d, dev); > + if ( rc ) > + dprintk(XENLOG_ERR, "Failed to deassign %s in domain %u\n", > + dt_node_full_name(dev), d->domain_id); > + } > +} > diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h > index d429e60..2aae047 100644 > --- a/xen/include/xen/device_tree.h > +++ b/xen/include/xen/device_tree.h > @@ -16,6 +16,7 @@ > #include <xen/string.h> > #include <xen/types.h> > #include <xen/stdbool.h> > +#include <xen/list.h> > > #define DEVICE_TREE_MAX_DEPTH 16 > > @@ -110,6 +111,9 @@ struct dt_device_node { > struct dt_device_node *next; /* TODO: Remove it. Only use to know the > last children */ > struct dt_device_node *allnext; > > + /* IOMMU specific fields */ > + bool is_protected; /* Tell if the device is protected by an IOMMU */ > + struct list_head next_assigned; > }; > > #define MAX_PHANDLE_ARGS 16 > @@ -325,6 +329,16 @@ static inline domid_t dt_device_used_by(const struct > dt_device_node *device) > return device->used_by; > } > > +static inline void dt_device_set_protected(struct dt_device_node *device) > +{ > + device->is_protected = true; > +} > + > +static inline bool dt_device_is_protected(const struct dt_device_node > *device) > +{ > + return device->is_protected; > +} > + > static inline bool_t dt_property_name_is_equal(const struct dt_property > *pp, > const char *name) > { > diff --git a/xen/include/xen/hvm/iommu.h b/xen/include/xen/hvm/iommu.h > index f8f8a93..1259e16 100644 > --- a/xen/include/xen/hvm/iommu.h > +++ b/xen/include/xen/hvm/iommu.h > @@ -21,6 +21,7 @@ > #define __XEN_HVM_IOMMU_H__ > > #include <xen/iommu.h> > +#include <xen/list.h> > #include <asm/hvm/iommu.h> > > struct hvm_iommu { > @@ -28,6 +29,11 @@ struct hvm_iommu { > > /* iommu_ops */ > const struct iommu_ops *platform_ops; > + > +#ifdef HAS_DEVICE_TREE > + /* List of DT devices assigned to this domain */ > + struct list_head dt_devices; > +#endif > }; > > #endif /* __XEN_HVM_IOMMU_H__ */ > diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h > index f556a7e..56f6c5c 100644 > --- a/xen/include/xen/iommu.h > +++ b/xen/include/xen/iommu.h > @@ -82,6 +82,16 @@ bool_t pt_irq_need_timer(uint32_t flags); > #define PT_IRQ_TIME_OUT MILLISECS(8) > #endif /* HAS_PCI */ > > +#ifdef HAS_DEVICE_TREE > +#include <xen/device_tree.h> > + > +int iommu_assign_dt_device(struct domain *d, struct dt_device_node *dev); > +int iommu_deassign_dt_device(struct domain *d, struct dt_device_node *dev); > +int iommu_dt_domain_init(struct domain *d); > +void iommu_dt_domain_destroy(struct domain *d); > + > +#endif /* HAS_DEVICE_TREE */ > + > #ifdef HAS_PCI > struct msi_desc; > struct msi_msg; > @@ -103,6 +113,12 @@ struct iommu_ops { > int (*update_ire_from_msi)(struct msi_desc *msi_desc, struct msi_msg > *msg); > void (*read_msi_from_ire)(struct msi_desc *msi_desc, struct msi_msg > *msg); > #endif /* HAS_PCI */ > +#ifdef HAS_DEVICE_TREE > + int (*assign_dt_device)(struct domain *d, const struct dt_device_node > *dev); > + int (*reassign_dt_device)(struct domain *s, struct domain *t, > + const struct dt_device_node *dev); > +#endif > + > void (*teardown)(struct domain *d); > int (*map_page)(struct domain *d, unsigned long gfn, unsigned long mfn, > unsigned int flags); > -- > 1.7.10.4
On Tue, 2014-03-11 at 15:49 +0000, Julien Grall wrote: > Add IOMMU helpers to support device tree assignment/deassignment. This patch > introduces 2 new fields in the dt_device_node: > - is_protected: Does the device is protected by an IOMMU > - next_assigned: Pointer to the next device assigned to the same > domain Am I correct that this list is not maintained for dom0? The behaviour of dt_assign_device and dt_deassign_device seems to rely on it? > Signed-off-by: Julien Grall <julien.grall@linaro.org> > Cc: Xiantao Zhang <xiantao.zhang@intel.com> > Cc: Jan Beulich <jbeulich@suse.com> Ian.
Hi Ian, On 03/18/2014 04:33 PM, Ian Campbell wrote: > On Tue, 2014-03-11 at 15:49 +0000, Julien Grall wrote: >> Add IOMMU helpers to support device tree assignment/deassignment. This patch >> introduces 2 new fields in the dt_device_node: >> - is_protected: Does the device is protected by an IOMMU >> - next_assigned: Pointer to the next device assigned to the same >> domain > > Am I correct that this list is not maintained for dom0? The behaviour of > dt_assign_device and dt_deassign_device seems to rely on it? DOM0 will call dt_assign_device for every device protected by the IOMMU (see patch #9). So it will itself have a list maintained. Regards,
On Tue, 2014-03-18 at 19:46 +0000, Julien Grall wrote: > Hi Ian, > > On 03/18/2014 04:33 PM, Ian Campbell wrote: > > On Tue, 2014-03-11 at 15:49 +0000, Julien Grall wrote: > >> Add IOMMU helpers to support device tree assignment/deassignment. This patch > >> introduces 2 new fields in the dt_device_node: > >> - is_protected: Does the device is protected by an IOMMU > >> - next_assigned: Pointer to the next device assigned to the same > >> domain > > > > Am I correct that this list is not maintained for dom0? The behaviour of > > dt_assign_device and dt_deassign_device seems to rely on it? > > DOM0 will call dt_assign_device for every device protected by the IOMMU > (see patch #9). So it will itself have a list maintained. Does this not mean that iommu_assign_dt_device will refuse to assign the device to another domain (next_assigned list is not empty). BTW, next_assigned is not a very good name for this list, since the "nextness" is abstracted away. Consider just device_list. Ian.
Hi Ian, On 19/03/14 10:12, Ian Campbell wrote: >> On 03/18/2014 04:33 PM, Ian Campbell wrote: >>> On Tue, 2014-03-11 at 15:49 +0000, Julien Grall wrote: >>>> Add IOMMU helpers to support device tree assignment/deassignment. This patch >>>> introduces 2 new fields in the dt_device_node: >>>> - is_protected: Does the device is protected by an IOMMU >>>> - next_assigned: Pointer to the next device assigned to the same >>>> domain >>> >>> Am I correct that this list is not maintained for dom0? The behaviour of >>> dt_assign_device and dt_deassign_device seems to rely on it? >> >> DOM0 will call dt_assign_device for every device protected by the IOMMU >> (see patch #9). So it will itself have a list maintained. > > Does this not mean that iommu_assign_dt_device will refuse to assign the > device to another domain (next_assigned list is not empty). Yes. After thinking, I'm not sure we want maintain a list for dom0. At least the iommu_deassign_dt_device function is wrong because the device is not reassigned to dom0 (from the list point of view). > BTW, next_assigned is not a very good name for this list, since the > "nextness" is abstracted away. Consider just device_list. I found device_list too generic. We don't know if it's the list of all the devices or just the ones of a specific domain. What about domain_device_list? Regards,
On Wed, 2014-03-19 at 10:42 +0000, Julien Grall wrote: > > BTW, next_assigned is not a very good name for this list, since the > > "nextness" is abstracted away. Consider just device_list. > > I found device_list too generic. We don't know if it's the list of all > the devices or just the ones of a specific domain. > > What about domain_device_list? Just domain_list would do I think, device is before the -> in the uses. Ian.
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c index 564f2bb..7c6b683 100644 --- a/xen/common/device_tree.c +++ b/xen/common/device_tree.c @@ -1695,6 +1695,10 @@ static unsigned long __init unflatten_dt_node(const void *fdt, np->full_name = ((char *)np) + sizeof(struct dt_device_node); /* By default dom0 owns the device */ np->used_by = 0; + /* By default the device is not protected */ + np->is_protected = false; + INIT_LIST_HEAD(&np->next_assigned); + if ( new_format ) { char *fn = np->full_name; diff --git a/xen/drivers/passthrough/Makefile b/xen/drivers/passthrough/Makefile index 6e08f89..5a0a35e 100644 --- a/xen/drivers/passthrough/Makefile +++ b/xen/drivers/passthrough/Makefile @@ -5,3 +5,4 @@ subdir-$(x86_64) += x86 obj-y += iommu.o obj-$(x86) += io.o obj-$(HAS_PCI) += pci.o +obj-$(HAS_DEVICE_TREE) += device_tree.o diff --git a/xen/drivers/passthrough/device_tree.c b/xen/drivers/passthrough/device_tree.c new file mode 100644 index 0000000..7384e73 --- /dev/null +++ b/xen/drivers/passthrough/device_tree.c @@ -0,0 +1,106 @@ +/* + * xen/drivers/passthrough/arm/device_tree.c + * + * Code to passthrough device tree node to a guest + * + * Julien Grall <julien.grall@linaro.org> + * Copyright (c) 2014 Linaro Limited. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <xen/lib.h> +#include <xen/sched.h> +#include <xen/iommu.h> +#include <xen/device_tree.h> + +static spinlock_t dtdevs_lock = SPIN_LOCK_UNLOCKED; + +int iommu_assign_dt_device(struct domain *d, struct dt_device_node *dev) +{ + int rc = -EBUSY; + struct hvm_iommu *hd = domain_hvm_iommu(d); + + if ( !iommu_enabled || !hd->platform_ops ) + return -EINVAL; + + if ( !dt_device_is_protected(dev) ) + return -EINVAL; + + spin_lock(&dtdevs_lock); + + if ( !list_empty(&dev->next_assigned) ) + goto fail; + + rc = hd->platform_ops->assign_dt_device(d, dev); + + if ( rc ) + goto fail; + + list_add(&dev->next_assigned, &hd->dt_devices); + dt_device_set_used_by(dev, d->domain_id); + +fail: + spin_unlock(&dtdevs_lock); + + return rc; +} + +int iommu_deassign_dt_device(struct domain *d, struct dt_device_node *dev) +{ + struct hvm_iommu *hd = domain_hvm_iommu(d); + int rc; + + if ( !iommu_enabled || !hd->platform_ops ) + return -EINVAL; + + if ( !dt_device_is_protected(dev) ) + return -EINVAL; + + spin_lock(&dtdevs_lock); + + rc = hd->platform_ops->reassign_dt_device(d, dom0, dev); + if ( rc ) + goto fail; + + dt_device_set_used_by(dev, dom0->domain_id); + + list_del(&dev->next_assigned); + +fail: + spin_unlock(&dtdevs_lock); + + return rc; +} + +int iommu_dt_domain_init(struct domain *d) +{ + struct hvm_iommu *hd = domain_hvm_iommu(d); + + INIT_LIST_HEAD(&hd->dt_devices); + + return 0; +} + +void iommu_dt_domain_destroy(struct domain *d) +{ + struct hvm_iommu *hd = domain_hvm_iommu(d); + struct dt_device_node *dev, *_dev; + int rc; + + list_for_each_entry_safe(dev, _dev, &hd->dt_devices, next_assigned) + { + rc = iommu_deassign_dt_device(d, dev); + if ( rc ) + dprintk(XENLOG_ERR, "Failed to deassign %s in domain %u\n", + dt_node_full_name(dev), d->domain_id); + } +} diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h index d429e60..2aae047 100644 --- a/xen/include/xen/device_tree.h +++ b/xen/include/xen/device_tree.h @@ -16,6 +16,7 @@ #include <xen/string.h> #include <xen/types.h> #include <xen/stdbool.h> +#include <xen/list.h> #define DEVICE_TREE_MAX_DEPTH 16 @@ -110,6 +111,9 @@ struct dt_device_node { struct dt_device_node *next; /* TODO: Remove it. Only use to know the last children */ struct dt_device_node *allnext; + /* IOMMU specific fields */ + bool is_protected; /* Tell if the device is protected by an IOMMU */ + struct list_head next_assigned; }; #define MAX_PHANDLE_ARGS 16 @@ -325,6 +329,16 @@ static inline domid_t dt_device_used_by(const struct dt_device_node *device) return device->used_by; } +static inline void dt_device_set_protected(struct dt_device_node *device) +{ + device->is_protected = true; +} + +static inline bool dt_device_is_protected(const struct dt_device_node *device) +{ + return device->is_protected; +} + static inline bool_t dt_property_name_is_equal(const struct dt_property *pp, const char *name) { diff --git a/xen/include/xen/hvm/iommu.h b/xen/include/xen/hvm/iommu.h index f8f8a93..1259e16 100644 --- a/xen/include/xen/hvm/iommu.h +++ b/xen/include/xen/hvm/iommu.h @@ -21,6 +21,7 @@ #define __XEN_HVM_IOMMU_H__ #include <xen/iommu.h> +#include <xen/list.h> #include <asm/hvm/iommu.h> struct hvm_iommu { @@ -28,6 +29,11 @@ struct hvm_iommu { /* iommu_ops */ const struct iommu_ops *platform_ops; + +#ifdef HAS_DEVICE_TREE + /* List of DT devices assigned to this domain */ + struct list_head dt_devices; +#endif }; #endif /* __XEN_HVM_IOMMU_H__ */ diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h index f556a7e..56f6c5c 100644 --- a/xen/include/xen/iommu.h +++ b/xen/include/xen/iommu.h @@ -82,6 +82,16 @@ bool_t pt_irq_need_timer(uint32_t flags); #define PT_IRQ_TIME_OUT MILLISECS(8) #endif /* HAS_PCI */ +#ifdef HAS_DEVICE_TREE +#include <xen/device_tree.h> + +int iommu_assign_dt_device(struct domain *d, struct dt_device_node *dev); +int iommu_deassign_dt_device(struct domain *d, struct dt_device_node *dev); +int iommu_dt_domain_init(struct domain *d); +void iommu_dt_domain_destroy(struct domain *d); + +#endif /* HAS_DEVICE_TREE */ + #ifdef HAS_PCI struct msi_desc; struct msi_msg; @@ -103,6 +113,12 @@ struct iommu_ops { int (*update_ire_from_msi)(struct msi_desc *msi_desc, struct msi_msg *msg); void (*read_msi_from_ire)(struct msi_desc *msi_desc, struct msi_msg *msg); #endif /* HAS_PCI */ +#ifdef HAS_DEVICE_TREE + int (*assign_dt_device)(struct domain *d, const struct dt_device_node *dev); + int (*reassign_dt_device)(struct domain *s, struct domain *t, + const struct dt_device_node *dev); +#endif + void (*teardown)(struct domain *d); int (*map_page)(struct domain *d, unsigned long gfn, unsigned long mfn, unsigned int flags);
Add IOMMU helpers to support device tree assignment/deassignment. This patch introduces 2 new fields in the dt_device_node: - is_protected: Does the device is protected by an IOMMU - next_assigned: Pointer to the next device assigned to the same domain Signed-off-by: Julien Grall <julien.grall@linaro.org> Cc: Xiantao Zhang <xiantao.zhang@intel.com> Cc: Jan Beulich <jbeulich@suse.com> --- Changes in v3: - Remove iommu_dt_domain_{init,destroy} call in common code. Let architecture code to call them - Fix indentation in xen/include/xen/hvm/iommu.h Changes in v2: - Patch added --- xen/common/device_tree.c | 4 ++ xen/drivers/passthrough/Makefile | 1 + xen/drivers/passthrough/device_tree.c | 106 +++++++++++++++++++++++++++++++++ xen/include/xen/device_tree.h | 14 +++++ xen/include/xen/hvm/iommu.h | 6 ++ xen/include/xen/iommu.h | 16 +++++ 6 files changed, 147 insertions(+) create mode 100644 xen/drivers/passthrough/device_tree.c