Re: [PATCH] arm/monitor vm-events: Implement guest-request support

[Razvan Cojocaru <rcojocaru@bitdefender.com>]

On 02/23/2016 01:00 PM, Corneliu ZUZU wrote:
> On 2/23/2016 12:54 PM, Razvan Cojocaru wrote:
>> On 02/23/2016 11:09 AM, Corneliu ZUZU wrote:
>>> On 2/18/2016 9:35 PM, Corneliu ZUZU wrote:
>>>> This patch adds ARM support for guest-request monitor vm-events.
>>>>
>>>> Summary of changes:
>>>> == Moved to common-side:
>>>>    * XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST handling (moved from X86
>>>>        arch_monitor_domctl_event to common monitor_domctl)
>>>>    * hvm_event_guest_request, hvm_event_traps (also added target vcpu
>>>> as param)
>>>>    * guest-request bits from X86 'struct arch_domain' (to common
>>>> 'struct domain')
>>>> == ARM implementations:
>>>>    * do_hvm_op now handling of HVMOP_guest_request_vm_event => calls
>>>>        hvm_event_guest_request (as on X86)
>>>>    * arch_monitor_get_capabilities: updated to reflect support for
>>>>        XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST
>>>>    * vm_event_init_domain (does nothing), vm_event_cleanup_domain
>>>> == Misc:
>>>>    * hvm_event_fill_regs renamed to arch_hvm_event_fill_regs, no longer
>>>>        X86-specific. ARM-side implementation of this function
>>>> currently does
>>>>        nothing, that will be added in a separate patch.
>>>>
>>>> Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com>
>>> Before sending in the next revision of this patch, I have a few
>>> questions I'd like to ask.
>>> That being the case, I thought it would be ok to also include *all* the
>>> changes that will be done in the next revision here for (potentially)
>>> additional feedback.
>>>
>>> Already discussed changes TBD:
>>> * Add #define altp2m_active(d) (0) and implement
>>> p2m_get_vcpu_altp2m_idx(v) for ARM to remove #ifdef CONFIG_X86 in
>>> hvm_event_traps (Stefano, Tamas)
>>> * Remove wrong copyright comment (Jan)
>>> * Change bitfields members type back to unsigned int (Jan)
>>> * Move hvm_event_traps and hvm_event_guest_request to common/vm_event.c
>>> and rename them to vm_event_traps and vm_event_guest_request (Jan)
>>>
>>> Questions:
>>>
>>> 1) I've noticed the practice in Xen is to prepend the arch_ prefix to
>>> functions that usually have a counterpart on the common-side, i.e. there
>>> are a lot of arch-specific functions missing this prefix. Would it then
>>> be advised to:
>>>     - rename arch_monitor_get_capabilities to
>>> vm_event_monitor_get_capabilities and move it to vm_event.h
>>>     - rename arch_hvm_event_fill_regs to vm_event_fill_regs and move it
>>> to vm_event.h
>> Please see the recent commit adc75eba8b15c7103a010f736fe62e3fb2383964 in
>> staging.
>>
>>
>> Cheers,
>> Razvan
>>
> 
> Great, that settles arch_hvm_event_fill_regs -> vm_event_fill_regs.
> Should I then do the same for arch_monitor_get_capabilities (->
> vm_event_monitor_get_capabilities)?

Sounds good to me, so yes, unless Tamas or Jan think otherwise.


Thanks,
Razvan