Re: [Xen-devel] [PATCH v2 1/4] x86/vvmx: fix virtual interrupt injection when Ack on exit control is used

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Feb 04, 2020 at 01:35:05AM +0000, Tian, Kevin wrote:
> > From: Roger Pau Monne <roger.pau@xxxxxxxxxx>
> > Sent: Monday, February 3, 2020 8:19 PM
> > 
> > When doing a virtual vmexit (ie: a vmexit handled by the L1 VMM)
> > interrupts shouldn't be injected using the virtual interrupt delivery
> > mechanism unless the Ack on exit vmexit control bit isn't set in the
> > nested vmcs.
> > 
> > Gate the call to nvmx_update_apicv helper on whether the nested vmcs
> > has the Ack on exit bit set in the vmexit control field.
> > 
> > Note that this fixes the usage of x2APIC by the L1 VMM, at least when
> > the L1 VMM is Xen.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>, with a small comment:
> 
> > ---
> > Changes since v1:
> >  - Call nvmx_update_apicv if the "Ack on exit" vmexit control bit
> >    isn't set.
> > ---
> >  xen/arch/x86/hvm/vmx/vvmx.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> > index d8ab167d62..3d97a293b2 100644
> > --- a/xen/arch/x86/hvm/vmx/vvmx.c
> > +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> > @@ -1393,7 +1393,12 @@ static void virtual_vmexit(struct cpu_user_regs
> > *regs)
> >      /* updating host cr0 to sync TS bit */
> >      __vmwrite(HOST_CR0, v->arch.hvm.vmx.host_cr0);
> > 
> > -    if ( cpu_has_vmx_virtual_intr_delivery )
> > +    if ( cpu_has_vmx_virtual_intr_delivery &&
> > +         /*
> > +          * Only inject the vector if the Ack on exit bit is not set, else 
> > the
> > +          * interrupt will be signaled in the vmcs VM_EXIT_INTR_INFO field.
> > +          */
> > +         !(get_vvmcs(v, VM_EXIT_CONTROLS) & VM_EXIT_ACK_INTR_ON_EXIT) )
> 
> It's a bit odd to put comment between code lines. Can you move it to
> the line before 'if'?

I usually try to keep the comments as close to the code it references
as possible, even if that means adding them in the middle of an if
condition.

I don't mind moving it, but maybe the committer can do it at commit
time?

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel