[PATCH] iommu/arm-smmu: Add support for MSI on SMMUv3

2015-10-06 Thread Marc Zyngier 
Despite being a platform device, the SMMUv3 is capable of signaling
interrupts using MSIs. Hook it into the platform MSI framework and
enjoy faults being reported in a new and exciting way.

Signed-off-by: Marc Zyngier 
---
 drivers/iommu/arm-smmu-v3.c | 78 +++--
 1 file changed, 76 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index dafaf59..38f24bf 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -2182,6 +2183,63 @@ static int arm_smmu_write_reg_sync(struct 
arm_smmu_device *smmu, u32 val,
  1, ARM_SMMU_POLL_TIMEOUT_US);
 }
 
+static void arm_smmu_write_msi_msg(struct msi_desc *desc, struct msi_msg *msg)
+{
+   struct device *dev = msi_desc_to_dev(desc);
+   struct arm_smmu_device *smmu = dev_get_drvdata(dev);
+   phys_addr_t cfg0_offset, cfg1_offset, cfg2_offset;
+   phys_addr_t doorbell;
+
+   switch (desc->platform.msi_index) {
+   case 0: /* EVTQ */
+   cfg0_offset = ARM_SMMU_EVTQ_IRQ_CFG0;
+   cfg1_offset = ARM_SMMU_EVTQ_IRQ_CFG1;
+   cfg2_offset = ARM_SMMU_EVTQ_IRQ_CFG2;
+   break;
+   case 1: /* GERROR */
+   cfg0_offset = ARM_SMMU_GERROR_IRQ_CFG0;
+   cfg1_offset = ARM_SMMU_GERROR_IRQ_CFG1;
+   cfg2_offset = ARM_SMMU_GERROR_IRQ_CFG2;
+   break;
+   case 2: /* PRIQ */
+   cfg0_offset = ARM_SMMU_PRIQ_IRQ_CFG0;
+   cfg1_offset = ARM_SMMU_PRIQ_IRQ_CFG1;
+   cfg2_offset = ARM_SMMU_PRIQ_IRQ_CFG2;
+   break;
+   default:/* Unknown */
+   return;
+   }
+
+   doorbell = (((u64)msg->address_hi) << 32) | msg->address_lo;
+   doorbell &= MSI_CFG0_ADDR_MASK << MSI_CFG0_ADDR_SHIFT;
+
+   writeq_relaxed(doorbell, smmu->base + cfg0_offset);
+   writew_relaxed(msg->data, smmu->base + cfg1_offset);
+   writew_relaxed(MSI_CFG2_MEMATTR_DEVICE_nGnRE,
+  smmu->base + cfg2_offset);
+}
+
+static void arm_smmu_msi_override_irqs(struct arm_smmu_device *smmu)
+{
+   struct msi_desc *desc;
+
+   for_each_msi_entry(desc, smmu->dev) {
+   switch (desc->platform.msi_index) {
+   case 0: /* EVTQ */
+   smmu->evtq.q.irq = desc->irq;
+   break;
+   case 1: /* GERROR */
+   smmu->gerr_irq = desc->irq;
+   break;
+   case 2: /* PRIQ */
+   smmu->priq.q.irq = desc->irq;
+   break;
+   default:/* Unknown */
+   continue;
+   }
+   }
+}
+
 static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
 {
int ret, irq;
@@ -2198,6 +2256,23 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device 
*smmu)
/* Clear the MSI address regs */
writeq_relaxed(0, smmu->base + ARM_SMMU_GERROR_IRQ_CFG0);
writeq_relaxed(0, smmu->base + ARM_SMMU_EVTQ_IRQ_CFG0);
+   if (smmu->features & ARM_SMMU_FEAT_PRI)
+   writeq_relaxed(0, smmu->base + ARM_SMMU_PRIQ_IRQ_CFG0);
+
+   /* Allocate MSIs for evtq, gerror and priq. Ignore cmdq */
+   if (smmu->features & ARM_SMMU_FEAT_MSI) {
+   int nvecs = 2;
+
+   if (smmu->features & ARM_SMMU_FEAT_PRI)
+   nvecs++;
+
+   ret = platform_msi_domain_alloc_irqs(smmu->dev, nvecs,
+arm_smmu_write_msi_msg);
+   if (ret)
+   dev_warn(smmu->dev, "failed to allocate MSIs\n");
+   else
+   arm_smmu_msi_override_irqs(smmu);
+   }
 
/* Request wired interrupt lines */
irq = smmu->evtq.q.irq;
@@ -2228,8 +2303,6 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device 
*smmu)
}
 
if (smmu->features & ARM_SMMU_FEAT_PRI) {
-   writeq_relaxed(0, smmu->base + ARM_SMMU_PRIQ_IRQ_CFG0);
-
irq = smmu->priq.q.irq;
if (irq) {
ret = devm_request_threaded_irq(smmu->dev, irq,
@@ -2562,6 +2635,7 @@ static int arm_smmu_device_dt_probe(struct 
platform_device *pdev)
return -ENOMEM;
}
smmu->dev = dev;
+   dev_set_drvdata(dev, smmu);
 
/* Base address */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-- 
2.1.4

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v2] iommu/arm-smmu: Add support for MSI on SMMUv3

2015-10-06 Thread Marc Zyngier 
Despite being a platform device, the SMMUv3 is capable of signaling
interrupts using MSIs. Hook it into the platform MSI framework and
enjoy faults being reported in a new and exciting way.

Signed-off-by: Marc Zyngier 
---
Now rebased on top of Will's iommu/devel branch, which leads to
a slightly different patch.

 drivers/iommu/arm-smmu-v3.c | 82 ++---
 1 file changed, 78 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 5b11b77..3ccc8ed 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -2176,6 +2177,63 @@ static int arm_smmu_write_reg_sync(struct 
arm_smmu_device *smmu, u32 val,
  1, ARM_SMMU_POLL_TIMEOUT_US);
 }
 
