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 842EAC001B0 for ; Wed, 16 Aug 2023 14:28:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1343616AbjHPO2A (ORCPT ); Wed, 16 Aug 2023 10:28:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33948 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1343651AbjHPO1s (ORCPT ); Wed, 16 Aug 2023 10:27:48 -0400 Received: from mail-yw1-x114a.google.com (mail-yw1-x114a.google.com [IPv6:2607:f8b0:4864:20::114a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 047B12709 for ; Wed, 16 Aug 2023 07:27:47 -0700 (PDT) Received: by mail-yw1-x114a.google.com with SMTP id 00721157ae682-58c8b2d6784so9439897b3.3 for ; Wed, 16 Aug 2023 07:27:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1692196066; x=1692800866; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=ocFVuDAoYa76ZBIYvekfvcCSNlPl+8WKQ7zX3J4QGgs=; b=nCDyOoH0oySdjuEsVcz9lUQEbt0d5xVy4T04c2jCprIc8UdB2GwFoxZz1UihZ+8CPg RchUF9UVRX/0ijV1r23vCJ9RJtbt225PJnFS1+zPdaWLb69pEXxa5GMMopLuXBVh3+R1 sI1GJOp8Yub6sMF0nBNjzN7CFf+Ii5MjUbyJkFBmWMRnnNLk4rtsozCNHU3XROmtlzk3 ETsES3u+oZGHW5tIJukvZ7SakW0f6EChxmcX5V1bkjju+6zhsKN2qR3r722lg/ESZznc 2ErdicH2oyAau+bkEu+pVQQa95zmnDny8w93x0oGM2RRgSu4R1tHE8vQN35d1UBO1auN uxsQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1692196066; x=1692800866; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ocFVuDAoYa76ZBIYvekfvcCSNlPl+8WKQ7zX3J4QGgs=; b=DpPLusXNjN3jAeTDAaKDB8ovHVE12AGbleU1EMgUYBUAY8Qtmcpiv5jL7zKlgSeZdu xNL++Xh18DRPhwePmkuFTTxyfeIk00wPPW6JtSOy+nWipd2VsYy2V5qXVYTPuk0P7MaB oFHu6dKSGLywVusqwMB8N6wEiMeLbPN4p0KavoNCPRAEHEAX6kEpsSRc8w4a6RsPjSE2 j5WGGpX798hItryMdacn0mvjPofkvH2QUBgpn1Vq71giWhPBdHqYJoobj8Y5PmNax4Ku l105muDnUrJ9Gt1eXide7a50hE0DwpfaibmX12CkbKU3wW8u33mExUlxHDFsn4hyVoWm eAYg== X-Gm-Message-State: AOJu0YyiL5K1HOiyJ82tNnx208FEs+T0shlOJ+KeqZspuY2Q1gFAcVkd CA6cA7HyYvoIKUV8bUywXb6VY46rPUk= X-Google-Smtp-Source: AGHT+IGmENvyzABpuuLUU3IFL4Ul5a1yBeBSAE3t8ZD0pBQsXUo3eUdpcxEPHaxQ0CpfwWt/G+VCpF8iq3g= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a81:4524:0:b0:58c:8552:458d with SMTP id s36-20020a814524000000b0058c8552458dmr28452ywa.3.1692196066288; Wed, 16 Aug 2023 07:27:46 -0700 (PDT) Date: Wed, 16 Aug 2023 07:27:44 -0700 In-Reply-To: Mime-Version: 1.0 References: <20230719024558.8539-1-guang.zeng@intel.com> <20230719024558.8539-5-guang.zeng@intel.com> Message-ID: Subject: Re: [PATCH v2 4/8] KVM: x86: Add X86EMUL_F_INVTLB and pass it in em_invlpg() From: Sean Christopherson To: Binbin Wu Cc: Zeng Guang , Paolo Bonzini , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , H Peter Anvin , kvm@vger.kernel.org, x86@kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="us-ascii" Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Wed, Aug 16, 2023, Binbin Wu wrote: > > > On 8/16/2023 7:11 AM, Sean Christopherson wrote: > > On Wed, Jul 19, 2023, Zeng Guang wrote: > > > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > > > index 8e706d19ae45..9b4b3ce6d52a 100644 > > > --- a/arch/x86/kvm/emulate.c > > > +++ b/arch/x86/kvm/emulate.c > > > @@ -3443,8 +3443,10 @@ static int em_invlpg(struct x86_emulate_ctxt *ctxt) > > > { > > > int rc; > > > ulong linear; > > > + unsigned max_size; > > unsigned int > Let me think why I use 'unsigned'... > It's because the exist code uses 'unsigned'. > I suppose it is considered bad practice? Yeah, use "unsigned int" when writing new code. > I will cleanup the exist code as well. Is it OK to cleanup it > opportunistically inside this patch? No, don't bother cleaning up existing usage. If a patch touches the "bad" code, then by all means do an opportunistic cleanup. But we have too much "legacy" code in KVM for a wholesale cleanup of bare unsigned usage to be worth the churn and git blame pollution. See also: https://lore.kernel.org/all/ZNvIRS%2FYExLtGO2B@google.com > > > diff --git a/arch/x86/kvm/kvm_emulate.h b/arch/x86/kvm/kvm_emulate.h > > > index c0e48f4fa7c4..c944055091e1 100644 > > > --- a/arch/x86/kvm/kvm_emulate.h > > > +++ b/arch/x86/kvm/kvm_emulate.h > > > @@ -93,6 +93,7 @@ struct x86_instruction_info { > > > #define X86EMUL_F_FETCH BIT(1) > > > #define X86EMUL_F_BRANCH BIT(2) > > > #define X86EMUL_F_IMPLICIT BIT(3) > > > +#define X86EMUL_F_INVTLB BIT(4) > > Why F_INVTLB instead of X86EMUL_F_INVLPG? Ah, because LAM is ignored for the > > linear address in the INVPCID and INVVPID descriptors. Hrm. > > > > I think my vote is to call this X86EMUL_F_INVLPG even though *in theory* it's not > > strictly limited to INVLPG. Odds are good KVM's emulator will never support > > INVPCID or INVVPID, > One case is kvm_handle_invpcid() is in the common kvm x86 code. > LAM doesn't apply to the address in descriptor of invpcid though, but I am > not sure if there will be the need for SVM in the future. Right, but the emulator itself doesn't handle INVPCID or INVVPID, so there's no direct "conflict" at this time. > But for now, F_INVLPG is OK if you think F_INVTLB brings confusion. Yeah, please use F_INVLPG unless someone has a strong objection.