* Re: [PATCH v2] KVM: SVM: Disable AVIC before setting V_IRQ
2020-05-07 2:35 [PATCH v2] KVM: SVM: Disable AVIC before setting V_IRQ Suravee Suthikulpanit
@ 2020-05-07 8:27 ` Paolo Bonzini
2020-05-10 12:13 ` Maxim Levitsky
2020-05-08 3:42 ` kbuild test robot
2020-05-08 15:32 ` kbuild test robot
2 siblings, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2020-05-07 8:27 UTC (permalink / raw)
To: Suravee Suthikulpanit, linux-kernel, kvm; +Cc: joro, jon.grimm, mlevitsk
On 07/05/20 04:35, Suravee Suthikulpanit wrote:
> The commit 64b5bd270426 ("KVM: nSVM: ignore L1 interrupt window
> while running L2 with V_INTR_MASKING=1") introduced a WARN_ON,
> which checks if AVIC is enabled when trying to set V_IRQ
> in the VMCB for enabling irq window.
>
> The following warning is triggered because the requesting vcpu
> (to deactivate AVIC) does not get to process APICv update request
> for itself until the next #vmexit.
>
> WARNING: CPU: 0 PID: 118232 at arch/x86/kvm/svm/svm.c:1372 enable_irq_window+0x6a/0xa0 [kvm_amd]
> RIP: 0010:enable_irq_window+0x6a/0xa0 [kvm_amd]
> Call Trace:
> kvm_arch_vcpu_ioctl_run+0x6e3/0x1b50 [kvm]
> ? kvm_vm_ioctl_irq_line+0x27/0x40 [kvm]
> ? _copy_to_user+0x26/0x30
> ? kvm_vm_ioctl+0xb3e/0xd90 [kvm]
> ? set_next_entity+0x78/0xc0
> kvm_vcpu_ioctl+0x236/0x610 [kvm]
> ksys_ioctl+0x8a/0xc0
> __x64_sys_ioctl+0x1a/0x20
> do_syscall_64+0x58/0x210
> entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
> Fixes by sending APICV update request to all other vcpus, and
> immediately update APIC for itself.
>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Link: https://lkml.org/lkml/2020/5/2/167
> Fixes: 64b5bd270426 ("KVM: nSVM: ignore L1 interrupt window while running L2 with V_INTR_MASKING=1")
> ---
> arch/x86/kvm/x86.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index df473f9..69a01ea 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -8085,6 +8085,7 @@ void kvm_vcpu_update_apicv(struct kvm_vcpu *vcpu)
> */
> void kvm_request_apicv_update(struct kvm *kvm, bool activate, ulong bit)
> {
> + struct kvm_vcpu *except;
> unsigned long old, new, expected;
>
> if (!kvm_x86_ops.check_apicv_inhibit_reasons ||
> @@ -8110,7 +8111,17 @@ void kvm_request_apicv_update(struct kvm *kvm, bool activate, ulong bit)
> trace_kvm_apicv_update_request(activate, bit);
> if (kvm_x86_ops.pre_update_apicv_exec_ctrl)
> kvm_x86_ops.pre_update_apicv_exec_ctrl(kvm, activate);
> - kvm_make_all_cpus_request(kvm, KVM_REQ_APICV_UPDATE);
> +
> + /*
> + * Sending request to update APICV for all other vcpus,
> + * while update the calling vcpu immediately instead of
> + * waiting for another #VMEXIT to handle the request.
> + */
> + except = kvm_get_running_vcpu();
> + kvm_make_all_cpus_request_except(kvm, KVM_REQ_APICV_UPDATE,
> + except);
> + if (except)
> + kvm_vcpu_update_apicv(except);
> }
> EXPORT_SYMBOL_GPL(kvm_request_apicv_update);
>
>
Queued, thanks.
Paolo
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] KVM: SVM: Disable AVIC before setting V_IRQ
2020-05-07 8:27 ` Paolo Bonzini
@ 2020-05-10 12:13 ` Maxim Levitsky
0 siblings, 0 replies; 5+ messages in thread
From: Maxim Levitsky @ 2020-05-10 12:13 UTC (permalink / raw)
To: Paolo Bonzini, Suravee Suthikulpanit, linux-kernel, kvm; +Cc: joro, jon.grimm
On Thu, 2020-05-07 at 10:27 +0200, Paolo Bonzini wrote:
> On 07/05/20 04:35, Suravee Suthikulpanit wrote:
> > The commit 64b5bd270426 ("KVM: nSVM: ignore L1 interrupt window
> > while running L2 with V_INTR_MASKING=1") introduced a WARN_ON,
> > which checks if AVIC is enabled when trying to set V_IRQ
> > in the VMCB for enabling irq window.
> >
> > The following warning is triggered because the requesting vcpu
> > (to deactivate AVIC) does not get to process APICv update request
> > for itself until the next #vmexit.
> >
> > WARNING: CPU: 0 PID: 118232 at arch/x86/kvm/svm/svm.c:1372 enable_irq_window+0x6a/0xa0 [kvm_amd]
> > RIP: 0010:enable_irq_window+0x6a/0xa0 [kvm_amd]
> > Call Trace:
> > kvm_arch_vcpu_ioctl_run+0x6e3/0x1b50 [kvm]
> > ? kvm_vm_ioctl_irq_line+0x27/0x40 [kvm]
> > ? _copy_to_user+0x26/0x30
> > ? kvm_vm_ioctl+0xb3e/0xd90 [kvm]
> > ? set_next_entity+0x78/0xc0
> > kvm_vcpu_ioctl+0x236/0x610 [kvm]
> > ksys_ioctl+0x8a/0xc0
> > __x64_sys_ioctl+0x1a/0x20
> > do_syscall_64+0x58/0x210
> > entry_SYSCALL_64_after_hwframe+0x44/0xa9
> >
> > Fixes by sending APICV update request to all other vcpus, and
> > immediately update APIC for itself.
> >
> > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> > Link: https://lkml.org/lkml/2020/5/2/167
> > Fixes: 64b5bd270426 ("KVM: nSVM: ignore L1 interrupt window while running L2 with V_INTR_MASKING=1")
> > ---
> > arch/x86/kvm/x86.c | 13 ++++++++++++-
> > 1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index df473f9..69a01ea 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -8085,6 +8085,7 @@ void kvm_vcpu_update_apicv(struct kvm_vcpu *vcpu)
> > */
> > void kvm_request_apicv_update(struct kvm *kvm, bool activate, ulong bit)
> > {
> > + struct kvm_vcpu *except;
> > unsigned long old, new, expected;
> >
> > if (!kvm_x86_ops.check_apicv_inhibit_reasons ||
> > @@ -8110,7 +8111,17 @@ void kvm_request_apicv_update(struct kvm *kvm, bool activate, ulong bit)
> > trace_kvm_apicv_update_request(activate, bit);
> > if (kvm_x86_ops.pre_update_apicv_exec_ctrl)
> > kvm_x86_ops.pre_update_apicv_exec_ctrl(kvm, activate);
> > - kvm_make_all_cpus_request(kvm, KVM_REQ_APICV_UPDATE);
> > +
> > + /*
> > + * Sending request to update APICV for all other vcpus,
> > + * while update the calling vcpu immediately instead of
> > + * waiting for another #VMEXIT to handle the request.
> > + */
> > + except = kvm_get_running_vcpu();
> > + kvm_make_all_cpus_request_except(kvm, KVM_REQ_APICV_UPDATE,
> > + except);
> > + if (except)
> > + kvm_vcpu_update_apicv(except);
> > }
> > EXPORT_SYMBOL_GPL(kvm_request_apicv_update);
> >
> >
>
> Queued, thanks.
>
> Paolo
>
I tested this patch today on top of kvm/queue,
the patch that add kvm_make_all_cpus_request_except and this patch
(the former patch needs slight adjustment to apply).
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] KVM: SVM: Disable AVIC before setting V_IRQ
2020-05-07 2:35 [PATCH v2] KVM: SVM: Disable AVIC before setting V_IRQ Suravee Suthikulpanit
2020-05-07 8:27 ` Paolo Bonzini
@ 2020-05-08 3:42 ` kbuild test robot
2020-05-08 15:32 ` kbuild test robot
2 siblings, 0 replies; 5+ messages in thread
From: kbuild test robot @ 2020-05-08 3:42 UTC (permalink / raw)
To: Suravee Suthikulpanit, linux-kernel, kvm, pbonzini, joro,
jon.grimm, mlevitsk
Cc: kbuild-all, clang-built-linux, pbonzini, joro, jon.grimm, mlevitsk
[-- Attachment #1: Type: text/plain, Size: 3704 bytes --]
Hi Suravee,
I love your patch! Yet something to improve:
[auto build test ERROR on kvm/linux-next]
[also build test ERROR on vhost/linux-next linus/master v5.7-rc4 next-20200507]
[cannot apply to linux/master]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Suravee-Suthikulpanit/KVM-SVM-Disable-AVIC-before-setting-V_IRQ/20200507-111704
base: https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 54b35c066417d4856e9d53313f7e98b354274584)
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> arch/x86/kvm/x86.c:8107:2: error: implicit declaration of function 'kvm_make_all_cpus_request_except' [-Werror,-Wimplicit-function-declaration]
kvm_make_all_cpus_request_except(kvm, KVM_REQ_APICV_UPDATE,
^
arch/x86/kvm/x86.c:8107:2: note: did you mean 'kvm_make_all_cpus_request'?
include/linux/kvm_host.h:818:6: note: 'kvm_make_all_cpus_request' declared here
bool kvm_make_all_cpus_request(struct kvm *kvm, unsigned int req);
^
1 error generated.
vim +/kvm_make_all_cpus_request_except +8107 arch/x86/kvm/x86.c
8065
8066 /*
8067 * NOTE: Do not hold any lock prior to calling this.
8068 *
8069 * In particular, kvm_request_apicv_update() expects kvm->srcu not to be
8070 * locked, because it calls __x86_set_memory_region() which does
8071 * synchronize_srcu(&kvm->srcu).
8072 */
8073 void kvm_request_apicv_update(struct kvm *kvm, bool activate, ulong bit)
8074 {
8075 struct kvm_vcpu *except;
8076 unsigned long old, new, expected;
8077
8078 if (!kvm_x86_ops.check_apicv_inhibit_reasons ||
8079 !kvm_x86_ops.check_apicv_inhibit_reasons(bit))
8080 return;
8081
8082 old = READ_ONCE(kvm->arch.apicv_inhibit_reasons);
8083 do {
8084 expected = new = old;
8085 if (activate)
8086 __clear_bit(bit, &new);
8087 else
8088 __set_bit(bit, &new);
8089 if (new == old)
8090 break;
8091 old = cmpxchg(&kvm->arch.apicv_inhibit_reasons, expected, new);
8092 } while (old != expected);
8093
8094 if (!!old == !!new)
8095 return;
8096
8097 trace_kvm_apicv_update_request(activate, bit);
8098 if (kvm_x86_ops.pre_update_apicv_exec_ctrl)
8099 kvm_x86_ops.pre_update_apicv_exec_ctrl(kvm, activate);
8100
8101 /*
8102 * Sending request to update APICV for all other vcpus,
8103 * while update the calling vcpu immediately instead of
8104 * waiting for another #VMEXIT to handle the request.
8105 */
8106 except = kvm_get_running_vcpu();
> 8107 kvm_make_all_cpus_request_except(kvm, KVM_REQ_APICV_UPDATE,
8108 except);
8109 if (except)
8110 kvm_vcpu_update_apicv(except);
8111 }
8112 EXPORT_SYMBOL_GPL(kvm_request_apicv_update);
8113
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 72774 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] KVM: SVM: Disable AVIC before setting V_IRQ
2020-05-07 2:35 [PATCH v2] KVM: SVM: Disable AVIC before setting V_IRQ Suravee Suthikulpanit
2020-05-07 8:27 ` Paolo Bonzini
2020-05-08 3:42 ` kbuild test robot
@ 2020-05-08 15:32 ` kbuild test robot
2 siblings, 0 replies; 5+ messages in thread
From: kbuild test robot @ 2020-05-08 15:32 UTC (permalink / raw)
To: Suravee Suthikulpanit, linux-kernel, kvm, pbonzini, joro,
jon.grimm, mlevitsk
Cc: kbuild-all, pbonzini, joro, jon.grimm, mlevitsk
[-- Attachment #1: Type: text/plain, Size: 3224 bytes --]
Hi Suravee,
I love your patch! Yet something to improve:
[auto build test ERROR on kvm/linux-next]
[also build test ERROR on vhost/linux-next linus/master v5.7-rc4 next-20200507]
[cannot apply to linux/master]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Suravee-Suthikulpanit/KVM-SVM-Disable-AVIC-before-setting-V_IRQ/20200507-111704
base: https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next
config: x86_64-rhel (attached as .config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
arch/x86/kvm/x86.c: In function 'kvm_request_apicv_update':
>> arch/x86/kvm/x86.c:8107:2: error: implicit declaration of function 'kvm_make_all_cpus_request_except'; did you mean 'kvm_make_all_cpus_request'? [-Werror=implicit-function-declaration]
kvm_make_all_cpus_request_except(kvm, KVM_REQ_APICV_UPDATE,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
kvm_make_all_cpus_request
cc1: all warnings being treated as errors
vim +8107 arch/x86/kvm/x86.c
8065
8066 /*
8067 * NOTE: Do not hold any lock prior to calling this.
8068 *
8069 * In particular, kvm_request_apicv_update() expects kvm->srcu not to be
8070 * locked, because it calls __x86_set_memory_region() which does
8071 * synchronize_srcu(&kvm->srcu).
8072 */
8073 void kvm_request_apicv_update(struct kvm *kvm, bool activate, ulong bit)
8074 {
8075 struct kvm_vcpu *except;
8076 unsigned long old, new, expected;
8077
8078 if (!kvm_x86_ops.check_apicv_inhibit_reasons ||
8079 !kvm_x86_ops.check_apicv_inhibit_reasons(bit))
8080 return;
8081
8082 old = READ_ONCE(kvm->arch.apicv_inhibit_reasons);
8083 do {
8084 expected = new = old;
8085 if (activate)
8086 __clear_bit(bit, &new);
8087 else
8088 __set_bit(bit, &new);
8089 if (new == old)
8090 break;
8091 old = cmpxchg(&kvm->arch.apicv_inhibit_reasons, expected, new);
8092 } while (old != expected);
8093
8094 if (!!old == !!new)
8095 return;
8096
8097 trace_kvm_apicv_update_request(activate, bit);
8098 if (kvm_x86_ops.pre_update_apicv_exec_ctrl)
8099 kvm_x86_ops.pre_update_apicv_exec_ctrl(kvm, activate);
8100
8101 /*
8102 * Sending request to update APICV for all other vcpus,
8103 * while update the calling vcpu immediately instead of
8104 * waiting for another #VMEXIT to handle the request.
8105 */
8106 except = kvm_get_running_vcpu();
> 8107 kvm_make_all_cpus_request_except(kvm, KVM_REQ_APICV_UPDATE,
8108 except);
8109 if (except)
8110 kvm_vcpu_update_apicv(except);
8111 }
8112 EXPORT_SYMBOL_GPL(kvm_request_apicv_update);
8113
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 44840 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread