On Thu, Dec 05, 2019 at 10:45:59AM +0100, Jan Beulich wrote:
> On 04.12.2019 17:20, Roger Pau Monne wrote:
> > + switch ( iommu_enable_x2apic() )
> > {
> > + case 0:
> > + iommu_x2apic_enabled = true;
> > + break;
> > +
> > + case -ENXIO: /* ACPI_DMAR_X2APIC_OPT_OUT set */
> > + if ( x2apic_enabled )
> > + panic("IOMMU requests xAPIC mode, but x2APIC already
> > enabled by firmware\n");
> > +
> > printk("Not enabling x2APIC (upon firmware request)\n");
> > - intremap_enabled = false;
> > + iommu_x2apic_enabled = false;
> > goto restore_out;
> > +
> > + default:
> > + printk(XENLOG_ERR "Failed to enable Interrupt Remapping\n");
> > + iommu_x2apic_enabled = false;
> > + break;
>
> I guess you still need to panic() in the failure cases if x2apic_phys
> is false. Unless you can still properly switch from cluster to
> physical mode at this point in time. (If you go the panic() route,
> I'd appreciate if you could avoid making x2apic_phys non-static.)
I don't think Xen needs to check x2apic_phys or panic here, the x2apic
probe that selects phys or cluster mode is done afterwards in
apic_x2apic_probe, which is called after the attempt to enable
interrupt remapping and hence will take this result into account.
> > @@ -938,13 +931,16 @@ void __init x2apic_bsp_setup(void)
> > printk("Switched to APIC driver %s\n", genapic.name);
> >
> > restore_out:
> > - /*
> > - * NB: do not use raw mode when restoring entries if the iommu has been
> > - * enabled during the process, because the entries need to be
> > translated
> > - * and added to the remapping table in that case.
> > - */
> > - restore_IO_APIC_setup(ioapic_entries, !intremap_enabled);
> > - unmask_8259A();
> > + if ( iommu_supports_x2apic() )
>
> Hmm, I first wanted to suggest to use iommu_x2apic_enabled here, but
> I realize the error cases above would then be wrong. Perhaps better
> to leave a brief comment to this effect?
Ack, would you be fine with:
"Note that iommu_x2apic_enabled cannot be used here because if the
IOMMU supports x2APIC but enabling failed Xen wouldn't restore the
IO-APIC and the 8259A state correctly."
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
