Hi Artem,
On 09/10/2019 15:20, Artem Mygaiev wrote:
Also do not set mark device as SMMU protected when there are no more
SMMU resources left
This is a sanity check on the content of the node, not a lack of resource in
this case.
TBH, the current placement is probably not correct. But I am not convinced the
new placement is better.
Xen 4.12 and earlier will ignore any failure and continue, so we cannot use this
device at all. Indeed, Xen will configure the SMMU to deny any transaction. If
you fail to initialize the device, then it will be in an unusable state. In this
case, we don't want a domain (including Dom0) to use it at all. This could be
achieved by calling dt_device_set_protected.
Xen 4.13 will stop booting if any of the SMMU fails to configure (this include
Master Device cannot be assigned). So there are no difference there.
My preference is to cater for all Xen versions so we can consider the patch for
a backport and potentially revert of the Xen 4.13 behavior (i.e. crashing when
one IOMMU is not correctly setup). So we would need to move the call at the
beginning of the function.
Coverity-ID: 1381862
Signed-off-by: Artem Mygaiev <[email protected]>
---
xen/drivers/passthrough/arm/smmu.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/xen/drivers/passthrough/arm/smmu.c
b/xen/drivers/passthrough/arm/smmu.c
index f151b9f5b5..cf42335eed 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -804,9 +804,6 @@ static int register_smmu_master(struct arm_smmu_device
*smmu,
master->of_node = masterspec->np;
master->cfg.num_streamids = masterspec->args_count;
- /* Xen: Let Xen know that the device is protected by an SMMU */
- dt_device_set_protected(masterspec->np);
-
for (i = 0; i < master->cfg.num_streamids; ++i) {
u16 streamid = masterspec->args[i];
@@ -815,10 +812,15 @@ static int register_smmu_master(struct arm_smmu_device *smmu,
dev_err(dev,
"stream ID for master device %s greater than maximum
allowed (%d)\n",
masterspec->np->name, smmu->num_mapping_groups);
+ devm_free(master);
return -ERANGE;
}
master->cfg.streamids[i] = streamid;
}
+
+ /* Xen: Let Xen know that the device is protected by an SMMU */
+ dt_device_set_protected(masterspec->np);
This code is using Linux coding style, so it should be hard tab here.
+
return insert_smmu_master(smmu, master);
}
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel