From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PART1 RFC v4 06/11] KVM: x86: Detect and Initialize AVIC support Date: Tue, 12 Apr 2016 23:54:06 +0200 Message-ID: <570D6E7E.20000@redhat.com> References: <1460017232-17429-1-git-send-email-Suravee.Suthikulpanit@amd.com> <1460017232-17429-7-git-send-email-Suravee.Suthikulpanit@amd.com> <20160411204823.GA1284@potion.brq.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: joro@8bytes.org, bp@alien8.de, gleb@kernel.org, alex.williamson@redhat.com, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, wei@redhat.com, sherry.hurwitz@amd.com To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Suravee Suthikulpanit Return-path: In-Reply-To: <20160411204823.GA1284@potion.brq.redhat.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 11/04/2016 22:48, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote: > 2016-04-07 03:20-0500, Suravee Suthikulpanit: >> This patch introduces AVIC-related data structure, and AVIC >> initialization code. >> >> There are three main data structures for AVIC: >> * Virtual APIC (vAPIC) backing page (per-VCPU) >> * Physical APIC ID table (per-VM) >> * Logical APIC ID table (per-VM) >> >> Currently, AVIC is disabled by default. Users can manually >> enable AVIC via kernel boot option kvm-amd.avic=3D1 or during >> kvm-amd module loading with parameter avic=3D1. >> >> Signed-off-by: Suravee Suthikulpanit >> --- >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c >> @@ -14,6 +14,9 @@ >> * the COPYING file in the top-level directory. >> * >> */ >> + >> +#define pr_fmt(fmt) "SVM: " fmt >> + >> #include >> =20 >> #include "irq.h" >> @@ -78,6 +81,11 @@ MODULE_DEVICE_TABLE(x86cpu, svm_cpu_id); >> #define TSC_RATIO_MIN 0x0000000000000001ULL >> #define TSC_RATIO_MAX 0x000000ffffffffffULL >> =20 >> +#define AVIC_HPA_MASK ~((0xFFFULL << 52) || 0xFFF) >> + >> +/* NOTE: Current max index allowed for physical APIC ID table is 25= 5 */ >> +#define AVIC_PHYSICAL_ID_MAX 0xFF >=20 > 0xff is broadcast, so shouldn't the actual last one be 0xfe? Right, actually 0xFF is the maximum *number* of physical APICs (numbere= d 0 to 254). But the code is correct and written for that convention, so we should just rename the macro: /* 0xff is broadcast, so the max index allowed for physical APIC ID * table is 0xfe. APIC IDs above 0xff are reserved. */ #define AVIC_MAX_PHYSICAL_ID_COUNT 255 You can then remove the comment where Radim pointed out you should use the constant: > + /* Note: APIC ID =3D 0xff is used for broadcast. > + * APIC ID > 0xff is reserved. > + */ > + if (index >=3D 0xff) Paolo