Query regarding modifying the DMA Mask based on the available memory in the system

Query regarding modifying the DMA Mask based on the available memory in the system

[Prakash, Sathya]

Hi All,
Currently the MPT Fusion drivers set DMA Mask to 64 bit if the
architecture of the system is 64bit and if the hardware supports 64 bit
DMA address. 
Also in all 64 bit systems the fusion drivers use SGESimple64_t to send
scatter gather elements to firmware. 

Even with 64 bit systems which has available memory less than 4GB the
SGESimple64_t is used, to increase the performance I am thinking of
modifying the driver to check the system memory in the system using the
function si_meminfo() and if the memory is less than 4GB, then the
driver will set the 32 bit DMA mask and will use SGESimple32_t to send
the SGE to firmware. 

I am not sure whether this change works fine? I couldn't see any SCSI
driver using the si_meminfo and this function seems not to return the
physical layout of the memory. 

Thanks
Sathya

Re: Query regarding modifying the DMA Mask based on the available memory in the system

[David Miller]

From: "Prakash, Sathya" <Sathya.Prakash@lsi.com>
Date: Thu, 17 Apr 2008 16:40:43 +0800

> I am not sure whether this change works fine? I couldn't see any SCSI
> driver using the si_meminfo and this function seems not to return the
> physical layout of the memory. 

Physical addressing has no connection with the types of addresses
that IOMMU based systems will give you, which are virtual and
can be above 4GB on 64-bit systems.

Re: Query regarding modifying the DMA Mask based on the available memory in the system

[Andi Kleen]

"Prakash, Sathya" <Sathya.Prakash@lsi.com> writes:

> Hi All,
> Currently the MPT Fusion drivers set DMA Mask to 64 bit if the
> architecture of the system is 64bit and if the hardware supports 64 bit
> DMA address. 
> Also in all 64 bit systems the fusion drivers use SGESimple64_t to send
> scatter gather elements to firmware. 
>
> Even with 64 bit systems which has available memory less than 4GB the

Actually it doesn't depend on the available memory, but where the
memory is mapped. In practice on x86 system due to the PCI memory hole
if you have >3GB (and sometimes >2GB) you already have memory mapped
beyond the 4GB boundary.

> SGESimple64_t is used, to increase the performance I am thinking of
> modifying the driver to check the system memory in the system using the
> function si_meminfo() and if the memory is less than 4GB, then the
> driver will set the 32 bit DMA mask and will use SGESimple32_t to send
> the SGE to firmware. 
>
> I am not sure whether this change works fine? I couldn't see any SCSI
> driver using the si_meminfo and this function seems not to return the
> physical layout of the memory. 

As David pointed out it wouldn't work on systems with a true IOMMU
(which will be more and in the future). I'm not sure it makes too much
sense to really optimize for the <4GB case on servers either because
these are getting rarer and rarer too at today's memory prices.

Anyways if you really still want to do this the right way would
be probably some new interface in the PCI-DMA API that DTRT 
with IOMMU (nothing) and without. The x86-64 pci dma
code already has some boot options to force this (iommu=forcesac), 
but it currently only works with the AMD GART driver.

I guess it could make somer sense to be able to control this from
the driver. e.g. something like pci_dma_prefer_mask(....) 
and the low level architecture would return if that makes
sense on the current setup or not. Wouldn't be very hard
to implement. But again I'm not sure it's really a good idea
because all the trends are against this.

Please don't do a #ifdef driver specific hack.

-Andi

Re: Query regarding modifying the DMA Mask based on the available memory in the system

[James Bottomley]

