[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH V2 2/6] iommu/arm: Add ability to handle deferred probing request



Hi Stefano,

On 14/08/2019 20:25, Stefano Stabellini wrote:
On Wed, 14 Aug 2019, Julien Grall wrote:
I agree that we should enable all IOMMUs or none. I don't think we want
to deal with partially initialized IOMMUs systems.

Failure to enable all IOMMUs should lead to returning an error from the
relevant function (arch_iommu_populate_page_table?) which should

The patch is:

|> start_xen()
|>   iommu_setup()
|>     iommu_hardware_setup()

translate into Xen failing to boot and crashing. Which I think it is
what you are suggesting, right?

That's correct. At the moment the return value of iommu_setup() is ignored. What I would like to suggest is something along the following lines:

rc = iommu_setup();
if ( iommu_enable && rc != -ENODEV )
  panic("Unable to setup IOMMUs");


(I wouldn't call panic() inside the IOMMU specific initializer, because
there might be iommu= command line options for Xen that specify a
different desired outcome. I would deal with this case the same way we
would deal with an error during initialization of a single IOMMU.)

I am not sure to understand this. If you have an half initialized IOMMU (note the "single" here!), then continuing is likely to make things much worst.

I don't advocate to put the panic() inside the IOMMU specific initializer (see above). But clearly, we should return an error no matter the content of 'iommu' command line if the user requested to enable the IOMMUs (if any). It wouldn't be right if the user can say "ignore IOMMU error" as most likely you will have unexpected error afterwards.

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.