linux-kernel - Re: [PATCH v2 2/3] mailbox: Add support for Hi3660 mailbox

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, 30 Oct 2017 10:19:40 +0000
From:   Mark Rutland <mark.rutland@....com>
To:     Leo Yan <leo.yan@...aro.org>
Cc:     Kaihua Zhong <zhongkaihua@...wei.com>, robh+dt@...nel.org,
        xuwei5@...ilicon.com, catalin.marinas@....com, will.deacon@....com,
        jassisinghbrar@...il.com, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        guodong.xu@...aro.org, haojian.zhuang@...aro.org,
        suzhuangluan@...ilicon.com, xuezhiliang@...ilicon.com,
        kevin.wangtao@...ilicon.com
Subject: Re: [PATCH v2 2/3] mailbox: Add support for Hi3660 mailbox

Hi,

On Mon, Oct 30, 2017 at 12:45:06PM +0800, Leo Yan wrote:
> On Fri, Oct 27, 2017 at 11:46:00AM +0100, Mark Rutland wrote:
> > On Fri, Oct 27, 2017 at 02:15:03PM +0800, Kaihua Zhong wrote:
> > > +static int hi3660_mbox_check_state(struct mbox_chan *chan)
> > > +{

> > > +	/* Ensure channel is released */
> > > +	writel_relaxed(0xffffffff, base + MBOX_IMASK_REG);
> > > +	writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SRC_REG);
> > > +	__asm__ volatile ("sev");
> > > +	return 0;
> > > +}
> > 
> > Drivers really shouldn't be using SEV directly (even if via the
> > sev() macro)...
> > 
> > This SEV isn't ordered w.r.t. anything, and it's unclear what
> > ordering you need, so this simply does not work.
> 
> I will leave your questions for Hisilicon colleagues, essentially your
> questions are related with mailbox mechanism.
> 
> But I'd like to firstly get clear your question for "This SEV isn't
> ordered w.r.t. anything". From my understanding, ARMv8 architecture
> natually adds DMB before SEV so all previous register writing
> opreations should be ensured to endpoint before SEV?

This is not the case; SEV does not add any implicit memory barrier, and
is not ordered w.r.t. memory accesses.

See ARM DDI 0487B.b, page D1-1905, "The Send Event instructions":

    The PE is not required to guarantee the ordering of this event with
    respect to the completion of memory accesses by instructions before
    the SEV instruction. Therefore, ARM recommends that software
    includes a DSB instruction before any SEV instruction.

Note that a DMB is not sufficient, as SEV is not a memory access.

Thanks,
Mark.