| Message ID | 20200203121919.15748-3-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/vvmx: fixes to interrupt injection | expand |
> From: Roger Pau Monne <roger.pau@citrix.com> > Sent: Monday, February 3, 2020 8:19 PM > > When VM_EXIT_ACK_INTR_ON_EXIT is set in the vmexit control vmcs set->cleared > register the bit 31 of VM_EXIT_INTR_INFO must be 0, in order to denote > that the field doesn't contain any interrupt information. This is not > currently acknowledged as the field always get filled with valid > interrupt information, regardless of whether VM_EXIT_ACK_INTR_ON_EXIT > is set. > > Fix this and only fill VM_EXIT_INTR_INFO when VM_EXIT_ACK_INTR_ON_EXIT > is not set. Note that this requires one minor change in not set -> set > nvmx_update_apicv in order to obtain the interrupt information from > the internal state rather than the nested vmcs register. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> with above fixed, Reviewed-by: Kevin Tian <kevin.tian@intel.com> > --- > Changes since v1: > - New in this version. > --- > xen/arch/x86/hvm/vmx/vvmx.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c > index 3d97a293b2..47eee1e5b9 100644 > --- a/xen/arch/x86/hvm/vmx/vvmx.c > +++ b/xen/arch/x86/hvm/vmx/vvmx.c > @@ -1283,6 +1283,7 @@ static void load_vvmcs_host_state(struct vcpu *v) > static void sync_exception_state(struct vcpu *v) > { > struct nestedvmx *nvmx = &vcpu_2_nvmx(v); > + uint32_t exit_ctrl = get_vvmcs(v, VM_EXIT_CONTROLS); > > if ( !(nvmx->intr.intr_info & INTR_INFO_VALID_MASK) ) > return; > @@ -1294,7 +1295,8 @@ static void sync_exception_state(struct vcpu *v) > set_vvmcs(v, VM_EXIT_REASON, EXIT_REASON_EXTERNAL_INTERRUPT); > set_vvmcs(v, EXIT_QUALIFICATION, 0); > set_vvmcs(v, VM_EXIT_INTR_INFO, > - nvmx->intr.intr_info); > + (exit_ctrl & VM_EXIT_ACK_INTR_ON_EXIT) ? nvmx->intr.intr_info > + : 0); > break; > > case X86_EVENTTYPE_HW_EXCEPTION: > @@ -1320,7 +1322,7 @@ static void nvmx_update_apicv(struct vcpu *v) > { > struct nestedvmx *nvmx = &vcpu_2_nvmx(v); > unsigned long reason = get_vvmcs(v, VM_EXIT_REASON); > - uint32_t intr_info = get_vvmcs(v, VM_EXIT_INTR_INFO); > + uint32_t intr_info = nvmx->intr.intr_info; > > if ( reason == EXIT_REASON_EXTERNAL_INTERRUPT && > nvmx->intr.source == hvm_intsrc_lapic && > -- > 2.25.0
On Tue, Feb 04, 2020 at 01:39:44AM +0000, Tian, Kevin wrote: > > From: Roger Pau Monne <roger.pau@citrix.com> > > Sent: Monday, February 3, 2020 8:19 PM > > > > When VM_EXIT_ACK_INTR_ON_EXIT is set in the vmexit control vmcs > > set->cleared > > > register the bit 31 of VM_EXIT_INTR_INFO must be 0, in order to denote > > that the field doesn't contain any interrupt information. This is not > > currently acknowledged as the field always get filled with valid > > interrupt information, regardless of whether VM_EXIT_ACK_INTR_ON_EXIT > > is set. > > > > Fix this and only fill VM_EXIT_INTR_INFO when VM_EXIT_ACK_INTR_ON_EXIT > > is not set. Note that this requires one minor change in > > not set -> set > > > nvmx_update_apicv in order to obtain the interrupt information from > > the internal state rather than the nested vmcs register. > > > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > with above fixed, Ouch yes, I've inverted the conditions. Thanks for noticing. > > Reviewed-by: Kevin Tian <kevin.tian@intel.com> To whoever picks this patch for committing: let me know if you would like me to resend with the commit log fixed. Roger.
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c index 3d97a293b2..47eee1e5b9 100644 --- a/xen/arch/x86/hvm/vmx/vvmx.c +++ b/xen/arch/x86/hvm/vmx/vvmx.c @@ -1283,6 +1283,7 @@ static void load_vvmcs_host_state(struct vcpu *v) static void sync_exception_state(struct vcpu *v) { struct nestedvmx *nvmx = &vcpu_2_nvmx(v); + uint32_t exit_ctrl = get_vvmcs(v, VM_EXIT_CONTROLS); if ( !(nvmx->intr.intr_info & INTR_INFO_VALID_MASK) ) return; @@ -1294,7 +1295,8 @@ static void sync_exception_state(struct vcpu *v) set_vvmcs(v, VM_EXIT_REASON, EXIT_REASON_EXTERNAL_INTERRUPT); set_vvmcs(v, EXIT_QUALIFICATION, 0); set_vvmcs(v, VM_EXIT_INTR_INFO, - nvmx->intr.intr_info); + (exit_ctrl & VM_EXIT_ACK_INTR_ON_EXIT) ? nvmx->intr.intr_info + : 0); break; case X86_EVENTTYPE_HW_EXCEPTION: @@ -1320,7 +1322,7 @@ static void nvmx_update_apicv(struct vcpu *v) { struct nestedvmx *nvmx = &vcpu_2_nvmx(v); unsigned long reason = get_vvmcs(v, VM_EXIT_REASON); - uint32_t intr_info = get_vvmcs(v, VM_EXIT_INTR_INFO); + uint32_t intr_info = nvmx->intr.intr_info; if ( reason == EXIT_REASON_EXTERNAL_INTERRUPT && nvmx->intr.source == hvm_intsrc_lapic &&
When VM_EXIT_ACK_INTR_ON_EXIT is set in the vmexit control vmcs register the bit 31 of VM_EXIT_INTR_INFO must be 0, in order to denote that the field doesn't contain any interrupt information. This is not currently acknowledged as the field always get filled with valid interrupt information, regardless of whether VM_EXIT_ACK_INTR_ON_EXIT is set. Fix this and only fill VM_EXIT_INTR_INFO when VM_EXIT_ACK_INTR_ON_EXIT is not set. Note that this requires one minor change in nvmx_update_apicv in order to obtain the interrupt information from the internal state rather than the nested vmcs register. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Changes since v1: - New in this version. --- xen/arch/x86/hvm/vmx/vvmx.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)