Re: [PATCH] iommu/ipmmu-vmsa: Fix IOMMU lookup when multiple IOMMUs are registered
Hi Joerg,
Could you please pick this patch up for v3.20 ?
On Saturday 24 January 2015 23:13:50 Laurent Pinchart wrote:
> When adding a new device the driver loops over all registered IOMMUs and
> calls the ipmmu_find_utlbs() function to parse the DT iommus attribute.
> The function returns an error when the IOMMU referenced in DT doesn't
> match the current IOMMU. The caller incorrectly breaks from the loop
> immediately when the error is reported, resulting in only the first
> IOMMU being considered.
>
> Fix this, and while at it move code that isn't specific to an IOMMU
> instance out of the loop.
>
> Signed-off-by: Laurent Pinchart
> ---
> drivers/iommu/ipmmu-vmsa.c | 49 +++
> 1 file changed, 21 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> index 791c3daec7c0..407324132587 100644
> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -1007,45 +1007,28 @@ static phys_addr_t ipmmu_iova_to_phys(struct
> iommu_domain *io_domain, }
>
> static int ipmmu_find_utlbs(struct ipmmu_vmsa_device *mmu, struct device
> *dev, - unsigned int **_utlbs)
> + unsigned int *utlbs, unsigned int num_utlbs)
> {
> - unsigned int *utlbs;
> unsigned int i;
> - int count;
> -
> - count = of_count_phandle_with_args(dev->of_node, "iommus",
> -"#iommu-cells");
> - if (count < 0)
> - return -EINVAL;
> -
> - utlbs = kcalloc(count, sizeof(*utlbs), GFP_KERNEL);
> - if (!utlbs)
> - return -ENOMEM;
>
> - for (i = 0; i < count; ++i) {
> + for (i = 0; i < num_utlbs; ++i) {
> struct of_phandle_args args;
> int ret;
>
> ret = of_parse_phandle_with_args(dev->of_node, "iommus",
>"#iommu-cells", i, &args);
> if (ret < 0)
> - goto error;
> + return ret;
>
> of_node_put(args.np);
>
> if (args.np != mmu->dev->of_node || args.args_count != 1)
> - goto error;
> + return -EINVAL;
>
> utlbs[i] = args.args[0];
> }
>
> - *_utlbs = utlbs;
> -
> - return count;
> -
> -error:
> - kfree(utlbs);
> - return -EINVAL;
> + return 0;
> }
>
> static int ipmmu_add_device(struct device *dev)
> @@ -1053,10 +1036,10 @@ static int ipmmu_add_device(struct device *dev)
> struct ipmmu_vmsa_archdata *archdata;
> struct ipmmu_vmsa_device *mmu;
> struct iommu_group *group = NULL;
> - unsigned int *utlbs = NULL;
> + unsigned int *utlbs;
> unsigned int i;
> - int num_utlbs = 0;
> - int ret;
> + int num_utlbs;
> + int ret = -ENODEV;
>
> if (dev->archdata.iommu) {
> dev_warn(dev, "IOMMU driver already assigned to device %s\n",
> @@ -1065,11 +1048,21 @@ static int ipmmu_add_device(struct device *dev)
> }
>
> /* Find the master corresponding to the device. */
> +
> + num_utlbs = of_count_phandle_with_args(dev->of_node, "iommus",
> +"#iommu-cells");
> + if (num_utlbs < 0)
> + return -ENODEV;
> +
> + utlbs = kcalloc(num_utlbs, sizeof(*utlbs), GFP_KERNEL);
> + if (!utlbs)
> + return -ENOMEM;
> +
> spin_lock(&ipmmu_devices_lock);
>
> list_for_each_entry(mmu, &ipmmu_devices, list) {
> - num_utlbs = ipmmu_find_utlbs(mmu, dev, &utlbs);
> - if (num_utlbs) {
> + ret = ipmmu_find_utlbs(mmu, dev, utlbs, num_utlbs);
> + if (!ret) {
> /*
>* TODO Take a reference to the MMU to protect
>* against device removal.
> @@ -1080,7 +1073,7 @@ static int ipmmu_add_device(struct device *dev)
>
> spin_unlock(&ipmmu_devices_lock);
>
> - if (num_utlbs <= 0)
> + if (ret < 0)
> return -ENODEV;
>
> for (i = 0; i < num_utlbs; ++i) {
--
Regards,
Laurent Pinchart
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 3/6] of: fix size when dma-range is not used
On Tue, Jan 27, 2015 at 06:55:15PM +, Murali Karicheri wrote:
> On 01/27/2015 06:27 AM, Robin Murphy wrote:
> > On 23/01/15 22:32, Murali Karicheri wrote:
> >> Fix the dma-range size when the DT attribute is missing. i.e set size to
> >> dev->coherent_dma_mask + 1 instead of dev->coherent_dma_mask. To detect
> >> overflow when mask is set to max of u64, add a check, log error and
> >> return.
> >> Some platform use mask format for size in DTS. So add a work around to
> >> catch this and fix.
> >>
> >> Cc: Joerg Roedel
> >> Cc: Grant Likely
> >> Cc: Rob Herring
> >> Cc: Bjorn Helgaas
> >> Cc: Will Deacon
> >> Cc: Russell King
> >> Cc: Arnd Bergmann
> >> Cc: Suravee Suthikulpanit
> >>
> >> Signed-off-by: Murali Karicheri
> >> ---
> >> drivers/of/device.c | 14 +-
> >> 1 file changed, 13 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/of/device.c b/drivers/of/device.c
> >> index 2de320d..0a5ff54 100644
> >> --- a/drivers/of/device.c
> >> +++ b/drivers/of/device.c
> >> @@ -105,12 +105,24 @@ void of_dma_configure(struct device *dev, struct
> >> device_node *np)
> >> ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
> >> if (ret < 0) {
> >> dma_addr = offset = 0;
> >> - size = dev->coherent_dma_mask;
> >> + size = dev->coherent_dma_mask + 1;
> >> } else {
> >> offset = PFN_DOWN(paddr - dma_addr);
> >> + /*
> >> + * Add a work around to treat the size as mask + 1 in case
> >> + * it is defined in DT as a mask.
> >> + */
> >> + if (size & 1)
> >> + size = size + 1;
> >> dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
> >> }
> >>
> >> + /* if size is 0, we have an overflow of u64 */
> >> + if (!size) {
> >> + dev_err(dev, "invalid size\n");
> >> + return;
> >> + }
> >> +
> >
> > This seems potentially fragile to dodgy DTs given that we might also be
> > using size to make a mask later. Would it make sense to double-up a
> > sanity check as mask-format detection? Something like:
> >
> > if is_power_of_2(size)
> > // use size
> > else if is_power_of_2(size + 1)
> > // use size + 1
> > else
> > // cry
>
> How about having the logic like this?
>
> ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
> if (ret < 0) {
> dma_addr = offset = 0;
> size = dev->coherent_dma_mask + 1;
> } else {
> offset = PFN_DOWN(paddr - dma_addr);
> dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
> }
>
> if (is_power_of_2(size + 1))
> size = size + 1;
> else if (!is_power_of_2(size))
> {
> dev_err(dev, "invalid size\n");
> return;
> }
In of_dma_configure(), we currently assume that the default coherent
mask is 32-bit. In this thread:
http://article.gmane.org/gmane.linux.kernel/1835096
we talked about setting the coherent mask based on size automatically.
I'm not sure about the size but I think we can assume is 32-bit mask + 1
if it is not specified in the DT. Instead of just assuming a default
mask, let's assume a default size and create the mask based on this
(untested):
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 5b33c6a21807..9ff8d1286b44 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -170,10 +170,10 @@ static void of_dma_configure(struct device *dev)
struct iommu_ops *iommu;
/*
-* Set default dma-mask to 32 bit. Drivers are expected to setup
-* the correct supported dma_mask.
+* Set default size to cover the 32-bit. Drivers are expected to setup
+* the correct size and dma_mask.
*/
- dev->coherent_dma_mask = DMA_BIT_MASK(32);
+ size = 1ULL << 32;
/*
* Set it to coherent_dma_mask by default if the architecture
@@ -185,13 +185,24 @@ static void of_dma_configure(struct device *dev)
ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size);
if (ret < 0) {
dma_addr = offset = 0;
- size = dev->coherent_dma_mask;
} else {
offset = PFN_DOWN(paddr - dma_addr);
dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", dev->dma_pfn_offset);
}
dev->dma_pfn_offset = offset;
+ /*
+* Workaround for DTs setting the size to a mask or 0.
+*/
+ if (is_power_of_2(size + 1))
+ size += 1;
+
+ /*
+* Coherent DMA masks larger than 32-bit must be explicitly set by the
+* driver.
+*/
+ dev->coherent_dma_mask = min(DMA_BIT_MASK(32),
DMA_BIT_MASK(ilog2(size)));
+
coherent = of_dma_is_coherent(dev->of_node);
dev_dbg(dev, "device is%sdma coherent\n",
coherent ? " " : " not ");
--
Catalin
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 3/8] of: fix size when dma-range is not used
Hi Murali,
[sorry, missed replying to yesterday's version]
On 27/01/15 21:00, Murali Karicheri wrote:
Fix the dma-range size when the DT attribute is missing. i.e set size to
dev->coherent_dma_mask + 1 instead of dev->coherent_dma_mask. Also add
code to check invalid values of size configured in DT and log error.
Cc: Joerg Roedel
Cc: Grant Likely
Cc: Rob Herring
Cc: Bjorn Helgaas
Cc: Will Deacon
Cc: Russell King
Cc: Arnd Bergmann
Cc: Suravee Suthikulpanit
Signed-off-by: Murali Karicheri
---
drivers/of/device.c |9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 2de320d..17504f4 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -105,12 +105,19 @@ void of_dma_configure(struct device *dev, struct
device_node *np)
ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
if (ret < 0) {
dma_addr = offset = 0;
- size = dev->coherent_dma_mask;
+ size = dev->coherent_dma_mask + 1;
} else {
offset = PFN_DOWN(paddr - dma_addr);
dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
}
+ if (is_power_of_2(size + 1))
+ size = size + 1;
+ else if (!is_power_of_2(size)) {
+ dev_err(dev, "invalid size\n");
+ return;
+ }
+
Couldn't these checks go into the "else" path above? We don't need to
check the non-DT case, because we know we've just set it to something
sensible.
Robin.
dev->dma_pfn_offset = offset;
coherent = of_dma_is_coherent(np);
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/6] of: iommu: add ptr to OF node arg to of_iommu_configure()
On Mon, Jan 26, 2015 at 06:49:01PM +, Murali Karicheri wrote:
> On 01/25/2015 08:32 AM, Laurent Pinchart wrote:
> > Hi Murali,
> >
> > Thank you for the patch.
> >
> > On Friday 23 January 2015 17:32:34 Murali Karicheri wrote:
> >> Function of_iommu_configure() is called from of_dma_configure() to
> >> setup iommu ops using DT property. This API is currently used for
> >> platform devices for which DMA configuration (including iommu ops)
> >> may come from device's parent. To extend this functionality for PCI
> >> devices, this API need to take a parent node ptr as an argument
> >> instead of assuming device's parent. This is needed since for PCI, the
> >> dma configuration may be defined in the DT node of the root bus bridge's
> >> parent device. Currently only dma-range is used for PCI and iommu is not
> >> supported. So return error if the device is PCI.
> >>
> >> Cc: Joerg Roedel
> >> Cc: Grant Likely
> >> Cc: Rob Herring
> >> Cc: Bjorn Helgaas
> >> Cc: Will Deacon
> >> Cc: Russell King
> >> Cc: Arnd Bergmann
> >> Cc: Suravee Suthikulpanit
> >>
> >> Signed-off-by: Murali Karicheri
> >> ---
> >> drivers/iommu/of_iommu.c | 10 --
> >> drivers/of/platform.c|2 +-
> >> include/linux/of_iommu.h |6 --
> >> 3 files changed, 13 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> >> index af1dc6a..439235b 100644
> >> --- a/drivers/iommu/of_iommu.c
> >> +++ b/drivers/iommu/of_iommu.c
> >> @@ -133,19 +133,25 @@ struct iommu_ops *of_iommu_get_ops(struct device_node
> >> *np) return ops;
> >> }
> >>
> >> -struct iommu_ops *of_iommu_configure(struct device *dev)
> >> +struct iommu_ops *of_iommu_configure(struct device *dev,
> >> + struct device_node *iommu_np)
> >> {
> >>struct of_phandle_args iommu_spec;
> >>struct device_node *np;
> >>struct iommu_ops *ops = NULL;
> >>int idx = 0;
> >>
> >> + if (dev_is_pci(dev)) {
> >> + dev_err(dev, "iommu is currently not supported for PCI\n");
> >> + return NULL;
> >> + }
> >> +
> >>/*
> >> * We don't currently walk up the tree looking for a parent IOMMU.
> >> * See the `Notes:' section of
> >> * Documentation/devicetree/bindings/iommu/iommu.txt
> >> */
> > Wouldn't it be better to fix this rather than passing the device node
> > pointer
> > to the function ? The solution would be more generic.
> Laurent,
>
> Will Deacon (Copied here) is working on this as we alteady discussed in
> this thread. So it will be
> addressed by him in another series.
Well, "working on this" equates to "has it somewhere near the bottom of
a very long list" :)
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/arm-smmu: use a threaded handler for context interrupts
On Fri, Jan 23, 2015 at 10:33:20PM +, Mitchel Humpherys wrote:
> On Fri, Jan 23 2015 at 03:24:15 AM, Will Deacon wrote:
> > On Thu, Jan 22, 2015 at 11:48:02PM +, Mitchel Humpherys wrote:
> >> Context interrupts can call domain-specific handlers which might sleep.
> >> Currently we register our handler with request_irq, so our handler is
> >> called in atomic context, so domain handlers that sleep result in an
> >> invalid context BUG. Fix this by using request_threaded_irq.
> >>
> >> This also prepares the way for doing things like enabling clocks within
> >> our interrupt handler.
> >>
> >> Signed-off-by: Mitchel Humpherys
> >> ---
> >> drivers/iommu/arm-smmu.c | 5 +++--
> >> 1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> >> index 6cd47b75286f..81f6b54d94b1 100644
> >> --- a/drivers/iommu/arm-smmu.c
> >> +++ b/drivers/iommu/arm-smmu.c
> >> @@ -973,8 +973,9 @@ static int arm_smmu_init_domain_context(struct
> >> iommu_domain *domain,
> >>spin_unlock_irqrestore(&smmu_domain->lock, flags);
> >>
> >>irq = smmu->irqs[smmu->num_global_irqs + cfg->irptndx];
> >> - ret = request_irq(irq, arm_smmu_context_fault, IRQF_SHARED,
> >> -"arm-smmu-context-fault", domain);
> >> + ret = request_threaded_irq(irq, NULL, arm_smmu_context_fault,
> >> + IRQF_ONESHOT | IRQF_SHARED,
> >> + "arm-smmu-context-fault", domain);
> >>if (IS_ERR_VALUE(ret)) {
> >>dev_err(smmu->dev, "failed to request context IRQ %d (%u)\n",
> >>cfg->irptndx, irq);
> >
> > I think I'd rather keep a simple atomic handler, then have a threaded
> > handler for actually issuing the report_iommu_fault. i.e. we only wake
> > the thread when it looks like there's some work to do. That also works
> > much better for shared interrupts.
>
> Are you still against adding clock support to the driver? If not, we'll
> need to move to a threaded handler when clocks come in anyways...
>
> Can you elaborate what you mean regarding shared interrupts? Even
> without clocks it seems like the code clarity / performance tradeoff
> would favor a threaded handler, given that performance isn't important
> here.
With a shared handler (e.g. a bunch of context banks have the same IRQ)
then I assume that we don't want to end up with multiple handler threads
all tripping over each other. I'd rather have one thread that handles work
queued up by multiple low-level handlers.
Do you have a preference either way?
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/6] of: iommu: add ptr to OF node arg to of_iommu_configure()
Hi Will,
On Wednesday 28 January 2015 11:33:00 Will Deacon wrote:
> On Mon, Jan 26, 2015 at 06:49:01PM +, Murali Karicheri wrote:
> > On 01/25/2015 08:32 AM, Laurent Pinchart wrote:
> >> On Friday 23 January 2015 17:32:34 Murali Karicheri wrote:
> >>> Function of_iommu_configure() is called from of_dma_configure() to
> >>> setup iommu ops using DT property. This API is currently used for
> >>> platform devices for which DMA configuration (including iommu ops)
> >>> may come from device's parent. To extend this functionality for PCI
> >>> devices, this API need to take a parent node ptr as an argument
> >>> instead of assuming device's parent. This is needed since for PCI, the
> >>> dma configuration may be defined in the DT node of the root bus
> >>> bridge's parent device. Currently only dma-range is used for PCI and
> >>> iommu is not supported. So return error if the device is PCI.
> >>>
> >>> Cc: Joerg Roedel
> >>> Cc: Grant Likely
> >>> Cc: Rob Herring
> >>> Cc: Bjorn Helgaas
> >>> Cc: Will Deacon
> >>> Cc: Russell King
> >>> Cc: Arnd Bergmann
> >>> Cc: Suravee Suthikulpanit
> >>>
> >>> Signed-off-by: Murali Karicheri
> >>> ---
> >>>
> >>> drivers/iommu/of_iommu.c | 10 --
> >>> drivers/of/platform.c|2 +-
> >>> include/linux/of_iommu.h |6 --
> >>> 3 files changed, 13 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> >>> index af1dc6a..439235b 100644
> >>> --- a/drivers/iommu/of_iommu.c
> >>> +++ b/drivers/iommu/of_iommu.c
> >>> @@ -133,19 +133,25 @@ struct iommu_ops *of_iommu_get_ops(struct
> >>> device_node *np)
> >>> return ops;
> >>> }
> >>>
> >>> -struct iommu_ops *of_iommu_configure(struct device *dev)
> >>> +struct iommu_ops *of_iommu_configure(struct device *dev,
> >>> + struct device_node *iommu_np)
> >>> {
> >>> struct of_phandle_args iommu_spec;
> >>> struct device_node *np;
> >>> struct iommu_ops *ops = NULL;
> >>> int idx = 0;
> >>>
> >>> + if (dev_is_pci(dev)) {
> >>> + dev_err(dev, "iommu is currently not supported for PCI\n");
> >>> + return NULL;
> >>> + }
> >>> +
> >>> /*
> >>>* We don't currently walk up the tree looking for a parent
> >>> IOMMU.
> >>>* See the `Notes:' section of
> >>>* Documentation/devicetree/bindings/iommu/iommu.txt
> >>>*/
> >>
> >> Wouldn't it be better to fix this rather than passing the device node
> >> pointer to the function ? The solution would be more generic.
> >
> > Laurent,
> >
> > Will Deacon (Copied here) is working on this as we alteady discussed in
> > this thread. So it will be addressed by him in another series.
>
> Well, "working on this" equates to "has it somewhere near the bottom of
> a very long list" :)
What's your opinion on this patch then, do you think adding the iommu_np
argument makes sense as a temporary hack, or should we instead walk up the
tree looking for an iommus attribute in parent nodes ? I don't expect that to
be insanely difficult to code.
--
Regards,
Laurent Pinchart
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/6] of: iommu: add ptr to OF node arg to of_iommu_configure()
On Wed, Jan 28, 2015 at 12:23:03PM +, Laurent Pinchart wrote:
> On Wednesday 28 January 2015 11:33:00 Will Deacon wrote:
> > On Mon, Jan 26, 2015 at 06:49:01PM +, Murali Karicheri wrote:
> > > On 01/25/2015 08:32 AM, Laurent Pinchart wrote:
> > >> On Friday 23 January 2015 17:32:34 Murali Karicheri wrote:
> > >>> Function of_iommu_configure() is called from of_dma_configure() to
> > >>> setup iommu ops using DT property. This API is currently used for
> > >>> platform devices for which DMA configuration (including iommu ops)
> > >>> may come from device's parent. To extend this functionality for PCI
> > >>> devices, this API need to take a parent node ptr as an argument
> > >>> instead of assuming device's parent. This is needed since for PCI, the
> > >>> dma configuration may be defined in the DT node of the root bus
> > >>> bridge's parent device. Currently only dma-range is used for PCI and
> > >>> iommu is not supported. So return error if the device is PCI.
> > >>>
> > >>> Cc: Joerg Roedel
> > >>> Cc: Grant Likely
> > >>> Cc: Rob Herring
> > >>> Cc: Bjorn Helgaas
> > >>> Cc: Will Deacon
> > >>> Cc: Russell King
> > >>> Cc: Arnd Bergmann
> > >>> Cc: Suravee Suthikulpanit
> > >>>
> > >>> Signed-off-by: Murali Karicheri
> > >>> ---
> > >>>
> > >>> drivers/iommu/of_iommu.c | 10 --
> > >>> drivers/of/platform.c|2 +-
> > >>> include/linux/of_iommu.h |6 --
> > >>> 3 files changed, 13 insertions(+), 5 deletions(-)
> > >>>
> > >>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> > >>> index af1dc6a..439235b 100644
> > >>> --- a/drivers/iommu/of_iommu.c
> > >>> +++ b/drivers/iommu/of_iommu.c
> > >>> @@ -133,19 +133,25 @@ struct iommu_ops *of_iommu_get_ops(struct
> > >>> device_node *np)
> > >>> return ops;
> > >>> }
> > >>>
> > >>> -struct iommu_ops *of_iommu_configure(struct device *dev)
> > >>> +struct iommu_ops *of_iommu_configure(struct device *dev,
> > >>> +struct device_node *iommu_np)
> > >>> {
> > >>> struct of_phandle_args iommu_spec;
> > >>> struct device_node *np;
> > >>> struct iommu_ops *ops = NULL;
> > >>> int idx = 0;
> > >>>
> > >>> + if (dev_is_pci(dev)) {
> > >>> + dev_err(dev, "iommu is currently not supported for
> > >>> PCI\n");
> > >>> + return NULL;
> > >>> + }
> > >>> +
> > >>> /*
> > >>> * We don't currently walk up the tree looking for a parent
> > >>> IOMMU.
> > >>> * See the `Notes:' section of
> > >>> * Documentation/devicetree/bindings/iommu/iommu.txt
> > >>> */
> > >>
> > >> Wouldn't it be better to fix this rather than passing the device node
> > >> pointer to the function ? The solution would be more generic.
> > >
> > > Will Deacon (Copied here) is working on this as we alteady discussed in
> > > this thread. So it will be addressed by him in another series.
> >
> > Well, "working on this" equates to "has it somewhere near the bottom of
> > a very long list" :)
>
> What's your opinion on this patch then, do you think adding the iommu_np
> argument makes sense as a temporary hack, or should we instead walk up the
> tree looking for an iommus attribute in parent nodes ? I don't expect that to
> be insanely difficult to code.
If walking up the tree is useful, then I think we should do that and update
the Documentation to reflect the new behaviour. The only reason that I
didn't code that in of_iommu_configure originally is because I didn't want
to go against the binding spec for the initial version, especially as we
didn't have a reason to look at parent nodes.
The only thing to double-check is that we don't break any existing users
of the binding with this change, but I don't think that we do.
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/6] of: iommu: add ptr to OF node arg to of_iommu_configure()
On Wednesday 28 January 2015 12:29:42 Will Deacon wrote:
> On Wed, Jan 28, 2015 at 12:23:03PM +, Laurent Pinchart wrote:
> > On Wednesday 28 January 2015 11:33:00 Will Deacon wrote:
> >> On Mon, Jan 26, 2015 at 06:49:01PM +, Murali Karicheri wrote:
> >>> On 01/25/2015 08:32 AM, Laurent Pinchart wrote:
> On Friday 23 January 2015 17:32:34 Murali Karicheri wrote:
> > Function of_iommu_configure() is called from of_dma_configure() to
> > setup iommu ops using DT property. This API is currently used for
> > platform devices for which DMA configuration (including iommu ops)
> > may come from device's parent. To extend this functionality for PCI
> > devices, this API need to take a parent node ptr as an argument
> > instead of assuming device's parent. This is needed since for PCI,
> > the dma configuration may be defined in the DT node of the root bus
> > bridge's parent device. Currently only dma-range is used for PCI and
> > iommu is not supported. So return error if the device is PCI.
> >
> > Cc: Joerg Roedel
> > Cc: Grant Likely
> > Cc: Rob Herring
> > Cc: Bjorn Helgaas
> > Cc: Will Deacon
> > Cc: Russell King
> > Cc: Arnd Bergmann
> > Cc: Suravee Suthikulpanit
> >
> > Signed-off-by: Murali Karicheri
> > ---
> >
> > drivers/iommu/of_iommu.c | 10 --
> > drivers/of/platform.c|2 +-
> > include/linux/of_iommu.h |6 --
> > 3 files changed, 13 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> > index af1dc6a..439235b 100644
> > --- a/drivers/iommu/of_iommu.c
> > +++ b/drivers/iommu/of_iommu.c
> > @@ -133,19 +133,25 @@ struct iommu_ops *of_iommu_get_ops(struct
> > device_node *np)
> > return ops;
> > }
> >
> > -struct iommu_ops *of_iommu_configure(struct device *dev)
> > +struct iommu_ops *of_iommu_configure(struct device *dev,
> > +struct device_node *iommu_np)
> > {
> > struct of_phandle_args iommu_spec;
> > struct device_node *np;
> > struct iommu_ops *ops = NULL;
> > int idx = 0;
> >
> > + if (dev_is_pci(dev)) {
> > + dev_err(dev, "iommu is currently not supported for
> > PCI\n");
> > + return NULL;
> > + }
> > +
> > /*
> > * We don't currently walk up the tree looking for a parent
> > IOMMU.
> > * See the `Notes:' section of
> > * Documentation/devicetree/bindings/iommu/iommu.txt
> > */
>
> Wouldn't it be better to fix this rather than passing the device node
> pointer to the function ? The solution would be more generic.
> >>>
> >>> Will Deacon (Copied here) is working on this as we alteady discussed
> >>> in this thread. So it will be addressed by him in another series.
> >>
> >> Well, "working on this" equates to "has it somewhere near the bottom of
> >> a very long list" :)
> >
> > What's your opinion on this patch then, do you think adding the iommu_np
> > argument makes sense as a temporary hack, or should we instead walk up the
> > tree looking for an iommus attribute in parent nodes ? I don't expect that
> > to be insanely difficult to code.
>
> If walking up the tree is useful, then I think we should do that and update
> the Documentation to reflect the new behaviour.
If I understand Murali's patch set right (please correct me if that's not the
case) the PCI code walks up the DT nodes hierarchy to the parent node that
contains the iommus attribute and passes a pointer to that node to this
function. It's thus a PCI-specific solution. As a temporary hack that's OK I
suppose, but if implementing it right straight away isn't difficult that would
be better.
> The only reason that I didn't code that in of_iommu_configure originally is
> because I didn't want to go against the binding spec for the initial
> version, especially as we didn't have a reason to look at parent nodes.
>
> The only thing to double-check is that we don't break any existing users
> of the binding with this change, but I don't think that we do.
--
Regards,
Laurent Pinchart
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/6] of: iommu: add ptr to OF node arg to of_iommu_configure()
On Wed, Jan 28, 2015 at 01:15:10PM +, Laurent Pinchart wrote:
> On Wednesday 28 January 2015 12:29:42 Will Deacon wrote:
> > On Wed, Jan 28, 2015 at 12:23:03PM +, Laurent Pinchart wrote:
> > > On Wednesday 28 January 2015 11:33:00 Will Deacon wrote:
> > >> On Mon, Jan 26, 2015 at 06:49:01PM +, Murali Karicheri wrote:
> > >>> On 01/25/2015 08:32 AM, Laurent Pinchart wrote:
> > On Friday 23 January 2015 17:32:34 Murali Karicheri wrote:
> > > Function of_iommu_configure() is called from of_dma_configure() to
> > > setup iommu ops using DT property. This API is currently used for
> > > platform devices for which DMA configuration (including iommu ops)
> > > may come from device's parent. To extend this functionality for PCI
> > > devices, this API need to take a parent node ptr as an argument
> > > instead of assuming device's parent. This is needed since for PCI,
> > > the dma configuration may be defined in the DT node of the root bus
> > > bridge's parent device. Currently only dma-range is used for PCI and
> > > iommu is not supported. So return error if the device is PCI.
> > >
> > > Cc: Joerg Roedel
> > > Cc: Grant Likely
> > > Cc: Rob Herring
> > > Cc: Bjorn Helgaas
> > > Cc: Will Deacon
> > > Cc: Russell King
> > > Cc: Arnd Bergmann
> > > Cc: Suravee Suthikulpanit
> > >
> > > Signed-off-by: Murali Karicheri
> > > ---
> > >
> > > drivers/iommu/of_iommu.c | 10 --
> > > drivers/of/platform.c|2 +-
> > > include/linux/of_iommu.h |6 --
> > > 3 files changed, 13 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> > > index af1dc6a..439235b 100644
> > > --- a/drivers/iommu/of_iommu.c
> > > +++ b/drivers/iommu/of_iommu.c
> > > @@ -133,19 +133,25 @@ struct iommu_ops *of_iommu_get_ops(struct
> > > device_node *np)
> > > return ops;
> > > }
> > >
> > > -struct iommu_ops *of_iommu_configure(struct device *dev)
> > > +struct iommu_ops *of_iommu_configure(struct device *dev,
> > > + struct device_node *iommu_np)
> > > {
> > > struct of_phandle_args iommu_spec;
> > > struct device_node *np;
> > > struct iommu_ops *ops = NULL;
> > > int idx = 0;
> > >
> > > + if (dev_is_pci(dev)) {
> > > + dev_err(dev, "iommu is currently not supported for
> > > PCI\n");
> > > + return NULL;
> > > + }
> > > +
> > > /*
> > >* We don't currently walk up the tree looking for a parent
> > >IOMMU.
> > >* See the `Notes:' section of
> > >* Documentation/devicetree/bindings/iommu/iommu.txt
> > >*/
> >
> > Wouldn't it be better to fix this rather than passing the device node
> > pointer to the function ? The solution would be more generic.
> > >>>
> > >>> Will Deacon (Copied here) is working on this as we alteady discussed
> > >>> in this thread. So it will be addressed by him in another series.
> > >>
> > >> Well, "working on this" equates to "has it somewhere near the bottom of
> > >> a very long list" :)
> > >
> > > What's your opinion on this patch then, do you think adding the iommu_np
> > > argument makes sense as a temporary hack, or should we instead walk up the
> > > tree looking for an iommus attribute in parent nodes ? I don't expect that
> > > to be insanely difficult to code.
> >
> > If walking up the tree is useful, then I think we should do that and update
> > the Documentation to reflect the new behaviour.
>
> If I understand Murali's patch set right (please correct me if that's not the
> case) the PCI code walks up the DT nodes hierarchy to the parent node that
> contains the iommus attribute and passes a pointer to that node to this
> function. It's thus a PCI-specific solution. As a temporary hack that's OK I
> suppose, but if implementing it right straight away isn't difficult that
> would
> be better.
It looks to me like the code walks the PCI topology to get the DT node for
the host controller, and passes *that* to of_dma_configure. That sounds
like the right thing to do to me, especially since the PCI topology is
likely not encoded in the device-tree. So actually, it is passing the
first parent node afaict.
The part I'm more interested in is the mapping of RequesterID (PCI dma
alias) to IOMMU ID when we transition from the PCI topology to the DT
topology. Currently, it seems to be 1:1 on the platforms I have, but I
doubt this will always be the case.
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 3/5] iommu: implement common IOMMU ops for DMA mapping
On Tue, Jan 27, 2015 at 12:38:09PM +, Joerg Roedel wrote: > On Tue, Jan 27, 2015 at 12:27:39PM +, Robin Murphy wrote: > > Laz^WPragmatism - I'm expecting quite a lot of changes to get this > > looking good, so keeping the series as lean as possible to aid > > reviewing/rebasing/etc. seemed sensible. In the same vein, since the > > other architectures already have code that works, my priority is > > getting something in place to fill the gap in arm64 (my current > > remit is "get the SMMUs on Juno working"); it seemed logical to > > minimise disruption and dependencies by aiming to get this merged > > with the one user, then start porting the others (and making the > > inevitable necessary tweaks) once it's in. > > > > I'll adjust the commit message to make that clearer - on re-reading > > it, it does come across as rather vague about that intent. > > Yeah, probably we can add other architectures later (like x86). But can > you at least merge it with the existing version of this for ARM32? That > should be easier to achieve than extending it for x86 by now and we do > not end up with two similar implementations. +1 on that front. We've already had some breakage by using the arm_iommu_* API for the automatic DMA mapping bits, so I'd love to have dma-mapping use the same core code between arm and arm64 as soon as we can, leaving the ARM-specific API for the (hopefully diminishing) set of explicit callers. Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 02/15] iommu: Introduce iommu domain types
Hi Joerg,
Thanks for posting this!
On Mon, Jan 26, 2015 at 11:51:32PM +, Joerg Roedel wrote:
> From: Joerg Roedel
>
> This allows to handle domains differently based on their
> type in the future. An IOMMU driver can implement certain
> optimizations for DMA-API domains for example.
>
> Signed-off-by: Joerg Roedel
> ---
> drivers/iommu/iommu.c | 5 +++--
> include/linux/iommu.h | 11 ++-
> 2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 684efc0..ab24d77 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -909,14 +909,15 @@ struct iommu_domain *iommu_domain_alloc(struct bus_type
> *bus)
> ops = bus->iommu_ops;
>
> if (ops->domain_alloc)
> - domain = ops->domain_alloc();
> + domain = ops->domain_alloc(IOMMU_DOMAIN_UNMANAGED);
> else
> domain = kzalloc(sizeof(*domain), GFP_KERNEL);
>
> if (!domain)
> return NULL;
>
> - domain->ops = bus->iommu_ops;
> + domain->ops = bus->iommu_ops;
> + domain->type = IOMMU_DOMAIN_UNMANAGED;
>
> if (ops->domain_init && domain->ops->domain_init(domain))
> goto out_free;
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 69d1d12..0b67f65 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -51,7 +51,16 @@ struct iommu_domain_geometry {
> bool force_aperture; /* DMA only allowed in mappable range? */
> };
>
> +/* This are the possible domain-types */
> +enum iommu_domain_type {
> + IOMMU_DOMAIN_DMA, /* Domain used for DMA-API */
> + IOMMU_DOMAIN_IDENTITY, /* Identity mapped domain */
What happens if somebody calls map or unmap on an identity-mapping domain?
Can we catch that in the IOMMU core before calling the IOMMU driver? That
also implies we need something extra to parameterise the attributes for
the mapping (e.g. cacheable, read-only) and also potentially the address
range.
> + IOMMU_DOMAIN_UNMANAGED, /* Domain mappings are managed by a third party
> +user (like KVM or VFIO) */
We already have the domain attributes (iommu_attr) to describe features
of a domain. Is there really a need for this extra type, or can we extend
the attribute set and allow for domain allocation with attributes?
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/5] iommu: Allocate a default domain for iommu groups
On Tue, Jan 27, 2015 at 12:08:56AM +, Joerg Roedel wrote:
> From: Joerg Roedel
>
> The default domain will be used (if supported by the iommu
> driver) when the devices in the iommu group are not attached
> to any other domain.
>
> Signed-off-by: Joerg Roedel
> ---
> drivers/iommu/iommu.c | 26 +++---
> 1 file changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 34636eb..8f33ddd3 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -76,6 +76,9 @@ struct iommu_group_attribute iommu_group_attr_##_name =
> \
> #define to_iommu_group(_kobj)\
> container_of(_kobj, struct iommu_group, kobj)
>
> +static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
> + enum iommu_domain_type type);
> +
> static ssize_t iommu_group_attr_show(struct kobject *kobj,
>struct attribute *__attr, char *buf)
> {
> @@ -362,6 +365,17 @@ rename:
>
> kobject_get(group->devices_kobj);
>
> + /*
> + * Try to allocate a default domain for the group, if this
> + * hasn't happened yet. This is not the best place to do that,
> + * it should happen in iommu_group_alloc(). But we have no
> + * iommu_ops there yet, so the allocation has to happen here for
> + * now.
> + */
> + if (group->default_domain == NULL)
> + group->default_domain = __iommu_domain_alloc(dev->bus,
> + IOMMU_DOMAIN_DMA);
Having a per-group domain is wasteful for IOMMUs that only support a modest
number of concurrent domains, so in reality I think we need to have one
domain per IOMMU instance. Is that possible?
One problem with the current per-bus approach is that __iommu_domain_alloc
can't figure out which IOMMU instance corresponds to the group.
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/5] iommu: Limit iommu_attach/detach_device to devices with their own group
On Tue, Jan 27, 2015 at 12:08:57AM +, Joerg Roedel wrote:
> From: Joerg Roedel
>
> This patch changes the behavior of the iommu_attach_device
> and iommu_detach_device functions. With this change these
> functions only work on devices that have their own group.
> For all other devices the iommu_group_attach/detach
> functions must be used.
I like this a lot. Currently, if somebody detaches a device from the ARM
SMMU, I end up detaching its group as well, which I've always found slightly
odd.
> Signed-off-by: Joerg Roedel
> ---
> drivers/iommu/iommu.c | 63
> +++
> 1 file changed, 59 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 8f33ddd3..b63a550 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -51,6 +51,7 @@ struct iommu_group {
> void (*iommu_data_release)(void *iommu_data);
> char *name;
> int id;
> + unsigned dev_cnt;
Is this actually used on a fast path, or can we just inspect the list of
devices on the group instead?
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 4/5] iommu: Make sure a device is always attached to a domain
On Tue, Jan 27, 2015 at 12:08:58AM +, Joerg Roedel wrote:
> From: Joerg Roedel
>
> Make use of the default domain and re-attach a device to it
> when it is detached from another domain. Also enforce that a
> device has to be in the default domain before it can be
> attached to a different domain.
>
> Signed-off-by: Joerg Roedel
> ---
> drivers/iommu/iommu.c | 34 +++---
> 1 file changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index b63a550..5080759 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -53,6 +53,7 @@ struct iommu_group {
> int id;
> unsigned dev_cnt;
> struct iommu_domain *default_domain;
> + struct iommu_domain *domain;
> };
>
> struct iommu_device {
> @@ -1040,8 +1041,17 @@ static int iommu_group_do_attach_device(struct device
> *dev, void *data)
>
> int iommu_attach_group(struct iommu_domain *domain, struct iommu_group
> *group)
> {
> - return iommu_group_for_each_dev(group, domain,
> - iommu_group_do_attach_device);
> + int ret;
> +
> + if (group->default_domain && group->domain != group->default_domain)
> + return -EBUSY;
I think this is now racy with itself and detach, whereas before we always
held the group->mutex by virtue of iommu_group_for_each_dev.
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu: disable on !MMU builds
A lot of the IOMMU support code does not build if the CPU does not have an MMU itself, and it's not clear if there is any use case for it, so let's just disable it and wait for anybody to need it. This avoids randconfig errors like ../arch/arm/mm/dma-mapping.c: In function '__iommu_alloc_remap': ../arch/arm/mm/dma-mapping.c:1278:34: error: 'VM_ARM_DMA_CONSISTENT' undeclared (first use in this function) area = get_vm_area_caller(size, VM_ARM_DMA_CONSISTENT | VM_USERMAP, ^ ../arch/arm/mm/dma-mapping.c:1278:34: note: each undeclared identifier is reported only once for each function it appears in ../arch/arm/mm/dma-mapping.c: In function '__atomic_get_pages': ../arch/arm/mm/dma-mapping.c:1358:27: error: 'atomic_pool' undeclared (first use in this function) struct dma_pool *pool = &atomic_pool; Signed-off-by: Arnd Bergmann diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig index c68c8db0e6f7..089da97d7c6b 100644 --- a/drivers/iommu/Kconfig +++ b/drivers/iommu/Kconfig @@ -4,6 +4,7 @@ config IOMMU_API menuconfig IOMMU_SUPPORT bool "IOMMU Hardware Support" + depends on MMU default y ---help--- Say Y here if you want to compile device drivers for IO Memory @@ -170,7 +171,7 @@ config IRQ_REMAP # OMAP IOMMU support config OMAP_IOMMU bool "OMAP IOMMU Support" - depends on ARCH_OMAP2PLUS + depends on ARCH_OMAP2PLUS && MMU select IOMMU_API config OMAP_IOMMU_DEBUG @@ -217,7 +218,7 @@ config TEGRA_IOMMU_SMMU config EXYNOS_IOMMU bool "Exynos IOMMU Support" - depends on ARCH_EXYNOS && ARM + depends on ARCH_EXYNOS && ARM && MMU select IOMMU_API select ARM_DMA_USE_IOMMU help @@ -246,7 +247,7 @@ config SHMOBILE_IPMMU_TLB config SHMOBILE_IOMMU bool "IOMMU for Renesas IPMMU/IPMMUI" default n - depends on ARM + depends on ARM && MMU depends on ARCH_SHMOBILE || COMPILE_TEST select IOMMU_API select ARM_DMA_USE_IOMMU @@ -336,6 +337,7 @@ config SPAPR_TCE_IOMMU config ARM_SMMU bool "ARM Ltd. System MMU (SMMU) Support" depends on ARM64 || ARM + depends on MMU select IOMMU_API select IOMMU_IO_PGTABLE_LPAE select ARM_DMA_USE_IOMMU if ARM ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 07/26] iommu/fsl: Fix checkpath type BRACES
WARNING:BRACES: braces {} are not necessary for single statement blocks
+ if (!paace) {
+ return -ENOENT;
+ }
WARNING:BRACES: braces {} are not necessary for single statement blocks
+ if (!paace) {
+ return -ENOENT;
+ }
WARNING:BRACES: braces {} are not necessary for single statement blocks
+ if (!ppaace) {
+ return -ENOENT;
+ }
CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
+{
+
CHECK:BRACES: Blank lines aren't necessary before a close brace '}'
+
+}
CHECK:BRACES: Blank lines aren't necessary before a close brace '}'
+
+}
CHECK:BRACES: Blank lines aren't necessary before a close brace '}'
+
+}
CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
+ for (i = 0; i < num; i++) {
+
CHECK:BRACES: braces {} should be used on all arms of this statement
+ if (pci_ctl->parent->iommu_group) {
[...]
+ } else
[...]
Signed-off-by: Emil Medve
---
drivers/iommu/fsl_pamu.c| 9 +++--
drivers/iommu/fsl_pamu_domain.c | 8 ++--
2 files changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
index ccdc5e5..6b28dd8 100644
--- a/drivers/iommu/fsl_pamu.c
+++ b/drivers/iommu/fsl_pamu.c
@@ -278,9 +278,8 @@ int pamu_update_paace_stash(int liodn, u32 subwin, u32
value)
}
if (subwin) {
paace = pamu_get_spaace(paace, subwin - 1);
- if (!paace) {
+ if (!paace)
return -ENOENT;
- }
}
set_bf(paace->impl_attr, PAACE_IA_CID, value);
@@ -301,9 +300,8 @@ int pamu_disable_spaace(int liodn, u32 subwin)
}
if (subwin) {
paace = pamu_get_spaace(paace, subwin - 1);
- if (!paace) {
+ if (!paace)
return -ENOENT;
- }
set_bf(paace->addr_bitfields, PAACE_AF_V,
PAACE_V_INVALID);
} else {
@@ -351,9 +349,8 @@ int pamu_config_ppaace(int liodn, phys_addr_t win_addr,
phys_addr_t win_size,
}
ppaace = pamu_get_ppaace(liodn);
- if (!ppaace) {
+ if (!ppaace)
return -ENOENT;
- }
/* window size is 2^(WSE+1) bytes */
set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE,
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index dc38db5..080ffa4 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -35,7 +35,6 @@ static DEFINE_SPINLOCK(device_domain_lock);
static int __init iommu_init_mempool(void)
{
-
fsl_pamu_domain_cache = kmem_cache_create("fsl_pamu_domain",
sizeof(struct fsl_dma_domain),
0,
@@ -153,7 +152,6 @@ static int map_liodn(int liodn, struct fsl_dma_domain
*dma_domain)
return map_subwins(liodn, dma_domain);
else
return map_win(liodn, dma_domain);
-
}
/* Update window/subwindow mapping for the LIODN */
@@ -380,7 +378,6 @@ static void attach_device(struct fsl_dma_domain
*dma_domain, int liodn, struct d
if (!dev->archdata.iommu_domain)
dev->archdata.iommu_domain = info;
spin_unlock_irqrestore(&device_domain_lock, flags);
-
}
static phys_addr_t fsl_pamu_iova_to_phys(struct iommu_domain *domain,
@@ -527,7 +524,6 @@ static void fsl_pamu_window_disable(struct iommu_domain
*domain, u32 wnd_nr)
}
spin_unlock_irqrestore(&dma_domain->domain_lock, flags);
-
}
static int fsl_pamu_window_enable(struct iommu_domain *domain, u32 wnd_nr,
@@ -616,7 +612,6 @@ static int handle_attach_device(struct fsl_dma_domain
*dma_domain,
spin_lock_irqsave(&dma_domain->domain_lock, flags);
for (i = 0; i < num; i++) {
-
/* Ensure that LIODN value is valid */
if (liodn[i] >= PAACE_NUMBER_ENTRIES) {
pr_debug("Invalid liodn %d, attach device failed for
%s\n",
@@ -951,8 +946,9 @@ static struct iommu_group *get_pci_device_group(struct
pci_dev *pdev)
if (pci_ctl->parent->iommu_group) {
group = get_device_iommu_group(pci_ctl->parent);
iommu_group_remove_device(pci_ctl->parent);
- } else
+ } else {
group = get_shared_pci_device_group(pdev);
+ }
}
if (!group)
--
2.2.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 13/26] iommu/fsl: Fix checkpatch type PREFER_PACKED
WARNING:PREFER_PACKED: __packed is preferred over __attribute__((packed))
+} __attribute__((packed));
Signed-off-by: Emil Medve
---
drivers/iommu/fsl_pamu.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h
index 86abc70..04dcd25 100644
--- a/drivers/iommu/fsl_pamu.h
+++ b/drivers/iommu/fsl_pamu.h
@@ -333,7 +333,7 @@ struct paace {
#define NUM_MOE 128
struct ome {
u8 moe[NUM_MOE];
-} __attribute__((packed));
+} __packed;
#define PAACT_SIZE (sizeof(struct paace) * PAACE_NUMBER_ENTRIES)
#define SPAACT_SIZE (sizeof(struct paace) * SPAACE_NUMBER_ENTRIES)
--
2.2.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 05/26] iommu/fsl: Fix checkpatch type LINE_SPACING
CHECK:LINE_SPACING: Please don't use multiple blank lines
+
+
CHECK:LINE_SPACING: Please don't use multiple blank lines
+
+
CHECK:LINE_SPACING: Please don't use multiple blank lines
+
+
CHECK:LINE_SPACING: Please don't use multiple blank lines
+
+
WARNING:LINE_SPACING: Missing a blank line after declarations
+ u32 win_cnt = dma_domain->win_cnt > 1 ?
dma_domain->win_cnt : 0;
+ ret = pamu_set_liodn(liodn[i], dev, dma_domain,
CHECK:LINE_SPACING: Please don't use multiple blank lines
+
+
CHECK:LINE_SPACING: Please don't use multiple blank lines
+
+
Signed-off-by: Emil Medve
---
drivers/iommu/fsl_pamu.c| 4
drivers/iommu/fsl_pamu_domain.c | 3 +--
2 files changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
index d3a9fff..df6007c 100644
--- a/drivers/iommu/fsl_pamu.c
+++ b/drivers/iommu/fsl_pamu.c
@@ -56,7 +56,6 @@ static const struct of_device_id guts_device_ids[]
__initconst = {
{}
};
-
/*
* Table for matching compatible strings, for device tree
* L3 cache controller node.
@@ -317,7 +316,6 @@ int pamu_disable_spaace(int liodn, u32 subwin)
return 0;
}
-
/**
* pamu_config_paace() - Sets up PPAACE entry for specified liodn
*
@@ -433,7 +431,6 @@ int pamu_config_spaace(int liodn, u32 subwin_cnt, u32
subwin,
{
struct paace *paace;
-
/* setup sub-windows */
if (!subwin_cnt) {
pr_debug("Invalid subwindow count\n");
@@ -824,7 +821,6 @@ irqreturn_t pamu_av_isr(int irq, void *arg)
}
}
-
return IRQ_HANDLED;
}
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 7ea93ae..dc38db5 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -633,6 +633,7 @@ static int handle_attach_device(struct fsl_dma_domain
*dma_domain,
*/
if (dma_domain->win_arr) {
u32 win_cnt = dma_domain->win_cnt > 1 ?
dma_domain->win_cnt : 0;
+
ret = pamu_set_liodn(liodn[i], dev, dma_domain,
&domain->geometry,
win_cnt);
@@ -822,7 +823,6 @@ static int fsl_pamu_set_domain_attr(struct iommu_domain
*domain,
struct fsl_dma_domain *dma_domain = domain->priv;
int ret = 0;
-
switch (attr_type) {
case DOMAIN_ATTR_GEOMETRY:
ret = configure_domain_geometry(domain, data);
@@ -848,7 +848,6 @@ static int fsl_pamu_get_domain_attr(struct iommu_domain
*domain,
struct fsl_dma_domain *dma_domain = domain->priv;
int ret = 0;
-
switch (attr_type) {
case DOMAIN_ATTR_FSL_PAMU_STASH:
memcpy((struct pamu_stash_attribute *) data,
&dma_domain->dma_stash,
--
2.2.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 12/26] iommu/fsl: Fix checkpatch type TYPO_SPELLING
CHECK:TYPO_SPELLING: 'accomodate' may be misspelled - perhaps 'accommodate'?
+ * Hard coded value for the PAACT size to accomodate
Signed-off-by: Emil Medve
---
drivers/iommu/fsl_pamu.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h
index 52a91af..86abc70 100644
--- a/drivers/iommu/fsl_pamu.h
+++ b/drivers/iommu/fsl_pamu.h
@@ -220,7 +220,7 @@ struct pamu_mmap_regs {
#define PAACE_TCEF_FORMAT0_8B 0x00
#define PAACE_TCEF_FORMAT1_RSVD 0x01
/*
- * Hard coded value for the PAACT size to accomodate
+ * Hard coded value for the PAACT size to accommodate
* maximum LIODN value generated by u-boot.
*/
#define PAACE_NUMBER_ENTRIES0x500
--
2.2.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 00/26] iommu/fsl: Various cleanup
Currently a PAMU driver patch is very likely to receive some checkpatch complaints about the code in the context of the patch. This sequence is an attempt to fix most of that and make the dirver more readable Also fixed a subset of the sparse and coccinelle reported issues Emil Medve (26): iommu/fsl: Sprinkle some __init* annotations iommu/fsl: Use SVR_* instead of magic numbers iommu/fsl: Remove unused/extra includes iommu/fsl: Fix checkpatch type SPACE_BEFORE_TAB iommu/fsl: Fix checkpatch type LINE_SPACING iommu/fsl: Fix checkpatch type LEADING_SPACE iommu/fsl: Fix checkpath type BRACES iommu/fsl: Fix checkpatch type CODE_INDENT iommu/fsl: Fix checkpatch type ALLOC_SIZEOF_STRUCT iommu/fsl: Fix checkpatch type ALLOC_WITH_MULTIPLY iommu/fsl: Fix checkpatch type OOM_MESSAGE iommu/fsl: Fix checkpatch type TYPO_SPELLING iommu/fsl: Fix checkpatch type PREFER_PACKED iommu/fsl: Fix checkpatch type QUOTED_WHITESPACE_BEFORE_NEWLINE iommu/fsl: Fix checkpatch type SPACING iommu/fsl: Use a device pointer to make lines shorter iommu/fsl: Remove pr/dev_*() prefixes iommu/fsl: Fix checkpatch type PARENTHESIS_ALIGNMENT iommu/fsl: Fix some comments alignment iommu/fsl: Fix alignment of some stray lines iommu/fsl: Fix checkpatch type LONG_LINE iommu/fsl: Make local symbols static iommu/fsl: Use NULL instead of zero iommu/fsl: Remove unneeded semicolon iommu/fsl: Don't use integers values with bool type iommu/fsl: Remove extra paranthesis arch/powerpc/include/asm/fsl_pamu_stash.h | 4 +- drivers/iommu/fsl_pamu.c | 204 ++ drivers/iommu/fsl_pamu.h | 15 ++- drivers/iommu/fsl_pamu_domain.c | 173 +++-- 4 files changed, 179 insertions(+), 217 deletions(-) -- 2.2.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 16/26] iommu/fsl: Use a device pointer to make lines shorter
Signed-off-by: Emil Medve
---
drivers/iommu/fsl_pamu.c | 39 ++-
1 file changed, 18 insertions(+), 21 deletions(-)
diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
index 18a604a..22b72e2 100644
--- a/drivers/iommu/fsl_pamu.c
+++ b/drivers/iommu/fsl_pamu.c
@@ -1002,6 +1002,7 @@ static const struct {
static int __init fsl_pamu_probe(struct platform_device *pdev)
{
+ struct device *dev = &pdev->dev;
void __iomem *pamu_regs = NULL;
struct ccsr_guts __iomem *guts_regs = NULL;
u32 pamubypenr, pamu_counter;
@@ -1026,16 +1027,16 @@ static int __init fsl_pamu_probe(struct platform_device
*pdev)
* NOTE : All PAMUs share the same LIODN tables.
*/
- pamu_regs = of_iomap(pdev->dev.of_node, 0);
+ pamu_regs = of_iomap(dev->of_node, 0);
if (!pamu_regs) {
- dev_err(&pdev->dev, "ioremap of PAMU node failed\n");
+ dev_err(dev, "ioremap of PAMU node failed\n");
return -ENOMEM;
}
- of_get_address(pdev->dev.of_node, 0, &size, NULL);
+ of_get_address(dev->of_node, 0, &size, NULL);
- irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+ irq = irq_of_parse_and_map(dev->of_node, 0);
if (irq == NO_IRQ) {
- dev_warn(&pdev->dev, "no interrupts listed in PAMU node\n");
+ dev_warn(dev, "no interrupts listed in PAMU node\n");
goto error;
}
@@ -1050,15 +1051,14 @@ static int __init fsl_pamu_probe(struct platform_device
*pdev)
/* The ISR needs access to the regs, so we won't iounmap them */
ret = request_irq(irq, pamu_av_isr, 0, "pamu", data);
if (ret < 0) {
- dev_err(&pdev->dev, "error %i installing ISR for irq %i\n",
- ret, irq);
+ dev_err(dev, "error %i installing ISR for irq %i\n", ret, irq);
goto error;
}
guts_node = of_find_matching_node(NULL, guts_device_ids);
if (!guts_node) {
- dev_err(&pdev->dev, "could not find GUTS node %s\n",
- pdev->dev.of_node->full_name);
+ dev_err(dev, "could not find GUTS node %s\n",
+ dev->of_node->full_name);
ret = -ENODEV;
goto error;
}
@@ -1066,7 +1066,7 @@ static int __init fsl_pamu_probe(struct platform_device
*pdev)
guts_regs = of_iomap(guts_node, 0);
of_node_put(guts_node);
if (!guts_regs) {
- dev_err(&pdev->dev, "ioremap of GUTS node failed\n");
+ dev_err(dev, "ioremap of GUTS node failed\n");
ret = -ENODEV;
goto error;
}
@@ -1086,7 +1086,7 @@ static int __init fsl_pamu_probe(struct platform_device
*pdev)
p = alloc_pages(GFP_KERNEL | __GFP_ZERO, order);
if (!p) {
- dev_err(&pdev->dev, "unable to allocate PAACT/SPAACT/OMT
block\n");
+ dev_err(dev, "unable to allocate PAACT/SPAACT/OMT block\n");
ret = -ENOMEM;
goto error;
}
@@ -1096,7 +1096,7 @@ static int __init fsl_pamu_probe(struct platform_device
*pdev)
/* Make sure the memory is naturally aligned */
if (ppaact_phys & ((PAGE_SIZE << order) - 1)) {
- dev_err(&pdev->dev, "PAACT/OMT block is unaligned\n");
+ dev_err(dev, "PAACT/OMT block is unaligned\n");
ret = -ENOMEM;
goto error;
}
@@ -1104,8 +1104,7 @@ static int __init fsl_pamu_probe(struct platform_device
*pdev)
spaact = (void *)ppaact + (PAGE_SIZE << get_order(PAACT_SIZE));
omt = (void *)spaact + (PAGE_SIZE << get_order(SPAACT_SIZE));
- dev_dbg(&pdev->dev, "ppaact virt=%p phys=0x%pa\n", ppaact,
- &ppaact_phys);
+ dev_dbg(dev, "ppaact virt=%p phys=0x%pa\n", ppaact, &ppaact_phys);
/* Check to see if we need to implement the work-around on this SOC */
@@ -1113,21 +1112,19 @@ static int __init fsl_pamu_probe(struct platform_device
*pdev)
for (i = 0; i < ARRAY_SIZE(port_id_map); i++) {
if (port_id_map[i].svr == (mfspr(SPRN_SVR) & ~SVR_SECURITY)) {
csd_port_id = port_id_map[i].port_id;
- dev_dbg(&pdev->dev, "found matching SVR %08x\n",
+ dev_dbg(dev, "found matching SVR %08x\n",
port_id_map[i].svr);
break;
}
}
if (csd_port_id) {
- dev_dbg(&pdev->dev, "creating coherency subdomain at address "
- "%pa, size %zu, port id 0x%08x", &ppaact_phys,
- mem_size, csd_port_id);
+ dev_dbg(dev, "creating coherency subdomain at address %pa, size
%zu, port id 0x%08x",
+ &ppaact_phys, mem_size, csd_port_id);
ret = cre
[PATCH 17/26] iommu/fsl: Remove pr/dev_*() prefixes
pr_fmt() and dev_*() already take care of it
Signed-off-by: Emil Medve
---
drivers/iommu/fsl_pamu.c| 2 +-
drivers/iommu/fsl_pamu_domain.c | 12 ++--
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
index 22b72e2..e46c75f 100644
--- a/drivers/iommu/fsl_pamu.c
+++ b/drivers/iommu/fsl_pamu.c
@@ -1135,7 +1135,7 @@ static int __init fsl_pamu_probe(struct platform_device
*pdev)
spaace_pool = gen_pool_create(ilog2(sizeof(struct paace)), -1);
if (!spaace_pool) {
ret = -ENOMEM;
- dev_err(dev, "PAMU : failed to allocate spaace gen pool\n");
+ dev_err(dev, "Failed to allocate spaace gen pool\n");
goto error;
}
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index c7af760..3e7954a 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -113,7 +113,7 @@ static int map_subwins(int liodn, struct fsl_dma_domain
*dma_domain)
sub_win_ptr[i].prot);
spin_unlock_irqrestore(&iommu_lock, flags);
if (ret) {
- pr_debug("PAMU SPAACE configuration failed for
liodn %d\n",
+ pr_debug("SPAACE configuration failed for liodn
%d\n",
liodn);
return ret;
}
@@ -139,7 +139,7 @@ static int map_win(int liodn, struct fsl_dma_domain
*dma_domain)
0, wnd->prot);
spin_unlock_irqrestore(&iommu_lock, flags);
if (ret)
- pr_debug("PAMU PAACE configuration failed for liodn %d\n",
+ pr_debug("PAACE configuration failed for liodn %d\n",
liodn);
return ret;
@@ -250,7 +250,7 @@ static int pamu_set_liodn(int liodn, struct device *dev,
dma_domain->stash_id, win_cnt, 0);
spin_unlock_irqrestore(&iommu_lock, flags);
if (ret) {
- pr_debug("PAMU PAACE configuration failed for liodn %d, win_cnt
=%d\n", liodn, win_cnt);
+ pr_debug("PAACE configuration failed for liodn %d, win_cnt
=%d\n", liodn, win_cnt);
return ret;
}
@@ -267,7 +267,7 @@ static int pamu_set_liodn(int liodn, struct device *dev,
0, 0);
spin_unlock_irqrestore(&iommu_lock, flags);
if (ret) {
- pr_debug("PAMU SPAACE configuration failed for
liodn %d\n", liodn);
+ pr_debug("SPAACE configuration failed for liodn
%d\n", liodn);
return ret;
}
}
@@ -283,13 +283,13 @@ static int check_size(u64 size, dma_addr_t iova)
* to PAMU page size.
*/
if ((size & (size - 1)) || size < PAMU_PAGE_SIZE) {
- pr_debug("%s: size too small or not a power of two\n",
__func__);
+ pr_debug("Size too small or not a power of two\n");
return -EINVAL;
}
/* iova must be page size aligned*/
if (iova & (size - 1)) {
- pr_debug("%s: address is not aligned with window size\n",
__func__);
+ pr_debug("Address is not aligned with window size\n");
return -EINVAL;
}
--
2.2.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 18/26] iommu/fsl: Fix checkpatch type PARENTHESIS_ALIGNMENT
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+ set_bf(paace->addr_bitfields, PAACE_AF_V,
+PAACE_V_INVALID);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+ set_bf(paace->addr_bitfields, PAACE_AF_AP,
+PAACE_AP_PERMS_DENIED);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+ set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE,
+ map_addrspace_size_to_wse(win_size));
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+ pr_debug("can't find next-level-cache at %s\n",
+ node->full_name);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+ out_be32((u32 *)(pamu_reg_base + PAMU_PICS),
+ PAMU_ACCESS_VIOLATION_ENABLE);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+ set_bf(ppaace->impl_attr, PAACE_IA_ATM,
+ PAACE_ATM_NO_XLATE);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+ set_bf(ppaace->addr_bitfields, PAACE_AF_AP,
+ PAACE_AP_PERMS_ALL);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+ pr_emerg("AVA=%016llx\n", make64(in_be32(p + PAMU_AVAH),
+ in_be32(p + PAMU_AVAL)));
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+ pr_emerg("POEA=%016llx\n", make64(in_be32(p +
PAMU_POEAH),
+ in_be32(p + PAMU_POEAL)));
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+ phys = make64(in_be32(p + PAMU_POEAH),
+ in_be32(p + PAMU_POEAL));
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+ setup_one_pamu(pamu_reg_base, pamu_reg_off, ppaact_phys,
+spaact_phys, omt_phys);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+ fsl_pamu_domain_cache = kmem_cache_create("fsl_pamu_domain",
+sizeof(struct fsl_dma_domain),
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+ iommu_devinfo_cache = kmem_cache_create("iommu_devinfo",
+sizeof(struct device_domain_info),
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+ pr_debug("PAACE configuration failed for liodn %d\n",
+ liodn);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+static int update_liodn_stash(int liodn, struct fsl_dma_domain *dma_domain,
+u32 val)
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+static int pamu_set_liodn(int liodn, struct device *dev,
+ struct fsl_dma_domain *dma_domain,
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+static phys_addr_t fsl_pamu_iova_to_phys(struct iommu_domain *domain,
+ dma_addr_t iova)
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+ if ((iova < domain->geometry.aperture_start) ||
+ iova > (domain->geometry.aperture_end))
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+ ret = pamu_set_liodn(info->liodn, info->dev, dma_domain,
+ geom_attr, win_cnt);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+static int handle_attach_device(struct fsl_dma_domain *dma_domain,
+struct device *dev, const u32 *liodn,
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+ pr_debug("Invalid liodn %d, attach device failed for
%s\n",
+ liodn[i], dev->of_node->full_name);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+ ret = pamu_set_liodn(liodn[i], dev, dma_domain,
+ &domain->geometry,
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+ ret = handle_attach_device(dma_domain, dev,
+liodn, liodn_cnt);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+static void fsl_pamu_detach_device(struct iommu_domain *domain,
+ struct device *dev)
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+ if (check_size(geom_size, geom_attr->aperture_start) ||
+ !geom_attr->force_aperture) {
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+ memcpy(&dma_domain->dma_stash, stash_attr,
+sizeof(struct
[PATCH 03/26] iommu/fsl: Remove unused/extra includes
linux/modules.h - drivers can't be a module linux/bootmem.h - no boot time memory allocator is used linux/notifier.h- not used etc. Factorize common includes into fsl_pamu.h Signed-off-by: Emil Medve --- drivers/iommu/fsl_pamu.c| 16 +++- drivers/iommu/fsl_pamu.h| 2 ++ drivers/iommu/fsl_pamu_domain.c | 20 ++-- 3 files changed, 7 insertions(+), 31 deletions(-) diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index 7d9f8a0..d3a9fff 100644 --- a/drivers/iommu/fsl_pamu.c +++ b/drivers/iommu/fsl_pamu.c @@ -18,24 +18,14 @@ #define pr_fmt(fmt)"fsl-pamu: %s: " fmt, __func__ -#include -#include -#include -#include -#include -#include +#include "fsl_pamu.h" + #include -#include -#include -#include #include -#include -#include + #include #include -#include "fsl_pamu.h" - /* define indexes for each operation mapping scenario */ #define OMI_QMAN0x00 #define OMI_FMAN0x01 diff --git a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h index 8fc1a12..a7794b5 100644 --- a/drivers/iommu/fsl_pamu.h +++ b/drivers/iommu/fsl_pamu.h @@ -19,6 +19,8 @@ #ifndef __FSL_PAMU_H #define __FSL_PAMU_H +#include + #include /* Bit Field macros diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index 48a72b3..7ea93ae 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -19,26 +19,10 @@ #define pr_fmt(fmt)"fsl-pamu-domain: %s: " fmt, __func__ -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include -#include - #include "fsl_pamu_domain.h" +#include + /* * Global spinlock that needs to be held while * configuring PAMU. -- 2.2.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 15/26] iommu/fsl: Fix checkpatch type SPACING
CHECK:SPACING: No space is necessary after a cast
+ pamu_reg_base = (unsigned long) pamu_regs + pamu_reg_off;
CHECK:SPACING: No space is necessary after a cast
+ memcpy((struct pamu_stash_attribute *) data,
&dma_domain->dma_stash,
Signed-off-by: Emil Medve
---
drivers/iommu/fsl_pamu.c| 2 +-
drivers/iommu/fsl_pamu_domain.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
index 6f9c976..18a604a 100644
--- a/drivers/iommu/fsl_pamu.c
+++ b/drivers/iommu/fsl_pamu.c
@@ -1151,7 +1151,7 @@ static int __init fsl_pamu_probe(struct platform_device
*pdev)
for (pamu_reg_off = 0, pamu_counter = 0x8000; pamu_reg_off < size;
pamu_reg_off += PAMU_OFFSET, pamu_counter >>= 1) {
- pamu_reg_base = (unsigned long) pamu_regs + pamu_reg_off;
+ pamu_reg_base = (unsigned long)pamu_regs + pamu_reg_off;
setup_one_pamu(pamu_reg_base, pamu_reg_off, ppaact_phys,
spaact_phys, omt_phys);
/* Disable PAMU bypass for this PAMU */
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index c9886ff..c7af760 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -845,8 +845,8 @@ static int fsl_pamu_get_domain_attr(struct iommu_domain
*domain,
switch (attr_type) {
case DOMAIN_ATTR_FSL_PAMU_STASH:
- memcpy((struct pamu_stash_attribute *) data,
&dma_domain->dma_stash,
-sizeof(struct pamu_stash_attribute));
+ memcpy(data, &dma_domain->dma_stash,
+ sizeof(struct pamu_stash_attribute));
break;
case DOMAIN_ATTR_FSL_PAMU_ENABLE:
*(int *)data = dma_domain->enabled;
--
2.2.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 04/26] iommu/fsl: Fix checkpatch type SPACE_BEFORE_TAB
WARNING:SPACE_BEFORE_TAB: please, no space before tabs
+^Iu32 ^Icpu;^I/* cpu number */$
WARNING:SPACE_BEFORE_TAB: please, no space before tabs
+^Iu32 ^Icache;^I/* cache to stash to: L1,L2,L3 */$
Signed-off-by: Emil Medve
---
arch/powerpc/include/asm/fsl_pamu_stash.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/fsl_pamu_stash.h
b/arch/powerpc/include/asm/fsl_pamu_stash.h
index caa1b21..38311c9 100644
--- a/arch/powerpc/include/asm/fsl_pamu_stash.h
+++ b/arch/powerpc/include/asm/fsl_pamu_stash.h
@@ -32,8 +32,8 @@ enum pamu_stash_target {
*/
struct pamu_stash_attribute {
- u32 cpu;/* cpu number */
- u32 cache; /* cache to stash to: L1,L2,L3 */
+ u32 cpu;/* cpu number */
+ u32 cache; /* cache to stash to: L1,L2,L3 */
};
#endif /* __FSL_PAMU_STASH_H */
--
2.2.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 11/26] iommu/fsl: Fix checkpatch type OOM_MESSAGE
WARNING:OOM_MESSAGE: Possible unnecessary 'out of memory' message
+ if (!data) {
+ dev_err(&pdev->dev, "PAMU isr data memory allocation failed\n");
Signed-off-by: Emil Medve
---
drivers/iommu/fsl_pamu.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
index 4f1926b..6f9c976 100644
--- a/drivers/iommu/fsl_pamu.c
+++ b/drivers/iommu/fsl_pamu.c
@@ -1041,7 +1041,6 @@ static int __init fsl_pamu_probe(struct platform_device
*pdev)
data = kzalloc(sizeof(*data), GFP_KERNEL);
if (!data) {
- dev_err(&pdev->dev, "PAMU isr data memory allocation failed\n");
ret = -ENOMEM;
goto error;
}
--
2.2.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 25/26] iommu/fsl: Don't use integers values with bool type
drivers/iommu/fsl_pamu_domain.c:884:9-10: WARNING: return of 0/1 in function 'check_pci_ctl_endpt_part' with return type bool Signed-off-by: Emil Medve --- drivers/iommu/fsl_pamu_domain.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index ebbc396..36622e2 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -880,10 +880,7 @@ static bool check_pci_ctl_endpt_part(struct pci_controller *pci_ctl) version = in_be32(pci_ctl->cfg_addr + (PCI_FSL_BRR1 >> 2)); version &= PCI_FSL_BRR1_VER; /* If PCI controller version is >= 0x204 we can partition endpoints */ - if (version >= 0x204) - return 1; - - return 0; + return version >= 0x204; } /* Get iommu group information from peer devices or devices on the parent bus */ -- 2.2.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 01/26] iommu/fsl: Sprinkle some __init* annotations
Signed-off-by: Emil Medve
---
drivers/iommu/fsl_pamu.c| 12 ++--
drivers/iommu/fsl_pamu_domain.c | 2 +-
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
index d958e65..652c34d 100644
--- a/drivers/iommu/fsl_pamu.c
+++ b/drivers/iommu/fsl_pamu.c
@@ -50,7 +50,7 @@ struct pamu_isr_data {
static struct paace *ppaact;
static struct paace *spaact;
-static struct ome *omt;
+static struct ome *omt __initdata;
/*
* Table for matching compatible strings, for device tree
@@ -59,7 +59,7 @@ static struct ome *omt;
* SOCs. For the older SOCs "fsl,qoriq-device-config-1.0"
* string would be used.
*/
-static const struct of_device_id guts_device_ids[] = {
+static const struct of_device_id guts_device_ids[] __initconst = {
{ .compatible = "fsl,qoriq-device-config-1.0", },
{ .compatible = "fsl,qoriq-device-config-2.0", },
{}
@@ -612,7 +612,7 @@ found_cpu_node:
* Memory accesses to QMAN and BMAN private memory need not be coherent, so
* clear the PAACE entry coherency attribute for them.
*/
-static void setup_qbman_paace(struct paace *ppaace, int paace_type)
+static void __init setup_qbman_paace(struct paace *ppaace, int paace_type)
{
switch (paace_type) {
case QMAN_PAACE:
@@ -679,7 +679,7 @@ static void __init setup_omt(struct ome *omt)
* Get the maximum number of PAACT table entries
* and subwindows supported by PAMU
*/
-static void get_pamu_cap_values(unsigned long pamu_reg_base)
+static void __init get_pamu_cap_values(unsigned long pamu_reg_base)
{
u32 pc_val;
@@ -689,7 +689,7 @@ static void get_pamu_cap_values(unsigned long pamu_reg_base)
}
/* Setup PAMU registers pointing to PAACT, SPAACT and OMT */
-int setup_one_pamu(unsigned long pamu_reg_base, unsigned long pamu_reg_size,
+static int __init setup_one_pamu(unsigned long pamu_reg_base, unsigned long
pamu_reg_size,
phys_addr_t ppaact_phys, phys_addr_t spaact_phys,
phys_addr_t omt_phys)
{
@@ -998,7 +998,7 @@ error:
static const struct {
u32 svr;
u32 port_id;
-} port_id_map[] = {
+} port_id_map[] __initconst = {
{0x82100010, 0xFF00}, /* P2040 1.0 */
{0x82100011, 0xFF00}, /* P2040 1.1 */
{0x82100110, 0xFF00}, /* P2041 1.0 */
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index c828f80..48a72b3 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -1095,7 +1095,7 @@ static const struct iommu_ops fsl_pamu_ops = {
.remove_device = fsl_pamu_remove_device,
};
-int pamu_domain_init(void)
+int __init pamu_domain_init(void)
{
int ret = 0;
--
2.2.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 21/26] iommu/fsl: Fix checkpatch type LONG_LINE
Only pr_*() here
WARNING:LONG_LINE: line over 80 characters
+ pr_debug("window size too small or not a power of two %pa\n",
&win_size);
WARNING:LONG_LINE: line over 80 characters
+ pr_debug("missing cache-stash-id at %s\n",
node->full_name);
ARNING:LONG_LINE: line over 80 characters
+ pr_debug("missing cache-stash-id at %s\n",
node->full_name);
WARNING:LONG_LINE: line over 80 characters
+ pr_emerg("PAACE[%u]=%08x\n", j,
in_be32(paace + j));
WARNING:LONG_LINE: line over 80 characters
+ pr_emerg("Disabling liodn %x\n", avs1 >>
PAMU_AVS1_LIODN_SHIFT);
WARNING:LONG_LINE: line over 80 characters
+ pr_debug("Subwindow reconfiguration failed for liodn
%d\n", liodn);
WARNING:LONG_LINE: line over 80 characters
+ pr_debug("Window reconfiguration failed for liodn
%d\n", liodn);
WARNING:LONG_LINE: line over 80 characters
+ pr_debug("Windows not configured, stash destination update
failed for liodn %d\n", liodn);
WARNING:LONG_LINE: line over 80 characters
+ pr_debug("Failed to update SPAACE %d field for liodn
%d\n ", i, liodn);
WARNING:LONG_LINE: line over 80 characters
+ pr_debug("PAACE configuration failed for liodn %d, win_cnt
=%d\n", liodn, win_cnt);
WARNING:LONG_LINE: line over 80 characters
+ pr_debug("SPAACE configuration failed for liodn
%d\n", liodn);
Signed-off-by: Emil Medve
---
drivers/iommu/fsl_pamu.c| 15 ++-
drivers/iommu/fsl_pamu_domain.c | 18 --
2 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
index ba5d1e0..3f83259 100644
--- a/drivers/iommu/fsl_pamu.c
+++ b/drivers/iommu/fsl_pamu.c
@@ -338,7 +338,8 @@ int pamu_config_ppaace(int liodn, phys_addr_t win_addr,
phys_addr_t win_size,
unsigned long fspi;
if ((win_size & (win_size - 1)) || win_size < PAMU_PAGE_SIZE) {
- pr_debug("window size too small or not a power of two %pa\n",
&win_size);
+ pr_debug("window size too small or not a power of two %pa\n",
+&win_size);
return -EINVAL;
}
@@ -530,7 +531,8 @@ u32 get_stash_id(u32 stash_dest_hint, u32 vcpu)
if (node) {
prop = of_get_property(node, "cache-stash-id", 0);
if (!prop) {
- pr_debug("missing cache-stash-id at %s\n",
node->full_name);
+ pr_debug("missing cache-stash-id at %s\n",
+node->full_name);
of_node_put(node);
return ~(u32)0;
}
@@ -556,7 +558,8 @@ found_cpu_node:
if (stash_dest_hint == cache_level) {
prop = of_get_property(node, "cache-stash-id", 0);
if (!prop) {
- pr_debug("missing cache-stash-id at %s\n",
node->full_name);
+ pr_debug("missing cache-stash-id at %s\n",
+node->full_name);
of_node_put(node);
return ~(u32)0;
}
@@ -793,7 +796,8 @@ irqreturn_t pamu_av_isr(int irq, void *arg)
/* Only the first four words are relevant */
for (j = 0; j < 4; j++)
- pr_emerg("PAACE[%u]=%08x\n", j,
in_be32(paace + j));
+ pr_emerg("PAACE[%u]=%08x\n",
+j, in_be32(paace + j));
}
/* clear access violation condition */
@@ -813,7 +817,8 @@ irqreturn_t pamu_av_isr(int irq, void *arg)
/* Disable the LIODN */
ret = pamu_disable_liodn(avs1 >>
PAMU_AVS1_LIODN_SHIFT);
BUG_ON(ret);
- pr_emerg("Disabling liodn %x\n", avs1 >>
PAMU_AVS1_LIODN_SHIFT);
+ pr_emerg("Disabling liodn %x\n",
+avs1 >> PAMU_AVS1_LIODN_SHIFT);
}
out_be32((p + PAMU_PICS), pics);
}
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index ae21305..38c26be 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -167,7 +167,8 @@ static int update_liodn(int liodn, struct fsl_dma_domain
*dma_domain, u32 wnd_nr
(wnd_nr > 0) ? 1 : 0,
wnd->prot);
[PATCH 08/26] iommu/fsl: Fix checkpatch type CODE_INDENT
ERROR:CODE_INDENT: code indent should use tabs where possible
+^I stash_dest_hint, vcpu);$
ERROR:CODE_INDENT: code indent should use tabs where possible
+^I phys_addr_t ppaact_phys, phys_addr_t spaact_phys,$
ERROR:CODE_INDENT: code indent should use tabs where possible
+^I^I dev->of_node->full_name);$
ERROR:CODE_INDENT: code indent should use tabs where possible
+^I^I dev->of_node->full_name);$
Signed-off-by: Emil Medve
---
drivers/iommu/fsl_pamu.c| 6 +++---
drivers/iommu/fsl_pamu_domain.c | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
index 6b28dd8..7f0181c 100644
--- a/drivers/iommu/fsl_pamu.c
+++ b/drivers/iommu/fsl_pamu.c
@@ -583,7 +583,7 @@ found_cpu_node:
}
pr_debug("stash dest not found for %d on vcpu %d\n",
- stash_dest_hint, vcpu);
+stash_dest_hint, vcpu);
return ~(u32)0;
}
@@ -675,8 +675,8 @@ static void __init get_pamu_cap_values(unsigned long
pamu_reg_base)
/* Setup PAMU registers pointing to PAACT, SPAACT and OMT */
static int __init setup_one_pamu(unsigned long pamu_reg_base, unsigned long
pamu_reg_size,
- phys_addr_t ppaact_phys, phys_addr_t spaact_phys,
- phys_addr_t omt_phys)
+phys_addr_t ppaact_phys, phys_addr_t
spaact_phys,
+phys_addr_t omt_phys)
{
u32 *pc;
struct pamu_mmap_regs *pamu_regs;
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 080ffa4..d5137dd 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -682,7 +682,7 @@ static int fsl_pamu_attach_device(struct iommu_domain
*domain,
liodn, liodn_cnt);
} else {
pr_debug("missing fsl,liodn property at %s\n",
- dev->of_node->full_name);
+dev->of_node->full_name);
ret = -EINVAL;
}
@@ -718,7 +718,7 @@ static void fsl_pamu_detach_device(struct iommu_domain
*domain,
detach_device(dev, dma_domain);
else
pr_debug("missing fsl,liodn property at %s\n",
- dev->of_node->full_name);
+dev->of_node->full_name);
}
static int configure_domain_geometry(struct iommu_domain *domain, void *data)
--
2.2.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 14/26] iommu/fsl: Fix checkpatch type QUOTED_WHITESPACE_BEFORE_NEWLINE
WARNING:QUOTED_WHITESPACE_BEFORE_NEWLINE: unnecessary whitespace before a
quoted newline
+ pr_debug("Invalid window size \n");
Signed-off-by: Emil Medve
---
drivers/iommu/fsl_pamu_domain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index dcb9bb4..c9886ff 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -556,7 +556,7 @@ static int fsl_pamu_window_enable(struct iommu_domain
*domain, u32 wnd_nr,
win_size = dma_domain->geom_size >> ilog2(dma_domain->win_cnt);
if (size > win_size) {
- pr_debug("Invalid window size \n");
+ pr_debug("Invalid window size\n");
spin_unlock_irqrestore(&dma_domain->domain_lock, flags);
return -EINVAL;
}
--
2.2.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 06/26] iommu/fsl: Fix checkpatch type LEADING_SPACE
WARNING:LEADING_SPACE: please, no spaces at the start of a line
+ return __ffs(subwindow_cnt) - 1;$
WARNING:LEADING_SPACE: please, no spaces at the start of a line
+(PAACE_ATM_WINDOW_XLATE | PAACE_ATM_PAGE_XLATE)$
Signed-off-by: Emil Medve
---
drivers/iommu/fsl_pamu.c | 4 ++--
drivers/iommu/fsl_pamu.h | 3 +--
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
index df6007c..ccdc5e5 100644
--- a/drivers/iommu/fsl_pamu.c
+++ b/drivers/iommu/fsl_pamu.c
@@ -169,8 +169,8 @@ static unsigned int map_addrspace_size_to_wse(phys_addr_t
addrspace_size)
/* Derive the PAACE window count encoding for the subwindow count */
static unsigned int map_subwindow_cnt_to_wce(u32 subwindow_cnt)
{
- /* window count is 2^(WCE+1) bytes */
- return __ffs(subwindow_cnt) - 1;
+ /* window count is 2^(WCE+1) bytes */
+ return __ffs(subwindow_cnt) - 1;
}
/*
diff --git a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h
index a7794b5..52a91af 100644
--- a/drivers/iommu/fsl_pamu.h
+++ b/drivers/iommu/fsl_pamu.h
@@ -200,8 +200,7 @@ struct pamu_mmap_regs {
#define PAACE_ATM_NO_XLATE 0x00
#define PAACE_ATM_WINDOW_XLATE 0x01
#define PAACE_ATM_PAGE_XLATE0x02
-#define PAACE_ATM_WIN_PG_XLATE \
-(PAACE_ATM_WINDOW_XLATE | PAACE_ATM_PAGE_XLATE)
+#define PAACE_ATM_WIN_PG_XLATE (PAACE_ATM_WINDOW_XLATE | PAACE_ATM_PAGE_XLATE)
#define PAACE_OTM_NO_XLATE 0x00
#define PAACE_OTM_IMMEDIATE 0x01
#define PAACE_OTM_INDEXED 0x02
--
2.2.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 02/26] iommu/fsl: Use SVR_* instead of magic numbers
Signed-off-by: Emil Medve
---
drivers/iommu/fsl_pamu.c | 27 ++-
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
index 652c34d..7d9f8a0 100644
--- a/drivers/iommu/fsl_pamu.c
+++ b/drivers/iommu/fsl_pamu.c
@@ -31,6 +31,7 @@
#include
#include
#include
+#include
#include
#include "fsl_pamu.h"
@@ -999,19 +1000,19 @@ static const struct {
u32 svr;
u32 port_id;
} port_id_map[] __initconst = {
- {0x82100010, 0xFF00}, /* P2040 1.0 */
- {0x82100011, 0xFF00}, /* P2040 1.1 */
- {0x82100110, 0xFF00}, /* P2041 1.0 */
- {0x82100111, 0xFF00}, /* P2041 1.1 */
- {0x82110310, 0xFF00}, /* P3041 1.0 */
- {0x82110311, 0xFF00}, /* P3041 1.1 */
- {0x82010020, 0xFFF8}, /* P4040 2.0 */
- {0x8220, 0xFFF8}, /* P4080 2.0 */
- {0x82210010, 0xFC00}, /* P5010 1.0 */
- {0x82210020, 0xFC00}, /* P5010 2.0 */
- {0x82200010, 0xFC00}, /* P5020 1.0 */
- {0x82050010, 0xFF80}, /* P5021 1.0 */
- {0x82040010, 0xFF80}, /* P5040 1.0 */
+ {(SVR_P2040 << 8) | 0x10, 0xFF00}, /* P2040 1.0 */
+ {(SVR_P2040 << 8) | 0x11, 0xFF00}, /* P2040 1.1 */
+ {(SVR_P2041 << 8) | 0x10, 0xFF00}, /* P2041 1.0 */
+ {(SVR_P2041 << 8) | 0x11, 0xFF00}, /* P2041 1.1 */
+ {(SVR_P3041 << 8) | 0x10, 0xFF00}, /* P3041 1.0 */
+ {(SVR_P3041 << 8) | 0x11, 0xFF00}, /* P3041 1.1 */
+ {(SVR_P4040 << 8) | 0x20, 0xFFF8}, /* P4040 2.0 */
+ {(SVR_P4080 << 8) | 0x20, 0xFFF8}, /* P4080 2.0 */
+ {(SVR_P5010 << 8) | 0x10, 0xFC00}, /* P5010 1.0 */
+ {(SVR_P5010 << 8) | 0x20, 0xFC00}, /* P5010 2.0 */
+ {(SVR_P5020 << 8) | 0x10, 0xFC00}, /* P5020 1.0 */
+ {(SVR_P5021 << 8) | 0x10, 0xFF80}, /* P5021 1.0 */
+ {(SVR_P5040 << 8) | 0x10, 0xFF80}, /* P5040 1.0 */
};
#define SVR_SECURITY 0x8 /* The Security (E) bit */
--
2.2.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 23/26] iommu/fsl: Use NULL instead of zero
drivers/iommu/fsl_pamu.c:532:72: warning: Using plain integer as NULL pointer
drivers/iommu/fsl_pamu.c:559:72: warning: Using plain integer as NULL pointer
drivers/iommu/fsl_pamu.c:570:66: warning: Using plain integer as NULL pointer
Signed-off-by: Emil Medve
---
drivers/iommu/fsl_pamu.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
index 1d9273a..7a4665d 100644
--- a/drivers/iommu/fsl_pamu.c
+++ b/drivers/iommu/fsl_pamu.c
@@ -529,7 +529,7 @@ u32 get_stash_id(u32 stash_dest_hint, u32 vcpu)
if (stash_dest_hint == PAMU_ATTR_CACHE_L3) {
node = of_find_matching_node(NULL, l3_device_ids);
if (node) {
- prop = of_get_property(node, "cache-stash-id", 0);
+ prop = of_get_property(node, "cache-stash-id", NULL);
if (!prop) {
pr_debug("missing cache-stash-id at %s\n",
node->full_name);
@@ -556,7 +556,7 @@ found_cpu_node:
/* find the hwnode that represents the cache */
for (cache_level = PAMU_ATTR_CACHE_L1; (cache_level <
PAMU_ATTR_CACHE_L3) && found; cache_level++) {
if (stash_dest_hint == cache_level) {
- prop = of_get_property(node, "cache-stash-id", 0);
+ prop = of_get_property(node, "cache-stash-id", NULL);
if (!prop) {
pr_debug("missing cache-stash-id at %s\n",
node->full_name);
@@ -567,7 +567,7 @@ found_cpu_node:
return be32_to_cpup(prop);
}
- prop = of_get_property(node, "next-level-cache", 0);
+ prop = of_get_property(node, "next-level-cache", NULL);
if (!prop) {
pr_debug("can't find next-level-cache at %s\n",
node->full_name);
--
2.2.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 20/26] iommu/fsl: Fix alignment of some stray lines
Signed-off-by: Emil Medve
---
drivers/iommu/fsl_pamu.h| 2 +-
drivers/iommu/fsl_pamu_domain.c | 15 ++-
2 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h
index 04dcd25..342ac6d 100644
--- a/drivers/iommu/fsl_pamu.h
+++ b/drivers/iommu/fsl_pamu.h
@@ -67,7 +67,7 @@ struct pamu_mmap_regs {
#define PAMU_AVS1_GCV 0x2000
#define PAMU_AVS1_PDV 0x4000
#define PAMU_AV_MASK(PAMU_AVS1_AV | PAMU_AVS1_OTV | PAMU_AVS1_APV |
PAMU_AVS1_WAV \
- | PAMU_AVS1_LAV | PAMU_AVS1_GCV | PAMU_AVS1_PDV)
+| PAMU_AVS1_LAV | PAMU_AVS1_GCV | PAMU_AVS1_PDV)
#define PAMU_AVS1_LIODN_SHIFT 16
#define PAMU_LAV_LIODN_NOT_IN_PPAACT 0x400
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index eea2212..ae21305 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -62,8 +62,7 @@ static int __init iommu_init_mempool(void)
static phys_addr_t get_phys_addr(struct fsl_dma_domain *dma_domain, dma_addr_t
iova)
{
u32 win_cnt = dma_domain->win_cnt;
- struct dma_window *win_ptr =
- &dma_domain->win_arr[0];
+ struct dma_window *win_ptr = &dma_domain->win_arr[0];
struct iommu_domain_geometry *geom;
geom = &dma_domain->iommu_domain->geometry;
@@ -92,15 +91,13 @@ static phys_addr_t get_phys_addr(struct fsl_dma_domain
*dma_domain, dma_addr_t i
static int map_subwins(int liodn, struct fsl_dma_domain *dma_domain)
{
- struct dma_window *sub_win_ptr =
- &dma_domain->win_arr[0];
+ struct dma_window *sub_win_ptr = &dma_domain->win_arr[0];
int i, ret;
unsigned long rpn, flags;
for (i = 0; i < dma_domain->win_cnt; i++) {
if (sub_win_ptr[i].valid) {
- rpn = sub_win_ptr[i].paddr >>
-PAMU_PAGE_SHIFT;
+ rpn = sub_win_ptr[i].paddr >> PAMU_PAGE_SHIFT;
spin_lock_irqsave(&iommu_lock, flags);
ret = pamu_config_spaace(liodn, dma_domain->win_cnt, i,
sub_win_ptr[i].size,
@@ -180,8 +177,8 @@ static int update_liodn(int liodn, struct fsl_dma_domain
*dma_domain, u32 wnd_nr
wnd->size,
~(u32)0,
wnd->paddr >> PAMU_PAGE_SHIFT,
- dma_domain->snoop_id,
dma_domain->stash_id,
- 0, wnd->prot);
+dma_domain->snoop_id,
dma_domain->stash_id,
+0, wnd->prot);
if (ret)
pr_debug("Window reconfiguration failed for liodn
%d\n", liodn);
}
@@ -679,7 +676,7 @@ static int fsl_pamu_attach_device(struct iommu_domain
*domain,
} else {
pr_debug("missing fsl,liodn property at %s\n",
dev->of_node->full_name);
- ret = -EINVAL;
+ ret = -EINVAL;
}
return ret;
--
2.2.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 19/26] iommu/fsl: Fix some comments alignment
Signed-off-by: Emil Medve
---
drivers/iommu/fsl_pamu.c| 24
drivers/iommu/fsl_pamu_domain.c | 6 +++---
2 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
index dc3955f..ba5d1e0 100644
--- a/drivers/iommu/fsl_pamu.c
+++ b/drivers/iommu/fsl_pamu.c
@@ -35,7 +35,7 @@
#define make64(high, low) (((u64)(high) << 32) | (low))
struct pamu_isr_data {
- void __iomem *pamu_reg_base;/* Base address of PAMU regs*/
+ void __iomem *pamu_reg_base;/* Base address of PAMU regs */
unsigned int count; /* The number of PAMUs */
};
@@ -49,7 +49,7 @@ static struct ome *omt __initdata;
* "fsl,qoriq-device-config-2.0" corresponds to T4 & B4
* SOCs. For the older SOCs "fsl,qoriq-device-config-1.0"
* string would be used.
-*/
+ */
static const struct of_device_id guts_device_ids[] __initconst = {
{ .compatible = "fsl,qoriq-device-config-1.0", },
{ .compatible = "fsl,qoriq-device-config-2.0", },
@@ -63,7 +63,7 @@ static const struct of_device_id guts_device_ids[]
__initconst = {
* "fsl,b4860-l3-cache-controller" corresponds to B4 &
* "fsl,p4080-l3-cache-controller" corresponds to other,
* SOCs.
-*/
+ */
static const struct of_device_id l3_device_ids[] = {
{ .compatible = "fsl,t4240-l3-cache-controller", },
{ .compatible = "fsl,b4860-l3-cache-controller", },
@@ -231,7 +231,7 @@ static struct paace *pamu_get_spaace(struct paace *paace,
u32 wnum)
* If no SPAACE entry is available or the allocator can not reserve the
required
* number of contiguous entries function returns ULONG_MAX indicating a
failure.
*
-*/
+ */
static unsigned long pamu_get_fspi_and_allocate(u32 subwin_cnt)
{
unsigned long spaace_addr;
@@ -494,11 +494,11 @@ int pamu_config_spaace(int liodn, u32 subwin_cnt, u32
subwin,
}
/**
-* get_ome_index() - Returns the index in the operation mapping table
-* for device.
-* @*omi_index: pointer for storing the index value
-*
-*/
+ * get_ome_index() - Returns the index in the operation mapping table
+ * for device.
+ * @*omi_index: pointer for storing the index value
+ *
+ */
void get_ome_index(u32 *omi_index, struct device *dev)
{
if (of_device_is_compatible(dev->of_node, "fsl,qman-portal"))
@@ -610,7 +610,7 @@ static void __init setup_qbman_paace(struct paace *ppaace,
int paace_type)
case QMAN_PORTAL_PAACE:
set_bf(ppaace->impl_attr, PAACE_IA_OTM, PAACE_OTM_INDEXED);
ppaace->op_encode.index_ot.omi = OMI_QMAN;
- /*Set DQRR and Frame stashing for the L3 cache */
+ /* Set DQRR and Frame stashing for the L3 cache */
set_bf(ppaace->impl_attr, PAACE_IA_CID,
get_stash_id(PAMU_ATTR_CACHE_L3, 0));
break;
case BMAN_PAACE:
@@ -937,7 +937,7 @@ static int __init create_csd(phys_addr_t phys, size_t size,
u32 csd_port_id)
}
if (i == 0 || i == num_laws) {
- /* This should never happen*/
+ /* This should never happen */
ret = -ENOENT;
goto error;
}
@@ -1163,7 +1163,7 @@ static int __init fsl_pamu_probe(struct platform_device
*pdev)
iounmap(guts_regs);
- /* Enable DMA for the LIODNs in the device tree*/
+ /* Enable DMA for the LIODNs in the device tree */
setup_liodns();
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 51a6750..eea2212 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -285,7 +285,7 @@ static int check_size(u64 size, dma_addr_t iova)
return -EINVAL;
}
- /* iova must be page size aligned*/
+ /* iova must be page size aligned */
if (iova & (size - 1)) {
pr_debug("Address is not aligned with window size\n");
return -EINVAL;
@@ -779,7 +779,7 @@ static int configure_domain_stash(struct fsl_dma_domain
*dma_domain, void *data)
return ret;
}
-/* Configure domain dma state i.e. enable/disable DMA*/
+/* Configure domain dma state i.e. enable/disable DMA */
static int configure_domain_dma_state(struct fsl_dma_domain *dma_domain, bool
enable)
{
struct device_domain_info *info;
@@ -876,7 +876,7 @@ static bool check_pci_ctl_endpt_part(struct pci_controller
*pci_ctl)
/* Check the PCI controller version number by readding BRR1 register */
version = in_be32(pci_ctl->cfg_addr + (PCI_FSL_BRR1 >> 2));
version &= PCI_FSL_BRR1_VER;
- /* If PCI controller version is >= 0x204 we can partition endpoints*/
+ /* If PCI controller version is >= 0x204 we can partition endpoints */
if (version >= 0x204)
return 1;
--
2.2.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.li
[PATCH 22/26] iommu/fsl: Make local symbols static
drivers/iommu/fsl_pamu.c:78:17: warning: symbol 'spaace_pool' was not declared.
Should it be static?
drivers/iommu/fsl_pamu.c:762:13: warning: symbol 'pamu_av_isr' was not
declared. Should it be static?
Signed-off-by: Emil Medve
---
drivers/iommu/fsl_pamu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
index 3f83259..1d9273a 100644
--- a/drivers/iommu/fsl_pamu.c
+++ b/drivers/iommu/fsl_pamu.c
@@ -75,7 +75,7 @@ static const struct of_device_id l3_device_ids[] = {
static u32 max_subwindow_count;
/* Pool for fspi allocation */
-struct gen_pool *spaace_pool;
+static struct gen_pool *spaace_pool;
/**
* pamu_get_max_subwin_cnt() - Return the maximum supported
@@ -759,7 +759,7 @@ static void __init setup_liodns(void)
}
}
-irqreturn_t pamu_av_isr(int irq, void *arg)
+static irqreturn_t pamu_av_isr(int irq, void *arg)
{
struct pamu_isr_data *data = arg;
phys_addr_t phys;
--
2.2.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 24/26] iommu/fsl: Remove unneeded semicolon
drivers/iommu/fsl_pamu_domain.c:859:2-3: Unneeded semicolon
drivers/iommu/fsl_pamu_domain.c:833:2-3: Unneeded semicolon
Signed-off-by: Emil Medve
---
drivers/iommu/fsl_pamu_domain.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 38c26be..ebbc396 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -830,7 +830,7 @@ static int fsl_pamu_set_domain_attr(struct iommu_domain
*domain,
pr_debug("Unsupported attribute type\n");
ret = -EINVAL;
break;
- };
+ }
return ret;
}
@@ -856,7 +856,7 @@ static int fsl_pamu_get_domain_attr(struct iommu_domain
*domain,
pr_debug("Unsupported attribute type\n");
ret = -EINVAL;
break;
- };
+ }
return ret;
}
--
2.2.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 26/26] iommu/fsl: Remove extra paranthesis
Signed-off-by: Emil Medve
---
drivers/iommu/fsl_pamu.c| 4 ++--
drivers/iommu/fsl_pamu.h| 4 ++--
drivers/iommu/fsl_pamu_domain.c | 8
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
index 7a4665d..ea49d9f 100644
--- a/drivers/iommu/fsl_pamu.c
+++ b/drivers/iommu/fsl_pamu.c
@@ -160,7 +160,7 @@ int pamu_disable_liodn(int liodn)
static unsigned int map_addrspace_size_to_wse(phys_addr_t addrspace_size)
{
/* Bug if not a power of 2 */
- BUG_ON((addrspace_size & (addrspace_size - 1)));
+ BUG_ON(addrspace_size & (addrspace_size - 1));
/* window size is 2^(WSE+1) bytes */
return fls64(addrspace_size) - 2;
@@ -801,7 +801,7 @@ static irqreturn_t pamu_av_isr(int irq, void *arg)
}
/* clear access violation condition */
- out_be32((p + PAMU_AVS1), avs1 & PAMU_AV_MASK);
+ out_be32(p + PAMU_AVS1, avs1 & PAMU_AV_MASK);
paace = pamu_get_ppaace(avs1 >> PAMU_AVS1_LIODN_SHIFT);
BUG_ON(!paace);
/* check if we got a violation for a disabled LIODN */
diff --git a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h
index 342ac6d..aab723f 100644
--- a/drivers/iommu/fsl_pamu.h
+++ b/drivers/iommu/fsl_pamu.h
@@ -26,8 +26,8 @@
/* Bit Field macros
* v = bit field variable; m = mask, m##_SHIFT = shift, x = value to load
*/
-#define set_bf(v, m, x)(v = ((v) & ~(m)) | (((x) <<
(m##_SHIFT)) & (m)))
-#define get_bf(v, m) (((v) & (m)) >> (m##_SHIFT))
+#define set_bf(v, m, x)(v = ((v) & ~(m)) | (((x) << m##_SHIFT)
& (m)))
+#define get_bf(v, m) (((v) & (m)) >> m##_SHIFT)
/* PAMU CCSR space */
#define PAMU_PGC 0x /* Allows all peripheral accesses */
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 36622e2..ceebd28 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -84,7 +84,7 @@ static phys_addr_t get_phys_addr(struct fsl_dma_domain
*dma_domain, dma_addr_t i
}
if (win_ptr->valid)
- return (win_ptr->paddr + (iova & (win_ptr->size - 1)));
+ return win_ptr->paddr + (iova & (win_ptr->size - 1));
return 0;
}
@@ -386,8 +386,8 @@ static phys_addr_t fsl_pamu_iova_to_phys(struct
iommu_domain *domain,
{
struct fsl_dma_domain *dma_domain = domain->priv;
- if ((iova < domain->geometry.aperture_start) ||
- iova > (domain->geometry.aperture_end))
+ if (iova < domain->geometry.aperture_start ||
+ iova > domain->geometry.aperture_end)
return 0;
return get_phys_addr(dma_domain, iova);
@@ -1029,7 +1029,7 @@ static int fsl_pamu_set_windows(struct iommu_domain
*domain, u32 w_count)
}
ret = pamu_set_domain_geometry(dma_domain, &domain->geometry,
- ((w_count > 1) ? w_count : 0));
+ w_count > 1 ? w_count : 0);
if (!ret) {
kfree(dma_domain->win_arr);
dma_domain->win_arr = kcalloc(w_count,
--
2.2.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 10/26] iommu/fsl: Fix checkpatch type ALLOC_WITH_MULTIPLY
WARNING:ALLOC_WITH_MULTIPLY: Prefer kcalloc over kzalloc with multiply
+ dma_domain->win_arr = kzalloc(sizeof(*dma_domain->win_arr) *
+ w_count, GFP_ATOMIC);
Signed-off-by: Emil Medve
---
drivers/iommu/fsl_pamu_domain.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 9417207..dcb9bb4 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -1037,8 +1037,9 @@ static int fsl_pamu_set_windows(struct iommu_domain
*domain, u32 w_count)
((w_count > 1) ? w_count : 0));
if (!ret) {
kfree(dma_domain->win_arr);
- dma_domain->win_arr = kzalloc(sizeof(*dma_domain->win_arr) *
- w_count, GFP_ATOMIC);
+ dma_domain->win_arr = kcalloc(w_count,
+ sizeof(*dma_domain->win_arr),
+ GFP_ATOMIC);
if (!dma_domain->win_arr) {
spin_unlock_irqrestore(&dma_domain->domain_lock, flags);
return -ENOMEM;
--
2.2.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 09/26] iommu/fsl: Fix checkpatch type ALLOC_SIZEOF_STRUCT
CHECK:ALLOC_SIZEOF_STRUCT: Prefer kzalloc(sizeof(*data)...) over
kzalloc(sizeof(struct pamu_isr_data)...)
+ data = kzalloc(sizeof(struct pamu_isr_data), GFP_KERNEL);
CHECK:ALLOC_SIZEOF_STRUCT: Prefer kzalloc(sizeof(*dma_domain->win_arr)...) over
kzalloc(sizeof(struct dma_window)...)
+ dma_domain->win_arr = kzalloc(sizeof(struct dma_window)
Signed-off-by: Emil Medve
---
drivers/iommu/fsl_pamu.c| 2 +-
drivers/iommu/fsl_pamu_domain.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
index 7f0181c..4f1926b 100644
--- a/drivers/iommu/fsl_pamu.c
+++ b/drivers/iommu/fsl_pamu.c
@@ -1039,7 +1039,7 @@ static int __init fsl_pamu_probe(struct platform_device
*pdev)
goto error;
}
- data = kzalloc(sizeof(struct pamu_isr_data), GFP_KERNEL);
+ data = kzalloc(sizeof(*data), GFP_KERNEL);
if (!data) {
dev_err(&pdev->dev, "PAMU isr data memory allocation failed\n");
ret = -ENOMEM;
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index d5137dd..9417207 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -1037,7 +1037,7 @@ static int fsl_pamu_set_windows(struct iommu_domain
*domain, u32 w_count)
((w_count > 1) ? w_count : 0));
if (!ret) {
kfree(dma_domain->win_arr);
- dma_domain->win_arr = kzalloc(sizeof(struct dma_window) *
+ dma_domain->win_arr = kzalloc(sizeof(*dma_domain->win_arr) *
w_count, GFP_ATOMIC);
if (!dma_domain->win_arr) {
spin_unlock_irqrestore(&dma_domain->domain_lock, flags);
--
2.2.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/5] iommu: Allocate a default domain for iommu groups
On 28/01/15 14:30, Will Deacon wrote:
On Tue, Jan 27, 2015 at 12:08:56AM +, Joerg Roedel wrote:
From: Joerg Roedel
The default domain will be used (if supported by the iommu
driver) when the devices in the iommu group are not attached
to any other domain.
Signed-off-by: Joerg Roedel
---
drivers/iommu/iommu.c | 26 +++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 34636eb..8f33ddd3 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -76,6 +76,9 @@ struct iommu_group_attribute iommu_group_attr_##_name =
\
#define to_iommu_group(_kobj) \
container_of(_kobj, struct iommu_group, kobj)
+static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
+enum iommu_domain_type type);
+
static ssize_t iommu_group_attr_show(struct kobject *kobj,
struct attribute *__attr, char *buf)
{
@@ -362,6 +365,17 @@ rename:
kobject_get(group->devices_kobj);
+ /*
+* Try to allocate a default domain for the group, if this
+* hasn't happened yet. This is not the best place to do that,
+* it should happen in iommu_group_alloc(). But we have no
+* iommu_ops there yet, so the allocation has to happen here for
+* now.
+*/
+ if (group->default_domain == NULL)
+ group->default_domain = __iommu_domain_alloc(dev->bus,
+IOMMU_DOMAIN_DMA);
Having a per-group domain is wasteful for IOMMUs that only support a modest
number of concurrent domains, so in reality I think we need to have one
domain per IOMMU instance. Is that possible?
Strictly speaking, it is, provided you can identify instances (I've
hacked up such a thing controlled from the DMA mapping side), but
there's a question of how to handle devices with differing DMA ranges.
The Intel IOVA allocator could actually handle them sharing one address
space, since you can perform individual allocations with different
constraints, but I'm not sure if that really makes sense. Perhaps one
domain per dma-ranges variation per instance?
One problem with the current per-bus approach is that __iommu_domain_alloc
can't figure out which IOMMU instance corresponds to the group.
Indeed - I think it might make sense to pass devices around instead of
buses, and for now stick in an abstraction like:
static const struct iommu_ops *get_iommu_for_dev(struct device *dev)
{
return dev->bus->iommu_ops;
}
in which we can then later hook up some sort of of_iommu_get_ops-based
lookup for non-PCI devices. Which ends up more or less looking like
Thierry's original idea, but kept private to the IOMMU API internals.
Robin.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/6] of: iommu: add ptr to OF node arg to of_iommu_configure()
On 01/28/2015 08:32 AM, Will Deacon wrote:
On Wed, Jan 28, 2015 at 01:15:10PM +, Laurent Pinchart wrote:
On Wednesday 28 January 2015 12:29:42 Will Deacon wrote:
On Wed, Jan 28, 2015 at 12:23:03PM +, Laurent Pinchart wrote:
On Wednesday 28 January 2015 11:33:00 Will Deacon wrote:
On Mon, Jan 26, 2015 at 06:49:01PM +, Murali Karicheri wrote:
On 01/25/2015 08:32 AM, Laurent Pinchart wrote:
On Friday 23 January 2015 17:32:34 Murali Karicheri wrote:
Function of_iommu_configure() is called from of_dma_configure() to
setup iommu ops using DT property. This API is currently used for
platform devices for which DMA configuration (including iommu ops)
may come from device's parent. To extend this functionality for PCI
devices, this API need to take a parent node ptr as an argument
instead of assuming device's parent. This is needed since for PCI,
the dma configuration may be defined in the DT node of the root bus
bridge's parent device. Currently only dma-range is used for PCI and
iommu is not supported. So return error if the device is PCI.
Cc: Joerg Roedel
Cc: Grant Likely
Cc: Rob Herring
Cc: Bjorn Helgaas
Cc: Will Deacon
Cc: Russell King
Cc: Arnd Bergmann
Cc: Suravee Suthikulpanit
Signed-off-by: Murali Karicheri
---
drivers/iommu/of_iommu.c | 10 --
drivers/of/platform.c|2 +-
include/linux/of_iommu.h |6 --
3 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index af1dc6a..439235b 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -133,19 +133,25 @@ struct iommu_ops *of_iommu_get_ops(struct
device_node *np)
return ops;
}
-struct iommu_ops *of_iommu_configure(struct device *dev)
+struct iommu_ops *of_iommu_configure(struct device *dev,
+struct device_node *iommu_np)
{
struct of_phandle_args iommu_spec;
struct device_node *np;
struct iommu_ops *ops = NULL;
int idx = 0;
+ if (dev_is_pci(dev)) {
+ dev_err(dev, "iommu is currently not supported for PCI\n");
+ return NULL;
+ }
+
/*
* We don't currently walk up the tree looking for a parent
IOMMU.
* See the `Notes:' section of
* Documentation/devicetree/bindings/iommu/iommu.txt
*/
Wouldn't it be better to fix this rather than passing the device node
pointer to the function ? The solution would be more generic.
Will Deacon (Copied here) is working on this as we alteady discussed
in this thread. So it will be addressed by him in another series.
Well, "working on this" equates to "has it somewhere near the bottom of
a very long list" :)
What's your opinion on this patch then, do you think adding the iommu_np
argument makes sense as a temporary hack, or should we instead walk up the
tree looking for an iommus attribute in parent nodes ? I don't expect that
to be insanely difficult to code.
If walking up the tree is useful, then I think we should do that and update
the Documentation to reflect the new behaviour.
If I understand Murali's patch set right (please correct me if that's not the
case) the PCI code walks up the DT nodes hierarchy to the parent node that
contains the iommus attribute and passes a pointer to that node to this
function. It's thus a PCI-specific solution. As a temporary hack that's OK I
suppose, but if implementing it right straight away isn't difficult that would
be better.
It looks to me like the code walks the PCI topology to get the DT node for
the host controller, and passes *that* to of_dma_configure. That sounds
like the right thing to do to me, especially since the PCI topology is
likely not encoded in the device-tree. So actually, it is passing the
first parent node afaict.
Laurent, Will,
I don't have an IOMMU hardware to do more work on this. My patch series
has been on this list for long and it is also increasing in scope :(
I would suggest a follow up patch to this from someone (probably Will)
and my patch goes as is with out further update. Hope this is reasonable.
Murali
The part I'm more interested in is the mapping of RequesterID (PCI dma
alias) to IOMMU ID when we transition from the PCI topology to the DT
topology. Currently, it seems to be 1:1 on the platforms I have, but I
doubt this will always be the case.
Will
--
Murali Karicheri
Linux Kernel, Texas Instruments
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [v3 02/26] iommu: Add new member capability to struct irq_remap_ops
On Fri, 2014-12-12 at 23:14 +0800, Feng Wu wrote: > This patch adds a new member capability to struct irq_remap_ops, > this new function ops can be used to check whether some > features are supported, such as VT-d Posted-Interrupts. > + /* Check some capability is supported */ > + bool (*capability)(enum irq_remap_cap); > + Does this need to be a function call? Or could we just have a set of flags in the irq_remap_ops instead, with less overhead to check them? -- David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation smime.p7s Description: S/MIME cryptographic signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [v3 04/26] iommu, x86: Implement irq_set_vcpu_affinity for intel_ir_chip
On Fri, 2014-12-12 at 23:14 +0800, Feng Wu wrote: > Implement irq_set_vcpu_affinity for intel_ir_chip. > > Signed-off-by: Feng Wu > Reviewed-by: Jiang Liu Acked-by: David.Woodhouse assuming a suitable answer to... > + vcpu_pi_info = (struct vcpu_data *)vcpu_info; > + memcpy(irte_pi, &ir_data->irte_entry, sizeof(struct irte)); > + > + irte_pi->urg = 0; > + irte_pi->vector = vcpu_pi_info->vector; > + irte_pi->pda_l = (vcpu_pi_info->pi_desc_addr >> > + (32 - PDA_LOW_BIT)) & ~(-1UL << PDA_LOW_BIT); > + irte_pi->pda_h = (vcpu_pi_info->pi_desc_addr >> 32) & > + ~(-1UL << PDA_HIGH_BIT); > + > + irte_pi->__reserved_1 = 0; > + irte_pi->__reserved_2 = 0; > + irte_pi->__reserved_3 = 0; > + irte_pi->__reserved_4 = 0; do we need a barrier here before we set this bit? > + irte_pi->pst = 1; > + > + modify_irte(&ir_data->irq_2_iommu, (struct irte *)irte_pi); -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [v3 03/26] iommu, x86: Define new irte structure for VT-d Posted-Interrupts
On Fri, 2014-12-12 at 23:14 +0800, Feng Wu wrote: > Add a new irte_pi structure for VT-d Posted-Interrupts. > > Signed-off-by: Feng Wu > Reviewed-by: Jiang Liu I think it makes most sense for this to go along with the other patches rather than through me, and I'm happy for it to do so. -- David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation smime.p7s Description: S/MIME cryptographic signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [v3 06/26] iommu, x86: No need to migrating irq for VT-d Posted-Interrupts
On Fri, 2014-12-12 at 23:14 +0800, Feng Wu wrote: > We don't need to migrate the irqs for VT-d Posted-Interrupts here. > When 'pst' is set in IRTE, the associated irq will be posted to > guests instead of interrupt remapping. The destination of the > interrupt is set in Posted-Interrupts Descriptor, and the migration > happens during vCPU scheduling. > > However, we still update the cached irte here, which can be used > when changing back to remapping mode. > > Signed-off-by: Feng Wu > Reviewed-by: Jiang Liu Acked-by: David Woodhouse -- David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation smime.p7s Description: S/MIME cryptographic signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 3/8] of: fix size when dma-range is not used
On 01/28/2015 06:21 AM, Robin Murphy wrote:
Hi Murali,
[sorry, missed replying to yesterday's version]
On 27/01/15 21:00, Murali Karicheri wrote:
Fix the dma-range size when the DT attribute is missing. i.e set size to
dev->coherent_dma_mask + 1 instead of dev->coherent_dma_mask. Also add
code to check invalid values of size configured in DT and log error.
Cc: Joerg Roedel
Cc: Grant Likely
Cc: Rob Herring
Cc: Bjorn Helgaas
Cc: Will Deacon
Cc: Russell King
Cc: Arnd Bergmann
Cc: Suravee Suthikulpanit
Signed-off-by: Murali Karicheri
---
drivers/of/device.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 2de320d..17504f4 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -105,12 +105,19 @@ void of_dma_configure(struct device *dev, struct
device_node *np)
ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
if (ret < 0) {
dma_addr = offset = 0;
- size = dev->coherent_dma_mask;
+ size = dev->coherent_dma_mask + 1;
} else {
offset = PFN_DOWN(paddr - dma_addr);
dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
}
+ if (is_power_of_2(size + 1))
+ size = size + 1;
+ else if (!is_power_of_2(size)) {
+ dev_err(dev, "invalid size\n");
+ return;
+ }
+
Couldn't these checks go into the "else" path above? We don't need to
check the non-DT case, because we know we've just set it to something
sensible.
Robin,
Sure it can. I was doing flip/flop on the choice and thought it doesn'
matter either way. Please also repond to Catalin's comment if you have
any issues so that I can avoid additional spin on this patch.
Thanks
Murali
Robin.
dev->dma_pfn_offset = offset;
coherent = of_dma_is_coherent(np);
--
Murali Karicheri
Linux Kernel, Texas Instruments
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [v3 07/26] iommu, x86: Add cap_pi_support() to detect VT-d PI capability
On Fri, 2014-12-12 at 23:14 +0800, Feng Wu wrote: > Add helper function to detect VT-d Posted-Interrupts capability. > > Signed-off-by: Feng Wu > Reviewed-by: Jiang Liu Acked-by: David Woodhouse -- David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation smime.p7s Description: S/MIME cryptographic signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [v3 08/26] iommu, x86: Add intel_irq_remapping_capability() for Intel
On Fri, 2014-12-12 at 23:14 +0800, Feng Wu wrote:
> Add the Intel side implementation for capability in
> struct irq_remap_ops.
>
> Signed-off-by: Feng Wu
> Reviewed-by: Jiang Liu
> +static bool intel_irq_remapping_capability(enum irq_remap_cap cap)
> +{
> + struct dmar_drhd_unit *drhd;
> + struct intel_iommu *iommu;
> +
> + switch (cap) {
> + case IRQ_POSTING_CAP:
> + /*
> + * If 1) posted-interrupts is disabled by user
> + * or 2) irq remapping is disabled, posted-interrupts
> + * is not supported.
> + */
> + if (disable_irq_post || !irq_remapping_enabled)
> + return 0;
> +
> + for_each_iommu(iommu, drhd)
> + if (!cap_pi_support(iommu->cap))
> + return 0;
> +
If a new IOMMU is hotplugged now which doesn't support posted
interrupts, what happens?
--
David WoodhouseOpen Source Technology Centre
david.woodho...@intel.com Intel Corporation
smime.p7s
Description: S/MIME cryptographic signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [v3 26/26] iommu/vt-d: Add a command line parameter for VT-d posted-interrupts
On Fri, 2014-12-12 at 23:15 +0800, Feng Wu wrote: > Enable VT-d Posted-Interrtups and add a command line > parameter for it. > > Signed-off-by: Feng Wu Acked-by: David Woodhouse -- David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation smime.p7s Description: S/MIME cryptographic signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 3/6] of: fix size when dma-range is not used
On Wed, Jan 28, 2015 at 5:05 AM, Catalin Marinas
wrote:
> On Tue, Jan 27, 2015 at 06:55:15PM +, Murali Karicheri wrote:
>> On 01/27/2015 06:27 AM, Robin Murphy wrote:
>> > On 23/01/15 22:32, Murali Karicheri wrote:
>> >> Fix the dma-range size when the DT attribute is missing. i.e set size to
>> >> dev->coherent_dma_mask + 1 instead of dev->coherent_dma_mask. To detect
>> >> overflow when mask is set to max of u64, add a check, log error and
>> >> return.
>> >> Some platform use mask format for size in DTS. So add a work around to
>> >> catch this and fix.
>> >>
>> >> Cc: Joerg Roedel
>> >> Cc: Grant Likely
>> >> Cc: Rob Herring
>> >> Cc: Bjorn Helgaas
>> >> Cc: Will Deacon
>> >> Cc: Russell King
>> >> Cc: Arnd Bergmann
>> >> Cc: Suravee Suthikulpanit
>> >>
>> >> Signed-off-by: Murali Karicheri
>> >> ---
>> >> drivers/of/device.c | 14 +-
>> >> 1 file changed, 13 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/of/device.c b/drivers/of/device.c
>> >> index 2de320d..0a5ff54 100644
>> >> --- a/drivers/of/device.c
>> >> +++ b/drivers/of/device.c
>> >> @@ -105,12 +105,24 @@ void of_dma_configure(struct device *dev, struct
>> >> device_node *np)
>> >> ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
>> >> if (ret < 0) {
>> >> dma_addr = offset = 0;
>> >> - size = dev->coherent_dma_mask;
>> >> + size = dev->coherent_dma_mask + 1;
>> >> } else {
>> >> offset = PFN_DOWN(paddr - dma_addr);
>> >> + /*
>> >> + * Add a work around to treat the size as mask + 1 in case
>> >> + * it is defined in DT as a mask.
>> >> + */
>> >> + if (size & 1)
>> >> + size = size + 1;
>> >> dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
>> >> }
>> >>
>> >> + /* if size is 0, we have an overflow of u64 */
>> >> + if (!size) {
>> >> + dev_err(dev, "invalid size\n");
>> >> + return;
>> >> + }
>> >> +
>> >
>> > This seems potentially fragile to dodgy DTs given that we might also be
>> > using size to make a mask later. Would it make sense to double-up a
>> > sanity check as mask-format detection? Something like:
>> >
>> > if is_power_of_2(size)
>> > // use size
>> > else if is_power_of_2(size + 1)
>> > // use size + 1
>> > else
>> > // cry
>>
>> How about having the logic like this?
>>
>> ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
>> if (ret < 0) {
>> dma_addr = offset = 0;
>> size = dev->coherent_dma_mask + 1;
>> } else {
>> offset = PFN_DOWN(paddr - dma_addr);
>> dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
>> }
>>
>> if (is_power_of_2(size + 1))
>> size = size + 1;
>> else if (!is_power_of_2(size))
>> {
>> dev_err(dev, "invalid size\n");
>> return;
>> }
>
> In of_dma_configure(), we currently assume that the default coherent
> mask is 32-bit. In this thread:
>
> http://article.gmane.org/gmane.linux.kernel/1835096
>
> we talked about setting the coherent mask based on size automatically.
> I'm not sure about the size but I think we can assume is 32-bit mask + 1
> if it is not specified in the DT. Instead of just assuming a default
> mask, let's assume a default size and create the mask based on this
> (untested):
>
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 5b33c6a21807..9ff8d1286b44 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -170,10 +170,10 @@ static void of_dma_configure(struct device *dev)
> struct iommu_ops *iommu;
>
> /*
> -* Set default dma-mask to 32 bit. Drivers are expected to setup
> -* the correct supported dma_mask.
> +* Set default size to cover the 32-bit. Drivers are expected to setup
> +* the correct size and dma_mask.
> */
> - dev->coherent_dma_mask = DMA_BIT_MASK(32);
> + size = 1ULL << 32;
>
> /*
> * Set it to coherent_dma_mask by default if the architecture
> @@ -185,13 +185,24 @@ static void of_dma_configure(struct device *dev)
> ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size);
> if (ret < 0) {
> dma_addr = offset = 0;
> - size = dev->coherent_dma_mask;
Are we assuming dma_addr, paddr and size are not touched on error? If
so, we can get rid of this clause entirely.
> } else {
> offset = PFN_DOWN(paddr - dma_addr);
> dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", dev->dma_pfn_offset);
> }
> dev->dma_pfn_offset = offset;
>
> + /*
> +* Workaround for DTs setting the size to a mask or 0.
> +*/
> + if (is_power_of_2(size + 1))
> + size += 1;
As I mentioned, I think power of 2 is too restrictive (from a DT
perspective even though the kernel may require a power of 2 here for
the mask). Just checking bit0 set should be enough.
Also, we need a WARN here so DTs get fixed.
> +
> + /*
> +* Coherent DMA masks larger than
Re: [PATCH v4 3/6] of: fix size when dma-range is not used
On 28/01/15 11:05, Catalin Marinas wrote:
On Tue, Jan 27, 2015 at 06:55:15PM +, Murali Karicheri wrote:
On 01/27/2015 06:27 AM, Robin Murphy wrote:
On 23/01/15 22:32, Murali Karicheri wrote:
Fix the dma-range size when the DT attribute is missing. i.e set size to
dev->coherent_dma_mask + 1 instead of dev->coherent_dma_mask. To detect
overflow when mask is set to max of u64, add a check, log error and
return.
Some platform use mask format for size in DTS. So add a work around to
catch this and fix.
Cc: Joerg Roedel
Cc: Grant Likely
Cc: Rob Herring
Cc: Bjorn Helgaas
Cc: Will Deacon
Cc: Russell King
Cc: Arnd Bergmann
Cc: Suravee Suthikulpanit
Signed-off-by: Murali Karicheri
---
drivers/of/device.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 2de320d..0a5ff54 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -105,12 +105,24 @@ void of_dma_configure(struct device *dev, struct
device_node *np)
ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
if (ret < 0) {
dma_addr = offset = 0;
- size = dev->coherent_dma_mask;
+ size = dev->coherent_dma_mask + 1;
} else {
offset = PFN_DOWN(paddr - dma_addr);
+ /*
+ * Add a work around to treat the size as mask + 1 in case
+ * it is defined in DT as a mask.
+ */
+ if (size & 1)
+ size = size + 1;
dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
}
+ /* if size is 0, we have an overflow of u64 */
+ if (!size) {
+ dev_err(dev, "invalid size\n");
+ return;
+ }
+
This seems potentially fragile to dodgy DTs given that we might also be
using size to make a mask later. Would it make sense to double-up a
sanity check as mask-format detection? Something like:
if is_power_of_2(size)
// use size
else if is_power_of_2(size + 1)
// use size + 1
else
// cry
How about having the logic like this?
ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
if (ret < 0) {
dma_addr = offset = 0;
size = dev->coherent_dma_mask + 1;
} else {
offset = PFN_DOWN(paddr - dma_addr);
dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
}
if (is_power_of_2(size + 1))
size = size + 1;
else if (!is_power_of_2(size))
{
dev_err(dev, "invalid size\n");
return;
}
In of_dma_configure(), we currently assume that the default coherent
mask is 32-bit. In this thread:
http://article.gmane.org/gmane.linux.kernel/1835096
we talked about setting the coherent mask based on size automatically.
I'm not sure about the size but I think we can assume is 32-bit mask + 1
if it is not specified in the DT. Instead of just assuming a default
mask, let's assume a default size and create the mask based on this
(untested):
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 5b33c6a21807..9ff8d1286b44 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -170,10 +170,10 @@ static void of_dma_configure(struct device *dev)
struct iommu_ops *iommu;
/*
-* Set default dma-mask to 32 bit. Drivers are expected to setup
-* the correct supported dma_mask.
+* Set default size to cover the 32-bit. Drivers are expected to setup
+* the correct size and dma_mask.
*/
- dev->coherent_dma_mask = DMA_BIT_MASK(32);
+ size = 1ULL << 32;
/*
* Set it to coherent_dma_mask by default if the architecture
@@ -185,13 +185,24 @@ static void of_dma_configure(struct device *dev)
ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size);
if (ret < 0) {
dma_addr = offset = 0;
- size = dev->coherent_dma_mask;
} else {
offset = PFN_DOWN(paddr - dma_addr);
dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", dev->dma_pfn_offset);
}
dev->dma_pfn_offset = offset;
+ /*
+* Workaround for DTs setting the size to a mask or 0.
+*/
+ if (is_power_of_2(size + 1))
+ size += 1;
In fact, since the ilog2 below ends up effectively rounding down, we
might as well do away with this check as well and just add 1
unconditionally. The only time it makes any difference is when we want
it to anyway!
I like this approach, it ends up looking a lot neater.
Robin.
+
+ /*
+* Coherent DMA masks larger than 32-bit must be explicitly set by the
+* driver.
+*/
+ dev->coherent_dma_mask = min(DMA_BIT_MASK(32),
DMA_BIT_MASK(ilog2(size)));
+
coherent = of_dma_is_coherent(dev->of_node);
dev_dbg(dev, "device is%sdma coherent\n",
coherent ? " " : " not ");
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 3/6] of: fix size when dma-range is not used
On Wed, Jan 28, 2015 at 03:45:19PM +, Rob Herring wrote:
> On Wed, Jan 28, 2015 at 5:05 AM, Catalin Marinas
> wrote:
> > On Tue, Jan 27, 2015 at 06:55:15PM +, Murali Karicheri wrote:
> >> How about having the logic like this?
> >>
> >> ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
> >> if (ret < 0) {
> >> dma_addr = offset = 0;
> >> size = dev->coherent_dma_mask + 1;
> >> } else {
> >> offset = PFN_DOWN(paddr - dma_addr);
> >> dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
> >> }
> >>
> >> if (is_power_of_2(size + 1))
> >> size = size + 1;
> >> else if (!is_power_of_2(size))
> >> {
> >> dev_err(dev, "invalid size\n");
> >> return;
> >> }
> >
> > In of_dma_configure(), we currently assume that the default coherent
> > mask is 32-bit. In this thread:
> >
> > http://article.gmane.org/gmane.linux.kernel/1835096
> >
> > we talked about setting the coherent mask based on size automatically.
> > I'm not sure about the size but I think we can assume is 32-bit mask + 1
> > if it is not specified in the DT. Instead of just assuming a default
> > mask, let's assume a default size and create the mask based on this
> > (untested):
> >
> > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > index 5b33c6a21807..9ff8d1286b44 100644
> > --- a/drivers/of/platform.c
> > +++ b/drivers/of/platform.c
> > @@ -170,10 +170,10 @@ static void of_dma_configure(struct device *dev)
> > struct iommu_ops *iommu;
> >
> > /*
> > -* Set default dma-mask to 32 bit. Drivers are expected to setup
> > -* the correct supported dma_mask.
> > +* Set default size to cover the 32-bit. Drivers are expected to
> > setup
> > +* the correct size and dma_mask.
> > */
> > - dev->coherent_dma_mask = DMA_BIT_MASK(32);
> > + size = 1ULL << 32;
> >
> > /*
> > * Set it to coherent_dma_mask by default if the architecture
> > @@ -185,13 +185,24 @@ static void of_dma_configure(struct device *dev)
> > ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size);
> > if (ret < 0) {
> > dma_addr = offset = 0;
> > - size = dev->coherent_dma_mask;
>
> Are we assuming dma_addr, paddr and size are not touched on error? If
> so, we can get rid of this clause entirely.
We can if we initialise dma_addr and offset to 0 when declared in this
function. The dma_addr and size variables are later passed to the
arch_setup_dma_ops(), so they need to have some sane values independent
of the presence of dma-ranges in the DT.
> > } else {
> > offset = PFN_DOWN(paddr - dma_addr);
> > dev_dbg(dev, "dma_pfn_offset(%#08lx)\n",
> > dev->dma_pfn_offset);
> > }
> > dev->dma_pfn_offset = offset;
> >
> > + /*
> > +* Workaround for DTs setting the size to a mask or 0.
> > +*/
> > + if (is_power_of_2(size + 1))
> > + size += 1;
>
> As I mentioned, I think power of 2 is too restrictive (from a DT
> perspective even though the kernel may require a power of 2 here for
> the mask). Just checking bit0 set should be enough.
The power of 2 was mainly to cover the case where the size is wrongly
written as a mask in the DT. If the size is deliberately not a power of
two and not a full mask, the above check won't change it. With my
proposal, ilog2 gets rid of extra bits in size, only that if the size
was a mask because of DT error, we lose a bit in the coherent_dma_mask.
> Also, we need a WARN here so DTs get fixed.
I agree.
--
Catalin
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 3/6] of: fix size when dma-range is not used
On Wed, Jan 28, 2015 at 03:55:57PM +, Robin Murphy wrote:
> On 28/01/15 11:05, Catalin Marinas wrote:
> > On Tue, Jan 27, 2015 at 06:55:15PM +, Murali Karicheri wrote:
> >> How about having the logic like this?
> >>
> >>ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
> >>if (ret < 0) {
> >>dma_addr = offset = 0;
> >>size = dev->coherent_dma_mask + 1;
> >>} else {
> >>offset = PFN_DOWN(paddr - dma_addr);
> >>dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
> >>}
> >>
> >>if (is_power_of_2(size + 1))
> >>size = size + 1;
> >>else if (!is_power_of_2(size))
> >>{
> >>dev_err(dev, "invalid size\n");
> >>return;
> >>}
> >
> > In of_dma_configure(), we currently assume that the default coherent
> > mask is 32-bit. In this thread:
> >
> > http://article.gmane.org/gmane.linux.kernel/1835096
> >
> > we talked about setting the coherent mask based on size automatically.
> > I'm not sure about the size but I think we can assume is 32-bit mask + 1
> > if it is not specified in the DT. Instead of just assuming a default
> > mask, let's assume a default size and create the mask based on this
> > (untested):
> >
> > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > index 5b33c6a21807..9ff8d1286b44 100644
> > --- a/drivers/of/platform.c
> > +++ b/drivers/of/platform.c
> > @@ -170,10 +170,10 @@ static void of_dma_configure(struct device *dev)
> > struct iommu_ops *iommu;
> >
> > /*
> > -* Set default dma-mask to 32 bit. Drivers are expected to setup
> > -* the correct supported dma_mask.
> > +* Set default size to cover the 32-bit. Drivers are expected to setup
> > +* the correct size and dma_mask.
> > */
> > - dev->coherent_dma_mask = DMA_BIT_MASK(32);
> > + size = 1ULL << 32;
> >
> > /*
> > * Set it to coherent_dma_mask by default if the architecture
> > @@ -185,13 +185,24 @@ static void of_dma_configure(struct device *dev)
> > ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size);
> > if (ret < 0) {
> > dma_addr = offset = 0;
> > - size = dev->coherent_dma_mask;
> > } else {
> > offset = PFN_DOWN(paddr - dma_addr);
> > dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", dev->dma_pfn_offset);
> > }
> > dev->dma_pfn_offset = offset;
> >
> > + /*
> > +* Workaround for DTs setting the size to a mask or 0.
> > +*/
> > + if (is_power_of_2(size + 1))
> > + size += 1;
>
> In fact, since the ilog2 below ends up effectively rounding down, we
> might as well do away with this check as well and just add 1
> unconditionally. The only time it makes any difference is when we want
> it to anyway!
Well, we could simply ignore the is_power_of_2() check but without
incrementing size as we don't know what arch_setup_dma_ops() does with
it.
I think we can remove this check altogether (we leaved without it for a
while) but we need to add 1 when calculating the mask:
dev->coherent_dma_mask = min(DMA_BIT_MASK(32),
DMA_BIT_MASK(ilog2(size + 1)));
--
Catalin
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 3/6] of: fix size when dma-range is not used
On 01/28/2015 10:45 AM, Rob Herring wrote:
On Wed, Jan 28, 2015 at 5:05 AM, Catalin Marinas
wrote:
On Tue, Jan 27, 2015 at 06:55:15PM +, Murali Karicheri wrote:
On 01/27/2015 06:27 AM, Robin Murphy wrote:
On 23/01/15 22:32, Murali Karicheri wrote:
Fix the dma-range size when the DT attribute is missing. i.e set size to
dev->coherent_dma_mask + 1 instead of dev->coherent_dma_mask. To detect
overflow when mask is set to max of u64, add a check, log error and
return.
Some platform use mask format for size in DTS. So add a work around to
catch this and fix.
Cc: Joerg Roedel
Cc: Grant Likely
Cc: Rob Herring
Cc: Bjorn Helgaas
Cc: Will Deacon
Cc: Russell King
Cc: Arnd Bergmann
Cc: Suravee Suthikulpanit
Signed-off-by: Murali Karicheri
---
drivers/of/device.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 2de320d..0a5ff54 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -105,12 +105,24 @@ void of_dma_configure(struct device *dev, struct
device_node *np)
ret = of_dma_get_range(np,&dma_addr,&paddr,&size);
if (ret< 0) {
dma_addr = offset = 0;
- size = dev->coherent_dma_mask;
+ size = dev->coherent_dma_mask + 1;
} else {
offset = PFN_DOWN(paddr - dma_addr);
+ /*
+ * Add a work around to treat the size as mask + 1 in case
+ * it is defined in DT as a mask.
+ */
+ if (size& 1)
+ size = size + 1;
dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
}
+ /* if size is 0, we have an overflow of u64 */
+ if (!size) {
+ dev_err(dev, "invalid size\n");
+ return;
+ }
+
This seems potentially fragile to dodgy DTs given that we might also be
using size to make a mask later. Would it make sense to double-up a
sanity check as mask-format detection? Something like:
if is_power_of_2(size)
// use size
else if is_power_of_2(size + 1)
// use size + 1
else
// cry
How about having the logic like this?
ret = of_dma_get_range(np,&dma_addr,&paddr,&size);
if (ret< 0) {
dma_addr = offset = 0;
size = dev->coherent_dma_mask + 1;
} else {
offset = PFN_DOWN(paddr - dma_addr);
dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
}
if (is_power_of_2(size + 1))
size = size + 1;
else if (!is_power_of_2(size))
{
dev_err(dev, "invalid size\n");
return;
}
In of_dma_configure(), we currently assume that the default coherent
mask is 32-bit. In this thread:
http://article.gmane.org/gmane.linux.kernel/1835096
we talked about setting the coherent mask based on size automatically.
I'm not sure about the size but I think we can assume is 32-bit mask + 1
if it is not specified in the DT. Instead of just assuming a default
mask, let's assume a default size and create the mask based on this
(untested):
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 5b33c6a21807..9ff8d1286b44 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -170,10 +170,10 @@ static void of_dma_configure(struct device *dev)
struct iommu_ops *iommu;
/*
-* Set default dma-mask to 32 bit. Drivers are expected to setup
-* the correct supported dma_mask.
+* Set default size to cover the 32-bit. Drivers are expected to setup
+* the correct size and dma_mask.
*/
- dev->coherent_dma_mask = DMA_BIT_MASK(32);
+ size = 1ULL<< 32;
/*
* Set it to coherent_dma_mask by default if the architecture
@@ -185,13 +185,24 @@ static void of_dma_configure(struct device *dev)
ret = of_dma_get_range(dev->of_node,&dma_addr,&paddr,&size);
if (ret< 0) {
dma_addr = offset = 0;
- size = dev->coherent_dma_mask;
Are we assuming dma_addr, paddr and size are not touched on error? If
so, we can get rid of this clause entirely.
Checking the code for of_dma_get_range() I see paddr is modified on
error case, but is used only for success case in this function. dma_addr
and size are not modified. So setting dma_addr and offset to zero before
hand like size might work as below
dma_addr = offset = 0;
size = 1ULL << 32;
ret = of_dma_get_range(dev->of_node,&dma_addr,&paddr,&size);
if (!ret) {
offset = PFN_DOWN(paddr - dma_addr);
}
.. rest of the code.
Murali
} else {
offset = PFN_DOWN(paddr - dma_addr);
dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", dev->dma_pfn_offset);
}
dev->dma_pfn_offset = offset;
+ /*
+* Workaround for DTs setting the size to a mask or 0.
+*/
+ if (is_power_of_2(size + 1))
+ size += 1;
As I mentioned, I think power of 2 is too restrictive (from a DT
perspective even though the kernel may require a power of 2 here for
the mask). Just checking bit0 set should be enough.
Also, we need a WARN here so DTs get fixed.
+
+ /*
+* Coherent D
Re: [PATCH v4 1/6] of: iommu: add ptr to OF node arg to of_iommu_configure()
Hi Will,
On Wednesday 28 January 2015 13:32:19 Will Deacon wrote:
> On Wed, Jan 28, 2015 at 01:15:10PM +, Laurent Pinchart wrote:
> > On Wednesday 28 January 2015 12:29:42 Will Deacon wrote:
> >> On Wed, Jan 28, 2015 at 12:23:03PM +, Laurent Pinchart wrote:
> >>> On Wednesday 28 January 2015 11:33:00 Will Deacon wrote:
> On Mon, Jan 26, 2015 at 06:49:01PM +, Murali Karicheri wrote:
> > On 01/25/2015 08:32 AM, Laurent Pinchart wrote:
> >> On Friday 23 January 2015 17:32:34 Murali Karicheri wrote:
> >>> Function of_iommu_configure() is called from of_dma_configure() to
> >>> setup iommu ops using DT property. This API is currently used for
> >>> platform devices for which DMA configuration (including iommu ops)
> >>> may come from device's parent. To extend this functionality for
> >>> PCI devices, this API need to take a parent node ptr as an argument
> >>> instead of assuming device's parent. This is needed since for PCI,
> >>> the dma configuration may be defined in the DT node of the root
> >>> bus bridge's parent device. Currently only dma-range is used for PCI
> >>> and iommu is not supported. So return error if the device is PCI.
> >>>
> >>> Cc: Joerg Roedel
> >>> Cc: Grant Likely
> >>> Cc: Rob Herring
> >>> Cc: Bjorn Helgaas
> >>> Cc: Will Deacon
> >>> Cc: Russell King
> >>> Cc: Arnd Bergmann
> >>> Cc: Suravee Suthikulpanit
> >>>
> >>> Signed-off-by: Murali Karicheri
> >>> ---
> >>>
> >>> drivers/iommu/of_iommu.c | 10 --
> >>> drivers/of/platform.c|2 +-
> >>> include/linux/of_iommu.h |6 --
> >>> 3 files changed, 13 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> >>> index af1dc6a..439235b 100644
> >>> --- a/drivers/iommu/of_iommu.c
> >>> +++ b/drivers/iommu/of_iommu.c
> >>> @@ -133,19 +133,25 @@ struct iommu_ops *of_iommu_get_ops(struct
> >>> device_node *np)
> >>> return ops;
> >>> }
> >>>
> >>> -struct iommu_ops *of_iommu_configure(struct device *dev)
> >>> +struct iommu_ops *of_iommu_configure(struct device *dev,
> >>> + struct device_node *iommu_np)
> >>> {
> >>> struct of_phandle_args iommu_spec;
> >>> struct device_node *np;
> >>> struct iommu_ops *ops = NULL;
> >>> int idx = 0;
> >>>
> >>> + if (dev_is_pci(dev)) {
> >>> + dev_err(dev, "iommu is currently not supported for
> >>> PCI\n");
> >>> + return NULL;
> >>> + }
> >>> +
> >>> /*
> >>>* We don't currently walk up the tree looking for a parent
> >>>IOMMU.
> >>>* See the `Notes:' section of
> >>>* Documentation/devicetree/bindings/iommu/iommu.txt
> >>>*/
> >>
> >> Wouldn't it be better to fix this rather than passing the device
> >> node pointer to the function ? The solution would be more generic.
> >
> > Will Deacon (Copied here) is working on this as we alteady discussed
> > in this thread. So it will be addressed by him in another series.
>
> Well, "working on this" equates to "has it somewhere near the bottom
> of a very long list" :)
> >>>
> >>> What's your opinion on this patch then, do you think adding the
> >>> iommu_np argument makes sense as a temporary hack, or should we instead
> >>> walk up the tree looking for an iommus attribute in parent nodes ? I
> >>> don't expect that to be insanely difficult to code.
> >>
> >> If walking up the tree is useful, then I think we should do that and
> >> update the Documentation to reflect the new behaviour.
> >
> > If I understand Murali's patch set right (please correct me if that's not
> > the case) the PCI code walks up the DT nodes hierarchy to the parent node
> > that contains the iommus attribute and passes a pointer to that node to
> > this function. It's thus a PCI-specific solution. As a temporary hack
> > that's OK I suppose, but if implementing it right straight away isn't
> > difficult that would be better.
>
> It looks to me like the code walks the PCI topology to get the DT node for
> the host controller, and passes *that* to of_dma_configure. That sounds
> like the right thing to do to me, especially since the PCI topology is
> likely not encoded in the device-tree. So actually, it is passing the
> first parent node afaict.
Indeed, that's right. I forgot for a moment that we have non-DT devices ;-)
Acked-by: Laurent Pinchart
Murali, nitpicking a bit, shouldn't the iommu_np parameter be renamed ? It
points to the node containing the iommus parameter, not to the iommu node, so
the current name is slightly confusing. A brief kerneldoc above the function
would also help. This can be the subject of a separate patch.
> The part I'm more int
RE: [v3 04/26] iommu, x86: Implement irq_set_vcpu_affinity for intel_ir_chip
> -Original Message- > From: David Woodhouse [mailto:dw...@infradead.org] > Sent: Wednesday, January 28, 2015 11:27 PM > To: Wu, Feng > Cc: t...@linutronix.de; mi...@redhat.com; h...@zytor.com; x...@kernel.org; > g...@kernel.org; pbonz...@redhat.com; j...@8bytes.org; > alex.william...@redhat.com; jiang@linux.intel.com; eric.au...@linaro.org; > linux-ker...@vger.kernel.org; iommu@lists.linux-foundation.org; > k...@vger.kernel.org > Subject: Re: [v3 04/26] iommu, x86: Implement irq_set_vcpu_affinity for > intel_ir_chip > > On Fri, 2014-12-12 at 23:14 +0800, Feng Wu wrote: > > Implement irq_set_vcpu_affinity for intel_ir_chip. > > > > Signed-off-by: Feng Wu > > Reviewed-by: Jiang Liu > > Acked-by: David.Woodhouse assuming a > suitable answer to... > > > + vcpu_pi_info = (struct vcpu_data *)vcpu_info; > > + memcpy(irte_pi, &ir_data->irte_entry, sizeof(struct irte)); > > + > > + irte_pi->urg = 0; > > + irte_pi->vector = vcpu_pi_info->vector; > > + irte_pi->pda_l = (vcpu_pi_info->pi_desc_addr >> > > +(32 - PDA_LOW_BIT)) & ~(-1UL << PDA_LOW_BIT); > > + irte_pi->pda_h = (vcpu_pi_info->pi_desc_addr >> 32) & > > +~(-1UL << PDA_HIGH_BIT); > > + > > + irte_pi->__reserved_1 = 0; > > + irte_pi->__reserved_2 = 0; > > + irte_pi->__reserved_3 = 0; > > + irte_pi->__reserved_4 = 0; > > do we need a barrier here before we set this bit? Thanks a lot for your Ack, David! I cannot find a reason why we need a barrier here, since 'irte_pi' is only a local variant here, the real operation to program hardware occurs in modify_irte(), in which spin lock is acquired, this means the there is an implicit barrier there. Thanks, Feng > > > + irte_pi->pst = 1; > > + > > + modify_irte(&ir_data->irq_2_iommu, (struct irte *)irte_pi); > > > -- > dwmw2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
