Re: [PATCH] pdx: cast PAGE_SIZE value ahead of shifting

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Aug 14, 2025 at 09:18:45AM +0200, Jan Beulich wrote:
> On 13.08.2025 14:55, Roger Pau Monne wrote:
> > --- a/xen/common/pdx.c
> > +++ b/xen/common/pdx.c
> > @@ -288,7 +288,7 @@ bool __init pfn_pdx_compression_setup(paddr_t base)
> >  
> >      pfn_pdx_hole_shift  = hole_shift;
> >      pfn_pdx_bottom_mask = (1UL << bottom_shift) - 1;
> > -    ma_va_bottom_mask   = (PAGE_SIZE << bottom_shift) - 1;
> > +    ma_va_bottom_mask   = ((paddr_t)PAGE_SIZE << bottom_shift) - 1;
> 
> Given
> 
> #define PAGE_SIZE           (_AC(1,L) << PAGE_SHIFT)
> 
> this shouldn't be needed, except maybe for Arm32. There, however, ...
> 
> >      pfn_hole_mask       = ((1UL << hole_shift) - 1) << bottom_shift;
> 
> ... this and the shift immediately ahead would also be a problem afaict,
> which makes me conclude this isn't what Coverity has looked at. I expect
> the problem is with the toolstack side definition of PAGE_SIZE, which imo
> would rather be addressed there. (And yes, I'm pretty averse to arbitrary
> casts like this being introduced.)

As I've realized while looking at this, wouldn't ma_va_bottom_mask
also better be of type paddr_t, since it's not operating on pfns, but
physical addresses.  I didn't adjust the type of ma_va_bottom_mask,
but I would be happy to do it if you agree.

Thanks, roger.