linux-kernel - Re: [PATCH v3 03/17] firmware: arm_ffa: Implement the notification bind and unbind interface

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: <CAHUa44ETFTKE17tUMsAiF5vYk2yT9wQT-zE+22ic3jHrHhrrJg@mail.gmail.com>
Date:   Thu, 5 Oct 2023 11:56:22 +0200
From:   Jens Wiklander <jens.wiklander@...aro.org>
To:     Sudeep Holla <sudeep.holla@....com>
Cc:     Olivier Deprez <Olivier.Deprez@....com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Marc Bonnici <Marc.Bonnici@....com>,
        Coboy Chen <coboy.chen@...iatek.com>,
        Lorenzo Pieralisi <lpieralisi@...nel.org>
Subject: Re: [PATCH v3 03/17] firmware: arm_ffa: Implement the notification
 bind and unbind interface

On Thu, Oct 5, 2023 at 10:51 AM Sudeep Holla <sudeep.holla@....com> wrote:
>
> On Thu, Oct 05, 2023 at 08:57:26AM +0200, Jens Wiklander wrote:
> > Hi Sudeep,
> >
> > On Wed, Oct 4, 2023 at 5:32 PM Sudeep Holla <sudeep.holla@....com> wrote:
> > >
> > > On Wed, Oct 04, 2023 at 10:50:26AM +0100, Olivier Deprez wrote:
> > > > Hi Jens,
> > > >
> > > > > dst_id and drv_info->vm_id should be swapped.
> > > >
> > > > I'm curious about this because swapping like this actually makes hafnium
> > > > fail. Need to check from the spec.
> > >
> > > I did check after I had swapped this in v2(because I was convinced Jens) was
> > > correct and you reported the failure. Reading the spec again the other day,
> > > I got corrected myself and agreed with Olivier and my original
> > > implementation(v1) which matches this patch(v3).
>
> Well, I am not exactly sure what is the root cause for the confusion here:
> My poor choice of variable names and their usage with this macro, or the
> macro definition itself(I am not sure)
>
> OR
>
> The wordings in the specification
>
> >
> > I don't get it. The spec says for FFA_NOTIFICATION_BIND:
> > Sender and Receiver endpoint IDs.
> > – Bit[31:16]: Sender endpoint ID.
> > – Bit[15:0]: Receiver endpoint ID.
> > This is exactly the same as for instance FFA_MSG_SEND_DIRECT_REQ.
> >
>
> Not really as per my understanding of the specification.
>
> > In ffa_msg_send_direct_req() you assign
> > src_dst_ids = PACK_TARGET_INFO(src_id, dst_id);
> >
>
> Correct and if you look at the callsite, it is
>         ffa_msg_send_direct_req(drv_info->vm_id, dev->vm_id,...)
>
> So the driver is the sender and the partition is the receiver. Probably
> this is simpler.
>
> > but here in ffa_notification_bind_common() you assign
> > src_dst_ids = PACK_TARGET_INFO(dst_id, drv_info->vm_id);
> >
>
> A receiver(FF-A driver) must bind a non-framework notification to a
> sender(SP) before the latter can signal the notification to the former.
> Only the sender can ring these doorbells. A receiver uses the
> FFA_NOTIFICATION_BIND interface to bind one or more notifications to the
> sender.
>
> So, based on this text(modified to refer sender and receiver in the driver
> context) from the spec, my understanding is the driver is the receiver
> and the SP is the sender of the notification.
>
> Do you think I am missing someting here ? Sorry for agreeing with you
> in v2 and silently changing it back without this actual discussion.
> Olivier raised the issue and then when I went back and looked at the
> spec, I realised why I had it this way from the beginning.

Thanks for the explanation, now I get it. My mistake was that I
thought that sender and receiver meant the sender and receiver of the
actual message being sent like with a direct request, it is using the
same register and the same wording after all. Instead, it means the
sender and receiver of an eventual notification, which of course is
the exact opposite.

Thanks,
Jens