[PATCH 0/2] Fix NVMe driver support on Power with 32-bit DMA

[PATCH 0/2] Fix NVMe driver support on Power with 32-bit DMA

[Nishanth Aravamudan]

We received a bug report recently when DDW (64-bit direct DMA on Power)
is not enabled for NVMe devices. In that case, we fall back to 32-bit
DMA via the IOMMU, which is always done via 4K TCEs (Translation Control
Entries).

The NVMe device driver, though, assumes that the DMA alignment for the
PRP entries will match the device's page size, and that the DMA aligment
matches the kernel's page aligment. On Power, the the IOMMU page size,
as mentioned above, can be 4K, while the device can have a page size of
8K, while the kernel has a page size of 64K. This eventually trips the
BUG_ON in nvme_setup_prps(), as we have a 'dma_len' that is a multiple
of 4K but not 8K (e.g., 0xF000).

In this particular case, and generally, we want to use the IOMMU's page
size for the default device page size, rather than the kernel's page
size.

This series consists of two patches, one of which exposes the IOMMU's
page shift on Power (currently only the page size is exposed, and it
seems unnecessary to ilog2 that value in the driver). The second patch
leverages this value on Power in the NVMe driver.

With these patches, a NVMe device survives our internal hardware
exerciser; the kernel BUGs within a few seconds without the patch.

[PATCH 0/2] Fix NVMe driver support on Power with 32-bit DMA

[Nishanth Aravamudan]

We received a bug report recently when DDW (64-bit direct DMA on Power)
is not enabled for NVMe devices. In that case, we fall back to 32-bit
DMA via the IOMMU, which is always done via 4K TCEs (Translation Control
Entries).

The NVMe device driver, though, assumes that the DMA alignment for the
PRP entries will match the device's page size, and that the DMA aligment
matches the kernel's page aligment. On Power, the the IOMMU page size,
as mentioned above, can be 4K, while the device can have a page size of
8K, while the kernel has a page size of 64K. This eventually trips the
BUG_ON in nvme_setup_prps(), as we have a 'dma_len' that is a multiple
of 4K but not 8K (e.g., 0xF000).

In this particular case, and generally, we want to use the IOMMU's page
size for the default device page size, rather than the kernel's page
size.

This series consists of two patches, one of which exposes the IOMMU's
page shift on Power (currently only the page size is exposed, and it
seems unnecessary to ilog2 that value in the driver). The second patch
leverages this value on Power in the NVMe driver.

With these patches, a NVMe device survives our internal hardware
exerciser; the kernel BUGs within a few seconds without the patch.

[PATCH 1/2] powerpc/iommu: expose IOMMU page shift

[Nishanth Aravamudan]

We will leverage this macro in the NVMe driver, which needs to know the
configured IOMMU page shift to properly configure its device's page
size.

Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>

---
Given this is available, it seems reasonable to expose -- and it doesn't
really make sense to make the driver do a log2 call on the existing
IOMMU_PAGE_SIZE() value.

diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
index ca18cff..6fdf857 100644
--- a/arch/powerpc/include/asm/iommu.h
+++ b/arch/powerpc/include/asm/iommu.h
@@ -36,6 +36,7 @@
 #define IOMMU_PAGE_MASK_4K       (~((1 << IOMMU_PAGE_SHIFT_4K) - 1))
 #define IOMMU_PAGE_ALIGN_4K(addr) _ALIGN_UP(addr, IOMMU_PAGE_SIZE_4K)
 
+#define IOMMU_PAGE_SHIFT(tblptr) (tblptr)->it_page_shift
 #define IOMMU_PAGE_SIZE(tblptr) (ASM_CONST(1) << (tblptr)->it_page_shift)
 #define IOMMU_PAGE_MASK(tblptr) (~((1 << (tblptr)->it_page_shift) - 1))
 #define IOMMU_PAGE_ALIGN(addr, tblptr) _ALIGN_UP(addr, IOMMU_PAGE_SIZE(tblptr))

[PATCH 1/2] powerpc/iommu: expose IOMMU page shift

[Nishanth Aravamudan]

We will leverage this macro in the NVMe driver, which needs to know the
configured IOMMU page shift to properly configure its device's page
size.

Signed-off-by: Nishanth Aravamudan <nacc at linux.vnet.ibm.com>

---
Given this is available, it seems reasonable to expose -- and it doesn't
really make sense to make the driver do a log2 call on the existing
IOMMU_PAGE_SIZE() value.

diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
index ca18cff..6fdf857 100644
--- a/arch/powerpc/include/asm/iommu.h
+++ b/arch/powerpc/include/asm/iommu.h
@@ -36,6 +36,7 @@
 #define IOMMU_PAGE_MASK_4K       (~((1 << IOMMU_PAGE_SHIFT_4K) - 1))
 #define IOMMU_PAGE_ALIGN_4K(addr) _ALIGN_UP(addr, IOMMU_PAGE_SIZE_4K)
 
+#define IOMMU_PAGE_SHIFT(tblptr) (tblptr)->it_page_shift
 #define IOMMU_PAGE_SIZE(tblptr) (ASM_CONST(1) << (tblptr)->it_page_shift)
 #define IOMMU_PAGE_MASK(tblptr) (~((1 << (tblptr)->it_page_shift) - 1))
 #define IOMMU_PAGE_ALIGN(addr, tblptr) _ALIGN_UP(addr, IOMMU_PAGE_SIZE(tblptr))

[PATCH 2/2] drivers/nvme: default to the IOMMU page size on Power

[Nishanth Aravamudan]

We received a bug report recently when DDW (64-bit direct DMA on Power)
is not enabled for NVMe devices. In that case, we fall back to 32-bit
DMA via the IOMMU, which is always done via 4K TCEs (Translation Control
Entries).

The NVMe device driver, though, assumes that the DMA alignment for the
PRP entries will match the device's page size, and that the DMA aligment
matches the kernel's page aligment. On Power, the the IOMMU page size,
as mentioned above, can be 4K, while the device can have a page size of
8K, while the kernel has a page size of 64K. This eventually trips the
BUG_ON in nvme_setup_prps(), as we have a 'dma_len' that is a multiple
of 4K but not 8K (e.g., 0xF000).

In this particular case, and generally, we want to use the IOMMU's page
size for the default device page size, rather than the kernel's page
size.

With this patch, a NVMe device survives our internal hardware
exerciser; the kernel BUGs within a few seconds without the patch.

Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index 7920c27..969a95e 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -42,6 +42,7 @@
 #include <linux/types.h>
 #include <scsi/sg.h>
 #include <asm-generic/io-64-nonatomic-lo-hi.h>
+#include <asm/iommu.h>
 
 #define NVME_MINORS		(1U << MINORBITS)
 #define NVME_Q_DEPTH		1024
@@ -1680,6 +1681,11 @@ static int nvme_configure_admin_queue(struct nvme_dev *dev)
 	unsigned page_shift = PAGE_SHIFT;
 	unsigned dev_page_min = NVME_CAP_MPSMIN(cap) + 12;
 	unsigned dev_page_max = NVME_CAP_MPSMAX(cap) + 12;
+#ifdef CONFIG_PPC64
+	struct iommu_table *tbl = get_iommu_table_base(dev->dev);
+	if (tbl)
+		page_shift = IOMMU_PAGE_SHIFT(tbl);
+#endif
 
 	if (page_shift < dev_page_min) {
 		dev_err(dev->dev,

[PATCH 2/2] drivers/nvme: default to the IOMMU page size on Power

[Nishanth Aravamudan]

We received a bug report recently when DDW (64-bit direct DMA on Power)
is not enabled for NVMe devices. In that case, we fall back to 32-bit
DMA via the IOMMU, which is always done via 4K TCEs (Translation Control
Entries).

The NVMe device driver, though, assumes that the DMA alignment for the
PRP entries will match the device's page size, and that the DMA aligment
matches the kernel's page aligment. On Power, the the IOMMU page size,
as mentioned above, can be 4K, while the device can have a page size of
8K, while the kernel has a page size of 64K. This eventually trips the
BUG_ON in nvme_setup_prps(), as we have a 'dma_len' that is a multiple
of 4K but not 8K (e.g., 0xF000).

In this particular case, and generally, we want to use the IOMMU's page
size for the default device page size, rather than the kernel's page
size.

With this patch, a NVMe device survives our internal hardware
exerciser; the kernel BUGs within a few seconds without the patch.

Signed-off-by: Nishanth Aravamudan <nacc at linux.vnet.ibm.com>

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index 7920c27..969a95e 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -42,6 +42,7 @@
 #include <linux/types.h>
 #include <scsi/sg.h>
 #include <asm-generic/io-64-nonatomic-lo-hi.h>
+#include <asm/iommu.h>
 
 #define NVME_MINORS		(1U << MINORBITS)
 #define NVME_Q_DEPTH		1024
@@ -1680,6 +1681,11 @@ static int nvme_configure_admin_queue(struct nvme_dev *dev)
 	unsigned page_shift = PAGE_SHIFT;
 	unsigned dev_page_min = NVME_CAP_MPSMIN(cap) + 12;
 	unsigned dev_page_max = NVME_CAP_MPSMAX(cap) + 12;
+#ifdef CONFIG_PPC64
+	struct iommu_table *tbl = get_iommu_table_base(dev->dev);
+	if (tbl)
+		page_shift = IOMMU_PAGE_SHIFT(tbl);
+#endif
 
 	if (page_shift < dev_page_min) {
 		dev_err(dev->dev,

Re: [PATCH 2/2] drivers/nvme: default to the IOMMU page size on Power

[Christoph Hellwig]

Hi Nishanth,

please expose this value through the generic DMA API instead of adding
architecture specific hacks to drivers.

[PATCH 2/2] drivers/nvme: default to the IOMMU page size on Power

[Christoph Hellwig]

Hi Nishanth,

please expose this value through the generic DMA API instead of adding
architecture specific hacks to drivers.

Re: [PATCH 2/2] drivers/nvme: default to the IOMMU page size on Power

[Nishanth Aravamudan]

On 02.10.2015 [10:25:44 -0700], Christoph Hellwig wrote:
> Hi Nishanth,
> 
> please expose this value through the generic DMA API instead of adding
> architecture specific hacks to drivers.

Ok, I'm happy to do that instead -- what I struggled with is that I
don't have enough knowledge of the various architectures to provide the
right default implementation. It should be sufficient for the default to
return PAGE_SHIFT, and on Power just override that to return the IOMMU
table's page size? Since the only user will be the NVMe driver
currently, that should be fine?

Sorry for the less-than-ideal patch!

-Nish

[PATCH 2/2] drivers/nvme: default to the IOMMU page size on Power

[Nishanth Aravamudan]

On 02.10.2015 [10:25:44 -0700], Christoph Hellwig wrote:
> Hi Nishanth,
> 
> please expose this value through the generic DMA API instead of adding
> architecture specific hacks to drivers.

Ok, I'm happy to do that instead -- what I struggled with is that I
don't have enough knowledge of the various architectures to provide the
right default implementation. It should be sufficient for the default to
return PAGE_SHIFT, and on Power just override that to return the IOMMU
table's page size? Since the only user will be the NVMe driver
currently, that should be fine?

Sorry for the less-than-ideal patch!

-Nish

Re: [PATCH 2/2] drivers/nvme: default to the IOMMU page size on Power

[Christoph Hellwig]

On Fri, Oct 02, 2015 at 10:39:47AM -0700, Nishanth Aravamudan wrote:
> Ok, I'm happy to do that instead -- what I struggled with is that I
> don't have enough knowledge of the various architectures to provide the
> right default implementation. It should be sufficient for the default to
> return PAGE_SHIFT, and on Power just override that to return the IOMMU
> table's page size? Since the only user will be the NVMe driver
> currently, that should be fine?

I think that's fine.

> Sorry for the less-than-ideal patch!

Np, it's a reasonable first attempt that we just need to refine.

[PATCH 2/2] drivers/nvme: default to the IOMMU page size on Power

[Christoph Hellwig]

On Fri, Oct 02, 2015@10:39:47AM -0700, Nishanth Aravamudan wrote:
> Ok, I'm happy to do that instead -- what I struggled with is that I
> don't have enough knowledge of the various architectures to provide the
> right default implementation. It should be sufficient for the default to
> return PAGE_SHIFT, and on Power just override that to return the IOMMU
> table's page size? Since the only user will be the NVMe driver
> currently, that should be fine?

I think that's fine.

> Sorry for the less-than-ideal patch!

Np, it's a reasonable first attempt that we just need to refine.

Re: [PATCH 2/2] drivers/nvme: default to the IOMMU page size on Power

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1140 bytes --]

Hi Nishanth,

[auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]

config: arm64-allmodconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All error/warnings (new ones prefixed by >>):

>> drivers/block/nvme-core.c:45:23: fatal error: asm/iommu.h: No such file or directory
    #include <asm/iommu.h>
                          ^
   compilation terminated.

vim +45 drivers/block/nvme-core.c

    39	#include <linux/sched.h>
    40	#include <linux/slab.h>
    41	#include <linux/t10-pi.h>
    42	#include <linux/types.h>
    43	#include <scsi/sg.h>
    44	#include <asm-generic/io-64-nonatomic-lo-hi.h>
  > 45	#include <asm/iommu.h>
    46	
    47	#define NVME_MINORS		(1U << MINORBITS)
    48	#define NVME_Q_DEPTH		1024

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 45525 bytes --]

[PATCH 2/2] drivers/nvme: default to the IOMMU page size on Power

[kbuild test robot]

Hi Nishanth,

[auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]

config: arm64-allmodconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All error/warnings (new ones prefixed by >>):

>> drivers/block/nvme-core.c:45:23: fatal error: asm/iommu.h: No such file or directory
    #include <asm/iommu.h>
                          ^
   compilation terminated.

vim +45 drivers/block/nvme-core.c

    39	#include <linux/sched.h>
    40	#include <linux/slab.h>
    41	#include <linux/t10-pi.h>
    42	#include <linux/types.h>
    43	#include <scsi/sg.h>
    44	#include <asm-generic/io-64-nonatomic-lo-hi.h>
  > 45	#include <asm/iommu.h>
    46	
    47	#define NVME_MINORS		(1U << MINORBITS)
    48	#define NVME_Q_DEPTH		1024

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 45525 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-nvme/attachments/20151003/961f1921/attachment-0001.obj>

Re: [PATCH 1/2] powerpc/iommu: expose IOMMU page shift

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1467 bytes --]

On Fri, Oct 02, 2015 at 10:18:00AM -0700, Nishanth Aravamudan wrote:
> We will leverage this macro in the NVMe driver, which needs to know the
> configured IOMMU page shift to properly configure its device's page
> size.
> 
> Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>
> 
> ---
> Given this is available, it seems reasonable to expose -- and it doesn't
> really make sense to make the driver do a log2 call on the existing
> IOMMU_PAGE_SIZE() value.
> 
> diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
> index ca18cff..6fdf857 100644
> --- a/arch/powerpc/include/asm/iommu.h
> +++ b/arch/powerpc/include/asm/iommu.h
> @@ -36,6 +36,7 @@
>  #define IOMMU_PAGE_MASK_4K       (~((1 << IOMMU_PAGE_SHIFT_4K) - 1))
>  #define IOMMU_PAGE_ALIGN_4K(addr) _ALIGN_UP(addr, IOMMU_PAGE_SIZE_4K)
>  
> +#define IOMMU_PAGE_SHIFT(tblptr) (tblptr)->it_page_shift
>  #define IOMMU_PAGE_SIZE(tblptr) (ASM_CONST(1) << (tblptr)->it_page_shift)

Seems like it would be a touch safer to alter IOMMU_PAGE_SIZE so it
uses the new IOMMU_PAGE_SHIFT macro.

>  #define IOMMU_PAGE_MASK(tblptr) (~((1 << (tblptr)->it_page_shift) - 1))
>  #define IOMMU_PAGE_ALIGN(addr, tblptr) _ALIGN_UP(addr, IOMMU_PAGE_SIZE(tblptr))
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH 1/2] powerpc/iommu: expose IOMMU page shift

