Re: [PATCH v5 10/10] xen/arm: smmuv3: Add support for SMMUv3 driver

[Julien Grall <julien@xxxxxxx>]

Hi Rahul,

Please try to trim the e-mail when quoting, otherwise it is quite 
difficult to find the only couple of answer you wrote.
On 21/01/2021 17:10, Rahul Singh wrote:
> > On 20 Jan 2021, at 8:31 pm, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:
> > > -static void __iomem *arm_smmu_ioremap(struct device *dev, resource_size_t 
> > > start,
> > > -                                     resource_size_t size)
> > > +
> > > +static void arm_smmu_free_structures(struct arm_smmu_device *smmu)
> > > {
> > > -       struct resource res = {
> > > -               .flags = IORESOURCE_MEM,
> > > -               .start = start,
> > > -               .end = start + size - 1,
> > > -       };
> > > +       if (smmu->cmdq.q.base)
> > > +               xfree(smmu->cmdq.q.base);
> > > +
> > > +       if (smmu->evtq.q.base)
> > > +               xfree(smmu->evtq.q.base);
> > >
> > > -       return devm_ioremap_resource(dev, &res);
> > > +       if (smmu->priq.q.base)
> > > +               xfree(smmu->priq.q.base);
> > > +
> > > +       if (smmu->strtab_cfg.strtab)
> > > +               xfree(smmu->strtab_cfg.strtab);
> > > +
> > > +       if (smmu->strtab_cfg.l1_desc)
> > > +               xfree(smmu->strtab_cfg.l1_desc);
> >  From what I can tell we also need to free somewhere
> > smmu->strtab_cfg->l1_desc->l2ptr allocated by arm_smmu_init_l2_strtab
> "l1_desc->l2ptr" is a pointer to the Level 1 Stream Table Descriptor if 2-level 
> Stream Table supported.
>
> If the device is protected by IOMMU, SMMUv3 driver will allocate the  
> "l1_desc->l2ptr” when the device is added to the IOMMU via 
> arm_smmu_add_device() function and device will be configured in bypass/abort mode.
>
> Once we assign the device to the domain(arm_smmu_assign_dev() ) smmuv3 hw will 
> be configured correctly to match the StreamID. If there is a failure in 
> assigning the device, that case also XEN will not remove the device and master 
> device still be in bypass/abort mode.
I am a bit confused with this answer. Wouldn't this mean that we are 
"leaking" memory if we fail to assign the device?
> As in XEN, there is no function to remove the master device from the IOMMU, because of that I 
> feel there is no need to free the "l1_desc->l2ptr” in case of failure also.
Hmmm... Xen is able to remove device from the IOMMU. The reason this is 
not implemented yet on Arm is because you can't hot-unplug "platform" 
device.
I expect the removal function to be useful for PCI.

Cheers,

--
Julien Grall