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

[Wei Liu]

On Tue, Mar 23, 2021 at 11:32:50AM +, 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 
> > Signed-off-by: Xu Yihang 
> > ---
> >  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.

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

[Wei Liu]

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 
> Signed-off-by: Xu Yihang 
> ---
>  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
> 

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

[Wei Liu]

On Mon, Mar 22, 2021 at 11:54:26AM +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 __maybe_unused added.
> 
> Reported-by: Hulk Robot 
> Signed-off-by: Xu Yihang 

I slightly modified the commit message and queued it up for hyperv-next.
Thanks.

Wei.

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

[Michael Kelley]

From: Ingo Molnar   Sent: Thursday, March 18, 2021 
4:24 AM
> 
> * 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]
> >
> > Reported-by: Hulk Robot 
> > Signed-off-by: Xu Yihang 
> > ---
> >  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

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

[Ingo Molnar]

* 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]
> 
> Reported-by: Hulk Robot 
> Signed-off-by: Xu Yihang 
> ---
>  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