Re: RFC: Additions to APIC driver

From: Ingo Molnar
Date: Fri Jun 26 2015 - 05:03:56 EST



* Steffen Persvold <sp@xxxxxxxxxxxxx> wrote:

> Hi,
>
> Weâre preparing our APIC driver (arch/x86/kernel/apic/apic_numachip.c) with
> next-gen hardware support and in that process I have a question on what the
> cleanest approach would be.
>
> Both current generation and next generation chips will share a lot of similar
> code, but some of the core functionality is slightly different (such as the
> address to which you communicate with the APIC ICR to send IPIs, how to derive
> APIC IDs etc.).
>
> The way I see it, we have few alternatives :
>
> 1) Create a new arc/x86/kernel/apic/apic_numachip2.c (and corresponding entry in
> the Makefile) which has a new âstruct apicâ with function pointers to the
> next-gen specific code. The new APIC driver would still only need
> CONFIG_X86_NUMACHIP to be compiled.
>
> 2) Modify the existing apic_numachip.c to recognise the different HW generations
> (trivial) and use function pointers to differentiate the IPI send calls (among
> other things), but use the *same* âstruct apicâ for both (the function pointers
> referenced in âstruct apicâ would need a new indirection level to differentiate
> between hardware revs).
>
> 3) Have two different âstruct apicâ entries in the existing apic_numachip.c
> source file, with separate oem_madt check functions etc. This would only be
> marginally different than 1) as far as implementation and code duplication goes,
> but it would be contained to one C source file and object file (silly question,
> maybe: would the apic_driver enumeration even work if itâs all in the same
> object file?)

So 1) is clearly suboptimal if the code is similar to a level of 90% (which your
description suggests).

So I'd use a combination of 3) and 2): i.e. have two struct apic's, one of which
gets picked during platform init, but try to share functions wherever possible.

If two different versions of functions are only trivially different, then figure
out a way to share the code. If they are materially different (such as APIC ID
calculation would be I suspect), split it into two functions.

If needed you can also put runtime data into struct apic, so that the base address
becomes configurable.

Thanks,

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