Received-SPF: pass (google.com: domain of linux-kernel+bounces-187603-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20240522082729.971123-1-ubizjak@gmail.com> <20240522082729.971123-2-ubizjak@gmail.com>
 <c1892ba9-4eae-40fc-b3ab-73d0f82a74ea@linux.intel.com> <CAFULd4Z=YkV1Hbs4DikPBwO-6rg8tfDLGeacSCnfbC02E5y+Cg@mail.gmail.com>
 <b100a3c4-e5c3-41da-8c02-3a4986b49eec@linux.intel.com>
In-Reply-To: <b100a3c4-e5c3-41da-8c02-3a4986b49eec@linux.intel.com>
From: Uros Bizjak <ubizjak@gmail.com>
Date: Thu, 23 May 2024 15:57:40 +0200
Message-ID: <CAFULd4aXBAxPS7hXJ_RKMzZu60yTr7gK1m3K8z0yq1mjYn3dyA@mail.gmail.com>
Subject: Re: [PATCH 2/3] iommu/vt-d: Use try_cmpxchg64() in intel_pasid_get_entry()
To: Baolu Lu <baolu.lu@linux.intel.com>
Cc: iommu@lists.linux.dev, linux-kernel@vger.kernel.org, 
	David Woodhouse <dwmw2@infradead.org>, Joerg Roedel <joro@8bytes.org>, Will Deacon <will@kernel.org>, 
	Robin Murphy <robin.murphy@arm.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, May 23, 2024 at 3:44=E2=80=AFPM Baolu Lu <baolu.lu@linux.intel.com>=
 wrote:
>
> On 2024/5/23 21:34, Uros Bizjak wrote:
> >>> +             if (!try_cmpxchg64(&dir[dir_index].val, &tmp,
> >>> +                                (u64)virt_to_phys(entries) | PASID_P=
TE_PRESENT)) {
> >> Above change will cause a dead loop during boot. It should be
> > No, it is correct as written:
> >
> > if (cmpxchg64(*ptr, 0, new))
> >
> > can be written as:
> >
> > if (cmpxchg64(*ptr, 0, new) !=3D 0)
> >
> > this is equivalent to:
> >
> > tmp =3D 0ULL;
> > if (!try_cmpxchg64(*ptr, &tmp, new))
>
> The return value of both cmpxchg64() and try_cmpxchg64() is the old
> value that was loaded from the memory location, right?

Actually, try_cmpxchg() returns true if successful and false if it fails.

            tmp =3D 0ULL;
            if (!try_cmpxchg64(*ptr, &tmp, new))

The logic in the above snippet can be interpreted as:

if we fail to compare *ptr with 0, then:

            iommu_free_page(entries);
            goto retry;

as intended in the original code.

Thanks,
Uros.