All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Gleb Natapov <gleb@redhat.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>,
	Avi Kivity <avi@redhat.com>, kvm-devel <kvm@vger.kernel.org>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
Subject: Re: Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Date: Fri, 9 Oct 2009 12:13:00 +0200	[thread overview]
Message-ID: <20091009101259.GA12027@redhat.com> (raw)
In-Reply-To: <20091009070049.GG19692@redhat.com>

On Fri, Oct 09, 2009 at 09:00:49AM +0200, Gleb Natapov wrote:
> On Fri, Oct 09, 2009 at 08:43:59AM +0200, Michael S. Tsirkin wrote:
> > On Thu, Oct 08, 2009 at 07:40:12PM +0100, Jamie Lokier wrote:
> > > Avi Kivity wrote:
> > > > On 10/08/2009 06:06 PM, Michael S. Tsirkin wrote:
> > > > >On Thu, Oct 08, 2009 at 05:29:29PM +0200, Avi Kivity wrote:
> > > > >   
> > > > >>On 10/08/2009 04:52 PM, Michael S. Tsirkin wrote:
> > > > >>     
> > > > >>>PCI memory should be disabled at reset, otherwise
> > > > >>>we might claim transactions at address 0.
> > > > >>>I/O should also be disabled, although for cirrus
> > > > >>>it is harmless to enable it as we do not
> > > > >>>have I/O bar.
> > > > >>>
> > > > >>>Note: need bios fix for this patch to work:
> > > > >>>currently pc-bios incorrently assumes that it does not
> > > > >>>need to enable i/o unless device has i/o bar.
> > > > >>>
> > > > >>>Signed-off-by: Michael S. Tsirkin<mst@redhat.com>
> > > > >>
> > > > >>This needs to be conditional on the machine type.  Old machines must
> > > > >>retain this code for live migration to work (we need to live migrate the
> > > > >>bios, so we can't assume the bios fix is in during live migration from
> > > > >>older qemus).
> > > > >>     
> > > > >No, if you migrate from older qemu you will be fine as command
> > > > >is enabled there on init.
> > > > >   
> > > > 
> > > > Right.
> > > 
> > > No, I think Avi was right the first time.
> > > 
> > > Migrating from an older qemu will be fine at first, but at the next
> > > reset _following_ migration, it'll be running the old BIOS on a new
> > > qemu and fail.
> > > 
> > > -- Jamie
> > 
> > I think this just means that there's another bug.
> > On reset BIOS should be re-read from flash.
> > qemu instead keeps it in RAM, this means that if
> > guest corrupts it, or BIOS itself runs self-modifying
> > code (which is not uncommon btw), bad things happen.
> > 
> How do you know it is not uncommon (or happens at all?)

I know memory corruptions are not uncommon, from experience :)

> If BIOS is not shadowed it runs directly from ROM. Hard to use
> self-modifying code there.

Sorry I don't really know how this is handled in qemu.
Can it run BIOS from ROM? Is ROM content sent over for migration?
I see this
     ret = load_image(filename, qemu_get_ram_ptr(bios_offset));
which seems to always load BIOS into RAM.
Maybe I misunderstand. Could you enlighten me please?

> > We should fix qemu to re-read bios from flash.
> > Makes sense?
> > 
> We have option_rom_setup_reset() already.

We don't, anymore :)

> Don't we use it for BIOSes
> too?
> 
> > More long-term, we have duplication between reset and init
> > routines. Maybe devices really should have init and cleanup,
> > and on reset we'd cleanup all devices and then init them again?
> > There are cases where state needs to be persistent across
> > resets (VPD comes to mind) but these are rare,
> > and probably need to be backed by writing to file anyway?
> > 
> > 
> > -- 
> > MST
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> 			Gleb.

WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Gleb Natapov <gleb@redhat.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>,
	Avi Kivity <avi@redhat.com>, kvm-devel <kvm@vger.kernel.org>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Date: Fri, 9 Oct 2009 12:13:00 +0200	[thread overview]
Message-ID: <20091009101259.GA12027@redhat.com> (raw)
In-Reply-To: <20091009070049.GG19692@redhat.com>

