RE: [PATCH v3 4/4] iommu/arm-smmu-v3: Enable HTTU for stage1 with io-pgtable mapping

From: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: "iommu@lists.linux.dev" <iommu@lists.linux.dev>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"robin.murphy@arm.com" <robin.murphy@arm.com>,
	"will@kernel.org" <will@kernel.org>,
	"joro@8bytes.org" <joro@8bytes.org>,
	"ryan.roberts@arm.com" <ryan.roberts@arm.com>,
	"kevin.tian@intel.com" <kevin.tian@intel.com>,
	"nicolinc@nvidia.com" <nicolinc@nvidia.com>,
	"mshavit@google.com" <mshavit@google.com>,
	"eric.auger@redhat.com" <eric.auger@redhat.com>,
	"joao.m.martins@oracle.com" <joao.m.martins@oracle.com>,
	jiangkunkun <jiangkunkun@huawei.com>,
	zhukeqian <zhukeqian1@huawei.com>, Linuxarm <linuxarm@huawei.com>
Subject: RE: [PATCH v3 4/4] iommu/arm-smmu-v3: Enable HTTU for stage1 with io-pgtable mapping
Date: Wed, 22 May 2024 13:26:20 +0000	[thread overview]
Message-ID: <3d0aa20bbb97452cb78b3bb94af24ebd@huawei.com> (raw)
In-Reply-To: <ZkCxTWbkK0Domuwr@nvidia.com>

> -----Original Message-----
> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Sunday, May 12, 2024 1:09 PM
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: iommu@lists.linux.dev; linux-arm-kernel@lists.infradead.org;
> robin.murphy@arm.com; will@kernel.org; joro@8bytes.org;
> ryan.roberts@arm.com; kevin.tian@intel.com; nicolinc@nvidia.com;
> mshavit@google.com; eric.auger@redhat.com; joao.m.martins@oracle.com;
> jiangkunkun <jiangkunkun@huawei.com>; zhukeqian
> <zhukeqian1@huawei.com>; Linuxarm <linuxarm@huawei.com>
> Subject: Re: [PATCH v3 4/4] iommu/arm-smmu-v3: Enable HTTU for stage1
> with io-pgtable mapping
> 
> On Tue, Apr 30, 2024 at 02:43:08PM +0100, Shameer Kolothum wrote:
> 
> > diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-
> arm.c
> > index da6cc52859ba..20ac0e833c7b 100644
> > --- a/drivers/iommu/io-pgtable-arm.c
> > +++ b/drivers/iommu/io-pgtable-arm.c
> > @@ -433,6 +433,8 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct
> arm_lpae_io_pgtable *data,
> >  		pte = ARM_LPAE_PTE_nG;
> >  		if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ))
> >  			pte |= ARM_LPAE_PTE_AP_RDONLY;
> > +		else if (data->iop.cfg.quirks & IO_PGTABLE_QUIRK_ARM_HD)
> > +			pte |= ARM_LPAE_PTE_AP_WRITABLE_CLEAN;
> 
> This seems a bit suboptimal, it means the HTTU will be generating
> dirty's before the tracking is turned on. As I understand it if the
> SMMU wants to write a dirty bit it has to do an atomic RMW to memory,
> so this would be a drag on baseline performance?

Our initial tests has not shown any difference so far. But it is a possibility.

> Should this start out as dirty and let the enable flow clean it to
> turn it on?

Ok. Just to be clear, so the suggestion is just set the DBM bit here and let the
read_and_clear_dirty() set the  AP_RDONLY_BIT through the iopt_set_dirty_tracking()
path.

> 
> Looks fine otherwise:
> 
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Thanks,
Shameer