Re: [Xen-devel] [PATCH v8 07/11] vpci/bars: add handlers to map the BARs

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

On Tue, Feb 27, 2018 at 07:10:51AM -0700, Jan Beulich wrote:
> >>> On 27.02.18 at 12:57, <roger.pau@xxxxxxxxxx> wrote:
> > On Tue, Feb 27, 2018 at 03:01:44AM -0700, Jan Beulich wrote:
> >> >>> On 27.02.18 at 10:21, <roger.pau@xxxxxxxxxx> wrote:
> >> > With the current approach in the unmap case there will be stale
> >> > mappings left behind.
> >> > 
> >> > I guess it's better then to not modify the memory decoding bit at all
> >> > until the operation finishes. That also rises the question of whether
> >> > the memory decoding bit should be modified if p2m mapping/unmapping
> >> > reports an error.
> >> > 
> >> > Should we also attempt to rollback failed map/unmap operations? What
> >> > happens if the rollback also fails?
> >> 
> >> It is in particular this last question why I don't think rollback makes
> >> sense. If there's any failure, I think the decode bit should be sticky
> >> clear; we may want (need?) to invent some magical mechanism to
> >> get a device back out of this state later on. But that way stale
> >> mappings are not an immediate problem (I think).
> > 
> > The only problem I see with this approach is that if an error happens
> > in the unmap case we will disable memory decoding and leave some stale
> > p2m mappings. Then the guest might change the position of the BARs,
> > and those stale mappings would be completely forgotten.
> 
> Well, the implication was that together with the decode enable bit
> not being allowed to be turned off would be that some recording
> would perhaps be needed of the stale mappings. Allowing the
> decode enable bit to be set again would then depend on the stale
> mappings first being dealt with. As long as the decode enable bit
> can't be set, the owning domain playing with the BARs is of no
> interest.

I've been giving some thought to this, and I cannot find a good
solution. So far the less worse one would be:

    +--------------+  YES  +---------+
    |Is broken set?|------>|No action|
    +--------------+       +---------+
         |
         | NO
         |
    +----v----+   SUCCESS  +---------------------------------+
    |map/unmap+------------>Change decoding or ROM enable bit|
    +----+----+            +---------------------------------+
         |
         |FAILURE
         |
+--------v----------------+
|Clear memory decoding bit|
|Set broken               |
+-------------------------+

Let me know what you think of this.

Thanks, Roger.

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