Re: [PATCH 2/5] iommu/arm-smmu: add support for PCI master devices

[Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>]

On Thu, Jul 03, 2014 at 03:22:37PM +0100, Varun Sethi wrote:
> Hi Will,

Hi Varun,

Thanks for taking a look at this!

> > +static struct arm_smmu_master_cfg *
> > +find_smmu_master_cfg(struct arm_smmu_device *smmu, struct device *dev)
> > +{
> > +     struct arm_smmu_master *master;
> > +
> > +     if (dev_is_pci(dev))
> > +             return dev->archdata.iommu;
> > +
> > +     master = find_smmu_master(smmu, dev_get_master_dev(dev)->of_node);
> [Sethi Varun-B16395] Why use dev_get_master_dev over here? You can simply use dev.

True; we've already done the PCI check above. I'll tidy this up.

> > @@ -1598,15 +1642,36 @@ static int arm_smmu_add_device(struct device
> > *dev)
> >               return PTR_ERR(group);
> >       }
> >
> > +     if (dev_is_pci(dev)) {
> > +             struct arm_smmu_master_cfg *cfg;
> > +             struct pci_dev *pdev = to_pci_dev(dev);
> > +
> > +             cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
> > +             if (!cfg) {
> > +                     ret = -ENOMEM;
> > +                     goto out_put_group;
> > +             }
> > +
> > +             cfg->num_streamids = 1;
> > +             pci_for_each_dma_alias(pdev, __arm_smmu_get_pci_sid,
> > +                                    &cfg->streamids[0]);
> [Sethi Varun-B16395] We need to look for upstream DMA device. We should be
> using pci_find_dma_isolation_root here. Also, this would also imply that
> there could be multiple devices sharing the same stream ID. So, we should
> check if a particular stream ID value has already been configured in the
> SMR registers.

That function doesn't seem to appear in mainline or next. I can move to it
when it's available, but in the meantime the above is working for me. I'm
making the assumption here that the system is configured so that there
aren't any duplicate stream IDs. What we actually need is a function here
which maps the requester ID to a stream ID using firmware tables (provided
by DT or ACPI). In the absence of those tables at the moment, I just assign
the ID directly, which happens to work on my platform (1:1 mapping).

Once Thierry's generic IOMMU binding is sorted, we should look at adding
support for the Stream ID description. Have you looked at that at all?

Will

BTW: You seem to have a rather strange quoting style on your replies. Is
there any way to configure your editor to limit your lines to 80 columns?
You also don't need the prefix with your name and number in brackets!