From: "Yao, Jiewen" <jiewen.yao@intel.com>
To: "Yao, Jiewen" <jiewen.yao@intel.com>,
"Zeng, Star" <star.zeng@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Subject: Re: [PATCH] IntelSiliconPkg IntelVTdDxe: Use TPL to protect list/engine operation
Date: Thu, 4 Jan 2018 06:17:22 +0000 [thread overview]
Message-ID: <74D8A39837DF1E4DA445A8C0B3885C503AA6FDEF@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503AA6FDBB@shsmsx102.ccr.corp.intel.com>
Correct typo.
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao,
> Jiewen
> Sent: Thursday, January 4, 2018 2:14 PM
> To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] IntelSiliconPkg IntelVTdDxe: Use TPL to protect
> list/engine operation
>
> It is good to have lock for linked list management.
>
> However, I do not think we should update TPL for ExitBootServices/LegacyBoot.
>
> I purposely use TPL_CALLBACK to make sure VTd is tear down later, so that other
> driver can stop DMA before that.
>
> Thank you
> Yao Jiewen
>
> > -----Original Message-----
> > From: Zeng, Star
> > Sent: Thursday, January 4, 2018 11:32 AM
> > To: edk2-devel@lists.01.org
> > Cc: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> > Subject: [PATCH] IntelSiliconPkg IntelVTdDxe: Use TPL to protect list/engine
> > operation
> >
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Star Zeng <star.zeng@intel.com>
> > ---
> > IntelSiliconPkg/Feature/VTd/IntelVTdDxe/BmDma.c | 19 ++++++-------
> > .../Feature/VTd/IntelVTdDxe/DmaProtection.c | 8 +++---
> > .../Feature/VTd/IntelVTdDxe/DmaProtection.h | 2 ++
> > .../Feature/VTd/IntelVTdDxe/IntelVTdDxe.c | 32
> > +++++++++-------------
> > 4 files changed, 28 insertions(+), 33 deletions(-)
> >
> > diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/BmDma.c
> > b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/BmDma.c
> > index e8685666e79a..57e086a64dbc 100644
> > --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/BmDma.c
> > +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/BmDma.c
> > @@ -1,7 +1,7 @@
> > /** @file
> > BmDma related function
> >
> > - Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> > + Copyright (c) 2017 - 2018, Intel Corporation. All rights reserved.<BR>
> > This program and the accompanying materials
> > are licensed and made available under the terms and conditions of the BSD
> > License
> > which accompanies this distribution. The full text of the license may be
> > found at
> > @@ -12,15 +12,7 @@
> >
> > **/
> >
> > -#include <PiDxe.h>
> > -
> > -#include <Protocol/IoMmu.h>
> > -
> > -#include <Library/BaseLib.h>
> > -#include <Library/DebugLib.h>
> > -#include <Library/BaseMemoryLib.h>
> > -#include <Library/MemoryAllocationLib.h>
> > -#include <Library/UefiBootServicesTableLib.h>
> > +#include "DmaProtection.h"
> >
> > // TBD: May make it a policy
> > #define DMA_MEMORY_TOP MAX_UINTN
> > @@ -76,6 +68,7 @@ IoMmuMap (
> > MAP_INFO *MapInfo;
> > EFI_PHYSICAL_ADDRESS
> > DmaMemoryTop;
> > BOOLEAN NeedRemap;
> > + EFI_TPL OriginalTpl;
> >
> > if (NumberOfBytes == NULL || DeviceAddress == NULL ||
> > Mapping == NULL) {
> > @@ -198,7 +191,9 @@ IoMmuMap (
> > MapInfo->DeviceAddress = MapInfo->HostAddress;
> > }
> >
> > + OriginalTpl = gBS->RaiseTPL (VTD_TPL_LEVEL);
> > InsertTailList (&gMaps, &MapInfo->Link);
> > + gBS->RestoreTPL (OriginalTpl);
> >
> > //
> > // The DeviceAddress is the address of the maped buffer below 4GB
> > @@ -233,6 +228,7 @@ IoMmuUnmap (
> > {
> > MAP_INFO *MapInfo;
> > LIST_ENTRY *Link;
> > + EFI_TPL OriginalTpl;
> >
> > DEBUG ((DEBUG_VERBOSE, "IoMmuUnmap: 0x%08x\n", Mapping));
> >
> > @@ -241,6 +237,7 @@ IoMmuUnmap (
> > return EFI_INVALID_PARAMETER;
> > }
> >
> > + OriginalTpl = gBS->RaiseTPL (VTD_TPL_LEVEL);
> > MapInfo = NULL;
> > for (Link = GetFirstNode (&gMaps)
> > ; !IsNull (&gMaps, Link)
> > @@ -255,10 +252,12 @@ IoMmuUnmap (
> > // Mapping is not a valid value returned by Map()
> > //
> > if (MapInfo != Mapping) {
> > + gBS->RestoreTPL (OriginalTpl);
> > DEBUG ((DEBUG_ERROR, "IoMmuUnmap: %r\n",
> > EFI_INVALID_PARAMETER));
> > return EFI_INVALID_PARAMETER;
> > }
> > RemoveEntryList (&MapInfo->Link);
> > + gBS->RestoreTPL (OriginalTpl);
> >
> > if (MapInfo->DeviceAddress != MapInfo->HostAddress) {
> > //
> > diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c
> > b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c
> > index 648f64c20b77..013823cc161f 100644
> > --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c
> > +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c
> > @@ -478,7 +478,7 @@ InitializeDmaProtection (
> >
> > Status = gBS->CreateEventEx (
> > EVT_NOTIFY_SIGNAL,
> > - TPL_CALLBACK,
> > + VTD_TPL_LEVEL,
> > AcpiNotificationFunc,
> > NULL,
> > &gEfiAcpi10TableGuid,
> > @@ -488,7 +488,7 @@ InitializeDmaProtection (
> >
> > Status = gBS->CreateEventEx (
> > EVT_NOTIFY_SIGNAL,
> > - TPL_CALLBACK,
> > + VTD_TPL_LEVEL,
> > AcpiNotificationFunc,
> > NULL,
> > &gEfiAcpi20TableGuid,
> > @@ -505,7 +505,7 @@ InitializeDmaProtection (
> >
> > Status = gBS->CreateEventEx (
> > EVT_NOTIFY_SIGNAL,
> > - TPL_CALLBACK,
> > + VTD_TPL_LEVEL,
> > OnExitBootServices,
> > NULL,
> > &gEfiEventExitBootServicesGuid,
> > @@ -514,7 +514,7 @@ InitializeDmaProtection (
> > ASSERT_EFI_ERROR (Status);
> >
> > Status = EfiCreateEventLegacyBootEx (
> > - TPL_CALLBACK,
> > + VTD_TPL_LEVEL,
> > OnLegacyBoot,
> > NULL,
> > &LegacyBootEvent
> > diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.h
> > b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.h
> > index 519a5ab00450..bc14ff9a6631 100644
> > --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.h
> > +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.h
> > @@ -48,6 +48,8 @@
> > #define ALIGN_VALUE_UP(Value, Alignment) (((Value) + (Alignment) - 1) &
> > (~((Alignment) - 1)))
> > #define ALIGN_VALUE_LOW(Value, Alignment) ((Value) & (~((Alignment) -
> 1)))
> >
> > +#define VTD_TPL_LEVEL TPL_NOTIFY
> > +
> > //
> > // This is the initial max PCI DATA number.
> > // The number may be enlarged later.
> > diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c
> > b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c
> > index 89d9bea3fc0f..570b47cf7364 100644
> > --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c
> > +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.c
> > @@ -12,16 +12,6 @@
> >
> > **/
> >
> > -#include <PiDxe.h>
> > -
> > -#include <Protocol/IoMmu.h>
> > -#include <Protocol/PciIo.h>
> > -
> > -#include <Library/IoLib.h>
> > -#include <Library/BaseLib.h>
> > -#include <Library/DebugLib.h>
> > -#include <Library/UefiBootServicesTableLib.h>
> > -
> > #include "DmaProtection.h"
> >
> > /**
> > @@ -306,18 +296,22 @@ IoMmuSetAttribute (
> > EFI_STATUS Status;
> > EFI_PHYSICAL_ADDRESS DeviceAddress;
> > UINTN NumberOfPages;
> > + EFI_TPL OriginalTpl;
> > +
> > + OriginalTpl = gBS->RaiseTPL (VTD_TPL_LEVEL);
> >
> > Status = GetDeviceInfoFromMapping (Mapping, &DeviceAddress,
> > &NumberOfPages);
> > - if (EFI_ERROR(Status)) {
> > - return Status;
> > + if (!EFI_ERROR(Status)) {
> > + Status = VTdSetAttribute (
> > + This,
> > + DeviceHandle,
> > + DeviceAddress,
> > + EFI_PAGES_TO_SIZE(NumberOfPages),
> > + IoMmuAccess
> > + );
> > }
> > - Status = VTdSetAttribute (
> > - This,
> > - DeviceHandle,
> > - DeviceAddress,
> > - EFI_PAGES_TO_SIZE(NumberOfPages),
> > - IoMmuAccess
> > - );
> > +
> > + gBS->RestoreTPL (OriginalTpl);
> >
> > return Status;
> > }
> > --
> > 2.7.0.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
next prev parent reply other threads:[~2018-01-04 6:12 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-04 3:32 [PATCH] IntelSiliconPkg IntelVTdDxe: Use TPL to protect list/engine operation Star Zeng
2018-01-04 6:13 ` Yao, Jiewen
2018-01-04 6:17 ` Yao, Jiewen [this message]
2018-01-04 6:17 ` Zeng, Star
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=74D8A39837DF1E4DA445A8C0B3885C503AA6FDEF@shsmsx102.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox