[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3] AMD/intremap: Prevent use of per-device vector maps until irq logic is fixed
On 27/06/13 09:47, 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 I am happy with either of the two suggested tweaks to the wording. ~Andrew > >>> 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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |