diff mbox

x86/boot: fix MB2 header to require EFI BS

Message ID 20171024194041.28188-1-cardoe@cardoe.com (mailing list archive)
State New, archived
Headers show

Commit Message

Douglas Goldstein Oct. 24, 2017, 7:40 p.m. UTC
The EFI multiboot2 entry point currently requires EFI BootServices to
not have been exited however the header currently tells the boot
loader that Xen optionally supports EFI BootServices having been exited.
With this change Xen properly advertises that EFI must not be exited
allowing the boot loader to report an error that it cannot boot Xen if
it is unable to meet its needs.

Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
---

This should likely be applied against Xen 4.9 and Xen 4.10 as well as
staging. I am trying to get multiboot2 support for iPXE and upstream
is concerned that leaving EFI BootServices enabled will not be
compatible with their aims to support Secure Boot. So when I build
my iPXE without support for passing on Boot Services, Xen will be
loaded by iPXE but then it will fall down with "ERR: Bootloader
shutdown EFI x64 boot services!" implying that this is required. By
having Xen expose in its header that its required it allows me to
handle the situation gracefully in iPXE.

To quote the multiboot2 spec exact:

"This tag indicates that payload supports starting without terminating
boot services."

Unfortunately the spec is a bit vague and how I am reading it is:
- no tag = exit boot services in the boot loader
- tag present marked optional = boot loader can or cannot exit boot services
- tag present marked required = boot loader cannot exit boot services

In the future I would like to add support to Xen to allow it to run
without boot services but presently that support isn't there.

---
 xen/arch/x86/boot/head.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Daniel Kiper Oct. 24, 2017, 8:08 p.m. UTC | #1
On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote:
> The EFI multiboot2 entry point currently requires EFI BootServices to
> not have been exited however the header currently tells the boot
> loader that Xen optionally supports EFI BootServices having been exited.
> With this change Xen properly advertises that EFI must not be exited
> allowing the boot loader to report an error that it cannot boot Xen if
> it is unable to meet its needs.
>
> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
> ---
>
> This should likely be applied against Xen 4.9 and Xen 4.10 as well as
> staging. I am trying to get multiboot2 support for iPXE and upstream
> is concerned that leaving EFI BootServices enabled will not be
> compatible with their aims to support Secure Boot. So when I build

Hmmm... What are exact arguments for that? How do they implement e.g.
chain loading then? What about the shim support?

> my iPXE without support for passing on Boot Services, Xen will be
> loaded by iPXE but then it will fall down with "ERR: Bootloader
> shutdown EFI x64 boot services!" implying that this is required. By
> having Xen expose in its header that its required it allows me to
> handle the situation gracefully in iPXE.
>
> To quote the multiboot2 spec exact:
>
> "This tag indicates that payload supports starting without terminating
> boot services."
>
> Unfortunately the spec is a bit vague and how I am reading it is:
> - no tag = exit boot services in the boot loader
> - tag present marked optional = boot loader can or cannot exit boot services
> - tag present marked required = boot loader cannot exit boot services

NACK, please take a look at section 3.1.4, Multiboot2 information request
in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the bootloader
than you think.

> In the future I would like to add support to Xen to allow it to run
> without boot services but presently that support isn't there.

I tried that. This is difficult but not impossible. Hmmm... IIRC, some
things are impossible. Please take a look at efi_multiboot2() and you
quickly will know. Though why not try again.

Daniel
Andrew Cooper Oct. 24, 2017, 8:22 p.m. UTC | #2
On 24/10/17 21:08, Daniel Kiper wrote:
> On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote:
>> The EFI multiboot2 entry point currently requires EFI BootServices to
>> not have been exited however the header currently tells the boot
>> loader that Xen optionally supports EFI BootServices having been exited.
>> With this change Xen properly advertises that EFI must not be exited
>> allowing the boot loader to report an error that it cannot boot Xen if
>> it is unable to meet its needs.
>>
>> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
>> ---
>>
>> This should likely be applied against Xen 4.9 and Xen 4.10 as well as
>> staging. I am trying to get multiboot2 support for iPXE and upstream
>> is concerned that leaving EFI BootServices enabled will not be
>> compatible with their aims to support Secure Boot. So when I build
> Hmmm... What are exact arguments for that? How do they implement e.g.
> chain loading then? What about the shim support?
>
>> my iPXE without support for passing on Boot Services, Xen will be
>> loaded by iPXE but then it will fall down with "ERR: Bootloader
>> shutdown EFI x64 boot services!" implying that this is required. By
>> having Xen expose in its header that its required it allows me to
>> handle the situation gracefully in iPXE.
>>
>> To quote the multiboot2 spec exact:
>>
>> "This tag indicates that payload supports starting without terminating
>> boot services."
>>
>> Unfortunately the spec is a bit vague and how I am reading it is:
>> - no tag = exit boot services in the boot loader
>> - tag present marked optional = boot loader can or cannot exit boot services
>> - tag present marked required = boot loader cannot exit boot services
> NACK, please take a look at section 3.1.4, Multiboot2 information request
> in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the bootloader
> than you think.

The meaning of tag, if understood by Grub, is "don't exit boot services
before passing control".

The tag is currently marked as optional, which means Grub is free to
ignore it if it doesn't understand it, resulting in EBS being called
before passing control.

Xen cannot cope with with EBS having been called, so must not be passed
control under those circumstances.

Doug's patch marks it as non-optional which, by that section above,
requires Grub to fail with an error rather than proceeding, if it does
not understand the tag.


By my reading, Doug's patch looks correct.

How does your interpretation of the spec differ?

~Andrew
Douglas Goldstein Oct. 24, 2017, 8:28 p.m. UTC | #3
On 10/24/17 3:08 PM, Daniel Kiper wrote:
> On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote:
>> The EFI multiboot2 entry point currently requires EFI BootServices to
>> not have been exited however the header currently tells the boot
>> loader that Xen optionally supports EFI BootServices having been exited.
>> With this change Xen properly advertises that EFI must not be exited
>> allowing the boot loader to report an error that it cannot boot Xen if
>> it is unable to meet its needs.
>>
>> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
>> ---
>>
>> This should likely be applied against Xen 4.9 and Xen 4.10 as well as
>> staging. I am trying to get multiboot2 support for iPXE and upstream
>> is concerned that leaving EFI BootServices enabled will not be
>> compatible with their aims to support Secure Boot. So when I build
> 
> Hmmm... What are exact arguments for that? How do they implement e.g.
> chain loading then? What about the shim support?

Look they have concerns about it. As we've talked about this in the past
and I encourage you communicate with them. You are the author of the
multiboot2 spec. I'm just trying to do my best to PXE boot Xen on EFI
systems and make all upstreams (Xen & iPXE) happy.

>>
>> Unfortunately the spec is a bit vague and how I am reading it is:
>> - no tag = exit boot services in the boot loader
>> - tag present marked optional = boot loader can or cannot exit boot services
>> - tag present marked required = boot loader cannot exit boot services
> 
> NACK, please take a look at section 3.1.4, Multiboot2 information request
> in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the bootloader
> than you think.
> 

I still don't see any issue with my interpretation based on what you
pointed me to. There's a hole here with what Xen asks for of the boot
loader to do.

The boot loader is told that Xen optionally supports the boot loader not
exiting boot services when in fact Xen requires the boot loader to not
exit boot services. Somehow we need to convey this to the boot loader.
Douglas Goldstein Oct. 24, 2017, 8:34 p.m. UTC | #4
On 10/24/17 3:08 PM, Daniel Kiper wrote:
> On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote:
>>
>> Unfortunately the spec is a bit vague and how I am reading it is:
>> - no tag = exit boot services in the boot loader
>> - tag present marked optional = boot loader can or cannot exit boot services
>> - tag present marked required = boot loader cannot exit boot services
> 
> NACK, please take a look at section 3.1.4, Multiboot2 information request
> in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the bootloader
> than you think.

In fact since there is some confusion here then could you possibly
expand some of the sections with examples to help clarify?
Douglas Goldstein Oct. 24, 2017, 8:49 p.m. UTC | #5
On 10/24/17 3:22 PM, Andrew Cooper wrote:
> On 24/10/17 21:08, Daniel Kiper wrote:
>> On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote:
>>> The EFI multiboot2 entry point currently requires EFI BootServices to
>>> not have been exited however the header currently tells the boot
>>> loader that Xen optionally supports EFI BootServices having been exited.
>>> With this change Xen properly advertises that EFI must not be exited
>>> allowing the boot loader to report an error that it cannot boot Xen if
>>> it is unable to meet its needs.
>>>
>>> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
>>> ---
>>>
>>> This should likely be applied against Xen 4.9 and Xen 4.10 as well as
>>> staging. I am trying to get multiboot2 support for iPXE and upstream
>>> is concerned that leaving EFI BootServices enabled will not be
>>> compatible with their aims to support Secure Boot. So when I build
>> Hmmm... What are exact arguments for that? How do they implement e.g.
>> chain loading then? What about the shim support?
>>
>>> my iPXE without support for passing on Boot Services, Xen will be
>>> loaded by iPXE but then it will fall down with "ERR: Bootloader
>>> shutdown EFI x64 boot services!" implying that this is required. By
>>> having Xen expose in its header that its required it allows me to
>>> handle the situation gracefully in iPXE.
>>>
>>> To quote the multiboot2 spec exact:
>>>
>>> "This tag indicates that payload supports starting without terminating
>>> boot services."
>>>
>>> Unfortunately the spec is a bit vague and how I am reading it is:
>>> - no tag = exit boot services in the boot loader
>>> - tag present marked optional = boot loader can or cannot exit boot services
>>> - tag present marked required = boot loader cannot exit boot services
>> NACK, please take a look at section 3.1.4, Multiboot2 information request
>> in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the bootloader
>> than you think.
> 
> The meaning of tag, if understood by Grub, is "don't exit boot services
> before passing control".
> 
> The tag is currently marked as optional, which means Grub is free to
> ignore it if it doesn't understand it, resulting in EBS being called
> before passing control.
> 
> Xen cannot cope with with EBS having been called, so must not be passed
> control under those circumstances.
> 
> Doug's patch marks it as non-optional which, by that section above,
> requires Grub to fail with an error rather than proceeding, if it does
> not understand the tag.
> 
> 
> By my reading, Doug's patch looks correct.
> 
> How does your interpretation of the spec differ?
> 
> ~Andrew
> 

So I've been sitting here reading it for a bit. I'm guessing what Daniel
is arguing is that the spec says that the boot loader MUST understand a
tag if its marked as required and does not have to understand it if its
marked as optional. The next sentence then seems to be a total escape
hatch because it seems to imply that the boot loader doesn't have to
respect any tag regardless of its required or optional settings. Which
seems to defeat the purpose of having any info requests at all. And
results in no guarantees that if your binary requires something that it
will get it before being executed. And therefore requires a binary to
support all cases always.

