Re: [edk2-devel] [PATCH v1 04/19] DynamicTablesPkg: Fix serial port subtype warning

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 8/23/19 12:55 PM, Sami Mujawar wrote:
> The VS2017 compiler reports 'warning C4244: '=': conversion
> from 'UINT16' to 'UINT8', possible loss of data' for the
> SPCR InterfaceType field assignment.
> 
> The SPCR InterfaceType field uses the same encoding as that
> of the DBG2 table Port Subtype field. However SPCR.InterfaceType
> is 8-bit while the Port Subtype field in DBG2 table is 16-bit.
> 
> Since the Configuration Manager represents the Serial port
> information using the struct CM_ARM_SERIAL_PORT_INFO, the
> PortSubtype member in this struct is 16-bit.
> 
> To fix the warning an explicit type case is added. A validation
> is also added to ensure that the Serial Port Subtype value
> provided by the Configuration Manager is within the 8-bit
> range (less than 256).
> 
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
>   DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c | 15 ++++++++++++++-
>   1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
> index 1404279f828d5c06bb7605cb2fe6d864ef7a080e..4b2580da7df9b4472ae9bcc3df2138a9e13004d0 100644
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/SpcrGenerator.c
> @@ -217,8 +217,21 @@ BuildSpcrTable (
>       goto error_handler;
>     }
>   
> +  // The SPCR InterfaceType uses the same encoding as that of the
> +  // DBG2 table Port Subtype field. However InterfaceType is 8-bit
> +  // while the Port Subtype field in the DBG2 table is 16-bit.
> +  if ((SerialPortInfo->PortSubtype & 0xFF00) != 0) {
> +    Status = EFI_INVALID_PARAMETER;
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "ERROR: SPCR: Invalid Port Sybtype (must be < 256). Status = %r\n",
> +      Status
> +      ));
> +    goto error_handler;
> +  }
> +
>     // Update the serial port subtype
> -  AcpiSpcr.InterfaceType = SerialPortInfo->PortSubtype;
> +  AcpiSpcr.InterfaceType = (UINT8)SerialPortInfo->PortSubtype;
>   
>     // Update the base address
>     AcpiSpcr.BaseAddress.Address = SerialPortInfo->BaseAddress;
> 

Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>