Re: [PATCH v2 1/6][4.16?] x86/x2APIC: defer probe until after IOMMU ACPI table parsing

[Jan Beulich <jbeulich@suse.com>]

On 08.11.2021 10:36, Roger Pau Monné wrote:
> On Mon, Nov 08, 2021 at 08:40:59AM +0100, Jan Beulich wrote:
>> On 05.11.2021 16:38, Roger Pau Monné wrote:
>>> On Fri, Nov 05, 2021 at 01:32:18PM +0100, Jan Beulich wrote:
>>>> --- a/xen/arch/x86/setup.c
>>>> +++ b/xen/arch/x86/setup.c
>>>> @@ -1699,6 +1699,13 @@ void __init noreturn __start_xen(unsigne
>>>>  
>>>>      dmi_scan_machine();
>>>>  
>>>> +    /*
>>>> +     * IOMMU-related ACPI table parsing has to happen before APIC probing, for
>>>> +     * check_x2apic_preenabled() to be able to observe respective findings, in
>>>> +     * particular iommu_intremap having got turned off.
>>>> +     */
>>>> +    acpi_iommu_init();
>>>
>>> If we pull this out I think we should add a check for acpi_disabled
>>> and if set turn off iommu_intremap and iommu_enable?
>>
>> Hmm, I should have added a note regarding this. If we want to exactly
>> retain prior behavior, acpi_ht would also need checking. Yet that has
>> gone wrong long ago: We parse way too many tables when acpi_disabled
>> && acpi_ht, and hence while correct wrt to prior behavior I'd consider
>> it wrong to (re)add a "!acpi_ht" check.
>>
>> As a result I'm of the opinion that checking acpi_disabled here also
>> isn't necessarily appropriate, and instead IOMMU disabling would
>> better be solely under the control of "iommu=".
> 
> I haven't looked very deeply, but will the acpi helpers work correctly
> in that case? As acpi_boot_table_init will be short-circuited if
>  `acpi_disabled && !acpi_ht`.

Oh, that's a good point you make.

Jan