[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] Re: [RFC, PATCH 5/24] i386 Vmi code patching

Chuck Ebbert wrote:
In-Reply-To: <200603222115.46926.ak@xxxxxxx>

On Wed, 22 Mar 2006 21:15:44 +0100, Andi Kleen wrote:

On Monday 13 March 2006 19:02, Zachary Amsden wrote:
The VMI ROM detection and code patching mechanism is illustrated in
setup.c.  There ROM is a binary block published by the hypervisor, and
and there are certainly implications of this.  ROMs certainly have a
history of being proprietary, very differently licensed pieces of
software, and mostly under non-free licenses.  Before jumping to the
conclusion that this is a bad thing, let us consider more carefully
why hiding the interface layer to the hypervisor is actually a good
How about you fix all these issues you describe here first and then submit it again?

The disassembly stuff indeed doesn't look like something
that belongs in the kernel.

I think they put the disassembler in there as a joke. ;)

It's not necessary for fixing up the call site, anyway. Something like
this should work, assuming there is always a call in every vmi

Very good observation. The code you illustrate is roughly what the patch mechanism used to do. The disassembly serves one purpose, which is incomplete in our current implementation. It provides live register information and constant propagation that can be used by an inliner in the VMI layer to inline hypervisor calls. This information gets encoded naturally in the push sequence before the call instruction.

But considering it is currently incomplete, and most of the benefit can be had be special casing just 4 VMI calls to allow selective inlining, it does seem like a lot of complexity for little payoff. I really don't like special casing, but in this scenario, it does seem to make sense. And of the 4 VMI calls, only one (SetInterruptMask) takes an input, and it takes that input in a fixed register.

Based on suggestions from Anthony Liguori and others, we are going to drop this extra complexity - we can get to hypervisor inlining later. Right now having the simplest and cleanest interface is more important. Actually, adding the required padding for inlining is quite easy:

#define FILL(n) ".fill " #n "-1,1,0x66; nop"

static inline void local_irq_restore(const unsigned long flags)
               SetInterruptMask, "pushl %0; popfl"  FILL(6),
               1, VMI_IREG1 (flags),
               XCONC("cc", "memory"));

Now you have 8 bytes to overwrite, which is sufficient for byte arithmetic operations to a memory address, plus a segment override. This seems like a much simpler solution than run-time disassembly.

-                               /*
-                                * Don't printk - it may acquire spinlocks with
-                                * partially completed VMI translations, causing
-                                * nuclear meltdown of the core.
-                                */
-                               BUG();
-                               return;
-               }
-       }

This part was a joke  ;)

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.