From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1pL9c3-0007X7-Gg for mharc-grub-devel@gnu.org; Thu, 26 Jan 2023 16:16:51 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pL9bl-0007WU-GF for grub-devel@gnu.org; Thu, 26 Jan 2023 16:16:37 -0500 Received: from mail-qt1-x832.google.com ([2607:f8b0:4864:20::832]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pL9bh-0007WP-MY for grub-devel@gnu.org; Thu, 26 Jan 2023 16:16:32 -0500 Received: by mail-qt1-x832.google.com with SMTP id o5so2432163qtr.11 for ; Thu, 26 Jan 2023 13:16:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=efficientek-com.20210112.gappssmtp.com; s=20210112; h=content-transfer-encoding:mime-version:reply-to:references :in-reply-to:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to; bh=OfdS3J5aFuj3CfllBloMuQkJcSlBHqvA5u0FyZC/w4s=; b=dVBfkSULVfHbbrcsHgGoB9qwwp0K0UgQwUaHdRYs/yABtK7+wPZJWBWj4upcdboy46 TalzqDQZzWvvr1NvchzLE9lqNvIoWZ0OiesBhTJfv/SptGcv+8B28Q4tiqwiKo9mxbVo 8Eo8pMuXCZR6QhHhzHM2i5eYWrlB9WjEXNeIxtaYEfjUkXLNwSpMqf97l164UzNvDDyJ waOEc07ePuBa3K3by2jT+xZzElXti+wsRS1bZnmV/NpU9bBU/JE4KIgL76d0YSOkdtYj pdfuTsK745gdfdBmoL0jfi97wQjI780zXhNjtAr05T3YENotKjPWqGuT5FNS+7TImndu VFGg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:reply-to:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=OfdS3J5aFuj3CfllBloMuQkJcSlBHqvA5u0FyZC/w4s=; b=CJzK+8A2la7Z0k2nlTBWp2F+RdOgjZh+RiGrmIdQT+WDPQNVd4/DAcJirFEh2RotLE S4Inc0SaoEhwlqsteT6IMTb7z5PMj6b4IX/3AJUCOZ4Ws4eEhTKcyPZN6iSPigw19GmD 9mvSNsFi3i1PonFnewym5dHJ5et7Te+RvSr0pZA4l0UuvnDAJ3ccgAmyEFHNGAS5LVmy z6qH/UB6C2LjF9H25w86Q4pcPqbjugKuMa1NI67eChOscSt/7/QUNtgzaUI10uNuJmKO Ib3A5yiHwIaUYrypmD36F1/UL5FljMF/IzoplcGaJA/Vdf9KD1u3uUV6bSznQH7p7sj/ prrQ== X-Gm-Message-State: AFqh2krCV/RYRrnSauXlOXqmlm1YUsjmnsJ9Pp9QNHofqHbQy2Txikyl LnJzuyWCP4eYYRPLzO4vR3v9fCdV10+g22jJ X-Google-Smtp-Source: AMrXdXsRj1hLA/ekRbLymPRb94PcW6kpHxWClDooYQsnSGukWucAuOUI1ElwakkZMbCaMnoi1qIztA== X-Received: by 2002:ac8:6b02:0:b0:3a9:2478:2d70 with SMTP id w2-20020ac86b02000000b003a924782d70mr52083352qts.24.1674767787178; Thu, 26 Jan 2023 13:16:27 -0800 (PST) Received: from crass-HP-ZBook-15-G2 ([37.218.244.251]) by smtp.gmail.com with ESMTPSA id cc5-20020a05622a410500b003b6953fbb8fsm1476158qtb.32.2023.01.26.13.16.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 26 Jan 2023 13:16:26 -0800 (PST) Date: Thu, 26 Jan 2023 15:16:07 -0600 From: Glenn Washburn To: Peter Zijlstra Cc: Daniel Kiper , grub-devel@gnu.org Subject: Re: [PATCH v2 2/3] term/serial: Add support for PCI serial devices Message-ID: <20230126151607.078dbcfe@crass-HP-ZBook-15-G2> In-Reply-To: References: <20220826110142.966628595@alderlake.programming.kicks-ass.net> <20220826111358.334758397@alderlake.programming.kicks-ass.net> <20221221125902.jw5kjwmxupeozt3b@tomti.i.net-space.pl> Reply-To: development@efficientek.com X-Mailer: Claws Mail 4.1.0 (GTK 3.24.34; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Received-SPF: pass client-ip=2607:f8b0:4864:20::832; envelope-from=development@efficientek.com; helo=mail-qt1-x832.google.com X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 26 Jan 2023 21:16:46 -0000 On Thu, 26 Jan 2023 10:37:35 +0100 Peter Zijlstra wrote: > On Wed, Dec 21, 2022 at 01:59:02PM +0100, Daniel Kiper wrote: > > Sorry for late reply... > > > > May I ask you to send the patches using "git send-email"? > > I'll try -- I'm one of the few quilt holdouts in Linux-land so I don't > have much experience with it. > > > > Please add a blurb how to use this driver to the docs/grub.texi > > file. > > > > > GRUB_TERMINAL="serial console" > > > > > > Signed-off-by: Peter Zijlstra (Intel) > > > --- > > > > > > Index: grub/grub-core/Makefile.core.def > > > =================================================================== > > > --- grub.orig/grub-core/Makefile.core.def > > > +++ grub/grub-core/Makefile.core.def > > > @@ -2047,6 +2047,7 @@ module = { > > > ieee1275 = term/ieee1275/serial.c; > > > mips_arc = term/arc/serial.c; > > > efi = term/efi/serial.c; > > > + pci = term/pci/serial.c; > > > > I think this should be "x86 = term/pci/serial.c". > > I thought the whole purpose of Glenn's patch was to allow doing the > 'pci =' thing, so that all platforms that support PCI get to have the > benefit. > > But if you really want I can make it 'x86 =' ofcourse. To throw in my two cents, I believe this is correct the way it is with 'pci =' as Peter mentioned. Am I also missing something here? Glenn > > > +static int > > > +find_pciserial (grub_pci_device_t dev, grub_pci_id_t pciid, void > > > *data) > > > > grub_pci_id_t pciid __attribute__ ((unused)) > > > > > +{ > > > + grub_pci_address_t cmd_addr, class_addr, bar_addr; > > > + struct grub_serial_port *port; > > > + grub_uint32_t class, bar; > > > + grub_uint16_t cmdreg; > > > + int *port_num = data; > > > + grub_err_t err; > > > + > > > + (void)pciid; > > > > ... and please drop this... > > > > > > +error: > > > > s/error/fail/ > > > > And please add a space before label... > > All done, except the Makefile thing, find the updated version below. > I'll attempt to git-send-email both patches once we agree on the > Makefile hunk. > > > --- > Subject: term/serial: Add support for PCI serial devices > > 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- 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 --speed=115200 > pci_00:16.3" GRUB_TERMINAL="serial console" > > Signed-off-by: Peter Zijlstra (Intel) > Tested-by: Glenn Washburn > Reviewed-by: Glenn Washburn > --- > > Index: grub/grub-core/Makefile.core.def > =================================================================== > --- grub.orig/grub-core/Makefile.core.def > +++ grub/grub-core/Makefile.core.def > @@ -2057,6 +2057,7 @@ module = { > ieee1275 = term/ieee1275/serial.c; > mips_arc = term/arc/serial.c; > efi = term/efi/serial.c; > + pci = term/pci/serial.c; > > enable = terminfomodule; > enable = ieee1275; > Index: grub/grub-core/term/pci/serial.c > =================================================================== > --- /dev/null > +++ grub/grub-core/term/pci/serial.c > @@ -0,0 +1,93 @@ > +/* > + * GRUB -- GRand Unified Bootloader > + * Copyright (C) 2022 Free Software Foundation, Inc. > + * > + * GRUB is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as > published by > + * the Free Software Foundation, either version 3 of the License, or > + * (at your option) any later version. > + * > + * GRUB is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with GRUB. If not, see . > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +static int > +find_pciserial (grub_pci_device_t dev, > + grub_pci_id_t pciid __attribute__ ((unused)), > + void *data __attribute__ ((unused))) > +{ > + grub_pci_address_t cmd_addr, class_addr, bar_addr; > + struct grub_serial_port *port; > + grub_uint32_t class, bar; > + grub_uint16_t cmdreg; > + grub_err_t err; > + > + cmd_addr = grub_pci_make_address (dev, GRUB_PCI_REG_COMMAND); > + cmdreg = grub_pci_read (cmd_addr); > + > + class_addr = grub_pci_make_address (dev, GRUB_PCI_REG_REVISION); > + class = grub_pci_read (class_addr); > + > + bar_addr = grub_pci_make_address (dev, GRUB_PCI_REG_ADDRESS_REG0); > + bar = grub_pci_read (bar_addr); > + > + /* 16550 compatible MODEM or SERIAL */ > + if (((class >> 16) != GRUB_PCI_CLASS_COMMUNICATION_MODEM && > + (class >> 16) != GRUB_PCI_CLASS_COMMUNICATION_SERIAL) || > + ((class >> 8) & 0xff) != GRUB_PCI_SERIAL_16550_COMPATIBLE) > + return 0; > + > + if ((bar & GRUB_PCI_ADDR_SPACE_MASK) != GRUB_PCI_ADDR_SPACE_IO) > + return 0; > + > + port = grub_zalloc (sizeof (*port)); > + if (port == NULL) > + return 0; > + > + 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 fail; > + > + grub_pci_write (cmd_addr, cmdreg | GRUB_PCI_COMMAND_IO_ENABLED); > + > + port->driver = &grub_ns8250_driver; > + port->port = bar & GRUB_PCI_ADDR_IO_MASK; > + err = grub_serial_config_defaults (port); > + if (err != GRUB_ERR_NONE) > + { > + grub_print_error (); > + goto fail; > + } > + > + err = grub_serial_register (port); > + if (err != GRUB_ERR_NONE) > + goto fail; > + > + return 0; > + > + fail: > + grub_free (port->name); > + grub_free (port); > + return 0; > +} > + > +void > +grub_pciserial_init (void) > +{ > + grub_pci_iterate (find_pciserial, NULL); > +} > Index: grub/grub-core/term/serial.c > =================================================================== > --- grub.orig/grub-core/term/serial.c > +++ grub/grub-core/term/serial.c > @@ -505,6 +505,9 @@ GRUB_MOD_INIT(serial) > #ifdef GRUB_MACHINE_ARC > grub_arcserial_init (); > #endif > +#ifdef GRUB_HAS_PCI > + grub_pciserial_init (); > +#endif > } > > GRUB_MOD_FINI(serial) > Index: grub/include/grub/pci.h > =================================================================== > --- grub.orig/include/grub/pci.h > +++ grub/include/grub/pci.h > @@ -80,8 +80,13 @@ > > #define GRUB_PCI_STATUS_DEVSEL_TIMING_SHIFT 9 > #define GRUB_PCI_STATUS_DEVSEL_TIMING_MASK 0x0600 > + > #define GRUB_PCI_CLASS_SUBCLASS_VGA 0x0300 > #define GRUB_PCI_CLASS_SUBCLASS_USB 0x0c03 > +#define GRUB_PCI_CLASS_COMMUNICATION_SERIAL 0x0700 > +#define GRUB_PCI_CLASS_COMMUNICATION_MODEM 0x0703 > + > +#define GRUB_PCI_SERIAL_16550_COMPATIBLE 0x02 > > #ifndef ASM_FILE > > Index: grub/include/grub/serial.h > =================================================================== > --- grub.orig/include/grub/serial.h > +++ grub/include/grub/serial.h > @@ -210,6 +210,10 @@ grub_arcserial_init (void); > struct grub_serial_port *grub_arcserial_add_port (const char *path); > #endif > > +#ifdef GRUB_HAS_PCI > +void grub_pciserial_init (void); > +#endif > + > struct grub_serial_port *grub_serial_find (const char *name); > extern struct grub_serial_driver grub_ns8250_driver; > void EXPORT_FUNC(grub_serial_unregister_driver) (struct > grub_serial_driver *driver); Index: grub/docs/grub.texi > =================================================================== > --- grub.orig/docs/grub.texi > +++ grub/docs/grub.texi > @@ -1386,6 +1386,7 @@ separated by spaces. > Valid terminal input names depend on the platform, but may include > @samp{console} (native platform console), @samp{serial} (serial > terminal), @samp{serial_} (serial terminal with explicit port > selection), +@samp{pci_:.} (PCI serial with > explicit location), @samp{at_keyboard} (PC AT keyboard), or > @samp{usb_keyboard} (USB keyboard using the HID Boot Protocol, for > cases where the firmware does not handle this). > @@ -1399,6 +1400,7 @@ separated by spaces. > Valid terminal output names depend on the platform, but may include > @samp{console} (native platform console), @samp{serial} (serial > terminal), @samp{serial_} (serial terminal with explicit port > selection), +@samp{pci_:.} (PCI serial with > explicit location), @samp{gfxterm} (graphics-mode output), > @samp{vga_text} (VGA text output), @samp{mda_text} (MDA text output), > @samp{morse} (Morse-coding using system beeper) or @samp{spkmodem} > (simple data protocol using system speaker). @@ -4165,7 +4167,7 @@ > Commands usable anywhere in the menu and @node serial > @subsection serial > > -@deffn Command serial [@option{--unit=unit}] [@option{--port=port}] > [@option{--speed=speed}] [@option{--word=word}] > [@option{--parity=parity}] [@option{--stop=stop}] +@deffn Command > serial [@option{--unit=unit}] [@option{--port=port}] > [@option{--speed=speed}] [@option{--word=word}] > [@option{--parity=parity}] [@option{--stop=stop}] [@option{name}] > Initialize a serial device. @var{unit} is a number in the range 0-3 > specifying which serial port to use; default is 0, which corresponds > to the port often called COM1. @@ -4197,6 +4199,19 @@ If passed no > @var{unit} nor @var{port}, the system default serial port and its > configuration. If this information is not available, it will default > to @var{unit} 0. +If used @var{name}, a machine and driver specific > name given during probing, +will be used to indicate which serial > port to use. Possible forms include: +@itemize @bullet +@item > +@samp{serial_} as an alternative to --port +@item > +@samp{usbN} the N'th USB serial detected > +@item > +@samp{efiN} the N'th EFI serial detected > +@item > +@samp{pci_:.} PCI serial at the specified location > +@end itemize > + > The serial port is not used as a communication channel unless the > @command{terminal_input} or @command{terminal_output} command is used > (@pxref{terminal_input}, @pxref{terminal_output}). > @@ -4205,6 +4220,7 @@ Examples: > @example > serial --port=3f8 --speed=9600 > serial --port=mmio,fefb0000.l --speed=115200 > +serial --speed=115200 pci_00:16.3 > @end example > > See also @ref{Serial terminal}.