From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 38948941485 for ; Wed, 28 Feb 2024 10:12:34 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=efjz1jRQ4V6G3X+eBbMzh4cKHAd68g4+EEXCAU0hQU4=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:Subject:To:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1709115152; v=1; b=IpFSCk76EJfM19/lWxSfh3x+M0Cnahf7JcTXJfiLaAHWIuMbxJoaUGEOUTgWoNgix4LExWv0 x+Uaa1l1tmoMBWSyOBM8exNX4RWDxBWCoMTWPaMK3VOS2z92fYn2iO8ZOXfMJgcyR+BJ6tn47ta dPr7nKVZ0/4m/dTT23c2IAiQ= X-Received: by 127.0.0.2 with SMTP id Y0L5YY7687511xpTKhqE8prl; Wed, 28 Feb 2024 02:12:32 -0800 X-Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web11.10046.1709115152071022297 for ; Wed, 28 Feb 2024 02:12:32 -0800 X-Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-68-mbado3fLMfuA1oufxD_ojg-1; Wed, 28 Feb 2024 05:12:29 -0500 X-MC-Unique: mbado3fLMfuA1oufxD_ojg-1 X-Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 8F2D3811E79; Wed, 28 Feb 2024 10:12:29 +0000 (UTC) X-Received: from [10.39.193.212] (unknown [10.39.193.212]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 00C6DC185C0; Wed, 28 Feb 2024 10:12:28 +0000 (UTC) Message-ID: <03bbfff1-b6de-3468-9f66-f8d513a1c9e2@redhat.com> Date: Wed, 28 Feb 2024 11:12:27 +0100 MIME-Version: 1.0 Subject: Re: [edk2-devel] [PATCH 1/1] MdeModulePkg: Load Serial driver earlier in DXE To: devel@edk2.groups.io, mateusz.albecki@intel.com References: <643d0b1f-1f90-97ea-721d-3462f74d14a1@redhat.com> <21276.1709054156537513390@groups.io> From: "Laszlo Ersek" In-Reply-To: <21276.1709054156537513390@groups.io> X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.8 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,lersek@redhat.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: O7FBIiJBxJLo8WmNsLweL6Kox7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=IpFSCk76; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=redhat.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On 2/27/24 18:15, Albecki, Mateusz wrote: > Is the idea to refactor PciSioSerialDxe to extract functions that access > the HW and wrap it in the SerialPortLib instance? That solution would > still save us some maintenance cost. However exploring the idea further > I see following problems: >=20 > 1. Relying on driver binding produces a fairly nice flow: platform > driver initializes enough platform HW for UART to work -> platform > driver calls ConnectController -> driver initializes UART. With > SerialDxe a depex would have to be injected through our instance of > SerialPortLib to stop the SerialDxe dispatch until platform driver made > the platform ready. > 2. I am wondering how it would work in case we want to allow dynamic > configuration of debug port(basically selecting which UART controller > would be used). With current solution we can select which one(or which > ones) will be used by simply installing and connecting corresponding > handles. With library solution I guess library would have to locate some > protocols(possibly the same PCI_IO and DEVICE_PATH) that were installed > by platform driver. It would also need to install notify on those > protocols installation in case platform wants to add more uarts later on > in the boot flow. I think these requirements are *way* out of scope, and too clever, for producing debug output. I'm now tempted to think that you are actually best served by your existent separate driver. SerialDxe and SerialPortLib are ill-suited if your system has multiple serial ports, not to mention if you want different configurations from boot to boot. I think I'm permitted to disagree with a proposed patch without having to counter-propose an alternative myself (i.e., the burden is on you, to find an alternative). But, I'll brainstorm one more: a. I'll assume that you store the debug port config in some non-volatile UEFI variable. b. In the PEI phase, have a PEIM turn that variable into a GUID HOB. (This is doable because PEI can have read-only variable access.) Or produce that HOB in some other way. c. In the early platform DXE driver (=3D debug driver), read the HOB, and publish a single instance of a new gEdkiiLpssUartDebugProtocolGuid. d. The protocol should have a single member function, WriteToAllConfiguredDebugPorts(). The debug driver should replicate the message that is passed to this API to all ports enabled in the HOB. e. Introduce a new DebugLib instance that, for DXE_DRIVER modules, has a DEPEX on gEdkiiLpssUartDebugProtocolGuid: [Depex.common.DXE_DRIVER] gEdkiiLpssUartDebugProtocolGuid f. The debug ports should never appear in the system as EFI handles (i.e., neither SerialIo nor PciIo nor DevicePath protocols -- no handles at all). g. The debug driver should use PciLib for configuring and accessing the ports. Configuration includes any necessary "platform configuration" too. h. If there is any LPSS UART access logic that's worth sharing -- for maintenance reasons -- between PciSioSerialDxe and the debug driver, then that should be extracted to new library class, and *two* library instances (same directory, though). Both instances would nearly be identical, but they'd have to *internally* abstract away PCI access. The DXE instance (underlying the debug driver) would use PciLib, the UEFI instance (underlying PciSioSerialDxe) would use PciIo. The code that interacted with the LPSS UARTs would be common though. i. The debug driver could register protocol notifies for the HII protocols (which would become available much later), and once those appeared, the debug driver could install HII forms, for managing the non-volatile UEFI variable that configures the debug ports. (This could perhaps be used for truly dynamic configuration, i.e., without having to reboot; although personally I'd steer very clear of that avenue.) As lower-hanging fruit, you could probably just implement step (h) in isolation, and rebase both your existent driver, and PciSioSerialDxe, onto that new library class (and its two library instances). Other opinions are very welcome, of course! Personally, I'm out of cycles on this topic. Laszlo > 3. We still end up with 2 UART drivers in flash since PciSioSerialDxe is > needed for PCI UARTs. >=20 > I also think this solution is comparable in effort to refactoring the > PciSioSerialDxe so that it doesn't use driver binding when used as a DXE > driver. In this solution driver would have one .c file for code with > driver binding and one .c file for code when everything is done in entry > point, it would still use DEVICE_PATH and PCI_IO/SIO. While I still > think using the driver as is in DXE is best for us, in case that > solution gets blocked I would like to understand if everyone would be ok > with such refactoring. >=20 > Thanks, > Mateusz >=20 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#116111): https://edk2.groups.io/g/devel/message/116111 Mute This Topic: https://groups.io/mt/104469297/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-