[Xen-devel] Re: Comments on Xen bug 1732

[Haitao Shan <maillists.shan@xxxxxxxxx>]

I am wondering whether we can trust msi->table_base for the moment?

Simply removing the warnings can of course suppress the symptom. But assuming MSIX table at pfn 0 and changing its p2m type are a concern to me. But probably this won't trigger anything bad, since this pfn is seldom used?

Shan Haitao 

2011/1/31 Keir Fraser <keir@xxxxxxx>

If the primary symptom is the large number of call traces, then simply
removing the 'offending' WARN_ON() for 4.1 might be a suitable fix. Even if
the root cause is really elsewhere (but we do not have resources to fix it
in time)?

 -- Keir

On 31/01/2011 10:02, "Haitao Shan" <maillists.shan@xxxxxxxxx> wrote:

> We are heading Chinese New Year, which typically take 1.5 weeks. I am afraid
> we don't have time slot to make a fix for this.
> Probably Keir can make a judgement on the impact of this bug? And decide
> whether a fix before 4.1 release is needed.
>
> Shan Haitao 
>
> 2011/1/31 Haitao Shan <maillists.shan@xxxxxxxxx>
>>
>>
>> 2011/1/31 Jan Beulich <JBeulich@xxxxxxxxxx>
>>
>>>>>> On 31.01.11 at 05:54, Haitao Shan <maillists.shan@xxxxxxxxx> wrote:
>>>> Hi, Jan,
>>>>
>>>> As you may already notice the bug 1732, (
>>>> http://bugzilla.xensource.com/bugzilla/show_bug.cgi?id=1732), the culprit
>>>> is
>>>> c/s 22182.
>>>>
>>>> I see the following attached code in your patch. It is pointless to check
>>>> msi->table_base against the value read from physical device if this
>>>> function
>>>> is a virtual function of SR-IOV device. VFs are required to have BARs
>>>> zeroed
>>>> by specifications. And for VFs, unless you can read these values from
>>>> corresponding PF, you will have to trust the "table_base" passed from dom0
>>>> via hypercall. Actually, this parameter is specifically introduced for
>>>> enabling SR-IOV.
>>>
>>> Quite possible, which would perhaps just require removing some
>>> or all of the warnings. In doing so, it must however be avoided to
>>> introduce new ways for things to go bad silently.
>>>
>>>> I am not familiar with this patch and hence its story. But I think it would
>>>> be very simple for you to fix this up?
>>>
>>> Not really, no. I had posted this patch as a draft after there was
>>> no reaction on the part of the original implementers of the MSI and
>>> pass-through code to address the security problem we're dealing
>>> with here (and afaict the issue still wasn't completely addressed,
>>> as I don't recall having seen corresponding adjustments to qemu).
>>> I never had hardware to test this with, and hence had to rely on
>>> Yunhong's testing and ack-ing of the patch.
>>>
>>>> BTW: I vaguely recall that MSI-X table base might not be the first page of
>>>> the corresponding BAR register.
>>>
>>> While I agree that the code is lacking the use of
>>> msix_table_offset_reg(), I would question what else would be
>>> in the range supplied by the BAR, as the specification allows
>>> only MSI-X table and PBA to share a BAR.
>>
>> This is what I copied from PCI spec 3.0. I don't see that the spec only
>> allows the two to be shared.
>> -----------------------------PCI----------
>> To enable system software to map MSI-X structures onto different processor
>> pages for
>> improved access control, it is recommended that a function dedicate separate
>> Base Address
>> registers for the MSI-X Table and MSI-X PBA, or else provide more than the
>> minimum
>> required isolation with address ranges.
>> If dedicated separate Base Address registers is not feasible, it is
>> recommended that a
>> function dedicate a single Base Address register for the MSI-X Table and
>> MSI-X PBA.
>> If a dedicated Base Address register is not feasible, it is recommended that
>> a function isolate
>> the MSI-X structures from the non-MSI-X structures with aligned 8 KB ranges
>> rather than
>> the mandatory aligned 4 KB ranges. 
>> --------------------------spec---------------
>>>
>>> Jan
>>>
>>
>
>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel