Re: [PATCH 0/1] ARM: Implement support for write-ctrlreg vm-events

[Razvan Cojocaru <rcojocaru@bitdefender.com>]

On 03/03/2016 04:10 PM, Corneliu ZUZU wrote:
> Q2) About VM_EVENT_FLAG_DENY
> 
>   Q2.1)
>     Doesn't it require sync = 1 (i.e. the vcpu to be paused) to work?
>     If so, shouldn't we call vm_event_register_write_resume only after checking
>     that VM_EVENT_FLAG_VCPU_PAUSED flag is set (vm_event_resume). Moreover, if
>     we do that, wouldn't it be 'cleaner' to rename
>     vm_event_register_write_resume->vm_event_deny, check for the
>     VM_EVENT_FLAG_DENY flag in vm_event_resume instead and call vm_event_deny
>     from there after this check?

Yes, it does require the VCPU to be paused to work, and yes, it's a good
idea to check that that flag is set in the response.

Beyond that, I'd prefer we keep vm_event_register_write_resume()
because, while today all we do there is check that VM_EVENT_FLAG_DENY is
set, we might want do to other things as well there as well (for
example, maybe validate the content of some register). That was really
the point of the "bigger-named" function.

But that's just my opinion, if Tamas prefers your rename suggestion I'll
consider myself outnumbered.

>   Q2.2)
>     VM_EVENT_FLAG_DENY functionality is not implemented w/ this change-set.
>     What is done instead is that the control-register write is done *before*
>     sending the vm-event (vm_event_put_request). This way, the user can
>     override the written register value after receiving the vm-event, which
>     in effect provides the same flexibility as VM_EVENT_FLAG_DENY does.
>     Using this strategy instead would simplify both Xen's code and the libxc
>     user's code.
>     Thoughts?

That's how I initially did it with CR events, but with an application
dealing with huge numbers of events, an extra hypercall (to re-set the
register) can be quite expensive, so I had to rework it to the present
state. On these grounds I'm opposed to it - and for consistency I would
prefer that all register write events are pre-write events, and deniable
with a single vm_event reply.


HTH,
Razvan

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