Re: [Xen-devel] vpci: deferral of register write until p2m changes are done

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

On Wed, Nov 28, 2018 at 09:22:16AM -0700, Jan Beulich wrote:
> >>> On 28.11.18 at 16:41, <roger.pau@xxxxxxxxxx> wrote:
> > On Wed, Nov 28, 2018 at 06:01:12AM -0700, Jan Beulich wrote:
> >> >>> On 28.11.18 at 11:09, <roger.pau@xxxxxxxxxx> wrote:
> >> > Hello,
> >> > 
> >> > While doing the recent vPCI fixes and also working on SR-IOV support
> >> > I've been thinking about how vPCI handles writes to PCI registers that
> >> > imply modifications to the p2m for PVH Dom0.
> >> > 
> >> > When memory decoding or ROM BARs are enabled Xen performs the
> >> > following flow:
> >> > 
> >> > 1. Create a rangeset with the memory regions that need to be
> >> > mapped/unmapped.
> >> > 2. Block the vCPU and perform the p2m changes in a preemptive way.
> >> > 3. After the p2m changes have been applied (or in case of error) write
> >> > to the register in order to enable/disable memory decoding or the ROM
> >> > BAR and mark the BARs as enabled.
> >> > 
> >> > I'm unsure about the benefit of deferring the register write (step 3)
> >> > for a PVH Dom0, so I would like to perform the register write before
> >> > applying the changes to the p2m.
> >> 
> >> As expressed while reviewing respective patches, I'm not sure either.
> >> Being not sure, putting ourselves on the safe side by disabling decode
> >> early and enabling decode late seems best to me though. Any
> >> deviation from this would imo require a conclusive discussion of why
> >> it is safe.
> > 
> > Right. So there are two different cases here:
> > 
> >  - Mapping: enabling memory decoding before mapping should have no
> >    effect on the guest, since the p2m entries for the BARs won't be
> >    set.
> 
> Depends on whether this is a first time map, or a movement. In the
> latter case it's the opposite of unmapping.

vPCI doesn't support BAR movements with memory decoding enabled ATM,
in order to move the position of a BAR memory decoding must be
disabled.

> >  - Unmapping: disabling the memory decoding bit before unmapping could
> >    allow the guest to access RAM or other MMIO regions that are
> >    exposed once the BARs are no longer mapped?
> 
> Disable before unmap is what we want for safety, so I'm not sure
> if you've simply mis-typed your reply. And disabling early is also in
> line with your desire of doing the cmd register writes first.

I agree, although I don't see the safety side of this. Disabling
before removing the p2m mappings gives the domain a window where it
can access whatever the BAR was mapped over, maybe a BAR from a
different device? In any case this won't work correctly to start
with.

> 
> >> In the interest of later enabling of the code for DomU, any
> >> such discussion should, as far as possible, avoid argumentation along
> >> the Dom0-only line.
> > 
> > My plan is that DomUs won't be allowed to toggle the memory decoding
> > bit, and it's going to be always enabled, like it's currently done for
> > pci-passthrough in QEMU. Toggling the memory decoding bit in a DomU is
> > going to trigger a change to the p2m (map or unmap) but the command
> > register will always have the memory decoding bit enabled.
> 
> But this isn't entirely correct, even if we've got away with this
> so far. But we're mostly considering well-behaved guests and
> devices. What if one actually triggers bus activity in parallel to
> a BAR change?

Well, that's likely to not work properly in any case with or without
disabling the memory decoding bit?

But I don't see how that's going to affect Xen stability (or what the
domain is attempting to achieve with it).

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel