mbox series

[edk2,0/4] MdeModulePkg, StandaloneMmPkg: work around VA vs PA ambiguity

Message ID 20190311153608.3251-1-ard.biesheuvel@linaro.org
Headers show
Series MdeModulePkg, StandaloneMmPkg: work around VA vs PA ambiguity | expand

Message

Ard Biesheuvel March 11, 2019, 3:36 p.m. UTC
This series proposes one possible approach to work around the issue that the
traditional MM and standalone MM implement versions of the communicate protocol
that are fundamentally incompatible from the point of view of the caller.

In traditional MM, the MM communicate protocol takes a physical pointer for
the buffer, so that the SMM execution context can access the memory directly
without having to translate it according to the translation regime of the
caller.

In standalone MM, the buffer that is shared with the MM context is preallocated,
and so it is up to the implementation of the MM communicate protocol to copy the
data from the caller allocated buffer into the preallocated shared buffer. In
order to be able to do so, the DXE driver needs to copy the contents, and for
this it needs to know the virtual address not the physical address.

So this means we have two incompatible versions of the same protocol, and given
that we have even re-used the EFI_SMM_COMMUNICATE_PROTOCOL GUID for the new
EFI_MM_COMMUNICATE_PROTOCOL, we cannot distinguish programmatically between a
MM context that takes physical addresses vs one that takes virtual ones.

Since this is known at build time, one way to deal with this is to have two
different implementations of a library that defines an abstract MmCommunicate()
function, allowing the correct implementation to be selected at integration
time.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Achin Gupta <achin.gupta@arm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
Cc: Jagadeesh Ujja <jagadeesh.ujja@arm.com>

Ard Biesheuvel (4):
  MdeModulePkg: introduce MmCommunicationLib library class
  MdeModulePkg: add implementation of MmCommunicateLib
  StandaloneMmPkg: add implementation of MmCommunicateLib
  MdeModulePkg/VariableSmmRuntimeDxe: switch to MmCommunicateLib library

 MdeModulePkg/MdeModulePkg.dec                                                     |   4 +
 MdeModulePkg/MdeModulePkg.dsc                                                     |   2 +
 MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.inf    |  51 +++++++++
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf              |   4 +-
 StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.inf |  51 +++++++++
 MdeModulePkg/Include/Library/MmCommunicateLib.h                                   |  50 +++++++++
 MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.c      | 114 ++++++++++++++++++++
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c                |  10 +-
 StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.c   | 113 +++++++++++++++++++
 MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.uni    |  19 ++++
 StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.uni |  19 ++++
 11 files changed, 428 insertions(+), 9 deletions(-)
 create mode 100644 MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.inf
 create mode 100644 StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.inf
 create mode 100644 MdeModulePkg/Include/Library/MmCommunicateLib.h
 create mode 100644 MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.c
 create mode 100644 StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.c
 create mode 100644 MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.uni
 create mode 100644 StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmCommunicateLib.uni

-- 
2.20.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Wu, Hao A March 15, 2019, 2:15 a.m. UTC | #1
> -----Original Message-----

> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> Sent: Monday, March 11, 2019 11:36 PM

> To: edk2-devel@lists.01.org

> Cc: Ard Biesheuvel; Wang, Jian J; Wu, Hao A; Zeng, Star; Kinney, Michael D;

> Gao, Liming; Achin Gupta; Yao, Jiewen; Supreeth Venkatesh; Jagadeesh Ujja

> Subject: [PATCH 0/4] MdeModulePkg, StandaloneMmPkg: work around VA

> vs PA ambiguity

> 

> This series proposes one possible approach to work around the issue that the

> traditional MM and standalone MM implement versions of the communicate

> protocol

> that are fundamentally incompatible from the point of view of the caller.

> 

> In traditional MM, the MM communicate protocol takes a physical pointer for

> the buffer, so that the SMM execution context can access the memory

> directly

> without having to translate it according to the translation regime of the

> caller.

> 

> In standalone MM, the buffer that is shared with the MM context is

> preallocated,

> and so it is up to the implementation of the MM communicate protocol to

> copy the

> data from the caller allocated buffer into the preallocated shared buffer. In

> order to be able to do so, the DXE driver needs to copy the contents, and for

> this it needs to know the virtual address not the physical address.

> 

> So this means we have two incompatible versions of the same protocol, and

> given

> that we have even re-used the EFI_SMM_COMMUNICATE_PROTOCOL GUID

> for the new

> EFI_MM_COMMUNICATE_PROTOCOL, we cannot distinguish

> programmatically between a

> MM context that takes physical addresses vs one that takes virtual ones.

> 

> Since this is known at build time, one way to deal with this is to have two

> different implementations of a library that defines an abstract

> MmCommunicate()

> function, allowing the correct implementation to be selected at integration

> time.


Hello Ard,

It seems to me that for platforms that include the VariableSmmRuntimeDxe
driver, they need to add the 'MmCommunicateLib' dependency.

Please grant us some time to evaluate this proposal and its impact. We
will inform you as soon as there is a result. Thanks.


Best Regards,
Hao Wu

> 

> Cc: Jian J Wang <jian.j.wang@intel.com>

> Cc: Hao Wu <hao.a.wu@intel.com>

> Cc: Star Zeng <star.zeng@intel.com>

> Cc: Michael D Kinney <michael.d.kinney@intel.com>

> Cc: Liming Gao <liming.gao@intel.com>

> Cc: Achin Gupta <achin.gupta@arm.com>

> Cc: Jiewen Yao <jiewen.yao@intel.com>

> Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>

> Cc: Jagadeesh Ujja <jagadeesh.ujja@arm.com>

> 

> Ard Biesheuvel (4):

>   MdeModulePkg: introduce MmCommunicationLib library class

>   MdeModulePkg: add implementation of MmCommunicateLib

>   StandaloneMmPkg: add implementation of MmCommunicateLib

>   MdeModulePkg/VariableSmmRuntimeDxe: switch to MmCommunicateLib

> library

> 

>  MdeModulePkg/MdeModulePkg.dec                                                     |   4 +

>  MdeModulePkg/MdeModulePkg.dsc                                                     |   2 +

> 

> MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmC

> ommunicateLib.inf    |  51 +++++++++

> 

> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.i

> nf              |   4 +-

> 

> StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeM

> mCommunicateLib.inf |  51 +++++++++

>  MdeModulePkg/Include/Library/MmCommunicateLib.h                                   |

> 50 +++++++++

> 

> MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmC

> ommunicateLib.c      | 114 ++++++++++++++++++++

> 

> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.

> c                |  10 +-

> 

> StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeM

> mCommunicateLib.c   | 113 +++++++++++++++++++

> 

> MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmC

> ommunicateLib.uni    |  19 ++++

> 

> StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeM

> mCommunicateLib.uni |  19 ++++

>  11 files changed, 428 insertions(+), 9 deletions(-)

>  create mode 100644

> MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmC

> ommunicateLib.inf

>  create mode 100644

> StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeM

> mCommunicateLib.inf

>  create mode 100644

> MdeModulePkg/Include/Library/MmCommunicateLib.h

>  create mode 100644

> MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmC

> ommunicateLib.c

>  create mode 100644

> StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeM

> mCommunicateLib.c

>  create mode 100644

> MdeModulePkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeMmC

> ommunicateLib.uni

>  create mode 100644

> StandaloneMmPkg/Library/RuntimeDxeMmCommunicateLib/RuntimeDxeM

> mCommunicateLib.uni

> 

> --

> 2.20.1


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel March 15, 2019, 8:12 a.m. UTC | #2
On Fri, 15 Mar 2019 at 03:17, Wu, Hao A <hao.a.wu@intel.com> wrote:
>

> > -----Original Message-----

> > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > Sent: Monday, March 11, 2019 11:36 PM

> > To: edk2-devel@lists.01.org

> > Cc: Ard Biesheuvel; Wang, Jian J; Wu, Hao A; Zeng, Star; Kinney, Michael D;

> > Gao, Liming; Achin Gupta; Yao, Jiewen; Supreeth Venkatesh; Jagadeesh Ujja

> > Subject: [PATCH 0/4] MdeModulePkg, StandaloneMmPkg: work around VA

> > vs PA ambiguity

> >

> > This series proposes one possible approach to work around the issue that the

> > traditional MM and standalone MM implement versions of the communicate

> > protocol

> > that are fundamentally incompatible from the point of view of the caller.

> >

> > In traditional MM, the MM communicate protocol takes a physical pointer for

> > the buffer, so that the SMM execution context can access the memory

> > directly

> > without having to translate it according to the translation regime of the

> > caller.

> >

> > In standalone MM, the buffer that is shared with the MM context is

> > preallocated,

> > and so it is up to the implementation of the MM communicate protocol to

> > copy the

> > data from the caller allocated buffer into the preallocated shared buffer. In

> > order to be able to do so, the DXE driver needs to copy the contents, and for

> > this it needs to know the virtual address not the physical address.

> >

> > So this means we have two incompatible versions of the same protocol, and

> > given

> > that we have even re-used the EFI_SMM_COMMUNICATE_PROTOCOL GUID

> > for the new

> > EFI_MM_COMMUNICATE_PROTOCOL, we cannot distinguish

> > programmatically between a

> > MM context that takes physical addresses vs one that takes virtual ones.

> >

> > Since this is known at build time, one way to deal with this is to have two

> > different implementations of a library that defines an abstract

> > MmCommunicate()

> > function, allowing the correct implementation to be selected at integration

> > time.

>

> Hello Ard,

>

> It seems to me that for platforms that include the VariableSmmRuntimeDxe

> driver, they need to add the 'MmCommunicateLib' dependency.

>

> Please grant us some time to evaluate this proposal and its impact. We

> will inform you as soon as there is a result. Thanks.

>


Thank you Hao.

Note that we intend to discuss the issue addressed by this series in
the PIWG call, but the next one is at least two weeks away.

So there is no urgency to reviewing this patch for inclusion, but any
feedback you can give is appreciated.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel