Re: [PATCH] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq

["Radim Krčmář" <rkrcmar@redhat.com>]

2015-03-09 23:05-0600, James Sullivan:
> Hi folks,
> 
> This is a small patch that implements logic to handle the RH bit
> being set in the msi message address register. Currently the DM bit is
> the only thing used to decide irq->dest_mode (logical when DM set,
> physical when unset). Documentation indicates that the DM bit will be
> ignored when the RH bit is unset, and physical destination mode is used
> in this case.

(I found that https://software.intel.com/en-us/forums/topic/288883 agrees
 with this definition of "ignored"; and it explains the weird design at
 the bottom.)

> Fixed this to set irq->dest_mode to logical just when both bits are set,
> and physical otherwise.
> 
> Signed-off-by: James Sullivan <sullivan.james.f@gmail.com>
> ---
> diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
> @@ -97,18 +97,26 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
>  static inline void kvm_set_msi_irq(struct kvm_kernel_irq_routing_entry *e,
>  				   struct kvm_lapic_irq *irq)
>  {
> +	bool phys;
>  	trace_kvm_msi_set_irq(e->msi.address_lo, e->msi.data);
> 
>  	irq->dest_id = (e->msi.address_lo &
>  			MSI_ADDR_DEST_ID_MASK) >> MSI_ADDR_DEST_ID_SHIFT;
>  	irq->vector = (e->msi.data &
>  			MSI_DATA_VECTOR_MASK) >> MSI_DATA_VECTOR_SHIFT;
> -	irq->dest_mode = (1 << MSI_ADDR_DEST_MODE_SHIFT) & e->msi.address_lo;
> +	/*
> +	 * Set dest_mode to logical just in case both the RH and DM
> +	 * bits are set, otherwise default to physical.
> +	 */
> +	phys = ((e->msi.address_lo & (MSI_ADDR_REDIRECTION_LOWPRI |
> +			MSI_ADDR_DEST_MODE_LOGICAL)) !=
> +		(MSI_ADDR_REDIRECTION_LOWPRI |
> +			MSI_ADDR_DEST_MODE_LOGICAL));
> +	irq->dest_mode = phys ? 0 : (MSI_ADDR_DEST_MODE_LOGICAL);

(Should be APIC_DEST_LOGICAL.  All works because it is a boolean and we
 only checked for APIC_DEST_PHYSICAL, which is 0.)

>  	irq->trig_mode = (1 << MSI_DATA_TRIGGER_SHIFT) & e->msi.data;
>  	irq->delivery_mode = e->msi.data & 0x700;
>  	irq->level = 1;
>  	irq->shorthand = 0;
> -	/* TODO Deal with RH bit of MSI message address */

RH bit still isn't deal with -- we do not use lowest-priority-like
delivery in logical destination mode ...

How does DM=1/RH=1 work on real hardware?
(There seem to be interesting corner cases with irq->delivery_mode like
 APIC_DM_NMI.)

Thanks.