Re: [PATCH v3 11/23] iommufd/viommu: Add IOMMUFD_CMD_VQUEUE_ALLOC ioctl

[Vasant Hegde <vasant.hegde@amd.com>]

Hi Nicolin, Jason,


On 5/8/2025 10:42 PM, Nicolin Chen wrote:
> On Thu, May 08, 2025 at 09:14:56AM -0300, Jason Gunthorpe wrote:
>> On Wed, May 07, 2025 at 10:56:17PM -0700, Nicolin Chen wrote:
>>
>>> 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.
>>
>> The main question is if the control is global to the entire VIOMMU and
>> all its HW QUEUE's or local to a single HW QUEUE.
> 
> Oh, that's right.. I recall AMD only has one Command Buffer,
> but can have dual Event Log Buffers and dual PPR Log Buffers.

Right.

> 
> And the EventIntEn or PprIntEn bit seem to be global for the
> dual buffers..

Yes. But there are other bit to configure dual buffers etc.
(like DualEventLogEn).

> 
>> If it is global then some "modify viommu" operation should be used to
>> change it.
>>
>> If it is local then some "modify hw queu" operation.
>>
>> IOMMUFD_CMD_OPTION could be used with an object_id == VIOMMU as a kind
>> of modify..
> 
> Vasant can confirm. But looks like it should be a vIOMMU
> option.

I think CMD_OPTION will work. So something like below?

if (cmd_option_id == IOMMU_OPTION_VIOMMU && cmd->object_id == viommu_id)
	iommufd_viommu->ops->viommu_options() ?


-Vasant