Re: [PATCH v3 1/3] x86/dom0: correctly set the maximum ->iomem_caps bound for PVH

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

On Thu, Feb 20, 2025 at 09:22:40AM +0100, Jan Beulich wrote:
> On 19.02.2025 17:48, Roger Pau Monne wrote:
> > The logic in dom0_setup_permissions() sets the maximum bound in
> > ->iomem_caps unconditionally using paddr_bits, which is not correct for HVM
> > based domains.  Instead use domain_max_paddr_bits() to get the correct
> > maximum paddr bits for each possible domain type.
> > 
> > Switch to using PFN_DOWN() instead of PAGE_SHIFT, as that's shorter.
> > 
> > Fixes: 53de839fb409 ('x86: constrain MFN range Dom0 may access')
> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > ---
> > The fixes tag might be dubious, IIRC at that time we had PVHv1 dom0, which
> > would likely also need such adjustment, but not the current PVHv2.
> 
> Probably better to omit it then. It would be one of the changes moving to
> PVHv2 that missed making the adjustment.

Well, PVHv1 would have needed such adjustment, as it was also limited
to hap_paddr_bits instead of paddr_bits.

> > --- a/xen/arch/x86/dom0_build.c
> > +++ b/xen/arch/x86/dom0_build.c
> > @@ -481,7 +481,8 @@ int __init dom0_setup_permissions(struct domain *d)
> >  
> >      /* The hardware domain is initially permitted full I/O capabilities. */
> >      rc = ioports_permit_access(d, 0, 0xFFFF);
> > -    rc |= iomem_permit_access(d, 0UL, (1UL << (paddr_bits - PAGE_SHIFT)) - 
> > 1);
> > +    rc |= iomem_permit_access(d, 0UL,
> > +                              PFN_DOWN(1UL << domain_max_paddr_bits(d)) - 
> > 1);
> 
> Why PFN_DOWN() rather than subtracting PAGE_SHIFT? That's two shifts rather
> than just one.

cosmetic: line length (it's mentioned in the commit message).  I can
switch back to PAGE_SHIFT, didn't think it was a big deal since it's
a one time only calculation.

> Personally I'd prefer if we continued using the subtraction,
> but either way:
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks, will switch back to PAGE_SHIFT if it doesn't turn out to be
too ugly.