On 2013-02-24 19:56, Avi Kivity wrote: > On Sun, Feb 24, 2013 at 12:49 PM, Jan Kiszka <jan.kiszka@xxxxxx> wrote: >> On 2013-02-24 11:11, Avi Kivity wrote: >>> On Sun, Feb 24, 2013 at 11:40 AM, Jan Kiszka <jan.kiszka@xxxxxx> wrote: >>>>>>> We have the same problem in KVM_SET_SREGS. >>>>>> >>>>>> I don't see the problem. kvm_arch_vcpu_ioctl_set_sregs open-codes the >>>>>> state update, not applying any transition checks. >>>>> >>>>> That's the problem. We have this open coding in three different >>>>> places (KVM_SET_SREGS, nvmx, nsvm). >>>>> >>>>> It's not as if vmx_set_cr0() is defined as "kvm_set_cr0() without the >>>>> transition checks". >>>> >>>> ...and without mmu updates. The latter is done via or after the closing >>>> cr3 update. Interestingly, nsvm does not perform kvm_set_cr3 on vmexit >>>> when in npt mode. Seems things aren't that regular. >>> >>> We do want the mmu updates. Of course they can't be attached to >>> kvm_set_cr0_without_the_checks() since there is cross-register >>> dependencies. >>> >>> Option 1 is kvm_set_multiple_cr(). This does the checks and updates, >>> but only after all registers are updated. >>> Option 2 is kvm_begin_cr_transaction()/kvm_commit_cr_transaction(). >>> Prettier and more flexible, but a more complicated to implement. >>> >> >> The only thing that these three use case truly have in common is the >> closing kvm_mmu_reset_context. Maybe nvmx and nsvm can share a bit more. >> But let's get nVMX right first, then think about sharing. > > They all need consistency checks, otherwise userspace or the guest and > inject inconsistent values and perhaps exploit the host. To my understanding, the hardware does this for us: If we try to enter the guest (L1, L2) with invalid CRx bits set or cleared, we get an error, at least on Intel. But I bet AMD does so as well - and, if not, it would make this test specific again. Jan
Attachment:
signature.asc
Description: OpenPGP digital signature
