On 20.02.2025 09:49, Roger Pau Monné wrote:
> 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....@citrix.com>
>>> ---
>>> 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.
Looks like I mis-interpreted your sentence then.
>>> --- 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).
Oh, I had overlooked that sentence there.
> I can
> switch back to PAGE_SHIFT, didn't think it was a big deal since it's
> a one time only calculation.
Feel free to keep as is then. I agree it's not a big deal here; my worry with
such
usually is that people seeing something in one place may then copy/clone the
same
to use elsewhere.
Jan
