Received-SPF: pass (google.com: domain of linux-kernel+bounces-142482-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Fri, 12 Apr 2024 18:18:23 +0800
From: Chao Gao <chao.gao@intel.com>
To: Jim Mattson <jmattson@google.com>
CC: <kvm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<daniel.sneddon@linux.intel.com>, <pawan.kumar.gupta@linux.intel.com>, "Sean
 Christopherson" <seanjc@google.com>, Paolo Bonzini <pbonzini@redhat.com>,
	Jonathan Corbet <corbet@lwn.net>, Thomas Gleixner <tglx@linutronix.de>, "Ingo
 Molnar" <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, Dave Hansen
	<dave.hansen@linux.intel.com>, <x86@kernel.org>, "H. Peter Anvin"
	<hpa@zytor.com>, <linux-doc@vger.kernel.org>
Subject: Re: [RFC PATCH v3 01/10] KVM: VMX: Virtualize Intel IA32_SPEC_CTRL
Message-ID: <ZhkKb+lgPDNRsYXa@chao-email>
References: <20240410143446.797262-1-chao.gao@intel.com>
 <20240410143446.797262-2-chao.gao@intel.com>
 <CALMp9eR294v_2-yXagKR8HM_WbqihJ5JcRwD1NTGvJxsOFsnyw@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CALMp9eR294v_2-yXagKR8HM_WbqihJ5JcRwD1NTGvJxsOFsnyw@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SS9DZGdxZjhSRjVoNmxkamR5VWRKRUU5VjBwWVpVOUpTN3hER0ZyRjlxRmVJ?=
 =?utf-8?B?Mk80aEdFQ2c3NFFxWjEzZ0MwMjR0Y0x6dXNCTGlIQ3ZJTU5uRzJNcjBGWE41?=
 =?utf-8?B?TS9lV29rNWN0NkdVQlBuQnBjTGdzeDNFODlpMDhYSUVDV1lhSWplVHlqb000?=
 =?utf-8?B?YU9OdVVjTEs3dlFWSVVNdnFyWDFSUXEzYno4RlUxcEdQbFVIS1pKUnlkck82?=
 =?utf-8?B?WmtNdzZsZ3JuZUhUTkRPSVZWdm9WRDV3cDBqbEpYdlZrU1BXUFlCUVVNc3VR?=
 =?utf-8?B?RCswNTl2UGNQcXNPMElpdnBKVGNyWnp6cXdQZTBYdysvMER3QlRXdmNtZk5K?=
 =?utf-8?B?WHVJNUlSRjR6L0diZit4SE95dTlHWHp1em1oZWxkbG5xZy9IeUZDWXFoZHZN?=
 =?utf-8?B?bUoxUVJ1VUFnUEtlTzlISlAyaldPSTRSZjRTZ1pTOFU2aS84R3ptVW1lT3du?=
 =?utf-8?B?QkFhN0NJdys1aGVZamRZRkZkY2tMWWk2RDhVTS9pSXBRWFRON0FKNWNiS0ZU?=
 =?utf-8?B?a3paN0duK2VyMUZyYTVMK2tQd0tBYSs0dkZlZnpjOFFOMVN2dHk3QkxQUGNy?=
 =?utf-8?B?cjlTSHFPajQvVUxidVNMN0EvOUlhdHdPVXlkWi80ZkVIS3JFUU1kWkdMNnNI?=
 =?utf-8?B?UXlEVWZTN0kxb3R5T1cxeG52UEVKTTk4L24yRnROZUg4RXZsWXdZVTR1Ly9X?=
 =?utf-8?B?bWFRRk9WdUtLVkdrQWVRbzdVTEhHckIrWDA4ZTh3Smg3Mm5Ic0NTSVdQRkJG?=
 =?utf-8?B?Yk5vK1BnV3YxQW83eU5YK3VsY3R0Z1RQdmxRa3lPUkIrZk1JZjZWVVJXbGZQ?=
 =?utf-8?B?N3ppVWVWblhjUGlkMTEzMm1VaWYvQzhGQ2ltNlh5aStyU01jRjBocnNmV1Va?=
 =?utf-8?B?MUN6NDFmaW9ZdjIyQndVWHpPM3lrZUgxcUZ6NXJNOVBOWEhieUJveFZaS3BF?=
 =?utf-8?B?c05HMXlCeWtEdmNjNHFJTGV0TE9hcUlaWUxCNWg1cS91eTg2bHNpSjJ4WkFP?=
 =?utf-8?B?S1E5bVRpR2xKbUJuWE9aMklwaWIvalcvWGpiWnhEK25hcWFqelcvdzdkTW9F?=
 =?utf-8?B?L2Y4NXpmK0lKTFhnT3hoZGpzQTVydjZNUW9CM0xYRXYyYjFvN25PQXNvdCtH?=
 =?utf-8?B?ZjQrYUdiVWM2S2VvQ0ZQT3lPZllFQXlpOVFmN1dNTytTOXNSbGpJMlhnWXRT?=
 =?utf-8?B?NEpQVnBxeGYyRnBBU0VGM3JCUXhWSGExQ3ZBY2t5TUhoNVhRWDRVZ2dBSnRE?=
 =?utf-8?B?U0RnbWZNTlpXY2V3WGRhbGk0WTFNQmt2ZTdJaFp2VmtuZEY1MmZXZXByOTFB?=
 =?utf-8?B?Ri9pc2g1UEFSQ1hYUkJZNk51RC9BUzdUaG5sV3hPRTRIWVc1ck9sSGpJd29x?=
 =?utf-8?B?SUNXeE1jeDMzcWZCU2oyVVFxWGNLR1lwYjNkZmNYMU4vejZnL1QxRHlmYTQw?=
 =?utf-8?B?NHNBeFZmelFESVRpOVRGWHhTU0V1VXV5bk9NWGJpdW5ObS9yZGJ3L0hWL0hJ?=
 =?utf-8?B?bUVneUtuUTdrVW5mUFh6SUVoN2o2Vm9WUlNvNWZ0M25mY3B2UTF4WWZHVkpz?=
 =?utf-8?B?b1hXWjlMM2MzQytWWXpOZm0yaXVPOGFOK2VaWllEbER3dkhhaTJHbS9TcEQr?=
 =?utf-8?B?U1ZCazJkUEpGd0Fxckt0bytjK1paaFJhbVRFU3g3U3ZHbCtlODZhTjZEdjk3?=
 =?utf-8?B?ZURIMFZUL0NFblpQWWladW11UFR3UUFSVU1RNkZXcit4NW9KNW1ycjZyY1Fu?=
 =?utf-8?B?bkx4TThLZEU2S2hFb2EvanpseS9WSEw2TGFaRDUyUE5oM0RLRE9FbVIvNkxE?=
 =?utf-8?B?OXcybEJjVHA3d3FBaTdlcGZaQzRITm5hN1p1MTRiVHRXODRZMmtsbXlqeHJR?=
 =?utf-8?B?RzR6ZjMvZnRqZmRoNEY4d2ZhZXQrUEJFZnNMSURhYzd2Nm5yZ0swc1NhWlJj?=
 =?utf-8?B?RmtaSHgvSXBlV2Zmc1UwUXlGWWc2cW9pcjlvMi9jUTBPYUx2dFZHeGtpMENB?=
 =?utf-8?B?ME5LbjJaVEY4OHFKblZVZnVVcXZzemRCZ2FhZUtkTGppNnlYU0Z4RElwZkll?=
 =?utf-8?B?TVVFNzlHemJBMEZJZ2FjRG05UHd3K2lvNTZvSmtWMGhMOXpBRC9JeWtpK3E3?=
 =?utf-8?Q?DVS7MWNN4Q9p6yl92alaIqf/L?=
X-MS-Exchange-CrossTenant-Network-Message-Id: e02d17de-2dbc-4150-435c-08dc5ad9e8fc
X-MS-Exchange-CrossTenant-AuthSource: CH3PR11MB8660.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Apr 2024 10:18:34.3958
 (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: uTlCEuiIut1S3gXoeH1wYwxKLOQaZ4RuVr1U5pxYbI1QUMhsaYcdA63meDBVFRGWWwj0QmtFEUvsr/hgjr53ng==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB6720
X-OriginatorOrg: intel.com

On Thu, Apr 11, 2024 at 09:07:31PM -0700, Jim Mattson wrote:
>On Wed, Apr 10, 2024 at 7:35 AM Chao Gao <chao.gao@intel.com> wrote:
>>
>> From: Daniel Sneddon <daniel.sneddon@linux.intel.com>
>>
>> Currently KVM disables interception of IA32_SPEC_CTRL after a non-0 is
>> written to IA32_SPEC_CTRL by guest. The guest is allowed to write any
>> value directly to hardware. There is a tertiary control for
>> IA32_SPEC_CTRL. This control allows for bits in IA32_SPEC_CTRL to be
>> masked to prevent guests from changing those bits.
>>
>> Add controls setting the mask for IA32_SPEC_CTRL and desired value for
>> masked bits.
>>
>> These new controls are especially helpful for protecting guests that
>> don't know about BHI_DIS_S and that are running on hardware that
>> supports it. This allows the hypervisor to set BHI_DIS_S to fully
>> protect the guest.
>>
>> Suggested-by: Sean Christopherson <seanjc@google.com>
>> Signed-off-by: Daniel Sneddon <daniel.sneddon@linux.intel.com>
>> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
>> [ add a new ioctl to report supported bits. Fix the inverted check ]
>> Signed-off-by: Chao Gao <chao.gao@intel.com>
>
>This looks quite Intel-centric. Isn't this feature essentially the
>same as AMD's V_SPEC_CTRL?

Yes. they are almost the same. one small difference is intel's version can
force some bits off though I don't see how forcing bits off can be useful.

>Can't we consolidate the code, rather than
>having completely independent implementations for AMD and Intel?

We surely can consolidate the code. I will do this.

I have a question about V_SPEC_CTRL. w/ V_SPEC_CTRL, the SPEC_CTRL MSR retains
the host's value on VM-enter:

macro RESTORE_GUEST_SPEC_CTRL
        /* No need to do anything if SPEC_CTRL is unset or V_SPEC_CTRL is set */
        ALTERNATIVE_2 "", \
                "jmp 800f", X86_FEATURE_MSR_SPEC_CTRL, \
                "", X86_FEATURE_V_SPEC_CTRL

Does this mean all mitigations used by the host will be enabled for the guest
and guests cannot disable them?

Is this intentional? this looks suboptimal. Why not set SPEC_CTRL value to 0 and
let guest decide which features to enable? On the VMX side, we need host to
apply certain hardware mitigations (i.e., BHI_DIS_S and RRSBA_DIS_S) for guest
because BHI's software mitigation may be ineffective. I am not sure why SVM is
enabling all mitigations used by the host for guests. Wouldn't it be better to
enable them on an as-needed basis?