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

[Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>]

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