Re: [PATCH v3 01/11] x86/boot: enumerate documentation for the x86 hardware_subarch

[Luis R. Rodriguez]

On Wed, Feb 24, 2016 at 09:32:59AM +0100, Ingo Molnar wrote:
> 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.

I've tried this and so far so good, and I agree and I like how the quirks
are bundled in one place.

> 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.

Sure, but I'll note I was not intending on spreading use of subarch further,
the use of subarch in pnpbios was certainly an overlooked mistake on my part.

There's only one problem with this strategy I can think so far which differs
from my original approach, which is partly why I actually started looking at
this stuff:

  it doesn't help us pro-actively vet each early boot sequence
  thrown at the x86 path well work on all required subarchs

The scope of addressing requirements I'm trying to address are things stuffed
into x86 init path or the kernel's init path from the first entry point down
to perhaps arch specific setup_arch() calls. Now granted we don't get
modifications in this area a lot, but when we do, if folks did not consider
our different requirements (supporting each subarch/entry path) chances are
high we'll crash or have a feature not fixed / not considered a subarch.

Case in point, kasan. Its still busted on Xen and no one seems to care.
Too late. How do we proactively prevent such things ? Granted peer review
should have caught this, but why not also do something proactively ?

The quirks stuff / proactive solution should perhaps be considered orthogonal.
It just so happened that I was able to address some quirks with what I was
doing, and obviously there's a better way for those. The use of
paravirt_enabled() for quirks also happened to reveal the lack of clarity
on semantics between paravirtualized environments / non-PV hypervisors and
late boot PV/hypervisor checks.

If you can think of another proactive strategy let me know.

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

Indeed.

  Luis