* [PATCH v3 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test.
@ 2020-02-25 12:00 Gaurav Jain
2020-02-26 1:54 ` Wu, Hao A
0 siblings, 1 reply; 3+ messages in thread
From: Gaurav Jain @ 2020-02-25 12:00 UTC (permalink / raw)
To: devel
Cc: Jian J Wang, Hao A Wu, Ray Ni, Ard Biesheuvel, Pankaj Bansal,
Gaurav Jain
ASSERT in PollMem_Conf, CopyMem_Conf, SetBarAttributes_Conf
Conformance Test.
SCT Test expect return as Invalid Parameter or Unsupported.
Added Checks for Function Parameters.
return Invalid or Unsupported if Check fails.
Added Checks in PciIoPollIo(), PciIoIoRead()
PciIoIoWrite()
Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com>
---
Notes:
v3
- Corrected Width check in PciIoIoRead, PciIoIoWrite.
- Removed BarIndex check as it is already in GetBarResource().
- Moved DEV_SUPPORTED_ATTRIBUTES Macro from NonDiscoverablePciDeviceIo.c
to NonDiscoverablePciDeviceIo.h
v2
- Reverted ASSERT(FALSE) code.
- Added Checks for Width, BarIndex, Buffer,
Address range in PciIoIoRead, PciIoIoWrite.
- Added Checks for Width, BarIndex, Result,
Address range in PciIoPollIo, PciIoPollMem,
PciIoCopyMem.
- Added Checks for Attributes, BarIndex,
Address range in PciIoSetBarAttributes.
.../NonDiscoverablePciDeviceIo.c | 155 +++++++++++++++++-
.../NonDiscoverablePciDeviceIo.h | 3 +
2 files changed, 155 insertions(+), 3 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
index 2d55c9699322..c3e83003a01c 100644
--- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
+++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
@@ -93,6 +93,31 @@ PciIoPollMem (
OUT UINT64 *Result
)
{
+ NON_DISCOVERABLE_PCI_DEVICE *Dev;
+ EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc;
+ UINTN Count;
+ EFI_STATUS Status;
+
+ if ((UINT32)Width > EfiPciIoWidthUint64) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (Result == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
+ Count = 1;
+
+ Status = GetBarResource (Dev, BarIndex, &Desc);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) {
+ return EFI_UNSUPPORTED;
+ }
+
ASSERT (FALSE);
return EFI_UNSUPPORTED;
}
@@ -126,6 +151,31 @@ PciIoPollIo (
OUT UINT64 *Result
)
{
+ NON_DISCOVERABLE_PCI_DEVICE *Dev;
+ EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc;
+ UINTN Count;
+ EFI_STATUS Status;
+
+ if ((UINT32)Width > EfiPciIoWidthUint64) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (Result == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
+ Count = 1;
+
+ Status = GetBarResource (Dev, BarIndex, &Desc);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) {
+ return EFI_UNSUPPORTED;
+ }
+
ASSERT (FALSE);
return EFI_UNSUPPORTED;
}
@@ -396,6 +446,29 @@ PciIoIoRead (
IN OUT VOID *Buffer
)
{
+ NON_DISCOVERABLE_PCI_DEVICE *Dev;
+ EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc;
+ EFI_STATUS Status;
+
+ if ((UINT32)Width >= EfiPciIoWidthMaximum) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (Buffer == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
+
+ Status = GetBarResource (Dev, BarIndex, &Desc);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) {
+ return EFI_UNSUPPORTED;
+ }
+
ASSERT (FALSE);
return EFI_UNSUPPORTED;
}
@@ -425,6 +498,29 @@ PciIoIoWrite (
IN OUT VOID *Buffer
)
{
+ NON_DISCOVERABLE_PCI_DEVICE *Dev;
+ EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc;
+ EFI_STATUS Status;
+
+ if ((UINT32)Width >= EfiPciIoWidthMaximum) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (Buffer == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
+
+ Status = GetBarResource (Dev, BarIndex, &Desc);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) {
+ return EFI_UNSUPPORTED;
+ }
+
ASSERT (FALSE);
return EFI_UNSUPPORTED;
}
@@ -556,6 +652,35 @@ PciIoCopyMem (
IN UINTN Count
)
{
+ NON_DISCOVERABLE_PCI_DEVICE *Dev;
+ EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *DestDesc;
+ EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *SrcDesc;
+ EFI_STATUS Status;
+
+ if ((UINT32)Width > EfiPciIoWidthUint64) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
+
+ Status = GetBarResource (Dev, DestBarIndex, &DestDesc);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ if (DestOffset + (Count << (Width & 0x3)) > DestDesc->AddrLen) {
+ return EFI_UNSUPPORTED;
+ }
+
+ Status = GetBarResource (Dev, SrcBarIndex, &SrcDesc);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ if (SrcOffset + (Count << (Width & 0x3)) > SrcDesc->AddrLen) {
+ return EFI_UNSUPPORTED;
+ }
+
ASSERT (FALSE);
return EFI_UNSUPPORTED;
}
@@ -1265,9 +1390,6 @@ PciIoAttributes (
NON_DISCOVERABLE_PCI_DEVICE *Dev;
BOOLEAN Enable;
- #define DEV_SUPPORTED_ATTRIBUTES \
- (EFI_PCI_DEVICE_ENABLE | EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE)
-
Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
if ((Attributes & (~(DEV_SUPPORTED_ATTRIBUTES))) != 0) {
@@ -1414,6 +1536,33 @@ PciIoSetBarAttributes (
IN OUT UINT64 *Length
)
{
+ NON_DISCOVERABLE_PCI_DEVICE *Dev;
+ EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc;
+ EFI_PCI_IO_PROTOCOL_WIDTH Width;
+ UINTN Count;
+ EFI_STATUS Status;
+
+ if ((Attributes & (~DEV_SUPPORTED_ATTRIBUTES)) != 0) {
+ return EFI_UNSUPPORTED;
+ }
+
+ if (Offset == NULL || Length == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
+ Width = EfiPciIoWidthUint8;
+ Count = (UINT32) *Length;
+
+ Status = GetBarResource(Dev, BarIndex, &Desc);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ if (*Offset + (Count << (Width & 0x3)) > Desc->AddrLen) {
+ return EFI_UNSUPPORTED;
+ }
+
ASSERT (FALSE);
return EFI_UNSUPPORTED;
}
diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.h b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.h
index 6511fbb13770..15541c281153 100644
--- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.h
+++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.h
@@ -30,6 +30,9 @@
CR (PciIoPointer, NON_DISCOVERABLE_PCI_DEVICE, PciIo, \
NON_DISCOVERABLE_PCI_DEVICE_SIG)
+#define DEV_SUPPORTED_ATTRIBUTES \
+ (EFI_PCI_DEVICE_ENABLE | EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE)
+
#define PCI_ID_VENDOR_UNKNOWN 0xffff
#define PCI_ID_DEVICE_DONTCARE 0x0000
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v3 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test.
2020-02-25 12:00 [PATCH v3 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test Gaurav Jain
@ 2020-02-26 1:54 ` Wu, Hao A
2020-03-05 5:56 ` [edk2-devel] " Wu, Hao A
0 siblings, 1 reply; 3+ messages in thread
From: Wu, Hao A @ 2020-02-26 1:54 UTC (permalink / raw)
To: Gaurav Jain, devel@edk2.groups.io
Cc: Wang, Jian J, Ni, Ray, Ard Biesheuvel, Pankaj Bansal
> -----Original Message-----
> From: Gaurav Jain [mailto:gaurav.jain@nxp.com]
> Sent: Tuesday, February 25, 2020 8:01 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J; Wu, Hao A; Ni, Ray; Ard Biesheuvel; Pankaj Bansal; Gaurav
> Jain
> Subject: [PATCH v3 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO
> Protocol Test.
>
> ASSERT in PollMem_Conf, CopyMem_Conf, SetBarAttributes_Conf
> Conformance Test.
> SCT Test expect return as Invalid Parameter or Unsupported.
> Added Checks for Function Parameters.
> return Invalid or Unsupported if Check fails.
>
> Added Checks in PciIoPollIo(), PciIoIoRead()
> PciIoIoWrite()
>
> Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com>
> ---
>
> Notes:
> v3
> - Corrected Width check in PciIoIoRead, PciIoIoWrite.
> - Removed BarIndex check as it is already in GetBarResource().
> - Moved DEV_SUPPORTED_ATTRIBUTES Macro from
> NonDiscoverablePciDeviceIo.c
> to NonDiscoverablePciDeviceIo.h
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
I will push this patch *after* the upcoming edk2-stable202002 tag.
Best Regards,
Hao Wu
>
> v2
> - Reverted ASSERT(FALSE) code.
> - Added Checks for Width, BarIndex, Buffer,
> Address range in PciIoIoRead, PciIoIoWrite.
> - Added Checks for Width, BarIndex, Result,
> Address range in PciIoPollIo, PciIoPollMem,
> PciIoCopyMem.
> - Added Checks for Attributes, BarIndex,
> Address range in PciIoSetBarAttributes.
>
> .../NonDiscoverablePciDeviceIo.c | 155 +++++++++++++++++-
> .../NonDiscoverablePciDeviceIo.h | 3 +
> 2 files changed, 155 insertions(+), 3 deletions(-)
>
> diff --git
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP
> ciDeviceIo.c
> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP
> ciDeviceIo.c
> index 2d55c9699322..c3e83003a01c 100644
> ---
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP
> ciDeviceIo.c
> +++
> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP
> ciDeviceIo.c
> @@ -93,6 +93,31 @@ PciIoPollMem (
> OUT UINT64 *Result
> )
> {
> + NON_DISCOVERABLE_PCI_DEVICE *Dev;
> + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc;
> + UINTN Count;
> + EFI_STATUS Status;
> +
> + if ((UINT32)Width > EfiPciIoWidthUint64) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + if (Result == NULL) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
> + Count = 1;
> +
> + Status = GetBarResource (Dev, BarIndex, &Desc);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) {
> + return EFI_UNSUPPORTED;
> + }
> +
> ASSERT (FALSE);
> return EFI_UNSUPPORTED;
> }
> @@ -126,6 +151,31 @@ PciIoPollIo (
> OUT UINT64 *Result
> )
> {
> + NON_DISCOVERABLE_PCI_DEVICE *Dev;
> + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc;
> + UINTN Count;
> + EFI_STATUS Status;
> +
> + if ((UINT32)Width > EfiPciIoWidthUint64) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + if (Result == NULL) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
> + Count = 1;
> +
> + Status = GetBarResource (Dev, BarIndex, &Desc);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) {
> + return EFI_UNSUPPORTED;
> + }
> +
> ASSERT (FALSE);
> return EFI_UNSUPPORTED;
> }
> @@ -396,6 +446,29 @@ PciIoIoRead (
> IN OUT VOID *Buffer
> )
> {
> + NON_DISCOVERABLE_PCI_DEVICE *Dev;
> + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc;
> + EFI_STATUS Status;
> +
> + if ((UINT32)Width >= EfiPciIoWidthMaximum) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + if (Buffer == NULL) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
> +
> + Status = GetBarResource (Dev, BarIndex, &Desc);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) {
> + return EFI_UNSUPPORTED;
> + }
> +
> ASSERT (FALSE);
> return EFI_UNSUPPORTED;
> }
> @@ -425,6 +498,29 @@ PciIoIoWrite (
> IN OUT VOID *Buffer
> )
> {
> + NON_DISCOVERABLE_PCI_DEVICE *Dev;
> + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc;
> + EFI_STATUS Status;
> +
> + if ((UINT32)Width >= EfiPciIoWidthMaximum) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + if (Buffer == NULL) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
> +
> + Status = GetBarResource (Dev, BarIndex, &Desc);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) {
> + return EFI_UNSUPPORTED;
> + }
> +
> ASSERT (FALSE);
> return EFI_UNSUPPORTED;
> }
> @@ -556,6 +652,35 @@ PciIoCopyMem (
> IN UINTN Count
> )
> {
> + NON_DISCOVERABLE_PCI_DEVICE *Dev;
> + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *DestDesc;
> + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *SrcDesc;
> + EFI_STATUS Status;
> +
> + if ((UINT32)Width > EfiPciIoWidthUint64) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
> +
> + Status = GetBarResource (Dev, DestBarIndex, &DestDesc);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + if (DestOffset + (Count << (Width & 0x3)) > DestDesc->AddrLen) {
> + return EFI_UNSUPPORTED;
> + }
> +
> + Status = GetBarResource (Dev, SrcBarIndex, &SrcDesc);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + if (SrcOffset + (Count << (Width & 0x3)) > SrcDesc->AddrLen) {
> + return EFI_UNSUPPORTED;
> + }
> +
> ASSERT (FALSE);
> return EFI_UNSUPPORTED;
> }
> @@ -1265,9 +1390,6 @@ PciIoAttributes (
> NON_DISCOVERABLE_PCI_DEVICE *Dev;
> BOOLEAN Enable;
>
> - #define DEV_SUPPORTED_ATTRIBUTES \
> - (EFI_PCI_DEVICE_ENABLE |
> EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE)
> -
> Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
>
> if ((Attributes & (~(DEV_SUPPORTED_ATTRIBUTES))) != 0) {
> @@ -1414,6 +1536,33 @@ PciIoSetBarAttributes (
> IN OUT UINT64 *Length
> )
> {
> + NON_DISCOVERABLE_PCI_DEVICE *Dev;
> + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc;
> + EFI_PCI_IO_PROTOCOL_WIDTH Width;
> + UINTN Count;
> + EFI_STATUS Status;
> +
> + if ((Attributes & (~DEV_SUPPORTED_ATTRIBUTES)) != 0) {
> + return EFI_UNSUPPORTED;
> + }
> +
> + if (Offset == NULL || Length == NULL) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
> + Width = EfiPciIoWidthUint8;
> + Count = (UINT32) *Length;
> +
> + Status = GetBarResource(Dev, BarIndex, &Desc);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + if (*Offset + (Count << (Width & 0x3)) > Desc->AddrLen) {
> + return EFI_UNSUPPORTED;
> + }
> +
> ASSERT (FALSE);
> return EFI_UNSUPPORTED;
> }
> diff --git
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP
> ciDeviceIo.h
> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP
> ciDeviceIo.h
> index 6511fbb13770..15541c281153 100644
> ---
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP
> ciDeviceIo.h
> +++
> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP
> ciDeviceIo.h
> @@ -30,6 +30,9 @@
> CR (PciIoPointer, NON_DISCOVERABLE_PCI_DEVICE, PciIo, \
> NON_DISCOVERABLE_PCI_DEVICE_SIG)
>
> +#define DEV_SUPPORTED_ATTRIBUTES \
> + (EFI_PCI_DEVICE_ENABLE |
> EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE)
> +
> #define PCI_ID_VENDOR_UNKNOWN 0xffff
> #define PCI_ID_DEVICE_DONTCARE 0x0000
>
> --
> 2.17.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [edk2-devel] [PATCH v3 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test.
2020-02-26 1:54 ` Wu, Hao A
@ 2020-03-05 5:56 ` Wu, Hao A
0 siblings, 0 replies; 3+ messages in thread
From: Wu, Hao A @ 2020-03-05 5:56 UTC (permalink / raw)
To: devel@edk2.groups.io, Wu, Hao A, Gaurav Jain
Cc: Wang, Jian J, Ni, Ray, Ard Biesheuvel, Pankaj Bansal
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Wu,
> Hao A
> Sent: Wednesday, February 26, 2020 9:55 AM
> To: Gaurav Jain; devel@edk2.groups.io
> Cc: Wang, Jian J; Ni, Ray; Ard Biesheuvel; Pankaj Bansal
> Subject: Re: [edk2-devel] [PATCH v3 1/1] MdeModulePkg/Pci: Fixed Asserts in
> SCT PCIIO Protocol Test.
>
> > -----Original Message-----
> > From: Gaurav Jain [mailto:gaurav.jain@nxp.com]
> > Sent: Tuesday, February 25, 2020 8:01 PM
> > To: devel@edk2.groups.io
> > Cc: Wang, Jian J; Wu, Hao A; Ni, Ray; Ard Biesheuvel; Pankaj Bansal; Gaurav
> > Jain
> > Subject: [PATCH v3 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO
> > Protocol Test.
> >
> > ASSERT in PollMem_Conf, CopyMem_Conf, SetBarAttributes_Conf
> > Conformance Test.
> > SCT Test expect return as Invalid Parameter or Unsupported.
> > Added Checks for Function Parameters.
> > return Invalid or Unsupported if Check fails.
> >
> > Added Checks in PciIoPollIo(), PciIoIoRead()
> > PciIoIoWrite()
> >
> > Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com>
> > ---
> >
> > Notes:
> > v3
> > - Corrected Width check in PciIoIoRead, PciIoIoWrite.
> > - Removed BarIndex check as it is already in GetBarResource().
> > - Moved DEV_SUPPORTED_ATTRIBUTES Macro from
> > NonDiscoverablePciDeviceIo.c
> > to NonDiscoverablePciDeviceIo.h
>
>
> Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
> I will push this patch *after* the upcoming edk2-stable202002 tag.
Patch has been pushed via commit 3b9cd71454.
Best Regards,
Hao Wu
>
> Best Regards,
> Hao Wu
>
>
> >
> > v2
> > - Reverted ASSERT(FALSE) code.
> > - Added Checks for Width, BarIndex, Buffer,
> > Address range in PciIoIoRead, PciIoIoWrite.
> > - Added Checks for Width, BarIndex, Result,
> > Address range in PciIoPollIo, PciIoPollMem,
> > PciIoCopyMem.
> > - Added Checks for Attributes, BarIndex,
> > Address range in PciIoSetBarAttributes.
> >
> > .../NonDiscoverablePciDeviceIo.c | 155 +++++++++++++++++-
> > .../NonDiscoverablePciDeviceIo.h | 3 +
> > 2 files changed, 155 insertions(+), 3 deletions(-)
> >
> > diff --git
> > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP
> > ciDeviceIo.c
> > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP
> > ciDeviceIo.c
> > index 2d55c9699322..c3e83003a01c 100644
> > ---
> > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP
> > ciDeviceIo.c
> > +++
> > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP
> > ciDeviceIo.c
> > @@ -93,6 +93,31 @@ PciIoPollMem (
> > OUT UINT64 *Result
> > )
> > {
> > + NON_DISCOVERABLE_PCI_DEVICE *Dev;
> > + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc;
> > + UINTN Count;
> > + EFI_STATUS Status;
> > +
> > + if ((UINT32)Width > EfiPciIoWidthUint64) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + if (Result == NULL) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
> > + Count = 1;
> > +
> > + Status = GetBarResource (Dev, BarIndex, &Desc);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > +
> > + if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) {
> > + return EFI_UNSUPPORTED;
> > + }
> > +
> > ASSERT (FALSE);
> > return EFI_UNSUPPORTED;
> > }
> > @@ -126,6 +151,31 @@ PciIoPollIo (
> > OUT UINT64 *Result
> > )
> > {
> > + NON_DISCOVERABLE_PCI_DEVICE *Dev;
> > + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc;
> > + UINTN Count;
> > + EFI_STATUS Status;
> > +
> > + if ((UINT32)Width > EfiPciIoWidthUint64) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + if (Result == NULL) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
> > + Count = 1;
> > +
> > + Status = GetBarResource (Dev, BarIndex, &Desc);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > +
> > + if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) {
> > + return EFI_UNSUPPORTED;
> > + }
> > +
> > ASSERT (FALSE);
> > return EFI_UNSUPPORTED;
> > }
> > @@ -396,6 +446,29 @@ PciIoIoRead (
> > IN OUT VOID *Buffer
> > )
> > {
> > + NON_DISCOVERABLE_PCI_DEVICE *Dev;
> > + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc;
> > + EFI_STATUS Status;
> > +
> > + if ((UINT32)Width >= EfiPciIoWidthMaximum) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + if (Buffer == NULL) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
> > +
> > + Status = GetBarResource (Dev, BarIndex, &Desc);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > +
> > + if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) {
> > + return EFI_UNSUPPORTED;
> > + }
> > +
> > ASSERT (FALSE);
> > return EFI_UNSUPPORTED;
> > }
> > @@ -425,6 +498,29 @@ PciIoIoWrite (
> > IN OUT VOID *Buffer
> > )
> > {
> > + NON_DISCOVERABLE_PCI_DEVICE *Dev;
> > + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc;
> > + EFI_STATUS Status;
> > +
> > + if ((UINT32)Width >= EfiPciIoWidthMaximum) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + if (Buffer == NULL) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
> > +
> > + Status = GetBarResource (Dev, BarIndex, &Desc);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > +
> > + if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) {
> > + return EFI_UNSUPPORTED;
> > + }
> > +
> > ASSERT (FALSE);
> > return EFI_UNSUPPORTED;
> > }
> > @@ -556,6 +652,35 @@ PciIoCopyMem (
> > IN UINTN Count
> > )
> > {
> > + NON_DISCOVERABLE_PCI_DEVICE *Dev;
> > + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *DestDesc;
> > + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *SrcDesc;
> > + EFI_STATUS Status;
> > +
> > + if ((UINT32)Width > EfiPciIoWidthUint64) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
> > +
> > + Status = GetBarResource (Dev, DestBarIndex, &DestDesc);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > +
> > + if (DestOffset + (Count << (Width & 0x3)) > DestDesc->AddrLen) {
> > + return EFI_UNSUPPORTED;
> > + }
> > +
> > + Status = GetBarResource (Dev, SrcBarIndex, &SrcDesc);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > +
> > + if (SrcOffset + (Count << (Width & 0x3)) > SrcDesc->AddrLen) {
> > + return EFI_UNSUPPORTED;
> > + }
> > +
> > ASSERT (FALSE);
> > return EFI_UNSUPPORTED;
> > }
> > @@ -1265,9 +1390,6 @@ PciIoAttributes (
> > NON_DISCOVERABLE_PCI_DEVICE *Dev;
> > BOOLEAN Enable;
> >
> > - #define DEV_SUPPORTED_ATTRIBUTES \
> > - (EFI_PCI_DEVICE_ENABLE |
> > EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE)
> > -
> > Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
> >
> > if ((Attributes & (~(DEV_SUPPORTED_ATTRIBUTES))) != 0) {
> > @@ -1414,6 +1536,33 @@ PciIoSetBarAttributes (
> > IN OUT UINT64 *Length
> > )
> > {
> > + NON_DISCOVERABLE_PCI_DEVICE *Dev;
> > + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc;
> > + EFI_PCI_IO_PROTOCOL_WIDTH Width;
> > + UINTN Count;
> > + EFI_STATUS Status;
> > +
> > + if ((Attributes & (~DEV_SUPPORTED_ATTRIBUTES)) != 0) {
> > + return EFI_UNSUPPORTED;
> > + }
> > +
> > + if (Offset == NULL || Length == NULL) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
> > + Width = EfiPciIoWidthUint8;
> > + Count = (UINT32) *Length;
> > +
> > + Status = GetBarResource(Dev, BarIndex, &Desc);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > +
> > + if (*Offset + (Count << (Width & 0x3)) > Desc->AddrLen) {
> > + return EFI_UNSUPPORTED;
> > + }
> > +
> > ASSERT (FALSE);
> > return EFI_UNSUPPORTED;
> > }
> > diff --git
> > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP
> > ciDeviceIo.h
> > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP
> > ciDeviceIo.h
> > index 6511fbb13770..15541c281153 100644
> > ---
> > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP
> > ciDeviceIo.h
> > +++
> > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP
> > ciDeviceIo.h
> > @@ -30,6 +30,9 @@
> > CR (PciIoPointer, NON_DISCOVERABLE_PCI_DEVICE, PciIo, \
> > NON_DISCOVERABLE_PCI_DEVICE_SIG)
> >
> > +#define DEV_SUPPORTED_ATTRIBUTES \
> > + (EFI_PCI_DEVICE_ENABLE |
> > EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE)
> > +
> > #define PCI_ID_VENDOR_UNKNOWN 0xffff
> > #define PCI_ID_DEVICE_DONTCARE 0x0000
> >
> > --
> > 2.17.1
>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-03-05 5:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-02-25 12:00 [PATCH v3 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test Gaurav Jain
2020-02-26 1:54 ` Wu, Hao A
2020-03-05 5:56 ` [edk2-devel] " Wu, Hao A
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox