On Sat, Mar 11, 2023 at 1:17 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > @@ -3047,6 +3047,19 @@ static int nested_vmx_check_guest_state(struct kvm_vcpu *vcpu, > > vmcs12->guest_ia32_perf_global_ctrl))) > > return -EINVAL; > > > > + if (CC((vmcs12->guest_cr0 & (X86_CR0_PG | X86_CR0_PE)) == X86_CR0_PG)) > > + return -EINVAL; > > + > > +#ifdef CONFIG_X86_64 > > The #ifdef isn't necessary, attempting to set for a 32-bit host should be rejected > by nested_vmx_check_controls() since nested_vmx_setup_ctls_msrs() clears the bit. > Ditto for the host logic related to VM_EXIT_HOST_ADDR_SPACE_SIZE, which looks > suspiciously similar ;-) Yeah, I noticed that too and decided that the idea could have been to allow some dead code elimination on 32-bit kernels, so I copied what the host state checks were doing. But if you prefer the more compact way I am absolutely not going to complain. > > + if (CC(ia32e && > > + (!(vmcs12->guest_cr4 & X86_CR4_PAE) || > > + !(vmcs12->guest_cr0 & X86_CR0_PG)))) > > + return -EINVAL; > > This is a lot easier to read IMO, and has the advantage of more precisely > identifying the failure in the tracepoint. > > if (CC(ia32e && !(vmcs12->guest_cr4 & X86_CR4_PAE)) || > CC(ia32e && !(vmcs12->guest_cr4 & X86_CR0_PG))) > return -EINVAL; Looks good. I squashed everything in. Paolo
