[PATCH v1 0/4] DynamicTablesPkg: Add access size parameter for serial ports

[PATCH v1 0/4] DynamicTablesPkg: Add access size parameter for serial ports

[Joey Gouly]

This series adds the Access Size parameter to the Serial Port in
DynamicTablesPkg.

This has been tested (by Sami) with kvmtool, which virtualises a UART
with 8 bit access size.

Changes available at https://github.com/jgouly/edk2/tree/1586_serial_port_access_size_v1

Joey Gouly (4):
  ShellPkg: Rename Address Size to Access size
  DynamicTablesPkg: Add access size to CM_ARM_SERIAL_PORT_INFO
  DynamicTablesPkg: Set the Access size for the SPCR table
  DynamicTablesPkg: Set the Access size for the DBG2 table

 .../Include/ArmNameSpaceObjects.h             |  3 +++
 .../Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c   | 24 ++++++++++++++++++-
 .../Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c   | 22 ++++++++++++++++-
 .../UefiShellAcpiViewCommandLib/AcpiParser.c  |  4 ++--
 4 files changed, 49 insertions(+), 4 deletions(-)

-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

[PATCH v1 1/4] ShellPkg: Rename Address Size to Access size

[Joey Gouly]

This matches the ACPI spec 6.3, table 5.1: Generic Address Structure.

Signed-off-by: Joey Gouly <joey.gouly@arm.com>
---
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
index 01ac9a9bafeb2ca12c1ba19f406d626b108f5fe2..74056e72c359c732758563a9fe14ff2582f39f21 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
@@ -1,7 +1,7 @@
 /** @file
   ACPI parser
 
-  Copyright (c) 2016 - 2020, Arm Limited. All rights reserved.
+  Copyright (c) 2016 - 2021, Arm Limited. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
 
@@ -628,7 +628,7 @@ STATIC CONST ACPI_PARSER GasParser[] = {
   {L"Address Space ID", 1, 0, L"0x%x", NULL, NULL, NULL, NULL},
   {L"Register Bit Width", 1, 1, L"0x%x", NULL, NULL, NULL, NULL},
   {L"Register Bit Offset", 1, 2, L"0x%x", NULL, NULL, NULL, NULL},
-  {L"Address Size", 1, 3, L"0x%x", NULL, NULL, NULL, NULL},
+  {L"Access Size", 1, 3, L"0x%x", NULL, NULL, NULL, NULL},
   {L"Address", 8, 4, L"0x%lx", NULL, NULL, NULL, NULL}
 };
 
-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

Re: [PATCH v1 1/4] ShellPkg: Rename Address Size to Access size

[Sami Mujawar]

Adding Zhichao.

Regards,

Sami Mujawar

On 15/04/2021, 13:19, "Joey Gouly" <joey.gouly@arm.com> wrote:

    This matches the ACPI spec 6.3, table 5.1: Generic Address Structure.

    Signed-off-by: Joey Gouly <joey.gouly@arm.com>
    ---
     ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c | 4 ++--
     1 file changed, 2 insertions(+), 2 deletions(-)

    diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
    index 01ac9a9bafeb2ca12c1ba19f406d626b108f5fe2..74056e72c359c732758563a9fe14ff2582f39f21 100644
    --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
    +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
    @@ -1,7 +1,7 @@
     /** @file
       ACPI parser

    -  Copyright (c) 2016 - 2020, Arm Limited. All rights reserved.
    +  Copyright (c) 2016 - 2021, Arm Limited. All rights reserved.
       SPDX-License-Identifier: BSD-2-Clause-Patent
     **/

    @@ -628,7 +628,7 @@ STATIC CONST ACPI_PARSER GasParser[] = {
       {L"Address Space ID", 1, 0, L"0x%x", NULL, NULL, NULL, NULL},
       {L"Register Bit Width", 1, 1, L"0x%x", NULL, NULL, NULL, NULL},
       {L"Register Bit Offset", 1, 2, L"0x%x", NULL, NULL, NULL, NULL},
    -  {L"Address Size", 1, 3, L"0x%x", NULL, NULL, NULL, NULL},
    +  {L"Access Size", 1, 3, L"0x%x", NULL, NULL, NULL, NULL},
       {L"Address", 8, 4, L"0x%lx", NULL, NULL, NULL, NULL}
     };

    -- 
    Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

