[edk2-devel] [PATCH v6 2/3] MdePkg: add SBI-based SerialPortLib for RISC-V

Andrei Warkentin posted 3 patches 2 years, 10 months ago
There is a newer version of this series
[edk2-devel] [PATCH v6 2/3] MdePkg: add SBI-based SerialPortLib for RISC-V
Posted by Andrei Warkentin 2 years, 10 months ago
These are implementations of SerialPortLib using SBI console services.
- BaseSerialPortLibRiscVSbiLib is appropriate for SEC/PEI (XIP) environments
- BaseSerialPortLibRiscVSbiLibRam is appropriate for PrePI/DXE environments

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>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Andrei Warkentin <andrei.warkentin@intel.com>
---
 MdePkg/MdePkg.dsc                                                                  |   2 +
 MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.inf       |  39 +++
 MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRiscVSbiLibRam.inf |  36 +++
 MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.c         | 233 ++++++++++++++++
 MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRiscVSbiLibRam.c   | 288 ++++++++++++++++++++
 MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.uni       |  16 ++
 MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRiscVSbiLibRam.uni |  16 ++
 7 files changed, 630 insertions(+)

diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index 0ac7618b4623..ceccc078ff04 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -192,5 +192,7 @@ [Components.ARM, Components.AARCH64]
 
 [Components.RISCV64]
   MdePkg/Library/BaseRiscVSbiLib/BaseRiscVSbiLib.inf
+  MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.inf
+  MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRiscVSbiLibRam.inf
 
 [BuildOptions]
diff --git a/MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.inf b/MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.inf
new file mode 100644
index 000000000000..09cf59f190f6
--- /dev/null
+++ b/MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.inf
@@ -0,0 +1,39 @@
+## @file
+#  Serial Port Library backed by SBI console.
+#
+#  Meant for SEC and PEI (XIP) environments.
+#
+#  Due to limitations of SBI console interface and XIP environments
+#  (on use of globals), this library instance does not implement reading
+#  and polling the serial port. See PrePiDxeSerialPortLibRiscVSbi for
+#  the full-featured variant meant for PrePi and DXE environments.
+#
+#  Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x0001001B
+  BASE_NAME                      = BaseSerialPortLibRiscVSbiLib
+  MODULE_UNI_FILE                = BaseSerialPortLibRiscVSbiLib.uni
+  FILE_GUID                      = 639fad38-4bfd-4eb9-9f09-e97c7947d480
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = SerialPortLib | SEC PEI_CORE PEIM
+
+
+#
+#  VALID_ARCHITECTURES           = RISCV64
+#
+
+[Sources]
+  BaseSerialPortLibRiscVSbiLib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  RiscVSbiLib
diff --git a/MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRiscVSbiLibRam.inf b/MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRiscVSbiLibRam.inf
new file mode 100644
index 000000000000..b7ad1548a309
--- /dev/null
+++ b/MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRiscVSbiLibRam.inf
@@ -0,0 +1,36 @@
+## @file
+#  Serial Port Library backed by SBI console.
+#
+#  Meant for PrePi and DXE environments (where globals are allowed). See
+#  BaseSerialPortLibRiscVSbiLib for a reduced variant appropriate for SEC
+#  and PEI (XIP) environments.
+#
+#  Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x0001001B
+  BASE_NAME                      = BaseSerialPortLibRiscVSbiLibRam
+  MODULE_UNI_FILE                = BaseSerialPortLibRiscVSbiLibRam.uni
+  FILE_GUID                      = 872af743-ab56-45b4-a065-602567f4820c
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = SerialPortLib | SEC DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER UEFI_DRIVER UEFI_APPLICATION
+
+
+#
+#  VALID_ARCHITECTURES           = RISCV64
+#
+
+[Sources]
+  BaseSerialPortLibRiscVSbiLibRam.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  RiscVSbiLib
diff --git a/MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.c b/MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.c
new file mode 100644
index 000000000000..0ad5931be3ae
--- /dev/null
+++ b/MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.c
@@ -0,0 +1,233 @@
+/** @file
+  Serial Port Library backed by SBI console.
+
+  Meant for SEC and PEI (XIP) environments.
+
+  Due to limitations of SBI console interface and XIP environments
+  (on use of globals), this library instance does not implement reading
+  and polling the serial port. See PrePiDxeSerialPortLibRiscVSbi for
+  the full-featured variant meant for PrePi and DXE environments.
+
+  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>
+
+/**
+  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
+  )
+{
+  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
+  )
+{
+  SBI_RET  Ret;
+  UINTN    Index;
+
+  Ret = SbiCall (SBI_EXT_BASE, SBI_EXT_BASE_PROBE_EXT, 1, SBI_EXT_DBCN);
+  if ((TranslateError (Ret.Error) == EFI_SUCCESS) &&
+      (Ret.Value != 0))
+  {
+    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;
+  }
+
+  Ret = SbiCall (SBI_EXT_BASE, SBI_EXT_BASE_PROBE_EXT, 1, SBI_EXT_0_1_CONSOLE_PUTCHAR);
+  if ((TranslateError (Ret.Error) == EFI_SUCCESS) &&
+      (Ret.Value != 0))
+  {
+    for (Index = 0; Index < NumberOfBytes; Index++) {
+      SbiCall (SBI_EXT_0_1_CONSOLE_PUTCHAR, 0, 1, Buffer[Index]);
+    }
+
+    return Index;
+  }
+
+  /*
+   * Neither DBCN or legacy extension were present.
+   */
+  return NumberOfBytes;
+}
+
+/**
+  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
+  )
+{
+  return 0;
+}
+
+/**
+  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
+  )
+{
+  return FALSE;
+}
+
+/**
+  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/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRiscVSbiLibRam.c b/MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRiscVSbiLibRam.c
new file mode 100644
index 000000000000..74277241bba4
--- /dev/null
+++ b/MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRiscVSbiLibRam.c
@@ -0,0 +1,288 @@
+/** @file
+  Serial Port Library backed by SBI console.
+
+  Meant for PrePi and DXE environments (where globals are allowed). See
+  BaseSerialPortLibRiscVSbiLib for a reduced variant appropriate for SEC
+  and PEI (XIP) environments.
+
+  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 BOOLEAN  mHaveLegacyPutchar = FALSE;
+STATIC BOOLEAN  mHaveLegacyGetchar = 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;
+  } else {
+    Ret = SbiCall (SBI_EXT_BASE, SBI_EXT_BASE_PROBE_EXT, 1, SBI_EXT_0_1_CONSOLE_PUTCHAR);
+    if ((TranslateError (Ret.Error) == EFI_SUCCESS) &&
+        (Ret.Value != 0))
+    {
+      mHaveLegacyPutchar = TRUE;
+    }
+
+    Ret = SbiCall (SBI_EXT_BASE, SBI_EXT_BASE_PROBE_EXT, 1, SBI_EXT_0_1_CONSOLE_GETCHAR);
+    if ((TranslateError (Ret.Error) == EFI_SUCCESS) &&
+        (Ret.Value != 0))
+    {
+      mHaveLegacyGetchar = 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;
+  } else if (mHaveLegacyPutchar) {
+    for (Index = 0; Index < NumberOfBytes; Index++) {
+      SbiCall (SBI_EXT_0_1_CONSOLE_PUTCHAR, 0, 1, Buffer[Index]);
+    }
+
+    return Index;
+  }
+
+  /*
+   * Neither DBCN or legacy extension were present.
+   */
+  return NumberOfBytes;
+}
+
+/**
+  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;
+
+  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;
+
+    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 if (mHaveLegacyGetchar) {
+    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/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.uni b/MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.uni
new file mode 100644
index 000000000000..7b22caa5a090
--- /dev/null
+++ b/MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.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."
+
diff --git a/MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRiscVSbiLibRam.uni b/MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRiscVSbiLibRam.uni
new file mode 100644
index 000000000000..7b22caa5a090
--- /dev/null
+++ b/MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRiscVSbiLibRam.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 (#102505): https://edk2.groups.io/g/devel/message/102505
Mute This Topic: https://groups.io/mt/98064147/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v6 2/3] MdePkg: add SBI-based SerialPortLib for RISC-V
Posted by Sunil V L 2 years, 10 months ago
Hi Andrei,

I had couple of questions on previous version of this patch. Adding them
inline again in case you had missed them. Please check if they are
valid.

Also, I think it would be better to have single directory
BaseSerialPortLibRiscVSbiLib and inside that we can have both INF files.
This way we can share UNI files etc between both libraries.

On Tue, Apr 04, 2023 at 11:43:58AM -0500, Andrei Warkentin wrote:
> These are implementations of SerialPortLib using SBI console services.
> - BaseSerialPortLibRiscVSbiLib is appropriate for SEC/PEI (XIP) environments
> - BaseSerialPortLibRiscVSbiLibRam is appropriate for PrePI/DXE environments
> 
> 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>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Andrei Warkentin <andrei.warkentin@intel.com>
> ---
>  MdePkg/MdePkg.dsc                                                                  |   2 +
>  MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.inf       |  39 +++
>  MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRiscVSbiLibRam.inf |  36 +++
>  MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.c         | 233 ++++++++++++++++
>  MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRiscVSbiLibRam.c   | 288 ++++++++++++++++++++
>  MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.uni       |  16 ++
>  MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRiscVSbiLibRam.uni |  16 ++
>  7 files changed, 630 insertions(+)
> 
> diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
> index 0ac7618b4623..ceccc078ff04 100644
> --- a/MdePkg/MdePkg.dsc
> +++ b/MdePkg/MdePkg.dsc
> @@ -192,5 +192,7 @@ [Components.ARM, Components.AARCH64]
>  
>  [Components.RISCV64]
>    MdePkg/Library/BaseRiscVSbiLib/BaseRiscVSbiLib.inf
> +  MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.inf
> +  MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRiscVSbiLibRam.inf
>  
>  [BuildOptions]
> diff --git a/MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.inf b/MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.inf
> new file mode 100644
> index 000000000000..09cf59f190f6
> --- /dev/null
> +++ b/MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.inf
> @@ -0,0 +1,39 @@
> +## @file
> +#  Serial Port Library backed by SBI console.
> +#
> +#  Meant for SEC and PEI (XIP) environments.
> +#
> +#  Due to limitations of SBI console interface and XIP environments
> +#  (on use of globals), this library instance does not implement reading
> +#  and polling the serial port. See PrePiDxeSerialPortLibRiscVSbi for
> +#  the full-featured variant meant for PrePi and DXE environments.
> +#
> +#  Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001B
> +  BASE_NAME                      = BaseSerialPortLibRiscVSbiLib
> +  MODULE_UNI_FILE                = BaseSerialPortLibRiscVSbiLib.uni
> +  FILE_GUID                      = 639fad38-4bfd-4eb9-9f09-e97c7947d480
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = SerialPortLib | SEC PEI_CORE PEIM
> +
> +
> +#
> +#  VALID_ARCHITECTURES           = RISCV64
> +#
> +
> +[Sources]
> +  BaseSerialPortLibRiscVSbiLib.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  RiscVSbiLib
> diff --git a/MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRiscVSbiLibRam.inf b/MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRiscVSbiLibRam.inf
> new file mode 100644
> index 000000000000..b7ad1548a309
> --- /dev/null
> +++ b/MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRiscVSbiLibRam.inf
> @@ -0,0 +1,36 @@
> +## @file
> +#  Serial Port Library backed by SBI console.
> +#
> +#  Meant for PrePi and DXE environments (where globals are allowed). See
> +#  BaseSerialPortLibRiscVSbiLib for a reduced variant appropriate for SEC
> +#  and PEI (XIP) environments.
> +#
> +#  Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001B
> +  BASE_NAME                      = BaseSerialPortLibRiscVSbiLibRam
> +  MODULE_UNI_FILE                = BaseSerialPortLibRiscVSbiLibRam.uni
> +  FILE_GUID                      = 872af743-ab56-45b4-a065-602567f4820c
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = SerialPortLib | SEC DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER UEFI_DRIVER UEFI_APPLICATION
> +
> +
> +#
> +#  VALID_ARCHITECTURES           = RISCV64
> +#
> +
> +[Sources]
> +  BaseSerialPortLibRiscVSbiLibRam.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  RiscVSbiLib
> diff --git a/MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.c b/MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.c
> new file mode 100644
> index 000000000000..0ad5931be3ae
> --- /dev/null
> +++ b/MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.c
> @@ -0,0 +1,233 @@
> +/** @file
> +  Serial Port Library backed by SBI console.
> +
> +  Meant for SEC and PEI (XIP) environments.
> +
> +  Due to limitations of SBI console interface and XIP environments
> +  (on use of globals), this library instance does not implement reading
> +  and polling the serial port. See PrePiDxeSerialPortLibRiscVSbi for
> +  the full-featured variant meant for PrePi and DXE environments.
> +
> +  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>
> +
> +/**
> +  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
> +  )
> +{
> +  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
> +  )
> +{
> +  SBI_RET  Ret;
> +  UINTN    Index;
> +
> +  Ret = SbiCall (SBI_EXT_BASE, SBI_EXT_BASE_PROBE_EXT, 1, SBI_EXT_DBCN);
> +  if ((TranslateError (Ret.Error) == EFI_SUCCESS) &&
> +      (Ret.Value != 0))
> +  {
> +    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;
> +  }
> +
> +  Ret = SbiCall (SBI_EXT_BASE, SBI_EXT_BASE_PROBE_EXT, 1, SBI_EXT_0_1_CONSOLE_PUTCHAR);
> +  if ((TranslateError (Ret.Error) == EFI_SUCCESS) &&
> +      (Ret.Value != 0))
> +  {
> +    for (Index = 0; Index < NumberOfBytes; Index++) {
> +      SbiCall (SBI_EXT_0_1_CONSOLE_PUTCHAR, 0, 1, Buffer[Index]);
What happens if it fails? Shouldn't it return immediately instead of
continuing with the loop?

> +    }
> +
> +    return Index;
> +  }
> +
> +  /*
> +   * Neither DBCN or legacy extension were present.
> +   */
> +  return NumberOfBytes;
Isn't it an error? And for earlier DBCN_WRITE error, 0 is returned. So,
shouldn't we return 0 here also instead of NumberOfBytes?

