Re: [Xen-devel] [PATCH] x86/monitor: add support for descriptor access events

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 03/14/2017 02:15 PM, Vlad-Ioan TOPAN wrote:
>>> @@ -2642,6 +2660,38 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
>>>      case VMEXIT_PAUSE:
>>>          svm_vmexit_do_pause(regs);
>>>          break;
>>> +    
>>> +    case VMEXIT_IDTR_READ:
>>> +        hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
>>> VM_EVENT_DESC_IDTR, 0);
>>> +        break;
>>> +
>>> +    case VMEXIT_GDTR_READ:
>>> +        hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
>>> VM_EVENT_DESC_GDTR, 0);
>>> +        break;
>>> +
>>> +    case VMEXIT_LDTR_READ:
>>> +        hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
>>> VM_EVENT_DESC_LDTR, 0);
>>> +        break;
>>> +
>>> +    case VMEXIT_TR_READ:
>>> +        hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
>>> VM_EVENT_DESC_TR, 0);
>>> +        break;
>>> +
>>> +    case VMEXIT_IDTR_WRITE:
>>> +        hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
>>> VM_EVENT_DESC_IDTR, 1);
>>> +        break;
>>> +
>>> +    case VMEXIT_GDTR_WRITE:
>>> +        hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
>>> VM_EVENT_DESC_GDTR, 1);
>>> +        break;
>>> +
>>> +    case VMEXIT_LDTR_WRITE:
>>> +        hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
>>> VM_EVENT_DESC_LDTR, 1);
>>> +        break;
>>> +
>>> +    case VMEXIT_TR_WRITE:
>>> +        hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0, 
>>> VM_EVENT_DESC_TR, 1);
>>> +        break;
>>
>> I think this can be halved in size by having
>> 'hvm_descriptor_access_intercept(..., exit_reason_write?1:0)'
>>
>> And maybe even collapse completely by having a lookup table mapping exit
>> reason to event.
> 
> The problem with both ideas is that they depend on assumptions about the
> values of the VMEXIT_* constants to make the code shorter and still
> keep it readable, which in my opinion would be bad. Although they will
> most likely stay sequential and keep their current numeric values, it's
> not something I'd hardcode. Without those assumptions, it's either
> another switch or a very long if, which would mean roughly the same
> amount of code, but less readable (it's the way I've written it
> initally before coming to this version).

I'm reading Boris' suggestion to mean:

case VMEXIT_IDTR_READ:
case VMEXIT_IDTR_WRITE:
    hvm_descriptor_access_intercept(vmcb->exitintinfo.bytes, 0,
        VM_EVENT_DESC_IDTR, exit_reason == VMEXIT_IDTR_WRITE);
    break;

I could be wrong.


Thanks,
Razvan

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