On Fri, Oct 09, 2009 at 09:00:49AM +0200, Gleb Natapov wrote:
> On Fri, Oct 09, 2009 at 08:43:59AM +0200, Michael S. Tsirkin wrote:
> > On Thu, Oct 08, 2009 at 07:40:12PM +0100, Jamie Lokier wrote:
> > > Avi Kivity wrote:
> > > > On 10/08/2009 06:06 PM, Michael S. Tsirkin wrote:
> > > > >On Thu, Oct 08, 2009 at 05:29:29PM +0200, Avi Kivity wrote:
> > > > >   
> > > > >>On 10/08/2009 04:52 PM, Michael S. Tsirkin wrote:
> > > > >>     
> > > > >>>PCI memory should be disabled at reset, otherwise
> > > > >>>we might claim transactions at address 0.
> > > > >>>I/O should also be disabled, although for cirrus
> > > > >>>it is harmless to enable it as we do not
> > > > >>>have I/O bar.
> > > > >>>
> > > > >>>Note: need bios fix for this patch to work:
> > > > >>>currently pc-bios incorrently assumes that it does not
> > > > >>>need to enable i/o unless device has i/o bar.
> > > > >>>
> > > > >>>Signed-off-by: Michael S. Tsirkin<mst@redhat.com>
> > > > >>
> > > > >>This needs to be conditional on the machine type.  Old machines must
> > > > >>retain this code for live migration to work (we need to live migrate the
> > > > >>bios, so we can't assume the bios fix is in during live migration from
> > > > >>older qemus).
> > > > >>     
> > > > >No, if you migrate from older qemu you will be fine as command
> > > > >is enabled there on init.
> > > > >   
> > > > 
> > > > Right.
> > > 
> > > No, I think Avi was right the first time.
> > > 
> > > Migrating from an older qemu will be fine at first, but at the next
> > > reset _following_ migration, it'll be running the old BIOS on a new
> > > qemu and fail.
> > > 
> > > -- Jamie
> > 
> > I think this just means that there's another bug.
> > On reset BIOS should be re-read from flash.
> > qemu instead keeps it in RAM, this means that if
> > guest corrupts it, or BIOS itself runs self-modifying
> > code (which is not uncommon btw), bad things happen.
> > 
> How do you know it is not uncommon (or happens at all?)

I know memory corruptions are not uncommon, from experience :)

> If BIOS is not shadowed it runs directly from ROM. Hard to use
> self-modifying code there.

Sorry I don't really know how this is handled in qemu.
Can it run BIOS from ROM? Is ROM content sent over for migration?
I see this
     ret = load_image(filename, qemu_get_ram_ptr(bios_offset));
which seems to always load BIOS into RAM.
Maybe I misunderstand. Could you enlighten me please?

> > We should fix qemu to re-read bios from flash.
> > Makes sense?
> > 
> We have option_rom_setup_reset() already.

We don't, anymore :)

> Don't we use it for BIOSes
> too?
> 
> > More long-term, we have duplication between reset and init
> > routines. Maybe devices really should have init and cleanup,
> > and on reset we'd cleanup all devices and then init them again?
> > There are cases where state needs to be persistent across
> > resets (VPD comes to mind) but these are rare,
> > and probably need to be backed by writing to file anyway?
> > 
> > 
> > -- 
> > MST
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> 			Gleb.

  reply	other threads:[~2009-10-09 10:13 UTC|newest]

