Hardware may or may not set exit_reason.bus_lock_detected on BUS_LOCK
VM-Exits. Dealing with KVM_RUN_X86_BUS_LOCK in handle_bus_lock_vmexit
could be redundant when exit_reason.basic is EXIT_REASON_BUS_LOCK.
We can remove redundant handling of bus lock vmexit. Force Setting
exit_reason.bus_lock_detected in handle_bus_lock_vmexit(), and deal with
KVM_RUN_X86_BUS_LOCK only in vmx_handle_exit().
Suggested-by: Sean Christopherson <[email protected]>
Signed-off-by: Hao Xiang <[email protected]>
---
v1 -> v2: a little modifications of comments
arch/x86/kvm/vmx/vmx.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 116b089..22be02e 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -5562,9 +5562,13 @@ static int handle_encls(struct kvm_vcpu *vcpu)
static int handle_bus_lock_vmexit(struct kvm_vcpu *vcpu)
{
- vcpu->run->exit_reason = KVM_EXIT_X86_BUS_LOCK;
- vcpu->run->flags |= KVM_RUN_X86_BUS_LOCK;
- return 0;
+ /*
+ * Hardware may or may not set the BUS_LOCK_DETECTED flag on BUS_LOCK
+ * VM-Exits, force setting the flag so that the logic in vmx_handle_exit()
+ * doesn't have to handle the flag and the basic exit reason.
+ */
+ to_vmx(vcpu)->exit_reason.bus_lock_detected = true;
+ return 1;
}
/*
@@ -6051,9 +6055,8 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath)
int ret = __vmx_handle_exit(vcpu, exit_fastpath);
/*
- * Even when current exit reason is handled by KVM internally, we
- * still need to exit to user space when bus lock detected to inform
- * that there is a bus lock in guest.
+ * Exit to user space when bus lock detected to inform that there is
+ * a bus lock in guest.
*/
if (to_vmx(vcpu)->exit_reason.bus_lock_detected) {
if (ret > 0)
--
1.8.3.1
On 10/15/2021 11:57 AM, Hao Xiang wrote:
> Hardware may or may not set exit_reason.bus_lock_detected on BUS_LOCK
> VM-Exits. Dealing with KVM_RUN_X86_BUS_LOCK in handle_bus_lock_vmexit
> could be redundant when exit_reason.basic is EXIT_REASON_BUS_LOCK.
>
> We can remove redundant handling of bus lock vmexit. Force Setting
> exit_reason.bus_lock_detected in handle_bus_lock_vmexit(), and deal with
> KVM_RUN_X86_BUS_LOCK only in vmx_handle_exit().
>
> Suggested-by: Sean Christopherson <[email protected]>
> Signed-off-by: Hao Xiang <[email protected]>
> ---
> v1 -> v2: a little modifications of comments
>
> arch/x86/kvm/vmx/vmx.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 116b089..22be02e 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -5562,9 +5562,13 @@ static int handle_encls(struct kvm_vcpu *vcpu)
>
> static int handle_bus_lock_vmexit(struct kvm_vcpu *vcpu)
> {
> - vcpu->run->exit_reason = KVM_EXIT_X86_BUS_LOCK;
> - vcpu->run->flags |= KVM_RUN_X86_BUS_LOCK;
> - return 0;
> + /*
> + * Hardware may or may not set the BUS_LOCK_DETECTED flag on BUS_LOCK
> + * VM-Exits, force setting the flag
> so that the logic in vmx_handle_exit()
> + * doesn't have to handle the flag and the basic exit reason.
The code itself looks good.
But the comment is partly incorrect. Now we rely only on the
bus_lock_detected flag so vmx_handle_exit() must check the flag.
> + */
> + to_vmx(vcpu)->exit_reason.bus_lock_detected = true;
> + return 1;
> }
>
> /*
> @@ -6051,9 +6055,8 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath)
> int ret = __vmx_handle_exit(vcpu, exit_fastpath);
>
> /*
> - * Even when current exit reason is handled by KVM internally, we
> - * still need to exit to user space when bus lock detected to inform
> - * that there is a bus lock in guest.
> + * Exit to user space when bus lock detected to inform that there is
> + * a bus lock in guest.
> */
> if (to_vmx(vcpu)->exit_reason.bus_lock_detected) {
> if (ret > 0)
>
On 2021/10/15 14:46, Xiaoyao Li wrote:
> On 10/15/2021 11:57 AM, Hao Xiang wrote:
>> Hardware may or may not set exit_reason.bus_lock_detected on BUS_LOCK
>> VM-Exits. Dealing with KVM_RUN_X86_BUS_LOCK in handle_bus_lock_vmexit
>> could be redundant when exit_reason.basic is EXIT_REASON_BUS_LOCK.
>>
>> We can remove redundant handling of bus lock vmexit. Force Setting
>> exit_reason.bus_lock_detected in handle_bus_lock_vmexit(), and deal with
>> KVM_RUN_X86_BUS_LOCK only in vmx_handle_exit().
>>
>> Suggested-by: Sean Christopherson <[email protected]>
>> Signed-off-by: Hao Xiang <[email protected]>
>> ---
>> v1 -> v2: a little modifications of comments
>>
>> arch/x86/kvm/vmx/vmx.c | 15 +++++++++------
>> 1 file changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>> index 116b089..22be02e 100644
>> --- a/arch/x86/kvm/vmx/vmx.c
>> +++ b/arch/x86/kvm/vmx/vmx.c
>> @@ -5562,9 +5562,13 @@ static int handle_encls(struct kvm_vcpu *vcpu)
>> static int handle_bus_lock_vmexit(struct kvm_vcpu *vcpu)
>> {
>> - vcpu->run->exit_reason = KVM_EXIT_X86_BUS_LOCK;
>> - vcpu->run->flags |= KVM_RUN_X86_BUS_LOCK;
>> - return 0;
>> + /*
>> + * Hardware may or may not set the BUS_LOCK_DETECTED flag on
>> BUS_LOCK
>> + * VM-Exits, force setting the flag
>
>> so that the logic in vmx_handle_exit()
>> + * doesn't have to handle the flag and the basic exit reason.
>
> The code itself looks good.
>
> But the comment is partly incorrect. Now we rely only on the
> bus_lock_detected flag so vmx_handle_exit() must check the flag.
>Thank you for you advice. Is the following comment ok?
/*
* Hardware may or may not set the BUS_LOCK_DETECTED flag on BUS_LOCK
* VM-Exits. Forcely set the flag here, then check the flag in
* vmx_handle_exit() and handle the bus lock vmexit if the flag is
* set.
*/
>> + */
>> + to_vmx(vcpu)->exit_reason.bus_lock_detected = true;
>> + return 1;
>> }
>> /*
>> @@ -6051,9 +6055,8 @@ static int vmx_handle_exit(struct kvm_vcpu
>> *vcpu, fastpath_t exit_fastpath)
>> int ret = __vmx_handle_exit(vcpu, exit_fastpath);
>> /*
>> - * Even when current exit reason is handled by KVM internally, we
>> - * still need to exit to user space when bus lock detected to inform
>> - * that there is a bus lock in guest.
>> + * Exit to user space when bus lock detected to inform that there is
>> + * a bus lock in guest.
>> */
>> if (to_vmx(vcpu)->exit_reason.bus_lock_detected) {
>> if (ret > 0)
>>
On 10/15/2021 3:28 PM, Hao Xiang wrote:
>
>
> On 2021/10/15 14:46, Xiaoyao Li wrote:
>> On 10/15/2021 11:57 AM, Hao Xiang wrote:
>>> Hardware may or may not set exit_reason.bus_lock_detected on BUS_LOCK
>>> VM-Exits. Dealing with KVM_RUN_X86_BUS_LOCK in handle_bus_lock_vmexit
>>> could be redundant when exit_reason.basic is EXIT_REASON_BUS_LOCK.
>>>
>>> We can remove redundant handling of bus lock vmexit. Force Setting
>>> exit_reason.bus_lock_detected in handle_bus_lock_vmexit(), and deal with
>>> KVM_RUN_X86_BUS_LOCK only in vmx_handle_exit().
>>>
>>> Suggested-by: Sean Christopherson <[email protected]>
>>> Signed-off-by: Hao Xiang <[email protected]>
>>> ---
>>> v1 -> v2: a little modifications of comments
>>>
>>> arch/x86/kvm/vmx/vmx.c | 15 +++++++++------
>>> 1 file changed, 9 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>>> index 116b089..22be02e 100644
>>> --- a/arch/x86/kvm/vmx/vmx.c
>>> +++ b/arch/x86/kvm/vmx/vmx.c
>>> @@ -5562,9 +5562,13 @@ static int handle_encls(struct kvm_vcpu *vcpu)
>>> static int handle_bus_lock_vmexit(struct kvm_vcpu *vcpu)
>>> {
>>> - vcpu->run->exit_reason = KVM_EXIT_X86_BUS_LOCK;
>>> - vcpu->run->flags |= KVM_RUN_X86_BUS_LOCK;
>>> - return 0;
>>> + /*
>>> + * Hardware may or may not set the BUS_LOCK_DETECTED flag on
>>> BUS_LOCK
>>> + * VM-Exits, force setting the flag
>>
>>> so that the logic in vmx_handle_exit()
>>> + * doesn't have to handle the flag and the basic exit reason.
>>
>> The code itself looks good.
>>
>> But the comment is partly incorrect. Now we rely only on the
>> bus_lock_detected flag so vmx_handle_exit() must check the flag.
>> Thank you for you advice. Is the following comment ok?
> /*
> * Hardware may or may not set the BUS_LOCK_DETECTED flag on BUS_LOCK
> * VM-Exits. Forcely set the flag here, then check the flag in
> * vmx_handle_exit() and handle the bus lock vmexit if the flag is
> * set.
> */
(I'm not sure if forcely is a valid word)
how about
/*
* Hardware may or may not set the BUS_LOCK_DETECTED flag on BUS_LOCK
* VM Exit. Unconditionally set the flag here and leave the handling to
* vmx_handle_exit().
*/
>>> + */
>>> + to_vmx(vcpu)->exit_reason.bus_lock_detected = true;
>>> + return 1;
>>> }
>>> /*
>>> @@ -6051,9 +6055,8 @@ static int vmx_handle_exit(struct kvm_vcpu
>>> *vcpu, fastpath_t exit_fastpath)
>>> int ret = __vmx_handle_exit(vcpu, exit_fastpath);
>>> /*
>>> - * Even when current exit reason is handled by KVM internally, we
>>> - * still need to exit to user space when bus lock detected to
>>> inform
>>> - * that there is a bus lock in guest.
>>> + * Exit to user space when bus lock detected to inform that
>>> there is
>>> + * a bus lock in guest.
>>> */
>>> if (to_vmx(vcpu)->exit_reason.bus_lock_detected) {
>>> if (ret > 0)
>>>