linux-kernel - Re: [PATCH v3 11/23] iommufd/viommu: Add IOMMUFD_CMD_VQUEUE

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 for Android: free password hash cracker in your pocket

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

Message-ID: <aBxHgf4llBd7vA5w@nvidia.com>
Date: Wed, 7 May 2025 22:56:17 -0700
From: Nicolin Chen <nicolinc@...dia.com>
To: Vasant Hegde <vasant.hegde@....com>
CC: Jason Gunthorpe <jgg@...dia.com>, <kevin.tian@...el.com>,
	<corbet@....net>, <will@...nel.org>, <bagasdotme@...il.com>,
	<robin.murphy@....com>, <joro@...tes.org>, <thierry.reding@...il.com>,
	<vdumpa@...dia.com>, <jonathanh@...dia.com>, <shuah@...nel.org>,
	<jsnitsel@...hat.com>, <nathan@...nel.org>, <peterz@...radead.org>,
	<yi.l.liu@...el.com>, <mshavit@...gle.com>, <praan@...gle.com>,
	<zhangzekun11@...wei.com>, <iommu@...ts.linux.dev>,
	<linux-doc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>, <linux-tegra@...r.kernel.org>,
	<linux-kselftest@...r.kernel.org>, <patches@...ts.linux.dev>,
	<mochs@...dia.com>, <alok.a.tiwari@...cle.com>, Suravee Suthikulpanit
	<suravee.suthikulpanit@....com>
Subject: Re: [PATCH v3 11/23] iommufd/viommu: Add IOMMUFD_CMD_VQUEUE_ALLOC
 ioctl

On Thu, May 08, 2025 at 10:16:51AM +0530, Vasant Hegde wrote:
> >>   - There is other bit "Completion wait interrupt enable"
> >>     This doesn't related to any buffer. Instead if we configure this for
> >> completion wait command it will generate interrupt.
> > 
> > This sounds like a modify on the VIOMMU object?
> 
> Again in my view its VIOMMU object as it tells HW what to do when it finishes
> completion wait command.

According to the spec:
https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/specifications/48882_IOMMU.pdf

This is for an interrupt from a COMPLETION_WAIT command:
"The COMPLETION_WAIT command allows software to serialize itself
 with IOMMU command processing. The COMPLETION_WAIT command does
 not finish until all older commands issued since a prior
 COMPLETION_WAIT have completely executed."

So, basically it's like the IRQ for CMD_SYNC on ARM. IMHO, this is
very specific to Command Buffer (i.e. a vQUEUE object, and now HW
QUEUE object), though the bit is located in a global IOMMU control
register.

Looking at this paragraph:
"
To restart the IOMMU command processing after the IOMMU halts it,
use the following procedure.
• Wait until CmdBufRun=0b in the IOMMU Status Register
   [MMIO Offset 2020h] so that all commands complete processing as
   the circumstances allow. CmdBufRun must be 0b to modify the
   command buffer registers properly.
• Set CmdBufEn=0b in the IOMMU Control Register [MMIO Offset 0018h].
• As necessary, change the following registers (e.g., to relocate
   the command buffer):
   • the Command Buffer Base Address Register [MMIO Offset 0008h],
   • the Command Buffer Head Pointer Register [MMIO Offset 2000h],
   • the Command Buffer Tail Pointer Register [MMIO Offset 2008h].
• Any or all command buffer entries may be copied from the old
   command buffer to the new and software must set the head and tail
   pointers appropriately.
• Write the IOMMU Control Register [MMIO Offset 0018h] with
   CmdBufEn=1b and ComWaitIntEn as desired
",
the ComWaitIntEn bit is suggested to be set along with the CmdBufEn
bit, i.e. it can be a part of the IOMMU_HW_QUEUE_ALLOC ioctl.

What I am not sure is if the HW allows setting the ComWaitIntEn bit
after CmdBufEn=1, which seems to be unlikely but the spec does not
highlight. If so, this would be an modification to the HW QUEUE, in
which case we could either do an relocation of the HW QUEUE (where
we can set the flag in the 2nd allocation) or add an new option via
IOMMUFD_CMD_OPTION (as Kevin suggested), and I think it should be
a per-HW_QUEUE option since it doesn't affect other type of queues
like Event/PRR Log Buffers.

Similarly, an Event Log Buffer can have an EventIntEn flag; and a
PPR Log Buffer can have an PprIntEn flag too, right?

Thanks
Nicolin