lkml.org 
[lkml]   [2016]   [Feb]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3 01/11] x86/boot: enumerate documentation for the x86 hardware_subarch

* Luis R. Rodriguez <mcgrof@kernel.org> wrote:

> > > > +enum x86_hardware_subarch {
> > > > X86_SUBARCH_PC = 0,
> > > > X86_SUBARCH_LGUEST,
> > > > X86_SUBARCH_XEN,
> > >
> > > No, this is really backwards.
> > >
> > > While I agree that we want to get rid of paravirt_enabled(), we _dont_ want to
> > > spread the use of (arguably broken) boot flags like this!
> >
> > I agree that we should not see the spread of boot flags around general x86
> > code, its not my goal to spread it though, the code that uses it here though
> > is *early boot code* (although in retrospect the pnpbios use was a fuckup),
> > and I have some special considerations for early boot code which I think does
> > give merit to it use. But also keep in mind my goal is to rather fold the boot
> > flag so its more just an architectural consideration eventually. More on this
> > below.
>
> It seems I TL;DR suck; all this is a long winded way of asking, can we keep the
> subarch just for EBDA and use the flags for the other things as you noted?

No, even for EBDA we should make the flag EBDA specific, because that really tells
us what it's about.

The EBDA code could not care less about what subarch it's running on - it only
cares about whether it's safe to search for the EBDA signature in the hardware's
low RAM.

So instead of this:

@@ -38,7 +38,7 @@ void __init reserve_ebda_region(void)
* that the paravirt case can handle memory setup
* correctly, without our help.
*/
- if (paravirt_enabled())
+ if (boot_params.hdr.hardware_subarch != X86_SUBARCH_PC)
return;

I'd suggest adding an EBDA search flag, something like this:

if (!x86_platform.legacy.ebda_search)
return;

Note that the 'legacy' intermediate structure can be used to group various
legacies, while making it clear that this is not something modern hardware should
care about.

The x86_platform.legacy.ebda_search flag can then be set up during (very) early
bootup:

if (boot_params.hdr.hardware_subarch == X86_SUBARCH_PC)
x86_platform.legacy.ebda_search = 1;

This might look like an unnecessary indirection but it isn't: beyond the
documentation value it also makes things a lot clearer should we introduce other
legacy flags in x86_platform.legacy.

For hard coded platform quirks I'd suggest we add x86_platform.quirks flags. For
example the F00F hack for Xen could be done via:

x86_platform.quirks.idt_remap = 0;

and would be set like this during early init:

void early_init_platform_quirks(void)
{
x86_platform.legacy.ebda_search = 0;
x86_platform.quirks.idt_remap = 1;

switch (boot_params.hdr.hardware_subarch) {
case X86_SUBARCH_PC:
x86_platform.legacy.ebda_search = 1;
break;
case X86_SUBARCH_XEN:
x86_platform.quirks.idt_remap = 0;
break;
case X86_SUBARCH_LGUEST:
x86_platform.quirks.idt_remap = 0;
break;
}
}

And if also add the legacy RTC flag, it becomes:

void early_init_hardcoded_platform_quirks(void)
{
x86_platform.legacy.ebda_search = 0;
x86_platform.quirks.idt_remap = 1;
x86_platform.legacy.rtc = 1;

switch (boot_params.hdr.hardware_subarch) {
case X86_SUBARCH_PC:
x86_platform.legacy.ebda_search = 1;
break;
case X86_SUBARCH_XEN:
x86_platform.quirks.idt_remap = 0;
x86_platform.legacy.rtc = 0;
break;
case X86_SUBARCH_LGUEST:
x86_platform.quirks.idt_remap = 0;
x86_platform.legacy.rtc = 0;
break;
}
}

Note that both opt-in and opt-out quirks/legacies are possible this way, and note
how cleanly and consistently it's all organized: setup of all hard coded
legacies/quirks is concentrated in a single function, and the actual usage sites
don't know anything about subarchitectures.

Ok? Functionally this approach is equivalent to your series, but it's cleaner, and
it's also easier to maintain in the long run: there's only a single place to look
to check all hard coded legacies/quirks - instead of the code being spread out all
around the x86 code.

Furthermore we should probably move a few other existing legacies to this flag
space as well, to make all this more consistent.

Thanks,

Ingo

\
 
 \ /
  Last update: 2016-02-24 10:21    [W:0.368 / U:0.620 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site