Can't boot as Xen dom0 due to commit fe055896

Commit Message

Boris Ostrovsky Dec. 15, 2016, 7:08 p.m. UTC

On 12/15/2016 12:36 PM, Borislav Petkov wrote:
> On Thu, Dec 15, 2016 at 12:27:49PM -0500, Boris Ostrovsky wrote:
>> It will probably fix it but I don't think we want this: it's a
>> build-time solution. Most kernels have XEN on even though they are
>> booted bare-metal.
> Lemme tell you want I want: a way to detect I'm running on xen. Does
> CPUID(4) work really early, at load_ucode_bsp() time?
>
> IOW, can I use some of the functionality hypervisor_cpuid_base() uses to
> detect xen and stop loading any further?

I think we need at least

                if (IS_ENABLED(CONFIG_X86_32) && (cont.size != -1)) {



This fixes my PV boot problem. I am still failing to boot HVM, will need
to look at this some more.

-boris

Comments

Borislav Petkov Dec. 15, 2016, 7:23 p.m. UTC | #1

On Thu, Dec 15, 2016 at 02:08:50PM -0500, Boris Ostrovsky wrote:
> This fixes my PV boot problem. I am still failing to boot HVM, will
> need to look at this some more.

No, no more stabbing in the dark and no more brown paper bags.

Please check whether CPUID(4) works that early in any xen guest and
let's add that check to a function which does something like:

bool loader_disabled(void)
{
	if (running_on_a_xen_guest)
		return true;

        if (check_loader_disabled_bsp())
                return true;

        if (!have_cpuid_p())
                return true;

	return false;

}

and call that at the entry points and be done with it.

Or if there's some other clean method to detect I'm running on a xen
guest.

Thanks.

Boris Ostrovsky Dec. 15, 2016, 7:36 p.m. UTC | #2

On 12/15/2016 02:23 PM, Borislav Petkov wrote:
> On Thu, Dec 15, 2016 at 02:08:50PM -0500, Boris Ostrovsky wrote:
>> This fixes my PV boot problem. I am still failing to boot HVM, will
>> need to look at this some more.
> No, no more stabbing in the dark and no more brown paper bags.

This fixes a bug that has nothing to do with Xen.

We are calling find_proper_container(..., &eq_id) and determine result
based on eq_id not being zero. If find_proper_container() doesn't find
anything it will not modify eq_id and so you get back whatever you
passed in.

What the patch that I sent does is no different from how
apply_microcode_early_amd() makes the call to find_proper_container.

The fact that  I am having problems with HVM may or may not have
anything to do with microcode. I don't know yet but it's separate from
save_microcode_in_initrd_amd() patch. I am pretty sure about that
because unlike PV it is failing in early boot code.

-boris

>
> Please check whether CPUID(4) works that early in any xen guest and
> let's add that check to a function which does something like:
>
> bool loader_disabled(void)
> {
> 	if (running_on_a_xen_guest)
> 		return true;
>
>         if (check_loader_disabled_bsp())
>                 return true;
>
>         if (!have_cpuid_p())
>                 return true;
>
> 	return false;
>
> }
>
> and call that at the entry points and be done with it.
>
> Or if there's some other clean method to detect I'm running on a xen
> guest.
>
> Thanks.
>

Borislav Petkov Dec. 15, 2016, 8:03 p.m. UTC | #3

On Thu, Dec 15, 2016 at 02:36:46PM -0500, Boris Ostrovsky wrote:
> We are calling find_proper_container(..., &eq_id) and determine result
> based on eq_id not being zero. If find_proper_container() doesn't find
> anything it will not modify eq_id and so you get back whatever you
> passed in.

The correct fix for that is to not return struct container at all from
those functions but simply an eq_id or even an int error value or so.
Passing back a struct is simply silly. I'll prep a proper patch for
that.

> The fact that  I am having problems with HVM may or may not have
> anything to do with microcode. I don't know yet but it's separate from
> save_microcode_in_initrd_amd() patch. I am pretty sure about that
> because unlike PV it is failing in early boot code.

I still need a fix for Jürgen's use case where he lands in the
microcode loader. And that he shouldn't do in the first place when
running in a xen guest.

Patch

diff --git a/arch/x86/kernel/cpu/microcode/amd.c
b/arch/x86/kernel/cpu/microcode/amd.c
index 6f353bd..383b635 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -427,7 +427,7 @@  int __init save_microcode_in_initrd_amd(unsigned int
fam)
 {
        enum ucode_state ret;
        int retval = 0;
-       u16 eq_id;
+       u16 eq_id = 0;
 
        if (!cont.data) {