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

["Jan Beulich" <JBeulich@xxxxxxxx>]

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

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

>> 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?

Jan



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