Re: [Qemu-devel] [PATCH] pc: Fix prog_if typo on PC_COMPAT_2_0

2014-06-25 Thread Michael S. Tsirkin
On Wed, Jun 25, 2014 at 09:00:59AM +0300, Michael S. Tsirkin wrote:
 On Tue, Jun 24, 2014 at 07:57:55PM -0300, Eduardo Habkost wrote:
  The property name is prog_if, not prof_if.
  
  Signed-off-by: Eduardo Habkost ehabk...@redhat.com
  Cc: BALATON Zoltan bala...@eik.bme.hu
 
 Should be reported-by, by the way.
 Cc: means I have notified this person just in case.

Fixed up on my branch no need to repost.

  ---
   include/hw/i386/pc.h | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
  
  diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
  index 486e98f..1331d5a 100644
  --- a/include/hw/i386/pc.h
  +++ b/include/hw/i386/pc.h
  @@ -358,7 +358,7 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t 
  *);
   },\
   {\
   .driver   = pci-serial-2x,\
  -.property = prof_if,\
  +.property = prog_if,\
   .value= stringify(0),\
   },\
   {\
  -- 
  1.9.3



Re: [Qemu-devel] [PATCH] pc: Fix prog_if typo on PC_COMPAT_2_0

2014-06-25 Thread Michael S. Tsirkin
On Tue, Jun 24, 2014 at 07:57:55PM -0300, Eduardo Habkost wrote:
 The property name is prog_if, not prof_if.
 
 Signed-off-by: Eduardo Habkost ehabk...@redhat.com
 Cc: BALATON Zoltan bala...@eik.bme.hu

Should be reported-by, by the way.
Cc: means I have notified this person just in case.

 ---
  include/hw/i386/pc.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
 index 486e98f..1331d5a 100644
 --- a/include/hw/i386/pc.h
 +++ b/include/hw/i386/pc.h
 @@ -358,7 +358,7 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t 
 *);
  },\
  {\
  .driver   = pci-serial-2x,\
 -.property = prof_if,\
 +.property = prog_if,\
  .value= stringify(0),\
  },\
  {\
 -- 
 1.9.3



Re: [Qemu-devel] [PATCH 2.1 34/36] qemu-char: make writes thread-safe

2014-06-25 Thread Stefan Weil
Am 18.06.2014 08:43, schrieb Paolo Bonzini:
 This will let threads other than the I/O thread raise QMP events.
 
 GIOChannel is thread-safe, and send and receive state is usually
 well-separated.  The only driver that requires some care is the
 pty driver, where some of the state is shared by the read and write
 sides.  That state is protected with the chr_write_lock too.
 
 Reviewed-by: Fam Zheng f...@redhat.com
 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
  include/sysemu/char.h | 11 ++
  qemu-char.c   | 58 
 ---
  2 files changed, 53 insertions(+), 16 deletions(-)
 

Latest QEMU is broken for w64. This command no longer starts the GTK gui:

wine64 x86_64-softmmu/qemu-system-x86_64 -L pc-bios

Wine reports a dead lock:

err:ntdll:RtlpWaitForCriticalSection section 0x964f0 ? wait timed out
in thread 0009, blocked by , retrying (60 sec)

git bisect finds this commit which introduced that regression:

9005b2a7589540a3733b3abdcfbccfe7746cd1a1 is the first bad commit
commit 9005b2a7589540a3733b3abdcfbccfe7746cd1a1
Author: Paolo Bonzini pbonz...@redhat.com
Date:   Wed Jun 18 08:43:58 2014 +0200

qemu-char: make writes thread-safe

This will let threads other than the I/O thread raise QMP events.

GIOChannel is thread-safe, and send and receive state is usually
well-separated.  The only driver that requires some care is the
pty driver, where some of the state is shared by the read and write
sides.  That state is protected with the chr_write_lock too.

Reviewed-by: Fam Zheng f...@redhat.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com

:04 04 0a8837feb639c89178d4d9fbdfa10e26dd702b40
5be463e0d46eb49e65a2b3e11deb494b63975906 M  include
:100644 100644 9961b02ce607764a3d8ea2d70353036b95f955f2
a9025e642e779c2b2ad0c462ae863d761365b35d M  qemu-char.c

I did not run tests with 32 bit versions, nor did I run a native test on
Windows.

Regards
Stefan




Re: [Qemu-devel] [PATCH v10 5/6] raw-posix: Add full image preallocation option

2014-06-25 Thread Hu Tao
On Sat, Jun 14, 2014 at 09:38:30PM +0200, Max Reitz wrote:
 On 12.06.2014 05:54, Hu Tao wrote:
 This patch adds a new option preallocation for raw format, and implements
 full preallocation.
 
 Signed-off-by: Hu Tao hu...@cn.fujitsu.com
 ---
   block/raw-posix.c | 59 
  ---
   1 file changed, 52 insertions(+), 7 deletions(-)
 
 diff --git a/block/raw-posix.c b/block/raw-posix.c
 index 35057f0..73b10cd 100644
 --- a/block/raw-posix.c
 +++ b/block/raw-posix.c
 @@ -30,6 +30,7 @@
   #include block/thread-pool.h
   #include qemu/iov.h
   #include raw-aio.h
 +#include qapi/util.h
   #if defined(__APPLE__)  (__MACH__)
   #include paths.h
 @@ -1279,6 +1280,8 @@ static int raw_create(const char *filename, 
 QEMUOptionParameter *options,
   int fd;
   int result = 0;
   int64_t total_size = 0;
 +PreallocMode prealloc = PREALLOC_MODE_OFF;
 +Error *error = NULL;
   strstart(filename, file:, filename);
 @@ -1286,6 +1289,14 @@ static int raw_create(const char *filename, 
 QEMUOptionParameter *options,
   while (options  options-name) {
   if (!strcmp(options-name, BLOCK_OPT_SIZE)) {
   total_size = ROUND_UP(options-value.n, BDRV_SECTOR_SIZE);
 +} else if (!strcmp(options-name, BLOCK_OPT_PREALLOC)) {
 +prealloc = qapi_enum_parse(PreallocMode_lookup, 
 options-value.s,
 +   PREALLOC_MODE_MAX, PREALLOC_MODE_OFF,
 +   error);
 +if (error) {
 +error_propagate(errp, error);
 +return -EINVAL;
 
 Could be result = -EINVAL; goto out; instead, but definitely isn't
 wrong this way.
 
 +}
   }
   options++;
   }
 @@ -1295,16 +1306,45 @@ static int raw_create(const char *filename, 
 QEMUOptionParameter *options,
   if (fd  0) {
   result = -errno;
   error_setg_errno(errp, -result, Could not create file);
 -} else {
 -if (ftruncate(fd, total_size) != 0) {
 -result = -errno;
 -error_setg_errno(errp, -result, Could not resize file);
 +goto out;
 +}
 +if (ftruncate(fd, total_size) != 0) {
 +result = -errno;
 +error_setg_errno(errp, -result, Could not resize file);
 +goto out_close;
 +}
 +if (prealloc == PREALLOC_MODE_METADATA) {
 +/* posix_fallocate() doesn't set errno. */
 +result = -posix_fallocate(fd, 0, total_size);
 +if (result != 0) {
 +error_setg_errno(errp, -result,
 + Could not preallocate data for the new file);
   }
 -if (qemu_close(fd) != 0) {
 -result = -errno;
 -error_setg_errno(errp, -result, Could not close the new file);
 +} else if (prealloc == PREALLOC_MODE_FULL) {
 +char *buf = g_malloc0(65536);
 +int64_t num = 0, left = total_size;
 +
 +while (left  0) {
 +num = MIN(left, 65536);
 +result = write(fd, buf, num);
 +if (result  0) {
 +result = -errno;
 +error_setg_errno(errp, -result,
 + Could not write to the new file);
 +g_free(buf);
 +goto out_close;
 +}
 +left -= num;
   }
 
 Technically, a raw file does not have any metadata, therefore
 preallocation=metadata is a bit ambiguous. I'd accept both
 allocating nothing and allocating everything; you chose the latter,
 which is fine.

qcow2's behaviour of metadata preallocation depends on raw's, this is
why I chose the latter here.

 
 However, why do you implement the preallocation differently for
 preallocation=full, then? posix_fallocate() does not seem to change
 the contents of the areas which were newly allocated; and the
 ftruncate() before made sure they are read back as zeroes.
 Therefore, using ftruncate() and then posix_fallocate() seems to be
 functionally equivalent to writing zeroes.

The difference is posix_fallocate() reserves space but ftruncate()
doesn't, as said in man page:

  After a successful call to posix_fallocate(), subsequent writes
  to bytes in the specified range are guaranteed not to fail because
  of lack of disk space.

however, posix_fallocate() doesn't guarantee this in other cases like
thin provisioning, this is why preallocation=metadata is different than
preallocation=full.

Hu




Re: [Qemu-devel] Live Migration with different block devices

2014-06-25 Thread Paolo Bonzini

Il 25/06/2014 02:08, Xiongzi Ge ha scritto:

Will the cache data in the guest be migrated to the other host?


Again, cache data is _not_ managed by QEMU.  It is managed by the guest 
kernel.  Its content and state is migrated together with the rest of RAM.


Paolo



Re: [Qemu-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support

2014-06-25 Thread Paolo Bonzini

Il 25/06/2014 04:17, Tiejun Chen ha scritto:

* Don't set that ISA class property, instead, just fake this ISA bridge
  with 00:1f.0.


How are you going to make this work for Q35 or another PCIe machine that 
already has an ISA bridge at 00:1f.0?


Paolo



Re: [Qemu-devel] [PATCH 3/4] machine: Introduce QEMU_COMPAT_* macros

2014-06-25 Thread Igor Mammedov
On Tue, 24 Jun 2014 22:20:50 +0300
Marcel Apfelbaum marce...@redhat.com wrote:

 On Tue, 2014-06-24 at 15:02 -0300, Eduardo Habkost wrote:
  The QEMU_COMPAT_* macros will contain compat properties that are not
  specific to PC, and may be reused by other machine-types.
  
  PC-specific properties were left on the PC_COMPAT_* macros.
  
  Signed-off-by: Eduardo Habkost ehabk...@redhat.com
  ---
   include/hw/boards.h  | 161 
  +++
   include/hw/i386/pc.h | 150 ++-
   2 files changed, 166 insertions(+), 145 deletions(-)
  
  diff --git a/include/hw/boards.h b/include/hw/boards.h
  index 605a970..709b582 100644
  --- a/include/hw/boards.h
  +++ b/include/hw/boards.h
  @@ -134,4 +134,165 @@ struct MachineState {
   const char *cpu_model;
   };
   
  +
  +/* Macros for compat_props corresponding to specific QEMU versions: */
 Only a suggestion: Maybe we can move all the compat props to a new header 
 file,
 say include/hw/compat.h so we don't need to modify board.h every time we need 
 a
 compat prop? board.h will remain cleaner this way.
these are PC specific compat props, so it should be moved into target specific
pc.h instead of global header.

 Thanks,
 Marcel
 
  +
  +#define QEMU_COMPAT_2_0 \
  +{\
  +.driver   = virtio-scsi-pci,\
  +.property = any_layout,\
  +.value= off,\
  +},\
  +{\
  +.driver   = apic,\
  +.property = version,\
  +.value= stringify(0x11),\
  +},\
  +{\
  +.driver   = nec-usb-xhci,\
  +.property = superspeed-ports-first,\
  +.value= off,\
  +},\
  +{\
  +.driver   = pci-serial,\
  +.property = prog_if,\
  +.value= stringify(0),\
  +},\
  +{\
  +.driver   = pci-serial-2x,\
  +.property = prof_if,\
  +.value= stringify(0),\
  +},\
  +{\
  +.driver   = pci-serial-4x,\
  +.property = prog_if,\
  +.value= stringify(0),\
  +},\
  +{\
  +.driver   = virtio-net-pci,\
  +.property = guest_announce,\
  +.value= off,\
  +}
  +
  +#define QEMU_COMPAT_1_7 \
  +{\
  +.driver   = TYPE_USB_DEVICE,\
  +.property = msos-desc,\
  +.value= no,\
  +}
  +
  +#define QEMU_COMPAT_1_6 \
  +{\
  +.driver   = e1000,\
  +.property = mitigation,\
  +.value= off,\
  +},{\
  +.driver   = qemu64- TYPE_X86_CPU,\
  +.property = model,\
  +.value= stringify(2),\
  +},{\
  +.driver   = qemu32- TYPE_X86_CPU,\
  +.property = model,\
  +.value= stringify(3),\
  +}
  +
  +#define QEMU_COMPAT_1_5 \
  +{\
  +.driver   = Conroe- TYPE_X86_CPU,\
  +.property = model,\
  +.value= stringify(2),\
  +},{\
  +.driver   = Conroe- TYPE_X86_CPU,\
  +.property = level,\
  +.value= stringify(2),\
  +},{\
  +.driver   = Penryn- TYPE_X86_CPU,\
  +.property = model,\
  +.value= stringify(2),\
  +},{\
  +.driver   = Penryn- TYPE_X86_CPU,\
  +.property = level,\
  +.value= stringify(2),\
  +},{\
  +.driver   = Nehalem- TYPE_X86_CPU,\
  +.property = model,\
  +.value= stringify(2),\
  +},{\
  +.driver   = Nehalem- TYPE_X86_CPU,\
  +.property = level,\
  +.value= stringify(2),\
  +},{\
  +.driver   = virtio-net-pci,\
  +.property = any_layout,\
  +.value= off,\
  +},{\
  +.driver = TYPE_X86_CPU,\
  +.property = pmu,\
  +.value = on,\
  +}
  +
  +#define QEMU_COMPAT_1_4 \
  +{\
  +.driver   = scsi-hd,\
  +.property = discard_granularity,\
  +.value= stringify(0),\
  +},{\
  +.driver   = scsi-cd,\
  +.property = discard_granularity,\
  +.value= stringify(0),\
  +},{\
  +.driver   = scsi-disk,\
  +.property = discard_granularity,\
  +.value= stringify(0),\
  +},{\
  +.driver   = ide-hd,\
  +.property = discard_granularity,\
  +.value= stringify(0),\
  +},{\
  +.driver   = ide-cd,\
  +.property = discard_granularity,\
  +.value= stringify(0),\
  +},{\
  +.driver   = ide-drive,\
  +.property = discard_granularity,\
  +.value= stringify(0),\
  +},{\
  +.driver   = virtio-blk-pci,\
  +.property = discard_granularity,\
  +.value= stringify(0),\
  +},{\
  +.driver   = virtio-serial-pci,\
  +.property = vectors,\
  +/* DEV_NVECTORS_UNSPECIFIED as a uint32_t string */\
  +.value= stringify(0x),\
  +},{ \
  +.driver   = virtio-net-pci, \
  +

Re: [Qemu-devel] [v5][PATCH 1/5] xen, gfx passthrough: basic graphics passthrough support

2014-06-25 Thread Paolo Bonzini

Il 25/06/2014 04:17, Tiejun Chen ha scritto:

+static int get_vgabios(unsigned char *buf, XenHostPCIDevice *dev)
+{
+char rom_file[64];
+FILE *fp;
+uint8_t val;
+struct stat st;
+uint16_t magic = 0;
+int ret = 0;
+
+snprintf(rom_file, sizeof(rom_file),
+ /sys/bus/pci/devices/%04x:%02x:%02x.%d/rom,
+ dev-domain, dev-bus, dev-dev,
+ dev-func);
+
+if (stat(rom_file, st)) {
+return -ENODEV;
+}
+
+if (access(rom_file, F_OK)) {
+XEN_PT_ERR(NULL, pci-assign: Insufficient privileges for %s,
+rom_file);
+return -ENODEV;
+}


These two checks (stat and access) are useless.


+/* Write 1 to the ROM file to enable it */
+fp = fopen(rom_file, r+);
+if (fp == NULL) {
+return -EACCES;
+}
+val = 1;
+if (fwrite(val, 1, 1, fp) != 1) {
+XEN_PT_LOG(%s\n, Failed to enable pci-sysfs rom file);
+ret = -EIO;
+goto close_rom;
+}
+fseek(fp, 0, SEEK_SET);
+
+/*
+ * Check if it a real bios extension.
+ * The magic number is 0xAA55.
+ */
+if (!fread(magic, sizeof(magic), 1, fp)) {
+XEN_PT_ERR(NULL, VGA: can't get magic.\n);
+ret = -ENODEV;
+goto close_rom;
+}
+if (magic != 0xAA55) {
+XEN_PT_ERR(NULL, VGA: wrong magic %x.\n, magic);
+ret = -ENODEV;
+goto close_rom;
+}
+fseek(fp, 0, SEEK_SET);
+
+if (!fread(buf, 1, st.st_size, fp)) {
+XEN_PT_ERR(NULL, VGA: pci-assign: Cannot read from host %s, 
rom_file);
+XEN_PT_LOG(NULL, VGA: Device option ROM contents are probably invalid 

+ (check dmesg).\nSkip option ROM probe with rombar=0, 
+ or load from file with romfile=\n);
+}
+


Why is this graphics passthrough specific?  Should QEMU's Xen PCI 
passthrough read ROM BAR contents from network cards too, for example?


Paolo



Re: [Qemu-devel] [v5][PATCH 2/5] xen, gfx passthrough: create pseudo intel isa bridge

2014-06-25 Thread Paolo Bonzini

Il 25/06/2014 04:17, Tiejun Chen ha scritto:

+static int create_pseudo_pch_isa_bridge(PCIBus *bus, XenHostPCIDevice *hdev)
+{
+struct PCIDevice *dev;
+
+char rid;
+
+/* We havt to use a simple PCI device to fake this ISA bridge
+ * to avoid making some confusion to BIOS and ACPI.
+ */
+dev = pci_create(bus, PCI_DEVFN(0x1f, 0), pseudo-intel-pch-isa-bridge);
+
+qdev_init_nofail(dev-qdev);
+
+pci_config_set_vendor_id(dev-config, hdev-vendor_id);
+pci_config_set_device_id(dev-config, hdev-device_id);
+
+xen_host_pci_get_block(hdev, PCI_REVISION_ID, (uint8_t *)rid, 1);
+
+pci_config_set_revision(dev-config, rid);
+
+XEN_PT_LOG(dev, The pseudo Intel PCH ISA bridge created.\n);
+return 0;
+}


This patch doesn't compile on its own (this static function is unused).

pci_create_pch should be moved in this patch.

Paolo



Re: [Qemu-devel] [v5][PATCH 4/5] xen, gfx passthrough: create host bridge to passthrough

2014-06-25 Thread Paolo Bonzini

Il 25/06/2014 04:17, Tiejun Chen ha scritto:

+if (xen_enabled()  xen_has_gfx_passthru) {
+d = pci_create_simple(b, 0, TYPE_I440FX_XEN_PCI_DEVICE);
+*pi440fx_state = I440FX_XEN_PCI_DEVICE(d);
+pci_create_pch(b);
+} else {
+d = pci_create_simple(b, 0, TYPE_I440FX_PCI_DEVICE);
+*pi440fx_state = I440FX_PCI_DEVICE(d);
+}


As mentioned in the review of v4, this should be a separate, 
Xen-specific machine.  pci_create_pch should not be called in generic PC 
code.


Paolo



Re: [Qemu-devel] [v5][PATCH 3/5] xen, gfx passthrough: support Intel IGD passthrough with VT-D

2014-06-25 Thread Paolo Bonzini

Il 25/06/2014 04:17, Tiejun Chen ha scritto:

+int pci_create_pch(PCIBus *bus)
+{
+XenHostPCIDevice hdev;
+int r = 0;
+
+if (!xen_has_gfx_passthru) {
+return r;
+}
+


You could make this an assertion, since the function is never called 
with xen_has_gfx_passthru == 0.  Or just drop the check completely, so 
the function can be moved in patch 2.


Paolo



Re: [Qemu-devel] [v5][PATCH 1/5] xen, gfx passthrough: basic graphics passthrough support

2014-06-25 Thread Michael S. Tsirkin
On Wed, Jun 25, 2014 at 10:17:17AM +0800, Tiejun Chen wrote:
 basic gfx passthrough support:
 - add a vga type for gfx passthrough
 - retrieve VGA bios from sysfs, then load it to guest at 0xC
 - register/unregister legacy VGA I/O ports and MMIOs for passthrough GFX
 
 The original patch is from Weidong Han weidong@intel.com
 
 Signed-off-by: Yang Zhang yang.z.zh...@intel.com
 Signed-off-by: Tiejun Chen tiejun.c...@intel.com
 Cc: Weidong Han weidong@intel.com

ROM loading code is duplicated from assigned_dev_load_option_rom.
Would it be a problem for you to create a memory region holding
the ROM?
You won't need cpu_physical_memory_rw then, either, or need
the VGA_BIOS_DEFAULT_SIZE hack.




 ---
 v5:
 
 * Just rebase.
 
 v4:
 
 * Fix some typos in the patch head description.
 * Improve some comments.
 * Given that xen_pt_register_vga_regions()/xen_pt_unregister_vga_regions()
   are called unconditionally, so we just return 0 there.
 * Remove one spurious change.
 
 v3:
 
 * Fix some typos.
 * Add more comments to make that readable.
 * Improve some return paths.
 
 v2:
 
 * retrieve VGA bios from sysfs properly.
 * redefine some function name.
 
  hw/xen/Makefile.objs |   2 +-
  hw/xen/xen-host-pci-device.c |   5 +
  hw/xen/xen-host-pci-device.h |   1 +
  hw/xen/xen_pt.c  |  10 ++
  hw/xen/xen_pt.h  |   4 +
  hw/xen/xen_pt_graphics.c | 232 
 +++
  qemu-options.hx  |   9 ++
  vl.c |  10 ++
  8 files changed, 272 insertions(+), 1 deletion(-)
  create mode 100644 hw/xen/xen_pt_graphics.c
 
 diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs
 index a0ca0aa..77b7dae 100644
 --- a/hw/xen/Makefile.objs
 +++ b/hw/xen/Makefile.objs
 @@ -2,4 +2,4 @@
  common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o
  
  obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o
 -obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o 
 xen_pt_msi.o
 +obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o 
 xen_pt_msi.o xen_pt_graphics.o
 diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
 index 743b37b..a54b7de 100644
 --- a/hw/xen/xen-host-pci-device.c
 +++ b/hw/xen/xen-host-pci-device.c
 @@ -376,6 +376,11 @@ int xen_host_pci_device_get(XenHostPCIDevice *d, 
 uint16_t domain,
  goto error;
  }
  d-irq = v;
 +rc = xen_host_pci_get_hex_value(d, class, v);
 +if (rc) {
 +goto error;
 +}
 +d-class_code = v;
  d-is_virtfn = xen_host_pci_dev_is_virtfn(d);
  
  return 0;
 diff --git a/hw/xen/xen-host-pci-device.h b/hw/xen/xen-host-pci-device.h
 index c2486f0..f1e1c30 100644
 --- a/hw/xen/xen-host-pci-device.h
 +++ b/hw/xen/xen-host-pci-device.h
 @@ -25,6 +25,7 @@ typedef struct XenHostPCIDevice {
  
  uint16_t vendor_id;
  uint16_t device_id;
 +uint32_t class_code;
  int irq;
  
  XenHostPCIIORegion io_regions[PCI_NUM_REGIONS - 1];
 diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
 index be4220b..dac4238 100644
 --- a/hw/xen/xen_pt.c
 +++ b/hw/xen/xen_pt.c
 @@ -450,6 +450,7 @@ static int xen_pt_register_regions(XenPCIPassthroughState 
 *s)
 d-rom.size, d-rom.base_addr);
  }
  
 +xen_pt_register_vga_regions(d);
  return 0;
  }
  
 @@ -470,6 +471,8 @@ static void 
 xen_pt_unregister_regions(XenPCIPassthroughState *s)
  if (d-rom.base_addr  d-rom.size) {
  memory_region_destroy(s-rom);
  }
 +
 +xen_pt_unregister_vga_regions(d);
  }
  
  /* region mapping */
 @@ -693,6 +696,13 @@ static int xen_pt_initfn(PCIDevice *d)
  /* Handle real device's MMIO/PIO BARs */
  xen_pt_register_regions(s);
  
 +/* Setup VGA bios for passthrough GFX */
 +if (xen_pt_setup_vga(s-real_device)  0) {
 +XEN_PT_ERR(d, Setup VGA BIOS of passthrough GFX failed!\n);
 +xen_host_pci_device_put(s-real_device);
 +return -1;
 +}
 +
  /* reinitialize each config register to be emulated */
  if (xen_pt_config_init(s)) {
  XEN_PT_ERR(d, PCI Config space initialisation failed.\n);
 diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
 index 942dc60..4d3a18d 100644
 --- a/hw/xen/xen_pt.h
 +++ b/hw/xen/xen_pt.h
 @@ -298,5 +298,9 @@ static inline bool 
 xen_pt_has_msix_mapping(XenPCIPassthroughState *s, int bar)
  return s-msix  s-msix-bar_index == bar;
  }
  
 +extern int xen_has_gfx_passthru;
 +int xen_pt_register_vga_regions(XenHostPCIDevice *dev);
 +int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev);
 +int xen_pt_setup_vga(XenHostPCIDevice *dev);
  
  #endif /* !XEN_PT_H */
 diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
 new file mode 100644
 index 000..461e526
 --- /dev/null
 +++ b/hw/xen/xen_pt_graphics.c
 @@ -0,0 +1,232 @@
 +/*
 + * graphics passthrough
 + */
 +#include xen_pt.h
 +#include xen-host-pci-device.h
 +#include hw/xen/xen_backend.h
 +
 +static int 

[Qemu-devel] vmstate: struct (VMS_STRUCT) migration

2014-06-25 Thread Alexey Kardashevskiy
Hi!

VMStateDescription supports enclosed VMStateDescription's via .vmsd. This
is used in multiple places and VMStateDescription definitions look the same
way - name, version_id, minimum_version_id, etc.

QEMU handles first level VMStateDescription and enclosed VMStateDescription
slightly different - for the latter it ignores the version_id from the
source and always uses the version_id on the destination side so version
fields are useless (code is below).

Is that by design? Or a bug? I cannot see how we could fix it without
breaking backward compatibility but I am sure the community does know that :)

If this is by design, it probably makes sense to remove unused
version_id/minimum_version_id fields completely for vmsd's which we know
are enclosed (or embedded?).

What do I miss here? Thanks!



int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
   void *opaque, int version_id)
{
VMStateField *field = vmsd-fields;
int ret;

if (version_id  vmsd-version_id) {
return -EINVAL;
}
if  (version_id  vmsd-minimum_version_id) {
if (vmsd-load_state_old 
version_id = vmsd-minimum_version_id_old) {
return vmsd-load_state_old(f, opaque, version_id);
}
return -EINVAL;
}
if (vmsd-pre_load) {
int ret = vmsd-pre_load(opaque);
if (ret) {
return ret;
}
}
while (field-name) {
if ((field-field_exists 
 field-field_exists(opaque, version_id)) ||
(!field-field_exists 
 field-version_id = version_id)) {
void *base_addr = vmstate_base_addr(opaque, field, true);
int i, n_elems = vmstate_n_elems(opaque, field);
int size = vmstate_size(opaque, field);

for (i = 0; i  n_elems; i++) {
void *addr = base_addr + size * i;

if (field-flags  VMS_ARRAY_OF_POINTER) {
addr = *(void **)addr;
}
if (field-flags  VMS_STRUCT) {
ret = vmstate_load_state(f, field-vmsd, addr,
 field-vmsd-version_id);



-- 
Alexey



Re: [Qemu-devel] [v5][PATCH 2/5] xen, gfx passthrough: create pseudo intel isa bridge

2014-06-25 Thread Michael S. Tsirkin
On Wed, Jun 25, 2014 at 10:17:18AM +0800, Tiejun Chen wrote:
 ISA bridge is needed since Intel gfx drive will probe with Dev31:Fun0
 to make graphics device passthrough work well for VMM, that only need
 to expose this pseudo ISA bridge to let driver know the real hardware
 underneath.
 
 The original patch is from Allen Kay allen.m@intel.com
 
 Signed-off-by: Yang Zhang yang.z.zh...@intel.com
 Signed-off-by: Tiejun Chen tiejun.c...@intel.com
 Cc: Allen Kay allen.m@intel.com
 ---
 v5:
 
 * Don't set this ISA class property, instead, just fake this ISA bridge
   with 00:1f.0. 
 
 v4:
 
 * Remove some unnecessary return in void foo().
 
 v3:
 
 * Fix some typos.
 * Improve some return paths.
 
 v2:
 
 * Nothing is changed.
 
  hw/xen/xen_pt_graphics.c | 61 
 
  1 file changed, 61 insertions(+)
 
 diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
 index 461e526..974b7e9 100644
 --- a/hw/xen/xen_pt_graphics.c
 +++ b/hw/xen/xen_pt_graphics.c
 @@ -230,3 +230,64 @@ out:
  g_free(bios);
  return rc;
  }
 +
 +static uint32_t isa_bridge_read_config(PCIDevice *d, uint32_t addr, int len)
 +{
 +return pci_default_read_config(d, addr, len);
 +}
 +
 +static void isa_bridge_write_config(PCIDevice *d, uint32_t addr, uint32_t v,
 +int len)
 +{
 +pci_default_write_config(d, addr, v, len);
 +}
 +
 +static void isa_bridge_class_init(ObjectClass *klass, void *data)
 +{
 +PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 +
 +k-config_read = isa_bridge_read_config;
 +k-config_write = isa_bridge_write_config;
 +};

You don't need these stubs, just don't fill anything,
pci core will use defaults then.

 +
 +typedef struct {
 +PCIDevice dev;
 +} ISABridgeState;
 +

Nor do you need an empty structure if it has no state.

 +static TypeInfo isa_bridge_info = {
 +.name  = pseudo-intel-pch-isa-bridge,
 +.parent= TYPE_PCI_DEVICE,
 +.instance_size = sizeof(ISABridgeState),
 +.class_init = isa_bridge_class_init,
 +};
 +
 +static void xen_pt_graphics_register_types(void)
 +{
 +type_register_static(isa_bridge_info);
 +}
 +
 +type_init(xen_pt_graphics_register_types)
 +
 +static int create_pseudo_pch_isa_bridge(PCIBus *bus, XenHostPCIDevice *hdev)
 +{
 +struct PCIDevice *dev;
 +
 +char rid;
 +
 +/* We havt to use a simple PCI device to fake this ISA bridge
 + * to avoid making some confusion to BIOS and ACPI.
 + */

A typo and confusing wording above, I'm not really
sure what this comment means.
Maybe:

/* Create a fake ISA bridge device at the location expected by guests. */


 +dev = pci_create(bus, PCI_DEVFN(0x1f, 0), pseudo-intel-pch-isa-bridge);
 +
 +qdev_init_nofail(dev-qdev);
 +
 +pci_config_set_vendor_id(dev-config, hdev-vendor_id);
 +pci_config_set_device_id(dev-config, hdev-device_id);
 +
 +xen_host_pci_get_block(hdev, PCI_REVISION_ID, (uint8_t *)rid, 1);

Host PCI device is the VGA card?
So why does it make sense to get it's vendor/device/revision and
stick in the ISA bridge?

Also change rid to uint8_t, you won't need to cast then.

 +
 +pci_config_set_revision(dev-config, rid);
 +
 +XEN_PT_LOG(dev, The pseudo Intel PCH ISA bridge created.\n);
 +return 0;
 +}
 -- 
 1.9.1



Re: [Qemu-devel] [PATCH 3/4] machine: Introduce QEMU_COMPAT_* macros

2014-06-25 Thread Marcel Apfelbaum
On Wed, 2014-06-25 at 08:20 +0300, Michael S. Tsirkin wrote:
 On Tue, Jun 24, 2014 at 03:02:03PM -0300, Eduardo Habkost wrote:
  The QEMU_COMPAT_* macros will contain compat properties that are not
  specific to PC, and may be reused by other machine-types.
  
  PC-specific properties were left on the PC_COMPAT_* macros.
  
  Signed-off-by: Eduardo Habkost ehabk...@redhat.com
  ---
   include/hw/boards.h  | 161 
  +++
   include/hw/i386/pc.h | 150 ++-
   2 files changed, 166 insertions(+), 145 deletions(-)
  
  diff --git a/include/hw/boards.h b/include/hw/boards.h
  index 605a970..709b582 100644
  --- a/include/hw/boards.h
  +++ b/include/hw/boards.h
  @@ -134,4 +134,165 @@ struct MachineState {
   const char *cpu_model;
   };
   
  +
  +/* Macros for compat_props corresponding to specific QEMU versions: */
  +
  +#define QEMU_COMPAT_2_0 \
  +{\
  +.driver   = virtio-scsi-pci,\
  +.property = any_layout,\
  +.value= off,\
  +},\
  +{\
  +.driver   = apic,\
  +.property = version,\
  +.value= stringify(0x11),\
  +},\
  +{\
  +.driver   = nec-usb-xhci,\
  +.property = superspeed-ports-first,\
  +.value= off,\
  +},\
  +{\
  +.driver   = pci-serial,\
  +.property = prog_if,\
  +.value= stringify(0),\
  +},\
  +{\
  +.driver   = pci-serial-2x,\
  +.property = prof_if,\
  +.value= stringify(0),\
  +},\
  +{\
  +.driver   = pci-serial-4x,\
  +.property = prog_if,\
  +.value= stringify(0),\
  +},\
  +{\
  +.driver   = virtio-net-pci,\
  +.property = guest_announce,\
  +.value= off,\
  +}
  +
  +#define QEMU_COMPAT_1_7 \
  +{\
  +.driver   = TYPE_USB_DEVICE,\
  +.property = msos-desc,\
  +.value= no,\
  +}
  +
  +#define QEMU_COMPAT_1_6 \
  +{\
  +.driver   = e1000,\
  +.property = mitigation,\
  +.value= off,\
  +},{\
  +.driver   = qemu64- TYPE_X86_CPU,\
  +.property = model,\
  +.value= stringify(2),\
  +},{\
  +.driver   = qemu32- TYPE_X86_CPU,\
  +.property = model,\
  +.value= stringify(3),\
  +}
  +
  +#define QEMU_COMPAT_1_5 \
  +{\
  +.driver   = Conroe- TYPE_X86_CPU,\
  +.property = model,\
  +.value= stringify(2),\
  +},{\
  +.driver   = Conroe- TYPE_X86_CPU,\
  +.property = level,\
  +.value= stringify(2),\
  +},{\
  +.driver   = Penryn- TYPE_X86_CPU,\
  +.property = model,\
  +.value= stringify(2),\
  +},{\
  +.driver   = Penryn- TYPE_X86_CPU,\
  +.property = level,\
  +.value= stringify(2),\
  +},{\
  +.driver   = Nehalem- TYPE_X86_CPU,\
  +.property = model,\
  +.value= stringify(2),\
  +},{\
  +.driver   = Nehalem- TYPE_X86_CPU,\
  +.property = level,\
  +.value= stringify(2),\
 
 Above are x86 CPUs - why have them in a common header?
 There's no chance any machine except PIIXQ35  needs these.
 
  +.driver   = virtio-net-pci,\
  +.property = any_layout,\
  +.value= off,\
 
 Here's an example.
 If you are using a non x86 target, QEMU 2.0
 has no way to select a machine with this
 value.
 So why expose it?
As we talked on the KVM call, the objective is to assign the
compat properties per QEMU version and close to their device.
As a *middle* step we shall put all together in a common file
per version and not per machine type, and then work on a registration
mechanism that will be based on the fact that compat properties
are per device and versioned by QEMU. 

Thanks,
Marcel

 
 
  +},{\
  +.driver = TYPE_X86_CPU,\
  +.property = pmu,\
  +.value = on,\
  +}
  +
  +#define QEMU_COMPAT_1_4 \
  +{\
  +.driver   = scsi-hd,\
  +.property = discard_granularity,\
  +.value= stringify(0),\
  +},{\
  +.driver   = scsi-cd,\
  +.property = discard_granularity,\
  +.value= stringify(0),\
  +},{\
  +.driver   = scsi-disk,\
  +.property = discard_granularity,\
  +.value= stringify(0),\
  +},{\
  +.driver   = ide-hd,\
  +.property = discard_granularity,\
  +.value= stringify(0),\
  +},{\
  +.driver   = ide-cd,\
  +.property = discard_granularity,\
  +.value= stringify(0),\
  +},{\
  +.driver   = ide-drive,\
  +.property = discard_granularity,\
  +.value= stringify(0),\
  +},{\
  +.driver   = virtio-blk-pci,\
  +.property = discard_granularity,\
  +.value= stringify(0),\
  + 

Re: [Qemu-devel] [RFC][PATCH] gpu:drm:i915:intel_detect_pch: back to check devfn instead of check class type

2014-06-25 Thread Paolo Bonzini

Il 22/06/2014 10:25, Chen, Tiejun ha scritto:

In qemu-upstream, as you commented we can't create this as a ISA class
type explicitly.


Note I didn't say that QEMU doesn't like having two ISA bridges.

I commented that the firmware will see two ISA bridges and will try to 
initialize both of them.  It currently doesn't just because it only 
knows of two southbridge PCI IDs, and both of them are old or relatively 
old (PIIX3/4 and ICH9).


But what would happen if you did graphics passthrough on a machine with 
an ICH9 southbridge?  Your code will create the PIIX4 ISA bridge, and 
will create an ICH9 southbridge just to please the i915 driver.  The 
BIOS will recognize the ICH9's vendor/device ids and try to initialize 
it.  It will write to the configuration space to set up PCI PIRQ[A-H] 
routing, for example, which makes no sense since your ICH9 is just a 
dummy device.


Second problem.  Your IGD passthrough code currently works with QEMU's 
PIIX4-based machine.  But what happens if you try to extend it, so that 
it works with QEMU's ICH9-based machine?  That's a more modern machine 
that has a PCIe chipset and hence has its ISA bridge at 00:1f.0.  Now 
you have a conflict.


In other words, if you want IGD passthrough in QEMU, you need a solution 
that is future-proof.



So we compromise by faking this ISA bridge without ISA
class type setting (as I recall you already said this way is slightly
better).


It is only slightly better, but the right solution is to fix the driver. 
 There is absolutely zero reason why a graphics driver should know 
about the vendor/device ids of the PCH.


The right way could be to make QEMU add a vendor-specific capability to 
the video device. The driver can probe for that capability before 
looking at the PCI bus.  QEMU can add the capability to the list, it is 
easy because all accesses to the video device's configuration space trap 
to QEMU.  Then you do not need to add fake devices to the machine.


In fact, it would be nice if Intel added such a capability on the next 
generation of integrated graphics, too.  On real hardware, ACPI or some 
other kind of firmware can probe the PCH at 00:1f.0 and initialize that 
vendor-specific capability.  QEMU would just forward the value from the 
host, so that virtualized guests never depend on the PCH at 00:1f.0.


Paolo


Maybe we will figure better way in the future. But anyway, this
is always registered as 00:15.0, right? So I think the i915 driver can
go back to probe the devfn like the native environment.





Re: [Qemu-devel] [RFC] alpha qemu arithmetic exceptions

2014-06-25 Thread Al Viro
On Tue, Jun 24, 2014 at 02:32:46PM -0700, Richard Henderson wrote:
 On 06/24/2014 02:24 PM, Al Viro wrote:
  Al, off to figure out the black magic TCG is using to generate calls...
 
 If you've a helper
 
 DEF_HELPER_1(halt, void, i64)
 
 then
 
   gen_helper_halt(...)
 
 will generate the tcg ops that result in the call.

Another fun issue:

CVTTQ is both underreporting the overflow *AND* reports the wrong kind - FOV
instead of IOV.

* it misses reporting overflows for case when it *knows* that
  overflow will happen - the need to shift up by more than 63 bits.
  Trivially fixed, of course.  There overflow cases leave wrong
  result as well - should be 0.
* it also misses reporting overflows for case when value is in
  ranges 2^63..2^64-1 and -2^64+1..-2^63-1.  And yes, it's
  asymmetric - 2^63 is an overflow, -2^63 isn't.
* overflow is reported by float_raise(float_flag_overflow, FP_STATUS).
  Wrong flag - it should be IOV, not FOV.  And it should be set
  in FPCR regardless of the trap modifier (IOV, this VI thing is
  wrong - we should deal with that only when we generate a trap).
* All overflow cases should raise INE as well.

Could we steal bit 1 in float_exception_flags for IOV?  It is (currently?)
unused -
enum {
float_flag_invalid   =  1,
float_flag_divbyzero =  4,
float_flag_overflow  =  8,
float_flag_underflow = 16,
float_flag_inexact   = 32,
float_flag_input_denormal = 64,
float_flag_output_denormal = 128
};

That would allow to deal with that crap nicely - we could have it raise
the new flag, then have helper_fp_exc_raise_... for default trap mode
mask it out (and yes, we need to set FPCR flags in default mode, as well
as /U and /V - confirmed by direct experiment *and* by TFM).

If we can't... well, we could put that flag separately, but it would be
more unpleasant.  Folks?



Re: [Qemu-devel] [PATCH 2.1 34/36] qemu-char: make writes thread-safe

2014-06-25 Thread Paolo Bonzini

Il 25/06/2014 08:06, Stefan Weil ha scritto:

Am 18.06.2014 08:43, schrieb Paolo Bonzini:

This will let threads other than the I/O thread raise QMP events.

GIOChannel is thread-safe, and send and receive state is usually
well-separated.  The only driver that requires some care is the
pty driver, where some of the state is shared by the read and write
sides.  That state is protected with the chr_write_lock too.

Reviewed-by: Fam Zheng f...@redhat.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 include/sysemu/char.h | 11 ++
 qemu-char.c   | 58 ---
 2 files changed, 53 insertions(+), 16 deletions(-)



Latest QEMU is broken for w64. This command no longer starts the GTK gui:

wine64 x86_64-softmmu/qemu-system-x86_64 -L pc-bios

Wine reports a dead lock:

err:ntdll:RtlpWaitForCriticalSection section 0x964f0 ? wait timed out
in thread 0009, blocked by , retrying (60 sec)


Actually this message is more likely to be a missing qemu_mutex_init, 
and in fact that's what it is.  I'm sorry, I'll send a patch right away.


Paolo



Re: [Qemu-devel] [v5][PATCH 3/5] xen, gfx passthrough: support Intel IGD passthrough with VT-D

2014-06-25 Thread Michael S. Tsirkin
On Wed, Jun 25, 2014 at 10:17:19AM +0800, Tiejun Chen wrote:
 Some registers of Intel IGD are mapped in host bridge, so it needs to
 passthrough these registers of physical host bridge to guest because
 emulated host bridge in guest doesn't have these mappings.
 
 The original patch is from Weidong Han weidong@intel.com
 
 Signed-off-by: Yang Zhang yang.z.zh...@intel.com
 Signed-off-by: Tiejun Chen tiejun.c...@intel.com
 Cc: Weidong Han weidong@intel.com
 ---
 v5:
 
 * Don't pass vendor/device ids in igd_pci_read().
 * Add to support offset 0x44/0x48.
 
 v4:
 
 * Given that pci_create_pch() is called unconditionally, so we just return 0
   even if its failed to check xen_has_gfx_passthru.
 * Remove one spurious change. 
 
 v3:
 
 * Improve comments to make that readable.
 
 v2:
 
 * To introduce is_igd_passthrough() to make sure we touch physical host bridge
   only in IGD case.
 
  hw/xen/xen_pt.h  |   4 ++
  hw/xen/xen_pt_graphics.c | 156 
 +++
  2 files changed, 160 insertions(+)
 
 diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
 index 4d3a18d..507165c 100644
 --- a/hw/xen/xen_pt.h
 +++ b/hw/xen/xen_pt.h
 @@ -302,5 +302,9 @@ extern int xen_has_gfx_passthru;
  int xen_pt_register_vga_regions(XenHostPCIDevice *dev);
  int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev);
  int xen_pt_setup_vga(XenHostPCIDevice *dev);
 +int pci_create_pch(PCIBus *bus);
 +void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr,
 +   uint32_t val, int len);
 +uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len);
  
  #endif /* !XEN_PT_H */
 diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
 index 974b7e9..f3fbfed 100644
 --- a/hw/xen/xen_pt_graphics.c
 +++ b/hw/xen/xen_pt_graphics.c
 @@ -4,6 +4,7 @@
  #include xen_pt.h
  #include xen-host-pci-device.h
  #include hw/xen/xen_backend.h
 +#include hw/pci/pci_bus.h
  
  static int is_vga_passthrough(XenHostPCIDevice *dev)
  {
 @@ -291,3 +292,158 @@ static int create_pseudo_pch_isa_bridge(PCIBus *bus, 
 XenHostPCIDevice *hdev)
  XEN_PT_LOG(dev, The pseudo Intel PCH ISA bridge created.\n);
  return 0;
  }
 +
 +int pci_create_pch(PCIBus *bus)


Please prefix all xen specific non static functions
with xen_ or something like this.
pci_ is for pci core.

In fact it's a good idea to do this for static functions
as well, in case we add a conflicting function in
some header.

 +{
 +XenHostPCIDevice hdev;
 +int r = 0;
 +
 +if (!xen_has_gfx_passthru) {
 +return r;
 +}
 +
 +r = xen_host_pci_device_get(hdev, 0, 0, 0x1f, 0);
 +if (r) {
 +XEN_PT_ERR(NULL, Failed to find Intel PCH on host\n);
 +goto err;
 +}
 +
 +if (hdev.vendor_id == PCI_VENDOR_ID_INTEL) {
 +r = create_pseudo_pch_isa_bridge(bus, hdev);
 +if (r) {
 +XEN_PT_ERR(NULL, Failed to create PCH ISA bridge.\n);
 +goto err;
 +}
 +}

Does it work on non intel?
It seems to return success.
Maybe you should just verify that vendor and device
ID have the expected values on the host, and
fail otherwise.

 +
 +xen_host_pci_device_put(hdev);
 +
 +err:
 +return r;
 +}
 +
 +/*
 + * Currently we just pass this physical host bridge for IGD, 00:02.0.
 + *
 + * Here pci_dev is just that host bridge, so we have to get that real
 + * passthrough device by that given devfn to further confirm.
 + */


confirm what?
Comments like this need to document what function does.

Maybe

/* Can we support IGD passthrough for this device?
 * We require ... XYZ - fill in here
 */

 +static int is_igd_passthrough(PCIDevice *pci_dev)
 +{
 +PCIDevice *f = pci_dev-bus-devices[PCI_DEVFN(2, 0)];
 +if (pci_dev-bus-devices[PCI_DEVFN(2, 0)]) {
 +XenPCIPassthroughState *s = DO_UPCAST(XenPCIPassthroughState, dev, 
 f);
 +return (is_vga_passthrough(s-real_device)
 + (s-real_device.vendor_id == PCI_VENDOR_ID_INTEL));
 +} else {
 +return 0;
 +}
 +}
 +
 +void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr,
 +   uint32_t val, int len)

Same here, xen_ everywhere please.

 +{
 +XenHostPCIDevice dev;
 +int r;
 +
 +/* IGD read/write is through the host bridge.
 + * ISA bridge is only for detect purpose. In i915 driver it will
 + * probe ISA bridge to discover the IGD, see comment in i915_drv.c:
 + * intel_detect_pch().

You mean in linux kernel I guess?

 + */
 +
 +assert(pci_dev-devfn == 0x00);
 +
 +if (!is_igd_passthrough(pci_dev)) {
 +goto write_default;
 +}
 +
 +/* Just work for the i915 driver. */
 +switch (config_addr) {
 +case 0x58:  /* PAVPC Offset */
 +break;
 +default:
 +/* Just sets the emulated values. */
 +goto write_default;
 +}
 +
 +/* Host write */
 +r = xen_host_pci_device_get(dev, 0, 0, 0, 0);
 +if (r) {
 +XEN_PT_ERR(pci_dev, Can't 

[Qemu-devel] [PATCH for 2.1] qemu-char: initialize out_lock

2014-06-25 Thread Paolo Bonzini
Otherwise, Windows fails with a deadlock.

Reported-by: Stefan Weil s...@weilnetz.de
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 qemu-char.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/qemu-char.c b/qemu-char.c
index 2e50a10..17bd360 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -94,6 +94,7 @@ static QTAILQ_HEAD(CharDriverStateHead, CharDriverState) 
chardevs =
 CharDriverState *qemu_chr_alloc(void)
 {
 CharDriverState *chr = g_malloc0(sizeof(CharDriverState));
+qemu_mutex_init(chr-chr_write_lock);
 return chr;
 }
 
-- 
1.9.3




Re: [Qemu-devel] [v5][PATCH 5/5] xen, gfx passthrough: add opregion mapping

2014-06-25 Thread Michael S. Tsirkin
On Wed, Jun 25, 2014 at 10:17:21AM +0800, Tiejun Chen wrote:
 The OpRegion shouldn't be mapped 1:1 because the address in the host
 can't be used in the guest directly.
 
 This patch traps read and write access to the opregion of the Intel
 GPU config space (offset 0xfc).
 
 The original patch is from Jean Guyader jean.guya...@eu.citrix.com
 
 Signed-off-by: Yang Zhang yang.z.zh...@intel.com
 Signed-off-by: Tiejun Chen tiejun.c...@intel.com
 Cc: Jean Guyader jean.guya...@eu.citrix.com
 ---
 v5:
 
 * Nothing is changed.
 
 v4:
 
 * Nothing is changed.
 
 v3:
 
 * Fix some typos.
 * Add more comments to make that readable.
 * To unmap igd_opregion when call xen_pt_unregister_vga_regions().
 * Improve some return paths.
 * We need to map 3 pages for opregion as hvmloader set. 
 * Force to convert igd_guest/host_opoegion as an unsigned long type
   while calling xc_domain_memory_mapping().
 
 v2:
 
 * We should return zero as an invalid address value while calling
   igd_read_opregion().
 
  hw/xen/xen_pt.h |  4 ++-
  hw/xen/xen_pt_config_init.c | 50 ++-
  hw/xen/xen_pt_graphics.c| 64 
 +
  3 files changed, 116 insertions(+), 2 deletions(-)
 
 diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
 index 507165c..25147cf 100644
 --- a/hw/xen/xen_pt.h
 +++ b/hw/xen/xen_pt.h
 @@ -63,7 +63,7 @@ typedef int (*xen_pt_conf_byte_read)
  #define XEN_PT_BAR_UNMAPPED (-1)
  
  #define PCI_CAP_MAX 48
 -
 +#define PCI_INTEL_OPREGION 0xfc
  

XEN_ please

PCI_CAP_MAX should be fixed too.


  typedef enum {
  XEN_PT_GRP_TYPE_HARDWIRED = 0,  /* 0 Hardwired reg group */
 @@ -306,5 +306,7 @@ int pci_create_pch(PCIBus *bus);
  void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr,
 uint32_t val, int len);
  uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len);
 +uint32_t igd_read_opregion(XenPCIPassthroughState *s);
 +void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val);
  
  #endif /* !XEN_PT_H */
 diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
 index de9a20f..6eaaa9a 100644
 --- a/hw/xen/xen_pt_config_init.c
 +++ b/hw/xen/xen_pt_config_init.c
 @@ -575,6 +575,22 @@ static int 
 xen_pt_exp_rom_bar_reg_write(XenPCIPassthroughState *s,
  return 0;
  }
  
 +static int xen_pt_intel_opregion_read(XenPCIPassthroughState *s,
 +  XenPTReg *cfg_entry,
 +  uint32_t *value, uint32_t valid_mask)
 +{
 +*value = igd_read_opregion(s);
 +return 0;
 +}
 +
 +static int xen_pt_intel_opregion_write(XenPCIPassthroughState *s,
 +   XenPTReg *cfg_entry, uint32_t *value,
 +   uint32_t dev_value, uint32_t 
 valid_mask)
 +{
 +igd_write_opregion(s, *value);
 +return 0;
 +}
 +
  /* Header Type0 reg static information table */
  static XenPTRegInfo xen_pt_emu_reg_header0[] = {
  /* Vendor ID reg */
 @@ -1440,6 +1456,20 @@ static XenPTRegInfo xen_pt_emu_reg_msix[] = {
  },
  };
  
 +static XenPTRegInfo xen_pt_emu_reg_igd_opregion[] = {
 +/* Intel IGFX OpRegion reg */
 +{
 +.offset = 0x0,
 +.size   = 4,
 +.init_val   = 0,
 +.no_wb  = 1,
 +.u.dw.read   = xen_pt_intel_opregion_read,
 +.u.dw.write  = xen_pt_intel_opregion_write,
 +},
 +{
 +.size = 0,
 +},
 +};
  
  /
   * Capabilities
 @@ -1677,6 +1707,14 @@ static const XenPTRegGroupInfo xen_pt_emu_reg_grps[] = 
 {
  .size_init   = xen_pt_msix_size_init,
  .emu_regs = xen_pt_emu_reg_msix,
  },
 +/* Intel IGD Opregion group */
 +{
 +.grp_id  = PCI_INTEL_OPREGION,
 +.grp_type= XEN_PT_GRP_TYPE_EMU,
 +.grp_size= 0x4,
 +.size_init   = xen_pt_reg_grp_size_init,
 +.emu_regs= xen_pt_emu_reg_igd_opregion,
 +},
  {
  .grp_size = 0,
  },
 @@ -1806,7 +1844,8 @@ int xen_pt_config_init(XenPCIPassthroughState *s)
  uint32_t reg_grp_offset = 0;
  XenPTRegGroup *reg_grp_entry = NULL;
  
 -if (xen_pt_emu_reg_grps[i].grp_id != 0xFF) {
 +if (xen_pt_emu_reg_grps[i].grp_id != 0xFF
 + xen_pt_emu_reg_grps[i].grp_id != PCI_INTEL_OPREGION) {
  if (xen_pt_hide_dev_cap(s-real_device,
  xen_pt_emu_reg_grps[i].grp_id)) {
  continue;
 @@ -1819,6 +1858,15 @@ int xen_pt_config_init(XenPCIPassthroughState *s)
  }
  }
  
 +/*
 + * By default we will trap up to 0x40 in the cfg space.
 + * If an intel device is pass through we need to trap 0xfc,
 + * therefore the size should be 0xff.
 + */
 +if (xen_pt_emu_reg_grps[i].grp_id == PCI_INTEL_OPREGION) {
 +reg_grp_offset = PCI_INTEL_OPREGION;
 +}
 +
   

Re: [Qemu-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support

2014-06-25 Thread Michael S. Tsirkin
On Wed, Jun 25, 2014 at 08:19:19AM +0200, Paolo Bonzini wrote:
 Il 25/06/2014 04:17, Tiejun Chen ha scritto:
 * Don't set that ISA class property, instead, just fake this ISA bridge
   with 00:1f.0.
 
 How are you going to make this work for Q35 or another PCIe machine that
 already has an ISA bridge at 00:1f.0?
 
 Paolo

Maybe it just works?
Is tweaking vendor/device/revision id to match host really necessary?

-- 
MST



Re: [Qemu-devel] broken win32 coroutines (was Re: qemu 1.6.1)

2014-06-25 Thread Michael W. Bombardieri
I applied the patch manually on qemu releases 1.7.1 and 2.0.0.

Configuration

* Host system is Lenovo ThinkPad T520 (windows 7)
* qemu built with --disable-sdl and --disable-gtk
* GCC version is 4.6.1 (MinGW win32)

Test (same steps/results for both 1.7.1 and 2.0.0)

* Created qcow2 disk image using qemu-img
* Started qemu-system-i386 with new disk image and OpenBSD/i386 5.5 install ISO
* Connected VM display using VNC
* VM booted successfully and install script started
* Install process too slow; gave up after 60 minutes

Following the same testing steps against a qemu 1.5.1 binary,
the OpenBSD 5.5 installation finished in 3 minutes on my system.

I haven't tried to profile the code.

- Michael


On Tue, Jun 24, 2014 at 07:22:59AM +0200, Paolo Bonzini wrote:
 Il 24/06/2014 03:41, Michael W. Bombardieri ha scritto:
 Hi,
 
 Thanks for adding me to this thread.
 I am not familiar with the qemu source code but I am aware
 of the coroutine crash and I can test a patch if you have
 one.
 
 qemu 1.5.1 was the last version I was able to build and use
 on win32. Later versions build without error but exhibit the
 coroutine crash.
 qemu binaries built with the coroutine feature disabled are
 to slow to use.
 
 Peter posted one here:
 
 http://article.gmane.org/gmane.comp.emulators.qemu/282189/raw
 
 Paolo
 



Re: [Qemu-devel] [PATCH 3/4] machine: Introduce QEMU_COMPAT_* macros

2014-06-25 Thread Michael S. Tsirkin
On Wed, Jun 25, 2014 at 09:46:40AM +0300, Marcel Apfelbaum wrote:
 On Wed, 2014-06-25 at 08:20 +0300, Michael S. Tsirkin wrote:
  On Tue, Jun 24, 2014 at 03:02:03PM -0300, Eduardo Habkost wrote:
   The QEMU_COMPAT_* macros will contain compat properties that are not
   specific to PC, and may be reused by other machine-types.
   
   PC-specific properties were left on the PC_COMPAT_* macros.
   
   Signed-off-by: Eduardo Habkost ehabk...@redhat.com
   ---
include/hw/boards.h  | 161 
   +++
include/hw/i386/pc.h | 150 
   ++-
2 files changed, 166 insertions(+), 145 deletions(-)
   
   diff --git a/include/hw/boards.h b/include/hw/boards.h
   index 605a970..709b582 100644
   --- a/include/hw/boards.h
   +++ b/include/hw/boards.h
   @@ -134,4 +134,165 @@ struct MachineState {
const char *cpu_model;
};

   +
   +/* Macros for compat_props corresponding to specific QEMU versions: */
   +
   +#define QEMU_COMPAT_2_0 \
   +{\
   +.driver   = virtio-scsi-pci,\
   +.property = any_layout,\
   +.value= off,\
   +},\
   +{\
   +.driver   = apic,\
   +.property = version,\
   +.value= stringify(0x11),\
   +},\
   +{\
   +.driver   = nec-usb-xhci,\
   +.property = superspeed-ports-first,\
   +.value= off,\
   +},\
   +{\
   +.driver   = pci-serial,\
   +.property = prog_if,\
   +.value= stringify(0),\
   +},\
   +{\
   +.driver   = pci-serial-2x,\
   +.property = prof_if,\
   +.value= stringify(0),\
   +},\
   +{\
   +.driver   = pci-serial-4x,\
   +.property = prog_if,\
   +.value= stringify(0),\
   +},\
   +{\
   +.driver   = virtio-net-pci,\
   +.property = guest_announce,\
   +.value= off,\
   +}
   +
   +#define QEMU_COMPAT_1_7 \
   +{\
   +.driver   = TYPE_USB_DEVICE,\
   +.property = msos-desc,\
   +.value= no,\
   +}
   +
   +#define QEMU_COMPAT_1_6 \
   +{\
   +.driver   = e1000,\
   +.property = mitigation,\
   +.value= off,\
   +},{\
   +.driver   = qemu64- TYPE_X86_CPU,\
   +.property = model,\
   +.value= stringify(2),\
   +},{\
   +.driver   = qemu32- TYPE_X86_CPU,\
   +.property = model,\
   +.value= stringify(3),\
   +}
   +
   +#define QEMU_COMPAT_1_5 \
   +{\
   +.driver   = Conroe- TYPE_X86_CPU,\
   +.property = model,\
   +.value= stringify(2),\
   +},{\
   +.driver   = Conroe- TYPE_X86_CPU,\
   +.property = level,\
   +.value= stringify(2),\
   +},{\
   +.driver   = Penryn- TYPE_X86_CPU,\
   +.property = model,\
   +.value= stringify(2),\
   +},{\
   +.driver   = Penryn- TYPE_X86_CPU,\
   +.property = level,\
   +.value= stringify(2),\
   +},{\
   +.driver   = Nehalem- TYPE_X86_CPU,\
   +.property = model,\
   +.value= stringify(2),\
   +},{\
   +.driver   = Nehalem- TYPE_X86_CPU,\
   +.property = level,\
   +.value= stringify(2),\
  
  Above are x86 CPUs - why have them in a common header?
  There's no chance any machine except PIIXQ35  needs these.
  
   +.driver   = virtio-net-pci,\
   +.property = any_layout,\
   +.value= off,\
  
  Here's an example.
  If you are using a non x86 target, QEMU 2.0
  has no way to select a machine with this
  value.
  So why expose it?
 As we talked on the KVM call, the objective is to assign the
 compat properties per QEMU version and close to their device.
 As a *middle* step we shall put all together in a common file
 per version and not per machine type, and then work on a registration
 mechanism that will be based on the fact that compat properties
 are per device and versioned by QEMU. 
 
 Thanks,
 Marcel

Yes but that'll stay on a branch and get merged after 2.1.
Let's keep this patch on that branch there as well.


  
  
   +},{\
   +.driver = TYPE_X86_CPU,\
   +.property = pmu,\
   +.value = on,\
   +}
   +
   +#define QEMU_COMPAT_1_4 \
   +{\
   +.driver   = scsi-hd,\
   +.property = discard_granularity,\
   +.value= stringify(0),\
   +},{\
   +.driver   = scsi-cd,\
   +.property = discard_granularity,\
   +.value= stringify(0),\
   +},{\
   +.driver   = scsi-disk,\
   +.property = discard_granularity,\
   +.value= stringify(0),\
   +},{\
   +.driver   = ide-hd,\
   +.property = discard_granularity,\
   +.value= stringify(0),\
   +},{\
   +.driver   = ide-cd,\
   +   

Re: [Qemu-devel] [PATCH 3/4] machine: Introduce QEMU_COMPAT_* macros

2014-06-25 Thread Marcel Apfelbaum
On Wed, 2014-06-25 at 10:18 +0300, Michael S. Tsirkin wrote:
 On Wed, Jun 25, 2014 at 09:46:40AM +0300, Marcel Apfelbaum wrote:
  On Wed, 2014-06-25 at 08:20 +0300, Michael S. Tsirkin wrote:
   On Tue, Jun 24, 2014 at 03:02:03PM -0300, Eduardo Habkost wrote:
The QEMU_COMPAT_* macros will contain compat properties that are not
specific to PC, and may be reused by other machine-types.

PC-specific properties were left on the PC_COMPAT_* macros.

Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
 include/hw/boards.h  | 161 
+++
 include/hw/i386/pc.h | 150 
++-
 2 files changed, 166 insertions(+), 145 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 605a970..709b582 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -134,4 +134,165 @@ struct MachineState {
 const char *cpu_model;
 };
 
+
+/* Macros for compat_props corresponding to specific QEMU versions: */
+
+#define QEMU_COMPAT_2_0 \
+{\
+.driver   = virtio-scsi-pci,\
+.property = any_layout,\
+.value= off,\
+},\
+{\
+.driver   = apic,\
+.property = version,\
+.value= stringify(0x11),\
+},\
+{\
+.driver   = nec-usb-xhci,\
+.property = superspeed-ports-first,\
+.value= off,\
+},\
+{\
+.driver   = pci-serial,\
+.property = prog_if,\
+.value= stringify(0),\
+},\
+{\
+.driver   = pci-serial-2x,\
+.property = prof_if,\
+.value= stringify(0),\
+},\
+{\
+.driver   = pci-serial-4x,\
+.property = prog_if,\
+.value= stringify(0),\
+},\
+{\
+.driver   = virtio-net-pci,\
+.property = guest_announce,\
+.value= off,\
+}
+
+#define QEMU_COMPAT_1_7 \
+{\
+.driver   = TYPE_USB_DEVICE,\
+.property = msos-desc,\
+.value= no,\
+}
+
+#define QEMU_COMPAT_1_6 \
+{\
+.driver   = e1000,\
+.property = mitigation,\
+.value= off,\
+},{\
+.driver   = qemu64- TYPE_X86_CPU,\
+.property = model,\
+.value= stringify(2),\
+},{\
+.driver   = qemu32- TYPE_X86_CPU,\
+.property = model,\
+.value= stringify(3),\
+}
+
+#define QEMU_COMPAT_1_5 \
+{\
+.driver   = Conroe- TYPE_X86_CPU,\
+.property = model,\
+.value= stringify(2),\
+},{\
+.driver   = Conroe- TYPE_X86_CPU,\
+.property = level,\
+.value= stringify(2),\
+},{\
+.driver   = Penryn- TYPE_X86_CPU,\
+.property = model,\
+.value= stringify(2),\
+},{\
+.driver   = Penryn- TYPE_X86_CPU,\
+.property = level,\
+.value= stringify(2),\
+},{\
+.driver   = Nehalem- TYPE_X86_CPU,\
+.property = model,\
+.value= stringify(2),\
+},{\
+.driver   = Nehalem- TYPE_X86_CPU,\
+.property = level,\
+.value= stringify(2),\
   
   Above are x86 CPUs - why have them in a common header?
   There's no chance any machine except PIIXQ35  needs these.
   
+.driver   = virtio-net-pci,\
+.property = any_layout,\
+.value= off,\
   
   Here's an example.
   If you are using a non x86 target, QEMU 2.0
   has no way to select a machine with this
   value.
   So why expose it?
  As we talked on the KVM call, the objective is to assign the
  compat properties per QEMU version and close to their device.
  As a *middle* step we shall put all together in a common file
  per version and not per machine type, and then work on a registration
  mechanism that will be based on the fact that compat properties
  are per device and versioned by QEMU. 
  
  Thanks,
  Marcel
 
 Yes but that'll stay on a branch and get merged after 2.1.
 Let's keep this patch on that branch there as well.
Fine by me

Thanks,
Marcel

 
 
   
   
+},{\
+.driver = TYPE_X86_CPU,\
+.property = pmu,\
+.value = on,\
+}
+
+#define QEMU_COMPAT_1_4 \
+{\
+.driver   = scsi-hd,\
+.property = discard_granularity,\
+.value= stringify(0),\
+},{\
+.driver   = scsi-cd,\
+.property = discard_granularity,\
+.value= stringify(0),\
+},{\
+.driver   = scsi-disk,\
+.property = 

Re: [Qemu-devel] [PATCH 0/4] Introduce common QEMU_COMPAT_* macros

2014-06-25 Thread Michael S. Tsirkin
On Tue, Jun 24, 2014 at 03:02:00PM -0300, Eduardo Habkost wrote:
 This series is an attempt to make the compat_props lists from the PC code
 reusable by other machine-types. All the compat bits that are on those lists 
 are
 not tied to a specific machine-type, but instead to the device code that was
 present on a given QEMU version.
 
 The last patch is a proposal to simply eliminate the PC-specific compat props
 macros, because we don't really need them today. All compat properties we have
 can be on global QEMU-version-specific lists, because PC-specific properties 
 are
 not going to affect other machine-types anyway.

So global properties are really not so special.
They just need to be specified with X.Y=Z syntax.
Can we just make them properties of a QEMU object?

I would like to make it possible for management to
ask what's the default value for that property with that
machine type.

Need to add API to make this painless, with not more lines of code
than currently.

 Eduardo Habkost (4):
   q35: Move q35-specific compat macros to pc_q35.c
   pc: Eliminate nesting of common PC_COMPAT_* macros
   machine: Introduce QEMU_COMPAT_* macros
   [RFC] Eliminate PC-specific compat_props
 
  hw/i386/pc_piix.c|  31 +--
  hw/i386/pc_q35.c |  19 +
  include/hw/boards.h  | 207 +++
  include/hw/i386/pc.h | 224 
 ---
  4 files changed, 251 insertions(+), 230 deletions(-)
 
 -- 
 1.9.3



Re: [Qemu-devel] [RFC][PATCH] gpu:drm:i915:intel_detect_pch: back to check devfn instead of check class type

2014-06-25 Thread Chen, Tiejun

On 2014/6/25 14:48, Paolo Bonzini wrote:

Il 22/06/2014 10:25, Chen, Tiejun ha scritto:

In qemu-upstream, as you commented we can't create this as a ISA class
type explicitly.


Note I didn't say that QEMU doesn't like having two ISA bridges.

I commented that the firmware will see two ISA bridges and will try to
initialize both of them.  It currently doesn't just because it only
knows of two southbridge PCI IDs, and both of them are old or relatively
old (PIIX3/4 and ICH9).

But what would happen if you did graphics passthrough on a machine with
an ICH9 southbridge?  Your code will create the PIIX4 ISA bridge, and
will create an ICH9 southbridge just to please the i915 driver.  The
BIOS will recognize the ICH9's vendor/device ids and try to initialize
it.  It will write to the configuration space to set up PCI PIRQ[A-H]
routing, for example, which makes no sense since your ICH9 is just a
dummy device.



Thanks for your detailed explanation.


Second problem.  Your IGD passthrough code currently works with QEMU's
PIIX4-based machine.  But what happens if you try to extend it, so that


Yes, current xen machine, xenpv, is based on pii4, and also I don't 
known if we will plan to migrate to q35 or others. So its hard to 
further say more now.



it works with QEMU's ICH9-based machine?  That's a more modern machine
that has a PCIe chipset and hence has its ISA bridge at 00:1f.0.  Now


But even in this case, could we set the real vendor/device ids for that 
ISA bridge at 00:1f.0? If not, what's broken?



you have a conflict.

In other words, if you want IGD passthrough in QEMU, you need a solution
that is future-proof.


So we compromise by faking this ISA bridge without ISA
class type setting (as I recall you already said this way is slightly
better).


It is only slightly better, but the right solution is to fix the driver.
  There is absolutely zero reason why a graphics driver should know
about the vendor/device ids of the PCH.


This means we have to fix this both on Linux and Windows but I'm not 
sure if this is feasible to us.




The right way could be to make QEMU add a vendor-specific capability to
the video device. The driver can probe for that capability before


Do you mean we can pick two unused offsets in the configuration space of 
the video device as a vendor-specific capability to hold the 
vendor/device ids of the PCH?



looking at the PCI bus.  QEMU can add the capability to the list, it is
easy because all accesses to the video device's configuration space trap
to QEMU.  Then you do not need to add fake devices to the machine.

In fact, it would be nice if Intel added such a capability on the next
generation of integrated graphics, too.  On real hardware, ACPI or some


Maybe, but even this would be implemented, shouldn't we need to be 
compatible with those old generations?


Thanks
Tiejun


other kind of firmware can probe the PCH at 00:1f.0 and initialize that
vendor-specific capability.  QEMU would just forward the value from the
host, so that virtualized guests never depend on the PCH at 00:1f.0.

Paolo


Maybe we will figure better way in the future. But anyway, this
is always registered as 00:15.0, right? So I think the i915 driver can
go back to probe the devfn like the native environment.








Re: [Qemu-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support

2014-06-25 Thread Chen, Tiejun

On 2014/6/25 14:19, Paolo Bonzini wrote:

Il 25/06/2014 04:17, Tiejun Chen ha scritto:

* Don't set that ISA class property, instead, just fake this ISA bridge
  with 00:1f.0.


How are you going to make this work for Q35 or another PCIe machine that
already has an ISA bridge at 00:1f.0?



Could we simply pass the vendor/device ids of the host ISA bridge here?

Thanks
Tiejun



Re: [Qemu-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support

2014-06-25 Thread Michael S. Tsirkin
On Wed, Jun 25, 2014 at 03:35:51PM +0800, Chen, Tiejun wrote:
 On 2014/6/25 14:19, Paolo Bonzini wrote:
 Il 25/06/2014 04:17, Tiejun Chen ha scritto:
 * Don't set that ISA class property, instead, just fake this ISA bridge
   with 00:1f.0.
 
 How are you going to make this work for Q35 or another PCIe machine that
 already has an ISA bridge at 00:1f.0?
 
 
 Could we simply pass the vendor/device ids of the host ISA bridge here?
 
 Thanks
 Tiejun

Will firmware have drivers for this bridge?

-- 
MST



[Qemu-devel] [PATCH] target-ppc: Add support for POWER8 pvr 0x4D0000

2014-06-25 Thread Alexey Kardashevskiy
At the moment QEMU knows about one version of POWER8 CPU with
PVR 0x4B.. This CPU class is defined as POWER8. The linux
kernel names it as POWER8E which is different from the name QEMU uses.

Now we get another version of POWER8 which is architecturally equivalent
to POWER8E but has different PVR 0x4D. so QEMU fails to find
a PPC CPU class on these machines. The linux kernel names these CPUs as
POWER8.

This renames the existing POWER8 to POWER8E to be more precise and
stay in sync with the linux kernel.

This adds a new POWER8 family which calls POWER8E class init function
and defines own PVR mask (used to match a CPU class) and desc (used to
create dynamic version-less CPU class).

This does not change CPU class fw_name attribute as the host POWER8
firmware keeps using PowerPC,POWER8 on both POWER8 and POWER8E.

Cc: Paul Mackerras pau...@samba.org
Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
Cc: Tom Musta tommu...@gmail.com
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
---

As far as I understand, this new CPU is used in entry-leven POWER8 box.
---
 target-ppc/cpu-models.c |  3 +++
 target-ppc/cpu-models.h |  7 +--
 target-ppc/translate_init.c | 20 
 3 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c
index 97a81d8..9a91af9 100644
--- a/target-ppc/cpu-models.c
+++ b/target-ppc/cpu-models.c
@@ -1138,6 +1138,8 @@
 POWER7 v2.3)
 POWERPC_DEF(POWER7+_v2.1,  CPU_POWERPC_POWER7P_v21,POWER7P,
 POWER7+ v2.1)
+POWERPC_DEF(POWER8E_v1.0,  CPU_POWERPC_POWER8E_v10,POWER8E,
+POWER8E v1.0)
 POWERPC_DEF(POWER8_v1.0,   CPU_POWERPC_POWER8_v10, POWER8,
 POWER8 v1.0)
 POWERPC_DEF(970,   CPU_POWERPC_970,970,
@@ -1386,6 +1388,7 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
 { POWER5gs, POWER5+ },
 { POWER7, POWER7_v2.3 },
 { POWER7+, POWER7+_v2.1 },
+{ POWER8E, POWER8E_v1.0 },
 { POWER8, POWER8_v1.0 },
 { 970fx, 970fx_v3.1 },
 { 970mp, 970mp_v1.1 },
diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
index db75896..c39d03a 100644
--- a/target-ppc/cpu-models.h
+++ b/target-ppc/cpu-models.h
@@ -559,9 +559,12 @@ enum {
 CPU_POWERPC_POWER7P_BASE   = 0x004A,
 CPU_POWERPC_POWER7P_MASK   = 0x,
 CPU_POWERPC_POWER7P_v21= 0x004A0201,
-CPU_POWERPC_POWER8_BASE= 0x004B,
+CPU_POWERPC_POWER8E_BASE   = 0x004B,
+CPU_POWERPC_POWER8E_MASK   = 0x,
+CPU_POWERPC_POWER8E_v10= 0x004B0100,
+CPU_POWERPC_POWER8_BASE= 0x004D,
 CPU_POWERPC_POWER8_MASK= 0x,
-CPU_POWERPC_POWER8_v10 = 0x004B0100,
+CPU_POWERPC_POWER8_v10 = 0x004D0100,
 CPU_POWERPC_970= 0x00390202,
 CPU_POWERPC_970FX_v10  = 0x00391100,
 CPU_POWERPC_970FX_v20  = 0x003C0200,
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 594f7ac..a3bb336 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -8189,16 +8189,16 @@ static void init_proc_POWER8(CPUPPCState *env)
 init_proc_book3s_64(env, BOOK3S_CPU_POWER8);
 }
 
-POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
+POWERPC_FAMILY(POWER8E)(ObjectClass *oc, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(oc);
 PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
 
 dc-fw_name = PowerPC,POWER8;
-dc-desc = POWER8;
+dc-desc = POWER8E;
 dc-props = powerpc_servercpu_properties;
-pcc-pvr = CPU_POWERPC_POWER8_BASE;
-pcc-pvr_mask = CPU_POWERPC_POWER8_MASK;
+pcc-pvr = CPU_POWERPC_POWER8E_BASE;
+pcc-pvr_mask = CPU_POWERPC_POWER8E_MASK;
 pcc-pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06;
 pcc-init_proc = init_proc_POWER8;
 pcc-check_pow = check_pow_nocheck;
@@ -8252,6 +8252,18 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
 pcc-l1_icache_size = 0x8000;
 pcc-interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
 }
+
+POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(oc);
+PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
+
+ppc_POWER8E_cpu_family_class_init(oc, data);
+
+dc-desc = POWER8;
+pcc-pvr = CPU_POWERPC_POWER8_BASE;
+pcc-pvr_mask = CPU_POWERPC_POWER8_MASK;
+}
 #endif /* defined (TARGET_PPC64) */
 
 
-- 
2.0.0




Re: [Qemu-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support

2014-06-25 Thread Paolo Bonzini

Il 25/06/2014 09:35, Chen, Tiejun ha scritto:

How are you going to make this work for Q35 or another PCIe machine that
already has an ISA bridge at 00:1f.0?



Could we simply pass the vendor/device ids of the host ISA bridge here?


No, because the firmware then would not recognize the host ISA bridge 
and would fail to initialize the ACPI PMBASE, the PCI interrupt routing, 
etc.


Paolo



Re: [Qemu-devel] [v5][PATCH 1/5] xen, gfx passthrough: basic graphics passthrough support

2014-06-25 Thread Chen, Tiejun



On 2014/6/25 14:21, Paolo Bonzini wrote:

Il 25/06/2014 04:17, Tiejun Chen ha scritto:

+static int get_vgabios(unsigned char *buf, XenHostPCIDevice *dev)
+{
+char rom_file[64];
+FILE *fp;
+uint8_t val;
+struct stat st;
+uint16_t magic = 0;
+int ret = 0;
+
+snprintf(rom_file, sizeof(rom_file),
+ /sys/bus/pci/devices/%04x:%02x:%02x.%d/rom,
+ dev-domain, dev-bus, dev-dev,
+ dev-func);
+
+if (stat(rom_file, st)) {
+return -ENODEV;
+}
+
+if (access(rom_file, F_OK)) {
+XEN_PT_ERR(NULL, pci-assign: Insufficient privileges for %s,
+rom_file);
+return -ENODEV;
+}


These two checks (stat and access) are useless.


Will remove this.




+/* Write 1 to the ROM file to enable it */
+fp = fopen(rom_file, r+);
+if (fp == NULL) {
+return -EACCES;
+}
+val = 1;
+if (fwrite(val, 1, 1, fp) != 1) {
+XEN_PT_LOG(%s\n, Failed to enable pci-sysfs rom file);
+ret = -EIO;
+goto close_rom;
+}
+fseek(fp, 0, SEEK_SET);
+
+/*
+ * Check if it a real bios extension.
+ * The magic number is 0xAA55.
+ */
+if (!fread(magic, sizeof(magic), 1, fp)) {
+XEN_PT_ERR(NULL, VGA: can't get magic.\n);
+ret = -ENODEV;
+goto close_rom;
+}
+if (magic != 0xAA55) {
+XEN_PT_ERR(NULL, VGA: wrong magic %x.\n, magic);
+ret = -ENODEV;
+goto close_rom;
+}
+fseek(fp, 0, SEEK_SET);
+
+if (!fread(buf, 1, st.st_size, fp)) {
+XEN_PT_ERR(NULL, VGA: pci-assign: Cannot read from host %s,
rom_file);
+XEN_PT_LOG(NULL, VGA: Device option ROM contents are
probably invalid 
+ (check dmesg).\nSkip option ROM probe with
rombar=0, 
+ or load from file with romfile=\n);
+}
+


Why is this graphics passthrough specific?  Should QEMU's Xen PCI
passthrough read ROM BAR contents from network cards too, for example?



I think this should not be same simply since we need to call VBIOS from 
RAM, not a ROM BAR of the device.


Thanks
Tiejun



Re: [Qemu-devel] [v5][PATCH 2/5] xen, gfx passthrough: create pseudo intel isa bridge

2014-06-25 Thread Chen, Tiejun

On 2014/6/25 14:22, Paolo Bonzini wrote:

Il 25/06/2014 04:17, Tiejun Chen ha scritto:

+static int create_pseudo_pch_isa_bridge(PCIBus *bus, XenHostPCIDevice
*hdev)
+{
+struct PCIDevice *dev;
+
+char rid;
+
+/* We havt to use a simple PCI device to fake this ISA bridge
+ * to avoid making some confusion to BIOS and ACPI.
+ */
+dev = pci_create(bus, PCI_DEVFN(0x1f, 0),
pseudo-intel-pch-isa-bridge);
+
+qdev_init_nofail(dev-qdev);
+
+pci_config_set_vendor_id(dev-config, hdev-vendor_id);
+pci_config_set_device_id(dev-config, hdev-device_id);
+
+xen_host_pci_get_block(hdev, PCI_REVISION_ID, (uint8_t *)rid, 1);
+
+pci_config_set_revision(dev-config, rid);
+
+XEN_PT_LOG(dev, The pseudo Intel PCH ISA bridge created.\n);
+return 0;
+}


This patch doesn't compile on its own (this static function is unused).

pci_create_pch should be moved in this patch.



Okay I will try this.

Thanks
Tiejun



Re: [Qemu-devel] [v5][PATCH 3/5] xen, gfx passthrough: support Intel IGD passthrough with VT-D

2014-06-25 Thread Chen, Tiejun

On 2014/6/25 14:25, Paolo Bonzini wrote:

Il 25/06/2014 04:17, Tiejun Chen ha scritto:

+int pci_create_pch(PCIBus *bus)
+{
+XenHostPCIDevice hdev;
+int r = 0;
+
+if (!xen_has_gfx_passthru) {
+return r;
+}
+


You could make this an assertion, since the function is never called
with xen_has_gfx_passthru == 0.  Or just drop the check completely, so


I will drop this completely.

Thanks
Tiejun


the function can be moved in patch 2.

Paolo






Re: [Qemu-devel] [RFC][PATCH] gpu:drm:i915:intel_detect_pch: back to check devfn instead of check class type

2014-06-25 Thread Paolo Bonzini

Il 25/06/2014 09:34, Chen, Tiejun ha scritto:

On 2014/6/25 14:48, Paolo Bonzini wrote:

Second problem.  Your IGD passthrough code currently works with QEMU's
PIIX4-based machine.  But what happens if you try to extend it, so that


Yes, current xen machine, xenpv, is based on pii4, and also I don't
known if we will plan to migrate to q35 or others. So its hard to
further say more now.


it works with QEMU's ICH9-based machine?  That's a more modern machine
that has a PCIe chipset and hence has its ISA bridge at 00:1f.0.  Now


But even in this case, could we set the real vendor/device ids for that
ISA bridge at 00:1f.0? If not, what's broken?


The config space layout changes for different vendor/device ids, so the 
guest firmware only works if you have the right vendor/device id.



It is only slightly better, but the right solution is to fix the driver.
  There is absolutely zero reason why a graphics driver should know
about the vendor/device ids of the PCH.


This means we have to fix this both on Linux and Windows but I'm not
sure if this is feasible to us.


You have to do it if you want this feature in QEMU in a future-proof way.

You _can_ provide the ugly PIIX4-specific hack as a compatibility 
fallback (and this patch is okay to make the compatibility fallback less 
hacky).  However, I don't think QEMU should accept the patch for IGD 
passthrough unless Intel is willing to make drivers 
virtualization-friendly.  Once you assign the IGD, it is not that 
integrated anymore and the drivers must take that into account.


It is worthwhile pointing out that neither AMD nor nVidia need any of this.


The right way could be to make QEMU add a vendor-specific capability to
the video device. The driver can probe for that capability before


Do you mean we can pick two unused offsets in the configuration space of
the video device as a vendor-specific capability to hold the
vendor/device ids of the PCH?


Yes, either that or add a new capability (which lets you choose the 
offsets more freely).


If the IGD driver needs config space fields of the MCH, those fields 
could also be mirrored in the new capability.  QEMU would forward them 
automatically.


It could even be a new BAR, which gives even more freedom to allocate 
the fields.



looking at the PCI bus.  QEMU can add the capability to the list, it is
easy because all accesses to the video device's configuration space trap
to QEMU.  Then you do not need to add fake devices to the machine.

In fact, it would be nice if Intel added such a capability on the next
generation of integrated graphics, too.  On real hardware, ACPI or some


Maybe, but even this would be implemented, shouldn't we need to be
compatible with those old generations?


Yes.

- old generation / old driver: use 00:1f.0 hack, only guaranteed to work 
on PIIX4-based virtual guest


- old generation / new driver: use 00:1f.0 hack on real hardware, use 
capability on 00:02.0 on virtual guest, can work on PCIe virtual guest


- new generation / old driver: doesn't exist

- new generation / new driver: always use capability on 00:02.0, can 
work on PCIe virtual guest.


Paolo



Re: [Qemu-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support

2014-06-25 Thread Paolo Bonzini

Il 25/06/2014 09:15, Michael S. Tsirkin ha scritto:

Maybe it just works?
Is tweaking vendor/device/revision id to match host really necessary?


Yes, the driver inspects the fields and tweaks its behavior according to 
them.  It also pokes at configuration space of the host bridge.  It's 
quite disgusting.


Paolo



[Qemu-devel] [PATCH] qemu-char: fix warning 'res' may be used uninitialized

2014-06-25 Thread Igor Mammedov
Signed-off-by: Igor Mammedov imamm...@redhat.com
---
 qemu-char.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 2e50a10..f6bdf2f 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -132,7 +132,7 @@ int qemu_chr_fe_write(CharDriverState *s, const uint8_t 
*buf, int len)
 int qemu_chr_fe_write_all(CharDriverState *s, const uint8_t *buf, int len)
 {
 int offset = 0;
-int res;
+int res = 0;
 
 qemu_mutex_lock(s-chr_write_lock);
 while (offset  len) {
-- 
1.7.1




Re: [Qemu-devel] [PATCH v3] mc146818rtc: add rtc-reset-reinjection QMP command

2014-06-25 Thread Michael S. Tsirkin
On Tue, Jun 24, 2014 at 06:55:11PM -0300, Marcelo Tosatti wrote:
 
 It is necessary to reset RTC interrupt reinjection backlog if
 guest time is synchronized via a different mechanism, such as
 QGA's guest-set-time command.
 
 Failing to do so causes both corrections to be applied (summed),
 resulting in an incorrect guest time.
 
 Signed-off-by: Marcelo Tosatti mtosa...@redhat.com

Doesn't seem to apply :(

error: patch failed: hw/timer/mc146818rtc.c:26
error: hw/timer/mc146818rtc.c: patch does not apply
error: patch failed: monitor.c:5475
error: monitor.c: patch does not apply
error: patch failed: qapi-schema.json:3080
error: qapi-schema.json: patch does not apply
error: patch failed: qmp-commands.hx:3574
error: qmp-commands.hx: patch does not apply



 diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
 index 1201f90..5bd8710 100644
 --- a/hw/timer/mc146818rtc.c
 +++ b/hw/timer/mc146818rtc.c
 @@ -26,6 +26,7 @@
  #include sysemu/sysemu.h
  #include hw/timer/mc146818rtc.h
  #include qapi/visitor.h
 +#include qmp-commands.h
  
  #ifdef TARGET_I386
  #include hw/i386/apic.h
 @@ -84,6 +85,7 @@ typedef struct RTCState {
  Notifier clock_reset_notifier;
  LostTickPolicy lost_tick_policy;
  Notifier suspend_notifier;
 +QLIST_ENTRY(RTCState) link;
  } RTCState;
  
  static void rtc_set_time(RTCState *s);
 @@ -522,6 +524,20 @@ static void rtc_get_time(RTCState *s, struct tm *tm)
  rtc_from_bcd(s, s-cmos_data[RTC_CENTURY]) * 100 - 1900;
  }
  
 +static QLIST_HEAD(, RTCState) rtc_devices =
 +QLIST_HEAD_INITIALIZER(rtc_devices);
 +
 +#ifdef TARGET_I386
 +void qmp_rtc_reset_reinjection(Error **errp)
 +{
 +RTCState *s;
 +
 +QLIST_FOREACH(s, rtc_devices, link) {
 +s-irq_coalesced = 0;
 +}
 +}
 +#endif
 +
  static void rtc_set_time(RTCState *s)
  {
  struct tm tm;
 @@ -910,6 +926,8 @@ ISADevice *rtc_init(ISABus *bus, int base_year, qemu_irq 
 intercept_irq)
  } else {
  isa_init_irq(isadev, s-irq, RTC_ISA_IRQ);
  }
 +QLIST_INSERT_HEAD(rtc_devices, s, link);
 +
  return isadev;
  }
  
 diff --git a/monitor.c b/monitor.c
 index 2901187..51c9a1d 100644
 --- a/monitor.c
 +++ b/monitor.c
 @@ -5475,3 +5475,12 @@ QemuOptsList qemu_mon_opts = {
  { /* end of list */ }
  },
  };
 +
 +#ifndef TARGET_I386
 +void qmp_rtc_reset_reinjection(Error **errp)
 +{
 +error_set(errp, QERR_COMMAND_NOT_FOUND, rtc-reset-reinjection);

Is there a way to get here on non 386?
If no let's do assert.
If yes, looks more like QERR_FEATURE_DISABLED to me.

 +return;

return not necessary.

 +}
 +#endif
 +

Adds whitespace at EOF.

 diff --git a/qapi-schema.json b/qapi-schema.json
 index dc2abe4..7e04d28 100644
 --- a/qapi-schema.json
 +++ b/qapi-schema.json
 @@ -3080,3 +3080,15 @@
'btn' : 'InputBtnEvent',
'rel' : 'InputMoveEvent',
'abs' : 'InputMoveEvent' } }
 +
 +##
 +# @rtc-reset-reinjection
 +#
 +# This command will reset the RTC interrupt reinjection backlog.
 +# Can be used if another mechanism to synchronize guest time
 +# is in effect, for example QEMU guest agent's guest-set-time
 +# command.
 +#
 +# Since: 2.1
 +##
 +{ 'command': 'rtc-reset-reinjection' }
 diff --git a/qmp-commands.hx b/qmp-commands.hx
 index d6bb0f4..87ce94a 100644
 --- a/qmp-commands.hx
 +++ b/qmp-commands.hx
 @@ -3574,3 +3574,26 @@ Example:
 } } ] }
  
  EQMP
 +
 +#if defined TARGET_I386
 +{
 +.name   = rtc-reset-reinjection,
 +.args_type  = ,
 +.mhandler.cmd_new = qmp_marshal_input_rtc_reset_reinjection,
 +},
 +#endif
 +
 +SQMP
 +rtc-reset-reinjection
 +-
 +
 +Reset the RTC interrupt reinjection backlog.
 +
 +Arguments: None.
 +
 +Example:
 +
 +- { execute: rtc-reset-reinjection }
 +- { return: {} }
 +
 +EQMP



Re: [Qemu-devel] [v5][PATCH 2/5] xen, gfx passthrough: create pseudo intel isa bridge

2014-06-25 Thread Chen, Tiejun

On 2014/6/25 14:45, Michael S. Tsirkin wrote:

On Wed, Jun 25, 2014 at 10:17:18AM +0800, Tiejun Chen wrote:

ISA bridge is needed since Intel gfx drive will probe with Dev31:Fun0
to make graphics device passthrough work well for VMM, that only need
to expose this pseudo ISA bridge to let driver know the real hardware
underneath.

The original patch is from Allen Kay allen.m@intel.com

Signed-off-by: Yang Zhang yang.z.zh...@intel.com
Signed-off-by: Tiejun Chen tiejun.c...@intel.com
Cc: Allen Kay allen.m@intel.com
---
v5:

* Don't set this ISA class property, instead, just fake this ISA bridge
   with 00:1f.0.

v4:

* Remove some unnecessary return in void foo().

v3:

* Fix some typos.
* Improve some return paths.

v2:

* Nothing is changed.

  hw/xen/xen_pt_graphics.c | 61 
  1 file changed, 61 insertions(+)

diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
index 461e526..974b7e9 100644
--- a/hw/xen/xen_pt_graphics.c
+++ b/hw/xen/xen_pt_graphics.c
@@ -230,3 +230,64 @@ out:
  g_free(bios);
  return rc;
  }
+
+static uint32_t isa_bridge_read_config(PCIDevice *d, uint32_t addr, int len)
+{
+return pci_default_read_config(d, addr, len);
+}
+
+static void isa_bridge_write_config(PCIDevice *d, uint32_t addr, uint32_t v,
+int len)
+{
+pci_default_write_config(d, addr, v, len);
+}
+
+static void isa_bridge_class_init(ObjectClass *klass, void *data)
+{
+PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+k-config_read = isa_bridge_read_config;
+k-config_write = isa_bridge_write_config;
+};


You don't need these stubs, just don't fill anything,
pci core will use defaults then.


I guess these stubs are left to extend something in the future. But 
maybe we can remove them now.





+
+typedef struct {
+PCIDevice dev;
+} ISABridgeState;
+


Nor do you need an empty structure if it has no state.


+static TypeInfo isa_bridge_info = {
+.name  = pseudo-intel-pch-isa-bridge,
+.parent= TYPE_PCI_DEVICE,
+.instance_size = sizeof(ISABridgeState),
+.class_init = isa_bridge_class_init,
+};
+
+static void xen_pt_graphics_register_types(void)
+{
+type_register_static(isa_bridge_info);
+}
+
+type_init(xen_pt_graphics_register_types)
+
+static int create_pseudo_pch_isa_bridge(PCIBus *bus, XenHostPCIDevice *hdev)
+{
+struct PCIDevice *dev;
+
+char rid;
+
+/* We havt to use a simple PCI device to fake this ISA bridge
+ * to avoid making some confusion to BIOS and ACPI.
+ */


A typo and confusing wording above, I'm not really
sure what this comment means.
Maybe:

/* Create a fake ISA bridge device at the location expected by guests. */



Good comments so thanks so much.




+dev = pci_create(bus, PCI_DEVFN(0x1f, 0), pseudo-intel-pch-isa-bridge);
+
+qdev_init_nofail(dev-qdev);
+
+pci_config_set_vendor_id(dev-config, hdev-vendor_id);
+pci_config_set_device_id(dev-config, hdev-device_id);
+
+xen_host_pci_get_block(hdev, PCI_REVISION_ID, (uint8_t *)rid, 1);


Host PCI device is the VGA card?


This is a real ISA bridge.


So why does it make sense to get it's vendor/device/revision and
stick in the ISA bridge?


The Intel generation of integrated graphics needs to probe this ISA 
bridge to initialize the i915 driver properly.


Thanks
Tiejun



Also change rid to uint8_t, you won't need to cast then.


+
+pci_config_set_revision(dev-config, rid);
+
+XEN_PT_LOG(dev, The pseudo Intel PCH ISA bridge created.\n);
+return 0;
+}
--
1.9.1







[Qemu-devel] [PATCH v2.5] spapr: Add a helper for node0_size calculation

2014-06-25 Thread Alexey Kardashevskiy
In multiple places there is a node0_size variable calculation
which assumes that NUMA node #0 and memory node #0 are the same
things which they are not. Since we are going to change it and
do not want to change it in multiple places, let's make a helper.

This adds a spapr_node0_size() helper and makes use of it.

Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
---
Changes:
v2.5:
* fixed old (but just reported) bug when QEMU is started with RAM size
bigger that the only NUMA node like this:
 -m 8192 -smp 4 -numa node,nodeid=0,cpus=0-3,mem=4096

v2:
* removed duplicated return ram_size from spapr_node0_size()

---
 hw/ppc/spapr.c | 19 ---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3c1f892..c14d6c2 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -351,6 +351,19 @@ static size_t create_page_sizes_prop(CPUPPCState *env, 
uint32_t *prop,
 return (p - prop) * sizeof(uint32_t);
 }
 
+static hwaddr spapr_node0_size(void)
+{
+if (nb_numa_nodes) {
+int i;
+for (i = 0; i  nb_numa_nodes; ++i) {
+if (node_mem[i]) {
+return MIN(pow2floor(node_mem[i]), ram_size);
+}
+}
+}
+return ram_size;
+}
+
 #define _FDT(exp) \
 do { \
 int ret = (exp);   \
@@ -860,8 +873,8 @@ static void spapr_reset_htab(sPAPREnvironment *spapr)
 
 /* Update the RMA size if necessary */
 if (spapr-vrma_adjust) {
-hwaddr node0_size = (nb_numa_nodes  1) ? node_mem[0] : ram_size;
-spapr-rma_size = kvmppc_rma_size(node0_size, spapr-htab_shift);
+spapr-rma_size = kvmppc_rma_size(spapr_node0_size(),
+  spapr-htab_shift);
 }
 }
 
@@ -1292,7 +1305,7 @@ static void ppc_spapr_init(MachineState *machine)
 MemoryRegion *sysmem = get_system_memory();
 MemoryRegion *ram = g_new(MemoryRegion, 1);
 hwaddr rma_alloc_size;
-hwaddr node0_size = (nb_numa_nodes  1) ? node_mem[0] : ram_size;
+hwaddr node0_size = spapr_node0_size();
 uint32_t initrd_base = 0;
 long kernel_size = 0, initrd_size = 0;
 long load_limit, rtas_limit, fw_size;
-- 
2.0.0




Re: [Qemu-devel] [Qemu-trivial] [PATCH v2] Add support for the arm breakpoint syscall

2014-06-25 Thread Michael Tokarev
24.06.2014 22:54, Riku Voipio wrote:
 On Tue, Jun 24, 2014 at 07:58:36PM +0400, Michael Tokarev wrote:
 20.06.2014 15:13, Hunter Laux wrote:
 OABI arm used a software interrupt(0xef9f0001) for breakpoints.
 Since 2005 gdb has used the break instruction(0xe7f001f0) for EABI.
 Apparently Steel Bank Common Lisp still uses the swi instruction.
  
 Applied to -trivial, despite the ugliness with the goto.  Oh well.
 
 It was already in my yesterdays linux-user pull req ( which I need
 to resend with the name_to_handle_at/open_by_handle_at syscalls fixed ).

If it is the same patch, git should do the Right Thing when
pulling your branch, without causing conflicts.

You could notify me about you applying this patch to -linux-user
branch.  I haven't noticed this patch in your pull req, -- I don't
always watch all pull requests.

Thanks,

/mjt



[Qemu-devel] [RFC PATCH v4 03/13] migration: make qemu_savevm_state public.

2014-06-25 Thread fred . konrad
From: KONRAD Frederic fred.kon...@greensocs.com

This makes qemu_savevm_state public for reverse-execution.

Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
Reviewed-by: Amit Shah amit.s...@redhat.com
---
 include/sysemu/sysemu.h | 1 +
 savevm.c| 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 285c45b..c2fb786 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -81,6 +81,7 @@ void do_info_snapshots(Monitor *mon, const QDict *qdict);
 
 void qemu_announce_self(void);
 
+int qemu_savevm_state(QEMUFile *f);
 bool qemu_savevm_state_blocked(Error **errp);
 void qemu_savevm_state_begin(QEMUFile *f,
  const MigrationParams *params);
diff --git a/savevm.c b/savevm.c
index ba900d3..77fe794 100644
--- a/savevm.c
+++ b/savevm.c
@@ -782,7 +782,7 @@ void qemu_savevm_state_cancel(void)
 }
 }
 
-static int qemu_savevm_state(QEMUFile *f)
+int qemu_savevm_state(QEMUFile *f)
 {
 int ret;
 MigrationParams params = {
-- 
1.9.0




[Qemu-devel] [RFC PATCH v4 00/13] Reverse execution.

2014-06-25 Thread fred . konrad
From: KONRAD Frederic fred.kon...@greensocs.com

Hi everybody,

This is the fourth version of this RFC (see the changes below).

Those are the two first patch-set we have been worked on for reverse execution.

The first part is fully reviewed except the icount: introduce icount timer
patch maybe we can merge them?

The first series:
  icount: put icount variables into TimerState.
  migration: migrate icount fields.
  migration: make qemu_savevm_state public.
  icount: introduce icount timer.
  icount: check for icount clock deadline when cpu loop exits.
  icount: make icount extra computed on icount clock as well.
  timer: add cpu_icount_to_ns function.

are various preparation patches for reverse execution.

The last patches:
  trace-events: add reverse-execution events.
  introduce reverse execution mechanism.
  gdbstub: allow reverse execution in gdb stub.
  cpu-exec: trigger a debug request when rexec stops.
  cexe: synchronize icount on the next event.
  cexe: allow to enable reverse execution.

are reverse execution introduction.

They can be clone at: git://git.greensocs.com/qemu_cexe.git:cexe_2_3_v4

The third series will be sent as soon as possible and have some issues with
QEMU's thread as it use fork.

This implementation of reverse execution works with instruction counting:

A new clock is implemented which is icount clock. It grows each time an
instruction is executed and is totally independant of host clock.

Snapshots are taken regularly (based on icount clock) with help of migration
code and written on the disk.

When user wants to use reverse-stepi:
 * Last snapshot is reloaded.
 * A stop callback is created to be triggered at the previous instruction.

This stop callback generates a debug exception so QEMU stops in debug mode.

Command line:
 * -cexe option is added to enable reverse execution, it needs icount 1.

About non determinism in QEMU:
 * This implementation doesn't take IO in account so any IO will cause non
   determinism and break reverse execution.

 * The icount warp mechanism have been disabled when reverse execution is
   enabled so the time grow differently inside the VM.

Testing:
 * It has been tested on ARM without any IO such as network or asynchronous file
   access to keep the deterministic behaviour of icount.

Known issues:
 * On ARM stepi seems to do some additional steps which are added to icount
   counter so reverse-stepi just after stepi is broken.

 * The IO replay explained above.

Changes:
 v3 - v4:
  * Fix icount_state_needed (As suggested by Amit).
  * Rebase.

 v2 - v3:
  * Use trace instead of debug printfs (As suggested by Lluis).

 v1 - v2:
  * Use subsection for icount migration (As suggested by Paolo).

  * Use with_bias parameters to get_icount instead of get_icount_wo_bias
function (As suggested by Paolo).

KONRAD Frederic (13):
  icount: put icount variables into TimerState.
  migration: migrate icount fields.
  migration: make qemu_savevm_state public.
  icount: introduce icount timer.
  icount: check for icount clock deadline when cpu loop exits.
  icount: make icount extra computed on icount clock as well.
  timer: add cpu_icount_to_ns function.
  trace-events: add reverse-execution events.
  introduce reverse execution mechanism.
  gdbstub: allow reverse execution in gdb stub.
  cpu-exec: trigger a debug request when rexec stops.
  cexe: synchronize icount on the next event.
  cexe: allow to enable reverse execution.

 Makefile.target |   1 +
 cpu-exec.c  |  13 ++
 cpus.c  | 117 ++---
 gdbstub.c   |  31 -
 include/qemu/timer.h|  18 ++-
 include/reverse-execution.h |  43 +++
 include/sysemu/sysemu.h |   1 +
 main-loop.c |  10 ++
 qemu-options.hx |   9 ++
 qemu-timer.c|   8 +-
 reverse-execution.c | 306 
 savevm.c|   2 +-
 stubs/Makefile.objs |   1 +
 stubs/cexe-stub.c   |  32 +
 stubs/cpu-get-icount.c  |  10 +-
 trace-events|   6 +
 vl.c|  23 +++-
 17 files changed, 605 insertions(+), 26 deletions(-)
 create mode 100644 include/reverse-execution.h
 create mode 100644 reverse-execution.c
 create mode 100644 stubs/cexe-stub.c

-- 
1.9.0




[Qemu-devel] [RFC PATCH v4 05/13] icount: check for icount clock deadline when cpu loop exits.

2014-06-25 Thread fred . konrad
From: KONRAD Frederic fred.kon...@greensocs.com

Notify events on icount clock when CPU loop exits.

Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
Reviewed-by: Paolo Bonzini pbonz...@redhat.com
---
 cpus.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/cpus.c b/cpus.c
index 7f99d5e..76de504 100644
--- a/cpus.c
+++ b/cpus.c
@@ -993,6 +993,11 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
 if (deadline == 0) {
 qemu_clock_notify(QEMU_CLOCK_VIRTUAL);
 }
+
+deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_ICOUNT);
+if (deadline == 0) {
+qemu_clock_notify(QEMU_CLOCK_ICOUNT);
+}
 }
 qemu_tcg_wait_io_event();
 }
-- 
1.9.0




[Qemu-devel] [RFC PATCH v4 01/13] icount: put icount variables into TimerState.

2014-06-25 Thread fred . konrad
From: KONRAD Frederic fred.kon...@greensocs.com

This puts qemu_icount and qemu_icount_bias into TimerState structure to allow
them to be migrated.

Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
Reviewed-by: Paolo Bonzini pbonz...@redhat.com
---
 cpus.c | 29 -
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/cpus.c b/cpus.c
index 5e7f2cf..127de1c 100644
--- a/cpus.c
+++ b/cpus.c
@@ -102,17 +102,12 @@ static bool all_cpu_threads_idle(void)
 
 /* Protected by TimersState seqlock */
 
-/* Compensate for varying guest execution speed.  */
-static int64_t qemu_icount_bias;
 static int64_t vm_clock_warp_start;
 /* Conversion factor from emulated instructions to virtual clock ticks.  */
 static int icount_time_shift;
 /* Arbitrarily pick 1MIPS as the minimum allowable speed.  */
 #define MAX_ICOUNT_SHIFT 10
 
-/* Only written by TCG thread */
-static int64_t qemu_icount;
-
 static QEMUTimer *icount_rt_timer;
 static QEMUTimer *icount_vm_timer;
 static QEMUTimer *icount_warp_timer;
@@ -129,6 +124,11 @@ typedef struct TimersState {
 int64_t cpu_clock_offset;
 int32_t cpu_ticks_enabled;
 int64_t dummy;
+
+/* Compensate for varying guest execution speed.  */
+int64_t qemu_icount_bias;
+/* Only written by TCG thread */
+int64_t qemu_icount;
 } TimersState;
 
 static TimersState timers_state;
@@ -139,14 +139,14 @@ static int64_t cpu_get_icount_locked(void)
 int64_t icount;
 CPUState *cpu = current_cpu;
 
-icount = qemu_icount;
+icount = timers_state.qemu_icount;
 if (cpu) {
 if (!cpu_can_do_io(cpu)) {
 fprintf(stderr, Bad clock read\n);
 }
 icount -= (cpu-icount_decr.u16.low + cpu-icount_extra);
 }
-return qemu_icount_bias + (icount  icount_time_shift);
+return timers_state.qemu_icount_bias + (icount  icount_time_shift);
 }
 
 int64_t cpu_get_icount(void)
@@ -284,7 +284,8 @@ static void icount_adjust(void)
 icount_time_shift++;
 }
 last_delta = delta;
-qemu_icount_bias = cur_icount - (qemu_icount  icount_time_shift);
+timers_state.qemu_icount_bias = cur_icount
+  - (timers_state.qemu_icount  
icount_time_shift);
 seqlock_write_unlock(timers_state.vm_clock_seqlock);
 }
 
@@ -333,7 +334,7 @@ static void icount_warp_rt(void *opaque)
 int64_t delta = cur_time - cur_icount;
 warp_delta = MIN(warp_delta, delta);
 }
-qemu_icount_bias += warp_delta;
+timers_state.qemu_icount_bias += warp_delta;
 }
 vm_clock_warp_start = -1;
 seqlock_write_unlock(timers_state.vm_clock_seqlock);
@@ -351,7 +352,7 @@ void qtest_clock_warp(int64_t dest)
 int64_t deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL);
 int64_t warp = qemu_soonest_timeout(dest - clock, deadline);
 seqlock_write_lock(timers_state.vm_clock_seqlock);
-qemu_icount_bias += warp;
+timers_state.qemu_icount_bias += warp;
 seqlock_write_unlock(timers_state.vm_clock_seqlock);
 
 qemu_clock_run_timers(QEMU_CLOCK_VIRTUAL);
@@ -1250,7 +1251,8 @@ static int tcg_cpu_exec(CPUArchState *env)
 int64_t count;
 int64_t deadline;
 int decr;
-qemu_icount -= (cpu-icount_decr.u16.low + cpu-icount_extra);
+timers_state.qemu_icount -= (cpu-icount_decr.u16.low
++ cpu-icount_extra);
 cpu-icount_decr.u16.low = 0;
 cpu-icount_extra = 0;
 deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL);
