> -----Original Message-----
> From: Gaurav Jain [mailto:gaurav.j...@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.j...@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...@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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#54834): https://edk2.groups.io/g/devel/message/54834
Mute This Topic: https://groups.io/mt/71528680/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
