xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* Unifying x86_64 / Xen init paths and reading hardware_subarch early
@ 2016-01-15 22:08 Luis R. Rodriguez
  2016-01-15 23:47 ` Andy Lutomirski
  0 siblings, 1 reply; 7+ messages in thread
From: Luis R. Rodriguez @ 2016-01-15 22:08 UTC (permalink / raw)
  To: xen-devel@lists.xensource.com
  Cc: Juergen Gross, Konrad Rzeszutek Wilk,
	linux-kernel@vger.kernel.org, X86 ML, Rusty Russell,
	Jeremy Fitzhardinge, H. Peter Anvin, Luis R. Rodriguez,
	Borislav Petkov, Andy Lutomirski

I will be respinning the generic Linux linker table solution [0] soon
based on hpa's feedback again now that I'm back from vacation. As I do
that though I wanted to highlight a feature I'm throwing into the
linker table solution which I am not sure many have paid close
attention to but I think is important to Xen. I'm making use of the
zero page hardware_subarch to enable us to detect if we're a specific
hypervisor solution *as early as is possible*. This has a few
implications, short term it is designed to provides a proactive
technical solution to bugs such as the cr4 shadow crash (see
5054daa285beaf706f051fbd395dc36c9f0f907f) and ensure that *new* x86
features get a proper Xen implementation proactively *or* at the very
least get annotated as unsupported properly, instead of having them
crash and later finding out. A valid example here is Kasan, which to
this day lacks proper Xen support. In the future, if the generic
linker table solution gets merged, it would mean developers would have
to *think* about if they support Xen or not at development time. It
does this in a not-disruptive way to Xen / x86_64 but most
*importantly* it does not extend pvops! This should avoid issues in
cases of developer / maintainer bandwidth, should some new features be
pushed onto Linux for x86_64 but a respective Xen solution is not
addressed, and that was not caught early in patch review, such as with
Kasan.

[0] https://lkml.kernel.org/r/1450217797-19295-1-git-send-email-mcgrof@do-not-panic.com

Two things I'd like to request a bit of help with and review / consideration:

1) I'd like some advice on a curious problem I've stumbled on. I'd
like to access hardware_subarch super early, and in my review with at
least two x86 folks this *should* work:

diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index c913b7eb5056..9168842821c8 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -141,6 +141,7 @@ static void __init copy_bootdata(char *real_mode_data)

 asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
 {
+ struct boot_params *params = (struct boot_params *)__va(real_mode_data);
  int i;

  /*
@@ -157,6 +158,8 @@ asmlinkage __visible void __init
x86_64_start_kernel(char * real_mode_data)
  (__START_KERNEL & PGDIR_MASK)));
  BUILD_BUG_ON(__fix_to_virt(__end_of_fixed_addresses) <= MODULES_END);

+ boot_params.hdr.hardware_subarch = params->hdr.hardware_subarch;
+
  cr4_init_shadow();

  /* Kill off the identity-map trampoline */

In practice today though this crashes the kernel. One does not need to
try to run Xen to test this, simply applying this change should crash
a bare metal / qemu instance. If you'd like to force a different value
for the subarch you could use this *debug patch* and just use kvm
which would set the subarch to a value not yet assigned on Linux:

http://drvbp1.linux-foundation.org/~mcgrof/patches/2016/01/15/qemu-add-subarch.patch

Simply getting away from he crash is my goal for now. The earliest I
can read the subarch as it stands is right after load_idt() on the x86
init path and I simply have no clue why! I'm told this in theory
should work. But clearly it does not. I tried running qemu with gdb
and I can't get anything sensible out of this so -- I need a bit more
x86 help.

Why do I want this? It would mean we can cover a proactive solution
all the way up to the earliest calls on Linux. Without this the
subarch becomes useful only after load_idt(). Since I'm using the
subarch to build dependency maps early on it also means that the
linker table solution becomes only useful on
x86_64_start_reservations() and not x86_64_start_kernel() which is the
first C Linux entry point for 64-bit. Having the subarch readible as
early as x86_64_start_kernel() means the linker table solution can be
used to proactively prevent issues even with discrepancies between
x86_64_start_kernel() and x86_64_start_reservations() and
xen_start_kernel(). There's another important reason listed below...

2) Provided we address 1) above it could mean it being possible to
unify *at least* the C Xen x86_64 init path and the bare metal x86_64
init paths without much code shuffling. Based on discussions at the
last Xen developer summit it seemed this was being considered and
perhaps desirable. Now the patch below would need a bit more work, but
ultimately this gives a small glance at what this could in theory
possibly look like:

http://drvbp1.linux-foundation.org/~mcgrof/patches/2015/12/15/x86-merge-x86-init-v1.patch

The xen init stuff just becomes a Xen specific subarch call. Folks
interested in this prospect are welcomed to help review or expand on
this work. If you are working on another type of unifying init
solution I'd like to hear it as well.

  Luis

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: Unifying x86_64 / Xen init paths and reading hardware_subarch early
  2016-01-15 22:08 Unifying x86_64 / Xen init paths and reading hardware_subarch early Luis R. Rodriguez
@ 2016-01-15 23:47 ` Andy Lutomirski
  2016-01-16  0:43   ` Luis R. Rodriguez
  2016-01-20 22:24   ` Konrad Rzeszutek Wilk
  0 siblings, 2 replies; 7+ messages in thread
From: Andy Lutomirski @ 2016-01-15 23:47 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: xen-devel@lists.xensource.com, Juergen Gross,
	Konrad Rzeszutek Wilk, linux-kernel@vger.kernel.org, X86 ML,
	Rusty Russell, Jeremy Fitzhardinge, H. Peter Anvin,
	Luis R. Rodriguez, Borislav Petkov

On Fri, Jan 15, 2016 at 2:08 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> I will be respinning the generic Linux linker table solution [0] soon
> based on hpa's feedback again now that I'm back from vacation. As I do
> that though I wanted to highlight a feature I'm throwing into the
> linker table solution which I am not sure many have paid close
> attention to but I think is important to Xen. I'm making use of the
> zero page hardware_subarch to enable us to detect if we're a specific
> hypervisor solution *as early as is possible*. This has a few
> implications, short term it is designed to provides a proactive
> technical solution to bugs such as the cr4 shadow crash (see
> 5054daa285beaf706f051fbd395dc36c9f0f907f) and ensure that *new* x86
> features get a proper Xen implementation proactively *or* at the very
> least get annotated as unsupported properly, instead of having them
> crash and later finding out. A valid example here is Kasan, which to
> this day lacks proper Xen support. In the future, if the generic
> linker table solution gets merged, it would mean developers would have
> to *think* about if they support Xen or not at development time. It
> does this in a not-disruptive way to Xen / x86_64 but most
> *importantly* it does not extend pvops! This should avoid issues in
> cases of developer / maintainer bandwidth, should some new features be
> pushed onto Linux for x86_64 but a respective Xen solution is not
> addressed, and that was not caught early in patch review, such as with
> Kasan.
>
> [0] https://lkml.kernel.org/r/1450217797-19295-1-git-send-email-mcgrof@do-not-panic.com
>
> Two things I'd like to request a bit of help with and review / consideration:
>
> 1) I'd like some advice on a curious problem I've stumbled on. I'd
> like to access hardware_subarch super early, and in my review with at
> least two x86 folks this *should* work:
>
> diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
> index c913b7eb5056..9168842821c8 100644
> --- a/arch/x86/kernel/head64.c
> +++ b/arch/x86/kernel/head64.c
> @@ -141,6 +141,7 @@ static void __init copy_bootdata(char *real_mode_data)
>
>  asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
>  {
> + struct boot_params *params = (struct boot_params *)__va(real_mode_data);
>   int i;

This is a mess :-p

If you want to access real_mode_data before load_idt, you'll need to do:

    for (i = 0; i < sizeof(boot_params); i += 4096)
        early_make_pgtable((unsigned long)params + i);

Of course, it's entirely possible that that will blow up if you try to
do it on Xen.

I think this would all be easier to understand if you try to separate
out the ideas of linker tables from the idea of rearranging early
init.  AFAICT the linker table thing is just an implementation detail.

If I understand right, you're trying to unify the Xen and native
startup as much as possible.  Why not add little shims, though?
Create a new start_kernel_common(int subarch, ...) where subarch
indicates native vs Xen and have its callers tell it which mode it's
in?

--Andy

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Unifying x86_64 / Xen init paths and reading hardware_subarch early
  2016-01-15 23:47 ` Andy Lutomirski
@ 2016-01-16  0:43   ` Luis R. Rodriguez
  2016-01-16  1:18     ` H. Peter Anvin
  2016-01-16  1:39     ` Luis R. Rodriguez
  2016-01-20 22:24   ` Konrad Rzeszutek Wilk
  1 sibling, 2 replies; 7+ messages in thread
From: Luis R. Rodriguez @ 2016-01-16  0:43 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: xen-devel@lists.xensource.com, Juergen Gross,
	Konrad Rzeszutek Wilk, linux-kernel@vger.kernel.org, X86 ML,
	Rusty Russell, Jeremy Fitzhardinge, H. Peter Anvin,
	Luis R. Rodriguez, Borislav Petkov

On Fri, Jan 15, 2016 at 03:47:25PM -0800, Andy Lutomirski wrote:
> On Fri, Jan 15, 2016 at 2:08 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> > I will be respinning the generic Linux linker table solution [0] soon
> > based on hpa's feedback again now that I'm back from vacation. As I do
> > that though I wanted to highlight a feature I'm throwing into the
> > linker table solution which I am not sure many have paid close
> > attention to but I think is important to Xen. I'm making use of the
> > zero page hardware_subarch to enable us to detect if we're a specific
> > hypervisor solution *as early as is possible*. This has a few
> > implications, short term it is designed to provides a proactive
> > technical solution to bugs such as the cr4 shadow crash (see
> > 5054daa285beaf706f051fbd395dc36c9f0f907f) and ensure that *new* x86
> > features get a proper Xen implementation proactively *or* at the very
> > least get annotated as unsupported properly, instead of having them
> > crash and later finding out. A valid example here is Kasan, which to
> > this day lacks proper Xen support. In the future, if the generic
> > linker table solution gets merged, it would mean developers would have
> > to *think* about if they support Xen or not at development time. It
> > does this in a not-disruptive way to Xen / x86_64 but most
> > *importantly* it does not extend pvops! This should avoid issues in
> > cases of developer / maintainer bandwidth, should some new features be
> > pushed onto Linux for x86_64 but a respective Xen solution is not
> > addressed, and that was not caught early in patch review, such as with
> > Kasan.
> >
> > [0] https://lkml.kernel.org/r/1450217797-19295-1-git-send-email-mcgrof@do-not-panic.com
> >
> > Two things I'd like to request a bit of help with and review / consideration:
> >
> > 1) I'd like some advice on a curious problem I've stumbled on. I'd
> > like to access hardware_subarch super early, and in my review with at
> > least two x86 folks this *should* work:
> >
> > diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
> > index c913b7eb5056..9168842821c8 100644
> > --- a/arch/x86/kernel/head64.c
> > +++ b/arch/x86/kernel/head64.c
> > @@ -141,6 +141,7 @@ static void __init copy_bootdata(char *real_mode_data)
> >
> >  asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
> >  {
> > + struct boot_params *params = (struct boot_params *)__va(real_mode_data);
> >   int i;
> 
> This is a mess :-p

Agreed. Doing what I can without extending pvops though ;)

> If you want to access real_mode_data before load_idt, you'll need to do:
> 
>     for (i = 0; i < sizeof(boot_params); i += 4096)
>         early_make_pgtable((unsigned long)params + i);

Thanks I'll give this a shot.

> Of course, it's entirely possible that that will blow up if you try to
> do it on Xen.

I'll check, if its safe and if the subarch strategy is desirable to help with
unifying init, then great. Otherwise we'd need to figure this out.

> I think this would all be easier to understand if you try to separate
> out the ideas of linker tables from the idea of rearranging early
> init.

Oh absolutely. The goal to unify init *or* to access subarch earlier provides
slightly different gains and possiblities. This is why I am addressing this
separately. Its important to highlight the prospects though given I think a few
folks may not have realized what might be possible here...

>  AFAICT the linker table thing is just an implementation detail.

Indeed, but just as a linker table is one thing, the *use* of the linker table
for x86 early init is another. Its a good example how how to use the linker
tables though.

The things I make mention of here are just possible *enhancements* of that work
provided the subarch can be read earlier.  Another possibility which I also had
not mentioned is the ability to also free annotated code on x86 init which we
*know* for sure we don't need, much as __init code after we boot, only this
could be done later at run time. That's also best technically considered later
but perhaps worth mentioning now as a future possibility.

Although the linker table series does not address unifying init, in this thread
we are talking about the prospect of being able to do that in the future. Its
best to consider this early than late.

> If I understand right, you're trying to unify the Xen and native
> startup as much as possible. 

That ultimately is a possibility. The original patches don't do that though.
They just pave the way with linker tables as baby steps.

Without access to the subarch so early unifying init is not possible with a
linker table solution though. As the series was posted though its late use
(after load_idt()) still holds promise to help annotate subarch support, which
in turn also helps provide dependency maps. This should help with a proactive
solution to ensure x86 developers think about x86 early requirements. Only
that gain would be restricted to after load_idt().

> Why not add little shims, though?
> Create a new start_kernel_common(int subarch, ...) where subarch
> indicates native vs Xen and have its callers tell it which mode it's
> in?

That's possible too, if you think that's best. However the subarch is already
part of the zero page, so figured why not just make use of it. Also if you make
use of it, it may even be possible to unify x86 init even on the asm front if
we ultimately even wanted that. One of the gains of striving to unify inits
*and* having a subarch dependency annotated for features is its a proactive
measure to ensure developers think of the different x86 requirements. If we
use the shim we may be able to unify some code but there would still be
some area not addressed. My litmus test was to try to strive to address
features such as Kasan, and that required even earlier subarch access.

  Luis

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Unifying x86_64 / Xen init paths and reading hardware_subarch early
  2016-01-16  0:43   ` Luis R. Rodriguez
@ 2016-01-16  1:18     ` H. Peter Anvin
  2016-01-16  1:39     ` Luis R. Rodriguez
  1 sibling, 0 replies; 7+ messages in thread
From: H. Peter Anvin @ 2016-01-16  1:18 UTC (permalink / raw)
  To: Luis R. Rodriguez, Andy Lutomirski
  Cc: xen-devel@lists.xensource.com, Juergen Gross,
	Konrad Rzeszutek Wilk, linux-kernel@vger.kernel.org, X86 ML,
	Rusty Russell, Jeremy Fitzhardinge, Luis R. Rodriguez,
	Borislav Petkov

On January 15, 2016 4:43:04 PM PST, "Luis R. Rodriguez" <mcgrof@suse.com> wrote:
>On Fri, Jan 15, 2016 at 03:47:25PM -0800, Andy Lutomirski wrote:
>> On Fri, Jan 15, 2016 at 2:08 PM, Luis R. Rodriguez <mcgrof@suse.com>
>wrote:
>> > I will be respinning the generic Linux linker table solution [0]
>soon
>> > based on hpa's feedback again now that I'm back from vacation. As I
>do
>> > that though I wanted to highlight a feature I'm throwing into the
>> > linker table solution which I am not sure many have paid close
>> > attention to but I think is important to Xen. I'm making use of the
>> > zero page hardware_subarch to enable us to detect if we're a
>specific
>> > hypervisor solution *as early as is possible*. This has a few
>> > implications, short term it is designed to provides a proactive
>> > technical solution to bugs such as the cr4 shadow crash (see
>> > 5054daa285beaf706f051fbd395dc36c9f0f907f) and ensure that *new* x86
>> > features get a proper Xen implementation proactively *or* at the
>very
>> > least get annotated as unsupported properly, instead of having them
>> > crash and later finding out. A valid example here is Kasan, which
>to
>> > this day lacks proper Xen support. In the future, if the generic
>> > linker table solution gets merged, it would mean developers would
>have
>> > to *think* about if they support Xen or not at development time. It
>> > does this in a not-disruptive way to Xen / x86_64 but most
>> > *importantly* it does not extend pvops! This should avoid issues in
>> > cases of developer / maintainer bandwidth, should some new features
>be
>> > pushed onto Linux for x86_64 but a respective Xen solution is not
>> > addressed, and that was not caught early in patch review, such as
>with
>> > Kasan.
>> >
>> > [0]
>https://lkml.kernel.org/r/1450217797-19295-1-git-send-email-mcgrof@do-not-panic.com
>> >
>> > Two things I'd like to request a bit of help with and review /
>consideration:
>> >
>> > 1) I'd like some advice on a curious problem I've stumbled on. I'd
>> > like to access hardware_subarch super early, and in my review with
>at
>> > least two x86 folks this *should* work:
>> >
>> > diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
>> > index c913b7eb5056..9168842821c8 100644
>> > --- a/arch/x86/kernel/head64.c
>> > +++ b/arch/x86/kernel/head64.c
>> > @@ -141,6 +141,7 @@ static void __init copy_bootdata(char
>*real_mode_data)
>> >
>> >  asmlinkage __visible void __init x86_64_start_kernel(char *
>real_mode_data)
>> >  {
>> > + struct boot_params *params = (struct boot_params
>*)__va(real_mode_data);
>> >   int i;
>> 
>> This is a mess :-p
>
>Agreed. Doing what I can without extending pvops though ;)
>
>> If you want to access real_mode_data before load_idt, you'll need to
>do:
>> 
>>     for (i = 0; i < sizeof(boot_params); i += 4096)
>>         early_make_pgtable((unsigned long)params + i);
>
>Thanks I'll give this a shot.
>
>> Of course, it's entirely possible that that will blow up if you try
>to
>> do it on Xen.
>
>I'll check, if its safe and if the subarch strategy is desirable to
>help with
>unifying init, then great. Otherwise we'd need to figure this out.
>
>> I think this would all be easier to understand if you try to separate
>> out the ideas of linker tables from the idea of rearranging early
>> init.
>
>Oh absolutely. The goal to unify init *or* to access subarch earlier
>provides
>slightly different gains and possiblities. This is why I am addressing
>this
>separately. Its important to highlight the prospects though given I
>think a few
>folks may not have realized what might be possible here...
>
>>  AFAICT the linker table thing is just an implementation detail.
>
>Indeed, but just as a linker table is one thing, the *use* of the
>linker table
>for x86 early init is another. Its a good example how how to use the
>linker
>tables though.
>
>The things I make mention of here are just possible *enhancements* of
>that work
>provided the subarch can be read earlier.  Another possibility which I
>also had
>not mentioned is the ability to also free annotated code on x86 init
>which we
>*know* for sure we don't need, much as __init code after we boot, only
>this
>could be done later at run time. That's also best technically
>considered later
>but perhaps worth mentioning now as a future possibility.
>
>Although the linker table series does not address unifying init, in
>this thread
>we are talking about the prospect of being able to do that in the
>future. Its
>best to consider this early than late.
>
>> If I understand right, you're trying to unify the Xen and native
>> startup as much as possible. 
>
>That ultimately is a possibility. The original patches don't do that
>though.
>They just pave the way with linker tables as baby steps.
>
>Without access to the subarch so early unifying init is not possible
>with a
>linker table solution though. As the series was posted though its late
>use
>(after load_idt()) still holds promise to help annotate subarch
>support, which
>in turn also helps provide dependency maps. This should help with a
>proactive
>solution to ensure x86 developers think about x86 early requirements.
>Only
>that gain would be restricted to after load_idt().
>
>> Why not add little shims, though?
>> Create a new start_kernel_common(int subarch, ...) where subarch
>> indicates native vs Xen and have its callers tell it which mode it's
>> in?
>
>That's possible too, if you think that's best. However the subarch is
>already
>part of the zero page, so figured why not just make use of it. Also if
>you make
>use of it, it may even be possible to unify x86 init even on the asm
>front if
>we ultimately even wanted that. One of the gains of striving to unify
>inits
>*and* having a subarch dependency annotated for features is its a
>proactive
>measure to ensure developers think of the different x86 requirements.
>If we
>use the shim we may be able to unify some code but there would still be
>some area not addressed. My litmus test was to try to strive to address
>features such as Kasan, and that required even earlier subarch access.
>
>  Luis

You can't assume setup_data is page aligned.
-- 
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Unifying x86_64 / Xen init paths and reading hardware_subarch early
  2016-01-16  0:43   ` Luis R. Rodriguez
  2016-01-16  1:18     ` H. Peter Anvin
@ 2016-01-16  1:39     ` Luis R. Rodriguez
  2016-01-16  9:09       ` Borislav Petkov
  1 sibling, 1 reply; 7+ messages in thread
From: Luis R. Rodriguez @ 2016-01-16  1:39 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: xen-devel@lists.xensource.com, Juergen Gross,
	Konrad Rzeszutek Wilk, linux-kernel@vger.kernel.org, X86 ML,
	Rusty Russell, Jeremy Fitzhardinge, H. Peter Anvin,
	Luis R. Rodriguez, Borislav Petkov

On Fri, Jan 15, 2016 at 4:43 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
>>     for (i = 0; i < sizeof(boot_params); i += 4096)
>>         early_make_pgtable((unsigned long)params + i);
>
>  I'll give this a shot.

Thanks again for this! It seems to let this boot now! But it does not
seem to provided the right value. If I use the qemu debug patch as I
listed before to set this to 5 for kvm, and boot it doesn't come up.
This can be tested with the qemu debug patch + this debug kernel patch
which prints it out and resets it from what it finds early.

If you comment out the boot_params.hdr.hardware_subarch =
my_hardware_subarch; assignment we get the right value from the
copy_bootdata() work. I use my_hardware_subarch just as a quick hack
to test and cache the value early code gets but that I can't print
early on.

diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index c913b7eb5056..6fc92553f272 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -139,9 +139,12 @@ static void __init copy_bootdata(char *real_mode_data)
  }
 }

+__u32 my_hardware_subarch;
+
 asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
 {
  int i;
+ struct boot_params *params = (struct boot_params *)__va(real_mode_data);

  /*
  * Build-time sanity checks on the kernel image and module
@@ -157,6 +160,13 @@ asmlinkage __visible void __init
x86_64_start_kernel(char * real_mode_data)
  (__START_KERNEL & PGDIR_MASK)));
  BUILD_BUG_ON(__fix_to_virt(__end_of_fixed_addresses) <= MODULES_END);

+ /* Make the zero page accessible as early as possible */
+ for (i = 0; i < sizeof(boot_params); i += 4096)
+ early_make_pgtable((unsigned long)params + i);
+
+ boot_params.hdr.hardware_subarch = params->hdr.hardware_subarch;
+ my_hardware_subarch = params->hdr.hardware_subarch;
+
  cr4_init_shadow();

  /* Kill off the identity-map trampoline */
@@ -173,6 +183,7 @@ asmlinkage __visible void __init
x86_64_start_kernel(char * real_mode_data)
  load_idt((const struct desc_ptr *)&idt_descr);

  copy_bootdata(__va(real_mode_data));
+ boot_params.hdr.hardware_subarch = my_hardware_subarch;

  /*
  * Load microcode early on BSP.
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index d3d80e6d42a2..c2f85f8ab52b 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -851,6 +851,8 @@ void __init setup_arch(char **cmdline_p)
  (unsigned long)__bss_stop - (unsigned long)_text);

  early_reserve_initrd();
+ pr_info("boot_params.hdr.hardware_subarch: 0x%04x\n",
+ boot_params.hdr.hardware_subarch);

  /*
  * At this point everything still needed from the boot loader


  Luis

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: Unifying x86_64 / Xen init paths and reading hardware_subarch early
  2016-01-16  1:39     ` Luis R. Rodriguez
@ 2016-01-16  9:09       ` Borislav Petkov
  0 siblings, 0 replies; 7+ messages in thread
From: Borislav Petkov @ 2016-01-16  9:09 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Andy Lutomirski, xen-devel@lists.xensource.com, Juergen Gross,
	Konrad Rzeszutek Wilk, linux-kernel@vger.kernel.org, X86 ML,
	Rusty Russell, Jeremy Fitzhardinge, H. Peter Anvin

On Fri, Jan 15, 2016 at 05:39:05PM -0800, Luis R. Rodriguez wrote:
> On Fri, Jan 15, 2016 at 4:43 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> >>     for (i = 0; i < sizeof(boot_params); i += 4096)
> >>         early_make_pgtable((unsigned long)params + i);
> >
> >  I'll give this a shot.
> 
> Thanks again for this! It seems to let this boot now! But it does not
> seem to provided the right value. If I use the qemu debug patch as I
> listed before to set this to 5 for kvm, and boot it doesn't come up.
> This can be tested with the qemu debug patch + this debug kernel patch
> which prints it out and resets it from what it finds early.
> 
> If you comment out the boot_params.hdr.hardware_subarch =
> my_hardware_subarch; assignment we get the right value from the
> copy_bootdata() work. I use my_hardware_subarch just as a quick hack
> to test and cache the value early code gets but that I can't print
> early on.

You can always do stupid debug loops:

	while (subarch == <expected_value>)
		rep_nop();

and when your guest stops booting and gdb points you here, then you know
what's going on. You can then dump interesting stuff too from gdb.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Unifying x86_64 / Xen init paths and reading hardware_subarch early
  2016-01-15 23:47 ` Andy Lutomirski
  2016-01-16  0:43   ` Luis R. Rodriguez
@ 2016-01-20 22:24   ` Konrad Rzeszutek Wilk
  1 sibling, 0 replies; 7+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-01-20 22:24 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Juergen Gross, Rusty Russell, xen-devel@lists.xensource.com,
	Luis R. Rodriguez, Luis R. Rodriguez, X86 ML,
	linux-kernel@vger.kernel.org, Jeremy Fitzhardinge, H. Peter Anvin,
	Borislav Petkov

On Fri, Jan 15, 2016 at 03:47:25PM -0800, Andy Lutomirski wrote:
> On Fri, Jan 15, 2016 at 2:08 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> > I will be respinning the generic Linux linker table solution [0] soon
> > based on hpa's feedback again now that I'm back from vacation. As I do
> > that though I wanted to highlight a feature I'm throwing into the
> > linker table solution which I am not sure many have paid close
> > attention to but I think is important to Xen. I'm making use of the
> > zero page hardware_subarch to enable us to detect if we're a specific
> > hypervisor solution *as early as is possible*. This has a few
> > implications, short term it is designed to provides a proactive
> > technical solution to bugs such as the cr4 shadow crash (see
> > 5054daa285beaf706f051fbd395dc36c9f0f907f) and ensure that *new* x86
> > features get a proper Xen implementation proactively *or* at the very
> > least get annotated as unsupported properly, instead of having them
> > crash and later finding out. A valid example here is Kasan, which to
> > this day lacks proper Xen support. In the future, if the generic
> > linker table solution gets merged, it would mean developers would have
> > to *think* about if they support Xen or not at development time. It
> > does this in a not-disruptive way to Xen / x86_64 but most
> > *importantly* it does not extend pvops! This should avoid issues in
> > cases of developer / maintainer bandwidth, should some new features be
> > pushed onto Linux for x86_64 but a respective Xen solution is not
> > addressed, and that was not caught early in patch review, such as with
> > Kasan.
> >
> > [0] https://lkml.kernel.org/r/1450217797-19295-1-git-send-email-mcgrof@do-not-panic.com
> >
> > Two things I'd like to request a bit of help with and review / consideration:
> >
> > 1) I'd like some advice on a curious problem I've stumbled on. I'd
> > like to access hardware_subarch super early, and in my review with at
> > least two x86 folks this *should* work:
> >
> > diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
> > index c913b7eb5056..9168842821c8 100644
> > --- a/arch/x86/kernel/head64.c
> > +++ b/arch/x86/kernel/head64.c
> > @@ -141,6 +141,7 @@ static void __init copy_bootdata(char *real_mode_data)
> >
> >  asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
> >  {
> > + struct boot_params *params = (struct boot_params *)__va(real_mode_data);
> >   int i;
> 
> This is a mess :-p
> 
> If you want to access real_mode_data before load_idt, you'll need to do:
> 
>     for (i = 0; i < sizeof(boot_params); i += 4096)
>         early_make_pgtable((unsigned long)params + i);
> 
> Of course, it's entirely possible that that will blow up if you try to
> do it on Xen.

That real_mode should have already been setup by Xen by the time you
call this code. (I hope).

> 
> I think this would all be easier to understand if you try to separate
> out the ideas of linker tables from the idea of rearranging early
> init.  AFAICT the linker table thing is just an implementation detail.
> 
> If I understand right, you're trying to unify the Xen and native
> startup as much as possible.  Why not add little shims, though?
> Create a new start_kernel_common(int subarch, ...) where subarch
> indicates native vs Xen and have its callers tell it which mode it's
> in?
> 
> --Andy

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-01-20 22:24 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-15 22:08 Unifying x86_64 / Xen init paths and reading hardware_subarch early Luis R. Rodriguez
2016-01-15 23:47 ` Andy Lutomirski
2016-01-16  0:43   ` Luis R. Rodriguez
2016-01-16  1:18     ` H. Peter Anvin
2016-01-16  1:39     ` Luis R. Rodriguez
2016-01-16  9:09       ` Borislav Petkov
2016-01-20 22:24   ` Konrad Rzeszutek Wilk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).