* [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