2021-03-18 07:48:07

by xuyihang

[permalink] [raw]
Subject: [PATCH -next] x86: Fix unused variable 'hi'

Fixes the following W=1 kernel build warning(s):
arch/x86/hyperv/hv_apic.c:58:15: warning: variable ‘hi’ set but not used [-Wunused-but-set-variable]

Reported-by: Hulk Robot <[email protected]>
Signed-off-by: Xu Yihang <[email protected]>
---
arch/x86/hyperv/hv_apic.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
index 284e73661a18..c0b0a5774f31 100644
--- a/arch/x86/hyperv/hv_apic.c
+++ b/arch/x86/hyperv/hv_apic.c
@@ -55,7 +55,8 @@ static void hv_apic_icr_write(u32 low, u32 id)

static u32 hv_apic_read(u32 reg)
{
- u32 reg_val, hi;
+ u32 hi __maybe_unused;
+ u32 reg_val;

switch (reg) {
case APIC_EOI:
--
2.17.1


2021-03-18 11:26:50

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH -next] x86: Fix unused variable 'hi'


* Xu Yihang <[email protected]> wrote:

> Fixes the following W=1 kernel build warning(s):
> arch/x86/hyperv/hv_apic.c:58:15: warning: variable ‘hi’ set but not used [-Wunused-but-set-variable]
>
> Reported-by: Hulk Robot <[email protected]>
> Signed-off-by: Xu Yihang <[email protected]>
> ---
> arch/x86/hyperv/hv_apic.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
> index 284e73661a18..c0b0a5774f31 100644
> --- a/arch/x86/hyperv/hv_apic.c
> +++ b/arch/x86/hyperv/hv_apic.c
> @@ -55,7 +55,8 @@ static void hv_apic_icr_write(u32 low, u32 id)
>
> static u32 hv_apic_read(u32 reg)
> {
> - u32 reg_val, hi;
> + u32 hi __maybe_unused;
> + u32 reg_val;
>
> switch (reg) {
> case APIC_EOI:

Why and under what config does this function trigger the warning?

Thanks,

Ingo

2021-03-19 18:59:49

by Michael Kelley (LINUX)

[permalink] [raw]
Subject: RE: [PATCH -next] x86: Fix unused variable 'hi'

From: Ingo Molnar <[email protected]> Sent: Thursday, March 18, 2021 4:24 AM
>
> * Xu Yihang <[email protected]> wrote:
>
> > Fixes the following W=1 kernel build warning(s):
> > arch/x86/hyperv/hv_apic.c:58:15: warning: variable ‘hi’ set but not used [-Wunused-but-
> set-variable]
> >
> > Reported-by: Hulk Robot <[email protected]>
> > Signed-off-by: Xu Yihang <[email protected]>
> > ---
> > arch/x86/hyperv/hv_apic.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
> > index 284e73661a18..c0b0a5774f31 100644
> > --- a/arch/x86/hyperv/hv_apic.c
> > +++ b/arch/x86/hyperv/hv_apic.c
> > @@ -55,7 +55,8 @@ static void hv_apic_icr_write(u32 low, u32 id)
> >
> > static u32 hv_apic_read(u32 reg)
> > {
> > - u32 reg_val, hi;
> > + u32 hi __maybe_unused;
> > + u32 reg_val;
> >
> > switch (reg) {
> > case APIC_EOI:
>
> Why and under what config does this function trigger the warning?
>
> Thanks,
>
> Ingo

The warning should trigger if CONFIG_HYPERV is "m" or "y", and W=1
is selected. The variable is indeed set but not used because only the
low order 32 bits of the synthetic MSR are relevant, but rdmsr() returns
both the low 32 and the high 32 bits.

Michael

2021-03-22 03:56:50

by xuyihang

[permalink] [raw]
Subject:


Git message updated.

2021-03-23 02:54:42

by xuyihang

[permalink] [raw]
Subject: [PATCH -next] x86: Fix unused variable 'hi'

Fixes the following W=1 kernel build warning(s):
arch/x86/hyperv/hv_apic.c:58:15: warning: variable ‘hi’ set but not used [-Wunused-but-set-variable]

Compiled with CONFIG_HYPERV enabled:
make allmodconfig ARCH=x86_64 CROSS_COMPILE=x86_64-linux-gnu-
make W=1 arch/x86/hyperv/hv_apic.o ARCH=x86_64 CROSS_COMPILE=x86_64-linux-gnu-

HV_X64_MSR_EOI stores on bit 31:0 and HV_X64_MSR_TPR stores in bit 7:0, which means higher
32 bits are not really used, therefore potentially cast to void in order to silent this warning.

Reported-by: Hulk Robot <[email protected]>
Signed-off-by: Xu Yihang <[email protected]>
---
arch/x86/hyperv/hv_apic.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
index 284e73661a18..a8b639498033 100644
--- a/arch/x86/hyperv/hv_apic.c
+++ b/arch/x86/hyperv/hv_apic.c
@@ -60,9 +60,11 @@ static u32 hv_apic_read(u32 reg)
switch (reg) {
case APIC_EOI:
rdmsr(HV_X64_MSR_EOI, reg_val, hi);
+ (void) hi;
return reg_val;
case APIC_TASKPRI:
rdmsr(HV_X64_MSR_TPR, reg_val, hi);
+ (void) hi;
return reg_val;

default:
--
2.17.1

2021-03-23 11:34:24

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH -next] x86: Fix unused variable 'hi'

On Tue, Mar 23, 2021 at 10:50:13AM +0800, Xu Yihang wrote:
> Fixes the following W=1 kernel build warning(s):
> arch/x86/hyperv/hv_apic.c:58:15: warning: variable ‘hi’ set but not used [-Wunused-but-set-variable]
>
> Compiled with CONFIG_HYPERV enabled:
> make allmodconfig ARCH=x86_64 CROSS_COMPILE=x86_64-linux-gnu-
> make W=1 arch/x86/hyperv/hv_apic.o ARCH=x86_64 CROSS_COMPILE=x86_64-linux-gnu-
>
> HV_X64_MSR_EOI stores on bit 31:0 and HV_X64_MSR_TPR stores in bit 7:0, which means higher
> 32 bits are not really used, therefore potentially cast to void in order to silent this warning.
>
> Reported-by: Hulk Robot <[email protected]>
> Signed-off-by: Xu Yihang <[email protected]>
> ---
> arch/x86/hyperv/hv_apic.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
> index 284e73661a18..a8b639498033 100644
> --- a/arch/x86/hyperv/hv_apic.c
> +++ b/arch/x86/hyperv/hv_apic.c
> @@ -60,9 +60,11 @@ static u32 hv_apic_read(u32 reg)
> switch (reg) {
> case APIC_EOI:
> rdmsr(HV_X64_MSR_EOI, reg_val, hi);
> + (void) hi;
> return reg_val;
> case APIC_TASKPRI:
> rdmsr(HV_X64_MSR_TPR, reg_val, hi);
> + (void) hi;

I would like to remove the space while committing this patch. There is
no need for you to do anything.

Wei.

> return reg_val;
>
> default:
> --
> 2.17.1
>

2021-03-24 12:17:35

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH -next] x86: Fix unused variable 'hi'

On Tue, Mar 23, 2021 at 11:32:50AM +0000, Wei Liu wrote:
> On Tue, Mar 23, 2021 at 10:50:13AM +0800, Xu Yihang wrote:
> > Fixes the following W=1 kernel build warning(s):
> > arch/x86/hyperv/hv_apic.c:58:15: warning: variable ‘hi’ set but not used [-Wunused-but-set-variable]
> >
> > Compiled with CONFIG_HYPERV enabled:
> > make allmodconfig ARCH=x86_64 CROSS_COMPILE=x86_64-linux-gnu-
> > make W=1 arch/x86/hyperv/hv_apic.o ARCH=x86_64 CROSS_COMPILE=x86_64-linux-gnu-
> >
> > HV_X64_MSR_EOI stores on bit 31:0 and HV_X64_MSR_TPR stores in bit 7:0, which means higher
> > 32 bits are not really used, therefore potentially cast to void in order to silent this warning.
> >
> > Reported-by: Hulk Robot <[email protected]>
> > Signed-off-by: Xu Yihang <[email protected]>
> > ---
> > arch/x86/hyperv/hv_apic.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
> > index 284e73661a18..a8b639498033 100644
> > --- a/arch/x86/hyperv/hv_apic.c
> > +++ b/arch/x86/hyperv/hv_apic.c
> > @@ -60,9 +60,11 @@ static u32 hv_apic_read(u32 reg)
> > switch (reg) {
> > case APIC_EOI:
> > rdmsr(HV_X64_MSR_EOI, reg_val, hi);
> > + (void) hi;
> > return reg_val;
> > case APIC_TASKPRI:
> > rdmsr(HV_X64_MSR_TPR, reg_val, hi);
> > + (void) hi;
>
> I would like to remove the space while committing this patch. There is
> no need for you to do anything.

Applied to hyperv-next.