public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] Update Graphics Info Hob FrameBufferSize Based on UEFI Spec 2.0
@ 2022-01-02 16:33 Ashraf Ali S
  2022-01-02 20:25 ` Michael D Kinney
  0 siblings, 1 reply; 3+ messages in thread
From: Ashraf Ali S @ 2022-01-02 16:33 UTC (permalink / raw)
  To: devel
  Cc: Ashraf Ali S, Rangasai V Chaganty, Digant H Solanki, Sangeetha V,
	Ray Ni, Michael D Kinney, Liming Gao, Zhiguang Liu

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3793
Basede on UEFI Spec 2.0 section 17.7.1 structure
EFI_GRAPHICS_OUTPUT_PROTOCOL_MODE say FrameBufferSize should be size of
UINTN, in MdePkg\Include\Guid\GraphicsInfoHob.h
EFI_PEI_GRAPHICS_INFO_HOB FrameBufferSize  is UINT32,

UefiPayloadPkg\GraphicsOutputDxe\GraphicsOutput.c
MdeModulePkg\Universal\Console\GraphicsOutputDxe\GraphicsOutput.c

Private->GraphicsOutputMode.FrameBufferSize = \
GraphicsInfo->FrameBufferSize;

UINT32 value is getting assigned to UINTN,
in X64 build compiler will throw possible loss of data error.

so update the EFI_PEI_GRAPHICS_INFO_HOB  based on UEFI Spec 2.0

Signed-off-by: Ashraf Ali S <ashraf.ali.s@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Digant H Solanki <digant.h.solanki@intel.com>
Cc: Sangeetha V <sangeetha.v@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
---
 MdePkg/Include/Guid/GraphicsInfoHob.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdePkg/Include/Guid/GraphicsInfoHob.h b/MdePkg/Include/Guid/GraphicsInfoHob.h
index 237911e63a..92bd907f20 100644
--- a/MdePkg/Include/Guid/GraphicsInfoHob.h
+++ b/MdePkg/Include/Guid/GraphicsInfoHob.h
@@ -26,7 +26,7 @@
 
 typedef struct {
   EFI_PHYSICAL_ADDRESS                    FrameBufferBase;
-  UINT32                                  FrameBufferSize;
+  UINTN                                   FrameBufferSize;
   EFI_GRAPHICS_OUTPUT_MODE_INFORMATION    GraphicsMode;
 } EFI_PEI_GRAPHICS_INFO_HOB;
 
-- 
2.30.2.windows.1


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

* Re: [PATCH] Update Graphics Info Hob FrameBufferSize Based on UEFI Spec 2.0
  2022-01-02 16:33 [PATCH] Update Graphics Info Hob FrameBufferSize Based on UEFI Spec 2.0 Ashraf Ali S
@ 2022-01-02 20:25 ` Michael D Kinney
  2022-01-03  7:33   ` Ashraf Ali S
  0 siblings, 1 reply; 3+ messages in thread
From: Michael D Kinney @ 2022-01-02 20:25 UTC (permalink / raw)
  To: S, Ashraf Ali, devel@edk2.groups.io, Kinney, Michael D
  Cc: Chaganty, Rangasai V, Solanki, Digant H, V, Sangeetha, Ni, Ray,
	Gao, Liming, Liu, Zhiguang

Hi Ashraf,

HOBs must never use UINTN or pointer fields.  HOBs must always be the same size no matter
what CPU or CPU mode the structure is compiler for.

This is also a non-backwards compatible change to a HOB structure.

For the case where the GraphicsOutputProtocol is build for X64 and uses this HOB as input,
the UINT32 value from the HOB can be cast to UINTN for the protocol.

Given that the cast is from UINT32 -> UINT32 for IA32 builds and UINT32->UINT64 for X64
builds, I do not see any case where there would be possible loss of data.  The cast would
always be to a type that is the same size or larger.

Can you please verify the compiler error you observed?

Thanks,

Mike

> -----Original Message-----
> From: S, Ashraf Ali <ashraf.ali.s@intel.com>
> Sent: Sunday, January 2, 2022 8:34 AM
> To: devel@edk2.groups.io
> Cc: S, Ashraf Ali <ashraf.ali.s@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Solanki, Digant H
> <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>
> Subject: [PATCH] Update Graphics Info Hob FrameBufferSize Based on UEFI Spec 2.0
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3793
> Basede on UEFI Spec 2.0 section 17.7.1 structure
> EFI_GRAPHICS_OUTPUT_PROTOCOL_MODE say FrameBufferSize should be size of
> UINTN, in MdePkg\Include\Guid\GraphicsInfoHob.h
> EFI_PEI_GRAPHICS_INFO_HOB FrameBufferSize  is UINT32,
> 
> UefiPayloadPkg\GraphicsOutputDxe\GraphicsOutput.c
> MdeModulePkg\Universal\Console\GraphicsOutputDxe\GraphicsOutput.c
> 
> Private->GraphicsOutputMode.FrameBufferSize = \
> GraphicsInfo->FrameBufferSize;
> 
> UINT32 value is getting assigned to UINTN,
> in X64 build compiler will throw possible loss of data error.
> 
> so update the EFI_PEI_GRAPHICS_INFO_HOB  based on UEFI Spec 2.0
> 
> Signed-off-by: Ashraf Ali S <ashraf.ali.s@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Digant H Solanki <digant.h.solanki@intel.com>
> Cc: Sangeetha V <sangeetha.v@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  MdePkg/Include/Guid/GraphicsInfoHob.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Include/Guid/GraphicsInfoHob.h b/MdePkg/Include/Guid/GraphicsInfoHob.h
> index 237911e63a..92bd907f20 100644
> --- a/MdePkg/Include/Guid/GraphicsInfoHob.h
> +++ b/MdePkg/Include/Guid/GraphicsInfoHob.h
> @@ -26,7 +26,7 @@
> 
>  typedef struct {
>    EFI_PHYSICAL_ADDRESS                    FrameBufferBase;
> -  UINT32                                  FrameBufferSize;
> +  UINTN                                   FrameBufferSize;
>    EFI_GRAPHICS_OUTPUT_MODE_INFORMATION    GraphicsMode;
>  } EFI_PEI_GRAPHICS_INFO_HOB;
> 
> --
> 2.30.2.windows.1


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

* Re: [PATCH] Update Graphics Info Hob FrameBufferSize Based on UEFI Spec 2.0
  2022-01-02 20:25 ` Michael D Kinney
@ 2022-01-03  7:33   ` Ashraf Ali S
  0 siblings, 0 replies; 3+ messages in thread
From: Ashraf Ali S @ 2022-01-03  7:33 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io
  Cc: Chaganty, Rangasai V, Solanki, Digant H, V, Sangeetha, Ni, Ray,
	Gao, Liming, Liu, Zhiguang

Hi., Michael

The issue was coming due to assign UINTN to UINT32  (warning C4244: '=': conversion from 'UINTN' to 'UINT32', possible loss of data)

I will fix this in my code instead of changing in the EDK2.

Closing this bug, Thanks

Happy New Year 😊

Regards,
Ashraf Ali S
Intel Technology India Pvt. Ltd. 

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com> 
Sent: Monday, January 3, 2022 1:55 AM
To: S, Ashraf Ali <ashraf.ali.s@intel.com>; devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Solanki, Digant H <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; Ni, Ray <ray.ni@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>
Subject: RE: [PATCH] Update Graphics Info Hob FrameBufferSize Based on UEFI Spec 2.0

Hi Ashraf,

HOBs must never use UINTN or pointer fields.  HOBs must always be the same size no matter what CPU or CPU mode the structure is compiler for.

This is also a non-backwards compatible change to a HOB structure.

For the case where the GraphicsOutputProtocol is build for X64 and uses this HOB as input, the UINT32 value from the HOB can be cast to UINTN for the protocol.

Given that the cast is from UINT32 -> UINT32 for IA32 builds and UINT32->UINT64 for X64 builds, I do not see any case where there would be possible loss of data.  The cast would always be to a type that is the same size or larger.

Can you please verify the compiler error you observed?

Thanks,

Mike

> -----Original Message-----
> From: S, Ashraf Ali <ashraf.ali.s@intel.com>
> Sent: Sunday, January 2, 2022 8:34 AM
> To: devel@edk2.groups.io
> Cc: S, Ashraf Ali <ashraf.ali.s@intel.com>; Chaganty, Rangasai V 
> <rangasai.v.chaganty@intel.com>; Solanki, Digant H 
> <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; 
> Ni, Ray <ray.ni@intel.com>; Kinney, Michael D 
> <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; 
> Liu, Zhiguang <zhiguang.liu@intel.com>
> Subject: [PATCH] Update Graphics Info Hob FrameBufferSize Based on 
> UEFI Spec 2.0
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3793
> Basede on UEFI Spec 2.0 section 17.7.1 structure 
> EFI_GRAPHICS_OUTPUT_PROTOCOL_MODE say FrameBufferSize should be size 
> of UINTN, in MdePkg\Include\Guid\GraphicsInfoHob.h
> EFI_PEI_GRAPHICS_INFO_HOB FrameBufferSize  is UINT32,
> 
> UefiPayloadPkg\GraphicsOutputDxe\GraphicsOutput.c
> MdeModulePkg\Universal\Console\GraphicsOutputDxe\GraphicsOutput.c
> 
> Private->GraphicsOutputMode.FrameBufferSize = \
> GraphicsInfo->FrameBufferSize;
> 
> UINT32 value is getting assigned to UINTN, in X64 build compiler will 
> throw possible loss of data error.
> 
> so update the EFI_PEI_GRAPHICS_INFO_HOB  based on UEFI Spec 2.0
> 
> Signed-off-by: Ashraf Ali S <ashraf.ali.s@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Digant H Solanki <digant.h.solanki@intel.com>
> Cc: Sangeetha V <sangeetha.v@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  MdePkg/Include/Guid/GraphicsInfoHob.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Include/Guid/GraphicsInfoHob.h 
> b/MdePkg/Include/Guid/GraphicsInfoHob.h
> index 237911e63a..92bd907f20 100644
> --- a/MdePkg/Include/Guid/GraphicsInfoHob.h
> +++ b/MdePkg/Include/Guid/GraphicsInfoHob.h
> @@ -26,7 +26,7 @@
> 
>  typedef struct {
>    EFI_PHYSICAL_ADDRESS                    FrameBufferBase;
> -  UINT32                                  FrameBufferSize;
> +  UINTN                                   FrameBufferSize;
>    EFI_GRAPHICS_OUTPUT_MODE_INFORMATION    GraphicsMode;
>  } EFI_PEI_GRAPHICS_INFO_HOB;
> 
> --
> 2.30.2.windows.1


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

end of thread, other threads:[~2022-01-03  7:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-02 16:33 [PATCH] Update Graphics Info Hob FrameBufferSize Based on UEFI Spec 2.0 Ashraf Ali S
2022-01-02 20:25 ` Michael D Kinney
2022-01-03  7:33   ` Ashraf Ali S

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