* [RFC][patch] Add a new library class RegisterFilterLib in edk2 to filter/trace port IO/MMIO/MSR access @ 2021-03-08 5:15 Dandan Bi 2021-03-08 15:37 ` [edk2-devel] " Laszlo Ersek 0 siblings, 1 reply; 4+ messages in thread From: Dandan Bi @ 2021-03-08 5:15 UTC (permalink / raw) To: devel; +Cc: Michael D Kinney, Liming Gao, Zhiguang Liu REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3246 1.Purpose: Skip port IO/MMIO/MSR access in some emulatoion env. Trace port IO/MMIO/MSR access. 2.Plan to do in Edk2: Filter and trace in low level APIs in BaseIoLibIntrinsic and BaseLib. Add a new library class (RegisterFilterLib) for the filter and trace functionality. 3.Plan to filter and trace scope in Edk2 : a. Port IO R/W: IA32 X64 (Only filter/trace for IA32 X64) b. MMIO R/W: IA32 X64 EBC ARM AARCH64 RISCV64 (Filter/trace for the Arches supported in BaseIoLibIntrinsic.inf) c. MSR R/W: IA32 X64 (Only filter/trace for IA32 X64, if other ARCH has similar use case can add new APIs per needs) 4.RegisterFilterLib Library Class: a. Add RegisterFilterLib library class for the filter and trace operation. b. Add RegisterFilterLib.h in MdePkg/Include/Library. c. 12 APIs will be added to filter and trace port IO, MMIO and MSR access. d. Add a NULL instance RegisterFilterLibNull in MdePkg/Library.(Verified that null instance will not impact binary size.) e. Platform can implement its own RegisterFilterLib instance. 12 APIs can be divided into 2 categories: 6 [Before] APIs use to check whether need to execute port IO/MMIO/MSR access or do some tracing before access. 6 [After] APIs use to trace after port IO/MMIO/MSR access. The detailed API definitions are included in this patch. For port IO access: FilterBeforeIoRead FilterAfterIoRead FilterBeforeIoWrite FilterAfterIoWrite For MMIO access: FilterBeforeMmIoRead FilterAfterMmIoRead FilterBeforeMmIoWrite FilterAfterMmIoWrite For MSR access: FilterBeforeMsrRead FilterAfterMsrRead FilterBeforeMsrWrite FilterAfterMsrWrite 5.Change and Impact a. Add the RegisterFilterLib libary class and RegisterFilterLibNull instance firstly. b. Update the dsc in edk2 and edk2-platform repo to consume the RegisterFilterLibNull instance. c. Update the BaseLib and IoLib to consume RegisterFilterLib. This is an incompatible change. No code change in BaseLib and IoLib consumers, only need to change dsc to consume new FilterLib instance. Update BaseIoLibIntrinsic.inf and BaseIoLibIntrinsicSev.inf to consume RegisterFilterLib for all supported Arch Update BaseLib.inf to consume RegisterFilterLib only for IA32 and X64 This topic has been reviewed in Tiano Design meeting of 2021/0305 RegisterFilterLib header file and desgin foil can be found in: https://edk2.groups.io/g/devel/files/Designs/2021/0305 Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Cc: Zhiguang Liu <zhiguang.liu@intel.com> Signed-off-by: Dandan Bi <dandan.bi@intel.com> --- MdePkg/Include/Library/RegisterFilterLib.h | 224 +++++++++++++++++++++ 1 file changed, 224 insertions(+) create mode 100644 MdePkg/Include/Library/RegisterFilterLib.h diff --git a/MdePkg/Include/Library/RegisterFilterLib.h b/MdePkg/Include/Library/RegisterFilterLib.h new file mode 100644 index 0000000000..be111304ba --- /dev/null +++ b/MdePkg/Include/Library/RegisterFilterLib.h @@ -0,0 +1,224 @@ +/** @file + Public include file for the Port IO/MMIO/MSR filter Library + +Copyright (c) 2021, Intel Corporation. All rights reserved.<BR> +SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + +#ifndef __REGISTER_FILTER_LIB_H__ +#define __REGISTER_FILTER_LIB_H__ + +typedef enum { + FilterWidth8, + FilterWidth16, + FilterWidth32, + FilterWidth64 +} FILTER_IO_WIDTH; + +/** + Filter IO read operation before read IO port. + It is used to filter IO read operation. + + It will return the flag to decide whether require read real IO port. + It can be used for emulation environment. + + @param[in] Width Signifies the width of the I/O operation. + @param[in] Address The base address of the I/O operation. + @param[in] Buffer The destination buffer to store the results. + +**/ +BOOLEAN +EFIAPI +FilterBeforeIoRead ( + IN FILTER_IO_WIDTH Width, + IN UINTN Address, + IN OUT VOID *Buffer + ); + +/** + Trace IO read operation after read IO port. + It is used to trace IO operation. + + @param[in] Width Signifies the width of the I/O operation. + @param[in] Address The base address of the I/O operation. + @param[in] Buffer The destination buffer to store the results. + +**/ +VOID +EFIAPI +FilterAfterIoRead ( + IN FILTER_IO_WIDTH Width, + IN UINTN Address, + IN VOID *Buffer + ); +/** + Filter IO Write operation before wirte IO port. + It is used to filter IO operation. + + It will return the flag to decide whether require read write IO port. + It can be used for emulation environment. + + @param[in] Width Signifies the width of the I/O operation. + @param[in] Address The base address of the I/O operation. + @param[in] Buffer The source buffer from which to BeforeWrite data. + +**/ +BOOLEAN +EFIAPI +FilterBeforeIoWrite ( + IN FILTER_IO_WIDTH Width, + IN UINTN Address, + IN VOID *Buffer + ); + + /** + Trace IO Write operation after wirte IO port. + It is used to trace IO operation. + + @param[in] Width Signifies the width of the I/O operation. + @param[in] Address The base address of the I/O operation. + @param[in] Buffer The source buffer from which to BeforeWrite data. + +**/ +VOID +EFIAPI +FilterAfterIoWrite ( + IN FILTER_IO_WIDTH Width, + IN UINTN Address, + IN VOID *Buffer + ); + +/** + Filter memory IO before Read operation. + + It will return the flag to decide whether require read real MMIO. + It can be used for emulation environment. + + @param[in] Width Signifies the width of the memory I/O operation. + @param[in] Address The base address of the memory I/O operation. + @param[in] Buffer The destination buffer to store the results. + +**/ +BOOLEAN +EFIAPI +FilterBeforeMmIoRead ( + IN FILTER_IO_WIDTH Width, + IN UINTN Address, + IN OUT VOID *Buffer + ); + +/** + Tracer memory IO after read operation + + @param[in] Width Signifies the width of the memory I/O operation. + @param[in] Address The base address of the memory I/O operation. + @param[in] Buffer The destination buffer to store the results. + +**/ +VOID +EFIAPI +FilterAfterMmIoRead ( + IN FILTER_IO_WIDTH Width, + IN UINTN Address, + IN VOID *Buffer + ); + +/** + Filter memory IO before write operation + + It will return the flag to decide whether require wirte real MMIO. + It can be used for emulation environment. + + @param[in] Width Signifies the width of the memory I/O operation. + @param[in] Address The base address of the memory I/O operation. + @param[in] Buffer The source buffer from which to BeforeWrite data. + +**/ +BOOLEAN +EFIAPI +FilterBeforeMmIoWrite ( + IN FILTER_IO_WIDTH Width, + IN UINTN Address, + IN VOID *Buffer + ); + +/** + Tracer memory IO after write operation + + @param[in] Width Signifies the width of the memory I/O operation. + @param[in] Address The base address of the memory I/O operation. + @param[in] Buffer The source buffer from which to BeforeWrite data. + +**/ +VOID +EFIAPI +FilterAfterMmIoWrite ( + IN FILTER_IO_WIDTH Width, + IN UINTN Address, + IN VOID *Buffer + ); + +/** + Filter MSR before read operation. + + It will return the flag to decide whether require read real MSR. + It can be used for emulation environment. + + @param Index The 8-bit Machine Specific Register index to BeforeWrite. + @param Value The 64-bit value to BeforeRead from the Machine Specific Register. + +**/ +BOOLEAN +EFIAPI +FilterBeforeMsrRead ( + IN UINT32 Index, + IN OUT UINT64 *Value + ); + +/** + Trace MSR after read operation + + @param Index The 8-bit Machine Specific Register index to BeforeWrite. + @param Value The 64-bit value to BeforeRead from the Machine Specific Register. + +**/ +VOID +EFIAPI +FilterAfterMsrRead ( + IN UINT32 Index, + IN UINT64 *Value + ); + +/** + Filter MSR before write operation + + It will return the flag to decide whether require write real MSR. + It can be used for emulation environment. + + @param Index The 8-bit Machine Specific Register index to BeforeWrite. + @param Value The 64-bit value to BeforeWrite to the Machine Specific Register. + +**/ +BOOLEAN +EFIAPI +FilterBeforeMsrWrite ( + IN UINT32 Index, + IN UINT64 *Value + ); + +/** + Trace MSR after write operation + + @param Index The 8-bit Machine Specific Register index to BeforeWrite. + @param Value The 64-bit value to BeforeWrite to the Machine Specific Register. + +**/ +VOID +EFIAPI +FilterAfterMsrWrite ( + IN UINT32 Index, + IN UINT64 *Value + ); + +#endif // __REGISTER_FILTER_LIB_H__ -- 2.18.0.windows.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [RFC][patch] Add a new library class RegisterFilterLib in edk2 to filter/trace port IO/MMIO/MSR access 2021-03-08 5:15 [RFC][patch] Add a new library class RegisterFilterLib in edk2 to filter/trace port IO/MMIO/MSR access Dandan Bi @ 2021-03-08 15:37 ` Laszlo Ersek 2021-03-08 21:55 ` Michael D Kinney 0 siblings, 1 reply; 4+ messages in thread From: Laszlo Ersek @ 2021-03-08 15:37 UTC (permalink / raw) To: devel, dandan.bi; +Cc: Michael D Kinney, Liming Gao, Zhiguang Liu On 03/08/21 06:15, Dandan Bi wrote: > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3246 > > 1.Purpose: > Skip port IO/MMIO/MSR access in some emulatoion env. > Trace port IO/MMIO/MSR access. > > 2.Plan to do in Edk2: > Filter and trace in low level APIs in BaseIoLibIntrinsic and BaseLib. > Add a new library class (RegisterFilterLib) for the filter and trace functionality. > > 3.Plan to filter and trace scope in Edk2 : > a. Port IO R/W: IA32 X64 (Only filter/trace for IA32 X64) > b. MMIO R/W: IA32 X64 EBC ARM AARCH64 RISCV64 (Filter/trace for the Arches supported in BaseIoLibIntrinsic.inf) > c. MSR R/W: IA32 X64 (Only filter/trace for IA32 X64, if other ARCH has similar use case can add new APIs per needs) > > 4.RegisterFilterLib Library Class: > a. Add RegisterFilterLib library class for the filter and trace operation. > b. Add RegisterFilterLib.h in MdePkg/Include/Library. > c. 12 APIs will be added to filter and trace port IO, MMIO and MSR access. > d. Add a NULL instance RegisterFilterLibNull in MdePkg/Library.(Verified that null instance will not impact binary size.) > e. Platform can implement its own RegisterFilterLib instance. > > 12 APIs can be divided into 2 categories: > 6 [Before] APIs use to check whether need to execute port IO/MMIO/MSR access or do some tracing before access. > 6 [After] APIs use to trace after port IO/MMIO/MSR access. > The detailed API definitions are included in this patch. > > For port IO access: > FilterBeforeIoRead > FilterAfterIoRead > FilterBeforeIoWrite > FilterAfterIoWrite > > For MMIO access: > FilterBeforeMmIoRead > FilterAfterMmIoRead > FilterBeforeMmIoWrite > FilterAfterMmIoWrite > > For MSR access: > FilterBeforeMsrRead > FilterAfterMsrRead > FilterBeforeMsrWrite > FilterAfterMsrWrite > > 5.Change and Impact > a. Add the RegisterFilterLib libary class and RegisterFilterLibNull instance firstly. > b. Update the dsc in edk2 and edk2-platform repo to consume the RegisterFilterLibNull instance. > c. Update the BaseLib and IoLib to consume RegisterFilterLib. > > This is an incompatible change. > No code change in BaseLib and IoLib consumers, only need to change dsc to consume new FilterLib instance. > Update BaseIoLibIntrinsic.inf and BaseIoLibIntrinsicSev.inf to consume RegisterFilterLib for all supported Arch > Update BaseLib.inf to consume RegisterFilterLib only for IA32 and X64 Seems like a sound plan, but there are more IoLib instances than the above: MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf MdePkg/Library/DxeIoLibCpuIo2/DxeIoLibCpuIo2.inf MdePkg/Library/PeiIoLibCpuIo/PeiIoLibCpuIo.inf MdePkg/Library/SmmIoLibSmmCpuIo2/SmmIoLibSmmCpuIo2.inf Thanks Laszlo > > This topic has been reviewed in Tiano Design meeting of 2021/0305 > RegisterFilterLib header file and desgin foil can be found in: > https://edk2.groups.io/g/devel/files/Designs/2021/0305 > > > Cc: Michael D Kinney <michael.d.kinney@intel.com> > Cc: Liming Gao <gaoliming@byosoft.com.cn> > Cc: Zhiguang Liu <zhiguang.liu@intel.com> > Signed-off-by: Dandan Bi <dandan.bi@intel.com> > --- > MdePkg/Include/Library/RegisterFilterLib.h | 224 +++++++++++++++++++++ > 1 file changed, 224 insertions(+) > create mode 100644 MdePkg/Include/Library/RegisterFilterLib.h > > diff --git a/MdePkg/Include/Library/RegisterFilterLib.h b/MdePkg/Include/Library/RegisterFilterLib.h > new file mode 100644 > index 0000000000..be111304ba > --- /dev/null > +++ b/MdePkg/Include/Library/RegisterFilterLib.h > @@ -0,0 +1,224 @@ > +/** @file > + Public include file for the Port IO/MMIO/MSR filter Library > + > +Copyright (c) 2021, Intel Corporation. All rights reserved.<BR> > +SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#ifndef __REGISTER_FILTER_LIB_H__ > +#define __REGISTER_FILTER_LIB_H__ > + > +typedef enum { > + FilterWidth8, > + FilterWidth16, > + FilterWidth32, > + FilterWidth64 > +} FILTER_IO_WIDTH; > + > +/** > + Filter IO read operation before read IO port. > + It is used to filter IO read operation. > + > + It will return the flag to decide whether require read real IO port. > + It can be used for emulation environment. > + > + @param[in] Width Signifies the width of the I/O operation. > + @param[in] Address The base address of the I/O operation. > + @param[in] Buffer The destination buffer to store the results. > + > +**/ > +BOOLEAN > +EFIAPI > +FilterBeforeIoRead ( > + IN FILTER_IO_WIDTH Width, > + IN UINTN Address, > + IN OUT VOID *Buffer > + ); > + > +/** > + Trace IO read operation after read IO port. > + It is used to trace IO operation. > + > + @param[in] Width Signifies the width of the I/O operation. > + @param[in] Address The base address of the I/O operation. > + @param[in] Buffer The destination buffer to store the results. > + > +**/ > +VOID > +EFIAPI > +FilterAfterIoRead ( > + IN FILTER_IO_WIDTH Width, > + IN UINTN Address, > + IN VOID *Buffer > + ); > +/** > + Filter IO Write operation before wirte IO port. > + It is used to filter IO operation. > + > + It will return the flag to decide whether require read write IO port. > + It can be used for emulation environment. > + > + @param[in] Width Signifies the width of the I/O operation. > + @param[in] Address The base address of the I/O operation. > + @param[in] Buffer The source buffer from which to BeforeWrite data. > + > +**/ > +BOOLEAN > +EFIAPI > +FilterBeforeIoWrite ( > + IN FILTER_IO_WIDTH Width, > + IN UINTN Address, > + IN VOID *Buffer > + ); > + > + /** > + Trace IO Write operation after wirte IO port. > + It is used to trace IO operation. > + > + @param[in] Width Signifies the width of the I/O operation. > + @param[in] Address The base address of the I/O operation. > + @param[in] Buffer The source buffer from which to BeforeWrite data. > + > +**/ > +VOID > +EFIAPI > +FilterAfterIoWrite ( > + IN FILTER_IO_WIDTH Width, > + IN UINTN Address, > + IN VOID *Buffer > + ); > + > +/** > + Filter memory IO before Read operation. > + > + It will return the flag to decide whether require read real MMIO. > + It can be used for emulation environment. > + > + @param[in] Width Signifies the width of the memory I/O operation. > + @param[in] Address The base address of the memory I/O operation. > + @param[in] Buffer The destination buffer to store the results. > + > +**/ > +BOOLEAN > +EFIAPI > +FilterBeforeMmIoRead ( > + IN FILTER_IO_WIDTH Width, > + IN UINTN Address, > + IN OUT VOID *Buffer > + ); > + > +/** > + Tracer memory IO after read operation > + > + @param[in] Width Signifies the width of the memory I/O operation. > + @param[in] Address The base address of the memory I/O operation. > + @param[in] Buffer The destination buffer to store the results. > + > +**/ > +VOID > +EFIAPI > +FilterAfterMmIoRead ( > + IN FILTER_IO_WIDTH Width, > + IN UINTN Address, > + IN VOID *Buffer > + ); > + > +/** > + Filter memory IO before write operation > + > + It will return the flag to decide whether require wirte real MMIO. > + It can be used for emulation environment. > + > + @param[in] Width Signifies the width of the memory I/O operation. > + @param[in] Address The base address of the memory I/O operation. > + @param[in] Buffer The source buffer from which to BeforeWrite data. > + > +**/ > +BOOLEAN > +EFIAPI > +FilterBeforeMmIoWrite ( > + IN FILTER_IO_WIDTH Width, > + IN UINTN Address, > + IN VOID *Buffer > + ); > + > +/** > + Tracer memory IO after write operation > + > + @param[in] Width Signifies the width of the memory I/O operation. > + @param[in] Address The base address of the memory I/O operation. > + @param[in] Buffer The source buffer from which to BeforeWrite data. > + > +**/ > +VOID > +EFIAPI > +FilterAfterMmIoWrite ( > + IN FILTER_IO_WIDTH Width, > + IN UINTN Address, > + IN VOID *Buffer > + ); > + > +/** > + Filter MSR before read operation. > + > + It will return the flag to decide whether require read real MSR. > + It can be used for emulation environment. > + > + @param Index The 8-bit Machine Specific Register index to BeforeWrite. > + @param Value The 64-bit value to BeforeRead from the Machine Specific Register. > + > +**/ > +BOOLEAN > +EFIAPI > +FilterBeforeMsrRead ( > + IN UINT32 Index, > + IN OUT UINT64 *Value > + ); > + > +/** > + Trace MSR after read operation > + > + @param Index The 8-bit Machine Specific Register index to BeforeWrite. > + @param Value The 64-bit value to BeforeRead from the Machine Specific Register. > + > +**/ > +VOID > +EFIAPI > +FilterAfterMsrRead ( > + IN UINT32 Index, > + IN UINT64 *Value > + ); > + > +/** > + Filter MSR before write operation > + > + It will return the flag to decide whether require write real MSR. > + It can be used for emulation environment. > + > + @param Index The 8-bit Machine Specific Register index to BeforeWrite. > + @param Value The 64-bit value to BeforeWrite to the Machine Specific Register. > + > +**/ > +BOOLEAN > +EFIAPI > +FilterBeforeMsrWrite ( > + IN UINT32 Index, > + IN UINT64 *Value > + ); > + > +/** > + Trace MSR after write operation > + > + @param Index The 8-bit Machine Specific Register index to BeforeWrite. > + @param Value The 64-bit value to BeforeWrite to the Machine Specific Register. > + > +**/ > +VOID > +EFIAPI > +FilterAfterMsrWrite ( > + IN UINT32 Index, > + IN UINT64 *Value > + ); > + > +#endif // __REGISTER_FILTER_LIB_H__ > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [RFC][patch] Add a new library class RegisterFilterLib in edk2 to filter/trace port IO/MMIO/MSR access 2021-03-08 15:37 ` [edk2-devel] " Laszlo Ersek @ 2021-03-08 21:55 ` Michael D Kinney 2021-03-09 12:37 ` Laszlo Ersek 0 siblings, 1 reply; 4+ messages in thread From: Michael D Kinney @ 2021-03-08 21:55 UTC (permalink / raw) To: Laszlo Ersek, devel@edk2.groups.io, Bi, Dandan, Kinney, Michael D Cc: Liming Gao, Liu, Zhiguang > -----Original Message----- > From: Laszlo Ersek <lersek@redhat.com> > Sent: Monday, March 8, 2021 7:38 AM > To: devel@edk2.groups.io; Bi, Dandan <dandan.bi@intel.com> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang > <zhiguang.liu@intel.com> > Subject: Re: [edk2-devel] [RFC][patch] Add a new library class RegisterFilterLib in edk2 to filter/trace port IO/MMIO/MSR > access > > On 03/08/21 06:15, Dandan Bi wrote: > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3246 > > > > 1.Purpose: > > Skip port IO/MMIO/MSR access in some emulatoion env. > > Trace port IO/MMIO/MSR access. > > > > 2.Plan to do in Edk2: > > Filter and trace in low level APIs in BaseIoLibIntrinsic and BaseLib. > > Add a new library class (RegisterFilterLib) for the filter and trace functionality. > > > > 3.Plan to filter and trace scope in Edk2 : > > a. Port IO R/W: IA32 X64 (Only filter/trace for IA32 X64) > > b. MMIO R/W: IA32 X64 EBC ARM AARCH64 RISCV64 (Filter/trace for the Arches supported in BaseIoLibIntrinsic.inf) > > c. MSR R/W: IA32 X64 (Only filter/trace for IA32 X64, if other ARCH has similar use case can add new APIs per needs) > > > > 4.RegisterFilterLib Library Class: > > a. Add RegisterFilterLib library class for the filter and trace operation. > > b. Add RegisterFilterLib.h in MdePkg/Include/Library. > > c. 12 APIs will be added to filter and trace port IO, MMIO and MSR access. > > d. Add a NULL instance RegisterFilterLibNull in MdePkg/Library.(Verified that null instance will not impact binary > size.) > > e. Platform can implement its own RegisterFilterLib instance. > > > > 12 APIs can be divided into 2 categories: > > 6 [Before] APIs use to check whether need to execute port IO/MMIO/MSR access or do some tracing before access. > > 6 [After] APIs use to trace after port IO/MMIO/MSR access. > > The detailed API definitions are included in this patch. > > > > For port IO access: > > FilterBeforeIoRead > > FilterAfterIoRead > > FilterBeforeIoWrite > > FilterAfterIoWrite > > > > For MMIO access: > > FilterBeforeMmIoRead > > FilterAfterMmIoRead > > FilterBeforeMmIoWrite > > FilterAfterMmIoWrite > > > > For MSR access: > > FilterBeforeMsrRead > > FilterAfterMsrRead > > FilterBeforeMsrWrite > > FilterAfterMsrWrite > > > > 5.Change and Impact > > a. Add the RegisterFilterLib libary class and RegisterFilterLibNull instance firstly. > > b. Update the dsc in edk2 and edk2-platform repo to consume the RegisterFilterLibNull instance. > > c. Update the BaseLib and IoLib to consume RegisterFilterLib. > > > > This is an incompatible change. > > No code change in BaseLib and IoLib consumers, only need to change dsc to consume new FilterLib instance. > > Update BaseIoLibIntrinsic.inf and BaseIoLibIntrinsicSev.inf to consume RegisterFilterLib for all supported Arch > > Update BaseLib.inf to consume RegisterFilterLib only for IA32 and X64 > > Seems like a sound plan, but there are more IoLib instances than the above: > > MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf > MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf > MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf I agree that all 3 of these need to be included in the plan. > MdePkg/Library/DxeIoLibCpuIo2/DxeIoLibCpuIo2.inf > MdePkg/Library/PeiIoLibCpuIo/PeiIoLibCpuIo.inf > MdePkg/Library/SmmIoLibSmmCpuIo2/SmmIoLibSmmCpuIo2.inf The IoLib instances all perform their I/O operation by calling a dynamic PPI/Protocol services. I would recommend that we do not update these instances, and instead only apply the RegisterFilterLib to IoLib instances that perform he direct access to the hardware. Any IoLib instances that access the hardware through a PPI/Protocol should not be updated. We have a few implementations of the CPI I/O PPI/Protocol that use the BaseIoLibIntrinsics, so those would actually be covered by the first set of lib instances. If a platform decides to implement a new version of the CPU I/O PPI/Protocol that does not use one of the BaseIoLibInstrinsic instances, then they would have the option of using the RegisterFilterLib in that new implementation of the CPI I/O PPI/Protocol. > > Thanks > Laszlo > > > > > This topic has been reviewed in Tiano Design meeting of 2021/0305 > > RegisterFilterLib header file and desgin foil can be found in: > > https://edk2.groups.io/g/devel/files/Designs/2021/0305 > > > > > > Cc: Michael D Kinney <michael.d.kinney@intel.com> > > Cc: Liming Gao <gaoliming@byosoft.com.cn> > > Cc: Zhiguang Liu <zhiguang.liu@intel.com> > > Signed-off-by: Dandan Bi <dandan.bi@intel.com> > > --- > > MdePkg/Include/Library/RegisterFilterLib.h | 224 +++++++++++++++++++++ > > 1 file changed, 224 insertions(+) > > create mode 100644 MdePkg/Include/Library/RegisterFilterLib.h > > > > diff --git a/MdePkg/Include/Library/RegisterFilterLib.h b/MdePkg/Include/Library/RegisterFilterLib.h > > new file mode 100644 > > index 0000000000..be111304ba > > --- /dev/null > > +++ b/MdePkg/Include/Library/RegisterFilterLib.h > > @@ -0,0 +1,224 @@ > > +/** @file > > + Public include file for the Port IO/MMIO/MSR filter Library > > + > > +Copyright (c) 2021, Intel Corporation. All rights reserved.<BR> > > +SPDX-License-Identifier: BSD-2-Clause-Patent > > + > > +**/ > > + > > +#ifndef __REGISTER_FILTER_LIB_H__ > > +#define __REGISTER_FILTER_LIB_H__ > > + > > +typedef enum { > > + FilterWidth8, > > + FilterWidth16, > > + FilterWidth32, > > + FilterWidth64 > > +} FILTER_IO_WIDTH; > > + > > +/** > > + Filter IO read operation before read IO port. > > + It is used to filter IO read operation. > > + > > + It will return the flag to decide whether require read real IO port. > > + It can be used for emulation environment. > > + > > + @param[in] Width Signifies the width of the I/O operation. > > + @param[in] Address The base address of the I/O operation. > > + @param[in] Buffer The destination buffer to store the results. > > + > > +**/ > > +BOOLEAN > > +EFIAPI > > +FilterBeforeIoRead ( > > + IN FILTER_IO_WIDTH Width, > > + IN UINTN Address, > > + IN OUT VOID *Buffer > > + ); > > + > > +/** > > + Trace IO read operation after read IO port. > > + It is used to trace IO operation. > > + > > + @param[in] Width Signifies the width of the I/O operation. > > + @param[in] Address The base address of the I/O operation. > > + @param[in] Buffer The destination buffer to store the results. > > + > > +**/ > > +VOID > > +EFIAPI > > +FilterAfterIoRead ( > > + IN FILTER_IO_WIDTH Width, > > + IN UINTN Address, > > + IN VOID *Buffer > > + ); > > +/** > > + Filter IO Write operation before wirte IO port. > > + It is used to filter IO operation. > > + > > + It will return the flag to decide whether require read write IO port. > > + It can be used for emulation environment. > > + > > + @param[in] Width Signifies the width of the I/O operation. > > + @param[in] Address The base address of the I/O operation. > > + @param[in] Buffer The source buffer from which to BeforeWrite data. > > + > > +**/ > > +BOOLEAN > > +EFIAPI > > +FilterBeforeIoWrite ( > > + IN FILTER_IO_WIDTH Width, > > + IN UINTN Address, > > + IN VOID *Buffer > > + ); > > + > > + /** > > + Trace IO Write operation after wirte IO port. > > + It is used to trace IO operation. > > + > > + @param[in] Width Signifies the width of the I/O operation. > > + @param[in] Address The base address of the I/O operation. > > + @param[in] Buffer The source buffer from which to BeforeWrite data. > > + > > +**/ > > +VOID > > +EFIAPI > > +FilterAfterIoWrite ( > > + IN FILTER_IO_WIDTH Width, > > + IN UINTN Address, > > + IN VOID *Buffer > > + ); > > + > > +/** > > + Filter memory IO before Read operation. > > + > > + It will return the flag to decide whether require read real MMIO. > > + It can be used for emulation environment. > > + > > + @param[in] Width Signifies the width of the memory I/O operation. > > + @param[in] Address The base address of the memory I/O operation. > > + @param[in] Buffer The destination buffer to store the results. > > + > > +**/ > > +BOOLEAN > > +EFIAPI > > +FilterBeforeMmIoRead ( > > + IN FILTER_IO_WIDTH Width, > > + IN UINTN Address, > > + IN OUT VOID *Buffer > > + ); > > + > > +/** > > + Tracer memory IO after read operation > > + > > + @param[in] Width Signifies the width of the memory I/O operation. > > + @param[in] Address The base address of the memory I/O operation. > > + @param[in] Buffer The destination buffer to store the results. > > + > > +**/ > > +VOID > > +EFIAPI > > +FilterAfterMmIoRead ( > > + IN FILTER_IO_WIDTH Width, > > + IN UINTN Address, > > + IN VOID *Buffer > > + ); > > + > > +/** > > + Filter memory IO before write operation > > + > > + It will return the flag to decide whether require wirte real MMIO. > > + It can be used for emulation environment. > > + > > + @param[in] Width Signifies the width of the memory I/O operation. > > + @param[in] Address The base address of the memory I/O operation. > > + @param[in] Buffer The source buffer from which to BeforeWrite data. > > + > > +**/ > > +BOOLEAN > > +EFIAPI > > +FilterBeforeMmIoWrite ( > > + IN FILTER_IO_WIDTH Width, > > + IN UINTN Address, > > + IN VOID *Buffer > > + ); > > + > > +/** > > + Tracer memory IO after write operation > > + > > + @param[in] Width Signifies the width of the memory I/O operation. > > + @param[in] Address The base address of the memory I/O operation. > > + @param[in] Buffer The source buffer from which to BeforeWrite data. > > + > > +**/ > > +VOID > > +EFIAPI > > +FilterAfterMmIoWrite ( > > + IN FILTER_IO_WIDTH Width, > > + IN UINTN Address, > > + IN VOID *Buffer > > + ); > > + > > +/** > > + Filter MSR before read operation. > > + > > + It will return the flag to decide whether require read real MSR. > > + It can be used for emulation environment. > > + > > + @param Index The 8-bit Machine Specific Register index to BeforeWrite. > > + @param Value The 64-bit value to BeforeRead from the Machine Specific Register. > > + > > +**/ > > +BOOLEAN > > +EFIAPI > > +FilterBeforeMsrRead ( > > + IN UINT32 Index, > > + IN OUT UINT64 *Value > > + ); > > + > > +/** > > + Trace MSR after read operation > > + > > + @param Index The 8-bit Machine Specific Register index to BeforeWrite. > > + @param Value The 64-bit value to BeforeRead from the Machine Specific Register. > > + > > +**/ > > +VOID > > +EFIAPI > > +FilterAfterMsrRead ( > > + IN UINT32 Index, > > + IN UINT64 *Value > > + ); > > + > > +/** > > + Filter MSR before write operation > > + > > + It will return the flag to decide whether require write real MSR. > > + It can be used for emulation environment. > > + > > + @param Index The 8-bit Machine Specific Register index to BeforeWrite. > > + @param Value The 64-bit value to BeforeWrite to the Machine Specific Register. > > + > > +**/ > > +BOOLEAN > > +EFIAPI > > +FilterBeforeMsrWrite ( > > + IN UINT32 Index, > > + IN UINT64 *Value > > + ); > > + > > +/** > > + Trace MSR after write operation > > + > > + @param Index The 8-bit Machine Specific Register index to BeforeWrite. > > + @param Value The 64-bit value to BeforeWrite to the Machine Specific Register. > > + > > +**/ > > +VOID > > +EFIAPI > > +FilterAfterMsrWrite ( > > + IN UINT32 Index, > > + IN UINT64 *Value > > + ); > > + > > +#endif // __REGISTER_FILTER_LIB_H__ > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [RFC][patch] Add a new library class RegisterFilterLib in edk2 to filter/trace port IO/MMIO/MSR access 2021-03-08 21:55 ` Michael D Kinney @ 2021-03-09 12:37 ` Laszlo Ersek 0 siblings, 0 replies; 4+ messages in thread From: Laszlo Ersek @ 2021-03-09 12:37 UTC (permalink / raw) To: Kinney, Michael D, devel@edk2.groups.io, Bi, Dandan Cc: Liming Gao, Liu, Zhiguang On 03/08/21 22:55, Kinney, Michael D wrote: > > >> -----Original Message----- >> From: Laszlo Ersek <lersek@redhat.com> >> Sent: Monday, March 8, 2021 7:38 AM >> To: devel@edk2.groups.io; Bi, Dandan <dandan.bi@intel.com> >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang >> <zhiguang.liu@intel.com> >> Subject: Re: [edk2-devel] [RFC][patch] Add a new library class RegisterFilterLib in edk2 to filter/trace port IO/MMIO/MSR >> access >> >> On 03/08/21 06:15, Dandan Bi wrote: >>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3246 >>> >>> 1.Purpose: >>> Skip port IO/MMIO/MSR access in some emulatoion env. >>> Trace port IO/MMIO/MSR access. >>> >>> 2.Plan to do in Edk2: >>> Filter and trace in low level APIs in BaseIoLibIntrinsic and BaseLib. >>> Add a new library class (RegisterFilterLib) for the filter and trace functionality. >>> >>> 3.Plan to filter and trace scope in Edk2 : >>> a. Port IO R/W: IA32 X64 (Only filter/trace for IA32 X64) >>> b. MMIO R/W: IA32 X64 EBC ARM AARCH64 RISCV64 (Filter/trace for the Arches supported in BaseIoLibIntrinsic.inf) >>> c. MSR R/W: IA32 X64 (Only filter/trace for IA32 X64, if other ARCH has similar use case can add new APIs per needs) >>> >>> 4.RegisterFilterLib Library Class: >>> a. Add RegisterFilterLib library class for the filter and trace operation. >>> b. Add RegisterFilterLib.h in MdePkg/Include/Library. >>> c. 12 APIs will be added to filter and trace port IO, MMIO and MSR access. >>> d. Add a NULL instance RegisterFilterLibNull in MdePkg/Library.(Verified that null instance will not impact binary >> size.) >>> e. Platform can implement its own RegisterFilterLib instance. >>> >>> 12 APIs can be divided into 2 categories: >>> 6 [Before] APIs use to check whether need to execute port IO/MMIO/MSR access or do some tracing before access. >>> 6 [After] APIs use to trace after port IO/MMIO/MSR access. >>> The detailed API definitions are included in this patch. >>> >>> For port IO access: >>> FilterBeforeIoRead >>> FilterAfterIoRead >>> FilterBeforeIoWrite >>> FilterAfterIoWrite >>> >>> For MMIO access: >>> FilterBeforeMmIoRead >>> FilterAfterMmIoRead >>> FilterBeforeMmIoWrite >>> FilterAfterMmIoWrite >>> >>> For MSR access: >>> FilterBeforeMsrRead >>> FilterAfterMsrRead >>> FilterBeforeMsrWrite >>> FilterAfterMsrWrite >>> >>> 5.Change and Impact >>> a. Add the RegisterFilterLib libary class and RegisterFilterLibNull instance firstly. >>> b. Update the dsc in edk2 and edk2-platform repo to consume the RegisterFilterLibNull instance. >>> c. Update the BaseLib and IoLib to consume RegisterFilterLib. >>> >>> This is an incompatible change. >>> No code change in BaseLib and IoLib consumers, only need to change dsc to consume new FilterLib instance. >>> Update BaseIoLibIntrinsic.inf and BaseIoLibIntrinsicSev.inf to consume RegisterFilterLib for all supported Arch >>> Update BaseLib.inf to consume RegisterFilterLib only for IA32 and X64 >> >> Seems like a sound plan, but there are more IoLib instances than the above: >> >> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf >> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf >> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf > > I agree that all 3 of these need to be included in the plan. > >> MdePkg/Library/DxeIoLibCpuIo2/DxeIoLibCpuIo2.inf >> MdePkg/Library/PeiIoLibCpuIo/PeiIoLibCpuIo.inf >> MdePkg/Library/SmmIoLibSmmCpuIo2/SmmIoLibSmmCpuIo2.inf > > The IoLib instances all perform their I/O operation by calling > a dynamic PPI/Protocol services. I would recommend that we do not update > these instances, and instead only apply the RegisterFilterLib to > IoLib instances that perform he direct access to the hardware. > Any IoLib instances that access the hardware through a PPI/Protocol > should not be updated. > > We have a few implementations of the CPI I/O PPI/Protocol that > use the BaseIoLibIntrinsics, so those would actually be covered > by the first set of lib instances. If a platform decides to > implement a new version of the CPU I/O PPI/Protocol that does not > use one of the BaseIoLibInstrinsic instances, then they would > have the option of using the RegisterFilterLib in that new > implementation of the CPI I/O PPI/Protocol. Thanks for the explanation; I agree. Laszlo ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-09 12:37 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-03-08 5:15 [RFC][patch] Add a new library class RegisterFilterLib in edk2 to filter/trace port IO/MMIO/MSR access Dandan Bi 2021-03-08 15:37 ` [edk2-devel] " Laszlo Ersek 2021-03-08 21:55 ` Michael D Kinney 2021-03-09 12:37 ` Laszlo Ersek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox