linux-kernel - Re: [PATCH v3] mailbox: pcc: Support HW-Reduced Communication Subspace type 2

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, 15 Aug 2016 09:45:24 -0700
From:	Hoan Tran <hotran@....com>
To:	"Rafael J. Wysocki" <rjw@...ysocki.net>
Cc:	Jassi Brar <jassisinghbrar@...il.com>,
	Ashwin Chaugule <ashwin.chaugule@...aro.org>,
	Robert Moore <robert.moore@...el.com>,
	Alexey Klimov <alexey.klimov@....com>,
	lkml <linux-kernel@...r.kernel.org>,
	linux acpi <linux-acpi@...r.kernel.org>, Loc Ho <lho@....com>,
	Duc Dang <dhdang@....com>,
	"Prakash, Prashanth" <pprakash@...eaurora.org>
Subject: Re: [PATCH v3] mailbox: pcc: Support HW-Reduced Communication
 Subspace type 2

On Mon, Jun 27, 2016 at 2:32 PM, Rafael J. Wysocki <rjw@...ysocki.net> wrote:
>
> On Monday, June 27, 2016 11:27:42 AM Hoan Tran wrote:
> > Hi Jassi and Rafael,
> >
> > On Wed, Jun 15, 2016 at 9:19 AM, Prakash, Prashanth
> > <pprakash@...eaurora.org> wrote:
> > >
> > >
> > > On 6/9/2016 4:43 PM, Hoan Tran wrote:
> > >> Hi Prashanth,
> > >>
> > >> On Thu, Jun 9, 2016 at 3:25 PM, Prakash, Prashanth
> > >> <pprakash@...eaurora.org> wrote:
> > >>>
> > >>> On 6/9/2016 2:47 PM, Hoan Tran wrote:
> > >>>> Hi Ashwin and Prashanth,
> > >>>>
> > >>>> On Wed, Jun 8, 2016 at 5:41 PM, Hoan Tran <hotran@....com> wrote:
> > >>>>> Hi Prashanth,
> > >>>>>
> > >>>>>
> > >>>>> On Wed, Jun 8, 2016 at 5:32 PM, Prakash, Prashanth
> > >>>>> <pprakash@...eaurora.org> wrote:
> > >>>>>> On 6/8/2016 10:24 AM, Hoan Tran wrote:
> > >>>>>>> Hi Ashwin,
> > >>>>>>>
> > >>>>>>> On Wed, Jun 8, 2016 at 5:18 AM, Ashwin Chaugule
> > >>>>>>> <ashwin.chaugule@...aro.org> wrote:
> > >>>>>>>> + Prashanth (Can you please have a look as well?)
> > >>>>>>>>
> > >>>>>>>> On 31 May 2016 at 15:35, Hoan Tran <hotran@....com> wrote:
> > >>>>>>>>> Hi Ashwin,
> > >>>>>>>> Hi,
> > >>>>>>>>
> > >>>>>>>> Sorry about the delay. I'm in the middle of switching jobs and
> > >>>>>>>> locations, so its been a bit crazy lately.
> > >>>>>>> It's ok and hope you're doing well.
> > >>>>>>>
> > >>>>>>>> I dont have any major
> > >>>>>>>> concerns with this code, although there could be subtle issues with
> > >>>>>>>> this IRQ thing. In this patchset, your intent is to add support for
> > >>>>>>>> PCC subspace type 2. But you're also adding support for tx command
> > >>>>>>>> completion which is not specific to Type 2. We could support that even
> > >>>>>>>> in Type 1. Hence I wanted to separate the two, not just for review,
> > >>>>>>>> but also the async IRQ completion has subtle issues esp. in the case
> > >>>>>>>> of async platform notification, where you could have a PCC client in
> > >>>>>>>> the OS writing to the cmd bit and the platform sending an async
> > >>>>>>>> notification by writing to some bits in the same 8byte address as the
> > >>>>>>>> cmd bit. So we need some mutual exclusivity there, otherwise the OS
> > >>>>>>>> and platform could step on each other. Perhaps Prashanth has better
> > >>>>>>>> insight into this.
> > >>>>>>> I think, this mutual exclusivity could be in another patch.
> > >>>>>> Ashwin,
> > >>>>>> Sorry, I am not sure how we can prevent platform and OSPM from stepping on
> > >>>>>> each other.  There is a line is spec that says "all operations on status field
> > >>>>>> must be made using interlocked operations", but not sure what these
> > >>>>>> interlocked operation translates to.
> > >>>>> Yes, I had the same question about how to prevent it.
> > >>>> For platform notification, if the hardware doesn't support interlocked
> > >>>> operations. I think we can use a workaround that, platform triggers
> > >>>> interrupt to OSPM without touching status field. The OSPM PCC client
> > >>>> will decide what to do with this interrupt. For example, OSPM sends a
> > >>>> consumer command to check it.
> > >>> How do we decide which platform can support this interlocked operation?
> > >>> and how do we decide between a completion notification and platform
> > >>> notification?
> > >> Truly, we should follow the specification. But I don't know if there's
> > >> any hardware support this interlocked operation.
> > >> For the decide between a completion notification and platform notification
> > >>  - Completion notification: Bit "Command Complete" is set.
> > >>  - Platform notification: Bit "Command Complete" is not set.
> > >>
> > >>> I think the ACPI spec on platform notification is quite ambiguous and it is
> > >>> best to get the necessary clarification and/or correction before implementing
> > >>> anything related to platform notification.
> > >> Agreed, a clarification inside ACPI Specification is needed
> > > This patch look good to me, as it doesn't deal with platform notification.
> > > We can try to get some clarification from spec side before handling the platform
> > > notification pieces.
> > >
> > > Reviewed-by: Prashanth Prakash <pprakash@...eaurora.org>
> >
> > Do you have plan to apply this patch ?
>
> Yes.
>
> Thanks,
> Rafael
>

Hi Rafael,

This patch had an ACK from Prashanth.  Can you consider to merge
this patch please?

Thanks
Hoan