Re: [edk2-devel] [PATCH] UefiPayloadPkg: Define default values for the DynamicEX PCDs
Hi. I commit a PR again, and the PR passed all the test. https://github.com/tianocore/edk2/pull/3792/commits Thanks! -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#97556): https://edk2.groups.io/g/devel/message/97556 Mute This Topic: https://groups.io/mt/94949814/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiPayloadPkg: Define default values for the DynamicEX PCDs
OK. I Commit a PR for the change: UefiPayloadPkg: Define default values for the DynamicEX PCDs #3785 Thanks! -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#97526): https://edk2.groups.io/g/devel/message/97526 Mute This Topic: https://groups.io/mt/94949814/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] UefiPayloadPkg: Define default values for the DynamicEX PCDs
The following PCDs have no value in UefiPayloadPkg.dsc，and they can not pass the Ecc tool check, so assign the default values the same as they are in *.dec file. gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport gEfiMdeModulePkgTokenSpaceGuid.PcdMrIovSupport gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSupport gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSystemPageSize gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize Signed-off-by: jdzhang --- UefiPayloadPkg/UefiPayloadPkg.dsc | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc index 1150be6acd..ce7d3a7f89 100644 --- a/UefiPayloadPkg/UefiPayloadPkg.dsc +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc @@ -530,14 +530,14 @@ gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity|$(UART_DEFAULT_PARITY) gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits|$(UART_DEFAULT_STOP_BITS) gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|$(DEFAULT_TERMINAL_TYPE) - gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport - gEfiMdeModulePkgTokenSpaceGuid.PcdMrIovSupport - gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSupport - gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSystemPageSize - gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds - gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode - gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress - gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize + gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport|TRUE + gEfiMdeModulePkgTokenSpaceGuid.PcdMrIovSupport|FALSE + gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSupport|TRUE + gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSystemPageSize|0x1 + gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|5 + gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode|1 + gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress|0x0 + gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize|0x0 !if ($(TARGET) == DEBUG || $(USE_CBMEM_FOR_CONSOLE) == TRUE) gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|TRUE !else -- 2.38.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#96217): https://edk2.groups.io/g/devel/message/96217 Mute This Topic: https://groups.io/mt/94949814/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiPayloadPkg: remove the redundant PCD definition.
OK, I got it, thanks! By the way, I don't know whether it is appropriate that the PCDs have no value in UefiPayloadPkg.dsc. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#96178): https://edk2.groups.io/g/devel/message/96178 Mute This Topic: https://groups.io/mt/94928007/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiPayloadPkg: remove the redundant PCD definition.
Yes, these pcds is used by PciBus driver, my comment is not correct. My intention is these PCDs have no value in the UefiPaylodPkd.dsc, we can use the default values in MdeModulePkg.dec or override the values in UefiPaylodPkd.dsc. On Thu, Nov 10, 2022 at 09:58 AM, Ni, Ray wrote: > > > > These PCDs are used by PciBus driver and MP init logic which are included > in the UefiPayloadPkg. > > > > > > > > *From:* devel@edk2.groups.io *On Behalf Of* Jiading > Zhang > *Sent:* Thursday, November 10, 2022 9:53 AM > *To:* devel@edk2.groups.io > *Subject:* [edk2-devel] [PATCH] UefiPayloadPkg: remove the redundant PCD > definition. > > > > > > > > > > > > > > The following PCDs have no value in the file of UefiPayloadPkg.dsc, and > they are not used in UefiPayloadPkg, so remove them from > [PcdsDynamicExDefault] section. > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdMrIovSupport > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSupport > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSystemPageSize > > > > > gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds > > > > > gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode > > > > > gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress > > > > > gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize > > > > > > > > > > Signed-off-by: jdzhang < jdzh...@kunluntech.com.cn > > > > > > --- > > > > > UefiPayloadPkg/UefiPayloadPkg.dsc | 8 > > > > > 1 file changed, 8 deletions(-) > > > > > > > > > > diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc > b/UefiPayloadPkg/UefiPayloadPkg.dsc > > > > > index 1150be6acd..43768909b3 100644 > > > > > --- a/UefiPayloadPkg/UefiPayloadPkg.dsc > > > > > +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc > > > > > @@ -530,14 +530,6 @@ > > > > > gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity|$(UART_DEFAULT_PARITY) > > > > > gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits|$(UART_DEFAULT_STOP_BITS) > > > > > gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|$(DEFAULT_TERMINAL_TYPE) > > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport > > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdMrIovSupport > > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSupport > > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSystemPageSize > > > > > - gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds > > > > > - gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode > > > > > - gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress > > > > > - gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize > > > > > !if ($(TARGET) == DEBUG || $(USE_CBMEM_FOR_CONSOLE) == TRUE) > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|TRUE > > > > > !else > > > > > -- > > > > > 2.38.1.windows.1 > > > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#96176): https://edk2.groups.io/g/devel/message/96176 Mute This Topic: https://groups.io/mt/94928007/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] UefiPayloadPkg: remove the redundant PCD definition.
The following PCDs have no value in the file of UefiPayloadPkg.dsc, and they are not used in UefiPayloadPkg, so remove them from [PcdsDynamicExDefault] section. gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport gEfiMdeModulePkgTokenSpaceGuid.PcdMrIovSupport gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSupport gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSystemPageSize gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize Signed-off-by: jdzhang --- UefiPayloadPkg/UefiPayloadPkg.dsc | 8 1 file changed, 8 deletions(-) diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc index 1150be6acd..43768909b3 100644 --- a/UefiPayloadPkg/UefiPayloadPkg.dsc +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc @@ -530,14 +530,6 @@ gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity|$(UART_DEFAULT_PARITY) gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits|$(UART_DEFAULT_STOP_BITS) gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|$(DEFAULT_TERMINAL_TYPE) - gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport - gEfiMdeModulePkgTokenSpaceGuid.PcdMrIovSupport - gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSupport - gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSystemPageSize - gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds - gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode - gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress - gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize !if ($(TARGET) == DEBUG || $(USE_CBMEM_FOR_CONSOLE) == TRUE) gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|TRUE !else -- 2.38.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#96173): https://edk2.groups.io/g/devel/message/96173 Mute This Topic: https://groups.io/mt/94928007/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Add a parameter to indicate whether the memory allocation is for TRB Ring or not.
Hi,
I renamed the parameter from ‘AllocationForTrbRing’ to ‘AllocationForRing’ in
UsbHcAllocMemFromBlock, which matchs the description comment. And the
attachment is the updated patch file.
According the Xhci Spec, TRB Rings may be larger than a Page, however they
shall not cross a 64K byte boundary, so add a parameter to indicate whether the
memory allocation is for TRB Rings or not. It will ensure the allocation not
crossing 64K boundary in UsbHcAllocMemFromBlock if the memory is allocated for
TRB Rings.
Signed-off-by: jdzhang
---
MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c | 37 +++-
MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.h | 10 +--
MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 16 +-
3 files changed, 44 insertions(+), 19 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c
b/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c
index 99fb3521d5..4373b2c99e 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c
@@ -132,8 +132,9 @@ UsbHcFreeMemBlock (
/**
Alloc some memory from the block.
- @param Block The memory block to allocate memory from.
- @param Units Number of memory units to allocate.
+ @param Block The memory block to allocate memory from.
+ @param Units Number of memory units to allocate.
+ @param AllocationForRing The allocated memory is for Ring or not.
@return The pointer to the allocated memory. If couldn't allocate the needed
memory,
the return value is NULL.
@@ -142,7 +143,8 @@ UsbHcFreeMemBlock (
VOID *
UsbHcAllocMemFromBlock (
IN USBHC_MEM_BLOCK *Block,
- IN UINTN Units
+ IN UINTN Units,
+ IN BOOLEAN AllocationForRing
)
{
UINTN Byte;
@@ -151,12 +153,15 @@ UsbHcAllocMemFromBlock (
UINT8 StartBit;
UINTN Available;
UINTN Count;
+ UINTN MemUnitAddr;
+ UINTN AlignmentMask;
ASSERT ((Block != 0) && (Units != 0));
StartByte = 0;
StartBit = 0;
Available = 0;
+ AlignmentMask = ~((UINTN)USBHC_MEM_TRB_RINGS_BOUNDARY - 1);
for (Byte = 0, Bit = 0; Byte < Block->BitsLen;) {
//
@@ -165,6 +170,20 @@ UsbHcAllocMemFromBlock (
// Available counts the consective number of zero bit.
//
if (!USB_HC_BIT_IS_SET (Block->Bits[Byte], Bit)) {
+ if (AllocationForRing && (Available != 0)) {
+ MemUnitAddr = (UINTN)Block->BufHost + (Byte * 8 + Bit) *
USBHC_MEM_UNIT;
+ if ((MemUnitAddr & AlignmentMask) != ((MemUnitAddr - USBHC_MEM_UNIT) &
AlignmentMask)) {
+ //
+ // If the TRB Ring memory cross 64K-byte boundary, then restart the
+ // search starting at current memory unit.
+ // Doing so is to meet the TRB Ring boundary requirement in XHCI
spec.
+ //
+ Available = 0;
+ StartByte = Byte;
+ StartBit = Bit;
+ }
+ }
+
Available++;
if (Available >= Units) {
@@ -438,8 +457,9 @@ UsbHcFreeMemPool (
Allocate some memory from the host controller's memory pool
which can be used to communicate with host controller.
- @param Pool The host controller's memory pool.
- @param Size Size of the memory to allocate.
+ @param Pool The host controller's memory pool.
+ @param Size Size of the memory to allocate.
+ @param AllocationForRing The allocated memory is for Ring or not.
@return The allocated memory or NULL.
@@ -447,7 +467,8 @@ UsbHcFreeMemPool (
VOID *
UsbHcAllocateMem (
IN USBHC_MEM_POOL *Pool,
- IN UINTN Size
+ IN UINTN Size,
+ IN BOOLEAN AllocationForRing
)
{
USBHC_MEM_BLOCK *Head;
@@ -466,7 +487,7 @@ UsbHcAllocateMem (
// First check whether current memory blocks can satisfy the allocation.
//
for (Block = Head; Block != NULL; Block = Block->Next) {
- Mem = UsbHcAllocMemFromBlock (Block, AllocSize / USBHC_MEM_UNIT);
+ Mem = UsbHcAllocMemFromBlock (Block, AllocSize / USBHC_MEM_UNIT,
AllocationForRing);
if (Mem != NULL) {
ZeroMem (Mem, Size);
@@ -501,7 +522,7 @@ UsbHcAllocateMem (
// Add the new memory block to the pool, then allocate memory from it
//
UsbHcInsertMemBlockToPool (Head, NewBlock);
- Mem = UsbHcAllocMemFromBlock (NewBlock, AllocSize / USBHC_MEM_UNIT);
+ Mem = UsbHcAllocMemFromBlock (NewBlock, AllocSize / USBHC_MEM_UNIT,
AllocationForRing);
if (Mem != NULL) {
ZeroMem (Mem, Size);
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.h
b/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.h
index 48ae86141c..dfea0e52b3 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.h
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.h
@@ -48,6 +48,8 @@ typedef struct _USBHC_MEM_POOL {
#define USBHC_MEM_ROUND(Len) (((Len) + USBHC_MEM_UNIT_MASK) &
(~USBHC_MEM_UNIT_MASK))
+#define USBHC_MEM_TRB_RINGS_BOUNDARY SIZE_64KB
+
//
// Advance the byte and bit to the next bit, adjust byte accordingly.
//
@@ -92,8 +94,9 @@ UsbHcFreeMemPool (
Allocate some memory from the host controller's memory pool
which can be used to communicate with host
[edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Add a parameter to indicate whether the memory allocation is for TRB Ring or not.
According the Xhci Spec, TRB Rings may be larger than a Page, however they
shall not cross a 64K byte boundary, so add a parameter to indicate whether the
memory allocation is for TRB or not. It will ensure the allocation not crossing
64K boundary in UsbHcAllocMemFromBlock if the memory is allocated for TRB.
Signed-off-by: jdzhang
---
MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c | 37 +++-
MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.h | 10 +--
MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 16 +-
3 files changed, 44 insertions(+), 19 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c
b/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c
index 99fb3521d5..24714502cd 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c
@@ -132,8 +132,9 @@ UsbHcFreeMemBlock (
/**
Alloc some memory from the block.
- @param Block The memory block to allocate memory from.
- @param Units Number of memory units to allocate.
+ @param Block The memory block to allocate memory from.
+ @param Units Number of memory units to allocate.
+ @param AllocationForRing The allocated memory is for Ring or not.
@return The pointer to the allocated memory. If couldn't allocate the needed
memory,
the return value is NULL.
@@ -142,7 +143,8 @@ UsbHcFreeMemBlock (
VOID *
UsbHcAllocMemFromBlock (
IN USBHC_MEM_BLOCK *Block,
- IN UINTN Units
+ IN UINTN Units,
+ IN BOOLEAN AllocationForTrbRing
)
{
UINTN Byte;
@@ -151,12 +153,15 @@ UsbHcAllocMemFromBlock (
UINT8 StartBit;
UINTN Available;
UINTN Count;
+ UINTN MemUnitAddr;
+ UINTN AlignmentMask;
ASSERT ((Block != 0) && (Units != 0));
StartByte = 0;
StartBit = 0;
Available = 0;
+ AlignmentMask = ~((UINTN)USBHC_MEM_TRB_RINGS_BOUNDARY - 1);
for (Byte = 0, Bit = 0; Byte < Block->BitsLen;) {
//
@@ -165,6 +170,20 @@ UsbHcAllocMemFromBlock (
// Available counts the consective number of zero bit.
//
if (!USB_HC_BIT_IS_SET (Block->Bits[Byte], Bit)) {
+ if (AllocationForTrbRing && (Available != 0)) {
+ MemUnitAddr = (UINTN)Block->BufHost + (Byte * 8 + Bit) *
USBHC_MEM_UNIT;
+ if ((MemUnitAddr & AlignmentMask) != ((MemUnitAddr - USBHC_MEM_UNIT) &
AlignmentMask)) {
+ //
+ // If the TRB Ring memory cross 64K-byte boundary, then restart the
+ // search starting at current memory unit.
+ // Doing so is to meet the TRB Ring boundary requirement in XHCI
spec.
+ //
+ Available = 0;
+ StartByte = Byte;
+ StartBit = Bit;
+ }
+ }
+
Available++;
if (Available >= Units) {
@@ -438,8 +457,9 @@ UsbHcFreeMemPool (
Allocate some memory from the host controller's memory pool
which can be used to communicate with host controller.
- @param Pool The host controller's memory pool.
- @param Size Size of the memory to allocate.
+ @param Pool The host controller's memory pool.
+ @param Size Size of the memory to allocate.
+ @param AllocationForRing The allocated memory is for Ring or not.
@return The allocated memory or NULL.
@@ -447,7 +467,8 @@ UsbHcFreeMemPool (
VOID *
UsbHcAllocateMem (
IN USBHC_MEM_POOL *Pool,
- IN UINTN Size
+ IN UINTN Size,
+ IN BOOLEAN AllocationForRing
)
{
USBHC_MEM_BLOCK *Head;
@@ -466,7 +487,7 @@ UsbHcAllocateMem (
// First check whether current memory blocks can satisfy the allocation.
//
for (Block = Head; Block != NULL; Block = Block->Next) {
- Mem = UsbHcAllocMemFromBlock (Block, AllocSize / USBHC_MEM_UNIT);
+ Mem = UsbHcAllocMemFromBlock (Block, AllocSize / USBHC_MEM_UNIT,
AllocationForRing);
if (Mem != NULL) {
ZeroMem (Mem, Size);
@@ -501,7 +522,7 @@ UsbHcAllocateMem (
// Add the new memory block to the pool, then allocate memory from it
//
UsbHcInsertMemBlockToPool (Head, NewBlock);
- Mem = UsbHcAllocMemFromBlock (NewBlock, AllocSize / USBHC_MEM_UNIT);
+ Mem = UsbHcAllocMemFromBlock (NewBlock, AllocSize / USBHC_MEM_UNIT,
AllocationForRing);
if (Mem != NULL) {
ZeroMem (Mem, Size);
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.h
b/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.h
index 48ae86141c..dfea0e52b3 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.h
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.h
@@ -48,6 +48,8 @@ typedef struct _USBHC_MEM_POOL {
#define USBHC_MEM_ROUND(Len) (((Len) + USBHC_MEM_UNIT_MASK) &
(~USBHC_MEM_UNIT_MASK))
+#define USBHC_MEM_TRB_RINGS_BOUNDARY SIZE_64KB
+
//
// Advance the byte and bit to the next bit, adjust byte accordingly.
//
@@ -92,8 +94,9 @@ UsbHcFreeMemPool (
Allocate some memory from the host controller's memory pool
which can be used to communicate with host controller.
- @param Pool The host controller's memory pool.
- @param Size Size of the memory to allocate.
+ @param Pool The host controller's memory pool.
+ @param Size
Re: [edk2-devel] 回复: [edk2-devel] 回复: [edk2-devel] 回复: [edk2-devel] [PATCH] MdeModulePkg VariablePei: Add Variable state check when find variable in IndexTable.
Hi Liming:
My case is special, the HOB is created in a special memory, neither cache nor
normal memory, and it is non-volatile for a warm reboot, so the change works
in my code. Maybe I should try to create the hob in a volatile cache or
memory, so I needn't change the open source code.
Thanks very much.
Jiading
On Fri, Oct 14, 2022 at 09:57 AM, gaoliming wrote:
>
>
>
> Jiading:
>
>
>
> HOB is created in volatile memory (cache or physical memory). And, HOB is
> re-created for every boot (normal boot, S3 boot). When the first call
> GetVariable, gEfiVariableIndexTableGuid guid hob should not exist. If this
> guid hob exits, it should be created by other module. Please check.
>
>
>
>
>
>
>
> Thanks
>
>
>
> Liming
>
>
>
> *发件人 :* Jiading Zhang
> *发送时间 :* 2022 年 10 月 10 日 16:36
> *收件人 :* gaoliming ; devel@edk2.groups.io
> *主题 :* Re: [edk2-devel] 回复 : [edk2-devel] 回复 : [edk2-devel] [PATCH]
> MdeModulePkg VariablePei: Add Variable state check when find variable in
> IndexTable.
>
>
>
>
>
>
>
>
> Hi liming:
> I checked the code, and found the root cause of the issue.
> As the following code in edk2, in my code, after the first time to
> read the variable in PEI phase, it cached the variable store info
> IndexTable into a Hob in a special non-volatile memory, but after the
> code running in dxe phase, the variable was changed. And after a warm
> reboot, maybe call s3 sleep is more exactly, when read the variable, it
> first get the IndexTable from the Hob but not build the IndexTable again,
> so it read the deleted variable if hasn't the condition check. This is
> really a special case.
>
>
>
> GuidHob = GetFirstGuidHob ();
>
>
>
>
> if (GuidHob != NULL) {
>
>
>
>
> StoreInfo->IndexTable = GET_GUID_HOB_DATA (GuidHob);
>
>
>
>
> } else {
>
>
>
>
> //
>
>
>
>
> // If it's the first time to access variable region in flash, create a
> guid hob to record
>
>
>
>
> // VAR_ADDED type variable info.
>
>
>
>
> // Note that as the resource of PEI phase is limited, only store the
> limited number of
>
>
>
>
> // VAR_ADDED type variables to reduce access time.
>
>
>
>
> //
>
>
>
>
> StoreInfo->IndexTable = (VARIABLE_INDEX_TABLE *) BuildGuidHob
> (, sizeof (VARIABLE_INDEX_TABLE));
>
>
>
>
> StoreInfo->IndexTable->Length = 0;
>
>
>
>
> StoreInfo->IndexTable->StartPtr = GetStartPointer
> (VariableStoreHeader);
>
>
>
>
> StoreInfo->IndexTable->EndPtr = GetEndPointer
> (VariableStoreHeader);
>
>
>
>
> StoreInfo->IndexTable->GoneThrough = 0;
>
>
>
>
> }
>
>
>
>
>
>
>
>
> On Mon, Oct 10, 2022 at 09:39 AM, gaoliming wrote:
>
>
>>
>>
>> Jiading:
>>
>>
>>
>> Please check why NV variable data is required to be changed in PEI phase.
>> This will be helpful for this issue.
>>
>>
>>
>>
>>
>>
>>
>> Thanks
>>
>>
>>
>> Liming
>>
>>
>>
>> *发件人 :* Jiading Zhang < jdzh...@kunluntech.com.cn >
>> *发送时间 :* 2022 年 10 月 10 日 8:35
>> *收件人 :* gaoliming < gaolim...@byosoft.com.cn >; devel@edk2.groups.io
>> *主题 :* Re: [edk2-devel] 回复 : [edk2-devel] [PATCH] MdeModulePkg VariablePei:
>> Add Variable state check when find variable in IndexTable.
>>
>>
>>
>>
>>
>>
>>
>>
>> Hi liming:
>> Yes, NV Variable Data is not changed in PEI phase in normal case. This
>> issue was found when we did a special coding, and when found variable in
>> the IndexTable, it found the variable before the last changed if didn't
>> add the following condition:
>> "if ((VariableHeader->State == VAR_ADDED) || (VariableHeader->State ==
>> (VAR_IN_DELETED_TRANSITION & VAR_ADDED)))"
>>
>> Maybe our specail coding had some defect ， and caused this issue.
>>
>> On Fri, Sep 30, 2022 at 10:46 AM, gaoliming wrote:
>>
>>
>>>
>>>
>>> Jiading:
>>>
>>>
>>>
>>> Hob Variable Store Info IndexTable is NULL. So, this logic doesn ’ t work
>>> for HOB variable store. NV Variable Store Info has IndexTable. When its
>>> IndexTable is initialized, its IndexTable will only record the variable
>>> with VAR_ADDED attribute. Because
[edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Allocate the Ring segments at 64K aligned address to avoid the Rings crossing a 64K byte boundary.
According the Xhci Spec, TRB Rings may be larger than a Page, however they
shall not cross a 64K byte boundary, so allocate the rings at 64K aligned
address to avoid they crossing a 64K byte boundary.
Signed-off-by: jdzhang
---
MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 112 +++
MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h | 4 +
2 files changed, 56 insertions(+), 60 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
index 4ae0297607..b020bb064b 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
@@ -473,7 +473,6 @@ XhcInitSched (
{
VOID *Dcbaa;
EFI_PHYSICAL_ADDRESS DcbaaPhy;
- UINT64 CmdRing;
EFI_PHYSICAL_ADDRESS CmdRingPhy;
UINTN Entries;
UINT32 MaxScratchpadBufs;
@@ -602,8 +601,7 @@ XhcInitSched (
// Transfer Ring it checks for a Cycle bit transition. If a transition
detected, the ring is empty.
// So we set RCS as inverted PCS init value to let Command Ring empty
//
- CmdRing = (UINT64)(UINTN)Xhc->CmdRing.RingSeg0;
- CmdRingPhy = UsbHcGetPciAddrForHostAddr (Xhc->MemPool, (VOID
*)(UINTN)CmdRing, sizeof (TRB_TEMPLATE) * CMD_RING_TRB_NUMBER);
+ CmdRingPhy = (UINT64)(UINTN)Xhc->CmdRing.RingPhy;
ASSERT ((CmdRingPhy & 0x3F) == 0);
CmdRingPhy |= XHC_CRCR_RCS;
//
@@ -790,23 +788,28 @@ CreateEventRing (
EVENT_RING_SEG_TABLE_ENTRY *ERSTBase;
UINTN Size;
EFI_PHYSICAL_ADDRESS ERSTPhy;
- EFI_PHYSICAL_ADDRESS DequeuePhy;
+ EFI_STATUS Status;
ASSERT (EventRing != NULL);
-
+
+ //To meet the 64KB Boundary Requirement in xhci spec chapter 6 Table 6-1,
allocate the Ring segments at 64K aligned address.
Size = sizeof (TRB_TEMPLATE) * EVENT_RING_TRB_NUMBER;
- Buf = UsbHcAllocateMem (Xhc->MemPool, Size);
- ASSERT (Buf != NULL);
- ASSERT (((UINTN)Buf & 0x3F) == 0);
- ZeroMem (Buf, Size);
+ Status = UsbHcAllocateAlignedPages (
+ Xhc->PciIo,
+ EFI_SIZE_TO_PAGES (Size),
+ SIZE_64KB,
+ (VOID **) &(EventRing->EventRingSeg0),
+ &(EventRing->EventRingPhy),
+ &(EventRing->EventRingMap)
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ ZeroMem (EventRing->EventRingSeg0, Size);
- EventRing->EventRingSeg0 = Buf;
EventRing->TrbNumber = EVENT_RING_TRB_NUMBER;
EventRing->EventRingDequeue = (TRB_TEMPLATE *)EventRing->EventRingSeg0;
EventRing->EventRingEnqueue = (TRB_TEMPLATE *)EventRing->EventRingSeg0;
- DequeuePhy = UsbHcGetPciAddrForHostAddr (Xhc->MemPool, Buf, Size);
-
//
// Software maintains an Event Ring Consumer Cycle State (CCS) bit,
initializing it to '1'
// and toggling it every time the Event Ring Dequeue Pointer wraps back to the
beginning of the Event Ring.
@@ -821,8 +824,8 @@ CreateEventRing (
ERSTBase = (EVENT_RING_SEG_TABLE_ENTRY *)Buf;
EventRing->ERSTBase = ERSTBase;
- ERSTBase->PtrLo = XHC_LOW_32BIT (DequeuePhy);
- ERSTBase->PtrHi = XHC_HIGH_32BIT (DequeuePhy);
+ ERSTBase->PtrLo = XHC_LOW_32BIT (EventRing->EventRingPhy);
+ ERSTBase->PtrHi = XHC_HIGH_32BIT (EventRing->EventRingPhy);
ERSTBase->RingTrbSize = EVENT_RING_TRB_NUMBER;
ERSTPhy = UsbHcGetPciAddrForHostAddr (Xhc->MemPool, ERSTBase, Size);
@@ -844,12 +847,12 @@ CreateEventRing (
XhcWriteRuntimeReg (
Xhc,
XHC_ERDP_OFFSET,
- XHC_LOW_32BIT ((UINT64)(UINTN)DequeuePhy)
+ XHC_LOW_32BIT ((UINT64)(UINTN)EventRing->EventRingPhy)
);
XhcWriteRuntimeReg (
Xhc,
XHC_ERDP_OFFSET + 4,
- XHC_HIGH_32BIT ((UINT64)(UINTN)DequeuePhy)
+ XHC_HIGH_32BIT ((UINT64)(UINTN)EventRing->EventRingPhy)
);
//
// Program the Interrupter Event Ring Segment Table Base Address (ERSTBA)
register(5.5.2.3.2)
@@ -888,16 +891,24 @@ CreateTransferRing (
OUT TRANSFER_RING *TransferRing
)
{
- VOID *Buf;
+ UINTN Size;
LINK_TRB *EndTrb;
- EFI_PHYSICAL_ADDRESS PhyAddr;
-
- Buf = UsbHcAllocateMem (Xhc->MemPool, sizeof (TRB_TEMPLATE) * TrbNum);
- ASSERT (Buf != NULL);
- ASSERT (((UINTN)Buf & 0x3F) == 0);
- ZeroMem (Buf, sizeof (TRB_TEMPLATE) * TrbNum);
+ EFI_STATUS Status;
+
+ //To meet the 64KB Boundary Requirement in xhci spec chapter 6 Table 6-1,
allocate the Ring segments at 64K aligned address.
+ Size = sizeof (TRB_TEMPLATE) * TrbNum;
+ Status = UsbHcAllocateAlignedPages (
+ Xhc->PciIo,
+ EFI_SIZE_TO_PAGES (Size),
+ SIZE_64KB,
+ (VOID **) &(TransferRing->RingSeg0),
+ &(TransferRing->RingPhy),
+ &(TransferRing->RingMap)
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ ZeroMem (TransferRing->RingSeg0, Size);
- TransferRing->RingSeg0 = Buf;
TransferRing->TrbNumber = TrbNum;
TransferRing->RingEnqueue = (TRB_TEMPLATE *)TransferRing->RingSeg0;
TransferRing->RingDequeue = (TRB_TEMPLATE *)TransferRing->RingSeg0;
@@ -907,11 +918,10 @@ CreateTransferRing (
// To form a ring (or
Re: [edk2-devel] 回复: [edk2-devel] 回复: [edk2-devel] [PATCH] MdeModulePkg VariablePei: Add Variable state check when find variable in IndexTable.
Hi liming:
I checked the code, and found the root cause of the issue.
As the following code in edk2, in my code, after the first time to read the
variable in PEI phase, it cached the variable store info IndexTable into a
Hob in a special non-volatile memory, but after the code running in dxe phase,
the variable was changed. And after a warm reboot, maybe call s3 sleep is more
exactly, when read the variable, it first get the IndexTable from the Hob but
not build the IndexTable again, so it read the deleted variable if hasn't the
condition check. This is really a special case.
GuidHob = GetFirstGuidHob ();
if (GuidHob != NULL) {
StoreInfo->IndexTable = GET_GUID_HOB_DATA (GuidHob);
} else {
//
// If it's the first time to access variable region in flash, create a guid hob
to record
// VAR_ADDED type variable info.
// Note that as the resource of PEI phase is limited, only store the limited
number of
// VAR_ADDED type variables to reduce access time.
//
StoreInfo->IndexTable = (VARIABLE_INDEX_TABLE *) BuildGuidHob
(, sizeof (VARIABLE_INDEX_TABLE));
StoreInfo->IndexTable->Length = 0;
StoreInfo->IndexTable->StartPtr = GetStartPointer (VariableStoreHeader);
StoreInfo->IndexTable->EndPtr = GetEndPointer (VariableStoreHeader);
StoreInfo->IndexTable->GoneThrough = 0;
}
On Mon, Oct 10, 2022 at 09:39 AM, gaoliming wrote:
>
>
>
> Jiading:
>
>
>
> Please check why NV variable data is required to be changed in PEI phase.
> This will be helpful for this issue.
>
>
>
>
>
>
>
> Thanks
>
>
>
> Liming
>
>
>
> *发件人 :* Jiading Zhang
> *发送时间 :* 2022 年 10 月 10 日 8:35
> *收件人 :* gaoliming ; devel@edk2.groups.io
> *主题 :* Re: [edk2-devel] 回复 : [edk2-devel] [PATCH] MdeModulePkg VariablePei:
> Add Variable state check when find variable in IndexTable.
>
>
>
>
>
>
>
>
> Hi liming:
> Yes, NV Variable Data is not changed in PEI phase in normal case. This
> issue was found when we did a special coding, and when found variable in
> the IndexTable, it found the variable before the last changed if didn't
> add the following condition:
> "if ((VariableHeader->State == VAR_ADDED) || (VariableHeader->State ==
> (VAR_IN_DELETED_TRANSITION & VAR_ADDED)))"
>
> Maybe our specail coding had some defect ， and caused this issue.
>
> On Fri, Sep 30, 2022 at 10:46 AM, gaoliming wrote:
>
>
>>
>>
>> Jiading:
>>
>>
>>
>> Hob Variable Store Info IndexTable is NULL. So, this logic doesn ’ t work
>> for HOB variable store. NV Variable Store Info has IndexTable. When its
>> IndexTable is initialized, its IndexTable will only record the variable
>> with VAR_ADDED attribute. Because NV Variable Data is not changed in PEI
>> phase, this check is not required by NV variable.
>>
>>
>>
>>
>>
>>
>>
>> Thanks
>>
>>
>>
>> Liming
>>
>>
>>
>> *发件人 :* devel@edk2.groups.io < devel@edk2.groups.io > *代表* Jiading Zhang
>> *发送时间 :* 2022 年 9 月 28 日 11:05
>> *收件人 :* devel@edk2.groups.io
>> *主题 :* [edk2-devel] [PATCH] MdeModulePkg VariablePei: Add Variable state
>> check when find variable in IndexTable.
>>
>>
>>
>>
>>
>>
>>
>>
>> When read a variable in PEI, it will find it first in the HOB, then find
>> in variable store. When find in variable store, it will check the variable
>> state, but find in HOB, it doesn't check the state, so if the variable was
>> changed, it will find the obsolete variable in the HOB.
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> Signed-off-by: jdzhang < jdzh...@kunluntech.com.cn >
>>
>>
>>
>>
>> ---
>>
>>
>>
>>
>> MdeModulePkg/Universal/Variable/Pei/Variable.c | 12 +++-
>>
>>
>>
>>
>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.c
>> b/MdeModulePkg/Universal/Variable/Pei/Variable.c
>>
>>
>>
>>
>> index 26a4c73b45..dffbd8cdb1 100644
>>
>>
>>
>>
>> --- a/MdeModulePkg/Universal/Variable/Pei/Variable.c
>>
>>
>>
>>
>> +++ b/MdeModulePkg/Universal/Variable/Pei/Variable.c
>>
>>
>>
>>
>> @@ -866,11 +866,13 @@ FindVariableEx (
>>
>>
>>
>>
>> Offset += IndexTable->Index[Index];
>>
>
Re: [edk2-devel] 回复: [edk2-devel] [PATCH] MdeModulePkg VariablePei: Add Variable state check when find variable in IndexTable.
Hi liming:
Yes, NV Variable Data is not changed in PEI phase in normal case. This issue
was found when we did a special coding, and when found variable in the
IndexTable, it found the variable before the last changed if didn't add the
following condition:
" if ((VariableHeader->State == VAR_ADDED) || (VariableHeader->State ==
(VAR_IN_DELETED_TRANSITION & VAR_ADDED)))"
Maybe our specail coding had some defect，and caused this issue.
On Fri, Sep 30, 2022 at 10:46 AM, gaoliming wrote:
>
>
>
> Jiading:
>
>
>
> Hob Variable Store Info IndexTable is NULL. So, this logic doesn’t work
> for HOB variable store. NV Variable Store Info has IndexTable. When its
> IndexTable is initialized, its IndexTable will only record the variable
> with VAR_ADDED attribute. Because NV Variable Data is not changed in PEI
> phase, this check is not required by NV variable.
>
>
>
>
>
>
>
> Thanks
>
>
>
> Liming
>
>
>
> *发件人 :* devel@edk2.groups.io *代表* Jiading Zhang
> *发送时间 :* 2022 年 9 月 28 日 11:05
> *收件人 :* devel@edk2.groups.io
> *主题 :* [edk2-devel] [PATCH] MdeModulePkg VariablePei: Add Variable state
> check when find variable in IndexTable.
>
>
>
>
>
>
>
>
> When read a variable in PEI, it will find it first in the HOB, then find
> in variable store. When find in variable store, it will check the variable
> state, but find in HOB, it doesn't check the state, so if the variable was
> changed, it will find the obsolete variable in the HOB.
>
>
>
>
>
>
>
>
>
> Signed-off-by: jdzhang < jdzh...@kunluntech.com.cn >
>
>
>
>
> ---
>
>
>
>
> MdeModulePkg/Universal/Variable/Pei/Variable.c | 12 +++-
>
>
>
>
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
>
>
>
>
>
>
>
>
> diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.c
> b/MdeModulePkg/Universal/Variable/Pei/Variable.c
>
>
>
>
> index 26a4c73b45..dffbd8cdb1 100644
>
>
>
>
> --- a/MdeModulePkg/Universal/Variable/Pei/Variable.c
>
>
>
>
> +++ b/MdeModulePkg/Universal/Variable/Pei/Variable.c
>
>
>
>
> @@ -866,11 +866,13 @@ FindVariableEx (
>
>
>
>
> Offset += IndexTable->Index[Index];
>
>
>
>
> MaxIndex = (VARIABLE_HEADER *)((UINT8 *)IndexTable->StartPtr + Offset);
>
>
>
>
> GetVariableHeader (StoreInfo, MaxIndex, );
>
>
>
>
> - if (CompareWithValidVariable (StoreInfo, MaxIndex, VariableHeader,
> VariableName, VendorGuid, PtrTrack) == EFI_SUCCESS) {
>
>
>
>
> - if (VariableHeader->State == (VAR_IN_DELETED_TRANSITION &
> VAR_ADDED)) {
>
>
>
>
> - InDeletedVariable = PtrTrack->CurrPtr;
>
>
>
>
> - } else {
>
>
>
>
> - return EFI_SUCCESS;
>
>
>
>
> + if ((VariableHeader->State == VAR_ADDED) || (VariableHeader->State
> == (VAR_IN_DELETED_TRANSITION & VAR_ADDED))) {
>
>
>
>
> + if (CompareWithValidVariable (StoreInfo, MaxIndex,
> VariableHeader, VariableName, VendorGuid, PtrTrack) == EFI_SUCCESS) {
>
>
>
>
> + if (VariableHeader->State == (VAR_IN_DELETED_TRANSITION &
> VAR_ADDED)) {
>
>
>
>
> + InDeletedVariable = PtrTrack->CurrPtr;
>
>
>
>
> + } else {
>
>
>
>
> + return EFI_SUCCESS;
>
>
>
>
> + }
>
>
>
>
> }
>
>
>
>
> }
>
>
>
>
> }
>
>
>
>
> --
>
>
>
>
> 2.20.1.windows.1
>
>
>
>
>
>
>
>
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#94851): https://edk2.groups.io/g/devel/message/94851
Mute This Topic: https://groups.io/mt/94009355/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] 回复: [edk2-devel] [PATCH] MdeModulePkg VariablePei: Add Variable state check when find variable in IndexTable.
Hi gaoliming: I am sorry I missed it, I will take a look, thanks very much! 9 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#94845): https://edk2.groups.io/g/devel/message/94845 Mute This Topic: https://groups.io/mt/94193123/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdeModulePkg VariablePei: Add Variable state check when find variable in IndexTable.
Hello, Can anyone please help to review the patch? Thanks very much! -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#94824): https://edk2.groups.io/g/devel/message/94824 Mute This Topic: https://groups.io/mt/93965445/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] MdeModulePkg VariablePei: Add Variable state check when find variable in IndexTable.
When read a variable in PEI, it will find it first in the HOB, then find in
variable store. When find in variable store, it will check the variable state,
but find in HOB, it doesn't check the state, so if the variable was changed, it
will find the obsolete variable in the HOB.
Signed-off-by: jdzhang
---
MdeModulePkg/Universal/Variable/Pei/Variable.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.c
b/MdeModulePkg/Universal/Variable/Pei/Variable.c
index 26a4c73b45..dffbd8cdb1 100644
--- a/MdeModulePkg/Universal/Variable/Pei/Variable.c
+++ b/MdeModulePkg/Universal/Variable/Pei/Variable.c
@@ -866,11 +866,13 @@ FindVariableEx (
Offset += IndexTable->Index[Index];
MaxIndex = (VARIABLE_HEADER *)((UINT8 *)IndexTable->StartPtr + Offset);
GetVariableHeader (StoreInfo, MaxIndex, );
- if (CompareWithValidVariable (StoreInfo, MaxIndex, VariableHeader,
VariableName, VendorGuid, PtrTrack) == EFI_SUCCESS) {
- if (VariableHeader->State == (VAR_IN_DELETED_TRANSITION & VAR_ADDED)) {
- InDeletedVariable = PtrTrack->CurrPtr;
- } else {
- return EFI_SUCCESS;
+ if ((VariableHeader->State == VAR_ADDED) || (VariableHeader->State ==
(VAR_IN_DELETED_TRANSITION & VAR_ADDED))) {
+ if (CompareWithValidVariable (StoreInfo, MaxIndex, VariableHeader,
VariableName, VendorGuid, PtrTrack) == EFI_SUCCESS) {
+ if (VariableHeader->State == (VAR_IN_DELETED_TRANSITION &
VAR_ADDED)) {
+ InDeletedVariable = PtrTrack->CurrPtr;
+ } else {
+ return EFI_SUCCESS;
+ }
}
}
}
--
2.20.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#94446): https://edk2.groups.io/g/devel/message/94446
Mute This Topic: https://groups.io/mt/93965445/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
From d0a1e384ad04185cab915d2d577ac854169c3921 Mon Sep 17 00:00:00 2001
From: jdzhang
Date: Wed, 28 Sep 2022 10:57:47 +0800
Subject: [PATCH] MdeModulePkg VariablePei: Add Variable state check when find
variable in IndexTable.
When read a variable in PEI, it will find it first in the HOB, then find in
variable store. When find in variable store, it will check the variable state,
but find in HOB, it doesn't check the state, so if the variable was changed, it
will find the obsolete variable in the HOB.
Signed-off-by: jdzhang
---
MdeModulePkg/Universal/Variable/Pei/Variable.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.c
b/MdeModulePkg/Universal/Variable/Pei/Variable.c
index 26a4c73b45..dffbd8cdb1 100644
--- a/MdeModulePkg/Universal/Variable/Pei/Variable.c
+++ b/MdeModulePkg/Universal/Variable/Pei/Variable.c
@@ -866,11 +866,13 @@ FindVariableEx (
Offset += IndexTable->Index[Index];
MaxIndex = (VARIABLE_HEADER *)((UINT8 *)IndexTable->StartPtr + Offset);
GetVariableHeader (StoreInfo, MaxIndex, );
- if (CompareWithValidVariable (StoreInfo, MaxIndex, VariableHeader,
VariableName, VendorGuid, PtrTrack) == EFI_SUCCESS) {
-if (VariableHeader->State == (VAR_IN_DELETED_TRANSITION & VAR_ADDED)) {
- InDeletedVariable = PtrTrack->CurrPtr;
-} else {
- return EFI_SUCCESS;
+ if ((VariableHeader->State == VAR_ADDED) || (VariableHeader->State ==
(VAR_IN_DELETED_TRANSITION & VAR_ADDED))) {
+if (CompareWithValidVariable (StoreInfo, MaxIndex, VariableHeader,
VariableName, VendorGuid, PtrTrack) == EFI_SUCCESS) {
+ if (VariableHeader->State == (VAR_IN_DELETED_TRANSITION &
VAR_ADDED)) {
+InDeletedVariable = PtrTrack->CurrPtr;
+ } else {
+return EFI_SUCCESS;
+ }
}
}
}
--
2.20.1.windows.1
Re: [edk2-devel] [PATCH] REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4074
OK. Thanks! -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#94051): https://edk2.groups.io/g/devel/message/94051 Mute This Topic: https://groups.io/mt/93819720/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4074
Update XhcEvaluateContext/XhcEvaluateContext64 to properly initialize the input context for Evaluate Context command. Signed-off-by: jdzhang --- MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c index c2906e06fd..4f2e9c3f6b 100644 --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c @@ -3957,6 +3957,7 @@ XhcEvaluateContext ( CMD_TRB_EVALUATE_CONTEXT CmdTrbEvalu; EVT_TRB_COMMAND_COMPLETION *EvtTrb; INPUT_CONTEXT *InputContext; + DEVICE_CONTEXT *OutputContext; EFI_PHYSICAL_ADDRESS PhyAddr; ASSERT (Xhc->UsbDevContext[SlotId].SlotId != 0); @@ -3965,10 +3966,14 @@ XhcEvaluateContext ( // 4.6.7 Evaluate Context // InputContext = Xhc->UsbDevContext[SlotId].InputContext; + OutputContext = Xhc->UsbDevContext[SlotId].OutputContext; ZeroMem (InputContext, sizeof (INPUT_CONTEXT)); + CopyMem (>EP[0], >EP[0], sizeof (ENDPOINT_CONTEXT)); + InputContext->InputControlContext.Dword2 |= BIT1; InputContext->EP[0].MaxPacketSize = MaxPacketSize; + InputContext->EP[0].EPState = 0; ZeroMem (, sizeof (CmdTrbEvalu)); PhyAddr = UsbHcGetPciAddrForHostAddr (Xhc->MemPool, InputContext, sizeof (INPUT_CONTEXT)); @@ -4013,6 +4018,7 @@ XhcEvaluateContext64 ( CMD_TRB_EVALUATE_CONTEXT CmdTrbEvalu; EVT_TRB_COMMAND_COMPLETION *EvtTrb; INPUT_CONTEXT_64 *InputContext; + DEVICE_CONTEXT_64 *OutputContext; EFI_PHYSICAL_ADDRESS PhyAddr; ASSERT (Xhc->UsbDevContext[SlotId].SlotId != 0); @@ -4021,10 +4027,14 @@ XhcEvaluateContext64 ( // 4.6.7 Evaluate Context // InputContext = Xhc->UsbDevContext[SlotId].InputContext; + OutputContext = Xhc->UsbDevContext[SlotId].OutputContext; ZeroMem (InputContext, sizeof (INPUT_CONTEXT_64)); + CopyMem (>EP[0], >EP[0], sizeof (ENDPOINT_CONTEXT_64)); + InputContext->InputControlContext.Dword2 |= BIT1; InputContext->EP[0].MaxPacketSize = MaxPacketSize; + InputContext->EP[0].EPState = 0; ZeroMem (, sizeof (CmdTrbEvalu)); PhyAddr = UsbHcGetPciAddrForHostAddr (Xhc->MemPool, InputContext, sizeof (INPUT_CONTEXT_64)); -- 2.20.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#94030): https://edk2.groups.io/g/devel/message/94030 Mute This Topic: https://groups.io/mt/93819720/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
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！ -- 张家定 固件产品部 产品三部 昆仑太科（北京）技术股份有限公司 手机：15810479495 电话：010-89056290-8050 邮箱：jdzh...@kunluntech.com.cn 地址：北京市海淀区北四环中路211号太极大厦10层 邮编：100083 发件人："Wu, Hao A" 发送日期：2022-09-20 14:27:21 收件人："devel@edk2.groups.io" ,"jdzh...@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 On Behalf Of Jiading Zhang Sent: Tuesday, September 20, 2022 2:00 PM To: Wu; Wu, Hao A ; 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! -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#93985): https://edk2.groups.io/g/devel/message/93985 Mute This Topic: https://groups.io/mt/93673303/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
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! -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#93981): https://edk2.groups.io/g/devel/message/93981 Mute This Topic: https://groups.io/mt/93673303/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
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. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#93976): https://edk2.groups.io/g/devel/message/93976 Mute This Topic: https://groups.io/mt/93673303/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
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. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#93741): https://edk2.groups.io/g/devel/message/93741 Mute This Topic: https://groups.io/mt/93673303/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[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 --- 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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#93737): https://edk2.groups.io/g/devel/message/93737 Mute This Topic: https://groups.io/mt/93673303/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