[David Gibson]

On Fri, Oct 02, 2015@10:18:00AM -0700, Nishanth Aravamudan wrote:
> We will leverage this macro in the NVMe driver, which needs to know the
> configured IOMMU page shift to properly configure its device's page
> size.
> 
> Signed-off-by: Nishanth Aravamudan <nacc at linux.vnet.ibm.com>
> 
> ---
> Given this is available, it seems reasonable to expose -- and it doesn't
> really make sense to make the driver do a log2 call on the existing
> IOMMU_PAGE_SIZE() value.
> 
> diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
> index ca18cff..6fdf857 100644
> --- a/arch/powerpc/include/asm/iommu.h
> +++ b/arch/powerpc/include/asm/iommu.h
> @@ -36,6 +36,7 @@
>  #define IOMMU_PAGE_MASK_4K       (~((1 << IOMMU_PAGE_SHIFT_4K) - 1))
>  #define IOMMU_PAGE_ALIGN_4K(addr) _ALIGN_UP(addr, IOMMU_PAGE_SIZE_4K)
>  
> +#define IOMMU_PAGE_SHIFT(tblptr) (tblptr)->it_page_shift
>  #define IOMMU_PAGE_SIZE(tblptr) (ASM_CONST(1) << (tblptr)->it_page_shift)

Seems like it would be a touch safer to alter IOMMU_PAGE_SIZE so it
uses the new IOMMU_PAGE_SHIFT macro.

>  #define IOMMU_PAGE_MASK(tblptr) (~((1 << (tblptr)->it_page_shift) - 1))
>  #define IOMMU_PAGE_ALIGN(addr, tblptr) _ALIGN_UP(addr, IOMMU_PAGE_SIZE(tblptr))
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-nvme/attachments/20151006/655bdd8a/attachment.sig>

Re: [PATCH 1/2] powerpc/iommu: expose IOMMU page shift

[Nishanth Aravamudan]

On 06.10.2015 [14:19:43 +1100], David Gibson wrote:
> On Fri, Oct 02, 2015 at 10:18:00AM -0700, Nishanth Aravamudan wrote:
> > We will leverage this macro in the NVMe driver, which needs to know the
> > configured IOMMU page shift to properly configure its device's page
> > size.
> > 
> > Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>
> > 
> > ---
> > Given this is available, it seems reasonable to expose -- and it doesn't
> > really make sense to make the driver do a log2 call on the existing
> > IOMMU_PAGE_SIZE() value.
> > 
> > diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
> > index ca18cff..6fdf857 100644
> > --- a/arch/powerpc/include/asm/iommu.h
> > +++ b/arch/powerpc/include/asm/iommu.h
> > @@ -36,6 +36,7 @@
> >  #define IOMMU_PAGE_MASK_4K       (~((1 << IOMMU_PAGE_SHIFT_4K) - 1))
> >  #define IOMMU_PAGE_ALIGN_4K(addr) _ALIGN_UP(addr, IOMMU_PAGE_SIZE_4K)
> >  
> > +#define IOMMU_PAGE_SHIFT(tblptr) (tblptr)->it_page_shift
> >  #define IOMMU_PAGE_SIZE(tblptr) (ASM_CONST(1) << (tblptr)->it_page_shift)
> 
> Seems like it would be a touch safer to alter IOMMU_PAGE_SIZE so it
> uses the new IOMMU_PAGE_SHIFT macro.

Yes absolutely! Sorry, I initially didn't add the first macro, so didn't
think that through. Will update.

-Nish

[PATCH 1/2] powerpc/iommu: expose IOMMU page shift

[Nishanth Aravamudan]

On 06.10.2015 [14:19:43 +1100], David Gibson wrote:
> On Fri, Oct 02, 2015@10:18:00AM -0700, Nishanth Aravamudan wrote:
> > We will leverage this macro in the NVMe driver, which needs to know the
> > configured IOMMU page shift to properly configure its device's page
> > size.
> > 
> > Signed-off-by: Nishanth Aravamudan <nacc at linux.vnet.ibm.com>
> > 
> > ---
> > Given this is available, it seems reasonable to expose -- and it doesn't
> > really make sense to make the driver do a log2 call on the existing
> > IOMMU_PAGE_SIZE() value.
> > 
> > diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
> > index ca18cff..6fdf857 100644
> > --- a/arch/powerpc/include/asm/iommu.h
> > +++ b/arch/powerpc/include/asm/iommu.h
> > @@ -36,6 +36,7 @@
> >  #define IOMMU_PAGE_MASK_4K       (~((1 << IOMMU_PAGE_SHIFT_4K) - 1))
> >  #define IOMMU_PAGE_ALIGN_4K(addr) _ALIGN_UP(addr, IOMMU_PAGE_SIZE_4K)
> >  
> > +#define IOMMU_PAGE_SHIFT(tblptr) (tblptr)->it_page_shift
> >  #define IOMMU_PAGE_SIZE(tblptr) (ASM_CONST(1) << (tblptr)->it_page_shift)
> 
> Seems like it would be a touch safer to alter IOMMU_PAGE_SIZE so it
> uses the new IOMMU_PAGE_SHIFT macro.

Yes absolutely! Sorry, I initially didn't add the first macro, so didn't
think that through. Will update.

-Nish

Re: [PATCH 1/2] powerpc/iommu: expose IOMMU page shift

[Nishanth Aravamudan]

On 12.10.2015 [09:03:52 -0700], Nishanth Aravamudan wrote:
> On 06.10.2015 [14:19:43 +1100], David Gibson wrote:
> > On Fri, Oct 02, 2015 at 10:18:00AM -0700, Nishanth Aravamudan wrote:
> > > We will leverage this macro in the NVMe driver, which needs to know the
> > > configured IOMMU page shift to properly configure its device's page
> > > size.
> > > 
> > > Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>
> > > 
> > > ---
> > > Given this is available, it seems reasonable to expose -- and it doesn't
> > > really make sense to make the driver do a log2 call on the existing
> > > IOMMU_PAGE_SIZE() value.
> > > 
> > > diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
> > > index ca18cff..6fdf857 100644
> > > --- a/arch/powerpc/include/asm/iommu.h
> > > +++ b/arch/powerpc/include/asm/iommu.h
> > > @@ -36,6 +36,7 @@
> > >  #define IOMMU_PAGE_MASK_4K       (~((1 << IOMMU_PAGE_SHIFT_4K) - 1))
> > >  #define IOMMU_PAGE_ALIGN_4K(addr) _ALIGN_UP(addr, IOMMU_PAGE_SIZE_4K)
> > >  
> > > +#define IOMMU_PAGE_SHIFT(tblptr) (tblptr)->it_page_shift
> > >  #define IOMMU_PAGE_SIZE(tblptr) (ASM_CONST(1) << (tblptr)->it_page_shift)
> > 
> > Seems like it would be a touch safer to alter IOMMU_PAGE_SIZE so it
> > uses the new IOMMU_PAGE_SHIFT macro.
> 
> Yes absolutely! Sorry, I initially didn't add the first macro, so didn't
> think that through. Will update.

Err, replied too quickly -- just got back from vacation -- \fthis is
from an old version of the patchset, we no longer introduce this macro.

-Nish

[PATCH 1/2] powerpc/iommu: expose IOMMU page shift

[Nishanth Aravamudan]

On 12.10.2015 [09:03:52 -0700], Nishanth Aravamudan wrote:
> On 06.10.2015 [14:19:43 +1100], David Gibson wrote:
> > On Fri, Oct 02, 2015@10:18:00AM -0700, Nishanth Aravamudan wrote:
> > > We will leverage this macro in the NVMe driver, which needs to know the
> > > configured IOMMU page shift to properly configure its device's page
> > > size.
> > > 
> > > Signed-off-by: Nishanth Aravamudan <nacc at linux.vnet.ibm.com>
> > > 
> > > ---
> > > Given this is available, it seems reasonable to expose -- and it doesn't
> > > really make sense to make the driver do a log2 call on the existing
> > > IOMMU_PAGE_SIZE() value.
> > > 
> > > diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
> > > index ca18cff..6fdf857 100644
> > > --- a/arch/powerpc/include/asm/iommu.h
> > > +++ b/arch/powerpc/include/asm/iommu.h
> > > @@ -36,6 +36,7 @@
> > >  #define IOMMU_PAGE_MASK_4K       (~((1 << IOMMU_PAGE_SHIFT_4K) - 1))
> > >  #define IOMMU_PAGE_ALIGN_4K(addr) _ALIGN_UP(addr, IOMMU_PAGE_SIZE_4K)
> > >  
> > > +#define IOMMU_PAGE_SHIFT(tblptr) (tblptr)->it_page_shift
> > >  #define IOMMU_PAGE_SIZE(tblptr) (ASM_CONST(1) << (tblptr)->it_page_shift)
> > 
> > Seems like it would be a touch safer to alter IOMMU_PAGE_SIZE so it
> > uses the new IOMMU_PAGE_SHIFT macro.
> 
> Yes absolutely! Sorry, I initially didn't add the first macro, so didn't
> think that through. Will update.

Err, replied too quickly -- just got back from vacation -- \fthis is
from an old version of the patchset, we no longer introduce this macro.

-Nish

[PATCH 0/5 v2] Fix NVMe driver support on Power with 32-bit DMA

[Nishanth Aravamudan]

We received a bug report recently when DDW (64-bit direct DMA on Power)
is not enabled for NVMe devices. In that case, we fall back to 32-bit
DMA via the IOMMU, which is always done via 4K TCEs (Translation Control
Entries).
 
The NVMe device driver, though, assumes that the DMA alignment for the
PRP entries will match the device's page size, and that the DMA aligment
matches the kernel's page aligment. On Power, the the IOMMU page size,
as mentioned above, can be 4K, while the device can have a page size of
8K, while the kernel has a page size of 64K. This eventually trips the
BUG_ON in nvme_setup_prps(), as we have a 'dma_len' that is a multiple
of 4K but not 8K (e.g., 0xF000).
 
In this particular case, and generally, we want to use the IOMMU's page
size for the default device page size, rather than the kernel's page
size.
 
This series consists of five patches:

1) add a generic dma_get_page_shift implementation that just returns
PAGE_SHIFT
2) override the generic implementation on Power to use the IOMMU table's
page shift if available
3) allow further specific overriding on power with machdep platform
overrides
4) use the machdep override on pseries, as the DDW code puts the TCE
shift in a special property and there is no IOMMU table available
5) leverage the new API in the NVMe driver
 
With these patches, a NVMe device survives our internal hardware
exerciser; the kernel BUGs within a few seconds without the patch.

 arch/powerpc/include/asm/dma-mapping.h   |  3 +++
 arch/powerpc/include/asm/machdep.h       |  3 ++-
 arch/powerpc/kernel/dma.c                | 11 +++++++++++
 arch/powerpc/platforms/pseries/iommu.c   | 36 ++++++++++++++++++++++++++++++++++++
 drivers/block/nvme-core.c                |  3 ++-
 include/asm-generic/dma-mapping-common.h |  7 +++++++
 6 files changed, 61 insertions(+), 2 deletions(-)

v1 -> v2:
  Based upon feedback from Christoph Hellwig, rather than using an
  arch-specific hack, expose the DMA page shift via a generic DMA API and
  override it on Power as needed.

[PATCH 0/5 v2] Fix NVMe driver support on Power with 32-bit DMA

[Nishanth Aravamudan]

We received a bug report recently when DDW (64-bit direct DMA on Power)
is not enabled for NVMe devices. In that case, we fall back to 32-bit
DMA via the IOMMU, which is always done via 4K TCEs (Translation Control
Entries).
 
The NVMe device driver, though, assumes that the DMA alignment for the
PRP entries will match the device's page size, and that the DMA aligment
matches the kernel's page aligment. On Power, the the IOMMU page size,
as mentioned above, can be 4K, while the device can have a page size of
8K, while the kernel has a page size of 64K. This eventually trips the
BUG_ON in nvme_setup_prps(), as we have a 'dma_len' that is a multiple
of 4K but not 8K (e.g., 0xF000).
 
In this particular case, and generally, we want to use the IOMMU's page
size for the default device page size, rather than the kernel's page
size.
 
This series consists of five patches:

1) add a generic dma_get_page_shift implementation that just returns
PAGE_SHIFT
2) override the generic implementation on Power to use the IOMMU table's
page shift if available
3) allow further specific overriding on power with machdep platform
overrides
4) use the machdep override on pseries, as the DDW code puts the TCE
shift in a special property and there is no IOMMU table available
5) leverage the new API in the NVMe driver
 
With these patches, a NVMe device survives our internal hardware
exerciser; the kernel BUGs within a few seconds without the patch.

 arch/powerpc/include/asm/dma-mapping.h   |  3 +++
 arch/powerpc/include/asm/machdep.h       |  3 ++-
 arch/powerpc/kernel/dma.c                | 11 +++++++++++
 arch/powerpc/platforms/pseries/iommu.c   | 36 ++++++++++++++++++++++++++++++++++++
 drivers/block/nvme-core.c                |  3 ++-
 include/asm-generic/dma-mapping-common.h |  7 +++++++
 6 files changed, 61 insertions(+), 2 deletions(-)

v1 -> v2:
  Based upon feedback from Christoph Hellwig, rather than using an
  arch-specific hack, expose the DMA page shift via a generic DMA API and
  override it on Power as needed.

[PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API

[Nishanth Aravamudan]

Drivers like NVMe need to be able to determine the page size used for
DMA transfers. Add a new API that defaults to return PAGE_SHIFT on all
architectures.

Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>

diff --git a/include/asm-generic/dma-mapping-common.h b/include/asm-generic/dma-mapping-common.h
index b1bc954..86e4e97 100644
--- a/include/asm-generic/dma-mapping-common.h
+++ b/include/asm-generic/dma-mapping-common.h
@@ -355,4 +355,11 @@ static inline int dma_set_mask(struct device *dev, u64 mask)
 }
 #endif
 
+#ifndef HAVE_ARCH_DMA_GET_PAGE_SHIFT
+static inline unsigned long dma_get_page_shift(struct device *dev)
+{
+	return PAGE_SHIFT;
+}
+#endif
+
 #endif

[PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API

[Nishanth Aravamudan]

Drivers like NVMe need to be able to determine the page size used for
DMA transfers. Add a new API that defaults to return PAGE_SHIFT on all
architectures.

Signed-off-by: Nishanth Aravamudan <nacc at linux.vnet.ibm.com>

diff --git a/include/asm-generic/dma-mapping-common.h b/include/asm-generic/dma-mapping-common.h
index b1bc954..86e4e97 100644
--- a/include/asm-generic/dma-mapping-common.h
+++ b/include/asm-generic/dma-mapping-common.h
@@ -355,4 +355,11 @@ static inline int dma_set_mask(struct device *dev, u64 mask)
 }
 #endif
 
+#ifndef HAVE_ARCH_DMA_GET_PAGE_SHIFT
+static inline unsigned long dma_get_page_shift(struct device *dev)
+{
+	return PAGE_SHIFT;
+}
+#endif
+
 #endif

[PATCH 2/5 v2] powerpc/dma-mapping: override dma_get_page_shift

[Nishanth Aravamudan]

On Power, the kernel's page size can differ from the IOMMU's page size,
so we need to override the generic implementation, which always returns
the kernel's page size. Lookup the IOMMU's page size from struct
iommu_table, if available. Fallback to the kernel's page size,
otherwise.

Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>

diff --git a/arch/powerpc/include/asm/dma-mapping.h b/arch/powerpc/include/asm/dma-mapping.h
index 7f522c0..c5638f4 100644
--- a/arch/powerpc/include/asm/dma-mapping.h
+++ b/arch/powerpc/include/asm/dma-mapping.h
@@ -125,6 +125,9 @@ static inline void set_dma_offset(struct device *dev, dma_addr_t off)
 #define HAVE_ARCH_DMA_SET_MASK 1
 extern int dma_set_mask(struct device *dev, u64 dma_mask);
 
+#define HAVE_ARCH_DMA_GET_PAGE_SHIFT 1
+extern unsigned long dma_get_page_shift(struct device *dev);
+
 #include <asm-generic/dma-mapping-common.h>
 
 extern int __dma_set_mask(struct device *dev, u64 dma_mask);
diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
index 59503ed..e805af2 100644
--- a/arch/powerpc/kernel/dma.c
+++ b/arch/powerpc/kernel/dma.c
@@ -335,6 +335,15 @@ int dma_set_mask(struct device *dev, u64 dma_mask)
 }
 EXPORT_SYMBOL(dma_set_mask);
 
+unsigned long dma_get_page_shift(struct device *dev)
+{
+	struct iommu_table *tbl = get_iommu_table_base(dev);
+	if (tbl)
+		return tbl->it_page_shift;
+	return PAGE_SHIFT;
+}
+EXPORT_SYMBOL(dma_get_page_shift);
+
 u64 __dma_get_required_mask(struct device *dev)
 {
 	struct dma_map_ops *dma_ops = get_dma_ops(dev);

[PATCH 2/5 v2] powerpc/dma-mapping: override dma_get_page_shift

[Nishanth Aravamudan]

On Power, the kernel's page size can differ from the IOMMU's page size,
so we need to override the generic implementation, which always returns
the kernel's page size. Lookup the IOMMU's page size from struct
iommu_table, if available. Fallback to the kernel's page size,
otherwise.

Signed-off-by: Nishanth Aravamudan <nacc at linux.vnet.ibm.com>

diff --git a/arch/powerpc/include/asm/dma-mapping.h b/arch/powerpc/include/asm/dma-mapping.h
index 7f522c0..c5638f4 100644
--- a/arch/powerpc/include/asm/dma-mapping.h
+++ b/arch/powerpc/include/asm/dma-mapping.h
@@ -125,6 +125,9 @@ static inline void set_dma_offset(struct device *dev, dma_addr_t off)
 #define HAVE_ARCH_DMA_SET_MASK 1
 extern int dma_set_mask(struct device *dev, u64 dma_mask);
 
+#define HAVE_ARCH_DMA_GET_PAGE_SHIFT 1
+extern unsigned long dma_get_page_shift(struct device *dev);
+
 #include <asm-generic/dma-mapping-common.h>
 
 extern int __dma_set_mask(struct device *dev, u64 dma_mask);
diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
index 59503ed..e805af2 100644
--- a/arch/powerpc/kernel/dma.c
+++ b/arch/powerpc/kernel/dma.c
@@ -335,6 +335,15 @@ int dma_set_mask(struct device *dev, u64 dma_mask)
 }
 EXPORT_SYMBOL(dma_set_mask);
 
