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;
Date:   Wed, 5 Apr 2023 07:50:34 +0200
From:   Petr =?UTF-8?B?VGVzYcWZw61r?= <petr@tesarici.cz>
To:     Dexuan Cui <decui@microsoft.com>
Cc:     Dexuan-Linux Cui <dexuan.linux@gmail.com>,
        Petr Tesarik <petrtesarik@huaweicloud.com>,
        Christoph Hellwig <hch@lst.de>,
        Marek Szyprowski <m.szyprowski@samsung.com>,
        Robin Murphy <robin.murphy@arm.com>,
        Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
        Jianxiong Gao <jxgao@google.com>,
        David Stevens <stevensd@chromium.org>,
        Joerg Roedel <jroedel@suse.de>,
        "open list:DMA MAPPING HELPERS" <iommu@lists.linux.dev>,
        open list <linux-kernel@vger.kernel.org>,
        Roberto Sassu <roberto.sassu@huawei.com>,
        "Michael Kelley (LINUX)" <mikelley@microsoft.com>,
        Tianyu Lan <Tianyu.Lan@microsoft.com>,
        "linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>
Subject: Re: [PATCH v1 2/2] swiotlb: Fix slot alignment checks
Message-ID: <20230405075034.3c36bb77@meshulam.tesarici.cz>
In-Reply-To: <20230405072801.05bb94ef@meshulam.tesarici.cz>
References: <cover.1679382779.git.petr.tesarik.ext@huawei.com>
        <c90887e4d75344abe219cc5e12f7c6dab980cfce.1679382779.git.petr.tesarik.ext@huawei.com>
        <CAA42JLa1y9jJ7BgQvXeUYQh-K2mDNHd2BYZ4iZUz33r5zY7oAQ@mail.gmail.com>
        <CO1PR21MB13320305E02BA121623213DABF939@CO1PR21MB1332.namprd21.prod.outlook.com>
        <20230405064019.6258ebb3@meshulam.tesarici.cz>
        <SA1PR21MB1335C5F774195F2C3431BF93BF909@SA1PR21MB1335.namprd21.prod.outlook.com>
        <20230405072801.05bb94ef@meshulam.tesarici.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, 5 Apr 2023 07:32:06 +0200
Petr Tesa=C5=99=C3=ADk <petr@tesarici.cz> wrote:

> On Wed, 5 Apr 2023 05:11:42 +0000
> Dexuan Cui <decui@microsoft.com> wrote:
>=20
> > > From: Petr Tesa=C5=99=C3=ADk <petr@tesarici.cz>
> > > Sent: Tuesday, April 4, 2023 9:40 PM   =20
> > > > > ...
> > > > > Hi Petr, this patch has gone into the mainline:
> > > > > 0eee5ae10256 ("swiotlb: fix slot alignment checks")
> > > > >
> > > > > Somehow it breaks Linux VMs on Hyper-V: a regular VM with
> > > > > swiotlb=3Dforce or a confidential VM (which uses swiotlb) fails t=
o boot.
> > > > > If I revert this patch, everything works fine. =20
> > > >
> > > > The log is pasted below. Looks like the SCSI driver hv_storvsc fail=
s to
> > > > detect the disk capacity:   =20
> > >=20
> > > The first thing I can imagine is that there are in fact no (free) slo=
ts
> > > in the SWIOTLB which match the alignment constraints, so the map
> > > operation fails. However, this would result in a "swiotlb buffer is
> > > full" message in the log, and I can see no such message in the log
> > > excerpt you have posted.
> > >=20
> > > Please, can you check if there are any "swiotlb" messages preceding t=
he
> > > first error message?
> > >=20
> > > Petr T   =20
> >=20
> > There is no "swiotlb buffer is full" error.
> >=20
> > The hv_storvsc driver (drivers/scsi/storvsc_drv.c) calls scsi_dma_map(),
> > which doesn't return -ENOMEM when the failure happens. =20
>=20
> I see...
>=20
> Argh, you're right. This is a braino. The alignment mask is in fact an
> INVERTED mask, i.e. it masks off bits that are not relevant for the
> alignment. The more strict alignment needed the more bits must be set,
> so the individual alignment constraints must be combined with an OR
> instead of an AND.
>=20
> Can you apply the following change and check if it fixes the issue?

Actually, this will not work either. The mask is used to mask off both
high address bits and low address bits (below swiotlb slot granularity).

What should help is this:

diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 5b919ef832b6..c924e53d679e 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -622,8 +622,7 @@ static int swiotlb_do_find_slots(struct device *dev, in=
t area_index,
 	dma_addr_t tbl_dma_addr =3D
 		phys_to_dma_unencrypted(dev, mem->start) & boundary_mask;
 	unsigned long max_slots =3D get_max_slots(boundary_mask);
-	unsigned int iotlb_align_mask =3D
-		dma_get_min_align_mask(dev) & ~(IO_TLB_SIZE - 1);
+	unsigned int iotlb_align_mask;
 	unsigned int nslots =3D nr_slots(alloc_size), stride;
 	unsigned int offset =3D swiotlb_align_offset(dev, orig_addr);
 	unsigned int index, slots_checked, count =3D 0, i;
@@ -639,8 +638,9 @@ static int swiotlb_do_find_slots(struct device *dev, in=
t area_index,
 	 * allocations.
 	 */
 	if (alloc_size >=3D PAGE_SIZE)
-		iotlb_align_mask &=3D PAGE_MASK;
-	iotlb_align_mask &=3D alloc_align_mask;
+		iotlb_align_mask |=3D ~PAGE_MASK;
+	iotlb_align_mask |=3D alloc_align_mask | dma_get_min_align_mask(dev);
+	iotlb_align_mask &=3D ~(IO_TLB_SIZE - 1);
=20
 	/*
 	 * For mappings with an alignment requirement don't bother looping to

Petr T