From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by mx.groups.io with SMTP id smtpd.web11.6924.1604632783827881477 for ; Thu, 05 Nov 2020 19:19:43 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@intel.onmicrosoft.com header.s=selector2-intel-onmicrosoft-com header.b=CV+7a/qH; spf=pass (domain: intel.com, ip: 192.55.52.151, mailfrom: hao.a.wu@intel.com) IronPort-SDR: kjjXILhGI/2/ljVorDZlIhKWgZ1atf+XhE8iMIc/sj68i0P0yyrHYR6sMUC1ErVnSPSaP6Pigd y1TZRPdUcHJA== X-IronPort-AV: E=McAfee;i="6000,8403,9796"; a="149346862" X-IronPort-AV: E=Sophos;i="5.77,454,1596524400"; d="scan'208";a="149346862" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Nov 2020 19:19:43 -0800 IronPort-SDR: ibkti2senG5p/oIUE7PV4VLxArObYljZwWJaH+xsQEjqpdetNF5pnMBn406Ixdaz+Nz24Ae8wg E0vVC0ovHn8Q== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,454,1596524400"; d="scan'208";a="358665538" Received: from orsmsx606.amr.corp.intel.com ([10.22.229.19]) by fmsmga002.fm.intel.com with ESMTP; 05 Nov 2020 19:19:43 -0800 Received: from orsmsx601.amr.corp.intel.com (10.22.229.14) by ORSMSX606.amr.corp.intel.com (10.22.229.19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Thu, 5 Nov 2020 19:19:42 -0800 Received: from ORSEDG602.ED.cps.intel.com (10.7.248.7) by orsmsx601.amr.corp.intel.com (10.22.229.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5 via Frontend Transport; Thu, 5 Nov 2020 19:19:42 -0800 Received: from NAM11-CO1-obe.outbound.protection.outlook.com (104.47.56.176) by edgegateway.intel.com (134.134.137.103) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.1713.5; Thu, 5 Nov 2020 19:19:40 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=mhZpyq1gL+iEEzL+XyIrXC/7YOUVBu0Glgm2nc8+402ZDhhwf5+OinPvAKs8yau491IVElH3repdVpSs6bCfw8KsYGaiYxAnAbSi1oEkQhcYF4nPv0U7lwi65ytSDBAXg9FI7UwBG2SnCzBwPWU8DJkb3msQnI+2q8fFcAL1yZ3gET+lnmzjbcVktnAwzZ2J/fHXysghVrR15RzW08rkgnKuGTHUsBlT8TWCVy3u3PYFUgtBdZAtUEy2oX0Wv4Czah3l/RyVrW4M9ppHTPRlUgEqjsTICaoMsDzUjuixQRSoZecCXp9KKwe/dT7nbp2pi8HbV+sIvzDfR4LzNEx6AQ== 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=McBU7AcPvlOWQklj06PLX3jgeu6xyFWTA56LyU3rl94=; b=XsQuhxOLde4F1BBPxWTP5pfBAexJSgQEfLrD/LF7NxGA8m7RcJqc28/vLokG+Gl+C8kGgZr1W+aI0z2ru7he3b5khPlUbGEhxtzWkV6yZMqG5FzGtUeHMeDHw7Pkza5OdkiG3PRHX5krYhgJFYm3oRvL0xY29/YsVf/tGlJzKIsmfjrYexXMOJPkiRDwJypa5fisrWjah+nZrps8qK17H4JZH10dfRR2wZXYcn1sQqMOsZELayBufEH5w+Gs8DE+fPuTbP0tnBjfg7/vGjOnUzTHlMMgq2SFUkh2rhYA43j89zHee8x5C/mZJcCoHrF+xyZN9WjJVDJzWmDATV//0A== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=McBU7AcPvlOWQklj06PLX3jgeu6xyFWTA56LyU3rl94=; b=CV+7a/qHMaqOhXlnPzDEz5ByOVKv5Zt76x1H1QExygfOF2eyerSNmH5RS9/FckBe3is4i76oUgVywpjEPMIbbkojsd/fwFyHltXy9g23oRMum/ZSISXUJ9JxehlQPvfavOoCw58X1IIgNtrwAchhJSltZqNO74YscR+lhHBL3VQ= Received: from BN8PR11MB3666.namprd11.prod.outlook.com (2603:10b6:408:8c::19) by BN8PR11MB3620.namprd11.prod.outlook.com (2603:10b6:408:91::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3499.18; Fri, 6 Nov 2020 03:19:39 +0000 Received: from BN8PR11MB3666.namprd11.prod.outlook.com ([fe80::c123:faac:1da3:f807]) by BN8PR11MB3666.namprd11.prod.outlook.com ([fe80::c123:faac:1da3:f807%5]) with mapi id 15.20.3499.032; Fri, 6 Nov 2020 03:19:39 +0000 From: "Wu, Hao A" To: "Albecki, Mateusz" , "devel@edk2.groups.io" CC: "Ni, Ray" Subject: Re: [PATCHv3 4/4] MdeModulePkg/AtaAtapiPassThru: Trace ATA packets Thread-Topic: [PATCHv3 4/4] MdeModulePkg/AtaAtapiPassThru: Trace ATA packets Thread-Index: AQHWs3IXuSWHQCFHj0qITE57FYExSam6b2Jg Date: Fri, 6 Nov 2020 03:19:39 +0000 Message-ID: References: <20201105124847.3136-2-mateusz.albecki@intel.com> <20201105124847.3136-5-mateusz.albecki@intel.com> In-Reply-To: <20201105124847.3136-5-mateusz.albecki@intel.com> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.5.1.3 authentication-results: intel.com; dkim=none (message not signed) header.d=none;intel.com; dmarc=none action=none header.from=intel.com; x-originating-ip: [192.198.147.218] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: c192f67f-5900-4cd0-7b17-08d88202cbd2 x-ms-traffictypediagnostic: BN8PR11MB3620: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:8882; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: BfcglRDnfQ1NO5zmCQ9xXeATDglergf1yfKa21DSEDXkOR5sByBlt8wo6d7s2N69cQpAQTyj6wmqICCFV37jyZ6KByDl8r4tTmNNC7gOKK3DXvjuHu9mG4RzqqYiWzJBMwepMW50MB0Lx0ggxJwTJ/bz2h/9078zVHU8rwusV+ARrnmaGIVnGmVIgs23D0d4kzkNIoiXxX8tgWnSiXo9V+NQ6liS6g/cU+qwbfliZwaJPuwFs5nc0PEi6WJZ1ZJuxX1f1qwu3Yj4g28BaahCUTZecFeLM7WI+WD/uPN8fAGJdlMVcO0/r55w+vZcp1laYTlnsm+0RFfj3cCGNcdJpA== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BN8PR11MB3666.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(396003)(39860400002)(376002)(136003)(366004)(346002)(66556008)(66946007)(110136005)(66476007)(66446008)(5660300002)(186003)(64756008)(6506007)(26005)(478600001)(316002)(52536014)(53546011)(8676002)(9686003)(55016002)(71200400001)(33656002)(4326008)(7696005)(107886003)(86362001)(8936002)(76116006)(83380400001)(2906002);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata: OJmrey4nWRU5cssOAdd1D5PJMSD1fQ7muTerPGmcsfa6p3jkZshZmNrirbP0qepMVkRKwlsrAggyuPnC0Gm18O2Jw6J0QEWjwcXda+pT69v2t0LhY3lY5DYxDpFVtkE+PcnGAZm5x9KOxqRtKzZp0wLqnqj+zCH3l/1+Y/BUlWD2ew68quVyaB9FMEUlOmNUebByoYM8O7fWxs8od3syNCvdEPmAMV/0IgCIjUI0tC15nfaVaaD10jpy7M7SDeHj60Ai5lRQp1chjxj7ZdmDW6mnGSKJeAe1bW1R/07tiIs0m0xvUZ+Okg6BFnsyJUKgKTcw8gzLqC6WCFSL/zp/Xil0YXNcKJULjic1MhfyJFSMoAIIkYITafEMcLA61pRptggMpRIS5jcQjosMfOx/eTr9gN9GY+ybskem5w2FKTFYSkz8pwVz+lF9KZKV70uRpjUoBxvZu6ljmLtB3HLVyuYDNH+5nwYz4waZoElZ0qCrqU+GeBYoKstQMub8aT6jE9KpO5NSXG/fO+307Fna1ZhkMo6GUtmzMOi8wqcpKA50FwSp47j+JgHJKPuuOIyu2J0LBuJZYdRWblp0Qeg/8LABSieUN5pdVwY9Blxp3Yzzp40ZJhIP1HjJIgtYaBZAMZhneT+7Y2234QpppnzIDA== MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: BN8PR11MB3666.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: c192f67f-5900-4cd0-7b17-08d88202cbd2 X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Nov 2020 03:19:39.2720 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: YxZshYDyEsQzKKOgcQdwZxMWBl1chFtV1cKS6emB1htVetQOXwyAGqKFOXsIphqcPtAo8ijOjMr/go9gQPlz6A== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN8PR11MB3620 Return-Path: hao.a.wu@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: Albecki, Mateusz > Sent: Thursday, November 5, 2020 8:49 PM > To: devel@edk2.groups.io > Cc: Albecki, Mateusz ; Ni, Ray > ; Wu, Hao A > Subject: [PATCHv3 4/4] MdeModulePkg/AtaAtapiPassThru: Trace ATA > packets >=20 > This simplify ATA driver debugging all ATA packets will be printed to deb= ug > port on DEBUG_VERBOSE level along with the packet execution status. > Additionally failed packets and the failed packet execution status will b= e > printed on DEBUG_ERROR level. >=20 > Signed-off-by: Mateusz Albecki >=20 > Cc: Ray Ni > Cc: Hao A Wu > --- > .../Bus/Ata/AtaAtapiPassThru/AhciMode.c | 94 +++++++++++++++++++ > 1 file changed, 94 insertions(+) >=20 > diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c > b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c > index 47275a851a..e506c8f2d5 100644 > --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c > +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c > @@ -846,6 +846,54 @@ AhciWaitUntilFisReceived ( > return EFI_TIMEOUT; > } >=20 > +/** > + Prints contents of the ATA command block into the debug port. > + > + @param[in] AtaCommandBlock AtaCommandBlock to print. > + @param[in] DebugLevel Debug level on which to print. > +**/ > +VOID > +AhciPrintCommandBlock ( > + IN EFI_ATA_COMMAND_BLOCK *AtaCommandBlock, > + IN UINT32 DebugLevel > + ) > +{ > + DEBUG ((DebugLevel, "ATA COMMAND BLOCK:\n")); > + DEBUG ((DebugLevel, "AtaCommand: %d\n", > +AtaCommandBlock->AtaCommand)); > + DEBUG ((DebugLevel, "AtaFeatures: %X\n", > +AtaCommandBlock->AtaFeatures)); > + DEBUG ((DebugLevel, "AtaSectorNumber: %d\n", > +AtaCommandBlock->AtaSectorNumber)); > + DEBUG ((DebugLevel, "AtaCylinderLow: %X\n", > +AtaCommandBlock->AtaCylinderHigh)); > + DEBUG ((DebugLevel, "AtaCylinderHigh: %X\n", > +AtaCommandBlock->AtaCylinderHigh)); > + DEBUG ((DebugLevel, "AtaDeviceHead: %d\n", > +AtaCommandBlock->AtaDeviceHead)); > + DEBUG ((DebugLevel, "AtaSectorNumberExp: %d\n", > +AtaCommandBlock->AtaSectorNumberExp)); > + DEBUG ((DebugLevel, "AtaCylinderLowExp: %X\n", > +AtaCommandBlock->AtaCylinderLowExp)); > + DEBUG ((DebugLevel, "AtaCylinderHighExp: %X\n", > +AtaCommandBlock->AtaCylinderHighExp)); > + DEBUG ((DebugLevel, "AtaFeaturesExp: %X\n", > +AtaCommandBlock->AtaFeaturesExp)); > + DEBUG ((DebugLevel, "AtaSectorCount: %d\n", > +AtaCommandBlock->AtaSectorCount)); > + DEBUG ((DebugLevel, "AtaSectorCountExp: %d\n", > +AtaCommandBlock->AtaSectorCountExp)); > +} > + > +/** > + Prints contents of the ATA status block into the debug port. > + > + @param[in] AtaStatusBlock AtaStatusBlock to print. > + @param[in] DebugLevel Debug level on which to print. > +**/ > +VOID > +AhciPrintStatusBlock ( > + IN EFI_ATA_STATUS_BLOCK *AtaStatusBlock, > + IN UINT32 DebugLevel > + ) > +{ > + // > + // Only print status and error since we have all of the rest printed > +as > + // a part of command block print. > + // > + DEBUG ((DebugLevel, "ATA STATUS BLOCK:\n")); > + DEBUG ((DebugLevel, "AtaStatus: %d\n", AtaStatusBlock->AtaStatus)); > + DEBUG ((DebugLevel, "AtaError: %d\n", AtaStatusBlock->AtaError)); } > + > /** > Start a PIO data transfer on specific port. >=20 > @@ -947,6 +995,8 @@ AhciPioTransfer ( > DataCount > ); >=20 > + DEBUG ((DEBUG_VERBOSE, "Starting command for PIO transfer:\n")); > + AhciPrintCommandBlock (AtaCommandBlock, DEBUG_VERBOSE); > Status =3D AhciStartCommand ( > PciIo, > Port, > @@ -1000,6 +1050,19 @@ AhciPioTransfer ( > ); >=20 > AhciDumpPortStatus (PciIo, AhciRegisters, Port, AtaStatusBlock); > + > + if (Status =3D=3D EFI_DEVICE_ERROR) { > + DEBUG ((DEBUG_ERROR, "Failed to execute command: for PIO > transfer\n")); The patch is good to me: Reviewed-by: Hao A Wu I will move the ':' to the end of the above debug message if there is no additional comment from others for this series. Best Regards, Hao Wu > + // > + // Repeat command block here to make sure it is printed on > + // device error debug level. > + // > + AhciPrintCommandBlock (AtaCommandBlock, DEBUG_ERROR); > + AhciPrintStatusBlock (AtaStatusBlock, DEBUG_ERROR); } else { > + AhciPrintStatusBlock (AtaStatusBlock, DEBUG_VERBOSE); } > + > return Status; > } >=20 > @@ -1132,6 +1195,8 @@ AhciDmaTransfer ( > DataCount > ); >=20 > + DEBUG ((DEBUG_VERBOSE, "Starting command for sync DMA > transfer:\n")); > + AhciPrintCommandBlock (AtaCommandBlock, DEBUG_VERBOSE); > Status =3D AhciStartCommand ( > PciIo, > Port, > @@ -1168,6 +1233,8 @@ AhciDmaTransfer ( > DataCount > ); >=20 > + DEBUG ((DEBUG_VERBOSE, "Starting command for async DMA > transfer:\n")); > + AhciPrintCommandBlock (AtaCommandBlock, DEBUG_VERBOSE); > Status =3D AhciStartCommand ( > PciIo, > Port, > @@ -1238,6 +1305,19 @@ AhciDmaTransfer ( > } >=20 > AhciDumpPortStatus (PciIo, AhciRegisters, Port, AtaStatusBlock); > + > + if (Status =3D=3D EFI_DEVICE_ERROR) { > + DEBUG ((DEBUG_ERROR, "Failed to execute command for DMA > transfer:\n")); > + // > + // Repeat command block here to make sure it is printed on > + // device error debug level. > + // > + AhciPrintCommandBlock (AtaCommandBlock, DEBUG_ERROR); > + AhciPrintStatusBlock (AtaStatusBlock, DEBUG_ERROR); } else { > + AhciPrintStatusBlock (AtaStatusBlock, DEBUG_VERBOSE); } > + > return Status; > } >=20 > @@ -1307,6 +1387,8 @@ AhciNonDataTransfer ( > 0 > ); >=20 > + DEBUG ((DEBUG_VERBOSE, "Starting command for non data > transfer:\n")); > + AhciPrintCommandBlock (AtaCommandBlock, DEBUG_VERBOSE); > Status =3D AhciStartCommand ( > PciIo, > Port, > @@ -1343,6 +1425,18 @@ AhciNonDataTransfer ( >=20 > AhciDumpPortStatus (PciIo, AhciRegisters, Port, AtaStatusBlock); >=20 > + if (Status =3D=3D EFI_DEVICE_ERROR) { > + DEBUG ((DEBUG_ERROR, "Failed to execute command for non data > transfer:\n")); > + // > + // Repeat command block here to make sure it is printed on > + // device error debug level. > + // > + AhciPrintCommandBlock (AtaCommandBlock, DEBUG_ERROR); > + AhciPrintStatusBlock (AtaStatusBlock, DEBUG_ERROR); } else { > + AhciPrintStatusBlock (AtaStatusBlock, DEBUG_VERBOSE); } > + > return Status; > } >=20 > -- > 2.28.0.windows.1