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

Re: [PATCH v8 1/6] AMD/IOMMU: obtain IVHD type to use earlier


  • To: Jan Beulich <jbeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Wed, 20 Oct 2021 00:34:23 +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=0tZwy9BckBcTDPJizTSESAjlQGQEun8HkD0ZtvpOMJE=; b=HYJ+dEalrmL0s6TW/891vYtNjCMPNMdhXUD8QdDnyrjrcyQnpQqf/elx8EgesL8eyZ4BKyYSc4pjvMKEsD0nb0y2p0GU9CnUD01+AfooEK4RikcY7GEwJjIRtlz1cDKLEQbeghl2c29z/A+/RPa6aG4JsOBP8iG9DkPZg4GcDbcudtcgpnSo3gUroMz558vU13rw6OXo4knKhNwLocauVemerQRSyjFwKhNcyQ+P6G74GKN3dePB1JuZrVEdc6cyTxN7rMJhbg7RnHOX2MCffC0N40/RHM9o2VQVLh4S78hmFRht2D6MFmhvhv8HDYSP/YIG39AvnGb5fsARC8vsvQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=S0cNbWPjBniOCcoBgb3UNzXpxk/HnVR+jguERs7BSeC4nJgS7jI4f0bZP9SmAQHeJ6HD0yfkaC80ijCMeu9w7qjfIgMebXB9CJmCuNvajzbdezsU/tTiS16e8k/+Eui6fkzQ6hon6jd+sAN1Reqxe+mKnGxwNkjzF9cAdrrW61qQLNmvujluj0Y9hnswLA60USHVjTT88U7uMLGZykX3AuH/2pIq2ZHqRfJIZe9o08cGgJuhNHx7K5nyixIHBcIgsJqYIdSk251ipmTeIP4tJ8oWnGjONBg7VLKiH46QwoghRFTEnh5iv1XeKkZeZjVCNliDpvgSqE1h2n+XfCwRTQ==
  • Authentication-results: esa4.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Paul Durrant <paul@xxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Delivery-date: Tue, 19 Oct 2021 23:34:45 +0000
  • Ironport-data: A9a23:RkfgZa7kT6GQfq6U8d+ubwxRtMTAchMFZxGqfqrLsTDasY5as4F+v mUaUGvQPKvZYmanctx/bIS3phkH7JPXyINqT1NlqSoyHi5G8cbLO4+Ufxz6V8+wwmwvb67FA +E2MISowBUcFyeEzvuV3zyIQUBUjclkfJKlYAL/En03FVAMpBsJ00o5wrdh298w3LBVPivW0 T/Mi5yHULOa82Yc3lI8s8pvfzs24ZweEBtB1rAPTagjUG32zhH5P7pGTU2FFFPqQ5E8IwKPb 72rIIdVXI/u10xF5tuNyt4Xe6CRK1LYFVDmZnF+A8BOjvXez8CbP2lS2Pc0MC9qZzu1c99Zk 85rrcy2aSgVDoaSxuM4TEFAMCRdBPgTkFPHCSDXXc27ykTHdz3nwul0DVFwNoodkgp1KTgQr 7pCcmlLN03dwbLtqF64YrAEasALBc/nJo4A/FpnyinUF60OSpHfWaTao9Rf2V/cg+gTQqqON pdBN1KDajzkYhN/PGdQEKscu9WxwWDAKDRnlkm88P9fD2/7k1UqjemF3MDuUsyHQ4BZk1iVo krC/n/lGVcKOdqH0z2H/3mwwOjVkkvTWogfCbm5/f5Cm0CIyyoYDxh+fVqko9Gph0imQdVdJ kcIvC00osAa7EW2SvHtUhv+p2SL1iPwQPIJTbd8slvUjPOJvUDJXQDoUwKtdvR9r+kuFToK+ WO5tI23Ahlw6KW2TFGko+L8QSyJBQAZKmoLZCkhRAQD4sX+rIxbsi8jXuqPA4bu0YWrQWCYL ySi6XFk3e1K3JFjO7CTpAif21qRSo71ohnZD+k9dlmu6R9lf8abbois5EmzAR1ofdvBEAfpU JTpnaGjAAEy4XOlyHPlrAYlRujBCxO53Nv02wIH834JrGzFxpJbVdoMiAyS3W8wWir+RRfnY VXIpSRa74JJMX2hYMdfOtzqV5p0nPC6SYy8D5g4i+aihLArKGdrGwk1PSatM53FyhBwwcnTx 7/KGSpTMZrqIfs+l2fnLwvs+bQq2jo/1QvuqWPTlHyaPU6lTCfNE98taQLWBshgtf/siFiFo r53aprRoz0CAbKWX8Ui2dNKRbz8BSNgXs6eRg0+XrPrHzeK70l7Va+Kn+18I9A/90mX/8+Rl kyAtoZj4AOXrVXMKBmQa2Alb7XqXJ1lqmk8MzBqNlGts0XPq672hEvGX5doL7Qh6sJ5yvt4E 6sMd8maW6wdQTXb4TUNK5L6qdU6JhisgAuPOQujYSQ+IME8F1CYpIe8c1u97jQKAwq2qdA6/ ++q2DTETMdRXA9lFsvXNq6ilgvjoXgHletudELUOd0PKl70+Y1nJnWp3P86Ks0BMzvZwT6e2 1rECBsUv7CV8YQ07MPIleaPqILwS7lyGU9THm/667eqNHaFojr/kNEYCOvRJGLTTmL5/qmmd N559fCkPa1VhktOvqp9D61vkfA06ezwquII1Q9jBnjKMQimU+syPnmc0MBTnaRR3bsF6xCuU 0eC99QGa7WEPMTpTAwYKAY/N7nR0PgVnn/Z7OgvIVW87yhypeLVXUJXNhiKqSpcMLoqb991n bZ/4JYbu16llx4nEtealSQFpW2DI0sJX7gjqpxHUpTgjRAmyw0abJHRYsMsDEpjtzmY3pEWH wKp
  • Ironport-hdrordr: A9a23:m1sqbas5rEVpDVk/omfvHF8h7skC/4Mji2hC6mlwRA09TyXGra 6TdaUguiMc1gx8ZJhBo7C90KnpewK5yXcH2/huAV7EZniYhILIFvAf0WKG+Vzd8kLFh5VgPM tbAs5D4ZjLfCVHZKXBkXqF+rQbsaG6GcmT7I+0pRodLnAJGtNdBkVCe2Gm+yVNNXl77PECZe OhD6R81l+dkDgsH76G7i5vZZmzmzSHruOoXTc2QzocrCWehzKh77D3VzCewxclSjtKhZMv63 LMnQDV7riq96jT8G6c60bjq7Bt3PfxwNpKA8KBzuATNzXXkw6tIKBsQaeLsjwZqPymrHwqjN 7PiRE9ONkb0QKeQkiF5T/WnyXw2jcn7HHvjXeenHvYuMT8AAk3DsJQ7LgpOCfx2g4FhpVRwa hL12WWu958FhXbhhnw4NDOSlVDile0iWBKq59Qs1VvFa8lLJNBp40W+01YVL0aGjjh1YwhGO 5ySOnB+fdtd0+AZXyxhBgt/DWVZAV2Iv66eDlEhiTMuAIm2kyRjnFohPD3p01wsa7UEPJ/lr 352qcBrsAEciZZV9MkOA+tKfHHfFAleii8RF56F26XXJ3vC0i93qIf349Fk91CWKZ4gafay6 6xHG+xiwYJCgvT4Iu1rcZ2ziw=
  • Ironport-sdr: P3B8VJk7g9SdpASMt2PyCc0gvC/kF6MsCXXWvKVTpCoJ05wimzfJxsoep093VvbhgsMaUDhLkX 2L3vVEkTFgrHSNieCuwVlwzZaH0x7IV0DIEYIfZvdMv3Vs9djQgSfcnM3NeP8Ekgq+zfZ5ibs2 JiO49384zRUKN3tlLYR9Uocoj1kZtawkbOsm1LNcEzCrox2JJCf/NTxJDjWuCni/J/e2hjOGbW ouNxKk++5zmwXpIZIdebavMVgVYvnbpLpXGksh2xCJkURgYcK3bzCJK3iqDfTBMWR3HXNVdWgT dfam8TI5KLejcjyRgTqOrXLX
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 22/09/2021 15:36, Jan Beulich wrote:
> Doing this in amd_iommu_prepare() is too late for it, in particular, to
> be used in amd_iommu_detect_one_acpi(), as a subsequent change will want
> to do. Moving it immediately ahead of amd_iommu_detect_acpi() is
> (luckily) pretty simple, (pretty importantly) without breaking
> amd_iommu_prepare()'s logic to prevent multiple processing.
>
> This involves moving table checksumming, as
> amd_iommu_get_supported_ivhd_type() ->  get_supported_ivhd_type() will
> now be invoked before amd_iommu_detect_acpi()  -> detect_iommu_acpi(). In
> the course of doing so stop open-coding acpi_tb_checksum(), seeing that
> we have other uses of this originally ACPI-private function elsewhere in
> the tree.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

I'm afraid this breaks booting on Skylake Server.  Yes, really - I
didn't believe the bisection at first either.

>From a bit of debugging, I've found:

(XEN) *** acpi_dmar_init() => -19
(XEN) *** amd_iommu_get_supported_ivhd_type() => -19

So VT-d is disabled in firmware.  Oops, but something we should cope with.

Then we fall into acpi_ivrs_init(), and take the new-in-this-patch early
exit with -ENOENT too.

It turns out ...

> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> @@ -179,9 +179,17 @@ static int __must_check amd_iommu_setup_
>  
>  int __init acpi_ivrs_init(void)
>  {
> +    int rc;
> +
>      if ( !iommu_enable && !iommu_intremap )
>          return 0;
>  
> +    rc = amd_iommu_get_supported_ivhd_type();
> +    if ( rc < 0 )
> +        return rc;
> +    BUG_ON(!rc);
> +    ivhd_type = rc;
> +
>      if ( (amd_iommu_detect_acpi() !=0) || (iommu_found() == 0) )
>      {
>          iommu_intremap = iommu_intremap_off;
>

... we're relying on this path (now skipped) to set iommu_intremap away
from iommu_intremap_full in the "no IOMMU anywhere to be found" case.

This explains why I occasionally during failure get spew about:

(XEN) CPU0: No irq handler for vector 7a (IRQ -2147483648, LAPIC)
[   17.117518] xhci_hcd 0000:00:14.0: Error while assigning device slot ID
[   17.121114] xhci_hcd 0000:00:14.0: Max number of devices this xHCI
host supports is 64.
[   17.125198] usb usb1-port2: couldn't allocate usb_device
[  248.317462] INFO: task kworker/u32:0:7 blocked for more than 120 seconds.

and eventually (gone 400s) get dumped in a dracut shell.

Booting with an explicit iommu=no-intremap, which clobbers
iommu_intremap during cmdline parsing, recovers the system.

This variable controls a whole lot of magic with interrupt handling.  It
should default to 0, not 2, and only become nonzero when an IOMMU is
properly established.  It also shouldn't be serving double duty as "what
the user wants" ahead of determining the system capabilities.

And not to open another can of worms, but our entire way of working
explodes if there are devices on the system not covered by an IOMMU.

~Andrew




 


Rackspace

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