Re: [PATCH 11/12] xen/arm: if xen_force don't try to setup the IOMMU

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 15 Apr 2020, Julien Grall wrote:
> Hi Stefano,
> 
> On 15/04/2020 02:02, Stefano Stabellini wrote:
> > If xen_force (which means xen,force-assign-without-iommu was requested)
> > don't try to add the device to the IOMMU. Return early instead.
> 
> 
> Could you explain why this is an issue to call xen_force after
> iommu_add_dt_device()?

There are two issues. I should add info about both of them to the commit
message.


The first issue is that an error returned by iommu_add_dt_device (for
any reason) would cause handle_passthrough_prop to stop and return error
right away. But actually the iommu is not needed for that device if
xen_force is set.

(In fact, one of the reasons why a user might want to set
force-assign-without-iommu is because there are iommu issues with a
device.)


The second issue is about the usage of "xen,force-assign-without-iommu":
it would be useful to let the user set "xen,force-assign-without-iommu"
for devices that are described as behind a SMMU in device tree, but
the SMMU can't actually be used for some reason. Of course, the user
could always manually edit the device tree to make it look like as if
the device is not behind an IOMMU. That would work OK. However, I think
it would be better to avoid making that a requirement.

If we want to allow "xen,force-assign-without-iommu" for a device behind
a SMMU then we need this patch, otherwise this would happen:

    res = iommu_add_dt_device(node); // succeeds
    if ( xen_force && !dt_device_is_protected(node) ) // fails because the 
device is protected
        return 0;
    return iommu_assign_dt_device(kinfo->d, node); // fails because 
!is_iommu_enabled(d) which is fine but then handle_prop_pfdt returns error too



All in all, I thought it would make sense to avoid any iommu settings
and potential iommu errors altogether if xen_force is set and return
early.