RE: [PATCH V7 00/11] IOMMU probe deferral support
Hi Hanjun, >On 2017/1/24 0:18, Sricharan R wrote: >> This series calls the dma ops configuration for the devices >> at a generic place so that it works for all busses. >> The dma_configure_ops for a device is now called during >> the device_attach callback just before the probe of the >> bus/driver is called. Similarly dma_deconfigure is called during >> device/driver_detach path. >> >> pci_bus_add_devices(platform/amba)(_device_create/driver_register) >>| | >> pci_bus_add_device (device_add/driver_register) >>| | >> device_attach device_initial_probe >>| | >> __device_attach_driver__device_attach_driver >>| >> driver_probe_device >>| >> really_probe >>| >> dma_configure >> >> Similarly on the device/driver_unregister path __device_release_driver is >> called which inturn calls dma_deconfigure. >> >> Took the reworked patches [2] from Robin's branch and >> rebased on top of Lorenzo's ACPI IORT ARM support series [3]. >> >> * Tested with platform and pci devices for probe deferral >> and reprobe on arm64 based platform. >> * Need help for testing with ACPI. > >Cherry picked this patch set with no conflict with my platform >msi patches, and test it on Hisilicon D03 (ARM64 platform with >SMMUv3) with ACPI, it boots ok as before and the native NIC and >SAS (platform devices) are working properly. > >for ACPI and the core code changes: > >Tested-by: Hanjun Guo Thanks for the testing with ACPI + SMMUV3 Regards, Sricharan ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH V7 08/11] drivers: acpi: Handle IOMMU lookup failure with deferred probing or error
Hi Lorenzo,
>[+hanjun, tomasz, sinan]
>
>It is quite a key patchset, I would be glad if they can test on their
>respective platforms with IORT.
>
>On Mon, Jan 23, 2017 at 09:48:10PM +0530, Sricharan R wrote:
>> This is an equivalent to the DT's handling of the iommu master's probe
>> with deferred probing when the corrsponding iommu is not probed yet.
>> The lack of a registered IOMMU can be caused by the lack of a driver for
>> the IOMMU, the IOMMU device probe not having been performed yet, having
>> been deferred, or having failed.
>>
>> The first case occurs when the firmware describes the bus master and
>> IOMMU topology correctly but no device driver exists for the IOMMU yet
>> or the device driver has not been compiled in. Return NULL, the caller
>> will configure the device without an IOMMU.
>>
>> The second and third cases are handled by deferring the probe of the bus
>> master device which will eventually get reprobed after the IOMMU.
>>
>> The last case is currently handled by deferring the probe of the bus
>> master device as well. A mechanism to either configure the bus master
>> device without an IOMMU or to fail the bus master device probe depending
>> on whether the IOMMU is optional or mandatory would be a good
>> enhancement.
>>
>> Signed-off-by: Sricharan R
>> ---
>> drivers/acpi/arm64/iort.c | 25 -
>> drivers/acpi/scan.c| 7 +--
>> drivers/base/dma-mapping.c | 2 +-
>> include/acpi/acpi_bus.h| 2 +-
>> include/linux/acpi.h | 7 +--
>> 5 files changed, 36 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
>> index bf0ed09..d01bae8 100644
>> --- a/drivers/acpi/arm64/iort.c
>> +++ b/drivers/acpi/arm64/iort.c
>> @@ -550,8 +550,17 @@ static const struct iommu_ops *iort_iommu_xlate(struct
>> device *dev,
>> return NULL;
>>
>> ops = iommu_get_instance(iort_fwnode);
>> +/*
>> + * If the ops look-up fails, this means that either
>> + * the SMMU drivers have not been probed yet or that
>> + * the SMMU drivers are not built in the kernel;
>> + * Depending on whether the SMMU drivers are built-in
>> + * in the kernel or not, defer the IOMMU configuration
>> + * or just abort it.
>> + */
>> if (!ops)
>> -return NULL;
>> +return iort_iommu_driver_enabled(node->type) ?
>> + ERR_PTR(-EPROBE_DEFER) : NULL;
>>
>> ret = arm_smmu_iort_xlate(dev, streamid, iort_fwnode, ops);
>> }
>> @@ -625,12 +634,26 @@ const struct iommu_ops *iort_iommu_configure(struct
>> device *dev)
>>
>> while (parent) {
>> ops = iort_iommu_xlate(dev, parent, streamid);
>> +if (IS_ERR_OR_NULL(ops))
>> +return ops;
>>
>> parent = iort_node_get_id(node, &streamid,
>>IORT_IOMMU_TYPE, i++);
>> }
>> }
>>
>> +/*
>> + * If we have reason to believe the IOMMU driver missed the initial
>> + * add_device callback for dev, replay it to get things in order.
>> + */
>> +if (!IS_ERR_OR_NULL(ops) && ops->add_device &&
>> +dev->bus && !dev->iommu_group) {
>> +int err = ops->add_device(dev);
>> +
>> +if (err)
>> +ops = ERR_PTR(err);
>> +}
>
>I think there is nothing ACPI specific in this add_device() replay
>path, so there is room for further DT/ACPI consolidation here.
>
ok, the only way is keep this in a function and call it for both DT and ACPI
cases.
>Without any further ado:
>
>Acked-by: Lorenzo Pieralisi
Thanks for that.
Regards,
Sricharan
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH V7 00/11] IOMMU probe deferral support
On 2017/1/24 0:18, Sricharan R wrote: This series calls the dma ops configuration for the devices at a generic place so that it works for all busses. The dma_configure_ops for a device is now called during the device_attach callback just before the probe of the bus/driver is called. Similarly dma_deconfigure is called during device/driver_detach path. pci_bus_add_devices(platform/amba)(_device_create/driver_register) | | pci_bus_add_device (device_add/driver_register) | | device_attach device_initial_probe | | __device_attach_driver__device_attach_driver | driver_probe_device | really_probe | dma_configure Similarly on the device/driver_unregister path __device_release_driver is called which inturn calls dma_deconfigure. Took the reworked patches [2] from Robin's branch and rebased on top of Lorenzo's ACPI IORT ARM support series [3]. * Tested with platform and pci devices for probe deferral and reprobe on arm64 based platform. * Need help for testing with ACPI. Cherry picked this patch set with no conflict with my platform msi patches, and test it on Hisilicon D03 (ARM64 platform with SMMUv3) with ACPI, it boots ok as before and the native NIC and SAS (platform devices) are working properly. for ACPI and the core code changes: Tested-by: Hanjun Guo Thanks Hanjun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 0/5] Implement SMMU passthrough using the default domain
Hi Will, >Hi all, > >A number of people have expressed interest in having the SMMU come up in >a passthrough configuration, and then allow subsequent translation for >things such as VFIO. Rather than do this in each SMMU driver, it's much >cleaner to allow the default domain to be configured to be something other >than DMA. > >This patch series implements a command-line option to configure the >default domain type. Currently, it supports "dma" and "identity" which >is sufficient for the passthrough use-case. > >Tested on an ARM fastmodel. > >All feedback welcome, > Thanks for this series. We had a case with the GPU. The GPU's iommu was setup by kernel and the GPU also does dynamic updates for on-the-fly switching between process pagetables. GPU driver was not using DMA domain and the GPU's firmware was always expecting to run out of contextbank '0' (although not correct) , which was not the case after the DMA domain was made default as '0' was getting allocated for DMA domain and there were concerns about reusing the DMA domain as well. Now with this series, looks there is an way out of that that can be tried. So should the default domain not be per device specific selectable ? Regards, Sricharan >Will > >--->8 > >Will Deacon (5): > iommu/arm-smmu: Restrict domain attributes to UNMANAGED domains > iommu/arm-smmu: Install bypass S2CRs for IOMMU_DOMAIN_IDENTITY domains > iommu/arm-smmu-v3: Install bypass STEs for IOMMU_DOMAIN_IDENTITY >domains > arm64: dma-mapping: Only swizzle DMA ops for IOMMU_DOMAIN_DMA > iommu: Allow default domain type to be set on the kernel command line > > arch/arm64/mm/dma-mapping.c | 17 - > drivers/iommu/arm-smmu-v3.c | 20 ++-- > drivers/iommu/arm-smmu.c| 26 +++--- > drivers/iommu/iommu.c | 19 +-- > 4 files changed, 70 insertions(+), 12 deletions(-) > >-- >2.1.4 > > >___ >linux-arm-kernel mailing list >linux-arm-ker...@lists.infradead.org >http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH V7 08/11] drivers: acpi: Handle IOMMU lookup failure with deferred probing or error
Hi Lorenzo, Sricharan, On 01/24/2017 08:37 PM, Lorenzo Pieralisi wrote: [+hanjun, tomasz, sinan] It is quite a key patchset, I would be glad if they can test on their respective platforms with IORT. ACPI patches are conflict with my acpi platform msi patches (I need them to enable devices on my ARM64 platform then test this patch set), I will fix the conflict and test it tomorrow (out of office now and can't reach the test machines) :) Thanks Hanjun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH V7 08/11] drivers: acpi: Handle IOMMU lookup failure with deferred probing or error
[+hanjun, tomasz, sinan]
It is quite a key patchset, I would be glad if they can test on their
respective platforms with IORT.
On Mon, Jan 23, 2017 at 09:48:10PM +0530, Sricharan R wrote:
> This is an equivalent to the DT's handling of the iommu master's probe
> with deferred probing when the corrsponding iommu is not probed yet.
> The lack of a registered IOMMU can be caused by the lack of a driver for
> the IOMMU, the IOMMU device probe not having been performed yet, having
> been deferred, or having failed.
>
> The first case occurs when the firmware describes the bus master and
> IOMMU topology correctly but no device driver exists for the IOMMU yet
> or the device driver has not been compiled in. Return NULL, the caller
> will configure the device without an IOMMU.
>
> The second and third cases are handled by deferring the probe of the bus
> master device which will eventually get reprobed after the IOMMU.
>
> The last case is currently handled by deferring the probe of the bus
> master device as well. A mechanism to either configure the bus master
> device without an IOMMU or to fail the bus master device probe depending
> on whether the IOMMU is optional or mandatory would be a good
> enhancement.
>
> Signed-off-by: Sricharan R
> ---
> drivers/acpi/arm64/iort.c | 25 -
> drivers/acpi/scan.c| 7 +--
> drivers/base/dma-mapping.c | 2 +-
> include/acpi/acpi_bus.h| 2 +-
> include/linux/acpi.h | 7 +--
> 5 files changed, 36 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index bf0ed09..d01bae8 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -550,8 +550,17 @@ static const struct iommu_ops *iort_iommu_xlate(struct
> device *dev,
> return NULL;
>
> ops = iommu_get_instance(iort_fwnode);
> + /*
> + * If the ops look-up fails, this means that either
> + * the SMMU drivers have not been probed yet or that
> + * the SMMU drivers are not built in the kernel;
> + * Depending on whether the SMMU drivers are built-in
> + * in the kernel or not, defer the IOMMU configuration
> + * or just abort it.
> + */
> if (!ops)
> - return NULL;
> + return iort_iommu_driver_enabled(node->type) ?
> +ERR_PTR(-EPROBE_DEFER) : NULL;
>
> ret = arm_smmu_iort_xlate(dev, streamid, iort_fwnode, ops);
> }
> @@ -625,12 +634,26 @@ const struct iommu_ops *iort_iommu_configure(struct
> device *dev)
>
> while (parent) {
> ops = iort_iommu_xlate(dev, parent, streamid);
> + if (IS_ERR_OR_NULL(ops))
> + return ops;
>
> parent = iort_node_get_id(node, &streamid,
> IORT_IOMMU_TYPE, i++);
> }
> }
>
> + /*
> + * If we have reason to believe the IOMMU driver missed the initial
> + * add_device callback for dev, replay it to get things in order.
> + */
> + if (!IS_ERR_OR_NULL(ops) && ops->add_device &&
> + dev->bus && !dev->iommu_group) {
> + int err = ops->add_device(dev);
> +
> + if (err)
> + ops = ERR_PTR(err);
> + }
I think there is nothing ACPI specific in this add_device() replay
path, so there is room for further DT/ACPI consolidation here.
Without any further ado:
Acked-by: Lorenzo Pieralisi
> +
> return ops;
> }
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 1926918..823b005 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1373,20 +1373,23 @@ enum dev_dma_attr acpi_get_dma_attr(struct
> acpi_device *adev)
> * @dev: The pointer to the device
> * @attr: device dma attributes
> */
> -void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
> +int acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
> {
> const struct iommu_ops *iommu;
>
> iort_set_dma_mask(dev);
>
> iommu = iort_iommu_configure(dev);
> -
> + if (IS_ERR(iommu))
> + return PTR_ERR(iommu);
> /*
>* Assume dma valid range starts at 0 and covers the whole
>* coherent_dma_mask.
>*/
> arch_setup_dma_ops(dev, 0, dev->coherent_dma_mask + 1, iommu,
> attr == DEV_DMA_COHERENT);
> +
> + return 0;
> }
> EXPORT_SYMBOL_GPL(acpi_dma_configure);
>
> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
> index 82bd45c..755a2b5 100644
> --- a/drivers/base/dma-mapping.c
> +++ b/drivers/base/dma-mapping.c
> @@ -368,7 +368,7 @@ int dma_configure(struct device *dev)
> } else if (has_acpi_companion(dma_dev)) {
> attr = acpi_get_dma_att
RE: [PATCH V7 00/11] IOMMU probe deferral support
Hi Marek, >On 2017-01-23 17:18, Sricharan R wrote: >> This series calls the dma ops configuration for the devices >> at a generic place so that it works for all busses. >> The dma_configure_ops for a device is now called during >> the device_attach callback just before the probe of the >> bus/driver is called. Similarly dma_deconfigure is called during >> device/driver_detach path. >> >> pci_bus_add_devices(platform/amba)(_device_create/driver_register) >> | | >> pci_bus_add_device (device_add/driver_register) >> | | >> device_attach device_initial_probe >> | | >> __device_attach_driver__device_attach_driver >> | >> driver_probe_device >> | >> really_probe >> | >> dma_configure >> >> Similarly on the device/driver_unregister path __device_release_driver is >> called which inturn calls dma_deconfigure. >> >> Took the reworked patches [2] from Robin's branch and >> rebased on top of Lorenzo's ACPI IORT ARM support series [3]. >> >> * Tested with platform and pci devices for probe deferral >>and reprobe on arm64 based platform. >> * Need help for testing with ACPI. > >Once again: > >Tested-by: Marek Szyprowski > >Works fine on Exynos machines (both, ARM and ARM64 based). This patchset is >urgently needed as more and more stuff depends on it and cannot be easily >workarounded without nasty hacks (for example pending runtime PM changes >for clocks and power domains integration for Exynos 5433). > Ok, Thanks for the testing once again. >Is there a chance to have it merged to v4.11? > Ya, hoping that this time it would get through. Regards, Sricharan >> Previous post of this series [6]. >> >> [V7] >> * Updated the subject and commit log for patch #6 as per >> comments from Lorenzo. No functional changes. >> >> [V6] >> * Fixed a bug in dma_configure function pointed out by >> Robin. >> * Reordered the patches as per comments from Robin and >> Lorenzo. >> * Added Tags. >> >> [V5] >> * Reworked the pci configuration code hanging outside and >> pushed it to dma_configure as in PATCH#5,6,7. >> Also added a couple of patches that Lorenzo provided for >> correcting the Probe deferring mechanism in case of >> ACPI devices from here [5]. >> >> [V4] >> * Took the reworked patches [2] from Robin's branch and >> rebased on top of Lorenzo's ACPI IORT ARM support series [3]. >> >> * Added the patches for moving the dma ops configuration of >> acpi based devices to probe time as well. >> [V3] >> * Removed the patch to split dma_masks/dma_ops configuration >> separately based on review comments that both masks and ops are >> required only during the device probe time. >> >> * Reworked the series based on Generic DT bindings series. >> >> * Added call to iommu's remove_device in the cleanup path for arm and >> arm64. >> >> * Removed the notifier trick in arm64 to handle early device >> registration. >> >> * Added reset of dma_ops in cleanup path for arm based on comments. >> >> * Fixed the pci_iommu_configure path and tested with PCI device as >> well. >> >> * Fixed a bug to return the correct iommu_ops from patch 7 [4] in >> last post. >> >> * Fixed few other cosmetic comments. >> >> [V2] >> * Updated the Initial post to call dma_configure/deconfigure from >> generic code >> >> * Added iommu add_device callback from of_iommu_configure path >> >> [V1] >> * Initial post from Laurent Pinchart [1] >> >> [1] http://lists.linuxfoundation.org/pipermail/iommu/2015-May/013016.html >> [2] >> http://www.linux-arm.org/git?p=linux-rm.git;a=shortlog;h=refs/heads/iommu/defer >> [3] https://lkml.org/lkml/2016/11/21/141 >> [4] >> https://www.mail-archive.com/iommu@lists.linux-foundation.org/msg13940.html >> [5] git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git >> iommu/probe-deferral >> [6] https://www.spinics.net/lists/arm-kernel/msg556546.html >> >> Laurent Pinchart (3): >>of: dma: Move range size workaround to of_dma_get_range() >>of: dma: Make of_dma_deconfigure() public >>iommu: of: Handle IOMMU lookup failure with deferred probing or error >> >> Lorenzo Pieralisi (2): >>ACPI/IORT: Add function to check SMMUs drivers presence >>ACPI/IORT: Remove linker section for IORT entries probing >> >> Robin Murphy (3): >>iommu/of: Refactor of_iommu_configure() for error handling >>iommu/of: Prepare for deferred IOMMU configuration >>iommu/arm-smmu: Clean up early-probing workarounds >> >> Sricharan R (3): >>of/acpi: Configure dma operations at probe time for platform/amba/pci bus >> devices >>drivers: acpi: Handle IOMMU lookup failure with deferred probing or >> error >>arm64: dma-mappi
Re: [PATCH/RFC 2/2] iommu/ipmmu-vmsa: Opt-in slave devices based on ES version
Hi Magnus,
On Tue, Jan 24, 2017 at 10:38 AM, Magnus Damm wrote:
> On Mon, Jan 23, 2017 at 9:50 PM, Geert Uytterhoeven
> wrote:
>> On Mon, Jan 23, 2017 at 1:12 PM, Magnus Damm wrote:
>>> From: Magnus Damm
>>>
>>> Match on r8a7795 ES2 and enable a certain DMA controller.
>>> In other cases the IPMMU driver remains disabled.
>>>
>>> Signed-off-by: Magnus Damm
>>> ---
>>>
>>> drivers/iommu/ipmmu-vmsa.c | 24
>>> 1 file changed, 24 insertions(+)
>>>
>>> --- 0001/drivers/iommu/ipmmu-vmsa.c
>>> +++ work/drivers/iommu/ipmmu-vmsa.c 2017-01-23 20:57:02.620607110 +0900
>>> @@ -23,6 +23,7 @@
>>> #include
>>> #include
>>> #include
>>> +#include
>>>
>>> #if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
>>> #include
>>> @@ -1002,16 +1003,39 @@ static void ipmmu_domain_free_dma(struct
>>> }
>>> }
>>>
>>> +static const struct soc_device_attribute r8a7795es2[] = {
>>> + { .soc_id = "r8a7795", .revision = "ES2.*" },
>>> + { /* sentinel */ }
>>> +};
>>> +
>>> +static int ipmmu_slave_whitelist(struct device *dev)
>>> +{
>>> + /* Opt-in slave devices based on SoC and ES version */
>>> + if (soc_device_match(r8a7795es2)) {
>>> + if (!strcmp(dev_name(dev), "e731.dma-controller"))
>>> + return 0;
>>> + }
>>
>> I have two comments about the construct above:
>> 1. IPMMU will be disabled on all non-r8a7795 SoCs.
>> Is that what you want?
>
> Sort of. This patch is just an example to stir up some discussion
> about this topic. I realize this code as-is changes R-Car Gen2
> behavior (that is merged upstream) so perhaps we should keep devices
> enabled for those SoCs.
Indeed.
Note that we don't have any "iommus" in upstream R-Car Gen2 DTSes.
>> 2. Usually we match on the old broken versions instead (e.g. against
>> "ES1.*"), as (1) it marks more clearly support for old SoCs, and
>> (2) it makes it easier to remove the check later when these
>> old SoCs are deemed extinct later.
>
> Right, if I understand correctly then you're saying opt-out might be
> better instead of opt-in. In case of IPMMU and R-Car Gen3 I believe it
> might be less work to use opt-in rather than excluding not-yet-working
> stuff. =)
Unfortunately that may be true :-(
> With this series I would like to propose to disconnect the DT
> integration timing from the enablement of IPMMU support for slave
> devices. If we can enable the IPMMU in DT early on we can reduce
> potential out-of-tree IPMMU enablement DT patches. So with the DT
> bindings fixed and accurate data sheet we can merge DT bits ahead of
> enablement time. And then use run time logic to determine what to
> enable based on test results.
>
> As you are aware, currently we have used the presence of "iommus" in
> DT to determine if a device is going to be enabled or not. So if the
> IPMMU Kconfig bits enable the IPMMU driver and the "iommus" DT
> property tie a certain slave device to the IPMMU then we will make use
> of IPMMU for a certain device. Currently we assume it will work on all
> ES versions that use that particular DTB.
>
> However ES specific hardware errata together with a wide range of ES
> versions for r8a7795 and r8a7796 (and whatever SoCs and ES versions
> that comes next) makes it difficult to use DT like above to enable
> stuff seemingly on one ES version without potentially breaking other
> ES versions. I would like to share DT files between the ES versions as
> much as possible but still only enable IPMMU support for devices that
> are known to work.
>
> Let me know if you think it makes sense to enable DT in a different
> way than my proposal.
That makes perfect sense to me: DT describes (ideal production) hardware,
and errata are handled in C code and tables.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH/RFC 2/2] iommu/ipmmu-vmsa: Opt-in slave devices based on ES version
Hi Geert,
On Mon, Jan 23, 2017 at 9:50 PM, Geert Uytterhoeven
wrote:
> Hi Magnus,
>
> On Mon, Jan 23, 2017 at 1:12 PM, Magnus Damm wrote:
>> From: Magnus Damm
>>
>> Match on r8a7795 ES2 and enable a certain DMA controller.
>> In other cases the IPMMU driver remains disabled.
>>
>> Signed-off-by: Magnus Damm
>> ---
>>
>> drivers/iommu/ipmmu-vmsa.c | 24
>> 1 file changed, 24 insertions(+)
>>
>> --- 0001/drivers/iommu/ipmmu-vmsa.c
>> +++ work/drivers/iommu/ipmmu-vmsa.c 2017-01-23 20:57:02.620607110 +0900
>> @@ -23,6 +23,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
>> #include
>> @@ -1002,16 +1003,39 @@ static void ipmmu_domain_free_dma(struct
>> }
>> }
>>
>> +static const struct soc_device_attribute r8a7795es2[] = {
>> + { .soc_id = "r8a7795", .revision = "ES2.*" },
>> + { /* sentinel */ }
>> +};
>> +
>> +static int ipmmu_slave_whitelist(struct device *dev)
>> +{
>> + /* Opt-in slave devices based on SoC and ES version */
>> + if (soc_device_match(r8a7795es2)) {
>> + if (!strcmp(dev_name(dev), "e731.dma-controller"))
>> + return 0;
>> + }
>
> I have two comments about the construct above:
> 1. IPMMU will be disabled on all non-r8a7795 SoCs.
> Is that what you want?
Sort of. This patch is just an example to stir up some discussion
about this topic. I realize this code as-is changes R-Car Gen2
behavior (that is merged upstream) so perhaps we should keep devices
enabled for those SoCs.
> 2. Usually we match on the old broken versions instead (e.g. against
> "ES1.*"), as (1) it marks more clearly support for old SoCs, and
> (2) it makes it easier to remove the check later when these
> old SoCs are deemed extinct later.
Right, if I understand correctly then you're saying opt-out might be
better instead of opt-in. In case of IPMMU and R-Car Gen3 I believe it
might be less work to use opt-in rather than excluding not-yet-working
stuff. =)
With this series I would like to propose to disconnect the DT
integration timing from the enablement of IPMMU support for slave
devices. If we can enable the IPMMU in DT early on we can reduce
potential out-of-tree IPMMU enablement DT patches. So with the DT
bindings fixed and accurate data sheet we can merge DT bits ahead of
enablement time. And then use run time logic to determine what to
enable based on test results.
As you are aware, currently we have used the presence of "iommus" in
DT to determine if a device is going to be enabled or not. So if the
IPMMU Kconfig bits enable the IPMMU driver and the "iommus" DT
property tie a certain slave device to the IPMMU then we will make use
of IPMMU for a certain device. Currently we assume it will work on all
ES versions that use that particular DTB.
However ES specific hardware errata together with a wide range of ES
versions for r8a7795 and r8a7796 (and whatever SoCs and ES versions
that comes next) makes it difficult to use DT like above to enable
stuff seemingly on one ES version without potentially breaking other
ES versions. I would like to share DT files between the ES versions as
much as possible but still only enable IPMMU support for devices that
are known to work.
Let me know if you think it makes sense to enable DT in a different
way than my proposal.
I'll have a look at putting the white list code in ->xlate() instead
of ->add_device().
Thanks for your comments!
Cheers,
/ magnus
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH/RFC 1/2] arm64: mm: Silently allow devices lacking IOMMU group
Hi Robin,
>> Consider failure of iommu_get_domain_for_dev() as non-critical and
>> get rid of the warning printout. This allows IOMMU properties to be
>> included in the DTB even though the kernel is configured with
>> CONFIG_IOMMU_API=n or in case a particular IOMMU driver refuses to
>> enable IOMMU support for a certain slave device and returns error
>> from the ->add_device() callback.
>>
>> This is only a cosmetic change that removes console warning printouts.
>
>The warning is there for a reason - at this point, we *expected* the
>device to be using an IOMMU for DMA, so a failure is significant. Rather
>than masking genuine failures in other cases because your case
>deliberately breaks that expectation, simply change the expectation -
>i.e. rather than letting of_xlate() succeed then failing add_device()
>later, reject the of_xlate() call up-front such that the DMA layer never
>gets told about the IOMMU in the first place.
>
>Robin.
>
With the iommu probe deferral patches, this behavior would change
where the arch_setup_dma_ops would never be called if there is
an error from xlate or add_device. But also the error value from
xlate/add_device is returned back and the probe of the device
would fail for any error. So if there can be cases like above, where
the xlate/add_device callbacks can return error for specific reasons,
should only EPROBE_DEFER be considered and rest of the errors
be filtered out with a WARN probably ?
Regards,
Sricharan
>> Signed-off-by: Magnus Damm
>> ---
>>
>> arch/arm64/mm/dma-mapping.c | 10 +-
>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> --- 0001/arch/arm64/mm/dma-mapping.c
>> +++ work/arch/arm64/mm/dma-mapping.c 2017-01-23 20:54:40.060607110 +0900
>> @@ -827,11 +827,19 @@ static bool do_iommu_attach(struct devic
>> struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
>>
>> /*
>> + * In case IOMMU support is excluded from the kernel or if the device
>> + * is not hooked up to any IOMMU group then be silent and keep the
>> + * old dma_ops.
>> + */
>> +if (!domain)
>> +return false;
>> +
>> +/*
>> * If the IOMMU driver has the DMA domain support that we require,
>> * then the IOMMU core will have already configured a group for this
>> * device, and allocated the default domain for that group.
>> */
>> -if (!domain || iommu_dma_init_domain(domain, dma_base, size, dev)) {
>> +if (iommu_dma_init_domain(domain, dma_base, size, dev)) {
>> pr_warn("Failed to set up IOMMU for device %s; retaining
>> platform DMA ops\n",
>> dev_name(dev));
>> return false;
>> ___
>> iommu mailing list
>> iommu@lists.linux-foundation.org
>> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>>
>
>___
>iommu mailing list
>iommu@lists.linux-foundation.org
>https://lists.linuxfoundation.org/mailman/listinfo/iommu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
