| Message ID | 1568388917-7287-8-git-send-email-olekstysh@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | iommu/arm: Add Renesas IPMMU-VMSA support + Linux's iommu_fwspec | expand |
On 13.09.2019 17:35, Oleksandr Tyshchenko wrote: > --- a/xen/include/xen/iommu.h > +++ b/xen/include/xen/iommu.h > @@ -239,6 +239,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, struct dt_phandle_args *args); > +#endif > }; Before I give my ack on this, would you please clarify whether indeed both parameters are intended to be written to by the hook function? If not, either or both should get "const" added. Jan
On 16.09.19 12:53, Jan Beulich wrote: Hi, Jan > On 13.09.2019 17:35, Oleksandr Tyshchenko wrote: >> --- a/xen/include/xen/iommu.h >> +++ b/xen/include/xen/iommu.h >> @@ -239,6 +239,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, struct dt_phandle_args *args); >> +#endif >> }; > Before I give my ack on this, would you please clarify whether > indeed both parameters are intended to be written to by the > hook function? If not, either or both should get "const" added. Good question. I will add "const" to args parameter.
Hi Oleksandr, On 13/09/2019 16:35, 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. It is called when constructing Dom0 since > "iommu_assign_dt_device" can be called for Dom0 also. The last sentence is misleading. Yes some devices may be assigned to the hardware domain (aka dom0) but other may be assigned to other domains. Here you are (ab)using the construction of the hardware domain to add all the devices to the IOMMU. This works fine now because the hardware domain will always be the first domain to be initialized. But I am not sure that whether this is correct to do long term. As mentioned earlier, my preference is to keep "add" and "assign" separated. So maybe what we want to do is: if ( need_mapping ) { iommu_add_dt_device(d, dev); if ( dt_device_is_protected(d) ) { res = iommu_assign_dt_device(...); if ( res ) /* error */ } } We would need similar code in iommu_do_dt_domctl. Although, device should alway be protected in this case. > > 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> > CC: Julien Grall <julien.grall@arm.com> > CC: Jan Beulich <jbeulich@suse.com> > > --- > 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 | 12 +++++++ > xen/drivers/passthrough/arm/iommu.c | 63 +++++++++++++++++++++++++++++++++++++ > xen/include/asm-arm/iommu.h | 11 +++++++ > xen/include/xen/iommu.h | 10 ++++++ > 4 files changed, 96 insertions(+) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index a0fee1e..0d79182 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -1240,6 +1240,7 @@ static int __init map_device_children(struct domain *d, > > /* > * For a given device node: > + * - Try to call iommu_add_dt_device to protect the device by an IOMMU > * - 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: > @@ -1257,6 +1258,17 @@ static int __init handle_device(struct domain *d, struct dt_device_node *dev, > u64 addr, size; > bool need_mapping = !dt_device_for_passthrough(dev); > > + 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 add %s to the IOMMU\n", > + dt_node_full_name(dev)); > + return res; > + } > + > nirq = dt_number_of_irq(dev); > naddr = dt_number_of_address(dev); > > diff --git a/xen/drivers/passthrough/arm/iommu.c b/xen/drivers/passthrough/arm/iommu.c > index 5a3d1d5..dea79ed 100644 > --- a/xen/drivers/passthrough/arm/iommu.c > +++ b/xen/drivers/passthrough/arm/iommu.c > @@ -20,6 +20,7 @@ > #include <xen/lib.h> > > #include <asm/device.h> > +#include <asm/iommu_fwspec.h> > > /* > * Deferred probe list is used to keep track of devices for which driver > @@ -141,3 +142,65 @@ int arch_iommu_populate_page_table(struct domain *d) > void __hwdom_init arch_iommu_hwdom_init(struct domain *d) > { > } > + > +int __init iommu_add_dt_device(struct dt_device_node *np) Sorry to only realise it now. Would it make sense to have this function implemented in xen/passthrough/device_tree.c? This would allow to keep arm/iommu.c as firmware agnostic as possible. > +{ > + 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; > +} > diff --git a/xen/include/asm-arm/iommu.h b/xen/include/asm-arm/iommu.h > index 11dedba..04f21a2 100644 > --- a/xen/include/asm-arm/iommu.h > +++ b/xen/include/asm-arm/iommu.h > @@ -27,6 +27,17 @@ const struct iommu_ops *iommu_get_ops(void); > void iommu_set_ops(const struct iommu_ops *ops); > > /* > + * 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); > + > +/* > * The mapping helpers below should only be used if P2M Table is shared > * between the CPU and the IOMMU. > */ > diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h > index ab258b8..59a2cee 100644 > --- a/xen/include/xen/iommu.h > +++ b/xen/include/xen/iommu.h > @@ -239,6 +239,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, struct dt_phandle_args *args); > +#endif > }; > > #include <asm/iommu.h> > Cheers,
On 19.09.19 14:35, Julien Grall wrote: > Hi Oleksandr, Hi, Julien. > > On 13/09/2019 16:35, 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. It is called when constructing Dom0 since >> "iommu_assign_dt_device" can be called for Dom0 also. > > The last sentence is misleading. Yes some devices may be assigned to > the hardware domain (aka dom0) but other may be assigned to other > domains. > > Here you are (ab)using the construction of the hardware domain to add > all the devices to the IOMMU. This works fine now because the hardware > domain will always be the first domain to be initialized. But I am not > sure that whether this is correct to do long term. > > As mentioned earlier, my preference is to keep "add" and "assign" > separated. So maybe what we want to do is: > > if ( need_mapping ) > { > iommu_add_dt_device(d, dev); > if ( dt_device_is_protected(d) ) > { > res = iommu_assign_dt_device(...); > if ( res ) > /* error */ > } > } > We would need similar code in iommu_do_dt_domctl. Although, device > should alway be protected in this case. Well, will modify this way. >> diff --git a/xen/drivers/passthrough/arm/iommu.c >> b/xen/drivers/passthrough/arm/iommu.c >> index 5a3d1d5..dea79ed 100644 >> --- a/xen/drivers/passthrough/arm/iommu.c >> +++ b/xen/drivers/passthrough/arm/iommu.c >> @@ -20,6 +20,7 @@ >> #include <xen/lib.h> >> #include <asm/device.h> >> +#include <asm/iommu_fwspec.h> >> /* >> * Deferred probe list is used to keep track of devices for which >> driver >> @@ -141,3 +142,65 @@ int arch_iommu_populate_page_table(struct domain >> *d) >> void __hwdom_init arch_iommu_hwdom_init(struct domain *d) >> { >> } >> + >> +int __init iommu_add_dt_device(struct dt_device_node *np) > > Sorry to only realise it now. Would it make sense to have this > function implemented in xen/passthrough/device_tree.c? Not entirely sure. device_tree.c is a common code. The iommu_fwspec stuff (widely used in this function) is ARM code.
Hi, On 19/09/2019 13:25, Oleksandr wrote: > > On 19.09.19 14:35, Julien Grall wrote: >> Hi Oleksandr, >> On 13/09/2019 16:35, 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. It is called when constructing Dom0 since >>> "iommu_assign_dt_device" can be called for Dom0 also. >> >> The last sentence is misleading. Yes some devices may be assigned to the >> hardware domain (aka dom0) but other may be assigned to other domains. >> >> Here you are (ab)using the construction of the hardware domain to add all the >> devices to the IOMMU. This works fine now because the hardware domain will >> always be the first domain to be initialized. But I am not sure that whether >> this is correct to do long term. >> >> As mentioned earlier, my preference is to keep "add" and "assign" separated. >> So maybe what we want to do is: >> >> if ( need_mapping ) >> { >> iommu_add_dt_device(d, dev); >> if ( dt_device_is_protected(d) ) >> { >> res = iommu_assign_dt_device(...); >> if ( res ) >> /* error */ >> } >> } >> We would need similar code in iommu_do_dt_domctl. Although, device should >> alway be protected in this case. > > > Well, will modify this way. > > >>> diff --git a/xen/drivers/passthrough/arm/iommu.c >>> b/xen/drivers/passthrough/arm/iommu.c >>> index 5a3d1d5..dea79ed 100644 >>> --- a/xen/drivers/passthrough/arm/iommu.c >>> +++ b/xen/drivers/passthrough/arm/iommu.c >>> @@ -20,6 +20,7 @@ >>> #include <xen/lib.h> >>> #include <asm/device.h> >>> +#include <asm/iommu_fwspec.h> >>> /* >>> * Deferred probe list is used to keep track of devices for which driver >>> @@ -141,3 +142,65 @@ int arch_iommu_populate_page_table(struct domain *d) >>> void __hwdom_init arch_iommu_hwdom_init(struct domain *d) >>> { >>> } >>> + >>> +int __init iommu_add_dt_device(struct dt_device_node *np) >> >> Sorry to only realise it now. Would it make sense to have this function >> implemented in xen/passthrough/device_tree.c? > > Not entirely sure. device_tree.c is a common code. The iommu_fwspec stuff > (widely used in this function) is ARM code. Some of the device_tree.c already contains Arm specific code (such as device.h). DT has been only used by Arm so far, so it is sadly fairly tie to the architecture. But it should be easy to make it generic if needs be (such as for RISCv). While iommu_fwspec is been implemented in Arm headers, this could potentially be made common. So I would still prefer this that function is moved in device_tree.c Cheers,
On 19.09.19 15:29, Julien Grall wrote: > Hi, Hi, Julien > >>>> + >>>> +int __init iommu_add_dt_device(struct dt_device_node *np) >>> >>> Sorry to only realise it now. Would it make sense to have this >>> function implemented in xen/passthrough/device_tree.c? >> >> Not entirely sure. device_tree.c is a common code. The iommu_fwspec >> stuff (widely used in this function) is ARM code. > > Some of the device_tree.c already contains Arm specific code (such as > device.h). > > DT has been only used by Arm so far, so it is sadly fairly tie to the > architecture. But it should be easy to make it generic if needs be > (such as for RISCv). > > While iommu_fwspec is been implemented in Arm headers, this could > potentially be made common. So I would still prefer this that function > is moved in device_tree.c Well, will move. Also I will remove __init as it can be called at runtime... As for runtime: The current implementation allows us to fail at early stage if something is wrong with the device which is behind an IOMMU (and needs to be protected). As we scan for all present devices, but not only for "passthrough". The "splitting" into handle_device() for hwdom and iommu_do_dt_domctl() for other guests will postpone an error recognition to the guest domain creation time. So, we would have non function system anyway. Wouldn't be better to fail early instead of continue and fail anyway?
On 19/09/2019 14:26, Oleksandr wrote: > > On 19.09.19 15:29, Julien Grall wrote: >> Hi, > > Hi, Julien Hi, > >> >>>>> + >>>>> +int __init iommu_add_dt_device(struct dt_device_node *np) >>>> >>>> Sorry to only realise it now. Would it make sense to have this function >>>> implemented in xen/passthrough/device_tree.c? >>> >>> Not entirely sure. device_tree.c is a common code. The iommu_fwspec stuff >>> (widely used in this function) is ARM code. >> >> Some of the device_tree.c already contains Arm specific code (such as device.h). >> >> DT has been only used by Arm so far, so it is sadly fairly tie to the >> architecture. But it should be easy to make it generic if needs be (such as >> for RISCv). >> >> While iommu_fwspec is been implemented in Arm headers, this could potentially >> be made common. So I would still prefer this that function is moved in >> device_tree.c > > Well, will move. Also I will remove __init as it can be called at runtime... > > > As for runtime: > > The current implementation allows us to fail at early stage if something is > wrong with the device which is behind an IOMMU (and needs to be protected). As > we scan for all present devices, but not only for "passthrough". > The "splitting" into handle_device() for hwdom and iommu_do_dt_domctl() for > other guests will postpone an error recognition to the guest domain creation > time. So, we would have non function system anyway. Wouldn't be better to fail > early instead of continue and fail anyway? Yes your implementation allows us to fail at early stage but then you are abusing the function handle_device(). There are actually no promise this will be called for every device going forward. Think about dom0less where the goal is to have no dom0 at all. You are also tying the order of the domains creation as dom0 would have to be always created before any other domains are created. Similar (ab)use of handle_device() does not exist, so I would rather not start to introduce them because this will become quickly unmaintainable as we are mixing dom0 creation and Xen initialization. Even without this series, assigning a device to the guest may not be a success because the IOMMU may not have enough context bank (used for configuring the IOMMU stage-2) or there are not enough memory. Not been able to add the device to the IOMMU driver is only another example where it may fail. But I would not consider this as not functional. The rest of your system may work perfectly even if this particular device is not usable. There are no security risk as the IOMMU should block any transaction by default. I am also not in favor of parsing again the Device-Tree (we have enough of them), so this is the best solution I can come up. Feel free to suggest something different. Cheers,
Hi Julien > >> >>> >>>>>> + >>>>>> +int __init iommu_add_dt_device(struct dt_device_node *np) >>>>> >>>>> Sorry to only realise it now. Would it make sense to have this >>>>> function implemented in xen/passthrough/device_tree.c? >>>> >>>> Not entirely sure. device_tree.c is a common code. The iommu_fwspec >>>> stuff (widely used in this function) is ARM code. >>> >>> Some of the device_tree.c already contains Arm specific code (such >>> as device.h). >>> >>> DT has been only used by Arm so far, so it is sadly fairly tie to >>> the architecture. But it should be easy to make it generic if needs >>> be (such as for RISCv). >>> >>> While iommu_fwspec is been implemented in Arm headers, this could >>> potentially be made common. So I would still prefer this that >>> function is moved in device_tree.c >> >> Well, will move. Also I will remove __init as it can be called at >> runtime... >> >> >> As for runtime: >> >> The current implementation allows us to fail at early stage if >> something is wrong with the device which is behind an IOMMU (and >> needs to be protected). As we scan for all present devices, but not >> only for "passthrough". >> The "splitting" into handle_device() for hwdom and >> iommu_do_dt_domctl() for other guests will postpone an error >> recognition to the guest domain creation time. So, we would have non >> function system anyway. Wouldn't be better to fail early instead of >> continue and fail anyway? > > Yes your implementation allows us to fail at early stage but then you > are abusing the function handle_device(). There are actually no > promise this will be called for every device going forward. Think > about dom0less where the goal is to have no dom0 at all. > > You are also tying the order of the domains creation as dom0 would > have to be always created before any other domains are created. > > Similar (ab)use of handle_device() does not exist, so I would rather > not start to introduce them because this will become quickly > unmaintainable as we are mixing dom0 creation and Xen initialization. > > Even without this series, assigning a device to the guest may not be a > success because the IOMMU may not have enough context bank (used for > configuring the IOMMU stage-2) or there are not enough memory. Not > been able to add the device to the IOMMU driver is only another > example where it may fail. > > But I would not consider this as not functional. The rest of your > system may work perfectly even if this particular device is not > usable. There are no security risk as the IOMMU should block any > transaction by default. > > I am also not in favor of parsing again the Device-Tree (we have > enough of them), so this is the best solution I can come up. Feel free > to suggest something different. I am happy with the explanation, sounds reasonable. Will modify patch as you suggested.
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index a0fee1e..0d79182 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -1240,6 +1240,7 @@ static int __init map_device_children(struct domain *d, /* * For a given device node: + * - Try to call iommu_add_dt_device to protect the device by an IOMMU * - 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: @@ -1257,6 +1258,17 @@ static int __init handle_device(struct domain *d, struct dt_device_node *dev, u64 addr, size; bool need_mapping = !dt_device_for_passthrough(dev); + 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 add %s to the IOMMU\n", + dt_node_full_name(dev)); + return res; + } + nirq = dt_number_of_irq(dev); naddr = dt_number_of_address(dev); diff --git a/xen/drivers/passthrough/arm/iommu.c b/xen/drivers/passthrough/arm/iommu.c index 5a3d1d5..dea79ed 100644 --- a/xen/drivers/passthrough/arm/iommu.c +++ b/xen/drivers/passthrough/arm/iommu.c @@ -20,6 +20,7 @@ #include <xen/lib.h> #include <asm/device.h> +#include <asm/iommu_fwspec.h> /* * Deferred probe list is used to keep track of devices for which driver @@ -141,3 +142,65 @@ int arch_iommu_populate_page_table(struct domain *d) void __hwdom_init arch_iommu_hwdom_init(struct domain *d) { } + +int __init 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; +} diff --git a/xen/include/asm-arm/iommu.h b/xen/include/asm-arm/iommu.h index 11dedba..04f21a2 100644 --- a/xen/include/asm-arm/iommu.h +++ b/xen/include/asm-arm/iommu.h @@ -27,6 +27,17 @@ const struct iommu_ops *iommu_get_ops(void); void iommu_set_ops(const struct iommu_ops *ops); /* + * 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); + +/* * The mapping helpers below should only be used if P2M Table is shared * between the CPU and the IOMMU. */ diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h index ab258b8..59a2cee 100644 --- a/xen/include/xen/iommu.h +++ b/xen/include/xen/iommu.h @@ -239,6 +239,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, struct dt_phandle_args *args); +#endif }; #include <asm/iommu.h>