On 19/10/2017 20:09, Jim Mattson wrote: > "(offset & 0x3) == 1" seems like an obfuscated way of writing the > predicate, is_mci_status_msr(msr). But other than that, this change > looks fine to me. > > I'm a little more concerned about the code above. At the very least, > it needs to let the host set an arbitrary value for save/restore to > work. Why? The guest cannot have written anything but the three allowed values, userspace cannot write anything else either outside save/restore without KVM_SET_MSR failing, and KVM itself (specifically kvm_vcpu_ioctl_x86_setup_mce) only ever initializes IA32_MCi_CTL to all ones. So save will only ever find those three values, and restore's KVM_SET_MSR restore should never fail either. Thanks, Paolo > Reviewed-by: Jim Mattson <jmattson@xxxxxxxxxx>