Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <a98497ce-3d58-92cc-fe0c-727c7a5d6929@oracle.com>
Date:   Thu, 20 Jan 2022 16:37:44 +0000
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH 8/9] KVM: SVM: Don't apply SEV+SMAP workaround on code
 fetch or PT access
Content-Language: en-GB
To:     Sean Christopherson <seanjc@google.com>,
        Paolo Bonzini <pbonzini@redhat.com>
Cc:     Vitaly Kuznetsov <vkuznets@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Joerg Roedel <joro@8bytes.org>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Tom Lendacky <thomas.lendacky@amd.com>,
        Brijesh Singh <brijesh.singh@amd.com>,
        Liam Merwick <liam.merwick@oracle.com>
References: <20220120010719.711476-1-seanjc@google.com>
 <20220120010719.711476-9-seanjc@google.com>
From:   Liam Merwick <liam.merwick@oracle.com>
In-Reply-To: <20220120010719.711476-9-seanjc@google.com>
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?QldFd01OaWpkL21YTGc4dFhValhOVE1LMnlPazd1UzRaNzRNYVV2YVliV2Zk?=
 =?utf-8?B?S0c2VlB4aTAyK2dXcGE3RzF6SENxTkhaQmtDeFdGS2lqMFQ4U0Roa0luSDJ4?=
 =?utf-8?B?YWZ0enlFUHNCcDRpNndKZE4rMFltanJoT2orVDZaVkJsQ1V1NHZoY1J4SDZ5?=
 =?utf-8?B?eGd3bVJjNmROSW9RT2hmUE1nS2Y0NHRUbEF4U3h0cHU4ajl2clUvZ0FmOFI0?=
 =?utf-8?B?Z3puK0tqSzJabEhxQlNQbkxWeGZvZSsySjVBOGFTSTN6VFhoK0dLMjVzOURN?=
 =?utf-8?B?Nmd6UGFadWlUNGdOVUJLQk1pb3Vhei9ER2hFSEhidzBTdFl1ejRYekowTFN6?=
 =?utf-8?B?VlQrVDVsUGtLcko4T0k4OTBSRTc0MVVkeUMxdEU1L0V3QjlXNGZ2RkppZEg2?=
 =?utf-8?B?WXlZUTJSaUFEYkl3QWtBZzIrd2FiSThuL3VEeUFPRFFqYkRpYmVwWFFoemd2?=
 =?utf-8?B?MXljdmJoVEplWk9PcnRGTDNKek80V2ZOdDRqa1doR2VURFJCYjUwUEIycUpP?=
 =?utf-8?B?UWF6dUFMRFhkdFByOFlCVGhQd0R3dWlOdVFPRysrSWROSjZwNmdrbVcwZ09r?=
 =?utf-8?B?eHNJbUpHSW1hSlJCdlgrTEwvNFRxQS9kdTdCSlhsNUp6cHFxUTJxTXVnVnFT?=
 =?utf-8?B?K0lIVTNzcVdxMkdsWXcwVUFkQkYyMmtnS2lXUnhUTHhHK3I1VWN0cENoSUkz?=
 =?utf-8?B?ak5ZVVhBb1ljZTkxbjlnTGx1M04zZWpiZllSdmxpZ25PVDJ2NjJRUzRKc28v?=
 =?utf-8?B?N0lPWW9DajBiTVpwRXpuN3lOSTB5elFDa3VseVNWVEZ6cHpDbzhxRHhNWkdp?=
 =?utf-8?B?TWVia0QvT1o0WDJBTXV4MTRKSEh2Vmh5VFJXS2p5VHlUaFlEQjU1cHMzVjNI?=
 =?utf-8?B?TmV3MFFocHUyQmFpN3dDZ1dnSGN2L2QxMlI1YlZDcGZ0bkdxVlFOWTBhMS9W?=
 =?utf-8?B?UlA1TnQ2S3ROclVHUGVlUnhiOWkvZEEwV2lWVi85R2ZqVUVqNmNPYUlaTGta?=
 =?utf-8?B?Y0RCOFVKWVZKNFA0M0RVUnplSTluQmtNK2hlbjUybStYa04wbVYyTnFNV3FL?=
 =?utf-8?B?bzRKYkw5aW5NODRwSmVWTk5EeXFtcGd4dFM4QkJURERCRmhERHRISzFJWFNs?=
 =?utf-8?B?YVpES2wweXIxQnV4NVU2RDBIZDFxTDRvSGY0NEJtSC9sbEdBVHNXUElpOVlS?=
 =?utf-8?B?VU84NUxGT0hsSHVHc3ZvVTBycHYxRDkwMkFwbDh0QmlYVW0rUnJiWnZmSUN5?=
 =?utf-8?B?TUdJVkdMaW1RdEdrclFSVDF6akM3WkNjdlZjZkhLSi9SQ0JKRkpsdFp4bFd5?=
 =?utf-8?B?ajZsay9ieGl5TFBxVkkwMjlXOHFzaXFFU1dRK005ck9VM3QwMWpLakJXdG1B?=
 =?utf-8?B?bmFXQnpEbWZuV1BiVnlPUXZSTDlDV1hsdGpHcEVsSVUzeDdrdVBRYng3N1Iv?=
 =?utf-8?B?VTVhOHpLK0g1UTVsdmZlWFdqZGw3ZUVVQXE4SEhVbStrU1k4N21UTE5JbHhi?=
 =?utf-8?B?cXpTZDR0R2d6b2hMbGdFYjl0UkRnTnp6K3RuaDJoeFdvNmJnUHZhblM3SDNj?=
 =?utf-8?B?MndiRzJ3cUV4ajFFblhpWFBGVGVaaTFXeDk2ZWRaZkdVUncxRUlrTTZCL1dU?=
 =?utf-8?B?K1RCRWY1OUV0emQ0VVFya015UW43OHFmazV0MUk1aW9EdTJPdnNiTFl1bDhC?=
 =?utf-8?B?djFRbGNVVTd2SkthSnJkRWtILzZkUXFaRTFvYUdwTjdvM25aN0NGK01ZeUpq?=
 =?utf-8?B?Vm1IVnVQU3UwY3lURXpSSi83bkJXUTZZa09nQTkxUWtLU3RSbkhpZjJYeUgx?=
 =?utf-8?B?WVJSM2tJVlNBdDJENnNnWTBBbThRLzRaRTdoK3Z0cnh6R08xWWppKzljMU5x?=
 =?utf-8?B?V1NmeG5xTmtaTWtCcnN4emJvRE5mOE8yTWJEc3ZxbngwSkpla0lSWDhFT0du?=
 =?utf-8?B?cEMzOUR2SUFkbE1jUmRTMEtoTXM4dThqazkyenNoY2ZUVEdFRlZFRzlpMGh4?=
 =?utf-8?B?UkdDcHhBWS9GUjRzclZTVUEwY1ZhRVZrQkFySkxoc1JBSXR1bitFdzFjU1R1?=
 =?utf-8?B?Slk0N1kwUVVqaTlsZ2JIbW5mbnZnSkQ3a3UvU1RmdG9weEhXeTV6cWJWOU5D?=
 =?utf-8?B?VFN0aGFxM2E2K0VkdnVTY2tjWGs1T1BQMmZCZ05EdThFOVAwKzEwUEpVS25o?=
 =?utf-8?Q?CD4cmWFVyWYG9gniC9EB9cg=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7ed02147-02b9-4e68-5fa4-08d9dc3332c0
