From mboxrd@z Thu Jan  1 00:00:00 1970
From: Radim =?utf-8?B?S3LEjW3DocWZ?= <rkrcmar@redhat.com>
Subject: Re: [PATCH 3/4] KVM: x86: allow 256 logical x2APICs again
Date: Thu, 27 Nov 2014 22:03:51 +0100
Message-ID: <20141127210350.GC7770@potion.brq.redhat.com>
References: <1417114994-25235-1-git-send-email-rkrcmar@redhat.com>
 <1417114994-25235-4-git-send-email-rkrcmar@redhat.com>
 <22F11C34-E2E4-459B-882B-A944AD0853FD@gmail.com>
 <20141127201641.GB383@potion.brq.redhat.com>
 <A459C9D7-B1A8-4507-94FD-272A9FEECFDC@gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	Paolo Bonzini <pbonzini@redhat.com>,
	Gleb Natapov <gleb@kernel.org>,
	Nadav Amit <namit@cs.technion.ac.il>
To: Nadav Amit <nadav.amit@gmail.com>
Return-path: <linux-kernel-owner@vger.kernel.org>
Content-Disposition: inline
In-Reply-To: <A459C9D7-B1A8-4507-94FD-272A9FEECFDC@gmail.com>
Sender: linux-kernel-owner@vger.kernel.org
List-Id: kvm.vger.kernel.org

2014-11-27 22:39+0200, Nadav Amit:
> Radim Kr=C4=8Dm=C3=A1=C5=99 <rkrcmar@redhat.com> wrote:
>=20
> > 2014-11-27 21:53+0200, Nadav Amit:
> >> Radim Kr=C4=8Dm=C3=A1=C5=99 <rkrcmar@redhat.com> wrote:
> >>> -			new->cid_mask =3D (1 << KVM_X2APIC_CID_BITS) - 1;
> >>> -			new->lid_mask =3D 0xffff;
> >>> +			new->cid_mask =3D new->lid_mask =3D 0xffff;
> >> You set cid_mask to 0xffff, while there are only 16 clusters. I th=
ink it is
> >> risky (if you twist my hand would come with a scenario).
> >=20
> > Let's see :) APIC id is 8 bit, and we compute cluster part of LDR b=
y
> > taking four upper bits, so 16 is enough.
> To clarify my concern - I am worried that some of the CPUs are still =
in
> xAPIC mode with LDR that does not follow x2APIC LDR scheme.

xAPIC has only the highest LDR byte nonzero, so it won't pass the lid
check.  (Logical xAPIC doesn't work with x2APIC in our implementation.)

I agree, it is really obsucure and I should have put it in v1.

> > We would incorrectly deliver messages intended for high clusters,
> > it has to be 0xffff.
> From the SDM, I am not sure you are correct, but your solution is fin=
e.

We'd have to change the code in irq_deliver...fast to account for that
change, which I think is uglier than checking here.
(If the message is for 0xf0000001 and 0x00000001 accepts it, SDM doesn'=
t
 approve.)