On Thu, 2008-04-17 at 16:40 +0800, Prakash, Sathya wrote:
> Currently the MPT Fusion drivers set DMA Mask to 64 bit if the
> architecture of the system is 64bit and if the hardware supports 64 bit
> DMA address. 
> Also in all 64 bit systems the fusion drivers use SGESimple64_t to send
> scatter gather elements to firmware. 
> 
> Even with 64 bit systems which has available memory less than 4GB the
> SGESimple64_t is used, to increase the performance I am thinking of
> modifying the driver to check the system memory in the system using the
> function si_meminfo() and if the memory is less than 4GB, then the
> driver will set the 32 bit DMA mask and will use SGESimple32_t to send
> the SGE to firmware. 
> 
> I am not sure whether this change works fine? I couldn't see any SCSI
> driver using the si_meminfo and this function seems not to return the
> physical layout of the memory. 

That's because it's not the right way to do this.

For drivers that can alter their descriptor types, we have this
function:

dma_get_required_mask()

It returns the largest mask necessary to reach all of memory.  Note:
This isn't necessarily the same as the largest physical memory on
systems because some systems with IOMMUs deliberately always return
addresses in 32 bits even in a 64 bit system.

For a usage example, see:

aic79xx_osm_pci.c:ahd_linux_pci_dev_probe()

The aic79xx has three possible descriptor formats:  A 32 bit one; a
packed 39 bit one and a full 64 bit one (which takes double the space).
It uses the above function to find out what it needs to use (the 64 bit
descriptor is operationally slower on the chip).

James

Re: Query regarding modifying the DMA Mask based on the available memory in the system

[Andi Kleen]

>
> For drivers that can alter their descriptor types, we have this
> function:
>
> dma_get_required_mask()

Are you sure we have it? It seems to be in drivers/base/platform.c
conditional on ARCH_HAS_DMA_GET_REQUIRED_MASK, but according to my
grep no architecture ever sets that flag.

I don't think it would be very hard to implement on x86 at least,
mind you. Just nobody seems to have done it so far.

-Andi

Re: Query regarding modifying the DMA Mask based on the available memory in the system

[James Bottomley]

On Thu, 2008-04-17 at 16:44 +0200, Andi Kleen wrote:
> >
> > For drivers that can alter their descriptor types, we have this
> > function:
> >
> > dma_get_required_mask()
> 
> Are you sure we have it? It seems to be in drivers/base/platform.c
> conditional on ARCH_HAS_DMA_GET_REQUIRED_MASK, but according to my
> grep no architecture ever sets that flag.

Yes ... positive.  That's the default and correct implementation based
on the largest addressable physical memory.

> I don't think it would be very hard to implement on x86 at least,
> mind you. Just nobody seems to have done it so far.

Really, only machines with IOMMUs that want to restrict this need
implement it.  Current safety is predicated on the assumption that no
architectures with IOMMUs never return bus physical addresses above
memory physical.  If that's not true, then they *have* to implement
this.  For all others its an optimisation to return a more restricted
range.

James

Re: Query regarding modifying the DMA Mask based on the available memory in the system

[Andi Kleen]

James Bottomley wrote:
> On Thu, 2008-04-17 at 16:44 +0200, Andi Kleen wrote:
>>> For drivers that can alter their descriptor types, we have this
>>> function:
>>>
>>> dma_get_required_mask()
>> Are you sure we have it? It seems to be in drivers/base/platform.c
>> conditional on ARCH_HAS_DMA_GET_REQUIRED_MASK, but according to my
>> grep no architecture ever sets that flag.
> 
> Yes ... positive.  That's the default and correct implementation based
> on the largest addressable physical memory.

Ok but still the whole thing is completely useless right now.

>> I don't think it would be very hard to implement on x86 at least,
>> mind you. Just nobody seems to have done it so far.
> 
> Really, only machines with IOMMUs that want to restrict this need
> implement it. 

I don't think so. For once in the scenario described by the original
poster it makes some sense even without IOMMU.

-Andi

Re: Query regarding modifying the DMA Mask based on the available memory in the system

[James Bottomley]

On Thu, 2008-04-17 at 17:06 +0200, Andi Kleen wrote:
> James Bottomley wrote:
> > On Thu, 2008-04-17 at 16:44 +0200, Andi Kleen wrote:
> >>> For drivers that can alter their descriptor types, we have this
> >>> function:
> >>>
> >>> dma_get_required_mask()
> >> Are you sure we have it? It seems to be in drivers/base/platform.c
> >> conditional on ARCH_HAS_DMA_GET_REQUIRED_MASK, but according to my
> >> grep no architecture ever sets that flag.
> > 
> > Yes ... positive.  That's the default and correct implementation based
> > on the largest addressable physical memory.
> 
> Ok but still the whole thing is completely useless right now.

Why?  At least for aic it performs as advertised: allows the driver to
select the most efficient descriptor format.

> >> I don't think it would be very hard to implement on x86 at least,
> >> mind you. Just nobody seems to have done it so far.
> > 
> > Really, only machines with IOMMUs that want to restrict this need
> > implement it. 
> 
> I don't think so. For once in the scenario described by the original
> poster it makes some sense even without IOMMU.

OK, perhaps I don't understand something then:  The fusion is
essentially similar to the aic except it doesn't have the 39 bit packed
descriptor format, so it wants to use the 32 bit format (which is
faster) if no physical memory will be over 4GB.  That's what the
dma_get_required_mask() was exactly designed for and how it is currently
being used.

If a machine has no iommu how can the required mask be different from
the lowest mask that will reach all of physical memory?

James

Re: Query regarding modifying the DMA Mask based on the available memory in the system

[Andi Kleen]

James Bottomley wrote:
> On Thu, 2008-04-17 at 17:06 +0200, Andi Kleen wrote:
>> James Bottomley wrote:
>>> On Thu, 2008-04-17 at 16:44 +0200, Andi Kleen wrote:
>>>>> For drivers that can alter their descriptor types, we have this
>>>>> function:
>>>>>
>>>>> dma_get_required_mask()
>>>> Are you sure we have it? It seems to be in drivers/base/platform.c
>>>> conditional on ARCH_HAS_DMA_GET_REQUIRED_MASK, but according to my
>>>> grep no architecture ever sets that flag.
>>> Yes ... positive.  That's the default and correct implementation based
>>> on the largest addressable physical memory.
>> Ok but still the whole thing is completely useless right now.
> 
> Why?  At least for aic it performs as advertised: allows the driver to
> select the most efficient descriptor format.

You were right sorry, I misread the code. Your interpretation
is correct. The interface would be correct to use for fusion,
although it should be probably extended to IOMMUs too.

-Andi

RE: Query regarding modifying the DMA Mask based on the available memory in the system

[Prakash, Sathya]

Thanks everyone. 
I will use the dma_get_required_mask() for deciding fusion's descriptor
format

-----Original Message-----
From: Andi Kleen [mailto:andi@firstfloor.org] 
Sent: Thursday, April 17, 2008 9:41 PM
To: James Bottomley
Cc: Prakash, Sathya; linux-scsi@vger.kernel.org
Subject: Re: Query regarding modifying the DMA Mask based on the
available memory in the system

James Bottomley wrote:
> On Thu, 2008-04-17 at 17:06 +0200, Andi Kleen wrote:
>> James Bottomley wrote:
>>> On Thu, 2008-04-17 at 16:44 +0200, Andi Kleen wrote:
>>>>> For drivers that can alter their descriptor types, we have this
>>>>> function:
>>>>>
>>>>> dma_get_required_mask()
>>>> Are you sure we have it? It seems to be in drivers/base/platform.c 
>>>> conditional on ARCH_HAS_DMA_GET_REQUIRED_MASK, but according to my 
>>>> grep no architecture ever sets that flag.
>>> Yes ... positive.  That's the default and correct implementation 
>>> based on the largest addressable physical memory.
>> Ok but still the whole thing is completely useless right now.
> 
> Why?  At least for aic it performs as advertised: allows the driver to

> select the most efficient descriptor format.

You were right sorry, I misread the code. Your interpretation is
correct. The interface would be correct to use for fusion, although it
should be probably extended to IOMMUs too.

-Andi