Message-ID: <e0d52194-d478-b33e-851b-d5067f4a7669@amd.com>
Date:   Mon, 30 Jan 2023 10:36:56 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH v13 1/4] virt/coco/sev-guest: Add throttling awareness
Content-Language: en-US
To:     Borislav Petkov <bp@alien8.de>,
        Dionna Glaze <dionnaglaze@google.com>
Cc:     linux-kernel@vger.kernel.org, x86@kernel.org,
        Paolo Bonzini <pbonzini@redhat.com>,
        Joerg Roedel <jroedel@suse.de>,
        Peter Gonda <pgonda@google.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Ingo Molnar <mingo@redhat.com>,
        "H. Peter Anvin" <hpa@zytor.com>,
        Venu Busireddy <venu.busireddy@oracle.com>,
        Michael Roth <michael.roth@amd.com>,
        "Kirill A. Shutemov" <kirill@shutemov.name>,
        Michael Sterritt <sterritt@google.com>
References: <20230124211455.2563674-1-dionnaglaze@google.com>
 <20230124211455.2563674-2-dionnaglaze@google.com> <Y9emVjoTBrM2+Y5P@zn.tnic>
From:   Tom Lendacky <thomas.lendacky@amd.com>
In-Reply-To: <Y9emVjoTBrM2+Y5P@zn.tnic>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?TG4wRGVkb1h6NFBtZTloc3pNVDVVY3V6UHJNUzNtOG5JaVpydGYzcFVpVEgy?=
 =?utf-8?B?ajgwRWZzQ0Zrd2JPNEdHVExmdXg2OWNKYzNuL2dxRnFwTi9yVk5wVkkrYWdM?=
 =?utf-8?B?SndDRUVwQ2xDc3VkekZhYUpPMWdJbVkxRnBJa2oxWnFydkh1ZllpNGlGVi9D?=
 =?utf-8?B?UGxCS2NWbm1XaW8xOENVSjJ5blFzeDFtYkUybkJrUWRXM0hySURHSldNcytX?=
 =?utf-8?B?eGYzekFWMDJ6ek1lM1FQZDU2SEZQYWxyNmtFZU41VC9tK3B4aDdUSWtnUE5V?=
 =?utf-8?B?ZGUrS2cvR3A0dVlhSTcvSjluMFNqRGlMRHR3U0dKMW0rT3BTbGxCZE5FVUF2?=
 =?utf-8?B?UHk1bVFpYzN3R2UwYitEQmNLak9JdG9JTThNVzRtTTh3aDhEQ1BackMyS3RT?=
 =?utf-8?B?czkrUThaVkNHbUZQQW5uSnBxSGRudlBBZkwrUjVNaWdZZWZmYlJWa1FWeGlU?=
 =?utf-8?B?TUZMc2sxcWxHY2trUVkweWFuNDVlVTVHVmNoOTAwRkI2ZTQ2VTNIaGhIU3Rz?=
 =?utf-8?B?a0hFcEhscHhFNmdhT1E0NEVMZlNxVTg0cTJad1JPRTNzY1NKZisyNzZiQS84?=
 =?utf-8?B?UG5KVzluaVVOcm1PVGp5ZDRxTElnckZYUVJ0blA5WTNUZlB4Mk14T1NoVVpn?=
 =?utf-8?B?UCtUc3ZiUUhDdnJ5WkxjMnNTSDRGOW9Dc2hUcll1UGFZZWh6R2NBNlJ1eXJz?=
 =?utf-8?B?em01VEZCbkpCSHd5RE53N3VUenZUUHRqcDRuTTVXRDBJSENPSHZjVkJ0MjFH?=
 =?utf-8?B?dHNqeDRFQkd2VnRhSElYZzJXMm5Hak1jODdKVzhzdVkrSFBxcEU1d0hCL3ZY?=
 =?utf-8?B?akd5cXBhZWhyd1I1NkpVbVVET3NRcW43bmU5UnZsQm1ZUXVlK1ZSWGVsYThj?=
 =?utf-8?B?aHV0czZySjF3dnEzamtaaVhjaDljSGdNZ2FOY0piRzBHK2o1MG5OMyt0Wkg4?=
 =?utf-8?B?VTZMK1drWEpyRWFJcHJ6Y0p0OFRKZ0FSMVJNbG1jTDhFcytzL1VHYmFySkox?=
 =?utf-8?B?dnFnSDlqUisxZ2pNWnZYU1dlRys4Z0dRellLUy9pWXg4WC9Kci9pb1FFaDJD?=
 =?utf-8?B?QSs3STJubGNwcmdESW1IbVMrbzJQVDRKenNoakVLKzhVcy9lYndnWTZXUkhv?=
 =?utf-8?B?YjZ6SG9FcjI5a1cxUnpOY0tNdlJuSnc4SEtOaThVWEJacUJ3YlZvbEF2YUFC?=
 =?utf-8?B?ZUwwVDE4WTJrbHh5OFF2c1c4N25BODlGSzZEajJ6d2h0b3ZQUEtDWVpkZWFt?=
 =?utf-8?B?d3NZb3QwWWxZTFBHMzh1ZGVNT3Z6MjJoekZvWjVRcUk3ZWN4b3B4ZDlVUDZG?=
 =?utf-8?B?SlNsUk5SaEUvdDlVeEFMS3FwT01aZGxPS3pJWmlTK1hJTXBqaUhDUGNoTkhp?=
 =?utf-8?B?SEhPb1Baek9zT1RVYy81MXdDdXlKNzloTWZIQUFDZGhiQno4YkoyU3BQWFlU?=
 =?utf-8?B?eTZIQmZjenRHZXhRWm9EYURsWXlnOHRIVEpvMUkvbGx4bjBtOVd3bUdZVjZR?=
 =?utf-8?B?dDg3V2RQTmV1ZFJKLzU2TGlwK1VwdmFUMWVBSjR4QXlyWGlQWExuemVrSDRI?=
 =?utf-8?B?UGZqMEVFelIyandXb3V1QUlDZDBFYUR2QlVxUlhzRkpleDIzcFlsTytlb2kz?=
 =?utf-8?B?YTZwbE93NUlTODFFM1YvazNQZWpMWWFHdjQ3djRSaTQrTDgvVHBWWlR3UWRG?=
 =?utf-8?B?ZG1mdUlEdXl3OHVnZ0c3YWFKNTBDRlNOSjVMQzFsbWxqL3grbFNFWUVyQXB2?=
 =?utf-8?B?TTJPNGZYN2xMMlJqVmFYb3BFd1ZhSnJlaVE0MXY0YU5LQ2NTUDRrbTlSeGNP?=
 =?utf-8?B?NWRWRVhlRFAvYlh4eWt0S1pDRVVBRWhXWDZrdUxrN04vZDgrM2FqMVA5WDUz?=
 =?utf-8?B?cGpHcmZLejRIRSs2ZW5UOXRCUHlaRnB5UG56bzdNbGIrM3VRSjlQblkyeHpa?=
 =?utf-8?B?Mk9rTENrQ1A1aERKM3pQMldpS3lhWDZhMlR5cDh5VDNoTUppaHU2dFhlOGlE?=
 =?utf-8?B?MzFWTWFNSGRmc1l4aGkwS3JoZE1FamRrWDBvd0RwRC9pVUVXM013a0ovWENn?=
 =?utf-8?B?ZXdkV2xINTN5K2I1MUJZaFpkd2lna3ZwK2grQm5PUDlvdTNtb2UxZHRqbE96?=
 =?utf-8?Q?Qc/kQechpBEteX6RB7ckr96U0?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ea68319d-3600-4d1f-835a-08db02e034d2
