Patchwork [2/2] msix: Pull in config.h for CONFIG_KVM

login
register
mail settings
Submitter Alex Williamson
Date Oct. 22, 2010, 8:40 p.m.
Message ID <20101022204037.10161.83407.stgit@s20.home>
Download mbox | patch
Permalink /patch/68965/
State New
Headers show

Comments

Alex Williamson - Oct. 22, 2010, 8:40 p.m.
We need to pull in config.h or else kvm.h doesn't pull in
linux/config.h, which we need if we ever want KVM_CAP_IRQCHIP
defined.  This requires moving the object over to Makefile.target
or else we can't find config-target.h

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

 Makefile.objs   |    2 --
 Makefile.target |    1 +
 hw/msix.c       |    1 +
 3 files changed, 2 insertions(+), 2 deletions(-)
Alex Williamson - Oct. 23, 2010, 1:50 a.m.
On Fri, 2010-10-22 at 14:40 -0600, Alex Williamson wrote:
> We need to pull in config.h or else kvm.h doesn't pull in
> linux/config.h, which we need if we ever want KVM_CAP_IRQCHIP
> defined.  This requires moving the object over to Makefile.target
> or else we can't find config-target.h
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
> 
>  Makefile.objs   |    2 --
>  Makefile.target |    1 +
>  hw/msix.c       |    1 +
>  3 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index ca2d2d0..c097d25 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -201,8 +201,6 @@ hw-obj-$(CONFIG_PIIX4) += piix4.o
>  # PCI watchdog devices
>  hw-obj-y += wdt_i6300esb.o
>  
> -hw-obj-y += msix.o
> -
>  # PCI network cards
>  hw-obj-y += ne2000.o
>  hw-obj-y += eepro100.o
> diff --git a/Makefile.target b/Makefile.target
> index 347ad6b..63da13b 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -185,6 +185,7 @@ obj-y += rwhandler.o
>  obj-$(CONFIG_KVM) += kvm.o kvm-all.o
>  obj-$(CONFIG_NO_KVM) += kvm-stub.o
>  obj-y += memory.o
> +obj-y += msix.o

Oops, memory.c isn't upstream, I'll push it down in my patch queue and
send a new one.

Alex

>  LIBS+=-lz
>  
> diff --git a/hw/msix.c b/hw/msix.c
> index 4122395..23256c9 100644
> --- a/hw/msix.c
> +++ b/hw/msix.c
> @@ -11,6 +11,7 @@
>   * the COPYING file in the top-level directory.
>   */
>  
> +#include "config.h"
>  #include "hw.h"
>  #include "msix.h"
>  #include "pci.h"
>
Paolo Bonzini - Oct. 23, 2010, 7:41 a.m.
On 10/23/2010 03:50 AM, Alex Williamson wrote:
> Oops, memory.c isn't upstream, I'll push it down in my patch queue and
> send a new one.

Neither is kvm_set_irq actually. :)  This patch is only needed for qemu-kvm.

BTW, maybe the better solution would be to move the kvm_*_irq* functions 
from qemu-kvm.c to kvm-all.c, add stubs to kvm-stub.c, and get rid of 
the #ifdef completely in msix.c

Paolo
Michael S. Tsirkin - Oct. 23, 2010, 4:16 p.m.
On Fri, Oct 22, 2010 at 02:40:39PM -0600, Alex Williamson wrote:
> We need to pull in config.h or else kvm.h doesn't pull in
> linux/config.h, which we need if we ever want KVM_CAP_IRQCHIP
> defined.  This requires moving the object over to Makefile.target
> or else we can't find config-target.h
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>

Why? We just moved it from .target to .objs, see
889e30cc18e21f2091b77267dca8096d7dd34f8b.

> ---
> 
>  Makefile.objs   |    2 --
>  Makefile.target |    1 +
>  hw/msix.c       |    1 +
>  3 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index ca2d2d0..c097d25 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -201,8 +201,6 @@ hw-obj-$(CONFIG_PIIX4) += piix4.o
>  # PCI watchdog devices
>  hw-obj-y += wdt_i6300esb.o
>  
> -hw-obj-y += msix.o
> -
>  # PCI network cards
>  hw-obj-y += ne2000.o
>  hw-obj-y += eepro100.o
> diff --git a/Makefile.target b/Makefile.target
> index 347ad6b..63da13b 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -185,6 +185,7 @@ obj-y += rwhandler.o
>  obj-$(CONFIG_KVM) += kvm.o kvm-all.o
>  obj-$(CONFIG_NO_KVM) += kvm-stub.o
>  obj-y += memory.o
> +obj-y += msix.o
>  
>  LIBS+=-lz
>  
> diff --git a/hw/msix.c b/hw/msix.c
> index 4122395..23256c9 100644
> --- a/hw/msix.c
> +++ b/hw/msix.c
> @@ -11,6 +11,7 @@
>   * the COPYING file in the top-level directory.
>   */
>  
> +#include "config.h"
>  #include "hw.h"
>  #include "msix.h"
>  #include "pci.h"
Alex Williamson - Oct. 23, 2010, 4:52 p.m.
On Sat, 2010-10-23 at 18:16 +0200, Michael S. Tsirkin wrote:
> On Fri, Oct 22, 2010 at 02:40:39PM -0600, Alex Williamson wrote:
> > We need to pull in config.h or else kvm.h doesn't pull in
> > linux/config.h, which we need if we ever want KVM_CAP_IRQCHIP
> > defined.  This requires moving the object over to Makefile.target
> > or else we can't find config-target.h
> > 
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> 
> Why? We just moved it from .target to .objs, see
> 889e30cc18e21f2091b77267dca8096d7dd34f8b.

Maybe that's why it used to work.  When building in the qemu-kvm.git
tree, I'm not getting CONFIG_KVM defined, which means I'm not getting
KVM_CAP_IRQCHIP defined, which results in msix_irq_entries not being
allocated.  Then when I call msix_vector_use, I get a seg fault.
Something is broken there.  Thanks,

Alex

> > ---
> > 
> >  Makefile.objs   |    2 --
> >  Makefile.target |    1 +
> >  hw/msix.c       |    1 +
> >  3 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Makefile.objs b/Makefile.objs
> > index ca2d2d0..c097d25 100644
> > --- a/Makefile.objs
> > +++ b/Makefile.objs
> > @@ -201,8 +201,6 @@ hw-obj-$(CONFIG_PIIX4) += piix4.o
> >  # PCI watchdog devices
> >  hw-obj-y += wdt_i6300esb.o
> >  
> > -hw-obj-y += msix.o
> > -
> >  # PCI network cards
> >  hw-obj-y += ne2000.o
> >  hw-obj-y += eepro100.o
> > diff --git a/Makefile.target b/Makefile.target
> > index 347ad6b..63da13b 100644
> > --- a/Makefile.target
> > +++ b/Makefile.target
> > @@ -185,6 +185,7 @@ obj-y += rwhandler.o
> >  obj-$(CONFIG_KVM) += kvm.o kvm-all.o
> >  obj-$(CONFIG_NO_KVM) += kvm-stub.o
> >  obj-y += memory.o
> > +obj-y += msix.o
> >  
> >  LIBS+=-lz
> >  
> > diff --git a/hw/msix.c b/hw/msix.c
> > index 4122395..23256c9 100644
> > --- a/hw/msix.c
> > +++ b/hw/msix.c
> > @@ -11,6 +11,7 @@
> >   * the COPYING file in the top-level directory.
> >   */
> >  
> > +#include "config.h"
> >  #include "hw.h"
> >  #include "msix.h"
> >  #include "pci.h"
Michael S. Tsirkin - Oct. 23, 2010, 5:29 p.m.
On Sat, Oct 23, 2010 at 10:52:43AM -0600, Alex Williamson wrote:
> On Sat, 2010-10-23 at 18:16 +0200, Michael S. Tsirkin wrote:
> > On Fri, Oct 22, 2010 at 02:40:39PM -0600, Alex Williamson wrote:
> > > We need to pull in config.h or else kvm.h doesn't pull in
> > > linux/config.h, which we need if we ever want KVM_CAP_IRQCHIP
> > > defined.  This requires moving the object over to Makefile.target
> > > or else we can't find config-target.h
> > > 
> > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > 
> > Why? We just moved it from .target to .objs, see
> > 889e30cc18e21f2091b77267dca8096d7dd34f8b.
> 
> Maybe that's why it used to work.  When building in the qemu-kvm.git
> tree, I'm not getting CONFIG_KVM defined, which means I'm not getting
> KVM_CAP_IRQCHIP defined, which results in msix_irq_entries not being
> allocated.  Then when I call msix_vector_use, I get a seg fault.
> Something is broken there.  Thanks,
> 
> Alex

This is hopefully fixed in the latest bits.
bd8b215bce453706c3951460cc7e6627ccb90314
Alex Williamson - Oct. 23, 2010, 6:42 p.m.
On Sat, 2010-10-23 at 19:29 +0200, Michael S. Tsirkin wrote:
> On Sat, Oct 23, 2010 at 10:52:43AM -0600, Alex Williamson wrote:
> > On Sat, 2010-10-23 at 18:16 +0200, Michael S. Tsirkin wrote:
> > > On Fri, Oct 22, 2010 at 02:40:39PM -0600, Alex Williamson wrote:
> > > > We need to pull in config.h or else kvm.h doesn't pull in
> > > > linux/config.h, which we need if we ever want KVM_CAP_IRQCHIP
> > > > defined.  This requires moving the object over to Makefile.target
> > > > or else we can't find config-target.h
> > > > 
> > > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > > 
> > > Why? We just moved it from .target to .objs, see
> > > 889e30cc18e21f2091b77267dca8096d7dd34f8b.
> > 
> > Maybe that's why it used to work.  When building in the qemu-kvm.git
> > tree, I'm not getting CONFIG_KVM defined, which means I'm not getting
> > KVM_CAP_IRQCHIP defined, which results in msix_irq_entries not being
> > allocated.  Then when I call msix_vector_use, I get a seg fault.
> > Something is broken there.  Thanks,
> > 
> > Alex
> 
> This is hopefully fixed in the latest bits.
> bd8b215bce453706c3951460cc7e6627ccb90314

Nope, my tree includes that.  It's not kvm_set_irq, it's kvm_msix_add,
which dereferences msix_irq_entries, which is only allocated in
msix_init if KVM_CAP_IRQCHIP is defined, which it's not.  Maybe you also
meant to remove the ifdef from msix_init?  I also note there's another
in msix_notify.  Thanks,

Alex
Michael S. Tsirkin - Oct. 23, 2010, 8:38 p.m.
On Sat, Oct 23, 2010 at 12:42:44PM -0600, Alex Williamson wrote:
> On Sat, 2010-10-23 at 19:29 +0200, Michael S. Tsirkin wrote:
> > On Sat, Oct 23, 2010 at 10:52:43AM -0600, Alex Williamson wrote:
> > > On Sat, 2010-10-23 at 18:16 +0200, Michael S. Tsirkin wrote:
> > > > On Fri, Oct 22, 2010 at 02:40:39PM -0600, Alex Williamson wrote:
> > > > > We need to pull in config.h or else kvm.h doesn't pull in
> > > > > linux/config.h, which we need if we ever want KVM_CAP_IRQCHIP
> > > > > defined.  This requires moving the object over to Makefile.target
> > > > > or else we can't find config-target.h
> > > > > 
> > > > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > > > 
> > > > Why? We just moved it from .target to .objs, see
> > > > 889e30cc18e21f2091b77267dca8096d7dd34f8b.
> > > 
> > > Maybe that's why it used to work.  When building in the qemu-kvm.git
> > > tree, I'm not getting CONFIG_KVM defined, which means I'm not getting
> > > KVM_CAP_IRQCHIP defined, which results in msix_irq_entries not being
> > > allocated.  Then when I call msix_vector_use, I get a seg fault.
> > > Something is broken there.  Thanks,
> > > 
> > > Alex
> > 
> > This is hopefully fixed in the latest bits.
> > bd8b215bce453706c3951460cc7e6627ccb90314
> 
> Nope, my tree includes that.  It's not kvm_set_irq, it's kvm_msix_add,
> which dereferences msix_irq_entries, which is only allocated in
> msix_init if KVM_CAP_IRQCHIP is defined, which it's not.  Maybe you also
> meant to remove the ifdef from msix_init?  I also note there's another
> in msix_notify.  Thanks,
> 
> Alex

Not sure what's wrong:
$git grep KVM_CAP_IRQCHIP origin/master -- hw/msix.c
$

http://git.kernel.org/?p=virt/kvm/qemu-kvm.git;a=blob;f=hw/msix.c;hb=HEAD

also does not show any ifdefs.
Alex Williamson - Oct. 23, 2010, 9:01 p.m.
On Sat, Oct 23, 2010 at 2:38 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Sat, Oct 23, 2010 at 12:42:44PM -0600, Alex Williamson wrote:
>> On Sat, 2010-10-23 at 19:29 +0200, Michael S. Tsirkin wrote:
>> > On Sat, Oct 23, 2010 at 10:52:43AM -0600, Alex Williamson wrote:
>> > > On Sat, 2010-10-23 at 18:16 +0200, Michael S. Tsirkin wrote:
>> > > > On Fri, Oct 22, 2010 at 02:40:39PM -0600, Alex Williamson wrote:
>> > > > > We need to pull in config.h or else kvm.h doesn't pull in
>> > > > > linux/config.h, which we need if we ever want KVM_CAP_IRQCHIP
>> > > > > defined.  This requires moving the object over to Makefile.target
>> > > > > or else we can't find config-target.h
>> > > > >
>> > > > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
>> > > >
>> > > > Why? We just moved it from .target to .objs, see
>> > > > 889e30cc18e21f2091b77267dca8096d7dd34f8b.
>> > >
>> > > Maybe that's why it used to work.  When building in the qemu-kvm.git
>> > > tree, I'm not getting CONFIG_KVM defined, which means I'm not getting
>> > > KVM_CAP_IRQCHIP defined, which results in msix_irq_entries not being
>> > > allocated.  Then when I call msix_vector_use, I get a seg fault.
>> > > Something is broken there.  Thanks,
>> > >
>> > > Alex
>> >
>> > This is hopefully fixed in the latest bits.
>> > bd8b215bce453706c3951460cc7e6627ccb90314
>>
>> Nope, my tree includes that.  It's not kvm_set_irq, it's kvm_msix_add,
>> which dereferences msix_irq_entries, which is only allocated in
>> msix_init if KVM_CAP_IRQCHIP is defined, which it's not.  Maybe you also
>> meant to remove the ifdef from msix_init?  I also note there's another
>> in msix_notify.  Thanks,
>>
>> Alex
>
> Not sure what's wrong:
> $git grep KVM_CAP_IRQCHIP origin/master -- hw/msix.c
> $
>
> http://git.kernel.org/?p=virt/kvm/qemu-kvm.git;a=blob;f=hw/msix.c;hb=HEAD
>
> also does not show any ifdefs.

Hmm, somehow my tree missed 763a04a920f1098e57ad6b46c91c3e531adc961d
Ignore this patch and I'll refresh again.  Sorry for the noise.

Alex

Patch

diff --git a/Makefile.objs b/Makefile.objs
index ca2d2d0..c097d25 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -201,8 +201,6 @@  hw-obj-$(CONFIG_PIIX4) += piix4.o
 # PCI watchdog devices
 hw-obj-y += wdt_i6300esb.o
 
-hw-obj-y += msix.o
-
 # PCI network cards
 hw-obj-y += ne2000.o
 hw-obj-y += eepro100.o
diff --git a/Makefile.target b/Makefile.target
index 347ad6b..63da13b 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -185,6 +185,7 @@  obj-y += rwhandler.o
 obj-$(CONFIG_KVM) += kvm.o kvm-all.o
 obj-$(CONFIG_NO_KVM) += kvm-stub.o
 obj-y += memory.o
+obj-y += msix.o
 
 LIBS+=-lz
 
diff --git a/hw/msix.c b/hw/msix.c
index 4122395..23256c9 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -11,6 +11,7 @@ 
  * the COPYING file in the top-level directory.
  */
 
+#include "config.h"
 #include "hw.h"
 #include "msix.h"
 #include "pci.h"