Re: [PATCH v7 2/5] x86/ioreq server: Add DMOP to map guest ram with p2m_ioreq_server to an ioreq server.

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
[snip]
> >>>> +struct hvm_ioreq_server *p2m_get_ioreq_server(struct domain *d,
> >>>> +                                              unsigned int *flags)
> >>>> +{
> >>>> +    struct p2m_domain *p2m = p2m_get_hostp2m(d);
> >>>> +    struct hvm_ioreq_server *s;
> >>>> +
> >>>> +    spin_lock(&p2m->ioreq.lock);
> >>>> +
> >>>> +    s = p2m->ioreq.server;
> >>>> +    *flags = p2m->ioreq.flags;
> >>>> +
> >>>> +    spin_unlock(&p2m->ioreq.lock);
> >>>> +    return s;
> >>>> +}
> >>> I'm afraid this question was asked before, but since there's no
> >>> comment here or anywhere, I can't recall if there was a reason why
> >>> s potentially being stale by the time the caller looks at it is not a
> >>> problem.
> >> Well, it is possibe that s is stale. I did not take it as a problem
> >> because the device model
> >> will later discard such io request. And I believe current
> >> hvm_select_ioreq_server() also
> >> has the same issue - the returned s should be considered to be stale, if
> >> the MMIO/PIO
> >> address is removed from the ioreq server's rangeset.

An enabled emulator has to be prepared to receive ioreqs for ranges it has unmapped since there is no domain_pause() to prevent a race.

> >>
> >> Another thought is, if you think it is inappropriate for device model to
> >> do the check,
> >> we can use spin_lock_recursive on ioreq_server.lock to protect all the
> >> ioreq server select
> >> and release the lock after the ioreq server is sent out.
> > Well, let's first ask Paul as to what his perspective here is, both
> > specifically for this change and more generally regarding what
> > you say above.
> 
> Paul, any suggestions on this and the above one? :)

Well, as I said above, the device model has to check whether it is willing to handle and ioreq it is passed and terminate it appropriately under all circumstances. There is no option for it to reject the I/O.
This may be ok for MMIO regions coming and going, but is there anything more to consider here it we change a page time from ioreq_server back to RAM? Clearly we need to make sure that there is no scope for I/O to that page being incorrectly handled during transition.

  Paul

> 
> Thanks
> Yu
> > Jan
> >


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel