Patchwork [07/12] kvm: Drop KVM_CAP build dependencies

login
register
mail settings
Submitter Jan Kiszka
Date June 8, 2011, 2:11 p.m.
Message ID <0392a4552957a340b66dbcb2031229a45527bce9.1307542247.git.jan.kiszka@siemens.com>
Download mbox | patch
Permalink /patch/99449/
State New
Headers show

Comments

Jan Kiszka - June 8, 2011, 2:11 p.m.
No longer needed with accompanied kernel headers. We are only left with
build dependencies that are controlled by kvm arch headers.

CC: Alexander Graf <agraf@suse.de>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kvm-all.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)
Alexander Graf - June 14, 2011, 11:05 a.m.
On 08.06.2011, at 16:11, Jan Kiszka wrote:

> No longer needed with accompanied kernel headers. We are only left with
> build dependencies that are controlled by kvm arch headers.

This should completely rule out all CAPs right? IIRC, all CAPs are defined in generic code, so we don't get number overlaps.

> 
> CC: Alexander Graf <agraf@suse.de>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> kvm-all.c |    8 --------
> 1 files changed, 0 insertions(+), 8 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 4a9910a..cbc2532 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -757,21 +757,17 @@ int kvm_init(void)
>     s->coalesced_mmio = kvm_check_extension(s, KVM_CAP_COALESCED_MMIO);
> 
>     s->broken_set_mem_region = 1;
> -#ifdef KVM_CAP_JOIN_MEMORY_REGIONS_WORKS
>     ret = kvm_check_extension(s, KVM_CAP_JOIN_MEMORY_REGIONS_WORKS);
>     if (ret > 0) {
>         s->broken_set_mem_region = 0;
>     }
> -#endif
> 
> #ifdef KVM_CAP_VCPU_EVENTS

... which leaves the question why this one is still here :).


Alex
Jan Kiszka - June 14, 2011, 11:07 a.m.
On 2011-06-14 13:05, Alexander Graf wrote:
> 
> On 08.06.2011, at 16:11, Jan Kiszka wrote:
> 
>> No longer needed with accompanied kernel headers. We are only left with
>> build dependencies that are controlled by kvm arch headers.
> 
> This should completely rule out all CAPs right? IIRC, all CAPs are defined in generic code, so we don't get number overlaps.

"We are only left with build dependencies that are controlled by kvm
arch headers." E.g. KVM_CAP_VCPU_EVENTS.

> 
>>
>> CC: Alexander Graf <agraf@suse.de>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>> kvm-all.c |    8 --------
>> 1 files changed, 0 insertions(+), 8 deletions(-)
>>
>> diff --git a/kvm-all.c b/kvm-all.c
>> index 4a9910a..cbc2532 100644
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -757,21 +757,17 @@ int kvm_init(void)
>>     s->coalesced_mmio = kvm_check_extension(s, KVM_CAP_COALESCED_MMIO);
>>
>>     s->broken_set_mem_region = 1;
>> -#ifdef KVM_CAP_JOIN_MEMORY_REGIONS_WORKS
>>     ret = kvm_check_extension(s, KVM_CAP_JOIN_MEMORY_REGIONS_WORKS);
>>     if (ret > 0) {
>>         s->broken_set_mem_region = 0;
>>     }
>> -#endif
>>
>> #ifdef KVM_CAP_VCPU_EVENTS
> 
> ... which leaves the question why this one is still here :).

See above (does PPC support it?).

Jan
Alexander Graf - June 14, 2011, 11:17 a.m.
On 14.06.2011, at 13:07, Jan Kiszka wrote:

