Message ID | 1549390526-24246-19-git-send-email-pbonzini@redhat.com |
---|---|
State | New |
Headers | show |
Series | [PULL,01/76] vhost-user-test: use g_cond_broadcast | expand |
Hi Stefano, On 2/5/19 7:14 PM, Paolo Bonzini wrote: > From: Stefano Garzarella <sgarzare@redhat.com> > > The new pvh.bin option rom can be used with SeaBIOS to boot > uncompressed kernel using the x86/HVM direct boot ABI. > > pvh.S contains the entry point of the option rom. It runs > in real mode, loads the e820 table querying the BIOS, and > then it switches to 32bit protected mode and jumps to the > pvh_load_kernel() written in pvh_main.c. > pvh_load_kernel() loads the cmdline and kernel entry_point > using fw_cfg, then it looks for RSDP, fills the > hvm_start_info required by x86/HVM ABI, and finally jumps > to the kernel entry_point. > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > Reviewed-by: Liam Merwick <liam.merwick@oracle.com> > --- > .gitignore | 4 + > Makefile | 2 +- > pc-bios/optionrom/Makefile | 5 +- > pc-bios/optionrom/pvh.S | 200 +++++++++++++++++++++++++++++++++++++++++++ > pc-bios/optionrom/pvh_main.c | 116 +++++++++++++++++++++++++ > pc-bios/pvh.bin | Bin 0 -> 1536 bytes > 6 files changed, 325 insertions(+), 2 deletions(-) > create mode 100644 pc-bios/optionrom/pvh.S > create mode 100644 pc-bios/optionrom/pvh_main.c > create mode 100644 pc-bios/pvh.bin > +++ b/pc-bios/optionrom/Makefile > @@ -37,7 +37,7 @@ Wa = -Wa, > ASFLAGS += -32 > QEMU_CFLAGS += $(call cc-c-option, $(QEMU_CFLAGS), $(Wa)-32) > > -build-all: multiboot.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin > +build-all: multiboot.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin pvh.bin > > +++ b/pc-bios/optionrom/pvh_main.c > @@ -0,0 +1,116 @@ > +/* > + * PVH Option ROM for fw_cfg DMA > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, see <http://www.gnu.org/licenses/>. > + * > + * Copyright (c) 2019 Red Hat Inc. > + * Authors: > + * Stefano Garzarella <sgarzare@redhat.com> > + */ > + > +asm (".code32"); /* this code will be executed in protected mode */ > + > +#include <stddef.h> > +#include <stdint.h> > +#include "optrom.h" > +#include "optrom_fw_cfg.h" > +#include "../../include/hw/xen/start_info.h" > + > +#define RSDP_SIGNATURE 0x2052545020445352LL /* "RSD PTR " */ > +#define RSDP_AREA_ADDR 0x000E0000 > +#define RSDP_AREA_SIZE 2048 > +#define EBDA_BASE_ADDR 0x0000040E > +#define EBDA_SIZE 1024 > + > +#define E820_MAXENTRIES 128 > +#define CMDLINE_BUFSIZE 4096 > + > +/* e820 table filled in pvh.S using int 0x15 */ > +struct pvh_e820_table { > + uint32_t entries; > + uint32_t reserved; > + struct hvm_memmap_table_entry table[E820_MAXENTRIES]; > +}; > + > +struct pvh_e820_table pvh_e820 asm("pvh_e820") __attribute__ ((aligned)); > + > +static struct hvm_start_info start_info; > +static uint8_t cmdline_buffer[CMDLINE_BUFSIZE]; > + > + > +/* Search RSDP signature. */ > +static uintptr_t search_rsdp(uint32_t start_addr, uint32_t end_addr) > +{ > + uint64_t *rsdp_p; > + > + /* RSDP signature is always on a 16 byte boundary */ > + for (rsdp_p = (uint64_t *)start_addr; rsdp_p < (uint64_t *)end_addr; > + rsdp_p += 2) { > + if (*rsdp_p == RSDP_SIGNATURE) { > + return (uintptr_t)rsdp_p; > + } > + } > + > + return 0; > +} gcc 10.2.1 "cc (Alpine 10.2.1_pre2) 10.2.1 20210313" reports: pc-bios/optionrom/pvh_main.c: In function 'search_rsdp': pc-bios/optionrom/pvh_main.c:61:21: warning: comparison is always false due to limited range of data type [-Wtype-limits] 61 | if (*rsdp_p == RSDP_SIGNATURE) { | ^~ Can you have a look? Thanks, Phil.
On Fri, Mar 19, 2021 at 03:06:25PM +0100, Philippe Mathieu-Daudé wrote: >Hi Stefano, > >On 2/5/19 7:14 PM, Paolo Bonzini wrote: >> From: Stefano Garzarella <sgarzare@redhat.com> >> >> The new pvh.bin option rom can be used with SeaBIOS to boot >> uncompressed kernel using the x86/HVM direct boot ABI. >> >> pvh.S contains the entry point of the option rom. It runs >> in real mode, loads the e820 table querying the BIOS, and >> then it switches to 32bit protected mode and jumps to the >> pvh_load_kernel() written in pvh_main.c. >> pvh_load_kernel() loads the cmdline and kernel entry_point >> using fw_cfg, then it looks for RSDP, fills the >> hvm_start_info required by x86/HVM ABI, and finally jumps >> to the kernel entry_point. >> >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> >> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> >> Reviewed-by: Liam Merwick <liam.merwick@oracle.com> >> --- >> .gitignore | 4 + >> Makefile | 2 +- >> pc-bios/optionrom/Makefile | 5 +- >> pc-bios/optionrom/pvh.S | 200 +++++++++++++++++++++++++++++++++++++++++++ >> pc-bios/optionrom/pvh_main.c | 116 +++++++++++++++++++++++++ >> pc-bios/pvh.bin | Bin 0 -> 1536 bytes >> 6 files changed, 325 insertions(+), 2 deletions(-) >> create mode 100644 pc-bios/optionrom/pvh.S >> create mode 100644 pc-bios/optionrom/pvh_main.c >> create mode 100644 pc-bios/pvh.bin > >> +++ b/pc-bios/optionrom/Makefile >> @@ -37,7 +37,7 @@ Wa = -Wa, >> ASFLAGS += -32 >> QEMU_CFLAGS += $(call cc-c-option, $(QEMU_CFLAGS), $(Wa)-32) >> >> -build-all: multiboot.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin >> +build-all: multiboot.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin pvh.bin >> >> +++ b/pc-bios/optionrom/pvh_main.c >> @@ -0,0 +1,116 @@ >> +/* >> + * PVH Option ROM for fw_cfg DMA >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program; if not, see <http://www.gnu.org/licenses/>. >> + * >> + * Copyright (c) 2019 Red Hat Inc. >> + * Authors: >> + * Stefano Garzarella <sgarzare@redhat.com> >> + */ >> + >> +asm (".code32"); /* this code will be executed in protected mode */ >> + >> +#include <stddef.h> >> +#include <stdint.h> >> +#include "optrom.h" >> +#include "optrom_fw_cfg.h" >> +#include "../../include/hw/xen/start_info.h" >> + >> +#define RSDP_SIGNATURE 0x2052545020445352LL /* "RSD PTR " */ >> +#define RSDP_AREA_ADDR 0x000E0000 >> +#define RSDP_AREA_SIZE 2048 >> +#define EBDA_BASE_ADDR 0x0000040E >> +#define EBDA_SIZE 1024 >> + >> +#define E820_MAXENTRIES 128 >> +#define CMDLINE_BUFSIZE 4096 >> + >> +/* e820 table filled in pvh.S using int 0x15 */ >> +struct pvh_e820_table { >> + uint32_t entries; >> + uint32_t reserved; >> + struct hvm_memmap_table_entry table[E820_MAXENTRIES]; >> +}; >> + >> +struct pvh_e820_table pvh_e820 asm("pvh_e820") __attribute__ ((aligned)); >> + >> +static struct hvm_start_info start_info; >> +static uint8_t cmdline_buffer[CMDLINE_BUFSIZE]; >> + >> + >> +/* Search RSDP signature. */ >> +static uintptr_t search_rsdp(uint32_t start_addr, uint32_t end_addr) >> +{ >> + uint64_t *rsdp_p; >> + >> + /* RSDP signature is always on a 16 byte boundary */ >> + for (rsdp_p = (uint64_t *)start_addr; rsdp_p < (uint64_t *)end_addr; >> + rsdp_p += 2) { >> + if (*rsdp_p == RSDP_SIGNATURE) { >> + return (uintptr_t)rsdp_p; >> + } >> + } >> + >> + return 0; >> +} > >gcc 10.2.1 "cc (Alpine 10.2.1_pre2) 10.2.1 20210313" reports: > >pc-bios/optionrom/pvh_main.c: In function 'search_rsdp': >pc-bios/optionrom/pvh_main.c:61:21: warning: comparison is always false >due to limited range of data type [-Wtype-limits] > 61 | if (*rsdp_p == RSDP_SIGNATURE) { > | ^~ > >Can you have a look? Sure, I'll take a look! It's strange that on my F33, gcc doesn't tell me anything also forcing -Wtype-limits: $ gcc --version gcc (GCC) 10.2.1 20201125 (Red Hat 10.2.1-9) Thanks, Stefano
On Fri, Mar 19, 2021 at 03:06:25PM +0100, Philippe Mathieu-Daudé wrote: >Hi Stefano, > >On 2/5/19 7:14 PM, Paolo Bonzini wrote: >> From: Stefano Garzarella <sgarzare@redhat.com> >> >> The new pvh.bin option rom can be used with SeaBIOS to boot >> uncompressed kernel using the x86/HVM direct boot ABI. >> >> pvh.S contains the entry point of the option rom. It runs >> in real mode, loads the e820 table querying the BIOS, and >> then it switches to 32bit protected mode and jumps to the >> pvh_load_kernel() written in pvh_main.c. >> pvh_load_kernel() loads the cmdline and kernel entry_point >> using fw_cfg, then it looks for RSDP, fills the >> hvm_start_info required by x86/HVM ABI, and finally jumps >> to the kernel entry_point. >> >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> >> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> >> Reviewed-by: Liam Merwick <liam.merwick@oracle.com> >> --- >> .gitignore | 4 + >> Makefile | 2 +- >> pc-bios/optionrom/Makefile | 5 +- >> pc-bios/optionrom/pvh.S | 200 +++++++++++++++++++++++++++++++++++++++++++ >> pc-bios/optionrom/pvh_main.c | 116 +++++++++++++++++++++++++ >> pc-bios/pvh.bin | Bin 0 -> 1536 bytes >> 6 files changed, 325 insertions(+), 2 deletions(-) >> create mode 100644 pc-bios/optionrom/pvh.S >> create mode 100644 pc-bios/optionrom/pvh_main.c >> create mode 100644 pc-bios/pvh.bin > >> +++ b/pc-bios/optionrom/Makefile >> @@ -37,7 +37,7 @@ Wa = -Wa, >> ASFLAGS += -32 >> QEMU_CFLAGS += $(call cc-c-option, $(QEMU_CFLAGS), $(Wa)-32) >> >> -build-all: multiboot.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin >> +build-all: multiboot.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin pvh.bin >> >> +++ b/pc-bios/optionrom/pvh_main.c >> @@ -0,0 +1,116 @@ >> +/* >> + * PVH Option ROM for fw_cfg DMA >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program; if not, see <http://www.gnu.org/licenses/>. >> + * >> + * Copyright (c) 2019 Red Hat Inc. >> + * Authors: >> + * Stefano Garzarella <sgarzare@redhat.com> >> + */ >> + >> +asm (".code32"); /* this code will be executed in protected mode */ >> + >> +#include <stddef.h> >> +#include <stdint.h> >> +#include "optrom.h" >> +#include "optrom_fw_cfg.h" >> +#include "../../include/hw/xen/start_info.h" >> + >> +#define RSDP_SIGNATURE 0x2052545020445352LL /* "RSD PTR " */ >> +#define RSDP_AREA_ADDR 0x000E0000 >> +#define RSDP_AREA_SIZE 2048 >> +#define EBDA_BASE_ADDR 0x0000040E >> +#define EBDA_SIZE 1024 >> + >> +#define E820_MAXENTRIES 128 >> +#define CMDLINE_BUFSIZE 4096 >> + >> +/* e820 table filled in pvh.S using int 0x15 */ >> +struct pvh_e820_table { >> + uint32_t entries; >> + uint32_t reserved; >> + struct hvm_memmap_table_entry table[E820_MAXENTRIES]; >> +}; >> + >> +struct pvh_e820_table pvh_e820 asm("pvh_e820") __attribute__ ((aligned)); >> + >> +static struct hvm_start_info start_info; >> +static uint8_t cmdline_buffer[CMDLINE_BUFSIZE]; >> + >> + >> +/* Search RSDP signature. */ >> +static uintptr_t search_rsdp(uint32_t start_addr, uint32_t end_addr) >> +{ >> + uint64_t *rsdp_p; >> + >> + /* RSDP signature is always on a 16 byte boundary */ >> + for (rsdp_p = (uint64_t *)start_addr; rsdp_p < (uint64_t *)end_addr; >> + rsdp_p += 2) { >> + if (*rsdp_p == RSDP_SIGNATURE) { >> + return (uintptr_t)rsdp_p; >> + } >> + } >> + >> + return 0; >> +} > >gcc 10.2.1 "cc (Alpine 10.2.1_pre2) 10.2.1 20210313" reports: > >pc-bios/optionrom/pvh_main.c: In function 'search_rsdp': >pc-bios/optionrom/pvh_main.c:61:21: warning: comparison is always false >due to limited range of data type [-Wtype-limits] > 61 | if (*rsdp_p == RSDP_SIGNATURE) { > | ^~ > >Can you have a look? I replicated with `make docker-test-build@alpine`, but I'm a bit confused. I tried to be more correct in this way: --- a/pc-bios/optionrom/pvh_main.c +++ b/pc-bios/optionrom/pvh_main.c @@ -27,7 +27,7 @@ asm (".code32"); /* this code will be executed in protected mode */ #include "optrom_fw_cfg.h" #include "../../include/hw/xen/start_info.h" -#define RSDP_SIGNATURE 0x2052545020445352LL /* "RSD PTR " */ +#define RSDP_SIGNATURE UINT64_C(0x2052545020445352) /* "RSD PTR " */ but I still had the warning, so the only way to mute the warning was with an explicit cast in the expression. I honestly don't understand, that value should be representable on 64 bits: @@ -58,7 +58,7 @@ static uintptr_t search_rsdp(uint32_t start_addr, uint32_t end_addr) /* RSDP signature is always on a 16 byte boundary */ for (rsdp_p = (uint64_t *)start_addr; rsdp_p < (uint64_t *)end_addr; rsdp_p += 2) { - if (*rsdp_p == RSDP_SIGNATURE) { + if (*rsdp_p == (uint64_t)RSDP_SIGNATURE) { return (uintptr_t)rsdp_p; } } Any thoughts? Thanks, Stefano
On 19/03/21 15:06, Philippe Mathieu-Daudé wrote: >> + >> +/* Search RSDP signature. */ >> +static uintptr_t search_rsdp(uint32_t start_addr, uint32_t end_addr) >> +{ >> + uint64_t *rsdp_p; >> + >> + /* RSDP signature is always on a 16 byte boundary */ >> + for (rsdp_p = (uint64_t *)start_addr; rsdp_p < (uint64_t *)end_addr; >> + rsdp_p += 2) { >> + if (*rsdp_p == RSDP_SIGNATURE) { >> + return (uintptr_t)rsdp_p; >> + } >> + } >> + >> + return 0; >> +} > gcc 10.2.1 "cc (Alpine 10.2.1_pre2) 10.2.1 20210313" reports: > > pc-bios/optionrom/pvh_main.c: In function 'search_rsdp': > pc-bios/optionrom/pvh_main.c:61:21: warning: comparison is always false > due to limited range of data type [-Wtype-limits] > 61 | if (*rsdp_p == RSDP_SIGNATURE) { > | ^~ This is probably a different bug, but I'll also add that uint64_t is supposed to be aligned to 64 bits, so you need either __attribute__((packed)), or use char* and memcmp. If you go for the latter, it would fix the issue that Philippe is reporting. Paolo
On Fri, Mar 19, 2021 at 06:03:59PM +0100, Paolo Bonzini wrote: >On 19/03/21 15:06, Philippe Mathieu-Daudé wrote: >>>+ >>>+/* Search RSDP signature. */ >>>+static uintptr_t search_rsdp(uint32_t start_addr, uint32_t end_addr) >>>+{ >>>+ uint64_t *rsdp_p; >>>+ >>>+ /* RSDP signature is always on a 16 byte boundary */ >>>+ for (rsdp_p = (uint64_t *)start_addr; rsdp_p < (uint64_t *)end_addr; >>>+ rsdp_p += 2) { >>>+ if (*rsdp_p == RSDP_SIGNATURE) { >>>+ return (uintptr_t)rsdp_p; >>>+ } >>>+ } >>>+ >>>+ return 0; >>>+} >>gcc 10.2.1 "cc (Alpine 10.2.1_pre2) 10.2.1 20210313" reports: >> >>pc-bios/optionrom/pvh_main.c: In function 'search_rsdp': >>pc-bios/optionrom/pvh_main.c:61:21: warning: comparison is always false >>due to limited range of data type [-Wtype-limits] >> 61 | if (*rsdp_p == RSDP_SIGNATURE) { >> | ^~ > >This is probably a different bug, but I'll also add that uint64_t is >supposed to be aligned to 64 bits, so you need either >__attribute__((packed)), or use char* and memcmp. If you go for the >latter, it would fix the issue that Philippe is reporting. Yes, memcmp maybe is also more readable, but being baremetal, I have to implement it right? Thanks, Stefano
It's likely that the compiler will online it. But indeed it's better to add -minline-all-stringops to the compiler command line. Paolo Il ven 19 mar 2021, 18:35 Stefano Garzarella <sgarzare@redhat.com> ha scritto: > On Fri, Mar 19, 2021 at 06:03:59PM +0100, Paolo Bonzini wrote: > >On 19/03/21 15:06, Philippe Mathieu-Daudé wrote: > >>>+ > >>>+/* Search RSDP signature. */ > >>>+static uintptr_t search_rsdp(uint32_t start_addr, uint32_t end_addr) > >>>+{ > >>>+ uint64_t *rsdp_p; > >>>+ > >>>+ /* RSDP signature is always on a 16 byte boundary */ > >>>+ for (rsdp_p = (uint64_t *)start_addr; rsdp_p < (uint64_t > *)end_addr; > >>>+ rsdp_p += 2) { > >>>+ if (*rsdp_p == RSDP_SIGNATURE) { > >>>+ return (uintptr_t)rsdp_p; > >>>+ } > >>>+ } > >>>+ > >>>+ return 0; > >>>+} > >>gcc 10.2.1 "cc (Alpine 10.2.1_pre2) 10.2.1 20210313" reports: > >> > >>pc-bios/optionrom/pvh_main.c: In function 'search_rsdp': > >>pc-bios/optionrom/pvh_main.c:61:21: warning: comparison is always false > >>due to limited range of data type [-Wtype-limits] > >> 61 | if (*rsdp_p == RSDP_SIGNATURE) { > >> | ^~ > > > >This is probably a different bug, but I'll also add that uint64_t is > >supposed to be aligned to 64 bits, so you need either > >__attribute__((packed)), or use char* and memcmp. If you go for the > >latter, it would fix the issue that Philippe is reporting. > > Yes, memcmp maybe is also more readable, but being baremetal, I have to > implement it right? > > Thanks, > Stefano > >
On Fri, Mar 19, 2021 at 06:52:39PM +0100, Paolo Bonzini wrote: >It's likely that the compiler will online it. But indeed it's better to add >-minline-all-stringops to the compiler command line. > Cool, I didn't know that one! I tried but I did something wrong because the linker is not happy, next week I'll check better: ld: pvh_main.o: in function `search_rsdp': /home/stefano/repos/qemu/pc-bios/optionrom/pvh_main.c:62: undefined reference to `memcmp' ld: /home/stefano/repos/qemu/pc-bios/optionrom/pvh_main.c:62: undefined reference to `memcmp' In the mean time, I simply tried to assign the RSDP_SIGNATURE macro to an uint64_t variable and I have this new warning, using gcc 10.2.1 "cc (Alpine 10.2.1_pre2) 10.2.1 20210313": In file included from /tmp/qemu-test/src/pc-bios/optionrom/pvh_main.c:25: /tmp/qemu-test/src/pc-bios/optionrom/pvh_main.c: In function 'search_rsdp': /tmp/qemu-test/src/pc-bios/optionrom/pvh_main.c:30:42: warning: conversion from 'long long unsigned int' to 'uint64_t' {aka 'long unsigned int'} changes value from '2329016660419433298' to '541348690' [-Woverflow] 30 | #define RSDP_SIGNATURE UINT64_C(0x2052545020445352) /* "RSD PTR " */ | ^~~~~~~~~~~~~~~~~~ /tmp/qemu-test/src/pc-bios/optionrom/pvh_main.c:69:31: note: in expansion of macro 'RSDP_SIGNATURE' 69 | uint64_t rsdp_signature = RSDP_SIGNATURE; | Using gcc (GCC) 10.2.1 20201125 (Red Hat 10.2.1-9) I don't have it. It seems a bit strange, I don't know if it's related to the fact that we compile with -m16, I'll check better. Thanks, Stefano >Paolo > >Il ven 19 mar 2021, 18:35 Stefano Garzarella <sgarzare@redhat.com> ha >scritto: > >> On Fri, Mar 19, 2021 at 06:03:59PM +0100, Paolo Bonzini wrote: >> >On 19/03/21 15:06, Philippe Mathieu-Daudé wrote: >> >>>+ >> >>>+/* Search RSDP signature. */ >> >>>+static uintptr_t search_rsdp(uint32_t start_addr, uint32_t end_addr) >> >>>+{ >> >>>+ uint64_t *rsdp_p; >> >>>+ >> >>>+ /* RSDP signature is always on a 16 byte boundary */ >> >>>+ for (rsdp_p = (uint64_t *)start_addr; rsdp_p < (uint64_t >> *)end_addr; >> >>>+ rsdp_p += 2) { >> >>>+ if (*rsdp_p == RSDP_SIGNATURE) { >> >>>+ return (uintptr_t)rsdp_p; >> >>>+ } >> >>>+ } >> >>>+ >> >>>+ return 0; >> >>>+} >> >>gcc 10.2.1 "cc (Alpine 10.2.1_pre2) 10.2.1 20210313" reports: >> >> >> >>pc-bios/optionrom/pvh_main.c: In function 'search_rsdp': >> >>pc-bios/optionrom/pvh_main.c:61:21: warning: comparison is always false >> >>due to limited range of data type [-Wtype-limits] >> >> 61 | if (*rsdp_p == RSDP_SIGNATURE) { >> >> | ^~ >> > >> >This is probably a different bug, but I'll also add that uint64_t is >> >supposed to be aligned to 64 bits, so you need either >> >__attribute__((packed)), or use char* and memcmp. If you go for the >> >latter, it would fix the issue that Philippe is reporting. >> >> Yes, memcmp maybe is also more readable, but being baremetal, I have to >> implement it right? >> >> Thanks, >> Stefano >> >>
On Fri, Mar 19, 2021 at 7:20 PM Stefano Garzarella <sgarzare@redhat.com> wrote: > > On Fri, Mar 19, 2021 at 06:52:39PM +0100, Paolo Bonzini wrote: > >It's likely that the compiler will online it. But indeed it's better to add > >-minline-all-stringops to the compiler command line. > > > > Cool, I didn't know that one! > I tried but I did something wrong because the linker is not happy, next > week I'll check better: > ld: pvh_main.o: in function `search_rsdp': > /home/stefano/repos/qemu/pc-bios/optionrom/pvh_main.c:62: undefined reference to `memcmp' > ld: /home/stefano/repos/qemu/pc-bios/optionrom/pvh_main.c:62: undefined reference to `memcmp' > > > In the mean time, I simply tried to assign the RSDP_SIGNATURE macro to > an uint64_t variable and I have this new warning, using gcc 10.2.1 "cc > (Alpine 10.2.1_pre2) 10.2.1 20210313": > > In file included from /tmp/qemu-test/src/pc-bios/optionrom/pvh_main.c:25: > /tmp/qemu-test/src/pc-bios/optionrom/pvh_main.c: In function 'search_rsdp': > /tmp/qemu-test/src/pc-bios/optionrom/pvh_main.c:30:42: warning: conversion from 'long long unsigned int' to 'uint64_t' {aka 'long unsigned int'} changes value from '2329016660419433298' to '541348690' [-Woverflow] > 30 | #define RSDP_SIGNATURE UINT64_C(0x2052545020445352) /* "RSD PTR " */ > | ^~~~~~~~~~~~~~~~~~ > /tmp/qemu-test/src/pc-bios/optionrom/pvh_main.c:69:31: note: in expansion of macro 'RSDP_SIGNATURE' > 69 | uint64_t rsdp_signature = RSDP_SIGNATURE; > | > > Using gcc (GCC) 10.2.1 20201125 (Red Hat 10.2.1-9) I don't have it. > > It seems a bit strange, I don't know if it's related to the fact that we > compile with -m16, I'll check better. Anyway I think that using memcmp() I can switch to a character array to store the signature, but this gcc warnings confused me a bit... Stefano
On Fri, Mar 19, 2021 at 7:25 PM Stefano Garzarella <sgarzare@redhat.com> wrote: > > On Fri, Mar 19, 2021 at 7:20 PM Stefano Garzarella <sgarzare@redhat.com> wrote: > > > > On Fri, Mar 19, 2021 at 06:52:39PM +0100, Paolo Bonzini wrote: > > >It's likely that the compiler will online it. But indeed it's better to add > > >-minline-all-stringops to the compiler command line. > > > > > > > Cool, I didn't know that one! > > I tried but I did something wrong because the linker is not happy, next > > week I'll check better: > > ld: pvh_main.o: in function `search_rsdp': > > /home/stefano/repos/qemu/pc-bios/optionrom/pvh_main.c:62: undefined reference to `memcmp' > > ld: /home/stefano/repos/qemu/pc-bios/optionrom/pvh_main.c:62: undefined reference to `memcmp' > > > > > > In the mean time, I simply tried to assign the RSDP_SIGNATURE macro to > > an uint64_t variable and I have this new warning, using gcc 10.2.1 "cc > > (Alpine 10.2.1_pre2) 10.2.1 20210313": > > > > In file included from /tmp/qemu-test/src/pc-bios/optionrom/pvh_main.c:25: > > /tmp/qemu-test/src/pc-bios/optionrom/pvh_main.c: In function 'search_rsdp': > > /tmp/qemu-test/src/pc-bios/optionrom/pvh_main.c:30:42: warning: conversion from 'long long unsigned int' to 'uint64_t' {aka 'long unsigned int'} changes value from '2329016660419433298' to '541348690' [-Woverflow] > > 30 | #define RSDP_SIGNATURE UINT64_C(0x2052545020445352) /* "RSD PTR " */ > > | ^~~~~~~~~~~~~~~~~~ > > /tmp/qemu-test/src/pc-bios/optionrom/pvh_main.c:69:31: note: in expansion of macro 'RSDP_SIGNATURE' > > 69 | uint64_t rsdp_signature = RSDP_SIGNATURE; > > | > > > > Using gcc (GCC) 10.2.1 20201125 (Red Hat 10.2.1-9) I don't have it. > > > > It seems a bit strange, I don't know if it's related to the fact that we > > compile with -m16, I'll check better. > > Anyway I think that using memcmp() I can switch to a character array to > store the signature, but this gcc warnings confused me a bit... > I'll send a patch to add a simple implementation of memcmp and use it since I wasn't able to embedded it with -minline-all-stringops. In the mean time, I played a bit with sizeof() to understand what's going on and I think there is something strange with Alpine gcc 10.2.1_pre2. I added this 3 lines in pvh_main.c to print the sizes at compile time (maybe there is a better way :-): + char (*__size1)[sizeof(uint64_t)] = 1; + char (*__size2)[sizeof(UINT64_C(1))] = 1; + char (*__size3)[sizeof(UINT64_C(0x2052545020445352))] = 1; If I build with gcc 10.2.1 20201125 (Red Hat 10.2.1-9) everything looks normal, since they are all 8 bytes: warning: initialization of ‘char (*)[8]’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion] 74 | char (*__size1)[sizeof(uint64_t)] = 1; | ^ warning: initialization of ‘char (*)[8]’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion] 75 | char (*__size2)[sizeof(UINT64_C(1))] = 1; | ^ warning: initialization of ‘char (*)[8]’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion] 76 | char (*__size3)[sizeof(UINT64_C(0x2052545020445352))] = 1; If I build with gcc 10.2.1 20210313 (Alpine 10.2.1_pre2) uint64_t and UINT64_C(1) have a size of 4 bytes, while UINT64_C(0x2052545020445352) has a size of 8 bytes: warning: initialization of ‘char (*)[4]’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion] 74 | char (*__size1)[sizeof(uint64_t)] = 1; | ^ warning: initialization of ‘char (*)[4]’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion] 75 | char (*__size2)[sizeof(UINT64_C(1))] = 1; | ^ warning: initialization of ‘char (*)[8]’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion] 76 | char (*__size3)[sizeof(UINT64_C(0x2052545020445352))] = 1; This is a bit strange... Thanks, Stefano
On 22/03/21 11:59, Stefano Garzarella wrote: > > If I build with gcc 10.2.1 20210313 (Alpine 10.2.1_pre2) uint64_t and > UINT64_C(1) have a size of 4 bytes, while UINT64_C(0x2052545020445352) > has a size of 8 bytes: > > warning: initialization of ‘char (*)[4]’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion] > 74 | char (*__size1)[sizeof(uint64_t)] = 1; > | ^ > warning: initialization of ‘char (*)[4]’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion] > 75 | char (*__size2)[sizeof(UINT64_C(1))] = 1; > | ^ > warning: initialization of ‘char (*)[8]’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion] > 76 | char (*__size3)[sizeof(UINT64_C(0x2052545020445352))] = 1; Looks like long is 4 bytes long with -m16 and -m32, but 8 bytes with -m64. The large constant is extended to long long because it's the only way to fit it. Paolo
On Mon, Mar 22, 2021 at 12:52:37PM +0100, Paolo Bonzini wrote: >On 22/03/21 11:59, Stefano Garzarella wrote: >> >>If I build with gcc 10.2.1 20210313 (Alpine 10.2.1_pre2) uint64_t and >>UINT64_C(1) have a size of 4 bytes, while UINT64_C(0x2052545020445352) >>has a size of 8 bytes: >> >> warning: initialization of ‘char (*)[4]’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion] >> 74 | char (*__size1)[sizeof(uint64_t)] = 1; >> | ^ >> warning: initialization of ‘char (*)[4]’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion] >> 75 | char (*__size2)[sizeof(UINT64_C(1))] = 1; >> | ^ >> warning: initialization of ‘char (*)[8]’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion] >> 76 | char (*__size3)[sizeof(UINT64_C(0x2052545020445352))] = 1; > >Looks like long is 4 bytes long with -m16 and -m32, but 8 bytes with >-m64. The large constant is extended to long long because it's the >only way to fit it. Yeah, but I expected uint64_t to always be on 8 bytes, regardless of the architecture. Stefano
On 22/03/21 14:57, Stefano Garzarella wrote: > On Mon, Mar 22, 2021 at 12:52:37PM +0100, Paolo Bonzini wrote: >> On 22/03/21 11:59, Stefano Garzarella wrote: >>> >>> If I build with gcc 10.2.1 20210313 (Alpine 10.2.1_pre2) uint64_t and >>> UINT64_C(1) have a size of 4 bytes, while UINT64_C(0x2052545020445352) >>> has a size of 8 bytes: >>> >>> warning: initialization of ‘char (*)[4]’ from ‘int’ makes pointer >>> from integer without a cast [-Wint-conversion] >>> 74 | char (*__size1)[sizeof(uint64_t)] = 1; >>> | ^ >>> warning: initialization of ‘char (*)[4]’ from ‘int’ makes pointer >>> from integer without a cast [-Wint-conversion] >>> 75 | char (*__size2)[sizeof(UINT64_C(1))] = 1; >>> | ^ >>> warning: initialization of ‘char (*)[8]’ from ‘int’ makes pointer >>> from integer without a cast [-Wint-conversion] >>> 76 | char (*__size3)[sizeof(UINT64_C(0x2052545020445352))] = 1; >> >> Looks like long is 4 bytes long with -m16 and -m32, but 8 bytes with >> -m64. The large constant is extended to long long because it's the >> only way to fit it. > > Yeah, but I expected uint64_t to always be on 8 bytes, regardless of the > architecture. It's somehow using the -m64 definition (long int) instead of the -m32 definition (long long int), even though -m16 is basically "same as -m32 but emitting 16-bit encoded instructions". Paolo
diff --git a/.gitignore b/.gitignore index 0430257..321095b 100644 --- a/.gitignore +++ b/.gitignore @@ -103,6 +103,10 @@ /pc-bios/optionrom/linuxboot_dma.bin /pc-bios/optionrom/linuxboot_dma.raw /pc-bios/optionrom/linuxboot_dma.img +/pc-bios/optionrom/pvh.asm +/pc-bios/optionrom/pvh.bin +/pc-bios/optionrom/pvh.raw +/pc-bios/optionrom/pvh.img /pc-bios/optionrom/multiboot.asm /pc-bios/optionrom/multiboot.bin /pc-bios/optionrom/multiboot.raw diff --git a/Makefile b/Makefile index 1278a3e..76f6ab4 100644 --- a/Makefile +++ b/Makefile @@ -673,7 +673,7 @@ efi-e1000.rom efi-eepro100.rom efi-ne2k_pci.rom \ efi-pcnet.rom efi-rtl8139.rom efi-virtio.rom \ efi-e1000e.rom efi-vmxnet3.rom \ bamboo.dtb canyonlands.dtb petalogix-s3adsp1800.dtb petalogix-ml605.dtb \ -multiboot.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin \ +multiboot.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin pvh.bin \ s390-ccw.img s390-netboot.img \ spapr-rtas.bin slof.bin skiboot.lid \ palcode-clipper \ diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile index a9a9e5e..e33a24d 100644 --- a/pc-bios/optionrom/Makefile +++ b/pc-bios/optionrom/Makefile @@ -37,7 +37,7 @@ Wa = -Wa, ASFLAGS += -32 QEMU_CFLAGS += $(call cc-c-option, $(QEMU_CFLAGS), $(Wa)-32) -build-all: multiboot.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin +build-all: multiboot.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin pvh.bin # suppress auto-removal of intermediate files .SECONDARY: @@ -46,6 +46,9 @@ build-all: multiboot.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin %.o: %.S $(call quiet-command,$(CPP) $(QEMU_INCLUDES) $(QEMU_DGFLAGS) -c -o - $< | $(AS) $(ASFLAGS) -o $@,"AS","$(TARGET_DIR)$@") +pvh.img: pvh.o pvh_main.o + $(call quiet-command,$(LD) $(LDFLAGS_NOPIE) -m $(LD_I386_EMULATION) -T $(SRC_PATH)/pc-bios/optionrom/flat.lds -s -o $@ $^,"BUILD","$(TARGET_DIR)$@") + %.img: %.o $(call quiet-command,$(LD) $(LDFLAGS_NOPIE) -m $(LD_I386_EMULATION) -T $(SRC_PATH)/pc-bios/optionrom/flat.lds -s -o $@ $<,"BUILD","$(TARGET_DIR)$@") diff --git a/pc-bios/optionrom/pvh.S b/pc-bios/optionrom/pvh.S new file mode 100644 index 0000000..e1d7f4a --- /dev/null +++ b/pc-bios/optionrom/pvh.S @@ -0,0 +1,200 @@ +/* + * PVH Option ROM + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, see <http://www.gnu.org/licenses/>. + * + * Copyright Novell Inc, 2009 + * Authors: Alexander Graf <agraf@suse.de> + * + * Copyright (c) 2019 Red Hat Inc. + * Authors: Stefano Garzarella <sgarzare@redhat.com> + */ + +#include "optionrom.h" + +#define BOOT_ROM_PRODUCT "PVH loader" + +#define GS_PROT_JUMP 0 +#define GS_GDT_DESC 6 + +#ifdef OPTION_ROM_START +#undef OPTION_ROM_START +#endif +#ifdef OPTION_ROM_END +#undef OPTION_ROM_END +#endif + +/* + * Redefine OPTION_ROM_START and OPTION_ROM_END, because this rom is produced + * linking multiple objects. + * signrom.py will add padding. + */ +#define OPTION_ROM_START \ + .code16; \ + .text; \ + .global _start; \ + _start:; \ + .short 0xaa55; \ + .byte 3; /* desired size in 512 units */ + +#define OPTION_ROM_END \ + _end: + +BOOT_ROM_START + +run_pvhboot: + + cli + cld + + mov %cs, %eax + shl $0x4, %eax + + /* set up a long jump descriptor that is PC relative */ + + /* move stack memory to %gs */ + mov %ss, %ecx + shl $0x4, %ecx + mov %esp, %ebx + add %ebx, %ecx + sub $0x20, %ecx + sub $0x30, %esp + shr $0x4, %ecx + mov %cx, %gs + + /* now push the indirect jump descriptor there */ + mov (prot_jump), %ebx + add %eax, %ebx + movl %ebx, %gs:GS_PROT_JUMP + mov $8, %bx + movw %bx, %gs:GS_PROT_JUMP + 4 + + /* fix the gdt descriptor to be PC relative */ + movw (gdt_desc), %bx + movw %bx, %gs:GS_GDT_DESC + movl (gdt_desc+2), %ebx + add %eax, %ebx + movl %ebx, %gs:GS_GDT_DESC + 2 + + /* initialize HVM memmap table using int 0x15(e820) */ + + /* ES = pvh_e820 struct */ + mov $pvh_e820, %eax + shr $4, %eax + mov %ax, %es + + /* start storing memmap table at %es:8 (pvh_e820.table) */ + mov $8,%edi + xor %ebx, %ebx + jmp memmap_loop + +memmap_loop_check: + /* pvh_e820 can contains up to 128 entries */ + cmp $128, %ebx + je memmap_done + +memmap_loop: + /* entry size (hvm_memmap_table_entry) & max buffer size (int15) */ + movl $24, %ecx + /* e820 */ + movl $0x0000e820, %eax + /* 'SMAP' magic */ + movl $0x534d4150, %edx + /* store counter value at %es:0 (pvh_e820.entries) */ + movl %ebx, %es:0 + + int $0x15 + /* error or last entry already done? */ + jb memmap_err + + /* %edi += entry size (hvm_memmap_table_entry) */ + add $24, %edi + + /* continuation value 0 means last entry */ + test %ebx, %ebx + jnz memmap_loop_check + + /* increase pvh_e820.entries to save the last entry */ + movl %es:0, %ebx + inc %ebx + +memmap_done: + movl %ebx, %es:0 + +memmap_err: + + /* load the GDT before going into protected mode */ +lgdt: + data32 lgdt %gs:GS_GDT_DESC + + /* get us to protected mode now */ + movl $1, %eax + movl %eax, %cr0 + + /* the LJMP sets CS for us and gets us to 32-bit */ +ljmp: + data32 ljmp *%gs:GS_PROT_JUMP + +prot_mode: +.code32 + + /* initialize all other segments */ + movl $0x10, %eax + movl %eax, %ss + movl %eax, %ds + movl %eax, %es + movl %eax, %fs + movl %eax, %gs + + jmp pvh_load_kernel + +/* Variables */ +.align 4, 0 +prot_jump: .long prot_mode + .short 8 + +.align 4, 0 +gdt: + /* 0x00 */ +.byte 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 + + /* + * 0x08: code segment + * (base=0, limit=0xfffff, type=32bit code exec/read, DPL=0, 4k) + */ +.byte 0xff, 0xff, 0x00, 0x00, 0x00, 0x9a, 0xcf, 0x00 + + /* + * 0x10: data segment + * (base=0, limit=0xfffff, type=32bit data read/write, DPL=0, 4k) + */ +.byte 0xff, 0xff, 0x00, 0x00, 0x00, 0x92, 0xcf, 0x00 + + /* + * 0x18: code segment + * (base=0, limit=0x0ffff, type=16bit code exec/read/conf, DPL=0, 1b) + */ +.byte 0xff, 0xff, 0x00, 0x00, 0x00, 0x9e, 0x00, 0x00 + + /* + * 0x20: data segment + * (base=0, limit=0x0ffff, type=16bit data read/write, DPL=0, 1b) + */ +.byte 0xff, 0xff, 0x00, 0x00, 0x00, 0x92, 0x00, 0x00 + +gdt_desc: +.short (5 * 8) - 1 +.long gdt + +BOOT_ROM_END diff --git a/pc-bios/optionrom/pvh_main.c b/pc-bios/optionrom/pvh_main.c new file mode 100644 index 0000000..1dcc5c9 --- /dev/null +++ b/pc-bios/optionrom/pvh_main.c @@ -0,0 +1,116 @@ +/* + * PVH Option ROM for fw_cfg DMA + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, see <http://www.gnu.org/licenses/>. + * + * Copyright (c) 2019 Red Hat Inc. + * Authors: + * Stefano Garzarella <sgarzare@redhat.com> + */ + +asm (".code32"); /* this code will be executed in protected mode */ + +#include <stddef.h> +#include <stdint.h> +#include "optrom.h" +#include "optrom_fw_cfg.h" +#include "../../include/hw/xen/start_info.h" + +#define RSDP_SIGNATURE 0x2052545020445352LL /* "RSD PTR " */ +#define RSDP_AREA_ADDR 0x000E0000 +#define RSDP_AREA_SIZE 2048 +#define EBDA_BASE_ADDR 0x0000040E +#define EBDA_SIZE 1024 + +#define E820_MAXENTRIES 128 +#define CMDLINE_BUFSIZE 4096 + +/* e820 table filled in pvh.S using int 0x15 */ +struct pvh_e820_table { + uint32_t entries; + uint32_t reserved; + struct hvm_memmap_table_entry table[E820_MAXENTRIES]; +}; + +struct pvh_e820_table pvh_e820 asm("pvh_e820") __attribute__ ((aligned)); + +static struct hvm_start_info start_info; +static uint8_t cmdline_buffer[CMDLINE_BUFSIZE]; + + +/* Search RSDP signature. */ +static uintptr_t search_rsdp(uint32_t start_addr, uint32_t end_addr) +{ + uint64_t *rsdp_p; + + /* RSDP signature is always on a 16 byte boundary */ + for (rsdp_p = (uint64_t *)start_addr; rsdp_p < (uint64_t *)end_addr; + rsdp_p += 2) { + if (*rsdp_p == RSDP_SIGNATURE) { + return (uintptr_t)rsdp_p; + } + } + + return 0; +} + +/* Force the asm name without leading underscore, even on Win32. */ +extern void pvh_load_kernel(void) asm("pvh_load_kernel"); + +void pvh_load_kernel(void) +{ + void *cmdline_addr = &cmdline_buffer; + void *kernel_entry; + uint32_t cmdline_size, fw_cfg_version = bios_cfg_version(); + + start_info.magic = XEN_HVM_START_MAGIC_VALUE; + start_info.version = 1; + + /* + * pvh_e820 is filled in the pvh.S before to switch in protected mode, + * because we can use int 0x15 only in real mode. + */ + start_info.memmap_entries = pvh_e820.entries; + start_info.memmap_paddr = (uintptr_t)pvh_e820.table; + + /* + * Search RSDP in the main BIOS area below 1 MB. + * SeaBIOS store the RSDP in this area, so we try it first. + */ + start_info.rsdp_paddr = search_rsdp(RSDP_AREA_ADDR, + RSDP_AREA_ADDR + RSDP_AREA_SIZE); + + /* Search RSDP in the EBDA if it is not found */ + if (!start_info.rsdp_paddr) { + /* + * Th EBDA address is stored at EBDA_BASE_ADDR. It contains 2 bytes + * segment pointer to EBDA, so we must convert it to a linear address. + */ + uint32_t ebda_paddr = ((uint32_t)*((uint16_t *)EBDA_BASE_ADDR)) << 4; + if (ebda_paddr > 0x400) { + uint32_t *ebda = (uint32_t *)ebda_paddr; + + start_info.rsdp_paddr = search_rsdp(*ebda, *ebda + EBDA_SIZE); + } + } + + bios_cfg_read_entry(&cmdline_size, FW_CFG_CMDLINE_SIZE, 4, fw_cfg_version); + bios_cfg_read_entry(cmdline_addr, FW_CFG_CMDLINE_DATA, cmdline_size, + fw_cfg_version); + start_info.cmdline_paddr = (uintptr_t)cmdline_addr; + + bios_cfg_read_entry(&kernel_entry, FW_CFG_KERNEL_ENTRY, 4, fw_cfg_version); + + asm volatile("jmp *%1" : : "b"(&start_info), "c"(kernel_entry)); +}