+static void arm_smmu_write_msi_msg(struct msi_desc *desc, struct msi_msg *msg)
+{
+   struct device *dev = msi_desc_to_dev(desc);
+   struct arm_smmu_device *smmu = dev_get_drvdata(dev);
+   phys_addr_t cfg0_offset, cfg1_offset, cfg2_offset;
+   phys_addr_t doorbell;
+
+   switch (desc->platform.msi_index) {
+   case 0: /* EVTQ */
+   cfg0_offset = ARM_SMMU_EVTQ_IRQ_CFG0;
+   cfg1_offset = ARM_SMMU_EVTQ_IRQ_CFG1;
+   cfg2_offset = ARM_SMMU_EVTQ_IRQ_CFG2;
+   break;
+   case 1: /* GERROR */
+   cfg0_offset = ARM_SMMU_GERROR_IRQ_CFG0;
+   cfg1_offset = ARM_SMMU_GERROR_IRQ_CFG1;
+   cfg2_offset = ARM_SMMU_GERROR_IRQ_CFG2;
+   break;
+   case 2: /* PRIQ */
+   cfg0_offset = ARM_SMMU_PRIQ_IRQ_CFG0;
+   cfg1_offset = ARM_SMMU_PRIQ_IRQ_CFG1;
+   cfg2_offset = ARM_SMMU_PRIQ_IRQ_CFG2;
+   break;
+   default:/* Unknown */
+   return;
+   }
+
+   doorbell = (((u64)msg->address_hi) << 32) | msg->address_lo;
+   doorbell &= MSI_CFG0_ADDR_MASK << MSI_CFG0_ADDR_SHIFT;
+
+   writeq_relaxed(doorbell, smmu->base + cfg0_offset);
+   writew_relaxed(msg->data, smmu->base + cfg1_offset);
+   writew_relaxed(MSI_CFG2_MEMATTR_DEVICE_nGnRE,
+  smmu->base + cfg2_offset);
+}
+
+static void arm_smmu_msi_override_irqs(struct arm_smmu_device *smmu)
+{
+   struct msi_desc *desc;
+
+   for_each_msi_entry(desc, smmu->dev) {
+   switch (desc->platform.msi_index) {
+   case 0: /* EVTQ */
+   smmu->evtq.q.irq = desc->irq;
+   break;
+   case 1: /* GERROR */
+   smmu->gerr_irq = desc->irq;
+   break;
+   case 2: /* PRIQ */
+   smmu->priq.q.irq = desc->irq;
+   break;
+   default:/* Unknown */
+   continue;
+   }
+   }
+}
+
 static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
 {
int ret, irq;
@@ -2192,6 +2250,23 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device 
*smmu)
/* Clear the MSI address regs */
writeq_relaxed(0, smmu->base + ARM_SMMU_GERROR_IRQ_CFG0);
writeq_relaxed(0, smmu->base + ARM_SMMU_EVTQ_IRQ_CFG0);
+   if (smmu->features & ARM_SMMU_FEAT_PRI)
+   writeq_relaxed(0, smmu->base + ARM_SMMU_PRIQ_IRQ_CFG0);
+
+   /* Allocate MSIs for evtq, gerror and priq. Ignore cmdq */
+   if (smmu->features & ARM_SMMU_FEAT_MSI) {
+   int nvecs = 2;
+
+   if (smmu->features & ARM_SMMU_FEAT_PRI)
+   nvecs++;
+
+   ret = platform_msi_domain_alloc_irqs(smmu->dev, nvecs,
+arm_smmu_write_msi_msg);
+   if (ret)
+   dev_warn(smmu->dev, "failed to allocate MSIs\n");
+   else
+   arm_smmu_msi_override_irqs(smmu);
+   }
 
/* Request wired interrupt lines */
irq = smmu->evtq.q.irq;
@@ -,8 +2297,6 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device 
*smmu)
}
 
