Re: [PATCH v2] xen/arm: Reorder SCI resource cleanup in domain destruction

[Julien Grall <julien@xxxxxxx>]

On 24/09/2025 09:54, Oleksii Moisieiev wrote:
> Hi Julien,
>
> On 22/09/2025 20:42, Julien Grall wrote:
> > (+ Release manager)
> >
> > Hi,
> >
> > On 14/09/2025 14:26, Oleksii Moisieiev wrote:
> > > Move the SCI (System Control and Management Interface) resource cleanup
> > > earlier in the domain_relinquish_resources() sequence to ensure proper
> > > cleanup ordering during domain destruction.
> > >
> > > The SCI cleanup is now performed before TEE (Trusted Execution
> > > Environment)
> > > cleanup rather than after P2M mapping cleanup. This reordering
> > > ensures that
> > > SCI resources are properly released before other subsystems that might
> > > depend on them are torn down.
> > >
> > > This change addresses potential resource cleanup dependencies where SCI
> > > resources need to be released before P2M mappings are cleaned up,
> > > preventing
> > > potential issues during domain destruction on ARM platforms with SCI
> > > support.
> > >
> > > Fixes: e2cc10867b (xen/arm: add generic SCI subsystem, 2025-09-04)
> > I am not sure where you found this syntax. This is not the one we use
> > for Xen. It should be:
> >
> > Fixes: <commit-id> ("<patch-subject>")
> >
> > Where the commit-id is 12 characters. For this patch it should be:
> >
> > Fixes: e2cc10867b63 ("xen/arm: add generic SCI subsystem")
> Got this by using command git show -s --pretty=reference <sha>
> Will fix.
> > > Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@xxxxxxxx>
> > > --->
> > > Changes in v2:
> > > - rearrange enum by placing PROG_sci before PROG_tee
> > > - add "Fixes:" tag
> > >
> > >    xen/arch/arm/domain.c | 11 ++++++-----
> > >    1 file changed, 6 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> > > index 1a8585d02b..e36719bce4 100644
> > > --- a/xen/arch/arm/domain.c
> > > +++ b/xen/arch/arm/domain.c
> > > @@ -1042,6 +1042,7 @@ static int relinquish_memory(struct domain *d,
> > > struct page_list_head *list)
> > >     */
> > >    enum {
> > >        PROG_pci = 1,
> > > +    PROG_sci,
> > Can you confirm this is fine to release the SCI resources *after* we
> > releases the devices? Does this mean they are not linked somehow? For
> > instance, if a device is re-assigned to another VM, could it fail
> > because the associated (?) SCI resources were not yet released?
> >
> > Cheers,
> This is not an issue for a single-agent. This is because single-agent
> doesn't implement relinquish_resources callback.
> For multiagent implementation relinquish_resources is done by sending
> SCMI_BASE_RESET_AGENT_CONFIGURATION message to the firmware which should
> drop all agent configuration previously done.
> If we start another VM with assigned device system will ask device
> permission from the firmware. And if device is assigned to another agent
> - error should be returned.
Thanks for the details. From what you wrote, I suspect we may need to 
move relinquishing SCI resources earlier. But as we don't have 
multi-agent right now, I will commit as-is and we can revisit.
Acked-by: Julien Grall <jgrall@xxxxxxxxxx>

Cheers,

--
Julien Grall