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

[George Dunlap <george.dunlap@eu.citrix.com>]

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@citrix.com> 
>>>>> 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>
>> 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()).

  -George