On Mon, 29 Aug 2022 12:53:54 +0200
Peter Zijlstra <pet...@infradead.org> 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-
> > > <TAbort- <MAbort- >SERR- <PERR- INTx-
> > > 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: 0000000000000000 Data: 0000
> > > 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
