Received-SPF: pass (google.com: domain of linux-kernel+bounces-154887-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: <f8dbd58c-78da-4b3f-a79b-6693c04fb104@amd.com>
Date: Tue, 23 Apr 2024 16:12:00 +0530
User-Agent: Mozilla Thunderbird
Reply-To: nikunj@amd.com
Subject: Re: [PATCH v8 06/16] virt: sev-guest: Move SNP Guest command mutex
To: Borislav Petkov <bp@alien8.de>
Cc: linux-kernel@vger.kernel.org, thomas.lendacky@amd.com, x86@kernel.org,
 kvm@vger.kernel.org, mingo@redhat.com, tglx@linutronix.de,
 dave.hansen@linux.intel.com, pgonda@google.com, seanjc@google.com,
 pbonzini@redhat.com
References: <20240215113128.275608-1-nikunj@amd.com>
 <20240215113128.275608-7-nikunj@amd.com>
 <20240422130012.GAZiZfXM5Z2yRvw7Cx@fat_crate.local>
 <6a7a8892-bb8d-4f03-a802-d7eee48045b5@amd.com>
 <20240423102829.GCZieNTcHyuAYMcRf5@fat_crate.local>
Content-Language: en-US
From: "Nikunj A. Dadhania" <nikunj@amd.com>
In-Reply-To: <20240423102829.GCZieNTcHyuAYMcRf5@fat_crate.local>
Content-Type: text/plain; charset=UTF-8
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?MlNSRTc5blRkVXpLcTBUTXU2QW9PclJBZ0NMSmNHelJPU3BuenBmcHovU1VI?=
 =?utf-8?B?dlhmRjFuWWt3WkNTN0hCL2pPdk9ubHJRU05qc0hZRkVveEtFVzRJSmk0SzJL?=
 =?utf-8?B?ZHVrTzFwS2cycGRydlhMdENiczRGdTMzZnBsY0JzTXJ1d25yK0tWcVZBbWJO?=
 =?utf-8?B?emRkWEtpYzFFYi9NTVpFUXp4dC9FREVqV25kVysxMEpaaklPZW16OUdCOFlj?=
 =?utf-8?B?V2d3SitMUjk4VmMyckF1K2VLc2hCc1kwM2h1SFQzMjFjWUUwVVhmSmtyaVlt?=
 =?utf-8?B?QWR5Z29zQTY1QVI5V3huS2lKRytvRDE1WFJlQkRMYjJvL21FM2JhbXloRWlM?=
 =?utf-8?B?dG1ITEcwd3ErdDhZMVBNQ2pSM0V0cXUvc1NaK3RQcUJnQWlEQUhpV2FWdDNp?=
 =?utf-8?B?M2JCQUlMbzJOVW5oajFSU0ljMDNsb3FnSVZaRGZWZUVZRVNFMVBnOGdhbjVP?=
 =?utf-8?B?NzdESnhac1d3SzlnRjB4R3R2U0FNT0dmaFBITlcrM291eEZ6bmNyMmlHSFo1?=
 =?utf-8?B?Rm5UVnVRa2xabC94Q2Q1c24yUy9qNXN0ZnFsMmFaRjZkYldMMVZwa2ZaejB3?=
 =?utf-8?B?ZmtiRnBtQVZsN1ppbTR1L09GMzNpclVGZk9tNStQei95UHBxYkhDL1hJVmRW?=
 =?utf-8?B?YVkyY1dqdWlmMlM4Q3k3dmdHNndlbDJmU0JYMmFjSTM4aXhkWk9zN0tHUXdH?=
 =?utf-8?B?ckdWSXM1V09SeG1sMjJYcWxvSE5qdGV6ZDlNQUZoWVJwWHFtaFBOS2VTWTBV?=
 =?utf-8?B?dlVsdTBwZ3VhTExZMi9sS1NacWoxNHlPUGp1ZXJIN3R4V3lCL3lxTkV4V1py?=
 =?utf-8?B?SURCQ3M1blcyU0lZVnU1aW1PdmdvbjZRTURIamhtbTdrUVRkUGtGS1VqMzgv?=
 =?utf-8?B?Z3grNm91QWlaakU1S0gvMlIyY1I5bHkyQWpwUmNMaGFSdmF5ZlZSL2ZaOUN4?=
 =?utf-8?B?VVRIZDB5TmRmQWdCNVZURmtjVzhqSjRnRGEvQTBLTE50ZXIvSnFBL2VuZ1hE?=
 =?utf-8?B?UFF6Qm4yZjNFWUZwRFAveGY3bk9lSkM3VzVGdWh3YjdPb3gwUit3aUozNXJ1?=
 =?utf-8?B?QUkyOEY2aGpCYzFLTHl0THkyQ2M1Y3kvK2QzQytiamVleWRvWFZQVG9iU3Nr?=
 =?utf-8?B?VlJKdFIxeHg1V1FwRW9YREo2QUtuYmFIL1RCcm5hSGNXYlFiU0wzTzhmd0lK?=
 =?utf-8?B?WVVtd2dsZ3IvUFdudjRsamRocDIxTC9Wc01xOVNpeGJJQXJwd1Jac0ZkWVhs?=
 =?utf-8?B?Tk5KaUwvVzlNQ0hkdFRSM3ZCazhnM0lvcHhwSnEyR2pJMnlXeTM4YkIvc0xw?=
 =?utf-8?B?eVRPSXZndUpYMWdqL1MrSnBLVUdQVlBPNVBTZUplaEtSZThHRElrMEpXa2pp?=
 =?utf-8?B?LzFsNHNCelEwdGpVMmZwSVBFdUQ4eEVsdEZUQk02QUQ0T0pUSHlsM0R1bXUy?=
 =?utf-8?B?T1ZQaEh0MktZZFFtTU9TN1dnQU9HMWVCazNMTXVpYmo1ODV0YjFDTVpqUENR?=
 =?utf-8?B?ZWg5bWRyTjE4MmFObmV4VXpnN05MZGpOMFRvOVhoeXZ3ajFmbFNvcTBOamJG?=
 =?utf-8?B?TDB6S0NCdmlMYktvaU5qNEYxRDJ5OXJIb2J6UXNJQXhwaVlvclhUbHlvNk5L?=
 =?utf-8?B?V0pUNVpkaDVxdzcycE5OU09vckFqZ1BraC95dnZ5enlkWW9ZRldFcHhNZ1d3?=
 =?utf-8?B?U1NHNUQ4MlJ1QzBEVnNGRUdjTTNLTUlNYWZtbE1peFNmTW41NUVXa0JUaHZs?=
 =?utf-8?B?YVdFeWJSU0xpV1NYcGI5RFp5RHdFdS8yeTgvTXBnY0VRSStjMm5tbUJLWW05?=
 =?utf-8?B?RnFnTVRJL3pjRUYwRUVMOGlBeFFXNDc1YlcvV1RjVUtKZnZzSUhLbE85aXNm?=
 =?utf-8?B?ZE1MT1M1UGRVREh4WXpnSEVTOEt3UXpqK1BUbmpQbU80NGljZklUYkNWWWwz?=
 =?utf-8?B?ZjJhWkpjQ1poOWVjTWZwd0N0aS9zb2pORGw3dXFxZ3ZweldvRUVpRlppVHN5?=
 =?utf-8?B?RWtoQWxjendlTmt1bjZYSFdua3k3UXdXdGN2TlNWRzd4d041Y3pJRVl3a3Ar?=
 =?utf-8?B?dnJjTW1TUU92alIxN0lhN3BrQ1FkMnd3Nk13b0Q5VmNXdCtFc3JZSHJGNEJT?=
 =?utf-8?Q?qCn6AshGkHnd8D4HSFSc1OBgm?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 87856ec6-6089-49aa-dbfa-08dc638207c4
X-MS-Exchange-CrossTenant-AuthSource: DS7PR12MB6309.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Apr 2024 10:42:11.0534
 (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: MIWBWlqPVminnC0IAJY826QAIMISPy1XhOr/ey+dBoYDOQENIn4COwotgpx7txPA8l0NPWjQCldc5Ml7/WjJfg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR12MB8538

On 4/23/2024 3:58 PM, Borislav Petkov wrote:
> On Tue, Apr 23, 2024 at 09:52:41AM +0530, Nikunj A. Dadhania wrote:
>> SNP guest messaging will be moving as part of sev.c, and Secure TSC code
>> will use this mutex.
> 
> No, this is all backwards.
> 
> You have a *static* function in sev-guest - snp_guest_ioctl- which takes
> an exported lock - snp_guest_cmd_lock - in order to synchronize with
> other callers which are only in that same sev-guest driver.
> 
> Why do you even need the guest messaging in sev.c?
> 
> I guess this: "Many of the required functions are implemented in the
> sev-guest driver and therefore not available at early boot."

Yes.

> 
> But then your API is misdesigned: the lock should be private to sev.c
> and none of the callers should pay attention to grabbing it - the
> callers simply call the functions and underneath the locking works
> automatically for them - they don't care. Just like any other shared
> resource, users see only the API they call and the actual
> synchronization is done behind the scenes.
>
> Sounds like you need to go back to the drawing board and think how this
> thing should look like.

Something like below ?

snp_guest_ioctl()
-> get_report()/get_derived_key()/get_ext_report()
  -> snp_send_guest_request()
       snp_guest_cmd_lock();
       ...
       snp_guest_cmd_lock();

With this the cmd_lock will be private to sev.c and lock/unlock function
doesn't need to be exported.

Regards
Nikunj