Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <310913b0a92bd06b93ede9e01e3f99cb5bce4e7a.camel@linux.ibm.com>
Subject: Re: [PATCH v3 2/7] iommu: Allow .iotlb_sync_map to fail and handle
 s390's -ENOMEM return
From:   Niklas Schnelle <schnelle@linux.ibm.com>
To:     Joerg Roedel <joro@8bytes.org>, Will Deacon <will@kernel.org>,
        Robin Murphy <robin.murphy@arm.com>,
        Jason Gunthorpe <jgg@nvidia.com>,
        Wenjia Zhang <wenjia@linux.ibm.com>
Cc:     Matthew Rosato <mjrosato@linux.ibm.com>,
        Gerd Bayer <gbayer@linux.ibm.com>,
        Pierre Morel <pmorel@linux.ibm.com>, iommu@lists.linux.dev,
        linux-s390@vger.kernel.org, borntraeger@linux.ibm.com,
        hca@linux.ibm.com, gor@linux.ibm.com,
        gerald.schaefer@linux.ibm.com, agordeev@linux.ibm.com,
        svens@linux.ibm.com, linux-kernel@vger.kernel.org,
        Julian Ruess <julianr@linux.ibm.com>
Date:   Tue, 03 Jan 2023 17:03:23 +0100
In-Reply-To: <20230102115619.2088685-3-schnelle@linux.ibm.com>
References: <20230102115619.2088685-1-schnelle@linux.ibm.com>
         <20230102115619.2088685-3-schnelle@linux.ibm.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.46.2 (3.46.2-1.fc37) 
MIME-Version: 1.0
Precedence: bulk

On Mon, 2023-01-02 at 12:56 +0100, Niklas Schnelle wrote:
> On s390 .iotlb_sync_map is used to sync mappings to an underlying
> hypervisor by letting the hypervisor inspect the synced IOVA range and
> updating its shadow table. This however means that it can fail as the
> hypervisor may run out of resources. This can be due to the hypervisor
> being unable to pin guest pages, due to a limit on concurrently mapped
> addresses such as vfio_iommu_type1.dma_entry_limit or other resources.
> Either way such a failure to sync a mapping should result in
> a DMA_MAPPING_EROR.
>=20
> Now especially when running with batched IOTLB flushes for unmap it may
> be that some IOVAs have already been invalidated but not yet synced via
> .iotlb_sync_map. Thus if the hypervisor indicates running out of
> resources, first do a global flush allowing the hypervisor to free
> resources associated with these mappings and only if that also fails
> report this error to callers.
>=20
> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> ---
>  drivers/iommu/amd/iommu.c   |  5 +++--
>  drivers/iommu/apple-dart.c  |  5 +++--
>  drivers/iommu/intel/iommu.c |  5 +++--
>  drivers/iommu/iommu.c       | 20 ++++++++++++++++----
>  drivers/iommu/msm_iommu.c   |  5 +++--
>  drivers/iommu/mtk_iommu.c   |  5 +++--
>  drivers/iommu/s390-iommu.c  | 29 ++++++++++++++++++++++++-----
>  drivers/iommu/sprd-iommu.c  |  5 +++--
>  drivers/iommu/tegra-gart.c  |  5 +++--
>  include/linux/iommu.h       |  4 ++--
>  10 files changed, 63 insertions(+), 25 deletions(-)

Ok kernel test robot reported that I missed an implementation of
.iotlb_sync_map in drivers/iommu/sun50i-iommu.c during rebase as that
was only added in v6.2-rc1 by commit e563cc0c787c85 ("iommu/sun50i:
Implement .iotlb_sync_map"). Will add and send a v4 including the
proposed commit message rewording too.

>=20
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index cbeaab55c0db..3df7d20e0e52 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -2180,14 +2180,15 @@ static int amd_iommu_attach_device(struct iommu_d=
omain *dom,
>  	return ret;
>  }
> =20
> -static void amd_iommu_iotlb_sync_map(struct iommu_domain *dom,
> -				     unsigned long iova, size_t size)
---8<---