[edk2] [PATCH 00/10] MdeModulePkg, OvmfPkg: unmap DMA buffers at ExitBootServices

Laszlo Ersek posted 10 patches 6 years, 7 months ago
Failed in applying to current master (apply log)
MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 103 +++++---
MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h |   7 +
OvmfPkg/IoMmuDxe/AmdSevIoMmu.c                           | 246 +++++++++++++++++---
OvmfPkg/VirtioBlkDxe/VirtioBlk.c                         |   7 +-
OvmfPkg/VirtioGpuDxe/Commands.c                          |  23 +-
OvmfPkg/VirtioRngDxe/VirtioRng.c                         |   7 +-
OvmfPkg/VirtioScsiDxe/VirtioScsi.c                       |   7 +-
7 files changed, 299 insertions(+), 101 deletions(-)
[edk2] [PATCH 00/10] MdeModulePkg, OvmfPkg: unmap DMA buffers at ExitBootServices
Posted by Laszlo Ersek 6 years, 7 months ago
Repo:   https://github.com/lersek/edk2.git
Branch: iommu_exit_boot

This series is the result of the discussion under

  [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common
                     buffers at ExitBootServices()
  https://lists.01.org/pipermail/edk2-devel/2017-September/014099.html

At ExitBootServices(), PCI and VirtIo drivers should only care about
aborting pending DMA on the devices. Cleaning up PciIo mappings (which
ultimately boil down to IOMMU mappings) for those aborted DMA operations
should be the job of the IOMMU driver.

Patches 01 through 03 clean up the AtaAtapiPassThru driver in
MdeModulePkg a little bit, because at present, (a) it unmaps the buffers
and disables BMDMA in the wrong order in its DriverBindingStop()
function, (b) it doesn't abort pending DMA at ExitBootServices().

This subset can be treated separately from the rest of the series, but I
thought they belonged loosely together (given that AtaAtapiPassThru is
used on QEMU's Q35 machine type).

Patches 04 through 07 remove VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer()
calls from the VirtIo drivers' ExitBootServices() handlers.

(The conversion of VirtioNetDxe to device addresses is still in progress
-- Brijesh, when you submit v2 of that, under this approach, there is no
need to change VirtioNetExitBoot() relative to current upstream, and you
can use VirtioOperationBusMasterRead to map outgoing packets.)

Patches 08 through 10 make OvmfPkg/IoMmuDxe track all mappings, and
unmap all mappings (Read, Write, CommonBuffer) that are in effect when
ExitBootServices() is called. It is ensured that PCI and VirtIo drivers
abort pending DMA first, and IoMmuDxe clean up the mappings last.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Star Zeng <star.zeng@intel.com>

Thanks
Laszlo

Laszlo Ersek (10):
  MdeModulePkg/AtaAtapiPassThru: cache EnabledPciAttributes
  MdeModulePkg/AtaAtapiPassThru: unmap DMA buffers after disabling BM
    DMA
  MdeModulePkg/AtaAtapiPassThru: disable the device at
    ExitBootServices()
  OvmfPkg/VirtioBlkDxe: don't unmap VRING at ExitBootServices()
  OvmfPkg/VirtioGpuDxe: don't unmap VRING & BackingStore at
    ExitBootServices
  OvmfPkg/VirtioRngDxe: don't unmap VRING at ExitBootServices()
  OvmfPkg/VirtioScsiDxe: don't unmap VRING at ExitBootServices()
  OvmfPkg/IoMmuDxe: track all mappings
  OvmfPkg/IoMmuDxe: generalize IoMmuUnmap() to IoMmuUnmapWorker()
  OvmfPkg/IoMmuDxe: unmap all IOMMU mappings at ExitBootServices()

 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 103 +++++---
 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h |   7 +
 OvmfPkg/IoMmuDxe/AmdSevIoMmu.c                           | 246 +++++++++++++++++---
 OvmfPkg/VirtioBlkDxe/VirtioBlk.c                         |   7 +-
 OvmfPkg/VirtioGpuDxe/Commands.c                          |  23 +-
 OvmfPkg/VirtioRngDxe/VirtioRng.c                         |   7 +-
 OvmfPkg/VirtioScsiDxe/VirtioScsi.c                       |   7 +-
 7 files changed, 299 insertions(+), 101 deletions(-)

-- 
2.14.1.3.gb7cf6e02401b

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 00/10] MdeModulePkg, OvmfPkg: unmap DMA buffers at ExitBootServices
Posted by Yao, Jiewen 6 years, 7 months ago
Patch 1~3 reviewed-by: Jiewen.yao@intel.com


> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Friday, September 8, 2017 6:41 AM
> To: edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Brijesh Singh
> <brijesh.singh@amd.com>; Dong, Eric <eric.dong@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Zeng,
> Star <star.zeng@intel.com>
> Subject: [PATCH 00/10] MdeModulePkg, OvmfPkg: unmap DMA buffers at
> ExitBootServices
> 
> Repo:   https://github.com/lersek/edk2.git
> Branch: iommu_exit_boot
> 
> This series is the result of the discussion under
> 
>   [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common
>                      buffers at ExitBootServices()
>   https://lists.01.org/pipermail/edk2-devel/2017-September/014099.html
> 
> At ExitBootServices(), PCI and VirtIo drivers should only care about
> aborting pending DMA on the devices. Cleaning up PciIo mappings (which
> ultimately boil down to IOMMU mappings) for those aborted DMA operations
> should be the job of the IOMMU driver.
> 
> Patches 01 through 03 clean up the AtaAtapiPassThru driver in
> MdeModulePkg a little bit, because at present, (a) it unmaps the buffers
> and disables BMDMA in the wrong order in its DriverBindingStop()
> function, (b) it doesn't abort pending DMA at ExitBootServices().
> 
> This subset can be treated separately from the rest of the series, but I
> thought they belonged loosely together (given that AtaAtapiPassThru is
> used on QEMU's Q35 machine type).
> 
> Patches 04 through 07 remove
> VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer()
> calls from the VirtIo drivers' ExitBootServices() handlers.
> 
> (The conversion of VirtioNetDxe to device addresses is still in progress
> -- Brijesh, when you submit v2 of that, under this approach, there is no
> need to change VirtioNetExitBoot() relative to current upstream, and you
> can use VirtioOperationBusMasterRead to map outgoing packets.)
> 
> Patches 08 through 10 make OvmfPkg/IoMmuDxe track all mappings, and
> unmap all mappings (Read, Write, CommonBuffer) that are in effect when
> ExitBootServices() is called. It is ensured that PCI and VirtIo drivers
> abort pending DMA first, and IoMmuDxe clean up the mappings last.
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> 
> Thanks
> Laszlo
> 
> Laszlo Ersek (10):
>   MdeModulePkg/AtaAtapiPassThru: cache EnabledPciAttributes
>   MdeModulePkg/AtaAtapiPassThru: unmap DMA buffers after disabling BM
>     DMA
>   MdeModulePkg/AtaAtapiPassThru: disable the device at
>     ExitBootServices()
>   OvmfPkg/VirtioBlkDxe: don't unmap VRING at ExitBootServices()
>   OvmfPkg/VirtioGpuDxe: don't unmap VRING & BackingStore at
>     ExitBootServices
>   OvmfPkg/VirtioRngDxe: don't unmap VRING at ExitBootServices()
>   OvmfPkg/VirtioScsiDxe: don't unmap VRING at ExitBootServices()
>   OvmfPkg/IoMmuDxe: track all mappings
>   OvmfPkg/IoMmuDxe: generalize IoMmuUnmap() to IoMmuUnmapWorker()
>   OvmfPkg/IoMmuDxe: unmap all IOMMU mappings at ExitBootServices()
> 
>  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 103 +++++---
>  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h |   7 +
>  OvmfPkg/IoMmuDxe/AmdSevIoMmu.c                           | 246
> +++++++++++++++++---
>  OvmfPkg/VirtioBlkDxe/VirtioBlk.c                         |   7 +-
>  OvmfPkg/VirtioGpuDxe/Commands.c                          |  23 +-
>  OvmfPkg/VirtioRngDxe/VirtioRng.c                         |   7 +-
>  OvmfPkg/VirtioScsiDxe/VirtioScsi.c                       |   7 +-
>  7 files changed, 299 insertions(+), 101 deletions(-)
> 
> --
> 2.14.1.3.gb7cf6e02401b

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 00/10] MdeModulePkg, OvmfPkg: unmap DMA buffers at ExitBootServices
Posted by Yao, Jiewen 6 years, 7 months ago
Patch 8~10: reviewed-by: Jiewen.yao@intel.com

From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao, Jiewen
Sent: Friday, September 8, 2017 3:28 PM
To: Laszlo Ersek <lersek@redhat.com>; edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Brijesh Singh <brijesh.singh@amd.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: Re: [edk2] [PATCH 00/10] MdeModulePkg, OvmfPkg: unmap DMA buffers at ExitBootServices

Patch 1~3 reviewed-by: Jiewen.yao@intel.com<mailto:Jiewen.yao@intel.com>


> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Friday, September 8, 2017 6:41 AM
> To: edk2-devel-01 <edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>>; Brijesh Singh
> <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>; Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>; Yao, Jiewen
> <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Justen, Jordan L <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>; Zeng,
> Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>
> Subject: [PATCH 00/10] MdeModulePkg, OvmfPkg: unmap DMA buffers at
> ExitBootServices
>
> Repo:   https://github.com/lersek/edk2.git
> Branch: iommu_exit_boot
>
> This series is the result of the discussion under
>
>   [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common
>                      buffers at ExitBootServices()
>   https://lists.01.org/pipermail/edk2-devel/2017-September/014099.html
>
> At ExitBootServices(), PCI and VirtIo drivers should only care about
> aborting pending DMA on the devices. Cleaning up PciIo mappings (which
> ultimately boil down to IOMMU mappings) for those aborted DMA operations
> should be the job of the IOMMU driver.
>
> Patches 01 through 03 clean up the AtaAtapiPassThru driver in
> MdeModulePkg a little bit, because at present, (a) it unmaps the buffers
> and disables BMDMA in the wrong order in its DriverBindingStop()
> function, (b) it doesn't abort pending DMA at ExitBootServices().
>
> This subset can be treated separately from the rest of the series, but I
> thought they belonged loosely together (given that AtaAtapiPassThru is
> used on QEMU's Q35 machine type).
>
> Patches 04 through 07 remove
> VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer()
> calls from the VirtIo drivers' ExitBootServices() handlers.
>
> (The conversion of VirtioNetDxe to device addresses is still in progress
> -- Brijesh, when you submit v2 of that, under this approach, there is no
> need to change VirtioNetExitBoot() relative to current upstream, and you
> can use VirtioOperationBusMasterRead to map outgoing packets.)
>
> Patches 08 through 10 make OvmfPkg/IoMmuDxe track all mappings, and
> unmap all mappings (Read, Write, CommonBuffer) that are in effect when
> ExitBootServices() is called. It is ensured that PCI and VirtIo drivers
> abort pending DMA first, and IoMmuDxe clean up the mappings last.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>>
> Cc: Brijesh Singh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>
> Cc: Eric Dong <eric.dong@intel.com<mailto:eric.dong@intel.com>>
> Cc: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
> Cc: Jordan Justen <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>
> Cc: Star Zeng <star.zeng@intel.com<mailto:star.zeng@intel.com>>
>
> Thanks
> Laszlo
>
> Laszlo Ersek (10):
>   MdeModulePkg/AtaAtapiPassThru: cache EnabledPciAttributes
>   MdeModulePkg/AtaAtapiPassThru: unmap DMA buffers after disabling BM
>     DMA
>   MdeModulePkg/AtaAtapiPassThru: disable the device at
>     ExitBootServices()
>   OvmfPkg/VirtioBlkDxe: don't unmap VRING at ExitBootServices()
>   OvmfPkg/VirtioGpuDxe: don't unmap VRING & BackingStore at
>     ExitBootServices
>   OvmfPkg/VirtioRngDxe: don't unmap VRING at ExitBootServices()
>   OvmfPkg/VirtioScsiDxe: don't unmap VRING at ExitBootServices()
>   OvmfPkg/IoMmuDxe: track all mappings
>   OvmfPkg/IoMmuDxe: generalize IoMmuUnmap() to IoMmuUnmapWorker()
>   OvmfPkg/IoMmuDxe: unmap all IOMMU mappings at ExitBootServices()
>
>  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 103 +++++---
>  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h |   7 +
>  OvmfPkg/IoMmuDxe/AmdSevIoMmu.c                           | 246
> +++++++++++++++++---
>  OvmfPkg/VirtioBlkDxe/VirtioBlk.c                         |   7 +-
>  OvmfPkg/VirtioGpuDxe/Commands.c                          |  23 +-
>  OvmfPkg/VirtioRngDxe/VirtioRng.c                         |   7 +-
>  OvmfPkg/VirtioScsiDxe/VirtioScsi.c                       |   7 +-
>  7 files changed, 299 insertions(+), 101 deletions(-)
>
> --
> 2.14.1.3.gb7cf6e02401b

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 00/10] MdeModulePkg, OvmfPkg: unmap DMA buffers at ExitBootServices
Posted by Laszlo Ersek 6 years, 7 months ago
On 09/08/17 00:41, Laszlo Ersek wrote:
> Repo:   https://github.com/lersek/edk2.git
> Branch: iommu_exit_boot
> 
> This series is the result of the discussion under
> 
>   [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common
>                      buffers at ExitBootServices()
>   https://lists.01.org/pipermail/edk2-devel/2017-September/014099.html
> 
> At ExitBootServices(), PCI and VirtIo drivers should only care about
> aborting pending DMA on the devices. Cleaning up PciIo mappings (which
> ultimately boil down to IOMMU mappings) for those aborted DMA operations
> should be the job of the IOMMU driver.
> 
> Patches 01 through 03 clean up the AtaAtapiPassThru driver in
> MdeModulePkg a little bit, because at present, (a) it unmaps the buffers
> and disables BMDMA in the wrong order in its DriverBindingStop()
> function, (b) it doesn't abort pending DMA at ExitBootServices().
> 
> This subset can be treated separately from the rest of the series, but I
> thought they belonged loosely together (given that AtaAtapiPassThru is
> used on QEMU's Q35 machine type).
> 
> Patches 04 through 07 remove VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer()
> calls from the VirtIo drivers' ExitBootServices() handlers.
> 
> (The conversion of VirtioNetDxe to device addresses is still in progress
> -- Brijesh, when you submit v2 of that, under this approach, there is no
> need to change VirtioNetExitBoot() relative to current upstream, and you
> can use VirtioOperationBusMasterRead to map outgoing packets.)
> 
> Patches 08 through 10 make OvmfPkg/IoMmuDxe track all mappings, and
> unmap all mappings (Read, Write, CommonBuffer) that are in effect when
> ExitBootServices() is called. It is ensured that PCI and VirtIo drivers
> abort pending DMA first, and IoMmuDxe clean up the mappings last.
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> 
> Thanks
> Laszlo
> 
> Laszlo Ersek (10):
>   MdeModulePkg/AtaAtapiPassThru: cache EnabledPciAttributes
>   MdeModulePkg/AtaAtapiPassThru: unmap DMA buffers after disabling BM
>     DMA
>   MdeModulePkg/AtaAtapiPassThru: disable the device at
>     ExitBootServices()
>   OvmfPkg/VirtioBlkDxe: don't unmap VRING at ExitBootServices()
>   OvmfPkg/VirtioGpuDxe: don't unmap VRING & BackingStore at
>     ExitBootServices
>   OvmfPkg/VirtioRngDxe: don't unmap VRING at ExitBootServices()
>   OvmfPkg/VirtioScsiDxe: don't unmap VRING at ExitBootServices()
>   OvmfPkg/IoMmuDxe: track all mappings
>   OvmfPkg/IoMmuDxe: generalize IoMmuUnmap() to IoMmuUnmapWorker()
>   OvmfPkg/IoMmuDxe: unmap all IOMMU mappings at ExitBootServices()
> 
>  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 103 +++++---
>  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h |   7 +
>  OvmfPkg/IoMmuDxe/AmdSevIoMmu.c                           | 246 +++++++++++++++++---
>  OvmfPkg/VirtioBlkDxe/VirtioBlk.c                         |   7 +-
>  OvmfPkg/VirtioGpuDxe/Commands.c                          |  23 +-
>  OvmfPkg/VirtioRngDxe/VirtioRng.c                         |   7 +-
>  OvmfPkg/VirtioScsiDxe/VirtioScsi.c                       |   7 +-
>  7 files changed, 299 insertions(+), 101 deletions(-)
> 

Supreme kudos to everyone for the feedback!; series pushed as commit
range 3281ebb4ae7d..7aee391fa3d0.

Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 00/10] MdeModulePkg, OvmfPkg: unmap DMA buffers at ExitBootServices
Posted by Brijesh Singh 6 years, 7 months ago
Patch 4 - 10

Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Tested-by: Brijesh Singh <brijesh.singh@amd.com>

Thank you Laszlo! I will work to finish virtio-net next week.

-Brijesh


On 09/07/2017 05:41 PM, Laszlo Ersek wrote:
> Repo:   https://github.com/lersek/edk2.git
> Branch: iommu_exit_boot
> 
> This series is the result of the discussion under
> 
>    [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common
>                       buffers at ExitBootServices()
>    https://lists.01.org/pipermail/edk2-devel/2017-September/014099.html
> 
> At ExitBootServices(), PCI and VirtIo drivers should only care about
> aborting pending DMA on the devices. Cleaning up PciIo mappings (which
> ultimately boil down to IOMMU mappings) for those aborted DMA operations
> should be the job of the IOMMU driver.
> 
> Patches 01 through 03 clean up the AtaAtapiPassThru driver in
> MdeModulePkg a little bit, because at present, (a) it unmaps the buffers
> and disables BMDMA in the wrong order in its DriverBindingStop()
> function, (b) it doesn't abort pending DMA at ExitBootServices().
> 
> This subset can be treated separately from the rest of the series, but I
> thought they belonged loosely together (given that AtaAtapiPassThru is
> used on QEMU's Q35 machine type).
> 
> Patches 04 through 07 remove VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer()
> calls from the VirtIo drivers' ExitBootServices() handlers.
> 
> (The conversion of VirtioNetDxe to device addresses is still in progress
> -- Brijesh, when you submit v2 of that, under this approach, there is no
> need to change VirtioNetExitBoot() relative to current upstream, and you
> can use VirtioOperationBusMasterRead to map outgoing packets.)
> 
> Patches 08 through 10 make OvmfPkg/IoMmuDxe track all mappings, and
> unmap all mappings (Read, Write, CommonBuffer) that are in effect when
> ExitBootServices() is called. It is ensured that PCI and VirtIo drivers
> abort pending DMA first, and IoMmuDxe clean up the mappings last.
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> 
> Thanks
> Laszlo
> 
> Laszlo Ersek (10):
>    MdeModulePkg/AtaAtapiPassThru: cache EnabledPciAttributes
>    MdeModulePkg/AtaAtapiPassThru: unmap DMA buffers after disabling BM
>      DMA
>    MdeModulePkg/AtaAtapiPassThru: disable the device at
>      ExitBootServices()
>    OvmfPkg/VirtioBlkDxe: don't unmap VRING at ExitBootServices()
>    OvmfPkg/VirtioGpuDxe: don't unmap VRING & BackingStore at
>      ExitBootServices
>    OvmfPkg/VirtioRngDxe: don't unmap VRING at ExitBootServices()
>    OvmfPkg/VirtioScsiDxe: don't unmap VRING at ExitBootServices()
>    OvmfPkg/IoMmuDxe: track all mappings
>    OvmfPkg/IoMmuDxe: generalize IoMmuUnmap() to IoMmuUnmapWorker()
>    OvmfPkg/IoMmuDxe: unmap all IOMMU mappings at ExitBootServices()
> 
>   MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 103 +++++---
>   MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h |   7 +
>   OvmfPkg/IoMmuDxe/AmdSevIoMmu.c                           | 246 +++++++++++++++++---
>   OvmfPkg/VirtioBlkDxe/VirtioBlk.c                         |   7 +-
>   OvmfPkg/VirtioGpuDxe/Commands.c                          |  23 +-
>   OvmfPkg/VirtioRngDxe/VirtioRng.c                         |   7 +-
>   OvmfPkg/VirtioScsiDxe/VirtioScsi.c                       |   7 +-
>   7 files changed, 299 insertions(+), 101 deletions(-)
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 00/10] MdeModulePkg, OvmfPkg: unmap DMA buffers at ExitBootServices
Posted by Laszlo Ersek 6 years, 7 months ago
On 09/08/17 17:50, Brijesh Singh wrote:
> Patch 4 - 10
> 
> Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
> Tested-by: Brijesh Singh <brijesh.singh@amd.com>
> 
> Thank you Laszlo! I will work to finish virtio-net next week.

Awesome!

Thanks!
Laszlo

> On 09/07/2017 05:41 PM, Laszlo Ersek wrote:
>> Repo:   https://github.com/lersek/edk2.git
>> Branch: iommu_exit_boot
>>
>> This series is the result of the discussion under
>>
>>    [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common
>>                       buffers at ExitBootServices()
>>    https://lists.01.org/pipermail/edk2-devel/2017-September/014099.html
>>
>> At ExitBootServices(), PCI and VirtIo drivers should only care about
>> aborting pending DMA on the devices. Cleaning up PciIo mappings (which
>> ultimately boil down to IOMMU mappings) for those aborted DMA operations
>> should be the job of the IOMMU driver.
>>
>> Patches 01 through 03 clean up the AtaAtapiPassThru driver in
>> MdeModulePkg a little bit, because at present, (a) it unmaps the buffers
>> and disables BMDMA in the wrong order in its DriverBindingStop()
>> function, (b) it doesn't abort pending DMA at ExitBootServices().
>>
>> This subset can be treated separately from the rest of the series, but I
>> thought they belonged loosely together (given that AtaAtapiPassThru is
>> used on QEMU's Q35 machine type).
>>
>> Patches 04 through 07 remove VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer()
>> calls from the VirtIo drivers' ExitBootServices() handlers.
>>
>> (The conversion of VirtioNetDxe to device addresses is still in progress
>> -- Brijesh, when you submit v2 of that, under this approach, there is no
>> need to change VirtioNetExitBoot() relative to current upstream, and you
>> can use VirtioOperationBusMasterRead to map outgoing packets.)
>>
>> Patches 08 through 10 make OvmfPkg/IoMmuDxe track all mappings, and
>> unmap all mappings (Read, Write, CommonBuffer) that are in effect when
>> ExitBootServices() is called. It is ensured that PCI and VirtIo drivers
>> abort pending DMA first, and IoMmuDxe clean up the mappings last.
>>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Cc: Brijesh Singh <brijesh.singh@amd.com>
>> Cc: Eric Dong <eric.dong@intel.com>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Star Zeng <star.zeng@intel.com>
>>
>> Thanks
>> Laszlo
>>
>> Laszlo Ersek (10):
>>    MdeModulePkg/AtaAtapiPassThru: cache EnabledPciAttributes
>>    MdeModulePkg/AtaAtapiPassThru: unmap DMA buffers after disabling BM
>>      DMA
>>    MdeModulePkg/AtaAtapiPassThru: disable the device at
>>      ExitBootServices()
>>    OvmfPkg/VirtioBlkDxe: don't unmap VRING at ExitBootServices()
>>    OvmfPkg/VirtioGpuDxe: don't unmap VRING & BackingStore at
>>      ExitBootServices
>>    OvmfPkg/VirtioRngDxe: don't unmap VRING at ExitBootServices()
>>    OvmfPkg/VirtioScsiDxe: don't unmap VRING at ExitBootServices()
>>    OvmfPkg/IoMmuDxe: track all mappings
>>    OvmfPkg/IoMmuDxe: generalize IoMmuUnmap() to IoMmuUnmapWorker()
>>    OvmfPkg/IoMmuDxe: unmap all IOMMU mappings at ExitBootServices()
>>
>>   MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 103 +++++---
>>   MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h |   7 +
>>   OvmfPkg/IoMmuDxe/AmdSevIoMmu.c                           | 246
>> +++++++++++++++++---
>>   OvmfPkg/VirtioBlkDxe/VirtioBlk.c                         |   7 +-
>>   OvmfPkg/VirtioGpuDxe/Commands.c                          |  23 +-
>>   OvmfPkg/VirtioRngDxe/VirtioRng.c                         |   7 +-
>>   OvmfPkg/VirtioScsiDxe/VirtioScsi.c                       |   7 +-
>>   7 files changed, 299 insertions(+), 101 deletions(-)
>>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 00/10] MdeModulePkg, OvmfPkg: unmap DMA buffers at ExitBootServices
Posted by Ard Biesheuvel 6 years, 7 months ago
(cc'ing the trinity)

On 7 September 2017 at 23:41, Laszlo Ersek <lersek@redhat.com> wrote:
> Repo:   https://github.com/lersek/edk2.git
> Branch: iommu_exit_boot
>
> This series is the result of the discussion under
>
>   [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common
>                      buffers at ExitBootServices()
>   https://lists.01.org/pipermail/edk2-devel/2017-September/014099.html
>
> At ExitBootServices(), PCI and VirtIo drivers should only care about
> aborting pending DMA on the devices. Cleaning up PciIo mappings (which
> ultimately boil down to IOMMU mappings) for those aborted DMA operations
> should be the job of the IOMMU driver.
>
> Patches 01 through 03 clean up the AtaAtapiPassThru driver in
> MdeModulePkg a little bit, because at present, (a) it unmaps the buffers
> and disables BMDMA in the wrong order in its DriverBindingStop()
> function, (b) it doesn't abort pending DMA at ExitBootServices().
>
> This subset can be treated separately from the rest of the series, but I
> thought they belonged loosely together (given that AtaAtapiPassThru is
> used on QEMU's Q35 machine type).
>
> Patches 04 through 07 remove VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer()
> calls from the VirtIo drivers' ExitBootServices() handlers.
>
> (The conversion of VirtioNetDxe to device addresses is still in progress
> -- Brijesh, when you submit v2 of that, under this approach, there is no
> need to change VirtioNetExitBoot() relative to current upstream, and you
> can use VirtioOperationBusMasterRead to map outgoing packets.)
>
> Patches 08 through 10 make OvmfPkg/IoMmuDxe track all mappings, and
> unmap all mappings (Read, Write, CommonBuffer) that are in effect when
> ExitBootServices() is called. It is ensured that PCI and VirtIo drivers
> abort pending DMA first, and IoMmuDxe clean up the mappings last.
>

The patches look fine to me

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Given that we are now depending on events signalled in an event
handler to be queued after all currently pending events, I think we
need to explicitly agree that this behavior that needs to be
preserved, and documented somewhere, given that the UEFI spec does not
offer this guarantee.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 00/10] MdeModulePkg, OvmfPkg: unmap DMA buffers at ExitBootServices
Posted by Laszlo Ersek 6 years, 7 months ago
On 09/08/17 10:53, Ard Biesheuvel wrote:
> (cc'ing the trinity)
> 
> On 7 September 2017 at 23:41, Laszlo Ersek <lersek@redhat.com> wrote:
>> Repo:   https://github.com/lersek/edk2.git
>> Branch: iommu_exit_boot
>>
>> This series is the result of the discussion under
>>
>>   [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common
>>                      buffers at ExitBootServices()
>>   https://lists.01.org/pipermail/edk2-devel/2017-September/014099.html
>>
>> At ExitBootServices(), PCI and VirtIo drivers should only care about
>> aborting pending DMA on the devices. Cleaning up PciIo mappings (which
>> ultimately boil down to IOMMU mappings) for those aborted DMA operations
>> should be the job of the IOMMU driver.
>>
>> Patches 01 through 03 clean up the AtaAtapiPassThru driver in
>> MdeModulePkg a little bit, because at present, (a) it unmaps the buffers
>> and disables BMDMA in the wrong order in its DriverBindingStop()
>> function, (b) it doesn't abort pending DMA at ExitBootServices().
>>
>> This subset can be treated separately from the rest of the series, but I
>> thought they belonged loosely together (given that AtaAtapiPassThru is
>> used on QEMU's Q35 machine type).
>>
>> Patches 04 through 07 remove VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer()
>> calls from the VirtIo drivers' ExitBootServices() handlers.
>>
>> (The conversion of VirtioNetDxe to device addresses is still in progress
>> -- Brijesh, when you submit v2 of that, under this approach, there is no
>> need to change VirtioNetExitBoot() relative to current upstream, and you
>> can use VirtioOperationBusMasterRead to map outgoing packets.)
>>
>> Patches 08 through 10 make OvmfPkg/IoMmuDxe track all mappings, and
>> unmap all mappings (Read, Write, CommonBuffer) that are in effect when
>> ExitBootServices() is called. It is ensured that PCI and VirtIo drivers
>> abort pending DMA first, and IoMmuDxe clean up the mappings last.
>>
> 
> The patches look fine to me
> 
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Thank you!

> Given that we are now depending on events signalled in an event
> handler to be queued after all currently pending events,

marking this (*)

> I think we
> need to explicitly agree that this behavior that needs to be
> preserved, and documented somewhere, given that the UEFI spec does not
> offer this guarantee.

The condition that you describe under (*) *is* guaranteed in the UEFI spec.

The *only* bit that is edk2 specific in the last patch is that we invoke
gBS->SignalEvent() from the notification function of an event that
*specifically* has type EVT_SIGNAL_EXIT_BOOT_SERVICES.

If the event, whose notification function we were calling
gBS->SignalEvent() from, was a plain EVT_NOTIFY_SIGNAL type event, then
*nothing* in the last patch would be edk2-specific.

* It is guaranteed by the UEFI spec that signaling an event group queues
the notification functions for all not-yet-signaled events in that
group, before the first notification function is invoked (regardless of
the signaled events' TPLs). From "CreateEventEx()": "All events are
guaranteed to be signaled before the first notification action is taken."

* The UEFI spec guarantees that, within the same TPL, if an event is
signaled that is not pending yet, the notify function will be queued
after notify functions already queued on the same TPL. See "CreateEvent()":

    Events exist in one of two states, “waiting” or “signaled.” When an
    event is created, firmware puts it in the “waiting” state. When the
    event is signaled, firmware changes its state to “signaled” and, if
    EVT_NOTIFY_SIGNAL is specified, places a call to its notification
    function in a FIFO queue. There is a queue for each of the “basic”
    task priority levels defined in Section 7.1 (TPL_CALLBACK, and
    TPL_NOTIFY). The functions in these queues are invoked in FIFO
    order, starting with the highest priority level queue and proceeding
    to the lowest priority queue that is unmasked by the current TPL. If
    the current TPL is equal to or greater than the queued notification,
    it will wait until the TPL is lowered via
    EFI_BOOT_SERVICES.RestoreTPL().

* gBS->SignalEvent() is valid to call at TPL_CALLBACK and TPL_NOTIFY
levels (see "Table 23. TPL Restrictions"); in fact it is one of the few
services that can even be called at TPL_HIGH_LEVEL (which is reserved
for the platform firmware).

The upshot is:

(1) assume you have Event1, Event2, Event3, Event4 in event group
EventGroupFoobarGuid

(2) assume all events are EVT_NOTIFY_SIGNAL type

(3) assume Event1 and Event2 have TPL_NOTIFY, Event3 and Event4 are
TPL_CALLBACK

(4) Assume Event5 is also of type EVT_NOTIFY_SIGNAL, not part of any
event group, and has TPL_CALLBACK task prio level

(5) Assume an agent running at TPL_APPLICATION creates an event
temporarily, with type 0 (see the code example in CreateEventEx), and
signals EventGroupFoobarGuid through it.

(6) All of Event1, Event2, Event3 and Event4 move into the signaled
state at once. NotifyFunction1 and NotifyFunction2 are queued in
unspecified order in the TPL_NOTIFY queue, and Event3 and Event4 are
queued in unspecified order in the TPL_CALLBACK queue.

(7) Because our current TPL is TPL_APPLICATION, and signaled events of
type EVT_NOTIFY_SIGNAL with higher TPLs exist, the SignalEvent() service
will start dispatching them immediately.

(8) Either NotifyFunction1 or NotifyFunction2 is called first, running
at TPL_NOTIFY.

(9) Assume that NotifyFunction2 signals Event5. (it doesn't matter if
NotifyFunction2 is called before or after NotifyFunction1).

NotifyFunction5 is not dispatched immediately (on the stack of
SignalEvent()) because the current TPL, TPL_NOTIFY, is higher than or
equal to, the TPL of Event5, which is TPL_CALLBACK. So NotifyFunction5
is queued instead.

Because NotifyFunction3 and NotifyFunction4 are already in the
TPL_CALLBACK queue (in unspecified relative order), from step (6),
NotifyFunction5 will be queued after both of them, in FIFO order.

(10) If NotifyFunction1 has not been invoked yet, it is invoked now. It
runs at TPL_NOTIFY. The TPL_NOTIFY queue becomes empty.

(11) NotifyFunction3 and NotifyFunction4 are invoked in unspecified
order. They both run at TPL_CALLBACK.

(12) NotifyFunction5 is invoked. It runs at TPL_CALLBACK. The
TPL_CALLBACK queue becomes empty.

(13) SignalEvent() returns to the agent mentioned in (5).


The one thing to remember, from the client programmer's POV, is that
*any* event of type EVT_NOTIFY_SIGNAL can only be dispatched from within
two service calls:

- SignalEvent(), if the caller is running at a TPL strictly below the
TPL of the event being signaled (directly or via event group GUID), and

- RestoreTPL(), if the restored TPL falls strictly below the TPL of the
event, and the event has already been signaled.

IOW, SignalEvent() "calls or queues", and RestoreTPL "de-queues and
calls, or does nothing".


So, again, the only question that the UEFI spec does not clarify is
whether we can call gBS->SignalEvent() specifically from an EBS
notification function.

The intent is likely just that, because:

  EVT_SIGNAL_EXIT_BOOT_SERVICES
    [...] This event is of type EVT_NOTIFY_SIGNAL [...]

and, again, if we call SignalEvent from the NotifyFunction of a plain
EVT_NOTIFY_SIGNAL event, then it's all covered by the spec already.

My apologies about the wall of text. I probably over-explained it five-fold.

Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 00/10] MdeModulePkg, OvmfPkg: unmap DMA buffers at ExitBootServices
Posted by Ard Biesheuvel 6 years, 7 months ago
On 8 September 2017 at 10:48, Laszlo Ersek <lersek@redhat.com> wrote:
> On 09/08/17 10:53, Ard Biesheuvel wrote:
>> (cc'ing the trinity)
>>
>> On 7 September 2017 at 23:41, Laszlo Ersek <lersek@redhat.com> wrote:
>>> Repo:   https://github.com/lersek/edk2.git
>>> Branch: iommu_exit_boot
>>>
>>> This series is the result of the discussion under
>>>
>>>   [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common
>>>                      buffers at ExitBootServices()
>>>   https://lists.01.org/pipermail/edk2-devel/2017-September/014099.html
>>>
>>> At ExitBootServices(), PCI and VirtIo drivers should only care about
>>> aborting pending DMA on the devices. Cleaning up PciIo mappings (which
>>> ultimately boil down to IOMMU mappings) for those aborted DMA operations
>>> should be the job of the IOMMU driver.
>>>
>>> Patches 01 through 03 clean up the AtaAtapiPassThru driver in
>>> MdeModulePkg a little bit, because at present, (a) it unmaps the buffers
>>> and disables BMDMA in the wrong order in its DriverBindingStop()
>>> function, (b) it doesn't abort pending DMA at ExitBootServices().
>>>
>>> This subset can be treated separately from the rest of the series, but I
>>> thought they belonged loosely together (given that AtaAtapiPassThru is
>>> used on QEMU's Q35 machine type).
>>>
>>> Patches 04 through 07 remove VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer()
>>> calls from the VirtIo drivers' ExitBootServices() handlers.
>>>
>>> (The conversion of VirtioNetDxe to device addresses is still in progress
>>> -- Brijesh, when you submit v2 of that, under this approach, there is no
>>> need to change VirtioNetExitBoot() relative to current upstream, and you
>>> can use VirtioOperationBusMasterRead to map outgoing packets.)
>>>
>>> Patches 08 through 10 make OvmfPkg/IoMmuDxe track all mappings, and
>>> unmap all mappings (Read, Write, CommonBuffer) that are in effect when
>>> ExitBootServices() is called. It is ensured that PCI and VirtIo drivers
>>> abort pending DMA first, and IoMmuDxe clean up the mappings last.
>>>
>>
>> The patches look fine to me
>>
>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> Thank you!
>
>> Given that we are now depending on events signalled in an event
>> handler to be queued after all currently pending events,
>
> marking this (*)
>
>> I think we
>> need to explicitly agree that this behavior that needs to be
>> preserved, and documented somewhere, given that the UEFI spec does not
>> offer this guarantee.
>
> The condition that you describe under (*) *is* guaranteed in the UEFI spec.
>
> The *only* bit that is edk2 specific in the last patch is that we invoke
> gBS->SignalEvent() from the notification function of an event that
> *specifically* has type EVT_SIGNAL_EXIT_BOOT_SERVICES.
>
> If the event, whose notification function we were calling
> gBS->SignalEvent() from, was a plain EVT_NOTIFY_SIGNAL type event, then
> *nothing* in the last patch would be edk2-specific.
>
> * It is guaranteed by the UEFI spec that signaling an event group queues
> the notification functions for all not-yet-signaled events in that
> group, before the first notification function is invoked (regardless of
> the signaled events' TPLs). From "CreateEventEx()": "All events are
> guaranteed to be signaled before the first notification action is taken."
>
> * The UEFI spec guarantees that, within the same TPL, if an event is
> signaled that is not pending yet, the notify function will be queued
> after notify functions already queued on the same TPL. See "CreateEvent()":
>
>     Events exist in one of two states, “waiting” or “signaled.” When an
>     event is created, firmware puts it in the “waiting” state. When the
>     event is signaled, firmware changes its state to “signaled” and, if
>     EVT_NOTIFY_SIGNAL is specified, places a call to its notification
>     function in a FIFO queue. There is a queue for each of the “basic”
>     task priority levels defined in Section 7.1 (TPL_CALLBACK, and
>     TPL_NOTIFY). The functions in these queues are invoked in FIFO
>     order, starting with the highest priority level queue and proceeding
>     to the lowest priority queue that is unmasked by the current TPL. If
>     the current TPL is equal to or greater than the queued notification,
>     it will wait until the TPL is lowered via
>     EFI_BOOT_SERVICES.RestoreTPL().
>
> * gBS->SignalEvent() is valid to call at TPL_CALLBACK and TPL_NOTIFY
> levels (see "Table 23. TPL Restrictions"); in fact it is one of the few
> services that can even be called at TPL_HIGH_LEVEL (which is reserved
> for the platform firmware).
>
> The upshot is:
>
> (1) assume you have Event1, Event2, Event3, Event4 in event group
> EventGroupFoobarGuid
>
> (2) assume all events are EVT_NOTIFY_SIGNAL type
>
> (3) assume Event1 and Event2 have TPL_NOTIFY, Event3 and Event4 are
> TPL_CALLBACK
>
> (4) Assume Event5 is also of type EVT_NOTIFY_SIGNAL, not part of any
> event group, and has TPL_CALLBACK task prio level
>
> (5) Assume an agent running at TPL_APPLICATION creates an event
> temporarily, with type 0 (see the code example in CreateEventEx), and
> signals EventGroupFoobarGuid through it.
>
> (6) All of Event1, Event2, Event3 and Event4 move into the signaled
> state at once. NotifyFunction1 and NotifyFunction2 are queued in
> unspecified order in the TPL_NOTIFY queue, and Event3 and Event4 are
> queued in unspecified order in the TPL_CALLBACK queue.
>
> (7) Because our current TPL is TPL_APPLICATION, and signaled events of
> type EVT_NOTIFY_SIGNAL with higher TPLs exist, the SignalEvent() service
> will start dispatching them immediately.
>
> (8) Either NotifyFunction1 or NotifyFunction2 is called first, running
> at TPL_NOTIFY.
>
> (9) Assume that NotifyFunction2 signals Event5. (it doesn't matter if
> NotifyFunction2 is called before or after NotifyFunction1).
>
> NotifyFunction5 is not dispatched immediately (on the stack of
> SignalEvent()) because the current TPL, TPL_NOTIFY, is higher than or
> equal to, the TPL of Event5, which is TPL_CALLBACK. So NotifyFunction5
> is queued instead.
>
> Because NotifyFunction3 and NotifyFunction4 are already in the
> TPL_CALLBACK queue (in unspecified relative order), from step (6),
> NotifyFunction5 will be queued after both of them, in FIFO order.
>
> (10) If NotifyFunction1 has not been invoked yet, it is invoked now. It
> runs at TPL_NOTIFY. The TPL_NOTIFY queue becomes empty.
>
> (11) NotifyFunction3 and NotifyFunction4 are invoked in unspecified
> order. They both run at TPL_CALLBACK.
>
> (12) NotifyFunction5 is invoked. It runs at TPL_CALLBACK. The
> TPL_CALLBACK queue becomes empty.
>
> (13) SignalEvent() returns to the agent mentioned in (5).
>
>
> The one thing to remember, from the client programmer's POV, is that
> *any* event of type EVT_NOTIFY_SIGNAL can only be dispatched from within
> two service calls:
>
> - SignalEvent(), if the caller is running at a TPL strictly below the
> TPL of the event being signaled (directly or via event group GUID), and
>
> - RestoreTPL(), if the restored TPL falls strictly below the TPL of the
> event, and the event has already been signaled.
>
> IOW, SignalEvent() "calls or queues", and RestoreTPL "de-queues and
> calls, or does nothing".
>
>
> So, again, the only question that the UEFI spec does not clarify is
> whether we can call gBS->SignalEvent() specifically from an EBS
> notification function.
>
> The intent is likely just that, because:
>
>   EVT_SIGNAL_EXIT_BOOT_SERVICES
>     [...] This event is of type EVT_NOTIFY_SIGNAL [...]
>
> and, again, if we call SignalEvent from the NotifyFunction of a plain
> EVT_NOTIFY_SIGNAL event, then it's all covered by the spec already.
>
> My apologies about the wall of text. I probably over-explained it five-fold.
>

OK
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 00/10] MdeModulePkg, OvmfPkg: unmap DMA buffers at ExitBootServices
Posted by Zeng, Star 6 years, 7 months ago
Reviewed-by: Star Zeng <star.zeng@intel.com> to MdeModulePkg changes.

-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com] 
Sent: Friday, September 8, 2017 6:41 AM
To: edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Brijesh Singh <brijesh.singh@amd.com>; Dong, Eric <eric.dong@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: [PATCH 00/10] MdeModulePkg, OvmfPkg: unmap DMA buffers at ExitBootServices

Repo:   https://github.com/lersek/edk2.git
Branch: iommu_exit_boot

This series is the result of the discussion under

  [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common
                     buffers at ExitBootServices()
  https://lists.01.org/pipermail/edk2-devel/2017-September/014099.html

At ExitBootServices(), PCI and VirtIo drivers should only care about aborting pending DMA on the devices. Cleaning up PciIo mappings (which ultimately boil down to IOMMU mappings) for those aborted DMA operations should be the job of the IOMMU driver.

Patches 01 through 03 clean up the AtaAtapiPassThru driver in MdeModulePkg a little bit, because at present, (a) it unmaps the buffers and disables BMDMA in the wrong order in its DriverBindingStop() function, (b) it doesn't abort pending DMA at ExitBootServices().

This subset can be treated separately from the rest of the series, but I thought they belonged loosely together (given that AtaAtapiPassThru is used on QEMU's Q35 machine type).

Patches 04 through 07 remove VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer()
calls from the VirtIo drivers' ExitBootServices() handlers.

(The conversion of VirtioNetDxe to device addresses is still in progress
-- Brijesh, when you submit v2 of that, under this approach, there is no need to change VirtioNetExitBoot() relative to current upstream, and you can use VirtioOperationBusMasterRead to map outgoing packets.)

Patches 08 through 10 make OvmfPkg/IoMmuDxe track all mappings, and unmap all mappings (Read, Write, CommonBuffer) that are in effect when
ExitBootServices() is called. It is ensured that PCI and VirtIo drivers abort pending DMA first, and IoMmuDxe clean up the mappings last.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Star Zeng <star.zeng@intel.com>

Thanks
Laszlo

Laszlo Ersek (10):
  MdeModulePkg/AtaAtapiPassThru: cache EnabledPciAttributes
  MdeModulePkg/AtaAtapiPassThru: unmap DMA buffers after disabling BM
    DMA
  MdeModulePkg/AtaAtapiPassThru: disable the device at
    ExitBootServices()
  OvmfPkg/VirtioBlkDxe: don't unmap VRING at ExitBootServices()
  OvmfPkg/VirtioGpuDxe: don't unmap VRING & BackingStore at
    ExitBootServices
  OvmfPkg/VirtioRngDxe: don't unmap VRING at ExitBootServices()
  OvmfPkg/VirtioScsiDxe: don't unmap VRING at ExitBootServices()
  OvmfPkg/IoMmuDxe: track all mappings
  OvmfPkg/IoMmuDxe: generalize IoMmuUnmap() to IoMmuUnmapWorker()
  OvmfPkg/IoMmuDxe: unmap all IOMMU mappings at ExitBootServices()

 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 103 +++++---
 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h |   7 +
 OvmfPkg/IoMmuDxe/AmdSevIoMmu.c                           | 246 +++++++++++++++++---
 OvmfPkg/VirtioBlkDxe/VirtioBlk.c                         |   7 +-
 OvmfPkg/VirtioGpuDxe/Commands.c                          |  23 +-
 OvmfPkg/VirtioRngDxe/VirtioRng.c                         |   7 +-
 OvmfPkg/VirtioScsiDxe/VirtioScsi.c                       |   7 +-
 7 files changed, 299 insertions(+), 101 deletions(-)

--
2.14.1.3.gb7cf6e02401b

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