| Message ID | 1366133175-23986-3-git-send-email-agraf@suse.de |
|---|---|
| State | New, archived |
| Headers | show |
Il 16/04/2013 19:26, Alexander Graf ha scritto: > Quite a bit of code in KVM has been conditionalized on availability of > IOAPIC emulation. However, most of it is generically applicable to > platforms that don't have an IOPIC, but a different type of irq chip. > > Introduce a new define to distinguish between generic code and IOAPIC > specific code. > > Signed-off-by: Alexander Graf <agraf@suse.de> > --- > arch/x86/include/uapi/asm/kvm.h | 1 + > include/linux/kvm_host.h | 4 ++-- > include/uapi/linux/kvm.h | 2 +- > virt/kvm/eventfd.c | 6 +++--- > 4 files changed, 7 insertions(+), 6 deletions(-) You need to touch arch/ia64 too. Yeah, it's likely broken but let's not break it on purpose. :) Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 17.04.2013, at 13:49, Paolo Bonzini wrote: > Il 16/04/2013 19:26, Alexander Graf ha scritto: >> Quite a bit of code in KVM has been conditionalized on availability of >> IOAPIC emulation. However, most of it is generically applicable to >> platforms that don't have an IOPIC, but a different type of irq chip. >> >> Introduce a new define to distinguish between generic code and IOAPIC >> specific code. >> >> Signed-off-by: Alexander Graf <agraf@suse.de> >> --- >> arch/x86/include/uapi/asm/kvm.h | 1 + >> include/linux/kvm_host.h | 4 ++-- >> include/uapi/linux/kvm.h | 2 +- >> virt/kvm/eventfd.c | 6 +++--- >> 4 files changed, 7 insertions(+), 6 deletions(-) > > You need to touch arch/ia64 too. Yeah, it's likely broken but let's not > break it on purpose. :) It depends on CONFIG_BROKEN, so I'd rather not touch it. Alex -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Il 17/04/2013 13:53, Alexander Graf ha scritto: >>> >> arch/x86/include/uapi/asm/kvm.h | 1 + >>> >> include/linux/kvm_host.h | 4 ++-- >>> >> include/uapi/linux/kvm.h | 2 +- >>> >> virt/kvm/eventfd.c | 6 +++--- >>> >> 4 files changed, 7 insertions(+), 6 deletions(-) >> > >> > You need to touch arch/ia64 too. Yeah, it's likely broken but let's not >> > break it on purpose. :) > It depends on CONFIG_BROKEN, so I'd rather not touch it. Well, we just got a patch to make it at least compile, and it enables a small further cleanup (see review in patch 5), so I think it's worthwhile to add a single #define... Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 17.04.2013, at 13:54, Paolo Bonzini wrote: > Il 17/04/2013 13:53, Alexander Graf ha scritto: >>>>>> arch/x86/include/uapi/asm/kvm.h | 1 + >>>>>> include/linux/kvm_host.h | 4 ++-- >>>>>> include/uapi/linux/kvm.h | 2 +- >>>>>> virt/kvm/eventfd.c | 6 +++--- >>>>>> 4 files changed, 7 insertions(+), 6 deletions(-) >>>> >>>> You need to touch arch/ia64 too. Yeah, it's likely broken but let's not >>>> break it on purpose. :) >> It depends on CONFIG_BROKEN, so I'd rather not touch it. > > Well, we just got a patch to make it at least compile, That was really just Yang (blindly?) fixing an issue I pointed out on ARM. There is no kvm user space support for IA64. The kernel side has been KConfig depending on BROKEN for a year already: commit a6bb7929677aacfce3f864c3cdacaa7d527945d5 Author: Avi Kivity <avi@redhat.com> Date: Thu May 17 13:14:08 2012 +0300 KVM: ia64: Mark ia64 KVM as BROKEN Practically all patches to ia64 KVM are build fixes; numerous warnings remain; the last patch from the maintainer was committed more than three years ago. It is clear that no one is using this thing. Mark as BROKEN to ensure people don't get hit by pointless build problems. Signed-off-by: Avi Kivity <avi@redhat.com> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> Signed-off-by: Avi Kivity <avi@redhat.com> The only patch I could see myself doing in arch/ia64 is an rm -rf arch/ia64/kvm. Alex -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Il 17/04/2013 13:59, Alexander Graf ha scritto: >> > Well, we just got a patch to make it at least compile, > That was really just Yang (blindly?) fixing an issue I pointed out on > ARM. There is no kvm user space support for IA64. The kernel side has > been KConfig depending on BROKEN for a year already: Yes, I know. Still, __KVM_HAVE_IRQCHIP is clearly a subset of __KVM_HAVE_IOAPIC; defining one without the other makes no sense and will cause compilation or link errors for trace_kvm_ack_irq. Either we drop it altogether, or we should not break compilation consciously---especially if the problem is so trivial and obvious that you had to think of leaving it out. Paolo > commit a6bb7929677aacfce3f864c3cdacaa7d527945d5 > Author: Avi Kivity <avi@redhat.com> > Date: Thu May 17 13:14:08 2012 +0300 > > KVM: ia64: Mark ia64 KVM as BROKEN > > Practically all patches to ia64 KVM are build fixes; numerous warnings remain; > the last patch from the maintainer was committed more than three years ago. It > is clear that no one is using this thing. > > Mark as BROKEN to ensure people don't get hit by pointless build problems. > > Signed-off-by: Avi Kivity <avi@redhat.com> > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> > Signed-off-by: Avi Kivity <avi@redhat.com> -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 17.04.2013, at 14:06, Paolo Bonzini wrote: > Il 17/04/2013 13:59, Alexander Graf ha scritto: >>>> Well, we just got a patch to make it at least compile, >> That was really just Yang (blindly?) fixing an issue I pointed out on >> ARM. There is no kvm user space support for IA64. The kernel side has >> been KConfig depending on BROKEN for a year already: > > Yes, I know. > > Still, __KVM_HAVE_IRQCHIP is clearly a subset of __KVM_HAVE_IOAPIC; > defining one without the other makes no sense and will cause compilation > or link errors for trace_kvm_ack_irq. > > Either we drop it altogether, or we should not break compilation > consciously---especially if the problem is so trivial and obvious that > you had to think of leaving it out. I disagree. I actually _want_ to break it on purpose, so we have even more reason to remove all that useless code if nobody complains. Alex -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Il 17/04/2013 14:10, Alexander Graf ha scritto: >>> Still, __KVM_HAVE_IRQCHIP is clearly a subset of >>> __KVM_HAVE_IOAPIC; defining one without the other makes no sense >>> and will cause compilation or link errors for trace_kvm_ack_irq. >>> >>> Either we drop it altogether, or we should not break compilation >>> consciously---especially if the problem is so trivial and obvious >>> that you had to think of leaving it out. > I disagree. I actually _want_ to break it on purpose, so we have even > more reason to remove all that useless code if nobody complains. Then remove it in patch 1, and move all the remaining IOAPIC code back to arch/x86 at the end. As things are, you're just leaving someone else to do the work (no matter if it is to fix it, or to zap it). Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 17.04.2013, at 14:15, Paolo Bonzini wrote: > Il 17/04/2013 14:10, Alexander Graf ha scritto: >>>> Still, __KVM_HAVE_IRQCHIP is clearly a subset of >>>> __KVM_HAVE_IOAPIC; defining one without the other makes no sense >>>> and will cause compilation or link errors for trace_kvm_ack_irq. >>>> >>>> Either we drop it altogether, or we should not break compilation >>>> consciously---especially if the problem is so trivial and obvious >>>> that you had to think of leaving it out. >> I disagree. I actually _want_ to break it on purpose, so we have even >> more reason to remove all that useless code if nobody complains. > > Then remove it in patch 1, and move all the remaining IOAPIC code back > to arch/x86 at the end. As things are, you're just leaving someone else > to do the work (no matter if it is to fix it, or to zap it). Gleb, Marcelo, any objections to me removing ia64? Alex -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 17, 2013 at 02:16:59PM +0200, Alexander Graf wrote: > > On 17.04.2013, at 14:15, Paolo Bonzini wrote: > > > Il 17/04/2013 14:10, Alexander Graf ha scritto: > >>>> Still, __KVM_HAVE_IRQCHIP is clearly a subset of > >>>> __KVM_HAVE_IOAPIC; defining one without the other makes no sense > >>>> and will cause compilation or link errors for trace_kvm_ack_irq. > >>>> > >>>> Either we drop it altogether, or we should not break compilation > >>>> consciously---especially if the problem is so trivial and obvious > >>>> that you had to think of leaving it out. > >> I disagree. I actually _want_ to break it on purpose, so we have even > >> more reason to remove all that useless code if nobody complains. > > > > Then remove it in patch 1, and move all the remaining IOAPIC code back > > to arch/x86 at the end. As things are, you're just leaving someone else > > to do the work (no matter if it is to fix it, or to zap it). > > Gleb, Marcelo, any objections to me removing ia64? > It was marked BROKEN less then year ago. I would give it one more year as a last chance. We can reconsider this during 3.11 development cycle, but if you want this code to make 3.10 can you please fix ia64 compilation. The patch on top of the series is OK. -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h index a65ec29..923478e 100644 --- a/arch/x86/include/uapi/asm/kvm.h +++ b/arch/x86/include/uapi/asm/kvm.h @@ -28,6 +28,7 @@ /* Select x86 specific features in <linux/kvm.h> */ #define __KVM_HAVE_PIT #define __KVM_HAVE_IOAPIC +#define __KVM_HAVE_IRQCHIP #define __KVM_HAVE_IRQ_LINE #define __KVM_HAVE_DEVICE_ASSIGNMENT #define __KVM_HAVE_MSI diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 303d05b..4b30906 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -304,7 +304,7 @@ struct kvm_kernel_irq_routing_entry { struct hlist_node link; }; -#ifdef __KVM_HAVE_IOAPIC +#ifdef __KVM_HAVE_IRQCHIP struct kvm_irq_routing_table { int chip[KVM_NR_IRQCHIPS][KVM_IRQCHIP_NUM_PINS]; @@ -432,7 +432,7 @@ void kvm_vcpu_uninit(struct kvm_vcpu *vcpu); int __must_check vcpu_load(struct kvm_vcpu *vcpu); void vcpu_put(struct kvm_vcpu *vcpu); -#ifdef __KVM_HAVE_IOAPIC +#ifdef __KVM_HAVE_IRQCHIP int kvm_irqfd_init(void); void kvm_irqfd_exit(void); #else diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index 74d0ff3..c38d269 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -579,7 +579,7 @@ struct kvm_ppc_smmu_info { #ifdef __KVM_HAVE_PIT #define KVM_CAP_REINJECT_CONTROL 24 #endif -#ifdef __KVM_HAVE_IOAPIC +#ifdef __KVM_HAVE_IRQCHIP #define KVM_CAP_IRQ_ROUTING 25 #endif #define KVM_CAP_IRQ_INJECT_STATUS 26 diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c index c5d43ff..0797571 100644 --- a/virt/kvm/eventfd.c +++ b/virt/kvm/eventfd.c @@ -35,7 +35,7 @@ #include "iodev.h" -#ifdef __KVM_HAVE_IOAPIC +#ifdef __KVM_HAVE_IRQCHIP /* * -------------------------------------------------------------------- * irqfd: Allows an fd to be used to inject an interrupt to the guest @@ -433,7 +433,7 @@ fail: void kvm_eventfd_init(struct kvm *kvm) { -#ifdef __KVM_HAVE_IOAPIC +#ifdef __KVM_HAVE_IRQCHIP spin_lock_init(&kvm->irqfds.lock); INIT_LIST_HEAD(&kvm->irqfds.items); INIT_LIST_HEAD(&kvm->irqfds.resampler_list); @@ -442,7 +442,7 @@ kvm_eventfd_init(struct kvm *kvm) INIT_LIST_HEAD(&kvm->ioeventfds); } -#ifdef __KVM_HAVE_IOAPIC +#ifdef __KVM_HAVE_IRQCHIP /* * shutdown any irqfd's that match fd+gsi */
Quite a bit of code in KVM has been conditionalized on availability of IOAPIC emulation. However, most of it is generically applicable to platforms that don't have an IOPIC, but a different type of irq chip. Introduce a new define to distinguish between generic code and IOAPIC specific code. Signed-off-by: Alexander Graf <agraf@suse.de> --- arch/x86/include/uapi/asm/kvm.h | 1 + include/linux/kvm_host.h | 4 ++-- include/uapi/linux/kvm.h | 2 +- virt/kvm/eventfd.c | 6 +++--- 4 files changed, 7 insertions(+), 6 deletions(-)