linux-kernel - Re: [PATCH] KVM: x86: Fix errors in vmcs12.c

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <036435eb-40c2-548a-1cfd-93a364f95634@wanadoo.fr>
Date:   Wed, 12 Jul 2023 19:32:15 +0200
From:   Christophe JAILLET <christophe.jaillet@...adoo.fr>
To:     shijie001@...suo.com, tglx@...utronix.de, mingo@...hat.com
Cc:     Hpa <hpa@...or.com>, Kvm <kvm@...r.kernel.org>,
        Linux Kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] KVM: x86: Fix errors in vmcs12.c

Le 12/07/2023 à 10:40, shijie001@...suo.com a écrit :
> The following checkpatch errors are removed:
> ERROR: space prohibited before open square bracket '['
> ERROR: Macros with complex values should be enclosed in parentheses
> 
> Signed-off-by: Jie Shi <shijie001@...suo.com>
> ---
>   arch/x86/kvm/vmx/vmcs12.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/vmcs12.c b/arch/x86/kvm/vmx/vmcs12.c
> index 106a72c923ca..da239ca58f90 100644
> --- a/arch/x86/kvm/vmx/vmcs12.c
> +++ b/arch/x86/kvm/vmx/vmcs12.c
> @@ -4,10 +4,10 @@
>   #include "vmcs12.h"
> 
>   #define VMCS12_OFFSET(x) offsetof(struct vmcs12, x)
> -#define FIELD(number, name)    [ROL16(number, 6)] = VMCS12_OFFSET(name)
> +#define FIELD(number, name)[ROL16(number, 6)] = VMCS12_OFFSET(name)

Hi,

Written this way, this is really counter-intuitive.
I think that the checkpatch warning should be ignored in this case.

>   #define FIELD64(number, name)                        \
>       FIELD(number, name),                        \
> -    [ROL16(number##_HIGH, 6)] = VMCS12_OFFSET(name) + sizeof(u32)
> +    [ROL16(number##_HIGH, 6)] = (VMCS12_OFFSET(name) + sizeof(u32))

This does not silence the checkpatch warning.
I think that the checkpatch warning should also be ignored in this case.

> 
>   const unsigned short vmcs12_field_offsets[] = {
>       FIELD(VIRTUAL_PROCESSOR_ID, virtual_processor_id),
> 

checkpatch output should always be taken with a grain of salt.
It just runs some heuristics on what looks improvable, but it is not THE 
law.

It is just a tool that can help in many cases, but not all.

Here, I think that the code is better as-is.


CJ