@@ -1265,7 +1267,7 @@ static int tcg_cpu_exec(CPUArchState *env)
 }
 
 count = qemu_icount_round(deadline);
-qemu_icount += count;
+timers_state.qemu_icount += count;
 decr = (count  0x) ? 0x : count;
 count -= decr;
 cpu-icount_decr.u16.low = decr;
@@ -1278,7 +1280,8 @@ static int tcg_cpu_exec(CPUArchState *env)
 if (use_icount) {
 /* Fold pending instructions back into the
instruction counter, and clear the interrupt flag.  */
-qemu_icount -= (cpu-icount_decr.u16.low + cpu-icount_extra);
+timers_state.qemu_icount -= (cpu-icount_decr.u16.low
++ cpu-icount_extra);
 cpu-icount_decr.u32 = 0;
 cpu-icount_extra = 0;
 }
-- 
1.9.0




[Qemu-devel] [RFC PATCH v4 02/13] migration: migrate icount fields.

2014-06-25 Thread fred . konrad
From: KONRAD Frederic fred.kon...@greensocs.com

This fixes a bug where qemu_icount and qemu_icount_bias are not migrated.
It adds a subsection timer/icount to vmstate_timers so icount is migrated only
when needed.

Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
Reviewed-by: Amit Shah amit.s...@redhat.com
---
 cpus.c | 28 
 1 file changed, 28 insertions(+)

diff --git a/cpus.c b/cpus.c
index 127de1c..8d2ddf0 100644
--- a/cpus.c
+++ b/cpus.c
@@ -429,6 +429,26 @@ void qemu_clock_warp(QEMUClockType type)
 }
 }
 
+static bool icount_state_needed(void *opaque)
+{
+return use_icount;
+}
+
+/*
+ * This is a subsection for icount migration.
+ */
+static const VMStateDescription icount_vmstate_timers = {
+.name = timer/icount,
+.version_id = 1,
+.minimum_version_id = 1,
+.minimum_version_id_old = 1,
+.fields  = (VMStateField[]) {
+VMSTATE_INT64(qemu_icount_bias, TimersState),
+VMSTATE_INT64(qemu_icount, TimersState),
+VMSTATE_END_OF_LIST()
+}
+};
+
 static const VMStateDescription vmstate_timers = {
 .name = timer,
 .version_id = 2,
@@ -438,6 +458,14 @@ static const VMStateDescription vmstate_timers = {
 VMSTATE_INT64(dummy, TimersState),
 VMSTATE_INT64_V(cpu_clock_offset, TimersState, 2),
 VMSTATE_END_OF_LIST()
+},
+.subsections = (VMStateSubsection[]) {
+{
+.vmsd = icount_vmstate_timers,
+.needed = icount_state_needed,
+}, {
+/* empty */
+}
 }
 };
 
-- 
1.9.0




[Qemu-devel] [RFC PATCH v4 06/13] icount: make icount extra computed on icount clock as well.

2014-06-25 Thread fred . konrad
From: KONRAD Frederic fred.kon...@greensocs.com

This takes icount clock in account for icount extra computation so icount
clock's timers will be triggered at the exact time.

Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
Reviewed-by: Paolo Bonzini pbonz...@redhat.com
---
 cpus.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/cpus.c b/cpus.c
index 76de504..4b551f1 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1288,6 +1288,7 @@ static int tcg_cpu_exec(CPUArchState *env)
 if (use_icount) {
 int64_t count;
 int64_t deadline;
+int64_t icount_deadline;
 int decr;
 timers_state.qemu_icount -= (cpu-icount_decr.u16.low
 + cpu-icount_extra);
@@ -1304,6 +1305,15 @@ static int tcg_cpu_exec(CPUArchState *env)
 deadline = INT32_MAX;
 }
 
+/*
+ * Take icount clock deadline in account too, and keep the nearest
+ * deadline.
+ */
+icount_deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_ICOUNT);
+if ((icount_deadline = 0)  (icount_deadline  deadline)) {
+deadline = icount_deadline;
+}
+
 count = qemu_icount_round(deadline);
 timers_state.qemu_icount += count;
 decr = (count  0x) ? 0x : count;
