Re: [Xen-devel] Issue policing writes from Xen to PV domain memory

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.05.14 at 06:11, <aravindp@xxxxxxxxx> wrote:
> I took your advice and tried to stop the second invocation of wait_event() 
> by using the method suggested by Andres which is to merge the recursive 
> faults.
> 
> <quote>
> 1. read last unconsumed
> 2. compare
> 3. different? write into ring
> 4. same? atomic_read consumer index to ensure still unconsumed 
> 5. consumed? write into ring <still a possible race below, but not a tragedy> 
> 6. not consumed? dropâ
> <end quote>

Whether that's safe/correct/... I have to leave to Andres; it doesn't
feel well to do things like that.

> +bool_t mem_event_check_duplicate(struct mem_event_domain *med,
> +                                 mem_event_request_t *req)
> +{
> +    mem_event_front_ring_t *front_ring;
> +    mem_event_request_t *ureq;
> +    RING_IDX req_event;
> +    bool_t rc = 0;
> +
> +    mem_event_ring_lock(med);
> +
> +    /* Due to the reservations, this step must succeed. */
> +    front_ring = &med->front_ring;
> +
> +    /* Index of last unconsumed request */
> +    req_event = front_ring->sring->req_event - 1;
> +    ureq = RING_GET_REQUEST(front_ring, req_event);
> +
> +    if ( req->gfn != ureq->gfn )
> +        goto out;
> +    if ( current->vcpu_id != ureq->vcpu_id )
> +        goto out;
> +    if ( req->access_r != ureq->access_r )
> +        goto out;
> +    if ( req->access_w != ureq->access_w )
> +        goto out;
> +    if ( req->access_x != ureq->access_x )
> +        goto out;
> +
> +    /* Check consumer index has not moved */
> +    if ( req_event == read_atomic(&front_ring->sring->req_event) - 1 )
> +        rc = 1;

This should all be one big if() imo.

> With this I am able to prevent recursive identical faults in the runstate 
> area causing the assert to fire. However there still exists a corner case. I 
> still do not know what to do about the situation when the ring is full, the 
> last unconsumed event was not for the runstate area, the runstate area is 
> marked not writable and the guest or xen writes to it. I _think_ this would 
> again cause the assert to fire. Can you give me some guidance as to what to 
> do in this situation? Should I corner case the runstate area?

No, that wouldn't help as that's not the only thing Xen writes to guest
memory.

And no, I have no good idea about how to deal with the situation. I
continue to think that this is the wrong overall approach though, due
to the resultant inconsistency with HVM (where Xen writes are being
ignored), i.e. I believe you ought to rather think about making these
not fault, or deal with the faults gracefully and without needing to
emulate the instructions. One possibility I can see would be to clear
CR0.WP in the fault handler (and set a flag that this was done),
restoring it on the path back out of guest memory access function(s).
But that of course implies that the situation can only ever arise for
writes. And it would need to be done extremely carefully to make
sure you don't inadvertently allow writes to regions that are truly
write protected.

> PS: I did try running with a hacked up version where the max ring buffer 
> entries (nr_ents) is 1 and I could not make this situation happen but I guess 
> it is still theoretically possible. Or am I mistaken?

No, you aren't - if the ring doesn't get consumed from, it will
unavoidably become full at some point.

Jan

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