[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Mon, 15 Nov 2021 16:07:55 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=MpUxpJBLLE5ifRi6X7EwQyAJ/YnWuq1e6oq4dYev0pw=; b=oB2WX9ad4RE4lZW4waZoU9iQWViuhFw+qnDXvDwluECHEoaBi+QvVdjD8GyDxcwCM71Jfm44A+hAaJEJnq3uocagG/T/Az3PHtj3Z7yGdBVqNXqromMkLvlXzWaK+IPiciYnvoKxo+dDroGQ/hR4W05oGCYeaBaaWt6aSTq/oTofnGp9rREFxgtq51JQCo2oMEtgC5C4R71JOoPuw4kPur0zG3WKMiZgY2Zndk6NmAMcjMSvxeIUiQ0D9+F07Z4YhOMJck2lwqMXyvU8HGv/8dRVmPq+gCdDJmQtGRZNJr2aPi92Ro3OiQ6iCwSlUMkeC14niQHxjIsJwvi5BvcdOw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=PTUz/egjEhfTD80SZZvvVv/GoxYbTwYcSmW/hwOFUYjYhN4QDi8Wo4aQBDDwYbycBvDoQgmcvDIc6KSUxOD9SzMiW60vjtJxG1vNpSEjlBetlWk8IP1a3fJm6SYC6FyMc8CxTLFut0qo7fDMMB1l0ce+0eSjNjEnNdpAWduPh870uDYS7gsT0mPwcKDYvZQm5rIkPoc8VpN/URs7VlxoE4AtnZN+/EIl9P7bnB74AsEicMxakJjo3mGlzbehWR8gx4bIV7215ByiGit4oxhf9Cgqbr8wXU+x6pJNx0PjzSRoxmrDiK7G/a80XGq+8LlqXmXkgb1HWFp8JiRkXo4Vsg==
  • Authentication-results: esa6.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Paul Durrant <paul@xxxxxxx>
  • Delivery-date: Mon, 15 Nov 2021 15:08:42 +0000
  • Ironport-data: A9a23:GsLTIqs04xpV88TKRRH6X27vKufnVKVZMUV32f8akzHdYApBsoF/q tZmKWCCPa2KYDHyKdtzPIuz/R8Bv5+HyYRlHlZopClhHyIT+JbJXdiXEBz9bniYRiHhoOOLz Cm8hv3odp1coqr0/0/1WlTZQP0VOZigHtIQMsadUsxKbVIiGHhJZS5LwbZj29cx2IXhWmthh PupyyHhEA79s9JLGjp8B5Kr8HuDa9yr5Vv0FnRnDRx6lAe2e0s9VfrzFonoR5fMeaFGH/bSe gr25OrRElU1XfsaIojNfr7TKiXmS1NJVOSEoiI+t6OK2nCuqsGuu0qS2TV1hUp/0l20c95NJ NpllMeoEV1wHYP1iO0lXyIAIXpSPaYc0eqSSZS/mZT7I0zudnLtx7NlDV0sPJ1e8eFyaY1M3 aVGcnZXNEnF3r/ohuLgIgVvrp1LwM3DJoQQt2sm1TjEJf0nXYrCU+PB4towMDIY258eRaqHP pVxhTxHQirlIANCCggrC5Mmht/xikbSKDFbgQfAzUYwyzeKl1EguFT3C/LUZd6iVchThlyfp G/N4yL+GB5yHMSW1D6t4n+qwOjVkkvTR4Y6BLC+sPlwjzW7xGYeFRkXXluTuuSihwi1XNc3A 1wZ/G8ioLY/8GSvT8LhRFuorXicpBkeVtFMVeog52mwJrH8uljDQDJeF3gYNYJg5JReqSEWO kGhvojxXWY2k5OsWXul+ozMn2ieOSNPBDpXDcMbdjct797mqYA1qxvASNd/DaK45uHI9SHML yOi93Zn2ehK5SIf/+DipA2c3WrwznTcZldtvl2/Y46z0u9uiGdJjaSM4EOT0/tPJZ3xorKp7 CldwJj2AAzj4PiweM2xrAclQenBCxWtamS0bbtT834JrWvFxpJbVdoMiAyS3W8wWir+RRfnY VXIpSRa74JJMX2hYMdfOtzqVZVzkfa9SYq5Cpg4i+aihLArLmdrGwk0OiatM53FyhBwwcnTx 7/GGSpTMZrqIfs+l2fnLwvs+bQq2jo/1QvuqWPTlHyaPU6lTCfNE98taQLWBshgtf/siFiFo r53aprRoz0CAbKWX8Ui2dNKRbz8BSNgXs6eRg0+XrPrHzeK70l9UaKMmu14JNQ+90mX/8+Rl kyAtoZj4AOXrVXMKBmQa2Alb7XqXJ1lqmk8MzBqNlGts0XPq670hEvGX5doL7Qh6sJ5yvt4E 6sMd8maW6wdQTXb4TUNK5L6qdU6JhisgAuPOQujYSQ+IME8F1CYpIe8c1u97jQKAwq2qdA6/ ++q2DTETMdRXA9lFsvXNq6ilgvjoXgHletudELUOd0PKl70+Y1nJnWp3P86Ks0BMzvZwT6e2 1rECBsUv7CV8YQ07MPIleaPqILwS7lyGU9THm/667eqNHaFojr/kNEYCOvRJGLTTmL5/qmmd N559fCkPa1VhktOvqp9D61vkfA06ezwquII1Q9jBnjKMQimU+syPnmc0MBTnaRR3bsF6xCuU 0eC99QGa7WEPMTpTAwYKAY/N7nR0PgVnn/Z7OgvIVW87yhypeLVXUJXNhiKqSpcMLoqb991n bZ/4JYbu16llx4nEtealSQFpW2DI0sJX7gjqpxHUpTgjRAmyw0abJHRYsMsDEpjtzmY3pEWH wKp
  • Ironport-hdrordr: A9a23:rKa0uar3C7MQTJ7r8SgUHNAaV5uxL9V00zEX/kB9WHVpm5Oj+P xGzc526farslsssREb+OxpOMG7MBThHLpOkPMs1NCZLXTbUQqTXfpfBO7ZrQEIdBeOlNK1uZ 0QFpSWTeeAcWSS7vyKkTVQcexQueVvmZrA7Yy1rwYPPHFXguNbnn9E426gYzNLrWJ9dPwE/f Snl656T23KQwVpUi33PAhOY8Hz4/nw0L72ax8PABAqrCGIkDOT8bb/VzyVxA0XXT9jyaortT GtqX202oyT99WAjjPM3W7a6Jpb3PPn19t4HcSJzuwYMC/lhAqEbJloH5eCoDc2iuey70tCqq iAnz4Qe+BIr1/BdGC8phXgnyHmzTYV8nfnjWSVhHPyyPaJDQ4SOo5kv8Z0YxHZ400vsJVXy6 RQxV+UsJJREFfpgDn9z8KgbWAqqmOE5V4Z1cIDhX1WVoUTLJVLq5YEwU9TGJAcWArn9YEcFv V0Bs203ocZTbqjVQGbgoBT+q3vYpxqdS32B3Tq+/blnAS+pUoJj3fxn6ck7zM9HJFUcegz2w 2LCNUuqFh0dL5lUUtKPpZ3fSKGMB2/ffvyChPmHb3GLtBOB5ufke+93F0KjNvaDKDgiqFC3q j8bA==
  • Ironport-sdr: 5Wh8LWvqGBWYTFWVXjYpzEh7lG5I6RmYwtTGuqM4hbZlVrbJuE2W6DbvEZRmEXjKYy0O0ZvBuX WwddWhdO5fmfDS7DchgAaE5+jNr5W+x2Y20C99T09eiOdvTz7xXaHDHMS85OwoCnFS7T0fw+H9 aND2jD4Ehp/meVdgCJhArTOsMeK1MW8x/8PoXLlTLaf+4bcOcDTdRQAqFSnkTWk8Xpmm/Z0nR7 pN93T9pN8/P9Ir+mq2hd+YcQEUCIXrvNIDjq8Ke8EhloHZFJpo/kTHqwGWhawWkwYpEuNmIdR2 iLvwUYPVcL+UgZWv2bXc6V93
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Mon, Nov 15, 2021 at 03:31:39PM +0100, Jan Beulich wrote:
> While commit 46c4061cd2bf ("x86/IOMMU: mark IOMMU / intremap not in use
> when ACPI tables are missing") deals with apic_x2apic_probe() as called
> from x2apic_bsp_setup(), the check_x2apic_preenabled() path is similarly
> affected: The call needs to occur after acpi_iommu_init(), such that
> iommu_intremap getting disabled there can be properly taken into account
> by apic_x2apic_probe().
> 
> Note that, for the time being (further cleanup patches following),
> reversing the order of the calls to generic_apic_probe() and
> acpi_boot_init() is not an option:
> - acpi_process_madt() calls clustered_apic_check() and hence relies on
>   genapic to have got filled before,
> - generic_bigsmp_probe() (called from acpi_process_madt()) needs to
>   occur after generic_apic_probe(),
> - acpi_parse_madt() (called from acpi_process_madt()) calls
>   acpi_madt_oem_check(), which wants to be after generic_apic_probe().
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Just one comment below.

> ---
> Based on code inspection only - I have no affected system and hence no
> way to actually test the case.
> ---
> v2.2: Move generic_apic_probe() again, but only past acpi_iommu_init().
> v2.1: Respect acpi_disabled in acpi_iommu_init().
> v2: Don't move generic_apic_probe() invocation, instead pull out
>     acpi_iommu_init() from acpi_boot_init().
> ---
> 4.16: While investigating the issue addressed by the referenced commit,
>       a variant of that problem was identified when firmware pre-enables
>       x2APIC mode. Whether that's something sane firmware would do when
>       at the same time IOMMU(s) is/are disabled is unclear, so this may
>       be a purely academical consideration. Working around the problem
>       also ought to be as simple as passing "iommu=no-intremap" on the
>       command line. Considering the fragility of the code (as further
>       demonstrated by v1 having been completely wrong), it may therefore
>       be advisable to defer this change until after branching.
>       Nevertheless it will then be a backporting candidate, so
>       considering to take it right away can't simply be put off.
> 
> --- a/xen/arch/x86/acpi/boot.c
> +++ b/xen/arch/x86/acpi/boot.c
> @@ -757,8 +757,6 @@ int __init acpi_boot_init(void)
>  
>       acpi_mmcfg_init();
>  
> -     acpi_iommu_init();
> -
>       erst_init();
>  
>       acpi_hest_init();
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1700,15 +1700,30 @@ void __init noreturn __start_xen(unsigne
>  
>      dmi_scan_machine();
>  
> -    generic_apic_probe();
> -
>      mmio_ro_ranges = rangeset_new(NULL, "r/o mmio ranges",
>                                    RANGESETF_prettyprint_hex);
>  
>      xsm_multiboot_init(module_map, mbi);
>  
> +    /*
> +     * IOMMU-related ACPI table parsing may require some of the system 
> domains
> +     * to be usable.

I would be a bit more specific and likely add "...to be usable in
order to hide IOMMU PCI devices.".

Thanks, Roger.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.