[Xen-devel] xen/arm: iommu: Panic if not all IOMMUs are initialized

Message ID 20190820122255.9864-1-julien.grall@arm.com
State New
Headers show
Series
  • [Xen-devel] xen/arm: iommu: Panic if not all IOMMUs are initialized
Related show

Commit Message

Julien Grall Aug. 20, 2019, 12:22 p.m.
At the moment, the platform can come up with only part of the IOMMUs
initialized. This could lead to a failure later on when building the
hardware domain or even trying to assign a device to a guest.

To avoid unwanted behavior, Xen will not continue if one of the IOMMUs
has not been initialized correctly.

Signed-off-by: Julien Grall <julien.grall@arm.com>

---

Cc: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

Basically, this is similar to forcing the use of IOMMU (i.e
iommu=force). Maybe we should drop the panic in setup.c and just
set force_iommu. Any opinion?
---
 xen/arch/arm/setup.c                | 5 ++++-
 xen/drivers/passthrough/arm/iommu.c | 8 ++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

Comments

Oleksandr Aug. 20, 2019, 2:28 p.m. | #1
On 20.08.19 15:22, Julien Grall wrote:

Hi, Julien

>   
> -    iommu_setup();
> +    rc = iommu_setup();
> +    if ( !iommu_enabled && rc != -ENODEV )
> +        panic("Couldn't configure correctly all the IOMMUs.");
>   

Please add "\n"


You can add:

Tested-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Stefano Stabellini Sept. 20, 2019, 12:31 a.m. | #2
On Tue, 20 Aug 2019, Oleksandr wrote:
> 
> On 20.08.19 15:22, Julien Grall wrote:
> 
> Hi, Julien
> 
> >   -    iommu_setup();
> > +    rc = iommu_setup();
> > +    if ( !iommu_enabled && rc != -ENODEV )
> > +        panic("Couldn't configure correctly all the IOMMUs.");
> >   
> 
> Please add "\n"
> 
> 
> You can add:
> 
> Tested-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

I added the "\n", fixed a typo in the commit message, and committed the
patch.
Oleksandr Sept. 20, 2019, 1:05 p.m. | #3
On 20.09.19 03:31, Stefano Stabellini wrote:

Hi, Stefano.

> On Tue, 20 Aug 2019, Oleksandr wrote:
>> On 20.08.19 15:22, Julien Grall wrote:
>>
>> Hi, Julien
>>
>>>    -    iommu_setup();
>>> +    rc = iommu_setup();
>>> +    if ( !iommu_enabled && rc != -ENODEV )
>>> +        panic("Couldn't configure correctly all the IOMMUs.");
>>>    
>> Please add "\n"
>>
>>
>> You can add:
>>
>> Tested-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
>
> I added the "\n", fixed a typo in the commit message, and committed the
> patch.


Thank you, I will re-base and drop dependency from the cover letter for 
the coming V5 (IPMMU+iommu_fwspec).

Patch

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 7509d76dd4..f8a4064d3e 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -755,6 +755,7 @@  void __init start_xen(unsigned long boot_phys_offset,
         .max_grant_frames = gnttab_dom0_frames(),
         .max_maptrack_frames = opt_max_maptrack_frames,
     };
+    int rc;
 
     dcache_line_bytes = read_dcache_line_bytes();
 
@@ -892,7 +893,9 @@  void __init start_xen(unsigned long boot_phys_offset,
 
     setup_virt_paging();
 
-    iommu_setup();
+    rc = iommu_setup();
+    if ( !iommu_enabled && rc != -ENODEV )
+        panic("Couldn't configure correctly all the IOMMUs.");
 
     do_initcalls();
 
diff --git a/xen/drivers/passthrough/arm/iommu.c b/xen/drivers/passthrough/arm/iommu.c
index 2135233736..f219de9ac3 100644
--- a/xen/drivers/passthrough/arm/iommu.c
+++ b/xen/drivers/passthrough/arm/iommu.c
@@ -51,6 +51,14 @@  int __init iommu_hardware_setup(void)
         rc = device_init(np, DEVICE_IOMMU, NULL);
         if ( !rc )
             num_iommus++;
+        /*
+         * Ignore the following error codes:
+         *   - EBADF: Indicate the current not is not an IOMMU
+         *   - ENODEV: The IOMMU is not present or cannot be used by
+         *     Xen.
+         */
+        else if ( rc != -EBADF && rc != -ENODEV )
+            return rc;
     }
 
     return ( num_iommus > 0 ) ? 0 : -ENODEV;