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 <[email protected]>
> > ---
> >  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

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to