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