From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sheng Yang Subject: Re: [PATCH 5/8] KVM: Using gsi_msg mapping for MSI device assignment Date: Wed, 24 Dec 2008 10:41:05 +0800 Message-ID: <200812241041.05458.sheng@linux.intel.com> References: <1230019231-16543-1-git-send-email-sheng@linux.intel.com> <1230019231-16543-6-git-send-email-sheng@linux.intel.com> <20081223180515.GC5449@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Cc: Avi Kivity , kvm@vger.kernel.org To: Marcelo Tosatti Return-path: Received: from mga06.intel.com ([134.134.136.21]:18345 "EHLO orsmga101.jf.intel.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751764AbYLXClI (ORCPT ); Tue, 23 Dec 2008 21:41:08 -0500 In-Reply-To: <20081223180515.GC5449@amt.cnet> Content-Disposition: inline Sender: kvm-owner@vger.kernel.org List-ID: On Wednesday 24 December 2008 02:05:15 Marcelo Tosatti wrote: > On Tue, Dec 23, 2008 at 04:00:28PM +0800, Sheng Yang wrote: > > Convert MSI userspace interface to support gsi_msg mapping(and nobody > > should be the user of the old interface...). > > > > Signed-off-by: Sheng Yang > > --- > > include/linux/kvm_host.h | 1 - > > virt/kvm/kvm_main.c | 33 ++++++++++++++++++++------------- > > 2 files changed, 20 insertions(+), 14 deletions(-) > > > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > > index db9de47..50b3ff6 100644 > > --- a/virt/kvm/kvm_main.c > > +++ b/virt/kvm/kvm_main.c > > @@ -92,20 +92,28 @@ static void assigned_device_msi_dispatch(struct > > kvm_assigned_dev_kernel *dev) int vcpu_id; > > struct kvm_vcpu *vcpu; > > struct kvm_ioapic *ioapic = ioapic_irqchip(dev->kvm); > > - int dest_id = (dev->guest_msi.address_lo & MSI_ADDR_DEST_ID_MASK) > > - >> MSI_ADDR_DEST_ID_SHIFT; > > - int vector = (dev->guest_msi.data & MSI_DATA_VECTOR_MASK) > > - >> MSI_DATA_VECTOR_SHIFT; > > - int dest_mode = test_bit(MSI_ADDR_DEST_MODE_SHIFT, > > - (unsigned long *)&dev->guest_msi.address_lo); > > - int trig_mode = test_bit(MSI_DATA_TRIGGER_SHIFT, > > - (unsigned long *)&dev->guest_msi.data); > > - int delivery_mode = test_bit(MSI_DATA_DELIVERY_MODE_SHIFT, > > - (unsigned long *)&dev->guest_msi.data); > > + struct kvm_gsi_msg *gsi_msg = > > + kvm_find_gsi_msg(dev->kvm, dev->guest_irq); > > kvm_find_gsi_msg requires the gsi_msg mutex held? Missed, thanks! > > > + delivery_mode = test_bit(MSI_DATA_DELIVERY_MODE_SHIFT, > > + (unsigned long *)&gsi_msg->msg.data); > > deliver_bitmask = kvm_ioapic_get_delivery_bitmask(ioapic, > > dest_id, dest_mode); > > /* IOAPIC delivery mode value is the same as MSI here */ > > Please unify the IOAPIC specific code in assigned_device_msi_dispatch > with ioapic_deliver. I meant to do this, but seems it won't reduce the complexity much... We got a large number of input(delivery mode), and two or three kind of output(for > FIXED, LOWPRI, and NMI for IOAPIC). What I can think of is unify output to a delivery_mask, and use something like: > > for (vcpu_id = 0; deliver_bitmask != 0; vcpu_id++) { > if (!(deliver_bitmask & (1 << vcpu_id))) > continue; > deliver_bitmask &= ~(1 << vcpu_id); > vcpu = ioapic->kvm->vcpus[vcpu_id]; > if (vcpu) { > r = xxx_inj_irq(ioapic, vcpu, vector, > trig_mode, delivery_mode); > } > } And xxx_inj_irq() need a judgment for the delivery mode NMI in ioapic... Maybe we can reduce ~20 lines by refactoring this. I will give a try... (Um, duplicate is always unbearable thing) -- regards Yang, Sheng