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

[Vlad-Ioan TOPAN <itopan@xxxxxxxxxxxxxxx>]

> > @@ -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).

--
Vlad-Ioan TOPAN
Linux Kernel Development Lead
Bitdefender

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