Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221019204626.3813043-1-cristian.marussi@arm.com>
 <CAKfTPtBJy4SdbYUNHFn2ZXEO_pnaMPYibfjXWNBnXy49P2h78Q@mail.gmail.com> <Y1vvPBw4xB7m23wY@e120937-lin>
In-Reply-To: <Y1vvPBw4xB7m23wY@e120937-lin>
From:   Vincent Guittot <vincent.guittot@linaro.org>
Date:   Fri, 28 Oct 2022 18:18:52 +0200
Message-ID: <CAKfTPtAfuqtCee7f4bREsLqb5KJcLWw1Y=-0Y+2t+3XfX_YctQ@mail.gmail.com>
Subject: Re: [PATCH v4 0/11] Introduce a unified API for SCMI Server testing
To:     Cristian Marussi <cristian.marussi@arm.com>
Cc:     linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        sudeep.holla@arm.com, james.quinlan@broadcom.com,
        Jonathan.Cameron@huawei.com, f.fainelli@gmail.com,
        etienne.carriere@linaro.org, souvik.chakravarty@arm.com,
        wleavitt@marvell.com, peter.hilber@opensynergy.com,
        nicola.mazzucato@arm.com, tarek.el-sherbiny@arm.com,
        quic_kshivnan@quicinc.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, 28 Oct 2022 at 17:04, Cristian Marussi <cristian.marussi@arm.com> wrote:
>
> On Fri, Oct 28, 2022 at 04:40:02PM +0200, Vincent Guittot wrote:
> > On Wed, 19 Oct 2022 at 22:46, Cristian Marussi <cristian.marussi@arm.com> wrote:
> > >
> > > Hi all,
> > >
>
> Hi Vincent,
>
> > > This series aims to introduce a new SCMI unified userspace interface meant
> > > to ease testing an SCMI Server implementation for compliance, fuzzing etc.,
> > > from the perspective of the OSPM agent (non-secure world only ...)
> > >

[ snip]

> > Hi Cristian,
> >
> > I have tested your series with an optee message transport layer and
> > been able to send raw messages to the scmi server PTA
> >
> > FWIW
> >
> > Tested-by: Vincent Guittot <vincent.guittot@linaro.org>
> >
>
> Thanks a lot for your test and feedback !
>
> ... but I was going to reply to this saying that I spotted another issue
> with the current SCMI Raw implementation (NOT related to optee/smc) so
> that I'll post a V5 next week :P
>
> Anyway, the issue is much related to the debugfs root used by SCMI Raw,
> i.e.:
>
>         /sys/kernel/debug/scmi_raw/
>
> ..this works fine unless you run it on a system sporting multiple DT-defined
> server instances ...that is not officially supported but....ehm...a little
> bird told these system with multiple servers do exists :D

;-)

>
> In such a case the SCMI core stack is probed multiuple times and so it
> will try to register multiple debugfs Raw roots: there is no chanche to
> root the SCMI Raw entries at the same point clearly ... and then anyway
> there is the issue of recognizing which server is rooted where ... with
> the additional pain that a server CANNOT be recognized by querying...cause
> there is only one by teh spec with agentID ZERO ... in theory :D...
>
> So my tentaive solution for V5 would be:
>
> - change the Raw root debugfs as:
>
>         /sys/kernel/debug/scmi_raw/0/... (first server defined)
>
>         /sys/kernel/debug/scmi_raw/1/... (possible additional server(s)..)
>
> - expose the DT scmi-server root-node name of the server somewhere under
>   that debugfs root like:
>
>         ..../scmi_raw/0/transport_name -> 'scmi-mbx'
>
>         ..../scmi_raw/1/transport_name -> 'scmi-virtio'

I was about to say that you would display the server name but that
means that you have send a request to the server which probably
defeats the purpose of the raw mode

>
>   so that if you know HOW you have configured your own system in the DT
>   (maybe multiple servers with different kind of transports ?), you can
>   easily select programmatically which one is which, and so decide
>   which Raw debugfs fs to use...
>
> ... I plan to leave the SCMI ACS suite use by default the first system
> rooted at /sys/kernel/debug/scmi_raw/0/...maybe adding a commandline
> option to choose an alternative path for SCMI Raw.
>
> Does all of this sound reasonable ?

Yes, adding an index looks good to me.

As we are there, should we consider adding a per channel entry in the
tree when there are several channels shared with the same server ?

Vincent

>
> Thanks,
> Cristian
>