Re: [PATCH v2 1/4] asm-x86/monitor: Enable svm monitor events

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

(Resending)

On 02/10/2018 11:30 AM, Boris Ostrovsky wrote:
>
>
> On 02/08/2018 10:25 AM, Alexandru Isaila wrote:
>> This commit separates the svm caps from the vmx caps.
>>
>> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
>>
>> ---
>> Changes since V1:
>>     - Removed the if ( cpu_has_svm )
>> ---
>>   xen/include/asm-x86/monitor.h | 34 +++++++++++++++++++---------------
>>   1 file changed, 19 insertions(+), 15 deletions(-)
>>
>> diff --git a/xen/include/asm-x86/monitor.h
>> b/xen/include/asm-x86/monitor.h
>> index a0444d1..b2b4e6a 100644
>> --- a/xen/include/asm-x86/monitor.h
>> +++ b/xen/include/asm-x86/monitor.h
>> @@ -71,24 +71,28 @@ static inline uint32_t
>> arch_monitor_get_capabilities(struct domain *d)
>>       uint32_t capabilities = 0;
>>         /*
>> -     * At the moment only Intel HVM domains are supported. However,
>> event
>> -     * delivery could be extended to AMD and PV domains.
>> +     * At the moment only Intel and AMD HVM domains are supported.
>> However, event
>> +     * delivery could be extended to and PV domains.
>>        */
>> -    if ( !is_hvm_domain(d) || !cpu_has_vmx )
>> +    if ( !is_hvm_domain(d) )
>>           return capabilities;
>>   -    capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
>> -                   (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
>> -                   (1U <<
>> XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
>> -                   (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST) |
>> -                   (1U << XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION) |
>> -                   (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
>> -                   (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT) |
>> -                   (1U << XEN_DOMCTL_MONITOR_EVENT_EMUL_UNIMPLEMENTED);
>> -
>> -    /* Since we know this is on VMX, we can just call the hvm func */
>> -    if ( hvm_is_singlestep_supported() )
>> -        capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
>> +    if( cpu_has_vmx )
>> +    {
>> +        capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
>> +                       (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
>> +                       (1U <<
>> XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
>> +                       (1U <<
>> XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION) |
>> +                       (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
>> +                       (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT) |
>> +                       (1U <<
>> XEN_DOMCTL_MONITOR_EVENT_EMUL_UNIMPLEMENTED);
>> +
>> +        /* Since we know this is on VMX, we can just call the hvm
>> func */
>> +        if ( hvm_is_singlestep_supported() )
>> +            capabilities |= (1U <<
>> XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
>> +    }
>> +
>> +    capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
>
>
> It's a nit but I'd start with setting common options and the OR in
> arch-specific ones. (i.e. move the line above to right after
> !is_hvm_domain(d) test).
>
> -boris
>     
>
>
>>         if ( hvm_funcs.set_descriptor_access_exiting )
>>           capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS);
>>


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