> From: Roger Pau Monne <[email protected]> > Sent: Wednesday, March 25, 2020 6:19 PM > > Updating SVI is required when an interrupt has been injected using the > Ack on exit VMEXIT feature, so that the in service interrupt in the > GUEST_INTR_STATUS matches the vector that is signaled in > VM_EXIT_INTR_INFO. > > Updating RVI however is not tied to the Ack on exit feature, as it > signals the next vector to be injected, and hence should always be > updated to the next pending vector, regardless of whether Ack on exit > is enabled. > > Signed-off-by: Roger Pau Monné <[email protected]> > --- > xen/arch/x86/hvm/vmx/vvmx.c | 22 +++++++++++++++++----- > 1 file changed, 17 insertions(+), 5 deletions(-) > > diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c > index 1753005c91..8431c912a1 100644 > --- a/xen/arch/x86/hvm/vmx/vvmx.c > +++ b/xen/arch/x86/hvm/vmx/vvmx.c > @@ -1384,26 +1384,38 @@ static void nvmx_update_apicv(struct vcpu *v) > struct nestedvmx *nvmx = &vcpu_2_nvmx(v); > unsigned long reason = get_vvmcs(v, VM_EXIT_REASON); > unsigned long intr_info = get_vvmcs(v, VM_EXIT_INTR_INFO); > + int rvi; > > if ( reason == EXIT_REASON_EXTERNAL_INTERRUPT && > nvmx->intr.source == hvm_intsrc_lapic && > (intr_info & INTR_INFO_VALID_MASK) ) > { > uint16_t status; > - uint32_t rvi, ppr; > - uint32_t vector = intr_info & 0xff; > + uint32_t ppr; > + unsigned int vector = intr_info & INTR_INFO_VECTOR_MASK; > struct vlapic *vlapic = vcpu_vlapic(v); > > + /* > + * Update SVI to record the currently in service interrupt that's > + * signaled in EXIT_INTR_INFO. > + */ > vlapic_ack_pending_irq(v, vector, 1); > > ppr = vlapic_set_ppr(vlapic); > WARN_ON((ppr & 0xf0) != (vector & 0xf0)); > > status = vector << VMX_GUEST_INTR_STATUS_SVI_OFFSET; > - rvi = vlapic_has_pending_irq(v); > - if ( rvi != -1 ) > - status |= rvi & VMX_GUEST_INTR_STATUS_SUBFIELD_BITMASK; > + __vmwrite(GUEST_INTR_STATUS, status); > + } > + > + rvi = vlapic_has_pending_irq(v); > + if ( rvi != -1 ) > + { > + unsigned long status; > > + __vmread(GUEST_INTR_STATUS, &status); > + status &= ~VMX_GUEST_INTR_STATUS_SUBFIELD_BITMASK; > + status |= rvi & VMX_GUEST_INTR_STATUS_SUBFIELD_BITMASK; > __vmwrite(GUEST_INTR_STATUS, status); > }
I have two minor comments. First, original code requires only one __vmwrite but now needs two writes and one read for Ack-on-exit is set. Can we optimize to sustain the original behavior? Second, although I didn't come up a case where always updating RVI (w/ pending interrupt) may bring some bad effect, it is anyway safer to put the whole logic within the check of vmexit reason and intr source. Thanks Kevin
