linux-kernel - RE: [PATCH v2] platform/mellanox: Add bootctl driver for Mellanox BlueField Soc

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: <DB6PR05MB32234336D53E88B2D81D1434A1910@DB6PR05MB3223.eurprd05.prod.outlook.com>
Date:   Thu, 31 Jan 2019 19:20:20 +0000
From:   Liming Sun <lsun@...lanox.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
CC:     Andy Shevchenko <andy@...radead.org>,
        Darren Hart <dvhart@...radead.org>,
        Vadim Pasternak <vadimp@...lanox.com>,
        David Woods <dwoods@...lanox.com>,
        Platform Driver <platform-driver-x86@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2] platform/mellanox: Add bootctl driver for Mellanox
 BlueField Soc

v3 has been posted with Changelog added.

Thanks!
Liming

> -----Original Message-----
> From: Liming Sun
> Sent: Thursday, January 31, 2019 12:18 PM
> To: 'Andy Shevchenko' <andy.shevchenko@...il.com>
> Cc: Andy Shevchenko <andy@...radead.org>; Darren Hart <dvhart@...radead.org>; Vadim Pasternak <vadimp@...lanox.com>; David
> Woods <dwoods@...lanox.com>; Platform Driver <platform-driver-x86@...r.kernel.org>; Linux Kernel Mailing List <linux-
> kernel@...r.kernel.org>
> Subject: RE: [PATCH v2] platform/mellanox: Add bootctl driver for Mellanox BlueField Soc
> 
> Please see my response inline. Will provide v3 once I solve the comments.
> 
> Thanks,
> Liming
> 
> > -----Original Message-----
> > From: Andy Shevchenko <andy.shevchenko@...il.com>
> > Sent: Thursday, January 31, 2019 12:02 PM
> > To: Liming Sun <lsun@...lanox.com>
> > Cc: Andy Shevchenko <andy@...radead.org>; Darren Hart <dvhart@...radead.org>; Vadim Pasternak <vadimp@...lanox.com>;
> David
> > Woods <dwoods@...lanox.com>; Platform Driver <platform-driver-x86@...r.kernel.org>; Linux Kernel Mailing List <linux-
> > kernel@...r.kernel.org>
> > Subject: Re: [PATCH v2] platform/mellanox: Add bootctl driver for Mellanox BlueField Soc
> >
> > On Thu, Jan 31, 2019 at 6:53 PM Liming Sun <lsun@...lanox.com> wrote:
> > >
> > > This commit adds the bootctl platform driver for Mellanox BlueField
> > > Soc, which controls the eMMC boot partition swapping and sends SMC
> > > calls to ATF running at exception level EL3 to program some system
> > > register. This register is only accessible in secure code and is
> > > used to enable the watchdog after reboot.
> > >
> > > Below are the sequences of a typical use case.
> > >
> > >   1. User-space tool upgrades one eMMC boot partition and requests
> > >      the boot partition swapping;
> > >
> > >   2. The bootctl driver handles such request and sends SMC call
> > >      to ATF. ATF programs register BREADCRUMB0 which has value
> > >      preserved during software reset. It also programs eMMC to
> > >      swap the boot partition;
> > >
> > >   3. After software reset (rebooting), ATF BL1 (BootRom) checks
> > >      register BREADCRUMB0 to enable watchdog if configured;
> > >
> > >   4. If booting fails, the watchdog timer will trigger rebooting.
> > >      In such case, ATF BootRom will switch the boot partition
> > >      to the previous one.
> >
> > Thanks for an update. My comments below.
> >
> >
> > > Reviewed-by: David Woods <dwoods@...lanox.com>
> >
> > I'm not sure I see this guy to review v2. Of course if you consider
> > all changes minor, you may leave this tag.
> 
> He sits besides me for internal review. I'll try to ask him to send
> comments to the mailing list.
> 
> >
> > > Signed-off-by: Liming Sun <lsun@...lanox.com>
> > > ---
> >
> > Here should be a changelog what had been done in new version.
> 
> Will provide changelog in the v3 email.
> Or please correct me if you meant adding changelog in the code or
> cover letter.
> 
> > > +/* UUID used to probe ATF service. */
> >
> > > +static const char * const mlxbf_bootctl_svc_uuid_str =
> > > +       "89c036b4-e7d7-11e6-8797-001aca00bfc4";
> >
> > static const char *xxx = ...;
> 
> Will update it in v3.
> 
> >
> > > +/*
> > > + * UUID structure used to match the returned value from ATF in
> > > + * four 32-bit words. No need to do endian conversion here.
> > > + */
> > > +union mlxbf_bootctl_uuid {
> > > +       guid_t guid;
> > > +       u32 words[4];
> > > +};
> >
> > I'm not sure it's the best you can do. instead of using union, better
> > to use conversion from and to corresponding uuid type.
> 
> I'll try to figure out another way without the union to compare the
> uuid directly using uuid api. Will update it in v3.
> 
> >
> > The rest I will comment on v3.
> >
> > --
> > With Best Regards,
> > Andy Shevchenko