From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Yang, Sheng" Subject: Re: [PATCH 2/5] KVM: Add irq ack notifier list Date: Tue, 29 Jul 2008 17:56:45 +0800 Message-ID: <200807291756.46376.sheng.yang@intel.com> References: <1217262388-7309-1-git-send-email-benami@il.ibm.com> <200807291514.18097.sheng.yang@intel.com> <200807291504.47162.amit.shah@qumranet.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, Marcelo Tosatti , Ben-Ami Yassour , avi@qumranet.com, muli@il.ibm.com, weidong.han@intel.com, anthony@codemonkey.ws To: Amit Shah Return-path: Received: from mga11.intel.com ([192.55.52.93]:36492 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753840AbYG2JyU (ORCPT ); Tue, 29 Jul 2008 05:54:20 -0400 In-Reply-To: <200807291504.47162.amit.shah@qumranet.com> Content-Disposition: inline Sender: kvm-owner@vger.kernel.org List-ID: On Tuesday 29 July 2008 17:34:47 Amit Shah wrote: > * On Tuesday 29 Jul 2008 12:44:17 Yang, Sheng wrote: > > On Tuesday 29 July 2008 00:26:25 Ben-Ami Yassour wrote: > > > FROM: Avi Kivity > > > > > > This can be used by kvm subsystems that are interested in when > > > interrupts > > > are acked, for example time drift compenstation. > > > > > > [Ben: add notification call to the pic and ioapic] > > > > > > Signed-off-by: Avi Kivity > > > Signed-off-by: Ben-Ami Yassour > > > --- > > > arch/x86/kvm/i8259.c | 1 + > > > arch/x86/kvm/irq.c | 22 ++++++++++++++++++++++ > > > arch/x86/kvm/irq.h | 5 +++++ > > > include/asm-x86/kvm_host.h | 7 +++++++ > > > virt/kvm/ioapic.c | 2 ++ > > > 5 files changed, 37 insertions(+), 0 deletions(-) > > > > > > diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c > > > index 55e179a..d2a61bf 100644 > > > --- a/arch/x86/kvm/i8259.c > > > +++ b/arch/x86/kvm/i8259.c > > > @@ -186,6 +186,7 @@ int kvm_pic_read_irq(struct kvm_pic *s) > > > irq = 7; > > > intno = s->pics[0].irq_base + irq; > > > } > > > + kvm_notify_acked_irq(s->irq_request_opaque, irq); > > > > It's not what I mean, sorry to not tell it clearly... Now it got > > confusing semantic. > > > > irq_request_opaque has nothing to do with acked_irq. What I mean > > is > > The change here uses the irq_request_opaque field which actually is > the kvm struct, pointed out by you, thanks for that. Yeah, I know that... I just meant the meaning of words has no relevant. :) > > > rename irq_request_opaque to struct* kvm in struct kvm_pic, and > > modify all irq_request() calling(three of them in all) with (void > > *)kvm. > > 'opaque' fields can be later made to point to other structures > without changing the structure itself. This is an advantage. Will > the kvm_pic struct be needed to change in the future? Very > unlikely. So we can rename it to struct kvm *, however, that gives > us no real benefit as against opaque (just readability). Yes, the readability... I think people would be very curious about why void kvm_notify_acked_irq(struct kvm *kvm, unsigned gsi) got a irq_request_opaque as a parameter. It's more like a hack, which is not my meaning... Anyway, it's trivial one and just a coding style. :) -- regards Yang, Sheng