Hi Mike, I have sent an updated patch v2 for review which uses the MP Services protocol API StarupThisAP() to read / write MSRs specific to CPU cores. Please review and do the needful.
Regards, JP -----Original Message----- From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jayaprakash, N Sent: Thursday, April 18, 2024 11:49 AM To: Kinney, Michael D <michael.d.kin...@intel.com>; devel@edk2.groups.io Cc: Rebecca Cran <rebe...@bsdio.com> Subject: Re: [edk2-devel] [edk2-libc Patch 1/1] edk2-libc:add rdmsr_ex & wrmsr_ex functions to read/write cpu specific msrs Thanks Mike. I shall make necessary changes and submit the PR again for review. Regards, JP -----Original Message----- From: Kinney, Michael D <michael.d.kin...@intel.com> Sent: Thursday, April 18, 2024 10:46 AM To: Jayaprakash, N <n.jayaprak...@intel.com>; devel@edk2.groups.io Cc: Rebecca Cran <rebe...@bsdio.com>; Kinney, Michael D <michael.d.kin...@intel.com> Subject: RE: [edk2-devel] [edk2-libc Patch 1/1] edk2-libc:add rdmsr_ex & wrmsr_ex functions to read/write cpu specific msrs Please use MP Services Protocol APIs StartupAllAPs() or StarupThisAP() to read/write MSRs on other logical processors. There and many examples of this in the UefiCpuPkg to programming MSRs on all the logical processors. Mike > -----Original Message----- > From: Jayaprakash, N <n.jayaprak...@intel.com> > Sent: Wednesday, April 17, 2024 8:16 PM > To: Kinney, Michael D <michael.d.kin...@intel.com>; > devel@edk2.groups.io > Cc: Rebecca Cran <rebe...@bsdio.com> > Subject: RE: [edk2-devel] [edk2-libc Patch 1/1] edk2-libc:add rdmsr_ex > & wrmsr_ex functions to read/write cpu specific msrs > > In the validation and debug scenarios, engineers tend to read MSRs and > write to MSRs of different CPUs. > So we are providing a simple mechanism through these APIs to enable > them to do these operations. > > These APIs will be part of the edk2 module of the Python interpreter > and will be used during the validation and debug scenarios only. > > This is not for switching the BSP for OS boot. This is only used > during the validation and debug use cases. > > Regards, > JP > -----Original Message----- > From: Kinney, Michael D <michael.d.kin...@intel.com> > Sent: Thursday, April 18, 2024 12:08 AM > To: Jayaprakash, N <n.jayaprak...@intel.com>; devel@edk2.groups.io > Cc: Rebecca Cran <rebe...@bsdio.com>; Kinney, Michael D > <michael.d.kin...@intel.com> > Subject: RE: [edk2-devel] [edk2-libc Patch 1/1] edk2-libc:add rdmsr_ex > & wrmsr_ex functions to read/write cpu specific msrs > > Hi JP, > > Is there a reason switch BSP is being used. That is not a common > operation and is typically used if the current BSP is not stable or > there is a good reason to switch the BSP for OS boot. > > The MP Services can be used to execute a C function on APs to execute > MSR related instructions. > > Mike > > > -----Original Message----- > > From: Jayaprakash, N <n.jayaprak...@intel.com> > > Sent: Sunday, April 14, 2024 10:33 PM > > To: devel@edk2.groups.io; Jayaprakash, N <n.jayaprak...@intel.com> > > Cc: Rebecca Cran <rebe...@bsdio.com>; Kinney, Michael D > > <michael.d.kin...@intel.com> > > Subject: RE: [edk2-devel] [edk2-libc Patch 1/1] edk2-libc:add > rdmsr_ex > > & wrmsr_ex functions to read/write cpu specific msrs > > > > + Rebecca and Mike, > > > > Would you be able to review this PR? > > > > Regards, > > JP > > > > -----Original Message----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of > > Jayaprakash, N > > Sent: Wednesday, April 10, 2024 11:49 AM > > To: devel@edk2.groups.io > > Cc: Jayaprakash, N <n.jayaprak...@intel.com>; Rebecca Cran > > <rebe...@bsdio.com>; Kinney, Michael D <michael.d.kin...@intel.com> > > Subject: [edk2-devel] [edk2-libc Patch 1/1] edk2-libc:add rdmsr_ex & > > wrmsr_ex functions to read/write cpu specific msrs > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4746 > > > > The rdmsr_ex and wrmsr_ex are extension APIs to the rdmsr and wrmsr > > APIs supported in edk2 module. These extension APIs makes it > possible > > to read / write MSRs from specific processors and fills an existing > > gap in this area. > > > > Cc: Rebecca Cran <rebe...@bsdio.com> > > Cc: Michael D Kinney <michael.d.kin...@intel.com> > > Cc: Jayaprakash N <n.jayaprak...@intel.com> > > Signed-off-by: Jayaprakash N <n.jayaprak...@intel.com> > > --- > > .../PyMod-3.6.8/Modules/edk2module.c | 159 > > +++++++++++++++++- > > .../Python/Python-3.6.8/Python368.inf | 3 + > > 2 files changed, 158 insertions(+), 4 deletions(-) > > > > 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 d6af8da..f1b13a6 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/edk2mo > > +++ dule.c > > @@ -3,7 +3,7 @@ > > Derived from posixmodule.c in Python 2.7.2. > > > > Copyright (c) 2015, Daryl McDaniel. All rights reserved.<BR> > > - Copyright (c) 2011 - 2023, Intel Corporation. All rights > > reserved.<BR> > > + Copyright (c) 2011 - 2024, Intel Corporation. All rights > > + reserved.<BR> > > This program and the accompanying materials are licensed and > made > > available under > > the terms and conditions of the BSD License that accompanies > this > > distribution. > > The full text of the license may be found at @@ -22,16 +22,23 > @@ > > #include <wchar.h> #include <sys/syslimits.h> #include <Uefi.h> > > +#include <Pi/PiDxeCis.h> // Needed for the definition of > > EFI_AP_PROCEDURE > > #include <Library/UefiLib.h> > > #include <Library/PciLib.h> > > #include <Library/IoLib.h> > > #include <Library/UefiRuntimeServicesTableLib.h> > > +#include <Library/UefiBootServicesTableLib.h> > > +#include <Protocol/MpService.h> > > > > #ifdef __cplusplus > > extern "C" { > > #endif > > > > PyTypeObject EfiGuidType; > > +EFI_MP_SERVICES_PROTOCOL *gpMpService = NULL; > > +UINTN gBSPProcessorNumber = 0; > > +UINTN gNumberOfProcessors = 0; > > +UINTN gNumberOfEnabledProcessors = 0; > > > > extern void _swsmi( unsigned int smi_code_data, unsigned int > > rax_value, unsigned int rbx_value, unsigned int rcx_value, unsigned > > int rdx_value, unsigned int rsi_value, unsigned int rdi_value ); // > - > > - Support routines @@ -169,6 +176,35 @@ PyDoc_STRVAR(edk2__doc__, > > /* dummy version. _PyVerify_fd() is already defined in fileobject.h > > */ #define _PyVerify_fd_dup2(A, B) (1) > > > > +static EFI_STATUS > > +MpServicesWhoAmI ( > > + IN EFI_MP_SERVICES_PROTOCOL *pMpService, > > + OUT UINTN *pProcessorNumber > > + ) > > +{ > > + return pMpService->WhoAmI (pMpService, pProcessorNumber); } > > + > > +static EFI_STATUS > > +MpServicesGetNumberOfProcessors ( > > + IN EFI_MP_SERVICES_PROTOCOL *pMpService, > > + OUT UINTN *pNumberOfProcessors, > > + OUT UINTN *pNumberOfEnabledProcessors > > + > > + ) > > +{ > > + return pMpService->GetNumberOfProcessors (pMpService, > > +pNumberOfProcessors, pNumberOfEnabledProcessors); } > > + > > +static EFI_STATUS > > +MpServicesSwitchBSP ( > > + IN EFI_MP_SERVICES_PROTOCOL *pMpService, > > + IN UINTN ProcessorNumber > > + ) > > +{ > > + return pMpService->SwitchBSP(pMpService, ProcessorNumber, TRUE); > } > > + > > #ifndef UEFI_C_SOURCE > > /* Return a dictionary corresponding to the POSIX environment table > > */ extern char **environ; @@ -3865,6 +3901,56 @@ > edk2_rdmsr(PyObject > > *self, PyObject *args) > > return Py_BuildValue("(II)", (unsigned long)veax, (unsigned > > long)vedx); } > > > > +PyDoc_STRVAR(efi_rdmsr_ex__doc__, > > +"rdmsr_ex(cpu, msr) -> (lower_32bits, higher_32bits)\n\ \n\ Read > the > > +given msr by switching to cpu and return the data as tuple.\n\ \n\ > > +Parameters:\n\ > > + cpu - The cpu number in hex or int format\n\ > > + msr - The msr in hex or int format\n\ \n\ Return Value:\n\ > > + a tuple with lower and higher 32 bit values read from the > msr\n\ > > +"); > > + > > +static PyObject * > > +edk2_rdmsr_ex(PyObject *self, PyObject *args) { > > + unsigned int cpu, vecx, veax, vedx; > > + unsigned int bsp_switched = 0; > > + EFI_STATUS status = 0; > > + UINT64 data = 0; > > + > > + if (!PyArg_ParseTuple(args, "II", &cpu, &vecx)) > > + return NULL; > > + > > + Py_BEGIN_ALLOW_THREADS > > + if (cpu != gBSPProcessorNumber && cpu < gNumberOfProcessors) { > > + //switch the BSP to the cpu > > + status = MpServicesSwitchBSP(gpMpService, cpu); > > + if (!EFI_ERROR(status)) > > + { > > + bsp_switched = 1; > > + } > > + } > > + > > + data = AsmReadMsr64(vecx); > > + > > + if (bsp_switched) > > + { > > + // switch BSP to the saved BSP processor > > + MpServicesSwitchBSP(gpMpService, gBSPProcessorNumber); > > + // update the saved BSP processor > > + MpServicesWhoAmI(gpMpService, &gBSPProcessorNumber); > > + } > > + Py_END_ALLOW_THREADS > > + veax = (UINT32)data; > > + vedx = (UINT64)data >> 32; > > + return Py_BuildValue("(II)", (unsigned long)veax, (unsigned > > +long)vedx); } > > + > > PyDoc_STRVAR(efi_wrmsr__doc__, > > "wrmsr(msr, lower_32bits, higher_32bits) -> None\n\ \n\ @@ -3889,6 > > +3975,58 @@ edk2_wrmsr(PyObject *self, PyObject *args) > > data = vedx << 32 | veax; > > Py_BEGIN_ALLOW_THREADS > > AsmWriteMsr64(vecx, data); > > + Py_END_ALLOW_THREADS > > + Py_INCREF(Py_None); > > + return Py_None; > > +} > > + > > +PyDoc_STRVAR(efi_wrmsr_ex__doc__, > > +"wrmsr_ex(cpu, msr, lower_32bits, higher_32bits) -> None\n\ \n\ > > Writes > > +higher_32bits:lower_32bits to the given msr.\n\ \n\ Parameters:\n\ > > + cpu - The cpu number in hex or int format\n\ > > + msr - The msr in hex or int format\n\ > > + lower_32bits - The lower 32 bit data for the msr\n\ > > + higher_32bits - The higher 32 bit data for the msr\n\ \n\ > Return > > +Value:\n\ > > + None\n\ > > +"); > > + > > +static PyObject * > > +edk2_wrmsr_ex(PyObject *self, PyObject *args) { > > + unsigned int cpu, msr, veax, vedx; > > + unsigned int bsp_switched = 0; > > + EFI_STATUS status = 0; > > + UINT64 data = 0; > > + > > + if (!PyArg_ParseTuple(args, "IIII", &cpu, &msr, &veax, &vedx)) > > + return NULL; > > + data = (((UINT64)vedx) << 32) | veax; > > + > > + Py_BEGIN_ALLOW_THREADS > > + if (cpu != gBSPProcessorNumber && cpu < gNumberOfProcessors) { > > + //switch the BSP to the cpu > > + status = MpServicesSwitchBSP(gpMpService, cpu); > > + if (!EFI_ERROR(status)) > > + { > > + bsp_switched = 1; > > + } > > + } > > + // write to MSR > > + AsmWriteMsr64(msr, data); > > + > > + if (bsp_switched) > > + { > > + // switch BSP to the saved BSP processor > > + MpServicesSwitchBSP(gpMpService, gBSPProcessorNumber); > > + // update the saved BSP processor > > + MpServicesWhoAmI(gpMpService, &gBSPProcessorNumber); } > > Py_END_ALLOW_THREADS > > Py_INCREF(Py_None); > > return Py_None; > > @@ -4576,7 +4714,9 @@ static PyMethodDef edk2_methods[] = { #endif > > {"abort", edk2_abort, METH_NOARGS, > > edk2_abort__doc__}, > > {"rdmsr", edk2_rdmsr, > METH_VARARGS, > > efi_rdmsr__doc__}, > > + {"rdmsr_ex", edk2_rdmsr_ex, > METH_VARARGS, > > efi_rdmsr_ex__doc__}, > > {"wrmsr", edk2_wrmsr, > METH_VARARGS, > > efi_wrmsr__doc__}, > > + {"wrmsr_ex", edk2_wrmsr_ex, > METH_VARARGS, > > efi_wrmsr_ex__doc__}, > > {"readpci", edk2_readpci, > METH_VARARGS, > > efi_readpci__doc__}, > > {"writepci", edk2_writepci, > METH_VARARGS, > > efi_writepci__doc__}, > > {"readmem", posix_readmem, > > METH_VARARGS, efi_readmem__doc__}, > > @@ -4813,13 +4953,24 @@ static struct PyModuleDef edk2module = { > > PyMODINIT_FUNC > > PyEdk2__Init(void) > > { > > - PyObject *m; > > + PyObject *m; > > + EFI_STATUS Status = 0; > > > > #ifndef UEFI_C_SOURCE > > PyObject *v; > > #endif > > + Status = gBS->LocateProtocol(&gEfiMpServiceProtocolGuid, NULL, > > &gpMpService); > > + if (EFI_ERROR(Status)) > > + { > > + printf("Unable to locate the Protocol MpServices protocol: > > %r\n", Status); > > + return NULL; > > + } > > + // Get the current BSP processor number > > + MpServicesWhoAmI(gpMpService, &gBSPProcessorNumber); > > + // Get the number of processors > > + MpServicesGetNumberOfProcessors(gpMpService, > > &gNumberOfProcessors, > > + &gNumberOfEnabledProcessors); > > > > - m = PyModule_Create(&edk2module); > > + m = PyModule_Create(&edk2module); > > if (m == NULL) > > return m; > > > > @@ -4870,7 +5021,7 @@ PyEdk2__Init(void) > > //PyModule_AddObject(m, "statvfs_result", > > // (PyObject*) &StatVFSResultType); > > initialized = 1; > > - return m; > > + return m; > > > > } > > > > diff --git a/AppPkg/Applications/Python/Python-3.6.8/Python368.inf > > b/AppPkg/Applications/Python/Python-3.6.8/Python368.inf > > index 8b7f677..eca98b5 100644 > > --- a/AppPkg/Applications/Python/Python-3.6.8/Python368.inf > > +++ b/AppPkg/Applications/Python/Python-3.6.8/Python368.inf > > @@ -46,6 +46,9 @@ > > #BsdSocketLib > > #EfiSocketLib > > > > +[Protocols] > > + gEfiMpServiceProtocolGuid ## CONSUMES > > + > > [FixedPcd] > > gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x0F > > gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x80000040 > > -- > > 2.40.0.windows.1 > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118232): https://edk2.groups.io/g/devel/message/118232 Mute This Topic: https://groups.io/mt/105530360/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-