1. Patchew
  2. EDK2
  3. View series

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

Satoshi Tanda posted 1 patch 2 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/edk2 tags/patchew/d014090344f4822b57e806cc8d6a27bd5233429d.1621829775.git.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(-)
[edk2-devel] [PATCH] MdePkg: Fix AsmReadSs() with GCC toolchain
Posted by Satoshi Tanda 2 years, 11 months ago 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3325

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)
     );
 
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#75541): https://edk2.groups.io/g/devel/message/75541
Mute This Topic: https://groups.io/mt/83062017/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdePkg: Fix AsmReadSs() with GCC toolchain
Posted by Laszlo Ersek 2 years, 11 months ago 
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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#75565): https://edk2.groups.io/g/devel/message/75565
Mute This Topic: https://groups.io/mt/83062017/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdePkg: Fix AsmReadSs() with GCC toolchain
Posted by Satoshi Tanda 2 years, 11 months ago 
On Tue, May 25, 2021 at 05:16 AM, Laszlo Ersek wrote:

> 
> (1) The BZ URL is incorrect;

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

Let me know if anyone needs the updates of the patch on those (vs updating them inline at PR). I will do it if required.

> 
> Should this go into edk2-stable202105?

No, to me. I do not find a reason to rush it neither.

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

This document should probably be updated to mention them later. Those are not explained.
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process

Thanks for the reviewing this and guiding me on the process.

Satoshi


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#75583): https://edk2.groups.io/g/devel/message/75583
Mute This Topic: https://groups.io/mt/83062017/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdePkg: Fix AsmReadSs() with GCC toolchain
Posted by Laszlo Ersek 2 years, 11 months ago 
On 05/25/21 16:28, Satoshi Tanda wrote:
> On Tue, May 25, 2021 at 05:16 AM, Laszlo Ersek wrote:
> 
>>
>> (1) The BZ URL is incorrect;
> 
>>
>> (2) Similarly, I think the subject should be clarified, upon merge
> 
> Let me know if anyone needs the updates of the patch on those (vs updating them inline at PR). I will do it if required.

I'll leave that decision to the MdePkg owners.

> 
>>
>> Should this go into edk2-stable202105?
> 
> No, to me. I do not find a reason to rush it neither.
> 
>>
>> CC'ing the MdePkg maintainers (see "Maintainers.txt" and "$EDK_TOOLS_PATH/Scripts/GetMaintainer.py").
>>
> 
> This document should probably be updated to mention them later. Those are not explained.
> https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process

It is spelled out in the "unkempt git guide", in this section:

https://github.com/tianocore/tianocore.github.io/wiki/Laszlo%27s-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-18

but indeed it's not part of the official (normative) article.

The problem with writing more documentation is that then you have to
keep more documentation up-to-date :/

Thanks,
Laszlo

> 
> Thanks for the reviewing this and guiding me on the process.
> 
> Satoshi
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#75586): https://edk2.groups.io/g/devel/message/75586
Mute This Topic: https://groups.io/mt/83062017/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdePkg: Fix AsmReadSs() with GCC toolchain
Posted by Satoshi Tanda 2 years, 11 months ago 
Could we get this patch reviewed and approved by required folks, or advise me how can I move forward?

Thanks!
Satoshi


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#76025): https://edk2.groups.io/g/devel/message/76025
Mute This Topic: https://groups.io/mt/83062017/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
回复: [edk2-devel] [PATCH] MdePkg: Fix AsmReadSs() with GCC toolchain
Posted by gaoliming 2 years, 11 months ago 
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

 

I have updated BZ link and title in the commit message. 

 

Create PR https://github.com/tianocore/edk2/pull/1687

 

Thanks

Liming

发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Satoshi Tanda
发送时间: 2021年6月4日 8:24
收件人: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io
主题: Re: [edk2-devel] [PATCH] MdePkg: Fix AsmReadSs() with GCC toolchain

 

Could we get this patch reviewed and approved by required folks, or advise me how can I move forward? 

Thanks!
Satoshi





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#76029): https://edk2.groups.io/g/devel/message/76029
Mute This Topic: https://groups.io/mt/83300488/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] 回复: [edk2-devel] [PATCH] MdePkg: Fix AsmReadSs() with GCC toolchain
Posted by Satoshi Tanda 2 years, 11 months ago 
Thank you, Liming!


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#76030): https://edk2.groups.io/g/devel/message/76030
Mute This Topic: https://groups.io/mt/83300488/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.