From mboxrd@z Thu Jan 1 00:00:00 1970 From: Aravindh Puthiyaparambil Subject: Re: [PATCH v3] Fix the mistake of exception execution Date: Tue, 15 May 2012 01:19:43 -0700 Message-ID: References: <403610A45A2B5242BD291EDAE8B37D300FDC35E2@SHSMSX102.ccr.corp.intel.com> <4FB1037302000078000836C8@nat28.tlf.novell.com> <403610A45A2B5242BD291EDAE8B37D300FDC40C0@SHSMSX102.ccr.corp.intel.com> <4FB218670200007800083A63@nat28.tlf.novell.com> <403610A45A2B5242BD291EDAE8B37D300FDC41A1@SHSMSX102.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5147208859588423856==" Return-path: In-Reply-To: <403610A45A2B5242BD291EDAE8B37D300FDC41A1@SHSMSX102.ccr.corp.intel.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "Hao, Xudong" Cc: "Nakajima, Jun" , "Dong, Eddie" , "xen-devel (xen-devel@lists.xen.org)" , "Keir Fraser(keir.xen@gmail.com)" , Jan Beulich , "Zhang, Xiantao" List-Id: xen-devel@lists.xenproject.org --===============5147208859588423856== Content-Type: multipart/alternative; boundary=e0cb4efe2af2655eaa04c00eddba --e0cb4efe2af2655eaa04c00eddba Content-Type: text/plain; charset=ISO-8859-1 On May 15, 2012 1:15 AM, "Hao, Xudong" wrote: > > > -----Original Message----- > > From: Aravindh Puthiyaparambil [mailto:aravindh@virtuata.com] > > Sent: Tuesday, May 15, 2012 3:23 PM > > To: Jan Beulich > > Cc: Hao, Xudong; Keir Fraser(keir.xen@gmail.com); Dong, Eddie; Nakajima, Jun; > > Zhang, Xiantao; xen-devel (xen-devel@lists.xen.org) > > Subject: Re: [PATCH v3] Fix the mistake of exception execution > > > > On Mon, May 14, 2012 at 11:48 PM, Jan Beulich wrote: > > > > > > >>> On 15.05.12 at 07:59, "Hao, Xudong" wrote: > > > >> From: Jan Beulich [mailto:JBeulich@suse.com] > > > >> >>> On 14.05.12 at 12:41, "Hao, Xudong" > > wrote: > > > >> > default: > > > >> > - if ( trap > TRAP_last_reserved ) > > > >> > - { > > > >> > - type = X86_EVENTTYPE_SW_EXCEPTION; > > > >> > - __vmwrite(VM_ENTRY_INSTRUCTION_LEN, 2); /* int > > imm8 */ > > > >> > - } > > > >> > > > >> So this undoes Aravindh's earlier change, without replacement. I > > > >> don't think that's acceptable. > > > >> > > > > > > > > This is the first patch that just correct some instruction in hw exception > > > > function, as function description above, int n (n > 32) is not delivered by > > > > this function. > > > > I'll write another patch of new function for int n handler. > > > > > > In that case it would have been nice to indicate that you don't expect > > > this to be applied just yet (i.e. by marking the patch RFC). > > > > > > >> > + __vmwrite(VM_ENTRY_INSTRUCTION_LEN, > > 1); /* int3, CC */ > > > >> > > > >> Still using a hard-coded 1 here, the more that afaict you can't > > > >> distinguish CC and CD 03 here. > > > >> > > > > > > > > Just copied it from original code, how about this replacement: > > > > > > > > + __vmwrite(VM_ENTRY_INSTRUCTION_LEN, > > __vmread(VM_EXIT_INSTRUCTION_LEN)); > > > > > > That's okay as long as on all possible code paths arriving here > > > VM_EXIT_INSTRUCTION_LEN is actually valid. I'm suspicious this might > > > not be the case (especially in the case of injection originating from > > > libxc). > > > > Your suspicion is warranted. IIRC this did not work for the libxc case > > injecting software interrupts. That is why I hard coded the > > instruction length. Maybe the instruction length can be made caller > > specific? > > > > What's traps did you inject? This patch has not handle the software interrupts, but hardware exceptions and #BP, #OF software exceptions. > The function handles software interrupts though marked as software exception. Incorrect it might be but it works. Your patch removes that code. Thanks, Aravindh --e0cb4efe2af2655eaa04c00eddba Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable


