From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: Xen 4.1 + Linux compiled with PVH == BOOM Date: Fri, 3 Jan 2014 09:35:48 -0500 Message-ID: <20140103143548.GA27019@phenom.dumpdata.com> References: <20131220175735.GA619@phenom.dumpdata.com> <52B97E95.9060900@cantab.net> <52B98447.9080404@citrix.com> <52B98686.9060009@cantab.net> <20131230195648.GA2937@phenom.dumpdata.com> <20140102193051.GA17665@andromeda.dapyr.net> <20140102212359.GA11592@pegasus.dumpdata.com> <52C6AFB4.9000007@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: <52C6AFB4.9000007@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Roger Pau =?iso-8859-1?Q?Monn=E9?= Cc: xen-devel@lists.xensource.com, ian.campbell@citrix.com, george.dunlap@eu.citrix.com, Andrew Cooper , David Vrabel , jbeulich@suse.com, Konrad Rzeszutek Wilk List-Id: xen-devel@lists.xenproject.org On Fri, Jan 03, 2014 at 01:40:20PM +0100, Roger Pau Monn=E9 wrote: > On 02/01/14 22:23, Konrad Rzeszutek Wilk wrote: > > On Thu, Jan 02, 2014 at 03:30:52PM -0400, Konrad Rzeszutek Wilk wrote: > >> On Mon, Dec 30, 2013 at 02:56:48PM -0500, Konrad Rzeszutek Wilk wrote: > >>> On Tue, Dec 24, 2013 at 01:05:10PM +0000, David Vrabel wrote: > >>>> On 24/12/2013 12:55, Andrew Cooper wrote: > >>>>> On 24/12/2013 12:31, David Vrabel wrote: > >>>>>> On 20/12/2013 17:57, Konrad Rzeszutek Wilk wrote: > >>>>>>> Hey, > >>>>>>> > >>>>>>> This is with Linux and > >>>>>>> > >>>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git stab= le/pvh.v11 > >>>>>>> > >>>>>>> I get Xen 4.1 (only) hypervisor to blow up with a Linux kernel th= at has been > >>>>>>> compiled with PVH. > >>>>>>> > >>>>>>> I think the same problem would show up if I tried to launch a PV = guest = > >>>>>>> compiled as PVH under Xen 4.1 as well - as the ELF parsing code i= s shared > >>>>>>> with the toolstack. > >>>>>> If a kernel with both PVH and PV support enabled cannot boot in PV= mode > >>>>>> with a non-PVH aware hypervisor/toolstack then the kernel is broke= n. > >>>>>> > >>>>>> Hypervisor/tool-side fixes aren't the correct fix here. Xen 4.1 a= nd > >>>>>> even older are still widely deployed. > >>>>>> > >>>>>> David > >>>>> > >>>>> I believe that the problem is because the elf parsing code is not > >>>>> sufficiently forward-compatible aware, and rejects the PVH kernel > >>>>> because it has an unrecognised Xen elf note field. This is not a k= ernel > >>>>> bug. > >>> > >>> It (Xen 4.1) has the logic to ignore unrecognized Xen elf note fields= . But > >>> it (all Xen versions) do not have the logic to ignore in the "SUPPORT= ED_FEATURES" > >>> an unrecognized string. > >>> > >>>>> > >>>>> The elf parsing should accept unrecognised fields for forward > >>>>> compatibility, which would then allow a PV & PVH compiled kernel to= run > >>>>> in PV mode. > >>>> > >>>> It should but it doesn't, so a different way needs to be found for t= he > >>>> kernel to report (optional) PVH support. A method that is compatible > >>>> with older toolstacks. > >>> > >>> Also known as changes to the PVH ABI. > >>> > >>> Mukesh, Roger, George (emailing Ian instead since he is now the Relea= se Manager-pro-temp), Jan, > >>> > >>> a). That means dropping the 'hvm_callback_vector' check from xc_dom_= core.c and > >>> just depending on: "writable_descriptor_tables|auto_translated_physma= p|supervisor_mode_kernel" > >>> for PVH guests. > >>> > >>> b) Or dropping that altogether and introducing a new Xen elf note fie= ld, say: > >>> > >>> XEN_ELFNOTE_PVH_VERSION > >>> > >> > >> c). > >> > >> Use the 'XEN_ELFNOTE_SUPPORTED_FEATURES' which says: > >> * > >> * Other than XEN_ELFNOTE_FEATURES on pre-4.2 Xen, this note allows a > >> * kernel to specify support for features that older hypervisors don't > >> * know about. The set of features 4.2 and newer hypervisors will > >> * consider supported by the kernel is the combination of the sets > >> * specified through this and the string note. > >> > >> for hvm_callback_vector parameter. > >> > >>> > >>> Which way should we do this? > >> > >> The c) way looks the best. Ian, would you be OK with that idea for 4.4? > > = > > Seems that not only does it work without any changes in Xen 4.4 but it > > is all in the Linux kernel, and it allows us to boot an Linux kernel > > with PV and PVH support > > = > > Seems that not only does it work without any changes in Xen 4.4 but it > > is all in the Linux kernel: > > = > > = > > = > > diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S > > index 56f42c0..2ce56bf 100644 > > --- a/arch/x86/xen/xen-head.S > > +++ b/arch/x86/xen/xen-head.S > > @@ -11,12 +11,22 @@ > > #include > > = > > #include > > +#include > > #include > > = > > #ifdef CONFIG_XEN_PVH > > -#define PVH_FEATURES_STR "|writable_descriptor_tables|auto_translated= _physmap|supervisor_mode_kernel|hvm_callback_vector" > > +#define PVH_FEATURES_STR "|writable_descriptor_tables|auto_translated= _physmap|supervisor_mode_kernel" > > +/* Note the lack of 'hvm_callback_vector'. Older hypervisor will > > + * balk at this being part of XEN_ELFNOTE_FEATURES, so we put it in > > + * XEN_ELFNOTE_SUPPORTED_FEATURES which older hypervisors will ignore. > > + */ > > +#define PVH_FEATURES ((1 << XENFEAT_writable_page_tables) | \ > > + (1 << XENFEAT_auto_translated_physmap) | \ > > + (1 << XENFEAT_supervisor_mode_kernel) | \ > > + (1 << XENFEAT_hvm_callback_vector)) > > #else > > #define PVH_FEATURES_STR "" > > +#define PVH_FEATURES (0) > > #endif > > = > > __INIT > > @@ -102,6 +112,9 @@ NEXT_HYPERCALL(arch_6) > > ELFNOTE(Xen, XEN_ELFNOTE_ENTRY, _ASM_PTR startup_xen) > > ELFNOTE(Xen, XEN_ELFNOTE_HYPERCALL_PAGE, _ASM_PTR hypercall_page) > > ELFNOTE(Xen, XEN_ELFNOTE_FEATURES, .ascii "!writable_page_table= s|pae_pgdir_above_4gb"; .asciz PVH_FEATURES_STR) > > + ELFNOTE(Xen, XEN_ELFNOTE_SUPPORTED_FEATURES, .long (PVH_FEATURES) | > > + (1 << XENFEAT_writable_page_tables) | > = > PVH_FEATURES already contains XENFEAT_writable_page_tables, shouldn't > you remove it from PVH_FEATURES if you are adding it unconditionally > here? (or is this just to make clear that you need > XENFEAT_writable_page_tables for both PVH and PV?) Yup, that was the only reason for it. I will flesh out the comment to make that clear. Thanks! > = > Roger.