Re: [PATCH v3] AMD/intremap: Prevent use of per-device vector maps until irq logic is fixed

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 04/06/13 17:38, Andrew Cooper wrote:
> XSA-36 changed the default vector map mode from global to per-device.  This is
> because a global vector map does not prevent one PCI device from impersonating
> another and launching a DoS on the system.
>
> However, the per-device vector map logic is broken for devices with multiple
> MSI-X vectors, which can either result in a failed ASSERT() or misprogramming
> of a guests interrupt remapping tables.  The core problem is not trivial to
> fix.
>
> In an effort to get AMD systems back to a non-regressed state, introduce a new
> type of vector map called per-device-global.  This uses per-device vector maps
> in the IOMMU, but uses a single used_vector map for the core IRQ logic.
>
> This patch is intended to be removed as soon as the per-device logic is fixed
> correctly.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Can we get a decision on this?  The 4.3 is looming and multi MSI-X PCI
functions are *still* broken on AMD systems, in all stable versions of
Xen, regressed by XSA-36.

>From my understanding of the points so far, we have agreed that this
patch is suitable for 4.3 and previous, with Jan's multi-MSI series
being the correct solution going forwards into 4.4.

The only query at the moment is for the exact wording, which has had no
attention for a week.

~Andrew

>
> ---
> Changes since v2:
>  * Do not override command line.
>  * reuse OPT_IRQ_VECTOR_MAP_GLOBAL.
>
> Changes since v1:
>  * Correct stupid mistake in commit message, making it confusing to read
>
> diff -r 2d37d2d652a8 -r a017d74f346d xen/drivers/passthrough/amd/pci_amd_iommu.c
> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> @@ -223,8 +223,19 @@ int __init amd_iov_detect(void)
>      {
>          if ( amd_iommu_perdev_intremap )
>          {
> -            printk("AMD-Vi: Enabling per-device vector maps\n");
> -            opt_irq_vector_map = OPT_IRQ_VECTOR_MAP_PERDEV;
> +            /* Per-device vector map logic is broken for devices with multiple
> +             * MSI-X interrupts (and would also be for multiple MSI, if Xen
> +             * supported it).
> +             *
> +             * Until this is fixed, use global vector tables as far as the irq
> +             * logic is concerned to avoid the buggy behaviour of per-device
> +             * maps in map_domain_pirq(), and use per-device tables as far as
> +             * intremap code is concerned to avoid the security issue.
> +             */
> +            printk(XENLOG_WARNING "AMD-Vi BUG: per-device vector map logic is broken.  "
> +                   "Using per-device-global maps instead until a fix is found\n");
> +
> +            opt_irq_vector_map = OPT_IRQ_VECTOR_MAP_GLOBAL;
>          }
>          else
>          {
> @@ -235,6 +246,12 @@ int __init amd_iov_detect(void)
>      else
>      {
>          printk("AMD-Vi: Not overriding irq_vector_map setting\n");
> +
> +        if ( opt_irq_vector_map != OPT_IRQ_VECTOR_MAP_GLOBAL )
> +        {
> +            printk(XENLOG_WARNING "AMD-Vi BUG: per-device vector map logic is broken.  "
> +                   "Use irq_vector_map=global to work around.");
> +        }
>      }
>      if ( !amd_iommu_perdev_intremap )
>          printk(XENLOG_WARNING "AMD-Vi: Using global interrupt remap table is not recommended (see XSA-36)!\n");
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel