From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751857AbcAZUab (ORCPT ); Tue, 26 Jan 2016 15:30:31 -0500 Received: from mx2.suse.de ([195.135.220.15]:50253 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750756AbcAZUa2 (ORCPT ); Tue, 26 Jan 2016 15:30:28 -0500 Date: Tue, 26 Jan 2016 21:30:23 +0100 From: "Luis R. Rodriguez" To: Boris Ostrovsky Cc: Andrew Cooper , Roger Pau =?iso-8859-1?Q?Monn=E9?= , Juergen Gross , Jeremy Fitzhardinge , Rusty Russell , "linux-kernel@vger.kernel.org" , Andy Lutomirski , David Vrabel , "H. Peter Anvin" , xen-devel@lists.xenproject.org, Borislav Petkov , X86 ML Subject: Re: [Xen-devel] [PATCH v1 04/12] xen/hvmlite: Bootstrap HVMlite guest Message-ID: <20160126203023.GI20964@wotan.suse.de> References: <1453498558-6028-1-git-send-email-boris.ostrovsky@oracle.com> <1453498558-6028-5-git-send-email-boris.ostrovsky@oracle.com> <20160122233218.GA20964@wotan.suse.de> <56A2C99A.2050701@citrix.com> <56A39300.8050802@citrix.com> <20160125221920.GG20964@wotan.suse.de> <56A6A7C6.8060906@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56A6A7C6.8060906@oracle.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jan 25, 2016 at 05:55:02PM -0500, Boris Ostrovsky wrote: > On 01/25/2016 05:19 PM, Luis R. Rodriguez wrote: > >On Sat, Jan 23, 2016 at 02:49:36PM +0000, Andrew Cooper wrote: > > > > > >>it causes inappropriate linkage between the > >>toolstack and the version of Linux wishing to be booted. > >There are ways to do it in such a way that it does not create dependency issues > >on Linux specific code. > > > > > >0) The Xen way and EFI way > > > >A generic data structure is passed to the entry point on the kernel to > >load the kernel. The kernel in turn must parse this generic specific struct > >and interprets it and translate it to the kernel specific values. > > > >1) The qemu way: > > > >One way is to simply not refer to the boot_params data structures but IMHO that > >produces really ugly code. The qemu folks did it this way, so qemu does not use > >arch/x86/include/uapi/asm/bootparam.h in any way, instead it uses offsets from > >a char *header. Refer to load_linux() in hw/i386/pc.c, for instance: > > > >header[0x211] |= 0x80; /* CAN_USE_HEAP */ > > > >2) The grub way: > > > >Another way, which grub uses, is to define their own data structures based > >on arch/x86/include/uapi/asm/bootparam.h, with their own data types, and refer > >to them in code, for instance refer to grub_cmd_linux() on the grub file > >grub-core/loader/i386/pc/linux.c and its copy definition of the header definition > >in include/grub/i386/linux.h. > > > >lh.loadflags |= GRUB_LINUX_FLAG_CAN_USE_HEAP; > > > >The way lguest does it in the lguest launcher is IMHO the cleanest of course, > >but it includes asm/bootparam.h and uses that in code: > > > >/* Boot protocol version: 2.07 supports the fields for lguest. */ > >boot->hdr.version = 0x207; > > > >But that does mean copying over or using the bootparam.h file and using > >linux data types. > > > >3) Merge of xen way and using subarch_data > > > >Only set the subarch and subarch_data pointer, and have the kernel then > >read the generic data structure and parse it as it used to, the benefit > >is sharing a common entry point. > > > >No one uses this yet. But I think its a reasonable compromise. > > > >Perhaps there are other ways as well. > > > >>>If true, then by no means, no matter how hard we try, and no matter > >>>what we do on the Linux front to help clean things up will we be able > >>>to have a unified bare metal / Xen entry. I'm noting it could be > >>>possible though provided we do just set the zero page, the subarch to > >>>Xen and subarch_data to the Xen custom data structure. > >>All you need is a very small stub which starts per the DMLite ABI, sets > >>up an appropriate zero_page, and jumps to the native entrypoint. > >Right. > > > I am trying to understand what your objection is to what is proposed > in this patch. It is the size of the stub? If yes -- what would you > like to leave out to be done later? I explained that here briefly: > >>However, this stub belongs in Linux, not in the Xen toolstack. That > >>way, when the Linux boot protocol is modified, both sides can be updated > >>accordingly. > > >Makes sense the different entry points just seems best avoided if possible. But let me elaborate: We want to strive towards avoiding new entry points on Linux at all costs, there have been historic issues with having multiple entry points, some of these may relate to Xen, some not so much but one should not ignore history. A few related patches, this incldues EFI entry points since Konrad brings up Xen's approach was inspired by that as well. Read from bottom up. Kasan: not yet fixed, the feature completely ignored Xen PV guests as a possible candidate for x86. Xen PV guests with Kasan enabled crash. The fix is not so trivial. What I'd prefer is for kasan to have a feature to be disabled at run time, and with that in place we can simply at early boot disable Kasan early on boot on Xen until Kasan is properly implemented and supported on Xen. 5054daa285beaf706f051fbd395dc36c9f0f907f - x86/xen: Initialize cr4 shadow for 64-bit PV(H) guests Due to xen's separate entry point and the addition of a cr4 shadow, only native had its feature added, this made Xen crash. f3670394c29ff3730638762c1760fd2f624e6d7b - Revert "x86/efi: Fixup GOT in all boot code paths" Fails to boot LInus' Sony Vaio Pro 11 9cb0e394234d244fe5a97e743ec9dd7ddff7e64b - x86/efi: Fixup GOT in all boot code paths Because of the multiple boot entry points the fixup for GOT needs to be done in different places. 7e8213c1f3acc064aef37813a39f13cbfe7c3ce7 - x86/efi: Correct EFI boot stub use of code32_start Title says it. b8ff87a6158886771677e6dc8139bac6e3cba717 - x86/efi: Firmware agnostic handover entry points Fixes the issue of a mismatch on 32-bit and 64-bit between firmware and kernel run. 99f857db8857aff691c51302f93648263ed07eb1 - x86, build: Dynamically find entry points in compressed startup code Boot loaders abuse static entry point. Perhaps not directly related to Xen but its a lesson to learn that regardless of what you document people may abuse the entry points in very unexpected ways. f791620fa7517e1045742c475a7f005db9a634b8 - x86, efi: Fix 32-bit EFI handover protocol entry point After the EFI handover protocol was added 32-bit boots had an issue. 9ca8f72a9297f2052d806bd1111e176533aa69bd - x86, efi: Handover Protocol *Only* use an EFI boot stub for EFI capable to avoid duplicate code in boot loaders to set up and boot Linux. 51b26ada79b605ed709ddcedbb6012e8f8e0ebed - x86: unify arch/x86/boot/compressed/vmlinux_*.lds Linus unifies vmlinux_*.lds and calls to unify startup_32() and startup_64(). This hasn't been done yet. -- As someone new reading all this code / commits all I can do is cringe when I see yet-another-entry-point being added. I think we can do better, so we should strive to that unless there are real technical impossibilities here. 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 ? 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? 3) C code sharing prospects: rebranding of PVH to HVMlite This code seems to share a lot from PV guest. Can we combine? 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. 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, 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. Luis