Xen devel - May 2007 - [patch 0/2] Fix rawhide FV booting on VMX

Hi all,

The following two patches --- one to the HV, one to the tools --- allow
current rawhide/FC7 iso images to boot under Xen full virtualisation on
VMX.

The bug is due to Xen''s insanely convoluted way of dealing with 16-bit
code on Intel VMX.  The vmxassist layer needs to be able to respond to
CPU traps properly.  To be able to distinguish between traps being sent
to the vmxassist, and interrupts being delivered to the 16-bit code
being sub-emulated, it remaps the 16-bit guest''s IRQs to generate
different interrupt numbers.  This is really really ugly --- whenever
the 16-bit guest performs IO to the (virtual) PIC, vmxassist watches to
see if it''s a request to reprogram the PIC interrupt vector; and if so,
rewrites the outb so that the PIC vector is 32 (master PIC) or 40 (slave
PIC). vmxassist then installs 32-bit trap handlers at those offsets in
the IDT, and those handlers bounce the interrupt back into 16-bit mode
at the appropriate vector for the guest (which is *FORCED* to be 8
(master) or 0x70 (slave), the int handlers which the hvmloader rombios
sets up.)

All of this works (barely) while we''re in 16-bit mode.  But when the
bootloader transitions into 32-bit mode, vmxassist gracefully steps out
of the way, but the guest''s PIC vectors still point to the vmxassist
interrupts (32/40), not the rombios ones (8/0x70).  So if an interrupt
is delivered in this state, it triggers the wrong trap in the guest.

Now, the FC7 iso boot loader has a 32-bit graphical menu module loaded
by syslinux.  That 32-bit code installs interrupt traps to bounce all
received interrupts back down to 16-bit mode.  So it relies heavily on
the PIC vectors set up by 16-bit mode still being valid while in 32-bit
mode.

vmxassist can''t really fix this on its own.  The only way to reprogram
the guest''s virtual PIC is to reinitialise the vPIC, but doing so
throws
away all sorts of important vPIC state (some of it recoverable, like the
initialisation modes; some of it not recoverable, such as edge-trigger
latches.)

So the fix here is to first of all extend the virtual PIC provided by
the hypervisor, supporting a new 2-byte control sequence which lets the
guests change the interrupt vectors _without_ fully reinitialising the
vPIC; and secondly to have vmxassist use that sequence to reset the vPIC
vectors appropriately whenever we transition between 16 and 32-bit mode.

This has been verified to allow rawhide installs to proceed under Xen
FV.  

Points for discussion:

* Do we need/want vmxassist to fail gracefully if the enhanced vPIC
sequence is not available?
* Is the magic sequence used here (0xff written to ICW1) genuinely
impossible for other guests to use? (see patch 1/2 to follow.)

--Stephen



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi,
> So the fix here is to first of all extend the virtual PIC provided by
> the hypervisor, supporting a new 2-byte control sequence which lets the
> guests change the interrupt vectors _without_ fully reinitialising the
> PIC
And this is the patch for that.  The new control sequence is:

Write 0xff to ICW1: this is an otherwise unused value, since it has bit
4 set (ie. it''s an initialisation register write), and yet bits 5--7
are
also set (only valid for MCS-80 mode, which the Xen vPIC doesn''t
emulate
at all.)  This sets the PIC to expect ICW2 to follow, just as ICW1
usually does, except that it does not reset the PIC internal state.

Write the new vector to ICW2: completes the initialisation sequence.
This the same as a normal ICW2 write, except that it does not expect to
be followed by the usual ICW3/ICW4 writes.

Note that the flag used to indicate this "special" ICW1 write is
shared
between the two vPICs (master and slave), so it is not legal for the
guest to write 0xff to both master/slave ICW1 before following up with
the ICW2 writes.  But since the only software that will use this is the
modified vmxassist, that''s OK.

--Stephen




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi,

On Tue, 2007-05-22 at 14:54 +0100, Stephen C. Tweedie wrote:
> The following two patches --- one to the HV, one to the tools --- allow
> rawhide isos to boot under Xen full virtualisation.
And this is the userland tools side of the fix.
> So the fix here is ... secondly to have vmxassist use that sequence to
reset the PIC
> vectors appropriately whenever we transition between 16 and 32-bit mode.
This code teaches vmxassist:

1) to remember how the guest has set up the virtual PIC interrupt
vectors; and
2) to reprogram the vPIC to point to the true guest vector when entering
32-bit mode, and to point back to the vmxassist bounce-irq traps when
reentering 16-bit mode.

--Stephen


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Tue, May 22, 2007 at 04:38:12PM +0100, Stephen C. Tweedie
wrote:
> So the fix here is to first of all extend the virtual PIC provided by
> the hypervisor, supporting a new 2-byte control sequence which lets the
> guests change the interrupt vectors _without_ fully reinitialising the
> vPIC; and secondly to have vmxassist use that sequence to reset the vPIC
> vectors appropriately whenever we transition between 16 and 32-bit mode.
> 
> This has been verified to allow rawhide installs to proceed under Xen
> FV.  
I''ve tested that with this patch applied, Xen 3.1.0 is able to
succesfully
boot  RHEL-3, RHEL-4, RHEL-5, both i386 & x86_64  fullyvirt, and also
Mandriva CS-4 boots. SuSE 10.1 still black screens with / without this
patch. So on the whole it seems to be an improvement, with no regressions
I''ve found yet.
> Points for discussion:
> 
> * Do we need/want vmxassist to fail gracefully if the enhanced vPIC
> sequence is not available?
> * Is the magic sequence used here (0xff written to ICW1) genuinely
> impossible for other guests to use? (see patch 1/2 to follow.)
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 22/5/07 16:42, "Stephen C. Tweedie" <sct@redhat.com> wrote:
>> So the fix here is to first of all extend the virtual PIC provided by
>> the hypervisor, supporting a new 2-byte control sequence which lets the
>> guests change the interrupt vectors _without_ fully reinitialising the
>> PIC
> 
> And this is the patch for that.  The new control sequence is:
Yeees. I have no problem with this hack in principle (in fact we definitely
want to take it), but this implementation is not good. The
custom_revector_flag cannot be a static variable: it needs to be per-domain!

Would it be possible to hack this PIC transition code into
vmx_world_{save,restore}? This would restrict the changes to code that will
obviously be killed off when vmxassist is removed, and might perhaps be
simpler and faster than making the changes via specialised pokes of the PIC
device from vmxassist itself?

If not, at least the patch to Xen itself needs to be fixed for correctness
and code style, and preferably enclosed in ifdef or given a big comment so
that it''s clear that the hack''s lifetime is limited to
vmxassist''s lifetime.

 Thanks,
 Keir


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi,

On Fri, 2007-05-25 at 10:35 +0100, Keir Fraser wrote:
> > And this is the patch for that.  The new control sequence is:
> 
> Yeees. I have no problem with this hack in principle (in fact we definitely
> want to take it), but this implementation is not good. The
> custom_revector_flag cannot be a static variable: it needs to be
per-domain!
Right, of course.  An alternative is to add a field to the vpic struct,
but that struct is a public one (used for domain save/restore I think)
so there are more dependencies involved in doing it that way.  That
would also cover the (hopefully rare) case when we get a save/restore
during the bootloader.
> Would it be possible to hack this PIC transition code into
> vmx_world_{save,restore}? 
Hmm.  That should be possible --- we know when we''re entering vmxassist
at that point, and it''s precisely when that happens that we want to
have
the translated PIC in place.
> This would restrict the changes to code that will
> obviously be killed off when vmxassist is removed, and might perhaps be
> simpler and faster than making the changes via specialised pokes of the PIC
> device from vmxassist itself?
Faster, yes, but speed was not an issue for the patch --- this is
something only ever expected to happen during boot-loading, which is
usually not performance-critical.  It''s not greatly simpler --- it
still
relies on poking at the vPIC from a vmxassist transition --- but at
least the two parts of the layering violation sit together in the
hypervisor in this case, which is an advantage.

I''ll try putting a patch like this together.

Cheers,
 Stephen



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 25/5/07 11:41, "Stephen C. Tweedie" <sct@redhat.com> wrote:
>> Yeees. I have no problem with this hack in principle (in fact we
definitely
>> want to take it), but this implementation is not good. The
>> custom_revector_flag cannot be a static variable: it needs to be
per-domain!
> 
> Right, of course.  An alternative is to add a field to the vpic struct,
> but that struct is a public one (used for domain save/restore I think)
> so there are more dependencies involved in doing it that way.  That
> would also cover the (hopefully rare) case when we get a save/restore
> during the bootloader.
Gack! :-)

I''m not actually a fan of the shareing of structures between public
interfaces and internal implementations because of precisely this kind of
issue. You end up with either implementation details getting made visible to
external entities, or you end up putting private state in inappropriate
locations to avoid scuzzing the public interfaces. The save/restore
interfaces are something I need to look into a bit more thoroughly.

 -- Keir


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi,