On May 15, 2012 1:15 AM, "Hao, Xudong" <xudong.hao@intel.com> wrote:
>
> > -----Original Message-----
> > From: Aravindh Puthiyaparambil [mailto:aravindh@virtuata.com]
> > Sent: Tuesday, May 15, 2012 3:23 PM
> > To: Jan Beulich
> > Cc: Hao, Xudong; Keir Fraser(keir.xen@gmail.com); Dong, Eddie; Nakajima, Jun;
> > Zhang, Xiantao; xen-devel (xen-devel@lists.xen.org)
> > Subject: Re: [PATCH v3] Fix the mistake of exception execution > >
> > On Mon, May 14, 2012 at 11:48 PM, Jan Beulich <JBeulich@suse.com> wrote:
> > >
> > > >>> On 15.05.12 at 07:59, "Hao, Xudong" &= lt;xudong.hao@intel.com> wro= te:
> > > >> From: Jan Beulich [mailto:JBeulich@suse.com]
> > > >> >>> On 14.05.12 at 12:41, "Hao, Xudon= g" <xudong.hao@intel.com>
> > wrote:
> > > >> > =A0 =A0 =A0default:
> > > >> > - =A0 =A0 =A0 =A0if ( trap > TRAP_last_rese= rved )
> > > >> > - =A0 =A0 =A0 =A0{
> > > >> > - =A0 =A0 =A0 =A0 =A0 =A0type =3D X86_EVENTTYP= E_SW_EXCEPTION;
> > > >> > - =A0 =A0 =A0 =A0 =A0 =A0__vmwrite(VM_ENTRY_IN= STRUCTION_LEN, 2); /* int
> > imm8 */
> > > >> > - =A0 =A0 =A0 =A0}
> > > >>
> > > >> So this undoes Aravindh's earlier change, witho= ut replacement. I
> > > >> don't think that's acceptable.
> > > >>
> > > >
> > > > This is the first patch that just correct some instruct= ion in hw exception
> > > > function, as function description above, int n (n > = 32) is not delivered by
> > > > this function.
> > > > I'll write another patch of new function for int n = handler.
> > >
> > > In that case it would have been nice to indicate that you do= n't expect
> > > this to be applied just yet (i.e. by marking the patch RFC).=
> > >
> > > >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0__vmw= rite(VM_ENTRY_INSTRUCTION_LEN,
> > 1); /* int3, CC */
> > > >>
> > > >> Still using a hard-coded 1 here, the more that afai= ct you can't
> > > >> distinguish CC and CD 03 here.
> > > >>
> > > >
> > > > Just copied it from original code, how about this repla= cement:
> > > >
> > > > + =A0 =A0 __vmwrite(VM_ENTRY_INSTRUCTION_LEN,
> > __vmread(VM_EXIT_INSTRUCTION_LEN));
> > >
> > > That's okay as long as on all possible code paths arrivi= ng here
> > > VM_EXIT_INSTRUCTION_LEN is actually valid. I'm suspiciou= s this might
> > > not be the case (especially in the case of injection origina= ting from
> > > libxc).
> >
> > Your suspicion is warranted. IIRC this did not work for the libxc= case
> > injecting software interrupts. That is why I hard coded the
> > instruction length. Maybe the instruction length can be made call= er
> > specific?
> >
>
> What's traps did you inject? This patch has not handle the softwar= e interrupts, but hardware exceptions and #BP, #OF software exceptions.
>

The function handles software interrupts though marked as software excep= tion. Incorrect it might be but it works. Your patch removes that code.

Thanks,
Aravindh

--e0cb4efe2af2655eaa04c00eddba-- --===============5147208859588423856== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============5147208859588423856==--