X-MS-Exchange-CrossTenant-AuthSource: DS7PR10MB5038.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Jan 2022 16:37:50.4733
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 3plWhSD0FFA3ATVLPsoetaUvD1o+rw4BFQT4PsP5EeBeCBhQvO4O+6l8Ag7nHbgSlqJmkeepq+7zgTlFWGpcRA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR10MB4523
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10233 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 phishscore=0
 mlxlogscore=999 malwarescore=0 bulkscore=0 suspectscore=0 spamscore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2110150000 definitions=main-2201200084
X-Proofpoint-GUID: p-84wWsTFY4ykROnBUy21Yndc0V9O_xC
X-Proofpoint-ORIG-GUID: p-84wWsTFY4ykROnBUy21Yndc0V9O_xC
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 20/01/2022 01:07, Sean Christopherson wrote:
> Resume the guest instead of synthesizing a triple fault shutdown if the
> instruction bytes buffer is empty due to the #NPF being on the code fetch
> itself or on a page table access.  The SMAP errata applies if and only if
> the code fetch was successful and ucode's subsequent data read from the
> code page encountered a SMAP violation.  In practice, the guest is likely
> hosed either way, but crashing the guest on a code fetch to emulated MMIO
> is technically wrong according to the behavior described in the APM.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>


Reviewed-by: Liam Merwick <liam.merwick@oracle.com>

