On 2/8/2016 8:15 PM, Tamas K Lengyel wrote:
On Mon, Feb 8, 2016 at 9:57 AM, Corneliu ZUZU <cz...@bitdefender.com
<mailto:cz...@bitdefender.com>> wrote:
+#if CONFIG_X86
Wouldn't it be safe to include these headers on ARM as well?
+#include <public/vm_event.h> /* for VM_EVENT_X86_CR3 */
+#include <asm/hvm/hvm.h> /* for hvm_update_guest_cr,
... */
+#endif
It wouldn't break anything, but they are only necessary on X86, so why
include them?
+
+ if ( unlikely(mop->op != XEN_DOMCTL_MONITOR_OP_ENABLE &&
+ mop->op != XEN_DOMCTL_MONITOR_OP_DISABLE) )
+ {
Please make a switch on mop->op instead where the default case is to
call arch_monitor_domctl. It would be a bit easier to follow.
+ if ( XEN_DOMCTL_MONITOR_OP_GET_CAPABILITIES == mop->op )
+ {
+ mop->event = arch_monitor_get_capabilities(d);
+ return 0;
+ }
+
Noted, good point.
"proly"->probably?
+ /* The monitor op is proly handled on the arch-side. */
+ if ( likely(arch_monitor_domctl_op(d, mop, &rc)) )
+ return rc;
Yep, will "proly" change that, heh, just kidding. Noted :D.
Empty line not needed. (*)
Noted.
So I don't particularly like this #if check here. IMHO this should be
done in arch-specific function that you call from here that is just
empty for ARM. It could be a static inline function as it's rather small.
+#if CONFIG_X86
+ if ( VM_EVENT_X86_CR3 == mop->u.mov_to_cr.index )
+ {
+ struct vcpu *v;
+ /* Latches new CR3 mask through CR0 code. */
+ for_each_vcpu ( d, v )
+ hvm_update_guest_cr(v, 0);
+ }
+#endif
I agree, but I was actually hoping to approach that in a future
ARM-patch I'm going to send after this one.
On ARM, that part of code (where hypervisor CR trapping is enabled based
on write_ctrlreg_enabled bits)
is done in another place (on the scheduling tail). I wanted to approach
removing that #ifdef and finding maybe
a solution to do that similarly for X86. Would it be ok to leave this
for that discussion? It would be simpler to illustrate
what I have in mind.
Looks good otherwise.
Thanks,
Tamas
Thanks,
Corneliu.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