if (smmu->features & ARM_SMMU_FEAT_PRI) {
-   writeq_relaxed(0, smmu->base + ARM_SMMU_PRIQ_IRQ_CFG0);
-
irq = smmu->priq.q.irq;
if (irq) {
ret = devm_request_threaded_irq(smmu->dev, irq,
@@ -2602,13 +2675,14 @@ static int arm_smmu_device_dt_probe(struct 
platform_device *pdev)
if (ret)
return ret;
 
+   /* Record our private device structure */
+   platform_set_drvdata(pdev, smmu);
+
/* Reset the device */
ret = arm_smmu_device_reset(smmu);
if (ret)
goto out_free_structures;
 
-   /* Record our private device structure */
-

Re: [dm-devel] AMD-Vi IO_PAGE_FAULTs and ata3.00: failed command: READ FPDMA QUEUED errors since Linux 4.0

2015-10-06 Thread Andreas Hartmann 
On 10/06/2015 at 12:13 PM, Joerg Roedel wrote:
> On Wed, Sep 30, 2015 at 04:52:47PM +0200, Andreas Hartmann wrote:
>>> Alternativly someone who can reproduce it should trace the calls to
>>> __map_single and __unmap_single in the AMD IOMMU driver to find out
>>> whether the addresses which the faults happen on are really mapped, or
>>> at least requested from the AMD IOMMU driver.
>>
>> How can I trace it?
> 
> Please apply the attached debug patch on-top of Linux v4.3-rc3 and boot
> the machine. After boot you run (as root):
> 
> 
>   # cat /sys/kernel/debug/tracing/trace_pipe > trace-data
> 
> Please run this in a seperate shell an keep it running.
> 
> Then trigger the problem while the above command is running. When you
> triggered it, please send me the (compressed) trace-data file, full
> dmesg and output of lspci on the box.

Hmm, *seems* to work fine w/ 4.3-rc2. But I have to do some more tests
to be really sure.


W/ 4.1.10, the problem can be seen most always during boot (systemd) -
but at this point, it is difficult to trace. I have to take a closer
look to find a place to start the trace already during boot process.


But there is another problem w/ 4.3-rc2: Starting a VM w/ PCIe
passthrough doesn't work any more. I'm getting the attached null pointer
dereference and the machine hangs.


Thanks,
regards,
Andreas
Oct  6 20:11:18 localhost kernel: [   32.461794] BUG: unable to handle kernel 
NULL pointer dereference at 00b8
Oct  6 20:11:18 localhost kernel: [   32.461853] IP: [] 
do_detach+0x24/0xa0
Oct  6 20:11:18 localhost kernel: [   32.461888] PGD 0 
Oct  6 20:11:18 localhost kernel: [   32.461902] Oops: 0002 [#1] PREEMPT SMP 
Oct  6 20:11:18 localhost kernel: [   32.461929] Modules linked in: nf_log_ipv4 
nf_log_common xt_LOG ipt_REJECT xt_tcpudp nf_conntrack_ipv4 nf_defrag_ipv4 
xt_conntrack nf_conntrack iptable_filter ip_tables x_tables vfio_iommu_type1 
vfio_pci vfio vfio_virqfd drbg ansi_cprng nfsd lockd grace nfs_acl auth_rpcgss 
sunrpc bridge stp llc tun it87 hwmon_vid snd_hda_codec_hdmi kvm_amd 
snd_hda_codec_realtek kvm snd_hda_codec_generic fam15h_power usb_storage 
snd_hda_intel pcspkr serio_raw snd_hda_codec edac_core snd_hda_core 
edac_mce_amd k10temp snd_hwdep snd_pcm firewire_ohci snd_seq e100 firewire_core 
crc_itu_t amdkfd sp5100_tco amd_iommu_v2 i2c_piix4 mxm_wmi sr_mod cdrom radeon 
snd_timer snd_seq_device snd ttm drm_kms_helper xhci_pci drm r8169 xhci_hcd mii 
fb_sys_fops sysimgblt sysfillrect syscopyarea soundcore i2c_algo_bit shpchp 
tpm_infineon tpm_tis tpm fjes 8250_fintek wmi button acpi_cpufreq sg thermal 
xfs libcrc32c linear crct10dif_pclmul crc32_pclmul crc32c_intel 
ghash_clmulni_intel ohci_pci processor scsi_dh_rdac scsi_dh_hp_sw scsi_dh_emc 
scsi_dh_alua raid456 async_raid6_recov async_pq async_xor xor async_memcpy 
async_tx raid6_pq raid10 raid1 raid0 md_mod dm_snapshot dm_bufio dm_mirror 
dm_region_hash dm_log dm_crypt dm_mod aesni_intel ablk_helper cryptd lrw 
gf128mul glue_helper aes_x86_64 ata_generic pata_atiixp
Oct  6 20:11:18 localhost kernel: [   32.462728] CPU: 0 PID: 9374 Comm: 
qemu-system-x86 Not tainted 4.3.0-rc2-4-desktop #1
Oct  6 20:11:18 localhost kernel: [   32.462767] Hardware name: Gigabyte 
Technology Co., Ltd. GA-990XA-UD3/GA-990XA-UD3, BIOS F14b 01/24/2013
Oct  6 20:11:18 localhost kernel: [   32.462814] task: 8805f4ecc080 ti: 
8805e1ec4000 task.ti: 8805e1ec4000
Oct  6 20:11:18 localhost kernel: [   32.462851] RIP: 0010:[] 
 [] do_detach+0x24/0xa0
Oct  6 20:11:18 localhost kernel: [   32.462894] RSP: 0018:8805e1ec7ca0  
EFLAGS: 00010006
Oct  6 20:11:18 localhost kernel: [   32.462922] RAX:  RBX: 
880614bef640 RCX: 00ff
Oct  6 20:11:18 localhost kernel: [   32.462959] RDX:  RSI: 
88062e70c098 RDI: 880614bef640
Oct  6 20:11:18 localhost kernel: [   32.462998] RBP: 8805e1ec7ca8 R08: 
880614befc40 R09: 
Oct  6 20:11:18 localhost kernel: [   32.463033] R10:  R11: 
81a58df8 R12: 880614befc40
Oct  6 20:11:18 localhost kernel: [   32.463071] R13: 8806144b9858 R14: 
0082 R15: 88062e70c098
Oct  6 20:11:18 localhost kernel: [   32.463110] FS:  7f27cc824b80() 
GS:88062ec0() knlGS:
Oct  6 20:11:18 localhost kernel: [   32.463148] CS:  0010 DS:  ES:  
CR0: 80050033
Oct  6 20:11:18 localhost kernel: [   32.463177] CR2: 00b8 CR3: 
c8011000 CR4: 000406f0
Oct  6 20:11:18 localhost kernel: [   32.463211] Stack:
Oct  6 20:11:18 localhost kernel: [   32.463223]  880614bef640 
8805e1ec7cd8 8147a96c 880614befc40
Oct  6 20:11:18 localhost kernel: [   32.463268]  88062e70c098 
0286 8806144b9800 8805e1ec7d08
Oct  6 20:11:18 localhost kernel: [   32.463310]  8147aaf5 
880615dd0bc0 8805e8354a00 880614befc40
Oct  6 20:11:18 localhost kernel: [   32.463355] Call

Re: [PATCH] iommu/arm-smmu: Only return IRQ_NONE if FSR is not set

2015-10-06 Thread Mitchel Humpherys 
On Mon, Oct 05 2015 at 03:24:03 PM, Will Deacon  wrote:
> Hi Mitch,
>
> On Sat, Sep 26, 2015 at 01:12:05AM +0100, Mitchel Humpherys wrote:
>> Currently we return IRQ_NONE from the context fault handler if the FSR
>> doesn't actually have the fault bit set (some sort of miswired
>> interrupt?) or if the client doesn't register an IOMMU fault handler.
>> However, registering a client fault handler is optional, so telling the
>> interrupt framework that the interrupt wasn't for this device if the
>> client doesn't register a handler isn't exactly accurate.  Fix this by
>> returning IRQ_HANDLED even if the client doesn't register a handler.
>> 
>> Signed-off-by: Mitchel Humpherys 
>> ---
>>  drivers/iommu/arm-smmu.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index 48a39dfa9777..95560d447a54 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -653,7 +653,7 @@ static irqreturn_t arm_smmu_context_fault(int irq, void 
>> *dev)
>>  dev_err_ratelimited(smmu->dev,
>>  "Unhandled context fault: iova=0x%08lx, fsynr=0x%x, 
>> cb=%d\n",
>>  iova, fsynr, cfg->cbndx);
>> -ret = IRQ_NONE;
>> +ret = IRQ_HANDLED;
>>  resume = RESUME_TERMINATE;
>
> Hmm, but if we haven't actually done anything to rectify the cause of the
> fault, what means that we won't take it again immediately? I guess I'm not
> understanding the use-case that triggered you to write this patch...

Does returning IRQ_NONE actually prevent us from taking another
interrupt (despite clearing the FSR below)?  We definitely take more
interrupts on our platform despite returning IRQ_NONE, but maybe we have
something misconfigured...

I thought that returning IRQ_NONE would simply affect spurious interrupt
accounting (only disabling the interrupt if we took enough of them in
close enough succession to flag a misbehaving device).

As far as a valid use case, I can't think of one.  I just thought we
were messing up the spurious interrupt accounting.


-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH char-misc-next v2 04/22] iommu: Allow iova to be used without requiring IOMMU_SUPPORT

2015-10-06 Thread gre...@linuxfoundation.org 
On Mon, Oct 05, 2015 at 10:23:38PM -0700, Sudeep Dutt wrote:
> On Tue, 2015-10-06 at 06:20 +0100, gre...@linuxfoundation.org wrote:
> > On Tue, Oct 06, 2015 at 06:12:40AM +0100, gre...@linuxfoundation.org wrote:
> > > On Mon, Oct 05, 2015 at 10:38:43AM -0700, Sudeep Dutt wrote:
> > > > On Mon, 2015-10-05 at 03:50 -0700, Woodhouse, David wrote:
> > > > > On Tue, 2015-09-29 at 18:09 -0700, Ashutosh Dixit wrote:
> > > > > > From: Sudeep Dutt 
> > > > > > 
> > > > > > iova is a library which can be built without IOMMU_SUPPORT
> > > > > > 
> > > > > > Signed-off-by: Sudeep Dutt 
> > > > > 
> > > > > The first three of these patches are in 4.3-rc4 already. Apologies for
> > > > > the delay in pushing them out.
> > > > > 
> > > > > This one looks sane enough too, but perhaps in that case we should 
> > > > > move
> > > > > the code *out* of drivers/iommu/ and into lib/iova/ ?
> > > > > 
> > > > 
> > > > Yes, moving the code into lib/iova is the correct long term solution. I
> > > > have sent Greg a patch which reverts this commit since it is no longer
> > > > required and will create a merge conflict for him unnecessarily as well
> > > > with 4.3-rc4.
> > > 
> > > I can handle merge issues, that's trivial.  Reverting the patch
> > > shoulnd't really be needed, right?  Let me see what happens when I merge
> > > to see if your patch is necessary...
> > 
> > Ok, I don't think it is needed, the merge was pretty trivial.
> > 
> > Can you test out my char-misc-testing branch right now to see if it's
> > all ok with the merge?  If so, I'll move it all over to the "real" place
> > for it to start showing up in linux-next, i.e. my char-misc-next branch.
> > 
> 
> Hi Greg,
> 
> I think it is best to revert this patch as it is incorrect. The iommu
> folder gets compiled only if IOMMU_SUPPORT is enabled so IOMMU_IOVA
> should indeed be included only when IOMMU_SUPPORT is enabled.
> 
> Sincere apologies for the mess here but I believe it will all get fixed
> up if you accept the revert of 353649e5da I sent across earlier today.

Again, look at the merge, I think I already handled this in that manner.
If not, let me know.

thanks,

greg k-h
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH char-misc-next v2 04/22] iommu: Allow iova to be used without requiring IOMMU_SUPPORT

2015-10-06 Thread Sudeep Dutt 
On Tue, 2015-10-06 at 08:56 +0100, gre...@linuxfoundation.org wrote:
> On Mon, Oct 05, 2015 at 10:23:38PM -0700, Sudeep Dutt wrote:
> > On Tue, 2015-10-06 at 06:20 +0100, gre...@linuxfoundation.org wrote:
> > > On Tue, Oct 06, 2015 at 06:12:40AM +0100, gre...@linuxfoundation.org 
> > > wrote:
> > > > On Mon, Oct 05, 2015 at 10:38:43AM -0700, Sudeep Dutt wrote:
> > > > > On Mon, 2015-10-05 at 03:50 -0700, Woodhouse, David wrote:
> > > > > > On Tue, 2015-09-29 at 18:09 -0700, Ashutosh Dixit wrote:
> > > > > > > From: Sudeep Dutt 
> > > > > > > 
> > > > > > > iova is a library which can be built without IOMMU_SUPPORT
> > > > > > > 
> > > > > > > Signed-off-by: Sudeep Dutt 
> > > > > > 
> > > > > > The first three of these patches are in 4.3-rc4 already. Apologies 
> > > > > > for
> > > > > > the delay in pushing them out.
> > > > > > 
> > > > > > This one looks sane enough too, but perhaps in that case we should 
> > > > > > move
> > > > > > the code *out* of drivers/iommu/ and into lib/iova/ ?
> > > > > > 
> > > > > 
> > > > > Yes, moving the code into lib/iova is the correct long term solution. 
> > > > > I
> > > > > have sent Greg a patch which reverts this commit since it is no longer
> > > > > required and will create a merge conflict for him unnecessarily as 
> > > > > well
> > > > > with 4.3-rc4.
> > > > 
> > > > I can handle merge issues, that's trivial.  Reverting the patch
> > > > shoulnd't really be needed, right?  Let me see what happens when I merge
> > > > to see if your patch is necessary...
> > > 
> > > Ok, I don't think it is needed, the merge was pretty trivial.
> > > 
> > > Can you test out my char-misc-testing branch right now to see if it's
> > > all ok with the merge?  If so, I'll move it all over to the "real" place
> > > for it to start showing up in linux-next, i.e. my char-misc-next branch.
> > > 
> > 
> > Hi Greg,
> > 
> > I think it is best to revert this patch as it is incorrect. The iommu
> > folder gets compiled only if IOMMU_SUPPORT is enabled so IOMMU_IOVA
> > should indeed be included only when IOMMU_SUPPORT is enabled.
> > 
> > Sincere apologies for the mess here but I believe it will all get fixed
> > up if you accept the revert of 353649e5da I sent across earlier today.
> 
> Again, look at the merge, I think I already handled this in that manner.
> If not, let me know.
> 

Hi Greg,

I took a look at your latest char-misc-testing tree and it needs to be
fixed up. IOMMU_IOVA should be inside the "if IOMMU_SUPPORT" block
instead of above it.

git revert 353649e5da in the char-misc-testing tree will fix everything
up or you could also apply the patch I sent earlier today which has the
same revert.

Thanks,
Sudeep Dutt

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH char-misc-next v2 04/22] iommu: Allow iova to be used without requiring IOMMU_SUPPORT

