Patchwork Re: [PATCH] provide a stub version of kvm-all.c if !CONFIG_KVM

login
register
mail settings
Submitter Blue Swirl
Date April 2, 2010, 7:04 p.m.
Message ID <w2yf43fc5581004021204w62027b44m2d2ba98d94d2dbe0@mail.gmail.com>
Download mbox | patch
Permalink /patch/49322/
State New
Headers show

Comments

Blue Swirl - April 2, 2010, 7:04 p.m.
On 4/2/10, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 04/02/2010 08:17 PM, Blue Swirl wrote:
>
> > I merged your patch and mine. Does it still look reasonable?
> >
>
>  Yes, of course.  I'd rather see them committed separately though.

This version is designed to be applied after your patch. I made my
patch a bit simpler.
Paolo Bonzini - April 3, 2010, 9:04 a.m.
On 04/02/2010 09:04 PM, Blue Swirl wrote:
> -int kvm_init(int smp_cpus);

I had missed this; I don't see a particular reason to move this out of 
kvm.h.  Anyway I don't feel strongly about this.

Paolo
Blue Swirl - April 3, 2010, 9:07 a.m.
On 4/3/10, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 04/02/2010 09:04 PM, Blue Swirl wrote:
>
> > -int kvm_init(int smp_cpus);
> >
>
>  I had missed this; I don't see a particular reason to move this out of
> kvm.h.  Anyway I don't feel strongly about this.

The reason is to avoid including kvm.h by vl.c.
Paolo Bonzini - April 4, 2010, 7:12 a.m.
On 04/03/2010 11:07 AM, Blue Swirl wrote:
> On 4/3/10, Paolo Bonzini<pbonzini@redhat.com>  wrote:
>> On 04/02/2010 09:04 PM, Blue Swirl wrote:
>>
>>> -int kvm_init(int smp_cpus);
>>>
>>
>>   I had missed this; I don't see a particular reason to move this out of
>> kvm.h.  Anyway I don't feel strongly about this.
>
> The reason is to avoid including kvm.h by vl.c.

But that's not a problem, kvm.h can be included by compiled-once files; 
that was the reason to introduce the stubs in the first place.  kvm_* 
should be declared in kvm.h.

Paolo
Blue Swirl - April 4, 2010, 7:25 a.m.
On 4/4/10, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 04/03/2010 11:07 AM, Blue Swirl wrote:
>
> > On 4/3/10, Paolo Bonzini<pbonzini@redhat.com>  wrote:
> >
> > > On 04/02/2010 09:04 PM, Blue Swirl wrote:
> > >
> > >
> > > > -int kvm_init(int smp_cpus);
> > > >
> > > >
> > >
> > >  I had missed this; I don't see a particular reason to move this out of
> > > kvm.h.  Anyway I don't feel strongly about this.
> > >
> >
> > The reason is to avoid including kvm.h by vl.c.
> >
>
>  But that's not a problem, kvm.h can be included by compiled-once files;
> that was the reason to introduce the stubs in the first place.  kvm_* should
> be declared in kvm.h.

That can't be safe because CONFIG_KVM will not be defined for files
compiled once. So even with the stubs, those files would use inlined
stubs where they shouldn't.
Paolo Bonzini - April 4, 2010, 6:13 p.m.
>>> The reason is to avoid including kvm.h by vl.c.
>>
>>   But that's not a problem, kvm.h can be included by compiled-once files;
>> that was the reason to introduce the stubs in the first place.  kvm_* should
>> be declared in kvm.h.
>
> That can't be safe because CONFIG_KVM will not be defined for files
> compiled once. So even with the stubs, those files would use inlined
> stubs where they shouldn't.

1) Stubs are linked, not inlined.

2) vl.c does not use kvm_enabled

3) even if it did, it is defined like this after my patch:

#if defined CONFIG_KVM || !defined NEED_CPU_H
#define kvm_enabled() (kvm_allowed)
#else
#define kvm_enabled() (0)
#endif

Paolo

Patch

From 1ce35cc1adc0eb54420d6647c3dfc46445788906 Mon Sep 17 00:00:00 2001
From: Blue Swirl <blauwirbel@gmail.com>
Date: Fri, 2 Apr 2010 19:00:35 +0000
Subject: [PATCH] Compile vl.c once

Remove dependency of vl.c to KVM, then we can partially revert
b33612d03540fda7fa67485f1c20395beb7a2bf0.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 Makefile.objs   |    2 +-
 Makefile.target |    2 +-
 kvm-all.c       |    3 +++
 kvm.h           |    4 +---
 sysemu.h        |    1 +
 vl.c            |    7 +++----
 6 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index 11e44a0..cb2ec2c 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -128,7 +128,7 @@  user-obj-y += cutils.o cache-utils.o
 # libhw
 
 hw-obj-y =
-hw-obj-y += loader.o
+hw-obj-y += vl.o loader.o
 hw-obj-y += virtio.o virtio-console.o
 hw-obj-y += fw_cfg.o pci.o pci_host.o pcie_host.o
 hw-obj-y += watchdog.o
diff --git a/Makefile.target b/Makefile.target
index c504617..c1bfc41 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -162,7 +162,7 @@  endif #CONFIG_BSD_USER
 # System emulator target
 ifdef CONFIG_SOFTMMU
 
-obj-y = arch_init.o cpus.o monitor.o machine.o gdbstub.o vl.o
+obj-y = arch_init.o cpus.o monitor.o machine.o gdbstub.o
 # virtio has to be here due to weird dependency between PCI and virtio-net.
 # need to fix this properly
 obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-pci.o virtio-serial-bus.o
diff --git a/kvm-all.c b/kvm-all.c
index 373fd34..f6317ee 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -51,6 +51,8 @@  typedef struct KVMSlot
 
 typedef struct kvm_dirty_log KVMDirtyLog;
 
+int kvm_allowed = 0;
+
 struct KVMState
 {
     KVMSlot slots[32];
@@ -670,6 +672,7 @@  int kvm_init(int smp_cpus)
 
     kvm_state = s;
     cpu_register_phys_memory_client(&kvm_cpu_phys_memory_client);
+    kvm_allowed = 1;
 
     return 0;
 
diff --git a/kvm.h b/kvm.h
index ea3c97d..ec66b2f 100644
--- a/kvm.h
+++ b/kvm.h
@@ -23,9 +23,9 @@ 
 #include <linux/kvm.h>
 #endif
 
+#if defined CONFIG_KVM || !defined NEED_CPU_H
 extern int kvm_allowed;
 
-#if defined CONFIG_KVM || !defined NEED_CPU_H
 #define kvm_enabled() (kvm_allowed)
 #else
 #define kvm_enabled() (0)
@@ -35,8 +35,6 @@  struct kvm_run;
 
 /* external API */
 
-int kvm_init(int smp_cpus);
-
 #ifdef NEED_CPU_H
 int kvm_init_vcpu(CPUState *env);
 
diff --git a/sysemu.h b/sysemu.h
index d0effa0..0b423db 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -243,4 +243,5 @@  void rtc_change_mon_event(struct tm *tm);
 
 void register_devices(void);
 
+int kvm_init(int smp_cpus);
 #endif
diff --git a/vl.c b/vl.c
index 9fe4682..1005163 100644
--- a/vl.c
+++ b/vl.c
@@ -145,7 +145,6 @@  int main(int argc, char **argv)
 #include "dma.h"
 #include "audio/audio.h"
 #include "migration.h"
-#include "kvm.h"
 #include "balloon.h"
 #include "qemu-option.h"
 #include "qemu-config.h"
@@ -241,7 +240,6 @@  uint8_t qemu_uuid[16];
 static QEMUBootSetHandler *boot_set_handler;
 static void *boot_set_opaque;
 
-int kvm_allowed = 0;
 uint32_t xen_domid;
 enum xen_mode xen_mode = XEN_EMULATE;
 
@@ -2649,6 +2647,7 @@  int main(int argc, char **argv, char **envp)
 #endif
     int show_vnc_port = 0;
     int defconfig = 1;
+    int enable_kvm = 0;
 
     error_set_progname(argv[0]);
 
@@ -3235,7 +3234,7 @@  int main(int argc, char **argv, char **envp)
                 do_smbios_option(optarg);
                 break;
             case QEMU_OPTION_enable_kvm:
-                kvm_allowed = 1;
+                enable_kvm = 1;
                 break;
             case QEMU_OPTION_usb:
                 usb_enabled = 1;
@@ -3581,7 +3580,7 @@  int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
-    if (kvm_allowed) {
+    if (enable_kvm) {
         int ret = kvm_init(smp_cpus);
         if (ret < 0) {
             if (!kvm_available()) {
-- 
1.5.6.5