Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Andrew Cooper]

On 07/08/2019 14:26, Marek Marczykowski-Górecki wrote:
> Hi,
>
> Xen 4.12 crashes when booting on UEFI (with multiboot2) unless I disable
> runtime services. The crash happens shortly after starting dom0 kernel.
> Unfortunately I don't have serial console there, so the only log I have
> is a photo of VGA console (attached). Below I retype part of the message:
>
> (XEN) ----[ Xen-4.12.0-3.fc29  x86_64  debug=n   Not tainted ]----
> (XEN) CPU:    0
> (XEN) RIP:    e008:[<00000000000000f6>] 00000000000000f6
> (XEN) RFLAGS: 0000000000010287   CONTEXT: hypervisor (d0v0)
> ...
> (XEN) Xen call trace:
> (XEN)    [<00000000000000f6>] 00000000000000f6
> (XEN)    [<ffff82d08026c6ad>] flushtlb.c#pre_flush+0x3d/0x80
> (XEN)    [                  ] efi_runtime_call+0x493/0xbd0
> (XEN)    [                  ] efi_runtime_call+0x441/0xbd0
> (XEN)    [                  ] vcpu_restore_fpu_nonlazy+0xe7/0x180
> (XEN)    [                  ] do_platform_op+0/0x1880
> (XEN)    [                  ] do_platform_op+0xb9c/0x1880
> (XEN)    [                  ] do_platform_op+0xb9c/0x1880
> (XEN)    [                  ] sched_credit2.c#csched2_schedule+0xcd0/0x13a0
> (XEN)    [                  ] lstar_enter+0xae/0x120
> (XEN)    [                  ] do_platform_op+0/0x1880
> (XEN)    [                  ] pv_hypercall+0x152/0x220
> (XEN)    [                  ] lstar_enter+0xae/0x120
> (XEN)    [                  ] lstar_enter+0xa2/0x120
> (XEN)    [                  ] lstar_enter+0xae/0x120
> (XEN)    [                  ] lstar_enter+0xa2/0x120
> (XEN)    [                  ] lstar_enter+0xae/0x120
> (XEN)    [                  ] lstar_enter+0xa2/0x120
> (XEN)    [                  ] lstar_enter+0xae/0x120
> (XEN)    [                  ] lstar_enter+0xa2/0x120
> (XEN)    [                  ] lstar_enter+0xae/0x120
> (XEN)    [                  ] lstar_enter+0xa2/0x120
> (XEN)    [                  ] lstar_enter+0xae/0x120
> (XEN)    [                  ] lstar_enter+0x10c/0x120
> (XEN)
> (XEN)
> (XEN) *****************************************
> (XEN) Panic on CPU 0:
> (XEN) FATAL TRAP: vector = 0 (divide error)
> (XEN) [error_code=0000]
> (XEN) *****************************************
>
> Any idea?

Very weird.

You got a #DE because the instruction under %rip is `div %bh`, but
judging from the photo, that page is poisoned anyway.

The chances are that something jumped to 0 and has executed it this far
through the poisoned page before actually faulting.

Can you disassemble pre_flush() ?  I don't see anything interesting at
the C level.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Jan Beulich]

On 07.08.2019 15:26, Marek Marczykowski-Górecki  wrote:
> Hi,
> 
> Xen 4.12 crashes when booting on UEFI (with multiboot2) unless I disable
> runtime services. The crash happens shortly after starting dom0 kernel.
> Unfortunately I don't have serial console there, so the only log I have
> is a photo of VGA console (attached). Below I retype part of the message:
> 
> (XEN) ----[ Xen-4.12.0-3.fc29  x86_64  debug=n   Not tainted ]----
> (XEN) CPU:    0
> (XEN) RIP:    e008:[<00000000000000f6>] 00000000000000f6
> (XEN) RFLAGS: 0000000000010287   CONTEXT: hypervisor (d0v0)
> ...
> (XEN) Xen call trace:
> (XEN)    [<00000000000000f6>] 00000000000000f6
> (XEN)    [<ffff82d08026c6ad>] flushtlb.c#pre_flush+0x3d/0x80
> (XEN)    [                  ] efi_runtime_call+0x493/0xbd0
> (XEN)    [                  ] efi_runtime_call+0x441/0xbd0
> (XEN)    [                  ] vcpu_restore_fpu_nonlazy+0xe7/0x180
> (XEN)    [                  ] do_platform_op+0/0x1880
> (XEN)    [                  ] do_platform_op+0xb9c/0x1880
> (XEN)    [                  ] do_platform_op+0xb9c/0x1880
> (XEN)    [                  ] sched_credit2.c#csched2_schedule+0xcd0/0x13a0
> (XEN)    [                  ] lstar_enter+0xae/0x120
> (XEN)    [                  ] do_platform_op+0/0x1880
> (XEN)    [                  ] pv_hypercall+0x152/0x220
> (XEN)    [                  ] lstar_enter+0xae/0x120
> (XEN)    [                  ] lstar_enter+0xa2/0x120
> (XEN)    [                  ] lstar_enter+0xae/0x120
> (XEN)    [                  ] lstar_enter+0xa2/0x120
> (XEN)    [                  ] lstar_enter+0xae/0x120
> (XEN)    [                  ] lstar_enter+0xa2/0x120
> (XEN)    [                  ] lstar_enter+0xae/0x120
> (XEN)    [                  ] lstar_enter+0xa2/0x120
> (XEN)    [                  ] lstar_enter+0xae/0x120
> (XEN)    [                  ] lstar_enter+0xa2/0x120
> (XEN)    [                  ] lstar_enter+0xae/0x120
> (XEN)    [                  ] lstar_enter+0x10c/0x120
> (XEN)
> (XEN)
> (XEN) *****************************************
> (XEN) Panic on CPU 0:
> (XEN) FATAL TRAP: vector = 0 (divide error)
> (XEN) [error_code=0000]
> (XEN) *****************************************
> 
> Any idea? Lack of serial console doesn't help with collecting logs...

May I suggest that you repeat this with a debug hypervisor, such that
the call trace becomes more usable/sensible? I think, for example,
that the pre_flush() that caught Andrew's eye is a red herring, and
that instead a call through NULL has happened in e.g.
efi_runtime_call().

Of course trying to capture more of Xen's boot log (in particular
the EFI memory map) may be helpful, too, if you could manage to do
so.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Jan Beulich]

On 07.08.2019 17:17, Marek Marczykowski-Górecki  wrote:
> On Wed, Aug 07, 2019 at 04:45:43PM +0200, Jan Beulich wrote:
>> On 07.08.2019 15:26, Marek Marczykowski-Górecki  wrote:
>>> Hi,
>>>
>>> Xen 4.12 crashes when booting on UEFI (with multiboot2) unless I disable
>>> runtime services. The crash happens shortly after starting dom0 kernel.
>>> Unfortunately I don't have serial console there, so the only log I have
>>> is a photo of VGA console (attached). Below I retype part of the message:
>>>
>>> (XEN) ----[ Xen-4.12.0-3.fc29  x86_64  debug=n   Not tainted ]----
>>> (XEN) CPU:    0
>>> (XEN) RIP:    e008:[<00000000000000f6>] 00000000000000f6
>>> (XEN) RFLAGS: 0000000000010287   CONTEXT: hypervisor (d0v0)
>>> ...
>>> (XEN) Xen call trace:
>>> (XEN)    [<00000000000000f6>] 00000000000000f6
>>> (XEN)    [<ffff82d08026c6ad>] flushtlb.c#pre_flush+0x3d/0x80
>>> (XEN)    [                  ] efi_runtime_call+0x493/0xbd0
>>> (XEN)    [                  ] efi_runtime_call+0x441/0xbd0
>>> (XEN)    [                  ] vcpu_restore_fpu_nonlazy+0xe7/0x180
>>> (XEN)    [                  ] do_platform_op+0/0x1880
>>> (XEN)    [                  ] do_platform_op+0xb9c/0x1880
>>> (XEN)    [                  ] do_platform_op+0xb9c/0x1880
>>> (XEN)    [                  ] sched_credit2.c#csched2_schedule+0xcd0/0x13a0
>>> (XEN)    [                  ] lstar_enter+0xae/0x120
>>> (XEN)    [                  ] do_platform_op+0/0x1880
>>> (XEN)    [                  ] pv_hypercall+0x152/0x220
>>> (XEN)    [                  ] lstar_enter+0xae/0x120
>>> (XEN)    [                  ] lstar_enter+0xa2/0x120
>>> (XEN)    [                  ] lstar_enter+0xae/0x120
>>> (XEN)    [                  ] lstar_enter+0xa2/0x120
>>> (XEN)    [                  ] lstar_enter+0xae/0x120
>>> (XEN)    [                  ] lstar_enter+0xa2/0x120
>>> (XEN)    [                  ] lstar_enter+0xae/0x120
>>> (XEN)    [                  ] lstar_enter+0xa2/0x120
>>> (XEN)    [                  ] lstar_enter+0xae/0x120
>>> (XEN)    [                  ] lstar_enter+0xa2/0x120
>>> (XEN)    [                  ] lstar_enter+0xae/0x120
>>> (XEN)    [                  ] lstar_enter+0x10c/0x120
>>> (XEN)
>>> (XEN)
>>> (XEN) *****************************************
>>> (XEN) Panic on CPU 0:
>>> (XEN) FATAL TRAP: vector = 0 (divide error)
>>> (XEN) [error_code=0000]
>>> (XEN) *****************************************
>>>
>>> Any idea? Lack of serial console doesn't help with collecting logs...
>>
>> May I suggest that you repeat this with a debug hypervisor, such that
>> the call trace becomes more usable/sensible?
> 
> Actually, I've already tried, but every other build I try, I get even
> less useful call trace, for example debug unstable build:
> 
>      Xen call trace:
>         [<000000007b751c09>] 000000007b751c09
>         [<8c2b0398e0000daa>] 8c2b0398e0000daa
> ...
>      GENERAL PROTECTION FAULT
>      [error_code=0000]

But this makes reasonable sense: The faulting insn is "call *0x18(%rax)"
and %rax is 6954b2b0, which points into a block of type
EfiBootServicesData (and hence likely reused by the time of the crash
for other purposes). Hence the "/mapbs" option of xen.efi might help
here too, but iirc there's no equivalent for it in the MB2 case.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 3998 bytes --]

On Wed, Aug 07, 2019 at 05:33:05PM +0200, Jan Beulich wrote:
> On 07.08.2019 17:17, Marek Marczykowski-Górecki  wrote:
> > On Wed, Aug 07, 2019 at 04:45:43PM +0200, Jan Beulich wrote:
> > > On 07.08.2019 15:26, Marek Marczykowski-Górecki  wrote:
> > > > Hi,
> > > > 
> > > > Xen 4.12 crashes when booting on UEFI (with multiboot2) unless I disable
> > > > runtime services. The crash happens shortly after starting dom0 kernel.
> > > > Unfortunately I don't have serial console there, so the only log I have
> > > > is a photo of VGA console (attached). Below I retype part of the message:
> > > > 
> > > > (XEN) ----[ Xen-4.12.0-3.fc29  x86_64  debug=n   Not tainted ]----
> > > > (XEN) CPU:    0
> > > > (XEN) RIP:    e008:[<00000000000000f6>] 00000000000000f6
> > > > (XEN) RFLAGS: 0000000000010287   CONTEXT: hypervisor (d0v0)
> > > > ...
> > > > (XEN) Xen call trace:
> > > > (XEN)    [<00000000000000f6>] 00000000000000f6
> > > > (XEN)    [<ffff82d08026c6ad>] flushtlb.c#pre_flush+0x3d/0x80
> > > > (XEN)    [                  ] efi_runtime_call+0x493/0xbd0
> > > > (XEN)    [                  ] efi_runtime_call+0x441/0xbd0
> > > > (XEN)    [                  ] vcpu_restore_fpu_nonlazy+0xe7/0x180
> > > > (XEN)    [                  ] do_platform_op+0/0x1880
> > > > (XEN)    [                  ] do_platform_op+0xb9c/0x1880
> > > > (XEN)    [                  ] do_platform_op+0xb9c/0x1880
> > > > (XEN)    [                  ] sched_credit2.c#csched2_schedule+0xcd0/0x13a0
> > > > (XEN)    [                  ] lstar_enter+0xae/0x120
> > > > (XEN)    [                  ] do_platform_op+0/0x1880
> > > > (XEN)    [                  ] pv_hypercall+0x152/0x220
> > > > (XEN)    [                  ] lstar_enter+0xae/0x120
> > > > (XEN)    [                  ] lstar_enter+0xa2/0x120
> > > > (XEN)    [                  ] lstar_enter+0xae/0x120
> > > > (XEN)    [                  ] lstar_enter+0xa2/0x120
> > > > (XEN)    [                  ] lstar_enter+0xae/0x120
> > > > (XEN)    [                  ] lstar_enter+0xa2/0x120
> > > > (XEN)    [                  ] lstar_enter+0xae/0x120
> > > > (XEN)    [                  ] lstar_enter+0xa2/0x120
> > > > (XEN)    [                  ] lstar_enter+0xae/0x120
> > > > (XEN)    [                  ] lstar_enter+0xa2/0x120
> > > > (XEN)    [                  ] lstar_enter+0xae/0x120
> > > > (XEN)    [                  ] lstar_enter+0x10c/0x120
> > > > (XEN)
> > > > (XEN)
> > > > (XEN) *****************************************
> > > > (XEN) Panic on CPU 0:
> > > > (XEN) FATAL TRAP: vector = 0 (divide error)
> > > > (XEN) [error_code=0000]
> > > > (XEN) *****************************************
> > > > 
> > > > Any idea? Lack of serial console doesn't help with collecting logs...
> > > 
> > > May I suggest that you repeat this with a debug hypervisor, such that
> > > the call trace becomes more usable/sensible?
> > 
> > Actually, I've already tried, but every other build I try, I get even
> > less useful call trace, for example debug unstable build:
> > 
> >      Xen call trace:
> >         [<000000007b751c09>] 000000007b751c09
> >         [<8c2b0398e0000daa>] 8c2b0398e0000daa
> > ...
> >      GENERAL PROTECTION FAULT
> >      [error_code=0000]
> 
> But this makes reasonable sense: The faulting insn is "call *0x18(%rax)"
> and %rax is 6954b2b0, which points into a block of type
> EfiBootServicesData (and hence likely reused by the time of the crash
> for other purposes). Hence the "/mapbs" option of xen.efi might help
> here too, but iirc there's no equivalent for it in the MB2 case.

Oh, yes, indeed in Qubes we have /mapbs enabled by default with xen.efi.
I'd like to add it to MB2 case too. Is command line parsed at this point
(efi_exit_boot()) already? 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Jan Beulich]

On 07.08.2019 17:51, Marek Marczykowski-Górecki  wrote:
> On Wed, Aug 07, 2019 at 05:33:05PM +0200, Jan Beulich wrote:
>> On 07.08.2019 17:17, Marek Marczykowski-Górecki  wrote:
>>> Actually, I've already tried, but every other build I try, I get even
>>> less useful call trace, for example debug unstable build:
>>>
>>>       Xen call trace:
>>>          [<000000007b751c09>] 000000007b751c09
>>>          [<8c2b0398e0000daa>] 8c2b0398e0000daa
>>> ...
>>>       GENERAL PROTECTION FAULT
>>>       [error_code=0000]
>>
>> But this makes reasonable sense: The faulting insn is "call *0x18(%rax)"
>> and %rax is 6954b2b0, which points into a block of type
>> EfiBootServicesData (and hence likely reused by the time of the crash
>> for other purposes). Hence the "/mapbs" option of xen.efi might help
>> here too, but iirc there's no equivalent for it in the MB2 case.
> 
> Oh, yes, indeed in Qubes we have /mapbs enabled by default with xen.efi.
> I'd like to add it to MB2 case too. Is command line parsed at this point
> (efi_exit_boot()) already?

I'm struggling a little how to reply, considering that I've already
said "iirc there's no equivalent for it in the MB2 case". So I guess
I'm simply not understanding your question, or more specifically
where you want to get with it.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 1888 bytes --]

On Wed, Aug 07, 2019 at 05:58:59PM +0200, Jan Beulich wrote:
> On 07.08.2019 17:51, Marek Marczykowski-Górecki  wrote:
> > On Wed, Aug 07, 2019 at 05:33:05PM +0200, Jan Beulich wrote:
> > > On 07.08.2019 17:17, Marek Marczykowski-Górecki  wrote:
> > > > Actually, I've already tried, but every other build I try, I get even
> > > > less useful call trace, for example debug unstable build:
> > > > 
> > > >       Xen call trace:
> > > >          [<000000007b751c09>] 000000007b751c09
> > > >          [<8c2b0398e0000daa>] 8c2b0398e0000daa
> > > > ...
> > > >       GENERAL PROTECTION FAULT
> > > >       [error_code=0000]
> > > 
> > > But this makes reasonable sense: The faulting insn is "call *0x18(%rax)"
> > > and %rax is 6954b2b0, which points into a block of type
> > > EfiBootServicesData (and hence likely reused by the time of the crash
> > > for other purposes). Hence the "/mapbs" option of xen.efi might help
> > > here too, but iirc there's no equivalent for it in the MB2 case.
> > 
> > Oh, yes, indeed in Qubes we have /mapbs enabled by default with xen.efi.
> > I'd like to add it to MB2 case too. Is command line parsed at this point
> > (efi_exit_boot()) already?
> 
> I'm struggling a little how to reply, considering that I've already
> said "iirc there's no equivalent for it in the MB2 case". So I guess
> I'm simply not understanding your question, or more specifically
> where you want to get with it.

/mapbs option is very specific to xen.efi. I'd like to add a means to
set map_bs variable in MB2 case too. I'm asking whether I can use
standard command line parser, or do I need something special extracting
it from MB2 structures directly.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Jan Beulich]

On 07.08.2019 18:04, Marek Marczykowski-Górecki  wrote:
> On Wed, Aug 07, 2019 at 05:58:59PM +0200, Jan Beulich wrote:
>> On 07.08.2019 17:51, Marek Marczykowski-Górecki  wrote:
>>> On Wed, Aug 07, 2019 at 05:33:05PM +0200, Jan Beulich wrote:
>>>> On 07.08.2019 17:17, Marek Marczykowski-Górecki  wrote:
>>>>> Actually, I've already tried, but every other build I try, I get even
>>>>> less useful call trace, for example debug unstable build:
>>>>>
>>>>>        Xen call trace:
>>>>>           [<000000007b751c09>] 000000007b751c09
>>>>>           [<8c2b0398e0000daa>] 8c2b0398e0000daa
>>>>> ...
>>>>>        GENERAL PROTECTION FAULT
>>>>>        [error_code=0000]
>>>>
>>>> But this makes reasonable sense: The faulting insn is "call *0x18(%rax)"
>>>> and %rax is 6954b2b0, which points into a block of type
>>>> EfiBootServicesData (and hence likely reused by the time of the crash
>>>> for other purposes). Hence the "/mapbs" option of xen.efi might help
>>>> here too, but iirc there's no equivalent for it in the MB2 case.
>>>
>>> Oh, yes, indeed in Qubes we have /mapbs enabled by default with xen.efi.
>>> I'd like to add it to MB2 case too. Is command line parsed at this point
>>> (efi_exit_boot()) already?
>>
>> I'm struggling a little how to reply, considering that I've already
>> said "iirc there's no equivalent for it in the MB2 case". So I guess
>> I'm simply not understanding your question, or more specifically
>> where you want to get with it.
> 
> /mapbs option is very specific to xen.efi. I'd like to add a means to
> set map_bs variable in MB2 case too. I'm asking whether I can use
> standard command line parser, or do I need something special extracting
> it from MB2 structures directly.

Well, efi_multiboot2() gets called from head.S, even before the
(non-EFI only) cmdline_parse_early. I don't suppose there's a way
to avoid mixing this new option into the "normal" command line,
and hence there's also no way around peeking into the command
line early. Of course there's the option to see whether what
efi_arch_process_memory_map() could be deferred until later. But
likely that would be a far more intrusive change than what you're
considering to carry out.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 4042 bytes --]

On Wed, Aug 07, 2019 at 09:26:00PM +0200, Marek Marczykowski-Górecki wrote:
> On Wed, Aug 07, 2019 at 06:34:09PM +0200, Jan Beulich wrote:
> > On 07.08.2019 18:04, Marek Marczykowski-Górecki  wrote:
> > > On Wed, Aug 07, 2019 at 05:58:59PM +0200, Jan Beulich wrote:
> > > > On 07.08.2019 17:51, Marek Marczykowski-Górecki  wrote:
> > > > > On Wed, Aug 07, 2019 at 05:33:05PM +0200, Jan Beulich wrote:
> > > > > > On 07.08.2019 17:17, Marek Marczykowski-Górecki  wrote:
> > > > > > > Actually, I've already tried, but every other build I try, I get even
> > > > > > > less useful call trace, for example debug unstable build:
> > > > > > > 
> > > > > > >        Xen call trace:
> > > > > > >           [<000000007b751c09>] 000000007b751c09
> > > > > > >           [<8c2b0398e0000daa>] 8c2b0398e0000daa
> > > > > > > ...
> > > > > > >        GENERAL PROTECTION FAULT
> > > > > > >        [error_code=0000]
> > > > > > 
> > > > > > But this makes reasonable sense: The faulting insn is "call *0x18(%rax)"
> > > > > > and %rax is 6954b2b0, which points into a block of type
> > > > > > EfiBootServicesData (and hence likely reused by the time of the crash
> > > > > > for other purposes). Hence the "/mapbs" option of xen.efi might help
> > > > > > here too, but iirc there's no equivalent for it in the MB2 case.
> > > > > 
> > > > > Oh, yes, indeed in Qubes we have /mapbs enabled by default with xen.efi.
> > > > > I'd like to add it to MB2 case too. Is command line parsed at this point
> > > > > (efi_exit_boot()) already?
> > > > 
> > > > I'm struggling a little how to reply, considering that I've already
> > > > said "iirc there's no equivalent for it in the MB2 case". So I guess
> > > > I'm simply not understanding your question, or more specifically
> > > > where you want to get with it.
> > > 
> > > /mapbs option is very specific to xen.efi. I'd like to add a means to
> > > set map_bs variable in MB2 case too. I'm asking whether I can use
> > > standard command line parser, or do I need something special extracting
> > > it from MB2 structures directly.
> > 
> > Well, efi_multiboot2() gets called from head.S, even before the
> > (non-EFI only) cmdline_parse_early. I don't suppose there's a way
> > to avoid mixing this new option into the "normal" command line,
> > and hence there's also no way around peeking into the command
> > line early. Of course there's the option to see whether what
> > efi_arch_process_memory_map() could be deferred until later. But
> > likely that would be a far more intrusive change than what you're
> > considering to carry out.
> 
> Ok, regardless of adding proper option for that, I've hardcoded map_bs=1
> and it still crashes, just slightly differently:
> 
>     Xen call trace:
>        [<0000000000000080>] 0000000000000080
>        [<8c2b0398e0000daa>] 8c2b0398e0000daa
> 
>     Pagetable walk from ffffffff858483a1:
>        L4[0x1ff] = 0000000000000000 ffffffffffffffff
> 
>     ****************************************
>     Panic on CPU 0:
>     FATAL PAGE FAULT
>     [error_code=0002]
>     Faulting linear address: ffffffff858483a1
>     ****************************************
> 
> Full message attached.

After playing more with it and also know workarounds for various EFI
issues, I've found a way to boot it: avoid calling Exit BootServices.
There was a patch from Konrad adding /noexit option, that never get
committed. Similar to efi=mapbs option, I'd add efi=no-exitboot too
(once efi=mapbs patch is accepted).

Anyway, I'm curious what exactly is wrong here. Is it that the firmware
is not happy about lack of SetVirtualAddressMap call? FWIW, the crash is
during GetVariable RS call. I've verified that the function itself is
within EfiRuntimeServicesCode, but I don't feel like tracing Lenovo
UEFI...

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Jan Beulich]

On 08.08.2019 04:53, Marek Marczykowski-Górecki  wrote:
> On Wed, Aug 07, 2019 at 09:26:00PM +0200, Marek Marczykowski-Górecki wrote:
>> On Wed, Aug 07, 2019 at 06:34:09PM +0200, Jan Beulich wrote:
>>> On 07.08.2019 18:04, Marek Marczykowski-Górecki  wrote:
>>>> On Wed, Aug 07, 2019 at 05:58:59PM +0200, Jan Beulich wrote:
>>>>> On 07.08.2019 17:51, Marek Marczykowski-Górecki  wrote:
>>>>>> On Wed, Aug 07, 2019 at 05:33:05PM +0200, Jan Beulich wrote:
>>>>>>> On 07.08.2019 17:17, Marek Marczykowski-Górecki  wrote:
>>>>>>>> Actually, I've already tried, but every other build I try, I get even
>>>>>>>> less useful call trace, for example debug unstable build:
>>>>>>>>
>>>>>>>>         Xen call trace:
>>>>>>>>            [<000000007b751c09>] 000000007b751c09
>>>>>>>>            [<8c2b0398e0000daa>] 8c2b0398e0000daa
>>>>>>>> ...
>>>>>>>>         GENERAL PROTECTION FAULT
>>>>>>>>         [error_code=0000]
>>>>>>>
>>>>>>> But this makes reasonable sense: The faulting insn is "call *0x18(%rax)"
>>>>>>> and %rax is 6954b2b0, which points into a block of type
>>>>>>> EfiBootServicesData (and hence likely reused by the time of the crash
>>>>>>> for other purposes). Hence the "/mapbs" option of xen.efi might help
>>>>>>> here too, but iirc there's no equivalent for it in the MB2 case.
>>>>>>
>>>>>> Oh, yes, indeed in Qubes we have /mapbs enabled by default with xen.efi.
>>>>>> I'd like to add it to MB2 case too. Is command line parsed at this point
>>>>>> (efi_exit_boot()) already?
>>>>>
>>>>> I'm struggling a little how to reply, considering that I've already
>>>>> said "iirc there's no equivalent for it in the MB2 case". So I guess
>>>>> I'm simply not understanding your question, or more specifically
>>>>> where you want to get with it.
>>>>
>>>> /mapbs option is very specific to xen.efi. I'd like to add a means to
>>>> set map_bs variable in MB2 case too. I'm asking whether I can use
>>>> standard command line parser, or do I need something special extracting
>>>> it from MB2 structures directly.
>>>
>>> Well, efi_multiboot2() gets called from head.S, even before the
>>> (non-EFI only) cmdline_parse_early. I don't suppose there's a way
>>> to avoid mixing this new option into the "normal" command line,
>>> and hence there's also no way around peeking into the command
>>> line early. Of course there's the option to see whether what
>>> efi_arch_process_memory_map() could be deferred until later. But
>>> likely that would be a far more intrusive change than what you're
>>> considering to carry out.
>>
>> Ok, regardless of adding proper option for that, I've hardcoded map_bs=1
>> and it still crashes, just slightly differently:
>>
>>      Xen call trace:
>>         [<0000000000000080>] 0000000000000080
>>         [<8c2b0398e0000daa>] 8c2b0398e0000daa
>>
>>      Pagetable walk from ffffffff858483a1:
>>         L4[0x1ff] = 0000000000000000 ffffffffffffffff
>>
>>      ****************************************
>>      Panic on CPU 0:
>>      FATAL PAGE FAULT
>>      [error_code=0002]
>>      Faulting linear address: ffffffff858483a1
>>      ****************************************
>>
>> Full message attached.
> 
> After playing more with it and also know workarounds for various EFI
> issues, I've found a way to boot it: avoid calling Exit BootServices.
> There was a patch from Konrad adding /noexit option, that never get
> committed. Similar to efi=mapbs option, I'd add efi=no-exitboot too
> (once efi=mapbs patch is accepted).
> 
> Anyway, I'm curious what exactly is wrong here. Is it that the firmware
> is not happy about lack of SetVirtualAddressMap call? FWIW, the crash is
> during GetVariable RS call. I've verified that the function itself is
> within EfiRuntimeServicesCode, but I don't feel like tracing Lenovo
> UEFI...

This suggests that the firmware zaps a few too many pointers
during ExitBootServices(). Perhaps internally they check
whether pointers point into BootServices* memory, and hence the
wrong marking in the memory map has consequences beyond the OS
re-using such memory?

A proper answer to your question can of course only be given
by someone knowing this specific firmware version.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 5254 bytes --]

On Thu, Aug 08, 2019 at 08:03:49AM +0200, Jan Beulich wrote:
> On 08.08.2019 04:53, Marek Marczykowski-Górecki  wrote:
> > On Wed, Aug 07, 2019 at 09:26:00PM +0200, Marek Marczykowski-Górecki wrote:
> > > Ok, regardless of adding proper option for that, I've hardcoded map_bs=1
> > > and it still crashes, just slightly differently:
> > > 
> > >      Xen call trace:
> > >         [<0000000000000080>] 0000000000000080
> > >         [<8c2b0398e0000daa>] 8c2b0398e0000daa
> > > 
> > >      Pagetable walk from ffffffff858483a1:
> > >         L4[0x1ff] = 0000000000000000 ffffffffffffffff
> > > 
> > >      ****************************************
> > >      Panic on CPU 0:
> > >      FATAL PAGE FAULT
> > >      [error_code=0002]
> > >      Faulting linear address: ffffffff858483a1
> > >      ****************************************
> > > 
> > > Full message attached.
> > 
> > After playing more with it and also know workarounds for various EFI
> > issues, I've found a way to boot it: avoid calling Exit BootServices.
> > There was a patch from Konrad adding /noexit option, that never get
> > committed. Similar to efi=mapbs option, I'd add efi=no-exitboot too
> > (once efi=mapbs patch is accepted).
> > 
> > Anyway, I'm curious what exactly is wrong here. Is it that the firmware
> > is not happy about lack of SetVirtualAddressMap call? FWIW, the crash is
> > during GetVariable RS call. I've verified that the function itself is
> > within EfiRuntimeServicesCode, but I don't feel like tracing Lenovo
> > UEFI...
> 
> This suggests that the firmware zaps a few too many pointers
> during ExitBootServices(). Perhaps internally they check
> whether pointers point into BootServices* memory, and hence the
> wrong marking in the memory map has consequences beyond the OS
> re-using such memory?
> 
> A proper answer to your question can of course only be given
> by someone knowing this specific firmware version.

I explored it a bit more and talked with a few people doing firmware
development and few conclusions:
1. Not calling SetVirtualAddressMap(), while technically legal, is
pretty uncommon and not recommended if you want to avoid less tested
(aka buggy) UEFI code paths.
2. Every UEFI call before SetVirtualAddressMap() call should be done
with flat physical memory. This include SetVirtualAddressMap() call
itself. Implicitly this means such calls can legally access memory areas
not marked with EFI_MEMORY_RUNTIME.

For the second point, relevant part of UEFI 2.7 spec (Runtime Services
- Virtual Memory Services chapter):

    This section contains function definitions for the virtual memory support that may be
    optionally used by an operating system at runtime. If an operating system chooses to
    make EFI runtime service calls in a virtual addressing mode instead of the flat physical
    mode, then the operating system must use the services in this section to switch the EFI
    runtime services from flat physical addressing to virtual addressing.
(...)
    The call to SetVirtualAddressMap() must be done with the physical mappings. On
    successful return from this function, the system must then make any future calls with the
    newly assigned virtual mappings. All address space mappings must be done in
    accordance to the cacheability flags as specified in the original address map.

I've tried to poke around this part of Xen code, including resurrecting
SetVirtualAddressMap() (#define USE_SET_VIRTUAL_ADDRESS_MAP in
common/efi/boot.c) and (unsurprisingly) hit multiple issues:
 - at this point of time, Xen is already relocated and paging is enabled
 - SetVirtualAddressMap() is indeed not happy about being called with
   new address map in place already
 - directmap - at which that code points - is mapped with NX, which breaks
   EfiRuntimeServicesCode area

Then I've tried a different approach: call SetVirtualAddressMap(), but
with an address map that tries to pretend physical addressing (the code
under #ifndef USE_SET_VIRTUAL_ADDRESS_MAP). This mostly worked, I needed
only few changes:
 - set VirtualStart back to PhysicalStart in that memory map (it was set
   to directmap)
 - map boot services (at least for the SetVirtualAddressMap() call time,
   but haven't tried unmapping it later)
 - call SetVirtualAddressMap() with that "1:1" map in place, using
   efi_rs_enter/efi_rs_leave.

This fixed the issue for me, now runtime services do work even without
disabling ExitBootServices() call. And without any extra
platform-specific command line arguments. And I think it also shouldn't break
kexec, since it uses 1:1-like map, but I haven't tried. One should
simply ignore EFI_UNSUPPORTED return code (I don't know how to avoid the
call at all after kexec).

Any thoughts? If the above sounds good, I'll cleanup the patch and
submit it.
BTW Does it qualify for 4.13? On one hand it may be seen as a bugfix
(fix booting on some UEFI firmwares), but on the other hand, I can't
think of all the side effects.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Jan Beulich]

On 08.10.2019 13:50, Marek Marczykowski-Górecki  wrote:
> On Thu, Aug 08, 2019 at 08:03:49AM +0200, Jan Beulich wrote:
>> On 08.08.2019 04:53, Marek Marczykowski-Górecki  wrote:
>>> On Wed, Aug 07, 2019 at 09:26:00PM +0200, Marek Marczykowski-Górecki wrote:
>>>> Ok, regardless of adding proper option for that, I've hardcoded map_bs=1
>>>> and it still crashes, just slightly differently:
>>>>
>>>>      Xen call trace:
>>>>         [<0000000000000080>] 0000000000000080
>>>>         [<8c2b0398e0000daa>] 8c2b0398e0000daa
>>>>
>>>>      Pagetable walk from ffffffff858483a1:
>>>>         L4[0x1ff] = 0000000000000000 ffffffffffffffff
>>>>
>>>>      ****************************************
>>>>      Panic on CPU 0:
>>>>      FATAL PAGE FAULT
>>>>      [error_code=0002]
>>>>      Faulting linear address: ffffffff858483a1
>>>>      ****************************************
>>>>
>>>> Full message attached.
>>>
>>> After playing more with it and also know workarounds for various EFI
>>> issues, I've found a way to boot it: avoid calling Exit BootServices.
>>> There was a patch from Konrad adding /noexit option, that never get
>>> committed. Similar to efi=mapbs option, I'd add efi=no-exitboot too
>>> (once efi=mapbs patch is accepted).
>>>
>>> Anyway, I'm curious what exactly is wrong here. Is it that the firmware
>>> is not happy about lack of SetVirtualAddressMap call? FWIW, the crash is
>>> during GetVariable RS call. I've verified that the function itself is
>>> within EfiRuntimeServicesCode, but I don't feel like tracing Lenovo
>>> UEFI...
>>
>> This suggests that the firmware zaps a few too many pointers
>> during ExitBootServices(). Perhaps internally they check
>> whether pointers point into BootServices* memory, and hence the
>> wrong marking in the memory map has consequences beyond the OS
>> re-using such memory?
>>
>> A proper answer to your question can of course only be given
>> by someone knowing this specific firmware version.
> 
> I explored it a bit more and talked with a few people doing firmware
> development and few conclusions:
> 1. Not calling SetVirtualAddressMap(), while technically legal, is
> pretty uncommon and not recommended if you want to avoid less tested
> (aka buggy) UEFI code paths.
> 2. Every UEFI call before SetVirtualAddressMap() call should be done
> with flat physical memory. This include SetVirtualAddressMap() call
> itself. Implicitly this means such calls can legally access memory areas
> not marked with EFI_MEMORY_RUNTIME.

I don't think this is quite right - whether non-runtime memory may
be touched depends exclusively on ExitBootServices() (not) having
got called (yet).

> Then I've tried a different approach: call SetVirtualAddressMap(), but
> with an address map that tries to pretend physical addressing (the code
> under #ifndef USE_SET_VIRTUAL_ADDRESS_MAP). This mostly worked, I needed
> only few changes:
>  - set VirtualStart back to PhysicalStart in that memory map (it was set
>    to directmap)
>  - map boot services (at least for the SetVirtualAddressMap() call time,
>    but haven't tried unmapping it later)
>  - call SetVirtualAddressMap() with that "1:1" map in place, using
>    efi_rs_enter/efi_rs_leave.
> 
> This fixed the issue for me, now runtime services do work even without
> disabling ExitBootServices() call. And without any extra
> platform-specific command line arguments. And I think it also shouldn't break
> kexec, since it uses 1:1-like map, but I haven't tried. One should
> simply ignore EFI_UNSUPPORTED return code (I don't know how to avoid the
> call at all after kexec).

That's the point - it can't be avoided, and hence it failing is not
an option. Or else there needs to be a protocol telling kexec what
it is to expect, and that it in particular can't change the virtual
address map to its liking. Back at the time when I put together the
EFI booting code, no such protocol existed, and hence calling
SetVirtualAddressMap() was not an option. I have no idea whether
things have changed in the meantime.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 6813 bytes --]

On Tue, Oct 08, 2019 at 03:08:29PM +0200, Jan Beulich wrote:
> On 08.10.2019 13:50, Marek Marczykowski-Górecki  wrote:
> > On Thu, Aug 08, 2019 at 08:03:49AM +0200, Jan Beulich wrote:
> >> On 08.08.2019 04:53, Marek Marczykowski-Górecki  wrote:
> >>> On Wed, Aug 07, 2019 at 09:26:00PM +0200, Marek Marczykowski-Górecki wrote:
> >>>> Ok, regardless of adding proper option for that, I've hardcoded map_bs=1
> >>>> and it still crashes, just slightly differently:
> >>>>
> >>>>      Xen call trace:
> >>>>         [<0000000000000080>] 0000000000000080
> >>>>         [<8c2b0398e0000daa>] 8c2b0398e0000daa
> >>>>
> >>>>      Pagetable walk from ffffffff858483a1:
> >>>>         L4[0x1ff] = 0000000000000000 ffffffffffffffff
> >>>>
> >>>>      ****************************************
> >>>>      Panic on CPU 0:
> >>>>      FATAL PAGE FAULT
> >>>>      [error_code=0002]
> >>>>      Faulting linear address: ffffffff858483a1
> >>>>      ****************************************
> >>>>
> >>>> Full message attached.
> >>>
> >>> After playing more with it and also know workarounds for various EFI
> >>> issues, I've found a way to boot it: avoid calling Exit BootServices.
> >>> There was a patch from Konrad adding /noexit option, that never get
> >>> committed. Similar to efi=mapbs option, I'd add efi=no-exitboot too
> >>> (once efi=mapbs patch is accepted).
> >>>
> >>> Anyway, I'm curious what exactly is wrong here. Is it that the firmware
> >>> is not happy about lack of SetVirtualAddressMap call? FWIW, the crash is
> >>> during GetVariable RS call. I've verified that the function itself is
> >>> within EfiRuntimeServicesCode, but I don't feel like tracing Lenovo
> >>> UEFI...
> >>
> >> This suggests that the firmware zaps a few too many pointers
> >> during ExitBootServices(). Perhaps internally they check
> >> whether pointers point into BootServices* memory, and hence the
> >> wrong marking in the memory map has consequences beyond the OS
> >> re-using such memory?
> >>
> >> A proper answer to your question can of course only be given
> >> by someone knowing this specific firmware version.
> > 
> > I explored it a bit more and talked with a few people doing firmware
> > development and few conclusions:
> > 1. Not calling SetVirtualAddressMap(), while technically legal, is
> > pretty uncommon and not recommended if you want to avoid less tested
> > (aka buggy) UEFI code paths.
> > 2. Every UEFI call before SetVirtualAddressMap() call should be done
> > with flat physical memory. This include SetVirtualAddressMap() call
> > itself. Implicitly this means such calls can legally access memory areas
> > not marked with EFI_MEMORY_RUNTIME.
> 
> I don't think this is quite right - whether non-runtime memory may
> be touched depends exclusively on ExitBootServices() (not) having
> got called (yet).

That would be logical. In practice however we have evidences firmware
vendors have different opinion... A comment from Linux (already quoted
here 2 months ago):

    /*
     * The UEFI specification makes it clear that the operating system is
     * free to do whatever it wants with boot services code after
     * ExitBootServices() has been called. Ignoring this recommendation a
     * significant bunch of EFI implementations continue calling into boot
     * services code (SetVirtualAddressMap). In order to work around such
     * buggy implementations we reserve boot services region during EFI
     * init and make sure it stays executable. Then, after
     * SetVirtualAddressMap(), it is discarded.
     *
     * However, some boot services regions contain data that is required
     * by drivers, so we need to track which memory ranges can never be
     * freed. This is done by tagging those regions with the
     * EFI_MEMORY_RUNTIME attribute.
     *
     * Any driver that wants to mark a region as reserved must use
     * efi_mem_reserve() which will insert a new EFI memory descriptor
     * into efi.memmap (splitting existing regions if necessary) and tag
     * it with EFI_MEMORY_RUNTIME.
     */

Regardless of SetVirtualAddressMap() discussion, I propose to
automatically map boot services code/data, to make Xen work on more
machines (even if _we_ consider those buggy). 

> > Then I've tried a different approach: call SetVirtualAddressMap(), but
> > with an address map that tries to pretend physical addressing (the code
> > under #ifndef USE_SET_VIRTUAL_ADDRESS_MAP). This mostly worked, I needed
> > only few changes:
> >  - set VirtualStart back to PhysicalStart in that memory map (it was set
> >    to directmap)
> >  - map boot services (at least for the SetVirtualAddressMap() call time,
> >    but haven't tried unmapping it later)
> >  - call SetVirtualAddressMap() with that "1:1" map in place, using
> >    efi_rs_enter/efi_rs_leave.
> > 
> > This fixed the issue for me, now runtime services do work even without
> > disabling ExitBootServices() call. And without any extra
> > platform-specific command line arguments. And I think it also shouldn't break
> > kexec, since it uses 1:1-like map, but I haven't tried. One should
> > simply ignore EFI_UNSUPPORTED return code (I don't know how to avoid the
> > call at all after kexec).
> 
> That's the point - it can't be avoided, and hence it failing is not
> an option. Or else there needs to be a protocol telling kexec what
> it is to expect, and that it in particular can't change the virtual
> address map to its liking. Back at the time when I put together the
> EFI booting code, no such protocol existed, and hence calling
> SetVirtualAddressMap() was not an option. I have no idea whether
> things have changed in the meantime.

Hmm, how is it different from the current situation? Not calling
SetVirtualAddressMap() means UEFI will not be notified about new address
map. It does _not_ mean it will use 1:1 map, it will use what was
previously set. What if Xen was kexec'ed from Linux?
In Linux case, it looks like it passes around the EFI memory map using
some Linux-specific mechanism, but I don't find it particularly
appealing option.

What about something in between: make this SetVirtualAddressMap() call
compile-time option (kconfig), depending on !CONFIG_KEXEC ? And when
enabled, properly handle SetVirtualAddressMap() failure. I my case,
where I do care about supporting various UEFI implementations, I don't
need kexec support. And apparently people carrying about kexec don't
have problems with lack of SetVirtualAddressMap(), so that would be
win-win, no?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Jan Beulich]

On 08.10.2019 15:52, Marek Marczykowski-Górecki  wrote:
> On Tue, Oct 08, 2019 at 03:08:29PM +0200, Jan Beulich wrote:
>> On 08.10.2019 13:50, Marek Marczykowski-Górecki  wrote:
>>> On Thu, Aug 08, 2019 at 08:03:49AM +0200, Jan Beulich wrote:
>>>> On 08.08.2019 04:53, Marek Marczykowski-Górecki  wrote:
>>>>> On Wed, Aug 07, 2019 at 09:26:00PM +0200, Marek Marczykowski-Górecki wrote:
>>>>>> Ok, regardless of adding proper option for that, I've hardcoded map_bs=1
>>>>>> and it still crashes, just slightly differently:
>>>>>>
>>>>>>      Xen call trace:
>>>>>>         [<0000000000000080>] 0000000000000080
>>>>>>         [<8c2b0398e0000daa>] 8c2b0398e0000daa
>>>>>>
>>>>>>      Pagetable walk from ffffffff858483a1:
>>>>>>         L4[0x1ff] = 0000000000000000 ffffffffffffffff
>>>>>>
>>>>>>      ****************************************
>>>>>>      Panic on CPU 0:
>>>>>>      FATAL PAGE FAULT
>>>>>>      [error_code=0002]
>>>>>>      Faulting linear address: ffffffff858483a1
>>>>>>      ****************************************
>>>>>>
>>>>>> Full message attached.
>>>>>
>>>>> After playing more with it and also know workarounds for various EFI
>>>>> issues, I've found a way to boot it: avoid calling Exit BootServices.
>>>>> There was a patch from Konrad adding /noexit option, that never get
>>>>> committed. Similar to efi=mapbs option, I'd add efi=no-exitboot too
>>>>> (once efi=mapbs patch is accepted).
>>>>>
>>>>> Anyway, I'm curious what exactly is wrong here. Is it that the firmware
>>>>> is not happy about lack of SetVirtualAddressMap call? FWIW, the crash is
>>>>> during GetVariable RS call. I've verified that the function itself is
>>>>> within EfiRuntimeServicesCode, but I don't feel like tracing Lenovo
>>>>> UEFI...
>>>>
>>>> This suggests that the firmware zaps a few too many pointers
>>>> during ExitBootServices(). Perhaps internally they check
>>>> whether pointers point into BootServices* memory, and hence the
>>>> wrong marking in the memory map has consequences beyond the OS
>>>> re-using such memory?
>>>>
>>>> A proper answer to your question can of course only be given
>>>> by someone knowing this specific firmware version.
>>>
>>> I explored it a bit more and talked with a few people doing firmware
>>> development and few conclusions:
>>> 1. Not calling SetVirtualAddressMap(), while technically legal, is
>>> pretty uncommon and not recommended if you want to avoid less tested
>>> (aka buggy) UEFI code paths.
>>> 2. Every UEFI call before SetVirtualAddressMap() call should be done
>>> with flat physical memory. This include SetVirtualAddressMap() call
>>> itself. Implicitly this means such calls can legally access memory areas
>>> not marked with EFI_MEMORY_RUNTIME.
>>
>> I don't think this is quite right - whether non-runtime memory may
>> be touched depends exclusively on ExitBootServices() (not) having
>> got called (yet).
> 
> That would be logical. In practice however we have evidences firmware
> vendors have different opinion... A comment from Linux (already quoted
> here 2 months ago):
> 
>     /*
>      * The UEFI specification makes it clear that the operating system is
>      * free to do whatever it wants with boot services code after
>      * ExitBootServices() has been called. Ignoring this recommendation a
>      * significant bunch of EFI implementations continue calling into boot
>      * services code (SetVirtualAddressMap). In order to work around such
>      * buggy implementations we reserve boot services region during EFI
>      * init and make sure it stays executable. Then, after
>      * SetVirtualAddressMap(), it is discarded.
>      *
>      * However, some boot services regions contain data that is required
>      * by drivers, so we need to track which memory ranges can never be
>      * freed. This is done by tagging those regions with the
>      * EFI_MEMORY_RUNTIME attribute.
>      *
>      * Any driver that wants to mark a region as reserved must use
>      * efi_mem_reserve() which will insert a new EFI memory descriptor
>      * into efi.memmap (splitting existing regions if necessary) and tag
>      * it with EFI_MEMORY_RUNTIME.
>      */

But you realize that the comment specifically talks about
the call tree underneath SetVirtualAddressMap() being the violator.
As long as we don't call this function, we're unaffected as far as
this comment goes.

> Regardless of SetVirtualAddressMap() discussion, I propose to
> automatically map boot services code/data, to make Xen work on more
> machines (even if _we_ consider those buggy). 

I remain on my prior position: Adding command line triggerable
workarounds for such cases is fine. Defaulting to assume buggy
firmware is acceptable only if this means no extra penalty to
systems with conforming firmware. Hence, for the case at hand,
I object to doing this automatically; we already have the
/mapbs workaround in place to deal with the case when xen.efi
is used. Judging from the title here there may need to be an
addition to also allow triggering this from the MB2 boot path.

>>> Then I've tried a different approach: call SetVirtualAddressMap(), but
>>> with an address map that tries to pretend physical addressing (the code
>>> under #ifndef USE_SET_VIRTUAL_ADDRESS_MAP). This mostly worked, I needed
>>> only few changes:
>>>  - set VirtualStart back to PhysicalStart in that memory map (it was set
>>>    to directmap)
>>>  - map boot services (at least for the SetVirtualAddressMap() call time,
>>>    but haven't tried unmapping it later)
>>>  - call SetVirtualAddressMap() with that "1:1" map in place, using
>>>    efi_rs_enter/efi_rs_leave.
>>>
>>> This fixed the issue for me, now runtime services do work even without
>>> disabling ExitBootServices() call. And without any extra
>>> platform-specific command line arguments. And I think it also shouldn't break
>>> kexec, since it uses 1:1-like map, but I haven't tried. One should
>>> simply ignore EFI_UNSUPPORTED return code (I don't know how to avoid the
>>> call at all after kexec).
>>
>> That's the point - it can't be avoided, and hence it failing is not
>> an option. Or else there needs to be a protocol telling kexec what
>> it is to expect, and that it in particular can't change the virtual
>> address map to its liking. Back at the time when I put together the
>> EFI booting code, no such protocol existed, and hence calling
>> SetVirtualAddressMap() was not an option. I have no idea whether
>> things have changed in the meantime.
> 
> Hmm, how is it different from the current situation? Not calling
> SetVirtualAddressMap() means UEFI will not be notified about new address
> map. It does _not_ mean it will use 1:1 map, it will use what was
> previously set.

What do you mean by "previously set"? SetVirtualAddressMap() can be
invoked only once during a given session (i.e. without intervening
boot). How would other than a 1:1 map have got set?

> What if Xen was kexec'ed from Linux?
> In Linux case, it looks like it passes around the EFI memory map using
> some Linux-specific mechanism, but I don't find it particularly
> appealing option.

Indeed.

> What about something in between: make this SetVirtualAddressMap() call
> compile-time option (kconfig), depending on !CONFIG_KEXEC ? And when
> enabled, properly handle SetVirtualAddressMap() failure.

What is "proper handling" here?

> I my case,
> where I do care about supporting various UEFI implementations, I don't
> need kexec support. And apparently people carrying about kexec don't
> have problems with lack of SetVirtualAddressMap(), so that would be
> win-win, no?

Allowing SetVirtualAddressMap() when !KEXEC would be fine with me.
The fly in the ointment here is that we'd prefer not to have such
Kconfig options (at least not without EXPERT qualifier), as
(security) supporting all the possible combinations would be a
nightmare. If an EXPERT dependency is okay with you, then I'll be
looking forward to your patch.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 7251 bytes --]

On Tue, Oct 08, 2019 at 04:19:13PM +0200, Jan Beulich wrote:
> On 08.10.2019 15:52, Marek Marczykowski-Górecki  wrote:
> > On Tue, Oct 08, 2019 at 03:08:29PM +0200, Jan Beulich wrote:
> >> On 08.10.2019 13:50, Marek Marczykowski-Górecki  wrote:
> >>> I explored it a bit more and talked with a few people doing firmware
> >>> development and few conclusions:
> >>> 1. Not calling SetVirtualAddressMap(), while technically legal, is
> >>> pretty uncommon and not recommended if you want to avoid less tested
> >>> (aka buggy) UEFI code paths.
> >>> 2. Every UEFI call before SetVirtualAddressMap() call should be done
> >>> with flat physical memory. This include SetVirtualAddressMap() call
> >>> itself. Implicitly this means such calls can legally access memory areas
> >>> not marked with EFI_MEMORY_RUNTIME.
> >>
> >> I don't think this is quite right - whether non-runtime memory may
> >> be touched depends exclusively on ExitBootServices() (not) having
> >> got called (yet).
> > 
> > That would be logical. In practice however we have evidences firmware
> > vendors have different opinion... A comment from Linux (already quoted
> > here 2 months ago):
> > 
> >     /*
> >      * The UEFI specification makes it clear that the operating system is
> >      * free to do whatever it wants with boot services code after
> >      * ExitBootServices() has been called. Ignoring this recommendation a
> >      * significant bunch of EFI implementations continue calling into boot
> >      * services code (SetVirtualAddressMap). In order to work around such
> >      * buggy implementations we reserve boot services region during EFI
> >      * init and make sure it stays executable. Then, after
> >      * SetVirtualAddressMap(), it is discarded.
> >      *
> >      * However, some boot services regions contain data that is required
> >      * by drivers, so we need to track which memory ranges can never be
> >      * freed. This is done by tagging those regions with the
> >      * EFI_MEMORY_RUNTIME attribute.
> >      *
> >      * Any driver that wants to mark a region as reserved must use
> >      * efi_mem_reserve() which will insert a new EFI memory descriptor
> >      * into efi.memmap (splitting existing regions if necessary) and tag
> >      * it with EFI_MEMORY_RUNTIME.
> >      */
> 
> But you realize that the comment specifically talks about
> the call tree underneath SetVirtualAddressMap() being the violator.
> As long as we don't call this function, we're unaffected as far as
> this comment goes.

Well, this very thread proves it isn't only about
SetVirtualAddressMap(). I _guess_ it's about calls before
SetVirtualAddressMap() returns (which supposedly do some cleanups). In
Linux case, it is only that one call.

> > Regardless of SetVirtualAddressMap() discussion, I propose to
> > automatically map boot services code/data, to make Xen work on more
> > machines (even if _we_ consider those buggy). 
> 
> I remain on my prior position: Adding command line triggerable
> workarounds for such cases is fine. Defaulting to assume buggy
> firmware is acceptable only if this means no extra penalty to
> systems with conforming firmware. Hence, for the case at hand,
> I object to doing this automatically; we already have the
> /mapbs workaround in place to deal with the case when xen.efi
> is used. Judging from the title here there may need to be an
> addition to also allow triggering this from the MB2 boot path.

What about mirroring Linux behavior? I.e. mapping those regions for the
SetVirtualAddressMap() time (when enabled) and unmapping after - unless
tagged with EFI_MEMORY_RUNTIME? 
Similarly to Andrew, I'd really prefer for Xen to work out of the box,
with as little as possible manual tweaks needed.

> >>> Then I've tried a different approach: call SetVirtualAddressMap(), but
> >>> with an address map that tries to pretend physical addressing (the code
> >>> under #ifndef USE_SET_VIRTUAL_ADDRESS_MAP). This mostly worked, I needed
> >>> only few changes:
> >>>  - set VirtualStart back to PhysicalStart in that memory map (it was set
> >>>    to directmap)
> >>>  - map boot services (at least for the SetVirtualAddressMap() call time,
> >>>    but haven't tried unmapping it later)
> >>>  - call SetVirtualAddressMap() with that "1:1" map in place, using
> >>>    efi_rs_enter/efi_rs_leave.
> >>>
> >>> This fixed the issue for me, now runtime services do work even without
> >>> disabling ExitBootServices() call. And without any extra
> >>> platform-specific command line arguments. And I think it also shouldn't break
> >>> kexec, since it uses 1:1-like map, but I haven't tried. One should
> >>> simply ignore EFI_UNSUPPORTED return code (I don't know how to avoid the
> >>> call at all after kexec).
> >>
> >> That's the point - it can't be avoided, and hence it failing is not
> >> an option. Or else there needs to be a protocol telling kexec what
> >> it is to expect, and that it in particular can't change the virtual
> >> address map to its liking. Back at the time when I put together the
> >> EFI booting code, no such protocol existed, and hence calling
> >> SetVirtualAddressMap() was not an option. I have no idea whether
> >> things have changed in the meantime.
> > 
> > Hmm, how is it different from the current situation? Not calling
> > SetVirtualAddressMap() means UEFI will not be notified about new address
> > map. It does _not_ mean it will use 1:1 map, it will use what was
> > previously set.
> 
> What do you mean by "previously set"? SetVirtualAddressMap() can be
> invoked only once during a given session (i.e. without intervening
> boot). How would other than a 1:1 map have got set?

Like, in the very next sentence of my answer:

> > What if Xen was kexec'ed from Linux?

> > In Linux case, it looks like it passes around the EFI memory map using
> > some Linux-specific mechanism, but I don't find it particularly
> > appealing option.
> 
> Indeed.
> 
> > What about something in between: make this SetVirtualAddressMap() call
> > compile-time option (kconfig), depending on !CONFIG_KEXEC ? And when
> > enabled, properly handle SetVirtualAddressMap() failure.
> 
> What is "proper handling" here?

Logging the error and either panic or disabling runtime services (I tend
to the latter).

> > I my case,
> > where I do care about supporting various UEFI implementations, I don't
> > need kexec support. And apparently people carrying about kexec don't
> > have problems with lack of SetVirtualAddressMap(), so that would be
> > win-win, no?
> 
> Allowing SetVirtualAddressMap() when !KEXEC would be fine with me.
> The fly in the ointment here is that we'd prefer not to have such
> Kconfig options (at least not without EXPERT qualifier), as
> (security) supporting all the possible combinations would be a
> nightmare. If an EXPERT dependency is okay with you, then I'll be
> looking forward to your patch.

EXPERT is fine with me.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 2420 bytes --]

On Tue, Oct 08, 2019 at 06:29:27PM +0200, Marek Marczykowski-Górecki wrote:
> On Tue, Oct 08, 2019 at 04:19:13PM +0200, Jan Beulich wrote:
> > On 08.10.2019 15:52, Marek Marczykowski-Górecki  wrote:
> > > Regardless of SetVirtualAddressMap() discussion, I propose to
> > > automatically map boot services code/data, to make Xen work on more
> > > machines (even if _we_ consider those buggy). 
> > 
> > I remain on my prior position: Adding command line triggerable
> > workarounds for such cases is fine. Defaulting to assume buggy
> > firmware is acceptable only if this means no extra penalty to
> > systems with conforming firmware. Hence, for the case at hand,
> > I object to doing this automatically; we already have the
> > /mapbs workaround in place to deal with the case when xen.efi
> > is used. Judging from the title here there may need to be an
> > addition to also allow triggering this from the MB2 boot path.
> 
> What about mirroring Linux behavior? I.e. mapping those regions for the
> SetVirtualAddressMap() time (when enabled) and unmapping after - unless
> tagged with EFI_MEMORY_RUNTIME? 

Oh, even better: I can call SetVirtualAddressMap() while still in 1:1,
just after ExitBootServices(), giving it 1:1-like map (for areas marked
with EFI_MEMORY_RUNTIME). This way I don't need to really map BootServices
areas (and exclude from Xen memory allocator), so there is no penalty
for systems with conforming firmware. And indeed calling
SetVirtualAddressMap() is enough for other runtime services (like
GetVariable*) to behave correctly, even if boot services are not mapped
anymore.
So, the only downside is incompatibility with kexec.

> Similarly to Andrew, I'd really prefer for Xen to work out of the box,
> with as little as possible manual tweaks needed.

> > Allowing SetVirtualAddressMap() when !KEXEC would be fine with me.
> > The fly in the ointment here is that we'd prefer not to have such
> > Kconfig options (at least not without EXPERT qualifier), as
> > (security) supporting all the possible combinations would be a
> > nightmare. If an EXPERT dependency is okay with you, then I'll be
> > looking forward to your patch.
> 
> EXPERT is fine with me.


-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Jan Beulich]

On 08.10.2019 18:29, Marek Marczykowski-Górecki  wrote:
> On Tue, Oct 08, 2019 at 04:19:13PM +0200, Jan Beulich wrote:
>> On 08.10.2019 15:52, Marek Marczykowski-Górecki  wrote:
>>> Regardless of SetVirtualAddressMap() discussion, I propose to
>>> automatically map boot services code/data, to make Xen work on more
>>> machines (even if _we_ consider those buggy). 
>>
>> I remain on my prior position: Adding command line triggerable
>> workarounds for such cases is fine. Defaulting to assume buggy
>> firmware is acceptable only if this means no extra penalty to
>> systems with conforming firmware. Hence, for the case at hand,
>> I object to doing this automatically; we already have the
>> /mapbs workaround in place to deal with the case when xen.efi
>> is used. Judging from the title here there may need to be an
>> addition to also allow triggering this from the MB2 boot path.
> 
> What about mirroring Linux behavior? I.e. mapping those regions for the
> SetVirtualAddressMap() time (when enabled) and unmapping after - unless
> tagged with EFI_MEMORY_RUNTIME? 
> Similarly to Andrew, I'd really prefer for Xen to work out of the box,
> with as little as possible manual tweaks needed.

If there's going to be a config where SetVirtualAddressMap() is to
be called - why not? But the same logic doesn't make sense when
such a call won#t happen in the first place.

>>>>> Then I've tried a different approach: call SetVirtualAddressMap(), but
>>>>> with an address map that tries to pretend physical addressing (the code
>>>>> under #ifndef USE_SET_VIRTUAL_ADDRESS_MAP). This mostly worked, I needed
>>>>> only few changes:
>>>>>  - set VirtualStart back to PhysicalStart in that memory map (it was set
>>>>>    to directmap)
>>>>>  - map boot services (at least for the SetVirtualAddressMap() call time,
>>>>>    but haven't tried unmapping it later)
>>>>>  - call SetVirtualAddressMap() with that "1:1" map in place, using
>>>>>    efi_rs_enter/efi_rs_leave.
>>>>>
>>>>> This fixed the issue for me, now runtime services do work even without
>>>>> disabling ExitBootServices() call. And without any extra
>>>>> platform-specific command line arguments. And I think it also shouldn't break
>>>>> kexec, since it uses 1:1-like map, but I haven't tried. One should
>>>>> simply ignore EFI_UNSUPPORTED return code (I don't know how to avoid the
>>>>> call at all after kexec).
>>>>
>>>> That's the point - it can't be avoided, and hence it failing is not
>>>> an option. Or else there needs to be a protocol telling kexec what
>>>> it is to expect, and that it in particular can't change the virtual
>>>> address map to its liking. Back at the time when I put together the
>>>> EFI booting code, no such protocol existed, and hence calling
>>>> SetVirtualAddressMap() was not an option. I have no idea whether
>>>> things have changed in the meantime.
>>>
>>> Hmm, how is it different from the current situation? Not calling
>>> SetVirtualAddressMap() means UEFI will not be notified about new address
>>> map. It does _not_ mean it will use 1:1 map, it will use what was
>>> previously set.
>>
>> What do you mean by "previously set"? SetVirtualAddressMap() can be
>> invoked only once during a given session (i.e. without intervening
>> boot). How would other than a 1:1 map have got set?
> 
> Like, in the very next sentence of my answer:
> 
>>> What if Xen was kexec'ed from Linux?

Honestly - I'm getting tired. You said yourself ...

>>> In Linux case, it looks like it passes around the EFI memory map using
>>> some Linux-specific mechanism, but I don't find it particularly
>>> appealing option.

... that this would require Xen following a Linux protocol.
This is nothing that can work building on EFI interfaces alone.

>>> What about something in between: make this SetVirtualAddressMap() call
>>> compile-time option (kconfig), depending on !CONFIG_KEXEC ? And when
>>> enabled, properly handle SetVirtualAddressMap() failure.
>>
>> What is "proper handling" here?
> 
> Logging the error and either panic or disabling runtime services (I tend
> to the latter).

Hmm, yes, disabling runtime services in this case makes sense.
But are you sure a SetVirtualAddressMap() failure doesn't incur
other potential issues later on? (Calling panic() is what I'd
rather not call "proper handling", but rather "emergency
handling".)

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 3820 bytes --]

On Wed, Oct 09, 2019 at 10:56:56AM +0200, Jan Beulich wrote:
> On 08.10.2019 18:29, Marek Marczykowski-Górecki  wrote:
> > On Tue, Oct 08, 2019 at 04:19:13PM +0200, Jan Beulich wrote:
> >> On 08.10.2019 15:52, Marek Marczykowski-Górecki  wrote:
> >>> Regardless of SetVirtualAddressMap() discussion, I propose to
> >>> automatically map boot services code/data, to make Xen work on more
> >>> machines (even if _we_ consider those buggy). 
> >>
> >> I remain on my prior position: Adding command line triggerable
> >> workarounds for such cases is fine. Defaulting to assume buggy
> >> firmware is acceptable only if this means no extra penalty to
> >> systems with conforming firmware. Hence, for the case at hand,
> >> I object to doing this automatically; we already have the
> >> /mapbs workaround in place to deal with the case when xen.efi
> >> is used. Judging from the title here there may need to be an
> >> addition to also allow triggering this from the MB2 boot path.
> > 
> > What about mirroring Linux behavior? I.e. mapping those regions for the
> > SetVirtualAddressMap() time (when enabled) and unmapping after - unless
> > tagged with EFI_MEMORY_RUNTIME? 
> > Similarly to Andrew, I'd really prefer for Xen to work out of the box,
> > with as little as possible manual tweaks needed.
> 
> If there's going to be a config where SetVirtualAddressMap() is to
> be called - why not? But the same logic doesn't make sense when
> such a call won#t happen in the first place.

See my other email, I think I've found a better (simple and working!)
solution.

> >>> What if Xen was kexec'ed from Linux?
> 
> Honestly - I'm getting tired. You said yourself ...
> 
> >>> In Linux case, it looks like it passes around the EFI memory map using
> >>> some Linux-specific mechanism, but I don't find it particularly
> >>> appealing option.
> 
> ... that this would require Xen following a Linux protocol.
> This is nothing that can work building on EFI interfaces alone.

Actually, there is something that could be used: presence of boot
services. If the call to SetVirtualAddressMap() is bound to initial
presence of boot services, then it surely won't happen after kexec, as
boot services are not available anymore. In fact the patch I've sent
does exactly that - call SetVirtualAddressMap() directly after
ExitBootServices(), but I've realized this property only now. In this
case, maybe kconfig option is not needed anymore?

BTW How runtime services work after kexec? I don't see EFI handles
handed over kexec, are they somehow re-discovered?

> >>> What about something in between: make this SetVirtualAddressMap() call
> >>> compile-time option (kconfig), depending on !CONFIG_KEXEC ? And when
> >>> enabled, properly handle SetVirtualAddressMap() failure.
> >>
> >> What is "proper handling" here?
> > 
> > Logging the error and either panic or disabling runtime services (I tend
> > to the latter).
> 
> Hmm, yes, disabling runtime services in this case makes sense.
> But are you sure a SetVirtualAddressMap() failure doesn't incur
> other potential issues later on? (Calling panic() is what I'd
> rather not call "proper handling", but rather "emergency
> handling".)

Well, as for being sure, one could say calling ExitBootServices() but
not SetVirtualAddressMap() definitely won't cause any troubles. I can't
say anything about UEFI for sure. But UEFI spec doesn't mention any side
effect of a failed call.

BTW Linux panic on failed SetVirtualAddressMap(). But on kexec, if
didn't received address map for EFI RS calls, it simply disable RS.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Jan Beulich]

On 09.10.2019 12:31, Marek Marczykowski-Górecki  wrote:
> On Wed, Oct 09, 2019 at 10:56:56AM +0200, Jan Beulich wrote:
>> On 08.10.2019 18:29, Marek Marczykowski-Górecki  wrote:
>>> On Tue, Oct 08, 2019 at 04:19:13PM +0200, Jan Beulich wrote:
>>>> On 08.10.2019 15:52, Marek Marczykowski-Górecki  wrote:
>>>>> In Linux case, it looks like it passes around the EFI memory map using
>>>>> some Linux-specific mechanism, but I don't find it particularly
>>>>> appealing option.
>>
>> ... that this would require Xen following a Linux protocol.
>> This is nothing that can work building on EFI interfaces alone.
> 
> Actually, there is something that could be used: presence of boot
> services. If the call to SetVirtualAddressMap() is bound to initial
> presence of boot services, then it surely won't happen after kexec, as
> boot services are not available anymore. In fact the patch I've sent
> does exactly that - call SetVirtualAddressMap() directly after
> ExitBootServices(), but I've realized this property only now. In this
> case, maybe kconfig option is not needed anymore?

I'm unaware of a property telling an EFI application whether
boot services are available. By the definition I know they're
available up and until ExitBootServices() gets called.

> BTW How runtime services work after kexec? I don't see EFI handles
> handed over kexec, are they somehow re-discovered?

What EFI handles are you talking about? For runtime services
what a consumer needs is a table pointer, which is a field
in the system table, which in turn is an argument passed to
the EFI application's entry point. I didn't think there are
provisions in the spec for either of these pointers being NULL.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 2593 bytes --]

On Wed, Oct 09, 2019 at 12:50:09PM +0200, Jan Beulich wrote:
> On 09.10.2019 12:31, Marek Marczykowski-Górecki  wrote:
> > On Wed, Oct 09, 2019 at 10:56:56AM +0200, Jan Beulich wrote:
> >> On 08.10.2019 18:29, Marek Marczykowski-Górecki  wrote:
> >>> On Tue, Oct 08, 2019 at 04:19:13PM +0200, Jan Beulich wrote:
> >>>> On 08.10.2019 15:52, Marek Marczykowski-Górecki  wrote:
> >>>>> In Linux case, it looks like it passes around the EFI memory map using
> >>>>> some Linux-specific mechanism, but I don't find it particularly
> >>>>> appealing option.
> >>
> >> ... that this would require Xen following a Linux protocol.
> >> This is nothing that can work building on EFI interfaces alone.
> > 
> > Actually, there is something that could be used: presence of boot
> > services. If the call to SetVirtualAddressMap() is bound to initial
> > presence of boot services, then it surely won't happen after kexec, as
> > boot services are not available anymore. In fact the patch I've sent
> > does exactly that - call SetVirtualAddressMap() directly after
> > ExitBootServices(), but I've realized this property only now. In this
> > case, maybe kconfig option is not needed anymore?
> 
> I'm unaware of a property telling an EFI application whether
> boot services are available. By the definition I know they're
> available up and until ExitBootServices() gets called.

Regardless of how it is done, calling SetVirtualAddressMap() directly
after ExitBootServices() should be fine. If for some reason Xen would
try to call ExitBootServices() when boot services are already gone, it
is a bug elsewhere (and probably will crash Xen before it event gets to
SetVirtualAddressMap() call). I'm simply relying on this property,
regardless of how it is technically done.

> > BTW How runtime services work after kexec? I don't see EFI handles
> > handed over kexec, are they somehow re-discovered?
> 
> What EFI handles are you talking about? For runtime services
> what a consumer needs is a table pointer, which is a field
> in the system table, which in turn is an argument passed to
> the EFI application's entry point.

Yes, I'm talking about those pointers (system table specifically).

> I didn't think there are
> provisions in the spec for either of these pointers being NULL.

But I don't see kexec using EFI application entry point. Am I missing
something?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Jan Beulich]

On 09.10.2019 13:00, Marek Marczykowski-Górecki  wrote:
> On Wed, Oct 09, 2019 at 12:50:09PM +0200, Jan Beulich wrote:
>> On 09.10.2019 12:31, Marek Marczykowski-Górecki  wrote:
>>> BTW How runtime services work after kexec? I don't see EFI handles
>>> handed over kexec, are they somehow re-discovered?
>>
>> What EFI handles are you talking about? For runtime services
>> what a consumer needs is a table pointer, which is a field
>> in the system table, which in turn is an argument passed to
>> the EFI application's entry point.
> 
> Yes, I'm talking about those pointers (system table specifically).
> 
>> I didn't think there are
>> provisions in the spec for either of these pointers being NULL.
> 
> But I don't see kexec using EFI application entry point. Am I missing
> something?

Can we stop thinking about a Linux -> Xen transition on this
thread please? It only serves to confuse matters imo. Kexec
_is_ different, and if anyone wants to get that transition
working, then they'll have to properly investigate what it
takes. Here we're concerned only about not breaking kexec-ing
_from_ Xen.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 1383 bytes --]

On Wed, Oct 09, 2019 at 01:48:38PM +0200, Jan Beulich wrote:
> On 09.10.2019 13:00, Marek Marczykowski-Górecki  wrote:
> > On Wed, Oct 09, 2019 at 12:50:09PM +0200, Jan Beulich wrote:
> >> On 09.10.2019 12:31, Marek Marczykowski-Górecki  wrote:
> >>> BTW How runtime services work after kexec? I don't see EFI handles
> >>> handed over kexec, are they somehow re-discovered?
> >>
> >> What EFI handles are you talking about? For runtime services
> >> what a consumer needs is a table pointer, which is a field
> >> in the system table, which in turn is an argument passed to
> >> the EFI application's entry point.
> > 
> > Yes, I'm talking about those pointers (system table specifically).
> > 
> >> I didn't think there are
> >> provisions in the spec for either of these pointers being NULL.
> > 
> > But I don't see kexec using EFI application entry point. Am I missing
> > something?
> 
> Can we stop thinking about a Linux -> Xen transition on this
> thread please? 

I'm talking about Xen->Xen transition here. How system table pointer is
passed from old Xen to new Xen instance? And how the new Xen instance
deals with boot services being not available anymore?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Jan Beulich]

On 09.10.2019 13:52, Marek Marczykowski-Górecki  wrote:
> On Wed, Oct 09, 2019 at 01:48:38PM +0200, Jan Beulich wrote:
>> On 09.10.2019 13:00, Marek Marczykowski-Górecki  wrote:
>>> On Wed, Oct 09, 2019 at 12:50:09PM +0200, Jan Beulich wrote:
>>>> On 09.10.2019 12:31, Marek Marczykowski-Górecki  wrote:
>>>>> BTW How runtime services work after kexec? I don't see EFI handles
>>>>> handed over kexec, are they somehow re-discovered?
>>>>
>>>> What EFI handles are you talking about? For runtime services
>>>> what a consumer needs is a table pointer, which is a field
>>>> in the system table, which in turn is an argument passed to
>>>> the EFI application's entry point.
>>>
>>> Yes, I'm talking about those pointers (system table specifically).
>>>
>>>> I didn't think there are
>>>> provisions in the spec for either of these pointers being NULL.
>>>
>>> But I don't see kexec using EFI application entry point. Am I missing
>>> something?
>>
>> Can we stop thinking about a Linux -> Xen transition on this
>> thread please? 
> 
> I'm talking about Xen->Xen transition here. How system table pointer is
> passed from old Xen to new Xen instance? And how the new Xen instance
> deals with boot services being not available anymore?

It doesn't. I should better have said "* -> Xen transitions" in
my earlier reply. I simply can't see how this can all work with
EFI underneath without some extra conveying of data from the old
to the new instance.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 2268 bytes --]

On Wed, Oct 09, 2019 at 02:07:05PM +0200, Jan Beulich wrote:
> On 09.10.2019 13:52, Marek Marczykowski-Górecki  wrote:
> > On Wed, Oct 09, 2019 at 01:48:38PM +0200, Jan Beulich wrote:
> >> On 09.10.2019 13:00, Marek Marczykowski-Górecki  wrote:
> >>> On Wed, Oct 09, 2019 at 12:50:09PM +0200, Jan Beulich wrote:
> >>>> On 09.10.2019 12:31, Marek Marczykowski-Górecki  wrote:
> >>>>> BTW How runtime services work after kexec? I don't see EFI handles
> >>>>> handed over kexec, are they somehow re-discovered?
> >>>>
> >>>> What EFI handles are you talking about? For runtime services
> >>>> what a consumer needs is a table pointer, which is a field
> >>>> in the system table, which in turn is an argument passed to
> >>>> the EFI application's entry point.
> >>>
> >>> Yes, I'm talking about those pointers (system table specifically).
> >>>
> >>>> I didn't think there are
> >>>> provisions in the spec for either of these pointers being NULL.
> >>>
> >>> But I don't see kexec using EFI application entry point. Am I missing
> >>> something?
> >>
> >> Can we stop thinking about a Linux -> Xen transition on this
> >> thread please? 
> > 
> > I'm talking about Xen->Xen transition here. How system table pointer is
> > passed from old Xen to new Xen instance? And how the new Xen instance
> > deals with boot services being not available anymore?
> 
> It doesn't. I should better have said "* -> Xen transitions" in
> my earlier reply. I simply can't see how this can all work with
> EFI underneath without some extra conveying of data from the old
> to the new instance.

Does it mean the whole discussion about SetVirtualAddressMap() being
incompatible with kexec is moot, because runtime services (including
SetVirtualAddressMap()) are not used by Xen after kexec anyway? If I
understand correctly, you just said the Xen after kexec don't have
runtime services pointer.
If not, can you explain what exactly is the case when second call to
SetVirtualAddressMap() could happen (being the reason for #ifdef-ing it
out in efi/boot.c)?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Jan Beulich]

On 09.10.2019 14:21, Marek Marczykowski-Górecki  wrote:
> On Wed, Oct 09, 2019 at 02:07:05PM +0200, Jan Beulich wrote:
>> On 09.10.2019 13:52, Marek Marczykowski-Górecki  wrote:
>>> I'm talking about Xen->Xen transition here. How system table pointer is
>>> passed from old Xen to new Xen instance? And how the new Xen instance
>>> deals with boot services being not available anymore?
>>
>> It doesn't. I should better have said "* -> Xen transitions" in
>> my earlier reply. I simply can't see how this can all work with
>> EFI underneath without some extra conveying of data from the old
>> to the new instance.
> 
> Does it mean the whole discussion about SetVirtualAddressMap() being
> incompatible with kexec is moot, because runtime services (including
> SetVirtualAddressMap()) are not used by Xen after kexec anyway? If I
> understand correctly, you just said the Xen after kexec don't have
> runtime services pointer.

The concern is about kexec-ing to Linux (based on what I recall
from when I wrote this code; as said the situation may have
changed for modern Linux).

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 1504 bytes --]

On Wed, Oct 09, 2019 at 02:24:31PM +0200, Jan Beulich wrote:
> On 09.10.2019 14:21, Marek Marczykowski-Górecki  wrote:
> > On Wed, Oct 09, 2019 at 02:07:05PM +0200, Jan Beulich wrote:
> >> On 09.10.2019 13:52, Marek Marczykowski-Górecki  wrote:
> >>> I'm talking about Xen->Xen transition here. How system table pointer is
> >>> passed from old Xen to new Xen instance? And how the new Xen instance
> >>> deals with boot services being not available anymore?
> >>
> >> It doesn't. I should better have said "* -> Xen transitions" in
> >> my earlier reply. I simply can't see how this can all work with
> >> EFI underneath without some extra conveying of data from the old
> >> to the new instance.
> > 
> > Does it mean the whole discussion about SetVirtualAddressMap() being
> > incompatible with kexec is moot, because runtime services (including
> > SetVirtualAddressMap()) are not used by Xen after kexec anyway? If I
> > understand correctly, you just said the Xen after kexec don't have
> > runtime services pointer.
> 
> The concern is about kexec-ing to Linux (based on what I recall
> from when I wrote this code; as said the situation may have
> changed for modern Linux).

But then, Linux won't have EFI system table pointer either, no? I don't
see Xen passing it over in any way.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Jan Beulich]

On 09.10.2019 14:27, Marek Marczykowski-Górecki  wrote:
> On Wed, Oct 09, 2019 at 02:24:31PM +0200, Jan Beulich wrote:
>> On 09.10.2019 14:21, Marek Marczykowski-Górecki  wrote:
>>> On Wed, Oct 09, 2019 at 02:07:05PM +0200, Jan Beulich wrote:
>>>> On 09.10.2019 13:52, Marek Marczykowski-Górecki  wrote:
>>>>> I'm talking about Xen->Xen transition here. How system table pointer is
>>>>> passed from old Xen to new Xen instance? And how the new Xen instance
>>>>> deals with boot services being not available anymore?
>>>>
>>>> It doesn't. I should better have said "* -> Xen transitions" in
>>>> my earlier reply. I simply can't see how this can all work with
>>>> EFI underneath without some extra conveying of data from the old
>>>> to the new instance.
>>>
>>> Does it mean the whole discussion about SetVirtualAddressMap() being
>>> incompatible with kexec is moot, because runtime services (including
>>> SetVirtualAddressMap()) are not used by Xen after kexec anyway? If I
>>> understand correctly, you just said the Xen after kexec don't have
>>> runtime services pointer.
>>
>> The concern is about kexec-ing to Linux (based on what I recall
>> from when I wrote this code; as said the situation may have
>> changed for modern Linux).
> 
> But then, Linux won't have EFI system table pointer either, no? I don't
> see Xen passing it over in any way.

Making the system table pointer available e.g. to kexec userspace
(so it can pass it in whatever suitable way) would be an easy
addition. Use of SetVirtualAddressMap(), otoh, would have been a
hard to undo step if I had made Xen's EFI boot path rely on it.
The kexec situation wrt EFI was very much in flux back then, and
hence I didn't want to take unnecessary risks of future
complications. Any step changing the current state of affairs
wants to provide assurance that it doesn't close roads which we
may need to go at some point.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen 4.12 panic on Thinkpad W540 with UEFI mutiboot2, efi=no-rs workarounds it

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 1868 bytes --]

On Wed, Oct 09, 2019 at 03:31:49PM +0200, Jan Beulich wrote:
> On 09.10.2019 14:27, Marek Marczykowski-Górecki  wrote:
> > But then, Linux won't have EFI system table pointer either, no? I don't
> > see Xen passing it over in any way.
> 
> Making the system table pointer available e.g. to kexec userspace
> (so it can pass it in whatever suitable way) would be an easy
> addition. Use of SetVirtualAddressMap(), otoh, would have been a
> hard to undo step if I had made Xen's EFI boot path rely on it.
> The kexec situation wrt EFI was very much in flux back then, and
> hence I didn't want to take unnecessary risks of future
> complications. Any step changing the current state of affairs
> wants to provide assurance that it doesn't close roads which we
> may need to go at some point.

I don't agree with the above being a problem - as we can see, expanding
the kexec mechanism to pass EFI system table isn't really necessary to
make it usable for Linux doing crash dump (this is the use case of kexec
here, right?). But even if it would be, we're talking about some new
(possibly Linux-specific) mechanism - in that case, I don't see why it
couldn't also pass over memory map for the runtime services (as set via
SetVirtualAddressMap()) - similar as Linux->Linux kexec does.
On the other hand, lack of SetVirtualAddressMap() do cause real problems
now, making Xen unbootable on some machines. Or noticeably limited (with
efi=no-rs workaround) - while RS aren't that useful for the crash kernel,
they are useful for the main system.

Anyway, it's your call, as the maintainer. The patch series I've sent
implements the approach by your preference.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel