Re: [PATCH 05/10] KVM: Merge MSI handling to kvm_set_irq

[Sheng Yang <sheng@linux.intel.com>]

On Thursday 08 January 2009 05:39:32 Marcelo Tosatti wrote:
> On Wed, Jan 07, 2009 at 06:42:41PM +0800, Sheng Yang wrote:
> > Using kvm_set_irq to handle all interrupt injection.
> >
> > Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> > ---
> >  include/linux/kvm_host.h |    2 +-
> >  virt/kvm/irq_comm.c      |   79
> > +++++++++++++++++++++++++++++++++++++++++++-- virt/kvm/kvm_main.c      | 
> >  79 +++------------------------------------------- 3 files changed, 81
> > insertions(+), 79 deletions(-)
> >
> > +static void gsi_dispatch(struct kvm *kvm, u32 gsi)
> > +{
> > +	int vcpu_id;
> > +	struct kvm_vcpu *vcpu;
> > +	struct kvm_ioapic *ioapic = ioapic_irqchip(kvm);
> > +	struct kvm_gsi_route_entry *gsi_entry;
> > +	int dest_id, vector, dest_mode, trig_mode, delivery_mode;
> > +	u32 deliver_bitmask;
> > +
> > +	BUG_ON(!ioapic);
> > +
> > +	gsi_entry = kvm_find_gsi_route_entry(kvm, gsi);
> > +	if (!gsi_entry) {
> > +		printk(KERN_WARNING "kvm: fail to find correlated gsi entry\n");
> > +		return;
> > +	}
> > +
> > +#ifdef CONFIG_X86
> > +	if (gsi_entry->type & KVM_GSI_ROUTE_MSI) {
> > +		dest_id = (gsi_entry->msi.address_lo & MSI_ADDR_DEST_ID_MASK)
> > +			>> MSI_ADDR_DEST_ID_SHIFT;
> > +		vector = (gsi_entry->msi.data & MSI_DATA_VECTOR_MASK)
> > +			>> MSI_DATA_VECTOR_SHIFT;
> > +		dest_mode = test_bit(MSI_ADDR_DEST_MODE_SHIFT,
> > +				(unsigned long *)&gsi_entry->msi.address_lo);
> > +		trig_mode = test_bit(MSI_DATA_TRIGGER_SHIFT,
> > +				(unsigned long *)&gsi_entry->msi.data);
> > +		delivery_mode = test_bit(MSI_DATA_DELIVERY_MODE_SHIFT,
> > +				(unsigned long *)&gsi_entry->msi.data);
> > +		deliver_bitmask = kvm_ioapic_get_delivery_bitmask(ioapic,
> > +				dest_id, dest_mode);
> > +		/* IOAPIC delivery mode value is the same as MSI here */
> > +		switch (delivery_mode) {
>
> Sheng,
>
> This code seems to ignore the RH bit (MSI_ADDR_REDIRECTION_SHIFT):
>
> 4.Destination mode (DM) — This bit indicates whether the Destination
> ID field should be interpreted as logical or physical APIC ID for
> delivery of the lowest priority interrupt. If RH is 1 and DM is 0,
> the Destination ID field is in physical destination mode and only the
> processor in the system that has the matching APIC ID is considered
> for delivery of that interrupt (this means no re-direction). If RH
> is 1 and DM is 1, the Destination ID Field is interpreted as in
> logical destination mode and the redirection is limited to only those
> processors that are part of the logical group of processors based
> on the processor’s logical APIC ID and the Destination ID field
> in the message. The logical group of processors consists of those
> identified by matching the 8-bit Destination ID with the logical
> destination identified by the Destination Format Register and the
> Logical Destination Register in each local APIC.
>
> Is that intentional?

Um... Partly... For RH bits is almost always 1... OK, I would add another 
patch for this bit later.

-- 
regards
Yang, Sheng