linux-kernel - RE: [char-misc-next 2/2] mei: send OS type to the FW

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: <5B8DA87D05A7694D9FA63FD143655C1B54319CEB@hasmsx108.ger.corp.intel.com>
Date:   Thu, 10 Nov 2016 13:00:54 +0000
From:   "Winkler, Tomas" <tomas.winkler@...el.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
CC:     "Usyskin, Alexander" <alexander.usyskin@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Jarkko Sakkinen" <jarkko.sakkinen@...ux.intel.com>
Subject: RE: [char-misc-next 2/2] mei: send OS type to the FW

> 
> On Thu, Nov 10, 2016 at 12:19:06PM +0000, Winkler, Tomas wrote:
> > >
> > > On Thu, Nov 10, 2016 at 12:00:29PM +0000, Winkler, Tomas wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Greg Kroah-Hartman [mailto:gregkh@...uxfoundation.org]
> > > > > Sent: Thursday, November 10, 2016 09:23
> > > > > To: Winkler, Tomas <tomas.winkler@...el.com>
> > > > > Cc: Usyskin, Alexander <alexander.usyskin@...el.com>; linux-
> > > > > kernel@...r.kernel.org; Jarkko Sakkinen
> > > > > <jarkko.sakkinen@...ux.intel.com>
> > > > > Subject: Re: [char-misc-next 2/2] mei: send OS type to the FW
> > > > >
> > > > > On Tue, Nov 08, 2016 at 06:26:09PM +0200, Tomas Winkler wrote:
> > > > > > From: Alexander Usyskin <alexander.usyskin@...el.com>
> > > > > >
> > > > > > Tell the FW that we are running a sane OS and TPM2_ChangeEPS()
> > > > > > is supported. This workaround was added to support other
> > > > > > broken OS and we need to follow here. The command is sent just
> > > > > > once at the boot
> > > time.
> > > > > >
> > > > > > Cc: Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
> > > > > > Signed-off-by: Tomas Winkler <tomas.winkler@...el.com>
> > > > > > Signed-off-by: Alexander Usyskin <alexander.usyskin@...el.com>
> > > > > > ---
> > > > > >  drivers/misc/mei/bus-fixup.c | 98
> > > > > > ++++++++++++++++++++++++++++++++++++++++++++
> > > > > >  1 file changed, 98 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/misc/mei/bus-fixup.c
> > > > > > b/drivers/misc/mei/bus-fixup.c index
> > > > > > 9e10d86e3887..344a0c99ee44
> > > > > > 100644
> > > > > > --- a/drivers/misc/mei/bus-fixup.c
> > > > > > +++ b/drivers/misc/mei/bus-fixup.c
> > > > > > @@ -38,6 +38,9 @@ static const uuid_le mei_nfc_info_guid =
> > > > > > MEI_UUID_NFC_INFO;  #define MEI_UUID_WD UUID_LE(0x05B79A6F,
> > > > > 0x4628, 0x4D7F, \
> > > > > >  			    0x89, 0x9D, 0xA9, 0x15, 0x14, 0xCB, 0x32,
> 0xAB)
> > > > > >
> > > > > > +#define MEI_UUID_MKHIF_FIX UUID_LE(0x55213584, 0x9a29,
> 0x4916, \
> > > > > > +			0xba, 0xdf, 0xf, 0xb7, 0xed, 0x68, 0x2a, 0xeb)
> > > > > > +
> > > > > >  #define MEI_UUID_ANY NULL_UUID_LE
> > > > > >
> > > > > >  /**
> > > > > > @@ -69,6 +72,100 @@ static void blacklist(struct mei_cl_device
> *cldev)
> > > > > >  	cldev->do_match = 0;
> > > > > >  }
> > > > > >
> > > > > > +#define OSTYPE_LINUX    2
> > > > > > +struct mei_os_ver {
> > > > > > +	u16 build;
> > > > > > +	u16 reserved1;
> > > > >
> > > > > Don't you need to specify the endian-type of these (well for
> > > > > build), as they get written to hardware?
> > > >
> > > > This is really x86 stuff only (depends on X86), the device lives
> > > > in PCH or SoC,  you cannot really plug it into PowerPC.  So we are
> > > > consciously not using endian-types in the HW interface.
> > >
> > > Then just mark them all as little endian and be done with it :)
> >
> > Then I will have to run the leX_to_cpu all over the code, which will
> > really do nothing.
> 
> It's not "nothing", it is explicitly stating what the hardware expects.

In this specific case this serves as annotation, so I suggest to solves this by adding documentation.  And anyhow fixing just this patch will just create more confusion. 

> You write code in C for developers, the compiler will make it all go away, but
> then the developer knows what is going on here, and it allows them to
> understand the logic a lot better.

I really wound 't care (and if you look back I was the one who put endian awareness  to iwlwifi) but I have some vague memory that we agreed on that when we matured the driver into 3.5 then this is end to end x86 only code and we won't annotate the device interface.

Thanks
Tomas