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

[Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>]

On 6/27/2013 3:47 AM, Jan Beulich wrote:
> > > > On 27.06.13 at 01:28, Suravee Suthikulanit <suravee.suthikulpanit@xxxxxxx>
> wrote:
> > On 6/26/2013 4:54 AM, Andrew Cooper wrote:
> > > 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@xxxxxxxxxx>
> > > 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.
> > Since the feedback suggesting that cleaning up is probably not
> > necessary, the only thing is probably the use of the word "BUG". Could
> > it be replaced with "Workaround" instead?
> I'd just drop the "BUG:". And I can certainly do so while applying.
> So in cases where you want something trivial changed, you could
> simply give an ack saying under what conditions that ack applies.
>
> Jan
Acked with removing the word "BUG" as suggested by Jan.

Suravee
> > > 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@xxxxxxxxxxxxx
> > > > http://lists.xen.org/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel