[PATCH v2 04/04] iommu/ipmmu-vmsa: Drop LPAE Kconfig dependency
From: Magnus Damm Neither the ARM page table code enabled by IOMMU_IO_PGTABLE_LPAE nor the IPMMU_VMSA driver actually depends on ARM_LPAE, so get rid of the dependency. Tested with ipmmu-vmsa on r8a7794 ALT and a kernel config using: # CONFIG_ARM_LPAE is not set Signed-off-by: Magnus Damm Acked-by: Laurent Pinchart --- Changes since V1: - Rebased on top of ARCH_RENESAS change - Added Acked-by from Laurent This time the result also compiles on x86. Need to be applied as last patch in the following series: [PATCH v2 00/04] iommu/ipmmu-vmsa: IPMMU multi-arch update V2 drivers/iommu/Kconfig |1 - 1 file changed, 1 deletion(-) --- 0001/drivers/iommu/Kconfig +++ work/drivers/iommu/Kconfig 2016-03-15 12:28:45.210513000 +0900 @@ -284,7 +284,6 @@ config EXYNOS_IOMMU_DEBUG config IPMMU_VMSA bool "Renesas VMSA-compatible IPMMU" - depends on ARM_LPAE depends on ARCH_RENESAS || COMPILE_TEST select IOMMU_API select IOMMU_IO_PGTABLE_LPAE ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 02/04] iommu/ipmmu-vmsa: Rework interrupt code and use bitmap for context
From: Magnus Damm
Introduce a bitmap for context handing and convert the
interrupt routine to go handle all registered contexts.
At this point the number of contexts are still limited.
Also remove the use of the ARM specific mapping variable
from ipmmu_irq() to allow compile on ARM64.
Signed-off-by: Magnus Damm
---
Changes since V1: (Thanks to Laurent for feedback!)
- Use simple find_first_zero()/set_bit()/clear_bit() for context management.
- For allocation rely on spinlock held when calling ipmmu_domain_init_context()
- For test/free use atomic bitops
- Return IRQ_HANDLED if any of the contexts generated interrupts
drivers/iommu/ipmmu-vmsa.c | 47
1 file changed, 35 insertions(+), 12 deletions(-)
--- 0003/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2016-03-15 12:42:18.940513000 +0900
@@ -8,6 +8,7 @@
* the Free Software Foundation; version 2 of the License.
*/
+#include
#include
#include
#include
@@ -26,12 +27,16 @@
#include "io-pgtable.h"
+#define IPMMU_CTX_MAX 1
+
struct ipmmu_vmsa_device {
struct device *dev;
void __iomem *base;
struct list_head list;
unsigned int num_utlbs;
+ DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
+ struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
struct dma_iommu_mapping *mapping;
};
@@ -296,6 +301,7 @@ static struct iommu_gather_ops ipmmu_gat
static int ipmmu_domain_init_context(struct ipmmu_vmsa_domain *domain)
{
u64 ttbr;
+ int ret;
/*
* Allocate the page table operations.
@@ -325,10 +331,17 @@ static int ipmmu_domain_init_context(str
return -EINVAL;
/*
-* TODO: When adding support for multiple contexts, find an unused
-* context.
+* Find an unused context.
*/
- domain->context_id = 0;
+ ret = find_first_zero_bit(domain->mmu->ctx, IPMMU_CTX_MAX);
+ if (ret == IPMMU_CTX_MAX) {
+ free_io_pgtable_ops(domain->iop);
+ return -EBUSY;
+ }
+
+ domain->context_id = ret;
+ domain->mmu->domains[ret] = domain;
+ set_bit(ret, domain->mmu->ctx);
/* TTBR0 */
ttbr = domain->cfg.arm_lpae_s1_cfg.ttbr[0];
@@ -372,6 +385,8 @@ static int ipmmu_domain_init_context(str
static void ipmmu_domain_destroy_context(struct ipmmu_vmsa_domain *domain)
{
+ clear_bit(domain->context_id, domain->mmu->ctx);
+
/*
* Disable the context. Flush the TLB as required when modifying the
* context registers.
@@ -389,10 +404,15 @@ static void ipmmu_domain_destroy_context
static irqreturn_t ipmmu_domain_irq(struct ipmmu_vmsa_domain *domain)
{
const u32 err_mask = IMSTR_MHIT | IMSTR_ABORT | IMSTR_PF | IMSTR_TF;
- struct ipmmu_vmsa_device *mmu = domain->mmu;
+ struct ipmmu_vmsa_device *mmu;
u32 status;
u32 iova;
+ if (!domain)
+ return IRQ_NONE;
+
+ mmu = domain->mmu;
+
status = ipmmu_ctx_read(domain, IMSTR);
if (!(status & err_mask))
return IRQ_NONE;
@@ -437,16 +457,18 @@ static irqreturn_t ipmmu_domain_irq(stru
static irqreturn_t ipmmu_irq(int irq, void *dev)
{
struct ipmmu_vmsa_device *mmu = dev;
- struct iommu_domain *io_domain;
- struct ipmmu_vmsa_domain *domain;
-
- if (!mmu->mapping)
- return IRQ_NONE;
+ irqreturn_t status = IRQ_NONE;
+ unsigned int i;
- io_domain = mmu->mapping->domain;
- domain = to_vmsa_domain(io_domain);
+ /* Check interrupts for all active contexts */
+ for (i = 0; i < IPMMU_CTX_MAX; i++) {
+ if (!test_bit(i, mmu->ctx))
+ continue;
+ if (ipmmu_domain_irq(mmu->domains[i]) == IRQ_HANDLED)
+ status = IRQ_HANDLED;
+ }
- return ipmmu_domain_irq(domain);
+ return status;
}
/*
-
@@ -774,6 +796,7 @@ static int ipmmu_probe(struct platform_d
mmu->dev = &pdev->dev;
mmu->num_utlbs = 32;
+ bitmap_zero(mmu->ctx, IPMMU_CTX_MAX);
/* Map I/O memory and request IRQ. */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 00/04] iommu/ipmmu-vmsa: IPMMU multi-arch update V2
iommu/ipmmu-vmsa: IPMMU multi-arch update V2 [PATCH v2 01/04] iommu/ipmmu-vmsa: Remove platform data handling [PATCH v2 02/04] iommu/ipmmu-vmsa: Rework interrupt code and use bitmap for context [PATCH v2 03/04] iommu/ipmmu-vmsa: Break out 32-bit ARM mapping code [PATCH v2 04/04] iommu/ipmmu-vmsa: Drop LPAE Kconfig dependency These patches update the IPMMU driver with a few minor changes to support build on multiple architectures. With these patches applied the driver is known to compile without issues on 32-bit ARM, 64-bit ARM and x86_64. Changes since V1: - Got rid of patch 2 and 3 from initial series - Updated bitmap code locking and also used lighter bitop functions - Updated the Kconfig bits to apply on top of ARCH_RENESAS Signed-off-by: Magnus Damm --- Built on top of next-20160314 drivers/iommu/Kconfig |1 drivers/iommu/ipmmu-vmsa.c | 146 +--- 2 files changed, 97 insertions(+), 50 deletions(-) ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 03/04] iommu/ipmmu-vmsa: Break out 32-bit ARM mapping code
From: Magnus Damm
Make the driver compile on more than just 32-bit ARM
by breaking out and wrapping ARM specific functions
in #ifdefs. Not pretty, but needed to be able to use
the driver on other architectures like ARM64.
Signed-off-by: Magnus Damm
---
Changes since V1:
- Rebased to work without patch 2 and 3 from V1 series
drivers/iommu/ipmmu-vmsa.c | 94 +---
1 file changed, 62 insertions(+), 32 deletions(-)
--- 0004/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2016-03-15 12:25:45.040513000 +0900
@@ -22,8 +22,10 @@
#include
#include
+#ifdef CONFIG_ARM
#include
#include
+#endif
#include "io-pgtable.h"
@@ -38,7 +40,9 @@ struct ipmmu_vmsa_device {
DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
+#ifdef CONFIG_ARM
struct dma_iommu_mapping *mapping;
+#endif
};
struct ipmmu_vmsa_domain {
@@ -615,6 +619,60 @@ static int ipmmu_find_utlbs(struct ipmmu
return 0;
}
+#ifdef CONFIG_ARM
+static int ipmmu_map_attach(struct device *dev, struct ipmmu_vmsa_device *mmu)
+{
+ int ret;
+
+ /*
+* Create the ARM mapping, used by the ARM DMA mapping core to allocate
+* VAs. This will allocate a corresponding IOMMU domain.
+*
+* TODO:
+* - Create one mapping per context (TLB).
+* - Make the mapping size configurable ? We currently use a 2GB mapping
+* at a 1GB offset to ensure that NULL VAs will fault.
+*/
+ if (!mmu->mapping) {
+ struct dma_iommu_mapping *mapping;
+
+ mapping = arm_iommu_create_mapping(&platform_bus_type,
+ SZ_1G, SZ_2G);
+ if (IS_ERR(mapping)) {
+ dev_err(mmu->dev, "failed to create ARM IOMMU
mapping\n");
+ return PTR_ERR(mapping);
+ }
+
+ mmu->mapping = mapping;
+ }
+
+ /* Attach the ARM VA mapping to the device. */
+ ret = arm_iommu_attach_device(dev, mmu->mapping);
+ if (ret < 0) {
+ dev_err(dev, "Failed to attach device to VA mapping\n");
+ arm_iommu_release_mapping(mmu->mapping);
+ }
+
+ return ret;
+}
+static inline void ipmmu_detach(struct device *dev)
+{
+ arm_iommu_detach_device(dev);
+}
+static inline void ipmmu_release_mapping(struct ipmmu_vmsa_device *mmu)
+{
+ arm_iommu_release_mapping(mmu->mapping);
+}
+#else
+static inline int ipmmu_map_attach(struct device *dev,
+ struct ipmmu_vmsa_device *mmu)
+{
+ return 0;
+}
+static inline void ipmmu_detach(struct device *dev) {}
+static inline void ipmmu_release_mapping(struct ipmmu_vmsa_device *mmu) {}
+#endif
+
static int ipmmu_add_device(struct device *dev)
{
struct ipmmu_vmsa_archdata *archdata;
@@ -695,41 +753,13 @@ static int ipmmu_add_device(struct devic
archdata->num_utlbs = num_utlbs;
dev->archdata.iommu = archdata;
- /*
-* Create the ARM mapping, used by the ARM DMA mapping core to allocate
-* VAs. This will allocate a corresponding IOMMU domain.
-*
-* TODO:
-* - Create one mapping per context (TLB).
-* - Make the mapping size configurable ? We currently use a 2GB mapping
-* at a 1GB offset to ensure that NULL VAs will fault.
-*/
- if (!mmu->mapping) {
- struct dma_iommu_mapping *mapping;
-
- mapping = arm_iommu_create_mapping(&platform_bus_type,
- SZ_1G, SZ_2G);
- if (IS_ERR(mapping)) {
- dev_err(mmu->dev, "failed to create ARM IOMMU
mapping\n");
- ret = PTR_ERR(mapping);
- goto error;
- }
-
- mmu->mapping = mapping;
- }
-
- /* Attach the ARM VA mapping to the device. */
- ret = arm_iommu_attach_device(dev, mmu->mapping);
- if (ret < 0) {
- dev_err(dev, "Failed to attach device to VA mapping\n");
+ ret = ipmmu_map_attach(dev, mmu);
+ if (ret < 0)
goto error;
- }
return 0;
error:
- arm_iommu_release_mapping(mmu->mapping);
-
kfree(dev->archdata.iommu);
kfree(utlbs);
@@ -745,7 +775,7 @@ static void ipmmu_remove_device(struct d
{
struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
- arm_iommu_detach_device(dev);
+ ipmmu_detach(dev);
iommu_group_remove_device(dev);
kfree(archdata->utlbs);
@@ -856,7 +886,7 @@ static int ipmmu_remove(struct platform_
list_del(&mmu->list);
spin_unlock(&ipmmu_devices_lock);
- arm_iommu_release_mapping(mmu->mapping);
+ ipmmu_release_mapping(mmu);
ipmmu_device_reset(mmu);
___
[PATCH v2 01/04] iommu/ipmmu-vmsa: Remove platform data handling
From: Magnus Damm
The IPMMU driver is using DT these days, and platform data is no longer
used by the driver. Remove unused code.
Signed-off-by: Magnus Damm
Reviewed-by: Laurent Pinchart
---
Changes since V1:
- Added Reviewed-by from Laurent
drivers/iommu/ipmmu-vmsa.c |5 -
1 file changed, 5 deletions(-)
--- 0001/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2016-03-15 10:59:25.590513000 +0900
@@ -766,11 +766,6 @@ static int ipmmu_probe(struct platform_d
int irq;
int ret;
- if (!IS_ENABLED(CONFIG_OF) && !pdev->dev.platform_data) {
- dev_err(&pdev->dev, "missing platform data\n");
- return -EINVAL;
- }
-
mmu = devm_kzalloc(&pdev->dev, sizeof(*mmu), GFP_KERNEL);
if (!mmu) {
dev_err(&pdev->dev, "cannot allocate device data\n");
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 04/06] iommu/ipmmu-vmsa: Rework interrupt code and use bitmap for context
Hi Laurent,
On Tue, Dec 29, 2015 at 9:14 AM, Laurent Pinchart
wrote:
> Hi Magnus,
>
> Thank you for the patch.
Thanks for your feedback!
> On Tuesday 15 December 2015 21:02:49 Magnus Damm wrote:
>> From: Magnus Damm
>>
>> Introduce a bitmap for context handing and convert the
>> interrupt routine to go handle all registered contexts.
>>
>> At this point the number of contexts are still limited.
>
> That's all nice, but without seeing support for multiple contexts it's hard to
> tell if the implementation is correct for multiple context purpose.
>
>> The purpose of this patch is to remove the use of the
>> ARM specific mapping variable from ipmmu_irq().
>
> Why do you want to do that ?
The purpose of this series is to be able to use the IPMMU driver on
other architectures than 32-bit ARM. The main goal is to use the
driver on 64-bit ARM where the mapping variable does not exist.
>> Signed-off-by: Magnus Damm
>> ---
>>
>> drivers/iommu/ipmmu-vmsa.c | 37 ++---
>> 1 file changed, 26 insertions(+), 11 deletions(-)
>>
>> --- 0007/drivers/iommu/ipmmu-vmsa.c
>> +++ work/drivers/iommu/ipmmu-vmsa.c 2015-12-15 13:14:35.540513000 +0900
>> @@ -8,6 +8,7 @@
>> * the Free Software Foundation; version 2 of the License.
>> */
>>
>> +#include
>> #include
>> #include
>> #include
>> @@ -26,12 +27,16 @@
>>
>> #include "io-pgtable.h"
>>
>> +#define IPMMU_CTX_MAX 1
>> +
>> struct ipmmu_vmsa_device {
>> struct device *dev;
>> void __iomem *base;
>> struct list_head list;
>>
>> unsigned int num_utlbs;
>> + DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
>
> We have up to 4 context on Gen2 and 8 on Gen3, a bitmap might be slightly
> overkill.
Maybe so, but I'd rather use something standard than rolling my own.
Can you think of a better data structure?
>> + struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
>>
>> struct dma_iommu_mapping *mapping;
>> };
>> @@ -319,6 +324,7 @@ static struct iommu_gather_ops ipmmu_gat
>> static int ipmmu_domain_init_context(struct ipmmu_vmsa_domain *domain)
>> {
>> phys_addr_t ttbr;
>> + int ret;
>>
>> /*
>>* Allocate the page table operations.
>> @@ -348,10 +354,16 @@ static int ipmmu_domain_init_context(str
>> return -EINVAL;
>>
>> /*
>> - * TODO: When adding support for multiple contexts, find an unused
>> - * context.
>> + * Find an unused context.
>
> We need to support multiple devices per context or we will very soon run out
> of contexts. How to pick a proper context is a topic that needs to be
> researched, I believe IOMMU groups might come into play.
The experimental 64-bit ARM patches for this driver (on top of this
series) that I've posted makes use of IOMMU groups.
>>*/
>> - domain->context_id = 0;
>> + ret = bitmap_find_free_region(domain->mmu->ctx, IPMMU_CTX_MAX, 0);
>> + if (ret < 0) {
>> + free_io_pgtable_ops(domain->iop);
>> + return ret;
>> + }
>> +
>> + domain->context_id = ret;
>> + domain->mmu->domains[ret] = domain;
>
> This requires locking to protect against races with the interrupt handler.
Hm, it seems that I mistakenly assumed that bitmap_find_free_region()
was built on top of atomic set_bit() and managed the bitmap in an
atomic way. I believe you are correct that locking is needed. Will
fix.
>>
>> /* TTBR0 */
>> ttbr = domain->cfg.arm_lpae_s1_cfg.ttbr[0];
>> @@ -395,6 +407,8 @@ static int ipmmu_domain_init_context(str
>>
>> static void ipmmu_domain_destroy_context(struct ipmmu_vmsa_domain *domain)
>> {
>> + bitmap_release_region(domain->mmu->ctx, domain->context_id, 0);
>> +
>> /*
>>* Disable the context. Flush the TLB as required when modifying the
>>* context registers.
>> @@ -460,16 +474,16 @@ static irqreturn_t ipmmu_domain_irq(stru
>> static irqreturn_t ipmmu_irq(int irq, void *dev)
>> {
>> struct ipmmu_vmsa_device *mmu = dev;
>> - struct iommu_domain *io_domain;
>> - struct ipmmu_vmsa_domain *domain;
>> -
>> - if (!mmu->mapping)
>> - return IRQ_NONE;
>> + irqreturn_t status = IRQ_NONE;
>> + unsigned int k;
>
> i is a perfectly fine loop counter :-)
>
>> - io_domain = mmu->mapping->domain;
>> - domain = to_vmsa_domain(io_domain);
>> + /* Check interrupts for all active contexts */
>> + for (k = find_first_bit(mmu->ctx, IPMMU_CTX_MAX);
>> + k < IPMMU_CTX_MAX && status == IRQ_NONE;
>> + k = find_next_bit(mmu->ctx, IPMMU_CTX_MAX, k))
>
> You can just loop over mmu->domains and skip NULL entries.
You are right, that may be easier!
>> + status = ipmmu_domain_irq(mmu->domains[k]);
>
> Only the status of the last domain is taken into account.
Will fix, thanks!
/ magnus
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header
Hi Peter/Boris/Joerg, On 3/14/16 23:39, Peter Zijlstra wrote: On Mon, Mar 14, 2016 at 03:19:45PM +0100, Borislav Petkov wrote: On Mon, Mar 14, 2016 at 08:37:02PM +0700, Suravee Suthikulpanit wrote: Basically, we are trying to match the current Perf hierarchy for AMD IOMMU (arch/x86/events/amd/iommu.c). I can put it into arch/x86/include/asm/perf_amd_iommu.h. What would you prefer? Yeah, I was going to say the same thing - match the hierarchy so that there are no confusions between paths. Makes sense to me. Well there's still the 'perf' vs' events' thing, but also what other files did you want to put there? For now I think I prefer a filename without extra directories; we can always move files about if there's more use later. Also, since its being used by both events/amd/iommu.c and drivers/iommu/amd_iommu.c you can also chose a name in the latter namespace. Actually, I also found that there is currently the include/linux/amd-iommu.h, which contains extern function declarations defined in drivers/iommu/amd_iommu_init.c and drivers/iommu/amd_iommu_v2.c. What if I just merge the newly introduced arch/x86/include/perf/amd/iommu.h into the include/linux/amd-iommu.h? I do not see the point of having to separate things out into two files. Joerg, since you were maintaining that file, do you have any objection? Thanks, Suravee ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 3/6] PCI: Add support for multiple DMA aliases
On Thu, 2016-02-25 at 08:38 -0600, Bjorn Helgaas wrote:
>
> > /*
> > - * Look for aliases to or from the given device for exisiting groups. The
> > - * dma_alias_devfn only supports aliases on the same bus, therefore the
> > search
> > + * Look for aliases to or from the given device for existing groups. DMA
> > + * aliases are only supported on the same bus, therefore the search
>
> I'm trying to reconcile this statement that "DMA aliases are only
> supported on the same bus" (which was there even before this patch)
> with the fact that pci_for_each_dma_alias() does not have that
> limitation.
Doesn't it? You can still only set a DMA alias on the same bus with
pci_add_dma_alias(), can't you?
> > * space is quite small (especially since we're really only looking at pcie
> > * device, and therefore only expect multiple slots on the root complex or
> > * downstream switch ports). It's conceivable though that a pair of
> > @@ -686,11 +692,8 @@ static struct iommu_group *get_pci_alias_group(struct
> > pci_dev *pdev,
> > continue;
> >
> > /* We alias them or they alias us */
> > - if (((pdev->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN) &&
> > - pdev->dma_alias_devfn == tmp->devfn) ||
> > - ((tmp->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN) &&
> > - tmp->dma_alias_devfn == pdev->devfn)) {
> > -
> > + if (dma_alias_is_enabled(pdev, tmp->devfn) ||
> > + dma_alias_is_enabled(tmp, pdev->devfn)) {
> > group = get_pci_alias_group(tmp, devfns);
>
> We basically have this:
>
> for_each_pci_dev(tmp) {
> if ()
> group = get_pci_alias_group();
> ...
> }
Strictly, that's:
for_each_pci_dev(tmp) {
if (pdev is an alias of tmp || tmp is an alias of pdev)
group = get_pci_alias_group();
...
}
> The DMA alias stuff relies on PCI internals, so it doesn't doesn't
> seem quite right to use things like PCI_DEV_FLAGS_DMA_ALIAS_DEVFN and
> dma_alias_devfn here in the IOMMU code.
>
> I'm trying to figure out why we don't do something like the following
> instead:
>
> callback(struct pci_dev *pdev, u16 alias, void *opaque)
> {
> struct iommu_group *group;
>
> group = get_pci_alias_group();
> if (group)
> return group;
>
> return 0;
> }
>
> pci_for_each_dma_alias(pdev, callback, ...);
And this would be equivalent to
for_each_pci_dev(tmp) {
if (tmp is an alias of pdev)
group = get_pci_alias_group();
...
}
The "is an alias of" property is not commutative. Perhaps it should be.
But that's hard because in some cases the alias doesn't even *exist* as
a real PCI device. It's just that you appear to get DMA transactions
from a given source-id.
--
dwmw2
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 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header
On Mon, Mar 14, 2016 at 03:19:45PM +0100, Borislav Petkov wrote: > On Mon, Mar 14, 2016 at 08:37:02PM +0700, Suravee Suthikulpanit wrote: > > Basically, we are trying to match the current Perf hierarchy for AMD IOMMU > > (arch/x86/events/amd/iommu.c). I can put it into > > arch/x86/include/asm/perf_amd_iommu.h. What would you prefer? > > Yeah, I was going to say the same thing - match the hierarchy so that > there are no confusions between paths. Makes sense to me. Well there's still the 'perf' vs' events' thing, but also what other files did you want to put there? For now I think I prefer a filename without extra directories; we can always move files about if there's more use later. Also, since its being used by both events/amd/iommu.c and drivers/iommu/amd_iommu.c you can also chose a name in the latter namespace. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header
On Mon, Mar 14, 2016 at 08:37:02PM +0700, Suravee Suthikulpanit wrote: > Basically, we are trying to match the current Perf hierarchy for AMD IOMMU > (arch/x86/events/amd/iommu.c). I can put it into > arch/x86/include/asm/perf_amd_iommu.h. What would you prefer? Yeah, I was going to say the same thing - match the hierarchy so that there are no confusions between paths. Makes sense to me. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header
Hi, On 3/14/16 16:58, Peter Zijlstra wrote: On Mon, Mar 14, 2016 at 12:26:00PM +0700, Suravee Suthikulpanit wrote: Hi, On 03/12/2016 08:22 PM, Peter Zijlstra wrote: On Tue, Feb 23, 2016 at 08:12:36AM -0600, Suravee Suthikulpanit wrote: From: Suravee Suthikulpanit First, this patch move arch/x86/events/amd/iommu.h to arch/x86/include/asm/perf/amd/iommu.h so that we easily include it in both perf-amd-iommu and amd-iommu drivers. Then, we consolidate declaration of AMD IOMMU performance counter APIs into one file. These seem two independent thingies; should this therefore not be 2 patches? Reviewed-by: Joerg Roedel Signed-off-by: Suravee Suthikulpanit --- arch/x86/events/amd/iommu.c | 2 +- arch/x86/events/amd/iommu.h | 40 - arch/x86/include/asm/perf/amd/iommu.h | 42 +++ That seems somewhat excessive. Not only do you create arch/x86/include/asm/perf/ you then put another directory on top of that. The original header files (arch/x86/events/amd/iommu.h and drivers/iommu/amd_iommu_proto.h) has duplicate function declarations. So, with the new header file being in the arch/x86/include/asm/perf/amd/iommu.h, we can just have one function declaration. So, you just want to separate the file moving part and the part that removes of the duplication? I'm fine with a new header, it just seems putting it in a two deep direcotry hierarchy of its own that seems excessive. Basically, we are trying to match the current Perf hierarchy for AMD IOMMU (arch/x86/events/amd/iommu.c). I can put it into arch/x86/include/asm/perf_amd_iommu.h. What would you prefer? Thanks, Suravee ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/2] iommu/io-pgtable: Add MTK 4GB mode in Short-descriptor
On 13/03/16 22:01, Yong Wu wrote:
In MT8173, Normally the first 1GB PA is for the HW SRAM and Regs,
so the PA will be 33bits if the dram size is 4GB. We have a
"DRAM 4GB mode" toggle bit for this. If it's enabled, from CPU's
point of view, the dram PA will be from 0x1_~0x1_.
In short descriptor, the pagetable descriptor is always 32bit.
Mediatek extend bit9 in the lvl1 and lvl2 pgtable descriptor
as the 4GB mode.
In the 4GB mode, the bit9 must be set, then M4U help add 0x1_
based on the PA in pagetable. Thus the M4U output address to EMI is
always 33bits(the input address is still 32bits).
We add a special quirk for this MTK-4GB mode. And in the standard
spec, Bit9 in the lvl1 is "IMPLEMENTATION DEFINED", while it's AP[2]
in the lvl2, therefore if this quirk is enabled, NO_PERMS is also
expected.
Cool, thanks for making it make sense.
Signed-off-by: Yong Wu
I guess there's some other hardware magic that deals with the address in
the TTBR being truncated to 32 bits, but for this code doing what it
claims to do:
Reviewed-by: Robin Murphy
---
drivers/iommu/io-pgtable-arm-v7s.c | 13 -
drivers/iommu/io-pgtable.h | 6 ++
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/io-pgtable-arm-v7s.c
b/drivers/iommu/io-pgtable-arm-v7s.c
index 9fcceb1..32b371b 100644
--- a/drivers/iommu/io-pgtable-arm-v7s.c
+++ b/drivers/iommu/io-pgtable-arm-v7s.c
@@ -121,6 +121,8 @@
#define ARM_V7S_TEX_MASK 0x7
#define ARM_V7S_ATTR_TEX(val) (((val) & ARM_V7S_TEX_MASK) <<
ARM_V7S_TEX_SHIFT)
+#define ARM_V7S_ATTR_MTK_4GB BIT(9) /* MTK extend it for 4GB mode */
+
/* *well, except for TEX on level 2 large pages, of course :( */
#define ARM_V7S_CONT_PAGE_TEX_SHIFT 6
#define ARM_V7S_CONT_PAGE_TEX_MASK(ARM_V7S_TEX_MASK <<
ARM_V7S_CONT_PAGE_TEX_SHIFT)
@@ -364,6 +366,9 @@ static int arm_v7s_init_pte(struct arm_v7s_io_pgtable *data,
if (lvl == 1 && (cfg->quirks & IO_PGTABLE_QUIRK_ARM_NS))
pte |= ARM_V7S_ATTR_NS_SECTION;
+ if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_4GB)
+ pte |= ARM_V7S_ATTR_MTK_4GB;
+
if (num_entries > 1)
pte = arm_v7s_pte_to_cont(pte, lvl);
@@ -625,9 +630,15 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct
io_pgtable_cfg *cfg,
if (cfg->quirks & ~(IO_PGTABLE_QUIRK_ARM_NS |
IO_PGTABLE_QUIRK_NO_PERMS |
- IO_PGTABLE_QUIRK_TLBI_ON_MAP))
+ IO_PGTABLE_QUIRK_TLBI_ON_MAP |
+ IO_PGTABLE_QUIRK_ARM_MTK_4GB))
return NULL;
+ /* If ARM_MTK_4GB is enabled, the NO_PERMS is also expected. */
+ if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_4GB &&
+ !(cfg->quirks & IO_PGTABLE_QUIRK_NO_PERMS))
+ return NULL;
+
data = kmalloc(sizeof(*data), GFP_KERNEL);
if (!data)
return NULL;
diff --git a/drivers/iommu/io-pgtable.h b/drivers/iommu/io-pgtable.h
index d4f5027..969d82c 100644
--- a/drivers/iommu/io-pgtable.h
+++ b/drivers/iommu/io-pgtable.h
@@ -60,10 +60,16 @@ struct io_pgtable_cfg {
* IO_PGTABLE_QUIRK_TLBI_ON_MAP: If the format forbids caching invalid
* (unmapped) entries but the hardware might do so anyway, perform
* TLB maintenance when mapping as well as when unmapping.
+*
+* IO_PGTABLE_QUIRK_ARM_MTK_4GB: (ARM v7s format) Set bit 9 in all
+* PTEs, for Mediatek IOMMUs which treat it as a 33rd address bit
+* when the SoC is in "4GB mode" and they can only access the high
+* remap of DRAM (0x1_ to 0x1_).
*/
#define IO_PGTABLE_QUIRK_ARM_NS BIT(0)
#define IO_PGTABLE_QUIRK_NO_PERMS BIT(1)
#define IO_PGTABLE_QUIRK_TLBI_ON_MAPBIT(2)
+ #define IO_PGTABLE_QUIRK_ARM_MTK_4GBBIT(3)
unsigned long quirks;
unsigned long pgsize_bitmap;
unsigned intias;
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header
On Mon, Mar 14, 2016 at 12:26:00PM +0700, Suravee Suthikulpanit wrote: > Hi, > > On 03/12/2016 08:22 PM, Peter Zijlstra wrote: > >On Tue, Feb 23, 2016 at 08:12:36AM -0600, Suravee Suthikulpanit wrote: > >>From: Suravee Suthikulpanit > >> > >>First, this patch move arch/x86/events/amd/iommu.h to > >>arch/x86/include/asm/perf/amd/iommu.h so that we easily include > >>it in both perf-amd-iommu and amd-iommu drivers. > >> > >>Then, we consolidate declaration of AMD IOMMU performance counter > >>APIs into one file. > > > >These seem two independent thingies; should this therefore not be 2 > >patches? > > > >>Reviewed-by: Joerg Roedel > >>Signed-off-by: Suravee Suthikulpanit > >>--- > >> arch/x86/events/amd/iommu.c | 2 +- > >> arch/x86/events/amd/iommu.h | 40 > >> - > >> arch/x86/include/asm/perf/amd/iommu.h | 42 > >> +++ > > > >That seems somewhat excessive. Not only do you create > >arch/x86/include/asm/perf/ you then put another directory on top of > >that. > > > > The original header files (arch/x86/events/amd/iommu.h and > drivers/iommu/amd_iommu_proto.h) has duplicate function declarations. So, > with the new header file being in the arch/x86/include/asm/perf/amd/iommu.h, > we can just have one function declaration. > > So, you just want to separate the file moving part and the part that removes > of the duplication? I'm fine with a new header, it just seems putting it in a two deep direcotry hierarchy of its own that seems excessive. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [v6, 5/5] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0
> -Original Message-
> From: Arnd Bergmann [mailto:a...@arndb.de]
> Sent: Monday, March 14, 2016 6:26 AM
> To: linuxppc-...@lists.ozlabs.org
> Cc: Yangbo Lu; devicet...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; linux-ker...@vger.kernel.org; linux-
> c...@vger.kernel.org; linux-...@vger.kernel.org; iommu@lists.linux-
> foundation.org; net...@vger.kernel.org; linux-...@vger.kernel.org;
> ulf.hans...@linaro.org; Zhao Qiang; Russell King; Bhupesh Sharma; Joerg
> Roedel; Santosh Shilimkar; Scott Wood; Rob Herring; Claudiu Manoil; Kumar
> Gala; Yang-Leo Li; Xiaobo Xie
> Subject: Re: [v6, 5/5] mmc: sdhci-of-esdhc: fix host version for T4240-
> R1.0-R2.0
>
> On Wednesday 09 March 2016 18:08:51 Yangbo Lu wrote:
> > @@ -567,10 +580,20 @@ static void esdhc_init(struct platform_device
> *pdev, struct sdhci_host *host)
> > struct sdhci_pltfm_host *pltfm_host;
> > struct sdhci_esdhc *esdhc;
> > u16 host_ver;
> > + u32 svr;
> >
> > pltfm_host = sdhci_priv(host);
> > esdhc = sdhci_pltfm_priv(pltfm_host);
> >
> > + fsl_guts_init();
> > + svr = fsl_guts_get_svr();
> > + if (svr) {
> > + esdhc->soc_ver = SVR_SOC_VER(svr);
> > + esdhc->soc_rev = SVR_REV(svr);
> > + } else {
> > + dev_err(&pdev->dev, "Failed to get SVR value!\n");
> > + }
> > +
>
> This makes the driver non-portable. Better identify the specific
> workarounds based on the compatible string for this device, or add a
> boolean DT property for the quirk.
>
> Arnd
[Lu Yangbo-B47093] Hi Arnd, we did have a discussion about using DTS in v1
before.
https://patchwork.kernel.org/patch/6834221/
We don't have a separate DTS file for each revision of an SOC and if we did,
we'd constantly have people using the wrong one.
In addition, the device tree is stable ABI and errata are often discovered
after device tree are deployed.
See the link for details.
So we decide to read SVR from the device-config/guts MMIO block other than
using DTS.
Thanks.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
