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
