Hi Will, Lorenzo, Robin,
I have created the patch to add DT support for this erratum.
However, currently I have only added support for pci-based devices.
I'm a bit stumped on how to add platform device support, or if we
should also add support at all. And I would rather ask before
sending the patches.
The specific issue is that I know of no platform device with an ITS
msi-parent which we would want reserve, i.e. do not translate msi.
And, if there were, how to do it.
The only platform devices I know off with msi ITS parents are SMMUv3
and mbigen. For mbigen, the msi are not translated. Actually, as I
see under IORT solution, for mbigen we don't reserve the hw msi
region as the SMMUv3 does not have an ID mapping. And we have no
equivalent ID mapping property for DT SMMUv3, so cannot create an
equivalent check.
So here is how the DT iommu get reserved region function with only
pci device support looks:
/**
* of_iommu_its_get_resv_regions - Reserved region driver helper
* @dev: Device from iommu_get_resv_regions()
* @list: Reserved region list from iommu_get_resv_regions()
*
* Returns: Number of reserved regions on success(0 if no associated ITS),
* appropriate error value otherwise.
*/
int of_iommu_its_get_resv_regions(struct device *dev, struct
list_head *head)
{
struct device_node *of_node = NULL;
struct device *parent_dev;
const __be32 *msi_map;
int num_mappings, i, err, len, resv = 0;
/* walk up to find the parent with a msi-map property */
for (parent_dev = dev; parent_dev; parent_dev = parent_dev->parent) {
if (!parent_dev->of_node)
continue;
/*
* Current logic to reserve ITS regions for only PCI root
* complex.
*/
msi_map = of_get_property(parent_dev->of_node, "msi-map", &len);
if (msi_map) {
of_node = parent_dev->of_node;
break;
}
}
if (!of_node)
return 0;
num_mappings = of_count_phandle_with_args(of_node, "msi-map",
NULL) / 4;
for (i = 0; i < num_mappings; i++) {
int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
struct iommu_resv_region *region;
struct device_node *msi_node;
struct resource resource;
u32 phandle;
phandle = be32_to_cpup(msi_map + 1);
msi_node = of_find_node_by_phandle(phandle);
if (!msi_node)
return -ENODEV;
/*
* There may be different types of msi-controller, so check
* for ITS.
*/
if (!of_device_is_compatible(msi_node, "arm,gic-v3-its")) {
of_node_put(msi_node);
continue;
}
err = of_address_to_resource(msi_node, 0, &resource);
of_node_put(msi_node);
if (err)
return err;
region = iommu_alloc_resv_region(resource.start, SZ_128K, prot,
IOMMU_RESV_MSI);
if (region) {
list_add_tail(®ion->list, head);
resv++;
}
}
return (resv == num_mappings) ? resv : -ENODEV;
}
Any feedback is appreciated.
Hi John,
I appreciate it is not trivial to make the code generic, part of the
snippet above can be shared between ACPI/IORT and OF, the only sticking
point is probably the "compatible" string that has to be parameterized
since having this code in generic IOMMU layer is a bit horrible to
have it ITS specific (and it is a problem that was existing already
in the original patch with the hardcoded ITS node type or function
name).
Hi Lorenzo,
Agreed, checking the ITS compatible string in IOMMU code is not nice.
However the function is just trying to replicate our ACPI version, which
calls IORT code directly, and this is ITS specific. Anyway, we can make
it generic.
To sum it up the hook:
- has to be called from SMMU driver in a firmware agnostic way
ok
- somehow it has to ascertain which interrupt controller "compatible"
(which in IORT is a node type) to match against so the hook has to
take an id of sorts
OK
I will mention 2 more points after discussion on OF solution with Shameer:
- for platform device, we can add suppport by checking for the devices
msi-parent property
- for both pci and platform device, we should check device rid for
msi-controller also, like IORT solution
BTW, even though merge window is open, we would like to send some
solution to the lists this week, so any outstanding topics can
potentially be discussed at LPC next week. Let me know if you're unhappy
about this.
All the best,
John
I need to go back and have a look at the original patch to see how
we can accommodate both OF/ACPI - certainly the region reservations
code can and should be shared.
Lorenzo
.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
