Re: [PATCH v1 04/12] xen/hvmlite: Bootstrap HVMlite guest

From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
To: "Luis R. Rodriguez" <mcgrof@suse.com>
Cc: "Juergen Gross" <jgross@suse.com>,
	"Jeremy Fitzhardinge" <jeremy@goop.org>,
	"X86 ML" <x86@kernel.org>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Rusty Russell" <rusty@rustcorp.com.au>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Andy Lutomirski" <luto@amacapital.net>,
	"David Vrabel" <david.vrabel@citrix.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	xen-devel@lists.xenproject.org, "Borislav Petkov" <bp@suse.de>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [PATCH v1 04/12] xen/hvmlite: Bootstrap HVMlite guest
Date: Tue, 26 Jan 2016 16:51:38 -0500	[thread overview]
Message-ID: <56A7EA6A.2030502__8265.97997588596$1453845208$gmane$org@oracle.com> (raw)
In-Reply-To: <20160126203023.GI20964@wotan.suse.de>

On 01/26/2016 03:30 PM, Luis R. Rodriguez wrote:
>
> What I'm proposing?
>
> 1) Lets see if we can put a proactive stop-gap to issues such as the cr4 shadow
> bug and Kasan bugs from creeping in. This should not only help PV but perhaps
> HVMLite. If you'd like to help with that refer to this thread:
>
> http://lkml.kernel.org/r/CAB=NE6VTCRCazcNpCdJ7pN1eD3=x_fcGOdH37MzVpxkKEN5esw@mail.gmail.com
>
> 2) asm code sharing prospects - rebranding of PVH to HVMlite
>
> It sounds like HVMlite is really just a clean PVH implementation so we'll
> be doing this it seems according to this approach:
>
> a) Since PVH brand is taken add new new Xen clean solution as "HVMLite implementation"
> b) Drop PVH
> c) Re-brand HVMLite as PVH
>
> Is there really no asm code to share between PV and HVMlite ?

Not the early boot code.

>   How about between
> PV and other Linux asm ? Specifically I'm talking about a huge new entry point
> called hvmlite_start_xen() of asm code. How much sharing or duplication
> is being avoided here?

I don't see that we can share much here. Especially given that 
hvmlite_start() is a 32-bit entry point so we can't start a 64-bit PV guest.

I suppose we can find some common (C) code that sets boot_params but on 
the first sight they are quite different too.

(And whether it's huge is a matter of opinion ;-). It is more than a 
couple of instructions, I'll give you that)

>
> 3) C code sharing prospects: rebranding of PVH to HVMlite
>
> This code seems to share a lot from PV guest. Can we combine?

HVMlite closely follows HVM (and baremetal) code path. There is some 
common setup code with PV (which is what the second patch in this series 
is trying to factor out)

>
> 4) hardware_subarch, hardware_subarch_data and future prospects
>
> Your patch relies on a *new* Linux entry point. Sure, we had one
> for EFI, and sure there is another for Xen PV, but since you're just
> rebranding PVH to HVMlite and given historic issues with any new
> Linux entry points I'd like for us to take a breather and evaluate
> the extent our lofty unification goals, and how the x86 boot protocol
> could help with this already.

I am not sure I see how you can avoid having new entry point. For 
example, all HVMlite guests start in 32-bit mode. Who will switch to 
long mode?

>
> Now, perhaps today it may seem as difficult to unify asm entry points
> today all over, but if we can take baby steps towards that I think that
> should be seriously evaluated.
>
> For instance, we should consider on relying on hardware_subarch and
> hardware_subarch_data to fetch the hvmlite_start_info by taking advantage of
> the x86 boot protocol.  The hvmlite_start_info is what Xen uses to send us info
> about the guest as your patch proposes (this matches the Xen PV style entry),
> we stash it into a standard Linux boot_params variable called
> xen_hvmlite_boot_params for the Xen guest in hvmlite_bootparam(). This
> data structure and code seems to match very much the PV guest structure,

No, the two don't match at all.

> why not just use a union and differentiate on PV subtype ? If you want to avoid
> a lot of PV calls for HVMlite it seems you could just take advantage of
> subarch Xen type, and differentiate on the subarch_data within Xen code
> to make that code just PV sepecific.
>
> I only see gains by using the Xen subarch, so would like to know why PC is
> being pushed.

It's not that subarch 0 is being pushed here. It's that I don't see how 
it can be useful for this particular guest type. Maybe as we add new 
features (or discover something that we've missed up till now) we can 
switch to using it. If you think we should delay initializing 
boot_params until then --- I will disagree: boot_params are used before 
we look at subarch and I don't believe it makes sense to pick and choose 
what to initialize before and what can wait.

(And I am not sure it can be useful on PV neither, at least the way it 
is used now. You will not reach the point in the (32-bit) code where it 
is tested. You will die way earlier (I think on startup_32()'s fourth 
instruction).)

-boris