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 X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 00728C4361B for ; Fri, 18 Dec 2020 09:20:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 9655223A7D for ; Fri, 18 Dec 2020 09:20:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388965AbgLRJUQ (ORCPT ); Fri, 18 Dec 2020 04:20:16 -0500 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:31141 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732777AbgLRJUO (ORCPT ); Fri, 18 Dec 2020 04:20:14 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1608283128; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=ejcHzuD9fFPSUG7/hU65XXRSqtWaCKdWCMMhWat4xMU=; b=iC6R/RP7TzoErWvZBPlF3P/T3jyt1ec09A5sGQEjgfEkP7hljYY0cB7vBPYpoJAQnQyw9Z unbFtcKQurEWXtUHuaxnc+PQ63pD4w2GDdXZ5+5qGxPpg9JtFDJMXnJ3oHsQIuEQbiUBRx r8K/FJxC8QSkScZqFCZoKv3YneaeeSs= Received: from mail-ed1-f71.google.com (mail-ed1-f71.google.com [209.85.208.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-181-b8GP4o6mOpSRRnhCDVOO0Q-1; Fri, 18 Dec 2020 04:18:46 -0500 X-MC-Unique: b8GP4o6mOpSRRnhCDVOO0Q-1 Received: by mail-ed1-f71.google.com with SMTP id y19so818746edw.16 for ; Fri, 18 Dec 2020 01:18:46 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references:date :message-id:mime-version; bh=ejcHzuD9fFPSUG7/hU65XXRSqtWaCKdWCMMhWat4xMU=; b=eIX0FyZm25/0iROi+7p77dEe/C2DOSySI8g7zM5XH8u9n535yuF0SRbGIS0y0R3sBQ f0ZaMqw5zqKxEjylHJS/S3dxr2fbmW/bsdfGPG0Mw+piPhHlSdugIrOi6WblRX8ePao7 hFc36lnHe8x+NQJvmXDcUUEqvt1iSw65zqNRuPgMmRHcR4JyDiisoTbjH3O8JllNGGfz i2zdXoiNKxK7LgN/XJg6MKECepYJ4zed/K5AoB9lcYK4CkIjt2e8Ekpiaqxw+dcRtIFS hUZZjXhCVXHTeciTBcxtXEWQR0V9ULuos72ja1rCVnQ32cA1iXiUSDsm9rbwhPwAXT3H PytQ== X-Gm-Message-State: AOAM530+AFtx3wFaXhrUQSg7is2yhwuyLE8Llu1i59GknUrh/P9oP2dW BpOTqiqdfZsH56rsekMcb82KT6G3O2vmT8kcp8Kc9PKHf5QkQ4rLh2rm+InK4TDQY9G7WxFInKz +OmvV3hnXSoSgd3cVHy4nhqhz X-Received: by 2002:a17:906:cc9c:: with SMTP id oq28mr3078134ejb.224.1608283125341; Fri, 18 Dec 2020 01:18:45 -0800 (PST) X-Google-Smtp-Source: ABdhPJzRlPQTAZK6n6UAHdwvwvC4SS1JkxSjJMpsyMSplOZqd1Lw14LdiOywycWCjjf7X9vdO2EVfA== X-Received: by 2002:a17:906:cc9c:: with SMTP id oq28mr3078122ejb.224.1608283125176; Fri, 18 Dec 2020 01:18:45 -0800 (PST) Received: from vitty.brq.redhat.com (g-server-2.ign.cz. [91.219.240.2]) by smtp.gmail.com with ESMTPSA id cf17sm24352748edb.16.2020.12.18.01.18.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 18 Dec 2020 01:18:44 -0800 (PST) From: Vitaly Kuznetsov To: Sean Christopherson , Paolo Bonzini Cc: Sean Christopherson , Wanpeng Li , Jim Mattson , Joerg Roedel , kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Ben Gardon , Richard Herbert Subject: Re: [PATCH 3/4] KVM: x86/mmu: Use raw level to index into MMIO walks' sptes array In-Reply-To: <20201218003139.2167891-4-seanjc@google.com> References: <20201218003139.2167891-1-seanjc@google.com> <20201218003139.2167891-4-seanjc@google.com> Date: Fri, 18 Dec 2020 10:18:43 +0100 Message-ID: <87o8irtqto.fsf@vitty.brq.redhat.com> MIME-Version: 1.0 Content-Type: text/plain Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Sean Christopherson writes: > Bump the size of the sptes array by one and use the raw level of the > SPTE to index into the sptes array. Using the SPTE level directly > improves readability by eliminating the need to reason out why the level > is being adjusted when indexing the array. The array is on the stack > and is not explicitly initialized; bumping its size is nothing more than > a superficial adjustment to the stack frame. > > Signed-off-by: Sean Christopherson > --- > arch/x86/kvm/mmu/mmu.c | 15 +++++++-------- > arch/x86/kvm/mmu/tdp_mmu.c | 2 +- > 2 files changed, 8 insertions(+), 9 deletions(-) > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > index 52f36c879086..4798a4472066 100644 > --- a/arch/x86/kvm/mmu/mmu.c > +++ b/arch/x86/kvm/mmu/mmu.c > @@ -3500,7 +3500,7 @@ static int get_walk(struct kvm_vcpu *vcpu, u64 addr, u64 *sptes, int *root_level > leaf = iterator.level; > spte = mmu_spte_get_lockless(iterator.sptep); > > - sptes[leaf - 1] = spte; > + sptes[leaf] = spte; > > if (!is_shadow_present_pte(spte)) > break; > @@ -3514,7 +3514,7 @@ static int get_walk(struct kvm_vcpu *vcpu, u64 addr, u64 *sptes, int *root_level > /* return true if reserved bit is detected on spte. */ > static bool get_mmio_spte(struct kvm_vcpu *vcpu, u64 addr, u64 *sptep) > { > - u64 sptes[PT64_ROOT_MAX_LEVEL]; > + u64 sptes[PT64_ROOT_MAX_LEVEL + 1]; > struct rsvd_bits_validate *rsvd_check; > int root, leaf, level; > bool reserved = false; > @@ -3537,16 +3537,15 @@ static bool get_mmio_spte(struct kvm_vcpu *vcpu, u64 addr, u64 *sptep) > rsvd_check = &vcpu->arch.mmu->shadow_zero_check; > > for (level = root; level >= leaf; level--) { > - if (!is_shadow_present_pte(sptes[level - 1])) > + if (!is_shadow_present_pte(sptes[level])) > break; > /* > * Use a bitwise-OR instead of a logical-OR to aggregate the > * reserved bit and EPT's invalid memtype/XWR checks to avoid > * adding a Jcc in the loop. > */ > - reserved |= __is_bad_mt_xwr(rsvd_check, sptes[level - 1]) | > - __is_rsvd_bits_set(rsvd_check, sptes[level - 1], > - level); > + reserved |= __is_bad_mt_xwr(rsvd_check, sptes[level]) | > + __is_rsvd_bits_set(rsvd_check, sptes[level], level); > } > > if (reserved) { > @@ -3554,10 +3553,10 @@ static bool get_mmio_spte(struct kvm_vcpu *vcpu, u64 addr, u64 *sptep) > __func__, addr); > for (level = root; level >= leaf; level--) > pr_err("------ spte 0x%llx level %d.\n", > - sptes[level - 1], level); > + sptes[level], level); > } > > - *sptep = sptes[leaf - 1]; > + *sptep = sptes[leaf]; > > return reserved; > } > diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c > index a4f9447f8327..efef571806ad 100644 > --- a/arch/x86/kvm/mmu/tdp_mmu.c > +++ b/arch/x86/kvm/mmu/tdp_mmu.c > @@ -1160,7 +1160,7 @@ int kvm_tdp_mmu_get_walk(struct kvm_vcpu *vcpu, u64 addr, u64 *sptes, > > tdp_mmu_for_each_pte(iter, mmu, gfn, gfn + 1) { > leaf = iter.level; > - sptes[leaf - 1] = iter.old_spte; > + sptes[leaf] = iter.old_spte; > } > > return leaf; An alretnitive solution would've been to reverse the array and fill it like sptes[PT64_ROOT_MAX_LEVEL - leaf] = iter.old_spte; but this may not reach the goal of 'improved readability' :-) Also, we may add an MMU_DEBUG ifdef-ed check that sptes[0] remains intact. Reviewed-by: Vitaly Kuznetsov -- Vitaly