2015-10-06 Thread gre...@linuxfoundation.org 
On Tue, Oct 06, 2015 at 01:05:27AM -0700, Sudeep Dutt wrote:
> On Tue, 2015-10-06 at 08:56 +0100, gre...@linuxfoundation.org wrote:
> > On Mon, Oct 05, 2015 at 10:23:38PM -0700, Sudeep Dutt wrote:
> > > On Tue, 2015-10-06 at 06:20 +0100, gre...@linuxfoundation.org wrote:
> > > > On Tue, Oct 06, 2015 at 06:12:40AM +0100, gre...@linuxfoundation.org 
> > > > wrote:
> > > > > On Mon, Oct 05, 2015 at 10:38:43AM -0700, Sudeep Dutt wrote:
> > > > > > On Mon, 2015-10-05 at 03:50 -0700, Woodhouse, David wrote:
> > > > > > > On Tue, 2015-09-29 at 18:09 -0700, Ashutosh Dixit wrote:
> > > > > > > > From: Sudeep Dutt 
> > > > > > > > 
> > > > > > > > iova is a library which can be built without IOMMU_SUPPORT
> > > > > > > > 
> > > > > > > > Signed-off-by: Sudeep Dutt 
> > > > > > > 
> > > > > > > The first three of these patches are in 4.3-rc4 already. 
> > > > > > > Apologies for
> > > > > > > the delay in pushing them out.
> > > > > > > 
> > > > > > > This one looks sane enough too, but perhaps in that case we 
> > > > > > > should move
> > > > > > > the code *out* of drivers/iommu/ and into lib/iova/ ?
> > > > > > > 
> > > > > > 
> > > > > > Yes, moving the code into lib/iova is the correct long term 
> > > > > > solution. I
> > > > > > have sent Greg a patch which reverts this commit since it is no 
> > > > > > longer
> > > > > > required and will create a merge conflict for him unnecessarily as 
> > > > > > well
> > > > > > with 4.3-rc4.
> > > > > 
> > > > > I can handle merge issues, that's trivial.  Reverting the patch
> > > > > shoulnd't really be needed, right?  Let me see what happens when I 
> > > > > merge
> > > > > to see if your patch is necessary...
> > > > 
> > > > Ok, I don't think it is needed, the merge was pretty trivial.
> > > > 
> > > > Can you test out my char-misc-testing branch right now to see if it's
> > > > all ok with the merge?  If so, I'll move it all over to the "real" place
> > > > for it to start showing up in linux-next, i.e. my char-misc-next branch.
> > > > 
> > > 
> > > Hi Greg,
> > > 
> > > I think it is best to revert this patch as it is incorrect. The iommu
> > > folder gets compiled only if IOMMU_SUPPORT is enabled so IOMMU_IOVA
> > > should indeed be included only when IOMMU_SUPPORT is enabled.
> > > 
> > > Sincere apologies for the mess here but I believe it will all get fixed
> > > up if you accept the revert of 353649e5da I sent across earlier today.
> > 
> > Again, look at the merge, I think I already handled this in that manner.
> > If not, let me know.
> > 
> 
> Hi Greg,
> 
> I took a look at your latest char-misc-testing tree and it needs to be
> fixed up. IOMMU_IOVA should be inside the "if IOMMU_SUPPORT" block
> instead of above it.
> 
> git revert 353649e5da in the char-misc-testing tree will fix everything
> up or you could also apply the patch I sent earlier today which has the
> same revert.

