diff mbox series

[01/16] iommu: Add checks before calling iommu suspend/resume

Message ID 5694f2b73e808ebdc5d5b60a31cfad2b24f0e9d8.1741164138.git.xakep.amatop@gmail.com (mailing list archive)
State New
Headers show
Series Suspend to RAM support for Xen on arm64 | expand

Commit Message

Mykola Kvach March 5, 2025, 9:11 a.m. UTC 
From: Mykyta Poturai <mykyta_poturai@epam.com>

These functions may be unimplemented, so check that they exist before
calling to prevent crashes.

Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
---
Introduced in patch series V3.
---
 xen/drivers/passthrough/iommu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Jan Beulich March 5, 2025, 4:45 p.m. UTC | #1 
On 05.03.2025 10:11, Mykola Kvach wrote:
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -613,7 +613,7 @@ int __init iommu_setup(void)
>  
>  int iommu_suspend(void)
>  {
> -    if ( iommu_enabled )
> +    if ( iommu_enabled && iommu_get_ops() && iommu_get_ops()->suspend )
>          return iommu_call(iommu_get_ops(), suspend);
>  
>      return 0;
> @@ -621,7 +621,7 @@ int iommu_suspend(void)
>  
>  void iommu_resume(void)
>  {
> -    if ( iommu_enabled )
> +    if ( iommu_enabled && iommu_get_ops() && iommu_get_ops()->resume )
>          iommu_vcall(iommu_get_ops(), resume);
>  }

When iommu_enabled is true, surely iommu_get_ops() is required to return
non-NULL?

Jan
Julien Grall March 11, 2025, 8:27 p.m. UTC | #2 
Hi,

On 05/03/2025 09:11, Mykola Kvach wrote:
> From: Mykyta Poturai <mykyta_poturai@epam.com>
> 
> These functions may be unimplemented, so check that they exist before
> calling to prevent crashes.

Looking at the cover letter, I see you wrote the following:

"Add suspend/resume handlers to IOMMU drivers (there aren’t any
problems with the current implementation because the domains used for
test are thin, and this patch series implements only the very basic
logic)"

which I read as this patch is a temporary hack until we implement IOMMU.
Is that correct? If so, can you tag it as HACK and move to the end to 
end up to merge it?

Cheers,
diff mbox series

Patch

diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 16aad86973..55b33c9719 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -613,7 +613,7 @@  int __init iommu_setup(void)
 
 int iommu_suspend(void)
 {
-    if ( iommu_enabled )
+    if ( iommu_enabled && iommu_get_ops() && iommu_get_ops()->suspend )
         return iommu_call(iommu_get_ops(), suspend);
 
     return 0;
@@ -621,7 +621,7 @@  int iommu_suspend(void)
 
 void iommu_resume(void)
 {
-    if ( iommu_enabled )
+    if ( iommu_enabled && iommu_get_ops() && iommu_get_ops()->resume )
         iommu_vcall(iommu_get_ops(), resume);
 }