1. Patchew
  2. Xen
  3. View series

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

Oleksii Moisieiev posted 1 patch 1 month, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20ec9d9a8533417489a95543c1b72f7f55865c9c.1757856381.git.oleksii._5Fmoisieiev@epam.com
xen/arch/arm/domain.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
[PATCH v2] xen/arm: Reorder SCI resource cleanup in domain destruction
Posted by Oleksii Moisieiev 1 month, 2 weeks ago 
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)

Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
---

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,
     PROG_tee,
     PROG_xen,
     PROG_page,
@@ -1049,7 +1050,6 @@ enum {
     PROG_p2m_root,
     PROG_p2m,
     PROG_p2m_pool,
-    PROG_sci,
     PROG_done,
 };
 
@@ -1090,6 +1090,11 @@ int domain_relinquish_resources(struct domain *d)
             return ret;
 #endif
 
+    PROGRESS(sci):
+        ret = sci_relinquish_resources(d);
+        if ( ret )
+            return ret;
+
     PROGRESS(tee):
         ret = tee_relinquish_resources(d);
         if (ret )
@@ -1109,10 +1114,6 @@ int domain_relinquish_resources(struct domain *d)
         ret = relinquish_p2m_mapping(d);
         if ( ret )
             return ret;
-    PROGRESS(sci):
-        ret = sci_relinquish_resources(d);
-        if ( ret )
-            return ret;
 
     PROGRESS(p2m_root):
         /*
-- 
2.34.1
Re: [PATCH v2] xen/arm: Reorder SCI resource cleanup in domain destruction
Posted by Julien Grall 1 month, 1 week ago 
(+ 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")

> 
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
 > --->
> 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,

-- 
Julien Grall
Re: [PATCH v2] xen/arm: Reorder SCI resource cleanup in domain destruction
Posted by Oleksii Moisieiev 1 month ago 
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@epam.com>
> > --->
>> 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.

--
Oleksii
Re: [PATCH v2] xen/arm: Reorder SCI resource cleanup in domain destruction
Posted by Julien Grall 1 month ago 

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@epam.com>
>>> --->
>>> 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@amazon.com>

Cheers,

-- 
Julien Grall
Re: [PATCH v2] xen/arm: Reorder SCI resource cleanup in domain destruction
Posted by Oleksii Moisieiev 1 month ago 

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@epam.com>
>>>> --->
>>>> 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@amazon.com>
>
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,
>
Re: [PATCH v2] xen/arm: Reorder SCI resource cleanup in domain destruction
Posted by Julien Grall 1 month ago 
Hi Oleksii,

On 28/09/2025 14:52, Oleksii Moisieiev wrote:
> 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?

No need for a new version. I have updated the commit message and committed.

Cheers,

-- 
Julien Grall

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.