Same questions for similar place in other library also.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102589): https://edk2.groups.io/g/devel/message/102589
Mute This Topic: https://groups.io/mt/98064147/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v6 2/3] MdePkg: add SBI-based SerialPortLib for RISC-V
Posted by Andrei Warkentin 2 years, 10 months ago
Thanks Sunil, I have in fact missed your email.

Will apply your feedback, retest and resend.

A

> -----Original Message-----
> From: Sunil V L <sunilvl@ventanamicro.com>
> Sent: Thursday, April 6, 2023 12:00 AM
> To: Warkentin, Andrei <andrei.warkentin@intel.com>
> Cc: devel@edk2.groups.io; Daniel Schaefer <git@danielschaefer.me>;
> Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Gerd
> Hoffmann <kraxel@redhat.com>
> Subject: Re: [PATCH v6 2/3] MdePkg: add SBI-based SerialPortLib for RISC-V
> 
> Hi Andrei,
> 
> I had couple of questions on previous version of this patch. Adding them
> inline again in case you had missed them. Please check if they are valid.
> 
> Also, I think it would be better to have single directory
> BaseSerialPortLibRiscVSbiLib and inside that we can have both INF files.
> This way we can share UNI files etc between both libraries.
> 
> On Tue, Apr 04, 2023 at 11:43:58AM -0500, Andrei Warkentin wrote:
> > These are implementations of SerialPortLib using SBI console services.
> > - BaseSerialPortLibRiscVSbiLib is appropriate for SEC/PEI (XIP)
> > environments
> > - BaseSerialPortLibRiscVSbiLibRam is appropriate for PrePI/DXE
> > environments
> >
> > 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>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> > Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> > Signed-off-by: Andrei Warkentin <andrei.warkentin@intel.com>
> > ---
> >  MdePkg/MdePkg.dsc                                                                  |   2 +
> >
> MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.in
> f       |  39 +++
> >
> MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRiscVSbiL
> ibRam.inf |  36 +++
> >
> MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.c
> | 233 ++++++++++++++++
> >
> MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRiscVSbiL
> ibRam.c   | 288 ++++++++++++++++++++
> >
> MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSbiLib.u
> ni       |  16 ++
> >
> > MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRiscVS
> > biLibRam.uni |  16 ++
> >  7 files changed, 630 insertions(+)
> >
> > diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc index
> > 0ac7618b4623..ceccc078ff04 100644
> > --- a/MdePkg/MdePkg.dsc
> > +++ b/MdePkg/MdePkg.dsc
> > @@ -192,5 +192,7 @@ [Components.ARM, Components.AARCH64]
> >
> >  [Components.RISCV64]
> >    MdePkg/Library/BaseRiscVSbiLib/BaseRiscVSbiLib.inf
> > +
> > + MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSb
> > + iLib.inf
> > + MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRisc
> > + VSbiLibRam.inf
> >
> >  [BuildOptions]
> > diff --git
> > a/MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSb
> > iLib.inf
> > b/MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSb
> > iLib.inf
> > new file mode 100644
> > index 000000000000..09cf59f190f6
> > --- /dev/null
> > +++ b/MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRis
> > +++ cVSbiLib.inf
> > @@ -0,0 +1,39 @@
> > +## @file
> > +#  Serial Port Library backed by SBI console.
> > +#
> > +#  Meant for SEC and PEI (XIP) environments.
> > +#
> > +#  Due to limitations of SBI console interface and XIP environments #
> > +(on use of globals), this library instance does not implement reading
> > +#  and polling the serial port. See PrePiDxeSerialPortLibRiscVSbi for
> > +#  the full-featured variant meant for PrePi and DXE environments.
> > +#
> > +#  Copyright (c) 2023, Intel Corporation. All rights reserved.<BR> #
> > +#  SPDX-License-Identifier: BSD-2-Clause-Patent # # ##
> > +
> > +[Defines]
> > +  INF_VERSION                    = 0x0001001B
> > +  BASE_NAME                      = BaseSerialPortLibRiscVSbiLib
> > +  MODULE_UNI_FILE                = BaseSerialPortLibRiscVSbiLib.uni
> > +  FILE_GUID                      = 639fad38-4bfd-4eb9-9f09-e97c7947d480
> > +  MODULE_TYPE                    = BASE
> > +  VERSION_STRING                 = 1.0
> > +  LIBRARY_CLASS                  = SerialPortLib | SEC PEI_CORE PEIM
> > +
> > +
> > +#
> > +#  VALID_ARCHITECTURES           = RISCV64
> > +#
> > +
> > +[Sources]
> > +  BaseSerialPortLibRiscVSbiLib.c
> > +
> > +[Packages]
> > +  MdePkg/MdePkg.dec
> > +
> > +[LibraryClasses]
> > +  RiscVSbiLib
> > diff --git
> > a/MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRisc
> > VSbiLibRam.inf
> > b/MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLibRisc
> > VSbiLibRam.inf
> > new file mode 100644
> > index 000000000000..b7ad1548a309
> > --- /dev/null
> > +++ b/MdePkg/Library/BaseSerialPortLibRiscVSbiLibRam/BaseSerialPortLib
> > +++ RiscVSbiLibRam.inf
> > @@ -0,0 +1,36 @@
> > +## @file
> > +#  Serial Port Library backed by SBI console.
> > +#
> > +#  Meant for PrePi and DXE environments (where globals are allowed).
> > +See #  BaseSerialPortLibRiscVSbiLib for a reduced variant appropriate
> > +for SEC #  and PEI (XIP) environments.
> > +#
> > +#  Copyright (c) 2023, Intel Corporation. All rights reserved.<BR> #
> > +#  SPDX-License-Identifier: BSD-2-Clause-Patent # # ##
> > +
> > +[Defines]
> > +  INF_VERSION                    = 0x0001001B
> > +  BASE_NAME                      = BaseSerialPortLibRiscVSbiLibRam
> > +  MODULE_UNI_FILE                = BaseSerialPortLibRiscVSbiLibRam.uni
> > +  FILE_GUID                      = 872af743-ab56-45b4-a065-602567f4820c
> > +  MODULE_TYPE                    = BASE
> > +  VERSION_STRING                 = 1.0
> > +  LIBRARY_CLASS                  = SerialPortLib | SEC DXE_CORE DXE_DRIVER
> DXE_RUNTIME_DRIVER UEFI_DRIVER UEFI_APPLICATION
> > +
> > +
> > +#
> > +#  VALID_ARCHITECTURES           = RISCV64
> > +#
> > +
> > +[Sources]
> > +  BaseSerialPortLibRiscVSbiLibRam.c
> > +
> > +[Packages]
> > +  MdePkg/MdePkg.dec
> > +
> > +[LibraryClasses]
> > +  RiscVSbiLib
> > diff --git
> > a/MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSb
> > iLib.c
> > b/MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRiscVSb
> > iLib.c
> > new file mode 100644
> > index 000000000000..0ad5931be3ae
> > --- /dev/null
> > +++ b/MdePkg/Library/BaseSerialPortLibRiscVSbiLib/BaseSerialPortLibRis
> > +++ cVSbiLib.c
> > @@ -0,0 +1,233 @@
> > +/** @file
> > +  Serial Port Library backed by SBI console.
> > +
> > +  Meant for SEC and PEI (XIP) environments.
> > +
> > +  Due to limitations of SBI console interface and XIP environments
> > + (on use of globals), this library instance does not implement
> > + reading  and polling the serial port. See
> > + PrePiDxeSerialPortLibRiscVSbi for  the full-featured variant meant for
> PrePi and DXE environments.
> > +
> > +  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>
> > +
> > +/**
> > +  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
> > +  )
> > +{
> > +  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
> > +  )
> > +{
> > +  SBI_RET  Ret;
> > +  UINTN    Index;
> > +
> > +  Ret = SbiCall (SBI_EXT_BASE, SBI_EXT_BASE_PROBE_EXT, 1,
> > + SBI_EXT_DBCN);  if ((TranslateError (Ret.Error) == EFI_SUCCESS) &&
> > +      (Ret.Value != 0))
> > +  {
> > +    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;
> > +  }
> > +
> > +  Ret = SbiCall (SBI_EXT_BASE, SBI_EXT_BASE_PROBE_EXT, 1,
> > + SBI_EXT_0_1_CONSOLE_PUTCHAR);  if ((TranslateError (Ret.Error) ==
> EFI_SUCCESS) &&
> > +      (Ret.Value != 0))
> > +  {
> > +    for (Index = 0; Index < NumberOfBytes; Index++) {
> > +      SbiCall (SBI_EXT_0_1_CONSOLE_PUTCHAR, 0, 1, Buffer[Index]);
> What happens if it fails? Shouldn't it return immediately instead of
> continuing with the loop?
> 
> > +    }
> > +
> > +    return Index;
> > +  }
> > +
> > +  /*
> > +   * Neither DBCN or legacy extension were present.
> > +   */
> > +  return NumberOfBytes;
> Isn't it an error? And for earlier DBCN_WRITE error, 0 is returned. So,
> shouldn't we return 0 here also instead of NumberOfBytes?
> 
> Same questions for similar place in other library also.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102662): https://edk2.groups.io/g/devel/message/102662
Mute This Topic: https://groups.io/mt/98064147/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-