Re: [RFC v1 3/5] xen/arm: introduce SCMI-SMC mediator driver

[Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>]

Hi Stefano, Oleksii,

Stefano Stabellini <sstabellini@xxxxxxxxxx> writes:

> On Wed, 22 Dec 2021, Oleksii Moisieiev wrote:
>> On Tue, Dec 21, 2021 at 01:22:50PM -0800, Stefano Stabellini wrote:
>> > On Tue, 21 Dec 2021, Oleksii Moisieiev wrote:
>> > > Hi Stefano,
>> > > 
>> > > On Mon, Dec 20, 2021 at 04:52:01PM -0800, Stefano Stabellini wrote:
>> > > > On Mon, 20 Dec 2021, Oleksii Moisieiev wrote:
>> > > > > Hi Stefano,
>> > > > > 
>> > > > > On Fri, Dec 17, 2021 at 06:14:55PM -0800, Stefano Stabellini wrote:
>> > > > > > On Tue, 14 Dec 2021, Oleksii Moisieiev wrote:

[...]

>> > > > In general we can't use properties that are not part of the device tree
>> > > > spec, either
>> > > > https://urldefense.com/v3/__https://www.devicetree.org/specifications/__;!!GF_29dbcQIUBPA!kNodtgmOQBc1iO76_6vTK-O1SoLxee_ChowYQiQYC595rMOsrnmof2zmk7BnhXCSnJPN$
>> > > > [devicetree[.]org] or
>> > > > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings__;!!GF_29dbcQIUBPA!kNodtgmOQBc1iO76_6vTK-O1SoLxee_ChowYQiQYC595rMOsrnmof2zmk7BnhXloYUaj$
>> > > > [git[.]kernel[.]org]
>> > > > 
>> > > > "linux,scmi_mem" is currently absent. Are you aware of any upstreaming
>> > > > activities to get "linux,scmi_mem" upstream under
>> > > > Documentation/devicetree/bindings in Linux?
>> > > > 
>> > > > If "linux,scmi_mem" is going upstream in Linux, then we could use it.
>> > > > Otherwise, first "linux,scmi_mem" needs to be added somewhere under
>> > > > Documentation/devicetree/bindings (probably
>> > > > Documentation/devicetree/bindings/firmware/arm,scmi.yaml), then we can
>> > > > work on the Xen code that makes use of it.
>> > > > 
>> > > > Does it make sense?
>> > > > 
>> > > 
>> > > Yes I agree. I think linux,scmi_mem and scmi_devid should be upstreamed.
>> > > I will add those properties to arm,scmi.yaml, mark them as related to 
>> > > XEN and send patch.
>> > 
>> > I didn't realize that linux,scmi_mem and scmi_devid are supposed to be
>> > Xen specific. In general, it would be best not to introduce Xen specific
>> > properties into generic bindings. It is a problem both from a
>> > specification perspective (because it has hard to handle Xen specific
>> > cases in fully generic bindings, especially as those bindings are
>> > maintained as part of the Linux kernel) and from a user perspective
>> > (because now the user has to deal with a Xen-specific dtb, or has to
>> > modify the host dtb to add Xen-specific information by hand.)
>> > 
>> > 
>> > Let me start from scmi_devid.  Why would scmi_devid be Xen-specific? It
>> > looks like a generic property that should be needed for the Linux SCMI
>> > driver too. Why the Linux driver doesn't need it?
>> > 
>> 
>> scmi_devid used during domain build. It passed as input parameter for 
>> SCMI_BASE_SET_DEVICE_PERMISSIONS message.
>> On non-virtualized systems - there is no need of this call, because OS
>> is the only one entity, running on the system.
>
> OK. Even if it is only required for virtualized systems, I think that
> scmi_devid is important enough that should be part of the upstream
> binding. I think it is worth starting an email thread on the LKML with
> Rob Herring and the SCMI maintainers to discuss the addition of
> scmi_devid to the binding.

Agree there. Also I want to point that there are other hypervisors, like
KVM, which may benefit from this.

Another approach is to name this node "xen,scmdi_devid", but I don't
like it.

>> I've chatted with Volodymyr_Babchuk and he gave a great idea to add a
>> list of device_ids to dom.cfg, such as:
>> sci_devs = [ 0, 1, 15, 35 ];
>>

Well, yes. We discussed this possibility, but personally I stick to
idea of re-using dt_dev, as we discussed in the different thread.

>> Using this approach, we can remove scmi_devid from the device tree and
>> just pass a list of scmi_devids to XEN using additional hypercall.
>> We can probably make hypercall taking devid list as input parameter.
>> This will take only 1 hypercall to setup sci permissions.
>
> But how would a user know which are the right SCMI IDs to add to the
> sci_devs list? Would the user have to go and read the reference manual
> of the platform to find the SCMI IDs and then write sci_devs by hand?
> If that is the case, then I think that it would be better to add
> scmi_devid to device tree.
>

Yes, ideally this needs to be done by BSP vendor in the device tree.


-- 
Volodymyr Babchuk at EPAM