Received-SPF: pass (google.com: domain of linux-kernel+bounces-162374-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <f6af3a35-7ddd-7f56-d7d9-34c77647614d@amd.com>
Date: Mon, 29 Apr 2024 08:35:39 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Content-Language: en-US
To: Dan Williams <dan.j.williams@intel.com>, linux-kernel@vger.kernel.org,
 x86@kernel.org, linux-coco@lists.linux.dev, svsm-devel@coconut-svsm.dev
Cc: Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>,
 Borislav Petkov <bp@alien8.de>, Dave Hansen <dave.hansen@linux.intel.com>,
 "H. Peter Anvin" <hpa@zytor.com>, Andy Lutomirski <luto@kernel.org>,
 Peter Zijlstra <peterz@infradead.org>, Michael Roth <michael.roth@amd.com>,
 Ashish Kalra <ashish.kalra@amd.com>
References: <cover.1713974291.git.thomas.lendacky@amd.com>
 <e3143c1f3d0b26fcd8884c6f75644b634a7138e8.1713974291.git.thomas.lendacky@amd.com>
 <662c239680842_b6e02944b@dwillia2-mobl3.amr.corp.intel.com.notmuch>
From: Tom Lendacky <thomas.lendacky@amd.com>
Subject: Re: [PATCH v4 13/15] x86/sev: Take advantage of configfs visibility
 support in TSM
In-Reply-To: <662c239680842_b6e02944b@dwillia2-mobl3.amr.corp.intel.com.notmuch>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?UWFVejgrV2NoVmpmdW1vYk9HZ25uTkxDVGR0dWY0cDZBNnozK0tPdHRxdTNL?=
 =?utf-8?B?MGVGeFVwQW51OHkyeDFUZDRsQmgxejJOWUMxcjdwSElwMENnUjRtQ3FvY00v?=
 =?utf-8?B?cEx2M0dLb250S1VicC9nWjFxZmVNSXhEeWV3blRiZjBqYXR4aExXOEdkNGF3?=
 =?utf-8?B?MEJGSUhiTWZpRlZxMDQ0WHlLUEJCTjFhYXZYTTV2S0NJeUpPSTBJU01VK1JL?=
 =?utf-8?B?YW10dXh4UXo2S3RyUllDb282alBybE1qSFBiRFVWaCtwRGFsam9OL04wSzZM?=
 =?utf-8?B?MTVkVE4rVmc0VERFRi90MUtraS96bitrKzNaWEQ3SS9YQ2V3WW1jUmV5U0lL?=
 =?utf-8?B?bUsxVGZmMlY2T00yWnBCOUtVclBZTXgzWUM3aXJpSncvK2RBZ2Z2bXgza3Rj?=
 =?utf-8?B?MEdrMkJJeWovRDBEaDh3QW9HZGp0NDFBdFUrQlN0a1FGRTlKNHFDY3ZmcDlr?=
 =?utf-8?B?S3VHbTVVb3ltanVhdXpLVG01eXVRV1p1QnFUM3hLUDdETlBDdkhHWEs1TEVm?=
 =?utf-8?B?ckY1U1pudkYwS0JTKzNDbllna2Q1c3FDVkJ6QU0vbFlkeVgxUHhmb28xdmpR?=
 =?utf-8?B?akdsT3o3UWQ4TXUrSEVUbVRKWDJyMXIzY01zMCsxVGkxa0JtWk1JZjFZdzA3?=
 =?utf-8?B?ZFBaVHAzektUTEViZ093UXVSeS9LN2RlU1ZJdDViZFVNU3NIR3pMUk4xVTQx?=
 =?utf-8?B?R0JucUNrblRhbmtnVEQzSVFwSStZV1NoSWNGQklCb1MzdUFSVlBEdEFZek55?=
 =?utf-8?B?ZVJVNFhGRWg1SVE2VGxLaDdIZXRvSlRCbnN1cUc3Z1IyS1dSL3JpWEhhcmtk?=
 =?utf-8?B?UXJCK0RZYXhmVUNXVUN6TVNWTW1kSkk3RVNmRjFpckNsVmM1NVVub0NaNHlP?=
 =?utf-8?B?OVlFZVRZSjdIQnBhQlJCeWZqWTVTazhsU21IYVJObWl6cE5lVE5oSlZBN2Rv?=
 =?utf-8?B?NFZhU2VRbmxlY2krY3AvVVJ3U0ozQ2VhQlNjZXRuVDNKNHpPWXdCNFh4N0xR?=
 =?utf-8?B?akdPT0xGTWZvWDBuZjFBYUpuMUMzVGZ4ODZNVzlsQjdnQXMrdUlWeGNHcVpy?=
 =?utf-8?B?M2t3Qm5MVzZuanhuYk5SME9jWjBYaHcwdHllTzBZL1RrazBSVGRsV3hLRjN1?=
 =?utf-8?B?NUtHb29BbGxFenhOV0FrUGZKVXdHalNSelNiYURkWjZyMFN2aUszdjdBNDlH?=
 =?utf-8?B?akd6M0Fyd2o0TU42ejI3SjA5MHZXNkhKQTFtQVBpcVJuSzEwaFc2eTU3Qjha?=
 =?utf-8?B?R3MrNXFBeWlQejJMbTZTbEM3M0lQMUFQb1Y1bzNpWWo4MElLTWFEMnJzaWlu?=
 =?utf-8?B?WFhxNVVjSGhXVTBHWkpLZmc4UHJDcytUZFlxL2syaDJsYklIakJaNmJ1aUg1?=
 =?utf-8?B?OVA0TzNYRTAyUlNkSTg3RXczSzhCMjl6WnloSDAxMWQ1b0JnY2NNYzhPNWN6?=
 =?utf-8?B?MkFkQ002VE44SDFIVGlJWFFLZ0pBTDJHWnZKTldrVEZqVWxiVUgyVGZ2VFJO?=
 =?utf-8?B?MkZFdTc0QUhocVV0MXUzRldvYitab3lHQnR4bU92MkY5bk96cXFadk1aSlp1?=
 =?utf-8?B?eEpEQm9LVGl1L1lVMkdPQXROajJpYU1NNXBud0Z2akk1ZElXMVd2eEFqVUZv?=
 =?utf-8?B?WFhPdWdVUDVWcnlWK1E5TmhzMDRqQjFtMmFLMnA1NjRBSkZOYndGY3NXRUVj?=
 =?utf-8?B?UWNSY3RFL3hKZy9OR3d2TTBubGhBTGhnT2FFUFBOOGVucm9aWFRBdmVkLzhm?=
 =?utf-8?B?MzJRZm9LUnVOczV6VlBTSy9Rb0J2UEZ1N1dkL1FiQ095dEZOK2hRSy9mQnUx?=
 =?utf-8?B?a3Uyb0YrRExqTUh6Z0ppTnRUZ25vRXhYNStmWW51SWhoc2pFN3FCcWhWRTRY?=
 =?utf-8?B?VHJKWGJJZ1JBU3NEY1ZBTStpU2J3L0Z2NEZodko2aHArc25lL0UrVkduMkNK?=
 =?utf-8?B?dkV2Zi9NVThWTUR0WXJuRTlYcUdCN2FhVHZCUzlmdmRqU29yUE44Z0cxNnY3?=
 =?utf-8?B?ODdsMllDZy9jQWlJbTZ4QWxsL2RBc3c5cmlsNDBWQ1JDTTJDdldQTENVUUtP?=
 =?utf-8?B?UFQxSzRZQmNnZ2U3WWdjNWJURjRvSVhUKzdSd1YraThXaXdJM3NIM1VPUlU0?=
 =?utf-8?Q?CAd/tCDMa6TXtb+9HDG4L2AA3?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9ac451dd-7760-4a56-7cef-08dc68514389
X-MS-Exchange-CrossTenant-AuthSource: BL1PR12MB5732.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Apr 2024 13:35:41.6444
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: OX8NifOWbrw7klPd+AVUaie9KyqW7mduQkn+rEjaV2XJ50sBqiOlZwnpem0/yxCEhI1RotjtL3uDUn6wCyHhAQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR12MB7487

On 4/26/24 16:58, Dan Williams wrote:
> Tom Lendacky wrote:
>> The TSM attestation report support provides multiple configfs attribute
>> types (both for standard and binary attributes) to allow for additional
>> attributes to be displayed for SNP as compared to TDX. With the ability
>> to hide attributes via configfs, consoldate the multiple attribute groups
>> into a single standard attribute group and a single binary attribute
>> group. Modify the TDX support to hide the attributes that were previously
>> "hidden" as a result of registering the selective attribute groups.
>>
>> Co-developed-by: Dan Williams <dan.j.williams@intel.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>> ---
>>   drivers/virt/coco/sev-guest/sev-guest.c |  3 +-
>>   drivers/virt/coco/tdx-guest/tdx-guest.c | 29 ++++++++-
>>   drivers/virt/coco/tsm.c                 | 82 ++++++++++++-------------
>>   include/linux/tsm.h                     | 41 ++++++++++---
>>   4 files changed, 102 insertions(+), 53 deletions(-)
> [..]
>> diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c
>> index 1253bf76b570..964af57f345c 100644
>> --- a/drivers/virt/coco/tdx-guest/tdx-guest.c
>> +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
> [..]
>> @@ -249,6 +250,30 @@ static int tdx_report_new(struct tsm_report *report, void *data)
>>   	return ret;
>>   }
>>   
>> +static bool tdx_report_attr_visible(struct config_item *item,
>> +				    struct configfs_attribute *attr, int n)
>> +{
>> +	switch (n) {
>> +	case TSM_REPORT_GENERATION:
>> +	case TSM_REPORT_PROVIDER:
>> +		return true;
>> +	}
>> +
>> +	return false;
>> +}
>> +
>> +static bool tdx_report_bin_attr_visible(struct config_item *item,
>> +					struct configfs_bin_attribute *attr, int n)
>> +{
>> +	switch (n) {
>> +	case TSM_REPORT_INBLOB:
>> +	case TSM_REPORT_OUTBLOB:
>> +		return true;
>> +	}
>> +
>> +	return false;
>> +}
> 
> Why do these callbacks need @item and @attr?

It is a generic callback from configfs, so outside of TSM, an 
implementation may find it useful to have these. But, with the code 
change to require the callback at the attribute level, now, these can be 
eliminated.

> 
> [..]
>> +static bool tsm_report_is_visible(struct config_item *item,
>> +				  struct configfs_attribute *attr, int n)
> 
> Per the comment on where to find the is_visible() callbacks for a given
> item type, I expect the need to pass @item here goes away when this can
> assume that there is only one way to have is_visible() invoked for
> @attr, right?

Yes.

Thanks,
Tom

> 
> Other than that, this conversion looks good to me.