From mboxrd@z Thu Jan  1 00:00:00 1970
From: James Sullivan <sullivan.james.f@gmail.com>
Subject: Re: [PATCH] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq
Date: Tue, 10 Mar 2015 16:39:42 -0600
Message-ID: <54FF72AE.3070507@gmail.com>
References: <54FE7B9E.8020202@gmail.com> <20150310144758.GA18573@potion.brq.redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: kvm@vger.kernel.org, gleb@kernel.org, pbonzini@redhat.com
To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= <rkrcmar@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-pd0-f169.google.com ([209.85.192.169]:45708 "EHLO
	mail-pd0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1753062AbbCJWlw (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 10 Mar 2015 18:41:52 -0400
Received: by pdjy10 with SMTP id y10so5710193pdj.12
        for <kvm@vger.kernel.org>; Tue, 10 Mar 2015 15:41:52 -0700 (PDT)
In-Reply-To: <20150310144758.GA18573@potion.brq.redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 03/10/2015 08:47 AM, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote:
=2E..
>> +	/*
>> +	 * Set dest_mode to logical just in case both the RH and DM
>> +	 * bits are set, otherwise default to physical.
>> +	 */
>> +	phys =3D ((e->msi.address_lo & (MSI_ADDR_REDIRECTION_LOWPRI |
>> +			MSI_ADDR_DEST_MODE_LOGICAL)) !=3D
>> +		(MSI_ADDR_REDIRECTION_LOWPRI |
>> +			MSI_ADDR_DEST_MODE_LOGICAL));
>> +	irq->dest_mode =3D phys ? 0 : (MSI_ADDR_DEST_MODE_LOGICAL);
>=20
> (Should be APIC_DEST_LOGICAL.  All works because it is a boolean and =
we
>  only checked for APIC_DEST_PHYSICAL, which is 0.)
>=20

Thank you, that should be as follows:

    irq->dest_mode =3D phys ? (APIC_DEST_PHYSICAL) : (APIC_DEST_LOGICAL=
);

?

>>  	irq->trig_mode =3D (1 << MSI_DATA_TRIGGER_SHIFT) & e->msi.data;
>>  	irq->delivery_mode =3D e->msi.data & 0x700;
>>  	irq->level =3D 1;
>>  	irq->shorthand =3D 0;
>> -	/* TODO Deal with RH bit of MSI message address */
>=20
> RH bit still isn't deal with -- we do not use lowest-priority-like
> delivery in logical destination mode ...

Just want to make sure I understand this comment-
Isn't low-pri delivery mode used in kvm_irq_delivery_to_apic_fast
when irq->dest_mode > APIC_DEST_PHYSICAL (ie, logical)? (See below.)

Do you mean that this patch will interfere with this? As long as
irq->dest_mode is still APIC_DEST_LOGICAL this shouldn't change.


bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *s=
rc,
		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map)
{
=2E....
    	if (irq->dest_mode =3D=3D APIC_DEST_PHYSICAL) {
		if (irq->dest_id >=3D ARRAY_SIZE(map->phys_map))
			goto out;

		dst =3D &map->phys_map[irq->dest_id];
	} else {
		u32 mda =3D irq->dest_id << (32 - map->ldr_bits);
		u16 cid =3D apic_cluster_id(map, mda);

		if (cid >=3D ARRAY_SIZE(map->logical_map))
			goto out;

		dst =3D map->logical_map[cid];

		bitmap =3D apic_logical_id(map, mda);

		if (irq->delivery_mode =3D=3D APIC_DM_LOWEST) {
			int l =3D -1;
			for_each_set_bit(i, &bitmap, 16) {
				if (!dst[i])
					continue;
				if (l < 0)
					l =3D i;
				else if (kvm_apic_compare_prio(dst[i]->vcpu, dst[l]->vcpu) < 0)
					l =3D i;
			}

			bitmap =3D (l >=3D 0) ? 1 << l : 0;
		}
	}
=2E....
}

> How does DM=3D1/RH=3D1 work on real hardware?
> (There seem to be interesting corner cases with irq->delivery_mode li=
ke
>  APIC_DM_NMI.)
>=20

The IA32 manual says that if DM=3D1/RH=3D1, we operate in logical desti=
nation mode
similarly to other IPIs. I don't believe this patch introduces any inva=
lid
settings listed in section 10-21, Vol. 3, so this shouldn't create any =
weirdness.

Thanks
-James