[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH V2 2/3] xen/vm_event: Support for guest-requested events
On 06/26/2015 10:02 AM, Jan Beulich wrote: >>>> On 15.06.15 at 11:03, <rcojocaru@xxxxxxxxxxxxxxx> wrote: >> Added support for a new class of vm_events: VM_EVENT_REASON_REQUEST, >> sent via HVMOP_request_vm_event. The guest can request that a >> generic vm_event (containing only the vm_event-filled guest registers >> as information) be sent to userspace by setting up the correct >> registers and doing a VMCALL. For example, for a 64-bit guest, this >> means: EAX = 34 (hvmop), EBX = 24 (HVMOP_request_vm_event). > > I suppose you mean a 32-bit guest here? Also I'm not sure it's a good > idea to explicitly define a guest exposed hypercall to omit one of the > arguments normally required for it (the interface structure pointer): > Should there ever be a reason to allow the guest to control further > aspects of the operation by passing a structure, you'd then have to > define a new sub-op instead of being able to re-use the current one. > I.e. I'd strongly recommend requiring NULL to be passed here, and > checking this in the implementation of the handler. You're right, I've tested it on a 64-bit guest but indeed compiled the executable as a Win32 binary, hence the confusion. I'll correct the patch comment. Will look into the NULL parameter option. >> --- a/xen/arch/x86/hvm/event.c >> +++ b/xen/arch/x86/hvm/event.c >> @@ -126,6 +126,20 @@ void hvm_event_msr(unsigned int msr, uint64_t value) >> hvm_event_traps(1, &req); >> } >> >> +void hvm_event_requested(void) >> +{ >> + struct vcpu *curr = current; >> + struct arch_domain *currad = &curr->domain->arch; >> + >> + vm_event_request_t req = { > > Please avoid blank lines between declarations - a blank line following > declaration is supposed to delimit them from statements. Ack. >> --- a/xen/arch/x86/hvm/hvm.c >> +++ b/xen/arch/x86/hvm/hvm.c >> @@ -6373,6 +6373,10 @@ long do_hvm_op(unsigned long op, >> XEN_GUEST_HANDLE_PARAM(void) arg) >> break; >> } >> >> + case HVMOP_request_vm_event: >> + hvm_event_requested(); >> + break; > > No XSM check here or in the handler? Shouldn't the admin controlling > guest properties from the host perspective be permitted control here? > Cc-ing Daniel for his input ... Not sure this warrants XSM checks, but sure, if they're required I'll try to put them in. >> --- a/xen/include/asm-x86/domain.h >> +++ b/xen/include/asm-x86/domain.h >> @@ -342,13 +342,15 @@ struct arch_domain >> >> /* Monitor options */ >> struct { >> - uint16_t write_ctrlreg_enabled : 4; >> - uint16_t write_ctrlreg_sync : 4; >> - uint16_t write_ctrlreg_onchangeonly : 4; >> - uint16_t mov_to_msr_enabled : 1; >> - uint16_t mov_to_msr_extended : 1; >> - uint16_t singlestep_enabled : 1; >> - uint16_t software_breakpoint_enabled : 1; >> + uint32_t write_ctrlreg_enabled : 4; >> + uint32_t write_ctrlreg_sync : 4; >> + uint32_t write_ctrlreg_onchangeonly : 4; >> + uint32_t mov_to_msr_enabled : 1; >> + uint32_t mov_to_msr_extended : 1; >> + uint32_t singlestep_enabled : 1; >> + uint32_t software_breakpoint_enabled : 1; >> + uint32_t request_enabled : 1; >> + uint32_t request_sync : 1; > > Can you please switch to plain unsigned int if you already have to > touch this? There's no reason I can see to use a fixed width integer > type here. Ack, will make it plain int. >> --- a/xen/include/public/hvm/hvm_op.h >> +++ b/xen/include/public/hvm/hvm_op.h >> @@ -389,6 +389,10 @@ DEFINE_XEN_GUEST_HANDLE(xen_hvm_evtchn_upcall_vector_t); >> >> #endif /* defined(__i386__) || defined(__x86_64__) */ >> >> +#if defined(__XEN__) || defined(__XEN_TOOLS__) >> +#define HVMOP_request_vm_event 24 >> +#endif /* defined(__XEN__) || defined(__XEN_TOOLS__) */ > > Isn't this _specifically_ meant to be usable by a guest? Yes, but we still need Xen tools to subscribe to the events, control how they are received (in a dom0 or similarly privileged domain) and process them. That was my train of thought anyway, maybe I'm missing something (or I'm misinterpreting the macros). Thanks, Razvan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |