Re: [Qemu-devel] [PATCH] kvm-unittest: fix build with gcc 4.3.X and older

2013-10-17 Thread Gleb Natapov
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

2013-10-17 Thread liu ping fan
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

2013-10-17 Thread Fam Zheng
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

2013-10-17 Thread Michael S. Tsirkin
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

2013-10-17 Thread Amos Kong
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

2013-10-17 Thread Amos Kong
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

2013-10-17 Thread Amos Kong
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

2013-10-17 Thread Stefan Hajnoczi
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()

2013-10-17 Thread Kevin Wolf
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

2013-10-17 Thread Michael S. Tsirkin
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()

2013-10-17 Thread Stefan Hajnoczi
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

2013-10-17 Thread Stefan Hajnoczi
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

2013-10-17 Thread Gerd Hoffmann
  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.

2013-10-17 Thread Richard W.M. Jones
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

2013-10-17 Thread Gleb Natapov
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

2013-10-17 Thread Stefan Hajnoczi
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

2013-10-17 Thread Michael S. Tsirkin
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

2013-10-17 Thread Michael S. Tsirkin
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

2013-10-17 Thread Amos Kong
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

2013-10-17 Thread Gleb Natapov
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

2013-10-17 Thread Amos Kong
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()

2013-10-17 Thread mike

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()

2013-10-17 Thread mike

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.

2013-10-17 Thread Peter Maydell
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

2013-10-17 Thread Alex Bennée

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

2013-10-17 Thread Michael S. Tsirkin
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

2013-10-17 Thread Gerd Hoffmann
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

2013-10-17 Thread Michael S. Tsirkin
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

2013-10-17 Thread Michael S. Tsirkin
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

2013-10-17 Thread Gleb Natapov
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

2013-10-17 Thread Alexander Graf

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

2013-10-17 Thread Michael S. Tsirkin
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

2013-10-17 Thread Gleb Natapov
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?

2013-10-17 Thread Markus Armbruster
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?

2013-10-17 Thread Markus Armbruster
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

2013-10-17 Thread Peter Maydell
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

2013-10-17 Thread Gerd Hoffmann
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

2013-10-17 Thread Gerd Hoffmann
  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

2013-10-17 Thread Gerd Hoffmann
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

2013-10-17 Thread Gerd Hoffmann
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.

2013-10-17 Thread Gerd Hoffmann
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

2013-10-17 Thread Paolo Bonzini
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

2013-10-17 Thread Paolo Bonzini
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

2013-10-17 Thread Gleb Natapov
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

2013-10-17 Thread Paolo Bonzini
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

2013-10-17 Thread Gerd Hoffmann
  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

2013-10-17 Thread Gerd Hoffmann
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

2013-10-17 Thread Gerd Hoffmann
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

2013-10-17 Thread Paolo Bonzini
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.

2013-10-17 Thread Gerd Hoffmann
  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.

2013-10-17 Thread Gerd Hoffmann
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

2013-10-17 Thread Gerd Hoffmann
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

2013-10-17 Thread Paolo Bonzini
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

2013-10-17 Thread Paolo Bonzini
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

2013-10-17 Thread Bohai (ricky)
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

2013-10-17 Thread Stefan Hajnoczi
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

2013-10-17 Thread Michael S. Tsirkin
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

2013-10-17 Thread Michael S. Tsirkin
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

2013-10-17 Thread Peter Maydell
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

2013-10-17 Thread Stefan Hajnoczi
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

2013-10-17 Thread Peter Maydell
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

2013-10-17 Thread Peter Maydell
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

2013-10-17 Thread Christophe Fergeau
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

2013-10-17 Thread Peter Maydell
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

2013-10-17 Thread Peter Maydell
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

2013-10-17 Thread Alexander Graf

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

2013-10-17 Thread Stefan Hajnoczi
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

2013-10-17 Thread Mark Cave-Ayland

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

2013-10-17 Thread Alexander Graf

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?

2013-10-17 Thread Alexey Kardashevskiy
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

2013-10-17 Thread Stefan Hajnoczi
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

2013-10-17 Thread Stefan Hajnoczi
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

2013-10-17 Thread Stefan Hajnoczi
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?

2013-10-17 Thread Paolo Bonzini
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

2013-10-17 Thread Stefan Hajnoczi
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.

2013-10-17 Thread Andrea Arcangeli
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

2013-10-17 Thread Kevin Wolf
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?

2013-10-17 Thread Alexander Graf

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

2013-10-17 Thread Stefan Hajnoczi
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?

2013-10-17 Thread Alexey Kardashevskiy
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

2013-10-17 Thread Richard Henderson
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

2013-10-17 Thread Luiz Capitulino
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

2013-10-17 Thread Stefan Hajnoczi
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?

2013-10-17 Thread Alexey Kardashevskiy
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?

2013-10-17 Thread Paolo Bonzini
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?

2013-10-17 Thread Paolo Bonzini
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

2013-10-17 Thread armbru
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

2013-10-17 Thread armbru
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

2013-10-17 Thread armbru
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

2013-10-17 Thread armbru
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

2013-10-17 Thread armbru
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

2013-10-17 Thread armbru
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

2013-10-17 Thread armbru
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

2013-10-17 Thread armbru
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

2013-10-17 Thread armbru
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

2013-10-17 Thread armbru
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

2013-10-17 Thread armbru
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

2013-10-17 Thread Stefan Hajnoczi
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

2013-10-17 Thread Peter Maydell
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.

2013-10-17 Thread Gerd Hoffmann
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






  1   2   >