Messages in this thread | | | Subject | Re: [PATCH 7/7] iommu: Add iommu_domain::domain_ops | From | Lu Baolu <> | Date | Tue, 25 Jan 2022 12:42:50 +0800 |
| |
On 1/24/22 5:37 PM, Christoph Hellwig wrote: > On Mon, Jan 24, 2022 at 03:11:02PM +0800, Lu Baolu wrote: >> Add a domain specific callback set, domain_ops, for vendor iommu driver >> to provide domain specific operations. Move domain-specific callbacks >> from iommu_ops to the domain_ops and hook them when a domain is allocated. > > Ah, that's what I meant earlier. Perfect! > > Nit: I don't think vendor is the right term here. > > Maybe something like: > > iommut: split struct iommu_ops > > Move the domain specific operations out of struct iommu_ops into a new > structure that only has domain specific operations. This solves > the problem of needing to know if the method vector for a given > operation needs to be retreived from the device or th domain.
Sure. Will use above description.
> >> +struct domain_ops { > > This needs to keep an iommu in the name, i.e. iommu_domain_ops.
Sure.
> >> + phys_addr_t (*iova_to_phys)(struct iommu_domain *domain, dma_addr_t iova); > > Overly long line.
./scripts/checkpatch.pl --strict *.patch
didn't give me a WARN or CHECK. I will make it short anyway.
> >> +static inline void iommu_domain_set_ops(struct iommu_domain *domain, >> + const struct domain_ops *ops) >> +{ >> + domain->ops = ops; >> +} > > Do we really need this helper?
Unnecessary. I can set the pointer directly in the drivers.
> >> +static const struct domain_ops amd_domain_ops; > > Can we avoid these forward declarations or would that cause too much > churn? >
I don't like this either. But it's common to put the ops at the bottom of the file in almost all iommu drivers.
Best regards, baolu
| |