From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.120; helo=mga04.intel.com; envelope-from=star.zeng@intel.com; receiver=edk2-devel@lists.01.org Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 114E72034D808 for ; Tue, 7 Nov 2017 00:40:01 -0800 (PST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Nov 2017 00:44:00 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,357,1505804400"; d="scan'208";a="1240722832" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga002.fm.intel.com with ESMTP; 07 Nov 2017 00:43:36 -0800 Received: from fmsmsx152.amr.corp.intel.com (10.18.125.5) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 7 Nov 2017 00:43:35 -0800 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by FMSMSX152.amr.corp.intel.com (10.18.125.5) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 7 Nov 2017 00:43:35 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.175]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.93]) with mapi id 14.03.0319.002; Tue, 7 Nov 2017 16:43:33 +0800 From: "Zeng, Star" To: "Ni, Ruiyu" , "edk2-devel@lists.01.org" CC: Laszlo Ersek , "Zeng, Star" Thread-Topic: [PATCH] PcAtChipsetPkg/IsaAcpiDxe: Restore PCI attributes correctly Thread-Index: AQHTVH3HhfQcOx/FikW6VTVvTCBboqMIn0dg Date: Tue, 7 Nov 2017 08:43:32 +0000 Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103B9B2853@shsmsx102.ccr.corp.intel.com> References: <20171103082836.125696-1-ruiyu.ni@intel.com> In-Reply-To: <20171103082836.125696-1-ruiyu.ni@intel.com> 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 Subject: Re: [PATCH] PcAtChipsetPkg/IsaAcpiDxe: Restore PCI attributes correctly X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 07 Nov 2017 08:40:02 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Some minor comments. 1. How about update structure field name 'OriginalAttribute' to be 'Origina= lAttributes'? 2. The comments below in Stop() need to be updated accordingly. // // Get supported PCI attributes // 3. Add the bugzilla link to the commit log. With that fixed, Reviewed-by: Star Zeng Thanks, Star -----Original Message----- From: Ni, Ruiyu=20 Sent: Friday, November 3, 2017 4:29 PM To: edk2-devel@lists.01.org Cc: Zeng, Star ; Laszlo Ersek Subject: [PATCH] PcAtChipsetPkg/IsaAcpiDxe: Restore PCI attributes correctl= y The original code enables some BITs in PCI attributes in Start(), but wrong= ly to disable these BITs in Stop(). The correct behavior is to save the original PCI attributes before enables = some BITs in Start(), and restore to original value in Stop(). Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni Cc: Star Zeng Cc: Laszlo Ersek --- PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.c | 44 +++++++++++++++++------------= ---- PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.h | 3 ++- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.c b/PcAtChipsetPkg/IsaAc= piDxe/PcatIsaAcpi.c index 32381b112d..60d2fb5a5b 100644 --- a/PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.c +++ b/PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.c @@ -172,6 +172,7 @@ PcatIsaAcpiDriverBindingStart ( EFI_PCI_IO_PROTOCOL *PciIo; PCAT_ISA_ACPI_DEV *PcatIsaAcpiDev; UINT64 Supports; + UINT64 OriginalAttributes; BOOLEAN Enabled; =20 Enabled =3D FALSE; @@ -210,9 +211,18 @@ PcatIsaAcpiDriverBindingStart ( if (Supports =3D=3D 0 || Supports =3D=3D (EFI_PCI_IO_ATTRIBUTE_ISA_IO | = EFI_PCI_IO_ATTRIBUTE_ISA_IO_16)) { Status =3D EFI_UNSUPPORTED; goto Done; - } =20 + } + + Status =3D PciIo->Attributes ( + PciIo, + EfiPciIoAttributeOperationGet, + 0, + &OriginalAttributes + ); + if (EFI_ERROR (Status)) { + goto Done; + } =20 - Enabled =3D TRUE; Status =3D PciIo->Attributes ( PciIo,=20 EfiPciIoAttributeOperationEnable, @@ -222,7 +232,8 @@ = PcatIsaAcpiDriverBindingStart ( if (EFI_ERROR (Status)) { goto Done; } - =20 + + Enabled =3D TRUE; // // Allocate memory for the PCAT ISA ACPI Device structure // @@ -239,9 +250,10 @@ PcatIsaAcpiDriverBindingStart ( // // Initialize the PCAT ISA ACPI Device structure // - PcatIsaAcpiDev->Signature =3D PCAT_ISA_ACPI_DEV_SIGNATURE; - PcatIsaAcpiDev->Handle =3D Controller; - PcatIsaAcpiDev->PciIo =3D PciIo; + PcatIsaAcpiDev->Signature =3D PCAT_ISA_ACPI_DEV_SIGNATURE; + PcatIsaAcpiDev->Handle =3D Controller; + PcatIsaAcpiDev->PciIo =3D PciIo; + PcatIsaAcpiDev->OriginalAttribute =3D OriginalAttributes; =20 // // Initialize PcatIsaAcpiDeviceList @@ -274,8 +286,8 @@ Done: if (PciIo !=3D NULL && Enabled) { PciIo->Attributes ( PciIo,=20 - EfiPciIoAttributeOperationDisable,=20 - EFI_PCI_DEVICE_ENABLE | Supports | EFI_PCI_IO_ATTRIBUTE_ISA= _MOTHERBOARD_IO, + EfiPciIoAttributeOperationSet,=20 + OriginalAttributes, NULL=20 ); } @@ -321,7 +333,6 @@ PcatIsaAcpiDriverBindingStop ( EFI_STATUS Status; EFI_ISA_ACPI_PROTOCOL *IsaAcpi; PCAT_ISA_ACPI_DEV *PcatIsaAcpiDev; - UINT64 Supports; =20 // // Get the ISA ACPI Protocol Interface @@ -348,23 +359,14 @@ PcatIsaAcpi= DriverBindingStop ( // Status =3D PcatIsaAcpiDev->PciIo->Attributes ( PcatIsaAcpiDev->PciIo, - EfiPciIoAttributeOperationSupported, - 0, - &Supports + EfiPciIoAttributeOperationSet, + PcatIsaAcpiDev->OriginalAttribute, + 0 ); if (EFI_ERROR (Status)) { return Status; } =20 - Supports &=3D (UINT64) (EFI_PCI_IO_ATTRIBUTE_ISA_IO | EFI_PCI_IO_ATTRIBU= TE_ISA_IO_16); - - PcatIsaAcpiDev->PciIo->Attributes ( - PcatIsaAcpiDev->PciIo,=20 - EfiPciIoAttributeOperationDisable,=20 - EFI_PCI_DEVICE_ENABLE | Supports | EFI_PCI_IO_A= TTRIBUTE_ISA_MOTHERBOARD_IO, - NULL=20 - ); -=20 // // Uninstall protocol interface: EFI_ISA_ACPI_PROTOCOL // diff --git a/PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.h b/PcAtChipsetPkg/IsaAc= piDxe/PcatIsaAcpi.h index 0671127644..3ad3a3f313 100644 --- a/PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.h +++ b/PcAtChipsetPkg/IsaAcpiDxe/PcatIsaAcpi.h @@ -1,7 +1,7 @@ /** @file EFI PCAT ISA ACPI Driver for a Generic PC Platform =20 -Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
This program and the accompanying materials =20 are licensed and made available under the terms and conditions of the BSD = License =20 which accompanies this distribution. The full text of the license may be = found at =20 @@ -43,6 +43,7 @@ typedef struct { EFI_HANDLE Handle; =20 EFI_ISA_ACPI_PROTOCOL IsaAcpi; EFI_PCI_IO_PROTOCOL *PciIo; + UINT64 OriginalAttribute; } PCAT_ISA_ACPI_DEV; =20 #define PCAT_ISA_ACPI_DEV_FROM_THIS(a) BASE_CR(a, PCAT_ISA_ACPI_DEV, IsaAc= pi) -- 2.12.2.windows.2