* Xen 4.1 + Linux compiled with PVH == BOOM
@ 2013-12-20 17:57 Konrad Rzeszutek Wilk
2013-12-21 1:47 ` Mukesh Rathor
` (2 more replies)
0 siblings, 3 replies; 21+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-12-20 17:57 UTC (permalink / raw)
To: xen-devel, Mukesh Rathor, JBeulich
Hey,
This is with Linux and
git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git stable/pvh.v11
I get Xen 4.1 (only) hypervisor to blow up with a Linux kernel that 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 is shared
with the toolstack.
The issue is that Xen 4.1 is missing git
commit 30832c06a8d1f9caff0987654ef9e24d59469d9a
Author: Mukesh Rathor <mukesh.rathor@oracle.com>
Date: Tue Sep 10 16:38:43 2013 +0200
libelf: add hvm callback vector feature
Add XENFEAT_hvm_callback_vector to elf_xen_feature_names so we can
ensure the kernel supports all features required for PVH mode when
building a PVH domU here. Note, hvm callback is required for PVH.
Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
where the message is a bit misleading. It also allows the hypervisor
to boot a PVH guest in PV mode.
Here is what the Xen serial log shows (with extra debugging sprinkled):
(XEN) elf_xen_parse_note: GUEST_OS = "linux"
(XEN) elf_xen_parse_note: GUEST_VERSION = "2.6"
(XEN) elf_xen_parse_note: XEN_VERSION = "xen-3.0"
(XEN) elf_xen_parse_note: VIRT_BASE = 0xffffffff80000000
(XEN) elf_xen_parse_note: ENTRY = 0xffffffff81cd51e0
(XEN) elf_xen_parse_note: HYPERCALL_PAGE = 0xffffffff81001000
(XEN) elf_xen_parse_note: FEATURES = "!writable_page_tables|pae_pgdir_above_4gb|writable_descriptor_tables|auto_translated_physmap|supervisor_mode_kernel|hvm_callback_vector"
(XEN) elf_xen_parse_features:70 r[writable_page_tables]
(XEN) elf_xen_parse_features:81 s[pae_pgdir_above_4gb]
(XEN) elf_xen_parse_features:81 s[writable_descriptor_tables]
(XEN) elf_xen_parse_features:81 s[auto_translated_physmap]
(XEN) elf_xen_parse_features:81 s[supervisor_mode_kernel]
(XEN) elf_xen_parse_features:88 5
(XEN) elf_xen_parse_note:206
(XEN) elf_xen_parse_notes:245 on Xen
We end up bailing out:
87 if ( i == elf_xen_features ) {
88 elf_msg(elf,"%s:%d %d\n", __func__, __LINE__, i);
89 return -1;
90 }
because the elf_xen_features (or rather elf_xen_feature_names)
does not have the "hvm_callback_vector parameter" and it can't parse
it - so it returns -1.
And that means:
203 case XEN_ELFNOTE_FEATURES:
204 if ( elf_xen_parse_features(elf, str, parms->f_supported,
205 parms->f_required) ) {
206 elf_msg(elf, "%s:%d\n", __func__, __LINE__);
207 return -1;
208 }
Which means we return at:
244 if ( elf_xen_parse_note(elf, parms, note) ) {
245 elf_msg(elf, "%s:%d on %s\n", __func__, __LINE__, note_name);
246 return ELF_NOTE_INVALID;
247 }
and never finish the construct_dom0.
Thought on how to fix it?
My thought was that we should return 0, that is in line with
some of the other code that deals with elf, such as elf_xen_parse_note
which returns 0:
130 if ( (type >= sizeof(note_desc) / sizeof(note_desc[0])) ||
131 (note_desc[type].name == NULL) )
132 {
133 elf_msg(elf, "%s: unknown xen elf note (0x%x)\n",
134 __FUNCTION__, type);
135 return 0;
136 }
Like this:
diff --git a/xen/common/libelf/libelf-dominfo.c b/xen/common/libelf/libelf-dominfo.c
index fda19e7..c9ff61e 100644
--- a/xen/common/libelf/libelf-dominfo.c
+++ b/xen/common/libelf/libelf-dominfo.c
@@ -83,7 +83,9 @@ elf_errorstatus elf_xen_parse_features(const char *features,
}
}
if ( i == elf_xen_features )
- return -1;
+ return 0; /* We don't recognize this feature, and let the
+ * caller figure out if it has all of the needed parts.
+ */
}
return 0;
But perhaps that is not the way to do it and we should just cherry-pick
30832c06a8d1f9caff0987654ef9e24d59469d9a in Xen 4.1?
cThoughts?
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: Xen 4.1 + Linux compiled with PVH == BOOM 2013-12-20 17:57 Xen 4.1 + Linux compiled with PVH == BOOM Konrad Rzeszutek Wilk @ 2013-12-21 1:47 ` Mukesh Rathor 2013-12-21 11:09 ` Ian Campbell 2013-12-24 12:31 ` David Vrabel 2 siblings, 0 replies; 21+ messages in thread From: Mukesh Rathor @ 2013-12-21 1:47 UTC (permalink / raw) To: Konrad Rzeszutek Wilk; +Cc: xen-devel, JBeulich On Fri, 20 Dec 2013 12:57:35 -0500 Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > Hey, > > This is with Linux and > > git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git ....... > But perhaps that is not the way to do it and we should just > cherry-pick 30832c06a8d1f9caff0987654ef9e24d59469d9a in Xen 4.1? > > cThoughts? IMO, that seems to be the easiest and safest solution. thanks, Mukesh ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Xen 4.1 + Linux compiled with PVH == BOOM 2013-12-20 17:57 Xen 4.1 + Linux compiled with PVH == BOOM Konrad Rzeszutek Wilk 2013-12-21 1:47 ` Mukesh Rathor @ 2013-12-21 11:09 ` Ian Campbell 2013-12-21 16:17 ` Konrad Rzeszutek Wilk 2013-12-23 9:37 ` Jan Beulich 2013-12-24 12:31 ` David Vrabel 2 siblings, 2 replies; 21+ messages in thread From: Ian Campbell @ 2013-12-21 11:09 UTC (permalink / raw) To: Konrad Rzeszutek Wilk; +Cc: xen-devel, JBeulich On Fri, 2013-12-20 at 12:57 -0500, Konrad Rzeszutek Wilk wrote: > My thought was that we should return 0[...] > Like this: > > diff --git a/xen/common/libelf/libelf-dominfo.c b/xen/common/libelf/libelf-dominfo.c > index fda19e7..c9ff61e 100644 > --- a/xen/common/libelf/libelf-dominfo.c > +++ b/xen/common/libelf/libelf-dominfo.c > @@ -83,7 +83,9 @@ elf_errorstatus elf_xen_parse_features(const char *features, > } > } > if ( i == elf_xen_features ) > - return -1; > + return 0; /* We don't recognize this feature, and let the > + * caller figure out if it has all of the needed parts. > + */ WRT the comment, if this code doesn't know about the feature then the caller is unlikely to know about it either, so silently ignoring it is more than likely completely fine. [...] > But perhaps that is not the way to do it and we should just cherry-pick > 30832c06a8d1f9caff0987654ef9e24d59469d9a in Xen 4.1? I think we should do both, i.e. backport 30832c06a8d1 now to solve the immediate problem and then look at fixing unstable to be more accepting of new features which it doesn't yet know about. Ultimately perhaps we'd want to backport that patch too. Ian. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Xen 4.1 + Linux compiled with PVH == BOOM 2013-12-21 11:09 ` Ian Campbell @ 2013-12-21 16:17 ` Konrad Rzeszutek Wilk 2013-12-23 9:37 ` Jan Beulich 1 sibling, 0 replies; 21+ messages in thread From: Konrad Rzeszutek Wilk @ 2013-12-21 16:17 UTC (permalink / raw) To: Ian Campbell; +Cc: xen-devel, JBeulich On Sat, Dec 21, 2013 at 11:09:54AM +0000, Ian Campbell wrote: > On Fri, 2013-12-20 at 12:57 -0500, Konrad Rzeszutek Wilk wrote: > > > My thought was that we should return 0[...] > > Like this: > > > > diff --git a/xen/common/libelf/libelf-dominfo.c b/xen/common/libelf/libelf-dominfo.c > > index fda19e7..c9ff61e 100644 > > --- a/xen/common/libelf/libelf-dominfo.c > > +++ b/xen/common/libelf/libelf-dominfo.c > > @@ -83,7 +83,9 @@ elf_errorstatus elf_xen_parse_features(const char *features, > > } > > } > > if ( i == elf_xen_features ) > > - return -1; > > + return 0; /* We don't recognize this feature, and let the > > + * caller figure out if it has all of the needed parts. > > + */ > > WRT the comment, if this code doesn't know about the feature then the > caller is unlikely to know about it either, so silently ignoring it is > more than likely completely fine. <nods> > > [...] > > But perhaps that is not the way to do it and we should just cherry-pick > > 30832c06a8d1f9caff0987654ef9e24d59469d9a in Xen 4.1? > > I think we should do both, i.e. backport 30832c06a8d1 now to solve the > immediate problem and then look at fixing unstable to be more accepting > of new features which it doesn't yet know about. Ultimately perhaps we'd > want to backport that patch too. Ok, will crank out a patch after New Year. Thanks! > > Ian. > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Xen 4.1 + Linux compiled with PVH == BOOM 2013-12-21 11:09 ` Ian Campbell 2013-12-21 16:17 ` Konrad Rzeszutek Wilk @ 2013-12-23 9:37 ` Jan Beulich 2013-12-23 11:49 ` Jan Beulich 1 sibling, 1 reply; 21+ messages in thread From: Jan Beulich @ 2013-12-23 9:37 UTC (permalink / raw) To: Ian.Campbell, konrad.wilk; +Cc: xen-devel >>> Ian Campbell <Ian.Campbell@citrix.com> 12/21/13 12:10 PM >>> >On Fri, 2013-12-20 at 12:57 -0500, Konrad Rzeszutek Wilk wrote: >> But perhaps that is not the way to do it and we should just cherry-pick >> 30832c06a8d1f9caff0987654ef9e24d59469d9a in Xen 4.1? > >I think we should do both, i.e. backport 30832c06a8d1 now to solve the >immediate problem and then look at fixing unstable to be more accepting >of new features which it doesn't yet know about. Hmm, not sure - without a split between necessary to be understood and acceptable to be unknown ones, I'm not sure either model will be the right thing. >Ultimately perhaps we'd want to backport that patch too. But not to 4.1, which is only getting security fixes backported. Jan ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Xen 4.1 + Linux compiled with PVH == BOOM 2013-12-23 9:37 ` Jan Beulich @ 2013-12-23 11:49 ` Jan Beulich 2013-12-24 1:56 ` Konrad Rzeszutek Wilk 0 siblings, 1 reply; 21+ messages in thread From: Jan Beulich @ 2013-12-23 11:49 UTC (permalink / raw) To: Ian.Campbell, konrad.wilk; +Cc: xen-devel >>> "Jan Beulich" <jbeulich@suse.com> 12/23/13 10:39 AM >>> >>>> Ian Campbell <Ian.Campbell@citrix.com> 12/21/13 12:10 PM >>> >>On Fri, 2013-12-20 at 12:57 -0500, Konrad Rzeszutek Wilk wrote: >>> But perhaps that is not the way to do it and we should just cherry-pick >>> 30832c06a8d1f9caff0987654ef9e24d59469d9a in Xen 4.1? >> >>I think we should do both, i.e. backport 30832c06a8d1 now to solve the >>immediate problem and then look at fixing unstable to be more accepting >>of new features which it doesn't yet know about. > >Hmm, not sure - without a split between necessary to be understood >and acceptable to be unknown ones, I'm not sure either model will be >the right thing. And actually, in the case at hand the "BOOM" is correct: If the kernel tells the hypervisor that it needs a feature the hypervisor doesn't even recognize, it's surely wrong to ignore this. The mistake here is for the kernel to require that feature statically in the first place - that should be done only if the kernel could _only_ boot in PVH mode. Jan ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Xen 4.1 + Linux compiled with PVH == BOOM 2013-12-23 11:49 ` Jan Beulich @ 2013-12-24 1:56 ` Konrad Rzeszutek Wilk 2014-01-07 8:23 ` Jan Beulich 0 siblings, 1 reply; 21+ messages in thread From: Konrad Rzeszutek Wilk @ 2013-12-24 1:56 UTC (permalink / raw) To: Jan Beulich; +Cc: xen-devel, Ian.Campbell On Mon, Dec 23, 2013 at 11:49:49AM +0000, Jan Beulich wrote: > >>> "Jan Beulich" <jbeulich@suse.com> 12/23/13 10:39 AM >>> > >>>> Ian Campbell <Ian.Campbell@citrix.com> 12/21/13 12:10 PM >>> > >>On Fri, 2013-12-20 at 12:57 -0500, Konrad Rzeszutek Wilk wrote: > >>> But perhaps that is not the way to do it and we should just cherry-pick > >>> 30832c06a8d1f9caff0987654ef9e24d59469d9a in Xen 4.1? > >> > >>I think we should do both, i.e. backport 30832c06a8d1 now to solve the > >>immediate problem and then look at fixing unstable to be more accepting > >>of new features which it doesn't yet know about. > > > >Hmm, not sure - without a split between necessary to be understood > >and acceptable to be unknown ones, I'm not sure either model will be > >the right thing. > > And actually, in the case at hand the "BOOM" is correct: If the kernel tells > the hypervisor that it needs a feature the hypervisor doesn't even recognize, > it's surely wrong to ignore this. The mistake here is for the kernel to require But it does not ignore it. It checks later on in construct_dom0 whether the 'required' parameters are present, like: if ( test_bit(XENFEAT_supervisor_mode_kernel, parms.f_required) ) > that feature statically in the first place - that should be done only if the kernel > could _only_ boot in PVH mode. The feature is not marked as "required" but rather - it can utilize said extension (so supported). I am advocating that the calleer checks that all of the required pieces are correct - and it can ignore the ones it has no idea off (which it does for some of the Xen ELF notes - ignores them if it has no idea of what they are). > > Jan > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Xen 4.1 + Linux compiled with PVH == BOOM 2013-12-24 1:56 ` Konrad Rzeszutek Wilk @ 2014-01-07 8:23 ` Jan Beulich 2014-01-07 11:31 ` Ian Campbell 0 siblings, 1 reply; 21+ messages in thread From: Jan Beulich @ 2014-01-07 8:23 UTC (permalink / raw) To: Konrad Rzeszutek Wilk; +Cc: xen-devel, Ian.Campbell >>> On 24.12.13 at 02:56, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > On Mon, Dec 23, 2013 at 11:49:49AM +0000, Jan Beulich wrote: >> >>> "Jan Beulich" <jbeulich@suse.com> 12/23/13 10:39 AM >>> >> >>>> Ian Campbell <Ian.Campbell@citrix.com> 12/21/13 12:10 PM >>> >> >>On Fri, 2013-12-20 at 12:57 -0500, Konrad Rzeszutek Wilk wrote: >> >>> But perhaps that is not the way to do it and we should just cherry-pick >> >>> 30832c06a8d1f9caff0987654ef9e24d59469d9a in Xen 4.1? >> >> >> >>I think we should do both, i.e. backport 30832c06a8d1 now to solve the >> >>immediate problem and then look at fixing unstable to be more accepting >> >>of new features which it doesn't yet know about. >> > >> >Hmm, not sure - without a split between necessary to be understood >> >and acceptable to be unknown ones, I'm not sure either model will be >> >the right thing. >> >> And actually, in the case at hand the "BOOM" is correct: If the kernel tells >> the hypervisor that it needs a feature the hypervisor doesn't even > recognize, >> it's surely wrong to ignore this. The mistake here is for the kernel to > require > > But it does not ignore it. It checks later on in construct_dom0 whether > the 'required' parameters are present, like: That's what I said: It is / would be wrong to ignore a feature th kernel requires. > if ( test_bit(XENFEAT_supervisor_mode_kernel, parms.f_required) ) This is even more confusing: 30832c06 is about hvm_callback_vector, not supervisor_mode_kernel. >> that feature statically in the first place - that should be done only if the > kernel >> could _only_ boot in PVH mode. > > The feature is not marked as "required" but rather - it can utilize said > extension (so supported). I am advocating that the calleer checks that > all of the required pieces are correct - and it can ignore the ones it > has no idea off (which it does for some of the Xen ELF notes - ignores > them if it has no idea of what they are). What would be to point of telling the hypervisor that the kernel can utilize a certain extension? The kernel could just utilize it, and the hypervisor would know by that simple fact. Jan ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Xen 4.1 + Linux compiled with PVH == BOOM 2014-01-07 8:23 ` Jan Beulich @ 2014-01-07 11:31 ` Ian Campbell 2014-01-07 11:50 ` Jan Beulich 0 siblings, 1 reply; 21+ messages in thread From: Ian Campbell @ 2014-01-07 11:31 UTC (permalink / raw) To: Jan Beulich; +Cc: xen-devel On Tue, 2014-01-07 at 08:23 +0000, Jan Beulich wrote: > >> that feature statically in the first place - that should be done only if the > > kernel > >> could _only_ boot in PVH mode. > > > > The feature is not marked as "required" but rather - it can utilize said > > extension (so supported). I am advocating that the calleer checks that > > all of the required pieces are correct - and it can ignore the ones it > > has no idea off (which it does for some of the Xen ELF notes - ignores > > them if it has no idea of what they are). > > What would be to point of telling the hypervisor that the kernel > can utilize a certain extension? The kernel could just utilize it, and > the hypervisor would know by that simple fact. But for PVH doesn't the hypervisor need to now at dom0 build time whether to build a PV or PVH domain? So it needs to know upfront if the kernel could do PVH or not, and then pick, but once it has picked the kernel had best follow that choice. So in the PVH case it's not just a simple case of the kernel deciding to utilize an optional feature, the optional feature has already been enabled. Ian ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Xen 4.1 + Linux compiled with PVH == BOOM 2014-01-07 11:31 ` Ian Campbell @ 2014-01-07 11:50 ` Jan Beulich 2014-01-07 13:39 ` Ian Campbell 0 siblings, 1 reply; 21+ messages in thread From: Jan Beulich @ 2014-01-07 11:50 UTC (permalink / raw) To: Ian Campbell; +Cc: xen-devel >>> On 07.01.14 at 12:31, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Tue, 2014-01-07 at 08:23 +0000, Jan Beulich wrote: >> >> that feature statically in the first place - that should be done only if > the >> > kernel >> >> could _only_ boot in PVH mode. >> > >> > The feature is not marked as "required" but rather - it can utilize said >> > extension (so supported). I am advocating that the calleer checks that >> > all of the required pieces are correct - and it can ignore the ones it >> > has no idea off (which it does for some of the Xen ELF notes - ignores >> > them if it has no idea of what they are). >> >> What would be to point of telling the hypervisor that the kernel >> can utilize a certain extension? The kernel could just utilize it, and >> the hypervisor would know by that simple fact. > > But for PVH doesn't the hypervisor need to now at dom0 build time > whether to build a PV or PVH domain? Which needs to be communicated via hypervisor command line option anyway. Specifying the option without have a suitable kernel is (of course) a user error (generally expected to result in a kernel crash). Jan > So it needs to know upfront if the > kernel could do PVH or not, and then pick, but once it has picked the > kernel had best follow that choice. > > So in the PVH case it's not just a simple case of the kernel deciding to > utilize an optional feature, the optional feature has already been > enabled. > > Ian ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Xen 4.1 + Linux compiled with PVH == BOOM 2014-01-07 11:50 ` Jan Beulich @ 2014-01-07 13:39 ` Ian Campbell 0 siblings, 0 replies; 21+ messages in thread From: Ian Campbell @ 2014-01-07 13:39 UTC (permalink / raw) To: Jan Beulich; +Cc: xen-devel On Tue, 2014-01-07 at 11:50 +0000, Jan Beulich wrote: > >>> On 07.01.14 at 12:31, Ian Campbell <Ian.Campbell@citrix.com> wrote: > > On Tue, 2014-01-07 at 08:23 +0000, Jan Beulich wrote: > >> >> that feature statically in the first place - that should be done only if > > the > >> > kernel > >> >> could _only_ boot in PVH mode. > >> > > >> > The feature is not marked as "required" but rather - it can utilize said > >> > extension (so supported). I am advocating that the calleer checks that > >> > all of the required pieces are correct - and it can ignore the ones it > >> > has no idea off (which it does for some of the Xen ELF notes - ignores > >> > them if it has no idea of what they are). > >> > >> What would be to point of telling the hypervisor that the kernel > >> can utilize a certain extension? The kernel could just utilize it, and > >> the hypervisor would know by that simple fact. > > > > But for PVH doesn't the hypervisor need to now at dom0 build time > > whether to build a PV or PVH domain? > > Which needs to be communicated via hypervisor command line option > anyway. I would expect that the plan is to eventually enable PVH by default if the kernel can cope with it. > Specifying the option without have a suitable kernel is (of > course) a user error (generally expected to result in a kernel crash). In the case where the user has specified the option sure. Note that the original issue was a PVH capable kernel under a non-PVH capable Xen, although we've strayed a bit from that topic. Ian. > > Jan > > > So it needs to know upfront if the > > kernel could do PVH or not, and then pick, but once it has picked the > > kernel had best follow that choice. > > > > So in the PVH case it's not just a simple case of the kernel deciding to > > utilize an optional feature, the optional feature has already been > > enabled. > > > > Ian > > > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Xen 4.1 + Linux compiled with PVH == BOOM 2013-12-20 17:57 Xen 4.1 + Linux compiled with PVH == BOOM Konrad Rzeszutek Wilk 2013-12-21 1:47 ` Mukesh Rathor 2013-12-21 11:09 ` Ian Campbell @ 2013-12-24 12:31 ` David Vrabel 2013-12-24 12:55 ` Andrew Cooper 2 siblings, 1 reply; 21+ messages in thread From: David Vrabel @ 2013-12-24 12:31 UTC (permalink / raw) To: Konrad Rzeszutek Wilk, xen-devel, Mukesh Rathor, JBeulich 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 stable/pvh.v11 > > I get Xen 4.1 (only) hypervisor to blow up with a Linux kernel that 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 is 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 broken. Hypervisor/tool-side fixes aren't the correct fix here. Xen 4.1 and even older are still widely deployed. David ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Xen 4.1 + Linux compiled with PVH == BOOM 2013-12-24 12:31 ` David Vrabel @ 2013-12-24 12:55 ` Andrew Cooper 2013-12-24 13:05 ` David Vrabel 0 siblings, 1 reply; 21+ messages in thread From: Andrew Cooper @ 2013-12-24 12:55 UTC (permalink / raw) To: David Vrabel, Konrad Rzeszutek Wilk, xen-devel, Mukesh Rathor, JBeulich 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 stable/pvh.v11 >> >> I get Xen 4.1 (only) hypervisor to blow up with a Linux kernel that 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 is 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 broken. > > Hypervisor/tool-side fixes aren't the correct fix here. Xen 4.1 and > 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 kernel bug. The elf parsing should accept unrecognised fields for forward compatibility, which would then allow a PV & PVH compiled kernel to run in PV mode. If once the kernel starts running, it (for some reason) is unable to boot in PV mode when it has PVH also compiled in, then this is certainly a kernel bug. ~Andrew ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Xen 4.1 + Linux compiled with PVH == BOOM 2013-12-24 12:55 ` Andrew Cooper @ 2013-12-24 13:05 ` David Vrabel 2013-12-30 19:56 ` Konrad Rzeszutek Wilk 0 siblings, 1 reply; 21+ messages in thread From: David Vrabel @ 2013-12-24 13:05 UTC (permalink / raw) To: Andrew Cooper, Konrad Rzeszutek Wilk, xen-devel, Mukesh Rathor, JBeulich 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 stable/pvh.v11 >>> >>> I get Xen 4.1 (only) hypervisor to blow up with a Linux kernel that 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 is 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 broken. >> >> Hypervisor/tool-side fixes aren't the correct fix here. Xen 4.1 and >> 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 kernel > bug. > > 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 the kernel to report (optional) PVH support. A method that is compatible with older toolstacks. David ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Xen 4.1 + Linux compiled with PVH == BOOM 2013-12-24 13:05 ` David Vrabel @ 2013-12-30 19:56 ` Konrad Rzeszutek Wilk 2014-01-02 19:30 ` Konrad Rzeszutek Wilk 2014-01-03 0:27 ` Mukesh Rathor 0 siblings, 2 replies; 21+ messages in thread From: Konrad Rzeszutek Wilk @ 2013-12-30 19:56 UTC (permalink / raw) To: David Vrabel, Mukesh Rathor, george.dunlap, ian.campbell, jbeulich, roger.pau Cc: Andrew Cooper, xen-devel, JBeulich 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 stable/pvh.v11 > >>> > >>> I get Xen 4.1 (only) hypervisor to blow up with a Linux kernel that 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 is 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 broken. > >> > >> Hypervisor/tool-side fixes aren't the correct fix here. Xen 4.1 and > >> 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 kernel > > 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 "SUPPORTED_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 the > 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 Release 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_physmap|supervisor_mode_kernel" for PVH guests. b) Or dropping that altogether and introducing a new Xen elf note field, say: XEN_ELFNOTE_PVH_VERSION Which way should we do this? P.S. It looks like that the git tree is not accessible so I cannot update to the latest git tree to produce an RFC patchset :-( ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Xen 4.1 + Linux compiled with PVH == BOOM 2013-12-30 19:56 ` Konrad Rzeszutek Wilk @ 2014-01-02 19:30 ` Konrad Rzeszutek Wilk 2014-01-02 21:23 ` Konrad Rzeszutek Wilk 2014-01-03 0:27 ` Mukesh Rathor 1 sibling, 1 reply; 21+ messages in thread From: Konrad Rzeszutek Wilk @ 2014-01-02 19:30 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: xen-devel, ian.campbell, george.dunlap, Andrew Cooper, David Vrabel, jbeulich, roger.pau 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 stable/pvh.v11 > > >>> > > >>> I get Xen 4.1 (only) hypervisor to blow up with a Linux kernel that 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 is 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 broken. > > >> > > >> Hypervisor/tool-side fixes aren't the correct fix here. Xen 4.1 and > > >> 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 kernel > > > 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 "SUPPORTED_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 the > > 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 Release 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_physmap|supervisor_mode_kernel" > for PVH guests. > > b) Or dropping that altogether and introducing a new Xen elf note field, 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? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Xen 4.1 + Linux compiled with PVH == BOOM 2014-01-02 19:30 ` Konrad Rzeszutek Wilk @ 2014-01-02 21:23 ` Konrad Rzeszutek Wilk 2014-01-03 12:40 ` Roger Pau Monné 0 siblings, 1 reply; 21+ messages in thread From: Konrad Rzeszutek Wilk @ 2014-01-02 21:23 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: xen-devel, ian.campbell, george.dunlap, Andrew Cooper, David Vrabel, jbeulich, roger.pau 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 stable/pvh.v11 > > > >>> > > > >>> I get Xen 4.1 (only) hypervisor to blow up with a Linux kernel that 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 is 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 broken. > > > >> > > > >> Hypervisor/tool-side fixes aren't the correct fix here. Xen 4.1 and > > > >> 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 kernel > > > > 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 "SUPPORTED_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 the > > > 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 Release 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_physmap|supervisor_mode_kernel" > > for PVH guests. > > > > b) Or dropping that altogether and introducing a new Xen elf note field, 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 <asm/page_types.h> #include <xen/interface/elfnote.h> +#include <xen/interface/features.h> #include <asm/xen/interface.h> #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_tables|pae_pgdir_above_4gb"; .asciz PVH_FEATURES_STR) + ELFNOTE(Xen, XEN_ELFNOTE_SUPPORTED_FEATURES, .long (PVH_FEATURES) | + (1 << XENFEAT_writable_page_tables) | + (1 << XENFEAT_dom0)) ELFNOTE(Xen, XEN_ELFNOTE_PAE_MODE, .asciz "yes") ELFNOTE(Xen, XEN_ELFNOTE_LOADER, .asciz "generic") ELFNOTE(Xen, XEN_ELFNOTE_L1_MFN_VALID, diff --git a/include/xen/interface/elfnote.h b/include/xen/interface/elfnote.h index 0360b15..6f4eae3 100644 --- a/include/xen/interface/elfnote.h +++ b/include/xen/interface/elfnote.h @@ -140,6 +140,19 @@ */ #define XEN_ELFNOTE_SUSPEND_CANCEL 14 +/* + * The features supported by this kernel (numeric). + * + * 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. + * + * LEGACY: FEATURES + */ +#define XEN_ELFNOTE_SUPPORTED_FEATURES 17 + #endif /* __XEN_PUBLIC_ELFNOTE_H__ */ /* ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: Xen 4.1 + Linux compiled with PVH == BOOM 2014-01-02 21:23 ` Konrad Rzeszutek Wilk @ 2014-01-03 12:40 ` Roger Pau Monné 2014-01-03 14:35 ` Konrad Rzeszutek Wilk 0 siblings, 1 reply; 21+ messages in thread From: Roger Pau Monné @ 2014-01-03 12:40 UTC (permalink / raw) To: Konrad Rzeszutek Wilk, Konrad Rzeszutek Wilk Cc: xen-devel, ian.campbell, george.dunlap, Andrew Cooper, David Vrabel, jbeulich 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 stable/pvh.v11 >>>>>>> >>>>>>> I get Xen 4.1 (only) hypervisor to blow up with a Linux kernel that 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 is 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 broken. >>>>>> >>>>>> Hypervisor/tool-side fixes aren't the correct fix here. Xen 4.1 and >>>>>> 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 kernel >>>>> 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 "SUPPORTED_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 the >>>> 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 Release 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_physmap|supervisor_mode_kernel" >>> for PVH guests. >>> >>> b) Or dropping that altogether and introducing a new Xen elf note field, 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 <asm/page_types.h> > > #include <xen/interface/elfnote.h> > +#include <xen/interface/features.h> > #include <asm/xen/interface.h> > > #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_tables|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?) Roger. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Xen 4.1 + Linux compiled with PVH == BOOM 2014-01-03 12:40 ` Roger Pau Monné @ 2014-01-03 14:35 ` Konrad Rzeszutek Wilk 0 siblings, 0 replies; 21+ messages in thread From: Konrad Rzeszutek Wilk @ 2014-01-03 14:35 UTC (permalink / raw) To: Roger Pau Monné Cc: xen-devel, ian.campbell, george.dunlap, Andrew Cooper, David Vrabel, jbeulich, Konrad Rzeszutek Wilk On Fri, Jan 03, 2014 at 01:40:20PM +0100, Roger Pau Monné 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 stable/pvh.v11 > >>>>>>> > >>>>>>> I get Xen 4.1 (only) hypervisor to blow up with a Linux kernel that 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 is 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 broken. > >>>>>> > >>>>>> Hypervisor/tool-side fixes aren't the correct fix here. Xen 4.1 and > >>>>>> 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 kernel > >>>>> 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 "SUPPORTED_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 the > >>>> 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 Release 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_physmap|supervisor_mode_kernel" > >>> for PVH guests. > >>> > >>> b) Or dropping that altogether and introducing a new Xen elf note field, 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 <asm/page_types.h> > > > > #include <xen/interface/elfnote.h> > > +#include <xen/interface/features.h> > > #include <asm/xen/interface.h> > > > > #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_tables|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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Xen 4.1 + Linux compiled with PVH == BOOM 2013-12-30 19:56 ` Konrad Rzeszutek Wilk 2014-01-02 19:30 ` Konrad Rzeszutek Wilk @ 2014-01-03 0:27 ` Mukesh Rathor 2014-01-06 11:01 ` Ian Campbell 1 sibling, 1 reply; 21+ messages in thread From: Mukesh Rathor @ 2014-01-03 0:27 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: xen-devel, ian.campbell, george.dunlap, Andrew Cooper, David Vrabel, jbeulich, roger.pau On Mon, 30 Dec 2013 14:56:48 -0500 Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > On Tue, Dec 24, 2013 at 01:05:10PM +0000, David Vrabel wrote: ...... > > > > > > 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 > > the 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 > Release 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_physmap|supervisor_mode_kernel" > for PVH guests. I"m not sure what the state of auto xlated with shadow and Ian's work for supervisor mode kernel is. If they are obsolete, then we can drop the hvm_callback_vector check. thanks mukesh ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Xen 4.1 + Linux compiled with PVH == BOOM 2014-01-03 0:27 ` Mukesh Rathor @ 2014-01-06 11:01 ` Ian Campbell 0 siblings, 0 replies; 21+ messages in thread From: Ian Campbell @ 2014-01-06 11:01 UTC (permalink / raw) To: Mukesh Rathor Cc: xen-devel, george.dunlap, Andrew Cooper, David Vrabel, jbeulich, roger.pau On Thu, 2014-01-02 at 16:27 -0800, Mukesh Rathor wrote: > On Mon, 30 Dec 2013 14:56:48 -0500 > Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > > > On Tue, Dec 24, 2013 at 01:05:10PM +0000, David Vrabel wrote: > ...... > > > > > > > > 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 > > > the 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 > > Release 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_physmap|supervisor_mode_kernel" > > for PVH guests. > > I"m not sure what the state of auto xlated with shadow and Ian's work > for supervisor mode kernel is. If they are obsolete, then we can drop > the hvm_callback_vector check. supervisor mode kernel is a decade old stunt, I don't think it has any relevance any more -- or at least I've no intention to work on it any further and no objections to it being removed (I must confess I thought it already had been...). Ian. ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2014-01-07 13:39 UTC | newest] Thread overview: 21+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-12-20 17:57 Xen 4.1 + Linux compiled with PVH == BOOM Konrad Rzeszutek Wilk 2013-12-21 1:47 ` Mukesh Rathor 2013-12-21 11:09 ` Ian Campbell 2013-12-21 16:17 ` Konrad Rzeszutek Wilk 2013-12-23 9:37 ` Jan Beulich 2013-12-23 11:49 ` Jan Beulich 2013-12-24 1:56 ` Konrad Rzeszutek Wilk 2014-01-07 8:23 ` Jan Beulich 2014-01-07 11:31 ` Ian Campbell 2014-01-07 11:50 ` Jan Beulich 2014-01-07 13:39 ` Ian Campbell 2013-12-24 12:31 ` David Vrabel 2013-12-24 12:55 ` Andrew Cooper 2013-12-24 13:05 ` David Vrabel 2013-12-30 19:56 ` Konrad Rzeszutek Wilk 2014-01-02 19:30 ` Konrad Rzeszutek Wilk 2014-01-02 21:23 ` Konrad Rzeszutek Wilk 2014-01-03 12:40 ` Roger Pau Monné 2014-01-03 14:35 ` Konrad Rzeszutek Wilk 2014-01-03 0:27 ` Mukesh Rathor 2014-01-06 11:01 ` Ian Campbell
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.