From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1oT7HG-0000Hq-95 for mharc-grub-devel@gnu.org; Tue, 30 Aug 2022 15:52:02 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:41066) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oT7H6-0000H2-68 for grub-devel@gnu.org; Tue, 30 Aug 2022 15:51:54 -0400 Received: from mail-vk1-xa2d.google.com ([2607:f8b0:4864:20::a2d]:33523) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1oT7H1-0008Nv-VE for grub-devel@gnu.org; Tue, 30 Aug 2022 15:51:50 -0400 Received: by mail-vk1-xa2d.google.com with SMTP id u11so3108173vkk.0 for ; Tue, 30 Aug 2022 12:51:46 -0700 (PDT) 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; bh=gibfFq6EKbjLwl9VEm8xKpXo5QpDc7AGBlbSKxJeAuU=; b=OlMDWhkjm4EP1m33enzkbYLlLKEm63yKbz8kSgPyCCEZmln4uhKOWX+opB2HjBEnVs ResYuiszT/JwmKtCXlQcP33xWh/R/8MkQv3D0+sfdh0Ra3wz91v2RIsxKvVspdJTSH92 xppGANHrk1ZhoyNJnZA3syMPziYd9L9Yg6mDwZq1t8JL6x4jwnIKbeJAPZLIH5EOQSOp dDoEZg2hkg35xfekYgOfpHEajHbMw/blxH7XQjOKqrRh1PFTE8DMZSmViD3AS+QEjGcY jNoAShPxyHu9mW39QU0J9I9QnoGPEifD2zZq7bZfe3tKhe5XP8cz7DiIfZDdMBDA/ABx cfZQ== 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; bh=gibfFq6EKbjLwl9VEm8xKpXo5QpDc7AGBlbSKxJeAuU=; b=BE47VAceqDlDzoFsV+8Dil7YXTtabnL0ptzl9z3VQBItRPwjDcCl+5DDypirITLWoA 1fAwV7A4b6r1enFle4Ku1IjsYIDCX5tJtSNyhEd/OR8kuxyzIUDVaStuLwCb0MENXzDX EZ2i2B34lKPe7yOQYK2W0u5I+Gzs//0ZNc2koSNjRgQfJvTFni3fVKdH4GvzNfV4WdTg qQVzxxJPRONhRVzezJyUTehfkYlx+/UAA4H3BGS4iCdkDeMRlz3Drvz1Rg3ckJNSfAqd IIynOqPtdUdFiFnLRxoE0BzkCoDXn8q+KaQpf0YUUwO+mqoQpComIWzVG+GY294Dy68l Wr9w== X-Gm-Message-State: ACgBeo3qh/ScegQ3P6swUkMJk1mELgK6hbpkuLaDdhWAJiu3chwiwAc+ VAKfYW5vWNl1btlHQGe4XHOpNw== X-Google-Smtp-Source: AA6agR5qqlR4ED6JhaARkkhWOsBLgmeV4nUoq3woZFPMJKDrwqq5EkBiD09HplzrX3zBzehoUMorWA== X-Received: by 2002:a1f:fe4f:0:b0:394:9b89:f9fb with SMTP id l76-20020a1ffe4f000000b003949b89f9fbmr1639325vki.2.1661889106099; Tue, 30 Aug 2022 12:51:46 -0700 (PDT) Received: from crass-HP-ZBook-15-G2 ([37.218.244.249]) by smtp.gmail.com with ESMTPSA id a123-20020a1fca81000000b003768e235513sm2482597vkg.0.2022.08.30.12.51.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 30 Aug 2022 12:51:45 -0700 (PDT) Date: Tue, 30 Aug 2022 14:51:39 -0500 From: Glenn Washburn To: Peter Zijlstra Cc: grub-devel@gnu.org, dkiper@net-space.pl Subject: Re: [PATCH v2 2/3] term/serial: Add support for PCI serial devices Message-ID: <20220830145139.767fba4b@crass-HP-ZBook-15-G2> In-Reply-To: References: <20220826110142.966628595@alderlake.programming.kicks-ass.net> <20220826111358.334758397@alderlake.programming.kicks-ass.net> <20220826133225.2471c2aa@crass-HP-ZBook-15-G2> 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::a2d; envelope-from=development@efficientek.com; helo=mail-vk1-xa2d.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, T_SCC_BODY_TEXT_LINE=-0.01 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: Tue, 30 Aug 2022 19:51:57 -0000 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 > > peterz@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: 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); > }