|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 1/3] arm,smmu: switch to using iommu_fwspec functions
Hello Stefano,
> On 26 Jan 2021, at 10:58 pm, Stefano Stabellini <sstabellini@xxxxxxxxxx>
> wrote:
>
> From: Brian Woods <brian.woods@xxxxxxxxxx>
>
> Modify the smmu driver so that it uses the iommu_fwspec helper
> functions. This means both ARM IOMMU drivers will both use the
> iommu_fwspec helper functions, making enabling generic device tree
> bindings in the SMMU driver much cleaner.
>
> Signed-off-by: Brian Woods <brian.woods@xxxxxxxxxx>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
Reviewed-by: Rahul Singh <rahul.singh@xxxxxxx>
Tested-by: Rahul Singh <rahul.singh@xxxxxxx>
Regards,
Rahul
> ---
> Changes in v3:
> - add a comment in iommu_add_dt_device
> - don't allocate fwspec twice in arm_smmu_add_device
> - reuse existing fwspec pointer, don't add a second one
> - add comment about supporting fwspec at the top of the file
> ---
> xen/drivers/passthrough/arm/smmu.c | 98 ++++++++++++++++-----------
> xen/drivers/passthrough/device_tree.c | 7 ++
> 2 files changed, 66 insertions(+), 39 deletions(-)
>
> diff --git a/xen/drivers/passthrough/arm/smmu.c
> b/xen/drivers/passthrough/arm/smmu.c
> index 3e8aa37866..3898d1d737 100644
> --- a/xen/drivers/passthrough/arm/smmu.c
> +++ b/xen/drivers/passthrough/arm/smmu.c
> @@ -32,6 +32,9 @@
> * - 4k and 64k pages, with contiguous pte hints.
> * - Up to 48-bit addressing (dependent on VA_BITS)
> * - Context fault reporting
> + *
> + * Changes compared to Linux driver:
> + * - support for fwspec
> */
>
>
> @@ -49,6 +52,7 @@
> #include <asm/atomic.h>
> #include <asm/device.h>
> #include <asm/io.h>
> +#include <asm/iommu_fwspec.h>
> #include <asm/platform.h>
>
> /* Xen: The below defines are redefined within the file. Undef it */
> @@ -302,9 +306,6 @@ static struct iommu_group *iommu_group_get(struct device
> *dev)
>
> /***** Start of Linux SMMU code *****/
>
> -/* Maximum number of stream IDs assigned to a single device */
> -#define MAX_MASTER_STREAMIDS MAX_PHANDLE_ARGS
> -
> /* Maximum number of context banks per SMMU */
> #define ARM_SMMU_MAX_CBS 128
>
> @@ -597,8 +598,6 @@ struct arm_smmu_smr {
> };
>
> struct arm_smmu_master_cfg {
> - int num_streamids;
> - u16 streamids[MAX_MASTER_STREAMIDS];
> struct arm_smmu_smr *smrs;
> };
>
> @@ -686,6 +685,14 @@ static struct arm_smmu_option_prop arm_smmu_options[] = {
> { 0, NULL},
> };
>
> +static inline struct iommu_fwspec *
> +arm_smmu_get_fwspec(struct arm_smmu_master_cfg *cfg)
> +{
> + struct arm_smmu_master *master = container_of(cfg,
> + struct
> arm_smmu_master, cfg);
> + return dev_iommu_fwspec_get(&master->of_node->dev);
> +}
> +
> static void parse_driver_options(struct arm_smmu_device *smmu)
> {
> int i = 0;
> @@ -779,8 +786,9 @@ static int register_smmu_master(struct arm_smmu_device
> *smmu,
> struct device *dev,
> struct of_phandle_args *masterspec)
> {
> - int i;
> + int i, ret = 0;
> struct arm_smmu_master *master;
> + struct iommu_fwspec *fwspec;
>
> master = find_smmu_master(smmu, masterspec->np);
> if (master) {
> @@ -790,34 +798,37 @@ static int register_smmu_master(struct arm_smmu_device
> *smmu,
> return -EBUSY;
> }
>
> - if (masterspec->args_count > MAX_MASTER_STREAMIDS) {
> - dev_err(dev,
> - "reached maximum number (%d) of stream IDs for master
> device %s\n",
> - MAX_MASTER_STREAMIDS, masterspec->np->name);
> - return -ENOSPC;
> - }
> -
> master = devm_kzalloc(dev, sizeof(*master), GFP_KERNEL);
> if (!master)
> return -ENOMEM;
> + master->of_node = masterspec->np;
>
> - master->of_node = masterspec->np;
> - master->cfg.num_streamids = masterspec->args_count;
> + ret = iommu_fwspec_init(&master->of_node->dev, smmu->dev);
> + if (ret) {
> + kfree(master);
> + return ret;
> + }
> + fwspec = dev_iommu_fwspec_get(dev);
> +
> + /* adding the ids here */
> + ret = iommu_fwspec_add_ids(&masterspec->np->dev,
> + masterspec->args,
> + masterspec->args_count);
> + if (ret)
> + return ret;
>
> /* 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];
> -
> - if (!(smmu->features & ARM_SMMU_FEAT_STREAM_MATCH) &&
> - (streamid >= smmu->num_mapping_groups)) {
> - dev_err(dev,
> - "stream ID for master device %s greater than
> maximum allowed (%d)\n",
> - masterspec->np->name, smmu->num_mapping_groups);
> - return -ERANGE;
> + if (!(smmu->features & ARM_SMMU_FEAT_STREAM_MATCH)) {
> + for (i = 0; i < fwspec->num_ids; ++i) {
> + if (masterspec->args[i] >= smmu->num_mapping_groups) {
> + dev_err(dev,
> + "stream ID for master device %s greater
> than maximum allowed (%d)\n",
> + masterspec->np->name,
> smmu->num_mapping_groups);
> + return -ERANGE;
> + }
> }
> - master->cfg.streamids[i] = streamid;
> }
> return insert_smmu_master(smmu, master);
> }
> @@ -1390,6 +1401,7 @@ static int arm_smmu_master_configure_smrs(struct
> arm_smmu_device *smmu,
> int i;
> struct arm_smmu_smr *smrs;
> void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
> + struct iommu_fwspec *fwspec = arm_smmu_get_fwspec(cfg);
>
> if (!(smmu->features & ARM_SMMU_FEAT_STREAM_MATCH))
> return 0;
> @@ -1397,15 +1409,14 @@ static int arm_smmu_master_configure_smrs(struct
> arm_smmu_device *smmu,
> if (cfg->smrs)
> return -EEXIST;
>
> - smrs = kmalloc_array(cfg->num_streamids, sizeof(*smrs), GFP_KERNEL);
> + smrs = kmalloc_array(fwspec->num_ids, sizeof(*smrs), GFP_KERNEL);
> if (!smrs) {
> - dev_err(smmu->dev, "failed to allocate %d SMRs\n",
> - cfg->num_streamids);
> + dev_err(smmu->dev, "failed to allocate %d SMRs\n",
> fwspec->num_ids);
> return -ENOMEM;
> }
>
> /* Allocate the SMRs on the SMMU */
> - for (i = 0; i < cfg->num_streamids; ++i) {
> + for (i = 0; i < fwspec->num_ids; ++i) {
> int idx = __arm_smmu_alloc_bitmap(smmu->smr_map, 0,
> smmu->num_mapping_groups);
> if (IS_ERR_VALUE(idx)) {
> @@ -1416,12 +1427,12 @@ static int arm_smmu_master_configure_smrs(struct
> arm_smmu_device *smmu,
> smrs[i] = (struct arm_smmu_smr) {
> .idx = idx,
> .mask = 0, /* We don't currently share SMRs */
> - .id = cfg->streamids[i],
> + .id = fwspec->ids[i],
> };
> }
>
> /* It worked! Now, poke the actual hardware */
> - for (i = 0; i < cfg->num_streamids; ++i) {
> + for (i = 0; i < fwspec->num_ids; ++i) {
> u32 reg = SMR_VALID | smrs[i].id << SMR_ID_SHIFT |
> smrs[i].mask << SMR_MASK_SHIFT;
> writel_relaxed(reg, gr0_base + ARM_SMMU_GR0_SMR(smrs[i].idx));
> @@ -1443,12 +1454,13 @@ static void arm_smmu_master_free_smrs(struct
> arm_smmu_device *smmu,
> int i;
> void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
> struct arm_smmu_smr *smrs = cfg->smrs;
> + struct iommu_fwspec *fwspec = arm_smmu_get_fwspec(cfg);
>
> if (!smrs)
> return;
>
> /* Invalidate the SMRs before freeing back to the allocator */
> - for (i = 0; i < cfg->num_streamids; ++i) {
> + for (i = 0; i < fwspec->num_ids; ++i) {
> u8 idx = smrs[i].idx;
>
> writel_relaxed(~SMR_VALID, gr0_base + ARM_SMMU_GR0_SMR(idx));
> @@ -1465,16 +1477,17 @@ static int arm_smmu_domain_add_master(struct
> arm_smmu_domain *smmu_domain,
> int i, ret;
> struct arm_smmu_device *smmu = smmu_domain->smmu;
> void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
> + struct iommu_fwspec *fwspec = arm_smmu_get_fwspec(cfg);
>
> /* Devices in an IOMMU group may already be configured */
> ret = arm_smmu_master_configure_smrs(smmu, cfg);
> if (ret)
> return ret == -EEXIST ? 0 : ret;
>
> - for (i = 0; i < cfg->num_streamids; ++i) {
> + for (i = 0; i < fwspec->num_ids; ++i) {
> u32 idx, s2cr;
>
> - idx = cfg->smrs ? cfg->smrs[i].idx : cfg->streamids[i];
> + idx = cfg->smrs ? cfg->smrs[i].idx : fwspec->ids[i];
> s2cr = S2CR_TYPE_TRANS |
> (smmu_domain->cfg.cbndx << S2CR_CBNDX_SHIFT);
> writel_relaxed(s2cr, gr0_base + ARM_SMMU_GR0_S2CR(idx));
> @@ -1489,6 +1502,7 @@ static void arm_smmu_domain_remove_master(struct
> arm_smmu_domain *smmu_domain,
> int i;
> struct arm_smmu_device *smmu = smmu_domain->smmu;
> void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
> + struct iommu_fwspec *fwspec = arm_smmu_get_fwspec(cfg);
>
> /* An IOMMU group is torn down by the first device to be removed */
> if ((smmu->features & ARM_SMMU_FEAT_STREAM_MATCH) && !cfg->smrs)
> @@ -1499,8 +1513,8 @@ static void arm_smmu_domain_remove_master(struct
> arm_smmu_domain *smmu_domain,
> * that it can be re-allocated immediately.
> * Xen: Unlike Linux, any access to non-configured stream will fault.
> */
> - for (i = 0; i < cfg->num_streamids; ++i) {
> - u32 idx = cfg->smrs ? cfg->smrs[i].idx : cfg->streamids[i];
> + for (i = 0; i < fwspec->num_ids; ++i) {
> + u32 idx = cfg->smrs ? cfg->smrs[i].idx : fwspec->ids[i];
>
> writel_relaxed(S2CR_TYPE_FAULT,
> gr0_base + ARM_SMMU_GR0_S2CR(idx));
> @@ -1903,9 +1917,9 @@ static int arm_smmu_add_device(struct device *dev)
> struct arm_smmu_device *smmu;
> struct arm_smmu_master_cfg *cfg;
> struct iommu_group *group;
> + struct iommu_fwspec *fwspec;
> void (*releasefn)(void *) = NULL;
> int ret;
> -
> smmu = find_smmu_for_device(dev);
> if (!smmu)
> return -ENODEV;
> @@ -1925,13 +1939,19 @@ static int arm_smmu_add_device(struct device *dev)
> goto out_put_group;
> }
>
> - cfg->num_streamids = 1;
> + ret = iommu_fwspec_init(dev, smmu->dev);
> + if (ret) {
> + kfree(cfg);
> + goto out_put_group;
> + }
> + fwspec = dev_iommu_fwspec_get(dev);
> +
> /*
> * Assume Stream ID == Requester ID for now.
> * We need a way to describe the ID mappings in FDT.
> */
> pci_for_each_dma_alias(pdev, __arm_smmu_get_pci_sid,
> - &cfg->streamids[0]);
> + &fwspec->ids[0]);
> releasefn = __arm_smmu_release_pci_iommudata;
> } else {
> struct arm_smmu_master *master;
> diff --git a/xen/drivers/passthrough/device_tree.c
> b/xen/drivers/passthrough/device_tree.c
> index 999b831d90..a51ae3c9c3 100644
> --- a/xen/drivers/passthrough/device_tree.c
> +++ b/xen/drivers/passthrough/device_tree.c
> @@ -140,6 +140,13 @@ int iommu_add_dt_device(struct dt_device_node *np)
> if ( !ops )
> return -EINVAL;
>
> + /*
> + * This is needed in case a device has both the iommus property and
> + * also apperars in the mmu-masters list.
> + */
> + if ( dt_device_is_protected(np) )
> + return 0;
> +
> if ( dev_iommu_fwspec_get(dev) )
> return -EEXIST;
>
> --
> 2.17.1
>
>
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |