[PATCH] pci: print out DMA mask info

[PATCH] pci: print out DMA mask info

[Yinghai Lu]

so can find out what is DMA mask is used for pci devices in addition to
default setting.

got:
ehci_hcd 0000:00:02.1: using 31bit consistent DMA mask
e1000 0000:0b:01.0: using 64bit DMA mask
e1000 0000:0b:01.0: using 64bit consistent DMA mask
e1000e 0000:04:00.0: using 64bit DMA mask
e1000e 0000:04:00.0: using 64bit consistent DMA mask
ixgb 0000:0c:01.0: using 64bit DMA mask
ixgb 0000:0c:01.0: using 64bit consistent DMA mask
aacraid 0000:86:00.0: using 32bit DMA mask
aacraid 0000:86:00.0: using 32bit consistent DMA mask
aacraid 0000:86:00.0: using 64bit DMA mask
aacraid 0000:86:00.0: using 64bit consistent DMA mask
qla2xxx 0000:0c:02.0: using 64bit consistent DMA mask
qla2xxx 0000:0c:02.1: using 64bit consistent DMA mask
lpfc 0000:06:00.0: using 64bit DMA mask
lpfc 0000:06:00.1: using 64bit DMA mask
pata_amd 0000:00:06.0: using 32bit DMA mask
pata_amd 0000:00:06.0: using 32bit consistent DMA mask
mptsas 0000:0c:04.0: using 64bit DMA mask
mptsas 0000:0c:04.0: using 64bit consistent DMA mask

forcedeth 0000:00:08.0: using 39bit DMA mask
forcedeth 0000:00:08.0: using 39bit consistent DMA mask
niu 0000:02:00.0: using 44bit DMA mask
niu 0000:02:00.0: using 44bit consistent DMA mask
sata_nv 0000:00:05.0: using 32bit DMA mask
sata_nv 0000:00:05.0: using 32bit consistent DMA mask
ib_mthca 0000:03:00.0: using 64bit DMA mask
ib_mthca 0000:03:00.0: using 64bit consistent DMA mask

wondering why: qlogic qla2xxx only set consistent to 64bit,
	       emulex lpfc not set consistent to 64bit

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/pci/pci.c |    4 ++++
 1 file changed, 4 insertions(+)

Index: linux-2.6/drivers/pci/pci.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci.c
+++ linux-2.6/drivers/pci/pci.c
@@ -1660,6 +1660,8 @@ pci_set_dma_mask(struct pci_dev *dev, u6
 		return -EIO;
 
 	dev->dma_mask = mask;
+	dev_printk(KERN_DEBUG, &dev->dev, "using %dbit DMA mask\n",
+		 fls64(mask));
 
 	return 0;
 }
@@ -1671,6 +1673,8 @@ pci_set_consistent_dma_mask(struct pci_d
 		return -EIO;
 
 	dev->dev.coherent_dma_mask = mask;
+	dev_printk(KERN_DEBUG, &dev->dev, "using %dbit consistent DMA mask\n",
+		 fls64(mask));
 
 	return 0;
 }

Re: [PATCH] pci: print out DMA mask info

[Grant Grundler]

On Wed, Oct 08, 2008 at 04:02:23PM -0700, Yinghai Lu wrote:
> so can find out what is DMA mask is used for pci devices in addition to
> default setting.

I like the idea. I don't like the additional boot time output.

But I'm thinking this could be an option to lspci.
lspci already knows about the /sys tree. Can PCI export the two masks
(dma_mask and dma_consistent_mask) and something like "lspci -td"
would dump those in a nice way?

Anyone else agree?

thanks,
grant

> 
> got:
> ehci_hcd 0000:00:02.1: using 31bit consistent DMA mask
> e1000 0000:0b:01.0: using 64bit DMA mask
> e1000 0000:0b:01.0: using 64bit consistent DMA mask
> e1000e 0000:04:00.0: using 64bit DMA mask
> e1000e 0000:04:00.0: using 64bit consistent DMA mask
> ixgb 0000:0c:01.0: using 64bit DMA mask
> ixgb 0000:0c:01.0: using 64bit consistent DMA mask
> aacraid 0000:86:00.0: using 32bit DMA mask
> aacraid 0000:86:00.0: using 32bit consistent DMA mask
> aacraid 0000:86:00.0: using 64bit DMA mask
> aacraid 0000:86:00.0: using 64bit consistent DMA mask
> qla2xxx 0000:0c:02.0: using 64bit consistent DMA mask
> qla2xxx 0000:0c:02.1: using 64bit consistent DMA mask
> lpfc 0000:06:00.0: using 64bit DMA mask
> lpfc 0000:06:00.1: using 64bit DMA mask
> pata_amd 0000:00:06.0: using 32bit DMA mask
> pata_amd 0000:00:06.0: using 32bit consistent DMA mask
> mptsas 0000:0c:04.0: using 64bit DMA mask
> mptsas 0000:0c:04.0: using 64bit consistent DMA mask
> 
> forcedeth 0000:00:08.0: using 39bit DMA mask
> forcedeth 0000:00:08.0: using 39bit consistent DMA mask
> niu 0000:02:00.0: using 44bit DMA mask
> niu 0000:02:00.0: using 44bit consistent DMA mask
> sata_nv 0000:00:05.0: using 32bit DMA mask
> sata_nv 0000:00:05.0: using 32bit consistent DMA mask
> ib_mthca 0000:03:00.0: using 64bit DMA mask
> ib_mthca 0000:03:00.0: using 64bit consistent DMA mask
> 
> wondering why: qlogic qla2xxx only set consistent to 64bit,
> 	       emulex lpfc not set consistent to 64bit
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> ---
>  drivers/pci/pci.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> Index: linux-2.6/drivers/pci/pci.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/pci.c
> +++ linux-2.6/drivers/pci/pci.c
> @@ -1660,6 +1660,8 @@ pci_set_dma_mask(struct pci_dev *dev, u6
>  		return -EIO;
>  
>  	dev->dma_mask = mask;
> +	dev_printk(KERN_DEBUG, &dev->dev, "using %dbit DMA mask\n",
> +		 fls64(mask));
>  
>  	return 0;
>  }
> @@ -1671,6 +1673,8 @@ pci_set_consistent_dma_mask(struct pci_d
>  		return -EIO;
>  
>  	dev->dev.coherent_dma_mask = mask;
> +	dev_printk(KERN_DEBUG, &dev->dev, "using %dbit consistent DMA mask\n",
> +		 fls64(mask));
>  
>  	return 0;
>  }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] pci: print out DMA mask info

[Yinghai Lu]

Grant Grundler wrote:
> On Wed, Oct 08, 2008 at 04:02:23PM -0700, Yinghai Lu wrote:
>> so can find out what is DMA mask is used for pci devices in addition to
>> default setting.
> 
> I like the idea. I don't like the additional boot time output.
> 
> But I'm thinking this could be an option to lspci.
> lspci already knows about the /sys tree. Can PCI export the two masks
> (dma_mask and dma_consistent_mask) and something like "lspci -td"
> would dump those in a nice way?
> 
in boot log, it could link with driver, pci info...
> 
>> got:
>> aacraid 0000:86:00.0: using 32bit DMA mask
>> aacraid 0000:86:00.0: using 32bit consistent DMA mask
>> aacraid 0000:86:00.0: using 64bit DMA mask
>> aacraid 0000:86:00.0: using 64bit consistent DMA mask

and aacraid do 32bit at first then 64bit...

so put that in boot message could be useful too.

YH

Re: [PATCH] pci: print out DMA mask info

[Matthew Wilcox]

On Thu, Oct 09, 2008 at 02:27:33PM -0700, Yinghai Lu wrote:
> >> aacraid 0000:86:00.0: using 32bit DMA mask
> >> aacraid 0000:86:00.0: using 32bit consistent DMA mask
> >> aacraid 0000:86:00.0: using 64bit DMA mask
> >> aacraid 0000:86:00.0: using 64bit consistent DMA mask
> 
> and aacraid do 32bit at first then 64bit...
> 
> so put that in boot message could be useful too.

Why's that interesting to the sysadmin of the machine?  To the driver
writer, certainly.  But what's the use of it to the people using the
machine?

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH] pci: print out DMA mask info

[Yinghai Lu]

On Thu, Oct 9, 2008 at 2:35 PM, Matthew Wilcox <matthew@wil.cx> wrote:

>
> Why's that interesting to the sysadmin of the machine?  To the driver
> writer, certainly.  But what's the use of it to the people using the
> machine?
>
> --
> Matthew Wilcox                          Intel Open Source Technology Centre
> "Bill, look, we understand that you're interested in selling us this
> operating system, but compare it to ours.  We can't possibly take such
> a retrograde step."

make linux kernel act like black box as other os?

YH

Re: [PATCH] pci: print out DMA mask info

[Grant Grundler]

On Thu, Oct 09, 2008 at 02:51:32PM -0700, Yinghai Lu wrote:
> On Thu, Oct 9, 2008 at 2:35 PM, Matthew Wilcox <matthew@wil.cx> wrote:
> > Why's that interesting to the sysadmin of the machine?  To the driver
> > writer, certainly.  But what's the use of it to the people using the
> > machine?
...
> make linux kernel act like black box as other os?

I don't understand your reply.
If someone thinks linux is a black box, printing this message won't help them.

"To flag use of bounce buffer or other suboptimal behaviors" could be debated.


Regarding associating the output with other PCI messages, I'd hope the fact
that the /sys entry is in the same directory as other sys files would be
enough clue to associate those together. e.g.:
grundler <2068>cd /sys/bus/pci/devices/0000\:01\:00.0/
grundler <2069>ls
broken_parity_status  driver@  irq         resource0   subsystem_device
bus@                  enable   local_cpus  resource1   subsystem_vendor
class                 i2c-0/   modalias    resource3   uevent
config                i2c-1/   power/      rom         vendor
device                i2c-2/   resource    subsystem@

means all those files refer to the same device. lspci is just a nice
wrapper to format/dump that info and perhaps help people associate
different bits of available info.

hth,
grant


> 
> YH
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] pci: print out DMA mask info

[Yinghai Lu]

Grant Grundler wrote:
> On Thu, Oct 09, 2008 at 02:51:32PM -0700, Yinghai Lu wrote:
>> On Thu, Oct 9, 2008 at 2:35 PM, Matthew Wilcox <matthew@wil.cx> wrote:
>>> Why's that interesting to the sysadmin of the machine?  To the driver
>>> writer, certainly.  But what's the use of it to the people using the
>>> machine?
> ...
>> make linux kernel act like black box as other os?
> 
> I don't understand your reply.
> If someone thinks linux is a black box, printing this message won't help them.
> 
could find out easily why some driver doesn't set dma mask correctly.
like why 
	qlogic qla2xxx only set consistent to 64bit,
	emulex lpfc not set consistent to 64bit
> 
> "To flag use of bounce buffer or other suboptimal behaviors" could be debated.
> 
> 
> Regarding associating the output with other PCI messages, I'd hope the fact
> that the /sys entry is in the same directory as other sys files would be
> enough clue to associate those together. e.g.:
> grundler <2068>cd /sys/bus/pci/devices/0000\:01\:00.0/
> grundler <2069>ls
> broken_parity_status  driver@  irq         resource0   subsystem_device
> bus@                  enable   local_cpus  resource1   subsystem_vendor
> class                 i2c-0/   modalias    resource3   uevent
> config                i2c-1/   power/      rom         vendor
> device                i2c-2/   resource    subsystem@
> 

add dma_mask coherent_dma_mask here?

YH

Re: [PATCH] pci: print out DMA mask info

[FUJITA Tomonori]

On Thu, 09 Oct 2008 16:05:57 -0700
Yinghai Lu <yinghai@kernel.org> wrote:

> Grant Grundler wrote:
> > On Thu, Oct 09, 2008 at 02:51:32PM -0700, Yinghai Lu wrote:
> >> On Thu, Oct 9, 2008 at 2:35 PM, Matthew Wilcox <matthew@wil.cx> wrote:
> >>> Why's that interesting to the sysadmin of the machine?  To the driver
> >>> writer, certainly.  But what's the use of it to the people using the
> >>> machine?
> > ...
> >> make linux kernel act like black box as other os?
> > 
> > I don't understand your reply.
> > If someone thinks linux is a black box, printing this message won't help them.
> > 
> could find out easily why some driver doesn't set dma mask correctly.
> like why 
> 	qlogic qla2xxx only set consistent to 64bit,
> 	emulex lpfc not set consistent to 64bit

IIRC, except for one SGI architecture, coherent_dma_mask is
meaningless, dma_mask is always equal to coherent_dma_mask. Lots of
IOMMU implementations ignore coherent_dma_mask and use dma_mask for
alloc_coherent(). Some drivers doesn't set up coherent_dma_mask.

Theoretically, we need to fix this but it doesn't cause any
problem. That's why nobody cares about it, I guess.

Re: [PATCH] pci: print out DMA mask info

[Yinghai Lu]

On Thu, Oct 9, 2008 at 7:40 PM, FUJITA Tomonori
<fujita.tomonori@lab.ntt.co.jp> wrote:
> On Thu, 09 Oct 2008 16:05:57 -0700
> Yinghai Lu <yinghai@kernel.org> wrote:
>
>> Grant Grundler wrote:
>> > On Thu, Oct 09, 2008 at 02:51:32PM -0700, Yinghai Lu wrote:
>> >> On Thu, Oct 9, 2008 at 2:35 PM, Matthew Wilcox <matthew@wil.cx> wrote:
>> >>> Why's that interesting to the sysadmin of the machine?  To the driver
>> >>> writer, certainly.  But what's the use of it to the people using the
>> >>> machine?
>> > ...
>> >> make linux kernel act like black box as other os?
>> >
>> > I don't understand your reply.
>> > If someone thinks linux is a black box, printing this message won't help them.
>> >
>> could find out easily why some driver doesn't set dma mask correctly.
>> like why
>>       qlogic qla2xxx only set consistent to 64bit,
>>       emulex lpfc not set consistent to 64bit
>
> IIRC, except for one SGI architecture, coherent_dma_mask is
> meaningless, dma_mask is always equal to coherent_dma_mask. Lots of
> IOMMU implementations ignore coherent_dma_mask and use dma_mask for
> alloc_coherent(). Some drivers doesn't set up coherent_dma_mask.

