arch/x86/boot/compressed/head_64.S | 2 +- arch/x86/kernel/head_64.S | 4 ++-- drivers/firmware/efi/libstub/x86-stub.c | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-)
From: Khalid Ali <khaliidcaliy@gmail.com> The current kernel entry point takes one argument which is boot_param from RSI. The only argument entry point recieves is pointer to boot_param. In order to comply with the ABI calling convension the entry point must recieve the boot_param from RDI instead of RSI. There were no specific use case used for RDI, so the kernel can safely recieve argument from that register to better comply with ABI. This patch makes the kernel to recieve boot_param which is the only argument it recieves, from RDI instead of RSI. All changes needed for stability and clarity have being changed. Changelog: * Kernel uncompressed entry point expects boot_param from RDI instead of RSI. * The decompressor has being adjusted to supply argument from RDI instead RSI. * libstub has being adjusted to supply argument from RDI instead of RSI. After throughly tested there were no regression and UDs has being observed. Looking forward for feedback. arch/x86/boot/compressed/head_64.S | 2 +- arch/x86/kernel/head_64.S | 4 ++-- drivers/firmware/efi/libstub/x86-stub.c | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) -- 2.49.0
On Mon, Jun 23, 2025 at 4:16 AM Khalid Ali <khaliidcaliy@gmail.com> wrote: > > From: Khalid Ali <khaliidcaliy@gmail.com> > > The current kernel entry point takes one argument which is boot_param > from RSI. The only argument entry point recieves is pointer to > boot_param. > > In order to comply with the ABI calling convension the entry point must > recieve the boot_param from RDI instead of RSI. There were no specific > use case used for RDI, so the kernel can safely recieve argument from > that register to better comply with ABI. > > This patch makes the kernel to recieve boot_param which is the only > argument it recieves, from RDI instead of RSI. All changes needed for > stability and clarity have being changed. > > Changelog: > * Kernel uncompressed entry point expects boot_param from RDI instead > of RSI. > * The decompressor has being adjusted to supply argument from RDI > instead RSI. > * libstub has being adjusted to supply argument from RDI instead of RSI. > > After throughly tested there were no regression and UDs has being > observed. Looking forward for feedback. > > arch/x86/boot/compressed/head_64.S | 2 +- > arch/x86/kernel/head_64.S | 4 ++-- > drivers/firmware/efi/libstub/x86-stub.c | 4 ++-- > 3 files changed, 5 insertions(+), 5 deletions(-) This was never intended to conform to the C ABI, why is it necessary to change it? Also, you cannot break this up into three patches. Every patch must be fully functional so that git bisect will work. Brian Gerst
> This was never intended to conform to the C ABI, why is it necessary > to change it? Technically speaking, you are right, however that doesn't mean we can put something where ever we like. We came from C code which is bootloader and we end up to C code, so we should comply the ABI here too. > Also, you cannot break this up into three patches. Every patch must > be fully functional so that git bisect will work. Thanks for the tip. I will do next time.
On 2025-06-23 09:29, Khalid Ali wrote: >> This was never intended to conform to the C ABI, why is it necessary >> to change it? > > Technically speaking, you are right, however that doesn't mean we can put something where > ever we like. We came from C code which is bootloader and we end up to C code, so we should > comply the ABI here too. > This is also invoked by some external bootloaders that boot the ELF image directly, even though this is strongly discouraged. Therefore this patchset is NAKed with extreme prejudice. -hpa
> This is also invoked by some external bootloaders that boot the ELF > image directly, even though this is strongly discouraged. > > Therefore this patchset is NAKed with extreme prejudice. Thanks both of you peter and brian, however, the boot protocol document saying "%rsi must hold the base address of the struct boot_params", it doesn't mention why. Maybe the document needs update to justify the reasons. I wouldn't have known it if you didn't tell me, so this shouldn't confuse anyone else. Thanks Khalid Ali
On June 23, 2025 11:39:09 AM PDT, Khalid Ali <khaliidcaliy@gmail.com> wrote: >> This is also invoked by some external bootloaders that boot the ELF >> image directly, even though this is strongly discouraged. >> >> Therefore this patchset is NAKed with extreme prejudice. > >Thanks both of you peter and brian, > >however, the boot protocol document saying "%rsi must hold the base address of the struct boot_params", >it doesn't mention why. Maybe the document needs update to justify the reasons. I wouldn't have known it >if you didn't tell me, so this shouldn't confuse anyone else. > >Thanks >Khalid Ali It is a *protocol*. An interface. "Because the interface specification says so" is really all the justification you need; otherwise you have to hunt down *every* user of the interface and verify your change.
On Mon, Jun 23, 2025 at 2:40 PM Khalid Ali <khaliidcaliy@gmail.com> wrote: > > > This is also invoked by some external bootloaders that boot the ELF > > image directly, even though this is strongly discouraged. > > > > Therefore this patchset is NAKed with extreme prejudice. > > Thanks both of you peter and brian, > > however, the boot protocol document saying "%rsi must hold the base address of the struct boot_params", > it doesn't mention why. Maybe the document needs update to justify the reasons. I wouldn't have known it > if you didn't tell me, so this shouldn't confuse anyone else. The use of RSI was inherited from the 32-bit kernel, but the real reason is lost to history. It's just always been that way and there is no compelling reason to change it. Brian Gerst
On June 23, 2025 12:24:50 PM PDT, Brian Gerst <brgerst@gmail.com> wrote: >On Mon, Jun 23, 2025 at 2:40 PM Khalid Ali <khaliidcaliy@gmail.com> wrote: >> >> > This is also invoked by some external bootloaders that boot the ELF >> > image directly, even though this is strongly discouraged. >> > >> > Therefore this patchset is NAKed with extreme prejudice. >> >> Thanks both of you peter and brian, >> >> however, the boot protocol document saying "%rsi must hold the base address of the struct boot_params", >> it doesn't mention why. Maybe the document needs update to justify the reasons. I wouldn't have known it >> if you didn't tell me, so this shouldn't confuse anyone else. > >The use of RSI was inherited from the 32-bit kernel, but the real >reason is lost to history. It's just always been that way and there >is no compelling reason to change it. > > >Brian Gerst > The reason isn't lost to history: I picked %esi because I found that none of the weird bootloaders which looked the protected mode jump clobbered that particular register.
© 2016 - 2025 Red Hat, Inc.