Message-ID: <m1fxe117n5.fsf@fess.ebiederm.org>
Date:	Mon, 15 Jun 2009 14:58:38 -0700
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Jeremy Fitzhardinge <jeremy@...p.org>
Cc:	Ingo Molnar <mingo@...hat.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>,
	the arch/x86 maintainers <x86@...nel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Xen-devel <xen-devel@...ts.xensource.com>
Subject: Re: [PATCH RFC] x86/acpi: don't ignore I/O APICs just because there's no local APIC

Jeremy Fitzhardinge <jeremy@...p.org> writes:

> On 06/15/09 03:47, Eric W. Biederman wrote:
>> For code reuse and maintainability that is a horrible separation of
>> responsibility.  Things looks similar to the existing cases until you
>> get up close and you discover all of the fundamental assumptions are
>> different so none of the existing code actually works unmodified.
>>    
>
> The I/O APIC code is used exactly as normal, routing from device->pin->vector;
> the whole interrupt emission path is unchanged.

But the algorithms that set it up are now useless.  Which implies most of the
current control logic is now useless.

> The local APIC code doesn't get used at all, because we have a different
> interrupt catcher operating at the irq_chip level.

Sure.  Which pretty much implies that all of io_apic.c and the setting up
of the interrupt controllers has to be replaced as well.

> In terms of system architecture its a reasonable place to make the split; the
> local APICs and I/O APICs are distinct entities which communicate via fairly
> well-defined path.  Xen puts the hypervisor/control domain split at the same
> place.  This is mainly because Xen itself cares about managing CPUs (and
> memory), but doesn't really care about the rest of the system hardware much - it
> leaves that up to the control domain.

Xen doesn't want to deal so it leaves the rest for us.  Making it so neither
kernel has full control of the irqs.  Neither has enough control to flexibly
set things up.

>> The only clean way I can see to handle this is to make xen dom0 it's own
>> weird separate subarch that does all of the table parsing of the
>> firmware tables in completely separate code.  Then once we have something
>> that works factoring out the commonalities into a helper library for
>> better long term maintenance.
>>    
> That seems like overkill.  We can get things working under Xen with 3 changes:

All of the subtle assumptions sound like the come out differently.  Which means
you can very easily start down the road of just reusing small bits and then
you find so many assumptions are different you have to scrap/replace or gunk
up with if (xen) tests.

>   1. make sure I/O APICs are discovered via ACPI properly (or MPTABLE
>      if ACPI isn't present)
>   2. get Xen to allocate a vector and bind that vector to an event channel

The x86 code doesn't allocate vectors.  It allocates a vector that is valid
on a single cpu, or a vector that is valid on all cpus.  Vectors are managed
as a per cpu resource and that is ultimately required to scale the number
of interrupts supported.

If Xen does not manage vectors as a per cpu resource.  That fundamentally
breaks one of the assumptions in the code.

>   3. make sure I/O APIC register writes get to the appropriate I/O APIC
>      in hardware (the normal pin->vector routing)
>
> These points already have fairly well-defined interfaces; there are no subtle
> interactions with the core of the APIC code.
>
> This patch achieves the first of these, in a fairly minimal way.  I'm still
> investigating better ways of achieving 2 & 3.

As I understand Xen so far I still disagree.

>> As it stands right now what Xen wants and what we need to do for normal
>> hardware are radically different, to the point of painful.  Things like
>> irq migration, and cpu hotplug require completely different algorithms.
>>    
>
> The control domain, being a virtual machine, has no access or visibility of
> physical CPUs in the system; all its CPUs are virtual (this is why a "local
> APIC" doesn't make much sense for it, since they're an inherent property of a
> physical CPU, and are not virtualized).
>
> The hypervisor is responsible for all management of physical CPUs, and is
> therefore responsible for physical-CPU things like hotplug and interrupt
> migration.  The kernel doesn't need new algorithms to handle these because it
> simply doesn't know or care about them.

So Xen calls the linux kernel when it wants to migrate an irq from one cpu
to another?

If Xen is calling us back when it wants to migrate an irq from one cpu to another
that implies we need knowledge of irqs.

> As far as the kernel is concerned, the interrupts look like events on event
> channels, like IPIs, timers, etc, and can be handled accordingly.  The irq_chip
> machinery is already in place for them.

If the kernel is responsible for programming the ioapics it definitely cares
about migration, and cpu hotplug.  All of those require reprogramming the
ioapics.

>> I think Xen dom0 has picked the wrong abstraction for this one.  There
>> seems to be no gain and a lot of pain asking the slave kernel to
>> program the ioapics for it, when Xen presents a wildly different
>> abstraction at the cpu level.
>>    
>
> Well, the bulk of the code is already present.  We avoid the local APIC part of
> the kernel completely, by installing a new irq_chip to handle incoming
> interrupts and deliver them into the core interrupt handling accordingly.  The
> control domain patches simply add the ability to bind a hardware-originated
> interrupt to an event channel to be delivered via this mechanism.
>
> And, as Xen contains no device drivers or real hardware knowledge of busses,
> interrupt routing, etc, it falls to the control domain to work out those
> aspects.  The I/O APIC side of the setup is the same as it would be in the
> native case (program a vector corresponding to a pin on an I/O APIC).

There is a lot more going on than that.  You need to know logical vs physical.
How many bits of cpu id you have.  

Which mode you program the ioapics in logical vs physical (and the variants)
of those is directly tied with how many cpus you have, and how that box
is arranged.

>> If what xen was provided looked like an ioapic semantically I would
>> suggest setting cpu_has_apic in a different fashion.
>
> cpu_has_apic has the specific meaning of "this CPU has a local APIC".  It
> doesn't say anything about the presence or absence of I/O APICs; conflating the
> two notions doesn't seem like a good idea.  I'm clearing cpu_has_apic to
> indicate this specific fact: the CPU has no usable local APIC, and there's no
> point pretending it does - but that doesn't mean the I/O APICs aren't
> functional.

If you don't have a local apic you can't receive apic messages.

The Xen ``whacky apic'' model is different.  

>>    We already have two local apic variants after all so a 3rd should not be too nasty.
>>    
> We currently avoid any need to have, or pretend to have, a local APIC by taking
> control of the interrupt delivery subsystem at the irq_chip level.  I don't
> think there's much to be gained by adding a Xen-specific lapic abstraction for
> this case.

To work with Xen dom0 we need to double the number of ioapic routing models.  This
is code that is tricky enough we are still struggling to get clean and sane without
Xen complications that violate some of the most basic assumptions in the code.

>> Except the Xen appears to have totally moved the responsibility around
>> in ways that over constrain the problem by taking, making the
>> existing code useless.
>>    
> I don't think that's true at at all.  The split is along hardware lines, and so
> puts the same constraints on kernel development that the hardware does.

The split is right in the middle of the irq handling.  It is happening in the middle
of deep architecture code.  So far voyager sounds easier to fit into the current
model then Xen.

>> Please put the Xen dom0 insanity somewhere off in a corner where the rest
>> of x86 can ignore it.
>>    
>
> Yep, trying to.

Then please don't hack up the normal x86 irq handling code.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists