Ramon van Handel wrote:

> 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 ;))

I'll check on this.  Will get back to you.


> 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.

We may be able to do some unifying here.


> PS do you mind if I relayout the prescan code like what
> most other FreeMWare code looks like ?

I suppose you're going to anyways. :^)



> 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()...

We could have a set of opcode virtualization maps (opcode_vmap_t),
depending on things like CPL, feature usage, and ultimately
different debugging/instrumentation demands.  It's not necessary
at the moment, but likely will be in the future.

Ultimately I envisioned a larger structure which would hold
the page pool for modified/unmodified/meta-cache pages.  There
needs to be additional info regarding these pages such as
CPL, segment size, and some other useful info.  So, perhaps
these won't be passed like this later.  For now, I'd say, just
leave them that way.



> // 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.

Actually, these were just some notes I wrote down as I looked through
the opcode map.  Some of them are for instructions which are
available on post Pentium processors.  We'll have to modify
the virtualization and decode maps to add certain instructions
as per the CPU capabilities.  We'll need a function to determine
the CPU and act accordinly.  Don't pay much attention to them.

> 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 :-/).

Sure we _could_ let instructions cross page boundaries if
we keep extra information, so that when pages are invalidated
due to writes, we dump the meta cache for the previous
pages.  It's a similar concept for out of page branches.
For branches, this can quickly get unruly, invalidating a large number
of pages for one write-hit, depending on the strategy you use.
I mentioned looking at page-clusters in the paper.

For the page boundary overlap specifically, we could store some
extra bits, letting us know if the current page links to the
previous and next linear page, to facilitate invalidation.
for changes to the page table, we have to do something intelligent.

Another boundary case, relates to the segment limit.  It is conceivable
that a segment limit could end in the middle of a page.  As long
as we can set the virtualized descriptor to do something similar,
the native mechanisms can protect for this, but I thought I'd
point it out.




> 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.

If either a non-branch or conditional branch instruction
fits, but abuts the page boundary, then execution of following
instruction is a certainty/possibility, and occurs in the
next page.  Keeping in mind, our goal is to _never_ let
execution get away from our control, for these situations,
we have to virtualize this instruction.  For instance,
in the last byte of the page, we have a NOP.

  offset 4095:  NOP

If we quit here, offset 0 of the next page will get
executed out-of-control, which is A Bad Thing.

For unconditional branch instructions, we can let
them abut the page boundary, as long as they fit.
This is what the padding variable is all about.


> 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?

Yes, this algorithm is a memory hog.  I have in mind
to keep this extremely modular, so we can
send it to /dev/null at some point.  Your hashing idea
may work out.  The gain/trade-off for now, is that this
was the simplist way to do this I could think of.
I'd say let's debug this and then look into hashing
functions after.

Some other thoughts.  If we get rid of the meta page,
then we have to start all over, scanning the page again
the next time the execution path goes through that page.
Otherwise, when code transfers here, we don't know whether
it's to a scanned address or not.

As we are currently monitoring execution, we could stamp
the meta cache with a reading from RDTSC.  Pages that have
not been used in a while, we could decide to free.  We
would have to start over the next time they are used.

An ultimate goal would be to let ring3 code run as is,
granted conditions are right.  In that case, we're prescanning
only ring0 code pages, which reduces the memory consumption.

Anyways, you can see why we need to keep in mind, the
ideas about keeping pages eligible for host swapping!

-Kevin

Reply via email to