> ---
>   arch/x86/kvm/svm/svm.c | 43 +++++++++++++++++++++++++++++++++---------
>   1 file changed, 34 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index d324183fc596..a4b02a6217fd 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -4262,6 +4262,7 @@ static bool svm_can_emulate_instruction(struct kvm_vcpu *vcpu, int emul_type,
>   {
>   	bool smep, smap, is_user;
>   	unsigned long cr4;
> +	u64 error_code;
>   
>   	/* Emulation is always possible when KVM has access to all guest state. */
>   	if (!sev_guest(vcpu->kvm))
> @@ -4325,22 +4326,31 @@ static bool svm_can_emulate_instruction(struct kvm_vcpu *vcpu, int emul_type,
>   	 * loap uop with CPL=0 privileges.  If the load hits a SMAP #PF, ucode
>   	 * gives up and does not fill the instruction bytes buffer.
>   	 *
> -	 * Detection:
> -	 * KVM reaches this point if the VM is an SEV guest, the CPU supports
> -	 * DecodeAssist, a #NPF was raised, KVM's page fault handler triggered
> -	 * emulation (e.g. for MMIO), and the CPU returned 0 in GuestIntrBytes
> -	 * field of the VMCB.
> +	 * As above, KVM reaches this point iff the VM is an SEV guest, the CPU
> +	 * supports DecodeAssist, a #NPF was raised, KVM's page fault handler
> +	 * triggered emulation (e.g. for MMIO), and the CPU returned 0 in the
> +	 * GuestIntrBytes field of the VMCB.
>   	 *
>   	 * This does _not_ mean that the erratum has been encountered, as the
>   	 * DecodeAssist will also fail if the load for CS:RIP hits a legitimate
>   	 * #PF, e.g. if the guest attempt to execute from emulated MMIO and
>   	 * encountered a reserved/not-present #PF.
>   	 *
> -	 * To reduce the likelihood of false positives, take action if and only
> -	 * if CR4.SMAP=1 (obviously required to hit the erratum) and CR4.SMEP=0
> -	 * or CPL=3.  If SMEP=1 and CPL!=3, the erratum cannot have been hit as
> -	 * the guest would have encountered a SMEP violation #PF, not a #NPF.
> +	 * To hit the erratum, the following conditions must be true:
> +	 *    1. CR4.SMAP=1 (obviously).
> +	 *    2. CR4.SMEP=0 || CPL=3.  If SMEP=1 and CPL<3, the erratum cannot
> +	 *       have been hit as the guest would have encountered a SMEP
> +	 *       violation #PF, not a #NPF.
> +	 *    3. The #NPF is not due to a code fetch, in which case failure to
> +	 *       retrieve the instruction bytes is legitimate (see abvoe).
> +	 *
> +	 * In addition, don't apply the erratum workaround if the #NPF occurred
> +	 * while translating guest page tables (see below).
>   	 */
> +	error_code = to_svm(vcpu)->vmcb->control.exit_info_1;
> +	if (error_code & (PFERR_GUEST_PAGE_MASK | PFERR_FETCH_MASK))
> +		goto resume_guest;
> +
>   	cr4 = kvm_read_cr4(vcpu);
>   	smep = cr4 & X86_CR4_SMEP;
>   	smap = cr4 & X86_CR4_SMAP;
> @@ -4350,6 +4360,21 @@ static bool svm_can_emulate_instruction(struct kvm_vcpu *vcpu, int emul_type,
>   		kvm_make_request(KVM_REQ_TRIPLE_FAULT, vcpu);
>   	}
>   
> +resume_guest:
> +	/*
> +	 * If the erratum was not hit, simply resume the guest and let it fault
> +	 * again.  While awful, e.g. the vCPU may get stuck in an infinite loop
> +	 * if the fault is at CPL=0, it's the lesser of all evils.  Exiting to
> +	 * userspace will kill the guest, and letting the emulator read garbage
> +	 * will yield random behavior and potentially corrupt the guest.
> +	 *
> +	 * Simply resuming the guest is technically not a violation of the SEV
> +	 * architecture.  AMD's APM states that all code fetches and page table
> +	 * accesses for SEV guest are encrypted, regardless of the C-Bit.  The
> +	 * APM also states that encrypted accesses to MMIO are "ignored", but
> +	 * doesn't explicitly define "ignored", i.e. doing nothing and letting
> +	 * the guest spin is technically "ignoring" the access.
> +	 */
>   	return false;
>   }
>