From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752037AbcGAIgZ (ORCPT ); Fri, 1 Jul 2016 04:36:25 -0400 Received: from mail-wm0-f66.google.com ([74.125.82.66]:35879 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750803AbcGAIgW (ORCPT ); Fri, 1 Jul 2016 04:36:22 -0400 Subject: Re: [PATCH v1 06/11] KVM: x86: use hardware-compatible format for APIC ID register To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , linux-kernel@vger.kernel.org, kvm@vger.kernel.org References: <20160630205429.16480-1-rkrcmar@redhat.com> <20160630205429.16480-7-rkrcmar@redhat.com> Cc: "Lan, Tianyu" , Igor Mammedov , Jan Kiszka , Peter Xu From: Paolo Bonzini Message-ID: <733b706a-f30f-3854-5fc7-402d6b5fd79a@redhat.com> Date: Fri, 1 Jul 2016 10:33:20 +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: <20160630205429.16480-7-rkrcmar@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 On 30/06/2016 22:54, Radim Krčmář wrote: > +static void __kvm_apic_state_fixup(struct kvm_vcpu *vcpu, > + struct kvm_lapic_state *s, bool set) > +{ > + if (apic_x2apic_mode(vcpu->arch.apic)) { > + u32 *id = (u32 *)(s->regs + APIC_ID); > + if (set) > + *id >>= 24; > + else > + *id <<= 24; > + } When setting, this should read from the apic_base being set. So I think you cannot use apic_x2apic_mode. With this change, you don't need the struct kvm_vcpu argument now; add a struct kvm argument instead in patch 10. > @@ -2780,6 +2780,7 @@ static int kvm_vcpu_ioctl_get_lapic(struct kvm_vcpu *vcpu, > kvm_x86_ops->sync_pir_to_irr(vcpu); > > memcpy(s->regs, vcpu->arch.apic->regs, sizeof *s); > + kvm_apic_state_get_fixup(vcpu, s); Instead of kvm_apic_state_get/set_fixup, group the memcpy and __kvm_apic_state_fixup in a new function kvm_apic_get_state(struct kvm_lapic *apic, char *regs). > return 0; > } > @@ -2787,6 +2788,7 @@ static int kvm_vcpu_ioctl_get_lapic(struct kvm_vcpu *vcpu, > static int kvm_vcpu_ioctl_set_lapic(struct kvm_vcpu *vcpu, > struct kvm_lapic_state *s) > { > + kvm_apic_state_set_fixup(vcpu, s); > kvm_apic_post_state_restore(vcpu, s); ... and likewise merge these two in a refactored kvm_apic_post_state_restore called kvm_apic_set_state(struct kvm_lapic *apic, char *regs), by calling __kvm_apic_state_fixup before kvm_apic_post_state_restore's memcpy. With these changes I guess there's no need for the underscores in __kvm_apic_state_fixup. And you can also change the struct kvm_lapic_state pointer in the function to a char *. Thanks, Paolo