[PATCH] powerpc/pseries/iommu: switch to Default DMA window during kdump

[PATCH] powerpc/pseries/iommu: switch to Default DMA window during kdump

[Gaurav Batra]

In PowerPC (pseries) a non-virtualized adapter will have 2 DMA windows -
2GB default and a larger Dynamic DMA Window (DDW). DDW is large enough to
map total RAM to a device.

During normal functioning of OS, since RAM is pre-mapped, 2GB default
window is not used. The only scenario it might get used is when buffers in
pmemory are mapped to the device for DMA.

As of today, during kdump, during early device discovery, pci_dma_find()
finds that the device has 2 DMA windows. It selects to use DDW. This is a
kdump path and DMA window is needed for IO to the device.

Since, in the previous life of the LPAR (before panic), RAM was pre-mapped
via DDW, the DDW is completely full. So, in the iommu table initialization
code, iommu_table_clear() frees KDUMP_MIN_TCE_ENTRIES (2K) number of
TCEs.

But, it seems these are not enough for NVMe over Fibre-channel. When kdump
is trying to save vmcore on storage device, which is NVMe-FC, the
TCE usage is much more than 2K number of entries. The driver is mapping
a lot more buffers for DMA. After all the TCEs are consumed, iommu returns
iommu_alloc failures and the driver is not able to further map buffers for
IO. kdump fails to copy vmcore to NVMe-FC storage device.

Here are the driver logs and stack

lpfc 0153:70:00.0: iommu_alloc failed,
tbl 0000000034ebcf5e vaddr 00000000d814df0b npages 1
lpfc 0153:70:00.0: FCP Op failed - cmdiu dma mapping failed.
lpfc 0153:70:00.0: iommu_alloc failed,
tbl 0000000034ebcf5e vaddr 000000009779e4d2 npages 1
lpfc 0153:70:00.0: FCP Op failed - cmdiu dma mapping failed.

iommu_map_phys+0x1c4/0x1f0 (unreliable)
dma_iommu_map_phys+0x54/0xa0
dma_map_phys+0x3f8/0x590
__nvme_fc_init_request+0x110/0x300 [nvme_fc]
nvme_fc_init_request+0x60/0xb8 [nvme_fc]
blk_mq_alloc_map_and_rqs+0x388/0x510
blk_mq_alloc_tag_set+0x2a4/0x5f0
nvme_alloc_io_tag_set+0xe0/0x1e0 [nvme_core]
nvme_fc_connect_ctrl_work+0x85c/0xdac [nvme_fc]
process_one_work+0x1e4/0x5a0
worker_thread+0x1ec/0x3e0

Increasing the number of free TCE entries in iommu_table_clear() will
increase the probability of hitting EEH since there could still be some
active IOs from the previous life of the kernel.

Instead, during kdump, we can switch to default 2GB DMA window. This window
will mostly be empty. Or, could be slightly used if buffers in pmemory
were mapped for IO.

