Message ID | 1569496834-7796-8-git-send-email-olekstysh@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | iommu/arm: Add Renesas IPMMU-VMSA support + Linux's iommu_fwspec | expand |
Hi, On 9/26/19 12:20 PM, Oleksandr Tyshchenko wrote: > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > > The main puprose of this patch is to add a way to register DT device > (which is behind the IOMMU) using the generic IOMMU DT bindings [1] > before assigning that device to a domain. > > So, this patch adds new "iommu_add_dt_device" API for adding DT device > to the IOMMU using generic IOMMU DT bindings and previously added > "iommu_fwspec" support. As devices can be assigned to the hardware domain > and other domains this function is called from two places: handle_device() > and iommu_do_dt_domctl(). > > Besides that, this patch adds new "dt_xlate" callback (borrowed from > Linux "of_xlate") for providing the driver with DT IOMMU specifier > which describes the IOMMU master interfaces of that device (device IDs, etc). > According to the generic IOMMU DT bindings the context of required > properties for IOMMU device/master node (#iommu-cells, iommus) depends > on many factors and is really driver depended thing. > > Please note, all IOMMU drivers which support generic IOMMU DT bindings > should use "dt_xlate" and "add_device" callbacks. > > [1] https://www.kernel.org/doc/Documentation/devicetree/bindings/iommu/iommu.txt > > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> Acked-by: Julien Grall <julien.grall@arm.com> Cheers, > CC: Julien Grall <julien.grall@arm.com> > CC: Jan Beulich <jbeulich@suse.com> > > --- > Changes V5 -> V6: > - added check for the function's return value in both cases > - removed extra check for dt_device_is_protected() from > iommu_do_dt_domctl() > > Changes V4 -> V5: > - added "const" to struct dt_phandle_args *args in dt_xlate > - moved iommu_add_dt_device() to xen/passthrough/device_tree.c > - modified logic, don't try to add "all" devices to the IOMMU > when constructing Dom0, but only devices which are going to be > assigned to hwdom > - updated iommu_do_dt_domctl() to call iommu_add_dt_device() > - clarified patch description > - removed "__init" from iommu_add_dt_device() > > Changes V3 -> V4: > - squashed with "iommu: Add of_xlate callback" patch > - renamed "of_xlate" to "dt_xlate" > - reworked patch description > - clarified comments in code, removed confusing word > "initialize device", etc > - updated debug message in handle_device() > - modified to check ops->of_xlate and ops->add_device > only if "iommus" property is exists > > Changes V2 -> V3: > - clarified patch description > - clarified comments in code > - modified to provide DT IOMMU specifier to the driver > using "of_xlate" callback > - documented function usage > - modified to return an error if ops is not present/implemented, > - added ability to return a possitive value to indicate > that device doesn't need to be protected > - removed check for the "iommu" property presence > in the common code > - included <asm/iommu_fwspec.h> directly > --- > xen/arch/arm/domain_build.c | 25 +++++++++--- > xen/drivers/passthrough/device_tree.c | 77 +++++++++++++++++++++++++++++++++++ > xen/include/xen/iommu.h | 21 ++++++++++ > 3 files changed, 118 insertions(+), 5 deletions(-) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index a0fee1e..b84a448 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -1243,6 +1243,7 @@ static int __init map_device_children(struct domain *d, > * - Give permission to the guest to manage IRQ and MMIO range > * - Retrieve the IRQ configuration (i.e edge/level) from device tree > * When the device is not marked for guest passthrough: > + * - Try to call iommu_add_dt_device to protect the device by an IOMMU > * - Assign the device to the guest if it's protected by an IOMMU > * - Map the IRQs and iomem regions to DOM0 > */ > @@ -1263,16 +1264,30 @@ static int __init handle_device(struct domain *d, struct dt_device_node *dev, > dt_dprintk("%s passthrough = %d nirq = %d naddr = %u\n", > dt_node_full_name(dev), need_mapping, nirq, naddr); > > - if ( dt_device_is_protected(dev) && need_mapping ) > + if ( need_mapping ) > { > - dt_dprintk("%s setup iommu\n", dt_node_full_name(dev)); > - res = iommu_assign_dt_device(d, dev); > - if ( res ) > + dt_dprintk("Check if %s is behind the IOMMU and add it\n", > + dt_node_full_name(dev)); > + > + res = iommu_add_dt_device(dev); > + if ( res < 0 ) > { > - printk(XENLOG_ERR "Failed to setup the IOMMU for %s\n", > + printk(XENLOG_ERR "Failed to add %s to the IOMMU\n", > dt_node_full_name(dev)); > return res; > } > + > + if ( dt_device_is_protected(dev) ) > + { > + dt_dprintk("%s setup iommu\n", dt_node_full_name(dev)); > + res = iommu_assign_dt_device(d, dev); > + if ( res ) > + { > + printk(XENLOG_ERR "Failed to setup the IOMMU for %s\n", > + dt_node_full_name(dev)); > + return res; > + } > + } > } > > /* Give permission and map IRQs */ > diff --git a/xen/drivers/passthrough/device_tree.c b/xen/drivers/passthrough/device_tree.c > index 921a6e5..cc900ba 100644 > --- a/xen/drivers/passthrough/device_tree.c > +++ b/xen/drivers/passthrough/device_tree.c > @@ -22,6 +22,8 @@ > #include <xen/sched.h> > #include <xsm/xsm.h> > > +#include <asm/iommu_fwspec.h> > + > static spinlock_t dtdevs_lock = SPIN_LOCK_UNLOCKED; > > int iommu_assign_dt_device(struct domain *d, struct dt_device_node *dev) > @@ -125,6 +127,68 @@ int iommu_release_dt_devices(struct domain *d) > return 0; > } > > +int iommu_add_dt_device(struct dt_device_node *np) > +{ > + const struct iommu_ops *ops = iommu_get_ops(); > + struct dt_phandle_args iommu_spec; > + struct device *dev = dt_to_dev(np); > + int rc = 1, index = 0; > + > + if ( !iommu_enabled ) > + return 1; > + > + if ( !ops ) > + return -EINVAL; > + > + if ( dev_iommu_fwspec_get(dev) ) > + return -EEXIST; > + > + /* > + * According to the Documentation/devicetree/bindings/iommu/iommu.txt > + * from Linux. > + */ > + while ( !dt_parse_phandle_with_args(np, "iommus", "#iommu-cells", > + index, &iommu_spec) ) > + { > + /* > + * The driver which supports generic IOMMU DT bindings must have > + * these callback implemented. > + */ > + if ( !ops->add_device || !ops->dt_xlate ) > + return -EINVAL; > + > + if ( !dt_device_is_available(iommu_spec.np) ) > + break; > + > + rc = iommu_fwspec_init(dev, &iommu_spec.np->dev); > + if ( rc ) > + break; > + > + /* > + * Provide DT IOMMU specifier which describes the IOMMU master > + * interfaces of that device (device IDs, etc) to the driver. > + * The driver is responsible to decide how to interpret them. > + */ > + rc = ops->dt_xlate(dev, &iommu_spec); > + if ( rc ) > + break; > + > + index++; > + } > + > + /* > + * Add master device to the IOMMU if latter is present and available. > + * The driver is responsible to mark that device as protected. > + */ > + if ( !rc ) > + rc = ops->add_device(0, dev); > + > + if ( rc < 0 ) > + iommu_fwspec_free(dev); > + > + return rc; > +} > + > int iommu_do_dt_domctl(struct xen_domctl *domctl, struct domain *d, > XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) > { > @@ -166,6 +230,19 @@ int iommu_do_dt_domctl(struct xen_domctl *domctl, struct domain *d, > break; > } > > + ret = iommu_add_dt_device(dev); > + /* > + * Ignore "-EEXIST" error code as it would mean that the device is > + * already added to the IOMMU (positive result). Such happens after > + * re-creating guest domain. > + */ > + if ( ret < 0 && ret != -EEXIST ) > + { > + printk(XENLOG_G_ERR "Failed to add %s to the IOMMU\n", > + dt_node_full_name(dev)); > + break; > + } > + > ret = iommu_assign_dt_device(d, dev); > > if ( ret ) > diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h > index 7c3003f..974bd3f 100644 > --- a/xen/include/xen/iommu.h > +++ b/xen/include/xen/iommu.h > @@ -182,6 +182,17 @@ int iommu_deassign_dt_device(struct domain *d, struct dt_device_node *dev); > int iommu_dt_domain_init(struct domain *d); > int iommu_release_dt_devices(struct domain *d); > > +/* > + * Helper to add master device to the IOMMU using generic IOMMU DT bindings. > + * > + * Return values: > + * 0 : device is protected by an IOMMU > + * <0 : device is not protected by an IOMMU, but must be (error condition) > + * >0 : device doesn't need to be protected by an IOMMU > + * (IOMMU is not enabled/present or device is not connected to it). > + */ > +int iommu_add_dt_device(struct dt_device_node *np); > + > int iommu_do_dt_domctl(struct xen_domctl *, struct domain *, > XEN_GUEST_HANDLE_PARAM(xen_domctl_t)); > > @@ -250,6 +261,16 @@ struct iommu_ops { > int __must_check (*iotlb_flush_all)(struct domain *d); > int (*get_reserved_device_memory)(iommu_grdm_t *, void *); > void (*dump_p2m_table)(struct domain *d); > + > +#ifdef CONFIG_HAS_DEVICE_TREE > + /* > + * All IOMMU drivers which support generic IOMMU DT bindings should use > + * this callback. This is a way for the framework to provide the driver > + * with DT IOMMU specifier which describes the IOMMU master interfaces of > + * that device (device IDs, etc). > + */ > + int (*dt_xlate)(device_t *dev, const struct dt_phandle_args *args); > +#endif > }; > > #include <asm/iommu.h> >
On 26.09.19 15:52, Julien Grall wrote: > Hi, Hi Julien > > On 9/26/19 12:20 PM, Oleksandr Tyshchenko wrote: >> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> >> >> The main puprose of this patch is to add a way to register DT device >> (which is behind the IOMMU) using the generic IOMMU DT bindings [1] >> before assigning that device to a domain. >> >> So, this patch adds new "iommu_add_dt_device" API for adding DT device >> to the IOMMU using generic IOMMU DT bindings and previously added >> "iommu_fwspec" support. As devices can be assigned to the hardware >> domain >> and other domains this function is called from two places: >> handle_device() >> and iommu_do_dt_domctl(). >> >> Besides that, this patch adds new "dt_xlate" callback (borrowed from >> Linux "of_xlate") for providing the driver with DT IOMMU specifier >> which describes the IOMMU master interfaces of that device (device >> IDs, etc). >> According to the generic IOMMU DT bindings the context of required >> properties for IOMMU device/master node (#iommu-cells, iommus) depends >> on many factors and is really driver depended thing. >> >> Please note, all IOMMU drivers which support generic IOMMU DT bindings >> should use "dt_xlate" and "add_device" callbacks. >> >> [1] >> https://www.kernel.org/doc/Documentation/devicetree/bindings/iommu/iommu.txt >> >> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > > Acked-by: Julien Grall <julien.grall@arm.com> Thank you!
On 26.09.2019 14:52, Julien Grall wrote: > On 9/26/19 12:20 PM, Oleksandr Tyshchenko wrote: >> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> >> >> The main puprose of this patch is to add a way to register DT device >> (which is behind the IOMMU) using the generic IOMMU DT bindings [1] >> before assigning that device to a domain. >> >> So, this patch adds new "iommu_add_dt_device" API for adding DT device >> to the IOMMU using generic IOMMU DT bindings and previously added >> "iommu_fwspec" support. As devices can be assigned to the hardware domain >> and other domains this function is called from two places: handle_device() >> and iommu_do_dt_domctl(). >> >> Besides that, this patch adds new "dt_xlate" callback (borrowed from >> Linux "of_xlate") for providing the driver with DT IOMMU specifier >> which describes the IOMMU master interfaces of that device (device IDs, etc). >> According to the generic IOMMU DT bindings the context of required >> properties for IOMMU device/master node (#iommu-cells, iommus) depends >> on many factors and is really driver depended thing. >> >> Please note, all IOMMU drivers which support generic IOMMU DT bindings >> should use "dt_xlate" and "add_device" callbacks. >> >> [1] https://www.kernel.org/doc/Documentation/devicetree/bindings/iommu/iommu.txt >> >> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > > Acked-by: Julien Grall <julien.grall@arm.com> Acked-by: Jan Beulich <jbeulich@suse.com>
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index a0fee1e..b84a448 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -1243,6 +1243,7 @@ static int __init map_device_children(struct domain *d, * - Give permission to the guest to manage IRQ and MMIO range * - Retrieve the IRQ configuration (i.e edge/level) from device tree * When the device is not marked for guest passthrough: + * - Try to call iommu_add_dt_device to protect the device by an IOMMU * - Assign the device to the guest if it's protected by an IOMMU * - Map the IRQs and iomem regions to DOM0 */ @@ -1263,16 +1264,30 @@ static int __init handle_device(struct domain *d, struct dt_device_node *dev, dt_dprintk("%s passthrough = %d nirq = %d naddr = %u\n", dt_node_full_name(dev), need_mapping, nirq, naddr); - if ( dt_device_is_protected(dev) && need_mapping ) + if ( need_mapping ) { - dt_dprintk("%s setup iommu\n", dt_node_full_name(dev)); - res = iommu_assign_dt_device(d, dev); - if ( res ) + dt_dprintk("Check if %s is behind the IOMMU and add it\n", + dt_node_full_name(dev)); + + res = iommu_add_dt_device(dev); + if ( res < 0 ) { - printk(XENLOG_ERR "Failed to setup the IOMMU for %s\n", + printk(XENLOG_ERR "Failed to add %s to the IOMMU\n", dt_node_full_name(dev)); return res; } + + if ( dt_device_is_protected(dev) ) + { + dt_dprintk("%s setup iommu\n", dt_node_full_name(dev)); + res = iommu_assign_dt_device(d, dev); + if ( res ) + { + printk(XENLOG_ERR "Failed to setup the IOMMU for %s\n", + dt_node_full_name(dev)); + return res; + } + } } /* Give permission and map IRQs */ diff --git a/xen/drivers/passthrough/device_tree.c b/xen/drivers/passthrough/device_tree.c index 921a6e5..cc900ba 100644 --- a/xen/drivers/passthrough/device_tree.c +++ b/xen/drivers/passthrough/device_tree.c @@ -22,6 +22,8 @@ #include <xen/sched.h> #include <xsm/xsm.h> +#include <asm/iommu_fwspec.h> + static spinlock_t dtdevs_lock = SPIN_LOCK_UNLOCKED; int iommu_assign_dt_device(struct domain *d, struct dt_device_node *dev) @@ -125,6 +127,68 @@ int iommu_release_dt_devices(struct domain *d) return 0; } +int iommu_add_dt_device(struct dt_device_node *np) +{ + const struct iommu_ops *ops = iommu_get_ops(); + struct dt_phandle_args iommu_spec; + struct device *dev = dt_to_dev(np); + int rc = 1, index = 0; + + if ( !iommu_enabled ) + return 1; + + if ( !ops ) + return -EINVAL; + + if ( dev_iommu_fwspec_get(dev) ) + return -EEXIST; + + /* + * According to the Documentation/devicetree/bindings/iommu/iommu.txt + * from Linux. + */ + while ( !dt_parse_phandle_with_args(np, "iommus", "#iommu-cells", + index, &iommu_spec) ) + { + /* + * The driver which supports generic IOMMU DT bindings must have + * these callback implemented. + */ + if ( !ops->add_device || !ops->dt_xlate ) + return -EINVAL; + + if ( !dt_device_is_available(iommu_spec.np) ) + break; + + rc = iommu_fwspec_init(dev, &iommu_spec.np->dev); + if ( rc ) + break; + + /* + * Provide DT IOMMU specifier which describes the IOMMU master + * interfaces of that device (device IDs, etc) to the driver. + * The driver is responsible to decide how to interpret them. + */ + rc = ops->dt_xlate(dev, &iommu_spec); + if ( rc ) + break; + + index++; + } + + /* + * Add master device to the IOMMU if latter is present and available. + * The driver is responsible to mark that device as protected. + */ + if ( !rc ) + rc = ops->add_device(0, dev); + + if ( rc < 0 ) + iommu_fwspec_free(dev); + + return rc; +} + int iommu_do_dt_domctl(struct xen_domctl *domctl, struct domain *d, XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) { @@ -166,6 +230,19 @@ int iommu_do_dt_domctl(struct xen_domctl *domctl, struct domain *d, break; } + ret = iommu_add_dt_device(dev); + /* + * Ignore "-EEXIST" error code as it would mean that the device is + * already added to the IOMMU (positive result). Such happens after + * re-creating guest domain. + */ + if ( ret < 0 && ret != -EEXIST ) + { + printk(XENLOG_G_ERR "Failed to add %s to the IOMMU\n", + dt_node_full_name(dev)); + break; + } + ret = iommu_assign_dt_device(d, dev); if ( ret ) diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h index 7c3003f..974bd3f 100644 --- a/xen/include/xen/iommu.h +++ b/xen/include/xen/iommu.h @@ -182,6 +182,17 @@ int iommu_deassign_dt_device(struct domain *d, struct dt_device_node *dev); int iommu_dt_domain_init(struct domain *d); int iommu_release_dt_devices(struct domain *d); +/* + * Helper to add master device to the IOMMU using generic IOMMU DT bindings. + * + * Return values: + * 0 : device is protected by an IOMMU + * <0 : device is not protected by an IOMMU, but must be (error condition) + * >0 : device doesn't need to be protected by an IOMMU + * (IOMMU is not enabled/present or device is not connected to it). + */ +int iommu_add_dt_device(struct dt_device_node *np); + int iommu_do_dt_domctl(struct xen_domctl *, struct domain *, XEN_GUEST_HANDLE_PARAM(xen_domctl_t)); @@ -250,6 +261,16 @@ struct iommu_ops { int __must_check (*iotlb_flush_all)(struct domain *d); int (*get_reserved_device_memory)(iommu_grdm_t *, void *); void (*dump_p2m_table)(struct domain *d); + +#ifdef CONFIG_HAS_DEVICE_TREE + /* + * All IOMMU drivers which support generic IOMMU DT bindings should use + * this callback. This is a way for the framework to provide the driver + * with DT IOMMU specifier which describes the IOMMU master interfaces of + * that device (device IDs, etc). + */ + int (*dt_xlate)(device_t *dev, const struct dt_phandle_args *args); +#endif }; #include <asm/iommu.h>