On Thu, Aug 30, 2018 at 12:22:10PM +0800, Robert Hoo wrote: > On Thu, 2018-08-23 at 14:11 -0300, Eduardo Habkost wrote: > > On Thu, Aug 23, 2018 at 02:28:28PM +0800, Robert Hoo wrote: > > > On Sat, 2018-08-18 at 12:05 -0300, Eduardo Habkost wrote: > > [...] > > > > We don't want QEMU to refuse to run if the kernel doesn't have > > > > KVM_CAP_GET_MSR_FEATURES. We can treat missing capability as > > > > equivalent to returning an empty list of MSRs. > > > Yes. I'll let caller (kvm_arch_init) ignore the return value but a > > > simple warning. > > > > Warnings tend to be ignored and are generally a sign that QEMU > > isn't doing the right thing. Sometimes we have no choice, but I > > don't think that's the case here. > > > > As far as I can see, we have only two possibilities here: > > 1) The host can run a VM that behaves exactly as requested on the > > command-line (no warning required). > > 2) The host can't run the requested configuration (fatal error, > > not a warning). > > > I mean the kvm_arch_init() --> kvm_get_supported_feature_msrs(), when > kvm_get_supported_feature_msrs() returns error, can we ignore it? > The cases of kvm_get_supported_feature_msrs() returning error: > 1) underlying KVM doesn't support GET_MSR_FEATURES capability. > 2) error in KVM_GET_MSR_FEATURE_INDEX_LIST. > > given the principle you guided before, I think we can ignore above error > cases, and later kvm_arch_get_supported_msr_feature() would always > return 0 (of course, except those special cases like > IA32_ARCH_CAPABILITIES.RSBA)
Making kvm_arch_get_supported_msr_feature() return 0 if the capability isn't available (except for RSBA) seems to be enough to ensure we do the right thing on both cases (1 and 2 above). Now, if KVM_GET_MSR_FEATURE_INDEX_LIST returns an unexpected error, it sounds like an exceptional situation that can justify a fatal error (or at least a warning). -- Eduardo
