From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 3FB7282412 for ; Thu, 22 Dec 2016 23:05:39 -0800 (PST) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga101.fm.intel.com with ESMTP; 22 Dec 2016 23:05:38 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,392,1477983600"; d="scan'208";a="42699629" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga004.jf.intel.com with ESMTP; 22 Dec 2016 23:05:38 -0800 Received: from fmsmsx151.amr.corp.intel.com (10.18.125.4) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 22 Dec 2016 23:05:38 -0800 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by FMSMSX151.amr.corp.intel.com (10.18.125.4) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 22 Dec 2016 23:05:31 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.88]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.204]) with mapi id 14.03.0248.002; Fri, 23 Dec 2016 15:05:28 +0800 From: "Zeng, Star" To: "Ni, Ruiyu" , "edk2-devel@lists.01.org" CC: "Kinney, Michael D" , "Tian, Feng" , "Zeng, Star" Thread-Topic: [edk2] [PATCH 1/5] MdeModulePkg UsbKbDxe: Execute key notify func at TPL_CALLBACK Thread-Index: AQHSXFmoN7Pk2EWeqUWuL2JIoJiER6EUldwAgACHECA= Date: Fri, 23 Dec 2016 07:05:27 +0000 Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103B7EF748@shsmsx102.ccr.corp.intel.com> References: <1482414310-33036-1-git-send-email-star.zeng@intel.com> <1482414310-33036-2-git-send-email-star.zeng@intel.com> <734D49CCEBEEF84792F5B80ED585239D5B86226D@SHSMSX104.ccr.corp.intel.com> In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5B86226D@SHSMSX104.ccr.corp.intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH 1/5] MdeModulePkg UsbKbDxe: Execute key notify func at TPL_CALLBACK X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 23 Dec 2016 07:05:39 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Ray, Good comment, it can make the code more clear, I agree it. Thanks, Star -----Original Message----- From: Ni, Ruiyu=20 Sent: Friday, December 23, 2016 3:01 PM To: Zeng, Star ; edk2-devel@lists.01.org Cc: Kinney, Michael D ; Tian, Feng ; Zeng, Star Subject: RE: [edk2] [PATCH 1/5] MdeModulePkg UsbKbDxe: Execute key notify f= unc at TPL_CALLBACK Star, I have one comment: When call Enque(), can we put comments above the call to say that the key n= otification function needs to run at TPL_CALLBACK while current TPL is TPL_= NOTIFY. It will be called in KeyNotifyProcessHandler() which runs at TPL_CA= LLBACK? Regards, Ray >-----Original Message----- >From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of=20 >Star Zeng >Sent: Thursday, December 22, 2016 9:45 PM >To: edk2-devel@lists.01.org >Cc: Ni, Ruiyu ; Kinney, Michael D=20 >; Tian, Feng ; Zeng,=20 >Star >Subject: [edk2] [PATCH 1/5] MdeModulePkg UsbKbDxe: Execute key notify=20 >func at TPL_CALLBACK > >Current implementation executes key notify function in TimerHandler at=20 >TPL_NOTIFY. The code change is to make key notify function executed at=20 >TPL_CALLBACK to reduce the time occupied at TPL_NOTIFY. > >The code will signal KeyNotify process event if the key pressed matches=20 >any key registered and insert the KeyData to the EFI Key queue for=20 >notify, then the KeyNotify process handler will invoke key notify=20 >functions at TPL_CALLBACK. > >Cc: Ruiyu Ni >Cc: Michael Kinney >Cc: Feng Tian >Contributed-under: TianoCore Contribution Agreement 1.0 >Signed-off-by: Star Zeng >--- > MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c | 64 +++++++++++++++++++++++++++= ++++- > MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h | 17 ++++++++- > MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 10 +++-- > 3 files changed, 86 insertions(+), 5 deletions(-) > >diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c=20 >b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c >index fb7558b73070..b1e1657173c3 100644 >--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c >+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c >@@ -2,7 +2,7 @@ > USB Keyboard Driver that manages USB keyboard and produces Simple Text = Input > Protocol and Simple Text Input Ex Protocol. > >-Copyright (c) 2004 - 2012, Intel Corporation. All rights reserved.
>+Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.
> This program and the accompanying materials are licensed and made=20 >available under the terms and conditions of the BSD License which=20 >accompanies this distribution. The full text of the license may be=20 >found at @@ -312,6 +312,17 @@ USBKeyboardDriverBindingStart ( > goto ErrorExit; > } > >+ Status =3D gBS->CreateEvent ( >+ EVT_NOTIFY_SIGNAL, >+ TPL_CALLBACK, >+ KeyNotifyProcessHandler, >+ UsbKeyboardDevice, >+ &UsbKeyboardDevice->KeyNotifyProcessEvent >+ ); >+ if (EFI_ERROR (Status)) { >+ goto ErrorExit; >+ } >+ > // > // Install Simple Text Input Protocol and Simple Text Input Ex Protocol > // for the USB keyboard device. >@@ -427,6 +438,9 @@ ErrorExit: > if (UsbKeyboardDevice->SimpleInputEx.WaitForKeyEx !=3D NULL) { > gBS->CloseEvent (UsbKeyboardDevice->SimpleInputEx.WaitForKeyEx); > } >+ if (UsbKeyboardDevice->KeyNotifyProcessEvent !=3D NULL) { >+ gBS->CloseEvent (UsbKeyboardDevice->KeyNotifyProcessEvent); >+ } > if (UsbKeyboardDevice->KeyboardLayoutEvent !=3D NULL) { > ReleaseKeyboardLayoutResources (UsbKeyboardDevice); > gBS->CloseEvent (UsbKeyboardDevice->KeyboardLayoutEvent); >@@ -548,6 +562,7 @@ USBKeyboardDriverBindingStop ( > gBS->CloseEvent (UsbKeyboardDevice->DelayedRecoveryEvent); > gBS->CloseEvent (UsbKeyboardDevice->SimpleInput.WaitForKey); > gBS->CloseEvent (UsbKeyboardDevice->SimpleInputEx.WaitForKeyEx); >+ gBS->CloseEvent (UsbKeyboardDevice->KeyNotifyProcessEvent); > KbdFreeNotifyList (&UsbKeyboardDevice->NotifyList); > > ReleaseKeyboardLayoutResources (UsbKeyboardDevice); @@ -559,6 +574,7=20 >@@ USBKeyboardDriverBindingStop ( > > DestroyQueue (&UsbKeyboardDevice->UsbKeyQueue); > DestroyQueue (&UsbKeyboardDevice->EfiKeyQueue); >+ DestroyQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify); > > FreePool (UsbKeyboardDevice); > >@@ -647,6 +663,7 @@ USBKeyboardReset ( > // > InitQueue (&UsbKeyboardDevice->UsbKeyQueue, sizeof (USB_KEY)); > InitQueue (&UsbKeyboardDevice->EfiKeyQueue, sizeof=20 >(EFI_KEY_DATA)); >+ InitQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, sizeof=20 >+ (EFI_KEY_DATA)); > > return EFI_SUCCESS; > } >@@ -1170,3 +1187,48 @@ USBKeyboardUnregisterKeyNotify ( > return EFI_INVALID_PARAMETER; > } > >+/** >+ Process key notify. >+ >+ @param Event Indicates the event that invoke this func= tion. >+ @param Context Indicates the calling context. >+**/ >+VOID >+EFIAPI >+KeyNotifyProcessHandler ( >+ IN EFI_EVENT Event, >+ IN VOID *Context >+ ) >+{ >+ USB_KB_DEV *UsbKeyboardDevice; >+ EFI_KEY_DATA KeyData; >+ LIST_ENTRY *Link; >+ LIST_ENTRY *NotifyList; >+ KEYBOARD_CONSOLE_IN_EX_NOTIFY *CurrentNotify; >+ EFI_TPL OldTpl; >+ >+ UsbKeyboardDevice =3D (USB_KB_DEV *) Context; >+ >+ // >+ // Invoke notification functions. >+ // >+ NotifyList =3D &UsbKeyboardDevice->NotifyList; while (!IsQueueEmpty=20 >+ (&UsbKeyboardDevice->EfiKeyQueueForNotify)) { >+ // >+ // Enter critical section >+ // >+ OldTpl =3D gBS->RaiseTPL (TPL_NOTIFY); >+ Dequeue (&UsbKeyboardDevice->EfiKeyQueueForNotify, &KeyData, sizeof (= KeyData)); >+ // >+ // Leave critical section >+ // >+ gBS->RestoreTPL (OldTpl); >+ for (Link =3D GetFirstNode (NotifyList); !IsNull (NotifyList, Link); = Link =3D GetNextNode (NotifyList, Link)) { >+ CurrentNotify =3D CR (Link, KEYBOARD_CONSOLE_IN_EX_NOTIFY,=20 >+ NotifyEntry, >USB_KB_CONSOLE_IN_EX_NOTIFY_SIGNATURE); >+ if (IsKeyRegistered (&CurrentNotify->KeyData, &KeyData)) { >+ CurrentNotify->KeyNotificationFn (&KeyData); >+ } >+ } >+ } >+} >+ >diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h=20 >b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h >index 58edb3f65f2b..089f113d5fd4 100644 >--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h >+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h >@@ -1,7 +1,7 @@ > /** @file > Header file for USB Keyboard Driver's Data Structures. > >-Copyright (c) 2004 - 2012, Intel Corporation. All rights reserved.
>+Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.
> This program and the accompanying materials are licensed and made=20 >available under the terms and conditions of the BSD License which=20 >accompanies this distribution. The full text of the license may be=20 >found at @@ -114,6 +114,7 @@ typedef struct { > > USB_SIMPLE_QUEUE UsbKeyQueue; > USB_SIMPLE_QUEUE EfiKeyQueue; >+ USB_SIMPLE_QUEUE EfiKeyQueueForNotify; > BOOLEAN CtrlOn; > BOOLEAN AltOn; > BOOLEAN ShiftOn; >@@ -149,6 +150,7 @@ typedef struct { > // Notification function list > // > LIST_ENTRY NotifyList; >+ EFI_EVENT KeyNotifyProcessEvent; > > // > // Non-spacing key list >@@ -596,5 +598,18 @@ USBKeyboardTimerHandler ( > IN VOID *Context > ); > >+/** >+ Process key notify. >+ >+ @param Event Indicates the event that invoke this func= tion. >+ @param Context Indicates the calling context. >+**/ >+VOID >+EFIAPI >+KeyNotifyProcessHandler ( >+ IN EFI_EVENT Event, >+ IN VOID *Context >+ ); >+ > #endif > >diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c=20 >b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c >index fef1449e3c35..a017d51e3b97 100644 >--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c >+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c >@@ -820,6 +820,7 @@ InitUSBKeyboard ( > > InitQueue (&UsbKeyboardDevice->UsbKeyQueue, sizeof (USB_KEY)); > InitQueue (&UsbKeyboardDevice->EfiKeyQueue, sizeof (EFI_KEY_DATA)); >+ InitQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, sizeof=20 >+ (EFI_KEY_DATA)); > > // > // Use the config out of the descriptor @@ -1665,20 +1666,23 @@=20 >UsbKeyCodeToEfiInputKey ( > KeyData->KeyState.KeyToggleState |=3D EFI_KEY_STATE_EXPOSED; > } > // >- // Invoke notification functions if the key is registered. >+ // Signal KeyNotify process event if this key pressed matches any key r= egistered. > // > NotifyList =3D &UsbKeyboardDevice->NotifyList; > for (Link =3D GetFirstNode (NotifyList); !IsNull (NotifyList, Link); Li= nk =3D GetNextNode (NotifyList, Link)) { > CurrentNotify =3D CR (Link, KEYBOARD_CONSOLE_IN_EX_NOTIFY,=20 >NotifyEntry, USB_KB_CONSOLE_IN_EX_NOTIFY_SIGNATURE); > if (IsKeyRegistered (&CurrentNotify->KeyData, KeyData)) { >- CurrentNotify->KeyNotificationFn (KeyData); >+ // >+ // Insert to the EFI Key queue for notify. >+ // >+ Enqueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, KeyData, sizeof = (*KeyData)); >+ gBS->SignalEvent (UsbKeyboardDevice->KeyNotifyProcessEvent); > } > } > > return EFI_SUCCESS; > } > >- > /** > Create the queue. > >-- >2.7.0.windows.1 > >_______________________________________________ >edk2-devel mailing list >edk2-devel@lists.01.org >https://lists.01.org/mailman/listinfo/edk2-devel