This is an implementation of SerialPortLib using SBI console services.
Tested with:
- Qemu RiscVVirt (non-DBCN case, backed by UART)
- TinyEMU + RiscVVirt (non-DBCN case, HTIF)
- TinyEMU + RiscVVirt (DBCN case, HTIF)
Cc: Daniel Schaefer <git@danielschaefer.me>
Cc: Sunil V L <sunilvl@ventanamicro.com>
Signed-off-by: Andrei Warkentin <andrei.warkentin@intel.com>
---
MdePkg/Library/BaseSerialPortLibRiscVSbi/BaseSerialPortLibRiscVSbi.inf | 32 +++
MdePkg/Library/BaseSerialPortLibRiscVSbi/BaseSerialPortLibRiscVSbi.c | 260 ++++++++++++++++++++
MdePkg/Library/BaseSerialPortLibRiscVSbi/BaseSerialPortLibRiscVSbi.uni | 16 ++
3 files changed, 308 insertions(+)
diff --git a/MdePkg/Library/BaseSerialPortLibRiscVSbi/BaseSerialPortLibRiscVSbi.inf b/MdePkg/Library/BaseSerialPortLibRiscVSbi/BaseSerialPortLibRiscVSbi.inf
new file mode 100644
index 000000000000..70c8e48ea48e
--- /dev/null
+++ b/MdePkg/Library/BaseSerialPortLibRiscVSbi/BaseSerialPortLibRiscVSbi.inf
@@ -0,0 +1,32 @@
+## @file
+# Serial Port Library backed by SBI console.
+#
+# Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = BaseSerialPortLibRiscVSbi
+ MODULE_UNI_FILE = BaseSerialPortLibRiscVSbi.uni
+ FILE_GUID = E4541241-8897-4bba-91F8-7D7E45654346
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = SerialPortLib
+
+
+#
+# VALID_ARCHITECTURES = RISCV64
+#
+
+[Sources]
+ BaseSerialPortLibRiscVSbi.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+
+[LibraryClasses]
+ RiscVSbiLib
diff --git a/MdePkg/Library/BaseSerialPortLibRiscVSbi/BaseSerialPortLibRiscVSbi.c b/MdePkg/Library/BaseSerialPortLibRiscVSbi/BaseSerialPortLibRiscVSbi.c
new file mode 100644
index 000000000000..46bdf26fec71
--- /dev/null
+++ b/MdePkg/Library/BaseSerialPortLibRiscVSbi/BaseSerialPortLibRiscVSbi.c
@@ -0,0 +1,260 @@
+/** @file
+ Serial Port Library backed by SBI console.
+
+ Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Base.h>
+#include <Library/SerialPortLib.h>
+#include <Library/BaseRiscVSbiLib.h>
+
+STATIC BOOLEAN mHaveDbcn = FALSE;
+STATIC INT64 mLastGetChar = -1;
+
+/**
+ Initialize the serial device hardware.
+
+ If no initialization is required, then return RETURN_SUCCESS.
+ If the serial device was successfully initialized, then return RETURN_SUCCESS.
+ If the serial device could not be initialized, then return RETURN_DEVICE_ERROR.
+
+ @retval RETURN_SUCCESS The serial device was initialized.
+ @retval RETURN_DEVICE_ERROR The serial device could not be initialized.
+
+**/
+RETURN_STATUS
+EFIAPI
+SerialPortInitialize (
+ VOID
+ )
+{
+ SBI_RET Ret;
+
+ Ret = SbiCall (SBI_EXT_BASE, SBI_EXT_BASE_PROBE_EXT, 1, SBI_EXT_DBCN);
+ if ((TranslateError (Ret.Error) == EFI_SUCCESS) &&
+ (Ret.Value != 0))
+ {
+ mHaveDbcn = TRUE;
+ }
+
+ return RETURN_SUCCESS;
+}
+
+/**
+ Write data from buffer to serial device.
+
+ Writes NumberOfBytes data bytes from Buffer to the serial device.
+ The number of bytes actually written to the serial device is returned.
+ If the return value is less than NumberOfBytes, then the write operation failed.
+ If Buffer is NULL, then ASSERT().
+ If NumberOfBytes is zero, then return 0.
+
+ @param Buffer The pointer to the data buffer to be written.
+ @param NumberOfBytes The number of bytes to written to the serial device.
+
+ @retval 0 NumberOfBytes is 0.
+ @retval >0 The number of bytes written to the serial device.
+ If this value is less than NumberOfBytes, then the write operation failed.
+
+**/
+UINTN
+EFIAPI
+SerialPortWrite (
+ IN UINT8 *Buffer,
+ IN UINTN NumberOfBytes
+ )
+{
+ UINTN Index;
+
+ if (mHaveDbcn) {
+ SBI_RET Ret;
+ Ret = SbiCall (
+ SBI_EXT_DBCN,
+ SBI_EXT_DBCN_WRITE,
+ 3,
+ NumberOfBytes,
+ ((UINTN)Buffer),
+ 0
+ );
+ if (TranslateError (Ret.Error) != EFI_SUCCESS) {
+ return 0;
+ }
+
+ return Ret.Value;
+ }
+
+ for (Index = 0; Index < NumberOfBytes; Index++) {
+ SbiCall (SBI_EXT_0_1_CONSOLE_PUTCHAR, 0, 1, Buffer[Index]);
+ }
+
+ return Index;
+}
+
+/**
+ Read data from serial device and save the datas in buffer.
+
+ Reads NumberOfBytes data bytes from a serial device into the buffer
+ specified by Buffer. The number of bytes actually read is returned.
+ If the return value is less than NumberOfBytes, then the rest operation failed.
+ If Buffer is NULL, then ASSERT().
+ If NumberOfBytes is zero, then return 0.
+
+ @param Buffer The pointer to the data buffer to store the data read from the serial device.
+ @param NumberOfBytes The number of bytes which will be read.
+
+ @retval 0 Read data failed; No data is to be read.
+ @retval >0 The actual number of bytes read from serial device.
+
+**/
+UINTN
+EFIAPI
+SerialPortRead (
+ OUT UINT8 *Buffer,
+ IN UINTN NumberOfBytes
+ )
+{
+ UINTN Index = 0;
+
+ while ((Index < NumberOfBytes) && SerialPortPoll ()) {
+ Buffer[Index++] = (UINT8)mLastGetChar;
+ mLastGetChar = -1;
+ }
+
+ return Index;
+}
+
+/**
+ Polls a serial device to see if there is any data waiting to be read.
+
+ Polls a serial device to see if there is any data waiting to be read.
+ If there is data waiting to be read from the serial device, then TRUE is returned.
+ If there is no data waiting to be read from the serial device, then FALSE is returned.
+
+ @retval TRUE Data is waiting to be read from the serial device.
+ @retval FALSE There is no data waiting to be read from the serial device.
+
+**/
+BOOLEAN
+EFIAPI
+SerialPortPoll (
+ VOID
+ )
+{
+ /*
+ * Careful. OpenSBI with HTIF console will return -1 followed by -2
+ * if there is no character received. So just check for values >= 0.
+ */
+
+ if (mLastGetChar >= 0) {
+ return TRUE;
+ }
+
+ if (mHaveDbcn) {
+ UINT8 Buffer;
+ SBI_RET Ret = SbiCall (
+ SBI_EXT_DBCN,
+ SBI_EXT_DBCN_READ,
+ 3,
+ 1,
+ ((UINTN)&Buffer),
+ 0
+ );
+ if ((TranslateError (Ret.Error) == EFI_SUCCESS) &&
+ (Ret.Value == 1))
+ {
+ mLastGetChar = Buffer;
+ }
+ } else {
+ mLastGetChar = (INT64)SbiCall (SBI_EXT_0_1_CONSOLE_GETCHAR, 0, 0).Error;
+ }
+
+ return mLastGetChar >= 0;
+}
+
+/**
+ Sets the control bits on a serial device.
+
+ @param Control Sets the bits of Control that are settable.
+
+ @retval RETURN_SUCCESS The new control bits were set on the serial device.
+ @retval RETURN_UNSUPPORTED The serial device does not support this operation.
+ @retval RETURN_DEVICE_ERROR The serial device is not functioning correctly.
+
+**/
+RETURN_STATUS
+EFIAPI
+SerialPortSetControl (
+ IN UINT32 Control
+ )
+{
+ return RETURN_SUCCESS;
+}
+
+/**
+ Retrieve the status of the control bits on a serial device.
+
+ @param Control A pointer to return the current control signals from the serial device.
+
+ @retval RETURN_SUCCESS The control bits were read from the serial device.
+ @retval RETURN_UNSUPPORTED The serial device does not support this operation.
+ @retval RETURN_DEVICE_ERROR The serial device is not functioning correctly.
+
+**/
+RETURN_STATUS
+EFIAPI
+SerialPortGetControl (
+ OUT UINT32 *Control
+ )
+{
+ *Control = 0;
+ return RETURN_SUCCESS;
+}
+
+/**
+ Sets the baud rate, receive FIFO depth, transmit/receice time out, parity,
+ data bits, and stop bits on a serial device.
+
+ @param BaudRate The requested baud rate. A BaudRate value of 0 will use the
+ device's default interface speed.
+ On output, the value actually set.
+ @param ReveiveFifoDepth The requested depth of the FIFO on the receive side of the
+ serial interface. A ReceiveFifoDepth value of 0 will use
+ the device's default FIFO depth.
+ On output, the value actually set.
+ @param Timeout The requested time out for a single character in microseconds.
+ This timeout applies to both the transmit and receive side of the
+ interface. A Timeout value of 0 will use the device's default time
+ out value.
+ On output, the value actually set.
+ @param Parity The type of parity to use on this serial device. A Parity value of
+ DefaultParity will use the device's default parity value.
+ On output, the value actually set.
+ @param DataBits The number of data bits to use on the serial device. A DataBits
+ vaule of 0 will use the device's default data bit setting.
+ On output, the value actually set.
+ @param StopBits The number of stop bits to use on this serial device. A StopBits
+ value of DefaultStopBits will use the device's default number of
+ stop bits.
+ On output, the value actually set.
+
+ @retval RETURN_SUCCESS The new attributes were set on the serial device.
+ @retval RETURN_UNSUPPORTED The serial device does not support this operation.
+ @retval RETURN_INVALID_PARAMETER One or more of the attributes has an unsupported value.
+ @retval RETURN_DEVICE_ERROR The serial device is not functioning correctly.
+
+**/
+RETURN_STATUS
+EFIAPI
+SerialPortSetAttributes (
+ IN OUT UINT64 *BaudRate,
+ IN OUT UINT32 *ReceiveFifoDepth,
+ IN OUT UINT32 *Timeout,
+ IN OUT EFI_PARITY_TYPE *Parity,
+ IN OUT UINT8 *DataBits,
+ IN OUT EFI_STOP_BITS_TYPE *StopBits
+ )
+{
+ return RETURN_SUCCESS;
+}
diff --git a/MdePkg/Library/BaseSerialPortLibRiscVSbi/BaseSerialPortLibRiscVSbi.uni b/MdePkg/Library/BaseSerialPortLibRiscVSbi/BaseSerialPortLibRiscVSbi.uni
new file mode 100644
index 000000000000..7b22caa5a090
--- /dev/null
+++ b/MdePkg/Library/BaseSerialPortLibRiscVSbi/BaseSerialPortLibRiscVSbi.uni
@@ -0,0 +1,16 @@
+// /** @file
+// Serial Port Library backed by SBI console.
+//
+// Serial Port Library backed by SBI console.
+//
+// Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
+//
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+// **/
+
+
+#string STR_MODULE_ABSTRACT #language en-US "Serial Port Library backed by SBI console"
+
+#string STR_MODULE_DESCRIPTION #language en-US "Serial Port Library backed by SBI console."
+
--
2.25.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100584): https://edk2.groups.io/g/devel/message/100584
Mute This Topic: https://groups.io/mt/97309875/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=- Hi,
> + while ((Index < NumberOfBytes) && SerialPortPoll ()) {
> + Buffer[Index++] = (UINT8)mLastGetChar;
> + mLastGetChar = -1;
> + }
Why do you use a global variable to pass the character? SerialPortPoll
could just return it directly ...
take care,
Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100585): https://edk2.groups.io/g/devel/message/100585
Mute This Topic: https://groups.io/mt/97309875/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
The library caller could call Poll() first, then call Read(), and the expectation is the char won't be lost. SBI doesn't have a poll call itself, so best you could do is read and stash. So you have to stash it globally. And read must use the stashed char, if it exists, before requesting more data from SBI. At that point, reading is easy to treat as continued polling - just to keep the code simple.
A
________________________________
От: Gerd Hoffmann <kraxel@redhat.com>
Отправлено: Wednesday, March 1, 2023 2:13:32 AM
Кому: devel@edk2.groups.io <devel@edk2.groups.io>; Warkentin, Andrei <andrei.warkentin@intel.com>
Копия: Daniel Schaefer <git@danielschaefer.me>; Sunil V L <sunilvl@ventanamicro.com>
Тема: Re: [edk2-devel] [edk2 2/2] MdePkg: add BaseSerialPortLibRiscVSbi
Hi,
> + while ((Index < NumberOfBytes) && SerialPortPoll ()) {
> + Buffer[Index++] = (UINT8)mLastGetChar;
> + mLastGetChar = -1;
> + }
Why do you use a global variable to pass the character? SerialPortPoll
could just return it directly ...
take care,
Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100586): https://edk2.groups.io/g/devel/message/100586
Mute This Topic: https://groups.io/mt/97309875/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On Wed, Mar 01, 2023 at 08:50:38AM +0000, Warkentin, Andrei wrote: > The library caller could call Poll() first, Ah, Poll is part of the library API, not just an internal helper. Ok, makes sense then. series: Acked-by: Gerd Hoffmann <kraxel@redhat.com> take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100587): https://edk2.groups.io/g/devel/message/100587 Mute This Topic: https://groups.io/mt/97309875/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Using a global is not compatible with XIP code where only const globals are supported. A module of type BASE is considered compatible with XIP components. Mike > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd Hoffmann > Sent: Wednesday, March 1, 2023 1:07 AM > To: Warkentin, Andrei <andrei.warkentin@intel.com> > Cc: devel@edk2.groups.io; Daniel Schaefer <git@danielschaefer.me>; Sunil V L <sunilvl@ventanamicro.com> > Subject: Re: [edk2-devel] [edk2 2/2] MdePkg: add BaseSerialPortLibRiscVSbi > > On Wed, Mar 01, 2023 at 08:50:38AM +0000, Warkentin, Andrei wrote: > > The library caller could call Poll() first, > > Ah, Poll is part of the library API, not just an internal helper. > Ok, makes sense then. > > series: > Acked-by: Gerd Hoffmann <kraxel@redhat.com> > > take care, > Gerd > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100600): https://edk2.groups.io/g/devel/message/100600 Mute This Topic: https://groups.io/mt/97309875/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi Michael, What module type should I use instead? The reason being this SerialPortLib implementation specifically fits into the class of UEFI implementations, where RAM is always available (initialized by something else prior to Tiano) and non-const globals can be used (e.g. PrePi). A -----Original Message----- From: Kinney, Michael D <michael.d.kinney@intel.com> Sent: Wednesday, March 1, 2023 10:56 AM To: devel@edk2.groups.io; kraxel@redhat.com; Warkentin, Andrei <andrei.warkentin@intel.com> Cc: Daniel Schaefer <git@danielschaefer.me>; Sunil V L <sunilvl@ventanamicro.com>; Kinney, Michael D <michael.d.kinney@intel.com> Subject: RE: [edk2-devel] [edk2 2/2] MdePkg: add BaseSerialPortLibRiscVSbi Using a global is not compatible with XIP code where only const globals are supported. A module of type BASE is considered compatible with XIP components. Mike > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd Hoffmann > Sent: Wednesday, March 1, 2023 1:07 AM > To: Warkentin, Andrei <andrei.warkentin@intel.com> > Cc: devel@edk2.groups.io; Daniel Schaefer <git@danielschaefer.me>; Sunil V L <sunilvl@ventanamicro.com> > Subject: Re: [edk2-devel] [edk2 2/2] MdePkg: add BaseSerialPortLibRiscVSbi > > On Wed, Mar 01, 2023 at 08:50:38AM +0000, Warkentin, Andrei wrote: > > The library caller could call Poll() first, > > Ah, Poll is part of the library API, not just an internal helper. > Ok, makes sense then. > > series: > Acked-by: Gerd Hoffmann <kraxel@redhat.com> > > take care, > Gerd > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100601): https://edk2.groups.io/g/devel/message/100601 Mute This Topic: https://groups.io/mt/97309875/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
If a dependency on MdePkg\Include\Library\ BaseRiscVSbiLib.h and SBI services assumed memory is available and code loaded is loaded into RAM and not XIP, then BASE is ok for this component. You may want to make sure the library class header file and INF file header for BaseRiscVSbiLib describe that environment assumption and the INF for BaseSerialPortLibRiscVSbi also describes that environment assumption. Mike > -----Original Message----- > From: Warkentin, Andrei <andrei.warkentin@intel.com> > Sent: Wednesday, March 1, 2023 9:26 AM > To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io; kraxel@redhat.com > Cc: Daniel Schaefer <git@danielschaefer.me>; Sunil V L <sunilvl@ventanamicro.com> > Subject: RE: [edk2-devel] [edk2 2/2] MdePkg: add BaseSerialPortLibRiscVSbi > > Hi Michael, > > What module type should I use instead? The reason being this SerialPortLib implementation specifically fits into the class of > UEFI implementations, where RAM is always available (initialized by something else prior to Tiano) and non-const globals can be > used (e.g. PrePi). > > A > > -----Original Message----- > From: Kinney, Michael D <michael.d.kinney@intel.com> > Sent: Wednesday, March 1, 2023 10:56 AM > To: devel@edk2.groups.io; kraxel@redhat.com; Warkentin, Andrei <andrei.warkentin@intel.com> > Cc: Daniel Schaefer <git@danielschaefer.me>; Sunil V L <sunilvl@ventanamicro.com>; Kinney, Michael D > <michael.d.kinney@intel.com> > Subject: RE: [edk2-devel] [edk2 2/2] MdePkg: add BaseSerialPortLibRiscVSbi > > Using a global is not compatible with XIP code where only const globals are supported. > > A module of type BASE is considered compatible with XIP components. > > > Mike > > > -----Original Message----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd Hoffmann > > Sent: Wednesday, March 1, 2023 1:07 AM > > To: Warkentin, Andrei <andrei.warkentin@intel.com> > > Cc: devel@edk2.groups.io; Daniel Schaefer <git@danielschaefer.me>; Sunil V L <sunilvl@ventanamicro.com> > > Subject: Re: [edk2-devel] [edk2 2/2] MdePkg: add BaseSerialPortLibRiscVSbi > > > > On Wed, Mar 01, 2023 at 08:50:38AM +0000, Warkentin, Andrei wrote: > > > The library caller could call Poll() first, > > > > Ah, Poll is part of the library API, not just an internal helper. > > Ok, makes sense then. > > > > series: > > Acked-by: Gerd Hoffmann <kraxel@redhat.com> > > > > take care, > > Gerd > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100602): https://edk2.groups.io/g/devel/message/100602 Mute This Topic: https://groups.io/mt/97309875/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Thanks... let me go a different route. I'll have 1 more variant that can be used in classical SEC and PEI - only capable of output (input doesn't matter) and rename the one I already shared. I'll name these SecPeiSerialPortLibRiscVSbi and PrePiDxeSerialPortLibRiscVSbi, respectively. Does this sound acceptable? -----Original Message----- From: Kinney, Michael D <michael.d.kinney@intel.com> Sent: Wednesday, March 1, 2023 11:51 AM To: Warkentin, Andrei <andrei.warkentin@intel.com>; devel@edk2.groups.io; kraxel@redhat.com Cc: Daniel Schaefer <git@danielschaefer.me>; Sunil V L <sunilvl@ventanamicro.com>; Kinney, Michael D <michael.d.kinney@intel.com> Subject: RE: [edk2-devel] [edk2 2/2] MdePkg: add BaseSerialPortLibRiscVSbi If a dependency on MdePkg\Include\Library\ BaseRiscVSbiLib.h and SBI services assumed memory is available and code loaded is loaded into RAM and not XIP, then BASE is ok for this component. You may want to make sure the library class header file and INF file header for BaseRiscVSbiLib describe that environment assumption and the INF for BaseSerialPortLibRiscVSbi also describes that environment assumption. Mike > -----Original Message----- > From: Warkentin, Andrei <andrei.warkentin@intel.com> > Sent: Wednesday, March 1, 2023 9:26 AM > To: Kinney, Michael D <michael.d.kinney@intel.com>; > devel@edk2.groups.io; kraxel@redhat.com > Cc: Daniel Schaefer <git@danielschaefer.me>; Sunil V L > <sunilvl@ventanamicro.com> > Subject: RE: [edk2-devel] [edk2 2/2] MdePkg: add > BaseSerialPortLibRiscVSbi > > Hi Michael, > > What module type should I use instead? The reason being this > SerialPortLib implementation specifically fits into the class of UEFI > implementations, where RAM is always available (initialized by something else prior to Tiano) and non-const globals can be used (e.g. PrePi). > > A > > -----Original Message----- > From: Kinney, Michael D <michael.d.kinney@intel.com> > Sent: Wednesday, March 1, 2023 10:56 AM > To: devel@edk2.groups.io; kraxel@redhat.com; Warkentin, Andrei > <andrei.warkentin@intel.com> > Cc: Daniel Schaefer <git@danielschaefer.me>; Sunil V L > <sunilvl@ventanamicro.com>; Kinney, Michael D > <michael.d.kinney@intel.com> > Subject: RE: [edk2-devel] [edk2 2/2] MdePkg: add > BaseSerialPortLibRiscVSbi > > Using a global is not compatible with XIP code where only const globals are supported. > > A module of type BASE is considered compatible with XIP components. > > > Mike > > > -----Original Message----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd > > Hoffmann > > Sent: Wednesday, March 1, 2023 1:07 AM > > To: Warkentin, Andrei <andrei.warkentin@intel.com> > > Cc: devel@edk2.groups.io; Daniel Schaefer <git@danielschaefer.me>; > > Sunil V L <sunilvl@ventanamicro.com> > > Subject: Re: [edk2-devel] [edk2 2/2] MdePkg: add > > BaseSerialPortLibRiscVSbi > > > > On Wed, Mar 01, 2023 at 08:50:38AM +0000, Warkentin, Andrei wrote: > > > The library caller could call Poll() first, > > > > Ah, Poll is part of the library API, not just an internal helper. > > Ok, makes sense then. > > > > series: > > Acked-by: Gerd Hoffmann <kraxel@redhat.com> > > > > take care, > > Gerd > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100603): https://edk2.groups.io/g/devel/message/100603 Mute This Topic: https://groups.io/mt/97309875/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi Andrei, It is not just mLastGetChar but mHaveDbcn also will have the same issue with XIP. So, I am wondering why not probe for the extension in every write for the SecPei case? I understand the performance concerns but does it really matter for debug output? I believe it is minor compared to the flexibility it provides for not having a fixed serial port implementation. Thanks, Sunil On Wed, Mar 01, 2023 at 05:56:36PM +0000, Andrei Warkentin wrote: > Thanks... let me go a different route. > > I'll have 1 more variant that can be used in classical SEC and PEI - only capable of output (input doesn't matter) and rename the one I already shared. I'll name these SecPeiSerialPortLibRiscVSbi and PrePiDxeSerialPortLibRiscVSbi, respectively. > > Does this sound acceptable? > > -----Original Message----- > From: Kinney, Michael D <michael.d.kinney@intel.com> > Sent: Wednesday, March 1, 2023 11:51 AM > To: Warkentin, Andrei <andrei.warkentin@intel.com>; devel@edk2.groups.io; kraxel@redhat.com > Cc: Daniel Schaefer <git@danielschaefer.me>; Sunil V L <sunilvl@ventanamicro.com>; Kinney, Michael D <michael.d.kinney@intel.com> > Subject: RE: [edk2-devel] [edk2 2/2] MdePkg: add BaseSerialPortLibRiscVSbi > > If a dependency on MdePkg\Include\Library\ BaseRiscVSbiLib.h and SBI services assumed memory is available and code loaded is loaded into RAM and not XIP, then BASE is ok for this component. > > You may want to make sure the library class header file and INF file header for BaseRiscVSbiLib describe that environment assumption and the INF for BaseSerialPortLibRiscVSbi also describes that environment assumption. > > Mike > > > -----Original Message----- > > From: Warkentin, Andrei <andrei.warkentin@intel.com> > > Sent: Wednesday, March 1, 2023 9:26 AM > > To: Kinney, Michael D <michael.d.kinney@intel.com>; > > devel@edk2.groups.io; kraxel@redhat.com > > Cc: Daniel Schaefer <git@danielschaefer.me>; Sunil V L > > <sunilvl@ventanamicro.com> > > Subject: RE: [edk2-devel] [edk2 2/2] MdePkg: add > > BaseSerialPortLibRiscVSbi > > > > Hi Michael, > > > > What module type should I use instead? The reason being this > > SerialPortLib implementation specifically fits into the class of UEFI > > implementations, where RAM is always available (initialized by something else prior to Tiano) and non-const globals can be used (e.g. PrePi). > > > > A > > > > -----Original Message----- > > From: Kinney, Michael D <michael.d.kinney@intel.com> > > Sent: Wednesday, March 1, 2023 10:56 AM > > To: devel@edk2.groups.io; kraxel@redhat.com; Warkentin, Andrei > > <andrei.warkentin@intel.com> > > Cc: Daniel Schaefer <git@danielschaefer.me>; Sunil V L > > <sunilvl@ventanamicro.com>; Kinney, Michael D > > <michael.d.kinney@intel.com> > > Subject: RE: [edk2-devel] [edk2 2/2] MdePkg: add > > BaseSerialPortLibRiscVSbi > > > > Using a global is not compatible with XIP code where only const globals are supported. > > > > A module of type BASE is considered compatible with XIP components. > > > > > > Mike > > > > > -----Original Message----- > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd > > > Hoffmann > > > Sent: Wednesday, March 1, 2023 1:07 AM > > > To: Warkentin, Andrei <andrei.warkentin@intel.com> > > > Cc: devel@edk2.groups.io; Daniel Schaefer <git@danielschaefer.me>; > > > Sunil V L <sunilvl@ventanamicro.com> > > > Subject: Re: [edk2-devel] [edk2 2/2] MdePkg: add > > > BaseSerialPortLibRiscVSbi > > > > > > On Wed, Mar 01, 2023 at 08:50:38AM +0000, Warkentin, Andrei wrote: > > > > The library caller could call Poll() first, > > > > > > Ah, Poll is part of the library API, not just an internal helper. > > > Ok, makes sense then. > > > > > > series: > > > Acked-by: Gerd Hoffmann <kraxel@redhat.com> > > > > > > take care, > > > Gerd > > > > > > > > > > > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100629): https://edk2.groups.io/g/devel/message/100629 Mute This Topic: https://groups.io/mt/97309875/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
That's exactly what I did. The SecPei variant has no globals and always probes (but can't read, only writes) Will send an updated patch set later today. A ________________________________ От: Sunil V L <sunilvl@ventanamicro.com> Отправлено: четверг, марта 2, 2023 4:35 AM Кому: devel@edk2.groups.io <devel@edk2.groups.io>; Warkentin, Andrei <andrei.warkentin@intel.com> Копия: Kinney, Michael D <michael.d.kinney@intel.com>; kraxel@redhat.com <kraxel@redhat.com>; Daniel Schaefer <git@danielschaefer.me> Тема: Re: [edk2-devel] [edk2 2/2] MdePkg: add BaseSerialPortLibRiscVSbi Hi Andrei, It is not just mLastGetChar but mHaveDbcn also will have the same issue with XIP. So, I am wondering why not probe for the extension in every write for the SecPei case? I understand the performance concerns but does it really matter for debug output? I believe it is minor compared to the flexibility it provides for not having a fixed serial port implementation. Thanks, Sunil On Wed, Mar 01, 2023 at 05:56:36PM +0000, Andrei Warkentin wrote: > Thanks... let me go a different route. > > I'll have 1 more variant that can be used in classical SEC and PEI - only capable of output (input doesn't matter) and rename the one I already shared. I'll name these SecPeiSerialPortLibRiscVSbi and PrePiDxeSerialPortLibRiscVSbi, respectively. > > Does this sound acceptable? > > -----Original Message----- > From: Kinney, Michael D <michael.d.kinney@intel.com> > Sent: Wednesday, March 1, 2023 11:51 AM > To: Warkentin, Andrei <andrei.warkentin@intel.com>; devel@edk2.groups.io; kraxel@redhat.com > Cc: Daniel Schaefer <git@danielschaefer.me>; Sunil V L <sunilvl@ventanamicro.com>; Kinney, Michael D <michael.d.kinney@intel.com> > Subject: RE: [edk2-devel] [edk2 2/2] MdePkg: add BaseSerialPortLibRiscVSbi > > If a dependency on MdePkg\Include\Library\ BaseRiscVSbiLib.h and SBI services assumed memory is available and code loaded is loaded into RAM and not XIP, then BASE is ok for this component. > > You may want to make sure the library class header file and INF file header for BaseRiscVSbiLib describe that environment assumption and the INF for BaseSerialPortLibRiscVSbi also describes that environment assumption. > > Mike > > > -----Original Message----- > > From: Warkentin, Andrei <andrei.warkentin@intel.com> > > Sent: Wednesday, March 1, 2023 9:26 AM > > To: Kinney, Michael D <michael.d.kinney@intel.com>; > > devel@edk2.groups.io; kraxel@redhat.com > > Cc: Daniel Schaefer <git@danielschaefer.me>; Sunil V L > > <sunilvl@ventanamicro.com> > > Subject: RE: [edk2-devel] [edk2 2/2] MdePkg: add > > BaseSerialPortLibRiscVSbi > > > > Hi Michael, > > > > What module type should I use instead? The reason being this > > SerialPortLib implementation specifically fits into the class of UEFI > > implementations, where RAM is always available (initialized by something else prior to Tiano) and non-const globals can be used (e.g. PrePi). > > > > A > > > > -----Original Message----- > > From: Kinney, Michael D <michael.d.kinney@intel.com> > > Sent: Wednesday, March 1, 2023 10:56 AM > > To: devel@edk2.groups.io; kraxel@redhat.com; Warkentin, Andrei > > <andrei.warkentin@intel.com> > > Cc: Daniel Schaefer <git@danielschaefer.me>; Sunil V L > > <sunilvl@ventanamicro.com>; Kinney, Michael D > > <michael.d.kinney@intel.com> > > Subject: RE: [edk2-devel] [edk2 2/2] MdePkg: add > > BaseSerialPortLibRiscVSbi > > > > Using a global is not compatible with XIP code where only const globals are supported. > > > > A module of type BASE is considered compatible with XIP components. > > > > > > Mike > > > > > -----Original Message----- > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd > > > Hoffmann > > > Sent: Wednesday, March 1, 2023 1:07 AM > > > To: Warkentin, Andrei <andrei.warkentin@intel.com> > > > Cc: devel@edk2.groups.io; Daniel Schaefer <git@danielschaefer.me>; > > > Sunil V L <sunilvl@ventanamicro.com> > > > Subject: Re: [edk2-devel] [edk2 2/2] MdePkg: add > > > BaseSerialPortLibRiscVSbi > > > > > > On Wed, Mar 01, 2023 at 08:50:38AM +0000, Warkentin, Andrei wrote: > > > > The library caller could call Poll() first, > > > > > > Ah, Poll is part of the library API, not just an internal helper. > > > Ok, makes sense then. > > > > > > series: > > > Acked-by: Gerd Hoffmann <kraxel@redhat.com> > > > > > > take care, > > > Gerd > > > > > > > > > > > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100635): https://edk2.groups.io/g/devel/message/100635 Mute This Topic: https://groups.io/mt/97309875/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.