Re: [edk2-devel] [PATCH v1 1/4] ShellPkg: Rename Address Size to Access size

[Sami Mujawar]

Hi Joey,

Thank you for this patch.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 15/04/2021, 13:29, "devel@edk2.groups.io on behalf of Sami Mujawar via groups.io" <devel@edk2.groups.io on behalf of sami.mujawar=arm.com@groups.io> wrote:

    Adding Zhichao.

    Regards,

    Sami Mujawar

    On 15/04/2021, 13:19, "Joey Gouly" <joey.gouly@arm.com> wrote:

        This matches the ACPI spec 6.3, table 5.1: Generic Address Structure.

        Signed-off-by: Joey Gouly <joey.gouly@arm.com>
        ---
         ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c | 4 ++--
         1 file changed, 2 insertions(+), 2 deletions(-)

        diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
        index 01ac9a9bafeb2ca12c1ba19f406d626b108f5fe2..74056e72c359c732758563a9fe14ff2582f39f21 100644
        --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
        +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
        @@ -1,7 +1,7 @@
         /** @file
           ACPI parser

        -  Copyright (c) 2016 - 2020, Arm Limited. All rights reserved.
        +  Copyright (c) 2016 - 2021, Arm Limited. All rights reserved.
           SPDX-License-Identifier: BSD-2-Clause-Patent
         **/

        @@ -628,7 +628,7 @@ STATIC CONST ACPI_PARSER GasParser[] = {
           {L"Address Space ID", 1, 0, L"0x%x", NULL, NULL, NULL, NULL},
           {L"Register Bit Width", 1, 1, L"0x%x", NULL, NULL, NULL, NULL},
           {L"Register Bit Offset", 1, 2, L"0x%x", NULL, NULL, NULL, NULL},
        -  {L"Address Size", 1, 3, L"0x%x", NULL, NULL, NULL, NULL},
        +  {L"Access Size", 1, 3, L"0x%x", NULL, NULL, NULL, NULL},
           {L"Address", 8, 4, L"0x%lx", NULL, NULL, NULL, NULL}
         };

        -- 
        Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")




    

Re: [edk2-devel] [PATCH v1 1/4] ShellPkg: Rename Address Size to Access size

[Gao, Zhichao]

Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

Thanks,
Zhichao

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sami
> Mujawar
> Sent: Friday, April 16, 2021 7:44 PM
> To: devel@edk2.groups.io; Sami Mujawar <Sami.Mujawar@arm.com>; Joey
> Gouly <Joey.Gouly@arm.com>
> Cc: Ni, Ray <ray.ni@intel.com>; nd <nd@arm.com>; Gao, Zhichao
> <zhichao.gao@intel.com>
> Subject: Re: [edk2-devel] [PATCH v1 1/4] ShellPkg: Rename Address Size to
> Access size
> 
> Hi Joey,
> 
> Thank you for this patch.
> 
> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
> 
> Regards,
> 
> Sami Mujawar
> 
> On 15/04/2021, 13:29, "devel@edk2.groups.io on behalf of Sami Mujawar via
> groups.io" <devel@edk2.groups.io on behalf of
> sami.mujawar=arm.com@groups.io> wrote:
> 
>     Adding Zhichao.
> 
>     Regards,
> 
>     Sami Mujawar
> 
>     On 15/04/2021, 13:19, "Joey Gouly" <joey.gouly@arm.com> wrote:
> 
>         This matches the ACPI spec 6.3, table 5.1: Generic Address Structure.
> 
>         Signed-off-by: Joey Gouly <joey.gouly@arm.com>
>         ---
>          ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c | 4 ++--
>          1 file changed, 2 insertions(+), 2 deletions(-)
> 
>         diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
>         index
> 01ac9a9bafeb2ca12c1ba19f406d626b108f5fe2..74056e72c359c732758563a9fe
> 14ff2582f39f21 100644
>         --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
>         +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
>         @@ -1,7 +1,7 @@
>          /** @file
>            ACPI parser
> 
>         -  Copyright (c) 2016 - 2020, Arm Limited. All rights reserved.
>         +  Copyright (c) 2016 - 2021, Arm Limited. All rights reserved.
>            SPDX-License-Identifier: BSD-2-Clause-Patent
>          **/
> 
>         @@ -628,7 +628,7 @@ STATIC CONST ACPI_PARSER GasParser[] = {
>            {L"Address Space ID", 1, 0, L"0x%x", NULL, NULL, NULL, NULL},
>            {L"Register Bit Width", 1, 1, L"0x%x", NULL, NULL, NULL, NULL},
>            {L"Register Bit Offset", 1, 2, L"0x%x", NULL, NULL, NULL, NULL},
>         -  {L"Address Size", 1, 3, L"0x%x", NULL, NULL, NULL, NULL},
>         +  {L"Access Size", 1, 3, L"0x%x", NULL, NULL, NULL, NULL},
>            {L"Address", 8, 4, L"0x%lx", NULL, NULL, NULL, NULL}
>          };
> 
>         --
>         Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 

[PATCH v1 2/4] DynamicTablesPkg: Add access size to CM_ARM_SERIAL_PORT_INFO

[Joey Gouly]

Add access size to CM_ARM_SERIAL_PORT_INFO so that this can be
passed down to the Generic Address Structure.

Signed-off-by: Joey Gouly <joey.gouly@arm.com>
---
 DynamicTablesPkg/Include/ArmNameSpaceObjects.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
index afcfe3704cb908afb1e6b17e6771b2ebe3f67f76..19dcae13b2191e5f0b03ea85edec1191d2a406bf 100644
--- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
@@ -293,6 +293,9 @@ typedef struct CmArmSerialPortInfo {
 
   /// The Base address length
   UINT64  BaseAddressLength;
+
+  /// The access size
+  UINT8   AccessSize;
 } CM_ARM_SERIAL_PORT_INFO;
 
 /** A structure that describes the
-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

Re: [PATCH v1 2/4] DynamicTablesPkg: Add access size to CM_ARM_SERIAL_PORT_INFO

[Sami Mujawar]

Hi Joey,

This patch looks good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 15/04/2021, 13:19, "Joey Gouly" <joey.gouly@arm.com> wrote:

    Add access size to CM_ARM_SERIAL_PORT_INFO so that this can be
    passed down to the Generic Address Structure.

    Signed-off-by: Joey Gouly <joey.gouly@arm.com>
    ---
     DynamicTablesPkg/Include/ArmNameSpaceObjects.h | 3 +++
     1 file changed, 3 insertions(+)

    diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
    index afcfe3704cb908afb1e6b17e6771b2ebe3f67f76..19dcae13b2191e5f0b03ea85edec1191d2a406bf 100644
    --- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
    +++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
    @@ -293,6 +293,9 @@ typedef struct CmArmSerialPortInfo {

       /// The Base address length
       UINT64  BaseAddressLength;
    +
    +  /// The access size
    +  UINT8   AccessSize;
     } CM_ARM_SERIAL_PORT_INFO;

     /** A structure that describes the
    -- 
    Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

[PATCH v1 3/4] DynamicTablesPkg: Set the Access size for the SPCR table

[Joey Gouly]

The SPCR table generator set the access size for the UART to
DWORD (4 bytes) by default. However, according to Section B
Generic UART, Arm Base System Architecture 1.0, Platform
Design Document, a Generic UART can have BYTE, WORD or DWORD
access sizes. To address this an AccessSize field has been
introduced in CM_ARM_SERIAL_PORT_INFO object.

This patch updates the SPCR generator to setup the AccessSize
field in the Generic Address Structure (GAS) for the UART in
the SPCR table with information provided by the platform.

Signed-off-by: Joey Gouly <joey.gouly@arm.com>
---
 DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c | 22 +++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
index 24bb5c014607b0746c4a8bb8bd260510fbdff08b..fecfd6bbabd63698f5f45f6bfbd494f25cf0faeb 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
@@ -1,7 +1,7 @@
 /** @file
   SPCR Table Generator
 
-  Copyright (c) 2017 - 2020, Arm Limited. All rights reserved.<BR>
+  Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -313,6 +313,26 @@ BuildSpcrTableEx (
   // Update the base address
   AcpiSpcr.BaseAddress.Address = SerialPortInfo->BaseAddress;
 
+  // Set the access size
+  if (SerialPortInfo->AccessSize >= EFI_ACPI_6_3_QWORD) {
+    Status = EFI_INVALID_PARAMETER;
+    DEBUG ((
+      DEBUG_ERROR,
+      "ERROR: SPCR: Access size must be <= 3 (DWORD). Status = %r\n",
+      Status
+      ));
+    goto error_handler;
+  } else if (SerialPortInfo->AccessSize == EFI_ACPI_6_3_UNDEFINED) {
+    // 0 Undefined (legacy reasons)
+    // Default to DWORD access size as the access
+    // size field was introduced at a later date
+    // and some ConfigurationManager implementations
+    // may not be providing this field data
+    AcpiSpcr.BaseAddress.AccessSize = EFI_ACPI_6_3_DWORD;
+  } else {
+    AcpiSpcr.BaseAddress.AccessSize = SerialPortInfo->AccessSize;
+  }
+
   // Update the UART interrupt
   AcpiSpcr.GlobalSystemInterrupt = SerialPortInfo->Interrupt;
 
-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

Re: [PATCH v1 3/4] DynamicTablesPkg: Set the Access size for the SPCR table

[Sami Mujawar]

Hi Joey,

This patch looks good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 15/04/2021, 13:19, "Joey Gouly" <joey.gouly@arm.com> wrote:

    The SPCR table generator set the access size for the UART to
    DWORD (4 bytes) by default. However, according to Section B
    Generic UART, Arm Base System Architecture 1.0, Platform
    Design Document, a Generic UART can have BYTE, WORD or DWORD
    access sizes. To address this an AccessSize field has been
    introduced in CM_ARM_SERIAL_PORT_INFO object.

    This patch updates the SPCR generator to setup the AccessSize
    field in the Generic Address Structure (GAS) for the UART in
    the SPCR table with information provided by the platform.

    Signed-off-by: Joey Gouly <joey.gouly@arm.com>
    ---
     DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c | 22 +++++++++++++++++++-
     1 file changed, 21 insertions(+), 1 deletion(-)

    diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
    index 24bb5c014607b0746c4a8bb8bd260510fbdff08b..fecfd6bbabd63698f5f45f6bfbd494f25cf0faeb 100644
    --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
    +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
    @@ -1,7 +1,7 @@
     /** @file
       SPCR Table Generator

    -  Copyright (c) 2017 - 2020, Arm Limited. All rights reserved.<BR>
    +  Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.<BR>

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

    @@ -313,6 +313,26 @@ BuildSpcrTableEx (
       // Update the base address
       AcpiSpcr.BaseAddress.Address = SerialPortInfo->BaseAddress;

    +  // Set the access size
    +  if (SerialPortInfo->AccessSize >= EFI_ACPI_6_3_QWORD) {
    +    Status = EFI_INVALID_PARAMETER;
    +    DEBUG ((
    +      DEBUG_ERROR,
    +      "ERROR: SPCR: Access size must be <= 3 (DWORD). Status = %r\n",
    +      Status
    +      ));
    +    goto error_handler;
    +  } else if (SerialPortInfo->AccessSize == EFI_ACPI_6_3_UNDEFINED) {
    +    // 0 Undefined (legacy reasons)
    +    // Default to DWORD access size as the access
    +    // size field was introduced at a later date
    +    // and some ConfigurationManager implementations
    +    // may not be providing this field data
    +    AcpiSpcr.BaseAddress.AccessSize = EFI_ACPI_6_3_DWORD;
    +  } else {
    +    AcpiSpcr.BaseAddress.AccessSize = SerialPortInfo->AccessSize;
    +  }
    +
       // Update the UART interrupt
       AcpiSpcr.GlobalSystemInterrupt = SerialPortInfo->Interrupt;

    -- 
    Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

[PATCH v1 4/4] DynamicTablesPkg: Set the Access size for the DBG2 table

[Joey Gouly]

The DBG2 table generator set the access size for the UART to
DWORD (4 bytes) by default. However, according to Section B
Generic UART, Arm Base System Architecture 1.0, Platform
Design Document, a Generic UART can have BYTE, WORD or DWORD
access sizes. To address this an AccessSize field has been
introduced in CM_ARM_SERIAL_PORT_INFO object.

This patch updates the DBG2 generator to setup the AccessSize
field in the Generic Address Structure (GAS) for the UART in
the DBG2 table with information provided by the platform.

Signed-off-by: Joey Gouly <joey.gouly@arm.com>
---
 DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c | 24 +++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
index d902bbc8463921624f1a6333e8d6bd84c6cb38f2..a7508d4a8834fd2038946a62de39e9cd0894bf79 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
@@ -1,7 +1,7 @@
 /** @file
   DBG2 Table Generator
 
-  Copyright (c) 2017 - 2020, Arm Limited. All rights reserved.<BR>
+  Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -420,6 +420,28 @@ BuildDbg2TableEx (
   AcpiDbg2.Dbg2DeviceInfo[INDEX_DBG_PORT0].BaseAddressRegister.Address =
     SerialPortInfo->BaseAddress;
 
+  // Set the access size
+  if (SerialPortInfo->AccessSize >= EFI_ACPI_6_3_QWORD) {
+    Status = EFI_INVALID_PARAMETER;
+    DEBUG ((
+      DEBUG_ERROR,
+      "ERROR: DBG2: Access size must be <= 3 (DWORD). Status = %r\n",
+      Status
+      ));
+    goto error_handler;
+  } else if (SerialPortInfo->AccessSize == EFI_ACPI_6_3_UNDEFINED) {
+    // 0 Undefined (legacy reasons)
+    // Default to DWORD access size as the access
+    // size field was introduced at a later date
+    // and some ConfigurationManager implementations
+    // may not be providing this field data
+    AcpiDbg2.Dbg2DeviceInfo[INDEX_DBG_PORT0].BaseAddressRegister.AccessSize =
+      EFI_ACPI_6_3_DWORD;
+  } else {
+    AcpiDbg2.Dbg2DeviceInfo[INDEX_DBG_PORT0].BaseAddressRegister.AccessSize =
+      SerialPortInfo->AccessSize;
+  }
+
   // Update the serial port subtype
   AcpiDbg2.Dbg2DeviceInfo[INDEX_DBG_PORT0].Dbg2Device.PortSubtype =
     SerialPortInfo->PortSubtype;
-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

Re: [PATCH v1 4/4] DynamicTablesPkg: Set the Access size for the DBG2 table

[Sami Mujawar]

Hi Joey,

This patch looks good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 15/04/2021, 13:17, "Joey Gouly" <joey.gouly@arm.com> wrote:

    The DBG2 table generator set the access size for the UART to
    DWORD (4 bytes) by default. However, according to Section B
    Generic UART, Arm Base System Architecture 1.0, Platform
    Design Document, a Generic UART can have BYTE, WORD or DWORD
    access sizes. To address this an AccessSize field has been
    introduced in CM_ARM_SERIAL_PORT_INFO object.

    This patch updates the DBG2 generator to setup the AccessSize
    field in the Generic Address Structure (GAS) for the UART in
    the DBG2 table with information provided by the platform.

    Signed-off-by: Joey Gouly <joey.gouly@arm.com>
    ---
     DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c | 24 +++++++++++++++++++-
     1 file changed, 23 insertions(+), 1 deletion(-)

    diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
    index d902bbc8463921624f1a6333e8d6bd84c6cb38f2..a7508d4a8834fd2038946a62de39e9cd0894bf79 100644
    --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
    +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c
    @@ -1,7 +1,7 @@
     /** @file
       DBG2 Table Generator

    -  Copyright (c) 2017 - 2020, Arm Limited. All rights reserved.<BR>
    +  Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.<BR>

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

    @@ -420,6 +420,28 @@ BuildDbg2TableEx (
       AcpiDbg2.Dbg2DeviceInfo[INDEX_DBG_PORT0].BaseAddressRegister.Address =
         SerialPortInfo->BaseAddress;

    +  // Set the access size
    +  if (SerialPortInfo->AccessSize >= EFI_ACPI_6_3_QWORD) {
    +    Status = EFI_INVALID_PARAMETER;
    +    DEBUG ((
    +      DEBUG_ERROR,
    +      "ERROR: DBG2: Access size must be <= 3 (DWORD). Status = %r\n",
    +      Status
    +      ));
    +    goto error_handler;
    +  } else if (SerialPortInfo->AccessSize == EFI_ACPI_6_3_UNDEFINED) {
    +    // 0 Undefined (legacy reasons)
    +    // Default to DWORD access size as the access
    +    // size field was introduced at a later date
    +    // and some ConfigurationManager implementations
    +    // may not be providing this field data
    +    AcpiDbg2.Dbg2DeviceInfo[INDEX_DBG_PORT0].BaseAddressRegister.AccessSize =
    +      EFI_ACPI_6_3_DWORD;
    +  } else {
    +    AcpiDbg2.Dbg2DeviceInfo[INDEX_DBG_PORT0].BaseAddressRegister.AccessSize =
    +      SerialPortInfo->AccessSize;
    +  }
    +
       // Update the serial port subtype
       AcpiDbg2.Dbg2DeviceInfo[INDEX_DBG_PORT0].Dbg2Device.PortSubtype =
         SerialPortInfo->PortSubtype;
    -- 
    Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

Re: [PATCH v1 0/4] DynamicTablesPkg: Add access size parameter for serial ports

[Sami Mujawar]

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

Series pushed as 99e7e48cc711..8c75a0720800

Thanks.

Regards,

Sami Mujawar

From: Joey Gouly <joey.gouly@arm.com>
Date: Thursday, 15 April 2021 at 13:19
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Joey Gouly <Joey.Gouly@arm.com>, ray.ni@intel.com <ray.ni@intel.com>, Sami Mujawar <Sami.Mujawar@arm.com>, nd <nd@arm.com>
Subject: [PATCH v1 0/4] DynamicTablesPkg: Add access size parameter for serial ports
This series adds the Access Size parameter to the Serial Port in
DynamicTablesPkg.

This has been tested (by Sami) with kvmtool, which virtualises a UART
with 8 bit access size.

Changes available at https://github.com/jgouly/edk2/tree/1586_serial_port_access_size_v1

Joey Gouly (4):
  ShellPkg: Rename Address Size to Access size
  DynamicTablesPkg: Add access size to CM_ARM_SERIAL_PORT_INFO
  DynamicTablesPkg: Set the Access size for the SPCR table
  DynamicTablesPkg: Set the Access size for the DBG2 table

 .../Include/ArmNameSpaceObjects.h             |  3 +++
 .../Acpi/Arm/AcpiDbg2LibArm/Dbg2Generator.c   | 24 ++++++++++++++++++-
 .../Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c   | 22 ++++++++++++++++-
 .../UefiShellAcpiViewCommandLib/AcpiParser.c  |  4 ++--
 4 files changed, 49 insertions(+), 4 deletions(-)

--
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

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