Message-ID: <42ac85ea-0ffe-55fc-57c2-6f4555e8dc5b@intel.com>
Date:   Wed, 22 Feb 2023 14:07:35 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH v2 4/7] platform/x86/intel/ifs: Implement Array BIST test
Content-Language: en-US
To:     Dave Hansen <dave.hansen@intel.com>, <hdegoede@redhat.com>,
        <markgross@kernel.org>
CC:     <tglx@linutronix.de>, <mingo@redhat.com>, <bp@alien8.de>,
        <dave.hansen@linux.intel.com>, <x86@kernel.org>, <hpa@zytor.com>,
        <gregkh@linuxfoundation.org>, <rostedt@goodmis.org>,
        <ashok.raj@intel.com>, <tony.luck@intel.com>,
        <linux-kernel@vger.kernel.org>,
        <platform-driver-x86@vger.kernel.org>, <patches@lists.linux.dev>,
        <ravi.v.shankar@intel.com>, <thiago.macieira@intel.com>,
        <athenas.jimenez.gonzalez@intel.com>, <sohil.mehta@intel.com>
References: <20230131234302.3997223-1-jithu.joseph@intel.com>
 <20230214234426.344960-1-jithu.joseph@intel.com>
 <20230214234426.344960-5-jithu.joseph@intel.com>
 <a24c65f8-978d-8968-7874-6b83e14b01ba@intel.com>
 <d9d18954-8434-4c85-88b8-8e1d99cd6a4b@intel.com>
 <dd501d6a-e7e9-be82-24fb-33b18fb6192b@intel.com>
 <b67d2125-4075-4eac-4cad-8e315b866144@intel.com>
 <5d57f42a-1bfc-5e6a-bb49-556ecb7402af@intel.com>
 <9c55a34b-c17e-2a62-9989-902ce940dde5@intel.com>
From:   "Joseph, Jithu" <jithu.joseph@intel.com>
In-Reply-To: <9c55a34b-c17e-2a62-9989-902ce940dde5@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?UTNYdDdsY2FVVnZ4N0NSSnVPQUMzV1dxTmhudzNyL2pVRzgvVlFYOGI4Tzhh?=
 =?utf-8?B?UExta0JMdDRhV3JMRWVGQnQ5SG11bFFnZEZleVk2cW1Sc0RZNk5oU05BRHdC?=
 =?utf-8?B?QWZNaENRUDA4RjJmMTNqR0tHamlTS1hUZFhTRmNFaWJIRnM1cldGTzJaUFFH?=
 =?utf-8?B?c3RVSlJ1TWV6L3FMdlVmNkNBaS9zMWY3WGdPMTV0UDNGbWtzOU5XUVBYeWJj?=
 =?utf-8?B?a0svd1gvcW4wR280TWdjODIrQjdoNU1BemdBS09MTUZ1YWNWZm52bTV1S3ZV?=
 =?utf-8?B?OUdEbXNLcGQzM0lOZSthZFFvMklyaFJZalZDcW91dithaU5leXc2U2h2U1Rw?=
 =?utf-8?B?OGJXWmdkaG85Qk1LdWUwWFdWMXl6UHF2TXExY1RZK0lBYUIxZHFlTkRjZnBu?=
 =?utf-8?B?RHczMnM1RjF5cXNyQjUyRzNNbHl3VU1BK3J4bGlQMHVhWkhDdzZmT1FzSkxU?=
 =?utf-8?B?clhpSkpXUmtoSm0zeW1zb1A0cHFJQUVmS0oyNkNMelE4WXE2aDczbHZ5cjMy?=
 =?utf-8?B?OGJaVjFsSnRUaHovWUtkeTNqaEFMWGltSGhjdVZUbzR1bDB2bUwvY09rTjJK?=
 =?utf-8?B?cVRsc3pSZVdsZjdKTGJsdGJMNURsK2pTRFZEYlJMQW9maGpmVWpaeFJnWTI5?=
 =?utf-8?B?SVRJejlMbFdHVS9SQU92T2d5Sjl1NmgwN2htTENpcm9aaTk0Q0dscUJiRjR5?=
 =?utf-8?B?bGdmemhQSEdDSVVIeWl5RW9uOTZPRzUwb1lQbTN6SWtCZUpqS2JHM0ljWGxW?=
 =?utf-8?B?YlpwSVMxMVhuOHNNN3E0KzNneGh5dlRkRTdRa2tBb01kR0xicVM3YVk3THho?=
 =?utf-8?B?aEpQOE9ONlJoV000UDF5WHU3bGZNRmRzUHczcjhXWU9nQ1VjZkhlVmdZL0Z5?=
 =?utf-8?B?NS93NWk0WTROSGpMdDhDZFdGenJvOU1IUDFGVzdBU1hwdE1EcEIyRWZDMEl1?=
 =?utf-8?B?WHBIUUpPdHN0a3hFS1RaUTJPNFRockhLUDNTMHlHa3krdWRHbVh0N09EK1RT?=
 =?utf-8?B?bU5qMURvaXk0SDVqc1FGUG9VNm51d0tOazZ1Q2ZZWGsyYlk0NnhIc2ZZalg3?=
 =?utf-8?B?SkFXaXRFRUNyNExSMy9ROGF0SVB6YlA1eEF6RUdPNnJHRFE1eWdPRFMzcklh?=
 =?utf-8?B?VEpLTWdvUklJZ2JZOHp5c0VkekxjaHd3dDU4bktEKzdMN0lscktxTDlyNEhy?=
 =?utf-8?B?Q1Qrdklkc0pEM1FSTGQrS2doRzM3VEdodG4wYU8wNDkyYUllY1YwYUc5c3lS?=
 =?utf-8?B?MjFwNzJ4cy9QTmQrN3YvcSt5cXdaRzJQNStJYlh4L3VidDBienlQWFZ3d3pz?=
 =?utf-8?B?bE5PSVc3L0Fta2laWGVWSzRKWkR1c091cDRoRDFpa2svNXVKcGZPUEtqZzNl?=
 =?utf-8?B?WFNabE50cUdITjhQMU96WjRrcHFhdkRxY2UxUHB3OExodEpIVmJJOWRYUDJR?=
 =?utf-8?B?RmVYc2NxV3NtR20yQ0tacFBWY0lJcTNMMmJtb1BuNzIyUnVOYjdhSnl1RVpV?=
 =?utf-8?B?TjZJd2VTeVRRc0VxRUF3YkRvd0F5c0JZbjRsbDUrYWlOQ3lwSDl4ZDhsUFJX?=
 =?utf-8?B?V293ZU1HWXFWY3d0alFRL2ZXZUgvVTFYSFJSRU9YaS9pTzY1MXZGU2NXbEhG?=
 =?utf-8?B?MWMwTFU1blJ2YnhtRkF6S1pwUHlQRGN0aCs3VGh0ejMyVEVnbmpEUlRkcmZ4?=
 =?utf-8?B?TTRKZ2FxQ3p2ZjY4MW5VeVVrVGc5Q09ZLzhmTFR6Y21DVFVvSGNBRzFxRHN0?=
 =?utf-8?B?SzVaTHNkcnE2dDcxWVhDa1FYckJuNjREUEZsQ2NoblVtZS82VGZvUTB1aDg2?=
 =?utf-8?B?TEMxS3JvakJuVTYvWTgweGMxUUdvR09HOWdPc3dqRUphUCtRcWc3NTgvRHlQ?=
 =?utf-8?B?MDRhNkZpVStTOFdMSU56a2pXaDhpS3VNNHdKOXplSk5TandMWW5xTE5FaWFY?=
 =?utf-8?B?c1JFUGtNMko4eEJBMm5qd1NVTUVHQy9WYXV5aUZpamZ5aEpDd0RqU1VuZ0Zy?=
 =?utf-8?B?VDIzcklqUzczaEJRZmpNamVwa1ZGOUp5NDhPcm9OMk14WE5ncHVnUkhHa2to?=
 =?utf-8?B?UzZ2djNzUVBDVzlqQXdWeHh4QnMxOUFCekx6MTc4Nk5CdURrbEZMSFdBU2NL?=
 =?utf-8?B?c2xrZzkyanN5azgyNFVQMHVvVHJGRVJBT2lNVnkwOTJSS201bUovaDZVVGsr?=
 =?utf-8?B?U2c9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 465838f2-a779-476c-e031-08db15213575
