Re: [RFC PATCH v3 0/3] Add segment limit checks to emulator

[Gleb Natapov <gleb@redhat.com>]

On Mon, Jul 12, 2010 at 04:51:10PM +0300, Avi Kivity wrote:
> On 07/12/2010 04:39 PM, Mohammed Gamal wrote:
> >
> >>>What happens is that guests are switched to big real mode so either
> >>>gPXE and SeaBIOS need to be modified to work with the way KVM handles
> >>>segment limits when switching to real mode, but that'd be only a
> >>>temporary solution. The other - and better IMO - option is to get
> >>>e_i_g_s=1 completely functional, which is something we want to do
> >>>anyway. So we can address all the comments you have on these patches
> >>>and eventually merge them along with the rest of e_i_g_s patches.
> >>>
> >>Does SeaBIOS use big real mode now?
> >I think so, ftrace shows a CR0 access just before the instruction that
> >causes the failure. I am not 100% sure though.
> 
> Ok, will be good to know.  In any case, I think it can be made to
> work even without e_i_g_s=1.
> 
> 
> >>What about expand-down segments?  and moving the limit check where the
> >>access is emulated (so we are sure we don't miss a check)?
> >Let me make sure I am understanding this correctly. I added a check in
> >do_insn_fetch_byte() checking for CS limit. Similar checks in
> >emulate_push() ane emulate_pop() for SS, and checks in
> >x86_decode_insn() for SrcSI and DstDI since they causes accesses to
> >segment override and ES respectively. Are we on the same page?
> 
> You have to do the check wherever you have a read or write that is
> qualified by a segment.  So the best place for them is in
> ->read_emulated(), ->write_emulated(), and similar.
> 
> A good way to do this is to add a segment variable to 'struct
> operand', and doing all the base adjustment at the end (instead of
> up front as we do now).  That means we'll have the minimum number of
> places to add checks to.
->read_emulated(), ->write_emulated() get liner address as a parameter
and know nothing about 'struct operand'. Luckily emulator.c has only one
call for each one of them, so segment checking can be done there just
before call to the functions. 

> 
> 
> >I haven't looked into expand down segments, but I don't think it's
> >much of an effort to add though.
> 
> It's needed, since guests will start failing mysteriously if they
> use those segments and the limits are incorrect (though I doubt
> there are any guests which use expand-down segments).
> 
> -- 
> error compiling committee.c: too many arguments to function
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
			Gleb.