Re: [PATCH v8 08/13] vpci/header: program p2m with guest BAR view

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.07.2023 15:16, Roger Pau Monné wrote:
> On Mon, Jul 24, 2023 at 12:43:26PM +0200, Jan Beulich wrote:
>> On 20.07.2023 02:32, Volodymyr Babchuk wrote:
>>> @@ -52,8 +66,8 @@ static int cf_check map_range(
>>>           * - {un}map_mmio_regions doesn't support preemption.
>>>           */
>>>  
>>> -        rc = map->map ? map_mmio_regions(map->d, _gfn(s), size, _mfn(s))
>>> -                      : unmap_mmio_regions(map->d, _gfn(s), size, _mfn(s));
>>> +        rc = map->map ? map_mmio_regions(map->d, start_gfn, size, _mfn(s))
>>> +                      : unmap_mmio_regions(map->d, start_gfn, size, 
>>> _mfn(s));
>>
>> Aiui this is the first direct exposure of these functions to DomU-s;
> 
> I guess it depends on how direct you consider exposure from
> XEN_DOMCTL_memory_mapping hypercall, as that's what gets called by
> QEMU also in order to set up BAR mappings.

Fair point - it is one of the few domctls not covered by XSA-77.

>> so far all calls were Xen-internal or from a domctl. There are a
>> couple of Arm TODOs listed in the comment ahead, but I'm not sure
>> that's all what is lacking here, and it's unclear whether this can
>> sensibly be left as a follow-on activity (at the very least known
>> open issues need mentioning as TODOs).
>>
>> For example the x86 function truncates an unsigned long local
>> variable to (signed) int in its main return statement. This may for
>> the moment still be only a theoretical issue, but will need dealing
>> with sooner or later, I think.
> 
> One bit that we need to add is the iomem_access_permitted() plus the
> xsm_iomem_mapping() checks to map_range().

The former would just be reassurance, wouldn't it? Assigning a PCI
device surely implies granting access to all its BARs (minus the
MSI-X page(s), if any). The latter would of course be more
"interesting", as XSM could in principle interject.

Jan