[PATCH] MdeModulePkg/Bus/Pci/XhciDxe: Fix the Bug of clean InputContext in XhcEvaluateContext64();

[PATCH] MdeModulePkg/Bus/Pci/XhciDxe: Fix the Bug of clean InputContext in XhcEvaluateContext64();

[Jiading Zhang]

[-- Attachment #1: Type: text/plain, Size: 868 bytes --]

The value of InputContext structure is initialized in XhcInitializeDeviceSlot/XhcInitializeDeviceSlot64, it shouldn't be cleared when used in XhcEvaluateContext64().

Signed-off-by: jdzhang <jdzhang@kunluntech.com.cn>
---
MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
index c2906e06fd..efbbe247c1 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
@@ -4021,7 +4021,6 @@ XhcEvaluateContext64 (
// 4.6.7 Evaluate Context
//
InputContext = Xhc->UsbDevContext[SlotId].InputContext;
-  ZeroMem (InputContext, sizeof (INPUT_CONTEXT_64));

InputContext->InputControlContext.Dword2 |= BIT1;
InputContext->EP[0].MaxPacketSize         = MaxPacketSize;
--
2.20.1.windows.1

[-- Attachment #2: Type: text/html, Size: 1258 bytes --]

Re: [edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/XhciDxe: Fix the Bug of clean InputContext in XhcEvaluateContext64();

[Wu, Hao A]

[-- Attachment #1: Type: text/plain, Size: 2492 bytes --]

Hello,

For the proposed patch, may I know what kind of issue was met?

After referring to the xHCI spec, my take is that the “Input Context” is different (and should be re-initialed) for each xHC command.

1. Under section 4.6.5 Address Device of the xHCI 1.1 spec:
The Add Context flags A0 and A1 of the Input Control Context data structure (in the Input Context) shall be
set to ‘1’, and all remaining Add Context and Drop Context flags shall all be cleared to ‘0’.

System software shall initialize Slot Context and Endpoint Context 0 entries of the Input Context. All other
Endpoint Contexts in the Input Context shall be ignored by the xHC during the execution of this command.

2. And for section 4.6.7 Evaluate Context:
The state of the Add Context flags depends on the specific endpoints
affected by the command. All Drop Context flags of the Input Control Context shall be cleared to ‘0’ (these
flags do not apply to the Evaluate Context Command). System software shall initialize Contexts of the
Input Context affected by the command. All Contexts not referenced by an Add Context flag in the Input
Context are ignored by the xHC.

Could you help to provide more information for the background of the patch? Thanks in advance.

Best Regards,
Hao Wu

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jiading Zhang
Sent: Wednesday, September 14, 2022 3:15 PM
To: devel@edk2.groups.io
Subject: [edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/XhciDxe: Fix the Bug of clean InputContext in XhcEvaluateContext64();

The value of InputContext structure is initialized in XhcInitializeDeviceSlot/XhcInitializeDeviceSlot64, it shouldn't be cleared when used in XhcEvaluateContext64().

Signed-off-by: jdzhang <jdzhang@kunluntech.com.cn<mailto:jdzhang@kunluntech.com.cn>>
---
 MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
index c2906e06fd..efbbe247c1 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
@@ -4021,7 +4021,6 @@ XhcEvaluateContext64 (
   // 4.6.7 Evaluate Context
   //
   InputContext = Xhc->UsbDevContext[SlotId].InputContext;
-  ZeroMem (InputContext, sizeof (INPUT_CONTEXT_64));

   InputContext->InputControlContext.Dword2 |= BIT1;
   InputContext->EP[0].MaxPacketSize         = MaxPacketSize;
--
2.20.1.windows.1



[-- Attachment #2: Type: text/html, Size: 6956 bytes --]

Re: [edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/XhciDxe: Fix the Bug of clean InputContext in XhcEvaluateContext64();

[Jiading Zhang]

[-- Attachment #1: Type: text/plain, Size: 789 bytes --]

When I use the newest XhciDxe driver in Phytium Platform( ARM) ,  I found if clearing “Input Context” in XhcEvaluateContext64(), the usb KB doesn't work, and mark the  code   ZeroMem (InputContext, sizeof (INPUT_CONTEXT_64));  it works.
I viewed the code again, maybe change the ZeroMem (InputContext, sizeof (INPUT_CONTEXT_64)); to ZeroMem (InputContext, sizeof (INPUT_CONTRL_CONTEXT_64)); is more resonable.  Because when clearing the INPUT_CONTEXT_64, the Slot context and the endpoint context are cleared too. But according the xHCI Spec section 6.2.2
Note：Unless otherwise stated: As Input, all fields of the Slot Context shall be initialized to the appropriate value by software before issuing a command.
And I test again, just clear INPUT_CONTRL_CONTEXT_64 is OK.

[-- Attachment #2: Type: text/html, Size: 859 bytes --]

Re: [edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/XhciDxe: Fix the Bug of clean InputContext in XhcEvaluateContext64();

[Wu, Hao A]

[-- Attachment #1: Type: text/plain, Size: 2575 bytes --]

Hello,

My take to the purpose of calling XhcEvaluateContext/XhcEvaluateContext64 functions is to “inform the xHC of the change to the Default Control endpoint’s Max Packet Size parameter.”
That is why the function only set bit 1 (Add Context bit 1) to inform the xHC only Endpoint Context for EP0 should be evaluated by xHC:
  InputContext->InputControlContext.Dword2 |= BIT1

I checked the xHCI spec, it does not prohibit only setting Add Context bit 1. According to section 4.6.7 Evaluate Context:
System software shall initialize Contexts of the
Input Context affected by the command. All Contexts not referenced by an Add Context flag in the Input
Context are ignored by the xHC.

If this is the case, the Slot Context should be ignored by the xHC, since Add Context bit 0 is cleared.

Then, according to section 6.2.3.3 Evaluate Context Command Usage:
A 'valid' Input Endpoint Context for an Evaluate Context Command requires that if the Add Context flag
(A1) for Default Control Endpoint is set to ‘1’, the Max Packet Size field shall be evaluated.

I do not think below code snippet violates with the above content in the spec:
  ZeroMem (InputContext, sizeof (INPUT_CONTEXT_64));

  InputContext->InputControlContext.Dword2 |= BIT1;
  InputContext->EP[0].MaxPacketSize         = MaxPacketSize;

Do you have a chance to test other USB devices on your environment? Is it possible that the issue is a device specific case?
Thanks in advance.

Best Regards,
Hao Wu

From: Jiading Zhang <jdzhang@kunluntech.com.cn>
Sent: Wednesday, September 14, 2022 5:23 PM
To: Wu; Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/XhciDxe: Fix the Bug of clean InputContext in XhcEvaluateContext64();

When I use the newest XhciDxe driver in Phytium Platform( ARM) ,  I found if clearing “Input Context” in XhcEvaluateContext64(), the usb KB doesn't work, and mark the  code   ZeroMem (InputContext, sizeof (INPUT_CONTEXT_64));  it works.
I viewed the code again, maybe change the ZeroMem (InputContext, sizeof (INPUT_CONTEXT_64)); to ZeroMem (InputContext, sizeof (INPUT_CONTRL_CONTEXT_64)); is more resonable.  Because when clearing the INPUT_CONTEXT_64, the Slot context and the endpoint context are cleared too. But according the xHCI Spec section 6.2.2
Note：Unless otherwise stated: As Input, all fields of the Slot Context shall be initialized to the appropriate value by software before issuing a command.
And I test again, just clear INPUT_CONTRL_CONTEXT_64 is OK.

[-- Attachment #2: Type: text/html, Size: 6139 bytes --]

Re: [edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/XhciDxe: Fix the Bug of clean InputContext in XhcEvaluateContext64();

[Jiading Zhang]

[-- Attachment #1: Type: text/plain, Size: 246 bytes --]

Hello,
I test for several times and find that if add the following line, the KB and MassStorage will work,
InputContext->EP[0].EPType = ED_CONTROL_BIDIR;

I don't know if the issue is brought in by the different of xHCI controllers or else.

[-- Attachment #2: Type: text/html, Size: 330 bytes --]

Re: [edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/XhciDxe: Fix the Bug of clean InputContext in XhcEvaluateContext64();

[Wu, Hao A]

[-- Attachment #1: Type: text/plain, Size: 1830 bytes --]

Thanks for the information.

Yes, I think the “EPType” field should be initialized properly for the Evaluate Context command to work.
The spec mentions in 6.2.3 Endpoint Context that:
* Note: Unless otherwise stated: As Input, all fields of the Endpoint Context shall be initialized to the appropriate value by software before issuing a command.
* Endpoint Type (EP Type). This field identifies whether an Endpoint Context is Valid

Could you help to update XhcEvaluateContext64 to (please help to update XhcEvaluateContext accordingly as well) and check below changes work on your environment?
  INPUT_CONTEXT_64            *InputContext;
  DEVICE_CONTEXT_64           *OutputContext;
  EFI_PHYSICAL_ADDRESS        PhyAddr;

  ASSERT (Xhc->UsbDevContext[SlotId].SlotId != 0);

  //
  // 4.6.7 Evaluate Context
  //
  InputContext  = Xhc->UsbDevContext[SlotId].InputContext;
  OutputContext = Xhc->UsbDevContext[SlotId].OutputContext;
  ZeroMem (InputContext, sizeof (INPUT_CONTEXT_64));
  CopyMem (&InputContext->EP[0], &OutputContext->EP[0], sizeof (ENDPOINT_CONTEXT_64));

My take is that doing so will initialize the input context of EP0 for the Evaluate Context command with previous output context of the Address Device command.

Best Regards,
Hao Wu

From: Jiading Zhang <jdzhang@kunluntech.com.cn>
Sent: Tuesday, September 20, 2022 8:46 AM
To: Wu; Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/XhciDxe: Fix the Bug of clean InputContext in XhcEvaluateContext64();

Hello,
   I test for several times and find that if add the following line, the KB and MassStorage will work,
       InputContext->EP[0].EPType = ED_CONTROL_BIDIR;

   I don't know if the issue is brought in by the different of xHCI controllers or else.

[-- Attachment #2: Type: text/html, Size: 7836 bytes --]

Re: [edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/XhciDxe: Fix the Bug of clean InputContext in XhcEvaluateContext64();

[Wu, Hao A]

[-- Attachment #1: Type: text/plain, Size: 2992 bytes --]

Sorry, missed one change, “EPState” field should be cleared to 0 for input context:

  INPUT_CONTEXT_64            *InputContext;
  DEVICE_CONTEXT_64           *OutputContext;
  EFI_PHYSICAL_ADDRESS        PhyAddr;

  ASSERT (Xhc->UsbDevContext[SlotId].SlotId != 0);

  //
  // 4.6.7 Evaluate Context
  //
  InputContext  = Xhc->UsbDevContext[SlotId].InputContext;
  OutputContext = Xhc->UsbDevContext[SlotId].OutputContext;
  ZeroMem (InputContext, sizeof (INPUT_CONTEXT_64));
  CopyMem (&InputContext->EP[0], &OutputContext->EP[0], sizeof (ENDPOINT_CONTEXT_64));

  InputContext->InputControlContext.Dword2 |= BIT1;
  InputContext->EP[0].MaxPacketSize         = MaxPacketSize;
  InputContext->EP[0].EPState               = 0;


From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, Hao A
Sent: Tuesday, September 20, 2022 11:04 AM
To: Jiading Zhang <jdzhang@kunluntech.com.cn>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/XhciDxe: Fix the Bug of clean InputContext in XhcEvaluateContext64();

Thanks for the information.

Yes, I think the “EPType” field should be initialized properly for the Evaluate Context command to work.
The spec mentions in 6.2.3 Endpoint Context that:
* Note: Unless otherwise stated: As Input, all fields of the Endpoint Context shall be initialized to the appropriate value by software before issuing a command.
* Endpoint Type (EP Type). This field identifies whether an Endpoint Context is Valid

Could you help to update XhcEvaluateContext64 to (please help to update XhcEvaluateContext accordingly as well) and check below changes work on your environment?
  INPUT_CONTEXT_64            *InputContext;
  DEVICE_CONTEXT_64           *OutputContext;
  EFI_PHYSICAL_ADDRESS        PhyAddr;

  ASSERT (Xhc->UsbDevContext[SlotId].SlotId != 0);

  //
  // 4.6.7 Evaluate Context
  //
  InputContext  = Xhc->UsbDevContext[SlotId].InputContext;
  OutputContext = Xhc->UsbDevContext[SlotId].OutputContext;
  ZeroMem (InputContext, sizeof (INPUT_CONTEXT_64));
  CopyMem (&InputContext->EP[0], &OutputContext->EP[0], sizeof (ENDPOINT_CONTEXT_64));

My take is that doing so will initialize the input context of EP0 for the Evaluate Context command with previous output context of the Address Device command.

Best Regards,
Hao Wu

From: Jiading Zhang <jdzhang@kunluntech.com.cn<mailto:jdzhang@kunluntech.com.cn>>
Sent: Tuesday, September 20, 2022 8:46 AM
To: Wu; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/XhciDxe: Fix the Bug of clean InputContext in XhcEvaluateContext64();

Hello,
   I test for several times and find that if add the following line, the KB and MassStorage will work,
       InputContext->EP[0].EPType = ED_CONTROL_BIDIR;

   I don't know if the issue is brought in by the different of xHCI controllers or else.


[-- Attachment #2: Type: text/html, Size: 9816 bytes --]

Re: [edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/XhciDxe: Fix the Bug of clean InputContext in XhcEvaluateContext64();

[Jiading Zhang]

[-- Attachment #1: Type: text/plain, Size: 122 bytes --]

I update XhcEvaluateContext64 according your suggesting and test in my platform, the KB and mass storage both can work!

[-- Attachment #2: Type: text/html, Size: 126 bytes --]

Re: [edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/XhciDxe: Fix the Bug of clean InputContext in XhcEvaluateContext64();

[Wu, Hao A]

[-- Attachment #1: Type: text/plain, Size: 712 bytes --]

Thanks.

Could you help to:
1) File a Bugzilla to track this issue at https://bugzilla.tianocore.org/enter_bug.cgi?product=EDK2;
2) Send a new version of the patch for code changes (please update XhcEvaluateContext function accordingly as well).

Best Regards,
Hao Wu

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jiading Zhang
Sent: Tuesday, September 20, 2022 2:00 PM
To: Wu; Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/XhciDxe: Fix the Bug of clean InputContext in XhcEvaluateContext64();

I update XhcEvaluateContext64 according your suggesting and test in my platform, the KB and mass storage both can work!


[-- Attachment #2: Type: text/html, Size: 3389 bytes --]

Re: [edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/XhciDxe: Fix the Bug of clean InputContext in XhcEvaluateContext64();

[Jiading Zhang]

[-- Attachment #1: Type: text/plain, Size: 1548 bytes --]

Hi Miki:
    I want to file a Bugzilla to track this issue , but I haven't a Bugzilla account yet,  could you please help to process?

Thanks！
--------------------------------------------------






------------------------------------------------ 
张家定  固件产品部  产品三部
昆仑太科（北京）技术股份有限公司 
手机：15810479495 
电话：010-89056290-8050 
邮箱：jdzhang@kunluntech.com.cn
地址：北京市海淀区北四环中路211号太极大厦10层 
邮编：100083



发件人："Wu, Hao A" <hao.a.wu@intel.com>
发送日期：2022-09-20 14:27:21
收件人："devel@edk2.groups.io" <devel@edk2.groups.io>,"jdzhang@kunluntech.com.cn" <jdzhang@kunluntech.com.cn>
主题：Re: [edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/XhciDxe: Fix the Bug of clean InputContext in XhcEvaluateContext64();
Thanks.
 
Could you help to:
1) File a Bugzilla to track this issue at https://bugzilla.tianocore.org/enter_bug.cgi?product=EDK2;
2) Send a new version of the patch for code changes (please update XhcEvaluateContext function accordingly as well).
 
Best Regards,
Hao Wu
 
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jiading Zhang
 Sent: Tuesday, September 20, 2022 2:00 PM
 To: Wu; Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io
 Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/XhciDxe: Fix the Bug of clean InputContext in XhcEvaluateContext64();


 
I update XhcEvaluateContext64 according your suggesting and test in my platform, the KB and mass storage both can work!










[-- Attachment #2: Type: text/html, Size: 4589 bytes --]

Re: [edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/XhciDxe: Fix the Bug of clean InputContext in XhcEvaluateContext64();

[Wu, Hao A]

[-- Attachment #1.1.1: Type: text/plain, Size: 2365 bytes --]

Hello,

I created the a Bugzilla tracker for this issue at: https://bugzilla.tianocore.org/show_bug.cgi?id=4074.
I have attached the tracker mail and please help to check if the issue report looks good to you. If needed I can help to update the tracker before you got the Bugzilla access.

Also, please help to add the below information in the commit log message:
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4074

Best Regards,
Hao Wu

From: 张家定 <jdzhang@kunluntech.com.cn>
Sent: Tuesday, September 20, 2022 6:01 PM
To: Wu, Hao A <hao.a.wu@intel.com>
Cc: devel@edk2.groups.io; Demeter, Miki <miki.demeter@intel.com>
Subject: Re:Re: [edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/XhciDxe: Fix the Bug of clean InputContext in XhcEvaluateContext64();

Hi Miki:
    I want to file a Bugzilla to track this issue , but I haven't a Bugzilla account yet,  could you please help to process?

Thanks！

--------------------------------------------------

[Image removed by sender.]


------------------------------------------------
张家定  固件产品部  产品三部
昆仑太科（北京）技术股份有限公司
手机：15810479495
电话：010-89056290-8050
邮箱：jdzhang@kunluntech.com.cn
地址：北京市海淀区北四环中路211号太极大厦10层
邮编：100083

发件人："Wu, Hao A" <hao.a.wu@intel.com>
发送日期：2022-09-20 14:27:21
收件人："devel@edk2.groups.io" <devel@edk2.groups.io>,"jdzhang@kunluntech.com.cn" <jdzhang@kunluntech.com.cn>
主题：Re: [edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/XhciDxe: Fix the Bug of clean InputContext in XhcEvaluateContext64();

Thanks.

Could you help to:
1) File a Bugzilla to track this issue at https://bugzilla.tianocore.org/enter_bug.cgi?product=EDK2;
2) Send a new version of the patch for code changes (please update XhcEvaluateContext function accordingly as well).

Best Regards,
Hao Wu

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jiading Zhang
Sent: Tuesday, September 20, 2022 2:00 PM
To: Wu; Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/Bus/Pci/XhciDxe: Fix the Bug of clean InputContext in XhcEvaluateContext64();

I update XhcEvaluateContext64 according your suggesting and test in my platform, the KB and mass storage both can work!



[-- Attachment #1.1.2: Type: text/html, Size: 12972 bytes --]

[-- Attachment #1.2: ~WRD0000.jpg --]
[-- Type: image/jpeg, Size: 823 bytes --]

[-- Attachment #2: Type: message/rfc822, Size: 14810 bytes --]

From: "bugzilla-daemon@bugzilla.tianocore.org" <bugzilla-daemon@bugzilla.tianocore.org>
To: Edk2+Bugs via Email Integration <bugs@edk2.groups.io>
Subject: [edk2-bugs] [Bug 4074] New: [MdeModulePkg][XhciDxe] Current endpoint input context is not fully initialized for evaluate context command
Date: Tue, 20 Sep 2022 21:28:20 +0000
Message-ID: <bug-4074-42@https.bugzilla.tianocore.org/>

https://bugzilla.tianocore.org/show_bug.cgi?id=4074

            Bug ID: 4074
           Summary: [MdeModulePkg][XhciDxe] Current endpoint input context
                    is not fully initialized for evaluate context command
           Product: EDK2
           Version: Current
          Hardware: All
                OS: All
            Status: UNCONFIRMED
          Severity: normal
          Priority: Lowest
         Component: Code
          Assignee: unassigned@tianocore.org
          Reporter: hao.a.wu@intel.com
                CC: edk2+bugs+int+994+563148131503455288@groups.io

From community report:
https://edk2.groups.io/g/devel/message/93737
https://edk2.groups.io/g/devel/message/93741
https://edk2.groups.io/g/devel/message/93976

The endpoint input context of Default Control Endpoint (EP[0]) for Evaluate
Context
command is not fully initialized according to the requirements in the xHCI
specification:

6.2.3 Endpoint Context:
* Note: Unless otherwise stated: As Input, all fields of the Endpoint Context
shall be initialized to the appropriate value by software before issuing a
command.
* Endpoint Type (EP Type). This field identifies whether an Endpoint Context is
Valid

According to the above community report
(https://edk2.groups.io/g/devel/message/93741), this will prevent USB devices
from being initialized on Phytium Platform (ARM).

Need to update XhcEvaluateContext/XhcEvaluateContext64 to properly initialize
the input context for Evaluate Context command.

--
You are receiving this mail because:
You are on the CC list for the bug.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#18913): https://edk2.groups.io/g/bugs/message/18913
Mute This Topic: https://groups.io/mt/93818337/1768737
Group Owner: bugs+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/bugs/unsub [hao.a.wu@intel.com]
-=-=-=-=-=-=-=-=-=-=-=-