Re: [edk2-devel] [PATCH] MdePkg: Fix AsmReadSs() with GCC toolchain

["Laszlo Ersek" <lersek@redhat.com>]

CC'ing the MdePkg maintainers (see "Maintainers.txt" and
"$EDK_TOOLS_PATH/Scripts/GetMaintainer.py").

On 05/24/21 06:50, Satoshi Tanda wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3325

(1) The BZ URL is incorrect; the right URL is

https://bugzilla.tianocore.org/show_bug.cgi?id=3405

But I think whoever merges this patch can update the BZ URL upon merge.


(2) Similarly, I think the subject should be clarified, upon merge:

  MdePkg/BaseLib: Fix AsmReadSs() with GCC toolchain

> 
> AsmReadSs() in Ia32/GccInlinePriv.c and X64/GccInlinePriv.c return the
> DS segment selector value instead of SS.
> 
> Signed-off-by: Satoshi Tanda <tanda.sat@gmail.com>
> ---
>  MdePkg/Library/BaseLib/Ia32/GccInlinePriv.c | 2 +-
>  MdePkg/Library/BaseLib/X64/GccInlinePriv.c  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseLib/Ia32/GccInlinePriv.c b/MdePkg/Library/BaseLib/Ia32/GccInlinePriv.c
> index 40e8c08beb..b8b5b85e73 100644
> --- a/MdePkg/Library/BaseLib/Ia32/GccInlinePriv.c
> +++ b/MdePkg/Library/BaseLib/Ia32/GccInlinePriv.c
> @@ -902,7 +902,7 @@ AsmReadSs (
>    UINT16  Data;
>  
>    __asm__ __volatile__ (
> -    "mov  %%ds, %0"
> +    "mov  %%ss, %0"
>      :"=a" (Data)
>      );
>  
> diff --git a/MdePkg/Library/BaseLib/X64/GccInlinePriv.c b/MdePkg/Library/BaseLib/X64/GccInlinePriv.c
> index 244bd62ee6..c3feb9f922 100644
> --- a/MdePkg/Library/BaseLib/X64/GccInlinePriv.c
> +++ b/MdePkg/Library/BaseLib/X64/GccInlinePriv.c
> @@ -911,7 +911,7 @@ AsmReadSs (
>    UINT16  Data;
>  
>    __asm__ __volatile__ (
> -    "mov  %%ds, %0"
> +    "mov  %%ss, %0"
>      :"=a" (Data)
>      );
>  
> 

With (1) and (2) fixed:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>


Should this go into edk2-stable202105?

On one hand, this is clearly a bugfix. On the other hand, we're very
close to the tag, maybe something depends on the buggy behavior, and the
function has been broken since (at least) commit cf683fedc773
(2010-05-18). Personally I wouldn't rush it.

Thanks
Laszlo