linux-kernel - Re: [PATCH v3 2/2] i2c: designware: Add AMD PSP I2C bus support

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]

Date:   Mon, 7 Feb 2022 13:41:12 +0200
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Jan Dąbroś <jsd@...ihalf.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-i2c <linux-i2c@...r.kernel.org>,
        Jarkko Nikula <jarkko.nikula@...ux.intel.com>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Hans de Goede <hdegoede@...hat.com>,
        Wolfram Sang <wsa@...nel.org>,
        Raul E Rangel <rrangel@...omium.org>,
        Marcin Wojtas <mw@...ihalf.com>,
        Grzegorz Jaszczyk <jaz@...ihalf.com>, upstream@...ihalf.com,
        Tom Lendacky <thomas.lendacky@....com>,
        "Deucher, Alexander" <alexander.deucher@....com>,
        "Easow, Nimesh" <Nimesh.Easow@....com>,
        "Limonciello, Mario" <mario.limonciello@....com>
Subject: Re: [PATCH v3 2/2] i2c: designware: Add AMD PSP I2C bus support

On Mon, Feb 07, 2022 at 09:27:12AM +0100, Jan Dąbroś wrote:
> śr., 2 lut 2022 o 17:16 Andy Shevchenko
> <andriy.shevchenko@...ux.intel.com> napisał(a):
> > On Wed, Feb 02, 2022 at 03:43:02PM +0100, Jan Dabros wrote:

...

> > > +struct psp_i2c_req {
> > > +     struct psp_req_buffer_hdr hdr;
> > > +     enum psp_i2c_req_type type;
> > > +} __aligned(32);
> >
> > I forgot if this alignment is requirement due to DMA or so?
> > We may use ____cacheline_aligned in such case.
> 
> I used some old code as a reference - this structure is mapped by PSP,
> thus alignment applied here. I'm checking this with AMD whether it is
> really needed or at least can be somehow limited - still waiting for
> their response.
> I propose to left this as is right now and eventually remove or make
> more liberal in the future.

Would be nice to clarify sooner than later.
In either case it needs a good comment.

...

> > > +     return readl_poll_timeout(&mbox->cmd_fields, tmp, (tmp == expected),
> > > +                               0, 1000 * PSP_CMD_TIMEOUT_MS);
> >
> > 0?!
> 
> Yes, we are checking for readiness of PSP mailbox in a tight loop. We
> would like to proceed further quickly as soon as this bit is set.
> Actually checking this twice per every ACQUIRE&RELEASE - once before
> sending command (to check whether PSP is ready to accept requests) and
> second time after sending it. Do you think we should increase
> @sleep_us value?

It depends on what you have in mind about hardware. I'm fine with either way,
but 0 has to be explained (in the comment).

-- 
With Best Regards,
Andy Shevchenko