Hi Julien, Michal On 28.08.25 00:42, Julien Grall wrote: > Hi Michal, > > On 26/08/2025 12:44, Orzel, Michal wrote: >> >> >> On 26/08/2025 12:48, Julien Grall wrote: >>> Hi, >>> >>> On 26/08/2025 10:47, Dmytro Firsov wrote: >>>> On 22.08.25 11:12, Orzel, Michal wrote: >>>> >>>> There are already a few places advertising the SMMU coherency: >>>> 1) smmu->features >>>> 2) d->iommu->features >>>> 3) platform_features >>>> >>>> All of them are better places than queue struct (that as you >>>> pointed out is not >>>> specific to coherency). I'd suggest maybe to use 3) and removing >>>> ro_after_init >>>> if you don't have access to 1) and 2). All in all, providing yet >>>> another place >>>> for coherency flag seems a bit too much. >>> > > >>>> First of all, thank you very much for review! I will consider using >>>> "platform_features" in next patch version, it looks more >>>> appropriate and >>>> should be available within the whole driver. Also, I believe >>>> "ro_after_init" >>>> is also OK, since I have no need to change it (only check if cache >>>> maintenance should be performed). >>> >>> I have to disagree with using "platform_features". Flushing the >>> queue is >>> a per-SMMU decision. But looking at the code, I think passing the SMMU >>> to the caller would look wrong (what if you mistakenly pass the wrong >>> SMMU?). So I think a boolean per queue is the right appraoch. >> For my own understanding: don't we consider SMMU coherency globally, >> not per >> SMMU (hence why I suggested re-using the global flag)? > > We set the coherency> feature in the flags if all SMMUs support it. > Do we want to diverge now and do >> the flushing per SMMU? > > The two contexts are quite different. > > We need the global flag when cleaning the stage-2 page-tables because > we don't always know at boot which SMMUs will be used for the devices > attached (think PCI hotplug). > > In the context of this patch, we know the queue is only used by a > given SMMU. So i tis better to take this decision per-SMMU/queue to > reduce the number of cache flush. This will be particularly important > for the 2-stage SMMU work because there will be a lot of commands sent > on behalf of the guest (every TLB flushes will be command). > > Cheers, Thank you for the review and comments.
I wanted to follow up on the discussion. Michal, I still need to clarify: do you agree with the per-queue flag approach? If so, I will simplify the header comment to "Is memory access coherent?", add Julien's RB tag, and post v2. Also, thank you, Mykola, for the testing and TB tag. --- BR, Dmytro.
