ethernet/sfc/ warnings with 32-bit dma_addr_t

ethernet/sfc/ warnings with 32-bit dma_addr_t

[Randy Dunlap]

Hi,

Does the drivers/net/ethernet/sfc/sfc driver require (expect)
dma_addr_t to be 64 bits (as opposed to 32 bits)?

I see that several #defines in ef100_regs.h are 64...

When used with DMA_BIT_MASK(64), does the value just need to be
truncated to 32 bits?  Will that work?


When I build this driver on i386 with 32-bit dma_addr_t, I see
the following build warnings:


  CC      drivers/net/ethernet/sfc/ef100.o
In file included from ../include/linux/skbuff.h:31:0,
                 from ../include/linux/if_ether.h:19,
                 from ../include/uapi/linux/ethtool.h:19,
                 from ../include/linux/ethtool.h:18,
                 from ../include/linux/netdevice.h:37,
                 from ../drivers/net/ethernet/sfc/net_driver.h:13,
                 from ../drivers/net/ethernet/sfc/ef100.c:12:
../drivers/net/ethernet/sfc/ef100.c: In function ‘ef100_pci_parse_continue_entry’:
../include/linux/dma-mapping.h:139:25: warning: large integer implicitly truncated to unsigned type [-Woverflow]
 #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
                         ^
../drivers/net/ethernet/sfc/ef100.c:145:6: note: in expansion of macro ‘DMA_BIT_MASK’
      DMA_BIT_MASK(ESF_GZ_TX_SEND_ADDR_WIDTH),
      ^~~~~~~~~~~~
../include/linux/dma-mapping.h:139:25: warning: large integer implicitly truncated to unsigned type [-Woverflow]
 #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
                         ^
../drivers/net/ethernet/sfc/ef100.c:163:6: note: in expansion of macro ‘DMA_BIT_MASK’
      DMA_BIT_MASK(ESF_GZ_TX_SEND_ADDR_WIDTH),
      ^~~~~~~~~~~~
../drivers/net/ethernet/sfc/ef100.c: In function ‘ef100_pci_parse_xilinx_cap’:
../include/linux/dma-mapping.h:139:25: warning: large integer implicitly truncated to unsigned type [-Woverflow]
 #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
                         ^
../drivers/net/ethernet/sfc/ef100.c:337:5: note: in expansion of macro ‘DMA_BIT_MASK’
     DMA_BIT_MASK(ESF_GZ_TX_SEND_ADDR_WIDTH),
     ^~~~~~~~~~~~
../drivers/net/ethernet/sfc/ef100.c: In function ‘ef100_pci_probe’:
../include/linux/dma-mapping.h:139:25: warning: large integer implicitly truncated to unsigned type [-Woverflow]
 #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
                         ^
../drivers/net/ethernet/sfc/ef100.c:498:5: note: in expansion of macro ‘DMA_BIT_MASK’
     DMA_BIT_MASK(ESF_GZ_TX_SEND_ADDR_WIDTH),
     ^~~~~~~~~~~~


thanks.
-- 
~Randy

Re: ethernet/sfc/ warnings with 32-bit dma_addr_t

[Edward Cree]

On 19/08/2020 01:28, Randy Dunlap wrote:
> Hi,
>
> Does the drivers/net/ethernet/sfc/sfc driver require (expect)
> dma_addr_t to be 64 bits (as opposed to 32 bits)?
>
> I see that several #defines in ef100_regs.h are 64...
>
> When used with DMA_BIT_MASK(64), does the value just need to be
> truncated to 32 bits?  Will that work?
As far as I can tell, truncation to 32 bits is harmless — the
 called function (efx_init_io) already tries every mask from the
 passed one down to 32 bits in case of PCIe hardware limitations.

The ef10 and siena versions also truncate like this (their
 #defines are 48 and 46 respectively), but because they are
 handled indirectly through efx_nic_type, the compiler isn't able
 to determine this statically as it can with ef100.
> When I build this driver on i386 with 32-bit dma_addr_t, I see
> the following build warnings:
Could you test whether explicitly casting to dma_addr_t suppresses
 the warnings?  I.e.

    efx_init_io(efx, bar,
                (dma_addr_t)DMA_BIT_MASK(ESF_GZ_TX_SEND_ADDR_WIDTH),
                pci_resource_len(efx->pci_dev, bar));

-ed

Re: ethernet/sfc/ warnings with 32-bit dma_addr_t

[Christoph Hellwig]

On Wed, Aug 19, 2020 at 11:37:00AM +0100, Edward Cree wrote:
> As far as I can tell, truncation to 32 bits is harmless ??? the
> ??called function (efx_init_io) already tries every mask from the
> ??passed one down to 32 bits in case of PCIe hardware limitations.

Which btw isn't needed.  These days dma_set_mask_and_coherent
and friends never fail because of a "too large" mask - it will
only fail in the rare case of a too small one.  So the two places
that set the dma mask could be simplified.

Re: ethernet/sfc/ warnings with 32-bit dma_addr_t

[Randy Dunlap]

On 8/19/20 3:37 AM, Edward Cree wrote:
> On 19/08/2020 01:28, Randy Dunlap wrote:
>> Hi,
>>
>> Does the drivers/net/ethernet/sfc/sfc driver require (expect)
>> dma_addr_t to be 64 bits (as opposed to 32 bits)?
>>
>> I see that several #defines in ef100_regs.h are 64...
>>
>> When used with DMA_BIT_MASK(64), does the value just need to be
>> truncated to 32 bits?  Will that work?
> As far as I can tell, truncation to 32 bits is harmless — the
>  called function (efx_init_io) already tries every mask from the
>  passed one down to 32 bits in case of PCIe hardware limitations.
> 
> The ef10 and siena versions also truncate like this (their
>  #defines are 48 and 46 respectively), but because they are
>  handled indirectly through efx_nic_type, the compiler isn't able
>  to determine this statically as it can with ef100.
>> When I build this driver on i386 with 32-bit dma_addr_t, I see
>> the following build warnings:
> Could you test whether explicitly casting to dma_addr_t suppresses
>  the warnings?  I.e.
> 
>     efx_init_io(efx, bar,
>                 (dma_addr_t)DMA_BIT_MASK(ESF_GZ_TX_SEND_ADDR_WIDTH),
>                 pci_resource_len(efx->pci_dev, bar));

Yes, that fixes the warnings.

thanks.
-- 
~Randy