[edk2-devel] [edk2-libc Patch 2 0/1] wrmsr function is not working as expected.

[edk2-devel] [edk2-libc Patch 2 0/1] wrmsr function is not working as expected.

[Jayaprakash, N]

The wrmsr function always writes 0s to the higher 32 bits of the msr register.
This PR fixes the this issue reported through the BZ4745.

Jayaprakash N (1):
  ek2-libc: wrmsr function available in edk2module is not working as
    expected

 .../Python/Python-3.6.8/PyMod-3.6.8/Modules/edk2module.c       | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

-- 
2.44.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117987): https://edk2.groups.io/g/devel/message/117987
Mute This Topic: https://groups.io/mt/105597212/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [edk2-libc Patch 2 1/1] ek2-libc: wrmsr function available in edk2module is not working as expected

[Jayaprakash, N]

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

This commit fixes the issue reported in the BZ4745.
The wrmsr was always writing 0 to the higher 32 bits of the msr register.
This was due to a logical flaw in the code, where the input variable of
type unsigned int was left shitted by 32 bits without explicitly
converting it to a 64 bit value.

The issue is with the below statement.
data = vedx << 32 | veax;
Where the vedx which is an unsigned int, after left shifting by 32 bits
its value will be set to 0. Because of this the higher 32 bits of the MSR
are always set to 0. This has been fixed by this commit.

Cc: Rebecca Cran <rebecca@bsdio.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Jayaprakash N <n.jayaprakash@intel.com>
Signed-off-by: Jayaprakash N <n.jayaprakash@intel.com>
---
 .../Python/Python-3.6.8/PyMod-3.6.8/Modules/edk2module.c       | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/AppPkg/Applications/Python/Python-3.6.8/PyMod-3.6.8/Modules/edk2module.c b/AppPkg/Applications/Python/Python-3.6.8/PyMod-3.6.8/Modules/edk2module.c
index 8786df8..06bcf82 100644
--- a/AppPkg/Applications/Python/Python-3.6.8/PyMod-3.6.8/Modules/edk2module.c
+++ b/AppPkg/Applications/Python/Python-3.6.8/PyMod-3.6.8/Modules/edk2module.c
@@ -3886,7 +3886,8 @@ edk2_wrmsr(PyObject *self, PyObject *args)
   UINT64       data = 0;
   if (!PyArg_ParseTuple(args, "III", &vecx, &veax, &vedx))
     return NULL;
-  data = vedx << 32 | veax;
+  data = LShiftU64(vedx, 32);
+  data = BitFieldOr64(data, 0, 31, veax);
   Py_BEGIN_ALLOW_THREADS
   AsmWriteMsr64(vecx, data);
   Py_END_ALLOW_THREADS
-- 
2.44.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117988): https://edk2.groups.io/g/devel/message/117988
Mute This Topic: https://groups.io/mt/105597214/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-libc Patch 2 1/1] ek2-libc: wrmsr function available in edk2module is not working as expected

[Michael D Kinney]

The use of BitFieldOr64() is really only required when updating
a portion of a 32-bit or 64-bit value that are not aligned on
a 32-bit boundary and do not have a 32-bit aligned width.  Since
this use case is setting the lower 32-bits, simpler logic should
be used.

  data = LShiftU64(vedx, 32) | veax;

Mike

> -----Original Message-----
> From: Jayaprakash, N <n.jayaprakash@intel.com>
> Sent: Thursday, April 18, 2024 5:55 AM
> To: devel@edk2.groups.io
> Cc: Jayaprakash, N <n.jayaprakash@intel.com>; Rebecca Cran
> <rebecca@bsdio.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: [edk2-libc Patch 2 1/1] ek2-libc: wrmsr function available in
> edk2module is not working as expected
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4745
> 
> This commit fixes the issue reported in the BZ4745.
> The wrmsr was always writing 0 to the higher 32 bits of the msr
> register.
> This was due to a logical flaw in the code, where the input variable
> of
> type unsigned int was left shitted by 32 bits without explicitly
> converting it to a 64 bit value.
> 
> The issue is with the below statement.
> data = vedx << 32 | veax;
> Where the vedx which is an unsigned int, after left shifting by 32
> bits
> its value will be set to 0. Because of this the higher 32 bits of the
> MSR
> are always set to 0. This has been fixed by this commit.
> 
> Cc: Rebecca Cran <rebecca@bsdio.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Jayaprakash N <n.jayaprakash@intel.com>
> Signed-off-by: Jayaprakash N <n.jayaprakash@intel.com>
> ---
>  .../Python/Python-3.6.8/PyMod-3.6.8/Modules/edk2module.c       | 3
> ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/AppPkg/Applications/Python/Python-3.6.8/PyMod-
> 3.6.8/Modules/edk2module.c b/AppPkg/Applications/Python/Python-
> 3.6.8/PyMod-3.6.8/Modules/edk2module.c
> index 8786df8..06bcf82 100644
> --- a/AppPkg/Applications/Python/Python-3.6.8/PyMod-
> 3.6.8/Modules/edk2module.c
> +++ b/AppPkg/Applications/Python/Python-3.6.8/PyMod-
> 3.6.8/Modules/edk2module.c
> @@ -3886,7 +3886,8 @@ edk2_wrmsr(PyObject *self, PyObject *args)
>    UINT64       data = 0;
>    if (!PyArg_ParseTuple(args, "III", &vecx, &veax, &vedx))
>      return NULL;
> -  data = vedx << 32 | veax;
> +  data = LShiftU64(vedx, 32);
> +  data = BitFieldOr64(data, 0, 31, veax);
>    Py_BEGIN_ALLOW_THREADS
>    AsmWriteMsr64(vecx, data);
>    Py_END_ALLOW_THREADS
> --
> 2.44.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117999): https://edk2.groups.io/g/devel/message/117999
Mute This Topic: https://groups.io/mt/105597214/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-libc Patch 2 1/1] ek2-libc: wrmsr function available in edk2module is not working as expected

[Jayaprakash, N]

Thanks Mike as suggested made the logic simpler.
Sent the updated v3 patch for review.

Regards,
JP

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com> 
Sent: Friday, April 19, 2024 2:35 AM
To: Jayaprakash, N <n.jayaprakash@intel.com>; devel@edk2.groups.io
Cc: Rebecca Cran <rebecca@bsdio.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [edk2-libc Patch 2 1/1] ek2-libc: wrmsr function available in edk2module is not working as expected

The use of BitFieldOr64() is really only required when updating a portion of a 32-bit or 64-bit value that are not aligned on a 32-bit boundary and do not have a 32-bit aligned width.  Since this use case is setting the lower 32-bits, simpler logic should be used.

  data = LShiftU64(vedx, 32) | veax;

Mike

> -----Original Message-----
> From: Jayaprakash, N <n.jayaprakash@intel.com>
> Sent: Thursday, April 18, 2024 5:55 AM
> To: devel@edk2.groups.io
> Cc: Jayaprakash, N <n.jayaprakash@intel.com>; Rebecca Cran 
> <rebecca@bsdio.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: [edk2-libc Patch 2 1/1] ek2-libc: wrmsr function available in 
> edk2module is not working as expected
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4745
> 
> This commit fixes the issue reported in the BZ4745.
> The wrmsr was always writing 0 to the higher 32 bits of the msr 
> register.
> This was due to a logical flaw in the code, where the input variable 
> of type unsigned int was left shitted by 32 bits without explicitly 
> converting it to a 64 bit value.
> 
> The issue is with the below statement.
> data = vedx << 32 | veax;
> Where the vedx which is an unsigned int, after left shifting by 32 
> bits its value will be set to 0. Because of this the higher 32 bits of 
> the MSR are always set to 0. This has been fixed by this commit.
> 
> Cc: Rebecca Cran <rebecca@bsdio.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Jayaprakash N <n.jayaprakash@intel.com>
> Signed-off-by: Jayaprakash N <n.jayaprakash@intel.com>
> ---
>  .../Python/Python-3.6.8/PyMod-3.6.8/Modules/edk2module.c       | 3
> ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/AppPkg/Applications/Python/Python-3.6.8/PyMod-
> 3.6.8/Modules/edk2module.c b/AppPkg/Applications/Python/Python-
> 3.6.8/PyMod-3.6.8/Modules/edk2module.c
> index 8786df8..06bcf82 100644
> --- a/AppPkg/Applications/Python/Python-3.6.8/PyMod-
> 3.6.8/Modules/edk2module.c
> +++ b/AppPkg/Applications/Python/Python-3.6.8/PyMod-
> 3.6.8/Modules/edk2module.c
> @@ -3886,7 +3886,8 @@ edk2_wrmsr(PyObject *self, PyObject *args)
>    UINT64       data = 0;
>    if (!PyArg_ParseTuple(args, "III", &vecx, &veax, &vedx))
>      return NULL;
> -  data = vedx << 32 | veax;
> +  data = LShiftU64(vedx, 32);
> +  data = BitFieldOr64(data, 0, 31, veax);
>    Py_BEGIN_ALLOW_THREADS
>    AsmWriteMsr64(vecx, data);
>    Py_END_ALLOW_THREADS
> --
> 2.44.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118013): https://edk2.groups.io/g/devel/message/118013
Mute This Topic: https://groups.io/mt/105597214/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-