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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

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

Jan