Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <4661da30-7e0d-8cab-ec55-9f963df5a582@intel.com>
Date:   Wed, 26 Oct 2022 16:53:55 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.0
Subject: Re: [PATCH 04/14] platform/x86/intel/ifs: Remove image loading during
 init
Content-Language: en-US
To:     Sohil Mehta <sohil.mehta@intel.com>
CC:     <tglx@linutronix.de>, <mingo@redhat.com>, <bp@alien8.de>,
        <dave.hansen@linux.intel.com>, <x86@kernel.org>, <hpa@zytor.com>,
        <gregkh@linuxfoundation.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>, <markgross@kernel.org>,
        <hdegoede@redhat.com>
References: <20221021203413.1220137-1-jithu.joseph@intel.com>
 <20221021203413.1220137-5-jithu.joseph@intel.com>
 <ec2a2799-9db5-8810-7a41-f9f6ca1b360b@intel.com>
 <2fce55c1-20cb-39c1-52ac-d9b3e2247296@intel.com>
 <8cc37fe6-86f7-bc0d-8bec-e17b061795b8@intel.com>
From:   "Joseph, Jithu" <jithu.joseph@intel.com>
In-Reply-To: <8cc37fe6-86f7-bc0d-8bec-e17b061795b8@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZjhTYjVaUkRaamNwa1BLVllmdDBZY2ZGT0dSMzJ1aHBVd01uTFkrV1FlUEgv?=
 =?utf-8?B?cnNqTkxrSXJKMDhYNkdFZkIvNXBQbGZQWEFZTm1sWlVQOTZHaEk5Z3ovbG9h?=
 =?utf-8?B?UnYxK3paN1g3aU9ZdUlNQ2xiK2hvVklpUjB0bDBudjRVSk91RGFNY2ErbUdx?=
 =?utf-8?B?VG94VWRnQXVqZFZWWlRRb1RjUExUdUhKNWFqQ3ZjVEZRR1REcW1Da1RXV2k2?=
 =?utf-8?B?ZEcrTGlRY0lBbFFPYU0yaEZoNWxieGlpa2VqVStGWEJuaVlBaFVCaGRKN3Bx?=
 =?utf-8?B?Zk5aUFYzL0tma21XMFNNMHNJaFhQL2hUK05rU0p5a2d5Q3Jac05nT3AxYVhI?=
 =?utf-8?B?QVY1TkVURzlSdUVEcm5MSzUxRFNSTEUyQ1RZemVYdWwxOHgyM1YzcmdVV0ZK?=
 =?utf-8?B?aDBNR3BtQVNKbDdWbWFoelV6d0hsb214ZlZ3TklEUitIYmEzWElaMVNlM251?=
 =?utf-8?B?UTBhblRDTjFvRnJCeUVxUVFJYUFlbjRUL0hqME03VU9ubHoxa3VUUXhUWmxM?=
 =?utf-8?B?a0drTmZhUzdheVN6WHh5Nlg4LzVVcWVSWU5lS3FpdHArN3g0MEFHQ0F3bjBw?=
 =?utf-8?B?c1RGTkQ5d1cyTVFmVjBrekRXYlF4bXI2YU91RDRVT0lOZ0sxSTRYM1pkNk5Q?=
 =?utf-8?B?VE9OWThtTG1qS01oVWpFTjBhN1NKdndPR2hNSXllemlUdk1LT2NGYU9qTTdB?=
 =?utf-8?B?a2ZTc2NnYVFLYmYrZ3dIYy9JamZWTTFmTkM5ai9JT2x1QnlsK1RPUWtSRGNG?=
 =?utf-8?B?bnIxTVpQbHY0K082Vk5TbXpOVmlJV3VGeGlWcDlrNEVTc29aT3NmR2c4TThN?=
 =?utf-8?B?SllxQXZPZ3RpZGZGU3ZpdDFVVFhDVlprWWx6UTZoQVN1eG1JblViN2JQZlUv?=
 =?utf-8?B?UVp1ZDB1bkZMdnUzVG5qWVZoaGFFSEdPQVgyTDB4QUtXSDNUQzNmczE1RzRV?=
 =?utf-8?B?ZmdKQTN1VE92MEdFT3ZVYmN4alo4RHdJcGNtUlZUWW4zTzJTRGt3dnRyZFll?=
 =?utf-8?B?VkRWUEJQdlVzdVBnLzNIU2FsZXdUYjAzdXJqcVV0d3BEeVVmUmt2SUVrV0lZ?=
 =?utf-8?B?cldpbHJzWmYxTVg4YTVIZzFZSkVxcGJBRGlWL2laMEVIbkhpcldIL0xHbldD?=
 =?utf-8?B?SEI3NHFZYlJUbTJDRVN2azhhc2tmVlI5QnZmNXlJWHpiVFR5MUJFWG9zRmlS?=
 =?utf-8?B?VkdoSU14RUJTZ2VIQ0xETjhLUnBrd1pQTEZweXFwTWpiYmVPbEVQMjNtZW8z?=
 =?utf-8?B?R1BEZjJjQWJ2YnhjUUcvc3BnSThYdUpTdkJPR3NJdWd1WjlHekhETkhuakpQ?=
 =?utf-8?B?OERzNlkycERXamZwalowUDJ2eVVtam12TVRoWDlRdmpzTkpvT2VJbUcxQ1lY?=
 =?utf-8?B?QkQvK0FpZ250Qzd6cDg2VzQ1NCtmVm44SVYvVCtDOXIyZkhTN1pkVHlJbVZN?=
 =?utf-8?B?SU9oUHQwYS92dkFhTzdTNHpJQVppbEZGNHdVN2RzcWNlS015UW0yZHZYNStj?=
 =?utf-8?B?blVkd2orOXBlV1FOeWdpMCt1cDIzKzBtOXUxeldadjdYam4xSEdYQUJyQUIw?=
 =?utf-8?B?NWZZMkpuSC9wTDlPWmFSZ2FpTDVpWDJXZ2VrdlYzdTdpRmxzWGNvVDBPeVQ5?=
 =?utf-8?B?WVFWV0ZzVjNpQ0ZVNTVURmJQclN3bXEycWFMWFdCREFleVZOK1IvaGhJQi9x?=
 =?utf-8?B?OG1IOG5JYWFVSHdVOEJhcWg0dkFsanJmeG41a2szVWlvU1lNSlhmWE1EU21I?=
 =?utf-8?B?bkxyeEJtbnJrS0h0NjYwZTZSaTlCNVYxaUN3aVp2ZHkzSEFtL3JKd09SS1lm?=
 =?utf-8?B?TTJqWUlYaFZTN2E2Y01nd3ZKc2QzdDBMSm1GWnRPTnNYdUdBZzBSdXlWQkcx?=
 =?utf-8?B?NDZBTVYvcDdSNlVqTTR0SEIxby95Qi9ETGRtODN1eWVrWGhQdGRFbGRabDZY?=
 =?utf-8?B?R1gzQ0M2aUkrZW1ReTdiYjloUUY4ekUvNzJHQXNRNHNMQWtnNTViSXRYT0ZF?=
 =?utf-8?B?SThtQ3RQS3BhdVZtanZCL3lBZGhwRVVVMWlVSVpZaW5Salk4YTZUeUw4S0NR?=
 =?utf-8?B?UlRQTnBtUHI1dDd2ZVpkUlVBWGszcjNDU1VNRDM1MGZFWkRJU1YzZ3hucnhw?=
 =?utf-8?B?ZGFlK0NmcHMrMFNxSmJNTmNJUXlWOGNqTEhGbUhBMXB1andWOE1Ud2RuYU5u?=
 =?utf-8?B?Ync9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 9aaa8ce6-60d6-48e5-5af9-08dab7ad58cc
X-MS-Exchange-CrossTenant-AuthSource: BYAPR11MB3768.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Oct 2022 23:53:57.4609
 (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: AQgnc2p7Qko96saJIvDvd658K8JYL6XHjZPDyzGZIzQK/pohSxOYABIQnhrnYvoIpu+MlAJPzCbmk8AhgR5XHQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB6721
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_NONE,URIBL_BLOCKED autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 10/24/2022 11:06 PM, Sohil Mehta wrote:
> On 10/24/2022 5:41 PM, Joseph, Jithu wrote:
>>>> diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
>>>> index 27204e3d674d..5fb7f655c291 100644
>>>> --- a/drivers/platform/x86/intel/ifs/core.c
>>>> +++ b/drivers/platform/x86/intel/ifs/core.c
>>>> @@ -51,12 +51,8 @@ static int __init ifs_init(void)
>>>>        ifs_device.misc.groups = ifs_get_groups();
>>>>          if ((msrval & BIT(ifs_device.data.integrity_cap_bit)) &&
>>>
>>> Is there a reason to store the integrity cap constant in the device.data global structure?
>>>
>>> .data = {
>>>      .integrity_cap_bit = MSR_INTEGRITY_CAPS_PERIODIC_BIST_BIT,
>>> },
>>
>> This was originally added so that, when future additional tests are supported by the driver, support can be checked using  the same if ((msrval & BIT(ifs_device.data.integrity_cap_bit)
>> Different tests would have different integrity_cap_bit assigned in the ifs_device[] array  (today it is just a single element and not an array
>>
>> Note that the current series doesn't introduce this
>>
>>
> 
> Sorry, I am still not able to follow.
> 
> Let's say you have an ifs_device[] array which has different integrity capabilities, there would still need to be some logic in the init code to differentiate between the resulting action that needs to be taken? Currently, the init function only registers the device. Maybe some example/code might be helpful to drive the point.

multiple devices will be created if support for more than one is advertised by MSR_INTEGRITY_CAPS as shown below

static int __init ifs_init(void)
{

 ....
	if (rdmsrl_safe(MSR_INTEGRITY_CAPS, &msrval))
		return -ENODEV;

	for (i = 0; i < IFS_NUMTESTS; i++) {
		if (!(msrval & BIT(ifs_devices[i].data.integrity_cap_bit)))
			continue;

		ifs_devices[i].misc.groups = ifs_get_groups();
		if (!misc_register(&ifs_devices[i].misc)) {
			ndevices++;
		}
	}

  return ndevices ? 0 : -ENODEV;
}

At the handler side we can branch to the corresponding handler by looking at this bit
Say for e.g the following function in runtest.c could be extended to something like

int do_core_test(int cpu, struct device *dev)
{
	struct ifs_data *ifsd = ifs_get_data(dev);

  .....
	
	switch (ifsd->integrity_cap_bit) {
	case MSR_INTEGRITY_CAPS_PERIODIC_BIST_BIT:
		ifs_test_core(cpu, dev);
		break;
	case MSR_INTEGRITY_CAPS_TEST2
		ifs_do_test2(cpu, dev);
		break;
	case MSR_INTEGRITY_CAPS_TEST3
		ifs_do_test3(cpu, dev);
		break;
	default:
		return -EINVAL;
	}

....
}


Jithu