svn commit: r357349 - in head/sys: conf modules/tpm

[Ian Lepore]

On Fri, 2020-01-31 at 23:36 +0100, Dimitry Andric wrote:
> Hmm yes, you are quite right.  Other parts of the code also seem to
> use ~TPM_XXX, and the WR4() inline function called takes a
> uint32_t.  I'll revert my change and apply the tilde version instead!
> 
> -Dimitry
> 

So you're going to switch from writing 0 to writing 0xfffffffe, and
just assume that will work the same?  Like, without looking at the
datasheet or TRM for the device?  Surely those other 31 bits you're
turning on in a control register can't do anything important, can they?

I haven't looked at the code, but I'll bet the other places that are
using ~SYMBOLNAME are doing so in the context of read-modify-write in a
way that preserves existing bits, which is completely different than
just turning on 31 bits as a side effect of turning one bit off.

-- Ian



> > On 31 Jan 2020, at 22:13, Conrad Meyer <cem at FreeBSD.org> wrote:
> > 
> > Hi Dimitry,
> > 
> > Do you think maybe the intent is to use ~TPM_CRB_CTRL_CANCEL_CMD
> > instead?  Plain "0" might also make sense.  But I think the
> > compiler
> > is right here and the warning should not be disabled — !BIT(foo)
> > doesn't really make sense for a register.  It happens to affect the
> > right bit only because CANCEL_CMD is BIT(0).
> > 
> > Thanks,
> > Conrad
> > 
> > On Fri, Jan 31, 2020 at 11:36 AM Dimitry Andric <dim at freebsd.org>
> > wrote:
> > > 
> > > Author: dim
> > > Date: Fri Jan 31 19:36:14 2020
> > > New Revision: 357349
> > > URL: https://svnweb.freebsd.org/changeset/base/357349
> > > 
> > > Log:
> > >  Merge r357348 from the clang 10.0.0 import branch:
> > > 
> > >  Disable new clang 10.0.0 warnings about converting the result of
> > > shift
> > >  operations to a boolean in tpm(4):
> > > 
> > >  sys/dev/tpm/tpm_crb.c:301:32: error: converting the result of
> > > '<<' to a boolean; did you mean '(1 << (0)) != 0'? [-Werror,-
> > > Wint-in-bool-context]
> > >          WR4(sc, TPM_CRB_CTRL_CANCEL, !TPM_CRB_CTRL_CANCEL_CMD);
> > >                                        ^
> > >  sys/dev/tpm/tpm_crb.c:73:34: note: expanded from macro
> > > 'TPM_CRB_CTRL_CANCEL_CMD'
> > >  #define TPM_CRB_CTRL_CANCEL_CMD         BIT(0)
> > >                                          ^
> > >  sys/dev/tpm/tpm20.h:60:19: note: expanded from macro 'BIT'
> > >  #define BIT(x) (1 << (x))
> > >                    ^
> > > 
> > >  Such warnings can be useful in C++ contexts, but not so much in
> > > kernel
> > >  drivers, where this type of bit twiddling is commonplace.  So
> > > disable it
> > >  for this case.
> > > 
> > >  MFC after:    3 days
> > > 
> > > Modified:
> > >  head/sys/conf/files.amd64
> > >  head/sys/conf/kern.mk
> > >  head/sys/modules/tpm/Makefile
> > > Directory Properties:
> > >  head/   (props changed)
> > > 
> > > Modified: head/sys/conf/files.amd64
> > > =================================================================
> > > =============
> > > --- head/sys/conf/files.amd64   Fri Jan 31 19:35:21
> > > 2020        (r357348)
> > > +++ head/sys/conf/files.amd64   Fri Jan 31 19:36:14
> > > 2020        (r357349)
> > > @@ -323,7 +323,8 @@
> > > dev/syscons/scvesactl.c             optional        sc vga vesa
> > > dev/syscons/scvgarndr.c                optional        sc vga
> > > dev/tpm/tpm.c                  optional        tpm
> > > dev/tpm/tpm20.c                optional        tpm
> > > -dev/tpm/tpm_crb.c              optional        tpm acpi
> > > +dev/tpm/tpm_crb.c              optional        tpm acpi \
> > > +       compile-with "${NORMAL_C} ${NO_WINT_IN_BOOL_CONTEXT}"
> > > dev/tpm/tpm_tis.c              optional        tpm acpi
> > > dev/tpm/tpm_acpi.c             optional        tpm acpi
> > > dev/tpm/tpm_isa.c              optional        tpm isa
> > > 
> > > Modified: head/sys/conf/kern.mk
> > > =================================================================
> > > =============
> > > --- head/sys/conf/kern.mk       Fri Jan 31 19:35:21
> > > 2020        (r357348)
> > > +++ head/sys/conf/kern.mk       Fri Jan 31 19:36:14
> > > 2020        (r357349)
> > > @@ -37,6 +37,9 @@ CWARNEXTRA+=  -Wno-error-shift-negative-value
> > > .if ${COMPILER_VERSION} >= 40000
> > > CWARNEXTRA+=   -Wno-address-of-packed-member
> > > .endif
> > > +.if ${COMPILER_VERSION} >= 100000
> > > +NO_WINT_IN_BOOL_CONTEXT=       -Wno-int-in-bool-context
> > > +.endif
> > > .endif
> > > 
> > > .if ${COMPILER_TYPE} == "gcc"
> > > 
> > > Modified: head/sys/modules/tpm/Makefile
> > > =================================================================
> > > =============
> > > --- head/sys/modules/tpm/Makefile       Fri Jan 31 19:35:21
> > > 2020        (r357348)
> > > +++ head/sys/modules/tpm/Makefile       Fri Jan 31 19:36:14
> > > 2020        (r357349)
> > > @@ -11,3 +11,5 @@ SRCS+=        tpm_isa.c tpm_acpi.c isa_if.h
> > > opt_acpi.h acpi_i
> > > SRCS+= tpm20.c tpm_crb.c tpm_tis.c opt_tpm.h
> > > 
> > > .include <bsd.kmod.mk>
> > > +
> > > +CWARNFLAGS.tpm_crb.c+= ${NO_WINT_IN_BOOL_CONTEXT}
> 
>