Re: [PATCH] Remove HFS support
Daniel Axtens writes: >>> Have you checked that you can't boot them with HFS+? Because HFS+ >>> came in 1998, which was (AFAICT) pretty early on in the G3 >>> lifecycle. So I'd be really surprised if the firmware didn't support >>> booting from HFS+. I'd be very keen to hear. >> >> I have not tested that due to lack of time. The problem is that some >> early firmware versions might have issues with HFS+ that we haven't >> verified yet. > > Any approach that says 'we must wait for test results for very old > macs' puts the grub community in a bind. I'm not aware of anyone else > stepping up to contribute test results on old macs, and I can't go > across to an apple store and buy one. So in order to test this, the > entire grub upstream stalls on (AFAICT) you personally. > > This not the first time we find ourselves in this situation either. > For example, RH is carrying the 'powerpc-ieee1275: support larger > core.elf images' series out of tree because they need it to boot on > modern Power boxes. It broke on your machine in a way no-one else has > reproduced, and I last emailled you asking for more information to > debug the failure in May. As the person currently responsible for the Red Hat tree: I am also not happy about this state of affairs. If a use case is to be supported, someone needs to actually do the leg work to support it. Bug reports are all well and good, but if no one's actually able to fix them, they're just making a pile that's in the way. What I mean is this: right now the project has people (you) *testing* power macs, but no one actually *fixing* power macs, and unless someone starts fixing problems that materialize, it's at odds with reality to call it a supported platform. (And to be clear here, problems that materialize includes other people's patches, and debugging/sending fixes to them as would be expected from a subject matter expert.) As you point out downthread, I could go out and spend money on a vintage mac almost as old as I am to attempt to debug the problem myself. (This money would have to be my own, because Red Hat, RHEL, and Fedora are all uninterested in supporting power macs.) I would then have to learn the ins and outs of a platform that the manufacturer has not supported for about fifteen years.[1] We're talking about at least a month of my time, probably more, and that assumes it even reproduces on my machine - which there's no guarantee of. (If I did all that, and it didn't, what then?) It's time I don't have, and quite frankly it's more important to me to have grub support currently shipping hardware. Or I could just take the POWER patches from Daniel and IBM folks downstream, and keep a platform that customers care about working. This takes very little of my time in comparison, but fails to heal the current divergence. Be well, --Robbie 1: Having grown up with these beige boxes, I did previously owned an iBook G3 on which I did run Linux - but have not in this decade (as it is far from a capable computing platform in the modern era), nor would I have been able to debug it at the time. I do still use the AEK & AEK II as daily drivers, though. signature.asc Description: PGP signature ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Remove HFS support
On 8/30/22 18:37, Robbie Harwood wrote: As the person currently responsible for the Red Hat tree: I am also not happy about this state of affairs. I don't want to sound rude, but GRUB isn't a RedHat-only project, it's a community project. So, while I understand RH's point of view, I would also like to ask you to see other people's perspectives. If a use case is to be supported, someone needs to actually do the leg work to support it. Bug reports are all well and good, but if no one's actually able to fix them, they're just making a pile that's in the way. Vladimir has asked multiple times for bug reports on the issues and he said, he would fix them. I also offered that we do a crowd-funding campaign to fund the development. I would be happy to throw in some money as well. So, if we could get some report on what needs to be addressed, that would be great. Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer `. `' Physicist `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Remove HFS support
"Vladimir 'phcoder' Serbinenko" writes: > Le ven. 26 août 2022, 15:47, Daniel Axtens a écrit : > >> Let me answer this out of order. >> >>> I understand the need to sometimes get rid of old code, but since >>> the HFS module can be blacklisted as Vladimir explains, I don't >>> really understand the reasoning in this particular case. >> >> I want _all_ grub code to reach a minimum standard of not crashing or >> corrupting memory in the presence of malicious input. HFS does not >> reach that standard. > > That is a very high standard. Products with a huge security team like > Chrome don't reach this standard. That's not accurate. See https://bughunters.google.com/about/rules/5745167867576320/chrome-vulnerability-reward-program-rules (the important parts of which are that Google not only pays out for fuzzing crashes but provides infrastructure to run your fuzzers on). >> Whether or not the HFS module could be omitted from a signed binary >> doesn't really bear on the fact that there are bugs in the code, the >> presence of bugs has been publicly known for over 18 months (see commit >> 1c15848838d9) and no-one has shown any intention of fixing the bugs. > > That is besides the point of using GRUB on PowerMac or any other > platform with unsigned bootchain. And for signed bootchains it can be > blacklisted in the code like it already is. Which problem do you try > to solve? Every time there's a CVE found in the project, it reflects poorly on the project. I think we both know that that's not really fair - CVEs just mean the project is being looked at, and are not a measure of quality - but we're talking about a public perception issue here. If a project knows that there are potential security problems in an area of its code, and not only doesn't take steps to address them but instead actively resists doing so, it will be assumed to be an unhealthy project - and rightly so, I think. >> (And as I said in another email, HFS has in fact been built in to a >> signed binary recently. Module-based protection is great in theory >> but this example demonstrates that it falls down in practice.) > > Then implement a better module-based security with security attributes > stored in a special section of the binary how we do with the license > to allow only EFISB-approved modules to load under lockdown It is incumbent on those who want HFS support to, you know, work at supporting HFS. Telling other people to do it instead does not further that goal - it just pushes them away. > It's fine to commit patches that improve for other PPC without waiting for > PowerMacs. In fact I have often seen benefits from such moves. Sure, but it is bad practice to commit patches that are known to break a platform (which is what we're talking about here) without some plan to handle that going forward. Specifically, I imagine Daniel Axtens and I would be happy if the "powerpc-ieee1275: support larger core.elf images" series landed as-is, but I doubt Adrian would be. >> This not the first time we find ourselves in this situation either. For >> example, RH is carrying the 'powerpc-ieee1275: support larger core.elf >> images' series out of tree because they need it to boot on modern Power >> boxes. It broke on your machine in a way no-one else has reproduced, and >> I last emailled you asking for more information to debug the failure in >> May. > > This can happen with EFI as well. And indeed have. Several times. Should we > drop EFI support? That's not how this works, or has worked. Those who care about EFI work with maintainers and patch authors to keep their use case operational. Be well, --Robbie signature.asc Description: PGP signature ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Remove HFS support
John Paul Adrian Glaubitz writes: > On 8/30/22 18:37, Robbie Harwood wrote: > >> As the person currently responsible for the Red Hat tree: I am also >> not happy about this state of affairs. > > I don't want to sound rude, but GRUB isn't a RedHat-only project, it's a > community project. Nowhere in my post did I say I was speaking for grub - the tag at the top literally says that my post is intended as the Red Hat tree maintainer. I'm not an upstream maintainer and it is not reasonable to presume that I speak for grub, or that my views represent those of the project. > So, while I understand RH's point of view, I would also like to ask > you to see other people's perspectives. If you're going to make claims like this, you need to actually engage with what others are saying. Seeing someone else's perspective doesn't mean you think they're right: it means you're trying to engage with their viewpoint in order to reach consensus. I do not see engagement with my viewpoint from you, and meanwhile I'm trying to engage with yours. >> If a use case is to be supported, someone needs to actually do the >> leg work to support it. Bug reports are all well and good, but if no >> one's actually able to fix them, they're just making a pile that's in >> the way. > > Vladimir has asked multiple times for bug reports on the issues and he > said, he would fix them. I also offered that we do a crowd-funding > campaign to fund the development. I would be happy to throw in some > money as well. That's nice of you to offer, but you should know that those of us already employed full-time in software will likely be unable to receive those funds. It is also not the same thing as having that work get done. For my part, I reviewed the original POWER patch and am happy to review changes to make it powermac compatible. > So, if we could get some report on what needs to be addressed, that would > be great. I think I was pretty clear about this in the part of my message that you trimmed out, but I'll reiterate just in case: the series Daniel Axtens and I are talking about is "powerpc-ieee1275: support larger core.elf images" on which you provided feedback on 2021-11-23. Daniel Axtens last asked you on-list for additional debugging information on 2022-05-17, to which you have not replied. Daniel also indicated in that post that they are likely at the limit of their ability to debug the failure regardless. Be well, --Robbie signature.asc Description: PGP signature ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 2/3] term/serial: Add support for PCI serial devices
On Mon, 29 Aug 2022 12:53:54 +0200
Peter Zijlstra wrote:
> On Fri, Aug 26, 2022 at 01:32:25PM -0500, Glenn Washburn wrote:
> > On Fri, 26 Aug 2022 13:01:44 +0200
> > pet...@infradead.org wrote:
> >
> > > Loosely based on early_pci_serial_init() from Linux, allow GRUB to make
> > > use of PCI serial devices.
> > >
> > > Specifically, my Alderlake NUC exposes the Intel AMT SoL UART as a PCI
> > > enumerated device but doesn't include it in the EFI tables.
> > >
> > > Tested and confirmed working on a "Lenovo P360 Tiny" with Intel AMT
> > > enabled. This specific machine has (from lspci -vv):
> > >
> > > 00:16.3 Serial controller: Intel Corporation Device 7aeb (rev 11)
> > > (prog-if 02 [16550])
> > > DeviceName: Onboard - Other
> > > Subsystem: Lenovo Device 330e
> > > Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop-
> > > ParErr- Stepping- SERR- FastB2B- DisINTx-
> > > Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort-
> > > SERR- > > Interrupt: pin D routed to IRQ 19
> > > Region 0: I/O ports at 40a0 [size=8]
> > > Region 1: Memory at b4224000 (32-bit, non-prefetchable) [size=4K]
> > > Capabilities: [40] MSI: Enable- Count=1/1 Maskable- 64bit+
> > > Address: Data:
> > > Capabilities: [50] Power Management version 3
> > > Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA
> > > PME(D0-,D1-,D2-,D3hot-,D3cold-)
> > > Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
> > > Kernel driver in use: serial
> > >
> > > >From which the following config (/etc/default/grub) gets a working
> > > serial setup:
> > >
> > > GRUB_CMDLINE_LINUX="console=tty0 earlyprintk=pciserial,00:16.3,115200
> > > console=ttyS0,115200"
> > > GRUB_SERIAL_COMMAND="serial --port=0x40a0 --speed=115200"
> >
> > Forgive my ignorance, I'm a bit confused why the above line does not
> > work without the patch, or does it? Is it because the line
> > "grub_pci_write (cmd_addr, cmdreg | GRUB_PCI_COMMAND_IO_ENABLED);" was
> > needed to enable that IO port?
>
> Correct. Without that the IO port doesn't function and life is sad :-)
>
> > Either way, I think it would be better to example would be something
> > like:
> >
> > GRUB_SERIAL_COMMAND="serial --speed=115200 pci0"
>
> This made me think the below delta might be a better option.
> Then we could write:
>
>GRUB_SERIAL_COMMAND="serial --speed=115200 pci,00:16.3"
>
> Hmm?
Hmm, I like this idea because its more descriptive for the user. I
don't particularly like the comma separator. Perhaps dash, underscore,
or forward slash are better or maybe no separator at all. I also don't
feel that strongly about it.
Glenn
>
> ---
>
> Index: grub/grub-core/term/pci/serial.c
> ===
> --- grub.orig/grub-core/term/pci/serial.c
> +++ grub/grub-core/term/pci/serial.c
> @@ -30,10 +30,10 @@ find_pciserial (grub_pci_device_t dev, g
>struct grub_serial_port *port;
>grub_uint32_t class, bar;
>grub_uint16_t cmdreg;
> - int *port_num = data;
>grub_err_t err;
>
>(void)pciid;
> + (void)data;
>
>cmd_addr = grub_pci_make_address (dev, GRUB_PCI_REG_COMMAND);
>cmdreg = grub_pci_read (cmd_addr);
> @@ -57,7 +57,10 @@ find_pciserial (grub_pci_device_t dev, g
>if (port == NULL)
> return 0;
>
> - port->name = grub_xasprintf ("pci%d", (*port_num));
> + port->name = grub_xasprintf ("pci,%02x:%02x.%x",
> +grub_pci_get_bus (dev),
> +grub_pci_get_device (dev),
> +grub_pci_get_function (dev));
>if (port->name == NULL)
> goto error;
>
> @@ -76,8 +79,6 @@ find_pciserial (grub_pci_device_t dev, g
>if (err != GRUB_ERR_NONE)
> goto error;
>
> - (*port_num)++;
> -
>return 0;
>
> error:
> @@ -89,7 +90,5 @@ error:
> void
> grub_pciserial_init (void)
> {
> - int port_num;
> -
> - grub_pci_iterate (find_pciserial, &port_num);
> + grub_pci_iterate (find_pciserial, NULL);
> }
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v6 2/2] cryptodisk: Allows UUIDs to be compared in a dash-insensitive manner
On Mon, 29 Aug 2022 07:38:24 +0200
Patrick Steinhardt wrote:
> On Fri, Aug 19, 2022 at 06:06:15PM -0500, Glenn Washburn wrote:
> > A user can now specify UUID strings with dashes, instead of having to remove
> > dashes. This is backwards-compatability preserving and also fixes a source
> > of user confusion over the inconsistency with how UUIDs are specified
> > between file system UUIDs and cryptomount UUIDs. Since cryptsetup, the
> > reference implementation for LUKS, displays and generates UUIDs with dashes
> > there has been additional confusion when using the UUID strings from
> > cryptsetup as exact input into GRUB does not find the expected cryptodisk.
> >
> > A new function grub_uuidcasecmp is added that is general enough to be used
> > other places where UUIDs are being compared.
> >
> > Signed-off-by: Glenn Washburn
> > ---
> > grub-core/disk/cryptodisk.c | 4 ++--
> > grub-core/disk/geli.c | 2 +-
> > grub-core/disk/luks.c | 21 -
> > grub-core/disk/luks2.c | 15 ---
> > include/grub/misc.h | 32
> > 5 files changed, 43 insertions(+), 31 deletions(-)
> >
> > diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> > index e89430812..a4cd8445f 100644
> > --- a/grub-core/disk/cryptodisk.c
> > +++ b/grub-core/disk/cryptodisk.c
> > @@ -702,7 +702,7 @@ grub_cryptodisk_open (const char *name, grub_disk_t
> > disk)
> >if (grub_memcmp (name, "cryptouuid/", sizeof ("cryptouuid/") - 1) == 0)
> > {
> >for (dev = cryptodisk_list; dev != NULL; dev = dev->next)
> > - if (grub_strcasecmp (name + sizeof ("cryptouuid/") - 1, dev->uuid) == 0)
> > + if (grub_uuidcasecmp (name + sizeof ("cryptouuid/") - 1, dev->uuid,
> > sizeof (dev->uuid)) == 0)
> > break;
> > }
> >else
> > @@ -929,7 +929,7 @@ grub_cryptodisk_get_by_uuid (const char *uuid)
> > {
> >grub_cryptodisk_t dev;
> >for (dev = cryptodisk_list; dev != NULL; dev = dev->next)
> > -if (grub_strcasecmp (dev->uuid, uuid) == 0)
> > +if (grub_uuidcasecmp (dev->uuid, uuid, sizeof (dev->uuid)) == 0)
> >return dev;
> >return NULL;
> > }
> > diff --git a/grub-core/disk/geli.c b/grub-core/disk/geli.c
> > index e190066f9..722910d64 100644
> > --- a/grub-core/disk/geli.c
> > +++ b/grub-core/disk/geli.c
> > @@ -305,7 +305,7 @@ geli_scan (grub_disk_t disk, grub_cryptomount_args_t
> > cargs)
> >return NULL;
> > }
> >
> > - if (cargs->search_uuid != NULL && grub_strcasecmp (cargs->search_uuid,
> > uuid) != 0)
> > + if (cargs->search_uuid != NULL && grub_uuidcasecmp (cargs->search_uuid,
> > uuid, sizeof (uuid)) != 0)
> > {
> >grub_dprintf ("geli", "%s != %s\n", uuid, cargs->search_uuid);
> >return NULL;
> > diff --git a/grub-core/disk/luks.c b/grub-core/disk/luks.c
> > index 7f837d52c..9e1e6a5d9 100644
> > --- a/grub-core/disk/luks.c
> > +++ b/grub-core/disk/luks.c
> > @@ -66,10 +66,7 @@ static grub_cryptodisk_t
> > luks_scan (grub_disk_t disk, grub_cryptomount_args_t cargs)
> > {
> >grub_cryptodisk_t newdev;
> > - const char *iptr;
> >struct grub_luks_phdr header;
> > - char *optr;
> > - char uuid[sizeof (header.uuid) + 1];
> >char ciphername[sizeof (header.cipherName) + 1];
> >char ciphermode[sizeof (header.cipherMode) + 1];
> >char hashspec[sizeof (header.hashSpec) + 1];
> > @@ -92,19 +89,9 @@ luks_scan (grub_disk_t disk, grub_cryptomount_args_t
> > cargs)
> >|| grub_be_to_cpu16 (header.version) != 1)
> > return NULL;
> >
> > - grub_memset (uuid, 0, sizeof (uuid));
> > - optr = uuid;
> > - for (iptr = header.uuid; iptr < &header.uuid[ARRAY_SIZE (header.uuid)];
> > - iptr++)
> > + if (cargs->search_uuid != NULL && grub_uuidcasecmp (cargs->search_uuid,
> > header.uuid, sizeof (header.uuid)) != 0)
> > {
> > - if (*iptr != '-')
> > - *optr++ = *iptr;
> > -}
> > - *optr = 0;
> > -
> > - if (cargs->search_uuid != NULL && grub_strcasecmp (cargs->search_uuid,
> > uuid) != 0)
> > -{
> > - grub_dprintf ("luks", "%s != %s\n", uuid, cargs->search_uuid);
> > + grub_dprintf ("luks", "%s != %s\n", header.uuid, cargs->search_uuid);
> >return NULL;
> > }
>
> I haven't noticed this in my previous review round, but I think this is
> wrong because `header.uuid` is never NUL-terminated. It is explicitly 40
> characters long and read directly from disk, so there wouldn't be any
> room for it to be NUL-terminated.
Why not? UUIDs are 32 hex characters with typically 4 dashes, that
makes 37 bytes needed including the NULL byte. In fact, I believe that
cryptsetup always creates the uuid field NULL terminated. Also, the
LUKS1 spec, and by extension LUKS2 spec, mandates NULL termination. The
uuid field is specified as "char[]" and "char[], a string stored as null
terminated sequence of 8-bit characters7".
>
> So we'd rather have to:
>
> grub_dprintf ("luks2", "%.*s != %s\n",
Re: [Regression] efi: Don't display a uefi-firmware entry if it's not supported
Philip Müller writes:
>> Hello Robbie, hello Daniel,
>>
>> with the commit 26031d3b101648352e4e427f04bf69d320088e77
>> 30_uefi-firmware will always call `fwsetup --is-supported' to check
>> if the system supports EFI or not. However most installed grub
>> versions on MBR don't support the '--is-supported' flag and crash the
>> menu creation routine.
>>
>> Arch Linux is tracking the issue here: https://bugs.archlinux.org/task/75701
>>
>> What would be the best approach with older installations of grub via
>> grub-install not having to reinstall grub to MBR as some users don't
>> even know on how to install grub properly as that job a graphical
>> installer does.
>
> Hi all,
Adding grub-devel to CC.
> I looked into the '30_uefi-firmware' changes a little more and also
> commented my findings at the Arch-Bugtracker:
> https://bugs.archlinux.org/task/75701#comment210684
>
> Before Menu changes we simply had this:
>
> ### BEGIN /etc/grub.d/30_uefi-firmware ###
> menuentry 'UEFI Firmware Settings' $menuentry_id_option 'uefi-firmware' {
>fwsetup
> }
> ### END /etc/grub.d/30_uefi-firmware ###
>
> After Menu changes we went to that:
>
> ### BEGIN /etc/grub.d/30_uefi-firmware ###
> fwsetup --is-supported
> if [ "$grub_platform" = "efi" -a "$?" = 0 ]; then
>menuentry 'UEFI Firmware Settings' $menuentry_id_option 'uefi-firmware' {
> fwsetup
>}
> fi
> ### END /etc/grub.d/30_uefi-firmware ###
>
> So 0eb684e8bfb0a9d2d42017a354740be25947babe I get and
> 26031d3b101648352e4e427f04bf69d320088e77 tries to improve things,
> however doesn't count in what will happen if 'fwsetup' is not supporting
> the flag. It may either crash due to
> 1e79bbfbda24a08cb856ff30f8b1bec460779b91 or start UEFI firmware instead.
Why doesn't grub on the MBR get updated when you install a new grub
package? That seems like the real issue here - what am I missing?
Regardless, if we can't count on fwsetup being updated, then I think we
need to go back to the original version of my patch which doesn't have
--is-supported.
> Simply don't break user-space when older grub got installed to MBR.
> Somehow this idea needs some more thinking and a solution for that
> case too.
Sure, what do you suggest?
Be well,
--Robbie
signature.asc
Description: PGP signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [Regression] efi: Don't display a uefi-firmware entry if it's not supported
On Tue, Aug 30, 2022 at 4:16 PM Robbie Harwood wrote: > Why doesn't grub on the MBR get updated when you install a new grub > package? That seems like the real issue here - what am I missing? I think the grub project tries to maintain config file compatibility with older versions so that users can use new config files even if an older grub version is still installed in /boot/grub. > Regardless, if we can't count on fwsetup being updated, then I think we > need to go back to the original version of my patch which doesn't have > --is-supported. You could add a feature flag, which causes grub-core to set an environment variable when a new feature is supported. See the features array in grub-core/normal/main.c. You would then check for this feature flag in the grub.d snippet before calling fwsetup --is-supported. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [Regression] efi: Don't display a uefi-firmware entry if it's not supported
On 8/30/22 23:14, Mike Gilbert wrote:> On Tue, Aug 30, 2022 at 4:16 PM Robbie Harwood wrote: >> Why doesn't grub on the MBR get updated when you install a new grub >> package? That seems like the real issue here - what am I missing? > > I think the grub project tries to maintain config file compatibility > with older versions so that users can use new config files even if an > older grub version is still installed in /boot/grub. > In my opinion the grub configuration scheme is already too complex (a script that executes a set of scripts that generate sections of a script executed on boot) to also throw backward compatibility in the mix. I agree with Robbie here, either don't re-generate your GRUB config file in a installed system or update the GRUB core image in the embedding gap too. Anything but that feels like an uphill battle to me and would just add much more complexity. >> Regardless, if we can't count on fwsetup being updated, then I think we >> need to go back to the original version of my patch which doesn't have >> --is-supported. > Indeed. The original patch didn't use that option, it was added to address some feedback that Robbie had on an earlier version. > You could add a feature flag, which causes grub-core to set an > environment variable when a new feature is supported. See the features > array in grub-core/normal/main.c. > > You would then check for this feature flag in the grub.d snippet > before calling fwsetup --is-supported. > Please don't. As mentioned, I think we should aim to simplify the grub.cfg instead of making it more complicated. -- Best regards, Javier Martinez Canillas Core Platforms Red Hat ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [Regression] efi: Don't display a uefi-firmware entry if it's not supported
On 30.08.22 23:34, Javier Martinez Canillas wrote: You could add a feature flag, which causes grub-core to set an environment variable when a new feature is supported. See the features array in grub-core/normal/main.c. You would then check for this feature flag in the grub.d snippet before calling fwsetup --is-supported. Please don't. As mentioned, I think we should aim to simplify the grub.cfg instead of making it more complicated. Well I think it would be the best approach to add backward compatibility as most users don't even know on how to install grub via grub-install. That is done via the graphical installer Calamares on most Arch-based Distributions. Updating the grub menu is common if you install multiple kernels or use snapshots via BTRFS. Simply calling 'fwsetup' is a big NO-NO to me and others. The old version runs into the EFI firware or simply turn off the PC during boot, which creates boot loops for some or unbootable systems. I checked on my end with an older grub in /boot and the updated menu.cfg script. Only when removing the snippet of 30_uefi-firmware the system is bootable again. Also social media is slowly picking up that issue: https://www.youtube.com/watch?v=b_KHtK2b5cA -- Best, Philip OpenPGP_signature Description: OpenPGP digital signature ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [Regression] efi: Don't display a uefi-firmware entry if it's not supported
On 8/31/22 00:07, Philip Müller wrote:
> On 30.08.22 23:34, Javier Martinez Canillas wrote:
>>> You could add a feature flag, which causes grub-core to set an
>>> environment variable when a new feature is supported. See the features
>>> array in grub-core/normal/main.c.
>>>
>>> You would then check for this feature flag in the grub.d snippet
>>> before calling fwsetup --is-supported.
>>>
>> Please don't. As mentioned, I think we should aim to simplify the grub.cfg
>> instead of making it more complicated.
>
> Well I think it would be the best approach to add backward compatibility
> as most users don't even know on how to install grub via grub-install.
> That is done via the graphical installer Calamares on most Arch-based
> Distributions. Updating the grub menu is common if you install multiple
> kernels or use snapshots via BTRFS.
>
> Simply calling 'fwsetup' is a big NO-NO to me and others. The old
> version runs into the EFI firware or simply turn off the PC during boot,
> which creates boot loops for some or unbootable systems.
>
I'm OK to not call fwsetup at all, that's what we originally had in the
series posted by Robbie, for example in v2:
https://lists.gnu.org/archive/html/grub-devel/2022-08/msg00169.html
Then they added the fwsetup --is-supported option as mentioned because
other developer asked for that. That patch was included in v3 onward:
https://lists.gnu.org/archive/html/grub-devel/2022-08/msg00196.html
> I checked on my end with an older grub in /boot and the updated menu.cfg
> script. Only when removing the snippet of 30_uefi-firmware the system is
> bootable again.
>
That's fair. Then probably what we should do is to partially revert
commit 26031d3b1016 ("efi: Don't display a uefi-firmware entry if
it's not supported"). Something like the following patch perhaps ?
From c3edc64687686d9a5a54f769ec03101b2c4cdef1 Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas
Date: Wed, 31 Aug 2022 00:30:31 +0200
Subject: [PATCH] templates: Don't execute fwsetup unconditionally
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Commit 26031d3b1016 ("efi: Don't display a uefi-firmware entry if it's not
supported") added a new `fwsetup --is-supported` option that could be used
to check if the firmware allows to enter into a firmware user interface.
That option was then used by /etc/grub.d/30_uefi-firmware script to figure
out whether a `fwsetup` entry should be included or not in the boot menu.
But unfortunately, old GRUB images will fail to boot if are booted with an
updated GRUB config file. Since the `fwsetup --is-supported` call would be
taken as a plan `fwsetup` with no options.
This could either lead to a crash (i.e: on legacy BIOS systems where that
is not supported) or to the machine entering into the firmware settings.
To prevent that, let's partially revert the mentioned commit and keep the
old logic that didn't execute the `fwsetup` command and just included an
entry for it if GRUB is executed in an EFI platform.
Fixes: 26031d3b1016 ("efi: Don't display a uefi-firmware entry if it's not
supported")
Reported-by: Philip Müller
Signed-off-by: Javier Martinez Canillas
---
util/grub.d/30_uefi-firmware.in | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/util/grub.d/30_uefi-firmware.in b/util/grub.d/30_uefi-firmware.in
index c1731e53..b6041b55e2a0 100644
--- a/util/grub.d/30_uefi-firmware.in
+++ b/util/grub.d/30_uefi-firmware.in
@@ -31,8 +31,7 @@ LABEL="UEFI Firmware Settings"
gettext_printf "Adding boot menu entry for UEFI Firmware Settings ...\n" >&2
cat << EOF
-fwsetup --is-supported
-if [ "\$grub_platform" = "efi" -a "\$?" = 0 ]; then
+if [ "\$grub_platform" = "efi" ]; then
menuentry '$LABEL' \$menuentry_id_option 'uefi-firmware' {
fwsetup
}
--
2.37.1
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [Regression] efi: Don't display a uefi-firmware entry if it's not supported
On 31.08.22 00:43, Javier Martinez Canillas wrote: To prevent that, let's partially revert the mentioned commit and keep the old logic that didn't execute the `fwsetup` command and just included an entry for it if GRUB is executed in an EFI platform. I was about to do the same on my end when we discussed it internally ... -- Best, Philip OpenPGP_signature Description: OpenPGP digital signature ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] fs/ext2: ignore the large_dir incompat feature
Recently, ext4 added the a large_dRecently, ext4 added the a large_dir
feature, which adds support for a 3 level htree directory support.
Grub supports existing file systems with htree directories by ignoring
their existence, and since the index nodes for the hash tree look like
deleted directory entries (by design), grub can simply do a brute
force O(n) linear search of directories. The same is true for 3 level
deep htrees indicated by large_dir feature flag.
Hence, it is safe for gub to ignore the large_dir incompat feature.
Fixes: https://savannah.gnu.org/bugs/?61606
Signed-off-by: Theodore Ts'o
---
I submitted this patch back in December 2021, not realizing that
apparently no one is actually scanning the bug tracking system on
savannah.gnu.org even though it is listed as the official bug tracking
system for GRUB. So I'm trying to submit this via grub-devel.
grub-core/fs/ext2.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/grub-core/fs/ext2.c b/grub-core/fs/ext2.c
index 0989e26e1..654580e9d 100644
--- a/grub-core/fs/ext2.c
+++ b/grub-core/fs/ext2.c
@@ -104,6 +104,7 @@ GRUB_MOD_LICENSE ("GPLv3+");
#define EXT4_FEATURE_INCOMPAT_MMP 0x0100
#define EXT4_FEATURE_INCOMPAT_FLEX_BG 0x0200
#define EXT4_FEATURE_INCOMPAT_CSUM_SEED0x2000
+#define EXT4_FEATURE_INCOMPAT_LARGEDIR 0x4000 /* >2GB or 3-lvl htree */
#define EXT4_FEATURE_INCOMPAT_ENCRYPT 0x1
/* The set of back-incompatible features this driver DOES support. Add (OR)
@@ -129,10 +130,17 @@ GRUB_MOD_LICENSE ("GPLv3+");
* checksummed filesystem. Safe to ignore for now since the
* driver doesn't support checksum verification. However, it
* has to be removed from this list if the support is added
later.
+ * large_dir: Not back-incompatible given that the grub ext2 driver does
+ * not implement EXT2_FEATURE_COMPAT_DIR_INDEX. If grub
+ * eventually supports the htree feature (aka dir_index)
+ * it should support 3 level htrees and then move
+ * EXT4_FEATURE_INCOMPAT_LARGEDIR to
+ * EXT2_DRIVER_SUPPORTED_INCOMPAT.
*/
#define EXT2_DRIVER_IGNORED_INCOMPAT ( EXT3_FEATURE_INCOMPAT_RECOVER \
| EXT4_FEATURE_INCOMPAT_MMP \
-| EXT4_FEATURE_INCOMPAT_CSUM_SEED)
+| EXT4_FEATURE_INCOMPAT_CSUM_SEED \
+| EXT4_FEATURE_INCOMPAT_LARGEDIR)
#define EXT3_JOURNAL_MAGIC_NUMBER 0xc03b3998U
--
2.31.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [Regression] efi: Don't display a uefi-firmware entry if it's not supported
Had this problem on my Arch Linux and EOS setup had to chroot in and fiddle with this last week: https://youtu.be/b_KHtK2b5cA On 8/31/22, Philip Müller via Grub-devel wrote: > On 31.08.22 00:43, Javier Martinez Canillas wrote: >> To prevent that, let's partially revert the mentioned commit and keep the >> old logic that didn't execute the `fwsetup` command and just included an >> entry for it if GRUB is executed in an EFI platform. > > I was about to do the same on my end when we discussed it internally ... > > -- > Best, Philip > > ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 3/5] ns8250: Add base support for MMIO UARTs
On Mon, 2022-05-23 at 19:11 +0200, Sven Anderson wrote: > > I had a couple of sleepless nights trying to find out why this didn't > work for my MMIO UART (Intel Cannon Lake PCH Intel C246), so I > thought I would share my findings with others in a similar situation. > (See below.) > .../... > > This code assumes that the registers are only 8 bit wide. Apparently > for my chipset they are 32 bits wide, so it only (finally) worked > when I rewrote this code to address and write full grub_uint32_t > values, like this: > > -- > volatile grub_uint32_t* p = (void*)(port->mmio_base); > *(p + reg) = (grub_uint32_t)(value); > -- Sorry, I dopped the ball on this for a while. Getting back to it and will re-submit the patches. Can you send me a dump of your SPCR table ? I'd like to see if it properly says "32-bit" there to auto-detect this. Cheers, Ben. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