Thread overview: 101+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1255013225.git.mst@redhat.com>
2009-10-08 14:52 ` [PATCH 1/3] pc-bios: enable io/memory unconditionally Michael S. Tsirkin
2009-10-08 14:52   ` [Qemu-devel] " Michael S. Tsirkin
2009-10-08 14:52 ` [PATCH 2/3] qemu: make cirrus init value pci spec compliant Michael S. Tsirkin
2009-10-08 14:52   ` [Qemu-devel] " Michael S. Tsirkin
2009-10-08 15:29   ` Avi Kivity
2009-10-08 15:29     ` [Qemu-devel] " Avi Kivity
2009-10-08 16:06     ` Michael S. Tsirkin
2009-10-08 16:06       ` [Qemu-devel] " Michael S. Tsirkin
2009-10-08 16:13       ` Avi Kivity
2009-10-08 16:13         ` [Qemu-devel] " Avi Kivity
2009-10-08 18:40         ` Jamie Lokier
2009-10-08 18:40           ` Jamie Lokier
2009-10-08 19:39           ` Michael S. Tsirkin
2009-10-08 19:39             ` Michael S. Tsirkin
2009-10-09  6:43           ` Michael S. Tsirkin
2009-10-09  6:43             ` Michael S. Tsirkin
2009-10-09  7:00             ` Gleb Natapov
2009-10-09  7:00               ` Gleb Natapov
2009-10-09 10:13               ` Michael S. Tsirkin [this message]
2009-10-09 10:13                 ` Michael S. Tsirkin
2009-10-09 11:49                 ` Gleb Natapov
2009-10-09 11:49                   ` Gleb Natapov
2009-10-09 12:59                   ` Michael S. Tsirkin
2009-10-09 12:59                     ` Michael S. Tsirkin
2009-10-09 15:08                     ` Gleb Natapov
2009-10-09 15:08                       ` Gleb Natapov
2009-10-12 13:45                   ` Gerd Hoffmann
2009-10-12 13:45                     ` Gerd Hoffmann
2009-10-12 13:53                     ` Gleb Natapov
2009-10-12 13:53                       ` Gleb Natapov
2009-10-12 14:50                       ` Gerd Hoffmann
2009-10-12 14:50                         ` Gerd Hoffmann
2009-10-12 14:52                         ` Gleb Natapov
2009-10-12 14:52                           ` Gleb Natapov
2009-10-12 15:21                           ` Jamie Lokier
2009-10-12 15:21                             ` Jamie Lokier
2009-10-12 15:43                           ` Gerd Hoffmann
2009-10-12 15:43                             ` [Qemu-devel] " Gerd Hoffmann
2009-10-12 16:57                             ` Michael S. Tsirkin
2009-10-12 16:57                               ` Michael S. Tsirkin
2009-10-09 11:37             ` Jamie Lokier
2009-10-09 11:37               ` Jamie Lokier
2009-10-09 11:54               ` Gleb Natapov
2009-10-09 11:54                 ` Gleb Natapov
2009-10-11 13:36           ` Michael S. Tsirkin
2009-10-11 13:36             ` Michael S. Tsirkin
2009-10-11 13:45             ` Gleb Natapov
2009-10-11 13:45               ` Gleb Natapov
2009-10-11 13:52               ` Michael S. Tsirkin
2009-10-11 13:52                 ` [Qemu-devel] " Michael S. Tsirkin
2009-10-11 13:57                 ` Gleb Natapov
2009-10-11 13:57                   ` Gleb Natapov
2009-10-11 14:35                   ` Michael S. Tsirkin
2009-10-11 14:35                     ` Michael S. Tsirkin
2009-10-11 14:39                     ` Gleb Natapov
2009-10-11 14:39                       ` Gleb Natapov
2009-10-11 14:45                       ` Michael S. Tsirkin
2009-10-11 14:45                         ` Michael S. Tsirkin
2009-10-11 15:00                         ` Gleb Natapov
2009-10-11 15:00                           ` [Qemu-devel] " Gleb Natapov
2009-10-12 13:44                       ` Anthony Liguori
2009-10-12 13:44                         ` [Qemu-devel] " Anthony Liguori
2009-10-12 14:21                         ` Gleb Natapov
2009-10-12 14:21                           ` Gleb Natapov
2009-10-12 14:31                           ` Anthony Liguori
2009-10-12 14:31                             ` Anthony Liguori
2009-10-12 15:00                         ` Kevin O'Connor
2009-10-12 15:00                           ` Kevin O'Connor
2009-10-11 14:42                     ` Avi Kivity
2009-10-11 14:42                       ` Avi Kivity
2009-10-11 14:44                       ` Michael S. Tsirkin
2009-10-11 14:44                         ` Michael S. Tsirkin
2009-10-11 14:53                         ` Avi Kivity
2009-10-11 14:53                           ` Avi Kivity
2009-10-11 16:12                           ` Michael S. Tsirkin
2009-10-11 16:12                             ` Michael S. Tsirkin
2009-10-11 16:24                             ` Avi Kivity
2009-10-11 16:24                               ` Avi Kivity
2009-10-11 16:33                               ` Michael S. Tsirkin
2009-10-11 16:33                                 ` Michael S. Tsirkin
2009-10-11 16:37                                 ` Avi Kivity
2009-10-11 16:37                                   ` Avi Kivity
2009-10-11 16:39                                   ` Michael S. Tsirkin
2009-10-11 16:39                                     ` [Qemu-devel] " Michael S. Tsirkin
2009-10-11 16:48                                     ` Avi Kivity
2009-10-11 16:48                                       ` [Qemu-devel] " Avi Kivity
2009-10-11 17:18                                       ` Michael S. Tsirkin
2009-10-11 17:18                                         ` Michael S. Tsirkin
2009-10-11 17:27                                         ` Avi Kivity
2009-10-11 17:27                                           ` Avi Kivity
2009-10-11 17:28                                           ` Avi Kivity
2009-10-11 17:28                                             ` Avi Kivity
2009-10-11 18:10                                             ` Michael S. Tsirkin
2009-10-11 18:10                                               ` Michael S. Tsirkin
2009-10-11 20:41                                               ` Avi Kivity
2009-10-11 20:41                                                 ` Avi Kivity
2009-10-08 14:52 ` [PATCH 3/3] qemu: cleanup unused macros in cirrus Michael S. Tsirkin
2009-10-08 14:52   ` [Qemu-devel] " Michael S. Tsirkin
2009-10-08 15:24   ` Juan Quintela
2009-10-08 16:07     ` Michael S. Tsirkin
2009-10-08 16:07       ` [Qemu-devel] " Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20091009101259.GA12027@redhat.com \
    --to=mst@redhat.com \
    --cc=aliguori@us.ibm.com \
    --cc=avi@redhat.com \
    --cc=gleb@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.