-- 
1.9.0




[Qemu-devel] [RFC PATCH v4 04/13] icount: introduce icount timer.

2014-06-25 Thread fred . konrad
From: KONRAD Frederic fred.kon...@greensocs.com

This introduces a new timer based only on instruction counter and without any
compensation.

Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
---
 cpus.c | 19 ---
 include/qemu/timer.h   |  9 -
 qemu-timer.c   |  8 +++-
 stubs/cpu-get-icount.c |  2 +-
 4 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/cpus.c b/cpus.c
index 8d2ddf0..7f99d5e 100644
--- a/cpus.c
+++ b/cpus.c
@@ -134,7 +134,7 @@ typedef struct TimersState {
 static TimersState timers_state;
 
 /* Return the virtual CPU time, based on the instruction counter.  */
-static int64_t cpu_get_icount_locked(void)
+static int64_t cpu_get_icount_locked(int with_bias)
 {
 int64_t icount;
 CPUState *cpu = current_cpu;
@@ -146,17 +146,22 @@ static int64_t cpu_get_icount_locked(void)
 }
 icount -= (cpu-icount_decr.u16.low + cpu-icount_extra);
 }
-return timers_state.qemu_icount_bias + (icount  icount_time_shift);
+
+if (with_bias) {
+return timers_state.qemu_icount_bias + (icount  icount_time_shift);
+} else {
+return icount  icount_time_shift;
+}
 }
 
-int64_t cpu_get_icount(void)
+int64_t cpu_get_icount(int with_bias)
 {
 int64_t icount;
 unsigned start;
 
 do {
 start = seqlock_read_begin(timers_state.vm_clock_seqlock);
-icount = cpu_get_icount_locked();
+icount = cpu_get_icount_locked(with_bias);
 } while (seqlock_read_retry(timers_state.vm_clock_seqlock, start));
 
 return icount;
@@ -169,7 +174,7 @@ int64_t cpu_get_ticks(void)
 int64_t ticks;
 
 if (use_icount) {
-return cpu_get_icount();
+return cpu_get_icount(true);
 }
 
 ticks = timers_state.cpu_ticks_offset;
@@ -267,7 +272,7 @@ static void icount_adjust(void)
 
 seqlock_write_lock(timers_state.vm_clock_seqlock);
 cur_time = cpu_get_clock_locked();
-cur_icount = cpu_get_icount_locked();
+cur_icount = cpu_get_icount_locked(true);
 
 delta = cur_icount - cur_time;
 /* FIXME: This is a very crude algorithm, somewhat prone to oscillation.  
*/
@@ -330,7 +335,7 @@ static void icount_warp_rt(void *opaque)
  * far ahead of real time.
  */
 int64_t cur_time = cpu_get_clock_locked();
-int64_t cur_icount = cpu_get_icount_locked();
+int64_t cur_icount = cpu_get_icount_locked(true);
 int64_t delta = cur_time - cur_icount;
 warp_delta = MIN(warp_delta, delta);
 }
diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 7f9a074..6204cab 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -36,12 +36,19 @@
  * is suspended, and it will reflect system time changes the host may
  * undergo (e.g. due to NTP). The host clock has the same precision as
  * the virtual clock.
+ *
+ * @QEMU_CLOCK_ICOUNT: icount clock
+ *
+ * The icount clock is based on instruction counter without any compensation 
for
+ * speed. It will run only when instruction are executed and make only sense in
+ * icount mode.
  */
 
 typedef enum {
 QEMU_CLOCK_REALTIME = 0,
 QEMU_CLOCK_VIRTUAL = 1,
 QEMU_CLOCK_HOST = 2,
+QEMU_CLOCK_ICOUNT = 3,
 QEMU_CLOCK_MAX
 } QEMUClockType;
 
@@ -743,7 +750,7 @@ static inline int64_t get_clock(void)
 #endif
 
 /* icount */
-int64_t cpu_get_icount(void);
+int64_t cpu_get_icount(int with_bias);
 int64_t cpu_get_clock(void);
 
 /***/
diff --git a/qemu-timer.c b/qemu-timer.c
index 00a5d35..3be80b1 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -554,7 +554,7 @@ int64_t qemu_clock_get_ns(QEMUClockType type)
 default:
 case QEMU_CLOCK_VIRTUAL:
 if (use_icount) {
-return cpu_get_icount();
+return cpu_get_icount(true);
 } else {
 return cpu_get_clock();
 }
@@ -566,6 +566,12 @@ int64_t qemu_clock_get_ns(QEMUClockType type)
 notifier_list_notify(clock-reset_notifiers, now);
 }
 return now;
+case QEMU_CLOCK_ICOUNT:
+if (use_icount) {
+return cpu_get_icount(false);
+} else {
+return -1;
+}
 }
 }
 
diff --git a/stubs/cpu-get-icount.c b/stubs/cpu-get-icount.c
index d685859..1968de7 100644
--- a/stubs/cpu-get-icount.c
+++ b/stubs/cpu-get-icount.c
@@ -3,7 +3,7 @@
 
 int use_icount;
 
-int64_t cpu_get_icount(void)
+int64_t cpu_get_icount(int with_bias)
 {
 abort();
 }
-- 
1.9.0




[Qemu-devel] [RFC PATCH v4 11/13] cpu-exec: trigger a debug request when rexec stops.

2014-06-25 Thread fred . konrad
From: KONRAD Frederic fred.kon...@greensocs.com

This allows QEMU to trigger a debug exception when cexe_dbg_requested
is set.

Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
---
 cpu-exec.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/cpu-exec.c b/cpu-exec.c
index 38e5f02..c92cfd1 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -22,6 +22,7 @@
 #include tcg.h
 #include qemu/atomic.h
 #include sysemu/qtest.h
+#include reverse-execution.h
 
 void cpu_loop_exit(CPUState *cpu)
 {
@@ -230,6 +231,18 @@ int cpu_exec(CPUArchState *env)
 /* This must be volatile so it is not trashed by longjmp() */
 volatile bool have_tb_lock = false;
 
+#ifndef CONFIG_USER_ONLY
+if (cexe_is_enabled()  cexe_dbg_requested()) {
+/*
+ * Reverse execution need to stop right now.
+ * So just generate a EXCP_DEBUG.
+ */
+cpu-exception_index = EXCP_DEBUG;
+cpu_handle_debug_exception(env);
+return EXCP_DEBUG;
+}
+#endif /* !CONFIG_USER_ONLY */
+
 if (cpu-halted) {
 if (!cpu_has_work(cpu)) {
 return EXCP_HALTED;
-- 
1.9.0




[Qemu-devel] [RFC PATCH v4 09/13] introduce reverse execution mechanism.

2014-06-25 Thread fred . konrad
From: KONRAD Frederic fred.kon...@greensocs.com

This introduces the basic reverse-execution mechanism.

Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
---
 Makefile.target |   1 +
 cpus.c  |   6 +
 include/reverse-execution.h |  41 ++
 reverse-execution.c | 306 
 vl.c|   7 +-
 5 files changed, 360 insertions(+), 1 deletion(-)
 create mode 100644 include/reverse-execution.h
 create mode 100644 reverse-execution.c

diff --git a/Makefile.target b/Makefile.target
index 6089d29..7cb5cae 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -120,6 +120,7 @@ endif #CONFIG_BSD_USER
 # System emulator target
 ifdef CONFIG_SOFTMMU
 obj-y += arch_init.o cpus.o monitor.o gdbstub.o balloon.o ioport.o numa.o
+obj-y += reverse-execution.o
 obj-y += qtest.o
 obj-y += hw/
 obj-$(CONFIG_FDT) += device_tree.o
diff --git a/cpus.c b/cpus.c
index 89c7a1a..e86045e 100644
--- a/cpus.c
+++ b/cpus.c
@@ -63,6 +63,8 @@
 
 #endif /* CONFIG_LINUX */
 
+#include reverse-execution.h
+
 static CPUState *next_cpu;
 
 bool cpu_is_stopped(CPUState *cpu)
@@ -593,7 +595,11 @@ static bool cpu_can_run(CPUState *cpu)
 
 static void cpu_handle_guest_debug(CPUState *cpu)
 {
+if (cexe_is_continuing_backward()) {
+cexe_step_done();
+}
 gdb_set_stop_cpu(cpu);
+cexe_stop_stepping_back_mode();
 qemu_system_debug_request();
 cpu-stopped = true;
 }
diff --git a/include/reverse-execution.h b/include/reverse-execution.h
new file mode 100644
index 000..bf42003
--- /dev/null
+++ b/include/reverse-execution.h
@@ -0,0 +1,41 @@
+/*
+ *  reverse execution.
+ *
+ *  Copyright (C) 2014 : GreenSocs Ltd
+ *  http://www.greensocs.com/ , email: i...@greensocs.com
+ *
+ *  Developed by :
+ *  Frederic Konrad   fred.kon...@greensocs.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see http://www.gnu.org/licenses/.
+ *
+ */
+
+#ifndef REVERSE_EXECUTION
+#define REVERSE_EXECUTION
+
+void cexe_setup(void);
+void cexe_step_backward(CPUState *cpu, uint64_t steps);
+void cexe_stop_stepping_back_mode(void);
+void cexe_continue_backward(CPUState *cpu);
+int cexe_is_continuing_backward(void);
+void cexe_next_reverse_continue_step(void);
+void cexe_stop_reverse_continue(void);
+void cexe_step_done(void);
+bool cexe_is_step_done(void);
+bool cexe_is_enabled(void);
+void cexe_cleanup(void);
+bool cexe_dbg_requested(void);
+
+#endif /* REVERSE_EXECUTION */
diff --git a/reverse-execution.c b/reverse-execution.c
new file mode 100644
index 000..9bf2ee8
--- /dev/null
+++ b/reverse-execution.c
@@ -0,0 +1,306 @@
+/*
+ *  reverse execution.
+ *
+ *  Copyright (C) 2014 : GreenSocs Ltd
+ *  http://www.greensocs.com/ , email: i...@greensocs.com
+ *
+ *  Developed by :
+ *  Frederic Konrad   fred.kon...@greensocs.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see http://www.gnu.org/licenses/.
+ *
+ */
+
+#include qemu/timer.h
+#include sysemu/sysemu.h
+#include migration/qemu-file.h
+
+#include reverse-execution.h
+
+#include trace.h  /* needed for trace event prototype */
+
+typedef struct snapshot_entry {
+uint32_t id;
+int64_t time;
+QLIST_ENTRY(snapshot_entry) next;
+} snapshot_entry;
+
+static QLIST_HEAD(, snapshot_entry) snapshot = 
QLIST_HEAD_INITIALIZER(snapshot);
+
+QEMUTimer *snap_timer;
+QEMUTimer *stop_timer;
+
+struct cexe_state {
+int stepping_back;
+int continue_backward_mode;
+int singlestep_was_enabled;
+bool step_done;
+bool stop_requested;
+};
+
+static bool cexe_enabled;
+struct cexe_state cexe_state;
+
+static snapshot_entry *new_snapshot(void)
+{
+snapshot_entry *snap = NULL;
+snap = g_malloc(sizeof(snapshot_entry));
+assert(snap);
+
+if (QLIST_FIRST(snapshot) != NULL) {
+snap-id = QLIST_FIRST(snapshot)-id + 

[Qemu-devel] [RFC PATCH v4 07/13] timer: add cpu_icount_to_ns function.

2014-06-25 Thread fred . konrad
From: KONRAD Frederic fred.kon...@greensocs.com

This adds cpu_icount_to_ns function which is needed for reverse execution.

It returns the time for a specific instruction.

Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
Reviewed-by: Paolo Bonzini pbonz...@redhat.com
---
 cpus.c   | 9 +++--
 include/qemu/timer.h | 1 +
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/cpus.c b/cpus.c
index 4b551f1..89c7a1a 100644
--- a/cpus.c
+++ b/cpus.c
@@ -148,9 +148,9 @@ static int64_t cpu_get_icount_locked(int with_bias)
 }
 
 if (with_bias) {
-return timers_state.qemu_icount_bias + (icount  icount_time_shift);
+return timers_state.qemu_icount_bias + cpu_icount_to_ns(icount);
 } else {
-return icount  icount_time_shift;
+return cpu_icount_to_ns(icount);
 }
 }
 
@@ -167,6 +167,11 @@ int64_t cpu_get_icount(int with_bias)
 return icount;
 }
 
+int64_t cpu_icount_to_ns(int64_t icount)
+{
+return icount  icount_time_shift;
+}
+
 /* return the host CPU cycle counter and handle stop/restart */
 /* Caller must hold the BQL */
 int64_t cpu_get_ticks(void)
diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 6204cab..0ae7f28 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -752,6 +752,7 @@ static inline int64_t get_clock(void)
 /* icount */
 int64_t cpu_get_icount(int with_bias);
 int64_t cpu_get_clock(void);
+int64_t cpu_icount_to_ns(int64_t icount);
 
 /***/
 /* host CPU ticks (if available) */
-- 
1.9.0




[Qemu-devel] [RFC PATCH v4 12/13] cexe: synchronize icount on the next event.

2014-06-25 Thread fred . konrad
From: KONRAD Frederic fred.kon...@greensocs.com

We don't want to warp on host clock as it is not deterministic for replay.
So this patch warp icount on the next QEMU_VIRTUAL_CLOCK event if reverse
execution is enabled.

The normal behaviour is kept when reverse execution is disabled.

Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
---
 cpus.c  | 19 +--
 include/qemu/timer.h|  8 
 include/reverse-execution.h |  2 ++
 main-loop.c | 10 ++
 stubs/Makefile.objs |  1 +
 stubs/cexe-stub.c   | 32 
 stubs/cpu-get-icount.c  |  8 
 7 files changed, 78 insertions(+), 2 deletions(-)
 create mode 100644 stubs/cexe-stub.c

diff --git a/cpus.c b/cpus.c
index e86045e..77d4700 100644
--- a/cpus.c
+++ b/cpus.c
@@ -321,8 +321,10 @@ static int64_t qemu_icount_round(int64_t count)
 return (count + (1  icount_time_shift) - 1)  icount_time_shift;
 }
 
-static void icount_warp_rt(void *opaque)
+void icount_warp_rt(void *opaque)
 {
+int64_t next_vm_deadline = -1;
+
 /* The icount_warp_timer is rescheduled soon after vm_clock_warp_start
  * changes from -1 to another value, so the race here is okay.
  */
@@ -330,6 +332,13 @@ static void icount_warp_rt(void *opaque)
 return;
 }
 
+if (cexe_is_enabled()) {
+/*
+ * We need this because the standard warp_delta is not deterministic.
+ */
+next_vm_deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL);
+}
+
 seqlock_write_lock(timers_state.vm_clock_seqlock);
 if (runstate_is_running()) {
 int64_t clock = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
@@ -346,7 +355,13 @@ static void icount_warp_rt(void *opaque)
 int64_t delta = cur_time - cur_icount;
 warp_delta = MIN(warp_delta, delta);
 }
-timers_state.qemu_icount_bias += warp_delta;
+if (cexe_is_enabled()) {
+if (next_vm_deadline  0) {
+timers_state.qemu_icount_bias += next_vm_deadline;
+}
+} else {
+timers_state.qemu_icount_bias += warp_delta;
+}
 }
 vm_clock_warp_start = -1;
 seqlock_write_unlock(timers_state.vm_clock_seqlock);
diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 0ae7f28..de2641a 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -754,6 +754,14 @@ int64_t cpu_get_icount(int with_bias);
 int64_t cpu_get_clock(void);
 int64_t cpu_icount_to_ns(int64_t icount);
 
+/**
+ * void icount_warp_rt:
+ *
+ * Move icount to the realtime clock or to the next QEMU_VIRTUAL_CLOCK event
+ * when reverse execution is enabled.
+ */
+void icount_warp_rt(void *opaque);
+
 /***/
 /* host CPU ticks (if available) */
 
diff --git a/include/reverse-execution.h b/include/reverse-execution.h
index bf42003..739572e 100644
--- a/include/reverse-execution.h
+++ b/include/reverse-execution.h
@@ -25,6 +25,8 @@
 #ifndef REVERSE_EXECUTION
 #define REVERSE_EXECUTION
 
+#include qom/cpu.h
+
 void cexe_setup(void);
 void cexe_step_backward(CPUState *cpu, uint64_t steps);
 void cexe_stop_stepping_back_mode(void);
diff --git a/main-loop.c b/main-loop.c
index 8a85493..6130438 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -34,6 +34,8 @@
 
 #include qemu/compatfd.h
 
+#include reverse-execution.h
+
 /* If we have signalfd, we mask out the signals we want to handle and then
  * use signalfd to listen for them.  We rely on whatever the current signal
  * handler is to dispatch the signals when we receive them.
@@ -489,6 +491,14 @@ int main_loop_wait(int nonblocking)
 
 qemu_clock_run_all_timers();
 
+/*
+ * Sometimes deadlock can appears because there is no pending event on
+ * virtual clock.
+ */
+if (cexe_is_enabled()) {
+icount_warp_rt(NULL);
+}
+
 return ret;
 }
 
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 528e161..5e362f6 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -39,3 +39,4 @@ stub-obj-$(CONFIG_WIN32) += fd-register.o
 stub-obj-y += cpus.o
 stub-obj-y += kvm.o
 stub-obj-y += qmp_pc_dimm_device_list.o
+stub-obj-y += cexe-stub.o
diff --git a/stubs/cexe-stub.c b/stubs/cexe-stub.c
new file mode 100644
index 000..7723998
--- /dev/null
+++ b/stubs/cexe-stub.c
@@ -0,0 +1,32 @@
+/*
+ *  cexe-stub.c
+ *
+ *  Copyright (C) 2014 : GreenSocs Ltd
+ *  http://www.greensocs.com/ , email: i...@greensocs.com
+ *
+ *  Developed by :
+ *  Frederic Konrad   fred.kon...@greensocs.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ 

[Qemu-devel] [RFC PATCH v4 08/13] trace-events: add reverse-execution events.

2014-06-25 Thread fred . konrad
From: KONRAD Frederic fred.kon...@greensocs.com

This adds some trace-events for reverse execution.

Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
---
 trace-events | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/trace-events b/trace-events
index ba01ad5..c1423e0 100644
--- a/trace-events
+++ b/trace-events
@@ -1292,3 +1292,9 @@ mhp_pc_dimm_assigned_address(uint64_t addr) 0x%PRIx64
 # target-s390x/kvm.c
 kvm_enable_cmma(int rc) CMMA: enabling with result code %d
 kvm_clear_cmma(int rc) CMMA: clearing with result code %d
+
+# reverse-execution.c
+snap_callback(uint64_t time, const char *filename) snapshot @%li - %s
+stop_callback(uint64_t time) stopping @%li
+cexe_stop_reverse_continue(void) stop reverse continue.
+cexe_step_backward(uint64_t time, uint64_t stop) stepping backward @%li stop 
@%li
-- 
1.9.0




[Qemu-devel] [RFC PATCH v4 13/13] cexe: allow to enable reverse execution.

2014-06-25 Thread fred . konrad
From: KONRAD Frederic fred.kon...@greensocs.com

This creates QEMU options for reverse execution.

Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
---
 qemu-options.hx |  9 +
 vl.c| 16 
 2 files changed, 25 insertions(+)

diff --git a/qemu-options.hx b/qemu-options.hx
index ff76ad4..2afb85d 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2946,6 +2946,15 @@ order cores with complex cache hierarchies.  The number 
of instructions
 executed often has little or no correlation with actual performance.
 ETEXI
 
+DEF(cexe, 0, QEMU_OPTION_cexe, \
+-cexe\n \
+enable reverse execution\n, QEMU_ARCH_ALL)
+STEXI
+@item -cexe
+@findex -cexe
+Enable reverse execution.
+ETEXI
+
 DEF(watchdog, HAS_ARG, QEMU_OPTION_watchdog, \
 -watchdog i6300esb|ib700\n \
 enable virtual hardware watchdog [default=none]\n,
diff --git a/vl.c b/vl.c
index d0352e3..a94563a 100644
--- a/vl.c
+++ b/vl.c
@@ -228,6 +228,7 @@ static int default_floppy = 1;
 static int default_cdrom = 1;
 static int default_sdcard = 1;
 static int default_vga = 1;
+static int default_cexe;
 
 static struct {
 const char *driver;
@@ -3821,6 +3822,9 @@ int main(int argc, char **argv, char **envp)
 case QEMU_OPTION_icount:
 icount_option = optarg;
 break;
+case QEMU_OPTION_cexe:
+default_cexe = true;
+break;
 case QEMU_OPTION_incoming:
 incoming = optarg;
 runstate_set(RUN_STATE_INMIGRATE);
@@ -4520,6 +4524,15 @@ int main(int argc, char **argv, char **envp)
 vm_start();
 }
 
+if (default_cexe) {
+if (!icount_option) {
+fprintf(stderr, Reverse execution requires icount.\n);
+exit(1);
+}
+
+cexe_setup();
+}
+
 os_setup_post();
 
 if (is_daemonized()) {
@@ -4536,5 +4549,8 @@ int main(int argc, char **argv, char **envp)
 tpm_cleanup();
 #endif
 
+if (cexe_is_enabled()) {
+cexe_cleanup();
+}
 return 0;
 }
-- 
1.9.0




[Qemu-devel] [RFC PATCH v4 10/13] gdbstub: allow reverse execution in gdb stub.

2014-06-25 Thread fred . konrad
From: KONRAD Frederic fred.kon...@greensocs.com

This allows gdb to reverse step QEMU: reverse-stepi and reverse-cont commands
are allowed.

When step_backward is called, QEMU restores a snapshot before the actual
instruction and stops (with a debug exit) when the previous instruction is
reached.

Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
---
 gdbstub.c | 31 ++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/gdbstub.c b/gdbstub.c
index 8afe0b7..99769af 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -33,6 +33,7 @@
 #include sysemu/char.h
 #include sysemu/sysemu.h
 #include exec/gdbstub.h
+#include reverse-execution.h
 #endif
 
 #define MAX_PACKET_LENGTH 4096
@@ -1113,6 +1114,17 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 if (cc-gdb_core_xml_file != NULL) {
 pstrcat(buf, sizeof(buf), ;qXfer:features:read+);
 }
+
+#ifndef CONFIG_USER_ONLY
+/*
+ * When reverse execution is enabled those additional features must
+ * be set so GDB allows reverse-stepi and reverse-continue command.
+ */
+if (cexe_is_enabled()) {
+pstrcat(buf, sizeof(buf), ;ReverseStep+;ReverseContinue+);
+}
+#endif /* !CONFIG_USER_ONLY */
+
 put_packet(s, buf);
 break;
 }
@@ -1161,7 +1173,23 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 }
 /* Unrecognised 'q' command.  */
 goto unknown_command;
-
+#ifndef CONFIG_USER_ONLY
+case 'b':
+/* Reverse execution. */
+switch (*p) {
+case 's':
+cexe_step_backward(s-c_cpu, 1);
+break;
+case 'c':
+cexe_continue_backward(s-c_cpu);
+break;
+default:
+buf[0] = '\0';
+put_packet(s, buf);
+break;
+}
+break;
+#endif /* !CONFIG_USER_ONLY */
 default:
 unknown_command:
 /* put empty packet */
@@ -1221,6 +1249,7 @@ static void gdb_vm_state_change(void *opaque, int 
running, RunState state)
 ret = GDB_SIGNAL_TRAP;
 break;
 case RUN_STATE_PAUSED:
+cexe_stop_reverse_continue();
 ret = GDB_SIGNAL_INT;
 break;
 case RUN_STATE_SHUTDOWN:
-- 
1.9.0




Re: [Qemu-devel] [PATCHv3 0/2] block/nfs: add knob to set readahead

2014-06-25 Thread Kevin Wolf
Am 25.06.2014 um 00:05 hat Peter Lieven geschrieben:
 upcoming libnfs will feature internal readahead support.
 Add a knob to pass the optional readahead value as a URL
 parameter.
 
 This series fixes also the incorrect usage of strncmp and
 atoi.

Thanks, applied to the block branch.

Kevin



Re: [Qemu-devel] [v5][PATCH 2/5] xen, gfx passthrough: create pseudo intel isa bridge

2014-06-25 Thread Michael S. Tsirkin
On Wed, Jun 25, 2014 at 04:10:44PM +0800, Chen, Tiejun wrote:
 On 2014/6/25 14:45, Michael S. Tsirkin wrote:
 On Wed, Jun 25, 2014 at 10:17:18AM +0800, Tiejun Chen wrote:
 ISA bridge is needed since Intel gfx drive will probe with Dev31:Fun0
 to make graphics device passthrough work well for VMM, that only need
 to expose this pseudo ISA bridge to let driver know the real hardware
 underneath.
 
 The original patch is from Allen Kay allen.m@intel.com
 
 Signed-off-by: Yang Zhang yang.z.zh...@intel.com
 Signed-off-by: Tiejun Chen tiejun.c...@intel.com
 Cc: Allen Kay allen.m@intel.com
 ---
 v5:
 
 * Don't set this ISA class property, instead, just fake this ISA bridge
with 00:1f.0.
 
 v4:
 
 * Remove some unnecessary return in void foo().
 
 v3:
 
 * Fix some typos.
 * Improve some return paths.
 
 v2:
 
 * Nothing is changed.
 
   hw/xen/xen_pt_graphics.c | 61 
  
   1 file changed, 61 insertions(+)
 
 diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
 index 461e526..974b7e9 100644
 --- a/hw/xen/xen_pt_graphics.c
 +++ b/hw/xen/xen_pt_graphics.c
 @@ -230,3 +230,64 @@ out:
   g_free(bios);
   return rc;
   }
 +
 +static uint32_t isa_bridge_read_config(PCIDevice *d, uint32_t addr, int 
 len)
 +{
 +return pci_default_read_config(d, addr, len);
 +}
 +
 +static void isa_bridge_write_config(PCIDevice *d, uint32_t addr, uint32_t 
 v,
 +int len)
 +{
 +pci_default_write_config(d, addr, v, len);
 +}
 +
 +static void isa_bridge_class_init(ObjectClass *klass, void *data)
 +{
 +PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 +
 +k-config_read = isa_bridge_read_config;
 +k-config_write = isa_bridge_write_config;
 +};
 
 You don't need these stubs, just don't fill anything,
 pci core will use defaults then.
 
 I guess these stubs are left to extend something in the future. But maybe we
 can remove them now.
 
 
 +
 +typedef struct {
 +PCIDevice dev;
 +} ISABridgeState;
 +
 
 Nor do you need an empty structure if it has no state.
 
 +static TypeInfo isa_bridge_info = {
 +.name  = pseudo-intel-pch-isa-bridge,
 +.parent= TYPE_PCI_DEVICE,
 +.instance_size = sizeof(ISABridgeState),
 +.class_init = isa_bridge_class_init,
 +};
 +
 +static void xen_pt_graphics_register_types(void)
 +{
 +type_register_static(isa_bridge_info);
 +}
 +
 +type_init(xen_pt_graphics_register_types)
 +
 +static int create_pseudo_pch_isa_bridge(PCIBus *bus, XenHostPCIDevice 
 *hdev)
 +{
 +struct PCIDevice *dev;
 +
 +char rid;
 +
 +/* We havt to use a simple PCI device to fake this ISA bridge
 + * to avoid making some confusion to BIOS and ACPI.
 + */
 
 A typo and confusing wording above, I'm not really
 sure what this comment means.
 Maybe:
 
 /* Create a fake ISA bridge device at the location expected by guests. */
 
 
 Good comments so thanks so much.
 
 
 +dev = pci_create(bus, PCI_DEVFN(0x1f, 0), 
 pseudo-intel-pch-isa-bridge);
 +
 +qdev_init_nofail(dev-qdev);
 +
 +pci_config_set_vendor_id(dev-config, hdev-vendor_id);
 +pci_config_set_device_id(dev-config, hdev-device_id);
 +
 +xen_host_pci_get_block(hdev, PCI_REVISION_ID, (uint8_t *)rid, 1);
 
 Host PCI device is the VGA card?
 
 This is a real ISA bridge.
 
 So why does it make sense to get it's vendor/device/revision and
 stick in the ISA bridge?
 
 The Intel generation of integrated graphics needs to probe this ISA bridge
 to initialize the i915 driver properly.
 
 Thanks
 Tiejun

So vendor/device/revision IDs must match real device then?
Stick this in the comment then.

In fact it's exactly what passthrough does.
I wonder if more bits from ./hw/i386/kvm/pci-assign.c
can be reused. How do you poke at the host device? sysfs?



 
 Also change rid to uint8_t, you won't need to cast then.
 
 +
 +pci_config_set_revision(dev-config, rid);
 +
 +XEN_PT_LOG(dev, The pseudo Intel PCH ISA bridge created.\n);
 +return 0;
 +}
 --
 1.9.1
 
 




Re: [Qemu-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support

2014-06-25 Thread Michael S. Tsirkin
On Wed, Jun 25, 2014 at 09:44:14AM +0200, Paolo Bonzini wrote:
 Il 25/06/2014 09:35, Chen, Tiejun ha scritto:
 How are you going to make this work for Q35 or another PCIe machine that
 already has an ISA bridge at 00:1f.0?
 
 
 Could we simply pass the vendor/device ids of the host ISA bridge here?
 
 No, because the firmware then would not recognize the host ISA bridge and
 would fail to initialize the ACPI PMBASE, the PCI interrupt routing, etc.
 
 Paolo

It might be possible to move the Q35 bridge elsewhere.
seabios doesn't care where the host bridge is.
ACPI tables in QEMU can be adjusted.



Re: [Qemu-devel] [v5][PATCH 2/5] xen, gfx passthrough: create pseudo intel isa bridge

2014-06-25 Thread Chen, Tiejun

On 2014/6/25 16:28, Michael S. Tsirkin wrote:

On Wed, Jun 25, 2014 at 04:10:44PM +0800, Chen, Tiejun wrote:

On 2014/6/25 14:45, Michael S. Tsirkin wrote:

On Wed, Jun 25, 2014 at 10:17:18AM +0800, Tiejun Chen wrote:

ISA bridge is needed since Intel gfx drive will probe with Dev31:Fun0
to make graphics device passthrough work well for VMM, that only need
to expose this pseudo ISA bridge to let driver know the real hardware
underneath.

The original patch is from Allen Kay allen.m@intel.com

Signed-off-by: Yang Zhang yang.z.zh...@intel.com
Signed-off-by: Tiejun Chen tiejun.c...@intel.com
Cc: Allen Kay allen.m@intel.com
---
v5:

* Don't set this ISA class property, instead, just fake this ISA bridge
   with 00:1f.0.

v4:

* Remove some unnecessary return in void foo().

v3:

* Fix some typos.
* Improve some return paths.

v2:

* Nothing is changed.

  hw/xen/xen_pt_graphics.c | 61 
  1 file changed, 61 insertions(+)

diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
index 461e526..974b7e9 100644
--- a/hw/xen/xen_pt_graphics.c
+++ b/hw/xen/xen_pt_graphics.c
@@ -230,3 +230,64 @@ out:
  g_free(bios);
  return rc;
  }
+
+static uint32_t isa_bridge_read_config(PCIDevice *d, uint32_t addr, int len)
+{
+return pci_default_read_config(d, addr, len);
+}
+
+static void isa_bridge_write_config(PCIDevice *d, uint32_t addr, uint32_t v,
+int len)
+{
+pci_default_write_config(d, addr, v, len);
+}
+
+static void isa_bridge_class_init(ObjectClass *klass, void *data)
+{
+PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+k-config_read = isa_bridge_read_config;
+k-config_write = isa_bridge_write_config;
+};


You don't need these stubs, just don't fill anything,
pci core will use defaults then.


I guess these stubs are left to extend something in the future. But maybe we
can remove them now.




+
+typedef struct {
+PCIDevice dev;
+} ISABridgeState;
+


Nor do you need an empty structure if it has no state.


