[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [RESEND] Re: [PATCH RFC 2/2] AMD IOMMU: allow command line overrides for broken IVRS tables



On 8/28/2013 10:43 AM, Jan Beulich wrote:
On 28.08.13 at 16:59, Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx> 
wrote:
The current patch does not handle the case where the "handle" value is
mis-configured in the IVRS.
Right, but I don't think we can handle all possible cases here.

And in any case (not the least because your patch seems to be
incremental to my earlier one) - do you still see any issues with that
one, or does the mail here represent sort of an ack to that earlier
one (in its split form - as indicated earlier, I broke out the actual
bug fixes from the workaround, which really is an enhancement)?
With the previous 2 patches, it would work fine for the case when the IVHD special is missing for an IOAPIC. The code looks fine, but I don't have such systems which I can test on. If it's working for Sander,
then I think it should be sufficient testing.

Acked-by: Suravee Suthikulpanit <suravee.suthikulapanit@xxxxxxx>

However, it is not handling the case (which I have seen a lot) where,
the special->handle is bad, but the special->used_id is ok. For this case,
we should be able to match the used_id to the BDF values given by users.
That's what my patch is trying to handle (but apparently failed... see below).

I have also included the patch which has additional check.  Below is the
output from the patch.

(XEN) AMD-Vi: IVHD Device Entry: type 0x48 id 0 flags 0xd7
(XEN) AMD-Vi: IVHD Special: 0000:00:14.0 variety 0x1 handle 0xff used_id
0xa0
(XEN) AMD-Vi: IVHD Special: Usinging command override value for IO-APIC 0x8, 
bdf=0xa0
(XEN) AMD-Vi: IVHD Device Entry: type 0x48 id 0 flags 0xd7
(XEN) AMD-Vi: IVHD Special: 0000:00:14.0 variety 0x2 handle 0 used_id 0xa0
(XEN) AMD-Vi: IVHD Device Entry: type 0x48 id 0 flags 0
(XEN) AMD-Vi: IVHD Special: 0000:00:00.1 variety 0x1 handle 0xff used_id 0x1
(XEN) AMD-Vi: IVHD Special: Usinging command override value for IO-APIC 0x8, 
bdf=0xa0
(XEN) AMD-Vi: IOMMU 0 Enabled.
(XEN) I/O virtualisation enabled
Which already indicates that you apparently only considered the
single IO-APIC case, because on a multi IO-APIC system ...
Oops, my logic broke when I was cleaning up the code. I'll fix this. I am planning to handle both IOAPICs.

@@ -715,29 +717,43 @@ static u16 __init parse_ivhd_device_special(
            */
           for ( apic = 0; apic < nr_ioapics; apic++ )
           {
-            if ( IO_APIC_ID(apic) != special->handle )
-                continue;
+            apic_id = special->handle;

-            if ( special->handle >= ARRAY_SIZE(ioapic_sbdf) )
+            if ( IO_APIC_ID(apic) != apic_id )
... this would otherwise trigger on all IO-APICs but the one we're
currently supposed to deal with ...
Sorry, logic got messed up during code clean up. I will send out the fix shortly.

Suravee


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.