Re: [Xen-devel] [PATCH V3 3/3] xen/vm_event: Deny register writes if refused by vm_event reply

["Lengyel, Tamas" <tlengyel@xxxxxxxxxxx>]

@@ -410,6 +414,8 @@ void vm_event_resume(struct domain *d, struct vm_event_domain *ved)

 #ifdef HAS_MEM_ACCESS
         case VM_EVENT_REASON_MEM_ACCESS:
+        case VM_EVENT_REASON_MOV_TO_MSR:
+        case VM_EVENT_REASON_WRITE_CTRLREG:

This doesn't really make much sense to be associated with MEM_ACCESS. I'm adding a separate arch-specific vm_event file in my other singlestep patch, I think these should trigger their appropriate handler there, not in mem_access_resume.

 

             mem_access_resume(v, &rsp);
             break;
 #endifdiff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
index f0da008..bc97334 100644
--- a/xen/include/public/vm_event.h
+++ b/xen/include/public/vm_event.h
@@ -158,6 +158,11 @@ struct vm_event_regs_x86 {
  * MEM_ACCESS_EMULATE_NOWRITE.
  */
 #define MEM_ACCESS_SET_EMUL_READ_DATA   (1 << 8)
+ /*
+  * Deny completion of the operation that triggered the event.
+  * Currently only useful for MSR, CR0, CR3 and CR4 write events.
+  */
+#define MEM_ACCESS_DENY                 (1 << 9)

Same comment here, this feature is not really denying a mem_access, it denies register writes. Associating it with mem_access just makes it confusing. IMHO defining it as a VM_EVENT_FLAG_DENY_REGISTER_CHANGE or something similar instead would make it a lot more descriptive and inline with that it is actually doing.

 

Cheers,

Tamas

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