From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::144; helo=mail-it1-x144.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it1-x144.google.com (mail-it1-x144.google.com [IPv6:2607:f8b0:4864:20::144]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 2695A2116DA05 for ; Fri, 12 Oct 2018 08:33:24 -0700 (PDT) Received: by mail-it1-x144.google.com with SMTP id k206-v6so13237121ite.0 for ; Fri, 12 Oct 2018 08:33:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=fYMZgigmW9ws2rr9YdhuV+WPqvqE28iyN/oFDcRVhIQ=; b=H0N09XdCd6j6rjQ4tNnqllo+E7tIhI+1MnLpHSwKbd4P22UUh/yGgFrDsS0rGeKt8f Vm5+lwZUzT87z5BP5XKq5SMmbbP5eudU+6XDYk5gFlhxxIPcuEYQip3aZaZMRKYiZLQW b7EuwC6L7guDsxKvt/2uRIeMZWGvM+JCqh2WQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=fYMZgigmW9ws2rr9YdhuV+WPqvqE28iyN/oFDcRVhIQ=; b=h0j/vn9lo0Yms5fR3F/2L7xBl285pc6cuqoR6Me+aOkaMe7vZHoKAeujr4ywey4VoS n4+jXGcwIF26FFpZfI3D0RRIjy4nH+vAnEsHOSLgvL8nnEfVijxaTmSSGi+z74Ai51Um 8OCt43st61mSyTJAHfTD4ymoDFLz9OZyTBOsSHrolU1/SSGpEd7KEHpmYoj7bv4D7mQM GCDtsYj5Sfllu0w64nA5OVU1RHX3w6qklZD/SUr3mz1W07BhZ/AF6OdOjjlTtxEw4qy9 6UaQyfY4o7X/y/4gdvasQzu8NPdQgeX2mGWEfn7Ow/VgeZ6V2t2dj7WtU7RyDiMBEAIN AOng== X-Gm-Message-State: ABuFfoiYiiHJpAQKAFMdzCHsfzZYIjQ3MwmMSNxUmsFQ1Y5r9DskSnM3 eqEzmTDRdamKb1CWZvfrqXyIJWcetZ9CorsLhnAhuw== X-Google-Smtp-Source: ACcGV600mCvhpIWZmyiuIJFc4rYJE1rbBziefDhA3mnZ/4HTUG39KHQBycCupEQpyCxfjTrNo2m984TqMT/CKsrww/o= X-Received: by 2002:a24:e08f:: with SMTP id c137-v6mr5376386ith.71.1539358403767; Fri, 12 Oct 2018 08:33:23 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:5910:0:0:0:0:0 with HTTP; Fri, 12 Oct 2018 08:33:23 -0700 (PDT) In-Reply-To: References: <20181012144009.48732-1-sami.mujawar@arm.com> <20181012144009.48732-2-sami.mujawar@arm.com> From: Ard Biesheuvel Date: Fri, 12 Oct 2018 17:33:23 +0200 Message-ID: To: Sami Mujawar Cc: "edk2-devel@lists.01.org" , Leif Lindholm , Ruiyu Ni , Evan Lloyd , Matteo Carlini , Stephanie Hughes-Fitt , nd Subject: Re: [PATCH v1 1/6] PcAtChipsetPkg: Add MMIO Support to SerialIo Lib X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Oct 2018 15:33:25 -0000 Content-Type: text/plain; charset="UTF-8" On 12 October 2018 at 17:06, Sami Mujawar wrote: > Hi Ard, > > I believe you are referring to MdeModulePkg\Library\BaseSerialPortLib16550\BaseSerialPortLib16550.inf > I have tried using this Serial Port Library. However, this has a dependency on PciLib which is difficult to resolve as we use the Serial port early in the boot. > Please do let me know if you know a workaround to solve this issue. > Yes. Just keep PcdSerialPciDeviceInfo at its default of 0xff, and the PCI dependencies in that driver are essentially circumvented, so you can use any PciLib implementation as a dummy (or create a new one that ASSERT()s in all functions if you prefer) > Regards, > > Sami Mujawar > > -----Original Message----- > From: Ard Biesheuvel > Sent: 12 October 2018 03:50 PM > To: Sami Mujawar > Cc: edk2-devel@lists.01.org; Leif Lindholm ; Ruiyu Ni ; Evan Lloyd ; Matteo Carlini ; Stephanie Hughes-Fitt ; nd > Subject: Re: [PATCH v1 1/6] PcAtChipsetPkg: Add MMIO Support to SerialIo Lib > > On 12 October 2018 at 16:40, Sami Mujawar wrote: >> Some virtual machine managers like kvmtool emulate the PC AT Serial >> Port UART in the MMIO space so that architectures that do not support >> I/O Mapped I/O can use the UART. >> >> This patch adds MMIO support to the PC AT SerialPortLib. >> >> PcdSerialUseMmio is used to select I/O or MMIO support. >> If PcdSerialUseMmio is >> TRUE - The value is read/written from MMIO space. >> FALSE - The value is read/written from I/O space. >> The Default is I/O space. >> > > IIUC kvmtool exposes a 8250, and our 8250 driver already implements support for MMIO mapped registers. Can't you use that instead? > >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Sami Mujawar >> --- >> The changes can be seen at >> https://github.com/samimujawar/edk2/commit/84f908387d2031def5d374759e7 >> ad4cf90786c91 >> >> Notes: >> v1: >> - Add support to read/write from UART registers using MMIO access >> [SAMI] >> >> PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf | 4 + >> PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c | 98 >> ++++++++++++++++---- >> 2 files changed, 84 insertions(+), 18 deletions(-) >> >> diff --git a/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf >> b/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf >> index >> 959d6e27c9812d201d44d9249070ab7758cbfe00..d0689793040fd930701b02dae51f >> f59ea16a10c4 100644 >> --- a/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf >> +++ b/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf >> @@ -2,6 +2,7 @@ >> # Library instance for SerialIo library class >> # >> # Copyright (c) 2006 - 2014, Intel Corporation. All rights >> reserved.
>> +# Copyright (c) 2018, ARM Limited. All rights reserved.
>> # This program and the accompanying materials # are licensed and >> made available under the terms and conditions of the BSD License # >> which accompanies this distribution. The full text of the license may >> be found at @@ -23,6 +24,7 @@ [Defines] >> >> [Packages] >> MdePkg/MdePkg.dec >> + MdeModulePkg/MdeModulePkg.dec >> >> [LibraryClasses] >> BaseLib >> @@ -31,3 +33,5 @@ [LibraryClasses] >> [Sources] >> SerialPortLib.c >> >> +[FixedPcd] >> + gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio ## CONSUMES >> diff --git a/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c >> b/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c >> index >> d1a1c6a03facad09e781b5605e22a24e5f51c618..898e5f957aae998624189c6b5389 >> 12da0439dfe8 100644 >> --- a/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c >> +++ b/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c >> @@ -2,6 +2,8 @@ >> UART Serial Port library functions >> >> Copyright (c) 2006 - 2018, Intel Corporation. All rights >> reserved.
>> + Copyright (c) 2018, ARM Limited. All rights reserved.
>> + >> This program and the accompanying materials >> are licensed and made available under the terms and conditions of the BSD License >> which accompanies this distribution. The full text of the license may be found at >> @@ -57,6 +59,66 @@ UINT8 gParity = 0; >> UINT8 gBreakSet = 0; >> >> /** >> + Reads an 8-bit value from the serial port. >> + >> + This function checks PcdSerialUseMmio to determine if I/O mapped >> + IO or Memory Mapped IO operations must be performed on the serial >> + port. It then uses the appropriate interface to read the Value from >> + the serial port. >> + >> + If PcdSerialUseMmio is TRUE, then the value is read from MMIO space. >> + If PcdSerialUseMmio is FALSE, then the value is read from I/O space. >> + >> + @param Address The UART register to read from. >> + >> + @return The value read from the serial port. >> + >> +**/ >> +STATIC >> +UINT8 >> +SerialPortRead8 ( >> + IN UINTN Address >> +) >> +{ >> + if (FixedPcdGetBool (PcdSerialUseMmio)) { >> + return MmioRead8 (Address); >> + } >> + >> + return IoRead8 (Address); >> +} >> + >> +/** >> + Writes an 8-bit value to the serial port. >> + >> + This function checks PcdSerialUseMmio to determine if I/O mapped >> + IO or Memory Mapped IO operations must be performed on the serial >> + port. It then uses the appropriate interface to write the Value to >> + the serial port. >> + >> + If PcdSerialUseMmio is TRUE, then the value is written to MMIO space. >> + If PcdSerialUseMmio is FALSE, then the value is written to I/O space. >> + >> + @param Address The UART register to write. >> + @param Value The value to write to the I/O port. >> + >> + @return The value written to the serial port. >> + >> +**/ >> +STATIC >> +UINT8 >> +SerialPortWrite8 ( >> + IN UINTN Address, >> + IN UINT8 Value >> +) >> +{ >> + if (FixedPcdGetBool (PcdSerialUseMmio)) { >> + return MmioWrite8 (Address, Value); >> + } >> + >> + return IoWrite8 (Address, Value); >> +} >> + >> +/** >> Initialize the serial device hardware. >> >> If no initialization is required, then return RETURN_SUCCESS. >> @@ -91,19 +153,19 @@ SerialPortInitialize ( >> // Set communications format >> // >> OutputData = (UINT8) ((DLAB << 7) | (gBreakSet << 6) | (gParity << >> 3) | (gStop << 2) | Data); >> - IoWrite8 (gUartBase + LCR_OFFSET, OutputData); >> + SerialPortWrite8 (gUartBase + LCR_OFFSET, OutputData); >> >> // >> // Configure baud rate >> // >> - IoWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >> 8)); >> - IoWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor & 0xff)); >> + SerialPortWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >> >> + 8)); >> + SerialPortWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor & >> + 0xff)); >> >> // >> // Switch back to bank 0 >> // >> OutputData = (UINT8) ( (gBreakSet << 6) | (gParity << 3) | (gStop >> << 2) | Data); >> - IoWrite8 (gUartBase + LCR_OFFSET, OutputData); >> + SerialPortWrite8 (gUartBase + LCR_OFFSET, OutputData); >> >> return RETURN_SUCCESS; >> } >> @@ -148,9 +210,9 @@ SerialPortWrite ( >> // Wait for the serail port to be ready. >> // >> do { >> - Data = IoRead8 ((UINT16) gUartBase + LSR_OFFSET); >> + Data = SerialPortRead8 ((UINT16) gUartBase + LSR_OFFSET); >> } while ((Data & LSR_TXRDY) == 0); >> - IoWrite8 ((UINT16) gUartBase, *Buffer++); >> + SerialPortWrite8 ((UINT16) gUartBase, *Buffer++); >> } >> >> return Result; >> @@ -189,10 +251,10 @@ SerialPortRead ( >> // Wait for the serail port to be ready. >> // >> do { >> - Data = IoRead8 ((UINT16) gUartBase + LSR_OFFSET); >> + Data = SerialPortRead8 ((UINT16) gUartBase + LSR_OFFSET); >> } while ((Data & LSR_RXDA) == 0); >> >> - *Buffer++ = IoRead8 ((UINT16) gUartBase); >> + *Buffer++ = SerialPortRead8 ((UINT16) gUartBase); >> } >> >> return Result; >> @@ -220,7 +282,7 @@ SerialPortPoll ( >> // >> // Read the serial port status. >> // >> - Data = IoRead8 ((UINT16) gUartBase + LSR_OFFSET); >> + Data = SerialPortRead8 ((UINT16) gUartBase + LSR_OFFSET); >> >> return (BOOLEAN) ((Data & LSR_RXDA) != 0); } @@ -253,7 +315,7 @@ >> SerialPortSetControl ( >> // >> // Read the Modem Control Register. >> // >> - Mcr = IoRead8 ((UINT16) gUartBase + MCR_OFFSET); >> + Mcr = SerialPortRead8 ((UINT16) gUartBase + MCR_OFFSET); >> Mcr &= (~(MCR_DTRC | MCR_RTS)); >> >> if ((Control & EFI_SERIAL_DATA_TERMINAL_READY) == >> EFI_SERIAL_DATA_TERMINAL_READY) { @@ -267,7 +329,7 @@ SerialPortSetControl ( >> // >> // Write the Modem Control Register. >> // >> - IoWrite8 ((UINT16) gUartBase + MCR_OFFSET, Mcr); >> + SerialPortWrite8 ((UINT16) gUartBase + MCR_OFFSET, Mcr); >> >> return RETURN_SUCCESS; >> } >> @@ -297,7 +359,7 @@ SerialPortGetControl ( >> // >> // Read the Modem Status Register. >> // >> - Msr = IoRead8 ((UINT16) gUartBase + MSR_OFFSET); >> + Msr = SerialPortRead8 ((UINT16) gUartBase + MSR_OFFSET); >> >> if ((Msr & MSR_CTS) == MSR_CTS) { >> *Control |= EFI_SERIAL_CLEAR_TO_SEND; @@ -318,7 +380,7 @@ >> SerialPortGetControl ( >> // >> // Read the Modem Control Register. >> // >> - Mcr = IoRead8 ((UINT16) gUartBase + MCR_OFFSET); >> + Mcr = SerialPortRead8 ((UINT16) gUartBase + MCR_OFFSET); >> >> if ((Mcr & MCR_DTRC) == MCR_DTRC) { >> *Control |= EFI_SERIAL_DATA_TERMINAL_READY; @@ -331,7 +393,7 @@ >> SerialPortGetControl ( >> // >> // Read the Line Status Register. >> // >> - Lsr = IoRead8 ((UINT16) gUartBase + LSR_OFFSET); >> + Lsr = SerialPortRead8 ((UINT16) gUartBase + LSR_OFFSET); >> >> if ((Lsr & LSR_TXRDY) == LSR_TXRDY) { >> *Control |= EFI_SERIAL_OUTPUT_BUFFER_EMPTY; @@ -470,19 +532,19 @@ >> SerialPortSetAttributes ( >> // Set communications format >> // >> OutputData = (UINT8) ((DLAB << 7) | (gBreakSet << 6) | (LcrParity >> << 3) | (LcrStop << 2) | LcrData); >> - IoWrite8 (gUartBase + LCR_OFFSET, OutputData); >> + SerialPortWrite8 (gUartBase + LCR_OFFSET, OutputData); >> >> // >> // Configure baud rate >> // >> - IoWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >> 8)); >> - IoWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor & 0xff)); >> + SerialPortWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >> >> + 8)); >> + SerialPortWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor & >> + 0xff)); >> >> // >> // Switch back to bank 0 >> // >> OutputData = (UINT8) ((gBreakSet << 6) | (LcrParity << 3) | >> (LcrStop << 2) | LcrData); >> - IoWrite8 (gUartBase + LCR_OFFSET, OutputData); >> + SerialPortWrite8 (gUartBase + LCR_OFFSET, OutputData); >> >> return RETURN_SUCCESS; >> } >> -- >> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' >> >>