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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.06.13 at 10:55, George Dunlap <george.dunlap@xxxxxxxxxxxxx> wrote:
> On 15/06/13 02:13, Suravee Suthikulanit wrote:
>> On 6/10/2013 7:25 AM, Jan Beulich wrote:
>>>>>> On 04.06.13 at 18:38, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> 
>>>>>> 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>
>>> Suravee, Jacob,
>>>
>>> no opinion on this at all? I've been talked into considering this
>>> acceptable
>>
>> Sorry for late reply, and for having missed this conversation previously.
>>
>> If we have to go with this solution temporary until we have the 
>> permanent fix.
>> I think that is okay with me.  Although, would you mind pointing out 
>> the affect
>> of having "per-device" vs. "global" irq vector map?  I am not quite 
>> familiar
>> with the differences.
>>
>>> (with a small coding style fixup, and with the question on
>>> the usefulness of the final warning message - imo redundant with the
>>> immediately preceding message that is being left untouched)
>>
>> I also think the messages are quite confusing.  Actually, now that we 
>> can have
>> irq vector map and intremap map with different mode, we should be more 
>> explicit
>> in the message.
>>
>> Also, the message "Not overriding irq_vector_map setting" is confusing 
>> to me.
>>
>> Would you mind considering the attached patch?  Here is the sample output
>>
>> (XEN) AMD-Vi: IOMMU 0 Enabled.
>> (XEN) AMD-Vi BUG: per-device vector map logic is broken.  Using 
>> per-device-global maps instead until a fix is found
> 
> At the very least it can't say BUG -- that needs to be reserved for 
> things that actually cause the host to crash (a la BUG_ON()).

That was worded this way in Andrew's original version of the patch
too, and I had also already noted that this wording is too strong.

Jan


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