From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (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 2FFFA8235B for ; Thu, 22 Dec 2016 23:01:13 -0800 (PST) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga103.fm.intel.com with ESMTP; 22 Dec 2016 23:01:12 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,392,1477983600"; d="scan'208";a="42698875" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga004.jf.intel.com with ESMTP; 22 Dec 2016 23:01:11 -0800 Received: from fmsmsx123.amr.corp.intel.com (10.18.125.38) 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:01:11 -0800 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by fmsmsx123.amr.corp.intel.com (10.18.125.38) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 22 Dec 2016 23:01:11 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.59]) by SHSMSX103.ccr.corp.intel.com ([10.239.4.69]) with mapi id 14.03.0248.002; Fri, 23 Dec 2016 15:01:09 +0800 From: "Ni, Ruiyu" To: "Zeng, Star" , "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: AQHSXFmo7uyo/T0M7ECAo43YrT6g+aEVGq+A Date: Fri, 23 Dec 2016 07:01:08 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5B86226D@SHSMSX104.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> In-Reply-To: <1482414310-33036-2-git-send-email-star.zeng@intel.com> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiM2U1MzE4YzctMmZmZS00NGYwLWJlNTQtYTFlMzY4MDM3MzlmIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IlI3ZytreVJxeXZqS2ZIeTZWUmJZMEN5OUVodUpuTlVCczA1U2FQcDVmQTQ9In0= x-ctpclassification: CTP_IC 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:01:13 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable 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 w= ill be called in KeyNotifyProcessHandler() which runs at TPL_CALLBACK? Regards, Ray >-----Original Message----- >From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Sta= r Zeng >Sent: Thursday, December 22, 2016 9:45 PM >To: edk2-devel@lists.01.org >Cc: Ni, Ruiyu ; Kinney, Michael D ; Tian, Feng ; >Zeng, Star >Subject: [edk2] [PATCH 1/5] MdeModulePkg UsbKbDxe: Execute key notify func= at TPL_CALLBACK > >Current implementation executes key notify function in TimerHandler >at TPL_NOTIFY. The code change is to make key notify function >executed at TPL_CALLBACK to reduce the time occupied at TPL_NOTIFY. > >The code will signal KeyNotify process event if the key pressed >matches any key registered and insert the KeyData to the EFI Key >queue for notify, then the KeyNotify process handler will invoke >key notify 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 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 available under the terms and conditions of the BSD= License > which accompanies this distribution. The full text of the license may be= 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 @@ 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 (EFI_KEY_DATA)); >+ InitQueue (&UsbKeyboardDevice->EfiKeyQueueForNotify, sizeof (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 (&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, NotifyEn= try, >USB_KB_CONSOLE_IN_EX_NOTIFY_SIGNATURE); >+ if (IsKeyRegistered (&CurrentNotify->KeyData, &KeyData)) { >+ CurrentNotify->KeyNotificationFn (&KeyData); >+ } >+ } >+ } >+} >+ >diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h 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 available under the terms and conditions of the BSD= License > which accompanies this distribution. The full text of the license may be= 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 b/MdeModulePkg/Bus/U= sb/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 (EFI_KEY_DA= TA)); > > // > // Use the config out of the descriptor >@@ -1665,20 +1666,23 @@ 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, NotifyEntr= y, >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