Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755965AbcLNNn3 (ORCPT ); Wed, 14 Dec 2016 08:43:29 -0500 Received: from mx1.redhat.com ([209.132.183.28]:46502 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753463AbcLNNn1 (ORCPT ); Wed, 14 Dec 2016 08:43:27 -0500 Date: Wed, 14 Dec 2016 14:41:32 +0100 From: Radim =?utf-8?B?S3LEjW3DocWZ?= To: David Hildenbrand Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, Paolo Bonzini , Igor Mammedov Subject: Re: [PATCH v2 2/4] KVM: x86: replace kvm_apic_id with kvm_{x,x2}apic_id Message-ID: <20161214134131.GA2156@potion> References: <20161213163001.4567-1-rkrcmar@redhat.com> <20161213163001.4567-3-rkrcmar@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Wed, 14 Dec 2016 13:41:35 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1922 Lines: 53 2016-12-14 13:39+0100, David Hildenbrand: > Am 13.12.2016 um 17:29 schrieb Radim Krčmář: >> There were three calls sites: >> - recalculate_apic_map and kvm_apic_match_physical_addr, where it would >> only complicate implementation of x2APIC hotplug; >> - in apic_debug, where it was still somewhat preserved, but keeping the >> old function just for apic_debug was not worth it >> >> Signed-off-by: Radim Krčmář >> --- >> arch/x86/kvm/lapic.c | 41 ++++++++++++++++++++++++++++++----------- >> arch/x86/kvm/lapic.h | 11 ----------- >> 2 files changed, 30 insertions(+), 22 deletions(-) >> >> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c >> index 09edd32b8e42..e645b4bc6437 100644 >> --- a/arch/x86/kvm/lapic.c >> +++ b/arch/x86/kvm/lapic.c >> @@ -115,6 +115,16 @@ static inline int apic_enabled(struct kvm_lapic *apic) >> (LVT_MASK | APIC_MODE_MASK | APIC_INPUT_POLARITY | \ >> APIC_LVT_REMOTE_IRR | APIC_LVT_LEVEL_TRIGGER) >> >> +static inline u8 kvm_xapic_id(struct kvm_lapic *apic) >> +{ >> + return kvm_lapic_get_reg(apic, APIC_ID) >> 24; >> +} >> + >> +static inline u32 kvm_x2apic_id(struct kvm_lapic *apic) >> +{ >> + return apic->vcpu->vcpu_id; >> +} >> + >> static inline bool kvm_apic_map_get_logical_dest(struct kvm_apic_map *map, >> u32 dest_id, struct kvm_lapic ***cluster, u16 *mask) { >> switch (map->mode) { >> @@ -159,13 +169,13 @@ static void recalculate_apic_map(struct kvm *kvm) >> struct kvm_apic_map *new, *old = NULL; >> struct kvm_vcpu *vcpu; >> int i; >> - u32 max_id = 255; >> + u32 max_id = 256; /* enough space for any xAPIC ID */ > > Why exactly do we have to increase this? > > I thought the maximum xapic id is 255, and we correctly allocate > 255+1 slots. > > Now, we would allocate 257 slots and set max_apic_id to 256. Right, it is a typical off-by-one error. The comment is sufficient ... Queued for v3, Thanks.