From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 55298C43217 for ; Mon, 4 Apr 2022 15:29:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1378266AbiDDPbT (ORCPT ); Mon, 4 Apr 2022 11:31:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53426 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1349937AbiDDPbQ (ORCPT ); Mon, 4 Apr 2022 11:31:16 -0400 Received: from mail-pg1-x52a.google.com (mail-pg1-x52a.google.com [IPv6:2607:f8b0:4864:20::52a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7FFE525289 for ; Mon, 4 Apr 2022 08:29:19 -0700 (PDT) Received: by mail-pg1-x52a.google.com with SMTP id s21so954150pgs.4 for ; Mon, 04 Apr 2022 08:29:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=WwaoVUBFVj9j7YaVgwWeUQJL/8oOimF01hIs03GSwEs=; b=KZsqHqLKTZjQ/n4pzslaH4ezeqg3fhSolbXjJDiLWpQ/F7MVzuHnt9f78kdKHToQch G70BhWHUbPG0PFNkIq0GPgyX81ktwvl+QHt8S/DabeaSL0+Iwo61HVAvu1HIBIY8S5yp tBulvFhDY/WVo/ktoFXDlHK0vM9E0BPOGCq4frsH67nAc9UFAmZQVYyLIZX1GWYJbVIo Qp9I2Ss9mAvRnWQ0sfLgx6U7mHlf1V5UPYz7zTqeXdXYXYAnZrrD7z8mYc2jz+gy5AQ6 9bRf4e6+0bJXpY3TR7YpsYxf50s9DZmxdEuNLIbT70dw6bLeUEjGMqzhAOlx0x3+Rlg5 Yn8Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=WwaoVUBFVj9j7YaVgwWeUQJL/8oOimF01hIs03GSwEs=; b=EV/V/xxCySEIHV29+vXOf/ny42uuowgm6Zu6lWbslI5g4i+DdePS8Pzn8mvc5URqLY /pqHK5Yxw/SZ9SYUNaG2ZCtbblYhSHZQZv92ACLkX+0qYchFSjc6B9JUReRswwGJRS5w ykp559/NXhR64o577iOauGoBGc490sGAbcl1W1u6ltWsneUtVKKseGF88Xhihvtn5/Pv dzHCPhlgmS5TSr+NuUMBGr8+/mhiqYgstAd7HCoArs6/JydmJuIcBxb06QuoAURrjujz KFAVg3p6ME3zzZJ32D+8b3QdpVHxeiJ5FwKaz1Uu+vxkhGyznU6tV/+XEg3c3qnRaL0T iaIA== X-Gm-Message-State: AOAM530B1964sgAsnL2zBIQfUIXBfRW68+Dtgp0uoPgGhjrKvjrOnhLw 5lrOK+5eNKfJzsA4F2Jst+G0SA== X-Google-Smtp-Source: ABdhPJwChen+B+MRC8z3HtJ9ZPqGfIpYJlCB7ilek3qAIXp3MkMC00v5kKiyVolwyc8KYvTSZ9Tagg== X-Received: by 2002:a63:d456:0:b0:399:4c5a:2682 with SMTP id i22-20020a63d456000000b003994c5a2682mr308856pgj.573.1649086158737; Mon, 04 Apr 2022 08:29:18 -0700 (PDT) Received: from google.com (157.214.185.35.bc.googleusercontent.com. [35.185.214.157]) by smtp.gmail.com with ESMTPSA id u11-20020a056a00158b00b004fb07effe2esm13383793pfk.130.2022.04.04.08.29.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 04 Apr 2022 08:29:17 -0700 (PDT) Date: Mon, 4 Apr 2022 15:29:13 +0000 From: Sean Christopherson To: Zeng Guang Cc: Paolo Bonzini , Vitaly Kuznetsov , Wanpeng Li , Jim Mattson , Joerg Roedel , "kvm@vger.kernel.org" , Dave Hansen , "Luck, Tony" , Kan Liang , Thomas Gleixner , Ingo Molnar , Borislav Petkov , "H. Peter Anvin" , Kim Phillips , Jarkko Sakkinen , Jethro Beekman , "Huang, Kai" , "x86@kernel.org" , "linux-kernel@vger.kernel.org" , "Hu, Robert" , "Gao, Chao" Subject: Re: [PATCH v7 5/8] KVM: x86: Add support for vICR APIC-write VM-Exits in x2APIC mode Message-ID: References: <20220304080725.18135-1-guang.zeng@intel.com> <20220304080725.18135-6-guang.zeng@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Apr 02, 2022, Zeng Guang wrote: > > > > - /* TODO: optimize to just emulate side effect w/o one more write */ > > > - kvm_lapic_reg_write(vcpu->arch.apic, offset, val); > > > + kvm_lapic_msr_read(apic, offset, &val); > > > + kvm_apic_send_ipi(apic, (u32)val, (u32)(val >> 32)); > > This needs to clear the APIC_ICR_BUSY bit. It'd also be nice to trace this write. > > The easiest thing is to use kvm_x2apic_icr_write(). Kinda silly as it'll generate > > an extra write, but on the plus side the TODO comment doesn't have to move :-D > > > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > > index c4c3155d98db..58bf296ee313 100644 > > --- a/arch/x86/kvm/lapic.c > > +++ b/arch/x86/kvm/lapic.c > > @@ -2230,6 +2230,7 @@ void kvm_apic_write_nodecode(struct kvm_vcpu *vcpu, u32 offset) > > struct kvm_lapic *apic = vcpu->arch.apic; > > u64 val; > > > > + /* TODO: optimize to just emulate side effect w/o one more write */ > > if (apic_x2apic_mode(apic)) { > > /* > > * When guest APIC is in x2APIC mode and IPI virtualization > > @@ -2240,10 +2241,9 @@ void kvm_apic_write_nodecode(struct kvm_vcpu *vcpu, u32 offset) > > return; > > > > kvm_lapic_msr_read(apic, offset, &val); > > - kvm_apic_send_ipi(apic, (u32)val, (u32)(val >> 32)); > > + kvm_x2apic_icr_write(apic, val); > > As SDM section 10.12.9 "ICR Operation in X2APIC mode" says "Delivery status > bit is removed since it is not needed in x2APIC mode" , so that's not > necessary to clear the APIC_ICR_BUSY bit here. Alternatively we can add trace > to this write by hardware. That same section later says With the removal of the Delivery Status bit, system software no longer has a reason to read the ICR. It remains readable only to aid in debugging; however, software should not assume the value returned by reading the ICR is the last written value. which means that it's at least legal for a hypervisor to clear the busy bit. That might be useful for debugging IPI issues? Probably a bit of a stretch, e.g. I doubt any kernels set the busy bit. But, I do think the tracing would be helpful, and at that point, the extra code should be an AND+MOV. I don't have a super strong opinion, and I'm being somewhat hypocritical (see commit b51818afdc1d ("KVM: SVM: Don't rewrite guest ICR on AVIC IPI virtualization failure"), though that has dedicated tracing), so either approach works for me.