From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by mx.groups.io with SMTP id smtpd.web10.8880.1583387776312011148 for ; Wed, 04 Mar 2020 21:56:16 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.115, mailfrom: hao.a.wu@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Mar 2020 21:56:15 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,516,1574150400"; d="scan'208";a="352284985" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga001.fm.intel.com with ESMTP; 04 Mar 2020 21:56:15 -0800 Received: from fmsmsx158.amr.corp.intel.com (10.18.116.75) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 4 Mar 2020 21:56:15 -0800 Received: from shsmsx153.ccr.corp.intel.com (10.239.6.53) by fmsmsx158.amr.corp.intel.com (10.18.116.75) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 4 Mar 2020 21:56:15 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.206]) by SHSMSX153.ccr.corp.intel.com ([169.254.12.96]) with mapi id 14.03.0439.000; Thu, 5 Mar 2020 13:56:12 +0800 From: "Wu, Hao A" To: "devel@edk2.groups.io" , "Wu, Hao A" , Gaurav Jain 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. Thread-Topic: [edk2-devel] [PATCH v3 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test. Thread-Index: AQHV66aW2jY/X1MOW0KTWXM7fPbRTKgstfWQgAzYCcA= Date: Thu, 5 Mar 2020 05:56:12 +0000 Message-ID: References: <20200225120041.31570-1-gaurav.jain@nxp.com> In-Reply-To: Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: hao.a.wu@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----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. >=20 > > -----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; G= aurav > > 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 > > --- > > > > 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 >=20 >=20 > Reviewed-by: Hao A Wu > I will push this patch *after* the upcoming edk2-stable202002 tag. Patch has been pushed via commit 3b9cd71454. Best Regards, Hao Wu >=20 > Best Regards, > Hao Wu >=20 >=20 > > > > 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 =3D=3D NULL) { > > + return EFI_INVALID_PARAMETER; > > + } > > + > > + Dev =3D NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); > > + Count =3D 1; > > + > > + Status =3D 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 =3D=3D NULL) { > > + return EFI_INVALID_PARAMETER; > > + } > > + > > + Dev =3D NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); > > + Count =3D 1; > > + > > + Status =3D 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 >=3D EfiPciIoWidthMaximum) { > > + return EFI_INVALID_PARAMETER; > > + } > > + > > + if (Buffer =3D=3D NULL) { > > + return EFI_INVALID_PARAMETER; > > + } > > + > > + Dev =3D NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); > > + > > + Status =3D 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 >=3D EfiPciIoWidthMaximum) { > > + return EFI_INVALID_PARAMETER; > > + } > > + > > + if (Buffer =3D=3D NULL) { > > + return EFI_INVALID_PARAMETER; > > + } > > + > > + Dev =3D NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); > > + > > + Status =3D 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 =3D NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); > > + > > + Status =3D GetBarResource (Dev, DestBarIndex, &DestDesc); > > + if (EFI_ERROR (Status)) { > > + return Status; > > + } > > + > > + if (DestOffset + (Count << (Width & 0x3)) > DestDesc->AddrLen) { > > + return EFI_UNSUPPORTED; > > + } > > + > > + Status =3D 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 =3D NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); > > > > if ((Attributes & (~(DEV_SUPPORTED_ATTRIBUTES))) !=3D 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)) !=3D 0) { > > + return EFI_UNSUPPORTED; > > + } > > + > > + if (Offset =3D=3D NULL || Length =3D=3D NULL) { > > + return EFI_INVALID_PARAMETER; > > + } > > + > > + Dev =3D NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); > > + Width =3D EfiPciIoWidthUint8; > > + Count =3D (UINT32) *Length; > > + > > + Status =3D 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 >=20 >=20 >=20