| Message ID | 4b42ae6d42d409a44496eb546efe72c5060b1fd9.1484335095.git-series.cardoe@cardoe.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 13.01.17 at 20:21, <cardoe@cardoe.com> wrote: > Doug v1 - fix incorrect assembly (identified by Andrew Cooper) > - fix issue where the trampoline size was left as 0 and the > way the memory is allocated for the trampolines we would go to > the end of an available section and then subtract off the size > to decide where to place it. The end result was that we would > always copy the trampolines and the 32-bit stack into some > form of reserved memory after the conventional region we > wanted to put things into. On some systems this did not > manifest as a crash while on others it did. Reworked the > changes to always reserve 64kb for both the stack and the size > of the trampolines. Added an ASSERT to make sure we never blow > through this size. Without having looked at the patch in detail, but knowing I did closely look at earlier versions (and iirc I was mostly fine with v10) the way the above is written would require me to either inter-diff the patches, or re-review the whole thing. For a large patch like this it would be rather helpful to be quite a bit more specific as to where exactly in the patch changes were made. Jan
On Mon, Jan 16, 2017 at 05:02:05AM -0700, Jan Beulich wrote: > >>> On 13.01.17 at 20:21, <cardoe@cardoe.com> wrote: > > Doug v1 - fix incorrect assembly (identified by Andrew Cooper) > > - fix issue where the trampoline size was left as 0 and the > > way the memory is allocated for the trampolines we would go to > > the end of an available section and then subtract off the size > > to decide where to place it. The end result was that we would > > always copy the trampolines and the 32-bit stack into some > > form of reserved memory after the conventional region we > > wanted to put things into. On some systems this did not > > manifest as a crash while on others it did. Reworked the > > changes to always reserve 64kb for both the stack and the size > > of the trampolines. Added an ASSERT to make sure we never blow > > through this size. > > Without having looked at the patch in detail, but knowing I did closely > look at earlier versions (and iirc I was mostly fine with v10) the way > the above is written would require me to either inter-diff the patches, > or re-review the whole thing. For a large patch like this it would be > rather helpful to be quite a bit more specific as to where exactly in the > patch changes were made. I would prefer to not have this patch series applied because it will make me more difficult to prepare v12. I hope that I will post it in about 2 weeks. Though I am going to take into account all comments posted by Doug for v11. Daniel
On 1/16/17 7:50 AM, Daniel Kiper wrote: > On Mon, Jan 16, 2017 at 05:02:05AM -0700, Jan Beulich wrote: >>>>> On 13.01.17 at 20:21, <cardoe@cardoe.com> wrote: >>> Doug v1 - fix incorrect assembly (identified by Andrew Cooper) >>> - fix issue where the trampoline size was left as 0 and the >>> way the memory is allocated for the trampolines we would go to >>> the end of an available section and then subtract off the size >>> to decide where to place it. The end result was that we would >>> always copy the trampolines and the 32-bit stack into some >>> form of reserved memory after the conventional region we >>> wanted to put things into. On some systems this did not >>> manifest as a crash while on others it did. Reworked the >>> changes to always reserve 64kb for both the stack and the size >>> of the trampolines. Added an ASSERT to make sure we never blow >>> through this size. >> >> Without having looked at the patch in detail, but knowing I did closely >> look at earlier versions (and iirc I was mostly fine with v10) the way >> the above is written would require me to either inter-diff the patches, >> or re-review the whole thing. For a large patch like this it would be >> rather helpful to be quite a bit more specific as to where exactly in the >> patch changes were made. > > I would prefer to not have this patch series applied because it will make me > more difficult to prepare v12. I hope that I will post it in about 2 weeks. > Though I am going to take into account all comments posted by Doug for v11. > > Daniel > Why? They're the first 4 patches remaining of your series. It'll literally be the following commands: git fetch origin git rebase origin/staging
On 1/16/17 7:02 AM, Jan Beulich wrote: >>>> On 13.01.17 at 20:21, <cardoe@cardoe.com> wrote: >> Doug v1 - fix incorrect assembly (identified by Andrew Cooper) >> - fix issue where the trampoline size was left as 0 and the >> way the memory is allocated for the trampolines we would go to >> the end of an available section and then subtract off the size >> to decide where to place it. The end result was that we would >> always copy the trampolines and the 32-bit stack into some >> form of reserved memory after the conventional region we >> wanted to put things into. On some systems this did not >> manifest as a crash while on others it did. Reworked the >> changes to always reserve 64kb for both the stack and the size >> of the trampolines. Added an ASSERT to make sure we never blow >> through this size. > > Without having looked at the patch in detail, but knowing I did closely > look at earlier versions (and iirc I was mostly fine with v10) the way > the above is written would require me to either inter-diff the patches, > or re-review the whole thing. For a large patch like this it would be > rather helpful to be quite a bit more specific as to where exactly in the > patch changes were made. > > Jan > I'll submit a diff against v11 to help show the difference. I can also submit a difference against v10 if you want as well.
On Mon, Jan 16, 2017 at 08:41:08AM -0500, Doug Goldstein wrote: > On 1/16/17 7:50 AM, Daniel Kiper wrote: > > On Mon, Jan 16, 2017 at 05:02:05AM -0700, Jan Beulich wrote: > >>>>> On 13.01.17 at 20:21, <cardoe@cardoe.com> wrote: > >>> Doug v1 - fix incorrect assembly (identified by Andrew Cooper) > >>> - fix issue where the trampoline size was left as 0 and the > >>> way the memory is allocated for the trampolines we would go to > >>> the end of an available section and then subtract off the size > >>> to decide where to place it. The end result was that we would > >>> always copy the trampolines and the 32-bit stack into some > >>> form of reserved memory after the conventional region we > >>> wanted to put things into. On some systems this did not > >>> manifest as a crash while on others it did. Reworked the > >>> changes to always reserve 64kb for both the stack and the size > >>> of the trampolines. Added an ASSERT to make sure we never blow > >>> through this size. > >> > >> Without having looked at the patch in detail, but knowing I did closely > >> look at earlier versions (and iirc I was mostly fine with v10) the way > >> the above is written would require me to either inter-diff the patches, > >> or re-review the whole thing. For a large patch like this it would be > >> rather helpful to be quite a bit more specific as to where exactly in the > >> patch changes were made. > > > > I would prefer to not have this patch series applied because it will make me > > more difficult to prepare v12. I hope that I will post it in about 2 weeks. > > Though I am going to take into account all comments posted by Doug for v11. > > > > Daniel > > > > Why? They're the first 4 patches remaining of your series. It'll > literally be the following commands: > > git fetch origin > git rebase origin/staging If you changed something then probably this is not so simple. Second, Jan asked me to reorder some patches in the series. Last but not least, your patch series does not contain whole functionality which is needed to properly load Xen using multiboot2 protocol on most EFI platforms. So, as above. Though I appreciate your feedback and I will take all your changes into account. Daniel
On 1/16/17 9:11 AM, Daniel Kiper wrote: > On Mon, Jan 16, 2017 at 08:41:08AM -0500, Doug Goldstein wrote: >> On 1/16/17 7:50 AM, Daniel Kiper wrote: >>> On Mon, Jan 16, 2017 at 05:02:05AM -0700, Jan Beulich wrote: >>>>>>> On 13.01.17 at 20:21, <cardoe@cardoe.com> wrote: >>>>> Doug v1 - fix incorrect assembly (identified by Andrew Cooper) >>>>> - fix issue where the trampoline size was left as 0 and the >>>>> way the memory is allocated for the trampolines we would go to >>>>> the end of an available section and then subtract off the size >>>>> to decide where to place it. The end result was that we would >>>>> always copy the trampolines and the 32-bit stack into some >>>>> form of reserved memory after the conventional region we >>>>> wanted to put things into. On some systems this did not >>>>> manifest as a crash while on others it did. Reworked the >>>>> changes to always reserve 64kb for both the stack and the size >>>>> of the trampolines. Added an ASSERT to make sure we never blow >>>>> through this size. >>>> >>>> Without having looked at the patch in detail, but knowing I did closely >>>> look at earlier versions (and iirc I was mostly fine with v10) the way >>>> the above is written would require me to either inter-diff the patches, >>>> or re-review the whole thing. For a large patch like this it would be >>>> rather helpful to be quite a bit more specific as to where exactly in the >>>> patch changes were made. >>> >>> I would prefer to not have this patch series applied because it will make me >>> more difficult to prepare v12. I hope that I will post it in about 2 weeks. >>> Though I am going to take into account all comments posted by Doug for v11. >>> >>> Daniel >>> >> >> Why? They're the first 4 patches remaining of your series. It'll >> literally be the following commands: >> >> git fetch origin >> git rebase origin/staging > > If you changed something then probably this is not so simple. > Second, Jan asked me to reorder some patches in the series. > Last but not least, your patch series does not contain whole > functionality which is needed to properly load Xen using > multiboot2 protocol on most EFI platforms. So, as above. > Though I appreciate your feedback and I will take all > your changes into account. > > Daniel > Yes there are some code changes which is the point of me sending this. But the work for you is the same as having to rebase your series over the past few years. Its still a rebase. Its the same thing that everyone submitting patches has to do when they submit another version of their patches. I believe you identified 1 machine that had an issue with the 1mb region. I've used a 12 machines to test this series and these 4 patches work on those machines. The relocation part of the series works on NONE of the machines I've tested with. So I would argue the opposite. I still haven't identified what's wrong with the relocation part of the series.
On Mon, Jan 16, 2017 at 09:28:45AM -0500, Doug Goldstein wrote: > On 1/16/17 9:11 AM, Daniel Kiper wrote: > > On Mon, Jan 16, 2017 at 08:41:08AM -0500, Doug Goldstein wrote: > >> On 1/16/17 7:50 AM, Daniel Kiper wrote: > >>> On Mon, Jan 16, 2017 at 05:02:05AM -0700, Jan Beulich wrote: > >>>>>>> On 13.01.17 at 20:21, <cardoe@cardoe.com> wrote: > >>>>> Doug v1 - fix incorrect assembly (identified by Andrew Cooper) > >>>>> - fix issue where the trampoline size was left as 0 and the > >>>>> way the memory is allocated for the trampolines we would go to > >>>>> the end of an available section and then subtract off the size > >>>>> to decide where to place it. The end result was that we would > >>>>> always copy the trampolines and the 32-bit stack into some > >>>>> form of reserved memory after the conventional region we > >>>>> wanted to put things into. On some systems this did not > >>>>> manifest as a crash while on others it did. Reworked the > >>>>> changes to always reserve 64kb for both the stack and the size > >>>>> of the trampolines. Added an ASSERT to make sure we never blow > >>>>> through this size. > >>>> > >>>> Without having looked at the patch in detail, but knowing I did closely > >>>> look at earlier versions (and iirc I was mostly fine with v10) the way > >>>> the above is written would require me to either inter-diff the patches, > >>>> or re-review the whole thing. For a large patch like this it would be > >>>> rather helpful to be quite a bit more specific as to where exactly in the > >>>> patch changes were made. > >>> > >>> I would prefer to not have this patch series applied because it will make me > >>> more difficult to prepare v12. I hope that I will post it in about 2 weeks. > >>> Though I am going to take into account all comments posted by Doug for v11. > >>> > >>> Daniel > >>> > >> > >> Why? They're the first 4 patches remaining of your series. It'll > >> literally be the following commands: > >> > >> git fetch origin > >> git rebase origin/staging > > > > If you changed something then probably this is not so simple. > > Second, Jan asked me to reorder some patches in the series. > > Last but not least, your patch series does not contain whole > > functionality which is needed to properly load Xen using > > multiboot2 protocol on most EFI platforms. So, as above. > > Though I appreciate your feedback and I will take all > > your changes into account. > > > > Daniel > > > > Yes there are some code changes which is the point of me sending this. > But the work for you is the same as having to rebase your series over > the past few years. Its still a rebase. Its the same thing that everyone > submitting patches has to do when they submit another version of their > patches. Yes, I know. It is always easy to say. And I do not know why you are in such a hurry. Could not you wait 1-2 weeks? > I believe you identified 1 machine that had an issue with the 1mb > region. I've used a 12 machines to test this series and these 4 patches > work on those machines. The relocation part of the series works on NONE > of the machines I've tested with. So I would argue the opposite. I still > haven't identified what's wrong with the relocation part of the series. This is very interesting. As I know similar thing is used on many machines for a year or two. And I have not received any complaints until now. Though I do not say that it is perfect. If you find something just drop me a line and I will fix it. Daniel
On Mon, Jan 16, 2017 at 3:16 PM, Daniel Kiper <daniel.kiper@oracle.com> wrote: > On Mon, Jan 16, 2017 at 09:28:45AM -0500, Doug Goldstein wrote: >> On 1/16/17 9:11 AM, Daniel Kiper wrote: >> > On Mon, Jan 16, 2017 at 08:41:08AM -0500, Doug Goldstein wrote: >> >> On 1/16/17 7:50 AM, Daniel Kiper wrote: >> >>> I would prefer to not have this patch series applied because it will make me >> >>> more difficult to prepare v12. I hope that I will post it in about 2 weeks. >> >>> Though I am going to take into account all comments posted by Doug for v11. [snip] >> >> Why? They're the first 4 patches remaining of your series. It'll >> >> literally be the following commands: >> >> >> >> git fetch origin >> >> git rebase origin/staging >> > >> > If you changed something then probably this is not so simple. >> > Second, Jan asked me to reorder some patches in the series. >> > Last but not least, your patch series does not contain whole >> > functionality which is needed to properly load Xen using >> > multiboot2 protocol on most EFI platforms. So, as above. >> > Though I appreciate your feedback and I will take all >> > your changes into account. [snip] >> Yes there are some code changes which is the point of me sending this. >> But the work for you is the same as having to rebase your series over >> the past few years. Its still a rebase. Its the same thing that everyone >> submitting patches has to do when they submit another version of their >> patches. > > Yes, I know. It is always easy to say. And I do not know why you are in > such a hurry. Could not you wait 1-2 weeks? I can't speak to the technical merits of Doug's patch, but I have to say I'm not very happy with this way of interacting. No one should be able to block other people's valid contributions from going in to the tree for their own convenience -- it discourages contribution and sets the incentives all wrong. Doing a difficult rebase is something everyone has to do sometimes; and it's not clear that it *will* be difficult, as it sounds like Daniel hasn't even tried it. -George
On Tue, Jan 17, 2017 at 12:05:21PM +0000, George Dunlap wrote: > On Mon, Jan 16, 2017 at 3:16 PM, Daniel Kiper <daniel.kiper@oracle.com> wrote: [...] > >> Yes there are some code changes which is the point of me sending this. > >> But the work for you is the same as having to rebase your series over > >> the past few years. Its still a rebase. Its the same thing that everyone > >> submitting patches has to do when they submit another version of their > >> patches. > > > > Yes, I know. It is always easy to say. And I do not know why you are in > > such a hurry. Could not you wait 1-2 weeks? > > I can't speak to the technical merits of Doug's patch, but I have to > say I'm not very happy with this way of interacting. No one should be > able to block other people's valid contributions from going in to the > tree for their own convenience -- it discourages contribution and sets > the incentives all wrong. Doing a difficult rebase is something > everyone has to do sometimes; and it's not clear that it *will* be > difficult, as it sounds like Daniel hasn't even tried it. First of all, I would like to underline that never ever I would not like to hinder a cooperation here or anywhere else. If it looks like that then sorry. It was not my intention. Though at least until now I try to maintain this patch series and nothing will change (at least I do not have such plans) until it will be fully taken into Xen tree. I appreciate any feedback and in this case Doug's one. I am happy that more and more people testing it and find it useful. However, I think that it makes a confusion if we step on each other foot and everybody tries to post own version of a given patch series. I would not have any concerns if this project would be abandoned. However, this is not the case. Anyway, if maintainers and in particular Jan and/or Andrew will take decision to merge some or all Doug's patches I will not argue. And once again, I appreciate everybody feedback and in particular Doug's one. If I am missing something drop me a line. Daniel
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S index d423fd8..876a6b1 100644 --- a/xen/arch/x86/boot/head.S +++ b/xen/arch/x86/boot/head.S @@ -89,6 +89,13 @@ multiboot2_header_start: 0, /* Number of the lines - no preference. */ \ 0 /* Number of bits per pixel - no preference. */ + /* Inhibit bootloader from calling ExitBootServices(). */ + mb2ht_init MB2_HT(EFI_BS), MB2_HT(OPTIONAL) + + /* EFI64 entry point. */ + mb2ht_init MB2_HT(ENTRY_ADDRESS_EFI64), MB2_HT(OPTIONAL), \ + sym_phys(__efi64_start) + /* Multiboot2 header end tag. */ mb2ht_init MB2_HT(END), MB2_HT(REQUIRED) .Lmultiboot2_header_end: @@ -100,20 +107,48 @@ multiboot2_header_start: gdt_boot_descr: .word 6*8-1 .long sym_phys(trampoline_gdt) + .long 0 /* Needed for 64-bit lgdt */ + + .align 4 +vga_text_buffer: + .long 0xb8000 + +efi_platform: + .byte 0 .Lbad_cpu_msg: .asciz "ERR: Not a 64-bit CPU!" .Lbad_ldr_msg: .asciz "ERR: Not a Multiboot bootloader!" +.Lbad_ldr_nbs: .asciz "ERR: Bootloader shutdown EFI x64 boot services!" +.Lbad_ldr_nst: .asciz "ERR: EFI SystemTable is not provided by bootloader!" +.Lbad_ldr_nih: .asciz "ERR: EFI ImageHandle is not provided by bootloader!" +.Lbad_efi_msg: .asciz "ERR: EFI IA-32 platforms are not supported!" .section .init.text, "ax", @progbits bad_cpu: mov $(sym_phys(.Lbad_cpu_msg)),%esi # Error message - jmp print_err + jmp .Lget_vtb not_multiboot: mov $(sym_phys(.Lbad_ldr_msg)),%esi # Error message -print_err: - mov $0xB8000,%edi # VGA framebuffer -1: mov (%esi),%bl + jmp .Lget_vtb +.Lmb2_no_st: + mov $(sym_phys(.Lbad_ldr_nst)),%esi # Error message + jmp .Lget_vtb +.Lmb2_no_ih: + mov $(sym_phys(.Lbad_ldr_nih)),%esi # Error message + jmp .Lget_vtb +.Lmb2_no_bs: + mov $(sym_phys(.Lbad_ldr_nbs)),%esi # Error message + xor %edi,%edi # No VGA text buffer + jmp .Lsend_chr +.Lmb2_efi_ia_32: + mov $(sym_phys(.Lbad_efi_msg)),%esi # Error message + xor %edi,%edi # No VGA text buffer + jmp .Lsend_chr +.Lget_vtb: + mov sym_phys(vga_text_buffer),%edi +.Lsend_chr: + mov (%esi),%bl test %bl,%bl # Terminate on '\0' sentinel je .Lhalt mov $0x3f8+5,%dx # UART Line Status Register @@ -123,13 +158,186 @@ print_err: mov $0x3f8+0,%dx # UART Transmit Holding Register mov %bl,%al out %al,%dx # Send a character over the serial line - movsb # Write a character to the VGA framebuffer + test %edi,%edi # Is the VGA text buffer available? + jz .Lsend_chr + movsb # Write a character to the VGA text buffer mov $7,%al - stosb # Write an attribute to the VGA framebuffer - jmp 1b + stosb # Write an attribute to the VGA text buffer + jmp .Lsend_chr .Lhalt: hlt jmp .Lhalt + .code64 + +__efi64_start: + cld + + /* VGA is not available on EFI platforms. */ + movl $0,vga_text_buffer(%rip) + + /* Check for Multiboot2 bootloader. */ + cmp $MULTIBOOT2_BOOTLOADER_MAGIC,%eax + je .Lefi_multiboot2_proto + + /* Jump to not_multiboot after switching CPU to x86_32 mode. */ + lea not_multiboot(%rip),%edi + jmp x86_32_switch + +.Lefi_multiboot2_proto: + /* Zero EFI SystemTable and EFI ImageHandle addresses. */ + xor %esi,%esi + xor %edi,%edi + + /* Skip Multiboot2 information fixed part. */ + lea (MB2_fixed_sizeof+MULTIBOOT2_TAG_ALIGN-1)(%rbx),%ecx + and $~(MULTIBOOT2_TAG_ALIGN-1),%ecx + +.Lefi_mb2_tsize: + /* Check Multiboot2 information total size. */ + mov %ecx,%r8d + sub %ebx,%r8d + cmp %r8d,MB2_fixed_total_size(%rbx) + jbe run_bs + + /* Are EFI boot services available? */ + cmpl $MULTIBOOT2_TAG_TYPE_EFI_BS,MB2_tag_type(%rcx) + jne .Lefi_mb2_st + + /* We are on EFI platform and EFI boot services are available. */ + incb efi_platform(%rip) + + /* + * Disable real mode and other legacy stuff which should not + * be run on EFI platforms. + */ + incb skip_realmode(%rip) + jmp .Lefi_mb2_next_tag + +.Lefi_mb2_st: + /* Get EFI SystemTable address from Multiboot2 information. */ + cmpl $MULTIBOOT2_TAG_TYPE_EFI64,MB2_tag_type(%rcx) + cmove MB2_efi64_st(%rcx),%rsi + je .Lefi_mb2_next_tag + + /* Get EFI ImageHandle address from Multiboot2 information. */ + cmpl $MULTIBOOT2_TAG_TYPE_EFI64_IH,MB2_tag_type(%rcx) + cmove MB2_efi64_ih(%rcx),%rdi + je .Lefi_mb2_next_tag + + /* Is it the end of Multiboot2 information? */ + cmpl $MULTIBOOT2_TAG_TYPE_END,MB2_tag_type(%rcx) + je run_bs + +.Lefi_mb2_next_tag: + /* Go to next Multiboot2 information tag. */ + add MB2_tag_size(%rcx),%ecx + add $(MULTIBOOT2_TAG_ALIGN-1),%ecx + and $~(MULTIBOOT2_TAG_ALIGN-1),%ecx + jmp .Lefi_mb2_tsize + +run_bs: + /* Are EFI boot services available? */ + cmpb $0,efi_platform(%rip) + jnz 0f + + /* Jump to .Lmb2_no_bs after switching CPU to x86_32 mode. */ + lea .Lmb2_no_bs(%rip),%edi + jmp x86_32_switch + +0: + /* Is EFI SystemTable address provided by boot loader? */ + test %rsi,%rsi + jnz 1f + + /* Jump to .Lmb2_no_st after switching CPU to x86_32 mode. */ + lea .Lmb2_no_st(%rip),%edi + jmp x86_32_switch + +1: + /* Is EFI ImageHandle address provided by boot loader? */ + test %rdi,%rdi + jnz 2f + + /* Jump to .Lmb2_no_ih after switching CPU to x86_32 mode. */ + lea .Lmb2_no_ih(%rip),%edi + jmp x86_32_switch + +2: + push %rax + push %rdi + + /* + * Initialize BSS (no nasty surprises!). + * It must be done earlier than in BIOS case + * because efi_multiboot2() touches it. + */ + lea __bss_start(%rip),%edi + lea __bss_end(%rip),%ecx + sub %edi,%ecx + shr $3,%ecx + xor %eax,%eax + rep stosq + + pop %rdi + + /* + * efi_multiboot2() is called according to System V AMD64 ABI: + * - IN: %rdi - EFI ImageHandle, %rsi - EFI SystemTable, + * - OUT: %rax - highest usable memory address below 1 MiB; + * memory above this address is reserved for trampoline; + * memory below this address is used as a storage for + * mbi struct created in reloc(). + * + * MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO tag is not provided + * on EFI platforms. Hence, it could not be used like + * on legacy BIOS platforms. + */ + call efi_multiboot2 + + /* Convert memory address to bytes/16 and store it in safe place. */ + shr $4,%eax + mov %eax,%ecx + + pop %rax + + /* Jump to trampoline_setup after switching CPU to x86_32 mode. */ + lea trampoline_setup(%rip),%edi + +x86_32_switch: + cli + + /* Initialize GDTR. */ + lgdt gdt_boot_descr(%rip) + + /* Reload code selector. */ + pushq $BOOT_CS32 + lea cs32_switch(%rip),%edx + push %rdx + lretq + + .code32 + +cs32_switch: + /* Initialize basic data segments. */ + mov $BOOT_DS,%edx + mov %edx,%ds + mov %edx,%es + mov %edx,%ss + /* %esp is initialized later. */ + + /* Load null descriptor to unused segment registers. */ + xor %edx,%edx + mov %edx,%fs + mov %edx,%gs + + /* Disable paging. */ + mov %cr0,%edx + and $(~X86_CR0_PG),%edx + mov %edx,%cr0 + + /* Jump to earlier loaded address. */ + jmp *%edi + __start: cld cli @@ -157,7 +365,7 @@ __start: /* Not available? BDA value will be fine. */ cmovnz MB_mem_lower(%ebx),%edx - jmp trampoline_setup + jmp trampoline_bios_setup .Lmultiboot2_proto: /* Skip Multiboot2 information fixed part. */ @@ -169,24 +377,33 @@ __start: mov %ecx,%edi sub %ebx,%edi cmp %edi,MB2_fixed_total_size(%ebx) - jbe trampoline_setup + jbe trampoline_bios_setup /* Get mem_lower from Multiboot2 information. */ cmpl $MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO,MB2_tag_type(%ecx) cmove MB2_mem_lower(%ecx),%edx - je trampoline_setup + je .Lmb2_next_tag + + /* EFI IA-32 platforms are not supported. */ + cmpl $MULTIBOOT2_TAG_TYPE_EFI32,MB2_tag_type(%ecx) + je .Lmb2_efi_ia_32 + + /* Bootloader shutdown EFI x64 boot services. */ + cmpl $MULTIBOOT2_TAG_TYPE_EFI64,MB2_tag_type(%ecx) + je .Lmb2_no_bs /* Is it the end of Multiboot2 information? */ cmpl $MULTIBOOT2_TAG_TYPE_END,MB2_tag_type(%ecx) - je trampoline_setup + je trampoline_bios_setup +.Lmb2_next_tag: /* Go to next Multiboot2 information tag. */ add MB2_tag_size(%ecx),%ecx add $(MULTIBOOT2_TAG_ALIGN-1),%ecx and $~(MULTIBOOT2_TAG_ALIGN-1),%ecx jmp .Lmb2_tsize -trampoline_setup: +trampoline_bios_setup: /* Set up trampoline segment 64k below EBDA */ movzwl 0x40e,%ecx /* EBDA segment */ cmp $0xa000,%ecx /* sanity check (high) */ @@ -202,16 +419,19 @@ trampoline_setup: * multiboot structure (if available) and use the smallest. */ cmp $0x100,%edx /* is the multiboot value too small? */ - jb 2f /* if so, do not use it */ + jb trampoline_setup /* if so, do not use it */ shl $10-4,%edx cmp %ecx,%edx /* compare with BDA value */ cmovb %edx,%ecx /* and use the smaller */ -2: /* Reserve 64kb for the trampoline */ + /* Reserve 64kb for the trampoline. */ sub $0x1000,%ecx /* From arch/x86/smpboot.c: start_eip had better be page-aligned! */ xor %cl, %cl + +trampoline_setup: + /* Save trampoline address for later use. */ shl $4, %ecx mov %ecx,sym_phys(trampoline_phys) @@ -223,7 +443,14 @@ trampoline_setup: call reloc mov %eax,sym_phys(multiboot_ptr) - /* Initialize BSS (no nasty surprises!) */ + /* + * Do not zero BSS on EFI platform here. + * It was initialized earlier. + */ + cmpb $0,sym_phys(efi_platform) + jnz 1f + + /* Initialize BSS (no nasty surprises!). */ mov $sym_phys(__bss_start),%edi mov $sym_phys(__bss_end),%ecx sub %edi,%ecx @@ -231,6 +458,7 @@ trampoline_setup: shr $2,%ecx rep stosl +1: /* Interrogate CPU extended features via CPUID. */ mov $0x80000000,%eax cpuid @@ -282,10 +510,16 @@ trampoline_setup: cmp $sym_phys(__trampoline_seg_stop),%edi jb 1b + /* Do not parse command line on EFI platform here. */ + cmpb $0,sym_phys(efi_platform) + jnz 1f + call cmdline_parse_early +1: /* Switch to low-memory stack. */ mov sym_phys(trampoline_phys),%edi + /* The stack starts 64kb after the location of trampoline_phys */ lea 0x10000(%edi),%esp lea trampoline_boot_cpu_entry-trampoline_start(%edi),%eax pushl $BOOT_CS32 diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h index 62c010e..3871de7 100644 --- a/xen/arch/x86/efi/efi-boot.h +++ b/xen/arch/x86/efi/efi-boot.h @@ -169,6 +169,7 @@ static void __init efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable, case EfiConventionalMemory: if ( !trampoline_phys && desc->PhysicalStart + len <= 0x100000 && len >= cfg.size && desc->PhysicalStart + len > cfg.addr ) + ASSERT(cfg.size > 0); cfg.addr = (desc->PhysicalStart + len - cfg.size) & PAGE_MASK; /* fall through */ case EfiLoaderCode: @@ -210,12 +211,14 @@ static void *__init efi_arch_allocate_mmap_buffer(UINTN map_size) static void __init efi_arch_pre_exit_boot(void) { - if ( !trampoline_phys ) - { - if ( !cfg.addr ) - blexit(L"No memory for trampoline"); + if ( trampoline_phys ) + return; + + if ( !cfg.addr ) + blexit(L"No memory for trampoline"); + + if ( efi_enabled(EFI_LOADER) ) relocate_trampoline(cfg.addr); - } } static void __init noreturn efi_arch_post_exit_boot(void) @@ -653,6 +656,47 @@ static bool_t __init efi_arch_use_config_file(EFI_SYSTEM_TABLE *SystemTable) static void efi_arch_flush_dcache_area(const void *vaddr, UINTN size) { } +paddr_t __init efi_multiboot2(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) +{ + EFI_GRAPHICS_OUTPUT_PROTOCOL *gop; + UINTN cols, gop_mode = ~0, rows; + + __set_bit(EFI_BOOT, &efi_flags); + __set_bit(EFI_RS, &efi_flags); + + efi_init(ImageHandle, SystemTable); + + efi_console_set_mode(); + + if ( StdOut->QueryMode(StdOut, StdOut->Mode->Mode, + &cols, &rows) == EFI_SUCCESS ) + efi_arch_console_init(cols, rows); + + gop = efi_get_gop(); + + if ( gop ) + gop_mode = efi_find_gop_mode(gop, 0, 0, 0); + + efi_arch_edd(); + efi_arch_cpu(); + + efi_tables(); + setup_efi_pci(); + efi_variables(); + + /* This is the maximum size of our trampoline + our low memory stack */ + cfg.size = 64 << 10; + ASSERT(cfg.size >= ((trampoline_end - trampoline_start) + 4096)); + + if ( gop ) + efi_set_gop_mode(gop, gop_mode); + + efi_exit_boot(ImageHandle, SystemTable); + + /* Return highest usable memory address below 1 MiB. */ + return cfg.addr; +} + /* * Local variables: * mode: C diff --git a/xen/arch/x86/efi/stub.c b/xen/arch/x86/efi/stub.c index 4158124..b81adc0 100644 --- a/xen/arch/x86/efi/stub.c +++ b/xen/arch/x86/efi/stub.c @@ -3,6 +3,44 @@ #include <xen/init.h> #include <xen/lib.h> #include <asm/page.h> +#include <asm/efibind.h> +#include <efi/efidef.h> +#include <efi/eficapsule.h> +#include <efi/eficon.h> +#include <efi/efidevp.h> +#include <efi/efiapi.h> + +/* + * Here we are in EFI stub. EFI calls are not supported due to lack + * of relevant functionality in compiler and/or linker. + * + * efi_multiboot2() is an exception. Please look below for more details. + */ + +paddr_t __init noreturn efi_multiboot2(EFI_HANDLE ImageHandle, + EFI_SYSTEM_TABLE *SystemTable) +{ + CHAR16 *err = L"Xen does not have EFI code build in!!!\r\nSystem halted!!!\r\n"; + SIMPLE_TEXT_OUTPUT_INTERFACE *StdErr; + + StdErr = SystemTable->StdErr ? SystemTable->StdErr : SystemTable->ConOut; + + /* + * Print error message and halt the system. + * + * We have to open code MS x64 calling convention + * in assembly because here this convention may + * not be directly supported by C compiler. + */ + asm volatile( + " call *%2 \n" + "0: hlt \n" + " jmp 0b \n" + : "+c" (StdErr), "+d" (err) : "g" (StdErr->OutputString) + : "rax", "r8", "r9", "r10", "r11", "memory"); + + unreachable(); +} bool efi_enabled(unsigned int feature) { diff --git a/xen/arch/x86/x86_64/asm-offsets.c b/xen/arch/x86/x86_64/asm-offsets.c index 92f5d81..f135654 100644 --- a/xen/arch/x86/x86_64/asm-offsets.c +++ b/xen/arch/x86/x86_64/asm-offsets.c @@ -175,6 +175,8 @@ void __dummy__(void) OFFSET(MB2_tag_type, multiboot2_tag_t, type); OFFSET(MB2_tag_size, multiboot2_tag_t, size); OFFSET(MB2_mem_lower, multiboot2_tag_basic_meminfo_t, mem_lower); + OFFSET(MB2_efi64_st, multiboot2_tag_efi64_t, pointer); + OFFSET(MB2_efi64_ih, multiboot2_tag_efi64_ih_t, pointer); BLANK(); OFFSET(DOMAIN_vm_assist, struct domain, vm_assist); diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S index b0b1c9b..e0e2529 100644 --- a/xen/arch/x86/xen.lds.S +++ b/xen/arch/x86/xen.lds.S @@ -331,5 +331,5 @@ ASSERT(IS_ALIGNED(__init_end, PAGE_SIZE), "__init_end misaligned") ASSERT(IS_ALIGNED(trampoline_start, 4), "trampoline_start misaligned") ASSERT(IS_ALIGNED(trampoline_end, 4), "trampoline_end misaligned") -ASSERT(IS_ALIGNED(__bss_start, 4), "__bss_start misaligned") -ASSERT(IS_ALIGNED(__bss_end, 4), "__bss_end misaligned") +ASSERT(IS_ALIGNED(__bss_start, 8), "__bss_start misaligned") +ASSERT(IS_ALIGNED(__bss_end, 8), "__bss_end misaligned") diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c index 36dbb71..b6cbdad 100644 --- a/xen/common/efi/boot.c +++ b/xen/common/efi/boot.c @@ -79,6 +79,17 @@ static size_t wstrlen(const CHAR16 * s); static int set_color(u32 mask, int bpp, u8 *pos, u8 *sz); static bool_t match_guid(const EFI_GUID *guid1, const EFI_GUID *guid2); +static void efi_init(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable); +static void efi_console_set_mode(void); +static EFI_GRAPHICS_OUTPUT_PROTOCOL *efi_get_gop(void); +static UINTN efi_find_gop_mode(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop, + UINTN cols, UINTN rows, UINTN depth); +static void efi_tables(void); +static void setup_efi_pci(void); +static void efi_variables(void); +static void efi_set_gop_mode(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop, UINTN gop_mode); +static void efi_exit_boot(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable); + static const EFI_BOOT_SERVICES *__initdata efi_bs; static UINT32 __initdata efi_bs_revision; static EFI_HANDLE __initdata efi_ih;