| Message ID | 20091008145241.GC12275@redhat.com |
|---|---|
| State | New |
| Headers | show |
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> > --- > hw/cirrus_vga.c | 1 - > 1 files changed, 0 insertions(+), 1 deletions(-) > > diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c > index 1feb827..c54ac65 100644 > --- a/hw/cirrus_vga.c > +++ b/hw/cirrus_vga.c > @@ -3248,7 +3248,6 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev) > /* setup PCI */ > pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_CIRRUS); > pci_config_set_device_id(pci_conf, device_id); > - pci_conf[0x04] = PCI_COMMAND_IOACCESS | PCI_COMMAND_MEMACCESS; > pci_config_set_class(pci_conf, PCI_CLASS_DISPLAY_VGA); > pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; > 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).
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> >> --- >> hw/cirrus_vga.c | 1 - >> 1 files changed, 0 insertions(+), 1 deletions(-) >> >> diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c >> index 1feb827..c54ac65 100644 >> --- a/hw/cirrus_vga.c >> +++ b/hw/cirrus_vga.c >> @@ -3248,7 +3248,6 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev) >> /* setup PCI */ >> pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_CIRRUS); >> pci_config_set_device_id(pci_conf, device_id); >> - pci_conf[0x04] = PCI_COMMAND_IOACCESS | PCI_COMMAND_MEMACCESS; >> pci_config_set_class(pci_conf, PCI_CLASS_DISPLAY_VGA); >> pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; >> > > 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. Only migration *to* older qemu might create a problem and then only if you manage to migrate after pci was reset but before bios set the register, which is a small window. Do we really want to update machine type for this? > -- > error compiling committee.c: too many arguments to function
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> >>> --- >>> hw/cirrus_vga.c | 1 - >>> 1 files changed, 0 insertions(+), 1 deletions(-) >>> >>> diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c >>> index 1feb827..c54ac65 100644 >>> --- a/hw/cirrus_vga.c >>> +++ b/hw/cirrus_vga.c >>> @@ -3248,7 +3248,6 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev) >>> /* setup PCI */ >>> pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_CIRRUS); >>> pci_config_set_device_id(pci_conf, device_id); >>> - pci_conf[0x04] = PCI_COMMAND_IOACCESS | PCI_COMMAND_MEMACCESS; >>> pci_config_set_class(pci_conf, PCI_CLASS_DISPLAY_VGA); >>> pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; >>> >>> >> 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. > Only migration *to* older qemu might create a problem and then only if > you manage to migrate after pci was reset but before bios set the > register, which is a small window. > > Do we really want to update machine type for this? > No, migration to older qemu is not supported.
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
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. OK but Avi was commenting on the patch that removed bit set in init, ignoring reset behaviour. So for old BIOS we want a flag not to clear I/O enable bit on reset for VGA devices. > -- Jamie
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. We should fix qemu to re-read bios from flash. Makes sense? 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?
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?) If BIOS is not shadowed it runs directly from ROM. Hard to use self-modifying code there. > We should fix qemu to re-read bios from flash. > Makes sense? > We have option_rom_setup_reset() already. 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.
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.
Michael S. Tsirkin wrote: > 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? It sounds look a good idea to me. That is, after all, what hardware reset often does :-) -- Jamie
On Fri, Oct 09, 2009 at 12:13:00PM +0200, Michael S. Tsirkin wrote: > 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 :) > I don't see how from presence of memory corruptions you made a conclusion that BIOSes run self-modifying code. > > 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? Yes it can. There is no much difference between ROM and RAM in qemu though. I don't know if ROM content is sent over for migration, but IMHO it shouldn't. This will solve the problem we currently discuss. Shadowed BIOS should be copied of course. > 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? Enlighten you about what? BIOS copies itself into shadow ram. At least BOCHS bios does. I hope seabios does the same. > > > > We should fix qemu to re-read bios from flash. > > > Makes sense? > > > > > We have option_rom_setup_reset() already. > > We don't, anymore :) We are fast! Well, we surely have something similar instead. > > > 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. -- Gleb.
On Fri, Oct 09, 2009 at 12:37:38PM +0100, Jamie Lokier wrote: > Michael S. Tsirkin wrote: > > 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? > > It sounds look a good idea to me. That is, after all, what hardware > reset often does :-) > That is a two stage reset that was discussed already and deemed to be the way to go. -- Gleb.
On Fri, Oct 09, 2009 at 01:49:03PM +0200, Gleb Natapov wrote: > On Fri, Oct 09, 2009 at 12:13:00PM +0200, Michael S. Tsirkin wrote: > > 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 :) > > > I don't see how from presence of memory corruptions you made a > conclusion that BIOSes run self-modifying code. > > > 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? > Yes it can. There is no much difference between ROM and RAM in qemu > though. I don't know if ROM content is sent over for migration, but > IMHO it shouldn't. This will solve the problem we currently discuss. > Shadowed BIOS should be copied of course. Interesting. OTOH this means that after migration, shadowed BIOS and ROM are different. In theory, this can create problems. In practice I doubt this. > > 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? > Enlighten you about what? BIOS copies itself into shadow ram. At least > BOCHS bios does. I hope seabios does the same. People were implying changing BIOS triggers migration/reset issues. Do you think so too? > > > > > > We should fix qemu to re-read bios from flash. > > > > Makes sense? > > > > > > > We have option_rom_setup_reset() already. > > > > We don't, anymore :) > We are fast! Well, we surely have something similar instead. > > > > > > 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. > > -- > Gleb.
On Fri, Oct 09, 2009 at 02:59:29PM +0200, Michael S. Tsirkin wrote: > On Fri, Oct 09, 2009 at 01:49:03PM +0200, Gleb Natapov wrote: > > On Fri, Oct 09, 2009 at 12:13:00PM +0200, Michael S. Tsirkin wrote: > > > 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 :) > > > > > I don't see how from presence of memory corruptions you made a > > conclusion that BIOSes run self-modifying code. > > > > 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? > > Yes it can. There is no much difference between ROM and RAM in qemu > > though. I don't know if ROM content is sent over for migration, but > > IMHO it shouldn't. This will solve the problem we currently discuss. > > Shadowed BIOS should be copied of course. > > Interesting. OTOH this means that after migration, > shadowed BIOS and ROM are different. In theory, this > can create problems. In practice I doubt this. > What kind of problems? After ROM BIOS is shadowed it is no logger used until reset. Its RAM copy is used. > > > 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? > > Enlighten you about what? BIOS copies itself into shadow ram. At least > > BOCHS bios does. I hope seabios does the same. > > People were implying changing BIOS triggers migration/reset issues. > Do you think so too? > In this thread people seems to be implying that _not_ changing BIOS trigger the problem on reset if destination no logger init VGA register and relies on BIOS to do that instead. Changing RAM copy of the BIOS at runtime may cause problems with TPR patching for instance. Changing ROM copy shouldn't be the problem. It will be the same as burning new ROM from running system. Until reset RAM copy of a BIOS will be used, after reset new BIOS will run and will shadow itself. > > > > > > > > We should fix qemu to re-read bios from flash. > > > > > Makes sense? > > > > > > > > > We have option_rom_setup_reset() already. > > > > > > We don't, anymore :) > > We are fast! Well, we surely have something similar instead. > > > > > > > > > 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. > > > > -- > > Gleb. -- Gleb.
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 So after discussion with Gleb, it seems that what will happen after migration to new qemu, old BIOS is shadowed, but memory is already enabled. After reset, new BIOS will be shadowed and it will enable memory. Makes sense?
On Sun, Oct 11, 2009 at 03:36:26PM +0200, Michael S. Tsirkin wrote: > > 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 > > So after discussion with Gleb, it seems that what will happen > after migration to new qemu, old BIOS is shadowed, but memory > is already enabled. After reset, new BIOS will be shadowed > and it will enable memory. Makes sense? > I don't see how you made this conclusion after the discussion. What should happen is after migration old bios should be used until reboot. After reboot new bios should be used. What really happens today I don't know. -- Gleb.
On Sun, Oct 11, 2009 at 03:45:59PM +0200, Gleb Natapov wrote: > On Sun, Oct 11, 2009 at 03:36:26PM +0200, Michael S. Tsirkin wrote: > > > 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 > > > > So after discussion with Gleb, it seems that what will happen > > after migration to new qemu, old BIOS is shadowed, but memory > > is already enabled. After reset, new BIOS will be shadowed > > and it will enable memory. Makes sense? > > > I don't see how you made this conclusion after the discussion. > What should happen is after migration old bios should be used until reboot. > After reboot new bios should be used. I think that's what I said. > What really happens today I don't know. > -- > Gleb.
On Sun, Oct 11, 2009 at 03:52:40PM +0200, Michael S. Tsirkin wrote: > On Sun, Oct 11, 2009 at 03:45:59PM +0200, Gleb Natapov wrote: > > On Sun, Oct 11, 2009 at 03:36:26PM +0200, Michael S. Tsirkin wrote: > > > > 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 > > > > > > So after discussion with Gleb, it seems that what will happen > > > after migration to new qemu, old BIOS is shadowed, but memory > > > is already enabled. After reset, new BIOS will be shadowed > > > and it will enable memory. Makes sense? > > > > > I don't see how you made this conclusion after the discussion. > > What should happen is after migration old bios should be used until reboot. > > After reboot new bios should be used. > > I think that's what I said. > First shadowing bios has nothing to do with it. Second what do you mean by "will happen"? That is how code works now or that is how it should work? I am not sure this is how it works. -- Gleb.
On Sun, Oct 11, 2009 at 03:57:48PM +0200, Gleb Natapov wrote: > On Sun, Oct 11, 2009 at 03:52:40PM +0200, Michael S. Tsirkin wrote: > > On Sun, Oct 11, 2009 at 03:45:59PM +0200, Gleb Natapov wrote: > > > On Sun, Oct 11, 2009 at 03:36:26PM +0200, Michael S. Tsirkin wrote: > > > > > 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 > > > > > > > > So after discussion with Gleb, it seems that what will happen > > > > after migration to new qemu, old BIOS is shadowed, but memory > > > > is already enabled. After reset, new BIOS will be shadowed > > > > and it will enable memory. Makes sense? > > > > > > > I don't see how you made this conclusion after the discussion. > > > What should happen is after migration old bios should be used until reboot. > > > After reboot new bios should be used. > > > > I think that's what I said. > > > First shadowing bios has nothing to do with it. Second what do you mean > by "will happen"? That is how code works now or that is how it should > work? I am not sure this is how it works. No, this is not how it works, I think. Currently BIOS ROM is loaded only on init, directly into qemu memory. BTW, I don't think it's write-protected and it probably should be? > > -- > Gleb.
On Sun, Oct 11, 2009 at 04:35:35PM +0200, Michael S. Tsirkin wrote: > On Sun, Oct 11, 2009 at 03:57:48PM +0200, Gleb Natapov wrote: > > On Sun, Oct 11, 2009 at 03:52:40PM +0200, Michael S. Tsirkin wrote: > > > On Sun, Oct 11, 2009 at 03:45:59PM +0200, Gleb Natapov wrote: > > > > On Sun, Oct 11, 2009 at 03:36:26PM +0200, Michael S. Tsirkin wrote: > > > > > > 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 > > > > > > > > > > So after discussion with Gleb, it seems that what will happen > > > > > after migration to new qemu, old BIOS is shadowed, but memory > > > > > is already enabled. After reset, new BIOS will be shadowed > > > > > and it will enable memory. Makes sense? > > > > > > > > > I don't see how you made this conclusion after the discussion. > > > > What should happen is after migration old bios should be used until reboot. > > > > After reboot new bios should be used. > > > > > > I think that's what I said. > > > > > First shadowing bios has nothing to do with it. Second what do you mean > > by "will happen"? That is how code works now or that is how it should > > work? I am not sure this is how it works. > > No, this is not how it works, I think. > Currently BIOS ROM is loaded only on init, directly into qemu memory. And have you checked if it is transfered to destination on migrate? > BTW, I don't think it's write-protected and it probably should be? > AFAIR it is not writable on plain qemu. -- Gleb.
On 10/11/2009 04:35 PM, Michael S. Tsirkin wrote: > No, this is not how it works, I think. > Currently BIOS ROM is loaded only on init, directly into qemu memory. > If we change it we break in-place upgrades - install qemu, run guest, upgrade qemu, reboot guest - now the old qemu runs with a new bios which may or may not work.
On Sun, Oct 11, 2009 at 04:42:27PM +0200, Avi Kivity wrote: > On 10/11/2009 04:35 PM, Michael S. Tsirkin wrote: >> No, this is not how it works, I think. >> Currently BIOS ROM is loaded only on init, directly into qemu memory. >> > > If we change it we break in-place upgrades - install qemu, run guest, > upgrade qemu, reboot guest - now the old qemu runs with a new bios which > may or may not work. Sorry, could not parse this. If you upgraded qemu, you are running with new qemu with new bios? > -- > error compiling committee.c: too many arguments to function
On Sun, Oct 11, 2009 at 04:39:46PM +0200, Gleb Natapov wrote: > On Sun, Oct 11, 2009 at 04:35:35PM +0200, Michael S. Tsirkin wrote: > > On Sun, Oct 11, 2009 at 03:57:48PM +0200, Gleb Natapov wrote: > > > On Sun, Oct 11, 2009 at 03:52:40PM +0200, Michael S. Tsirkin wrote: > > > > On Sun, Oct 11, 2009 at 03:45:59PM +0200, Gleb Natapov wrote: > > > > > On Sun, Oct 11, 2009 at 03:36:26PM +0200, Michael S. Tsirkin wrote: > > > > > > > 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 > > > > > > > > > > > > So after discussion with Gleb, it seems that what will happen > > > > > > after migration to new qemu, old BIOS is shadowed, but memory > > > > > > is already enabled. After reset, new BIOS will be shadowed > > > > > > and it will enable memory. Makes sense? > > > > > > > > > > > I don't see how you made this conclusion after the discussion. > > > > > What should happen is after migration old bios should be used until reboot. > > > > > After reboot new bios should be used. > > > > > > > > I think that's what I said. > > > > > > > First shadowing bios has nothing to do with it. Second what do you mean > > > by "will happen"? That is how code works now or that is how it should > > > work? I am not sure this is how it works. > > > > No, this is not how it works, I think. > > Currently BIOS ROM is loaded only on init, directly into qemu memory. > And have you checked if it is transfered to destination on migrate? > > > BTW, I don't think it's write-protected and it probably should be? > > > AFAIR it is not writable on plain qemu. What makes it non-writeable? It goes: bios_offset = qemu_ram_alloc(bios_size); ret = load_image(filename, qemu_get_ram_ptr(bios_offset)); Does qemu_ram_alloc allocate read-only memory then? > -- > Gleb.
On 10/11/2009 04:44 PM, Michael S. Tsirkin wrote: > On Sun, Oct 11, 2009 at 04:42:27PM +0200, Avi Kivity wrote: > >> On 10/11/2009 04:35 PM, Michael S. Tsirkin wrote: >> >>> No, this is not how it works, I think. >>> Currently BIOS ROM is loaded only on init, directly into qemu memory. >>> >>> >> If we change it we break in-place upgrades - install qemu, run guest, >> upgrade qemu, reboot guest - now the old qemu runs with a new bios which >> may or may not work. >> > Sorry, could not parse this. > If you upgraded qemu, you are running with new qemu with new bios? > The guest is running during the upgrade, so the qemu executable is still from the old install (but deleted).
On Sun, Oct 11, 2009 at 04:45:46PM +0200, Michael S. Tsirkin wrote: > > AFAIR it is not writable on plain qemu. > > What makes it non-writeable? > It goes: > bios_offset = qemu_ram_alloc(bios_size); > ret = load_image(filename, qemu_get_ram_ptr(bios_offset)); > Does qemu_ram_alloc allocate read-only memory then? > Look closer. -- Gleb.
On Sun, Oct 11, 2009 at 04:53:39PM +0200, Avi Kivity wrote: > On 10/11/2009 04:44 PM, Michael S. Tsirkin wrote: >> On Sun, Oct 11, 2009 at 04:42:27PM +0200, Avi Kivity wrote: >> >>> On 10/11/2009 04:35 PM, Michael S. Tsirkin wrote: >>> >>>> No, this is not how it works, I think. >>>> Currently BIOS ROM is loaded only on init, directly into qemu memory. >>>> >>>> >>> If we change it we break in-place upgrades - install qemu, run guest, >>> upgrade qemu, reboot guest - now the old qemu runs with a new bios which >>> may or may not work. >>> >> Sorry, could not parse this. >> If you upgraded qemu, you are running with new qemu with new bios? >> > > The guest is running during the upgrade, so the qemu executable is still > from the old install (but deleted). Oh, I don't think we should re-read it from file each time. We could read it at startup, keep a copy around for reboots. > -- > error compiling committee.c: too many arguments to function
On 10/11/2009 06:12 PM, Michael S. Tsirkin wrote: > >> The guest is running during the upgrade, so the qemu executable is still >> from the old install (but deleted). >> > Oh, I don't think we should re-read it from file each time. > We could read it at startup, keep a copy around for reboots. > We do that, see hw/loader.c.
On Sun, Oct 11, 2009 at 06:24:30PM +0200, Avi Kivity wrote: > On 10/11/2009 06:12 PM, Michael S. Tsirkin wrote: >> >>> The guest is running during the upgrade, so the qemu executable is still >>> from the old install (but deleted). >>> >> Oh, I don't think we should re-read it from file each time. >> We could read it at startup, keep a copy around for reboots. >> > > We do that, see hw/loader.c. Which function specifically? > -- > error compiling committee.c: too many arguments to function
On 10/11/2009 06:33 PM, Michael S. Tsirkin wrote: > On Sun, Oct 11, 2009 at 06:24:30PM +0200, Avi Kivity wrote: > >> On 10/11/2009 06:12 PM, Michael S. Tsirkin wrote: >> >>> >>>> The guest is running during the upgrade, so the qemu executable is still >>>> from the old install (but deleted). >>>> >>>> >>> Oh, I don't think we should re-read it from file each time. >>> We could read it at startup, keep a copy around for reboots. >>> >>> >> We do that, see hw/loader.c. >> > Which function specifically? > rom_add_file().
On Sun, Oct 11, 2009 at 06:37:17PM +0200, Avi Kivity wrote: > On 10/11/2009 06:33 PM, Michael S. Tsirkin wrote: >> On Sun, Oct 11, 2009 at 06:24:30PM +0200, Avi Kivity wrote: >> >>> On 10/11/2009 06:12 PM, Michael S. Tsirkin wrote: >>> >>>> >>>>> The guest is running during the upgrade, so the qemu executable is still >>>>> from the old install (but deleted). >>>>> >>>>> >>>> Oh, I don't think we should re-read it from file each time. >>>> We could read it at startup, keep a copy around for reboots. >>>> >>>> >>> We do that, see hw/loader.c. >>> >> Which function specifically? >> > > rom_add_file(). Is it used for BIOS? pc.c seems to call this for initrd only? > -- > error compiling committee.c: too many arguments to function
On 10/11/2009 06:39 PM, Michael S. Tsirkin wrote: >> rom_add_file(). >> > Is it used for BIOS? pc.c seems to call this for initrd only? > It's used indirectly for everything.
On Sun, Oct 11, 2009 at 06:48:34PM +0200, Avi Kivity wrote: > On 10/11/2009 06:39 PM, Michael S. Tsirkin wrote: >>> rom_add_file(). >>> >> Is it used for BIOS? pc.c seems to call this for initrd only? >> > > It's used indirectly for everything. So we are fine then? After reset new BIOS will get used? > -- > error compiling committee.c: too many arguments to function
On 10/11/2009 07:18 PM, Michael S. Tsirkin wrote: > On Sun, Oct 11, 2009 at 06:48:34PM +0200, Avi Kivity wrote: > >> On 10/11/2009 06:39 PM, Michael S. Tsirkin wrote: >> >>>> rom_add_file(). >>>> >>>> >>> Is it used for BIOS? pc.c seems to call this for initrd only? >>> >>> >> It's used indirectly for everything. >> > So we are fine then? After reset new BIOS will get used? > Yes.
On 10/11/2009 07:27 PM, Avi Kivity wrote: > So we are fine then? After reset new BIOS will get used? > > Yes. > I meant, we are fine for upgrade, and no, the new BIOS will not be used. The file is only read once.
On Sun, Oct 11, 2009 at 07:28:13PM +0200, Avi Kivity wrote: > On 10/11/2009 07:27 PM, Avi Kivity wrote: >> So we are fine then? After reset new BIOS will get used? >> >> Yes. >> > > I meant, we are fine for upgrade, and no, the new BIOS will not be used. > The file is only read once. So this is what happens: - after migration from old qemu to new old BIOS is used until reset - after reset new BIOS is used - after upgrade from old version of qemu upgrade does not have any effect until qemu is restarted If this is a fair summary, the only issue with fixing BIOS bug is migration from new qemu to old qemu, and we do not care much. > -- > Do not meddle in the internals of kernels, for they are subtle and quick to panic.
On 10/11/2009 08:10 PM, Michael S. Tsirkin wrote: > > So this is what happens: > - after migration from old qemu to new old BIOS is used until reset > - after reset new BIOS is used > - after upgrade from old version of qemu upgrade does not have any > effect until qemu is restarted > > If this is a fair summary, the only issue with fixing BIOS bug > is migration from new qemu to old qemu, and we do not care much. > Yes.
Gleb Natapov wrote >> BTW, I don't think it's write-protected and it probably should be? >> >> > AFAIR it is not writable on plain qemu. > KVM wants it to be writable for the TPR optimization. Historically, it was read-only in QEMU but it changed to read-write in order to fake coreboot into thinking that the bios implemented PMM which it doesn't.
>>>> We should fix qemu to re-read bios from flash. >>>> Makes sense? >>>> >>> We have option_rom_setup_reset() already. >> >> We don't, anymore :) > We are fast! Well, we surely have something similar instead. Sure. Try 'info roms'. Everything listed there will be reloaded on reset (unless mem=rom, in which case the guest should not be able to modify the bits in the first place). That includes vgabios, optionroms, -kernel images, ... cheers, Gerd
On Mon, Oct 12, 2009 at 03:45:47PM +0200, Gerd Hoffmann wrote: > >>>>We should fix qemu to re-read bios from flash. > >>>>Makes sense? > >>>> > >>>We have option_rom_setup_reset() already. > >> > >>We don't, anymore :) > >We are fast! Well, we surely have something similar instead. > > Sure. Try 'info roms'. Everything listed there will be reloaded on > reset (unless mem=rom, in which case the guest should not be able to > modify the bits in the first place). That includes vgabios, > optionroms, -kernel images, ... > And what about bios? I don't see it there. -- Gleb.
On Mon, Oct 12, 2009 at 08:44:56AM -0500, Anthony Liguori wrote: > Gleb Natapov wrote > >>BTW, I don't think it's write-protected and it probably should be? > >> > >AFAIR it is not writable on plain qemu. > > KVM wants it to be writable for the TPR optimization. Historically, I don't think so. TPR will work after BIOS will be shadowed. This is simply KVM shortcoming. > it was read-only in QEMU but it changed to read-write in order to I just checked. It is still read-only in QEMU _before_ BIOS is shadowed. > fake coreboot into thinking that the bios implemented PMM which it > doesn't. What is PMM? Post memory manager? How is it related? -- Gleb.
Gleb Natapov wrote: > On Mon, Oct 12, 2009 at 08:44:56AM -0500, Anthony Liguori wrote: > >> Gleb Natapov wrote >> >>>> BTW, I don't think it's write-protected and it probably should be? >>>> >>>> >>> AFAIR it is not writable on plain qemu. >>> >> KVM wants it to be writable for the TPR optimization. Historically, >> > I don't think so. TPR will work after BIOS will be shadowed. This is simply > KVM shortcoming. > > >> it was read-only in QEMU but it changed to read-write in order to >> > I just checked. It is still read-only in QEMU _before_ BIOS is shadowed. > > >> fake coreboot into thinking that the bios implemented PMM which it >> doesn't. >> > What is PMM? Post memory manager? How is it related? > I was thinking of option rom memory. BIOS memory is still read-only. Regards, Anthony Liguori
On 10/12/09 15:53, Gleb Natapov wrote: > On Mon, Oct 12, 2009 at 03:45:47PM +0200, Gerd Hoffmann wrote: >>>>>> We should fix qemu to re-read bios from flash. >>>>>> Makes sense? >>>>>> >>>>> We have option_rom_setup_reset() already. >>>> >>>> We don't, anymore :) >>> We are fast! Well, we surely have something similar instead. >> >> Sure. Try 'info roms'. Everything listed there will be reloaded on >> reset (unless mem=rom, in which case the guest should not be able to >> modify the bits in the first place). That includes vgabios, >> optionroms, -kernel images, ... >> > And what about bios? I don't see it there. pc.c does some magic here to map the complete bios into high memory and the legacy part at 0xe0000, thats why it is loaded in a different way. It gets loaded to IO_MEM_ROM memory though, so the guest should not be able to corrupt it. cheers, Gerd
On Mon, Oct 12, 2009 at 04:50:35PM +0200, Gerd Hoffmann wrote: > On 10/12/09 15:53, Gleb Natapov wrote: > >On Mon, Oct 12, 2009 at 03:45:47PM +0200, Gerd Hoffmann wrote: > >>>>>>We should fix qemu to re-read bios from flash. > >>>>>>Makes sense? > >>>>>> > >>>>>We have option_rom_setup_reset() already. > >>>> > >>>>We don't, anymore :) > >>>We are fast! Well, we surely have something similar instead. > >> > >>Sure. Try 'info roms'. Everything listed there will be reloaded on > >>reset (unless mem=rom, in which case the guest should not be able to > >>modify the bits in the first place). That includes vgabios, > >>optionroms, -kernel images, ... > >> > >And what about bios? I don't see it there. > > pc.c does some magic here to map the complete bios into high memory > and the legacy part at 0xe0000, thats why it is loaded in a > different way. It gets loaded to IO_MEM_ROM memory though, so the > guest should not be able to corrupt it. > But KVM doesn't support it (memory is always writable). And we want to reload BIOS on reboot for migration purposes. -- Gleb.
On Mon, Oct 12, 2009 at 08:44:56AM -0500, Anthony Liguori wrote: > Gleb Natapov wrote >>> BTW, I don't think it's write-protected and it probably should be? >>> >>> >> AFAIR it is not writable on plain qemu. >> > > KVM wants it to be writable for the TPR optimization. Historically, it > was read-only in QEMU but it changed to read-write in order to fake > coreboot into thinking that the bios implemented PMM which it doesn't. During init, SeaBIOS will enable writes to all memory between 0xc0000-0x100000 - shadowing the ram as needed. Just prior to starting the boot phase, SeaBIOS will make the bios f-segment and all option roms read only. (Note, this differs from bochs-bios which doesn't make optionroms writable - seabios makes them writable because some optionroms need it.) As an aside, it would be nice if qemu would just start off with this memory as read/writable, as the shadoing process is ugly, it's qemu specific, and it always ends up as read/writable anyway. I'm not sure where coreboot or PMM comes into this. SeaBIOS does implement the Post Memory Manager calls. -Kevin
Gleb Natapov wrote:
> But KVM doesn't support it (memory is always writable).
That seems like something which could be fixed.
-- Jamie
On 10/12/09 16:52, Gleb Natapov wrote: > On Mon, Oct 12, 2009 at 04:50:35PM +0200, Gerd Hoffmann wrote: >> pc.c does some magic here to map the complete bios into high memory >> and the legacy part at 0xe0000, thats why it is loaded in a >> different way. It gets loaded to IO_MEM_ROM memory though, so the >> guest should not be able to corrupt it. >> > But KVM doesn't support it (memory is always writable). And we want to reload > BIOS on reboot for migration purposes. Then it should be switched over to the new rom loader. Also the logic which does *not* reload stuff which goes to IO_MEM_ROM must be turned off for kvm them. cheers, Gerd
On Mon, Oct 12, 2009 at 05:43:39PM +0200, Gerd Hoffmann wrote: > On 10/12/09 16:52, Gleb Natapov wrote: >> On Mon, Oct 12, 2009 at 04:50:35PM +0200, Gerd Hoffmann wrote: >>> pc.c does some magic here to map the complete bios into high memory >>> and the legacy part at 0xe0000, thats why it is loaded in a >>> different way. It gets loaded to IO_MEM_ROM memory though, so the >>> guest should not be able to corrupt it. >>> >> But KVM doesn't support it (memory is always writable). And we want to reload >> BIOS on reboot for migration purposes. > > Then it should be switched over to the new rom loader. Also the logic > which does *not* reload stuff which goes to IO_MEM_ROM must be turned > off for kvm them. I think it needs to be turned off anyway. Since ROM is sent over for migration, it might get out of sync with file even if it is read-only. > cheers, > Gerd
diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c index 1feb827..c54ac65 100644 --- a/hw/cirrus_vga.c +++ b/hw/cirrus_vga.c @@ -3248,7 +3248,6 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev) /* setup PCI */ pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_CIRRUS); pci_config_set_device_id(pci_conf, device_id); - pci_conf[0x04] = PCI_COMMAND_IOACCESS | PCI_COMMAND_MEMACCESS; pci_config_set_class(pci_conf, PCI_CLASS_DISPLAY_VGA); pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL;
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> --- hw/cirrus_vga.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)