Re: [PATCH v2 17/17] kvm: Drop dependencies on very old capabilities

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 1867 bytes --]

Am 03.01.2011 18:01, Avi Kivity wrote:
> On 01/03/2011 06:54 PM, Jan Kiszka wrote:
>> Am 03.01.2011 17:08, Avi Kivity wrote:
>> >  On 01/03/2011 10:33 AM, Jan Kiszka wrote:
>> >>  From: Jan Kiszka<jan.kiszka@siemens.com>
>> >>
>> >>  COALESCED_MMIO, SYNC_MMU, EXT_CPUID, CLOCKSOURCE, NOP_IO_DELAY,
>> PV_MMU -
>> >>  all these caps predate features on which we already depend at build
>> >>  time. Moreover, the check for KVM_CAP_EXT_CPUID is unneeded as we
>> >>  already test&   fail is a more recent feature is missing.
>> >
>> >  No.  Each test documents a dependency of qemu on a kvm feature.  Even
>> >  though something like SYNC_MMU is unlikely to go away, as long as we
>> >  depend on it, we require the feature.
>> >
>>
>> Then at least move all those KVM_CAPs we need at build time into
>> configure.
> 
> Need a run time check as well (build on new kernel, run on old kernel,
> or run on even newer kernel that lost a feature).
> 
>> I really see no value in keeping ugly conditional code
>> around, A) because those paths won't be tested and B) none of the CAPs
>> touched here are to pass away without a replacement that will require
>> user space adaption anyway.
> 
> I'm fine with a series of checks during init time with no fallback.  I'm
> not fine with just dropping those away.  Reducing code size is great,
> but not at the cost of undiagnosed runtime failures.

My worry was not code size but untested code. And looking at the
EXT_CPUID case again, this is what would happen:
kvm_x86_get_supported_cpuid will return -1 if we lack EXT_CPUID, but
that value is interpreted by the callers as "all requested features
available" - likely not that helpful in all cases. This also affects
qemu-kvm.

I will add generic and per-arch check sections at build and runtime for
CAPs we don't want to miss.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]