Ramon van Handel wrote:
> 
> Kevin P. Lawton wrote:
> >
> >    ftp://bochs.com/pub/freemware/prescan-2000_0222a.tar.gz
> 
> Kevin,
> Great work !!!
> 
> I'm just browsing through the code, here are some initial comments.
> 
> First, the useless little details:
> 
> ------
>  *  Copyright (C) 2000  Free Software Foundation, Inc.
>  *  59 Temple Place - Suite 330, Boston, MA  02111-1307, USA
> FreeMWare isn't (C) FSF because we're not a GNU project.
> We could become a GNU project, but right now we arent ;)
> ------
>  *  Authors:
>  *    Kevin P. Lawton (initial implementation)
> I think we're better off leaving this off the comments
> in the files, but have an AUTHORS file in stead, like
> linux does (or was it called CONTRIBUTORS ?).  That way
> it's probably easier to see who's worked on a certain
> part (i.e. is expected to fix it if it's broken ;))
> ------
> Now we have a total of *three* instruction decoders
> in FreeMWare --- Ulrich's decoder, BOCHS decoder,
> and the custom decoder in kernel/emulation.c.
> Isn't this a bit overkill ?  Probably, one instruction
> decoder should be more than enough.  We should take
> one decoder (perhaps the BOCHS one) and strip all
> the unneccessary code to make it as fast as possible,
> and consequently use it everywhere.
> ------
> PS do you mind if I relayout the prescan code like what
> most other FreeMWare code looks like ?
> ------
> 
> Diving into the code:
> 
> ------
> void prescan(u8 *modified, u8 *unmodified, u32 offset, unsigned seg32,
>              opcode_vmap_t *op_vmap, u8 *, unsigned depth);
> Do we need to forward all of these as parameters ?
> Like op_vmap, I'd assume it's the same for all invocations
> of prescan()...
> ------
> // Some special cases I've noted for whatever reasons.
> //   push esp, pop esp? 54, 5c
> 
> Hmm.  Why does this need to be virtualised ?
> 
> //   cmpxchg_xbts/ibts 0f a6/a7
> //   cmpxchg_e.g. 0f b0/b1
> 
> Not sure about this, but I think these only need
> to be virtualised if
> - the host processor has the F00F bug, and
> - the instruction is F00F (LOCK CMPXCHG8B EAX)
> Or am I missing something ?
> 
> //   bswap_e.x 0f c8..cf
> 
> Hm.  Why ?
> I don't really know the other instructions on
> that list, perhaps it would be a good idea to
> elaborate on why they need to be caught.
> ------
>   // If fetch-decode ran up against a boundary and could not
>   // fetch enough bytes, or if the opcode is invalid, then mark
>   // the instruction as needing virtualization.
> Looking at the earlier code, by "boundary" you mean
> page boundary.  Do we always need to trap the last
> instruction on the page, if it doesn't fit ?  In
> principle it should be needed only once, when the
> next page hasn't been scanned yet.  I guess this
> means that we need to "remember" that an instruction
> was marked on a page boundary -- when it is executed,
> the exception handler code needs to
>  (1) replace the original code
>  (2) scan the next page
> There are more of these things (like branching outside
> of the page).  I guess we should write a prototype
> exception handling code as well... perhaps I can work
> on this in the weekend (if I have the time... always less
> than I'd like :-/).
> ------
>   // We need to make sure that this instruction fits within the
>   // page boundary, otherwise we have to virtualize it.
> I'm probably being stupid here, but how can this possibly
> *not* be the case ?  After all, we gave fetchDecode()
> the max_len attribute above, so if it hits a page boundary
> we should never get to this code anyway.
> ------
> 
> Now a general worry: using this algorithm, we need
> *three* pages of memory per code page.  Isn't this
> going to be a terrible memory hog ?  Actually, I guess
> we can throw away the meta page when we're done
> processing a page (probably easiest to allocate a page
> permanently per VM, and to reuse it on every invocation
> of prescan()) but then we're still using two times
> as much memory as we actually need --- we can assume
> there are way fewer modifications in a page than
> "good" code.  Can't we, in stead of using a whole
> separate page, make the modifications directly in
> the code page and copy the original code into a
> hash table, or something similar ?  What do you think?
> 
> -- Ramon

Reply via email to