Re: [Xen-devel] [PATCH 1/2] AMD IOMMU: also spot missing IO-APIC entries in IVRS table

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 2/6/2013 9:52 AM, Jan Beulich wrote:
> > > > On 06.02.13 at 15:41, Boris Ostrovsky<boris.ostrovsky@xxxxxxxxxx>  wrote:
> > On 2/6/2013 8:12 AM, Jan Beulich wrote:
> >
> > > +    /* Each IO-APIC must have been mentioned in the table. */
> > > +    for ( apic = 0; !error&&   apic<   nr_ioapics; ++apic )
> > > +    {
> > > +        if ( !nr_ioapic_entries[apic] ||
> > > +             ioapic_sbdf[IO_APIC_ID(apic)].pin_setup )
> > > +            continue;
> > > +
> > > +        printk(XENLOG_ERR "IVHD Error: no information for IO-APIC %#x\n",
> > > +               IO_APIC_ID(apic));
> > > +        if ( amd_iommu_perdev_intremap )
> > > +            error = -ENXIO;
> > > +        else
> > > +        {
> > > +            ioapic_sbdf[IO_APIC_ID(apic)].pin_setup = xzalloc_array(
> > > +                unsigned long, BITS_TO_LONGS(nr_ioapic_entries[apic]));
> > > +            if ( !ioapic_sbdf[IO_APIC_ID(apic)].pin_setup )
> > > +            {
> > > +                printk(XENLOG_ERR "IVHD Error: Out of memory\n");
> > > +                error = -ENOMEM;
> > > +            }
> > > +        }
> > > +    }
> > > +
> > >        return error;
> > >    }
> > Don't we end up with ioapic_sbdf[IO_APIC_ID(apic)].bdf/seg being
> > uninitialized? They are usually set in parse_ivhd_device_special(), at
> > the same time pin_setup is allocated, but with IVRS broken in this way
> > we'll never get there, will we?
> Correct. .bdf/.seg being uninitialized is no much of a problem
> when using global intremap tables though.
Since this patch has been tested it clearly must have worked somehow but 
I don't understand where you'd get bdf and seg in 
amd_iommu_ioapic_update_ire() and then manage to find the right IOMMU.
-boris

> And certainly not on
> a system with just a single IOMMU (as was the case on the
> crashing system). Do you see alternatives? Disable the IOMMU
> always, even if not using global remap tables? That could be
> seen as a regression, as at least global remap tables worked fine
> so far on such systems.





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