Re: [PATCH] dma: Fix max PFN arithmetic overflow on 32 bit systems

[Alexander Dahl]

Hello Robin,

On Thu, Mar 19, 2020 at 01:50:56PM +0000, Robin Murphy wrote:
> On 2020-03-02 6:16 pm, Alexander Dahl wrote:
> > ---
> >   arch/x86/include/asm/dma.h | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/include/asm/dma.h b/arch/x86/include/asm/dma.h
> > index 00f7cf45e699..e25514eca8d6 100644
> > --- a/arch/x86/include/asm/dma.h
> > +++ b/arch/x86/include/asm/dma.h
> > @@ -74,7 +74,7 @@
> >   #define MAX_DMA_PFN   ((16UL * 1024 * 1024) >> PAGE_SHIFT)
> >   /* 4GB broken PCI/AGP hardware bus master zone */
> > -#define MAX_DMA32_PFN ((4UL * 1024 * 1024 * 1024) >> PAGE_SHIFT)
> > +#define MAX_DMA32_PFN (4UL * ((1024 * 1024 * 1024) >> PAGE_SHIFT))
> 
> FWIW, wouldn't s/UL/ULL/ in the original expression suffice? Failing that,
> rather than awkward parenthesis trickery it might be clearer to just copy
> the one from arch/mips/include/asm/dma.h.

Both of your suggestions yield the correct result, and at least for me
both look easier to understand than what Alan proposed in the first
place. 

I would opt for the variant which is already in arch/mips, because it
avoids 64 bit calculation, is most obvious in intent in my eyes, and
we have the same calculation twice then instead of two variants.

Thanks for your review, I'll send a v2. :-)

Greets
Alex

-- 
/"\ ASCII RIBBON | »With the first link, the chain is forged. The first
\ / CAMPAIGN     | speech censured, the first thought forbidden, the
 X  AGAINST      | first freedom denied, chains us all irrevocably.«
/ \ HTML MAIL    | (Jean-Luc Picard, quoting Judge Aaron Satie)

Attachment: signature.asc
Description: PGP signature