If that's truly the case you are arguing for Daniel then this whole spec
really has too big of a loophole to be safely considered as useful. I
know that's a bit harsh but as more tags are added over time the matrix
of required support will snowball.
Daniel Kiper Oct. 24, 2017, 9:11 p.m. UTC | #6
On Tue, Oct 24, 2017 at 09:22:20PM +0100, Andrew Cooper wrote:
> On 24/10/17 21:08, Daniel Kiper wrote:
> > On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote:
> >> The EFI multiboot2 entry point currently requires EFI BootServices to
> >> not have been exited however the header currently tells the boot
> >> loader that Xen optionally supports EFI BootServices having been exited.
> >> With this change Xen properly advertises that EFI must not be exited
> >> allowing the boot loader to report an error that it cannot boot Xen if
> >> it is unable to meet its needs.
> >>
> >> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
> >> ---
> >>
> >> This should likely be applied against Xen 4.9 and Xen 4.10 as well as
> >> staging. I am trying to get multiboot2 support for iPXE and upstream
> >> is concerned that leaving EFI BootServices enabled will not be
> >> compatible with their aims to support Secure Boot. So when I build
> > Hmmm... What are exact arguments for that? How do they implement e.g.
> > chain loading then? What about the shim support?
> >
> >> my iPXE without support for passing on Boot Services, Xen will be
> >> loaded by iPXE but then it will fall down with "ERR: Bootloader
> >> shutdown EFI x64 boot services!" implying that this is required. By
> >> having Xen expose in its header that its required it allows me to
> >> handle the situation gracefully in iPXE.
> >>
> >> To quote the multiboot2 spec exact:
> >>
> >> "This tag indicates that payload supports starting without terminating
> >> boot services."
> >>
> >> Unfortunately the spec is a bit vague and how I am reading it is:
> >> - no tag = exit boot services in the boot loader
> >> - tag present marked optional = boot loader can or cannot exit boot services
> >> - tag present marked required = boot loader cannot exit boot services
> > NACK, please take a look at section 3.1.4, Multiboot2 information request
> > in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the bootloader
> > than you think.
>
> The meaning of tag, if understood by Grub, is "don't exit boot services
> before passing control".

Yep.

> The tag is currently marked as optional, which means Grub is free to
> ignore it if it doesn't understand it, resulting in EBS being called
> before passing control.

Yep, but you are forgetting about legacy BIOS platforms with old GRUB2.
Right now it is possible to boot Xen via Multiboot2 in such configs.
If you set this flag to REQUIRED then old GRUB2 on BIOS platforms will
not boot Xen in such cases. If we do not care about that then OK. However,
then I would request additional line or so to the commit message saying
that such configs are broken deliberately because...

> Xen cannot cope with with EBS having been called, so must not be passed
> control under those circumstances.

Even with REQUIRED flag there is no guarantee that booloader will do
what Xen asks. So, it has to check the boot services presence anyway.
And it does.

Daniel
Andrew Cooper Oct. 24, 2017, 9:40 p.m. UTC | #7
On 24/10/2017 22:11, Daniel Kiper wrote:
> On Tue, Oct 24, 2017 at 09:22:20PM +0100, Andrew Cooper wrote:
>> On 24/10/17 21:08, Daniel Kiper wrote:
>>> On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote:
>>>> The EFI multiboot2 entry point currently requires EFI BootServices to
>>>> not have been exited however the header currently tells the boot
>>>> loader that Xen optionally supports EFI BootServices having been exited.
>>>> With this change Xen properly advertises that EFI must not be exited
>>>> allowing the boot loader to report an error that it cannot boot Xen if
>>>> it is unable to meet its needs.
>>>>
>>>> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
>>>> ---
>>>>
>>>> This should likely be applied against Xen 4.9 and Xen 4.10 as well as
>>>> staging. I am trying to get multiboot2 support for iPXE and upstream
>>>> is concerned that leaving EFI BootServices enabled will not be
>>>> compatible with their aims to support Secure Boot. So when I build
>>> Hmmm... What are exact arguments for that? How do they implement e.g.
>>> chain loading then? What about the shim support?
>>>
>>>> my iPXE without support for passing on Boot Services, Xen will be
>>>> loaded by iPXE but then it will fall down with "ERR: Bootloader
>>>> shutdown EFI x64 boot services!" implying that this is required. By
>>>> having Xen expose in its header that its required it allows me to
>>>> handle the situation gracefully in iPXE.
>>>>
>>>> To quote the multiboot2 spec exact:
>>>>
>>>> "This tag indicates that payload supports starting without terminating
>>>> boot services."
>>>>
>>>> Unfortunately the spec is a bit vague and how I am reading it is:
>>>> - no tag = exit boot services in the boot loader
>>>> - tag present marked optional = boot loader can or cannot exit boot services
>>>> - tag present marked required = boot loader cannot exit boot services
>>> NACK, please take a look at section 3.1.4, Multiboot2 information request
>>> in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the bootloader
>>> than you think.
>> The meaning of tag, if understood by Grub, is "don't exit boot services
>> before passing control".
> Yep.
>
>> The tag is currently marked as optional, which means Grub is free to
>> ignore it if it doesn't understand it, resulting in EBS being called
>> before passing control.
> Yep, but you are forgetting about legacy BIOS platforms with old GRUB2.
> Right now it is possible to boot Xen via Multiboot2 in such configs.
> If you set this flag to REQUIRED then old GRUB2 on BIOS platforms will
> not boot Xen in such cases. If we do not care about that then OK. However,
> then I would request additional line or so to the commit message saying
> that such configs are broken deliberately because...

