Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965576AbcDLVyP (ORCPT ); Tue, 12 Apr 2016 17:54:15 -0400 Received: from mail-wm0-f66.google.com ([74.125.82.66]:34780 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933721AbcDLVyO (ORCPT ); Tue, 12 Apr 2016 17:54:14 -0400 Subject: Re: [PART1 RFC v4 06/11] KVM: x86: Detect and Initialize AVIC support To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Suravee Suthikulpanit 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> 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 From: Paolo Bonzini Message-ID: <570D6E7E.20000@redhat.com> Date: Tue, 12 Apr 2016 23:54:06 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1 MIME-Version: 1.0 In-Reply-To: <20160411204823.GA1284@potion.brq.redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1843 Lines: 58 On 11/04/2016 22:48, Radim Krčmář 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=1 or during >> kvm-amd module loading with parameter avic=1. >> >> 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 >> >> #include "irq.h" >> @@ -78,6 +81,11 @@ MODULE_DEVICE_TABLE(x86cpu, svm_cpu_id); >> #define TSC_RATIO_MIN 0x0000000000000001ULL >> #define TSC_RATIO_MAX 0x000000ffffffffffULL >> >> +#define AVIC_HPA_MASK ~((0xFFFULL << 52) || 0xFFF) >> + >> +/* NOTE: Current max index allowed for physical APIC ID table is 255 */ >> +#define AVIC_PHYSICAL_ID_MAX 0xFF > > 0xff is broadcast, so shouldn't the actual last one be 0xfe? Right, actually 0xFF is the maximum *number* of physical APICs (numbered 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 = 0xff is used for broadcast. > + * APIC ID > 0xff is reserved. > + */ > + if (index >= 0xff) Paolo