On 03.11.2021 17:19, Ian Jackson wrote:
> Jan Beulich writes ("Re: [PATCH v2 0/3][4.16] x86/IOMMU: enabled / intremap
> handling"):
>> On 02.11.2021 11:17, Jan Beulich wrote:
>>> On 21.10.2021 11:57, Jan Beulich wrote:
>>>> In the course of reading the response to v1 (patch 1 only) I realized
>>>> that not only that patch needs further adjustment, but that also
>>>> further changes are needed (and there's likely yet more amiss).
>>>>
>>>> 1: x86/IOMMU: mark IOMMU / intremap not in use when ACPI tables are missing
>>>> 2: x86/APIC: avoid iommu_supports_x2apic() on error path
>>>> 3: AMD/IOMMU: iommu_enable vs iommu_intremap
>>>
>>> Ian, while we further discuss / refine patch 3, the first two have the
>>> needed R-b, but will now need you release-ack aiui.
>>
>> Seeing your reply on IRC, here an attempt at a release justification
>> (the patches were ready by Oct 29, but no-one cared to commit them
>> in my absence, so I thought I'd get away without such a write-up):
>>
>> Patch 1 addresses a regression identified by Andrew. The main risk I
>> see here (which has turned up only very recently) is disagreement on
>> patch 3 which imo has an effect also on what patch 1 does, as to the
>> (non-)effects of "iommu=off" on the hypervisor command line. This,
>> however, is not an effect of the patch, but pre-existing behavior.
>> The behavioral change (in this regard) is in patch 3, which is still
>> under discussion.
>
> Thank you. I also went to the list and read the thread there.
>
> Patch 1:
>
> Reviewed-by: Ian Jackson <i...@xenproject.org>
>
>> Patch 2 corrects an (unlikely but not impossible to be taken) error
>> path, supposedly making systems functional again in case they would
>> in fact cause that error path to be taken. The risk looks low to me,
>> given that two function calls with previously assumed to be
>> identical results now get folded into one with the result latched.
>
> This one also:
>
> Release-Acked-by: Ian Jackson <i...@xenproject.org>
Thanks, but your reply leaves me a little confused: Your use of "also"
may mean R-b for both patches but R-a-b only for patch 2. But I could
also find a variety of other interpretations, including that the
first R-b really was meant to be R-a-b (which otherwise I'd need on
top of the R-b anyway). Please clarify.
> I think, from reading the thread, that patch 3 is not targeting 4.16.
Correct. The other related one now targeting 4.16 is the separately
submitted "x86/x2APIC: defer probe until after IOMMU ACPI table
parsing". But I can see reasons for you to prefer to have that
deferred.
Jan