Such older cases wouldn't boot either, because Xen would bail out saying
"I didn't retain BS like I need".

>
>> Xen cannot cope with with EBS having been called, so must not be passed
>> control under those circumstances.
> Even with REQUIRED flag there is no guarantee that booloader will do
> what Xen asks. So, it has to check the boot services presence anyway.
> And it does.

Indeed, and rightly so.

The difference between Grub bailing out with an error, and Xen bailing
out with an error, is that one still leaves you at a grub prompt, while
one locks your system up until you remove some electrons from it.

Setting the REQUIRED flag appears to be strictly better behaviour from
the users point of view.

~Andrew
Daniel Kiper Oct. 24, 2017, 9:40 p.m. UTC | #8
On Tue, Oct 24, 2017 at 03:28:52PM -0500, Doug Goldstein wrote:
> On 10/24/17 3:08 PM, Daniel Kiper wrote:
> > On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote:
> >> The EFI multiboot2 entry point currently requires EFI BootServices to
> >> not have been exited however the header currently tells the boot
> >> loader that Xen optionally supports EFI BootServices having been exited.
> >> With this change Xen properly advertises that EFI must not be exited
> >> allowing the boot loader to report an error that it cannot boot Xen if
> >> it is unable to meet its needs.
> >>
> >> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
> >> ---
> >>
> >> This should likely be applied against Xen 4.9 and Xen 4.10 as well as
> >> staging. I am trying to get multiboot2 support for iPXE and upstream
> >> is concerned that leaving EFI BootServices enabled will not be
> >> compatible with their aims to support Secure Boot. So when I build
> >
> > Hmmm... What are exact arguments for that? How do they implement e.g.
> > chain loading then? What about the shim support?
>
> Look they have concerns about it. As we've talked about this in the past

If I do something I like to know why I have to do it.

> and I encourage you communicate with them. You are the author of the

I remember but, sorry, IIRC, I heard just only vague statements like that.
So, I would like to know exact reasons finally. And I hoped that they told
you more then simple "NO".

> multiboot2 spec. I'm just trying to do my best to PXE boot Xen on EFI
> systems and make all upstreams (Xen & iPXE) happy.

Once again, I am happy to help. Though I have to know why I have to do
this or that. No more no less.

> >> Unfortunately the spec is a bit vague and how I am reading it is:
> >> - no tag = exit boot services in the boot loader
> >> - tag present marked optional = boot loader can or cannot exit boot services
> >> - tag present marked required = boot loader cannot exit boot services
> >
> > NACK, please take a look at section 3.1.4, Multiboot2 information request
> > in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the bootloader
> > than you think.
>
> I still don't see any issue with my interpretation based on what you
> pointed me to. There's a hole here with what Xen asks for of the boot
> loader to do.
>
> The boot loader is told that Xen optionally supports the boot loader not
> exiting boot services when in fact Xen requires the boot loader to not
> exit boot services. Somehow we need to convey this to the boot loader.

Sorry, maybe I was too vague this time. Please look at my replay to Andrew.
It should help.

Daniel
Daniel Kiper Oct. 24, 2017, 10:16 p.m. UTC | #9
On Tue, Oct 24, 2017 at 03:49:10PM -0500, Doug Goldstein wrote:
> On 10/24/17 3:22 PM, Andrew Cooper wrote:
> > On 24/10/17 21:08, Daniel Kiper wrote:
> >> On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote:
> >>> The EFI multiboot2 entry point currently requires EFI BootServices to
> >>> not have been exited however the header currently tells the boot
> >>> loader that Xen optionally supports EFI BootServices having been exited.
> >>> With this change Xen properly advertises that EFI must not be exited
> >>> allowing the boot loader to report an error that it cannot boot Xen if
> >>> it is unable to meet its needs.
> >>>
> >>> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
> >>> ---
> >>>
> >>> This should likely be applied against Xen 4.9 and Xen 4.10 as well as
> >>> staging. I am trying to get multiboot2 support for iPXE and upstream
> >>> is concerned that leaving EFI BootServices enabled will not be
> >>> compatible with their aims to support Secure Boot. So when I build
> >> Hmmm... What are exact arguments for that? How do they implement e.g.
> >> chain loading then? What about the shim support?
> >>
> >>> my iPXE without support for passing on Boot Services, Xen will be
> >>> loaded by iPXE but then it will fall down with "ERR: Bootloader
> >>> shutdown EFI x64 boot services!" implying that this is required. By
> >>> having Xen expose in its header that its required it allows me to
> >>> handle the situation gracefully in iPXE.
> >>>
> >>> To quote the multiboot2 spec exact:
> >>>
> >>> "This tag indicates that payload supports starting without terminating
> >>> boot services."
> >>>
> >>> Unfortunately the spec is a bit vague and how I am reading it is:
> >>> - no tag = exit boot services in the boot loader
> >>> - tag present marked optional = boot loader can or cannot exit boot services
> >>> - tag present marked required = boot loader cannot exit boot services
> >> NACK, please take a look at section 3.1.4, Multiboot2 information request
> >> in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the bootloader
> >> than you think.
> >
> > The meaning of tag, if understood by Grub, is "don't exit boot services
> > before passing control".
> >
> > The tag is currently marked as optional, which means Grub is free to
> > ignore it if it doesn't understand it, resulting in EBS being called
> > before passing control.
> >
> > Xen cannot cope with with EBS having been called, so must not be passed
> > control under those circumstances.
> >
> > Doug's patch marks it as non-optional which, by that section above,
> > requires Grub to fail with an error rather than proceeding, if it does
> > not understand the tag.
> >
> >
> > By my reading, Doug's patch looks correct.
> >
> > How does your interpretation of the spec differ?
> >
> > ~Andrew
> >
>
> So I've been sitting here reading it for a bit. I'm guessing what Daniel
> is arguing is that the spec says that the boot loader MUST understand a
> tag if its marked as required and does not have to understand it if its
> marked as optional. The next sentence then seems to be a total escape
> hatch because it seems to imply that the boot loader doesn't have to
> respect any tag regardless of its required or optional settings. Which
> seems to defeat the purpose of having any info requests at all. And
> results in no guarantees that if your binary requires something that it
> will get it before being executed. And therefore requires a binary to
> support all cases always.

Sadly you are right here.

> If that's truly the case you are arguing for Daniel then this whole spec
> really has too big of a loophole to be safely considered as useful. I
> know that's a bit harsh but as more tags are added over time the matrix
> of required support will snowball.

I think that we can use another bit from flags and if it set then
interpret it as: I (image/OS/Xen/...) have to have this data. If
you (the booloader) are not able to provide it then do not start
me and fail nicely, e.g. display user prompt. This bit should be
checked only if current bit is in REQUIRED state. Otherwise, if new
bit is in REQUIRED state and current one is in OPTIONAL state then
the bootloader should complain. This way older GRUB will fail if it
sees an unsupported option and newer one will always provide required
data to the loaded image. And by the way, we should check unused
flags bits for 1. If one of it is set then the bootloader should fail.
Right now at least GRUB2 does not do that.

Does it make sense?

Daniel
Daniel Kiper Oct. 24, 2017, 10:20 p.m. UTC | #10
On Tue, Oct 24, 2017 at 10:40:26PM +0100, Andrew Cooper wrote:
> On 24/10/2017 22:11, Daniel Kiper wrote:
> > On Tue, Oct 24, 2017 at 09:22:20PM +0100, Andrew Cooper wrote:
> >> On 24/10/17 21:08, Daniel Kiper wrote:
> >>> On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote:
> >>>> The EFI multiboot2 entry point currently requires EFI BootServices to
> >>>> not have been exited however the header currently tells the boot
> >>>> loader that Xen optionally supports EFI BootServices having been exited.
> >>>> With this change Xen properly advertises that EFI must not be exited
> >>>> allowing the boot loader to report an error that it cannot boot Xen if
> >>>> it is unable to meet its needs.
> >>>>
> >>>> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
> >>>> ---
> >>>>
> >>>> This should likely be applied against Xen 4.9 and Xen 4.10 as well as
> >>>> staging. I am trying to get multiboot2 support for iPXE and upstream
> >>>> is concerned that leaving EFI BootServices enabled will not be
> >>>> compatible with their aims to support Secure Boot. So when I build
> >>> Hmmm... What are exact arguments for that? How do they implement e.g.
> >>> chain loading then? What about the shim support?
> >>>
> >>>> my iPXE without support for passing on Boot Services, Xen will be
> >>>> loaded by iPXE but then it will fall down with "ERR: Bootloader
> >>>> shutdown EFI x64 boot services!" implying that this is required. By
> >>>> having Xen expose in its header that its required it allows me to
> >>>> handle the situation gracefully in iPXE.
> >>>>
> >>>> To quote the multiboot2 spec exact:
> >>>>
> >>>> "This tag indicates that payload supports starting without terminating
> >>>> boot services."
> >>>>
> >>>> Unfortunately the spec is a bit vague and how I am reading it is:
> >>>> - no tag = exit boot services in the boot loader
> >>>> - tag present marked optional = boot loader can or cannot exit boot services
> >>>> - tag present marked required = boot loader cannot exit boot services
> >>> NACK, please take a look at section 3.1.4, Multiboot2 information request
> >>> in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the bootloader
> >>> than you think.
> >> The meaning of tag, if understood by Grub, is "don't exit boot services
> >> before passing control".
> > Yep.
> >
> >> The tag is currently marked as optional, which means Grub is free to
> >> ignore it if it doesn't understand it, resulting in EBS being called
> >> before passing control.
> > Yep, but you are forgetting about legacy BIOS platforms with old GRUB2.
> > Right now it is possible to boot Xen via Multiboot2 in such configs.
> > If you set this flag to REQUIRED then old GRUB2 on BIOS platforms will
> > not boot Xen in such cases. If we do not care about that then OK. However,
> > then I would request additional line or so to the commit message saying
> > that such configs are broken deliberately because...
>
> Such older cases wouldn't boot either, because Xen would bail out saying
> "I didn't retain BS like I need".

Nope, you should remember that legacy entry point (__start) will be used then.

> >> Xen cannot cope with with EBS having been called, so must not be passed
> >> control under those circumstances.
> > Even with REQUIRED flag there is no guarantee that booloader will do
> > what Xen asks. So, it has to check the boot services presence anyway.
> > And it does.
>
> Indeed, and rightly so.
>
> The difference between Grub bailing out with an error, and Xen bailing
> out with an error, is that one still leaves you at a grub prompt, while
> one locks your system up until you remove some electrons from it.
>
> Setting the REQUIRED flag appears to be strictly better behaviour from
> the users point of view.

Yep, I put a solution proposal for that issue in other email.

Daniel
Douglas Goldstein Oct. 25, 2017, 1:52 p.m. UTC | #11
On 10/24/17 5:20 PM, Daniel Kiper wrote:
> On Tue, Oct 24, 2017 at 10:40:26PM +0100, Andrew Cooper wrote:
>> On 24/10/2017 22:11, Daniel Kiper wrote:
>>> On Tue, Oct 24, 2017 at 09:22:20PM +0100, Andrew Cooper wrote:
>>>> On 24/10/17 21:08, Daniel Kiper wrote:
>>>>> On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote:
>>>>>> The EFI multiboot2 entry point currently requires EFI BootServices to
>>>>>> not have been exited however the header currently tells the boot
>>>>>> loader that Xen optionally supports EFI BootServices having been exited.
>>>>>> With this change Xen properly advertises that EFI must not be exited
>>>>>> allowing the boot loader to report an error that it cannot boot Xen if
>>>>>> it is unable to meet its needs.
>>>>>>
>>>>>> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
>>>>>> ---
>>>>>>
>>>>>> This should likely be applied against Xen 4.9 and Xen 4.10 as well as
>>>>>> staging. I am trying to get multiboot2 support for iPXE and upstream
>>>>>> is concerned that leaving EFI BootServices enabled will not be
>>>>>> compatible with their aims to support Secure Boot. So when I build
>>>>> Hmmm... What are exact arguments for that? How do they implement e.g.
>>>>> chain loading then? What about the shim support?
>>>>>
>>>>>> my iPXE without support for passing on Boot Services, Xen will be
>>>>>> loaded by iPXE but then it will fall down with "ERR: Bootloader
>>>>>> shutdown EFI x64 boot services!" implying that this is required. By
>>>>>> having Xen expose in its header that its required it allows me to
>>>>>> handle the situation gracefully in iPXE.
>>>>>>
>>>>>> To quote the multiboot2 spec exact:
>>>>>>
>>>>>> "This tag indicates that payload supports starting without terminating
>>>>>> boot services."
>>>>>>
>>>>>> Unfortunately the spec is a bit vague and how I am reading it is:
>>>>>> - no tag = exit boot services in the boot loader
>>>>>> - tag present marked optional = boot loader can or cannot exit boot services
>>>>>> - tag present marked required = boot loader cannot exit boot services
>>>>> NACK, please take a look at section 3.1.4, Multiboot2 information request
>>>>> in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the bootloader
>>>>> than you think.
>>>> The meaning of tag, if understood by Grub, is "don't exit boot services
>>>> before passing control".
>>> Yep.
>>>
>>>> The tag is currently marked as optional, which means Grub is free to
>>>> ignore it if it doesn't understand it, resulting in EBS being called
>>>> before passing control.
>>> Yep, but you are forgetting about legacy BIOS platforms with old GRUB2.
>>> Right now it is possible to boot Xen via Multiboot2 in such configs.
>>> If you set this flag to REQUIRED then old GRUB2 on BIOS platforms will
>>> not boot Xen in such cases. If we do not care about that then OK. However,
>>> then I would request additional line or so to the commit message saying
>>> that such configs are broken deliberately because...
>>
>> Such older cases wouldn't boot either, because Xen would bail out saying
>> "I didn't retain BS like I need".
> 
> Nope, you should remember that legacy entry point (__start) will be used then.

But based on what you said the spec says the boot loader only has to
understand the tag and not actually do anything with it. So marking it
required wouldn't affect anything cause Grub would understand the tag in
the BIOS boot case and not do anything with it.
diff mbox

Patch

diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index 9cc35da558..f76c2c0664 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -98,8 +98,8 @@  multiboot2_header_start:
                    0, /* Number of the lines - no preference. */ \
                    0  /* Number of bits per pixel - no preference. */
 
-        /* Request that ExitBootServices() not be called. */
-        mb2ht_init MB2_HT(EFI_BS), MB2_HT(OPTIONAL)
+        /* Require that ExitBootServices() not be called. */
+        mb2ht_init MB2_HT(EFI_BS), MB2_HT(REQUIRED)
 
         /* EFI64 Multiboot2 entry point. */
         mb2ht_init MB2_HT(ENTRY_ADDRESS_EFI64), MB2_HT(OPTIONAL), \