linux-kernel - RE: [PATCH] iommu/vt-d: Handle volatile descriptor status read

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] [thread-next>] [day] [month] [year] [list]

Date: Mon, 17 Jun 2024 03:04:36 +0000
From: "Tian, Kevin" <kevin.tian@...el.com>
To: Jacob Pan <jacob.jun.pan@...ux.intel.com>, Lu Baolu
	<baolu.lu@...ux.intel.com>, LKML <linux-kernel@...r.kernel.org>,
	"iommu@...ts.linux.dev" <iommu@...ts.linux.dev>
CC: "Liu, Yi L" <yi.l.liu@...el.com>, "Kumar, Sanjay K"
	<sanjay.k.kumar@...el.com>
Subject: RE: [PATCH] iommu/vt-d: Handle volatile descriptor status read

> From: Jacob Pan <jacob.jun.pan@...ux.intel.com>
> Sent: Saturday, June 8, 2024 1:38 AM
> 
> Queued invalidation wait descriptor status is volatile in that IOMMU
> hardware
> writes the data upon completion.
> 
> Use READ_ONCE() to prevent compiler optimizations which ensures memory
> reads every time. As a side effect, READ_ONCE() also enforces strict types
> and
> may add an extra instruction. But it should not have negative
> performance impact since we use cpu_relax anyway and the extra time(by
> adding an instruction) may allow IOMMU HW request cacheline ownership
> easier.

I didn't get the meaning of the last sentence.

> 
> e.g. gcc 12.3
> BEFORE:
> 	81 38 ad de 00 00       cmpl   $0x2,(%rax)
> 
> AFTER (with READ_ONCE())
>     772f:       8b 00                   mov    (%rax),%eax
>     7731:       3d ad de 00 00          cmp    $0x2,%eax //status data is 32 bit
> 
> Signed-off-by: Jacob Pan <jacob.jun.pan@...ux.intel.com>

Do we need a fix tag here?

otherwise looks good to me:

Reviewed-by: Kevin Tian <kevin.tian@...el.com>