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]

Message-ID: <CAFHUOYyzQiUtpDL_oULCdbvMXxoXK5_bAvkW8SZV0BcrvPqa-Q@mail.gmail.com>
Date:	Wed, 8 Jun 2016 17:41:20 -0700
From:	Hoan Tran <hotran@....com>
To:	"Prakash, Prashanth" <pprakash@...eaurora.org>
Cc:	Ashwin Chaugule <ashwin.chaugule@...aro.org>,
	Jassi Brar <jassisinghbrar@...il.com>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	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>
Subject: Re: [PATCH v3] mailbox: pcc: Support HW-Reduced Communication
 Subspace type 2

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.

>
> Hoan,
> Even if we are not using platform notification, we still need to clear the doorbell
> interrupt bit in the PCC interrupt handler (Section14.2.2 and 14.4).  I didn't see
> clearing the doorbell interrupt bit in this patch (and platform is supposed to set
> it again when it is sending the interrupt again). Did I miss it? or is it intentionally
> left out to avoid the race that Ashwin mentioned above?
>

The PCC client driver is supposed to do that. Which mean, the
mbox_chan_received_data() function should clear it.

Thanks
Hoan

>
> Thanks,
> Prashanth
>
>