On Tue, 2022-11-29 at 16:09 -0400, Jason Gunthorpe wrote: > On Tue, Nov 29, 2022 at 06:41:22PM +0000, Robin Murphy wrote: > > On 2022-11-29 17:33, Jason Gunthorpe wrote: > > > On Mon, Nov 28, 2022 at 09:01:43PM +0000, Robin Murphy wrote: > > > > > > > I'm hardly an advocate for trying to save users from themselves, but I > > > > honestly can't see any justifiable reason for not having sysfs respect > > > > iommu_get_def_domain_type(). > > > > > > We really need to rename this value if it is not actually just an > > > advisory "default" but a functional requirement .. > > > > It represents a required default domain type. As in, the type for the > > device's default domain. Not the default type for a domain. It's the > > iommu_def_domain_type variable that holds the *default* default domain type > > ;) > > I find the name "default domain" incredibly confusing at this point in > time. Yes it definitely confused me as evidenced by this patch. > > I would like to call that the "dma-api domain" - its primary purpose > is to be the domain that the DMA API uses to operate the IOMMU, there > is little "default" about it. This meshes better with our apis talking > about ownership and so forth. > > So, if the op was called > get_dma_api_domain_type() > > It is pretty clear that it is the exact type of domain that should be > created to support the DMA API, which is what I think you have been > describing it is supposed to do? > > And with Lu's series we have the set_platform_dma() (Lu perhaps you > should call this set_platform_dma_api() to re-enforce it is about the > DMA API, not some nebulous DMA thing) > > Which is basically the other way to configure the DMA API for > operation. > > And encapsulating more of the logic to setup and manage the DMA API's > domain into dma-iommu.c would also be helpful to understanding. > > > Which reminds me I should finish that patch undoing my terrible > > ops->default_domain_ops idea, not least because they are misleadingly > > unrelated to default domains... > > :) > > > > It is close to being clear, once we get the last touches of dma-iommu > > > stuff out of the drivers it should be quite clear > > > > Cool, some upheaval of .domain_alloc is next on my hitlist anyway, so that > > might be a good excuse to upheave it a bit more and streamline the type > > stuff along the way. > > Yes, I think so. I want to tidy things a bit so adding this "user > space" domain concept is a little nicer > > Jason Ok, so it sounds to me like there is going to be quite a bit of change in this area. Thus I'm a little unsure however how to proceed here. I think especially with the proposed multiple queue types in this series it makes sense for an IOMMU driver to express its preference of a particular domain type if it supports multiple which clearly isn't what iommu_get_def_domain_type() is currently supposed to do. Looking at the current code I don't see a trivial way to integrate this especially with a lot of reworks going on. At the moment the preference for IOMMU_DOMAIN_DMA_FQ over IOMMU_DOMAIN_DMA during initial boot is hard coded in iommu_subsys_init() before we even know what IOMMU driver will be used. so can't really access its ops there. On the other hand this decision may still be rolled back if iommu_dma_init_fq() fails in iommu_dma_init_domain() so maybe it would make sense to assume a DMA domain type of IOMMU_DOMAIN_DMA until that point and only then prefer IOMMU_DOMAIN_DMA_FQ or something else if the driver has a preference. Maybe it would also make sense not to mix the type of flush queue used with the domain and maybe the queue type could be passed in via iommu_setup_dma_ops() -> iommu_dma_init_domain() though I do remember that there is also a planned rework for that. Any suggestions? Thanks, Niklas
