Re: [Xen-devel] [Xen-users] xc_hvm_inject_trap() failing for int3 traps under Xen 4.2.2

["Aravindh Puthiyaparambil (aravindp)" <aravindp@xxxxxxxxx>]

> (Cc-ing a few more people, moving xen-users to Bcc)
> 
> At 14:51 +0000 on 15 Jun (1371307878), Antony Saba wrote:
> > >> 2) xc_hvm_inject_trap() always returns a negative value, even when
> > >> there is not a problem and the guest receives the trap as expected.
> > >> There hasn't been a clarification as to whether it's supposed to
> > >> return non-negative, but one would assume that it should because of
> > >> the way the xen-access.c example checks for it.
> > >
> > > That looks like a hypervisor bug to me: does this (untested) patch
> > > fix it for you?
> > >
> > > commit 67b9272fcedcb5dc73cc77a2adf580f2572117d7
> > > Author: Tim Deegan <tim@xxxxxxx>
> > > Date:   Mon Jun 10 19:35:34 2013 +0100
> > >
> > >     x86/hvm: Fix HVMOP_inject_trap return value on success.
> > >
> > >     Reported-by: Antony Saba <Antony.Saba@xxxxxxxxxxxx>
> > >     Signed-off-by: Tim Deegan <tim@xxxxxxx>
> > >
> > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index
> > > ce44bff..6c86fc2 100644
> > > --- a/xen/arch/x86/hvm/hvm.c
> > > +++ b/xen/arch/x86/hvm/hvm.c
> > > @@ -4430,6 +4430,7 @@ long do_hvm_op(unsigned long op,
> XEN_GUEST_HANDLE_PARAM(void) arg)
> > >              v->arch.hvm_vcpu.inject_trap.error_code = tr.error_code;
> > >              v->arch.hvm_vcpu.inject_trap.insn_len = tr.insn_len;
> > >              v->arch.hvm_vcpu.inject_trap.cr2 = tr.cr2;
> > > +            rc = 0;
> > >          }
> > >
> > >      param_fail8:
> > >
> > >
> > >
> >
> > This works
> 
> Thanks.  I'll take that as a Tested-by:.
> 
> Keir, Jan, can I get an Ack?
> 
> George, this is a clean bug-fix for something seen in the field, in a path 
> that
> doesn't affect any other features.  OK before 4.3?
> 
> Jan, this is a candidate for backporting to 4.1.
> 
> > but the instruction size must be set to 1, at least on 4.2.2 to work
> > for me.  Here is the patch against RELEASE-4.2.2.
> 
> Sorry, I wasn't clear: setting it to 1 is certainly an improvement over zero
> (which is always wrong), but if you're relying on this to be correct you 
> should
> also handle cases where prefixes make the instruction longer than 1 byte.

I agree this is definitely an improvement. I will try adding the prefix check 
when I get the chance.
 
> In any case, this tools-side change needs to be acked/nacked by a tools
> maintainer, and probably Aravindh too.

Acked-by: Aravindh Puthiyaparambil <aravindp@xxxxxxxxx>

Thanks,
Aravindh

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel