Re: [PATCH v2 0/5] loongarch: add relaxation support
On 6/13/23 11:25, mengqinggang wrote: Is this patch used to check if ld supports --no-relax option? It need to pass -mno-relax option to as or gcc to disable binutils generate relaxation relocations. It shouldn't be necessary to disable relaxation that early, because code before relaxation should behave the same, only bigger. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] kern/acpi: use xsdt_addr if present
According to the ACPI specification, in ACPI 2.0 or later, an ACPI-compatible OS must use the XSDT if present. So, we should use xsdt_addr instead of rsdt_addr if xsdt_addr is valid. Signed-off-by: Qiumiao Zhang --- grub-core/kern/acpi.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/grub-core/kern/acpi.c b/grub-core/kern/acpi.c index a9bcd85fc..c61115dcd 100644 --- a/grub-core/kern/acpi.c +++ b/grub-core/kern/acpi.c @@ -101,12 +101,6 @@ grub_acpi_find_table (const char *sig) if (r) return r; rsdpv2 = grub_machine_acpi_get_rsdpv2 (); - if (rsdpv2) -r = grub_acpi_rsdt_find_table ((struct grub_acpi_table_header *) - (grub_addr_t) rsdpv2->rsdpv1.rsdt_addr, - sig); - if (r) -return r; if (rsdpv2 #if GRUB_CPU_SIZEOF_VOID_P != 8 && !(rsdpv2->xsdt_addr >> 32) @@ -117,6 +111,12 @@ grub_acpi_find_table (const char *sig) sig); if (r) return r; + if (rsdpv2) +r = grub_acpi_rsdt_find_table ((struct grub_acpi_table_header *) + (grub_addr_t) rsdpv2->rsdpv1.rsdt_addr, + sig); + if (r) +return r; return 0; } -- 2.28.0.windows.1 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2] commands/acpi: use xsdt_addr if present
According to the ACPI specification, in ACPI 2.0 or later, an ACPI-compatible OS must use the XSDT if present. So, we should use xsdt_addr instead of rsdt_addr if xsdt_addr is valid. Signed-off-by: Qiumiao Zhang --- Changes since v2: * update the commit message Qiumiao --- grub-core/commands/acpi.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/grub-core/commands/acpi.c b/grub-core/commands/acpi.c index deec4bb43..6a7c9ad87 100644 --- a/grub-core/commands/acpi.c +++ b/grub-core/commands/acpi.c @@ -514,7 +514,11 @@ grub_cmd_acpi (struct grub_extcmd_context *ctxt, int argc, char **args) /* Set revision variables to replicate the same version as host. */ rev1 = ! rsdp->revision; rev2 = rsdp->revision; - rsdt = (struct grub_acpi_table_header *) (grub_addr_t) rsdp->rsdt_addr; + if (rev2 && ((struct grub_acpi_rsdp_v20 *) rsdp)->xsdt_addr != NULL) + rsdt = (struct grub_acpi_table_header *) (grub_addr_t) ((struct grub_acpi_rsdp_v20 *) rsdp)->xsdt_addr; + else + rsdt = (struct grub_acpi_table_header *) (grub_addr_t) rsdp->rsdt_addr; + /* Load host tables. */ for (entry_ptr = (grub_uint32_t *) (rsdt + 1); entry_ptr < (grub_uint32_t *) (((grub_uint8_t *) rsdt) -- 2.28.0.windows.1 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: Re: [PATCH] acpi: use xsdt_addr if present
On Thu, Jun 08, 2023 at 07:09:36PM +0800, Qiumiao Zhang via Grub-devel wrote: >> According to the UEFI specification, in ACPI 2.0 or later, an >> ACPI-compatible OS must use the XSDT if present. > Sorry, I cannot find this in the UEFI spec. Though something more generic is > in the ACPI spec. Could you update the commit message accordingly? Sorry, I have confused the UEFI spec with the ACPI spec, I will update the commit message. >> So, we should use xsdt_addr instead of rsdt_addr if xsdt_addr is valid. > Ha! We have the same problem in the > grub-core/kern/acpi.c:grub_acpi_find_table(). > May I ask you to fix this function too? Of course in separate patch. I would be happy to do this. Qiumiao ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 0/5] loongarch: add relaxation support
New patch is ready: https://github.com/loongarch64/grub/commits/dev-master I need your help to confirm that binutils-2.40 and binutils-dev code have the same behavior when using compile options, thanks. 在 2023-06-12星期一的 06:35 +0800，Xi Ruoyao via Grub-devel写道： > On Wed, 2023-06-07 at 15:34 +0800, Xiaotian Wu wrote: > > Because the binutils of the loongarch architecture adds relaxation > > support [1], the next version of binutils will not be able to build > > grub. > > > > So we added the R_LARCH_B16, R_LARCH_B21 and R_LARCH_RELAX > > relocations > > to enhance grub compatibility. > > Wouldn't it be easier to just pass -mno-relax to the toolchain when > we > build GRUB? I don't think it makes too much sense to perform > relaxation > on a boot loader. The boot loader is generally the coldest code > paths > in a system, so relaxing it won't give any real benefit (well, if you > reboot a system 100 times you may finally save one second), but > increases the maintenance burden. > > Consider a new relaxation pattern is added after we release GRUB > 2.12. > Then if we simply pass -mno-relax, GRUB 2.12 will continue to work. > But > if we try to "support" relaxation this way, every distro will have to > patch GRUB 2.12 when the toolchain is updated. > > > [1]: > > https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=56576f4a722b7398d35802ecf7d4185c27d6d69b > > > > v1->v2: > > - split patch > > - drop cast code > > > > Xiaotian Wu (5): > > Use the correct format specifier for formatted output > > loongarch: Optimize code using pc variable > > loongarch: Rename function names > > loongarch: Add ELF relocation types documentation and comments > > loongarch: Add relaxation support > > > > grub-core/kern/arm64/dl_helper.c | 4 +- > > grub-core/kern/loongarch64/dl.c | 21 +++- > > grub-core/kern/loongarch64/dl_helper.c | 72 > > - > > - > > include/grub/elf.h | 3 ++ > > include/grub/loongarch64/reloc.h | 6 ++- > > util/grub-mkimagexx.c | 28 -- > > util/grub-module-verifier.c | 3 ++ > > 7 files changed, 124 insertions(+), 13 deletions(-) > > > -- Best Regards Xiaotian Wu ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v4 3/5] efi: Drop all uses of efi_call_XX wrappers
Le 12/06/2023 à 21:05, Daniel Kiper a écrit : On Mon, Jun 12, 2023 at 07:19:53PM +0200, Christian Hesse wrote: Ard Biesheuvel on Tue, 2023/05/23 17:31: Now that GCC can generate function calls using the correct calling convention for us, we can stop using the efi_call_XX () wrappers, and just dereference the function pointers directly. This avoids the untyped variadic wrapper routines, which means better type checking for the method calls. Building an Arch package from current git master makes the system unbootable with just a black screen. We did biscect this and found this commit (bb4aa6e06ee3877110a1dc0eb0d766ffac978993 in master) to be the first bad one. As gcc is mentioned in the commit message... We are building with gcc 13.1.1-1. Any idea what is going on here? Could you provide us more details? On which architecture/target do you run tests? Does GRUB start? If yes which kernel version do you try to boot? Could you send us a link to the GRUB and kernel packages and sources? Etc... Daniel Hi, Tests have been ran on a x86_64 laptop (but the issue has been reproduced on several other x86_64 machines). GRUB does not start, we don't even reach the GRUB menu, nothing more than a black screen at boot. The kernel we're trying to boot is `linux 6.3.7-arch1-1` [1] (package [2]), but I doubt the kernel itself is in cause as we're able to boot it with a GRUB package built without the bb4aa6e06ee3877110a1dc0eb0d766ffac978993 commit. Here's a working GRUB package [3] and a faulty one [4]. Sources are taken from https://git.savannah.gnu.org/git/grub.git [5]. I remain available if you need specific tests or additional information. [1] https://archlinux.org/packages/core/x86_64/linux/ [2] http://www.gtlib.gatech.edu/pub/archlinux/core/os/x86_64/linux-6.3.7.arch1-1-x86_64.pkg.tar.zst [3] https://pkgbuild.com/~eworm/grub/grub-2:2.06.r535.g6a080b9cd-2-x86_64.pkg.tar.zst [4] https://pkgbuild.com/~eworm/grub/grub-2:2.06.r554.gc016a969d-2-x86_64.pkg.tar.zst [5] https://gitlab.archlinux.org/archlinux/packaging/packages/grub/-/blob/main/PKGBUILD#L60 -- Regards, Robin Candau / Antiz OpenPGP_0xFDC3040B92ACA748.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v4 3/5] efi: Drop all uses of efi_call_XX wrappers
On Mon, Jun 12, 2023 at 07:19:53PM +0200, Christian Hesse wrote: > Ard Biesheuvel on Tue, 2023/05/23 17:31: > > Now that GCC can generate function calls using the correct calling > > convention for us, we can stop using the efi_call_XX () wrappers, and > > just dereference the function pointers directly. > > > > This avoids the untyped variadic wrapper routines, which means better > > type checking for the method calls. > > Building an Arch package from current git master makes the system unbootable > with just a black screen. We did biscect this and found this commit > (bb4aa6e06ee3877110a1dc0eb0d766ffac978993 in master) to be the first bad one. > > As gcc is mentioned in the commit message... We are building with gcc > 13.1.1-1. Any idea what is going on here? Could you provide us more details? On which architecture/target do you run tests? Does GRUB start? If yes which kernel version do you try to boot? Could you send us a link to the GRUB and kernel packages and sources? Etc... Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v4 3/5] efi: Drop all uses of efi_call_XX wrappers
Ard Biesheuvel on Tue, 2023/05/23 17:31:
> Now that GCC can generate function calls using the correct calling
> convention for us, we can stop using the efi_call_XX () wrappers, and
> just dereference the function pointers directly.
>
> This avoids the untyped variadic wrapper routines, which means better
> type checking for the method calls.
Building an Arch package from current git master makes the system unbootable
with just a black screen. We did biscect this and found this commit
(bb4aa6e06ee3877110a1dc0eb0d766ffac978993 in master) to be the first bad one.
As gcc is mentioned in the commit message... We are building with gcc
13.1.1-1. Any idea what is going on here?
--
main(a){char*c=/*Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/*Best regards my address:*/=0;b=c[a++];)
putchar(b-1/(/*Chriscc -ox -xc - && ./x*/b/42*2-3)*42);}
pgpFryIqi6gT1.pgp
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v3] docs: Add debugging chapter to development documentation
On Tue, Jun 06, 2023 at 12:48:39AM -0500, Glenn Washburn wrote: > Debugging GRUB can be tricky and require arcane knowledge. This will > help those unfamiliar with the process to get started debugging GRUB > with less effort. > > Signed-off-by: Glenn Washburn Reviewed-by: Daniel Kiper Thank you for adding this debugging chapter! Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] osdep/linux: Fix md array device enumeration
On Wed, Jun 07, 2023 at 02:33:35PM -0700, Kees Cook wrote: > On Wed, Jun 07, 2023 at 03:39:24PM +0200, Daniel Kiper wrote: > > On Tue, Jun 06, 2023 at 11:02:31AM -0700, Kees Cook wrote: > > > On Tue, Jun 6, 2023 at 10:27 AM Julian Andres Klode > > > wrote: > > > > > > > > On Tue, Jun 06, 2023 at 07:09:26PM +0200, Daniel Kiper wrote: > > > > > On Tue, Jun 06, 2023 at 06:15:27PM +0200, Julian Andres Klode wrote: > > > > > > On Tue, Jun 06, 2023 at 06:10:21PM +0200, Julian Andres Klode wrote: > > > > > [...] > > > > > This patch is in upstream as commit c39f27cd6 (osdep/linux: Fix md > > > > > array > > > > > device enumeration). > > > > > > Oh good. I really thought it had landed already, so thanks for > > > checking. I got worried this morning when I saw the email to > > > grub-devel. :P "Wasn't that fixed already?" :) But thank you for > > > making sure it hadn't gotten lost! Is there a way to close the tracker > > > item for it? > > > > I think you should be able to do that. > > Ah-ha, yes, I've closed it now. :) > https://salsa.debian.org/grub-team/grub/-/merge_requests/23 Cool! Thanks a lot! > > > > [...] > > > > > I realized right now that MD_MAX_DISKS defined in commit c39f27cd6 > > > > > (osdep/linux: Fix md array device enumeration) is not in sync with > > > > > commit 2a5e3c1f2 (disk/diskfilter: Don't make a RAID array with more > > > > > than 1024 disks). I think we should sync both numbers down to 1024... > > > > > > > > +1 > > > > > > Yeah, seems reasonable, though as I hinted in the original patch, this > > > number appeared to have been arbitrarily chosen by mdadm at the time. > > > > OK, we will bump it to 4096 as well. > > Yeah, I think _technically_ it can be higher than 1024, though ... I > struggle to imagine this for a boot device. ;) Yeah, same here... :-) Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 1/1] fs/udf: Fix out of bounds access
On Wed, Jun 07, 2023 at 08:26:44PM +0100, Darren Kenny wrote: > Hi Li,, > > LGTM! > > Reviewed-by: Darren Kenny Reviewed-by: Daniel Kiper Thank you for fixing these issues! Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] acpi: use xsdt_addr if present
On Thu, Jun 08, 2023 at 07:09:36PM +0800, Qiumiao Zhang via Grub-devel wrote: > According to the UEFI specification, in ACPI 2.0 or later, an ACPI-compatible > OS must use the XSDT if present. Sorry, I cannot find this in the UEFI spec. Though something more generic is in the ACPI spec. Could you update the commit message accordingly? > So, we should use xsdt_addr instead of rsdt_addr if xsdt_addr is valid. Ha! We have the same problem in the grub-core/kern/acpi.c:grub_acpi_find_table(). May I ask you to fix this function too? Of course in separate patch. > Signed-off-by: Qiumiao Zhang > --- > grub-core/commands/acpi.c | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/grub-core/commands/acpi.c b/grub-core/commands/acpi.c > index deec4bb43..6a7c9ad87 100644 > --- a/grub-core/commands/acpi.c > +++ b/grub-core/commands/acpi.c > @@ -514,7 +514,11 @@ grub_cmd_acpi (struct grub_extcmd_context *ctxt, int > argc, char **args) >/* Set revision variables to replicate the same version as host. */ >rev1 = ! rsdp->revision; >rev2 = rsdp->revision; > - rsdt = (struct grub_acpi_table_header *) (grub_addr_t) rsdp->rsdt_addr; > + if (rev2 && ((struct grub_acpi_rsdp_v20 *) rsdp)->xsdt_addr != NULL) > + rsdt = (struct grub_acpi_table_header *) (grub_addr_t) ((struct > grub_acpi_rsdp_v20 *) rsdp)->xsdt_addr; > + else > + rsdt = (struct grub_acpi_table_header *) (grub_addr_t) rsdp->rsdt_addr; This LGTM... Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 0/5] loongarch: add relaxation support
On Mon, Jun 12, 2023 at 04:20:41PM +0800, Xi Ruoyao via Grub-devel wrote: > On Mon, 2023-06-12 at 14:54 +0800, WANG Xuerui wrote: > > On 6/12/23 11:09, Xiaotian Wu wrote: > > > 在 2023-06-12星期一的 06:35 +0800，Xi Ruoyao via Grub-devel写道： > > > > On Wed, 2023-06-07 at 15:34 +0800, Xiaotian Wu wrote: > > > > > Because the binutils of the loongarch architecture adds relaxation > > > > > support [1], the next version of binutils will not be able to build > > > > > grub. > > > > > > > > > > So we added the R_LARCH_B16, R_LARCH_B21 and R_LARCH_RELAX > > > > > relocations > > > > > to enhance grub compatibility. > > > > Wouldn't it be easier to just pass -mno-relax to the toolchain when > > > > we > > > > build GRUB? I don't think it makes too much sense to perform > > > > relaxation > > > > on a boot loader. The boot loader is generally the coldest code > > > > paths > > > > in a system, so relaxing it won't give any real benefit (well, if you > > > > reboot a system 100 times you may finally save one second), but > > > > increases the maintenance burden. > > > > > > > Yes, it's easy to pass -mno-relax, but binutils-2.40 doesn't support > > > this option. > > Then maybe probe it in grub's configure and add it to CFLAGS when it's > > supported? IIUC every toolchain with support for relaxation should > > provide this toggle, and we already probe several LoongArch-specific > > flags right now so the infra is already there. > > --no-relax is accepted even with ld-2.38, the man page says: > >On platforms where the feature is not supported, both --relax and >--no-relax are accepted, but ignored. > > And it *should* disable relax for ld-2.41. If not I'd consider it a bug > and we should fix it for Binutils. > > If --no-relax behaves as expected I guess we just need > > LDFLAGS_PLATFORM += -Wl,--no-relax > > in "if test "x$target_cpu" = xloongarch64" block. It looks we have "no-relax" flag detection in the configure for sparc64 target. So, detection code is in place... Anyway, I do not have very strong preference here because relaxation code is not that big. So, we can carry it. Just please decide ASAP which solution you prefer. I am going to do code freeze this week... Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 0/5] loongarch: add relaxation support
On Wed, Jun 07, 2023 at 03:34:52PM +0800, Xiaotian Wu wrote: > Because the binutils of the loongarch architecture adds relaxation support > [1], the next version of binutils will not be able to build grub. > > So we added the R_LARCH_B16, R_LARCH_B21 and R_LARCH_RELAX relocations to > enhance grub compatibility. > > [1]: > https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=56576f4a722b7398d35802ecf7d4185c27d6d69b > > v1->v2: > - split patch > - drop cast code > > Xiaotian Wu (5): > Use the correct format specifier for formatted output > loongarch: Optimize code using pc variable > loongarch: Rename function names > loongarch: Add ELF relocation types documentation and comments For the patches 1-4 Reviewed-by: Daniel Kiper ... Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 0/5] loongarch: add relaxation support
On Mon, 2023-06-12 at 14:54 +0800, WANG Xuerui wrote: > On 6/12/23 11:09, Xiaotian Wu wrote: > > 在 2023-06-12星期一的 06:35 +0800，Xi Ruoyao via Grub-devel写道： > > > On Wed, 2023-06-07 at 15:34 +0800, Xiaotian Wu wrote: > > > > Because the binutils of the loongarch architecture adds relaxation > > > > support [1], the next version of binutils will not be able to build > > > > grub. > > > > > > > > So we added the R_LARCH_B16, R_LARCH_B21 and R_LARCH_RELAX > > > > relocations > > > > to enhance grub compatibility. > > > Wouldn't it be easier to just pass -mno-relax to the toolchain when > > > we > > > build GRUB? I don't think it makes too much sense to perform > > > relaxation > > > on a boot loader. The boot loader is generally the coldest code > > > paths > > > in a system, so relaxing it won't give any real benefit (well, if you > > > reboot a system 100 times you may finally save one second), but > > > increases the maintenance burden. > > > > > Yes, it's easy to pass -mno-relax, but binutils-2.40 doesn't support > > this option. > Then maybe probe it in grub's configure and add it to CFLAGS when it's > supported? IIUC every toolchain with support for relaxation should > provide this toggle, and we already probe several LoongArch-specific > flags right now so the infra is already there. --no-relax is accepted even with ld-2.38, the man page says: On platforms where the feature is not supported, both --relax and --no-relax are accepted, but ignored. And it *should* disable relax for ld-2.41. If not I'd consider it a bug and we should fix it for Binutils. If --no-relax behaves as expected I guess we just need LDFLAGS_PLATFORM += -Wl,--no-relax in "if test "x$target_cpu" = xloongarch64" block. -- Xi Ruoyao School of Aerospace Science and Technology, Xidian University ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 0/5] loongarch: add relaxation support
On 6/12/23 11:09, Xiaotian Wu wrote: 在 2023-06-12星期一的 06:35 +0800，Xi Ruoyao via Grub-devel写道： On Wed, 2023-06-07 at 15:34 +0800, Xiaotian Wu wrote: Because the binutils of the loongarch architecture adds relaxation support [1], the next version of binutils will not be able to build grub. So we added the R_LARCH_B16, R_LARCH_B21 and R_LARCH_RELAX relocations to enhance grub compatibility. Wouldn't it be easier to just pass -mno-relax to the toolchain when we build GRUB? I don't think it makes too much sense to perform relaxation on a boot loader. The boot loader is generally the coldest code paths in a system, so relaxing it won't give any real benefit (well, if you reboot a system 100 times you may finally save one second), but increases the maintenance burden. Yes, it's easy to pass -mno-relax, but binutils-2.40 doesn't support this option. Then maybe probe it in grub's configure and add it to CFLAGS when it's supported? IIUC every toolchain with support for relaxation should provide this toggle, and we already probe several LoongArch-specific flags right now so the infra is already there. Consider a new relaxation pattern is added after we release GRUB 2.12. Then if we simply pass -mno-relax, GRUB 2.12 will continue to work. But if we try to "support" relaxation this way, every distro will have to patch GRUB 2.12 when the toolchain is updated. GRUB 2.12 has not been released yet. If this patch can be merged before the release of grub 2.12, every distribution does not need to be patched. Moreover, both binutils-2.40 and future binutils 2.41+ will be able to compile normally. Consider a possible scenario where more relaxation patterns are implemented by future toolchains; I don't think grub should care, or get changed every single time when that happens. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
