Messages in this thread | | | Subject | Re: [PATCH v13 4/6] iommu/vt-d: Add support for IOMMU default DMA mode build options | From | Lu Baolu <> | Date | Fri, 18 Jun 2021 09:46:24 +0800 |
| |
Hi John,
On 6/17/21 4:00 PM, John Garry wrote: > On 17/06/2021 08:32, Lu Baolu wrote: >> On 6/16/21 7:03 PM, John Garry wrote: >>> @@ -4382,9 +4380,9 @@ int __init intel_iommu_init(void) >>> * is likely to be much lower than the overhead of >>> synchronizing >>> * the virtual and physical IOMMU page-tables. >>> */ >>> - if (!intel_iommu_strict && cap_caching_mode(iommu->cap)) { >>> - pr_warn("IOMMU batching is disabled due to >>> virtualization"); >>> - intel_iommu_strict = 1; >>> + if (cap_caching_mode(iommu->cap)) { >>> + pr_warn("IOMMU batching disallowed due to >>> virtualization\n"); >>> + iommu_set_dma_strict(true); >> >> With this change, VM guest will always show this warning. > > Would they have got it before also normally? > > I mean, default is intel_iommu_strict=0, so if > cap_caching_mode(iommu->cap) is true and intel_iommu_strict not set to 1 > elsewhere previously, then we would get this print.
Yes. You are right.
> >> How about >> removing this message? Users could get the same information through the >> kernel message added by "[PATCH v13 2/6] iommu: Print strict or lazy >> mode at init time". > > I think that the print from 2/6 should occur before this print. > > Regardless I would think that you would still like to be notified of > this change in policy, right? > > However I now realize that the print is in a loop per iommu, so we would > get it per iommu: > > for_each_active_iommu(iommu, drhd) { > /* > * The flush queue implementation does not perform > * page-selective invalidations that are required for efficient > * TLB flushes in virtual environments. The benefit of batching > * is likely to be much lower than the overhead of synchronizing > * the virtual and physical IOMMU page-tables. > */ > if (!intel_iommu_strict && cap_caching_mode(iommu->cap)) { > pr_warn("IOMMU batching is disabled due to virtualization"); > intel_iommu_strict = 1; > } > ... > } > > I need to change that. How about this: > > bool print_warning = false; > > for_each_active_iommu(iommu, drhd) { > /* > * The flush queue implementation does not perform > * page-selective invalidations that are required for efficient > * TLB flushes in virtual environments. The benefit of batching > * is likely to be much lower than the overhead of synchronizing > * the virtual and physical IOMMU page-tables. > */ > if (!print_warning && cap_caching_mode(iommu->cap)) { > pr_warn("IOMMU batching disallowed due to virtualization\n"); > iommu_set_dma_strict(true); > print_warning = true; > } > ... > } > > or use pr_warn_once().
From my p.o.v, pr_xxxx_once() is better.
How about using a pr_info_once()? I don't think it's a warning, it's just a policy choice in VM environment.
Best regards, baolu
| |