From: "Wu, Hao A" <hao.a.wu@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
"Wu, Hao A" <hao.a.wu@intel.com>,
Gaurav Jain <gaurav.jain@nxp.com>
Cc: "Wang, Jian J" <jian.j.wang@intel.com>,
"Ni, Ray" <ray.ni@intel.com>,
"Ard Biesheuvel" <ard.biesheuvel@linaro.org>,
Pankaj Bansal <pankaj.bansal@nxp.com>
Subject: Re: [edk2-devel] [PATCH v3 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test.
Date: Thu, 5 Mar 2020 05:56:12 +0000 [thread overview]
Message-ID: <B80AF82E9BFB8E4FBD8C89DA810C6A093C9BA36D@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <B80AF82E9BFB8E4FBD8C89DA810C6A093C9ABD13@SHSMSX104.ccr.corp.intel.com>
> -----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
>
>
>
prev parent reply other threads:[~2020-03-05 5:56 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
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 ` Wu, Hao A [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=B80AF82E9BFB8E4FBD8C89DA810C6A093C9BA36D@SHSMSX104.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox