All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Colin King <colin.king@canonical.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	x86@kernel.org, "H . Peter Anvin" <hpa@zytor.com>,
	kvm@vger.kernel.org, kernel-janitors@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH][next] KVM: x86: simplify zero'ing of entry->ebx
Date: Thu, 22 Apr 2021 15:07:57 +0000	[thread overview]
Message-ID: <YIGRTfMm0MfypN22@google.com> (raw)
In-Reply-To: <20210422141129.250525-1-colin.king@canonical.com>

On Thu, Apr 22, 2021, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> Currently entry->ebx is being zero'd by masking itself with zero.
> Simplify this by just assigning zero, cleans up static analysis
> warning.
> 
> Addresses-Coverity: ("Bitwise-and with zero")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  arch/x86/kvm/cpuid.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 57744a5d1bc2..9bcc2ff4b232 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -851,7 +851,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
>  		entry->eax &= SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT |
>  			      SGX_ATTR_PROVISIONKEY | SGX_ATTR_EINITTOKENKEY |
>  			      SGX_ATTR_KSS;
> -		entry->ebx &= 0;
> +		entry->ebx = 0;

I 100% understand the code is funky, but using &= is intentional.  ebx:eax holds
a 64-bit value that is a effectively a set of feature flags.  While the upper
32 bits are extremely unlikely to be used any time soon, if a feature comes
along then the correct behavior would be:

		entry->ebx &= SGX_ATTR_FANCY_NEW_FEATURE;

While directly setting entry->ebx would be incorrect.  The idea is to set up a
future developer for success so that they don't forget to add the "&".

TL;DR: I'd prefer to keep this as is, even though it's rather ridiculous.

>  		break;
>  	/* Intel PT */
>  	case 0x14:
> -- 
> 2.30.2
> 

  reply	other threads:[~2021-04-22 15:08 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-22 14:11 [PATCH][next] KVM: x86: simplify zero'ing of entry->ebx Colin King
2021-04-22 15:07 ` Sean Christopherson [this message]
2021-04-22 15:11   ` Colin Ian King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YIGRTfMm0MfypN22@google.com \
    --to=seanjc@google.com \
    --cc=bp@alien8.de \
    --cc=colin.king@canonical.com \
    --cc=hpa@zytor.com \
    --cc=jmattson@google.com \
    --cc=joro@8bytes.org \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.