Fixes: 09a3c1e46142 ("powerpc/pseries/iommu: IOMMU table is not initialized for kdump over SR-IOV")
Signed-off-by: Gaurav Batra <gbatra@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/iommu.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 3e1f915fe4f6..272e9aab66d4 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -812,18 +812,11 @@ static struct device_node *pci_dma_find(struct device_node *dn,
 
 	/* parse DMA window property. During normal system boot, only default
 	 * DMA window is passed in OF. But, for kdump, a dedicated adapter might
-	 * have both default and DDW in FDT. In this scenario, DDW takes precedence
-	 * over default window.
+	 * have both default and DDW in FDT. In this scenario, default window
+	 * takes precedence over DDW. For a dedicated adapter, default window will
+	 * potentially have more unused TCEs.
 	 */
-	if (ddw_win) {
-		struct dynamic_dma_window_prop *p;
-
-		p = (struct dynamic_dma_window_prop *)ddw_prop;
-		prop->liobn = p->liobn;
-		prop->dma_base = p->dma_base;
-		prop->tce_shift = p->tce_shift;
-		prop->window_shift = p->window_shift;
-	} else if (default_win) {
+	if (default_win) {
 		unsigned long offset, size, liobn;
 
 		of_parse_dma_window(rdn, default_prop, &liobn, &offset, &size);
@@ -832,6 +825,14 @@ static struct device_node *pci_dma_find(struct device_node *dn,
 		prop->dma_base = cpu_to_be64(offset);
 		prop->tce_shift = cpu_to_be32(IOMMU_PAGE_SHIFT_4K);
 		prop->window_shift = cpu_to_be32(order_base_2(size));
+	} else {
+		struct dynamic_dma_window_prop *p;
+
+		p = (struct dynamic_dma_window_prop *)ddw_prop;
+		prop->liobn = p->liobn;
+		prop->dma_base = p->dma_base;
+		prop->tce_shift = p->tce_shift;
+		prop->window_shift = p->window_shift;
 	}
 
 	return rdn;

base-commit: 4549871118cf616eecdd2d939f78e3b9e1dddc48
-- 
2.39.3

Re: [PATCH] powerpc/pseries/iommu: switch to Default DMA window during kdump

[Ritesh Harjani]

Hi Gaurav,

Thanks for the patch. Few observation, inputs and queries -

Gaurav Batra <gbatra@linux.ibm.com> writes:

> In PowerPC (pseries) a non-virtualized adapter will have 2 DMA windows -
> 2GB default and a larger Dynamic DMA Window (DDW). DDW is large enough to
> map total RAM to a device.
>
> During normal functioning of OS, since RAM is pre-mapped, 2GB default
> window is not used. The only scenario it might get used is when buffers in
> pmemory are mapped to the device for DMA.
>
> As of today, during kdump, during early device discovery, pci_dma_find()
> finds that the device has 2 DMA windows. It selects to use DDW. This is a
> kdump path and DMA window is needed for IO to the device.
>
> Since, in the previous life of the LPAR (before panic), RAM was pre-mapped
> via DDW, the DDW is completely full. So, in the iommu table initialization
> code, iommu_table_clear() frees KDUMP_MIN_TCE_ENTRIES (2K) number of
> TCEs.
>
> But, it seems these are not enough for NVMe over Fibre-channel. When kdump
> is trying to save vmcore on storage device, which is NVMe-FC, the
> TCE usage is much more than 2K number of entries. The driver is mapping
> a lot more buffers for DMA. After all the TCEs are consumed, iommu returns
> iommu_alloc failures and the driver is not able to further map buffers for
> IO. kdump fails to copy vmcore to NVMe-FC storage device.

Some context I collected while reviewing this patch - 

1. kdump environment generally prefers configurations so that we could
   avoid issues like memory allocation failures. I guess, we don't want to
   be running the system with max configurations - that is also the reason
   why distros keep nr_cpus to a lower value during kdump case.
   For e.g. see this [1]:
        scsi: lpfc: Limit xri count for kdump environment

        scsi-mq operation inherently performs pre-allocation of resources for
        blk-mq request queues. Even though the kdump environment reduces the
        configuration to a single CPU, thus 1 hardware queue, which helps
        significantly, the resources are still rather large due to the per request
        allocations.
        <...>
   [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=31f06d2e73726160645f8d9976a0b3f42e136da7

2. While looking more into this call stack path I also saw -
            /*
            * If a crashdump is active, then we are potentially in a very
        -	 * memory constrained environment. Limit us to 1 queue and
        -	 * 64 tags to prevent using too much memory.
        +	 * memory constrained environment. Limit us to  64 tags to prevent
        +	 * using too much memory.
            */
        -	if (is_kdump_kernel()) {
        -		set->nr_hw_queues = 1;
        -		set->nr_maps = 1;
        +	if (is_kdump_kernel())
                set->queue_depth = min(64U, set->queue_depth);
        -	}
        +
        @@ -4515,7 +4513,7 @@ int blk_mq_alloc_tag_set(struct blk_mq_tag_set *set)
                                GFP_KERNEL, set->numa_node);
                if (!set->map[i].mq_map)
                    goto out_free_mq_map;
        -		set->map[i].nr_queues = is_kdump_kernel() ? 1 : set->nr_hw_queues;
        +		set->map[i].nr_queues = set->nr_hw_queues;
            }

            blk_mq_update_queue_map(set); 

   So looks like we already reduce blk-mq queue_depth to 64 in
   blk_mq_alloc_tag_set(), no matter what the queue_depth is passed to
   us by the driver.

3. Also with above patch from v6.9 onwards, we made nr_queues as
   set->nr_hw_queues, whereas earlier it was clamped to 1. Because the
   code assumes that in kdump kernel we boot with nr_cpus=1..
        blk-mq: don't change nr_hw_queues and nr_maps for kdump kernel

        For most of ARCHs, 'nr_cpus=1' is passed for kdump kernel, so
        nr_hw_queues for each mapping is supposed to be 1 already.

        More importantly, this way may cause trouble for driver, because blk-mq and
        driver see different queue mapping since driver should setup hardware
        queue setting before calling into allocating blk-mq tagset.

        So not overriding nr_hw_queues and nr_maps for kdump kernel.

   [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ec30b461f3d067bd322a6c3a6c5105746ed9bf14


So looking at above, I had few questions -
1. In your kdump kernel how many nr_cpus you are booting up with?
   Recently what I heard rhel/sles might be using nr_cpus=16/32. @Sourabh?
   So the calculation in nvme-fc driver then becomes:
     32(nr_cpus) * 64(queue_depth) * 2(cmd+resp) = 4096

  If you are able to reproduce this issue 100% of the time - then can
  you try kdump with nr_cpus=1 and see whether it fixes your iommu alloc
  failure?
2. Also were there more than 1 controller attached? Which can change the
  above calculation then.

Looking at the lpfc and nvme-fc driver - a lot of the calculation are
based on nr_online_cpus. I somehow think if we clamp that value of
nr_online_cpus, we should stop seeing these alloc failures.
Thoughts?

Hopefully, if you can work on some above points further, it will also
explain why are we seeing this failures only now. Is this something that
has caused an issue after RHEL/SLES moved to nr_cpus=16/32?
Or was it after this commit from v6.9?
   ec30b461f3d: ("blk-mq: don't change nr_hw_queues and nr_maps for kdump kernel")

>
> Here are the driver logs and stack
>
> lpfc 0153:70:00.0: iommu_alloc failed,
> tbl 0000000034ebcf5e vaddr 00000000d814df0b npages 1
> lpfc 0153:70:00.0: FCP Op failed - cmdiu dma mapping failed.
> lpfc 0153:70:00.0: iommu_alloc failed,
> tbl 0000000034ebcf5e vaddr 000000009779e4d2 npages 1
> lpfc 0153:70:00.0: FCP Op failed - cmdiu dma mapping failed.
>
> iommu_map_phys+0x1c4/0x1f0 (unreliable)
> dma_iommu_map_phys+0x54/0xa0
> dma_map_phys+0x3f8/0x590
> __nvme_fc_init_request+0x110/0x300 [nvme_fc]
> nvme_fc_init_request+0x60/0xb8 [nvme_fc]
> blk_mq_alloc_map_and_rqs+0x388/0x510
> blk_mq_alloc_tag_set+0x2a4/0x5f0
> nvme_alloc_io_tag_set+0xe0/0x1e0 [nvme_core]
> nvme_fc_connect_ctrl_work+0x85c/0xdac [nvme_fc]
> process_one_work+0x1e4/0x5a0
> worker_thread+0x1ec/0x3e0
>
> Increasing the number of free TCE entries in iommu_table_clear() will
> increase the probability of hitting EEH since there could still be some
> active IOs from the previous life of the kernel.
>
> Instead, during kdump, we can switch to default 2GB DMA window. This window
> will mostly be empty. Or, could be slightly used if buffers in pmemory
> were mapped for IO.
>

This will still remain a problem when we have SR-IOV adapter attached
correct? Because in that case we only get 1 window, so we anyway can't
use default window in kdump case. Correct?

-ritesh

Re: [PATCH] powerpc/pseries/iommu: switch to Default DMA window during kdump

[Gaurav Batra]

Hello Ritesh,

Thanks a lot for reviewing this patch. My responses are inline

On 7/30/26 10:09 AM, Ritesh Harjani (IBM) wrote:
> Hi Gaurav,
>
> Thanks for the patch. Few observation, inputs and queries -
>
> Gaurav Batra <gbatra@linux.ibm.com> writes:
>
>> In PowerPC (pseries) a non-virtualized adapter will have 2 DMA windows -
>> 2GB default and a larger Dynamic DMA Window (DDW). DDW is large enough to
>> map total RAM to a device.
>>
>> During normal functioning of OS, since RAM is pre-mapped, 2GB default
>> window is not used. The only scenario it might get used is when buffers in
>> pmemory are mapped to the device for DMA.
>>
>> As of today, during kdump, during early device discovery, pci_dma_find()
>> finds that the device has 2 DMA windows. It selects to use DDW. This is a
>> kdump path and DMA window is needed for IO to the device.
>>
>> Since, in the previous life of the LPAR (before panic), RAM was pre-mapped
>> via DDW, the DDW is completely full. So, in the iommu table initialization
>> code, iommu_table_clear() frees KDUMP_MIN_TCE_ENTRIES (2K) number of
>> TCEs.
>>
>> But, it seems these are not enough for NVMe over Fibre-channel. When kdump
>> is trying to save vmcore on storage device, which is NVMe-FC, the
>> TCE usage is much more than 2K number of entries. The driver is mapping
>> a lot more buffers for DMA. After all the TCEs are consumed, iommu returns
>> iommu_alloc failures and the driver is not able to further map buffers for
>> IO. kdump fails to copy vmcore to NVMe-FC storage device.
> Some context I collected while reviewing this patch -
>
> 1. kdump environment generally prefers configurations so that we could
>     avoid issues like memory allocation failures. I guess, we don't want to
>     be running the system with max configurations - that is also the reason
>     why distros keep nr_cpus to a lower value during kdump case.
>     For e.g. see this [1]:
>          scsi: lpfc: Limit xri count for kdump environment
>
>          scsi-mq operation inherently performs pre-allocation of resources for
>          blk-mq request queues. Even though the kdump environment reduces the
>          configuration to a single CPU, thus 1 hardware queue, which helps
>          significantly, the resources are still rather large due to the per request
>          allocations.
>          <...>
>     [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=31f06d2e73726160645f8d9976a0b3f42e136da7
>
> 2. While looking more into this call stack path I also saw -
>              /*
>              * If a crashdump is active, then we are potentially in a very
>          -	 * memory constrained environment. Limit us to 1 queue and
>          -	 * 64 tags to prevent using too much memory.
>          +	 * memory constrained environment. Limit us to  64 tags to prevent
>          +	 * using too much memory.
>              */
>          -	if (is_kdump_kernel()) {
>          -		set->nr_hw_queues = 1;
>          -		set->nr_maps = 1;
>          +	if (is_kdump_kernel())
>                  set->queue_depth = min(64U, set->queue_depth);
>          -	}
>          +
>          @@ -4515,7 +4513,7 @@ int blk_mq_alloc_tag_set(struct blk_mq_tag_set *set)
>                                  GFP_KERNEL, set->numa_node);
>                  if (!set->map[i].mq_map)
>                      goto out_free_mq_map;
>          -		set->map[i].nr_queues = is_kdump_kernel() ? 1 : set->nr_hw_queues;
>          +		set->map[i].nr_queues = set->nr_hw_queues;
>              }
>
>              blk_mq_update_queue_map(set);
>
>     So looks like we already reduce blk-mq queue_depth to 64 in
>     blk_mq_alloc_tag_set(), no matter what the queue_depth is passed to
>     us by the driver.
>
> 3. Also with above patch from v6.9 onwards, we made nr_queues as
>     set->nr_hw_queues, whereas earlier it was clamped to 1. Because the
>     code assumes that in kdump kernel we boot with nr_cpus=1..
>          blk-mq: don't change nr_hw_queues and nr_maps for kdump kernel
>
>          For most of ARCHs, 'nr_cpus=1' is passed for kdump kernel, so
>          nr_hw_queues for each mapping is supposed to be 1 already.
>
>          More importantly, this way may cause trouble for driver, because blk-mq and
>          driver see different queue mapping since driver should setup hardware
>          queue setting before calling into allocating blk-mq tagset.
>
>          So not overriding nr_hw_queues and nr_maps for kdump kernel.
>
>     [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ec30b461f3d067bd322a6c3a6c5105746ed9bf14

I agree with your assessment that kdump executes in a very limited 
environment

and drivers make an effort to use less resources. The goal is to dump 
memory and

not performance.

>
> So looking at above, I had few questions -
> 1. In your kdump kernel how many nr_cpus you are booting up with?
>     Recently what I heard rhel/sles might be using nr_cpus=16/32. @Sourabh?
>     So the calculation in nvme-fc driver then becomes:
>       32(nr_cpus) * 64(queue_depth) * 2(cmd+resp) = 4096
>
>    If you are able to reproduce this issue 100% of the time - then can
>    you try kdump with nr_cpus=1 and see whether it fixes your iommu alloc
>    failure?

Initially, when I reproduced this issue, I was not passing any value to

nr_cpus to the kdump kernel. I made changes to /etc/sysconfig/kdump to pass

nr_cpus=1. With this the kdump was successful. I tried with 
nr_cpus=16/32. These

were successful as well. Though, in these cases, I did notice a few 
iommu_alloc failures

(maybe < 10), vmcore was gathered successfully.

I started to see the issue with nr_cpus=64. My LPAR is configured with 
max cpus = 64. So,

earlier, when I was not specifying nr_cpus in the /etc/sysconfig/kdump, 
kdump could be

defaulting to 64 CPUs and hence allocating more resources during kdump.

> 2. Also were there more than 1 controller attached? Which can change the
>    above calculation then.

Only 1 controller. Here is the output of lscpi

ltcd41-lp11:~ # lspci
0153:70:00.0 Fibre Channel: Emulex Corporation LPe37000/LPe38000 Series 
32Gb/64Gb Fibre Channel Adapter (rev 10)
0153:70:00.1 Fibre Channel: Emulex Corporation LPe37000/LPe38000 Series 
32Gb/64Gb Fibre Channel Adapter (rev 10)
0153:70:00.2 Fibre Channel: Emulex Corporation LPe37000/LPe38000 Series 
32Gb/64Gb Fibre Channel Adapter (rev 10)
0153:70:00.3 Fibre Channel: Emulex Corporation LPe37000/LPe38000 Series 
32Gb/64Gb Fibre Channel Adapter (rev 10)

>
> Looking at the lpfc and nvme-fc driver - a lot of the calculation are
> based on nr_online_cpus. I somehow think if we clamp that value of
> nr_online_cpus, we should stop seeing these alloc failures.
> Thoughts?
>
> Hopefully, if you can work on some above points further, it will also
> explain why are we seeing this failures only now. Is this something that
> has caused an issue after RHEL/SLES moved to nr_cpus=16/32?

it seems this got exposed because in my test LPAR, nr_cpus=1/16/32 was not

getting passed to the kdump kernel.

> Or was it after this commit from v6.9?
>     ec30b461f3d: ("blk-mq: don't change nr_hw_queues and nr_maps for kdump kernel")
>
>> Here are the driver logs and stack
>>
>> lpfc 0153:70:00.0: iommu_alloc failed,
>> tbl 0000000034ebcf5e vaddr 00000000d814df0b npages 1
>> lpfc 0153:70:00.0: FCP Op failed - cmdiu dma mapping failed.
>> lpfc 0153:70:00.0: iommu_alloc failed,
>> tbl 0000000034ebcf5e vaddr 000000009779e4d2 npages 1
>> lpfc 0153:70:00.0: FCP Op failed - cmdiu dma mapping failed.
>>
>> iommu_map_phys+0x1c4/0x1f0 (unreliable)
>> dma_iommu_map_phys+0x54/0xa0
>> dma_map_phys+0x3f8/0x590
>> __nvme_fc_init_request+0x110/0x300 [nvme_fc]
>> nvme_fc_init_request+0x60/0xb8 [nvme_fc]
>> blk_mq_alloc_map_and_rqs+0x388/0x510
>> blk_mq_alloc_tag_set+0x2a4/0x5f0
>> nvme_alloc_io_tag_set+0xe0/0x1e0 [nvme_core]
>> nvme_fc_connect_ctrl_work+0x85c/0xdac [nvme_fc]
>> process_one_work+0x1e4/0x5a0
>> worker_thread+0x1ec/0x3e0
>>
>> Increasing the number of free TCE entries in iommu_table_clear() will
>> increase the probability of hitting EEH since there could still be some
>> active IOs from the previous life of the kernel.
>>
>> Instead, during kdump, we can switch to default 2GB DMA window. This window
>> will mostly be empty. Or, could be slightly used if buffers in pmemory
>> were mapped for IO.
>>
> This will still remain a problem when we have SR-IOV adapter attached
> correct? Because in that case we only get 1 window, so we anyway can't
> use default window in kdump case. Correct?

you are right. The patch is fixing the dedicated adapter path only by 
switching to

default window for kdump. Before I submitted the patch, I did try SR-IOV 
path as well.

Here, I assigned a virtualized adapter to LPAR and gathered kdump over 
NFS. I checked the

footprint of DMA buffers in this path. They were not much. I think, I 
did sent these details

in my emails (the discussion/advice).

As of now SR-IOV path doesn't seems to be of concern. But, I think, the 
correct overall fix

should be to maintain the DDW state --> if it is pre-mapped DDW, 
transfer this knowledge to kdump.

With this, the DDW will be intact and buffers pre-mapped, as before. 
But, this requires more work

and thorough testing by FVT/ISST. So, I kept this for later. For now, 
switching to default window

seems to me the least invasive fix for this very narrow problem.

Your insight and thoughts?


Thanks a lot

Gaurav

>
> -ritesh
>

Re: [PATCH] powerpc/pseries/iommu: switch to Default DMA window during kdump

[Ritesh Harjani]

Gaurav Batra <gbatra@linux.ibm.com> writes:

>>
>> So looking at above, I had few questions -
>> 1. In your kdump kernel how many nr_cpus you are booting up with?
>>     Recently what I heard rhel/sles might be using nr_cpus=16/32. @Sourabh?
>>     So the calculation in nvme-fc driver then becomes:
>>       32(nr_cpus) * 64(queue_depth) * 2(cmd+resp) = 4096
>>
>>    If you are able to reproduce this issue 100% of the time - then can
>>    you try kdump with nr_cpus=1 and see whether it fixes your iommu alloc
>>    failure?
>
> Initially, when I reproduced this issue, I was not passing any value to
>
> nr_cpus to the kdump kernel. I made changes to /etc/sysconfig/kdump to pass
>
> nr_cpus=1. With this the kdump was successful. I tried with 
> nr_cpus=16/32. These
>
> were successful as well. Though, in these cases, I did notice a few 
> iommu_alloc failures
>
> (maybe < 10), vmcore was gathered successfully.
>
> I started to see the issue with nr_cpus=64. My LPAR is configured with 
> max cpus = 64. So,
>
> earlier, when I was not specifying nr_cpus in the /etc/sysconfig/kdump, 
> kdump could be
>
> defaulting to 64 CPUs and hence allocating more resources during kdump.
>

Was this issue not reported via any system testing? Is this something
that you found on your own when you attach nvme-fc?
The reason I am interested in knowing that was - because the distro
default for rhel/sles will be 16/32 cpus, so was wondering if it was
repored with the default values as well and what were they?


>> 2. Also were there more than 1 controller attached? Which can change the
>>    above calculation then.
>
> Only 1 controller. Here is the output of lscpi
>
> ltcd41-lp11:~ # lspci
> 0153:70:00.0 Fibre Channel: Emulex Corporation LPe37000/LPe38000 Series 
> 32Gb/64Gb Fibre Channel Adapter (rev 10)
> 0153:70:00.1 Fibre Channel: Emulex Corporation LPe37000/LPe38000 Series 
> 32Gb/64Gb Fibre Channel Adapter (rev 10)
> 0153:70:00.2 Fibre Channel: Emulex Corporation LPe37000/LPe38000 Series 
> 32Gb/64Gb Fibre Channel Adapter (rev 10)
> 0153:70:00.3 Fibre Channel: Emulex Corporation LPe37000/LPe38000 Series 
> 32Gb/64Gb Fibre Channel Adapter (rev 10)
>
>>
>> Looking at the lpfc and nvme-fc driver - a lot of the calculation are
>> based on nr_online_cpus. I somehow think if we clamp that value of
>> nr_online_cpus, we should stop seeing these alloc failures.
>> Thoughts?
>>
>> Hopefully, if you can work on some above points further, it will also
>> explain why are we seeing this failures only now. Is this something that
>> has caused an issue after RHEL/SLES moved to nr_cpus=16/32?
>
> it seems this got exposed because in my test LPAR, nr_cpus=1/16/32 was not
>
> getting passed to the kdump kernel.
>
>> Or was it after this commit from v6.9?
>>     ec30b461f3d: ("blk-mq: don't change nr_hw_queues and nr_maps for kdump kernel")
>>
>>> Here are the driver logs and stack
>>>
>>> lpfc 0153:70:00.0: iommu_alloc failed,
>>> tbl 0000000034ebcf5e vaddr 00000000d814df0b npages 1
>>> lpfc 0153:70:00.0: FCP Op failed - cmdiu dma mapping failed.
>>> lpfc 0153:70:00.0: iommu_alloc failed,
>>> tbl 0000000034ebcf5e vaddr 000000009779e4d2 npages 1
>>> lpfc 0153:70:00.0: FCP Op failed - cmdiu dma mapping failed.
>>>
>>> iommu_map_phys+0x1c4/0x1f0 (unreliable)
>>> dma_iommu_map_phys+0x54/0xa0
>>> dma_map_phys+0x3f8/0x590
>>> __nvme_fc_init_request+0x110/0x300 [nvme_fc]
>>> nvme_fc_init_request+0x60/0xb8 [nvme_fc]
>>> blk_mq_alloc_map_and_rqs+0x388/0x510
>>> blk_mq_alloc_tag_set+0x2a4/0x5f0
>>> nvme_alloc_io_tag_set+0xe0/0x1e0 [nvme_core]
>>> nvme_fc_connect_ctrl_work+0x85c/0xdac [nvme_fc]
>>> process_one_work+0x1e4/0x5a0
>>> worker_thread+0x1ec/0x3e0
>>>
>>> Increasing the number of free TCE entries in iommu_table_clear() will
>>> increase the probability of hitting EEH since there could still be some
>>> active IOs from the previous life of the kernel.
>>>
>>> Instead, during kdump, we can switch to default 2GB DMA window. This window
>>> will mostly be empty. Or, could be slightly used if buffers in pmemory
>>> were mapped for IO.
>>>
>> This will still remain a problem when we have SR-IOV adapter attached
>> correct? Because in that case we only get 1 window, so we anyway can't
>> use default window in kdump case. Correct?
>
> you are right. The patch is fixing the dedicated adapter path only by 
> switching to
>
> default window for kdump. Before I submitted the patch, I did try SR-IOV 
> path as well.
>
> Here, I assigned a virtualized adapter to LPAR and gathered kdump over 
> NFS. I checked the
>
> footprint of DMA buffers in this path. They were not much. I think, I 
> did sent these details
>
> in my emails (the discussion/advice).
>
>
> seems to me the least invasive fix for this very narrow problem.

I went back and looked at the history of why do we use DDW window during
kdump instead of default window. It looks like this commit itself
changed the default to DDW instead of default window :) i.e.
  Fixes: 09a3c1e46142 ("powerpc/pseries/iommu: IOMMU table is not initialized for kdump over SR-IOV")

The commit msg is very detailed and explains a lof of things but somehow
it didn't explain on why did we change the default to always use ddw
window instead of using default window.

>
> Your insight and thoughts?
>

Somehow in the current commit msg we didn't mention that this commit is
just a partial revert of that previous fixes commit - i.e. we are
switching back to using 2GB default DMA window as the preferred window
for kdump case.

IMO, let's please also _add_ something like this to the existing commit
msg, so that it becomes clear for others.

<snip>
  (after this) ...kdump path and DMA window is needed for IO to the device.

Although this commit fixed an issue during kdump with SR-IOV case,
  09a3c1e46142 ("powerpc/pseries/iommu: IOMMU table is not initialized for kdump over SR-IOV")
but this also made the kdump prefer DDW over the default DMA window when
both are present (dedicated adapter case). Since the DDW is fully mapped
by the previous kernel, iommu_table_clear() can free only
KDUMP_MIN_TCE_ENTRIES (2048) TCEs for use by kdump kernel.

This is not enough when the dump device is NVMe over Fibre Channel.
Because nvme-fc driver DMA-maps the cmds and resp IUs of every
pre-allocated request and each such mapping consumes roughly:
       32(IO queues, one per cpus = nr_cpus) * 64(queue_depth, blk-mq kdump limit) * 2(cmd+resp) = 4096

This is already double of what we have without counting admin queues and
lpfc driver's own allocations / mapping requirement. Hence this results
into iommu_alloc failures like - 

lpfc 0153:70:00.0: iommu_alloc failed,
tbl 0000000034ebcf5e vaddr 00000000d814df0b npages 1
lpfc 0153:70:00.0: FCP Op failed - cmdiu dma mapping failed.
lpfc 0153:70:00.0: iommu_alloc failed,
tbl 0000000034ebcf5e vaddr 000000009779e4d2 npages 1
lpfc 0153:70:00.0: FCP Op failed - cmdiu dma mapping failed.

iommu_map_phys+0x1c4/0x1f0 (unreliable)
dma_iommu_map_phys+0x54/0xa0
dma_map_phys+0x3f8/0x590
__nvme_fc_init_request+0x110/0x300 [nvme_fc]
nvme_fc_init_request+0x60/0xb8 [nvme_fc]
blk_mq_alloc_map_and_rqs+0x388/0x510
blk_mq_alloc_tag_set+0x2a4/0x5f0
nvme_alloc_io_tag_set+0xe0/0x1e0 [nvme_core]
nvme_fc_connect_ctrl_work+0x85c/0xdac [nvme_fc]
process_one_work+0x1e4/0x5a0
worker_thread+0x1ec/0x3e0

Increasing the number of free TCE entries in iommu_table_clear() will
increase the probability of hitting EEH since there could still be some
active IOs from the previous life of the kernel.

Hence this patch partially reverts the previous fixes commit and
switches the kdump's default back to 2GB default DMA window instead of
DDW window. This window will mostly be empty. Or, could be slightly used
if buffers in pmemory were mapped for IO.

Fixes: 09a3c1e46142 ("powerpc/pseries/iommu: IOMMU table is not initialized for kdump over SR-IOV")
Cc: stable@vger.kernel.org

(before this) ...Signed-off-by:...
</snip>


With that added to the commit msg - please also feel free to add:
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>

>
> Thanks a lot
>
> Gaurav
>

Thanks again for adding detailed info. After looking at the history, I
agree we can partially revert the previous patch to prefer the default
DMA window for kdump case.

> As of now SR-IOV path doesn't seems to be of concern. But, I think, the 
> correct overall fix
>
> should be to maintain the DDW state --> if it is pre-mapped DDW, 
> transfer this knowledge to kdump.
>
> With this, the DDW will be intact and buffers pre-mapped, as before. 
> But, this requires more work
>
> and thorough testing by FVT/ISST. So, I kept this for later. For now, 
> switching to default window

I guess for kdump using 2GB default DMA window is not an issue, however
I agree that for kexec case we should find a way to fix this.
Because IIUC - kexec path on pseries is not performant today. It doesn't
use pre-mapped TCEs. So if someone is doing any I/O performance
measurement, we have to go via the full reboot cycle instead of just
using kexec.

-ritesh