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

[Oleksii Moisieiev <Oleksii_Moisieiev@xxxxxxxx>]

On 27/09/2025 20:07, Julien Grall wrote:
>
>
> 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>
>
Thanks. I'm preparing patch-series with multi-agent support and will see 
where it could be moved.
Should I send a new patch with
"Fixes: <commit-id> ("<patch-subject>")"
syntax or it can be fixed inplace during commit?
> Cheers,
>