Re: [RFC PATCH v3 1/7] xen/arm: add generic SCI subsystem

[Grygorii Strashko <grygorii_strashko@xxxxxxxx>]

On 24.03.25 16:36, Jan Beulich wrote:
> On 24.03.2025 15:25, Grygorii Strashko wrote:
> > On 11.03.25 13:43, Jan Beulich wrote:> On 11.03.2025 12:16, Grygorii Strashko 
> > wrote:
> > > > @@ -851,6 +852,18 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
> > > > u_domctl)
> > > >        case XEN_DOMCTL_deassign_device:
> > > >        case XEN_DOMCTL_get_device_group:
> > > >            ret = iommu_do_domctl(op, d, u_domctl);
> > > > +
> > > > +        if ( ret >= 0 || (ret == -EOPNOTSUPP) || (ret == -ENODEV) )
> > > > +        {
> > > > +            /*
> > > > +             * TODO: RFC
> > > > +             * This change will allow to pass DT nodes/devices to
> > > > +             * XEN_DOMCTL_assign_device OP using xl.cfg:"dtdev" property even
> > > > +             * if those DT nodes/devices even are not behind IOMMU (or IOMMU
> > > > +             * is disabled) without failure.
> > > > +             */
> > > > +            ret = sci_do_domctl(op, d, u_domctl);
> > > > +        }
> > > >            break;
> > > Despite the comment I fear I don't understand what you're trying to do here.
> > It enables in toolstack "Device specific access control" function, which is 
> > implemented in SCI FW.
> > SCI FW has privileged management interface assigned to Xen,
> > and non-privileged interfaces assigned to guest (VM) and identified by agent_id.
> >
> > SCI FW manages access to HW resources clocks, resets, etc, which considered 
> > shared and
> > which can't be accessed from multiple domains due to HW limitations.
> > SCI FW can also manage safety specific resources like HW firewalls for example.
> >
> > Each device identified by device_id and can have HW resources assigned to it
> > device_id_res = {clk_1, clk_2, reset_1, pd_1 } - FW implementation specific.
> >
> > Device can be assigned:
> > 1) to any VM, but only to one - dynamic configuration;
> > 2) only one, specific VM identified by agent_id - static configuration.
> > The policy is FW implementation specific.
> >
> > Here and below the case (1) is considered, while in the case (2) - nothing need 
> > to be done.
> > To enable VMx access to device_id (and its resources) the special request need
> > to be sent to the FW management interface to get device_id accessible from VMx.
> >
> > In case of SCMI, ARM System Control and Management Interface (SCMI)
> > specification (DEN0056E) - functionality defined in sections
> > 4.2.1.1 Device specific access control
> > 4.2.2.11 BASE_SET_DEVICE_PERMISSIONS
> >
> > The HW configuration described in device tree, like in the below example
> > (abstract, not related to any specific FW, but principle is generic)
> >
> > Host DT:
> > /sci_fw {
> >         #access-controller-cells = <1>;
> >         #reset-cells = <1>;
> >         #clock-cells = <1>;
> >         #power-domain-cells = <1>;
> > }
> >
> > /soc/deviceA {
> >         clocks = <&sci_fw 1>, <&sci_fw 2>;
> >         power-domains = <&sci_fw 1>;
> >         resets = <&sci_fw 1>;
> >         access-controllers = <&sci_fw 0>;
> > }
> >
> > To trigger SCI FW it required to pass Host DT device path "/soc/deviceA" down
> > to the corresponding SCI FW driver during domain creation by toolstack.
> > And it has to be done as for devices behind IOMMU, as for devices
> > not protected by IOMMU.
> >
> > To achieve above xl.cfg:"dtdev" property was selected to be used due to:
> > - xen doc says
> > "
> > Specifies the host device tree nodes to pass through to this guest.
> > Each DTDEV_PATH is an absolute path in the device tree.
> > "
> > - toolstack triggers XEN_DOMCTL_assign_device(XEN_DOMCTL_DEV_DT) hypercall
> > nothing from above says it's strictly limited to IOMMU-protected devices only.
> >
> > But now ARM XEN_DOMCTL_assign_device actually limited to IOMMU-protected devices
> > and will return to toolstack:
> > -EOPNOTSUPP if iommu is not enabled
> > -EINVAL if DT device is not IOMMU-protected
> >
> > in both cases toolstack will fail.
> >
> > Idea behind this change (and change in iommu_do_dt_domctl()) is to enable
> > XEN_DOMCTL_assign_device(XEN_DOMCTL_DEV_DT) and so xl.cfg:"dtdev"
> > for DT devices which
> > - IOMMU-protected only (has "iommus" property)
> > - require device access control (has "access-controllers" property)
> > - both
> Thanks for all the details, but I feel overwhelmed. I'd like to see this 
> clarified
> in more basic terms. For example the comment says "This change will allow ...".
> What's "this change" here? Together with "TODO: RFC" it feels a little as if the
> code comment was really meant to live elsewhere (patch description? post-commit-
> message area of the submission?), and thus offering little value to an observer
> like me. Yet as this is common code, I'd like to have at least a rough, high
> level understanding of what's going on here. This doesn't need to go into any 
> Arm
> details that I may not fully understand / appreciate anyway.
I'm very sorry for the confusion, I'll move changes in question in separate 
patch.

--
Best regards,
-grygorii