Re: [PATCH v1 6/8] gpu/host1x: Use iommu_attach/detach_device()

[Jason Gunthorpe <jgg@nvidia.com>]

On Fri, Jan 07, 2022 at 08:35:34AM +0800, Lu Baolu wrote:
> On 1/6/22 11:35 PM, Jason Gunthorpe wrote:
> > On Thu, Jan 06, 2022 at 10:20:51AM +0800, Lu Baolu wrote:
> > > Ordinary drivers should use iommu_attach/detach_device() for domain
> > > attaching and detaching.
> > > 
> > > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> > >   drivers/gpu/host1x/dev.c | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
> > > index fbb6447b8659..6e08cb6202cc 100644
> > > +++ b/drivers/gpu/host1x/dev.c
> > > @@ -265,7 +265,7 @@ static struct iommu_domain *host1x_iommu_attach(struct host1x *host)
> > >   			goto put_cache;
> > >   		}
> > > -		err = iommu_attach_group(host->domain, host->group);
> > > +		err = iommu_attach_device(host->domain, host->dev);
> > >   		if (err) {
> > >   			if (err == -ENODEV)
> > >   				err = 0;
> > > @@ -335,7 +335,7 @@ static void host1x_iommu_exit(struct host1x *host)
> > >   {
> > >   	if (host->domain) {
> > >   		put_iova_domain(&host->iova);
> > > -		iommu_detach_group(host->domain, host->group);
> > > +		iommu_detach_device(host->domain, host->dev);
> > >   		iommu_domain_free(host->domain);
> > >   		host->domain = NULL;
> > 
> > Shouldn't this add the flag to tegra_host1x_driver ?
> 
> This is called for a single driver. The call trace looks like below:
> 
> static struct platform_driver tegra_host1x_driver = {
>         .driver = {
>                 .name = "tegra-host1x",
>                 .of_match_table = host1x_of_match,
>         },
>         .probe = host1x_probe,
>         .remove = host1x_remove,
> };
> 
> host1x_probe(dev)
> ->host1x_iommu_init(host)	//host is a wrapper of dev
>      iommu_domain_alloc(&platform_bus_type)
>      iommu_attach_group(domain, group);

The main question is if the iommu group is being shared with other
drivers, not the call chain for this function.

For tegra you have to go look in each entry of the of_match_table:

        { .compatible = "nvidia,tegra114-host1x", .data = &host1x02_info, },

And find the DTS block:

        host1x@50000000 {
                compatible = "nvidia,tegra114-host1x";
                reg = <0x50000000 0x00028000>;
                interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>, /* syncpt */
                             <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>; /* general */
                interrupt-names = "syncpt", "host1x";
                clocks = <&tegra_car TEGRA114_CLK_HOST1X>;
                clock-names = "host1x";
                resets = <&tegra_car 28>;
                reset-names = "host1x";
                iommus = <&mc TEGRA_SWGROUP_HC>;

Then check if any other devices in the DTS use the same 'iommus' which
is how the groups are setup.

I checked everything and it does look like this is a single device
group.

Jason