Re: [Xen-devel] [RFC] Add the panic info when disable VT-d

["Li, Liang Z" <liang.z.li@xxxxxxxxx>]

> >>> On 19.01.15 at 10:00, <liang.z.li@xxxxxxxxx> wrote:
> > --- a/xen/arch/x86/apic.c
> > +++ b/xen/arch/x86/apic.c
> > @@ -915,6 +915,11 @@ void __init x2apic_bsp_setup(void)
> >              return;
> >          }
> >          printk("x2APIC: Already enabled by BIOS: Ignoring cmdline
> > disable.\n");
> > +    } else {
> > +        if ( !iommu_enable) {
> > +            panic("x2APIC should be disabled while IOMMU is disabled,"
> > +              "try to set x2apic=0 in cmdline and disable x2apic in BIOS");
> > +        }
> 
> Putting aside the coding style violations (figure braces on their own lines, 
> no
> hard tabs), you tie this to the wrong thing: You need interrupt remapping to
> be enabled, whereas iommu_enable may only mean DMA remapping. And
> I'm afraid you'd run into an ordering problem (iommu_intremap getting set
> to its final value vs. the code above being run) if you tried to correct this.

I don't understand the ordering problem you referred to, the patch is just 
change the panic info, 
and tell the user what they should do to avoid the panic, I didn't change the 
logic of the current code.
Without my patch, the current code will also print the panic info like this:

(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) Couldn't enable IOMMU and iommu=required/force
(XEN) ****************************************
 
It's odd because the user have set ' iommu=0', anyway, it should be fixed.

How about change the panic info to this.
 +    } else {
 +        if ( !iommu_enable) {
 +            panic("x2APIC should be disabled while iommu=0 is set,"
 +                    "try to set x2apic=0 option and disable x2apic in BIOS to 
avoid this");
 +    }

or could you give some suggestion?

Liang


> Jan


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