[edk2-devel] [PATCH v3 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test.
Wu, Hao A
hao.a.wu at intel.com
Thu Mar 5 05:56:12 UTC 2020
> -----Original Message-----
> From: devel at edk2.groups.io [mailto:devel at edk2.groups.io] On Behalf Of Wu,
> Hao A
> Sent: Wednesday, February 26, 2020 9:55 AM
> To: Gaurav Jain; devel at 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 at nxp.com]
> > Sent: Tuesday, February 25, 2020 8:01 PM
> > To: devel at 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 at 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 at 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
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#55474): https://edk2.groups.io/g/devel/message/55474
Mute This Topic: https://groups.io/mt/71528680/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list