linux-kernel - Re: [PATCH arm/aspeed/ast2500 v4 1/2] ipmi: add a KCS IPMI BMC driver

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <a84e8e19-dbd3-786e-b334-28b6202c0f1c@linux.intel.com>
Date:   Fri, 2 Feb 2018 09:33:49 +0800
From:   "Wang, Haiyue" <haiyue.wang@...ux.intel.com>
To:     minyard@....org, joel@....id.au, openbmc@...ts.ozlabs.org,
        openipmi-developer@...ts.sourceforge.net,
        linux-kernel@...r.kernel.org
Cc:     andriy.shevchenko@...el.com
Subject: Re: [PATCH arm/aspeed/ast2500 v4 1/2] ipmi: add a KCS IPMI BMC driver



On 2018-02-02 09:10, Corey Minyard wrote:
>
> I loaded this in, tried a compile on x86_64, and got the following:
>
> In file included from ../drivers/char/ipmi/kcs_bmc.c:15:0:
> ../drivers/char/ipmi/kcs_bmc.h: In function ‘kcs_bmc_priv’:
> ../drivers/char/ipmi/kcs_bmc.h:100:9: warning: return discards ‘const’ 
> qualifier from pointer target type [-Wdiscarded-qualifiers]
>   return kcs_bmc->priv;
>          ^
-static inline void *kcs_bmc_priv(const struct kcs_bmc *kcs_bmc)
+static inline void *kcs_bmc_priv(struct kcs_bmc *kcs_bmc)  <-- Can this 
fix error on x86_64 ?
{
     return kcs_bmc->priv;
}
> In file included from ../include/linux/printk.h:7:0,
>                  from ../include/linux/kernel.h:14,
>                  from ../include/asm-generic/bug.h:18,
>                  from ../arch/x86/include/asm/bug.h:82,
>                  from ../include/linux/bug.h:5,
>                  from ../include/linux/io.h:23,
>                  from ../drivers/char/ipmi/kcs_bmc.c:7:
> ../drivers/char/ipmi/kcs_bmc.c: In function ‘kcs_bmc_read’:
> ../include/linux/kern_levels.h:5:18: warning: format ‘%u’ expects 
> argument of type ‘unsigned int’, but argument 3 has type ‘size_t {aka 
> long unsigned int}’ [-Wformat=]
>  #define KERN_SOH "\001"  /* ASCII Start Of Header */
>                   ^
> ../include/linux/kern_levels.h:11:18: note: in expansion of macro 
> ‘KERN_SOH’
>  #define KERN_ERR KERN_SOH "3" /* error conditions */
>                   ^
> ../include/linux/printk.h:301:9: note: in expansion of macro ‘KERN_ERR’
>   printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
>          ^
> ../drivers/char/ipmi/kcs_bmc.c:307:3: note: in expansion of macro 
> ‘pr_err’
>    pr_err("channel=%u with too large data : %u\n",
>    ^
https://elinux.org/Debugging_by_printing
However please*note*: always use/%zu/,/%zd/or/%zx/for 
printing/size_t/and/ssize_t/values. ssize_t and size_t are quite common 
values in the kernel, so please use the/%z/to avoid annoying compile 
warnings.
So I change it from "%u" to "%zu", it is passed on my arm-32 compile, is 
it OK on your X64 ?
pr_err("channel=%u with too large data : %zu\n",
> In file included from ../drivers/char/ipmi/kcs_bmc_aspeed.c:20:0:
> ../drivers/char/ipmi/kcs_bmc.h: In function ‘kcs_bmc_priv’:
> ../drivers/char/ipmi/kcs_bmc.h:100:9: warning: return discards ‘const’ 
> qualifier from pointer target type [-Wdiscarded-qualifiers]
>   return kcs_bmc->priv;
>          ^
>
> So that needs to be fixed before it goes in.
>
> Also, since you are respinning, can you make ASPEED_KCS_IPMI_BMC 
> select IPMI_KCS_BMC, like:
>
> diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
> index bc2568a..d34f40e 100644
> --- a/drivers/char/ipmi/Kconfig
> +++ b/drivers/char/ipmi/Kconfig
> @@ -99,16 +99,11 @@ config IPMI_POWEROFF
>  endif # IPMI_HANDLER
>
>  config IPMI_KCS_BMC
> -       tristate 'IPMI KCS BMC Interface'
> -       help
> -         Provides a device driver for the KCS (Keyboard Controller 
> Style)
> -         IPMI interface which meets the requirement of the BMC 
> (Baseboard
> -         Management Controllers) side for handling the IPMI request from
> -         host system software.
> +       tristate
>
>  config ASPEED_KCS_IPMI_BMC
>         depends on ARCH_ASPEED || COMPILE_TEST
> -       depends on IPMI_KCS_BMC
> +       select IPMI_KCS_BMC
>         select REGMAP_MMIO
>         tristate "Aspeed KCS IPMI BMC driver"
>         help
>
> It doesn't make much sense to have IPMI_KCS_BMC on its own.  I was 
> going to do this till I saw the compiler error.
>
Got it, will change it to 'select'
> -corey