Re: [Qemu-devel] [PATCH] kvm-unittest: fix build with gcc 4.3.X and older
On Wed, Oct 16, 2013 at 10:46:53PM +0300, Michael S. Tsirkin wrote: Old GCC didn't let you reference variable by number if it is listed with a specific register constraint, on the assumption you can just use the register name explicitly. Build fails with errors like this: a.c:6: error: invalid 'asm': invalid operand code 'd' Is it worth to support such ancient compiler? Nobody complained till now. BTW with your patch I still cannot compile with 4.2: x86/s3.c: In function 'main': x86/s3.c:145: error: inconsistent operand constraints in an 'asm' To fix, let's just use %eax %al etc. Only %d0 does not work and dropping d fixes it since compiler can figure out correct register from variable size. The patch bellow fixes compilation for 4.2. diff --git a/lib/x86/pci.c b/lib/x86/pci.c index f95cd88..231668a 100644 --- a/lib/x86/pci.c +++ b/lib/x86/pci.c @@ -3,13 +3,13 @@ static void outl(unsigned short port, unsigned val) { -asm volatile(outl %d0, %w1 : : a(val), Nd(port)); +asm volatile(outl %0, %w1 : : a(val), Nd(port)); } static unsigned inl(unsigned short port) { unsigned data; -asm volatile(inl %w1, %d0 : =a(data) : Nd(port)); +asm volatile(inl %w1, %0 : =a(data) : Nd(port)); return data; } static uint32_t pci_config_read(pcidevaddr_t dev, uint8_t reg) diff --git a/x86/s3.c b/x86/s3.c index 71d3ff9..d568aa7 100644 --- a/x86/s3.c +++ b/x86/s3.c @@ -143,14 +143,14 @@ static inline int rtc_in(u8 reg) { u8 x = reg; asm volatile(outb %b1, $0x70; inb $0x71, %b0 -: +a(x) : 0(x)); +: =a(x) : 0(x)); return x; } static inline void rtc_out(u8 reg, u8 val) { asm volatile(outb %b1, $0x70; mov %b2, %b1; outb %b1, $0x71 -: +a(reg) : 0(reg), ri(val)); +: =a(reg) : 0(reg), ri(val)); } extern char resume_start, resume_end; diff --git a/x86/vmexit.c b/x86/vmexit.c index 3b945de..7e9af15 100644 --- a/x86/vmexit.c +++ b/x86/vmexit.c @@ -26,7 +26,7 @@ static void outw(unsigned short port, unsigned val) static void outl(unsigned short port, unsigned val) { -asm volatile(outl %d0, %w1 : : a(val), Nd(port)); +asm volatile(outl %0, %w1 : : a(val), Nd(port)); } static unsigned int inb(unsigned short port) -- Gleb.
Re: [Qemu-devel] [PATCH v7 4/4] PC: differentiate hpet's interrupt capability on piix and q35
On Thu, Oct 17, 2013 at 1:44 PM, Michael S. Tsirkin m...@redhat.com wrote: On Thu, Oct 17, 2013 at 11:16:05AM +0800, Liu Ping Fan wrote: For pc-piix-*, hpet's intcap is always hard coded as IRQ2. For q35, if it is pc-q35-1.7 and earlier, we use IRQ2 for compat reason, otherwise IRQ2, IRQ8, and IRQ16~23 are allowed. Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com --- hw/i386/pc.c | 20 +++- hw/i386/pc_piix.c| 7 ++- hw/i386/pc_q35.c | 6 +- include/hw/i386/pc.h | 11 ++- 4 files changed, 36 insertions(+), 8 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 72cc850..3e98ff0 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1219,7 +1219,8 @@ static const MemoryRegionOps ioportF0_io_ops = { void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, ISADevice **rtc_state, ISADevice **floppy, - bool no_vmport) + bool no_vmport, + bool hpet_irqs) { int i; DriveInfo *fd[MAX_FD]; @@ -1249,10 +1250,19 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, /* In order to set property, here not using sysbus_try_create_simple */ hpet = qdev_try_create(NULL, hpet); if (hpet) { -/* tmp fix. For compat, hard code to IRQ2 until we have correct - * compat property and differentiate pc-iix with pc-q35 - */ -qdev_prop_set_uint32(hpet, HPET_INTCAP, 0x4); +/* For pc-piix-*, hpet's intcap is always IRQ2. */ +if (!hpet_irqs) { So for piix the property is ignored even if set. Yes. +qdev_prop_set_uint32(hpet, HPET_INTCAP, 0x4); +} else { +/* For pc-q35-1.7 and earlier, use IRQ2 for compat. Now you lost me. It's still 4 for 1.7? You only target 1.8 then? Yes, since the compat for guest. And start this fix with 1.8 + * Otherwise, use IRQ16~23, IRQ8 and IRQ2. + */ +uint8_t compat = object_property_get_int(OBJECT(hpet), +HPET_INTCAP, NULL); +if (!compat) { +qdev_prop_set_uint32(hpet, HPET_INTCAP, 0xff0104); +} +} qdev_init_nofail(hpet); sysbus_mmio_map(SYS_BUS_DEVICE(hpet), 0, HPET_BASE); diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index c6042c7..a45ce11 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -180,7 +180,8 @@ static void pc_init1(QEMUMachineInitArgs *args, pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL); /* init basic PC hardware */ -pc_basic_device_init(isa_bus, gsi, rtc_state, floppy, xen_enabled()); +pc_basic_device_init(isa_bus, gsi, rtc_state, floppy, xen_enabled(), +false); pc_nic_init(isa_bus, pci_bus); @@ -346,6 +347,10 @@ static QEMUMachine pc_i440fx_machine_v1_7 = { .alias = pc, .init = pc_init_pci, .is_default = 1, +.compat_props = (GlobalProperty[]) { +PC_COMPAT_1_7, So you add this property for PIIX but it's then ignored? That's kind of ugly. Yes a little ugly, but PIIX and ich9 diverge for hpet's hardware. Not sure how to fix this. Paolo? +{ /* end of list */ } +}, }; #define PC_I440FX_1_6_MACHINE_OPTIONS PC_I440FX_MACHINE_OPTIONS diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index ca84e1c..124ecc1 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -181,7 +181,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args) pc_register_ferr_irq(gsi[13]); /* init basic PC hardware */ -pc_basic_device_init(isa_bus, gsi, rtc_state, floppy, false); +pc_basic_device_init(isa_bus, gsi, rtc_state, floppy, false, true); /* connect pm stuff to lpc */ ich9_lpc_pm_init(lpc); @@ -270,6 +270,10 @@ static QEMUMachine pc_q35_machine_v1_7 = { .name = pc-q35-1.7, .alias = q35, .init = pc_q35_init, +.compat_props = (GlobalProperty[]) { +PC_COMPAT_1_7, +{ /* end of list */ } +}, }; #define PC_Q35_1_6_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 1361a27..bfeccf2 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -136,7 +136,8 @@ DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus); void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, ISADevice **rtc_state, ISADevice **floppy, - bool no_vmport); + bool no_vmport, + bool hpet_irqs); void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd); void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size, const char *boot_device, @@ -227,7 +228,15 @@ void
Re: [Qemu-devel] [PATCH] qemu-iotests: fix 030 for faster machines
On Wed, 10/16 20:45, Max Reitz wrote: On 2013-10-15 04:41, Fam Zheng wrote: If the block job completes too fast, the test can fail. Change the numbers so the qmp events are more stably captured by the script. A sleep is removed for the same reason. Signed-off-by: Fam Zheng f...@redhat.com --- tests/qemu-iotests/030 | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030 index ae56f3b..188b182 100755 --- a/tests/qemu-iotests/030 +++ b/tests/qemu-iotests/030 @@ -403,14 +403,13 @@ class TestStreamStop(iotests.QMPTestCase): result = self.vm.qmp('block-stream', device='drive0') self.assert_qmp(result, 'return', {}) -time.sleep(0.1) Hm, I'm not sure whether removing the sleep actually removes the underlying race condition… It should work in most cases and the foreseeable future, though. events = self.vm.get_qmp_events(wait=False) self.assertEqual(events, [], 'unexpected QMP event: %s' % events) self.cancel_and_wait() class TestSetSpeed(iotests.QMPTestCase): -image_len = 80 * 1024 * 1024 # MB +image_len = 512 * 1024 * 1024 # MB def setUp(self): qemu_img('create', backing_img, str(TestSetSpeed.image_len)) @@ -457,23 +456,23 @@ class TestSetSpeed(iotests.QMPTestCase): self.assert_qmp(result, 'return[0]/device', 'drive0') self.assert_qmp(result, 'return[0]/speed', 0) -result = self.vm.qmp('block-job-set-speed', device='drive0', speed=8 * 1024 * 1024) +result = self.vm.qmp('block-job-set-speed', device='drive0', speed=8 * 1024) So the limit was already 8 MB/s? Doesn't this mean that the job should have taken 10 seconds anyway? Sounds to me like the block job speed is basically disregarded anyway. No, see below... If I re-add the sleep you removed in this patch, this test fails again for me. This further suggests block-job-set-speed to be kind of a no-op and the changes concerning the image length and the block job speed not really contributing to fixing the test. So I think removing the sleep is all that would have to be done right now. OTOH, this is not really a permanent fix, either (the fundamental race condition remains). Furthermore, I guess there is some reason for having a sleep there - else it would not exist in the first place (and it apparently already caused problems some time ago which were fixed by replacing the previous sleep(1) by sleep(0.1)). All in all, if someone can assure me of the uneccessity of the sleep in question, I think removing it is all that's needed. Max Both failure cases are just that setting speed or checking status comes too late: the streaming finishes or goes close to finish in negligible no time once the job is started. In other words dropping the speed change but only increase image_len and remove sleep will fix it for me too. Fam
Re: [Qemu-devel] [PATCH v7 4/4] PC: differentiate hpet's interrupt capability on piix and q35
On Thu, Oct 17, 2013 at 02:27:43PM +0800, liu ping fan wrote: On Thu, Oct 17, 2013 at 1:44 PM, Michael S. Tsirkin m...@redhat.com wrote: On Thu, Oct 17, 2013 at 11:16:05AM +0800, Liu Ping Fan wrote: For pc-piix-*, hpet's intcap is always hard coded as IRQ2. For q35, if it is pc-q35-1.7 and earlier, we use IRQ2 for compat reason, otherwise IRQ2, IRQ8, and IRQ16~23 are allowed. Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com --- hw/i386/pc.c | 20 +++- hw/i386/pc_piix.c| 7 ++- hw/i386/pc_q35.c | 6 +- include/hw/i386/pc.h | 11 ++- 4 files changed, 36 insertions(+), 8 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 72cc850..3e98ff0 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1219,7 +1219,8 @@ static const MemoryRegionOps ioportF0_io_ops = { void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, ISADevice **rtc_state, ISADevice **floppy, - bool no_vmport) + bool no_vmport, + bool hpet_irqs) { int i; DriveInfo *fd[MAX_FD]; @@ -1249,10 +1250,19 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, /* In order to set property, here not using sysbus_try_create_simple */ hpet = qdev_try_create(NULL, hpet); if (hpet) { -/* tmp fix. For compat, hard code to IRQ2 until we have correct - * compat property and differentiate pc-iix with pc-q35 - */ -qdev_prop_set_uint32(hpet, HPET_INTCAP, 0x4); +/* For pc-piix-*, hpet's intcap is always IRQ2. */ +if (!hpet_irqs) { So for piix the property is ignored even if set. Yes. +qdev_prop_set_uint32(hpet, HPET_INTCAP, 0x4); +} else { +/* For pc-q35-1.7 and earlier, use IRQ2 for compat. Now you lost me. It's still 4 for 1.7? You only target 1.8 then? Yes, since the compat for guest. And start this fix with 1.8 + * Otherwise, use IRQ16~23, IRQ8 and IRQ2. + */ +uint8_t compat = object_property_get_int(OBJECT(hpet), +HPET_INTCAP, NULL); +if (!compat) { +qdev_prop_set_uint32(hpet, HPET_INTCAP, 0xff0104); +} +} qdev_init_nofail(hpet); sysbus_mmio_map(SYS_BUS_DEVICE(hpet), 0, HPET_BASE); diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index c6042c7..a45ce11 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -180,7 +180,8 @@ static void pc_init1(QEMUMachineInitArgs *args, pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL); /* init basic PC hardware */ -pc_basic_device_init(isa_bus, gsi, rtc_state, floppy, xen_enabled()); +pc_basic_device_init(isa_bus, gsi, rtc_state, floppy, xen_enabled(), +false); pc_nic_init(isa_bus, pci_bus); @@ -346,6 +347,10 @@ static QEMUMachine pc_i440fx_machine_v1_7 = { .alias = pc, .init = pc_init_pci, .is_default = 1, +.compat_props = (GlobalProperty[]) { +PC_COMPAT_1_7, So you add this property for PIIX but it's then ignored? That's kind of ugly. Yes a little ugly, but PIIX and ich9 diverge for hpet's hardware. Maybe only add it for Q35 then? Create PC_Q35_COMPAT_1_7 and use everywhere ... Not sure how to fix this. Paolo? +{ /* end of list */ } +}, }; #define PC_I440FX_1_6_MACHINE_OPTIONS PC_I440FX_MACHINE_OPTIONS diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index ca84e1c..124ecc1 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -181,7 +181,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args) pc_register_ferr_irq(gsi[13]); /* init basic PC hardware */ -pc_basic_device_init(isa_bus, gsi, rtc_state, floppy, false); +pc_basic_device_init(isa_bus, gsi, rtc_state, floppy, false, true); /* connect pm stuff to lpc */ ich9_lpc_pm_init(lpc); @@ -270,6 +270,10 @@ static QEMUMachine pc_q35_machine_v1_7 = { .name = pc-q35-1.7, .alias = q35, .init = pc_q35_init, +.compat_props = (GlobalProperty[]) { +PC_COMPAT_1_7, +{ /* end of list */ } +}, }; #define PC_Q35_1_6_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 1361a27..bfeccf2 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -136,7 +136,8 @@ DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus); void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, ISADevice **rtc_state, ISADevice **floppy, - bool no_vmport); +
[Qemu-devel] [PATCH 1/2] net/e1000: update network information when macaddr is changed in guest
If we change macaddr in guest by 'ifconfig eth0 hw ether 12:12:12:34:35:36', the mac register of e1000 is already updated, but we don't update network information in qemu. Therefor, the information in monitor is wrong. This patch updates nic info when the second part of macaddr is written. Signed-off-by: Amos Kong ak...@redhat.com --- hw/net/e1000.c | 8 1 file changed, 8 insertions(+) diff --git a/hw/net/e1000.c b/hw/net/e1000.c index 151d25e..7769be0 100644 --- a/hw/net/e1000.c +++ b/hw/net/e1000.c @@ -1105,7 +1105,15 @@ mac_read_clr8(E1000State *s, int index) static void mac_writereg(E1000State *s, int index, uint32_t val) { +uint32_t macaddr[2]; + s-mac_reg[index] = val; + +if (index == RA + 1) { +macaddr[0] = cpu_to_le32(s-mac_reg[RA]); +macaddr[1] = cpu_to_le32(s-mac_reg[RA + 1]); +qemu_format_nic_info_str(qemu_get_queue(s-nic), (uint8_t *)macaddr); +} } static void -- 1.8.3.1
[Qemu-devel] [PATCH 0/2] fix updating of nic info
I tried to change macaddr in guest, addr in guest is updated, and guest network is fine. But the nic information in monitor isn't update. This problem both exists in e1000 and rtl8139. 1) change macaddr in guest by ifconfig guest)# ifconfig eth0 hw ether 12:12:12:34:35:36 guest)# ifconfig eth0 2) check network information in monitor (qemu) info network Amos Kong (2): net/e1000: update network information when macaddr is changed in guest net/rtl8139: update network information when macaddr is changed in guest hw/net/e1000.c | 8 hw/net/rtl8139.c | 6 +- 2 files changed, 13 insertions(+), 1 deletion(-) -- 1.8.3.1
[Qemu-devel] [PATCH 2/2] net/rtl8139: update network information when macaddr is changed in guest
rtl8139 has same problem as e1000, nic info isn't updated when macaddr is changed in guest. This patch updates the nic info when the last bit of macaddr is written. Signed-off-by: Amos Kong ak...@redhat.com --- hw/net/rtl8139.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c index c31199f..248d9e9 100644 --- a/hw/net/rtl8139.c +++ b/hw/net/rtl8139.c @@ -2740,8 +2740,12 @@ static void rtl8139_io_writeb(void *opaque, uint8_t addr, uint32_t val) switch (addr) { -case MAC0 ... MAC0+5: +case MAC0 ... MAC0+4: +s-phys[addr - MAC0] = val; +break; +case MAC0+5: s-phys[addr - MAC0] = val; +qemu_format_nic_info_str(qemu_get_queue(s-nic), s-phys); break; case MAC0+6 ... MAC0+7: /* reserved */ -- 1.8.3.1
Re: [Qemu-devel] [PATCH 0/2] fix updating of nic info
On Thu, Oct 17, 2013 at 03:02:48PM +0800, Amos Kong wrote: I tried to change macaddr in guest, addr in guest is updated, and guest network is fine. But the nic information in monitor isn't update. This problem both exists in e1000 and rtl8139. 1) change macaddr in guest by ifconfig guest)# ifconfig eth0 hw ether 12:12:12:34:35:36 guest)# ifconfig eth0 2) check network information in monitor (qemu) info network Amos Kong (2): net/e1000: update network information when macaddr is changed in guest net/rtl8139: update network information when macaddr is changed in guest hw/net/e1000.c | 8 hw/net/rtl8139.c | 6 +- 2 files changed, 13 insertions(+), 1 deletion(-) Thanks, applied to my net tree: https://github.com/stefanha/qemu/commits/net Stefan
Re: [Qemu-devel] [PATCH v2] hmp: solve '\n' in monitor_printf()
Am 17.10.2013 um 05:16 hat Mike Qiu geschrieben: Change to v1: remove '[not inserted]' line instead of adding '\n' Please put such notes for reviewers below the --- line, so that git am automatically removes them and they don't end up in the git commit message. Output of 'info block' scsi0-hd0: /images/f18-ppc64.qcow2 (qcow2) [not inserted] scsi0-cd2: [not inserted] Removable device: not locked, tray closed floppy0: [not inserted] Removable device: not locked, tray closed sd0: [not inserted] Removable device: not locked, tray closed There will be no additional lines between scsi0-hd0 and scsi0-cd2. At the same time, scsi0-hd0 already inserted, but still has '[not inserted]' flag. This line should be removed. This patch is to solve this. Signed-off-by: Mike Qiu qiud...@linux.vnet.ibm.com Reviewed-by: Kevin Wolf kw...@redhat.com
Re: [Qemu-devel] [PATCH 0/2] fix updating of nic info
On Thu, Oct 17, 2013 at 09:58:47AM +0200, Stefan Hajnoczi wrote: On Thu, Oct 17, 2013 at 03:02:48PM +0800, Amos Kong wrote: I tried to change macaddr in guest, addr in guest is updated, and guest network is fine. But the nic information in monitor isn't update. This problem both exists in e1000 and rtl8139. 1) change macaddr in guest by ifconfig guest)# ifconfig eth0 hw ether 12:12:12:34:35:36 guest)# ifconfig eth0 2) check network information in monitor (qemu) info network Amos Kong (2): net/e1000: update network information when macaddr is changed in guest net/rtl8139: update network information when macaddr is changed in guest hw/net/e1000.c | 8 hw/net/rtl8139.c | 6 +- 2 files changed, 13 insertions(+), 1 deletion(-) Thanks, applied to my net tree: https://github.com/stefanha/qemu/commits/net Stefan Please revert, this is buggy: info is changed on guest mac write but is not reverted on reset. Let's fix it properly, no need to introduce new regressions when fixing old bugs :) -- MST
Re: [Qemu-devel] [PATCH v2] hmp: solve '\n' in monitor_printf()
On Wed, Oct 16, 2013 at 11:16:01PM -0400, Mike Qiu wrote: Change to v1: remove '[not inserted]' line instead of adding '\n' Output of 'info block' scsi0-hd0: /images/f18-ppc64.qcow2 (qcow2) [not inserted] scsi0-cd2: [not inserted] Removable device: not locked, tray closed floppy0: [not inserted] Removable device: not locked, tray closed sd0: [not inserted] Removable device: not locked, tray closed There will be no additional lines between scsi0-hd0 and scsi0-cd2. At the same time, scsi0-hd0 already inserted, but still has '[not inserted]' flag. This line should be removed. This patch is to solve this. Signed-off-by: Mike Qiu qiud...@linux.vnet.ibm.com --- hmp.c | 2 -- 1 file changed, 2 deletions(-) The commit message is out-of-date, this patch now drops the bogus [not inserted] output. I fixed up the commit message. In the future, please put the patch changelog after the --- line so that git am does not include the changelog in the commit description. Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
Re: [Qemu-devel] [PATCH] sd: Avoid access to NULL BlockDriverState
On Wed, Oct 16, 2013 at 03:24:01PM +0200, Andreas Färber wrote: Commit 4f8a066b5fc254eeaabbbde56ba4f5b29cc68fdf (blockdev: Remove IF_* check for read-only blockdev_init) added a usage of bdrv_is_read_only() to sd_init(), which is called for versatilepb, versatileab and xilinx-zynq-a9 machines among others with NULL argument by default, causing the new qom-test to fail. Add a check to prevent this. Suggested-by: Kevin Wolf kw...@redhat.com Signed-off-by: Andreas Färber afaer...@suse.de --- hw/sd/sd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
Re: [Qemu-devel] [PULL 42/43] piix4: add acpi pci hotplug support
Hi, By far the best way to test this is to boot some guest, download tables, then run iasl -d on them. Fully agree. /me did the same when testing with coreboot. Boot linux, with old seabios, with new seabios, with (patched) coreboot, then diffed the iasl decompiled tables. cheers, Gerd
Re: [Qemu-devel] [PATCH v8 0/3] block: Add support for Secure Shell (ssh) block device.
On Tue, Apr 09, 2013 at 10:56:30AM +0100, Richard W.M. Jones wrote: Changes since v7: - fsync (ie. bdrv_co_flush_to_disk) is now supported, *if* you have the following patches to libssh2 and OpenSSH: https://bugzilla.mindrot.org/show_bug.cgi?id=1798 (OpenSSH: accepted, but not upstream) FYI: OpenSSH 6.4 has finally added fsync support upstream! Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
Re: [Qemu-devel] [PATCH] kvm-unittest: fix build with gcc 4.3.X and older
On Thu, Oct 17, 2013 at 11:12:31AM +0300, Michael S. Tsirkin wrote: On Thu, Oct 17, 2013 at 09:27:51AM +0300, Gleb Natapov wrote: On Wed, Oct 16, 2013 at 10:46:53PM +0300, Michael S. Tsirkin wrote: Old GCC didn't let you reference variable by number if it is listed with a specific register constraint, on the assumption you can just use the register name explicitly. Build fails with errors like this: a.c:6: error: invalid 'asm': invalid operand code 'd' Is it worth to support such ancient compiler? Nobody complained till now. Well it's not as widely used as kvm itself yet :) The patch seems simple enough though. BTW with your patch I still cannot compile with 4.2: x86/s3.c: In function 'main': x86/s3.c:145: error: inconsistent operand constraints in an 'asm' OK that's easy to fix. To fix, let's just use %eax %al etc. Only %d0 does not work and dropping d fixes it since compiler can figure out correct register from variable size. The patch bellow fixes compilation for 4.2. It does produce warnings with -Wall though: Assembler messages: Warning: using `%ax' instead of `%eax' due to `w' suffix Warning: using `%al' instead of `%eax' due to `b' suffix Not for me. No warning with 4.7.3 and 4.2. .s file produced by gcc shows correct assembly with my patch. -- Gleb.
Re: [Qemu-devel] [PATCH v2] blockdev: fix cdrom read_only flag
On Tue, Oct 15, 2013 at 05:45:50PM +0800, Fam Zheng wrote: Since 0ebd24e0, cdrom doesn't have read-only on by default, which will error out when using an read only image. Fix it by setting the default value when parsing opts. Reported-by: Edivaldo de Araujo Pereira edivaldoapere...@yahoo.com.br Signed-off-by: Fam Zheng f...@redhat.com --- v2: fix backward compatibility by force read-only with cdrom. (Kevin) Signed-off-by: Fam Zheng f...@redhat.com --- blockdev.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
Re: [Qemu-devel] [PATCH] kvm-unittest: fix build with gcc 4.3.X and older
On Thu, Oct 17, 2013 at 11:20:27AM +0300, Gleb Natapov wrote: On Thu, Oct 17, 2013 at 11:12:31AM +0300, Michael S. Tsirkin wrote: On Thu, Oct 17, 2013 at 09:27:51AM +0300, Gleb Natapov wrote: On Wed, Oct 16, 2013 at 10:46:53PM +0300, Michael S. Tsirkin wrote: Old GCC didn't let you reference variable by number if it is listed with a specific register constraint, on the assumption you can just use the register name explicitly. Build fails with errors like this: a.c:6: error: invalid 'asm': invalid operand code 'd' Is it worth to support such ancient compiler? Nobody complained till now. Well it's not as widely used as kvm itself yet :) The patch seems simple enough though. BTW with your patch I still cannot compile with 4.2: x86/s3.c: In function 'main': x86/s3.c:145: error: inconsistent operand constraints in an 'asm' OK that's easy to fix. To fix, let's just use %eax %al etc. Only %d0 does not work and dropping d fixes it since compiler can figure out correct register from variable size. The patch bellow fixes compilation for 4.2. It does produce warnings with -Wall though: Assembler messages: Warning: using `%ax' instead of `%eax' due to `w' suffix Warning: using `%al' instead of `%eax' due to `b' suffix Not for me. No warning with 4.7.3 and 4.2. .s file produced by gcc shows correct assembly with my patch. 4.3 doesn't. We have the a constraint there anyway - so what's the issue with just writing %eax etc? I think it's safer than relying on compiler to do the right thing. -- Gleb.
Re: [Qemu-devel] [PATCH] kvm-unittest: fix build with gcc 4.3.X and older
On Thu, Oct 17, 2013 at 09:27:51AM +0300, Gleb Natapov wrote: On Wed, Oct 16, 2013 at 10:46:53PM +0300, Michael S. Tsirkin wrote: Old GCC didn't let you reference variable by number if it is listed with a specific register constraint, on the assumption you can just use the register name explicitly. Build fails with errors like this: a.c:6: error: invalid 'asm': invalid operand code 'd' Is it worth to support such ancient compiler? Nobody complained till now. Well it's not as widely used as kvm itself yet :) The patch seems simple enough though. BTW with your patch I still cannot compile with 4.2: x86/s3.c: In function 'main': x86/s3.c:145: error: inconsistent operand constraints in an 'asm' OK that's easy to fix. To fix, let's just use %eax %al etc. Only %d0 does not work and dropping d fixes it since compiler can figure out correct register from variable size. The patch bellow fixes compilation for 4.2. It does produce warnings with -Wall though: Assembler messages: Warning: using `%ax' instead of `%eax' due to `w' suffix Warning: using `%al' instead of `%eax' due to `b' suffix So it's not the compiler, the compiler generated wrong code with outb %eax. But assembler saw outb and %eax which are inconsistent and decided that outb should take precedence. I'm not sure it's wize to rely on this behaviour though. How's this? This should fix 4.2 for you too. Maybe we should tweak outb/outw as well to keep it all consistent. I can do this - what do you think? diff --git a/lib/x86/pci.c b/lib/x86/pci.c index f95cd88..08f7ebf 100644 --- a/lib/x86/pci.c +++ b/lib/x86/pci.c @@ -3,13 +3,13 @@ static void outl(unsigned short port, unsigned val) { -asm volatile(outl %d0, %w1 : : a(val), Nd(port)); +asm volatile(outl %%eax, %w1 : : a(val), Nd(port)); } static unsigned inl(unsigned short port) { unsigned data; -asm volatile(inl %w1, %d0 : =a(data) : Nd(port)); +asm volatile(inl %w1, %%eax : =a(data) : Nd(port)); return data; } static uint32_t pci_config_read(pcidevaddr_t dev, uint8_t reg) diff --git a/x86/s3.c b/x86/s3.c index 1feb452..bfa4206 100644 --- a/x86/s3.c +++ b/x86/s3.c @@ -142,15 +142,15 @@ u32* find_resume_vector_addr(void) static inline int rtc_in(u8 reg) { u8 x = reg; -asm volatile(outb %b1, $0x70; inb $0x71, %b0 -: +a(x) : 0(x)); +asm volatile(outb %%al, $0x70; inb $0x71, %%al +: +a(x)); return x; } static inline void rtc_out(u8 reg, u8 val) { -asm volatile(outb %b1, $0x70; mov %b2, %b1; outb %b1, $0x71 -: +a(reg) : 0(reg), ri(val)); +asm volatile(outb %%al, $0x70; mov %b1, %%al; outb %%al, $0x71 +: +a(reg) : ri(val)); } extern char resume_start, resume_end; diff --git a/x86/vmexit.c b/x86/vmexit.c index 3b945de..29bc582 100644 --- a/x86/vmexit.c +++ b/x86/vmexit.c @@ -26,7 +26,7 @@ static void outw(unsigned short port, unsigned val) static void outl(unsigned short port, unsigned val) { -asm volatile(outl %d0, %w1 : : a(val), Nd(port)); +asm volatile(outl %%eax, %w1 : : a(val), Nd(port)); } static unsigned int inb(unsigned short port) -- MST
Re: [Qemu-devel] [PATCH 0/2] fix updating of nic info
On Thu, Oct 17, 2013 at 11:15:21AM +0300, Michael S. Tsirkin wrote: On Thu, Oct 17, 2013 at 09:58:47AM +0200, Stefan Hajnoczi wrote: On Thu, Oct 17, 2013 at 03:02:48PM +0800, Amos Kong wrote: I tried to change macaddr in guest, addr in guest is updated, and guest network is fine. But the nic information in monitor isn't update. This problem both exists in e1000 and rtl8139. 1) change macaddr in guest by ifconfig guest)# ifconfig eth0 hw ether 12:12:12:34:35:36 guest)# ifconfig eth0 2) check network information in monitor (qemu) info network Amos Kong (2): net/e1000: update network information when macaddr is changed in guest net/rtl8139: update network information when macaddr is changed in guest hw/net/e1000.c | 8 hw/net/rtl8139.c | 6 +- 2 files changed, 13 insertions(+), 1 deletion(-) Thanks, applied to my net tree: https://github.com/stefanha/qemu/commits/net Stefan Please revert, this is buggy: info is changed on guest mac write but is not reverted on reset. Let's fix it properly, no need to introduce new regressions when fixing old bugs :) After 'system_reset' or 'reboot', nic info can't be changed until device init. Updating nic info during reset is another issue, I will send patch to fix it. -- MST -- Amos.
Re: [Qemu-devel] [PATCH] kvm-unittest: fix build with gcc 4.3.X and older
On Thu, Oct 17, 2013 at 11:27:37AM +0300, Michael S. Tsirkin wrote: On Thu, Oct 17, 2013 at 11:20:27AM +0300, Gleb Natapov wrote: On Thu, Oct 17, 2013 at 11:12:31AM +0300, Michael S. Tsirkin wrote: On Thu, Oct 17, 2013 at 09:27:51AM +0300, Gleb Natapov wrote: On Wed, Oct 16, 2013 at 10:46:53PM +0300, Michael S. Tsirkin wrote: Old GCC didn't let you reference variable by number if it is listed with a specific register constraint, on the assumption you can just use the register name explicitly. Build fails with errors like this: a.c:6: error: invalid 'asm': invalid operand code 'd' Is it worth to support such ancient compiler? Nobody complained till now. Well it's not as widely used as kvm itself yet :) The patch seems simple enough though. BTW with your patch I still cannot compile with 4.2: x86/s3.c: In function 'main': x86/s3.c:145: error: inconsistent operand constraints in an 'asm' OK that's easy to fix. To fix, let's just use %eax %al etc. Only %d0 does not work and dropping d fixes it since compiler can figure out correct register from variable size. The patch bellow fixes compilation for 4.2. It does produce warnings with -Wall though: Assembler messages: Warning: using `%ax' instead of `%eax' due to `w' suffix Warning: using `%al' instead of `%eax' due to `b' suffix Not for me. No warning with 4.7.3 and 4.2. .s file produced by gcc shows correct assembly with my patch. 4.3 doesn't. I pretty much doubt that 4.2 and 4.7 produce correct assembly and 4.3 does not. Which file is it? Send me .s file produced by it. We have the a constraint there anyway - so what's the issue with just writing %eax etc? I think it's safer than relying on compiler to do the right thing. It just papers over the problem. Compiler should either complain that it does not know what %w0 or complain that variable length does not match assembly or use correct register. -- Gleb.
[Qemu-devel] [PATCH] net: update nic info during device reset
macaddr is reset during device reset, but nic info isn't updated, this problem exists in e1000 rtl8139 Signed-off-by: Amos Kong ak...@redhat.com --- hw/net/e1000.c | 1 + hw/net/rtl8139.c | 1 + 2 files changed, 2 insertions(+) diff --git a/hw/net/e1000.c b/hw/net/e1000.c index 7769be0..70a59fd 100644 --- a/hw/net/e1000.c +++ b/hw/net/e1000.c @@ -401,6 +401,7 @@ static void e1000_reset(void *opaque) d-mac_reg[RA] |= macaddr[i] (8 * i); d-mac_reg[RA + 1] |= (i 2) ? macaddr[i + 4] (8 * i) : 0; } +qemu_format_nic_info_str(qemu_get_queue(d-nic), macaddr); } static void diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c index 248d9e9..3225f3d 100644 --- a/hw/net/rtl8139.c +++ b/hw/net/rtl8139.c @@ -1214,6 +1214,7 @@ static void rtl8139_reset(DeviceState *d) /* restore MAC address */ memcpy(s-phys, s-conf.macaddr.a, 6); +qemu_format_nic_info_str(qemu_get_queue(s-nic), s-phys); /* reset interrupt mask */ s-IntrStatus = 0; -- 1.8.3.1
Re: [Qemu-devel] [PATCH v2] hmp: solve '\n' in monitor_printf()
On 10/17/2013 04:11 PM, Kevin Wolf wrote: Am 17.10.2013 um 05:16 hat Mike Qiu geschrieben: Change to v1: remove '[not inserted]' line instead of adding '\n' Please put such notes for reviewers below the --- line, so that git am automatically removes them and they don't end up in the git commit message. OK, thanks for your kindly remind, Thanks Mike Output of 'info block' scsi0-hd0: /images/f18-ppc64.qcow2 (qcow2) [not inserted] scsi0-cd2: [not inserted] Removable device: not locked, tray closed floppy0: [not inserted] Removable device: not locked, tray closed sd0: [not inserted] Removable device: not locked, tray closed There will be no additional lines between scsi0-hd0 and scsi0-cd2. At the same time, scsi0-hd0 already inserted, but still has '[not inserted]' flag. This line should be removed. This patch is to solve this. Signed-off-by: Mike Qiu qiud...@linux.vnet.ibm.com Reviewed-by: Kevin Wolf kw...@redhat.com
Re: [Qemu-devel] [PATCH v2] hmp: solve '\n' in monitor_printf()
On 10/17/2013 04:14 PM, Stefan Hajnoczi wrote: On Wed, Oct 16, 2013 at 11:16:01PM -0400, Mike Qiu wrote: Change to v1: remove '[not inserted]' line instead of adding '\n' Output of 'info block' scsi0-hd0: /images/f18-ppc64.qcow2 (qcow2) [not inserted] scsi0-cd2: [not inserted] Removable device: not locked, tray closed floppy0: [not inserted] Removable device: not locked, tray closed sd0: [not inserted] Removable device: not locked, tray closed There will be no additional lines between scsi0-hd0 and scsi0-cd2. At the same time, scsi0-hd0 already inserted, but still has '[not inserted]' flag. This line should be removed. This patch is to solve this. Signed-off-by: Mike Qiu qiud...@linux.vnet.ibm.com --- hmp.c | 2 -- 1 file changed, 2 deletions(-) The commit message is out-of-date, this patch now drops the bogus [not inserted] output. I fixed up the commit message. In the future, please put the patch changelog after the --- line so that git am does not include the changelog in the commit description. OK, Got it, thanks for your kindly remind :) Thanks Mike Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
Re: [Qemu-devel] qemu-system-or32 is not working on OS X, ask for help.
On 17 October 2013 04:17, Jia Liu pro...@gmail.com wrote: On Fri, Oct 11, 2013 at 10:41 AM, Jia Liu pro...@gmail.com wrote: I'm not sure about why qemu-system-or32 is not working on OS X, is it a AREG0 problem? May you please give me some suggestion, I want to test it on OS X, not Ubuntu any longer. The things that meant we had to use gcc for its put variable in a fixed native register functionality have gone away, so OSX+clang should work OK. I test the ARM targets from time to time and they work. GCC on OS X is OK, it looks like a Clang and GCC different, any suggestion, please? You'll need to debug the problem, that's all. Watch out that OSX gdb is broken in an odd way that means you can't really debug qemu under it (it breaks sigwait()). In general I think OSX is a poor target for trying to develop and debug qemu under, not least because of that gdb issue. But it should in principle work if you're not trying to run under the debugger. -- PMM
Re: [Qemu-devel] [RFC PATCH v3 4/5] Update documentation for LTTng ust tracing
mohamad.ge...@polymtl.ca writes: On 13-10-16 08:05 AM, Alex Bennée wrote: Running this gives me: quote UST events: - None /quote Before or after running qemu. What is the mechanism lttng expects to find out all these events? Either the user should belong the group tracing, or launch the lttng-sessiond daemon (lttng-sessiond -d). snip Hmm I've done both and I get nothing still. I can enable all tracepoints though. This could just be a Ubuntu weirdness thing though. That's weird. I did test it on a clean Ubuntu Precise (in a VM) and I didn't have any problem after either one of these steps. Can you please make sure that the user belongs to the tracing group and that the lttng-sessiond daemon is running before running any instance of Qemu. Also, Qemu should be running for LTTng to be able to list the events. Ahh that was it. I suggest you change the wording from: Package lttng-tools is required for userspace tracing. After running Qemu, LTTng should be able to list all available events: to Package lttng-tools is required for userspace tracing. While running and instrumented Qemu, LTTng should be able to list all available events: Does lttng enable-event -a -u/start/stop/view show any event? Yes, as I said the rest worked fine. With that minor wording fix I'm happy. Thanks for getting this back into shape :-) -- Alex Bennée
Re: [Qemu-devel] [PATCH] kvm-unittest: fix build with gcc 4.3.X and older
On Thu, Oct 17, 2013 at 11:34:41AM +0300, Gleb Natapov wrote: On Thu, Oct 17, 2013 at 11:27:37AM +0300, Michael S. Tsirkin wrote: On Thu, Oct 17, 2013 at 11:20:27AM +0300, Gleb Natapov wrote: On Thu, Oct 17, 2013 at 11:12:31AM +0300, Michael S. Tsirkin wrote: On Thu, Oct 17, 2013 at 09:27:51AM +0300, Gleb Natapov wrote: On Wed, Oct 16, 2013 at 10:46:53PM +0300, Michael S. Tsirkin wrote: Old GCC didn't let you reference variable by number if it is listed with a specific register constraint, on the assumption you can just use the register name explicitly. Build fails with errors like this: a.c:6: error: invalid 'asm': invalid operand code 'd' Is it worth to support such ancient compiler? Nobody complained till now. Well it's not as widely used as kvm itself yet :) The patch seems simple enough though. BTW with your patch I still cannot compile with 4.2: x86/s3.c: In function 'main': x86/s3.c:145: error: inconsistent operand constraints in an 'asm' OK that's easy to fix. To fix, let's just use %eax %al etc. Only %d0 does not work and dropping d fixes it since compiler can figure out correct register from variable size. The patch bellow fixes compilation for 4.2. It does produce warnings with -Wall though: Assembler messages: Warning: using `%ax' instead of `%eax' due to `w' suffix Warning: using `%al' instead of `%eax' due to `b' suffix Not for me. No warning with 4.7.3 and 4.2. .s file produced by gcc shows correct assembly with my patch. 4.3 doesn't. I pretty much doubt that 4.2 and 4.7 produce correct assembly and 4.3 does not. Which file is it? Send me .s file produced by it. Tried to get it but problem went away after I reset and re-applied your patch. I think I had some other change in there that caused it, and blamed your patch incorrectly. Sorry. We have the a constraint there anyway - so what's the issue with just writing %eax etc? I think it's safer than relying on compiler to do the right thing. It just papers over the problem. Compiler should either complain that it does not know what %w0 or complain that variable length does not match assembly Of course it can't. Compiler does not parse assembly at all: these are just constant strings as far as it's concerned. or use correct register. Actually it does: it seems to correctly deduce size from variable type. So your patch looks good to me, please apply. -- Gleb.
Re: [Qemu-devel] [PATCH] Fix VNC SASL authentication when using a QXL device
On Mi, 2013-10-16 at 17:52 +0200, Christophe Fergeau wrote: ui/vnc.c:vnc_display_open() and spice-server/server/reds.c:do_spice_init() are both calling sasl_server_init(). If spice_server_set_sasl_appname() hasn't been called, spice-server will call it with spice as an appname, causing cyrus-sasl to try to use a /etc/sasl2/spice.conf config file rather than the /etc/sasl2/qemu.conf file that QEMU uses. When using -spice sasl on the command line, QEMU properly calls spice_server_set_sasl_appname() to set the SASL appname as qemu, but when using a QXL device without using SPICE, spice_server_init() is called from qemu_spice_add_interface() without setting the appname to qemu, which then causes the VNC code to try to use spice.conf instead of qemu.conf. patch added to spice patch queue. thanks, Gerd
Re: [Qemu-devel] [PATCH 0/2] fix updating of nic info
On Thu, Oct 17, 2013 at 04:31:22PM +0800, Amos Kong wrote: On Thu, Oct 17, 2013 at 11:15:21AM +0300, Michael S. Tsirkin wrote: On Thu, Oct 17, 2013 at 09:58:47AM +0200, Stefan Hajnoczi wrote: On Thu, Oct 17, 2013 at 03:02:48PM +0800, Amos Kong wrote: I tried to change macaddr in guest, addr in guest is updated, and guest network is fine. But the nic information in monitor isn't update. This problem both exists in e1000 and rtl8139. 1) change macaddr in guest by ifconfig guest)# ifconfig eth0 hw ether 12:12:12:34:35:36 guest)# ifconfig eth0 2) check network information in monitor (qemu) info network Amos Kong (2): net/e1000: update network information when macaddr is changed in guest net/rtl8139: update network information when macaddr is changed in guest hw/net/e1000.c | 8 hw/net/rtl8139.c | 6 +- 2 files changed, 13 insertions(+), 1 deletion(-) Thanks, applied to my net tree: https://github.com/stefanha/qemu/commits/net Stefan Please revert, this is buggy: info is changed on guest mac write but is not reverted on reset. Let's fix it properly, no need to introduce new regressions when fixing old bugs :) After 'system_reset' or 'reboot', nic info can't be changed until device init. Updating nic info during reset is another issue, I will send patch to fix it. OK so let's apply the reset change first, then these patches on top. This way we don't break bisect. Stefan can you rearrange pls? -- MST -- Amos.
Re: [Qemu-devel] [PATCH] net: update nic info during device reset
On Thu, Oct 17, 2013 at 04:38:34PM +0800, Amos Kong wrote: macaddr is reset during device reset, but nic info isn't updated, this problem exists in e1000 rtl8139 Signed-off-by: Amos Kong ak...@redhat.com Acked-by: Michael S. Tsirkin m...@redhat.com --- hw/net/e1000.c | 1 + hw/net/rtl8139.c | 1 + 2 files changed, 2 insertions(+) diff --git a/hw/net/e1000.c b/hw/net/e1000.c index 7769be0..70a59fd 100644 --- a/hw/net/e1000.c +++ b/hw/net/e1000.c @@ -401,6 +401,7 @@ static void e1000_reset(void *opaque) d-mac_reg[RA] |= macaddr[i] (8 * i); d-mac_reg[RA + 1] |= (i 2) ? macaddr[i + 4] (8 * i) : 0; } +qemu_format_nic_info_str(qemu_get_queue(d-nic), macaddr); } static void diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c index 248d9e9..3225f3d 100644 --- a/hw/net/rtl8139.c +++ b/hw/net/rtl8139.c @@ -1214,6 +1214,7 @@ static void rtl8139_reset(DeviceState *d) /* restore MAC address */ memcpy(s-phys, s-conf.macaddr.a, 6); +qemu_format_nic_info_str(qemu_get_queue(s-nic), s-phys); /* reset interrupt mask */ s-IntrStatus = 0; -- 1.8.3.1
Re: [Qemu-devel] [PATCH] kvm-unittest: fix build with gcc 4.3.X and older
On Thu, Oct 17, 2013 at 12:28:58PM +0300, Michael S. Tsirkin wrote: On Thu, Oct 17, 2013 at 11:34:41AM +0300, Gleb Natapov wrote: On Thu, Oct 17, 2013 at 11:27:37AM +0300, Michael S. Tsirkin wrote: On Thu, Oct 17, 2013 at 11:20:27AM +0300, Gleb Natapov wrote: On Thu, Oct 17, 2013 at 11:12:31AM +0300, Michael S. Tsirkin wrote: On Thu, Oct 17, 2013 at 09:27:51AM +0300, Gleb Natapov wrote: On Wed, Oct 16, 2013 at 10:46:53PM +0300, Michael S. Tsirkin wrote: Old GCC didn't let you reference variable by number if it is listed with a specific register constraint, on the assumption you can just use the register name explicitly. Build fails with errors like this: a.c:6: error: invalid 'asm': invalid operand code 'd' Is it worth to support such ancient compiler? Nobody complained till now. Well it's not as widely used as kvm itself yet :) The patch seems simple enough though. BTW with your patch I still cannot compile with 4.2: x86/s3.c: In function 'main': x86/s3.c:145: error: inconsistent operand constraints in an 'asm' OK that's easy to fix. To fix, let's just use %eax %al etc. Only %d0 does not work and dropping d fixes it since compiler can figure out correct register from variable size. The patch bellow fixes compilation for 4.2. It does produce warnings with -Wall though: Assembler messages: Warning: using `%ax' instead of `%eax' due to `w' suffix Warning: using `%al' instead of `%eax' due to `b' suffix Not for me. No warning with 4.7.3 and 4.2. .s file produced by gcc shows correct assembly with my patch. 4.3 doesn't. I pretty much doubt that 4.2 and 4.7 produce correct assembly and 4.3 does not. Which file is it? Send me .s file produced by it. Tried to get it but problem went away after I reset and re-applied your patch. I think I had some other change in there that caused it, and blamed your patch incorrectly. Sorry. NP, that makes more sense. We have the a constraint there anyway - so what's the issue with just writing %eax etc? I think it's safer than relying on compiler to do the right thing. It just papers over the problem. Compiler should either complain that it does not know what %w0 or complain that variable length does not match assembly Of course it can't. Compiler does not parse assembly at all: these are just constant strings as far as it's concerned. Compiler does not pars assembly itself but it parses things like %w0 to know what assembly to emit. That is why it complained about %d0 in the first place. or use correct register. Actually it does: it seems to correctly deduce size from variable type. So your patch looks good to me, please apply. Will do. -- Gleb.
Re: [Qemu-devel] [Qemu-ppc] [PATCH V2] Fix float64_to_uint64
On 16.10.2013, at 23:10, Tom Musta tommu...@gmail.com wrote: The comment preceding the float64_to_uint64 routine suggests that the implementation is broken. And this is, indeed, the case. This patch properly implements the conversion of a 64-bit floating point number to an unsigned, 64 bit integer. Note that the patch does not pass scripts/checkpatch.pl because it maintains the coding style of fpu/softfloat.c. V2: This contribution can be licensed under either the softfloat-2a or -2b license. Missing a SoB line. Alex --- fpu/softfloat.c | 92 ++ 1 files changed, 85 insertions(+), 7 deletions(-) diff --git a/fpu/softfloat.c b/fpu/softfloat.c index 7ba51b6..f8c7f92 100644 --- a/fpu/softfloat.c +++ b/fpu/softfloat.c @@ -204,6 +204,46 @@ static int64 roundAndPackInt64( flag zSign, uint64_t absZ0, uint64_t absZ1 STATU } /* +| Takes the 128-bit fixed-point value formed by concatenating `absZ0' and +| `absZ1', with binary point between bits 63 and 64 (between the input words), +| and returns the properly rounded 64-bit unsigned integer corresponding to the +| input. Ordinarily, the fixed-point input is simply rounded to an integer, +| with the inexact exception raised if the input cannot be represented exactly +| as an integer. However, if the fixed-point input is too large, the invalid +| exception is raised and the largest unsigned integer is returned. +**/ + +static int64 roundAndPackUint64( uint64_t absZ0, uint64_t absZ1 STATUS_PARAM) +{ +int8 roundingMode; +flag roundNearestEven, increment; +int64_t z; + +roundingMode = STATUS(float_rounding_mode); +roundNearestEven = ( roundingMode == float_round_nearest_even ); +increment = ( (int64_t) absZ1 0 ); +if ( ! roundNearestEven ) { +if ( roundingMode == float_round_to_zero ) { +increment = 0; +} +else { +increment = ( roundingMode == float_round_up ) absZ1; +} +} +if ( increment ) { +++absZ0; +if ( absZ0 == 0 ) { +float_raise( float_flag_invalid STATUS_VAR); +return LIT64( 0x ); +} +absZ0 = ~ ( ( (uint64_t) ( absZ11 ) == 0 ) roundNearestEven ); +} +z = absZ0; +if ( absZ1 ) STATUS(float_exception_flags) |= float_flag_inexact; +return z; +} + +/* | Returns the fraction bits of the single-precision floating-point value `a'. **/ @@ -6536,18 +6576,56 @@ uint_fast16_t float64_to_uint16_round_to_zero(float64 a STATUS_PARAM) return res; } -/* FIXME: This looks broken. */ -uint64_t float64_to_uint64 (float64 a STATUS_PARAM) +/* +| Returns the result of converting the double-precision floating-point value +| `a' to the 64-bit unsigned integer format. The conversion is +| performed according to the IEC/IEEE Standard for Binary Floating-Point +| Arithmetic---which means in particular that the conversion is rounded +| according to the current rounding mode. If `a' is a NaN, the largest +| positive integer is returned. If the conversion overflows, the +| largest unsigned integer is returned. If 'a' is negative, zero is +| returned. +**/ + +uint64_t float64_to_uint64( float64 a STATUS_PARAM ) { -int64_t v; +flag aSign; +int_fast16_t aExp, shiftCount; +uint64_t aSig, aSigExtra; +a = float64_squash_input_denormal(a STATUS_VAR); -v = float64_val(int64_to_float64(INT64_MIN STATUS_VAR)); -v += float64_val(a); -v = float64_to_int64(make_float64(v) STATUS_VAR); +aSig = extractFloat64Frac( a ); +aExp = extractFloat64Exp( a ); +aSign = extractFloat64Sign( a ); +if ( aSign ) { +if ( aExp ) { +float_raise( float_flag_invalid STATUS_VAR); +} else if ( aSig ) { /* negative denormalized */ +float_raise( float_flag_inexact STATUS_VAR); +} +return 0; +} +if ( aExp ) aSig |= LIT64( 0x0010 ); +shiftCount = 0x433 - aExp; +if ( shiftCount = 0 ) { +if ( 0x43E aExp ) { +if ( ( aSig != LIT64( 0x0010 ) ) || + ( aExp == 0x7FF ) ) { +float_raise( float_flag_invalid STATUS_VAR); +} +return LIT64( 0x ); +} +aSigExtra = 0; +aSig = - shiftCount; +} +else { +shift64ExtraRightJamming( aSig, 0,
Re: [Qemu-devel] [PATCH] kvm-unittest: fix build with gcc 4.3.X and older
On Thu, Oct 17, 2013 at 12:33:39PM +0300, Gleb Natapov wrote: It just papers over the problem. Compiler should either complain that it does not know what %w0 or complain that variable length does not match assembly Of course it can't. Compiler does not parse assembly at all: these are just constant strings as far as it's concerned. Compiler does not pars assembly itself but it parses things like %w0 to know what assembly to emit. That is why it complained about %d0 in the first place. Right. I meant that with this: outl %0 it has no chance to figure out that outl needs eax. It has to go by variable type.
Re: [Qemu-devel] [PATCH] kvm-unittest: fix build with gcc 4.3.X and older
On Thu, Oct 17, 2013 at 12:44:46PM +0300, Michael S. Tsirkin wrote: On Thu, Oct 17, 2013 at 12:33:39PM +0300, Gleb Natapov wrote: It just papers over the problem. Compiler should either complain that it does not know what %w0 or complain that variable length does not match assembly Of course it can't. Compiler does not parse assembly at all: these are just constant strings as far as it's concerned. Compiler does not pars assembly itself but it parses things like %w0 to know what assembly to emit. That is why it complained about %d0 in the first place. Right. I meant that with this: outl %0 it has no chance to figure out that outl needs eax. It has to go by variable type. Yes. -- Gleb.
Re: [Qemu-devel] Which functions of southbridges should be no-user?
Andreas Färber afaer...@suse.de writes: Am 16.10.2013 12:00, schrieb Markus Armbruster: Anthony Liguori anth...@codemonkey.ws writes: On Tue, Oct 15, 2013 at 7:41 AM, Kevin Wolf kw...@redhat.com wrote: Am 15.10.2013 um 15:31 hat Andreas Färber geschrieben: Am 15.10.2013 15:21, schrieb Markus Armbruster: Andreas, To go beyond RFC with this series, I need to explain why the IDE controller functions of southbridges piix3-ide, piix3-ide-xen, piix4-ide and via-ide cannot_instantiate_with_device_add_yet, or drop that. I'd appreciate your help. Our modelling of PCI devices is weird, to put it politely. One of many weird things is that we don't distinguish between a function and a complete device: our function models are actually PCI device models, and can be used as such, even though they only exist as functions of a multifunction device in the real world. We permit collecting aribitrary PCI devices into multifunction devices. One instance of multifunction PCI devices are southbridges. For example, the ICH9 southbridge's PCI device 00:1F consists of ISA bridge (ICH9 LPC), IDE controller (ich9-ahci), SMB controller (ICH9 SMB), and Thermal Subsystem (which we don't implement). The PIIX3 southbridge consists of ISA bridge (PIIX3, IDE controller (piix3-ide), USB controller (piix3-usb-uhci, can be suppressed), and SMB controller (PIIX4_PM, can be suppressed). Some functions of southbridges still need to be wired up in code (ICH9 LPC, ICH9 SMB, PIIX4_PM, PIIX4, PIIX3, PIIX3-xen, see PATCH 5-6/9), thus cannot_instantiate_with_device_add_yet. The IDE controller functions have always been cannot_instantiate_with_device_add_yet, but it's not obvious to me why. The other functions are available with device-add. Users device-add'ing them would of course be odd, but if it works... I don't actually know whether it works for all of them. Should all southbridge functions be made unavailable with device-add for consistency, at least for now? Or should all functions be made available, except for the ones that cannot possibly work with device-add? If the latter, can you think of any specific reason why the IDE controllers couldn't work? I would've thought you and Kevin know more about IDE than me. ;) No idea why it is how it is. Two aspects: 1) PCI devices/functions can technically be hotplugged. 2) Drivers might not expect such devices to be hot-added/removed. I would tend for the latter proposal. I'm not sure how IDE hardware really works, but I don't think you can handle it as a pure PCI device. On PCs, the IDE controller still provides the good old IDE registers on the I/O ports that were already used in ISA times, and they are not described in any BARs, for example. Legacy IDE and VGA accesses are positively decoded in the i440fx and dispatched to the first PCI device with the appropriate class code. From a software point of view, the PCI device is just for configuring Busmaster DMA. Perhaps in reality it should be two devices, one for DMA on the PCI bus and another one for the core on sysbus or ISA? It's definitely all a PCI device. It could realistically be a discrete device too that's plugged into a PCI bus that lacks a super I/O chipset. Anyway, having two IDE controllers using the same I/O ports won't work, obviously. So if you would allow -device or device-add for them, you'd need options to configure the ports at least. It's allowed but the PCI bus will only route the legacy requests to one of them. Any objections to dropping cannot_instantiate_with_device_add_yet from all IDE controller southbridge functions? These are: piix3-ide, piix3-ide-xen, piix4-ide and via-ide. I understood Anthony as describing expected hardware behavior. At least with the old ioport API registering a port twice used to abort. If I add a second piix3-ide controller in the wrong order, the BIOS attempts to boot from the wrong one, and fails. If I add it in the other order, Linux starts booting, but fails to pivot the root device, and drops me in an emergency shell. I can see both controllers in /sys/devices/ there. Unfortunately, I don't have time today to figure out how to make the second controller work. Good enough to drop cannot_instantiate_with_device_add_yet?
Re: [Qemu-devel] Which functions of southbridges should be no-user?
Anthony Liguori anth...@codemonkey.ws writes: On Wed, Oct 16, 2013 at 3:00 AM, Markus Armbruster arm...@redhat.com wrote: Anthony Liguori anth...@codemonkey.ws writes: On Tue, Oct 15, 2013 at 7:41 AM, Kevin Wolf kw...@redhat.com wrote: Am 15.10.2013 um 15:31 hat Andreas Färber geschrieben: Am 15.10.2013 15:21, schrieb Markus Armbruster: Andreas, To go beyond RFC with this series, I need to explain why the IDE controller functions of southbridges piix3-ide, piix3-ide-xen, piix4-ide and via-ide cannot_instantiate_with_device_add_yet, or drop that. I'd appreciate your help. Our modelling of PCI devices is weird, to put it politely. One of many weird things is that we don't distinguish between a function and a complete device: our function models are actually PCI device models, and can be used as such, even though they only exist as functions of a multifunction device in the real world. We permit collecting aribitrary PCI devices into multifunction devices. One instance of multifunction PCI devices are southbridges. For example, the ICH9 southbridge's PCI device 00:1F consists of ISA bridge (ICH9 LPC), IDE controller (ich9-ahci), SMB controller (ICH9 SMB), and Thermal Subsystem (which we don't implement). The PIIX3 southbridge consists of ISA bridge (PIIX3, IDE controller (piix3-ide), USB controller (piix3-usb-uhci, can be suppressed), and SMB controller (PIIX4_PM, can be suppressed). Some functions of southbridges still need to be wired up in code (ICH9 LPC, ICH9 SMB, PIIX4_PM, PIIX4, PIIX3, PIIX3-xen, see PATCH 5-6/9), thus cannot_instantiate_with_device_add_yet. The IDE controller functions have always been cannot_instantiate_with_device_add_yet, but it's not obvious to me why. The other functions are available with device-add. Users device-add'ing them would of course be odd, but if it works... I don't actually know whether it works for all of them. Should all southbridge functions be made unavailable with device-add for consistency, at least for now? Or should all functions be made available, except for the ones that cannot possibly work with device-add? If the latter, can you think of any specific reason why the IDE controllers couldn't work? I would've thought you and Kevin know more about IDE than me. ;) No idea why it is how it is. Two aspects: 1) PCI devices/functions can technically be hotplugged. 2) Drivers might not expect such devices to be hot-added/removed. I would tend for the latter proposal. I'm not sure how IDE hardware really works, but I don't think you can handle it as a pure PCI device. On PCs, the IDE controller still provides the good old IDE registers on the I/O ports that were already used in ISA times, and they are not described in any BARs, for example. Legacy IDE and VGA accesses are positively decoded in the i440fx and dispatched to the first PCI device with the appropriate class code. From a software point of view, the PCI device is just for configuring Busmaster DMA. Perhaps in reality it should be two devices, one for DMA on the PCI bus and another one for the core on sysbus or ISA? It's definitely all a PCI device. It could realistically be a discrete device too that's plugged into a PCI bus that lacks a super I/O chipset. Anyway, having two IDE controllers using the same I/O ports won't work, obviously. So if you would allow -device or device-add for them, you'd need options to configure the ports at least. It's allowed but the PCI bus will only route the legacy requests to one of them. Any objections to dropping cannot_instantiate_with_device_add_yet from all IDE controller southbridge functions? These are: piix3-ide, piix3-ide-xen, piix4-ide and via-ide. No, I don't object. As Andreas points out, we may have problems today if you try to add two IDE controllers but those are bugs that should be fixed. The mission of cannot_instantiate_with_device_add_yet is protecting users from device-adds that cannot possibly work. So, *if* these devices actually have bugs that are so severe that device-add becomes effectively useless, then we should keep cannot_instantiate_with_device_add_yet for now. But that's detail. In my very limited testing, I wasn't able to get a system with two piix3-ide to boot, but it didn't look flat out impossible. See my reply to Andreas. It used to be possible to buy secondary PCI IDE controllers to add more than 4 disks. You could only access them with proper drivers (so you couldn't boot from the 5th disk) but otherwise it worked. Yes, but these IDE boards didn't identify as intel 82371SB. For what it's worth, we have an IDE controller device that isn't a southbridge function: cmd646-ide.
Re: [Qemu-devel] [PATCH v1 4/4] target-arm: Add CP15 VBAR support
On 10 July 2013 05:23, peter.crosthwa...@xilinx.com wrote: From: Nathan Rossi nathan.ro...@xilinx.com Added Vector Base Address remapping on ARM v7. Apologies for this dropping off my radar for so long. I've had a bit of a think and I think that you're right that we can put in this register as part of our random things we provide even though we aren't strictly implementing all of TZ, like the existing SCR register support. +static int vbar_write(CPUARMState *env, const ARMCPRegInfo *ri, + uint64_t value) +{ +value = (1 31); This seems spurious -- we end up ignoring all but the high bit of the written value. +env-cp15.c12_vbar = value ~0x1Ful; +return 0; +} + Otherwise looks OK. We should probably put in an ARM_FEATURE_TZ at some point, but for now putting it in the v7 regs with SCR is OK I think. -- PMM
[Qemu-devel] [PATCH 4/4] spice: fix multihead support
This patch fixes spice display initialization to handle multihead properly. spice-core now keeps track of which QemuConsole has a spice display channel attached to it and which has not. It also manages display channel ids. spice-display looks at all QemuConsoles and will pick up any graphic console not yet bound to a spice channel (which in practice are all non-qxl graphic devices). Result is that (a) you'll get a spice client window for each graphical device now (first only without this patch), and (b) mixing qxl and non-qxl vga cards works properly. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hw/display/qxl.c| 3 +-- include/ui/qemu-spice.h | 5 +++-- ui/spice-core.c | 24 ui/spice-display.c | 23 --- vl.c| 4 ++-- 5 files changed, 46 insertions(+), 13 deletions(-) diff --git a/hw/display/qxl.c b/hw/display/qxl.c index 074ed43..c2cea1c 100644 --- a/hw/display/qxl.c +++ b/hw/display/qxl.c @@ -2037,8 +2037,7 @@ static int qxl_init_common(PCIQXLDevice *qxl) qxl-vram32_size qxl-vram_size ? [region 4] : [unmapped]); qxl-ssd.qxl.base.sif = qxl_interface.base; -qxl-ssd.qxl.id = qxl-id; -if (qemu_spice_add_interface(qxl-ssd.qxl.base) != 0) { +if (qemu_spice_add_display_interface(qxl-ssd.qxl, qxl-vga.con) != 0) { error_report(qxl interface %d.%d not supported by spice-server, SPICE_INTERFACE_QXL_MAJOR, SPICE_INTERFACE_QXL_MINOR); return -1; diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h index c6c756b..86c75c7 100644 --- a/include/ui/qemu-spice.h +++ b/include/ui/qemu-spice.h @@ -27,14 +27,15 @@ #include monitor/monitor.h extern int using_spice; -extern int spice_displays; void qemu_spice_init(void); void qemu_spice_input_init(void); void qemu_spice_audio_init(void); -void qemu_spice_display_init(DisplayState *ds); +void qemu_spice_display_init(void); int qemu_spice_display_add_client(int csock, int skipauth, int tls); int qemu_spice_add_interface(SpiceBaseInstance *sin); +bool qemu_spice_have_display_interface(QemuConsole *con); +int qemu_spice_add_display_interface(QXLInstance *qxlin, QemuConsole *con); int qemu_spice_set_passwd(const char *passwd, bool fail_if_connected, bool disconnect_if_connected); int qemu_spice_set_pw_expire(time_t expires); diff --git a/ui/spice-core.c b/ui/spice-core.c index 1976b71..e4d533d 100644 --- a/ui/spice-core.c +++ b/ui/spice-core.c @@ -48,7 +48,6 @@ static char *auth_passwd; static time_t auth_expires = TIME_MAX; static int spice_migration_completed; int using_spice = 0; -int spice_displays; static QemuThread me; @@ -837,11 +836,28 @@ int qemu_spice_add_interface(SpiceBaseInstance *sin) qemu_add_vm_change_state_handler(vm_change_state_handler, NULL); } -if (strcmp(sin-sif-type, SPICE_INTERFACE_QXL) == 0) { -spice_displays++; +return spice_server_add_interface(spice_server, sin); +} + +static GSList *spice_consoles; +static int display_id; + +bool qemu_spice_have_display_interface(QemuConsole *con) +{ +if (g_slist_find(spice_consoles, con)) { +return true; } +return false; +} -return spice_server_add_interface(spice_server, sin); +int qemu_spice_add_display_interface(QXLInstance *qxlin, QemuConsole *con) +{ +if (g_slist_find(spice_consoles, con)) { +return -1; +} +qxlin-id = display_id++; +spice_consoles = g_slist_append(spice_consoles, con); +return qemu_spice_add_interface(qxlin-base); } static int qemu_spice_set_ticket(bool fail_if_conn, bool disconnect_if_conn) diff --git a/ui/spice-display.c b/ui/spice-display.c index c15c555..f23a318 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -612,21 +612,38 @@ static const DisplayChangeListenerOps display_listener_ops = { .dpy_refresh = display_refresh, }; -void qemu_spice_display_init(DisplayState *ds) +static void qemu_spice_display_init_one(QemuConsole *con) { SimpleSpiceDisplay *ssd = g_new0(SimpleSpiceDisplay, 1); qemu_spice_display_init_common(ssd); ssd-qxl.base.sif = dpy_interface.base; -qemu_spice_add_interface(ssd-qxl.base); +qemu_spice_add_display_interface(ssd-qxl, con); assert(ssd-worker); qemu_spice_create_host_memslot(ssd); ssd-dcl.ops = display_listener_ops; -ssd-dcl.con = qemu_console_lookup_by_index(0); +ssd-dcl.con = con; register_displaychangelistener(ssd-dcl); qemu_spice_create_host_primary(ssd); } + +void qemu_spice_display_init(void) +{ +QemuConsole *con; +int i; + +for (i = 0;; i++) { +con = qemu_console_lookup_by_index(i); +if (!con || !qemu_console_is_graphic(con)) { +break; +} +if (qemu_spice_have_display_interface(con)) { +continue; +} +qemu_spice_display_init_one(con); +} +} diff --git a/vl.c
[Qemu-devel] [PULL 0/4] spice patch queue
Hi, Here comes the spice patch queue with a small collection of fixes. cheers, Gerd The following changes since commit 1680d485777ecf436d724631ea8722cc0c66990e: Merge remote-tracking branch 'rth/tcg-ldst-6' into staging (2013-10-14 09:59:59 -0700) are available in the git repository at: git://anongit.freedesktop.org/spice/qemu spice.v75 for you to fetch changes up to 9fa032866daae68357d99abc725c18fe9ed4b61b: spice: fix multihead support (2013-10-17 12:42:54 +0200) Christophe Fergeau (1): Fix VNC SASL authentication when using a QXL device Gerd Hoffmann (2): spice-display: add display channel id to the debug messages. spice: fix multihead support Marc-André Lureau (1): spice: replace use of deprecated API hw/display/qxl.c| 19 include/ui/qemu-spice.h | 5 +++-- ui/spice-core.c | 40 +++-- ui/spice-display.c | 60 - vl.c| 4 ++-- 5 files changed, 81 insertions(+), 47 deletions(-)
[Qemu-devel] [PATCH 2/4] Fix VNC SASL authentication when using a QXL device
From: Christophe Fergeau cferg...@redhat.com ui/vnc.c:vnc_display_open() and spice-server/server/reds.c:do_spice_init() are both calling sasl_server_init(). If spice_server_set_sasl_appname() hasn't been called, spice-server will call it with spice as an appname, causing cyrus-sasl to try to use a /etc/sasl2/spice.conf config file rather than the /etc/sasl2/qemu.conf file that QEMU uses. When using -spice sasl on the command line, QEMU properly calls spice_server_set_sasl_appname() to set the SASL appname as qemu, but when using a QXL device without using SPICE, spice_server_init() is called from qemu_spice_add_interface() without setting the appname to qemu, which then causes the VNC code to try to use spice.conf instead of qemu.conf. Signed-off-by: Christophe Fergeau cferg...@redhat.com Signed-off-by: Gerd Hoffmann kra...@redhat.com --- ui/spice-core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ui/spice-core.c b/ui/spice-core.c index 79020a1..1976b71 100644 --- a/ui/spice-core.c +++ b/ui/spice-core.c @@ -832,6 +832,7 @@ int qemu_spice_add_interface(SpiceBaseInstance *sin) * With a command line like '-vnc :0 -vga qxl' you'll end up here. */ spice_server = spice_server_new(); +spice_server_set_sasl_appname(spice_server, qemu); spice_server_init(spice_server, core_interface); qemu_add_vm_change_state_handler(vm_change_state_handler, NULL); } -- 1.8.3.1
[Qemu-devel] [PATCH 1/4] spice: replace use of deprecated API
From: Marc-André Lureau marcandre.lur...@gmail.com hose API are deprecated since 0.11, and qemu depends on 0.12 already. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hw/display/qxl.c | 16 ui/spice-core.c| 15 +++ ui/spice-display.c | 10 +- 3 files changed, 20 insertions(+), 21 deletions(-) diff --git a/hw/display/qxl.c b/hw/display/qxl.c index ee2db0d..074ed43 100644 --- a/hw/display/qxl.c +++ b/hw/display/qxl.c @@ -162,7 +162,7 @@ void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t surface_id, trace_qxl_spice_update_area_rest(qxl-id, num_dirty_rects, clear_dirty_region); if (async == QXL_SYNC) { -qxl-ssd.worker-update_area(qxl-ssd.worker, surface_id, area, +spice_qxl_update_area(qxl-ssd.qxl, surface_id, area, dirty_rects, num_dirty_rects, clear_dirty_region); } else { assert(cookie != NULL); @@ -193,7 +193,7 @@ static void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id, cookie-u.surface_id = id; spice_qxl_destroy_surface_async(qxl-ssd.qxl, id, (uintptr_t)cookie); } else { -qxl-ssd.worker-destroy_surface_wait(qxl-ssd.worker, id); +spice_qxl_destroy_surface_wait(qxl-ssd.qxl, id); qxl_spice_destroy_surface_wait_complete(qxl, id); } } @@ -211,19 +211,19 @@ void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext, uint32_t count) { trace_qxl_spice_loadvm_commands(qxl-id, ext, count); -qxl-ssd.worker-loadvm_commands(qxl-ssd.worker, ext, count); +spice_qxl_loadvm_commands(qxl-ssd.qxl, ext, count); } void qxl_spice_oom(PCIQXLDevice *qxl) { trace_qxl_spice_oom(qxl-id); -qxl-ssd.worker-oom(qxl-ssd.worker); +spice_qxl_oom(qxl-ssd.qxl); } void qxl_spice_reset_memslots(PCIQXLDevice *qxl) { trace_qxl_spice_reset_memslots(qxl-id); -qxl-ssd.worker-reset_memslots(qxl-ssd.worker); +spice_qxl_reset_memslots(qxl-ssd.qxl); } static void qxl_spice_destroy_surfaces_complete(PCIQXLDevice *qxl) @@ -244,7 +244,7 @@ static void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl, qxl_async_io async) (uintptr_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO, QXL_IO_DESTROY_ALL_SURFACES_ASYNC)); } else { -qxl-ssd.worker-destroy_surfaces(qxl-ssd.worker); +spice_qxl_destroy_surfaces(qxl-ssd.qxl); qxl_spice_destroy_surfaces_complete(qxl); } } @@ -278,13 +278,13 @@ static void qxl_spice_monitors_config_async(PCIQXLDevice *qxl, int replay) void qxl_spice_reset_image_cache(PCIQXLDevice *qxl) { trace_qxl_spice_reset_image_cache(qxl-id); -qxl-ssd.worker-reset_image_cache(qxl-ssd.worker); +spice_qxl_reset_image_cache(qxl-ssd.qxl); } void qxl_spice_reset_cursor(PCIQXLDevice *qxl) { trace_qxl_spice_reset_cursor(qxl-id); -qxl-ssd.worker-reset_cursor(qxl-ssd.worker); +spice_qxl_reset_cursor(qxl-ssd.qxl); qemu_mutex_lock(qxl-track_lock); qxl-guest_cursor = 0; qemu_mutex_unlock(qxl-track_lock); diff --git a/ui/spice-core.c b/ui/spice-core.c index 33ef837..79020a1 100644 --- a/ui/spice-core.c +++ b/ui/spice-core.c @@ -383,17 +383,16 @@ static SpiceChannelList *qmp_query_spice_channels(void) struct sockaddr *paddr; socklen_t plen; +if (!(item-info-flags SPICE_CHANNEL_EVENT_FLAG_ADDR_EXT)) { +error_report(invalid channel event); +return NULL; +} + chan = g_malloc0(sizeof(*chan)); chan-value = g_malloc0(sizeof(*chan-value)); -if (item-info-flags SPICE_CHANNEL_EVENT_FLAG_ADDR_EXT) { -paddr = (struct sockaddr *)item-info-paddr_ext; -plen = item-info-plen_ext; -} else { -paddr = item-info-paddr; -plen = item-info-plen; -} - +paddr = (struct sockaddr *)item-info-paddr_ext; +plen = item-info-plen_ext; getnameinfo(paddr, plen, host, sizeof(host), port, sizeof(port), NI_NUMERICHOST | NI_NUMERICSERV); diff --git a/ui/spice-display.c b/ui/spice-display.c index 82d8b9f..0297373 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -83,14 +83,14 @@ void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot, (uintptr_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO, QXL_IO_MEMSLOT_ADD_ASYNC)); } else { -ssd-worker-add_memslot(ssd-worker, memslot); +spice_qxl_add_memslot(ssd-qxl, memslot); } } void qemu_spice_del_memslot(SimpleSpiceDisplay *ssd, uint32_t gid, uint32_t sid) { trace_qemu_spice_del_memslot(ssd-qxl.id, gid, sid); -ssd-worker-del_memslot(ssd-worker, gid, sid); +spice_qxl_del_memslot(ssd-qxl, gid, sid); } void qemu_spice_create_primary_surface(SimpleSpiceDisplay
[Qemu-devel] [PATCH 3/4] spice-display: add display channel id to the debug messages.
And s/__FUNCTION__/__func__/ while being at it. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- ui/spice-display.c | 27 ++- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/ui/spice-display.c b/ui/spice-display.c index 0297373..c15c555 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -297,7 +297,7 @@ void qemu_spice_create_host_memslot(SimpleSpiceDisplay *ssd) { QXLDevMemSlot memslot; -dprint(1, %s:\n, __FUNCTION__); +dprint(1, %s/%d:\n, __func__, ssd-qxl.id); memset(memslot, 0, sizeof(memslot)); memslot.slot_group_id = MEMSLOT_GROUP_HOST; @@ -311,7 +311,7 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd) memset(surface, 0, sizeof(surface)); -dprint(1, %s: %dx%d\n, __FUNCTION__, +dprint(1, %s/%d: %dx%d\n, __func__, ssd-qxl.id, surface_width(ssd-ds), surface_height(ssd-ds)); surface.format = SPICE_SURFACE_FMT_32_xRGB; @@ -329,7 +329,7 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd) void qemu_spice_destroy_host_primary(SimpleSpiceDisplay *ssd) { -dprint(1, %s:\n, __FUNCTION__); +dprint(1, %s/%d:\n, __func__, ssd-qxl.id); qemu_spice_destroy_primary_surface(ssd, 0, QXL_SYNC); } @@ -354,7 +354,8 @@ void qemu_spice_display_update(SimpleSpiceDisplay *ssd, { QXLRect update_area; -dprint(2, %s: x %d y %d w %d h %d\n, __FUNCTION__, x, y, w, h); +dprint(2, %s/%d: x %d y %d w %d h %d\n, __func__, + ssd-qxl.id, x, y, w, h); update_area.left = x, update_area.right = x + w; update_area.top = y; @@ -371,7 +372,7 @@ void qemu_spice_display_switch(SimpleSpiceDisplay *ssd, { SimpleSpiceUpdate *update; -dprint(1, %s:\n, __FUNCTION__); +dprint(1, %s/%d:\n, __func__, ssd-qxl.id); memset(ssd-dirty, 0, sizeof(ssd-dirty)); if (ssd-surface) { @@ -413,7 +414,7 @@ void qemu_spice_cursor_refresh_unlocked(SimpleSpiceDisplay *ssd) void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd) { -dprint(3, %s:\n, __func__); +dprint(3, %s/%d:\n, __func__, ssd-qxl.id); graphic_hw_update(ssd-dcl.con); qemu_mutex_lock(ssd-lock); @@ -427,7 +428,7 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd) if (ssd-notify) { ssd-notify = 0; qemu_spice_wakeup(ssd); -dprint(2, %s: notify\n, __FUNCTION__); +dprint(2, %s/%d: notify\n, __func__, ssd-qxl.id); } } @@ -437,19 +438,19 @@ static void interface_attach_worker(QXLInstance *sin, QXLWorker *qxl_worker) { SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl); -dprint(1, %s:\n, __FUNCTION__); +dprint(1, %s/%d:\n, __func__, ssd-qxl.id); ssd-worker = qxl_worker; } static void interface_set_compression_level(QXLInstance *sin, int level) { -dprint(1, %s:\n, __FUNCTION__); +dprint(1, %s/%d:\n, __func__, sin-id); /* nothing to do */ } static void interface_set_mm_time(QXLInstance *sin, uint32_t mm_time) { -dprint(3, %s:\n, __FUNCTION__); +dprint(3, %s/%d:\n, __func__, sin-id); /* nothing to do */ } @@ -472,7 +473,7 @@ static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext) SimpleSpiceUpdate *update; int ret = false; -dprint(3, %s:\n, __FUNCTION__); +dprint(3, %s/%d:\n, __func__, ssd-qxl.id); qemu_mutex_lock(ssd-lock); update = QTAILQ_FIRST(ssd-updates); @@ -488,7 +489,7 @@ static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext) static int interface_req_cmd_notification(QXLInstance *sin) { -dprint(1, %s:\n, __FUNCTION__); +dprint(1, %s/%d:\n, __func__, sin-id); return 1; } @@ -498,7 +499,7 @@ static void interface_release_resource(QXLInstance *sin, SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl); uintptr_t id; -dprint(2, %s:\n, __FUNCTION__); +dprint(2, %s/%d:\n, __func__, ssd-qxl.id); id = ext.info-id; qemu_spice_destroy_update(ssd, (void*)id); } -- 1.8.3.1
Re: [Qemu-devel] [PATCH] kvm-unittest: fix build with gcc 4.3.X and older
Il 16/10/2013 21:46, Michael S. Tsirkin ha scritto: Old GCC didn't let you reference variable by number if it is listed with a specific register constraint, on the assumption you can just use the register name explicitly. Tell us the truth, you made this up. :) Who doesn't do that for invalid asm error messages... Build fails with errors like this: a.c:6: error: invalid 'asm': invalid operand code 'd' To fix, let's just use %eax %al etc. The problem is that %d0 is wrong. The d modifier is for print duplicated register operand for AVX instruction (whatever that means) according to GCC source code (gcc/config/i386/i386.md). You need to use %k0 according to the same file. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- lib/x86/pci.c | 4 ++-- x86/s3.c | 4 ++-- x86/vmexit.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/x86/pci.c b/lib/x86/pci.c index f95cd88..08f7ebf 100644 --- a/lib/x86/pci.c +++ b/lib/x86/pci.c @@ -3,13 +3,13 @@ static void outl(unsigned short port, unsigned val) { -asm volatile(outl %d0, %w1 : : a(val), Nd(port)); +asm volatile(outl %%eax, %w1 : : a(val), Nd(port)); } static unsigned inl(unsigned short port) { unsigned data; -asm volatile(inl %w1, %d0 : =a(data) : Nd(port)); +asm volatile(inl %w1, %%eax : =a(data) : Nd(port)); return data; } static uint32_t pci_config_read(pcidevaddr_t dev, uint8_t reg) diff --git a/x86/s3.c b/x86/s3.c index 1feb452..eeffa17 100644 --- a/x86/s3.c +++ b/x86/s3.c @@ -149,8 +149,8 @@ static inline int rtc_in(u8 reg) static inline void rtc_out(u8 reg, u8 val) { -asm volatile(outb %b1, $0x70; mov %b2, %b1; outb %b1, $0x71 - : +a(reg) : 0(reg), ri(val)); +asm volatile(outb %%al, $0x70; mov %b1, %%al; outb %%al, $0x71 + : +a(reg) : ri(val)); Are you sure about this? The error message here is definitely not invalid operand code 'b'. That said, the patch is correct. It's only the commit message that is wrong. I'm not sure about Gleb's patch. I'll reply to his message. Paolo } extern char resume_start, resume_end; diff --git a/x86/vmexit.c b/x86/vmexit.c index 3b945de..29bc582 100644 --- a/x86/vmexit.c +++ b/x86/vmexit.c @@ -26,7 +26,7 @@ static void outw(unsigned short port, unsigned val) static void outl(unsigned short port, unsigned val) { -asm volatile(outl %d0, %w1 : : a(val), Nd(port)); +asm volatile(outl %%eax, %w1 : : a(val), Nd(port)); } static unsigned int inb(unsigned short port)
Re: [Qemu-devel] [PATCH] kvm-unittest: fix build with gcc 4.3.X and older
Il 17/10/2013 08:27, Gleb Natapov ha scritto: On Wed, Oct 16, 2013 at 10:46:53PM +0300, Michael S. Tsirkin wrote: Old GCC didn't let you reference variable by number if it is listed with a specific register constraint, on the assumption you can just use the register name explicitly. Build fails with errors like this: a.c:6: error: invalid 'asm': invalid operand code 'd' Is it worth to support such ancient compiler? Nobody complained till now. BTW with your patch I still cannot compile with 4.2: x86/s3.c: In function 'main': x86/s3.c:145: error: inconsistent operand constraints in an 'asm' To fix, let's just use %eax %al etc. Only %d0 does not work and dropping d fixes it since compiler can figure out correct register from variable size. The patch bellow fixes compilation for 4.2. diff --git a/lib/x86/pci.c b/lib/x86/pci.c index f95cd88..231668a 100644 --- a/lib/x86/pci.c +++ b/lib/x86/pci.c @@ -3,13 +3,13 @@ static void outl(unsigned short port, unsigned val) { -asm volatile(outl %d0, %w1 : : a(val), Nd(port)); +asm volatile(outl %0, %w1 : : a(val), Nd(port)); } static unsigned inl(unsigned short port) { unsigned data; -asm volatile(inl %w1, %d0 : =a(data) : Nd(port)); +asm volatile(inl %w1, %0 : =a(data) : Nd(port)); return data; } This is okay. static uint32_t pci_config_read(pcidevaddr_t dev, uint8_t reg) diff --git a/x86/s3.c b/x86/s3.c index 71d3ff9..d568aa7 100644 --- a/x86/s3.c +++ b/x86/s3.c @@ -143,14 +143,14 @@ static inline int rtc_in(u8 reg) { u8 x = reg; asm volatile(outb %b1, $0x70; inb $0x71, %b0 - : +a(x) : 0(x)); + : =a(x) : 0(x)); return x; } This should be wrong. GCC should complain that the same operand is used for both input and output but has an = constraint. static inline void rtc_out(u8 reg, u8 val) { asm volatile(outb %b1, $0x70; mov %b2, %b1; outb %b1, $0x71 - : +a(reg) : 0(reg), ri(val)); + : =a(reg) : 0(reg), ri(val)); } Same here. But I'm not sure what is the error message for older GCC for s3.c, as I wrote in reply to Michael. extern char resume_start, resume_end; diff --git a/x86/vmexit.c b/x86/vmexit.c index 3b945de..7e9af15 100644 --- a/x86/vmexit.c +++ b/x86/vmexit.c @@ -26,7 +26,7 @@ static void outw(unsigned short port, unsigned val) static void outl(unsigned short port, unsigned val) { -asm volatile(outl %d0, %w1 : : a(val), Nd(port)); +asm volatile(outl %0, %w1 : : a(val), Nd(port)); } Okay. Paolo static unsigned int inb(unsigned short port) -- Gleb.
Re: [Qemu-devel] [PATCH] kvm-unittest: fix build with gcc 4.3.X and older
On Thu, Oct 17, 2013 at 12:55:16PM +0200, Paolo Bonzini wrote: Il 17/10/2013 08:27, Gleb Natapov ha scritto: On Wed, Oct 16, 2013 at 10:46:53PM +0300, Michael S. Tsirkin wrote: Old GCC didn't let you reference variable by number if it is listed with a specific register constraint, on the assumption you can just use the register name explicitly. Build fails with errors like this: a.c:6: error: invalid 'asm': invalid operand code 'd' Is it worth to support such ancient compiler? Nobody complained till now. BTW with your patch I still cannot compile with 4.2: x86/s3.c: In function 'main': x86/s3.c:145: error: inconsistent operand constraints in an 'asm' To fix, let's just use %eax %al etc. Only %d0 does not work and dropping d fixes it since compiler can figure out correct register from variable size. The patch bellow fixes compilation for 4.2. diff --git a/lib/x86/pci.c b/lib/x86/pci.c index f95cd88..231668a 100644 --- a/lib/x86/pci.c +++ b/lib/x86/pci.c @@ -3,13 +3,13 @@ static void outl(unsigned short port, unsigned val) { -asm volatile(outl %d0, %w1 : : a(val), Nd(port)); +asm volatile(outl %0, %w1 : : a(val), Nd(port)); } static unsigned inl(unsigned short port) { unsigned data; -asm volatile(inl %w1, %d0 : =a(data) : Nd(port)); +asm volatile(inl %w1, %0 : =a(data) : Nd(port)); return data; } This is okay. static uint32_t pci_config_read(pcidevaddr_t dev, uint8_t reg) diff --git a/x86/s3.c b/x86/s3.c index 71d3ff9..d568aa7 100644 --- a/x86/s3.c +++ b/x86/s3.c @@ -143,14 +143,14 @@ static inline int rtc_in(u8 reg) { u8 x = reg; asm volatile(outb %b1, $0x70; inb $0x71, %b0 -: +a(x) : 0(x)); +: =a(x) : 0(x)); return x; } This should be wrong. GCC should complain that the same operand is used for both input and output but has an = constraint. static inline void rtc_out(u8 reg, u8 val) { asm volatile(outb %b1, $0x70; mov %b2, %b1; outb %b1, $0x71 -: +a(reg) : 0(reg), ri(val)); +: =a(reg) : 0(reg), ri(val)); } Same here. But I'm not sure what is the error message for older GCC for s3.c, as I wrote in reply to Michael. x86/s3.c: In function 'main': x86/s3.c:145: error: inconsistent operand constraints in an 'asm' And I am puzzled by this too. extern char resume_start, resume_end; diff --git a/x86/vmexit.c b/x86/vmexit.c index 3b945de..7e9af15 100644 --- a/x86/vmexit.c +++ b/x86/vmexit.c @@ -26,7 +26,7 @@ static void outw(unsigned short port, unsigned val) static void outl(unsigned short port, unsigned val) { -asm volatile(outl %d0, %w1 : : a(val), Nd(port)); +asm volatile(outl %0, %w1 : : a(val), Nd(port)); } Okay. Paolo static unsigned int inb(unsigned short port) -- Gleb. -- Gleb.
Re: [Qemu-devel] [PATCH] kvm-unittest: fix build with gcc 4.3.X and older
Il 17/10/2013 12:58, Gleb Natapov ha scritto: @@ -143,14 +143,14 @@ static inline int rtc_in(u8 reg) { u8 x = reg; asm volatile(outb %b1, $0x70; inb $0x71, %b0 -: +a(x) : 0(x)); +: =a(x) : 0(x)); return x; } This should be wrong. GCC should complain that the same operand is used for both input and output but has an = constraint. static inline void rtc_out(u8 reg, u8 val) { asm volatile(outb %b1, $0x70; mov %b2, %b1; outb %b1, $0x71 -: +a(reg) : 0(reg), ri(val)); +: =a(reg) : 0(reg), ri(val)); } Same here. But I'm not sure what is the error message for older GCC for s3.c, as I wrote in reply to Michael. x86/s3.c: In function 'main': x86/s3.c:145: error: inconsistent operand constraints in an 'asm' And I am puzzled by this too. Hmm, looks like my version is the incorrect one and, if you use +a you need not use the matching input constraint. So it's either your version, or one that removes the 0 altogether. Thus, Reviewed-by: Paolo Bonzini pbonz...@redhat.com Paolo
[Qemu-devel] [PATCH 0/2] vga: add secondary stdvga variant
Hi, This little series adds a legacy-free variant of the qemu standard vga, which can easily be used as secondary vga. No BIOS, thus no vesa support, thus you need guest drivers. If you wanna play with this walk over here: http://www.kraxel.org/cgit/linux/log/?h=qemu-drm The bochs drm driver handles both this new device and the qemu stdvga. Gerd Hoffmann (2): vga: allow non-global vmstate vga: add secondary stdvga variant docs/specs/standard-vga.txt | 13 ++--- hw/display/cirrus_vga.c | 4 +-- hw/display/qxl.c| 2 +- hw/display/vga-isa-mm.c | 2 +- hw/display/vga-isa.c| 2 +- hw/display/vga-pci.c| 64 - hw/display/vga.c| 8 -- hw/display/vga_int.h| 2 +- hw/display/vmware_vga.c | 2 +- 9 files changed, 86 insertions(+), 13 deletions(-) -- 1.8.3.1
[Qemu-devel] [PATCH 2/2] vga: add secondary stdvga variant
Add a standard vga variant which doesn't occupy any legacy ressources and thus can easily be used as secondary (or legacy-free) graphics adapter. Programming must be done using the MMIO bar. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- docs/specs/standard-vga.txt | 13 +++--- hw/display/vga-pci.c| 62 + hw/display/vga.c| 4 +++ 3 files changed, 76 insertions(+), 3 deletions(-) diff --git a/docs/specs/standard-vga.txt b/docs/specs/standard-vga.txt index 8a4c1e9..f82773e 100644 --- a/docs/specs/standard-vga.txt +++ b/docs/specs/standard-vga.txt @@ -5,9 +5,10 @@ QEMU Standard VGA Exists in two variants, for isa and pci. command line switches: --vga std[ picks isa for -M isapc, otherwise pci ] --device VGA [ pci variant ] --device isa-vga [ isa variant ] +-vga std [ picks isa for -M isapc, otherwise pci ] +-device VGA[ pci variant ] +-device isa-vga[ isa variant ] +-device secondary-vga [ legacy-free pci variant ] PCI spec @@ -31,9 +32,15 @@ PCI ROM Region: Holds the vgabios (qemu 0.14+). +The legacy-free variant has no ROM and has PCI_CLASS_DISPLAY_OTHER +instead of PCI_CLASS_DISPLAY_VGA. + + IO ports used - +Doesn't apply to the legacy-free pci variant, use the MMIO bar instead. + 03c0 - 03df : standard vga ports 01ce: bochs vbe interface index port 01cf: bochs vbe interface data port (x86 only) diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c index dee180f..38a0fca 100644 --- a/hw/display/vga-pci.c +++ b/hw/display/vga-pci.c @@ -179,12 +179,52 @@ static int pci_std_vga_initfn(PCIDevice *dev) return 0; } +static int pci_secondary_vga_initfn(PCIDevice *dev) +{ +PCIVGAState *d = DO_UPCAST(PCIVGAState, dev, dev); +VGACommonState *s = d-vga; + +/* vga + console init */ +vga_common_init(s, OBJECT(dev), false); +s-con = graphic_console_init(DEVICE(dev), s-hw_ops, s); + +/* mmio bar */ +memory_region_init(d-mmio, OBJECT(dev), vga.mmio, 4096); +memory_region_init_io(d-ioport, OBJECT(dev), pci_vga_ioport_ops, d, + vga ioports remapped, PCI_VGA_IOPORT_SIZE); +memory_region_init_io(d-bochs, OBJECT(dev), pci_vga_bochs_ops, d, + bochs dispi interface, PCI_VGA_BOCHS_SIZE); + +memory_region_add_subregion(d-mmio, PCI_VGA_IOPORT_OFFSET, +d-ioport); +memory_region_add_subregion(d-mmio, PCI_VGA_BOCHS_OFFSET, +d-bochs); + +pci_register_bar(d-dev, 0, (PCI_BASE_ADDRESS_MEM_PREFETCH | + PCI_BASE_ADDRESS_MEM_TYPE_64), s-vram); +pci_register_bar(d-dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, d-mmio); + +return 0; +} + +static void pci_secondary_vga_reset(DeviceState *dev) +{ +PCIVGAState *d = DO_UPCAST(PCIVGAState, dev.qdev, dev); + +vga_common_reset(d-vga); +} + static Property vga_pci_properties[] = { DEFINE_PROP_UINT32(vgamem_mb, PCIVGAState, vga.vram_size_mb, 16), DEFINE_PROP_BIT(mmio, PCIVGAState, flags, PCI_VGA_FLAG_ENABLE_MMIO, true), DEFINE_PROP_END_OF_LIST(), }; +static Property secondary_pci_properties[] = { +DEFINE_PROP_UINT32(vgamem_mb, PCIVGAState, vga.vram_size_mb, 16), +DEFINE_PROP_END_OF_LIST(), +}; + static void vga_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -201,6 +241,20 @@ static void vga_class_init(ObjectClass *klass, void *data) set_bit(DEVICE_CATEGORY_DISPLAY, dc-categories); } +static void secondary_class_init(ObjectClass *klass, void *data) +{ +DeviceClass *dc = DEVICE_CLASS(klass); +PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); + +k-init = pci_secondary_vga_initfn; +k-vendor_id = PCI_VENDOR_ID_QEMU; +k-device_id = PCI_DEVICE_ID_QEMU_VGA; +k-class_id = PCI_CLASS_DISPLAY_OTHER; +dc-vmsd = vmstate_vga_pci; +dc-props = secondary_pci_properties; +dc-reset = pci_secondary_vga_reset; +} + static const TypeInfo vga_info = { .name = VGA, .parent= TYPE_PCI_DEVICE, @@ -208,9 +262,17 @@ static const TypeInfo vga_info = { .class_init= vga_class_init, }; +static const TypeInfo secondary_info = { +.name = secondary-vga, +.parent= TYPE_PCI_DEVICE, +.instance_size = sizeof(PCIVGAState), +.class_init= secondary_class_init, +}; + static void vga_register_types(void) { type_register_static(vga_info); +type_register_static(secondary_info); } type_init(vga_register_types) diff --git a/hw/display/vga.c b/hw/display/vga.c index fea30e5..8eb4dc4 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -171,6 +171,10 @@ static void vga_update_memory_access(VGACommonState *s) MemoryRegion *region, *old_region = s-chain4_alias; hwaddr base, offset, size; +if
[Qemu-devel] [PATCH 1/2] vga: allow non-global vmstate
Need a way to opt-out from vga.vram being global vmstate, for secondary vga cards. Add a bool parameter to vga_common_init to support this. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hw/display/cirrus_vga.c | 4 ++-- hw/display/qxl.c| 2 +- hw/display/vga-isa-mm.c | 2 +- hw/display/vga-isa.c| 2 +- hw/display/vga-pci.c| 2 +- hw/display/vga.c| 4 ++-- hw/display/vga_int.h| 2 +- hw/display/vmware_vga.c | 2 +- 8 files changed, 10 insertions(+), 10 deletions(-) diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index dbd1f4a..3514761 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -2914,7 +2914,7 @@ static void isa_cirrus_vga_realizefn(DeviceState *dev, Error **errp) ISACirrusVGAState *d = ISA_CIRRUS_VGA(dev); VGACommonState *s = d-cirrus_vga.vga; -vga_common_init(s, OBJECT(dev)); +vga_common_init(s, OBJECT(dev), true); cirrus_init_common(d-cirrus_vga, OBJECT(dev), CIRRUS_ID_CLGD5430, 0, isa_address_space(isadev), isa_address_space_io(isadev)); @@ -2961,7 +2961,7 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev) int16_t device_id = pc-device_id; /* setup VGA */ - vga_common_init(s-vga, OBJECT(dev)); + vga_common_init(s-vga, OBJECT(dev), true); cirrus_init_common(s, OBJECT(dev), device_id, 1, pci_address_space(dev), pci_address_space_io(dev)); s-vga.con = graphic_console_init(DEVICE(dev), s-vga.hw_ops, s-vga); diff --git a/hw/display/qxl.c b/hw/display/qxl.c index ee2db0d..6ebf471 100644 --- a/hw/display/qxl.c +++ b/hw/display/qxl.c @@ -2069,7 +2069,7 @@ static int qxl_init_primary(PCIDevice *dev) qxl-id = 0; qxl_init_ramsize(qxl); vga-vram_size_mb = qxl-vga.vram_size 20; -vga_common_init(vga, OBJECT(dev)); +vga_common_init(vga, OBJECT(dev), true); vga_init(vga, OBJECT(dev), pci_address_space(dev), pci_address_space_io(dev), false); portio_list_init(qxl_vga_port_list, OBJECT(dev), qxl_vga_portio_list, diff --git a/hw/display/vga-isa-mm.c b/hw/display/vga-isa-mm.c index 8b514cc..cc5c582 100644 --- a/hw/display/vga-isa-mm.c +++ b/hw/display/vga-isa-mm.c @@ -132,7 +132,7 @@ int isa_vga_mm_init(hwaddr vram_base, s = g_malloc0(sizeof(*s)); s-vga.vram_size_mb = VGA_RAM_SIZE 20; -vga_common_init(s-vga, NULL); +vga_common_init(s-vga, NULL, true); vga_mm_init(s, vram_base, ctrl_base, it_shift, address_space); s-vga.con = graphic_console_init(NULL, s-vga.hw_ops, s); diff --git a/hw/display/vga-isa.c b/hw/display/vga-isa.c index c2a19ad..f5742ad 100644 --- a/hw/display/vga-isa.c +++ b/hw/display/vga-isa.c @@ -56,7 +56,7 @@ static void vga_isa_realizefn(DeviceState *dev, Error **errp) MemoryRegion *vga_io_memory; const MemoryRegionPortio *vga_ports, *vbe_ports; -vga_common_init(s, OBJECT(dev)); +vga_common_init(s, OBJECT(dev), true); s-legacy_address_space = isa_address_space(isadev); vga_io_memory = vga_init_io(s, OBJECT(dev), vga_ports, vbe_ports); isa_register_portio_list(isadev, 0x3b0, vga_ports, s, vga); diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c index b3a45c8..dee180f 100644 --- a/hw/display/vga-pci.c +++ b/hw/display/vga-pci.c @@ -147,7 +147,7 @@ static int pci_std_vga_initfn(PCIDevice *dev) VGACommonState *s = d-vga; /* vga + console init */ -vga_common_init(s, OBJECT(dev)); +vga_common_init(s, OBJECT(dev), true); vga_init(s, OBJECT(dev), pci_address_space(dev), pci_address_space_io(dev), true); diff --git a/hw/display/vga.c b/hw/display/vga.c index 7b91d9c..fea30e5 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -2257,7 +2257,7 @@ static const GraphicHwOps vga_ops = { .text_update = vga_update_text, }; -void vga_common_init(VGACommonState *s, Object *obj) +void vga_common_init(VGACommonState *s, Object *obj, bool global_vmstate) { int i, j, v, b; @@ -2294,7 +2294,7 @@ void vga_common_init(VGACommonState *s, Object *obj) s-is_vbe_vmstate = 1; memory_region_init_ram(s-vram, obj, vga.vram, s-vram_size); -vmstate_register_ram_global(s-vram); +vmstate_register_ram(s-vram, global_vmstate ? NULL : DEVICE(obj)); xen_register_framebuffer(s-vram); s-vram_ptr = memory_region_get_ram_ptr(s-vram); s-get_bpp = vga_get_bpp; diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h index e641890..d42ac92 100644 --- a/hw/display/vga_int.h +++ b/hw/display/vga_int.h @@ -177,7 +177,7 @@ static inline int c6_to_8(int v) return (v 2) | (b 1) | b; } -void vga_common_init(VGACommonState *s, Object *obj); +void vga_common_init(VGACommonState *s, Object *obj, bool global_vmstate); void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space, MemoryRegion *address_space_io, bool init_vga_ports); MemoryRegion *vga_init_io(VGACommonState *s, Object *obj, diff --git
Re: [Qemu-devel] [PULL 42/43] piix4: add acpi pci hotplug support
Il 17/10/2013 07:34, Michael S. Tsirkin ha scritto: So we could have a qtest for sanity checking ACPI tables. At least fw_cfg is one of the few components that has qtest infrastructure... I don't think we need to do more than that though. The set of sanity checks can start with a simple list of tables that have to be there for a given machine type. Well this means these tests won't pass on old qemu so they would be useless for comparing old to new. Yes, they would be correctness tests not compatibility tests. And in this case, while it's a good idea to have them I don't see why it's a blocker for merging. In principle, correctness tests should be a prerequisite for merging something. Would you merge RCU without an equivalent of rcutorture? We rarely if ever obey that principle, but we sometimes do (e.g. QAPI has pretty good test cases, and new additions to util/ almost always get new testcases these days). Paolo
[Qemu-devel] [PULL 0/1] e820: pass high memory too.
Hi, See patch description for all the details. Patch has been out for review for a while without objections. please pull, Gerd The following changes since commit 1680d485777ecf436d724631ea8722cc0c66990e: Merge remote-tracking branch 'rth/tcg-ldst-6' into staging (2013-10-14 09:59:59 -0700) are available in the git repository at: git://git.kraxel.org/qemu e820.1 for you to fetch changes up to 0624c7f916b4d97f17726d9b295d6a6b0dc5076d: e820: pass high memory too. (2013-10-17 13:06:11 +0200) Gerd Hoffmann (1): e820: pass high memory too. hw/i386/pc.c | 8 1 file changed, 8 insertions(+)
[Qemu-devel] [PATCH 1/1] e820: pass high memory too.
We have a fw_cfg entry to pass e820 entries from qemu to the firmware. Today it's used to pass reservations only. This patch makes qemu pass entries for RAM too. This allows to pass RAM sizes larger than 1TB to the firmware and it will also allow to pass non-contignous memory ramges should we decide to implement that some day, say for our virtual numa nodes. Obviously this needs some extra care to not break existing firware. SeaBIOS loads the entries and happily adds them without looking at the type. Which is problematic for memory below 4g as this will overwrite reservations added for bios memory etc. For memory above 4g it works just fine, seabios will merge the entry derived from cmos with the one loaded from fw_cfg. OVMF doesn't look at the fw_cfg e820 table. coreboot doesn't look at the fw_cfg e820 table. Cc: Andrea Arcangeli aarca...@redhat.com Signed-off-by: Gerd Hoffmann kra...@redhat.com Reviewed-By: Igor Mammedov imamm...@redhat.com --- hw/i386/pc.c | 8 1 file changed, 8 insertions(+) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 0c313fe..ec5508b 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1134,12 +1134,20 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory, memory_region_init_alias(ram_below_4g, NULL, ram-below-4g, ram, 0, below_4g_mem_size); memory_region_add_subregion(system_memory, 0, ram_below_4g); +if (0) { +/* + * Ideally we should do that too, but that would ruin the e820 + * reservations added by seabios before initializing fw_cfg. + */ +e820_add_entry(0, below_4g_mem_size, E820_RAM); +} if (above_4g_mem_size 0) { ram_above_4g = g_malloc(sizeof(*ram_above_4g)); memory_region_init_alias(ram_above_4g, NULL, ram-above-4g, ram, below_4g_mem_size, above_4g_mem_size); memory_region_add_subregion(system_memory, 0x1ULL, ram_above_4g); +e820_add_entry(0x1ULL, above_4g_mem_size, E820_RAM); } -- 1.8.3.1
[Qemu-devel] [PATCH] add firmware to machine options
Signed-off-by: Gerd Hoffmann kra...@redhat.com --- vl.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/vl.c b/vl.c index 7e1f408..ef450ee 100644 --- a/vl.c +++ b/vl.c @@ -428,6 +428,10 @@ static QemuOptsList qemu_machine_opts = { .name = usb, .type = QEMU_OPT_BOOL, .help = Set on/off to enable/disable usb, +},{ +.name = firmware, +.type = QEMU_OPT_STRING, +.help = firmware image, }, { /* End of list */ } }, @@ -3228,7 +3232,7 @@ int main(int argc, char **argv, char **envp) } break; case QEMU_OPTION_bios: -bios_name = optarg; +qemu_opts_set(qemu_find_opts(machine), 0, firmware, optarg); break; case QEMU_OPTION_singlestep: singlestep = 1; @@ -4049,6 +4053,7 @@ int main(int argc, char **argv, char **envp) kernel_filename = qemu_opt_get(machine_opts, kernel); initrd_filename = qemu_opt_get(machine_opts, initrd); kernel_cmdline = qemu_opt_get(machine_opts, append); +bios_name = qemu_opt_get(machine_opts, firmware); boot_order = machine-default_boot_order; opts = qemu_opts_find(qemu_find_opts(boot-opts), NULL); -- 1.8.3.1
Re: [Qemu-devel] [PATCH v7 4/4] PC: differentiate hpet's interrupt capability on piix and q35
Il 17/10/2013 05:16, Liu Ping Fan ha scritto: + bool hpet_irqs) { int i; DriveInfo *fd[MAX_FD]; @@ -1249,10 +1250,19 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, /* In order to set property, here not using sysbus_try_create_simple */ hpet = qdev_try_create(NULL, hpet); if (hpet) { -/* tmp fix. For compat, hard code to IRQ2 until we have correct - * compat property and differentiate pc-iix with pc-q35 - */ -qdev_prop_set_uint32(hpet, HPET_INTCAP, 0x4); +/* For pc-piix-*, hpet's intcap is always IRQ2. */ +if (!hpet_irqs) { +qdev_prop_set_uint32(hpet, HPET_INTCAP, 0x4); +} else { +/* For pc-q35-1.7 and earlier, use IRQ2 for compat. + * Otherwise, use IRQ16~23, IRQ8 and IRQ2. + */ +uint8_t compat = object_property_get_int(OBJECT(hpet), +HPET_INTCAP, NULL); +if (!compat) { +qdev_prop_set_uint32(hpet, HPET_INTCAP, 0xff0104); +} +} Simpler: if (hpet) { uint8_t compat = object_property_get_int(OBJECT(hpet), HPET_INTCAP, NULL); if (!compat) { qdev_prop_set_uint32(hpet, HPET_INTCAP, 0xff0104); } } and just pass 0x4 or 0xff0104 to the function. I think the unused property is okay. It is a matter of fact that before these patches intcap was hardcoded to 0x4. Who cares if it remains 0x4 in some cases. Note that when this will go into 1.8 you will need to add the 1.8 machines. Paolo
Re: [Qemu-devel] [PATCH v7 4/4] PC: differentiate hpet's interrupt capability on piix and q35
Il 17/10/2013 05:16, Liu Ping Fan ha scritto: + bool hpet_irqs) { int i; DriveInfo *fd[MAX_FD]; @@ -1249,10 +1250,19 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, /* In order to set property, here not using sysbus_try_create_simple */ hpet = qdev_try_create(NULL, hpet); if (hpet) { -/* tmp fix. For compat, hard code to IRQ2 until we have correct - * compat property and differentiate pc-iix with pc-q35 - */ -qdev_prop_set_uint32(hpet, HPET_INTCAP, 0x4); +/* For pc-piix-*, hpet's intcap is always IRQ2. */ +if (!hpet_irqs) { +qdev_prop_set_uint32(hpet, HPET_INTCAP, 0x4); +} else { +/* For pc-q35-1.7 and earlier, use IRQ2 for compat. + * Otherwise, use IRQ16~23, IRQ8 and IRQ2. + */ +uint8_t compat = object_property_get_int(OBJECT(hpet), +HPET_INTCAP, NULL); +if (!compat) { +qdev_prop_set_uint32(hpet, HPET_INTCAP, 0xff0104); +} +} Simpler: if (hpet) { uint8_t compat = object_property_get_int(OBJECT(hpet), HPET_INTCAP, NULL); if (!compat) { qdev_prop_set_uint32(hpet, HPET_INTCAP, hpet_irqs); } } and just pass 0x4 or 0xff0104 to the function. I think the unused property is okay. It is a matter of fact that before these patches intcap was hardcoded to 0x4. Who cares if it remains 0x4 in some cases. Note that when this will go into 1.8 you will need to add the 1.8 machines. Paolo
[Qemu-devel] Qemu VGA Driver With XP Guest
Hi All, When I user '-vga std' to run winxp Guest. The driver in the guest seems not normal . The vendor, display memory size can't be read by the operation system. In the hardware device management, the vga device's status is not normal. Is this a problem of qemu's std vga device implement? Or is there any method to resolve the problem?Any hint or advice is welcome. I apologize if I am missing something very obvious. Best regards to you. Ricky
Re: [Qemu-devel] [PATCH v3 1/4] Curling: add doc
On Tue, Oct 15, 2013 at 03:26:20PM +0800, Jules Wang wrote: +Usage += +The steps of curling are the same as the steps of live migration except the +following: +1. Start ft in the qemu monitor of sender vm by following cmdline: +migrate_set_speed full bandwidth +migrate -f tcp:address:port +2. Connect to the receiver vm by vnc or spice. The screen of the vm is displayed +when ft is ready. Management tools (like libvirt) need a QMP event that reports when FT is active. This allows users to check the FT status of a guest and understand when the guest is protected. Stefan
Re: [Qemu-devel] [PATCH] kvm-unittest: fix build with gcc 4.3.X and older
On Thu, Oct 17, 2013 at 12:53:30PM +0200, Paolo Bonzini wrote: Il 16/10/2013 21:46, Michael S. Tsirkin ha scritto: Old GCC didn't let you reference variable by number if it is listed with a specific register constraint, on the assumption you can just use the register name explicitly. Tell us the truth, you made this up. :) Who doesn't do that for invalid asm error messages... I guessed at the reason, yes. Build fails with errors like this: a.c:6: error: invalid 'asm': invalid operand code 'd' To fix, let's just use %eax %al etc. The problem is that %d0 is wrong. The d modifier is for print duplicated register operand for AVX instruction (whatever that means) according to GCC source code (gcc/config/i386/i386.md). You need to use %k0 according to the same file. Aha! Signed-off-by: Michael S. Tsirkin m...@redhat.com --- lib/x86/pci.c | 4 ++-- x86/s3.c | 4 ++-- x86/vmexit.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/x86/pci.c b/lib/x86/pci.c index f95cd88..08f7ebf 100644 --- a/lib/x86/pci.c +++ b/lib/x86/pci.c @@ -3,13 +3,13 @@ static void outl(unsigned short port, unsigned val) { -asm volatile(outl %d0, %w1 : : a(val), Nd(port)); +asm volatile(outl %%eax, %w1 : : a(val), Nd(port)); } static unsigned inl(unsigned short port) { unsigned data; -asm volatile(inl %w1, %d0 : =a(data) : Nd(port)); +asm volatile(inl %w1, %%eax : =a(data) : Nd(port)); return data; } static uint32_t pci_config_read(pcidevaddr_t dev, uint8_t reg) diff --git a/x86/s3.c b/x86/s3.c index 1feb452..eeffa17 100644 --- a/x86/s3.c +++ b/x86/s3.c @@ -149,8 +149,8 @@ static inline int rtc_in(u8 reg) static inline void rtc_out(u8 reg, u8 val) { -asm volatile(outb %b1, $0x70; mov %b2, %b1; outb %b1, $0x71 -: +a(reg) : 0(reg), ri(val)); +asm volatile(outb %%al, $0x70; mov %b1, %%al; outb %%al, $0x71 +: +a(reg) : ri(val)); Are you sure about this? The error message here is definitely not invalid operand code 'b'. This produced a different error about conflict in constrains I think, Gleb saw it too. That said, the patch is correct. It's only the commit message that is wrong. I'm not sure about Gleb's patch. I'll reply to his message. Paolo } extern char resume_start, resume_end; diff --git a/x86/vmexit.c b/x86/vmexit.c index 3b945de..29bc582 100644 --- a/x86/vmexit.c +++ b/x86/vmexit.c @@ -26,7 +26,7 @@ static void outw(unsigned short port, unsigned val) static void outl(unsigned short port, unsigned val) { -asm volatile(outl %d0, %w1 : : a(val), Nd(port)); +asm volatile(outl %%eax, %w1 : : a(val), Nd(port)); } static unsigned int inb(unsigned short port)
Re: [Qemu-devel] [PATCH] kvm-unittest: fix build with gcc 4.3.X and older
On Thu, Oct 17, 2013 at 01:02:17PM +0200, Paolo Bonzini wrote: Il 17/10/2013 12:58, Gleb Natapov ha scritto: @@ -143,14 +143,14 @@ static inline int rtc_in(u8 reg) { u8 x = reg; asm volatile(outb %b1, $0x70; inb $0x71, %b0 - : +a(x) : 0(x)); + : =a(x) : 0(x)); return x; } This should be wrong. GCC should complain that the same operand is used for both input and output but has an = constraint. static inline void rtc_out(u8 reg, u8 val) { asm volatile(outb %b1, $0x70; mov %b2, %b1; outb %b1, $0x71 - : +a(reg) : 0(reg), ri(val)); + : =a(reg) : 0(reg), ri(val)); } Same here. But I'm not sure what is the error message for older GCC for s3.c, as I wrote in reply to Michael. x86/s3.c: In function 'main': x86/s3.c:145: error: inconsistent operand constraints in an 'asm' And I am puzzled by this too. Hmm, looks like my version is the incorrect one and, if you use +a you need not use the matching input constraint. So it's either your version, or one that removes the 0 altogether. Thus, Reviewed-by: Paolo Bonzini pbonz...@redhat.com Yea, you can add Reviewed-by: Michael S. Tsirkin m...@redhat.com too. But maybe we can add %k constraints like Paolo said? Being extra careful ... Paolo
Re: [Qemu-devel] [PATCH v2 1/6] osdep: Create qemu_getauxval and qemu_init_auxval
On 10 September 2013 23:07, Richard Henderson r...@twiddle.net wrote: Abstract away dependence on a system implementation of getauxval. Signed-off-by: Richard Henderson r...@twiddle.net There are unfortunately some trivial conflicts now so this series needs a respin :-( --- include/qemu/osdep.h | 20 linux-user/main.c| 1 + util/Makefile.objs | 1 + util/getauxval.c | 91 vl.c | 1 + 5 files changed, 114 insertions(+) create mode 100644 util/getauxval.c diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index 26136f1..8d1948b 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -215,4 +215,24 @@ bool fips_get_state(void); */ char *qemu_get_local_state_pathname(const char *relative_pathname); +/* Return a value for an AT_* type in the auxiliary vector. Return 0 if the + * key is not present. We attempt to emulate AT_HWCAP and AT_PLATFORM as + * best we can when real host support is not present. + */ Would be nice for these to be in the proper doc-comment format. +#ifdef _WIN32 +static inline unsigned long qemu_getauxval(unsigned long type) { return 0; } +#else +unsigned long qemu_getauxval(unsigned long type); +#endif + +/* If supported and required, locate the auxiliary vector at program startup. + * + * @envp must be the third argument to main. + */ +#if defined(CONFIG_GETAUXVAL) || defined(_WIN32) +static inline void qemu_init_auxval(char **envp) { } +#else +void qemu_init_auxval(char **envp); +#endif + #endif diff --git a/linux-user/main.c b/linux-user/main.c index 5c2f7b2..ebd9cf4 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -3553,6 +3553,7 @@ int main(int argc, char **argv, char **envp) module_call_init(MODULE_INIT_QOM); +qemu_init_auxval(envp); qemu_cache_utils_init(envp); if ((envlist = envlist_create()) == NULL) { diff --git a/util/Makefile.objs b/util/Makefile.objs index dc72ab0..8e0c929 100644 --- a/util/Makefile.objs +++ b/util/Makefile.objs @@ -11,3 +11,4 @@ util-obj-y += iov.o aes.o qemu-config.o qemu-sockets.o uri.o notify.o util-obj-y += qemu-option.o qemu-progress.o util-obj-y += hexdump.o util-obj-y += crc32c.o +util-obj-y += getauxval.o diff --git a/util/getauxval.c b/util/getauxval.c new file mode 100644 index 000..59f53ff --- /dev/null +++ b/util/getauxval.c @@ -0,0 +1,91 @@ +/* + * QEMU access to the auxiliary vector + * + * Copyright (C) 2013 Red Hat, Inc + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the Software), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include qemu-common.h +#include qemu/osdep.h + +#ifdef CONFIG_GETAUXVAL +/* Don't inline this in qemu/osdep.h, because pulling in sys/auxv.h for + the system declaration of getauxval pulls in the system elf.h, which + conflicts with qemu's version. */ Ugh, but that's a cleanup for another day I guess. + +#include sys/auxv.h + +unsigned long qemu_getauxval(unsigned long key) +{ +return getauxval(key); +} +#elif !defined(_WIN32) +#include elf.h + +/* Our elf.h doesn't contain Elf32_auxv_t and Elf64_auxv_t, which is ok because + that just makes it easier to define it properly for the host here. */ +typedef struct { +unsigned long a_type; +unsigned long a_val; +} ElfW_auxv_t; + +static const ElfW_auxv_t *auxv; + +void qemu_init_auxval(char **envp) +{ +#ifdef __linux__ +/* The auxiliary vector is located just beyond the initial environment. */ +while (*envp++ != NULL) +continue; Braces. +auxv = (const ElfW_auxv_t *)envp; +#endif +} + +static const char *default_platform(void) +{ +return NULL; +} + +static unsigned long default_hwcap(void) +{ +return 0; +} + +unsigned long qemu_getauxval(unsigned long type) +{ +/* If we were able to find the auxiliary vector, use it. */ +if (auxv) { +
Re: [Qemu-devel] [PATCH v3 0/4] Curling: KVM Fault Tolerance
On Tue, Oct 15, 2013 at 03:26:19PM +0800, Jules Wang wrote: v2 - v3: * add documentation of new option in qapi-schema. * long option name: ft - fault-tolerant v1 - v2: * cmdline: migrate curling:tcp:address:port - migrate -f tcp:address:port * sender: use QEMU_VM_FILE_MAGIC_FT as the header of the migration to indicate this is a ft migration. * receiver: look for the signature: QEMU_VM_EOF_MAGIC + QEMU_VM_FILE_MAGIC_FT(64bit total) which indicates the end of one migration. -- Jules Wang (4): Curling: add doc Curling: cmdline interface. Curling: the sender Curling: the receiver It would be helpful to clarify the status of Curling in the cover letter email so reviewers know what to expect. This series does not address I/O or failover. I guess you are aware of the missing topics that I mentioned, here are my thoughts on them: I/O needs to be held back until the destination host has acknowledged receiving the last full migration state. The outside world cannot witness state changes in the guest until the migration state has been successfully transferred to the destination host. Otherwise the guest may appear to act incorrectly when resuming execution from the last snapshot. The time period used by the FT sender thread determines how much latency is added to I/O requests. Failover functionality is missing from these patches. We cannot simply start executing on the destination host when the migration connection ends. If the guest disk image is located on shared storage then split-brain occurs when a network error terminates the migration connection - will both hosts begin accessing the shared disk? What is your plan to address these issues? Stefan
Re: [Qemu-devel] [PATCH v2 2/6] tcg-ppc64: Use qemu_getauxval
On 10 September 2013 23:07, Richard Henderson r...@twiddle.net wrote: Allow host detection on linux systems without glibc 2.16 or later. Signed-off-by: Richard Henderson r...@twiddle.net Reviewed-by: Peter Maydell peter.mayd...@linaro.org -- PMM
Re: [Qemu-devel] [PATCH v2 3/6] tcg-arm: Use qemu_getauxval
On 10 September 2013 23:07, Richard Henderson r...@twiddle.net wrote: Allow host detection on linux systems without glibc 2.16 or later. Signed-off-by: Richard Henderson r...@twiddle.net --- include/elf.h| 22 ++ tcg/arm/tcg-target.c | 15 ++- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/include/elf.h b/include/elf.h index 7fdd3df..e95fa95 100644 --- a/include/elf.h +++ b/include/elf.h @@ -411,6 +411,28 @@ typedef struct { #define R_SPARC_5 44 #define R_SPARC_6 45 +/* Bits present in AT_HWCAP for ARM. */ + +#define HWCAP_ARM_SWP 1 +#define HWCAP_ARM_HALF 2 Don't use hardcoded tabs, please. +#define HWCAP_ARM_THUMB4 +#define HWCAP_ARM_26BIT8 +#define HWCAP_ARM_FAST_MULT16 +#define HWCAP_ARM_FPA 32 +#define HWCAP_ARM_VFP 64 +#define HWCAP_ARM_EDSP 128 +#define HWCAP_ARM_JAVA 256 +#define HWCAP_ARM_IWMMXT 512 +#define HWCAP_ARM_CRUNCH 1024 +#define HWCAP_ARM_THUMBEE 2048 +#define HWCAP_ARM_NEON 4096 +#define HWCAP_ARM_VFPv38192 +#define HWCAP_ARM_VFPv3D16 16384 +#define HWCAP_ARM_TLS 32768 +#define HWCAP_ARM_VFPv465536 +#define HWCAP_ARM_IDIVA131072 +#define HWCAP_ARM_IDIVT262144 + /* Bits present in AT_HWCAP for PowerPC. */ #define PPC_FEATURE_32 0x8000 diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c index eb0e84c..77af1f2 100644 --- a/tcg/arm/tcg-target.c +++ b/tcg/arm/tcg-target.c @@ -22,6 +22,8 @@ * THE SOFTWARE. */ +#include elf.h + /* The __ARM_ARCH define is provided by gcc 4.8. Construct it otherwise. */ #ifndef __ARM_ARCH # if defined(__ARM_ARCH_7__) || defined(__ARM_ARCH_7A__) \ @@ -56,9 +58,6 @@ static int arm_arch = __ARM_ARCH; #ifndef use_idiv_instructions bool use_idiv_instructions; #endif -#ifdef CONFIG_GETAUXVAL -# include sys/auxv.h -#endif #ifndef NDEBUG static const char * const tcg_target_reg_names[TCG_TARGET_NB_REGS] = { @@ -2028,22 +2027,20 @@ static const TCGTargetOpDef arm_op_defs[] = { static void tcg_target_init(TCGContext *s) { -#if defined(CONFIG_GETAUXVAL) /* Only probe for the platform and capabilities if we havn't already determined maximum values at compile time. */ -# if !defined(use_idiv_instructions) +#ifndef use_idiv_instructions { -unsigned long hwcap = getauxval(AT_HWCAP); +unsigned long hwcap = qemu_getauxval(AT_HWCAP); use_idiv_instructions = (hwcap HWCAP_ARM_IDIVA) != 0; } -# endif +#endif if (__ARM_ARCH 7) { -const char *pl = (const char *)getauxval(AT_PLATFORM); +const char *pl = (const char *)qemu_getauxval(AT_PLATFORM); if (pl != NULL pl[0] == 'v' pl[1] = '4' pl[1] = '9') { arm_arch = pl[1] - '0'; } } -#endif /* GETAUXVAL */ tcg_regset_set32(tcg_target_available_regs[TCG_TYPE_I32], 0, 0x); tcg_regset_set32(tcg_target_call_clobber_regs, 0, -- 1.8.1.4 Otherwise Reviewed-by: Peter Maydell peter.mayd...@linaro.org -- PMM
[Qemu-devel] [PATCH] Fix another corner-case of using VNC+SASL+SPICE
Similarly to the previous commit fixing VNC+SASL+QXL, when starting QEMU with SPICE but no SASL, and also VNC with SASL, then spice_server_init() will get called without a previous call to spice_server_set_sasl_appname(), which will cause cyrus-sasl to try to use /etc/sasl2/spice.conf (spice-server uses spice as its default appname) rather than the expected /etc/sasl2/qemu.conf. This commit unconditionnally calls spice_server_set_sasl_appname() before calling spice_server_init() in order to use the correct appname even if SPICE without SASL was requested on qemu command line. --- ui/spice-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/spice-core.c b/ui/spice-core.c index d7566b0..b8af63b 100644 --- a/ui/spice-core.c +++ b/ui/spice-core.c @@ -728,8 +728,7 @@ void qemu_spice_init(void) spice_server_set_ticket(spice_server, password, 0, 0, 0); } if (qemu_opt_get_bool(opts, sasl, 0)) { -if (spice_server_set_sasl_appname(spice_server, qemu) == -1 || -spice_server_set_sasl(spice_server, 1) == -1) { +if (spice_server_set_sasl(spice_server, 1) == -1) { error_report(spice: failed to enable sasl); exit(1); } @@ -792,6 +791,7 @@ void qemu_spice_init(void) seamless_migration = qemu_opt_get_bool(opts, seamless-migration, 0); spice_server_set_seamless_migration(spice_server, seamless_migration); +spice_server_set_sasl_appname(spice_server, qemu); if (0 != spice_server_init(spice_server, core_interface)) { error_report(failed to initialize spice server); exit(1); -- 1.8.3.1
Re: [Qemu-devel] [PATCH v2 5/6] util: Provide fallback hwcap and platform for powerpc
On 10 September 2013 23:07, Richard Henderson r...@twiddle.net wrote: Allow host detection on non-linux hosts. Signed-off-by: Richard Henderson r...@twiddle.net --- util/getauxval.c | 56 ++-- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/util/getauxval.c b/util/getauxval.c index 59f53ff..e7f7d63 100644 --- a/util/getauxval.c +++ b/util/getauxval.c @@ -60,12 +60,64 @@ void qemu_init_auxval(char **envp) static const char *default_platform(void) { -return NULL; +const char *ret = NULL; + +#ifdef _ARCH_PPC +# if defined(_ARCH_PWR8) +ret = power8; +# elif defined(_ARCH_PWR7) All these arch-specific ifdefs seem out of place in a generic util/ source file. -- PMM
Re: [Qemu-devel] [PATCH v2 6/6] util: Use qemu_getauxval in linux qemu_cache_utils_init
On 10 September 2013 23:07, Richard Henderson r...@twiddle.net wrote: With this we no longer pass down envp, and thus all systems can have the same void prototype. So also eliminate a useless thunk. Signed-off-by: Richard Henderson r...@twiddle.net Reviewed-by: Peter Maydell peter.mayd...@linaro.org -- PMM
Re: [Qemu-devel] [PATCH 00/60] AArch64 TCG emulation support
On 16.10.2013, at 21:54, Edgar E. Iglesias edgar.igles...@gmail.com wrote: On Fri, Sep 27, 2013 at 02:47:54AM +0200, Alexander Graf wrote: Howdy, This is the first batch of patches to implement AArch64 instruction emulation in QEMU. It implements enough to execute simple AArch64 programs in linux-user mode. We still have quite a big number of patches outstanding that will come after this initial set, both in linux-user code as well as in the AArch64 instruction emulator. But this series is already quite big, so let's get this one through first. Impressive work Alex! It would be fun to try this out, do you have a public repo with these patches? I even have a repo with a not as clean, but fully working backend: https://github.com/openSUSE/qemu/commits/aarch64-work How much progress have you made on system emulation? None :). So far the target was user emulation. When I get a bit of spare time on my hands again I'll take a stab at system emulation too, but so far I haven't gotten around to it. In fact, doing v2 is definitely higher on my todo list than system emulation ;). Alex
Re: [Qemu-devel] [PATCH v2] net/net: Change the default mac address of nic
On Tue, Oct 15, 2013 at 09:33:06PM +0800, mike wrote: On 10/15/2013 08:36 PM, Eric Blake wrote: On 10/14/2013 11:07 PM, Stefan Weil wrote: Is it reasonable to get a random mac address in your guest? I don't think so. It would no longer be possible to connect to a guest using ssh, restart that guest and connect again with ssh. Agreed - libvirt ALWAYS passes a MAC to qemu, even if the user did not specify a MAC to libvirt, precisely because the MAC must be reproducible rather than random to avoid changing the guest ABI. I don't think this patch is needed - it's up to management to use qemu correctly. Yes, you are right in this condition. But qemu support Mac address unset. Also we can get the ip address through a lot of different ways, like use monitor to get the mac and then get the ip. So we can login use ssh. But as you mentioned, this patch is not needed, I don't agree with you. First, this patch just fix the Potential issue of this feature. Now libvirt maybe can't triggered this issue, who can promise in future will not. The second is, qemu not only be used by libvirt, lots of developers like to use the command line to boot up the guest. And in the future, we are not sure about other program will use qemu. The third is, when one feature has a issue in qemu, no matter when it is been triggered, should we not fix it? NACK I'm not going to merge this patch: If you terminate QEMU and launch it again the NIC gets a different MAC address. Some guest operating systems are sensitive to this - under many Linux distros the network interfaces names change due to the MAC address change. As a result firewall configuration will break and other services may fail to start because they cannot find the interface. If you have multiple guests or want control over the MAC address, set it explicitly using -device nic-model,mac=XX:XX:XX:XX:XX:XX. Stefan
Re: [Qemu-devel] [PATCH 1/2] sun4m: Add Sun CG3 framebuffer and corresponding OpenBIOS FCode ROM
On 15/10/13 22:46, Peter Maydell wrote: On 15 October 2013 21:19, Mark Cave-Ayland mark.cave-ayl...@ilande.co.uk wrote: +/* FCode ROM */ +memory_region_init_ram(s-rom, NULL, cg3.prom, FCODE_MAX_ROM_SIZE); +vmstate_register_ram_global(s-rom); +memory_region_set_readonly(s-rom, true); +sysbus_init_mmio(dev,s-rom); + +fcode_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, CG3_ROM_FILE); +if (fcode_filename) { +ret = load_image_targphys(fcode_filename, s-prom_addr, + FCODE_MAX_ROM_SIZE); +} Ideally we would have a 'load image into RAM memory region' function, and then we wouldn't need to pass the device the address of its own memory region. Oh well... Yes, definitely the ROM image code is lacking from post-MemoryRegion love. For example load_image_targphys() indirectly creates a MemoryRegion to hold the loaded ROM contents, but it only becomes visible (presumably because of SysBus) when another RAM MemoryRegion is installed at the same address. Hence you actually end up having to claim twice the memory requirement of your original ROM. ATB, Mark.
Re: [Qemu-devel] [PATCH v3 0/2] target-ppc: Tidy sPAPR device tree CPU nodes
On 15.10.2013, at 18:33, Andreas Färber afaer...@suse.de wrote: Hello Alexey and Alex, This series cleans up the fdt CPU nodes for -M pseries as attempted by Prerna. v3 uses DeviceClass::fw_name for name storage exclusively, with PowerPC,UNKNOWN as fallback. Thanks, applied all to ppc-next. Alex
Re: [Qemu-devel] virtio-blk-pci: how to tell if it is CD or HDD?
On 10/16/2013 06:59 PM, Alexey Kardashevskiy wrote: On 10/16/2013 05:36 PM, Paolo Bonzini wrote: Il 16/10/2013 07:04, Alexey Kardashevskiy ha scritto: Hi! Normally on sPAPR platform the IBMVSCSI host bus adapter is used which is SCSI. So when we want some image to appear as a DVD to the guest (particularly SLOF - our firmware), we use -device scsi-cd. Or QEMU extracts this automatically from the media=cdrom property of -drive (correct?). And then the SCSI bus tells the guest what is what. SLOF firmware uses this to create disk and cdrom aliases to correcly apply the boot order. So far so good. Now we are trying (via libvirt) to add both HDD and DVD as virtio-blk-pci devices: -drive file=virtimg/rhel6-root.img,if=none,format=raw,\ id=drive-virtio-disk1,cache=none \ -device virtio-blk-pci,bus=pci,addr=0x4,\ drive=drive-virtio-disk1,id=virtio-disk1,bootindex=2 \ \ -drive file=virtimg/SLES-11-SP3-DVD-ppc64-GM-DVD1.iso,\ if=none,media=cdrom,id=drive-virtio-disk2,readonly=on,\ format=raw\ -device virtio-blk-pci,bus=pci,addr=0x5,\ drive=drive-virtio-disk2,id=virtio-disk2,bootindex=1 No SCSI bus is created in this case. Both devices appear to SLOF as HDDs so it creates just one disk alias and no cdrom alias and if we are not lucky and DVD got bigger PCI slot#, we will never be able to boot from DVD. Is there any way to distinguish HDD from DVD via virtio protocol from the guest (i.e. SLOF)? Thanks! No, virtio-blk is always a HDD. Thanks, good to know. Does bootindex work with pSeries? Nope. Seems the time to support this has come :) I need some help to understand how to do that properly. There are 2 sets of callbacks - get_dev_path() and get_fw_dev_path(). And we also have device-tree which SLOF parses and creates aliases from so it is a good hint what SLOF can boot from. So I added some debug output and run a test. The parameters below create virtio-scsi bus with 2 DVDs, ibmvscsi bus with 2 DVDs (same ones but this is not the point), virtio-net-pci. -device virtio-scsi-pci,id=v11 \ -drive file=virtimg/SLES-11-SP3-DVD-ppc64-GM-DVD1.iso,if=none,id=vi11,\ readonly=on,format=raw \ -device scsi-cd,bus=v11.0,channel=0,scsi-id=3,lun=1,\ drive=vi11,bootindex=13 \ -drive file=virtimg/Fedora-18-ppc64-DVD.iso,if=none,id=vi12,\ readonly=on,format=raw \ -device scsi-cd,bus=v11.0,channel=0,scsi-id=3,lun=2,\ drive=vi12,bootindex=12 \ -netdev tap,id=netdev5000,ifname=tap5000,script=ifup.sh,\ downscript=ifdown.sh \ -device virtio-net-pci,netdev=netdev5000,bus=pci,addr=1.0,\ mac=C0:41:49:4b:00:00,bootindex=11 \ -device spapr-vscsi,id=s11 \ -drive file=virtimg/SLES-11-SP3-DVD-ppc64-GM-DVD1.iso,if=none,id=i11,\ readonly=on,format=raw \ -device scsi-cd,bus=s11.0,channel=0,scsi-id=3,lun=1,\ drive=i11,bootindex=3 \ -drive file=virtimg/Fedora-18-ppc64-DVD.iso,if=none,id=i12,\ readonly=on,format=raw \ -device scsi-cd,bus=s11.0,channel=0,scsi-id=3,lun=2,\ drive=i12,bootindex=2 \ I did few changes in QEMU (diffs are below) to replace default /spapr-pci-host-bridge/ with /pci@8002000/ in what qdev_get_dev_path/qdev_get_fw_dev_path return, this is to distinguish PHBs. I added some printfs to see what qdev_*dev_path() do: qdev_get_dev_path: v-scsi@7101/0:3:2 suffix=(null) v-scsi@7101/0:3:1 suffix=(null) pci@8002000:01.0 suffix=/ethernet-phy@0 pci@8002000:00.0/0:3:2 suffix=(null) pci@8002000:00.0/0:3:1 suffix=(null) qdev_get_fw_dev_path: /spapr-vio-bridge/spapr-vscsi/channel@0/disk@3,2 suffix=(null) /spapr-vio-bridge/spapr-vscsi/channel@0/disk@3,1 suffix=(null) /pci@8002000/ethernet@1 suffix=/ethernet-phy@0 /pci@8002000/scsi@0/channel@0/disk@3,2 suffix=(null) /pci@8002000/scsi@0/channel@0/disk@3,1 suffix=(null) SLOF: 0 devalias cdrom123 : /pci@8002000/scsi@0/disk@1030001 cdrom12 : /pci@8002000/scsi@0/disk@1030002 hvterm : /vdevice/vty@71000100 net : /pci@8002000/ethernet@1 scsi : /vdevice/v-scsi@7101 cdrom1 : /vdevice/v-scsi@7101/disk@8301 cdrom : /vdevice/v-scsi@7101/disk@8302 nvram : /vdevice/nvram@7100 ok In ideal world I would want to get in QEMU what SLOF can understand and pass this to SLOF. But QEMU APIs return something which cannot be converted straight away. Or I could simply put bootindex to the device tree nodes (as qemu,bootindex) but in this case wildcard nodes support fails as there is just a single node /vdevice/v-scsi@7101/disk in the device tree for all LUNs. And we definitely do not want to create nodes for all disk devices. Or I can implement a smart converter from QEMU strings to OF pathnames. Or I can implement third set of callbacks, something like qdev_OF_dev_path(). Or not support bootindex at all. All possibilities suck but which one sucks less? :) Thanks! btw what format does qdev_get_fw_dev_path() use? This is not OF1275 so what is it? diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c index
Re: [Qemu-devel] [PATCH] qemu-iotests: fix 030 for faster machines
On Tue, Oct 15, 2013 at 10:41:51AM +0800, Fam Zheng wrote: If the block job completes too fast, the test can fail. Change the numbers so the qmp events are more stably captured by the script. A sleep is removed for the same reason. Signed-off-by: Fam Zheng f...@redhat.com --- tests/qemu-iotests/030 | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) Can you try using the blkdebug suspend command to pause I/O? That way the test can be made reliable. Stefan
Re: [Qemu-devel] [PATCH v2] mirror: drop local_err in mirror_complete
On Wed, Oct 16, 2013 at 08:56:49PM +0200, Max Reitz wrote: On 2013-10-15 04:23, Fam Zheng wrote: The reason I object it here is that error_propagate *currently* is a no-op. But this may change in the future: I have already sent an RFC which extends error_propagate so it can generate an error backtrace if enabled through configure. If this (or something similar which extends error_propagate to do more than basically just *errp = local_err) is merged to/implemented in qemu later on, I believe we want to call error_propagate in every function that touches an error variable in order to generate a backtrace with maximum verbosity. Did you check if glib's backtrace(3) APIs can be used in error_set() instead of rolling our own backtrace? Also, what is the purpose of the backtrace? If the problem is that error messages don't identify unique errors, then we should fix that instead of relying on backtraces. For example, a function that opens two separate files shouldn't just fail with File not found since we don't know which of the two files wasn't found. Stefan
Re: [Qemu-devel] [PATCH v2] mirror: drop local_err in mirror_complete
On Wed, Oct 16, 2013 at 08:56:49PM +0200, Max Reitz wrote: However, this is not the reason I'd object a patch doing something different (here: dropping the unused backing_filename code) BTW I agree with this. This should be a separate patch. Stefan
Re: [Qemu-devel] virtio-blk-pci: how to tell if it is CD or HDD?
Il 17/10/2013 14:38, Alexey Kardashevskiy ha scritto: qdev_get_fw_dev_path: /spapr-vio-bridge/spapr-vscsi/channel@0/disk@3,2 suffix=(null) /spapr-vio-bridge/spapr-vscsi/channel@0/disk@3,1 suffix=(null) You need to implement qdev_fw_get_path to change spapr-vio-bridge - vdevice spapr-vscsi - v-scsi@REG /pci@8002000/ethernet@1 suffix=/ethernet-phy@0 The extra suffix is not a problem since you can parse a prefix successfully. /pci@8002000/scsi@0/channel@0/disk@3,2 suffix=(null) /pci@8002000/scsi@0/channel@0/disk@3,1 suffix=(null) I guess this is virtio-scsi. SLOF: 0 devalias cdrom123 : /pci@8002000/scsi@0/disk@1030001 cdrom12 : /pci@8002000/scsi@0/disk@1030002 hvterm : /vdevice/vty@71000100 net : /pci@8002000/ethernet@1 scsi : /vdevice/v-scsi@7101 cdrom1 : /vdevice/v-scsi@7101/disk@8301 cdrom : /vdevice/v-scsi@7101/disk@8302 nvram : /vdevice/nvram@7100 ok In ideal world I would want to get in QEMU what SLOF can understand and pass this to SLOF. But QEMU APIs return something which cannot be converted straight away. Or I could simply put bootindex to the device tree nodes (as qemu,bootindex) but in this case wildcard nodes support fails as there is just a single node /vdevice/v-scsi@7101/disk in the device tree for all LUNs. And we definitely do not want to create nodes for all disk devices. Or I can implement a smart converter from QEMU strings to OF pathnames. Or I can implement third set of callbacks, something like qdev_OF_dev_path(). Or not support bootindex at all. All possibilities suck but which one sucks less? :) Thanks! In general, try to make QEMU produce SLOF APIs by modifying the devices that instantiate the buses. On top of this, fix the remaining QEMU-OF differences using a callback in QEMUMachine. This callback would be called by qdev_get_fw_dev_path_helper and, if it returns something non-NULL, the result would be used instead of calling bus_get_fw_dev_path. btw what format does qdev_get_fw_dev_path() use? This is not OF1275 so what is it? It is based on open-firmware. For SCSI however openfirmware had disk@TARGET,LUN but that does not include the channel. Paolo
Re: [Qemu-devel] [PATCH] block/raw-win32: Always use -errno in hdev_open
On Fri, Oct 11, 2013 at 02:30:16PM +0200, Max Reitz wrote: On one occasion, hdev_open() returned -1 in case of an unknown error instead of a proper -errno value. Adjust this to match the behavior of raw_open() (in raw-win32), which is to return -EINVAL in this case. Also, change the call to error_setg*() to match the one in raw_open() as well. Signed-off-by: Max Reitz mre...@redhat.com --- Follow-up to (as suggested by Eric): - block/raw-win32: Employ error parameter (3/5 from the block: Employ error parameter series) --- block/raw-win32.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
Re: [Qemu-devel] [PATCH 1/1] e820: pass high memory too.
Hi, On Thu, Oct 17, 2013 at 01:09:38PM +0200, Gerd Hoffmann wrote: We have a fw_cfg entry to pass e820 entries from qemu to the firmware. Today it's used to pass reservations only. This patch makes qemu pass entries for RAM too. This allows to pass RAM sizes larger than 1TB to the firmware and it will also allow to pass non-contignous memory ramges should we decide to implement that some day, say for our virtual numa nodes. Obviously this needs some extra care to not break existing firware. SeaBIOS loads the entries and happily adds them without looking at the type. Which is problematic for memory below 4g as this will overwrite reservations added for bios memory etc. For memory above 4g it works just fine, seabios will merge the entry derived from cmos with the one loaded from fw_cfg. The reason for not fixing the cmos and defer the fixage of the 1TB boot, is to develop a better approach, and this mixture of e820 and cmos doesn't look like an improvement. The only thing it avoids is to touch seabios but it provides no benefit whatsoever if compared to fixing the cmos which looks cleaner to me than having to compute a mix of cmos and e820 in seabios (and potentially having other bioses following this mix-incomplete-API). I thought the reason of deferring the fixage of 1TB boot to wait for a better approach and better API, I didn't think the end result had to be a mix API that adds no value. The premise that this will also allow to pass non-contiguous memory is partly false, as you can't use the e820 API below 4g so there's no way to create non contiguous memory with this mix-cmos-e820-API. So instead of adding if (0) patches and requiring bioses to mix information from e820 maps and cmos to boot with more than 1TB, why can't simply seabios can be fixed to preserve its own reservations (fragmenting the e820 map passed by qemu) while it build the e820 map? So then we the qemu API becomes: e820_add_entry(0, ram_size, E820_RAM); No mix. And seabios shall as well crash if the highest ram address in the e820 map provided by qemu (truncated to 40 bits) doesn't match the cmos information, to be sure we notice if we break backwards compatibility with the cmos API. This intermediate mixed paravirt API doesn't make much sense to me.
Re: [Qemu-devel] [PATCH v2] mirror: drop local_err in mirror_complete
Am 17.10.2013 um 14:49 hat Stefan Hajnoczi geschrieben: On Wed, Oct 16, 2013 at 08:56:49PM +0200, Max Reitz wrote: On 2013-10-15 04:23, Fam Zheng wrote: The reason I object it here is that error_propagate *currently* is a no-op. But this may change in the future: I have already sent an RFC which extends error_propagate so it can generate an error backtrace if enabled through configure. If this (or something similar which extends error_propagate to do more than basically just *errp = local_err) is merged to/implemented in qemu later on, I believe we want to call error_propagate in every function that touches an error variable in order to generate a backtrace with maximum verbosity. Did you check if glib's backtrace(3) APIs can be used in error_set() instead of rolling our own backtrace? Also, what is the purpose of the backtrace? If the problem is that error messages don't identify unique errors, then we should fix that instead of relying on backtraces. For example, a function that opens two separate files shouldn't just fail with File not found since we don't know which of the two files wasn't found. Mostly debugging, I guess. Even if you have unique error messages that can only come from a single place, finding that single place by looking at all called functions from a given QMP command can take quite a bit of time. I can see it useful. And we don't even have the unique error messages yet, so we shouldn't fall in the trap of rejecting an improvement because it's not perfect. Kevin
Re: [Qemu-devel] virtio-blk-pci: how to tell if it is CD or HDD?
On 17.10.2013, at 14:54, Paolo Bonzini pbonz...@redhat.com wrote: Il 17/10/2013 14:38, Alexey Kardashevskiy ha scritto: qdev_get_fw_dev_path: /spapr-vio-bridge/spapr-vscsi/channel@0/disk@3,2 suffix=(null) /spapr-vio-bridge/spapr-vscsi/channel@0/disk@3,1 suffix=(null) You need to implement qdev_fw_get_path to change spapr-vio-bridge - vdevice spapr-vscsi - v-scsi@REG /pci@8002000/ethernet@1 suffix=/ethernet-phy@0 The extra suffix is not a problem since you can parse a prefix successfully. /pci@8002000/scsi@0/channel@0/disk@3,2 suffix=(null) /pci@8002000/scsi@0/channel@0/disk@3,1 suffix=(null) I guess this is virtio-scsi. SLOF: 0 devalias cdrom123 : /pci@8002000/scsi@0/disk@1030001 cdrom12 : /pci@8002000/scsi@0/disk@1030002 hvterm : /vdevice/vty@71000100 net : /pci@8002000/ethernet@1 scsi : /vdevice/v-scsi@7101 cdrom1 : /vdevice/v-scsi@7101/disk@8301 cdrom : /vdevice/v-scsi@7101/disk@8302 nvram : /vdevice/nvram@7100 ok In ideal world I would want to get in QEMU what SLOF can understand and pass this to SLOF. But QEMU APIs return something which cannot be converted straight away. Or I could simply put bootindex to the device tree nodes (as qemu,bootindex) but in this case wildcard nodes support fails as there is just a single node /vdevice/v-scsi@7101/disk in the device tree for all LUNs. And we definitely do not want to create nodes for all disk devices. Or I can implement a smart converter from QEMU strings to OF pathnames. Or I can implement third set of callbacks, something like qdev_OF_dev_path(). Or not support bootindex at all. All possibilities suck but which one sucks less? :) Thanks! In general, try to make QEMU produce SLOF APIs by modifying the devices that instantiate the buses. But please make sure to not block the path for non-SLOF machines. -M mac99 should still be able to get different path names for PCI devices for example. Alex
Re: [Qemu-devel] [PATCH v3 2/2] vmdk: Implment bdrv_get_specific_info
On Fri, Oct 11, 2013 at 08:08:35PM +0800, Fam Zheng wrote: @@ -814,6 +821,7 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags, ret = -ENOTSUP; goto exit; } +s-create_type = g_strdup(ct); Where is this freed if opening the file fails?
Re: [Qemu-devel] virtio-blk-pci: how to tell if it is CD or HDD?
On 10/17/2013 11:54 PM, Paolo Bonzini wrote: Il 17/10/2013 14:38, Alexey Kardashevskiy ha scritto: qdev_get_fw_dev_path: /spapr-vio-bridge/spapr-vscsi/channel@0/disk@3,2 suffix=(null) /spapr-vio-bridge/spapr-vscsi/channel@0/disk@3,1 suffix=(null) You need to implement qdev_fw_get_path to change spapr-vio-bridge - vdevice spapr-vscsi - v-scsi@REG /pci@8002000/ethernet@1 suffix=/ethernet-phy@0 The extra suffix is not a problem since you can parse a prefix successfully. /pci@8002000/scsi@0/channel@0/disk@3,2 suffix=(null) /pci@8002000/scsi@0/channel@0/disk@3,1 suffix=(null) I guess this is virtio-scsi. Correct. SLOF: 0 devalias cdrom123 : /pci@8002000/scsi@0/disk@1030001 cdrom12 : /pci@8002000/scsi@0/disk@1030002 hvterm : /vdevice/vty@71000100 net : /pci@8002000/ethernet@1 scsi : /vdevice/v-scsi@7101 cdrom1 : /vdevice/v-scsi@7101/disk@8301 cdrom : /vdevice/v-scsi@7101/disk@8302 nvram : /vdevice/nvram@7100 ok In ideal world I would want to get in QEMU what SLOF can understand and pass this to SLOF. But QEMU APIs return something which cannot be converted straight away. Or I could simply put bootindex to the device tree nodes (as qemu,bootindex) but in this case wildcard nodes support fails as there is just a single node /vdevice/v-scsi@7101/disk in the device tree for all LUNs. And we definitely do not want to create nodes for all disk devices. Or I can implement a smart converter from QEMU strings to OF pathnames. Or I can implement third set of callbacks, something like qdev_OF_dev_path(). Or not support bootindex at all. All possibilities suck but which one sucks less? :) Thanks! In general, try to make QEMU produce SLOF APIs by modifying the devices that instantiate the buses. channel@0 - ? This is a generic scsi bus, cannot change this. disk@3,2 - disk@8302? This is a generic scsi-cd, cannot change this either On top of this, fix the remaining QEMU-OF differences using a callback in QEMUMachine. This callback would be called by qdev_get_fw_dev_path_helper and, if it returns something non-NULL, the result would be used instead of calling bus_get_fw_dev_path. A single machine callback which will recognize all possible bootable devices and replace things like disk@3,2 - disk@8302? Hm. I mean I can do all of that but is it still kosher? :) Or I am missing the point, again. btw what format does qdev_get_fw_dev_path() use? This is not OF1275 so what is it? It is based on open-firmware. For SCSI however openfirmware had disk@TARGET,LUN but that does not include the channel. I am confused now. What standard/format/spec defines this channel@ thingy or it is made up by QEMU and the x86 bios shipped with QEMU? open firmware == IEEE1275, right? -- Alexey
Re: [Qemu-devel] [PATCH v2 5/6] util: Provide fallback hwcap and platform for powerpc
On 10/17/2013 05:03 AM, Peter Maydell wrote: On 10 September 2013 23:07, Richard Henderson r...@twiddle.net wrote: Allow host detection on non-linux hosts. Signed-off-by: Richard Henderson r...@twiddle.net --- util/getauxval.c | 56 ++-- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/util/getauxval.c b/util/getauxval.c index 59f53ff..e7f7d63 100644 --- a/util/getauxval.c +++ b/util/getauxval.c @@ -60,12 +60,64 @@ void qemu_init_auxval(char **envp) static const char *default_platform(void) { -return NULL; +const char *ret = NULL; + +#ifdef _ARCH_PPC +# if defined(_ARCH_PWR8) +ret = power8; +# elif defined(_ARCH_PWR7) All these arch-specific ifdefs seem out of place in a generic util/ source file. Where should they go? I'm tempted to drop this whole default thing... r~
Re: [Qemu-devel] [PATCH 1.7] monitor: eliminate monitor_event_state_lock
On Wed, 16 Oct 2013 19:17:08 +0200 Paolo Bonzini pbonz...@redhat.com wrote: This lock does not protect anything that the BQL does not already protect. Furthermore, with -nodefaults and no monitor, the mutex is not initialized but monitor_protocol_event_queue is called anyway, which causes a crash under mingw (and only works by luck. under Linux or other POSIX OSes). Reported-by: Orx Goshen orx.gos...@intel.com Cc: Daniel Berrange berra...@redhat.com Cc: qemu-sta...@nongnu.org Signed-off-by: Paolo Bonzini pbonz...@redhat.com Applied to the qmp branch, thanks. --- Maybe I'm missing something? I don't think so. monitor.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/monitor.c b/monitor.c index 74f3f1b..0ae99dc 100644 --- a/monitor.c +++ b/monitor.c @@ -511,7 +511,6 @@ static const char *monitor_event_names[] = { QEMU_BUILD_BUG_ON(ARRAY_SIZE(monitor_event_names) != QEVENT_MAX) MonitorEventState monitor_event_state[QEVENT_MAX]; -QemuMutex monitor_event_state_lock; /* * Emits the event to every monitor instance @@ -543,7 +542,6 @@ monitor_protocol_event_queue(MonitorEvent event, int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); assert(event QEVENT_MAX); -qemu_mutex_lock(monitor_event_state_lock); evstate = (monitor_event_state[event]); trace_monitor_protocol_event_queue(event, data, @@ -576,7 +574,6 @@ monitor_protocol_event_queue(MonitorEvent event, evstate-last = now; } } -qemu_mutex_unlock(monitor_event_state_lock); } @@ -589,7 +586,6 @@ static void monitor_protocol_event_handler(void *opaque) MonitorEventState *evstate = opaque; int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); -qemu_mutex_lock(monitor_event_state_lock); trace_monitor_protocol_event_handler(evstate-event, evstate-data, @@ -601,7 +597,6 @@ static void monitor_protocol_event_handler(void *opaque) evstate-data = NULL; } evstate-last = now; -qemu_mutex_unlock(monitor_event_state_lock); } @@ -638,7 +633,6 @@ monitor_protocol_event_throttle(MonitorEvent event, * and initialize state */ static void monitor_protocol_event_init(void) { -qemu_mutex_init(monitor_event_state_lock); /* Limit RTC BALLOON events to 1 per second */ monitor_protocol_event_throttle(QEVENT_RTC_CHANGE, 1000); monitor_protocol_event_throttle(QEVENT_BALLOON_CHANGE, 1000);
Re: [Qemu-devel] [PATCH] vmdk: Only read cid from image file when opening
On Fri, Oct 11, 2013 at 03:55:39PM +0800, Fam Zheng wrote: @@ -378,6 +359,22 @@ static int vmdk_parent_open(BlockDriverState *bs) } pstrcpy(bs-backing_file, end_name - p_name + 1, p_name); + +ret = bdrv_open_backing_file(bs, NULL, errp); This breaks 'backing.' options since we ignore them here. I suggest letting the block layer open the backing file as normal instead of duplicating the code here. Simply add a bool s-cid_checked field to remember whether the CID needs to be checked. Then it will only be done once.
Re: [Qemu-devel] virtio-blk-pci: how to tell if it is CD or HDD?
On 10/18/2013 12:02 AM, Alexander Graf wrote: On 17.10.2013, at 14:54, Paolo Bonzini pbonz...@redhat.com wrote: Il 17/10/2013 14:38, Alexey Kardashevskiy ha scritto: qdev_get_fw_dev_path: /spapr-vio-bridge/spapr-vscsi/channel@0/disk@3,2 suffix=(null) /spapr-vio-bridge/spapr-vscsi/channel@0/disk@3,1 suffix=(null) You need to implement qdev_fw_get_path to change spapr-vio-bridge - vdevice spapr-vscsi - v-scsi@REG /pci@8002000/ethernet@1 suffix=/ethernet-phy@0 The extra suffix is not a problem since you can parse a prefix successfully. /pci@8002000/scsi@0/channel@0/disk@3,2 suffix=(null) /pci@8002000/scsi@0/channel@0/disk@3,1 suffix=(null) I guess this is virtio-scsi. SLOF: 0 devalias cdrom123 : /pci@8002000/scsi@0/disk@1030001 cdrom12 : /pci@8002000/scsi@0/disk@1030002 hvterm : /vdevice/vty@71000100 net : /pci@8002000/ethernet@1 scsi : /vdevice/v-scsi@7101 cdrom1 : /vdevice/v-scsi@7101/disk@8301 cdrom : /vdevice/v-scsi@7101/disk@8302 nvram : /vdevice/nvram@7100 ok In ideal world I would want to get in QEMU what SLOF can understand and pass this to SLOF. But QEMU APIs return something which cannot be converted straight away. Or I could simply put bootindex to the device tree nodes (as qemu,bootindex) but in this case wildcard nodes support fails as there is just a single node /vdevice/v-scsi@7101/disk in the device tree for all LUNs. And we definitely do not want to create nodes for all disk devices. Or I can implement a smart converter from QEMU strings to OF pathnames. Or I can implement third set of callbacks, something like qdev_OF_dev_path(). Or not support bootindex at all. All possibilities suck but which one sucks less? :) Thanks! In general, try to make QEMU produce SLOF APIs by modifying the devices that instantiate the buses. But please make sure to not block the path for non-SLOF machines. -M mac99 should still be able to get different path names for PCI devices for example. Ok. Then question for you. I need to change root PHB name from spapr-pci-host-bridge (which is a class name) to something reasonable. For example, PCIHostBridgeClass::root_bus_path does the job. But this part of OF path is made by sysbus_get_fw_dev_path() from hw/core/sysbus.c which does not know about PCI. So it cannot call PCI callbacks. I workarounded this by setting PHB's id to what sysbus_get_fw_dev_path() returns but this is barely the proper fix. I could initialize my PHBs s-mmio[0].addr to BUID and sysbus_get_fw_dev_path() would return what I need but it won't be MMIO by any mean and I do not really want to be responsible for all side effect it may have :) So how to fix it correctly? Thanks. -- Alexey
Re: [Qemu-devel] virtio-blk-pci: how to tell if it is CD or HDD?
Il 17/10/2013 15:09, Alexey Kardashevskiy ha scritto: In general, try to make QEMU produce SLOF APIs by modifying the devices that instantiate the buses. channel@0 - ? This is a generic scsi bus, cannot change this. disk@3,2 - disk@8302? This is a generic scsi-cd, cannot change this either Yes, I was referring more to the vio cases. On top of this, fix the remaining QEMU-OF differences using a callback in QEMUMachine. This callback would be called by qdev_get_fw_dev_path_helper and, if it returns something non-NULL, the result would be used instead of calling bus_get_fw_dev_path. A single machine callback which will recognize all possible bootable devices and replace things like disk@3,2 - disk@8302? Hm. I mean I can do all of that but is it still kosher? :) Or I am missing the point, again. No, a machine callback that will recognize SCSI disks and return disk@8302. This will replace the channel@0/disk@3,2 path returned by default. btw what format does qdev_get_fw_dev_path() use? This is not OF1275 so what is it? It is based on open-firmware. For SCSI however openfirmware had disk@TARGET,LUN but that does not include the channel. I am confused now. What standard/format/spec defines this channel@ thingy or it is made up by QEMU and the x86 bios shipped with QEMU? open firmware == IEEE1275, right? It's made up. Paolo
Re: [Qemu-devel] virtio-blk-pci: how to tell if it is CD or HDD?
Il 17/10/2013 15:36, Alexey Kardashevskiy ha scritto: But please make sure to not block the path for non-SLOF machines. -M mac99 should still be able to get different path names for PCI devices for example. Ok. Then question for you. I need to change root PHB name from spapr-pci-host-bridge (which is a class name) to something reasonable. For example, PCIHostBridgeClass::root_bus_path does the job. But this part of OF path is made by sysbus_get_fw_dev_path() from hw/core/sysbus.c which does not know about PCI. So it cannot call PCI callbacks. I workarounded this by setting PHB's id to what sysbus_get_fw_dev_path() returns but this is barely the proper fix. I could initialize my PHBs s-mmio[0].addr to BUID and sysbus_get_fw_dev_path() would return what I need but it won't be MMIO by any mean and I do not really want to be responsible for all side effect it may have :) So how to fix it correctly? Set dc-fw_name for the spapr-pci-host-bridge class, and similarly for the vio bridge. Paolo
[Qemu-devel] [PATCH 04/10] apic: Document why cannot_instantiate_with_device_add_yet
From: Markus Armbruster arm...@redhat.com Signed-off-by: Markus Armbruster arm...@redhat.com --- hw/intc/apic_common.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c index ea420c7..aaef054 100644 --- a/hw/intc/apic_common.c +++ b/hw/intc/apic_common.c @@ -386,9 +386,13 @@ static void apic_common_class_init(ObjectClass *klass, void *data) dc-vmsd = vmstate_apic_common; dc-reset = apic_reset_common; -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ dc-props = apic_properties_common; idc-init = apic_init_common; +/* + * Reason: APIC and CPU need to be wired up by + * x86_cpu_apic_create() + */ +dc-cannot_instantiate_with_device_add_yet = true; } static const TypeInfo apic_common_type = { -- 1.8.1.4
[Qemu-devel] [PATCH 00/10] Clean up and fix no_user
From: Markus Armbruster arm...@redhat.com In an ideal world, machines can be built by wiring devices together with configuration, not code. Unfortunately, that's not the world we live in right now. We still have quite a few devices that need to be wired up by code. If you try to device_add such a device, it'll fail in sometimes mysterious ways. If you're lucky, you get an unmysterious immediate crash. We used to protect users from such badness by marking devices where device_add cannot possibly work no-user, and refusing to device_add them. Anthony silently broke the protection in v1.1. He has rejected attempts to unbreak it with the argument that the protection makes it impossible to wire devices together with configuration, not code, and that the protection is being misused[*]. On the former, I disagree. The problem isn't protecting users from devices that cannot be wired up that way, it's devices that cannot be wired up that way. On the latter, Anthony has a point: the purpose of the no-user flag isn't obvious, and some of its uses are suspect. So, instead of just fixing the regression, this series first addresses that point. PATCH 1 clarifies the purpose of no-user. PATCH 2-9 clean up and document its use. PATCH 10 fixes the regression. Markus Armbruster (10): qdev: Replace no_user by cannot_instantiate_with_device_add_yet sysbus: Set cannot_instantiate_with_device_add_yet cpu: Document why cannot_instantiate_with_device_add_yet apic: Document why cannot_instantiate_with_device_add_yet pci-host: Consistently set cannot_instantiate_with_device_add_yet ich9: Document why cannot_instantiate_with_device_add_yet piix3 piix4: Clean up use of cannot_instantiate_with_device_add_yet vt82c686: Clean up use of cannot_instantiate_with_device_add_yet isa: Clean up use of cannot_instantiate_with_device_add_yet qdev: Do not let the user try to device_add when it cannot work hw/acpi/piix4.c| 7 ++- hw/alpha/typhoon.c | 2 -- hw/arm/versatilepb.c | 1 - hw/audio/pcspk.c | 3 ++- hw/audio/pl041.c | 1 - hw/block/fdc.c | 1 - hw/core/sysbus.c | 7 +++ hw/display/pl110.c | 1 - hw/dma/pl080.c | 1 - hw/i2c/smbus_ich9.c| 6 +- hw/i386/kvm/clock.c| 1 - hw/i386/kvmvapic.c | 1 - hw/i386/pc.c | 1 - hw/ide/piix.c | 3 --- hw/ide/via.c | 1 - hw/input/pckbd.c | 1 - hw/input/vmmouse.c | 3 ++- hw/intc/apic_common.c | 6 +- hw/intc/arm_gic.c | 1 - hw/intc/arm_gic_common.c | 1 - hw/intc/arm_gic_kvm.c | 1 - hw/intc/i8259_common.c | 8 +++- hw/intc/ioapic_common.c| 1 - hw/intc/pl190.c| 1 - hw/isa/isa-bus.c | 1 - hw/isa/lpc_ich9.c | 7 +-- hw/isa/piix4.c | 6 +- hw/isa/vt82c686.c | 6 +- hw/mips/gt64xxx_pci.c | 5 + hw/misc/arm_l2x0.c | 1 - hw/misc/vmport.c | 3 ++- hw/nvram/fw_cfg.c | 1 - hw/pci-bridge/dec.c| 5 + hw/pci-host/apb.c | 5 + hw/pci-host/bonito.c | 8 +--- hw/pci-host/grackle.c | 8 +--- hw/pci-host/piix.c | 19 +++ hw/pci-host/ppce500.c | 5 + hw/pci-host/prep.c | 7 +-- hw/pci-host/q35.c | 5 + hw/pci-host/uninorth.c | 20 hw/pci-host/versatile.c| 5 + hw/ppc/ppc4xx_pci.c| 5 + hw/ppc/spapr_vio.c | 2 -- hw/s390x/ipl.c | 1 - hw/s390x/s390-virtio-bus.c | 2 -- hw/s390x/virtio-ccw.c | 2 -- hw/sd/pl181.c | 1 - hw/sh4/sh_pci.c| 5 + hw/timer/arm_mptimer.c | 1 - hw/timer/hpet.c| 1 - hw/timer/i8254_common.c| 1 - hw/timer/m48t59.c | 1 - hw/timer/mc146818rtc.c | 1 - hw/timer/pl031.c | 1 - include/hw/qdev-core.h | 13 - qdev-monitor.c | 7 --- qom/cpu.c | 6 +- 58 files changed, 162 insertions(+), 65 deletions(-) -- 1.8.1.4
[Qemu-devel] [PATCH 08/10] vt82c686: Clean up use of cannot_instantiate_with_device_add_yet
From: Markus Armbruster arm...@redhat.com A VT82C686B southbridge has multiple functions. We model each function as a separate qdev. One of them need some special wiring set up in mips_fulong2e_init() to work: the ISA bridge at 05.0. The IDE controller at 05.1 (via-ide) has always had cannot_instantiate_with_device_add_yet set, but there is no obvious reason why device_add could not work for them. Drop it. Signed-off-by: Markus Armbruster arm...@redhat.com --- hw/ide/via.c | 1 - hw/isa/vt82c686.c | 6 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/hw/ide/via.c b/hw/ide/via.c index b556c14..198123b 100644 --- a/hw/ide/via.c +++ b/hw/ide/via.c @@ -225,7 +225,6 @@ static void via_ide_class_init(ObjectClass *klass, void *data) k-revision = 0x06; k-class_id = PCI_CLASS_STORAGE_IDE; set_bit(DEVICE_CATEGORY_STORAGE, dc-categories); -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ } static const TypeInfo via_ide_info = { diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c index 3e8ec80..ec7c259 100644 --- a/hw/isa/vt82c686.c +++ b/hw/isa/vt82c686.c @@ -480,8 +480,12 @@ static void via_class_init(ObjectClass *klass, void *data) k-class_id = PCI_CLASS_BRIDGE_ISA; k-revision = 0x40; dc-desc = ISA bridge; -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ dc-vmsd = vmstate_via; +/* + * Reason: part of VIA VT82C686 southbridge, needs to be wired up, + * e.g. by mips_fulong2e_init() + */ +dc-cannot_instantiate_with_device_add_yet = true; } static const TypeInfo via_info = { -- 1.8.1.4
[Qemu-devel] [PATCH 02/10] sysbus: Set cannot_instantiate_with_device_add_yet
From: Markus Armbruster arm...@redhat.com device_add plugs devices into suitable bus. For real buses, that actually connects the device. For sysbus, the connections need to be made separately, and device_add can't do that. The device would be left unconncected, and could not possibly work. Many, but not all sysbus devices alreasy set cannot_instantiate_with_device_add_yet in their class init function. Set it in their abstract base's class init function sysbus_device_class_init(), and remove the now redundant assignments from device class init functions. Signed-off-by: Markus Armbruster arm...@redhat.com --- hw/alpha/typhoon.c | 2 -- hw/arm/versatilepb.c | 1 - hw/audio/pl041.c | 1 - hw/core/sysbus.c | 7 +++ hw/display/pl110.c | 1 - hw/dma/pl080.c | 1 - hw/i386/kvm/clock.c| 1 - hw/i386/kvmvapic.c | 1 - hw/intc/arm_gic.c | 1 - hw/intc/arm_gic_common.c | 1 - hw/intc/arm_gic_kvm.c | 1 - hw/intc/ioapic_common.c| 1 - hw/intc/pl190.c| 1 - hw/isa/isa-bus.c | 1 - hw/misc/arm_l2x0.c | 1 - hw/nvram/fw_cfg.c | 1 - hw/pci-host/bonito.c | 2 -- hw/pci-host/grackle.c | 2 -- hw/pci-host/piix.c | 1 - hw/pci-host/prep.c | 1 - hw/ppc/spapr_vio.c | 2 -- hw/s390x/ipl.c | 1 - hw/s390x/s390-virtio-bus.c | 2 -- hw/s390x/virtio-ccw.c | 2 -- hw/sd/pl181.c | 1 - hw/timer/arm_mptimer.c | 1 - hw/timer/hpet.c| 1 - hw/timer/pl031.c | 1 - 28 files changed, 7 insertions(+), 33 deletions(-) diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c index 60987ed..71a5a37 100644 --- a/hw/alpha/typhoon.c +++ b/hw/alpha/typhoon.c @@ -934,11 +934,9 @@ static int typhoon_pcihost_init(SysBusDevice *dev) static void typhoon_pcihost_class_init(ObjectClass *klass, void *data) { -DeviceClass *dc = DEVICE_CLASS(klass); SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); k-init = typhoon_pcihost_init; -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ } static const TypeInfo typhoon_pcihost_info = { diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c index bb0c0ba..aef2bde 100644 --- a/hw/arm/versatilepb.c +++ b/hw/arm/versatilepb.c @@ -390,7 +390,6 @@ static void vpb_sic_class_init(ObjectClass *klass, void *data) SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); k-init = vpb_sic_init; -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ dc-vmsd = vmstate_vpb_sic; } diff --git a/hw/audio/pl041.c b/hw/audio/pl041.c index 8ba661a..ed82be5 100644 --- a/hw/audio/pl041.c +++ b/hw/audio/pl041.c @@ -632,7 +632,6 @@ static void pl041_device_class_init(ObjectClass *klass, void *data) k-init = pl041_init; set_bit(DEVICE_CATEGORY_SOUND, dc-categories); -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ dc-reset = pl041_device_reset; dc-vmsd = vmstate_pl041; dc-props = pl041_device_properties; diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c index b84cd4a..6e880a8 100644 --- a/hw/core/sysbus.c +++ b/hw/core/sysbus.c @@ -257,6 +257,13 @@ static void sysbus_device_class_init(ObjectClass *klass, void *data) DeviceClass *k = DEVICE_CLASS(klass); k-init = sysbus_device_init; k-bus_type = TYPE_SYSTEM_BUS; +/* + * device_add plugs devices into suitable bus. For real buses, + * that actually connects the device. For sysbus, the connections + * need to be made separately, and device_add can't do that. The + * device would be left unconncected, and could not possibly work. + */ +k-cannot_instantiate_with_device_add_yet = true; } static const TypeInfo sysbus_device_type_info = { diff --git a/hw/display/pl110.c b/hw/display/pl110.c index 7ad5972..ab689e9 100644 --- a/hw/display/pl110.c +++ b/hw/display/pl110.c @@ -496,7 +496,6 @@ static void pl110_class_init(ObjectClass *klass, void *data) k-init = pl110_initfn; set_bit(DEVICE_CATEGORY_DISPLAY, dc-categories); -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ dc-vmsd = vmstate_pl110; } diff --git a/hw/dma/pl080.c b/hw/dma/pl080.c index a515621..cb7bda9 100644 --- a/hw/dma/pl080.c +++ b/hw/dma/pl080.c @@ -381,7 +381,6 @@ static void pl080_class_init(ObjectClass *oc, void *data) { DeviceClass *dc = DEVICE_CLASS(oc); -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ dc-vmsd = vmstate_pl080; } diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c index abd2ce8..892aa02 100644 --- a/hw/i386/kvm/clock.c +++ b/hw/i386/kvm/clock.c @@ -114,7 +114,6 @@ static void kvmclock_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); dc-realize = kvmclock_realize; -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */
[Qemu-devel] [PATCH 09/10] isa: Clean up use of cannot_instantiate_with_device_add_yet
From: Markus Armbruster arm...@redhat.com Drop it when there's no obvious reason why device_add could not work. Else keep and document why. * isa-fdc, port92, i8042, m48t59_isa, mc146818rtc, isa-pit, kvm-pit: drop (from the last two by dropping it from their abstract base pit-common) * pcspk: keep because of pointer property pit, and because realize sets global pcspk_state * vmmouse: keep because of pointer property ps2_mouse * vmport: keep because realize sets global port_state * isa-i8259, kvm-i8259: keep (in their abstract base pic-common), because the PICs need additional wiring: its IRQ input lines are set up by board code, and the wiring of the slave to the master is hard-coded in device model code. Signed-off-by: Markus Armbruster arm...@redhat.com --- hw/audio/pcspk.c| 3 ++- hw/block/fdc.c | 1 - hw/i386/pc.c| 1 - hw/input/pckbd.c| 1 - hw/input/vmmouse.c | 3 ++- hw/intc/i8259_common.c | 8 +++- hw/misc/vmport.c| 3 ++- hw/timer/i8254_common.c | 1 - hw/timer/m48t59.c | 1 - hw/timer/mc146818rtc.c | 1 - 10 files changed, 13 insertions(+), 10 deletions(-) diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c index 8e3e178..f980d66 100644 --- a/hw/audio/pcspk.c +++ b/hw/audio/pcspk.c @@ -192,8 +192,9 @@ static void pcspk_class_initfn(ObjectClass *klass, void *data) dc-realize = pcspk_realizefn; set_bit(DEVICE_CATEGORY_SOUND, dc-categories); -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ dc-props = pcspk_properties; +/* Reason: pointer property pit, realize sets global pcspk_state */ +dc-cannot_instantiate_with_device_add_yet = true; } static const TypeInfo pcspk_info = { diff --git a/hw/block/fdc.c b/hw/block/fdc.c index 86f4920..592b58f 100644 --- a/hw/block/fdc.c +++ b/hw/block/fdc.c @@ -2234,7 +2234,6 @@ static void isabus_fdc_class_init(ObjectClass *klass, void *data) dc-realize = isabus_fdc_realize; dc-fw_name = fdc; -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ dc-reset = fdctrl_external_reset_isa; dc-vmsd = vmstate_isa_fdc; dc-props = isa_fdc_properties; diff --git a/hw/i386/pc.c b/hw/i386/pc.c index fe33843..bebda79 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -544,7 +544,6 @@ static void port92_class_initfn(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ dc-realize = port92_realizefn; dc-reset = port92_reset; dc-vmsd = vmstate_port92_isa; diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c index dee31a6..655b8c5 100644 --- a/hw/input/pckbd.c +++ b/hw/input/pckbd.c @@ -522,7 +522,6 @@ static void i8042_class_initfn(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); dc-realize = i8042_realizefn; -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ dc-vmsd = vmstate_kbd_isa; } diff --git a/hw/input/vmmouse.c b/hw/input/vmmouse.c index 600e4a2..6a50533 100644 --- a/hw/input/vmmouse.c +++ b/hw/input/vmmouse.c @@ -282,10 +282,11 @@ static void vmmouse_class_initfn(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); dc-realize = vmmouse_realizefn; -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ dc-reset = vmmouse_reset; dc-vmsd = vmstate_vmmouse; dc-props = vmmouse_properties; +/* Reason: pointer property ps2_mouse */ +dc-cannot_instantiate_with_device_add_yet = true; } static const TypeInfo vmmouse_info = { diff --git a/hw/intc/i8259_common.c b/hw/intc/i8259_common.c index 2acdbfe..9d29399 100644 --- a/hw/intc/i8259_common.c +++ b/hw/intc/i8259_common.c @@ -135,9 +135,15 @@ static void pic_common_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); dc-vmsd = vmstate_pic_common; -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ dc-props = pic_properties_common; dc-realize = pic_common_realize; +/* + * Reason: unlike ordinary ISA devices, the PICs need additional + * wiring: its IRQ input lines are set up by board code, and the + * wiring of the slave to the master is hard-coded in device model + * code. + */ +dc-cannot_instantiate_with_device_add_yet = true; } static const TypeInfo pic_common_type = { diff --git a/hw/misc/vmport.c b/hw/misc/vmport.c index 94ae6ae..cd5716a 100644 --- a/hw/misc/vmport.c +++ b/hw/misc/vmport.c @@ -162,7 +162,8 @@ static void vmport_class_initfn(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); dc-realize = vmport_realizefn; -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ +/* Reason: realize sets global port_state */ +dc-cannot_instantiate_with_device_add_yet = true; } static const TypeInfo
[Qemu-devel] [PATCH 01/10] qdev: Replace no_user by cannot_instantiate_with_device_add_yet
From: Markus Armbruster arm...@redhat.com In an ideal world, machines can be built by wiring devices together with configuration, not code. Unfortunately, that's not the world we live in right now. We still have quite a few devices that need to be wired up by code. If you try to device_add such a device, it'll fail in sometimes mysterious ways. If you're lucky, you get an unmysterious immediate crash. To protect users from such badness, DeviceClass member no_user used to make device models unavailable with -device / device_add, but that regressed in commit 18b6dad. The device model is still omitted from help, but is available anyway. Attempts to fix the regression have been rejected with the argument that the purpose of no_user isn't clear, and it's prone to misuse. This commit clarifies no_user's purpose. Anthony suggested to rename it cannot_instantiate_with_device_add_yet_due_to_internal_bugs, which I shorten somewhat to keep checkpatch happy. While there, make it bool. Every use of cannot_instantiate_with_device_add_yet gets a FIXME comment asking for rationale. The next few commits will clean them all up, either by providing a rationale, or by getting rid of the use. With that done, the regression fix is hopefully acceptable. Signed-off-by: Markus Armbruster arm...@redhat.com --- hw/acpi/piix4.c| 2 +- hw/alpha/typhoon.c | 2 +- hw/arm/versatilepb.c | 2 +- hw/audio/pcspk.c | 2 +- hw/audio/pl041.c | 2 +- hw/block/fdc.c | 2 +- hw/display/pl110.c | 2 +- hw/dma/pl080.c | 2 +- hw/i2c/smbus_ich9.c| 2 +- hw/i386/kvm/clock.c| 2 +- hw/i386/kvmvapic.c | 2 +- hw/i386/pc.c | 2 +- hw/ide/piix.c | 6 +++--- hw/ide/via.c | 2 +- hw/input/pckbd.c | 2 +- hw/input/vmmouse.c | 2 +- hw/intc/apic_common.c | 2 +- hw/intc/arm_gic.c | 2 +- hw/intc/arm_gic_common.c | 2 +- hw/intc/arm_gic_kvm.c | 2 +- hw/intc/i8259_common.c | 2 +- hw/intc/ioapic_common.c| 2 +- hw/intc/pl190.c| 2 +- hw/isa/isa-bus.c | 2 +- hw/isa/lpc_ich9.c | 2 +- hw/isa/piix4.c | 2 +- hw/isa/vt82c686.c | 2 +- hw/misc/arm_l2x0.c | 2 +- hw/misc/vmport.c | 2 +- hw/nvram/fw_cfg.c | 2 +- hw/pci-host/bonito.c | 4 ++-- hw/pci-host/grackle.c | 4 ++-- hw/pci-host/piix.c | 8 hw/pci-host/prep.c | 4 ++-- hw/ppc/spapr_vio.c | 2 +- hw/s390x/ipl.c | 2 +- hw/s390x/s390-virtio-bus.c | 2 +- hw/s390x/virtio-ccw.c | 2 +- hw/sd/pl181.c | 2 +- hw/timer/arm_mptimer.c | 2 +- hw/timer/hpet.c| 2 +- hw/timer/i8254_common.c| 2 +- hw/timer/m48t59.c | 2 +- hw/timer/mc146818rtc.c | 2 +- hw/timer/pl031.c | 2 +- include/hw/qdev-core.h | 13 - qdev-monitor.c | 5 +++-- qom/cpu.c | 2 +- 48 files changed, 69 insertions(+), 57 deletions(-) diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index b46bd5e..c29a703 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -508,7 +508,7 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data) k-revision = 0x03; k-class_id = PCI_CLASS_BRIDGE_OTHER; dc-desc = PM; -dc-no_user = 1; +dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ dc-vmsd = vmstate_acpi; dc-props = piix4_pm_properties; } diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c index 59e1bb8..60987ed 100644 --- a/hw/alpha/typhoon.c +++ b/hw/alpha/typhoon.c @@ -938,7 +938,7 @@ static void typhoon_pcihost_class_init(ObjectClass *klass, void *data) SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); k-init = typhoon_pcihost_init; -dc-no_user = 1; +dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ } static const TypeInfo typhoon_pcihost_info = { diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c index f7e8b7e..bb0c0ba 100644 --- a/hw/arm/versatilepb.c +++ b/hw/arm/versatilepb.c @@ -390,7 +390,7 @@ static void vpb_sic_class_init(ObjectClass *klass, void *data) SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); k-init = vpb_sic_init; -dc-no_user = 1; +dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ dc-vmsd = vmstate_vpb_sic; } diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c index 9004ce3..8e3e178 100644 --- a/hw/audio/pcspk.c +++ b/hw/audio/pcspk.c @@ -192,7 +192,7 @@ static void pcspk_class_initfn(ObjectClass *klass, void *data) dc-realize = pcspk_realizefn; set_bit(DEVICE_CATEGORY_SOUND, dc-categories); -dc-no_user = 1; +dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ dc-props = pcspk_properties; } diff --git a/hw/audio/pl041.c
[Qemu-devel] [PATCH 10/10] qdev: Do not let the user try to device_add when it cannot work
From: Markus Armbruster arm...@redhat.com Such devices have always been unavailable and omitted from the list of available devices shown by device_add help. Until commit 18b6dad silently broke the former, setting up nasty traps for unwary users, like this one: $ qemu-system-x86_64 -nodefaults -monitor stdio -display none QEMU 1.6.50 monitor - type 'help' for more information (qemu) device_add apic Segmentation fault (core dumped) I call that a regression. Fix it. Signed-off-by: Markus Armbruster arm...@redhat.com --- qdev-monitor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qdev-monitor.c b/qdev-monitor.c index 36f6f09..c538fec 100644 --- a/qdev-monitor.c +++ b/qdev-monitor.c @@ -477,7 +477,7 @@ DeviceState *qdev_device_add(QemuOpts *opts) } } -if (!obj) { +if (!obj || DEVICE_CLASS(obj)-cannot_instantiate_with_device_add_yet) { qerror_report(QERR_INVALID_PARAMETER_VALUE, driver, device type); return NULL; } -- 1.8.1.4
[Qemu-devel] [PATCH 03/10] cpu: Document why cannot_instantiate_with_device_add_yet
From: Markus Armbruster arm...@redhat.com Signed-off-by: Markus Armbruster arm...@redhat.com --- qom/cpu.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/qom/cpu.c b/qom/cpu.c index 09c15e6..6e0d54e 100644 --- a/qom/cpu.c +++ b/qom/cpu.c @@ -254,7 +254,11 @@ static void cpu_class_init(ObjectClass *klass, void *data) k-gdb_read_register = cpu_common_gdb_read_register; k-gdb_write_register = cpu_common_gdb_write_register; dc-realize = cpu_common_realizefn; -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ +/* + * Reason: CPUs still need special care by board code: wiring up + * IRQs, adding reset handlers, halting non-first CPUS, ... + */ +dc-cannot_instantiate_with_device_add_yet = true; } static const TypeInfo cpu_type_info = { -- 1.8.1.4
[Qemu-devel] [PATCH 06/10] ich9: Document why cannot_instantiate_with_device_add_yet
From: Markus Armbruster arm...@redhat.com An ICH9 southbridge contains several PCI devices, some of them with multiple functions. We model each function as a separate qdev. Two of them need some special wiring set up in pc_q35_init() to work: the LPC controller at 00:1f.0, and the SMBus controller at 00:1f.3. Signed-off-by: Markus Armbruster arm...@redhat.com --- hw/i2c/smbus_ich9.c | 6 +- hw/isa/lpc_ich9.c | 7 +-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/hw/i2c/smbus_ich9.c b/hw/i2c/smbus_ich9.c index c1ffa34..8d47eaf 100644 --- a/hw/i2c/smbus_ich9.c +++ b/hw/i2c/smbus_ich9.c @@ -97,11 +97,15 @@ static void ich9_smb_class_init(ObjectClass *klass, void *data) k-device_id = PCI_DEVICE_ID_INTEL_ICH9_6; k-revision = ICH9_A2_SMB_REVISION; k-class_id = PCI_CLASS_SERIAL_SMBUS; -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ dc-vmsd = vmstate_ich9_smbus; dc-desc = ICH9 SMBUS Bridge; k-init = ich9_smbus_initfn; k-config_write = ich9_smbus_write_config; +/* + * Reason: part of ICH9 southbridge, needs to be wired up by + * pc_q35_init() + */ +dc-cannot_instantiate_with_device_add_yet = true; } i2c_bus *ich9_smb_init(PCIBus *bus, int devfn, uint32_t smb_io_base) diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c index ad841b5..d00d698 100644 --- a/hw/isa/lpc_ich9.c +++ b/hw/isa/lpc_ich9.c @@ -604,14 +604,17 @@ static void ich9_lpc_class_init(ObjectClass *klass, void *data) dc-reset = ich9_lpc_reset; k-init = ich9_lpc_initfn; dc-vmsd = vmstate_ich9_lpc; -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ k-config_write = ich9_lpc_config_write; dc-desc = ICH9 LPC bridge; k-vendor_id = PCI_VENDOR_ID_INTEL; k-device_id = PCI_DEVICE_ID_INTEL_ICH9_8; k-revision = ICH9_A2_LPC_REVISION; k-class_id = PCI_CLASS_BRIDGE_ISA; - +/* + * Reason: part of ICH9 southbridge, needs to be wired up by + * pc_q35_init() + */ +dc-cannot_instantiate_with_device_add_yet = true; } static const TypeInfo ich9_lpc_info = { -- 1.8.1.4
[Qemu-devel] [PATCH 07/10] piix3 piix4: Clean up use of cannot_instantiate_with_device_add_yet
From: Markus Armbruster arm...@redhat.com A PIIX3/PIIX4 southbridge has multiple functions. We model each function as a separate qdev. Two of them need some special wiring set up in pc_init1() or mips_malta_init() to work: the ISA bridge at 01.0, and the SMBus controller at 01.3. The IDE controller at 01.1 (piix3-ide, piix3-ide-xen, piix4-ide) has always had cannot_instantiate_with_device_add_yet set, but there is no obvious reason why device_add could not work for them. Drop it. Signed-off-by: Markus Armbruster arm...@redhat.com --- hw/acpi/piix4.c| 7 ++- hw/ide/piix.c | 3 --- hw/isa/piix4.c | 6 +- hw/pci-host/piix.c | 12 ++-- 4 files changed, 21 insertions(+), 7 deletions(-) diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index c29a703..c0ad010 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -508,9 +508,14 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data) k-revision = 0x03; k-class_id = PCI_CLASS_BRIDGE_OTHER; dc-desc = PM; -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ +dc-cannot_instantiate_with_device_add_yet = true; dc-vmsd = vmstate_acpi; dc-props = piix4_pm_properties; +/* + * Reason: part of PIIX4 southbridge, needs to be wired up, + * e.g. by mips_malta_init() + */ +dc-cannot_instantiate_with_device_add_yet = true; } static const TypeInfo piix4_pm_info = { diff --git a/hw/ide/piix.c b/hw/ide/piix.c index 27b08e1..9b5960b 100644 --- a/hw/ide/piix.c +++ b/hw/ide/piix.c @@ -248,7 +248,6 @@ static void piix3_ide_class_init(ObjectClass *klass, void *data) k-device_id = PCI_DEVICE_ID_INTEL_82371SB_1; k-class_id = PCI_CLASS_STORAGE_IDE; set_bit(DEVICE_CATEGORY_STORAGE, dc-categories); -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ } static const TypeInfo piix3_ide_info = { @@ -267,7 +266,6 @@ static void piix3_ide_xen_class_init(ObjectClass *klass, void *data) k-device_id = PCI_DEVICE_ID_INTEL_82371SB_1; k-class_id = PCI_CLASS_STORAGE_IDE; set_bit(DEVICE_CATEGORY_STORAGE, dc-categories); -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ dc-unplug = pci_piix3_xen_ide_unplug; } @@ -289,7 +287,6 @@ static void piix4_ide_class_init(ObjectClass *klass, void *data) k-device_id = PCI_DEVICE_ID_INTEL_82371AB; k-class_id = PCI_CLASS_STORAGE_IDE; set_bit(DEVICE_CATEGORY_STORAGE, dc-categories); -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ } static const TypeInfo piix4_ide_info = { diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c index d9dac61..def6fe3 100644 --- a/hw/isa/piix4.c +++ b/hw/isa/piix4.c @@ -113,8 +113,12 @@ static void piix4_class_init(ObjectClass *klass, void *data) k-device_id = PCI_DEVICE_ID_INTEL_82371AB_0; k-class_id = PCI_CLASS_BRIDGE_ISA; dc-desc = ISA bridge; -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ dc-vmsd = vmstate_piix4; +/* + * Reason: part of PIIX4 southbridge, needs to be wired up, + * e.g. by mips_malta_init() + */ +dc-cannot_instantiate_with_device_add_yet = true; } static const TypeInfo piix4_info = { diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c index 8089fd6..1526fd4 100644 --- a/hw/pci-host/piix.c +++ b/hw/pci-host/piix.c @@ -644,7 +644,6 @@ static void piix3_class_init(ObjectClass *klass, void *data) dc-desc= ISA bridge; dc-vmsd= vmstate_piix3; -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ k-no_hotplug = 1; k-init = piix3_initfn; k-config_write = piix3_write_config; @@ -652,6 +651,11 @@ static void piix3_class_init(ObjectClass *klass, void *data) /* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */ k-device_id= PCI_DEVICE_ID_INTEL_82371SB_0; k-class_id = PCI_CLASS_BRIDGE_ISA; +/* + * Reason: part of PIIX3 southbridge, needs to be wired up by + * pc_piix.c's pc_init1() + */ +dc-cannot_instantiate_with_device_add_yet = true; } static const TypeInfo piix3_info = { @@ -668,7 +672,6 @@ static void piix3_xen_class_init(ObjectClass *klass, void *data) dc-desc= ISA bridge; dc-vmsd= vmstate_piix3; -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ k-no_hotplug = 1; k-init = piix3_initfn; k-config_write = piix3_write_config_xen; @@ -676,6 +679,11 @@ static void piix3_xen_class_init(ObjectClass *klass, void *data) /* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */ k-device_id= PCI_DEVICE_ID_INTEL_82371SB_0; k-class_id = PCI_CLASS_BRIDGE_ISA; +/* + * Reason: part of PIIX3 southbridge, needs to be wired up by + * pc_piix.c's pc_init1() + */ +dc-cannot_instantiate_with_device_add_yet = true; }; static const TypeInfo piix3_xen_info = { -- 1.8.1.4
[Qemu-devel] [PATCH 05/10] pci-host: Consistently set cannot_instantiate_with_device_add_yet
From: Markus Armbruster arm...@redhat.com Many PCI host bridges consist of a sysbus device and a PCI device. You need both for the thing to work. Arguably, these bridges should be modelled as a single, composite devices instead of pairs of seemingly independent devices you can only use together, but we're not there, yet. Since the sysbus part can't be instantiated with device_add, yet, permitting it with the PCI part is useless. We shouldn't offer useless options to the user, so let's set cannot_instantiate_with_device_add_yet for them. It's already set for Bonito, grackle, i440FX, and raven. Document why. Set it for the others: dec-21154, e500-host-bridge, gt64120_pci, pbm-pci, ppc4xx-host-bridge, sh_pci_host, u3-agp, uni-north-agp, uni-north-internal-pci, uni-north-pci, and versatile_pci_host. Signed-off-by: Markus Armbruster arm...@redhat.com --- hw/mips/gt64xxx_pci.c | 5 + hw/pci-bridge/dec.c | 5 + hw/pci-host/apb.c | 5 + hw/pci-host/bonito.c| 6 +- hw/pci-host/grackle.c | 6 +- hw/pci-host/piix.c | 6 +- hw/pci-host/ppce500.c | 5 + hw/pci-host/prep.c | 6 +- hw/pci-host/q35.c | 5 + hw/pci-host/uninorth.c | 20 hw/pci-host/versatile.c | 5 + hw/ppc/ppc4xx_pci.c | 5 + hw/sh4/sh_pci.c | 5 + 13 files changed, 80 insertions(+), 4 deletions(-) diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c index 3da2e67..4756bf1 100644 --- a/hw/mips/gt64xxx_pci.c +++ b/hw/mips/gt64xxx_pci.c @@ -1157,6 +1157,11 @@ static void gt64120_pci_class_init(ObjectClass *klass, void *data) k-device_id = PCI_DEVICE_ID_MARVELL_GT6412X; k-revision = 0x10; k-class_id = PCI_CLASS_BRIDGE_HOST; +/* + * PCI-facing part of the host bridge, not usable without the + * host-facing part, which can't be device_add'ed, yet. + */ +k-parent_class.cannot_instantiate_with_device_add_yet = true; } static const TypeInfo gt64120_pci_info = { diff --git a/hw/pci-bridge/dec.c b/hw/pci-bridge/dec.c index e5e3be8..e27ecfc 100644 --- a/hw/pci-bridge/dec.c +++ b/hw/pci-bridge/dec.c @@ -123,6 +123,11 @@ static void dec_21154_pci_host_class_init(ObjectClass *klass, void *data) k-revision = 0x02; k-class_id = PCI_CLASS_BRIDGE_PCI; k-is_bridge = 1; +/* + * PCI-facing part of the host bridge, not usable without the + * host-facing part, which can't be device_add'ed, yet. + */ +k-parent_class.cannot_instantiate_with_device_add_yet = true; } static const TypeInfo dec_21154_pci_host_info = { diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c index 92f289f..b8df0a6 100644 --- a/hw/pci-host/apb.c +++ b/hw/pci-host/apb.c @@ -521,6 +521,11 @@ static void pbm_pci_host_class_init(ObjectClass *klass, void *data) k-vendor_id = PCI_VENDOR_ID_SUN; k-device_id = PCI_DEVICE_ID_SUN_SABRE; k-class_id = PCI_CLASS_BRIDGE_HOST; +/* + * PCI-facing part of the host bridge, not usable without the + * host-facing part, which can't be device_add'ed, yet. + */ +k-parent_class.cannot_instantiate_with_device_add_yet = true; } static const TypeInfo pbm_pci_host_info = { diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c index bfb9820..902441f 100644 --- a/hw/pci-host/bonito.c +++ b/hw/pci-host/bonito.c @@ -806,8 +806,12 @@ static void bonito_class_init(ObjectClass *klass, void *data) k-revision = 0x01; k-class_id = PCI_CLASS_BRIDGE_HOST; dc-desc = Host bridge; -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ dc-vmsd = vmstate_bonito; +/* + * PCI-facing part of the host bridge, not usable without the + * host-facing part, which can't be device_add'ed, yet. + */ +dc-cannot_instantiate_with_device_add_yet = true; } static const TypeInfo bonito_info = { diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c index c178375..7d95821 100644 --- a/hw/pci-host/grackle.c +++ b/hw/pci-host/grackle.c @@ -130,7 +130,11 @@ static void grackle_pci_class_init(ObjectClass *klass, void *data) k-device_id = PCI_DEVICE_ID_MOTOROLA_MPC106; k-revision = 0x00; k-class_id = PCI_CLASS_BRIDGE_HOST; -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ +/* + * PCI-facing part of the host bridge, not usable without the + * host-facing part, which can't be device_add'ed, yet. + */ +dc-cannot_instantiate_with_device_add_yet = true; } static const TypeInfo grackle_pci_info = { diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c index 21ffe97..8089fd6 100644 --- a/hw/pci-host/piix.c +++ b/hw/pci-host/piix.c @@ -698,8 +698,12 @@ static void i440fx_class_init(ObjectClass *klass, void *data) k-revision = 0x02; k-class_id = PCI_CLASS_BRIDGE_HOST; dc-desc = Host bridge; -dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why */ dc-vmsd = vmstate_i440fx; +/* +
Re: [Qemu-devel] [PATCH v7 16/19] block: vhdx - add .bdrv_create() support
On Fri, Oct 11, 2013 at 02:17:36PM -0400, Jeff Cody wrote: +if (image_size VHDX_MAX_IMAGE_SIZE) { +fprintf(stderr, Image size too large; max of 64TB\n); Please use error_setg_errno(errp, EINVAL, Image size too large; max of 64TB) instead of fprintf(). There are other instances in this file that should be converted too.
Re: [Qemu-devel] [PATCH v7 0/3] hw/arm: Add 'virt' platform
On 15 October 2013 16:14, Tom Sutcliffe tom.sutcli...@bromium.com wrote: On 15 Oct 2013, at 16:00, Peter Maydell peter.mayd...@linaro.org wrote: On 15 October 2013 15:58, Tom Sutcliffe tom.sutcli...@bromium.com wrote: Thumbs up from me testing on Arndale. My only issue is that virt and vexpress-a15 add virtio-mmio devices in the opposite order to each other, for the same set of -device command line arguments. It would avoid future headaches if we could have these behave the same. My preference would be for the virt behaviour, as the -device order matches the order in which the guest Linux kernel adds them to /dev (for virtio-blk-devices at least). Oh yes, I'd forgotten you mentioned that. Did anybody ever track down *why* the kernel is reading the device tree backwards? Not me :) So apparently the kernel makes no guarantees at all about what order it might process the virtio-mmio transports in. This means that users mustn't rely on /dev/vda and /dev/vdb corresponding to particular virtio-blk devices on QEMU's command line -- you need to use UUIDs or something similar instead. I think this sucks, but that's the kernel for you. I'll probably change QEMU anyway, just because if there's no guarantee we might as well make qemu code do a simple forwards loop rather than a backwards one. -- PMM
Re: [Qemu-devel] [PATCH 1/1] e820: pass high memory too.
On Do, 2013-10-17 at 15:00 +0200, Andrea Arcangeli wrote: Hi, On Thu, Oct 17, 2013 at 01:09:38PM +0200, Gerd Hoffmann wrote: We have a fw_cfg entry to pass e820 entries from qemu to the firmware. Today it's used to pass reservations only. This patch makes qemu pass entries for RAM too. This allows to pass RAM sizes larger than 1TB to the firmware and it will also allow to pass non-contignous memory ramges should we decide to implement that some day, say for our virtual numa nodes. Obviously this needs some extra care to not break existing firware. SeaBIOS loads the entries and happily adds them without looking at the type. Which is problematic for memory below 4g as this will overwrite reservations added for bios memory etc. For memory above 4g it works just fine, seabios will merge the entry derived from cmos with the one loaded from fw_cfg. The reason for not fixing the cmos and defer the fixage of the 1TB boot, is to develop a better approach, and this mixture of e820 and cmos doesn't look like an improvement. The only thing it avoids is to touch seabios but it provides no benefit whatsoever if compared to fixing the cmos which looks cleaner to me than having to compute a mix of cmos and e820 in seabios (and potentially having other bioses following this mix-incomplete-API). e820 allows to pass non-contignous ram ranges to seabios (not that qemu supports that today, but when implemented the qemu/seabios interface will deal with it just fine). How you'll do that with the cmos? I thought the reason of deferring the fixage of 1TB boot to wait for a better approach and better API, I didn't think the end result had to be a mix API that adds no value. IMO e820 is better than CMOS. The premise that this will also allow to pass non-contiguous memory is partly false, as you can't use the e820 API below 4g so there's no way to create non contiguous memory with this mix-cmos-e820-API. Sure you can. Why do you think you can't? So instead of adding if (0) patches and requiring bioses to mix information from e820 maps and cmos to boot with more than 1TB, why can't simply seabios can be fixed to preserve its own reservations (fragmenting the e820 map passed by qemu) while it build the e820 map? So then we the qemu API becomes: e820_add_entry(0, ram_size, E820_RAM); That is the goal. seabios will be fixed to deal with this correctly. I don't want break old seabios versions though (especially not before we have a seabios release which can handle it), so I'll wait with flipping the switch for that. cheers, Gerd