+static TypeInfo isa_bridge_info = {
+.name  = pseudo-intel-pch-isa-bridge,
+.parent= TYPE_PCI_DEVICE,
+.instance_size = sizeof(ISABridgeState),
+.class_init = isa_bridge_class_init,
+};
+
+static void xen_pt_graphics_register_types(void)
+{
+type_register_static(isa_bridge_info);
+}
+
+type_init(xen_pt_graphics_register_types)
+
+static int create_pseudo_pch_isa_bridge(PCIBus *bus, XenHostPCIDevice *hdev)
+{
+struct PCIDevice *dev;
+
+char rid;
+
+/* We havt to use a simple PCI device to fake this ISA bridge
+ * to avoid making some confusion to BIOS and ACPI.
+ */


A typo and confusing wording above, I'm not really
sure what this comment means.
Maybe:

/* Create a fake ISA bridge device at the location expected by guests. */



Good comments so thanks so much.




+dev = pci_create(bus, PCI_DEVFN(0x1f, 0), pseudo-intel-pch-isa-bridge);
+
+qdev_init_nofail(dev-qdev);
+
+pci_config_set_vendor_id(dev-config, hdev-vendor_id);
+pci_config_set_device_id(dev-config, hdev-device_id);
+
+xen_host_pci_get_block(hdev, PCI_REVISION_ID, (uint8_t *)rid, 1);


Host PCI device is the VGA card?


This is a real ISA bridge.


So why does it make sense to get it's vendor/device/revision and
stick in the ISA bridge?


The Intel generation of integrated graphics needs to probe this ISA bridge
to initialize the i915 driver properly.

Thanks
Tiejun


So vendor/device/revision IDs must match real device then?


Yes, the i915 driver needs these information of PCH to determine how to 
work. And this is just why we expose this ISA bridge to guest ugly :(



Stick this in the comment then.



Sure.


In fact it's exactly what passthrough does.
I wonder if more bits from ./hw/i386/kvm/pci-assign.c
can be reused. How do you poke at the host device? sysfs?


Yes, sysfs.

Thanks
Tiejun







Also change rid to uint8_t, you won't need to cast then.


+
+pci_config_set_revision(dev-config, rid);
+
+XEN_PT_LOG(dev, The pseudo Intel PCH ISA bridge created.\n);
+return 0;
+}
--
1.9.1











Re: [Qemu-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support

2014-06-25 Thread Paolo Bonzini

Il 25/06/2014 10:31, Michael S. Tsirkin ha scritto:

It might be possible to move the Q35 bridge elsewhere.
seabios doesn't care where the host bridge is.
ACPI tables in QEMU can be adjusted.


But why?  It's always in 00:1f.0 on real hardware.  If the i915 driver 
wants to run under virtual machines, it should stop acting as if it 
knows what the whole machine looks like.


Paolo



Re: [Qemu-devel] [v5][PATCH 2/5] xen, gfx passthrough: create pseudo intel isa bridge

2014-06-25 Thread Michael S. Tsirkin
On Wed, Jun 25, 2014 at 04:39:07PM +0800, Chen, Tiejun wrote:
 In fact it's exactly what passthrough does.
 I wonder if more bits from ./hw/i386/kvm/pci-assign.c
 can be reused. How do you poke at the host device? sysfs?
 
 Yes, sysfs.
 
 Thanks
 Tiejun

Then you should be able to re-use large chunks of
./hw/i386/kvm/pci-assign.c: basically everything
that deals with emulation.





Re: [Qemu-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support

2014-06-25 Thread Michael S. Tsirkin
On Wed, Jun 25, 2014 at 10:39:43AM +0200, Paolo Bonzini wrote:
 Il 25/06/2014 10:31, Michael S. Tsirkin ha scritto:
 It might be possible to move the Q35 bridge elsewhere.
 seabios doesn't care where the host bridge is.
 ACPI tables in QEMU can be adjusted.
 
 But why?  It's always in 00:1f.0 on real hardware.  If the i915 driver wants
 to run under virtual machines, it should stop acting as if it knows what the
 whole machine looks like.
 
 Paolo

If guest driver can be fixed that seems ideal.
I don't know how it works but presumably you guys do?

-- 
MST



Re: [Qemu-devel] [v5][PATCH 2/5] xen, gfx passthrough: create pseudo intel isa bridge

2014-06-25 Thread Chen, Tiejun

On 2014/6/25 16:43, Michael S. Tsirkin wrote:

On Wed, Jun 25, 2014 at 04:39:07PM +0800, Chen, Tiejun wrote:

In fact it's exactly what passthrough does.
I wonder if more bits from ./hw/i386/kvm/pci-assign.c
can be reused. How do you poke at the host device? sysfs?


Yes, sysfs.

Thanks
Tiejun


Then you should be able to re-use large chunks of
./hw/i386/kvm/pci-assign.c: basically everything
that deals with emulation.


Do you mean those hooks to get info from the real device? Xen have its 
own wrapper, xen_host_pci_get_block(), so we always go there in xen 
scenario.


Thanks
Tiejun









[Qemu-devel] [PATCH] qemu_opts_append: Play nicely with QemuOptsList's head

2014-06-25 Thread Michal Privoznik
When running a libvirt test suite I've noticed the qemu-img is
crashing occasionally. Tracing the problem down led me to the
following valgrind output:

qemu.git $ valgrind -q ./qemu-img create -f qed 
-obacking_file=/dev/null,backing_fmt=raw qed
==14881== Invalid write of size 8
==14881==at 0x1D263F: qemu_opts_create (qemu-option.c:692)
==14881==by 0x130782: bdrv_img_create (block.c:5531)
==14881==by 0x118DE0: img_create (qemu-img.c:462)
==14881==by 0x11E7E4: main (qemu-img.c:2830)
==14881==  Address 0x11fedd38 is 24 bytes inside a block of size 232 free'd
==14881==at 0x4C2CA5E: realloc (in 
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==14881==by 0x592D35E: g_realloc (in /usr/lib64/libglib-2.0.so.0.3800.2)
==14881==by 0x1D38D8: qemu_opts_append (qemu-option.c:1129)
==14881==by 0x13075E: bdrv_img_create (block.c:5528)
==14881==by 0x118DE0: img_create (qemu-img.c:462)
==14881==by 0x11E7E4: main (qemu-img.c:2830)
==14881==
Formatting 'qed', fmt=qed size=0 backing_file='/dev/null' backing_fmt='raw' 
cluster_size=65536
==14881== Invalid write of size 8
==14881==at 0x1D28BE: qemu_opts_del (qemu-option.c:750)
==14881==by 0x130BF3: bdrv_img_create (block.c:5638)
==14881==by 0x118DE0: img_create (qemu-img.c:462)
==14881==by 0x11E7E4: main (qemu-img.c:2830)
==14881==  Address 0x11fedd38 is 24 bytes inside a block of size 232 free'd
==14881==at 0x4C2CA5E: realloc (in 
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==14881==by 0x592D35E: g_realloc (in /usr/lib64/libglib-2.0.so.0.3800.2)
==14881==by 0x1D38D8: qemu_opts_append (qemu-option.c:1129)
==14881==by 0x13075E: bdrv_img_create (block.c:5528)
==14881==by 0x118DE0: img_create (qemu-img.c:462)
==14881==by 0x11E7E4: main (qemu-img.c:2830)
==14881==

The problem is apparently in the qemu_opts_append(). Well, if it
gets called twice or more. On the first call, when @dst is NULL
some initialization is done during which @dst-head list gets
initialized. The list is initialized in a way, so that the list
tail points at the list head. However, the next time
qemu_opts_append() is called for new options to be added,
g_realloc() may move @dst at new address making the old list tail
point at invalid address. If that's the case we must update the
list pointers.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 util/qemu-option.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/util/qemu-option.c b/util/qemu-option.c
index 43de3ad..6ad2cf2 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -,6 +,7 @@ QemuOptsList *qemu_opts_append(QemuOptsList *dst,
 size_t num_opts, num_dst_opts;
 QemuOptDesc *desc;
 bool need_init = false;
+bool need_head_update;
 
 if (!list) {
 return dst;
@@ -1121,6 +1122,12 @@ QemuOptsList *qemu_opts_append(QemuOptsList *dst,
  */
 if (!dst) {
 need_init = true;
+need_head_update = true;
+} else {
+/* Moreover, even if dst is not NULL, the realloc may move it at a
+ * different address in which case we may get a stale tail pointer
+ * in dst-head. */
+need_head_update = QTAILQ_EMPTY(dst-head);
 }
 
 num_opts = count_opts_list(dst);
@@ -1131,9 +1138,11 @@ QemuOptsList *qemu_opts_append(QemuOptsList *dst,
 if (need_init) {
 dst-name = NULL;
 dst-implied_opt_name = NULL;
-QTAILQ_INIT(dst-head);
 dst-merge_lists = false;
 }
+if (need_head_update) {
+QTAILQ_INIT(dst-head);
+}
 dst-desc[num_dst_opts].name = NULL;
 
 /* append list-desc to dst-desc */
-- 
1.8.5.5




Re: [Qemu-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support

2014-06-25 Thread Chen, Tiejun

On 2014/6/25 16:48, Michael S. Tsirkin wrote:

On Wed, Jun 25, 2014 at 10:39:43AM +0200, Paolo Bonzini wrote:

Il 25/06/2014 10:31, Michael S. Tsirkin ha scritto:

It might be possible to move the Q35 bridge elsewhere.
seabios doesn't care where the host bridge is.
ACPI tables in QEMU can be adjusted.


But why?  It's always in 00:1f.0 on real hardware.  If the i915 driver wants
to run under virtual machines, it should stop acting as if it knows what the
whole machine looks like.

Paolo


If guest driver can be fixed that seems ideal.
I don't know how it works but presumably you guys do?


Paolo prefer we need to fix this in the driver like:


 The right way could be to make QEMU add a vendor-specific capability to
 the video device. The driver can probe for that capability before

 Do you mean we can pick two unused offsets in the configuration space of
 the video device as a vendor-specific capability to hold the
 vendor/device ids of the PCH?

 Yes, either that or add a new capability (which lets you choose the
 offsets more freely).

 If the IGD driver needs config space fields of the MCH, those fields
 could also be mirrored in the new capability.  QEMU would forward them
 automatically.

 It could even be a new BAR, which gives even more freedom to allocate
 the fields.


Thanks
Tiejun



[Qemu-devel] [PATCH] check if we have space left for hotplugged memory

2014-06-25 Thread Hu Tao
If pc-dimm is specified on qemu command line, but only with
-m size (aka not -m size,maxmem,slots) then qemu will core dump.

This patch fixes the problem.

Signed-off-by: Hu Tao hu...@cn.fujitsu.com
---
 hw/i386/pc.c | 5 +
 hw/mem/pc-dimm.c | 5 +
 2 files changed, 10 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2cf22b1..1a0d96e 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1566,6 +1566,11 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
 goto out;
 }
 
+if (memory_region_size(pcms-hotplug_memory) == 0) {
+error_setg(local_err, no space left for hotplugged memory.);
+goto out;
+}
+
 addr = pc_dimm_get_free_addr(pcms-hotplug_memory_base,
  memory_region_size(pcms-hotplug_memory),
  !addr ? NULL : addr,
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index ad176b7..59ed9c0 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -146,6 +146,11 @@ uint64_t pc_dimm_get_free_addr(uint64_t 
address_space_start,
 uint64_t new_addr, ret = 0;
 uint64_t address_space_end = address_space_start + address_space_size;
 
+if (address_space_size == 0) {
+error_setg(errp, no space left for hotplugged memory.);
+goto out;
+}
+
 assert(address_space_end  address_space_size);
 object_child_foreach(qdev_get_machine(), pc_dimm_built_list, list);
 
-- 
1.9.3




Re: [Qemu-devel] [v5][PATCH 2/5] xen, gfx passthrough: create pseudo intel isa bridge

2014-06-25 Thread Michael S. Tsirkin
On Wed, Jun 25, 2014 at 04:48:02PM +0800, Chen, Tiejun wrote:
 On 2014/6/25 16:43, Michael S. Tsirkin wrote:
 On Wed, Jun 25, 2014 at 04:39:07PM +0800, Chen, Tiejun wrote:
 In fact it's exactly what passthrough does.
 I wonder if more bits from ./hw/i386/kvm/pci-assign.c
 can be reused. How do you poke at the host device? sysfs?
 
 Yes, sysfs.
 
 Thanks
 Tiejun
 
 Then you should be able to re-use large chunks of
 ./hw/i386/kvm/pci-assign.c: basically everything
 that deals with emulation.
 
 Do you mean those hooks to get info from the real device? Xen have its own
 wrapper, xen_host_pci_get_block(), so we always go there in xen scenario.
 
 Thanks
 Tiejun

Yes and that's not good.  We have two pieces of code doing mostly
identical things slightly differently.
hw/i386/kvm/pci-assign.c is a bit younger so it's cleaner,
but these really need to be unified.

 
 
 
 



[Qemu-devel] [PATCH] numa: check for busy memory backend

2014-06-25 Thread Hu Tao
..to prevent one memory backend from being used by more than one numa
node.

Signed-off-by: Hu Tao hu...@cn.fujitsu.com
---
 numa.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/numa.c b/numa.c
index e471afe..6c1c554 100644
--- a/numa.c
+++ b/numa.c
@@ -279,6 +279,13 @@ void memory_region_allocate_system_memory(MemoryRegion 
*mr, Object *owner,
 exit(1);
 }
 
+if (memory_region_is_mapped(seg)) {
+char *path = object_get_canonical_path_component(OBJECT(backend));
+error_report(memory backend %s is busy, path);
+g_free(path);
+exit(1);
+}
+
 memory_region_add_subregion(mr, addr, seg);
 vmstate_register_ram_global(seg);
 addr += size;
-- 
1.9.3




Re: [Qemu-devel] [v5][PATCH 1/5] xen, gfx passthrough: basic graphics passthrough support

2014-06-25 Thread Chen, Tiejun

On 2014/6/25 14:35, Michael S. Tsirkin wrote:

On Wed, Jun 25, 2014 at 10:17:17AM +0800, Tiejun Chen wrote:

basic gfx passthrough support:
- add a vga type for gfx passthrough
- retrieve VGA bios from sysfs, then load it to guest at 0xC
- register/unregister legacy VGA I/O ports and MMIOs for passthrough GFX

The original patch is from Weidong Han weidong@intel.com

Signed-off-by: Yang Zhang yang.z.zh...@intel.com
Signed-off-by: Tiejun Chen tiejun.c...@intel.com
Cc: Weidong Han weidong@intel.com


ROM loading code is duplicated from assigned_dev_load_option_rom.


Yes. Do you hint we need to split this?


Would it be a problem for you to create a memory region holding
the ROM?


Sorry, could you tell me what should do exactly?


You won't need cpu_physical_memory_rw then, either, or need


How to make sure this is fixed at 0xc?

Thanks
Tiejun


the VGA_BIOS_DEFAULT_SIZE hack.





---
v5:

* Just rebase.

v4:

* Fix some typos in the patch head description.
* Improve some comments.
* Given that xen_pt_register_vga_regions()/xen_pt_unregister_vga_regions()
   are called unconditionally, so we just return 0 there.
* Remove one spurious change.

v3:

* Fix some typos.
* Add more comments to make that readable.
* Improve some return paths.

v2:

* retrieve VGA bios from sysfs properly.
* redefine some function name.

  hw/xen/Makefile.objs |   2 +-
  hw/xen/xen-host-pci-device.c |   5 +
  hw/xen/xen-host-pci-device.h |   1 +
  hw/xen/xen_pt.c  |  10 ++
  hw/xen/xen_pt.h  |   4 +
  hw/xen/xen_pt_graphics.c | 232 +++
  qemu-options.hx  |   9 ++
  vl.c |  10 ++
  8 files changed, 272 insertions(+), 1 deletion(-)
  create mode 100644 hw/xen/xen_pt_graphics.c

diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs
index a0ca0aa..77b7dae 100644
--- a/hw/xen/Makefile.objs
+++ b/hw/xen/Makefile.objs
@@ -2,4 +2,4 @@
  common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o

  obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o
-obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o xen_pt_msi.o
+obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o 
xen_pt_msi.o xen_pt_graphics.o
diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
index 743b37b..a54b7de 100644
--- a/hw/xen/xen-host-pci-device.c
+++ b/hw/xen/xen-host-pci-device.c
@@ -376,6 +376,11 @@ int xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t 
domain,
  goto error;
  }
  d-irq = v;
+rc = xen_host_pci_get_hex_value(d, class, v);
+if (rc) {
+goto error;
+}
+d-class_code = v;
  d-is_virtfn = xen_host_pci_dev_is_virtfn(d);

  return 0;
diff --git a/hw/xen/xen-host-pci-device.h b/hw/xen/xen-host-pci-device.h
index c2486f0..f1e1c30 100644
--- a/hw/xen/xen-host-pci-device.h
+++ b/hw/xen/xen-host-pci-device.h
@@ -25,6 +25,7 @@ typedef struct XenHostPCIDevice {

  uint16_t vendor_id;
  uint16_t device_id;
+uint32_t class_code;
  int irq;

  XenHostPCIIORegion io_regions[PCI_NUM_REGIONS - 1];
diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index be4220b..dac4238 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -450,6 +450,7 @@ static int xen_pt_register_regions(XenPCIPassthroughState 
*s)
 d-rom.size, d-rom.base_addr);
  }

+xen_pt_register_vga_regions(d);
  return 0;
  }

@@ -470,6 +471,8 @@ static void 
xen_pt_unregister_regions(XenPCIPassthroughState *s)
  if (d-rom.base_addr  d-rom.size) {
  memory_region_destroy(s-rom);
  }
+
+xen_pt_unregister_vga_regions(d);
  }

  /* region mapping */
@@ -693,6 +696,13 @@ static int xen_pt_initfn(PCIDevice *d)
  /* Handle real device's MMIO/PIO BARs */
  xen_pt_register_regions(s);

+/* Setup VGA bios for passthrough GFX */
+if (xen_pt_setup_vga(s-real_device)  0) {
+XEN_PT_ERR(d, Setup VGA BIOS of passthrough GFX failed!\n);
+xen_host_pci_device_put(s-real_device);
+return -1;
+}
+
  /* reinitialize each config register to be emulated */
  if (xen_pt_config_init(s)) {
  XEN_PT_ERR(d, PCI Config space initialisation failed.\n);
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 942dc60..4d3a18d 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -298,5 +298,9 @@ static inline bool 
xen_pt_has_msix_mapping(XenPCIPassthroughState *s, int bar)
  return s-msix  s-msix-bar_index == bar;
  }

+extern int xen_has_gfx_passthru;
+int xen_pt_register_vga_regions(XenHostPCIDevice *dev);
+int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev);
+int xen_pt_setup_vga(XenHostPCIDevice *dev);

  #endif /* !XEN_PT_H */
diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
new file mode 100644
index 000..461e526
--- /dev/null
+++ b/hw/xen/xen_pt_graphics.c
@@ -0,0 +1,232 @@
+/*
+ * graphics passthrough
+ */
+#include xen_pt.h
+#include 

Re: [Qemu-devel] [PATCH] qemu-char: fix warning 'res' may be used uninitialized

2014-06-25 Thread Markus Armbruster
Igor Mammedov imamm...@redhat.com writes:

 Signed-off-by: Igor Mammedov imamm...@redhat.com

Broken in commit 9005b2a (author cc'ed).  Bites only when passed zero
len, which seems unlikely.

Reviewed-by: Markus Armbruster arm...@redhat.com



Re: [Qemu-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support

2014-06-25 Thread Michael S. Tsirkin
On Wed, Jun 25, 2014 at 04:55:25PM +0800, Chen, Tiejun wrote:
 On 2014/6/25 16:48, Michael S. Tsirkin wrote:
 On Wed, Jun 25, 2014 at 10:39:43AM +0200, Paolo Bonzini wrote:
 Il 25/06/2014 10:31, Michael S. Tsirkin ha scritto:
 It might be possible to move the Q35 bridge elsewhere.
 seabios doesn't care where the host bridge is.
 ACPI tables in QEMU can be adjusted.
 
 But why?  It's always in 00:1f.0 on real hardware.  If the i915 driver wants
 to run under virtual machines, it should stop acting as if it knows what the
 whole machine looks like.
 
 Paolo
 
 If guest driver can be fixed that seems ideal.
 I don't know how it works but presumably you guys do?
 
 Paolo prefer we need to fix this in the driver like:
 
 
  The right way could be to make QEMU add a vendor-specific capability to
  the video device. The driver can probe for that capability before
 
  Do you mean we can pick two unused offsets in the configuration space of
  the video device as a vendor-specific capability to hold the
  vendor/device ids of the PCH?
 
  Yes, either that or add a new capability (which lets you choose the
  offsets more freely).
 
  If the IGD driver needs config space fields of the MCH, those fields
  could also be mirrored in the new capability.  QEMU would forward them
  automatically.
 
  It could even be a new BAR, which gives even more freedom to allocate
  the fields.
 
 
 Thanks
 Tiejun

Adding a vendor-specific capability or BAR
in an existing device is painful - hard to find
free space for it.
We could add a dummy device in PCI or ACPI, but
driver should really look for it using device/vendor id,
not at a specific address.
What exactly is it that the driver wants to know?

-- 
MST



Re: [Qemu-devel] [v5][PATCH 2/5] xen, gfx passthrough: create pseudo intel isa bridge

2014-06-25 Thread Chen, Tiejun

On 2014/6/25 17:04, Michael S. Tsirkin wrote:

On Wed, Jun 25, 2014 at 04:48:02PM +0800, Chen, Tiejun wrote:

On 2014/6/25 16:43, Michael S. Tsirkin wrote:

On Wed, Jun 25, 2014 at 04:39:07PM +0800, Chen, Tiejun wrote:

In fact it's exactly what passthrough does.
I wonder if more bits from ./hw/i386/kvm/pci-assign.c
can be reused. How do you poke at the host device? sysfs?


Yes, sysfs.

Thanks
Tiejun


Then you should be able to re-use large chunks of
./hw/i386/kvm/pci-assign.c: basically everything
that deals with emulation.


Do you mean those hooks to get info from the real device? Xen have its own
wrapper, xen_host_pci_get_block(), so we always go there in xen scenario.

Thanks
Tiejun


Yes and that's not good.  We have two pieces of code doing mostly
identical things slightly differently.
hw/i386/kvm/pci-assign.c is a bit younger so it's cleaner,
but these really need to be unified.



Sorry, take a look at this again,

xen_host_pci_get_block(XenHostPCIDevice *d, int pos, uint8_t *buf, int len)
|
+ xen_host_pci_config_read(d, pos, buf, len)
|
+ pread(d-config_fd, buf, len, pos)

I thinks this should be same as kvm.

Thanks
Tiejun



Re: [Qemu-devel] [PATCH] check if we have space left for hotplugged memory

2014-06-25 Thread Michael S. Tsirkin
On Wed, Jun 25, 2014 at 05:01:04PM +0800, Hu Tao wrote:
 If pc-dimm is specified on qemu command line, but only with
 -m size (aka not -m size,maxmem,slots) then qemu will core dump.
 
 This patch fixes the problem.
 
 Signed-off-by: Hu Tao hu...@cn.fujitsu.com
 ---
  hw/i386/pc.c | 5 +
  hw/mem/pc-dimm.c | 5 +
  2 files changed, 10 insertions(+)
 
 diff --git a/hw/i386/pc.c b/hw/i386/pc.c
 index 2cf22b1..1a0d96e 100644
 --- a/hw/i386/pc.c
 +++ b/hw/i386/pc.c
 @@ -1566,6 +1566,11 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
  goto out;
  }
  
 +if (memory_region_size(pcms-hotplug_memory) == 0) {
 +error_setg(local_err, no space left for hotplugged memory.);
 +goto out;
 +}
 +
  addr = pc_dimm_get_free_addr(pcms-hotplug_memory_base,
   memory_region_size(pcms-hotplug_memory),
   !addr ? NULL : addr,


Why two errors in two places?

 diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
 index ad176b7..59ed9c0 100644
 --- a/hw/mem/pc-dimm.c
 +++ b/hw/mem/pc-dimm.c
 @@ -146,6 +146,11 @@ uint64_t pc_dimm_get_free_addr(uint64_t 
 address_space_start,
  uint64_t new_addr, ret = 0;
  uint64_t address_space_end = address_space_start + address_space_size;
  
 +if (address_space_size == 0) {
 +error_setg(errp, no space left for hotplugged memory.);
 +goto out;
 +}
 +
  assert(address_space_end  address_space_size);
  object_child_foreach(qdev_get_machine(), pc_dimm_built_list, list);
  


Didn't we want a more specific message?

 -- 
 1.9.3



Re: [Qemu-devel] [PATCH] linux-user: Handle new ARM breakpoint instruction

2014-06-25 Thread Peter Maydell
On 25 June 2014 05:08, Hunter Laux hunterl...@gmail.com wrote:
 This instruction space is guaranteed to be undefined.
 ARM:    0111      
 Thumb: 1101 1110  

 The breakpoint instructions were selected from this instruction space.
 Linux traps the illegal instruction and sends a SIGTRAP if it is a breakpoint.

 Here is the Linux implementation:
 http://lxr.free-electrons.com/source/arch/arm/kernel/ptrace.c#L221

 Signed-off-by: Hunter Laux hunterl...@gmail.com
 ---
  linux-user/main.c | 18 ++
  1 file changed, 18 insertions(+)

 diff --git a/linux-user/main.c b/linux-user/main.c
 index 900a17f..91f2681 100644
 --- a/linux-user/main.c
 +++ b/linux-user/main.c
 @@ -688,11 +688,29 @@ void cpu_loop(CPUARMState *env)
  uint32_t opcode;
  int rc;

 +const uint32_t arm_bkpt_mask= 0x0FFF;
 +const uint32_t arm_bkpt = 0x07F001F0;
 +const uint32_t arm_bkpt_thumb_mask  = 0x;
 +const uint32_t arm_bkpt_thumb   = 0xDE01;
 +const uint32_t arm_bkpt_thumb2_mask = 0x;
 +const uint32_t arm_bkpt_thumb2  = 0xF7F0A000;
 +
  /* we handle the FPU emulation here, as Linux */
  /* we get the opcode */
  /* FIXME - what to do if get_user() fails? */
  get_user_code_u32(opcode, env-regs[15], env-bswap_code);

 +if (env-thumb) {
 +if ((opcode  arm_bkpt_thumb_mask) == arm_bkpt_thumb
 +|| (opcode  arm_bkpt_thumb2_mask) == 
 arm_bkpt_thumb2) {
 +goto excp_debug;
 +}
 +} else {
 +if ((opcode  arm_bkpt_mask) == arm_bkpt) {
 +goto excp_debug;
 +}
 +}
 +
  rc = EmulateAll(opcode, ts-fpa, env);
  if (rc == 0) { /* illegal instruction */
  info.si_signo = SIGILL;

This shouldn't be necessary, because our instruction decoder
causes the BKPT instructions to generate an EXCP_BKPT
(see target-arm/translate.c). So we should never go through
this code path for these instructions...

thanks
-- PMM



Re: [Qemu-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support

2014-06-25 Thread Chen, Tiejun

On 2014/6/25 17:09, Michael S. Tsirkin wrote:

On Wed, Jun 25, 2014 at 04:55:25PM +0800, Chen, Tiejun wrote:

On 2014/6/25 16:48, Michael S. Tsirkin wrote:

On Wed, Jun 25, 2014 at 10:39:43AM +0200, Paolo Bonzini wrote:

Il 25/06/2014 10:31, Michael S. Tsirkin ha scritto:

It might be possible to move the Q35 bridge elsewhere.
seabios doesn't care where the host bridge is.
ACPI tables in QEMU can be adjusted.


But why?  It's always in 00:1f.0 on real hardware.  If the i915 driver wants
to run under virtual machines, it should stop acting as if it knows what the
whole machine looks like.

Paolo


If guest driver can be fixed that seems ideal.
I don't know how it works but presumably you guys do?


Paolo prefer we need to fix this in the driver like:



The right way could be to make QEMU add a vendor-specific capability to
the video device. The driver can probe for that capability before


Do you mean we can pick two unused offsets in the configuration space of
the video device as a vendor-specific capability to hold the
vendor/device ids of the PCH?


Yes, either that or add a new capability (which lets you choose the
offsets more freely).

If the IGD driver needs config space fields of the MCH, those fields
could also be mirrored in the new capability.  QEMU would forward them
automatically.

It could even be a new BAR, which gives even more freedom to allocate
the fields.



Thanks
Tiejun


Adding a vendor-specific capability or BAR
in an existing device is painful - hard to find
free space for it.


Yes, this is a potential risk as well since we can't guarantee current 
free space is always valid for ever.



We could add a dummy device in PCI or ACPI, but
driver should really look for it using device/vendor id,
not at a specific address.
What exactly is it that the driver wants to know?



The i915 driver need to use the vendor/device ids to get what pch type 
is running, then go different paths.


Thanks
Tiejun




Re: [Qemu-devel] [v5][PATCH 2/5] xen, gfx passthrough: create pseudo intel isa bridge

2014-06-25 Thread Michael S. Tsirkin
On Wed, Jun 25, 2014 at 05:14:30PM +0800, Chen, Tiejun wrote:
 On 2014/6/25 17:04, Michael S. Tsirkin wrote:
 On Wed, Jun 25, 2014 at 04:48:02PM +0800, Chen, Tiejun wrote:
 On 2014/6/25 16:43, Michael S. Tsirkin wrote:
 On Wed, Jun 25, 2014 at 04:39:07PM +0800, Chen, Tiejun wrote:
 In fact it's exactly what passthrough does.
 I wonder if more bits from ./hw/i386/kvm/pci-assign.c
 can be reused. How do you poke at the host device? sysfs?
 
 Yes, sysfs.
 
 Thanks
 Tiejun
 
 Then you should be able to re-use large chunks of
 ./hw/i386/kvm/pci-assign.c: basically everything
 that deals with emulation.
 
 Do you mean those hooks to get info from the real device? Xen have its own
 wrapper, xen_host_pci_get_block(), so we always go there in xen scenario.
 
 Thanks
 Tiejun
 
 Yes and that's not good.  We have two pieces of code doing mostly
 identical things slightly differently.
 hw/i386/kvm/pci-assign.c is a bit younger so it's cleaner,
 but these really need to be unified.
 
 
 Sorry, take a look at this again,
 
 xen_host_pci_get_block(XenHostPCIDevice *d, int pos, uint8_t *buf, int len)
   |
   + xen_host_pci_config_read(d, pos, buf, len)
   |
   + pread(d-config_fd, buf, len, pos)
 
 I thinks this should be same as kvm.
 
 Thanks
 Tiejun

get_block is trivial.

I really mean the whole PT infrastructure for
- discovering host devices through sysfs
- virtualizing devices

rom, bars, msi ...
the list goes on.

logic is mostly the same.



-- 
MST



Re: [Qemu-devel] [PATCH v4 8/9] virtio: fix virtio-blk child refcount in transports

2014-06-25 Thread Peter Crosthwaite
On Wed, Jun 18, 2014 at 7:58 PM, Stefan Hajnoczi stefa...@redhat.com wrote:
 object_initialize() leaves the object with a refcount of 1.
 object_property_add_child() adds its own reference which is dropped
 again when the property is deleted.

 The upshot of this is that we always have a refcount = 1.  Upon hot
 unplug the virtio-blk child is not finalized!

 Drop our reference after the child property has been added to the
 parent.

 Signed-off-by: Stefan Hajnoczi stefa...@redhat.com

Still not a fan, but I can't see a better way without more core QOM or
hotplug plumbing and that's not going to happen inside 2.1. So

Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com

Regards,
Peter

 ---
  hw/s390x/s390-virtio-bus.c | 1 +
  hw/s390x/virtio-ccw.c  | 1 +
  hw/virtio/virtio-pci.c | 1 +
  3 files changed, 3 insertions(+)

 diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
 index 38984ab..3438a88 100644
 --- a/hw/s390x/s390-virtio-bus.c
 +++ b/hw/s390x/s390-virtio-bus.c
 @@ -179,6 +179,7 @@ static void s390_virtio_blk_instance_init(Object *obj)
  VirtIOBlkS390 *dev = VIRTIO_BLK_S390(obj);
  object_initialize(dev-vdev, sizeof(dev-vdev), TYPE_VIRTIO_BLK);
  object_property_add_child(obj, virtio-backend, OBJECT(dev-vdev), 
 NULL);
 +object_unref(OBJECT(dev-vdev));
  qdev_alias_all_properties(DEVICE(dev-vdev), obj);
  }

 diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
 index 9fa6f32..0553fea 100644
 --- a/hw/s390x/virtio-ccw.c
 +++ b/hw/s390x/virtio-ccw.c
 @@ -813,6 +813,7 @@ static void virtio_ccw_blk_instance_init(Object *obj)
  VirtIOBlkCcw *dev = VIRTIO_BLK_CCW(obj);
  object_initialize(dev-vdev, sizeof(dev-vdev), TYPE_VIRTIO_BLK);
  object_property_add_child(obj, virtio-backend, OBJECT(dev-vdev), 
 NULL);
 +object_unref(OBJECT(dev-vdev));
  qdev_alias_all_properties(DEVICE(dev-vdev), obj);
  }

 diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
 index 3bb782f..abf05a9 100644
 --- a/hw/virtio/virtio-pci.c
 +++ b/hw/virtio/virtio-pci.c
 @@ -1102,6 +1102,7 @@ static void virtio_blk_pci_instance_init(Object *obj)
  VirtIOBlkPCI *dev = VIRTIO_BLK_PCI(obj);
  object_initialize(dev-vdev, sizeof(dev-vdev), TYPE_VIRTIO_BLK);
  object_property_add_child(obj, virtio-backend, OBJECT(dev-vdev), 
 NULL);
 +object_unref(OBJECT(dev-vdev));
  qdev_alias_all_properties(DEVICE(dev-vdev), obj);
  }

 --
 1.9.3





Re: [Qemu-devel] [RFC] alpha qemu arithmetic exceptions

2014-06-25 Thread Peter Maydell
On 25 June 2014 08:01, Al Viro v...@zeniv.linux.org.uk wrote:
 Could we steal bit 1 in float_exception_flags for IOV?  It is (currently?)
 unused -
 enum {
 float_flag_invalid   =  1,
 float_flag_divbyzero =  4,
 float_flag_overflow  =  8,
 float_flag_underflow = 16,
 float_flag_inexact   = 32,
 float_flag_input_denormal = 64,
 float_flag_output_denormal = 128
 };

 That would allow to deal with that crap nicely - we could have it raise
 the new flag, then have helper_fp_exc_raise_... for default trap mode
 mask it out (and yes, we need to set FPCR flags in default mode, as well
 as /U and /V - confirmed by direct experiment *and* by TFM).

 If we can't... well, we could put that flag separately, but it would be
 more unpleasant.  Folks?

I think it's OK to put extra float_flags in, provided you can define
their semantics in terms that make sense for more than one
architecture (even if only one arch actually happens to need them).
The input_denormal/output_denormal flags only get used for ARM,
for instance. However if you wanted to split overflow from integer
overflow you'd need to fix up all the other targets which expect
them to generate just one exception flag...

(Note that any patch touching softfloat files needs to come with
a statement that you're happy to license it under either the
softfloat-2a or softfloat-2b licenses, because we're currently
midway through the tedious process of trying to relicense it.)

thanks
-- PMM



Re: [Qemu-devel] [v5][PATCH 2/5] xen, gfx passthrough: create pseudo intel isa bridge

2014-06-25 Thread Chen, Tiejun

On 2014/6/25 17:21, Michael S. Tsirkin wrote:

On Wed, Jun 25, 2014 at 05:14:30PM +0800, Chen, Tiejun wrote:

On 2014/6/25 17:04, Michael S. Tsirkin wrote:

On Wed, Jun 25, 2014 at 04:48:02PM +0800, Chen, Tiejun wrote:

On 2014/6/25 16:43, Michael S. Tsirkin wrote:

On Wed, Jun 25, 2014 at 04:39:07PM +0800, Chen, Tiejun wrote:

In fact it's exactly what passthrough does.
I wonder if more bits from ./hw/i386/kvm/pci-assign.c
can be reused. How do you poke at the host device? sysfs?


Yes, sysfs.

Thanks
Tiejun


Then you should be able to re-use large chunks of
./hw/i386/kvm/pci-assign.c: basically everything
that deals with emulation.


Do you mean those hooks to get info from the real device? Xen have its own
wrapper, xen_host_pci_get_block(), so we always go there in xen scenario.

Thanks
Tiejun


Yes and that's not good.  We have two pieces of code doing mostly
identical things slightly differently.
hw/i386/kvm/pci-assign.c is a bit younger so it's cleaner,
but these really need to be unified.



Sorry, take a look at this again,

xen_host_pci_get_block(XenHostPCIDevice *d, int pos, uint8_t *buf, int len)
|
+ xen_host_pci_config_read(d, pos, buf, len)
|
+ pread(d-config_fd, buf, len, pos)

I thinks this should be same as kvm.

Thanks
Tiejun


get_block is trivial.

I really mean the whole PT infrastructure for
- discovering host devices through sysfs
- virtualizing devices

rom, bars, msi ...
the list goes on.

logic is mostly the same.



Looks you mean we can unify the entire PT infrastructure between kvm and 
xen inside qemu. But I'm afraid its not easy to do in a short time, so 
maybe we can queue this as next phase.


Thanks
Tiejun



[Qemu-devel] [PATCH] Makefile: Install qmp-events.txt

2014-06-25 Thread Michal Privoznik
Since fe069d9d5946 the qmp-events.txt does not exist anymore. This is
unhappy as users still may want to know what events they can await
from qemu.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 Makefile | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 145adb6..b93aa38 100644
--- a/Makefile
+++ b/Makefile
@@ -77,7 +77,7 @@ LIBS+=-lz $(LIBS_TOOLS)
 HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF)
 
 ifdef BUILD_DOCS
-DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8 qmp-commands.txt
+DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8 
qmp-commands.txt qmp-events.txt
 ifdef CONFIG_VIRTFS
 DOCS+=fsdev/virtfs-proxy-helper.1
 endif
@@ -353,6 +353,7 @@ install-doc: $(DOCS)
$(INSTALL_DIR) $(DESTDIR)$(qemu_docdir)
$(INSTALL_DATA) qemu-doc.html  qemu-tech.html $(DESTDIR)$(qemu_docdir)
$(INSTALL_DATA) qmp-commands.txt $(DESTDIR)$(qemu_docdir)
+   $(INSTALL_DATA) qmp-events.txt $(DESTDIR)$(qemu_docdir)
 ifdef CONFIG_POSIX
$(INSTALL_DIR) $(DESTDIR)$(mandir)/man1
$(INSTALL_DATA) qemu.1 $(DESTDIR)$(mandir)/man1
@@ -455,6 +456,9 @@ qemu-monitor.texi: $(SRC_PATH)/hmp-commands.hx
 qmp-commands.txt: $(SRC_PATH)/qmp-commands.hx
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -q  $  $@,  GEN  
 $@)
 
+qmp-events.txt: $(SRC_PATH)/qapi-event.json
+   $(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h  $  $@,  GEN  
 $@)
+
 qemu-img-cmds.texi: $(SRC_PATH)/qemu-img-cmds.hx
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -t  $  $@,  GEN  
 $@)
 
-- 
1.8.5.5




Re: [Qemu-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support

2014-06-25 Thread Paolo Bonzini

Il 25/06/2014 11:21, Chen, Tiejun ha scritto:

Adding a vendor-specific capability or BAR
in an existing device is painful - hard to find
free space for it.


Yes, this is a potential risk as well since we can't guarantee current
free space is always valid for ever.


For past devices, we know which BARs they use.  For future devices, it 
would be nice if the PCH/MCH backdoor was specified so that we know they 
will leave a free BAR for virtualization.


Paolo



Re: [Qemu-devel] [Bug 1332297] Re: qemu-img: crash on check of an image with large value in the 'size' header field

2014-06-25 Thread M.Kustova
On Tue, Jun 24, 2014 at 7:36 PM, Kevin Wolf kw...@redhat.com wrote:
 Am 24.06.2014 um 15:19 hat M.Kustova geschrieben:
 On Mon, Jun 23, 2014 at 12:02 PM, Stefan Hajnoczi stefa...@gmail.com wrote:
  On Thu, Jun 19, 2014 at 07:19:55PM -, Maria Kustova wrote:
  The bug description missed qemu-img error:
 
  (process:12283): GLib-ERROR **: gmem.c:110: failed to allocate
  18446744059294601304 bytes
 
  Thanks, there has been recent work by Kevin Wolf to handle memory
  allocation failures gracefully without terminating QEMU.  This sounds
  like a candidate for g_try_malloc() and friends.
 
  Does the following patch series solve the problem?
  https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg01275.html

 These patches are conflicting with current master. So I can't test
 them as they are.

 Do you have a developer repository or branch containing these patches,
 so I could test it on the pre-release base?

 I'm just about to send a new version, I'll keep you CCed there.

[PATCH v4 21/21] qcow2: Return useful error code in refcount_init()
is still broken for the current master.


 Kevin

Maria.



Re: [Qemu-devel] [PATCH] Makefile: Install qmp-events.txt

2014-06-25 Thread Paolo Bonzini

Il 25/06/2014 11:30, Michal Privoznik ha scritto:

Since fe069d9d5946 the qmp-events.txt does not exist anymore. This is
unhappy as users still may want to know what events they can await
from qemu.


hxtool however doesn't understand the json format. :(  I guess for now 
we could just revert the deletion, and aim at automatically-generating 
the documentation in the future.


Paolo



Re: [Qemu-devel] [PATCH] check if we have space left for hotplugged memory

2014-06-25 Thread Hu Tao
On Wed, Jun 25, 2014 at 12:17:57PM +0300, Michael S. Tsirkin wrote:
 On Wed, Jun 25, 2014 at 05:01:04PM +0800, Hu Tao wrote:
  If pc-dimm is specified on qemu command line, but only with
  -m size (aka not -m size,maxmem,slots) then qemu will core dump.
  
  This patch fixes the problem.
  
  Signed-off-by: Hu Tao hu...@cn.fujitsu.com
  ---
   hw/i386/pc.c | 5 +
   hw/mem/pc-dimm.c | 5 +
   2 files changed, 10 insertions(+)
  
  diff --git a/hw/i386/pc.c b/hw/i386/pc.c
  index 2cf22b1..1a0d96e 100644
  --- a/hw/i386/pc.c
  +++ b/hw/i386/pc.c
  @@ -1566,6 +1566,11 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
   goto out;
   }
   
  +if (memory_region_size(pcms-hotplug_memory) == 0) {
  +error_setg(local_err, no space left for hotplugged memory.);
  +goto out;
  +}
  +
   addr = pc_dimm_get_free_addr(pcms-hotplug_memory_base,
memory_region_size(pcms-hotplug_memory),
!addr ? NULL : addr,
 
 
 Why two errors in two places?

This is what Igor suggested(IIUC) which I think is reasonable. The check
here prevents further operation even if there is no call to
pc_dimm_get_free_addr.

 
  diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
  index ad176b7..59ed9c0 100644
  --- a/hw/mem/pc-dimm.c
  +++ b/hw/mem/pc-dimm.c
  @@ -146,6 +146,11 @@ uint64_t pc_dimm_get_free_addr(uint64_t 
  address_space_start,
   uint64_t new_addr, ret = 0;
   uint64_t address_space_end = address_space_start + address_space_size;
   
  +if (address_space_size == 0) {
  +error_setg(errp, no space left for hotplugged memory.);
  +goto out;
  +}
  +
   assert(address_space_end  address_space_size);
   object_child_foreach(qdev_get_machine(), pc_dimm_built_list, list);
   
 
 
 Didn't we want a more specific message?

What about:

  no space left for hotplugged memory, please check maxmem option

?

Igor pointed out that explicitly state -m maxmem=maxmem here may not
sync with future -m change.


Hu



Re: [Qemu-devel] [PATCH] Makefile: Install qmp-events.txt

2014-06-25 Thread Michal Privoznik

On 25.06.2014 11:34, Paolo Bonzini wrote:

Il 25/06/2014 11:30, Michal Privoznik ha scritto:

Since fe069d9d5946 the qmp-events.txt does not exist anymore. This is
unhappy as users still may want to know what events they can await
from qemu.


hxtool however doesn't understand the json format. :(  I guess for now
we could just revert the deletion, and aim at automatically-generating
the documentation in the future.


Well, the patch is using 'hxtool -h' which effectively is 'cp'. Yes, it 
is hackish.


Michal



Re: [Qemu-devel] [PATCH qom v2 0/4] QOMify IRQs

2014-06-25 Thread Peter Crosthwaite
Ping!

This is fully reviewed and should be rdy for a merge. I'd like to see
this through for 2.1.

Regards,
Peter

On Wed, Jun 18, 2014 at 5:53 PM, Peter Crosthwaite
peter.crosthwa...@xilinx.com wrote:
 Hi Andreas and all,

 I have done some cleanup of your WIP IRQ QOMification and have it in a
 hopefully ready state. Its now link safe and the allocation/freeing
 process is not as complex as before.

 For fuller context of the motivation behind this series, please see:
 http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03265.html

 changed since v1:
 Fixed sh4 instance of [0] bug (Kirill review)

 Regards,
 Peter


 Andreas Färber (3):
   sdhci: Fix misuse of qemu_free_irqs()
   hw: Fix qemu_allocate_irqs() leaks
   irq: Slim conversion of qemu_irq to QOM

 Peter Crosthwaite (1):
   irq: Allocate IRQs individually

  hw/arm/omap1.c  | 14 +++---
  hw/arm/omap2.c  |  2 +-
  hw/arm/pxa2xx.c |  4 ++--
  hw/arm/spitz.c  |  4 ++--
  hw/arm/z2.c |  2 +-
  hw/char/serial-pci.c|  2 +-
  hw/core/irq.c   | 46 +-
  hw/core/qdev.c  |  2 +-
  hw/dma/omap_dma.c   |  4 ++--
  hw/ide/microdrive.c |  2 +-
  hw/ipack/ipack.c|  2 +-
  hw/misc/cbus.c  |  6 +++---
  hw/pcmcia/pxa2xx.c  |  2 +-
  hw/sd/omap_mmc.c|  2 +-
  hw/sd/sdhci.c   |  8 
  hw/sh4/sh7750.c |  3 +--
  hw/timer/omap_gptimer.c |  4 ++--
  include/hw/irq.h|  4 +++-
  18 files changed, 63 insertions(+), 50 deletions(-)

 --
 2.0.0




Re: [Qemu-devel] [Bug 1332297] Re: qemu-img: crash on check of an image with large value in the 'size' header field

2014-06-25 Thread Kevin Wolf
Am 25.06.2014 um 11:32 hat M.Kustova geschrieben:
 On Tue, Jun 24, 2014 at 7:36 PM, Kevin Wolf kw...@redhat.com wrote:
  Am 24.06.2014 um 15:19 hat M.Kustova geschrieben:
  On Mon, Jun 23, 2014 at 12:02 PM, Stefan Hajnoczi stefa...@gmail.com 
  wrote:
   On Thu, Jun 19, 2014 at 07:19:55PM -, Maria Kustova wrote:
   The bug description missed qemu-img error:
  
   (process:12283): GLib-ERROR **: gmem.c:110: failed to allocate
   18446744059294601304 bytes
  
   Thanks, there has been recent work by Kevin Wolf to handle memory
   allocation failures gracefully without terminating QEMU.  This sounds
   like a candidate for g_try_malloc() and friends.
  
   Does the following patch series solve the problem?
   https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg01275.html
 
  These patches are conflicting with current master. So I can't test
  them as they are.
 
  Do you have a developer repository or branch containing these patches,
  so I could test it on the pre-release base?
 
  I'm just about to send a new version, I'll keep you CCed there.
 
 [PATCH v4 21/21] qcow2: Return useful error code in refcount_init()
 is still broken for the current master.

In which way? I can cleanly apply the whole patch series on master (even
tried applying the emails from my inbox to be sure).

Kevin



Re: [Qemu-devel] [v5][PATCH 2/5] xen, gfx passthrough: create pseudo intel isa bridge

2014-06-25 Thread Michael S. Tsirkin
On Wed, Jun 25, 2014 at 05:28:48PM +0800, Chen, Tiejun wrote:
 On 2014/6/25 17:21, Michael S. Tsirkin wrote:
 On Wed, Jun 25, 2014 at 05:14:30PM +0800, Chen, Tiejun wrote:
 On 2014/6/25 17:04, Michael S. Tsirkin wrote:
 On Wed, Jun 25, 2014 at 04:48:02PM +0800, Chen, Tiejun wrote:
 On 2014/6/25 16:43, Michael S. Tsirkin wrote:
 On Wed, Jun 25, 2014 at 04:39:07PM +0800, Chen, Tiejun wrote:
 In fact it's exactly what passthrough does.
 I wonder if more bits from ./hw/i386/kvm/pci-assign.c
 can be reused. How do you poke at the host device? sysfs?
 
 Yes, sysfs.
 
 Thanks
 Tiejun
 
 Then you should be able to re-use large chunks of
 ./hw/i386/kvm/pci-assign.c: basically everything
 that deals with emulation.
 
 Do you mean those hooks to get info from the real device? Xen have its own
 wrapper, xen_host_pci_get_block(), so we always go there in xen scenario.
 
 Thanks
 Tiejun
 
 Yes and that's not good.  We have two pieces of code doing mostly
 identical things slightly differently.
 hw/i386/kvm/pci-assign.c is a bit younger so it's cleaner,
 but these really need to be unified.
 
 
 Sorry, take a look at this again,
 
 xen_host_pci_get_block(XenHostPCIDevice *d, int pos, uint8_t *buf, int len)
 |
 + xen_host_pci_config_read(d, pos, buf, len)
 |
 + pread(d-config_fd, buf, len, pos)
 
 I thinks this should be same as kvm.
 
 Thanks
 Tiejun
 
 get_block is trivial.
 
 I really mean the whole PT infrastructure for
 - discovering host devices through sysfs
 - virtualizing devices
 
 rom, bars, msi ...
 the list goes on.
 
 logic is mostly the same.
 
 
 Looks you mean we can unify the entire PT infrastructure between kvm and xen
 inside qemu. But I'm afraid its not easy to do in a short time, so maybe we
 can queue this as next phase.
 
 Thanks
 Tiejun

I'm afraid once we merge your code, you'll lose interest :)

At least, don't add duplicate code for ROM.


-- 
MST



Re: [Qemu-devel] [RFC PATCH 00/13] Light memory region QOMification

2014-06-25 Thread Peter Crosthwaite
On Tue, Jun 17, 2014 at 10:35 PM, Paolo Bonzini pbonz...@redhat.com wrote:
 Il 17/06/2014 14:27, Peter Crosthwaite ha scritto:

 This looks good to me, the only change I see worth making pre-merge is
 moving object_property_add_child_array to the QOM core. All other
 review discussions have been shelved or closed (as they are additive
 suggestions mostly).


 What do you think of the idea of special-casing properties that end with
 [*] directly in object_property_add{,_full}?  This would have to wait for
 2.2, but these patches can go in 2.1 as well.

 I'll submit the series as soon as I get acks for the preliminary patches:

 libqtest;  http://article.gmane.org/gmane.comp.emulators.qemu/279836
 prop-resolve: http://article.gmane.org/gmane.comp.emulators.qemu/279594


Hi Paolo,

Where we at with this? Is it possible to get this through in some form
inside 2.1 window and should I respin it? I think we are largely in
agreement on this series.

Regards,
Peter

 Paolo




Re: [Qemu-devel] [RFC PATCH 00/13] Light memory region QOMification

2014-06-25 Thread Paolo Bonzini

Il 25/06/2014 11:45, Peter Crosthwaite ha scritto:


Where we at with this? Is it possible to get this through in some form
inside 2.1 window and should I respin it? I think we are largely in
agreement on this series.


I'm waiting for Andreas to review Stefan's object_property_add_alias 
series, because we agreed that it should go in first.


Paolo



Re: [Qemu-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support

2014-06-25 Thread Chen, Tiejun

On 2014/6/25 17:31, Paolo Bonzini wrote:

Il 25/06/2014 11:21, Chen, Tiejun ha scritto:

Adding a vendor-specific capability or BAR
in an existing device is painful - hard to find
free space for it.


Yes, this is a potential risk as well since we can't guarantee current
free space is always valid for ever.


For past devices, we know which BARs they use.  For future devices, it
would be nice if the PCH/MCH backdoor was specified so that we know they
will leave a free BAR for virtualization.



Now I'm a bit confused about BAR here.

You're saying we will reserve a free BAR to address those information to 
expose to guest, but which device does this free BAR belong to? The 
video device? Or PCH/MCH?


Thanks
Tiejun



Re: [Qemu-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support

2014-06-25 Thread Paolo Bonzini

Il 25/06/2014 11:50, Chen, Tiejun ha scritto:


For past devices, we know which BARs they use.  For future devices, it
would be nice if the PCH/MCH backdoor was specified so that we know they
will leave a free BAR for virtualization.



Now I'm a bit confused about BAR here.

You're saying we will reserve a free BAR to address those information to
expose to guest, but which device does this free BAR belong to? The
video device? Or PCH/MCH?


The video device.  If the host device does not have the BAR (which will 
be the common case), QEMU can emulate it like this:


- offsets 0x..0x0fff map to configuration space of the host MCH

- offsets 0x1000..0x1fff map to configuration space of the host PCH

Of course this is only limited to offsets that are needed by the driver.

Paolo



Re: [Qemu-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support

2014-06-25 Thread Michael S. Tsirkin
On Wed, Jun 25, 2014 at 05:50:16PM +0800, Chen, Tiejun wrote:
 On 2014/6/25 17:31, Paolo Bonzini wrote:
 Il 25/06/2014 11:21, Chen, Tiejun ha scritto:
 Adding a vendor-specific capability or BAR
 in an existing device is painful - hard to find
 free space for it.
 
 Yes, this is a potential risk as well since we can't guarantee current
 free space is always valid for ever.
 
 For past devices, we know which BARs they use.  For future devices, it
 would be nice if the PCH/MCH backdoor was specified so that we know they
 will leave a free BAR for virtualization.
 
 
 Now I'm a bit confused about BAR here.
 
 You're saying we will reserve a free BAR to address those information to
 expose to guest, but which device does this free BAR belong to? The video
 device? Or PCH/MCH?
 
 Thanks
 Tiejun

If you just want to pass a couple of IDs, then don't, it's a waste.
But I still don't know what problem you are trying to solve,
looking at guest driver did not help.

-- 
MST



Re: [Qemu-devel] [Bug 1332297] Re: qemu-img: crash on check of an image with large value in the 'size' header field

2014-06-25 Thread M.Kustova
On Wed, Jun 25, 2014 at 1:42 PM, Kevin Wolf kw...@redhat.com wrote:
 Am 25.06.2014 um 11:32 hat M.Kustova geschrieben:
 On Tue, Jun 24, 2014 at 7:36 PM, Kevin Wolf kw...@redhat.com wrote:
  Am 24.06.2014 um 15:19 hat M.Kustova geschrieben:
  On Mon, Jun 23, 2014 at 12:02 PM, Stefan Hajnoczi stefa...@gmail.com 
  wrote:
   On Thu, Jun 19, 2014 at 07:19:55PM -, Maria Kustova wrote:
   The bug description missed qemu-img error:
  
   (process:12283): GLib-ERROR **: gmem.c:110: failed to allocate
   18446744059294601304 bytes
  
   Thanks, there has been recent work by Kevin Wolf to handle memory
   allocation failures gracefully without terminating QEMU.  This sounds
   like a candidate for g_try_malloc() and friends.
  
   Does the following patch series solve the problem?
   https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg01275.html
 
  These patches are conflicting with current master. So I can't test
  them as they are.
 
  Do you have a developer repository or branch containing these patches,
  so I could test it on the pre-release base?
 
  I'm just about to send a new version, I'll keep you CCed there.

 [PATCH v4 21/21] qcow2: Return useful error code in refcount_init()
 is still broken for the current master.

 In which way? I can cleanly apply the whole patch series on master (even
 tried applying the emails from my inbox to be sure).

Beginning from line #49 in master:

if (s-refcount_table_size  0) {
BLKDBG_EVENT(bs-file, BLKDBG_REFTABLE_LOAD);
ret = bdrv_pread(bs-file, s-refcount_table_offset,

The patch:

   if (s-refcount_table_size  0) {^M
 if (s-refcount_table == NULL) {^M
+ret = -ENOMEM;^M
 goto fail;^M
 }^M
 BLKDBG_EVENT(bs-file, BLKDBG_REFTABLE_LOAD);^M
 ret = bdrv_pread(bs-file, s-refcount_table_offset,^M

At least master version doesn't have this condition.

 Kevin
Maria



Re: [Qemu-devel] [PATCH] check if we have space left for hotplugged memory

2014-06-25 Thread Igor Mammedov
On Wed, 25 Jun 2014 17:33:31 +0800
Hu Tao hu...@cn.fujitsu.com wrote:

 On Wed, Jun 25, 2014 at 12:17:57PM +0300, Michael S. Tsirkin wrote:
  On Wed, Jun 25, 2014 at 05:01:04PM +0800, Hu Tao wrote:
   If pc-dimm is specified on qemu command line, but only with
   -m size (aka not -m size,maxmem,slots) then qemu will core dump.
   
   This patch fixes the problem.
   
   Signed-off-by: Hu Tao hu...@cn.fujitsu.com
   ---
hw/i386/pc.c | 5 +
hw/mem/pc-dimm.c | 5 +
2 files changed, 10 insertions(+)
   
   diff --git a/hw/i386/pc.c b/hw/i386/pc.c
   index 2cf22b1..1a0d96e 100644
   --- a/hw/i386/pc.c
   +++ b/hw/i386/pc.c
   @@ -1566,6 +1566,11 @@ static void pc_dimm_plug(HotplugHandler 
   *hotplug_dev,
goto out;
}

   +if (memory_region_size(pcms-hotplug_memory) == 0) {
   +error_setg(local_err, no space left for hotplugged memory.);
   +goto out;
   +}
   +
addr = pc_dimm_get_free_addr(pcms-hotplug_memory_base,
 
   memory_region_size(pcms-hotplug_memory),
 !addr ? NULL : addr,
  
  
  Why two errors in two places?
 
 This is what Igor suggested(IIUC) which I think is reasonable. The check
 here prevents further operation even if there is no call to
 pc_dimm_get_free_addr.
I'd prefer error out from pc_dimm_get_free_addr()

 
  
   diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
   index ad176b7..59ed9c0 100644
   --- a/hw/mem/pc-dimm.c
   +++ b/hw/mem/pc-dimm.c
   @@ -146,6 +146,11 @@ uint64_t pc_dimm_get_free_addr(uint64_t 
   address_space_start,
uint64_t new_addr, ret = 0;
uint64_t address_space_end = address_space_start + 
   address_space_size;

   +if (address_space_size == 0) {
   +error_setg(errp, no space left for hotplugged memory.);
   +goto out;
   +}
   +
assert(address_space_end  address_space_size);
object_child_foreach(qdev_get_machine(), pc_dimm_built_list, list);

  
  
  Didn't we want a more specific message?
 
 What about:
 
   no space left for hotplugged memory, please check maxmem option
 
 ?
 
 Igor pointed out that explicitly state -m maxmem=maxmem here may not
 sync with future -m change.
 
 
 Hu




[Qemu-devel] [PATCH] MAINTAINERS: add myself as maintainer of numa.c

2014-06-25 Thread Hu Tao
After commit 96d0e26c238e8 this file becomes unmaintained, add myself as
the maintainer.
---
 MAINTAINERS | 5 +
 1 file changed, 5 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9b93edd..47dfc37 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -858,6 +858,11 @@ S: Supported
 F: qemu-seccomp.c
 F: include/sysemu/seccomp.h
 
+NUMA
+M: Hu Tao hu...@cn.fujitsu.com
+S: Maintained
+F: numa.c
+
 Usermode Emulation
 --
 BSD user
-- 
1.9.3




[Qemu-devel] [PATCH RESEND] MAINTAINERS: add myself as maintainer of numa.c

2014-06-25 Thread Hu Tao
After commit 96d0e26c238e8 this file becomes unmaintained, add myself as
the maintainer.

Signed-off-by: Hu Tao hu...@cn.fujitsu.com
---
 MAINTAINERS | 5 +
 1 file changed, 5 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9b93edd..47dfc37 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -858,6 +858,11 @@ S: Supported
 F: qemu-seccomp.c
 F: include/sysemu/seccomp.h
 
+NUMA
+M: Hu Tao hu...@cn.fujitsu.com
+S: Maintained
+F: numa.c
+
 Usermode Emulation
 --
 BSD user
-- 
1.9.3




[Qemu-devel] [PATCH] pc-dimm: error out if memory hotplug is not enabled

2014-06-25 Thread Igor Mammedov
fixes QEMU abort in case it's started without memory
hotplug enabled.

as result of fix it will print following messages:

-device pc-dimm,id=d1,memdev=m1: memory hotplug is not enabled, enable it on 
startup
-device pc-dimm,id=d1,memdev=m1: Device 'pc-dimm' could not be initialized


Also fixup assert condition to detect hotplug address
space overflow.

Signed-off-by: Igor Mammedov imamm...@redhat.com
Reported-by:  Hu Tao hu...@cn.fujitsu.com
---
 hw/mem/pc-dimm.c |7 ++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index ad176b7..991f0f8 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -146,7 +146,12 @@ uint64_t pc_dimm_get_free_addr(uint64_t 
address_space_start,
 uint64_t new_addr, ret = 0;
 uint64_t address_space_end = address_space_start + address_space_size;
 
-assert(address_space_end  address_space_size);
+if (!address_space_size) {
+error_setg(errp, memory hotplug is not enabled, enable it on 
startup);
+goto out;
+}
+
+assert(address_space_end  address_space_start);
 object_child_foreach(qdev_get_machine(), pc_dimm_built_list, list);
 
 if (hint) {
-- 
1.7.1




Re: [Qemu-devel] [v5][PATCH 2/5] xen, gfx passthrough: create pseudo intel isa bridge

2014-06-25 Thread Chen, Tiejun

On 2014/6/25 17:44, Michael S. Tsirkin wrote:

On Wed, Jun 25, 2014 at 05:28:48PM +0800, Chen, Tiejun wrote:

On 2014/6/25 17:21, Michael S. Tsirkin wrote:

On Wed, Jun 25, 2014 at 05:14:30PM +0800, Chen, Tiejun wrote:

On 2014/6/25 17:04, Michael S. Tsirkin wrote:

On Wed, Jun 25, 2014 at 04:48:02PM +0800, Chen, Tiejun wrote:

On 2014/6/25 16:43, Michael S. Tsirkin wrote:

On Wed, Jun 25, 2014 at 04:39:07PM +0800, Chen, Tiejun wrote:

In fact it's exactly what passthrough does.
I wonder if more bits from ./hw/i386/kvm/pci-assign.c
can be reused. How do you poke at the host device? sysfs?


Yes, sysfs.

Thanks
Tiejun


Then you should be able to re-use large chunks of
./hw/i386/kvm/pci-assign.c: basically everything
that deals with emulation.


Do you mean those hooks to get info from the real device? Xen have its own
wrapper, xen_host_pci_get_block(), so we always go there in xen scenario.

Thanks
Tiejun


Yes and that's not good.  We have two pieces of code doing mostly
identical things slightly differently.
hw/i386/kvm/pci-assign.c is a bit younger so it's cleaner,
but these really need to be unified.



Sorry, take a look at this again,

xen_host_pci_get_block(XenHostPCIDevice *d, int pos, uint8_t *buf, int len)
|
+ xen_host_pci_config_read(d, pos, buf, len)
|
+ pread(d-config_fd, buf, len, pos)

I thinks this should be same as kvm.

Thanks
Tiejun


get_block is trivial.

I really mean the whole PT infrastructure for
- discovering host devices through sysfs
- virtualizing devices

rom, bars, msi ...
the list goes on.

logic is mostly the same.



Looks you mean we can unify the entire PT infrastructure between kvm and xen
inside qemu. But I'm afraid its not easy to do in a short time, so maybe we
can queue this as next phase.

Thanks
Tiejun


I'm afraid once we merge your code, you'll lose interest :)



Currently we have to push this feature into upstream as our first 
priority, so unless something is really needed to address. Of course I 
hope this point what we're talking is not such a thing :)


But I can promise here I'd like to do this optimization with your guide 
next :)



At least, don't add duplicate code for ROM.



Let me try this.

Thanks
Tiejun



Re: [Qemu-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support

2014-06-25 Thread Paolo Bonzini

Il 25/06/2014 11:55, Michael S. Tsirkin ha scritto:

 You're saying we will reserve a free BAR to address those information to
 expose to guest, but which device does this free BAR belong to? The video
 device? Or PCH/MCH?

If you just want to pass a couple of IDs, then don't, it's a waste.
But I still don't know what problem you are trying to solve,
looking at guest driver did not help.


It's not just a couple of IDs, it's also random fields of the MCH 
configuration space.  Grep drivers/gpu/drm/i915 for bridge_dev.


Paolo



  1   2   3   4   5   >