ehci_hcd 0000:00:02.1: using 31bit consistent DMA mask
==> ck804 ehci, is using 31bit for consistent dma mask, at still use
32 bit for dma mask.

qlogic qla2xxx  and emulex lpfc dma mask and consistent_dma_mask is different...
could have some story for them

at least gart iommu is honoring the consistent dma mask.
by calling dma_alloc_coherent_mask(dev, flag)

if device could use 64 bit coherent dma mask, that is driver problem...
print it out could put them in focus.

YH

Re: [PATCH] pci: print out DMA mask info

[FUJITA Tomonori]

On Thu, 9 Oct 2008 21:56:33 -0700
"Yinghai Lu" <yinghai@kernel.org> wrote:

> On Thu, Oct 9, 2008 at 7:40 PM, FUJITA Tomonori
> <fujita.tomonori@lab.ntt.co.jp> wrote:
> > On Thu, 09 Oct 2008 16:05:57 -0700
> > Yinghai Lu <yinghai@kernel.org> wrote:
> >
> >> Grant Grundler wrote:
> >> > On Thu, Oct 09, 2008 at 02:51:32PM -0700, Yinghai Lu wrote:
> >> >> On Thu, Oct 9, 2008 at 2:35 PM, Matthew Wilcox <matthew@wil.cx> wrote:
> >> >>> Why's that interesting to the sysadmin of the machine?  To the driver
> >> >>> writer, certainly.  But what's the use of it to the people using the
> >> >>> machine?
> >> > ...
> >> >> make linux kernel act like black box as other os?
> >> >
> >> > I don't understand your reply.
> >> > If someone thinks linux is a black box, printing this message won't help them.
> >> >
> >> could find out easily why some driver doesn't set dma mask correctly.
> >> like why
> >>       qlogic qla2xxx only set consistent to 64bit,
> >>       emulex lpfc not set consistent to 64bit
> >
> > IIRC, except for one SGI architecture, coherent_dma_mask is
> > meaningless, dma_mask is always equal to coherent_dma_mask. Lots of
> > IOMMU implementations ignore coherent_dma_mask and use dma_mask for
> > alloc_coherent(). Some drivers doesn't set up coherent_dma_mask.
> 
> ehci_hcd 0000:00:02.1: using 31bit consistent DMA mask
> ==> ck804 ehci, is using 31bit for consistent dma mask, at still use
> 32 bit for dma mask.

ehci_hcd needs to set 31bit to dma_mask, I guess.


> qlogic qla2xxx  and emulex lpfc dma mask and consistent_dma_mask is different...
> could have some story for them

Check out qla2xxx again. I think that it uses dma_set_mask() to set
dma_mask. qla2xxx uses the same value for both dma_mask and
consistent_dma_mask.

lpfc had better set 64bit to consistent_dma_mask but as I said in the
previous mail, not setting consistent_dma_mask doesn't cause any
problem. It means that some IOMMUs (uses consistent_dma_mask properly)
allocates an address < 4GB in alloc_coherent() and some IOMMUs alloc
address > 4GB. lpfc can handle both anyway.


> at least gart iommu is honoring the consistent dma mask.
> by calling dma_alloc_coherent_mask(dev, flag)

Well, that's because I wrote gart's alloc_coherent and introduced
dma_alloc_coherent_mask. ;)


> if device could use 64 bit coherent dma mask, that is driver problem...

Can you be more specific? As I wrote above, if 64bit-dma-capable
devices don't set consistent_dma_mask, we don't have any problem.

Yes, drivers that have dma_mask < 32bit need to set up
consistent_dma_mask. But few devices have dma_mask < 32bit and we can
fix them without the information at boot time, I think.


> print it out could put them in focus.

Re: [PATCH] pci: print out DMA mask info

[Yinghai Lu]

FUJITA Tomonori wrote:
> On Thu, 9 Oct 2008 21:56:33 -0700
> "Yinghai Lu" <yinghai@kernel.org> wrote:
> 
>> On Thu, Oct 9, 2008 at 7:40 PM, FUJITA Tomonori
>> <fujita.tomonori@lab.ntt.co.jp> wrote:
>>> On Thu, 09 Oct 2008 16:05:57 -0700
>>> Yinghai Lu <yinghai@kernel.org> wrote:
>>>
>>>> Grant Grundler wrote:
>>>>> On Thu, Oct 09, 2008 at 02:51:32PM -0700, Yinghai Lu wrote:
>>>>>> On Thu, Oct 9, 2008 at 2:35 PM, Matthew Wilcox <matthew@wil.cx> wrote:
>>>>>>> Why's that interesting to the sysadmin of the machine?  To the driver
>>>>>>> writer, certainly.  But what's the use of it to the people using the
>>>>>>> machine?
>>>>> ...
>>>>>> make linux kernel act like black box as other os?
>>>>> I don't understand your reply.
>>>>> If someone thinks linux is a black box, printing this message won't help them.
>>>>>
>>>> could find out easily why some driver doesn't set dma mask correctly.
>>>> like why
>>>>       qlogic qla2xxx only set consistent to 64bit,
>>>>       emulex lpfc not set consistent to 64bit
>>> IIRC, except for one SGI architecture, coherent_dma_mask is
>>> meaningless, dma_mask is always equal to coherent_dma_mask. Lots of
>>> IOMMU implementations ignore coherent_dma_mask and use dma_mask for
>>> alloc_coherent(). Some drivers doesn't set up coherent_dma_mask.
>> ehci_hcd 0000:00:02.1: using 31bit consistent DMA mask
>> ==> ck804 ehci, is using 31bit for consistent dma mask, at still use
>> 32 bit for dma mask.
> 
> ehci_hcd needs to set 31bit to dma_mask, I guess.

in ehci_pci_setup() 

        switch (pdev->vendor) {
        case PCI_VENDOR_ID_NVIDIA:
                /* NVidia reports that certain chips don't handle
                 * QH, ITD, or SITD addresses above 2GB.  (But TD,
                 * data buffer, and periodic schedule are normal.)
                 */
                switch (pdev->device) {
                case 0x003c:    /* MCP04 */
                case 0x005b:    /* CK804 */
                case 0x00d8:    /* CK8 */
                case 0x00e8:    /* CK8S */
                        if (pci_set_consistent_dma_mask(pdev,
                                                DMA_31BIT_MASK) < 0)
                                ehci_warn(ehci, "can't enable NVidia "
                                        "workaround for >2GB RAM\n");
                        break;
                }
                break;
        }

so that is strange silicon bug for old ck804 and before. dma_mask could be 32bit. but consistent_dma_mask is 31bit
mcp55 is ok.


> 
> 
>> qlogic qla2xxx  and emulex lpfc dma mask and consistent_dma_mask is different...
>> could have some story for them
> 
> Check out qla2xxx again. I think that it uses dma_set_mask() to set
> dma_mask. qla2xxx uses the same value for both dma_mask and
> consistent_dma_mask.
> 
> lpfc had better set 64bit to consistent_dma_mask but as I said in the
> previous mail, not setting consistent_dma_mask doesn't cause any
> problem. It means that some IOMMUs (uses consistent_dma_mask properly)
> allocates an address < 4GB in alloc_coherent() and some IOMMUs alloc
> address > 4GB. lpfc can handle both anyway.
> 
> 
>> at least gart iommu is honoring the consistent dma mask.
>> by calling dma_alloc_coherent_mask(dev, flag)
> 
> Well, that's because I wrote gart's alloc_coherent and introduced
> dma_alloc_coherent_mask. ;)

oh. it is in tip

> 
> 
>> if device could use 64 bit coherent dma mask, that is driver problem...
> 
> Can you be more specific? As I wrote above, if 64bit-dma-capable
> devices don't set consistent_dma_mask, we don't have any problem.

then can we remove consistent_dma_mask? just use dma_mask instead for all.

> 
> Yes, drivers that have dma_mask < 32bit need to set up
> consistent_dma_mask. But few devices have dma_mask < 32bit and we can
> fix them without the information at boot time, I think.

YH

Re: [PATCH] pci: print out DMA mask info

[FUJITA Tomonori]

On Thu, 09 Oct 2008 23:32:51 -0700
Yinghai Lu <yinghai@kernel.org> wrote:

> in ehci_pci_setup() 
> 
>         switch (pdev->vendor) {
>         case PCI_VENDOR_ID_NVIDIA:
>                 /* NVidia reports that certain chips don't handle
>                  * QH, ITD, or SITD addresses above 2GB.  (But TD,
>                  * data buffer, and periodic schedule are normal.)
>                  */
>                 switch (pdev->device) {
>                 case 0x003c:    /* MCP04 */
>                 case 0x005b:    /* CK804 */
>                 case 0x00d8:    /* CK8 */
>                 case 0x00e8:    /* CK8S */
>                         if (pci_set_consistent_dma_mask(pdev,
>                                                 DMA_31BIT_MASK) < 0)
>                                 ehci_warn(ehci, "can't enable NVidia "
>                                         "workaround for >2GB RAM\n");
>                         break;
>                 }
>                 break;
>         }
> 
> so that is strange silicon bug for old ck804 and before. dma_mask could be 32bit. but consistent_dma_mask is 31bit
> mcp55 is ok.

Hmm, looks broken hardware...


> >> qlogic qla2xxx  and emulex lpfc dma mask and consistent_dma_mask is different...
> >> could have some story for them
> > 
> > Check out qla2xxx again. I think that it uses dma_set_mask() to set
> > dma_mask. qla2xxx uses the same value for both dma_mask and
> > consistent_dma_mask.
> > 
> > lpfc had better set 64bit to consistent_dma_mask but as I said in the
> > previous mail, not setting consistent_dma_mask doesn't cause any
> > problem. It means that some IOMMUs (uses consistent_dma_mask properly)
> > allocates an address < 4GB in alloc_coherent() and some IOMMUs alloc
> > address > 4GB. lpfc can handle both anyway.
> > 
> > 
> >> at least gart iommu is honoring the consistent dma mask.
> >> by calling dma_alloc_coherent_mask(dev, flag)
> > 
> > Well, that's because I wrote gart's alloc_coherent and introduced
> > dma_alloc_coherent_mask. ;)
> 
> oh. it is in tip

Yeah, it's in tip. But the current gart code uses coherent_dma_mask
properly (GART in tip works in the same way as the current
GART). IIRC, other X86 hardware IOMMUs (VT-d, Calgary, AMD) uses
dma_mask in dma_alloc_coherent (AMD in tip uses coherent_dma_mask).



> >> if device could use 64 bit coherent dma mask, that is driver problem...
> > 
> > Can you be more specific? As I wrote above, if 64bit-dma-capable
> > devices don't set consistent_dma_mask, we don't have any problem.
> 
> then can we remove consistent_dma_mask? just use dma_mask instead for all.

I don't think we can. One architecture needs it. The above usb
chip seems to need it.

Re: [PATCH] pci: print out DMA mask info

[Grant Grundler]

On Fri, Oct 10, 2008 at 04:32:07PM +0900, FUJITA Tomonori wrote:
...
> > > Well, that's because I wrote gart's alloc_coherent and introduced
> > > dma_alloc_coherent_mask. ;)
> > 
> > oh. it is in tip
> 
> Yeah, it's in tip. But the current gart code uses coherent_dma_mask
> properly (GART in tip works in the same way as the current
> GART). IIRC, other X86 hardware IOMMUs (VT-d, Calgary, AMD) uses
> dma_mask in dma_alloc_coherent (AMD in tip uses coherent_dma_mask).

This is a bug. Care to submit a patch to fix it?
I don't have any of the docs or HW (VT-d, Calgary) to test it.


> > then can we remove consistent_dma_mask? just use dma_mask instead for all.
> 
> I don't think we can. One architecture needs it. The above usb
> chip seems to need it.

As noted before, several PCI-X devices support 64-bit DMA for payload
data but only 32-bit DMA for control data.

thanks,
grant

Re: [PATCH] pci: print out DMA mask info

[Grant Grundler]

On Fri, Oct 10, 2008 at 11:40:56AM +0900, FUJITA Tomonori wrote:
> IIRC, except for one SGI architecture, coherent_dma_mask is
> meaningless, dma_mask is always equal to coherent_dma_mask.

Not correct. Several PCI-X devices can only do 32-bit DMA to control
data but can do 64-bit DMA for payload data. I don't have the list off
the top of my head but that is the origin of coherent DMA mask.

> Lots of
> IOMMU implementations ignore coherent_dma_mask and use dma_mask for
> alloc_coherent().

That sounds like a bug. And I don't think it's "lots".

> Some drivers don't set up coherent_dma_mask.

They probably should. Current default is 32-bits for PCI devices.

> Theoretically, we need to fix this but it doesn't cause any
> problem. That's why nobody cares about it, I guess.

Agreed - for the "supported" configurations, it works.

hth,
grant

Re: [PATCH] pci: print out DMA mask info

[FUJITA Tomonori]

On Fri, 10 Oct 2008 16:45:08 -0600
Grant Grundler <grundler@parisc-linux.org> wrote:

> On Fri, Oct 10, 2008 at 11:40:56AM +0900, FUJITA Tomonori wrote:
> > IIRC, except for one SGI architecture, coherent_dma_mask is
> > meaningless, dma_mask is always equal to coherent_dma_mask.
> 
> Not correct. Several PCI-X devices can only do 32-bit DMA to control
> data but can do 64-bit DMA for payload data. I don't have the list off
> the top of my head but that is the origin of coherent DMA mask.

Thanks. I misunderstood coherent_dma_mask. I needed to read several
old mails (posted in 2002 and 2003) in ia64 mailing list.


> > Lots of
> > IOMMU implementations ignore coherent_dma_mask and use dma_mask for
> > alloc_coherent().
> 
> That sounds like a bug. And I don't think it's "lots".

Maybe it's not "lots" but seems that only IA64 and x86 non-hardware
IOMMU code (pci-nommu.c and GART) use coherent_dma_mask for
alloc_coherent. 

Re: [PATCH] pci: print out DMA mask info

[Grant Grundler]

On Thu, Oct 09, 2008 at 04:05:57PM -0700, Yinghai Lu wrote:
> Grant Grundler wrote:
> > On Thu, Oct 09, 2008 at 02:51:32PM -0700, Yinghai Lu wrote:
> >> On Thu, Oct 9, 2008 at 2:35 PM, Matthew Wilcox <matthew@wil.cx> wrote:
> >>> Why's that interesting to the sysadmin of the machine?  To the driver
> >>> writer, certainly.  But what's the use of it to the people using the
> >>> machine?
> > ...
> >> make linux kernel act like black box as other os?
> > 
> > I don't understand your reply.
> > If someone thinks linux is a black box, printing this message won't help them.
> > 
> could find out easily why some driver doesn't set dma mask correctly.
> like why 
> 	qlogic qla2xxx only set consistent to 64bit,
> 	emulex lpfc not set consistent to 64bit

I agree making the information available is a good idea. It's just
not going to help anyone 99% of the time and isn't a requirement for
booting the machine.

> > 
> > "To flag use of bounce buffer or other suboptimal behaviors" could be debated.
> > 
> > 
> > Regarding associating the output with other PCI messages, I'd hope the fact
> > that the /sys entry is in the same directory as other sys files would be
> > enough clue to associate those together. e.g.:
> > grundler <2068>cd /sys/bus/pci/devices/0000\:01\:00.0/
> > grundler <2069>ls
> > broken_parity_status  driver@  irq         resource0   subsystem_device
> > bus@                  enable   local_cpus  resource1   subsystem_vendor
> > class                 i2c-0/   modalias    resource3   uevent
> > config                i2c-1/   power/      rom         vendor
> > device                i2c-2/   resource    subsystem@
> > 
> 
> add dma_mask coherent_dma_mask here?

Yes, if there is no other more obvious place.
It's just a suggestion which I happen to think is better than
adding more boot time messages.

thanks,
grant

[PATCH] pci: show dma_mask bits in /sys

[Yinghai Lu]

Grant prefer to add it /sys instead of showing in bootlog

so could catch if the driver set the correct dma_mask.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/pci/pci-sysfs.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Index: linux-2.6/drivers/pci/pci-sysfs.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci-sysfs.c
+++ linux-2.6/drivers/pci/pci-sysfs.c
@@ -169,6 +169,21 @@ numa_node_show(struct device *dev, struc
 #endif
 
 static ssize_t
+dma_mask_bits_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+
+	return sprintf (buf, "%d\n", fls64(pdev->dma_mask));
+}
+
+static ssize_t
+consistent_dma_mask_bits_show(struct device *dev, struct device_attribute *attr,
+				 char *buf)
+{
+	return sprintf (buf, "%d\n", fls64(dev->coherent_dma_mask));
+}
+
+static ssize_t
 msi_bus_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
@@ -223,6 +238,8 @@ struct device_attribute pci_dev_attrs[]
 #ifdef CONFIG_NUMA
 	__ATTR_RO(numa_node),
 #endif
+	__ATTR_RO(dma_mask_bits),
+	__ATTR_RO(consistent_dma_mask_bits),
 	__ATTR(enable, 0600, is_enabled_show, is_enabled_store),
 	__ATTR(broken_parity_status,(S_IRUGO|S_IWUSR),
 		broken_parity_status_show,broken_parity_status_store),

Re: [PATCH] pci: show dma_mask bits in /sys

[Matthew Wilcox]

On Wed, Oct 22, 2008 at 06:45:10PM -0700, Yinghai Lu wrote:
> Grant prefer to add it /sys instead of showing in bootlog
> 
> so could catch if the driver set the correct dma_mask.

I still don't think this is useful information to be exposing.

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH] pci: show dma_mask bits in /sys

[Yinghai Lu]

On Wed, Oct 22, 2008 at 8:28 PM, Matthew Wilcox <matthew@wil.cx> wrote:
> On Wed, Oct 22, 2008 at 06:45:10PM -0700, Yinghai Lu wrote:
>> Grant prefer to add it /sys instead of showing in bootlog
>>
>> so could catch if the driver set the correct dma_mask.
>
> I still don't think this is useful information to be exposing.
>

RHEL 4.6 driver sata_mv is very run out of gart IOMMU, and need iommu
apterture to be 512M...
when loading is high, and it turns out every those card support 64bit
DMA, the driver DID NOT set dma to 64bit
so it still use 32 bit...

YH

Re: [PATCH] pci: show dma_mask bits in /sys

[Grant Grundler]

On Wed, Oct 22, 2008 at 09:28:33PM -0600, Matthew Wilcox wrote:
> On Wed, Oct 22, 2008 at 06:45:10PM -0700, Yinghai Lu wrote:
> > Grant prefer to add it /sys instead of showing in bootlog
> > 
> > so could catch if the driver set the correct dma_mask.
> 
> I still don't think this is useful information to be exposing.

It's useful for anyone involved with device drivers - we agree that's
a very limited subset of users.  I certainly get fed up with trying
to figure out which dma_mask each driver is using since in some
cases it might vary depending on which HW is installed and which
kernel is running.

If the /sys patch added a PCI_DEBUG option to the kernel,
would that work for you?
I'm not keen on that but just looking for alternatives.
Many other subsystems (SCSI, ACPI, PM, HID, ...) have a FOO_DEBUG option.

Lastly, /sys seems to have over run even /proc. On an rx2600 (ia64):
grundler@gsyprf3:/usr/src/linux-2.6$ find /sys ! -type d | wc
   6288    6296  264893
grundler@gsyprf3:/usr/src/linux-2.6$ find /proc ! -type d | wc
find: `/proc/tty/driver': Permission denied
...
   4104    4104  100797

Pruning some of the crap out of there sounds like another task suitable for
"kernel janitor" project.
/me runs and hides :)

thanks,
grant

Re: [PATCH] pci: show dma_mask bits in /sys

[Rolf Eike Beer]

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

Grant Grundler wrote:

> Lastly, /sys seems to have over run even /proc. On an rx2600 (ia64):
> grundler@gsyprf3:/usr/src/linux-2.6$ find /sys ! -type d | wc
>    6288    6296  264893

/sys is full of symlinks which /proc is not.

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] pci: show dma_mask bits in /sys

[Greg KH]

On Thu, Oct 23, 2008 at 12:44:12AM -0600, Grant Grundler wrote:
> 
> Lastly, /sys seems to have over run even /proc. On an rx2600 (ia64):
> grundler@gsyprf3:/usr/src/linux-2.6$ find /sys ! -type d | wc
>    6288    6296  264893

Is this causing some kind of problem for your systems?

/sys shows far more information than /proc does, so of course it would
be larger in quantity.

thanks,

greg k-h

Re: [PATCH] pci: show dma_mask bits in /sys

[H. Peter Anvin]

Greg KH wrote:
> On Thu, Oct 23, 2008 at 12:44:12AM -0600, Grant Grundler wrote:
>> Lastly, /sys seems to have over run even /proc. On an rx2600 (ia64):
>> grundler@gsyprf3:/usr/src/linux-2.6$ find /sys ! -type d | wc
>>    6288    6296  264893
> 
> Is this causing some kind of problem for your systems?
> 
> /sys shows far more information than /proc does, so of course it would
> be larger in quantity.
> 

Furthermore, it is actually structured as individual data items; a lot 
of /proc files are aggregate freeform data.

	-hpa

Re: [PATCH] pci: show dma_mask bits in /sys

[Grant Grundler]

On Thu, Oct 23, 2008 at 08:39:23AM -0700, Greg KH wrote:
> On Thu, Oct 23, 2008 at 12:44:12AM -0600, Grant Grundler wrote:
> > 
> > Lastly, /sys seems to have over run even /proc. On an rx2600 (ia64):
> > grundler@gsyprf3:/usr/src/linux-2.6$ find /sys ! -type d | wc
> >    6288    6296  264893
> 
> Is this causing some kind of problem for your systems?

Nope. Just an observation on the "forest" (vs individual "trees").

> /sys shows far more information than /proc does, so of course it would
> be larger in quantity.

Yes - and /sys is better organized (hierarchial in general).
But it's getting harder to find stuff because there is so much now.
Any comments on adding DMA attributes (e.g. dma_mask_bits) to /sys?

Willy has objected and I'm waiting to hear if he's ok with limiting
this to a DEBUG compile time flag.

thanks,
grant

Re: [PATCH] pci: show dma_mask bits in /sys

[Greg KH]

On Thu, Oct 23, 2008 at 01:36:52PM -0600, Grant Grundler wrote:
> On Thu, Oct 23, 2008 at 08:39:23AM -0700, Greg KH wrote:
> > On Thu, Oct 23, 2008 at 12:44:12AM -0600, Grant Grundler wrote:
> > > 
> > > Lastly, /sys seems to have over run even /proc. On an rx2600 (ia64):
> > > grundler@gsyprf3:/usr/src/linux-2.6$ find /sys ! -type d | wc
> > >    6288    6296  264893
> > 
> > Is this causing some kind of problem for your systems?
> 
> Nope. Just an observation on the "forest" (vs individual "trees").
> 
> > /sys shows far more information than /proc does, so of course it would
> > be larger in quantity.
> 
> Yes - and /sys is better organized (hierarchial in general).
> But it's getting harder to find stuff because there is so much now.
> Any comments on adding DMA attributes (e.g. dma_mask_bits) to /sys?

Sorry, wasn't paying attention.  All I ask is that if you do add new
sysfs files, you document them in the proper format in
Documentation/ABI/

thanks,

greg k-h

Re: [PATCH] pci: show dma_mask bits in /sys

[Andi Kleen]

Grant Grundler <grundler@parisc-linux.org> writes:

> On Wed, Oct 22, 2008 at 09:28:33PM -0600, Matthew Wilcox wrote:
>> On Wed, Oct 22, 2008 at 06:45:10PM -0700, Yinghai Lu wrote:
>> > Grant prefer to add it /sys instead of showing in bootlog
>> > 
>> > so could catch if the driver set the correct dma_mask.
>> 
>> I still don't think this is useful information to be exposing.
>
> It's useful for anyone involved with device drivers - we agree that's
> a very limited subset of users.  I certainly get fed up with trying
> to figure out which dma_mask each driver is using since in some

One simple way that doesn't need any kernel changes is to use
crash on the running kernel. I personally just stuck in printks
when I needed that, but it doesn't strike me as something that 
is generally useful in standard kernels (agreeing with Willy)

-Andi
-- 
ak@linux.intel.com

Re: [PATCH] pci: show dma_mask bits in /sys

[Grant Grundler]

On Fri, Oct 24, 2008 at 12:50:12PM +0200, Andi Kleen wrote:
> Grant Grundler <grundler@parisc-linux.org> writes:
> 
> > On Wed, Oct 22, 2008 at 09:28:33PM -0600, Matthew Wilcox wrote:
> >> On Wed, Oct 22, 2008 at 06:45:10PM -0700, Yinghai Lu wrote:
> >> > Grant prefer to add it /sys instead of showing in bootlog
> >> > 
> >> > so could catch if the driver set the correct dma_mask.
> >> 
> >> I still don't think this is useful information to be exposing.
> >
> > It's useful for anyone involved with device drivers - we agree that's
> > a very limited subset of users.  I certainly get fed up with trying
> > to figure out which dma_mask each driver is using since in some
> 
> One simple way that doesn't need any kernel changes is to use
> crash on the running kernel.

I would agree if someone can show me how to script crash to dump
all the struct pci_dev DMA masks and associate that output with
a driver and device instance.

> I personally just stuck in printks
> when I needed that, but it doesn't strike me as something that 
> is generally useful in standard kernels (agreeing with Willy)

Ok.

thanks,
grant

Re: [PATCH] pci: show dma_mask bits in /sys

[Grant Grundler]

On Wed, Oct 22, 2008 at 06:45:10PM -0700, Yinghai Lu wrote:
> Grant prefer to add it /sys instead of showing in bootlog
> 
> so could catch if the driver set the correct dma_mask.
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> ---
>  drivers/pci/pci-sysfs.c |   17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> Index: linux-2.6/drivers/pci/pci-sysfs.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/pci-sysfs.c
> +++ linux-2.6/drivers/pci/pci-sysfs.c
> @@ -169,6 +169,21 @@ numa_node_show(struct device *dev, struc
>  #endif
>  
>  static ssize_t
> +dma_mask_bits_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +
> +	return sprintf (buf, "%d\n", fls64(pdev->dma_mask));

Is there any reason to use %d instead of "0x%x" ?
I'd much rather see this dumped in hex.

> +}
> +
> +static ssize_t
> +consistent_dma_mask_bits_show(struct device *dev, struct device_attribute *attr,
> +				 char *buf)
> +{
> +	return sprintf (buf, "%d\n", fls64(dev->coherent_dma_mask));
> +}
> +
> +static ssize_t
>  msi_bus_show(struct device *dev, struct device_attribute *attr, char *buf)
>  {
>  	struct pci_dev *pdev = to_pci_dev(dev);
> @@ -223,6 +238,8 @@ struct device_attribute pci_dev_attrs[]
>  #ifdef CONFIG_NUMA
>  	__ATTR_RO(numa_node),
>  #endif
> +	__ATTR_RO(dma_mask_bits),
> +	__ATTR_RO(consistent_dma_mask_bits),
>  	__ATTR(enable, 0600, is_enabled_show, is_enabled_store),
>  	__ATTR(broken_parity_status,(S_IRUGO|S_IWUSR),
>  		broken_parity_status_show,broken_parity_status_store),

Otherwise looks good to me.

thanks,
grant

Re: [PATCH] pci: show dma_mask bits in /sys

[Yinghai Lu]

On Wed, Oct 22, 2008 at 11:48 PM, Grant Grundler
<grundler@parisc-linux.org> wrote:
> On Wed, Oct 22, 2008 at 06:45:10PM -0700, Yinghai Lu wrote:
>> Grant prefer to add it /sys instead of showing in bootlog
>>
>> so could catch if the driver set the correct dma_mask.
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>
>> ---
>>  drivers/pci/pci-sysfs.c |   17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> Index: linux-2.6/drivers/pci/pci-sysfs.c
>> ===================================================================
>> --- linux-2.6.orig/drivers/pci/pci-sysfs.c
>> +++ linux-2.6/drivers/pci/pci-sysfs.c
>> @@ -169,6 +169,21 @@ numa_node_show(struct device *dev, struc
>>  #endif
>>
>>  static ssize_t
>> +dma_mask_bits_show(struct device *dev, struct device_attribute *attr, char *buf)
>> +{
>> +     struct pci_dev *pdev = to_pci_dev(dev);
>> +
>> +     return sprintf (buf, "%d\n", fls64(pdev->dma_mask));
>
> Is there any reason to use %d instead of "0x%x" ?
> I'd much rather see this dumped in hex.

that is bits, so it will be 64, 41 instead of 0xfffff

YH

Re: [PATCH] pci: show dma_mask bits in /sys

[Grant Grundler]

On Wed, Oct 22, 2008 at 11:51:36PM -0700, Yinghai Lu wrote:
> On Wed, Oct 22, 2008 at 11:48 PM, Grant Grundler
> <grundler@parisc-linux.org> wrote:
> > On Wed, Oct 22, 2008 at 06:45:10PM -0700, Yinghai Lu wrote:
> >> Grant prefer to add it /sys instead of showing in bootlog
> >>
> >> so could catch if the driver set the correct dma_mask.
> >>
> >> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> >>
> >> ---
> >>  drivers/pci/pci-sysfs.c |   17 +++++++++++++++++
> >>  1 file changed, 17 insertions(+)
> >>
> >> Index: linux-2.6/drivers/pci/pci-sysfs.c
> >> ===================================================================
> >> --- linux-2.6.orig/drivers/pci/pci-sysfs.c
> >> +++ linux-2.6/drivers/pci/pci-sysfs.c
> >> @@ -169,6 +169,21 @@ numa_node_show(struct device *dev, struc
> >>  #endif
> >>
> >>  static ssize_t
> >> +dma_mask_bits_show(struct device *dev, struct device_attribute *attr, char *buf)
> >> +{
> >> +     struct pci_dev *pdev = to_pci_dev(dev);
> >> +
> >> +     return sprintf (buf, "%d\n", fls64(pdev->dma_mask));
> >
> > Is there any reason to use %d instead of "0x%x" ?
> > I'd much rather see this dumped in hex.
> 
> that is bits, so it will be 64, 41 instead of 0xfffff

Sorry - I missed the fls64(). Looks good to me.

We only need to know if willy agrees to how/when to
limit when this is available.

thanks,
grant

Re: [PATCH] pci: print out DMA mask info

[FUJITA Tomonori]

On Thu, 09 Oct 2008 14:27:33 -0700
Yinghai Lu <yinghai@kernel.org> wrote:

> Grant Grundler wrote:
> > On Wed, Oct 08, 2008 at 04:02:23PM -0700, Yinghai Lu wrote:
> >> so can find out what is DMA mask is used for pci devices in addition to
> >> default setting.
> > 
> > I like the idea. I don't like the additional boot time output.
> > 
> > But I'm thinking this could be an option to lspci.
> > lspci already knows about the /sys tree. Can PCI export the two masks
> > (dma_mask and dma_consistent_mask) and something like "lspci -td"
> > would dump those in a nice way?
> > 
> in boot log, it could link with driver, pci info...
> > 
> >> got:
> >> aacraid 0000:86:00.0: using 32bit DMA mask
> >> aacraid 0000:86:00.0: using 32bit consistent DMA mask
> >> aacraid 0000:86:00.0: using 64bit DMA mask
> >> aacraid 0000:86:00.0: using 64bit consistent DMA mask
> 
> and aacraid do 32bit at first then 64bit...
> 
> so put that in boot message could be useful too.

IIRC, aacraid uses 32bit dma_mask at startup then it executes a
special command to get dma information from the card, and sets proper
dma_mask. So the above message is not wrong for aacraid, I think.

Re: [PATCH] pci: print out DMA mask info

[FUJITA Tomonori]

On Fri, 10 Oct 2008 11:40:51 +0900
FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:

> On Thu, 09 Oct 2008 14:27:33 -0700
> Yinghai Lu <yinghai@kernel.org> wrote:
> 
> > Grant Grundler wrote:
> > > On Wed, Oct 08, 2008 at 04:02:23PM -0700, Yinghai Lu wrote:
> > >> so can find out what is DMA mask is used for pci devices in addition to
> > >> default setting.
> > > 
> > > I like the idea. I don't like the additional boot time output.
> > > 
> > > But I'm thinking this could be an option to lspci.
> > > lspci already knows about the /sys tree. Can PCI export the two masks
> > > (dma_mask and dma_consistent_mask) and something like "lspci -td"
> > > would dump those in a nice way?
> > > 
> > in boot log, it could link with driver, pci info...
> > > 
> > >> got:
> > >> aacraid 0000:86:00.0: using 32bit DMA mask
> > >> aacraid 0000:86:00.0: using 32bit consistent DMA mask
> > >> aacraid 0000:86:00.0: using 64bit DMA mask
> > >> aacraid 0000:86:00.0: using 64bit consistent DMA mask
> > 
> > and aacraid do 32bit at first then 64bit...
> > 
> > so put that in boot message could be useful too.
> 
> IIRC, aacraid uses 32bit dma_mask at startup then it executes a
> special command to get dma information from the card, and sets proper
> dma_mask. So the above message is not wrong for aacraid, I think.

Oh, what you wanted to say is, "pci uses 32bit dma_mask and
dma_consistent_mask by default so aacraid doesn't need to set dma_mask
and dma_consistent_mask at startup", then it's true. You can remove
it. But it's just harmless unnecessary code.

But again, I don't think this is a good reason to print such dma
information for everyone at boot time.

Re: [PATCH] pci: print out DMA mask info

[FUJITA Tomonori]

On Thu, 9 Oct 2008 15:18:37 -0600
Grant Grundler <grundler@parisc-linux.org> wrote:

> On Wed, Oct 08, 2008 at 04:02:23PM -0700, Yinghai Lu wrote:
> > so can find out what is DMA mask is used for pci devices in addition to
> > default setting.
> 
> I like the idea. I don't like the additional boot time output.
> 
> But I'm thinking this could be an option to lspci.
> lspci already knows about the /sys tree. Can PCI export the two masks
> (dma_mask and dma_consistent_mask) and something like "lspci -td"
> would dump those in a nice way?
> 
> Anyone else agree?

Agreed.

dma_mask and dma_consistent_mask is not useful for the
majority. Printing them at boot time doesn't make sense for me.

Adding under sysfs and using lspci sounds reasonable if we really need
these information.

Re: [PATCH] pci: print out DMA mask info

[Jesse Barnes]

On Thursday, October 9, 2008 7:40 pm FUJITA Tomonori wrote:
> On Thu, 9 Oct 2008 15:18:37 -0600
>
> Grant Grundler <grundler@parisc-linux.org> wrote:
> > On Wed, Oct 08, 2008 at 04:02:23PM -0700, Yinghai Lu wrote:
> > > so can find out what is DMA mask is used for pci devices in addition to
> > > default setting.
> >
> > I like the idea. I don't like the additional boot time output.
> >
> > But I'm thinking this could be an option to lspci.
> > lspci already knows about the /sys tree. Can PCI export the two masks
> > (dma_mask and dma_consistent_mask) and something like "lspci -td"
> > would dump those in a nice way?
> >
> > Anyone else agree?
>
> Agreed.
>
> dma_mask and dma_consistent_mask is not useful for the
> majority. Printing them at boot time doesn't make sense for me.
>
> Adding under sysfs and using lspci sounds reasonable if we really need
> these information.

Another option would be to add a 'warn_dma_mask=0xxxxxxxxx' boot option that 
would trigger in the actual DMA mask setting routines.  It would let you 
choose the mask you'd like to see warnings about.  Yinghai?

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] pci: print out DMA mask info

[Yinghai Lu]

On Fri, Oct 10, 2008 at 8:48 AM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Thursday, October 9, 2008 7:40 pm FUJITA Tomonori wrote:
>> On Thu, 9 Oct 2008 15:18:37 -0600
>>
>> Grant Grundler <grundler@parisc-linux.org> wrote:
>> > On Wed, Oct 08, 2008 at 04:02:23PM -0700, Yinghai Lu wrote:
>> > > so can find out what is DMA mask is used for pci devices in addition to
>> > > default setting.
>> >
>> > I like the idea. I don't like the additional boot time output.
>> >
>> > But I'm thinking this could be an option to lspci.
>> > lspci already knows about the /sys tree. Can PCI export the two masks
>> > (dma_mask and dma_consistent_mask) and something like "lspci -td"
>> > would dump those in a nice way?
>> >
>> > Anyone else agree?
>>
>> Agreed.
>>
>> dma_mask and dma_consistent_mask is not useful for the
>> majority. Printing them at boot time doesn't make sense for me.
>>
>> Adding under sysfs and using lspci sounds reasonable if we really need
>> these information.
>
> Another option would be to add a 'warn_dma_mask=0xxxxxxxxx' boot option that
> would trigger in the actual DMA mask setting routines.  It would let you
> choose the mask you'd like to see warnings about.  Yinghai?
>

another command line parameter for debug?

anyway looking at dma_mask print out is interesting..., some uses 39,
and some use 44bits...

YH

Re: [PATCH] pci: print out DMA mask info

[Jesse Barnes]

On Friday, October 10, 2008 9:19 am Yinghai Lu wrote:
> another command line parameter for debug?

Sure, why not? :)

> anyway looking at dma_mask print out is interesting..., some uses 39,
> and some use 44bits...

Yeah, many devices have limitations that prevent them from doing full 64 or 32 
bit address cycles, so you'll see all sorts of values.


-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] pci: print out DMA mask info

[Matthew Wilcox]

On Fri, Oct 10, 2008 at 09:19:08AM -0700, Yinghai Lu wrote:
> anyway looking at dma_mask print out is interesting..., some uses 39,
> and some use 44bits...

Yes, that reflects the limitations of the device.  You can see many of
the common ones in include/linux/dma-mapping.h.

By the way, commit 8f286c33f1e838d631f4a3260b33efce4bc5973c is a really
bad idea.  It is very helpful to see which bitmasks are used by real
devices, and which ones aren't.  It should be reverted, IMO.

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH] pci: print out DMA mask info

[Yinghai Lu]

Matthew Wilcox wrote:
> 
> By the way, commit 8f286c33f1e838d631f4a3260b33efce4bc5973c is a really
> bad idea.  It is very helpful to see which bitmasks are used by real
> devices, and which ones aren't.  It should be reverted, IMO.
> 

why? did you point to wrong commit?

commit 8f286c33f1e838d631f4a3260b33efce4bc5973c
Author: Andrew Morton <akpm@linux-foundation.org>
Date:   Thu Oct 18 03:05:07 2007 -0700

    stop using DMA_xxBIT_MASK
    
    Now that we have DMA_BIT_MASK(), these macros are pointless.
    
    Cc: Jeremy Fitzhardinge <jeremy@goop.org>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 29b0285..101a2d4 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -13,9 +13,15 @@ enum dma_data_direction {
 	DMA_NONE = 3,
 };
 
-#define DMA_BIT_MASK(n)	((1ULL<<(n))-1)
+#define DMA_BIT_MASK(n)	(((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
 
-#define DMA_64BIT_MASK	(~0ULL)
+/*
+ * NOTE: do not use the below macros in new code and do not add new definitions
+ * here.
+ *
+ * Instead, just open-code DMA_BIT_MASK(n) within your driver
+ */
+#define DMA_64BIT_MASK	DMA_BIT_MASK(64)
 #define DMA_48BIT_MASK	DMA_BIT_MASK(48)
 #define DMA_47BIT_MASK	DMA_BIT_MASK(47)
 #define DMA_40BIT_MASK	DMA_BIT_MASK(40)

Re: [PATCH] pci: print out DMA mask info

[Matthew Wilcox]

On Fri, Oct 10, 2008 at 09:46:33AM -0700, Yinghai Lu wrote:
> Matthew Wilcox wrote:
> > 
> > By the way, commit 8f286c33f1e838d631f4a3260b33efce4bc5973c is a really
> > bad idea.  It is very helpful to see which bitmasks are used by real
> > devices, and which ones aren't.  It should be reverted, IMO.
> 
> why? did you point to wrong commit?

No, that's the commit I meant.

> commit 8f286c33f1e838d631f4a3260b33efce4bc5973c
> Author: Andrew Morton <akpm@linux-foundation.org>
> Date:   Thu Oct 18 03:05:07 2007 -0700
> 
> -#define DMA_BIT_MASK(n)	((1ULL<<(n))-1)
> +#define DMA_BIT_MASK(n)	(((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
>  
> -#define DMA_64BIT_MASK	(~0ULL)
> +/*
> + * NOTE: do not use the below macros in new code and do not add new definitions
> + * here.
> + *
> + * Instead, just open-code DMA_BIT_MASK(n) within your driver

Open-coding DMA_BIT_MASK() within your driver means that we no longer
have a canonical list of DMA masks in one place.  Instead we have to
grep the entire tree and come up with more-or-less complex scripts to
figure out which bit masks are actually in use.

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH] pci: print out DMA mask info

[Roland Dreier]

 > Open-coding DMA_BIT_MASK() within your driver means that we no longer
 > have a canonical list of DMA masks in one place.  Instead we have to
 > grep the entire tree and come up with more-or-less complex scripts to
 > figure out which bit masks are actually in use.

Out of curiousity, why do we care about that list?

 - R.

Re: [PATCH] pci: print out DMA mask info

[Grant Grundler]

On Fri, Oct 10, 2008 at 10:18:33AM -0700, Roland Dreier wrote:
>  > Open-coding DMA_BIT_MASK() within your driver means that we no longer
>  > have a canonical list of DMA masks in one place.  Instead we have to
>  > grep the entire tree and come up with more-or-less complex scripts to
>  > figure out which bit masks are actually in use.
> 
> Out of curiousity, why do we care about that list?

Originally, I used to track down IO perf issues when porting drivers to ia64.

But knowing the dma masks in use on specific platforms might allow
folks to disable DMA_ZONE (like ia64 since it's always guarateed
either a SWIOTLB or real IOMMU).

Any other uses?

thanks,
grant

> 
>  - R.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] pci: print out DMA mask info

[Grant Grundler]

On Fri, Oct 10, 2008 at 08:48:08AM -0700, Jesse Barnes wrote:
...
> Another option would be to add a 'warn_dma_mask=0xxxxxxxxx' boot option that 
> would trigger in the actual DMA mask setting routines.  It would let you 
> choose the mask you'd like to see warnings about.  Yinghai?

No, please don't. We have too many command line options already.

Dumping dma_mask and dma_consistent_mask from /sys is not hard.
I'd prefer Yinghai get credit for doing this since he's raised the issue
and took the time to submit a patch.

thanks,
grant