linux-kernel - Re: [PATCH v10 05/10] iommu/amd: Introduce helper function to update 256-bit DTE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAFULd4Zi-ua=G-JR2JytwkDtghJSaV-Q_dAPFswkKiUB0rX5oA@mail.gmail.com>
Date: Thu, 14 Nov 2024 21:25:40 +0100
From: Uros Bizjak <ubizjak@...il.com>
To: Jason Gunthorpe <jgg@...dia.com>
Cc: Arnd Bergmann <arnd@...db.de>, Suravee Suthikulpanit <suravee.suthikulpanit@....com>, 
	linux-kernel@...r.kernel.org, iommu@...ts.linux.dev, 
	Joerg Roedel <joro@...tes.org>, Robin Murphy <robin.murphy@....com>, vasant.hegde@....com, 
	Linux-Arch <linux-arch@...r.kernel.org>, Kevin Tian <kevin.tian@...el.com>, jon.grimm@....com, 
	santosh.shukla@....com, pandoh@...gle.com, kumaranand@...gle.com
Subject: Re: [PATCH v10 05/10] iommu/amd: Introduce helper function to update
 256-bit DTE

On Thu, Nov 14, 2024 at 5:29 PM Jason Gunthorpe <jgg@...dia.com> wrote:
>
> On Wed, Nov 13, 2024 at 08:50:08PM +0100, Uros Bizjak wrote:
>
> > Then write_dte_upper128() would look like:
> >
> > static void write_dte_upper128(struct dev_table_entry *ptr, struct
> > dev_table_entry *new)
> > {
> >     struct dev_table_entry old = {}; <--- do we need to initialize struct here?
> >
> >     old.data128[1] = ptr->data128[1];
> >
> >     /*
> >      * Preserve DTE_DATA2_INTR_MASK. This needs to be
> >      * done here since it requires to be inside
> >      * spin_lock(&dev_data->dte_lock) context.
> >      */
> >     new->data[2] &= ~DTE_DATA2_INTR_MASK;
> >     new->data[2] |= old.data[2] & DTE_DATA2_INTR_MASK;
> >
> >     iommu_atomic128_set(&ptr->data128[1], new->data128[1]);
> > }
> >
> > and in a similar way implement write_dte_lower128().
>
> That makes sense to me, but also we have drifted really far from the
> purpose of this series and missed this merge window because of this :\
>
> Let's conclude something please

The most important fact here is that data won't change (I was not
aware of that), and that we would like to exercise only the 128bit
atomic write property of the cmpxchg16b. So, we actually don't need a
"lockless" approach, where we would need __READ_ONCE() and cmpxchg
loop. The atomic write can be a simple arch_cmpxchg128(ptr, *ptr,
new), and because *ptr won't change between preload and compare, the
xchg will always succeed.

Uros.