Re: [PATCH v5 1/9] vpci: move BAR mapping permissions checks

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.07.2026 16:57, Roger Pau Monné wrote:
> On Thu, Jul 23, 2026 at 04:44:26PM +0200, Jan Beulich wrote:
>> On 08.07.2026 23:02, Stewart Hildebrand wrote:
>>> --- a/xen/drivers/vpci/header.c
>>> +++ b/xen/drivers/vpci/header.c
>>> @@ -58,24 +58,6 @@ static int cf_check map_range(
>>>           * offset of the current address from the BAR start.
>>>           */
>>>          unsigned long map_mfn = start_mfn + s - start_gfn;
>>> -        unsigned long m_end = map_mfn + size - 1;
>>> -
>>> -        if ( !iomem_access_permitted(map->d, map_mfn, m_end) )
>>> -        {
>>> -            printk(XENLOG_G_WARNING
>>> -                   "%pd denied access to MMIO range [%#lx, %#lx]\n",
>>> -                   map->d, map_mfn, m_end);
>>> -            return -EPERM;
>>> -        }
>>> -
>>> -        rc = xsm_iomem_mapping_vpci(XSM_HOOK, map->d, map_mfn, m_end, 
>>> map->map);
>>> -        if ( rc )
>>> -        {
>>> -            printk(XENLOG_G_WARNING
>>> -                   "%pd XSM denied access to MMIO range [%#lx, %#lx]: 
>>> %d\n",
>>> -                   map->d, map_mfn, m_end, rc);
>>
>> I realize you literally only take what's here, but ...
>>
>>> @@ -369,6 +351,28 @@ static int modify_bars(const struct pci_dev *pdev, 
>>> uint16_t cmd, bool rom_only)
>>>              return -EINVAL;
>>>          }
>>>  
>>> +        if ( !iomem_access_permitted(pdev->domain, start, end) )
>>> +        {
>>> +            printk(XENLOG_G_WARNING
>>> +                   "%pd denied access to MMIO range [%#lx, %#lx]\n",
>>> +                   pdev->domain, start, end);
>>> +            if ( !is_hardware_domain(pdev->domain) )
>>> +                domain_crash(pdev->domain);
>>> +            return -EPERM;
>>> +        }
>>> +
>>> +        rc = xsm_iomem_mapping_vpci(XSM_HOOK, pdev->domain, start, end,
>>> +                                    !!(cmd & PCI_COMMAND_MEMORY));
>>> +        if ( rc )
>>> +        {
>>> +            printk(XENLOG_G_WARNING
>>> +                   "%pd XSM denied access to MMIO range [%#lx, %#lx]: 
>>> %d\n",
>>> +                   pdev->domain, start, end, rc);
>>
>> ... is the logging of rc actually useful? Afaict it's only ever going to be
>> -EPERM.
> 
> I think Flask can also return -EACCES, not that it's very relevant I'm
> afraid.

Hmm, looks like this can indeed happen. Please disregard this comment of
mine then.

>  It's likely too late now, but those XSM checks should instead
> return a bool to avoid this kind of confusion with the returned
> error? (or whether diverse return codes are indeed expected).

Depends on whether having distinct error codes is wanted. Daniel?

Jan