> On 2011-06-14 13:05, Alexander Graf wrote:
>> 
>> On 08.06.2011, at 16:11, Jan Kiszka wrote:
>> 
>>> No longer needed with accompanied kernel headers. We are only left with
>>> build dependencies that are controlled by kvm arch headers.
>> 
>> This should completely rule out all CAPs right? IIRC, all CAPs are defined in generic code, so we don't get number overlaps.
> 
> "We are only left with build dependencies that are controlled by kvm
> arch headers." E.g. KVM_CAP_VCPU_EVENTS.
> 
>> 
>>> 
>>> CC: Alexander Graf <agraf@suse.de>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>> kvm-all.c |    8 --------
>>> 1 files changed, 0 insertions(+), 8 deletions(-)
>>> 
>>> diff --git a/kvm-all.c b/kvm-all.c
>>> index 4a9910a..cbc2532 100644
>>> --- a/kvm-all.c
>>> +++ b/kvm-all.c
>>> @@ -757,21 +757,17 @@ int kvm_init(void)
>>>    s->coalesced_mmio = kvm_check_extension(s, KVM_CAP_COALESCED_MMIO);
>>> 
>>>    s->broken_set_mem_region = 1;
>>> -#ifdef KVM_CAP_JOIN_MEMORY_REGIONS_WORKS
>>>    ret = kvm_check_extension(s, KVM_CAP_JOIN_MEMORY_REGIONS_WORKS);
>>>    if (ret > 0) {
>>>        s->broken_set_mem_region = 0;
>>>    }
>>> -#endif
>>> 
>>> #ifdef KVM_CAP_VCPU_EVENTS
>> 
>> ... which leaves the question why this one is still here :).
> 
> See above (does PPC support it?).

Well, regardless of support, the #ifdef shouldn't be required, right?

include/linux/kvm.h:518:#define KVM_CAP_VCPU_EVENTS 41

... as long as there's a runtime check :)


Alex
Jan Kiszka - June 14, 2011, 11:19 a.m.
On 2011-06-14 13:17, Alexander Graf wrote:
> 
> On 14.06.2011, at 13:07, Jan Kiszka wrote:
> 
>> On 2011-06-14 13:05, Alexander Graf wrote:
>>>
>>> On 08.06.2011, at 16:11, Jan Kiszka wrote:
>>>
>>>> No longer needed with accompanied kernel headers. We are only left with
>>>> build dependencies that are controlled by kvm arch headers.
>>>
>>> This should completely rule out all CAPs right? IIRC, all CAPs are defined in generic code, so we don't get number overlaps.
>>
>> "We are only left with build dependencies that are controlled by kvm
>> arch headers." E.g. KVM_CAP_VCPU_EVENTS.
>>
>>>
>>>>
>>>> CC: Alexander Graf <agraf@suse.de>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>> ---
>>>> kvm-all.c |    8 --------
>>>> 1 files changed, 0 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/kvm-all.c b/kvm-all.c
>>>> index 4a9910a..cbc2532 100644
>>>> --- a/kvm-all.c
>>>> +++ b/kvm-all.c
>>>> @@ -757,21 +757,17 @@ int kvm_init(void)
>>>>    s->coalesced_mmio = kvm_check_extension(s, KVM_CAP_COALESCED_MMIO);
>>>>
>>>>    s->broken_set_mem_region = 1;
>>>> -#ifdef KVM_CAP_JOIN_MEMORY_REGIONS_WORKS
>>>>    ret = kvm_check_extension(s, KVM_CAP_JOIN_MEMORY_REGIONS_WORKS);
>>>>    if (ret > 0) {
>>>>        s->broken_set_mem_region = 0;
>>>>    }
>>>> -#endif
>>>>
>>>> #ifdef KVM_CAP_VCPU_EVENTS
>>>
>>> ... which leaves the question why this one is still here :).
>>
>> See above (does PPC support it?).
> 
> Well, regardless of support, the #ifdef shouldn't be required, right?
> 
> include/linux/kvm.h:518:#define KVM_CAP_VCPU_EVENTS 41
> 
> ... as long as there's a runtime check :)

Are all types and dependent constants available? Maybe it is the case
here, but you cannot generally assume this if a CAP is per-arch.

Jan
Alexander Graf - June 14, 2011, 11:25 a.m.
On 14.06.2011, at 13:19, Jan Kiszka wrote:

> On 2011-06-14 13:17, Alexander Graf wrote:
>> 
>> On 14.06.2011, at 13:07, Jan Kiszka wrote:
>> 
>>> On 2011-06-14 13:05, Alexander Graf wrote:
>>>> 
>>>> On 08.06.2011, at 16:11, Jan Kiszka wrote:
>>>> 
>>>>> No longer needed with accompanied kernel headers. We are only left with
>>>>> build dependencies that are controlled by kvm arch headers.
>>>> 
>>>> This should completely rule out all CAPs right? IIRC, all CAPs are defined in generic code, so we don't get number overlaps.
>>> 
>>> "We are only left with build dependencies that are controlled by kvm
>>> arch headers." E.g. KVM_CAP_VCPU_EVENTS.
>>> 
>>>> 
>>>>> 
>>>>> CC: Alexander Graf <agraf@suse.de>
>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>> ---
>>>>> kvm-all.c |    8 --------
>>>>> 1 files changed, 0 insertions(+), 8 deletions(-)
>>>>> 
>>>>> diff --git a/kvm-all.c b/kvm-all.c
>>>>> index 4a9910a..cbc2532 100644
>>>>> --- a/kvm-all.c
>>>>> +++ b/kvm-all.c
>>>>> @@ -757,21 +757,17 @@ int kvm_init(void)
>>>>>   s->coalesced_mmio = kvm_check_extension(s, KVM_CAP_COALESCED_MMIO);
>>>>> 
>>>>>   s->broken_set_mem_region = 1;
>>>>> -#ifdef KVM_CAP_JOIN_MEMORY_REGIONS_WORKS
>>>>>   ret = kvm_check_extension(s, KVM_CAP_JOIN_MEMORY_REGIONS_WORKS);
>>>>>   if (ret > 0) {
>>>>>       s->broken_set_mem_region = 0;
>>>>>   }
>>>>> -#endif
>>>>> 
>>>>> #ifdef KVM_CAP_VCPU_EVENTS
>>>> 
>>>> ... which leaves the question why this one is still here :).
>>> 
>>> See above (does PPC support it?).
>> 
>> Well, regardless of support, the #ifdef shouldn't be required, right?
>> 
>> include/linux/kvm.h:518:#define KVM_CAP_VCPU_EVENTS 41
>> 
>> ... as long as there's a runtime check :)
> 
> Are all types and dependent constants available? Maybe it is the case
> here, but you cannot generally assume this if a CAP is per-arch.

Ah, I see what you mean. Maybe it'd be easier to use __KVM_HAVE_VCPU_EVENTS instead of the implicitly defined CAP? But then again, once all the version CAP #ifdefs are gone, the only ones left are the ones for arch features, so I guess your approach is valid :)


Alex

Patch

diff --git a/kvm-all.c b/kvm-all.c
index 4a9910a..cbc2532 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -757,21 +757,17 @@  int kvm_init(void)
     s->coalesced_mmio = kvm_check_extension(s, KVM_CAP_COALESCED_MMIO);
 
     s->broken_set_mem_region = 1;
-#ifdef KVM_CAP_JOIN_MEMORY_REGIONS_WORKS
     ret = kvm_check_extension(s, KVM_CAP_JOIN_MEMORY_REGIONS_WORKS);
     if (ret > 0) {
         s->broken_set_mem_region = 0;
     }
-#endif
 
 #ifdef KVM_CAP_VCPU_EVENTS
     s->vcpu_events = kvm_check_extension(s, KVM_CAP_VCPU_EVENTS);
 #endif
 
-#ifdef KVM_CAP_X86_ROBUST_SINGLESTEP
     s->robust_singlestep =
         kvm_check_extension(s, KVM_CAP_X86_ROBUST_SINGLESTEP);
-#endif
 
 #ifdef KVM_CAP_DEBUGREGS
     s->debugregs = kvm_check_extension(s, KVM_CAP_DEBUGREGS);
@@ -850,7 +846,6 @@  static void kvm_handle_io(uint16_t port, void *data, int direction, int size,
     }
 }
 
-#ifdef KVM_CAP_INTERNAL_ERROR_DATA
 static int kvm_handle_internal_error(CPUState *env, struct kvm_run *run)
 {
     fprintf(stderr, "KVM internal error.");
@@ -877,7 +872,6 @@  static int kvm_handle_internal_error(CPUState *env, struct kvm_run *run)
      */
     return -1;
 }
-#endif
 
 void kvm_flush_coalesced_mmio_buffer(void)
 {
@@ -1008,11 +1002,9 @@  int kvm_cpu_exec(CPUState *env)
                     (uint64_t)run->hw.hardware_exit_reason);
             ret = -1;
             break;
-#ifdef KVM_CAP_INTERNAL_ERROR_DATA
         case KVM_EXIT_INTERNAL_ERROR:
             ret = kvm_handle_internal_error(env, run);
             break;
-#endif
         default:
             DPRINTF("kvm_arch_handle_exit\n");
             ret = kvm_arch_handle_exit(env, run);