Re: [PATCH 2/2] Ensure nvram is available and functional on IEEE1275

2022-08-29 Thread Ismael Luceno
On Thu, 25 Aug 2022 12:24:17 +0800
Michael Chang  wrote:
<...> 
> Apparently there's missing grub_set_install_backup_ponr between
> successful image embedding and grub_install_register_ieee1275 and we
> should fix that as well.

Thanks for the feedback; I've sent v2.

<...>
> > +  if (linux_kmod_load("nvram"))
> > +grub_util_error (_("%s: kernel module not found"), "nvram");
> > +  fd = open ("/dev/nvram", O_RDWR);
> > +  if (fd == -1)
> > +grub_util_error ("/dev/nvram: %s", strerror(errno));
<...> 
> I'm wondering why it is needed. The nvram module should be loaded
> on-demand via linux kernel's request_module() and modalias trick
> whever /dev/nvram is accessed.

It was based on another patch that did so but without the extra
checking, I guess both are wrong.

It's enough checking /dev/nvram is operational; it's necessary
to fail early because other commands down the line may get ENODEV if
the module fails to load (e.g. the file may have been removed). I moved
this to an earlier point in grub-install.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v2 2/2] grub-install: Ensure a functional /dev/nvram

2022-08-29 Thread Ismael Luceno
This enables an early failure; for i386-ieee1275 and powerpc-ieee1275 on
Linux, without /dev/nvram the system may be left in an unbootable state.

Signed-off-by: Ismael Luceno 
---
 util/grub-install.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/util/grub-install.c b/util/grub-install.c
index 527b85e27aa7..c84a2fb0526f 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -827,6 +827,16 @@ fill_core_services (const char *core_services)
   free (sysv_plist);
 }
 
+static void
+try_open (const char *path)
+{
+  FILE *f;
+  f = grub_util_fopen (path, "r+");
+  if (!f)
+grub_util_error (_("Unable to open %s: %s"), path, strerror(errno));
+  fclose (f);
+}
+
 int
 main (int argc, char *argv[])
 {
@@ -1026,6 +1036,19 @@ main (int argc, char *argv[])
   break;
 }
 
+  switch (platform)
+{
+case GRUB_INSTALL_PLATFORM_I386_IEEE1275:
+case GRUB_INSTALL_PLATFORM_POWERPC_IEEE1275:
+#ifdef __linux__
+  /* On Linux, ensure /dev/nvram is _functional_.  */
+  try_open ("/dev/nvram");
+#endif
+  break;
+default:
+  break;
+}
+
   /* Find the EFI System Partition.  */
 
   if (is_efi)
-- 
2.37.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v2 0/2] Fix installation issues on ppc64le

2022-08-29 Thread Ismael Luceno
If the nvram device is non-functional, e.g. because the nvram module isn't
loaded and it's file been removed from the filesystem, thus can't be
loaded, the installation will be attempted but the system will be left in
an unbootable state.

The boot process shows:

Welcome to GRUB!

error: ../../grub-core/kern/dl.c:380:symbol `grub_disk_get_size' not found.
Entering rescue mode...
grub rescue>

The patches in this series introduce a couple of points of no return
before updating the nvram, and make sure /dev/nvram is functional or
fail early.

Ismael Luceno (2):
  grub-install: Add missing points of no return for IEEE1275 on
i386/powerpc
  grub-install: Ensure a functional /dev/nvram

 util/grub-install.c | 25 +
 1 file changed, 25 insertions(+)

-- 
2.37.1

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v2 1/2] grub-install: Add missing points of no return for IEEE1275 on i386/powerpc

2022-08-29 Thread Ismael Luceno
Signed-off-by: Ismael Luceno 
---
 util/grub-install.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/util/grub-install.c b/util/grub-install.c
index 7b04bd3c534b..527b85e27aa7 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -1835,6 +1835,7 @@ main (int argc, char *argv[])
{
  if (write_to_disk (ins_dev, imgfile))
grub_util_error ("%s", _("failed to copy Grub to the PReP 
partition"));
+ grub_set_install_backup_ponr ();
}
  else
{
@@ -1859,6 +1860,7 @@ main (int argc, char *argv[])
  partno = grub_dev->disk->partition
? grub_dev->disk->partition->number + 1 : 0;
  dev = grub_util_get_os_disk (grub_devices[0]);
+ grub_set_install_backup_ponr ();
  grub_install_register_ieee1275 (0, dev,
  partno, relpath);
}
-- 
2.37.1


___
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

2022-08-29 Thread Peter Zijlstra
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?

---

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