Received-SPF: pass (google.com: domain of linux-kernel+bounces-23751-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <cf043809-430a-4072-b0fd-201cd469b602@intel.com>
Date: Thu, 11 Jan 2024 22:56:55 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v8 00/26] Enable CET Virtualization
To: Sean Christopherson <seanjc@google.com>
CC: Rick P Edgecombe <rick.p.edgecombe@intel.com>, Chao Gao
	<chao.gao@intel.com>, Dave Hansen <dave.hansen@intel.com>,
	"peterz@infradead.org" <peterz@infradead.org>, "john.allen@amd.com"
	<john.allen@amd.com>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>, "pbonzini@redhat.com" <pbonzini@redhat.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>, "mlevitsk@redhat.com"
	<mlevitsk@redhat.com>
References: <20231221140239.4349-1-weijiang.yang@intel.com>
 <93f118670137933980e9ed263d01afdb532010ed.camel@intel.com>
 <5f57ce03-9568-4739-b02d-e9fac6ed381a@intel.com>
 <6179ddcb25c683bd178e74e7e2455cee63ba74de.camel@intel.com>
 <ZZdLG5W5u19PsnTo@google.com>
 <a2344e2143ef2b9eca0d153c86091e58e596709d.camel@intel.com>
 <ZZdSSzCqvd-3sdBL@google.com>
 <8f070910-2b2e-425d-995e-dfa03a7695de@intel.com>
 <ZZgsipXoXTKyvCZT@google.com>
 <06fdd362-cb7f-47df-9d1a-9b85d2ed05b5@intel.com>
 <ZZ1h9GW93ckc3FlE@google.com>
Content-Language: en-US
From: "Yang, Weijiang" <weijiang.yang@intel.com>
In-Reply-To: <ZZ1h9GW93ckc3FlE@google.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?U3VPSE5Hd3pFQ1BWZjNNZlorS2hFUUx2cTNrMGxQRHdpdCtUWmppWFgxUlk5?=
 =?utf-8?B?QU9xVFFTdGpFSVdYZERKeXhXemM4VE9Ub1d3WWhYNnovUkhPeDJ4T0U5djRj?=
 =?utf-8?B?SThKTVNEWTdOclc0WmQ3cG12YXkwWVJuU2t0eFg0RjJHK1FhaVo4OHBHNi9T?=
 =?utf-8?B?QXJaTkkzM1lKN2JYbWdJVnRDZW84dWNzTDhUeU9xYUNScXF2ZTlMQndJd1NY?=
 =?utf-8?B?MHVvUWlDaFJRRWhmWVZISFcxSnJCbXZHMTV5K1hjUVUxNXFhcytSWGwwclM3?=
 =?utf-8?B?TTNEQTRHdlpYd3NQWFBVNmhrd0hLa3ovS0tSUWxwRDJmZGhab0VkOS9INWZa?=
 =?utf-8?B?cVJQRmNIMTlSclpTcVhUT0xiT1Jjb1hvT0JoWGVRa05FWk8vK08xU1I1NFVL?=
 =?utf-8?B?VkhOK0FtOTFVaWRCeGtuQzdBYnpUODhvRjRLTmtpdzJFVUJJV1h5eUFMYzlK?=
 =?utf-8?B?RnIwTjlVK1pTN2pPWW5zcFY0VEgwUkdKWndCeHZ3M09pTmtKeTZPTi9qM1RE?=
 =?utf-8?B?MU9WK1dscG9OVy9FMTZwYms4N3lWbWQ5bXoyRG9wdnh6VTVMQ3daWkx0VHZr?=
 =?utf-8?B?cTFwT2lDN2xYT1E2VVRET0U4T0RMY2ZIZU13U3prT2ZwN0Eyd2kydWlwUHZn?=
 =?utf-8?B?UHdyQi9QRVNCVmJXaFFUZU9lUVJ2MlorNm9IZ0dDdWJCNDY5SGk1b1FvMVRT?=
 =?utf-8?B?eG9JcSszNTYxMytyaWVxYnRKTlRtL3NTMGhlUGRJMFMreUhTU3hoUlg1NFYy?=
 =?utf-8?B?c1dSYm1vYzU2UlBxQzdQdkhQWkIzVElsay9VQTlhSUNsNzQ2VFk2TURBUDd5?=
 =?utf-8?B?KzRucStJem93WVByL2hnaWJ6dTNzVi9QRmJHdjdOYnRQNXhaYUc0ZG8xc055?=
 =?utf-8?B?d3Z5NEpMRVlUSHNjUGZFc3FiRGhoQ2d4ZlFjQnM1Wm9WbzZqZXRaMDFvQm8x?=
 =?utf-8?B?bjJKU2JLcEtZR1BXVGRHc1UxNlFqV25qVnp5b1AvZWNreXk2eWJuUlVsdVNm?=
 =?utf-8?B?QXNBVkhTSFVla0ZscWtJK3dvNmtTUFl0RTV2bHNkR1NnRHc2N012R21zOTc1?=
 =?utf-8?B?MmJobjR0NzQyeDFoN2Zzell2T2kvV3pQbW5HcDJoWTAvc0k3OEIrUTZoZ0p1?=
 =?utf-8?B?cTNrYm9lVkp5VUVwek11c3krZ1dBUlQ4YkFMMUFud21DMGF1Q3hMSW1EbU5z?=
 =?utf-8?B?dEM3Q1Zld2k3RXJsTmlBK0NCT28xY3Z2NHFQZlR5UE9NemtqMThRekFtZDBk?=
 =?utf-8?B?ZzZiU2YxaXJLdHIvTUY1ZTl3VCt3MUFwQkVQNmpGcWx4cGpwazUzSWtpbEx6?=
 =?utf-8?B?cVNmT3pqcEpXbTZCNVdZY2ZIK09TU1ozRG9sbm1CMVd1WmR2OHRYL1JrM3hT?=
 =?utf-8?B?UTVNNktLSVBaNUxDMWQzSlhaT2xaSXVIREFwQlBvZ2wvUHQrWDRKNURFaFpL?=
 =?utf-8?B?SU84NnBmMk9KUjNUdFQyOVRudEFNV2RzKzI3TEx4R2lIWnVPbkduWWlFU3Qv?=
 =?utf-8?B?RnhoWlFpRXJIeHNNT2RPVWhPcHlVSklVeFpjTE10UFBZZzBNUS8xUGh3eHE0?=
 =?utf-8?B?ekx3ZHF0TjVZdDhEckF5TnM2a0JFOVdwTXhXaXd3eENTTXg5NGVyWmlHcTdz?=
 =?utf-8?B?Q2VxQ0NNdS9WZXh0SlZEdjJpV2g0RkFmd2pxblhmR2FPbnVXWVlrelNjUWVl?=
 =?utf-8?B?eWxLbUV3dE1NYnlTeEJ6Ynl1RlQxcW5LRk9CaEI3bmRtcWZRQXByVnF1S3h4?=
 =?utf-8?B?TXN6V0p5TWlJdURGdkNRSDQzTVpTKzNZT2REeEZucXJVc3k3TzdiRW42bHJJ?=
 =?utf-8?B?MjYxUHRhQmFHdXZHeDdSb2FKSStKTE9wWlhhQ3VkdG5qakJpNjIxbCtISWd5?=
 =?utf-8?B?N2VkeVA2TXdRQ0phVC8wTlp0dzNHR25Ddkd0WXNQRW1vWFBrTEFtRXBveHVS?=
 =?utf-8?B?NUxIaDQ4dWRMWFRRRzJvL0pGQzQyeEN3RTNVcjFmenBmTkRIaWtFOGM3MGly?=
 =?utf-8?B?TkdGMlZML2NrY1RON0Jhbk43RFNVS1hYT0tPVXR3WmNDWFBieC8yRmxzT25Q?=
 =?utf-8?B?aDVZdGc1M0VlMko2OHE3aXVZZmNzVlZzcHpDVkszNDB5ZFhOZDRvU3dsYVdh?=
 =?utf-8?B?c1ZVSlBZRkFVK1A0MG0zZkJTck16WnVJWHBTeWd3bFh0ekxkR1dJeTZKaHpu?=
 =?utf-8?B?NlE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: f048a5aa-51d8-4b5f-0158-08dc12b594d2
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB4965.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Jan 2024 14:57:07.8849
 (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: t17Vx6N3hp2xx9cJHwV7hP2kauZSX7o5QYpKZPMtT2H4RECC67wfxTUWElcmysWfEj0buUa94HAPsw6ESfyRcw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR11MB5186
X-OriginatorOrg: intel.com

On 1/9/2024 11:10 PM, Sean Christopherson wrote:
> On Mon, Jan 08, 2024, Weijiang Yang wrote:
>> On 1/6/2024 12:21 AM, Sean Christopherson wrote:
>>> On Fri, Jan 05, 2024, Weijiang Yang wrote:
>>>> On 1/5/2024 8:54 AM, Sean Christopherson wrote:
>>>>> On Fri, Jan 05, 2024, Rick P Edgecombe wrote:
>>>>>>> For CALL/RET (and presumably any branch instructions with IBT?) other
>>>>>>> instructions that are directly affected by CET, the simplest thing would
>>>>>>> probably be to disable those in KVM's emulator if shadow stacks and/or IBT
>>>>>>> are enabled, and let KVM's failure paths take it from there.
>>>>>> Right, that is what I was wondering might be the normal solution for
>>>>>> situations like this.
>>>>> If KVM can't emulate something, it either retries the instruction (with some
>>>>> decent logic to guard against infinite retries) or punts to userspace.
>>>> What kind of error is proper if KVM has to punt to userspace?
>>> KVM_INTERNAL_ERROR_EMULATION.  See prepare_emulation_failure_exit().
>>>
>>>> Or just inject #UD into guest on detecting this case?
>>> No, do not inject #UD or do anything else that deviates from architecturally
>>> defined behavior.
>> Thanks!
>> But based on current KVM implementation and patch 24, seems that if CET is exposed
>> to guest, the emulation code or shadow paging mode couldn't be activated at the same time:
> No, requiring unrestricted guest only disables the paths where KVM *delibeately*
> emulates the entire guest code stream.  In no way, shape, or form does it prevent
> KVM from attempting to emulate arbitrary instructions.

Yes, also need to prevent sporadic emulation, how about adding below patch in emulator?


diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index e223043ef5b2..e817d8560ceb 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -178,6 +178,7 @@
  #define IncSP       ((u64)1 << 54)  /* SP is incremented before ModRM calc */
  #define TwoMemOp    ((u64)1 << 55)  /* Instruction has two memory operand */
  #define IsBranch    ((u64)1 << 56)  /* Instruction is considered a branch. */
+#define IsProtected ((u64)1 << 57)  /* Instruction is protected by CET. */

  #define DstXacc     (DstAccLo | SrcAccHi | SrcWrite)

@@ -4098,9 +4099,9 @@ static const struct opcode group4[] = {
  static const struct opcode group5[] = {
         F(DstMem | SrcNone | Lock,              em_inc),
         F(DstMem | SrcNone | Lock,              em_dec),
-       I(SrcMem | NearBranch | IsBranch,       em_call_near_abs),
-       I(SrcMemFAddr | ImplicitOps | IsBranch, em_call_far),
-       I(SrcMem | NearBranch | IsBranch,       em_jmp_abs),
+       I(SrcMem | NearBranch | IsBranch | IsProtected, em_call_near_abs),
+       I(SrcMemFAddr | ImplicitOps | IsBranch | IsProtected, em_call_far),
+       I(SrcMem | NearBranch | IsBranch | IsProtected, em_jmp_abs),
         I(SrcMemFAddr | ImplicitOps | IsBranch, em_jmp_far),
         I(SrcMem | Stack | TwoMemOp,            em_push), D(Undefined),
  };
@@ -4362,11 +4363,11 @@ static const struct opcode opcode_table[256] = {
         /* 0xC8 - 0xCF */
         I(Stack | SrcImmU16 | Src2ImmByte | IsBranch, em_enter),
         I(Stack | IsBranch, em_leave),
-       I(ImplicitOps | SrcImmU16 | IsBranch, em_ret_far_imm),
-       I(ImplicitOps | IsBranch, em_ret_far),
-       D(ImplicitOps | IsBranch), DI(SrcImmByte | IsBranch, intn),
+       I(ImplicitOps | SrcImmU16 | IsBranch | IsProtected, em_ret_far_imm),
+       I(ImplicitOps | IsBranch | IsProtected, em_ret_far),
+       D(ImplicitOps | IsBranch), DI(SrcImmByte | IsBranch | IsProtected, intn),
         D(ImplicitOps | No64 | IsBranch),
-       II(ImplicitOps | IsBranch, em_iret, iret),
+       II(ImplicitOps | IsBranch | IsProtected, em_iret, iret),
         /* 0xD0 - 0xD7 */
         G(Src2One | ByteOp, group2), G(Src2One, group2),
         G(Src2CL | ByteOp, group2), G(Src2CL, group2),
@@ -4382,7 +4383,7 @@ static const struct opcode opcode_table[256] = {
         I2bvIP(SrcImmUByte | DstAcc, em_in,  in,  check_perm_in),
         I2bvIP(SrcAcc | DstImmUByte, em_out, out, check_perm_out),
         /* 0xE8 - 0xEF */
-       I(SrcImm | NearBranch | IsBranch, em_call),
+       I(SrcImm | NearBranch | IsBranch | IsProtected, em_call),
         D(SrcImm | ImplicitOps | NearBranch | IsBranch),
         I(SrcImmFAddr | No64 | IsBranch, em_jmp_far),
         D(SrcImmByte | ImplicitOps | NearBranch | IsBranch),
@@ -4401,7 +4402,7 @@ static const struct opcode opcode_table[256] = {
  static const struct opcode twobyte_table[256] = {
         /* 0x00 - 0x0F */
         G(0, group6), GD(0, &group7), N, N,
-       N, I(ImplicitOps | EmulateOnUD | IsBranch, em_syscall),
+       N, I(ImplicitOps | EmulateOnUD | IsBranch | IsProtected, em_syscall),
         II(ImplicitOps | Priv, em_clts, clts), N,
         DI(ImplicitOps | Priv, invd), DI(ImplicitOps | Priv, wbinvd), N, N,
         N, D(ImplicitOps | ModRM | SrcMem | NoAccess), N, N,
@@ -4432,8 +4433,8 @@ static const struct opcode twobyte_table[256] = {
         IIP(ImplicitOps, em_rdtsc, rdtsc, check_rdtsc),
         II(ImplicitOps | Priv, em_rdmsr, rdmsr),
         IIP(ImplicitOps, em_rdpmc, rdpmc, check_rdpmc),
-       I(ImplicitOps | EmulateOnUD | IsBranch, em_sysenter),
-       I(ImplicitOps | Priv | EmulateOnUD | IsBranch, em_sysexit),
+       I(ImplicitOps | EmulateOnUD | IsBranch | IsProtected, em_sysenter),
+       I(ImplicitOps | Priv | EmulateOnUD | IsBranch | IsProtected, em_sysexit),
         N, N,
         N, N, N, N, N, N, N, N,
         /* 0x40 - 0x4F */
@@ -4971,6 +4972,12 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len, int
         if (ctxt->d == 0)
                 return EMULATION_FAILED;
+       if ((opcode.flags & IsProtected) &&
+           (ctxt->ops->get_cr(ctxt, 4) & X86_CR4_CET)) {
+               WARN_ONCE(1, "CET is active, emulation aborted.\n");
+               return EMULATION_FAILED;
+       }
+
         ctxt->execute = opcode.u.execute;

         if (unlikely(emulation_type & EMULTYPE_TRAP_UD) &&