From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759521AbcGKQE6 (ORCPT ); Mon, 11 Jul 2016 12:04:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49401 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755085AbcGKQE4 (ORCPT ); Mon, 11 Jul 2016 12:04:56 -0400 Subject: Re: [PATCH v2 04/13] KVM: x86: dynamic kvm_apic_map To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= References: <20160707171550.14675-1-rkrcmar@redhat.com> <20160707171550.14675-5-rkrcmar@redhat.com> <963b542a-1111-db83-8338-c32d44f98874@gmail.com> <3a5d86b6-9f1a-a6cf-8af4-ef6bf3936996@gmail.com> <20160711134837.GD21976@potion> <6dda9370-e538-2c7c-e95e-b6a6f1518bbf@redhat.com> <20160711155229.GE21976@potion> Cc: Yang Zhang , linux-kernel@vger.kernel.org, kvm@vger.kernel.org, "Lan, Tianyu" , Igor Mammedov , Jan Kiszka , Peter Xu From: Paolo Bonzini Message-ID: <67c6a1d4-86f1-a4b6-2ba3-891f091fc1db@redhat.com> Date: Mon, 11 Jul 2016 18:04:51 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <20160711155229.GE21976@potion> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Mon, 11 Jul 2016 16:04:56 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/07/2016 17:52, Radim Krčmář wrote: > 2016-07-11 16:14+0200, Paolo Bonzini: >> On 11/07/2016 15:48, Radim Krčmář wrote: >>>>> I guess the easiest solution is to replace kvm_apic_id with a field in >>>>> struct kvm_lapic, which is already shifted right by 24 in xAPIC mode. >>> >>> (I guess the fewest LOC is to look at vcpu->vcpu_id, which is equal to >>> x2apic id. xapic id cannot be greater than 255 and all of those are >>> covered by the initial value of max_id.) >> >> Yes, this would work too. Or even better perhaps, look at vcpu->vcpu_id >> in kvm_apic_id? > > APIC ID is writeable in xAPIC mode, which would make the implementation > weird without an extra variable. Always read-only APIC ID would be > best, IMO. You can do if (x2apic mode) return lapic->vcpu->vcpu_id; else return get_reg(APIC_ID) >> 24; The point is to avoid returning a shifted APIC_ID without shifting it. The alternative of course is just caching it, which at this point is not particularly harder... Paolo >>> (What makes a bit wary is that it doesn't avoid the same problem if we >>> changed KVM to reset apic id to xapic id first when disabling apic.) >> >> Yes, this is why I prefer it fixed once and for all in kvm_apic_id... > > Seems most reasonable. We'll need to be careful to have a correct value > in the apic page, but there shouldn't be any races there. > >>> Races in recalculation and APIC ID changes also lead to invalid physical >>> maps, which haven't been taken care of properly ... >> >> Hmm, true, but can be fixed separately. Probably the mutex should be >> renamed so that it can be taken outside recalculate_apic_map... > > Good point, it'll make reasoning easier and shouldn't introduce any > extra scalability issues.