Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <7b4d5df0-f554-2fc0-5c19-021f8eb3f6aa@amd.com>
Date:   Wed, 2 Aug 2023 10:26:31 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [Question] int3 instruction generates a #UD in SEV VM
Content-Language: en-US
To:     Sean Christopherson <seanjc@google.com>
Cc:     Wu Zongyo <wuzongyo@mail.ustc.edu.cn>,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org, x86@kernel.org,
        linux-coco@lists.linux.dev
References: <8eb933fd-2cf3-d7a9-32fe-2a1d82eac42a@mail.ustc.edu.cn>
 <ZMfFaF2M6Vrh/QdW@google.com> <4ebb3e20-a043-8ad3-ef6c-f64c2443412c@amd.com>
 <544b7f95-4b34-654d-a57b-3791a6f4fd5f@mail.ustc.edu.cn>
 <ZMpEUVsv5hSmrcH8@iZuf6hx7901barev1c282cZ> <ZMphvF+0H9wHQr5B@google.com>
 <bbc52f40-2661-3fa2-8e09-bec772728812@amd.com>
 <7a4f3f59-1482-49c4-92b2-aa621e9b06b3@amd.com> <ZMpwiSw9CBZh9xcc@google.com>
From:   Tom Lendacky <thomas.lendacky@amd.com>
In-Reply-To: <ZMpwiSw9CBZh9xcc@google.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?R0s0Z0NVZXo1VTA0REk5NFRLaUhNQXF4WlVXaDl5b0JNbGVHL1h1QzEyZlE5?=
 =?utf-8?B?bGZjVmhweVJVQlJ6Q3VLOGJjRmNEVU8rd3pWNE8zL2ZHVitWMTY3R0FMc0tu?=
 =?utf-8?B?NUYwRC9hcU9uRzA5czQ5bkJNbGFNYWZFWWV0a0htb0JLMXljVGsvS1J1aVU1?=
 =?utf-8?B?b0hmaTMwdEYvQ2ZqOUY4d0dOSVZKdVhmc2d4cUlVTUF5UHFxL0dtcGtCR0NU?=
 =?utf-8?B?MWdZTVZTZmJJWm1IR2ZmMjF3LzdBanQxSXR4L1FnTUtZdFVUNlR0UFNpWUVM?=
 =?utf-8?B?eUdRNGd4RXpZUndSREQ4OFJhVGtKblJDb2VyMkpSK0xzdTEwVDFOK1BuanhB?=
 =?utf-8?B?czhrVUg0bFRFWVk3a05vcGNxQjhqdndRZWdDc2ZZd040NlBMNENkNk5VVlpj?=
 =?utf-8?B?THFzV3B6bnEzbEdXSWdvbTRtQ0JxNlBzRE5nQnZMbS9yRTFDUTI4QWZOVFdi?=
 =?utf-8?B?cndJSjY3bVdLajhOWTYvcGFIbit4ajFkcnVOUGJSMnFFbUViS0NnK1dNWlh1?=
 =?utf-8?B?VlJ4THI0TEN0djhMdWJCY1BMOWFrK0E4YkpRNjQ4N1Y3ekJ0UXhtUkwyMTVO?=
 =?utf-8?B?QnVJemZWY3ZINnRWM3J2VnkxOEtaSWN3NTZyZ0RYaUFraUVRQytUa3dsSUtR?=
 =?utf-8?B?ZkV6WkRCVVJ3ZHhMeklkcTBQM1FhUndlbzkyUllTREcyMHg5bXcxSHhwYzhJ?=
 =?utf-8?B?MURhQ0VMTDlQR0NlRnl6eko2MkQ3UERoWUVmU2JXcGM3WVhmdEVDYzF5cUc0?=
 =?utf-8?B?VzZQcGxDQmlsN0kyTGNLQW5mUkRTTmdobDlrZGgwd0JzbW9YZmFFS0NONGRR?=
 =?utf-8?B?OVVIWmpEM0tOT0JmeWhmNXJsd0xrcSs0d1RGNU01cnhVMzY0Zmh2NGJOdVhw?=
 =?utf-8?B?dXA3ZDFPSnE3a1FwUG5HeW5CemlzWlVaOGFmNEJoV2JtN25VK1JnM0M3Y3FP?=
 =?utf-8?B?Ujl1ckhNQkxlblV6Q3Q0QTIzT1JBUVQzMElBWFdsQVk3UDdxRDBnY1Arcm9P?=
 =?utf-8?B?MGlJbjBENUlTMDdEcEQyVWg5TnBWMjNDeWtJVG4rSFo2RmtWQkhhekI2QjlR?=
 =?utf-8?B?TXE3bmNBaU5rUnIxVHlhV1dPVWpHcEg3K0c2VmI5UWdtVFpoWi96dURJb01u?=
 =?utf-8?B?LzBHa2VZR09abncvZ25FaktxVHVDbFpITkhyQnF4SzZvVkVXakhINkZGNnFB?=
 =?utf-8?B?Z1MwTzBmbnpEb2IvVG9RQTcvOUdjUU1iYk1senJNUjBQTUsrcnk3cERQN3lh?=
 =?utf-8?B?VUpYS2E2RjlldlhtdHhXTUZqeFI5b3R3VEdrZUR2T1Vieno3WElkSVdRd3NH?=
 =?utf-8?B?MUthbm1PcEh5TGRGbmovODZoRXZQZkgzcEw4V0NFVWxiejdCaW8wZVhWVkFw?=
 =?utf-8?B?b1BiYVJuTzlkN1daSkpKOXpnUFk0c2ZBWWp6ckI0a25SaVhvcmQxRmpPTGVW?=
 =?utf-8?B?UGVPY2JueS9mMlhXdU51RHAxend6Mncyc05FQlhQaWVoVjZKbGpadUk0MHA4?=
 =?utf-8?B?OFNmTi9jVVRUdEVwS3AwVEU2WTZxVzBaM1d6a0Q5V0gvUE95NFdhQXhLR3dx?=
 =?utf-8?B?NzBvVjROSnBPTng2ejZtb0pLY2U3dHVwdUZ4dEFRZ1IxcVR0cFVBNGx6N1pT?=
 =?utf-8?B?Nkwwdy9udWRidnQ1aWtHV09LNEFadVZjcHNkMVN0RWE5ZnJ6UDhFNmVlU1ox?=
 =?utf-8?B?SEpGazY1cThaMlMxalZoaThkakVQb2YrV3E4V0xIbkdvbjkwNjZTYS9JWTZl?=
 =?utf-8?B?Rm1paEMzMytRREdMZmR5UFhVVWZCeEQzTkphOFp5TWtVSHNIb2dzNlkvbk9B?=
 =?utf-8?B?dG8yUkVoMmRyaGx4dWVWQVhlOUgyeDVZRWhyejlDSFJKbHU3WDlwZEh2cVg0?=
 =?utf-8?B?ekpXc2x3TG5mbjBQMVZqVkM3Y091Z05QaEdMSTA4MDJZWnpYNCtCV1E5bzQx?=
 =?utf-8?B?NVh5dHNHcm5qMjV4RDlaWHQwR3YxdjJyZW5DZjNqYzNxSnVsb3RMa25sQ1V4?=
 =?utf-8?B?em5rczdBUWMyOVBON3l4ZTBGTVo0MSswbVkvR2dPTzFReDI2Mk0rbW5lSU9T?=
 =?utf-8?B?RW9NaVVRSytQSGV4cVhITC92WVMvQjhRT1hDN2JRbng5RU96VjlxM1NVSEtF?=
 =?utf-8?Q?TDr3LsJ93p0zHw7njFegrHk+V?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2c53dc0e-2675-400d-aeff-08db936cda88
X-MS-Exchange-CrossTenant-AuthSource: DM4PR12MB5229.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Aug 2023 15:26:33.6462
 (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: L3yh+FLEZHb1bFbB7iqcBOTRrg1aacAy0ax+vpHZyGW+zEBr6MRt6iaCzkXMc15c9ZiAdkdul8PwI6nmKUftRA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL3PR12MB6377
X-Spam-Status: No, score=-1.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,NICE_REPLY_A,
        RCVD_IN_DNSWL_BLOCKED,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=no autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 8/2/23 10:04, Sean Christopherson wrote:
> On Wed, Aug 02, 2023, Tom Lendacky wrote:
>> On 8/2/23 09:25, Tom Lendacky wrote:
>>> On 8/2/23 09:01, Sean Christopherson wrote:
>>>>> You're right. The #UD is injected by KVM.
>>>>>
>>>>> The path I found is:
>>>>>       svm_vcpu_run
>>>>>           svm_complete_interrupts
>>>>>          kvm_requeue_exception // vector = 3
>>>>>              kvm_make_request
>>>>>
>>>>>       vcpu_enter_guest
>>>>>           kvm_check_and_inject_events
>>>>>          svm_inject_exception
>>>>>              svm_update_soft_interrupt_rip
>>>>>              __svm_skip_emulated_instruction
>>>>>                  x86_emulate_instruction
>>>>>                  svm_can_emulate_instruction
>>>>>                      kvm_queue_exception(vcpu, UD_VECTOR)
>>>>>
>>>>> Does this mean a #PF intercept occur when the guest try to deliver a
>>>>> #BP through the IDT? But why?
>>>>
>>>> I doubt it's a #PF.  A #NPF is much more likely, though it could be
>>>> something
>>>> else entirely, but I'm pretty sure that would require bugs in both
>>>> the host and
>>>> guest.
>>>>
>>>> What is the last exit recorded by trace_kvm_exit() before the #UD is
>>>> injected?
>>>
>>> I'm guessing it was a #NPF, too. Could it be related to the changes that
>>> went in around svm_update_soft_interrupt_rip()?
>>>
>>> 6ef88d6e36c2 ("KVM: SVM: Re-inject INT3/INTO instead of retrying the
>>> instruction")
>>
>> Sorry, that should have been:
>>
>> 7e5b5ef8dca3 ("KVM: SVM: Re-inject INTn instead of retrying the insn on "failure"")
>>
>>>
>>> Before this the !nrips check would prevent the call into
>>> svm_skip_emulated_instruction(). But now, there is a call to:
>>>
>>>     svm_update_soft_interrupt_rip()
>>>       __svm_skip_emulated_instruction()
>>>         kvm_emulate_instruction()
>>>           x86_emulate_instruction() (passed a NULL insn pointer)
>>>             kvm_can_emulate_insn() (passed a NULL insn pointer)
>>>               svm_can_emulate_instruction() (passed NULL insn pointer)
>>>
>>> Because it is an SEV guest, it ends up in the "if (unlikely(!insn))" path
>>> and injects the #UD.
> 
> Yeah, my money is on that too.  I believe this is the least awful solution:
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index d381ad424554..2eace114a934 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -385,6 +385,9 @@ static int __svm_skip_emulated_instruction(struct kvm_vcpu *vcpu,
>          }
>   
>          if (!svm->next_rip) {
> +               if (sev_guest(vcpu->kvm))
> +                       return 0;
> +
>                  if (unlikely(!commit_side_effects))
>                          old_rflags = svm->vmcb->save.rflags;
>   
> I'll send a formal patch (with a comment) if that solves the problem.
> 
> 
> Side topic, KVM should require nrips for SEV and beyond, I don't see how SEV can
> possibly work if KVM doesn't utilize nrips.  E.g. this
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 2eace114a934..43e500503d48 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -5111,9 +5111,11 @@ static __init int svm_hardware_setup(void)
>   
>          svm_adjust_mmio_mask();
>   
> +       nrips = nrips && boot_cpu_has(X86_FEATURE_NRIPS);
> +
>          /*
>           * Note, SEV setup consumes npt_enabled and enable_mmio_caching (which
> -        * may be modified by svm_adjust_mmio_mask()).
> +        * may be modified by svm_adjust_mmio_mask()), as well as nrips.
>           */
>          sev_hardware_setup();

You moved the setting of nrips up, I'm assuming you then want to add a 
check in sev_hardware_setup() for nrips?

Thanks,
Tom

>   
> @@ -5125,11 +5127,6 @@ static __init int svm_hardware_setup(void)
>                          goto err;
>          }
>   
> -       if (nrips) {
> -               if (!boot_cpu_has(X86_FEATURE_NRIPS))
> -                       nrips = false;
> -       }
> -
>          enable_apicv = avic = avic && avic_hardware_setup();
>   
>          if (!enable_apicv) {
>