X-MS-Exchange-CrossTenant-AuthSource: DM4PR12MB5229.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Jan 2023 16:36:58.9104
 (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: 9B2GpcrZ/tdZQiWjnAo8YFB95U9ablwDzE/PRSmrTi2t7c7HgOp39V2T31OiOERpprjFLKqTcbPYsmTngrF1QA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0PR12MB5785
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 1/30/23 05:13, Borislav Petkov wrote:
> On Tue, Jan 24, 2023 at 09:14:52PM +0000, Dionna Glaze wrote:
>> The host is permitted and encouraged to throttle guest requests to the
>> AMD-SP since it is a shared resource across all VMs. Without
>> throttling-awareness, the host returning an error will immediately lock
>> out access to the VMPCK, which makes the VM less useful as it can't
>> attest itself. Since throttling is expected to be a common occurrence, a
>> cooperative host can return a VMM error code that the request was
>> throttled.
> 
> So where is this concept of guest throttling documented?
> 
> It sounds like this is something hypervisors do and it is all fine and
> great to do that but where does it say: yes, this is what we do and this
> is the usual behavior that's expected from guests and HVs to adhere to
> when accessing this shared resource?
> 
> Tom, is that in the spec somewhere perhaps? Or was this decided upon on
> some call?

Yes, this is part of the GHCB 2.02 specification document that is in the 
process of being published.

Thanks,
Tom

> 
> In any case, I'd like for us to document it somewhere eventually if that
> hasn't happened yet so that all parties are clear on what is supposed to
> happen and what the protocol is.
> 
>> +retry:
>>   	/*
>>   	 * Call firmware to process the request. In this function the encrypted
>>   	 * message enters shared memory with the host. So after this call the
>> @@ -346,6 +347,14 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in
>>   	 */
>>   	rc = snp_issue_guest_request(exit_code, &snp_dev->input, &err);
>>   
>> +	/*
>> +	 * The host may return SNP_GUEST_REQ_ERR_EBUSY if the request has been
>> +	 * throttled. Retry in the driver to avoid returning and reusing the
>> +	 * message sequence number on a different message.
>> +	 */
>> +	if (err == SNP_GUEST_REQ_ERR_BUSY)
>> +		goto retry;
> 
> I don't like even potential endless loops.
> 
> How about you turn this into a loop with a sufficiently large retry
> count which, when depleted, gets this request failed with a -ETIMEDOUT
> or what not?
> 
> You could also stick a cond_resched() in that loop so that it can take a
> breather between the requests and doesn't hammer the hw as much.
> 
> Thx.
>