On 19/07/17 11:48, Shameerali Kolothum Thodi wrote: > > >> -----Original Message----- >> From: Will Deacon [mailto:will.deacon@xxxxxxx] >> Sent: Friday, July 14, 2017 8:33 PM >> To: Shameerali Kolothum Thodi >> Cc: lorenzo.pieralisi@xxxxxxx; marc.zyngier@xxxxxxx; >> sudeep.holla@xxxxxxx; robin.murphy@xxxxxxx; hanjun.guo@xxxxxxxxxx; >> Gabriele Paoloni; John Garry; Linuxarm; linux-acpi@xxxxxxxxxxxxxxx; >> iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx; Wangzhou (B); Guohanjun (Hanjun Guo); >> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; devel@xxxxxxxxxx >> Subject: Re: [PATCH v3 2/2] iommu/arm-smmu-v3:Enable ACPI based >> HiSilicon erratum 161010801 >> > [...] >>>>> - list_add_tail(®ion->list, head); >>>>> + if ((smmu->options & ARM_SMMU_OPT_RESV_HW_MSI)) { >>>>> + >>>>> + if (!is_of_node(smmu->dev->fwnode)) >>>>> + resv = iort_iommu_its_get_resv_regions(dev, head); >>>> >>>> How does this work when we're not using ACPI? Shouldn't of vs ACPI >>>> be abstracted from the driver? >>> >>> At present ARM_SMMU_OPT_RESV_HW_MSI is only set for ACPI and DT >>> support for this is a low priority for us at the moment. Is the >>> suggestion is to have a common function outside the smmu driver for >>> _iommu_its_get_resv_regions() ? I am not sure what is the best way here. >> >> Right, something like that. The driver shouldn't need to care whether or not >> it's using ACPI or DT when setting these options. > > Below is what I have in mind for the common function for msi reserve. > But just wondering invoking iort_ functions from iommu code > is acceptable or not . Could you please take a look and let me know. At that point, it seems like we might as well just roll it into iommu_dma_get_resv_regions() directly[1]. It probably makes sense for any DT equivalent to be described generically, rather than SMMU-specific, so parsing that would fit into common code as well. Then in the SMMU drivers we can skip creating the SW_MSI region if iommu-dma gave us back any real ones (and remove the apparently unnecessary resv_msi check in VFIO). Or be lazy and just leave it, as it doesn't seem to do much harm to have both. Robin. [1] This is what I hacked up locally on top of patch #1: ----->8----- diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index 9d1cebe7f6cb..50292827da49 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -19,6 +19,7 @@ * along with this program. If not, see <http://www.gnu.org/licenses/>. */ +#include <linux/acpi_iort.h> #include <linux/device.h> #include <linux/dma-iommu.h> #include <linux/gfp.h> @@ -174,6 +175,10 @@ void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list) struct pci_host_bridge *bridge; struct resource_entry *window; + if (!is_of_node(dev->iommu_fwspec->iommu_fwnode) && + iort_iommu_its_get_resv_regions(dev, list) < 0) + return; + if (!dev_is_pci(dev)) return; -- 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
