pc-bios: enable io/memory unconditionally
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/3 pc-bios: enable io/memory unconditionally
VGA adapters need to claim memory and i/o
transactions even if they do not have any
i/o or memory bars. E.g. PCI spec, page 297,
gives an example of such a device:
Programming interface 0000 0000b
VGA-compatible controller. Memory
addresses 0A 0000h through 0B
FFFFh. I/O addresses 3B0h to 3BBh
and 3C0h to 3DFh and all aliases of
these addresses.
While we could check for these devices and special-case them, it is easier to fix this by enabling i/o and memory space unconditionally:
devices that do not support it will just ignore this setting.
--- kvm/bios/rombios32.c | 18 +++++++----------- 1 files changed, 7 insertions(+), 11 deletions(-)
kvm/bios/rombios32.c +7 -11
@@ -700,6 +700,7 @@ void smp_probe(void)#define PCI_COMMAND 0x04/* 16 bits */ #define PCI_COMMAND_IO 0x1/* Enable response in I/O space */ #define PCI_COMMAND_MEMORY 0x2/* Enable response in Memory space */ +#define PCI_CLASS_PROG 0x09/* Reg. Level Programming Interface */ #define PCI_CLASS_DEVICE 0x0a/* Device class */ #define PCI_INTERRUPT_LINE 0x3c/* 8 bits */ #define PCI_INTERRUPT_PIN 0x3d/* 8 bits */
@@ -767,7 +768,6 @@ static uint32_t pci_config_readb(PCIDevice *d, uint32_t addr)static void pci_set_io_region_addr (PCIDevice *d,int region_num,uint32_t addr) { -uint16_t cmd;uint32_t ofs, old_addr;if ( region_num ==PCI_ROM_SLOT ) {
@@ -780,16 +780,6 @@ static void pci_set_io_region_addr(PCIDevice *d, int region_num, uint32_t addr)pci_config_writel (d, ofs, addr);BX_INFO ("region %d: 0x%08x\n" , region_num, addr); - -/* enable memory mappings */ - cmd =pci_config_readw (d,PCI_COMMAND ); -if ( region_num ==PCI_ROM_SLOT ) - cmd |=2 ; -else if (old_addr &PCI_ADDRESS_SPACE_IO ) - cmd |=1 ; -else - cmd |=2 ; -pci_config_writew (d,PCI_COMMAND , cmd); }/* return the global irq number corresponding to a given device irq
@@ -946,6 +936,7 @@ static void pci_bios_init_device(PCIDevice *d) {int class;uint32_t *paddr; +uint16_t cmd;int i, pin, pic_irq, vendor_id, device_id; class =pci_config_readw (d,PCI_CLASS_DEVICE );
@@ -1024,6 +1015,11 @@ static void pci_bios_init_device(PCIDevice *d)break ; } +/* enable memory mappings */ + cmd =pci_config_readw (d,PCI_COMMAND ); + cmd |=PCI_COMMAND_IO |PCI_COMMAND_MEMORY ; +pci_config_writew (d,PCI_COMMAND , cmd); +/* map the interrupt */ pin =pci_config_readb (d,PCI_INTERRUPT_PIN );if (pin !=0 ) {
Signature · 1 line
1.6.5.rc2
PATCH 2/3 qemu: make cirrus init value pci spec compliant
In reply to a message not in the archivePCI 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.
--- hw/cirrus_vga.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
hw/cirrus_vga.c +0 -1
@@ -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 ;
Signature · 1 line
1.6.5.rc2
Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 20 lines · Michael S. Tsirkin
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<[email protected]>--- hw/cirrus_vga.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)hw/cirrus_vga.c +0
-0
hw/cirrus_vga.c static int pci_cirrus_vga_initfn(PCIDevice *dev) · line 3253
Quoted 7 lines
hw/cirrus_vga.c +0
-1@@ -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).
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 25 lines · Avi Kivity
Quoted 24 lines · Michael S. Tsirkin
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<[email protected]>--- hw/cirrus_vga.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)hw/cirrus_vga.c +0
-1@@ -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 ;
Quoted 8 lines
hw/cirrus_vga.c static int pci_cirrus_vga_initfn(PCIDevice *dev) · line 3253
Quoted 3 lines
hw/cirrus_vga.c +0
-0pci_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?
Quoted 2 lines
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 37 lines · Michael S. Tsirkin
Quoted 26 lines · Avi Kivity
Quoted 24 lines · Michael S. Tsirkin
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<[email protected]>--- hw/cirrus_vga.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)hw/cirrus_vga.c +0
-1@@ -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 ;Quoted 9 lines
hw/cirrus_vga.c static int pci_cirrus_vga_initfn(PCIDevice *dev) · line 3253
Quoted 4 lines
hw/cirrus_vga.c +0
-0pci_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).
Quoted 3 lines
No, if you migrate from older qemu you will be fine as command is enabled there on init.
Right.
Quoted 6 lines
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.
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 22 lines · Avi Kivity
Quoted 21 lines · Michael S. Tsirkin
Quoted 19 lines · Avi Kivity
Quoted 11 lines · Michael S. Tsirkin
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<[email protected]>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).
Quoted 5 lines
Quoted 3 lines
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
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 27 lines · Jamie Lokier
Quoted 26 lines · Avi Kivity
Quoted 24 lines · Michael S. Tsirkin
Quoted 12 lines · Avi Kivity
Quoted 10 lines · Michael S. Tsirkin
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.Quoted 7 lines
Quoted 1 line
Signed-off-by Michael S. Tsirkin<[email protected]>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.
Quoted 7 lines
Quoted 1 line
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.
Quoted 1 line
-- Jamie
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
In reply to Jamie LokierQuoted 29 lines · Jamie Lokier
Quoted 22 lines · Avi Kivity
Quoted 21 lines · Michael S. Tsirkin
Quoted 19 lines · Avi Kivity
Quoted 11 lines · Michael S. Tsirkin
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<[email protected]>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).
Quoted 5 lines
Quoted 3 lines
No, if you migrate from older qemu you will be fine as command is enabled there on init.
Right.
Quoted 7 lines
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?
Signature · 1 line
MST
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 36 lines · Michael S. Tsirkin
Quoted 35 lines · Jamie Lokier
Quoted 22 lines · Avi Kivity
Quoted 21 lines · Michael S. Tsirkin
Quoted 19 lines · Avi Kivity
Quoted 11 lines · Michael S. Tsirkin
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<[email protected]>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).
Quoted 5 lines
Quoted 3 lines
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.
Quoted 8 lines
Quoted 1 line
-- 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.
Quoted 3 lines
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?
Quoted 15 lines
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?Signature · 6 lines
MST -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Gleb.
Re: Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 39 lines · Gleb Natapov
Quoted 38 lines · Michael S. Tsirkin
Quoted 36 lines · Jamie Lokier
Quoted 22 lines · Avi Kivity
Quoted 21 lines · Michael S. Tsirkin
Quoted 19 lines · Avi Kivity
Quoted 11 lines · Michael S. Tsirkin
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<[email protected]>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).
Quoted 5 lines
Quoted 3 lines
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
Quoted 7 lines
Quoted 6 lines
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 :)
Quoted 2 lines
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?
Quoted 4 lines
Quoted 3 lines
We should fix qemu to re-read bios from flash.
Makes sense?We have
option_rom_setup_reset()already.
We don't, anymore :)
Quoted 21 lines
Don't we use it for BIOSes
too?Quoted 15 lines
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?Signature · 6 lines
MST -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html--
Gleb.
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 46 lines · Michael S. Tsirkin
Quoted 45 lines · Gleb Natapov
Quoted 44 lines · Michael S. Tsirkin
Quoted 29 lines · Jamie Lokier
Quoted 22 lines · Avi Kivity
Quoted 21 lines · Michael S. Tsirkin
Quoted 19 lines · Avi Kivity
Quoted 11 lines · Michael S. Tsirkin
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<[email protected]>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).
Quoted 5 lines
Quoted 3 lines
No, if you migrate from older qemu you will be fine as command is enabled there on init.
Right.
Quoted 7 lines
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.
Quoted 4 lines
Quoted 1 line
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.
Quoted 5 lines
Quoted 2 lines
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.
Quoted 4 lines
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.
Quoted 7 lines
Quoted 5 lines
Quoted 4 lines
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.
Quoted 15 lines
Quoted 15 lines
Don't we use it for BIOSes
too?Quoted 11 lines
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?Signature · 1 line
MST
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines
--
To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html--
Gleb.
--
Gleb.
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 56 lines · Gleb Natapov
Quoted 46 lines · Michael S. Tsirkin
Quoted 45 lines · Gleb Natapov
Quoted 44 lines · Michael S. Tsirkin
Quoted 29 lines · Jamie Lokier
Quoted 22 lines · Avi Kivity
Quoted 21 lines · Michael S. Tsirkin
Quoted 19 lines · Avi Kivity
Quoted 11 lines · Michael S. Tsirkin
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<[email protected]>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).
Quoted 5 lines
Quoted 3 lines
No, if you migrate from older qemu you will be fine as command is enabled there on init.
Right.
Quoted 7 lines
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.
Quoted 4 lines
Quoted 1 line
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.
Quoted 3 lines
Quoted 2 lines
If BIOS is not shadowed it runs directly from ROM. Hard to use self-modifying code there.
Quoted 6 lines
Quoted 2 lines
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.
Quoted 6 lines
Quoted 4 lines
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?
Quoted 34 lines
Quoted 7 lines
Quoted 5 lines
Quoted 4 lines
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.
Quoted 15 lines
Quoted 15 lines
Don't we use it for BIOSes
too?Quoted 11 lines
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?Signature · 1 line
MSTQuoted 7 lines
Quoted 7 lines
Quoted 4 lines
--
To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html--
Gleb.--
Gleb.
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 59 lines · Michael S. Tsirkin
Quoted 58 lines · Gleb Natapov
Quoted 46 lines · Michael S. Tsirkin
Quoted 45 lines · Gleb Natapov
Quoted 44 lines · Michael S. Tsirkin
Quoted 29 lines · Jamie Lokier
Quoted 22 lines · Avi Kivity
Quoted 21 lines · Michael S. Tsirkin
Quoted 19 lines · Avi Kivity
Quoted 11 lines · Michael S. Tsirkin
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<[email protected]>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).
Quoted 5 lines
Quoted 3 lines
No, if you migrate from older qemu you will be fine as command is enabled there on init.
Right.
Quoted 7 lines
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.
Quoted 4 lines
Quoted 1 line
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.
Quoted 5 lines
Quoted 2 lines
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?
Quoted 9 lines
Quoted 4 lines
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.
Quoted 10 lines
Quoted 6 lines
Quoted 4 lines
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.
Quoted 29 lines
Quoted 29 lines
Quoted 7 lines
Quoted 5 lines
Quoted 4 lines
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.
Quoted 20 lines
Quoted 20 lines
Don't we use it for BIOSes
too?Quoted 15 lines
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?Signature · 6 lines
MST -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 5 lines
Quoted 5 lines
Quoted 2 lines
Quoted 2 lines
--
Gleb.--
Gleb.
--
Gleb.
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
In reply to Gleb NatapovQuoted 7 lines
Quoted 6 lines
Quoted 4 lines
Quoted 3 lines
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
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 5 lines · Gerd Hoffmann
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
Quoted 3 lines
We should fix qemu to re-read bios from flash.
Makes sense?We have
option_rom_setup_reset()already.
Quoted 8 lines
Quoted 2 lines
Quoted 1 line
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.
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 7 lines · Gleb Natapov
Quoted 6 lines · Gerd Hoffmann
Quoted 6 lines
Quoted 6 lines
Quoted 4 lines
Quoted 3 lines
We should fix qemu to re-read bios from flash.
Makes sense?We have
option_rom_setup_reset()already.We don't, anymore :)
Quoted 8 lines
Quoted 7 lines
Quoted 1 line
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
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 15 lines · Gerd Hoffmann
Quoted 14 lines · Gleb Natapov
Quoted 13 lines · Gerd Hoffmann
Quoted 7 lines
Quoted 6 lines
Quoted 4 lines
Quoted 3 lines
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, ...
Quoted 7 lines
Quoted 1 line
And what about bios? I don't see it there.
pc.cdoes some magic here to map the complete bios into high memory and the legacy part at0xe0000, thats why it is loaded in a different way. It gets loaded toIO_MEM_ROMmemory 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.
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 1 line · Gleb Natapov
But KVM doesn't support it (memory is always writable).
That seems like something which could be fixed.
-- Jamie
Re: Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
In reply to Gleb NatapovQuoted 8 lines · Gleb Natapov
Quoted 5 lines · Gerd Hoffmann
pc.cdoes some magic here to map the complete bios into high memory and the legacy part at0xe0000, thats why it is loaded in a different way. It gets loaded toIO_MEM_ROMmemory 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
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 7 lines · Gerd Hoffmann
Quoted 6 lines · Gleb Natapov
Quoted 5 lines · Gerd Hoffmann
pc.cdoes some magic here to map the complete bios into high memory and the legacy part at0xe0000, thats why it is loaded in a different way. It gets loaded toIO_MEM_ROMmemory though, so the guest should not be able to corrupt it.
Quoted 6 lines
Quoted 2 lines
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_ROMmust 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.
Quoted 2 lines
cheers,
Gerd
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
In reply to Michael S. TsirkinQuoted 3 lines · Michael S. Tsirkin
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
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 8 lines · Jamie Lokier
Quoted 3 lines · Michael S. Tsirkin
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.
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
In reply to Jamie LokierQuoted 29 lines · Jamie Lokier
Quoted 22 lines · Avi Kivity
Quoted 21 lines · Michael S. Tsirkin
Quoted 19 lines · Avi Kivity
Quoted 11 lines · Michael S. Tsirkin
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<[email protected]>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).
Quoted 5 lines
Quoted 3 lines
No, if you migrate from older qemu you will be fine as command is enabled there on init.
Right.
Quoted 7 lines
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?
Signature · 1 line
MST
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 6 lines · Michael S. Tsirkin
Quoted 6 lines
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.
Quoted 7 lines
Quoted 1 line
-- 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.
Re: Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 9 lines · Gleb Natapov
Quoted 8 lines · Michael S. Tsirkin
Quoted 7 lines
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
Quoted 8 lines
Quoted 5 lines
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.
Quoted 3 lines
What really happens today I don't know.
--
Gleb.
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 15 lines · Michael S. Tsirkin
Quoted 14 lines · Gleb Natapov
Quoted 13 lines · Michael S. Tsirkin
Quoted 7 lines
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?
Quoted 6 lines
Quoted 3 lines
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.
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 20 lines · Gleb Natapov
Quoted 19 lines · Michael S. Tsirkin
Quoted 17 lines · Gleb Natapov
Quoted 6 lines · Michael S. Tsirkin
Quoted 6 lines
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.
Quoted 7 lines
Quoted 1 line
-- 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.
Quoted 5 lines
Quoted 2 lines
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?
Quoted 3 lines
--
Gleb.
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 21 lines · Michael S. Tsirkin
Quoted 20 lines · Gleb Natapov
Quoted 19 lines · Michael S. Tsirkin
Quoted 17 lines · Gleb Natapov
Quoted 6 lines · Michael S. Tsirkin
Quoted 6 lines
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.
Quoted 7 lines
Quoted 1 line
-- 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.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
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?
Quoted 2 lines
BTW, I don't think it's write-protected and it probably should be?
AFAIR it is not writable on plain qemu.
--
Gleb.
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 28 lines · Gleb Natapov
Quoted 27 lines · Michael S. Tsirkin
Quoted 25 lines · Gleb Natapov
Quoted 15 lines · Michael S. Tsirkin
Quoted 14 lines · Gleb Natapov
Quoted 13 lines · Michael S. Tsirkin
Quoted 7 lines
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?
Quoted 6 lines
Quoted 3 lines
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.
Quoted 7 lines
Quoted 2 lines
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?
Quoted 2 lines
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?
Quoted 2 lines
--
Gleb.
Re: Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 8 lines · Michael S. Tsirkin
Quoted 1 line
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_allocallocate read-only memory then?
Look closer.
--
Gleb.
Re: Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
In reply to Gleb NatapovGleb Natapov wrote
Quoted 5 lines
Quoted 3 lines
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.
Signature · 3 lines
Regards, Anthony Liguori
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 6 lines · Anthony Liguori
Gleb Natapov wrote
Quoted 3 lines
Quoted 2 lines
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.
Quoted 1 line
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.
Quoted 2 lines
fake coreboot into thinking that the bios implemented PMM which it doesn't.
What is PMM? Post memory manager? How is it related?
--
Gleb.
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 17 lines · Gleb Natapov
Quoted 9 lines · Anthony Liguori
Gleb Natapov wrote
Quoted 5 lines
Quoted 3 lines
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.
Quoted 2 lines
it was read-only in QEMU but it changed to read-write in order to
Quoted 8 lines
I just checked. It is still read-only in QEMU _before_ BIOS is shadowed.
Quoted 3 lines
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
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
In reply to Anthony LiguoriQuoted 10 lines · Anthony Liguori
Gleb Natapov wrote
Quoted 5 lines
Quoted 3 lines
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
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
In reply to Michael S. TsirkinQuoted 3 lines · Michael S. Tsirkin
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.
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 8 lines · Avi Kivity
Quoted 3 lines · Michael S. Tsirkin
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?
Quoted 2 lines
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 8 lines · Michael S. Tsirkin
Quoted 6 lines · Avi Kivity
Quoted 4 lines · Michael S. Tsirkin
No, this is not how it works, I think.
Currently BIOS ROM is loaded only on init, directly into qemu memory.
Quoted 7 lines
Quoted 4 lines
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).
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 13 lines · Avi Kivity
Quoted 12 lines · Michael S. Tsirkin
Quoted 10 lines · Avi Kivity
Quoted 4 lines · Michael S. Tsirkin
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.
Quoted 6 lines
Quoted 3 lines
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.
Quoted 2 lines
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 7 lines · Michael S. Tsirkin
Quoted 4 lines
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.
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 10 lines · Avi Kivity
Quoted 7 lines · Michael S. Tsirkin
Quoted 4 lines
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?
Quoted 2 lines
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 9 lines · Michael S. Tsirkin
Quoted 7 lines · Avi Kivity
Quoted 5 lines · Michael S. Tsirkin
Quoted 5 lines
The guest is running during the upgrade, so the qemu executable is still from the old install (but deleted).
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
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().
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 14 lines · Avi Kivity
Quoted 13 lines · Michael S. Tsirkin
Quoted 11 lines · Avi Kivity
Quoted 9 lines · Michael S. Tsirkin
Quoted 5 lines
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.
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
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?
Quoted 2 lines
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 4 lines · Michael S. Tsirkin
Quoted 2 lines
rom_add_file().Is it used for BIOS?
pc.cseems to call this for initrd only?
It's used indirectly for everything.
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 7 lines · Avi Kivity
Quoted 4 lines · Michael S. Tsirkin
Quoted 2 lines
rom_add_file().Is it used for BIOS?
pc.cseems to call this for initrd only?It's used indirectly for everything.
So we are fine then? After reset new BIOS will get used?
Quoted 2 lines
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 7 lines · Michael S. Tsirkin
Quoted 5 lines · Avi Kivity
Quoted 3 lines · Michael S. Tsirkin
Quoted 3 lines
rom_add_file().
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Is it used for BIOS?
pc.cseems 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.
Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 4 lines · Avi Kivity
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.
Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 8 lines · Avi Kivity
Quoted 4 lines · Avi Kivity
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.
Quoted 2 lines
Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
Re: [Qemu-devel] Re: [PATCH 2/3] qemu: make cirrus init value pci spec compliant
Quoted 10 lines · Michael S. Tsirkin
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 restartedIf 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.
Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
PATCH 3/3 qemu: cleanup unused macros in cirrus
In reply to a message not in the archiveCirrus vga has a copy of many PCI macros,
and it doesn't even use them. Clean up.
--- hw/cirrus_vga.c | 37 +------------------------------------ 1 files changed, 1 insertions(+), 36 deletions(-)
hw/cirrus_vga.c +1 -36
@@ -172,41 +172,6 @@#define CIRRUS_MMIO_LINEDRAW_MODE 0x39 // byte#define CIRRUS_MMIO_BLTSTATUS 0x40 // byte -// PCI 0x04: command(word), 0x06(word): status -#define PCI_COMMAND_IOACCESS 0x0001 -#define PCI_COMMAND_MEMACCESS 0x0002 -#define PCI_COMMAND_BUSMASTER 0x0004 -#define PCI_COMMAND_SPECIALCYCLE 0x0008 -#define PCI_COMMAND_MEMWRITEINVALID 0x0010 -#define PCI_COMMAND_PALETTESNOOPING 0x0020 -#define PCI_COMMAND_PARITYDETECTION 0x0040 -#define PCI_COMMAND_ADDRESSDATASTEPPING 0x0080 -#define PCI_COMMAND_SERR 0x0100 -#define PCI_COMMAND_BACKTOBACKTRANS 0x0200 -// PCI 0x08, 0xff000000 (0x09-0x0b:class,0x08:rev) -#define PCI_CLASS_BASE_DISPLAY 0x03 -// PCI 0x08, 0x00ff0000 -#define PCI_CLASS_SUB_VGA 0x00 -// PCI 0x0c, 0x00ff0000 (0x0c:cacheline,0x0d:latency,0x0e:headertype,0x0f:Built-in self test) -// 0x10-0x3f (headertype 00h) -// PCI 0x10,0x14,0x18,0x1c,0x20,0x24: base address mapping registers -// 0x10: MEMBASE, 0x14: IOBASE(hard-coded in XFree86 3.x) -#define PCI_MAP_MEM 0x0 -#define PCI_MAP_IO 0x1 -#define PCI_MAP_MEM_ADDR_MASK (~0xf) -#define PCI_MAP_IO_ADDR_MASK (~0x3) -#define PCI_MAP_MEMFLAGS_32BIT 0x0 -#define PCI_MAP_MEMFLAGS_32BIT_1M 0x1 -#define PCI_MAP_MEMFLAGS_64BIT 0x4 -#define PCI_MAP_MEMFLAGS_CACHEABLE 0x8 -// PCI 0x28: cardbus CIS pointer -// PCI 0x2c: subsystem vendor id, 0x2e: subsystem id -// PCI 0x30: expansion ROM base address -#define PCI_ROMBIOS_ENABLED 0x1 -// PCI 0x34: 0xffffff00=reserved, 0x000000ff=capabilities pointer -// PCI 0x38: reserved -// PCI 0x3c: 0x3c=int-line, 0x3d=int-pin, 0x3e=min-gnt, 0x3f=maax-lat -#define CIRRUS_PNPMMIO_SIZE 0x1000#define ABS (a) ((signed)(a) > 0 ? a : -a)
@@ -3248,8 +3213,8 @@ 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 ;/* setup memory space */ /* memory #0 LFB */
Signature · 1 line
1.6.5.rc2
Re: [PATCH 3/3] qemu: cleanup unused macros in cirrus
hw/cirrus_vga.c static int pci_cirrus_vga_initfn(PCIDevice *dev) · line 3216
Quoted 7 lines · Michael S. Tsirkin
Cirrus vga has a copy of many PCI macros,
and it doesn't even use them. Clean up.hw/cirrus_vga.c +1
-0@@ -3248,8 +3213,8 @@ 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;
If it is wrong as you claim in the other patch, just remove it, or at least put a comment.
hw/cirrus_vga.c static int pci_cirrus_vga_initfn(PCIDevice *dev) · line 3252
Quoted 2 lines
hw/cirrus_vga.c +0
-1pci_config_set_class (pci_conf,PCI_CLASS_DISPLAY_VGA ); - pci_conf[PCI_HEADER_TYPE ] =PCI_HEADER_TYPE_NORMAL ;
Did yo;u wanted to remove this one also?
hw/cirrus_vga.c static int pci_cirrus_vga_initfn(PCIDevice *dev) · line 3220
Quoted 3 lines
hw/cirrus_vga.c +0
-0/* setup memory space */ /* memory #0 LFB */
Nice cleanup for the rest :)
Later, Juan.
Re: [PATCH 3/3] qemu: cleanup unused macros in cirrus
Quoted 11 lines · Juan Quintela
hw/cirrus_vga.c static int pci_cirrus_vga_initfn(PCIDevice *dev) · line 3216
Quoted 7 lines · Michael S. Tsirkin
Cirrus vga has a copy of many PCI macros,
and it doesn't even use them. Clean up.hw/cirrus_vga.c +1
-0@@ -3248,8 +3213,8 @@ 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; If it is wrong as you claim in the other patch, just remove it, or at least put a comment.
Of course. sorry.
Quoted 4 lines
hw/cirrus_vga.c static int pci_cirrus_vga_initfn(PCIDevice *dev) · line 3252
Quoted 2 lines
hw/cirrus_vga.c +0
-1pci_config_set_class (pci_conf,PCI_CLASS_DISPLAY_VGA ); - pci_conf[PCI_HEADER_TYPE ] =PCI_HEADER_TYPE_NORMAL ;Did yo;u wanted to remove this one also?
Yes, this is the default anyway.
Quoted 7 lines
hw/cirrus_vga.c static int pci_cirrus_vga_initfn(PCIDevice *dev) · line 3220
Quoted 3 lines
hw/cirrus_vga.c +0
-0/* setup memory space */ /* memory #0 LFB */ Nice cleanup for the rest :)
Later, Juan.