* [PATCH][next] KVM: x86: simplify zero'ing of entry->ebx
@ 2021-04-22 14:11 Colin King
2021-04-22 15:07 ` Sean Christopherson
0 siblings, 1 reply; 3+ messages in thread
From: Colin King @ 2021-04-22 14:11 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
Jim Mattson, Joerg Roedel, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, x86, H . Peter Anvin, kvm
Cc: kernel-janitors, linux-kernel
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;
break;
/* Intel PT */
case 0x14:
--
2.30.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH][next] KVM: x86: simplify zero'ing of entry->ebx
2021-04-22 14:11 [PATCH][next] KVM: x86: simplify zero'ing of entry->ebx Colin King
@ 2021-04-22 15:07 ` Sean Christopherson
2021-04-22 15:11 ` Colin Ian King
0 siblings, 1 reply; 3+ messages in thread
From: Sean Christopherson @ 2021-04-22 15:07 UTC (permalink / raw)
To: Colin King
Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H . Peter Anvin, kvm, kernel-janitors, linux-kernel
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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH][next] KVM: x86: simplify zero'ing of entry->ebx
2021-04-22 15:07 ` Sean Christopherson
@ 2021-04-22 15:11 ` Colin Ian King
0 siblings, 0 replies; 3+ messages in thread
From: Colin Ian King @ 2021-04-22 15:11 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H . Peter Anvin, kvm, kernel-janitors, linux-kernel
On 22/04/2021 16:07, Sean Christopherson wrote:
> 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.
OK, makes sense. Thanks for explaining.
>
>> break;
>> /* Intel PT */
>> case 0x14:
>> --
>> 2.30.2
>>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-04-22 15:11 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-22 14:11 [PATCH][next] KVM: x86: simplify zero'ing of entry->ebx Colin King
2021-04-22 15:07 ` Sean Christopherson
2021-04-22 15:11 ` Colin Ian King
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).