On Fri, 2007-05-25 at 10:35 +0100, Keir Fraser wrote:
> Would it be possible to hack this PIC transition code into
> vmx_world_{save,restore}? This would restrict the changes to code that will
> obviously be killed off when vmxassist is removed, and might perhaps be
> simpler and faster than making the changes via specialised pokes of the PIC
> device from vmxassist itself?
This turned out to be trivial to do.  I was just stalled for a while in
testing it since it uncovered another regression --- Xen FV seems to
have problem with the latest dynticks patches from FC7 kernels. 

The new patch is much simpler; at the same time it''s uglier in that
it''s
an (even more) gross layering violation.  

vmxassist does the rewriting of the guests vPIC vectors, so the HV
*cannot* have any idea what vectors the guest actually intended to write
when we go back to 32-bit mode.  However, vmxassist already hard-codes
the vectors in several places: the 16-bit irq_bases are hard-coded to
0x08 and 0x70 in the trap bouncing code, and those bounce traps are
hard-coded to live at 0x20 and 0x28.  If we are willing to extend that
hard-coding into the hypervisor (but JUST the existing vmxassist-
specific portion, not the general vPIC code), then the patch reduces to
just a few lines, as below.

--Stephen




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi,

On Thu, 2007-05-31 at 21:35 +0100, Stephen C. Tweedie wrote:
> If we are willing to extend that
> hard-coding into the hypervisor (but JUST the existing vmxassist-
> specific portion, not the general vPIC code), then the patch reduces to
> just a few lines, as below.
Reposted, this time complete with commit header and signed-off-by.

--Stephen


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 31/5/07 21:35, "Stephen C. Tweedie" <sct@redhat.com> wrote:
> This turned out to be trivial to do.  I was just stalled for a while in
> testing it since it uncovered another regression --- Xen FV seems to
> have problem with the latest dynticks patches from FC7 kernels.
> 
> The new patch is much simpler; at the same time it''s uglier in
that it''s
> an (even more) gross layering violation.
> 
> vmxassist does the rewriting of the guests vPIC vectors, so the HV
> *cannot* have any idea what vectors the guest actually intended to write
> when we go back to 32-bit mode.  However, vmxassist already hard-codes
> the vectors in several places: the 16-bit irq_bases are hard-coded to
> 0x08 and 0x70 in the trap bouncing code, and those bounce traps are
> hard-coded to live at 0x20 and 0x28.  If we are willing to extend that
> hard-coding into the hypervisor (but JUST the existing vmxassist-
> specific portion, not the general vPIC code), then the patch reduces to
> just a few lines, as below.
That''s quite nasty, but I think that''s okay in this case.
I''ll apply it.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

>This code teaches vmxassist:
>
>1) to remember how the guest has set up the virtual PIC interrupt
>vectors; and
>2) to reprogram the vPIC to point to the true guest vector 
>when entering
>32-bit mode, and to point back to the vmxassist bounce-irq traps when
>reentering 16-bit mode.
>
In Dec 2006, I created a patch to fix this issue, and may be better.
I didn''t send it out just because some other critical issue in
VMXassist, hopefully the emulation way will be done soon.
thanks
-Xin 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

RHEL4U4 PAE SMP guest will crash on current xen-unstable tree, and we
found changeset 15214 introduced it, test show that the attached patch
can fix the crash.



Respect irqbase set by protected mode in mode switching with VMXAssist.
Signed-off-by: Xin Li <xin.b.li@intel.com>
Signed-off-by: Dexuan Cui <dexuan.cui@intel.com>


>-----Original Message-----
>From: xen-devel-bounces@lists.xensource.com 
>[mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Li, Xin B
>Sent: Thursday, June 07, 2007 12:34 AM
>To: Stephen C. Tweedie; xen-devel@lists.xensource.com
>Subject: RE: [Xen-devel] [patch 2/2] Preserve correct PIC 
>vectors acrossXenvmxassist 16/32-bit transitions
>
>
>>This code teaches vmxassist:
>>
>>1) to remember how the guest has set up the virtual PIC interrupt
>>vectors; and
>>2) to reprogram the vPIC to point to the true guest vector 
>>when entering
>>32-bit mode, and to point back to the vmxassist bounce-irq traps when
>>reentering 16-bit mode.
>>
>
>In Dec 2006, I created a patch to fix this issue, and may be better.
>I didn''t send it out just because some other critical issue in
>VMXassist, hopefully the emulation way will be done soon.
>thanks
>-Xin 
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel