public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [PATCH 0/1] MdeModulePkg: Load Serial driver in early DXE
@ 2024-05-07 13:09 Borzeszkowski, Alan
  2024-05-07 13:09 ` [edk2-devel] [PATCH 1/1] " Borzeszkowski, Alan
  0 siblings, 1 reply; 6+ messages in thread
From: Borzeszkowski, Alan @ 2024-05-07 13:09 UTC (permalink / raw)
  To: devel; +Cc: Alan Borzeszkowski

On Intel platforms, we use LPSS UART for debug prints in DXE phase. Current implementation involves using custom driver.
In order to reduce code maintenance cost and flash usage, we want to switch to EDK2 Serial driver.
Given that PciSioSerialDxe is a UEFI driver and for purposes of loading this driver in early DXE,
new driver entrypoint with separate .inf file was added. This way, Serial driver can be treated as DXE driver.
Also, several smaller changes were made in Serial.c and SerialIo.c to enable debug prints.

Change was tested on Intel platform, debug prints appeared shortly after DXE phase begun.
https://github.com/tianocore/edk2/pull/5542

Alan Borzeszkowski (1):
  MdeModulePkg: Load Serial driver in early DXE

 .../PciSioSerialDxe/PciSioSerialDxeEarly.inf  |  81 ++++++++
 MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c | 184 ++++++++++++++----
 .../Bus/Pci/PciSioSerialDxe/SerialIo.c        |  16 +-
 MdeModulePkg/MdeModulePkg.dsc                 |   1 +
 4 files changed, 245 insertions(+), 37 deletions(-)
 create mode 100644 MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxeEarly.inf

-- 
2.34.1

---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Spolka oswiadcza, ze posiada status duzego przedsiebiorcy w rozumieniu ustawy z dnia 8 marca 2013 r. o przeciwdzialaniu nadmiernym opoznieniom w transakcjach handlowych.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118643): https://edk2.groups.io/g/devel/message/118643
Mute This Topic: https://groups.io/mt/105959578/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [edk2-devel] [PATCH 1/1] MdeModulePkg: Load Serial driver in early DXE
  2024-05-07 13:09 [edk2-devel] [PATCH 0/1] MdeModulePkg: Load Serial driver in early DXE Borzeszkowski, Alan
@ 2024-05-07 13:09 ` Borzeszkowski, Alan
  2024-05-08  3:27   ` Ni, Ray
  0 siblings, 1 reply; 6+ messages in thread
From: Borzeszkowski, Alan @ 2024-05-07 13:09 UTC (permalink / raw)
  To: devel; +Cc: Alan Borzeszkowski, Zhichao Gao, Ray Ni, Michael D Kinney

For the purpose of UEFI debug prints enablement in early DXE,
Serial driver should load earlier. To comply with EDK2 specification
and maximize code reuse, new driver entrypoint was created
and separate .inf file was added.

Cc: Zhichao Gao <zhichao.gao@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Alan Borzeszkowski <alan.borzeszkowski@intel.com>
---
 .../PciSioSerialDxe/PciSioSerialDxeEarly.inf  |  81 ++++++++
 MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c | 184 ++++++++++++++----
 .../Bus/Pci/PciSioSerialDxe/SerialIo.c        |  16 +-
 MdeModulePkg/MdeModulePkg.dsc                 |   1 +
 4 files changed, 245 insertions(+), 37 deletions(-)
 create mode 100644 MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxeEarly.inf

diff --git a/MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxeEarly.inf b/MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxeEarly.inf
new file mode 100644
index 0000000000..1b88f7a1cc
--- /dev/null
+++ b/MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxeEarly.inf
@@ -0,0 +1,81 @@
+## @file
+# Serial driver for standard UARTS on a SIO chip or PCI/PCIE card.
+#
+# Produces the Serial I/O protocol for standard UARTS using Super I/O or PCI I/O.
+#
+# This is Early DXE version of Serial driver. It's intended use is for debug purposes
+# in early DXE. There is added dependency on either Super I/O Protocol or PCI I/O Protocol.
+#
+# Copyright (c) 2007 - 2024, Intel Corporation. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = PciSioSerialDxeEarly
+  MODULE_UNI_FILE                = PciSioSerialDxe.uni
+  FILE_GUID                      = E47C4BFB-9CCC-47B6-A3C1-79C3FBF8B6C8
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = DxePciSioSerialDriverEntrypoint
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = IA32 X64 EBC
+#
+#  DRIVER_BINDING                =  gSerialControllerDriver
+#  COMPONENT_NAME                =  gPciSioSerialComponentName
+#  COMPONENT_NAME2               =  gPciSioSerialComponentName2
+#
+
+[Sources]
+  ComponentName.c
+  SerialIo.c
+  Serial.h
+  Serial.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+  PcdLib
+  ReportStatusCodeLib
+  UefiBootServicesTableLib
+  MemoryAllocationLib
+  BaseMemoryLib
+  DevicePathLib
+  UefiLib
+  UefiDriverEntryPoint
+  DebugLib
+  IoLib
+
+[Guids]
+  gEfiUartDevicePathGuid                        ## SOMETIMES_CONSUMES   ## GUID
+
+[Protocols]
+  gEfiSioProtocolGuid                           ## TO_START
+  gEfiDevicePathProtocolGuid                    ## TO_START
+  gEfiPciIoProtocolGuid                         ## TO_START
+  gEfiSerialIoProtocolGuid                      ## BY_START
+  gEfiDevicePathProtocolGuid                    ## BY_START
+
+[FeaturePcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseHalfHandshake|FALSE   ## CONSUMES
+
+[Pcd]
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200    ## CONSUMES
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits|8         ## CONSUMES
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity|1           ## CONSUMES
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits|1         ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|1843200 ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPciSerialParameters     ## CONSUMES
+
+[UserExtensions.TianoCore."ExtraFiles"]
+  PciSioSerialDxeExtra.uni
+
+[Depex]
+  gEfiSioProtocolGuid OR gEfiPciIoProtocolGuid
diff --git a/MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c b/MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c
index 8b1ce70118..cc77d12748 100644
--- a/MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c
+++ b/MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c
@@ -156,6 +156,7 @@ InitializePciSioSerial (
 /**
   Return whether the controller is a SIO serial controller.
 
+  @param  This         A pointer to the EFI_DRIVER_BINDING_PROTOCOL instance.
   @param  Controller   The controller handle.
 
   @retval EFI_SUCCESS  The controller is a SIO serial controller.
@@ -163,7 +164,8 @@ InitializePciSioSerial (
 **/
 EFI_STATUS
 IsSioSerialController (
-  EFI_HANDLE  Controller
+  IN EFI_DRIVER_BINDING_PROTOCOL  *This,
+  EFI_HANDLE                      Controller
   )
 {
   EFI_STATUS                Status;
@@ -178,7 +180,7 @@ IsSioSerialController (
                   Controller,
                   &gEfiSioProtocolGuid,
                   (VOID **)&Sio,
-                  gSerialControllerDriver.DriverBindingHandle,
+                  This->DriverBindingHandle,
                   Controller,
                   EFI_OPEN_PROTOCOL_BY_DRIVER
                   );
@@ -193,7 +195,7 @@ IsSioSerialController (
     gBS->CloseProtocol (
            Controller,
            &gEfiSioProtocolGuid,
-           gSerialControllerDriver.DriverBindingHandle,
+           This->DriverBindingHandle,
            Controller
            );
 
@@ -201,7 +203,7 @@ IsSioSerialController (
                     Controller,
                     &gEfiDevicePathProtocolGuid,
                     (VOID **)&DevicePath,
-                    gSerialControllerDriver.DriverBindingHandle,
+                    This->DriverBindingHandle,
                     Controller,
                     EFI_OPEN_PROTOCOL_BY_DRIVER
                     );
@@ -228,7 +230,7 @@ IsSioSerialController (
     gBS->CloseProtocol (
            Controller,
            &gEfiDevicePathProtocolGuid,
-           gSerialControllerDriver.DriverBindingHandle,
+           This->DriverBindingHandle,
            Controller
            );
   }
@@ -239,14 +241,16 @@ IsSioSerialController (
 /**
   Return whether the controller is a PCI serial controller.
 
-  @param  Controller   The controller handle.
+  @param  This            A pointer to the EFI_DRIVER_BINDING_PROTOCOL instance.
+  @param  Controller      The controller handle.
 
   @retval EFI_SUCCESS  The controller is a PCI serial controller.
   @retval others       The controller is not a PCI serial controller.
 **/
 EFI_STATUS
 IsPciSerialController (
-  EFI_HANDLE  Controller
+  IN EFI_DRIVER_BINDING_PROTOCOL  *This,
+  EFI_HANDLE                      Controller
   )
 {
   EFI_STATUS                Status;
@@ -262,7 +266,7 @@ IsPciSerialController (
                   Controller,
                   &gEfiPciIoProtocolGuid,
                   (VOID **)&PciIo,
-                  gSerialControllerDriver.DriverBindingHandle,
+                  This->DriverBindingHandle,
                   Controller,
                   EFI_OPEN_PROTOCOL_BY_DRIVER
                   );
@@ -301,7 +305,7 @@ IsPciSerialController (
     gBS->CloseProtocol (
            Controller,
            &gEfiPciIoProtocolGuid,
-           gSerialControllerDriver.DriverBindingHandle,
+           This->DriverBindingHandle,
            Controller
            );
   }
@@ -317,7 +321,7 @@ IsPciSerialController (
                   Controller,
                   &gEfiDevicePathProtocolGuid,
                   (VOID **)&DevicePath,
-                  gSerialControllerDriver.DriverBindingHandle,
+                  This->DriverBindingHandle,
                   Controller,
                   EFI_OPEN_PROTOCOL_BY_DRIVER
                   );
@@ -329,7 +333,7 @@ IsPciSerialController (
   gBS->CloseProtocol (
          Controller,
          &gEfiDevicePathProtocolGuid,
-         gSerialControllerDriver.DriverBindingHandle,
+         This->DriverBindingHandle,
          Controller
          );
 
@@ -348,12 +352,11 @@ IsPciSerialController (
 **/
 EFI_STATUS
 EFIAPI
-SerialControllerDriverSupported (
+SerialDriverSupported (
   IN EFI_DRIVER_BINDING_PROTOCOL  *This,
   IN EFI_HANDLE                   Controller,
   IN EFI_DEVICE_PATH_PROTOCOL     *RemainingDevicePath
   )
-
 {
   EFI_STATUS                     Status;
   UART_DEVICE_PATH               *Uart;
@@ -393,17 +396,40 @@ SerialControllerDriverSupported (
     }
   }
 
-  Status = IsSioSerialController (Controller);
+  Status = IsSioSerialController (This, Controller);
   if (EFI_ERROR (Status)) {
-    Status = IsPciSerialController (Controller);
+    Status = IsPciSerialController (This, Controller);
   }
 
   return Status;
 }
 
+/**
+  Check to see if this driver supports the given controller
+
+  @param  This                 A pointer to the EFI_DRIVER_BINDING_PROTOCOL instance.
+  @param  Controller           The handle of the controller to test.
+  @param  RemainingDevicePath  A pointer to the remaining portion of a device path.
+
+  @return EFI_SUCCESS          This driver can support the given controller
+
+**/
+EFI_STATUS
+EFIAPI
+SerialControllerDriverSupported (
+  IN EFI_DRIVER_BINDING_PROTOCOL  *This,
+  IN EFI_HANDLE                   Controller,
+  IN EFI_DEVICE_PATH_PROTOCOL     *RemainingDevicePath
+  )
+
+{
+  return SerialDriverSupported (This, Controller, RemainingDevicePath);
+}
+
 /**
   Create the child serial device instance.
 
+  @param  This                A pointer to the EFI_DRIVER_BINDING_PROTOCOL instance.
   @param Controller           The parent controller handle.
   @param Uart                 Pointer to the UART device path node in RemainingDevicePath,
                               or NULL if RemainingDevicePath is NULL.
@@ -423,14 +449,15 @@ SerialControllerDriverSupported (
 **/
 EFI_STATUS
 CreateSerialDevice (
-  IN EFI_HANDLE                Controller,
-  IN UART_DEVICE_PATH          *Uart,
-  IN EFI_DEVICE_PATH_PROTOCOL  *ParentDevicePath,
-  IN BOOLEAN                   CreateControllerNode,
-  IN UINT32                    Instance,
-  IN PARENT_IO_PROTOCOL_PTR    ParentIo,
-  IN PCI_SERIAL_PARAMETER      *PciSerialParameter  OPTIONAL,
-  IN PCI_DEVICE_INFO           *PciDeviceInfo       OPTIONAL
+  IN EFI_DRIVER_BINDING_PROTOCOL  *This,
+  IN EFI_HANDLE                   Controller,
+  IN UART_DEVICE_PATH             *Uart,
+  IN EFI_DEVICE_PATH_PROTOCOL     *ParentDevicePath,
+  IN BOOLEAN                      CreateControllerNode,
+  IN UINT32                       Instance,
+  IN PARENT_IO_PROTOCOL_PTR       ParentIo,
+  IN PCI_SERIAL_PARAMETER         *PciSerialParameter  OPTIONAL,
+  IN PCI_DEVICE_INFO              *PciDeviceInfo       OPTIONAL
   )
 {
   EFI_STATUS                                  Status;
@@ -685,7 +712,7 @@ CreateSerialDevice (
                   Controller,
                   PciSerialParameter != NULL ? &gEfiPciIoProtocolGuid : &gEfiSioProtocolGuid,
                   (VOID **)&ParentIo,
-                  gSerialControllerDriver.DriverBindingHandle,
+                  This->DriverBindingHandle,
                   SerialDevice->Handle,
                   EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER
                   );
@@ -720,6 +747,7 @@ CreateError:
 /**
   Returns an array of pointers containing all the child serial device pointers.
 
+  @param  This           A pointer to the EFI_DRIVER_BINDING_PROTOCOL instance.
   @param Controller      The parent controller handle.
   @param IoProtocolGuid  The protocol GUID, either equals to gEfiSioProtocolGuid
                          or equals to gEfiPciIoProtocolGuid.
@@ -729,9 +757,10 @@ CreateError:
 **/
 SERIAL_DEV **
 GetChildSerialDevices (
-  IN EFI_HANDLE  Controller,
-  IN EFI_GUID    *IoProtocolGuid,
-  OUT UINTN      *Count
+  IN EFI_DRIVER_BINDING_PROTOCOL  *This,
+  IN EFI_HANDLE                   Controller,
+  IN EFI_GUID                     *IoProtocolGuid,
+  OUT UINTN                       *Count
   )
 {
   EFI_STATUS                           Status;
@@ -768,7 +797,7 @@ GetChildSerialDevices (
                       OpenInfoBuffer[Index].ControllerHandle,
                       &gEfiSerialIoProtocolGuid,
                       (VOID **)&SerialIo,
-                      gSerialControllerDriver.DriverBindingHandle,
+                      This->DriverBindingHandle,
                       Controller,
                       EFI_OPEN_PROTOCOL_GET_PROTOCOL
                       );
@@ -778,7 +807,7 @@ GetChildSerialDevices (
     }
 
     if ((OpenInfoBuffer[Index].Attributes & EFI_OPEN_PROTOCOL_BY_DRIVER) != 0) {
-      ASSERT (OpenInfoBuffer[Index].AgentHandle == gSerialControllerDriver.DriverBindingHandle);
+      ASSERT (OpenInfoBuffer[Index].AgentHandle == This->DriverBindingHandle);
       OpenByDriver = TRUE;
     }
   }
@@ -793,7 +822,7 @@ GetChildSerialDevices (
 }
 
 /**
-  Start to management the controller passed in
+  Start to manage the controller passed in
 
   @param  This                 A pointer to the EFI_DRIVER_BINDING_PROTOCOL instance.
   @param  Controller           The handle of the controller to test.
@@ -803,7 +832,7 @@ GetChildSerialDevices (
 **/
 EFI_STATUS
 EFIAPI
-SerialControllerDriverStart (
+SerialDriverStart (
   IN EFI_DRIVER_BINDING_PROTOCOL  *This,
   IN EFI_HANDLE                   Controller,
   IN EFI_DEVICE_PATH_PROTOCOL     *RemainingDevicePath
@@ -890,7 +919,7 @@ SerialControllerDriverStart (
 
   ControllerNumber       = 0;
   ContainsControllerNode = FALSE;
-  SerialDevices          = GetChildSerialDevices (Controller, IoProtocolGuid, &SerialDeviceCount);
+  SerialDevices          = GetChildSerialDevices (This, Controller, IoProtocolGuid, &SerialDeviceCount);
 
   if (SerialDeviceCount != 0) {
     if (RemainingDevicePath == NULL) {
@@ -991,7 +1020,7 @@ SerialControllerDriverStart (
         Node = NextDevicePathNode (Node);
       } while (!IsDevicePathEnd (Node));
 
-      Status = CreateSerialDevice (Controller, Uart, ParentDevicePath, FALSE, Acpi->UID, ParentIo, NULL, NULL);
+      Status = CreateSerialDevice (This, Controller, Uart, ParentDevicePath, FALSE, Acpi->UID, ParentIo, NULL, NULL);
       DEBUG ((DEBUG_INFO, "PciSioSerial: Create SIO child serial device - %r\n", Status));
     }
   } else {
@@ -1073,7 +1102,7 @@ SerialControllerDriverStart (
             PciSerialParameter = PcdGetPtr (PcdPciSerialParameters);
           }
 
-          Status = CreateSerialDevice (Controller, Uart, ParentDevicePath, FALSE, 0, ParentIo, PciSerialParameter, PciDeviceInfo);
+          Status = CreateSerialDevice (This, Controller, Uart, ParentDevicePath, FALSE, 0, ParentIo, PciSerialParameter, PciDeviceInfo);
           DEBUG ((DEBUG_INFO, "PciSioSerial: Create PCI child serial device (single) - %r\n", Status));
           if (!EFI_ERROR (Status)) {
             PciDeviceInfo->ChildCount++;
@@ -1094,7 +1123,7 @@ SerialControllerDriverStart (
               //
               // Create controller node when PCI serial device contains multiple UARTs
               //
-              Status = CreateSerialDevice (Controller, Uart, ParentDevicePath, TRUE, PciSerialCount, ParentIo, PciSerialParameter, PciDeviceInfo);
+              Status = CreateSerialDevice (This, Controller, Uart, ParentDevicePath, TRUE, PciSerialCount, ParentIo, PciSerialParameter, PciDeviceInfo);
               PciSerialCount++;
               DEBUG ((DEBUG_INFO, "PciSioSerial: Create PCI child serial device (multiple) - %r\n", Status));
               if (!EFI_ERROR (Status)) {
@@ -1147,6 +1176,91 @@ SerialControllerDriverStart (
   return Status;
 }
 
+/**
+  Start to manage the controller passed in
+
+  @param  This                 A pointer to the EFI_DRIVER_BINDING_PROTOCOL instance.
+  @param  Controller           The handle of the controller to test.
+  @param  RemainingDevicePath  A pointer to the remaining portion of a device path.
+
+  @return EFI_SUCCESS   Driver is started successfully
+**/
+EFI_STATUS
+EFIAPI
+SerialControllerDriverStart (
+  IN EFI_DRIVER_BINDING_PROTOCOL  *This,
+  IN EFI_HANDLE                   Controller,
+  IN EFI_DEVICE_PATH_PROTOCOL     *RemainingDevicePath
+  )
+{
+  return SerialDriverStart (This, Controller, RemainingDevicePath);
+}
+
+/**
+  Initialize Serial driver in DXE phase
+
+  @param ImageHandle  Handle for the image of this driver.
+  @param SystemTable  Pointer to the EFI System Table.
+
+  @return EFI_SUCCESS   Driver is started successfully
+**/
+EFI_STATUS
+EFIAPI
+DxePciSioSerialDriverEntrypoint (
+  IN EFI_HANDLE        ImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS                   Status;
+  EFI_DEVICE_PATH_PROTOCOL     *EfiDevicePath;
+  EFI_HANDLE                   *HandleBuffer;
+  UINTN                        NumberOfHandles;
+  EFI_DRIVER_BINDING_PROTOCOL  *SerialControllerDriver;
+  UINTN                        Index;
+
+  EfiDevicePath = NULL;
+  HandleBuffer  = NULL;
+  //
+  // Initialize the serial controller instance
+  //
+  SerialControllerDriver = AllocateCopyPool (sizeof (EFI_DRIVER_BINDING_PROTOCOL), &gSerialControllerDriver);
+  ASSERT (SerialControllerDriver != NULL);
+
+  SerialControllerDriver->DriverBindingHandle = ImageHandle;
+  //
+  // Initialize UART default setting in gSerialDevTemplate
+  //
+  gSerialDevTemplate.SerialMode.BaudRate     = PcdGet64 (PcdUartDefaultBaudRate);
+  gSerialDevTemplate.SerialMode.DataBits     = PcdGet8 (PcdUartDefaultDataBits);
+  gSerialDevTemplate.SerialMode.Parity       = PcdGet8 (PcdUartDefaultParity);
+  gSerialDevTemplate.SerialMode.StopBits     = PcdGet8 (PcdUartDefaultStopBits);
+  gSerialDevTemplate.UartDevicePath.BaudRate = PcdGet64 (PcdUartDefaultBaudRate);
+  gSerialDevTemplate.UartDevicePath.DataBits = PcdGet8 (PcdUartDefaultDataBits);
+  gSerialDevTemplate.UartDevicePath.Parity   = PcdGet8 (PcdUartDefaultParity);
+  gSerialDevTemplate.UartDevicePath.StopBits = PcdGet8 (PcdUartDefaultStopBits);
+  gSerialDevTemplate.ClockRate               = PcdGet32 (PcdSerialClockRate);
+
+  // Check all handles for compatible Device Path and appropriate I/O Protocol
+  Status = gBS->LocateHandleBuffer (ByProtocol, &gEfiDevicePathProtocolGuid, NULL, &NumberOfHandles, &HandleBuffer);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  for (Index = 0; Index < NumberOfHandles; Index++) {
+    EfiDevicePath = DevicePathFromHandle (HandleBuffer[Index]);
+    Status        = SerialDriverSupported (SerialControllerDriver, HandleBuffer[Index], EfiDevicePath);
+    DEBUG ((DEBUG_INFO, "SerialDriverSupported status: %r\n", Status));
+    if (EFI_ERROR (Status)) {
+      continue;
+    }
+
+    Status = SerialDriverStart (SerialControllerDriver, HandleBuffer[Index], EfiDevicePath);
+    DEBUG ((DEBUG_INFO, "SerialDriverStart status: %r\n", Status));
+  }
+
+  return EFI_SUCCESS;
+}
+
 /**
   Disconnect this driver with the controller, uninstall related protocol instance
 
diff --git a/MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c b/MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c
index 8a85a6c3b8..c052d43b84 100644
--- a/MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c
+++ b/MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c
@@ -1367,7 +1367,13 @@ SerialReadRegister (
   EFI_STATUS  Status;
 
   if (SerialDev->PciDeviceInfo == NULL) {
-    return IoRead8 ((UINTN)SerialDev->BaseAddress + Offset * SerialDev->RegisterStride);
+    if (SerialDev->MmioAccess)
+    {
+      return MmioRead8((UINTN)SerialDev->BaseAddress + Offset * SerialDev->RegisterStride);
+    }
+    else {
+      return IoRead8 ((UINTN)SerialDev->BaseAddress + Offset * SerialDev->RegisterStride);
+    }
   } else {
     if (SerialDev->MmioAccess) {
       Status = SerialDev->PciDeviceInfo->PciIo->Mem.Read (
@@ -1411,7 +1417,13 @@ SerialWriteRegister (
   EFI_STATUS  Status;
 
   if (SerialDev->PciDeviceInfo == NULL) {
-    IoWrite8 ((UINTN)SerialDev->BaseAddress + Offset * SerialDev->RegisterStride, Data);
+    if (SerialDev->MmioAccess)
+    {
+      MmioWrite8((UINTN)SerialDev->BaseAddress + Offset * SerialDev->RegisterStride, Data);
+    }
+    else {
+      IoWrite8 ((UINTN)SerialDev->BaseAddress + Offset * SerialDev->RegisterStride, Data);
+    }
   } else {
     if (SerialDev->MmioAccess) {
       Status = SerialDev->PciDeviceInfo->PciIo->Mem.Write (
diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index c0f1df3bfb..b209fe2696 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -240,6 +240,7 @@
 
   MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
   MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
+  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxeEarly.inf
   MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
   MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupportDxe.inf
   MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf
-- 
2.34.1

---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Spolka oswiadcza, ze posiada status duzego przedsiebiorcy w rozumieniu ustawy z dnia 8 marca 2013 r. o przeciwdzialaniu nadmiernym opoznieniom w transakcjach handlowych.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118644): https://edk2.groups.io/g/devel/message/118644
Mute This Topic: https://groups.io/mt/105959587/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [edk2-devel] [PATCH 1/1] MdeModulePkg: Load Serial driver in early DXE
  2024-05-07 13:09 ` [edk2-devel] [PATCH 1/1] " Borzeszkowski, Alan
@ 2024-05-08  3:27   ` Ni, Ray
  2024-05-08 13:24     ` Borzeszkowski, Alan
  0 siblings, 1 reply; 6+ messages in thread
From: Ni, Ray @ 2024-05-08  3:27 UTC (permalink / raw)
  To: Borzeszkowski, Alan, devel@edk2.groups.io; +Cc: Gao, Zhichao, Kinney, Michael D

[-- Attachment #1: Type: text/plain, Size: 22948 bytes --]

Why not implement a customized SerialPortLib and use MdeModulePkg/Universal/SerialDxe/SerialDxe.inf?

Thanks,
Ray
________________________________
From: Borzeszkowski, Alan <alan.borzeszkowski@intel.com>
Sent: Tuesday, May 7, 2024 21:09
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Borzeszkowski, Alan <alan.borzeszkowski@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: [PATCH 1/1] MdeModulePkg: Load Serial driver in early DXE

For the purpose of UEFI debug prints enablement in early DXE,
Serial driver should load earlier. To comply with EDK2 specification
and maximize code reuse, new driver entrypoint was created
and separate .inf file was added.

Cc: Zhichao Gao <zhichao.gao@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Alan Borzeszkowski <alan.borzeszkowski@intel.com>
---
 .../PciSioSerialDxe/PciSioSerialDxeEarly.inf  |  81 ++++++++
 MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c | 184 ++++++++++++++----
 .../Bus/Pci/PciSioSerialDxe/SerialIo.c        |  16 +-
 MdeModulePkg/MdeModulePkg.dsc                 |   1 +
 4 files changed, 245 insertions(+), 37 deletions(-)
 create mode 100644 MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxeEarly.inf

diff --git a/MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxeEarly.inf b/MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxeEarly.inf
new file mode 100644
index 0000000000..1b88f7a1cc
--- /dev/null
+++ b/MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxeEarly.inf
@@ -0,0 +1,81 @@
+## @file

+# Serial driver for standard UARTS on a SIO chip or PCI/PCIE card.

+#

+# Produces the Serial I/O protocol for standard UARTS using Super I/O or PCI I/O.

+#

+# This is Early DXE version of Serial driver. It's intended use is for debug purposes

+# in early DXE. There is added dependency on either Super I/O Protocol or PCI I/O Protocol.

+#

+# Copyright (c) 2007 - 2024, Intel Corporation. All rights reserved.<BR>

+#

+# SPDX-License-Identifier: BSD-2-Clause-Patent

+#

+##

+

+[Defines]

+  INF_VERSION                    = 0x00010005

+  BASE_NAME                      = PciSioSerialDxeEarly

+  MODULE_UNI_FILE                = PciSioSerialDxe.uni

+  FILE_GUID                      = E47C4BFB-9CCC-47B6-A3C1-79C3FBF8B6C8

+  MODULE_TYPE                    = DXE_DRIVER

+  VERSION_STRING                 = 1.0

+  ENTRY_POINT                    = DxePciSioSerialDriverEntrypoint

+

+#

+# The following information is for reference only and not required by the build tools.

+#

+#  VALID_ARCHITECTURES           = IA32 X64 EBC

+#

+#  DRIVER_BINDING                =  gSerialControllerDriver

+#  COMPONENT_NAME                =  gPciSioSerialComponentName

+#  COMPONENT_NAME2               =  gPciSioSerialComponentName2

+#

+

+[Sources]

+  ComponentName.c

+  SerialIo.c

+  Serial.h

+  Serial.c

+

+[Packages]

+  MdePkg/MdePkg.dec

+  MdeModulePkg/MdeModulePkg.dec

+

+[LibraryClasses]

+  PcdLib

+  ReportStatusCodeLib

+  UefiBootServicesTableLib

+  MemoryAllocationLib

+  BaseMemoryLib

+  DevicePathLib

+  UefiLib

+  UefiDriverEntryPoint

+  DebugLib

+  IoLib

+

+[Guids]

+  gEfiUartDevicePathGuid                        ## SOMETIMES_CONSUMES   ## GUID

+

+[Protocols]

+  gEfiSioProtocolGuid                           ## TO_START

+  gEfiDevicePathProtocolGuid                    ## TO_START

+  gEfiPciIoProtocolGuid                         ## TO_START

+  gEfiSerialIoProtocolGuid                      ## BY_START

+  gEfiDevicePathProtocolGuid                    ## BY_START

+

+[FeaturePcd]

+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseHalfHandshake|FALSE   ## CONSUMES

+

+[Pcd]

+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200    ## CONSUMES

+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits|8         ## CONSUMES

+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity|1           ## CONSUMES

+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits|1         ## CONSUMES

+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|1843200 ## CONSUMES

+  gEfiMdeModulePkgTokenSpaceGuid.PcdPciSerialParameters     ## CONSUMES

+

+[UserExtensions.TianoCore."ExtraFiles"]

+  PciSioSerialDxeExtra.uni

+

+[Depex]

+  gEfiSioProtocolGuid OR gEfiPciIoProtocolGuid

diff --git a/MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c b/MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c
index 8b1ce70118..cc77d12748 100644
--- a/MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c
+++ b/MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c
@@ -156,6 +156,7 @@ InitializePciSioSerial (
 /**

   Return whether the controller is a SIO serial controller.



+  @param  This         A pointer to the EFI_DRIVER_BINDING_PROTOCOL instance.

   @param  Controller   The controller handle.



   @retval EFI_SUCCESS  The controller is a SIO serial controller.

@@ -163,7 +164,8 @@ InitializePciSioSerial (
 **/

 EFI_STATUS

 IsSioSerialController (

-  EFI_HANDLE  Controller

+  IN EFI_DRIVER_BINDING_PROTOCOL  *This,

+  EFI_HANDLE                      Controller

   )

 {

   EFI_STATUS                Status;

@@ -178,7 +180,7 @@ IsSioSerialController (
                   Controller,

                   &gEfiSioProtocolGuid,

                   (VOID **)&Sio,

-                  gSerialControllerDriver.DriverBindingHandle,

+                  This->DriverBindingHandle,

                   Controller,

                   EFI_OPEN_PROTOCOL_BY_DRIVER

                   );

@@ -193,7 +195,7 @@ IsSioSerialController (
     gBS->CloseProtocol (

            Controller,

            &gEfiSioProtocolGuid,

-           gSerialControllerDriver.DriverBindingHandle,

+           This->DriverBindingHandle,

            Controller

            );



@@ -201,7 +203,7 @@ IsSioSerialController (
                     Controller,

                     &gEfiDevicePathProtocolGuid,

                     (VOID **)&DevicePath,

-                    gSerialControllerDriver.DriverBindingHandle,

+                    This->DriverBindingHandle,

                     Controller,

                     EFI_OPEN_PROTOCOL_BY_DRIVER

                     );

@@ -228,7 +230,7 @@ IsSioSerialController (
     gBS->CloseProtocol (

            Controller,

            &gEfiDevicePathProtocolGuid,

-           gSerialControllerDriver.DriverBindingHandle,

+           This->DriverBindingHandle,

            Controller

            );

   }

@@ -239,14 +241,16 @@ IsSioSerialController (
 /**

   Return whether the controller is a PCI serial controller.



-  @param  Controller   The controller handle.

+  @param  This            A pointer to the EFI_DRIVER_BINDING_PROTOCOL instance.

+  @param  Controller      The controller handle.



   @retval EFI_SUCCESS  The controller is a PCI serial controller.

   @retval others       The controller is not a PCI serial controller.

 **/

 EFI_STATUS

 IsPciSerialController (

-  EFI_HANDLE  Controller

+  IN EFI_DRIVER_BINDING_PROTOCOL  *This,

+  EFI_HANDLE                      Controller

   )

 {

   EFI_STATUS                Status;

@@ -262,7 +266,7 @@ IsPciSerialController (
                   Controller,

                   &gEfiPciIoProtocolGuid,

                   (VOID **)&PciIo,

-                  gSerialControllerDriver.DriverBindingHandle,

+                  This->DriverBindingHandle,

                   Controller,

                   EFI_OPEN_PROTOCOL_BY_DRIVER

                   );

@@ -301,7 +305,7 @@ IsPciSerialController (
     gBS->CloseProtocol (

            Controller,

            &gEfiPciIoProtocolGuid,

-           gSerialControllerDriver.DriverBindingHandle,

+           This->DriverBindingHandle,

            Controller

            );

   }

@@ -317,7 +321,7 @@ IsPciSerialController (
                   Controller,

                   &gEfiDevicePathProtocolGuid,

                   (VOID **)&DevicePath,

-                  gSerialControllerDriver.DriverBindingHandle,

+                  This->DriverBindingHandle,

                   Controller,

                   EFI_OPEN_PROTOCOL_BY_DRIVER

                   );

@@ -329,7 +333,7 @@ IsPciSerialController (
   gBS->CloseProtocol (

          Controller,

          &gEfiDevicePathProtocolGuid,

-         gSerialControllerDriver.DriverBindingHandle,

+         This->DriverBindingHandle,

          Controller

          );



@@ -348,12 +352,11 @@ IsPciSerialController (
 **/

 EFI_STATUS

 EFIAPI

-SerialControllerDriverSupported (

+SerialDriverSupported (

   IN EFI_DRIVER_BINDING_PROTOCOL  *This,

   IN EFI_HANDLE                   Controller,

   IN EFI_DEVICE_PATH_PROTOCOL     *RemainingDevicePath

   )

-

 {

   EFI_STATUS                     Status;

   UART_DEVICE_PATH               *Uart;

@@ -393,17 +396,40 @@ SerialControllerDriverSupported (
     }

   }



-  Status = IsSioSerialController (Controller);

+  Status = IsSioSerialController (This, Controller);

   if (EFI_ERROR (Status)) {

-    Status = IsPciSerialController (Controller);

+    Status = IsPciSerialController (This, Controller);

   }



   return Status;

 }



+/**

+  Check to see if this driver supports the given controller

+

+  @param  This                 A pointer to the EFI_DRIVER_BINDING_PROTOCOL instance.

+  @param  Controller           The handle of the controller to test.

+  @param  RemainingDevicePath  A pointer to the remaining portion of a device path.

+

+  @return EFI_SUCCESS          This driver can support the given controller

+

+**/

+EFI_STATUS

+EFIAPI

+SerialControllerDriverSupported (

+  IN EFI_DRIVER_BINDING_PROTOCOL  *This,

+  IN EFI_HANDLE                   Controller,

+  IN EFI_DEVICE_PATH_PROTOCOL     *RemainingDevicePath

+  )

+

+{

+  return SerialDriverSupported (This, Controller, RemainingDevicePath);

+}

+

 /**

   Create the child serial device instance.



+  @param  This                A pointer to the EFI_DRIVER_BINDING_PROTOCOL instance.

   @param Controller           The parent controller handle.

   @param Uart                 Pointer to the UART device path node in RemainingDevicePath,

                               or NULL if RemainingDevicePath is NULL.

@@ -423,14 +449,15 @@ SerialControllerDriverSupported (
 **/

 EFI_STATUS

 CreateSerialDevice (

-  IN EFI_HANDLE                Controller,

-  IN UART_DEVICE_PATH          *Uart,

-  IN EFI_DEVICE_PATH_PROTOCOL  *ParentDevicePath,

-  IN BOOLEAN                   CreateControllerNode,

-  IN UINT32                    Instance,

-  IN PARENT_IO_PROTOCOL_PTR    ParentIo,

-  IN PCI_SERIAL_PARAMETER      *PciSerialParameter  OPTIONAL,

-  IN PCI_DEVICE_INFO           *PciDeviceInfo       OPTIONAL

+  IN EFI_DRIVER_BINDING_PROTOCOL  *This,

+  IN EFI_HANDLE                   Controller,

+  IN UART_DEVICE_PATH             *Uart,

+  IN EFI_DEVICE_PATH_PROTOCOL     *ParentDevicePath,

+  IN BOOLEAN                      CreateControllerNode,

+  IN UINT32                       Instance,

+  IN PARENT_IO_PROTOCOL_PTR       ParentIo,

+  IN PCI_SERIAL_PARAMETER         *PciSerialParameter  OPTIONAL,

+  IN PCI_DEVICE_INFO              *PciDeviceInfo       OPTIONAL

   )

 {

   EFI_STATUS                                  Status;

@@ -685,7 +712,7 @@ CreateSerialDevice (
                   Controller,

                   PciSerialParameter != NULL ? &gEfiPciIoProtocolGuid : &gEfiSioProtocolGuid,

                   (VOID **)&ParentIo,

-                  gSerialControllerDriver.DriverBindingHandle,

+                  This->DriverBindingHandle,

                   SerialDevice->Handle,

                   EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER

                   );

@@ -720,6 +747,7 @@ CreateError:
 /**

   Returns an array of pointers containing all the child serial device pointers.



+  @param  This           A pointer to the EFI_DRIVER_BINDING_PROTOCOL instance.

   @param Controller      The parent controller handle.

   @param IoProtocolGuid  The protocol GUID, either equals to gEfiSioProtocolGuid

                          or equals to gEfiPciIoProtocolGuid.

@@ -729,9 +757,10 @@ CreateError:
 **/

 SERIAL_DEV **

 GetChildSerialDevices (

-  IN EFI_HANDLE  Controller,

-  IN EFI_GUID    *IoProtocolGuid,

-  OUT UINTN      *Count

+  IN EFI_DRIVER_BINDING_PROTOCOL  *This,

+  IN EFI_HANDLE                   Controller,

+  IN EFI_GUID                     *IoProtocolGuid,

+  OUT UINTN                       *Count

   )

 {

   EFI_STATUS                           Status;

@@ -768,7 +797,7 @@ GetChildSerialDevices (
                       OpenInfoBuffer[Index].ControllerHandle,

                       &gEfiSerialIoProtocolGuid,

                       (VOID **)&SerialIo,

-                      gSerialControllerDriver.DriverBindingHandle,

+                      This->DriverBindingHandle,

                       Controller,

                       EFI_OPEN_PROTOCOL_GET_PROTOCOL

                       );

@@ -778,7 +807,7 @@ GetChildSerialDevices (
     }



     if ((OpenInfoBuffer[Index].Attributes & EFI_OPEN_PROTOCOL_BY_DRIVER) != 0) {

-      ASSERT (OpenInfoBuffer[Index].AgentHandle == gSerialControllerDriver.DriverBindingHandle);

+      ASSERT (OpenInfoBuffer[Index].AgentHandle == This->DriverBindingHandle);

       OpenByDriver = TRUE;

     }

   }

@@ -793,7 +822,7 @@ GetChildSerialDevices (
 }



 /**

-  Start to management the controller passed in

+  Start to manage the controller passed in



   @param  This                 A pointer to the EFI_DRIVER_BINDING_PROTOCOL instance.

   @param  Controller           The handle of the controller to test.

@@ -803,7 +832,7 @@ GetChildSerialDevices (
 **/

 EFI_STATUS

 EFIAPI

-SerialControllerDriverStart (

+SerialDriverStart (

   IN EFI_DRIVER_BINDING_PROTOCOL  *This,

   IN EFI_HANDLE                   Controller,

   IN EFI_DEVICE_PATH_PROTOCOL     *RemainingDevicePath

@@ -890,7 +919,7 @@ SerialControllerDriverStart (


   ControllerNumber       = 0;

   ContainsControllerNode = FALSE;

-  SerialDevices          = GetChildSerialDevices (Controller, IoProtocolGuid, &SerialDeviceCount);

+  SerialDevices          = GetChildSerialDevices (This, Controller, IoProtocolGuid, &SerialDeviceCount);



   if (SerialDeviceCount != 0) {

     if (RemainingDevicePath == NULL) {

@@ -991,7 +1020,7 @@ SerialControllerDriverStart (
         Node = NextDevicePathNode (Node);

       } while (!IsDevicePathEnd (Node));



-      Status = CreateSerialDevice (Controller, Uart, ParentDevicePath, FALSE, Acpi->UID, ParentIo, NULL, NULL);

+      Status = CreateSerialDevice (This, Controller, Uart, ParentDevicePath, FALSE, Acpi->UID, ParentIo, NULL, NULL);

       DEBUG ((DEBUG_INFO, "PciSioSerial: Create SIO child serial device - %r\n", Status));

     }

   } else {

@@ -1073,7 +1102,7 @@ SerialControllerDriverStart (
             PciSerialParameter = PcdGetPtr (PcdPciSerialParameters);

           }



-          Status = CreateSerialDevice (Controller, Uart, ParentDevicePath, FALSE, 0, ParentIo, PciSerialParameter, PciDeviceInfo);

+          Status = CreateSerialDevice (This, Controller, Uart, ParentDevicePath, FALSE, 0, ParentIo, PciSerialParameter, PciDeviceInfo);

           DEBUG ((DEBUG_INFO, "PciSioSerial: Create PCI child serial device (single) - %r\n", Status));

           if (!EFI_ERROR (Status)) {

             PciDeviceInfo->ChildCount++;

@@ -1094,7 +1123,7 @@ SerialControllerDriverStart (
               //

               // Create controller node when PCI serial device contains multiple UARTs

               //

-              Status = CreateSerialDevice (Controller, Uart, ParentDevicePath, TRUE, PciSerialCount, ParentIo, PciSerialParameter, PciDeviceInfo);

+              Status = CreateSerialDevice (This, Controller, Uart, ParentDevicePath, TRUE, PciSerialCount, ParentIo, PciSerialParameter, PciDeviceInfo);

               PciSerialCount++;

               DEBUG ((DEBUG_INFO, "PciSioSerial: Create PCI child serial device (multiple) - %r\n", Status));

               if (!EFI_ERROR (Status)) {

@@ -1147,6 +1176,91 @@ SerialControllerDriverStart (
   return Status;

 }



+/**

+  Start to manage the controller passed in

+

+  @param  This                 A pointer to the EFI_DRIVER_BINDING_PROTOCOL instance.

+  @param  Controller           The handle of the controller to test.

+  @param  RemainingDevicePath  A pointer to the remaining portion of a device path.

+

+  @return EFI_SUCCESS   Driver is started successfully

+**/

+EFI_STATUS

+EFIAPI

+SerialControllerDriverStart (

+  IN EFI_DRIVER_BINDING_PROTOCOL  *This,

+  IN EFI_HANDLE                   Controller,

+  IN EFI_DEVICE_PATH_PROTOCOL     *RemainingDevicePath

+  )

+{

+  return SerialDriverStart (This, Controller, RemainingDevicePath);

+}

+

+/**

+  Initialize Serial driver in DXE phase

+

+  @param ImageHandle  Handle for the image of this driver.

+  @param SystemTable  Pointer to the EFI System Table.

+

+  @return EFI_SUCCESS   Driver is started successfully

+**/

+EFI_STATUS

+EFIAPI

+DxePciSioSerialDriverEntrypoint (

+  IN EFI_HANDLE        ImageHandle,

+  IN EFI_SYSTEM_TABLE  *SystemTable

+  )

+{

+  EFI_STATUS                   Status;

+  EFI_DEVICE_PATH_PROTOCOL     *EfiDevicePath;

+  EFI_HANDLE                   *HandleBuffer;

+  UINTN                        NumberOfHandles;

+  EFI_DRIVER_BINDING_PROTOCOL  *SerialControllerDriver;

+  UINTN                        Index;

+

+  EfiDevicePath = NULL;

+  HandleBuffer  = NULL;

+  //

+  // Initialize the serial controller instance

+  //

+  SerialControllerDriver = AllocateCopyPool (sizeof (EFI_DRIVER_BINDING_PROTOCOL), &gSerialControllerDriver);

+  ASSERT (SerialControllerDriver != NULL);

+

+  SerialControllerDriver->DriverBindingHandle = ImageHandle;

+  //

+  // Initialize UART default setting in gSerialDevTemplate

+  //

+  gSerialDevTemplate.SerialMode.BaudRate     = PcdGet64 (PcdUartDefaultBaudRate);

+  gSerialDevTemplate.SerialMode.DataBits     = PcdGet8 (PcdUartDefaultDataBits);

+  gSerialDevTemplate.SerialMode.Parity       = PcdGet8 (PcdUartDefaultParity);

+  gSerialDevTemplate.SerialMode.StopBits     = PcdGet8 (PcdUartDefaultStopBits);

+  gSerialDevTemplate.UartDevicePath.BaudRate = PcdGet64 (PcdUartDefaultBaudRate);

+  gSerialDevTemplate.UartDevicePath.DataBits = PcdGet8 (PcdUartDefaultDataBits);

+  gSerialDevTemplate.UartDevicePath.Parity   = PcdGet8 (PcdUartDefaultParity);

+  gSerialDevTemplate.UartDevicePath.StopBits = PcdGet8 (PcdUartDefaultStopBits);

+  gSerialDevTemplate.ClockRate               = PcdGet32 (PcdSerialClockRate);

+

+  // Check all handles for compatible Device Path and appropriate I/O Protocol

+  Status = gBS->LocateHandleBuffer (ByProtocol, &gEfiDevicePathProtocolGuid, NULL, &NumberOfHandles, &HandleBuffer);

+  if (EFI_ERROR (Status)) {

+    return Status;

+  }

+

+  for (Index = 0; Index < NumberOfHandles; Index++) {

+    EfiDevicePath = DevicePathFromHandle (HandleBuffer[Index]);

+    Status        = SerialDriverSupported (SerialControllerDriver, HandleBuffer[Index], EfiDevicePath);

+    DEBUG ((DEBUG_INFO, "SerialDriverSupported status: %r\n", Status));

+    if (EFI_ERROR (Status)) {

+      continue;

+    }

+

+    Status = SerialDriverStart (SerialControllerDriver, HandleBuffer[Index], EfiDevicePath);

+    DEBUG ((DEBUG_INFO, "SerialDriverStart status: %r\n", Status));

+  }

+

+  return EFI_SUCCESS;

+}

+

 /**

   Disconnect this driver with the controller, uninstall related protocol instance



diff --git a/MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c b/MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c
index 8a85a6c3b8..c052d43b84 100644
--- a/MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c
+++ b/MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c
@@ -1367,7 +1367,13 @@ SerialReadRegister (
   EFI_STATUS  Status;



   if (SerialDev->PciDeviceInfo == NULL) {

-    return IoRead8 ((UINTN)SerialDev->BaseAddress + Offset * SerialDev->RegisterStride);

+    if (SerialDev->MmioAccess)

+    {

+      return MmioRead8((UINTN)SerialDev->BaseAddress + Offset * SerialDev->RegisterStride);

+    }

+    else {

+      return IoRead8 ((UINTN)SerialDev->BaseAddress + Offset * SerialDev->RegisterStride);

+    }

   } else {

     if (SerialDev->MmioAccess) {

       Status = SerialDev->PciDeviceInfo->PciIo->Mem.Read (

@@ -1411,7 +1417,13 @@ SerialWriteRegister (
   EFI_STATUS  Status;



   if (SerialDev->PciDeviceInfo == NULL) {

-    IoWrite8 ((UINTN)SerialDev->BaseAddress + Offset * SerialDev->RegisterStride, Data);

+    if (SerialDev->MmioAccess)

+    {

+      MmioWrite8((UINTN)SerialDev->BaseAddress + Offset * SerialDev->RegisterStride, Data);

+    }

+    else {

+      IoWrite8 ((UINTN)SerialDev->BaseAddress + Offset * SerialDev->RegisterStride, Data);

+    }

   } else {

     if (SerialDev->MmioAccess) {

       Status = SerialDev->PciDeviceInfo->PciIo->Mem.Write (

diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index c0f1df3bfb..b209fe2696 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -240,6 +240,7 @@


   MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf

   MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf

+  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxeEarly.inf

   MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf

   MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupportDxe.inf

   MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf

--
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118657): https://edk2.groups.io/g/devel/message/118657
Mute This Topic: https://groups.io/mt/105959587/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 44956 bytes --]

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [edk2-devel] [PATCH 1/1] MdeModulePkg: Load Serial driver in early DXE
  2024-05-08  3:27   ` Ni, Ray
@ 2024-05-08 13:24     ` Borzeszkowski, Alan
  2024-05-10  3:18       ` Ni, Ray
  0 siblings, 1 reply; 6+ messages in thread
From: Borzeszkowski, Alan @ 2024-05-08 13:24 UTC (permalink / raw)
  To: Ni, Ray, devel

[-- Attachment #1: Type: text/plain, Size: 599 bytes --]

We have considered that; however, we aim to avoid maintaining our own implementation of functions that communicate with UART.

Please see discussion over previous approach:

https://edk2.groups.io/g/devel/topic/104469297#115731


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118672): https://edk2.groups.io/g/devel/message/118672
Mute This Topic: https://groups.io/mt/105959587/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 1622 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [edk2-devel] [PATCH 1/1] MdeModulePkg: Load Serial driver in early DXE
  2024-05-08 13:24     ` Borzeszkowski, Alan
@ 2024-05-10  3:18       ` Ni, Ray
  2024-05-10 15:03         ` Borzeszkowski, Alan
  0 siblings, 1 reply; 6+ messages in thread
From: Ni, Ray @ 2024-05-10  3:18 UTC (permalink / raw)
  To: devel@edk2.groups.io, Borzeszkowski, Alan

[-- Attachment #1: Type: text/plain, Size: 1352 bytes --]

Mike did not recommend the approach used by the patch.

Your patch exposes a new pattern that's anti-driver-model, IMO.

If you want to avoid code duplication, solve that problem in a way that does not introduce such a pattern.
Please be aware that any one piece of code introduced in edk2, could be cloned to multiple similar pieces of code. So we need to be very careful.


Thanks,
Ray
________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Borzeszkowski, Alan <alan.borzeszkowski@intel.com>
Sent: Wednesday, May 8, 2024 21:24
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Subject: Re: [edk2-devel] [PATCH 1/1] MdeModulePkg: Load Serial driver in early DXE


We have considered that; however, we aim to avoid maintaining our own implementation of functions that communicate with UART.

Please see discussion over previous approach:

https://edk2.groups.io/g/devel/topic/104469297#115731




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118788): https://edk2.groups.io/g/devel/message/118788
Mute This Topic: https://groups.io/mt/105959587/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 4703 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [edk2-devel] [PATCH 1/1] MdeModulePkg: Load Serial driver in early DXE
  2024-05-10  3:18       ` Ni, Ray
@ 2024-05-10 15:03         ` Borzeszkowski, Alan
  0 siblings, 0 replies; 6+ messages in thread
From: Borzeszkowski, Alan @ 2024-05-10 15:03 UTC (permalink / raw)
  To: Ni, Ray, devel

[-- Attachment #1: Type: text/plain, Size: 975 bytes --]

Sorry, I did not provide full picture. After this discussion on devel I've linked, I spoke to Michael what changes would be in compliance with EDK2 driver model guidelines. An agreement was reached: Root Bridge driver that implements SIO Protocol and Serial I/O driver (with new entrypoint) that depends on SIO by DEPEX.

This way, driver would be a DXE Driver that runs new entrypoint only when SIO Protocol is present. I believe this approach doesn't violate UEFI Driver model rules since it uses standard DXE methods.

Also, the approach of modifying driver's entrypoint was suggested by Zhichao Gao.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118822): https://edk2.groups.io/g/devel/message/118822
Mute This Topic: https://groups.io/mt/105959587/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 1987 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-05-10 15:03 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-07 13:09 [edk2-devel] [PATCH 0/1] MdeModulePkg: Load Serial driver in early DXE Borzeszkowski, Alan
2024-05-07 13:09 ` [edk2-devel] [PATCH 1/1] " Borzeszkowski, Alan
2024-05-08  3:27   ` Ni, Ray
2024-05-08 13:24     ` Borzeszkowski, Alan
2024-05-10  3:18       ` Ni, Ray
2024-05-10 15:03         ` Borzeszkowski, Alan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox