[PATCH] support piix PAM registers in KVM

[Gleb Natapov]

Without this BIOS fails to remap 0xf0000 memory from ROM to RAM so writes
to F-segment modify ROM content instead of memory copy. Since QEMU does
not reloads ROMs during reset on next boot modified copy of BIOS is used.

Signed-off-by: Gleb Natapov 
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 933ad86..0bf435d 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -99,10 +99,6 @@ static void i440fx_update_memory_mappings(PCII440FXState *d)
     int i, r;
     uint32_t smram, addr;

-    if (kvm_enabled()) {
-        /* FIXME: Support remappings and protection changes. */
-        return;
-    }
     update_pam(d, 0xf0000, 0x100000, (d->dev.config[I440FX_PAM] >> 4) & 3);
     for(i = 0; i < 12; i++) {
         r = (d->dev.config[(i >> 1) + (I440FX_PAM + 1)] >> ((i & 1) * 4)) & 3;
--
    		Gleb.

[Marcelo Tosatti]

On Tue, Sep 21, 2010 at 02:31:42PM +0200, Gleb Natapov wrote:
> Without this BIOS fails to remap 0xf0000 memory from ROM to RAM so writes
> to F-segment modify ROM content instead of memory copy. Since QEMU does
> not reloads ROMs during reset on next boot modified copy of BIOS is used.
>
> Signed-off-by: Gleb Natapov 
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index 933ad86..0bf435d 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -99,10 +99,6 @@ static void i440fx_update_memory_mappings(PCII440FXState *d)
>      int i, r;
>      uint32_t smram, addr;
> if (kvm_enabled()) {
>    /* FIXME: Support remappings and protection changes. */
>    return;
> }
> update_pam(d, 0xf0000, 0x100000, (d->dev.config[I440FX_PAM] >> 4) & 3);
> for(i = 0; i < 12; i++) {
>     r = (d->dev.config[(i >> 1) + (I440FX_PAM + 1)] >> ((i & 1) * 4)) & 3;

Ugh, we should support ro slots to avoid such kind of problems...

Applied, thanks.

[Kevin O'Connor]

On Tue, Sep 21, 2010 at 04:06:01PM -0300, Marcelo Tosatti wrote:
> On Tue, Sep 21, 2010 at 02:31:42PM +0200, Gleb Natapov wrote:
> > Without this BIOS fails to remap 0xf0000 memory from ROM to RAM so writes
> > to F-segment modify ROM content instead of memory copy. Since QEMU does
> > not reloads ROMs during reset on next boot modified copy of BIOS is used.
> >
> > Signed-off-by: Gleb Natapov
>
> Applied, thanks.

Thanks.  I've committed the seabios relocation patches that depended
on this fix.

-Kevin

[Jason Krieg]

On 09/21/2010 02:31 PM, Gleb Natapov wrote:
> Without this BIOS fails to remap 0xf0000 memory from ROM to RAM so writes
> to F-segment modify ROM content instead of memory copy. Since QEMU does
> not reloads ROMs during reset on next boot modified copy of BIOS is used.
>
> Signed-off-by: Gleb Natapov
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index 933ad86..0bf435d 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -99,10 +99,6 @@ static void i440fx_update_memory_mappings(PCII440FXState *d)
>       int i, r;
>       uint32_t smram, addr;
> if (kvm_enabled()) {
>    /* FIXME: Support remappings and protection changes. */
>    return;
> }
>  update_pam(d, 0xf0000, 0x100000, (d->dev.config[I440FX_PAM]>>  4)&  3);
>  for(i = 0; i<  12; i++) {
>      r = (d->dev.config[(i>>  1) + (I440FX_PAM + 1)]>>  ((i&  1) * 4))&  3;

Hi,

While testing migration from old to new kvm ( 0.12.5 to 0.14.x ) and 
after fixing
some other problems mainly version_id probs in some of the 
VMStateDescriptions
everything was working fine until I tried to migrate Windows guests they 
would crash
after running some time. Linux guests are running stable.

So I decided to do a git bisect to identify the according commit.
Reverting this commit fixes this problem with Windows guests.

What consequences might it have not updating these memory mappings ?

Does this commit need a specific seabios version, we have seabios 0.6.0 
with qemu-kvm 0.12.5
and seabios 0.6.1.2 with qemu-kvm 0.14.1 ?

Maybe instead of reverting this commit one could check the seabios 
version in this method
and only do an update of these piix PAM registers if running with a 
newer seabios version ?

Regards,
Jason

[Avi Kivity]

On 06/15/2011 02:58 PM, Jason Krieg wrote:
> On 09/21/2010 02:31 PM, Gleb Natapov wrote:
> > Without this BIOS fails to remap 0xf0000 memory from ROM to RAM so 
> > writes
> > to F-segment modify ROM content instead of memory copy. Since QEMU does
> > not reloads ROMs during reset on next boot modified copy of BIOS is 
> > used.
> >
> > Signed-off-by: Gleb Natapov
> > diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> > index 933ad86..0bf435d 100644
> > --- a/hw/piix_pci.c
> > +++ b/hw/piix_pci.c
> > @@ -99,10 +99,6 @@ static void 
> > i440fx_update_memory_mappings(PCII440FXState *d)
> >       int i, r;
> >       uint32_t smram, addr;
> > if (kvm_enabled()) {
> >    /* FIXME: Support remappings and protection changes. */
> >    return;
> > }
> >  update_pam(d, 0xf0000, 0x100000, (d->dev.config[I440FX_PAM]>>
> >
> > 4)&  3);
> >       for(i = 0; i<  12; i++) {
> >           r = (d->dev.config[(i>>  1) + (I440FX_PAM + 1)]>>  ((i&  1)
> > 4))&  3;
>
> Hi,
>
> While testing migration from old to new kvm ( 0.12.5 to 0.14.x ) and 
> after fixing
> some other problems mainly version_id probs in some of the 
> VMStateDescriptions
> everything was working fine until I tried to migrate Windows guests 
> they would crash
> after running some time. Linux guests are running stable.
>
> So I decided to do a git bisect to identify the according commit.
> Reverting this commit fixes this problem with Windows guests.
>
> What consequences might it have not updating these memory mappings ?

Resets may fail.
> Does this commit need a specific seabios version, we have seabios 
> 0.6.0 with qemu-kvm 0.12.5
> and seabios 0.6.1.2 with qemu-kvm 0.14.1 ?

IIUC newer seabios depends on this commit, but this commit does not 
depend on seabios.
> Maybe instead of reverting this commit one could check the seabios 
> version in this method
> and only do an update of these piix PAM registers if running with a 
> newer seabios version ?

It would be better to first understand what's going wrong.

[Jason Krieg]

On 06/15/2011 03:17 PM, Avi Kivity wrote:
> On 06/15/2011 02:58 PM, Jason Krieg wrote:
> > On 09/21/2010 02:31 PM, Gleb Natapov wrote:
> > > Without this BIOS fails to remap 0xf0000 memory from ROM to RAM so 
> > > writes
> > > to F-segment modify ROM content instead of memory copy. Since QEMU does
> > > not reloads ROMs during reset on next boot modified copy of BIOS is 
> > > used.
> > >
> > > Signed-off-by: Gleb Natapov
> > > diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> > > index 933ad86..0bf435d 100644
> > > --- a/hw/piix_pci.c
> > > +++ b/hw/piix_pci.c
> > > @@ -99,10 +99,6 @@ static void 
> > > i440fx_update_memory_mappings(PCII440FXState *d)
> > >       int i, r;
> > >       uint32_t smram, addr;
> > > if (kvm_enabled()) {
> > >    /* FIXME: Support remappings and protection changes. */
> > >    return;
> > > }
> > >  update_pam(d, 0xf0000, 0x100000, (d->dev.config[I440FX_PAM]>>
> > >
> > > 4)&  3);
> > >       for(i = 0; i<  12; i++) {
> > >           r = (d->dev.config[(i>>  1) + (I440FX_PAM + 1)]>>  ((i&
> > > 4))&  3;
> >
> > Hi,
> >
> > While testing migration from old to new kvm ( 0.12.5 to 0.14.x ) and 
> > after fixing
> > some other problems mainly version_id probs in some of the 
> > VMStateDescriptions
> > everything was working fine until I tried to migrate Windows guests 
> > they would crash
> > after running some time. Linux guests are running stable.
> >
> > So I decided to do a git bisect to identify the according commit.
> > Reverting this commit fixes this problem with Windows guests.
> >
> > What consequences might it have not updating these memory mappings ?
>
> Resets may fail.
> > Does this commit need a specific seabios version, we have seabios 
> > 0.6.0 with qemu-kvm 0.12.5
> > and seabios 0.6.1.2 with qemu-kvm 0.14.1 ?
>
> IIUC newer seabios depends on this commit, but this commit does not 
> depend on seabios.
> > Maybe instead of reverting this commit one could check the seabios 
> > version in this method
> > and only do an update of these piix PAM registers if running with a 
> > newer seabios version ?
>
> It would be better to first understand what's going wrong.

I presume (just guessing) Windows wrote to F-segment modifying the ROM 
and now gets
confused if these PAM registers are updated.

Any ideas for what I could look for ?

[Avi Kivity]

On 06/15/2011 04:54 PM, Jason Krieg wrote:
> > > Maybe instead of reverting this commit one could check the seabios 
> > > version in this method
> > > and only do an update of these piix PAM registers if running with a 
> > > newer seabios version ?
> >
> > It would be better to first understand what's going wrong.
>
> I presume (just guessing) Windows wrote to F-segment modifying the ROM 
> and now gets
> confused if these PAM registers are updated.

It shouldn't, really.
> Any ideas for what I could look for ?

Try -no-tpr-opt (on both sides).

[Avi Kivity]

On 06/15/2011 04:57 PM, Avi Kivity wrote:
> > Any ideas for what I could look for ?
>
> Try -no-tpr-opt (on both sides).

There's no such option.  Try commenting out the device_init line of 
kvm-tpr-opt.c.

[Jason Krieg]

On 06/15/2011 03:59 PM, Avi Kivity wrote:
> On 06/15/2011 04:57 PM, Avi Kivity wrote:
> > > Any ideas for what I could look for ?
> >
> > Try -no-tpr-opt (on both sides).
>
> There's no such option.  Try commenting out the device_init line of 
> kvm-tpr-opt.c.

with 0.12.5 there is no device_init in kvm-tpr-opt.c
should I still try without the device_init() only with 0.14.1 ?

[Jason Krieg]

On 06/15/2011 04:11 PM, Jason Krieg wrote:
> On 06/15/2011 03:59 PM, Avi Kivity wrote:
> > On 06/15/2011 04:57 PM, Avi Kivity wrote:
> > > > Any ideas for what I could look for ?
> > >
> > > Try -no-tpr-opt (on both sides).
> >
> > There's no such option.  Try commenting out the device_init line of 
> > kvm-tpr-opt.c.
>
> with 0.12.5 there is no device_init in kvm-tpr-opt.c
> should I still try without the device_init() only with 0.14.1 ?

removing kvm_tpr_opt_setup() in qemu-kvm-x86.c is probably the 
equivalent to device_init(kvm_tpr_opt_setup)

[Jason Krieg]

On 06/15/2011 03:59 PM, Avi Kivity wrote:
> On 06/15/2011 04:57 PM, Avi Kivity wrote:
> > > Any ideas for what I could look for ?
> >
> > Try -no-tpr-opt (on both sides).
>
> There's no such option.  Try commenting out the device_init line of 
> kvm-tpr-opt.c.

I disabled this on both sides but still the same behaviour windows 
crashes after ~45min

[Avi Kivity]

On 06/15/2011 06:16 PM, Jason Krieg wrote:
> On 06/15/2011 03:59 PM, Avi Kivity wrote:
> > On 06/15/2011 04:57 PM, Avi Kivity wrote:
> > > > Any ideas for what I could look for ?
> > >
> > > Try -no-tpr-opt (on both sides).
> >
> > There's no such option.  Try commenting out the device_init line of 
> > kvm-tpr-opt.c.
>
> I disabled this on both sides but still the same behaviour windows 
> crashes after ~45min

45 minutes!

Anything from the crash to indicate what's wrong?

[Jason Krieg]

On 06/15/2011 05:19 PM, Avi Kivity wrote:
> On 06/15/2011 06:16 PM, Jason Krieg wrote:
> > On 06/15/2011 03:59 PM, Avi Kivity wrote:
> > > On 06/15/2011 04:57 PM, Avi Kivity wrote:
> > > > > Any ideas for what I could look for ?
> > > >
> > > > Try -no-tpr-opt (on both sides).
> > >
> > > There's no such option.  Try commenting out the device_init line of 
> > > kvm-tpr-opt.c.
> >
> > I disabled this on both sides but still the same behaviour windows 
> > crashes after ~45min
>
> 45 minutes!
>
> Anything from the crash to indicate what's wrong?

debugging Windows is not really easy mostly just saying nothing.

windows runs at first normally but after some time somewhere between a 
few minutes and 1.5h
the windows kernel process goes crazy running with 100% cpu and after 
running like this for a few
more minutes windows is completely frozen.

[Avi Kivity]

On 06/15/2011 06:27 PM, Jason Krieg wrote:
> On 06/15/2011 05:19 PM, Avi Kivity wrote:
> > On 06/15/2011 06:16 PM, Jason Krieg wrote:
> > > On 06/15/2011 03:59 PM, Avi Kivity wrote:
> > > > On 06/15/2011 04:57 PM, Avi Kivity wrote:
> > > > > > Any ideas for what I could look for ?
> > > > >
> > > > > Try -no-tpr-opt (on both sides).
> > > >
> > > > There's no such option.  Try commenting out the device_init line of 
> > > > kvm-tpr-opt.c.
> > >
> > > I disabled this on both sides but still the same behaviour windows 
> > > crashes after ~45min
> >
> > 45 minutes!
> >
> > Anything from the crash to indicate what's wrong?
>
> debugging Windows is not really easy mostly just saying nothing.
>
> windows runs at first normally but after some time somewhere between a 
> few minutes and 1.5h
> the windows kernel process goes crazy running with 100% cpu and after 
> running like this for a few
> more minutes windows is completely frozen.

Sometimes examining the running code and registers in the qemu monitor, 
perhaps resolving symbols via WinDbg, helps in getting a clue...  it's 
not easy though.