Re: [PATCH] iommu: Print default strict or lazy mode at init time

[Robin Murphy]

On 2021-06-01 16:50, John Garry wrote:
> On 01/06/2021 10:09, Robin Murphy wrote:
> > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > > index 808ab70d5df5..f25fae62f077 100644
> > > --- a/drivers/iommu/iommu.c
> > > +++ b/drivers/iommu/iommu.c
> > > @@ -138,6 +138,11 @@ static int __init iommu_subsys_init(void)
> > >           (iommu_cmd_line & IOMMU_CMD_LINE_DMA_API) ?
> > >               "(set via kernel command line)" : "");
> > > +    pr_info("Default DMA domain mode: %s %s\n",
> >
> > Nit: I think this might be a little unclear for end-users - *I'm* not 
> > even sure whether "Default" here is meant to refer to the mode setting 
> > itself or to default domains (of DMA type). Maybe something like "DMA 
> > domain TLB invalidation policy"? Certainly it seems like a good idea 
> > to explicitly mention invalidation to correlate with the documentation 
> > of the "iommu.strict" parameter.
> >
> > Ack to the general idea though.
>
> ok, so I'll go with this:
>
> pr_info("DMA domain default TLB invalidation policy: %s mode %s\n",
>                iommu_dma_strict ? "strict" : "lazy",
>                 (iommu_cmd_line & IOMMU_CMD_LINE_STRICT) ?
>                         "(set via kernel command line)" : "");
>
> I think it's worth mentioning "default" somewhere, as not all IOMMUs or 
> devices will use lazy mode even if it's default.

But that's part of what I think is misleading - I boot and see that the 
default is something, so I reboot with iommu.strict to explicitly set it 
the other way, but now that's the default... huh?

The way I see it, we're saying what the current IOMMU API policy is - 
the value of iommu_dma_strict at any given time is fact - but we're not 
necessarily saying how widely that policy is enforced. We similarly 
report the type for default domains from global policy even though that 
may also be overridden per-group by drivers and/or userspace later; we 
don't say it's the *default* default domain type.

However, having now debugged the AMD issue from another thread, I think 
doing this at subsys_initcall is in fact going to be too early to be 
meaningful, since it ignores drivers' ability to change the global policy :(

Robin.