X-MS-Exchange-CrossTenant-AuthSource: BYAPR11MB3768.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Feb 2023 22:07:38.0734
 (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: SC5IUVs82fPDMTIhLxyxohGkBw8clkrbwReiYFqNB7YrDeESroWJtjmqX3+jKVe+bQq+cWoJ9eH7HrfB5IxFDA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR11MB4601
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 2/22/2023 12:12 PM, Dave Hansen wrote:
> 
> I actually really think this tracing should go away, *ESPECIALLY* if you
> insist on dumping out raw, non-human-readable MSR values.
> 
> I say NAK on the tracing.  You haven't convinced me that it's necessary
> on top of what we have today (MSR tracing).


Thanks for the pointer on MSR trace. I agree that setting the appropriate filter ("msr == 0x105") would generate the read and write values (snip2 below).

The primary value addition for the custom trace added by this driver was readability / convenience (snip1 below).  The format it is dumping today is most convenient for tracking  the progress of this test  from start to completion. (Two formatted input fields (wrmsr) and one output field (rdmsr) on the same line ).
Another convenience is that, by enabling the high-level intel_ifs trace,  we can see all  activity related to all IFS tests (and not having to explicitly remember and enable the MSRs corresponding to each of the IFS tests)

Since the trace has to be explicitly enabled (I thought it is okay to add a more convenient custom one only to be enabled for detailed analysis when required).

<Snip 1 added by driver>
     bash-9411    [123] ..... 423555.355664: ifs_array: cpu: 10, array_list: ffffffff, array_bank: 0000, status: 0000010000007ff2
     bash-9411    [123] ..... 423555.355858: ifs_array: cpu: 10, array_list: 00007ff2, array_bank: 0100, status: 0000000000007fe0
     bash-9411    [123] ..... 423555.355891: ifs_array: cpu: 10, array_list: 00007fe0, array_bank: 0000, status: 0000010000007fe0
     bash-9411    [123] ..... 423555.355923: ifs_array: cpu: 10, array_list: 00007fe0, array_bank: 0100, status: 0000000100007fe0
</snip1>


<snip2 using msr trace>
   migration/10-76      [010] d..1. 423672.955522: write_msr: 105, value ffffffff
   migration/10-76      [010] d..1. 423672.955525: read_msr: 105, value 10000007ff2
   migration/10-76      [010] d..1. 423672.955733: write_msr: 105, value 10000007ff2
   migration/10-76      [010] d..1. 423672.955736: read_msr: 105, value 7fe0
   migration/10-76      [010] d..1. 423672.955783: write_msr: 105, value 7fe0
   migration/10-76      [010] d..1. 423672.955786: read_msr: 105, value 10000007fe0
   migration/10-76      [010] d..1. 423672.955819: write_msr: 105, value 10000007fe0
   migration/10-76      [010] d..1. 423672.955822: read_msr: 105, value 100007fe0
</snip2 msr trace>


Jithu