Re: [PATCH kernel v3 0/3] powerpc/ioda2: Yet another attempt to allow DMA masks between 32 and 59

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 07/06/2019 11:41, Alistair Popple wrote:
> On Thursday, 6 June 2019 10:07:54 PM AEST Oliver wrote:
>> On Thu, Jun 6, 2019 at 5:17 PM Alistair Popple <alistair@popple.id.au> 
> wrote:
>>> I have been hitting EEH address errors testing this with some network
>>> cards which map/unmap DMA addresses more frequently. For example:
>>>
>>> PHB4 PHB#5 Diag-data (Version: 1)
>>> brdgCtl:    00000002
>>> RootSts:    00060020 00402000 a0220008 00100107 00000800
>>> PhbSts:     0000001c00000000 0000001c00000000
>>> Lem:        0000000100000080 0000000000000000 0000000000000080
>>> PhbErr:     0000028000000000 0000020000000000 2148000098000240
>>> a008400000000000 RxeTceErr:  2000000000000000 2000000000000000
>>> c000000000000000 0000000000000000 PblErr:     0000000000020000
>>> 0000000000020000 0000000000000000 0000000000000000 RegbErr:   
>>> 0000004000000000 0000004000000000 61000c4800000000 0000000000000000
>>> PE[000] A/B: 8300b03800000000 8000000000000000
>>>
>>> Interestingly the PE[000] A/B data is the same across different cards
>>> and drivers.
>>
>> TCE page fault due to permissions so odds are the DMA address was unmapped.
>>
>> What cards did you get this with? I tried with one of the common
>> BCM5719 NICs and generated network traffic by using rsync to copy a
>> linux git tree to the system and it worked fine.
> 
> Personally I've seen it with the BCM5719 with the driver modified to set a DMA 
> mask of 48 bits instead of 64 and using scp to copy a random 1GB file to the 
> system repeatedly until it crashes. 
> 
> I have also had reports of someone hitting the same error using a Mellanox 
> CX-5 adaptor with a similar driver modification.
> 
> - Alistair


Seems to be a race (I could see broken TCEs), this helps on my setup:


diff --git a/arch/powerpc/include/asm/iommu.h
b/arch/powerpc/include/asm/iommu.h
index f920697c03ef..215351e16ae8 100644
--- a/arch/powerpc/include/asm/iommu.h
+++ b/arch/powerpc/include/asm/iommu.h
@@ -113,6 +113,7 @@ struct iommu_table {
        int it_nid;
        unsigned long it_reserved_start; /* Start of not-DMA-able (MMIO)
area */
        unsigned long it_reserved_end;
+       spinlock_t lock;
 };

 #define IOMMU_TABLE_USERSPACE_ENTRY_RO(tbl, entry) \
diff --git a/arch/powerpc/platforms/powernv/pci-ioda-tce.c
b/arch/powerpc/platforms/powernv/pci-ioda-tce.c
index c75ec37bf0cd..7045b6518243 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda-tce.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda-tce.c
@@ -29,6 +29,7 @@ void pnv_pci_setup_iommu_table(struct iommu_table *tbl,
        tbl->it_size = tce_size >> 3;
        tbl->it_busno = 0;
        tbl->it_type = TCE_PCI;
+       spin_lock_init(&tbl->lock);
 }

 static __be64 *pnv_alloc_tce_level(int nid, unsigned int shift)
@@ -65,14 +66,17 @@ static __be64 *pnv_tce(struct iommu_table *tbl, bool
user, long idx, bool alloc)

                        if (!alloc)
                                return NULL;
-
-                       tmp2 = pnv_alloc_tce_level(tbl->it_nid,
-                                       ilog2(tbl->it_level_size) + 3);
-                       if (!tmp2)
-                               return NULL;
-
-                       tmp[n] = cpu_to_be64(__pa(tmp2) |
-                                       TCE_PCI_READ | TCE_PCI_WRITE);
+                       }
+                       spin_lock(&tbl->lock);
+                       if (tmp[n] == 0) {
+                               tmp2 = pnv_alloc_tce_level(tbl->it_nid,
+
ilog2(tbl->it_level_size) + 3);
+                               if (!tmp2)
+                                       return NULL;
+                               tmp[n] = cpu_to_be64(__pa(tmp2) |
+                                               TCE_PCI_READ |
TCE_PCI_WRITE);
+                       }
+                       spin_unlock(&tbl->lock);
                }
                tce = be64_to_cpu(tmp[n]);




-- 
Alexey