On 20.05.2024 12:29, Roger Pau Monné wrote: > On Wed, May 15, 2024 at 12:07:50PM +0200, Jan Beulich wrote: >> On 06.05.2024 15:53, Roger Pau Monné wrote: >>> On Mon, May 06, 2024 at 03:20:38PM +0200, Jan Beulich wrote: >>>> On 06.05.2024 14:42, Roger Pau Monné wrote: >>>>> On Thu, Feb 15, 2024 at 11:15:39AM +0100, Jan Beulich wrote: >>>>>> @@ -196,7 +196,7 @@ static int __must_check amd_iommu_setup_ >>>>>> dte->sys_mgt = MASK_EXTR(ivrs_dev->device_flags, >>>>>> ACPI_IVHD_SYSTEM_MGMT); >>>>>> >>>>>> if ( use_ats(pdev, iommu, ivrs_dev) ) >>>>>> - dte->i = ats_enabled; >>>>>> + dte->i = true; >>>>> >>>>> Might be easier to just use: >>>>> >>>>> dte->i = use_ats(pdev, iommu, ivrs_dev); >>>> >>>> I'm hesitant here, as in principle we might be overwriting a "true" by >>>> "false" then. >>> >>> Hm, but that would be fine, what's the point in enabling the IOMMU to >>> reply to ATS requests if ATS is not enabled on the device? >>> >>> IOW: overwriting a "true" with a "false" seem like the correct >>> behavior if it's based on the output of use_ats(). >> >> I don't think so, unless there were flow guarantees excluding the possibility >> of taking this path twice without intermediately disabling the device again. >> Down from here the enabling of ATS is gated on use_ats(). Hence if, in an >> earlier invocation, we enabled ATS (and set dte->i), we wouldn't turn off ATS >> below (there's only code to turn it on), yet with what you suggest we'd clear >> dte->i. > > Please bear with me, I think I'm confused, why would use_ats(), and if > that's the case, don't we want to update dte->i so that it matches the > ATS state?
I'm afraid I can't parse this. Maybe a result of incomplete editing? The topic is complex enough that I don't want to even try to guess what you may have meant to ask ... > Otherwise we would fail to disable IOMMU device address translation > support if ATS was disabled? I think the answer here is "no", but with the above I'm not really sure here, either. Jan