+unsigned long dma_get_page_shift(struct device *dev)
+{
+	struct iommu_table *tbl = get_iommu_table_base(dev);
+	if (tbl)
+		return tbl->it_page_shift;
+	return PAGE_SHIFT;
+}
+EXPORT_SYMBOL(dma_get_page_shift);
+
 u64 __dma_get_required_mask(struct device *dev)
 {
 	struct dma_map_ops *dma_ops = get_dma_ops(dev);

[PATCH 3/5 v2] powerpc/dma: implement per-platform dma_get_page_shift

[Nishanth Aravamudan]

The IOMMU page size is not always stored in struct iommu on Power.
Specifically if a device is configured for DDW (Dynamic DMA Windows aka.
64-bit direct DMA), the used TCE (Translation Control Entry) size is
stored in a special device property created at run-time by the DDW
configuration code. DDW is a pseries-specific feature, so allow
platforms to override the implementation of dma_get_page_shift if
desired.

Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>

diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
index cab6753..5c372e3 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -78,9 +78,10 @@ struct machdep_calls {
 #endif
 #endif /* CONFIG_PPC64 */
 
-	/* Platform set_dma_mask and dma_get_required_mask overrides */
+	/* Platform overrides */
 	int		(*dma_set_mask)(struct device *dev, u64 dma_mask);
 	u64		(*dma_get_required_mask)(struct device *dev);
+	unsigned long	(*dma_get_page_shift)(struct device *dev);
 
 	int		(*probe)(void);
 	void		(*setup_arch)(void); /* Optional, may be NULL */
diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
index e805af2..c363896 100644
--- a/arch/powerpc/kernel/dma.c
+++ b/arch/powerpc/kernel/dma.c
@@ -338,6 +338,8 @@ EXPORT_SYMBOL(dma_set_mask);
 unsigned long dma_get_page_shift(struct device *dev)
 {
 	struct iommu_table *tbl = get_iommu_table_base(dev);
+	if (ppc_md.dma_get_page_shift)
+		return ppc_md.dma_get_page_shift(dev);
 	if (tbl)
 		return tbl->it_page_shift;
 	return PAGE_SHIFT;

[PATCH 3/5 v2] powerpc/dma: implement per-platform dma_get_page_shift

[Nishanth Aravamudan]

The IOMMU page size is not always stored in struct iommu on Power.
Specifically if a device is configured for DDW (Dynamic DMA Windows aka.
64-bit direct DMA), the used TCE (Translation Control Entry) size is
stored in a special device property created at run-time by the DDW
configuration code. DDW is a pseries-specific feature, so allow
platforms to override the implementation of dma_get_page_shift if
desired.

Signed-off-by: Nishanth Aravamudan <nacc at linux.vnet.ibm.com>

diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
index cab6753..5c372e3 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -78,9 +78,10 @@ struct machdep_calls {
 #endif
 #endif /* CONFIG_PPC64 */
 
-	/* Platform set_dma_mask and dma_get_required_mask overrides */
+	/* Platform overrides */
 	int		(*dma_set_mask)(struct device *dev, u64 dma_mask);
 	u64		(*dma_get_required_mask)(struct device *dev);
+	unsigned long	(*dma_get_page_shift)(struct device *dev);
 
 	int		(*probe)(void);
 	void		(*setup_arch)(void); /* Optional, may be NULL */
diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
index e805af2..c363896 100644
--- a/arch/powerpc/kernel/dma.c
+++ b/arch/powerpc/kernel/dma.c
@@ -338,6 +338,8 @@ EXPORT_SYMBOL(dma_set_mask);
 unsigned long dma_get_page_shift(struct device *dev)
 {
 	struct iommu_table *tbl = get_iommu_table_base(dev);
+	if (ppc_md.dma_get_page_shift)
+		return ppc_md.dma_get_page_shift(dev);
 	if (tbl)
 		return tbl->it_page_shift;
 	return PAGE_SHIFT;

[PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[Nishanth Aravamudan]

When DDW (Dynamic DMA Windows) are present for a device, we have stored
the TCE (Translation Control Entry) size in a special device tree
property. Check if we have enabled DDW for the device and return the TCE
size from that property if present. If the property isn't present,
fallback to looking the value up in struct iommu_table. If we don't find
a iommu_table, fallback to the kernel's page size.

Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>

diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 0946b98..1bf6471 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -1292,6 +1292,40 @@ static u64 dma_get_required_mask_pSeriesLP(struct device *dev)
 	return dma_iommu_ops.get_required_mask(dev);
 }
 
+static unsigned long dma_get_page_shift_pSeriesLP(struct device *dev)
+{
+	struct iommu_table *tbl;
+
+	if (!disable_ddw && dev_is_pci(dev)) {
+		struct pci_dev *pdev = to_pci_dev(dev);
+		struct device_node *dn;
+
+		dn = pci_device_to_OF_node(pdev);
+
+		/* search upwards for ibm,dma-window */
+		for (; dn && PCI_DN(dn) && !PCI_DN(dn)->table_group;
+				dn = dn->parent)
+			if (of_get_property(dn, "ibm,dma-window", NULL))
+				break;
+		/*
+		 * if there is a DDW configuration, the TCE shift is stored in
+		 * the property
+		 */
+		if (dn && PCI_DN(dn)) {
+			const struct dynamic_dma_window_prop *direct64 =
+				of_get_property(dn, DIRECT64_PROPNAME, NULL);
+			if (direct64)
+				return be32_to_cpu(direct64->tce_shift);
+		}
+	}
+
+	tbl = get_iommu_table_base(dev);
+	if (tbl)
+		return tbl->it_page_shift;
+
+	return PAGE_SHIFT;
+}
+
 #else  /* CONFIG_PCI */
 #define pci_dma_bus_setup_pSeries	NULL
 #define pci_dma_dev_setup_pSeries	NULL
@@ -1299,6 +1333,7 @@ static u64 dma_get_required_mask_pSeriesLP(struct device *dev)
 #define pci_dma_dev_setup_pSeriesLP	NULL
 #define dma_set_mask_pSeriesLP		NULL
 #define dma_get_required_mask_pSeriesLP	NULL
+#define dma_get_page_shift_pSeriesLP	NULL
 #endif /* !CONFIG_PCI */
 
 static int iommu_mem_notifier(struct notifier_block *nb, unsigned long action,
@@ -1395,6 +1430,7 @@ void iommu_init_early_pSeries(void)
 		pseries_pci_controller_ops.dma_dev_setup = pci_dma_dev_setup_pSeriesLP;
 		ppc_md.dma_set_mask = dma_set_mask_pSeriesLP;
 		ppc_md.dma_get_required_mask = dma_get_required_mask_pSeriesLP;
+		ppc_md.dma_get_page_shift = dma_get_page_shift_pSeriesLP;
 	} else {
 		pseries_pci_controller_ops.dma_bus_setup = pci_dma_bus_setup_pSeries;
 		pseries_pci_controller_ops.dma_dev_setup = pci_dma_dev_setup_pSeries;

[PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[Nishanth Aravamudan]

When DDW (Dynamic DMA Windows) are present for a device, we have stored
the TCE (Translation Control Entry) size in a special device tree
property. Check if we have enabled DDW for the device and return the TCE
size from that property if present. If the property isn't present,
fallback to looking the value up in struct iommu_table. If we don't find
a iommu_table, fallback to the kernel's page size.

Signed-off-by: Nishanth Aravamudan <nacc at linux.vnet.ibm.com>

diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 0946b98..1bf6471 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -1292,6 +1292,40 @@ static u64 dma_get_required_mask_pSeriesLP(struct device *dev)
 	return dma_iommu_ops.get_required_mask(dev);
 }
 
+static unsigned long dma_get_page_shift_pSeriesLP(struct device *dev)
+{
+	struct iommu_table *tbl;
+
+	if (!disable_ddw && dev_is_pci(dev)) {
+		struct pci_dev *pdev = to_pci_dev(dev);
+		struct device_node *dn;
+
+		dn = pci_device_to_OF_node(pdev);
+
+		/* search upwards for ibm,dma-window */
+		for (; dn && PCI_DN(dn) && !PCI_DN(dn)->table_group;
+				dn = dn->parent)
+			if (of_get_property(dn, "ibm,dma-window", NULL))
+				break;
+		/*
+		 * if there is a DDW configuration, the TCE shift is stored in
+		 * the property
+		 */
+		if (dn && PCI_DN(dn)) {
+			const struct dynamic_dma_window_prop *direct64 =
+				of_get_property(dn, DIRECT64_PROPNAME, NULL);
+			if (direct64)
+				return be32_to_cpu(direct64->tce_shift);
+		}
+	}
+
+	tbl = get_iommu_table_base(dev);
+	if (tbl)
+		return tbl->it_page_shift;
+
+	return PAGE_SHIFT;
+}
+
 #else  /* CONFIG_PCI */
 #define pci_dma_bus_setup_pSeries	NULL
 #define pci_dma_dev_setup_pSeries	NULL
@@ -1299,6 +1333,7 @@ static u64 dma_get_required_mask_pSeriesLP(struct device *dev)
 #define pci_dma_dev_setup_pSeriesLP	NULL
 #define dma_set_mask_pSeriesLP		NULL
 #define dma_get_required_mask_pSeriesLP	NULL
+#define dma_get_page_shift_pSeriesLP	NULL
 #endif /* !CONFIG_PCI */
 
 static int iommu_mem_notifier(struct notifier_block *nb, unsigned long action,
@@ -1395,6 +1430,7 @@ void iommu_init_early_pSeries(void)
 		pseries_pci_controller_ops.dma_dev_setup = pci_dma_dev_setup_pSeriesLP;
 		ppc_md.dma_set_mask = dma_set_mask_pSeriesLP;
 		ppc_md.dma_get_required_mask = dma_get_required_mask_pSeriesLP;
+		ppc_md.dma_get_page_shift = dma_get_page_shift_pSeriesLP;
 	} else {
 		pseries_pci_controller_ops.dma_bus_setup = pci_dma_bus_setup_pSeries;
 		pseries_pci_controller_ops.dma_dev_setup = pci_dma_dev_setup_pSeries;

[PATCH 5/5 v2] drivers/nvme: default to the IOMMU page size

[Nishanth Aravamudan]

We received a bug report recently when DDW (64-bit direct DMA on Power)
is not enabled for NVMe devices. In that case, we fall back to 32-bit
DMA via the IOMMU, which is always done via 4K TCEs (Translation Control
Entries).

The NVMe device driver, though, assumes that the DMA alignment for the
PRP entries will match the device's page size, and that the DMA aligment
matches the kernel's page aligment. On Power, the the IOMMU page size,
as mentioned above, can be 4K, while the device can have a page size of
8K, while the kernel has a page size of 64K. This eventually trips the
BUG_ON in nvme_setup_prps(), as we have a 'dma_len' that is a multiple
of 4K but not 8K (e.g., 0xF000).

In this particular case of page sizes, we clearly want to use the
IOMMU's page size in the driver. And generally, the NVMe driver in this
function should be using the IOMMU's page size for the default device
page size, rather than the kernel's page size.

With this patch, a NVMe device survives our internal hardware
exerciser; the kernel BUGs within a few seconds without the patch.

---
v1 -> v2:
  Based upon feedback from Christoph Hellwig, implement the IOMMU page
  size lookup as a generic DMA API, rather than an architecture-specific
  hack.

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index b97fc3f..c561137 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -18,6 +18,7 @@
 #include <linux/blk-mq.h>
 #include <linux/cpu.h>
 #include <linux/delay.h>
+#include <linux/dma-mapping.h>
 #include <linux/errno.h>
 #include <linux/fs.h>
 #include <linux/genhd.h>
@@ -1713,7 +1714,7 @@ static int nvme_configure_admin_queue(struct nvme_dev *dev)
 	u32 aqa;
 	u64 cap = readq(&dev->bar->cap);
 	struct nvme_queue *nvmeq;
-	unsigned page_shift = PAGE_SHIFT;
+	unsigned page_shift = dma_get_page_shift(dev->dev);
 	unsigned dev_page_min = NVME_CAP_MPSMIN(cap) + 12;
 	unsigned dev_page_max = NVME_CAP_MPSMAX(cap) + 12;
 

[PATCH 5/5 v2] drivers/nvme: default to the IOMMU page size

[Nishanth Aravamudan]

We received a bug report recently when DDW (64-bit direct DMA on Power)
is not enabled for NVMe devices. In that case, we fall back to 32-bit
DMA via the IOMMU, which is always done via 4K TCEs (Translation Control
Entries).

The NVMe device driver, though, assumes that the DMA alignment for the
PRP entries will match the device's page size, and that the DMA aligment
matches the kernel's page aligment. On Power, the the IOMMU page size,
as mentioned above, can be 4K, while the device can have a page size of
8K, while the kernel has a page size of 64K. This eventually trips the
BUG_ON in nvme_setup_prps(), as we have a 'dma_len' that is a multiple
of 4K but not 8K (e.g., 0xF000).

In this particular case of page sizes, we clearly want to use the
IOMMU's page size in the driver. And generally, the NVMe driver in this
function should be using the IOMMU's page size for the default device
page size, rather than the kernel's page size.

With this patch, a NVMe device survives our internal hardware
exerciser; the kernel BUGs within a few seconds without the patch.

---
v1 -> v2:
  Based upon feedback from Christoph Hellwig, implement the IOMMU page
  size lookup as a generic DMA API, rather than an architecture-specific
  hack.

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index b97fc3f..c561137 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -18,6 +18,7 @@
 #include <linux/blk-mq.h>
 #include <linux/cpu.h>
 #include <linux/delay.h>
+#include <linux/dma-mapping.h>
 #include <linux/errno.h>
 #include <linux/fs.h>
 #include <linux/genhd.h>
@@ -1713,7 +1714,7 @@ static int nvme_configure_admin_queue(struct nvme_dev *dev)
 	u32 aqa;
 	u64 cap = readq(&dev->bar->cap);
 	struct nvme_queue *nvmeq;
-	unsigned page_shift = PAGE_SHIFT;
+	unsigned page_shift = dma_get_page_shift(dev->dev);
 	unsigned dev_page_min = NVME_CAP_MPSMIN(cap) + 12;
 	unsigned dev_page_max = NVME_CAP_MPSMAX(cap) + 12;
 

Re: [PATCH 5/5 v2] drivers/nvme: default to the IOMMU page size

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1363 bytes --]

Hi Nishanth,

[auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]

config: sparc64-allyesconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sparc64 

All error/warnings (new ones prefixed by >>):

   drivers/block/nvme-core.c: In function 'nvme_configure_admin_queue':
>> drivers/block/nvme-core.c:1717:2: error: implicit declaration of function 'dma_get_page_shift' [-Werror=implicit-function-declaration]
     unsigned page_shift = dma_get_page_shift(dev->dev);
     ^
   cc1: some warnings being treated as errors

vim +/dma_get_page_shift +1717 drivers/block/nvme-core.c

  1711	static int nvme_configure_admin_queue(struct nvme_dev *dev)
  1712	{
  1713		int result;
  1714		u32 aqa;
  1715		u64 cap = readq(&dev->bar->cap);
  1716		struct nvme_queue *nvmeq;
> 1717		unsigned page_shift = dma_get_page_shift(dev->dev);
  1718		unsigned dev_page_min = NVME_CAP_MPSMIN(cap) + 12;
  1719		unsigned dev_page_max = NVME_CAP_MPSMAX(cap) + 12;
  1720	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 43733 bytes --]

[PATCH 5/5 v2] drivers/nvme: default to the IOMMU page size

[kbuild test robot]

Hi Nishanth,

[auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]

config: sparc64-allyesconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sparc64 

All error/warnings (new ones prefixed by >>):

   drivers/block/nvme-core.c: In function 'nvme_configure_admin_queue':
>> drivers/block/nvme-core.c:1717:2: error: implicit declaration of function 'dma_get_page_shift' [-Werror=implicit-function-declaration]
     unsigned page_shift = dma_get_page_shift(dev->dev);
     ^
   cc1: some warnings being treated as errors

vim +/dma_get_page_shift +1717 drivers/block/nvme-core.c

  1711	static int nvme_configure_admin_queue(struct nvme_dev *dev)
  1712	{
  1713		int result;
  1714		u32 aqa;
  1715		u64 cap = readq(&dev->bar->cap);
  1716		struct nvme_queue *nvmeq;
> 1717		unsigned page_shift = dma_get_page_shift(dev->dev);
  1718		unsigned dev_page_min = NVME_CAP_MPSMIN(cap) + 12;
  1719		unsigned dev_page_max = NVME_CAP_MPSMAX(cap) + 12;
  1720	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 43733 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-nvme/attachments/20151003/9547770b/attachment-0001.obj>

Re: [PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1533 bytes --]

Hi Nishanth,

[auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]

config: powerpc-defconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

All error/warnings (new ones prefixed by >>):

   arch/powerpc/platforms/pseries/iommu.c: In function 'iommu_init_early_pSeries':
>> arch/powerpc/platforms/pseries/iommu.c:1433:9: error: 'struct machdep_calls' has no member named 'dma_get_page_shift'
      ppc_md.dma_get_page_shift = dma_get_page_shift_pSeriesLP;
            ^

vim +1433 arch/powerpc/platforms/pseries/iommu.c

  1427	
  1428		if (firmware_has_feature(FW_FEATURE_LPAR)) {
  1429			pseries_pci_controller_ops.dma_bus_setup = pci_dma_bus_setup_pSeriesLP;
  1430			pseries_pci_controller_ops.dma_dev_setup = pci_dma_dev_setup_pSeriesLP;
  1431			ppc_md.dma_set_mask = dma_set_mask_pSeriesLP;
  1432			ppc_md.dma_get_required_mask = dma_get_required_mask_pSeriesLP;
> 1433			ppc_md.dma_get_page_shift = dma_get_page_shift_pSeriesLP;
  1434		} else {
  1435			pseries_pci_controller_ops.dma_bus_setup = pci_dma_bus_setup_pSeries;
  1436			pseries_pci_controller_ops.dma_dev_setup = pci_dma_dev_setup_pSeries;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 21511 bytes --]

[PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[kbuild test robot]

Hi Nishanth,

[auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]

config: powerpc-defconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

All error/warnings (new ones prefixed by >>):

   arch/powerpc/platforms/pseries/iommu.c: In function 'iommu_init_early_pSeries':
>> arch/powerpc/platforms/pseries/iommu.c:1433:9: error: 'struct machdep_calls' has no member named 'dma_get_page_shift'
      ppc_md.dma_get_page_shift = dma_get_page_shift_pSeriesLP;
            ^

vim +1433 arch/powerpc/platforms/pseries/iommu.c

  1427	
  1428		if (firmware_has_feature(FW_FEATURE_LPAR)) {
  1429			pseries_pci_controller_ops.dma_bus_setup = pci_dma_bus_setup_pSeriesLP;
  1430			pseries_pci_controller_ops.dma_dev_setup = pci_dma_dev_setup_pSeriesLP;
  1431			ppc_md.dma_set_mask = dma_set_mask_pSeriesLP;
  1432			ppc_md.dma_get_required_mask = dma_get_required_mask_pSeriesLP;
> 1433			ppc_md.dma_get_page_shift = dma_get_page_shift_pSeriesLP;
  1434		} else {
  1435			pseries_pci_controller_ops.dma_bus_setup = pci_dma_bus_setup_pSeries;
  1436			pseries_pci_controller_ops.dma_dev_setup = pci_dma_dev_setup_pSeries;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 21511 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-nvme/attachments/20151003/f5223439/attachment-0001.obj>

Re: [PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[Michael Ellerman]

On Sat, 2015-10-03 at 04:33 +0800, kbuild test robot wrote:
> Hi Nishanth,
> 
> [auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]
> 
> config: powerpc-defconfig (attached as .config)
> reproduce:
>         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=powerpc 
> 
> All error/warnings (new ones prefixed by >>):
> 
>    arch/powerpc/platforms/pseries/iommu.c: In function 'iommu_init_early_pSeries':
> >> arch/powerpc/platforms/pseries/iommu.c:1433:9: error: 'struct machdep_calls' has no member named 'dma_get_page_shift'
>       ppc_md.dma_get_page_shift = dma_get_page_shift_pSeriesLP;

It was added in patch 3/5, so I think this error is bogus. Unless there's a
typo I'm missing?

cheers

[PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[Michael Ellerman]

On Sat, 2015-10-03@04:33 +0800, kbuild test robot wrote:
> Hi Nishanth,
> 
> [auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]
> 
> config: powerpc-defconfig (attached as .config)
> reproduce:
>         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=powerpc 
> 
> All error/warnings (new ones prefixed by >>):
> 
>    arch/powerpc/platforms/pseries/iommu.c: In function 'iommu_init_early_pSeries':
> >> arch/powerpc/platforms/pseries/iommu.c:1433:9: error: 'struct machdep_calls' has no member named 'dma_get_page_shift'
>       ppc_md.dma_get_page_shift = dma_get_page_shift_pSeriesLP;

It was added in patch 3/5, so I think this error is bogus. Unless there's a
typo I'm missing?

cheers

Re: [kbuild-all] [PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[Fengguang Wu]

On Tue, Oct 06, 2015 at 02:39:06PM +1100, Michael Ellerman wrote:
> On Sat, 2015-10-03 at 04:33 +0800, kbuild test robot wrote:
> > Hi Nishanth,
> > 
> > [auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]
> > 
> > config: powerpc-defconfig (attached as .config)
> > reproduce:
> >         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # save the attached .config to linux build tree
> >         make.cross ARCH=powerpc 
> > 
> > All error/warnings (new ones prefixed by >>):
> > 
> >    arch/powerpc/platforms/pseries/iommu.c: In function 'iommu_init_early_pSeries':
> > >> arch/powerpc/platforms/pseries/iommu.c:1433:9: error: 'struct machdep_calls' has no member named 'dma_get_page_shift'
> >       ppc_md.dma_get_page_shift = dma_get_page_shift_pSeriesLP;
> 
> It was added in patch 3/5, so I think this error is bogus. Unless there's a
> typo I'm missing?

Yes sorry, the patchset was not detected correctly in your case,
ending up the patches being tested as individual ones.

I'll fixup the robot.

Thanks,
Fengguang

[kbuild-all] [PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[Fengguang Wu]

On Tue, Oct 06, 2015@02:39:06PM +1100, Michael Ellerman wrote:
> On Sat, 2015-10-03@04:33 +0800, kbuild test robot wrote:
> > Hi Nishanth,
> > 
> > [auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]
> > 
> > config: powerpc-defconfig (attached as .config)
> > reproduce:
> >         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # save the attached .config to linux build tree
> >         make.cross ARCH=powerpc 
> > 
> > All error/warnings (new ones prefixed by >>):
> > 
> >    arch/powerpc/platforms/pseries/iommu.c: In function 'iommu_init_early_pSeries':
> > >> arch/powerpc/platforms/pseries/iommu.c:1433:9: error: 'struct machdep_calls' has no member named 'dma_get_page_shift'
> >       ppc_md.dma_get_page_shift = dma_get_page_shift_pSeriesLP;
> 
> It was added in patch 3/5, so I think this error is bogus. Unless there's a
> typo I'm missing?

Yes sorry, the patchset was not detected correctly in your case,
ending up the patches being tested as individual ones.

I'll fixup the robot.

Thanks,
Fengguang

Re: [kbuild-all] [PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[Michael Ellerman]

On Wed, 2015-10-07 at 21:56 +0800, Fengguang Wu wrote:
> On Tue, Oct 06, 2015 at 02:39:06PM +1100, Michael Ellerman wrote:
> > On Sat, 2015-10-03 at 04:33 +0800, kbuild test robot wrote:
> > > Hi Nishanth,
> > > 
> > > [auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]
> > > 
> > > config: powerpc-defconfig (attached as .config)
> > > reproduce:
> > >         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
> > >         chmod +x ~/bin/make.cross
> > >         # save the attached .config to linux build tree
> > >         make.cross ARCH=powerpc 
> > > 
> > > All error/warnings (new ones prefixed by >>):
> > > 
> > >    arch/powerpc/platforms/pseries/iommu.c: In function 'iommu_init_early_pSeries':
> > > >> arch/powerpc/platforms/pseries/iommu.c:1433:9: error: 'struct machdep_calls' has no member named 'dma_get_page_shift'
> > >       ppc_md.dma_get_page_shift = dma_get_page_shift_pSeriesLP;
> > 
> > It was added in patch 3/5, so I think this error is bogus. Unless there's a
> > typo I'm missing?
> 
> Yes sorry, the patchset was not detected correctly in your case,
> ending up the patches being tested as individual ones.
> 
> I'll fixup the robot.

Thanks.

How did the robot decide to build this series in the first place? Does it build
everything sent to one of the lists on CC?

cheers

Re: [kbuild-all] [PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[Michael Ellerman]

On Wed, 2015-10-07 at 21:56 +0800, Fengguang Wu wrote:
> On Tue, Oct 06, 2015 at 02:39:06PM +1100, Michael Ellerman wrote:
> > On Sat, 2015-10-03 at 04:33 +0800, kbuild test robot wrote:
> > > Hi Nishanth,
> > > 
> > > [auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]
> > > 
> > > config: powerpc-defconfig (attached as .config)
> > > reproduce:
> > >         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
> > >         chmod +x ~/bin/make.cross
> > >         # save the attached .config to linux build tree
> > >         make.cross ARCH=powerpc 
> > > 
> > > All error/warnings (new ones prefixed by >>):
> > > 
> > >    arch/powerpc/platforms/pseries/iommu.c: In function 'iommu_init_early_pSeries':
> > > >> arch/powerpc/platforms/pseries/iommu.c:1433:9: error: 'struct machdep_calls' has no member named 'dma_get_page_shift'
> > >       ppc_md.dma_get_page_shift = dma_get_page_shift_pSeriesLP;
> > 
> > It was added in patch 3/5, so I think this error is bogus. Unless there's a
> > typo I'm missing?
> 
> Yes sorry, the patchset was not detected correctly in your case,
> ending up the patches being tested as individual ones.
> 
> I'll fixup the robot.

Thanks.

How did the robot decide to build this series in the first place? Does it build
everything sent to one of the lists on CC?

cheers

[kbuild-all] [PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[Michael Ellerman]

On Wed, 2015-10-07@21:56 +0800, Fengguang Wu wrote:
> On Tue, Oct 06, 2015@02:39:06PM +1100, Michael Ellerman wrote:
> > On Sat, 2015-10-03@04:33 +0800, kbuild test robot wrote:
> > > Hi Nishanth,
> > > 
> > > [auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]
> > > 
> > > config: powerpc-defconfig (attached as .config)
> > > reproduce:
> > >         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
> > >         chmod +x ~/bin/make.cross
> > >         # save the attached .config to linux build tree
> > >         make.cross ARCH=powerpc 
> > > 
> > > All error/warnings (new ones prefixed by >>):
> > > 
> > >    arch/powerpc/platforms/pseries/iommu.c: In function 'iommu_init_early_pSeries':
> > > >> arch/powerpc/platforms/pseries/iommu.c:1433:9: error: 'struct machdep_calls' has no member named 'dma_get_page_shift'
> > >       ppc_md.dma_get_page_shift = dma_get_page_shift_pSeriesLP;
> > 
> > It was added in patch 3/5, so I think this error is bogus. Unless there's a
> > typo I'm missing?
> 
> Yes sorry, the patchset was not detected correctly in your case,
> ending up the patches being tested as individual ones.
> 
> I'll fixup the robot.

Thanks.

How did the robot decide to build this series in the first place? Does it build
everything sent to one of the lists on CC?

cheers

Re: [kbuild-all] [PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[Fengguang Wu]

On Thu, Oct 08, 2015 at 11:11:59AM +1100, Michael Ellerman wrote:
> On Wed, 2015-10-07 at 21:56 +0800, Fengguang Wu wrote:
> > On Tue, Oct 06, 2015 at 02:39:06PM +1100, Michael Ellerman wrote:
> > > On Sat, 2015-10-03 at 04:33 +0800, kbuild test robot wrote:
> > > > Hi Nishanth,
> > > > 
> > > > [auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]
> > > > 
> > > > config: powerpc-defconfig (attached as .config)
> > > > reproduce:
> > > >         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
> > > >         chmod +x ~/bin/make.cross
> > > >         # save the attached .config to linux build tree
> > > >         make.cross ARCH=powerpc 
> > > > 
> > > > All error/warnings (new ones prefixed by >>):
> > > > 
> > > >    arch/powerpc/platforms/pseries/iommu.c: In function 'iommu_init_early_pSeries':
> > > > >> arch/powerpc/platforms/pseries/iommu.c:1433:9: error: 'struct machdep_calls' has no member named 'dma_get_page_shift'
> > > >       ppc_md.dma_get_page_shift = dma_get_page_shift_pSeriesLP;
> > > 
> > > It was added in patch 3/5, so I think this error is bogus. Unless there's a
> > > typo I'm missing?
> > 
> > Yes sorry, the patchset was not detected correctly in your case,
> > ending up the patches being tested as individual ones.
> > 
> > I'll fixup the robot.
> 
> Thanks.
> 
> How did the robot decide to build this series in the first place? Does it build
> everything sent to one of the lists on CC?

Yes, currently the following mailing lists are subscribed.  Patches
sent to them will be tested if they can be git-am to RC or linux-next
kernels:

kvm
linux-acpi
linux-bluetooth
linux-btrfs
linux-embedded
linux-ext4
linux-fsdevel
linux-media
linux-mmc
linux-nfs
linux-omap
linux-pci
linux-pm
linux-raid
linux-rdma
linux-scsi
linux-usb
linux-wireless
netdev
netfilter-devel
linux-mm
driverdev-devel
intel-gfx

Thanks,
Fengguang

[kbuild-all] [PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[Fengguang Wu]

On Thu, Oct 08, 2015@11:11:59AM +1100, Michael Ellerman wrote:
> On Wed, 2015-10-07@21:56 +0800, Fengguang Wu wrote:
> > On Tue, Oct 06, 2015@02:39:06PM +1100, Michael Ellerman wrote:
> > > On Sat, 2015-10-03@04:33 +0800, kbuild test robot wrote:
> > > > Hi Nishanth,
> > > > 
> > > > [auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]
> > > > 
> > > > config: powerpc-defconfig (attached as .config)
> > > > reproduce:
> > > >         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
> > > >         chmod +x ~/bin/make.cross
> > > >         # save the attached .config to linux build tree
> > > >         make.cross ARCH=powerpc 
> > > > 
> > > > All error/warnings (new ones prefixed by >>):
> > > > 
> > > >    arch/powerpc/platforms/pseries/iommu.c: In function 'iommu_init_early_pSeries':
> > > > >> arch/powerpc/platforms/pseries/iommu.c:1433:9: error: 'struct machdep_calls' has no member named 'dma_get_page_shift'
> > > >       ppc_md.dma_get_page_shift = dma_get_page_shift_pSeriesLP;
> > > 
> > > It was added in patch 3/5, so I think this error is bogus. Unless there's a
> > > typo I'm missing?
> > 
> > Yes sorry, the patchset was not detected correctly in your case,
> > ending up the patches being tested as individual ones.
> > 
> > I'll fixup the robot.
> 
> Thanks.
> 
> How did the robot decide to build this series in the first place? Does it build
> everything sent to one of the lists on CC?

Yes, currently the following mailing lists are subscribed.  Patches
sent to them will be tested if they can be git-am to RC or linux-next
kernels:

kvm
linux-acpi
linux-bluetooth
linux-btrfs
linux-embedded
linux-ext4
linux-fsdevel
linux-media
linux-mmc
linux-nfs
linux-omap
linux-pci
linux-pm
linux-raid
linux-rdma
linux-scsi
linux-usb
linux-wireless
netdev
netfilter-devel
linux-mm
driverdev-devel
intel-gfx

Thanks,
Fengguang

Re: [kbuild-all] [PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[Fengguang Wu]

On Thu, Oct 08, 2015 at 09:06:03AM +0800, Fengguang Wu wrote:
> On Thu, Oct 08, 2015 at 11:11:59AM +1100, Michael Ellerman wrote:
> > On Wed, 2015-10-07 at 21:56 +0800, Fengguang Wu wrote:
> > > On Tue, Oct 06, 2015 at 02:39:06PM +1100, Michael Ellerman wrote:
> > > > On Sat, 2015-10-03 at 04:33 +0800, kbuild test robot wrote:
> > > > > Hi Nishanth,
> > > > > 
> > > > > [auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]
> > > > > 
> > > > > config: powerpc-defconfig (attached as .config)
> > > > > reproduce:
> > > > >         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
> > > > >         chmod +x ~/bin/make.cross
> > > > >         # save the attached .config to linux build tree
> > > > >         make.cross ARCH=powerpc 
> > > > > 
> > > > > All error/warnings (new ones prefixed by >>):
> > > > > 
> > > > >    arch/powerpc/platforms/pseries/iommu.c: In function 'iommu_init_early_pSeries':
> > > > > >> arch/powerpc/platforms/pseries/iommu.c:1433:9: error: 'struct machdep_calls' has no member named 'dma_get_page_shift'
> > > > >       ppc_md.dma_get_page_shift = dma_get_page_shift_pSeriesLP;
> > > > 
> > > > It was added in patch 3/5, so I think this error is bogus. Unless there's a
> > > > typo I'm missing?
> > > 
> > > Yes sorry, the patchset was not detected correctly in your case,
> > > ending up the patches being tested as individual ones.
> > > 
> > > I'll fixup the robot.
> > 
> > Thanks.
> > 
> > How did the robot decide to build this series in the first place? Does it build
> > everything sent to one of the lists on CC?
> 
> Yes, currently the following mailing lists are subscribed.  Patches
> sent to them will be tested if they can be git-am to RC or linux-next
> kernels:
> 
> kvm
> linux-acpi
> linux-bluetooth
> linux-btrfs
> linux-embedded
> linux-ext4
> linux-fsdevel
> linux-media
> linux-mmc
> linux-nfs
> linux-omap
> linux-pci
> linux-pm
> linux-raid
> linux-rdma
> linux-scsi
> linux-usb
> linux-wireless
> netdev
> netfilter-devel
> linux-mm
> driverdev-devel
> intel-gfx

And of course linux-kernel. More lists could be added in future.

Thanks,
Fengguang

[kbuild-all] [PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[Fengguang Wu]

On Thu, Oct 08, 2015@09:06:03AM +0800, Fengguang Wu wrote:
> On Thu, Oct 08, 2015@11:11:59AM +1100, Michael Ellerman wrote:
> > On Wed, 2015-10-07@21:56 +0800, Fengguang Wu wrote:
> > > On Tue, Oct 06, 2015@02:39:06PM +1100, Michael Ellerman wrote:
> > > > On Sat, 2015-10-03@04:33 +0800, kbuild test robot wrote:
> > > > > Hi Nishanth,
> > > > > 
> > > > > [auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]
> > > > > 
> > > > > config: powerpc-defconfig (attached as .config)
> > > > > reproduce:
> > > > >         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
> > > > >         chmod +x ~/bin/make.cross
> > > > >         # save the attached .config to linux build tree
> > > > >         make.cross ARCH=powerpc 
> > > > > 
> > > > > All error/warnings (new ones prefixed by >>):
> > > > > 
> > > > >    arch/powerpc/platforms/pseries/iommu.c: In function 'iommu_init_early_pSeries':
> > > > > >> arch/powerpc/platforms/pseries/iommu.c:1433:9: error: 'struct machdep_calls' has no member named 'dma_get_page_shift'
> > > > >       ppc_md.dma_get_page_shift = dma_get_page_shift_pSeriesLP;
> > > > 
> > > > It was added in patch 3/5, so I think this error is bogus. Unless there's a
> > > > typo I'm missing?
> > > 
> > > Yes sorry, the patchset was not detected correctly in your case,
> > > ending up the patches being tested as individual ones.
> > > 
> > > I'll fixup the robot.
> > 
> > Thanks.
> > 
> > How did the robot decide to build this series in the first place? Does it build
> > everything sent to one of the lists on CC?
> 
> Yes, currently the following mailing lists are subscribed.  Patches
> sent to them will be tested if they can be git-am to RC or linux-next
> kernels:
> 
> kvm
> linux-acpi
> linux-bluetooth
> linux-btrfs
> linux-embedded
> linux-ext4
> linux-fsdevel
> linux-media
> linux-mmc
> linux-nfs
> linux-omap
> linux-pci
> linux-pm
> linux-raid
> linux-rdma
> linux-scsi
> linux-usb
> linux-wireless
> netdev
> netfilter-devel
> linux-mm
> driverdev-devel
> intel-gfx

And of course linux-kernel. More lists could be added in future.

Thanks,
Fengguang

Re: [kbuild-all] [PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[Michael Ellerman]

On Thu, 2015-10-08 at 09:16 +0800, Fengguang Wu wrote:
> On Thu, Oct 08, 2015 at 09:06:03AM +0800, Fengguang Wu wrote:
> > On Thu, Oct 08, 2015 at 11:11:59AM +1100, Michael Ellerman wrote:
> > > On Wed, 2015-10-07 at 21:56 +0800, Fengguang Wu wrote:
> > > > On Tue, Oct 06, 2015 at 02:39:06PM +1100, Michael Ellerman wrote:
> > > > > On Sat, 2015-10-03 at 04:33 +0800, kbuild test robot wrote:
> > > > > > Hi Nishanth,
> > > > > > 
> > > > > > [auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]
> > > > > > 
> > > > > > config: powerpc-defconfig (attached as .config)
> > > > > > reproduce:
> > > > > >         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
> > > > > >         chmod +x ~/bin/make.cross
> > > > > >         # save the attached .config to linux build tree
> > > > > >         make.cross ARCH=powerpc 
> > > > > > 
> > > > > > All error/warnings (new ones prefixed by >>):
> > > > > > 
> > > > > >    arch/powerpc/platforms/pseries/iommu.c: In function 'iommu_init_early_pSeries':
> > > > > > >> arch/powerpc/platforms/pseries/iommu.c:1433:9: error: 'struct machdep_calls' has no member named 'dma_get_page_shift'
> > > > > >       ppc_md.dma_get_page_shift = dma_get_page_shift_pSeriesLP;
> > > > > 
> > > > > It was added in patch 3/5, so I think this error is bogus. Unless there's a
> > > > > typo I'm missing?
> > > > 
> > > > Yes sorry, the patchset was not detected correctly in your case,
> > > > ending up the patches being tested as individual ones.
> > > > 
> > > > I'll fixup the robot.
> > > 
> > > Thanks.
> > > 
> > > How did the robot decide to build this series in the first place? Does it build
> > > everything sent to one of the lists on CC?
> > 
> > Yes, currently the following mailing lists are subscribed.  Patches
> > sent to them will be tested if they can be git-am to RC or linux-next
> > kernels:
> > 
> > kvm
> > linux-acpi
> > linux-bluetooth
> > linux-btrfs
> > linux-embedded
> > linux-ext4
> > linux-fsdevel
> > linux-media
> > linux-mmc
> > linux-nfs
> > linux-omap
> > linux-pci
> > linux-pm
> > linux-raid
> > linux-rdma
> > linux-scsi
> > linux-usb
> > linux-wireless
> > netdev
> > netfilter-devel
> > linux-mm
> > driverdev-devel
> > intel-gfx
> 
> And of course linux-kernel. More lists could be added in future.

So do you mind adding linuxppc-dev@lists.ozlabs.org ? :)

It's pretty low traffic compared to lkml.

cheers

Re: [kbuild-all] [PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[Michael Ellerman]

On Thu, 2015-10-08 at 09:16 +0800, Fengguang Wu wrote:
> On Thu, Oct 08, 2015 at 09:06:03AM +0800, Fengguang Wu wrote:
> > On Thu, Oct 08, 2015 at 11:11:59AM +1100, Michael Ellerman wrote:
> > > On Wed, 2015-10-07 at 21:56 +0800, Fengguang Wu wrote:
> > > > On Tue, Oct 06, 2015 at 02:39:06PM +1100, Michael Ellerman wrote:
> > > > > On Sat, 2015-10-03 at 04:33 +0800, kbuild test robot wrote:
> > > > > > Hi Nishanth,
> > > > > > 
> > > > > > [auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]
> > > > > > 
> > > > > > config: powerpc-defconfig (attached as .config)
> > > > > > reproduce:
> > > > > >         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
> > > > > >         chmod +x ~/bin/make.cross
> > > > > >         # save the attached .config to linux build tree
> > > > > >         make.cross ARCH=powerpc 
> > > > > > 
> > > > > > All error/warnings (new ones prefixed by >>):
> > > > > > 
> > > > > >    arch/powerpc/platforms/pseries/iommu.c: In function 'iommu_init_early_pSeries':
> > > > > > >> arch/powerpc/platforms/pseries/iommu.c:1433:9: error: 'struct machdep_calls' has no member named 'dma_get_page_shift'
> > > > > >       ppc_md.dma_get_page_shift = dma_get_page_shift_pSeriesLP;
> > > > > 
> > > > > It was added in patch 3/5, so I think this error is bogus. Unless there's a
> > > > > typo I'm missing?
> > > > 
> > > > Yes sorry, the patchset was not detected correctly in your case,
> > > > ending up the patches being tested as individual ones.
> > > > 
> > > > I'll fixup the robot.
> > > 
> > > Thanks.
> > > 
> > > How did the robot decide to build this series in the first place? Does it build
> > > everything sent to one of the lists on CC?
> > 
> > Yes, currently the following mailing lists are subscribed.  Patches
> > sent to them will be tested if they can be git-am to RC or linux-next
> > kernels:
> > 
> > kvm
> > linux-acpi
> > linux-bluetooth
> > linux-btrfs
> > linux-embedded
> > linux-ext4
> > linux-fsdevel
> > linux-media
> > linux-mmc
> > linux-nfs
> > linux-omap
> > linux-pci
> > linux-pm
> > linux-raid
> > linux-rdma
> > linux-scsi
> > linux-usb
> > linux-wireless
> > netdev
> > netfilter-devel
> > linux-mm
> > driverdev-devel
> > intel-gfx
> 
> And of course linux-kernel. More lists could be added in future.

So do you mind adding linuxppc-dev@lists.ozlabs.org ? :)

It's pretty low traffic compared to lkml.

cheers

[kbuild-all] [PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[Michael Ellerman]

On Thu, 2015-10-08@09:16 +0800, Fengguang Wu wrote:
> On Thu, Oct 08, 2015@09:06:03AM +0800, Fengguang Wu wrote:
> > On Thu, Oct 08, 2015@11:11:59AM +1100, Michael Ellerman wrote:
> > > On Wed, 2015-10-07@21:56 +0800, Fengguang Wu wrote:
> > > > On Tue, Oct 06, 2015@02:39:06PM +1100, Michael Ellerman wrote:
> > > > > On Sat, 2015-10-03@04:33 +0800, kbuild test robot wrote:
> > > > > > Hi Nishanth,
> > > > > > 
> > > > > > [auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]
> > > > > > 
> > > > > > config: powerpc-defconfig (attached as .config)
> > > > > > reproduce:
> > > > > >         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
> > > > > >         chmod +x ~/bin/make.cross
> > > > > >         # save the attached .config to linux build tree
> > > > > >         make.cross ARCH=powerpc 
> > > > > > 
> > > > > > All error/warnings (new ones prefixed by >>):
> > > > > > 
> > > > > >    arch/powerpc/platforms/pseries/iommu.c: In function 'iommu_init_early_pSeries':
> > > > > > >> arch/powerpc/platforms/pseries/iommu.c:1433:9: error: 'struct machdep_calls' has no member named 'dma_get_page_shift'
> > > > > >       ppc_md.dma_get_page_shift = dma_get_page_shift_pSeriesLP;
> > > > > 
> > > > > It was added in patch 3/5, so I think this error is bogus. Unless there's a
> > > > > typo I'm missing?
> > > > 
> > > > Yes sorry, the patchset was not detected correctly in your case,
> > > > ending up the patches being tested as individual ones.
> > > > 
> > > > I'll fixup the robot.
> > > 
> > > Thanks.
> > > 
> > > How did the robot decide to build this series in the first place? Does it build
> > > everything sent to one of the lists on CC?
> > 
> > Yes, currently the following mailing lists are subscribed.  Patches
> > sent to them will be tested if they can be git-am to RC or linux-next
> > kernels:
> > 
> > kvm
> > linux-acpi
> > linux-bluetooth
> > linux-btrfs
> > linux-embedded
> > linux-ext4
> > linux-fsdevel
> > linux-media
> > linux-mmc
> > linux-nfs
> > linux-omap
> > linux-pci
> > linux-pm
> > linux-raid
> > linux-rdma
> > linux-scsi
> > linux-usb
> > linux-wireless
> > netdev
> > netfilter-devel
> > linux-mm
> > driverdev-devel
> > intel-gfx
> 
> And of course linux-kernel. More lists could be added in future.

So do you mind adding linuxppc-dev at lists.ozlabs.org ? :)

It's pretty low traffic compared to lkml.

cheers

Re: [kbuild-all] [PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[Fengguang Wu]

Hi Michael,

> > > > How did the robot decide to build this series in the first place? Does it build
> > > > everything sent to one of the lists on CC?
> > > 
> > > Yes, currently the following mailing lists are subscribed.  Patches
> > > sent to them will be tested if they can be git-am to RC or linux-next
> > > kernels:
> > > 
> > > kvm
> > > linux-acpi
> > > linux-bluetooth
> > > linux-btrfs
> > > linux-embedded
> > > linux-ext4
> > > linux-fsdevel
> > > linux-media
> > > linux-mmc
> > > linux-nfs
> > > linux-omap
> > > linux-pci
> > > linux-pm
> > > linux-raid
> > > linux-rdma
> > > linux-scsi
> > > linux-usb
> > > linux-wireless
> > > netdev
> > > netfilter-devel
> > > linux-mm
> > > driverdev-devel
> > > intel-gfx
> > 
> > And of course linux-kernel. More lists could be added in future.
> 
> So do you mind adding linuxppc-dev@lists.ozlabs.org ? :)
> 
> It's pretty low traffic compared to lkml.

Sure, I'm subscribing to it. :)

Thanks,
Fengguang

[kbuild-all] [PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[Fengguang Wu]

Hi Michael,

> > > > How did the robot decide to build this series in the first place? Does it build
> > > > everything sent to one of the lists on CC?
> > > 
> > > Yes, currently the following mailing lists are subscribed.  Patches
> > > sent to them will be tested if they can be git-am to RC or linux-next
> > > kernels:
> > > 
> > > kvm
> > > linux-acpi
> > > linux-bluetooth
> > > linux-btrfs
> > > linux-embedded
> > > linux-ext4
> > > linux-fsdevel
> > > linux-media
> > > linux-mmc
> > > linux-nfs
> > > linux-omap
> > > linux-pci
> > > linux-pm
> > > linux-raid
> > > linux-rdma
> > > linux-scsi
> > > linux-usb
> > > linux-wireless
> > > netdev
> > > netfilter-devel
> > > linux-mm
> > > driverdev-devel
> > > intel-gfx
> > 
> > And of course linux-kernel. More lists could be added in future.
> 
> So do you mind adding linuxppc-dev at lists.ozlabs.org ? :)
> 
> It's pretty low traffic compared to lkml.

Sure, I'm subscribing to it. :)

Thanks,
Fengguang

Re: [kbuild-all] [PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[Michael Ellerman]

On Sun, 2015-10-11 at 22:22 +0800, Fengguang Wu wrote:
> > 
> > So do you mind adding linuxppc-dev@lists.ozlabs.org ? :)
> > 
> > It's pretty low traffic compared to lkml.
> 
> Sure, I'm subscribing to it. :)

Thanks.

cheers

Re: [kbuild-all] [PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[Michael Ellerman]

On Sun, 2015-10-11 at 22:22 +0800, Fengguang Wu wrote:
> > 
> > So do you mind adding linuxppc-dev@lists.ozlabs.org ? :)
> > 
> > It's pretty low traffic compared to lkml.
> 
> Sure, I'm subscribing to it. :)

Thanks.

cheers

[kbuild-all] [PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[Michael Ellerman]

On Sun, 2015-10-11@22:22 +0800, Fengguang Wu wrote:
> > 
> > So do you mind adding linuxppc-dev at lists.ozlabs.org ? :)
> > 
> > It's pretty low traffic compared to lkml.
> 
> Sure, I'm subscribing to it. :)

Thanks.

cheers

Re: [kbuild-all] [PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[Christoph Hellwig]

Hi Fengguang,

I think this proactive testing does a little more harm than good in
it's current form.  While offering testing for patches that aren't in
git trees and or by people that don't even have a git tree that the
build bots known about does seem useful, blindly doing it for every
patch against something that most likely isn't the right base seems
counter intertuitive.  We'll probaby need some annotation in the O/n
mail that asks for a test and sets a base tree to actually make it
useful.  With those few tweaks it should be really useful!

Maybe we should have a discussion about this at kernel summit?


On Thu, Oct 08, 2015 at 09:16:09AM +0800, Fengguang Wu wrote:
> And of course linux-kernel. More lists could be added in future.
> 
> Thanks,
> Fengguang
---end quoted text---

[kbuild-all] [PATCH 4/5 v2] pseries/iommu: implement DDW-aware dma_get_page_shift

[Christoph Hellwig]

Hi Fengguang,

I think this proactive testing does a little more harm than good in
it's current form.  While offering testing for patches that aren't in
git trees and or by people that don't even have a git tree that the
build bots known about does seem useful, blindly doing it for every
patch against something that most likely isn't the right base seems
counter intertuitive.  We'll probaby need some annotation in the O/n
mail that asks for a test and sets a base tree to actually make it
useful.  With those few tweaks it should be really useful!

Maybe we should have a discussion about this at kernel summit?


On Thu, Oct 08, 2015@09:16:09AM +0800, Fengguang Wu wrote:
> And of course linux-kernel. More lists could be added in future.
> 
> Thanks,
> Fengguang
---end quoted text---

testing email patches

[Fengguang Wu]

Hi Christoph,

On Thu, Oct 08, 2015 at 12:46:16AM -0700, Christoph Hellwig wrote:
> Hi Fengguang,
> 
> I think this proactive testing does a little more harm than good in
> it's current form.  While offering testing for patches that aren't in
> git trees and or by people that don't even have a git tree that the
> build bots known about does seem useful, blindly doing it for every
> patch against something that most likely isn't the right base seems
> counter intertuitive.  We'll probaby need some annotation in the O/n
> mail that asks for a test and sets a base tree to actually make it
> useful.  With those few tweaks it should be really useful!
> 
> Maybe we should have a discussion about this at kernel summit?

Yes that may be a good topic for gathering feedbacks and ideas for
improving this useful but messy testing feature.

The best option could be to define a way to annotate the base tree's
branch/commit of a patchset and possibly automate the annotation in
the git/quilt email clients. Currently git does generate

        index c933675..a2aa0cd

for each diff files, however I find it far from enough -- there are
~50000 files in the kernel tree, knowing hash numbers for the typical
1-100 files a patchset may touch is pretty helpless in narrowing down
the search space of possible base trees.

It may also be useful to specify whether or not a patchset needs such
kind of testing. That'd be an easier task -- the robot can detect some
magic words in the email and take action accordingly. And it can auto
skip testing in some cases -- for example, Peter Zijlstra offered a
good point to skip testing patches without "Signed-off-by:" in a "Re:"
email. The patches already tested as git tree commits in the 500+ git
trees the 0day robot monitors will be auto skipped, too.

Also we could test against both mainline and linux-next -- an
excellent suggestion from Dan Carpenter, which is just implemented,
thanks!

Not knowing the exact base tree leads to inherent messiness, and the
possible solutions/workarounds are

1) annotations to tell the base tree (best option)
2) annotations & heuristics to skip tests
3) guess the suitable tree the maintainer would apply patches to
4) test on mainline & linux-next to filter out unstable build errors

(3) is kind of dirty work and enlightenments from subsystem
maintainers are highly welcome -- typically the suggestions can be
offered when you find a bug report to be unsuitable.

As for now I'm trying to teach 0day robot to apply patches to the
matching subsystem's "for-next" branch based on information in the
MAINTAINERS file. 

Thanks,
Fengguang

> On Thu, Oct 08, 2015 at 09:16:09AM +0800, Fengguang Wu wrote:
> > And of course linux-kernel. More lists could be added in future.
> > 
> > Thanks,
> > Fengguang
> ---end quoted text---
> _______________________________________________
> kbuild-all mailing list
> kbuild-all@lists.01.org
> https://lists.01.org/mailman/listinfo/kbuild-all

testing email patches

[Fengguang Wu]

Hi Christoph,

On Thu, Oct 08, 2015@12:46:16AM -0700, Christoph Hellwig wrote:
> Hi Fengguang,
> 
> I think this proactive testing does a little more harm than good in
> it's current form.  While offering testing for patches that aren't in
> git trees and or by people that don't even have a git tree that the
> build bots known about does seem useful, blindly doing it for every
> patch against something that most likely isn't the right base seems
> counter intertuitive.  We'll probaby need some annotation in the O/n
> mail that asks for a test and sets a base tree to actually make it
> useful.  With those few tweaks it should be really useful!
> 
> Maybe we should have a discussion about this at kernel summit?

Yes that may be a good topic for gathering feedbacks and ideas for
improving this useful but messy testing feature.

The best option could be to define a way to annotate the base tree's
branch/commit of a patchset and possibly automate the annotation in
the git/quilt email clients. Currently git does generate

        index c933675..a2aa0cd

for each diff files, however I find it far from enough -- there are
~50000 files in the kernel tree, knowing hash numbers for the typical
1-100 files a patchset may touch is pretty helpless in narrowing down
the search space of possible base trees.

It may also be useful to specify whether or not a patchset needs such
kind of testing. That'd be an easier task -- the robot can detect some
magic words in the email and take action accordingly. And it can auto
skip testing in some cases -- for example, Peter Zijlstra offered a
good point to skip testing patches without "Signed-off-by:" in a "Re:"
email. The patches already tested as git tree commits in the 500+ git
trees the 0day robot monitors will be auto skipped, too.

Also we could test against both mainline and linux-next -- an
excellent suggestion from Dan Carpenter, which is just implemented,
thanks!

Not knowing the exact base tree leads to inherent messiness, and the
possible solutions/workarounds are

1) annotations to tell the base tree (best option)
2) annotations & heuristics to skip tests
3) guess the suitable tree the maintainer would apply patches to
4) test on mainline & linux-next to filter out unstable build errors

(3) is kind of dirty work and enlightenments from subsystem
maintainers are highly welcome -- typically the suggestions can be
offered when you find a bug report to be unsuitable.

As for now I'm trying to teach 0day robot to apply patches to the
matching subsystem's "for-next" branch based on information in the
MAINTAINERS file. 

Thanks,
Fengguang

> On Thu, Oct 08, 2015@09:16:09AM +0800, Fengguang Wu wrote:
> > And of course linux-kernel. More lists could be added in future.
> > 
> > Thanks,
> > Fengguang
> ---end quoted text---
> _______________________________________________
> kbuild-all mailing list
> kbuild-all at lists.01.org
> https://lists.01.org/mailman/listinfo/kbuild-all

Re: [PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API

[Michael Ellerman]

On Fri, 2015-10-02 at 13:11 -0700, Nishanth Aravamudan wrote:
> Drivers like NVMe need to be able to determine the page size used for
> DMA transfers. Add a new API that defaults to return PAGE_SHIFT on all
> architectures.
> 
> Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>
> 
> diff --git a/include/asm-generic/dma-mapping-common.h b/include/asm-generic/dma-mapping-common.h
> index b1bc954..86e4e97 100644
> --- a/include/asm-generic/dma-mapping-common.h
> +++ b/include/asm-generic/dma-mapping-common.h
> @@ -355,4 +355,11 @@ static inline int dma_set_mask(struct device *dev, u64 mask)
>  }
>  #endif
>  
> +#ifndef HAVE_ARCH_DMA_GET_PAGE_SHIFT
> +static inline unsigned long dma_get_page_shift(struct device *dev)
> +{
> +	return PAGE_SHIFT;
> +}
> +#endif
> +
>  #endif
> 

Adding Arnd who is asm-generic maintainer.

Arnd/Christoph can one of you give us an ack for this patch?

cheers

Re: [PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API

[Michael Ellerman]

On Fri, 2015-10-02 at 13:11 -0700, Nishanth Aravamudan wrote:
> Drivers like NVMe need to be able to determine the page size used for
> DMA transfers. Add a new API that defaults to return PAGE_SHIFT on all
> architectures.
> 
> Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>
> 
> diff --git a/include/asm-generic/dma-mapping-common.h b/include/asm-generic/dma-mapping-common.h
> index b1bc954..86e4e97 100644
> --- a/include/asm-generic/dma-mapping-common.h
> +++ b/include/asm-generic/dma-mapping-common.h
> @@ -355,4 +355,11 @@ static inline int dma_set_mask(struct device *dev, u64 mask)
>  }
>  #endif
>  
> +#ifndef HAVE_ARCH_DMA_GET_PAGE_SHIFT
> +static inline unsigned long dma_get_page_shift(struct device *dev)
> +{
> +	return PAGE_SHIFT;
> +}
> +#endif
> +
>  #endif
> 

Adding Arnd who is asm-generic maintainer.

Arnd/Christoph can one of you give us an ack for this patch?

cheers

[PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API

[Michael Ellerman]

On Fri, 2015-10-02@13:11 -0700, Nishanth Aravamudan wrote:
> Drivers like NVMe need to be able to determine the page size used for
> DMA transfers. Add a new API that defaults to return PAGE_SHIFT on all
> architectures.
> 
> Signed-off-by: Nishanth Aravamudan <nacc at linux.vnet.ibm.com>
> 
> diff --git a/include/asm-generic/dma-mapping-common.h b/include/asm-generic/dma-mapping-common.h
> index b1bc954..86e4e97 100644
> --- a/include/asm-generic/dma-mapping-common.h
> +++ b/include/asm-generic/dma-mapping-common.h
> @@ -355,4 +355,11 @@ static inline int dma_set_mask(struct device *dev, u64 mask)
>  }
>  #endif
>  
> +#ifndef HAVE_ARCH_DMA_GET_PAGE_SHIFT
> +static inline unsigned long dma_get_page_shift(struct device *dev)
> +{
> +	return PAGE_SHIFT;
> +}
> +#endif
> +
>  #endif
> 

Adding Arnd who is asm-generic maintainer.

Arnd/Christoph can one of you give us an ack for this patch?

cheers

Re: [PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API

[Christoph Hellwig]

Do we need a function here or can we just have a IOMMU_PAGE_SHIFT define
with an #ifndef in common code?

Also not all architectures use dma-mapping-common.h yet, so you either
need to update all of those as well, or just add the #ifndef directly
to linux/dma-mapping.h.

[PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API

[Christoph Hellwig]

Do we need a function here or can we just have a IOMMU_PAGE_SHIFT define
with an #ifndef in common code?

Also not all architectures use dma-mapping-common.h yet, so you either
need to update all of those as well, or just add the #ifndef directly
to linux/dma-mapping.h.

Re: [PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API

[Nishanth Aravamudan]

On 06.10.2015 [02:51:36 -0700], Christoph Hellwig wrote:
> Do we need a function here or can we just have a IOMMU_PAGE_SHIFT define
> with an #ifndef in common code?

I suppose we could do that -- I wasn't sure if the macro would be
palatable.

> Also not all architectures use dma-mapping-common.h yet, so you either
> need to update all of those as well, or just add the #ifndef directly
> to linux/dma-mapping.h.

Ok, I will take a look at that and spin up a v3.

Thanks for the feedback!

-Nish

[PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API

[Nishanth Aravamudan]

On 06.10.2015 [02:51:36 -0700], Christoph Hellwig wrote:
> Do we need a function here or can we just have a IOMMU_PAGE_SHIFT define
> with an #ifndef in common code?

I suppose we could do that -- I wasn't sure if the macro would be
palatable.

> Also not all architectures use dma-mapping-common.h yet, so you either
> need to update all of those as well, or just add the #ifndef directly
> to linux/dma-mapping.h.

Ok, I will take a look at that and spin up a v3.

Thanks for the feedback!

-Nish

Re: [PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API

[Nishanth Aravamudan]

On 06.10.2015 [02:51:36 -0700], Christoph Hellwig wrote:
> Do we need a function here or can we just have a IOMMU_PAGE_SHIFT define
> with an #ifndef in common code?

On Power, since it's technically variable, we'd need a function. So are
you suggesting define'ing it to a function just on Power and leaving it
a constant elsewhere?

I noticed that sparc has a IOMMU_PAGE_SHIFT already, fwiw.

-Nish

[PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API

[Nishanth Aravamudan]

On 06.10.2015 [02:51:36 -0700], Christoph Hellwig wrote:
> Do we need a function here or can we just have a IOMMU_PAGE_SHIFT define
> with an #ifndef in common code?

On Power, since it's technically variable, we'd need a function. So are
you suggesting define'ing it to a function just on Power and leaving it
a constant elsewhere?

I noticed that sparc has a IOMMU_PAGE_SHIFT already, fwiw.

-Nish

Re: [PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API

[Nishanth Aravamudan]

Hi Christoph,

On 12.10.2015 [14:06:51 -0700], Nishanth Aravamudan wrote:
> On 06.10.2015 [02:51:36 -0700], Christoph Hellwig wrote:
> > Do we need a function here or can we just have a IOMMU_PAGE_SHIFT define
> > with an #ifndef in common code?
> 
> On Power, since it's technically variable, we'd need a function. So are
> you suggesting define'ing it to a function just on Power and leaving it
> a constant elsewhere?
> 
> I noticed that sparc has a IOMMU_PAGE_SHIFT already, fwiw.

Sorry, I should have been more specific -- I'm ready to spin out a v3,
with a sparc-specific function.

Are you ok with leaving it a function for now (the only caller is in
NVMe obviously).

-Nish

[PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API

[Nishanth Aravamudan]

Hi Christoph,

On 12.10.2015 [14:06:51 -0700], Nishanth Aravamudan wrote:
> On 06.10.2015 [02:51:36 -0700], Christoph Hellwig wrote:
> > Do we need a function here or can we just have a IOMMU_PAGE_SHIFT define
> > with an #ifndef in common code?
> 
> On Power, since it's technically variable, we'd need a function. So are
> you suggesting define'ing it to a function just on Power and leaving it
> a constant elsewhere?
> 
> I noticed that sparc has a IOMMU_PAGE_SHIFT already, fwiw.

Sorry, I should have been more specific -- I'm ready to spin out a v3,
with a sparc-specific function.

Are you ok with leaving it a function for now (the only caller is in
NVMe obviously).

-Nish

Re: [PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API

[Christoph Hellwig]

Hi Nishanth,

sorry for the late reply.

> > On Power, since it's technically variable, we'd need a function. So are
> > you suggesting define'ing it to a function just on Power and leaving it
> > a constant elsewhere?
> > 
> > I noticed that sparc has a IOMMU_PAGE_SHIFT already, fwiw.
> 
> Sorry, I should have been more specific -- I'm ready to spin out a v3,
> with a sparc-specific function.
> 
> Are you ok with leaving it a function for now (the only caller is in
> NVMe obviously).


I guess we do indeed need a function then.  I'll take a look at your
patch, but as long you found a way to avoid adding too much boilerplate
code it should be fine.

[PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API

[Christoph Hellwig]

Hi Nishanth,

sorry for the late reply.

> > On Power, since it's technically variable, we'd need a function. So are
> > you suggesting define'ing it to a function just on Power and leaving it
> > a constant elsewhere?
> > 
> > I noticed that sparc has a IOMMU_PAGE_SHIFT already, fwiw.
> 
> Sorry, I should have been more specific -- I'm ready to spin out a v3,
> with a sparc-specific function.
> 
> Are you ok with leaving it a function for now (the only caller is in
> NVMe obviously).


I guess we do indeed need a function then.  I'll take a look at your
patch, but as long you found a way to avoid adding too much boilerplate
code it should be fine.

Re: [PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API

[Nishanth Aravamudan]

On 14.10.2015 [08:42:51 -0700], Christoph Hellwig wrote:
> Hi Nishanth,
> 
> sorry for the late reply.
> 
> > > On Power, since it's technically variable, we'd need a function. So are
> > > you suggesting define'ing it to a function just on Power and leaving it
> > > a constant elsewhere?
> > > 
> > > I noticed that sparc has a IOMMU_PAGE_SHIFT already, fwiw.
> > 
> > Sorry, I should have been more specific -- I'm ready to spin out a v3,
> > with a sparc-specific function.
> > 
> > Are you ok with leaving it a function for now (the only caller is in
> > NVMe obviously).
> 
> 
> I guess we do indeed need a function then.  I'll take a look at your
> patch, but as long you found a way to avoid adding too much boilerplate
> code it should be fine.

Ok, so I've got the moved function (include/linux/dma-mapping.h instead
of dma-mapping-common.h) ready to go, which should only involve changing
the first patch in the series. But I'm really mystified by what to do
for sparc, which defines IOMMU_PAGE_SHIFT and IO_PAGE_SHIFT in
arch/sparc/kernel/iommu_common.h.

1) Which constant reflects the value we mean for this function on sparc?
I assume it should be IOMMU_PAGE_SHIFT, but they are the same value and
I want to make sure I get the semantics right.

2) Where would I put sparc's definition of dma_get_page_shift()? Should
it be in a asm/dma-mapping.h? Should we move some of the constants from
arch/sparc/kernel/iommu_common.h to
arch/sparc/include/asm/iommu_common.h and then #include that in
asm/dma-mapping.h?

Dave M., any opinions/insights? Essentially, this helper function
assists the NVMe driver in determining what page size it should use to
satisfy both the device and IOMMU's requirements. Maybe I misunderstand
the constants on sparc and PAGE_SHIFT is fine there too?

-Nish

[PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API

[Nishanth Aravamudan]

On 14.10.2015 [08:42:51 -0700], Christoph Hellwig wrote:
> Hi Nishanth,
> 
> sorry for the late reply.
> 
> > > On Power, since it's technically variable, we'd need a function. So are
> > > you suggesting define'ing it to a function just on Power and leaving it
> > > a constant elsewhere?
> > > 
> > > I noticed that sparc has a IOMMU_PAGE_SHIFT already, fwiw.
> > 
> > Sorry, I should have been more specific -- I'm ready to spin out a v3,
> > with a sparc-specific function.
> > 
> > Are you ok with leaving it a function for now (the only caller is in
> > NVMe obviously).
> 
> 
> I guess we do indeed need a function then.  I'll take a look at your
> patch, but as long you found a way to avoid adding too much boilerplate
> code it should be fine.

Ok, so I've got the moved function (include/linux/dma-mapping.h instead
of dma-mapping-common.h) ready to go, which should only involve changing
the first patch in the series. But I'm really mystified by what to do
for sparc, which defines IOMMU_PAGE_SHIFT and IO_PAGE_SHIFT in
arch/sparc/kernel/iommu_common.h.

1) Which constant reflects the value we mean for this function on sparc?
I assume it should be IOMMU_PAGE_SHIFT, but they are the same value and
I want to make sure I get the semantics right.

2) Where would I put sparc's definition of dma_get_page_shift()? Should
it be in a asm/dma-mapping.h? Should we move some of the constants from
arch/sparc/kernel/iommu_common.h to
arch/sparc/include/asm/iommu_common.h and then #include that in
asm/dma-mapping.h?

Dave M., any opinions/insights? Essentially, this helper function
assists the NVMe driver in determining what page size it should use to
satisfy both the device and IOMMU's requirements. Maybe I misunderstand
the constants on sparc and PAGE_SHIFT is fine there too?

-Nish

Re: [PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API

[Nishanth Aravamudan]

On 14.10.2015 [08:42:51 -0700], Christoph Hellwig wrote:
> Hi Nishanth,
> 
> sorry for the late reply.
> 
> > > On Power, since it's technically variable, we'd need a function. So are
> > > you suggesting define'ing it to a function just on Power and leaving it
> > > a constant elsewhere?
> > > 
> > > I noticed that sparc has a IOMMU_PAGE_SHIFT already, fwiw.
> > 
> > Sorry, I should have been more specific -- I'm ready to spin out a v3,
> > with a sparc-specific function.
> > 
> > Are you ok with leaving it a function for now (the only caller is in
> > NVMe obviously).
> 
> 
> I guess we do indeed need a function then.  I'll take a look at your
> patch, but as long you found a way to avoid adding too much boilerplate
> code it should be fine.

Ok, so I've got the moved function (include/linux/dma-mapping.h instead
of dma-mapping-common.h) ready to go, which should only involve changing
the first patch in the series. But I'm really mystified by what to do
for sparc, which defines IOMMU_PAGE_SHIFT and IO_PAGE_SHIFT in
arch/sparc/kernel/iommu_common.h.

1) Which constant reflects the value we mean for this function on sparc?
I assume it should be IOMMU_PAGE_SHIFT, but they are the same value and
I want to make sure I get the semantics right.

2) Where would I put sparc's definition of dma_get_page_shift()? Should
it be in a asm/dma-mapping.h? Should we move some of the constants from
arch/sparc/kernel/iommu_common.h to
arch/sparc/include/asm/iommu_common.h and then #include that in
asm/dma-mapping.h?

Dave M., any opinions/insights? Essentially, this helper function
assists the NVMe driver in determining what page size it should use to
satisfy both the device and IOMMU's requirements. Maybe I misunderstand
the constants on sparc and PAGE_SHIFT is fine there too?

-Nish

Re: [PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API

[Nishanth Aravamudan]

On 15.10.2015 [15:52:19 -0700], Nishanth Aravamudan wrote:
> On 14.10.2015 [08:42:51 -0700], Christoph Hellwig wrote:
> > Hi Nishanth,
> > 
> > sorry for the late reply.
> > 
> > > > On Power, since it's technically variable, we'd need a function. So are
> > > > you suggesting define'ing it to a function just on Power and leaving it
> > > > a constant elsewhere?
> > > > 
> > > > I noticed that sparc has a IOMMU_PAGE_SHIFT already, fwiw.
> > > 
> > > Sorry, I should have been more specific -- I'm ready to spin out a v3,
> > > with a sparc-specific function.
> > > 
> > > Are you ok with leaving it a function for now (the only caller is in
> > > NVMe obviously).
> > 
> > 
> > I guess we do indeed need a function then.  I'll take a look at your
> > patch, but as long you found a way to avoid adding too much boilerplate
> > code it should be fine.
> 
> Ok, so I've got the moved function (include/linux/dma-mapping.h
> instead of dma-mapping-common.h) ready to go, which should only
> involve changing the first patch in the series. But I'm really
> mystified by what to do for sparc, which defines IOMMU_PAGE_SHIFT and
> IO_PAGE_SHIFT in arch/sparc/kernel/iommu_common.h.
> 
> 1) Which constant reflects the value we mean for this function on
> sparc?  I assume it should be IOMMU_PAGE_SHIFT, but they are the same
> value and I want to make sure I get the semantics right.
> 
> 2) Where would I put sparc's definition of dma_get_page_shift()?
> Should it be in a asm/dma-mapping.h? Should we move some of the
> constants from arch/sparc/kernel/iommu_common.h to
> arch/sparc/include/asm/iommu_common.h and then #include that in
> asm/dma-mapping.h?
> 
> Dave M., any opinions/insights? Essentially, this helper function
> assists the NVMe driver in determining what page size it should use to
> satisfy both the device and IOMMU's requirements. Maybe I
> misunderstand the constants on sparc and PAGE_SHIFT is fine there too?

Anyone with sparc knowledge have an opinion/input?

-Nish

[PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API

[Nishanth Aravamudan]

On 15.10.2015 [15:52:19 -0700], Nishanth Aravamudan wrote:
> On 14.10.2015 [08:42:51 -0700], Christoph Hellwig wrote:
> > Hi Nishanth,
> > 
> > sorry for the late reply.
> > 
> > > > On Power, since it's technically variable, we'd need a function. So are
> > > > you suggesting define'ing it to a function just on Power and leaving it
> > > > a constant elsewhere?
> > > > 
> > > > I noticed that sparc has a IOMMU_PAGE_SHIFT already, fwiw.
> > > 
> > > Sorry, I should have been more specific -- I'm ready to spin out a v3,
> > > with a sparc-specific function.
> > > 
> > > Are you ok with leaving it a function for now (the only caller is in
> > > NVMe obviously).
> > 
> > 
> > I guess we do indeed need a function then.  I'll take a look at your
> > patch, but as long you found a way to avoid adding too much boilerplate
> > code it should be fine.
> 
> Ok, so I've got the moved function (include/linux/dma-mapping.h
> instead of dma-mapping-common.h) ready to go, which should only
> involve changing the first patch in the series. But I'm really
> mystified by what to do for sparc, which defines IOMMU_PAGE_SHIFT and
> IO_PAGE_SHIFT in arch/sparc/kernel/iommu_common.h.
> 
> 1) Which constant reflects the value we mean for this function on
> sparc?  I assume it should be IOMMU_PAGE_SHIFT, but they are the same
> value and I want to make sure I get the semantics right.
> 
> 2) Where would I put sparc's definition of dma_get_page_shift()?
> Should it be in a asm/dma-mapping.h? Should we move some of the
> constants from arch/sparc/kernel/iommu_common.h to
> arch/sparc/include/asm/iommu_common.h and then #include that in
> asm/dma-mapping.h?
> 
> Dave M., any opinions/insights? Essentially, this helper function
> assists the NVMe driver in determining what page size it should use to
> satisfy both the device and IOMMU's requirements. Maybe I
> misunderstand the constants on sparc and PAGE_SHIFT is fine there too?

Anyone with sparc knowledge have an opinion/input?

-Nish

Re: [PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API

[Nishanth Aravamudan]

On 15.10.2015 [15:52:19 -0700], Nishanth Aravamudan wrote:
> On 14.10.2015 [08:42:51 -0700], Christoph Hellwig wrote:
> > Hi Nishanth,
> > 
> > sorry for the late reply.
> > 
> > > > On Power, since it's technically variable, we'd need a function. So are
> > > > you suggesting define'ing it to a function just on Power and leaving it
> > > > a constant elsewhere?
> > > > 
> > > > I noticed that sparc has a IOMMU_PAGE_SHIFT already, fwiw.
> > > 
> > > Sorry, I should have been more specific -- I'm ready to spin out a v3,
> > > with a sparc-specific function.
> > > 
> > > Are you ok with leaving it a function for now (the only caller is in
> > > NVMe obviously).
> > 
> > 
> > I guess we do indeed need a function then.  I'll take a look at your
> > patch, but as long you found a way to avoid adding too much boilerplate
> > code it should be fine.
> 
> Ok, so I've got the moved function (include/linux/dma-mapping.h
> instead of dma-mapping-common.h) ready to go, which should only
> involve changing the first patch in the series. But I'm really
> mystified by what to do for sparc, which defines IOMMU_PAGE_SHIFT and
> IO_PAGE_SHIFT in arch/sparc/kernel/iommu_common.h.
> 
> 1) Which constant reflects the value we mean for this function on
> sparc?  I assume it should be IOMMU_PAGE_SHIFT, but they are the same
> value and I want to make sure I get the semantics right.
> 
> 2) Where would I put sparc's definition of dma_get_page_shift()?
> Should it be in a asm/dma-mapping.h? Should we move some of the
> constants from arch/sparc/kernel/iommu_common.h to
> arch/sparc/include/asm/iommu_common.h and then #include that in
> asm/dma-mapping.h?
> 
> Dave M., any opinions/insights? Essentially, this helper function
> assists the NVMe driver in determining what page size it should use to
> satisfy both the device and IOMMU's requirements. Maybe I
> misunderstand the constants on sparc and PAGE_SHIFT is fine there too?

Anyone with sparc knowledge have an opinion/input?

-Nish

Re: [PATCH 0/5 v2] Fix NVMe driver support on Power with 32-bit DMA

[Benjamin Herrenschmidt]

On Fri, 2015-10-02 at 13:09 -0700, Nishanth Aravamudan wrote:

> 1) add a generic dma_get_page_shift implementation that just returns
> PAGE_SHIFT

So you chose to return the granularity of the iommu to the driver
rather than providing a way for the driver to request a specific
alignment for DMA mappings. Any specific reason ?

Cheers,
Ben.

[PATCH 0/5 v2] Fix NVMe driver support on Power with 32-bit DMA

[Benjamin Herrenschmidt]

On Fri, 2015-10-02@13:09 -0700, Nishanth Aravamudan wrote:

> 1) add a generic dma_get_page_shift implementation that just returns
> PAGE_SHIFT

So you chose to return the granularity of the iommu to the driver
rather than providing a way for the driver to request a specific
alignment for DMA mappings. Any specific reason ?

Cheers,
Ben.

Re: [PATCH 0/5 v2] Fix NVMe driver support on Power with 32-bit DMA

[Nishanth Aravamudan]

On 03.10.2015 [06:51:06 +1000], Benjamin Herrenschmidt wrote:
> On Fri, 2015-10-02 at 13:09 -0700, Nishanth Aravamudan wrote:
> 
> > 1) add a generic dma_get_page_shift implementation that just returns
> > PAGE_SHIFT
> 
> So you chose to return the granularity of the iommu to the driver
> rather than providing a way for the driver to request a specific
> alignment for DMA mappings. Any specific reason ?

Right, I did start with your advice and tried that approach, but it
turned out I was wrong about the actual issue at the time. The problem
for NVMe isn't actually the starting address alignment (which it can
handle not being aligned to the device's page size). It doesn't handle
(addr + len % dev_page_size != 0). That is, it's really a length
alignment issue.

It seems incredibly device specific to have a an API into the DMA code
to request an end alignment -- no other device seems to have this
issue/design. If you think that's better, I can fiddle with that
instead.

Sorry, I should have called this out better as an alternative
consideration.

-Nish

[PATCH 0/5 v2] Fix NVMe driver support on Power with 32-bit DMA

[Nishanth Aravamudan]

On 03.10.2015 [06:51:06 +1000], Benjamin Herrenschmidt wrote:
> On Fri, 2015-10-02@13:09 -0700, Nishanth Aravamudan wrote:
> 
> > 1) add a generic dma_get_page_shift implementation that just returns
> > PAGE_SHIFT
> 
> So you chose to return the granularity of the iommu to the driver
> rather than providing a way for the driver to request a specific
> alignment for DMA mappings. Any specific reason ?

Right, I did start with your advice and tried that approach, but it
turned out I was wrong about the actual issue at the time. The problem
for NVMe isn't actually the starting address alignment (which it can
handle not being aligned to the device's page size). It doesn't handle
(addr + len % dev_page_size != 0). That is, it's really a length
alignment issue.

It seems incredibly device specific to have a an API into the DMA code
to request an end alignment -- no other device seems to have this
issue/design. If you think that's better, I can fiddle with that
instead.

Sorry, I should have called this out better as an alternative
consideration.

-Nish

Re: [PATCH 0/5 v2] Fix NVMe driver support on Power with 32-bit DMA

[Benjamin Herrenschmidt]

On Fri, 2015-10-02 at 14:04 -0700, Nishanth Aravamudan wrote:
> Right, I did start with your advice and tried that approach, but it
> turned out I was wrong about the actual issue at the time. The problem
> for NVMe isn't actually the starting address alignment (which it can
> handle not being aligned to the device's page size). It doesn't handle
> (addr + len % dev_page_size != 0). That is, it's really a length
> alignment issue.
> 
> It seems incredibly device specific to have a an API into the DMA code
> to request an end alignment -- no other device seems to have this
> issue/design. If you think that's better, I can fiddle with that
> instead.
> 
> Sorry, I should have called this out better as an alternative
> consideration.

Nah it's fine. Ok. Also adding the alignment requirement to the API
would have been a much more complex patch since it would have had to
be implemented for all archs.

I think your current solution is fine.

Cheers,
Ben.

[PATCH 0/5 v2] Fix NVMe driver support on Power with 32-bit DMA

[Benjamin Herrenschmidt]

On Fri, 2015-10-02@14:04 -0700, Nishanth Aravamudan wrote:
> Right, I did start with your advice and tried that approach, but it
> turned out I was wrong about the actual issue at the time. The problem
> for NVMe isn't actually the starting address alignment (which it can
> handle not being aligned to the device's page size). It doesn't handle
> (addr + len % dev_page_size != 0). That is, it's really a length
> alignment issue.
> 
> It seems incredibly device specific to have a an API into the DMA code
> to request an end alignment -- no other device seems to have this
> issue/design. If you think that's better, I can fiddle with that
> instead.
> 
> Sorry, I should have called this out better as an alternative
> consideration.

Nah it's fine. Ok. Also adding the alignment requirement to the API
would have been a much more complex patch since it would have had to
be implemented for all archs.

I think your current solution is fine.

Cheers,
Ben.

Re: [PATCH 0/5 v2] Fix NVMe driver support on Power with 32-bit DMA

[Nishanth Aravamudan]

On 03.10.2015 [07:35:09 +1000], Benjamin Herrenschmidt wrote:
> On Fri, 2015-10-02 at 14:04 -0700, Nishanth Aravamudan wrote:
> > Right, I did start with your advice and tried that approach, but it
> > turned out I was wrong about the actual issue at the time. The problem
> > for NVMe isn't actually the starting address alignment (which it can
> > handle not being aligned to the device's page size). It doesn't handle
> > (addr + len % dev_page_size != 0). That is, it's really a length
> > alignment issue.
> > 
> > It seems incredibly device specific to have a an API into the DMA code
> > to request an end alignment -- no other device seems to have this
> > issue/design. If you think that's better, I can fiddle with that
> > instead.
> > 
> > Sorry, I should have called this out better as an alternative
> > consideration.
> 
> Nah it's fine. Ok. Also adding the alignment requirement to the API
> would have been a much more complex patch since it would have had to
> be implemented for all archs.
> 
> I think your current solution is fine.

Great, thanks. Also, while it's possible an alignment API would be more
performant...we're already not using DDW on Power in this case,
performance is not a primary concern. We want to simply be
functional/correct in this configuration.

-Nish

[PATCH 0/5 v2] Fix NVMe driver support on Power with 32-bit DMA

[Nishanth Aravamudan]

On 03.10.2015 [07:35:09 +1000], Benjamin Herrenschmidt wrote:
> On Fri, 2015-10-02@14:04 -0700, Nishanth Aravamudan wrote:
> > Right, I did start with your advice and tried that approach, but it
> > turned out I was wrong about the actual issue at the time. The problem
> > for NVMe isn't actually the starting address alignment (which it can
> > handle not being aligned to the device's page size). It doesn't handle
> > (addr + len % dev_page_size != 0). That is, it's really a length
> > alignment issue.
> > 
> > It seems incredibly device specific to have a an API into the DMA code
> > to request an end alignment -- no other device seems to have this
> > issue/design. If you think that's better, I can fiddle with that
> > instead.
> > 
> > Sorry, I should have called this out better as an alternative
> > consideration.
> 
> Nah it's fine. Ok. Also adding the alignment requirement to the API
> would have been a much more complex patch since it would have had to
> be implemented for all archs.
> 
> I think your current solution is fine.

Great, thanks. Also, while it's possible an alignment API would be more
performant...we're already not using DDW on Power in this case,
performance is not a primary concern. We want to simply be
functional/correct in this configuration.

-Nish

Re: [PATCH 0/5 v2] Fix NVMe driver support on Power with 32-bit DMA

[Christoph Hellwig]

On Sat, Oct 03, 2015 at 06:51:06AM +1000, Benjamin Herrenschmidt wrote:
> On Fri, 2015-10-02 at 13:09 -0700, Nishanth Aravamudan wrote:
> 
> > 1) add a generic dma_get_page_shift implementation that just returns
> > PAGE_SHIFT
> 
> So you chose to return the granularity of the iommu to the driver
> rather than providing a way for the driver to request a specific
> alignment for DMA mappings. Any specific reason ?

At least for NVMe that's the way to go - it allows to set a page set in
the device which should fit the IOMMU page size.

[PATCH 0/5 v2] Fix NVMe driver support on Power with 32-bit DMA

[Christoph Hellwig]

On Sat, Oct 03, 2015@06:51:06AM +1000, Benjamin Herrenschmidt wrote:
> On Fri, 2015-10-02@13:09 -0700, Nishanth Aravamudan wrote:
> 
> > 1) add a generic dma_get_page_shift implementation that just returns
> > PAGE_SHIFT
> 
> So you chose to return the granularity of the iommu to the driver
> rather than providing a way for the driver to request a specific
> alignment for DMA mappings. Any specific reason ?

At least for NVMe that's the way to go - it allows to set a page set in
the device which should fit the IOMMU page size.