| 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 | expand |
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>
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.
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).
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;
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(-)