On 2013-03-04 16:37, Nadav Har'El wrote: > On Sun, Mar 03, 2013, Jan Kiszka wrote about "[PATCH] KVM: nVMX: Fix content of MSR_IA32_VMX_ENTRY/EXIT_CTLS": >> /* Note that guest use of VM_EXIT_ACK_INTR_ON_EXIT is not supported. */ >> #ifdef CONFIG_X86_64 >> nested_vmx_exit_ctls_high = VM_EXIT_HOST_ADDR_SPACE_SIZE; >> #else >> nested_vmx_exit_ctls_high = 0; >> #endif >> + nested_vmx_exit_ctls_high |= 0x36dff; > > Can you please compose this 0x36dff out of constants? Is > VM_EXIT_HOST_ADDR_SPACE_SIZE one of them? Nope. These are bits enumerated by the spec to be always 1 unless that bit 55 is 1 - what happens to them then remains unclear to me, but is not important right now. I'm about to stick them all into a constant. > > It's important to verify that we actually support all these bits - even > if we *should* support them, it doesn't mean we actually do (but if we > do, we should say we do). No, we just implement what the spec demands here. Leaving any of them 0 would make our implementation non-conforming, not just "special" as it is right now. > >> - nested_vmx_entry_ctls_low = 0; >> + /* If bit 55 of VMX_BASIC is off, bits 0-8 and 12 must be 1. */ >> + nested_vmx_entry_ctls_low = 0x11ff; > > Setting nested_vmx_entry_ctls_low = 0 just means that although the spec > says only 1 setting is supported, we *also* support 0 setting. I'm not > sure why this is a bad thing. Our VMX will be even better than the > real processors' ;-) Bad idea. We first need to make it correct, then we can think about optimizations. This is very important if you want to support hypervisors for which you have no code, just the knowledge that they work on real hw. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