Ok, I've applied your patch, rolled back the merge, and that should be
good, right?  If so, I'll push this branch out to char-misc-next and
then merge in 4.3-rc4 just to keep everything up to date.

thanks,

greg k-h
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [dm-devel] AMD-Vi IO_PAGE_FAULTs and ata3.00: failed command: READ FPDMA QUEUED errors since Linux 4.0

2015-10-06 Thread Joerg Roedel 
On Wed, Sep 30, 2015 at 04:52:47PM +0200, Andreas Hartmann wrote:
> > Alternativly someone who can reproduce it should trace the calls to
> > __map_single and __unmap_single in the AMD IOMMU driver to find out
> > whether the addresses which the faults happen on are really mapped, or
> > at least requested from the AMD IOMMU driver.
> 
> How can I trace it?

Please apply the attached debug patch on-top of Linux v4.3-rc3 and boot
the machine. After boot you run (as root):


# cat /sys/kernel/debug/tracing/trace_pipe > trace-data

Please run this in a seperate shell an keep it running.

Then trigger the problem while the above command is running. When you
triggered it, please send me the (compressed) trace-data file, full
dmesg and output of lspci on the box.

Please let me know if you have further questions.


Thanks,

Joerg

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index f82060e7..0002e79 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2465,6 +2465,7 @@ static dma_addr_t __map_single(struct device *dev,
 {
 	dma_addr_t offset = paddr & ~PAGE_MASK;
 	dma_addr_t address, start, ret;
+	phys_addr_t old_paddr = paddr;
 	unsigned int pages;
 	unsigned long align_mask = 0;
 	int i;
@@ -2521,6 +2522,8 @@ retry:
 		domain_flush_pages(_dom->domain, address, size);
 
 out:
+	trace_printk("%s: mapped %llx paddr %llx size %zu\n",
+			dev_name(dev), address, old_paddr, size);
 	return address;
 
 out_unmap:
@@ -2532,6 +2535,9 @@ out_unmap:
 
 	dma_ops_free_addresses(dma_dom, address, pages);
 
+	trace_printk("%s: return DMA_ERROR_CODE paddr %llx size %zu\n",
+			dev_name(dev), old_paddr, size);
+
 	return DMA_ERROR_CODE;
 }
 
@@ -2628,6 +2634,8 @@ static void unmap_page(struct device *dev, dma_addr_t dma_addr, size_t size,
 
 	spin_lock_irqsave(>lock, flags);
 
+	trace_printk("%s: unmap dma_addr %llx size %zu\n",
+			dev_name(dev), dma_addr, size);
 	__unmap_single(domain->priv, dma_addr, size, dir);
 
 	domain_flush_complete(domain);
@@ -2683,9 +2691,13 @@ out:
 	return mapped_elems;
 unmap:
 	for_each_sg(sglist, s, mapped_elems, i) {
-		if (s->dma_address)
+		if (s->dma_address) {
+			trace_printk("%s: unmap dma_addr %llx size %u\n",
+	dev_name(dev), s->dma_address,
+	s->dma_length);
 			__unmap_single(domain->priv, s->dma_address,
    s->dma_length, dir);
+		}
 		s->dma_address = s->dma_length = 0;
 	}
 
@@ -2716,6 +2728,9 @@ static void unmap_sg(struct device *dev, struct scatterlist *sglist,
 	spin_lock_irqsave(>lock, flags);
 
 	for_each_sg(sglist, s, nelems, i) {
+	trace_printk("%s: unmap dma_addr %llx size %u\n",
+			dev_name(dev), s->dma_address, s->dma_length);
+
 		__unmap_single(domain->priv, s->dma_address,
 			   s->dma_length, dir);
 		s->dma_address = s->dma_length = 0;
@@ -2813,6 +2828,9 @@ static void free_coherent(struct device *dev, size_t size,
 
 	spin_lock_irqsave(>lock, flags);
 
+	trace_printk("%s: unmap dma_addr %llx size %zu\n",
+			dev_name(dev), dma_addr, size);
+
 	__unmap_single(domain->priv, dma_addr, size, DMA_BIDIRECTIONAL);
 
 	domain_flush_complete(domain);
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu/s390: add iommu api for s390 pci devices

2015-10-06 Thread Joerg Roedel 
On Thu, Oct 01, 2015 at 07:30:28PM +0200, Gerald Schaefer wrote:
> Yes, the DMA API is already implemented in arch/s390/pci/pci_dma.c.
> I thought about moving it over to the new location in drivers/iommu/,
> but I don't see any benefit from it.

Okay, this is true for now. At some point we hopefully have a common
DMA-API implementation for all IOMMU driver, at which point s390 can
make use of it too and abandon its own implementation.

> Also, the two APIs are quite different on s390 and must not be mixed-up.
> For example, we have optimizations in the DMA API to reduce TLB flushes
> based on iommu bitmap wrap-around, which is not possible for the map/unmap
> logic in the IOMMU API. There is also the requirement that each device has
> its own DMA page table (not shared), which is important for DMA API device
> recovery and map/unmap on s390.

This sounds quite similar to what other IOMMU drivers also implement,
especially the AMD IOMMU driver. It also uses non-shared page-tables for
devices and implements the bitmap-allocator optimization.

> Hmm, not sure how this can replace my own struct. I need the struct to
> maintain a list of all devices that share a dma page table. And the
> devices need to be added and removed to/from that list in attach/detach_dev.
> 
> I also need that list during map/unmap, in order to do a TLB flush for
> all affected devices, and this happens under a spin lock.
> 
> So I guess I cannot use the iommu_group->devices list, which is managed
> in add/remove_device and under a mutex, if that was on your mind.

Yeah, right. Thanks for the explanation.


Joerg

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu/s390: add iommu api for s390 pci devices

2015-10-06 Thread Joerg Roedel 
On Thu, Aug 27, 2015 at 03:33:03PM +0200, Gerald Schaefer wrote:
> This adds an IOMMU API implementation for s390 PCI devices.
> 
> Reviewed-by: Sebastian Ott 
> Signed-off-by: Gerald Schaefer 
> ---
>  MAINTAINERS |   7 +
>  arch/s390/Kconfig   |   1 +
>  arch/s390/include/asm/pci.h |   4 +
>  arch/s390/include/asm/pci_dma.h |   5 +-
>  arch/s390/pci/pci_dma.c |  37 +++--
>  drivers/iommu/Kconfig   |   7 +
>  drivers/iommu/Makefile  |   1 +
>  drivers/iommu/s390-iommu.c  | 337 
> 
>  8 files changed, 386 insertions(+), 13 deletions(-)
>  create mode 100644 drivers/iommu/s390-iommu.c

Applied, thanks.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v6 2/3] arm64: Add IOMMU dma_ops

2015-10-06 Thread Yong Wu 
On Thu, 2015-10-01 at 20:13 +0100, Robin Murphy wrote:
> Taking some inspiration from the arch/arm code, implement the
> arch-specific side of the DMA mapping ops using the new IOMMU-DMA layer.
> 
> Since there is still work to do elsewhere to make DMA configuration happen
> in a more appropriate order and properly support platform devices in the
> IOMMU core, the device setup code unfortunately starts out carrying some
> workarounds to ensure it works correctly in the current state of things.
> 
> Signed-off-by: Robin Murphy 
> ---
>  arch/arm64/mm/dma-mapping.c | 435 
> 
>  1 file changed, 435 insertions(+)
> 
[...]
> +/*
> + * TODO: Right now __iommu_setup_dma_ops() gets called too early to do
> + * everything it needs to - the device is only partially created and the
> + * IOMMU driver hasn't seen it yet, so it can't have a group. Thus we
> + * need this delayed attachment dance. Once IOMMU probe ordering is sorted
> + * to move the arch_setup_dma_ops() call later, all the notifier bits below
> + * become unnecessary, and will go away.
> + */

Hi Robin,
  Could I ask a question about the plan in the future:
  How to move arch_setup_dma_ops() call later than IOMMU probe?
  
  arch_setup_dma_ops is from of_dma_configure which is from
arm64_device_init, and IOMMU probe is subsys_init. So
arch_setup_dma_ops will run before IOMMU probe normally, is it right?
  Does Laurent's probe-deferral series could help do this? what's
the state of this series.

> +struct iommu_dma_notifier_data {
> + struct list_head list;
> + struct device *dev;
> + const struct iommu_ops *ops;
> + u64 dma_base;
> + u64 size;
> +};
> +static LIST_HEAD(iommu_dma_masters);
> +static DEFINE_MUTEX(iommu_dma_notifier_lock);
> +
> +/*
> + * Temporarily "borrow" a domain feature flag to to tell if we had to resort
> + * to creating our own domain here, in case we need to clean it up again.
> + */
> +#define __IOMMU_DOMAIN_FAKE_DEFAULT  (1U << 31)
> +
> +static bool do_iommu_attach(struct device *dev, const struct iommu_ops *ops,
> +u64 dma_base, u64 size)
> +{
> + struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
> +
> + /*
> +  * Best case: The device is either part of a group which was
> +  * already attached to a domain in a previous call, or it's
> +  * been put in a default DMA domain by the IOMMU core.
> +  */
> + if (!domain) {
> + /*
> +  * Urgh. The IOMMU core isn't going to do default domains
> +  * for non-PCI devices anyway, until it has some means of
> +  * abstracting the entirely implementation-specific
> +  * sideband data/SoC topology/unicorn dust that may or
> +  * may not differentiate upstream masters.
> +  * So until then, HORRIBLE HACKS!
> +  */
> + domain = ops->domain_alloc(IOMMU_DOMAIN_DMA);
> + if (!domain)
> + goto out_no_domain;
> +
> + domain->ops = ops;
> + domain->type = IOMMU_DOMAIN_DMA | __IOMMU_DOMAIN_FAKE_DEFAULT;
> +
> + if (iommu_attach_device(domain, dev))
> + goto out_put_domain;
> + }
> +
> + if (iommu_dma_init_domain(domain, dma_base, size))
> + goto out_detach;
> +
> + dev->archdata.dma_ops = _dma_ops;
> + return true;
> +
> +out_detach:
> + iommu_detach_device(domain, dev);
> +out_put_domain:
> + if (domain->type & __IOMMU_DOMAIN_FAKE_DEFAULT)
> + iommu_domain_free(domain);
> +out_no_domain:
> + pr_warn("Failed to set up IOMMU for device %s; retaining platform DMA 
> ops\n",
> + dev_name(dev));
> + return false;
> +}
[...]
> +static void __iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
> +   const struct iommu_ops *ops)
> +{
> + struct iommu_group *group;
> +
> + if (!ops)
> + return;
> + /*
> +  * TODO: As a concession to the future, we're ready to handle being
> +  * called both early and late (i.e. after bus_add_device). Once all
> +  * the platform bus code is reworked to call us late and the notifier
> +  * junk above goes away, move the body of do_iommu_attach here.
> +  */
> + group = iommu_group_get(dev);

   If iommu_setup_dma_ops run after bus_add_device, then the device has
its group here. It will enter do_iommu_attach which will alloc a default
iommu domain and attach this device to the new iommu domain.
   But mtk-iommu don't expect like this, we would like to attach to the
same domain. So we should alloc a default iommu domain(if there is no
iommu domain at that time) and attach the device to the same domain in
our xx_add_device, is it right?

> + if (group) {
> + do_iommu_attach(dev, ops, dma_base, size);
> + iommu_group_put(group);

Re: [PATCH char-misc-next v2 04/22] iommu: Allow iova to be used without requiring IOMMU_SUPPORT

2015-10-06 Thread Sudeep Dutt 
On Tue, 2015-10-06 at 09:33 +0100, gre...@linuxfoundation.org wrote:
> On Tue, Oct 06, 2015 at 01:05:27AM -0700, Sudeep Dutt wrote:
> > On Tue, 2015-10-06 at 08:56 +0100, gre...@linuxfoundation.org wrote:
> > > On Mon, Oct 05, 2015 at 10:23:38PM -0700, Sudeep Dutt wrote:
> > > > On Tue, 2015-10-06 at 06:20 +0100, gre...@linuxfoundation.org wrote:
> > > > > On Tue, Oct 06, 2015 at 06:12:40AM +0100, gre...@linuxfoundation.org 
> > > > > wrote:
> > > > > > On Mon, Oct 05, 2015 at 10:38:43AM -0700, Sudeep Dutt wrote:
> > > > > > > On Mon, 2015-10-05 at 03:50 -0700, Woodhouse, David wrote:
> > > > > > > > On Tue, 2015-09-29 at 18:09 -0700, Ashutosh Dixit wrote:
> > > > > > > > > From: Sudeep Dutt 
> > > > > > > > > 
> > > > > > > > > iova is a library which can be built without IOMMU_SUPPORT
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Sudeep Dutt 
> > > > > > > > 
> > > > > > > > The first three of these patches are in 4.3-rc4 already. 
> > > > > > > > Apologies for
> > > > > > > > the delay in pushing them out.
> > > > > > > > 
> > > > > > > > This one looks sane enough too, but perhaps in that case we 
> > > > > > > > should move
> > > > > > > > the code *out* of drivers/iommu/ and into lib/iova/ ?
> > > > > > > > 
> > > > > > > 
> > > > > > > Yes, moving the code into lib/iova is the correct long term 
> > > > > > > solution. I
> > > > > > > have sent Greg a patch which reverts this commit since it is no 
> > > > > > > longer
> > > > > > > required and will create a merge conflict for him unnecessarily 
> > > > > > > as well
> > > > > > > d3bd-0010
> > > > > > 
> > > > > > I can handle merge issues, that's trivial.  Reverting the patch
> > > > > > shoulnd't really be needed, right?  Let me see what happens when I 
> > > > > > merge
> > > > > > to see if your patch is necessary...
> > > > > 
> > > > > Ok, I don't think it is needed, the merge was pretty trivial.
> > > > > 
> > > > > Can you test out my char-misc-testing branch right now to see if it's
> > > > > all ok with the merge?  If so, I'll move it all over to the "real" 
> > > > > place
> > > > > for it to start showing up in linux-next, i.e. my char-misc-next 
> > > > > branch.
> > > > > 
> > > > 
> > > > Hi Greg,
> > > > 
> > > > I think it is best to revert this patch as it is incorrect. The iommu
> > > > folder gets compiled only if IOMMU_SUPPORT is enabled so IOMMU_IOVA
> > > > should indeed be included only when IOMMU_SUPPORT is enabled.
> > > > 
> > > > Sincere apologies for the mess here but I believe it will all get fixed
> > > > up if you accept the revert of 353649e5da I sent across earlier today.
> > > 
> > > Again, look at the merge, I think I already handled this in that manner.
> > > If not, let me know.
> > > 
> > 
> > Hi Greg,
> > 
> > I took a look at your latest char-misc-testing tree and it needs to be
> > fixed up. IOMMU_IOVA should be inside the "if IOMMU_SUPPORT" block
> > instead of above it.
> > 
> > git revert 353649e5da in the char-misc-testing tree will fix everything
> > up or you could also apply the patch I sent earlier today which has the
> > same revert.
> 
> Ok, I've applied your patch, rolled back the merge, and that should be
> good, right?  If so, I'll push this branch out to char-misc-next and
> then merge in 4.3-rc4 just to keep everything up to date.

Yes, it looks good now.

Thanks,
Sudeep Dutt

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu