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

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

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.

> 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().

> Furthermore this yet again allows DomU-s to fiddle with their p2m.
> To a degree this is unavoidable, I suppose. But some thought may
> need putting into this anyway. Aiui on real hardware if a BAR is
> placed over RAM, behavior is simply undefined. Once the BAR is
> moved away though, behavior will become defined again: The RAM will
> "reappear" in case the earlier undefined-ness made it disappear. I
> don't know how the Arm variants of the functions behave, but on x86
> the RAM pages will disappear from the guest's p2m upon putting a
> BAR there, but they won't reappear upon unmapping of the BAR.

Yeah, that's unfortunate, but I'm afraid it's the same behavior when
using QEMU, so I wouldn't consider it strictly a regression from the
current handling that we do for BARs when doing PCI passthrough.

Furthermore, I don't see any easy way to deal with this so that RAM
can be re-added when the BAR is re positioned to not overlap a RAM
region.

> Luckily at least preemption looks to be handled in a satisfactory
> manner already.

I spend quite a lot of time trying to make sure this was at least
attempted to solve properly.

Thanks, Roger.