On Tue, Apr 12, 2022, Paolo Bonzini wrote: > On 4/11/22 21:08, Sean Christopherson wrote: > > > + if (!(mcg_cap & MCG_CMCI_P) && > > > + (data || !msr_info->host_initiated)) > > This looks wrong, userspace should either be able to write the MSR or not, '0' > > isn't special. Unless there's a danger to KVM, which I don't think there is, > > userspace should be allowed to ignore architectural restrictions, i.e. bypass > > the MCG_CMCI_P check, so that KVM doesn't create an unnecessary dependency between > > ioctls. I.e. this should be: > > > > if (!(mcg_cap & MCG_CMCI_P) && !msr_info->host_initiated) > > return 1; > > > > This is somewhat dangerous as it complicates (or removes) the invariants > that other code can rely on. Thus, usually, only the default value is > allowed for KVM_SET_MSR. Heh, I don't know if "usually" is the right word, that implies KVM is consistent enough to have a simple majority for any behavior, whatever that behavior may be :-) Anyways, on second look, I agree that KVM should require that userspace first enable CMCI via mcg_cap. I thought that vcpu->arch.mcg_cap could be written via the MSR interface, i.e. via userspace writes to MSR_IA32_MCG_CAP, and could create dependencies within KVM_SET_MSRS. But KVM only allows reading the MSR.
