From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from EUR01-DB5-obe.outbound.protection.outlook.com (EUR01-DB5-obe.outbound.protection.outlook.com [40.107.15.58]) by mx.groups.io with SMTP id smtpd.web10.18317.1582533753781835082 for ; Mon, 24 Feb 2020 00:42:34 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@nxp.com header.s=selector2 header.b=kTwT7Sdw; spf=pass (domain: nxp.com, ip: 40.107.15.58, mailfrom: gaurav.jain@nxp.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=MDodPe05o7t5OOjWoudvg+nqfxfJO4/BYGA9kqH3PNQpmzbukf9H7tPkqAG9xTzNKw632WFVUVweXswwKMCHysIzfr7a366SjfT7puFimldBNevEmJaOL4ADtmsjT3tGOmuxqM9BscDqVbB64sL6r2hz1Py9lE6vgTVyL/XjJpkldWRVCN+V4tVHtb1v0BSUy9qn4smOPXfcSNb4ZFkNACs+yPhxP4gcjo6EDxF1vg/hpNzGU8ZcWgHDyGJuq+hyBhiullFFz0mAthI8y6IoCPQuQDwLmvp53msc+6FDkh1t2QkrI/VsQ8xpJbRgtSqtf4YoCRovUmAABh1oICxmmw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=pg9H3RFOFmz/vSMzE9/152eHnpA7b9CAhZqjZZiZbsA=; b=XVuFA0LFbDoIt6AWHRzFoheEz9VfsHqWpMT4kWIl+zeAF5LC7hXQC9rEjqvfvlB8q7Hu5aul4tgYq7LDxYo0MvLZLPL1DHrxJRwMTYb/ZEdvQxaYpD6y/WbkzcCDKq9dKoSj6oY5hCnLSXmEaxq0ZAko0sXxIjpA4Vb2LHYBwHBI4nXHinWOqlWb0XeTDVnYQpDL/PP7b6Z47mdL1dALf2LiLeABUlm1hovD3oOHZxnHYz7h7jzcrRnmU4GPhgoxlQM+CyN9Ib10/EidGmbI17Xn3dA2B6CSPus8k4EyBNtYXBQQQ4Kvyf0a4ORMNk3gogYkNQvioy6gtBZL+UMzYg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nxp.com; dmarc=pass action=none header.from=nxp.com; dkim=pass header.d=nxp.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=pg9H3RFOFmz/vSMzE9/152eHnpA7b9CAhZqjZZiZbsA=; b=kTwT7Sdw9umJIQxDsYmaSG6WVKFAk6WkhxZf0NvdHUYj6GLGNgW78COfiAiJmb3mpIia59mUZLji6LV61oqHK34PcW+7h6M7YD2qa4DxUnu0WpG4GLhMpldxteeGOvi+CzvtOp+1uKvfiycqyVmab/Fx9s/kzfRH56Q5Sz1YMfo= Received: from DB7PR04MB4091.eurprd04.prod.outlook.com (52.134.110.144) by DB7PR04MB4329.eurprd04.prod.outlook.com (52.135.128.157) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2750.21; Mon, 24 Feb 2020 08:42:31 +0000 Received: from DB7PR04MB4091.eurprd04.prod.outlook.com ([fe80::183:969:2508:3c92]) by DB7PR04MB4091.eurprd04.prod.outlook.com ([fe80::183:969:2508:3c92%6]) with mapi id 15.20.2750.021; Mon, 24 Feb 2020 08:42:31 +0000 From: "Gaurav Jain" To: "Wu, Hao A" , "devel@edk2.groups.io" , "Gao, Liming" , "afish@apple.com" , "lersek@redhat.com" , "leif@nuviainc.com" , "Kinney, Michael D" CC: "Wang, Jian J" , "Ni, Ray" , Ard Biesheuvel , Pankaj Bansal Subject: Re: [EXT] RE: [edk2-stable202002][edk2-devel] [PATCH v2 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test. Thread-Topic: [EXT] RE: [edk2-stable202002][edk2-devel] [PATCH v2 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test. Thread-Index: AdXoVX1UfqTUDarfQqSjaIalQXdrhgCiUCDgAAM/TyAAAFiDUA== Date: Mon, 24 Feb 2020 08:42:30 +0000 Message-ID: References: In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=gaurav.jain@nxp.com; x-originating-ip: [92.120.1.65] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 7c679bdb-a47b-4eb9-dd84-08d7b9057c8f x-ms-traffictypediagnostic: DB7PR04MB4329:|DB7PR04MB4329: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:9508; x-forefront-prvs: 032334F434 x-forefront-antispam-report: SFV:NSPM;SFS:(10001)(10009020)(4636009)(39860400002)(346002)(396003)(376002)(136003)(366004)(189003)(199004)(81156014)(81166006)(33656002)(478600001)(8676002)(186003)(7696005)(26005)(45080400002)(71200400001)(8936002)(53546011)(6506007)(44832011)(5660300002)(54906003)(110136005)(4326008)(66556008)(86362001)(66946007)(66446008)(55016002)(64756008)(2906002)(9686003)(76116006)(52536014)(66476007)(30864003)(316002)(7416002)(966005)(921003)(1121003);DIR:OUT;SFP:1101;SCL:1;SRVR:DB7PR04MB4329;H:DB7PR04MB4091.eurprd04.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;MX:1;A:1; received-spf: None (protection.outlook.com: nxp.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: JQvc7KFkbHbAA4DS9cfViaStrWY72MckOE6Lz9NFnLPnNJOIIfHeK75YmEZekFgJqKQ2XO60ev6QAKAkSKws01csmDNJOBrlNZgXAIvjCPhswtEW0891eYSZxfy4wAzv4+LzpLWQES+aVQqiUwtDNDQSgbvmfBBfU5OS59QF00I+Vy+h5QfAEwhPAv/6ZIHy5/9YehpCHIKS4WOIsbWjykH9sAbBiZnAg93li5Irv736QrF1Ari8jpw24jr5yiTHRyPYkHhLwPqKnRLIHrG7rl+rO21vovOwiLbHn8NULxYMI6EFfXFemeMuW/1I0judOJIzLGb/UnbgTQqYZn409FdGnlzBxeGZlT27Yi+9UnDOSR/+882d26ClIlii4o8ilegC+wytDLkg+ZoNhz21H2NT5FamlBbQ8v6jioVbcORK/MHXXK+/P09d/74Lhu25TeSKJHBYAlJmwbabWmZ5IOE3+J2SRL2am+DIcl02DeWmmUh/eA9TJZrU5X+JTunI5Hew4LaVQH6Qll2n/Xkt+433DlEsndVFCsVl27RdjgR13lHS3peZ+rjbLWNHfz6uiwUw7GJRzvb1a5nGW8TohLFLD4N389I0laO9Ft6yvBDGAPExzbmixg3budpoLWChJgvKZ+3IkyCfcJSbCZEFZtipukzJD6Q6Bp9yioNL/im17YgcU6njtf88uqnFy2QX x-ms-exchange-antispam-messagedata: w9sKdgYl/mK8ZiVE6JT2dLEBUWHNXf3tFtJXDkj42XYjT/ttIROTfMc+9RAn6h8FBwJF82aneubPW5ORfV9uFWqkU4F6jgCU0fB+t+T43n9zRxRUVHDoI4IoI9FELNjQHjcuXpX4OYpoVuu2MYIVVw== MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-Network-Message-Id: 7c679bdb-a47b-4eb9-dd84-08d7b9057c8f X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Feb 2020 08:42:30.9414 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: +Pyei3ExhqVP1PE8VzqwPFWpd8KUHl/TCezPg9do5/JlBLKJoUQnrQRccIzPIDkC/eFacnV+m38745ZxqSTLqw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB7PR04MB4329 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: Wu, Hao A > Sent: Monday, February 24, 2020 1:56 PM > To: Gaurav Jain ; devel@edk2.groups.io; Gao, Liming > ; afish@apple.com; lersek@redhat.com; > leif@nuviainc.com; Kinney, Michael D > Cc: Wang, Jian J ; Ni, Ray ; Ar= d > Biesheuvel ; Pankaj Bansal > > Subject: RE: [EXT] RE: [edk2-stable202002][edk2-devel] [PATCH v2 1/1] > MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test. >=20 > Caution: EXT Email >=20 > > -----Original Message----- > > From: Gaurav Jain [mailto:gaurav.jain@nxp.com] > > Sent: Monday, February 24, 2020 3:04 PM > > To: Wu, Hao A; devel@edk2.groups.io; Gao, Liming; afish@apple.com; > > lersek@redhat.com; leif@nuviainc.com; Kinney, Michael D > > Cc: Wang, Jian J; Ni, Ray; Ard Biesheuvel; Pankaj Bansal > > Subject: RE: [EXT] RE: [edk2-stable202002][edk2-devel] [PATCH v2 1/1] > > MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test. > > > > > > > I think the above check for 'Attributes' can be dropped. > > > I found that the implementation of the PciIoGetBarAttributes() > > > function > > does not > > > expose any configurable attributes. So the logic can fall through to > > > the > > ASSERT > > > (for DEBUG images) and then returns EFI_UNSUPPORTED. > > > > I agree that PciIoGetBarAttributes() function sets *Supports as 0. > > But In SCT Test for SetBarAttributes, there is a test case for > > Unsupported Attribute which expects EFI_UNSUPPORTED. If I drop this > > check, ASSERT will come, which is not expected. > > Can we keep check for 'Attributes'? >=20 >=20 > Oh, I forgot that. >=20 > I have one question, is there any special reason for you to pick the sup= ported > bits specified by: > EFI_PCI_DEVICE_ENABLE | EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE >=20 > Is it relating with the SCT test case? In PciIoAttributes() function, I can see the code #define DEV_SUPPORTED_ATTRIBUTES \ (EFI_PCI_DEVICE_ENABLE | EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE) So I used the same bits in PciIoSetBarAttributes() to have a check for val= id attributes. In SCT Test code First get the Bar attributes and set one of Unsupported attribute bit. Call PciIoSetBarAttributes() with Unsupported attribute and in return, tes= t expects EFI_UNSUPPORTED. Regards Gaurav Jain >=20 > Best Regards, > Hao Wu >=20 >=20 > > > > > -----Original Message----- > > > From: Wu, Hao A > > > Sent: Friday, February 21, 2020 6:53 AM > > > To: devel@edk2.groups.io; Gaurav Jain ; Gao, > > Liming > > > ; afish@apple.com; lersek@redhat.com; > > > leif@nuviainc.com; Kinney, Michael D > > > Cc: Wang, Jian J ; Ni, Ray > > > ; Ard Biesheuvel ; > > > Pankaj Bansal > > > Subject: [EXT] RE: [edk2-stable202002][edk2-devel] [PATCH v2 1/1] > > > MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test. > > > > > > Caution: EXT Email > > > > > > A couple of inline comments below. Please help to handle them in the > > > next version of patch. > > > With them addressed, > > > Reviewed-by: Hao A Wu > > > > > > > > > Hello Liming and Stewards, > > > > > > I would like to confirm with you for whether the patch should catch > > > the upcoming stable tag. > > > > > > My personal take is that the patch is more like a code refinement > > > rather > > than a > > > bug fix. > > > > > > Could you help to make a final call for this one? Thanks in advance. > > > > > > Best Regards, > > > Hao Wu > > > > > > > > > > -----Original Message----- > > > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf > > Of > > > > Gaurav Jain > > > > Sent: Thursday, February 20, 2020 11:40 PM > > > > To: devel@edk2.groups.io > > > > Cc: Wang, Jian J; Wu, Hao A; Ni, Ray; Ard Biesheuvel; Pankaj > > > > Bansal; Gaurav Jain > > > > Subject: [edk2-devel] [PATCH v2 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: > > > > 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 | 180 +++++++++++++= +++++ > > > > 1 file changed, 180 insertions(+) > > > > > > > > diff --git > > > > > > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP > > > > ciDeviceIo.c > > > > > > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP > > > > ciDeviceIo.c > > > > index 2d55c9699322..4dd804356021 100644 > > > > --- > > > > > > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP > > > > ciDeviceIo.c > > > > +++ > > > > > > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP > > > > ciDeviceIo.c > > > > @@ -93,6 +93,35 @@ 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 (BarIndex >=3D PCI_MAX_BAR) { > > > > + return EFI_UNSUPPORTED; > > > > + } > > > > + > > > > + 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_ERRO= R > > > > + (Status)) { > > > > + return Status; > > > > + } > > > > + > > > > + if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) { > > > > + return EFI_UNSUPPORTED; > > > > + } > > > > + > > > > ASSERT (FALSE); > > > > return EFI_UNSUPPORTED; > > > > } > > > > @@ -126,6 +155,35 @@ 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 (BarIndex >=3D PCI_MAX_BAR) { > > > > + return EFI_UNSUPPORTED; > > > > + } > > > > + > > > > + 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_ERRO= R > > > > + (Status)) { > > > > + return Status; > > > > + } > > > > + > > > > + if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) { > > > > + return EFI_UNSUPPORTED; > > > > + } > > > > + > > > > ASSERT (FALSE); > > > > return EFI_UNSUPPORTED; > > > > } > > > > @@ -396,6 +454,33 @@ PciIoIoRead ( > > > > IN OUT VOID *Buffer > > > > ) > > > > { > > > > + NON_DISCOVERABLE_PCI_DEVICE *Dev; > > > > + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc; > > > > + EFI_STATUS Status; > > > > + > > > > + if ((UINT32)Width > EfiPciIoWidthUint64) { > > > > + return EFI_INVALID_PARAMETER; } > > > > > > > > > For PciIoIoRead(), I think enum values smaller than > > > EfiPciIoWidthMaximum > > are > > > all valid. The above check seems to strict. > > > > Will address this in v3. > > > > > > > > > > + > > > > + if (BarIndex >=3D PCI_MAX_BAR) { > > > > + return EFI_UNSUPPORTED; > > > > + } > > > > + > > > > + 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_ERRO= R > > > > + (Status)) { > > > > + return Status; > > > > + } > > > > + > > > > + if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) { > > > > + return EFI_UNSUPPORTED; > > > > + } > > > > + > > > > ASSERT (FALSE); > > > > return EFI_UNSUPPORTED; > > > > } > > > > @@ -425,6 +510,33 @@ PciIoIoWrite ( > > > > IN OUT VOID *Buffer > > > > ) > > > > { > > > > + NON_DISCOVERABLE_PCI_DEVICE *Dev; > > > > + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc; > > > > + EFI_STATUS Status; > > > > + > > > > + if ((UINT32)Width > EfiPciIoWidthUint64) { > > > > + return EFI_INVALID_PARAMETER; } > > > > > > > > > For PciIoIoWrite(), I think enum values smaller than > > > EfiPciIoWidthMaximum > > are > > > all valid. The above check seems to strict. > > > > Will address this in v3. > > > > > > > > > > + > > > > + if (BarIndex >=3D PCI_MAX_BAR) { > > > > + return EFI_UNSUPPORTED; > > > > + } > > > > + > > > > + 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_ERRO= R > > > > + (Status)) { > > > > + return Status; > > > > + } > > > > + > > > > + if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) { > > > > + return EFI_UNSUPPORTED; > > > > + } > > > > + > > > > ASSERT (FALSE); > > > > return EFI_UNSUPPORTED; > > > > } > > > > @@ -556,6 +668,40 @@ 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; } > > > > + > > > > + if (DestBarIndex >=3D PCI_MAX_BAR || > > > > + SrcBarIndex >=3D PCI_MAX_BAR) { > > > > + return EFI_UNSUPPORTED; > > > > + } > > > > + > > > > + 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; > > > > } > > > > @@ -1414,6 +1560,40 @@ 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; > > > > + > > > > + #define DEV_SUPPORTED_ATTRIBUTES \ > > > > + (EFI_PCI_DEVICE_ENABLE | > > > > EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE) > > > > + > > > > + if ((Attributes & (~DEV_SUPPORTED_ATTRIBUTES)) !=3D 0) { > > > > + return EFI_UNSUPPORTED; > > > > + } > > > > > > > > > I think the above check for 'Attributes' can be dropped. > > > I found that the implementation of the PciIoGetBarAttributes() > > > function > > does not > > > expose any configurable attributes. So the logic can fall through to > > > the > > ASSERT > > > (for DEBUG images) and then returns EFI_UNSUPPORTED. > > > > > > Best Regards, > > > HaoWu > > > > > > > > > > + > > > > + if (BarIndex >=3D PCI_MAX_BAR) { > > > > + 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; > > > > } > > > > -- > > > > 2.17.1 > > > > > > > > > > > >=20