Re: [Qemu-devel] [PATCH 2/2 v2] pcie_aer: clear cmask for Advanced Error Interrupt Message Number
On Tue, Sep 04, 2012 at 04:22:46PM -0400, Jason Baron wrote:
On Fri, Aug 31, 2012 at 11:43:31AM -0400, Jason Baron wrote:
On Fri, Aug 31, 2012 at 06:35:13PM +0300, Michael S. Tsirkin wrote:
On Fri, Aug 31, 2012 at 10:45:52AM -0400, Jason Baron wrote:
On Fri, Aug 31, 2012 at 11:42:27AM +0300, Michael S. Tsirkin wrote:
Some minor nits below. If you dont get to it I will tweak this patch
when I apply it early next week.
On Thu, Aug 30, 2012 at 01:51:15PM -0400, Jason Baron wrote:
The Advanced Error Interrupt Message Number (bits 31:27 of the Root
Error Status Register) is updated when the number of msi messages
assigned to a
device changes. Migration of windows 7 on q35 chipset failed
because the check
in get_pci_config_device() fails due to wmask being set on these
bits.
I think you actually mean 'not being set on these bits'?
No, the check is:
static int get_pci_config_device(QEMUFile *f, void *pv, size_t size)
{
PCIDevice *s = container_of(pv, PCIDevice, config);
uint8_t *config;
int i;
assert(size == pci_config_size(s));
config = g_malloc(size);
qemu_get_buffer(f, config, size);
for (i = 0; i size; ++i) {
if ((config[i] ^ s-config[i])
s-cmask[i] ~s-wmask[i] ~s-w1cmask[i]) {
g_free(config);
return -EINVAL;
}
}
.
So because cmask is set and these bits differ in config space (due to
them being updated before migration), the migration aborts.
Ah so you mean 'cmask being set' - not wmask as you wrote.
Sorry - my mistake, yes I meant 'cmask'. I will fix the changelog in the
re-post.
Ok, here's a v2:
---
Just a small request: in the future please do not put --- before patch
please: git am ignores everything after ---. I fixed this up manually.
pcie_aer: clear cmask for Advanced Error Interrupt Message Number
The Advanced Error Interrupt Message Number (bits 31:27 of the Root
Error Status Register) is updated when the number of msi messages assigned to
a
device changes. Migration of windows 7 on q35 chipset failed because the check
in get_pci_config_device() fails due to cmask being set on these bits. Its
valid
to update these bits and we must restore this state across migration.
Signed-off-by: Jason Baron jba...@redhat.com
---
v2:
- Based on Michael Tsirkin's feedback:
-updated changelog 'wmask' - 'cmask'
-Cleaned up comments
-Make cmask set more readable
hw/pcie_aer.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/hw/pcie_aer.c b/hw/pcie_aer.c
index 3b6981c..b04c164 100644
--- a/hw/pcie_aer.c
+++ b/hw/pcie_aer.c
@@ -738,6 +738,11 @@ void pcie_aer_root_init(PCIDevice *dev)
PCI_ERR_ROOT_CMD_EN_MASK);
pci_set_long(dev-w1cmask + pos + PCI_ERR_ROOT_STATUS,
PCI_ERR_ROOT_STATUS_REPORT_MASK);
+/* PCI_ERR_ROOT_IRQ is RO but devices change it using a
+ * device-specific method.
+ */
+pci_set_long(dev-cmask + pos + PCI_ERR_ROOT_STATUS,
+ ~PCI_ERR_ROOT_IRQ);
}
void pcie_aer_root_reset(PCIDevice *dev)
--
1.7.1
Re: [Qemu-devel] [RFC-v3 0/5] vhost-scsi: Add support for host virtualized target
On Tue, Aug 21, 2012 at 08:52:06PM +, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org Hi folks, This is the third RFC for vhost-scsi patches against mainline QEMU v1.1 to support the upstream tcm_vhost host virtualized target driver now available in v3.6-rc kernel code. This series is based upon last week's commit 346fe0c4c0b, and is aiming for a future QEMU v1.3 merge. The patch series is available directly from: git://git.kernel.org/pub/scm/virt/kvm/nab/qemu-kvm.git vhost-scsi-merge-v3 This -v3 series contains further review changes based upon feedback from MST, Paolo, and Blue. It also contains the changes to function against the changes in target-pending/master - headed for v3.6-rc3 code. ACK series. Paolo can you ack virtio-scsi changes please? Nicholas, do you want me to take this through my tree? Changes from v3 - v2: - Move qdev_prop_vhost_scsi + DEFINE_PROP_VHOST_SCSI defs into vhost-scsi.[c,h] (reported by MST) - Add enum vhost_scsi_vq_list for VHostSCSI-vqs[] enumeration (reported by MST) - Add missing braces around single like if statement to following QEMU style (reported by Blue Swirl) - Change vhost_scsi_target-vhost_wwpn to char *, in order to drop casts to pstrcpy in vhost_scsi_start() + vhost_scsi_stop() (reported by Blue Swirl) - Change VHOST_SCSI_GET_ABI_VERSION to 'int' type (MST) - Add vhost-scsi.h include for DEFINE_PROP_VHOST_SCSI (mst + nab) - Move vhost-scsi related struct members ahead of *cmd_vqs[0] within VirtIOSCSI definition. (paolo + nab) - Fix 4 byte alignment of vhost_scsi_target (MST) - Convert fprintf(stderr, ...) usage to - error_report() (reported by MST) - Do explict memset of backend before calling VHOST_SCSI_CLEAR_ENDPOINT in vhost_scsi_stop() (reported by MST) - Add support for vhostfd passing in vhost_scsi_add() (reported by MST) - Move net_handle_fd_param() - monitor_handle_fd_param() for generic usage by net/ + vhost-scsi (reported by MST) - Change vhost_scsi_add() to use monitor_handle_fd_param() (reported by MST) Changes from v1 - v2: - Expose ABI version via VHOST_SCSI_GET_ABI_VERSION + use Rev 0 as starting point for v3.6-rc code (Stefan + ALiguori + nab) - Fix upstream qemu conflict in hw/qdev-properties.c - Make GET_ABI_VERSION use int (nab + mst) - Drop unnecessary event-notifier changes (nab) - Fix vhost-scsi case lables in configure (reported by paolo) - Convert qdev_prop_vhost_scsi to use -get() + -set() following qdev_prop_netdev (reported by paolo) - Fix typo in qemu-options.hx definition of vhost-scsi (reported by paolo) - Squash virtio-scsi: use the vhost-scsi host device from stefan (nab) - Fix up virtio_scsi_properties[] conflict w/ upstream qemu (nab) - Drop usage of to_virtio_scsi() in virtio_scsi_set_status() (reported by paolo) - Use modern VirtIOSCSIConf define in virtio-scsi.h (reported by paolo) - Use s-conf-vhost_scsi instead of proxyconf-vhost_scsi in virtio_scsi_init() (reported by paolo) - Only register QEMU SCSI bus is vhost-scsi is not active (reported by paolo) - Fix incorrect VirtIOSCSI-cmd_vqs[0] definition (nab) Thanks again to everyone who has been reviewing this series! --nab Nicholas Bellinger (2): monitor: Rename+move net_handle_fd_param - monitor_handle_fd_param virtio-scsi: Set max_target=0 during vhost-scsi operation Stefan Hajnoczi (3): vhost: Pass device path to vhost_dev_init() vhost-scsi: add -vhost-scsi host device for use with tcm-vhost virtio-scsi: Add start/stop functionality for vhost-scsi configure| 10 +++ hw/Makefile.objs |1 + hw/qdev-properties.c | 41 +++ hw/vhost-scsi.c | 190 ++ hw/vhost-scsi.h | 62 hw/vhost.c |5 +- hw/vhost.h |3 +- hw/vhost_net.c |2 +- hw/virtio-pci.c |2 + hw/virtio-scsi.c | 55 ++- hw/virtio-scsi.h |1 + monitor.c| 18 + monitor.h|1 + net.c| 18 - net.h|2 - net/socket.c |2 +- net/tap.c|4 +- qemu-common.h|1 + qemu-config.c| 19 + qemu-options.hx |4 + vl.c | 18 + 21 files changed, 431 insertions(+), 28 deletions(-) create mode 100644 hw/vhost-scsi.c create mode 100644 hw/vhost-scsi.h -- 1.7.2.5
Re: [Qemu-devel] [RFC-v3 3/5] vhost-scsi: add -vhost-scsi host device for use with tcm-vhost
On Tue, Aug 21, 2012 at 08:52:09PM +, Nicholas A. Bellinger wrote: From: Stefan Hajnoczi stefa...@linux.vnet.ibm.com This patch adds a new type of host device that drives the vhost_scsi device. The syntax to add vhost-scsi is: qemu -vhost-scsi id=vhost-scsi0,wwpn=...,tpgt=123 The virtio-scsi emulated device will make use of vhost-scsi to process virtio-scsi requests inside the kernel and hand them to the in-kernel SCSI target stack using the tcm_vhost fabric driver. The tcm_vhost driver was merged into the upstream linux kernel for 3.6-rc2, and the commit can be found here: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=057cbf49a1f08297 In the future, please put changelog after --- or in the cover letter: we do not need it in git history. I fixed this up. Changelog v2 - v3: - Move qdev_prop_vhost_scsi + DEFINE_PROP_VHOST_SCSI defs into vhost-scsi.[c,h] (reported by MST) - Add enum vhost_scsi_vq_list for VHostSCSI-vqs[] enumeration (reported by MST) - Add missing braces around single like if statement to following QEMU style (reported by Blue Swirl) - Change vhost_scsi_target-vhost_wwpn to char *, in order to drop casts to pstrcpy in vhost_scsi_start() + vhost_scsi_stop() (reported by Blue Swirl) - Change VHOST_SCSI_GET_ABI_VERSION to 'int' type (MST) - Fix 4 byte alignment of vhost_scsi_target (MST) - Convert fprintf(stderr, ...) usage to - error_report() (reported by MST) - Do explict memset of backend before calling VHOST_SCSI_CLEAR_ENDPOINT in vhost_scsi_stop() (reported by MST) - Add support for vhostfd passing in vhost_scsi_add() (reported by MST) - Change vhost_scsi_add() to use monitor_handle_fd_param() (reported by MST) Changelog v1 - v2: - Expose ABI version via VHOST_SCSI_GET_ABI_VERSION + use Rev 0 as starting point for v3.6-rc code (Stefan + ALiguori + nab) - Fix upstream qemu conflict in hw/qdev-properties.c - Make GET_ABI_VERSION use int (nab + mst) - Fix vhost-scsi case lables in configure (reported by paolo) - Convert qdev_prop_vhost_scsi to use -get() + -set() following qdev_prop_netdev (reported by paolo) - Fix typo in qemu-options.hx definition of vhost-scsi (reported by paolo) Changelog v0 - v1: - Add VHOST_SCSI_SET_ENDPOINT call (stefan) - Enable vhost notifiers for multiple queues (Zhi) - clear vhost-scsi endpoint on stopped (Zhi) - Add CONFIG_VHOST_SCSI for QEMU build configure (nab) - Rename vhost_vring_target - vhost_scsi_target (mst + nab) - Add support for VHOST_SCSI_GET_ABI_VERSION ioctl (aliguori + nab) Cc: Stefan Hajnoczi stefa...@linux.vnet.ibm.com Cc: Zhi Yong Wu wu...@linux.vnet.ibm.com Cc: Anthony Liguori aligu...@us.ibm.com Cc: Paolo Bonzini pbonz...@redhat.com Cc: Michael S. Tsirkin m...@redhat.com Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org --- configure| 10 +++ hw/Makefile.objs |1 + hw/qdev-properties.c | 41 +++ hw/vhost-scsi.c | 190 ++ hw/vhost-scsi.h | 62 qemu-common.h|1 + qemu-config.c| 19 + qemu-options.hx |4 + vl.c | 18 + 9 files changed, 346 insertions(+), 0 deletions(-) create mode 100644 hw/vhost-scsi.c create mode 100644 hw/vhost-scsi.h diff --git a/configure b/configure index f0dbc03..1f03202 100755 --- a/configure +++ b/configure @@ -168,6 +168,7 @@ libattr= xfs= vhost_net=no +vhost_scsi=no kvm=no gprof=no debug_tcg=no @@ -513,6 +514,7 @@ Haiku) usb=linux kvm=yes vhost_net=yes + vhost_scsi=yes if [ $cpu = i386 -o $cpu = x86_64 ] ; then audio_possible_drivers=$audio_possible_drivers fmod fi @@ -818,6 +820,10 @@ for opt do ;; --enable-vhost-net) vhost_net=yes ;; + --disable-vhost-scsi) vhost_scsi=no + ;; + --enable-vhost-scsi) vhost_scsi=yes + ;; --disable-opengl) opengl=no ;; --enable-opengl) opengl=yes @@ -3116,6 +3122,7 @@ echo posix_madvise $posix_madvise echo uuid support $uuid echo libcap-ng support $cap_ng echo vhost-net support $vhost_net +echo vhost-scsi support $vhost_scsi echo Trace backend $trace_backend echo Trace output file $trace_file-pid echo spice support $spice @@ -3828,6 +3835,9 @@ case $target_arch2 in if test $vhost_net = yes ; then echo CONFIG_VHOST_NET=y $config_target_mak fi + if test $vhost_scsi = yes ; then +echo CONFIG_VHOST_SCSI=y $config_target_mak + fi fi esac case $target_arch2 in diff --git a/hw/Makefile.objs b/hw/Makefile.objs index 3ba5dd0..6ab75ec 100644 --- a/hw/Makefile.objs +++ b/hw/Makefile.objs @@ -169,6 +169,7 @@ obj-$(CONFIG_VIRTIO) += virtio.o virtio-blk.o virtio-balloon.o virtio-net.o obj-$(CONFIG_VIRTIO) += virtio-serial-bus.o virtio-scsi.o obj-$(CONFIG_SOFTMMU) += vhost_net.o obj-$(CONFIG_VHOST_NET) +=
[Qemu-devel] [PULL] pci minor tweaks
The following changes since commit 6e4c0d1f03d6ab407509c32fab7cb4b8230f57ff: hw/pl110: Fix spelling of 'palette' (2012-09-06 17:04:33 +0200) are available in the git repository at: git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_anthony for you to fetch changes up to 0e180d9c8a7429c55d23d2e7855f1e490a063aaa: pcie_aer: clear cmask for Advanced Error Interrupt Message Number (2012-09-07 09:02:44 +0300) pci minor tweaks Some minor tweaks - nothing really exciting but merging now in the hope this speeds up progress. Signed-off-by: Michael S. Tsirkin m...@redhat.com Jason Baron (2): pcie: drop version_id field for live migration pcie_aer: clear cmask for Advanced Error Interrupt Message Number Michael S. Tsirkin (1): qemu: add .exrc .exrc | 7 +++ hw/pci.c | 2 +- hw/pcie.h | 1 - hw/pcie_aer.c | 5 + 4 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 .exrc
Re: [Qemu-devel] [RFC-v3 0/5] vhost-scsi: Add support for host virtualized target
On Tue, Aug 21, 2012 at 08:52:06PM +, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org Hi folks, This is the third RFC for vhost-scsi patches against mainline QEMU v1.1 I rebased on top of 1.2 and put this in my tree: git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git pci Please check it out, meanwhile we'll see if anyone complains. -- MST
Re: [Qemu-devel] [RFC-v3 0/5] vhost-scsi: Add support for host virtualized target
On Fri, Sep 07, 2012 at 09:23:22AM +0300, Michael S. Tsirkin wrote: On Tue, Aug 21, 2012 at 08:52:06PM +, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org Hi folks, This is the third RFC for vhost-scsi patches against mainline QEMU v1.1 I rebased on top of 1.2 and put this in my tree: git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git pci Please check it out, meanwhile we'll see if anyone complains. OK I will be the first :). Looks like build *without* CONFIG_VHOST_SCSI is broken now: LINK sparc64-softmmu/qemu-system-sparc64 ../hw/qdev-properties.o: In function `parse_vhost_scsi_dev': /scm/qemu/hw/qdev-properties.c:706: undefined reference to `find_vhost_scsi' ../hw/qdev-properties.o: In function `print_vhost_scsi_dev': /scm/qemu/hw/qdev-properties.c:719: undefined reference to `vhost_scsi_get_id' ../libhw64/vl.o: In function `vhost_scsi_init_func': /scm/qemu/vl.c:1943: undefined reference to `vhost_scsi_add_opts' hw/virtio-scsi.o: In function `virtio_scsi_set_status': /scm/qemu/hw/virtio-scsi.c:733: undefined reference to `vhost_scsi_stop' /scm/qemu/hw/virtio-scsi.c:724: undefined reference to `vhost_scsi_start' Please add stubs *and test* without CONFIG_VHOST_SCSI. While at it can we please rename file to vhost_scsi.c, vhost_scsi.h? qemu is inconsistent but vhost files all use _ as separator: this means function names and file names are in sync. Reverted for now, waiting for v4. Thanks! -- MST
Re: [Qemu-devel] [RFC-v3 0/5] vhost-scsi: Add support for host virtualized target
On Fri, Sep 07, 2012 at 09:32:37AM +0300, Michael S. Tsirkin wrote: On Fri, Sep 07, 2012 at 09:23:22AM +0300, Michael S. Tsirkin wrote: On Tue, Aug 21, 2012 at 08:52:06PM +, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org Hi folks, This is the third RFC for vhost-scsi patches against mainline QEMU v1.1 I rebased on top of 1.2 and put this in my tree: git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git pci Please check it out, meanwhile we'll see if anyone complains. OK I will be the first :). Looks like build *without* CONFIG_VHOST_SCSI is broken now: LINK sparc64-softmmu/qemu-system-sparc64 ../hw/qdev-properties.o: In function `parse_vhost_scsi_dev': /scm/qemu/hw/qdev-properties.c:706: undefined reference to `find_vhost_scsi' ../hw/qdev-properties.o: In function `print_vhost_scsi_dev': /scm/qemu/hw/qdev-properties.c:719: undefined reference to `vhost_scsi_get_id' ../libhw64/vl.o: In function `vhost_scsi_init_func': /scm/qemu/vl.c:1943: undefined reference to `vhost_scsi_add_opts' hw/virtio-scsi.o: In function `virtio_scsi_set_status': /scm/qemu/hw/virtio-scsi.c:733: undefined reference to `vhost_scsi_stop' /scm/qemu/hw/virtio-scsi.c:724: undefined reference to `vhost_scsi_start' Please add stubs *and test* without CONFIG_VHOST_SCSI. While at it can we please rename file to vhost_scsi.c, vhost_scsi.h? qemu is inconsistent but vhost files all use _ as separator: this means function names and file names are in sync. Reverted for now, waiting for v4. Please note vhost, net and monitor changes are all merged. That's patches 1/5 and 2/5. Thanks! -- MST
Re: [Qemu-devel] [PATCH] smbios: convert to use QemuOpts
Hello, this is your friendly remote compile service.
Anthony Liguori aligu...@us.ibm.com writes:
Signed-off-by: Anthony Liguori aligu...@us.ibm.com
---
arch_init.c | 10 +-
arch_init.h |2 +-
hw/smbios.c | 53 +++--
hw/smbios.h |2 +-
qemu-config.c | 50 ++
vl.c |4 +++-
6 files changed, 91 insertions(+), 30 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 5a1173e..e43ace9 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -1033,13 +1033,13 @@ void do_acpitable_option(const char *optarg)
#endif
}
-void do_smbios_option(const char *optarg)
+int do_smbios_option(QemuOpts *opts, void *opaque)
{
#ifdef TARGET_I386
-if (smbios_entry_add(optarg) 0) {
-fprintf(stderr, Wrong smbios provided\n);
-exit(1);
-}
+return smbios_entry_add(opts);
+#else
+fprintf(stderr, -smbios option not supported on this platform\n);
+return -ENOSYS;
#endif
}
diff --git a/arch_init.h b/arch_init.h
index d9c572a..2dce578 100644
--- a/arch_init.h
+++ b/arch_init.h
@@ -26,7 +26,7 @@ extern const uint32_t arch_type;
void select_soundhw(const char *optarg);
void do_acpitable_option(const char *optarg);
-void do_smbios_option(const char *optarg);
+int do_smbios_option(QemuOpts *opts, void *opaque);
void cpudef_init(void);
int audio_available(void);
void audio_init(ISABus *isa_bus, PCIBus *pci_bus);
diff --git a/hw/smbios.c b/hw/smbios.c
index c57237d..095bccc 100644
--- a/hw/smbios.c
+++ b/hw/smbios.c
@@ -124,21 +124,24 @@ void smbios_add_field(int type, int offset, int len,
void *data)
cpu_to_le16(le16_to_cpu(*(uint16_t *)smbios_entries) + 1);
}
-static void smbios_build_type_0_fields(const char *t)
+static void smbios_build_type_0_fields(QemuOpts *opts)
{
char buf[1024];
-if (get_param_value(buf, sizeof(buf), vendor, t))
+if ((buf = qemu_opt_get(opts, vendor))) {
error: incompatible types when assigning to type ‘char[1024]’ from type ‘const
char *’
smbios_add_field(0, offsetof(struct smbios_type_0, vendor_str),
strlen(buf) + 1, buf);
-if (get_param_value(buf, sizeof(buf), version, t))
+}
+if ((buf = qemu_opt_get(opts, version))) {
smbios_add_field(0, offsetof(struct smbios_type_0, bios_version_str),
strlen(buf) + 1, buf);
-if (get_param_value(buf, sizeof(buf), date, t))
+}
+if ((buf = qemu_opt_get(opts, date))) {
smbios_add_field(0, offsetof(struct smbios_type_0,
bios_release_date_str),
strlen(buf) + 1, buf);
-if (get_param_value(buf, sizeof(buf), release, t)) {
+}
+if ((buf = qemu_opt_get(opts, release))) {
int major, minor;
sscanf(buf, %d.%d, major, minor);
smbios_add_field(0, offsetof(struct smbios_type_0,
@@ -148,41 +151,47 @@ static void smbios_build_type_0_fields(const char *t)
}
}
-static void smbios_build_type_1_fields(const char *t)
+static void smbios_build_type_1_fields(QemuOpts *opts)
{
-char buf[1024];
+const char *buf;
-if (get_param_value(buf, sizeof(buf), manufacturer, t))
+if ((buf = qemu_opt_get(opts, manufacturer))) {
warning: passing argument 4 of ‘smbios_add_field’ discards ‘const’ qualifier
from pointer target type [enabled by default]
smbios_add_field(1, offsetof(struct smbios_type_1, manufacturer_str),
strlen(buf) + 1, buf);
-if (get_param_value(buf, sizeof(buf), product, t))
+}
+if ((buf = qemu_opt_get(opts, product))) {
smbios_add_field(1, offsetof(struct smbios_type_1, product_name_str),
strlen(buf) + 1, buf);
-if (get_param_value(buf, sizeof(buf), version, t))
+}
+if ((buf = qemu_opt_get(opts, version))) {
smbios_add_field(1, offsetof(struct smbios_type_1, version_str),
strlen(buf) + 1, buf);
-if (get_param_value(buf, sizeof(buf), serial, t))
+}
+if ((buf = qemu_opt_get(opts, serial))) {
smbios_add_field(1, offsetof(struct smbios_type_1,
serial_number_str),
strlen(buf) + 1, buf);
-if (get_param_value(buf, sizeof(buf), uuid, t)) {
+}
+if ((buf = qemu_opt_get(opts, uuid))) {
if (qemu_uuid_parse(buf, qemu_uuid) != 0) {
fprintf(stderr, Invalid SMBIOS UUID string\n);
exit(1);
}
}
-if (get_param_value(buf, sizeof(buf), sku, t))
+if ((buf = qemu_opt_get(opts, sku))) {
smbios_add_field(1, offsetof(struct smbios_type_1, sku_number_str),
strlen(buf) + 1, buf);
-if (get_param_value(buf, sizeof(buf), family, t))
+}
+if ((buf =
[Qemu-devel] [PATCH 0/5] vhost-scsi: Add support for host virtualized target
From: Nicholas Bellinger n...@linux-iscsi.org Hello Anthony Co, This is the fourth installment to add host virtualized target support for the mainline tcm_vhost fabric driver using Linux v3.6-rc into QEMU 1.3.0-rc. The series is available directly from the following git branch: git://git.kernel.org/pub/scm/virt/kvm/nab/qemu-kvm.git vhost-scsi-for-1.3 Note the code is cut against yesterday's QEMU head, and dispite the name of the tree is based upon mainline qemu.org git code + has thus far been running overnight with 100K IOPs small block 4k workloads using v3.6-rc2+ based target code with RAMDISK_DR backstores. Other than some minor fuzz between jumping from QEMU 1.2.0 - 1.2.50, this series is functionally identical to what's been posted for vhost-scsi RFC-v3 to qemu-devel. Please consider applying these patches for an initial vhost-scsi merge into QEMU 1.3.0-rc code, or let us know what else you'd like to see addressed for this series to in order to merge. Thank you! --nab Nicholas Bellinger (2): monitor: Rename+move net_handle_fd_param - monitor_handle_fd_param virtio-scsi: Set max_target=0 during vhost-scsi operation Stefan Hajnoczi (3): vhost: Pass device path to vhost_dev_init() vhost-scsi: add -vhost-scsi host device for use with tcm-vhost virtio-scsi: Add start/stop functionality for vhost-scsi configure| 10 +++ hw/Makefile.objs |1 + hw/qdev-properties.c | 41 +++ hw/vhost-scsi.c | 190 ++ hw/vhost-scsi.h | 62 hw/vhost.c |5 +- hw/vhost.h |3 +- hw/vhost_net.c |2 +- hw/virtio-pci.c |2 + hw/virtio-scsi.c | 55 ++- hw/virtio-scsi.h |1 + monitor.c| 18 + monitor.h|1 + net.c| 18 - net.h|2 - net/socket.c |2 +- net/tap.c|4 +- qemu-common.h|1 + qemu-config.c| 19 + qemu-options.hx |4 + vl.c | 18 + 21 files changed, 431 insertions(+), 28 deletions(-) create mode 100644 hw/vhost-scsi.c create mode 100644 hw/vhost-scsi.h -- 1.7.2.5
Re: [Qemu-devel] [RFC-v3 0/5] vhost-scsi: Add support for host virtualized target
On Fri, 2012-09-07 at 09:37 +0300, Michael S. Tsirkin wrote: On Fri, Sep 07, 2012 at 09:32:37AM +0300, Michael S. Tsirkin wrote: On Fri, Sep 07, 2012 at 09:23:22AM +0300, Michael S. Tsirkin wrote: On Tue, Aug 21, 2012 at 08:52:06PM +, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org Hi folks, This is the third RFC for vhost-scsi patches against mainline QEMU v1.1 I rebased on top of 1.2 and put this in my tree: git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git pci Please check it out, meanwhile we'll see if anyone complains. OK I will be the first :). Looks like build *without* CONFIG_VHOST_SCSI is broken now: LINK sparc64-softmmu/qemu-system-sparc64 ../hw/qdev-properties.o: In function `parse_vhost_scsi_dev': /scm/qemu/hw/qdev-properties.c:706: undefined reference to `find_vhost_scsi' ../hw/qdev-properties.o: In function `print_vhost_scsi_dev': /scm/qemu/hw/qdev-properties.c:719: undefined reference to `vhost_scsi_get_id' ../libhw64/vl.o: In function `vhost_scsi_init_func': /scm/qemu/vl.c:1943: undefined reference to `vhost_scsi_add_opts' hw/virtio-scsi.o: In function `virtio_scsi_set_status': /scm/qemu/hw/virtio-scsi.c:733: undefined reference to `vhost_scsi_stop' /scm/qemu/hw/virtio-scsi.c:724: undefined reference to `vhost_scsi_start' Please add stubs *and test* without CONFIG_VHOST_SCSI. While at it can we please rename file to vhost_scsi.c, vhost_scsi.h? qemu is inconsistent but vhost files all use _ as separator: this means function names and file names are in sync. Reverted for now, waiting for v4. Please note vhost, net and monitor changes are all merged. That's patches 1/5 and 2/5. Hi MST! Looks like we just cross emails here within 10 mins here.. I'm happy to respin a PATCH and/or GIT PULL request minus 1-2 depending on the vhost-scsi workflow that ALiguori is comfortable with here. Thank you! --nab
[Qemu-devel] [PATCH 4/5] virtio-scsi: Add start/stop functionality for vhost-scsi
From: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
This patch starts and stops vhost as the virtio device transitions
through its status phases. Vhost can only be started once the guest
reports its driver has successfully initialized, which means the
virtqueues have been set up by the guest.
v3: - Add vhost-scsi.h include for DEFINE_PROP_VHOST_SCSI (mst + nab)
- Move vhost-scsi related struct members ahead of *cmd_vqs[0] within
VirtIOSCSI definition. (paolo + nab)
v2: - Squash virtio-scsi: use the vhost-scsi host device from stefan (nab)
- Fix up virtio_scsi_properties[] conflict w/ upstream qemu (nab)
- Drop usage of to_virtio_scsi() in virtio_scsi_set_status()
(reported by paolo)
- Use modern VirtIOSCSIConf define in virtio-scsi.h (reported by paolo)
- Use s-conf-vhost_scsi instead of proxyconf-vhost_scsi in
virtio_scsi_init() (reported by paolo)
- Only register QEMU SCSI bus is vhost-scsi is not active (reported
by paolo)
Cc: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
Cc: Zhi Yong Wu wu...@linux.vnet.ibm.com
Cc: Michael S. Tsirkin m...@redhat.com
Cc: Paolo Bonzini pbonz...@redhat.com
Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
---
hw/virtio-pci.c |2 ++
hw/virtio-scsi.c | 49 +
hw/virtio-scsi.h |1 +
3 files changed, 52 insertions(+), 0 deletions(-)
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 125eded..8ec7cf1 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -22,6 +22,7 @@
#include virtio-net.h
#include virtio-serial.h
#include virtio-scsi.h
+#include vhost-scsi.h
#include pci.h
#include qemu-error.h
#include msi.h
@@ -1036,6 +1037,7 @@ static void virtio_scsi_exit_pci(PCIDevice *pci_dev)
}
static Property virtio_scsi_properties[] = {
+DEFINE_PROP_VHOST_SCSI(vhost-scsi, VirtIOPCIProxy, scsi.vhost_scsi),
DEFINE_PROP_BIT(ioeventfd, VirtIOPCIProxy, flags,
VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
DEFINE_PROP_UINT32(vectors, VirtIOPCIProxy, nvectors,
DEV_NVECTORS_UNSPECIFIED),
DEFINE_VIRTIO_SCSI_PROPERTIES(VirtIOPCIProxy, host_features, scsi),
diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c
index 5f737ac..edda097 100644
--- a/hw/virtio-scsi.c
+++ b/hw/virtio-scsi.c
@@ -13,9 +13,13 @@
*
*/
+#include qemu-common.h
+#include qemu-error.h
+#include vhost-scsi.h
#include virtio-scsi.h
#include hw/scsi.h
#include hw/scsi-defs.h
+#include vhost.h
#define VIRTIO_SCSI_VQ_SIZE 128
#define VIRTIO_SCSI_CDB_SIZE32
@@ -144,6 +148,10 @@ typedef struct {
uint32_t cdb_size;
int resetting;
bool events_dropped;
+
+bool vhost_started;
+VHostSCSI *vhost_scsi;
+
VirtQueue *ctrl_vq;
VirtQueue *event_vq;
VirtQueue *cmd_vqs[0];
@@ -699,6 +707,38 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = {
.load_request = virtio_scsi_load_request,
};
+static bool virtio_scsi_started(VirtIOSCSI *s, uint8_t val)
+{
+return (val VIRTIO_CONFIG_S_DRIVER_OK) s-vdev.vm_running;
+}
+
+static void virtio_scsi_set_status(VirtIODevice *vdev, uint8_t val)
+{
+VirtIOSCSI *s = (VirtIOSCSI *)vdev;
+bool start = virtio_scsi_started(s, val);
+
+if (s-vhost_started == start) {
+return;
+}
+
+if (start) {
+int ret;
+
+ret = vhost_scsi_start(s-vhost_scsi, vdev);
+if (ret 0) {
+error_report(virtio-scsi: unable to start vhost: %s\n,
+ strerror(-ret));
+
+/* There is no userspace virtio-scsi fallback so exit */
+exit(1);
+}
+} else {
+vhost_scsi_stop(s-vhost_scsi, vdev);
+}
+
+s-vhost_started = start;
+}
+
VirtIODevice *virtio_scsi_init(DeviceState *dev, VirtIOSCSIConf *proxyconf)
{
VirtIOSCSI *s;
@@ -712,12 +752,17 @@ VirtIODevice *virtio_scsi_init(DeviceState *dev,
VirtIOSCSIConf *proxyconf)
s-qdev = dev;
s-conf = proxyconf;
+s-vhost_started = false;
+s-vhost_scsi = s-conf-vhost_scsi;
/* TODO set up vdev function pointers */
s-vdev.get_config = virtio_scsi_get_config;
s-vdev.set_config = virtio_scsi_set_config;
s-vdev.get_features = virtio_scsi_get_features;
s-vdev.reset = virtio_scsi_reset;
+if (s-vhost_scsi) {
+s-vdev.set_status = virtio_scsi_set_status;
+}
s-ctrl_vq = virtio_add_queue(s-vdev, VIRTIO_SCSI_VQ_SIZE,
virtio_scsi_handle_ctrl);
@@ -743,5 +788,9 @@ void virtio_scsi_exit(VirtIODevice *vdev)
{
VirtIOSCSI *s = (VirtIOSCSI *)vdev;
unregister_savevm(s-qdev, virtio-scsi, s);
+
+/* This will stop vhost backend if appropriate. */
+virtio_scsi_set_status(vdev, 0);
+
virtio_cleanup(vdev);
}
diff --git a/hw/virtio-scsi.h b/hw/virtio-scsi.h
index 4bc889d..74e9422 100644
--- a/hw/virtio-scsi.h
+++ b/hw/virtio-scsi.h
@@ -22,6 +22,7 @@
#define VIRTIO_SCSI_F_CHANGE 2
struct VirtIOSCSIConf {
+VHostSCSI *vhost_scsi;
[Qemu-devel] [PATCH 1/5] monitor: Rename+move net_handle_fd_param - monitor_handle_fd_param
From: Nicholas Bellinger n...@linux-iscsi.org
This patch renames+moves the net_handle_fd_param() caller used to
obtain a file descriptor from either qemu_parse_fd() (the normal case)
or from monitor_get_fd() (migration case) into a generically prefixed
monitor_handle_fd_param() to be used by vhost-scsi code.
Also update net/[socket,tap].c consumers to use the new prefix.
Reported-by: Michael S. Tsirkin m...@redhat.com
Cc: Michael S. Tsirkin m...@redhat.com
Cc: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
Cc: Paolo Bonzini pbonz...@redhat.com
Cc: Anthony Liguori aligu...@us.ibm.com
Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
---
monitor.c| 18 ++
monitor.h|1 +
net.c| 18 --
net.h|2 --
net/socket.c |2 +-
net/tap.c|4 ++--
6 files changed, 22 insertions(+), 23 deletions(-)
diff --git a/monitor.c b/monitor.c
index 49dccfe..0641efe 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2389,6 +2389,24 @@ int monitor_get_fd(Monitor *mon, const char *fdname)
return -1;
}
+int monitor_handle_fd_param(Monitor *mon, const char *fdname)
+{
+int fd;
+
+if (!qemu_isdigit(fdname[0]) mon) {
+
+fd = monitor_get_fd(mon, fdname);
+if (fd == -1) {
+error_report(No file descriptor named %s found, fdname);
+return -1;
+}
+} else {
+fd = qemu_parse_fd(fdname);
+}
+
+return fd;
+}
+
/* mon_cmds and info_cmds would be sorted at runtime */
static mon_cmd_t mon_cmds[] = {
#include hmp-commands.h
diff --git a/monitor.h b/monitor.h
index 5f4de1b..d557e97 100644
--- a/monitor.h
+++ b/monitor.h
@@ -65,6 +65,7 @@ int monitor_read_block_device_key(Monitor *mon, const char
*device,
void *opaque);
int monitor_get_fd(Monitor *mon, const char *fdname);
+int monitor_handle_fd_param(Monitor *mon, const char *fdname);
void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
GCC_FMT_ATTR(2, 0);
diff --git a/net.c b/net.c
index 60043dd..e5d25d4 100644
--- a/net.c
+++ b/net.c
@@ -522,24 +522,6 @@ int qemu_find_nic_model(NICInfo *nd, const char * const
*models,
return -1;
}
-int net_handle_fd_param(Monitor *mon, const char *param)
-{
-int fd;
-
-if (!qemu_isdigit(param[0]) mon) {
-
-fd = monitor_get_fd(mon, param);
-if (fd == -1) {
-error_report(No file descriptor named %s found, param);
-return -1;
-}
-} else {
-fd = qemu_parse_fd(param);
-}
-
-return fd;
-}
-
static int net_init_nic(const NetClientOptions *opts, const char *name,
NetClientState *peer)
{
diff --git a/net.h b/net.h
index 2975056..04fda1d 100644
--- a/net.h
+++ b/net.h
@@ -168,8 +168,6 @@ int qmp_netdev_add(Monitor *mon, const QDict *qdict,
QObject **ret);
void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd);
-int net_handle_fd_param(Monitor *mon, const char *param);
-
#define POLYNOMIAL 0x04c11db6
unsigned compute_mcast_idx(const uint8_t *ep);
diff --git a/net/socket.c b/net/socket.c
index c172c24..7c602e4 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -629,7 +629,7 @@ int net_init_socket(const NetClientOptions *opts, const
char *name,
if (sock-has_fd) {
int fd;
-fd = net_handle_fd_param(cur_mon, sock-fd);
+fd = monitor_handle_fd_param(cur_mon, sock-fd);
if (fd == -1 || !net_socket_fd_init(peer, socket, name, fd, 1)) {
return -1;
}
diff --git a/net/tap.c b/net/tap.c
index 1971525..a88ae8f 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -610,7 +610,7 @@ int net_init_tap(const NetClientOptions *opts, const char
*name,
return -1;
}
-fd = net_handle_fd_param(cur_mon, tap-fd);
+fd = monitor_handle_fd_param(cur_mon, tap-fd);
if (fd == -1) {
return -1;
}
@@ -686,7 +686,7 @@ int net_init_tap(const NetClientOptions *opts, const char
*name,
int vhostfd;
if (tap-has_vhostfd) {
-vhostfd = net_handle_fd_param(cur_mon, tap-vhostfd);
+vhostfd = monitor_handle_fd_param(cur_mon, tap-vhostfd);
if (vhostfd == -1) {
return -1;
}
--
1.7.2.5
[Qemu-devel] [PATCH 3/5] vhost-scsi: add -vhost-scsi host device for use with tcm-vhost
From: Stefan Hajnoczi stefa...@linux.vnet.ibm.com This patch adds a new type of host device that drives the vhost_scsi device. The syntax to add vhost-scsi is: qemu -vhost-scsi id=vhost-scsi0,wwpn=...,tpgt=123 The virtio-scsi emulated device will make use of vhost-scsi to process virtio-scsi requests inside the kernel and hand them to the in-kernel SCSI target stack using the tcm_vhost fabric driver. The tcm_vhost driver was merged into the upstream linux kernel for 3.6-rc2, and the commit can be found here: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=057cbf49a1f08297 Changelog v2 - v3: - Move qdev_prop_vhost_scsi + DEFINE_PROP_VHOST_SCSI defs into vhost-scsi.[c,h] (reported by MST) - Add enum vhost_scsi_vq_list for VHostSCSI-vqs[] enumeration (reported by MST) - Add missing braces around single like if statement to following QEMU style (reported by Blue Swirl) - Change vhost_scsi_target-vhost_wwpn to char *, in order to drop casts to pstrcpy in vhost_scsi_start() + vhost_scsi_stop() (reported by Blue Swirl) - Change VHOST_SCSI_GET_ABI_VERSION to 'int' type (MST) - Fix 4 byte alignment of vhost_scsi_target (MST) - Convert fprintf(stderr, ...) usage to - error_report() (reported by MST) - Do explict memset of backend before calling VHOST_SCSI_CLEAR_ENDPOINT in vhost_scsi_stop() (reported by MST) - Add support for vhostfd passing in vhost_scsi_add() (reported by MST) - Change vhost_scsi_add() to use monitor_handle_fd_param() (reported by MST) Changelog v1 - v2: - Expose ABI version via VHOST_SCSI_GET_ABI_VERSION + use Rev 0 as starting point for v3.6-rc code (Stefan + ALiguori + nab) - Fix upstream qemu conflict in hw/qdev-properties.c - Make GET_ABI_VERSION use int (nab + mst) - Fix vhost-scsi case lables in configure (reported by paolo) - Convert qdev_prop_vhost_scsi to use -get() + -set() following qdev_prop_netdev (reported by paolo) - Fix typo in qemu-options.hx definition of vhost-scsi (reported by paolo) Changelog v0 - v1: - Add VHOST_SCSI_SET_ENDPOINT call (stefan) - Enable vhost notifiers for multiple queues (Zhi) - clear vhost-scsi endpoint on stopped (Zhi) - Add CONFIG_VHOST_SCSI for QEMU build configure (nab) - Rename vhost_vring_target - vhost_scsi_target (mst + nab) - Add support for VHOST_SCSI_GET_ABI_VERSION ioctl (aliguori + nab) Cc: Stefan Hajnoczi stefa...@linux.vnet.ibm.com Cc: Zhi Yong Wu wu...@linux.vnet.ibm.com Cc: Anthony Liguori aligu...@us.ibm.com Cc: Paolo Bonzini pbonz...@redhat.com Cc: Michael S. Tsirkin m...@redhat.com Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org --- configure| 10 +++ hw/Makefile.objs |1 + hw/qdev-properties.c | 41 +++ hw/vhost-scsi.c | 190 ++ hw/vhost-scsi.h | 62 qemu-common.h|1 + qemu-config.c| 19 + qemu-options.hx |4 + vl.c | 18 + 9 files changed, 346 insertions(+), 0 deletions(-) create mode 100644 hw/vhost-scsi.c create mode 100644 hw/vhost-scsi.h diff --git a/configure b/configure index f0dbc03..1f03202 100755 --- a/configure +++ b/configure @@ -168,6 +168,7 @@ libattr= xfs= vhost_net=no +vhost_scsi=no kvm=no gprof=no debug_tcg=no @@ -513,6 +514,7 @@ Haiku) usb=linux kvm=yes vhost_net=yes + vhost_scsi=yes if [ $cpu = i386 -o $cpu = x86_64 ] ; then audio_possible_drivers=$audio_possible_drivers fmod fi @@ -818,6 +820,10 @@ for opt do ;; --enable-vhost-net) vhost_net=yes ;; + --disable-vhost-scsi) vhost_scsi=no + ;; + --enable-vhost-scsi) vhost_scsi=yes + ;; --disable-opengl) opengl=no ;; --enable-opengl) opengl=yes @@ -3116,6 +3122,7 @@ echo posix_madvise $posix_madvise echo uuid support $uuid echo libcap-ng support $cap_ng echo vhost-net support $vhost_net +echo vhost-scsi support $vhost_scsi echo Trace backend $trace_backend echo Trace output file $trace_file-pid echo spice support $spice @@ -3828,6 +3835,9 @@ case $target_arch2 in if test $vhost_net = yes ; then echo CONFIG_VHOST_NET=y $config_target_mak fi + if test $vhost_scsi = yes ; then +echo CONFIG_VHOST_SCSI=y $config_target_mak + fi fi esac case $target_arch2 in diff --git a/hw/Makefile.objs b/hw/Makefile.objs index 3ba5dd0..6ab75ec 100644 --- a/hw/Makefile.objs +++ b/hw/Makefile.objs @@ -169,6 +169,7 @@ obj-$(CONFIG_VIRTIO) += virtio.o virtio-blk.o virtio-balloon.o virtio-net.o obj-$(CONFIG_VIRTIO) += virtio-serial-bus.o virtio-scsi.o obj-$(CONFIG_SOFTMMU) += vhost_net.o obj-$(CONFIG_VHOST_NET) += vhost.o +obj-$(CONFIG_VHOST_SCSI) += vhost-scsi.o obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/ obj-$(CONFIG_NO_PCI) += pci-stub.o obj-$(CONFIG_VGA) += vga.o diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 8aca0d4..8b505ca 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -4,6 +4,7 @@ #include blockdev.h
[Qemu-devel] [PATCH 2/5] vhost: Pass device path to vhost_dev_init()
From: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
The path to /dev/vhost-net is currently hardcoded in vhost_dev_init().
This needs to be changed so that /dev/vhost-scsi can be used. Pass in
the device path instead of hardcoding it.
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
Cc: Paolo Bonzini pbonz...@redhat.com
Cc: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
---
hw/vhost.c |5 +++--
hw/vhost.h |3 ++-
hw/vhost_net.c |2 +-
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/hw/vhost.c b/hw/vhost.c
index 0fd8da8..d0ce5aa 100644
--- a/hw/vhost.c
+++ b/hw/vhost.c
@@ -747,14 +747,15 @@ static void vhost_eventfd_del(MemoryListener *listener,
{
}
-int vhost_dev_init(struct vhost_dev *hdev, int devfd, bool force)
+int vhost_dev_init(struct vhost_dev *hdev, int devfd, const char *devpath,
+ bool force)
{
uint64_t features;
int r;
if (devfd = 0) {
hdev-control = devfd;
} else {
-hdev-control = open(/dev/vhost-net, O_RDWR);
+hdev-control = open(devpath, O_RDWR);
if (hdev-control 0) {
return -errno;
}
diff --git a/hw/vhost.h b/hw/vhost.h
index 80e64df..0c47229 100644
--- a/hw/vhost.h
+++ b/hw/vhost.h
@@ -44,7 +44,8 @@ struct vhost_dev {
bool force;
};
-int vhost_dev_init(struct vhost_dev *hdev, int devfd, bool force);
+int vhost_dev_init(struct vhost_dev *hdev, int devfd, const char *devpath,
+ bool force);
void vhost_dev_cleanup(struct vhost_dev *hdev);
bool vhost_dev_query(struct vhost_dev *hdev, VirtIODevice *vdev);
int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev);
diff --git a/hw/vhost_net.c b/hw/vhost_net.c
index ecaa22d..df2c4a3 100644
--- a/hw/vhost_net.c
+++ b/hw/vhost_net.c
@@ -109,7 +109,7 @@ struct vhost_net *vhost_net_init(NetClientState *backend,
int devfd,
(1 VHOST_NET_F_VIRTIO_NET_HDR);
net-backend = r;
-r = vhost_dev_init(net-dev, devfd, force);
+r = vhost_dev_init(net-dev, devfd, /dev/vhost-net, force);
if (r 0) {
goto fail;
}
--
1.7.2.5
[Qemu-devel] [PATCH 5/5] virtio-scsi: Set max_target=0 during vhost-scsi operation
From: Nicholas Bellinger n...@linux-iscsi.org
This QEMU patch sets VirtIOSCSIConfig-max_target=0 for vhost-scsi operation
to restrict virtio-scsi LLD guest scanning to max_id=0 (a single target ID
instance) when connected to individual tcm_vhost endpoints.
This ensures that virtio-scsi LLD only attempts to scan target IDs up to
VIRTIO_SCSI_MAX_TARGET when connected via virtio-scsi-raw.
Cc: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
Cc: Zhi Yong Wu wu...@linux.vnet.ibm.com
Cc: Paolo Bonzini pbonz...@redhat.com
Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
---
hw/virtio-scsi.c |6 +-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c
index edda097..ab5ca72 100644
--- a/hw/virtio-scsi.c
+++ b/hw/virtio-scsi.c
@@ -546,7 +546,11 @@ static void virtio_scsi_get_config(VirtIODevice *vdev,
stl_raw(scsiconf-sense_size, s-sense_size);
stl_raw(scsiconf-cdb_size, s-cdb_size);
stl_raw(scsiconf-max_channel, VIRTIO_SCSI_MAX_CHANNEL);
-stl_raw(scsiconf-max_target, VIRTIO_SCSI_MAX_TARGET);
+if (s-vhost_scsi) {
+stl_raw(scsiconf-max_target, 0);
+} else {
+stl_raw(scsiconf-max_target, VIRTIO_SCSI_MAX_TARGET);
+}
stl_raw(scsiconf-max_lun, VIRTIO_SCSI_MAX_LUN);
}
--
1.7.2.5
Re: [Qemu-devel] [Qemu-ppc] [PATCH: RFC] Adding BAR0 for e500 PCI controller
On 07.09.2012, at 01:15, Scott Wood scottw...@freescale.com wrote: On 09/03/2012 01:44 AM, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Wednesday, August 15, 2012 6:59 AM To: Bhushan Bharat-R65777 Cc: qemu-devel@nongnu.org; qemu-...@nongnu.org; ag...@suse.de; Bhushan Bharat- R65777 Subject: Re: [Qemu-ppc] [PATCH: RFC] Adding BAR0 for e500 PCI controller On 08/14/2012 07:50 AM, Bharat Bhushan wrote: PCI Root complex have TYPE-1 configuration header while PCI endpoint have type-0 configuration header. The type-1 configuration header have a BAR (BAR0). In Freescale PCI controller BAR0 is used for mapping pci address space to CCSR address space. This can used for 2 purposes: 1) for MSI interrupt generation 2) Allow CCSR registers access when configured as PCI endpoint, which I am not sure is a use case with QEMU-KVM guest. What I observed is that when guest read the size of BAR0 of host controller configuration header (TYPE1 header) then it always reads it as 0. When looking into the QEMU hw/ppce500_pci.c, I do not find the PCI controller device registering BAR0. I do not find any other controller also doing so may they do not use BAR0. There are two issues when BAR0 is not there (which I can think of): 1) There should be BAR0 emulated for PCI Root comaplex (TYPE1 header) and when reading the size of BAR0, it should give size as per real h/w. 2) Do we need this BAR0 inbound address translation? When BAR0 is of non-zero size then it will be configured for PCI address space to local address(CCSR) space translation on inbound access. The primary use case is for MSI interrupt generation. The device is configured with a address offsets in PCI address space, which will be translated to MSI interrupt generation MPIC registers. Currently I do not understand the MSI interrupt generation mechanism in QEMU and also IIRC we do not use QEMU MSI interrupt mechanism on e500 guest machines. But this BAR0 will be used when using MSI on e500. This patch is only trying to address #1, right? I don't see any connection from this BAR to CCSR. +memory_region_init_io(h-bar0, pci_host_conf_be_ops, h, + PCIHOST-bar0, 0x100); 0x0100 is correct for e500mc-based systems, but it should be 0x0010 for e500v2-based systems. Scott, Currently we have a generic e500 machine which have CCSR size 0x0010 (MPC8544_CCSRBAR_SIZE). We do not have e500mc and e500v2 machines. So should we make this 0x0010 as per generic e500 machine? Yes, but structure it so that board code decides the size, not the PCI code. Can we somehow pass this via qdev/varargs from machine emulation code (hw/ppc/e500.c) ? Possibly, though it may not be the best idea to express every single aspect of intercomponent integration via qdev -- maybe that's best left for things that are reasonably user-tweakable. If CCSR size is user tweakable, it would be somewhere other than the PCI controller. It depends. Qdev properties are basically object constructor parameters. So if you were weiting C++ code and would have a constructor that gets the size as argument, it would end up being modeled as qdev property. If however actual functionality differs, thus you would in OO speech create a subclass / child class, then you are better off creating a new device struct. In this case, I'm not sure. They are different devices really, but are close enough that the differences could be expressed through qdev properties. Let's ask someone who knows his way around these spheres. Andreas? :) Alex
[Qemu-devel] [PATCH 1/3] set the readonly property of rom memory region in pc
make sure the memory regions of rom in pc have readonly property, prepare for plug these memory to kvm as readonly memory slots. Signed-off-by: Liu Sheng liush...@linux.vnet.ibm.com --- hw/pc.c |1 + hw/pci.c |1 + 2 files changed, 2 insertions(+), 0 deletions(-) diff --git a/hw/pc.c b/hw/pc.c index c32ee8e..b7fb058 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -992,6 +992,7 @@ void *pc_memory_init(MemoryRegion *system_memory, option_rom_mr = g_malloc(sizeof(*option_rom_mr)); memory_region_init_ram(option_rom_mr, pc.rom, PC_ROM_SIZE); vmstate_register_ram_global(option_rom_mr); +memory_region_set_readonly(option_rom_mr, true); memory_region_add_subregion_overlap(rom_memory, PC_ROM_MIN_VGA, option_rom_mr, diff --git a/hw/pci.c b/hw/pci.c index 4d95984..195971c 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -1786,6 +1786,7 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom) pdev-has_rom = true; memory_region_init_ram(pdev-rom, name, size); vmstate_register_ram(pdev-rom, pdev-qdev); +memory_region_set_readonly(pdev-rom, true); ptr = memory_region_get_ram_ptr(pdev-rom); load_image(path, ptr); g_free(path); -- 1.7.7.6
[Qemu-devel] [PATCH 2/3] update kvm related the head file from kernel
updated the head file from kernel for readonly memory
feature.
Signed-off-by: Liu Sheng liush...@linux.vnet.ibm.com
---
linux-headers/asm-x86/kvm.h |1 +
linux-headers/linux/kvm.h | 16 +---
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/linux-headers/asm-x86/kvm.h b/linux-headers/asm-x86/kvm.h
index 246617e..521bf25 100644
--- a/linux-headers/asm-x86/kvm.h
+++ b/linux-headers/asm-x86/kvm.h
@@ -25,6 +25,7 @@
#define __KVM_HAVE_DEBUGREGS
#define __KVM_HAVE_XSAVE
#define __KVM_HAVE_XCRS
+#define __KVM_HAVE_READONLY_MEM
/* Architectural interrupt line count. */
#define KVM_NR_INTERRUPTS 256
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index 4b9e575..02fae9e 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -101,9 +101,13 @@ struct kvm_userspace_memory_region {
__u64 userspace_addr; /* start of the userspace allocated memory */
};
-/* for kvm_memory_region::flags */
-#define KVM_MEM_LOG_DIRTY_PAGES 1UL
-#define KVM_MEMSLOT_INVALID (1UL 1)
+/*
+ * The bit 0 ~ bit 15 of kvm_memory_region::flags are visible for userspace,
+ * other bits are reserved for kvm internal use which are defined in
+ * include/linux/kvm_host.h.
+ */
+#define KVM_MEM_LOG_DIRTY_PAGES(1UL 0)
+#define KVM_MEM_READONLY (1UL 1)
/* for KVM_IRQ_LINE */
struct kvm_irq_level {
@@ -222,6 +226,9 @@ struct kvm_run {
__u8 data[8];
__u32 len;
__u8 is_write;
+#ifdef __KVM_HAVE_READONLY_MEM
+ __u8 write_readonly_mem;
+#endif
} mmio;
/* KVM_EXIT_HYPERCALL */
struct {
@@ -618,6 +625,9 @@ struct kvm_ppc_smmu_info {
#define KVM_CAP_PPC_GET_SMMU_INFO 78
#define KVM_CAP_S390_COW 79
#define KVM_CAP_PPC_ALLOC_HTAB 80
+#ifdef __KVM_HAVE_READONLY_MEM
+#define KVM_CAP_READONLY_MEM 81
+#endif
#ifdef KVM_CAP_IRQ_ROUTING
--
1.7.7.6
[Qemu-devel] [PATCH 0/3] support kvm readonly memory slot in qemu
This patch set make the readonly memory in qemu really readonly, using readonly memory slots feature in kvm, to make qemu-kvm safer. Memory regions with readonly property would be plug into kvm as readonly memory slots. This patch set depends on the last patch Introduce write_readonly_mem in mmio-exit-info to indicate this exit is caused by write access on readonly memslot http://marc.info/?l=linux-kernelm=134563967500539w=2 of readonly memory slots in kvm, which has not been accepted yet. Liu Sheng (3): set the readonly property of rom memory region in pc update kvm related the head file from kernel support readonly memory feature in qemu hw/pc.c |1 + hw/pci.c|1 + kvm-all.c | 47 -- linux-headers/asm-x86/kvm.h |1 + linux-headers/linux/kvm.h | 16 +++-- 5 files changed, 52 insertions(+), 14 deletions(-) -- 1.7.7.6
[Qemu-devel] [PATCH 3/3] support readonly memory feature in qemu
as readonly memory is support in kvm, this patch supports this feature
in qemu, mainly pluging the memory region with KVM_MEM_READONLY flag
to kvm.
this can be tested by a kernel module from the author of kvm readonly
memory slot:
static int rom_tester_probe(struct pci_dev *dev, const struct pci_device_id *id)
{
struct resource *res = dev-resource[PCI_ROM_RESOURCE];
char buf[6];
size_t rom_size;
char * __iomem map;
int i;
if (res-flags (IORESOURCE_ROM_SHADOW | IORESOURCE_ROM_COPY |
IORESOURCE_ROM_BIOS_COPY)) {
dev_printk(KERN_INFO, dev-dev, skip ROM COPY.\n);
return 0;
}
if (res-flags
(IORESOURCE_ROM_COPY | IORESOURCE_ROM_BIOS_COPY))
dev_printk(KERN_INFO, dev-dev, rom tester\n);
if (pci_enable_rom(dev)) {
dev_printk(KERN_INFO, dev-dev, do not found Rom\n);
goto exit;
}
map = pci_map_rom(dev, rom_size);
if (!map) {
dev_printk(KERN_INFO, dev-dev, map rom fail.\n);
goto disable_exit;
}
dev_printk(KERN_INFO, dev-dev, Rom map: %p [size: %lx],
phsyc:%llx.\n,
map, rom_size, pci_resource_start(dev, PCI_ROM_RESOURCE));
if (rom_size 6) {
printk(map size 6.\n);
goto unmap_exit;
}
printk(The first 6 bytes:\n);
for (i = 0; i 6; i++) {
buf[i] = map[i];
printk(%x , buf[i]);
}
printk(\n\n);
memcpy(map, KVMKVM, 6);
if (!memcmp(map, KVMKVM, 6)) {
printk(Rom Test: fail.\n);
goto unmap_exit;
}
for (i = 0; i 6; i++)
if (buf[i] != ((char *)map)[i]) {
printk(The %d byte is changed: %x - %x.\n,
i, buf[i], map[i]);
printk(Rom Test: fail.\n);
goto unmap_exit;
}
printk(Rom Test: Okay.\n);
unmap_exit:
pci_unmap_rom(dev, map);
disable_exit:
pci_disable_rom(dev);
exit:
return 0;
}
static DEFINE_PCI_DEVICE_TABLE(rom_tester_tbl) = {
{ PCI_DEVICE(PCI_ANY_ID, PCI_ANY_ID)},
{0,}/* 0 terminated list. */
};
MODULE_DEVICE_TABLE(pci, rom_tester_tbl);
static struct pci_driver rom_tester = {
.name = pci-rom-tester,
.id_table = rom_tester_tbl,
.probe = rom_tester_probe,
};
static int __init pci_rom_test_init(void)
{
int rc;
rc = pci_register_driver(rom_tester);
if (rc)
return rc;
return 0;
}
static void __exit pci_rom_test_exit(void)
{
pci_unregister_driver(rom_tester);
}
module_init(pci_rom_test_init);
module_exit(pci_rom_test_exit);
MODULE_LICENSE(GPL);
MODULE_AUTHOR(Xiao Guangrong xiaoguangr...@linux.vnet.ibm.com);
we test it with the rom of Intel 82540EM Gigabit Ethernet Controller.
0. start qemu:
qemu-system-x86_64 -enable-kvm -m 1G -smp 2 -hda fedora16.qcow2 \
-nic nic,model=e1000,vlan=1,macadrr=52:00:12:34:56 \
-net user,vlan=1
1. unbind the device:
echo :00:03.0 /sys/bus/pci/devices/\:00\:03.0/driver/unbind
2. install the test kernel module, here we name it write_rom:
modprobe write_rom
3. print dmesg to verify the result is ok or fail:
dmesg
4. remove the test kernel module.
rmmod write_rom
5. rebind the device to its driver, test if the nic still works:
echo :00:03.0 /sys/bus/pci/drivers/e1000/bind
open firefox and try some web page.
when I use kvm without readonly memory slot, in step 2 it reports:
Rom Test: fail. this means we can write to the memory region of a rom,
which is quite not safe for the guest and host.
when I use kvm with readonly memory slot, in step 2 it reports:
Rom Test: Okay.
and the error message prints out in host console:
Guest is writing readonly memory, phys_addr: febc, len:4, data[0]:K,
skip it
Guest is writing readonly memory, phys_addr: febc, len:2, data[0]:V,
skip it
this is what we write KVMKVM.
this means write operation is not successful, and return back to qemu from kvm.
thus we can make the rom real rom.
Signed-off-by: Liu Sheng liush...@linux.vnet.ibm.com
---
kvm-all.c | 47 ---
1 files changed, 36 insertions(+), 11 deletions(-)
diff --git a/kvm-all.c b/kvm-all.c
index badf1d8..1b66183 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -97,6 +97,9 @@ struct KVMState
QTAILQ_HEAD(msi_hashtab, KVMMSIRoute) msi_hashtab[KVM_MSI_HASHTAB_SIZE];
bool direct_msi;
#endif
+#ifdef KVM_CAP_READONLY_MEM
+int readonly_mem;
+#endif
};
KVMState *kvm_state;
@@ -261,9 +264,18 @@ err:
* dirty pages logging control
*/
-static int kvm_mem_flags(KVMState *s, bool log_dirty)
Re: [Qemu-devel] [PATCH RFC] remove QEMUOptionParameter
Some overlap with what I'm working on, but since you have code to show,
and I don't, I'll review yours as if mine didn't exist.
Dong Xu Wang wdon...@linux.vnet.ibm.com writes:
QEMU now has QEMUOptionParameter and QemuOpts to parse parameters, so we can
remove one safely. This RFC removed QEMUOptionParameter and use QemuOpts. But
the patch is not completed, I think it is better to send a RFC first. In the
RFC, I only allow qcow2 and raw file format and did not test it completly, if
you have any concerns and suggestions about the main idea, please let me know,
that wil be very grateful.
Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
---
block.c | 106 -
block.h |8 +-
block/Makefile.objs |9 +-
block/qcow2.c | 123 --
block/raw-posix.c | 45 +
block/raw.c | 12 +--
block_int.h |5 +-
qemu-config.c | 85 ++
qemu-img.c | 67
qemu-option.c | 456
+++
qemu-option.h | 47 +-
11 files changed, 417 insertions(+), 546 deletions(-)
diff --git a/block.c b/block.c
index 470bdcc..8e973ff 100644
--- a/block.c
+++ b/block.c
@@ -351,7 +351,7 @@ BlockDriver *bdrv_find_whitelisted_format(const char
*format_name)
typedef struct CreateCo {
BlockDriver *drv;
char *filename;
-QEMUOptionParameter *options;
+QemuOpts *options;
Consider renaming to opts, because that's the commonly used name. You
did it already elsewhere.
int ret;
} CreateCo;
@@ -364,7 +364,7 @@ static void coroutine_fn bdrv_create_co_entry(void
*opaque)
}
int bdrv_create(BlockDriver *drv, const char* filename,
-QEMUOptionParameter *options)
+QemuOpts *opts)
{
int ret;
@@ -372,7 +372,7 @@ int bdrv_create(BlockDriver *drv, const char* filename,
CreateCo cco = {
.drv = drv,
.filename = g_strdup(filename),
-.options = options,
+.options = opts,
.ret = NOT_DONE,
};
@@ -397,7 +397,7 @@ int bdrv_create(BlockDriver *drv, const char* filename,
return ret;
}
-int bdrv_create_file(const char* filename, QEMUOptionParameter *options)
+int bdrv_create_file(const char *filename, QemuOpts *opts)
{
BlockDriver *drv;
@@ -406,7 +406,7 @@ int bdrv_create_file(const char* filename,
QEMUOptionParameter *options)
return -ENOENT;
}
-return bdrv_create(drv, filename, options);
+return bdrv_create(drv, filename, opts);
}
/*
@@ -742,8 +742,9 @@ int bdrv_open(BlockDriverState *bs, const char *filename,
int flags,
int64_t total_size;
int is_protocol = 0;
BlockDriver *bdrv_qcow2;
-QEMUOptionParameter *options;
+QemuOpts *options;
char backing_filename[PATH_MAX];
+char buf_total_size[1024];
/* if snapshot, we create a temporary backing file and open it
instead of opening 'filename' directly */
@@ -775,17 +776,19 @@ int bdrv_open(BlockDriverState *bs, const char
*filename, int flags,
return -errno;
bdrv_qcow2 = bdrv_find_format(qcow2);
-options = parse_option_parameters(, bdrv_qcow2-create_options,
NULL);
+options = qemu_opts_create(qemu_find_opts(qcow2_create_options),
+NULL, 0, NULL);
I'm afraid you named them qcow2_create_opts in qemu-config.c.
-set_option_parameter_int(options, BLOCK_OPT_SIZE, total_size);
-set_option_parameter(options, BLOCK_OPT_BACKING_FILE,
backing_filename);
+snprintf(buf_total_size, sizeof(buf_total_size),
+% PRId64, total_size);
+qemu_opt_set(options, BLOCK_OPT_SIZE, buf_total_size);
This is a bit awkward.
We could have qemu_opt_set_number(), like qemu_opt_set_bool(). Except
qemu_opt_set_bool() has issues. Luiz's fix is discussed here:
http://lists.nongnu.org/archive/html/qemu-devel/2012-07/msg02716.html
Luiz, do you plan to respin?
+qemu_opt_set(options, BLOCK_OPT_BACKING_FILE, backing_filename);
if (drv) {
-set_option_parameter(options, BLOCK_OPT_BACKING_FMT,
+qemu_opt_set(options, BLOCK_OPT_BACKING_FMT,
drv-format_name);
}
ret = bdrv_create(bdrv_qcow2, tmp_filename, options);
-free_option_parameters(options);
if (ret 0) {
return ret;
}
This means ownership of options now passes to bdrv_create(). Which
doesn't free it either. Should it?
@@ -3901,12 +3904,16 @@ int bdrv_img_create(const char *filename, const char
*fmt,
const char *base_filename, const char *base_fmt,
char *options, uint64_t img_size, int flags)
{
-QEMUOptionParameter *param = NULL, *create_options = NULL;
-QEMUOptionParameter *backing_fmt, *backing_file, *size;
+
Re: [Qemu-devel] [PATCH] target-cris: Fix buffer overflow
On Tue, Sep 04, 2012 at 07:45:52AM +0200, Stefan Weil wrote: Report from smatch: target-cris/translate.c:3464 cpu_dump_state(32) error: buffer overflow 'env-sregs' 4 = 255 sregs is declared 'uint32_t sregs[4][16]', so the first index must be less than 4. Hi Stefan, I think it would be better to use ARRAY_SIZE(env-sregs) instead of 4. The cris arch allows up to 256 sregs, but we only implement 4 at the moment. There are other uses of hardcoded 4 in the code that could be fixed aswell if you have time. Thanks, Edgar
Re: [Qemu-devel] [PATCH 2/3] update kvm related the head file from kernel
On 2012-09-07 10:26, Liu Sheng wrote:
updated the head file from kernel for readonly memory
feature.
Signed-off-by: Liu Sheng liush...@linux.vnet.ibm.com
---
linux-headers/asm-x86/kvm.h |1 +
linux-headers/linux/kvm.h | 16 +---
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/linux-headers/asm-x86/kvm.h b/linux-headers/asm-x86/kvm.h
index 246617e..521bf25 100644
--- a/linux-headers/asm-x86/kvm.h
+++ b/linux-headers/asm-x86/kvm.h
@@ -25,6 +25,7 @@
#define __KVM_HAVE_DEBUGREGS
#define __KVM_HAVE_XSAVE
#define __KVM_HAVE_XCRS
+#define __KVM_HAVE_READONLY_MEM
/* Architectural interrupt line count. */
#define KVM_NR_INTERRUPTS 256
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index 4b9e575..02fae9e 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -101,9 +101,13 @@ struct kvm_userspace_memory_region {
__u64 userspace_addr; /* start of the userspace allocated memory */
};
-/* for kvm_memory_region::flags */
-#define KVM_MEM_LOG_DIRTY_PAGES 1UL
-#define KVM_MEMSLOT_INVALID (1UL 1)
+/*
+ * The bit 0 ~ bit 15 of kvm_memory_region::flags are visible for userspace,
+ * other bits are reserved for kvm internal use which are defined in
+ * include/linux/kvm_host.h.
+ */
+#define KVM_MEM_LOG_DIRTY_PAGES (1UL 0)
+#define KVM_MEM_READONLY (1UL 1)
/* for KVM_IRQ_LINE */
struct kvm_irq_level {
@@ -222,6 +226,9 @@ struct kvm_run {
__u8 data[8];
__u32 len;
__u8 is_write;
+#ifdef __KVM_HAVE_READONLY_MEM
+ __u8 write_readonly_mem;
+#endif
} mmio;
/* KVM_EXIT_HYPERCALL */
struct {
@@ -618,6 +625,9 @@ struct kvm_ppc_smmu_info {
#define KVM_CAP_PPC_GET_SMMU_INFO 78
#define KVM_CAP_S390_COW 79
#define KVM_CAP_PPC_ALLOC_HTAB 80
+#ifdef __KVM_HAVE_READONLY_MEM
+#define KVM_CAP_READONLY_MEM 81
+#endif
Sorry for commenting on the wrong patch, this must be fixed in the
kernel: no more conditional CAPs, please. Usually, they only require
#ifdef messes in QEMU (see patch 3).
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 3/3] support readonly memory feature in qemu
On 2012-09-07 10:26, Liu Sheng wrote:
as readonly memory is support in kvm, this patch supports this feature
in qemu, mainly pluging the memory region with KVM_MEM_READONLY flag
to kvm.
this can be tested by a kernel module from the author of kvm readonly
memory slot:
static int rom_tester_probe(struct pci_dev *dev, const struct pci_device_id
*id)
{
struct resource *res = dev-resource[PCI_ROM_RESOURCE];
char buf[6];
size_t rom_size;
char * __iomem map;
int i;
if (res-flags (IORESOURCE_ROM_SHADOW | IORESOURCE_ROM_COPY |
IORESOURCE_ROM_BIOS_COPY)) {
dev_printk(KERN_INFO, dev-dev, skip ROM COPY.\n);
return 0;
}
if (res-flags
(IORESOURCE_ROM_COPY | IORESOURCE_ROM_BIOS_COPY))
dev_printk(KERN_INFO, dev-dev, rom tester\n);
if (pci_enable_rom(dev)) {
dev_printk(KERN_INFO, dev-dev, do not found Rom\n);
goto exit;
}
map = pci_map_rom(dev, rom_size);
if (!map) {
dev_printk(KERN_INFO, dev-dev, map rom fail.\n);
goto disable_exit;
}
dev_printk(KERN_INFO, dev-dev, Rom map: %p [size: %lx],
phsyc:%llx.\n,
map, rom_size, pci_resource_start(dev, PCI_ROM_RESOURCE));
if (rom_size 6) {
printk(map size 6.\n);
goto unmap_exit;
}
printk(The first 6 bytes:\n);
for (i = 0; i 6; i++) {
buf[i] = map[i];
printk(%x , buf[i]);
}
printk(\n\n);
memcpy(map, KVMKVM, 6);
if (!memcmp(map, KVMKVM, 6)) {
printk(Rom Test: fail.\n);
goto unmap_exit;
}
for (i = 0; i 6; i++)
if (buf[i] != ((char *)map)[i]) {
printk(The %d byte is changed: %x - %x.\n,
i, buf[i], map[i]);
printk(Rom Test: fail.\n);
goto unmap_exit;
}
printk(Rom Test: Okay.\n);
unmap_exit:
pci_unmap_rom(dev, map);
disable_exit:
pci_disable_rom(dev);
exit:
return 0;
}
static DEFINE_PCI_DEVICE_TABLE(rom_tester_tbl) = {
{ PCI_DEVICE(PCI_ANY_ID, PCI_ANY_ID)},
{0,}/* 0 terminated list. */
};
MODULE_DEVICE_TABLE(pci, rom_tester_tbl);
static struct pci_driver rom_tester = {
.name = pci-rom-tester,
.id_table = rom_tester_tbl,
.probe = rom_tester_probe,
};
static int __init pci_rom_test_init(void)
{
int rc;
rc = pci_register_driver(rom_tester);
if (rc)
return rc;
return 0;
}
static void __exit pci_rom_test_exit(void)
{
pci_unregister_driver(rom_tester);
}
module_init(pci_rom_test_init);
module_exit(pci_rom_test_exit);
MODULE_LICENSE(GPL);
MODULE_AUTHOR(Xiao Guangrong xiaoguangr...@linux.vnet.ibm.com);
we test it with the rom of Intel 82540EM Gigabit Ethernet Controller.
0. start qemu:
qemu-system-x86_64 -enable-kvm -m 1G -smp 2 -hda fedora16.qcow2 \
-nic nic,model=e1000,vlan=1,macadrr=52:00:12:34:56 \
-net user,vlan=1
1. unbind the device:
echo :00:03.0 /sys/bus/pci/devices/\:00\:03.0/driver/unbind
2. install the test kernel module, here we name it write_rom:
modprobe write_rom
3. print dmesg to verify the result is ok or fail:
dmesg
4. remove the test kernel module.
rmmod write_rom
5. rebind the device to its driver, test if the nic still works:
echo :00:03.0 /sys/bus/pci/drivers/e1000/bind
open firefox and try some web page.
when I use kvm without readonly memory slot, in step 2 it reports:
Rom Test: fail. this means we can write to the memory region of a rom,
which is quite not safe for the guest and host.
when I use kvm with readonly memory slot, in step 2 it reports:
Rom Test: Okay.
and the error message prints out in host console:
Guest is writing readonly memory, phys_addr: febc, len:4, data[0]:K,
skip it
Guest is writing readonly memory, phys_addr: febc, len:2, data[0]:V,
skip it
this is what we write KVMKVM.
this means write operation is not successful, and return back to qemu from
kvm.
thus we can make the rom real rom.
Signed-off-by: Liu Sheng liush...@linux.vnet.ibm.com
---
kvm-all.c | 47 ---
1 files changed, 36 insertions(+), 11 deletions(-)
diff --git a/kvm-all.c b/kvm-all.c
index badf1d8..1b66183 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -97,6 +97,9 @@ struct KVMState
QTAILQ_HEAD(msi_hashtab, KVMMSIRoute) msi_hashtab[KVM_MSI_HASHTAB_SIZE];
bool direct_msi;
#endif
+#ifdef KVM_CAP_READONLY_MEM
+int readonly_mem;
+#endif
See comment on patch 2: Make sure that KVM_CAP_READONLY_MEM is always
Re: [Qemu-devel] [PATCH v6 2/2] block: Support GlusterFS as a QEMU block backend
On Fri, Sep 07, 2012 at 08:54:02AM +0530, Bharata B Rao wrote: On Thu, Sep 06, 2012 at 04:47:17PM +0100, Daniel P. Berrange wrote: IMHO this is all gross. URIs already have a well defined way to provide multiple parameters, dealing with escaping of special characters. ie query parameters. The whole benefit of using URI syntax is to let apps process the URIs using standard APIs. We should not be trying to define some extra magic encoding to let us stuff 2 separate parameters into the path component since that means apps have to now write custom parsing code again. Either the UNIX socket path, or the volume path should be in the URI path, not both. The other part should be a URI parameter. I'd really expect us to use: gluster:///volname/image?transport=unixsockpath=/path/to/unix/sock ^why 3 /// here ? volname is not a path, but image is. The first 2 '/' are the protocol/hostname separator and the 3rd / is the hostname/path separator. Since there is no hostname in this example, you get /// So when transport=socket or rdma, is it acceptable to still use the following format ? gluster://server[:port]/volname/path/to/image[?transport=...] Of course. With that, the different options possible are unix - gluster://testvol/dir/a.img?transport=unixsockpath=/tmp/glusterd.socket ipv4 - gluster://1.2.3.4:0/testvol/dir/a.img?transport=socket I assume you intend 'transport=socket' to be optional here ipv6 - gluster://[1:2:3:4:5:6:7:8]:0/testvol/a.img hostname - gluster://example.org/testvol/dir/a.img rdma - gluster://1.2.3.4:0/testvol/a.img?transport=rdma Does this look complete from the specification point of view ? Yes, this looks like a reasonable use of URIs. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH for-1.2 v2 0/5] target-ppc: MAINTAINERS additions
On Wed, Aug 29, 2012 at 05:51:09PM +0200, Andreas Färber wrote: Am 22.08.2012 17:48, schrieb Andreas Färber: Hi Alex, Here's an updated and squashed series adding some MAINTAINERS sections for ppc machines and their devices. Based on Peter's suggestion the machines are no longer grouped by ppc4xx chipset but ordered alphabetically, and the shared devices both for ppc4xx and e500 get their own Devices sections for rule sharing across machines. Since this is documentation only I would like to get this into rc1. Feel free to adapt/reorder in any way you see fit to document things. Ping? Ack on the part that adds me as maintainer for the virtex board. I'm assuming this should go via Alex ppc tree. Acked-by: Edgar E. Iglesias edgar.igles...@gmail.com Cheers, Edgar
Re: [Qemu-devel] [PATCH v6 2/2] block: Support GlusterFS as a QEMU block backend
Il 07/09/2012 05:24, Bharata B Rao ha scritto: gluster:///volname/image?transport=unixsockpath=/path/to/unix/sock ^why 3 /// here ? volname is not a path, but image is. Because the host is the local computer, i.e. empty. gluster://server[:port]/volname/path/to/image[?transport=...] With that, the different options possible are unix - gluster://testvol/dir/a.img?transport=unixsockpath=/tmp/glusterd.socket ipv4 - gluster://1.2.3.4:0/testvol/dir/a.img?transport=socket ipv6 - gluster://[1:2:3:4:5:6:7:8]:0/testvol/a.img hostname - gluster://example.org/testvol/dir/a.img rdma - gluster://1.2.3.4:0/testvol/a.img?transport=rdma Does this look complete from the specification point of view ? Hmm, why don't we do the exact same thing as libvirt (http://libvirt.org/remote.html): ipv4 - gluster+tcp://1.2.3.4:0/testvol/dir/a.img ipv6 - gluster+tcp://[1:2:3:4:5:6:7:8]:0/testvol/a.img host - gluster+tcp://example.org/testvol/dir/a.img unix - gluster+unix:///testvol/dir/a.img?socket=/tmp/glusterd.socket rdma - gluster+rdma://1.2.3.4:0/testvol/a.img Where the default transport is tcp (i.e. gluster+tcp is the same as gluster). This is easily extensible, theoretically you could have something like this: ssh - gluster+ssh://user@host/testvol/a.img?socket=/tmp/glusterd.socket Paolo
Re: [Qemu-devel] [PATCH 2/3] update kvm related the head file from kernel
Hi Jan,
Thanks for your review.
On 09/07/2012 04:49 PM, Jan Kiszka wrote:
+#ifdef __KVM_HAVE_READONLY_MEM
+__u8 write_readonly_mem;
+#endif
} mmio;
/* KVM_EXIT_HYPERCALL */
struct {
@@ -618,6 +625,9 @@ struct kvm_ppc_smmu_info {
#define KVM_CAP_PPC_GET_SMMU_INFO 78
#define KVM_CAP_S390_COW 79
#define KVM_CAP_PPC_ALLOC_HTAB 80
+#ifdef __KVM_HAVE_READONLY_MEM
+#define KVM_CAP_READONLY_MEM 81
+#endif
Sorry for commenting on the wrong patch, this must be fixed in the
kernel: no more conditional CAPs, please.
There are some common code depend on KVM_CAP_READONLY_MEM, such as,
check_memory_region_flags() and write_readonly_mem field in mmio-exit-info.
Yes, we can use #ifdef CONFIG_X86 to enable it only on x86, but it
is hard to expand this feather to other arches in the further.
So, i used __KVM_HAVE_READONLY_MEM and only defined it on x86. :)
Re: [Qemu-devel] [PATCH v6 2/2] block: Support GlusterFS as a QEMU block backend
Am 07.09.2012 11:36, schrieb Paolo Bonzini: Il 07/09/2012 05:24, Bharata B Rao ha scritto: gluster:///volname/image?transport=unixsockpath=/path/to/unix/sock ^why 3 /// here ? volname is not a path, but image is. Because the host is the local computer, i.e. empty. gluster://server[:port]/volname/path/to/image[?transport=...] With that, the different options possible are unix - gluster://testvol/dir/a.img?transport=unixsockpath=/tmp/glusterd.socket ipv4 - gluster://1.2.3.4:0/testvol/dir/a.img?transport=socket ipv6 - gluster://[1:2:3:4:5:6:7:8]:0/testvol/a.img hostname - gluster://example.org/testvol/dir/a.img rdma - gluster://1.2.3.4:0/testvol/a.img?transport=rdma Does this look complete from the specification point of view ? Hmm, why don't we do the exact same thing as libvirt (http://libvirt.org/remote.html): ipv4 - gluster+tcp://1.2.3.4:0/testvol/dir/a.img ipv6 - gluster+tcp://[1:2:3:4:5:6:7:8]:0/testvol/a.img host - gluster+tcp://example.org/testvol/dir/a.img unix - gluster+unix:///testvol/dir/a.img?socket=/tmp/glusterd.socket rdma - gluster+rdma://1.2.3.4:0/testvol/a.img Where the default transport is tcp (i.e. gluster+tcp is the same as gluster). This is easily extensible, theoretically you could have something like this: ssh - gluster+ssh://user@host/testvol/a.img?socket=/tmp/glusterd.socket I like this. Having the type only as a parameter when it decides how the schema must be parsed has bothered me from the start, but I hadn't thought of this way. Strong +1 from me. Kevin
Re: [Qemu-devel] [PATCH v6 2/2] block: Support GlusterFS as a QEMU block backend
Am 06.09.2012 17:47, schrieb Daniel P. Berrange: On Thu, Sep 06, 2012 at 09:10:04PM +0530, Bharata B Rao wrote: On Thu, Sep 06, 2012 at 11:29:36AM +0300, Avi Kivity wrote: On 08/14/2012 12:58 PM, Kevin Wolf wrote: While we are at this, let me bring out another issue. Gluster supports 3 transport types: - socket in which case the server will be hostname, ipv4 or ipv4 address. - rdma in which case server will be interpreted similar to socket. - unix in which case server will be a path to unix domain socket and this will look like any other filesystem path. (Eg. /tmp/glusterd.socket) I don't think we can fit 'unix' within the standard URI scheme (RFC 3986) easily, but I am planning to specify the 'unix' transport as below: gluster://[/path/to/unix/domain/socket]/volname/image?transport=unix i,e., I am asking the user to put the unix domain socket path within square brackets when transport type is unix. Do you think this is fine ? Never saw something like this before, but it does seem reasonable to me. Excludes ] from the valid characters in the file name of the socket, but that shouldn't be a problem in practice. Bikeshedding, but I prefer gluster:///path/to/unix/domain/socket:/volname/image?transport=unix So if the unix domain socket is /tmp/glusterd.socket, then this would look like: gluster:///tmp/glusterd.socket:/volname/image?transport=unix. So you are saying : will the separator b/n the unix domain socket path and rest of the URI components. Unless you or others strongly feel about this, I would like to go with [ ] based spec, which I feel is less prone to errors like missing a colon by mistake :) as being more similar to file://, or even gluster:///path/to/unix/domain/socket/volname/image?transport=unix with the last two components implied to be part of the payload, not the path. Note that image is a file path by itself like /dir1/a.img. So I guess it becomes difficult to figure out where the unix domain socket path ends and rest of the URI components begin w/o a separator in between. IMHO this is all gross. URIs already have a well defined way to provide multiple parameters, dealing with escaping of special characters. ie query parameters. The whole benefit of using URI syntax is to let apps process the URIs using standard APIs. We should not be trying to define some extra magic encoding to let us stuff 2 separate parameters into the path component since that means apps have to now write custom parsing code again. Either the UNIX socket path, or the volume path should be in the URI path, not both. The other part should be a URI parameter. I'd really expect us to use: gluster:///volname/image?transport=unixsockpath=/path/to/unix/sock I think doing it the other way round would be more logical: gluster+unix:///path/to/unix/sock?image=volname/image This way you have the socket first, which you also must open first. Having it as a parameter without which you can't make sense of the path feels a bit less than ideal. Kevin
Re: [Qemu-devel] [PATCH v6 2/2] block: Support GlusterFS as a QEMU block backend
On Fri, Sep 07, 2012 at 12:00:50PM +0200, Kevin Wolf wrote: Am 06.09.2012 17:47, schrieb Daniel P. Berrange: On Thu, Sep 06, 2012 at 09:10:04PM +0530, Bharata B Rao wrote: On Thu, Sep 06, 2012 at 11:29:36AM +0300, Avi Kivity wrote: On 08/14/2012 12:58 PM, Kevin Wolf wrote: While we are at this, let me bring out another issue. Gluster supports 3 transport types: - socket in which case the server will be hostname, ipv4 or ipv4 address. - rdma in which case server will be interpreted similar to socket. - unix in which case server will be a path to unix domain socket and this will look like any other filesystem path. (Eg. /tmp/glusterd.socket) I don't think we can fit 'unix' within the standard URI scheme (RFC 3986) easily, but I am planning to specify the 'unix' transport as below: gluster://[/path/to/unix/domain/socket]/volname/image?transport=unix i,e., I am asking the user to put the unix domain socket path within square brackets when transport type is unix. Do you think this is fine ? Never saw something like this before, but it does seem reasonable to me. Excludes ] from the valid characters in the file name of the socket, but that shouldn't be a problem in practice. Bikeshedding, but I prefer gluster:///path/to/unix/domain/socket:/volname/image?transport=unix So if the unix domain socket is /tmp/glusterd.socket, then this would look like: gluster:///tmp/glusterd.socket:/volname/image?transport=unix. So you are saying : will the separator b/n the unix domain socket path and rest of the URI components. Unless you or others strongly feel about this, I would like to go with [ ] based spec, which I feel is less prone to errors like missing a colon by mistake :) as being more similar to file://, or even gluster:///path/to/unix/domain/socket/volname/image?transport=unix with the last two components implied to be part of the payload, not the path. Note that image is a file path by itself like /dir1/a.img. So I guess it becomes difficult to figure out where the unix domain socket path ends and rest of the URI components begin w/o a separator in between. IMHO this is all gross. URIs already have a well defined way to provide multiple parameters, dealing with escaping of special characters. ie query parameters. The whole benefit of using URI syntax is to let apps process the URIs using standard APIs. We should not be trying to define some extra magic encoding to let us stuff 2 separate parameters into the path component since that means apps have to now write custom parsing code again. Either the UNIX socket path, or the volume path should be in the URI path, not both. The other part should be a URI parameter. I'd really expect us to use: gluster:///volname/image?transport=unixsockpath=/path/to/unix/sock I think doing it the other way round would be more logical: gluster+unix:///path/to/unix/sock?image=volname/image This way you have the socket first, which you also must open first. Having it as a parameter without which you can't make sense of the path feels a bit less than ideal. The issue here is that the volume/path/to/image part is something that is required for all gluster transports. The /path/to/unix/sock is something that is only required for the unix transport. To have consistent URI scheme across all transports, you really want the volume/path/to/image bit to use the URI path component. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH v6 2/2] block: Support GlusterFS as a QEMU block backend
Il 07/09/2012 12:03, Daniel P. Berrange ha scritto: I think doing it the other way round would be more logical: gluster+unix:///path/to/unix/sock?image=volname/image This way you have the socket first, which you also must open first. Having it as a parameter without which you can't make sense of the path feels a bit less than ideal. The issue here is that the volume/path/to/image part is something that is required for all gluster transports. The /path/to/unix/sock is something that is only required for the unix transport. To have consistent URI scheme across all transports, you really want the volume/path/to/image bit to use the URI path component. I was writing the same---plus, perhaps there is a default for the Unix socket path, so that in the common case you could avoid the parameters altogether? Paolo
Re: [Qemu-devel] [PATCH 2/3] update kvm related the head file from kernel
On 2012-09-07 11:39, Xiao Guangrong wrote:
Hi Jan,
Thanks for your review.
On 09/07/2012 04:49 PM, Jan Kiszka wrote:
+#ifdef __KVM_HAVE_READONLY_MEM
+ __u8 write_readonly_mem;
+#endif
} mmio;
/* KVM_EXIT_HYPERCALL */
struct {
@@ -618,6 +625,9 @@ struct kvm_ppc_smmu_info {
#define KVM_CAP_PPC_GET_SMMU_INFO 78
#define KVM_CAP_S390_COW 79
#define KVM_CAP_PPC_ALLOC_HTAB 80
+#ifdef __KVM_HAVE_READONLY_MEM
+#define KVM_CAP_READONLY_MEM 81
+#endif
Sorry for commenting on the wrong patch, this must be fixed in the
kernel: no more conditional CAPs, please.
There are some common code depend on KVM_CAP_READONLY_MEM, such as,
Let is depend on KVM_HAVE or some CONFIG_xxx, not on something that is
going to be exported as userspace API.
Jan
check_memory_region_flags() and write_readonly_mem field in mmio-exit-info.
Yes, we can use #ifdef CONFIG_X86 to enable it only on x86, but it
is hard to expand this feather to other arches in the further.
So, i used __KVM_HAVE_READONLY_MEM and only defined it on x86. :)
--
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [RFC v2 PATCH 1/6] block: add support functions for live commit, to find and delete images.
Am 06.09.2012 16:59, schrieb Jeff Cody:
On 09/06/2012 09:23 AM, Kevin Wolf wrote:
Am 30.08.2012 20:47, schrieb Jeff Cody:
Add bdrv_find_child(), and bdrv_delete_intermediate().
bdrv_find_child(): given 'bs' and the active (topmost) BDS of an image
chain,
find the image that is the immediate top of 'bs'
bdrv_delete_intermediate():
Given 3 BDS (active, top, base), delete images above
base up to and including top, and set base to be the
parent of top's child node.
E.g., this converts:
bottom - base - intermediate - top - active
to
bottom - base - active
where top == active is permitted, although active
will not be deleted.
Signed-off-by: Jeff Cody jc...@redhat.com
At first, when just reading the function name, I thought this would
actually delete the image file. Of course, it only removes it from the
backing file chain, but leaves the image file around. I don't have a
good suggestion, but if someone has a better name, I think we should
change it.
Hmm, the naming seems consistent with bdrv_delete(), which does not
actually delete the image files either (and, that is essentially what
this does... calls bdrv_delete(), on the intermediate images).
However, here are some other name proposals:
* bdrv_disconnect_intermediate()
* bdrv_drop_intermediate()
* bdrv_shorten_chain()
bdrv_drop_intermediate() sounds good to me.
+
+typedef struct BlkIntermediateStates {
+BlockDriverState *bs;
+QSIMPLEQ_ENTRY(BlkIntermediateStates) entry;
+} BlkIntermediateStates;
+
+
+/* deletes images above 'base' up to and including 'top', and sets the
image
+ * above 'top' to have base as its backing file.
+ *
+ * E.g., this will convert the following chain:
+ * bottom - base - intermediate - top - active
+ *
+ * to
+ *
+ * bottom - base - active
+ *
+ * It is allowed for bottom==base, in which case it converts:
+ *
+ * base - intermediate - top - active
+ *
+ * to
+ *
+ * base - active
+ *
+ * It is also allowed for top==active, except in that case active is not
+ * deleted:
Hm, makes the interface inconsistent. Shouldn't you be using top ==
intermediate and it would work without any special casing?
To remain consistent, maybe we should define it as an error if
top==active, and return error in that case? The caller can be
responsible for checking for that - if the caller wants to merge down
the active layer, there are additional steps to be taken anyway.
Yes, why not.
And we can always revisit when implementing the additional functionality.
+/* we could not find the image above 'top', this is an error */
+goto exit;
+}
+
+/* if the active and top image passed in are the same, then we
+ * can't delete the active, so we start one below
+ */
+intermediate = (active == top) ? active-backing_hd : top;
Aha. So intermediate is used to undo the special case. Now we're always
on the last image to be deleted.
This is equivalent to an unconditional new_top_bs-backing_hd.
How about changing this to use the simpler unconditional version?
Kevin
Re: [Qemu-devel] [PATCH 0/8] fdc: fix FD_SR0_SEEK flag + implement VERIFY
Am 06.09.2012 21:17, schrieb Hervé Poussineau: Attached patches have 3 goals: - patches 1-4 fix the NT 3.1 floppy driver problem. - patches 5-6 implement the VERIFY command - patches 7-8 are some simple cleanup Any specific reason for resending and reordering my patches? They are already reviewed and applied to block-next, so just posting something on top of it would be easier. Even if my patches were wrong, I think it would be useful to have the fix on top because then you could add a qtest case that shows that my patches were wrong and your changes are correct. Kevin
Re: [Qemu-devel] [PATCH 3/7] block: raw-posix image file reopen
Am 06.09.2012 17:34, schrieb Corey Bryant:
On 09/06/2012 05:23 AM, Kevin Wolf wrote:
Am 05.09.2012 18:43, schrieb Jeff Cody:
+}
+
+int fcntl_flags = O_APPEND | O_ASYNC | O_NONBLOCK;
+#ifdef O_NOATIME
+fcntl_flags |= O_NOATIME;
+#endif
+if ((raw_s-open_flags ~fcntl_flags) == (s-open_flags
~fcntl_flags)) {
+/* dup the original fd */
+/* TODO: use qemu fcntl wrapper */
+raw_s-fd = fcntl(s-fd, F_DUPFD_CLOEXEC, 0);
+if (raw_s-fd == -1) {
+ret = -1;
+goto error;
+}
+ret = fcntl_setfl(raw_s-fd, raw_s-open_flags);
+} else {
+raw_s-fd = qemu_open(state-bs-filename, raw_s-open_flags,
0644);
+if (raw_s-fd == -1) {
+ret = -1;
+}
Ignoring this part for now, with qemu_dup_flags() it's going to look a
bit different. In particular, I'm hoping that we don't get a second
fcntl_flags enumeration here, but can just fall back to qemu_open()
whenever qemu_dup_flags() fails.
That will require modification to qemu_dup_flags()... I believe
qemu_dup_flags() silently filters out fcntl incompatible flags.
Maybe it would be best to create a small helper function in osdep.c, that
fetches the fcntl_flags. Then qemu_dup_flags() and this function would
use the same helper to fetch fcntl_flags. The results of that would
determine if we call qemu_dup_flags() or qemu_open().
Although, I do think it makes sense to always try qemu_open() if
qemu_dup_flags() fails for some reason.
I'm curious why you can't always call qemu_open().
I believe the original reason was that qemu_open() is more likely to
fail, for example if the image file has been renamed/moved/deleted since
the first open. You could still use fcntl() on an existing file
descriptor, but reopening would fail.
Some things to consider so that fd passing doesn't break when a reopen
occurs. Mainly all the concerns revolve around how fd passing keeps
track of references to fd sets (note: adding and removing fd set
references is all done in qemu_open and qemu_close).
* When reopening, qemu_open needs to be called before qemu_close. This
will prevent the reference list for an fdset from becoming empty. If
qemu_close is called before qemu_open, the reference list can become
empty, and the fdset could be cleaned up before the qemu_open. Then
qemu_open would fail.
Will automatically be right when we properly implement transactional
semantics.
* qemu_open/qemu_close need to be used rather than open/close so that
the references for fd passing are properly accounted for.
Congratulations, you've just discovered a bug in Jeff's patches. It was
a good idea to CC you. ;-)
* I don't think you want to call qemu_dup_flags directly since it
doesn't update the reference list for fd passing. Only qemu_open and
qemu_close update the reference list.
That's a good point, too. So probably a small wrapper that just updates
the reference list in addition?
If we can modify qemu_dup_flags() to fail if it can't provide the right
set of flags, then I think we should do it - and I think we can. Even
for the existing cases with fd passing it shouldn't break anything, but
only add an additional safety check.
And if touching the function motivates Corey to write some fd passing
test cases so that you can't break it, even better. ;-)
:) Sorry, I do plan to do this soon. I've just been side-tracked with
some other things.
No problem, it was just such a great opportunity to remind you. ;-)
Kevin
Re: [Qemu-devel] [PATCH 7/8] tcg/optimize: add constant folding for setcond
On Thu, Sep 06, 2012 at 09:40:57AM -0700, Richard Henderson wrote:
On 09/06/2012 08:00 AM, Aurelien Jarno wrote:
Signed-off-by: Aurelien Jarno aurel...@aurel32.net
---
tcg/optimize.c | 79
1 file changed, 79 insertions(+)
diff --git a/tcg/optimize.c b/tcg/optimize.c
index 7debc8a..c4af1e8 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -267,6 +267,65 @@ static TCGArg do_constant_folding(TCGOpcode op, TCGArg
x, TCGArg y)
return res;
}
+static TCGArg do_constant_folding_cond(TCGOpcode op, TCGArg x,
+ TCGArg y, TCGCond c)
+{
+switch (op_bits(op)) {
+case 32:
+switch (c) {
+case TCG_COND_EQ:
+return (uint32_t)x == (uint32_t)y;
+case TCG_COND_NE:
+return (uint32_t)x != (uint32_t)y;
+case TCG_COND_LT:
+return (int32_t)x (int32_t)y;
+case TCG_COND_GE:
+return (int32_t)x = (int32_t)y;
+case TCG_COND_LE:
+return (int32_t)x = (int32_t)y;
+case TCG_COND_GT:
+return (int32_t)x (int32_t)y;
+case TCG_COND_LTU:
+return (uint32_t)x (uint32_t)y;
+case TCG_COND_GEU:
+return (uint32_t)x = (uint32_t)y;
+case TCG_COND_LEU:
+return (uint32_t)x = (uint32_t)y;
+case TCG_COND_GTU:
+return (uint32_t)x (uint32_t)y;
+}
+case 64:
+switch (c) {
+case TCG_COND_EQ:
+return (uint64_t)x == (uint64_t)y;
+case TCG_COND_NE:
+return (uint64_t)x != (uint64_t)y;
+case TCG_COND_LT:
+return (int64_t)x (int64_t)y;
+case TCG_COND_GE:
+return (int64_t)x = (int64_t)y;
+case TCG_COND_LE:
+return (int64_t)x = (int64_t)y;
+case TCG_COND_GT:
+return (int64_t)x (int64_t)y;
+case TCG_COND_LTU:
+return (uint64_t)x (uint64_t)y;
+case TCG_COND_GEU:
+return (uint64_t)x = (uint64_t)y;
+case TCG_COND_LEU:
+return (uint64_t)x = (uint64_t)y;
+case TCG_COND_GTU:
+return (uint64_t)x (uint64_t)y;
+}
+default:
+fprintf(stderr,
+Unrecognized bitness %d or condition %d in
+do_constant_folding_cond.\n, op_bits(op), c);
+tcg_abort();
+}
You probably don't want the default here, but the statements after
the outer switch, and with proper breaks between the two cases.
Otherwise the error doesn't do what you wanted it to do.
Good catch, i'll fix that in version 2.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH 0/8] Improve TCG optimizer
On Thu, Sep 06, 2012 at 09:43:08AM -0700, Richard Henderson wrote: On 09/06/2012 08:00 AM, Aurelien Jarno wrote: This patch series improves the TCG optimizer, based on patterns found while executing various guest. The brcond ad setcond constant folding are useful especially useful when they are used to avoid some argument values (e.g. division by 0), and thus can be optimized when this argument is a constant. This bring around 0.5% improvement on openssl like benchmarks. Aurelien Jarno (8): tcg: improve profiler tcg/optimize: split expression simplification tcg/optimize: simplify or/xor r, a, 0 cases tcg/optimize: simplify and r, a, 0 cases tcg/optimize: simplify shift/rot r, 0, a = movi r, 0 cases tcg/optimize: swap brcond/setcond arguments when possible tcg/optimize: add constant folding for setcond tcg/optimize: add constant folding for brcond Patches 1-6,8: Reviewed-by: Richard Henderson r...@twiddle.net Patch 7 contains a trivial error. With that fixed it could also bear my Reviewed-by mark. Thanks for the review. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH RFC] remove QEMUOptionParameter
Am 07.09.2012 10:42, schrieb Markus Armbruster:
@@ -1628,51 +1625,6 @@ static int qcow2_load_vmstate(BlockDriverState *bs,
uint8_t *buf,
return ret;
}
-static QEMUOptionParameter qcow2_create_options[] = {
-{
-.name = BLOCK_OPT_SIZE,
-.type = OPT_SIZE,
-.help = Virtual disk size
-},
-{
-.name = BLOCK_OPT_COMPAT_LEVEL,
-.type = OPT_STRING,
-.help = Compatibility level (0.10 or 1.1)
-},
-{
-.name = BLOCK_OPT_BACKING_FILE,
-.type = OPT_STRING,
-.help = File name of a base image
-},
-{
-.name = BLOCK_OPT_BACKING_FMT,
-.type = OPT_STRING,
-.help = Image format of the base image
-},
-{
-.name = BLOCK_OPT_ENCRYPT,
-.type = OPT_FLAG,
-.help = Encrypt the image
-},
-{
-.name = BLOCK_OPT_CLUSTER_SIZE,
-.type = OPT_SIZE,
-.help = qcow2 cluster size,
-.value = { .n = DEFAULT_CLUSTER_SIZE },
-},
-{
-.name = BLOCK_OPT_PREALLOC,
-.type = OPT_STRING,
-.help = Preallocation mode (allowed values: off, metadata)
-},
-{
-.name = BLOCK_OPT_LAZY_REFCOUNTS,
-.type = OPT_FLAG,
-.help = Postpone refcount updates,
-},
-{ NULL }
-};
-
Replacement moves to qemu-config.c. Not sure that's an improvement, but
it's how QemuOpts generally work. For an exception, see QemuOptsList
use in blkdebug.c.
Yes, please follow the example of blkdebug.c. There's no reason to have
these QemuOptsLists global as they don't make sense in a qemu config file.
Another important reason is that in order to add or remove a file
format, you don't have to change anything outside the image format
implementation, you just link an additional object file. Moving part of
it into qemu-config.c hurts the modularity.
Kevin
[Qemu-devel] [PULL for usb-next]: Add support for live-migration to usb-redir
Hi Gerd, I'm very happy to present to you a pull-request for usb-redir live-migration support. I've tested this combined with Spice seamless migration, and it can successful: 1) migrate a vm while running dd if=/dev/zero of=/dev/sdb1 bs=32K inside the guest with sdb being a redirect USB-2 mass storage device. 2) migrate a vm while running camorama inside the vm showing a 720p video from a redirected USB-2 webcam at 30 fps! Note this is based on usb-next rather then master / usb.62, since one of my patches would otherwise conflict with your recent ehci changes. The following changes since commit a44fd2e0c66b2276f586948702e5ebc7136fdb73: usb-host: allow emulated (non-async) control requests without USBPacket (2012-09-06 12:03:41 +0200) are available in the git repository at: git://people.freedesktop.org/~jwrdegoede/qemu usb-for-gerd for you to fetch changes up to 5f5f0f1eaa29ec1cb07fc906acf917d5648b3bcf: usb-redir: Add chardev open / close debug logging (2012-09-07 13:44:49 +0200) Hans de Goede (9): ehci: Don't set seen to 0 when removing unseen queue-heads ehci: Walk async schedule before and after migration ehci: Don't process too much frames in 1 timer tick usb: Migrate over device speed and speedmask usb-redir: Change cancelled packet code into a generic packet-id queue usb-redir: Add an already_in_flight packet-id queue usb-redir: Store max_packet_size in endp_data usb-redir: Add support for migration usb-redir: Add chardev open / close debug logging hw/usb.h | 4 +- hw/usb/bus.c | 2 + hw/usb/hcd-ehci.c | 61 ++- hw/usb/redirect.c | 482 ++ 4 files changed, 508 insertions(+), 41 deletions(-) Thanks Regards, Hans
Re: [Qemu-devel] [PATCH 0/8] Improve TCG optimizer
On 6 September 2012 16:00, Aurelien Jarno aurel...@aurel32.net wrote: This patch series improves the TCG optimizer, based on patterns found while executing various guest. The brcond ad setcond constant folding are useful especially useful when they are used to avoid some argument values (e.g. division by 0), and thus can be optimized when this argument is a constant. This bring around 0.5% improvement on openssl like benchmarks. This didn't overall seem to make much difference on my popular embedded benchmark setup. However I am rapidly losing confidence in the benchmark since from run to run individual tests can have results which vary by a factor of two, which is such high variation it's almost impossible to say whether a change has had an overall +1% or -1% effect. Hohum. -- PMM
[Qemu-devel] qemu 1.2 : lsi controller + scsi-block don't boot.
Hi, I'm trying to boot scsi-block device with lsi controller, and it doesn't boot. (don't find devices). lsi + scsi-block : don't boot lsi + scsi-hd : boot virtio-scsi + scsi-block : boot Regards, Alexandre Derumier
Re: [Qemu-devel] [PATCH v2] ahci: properly reset PxCMD on HBA reset
Am 05.09.2012 05:28, schrieb Alexander Graf: On 04.09.2012, at 16:08, Jason Baron wrote: While testing q35, I found that windows 7 (specifically, windows 7 ultimate with sp1 x64), wouldn't install because it can't find the cdrom or disk drive. The failure message is: 'A required cd/dvd device driver is missing. If you have a driver floppy disk, CD, DVD, or USB flash drive, please insert it now.' This can also be reproduced on piix by adding an ahci controller, and observing that windows 7 does not see any devices behind it. The problem is that when windows issues a HBA reset, qemu does not reset the individual ports' PxCMD register. Windows 7 then reads back the PxCMD register and presumably assumes that the ahci controller has already been initialized. Windows then never sets up the PxIE register to enable interrupts, and thus it never gets irqs back when it sends ata device inquiry commands. This change brings qemu into ahci 1.3 specification compliance. Section 10.4.3 HBA Reset: When GHC.HR is set to '1', GHC.AE, GHC.IE, the IS register, and all port register fields (except PxFB/PxFBU/PxCLB/PxCLBU) that are not HwInit in the HBA's register memory space are reset. I've also re-tested Fedora 16 and 17 to verify that they continue to work with this change. Signed-off-by: Jason Baron jba...@redhat.com Awesome, a lot cleaner now :). Acked-by: Alexander Graf ag...@suse.de Thanks, applied to block-next. Kevin
Re: [Qemu-devel] [PATCH 0/8] Improve TCG optimizer
On Fri, Sep 07, 2012 at 01:34:10PM +0100, Peter Maydell wrote: On 6 September 2012 16:00, Aurelien Jarno aurel...@aurel32.net wrote: This patch series improves the TCG optimizer, based on patterns found while executing various guest. The brcond ad setcond constant folding are useful especially useful when they are used to avoid some argument values (e.g. division by 0), and thus can be optimized when this argument is a constant. This bring around 0.5% improvement on openssl like benchmarks. This didn't overall seem to make much difference on my popular embedded benchmark setup. However I am rapidly losing confidence in the benchmark since from run to run individual tests can have results which vary by a factor of two, which is such high variation it's almost impossible to say whether a change has had an overall +1% or -1% effect. Hohum. I am usually doing the tests by setting the CPU performance to performance and by pinning QEMU to a given CPU, on a machine without or with very few other tasks. This improve the stability of the results. Unfortunately it's not easy to do that on a laptop, especially when running on battery. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH] smbios: convert to use QemuOpts
Markus Armbruster arm...@redhat.com writes:
Hello, this is your friendly remote compile service.
dirty tree + git-send-email == FAIL
Sorry for the noise. At any rate, I've abandoned this patch and picked
up Paolo's tree. I'll post the full series this afternoon. I'm still
playing around with some additional cleanup.
https://github.com/aliguori/qemu/tree/qemuopts.2
Regards,
Anthony Liguori
Anthony Liguori aligu...@us.ibm.com writes:
Signed-off-by: Anthony Liguori aligu...@us.ibm.com
---
arch_init.c | 10 +-
arch_init.h |2 +-
hw/smbios.c | 53 +++--
hw/smbios.h |2 +-
qemu-config.c | 50 ++
vl.c |4 +++-
6 files changed, 91 insertions(+), 30 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 5a1173e..e43ace9 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -1033,13 +1033,13 @@ void do_acpitable_option(const char *optarg)
#endif
}
-void do_smbios_option(const char *optarg)
+int do_smbios_option(QemuOpts *opts, void *opaque)
{
#ifdef TARGET_I386
-if (smbios_entry_add(optarg) 0) {
-fprintf(stderr, Wrong smbios provided\n);
-exit(1);
-}
+return smbios_entry_add(opts);
+#else
+fprintf(stderr, -smbios option not supported on this platform\n);
+return -ENOSYS;
#endif
}
diff --git a/arch_init.h b/arch_init.h
index d9c572a..2dce578 100644
--- a/arch_init.h
+++ b/arch_init.h
@@ -26,7 +26,7 @@ extern const uint32_t arch_type;
void select_soundhw(const char *optarg);
void do_acpitable_option(const char *optarg);
-void do_smbios_option(const char *optarg);
+int do_smbios_option(QemuOpts *opts, void *opaque);
void cpudef_init(void);
int audio_available(void);
void audio_init(ISABus *isa_bus, PCIBus *pci_bus);
diff --git a/hw/smbios.c b/hw/smbios.c
index c57237d..095bccc 100644
--- a/hw/smbios.c
+++ b/hw/smbios.c
@@ -124,21 +124,24 @@ void smbios_add_field(int type, int offset, int len,
void *data)
cpu_to_le16(le16_to_cpu(*(uint16_t *)smbios_entries) + 1);
}
-static void smbios_build_type_0_fields(const char *t)
+static void smbios_build_type_0_fields(QemuOpts *opts)
{
char buf[1024];
-if (get_param_value(buf, sizeof(buf), vendor, t))
+if ((buf = qemu_opt_get(opts, vendor))) {
error: incompatible types when assigning to type ‘char[1024]’ from type
‘const char *’
smbios_add_field(0, offsetof(struct smbios_type_0, vendor_str),
strlen(buf) + 1, buf);
-if (get_param_value(buf, sizeof(buf), version, t))
+}
+if ((buf = qemu_opt_get(opts, version))) {
smbios_add_field(0, offsetof(struct smbios_type_0,
bios_version_str),
strlen(buf) + 1, buf);
-if (get_param_value(buf, sizeof(buf), date, t))
+}
+if ((buf = qemu_opt_get(opts, date))) {
smbios_add_field(0, offsetof(struct smbios_type_0,
bios_release_date_str),
strlen(buf) + 1, buf);
-if (get_param_value(buf, sizeof(buf), release, t)) {
+}
+if ((buf = qemu_opt_get(opts, release))) {
int major, minor;
sscanf(buf, %d.%d, major, minor);
smbios_add_field(0, offsetof(struct smbios_type_0,
@@ -148,41 +151,47 @@ static void smbios_build_type_0_fields(const char *t)
}
}
-static void smbios_build_type_1_fields(const char *t)
+static void smbios_build_type_1_fields(QemuOpts *opts)
{
-char buf[1024];
+const char *buf;
-if (get_param_value(buf, sizeof(buf), manufacturer, t))
+if ((buf = qemu_opt_get(opts, manufacturer))) {
warning: passing argument 4 of ‘smbios_add_field’ discards ‘const’ qualifier
from pointer target type [enabled by default]
smbios_add_field(1, offsetof(struct smbios_type_1,
manufacturer_str),
strlen(buf) + 1, buf);
-if (get_param_value(buf, sizeof(buf), product, t))
+}
+if ((buf = qemu_opt_get(opts, product))) {
smbios_add_field(1, offsetof(struct smbios_type_1,
product_name_str),
strlen(buf) + 1, buf);
-if (get_param_value(buf, sizeof(buf), version, t))
+}
+if ((buf = qemu_opt_get(opts, version))) {
smbios_add_field(1, offsetof(struct smbios_type_1, version_str),
strlen(buf) + 1, buf);
-if (get_param_value(buf, sizeof(buf), serial, t))
+}
+if ((buf = qemu_opt_get(opts, serial))) {
smbios_add_field(1, offsetof(struct smbios_type_1,
serial_number_str),
strlen(buf) + 1, buf);
-if (get_param_value(buf, sizeof(buf), uuid, t)) {
+}
+if ((buf = qemu_opt_get(opts, uuid))) {
if (qemu_uuid_parse(buf, qemu_uuid) != 0) {
fprintf(stderr, Invalid SMBIOS UUID string\n);
Re: [Qemu-devel] [PATCH v3] block: output more error messages if failed to create temporary snapshot
Am 05.09.2012 15:24, schrieb riegama...@gmail.com:
From: Dunrong Huang riegama...@gmail.com
If we failed to create temporary snapshot, the error message did not match
with the error, for example:
$ TMPDIR=/tmp/bad_path qemu-system-x86_64 -enable-kvm debian.qcow2 -snapshot
qemu-system-x86_64: -enable-kvm: could not open disk image
/home/mathslinux/Images/debian.qcow2: No such file or directory
Indeed, the file which cant be created is /tmp/bad_path/vl.xx, not
debian.qcow2. so the error message makes users feel confused.
Signed-off-by: Dunrong Huang riegama...@gmail.com
---
v1 - v2:
Output error message only if fd 0
v2 - v3:
Output error message in the caller of get_tmp_filename()
block.c | 2 ++
1 个文件被修改，插入 2 行(+)
diff --git a/block.c b/block.c
index 470bdcc..074987e 100644
--- a/block.c
+++ b/block.c
@@ -764,6 +764,8 @@ int bdrv_open(BlockDriverState *bs, const char *filename,
int flags,
ret = get_tmp_filename(tmp_filename, sizeof(tmp_filename));
if (ret 0) {
+fprintf(stderr, Could not create temporary snapshot %s: %s\n,
+tmp_filename, strerror(errno));
This should be strerror(-ret). Also consider using error_report()
instead of fprintf().
return ret;
}
Kevin
Re: [Qemu-devel] [PATCH V8 0/2] Add JSON output to qemu-img info
Am 05.09.2012 13:09, schrieb Benoît Canet: This patchset add a JSON output mode to the qemu-img info command. It's a rewrite from scratch of the original patchset by Wenchao Xia following Anthony Liguori advices on JSON formating. the --output=(json|human) option is now mandatory on the command line. Benoît Canet (3): qapi: Add SnapshotInfo. qapi: Add ImageInfo. qemu-img: Add json output option to the info command. Thanks, applied both to block-next. Kevin
[Qemu-devel] [PATCH v2 2/9] tcg/optimize: split expression simplification
Split expression simplification in multiple parts so that a given op
can appear multiple times. This patch should not change anything.
Reviewed-by: Richard Henderson r...@twiddle.net
Signed-off-by: Aurelien Jarno aurel...@aurel32.net
---
tcg/optimize.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/tcg/optimize.c b/tcg/optimize.c
index 9c65474..63f970d 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -322,7 +322,7 @@ static TCGArg *tcg_constant_folding(TCGContext *s, uint16_t
*tcg_opc_ptr,
break;
}
-/* Simplify expression if possible. */
+/* Simplify expression for op r, a, 0 = mov r, a cases */
switch (op) {
CASE_OP_32_64(add):
CASE_OP_32_64(sub):
@@ -352,6 +352,12 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
uint16_t *tcg_opc_ptr,
continue;
}
break;
+default:
+break;
+}
+
+/* Simplify expression for op r, a, 0 = movi r, 0 cases */
+switch (op) {
CASE_OP_32_64(mul):
if ((temps[args[2]].state == TCG_TEMP_CONST
temps[args[2]].val == 0)) {
@@ -362,6 +368,12 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
uint16_t *tcg_opc_ptr,
continue;
}
break;
+default:
+break;
+}
+
+/* Simplify expression for op r, a, a = mov r, a cases */
+switch (op) {
CASE_OP_32_64(or):
CASE_OP_32_64(and):
if (args[1] == args[2]) {
--
1.7.10.4
[Qemu-devel] [PATCH v2 9/9] tcg/optimize: fix if/else/break coding style
optimizer.c contains some cases were the break is appearing in both the
if and the else parts. Fix that by moving it to the outer part. Also
move some common code there.
Signed-off-by: Aurelien Jarno aurel...@aurel32.net
---
tcg/optimize.c | 34 +++---
1 file changed, 11 insertions(+), 23 deletions(-)
diff --git a/tcg/optimize.c b/tcg/optimize.c
index 156e8d9..fba0ed9 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -441,15 +441,14 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
uint16_t *tcg_opc_ptr,
if ((temps[args[0]].state == TCG_TEMP_COPY
temps[args[0]].val == args[1])
|| args[0] == args[1]) {
-args += 3;
gen_opc_buf[op_index] = INDEX_op_nop;
} else {
gen_opc_buf[op_index] = op_to_mov(op);
tcg_opt_gen_mov(s, gen_args, args[0], args[1],
nb_temps, nb_globals);
gen_args += 2;
-args += 3;
}
+args += 3;
continue;
}
break;
@@ -480,15 +479,14 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
uint16_t *tcg_opc_ptr,
CASE_OP_32_64(and):
if (args[1] == args[2]) {
if (args[1] == args[0]) {
-args += 3;
gen_opc_buf[op_index] = INDEX_op_nop;
} else {
gen_opc_buf[op_index] = op_to_mov(op);
tcg_opt_gen_mov(s, gen_args, args[0], args[1], nb_temps,
nb_globals);
gen_args += 2;
-args += 3;
}
+args += 3;
continue;
}
break;
@@ -538,17 +536,14 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
uint16_t *tcg_opc_ptr,
gen_opc_buf[op_index] = op_to_movi(op);
tmp = do_constant_folding(op, temps[args[1]].val, 0);
tcg_opt_gen_movi(gen_args, args[0], tmp, nb_temps, nb_globals);
-gen_args += 2;
-args += 2;
-break;
} else {
reset_temp(args[0], nb_temps, nb_globals);
gen_args[0] = args[0];
gen_args[1] = args[1];
-gen_args += 2;
-args += 2;
-break;
}
+gen_args += 2;
+args += 2;
+break;
CASE_OP_32_64(add):
CASE_OP_32_64(sub):
CASE_OP_32_64(mul):
@@ -572,17 +567,15 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
uint16_t *tcg_opc_ptr,
temps[args[2]].val);
tcg_opt_gen_movi(gen_args, args[0], tmp, nb_temps, nb_globals);
gen_args += 2;
-args += 3;
-break;
} else {
reset_temp(args[0], nb_temps, nb_globals);
gen_args[0] = args[0];
gen_args[1] = args[1];
gen_args[2] = args[2];
gen_args += 3;
-args += 3;
-break;
}
+args += 3;
+break;
CASE_OP_32_64(setcond):
if (temps[args[1]].state == TCG_TEMP_CONST
temps[args[2]].state == TCG_TEMP_CONST) {
@@ -591,8 +584,6 @@ static TCGArg *tcg_constant_folding(TCGContext *s, uint16_t
*tcg_opc_ptr,
temps[args[2]].val, args[3]);
tcg_opt_gen_movi(gen_args, args[0], tmp, nb_temps, nb_globals);
gen_args += 2;
-args += 4;
-break;
} else {
reset_temp(args[0], nb_temps, nb_globals);
gen_args[0] = args[0];
@@ -600,9 +591,9 @@ static TCGArg *tcg_constant_folding(TCGContext *s, uint16_t
*tcg_opc_ptr,
gen_args[2] = args[2];
gen_args[3] = args[3];
gen_args += 4;
-args += 4;
-break;
}
+args += 4;
+break;
CASE_OP_32_64(brcond):
if (temps[args[0]].state == TCG_TEMP_CONST
temps[args[1]].state == TCG_TEMP_CONST) {
@@ -612,12 +603,9 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
uint16_t *tcg_opc_ptr,
gen_opc_buf[op_index] = INDEX_op_br;
gen_args[0] = args[3];
gen_args += 1;
-args += 4;
} else {
gen_opc_buf[op_index] = INDEX_op_nop;
-args += 4;
}
-break;
} else {
memset(temps, 0, nb_temps * sizeof(struct tcg_temp_info));
reset_temp(args[0],
[Qemu-devel] [PATCH v2 5/9] tcg/optimize: simplify shift/rot r, 0, a = movi r, 0 cases
shift/rot r, 0, a is equivalent to movi r, 0.
Reviewed-by: Richard Henderson r...@twiddle.net
Signed-off-by: Aurelien Jarno aurel...@aurel32.net
---
tcg/optimize.c | 20
1 file changed, 20 insertions(+)
diff --git a/tcg/optimize.c b/tcg/optimize.c
index c12cb2b..1698ba3 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -322,6 +322,26 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
uint16_t *tcg_opc_ptr,
break;
}
+/* Simplify expressions for shift/rot r, 0, a = movi r, 0 */
+switch (op) {
+CASE_OP_32_64(shl):
+CASE_OP_32_64(shr):
+CASE_OP_32_64(sar):
+CASE_OP_32_64(rotl):
+CASE_OP_32_64(rotr):
+if (temps[args[1]].state == TCG_TEMP_CONST
+ temps[args[1]].val == 0) {
+gen_opc_buf[op_index] = op_to_movi(op);
+tcg_opt_gen_movi(gen_args, args[0], 0, nb_temps, nb_globals);
+args += 3;
+gen_args += 2;
+continue;
+}
+break;
+default:
+break;
+}
+
/* Simplify expression for op r, a, 0 = mov r, a cases */
switch (op) {
CASE_OP_32_64(add):
--
1.7.10.4
[Qemu-devel] [PATCH v2 7/9] tcg/optimize: add constant folding for setcond
Signed-off-by: Aurelien Jarno aurel...@aurel32.net
---
tcg/optimize.c | 81
1 file changed, 81 insertions(+)
diff --git a/tcg/optimize.c b/tcg/optimize.c
index 7debc8a..1cb1f36 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -267,6 +267,67 @@ static TCGArg do_constant_folding(TCGOpcode op, TCGArg x,
TCGArg y)
return res;
}
+static TCGArg do_constant_folding_cond(TCGOpcode op, TCGArg x,
+ TCGArg y, TCGCond c)
+{
+switch (op_bits(op)) {
+case 32:
+switch (c) {
+case TCG_COND_EQ:
+return (uint32_t)x == (uint32_t)y;
+case TCG_COND_NE:
+return (uint32_t)x != (uint32_t)y;
+case TCG_COND_LT:
+return (int32_t)x (int32_t)y;
+case TCG_COND_GE:
+return (int32_t)x = (int32_t)y;
+case TCG_COND_LE:
+return (int32_t)x = (int32_t)y;
+case TCG_COND_GT:
+return (int32_t)x (int32_t)y;
+case TCG_COND_LTU:
+return (uint32_t)x (uint32_t)y;
+case TCG_COND_GEU:
+return (uint32_t)x = (uint32_t)y;
+case TCG_COND_LEU:
+return (uint32_t)x = (uint32_t)y;
+case TCG_COND_GTU:
+return (uint32_t)x (uint32_t)y;
+}
+break;
+case 64:
+switch (c) {
+case TCG_COND_EQ:
+return (uint64_t)x == (uint64_t)y;
+case TCG_COND_NE:
+return (uint64_t)x != (uint64_t)y;
+case TCG_COND_LT:
+return (int64_t)x (int64_t)y;
+case TCG_COND_GE:
+return (int64_t)x = (int64_t)y;
+case TCG_COND_LE:
+return (int64_t)x = (int64_t)y;
+case TCG_COND_GT:
+return (int64_t)x (int64_t)y;
+case TCG_COND_LTU:
+return (uint64_t)x (uint64_t)y;
+case TCG_COND_GEU:
+return (uint64_t)x = (uint64_t)y;
+case TCG_COND_LEU:
+return (uint64_t)x = (uint64_t)y;
+case TCG_COND_GTU:
+return (uint64_t)x (uint64_t)y;
+}
+break;
+}
+
+fprintf(stderr,
+Unrecognized bitness %d or condition %d in
+do_constant_folding_cond.\n, op_bits(op), c);
+tcg_abort();
+}
+
+
/* Propagate constants and copies, fold constant expressions. */
static TCGArg *tcg_constant_folding(TCGContext *s, uint16_t *tcg_opc_ptr,
TCGArg *args, TCGOpDef *tcg_op_defs)
@@ -522,6 +583,26 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
uint16_t *tcg_opc_ptr,
args += 3;
break;
}
+CASE_OP_32_64(setcond):
+if (temps[args[1]].state == TCG_TEMP_CONST
+ temps[args[2]].state == TCG_TEMP_CONST) {
+gen_opc_buf[op_index] = op_to_movi(op);
+tmp = do_constant_folding_cond(op, temps[args[1]].val,
+ temps[args[2]].val, args[3]);
+tcg_opt_gen_movi(gen_args, args[0], tmp, nb_temps, nb_globals);
+gen_args += 2;
+args += 4;
+break;
+} else {
+reset_temp(args[0], nb_temps, nb_globals);
+gen_args[0] = args[0];
+gen_args[1] = args[1];
+gen_args[2] = args[2];
+gen_args[3] = args[3];
+gen_args += 4;
+args += 4;
+break;
+}
case INDEX_op_call:
nb_call_args = (args[0] 16) + (args[0] 0x);
if (!(args[nb_call_args + 1] (TCG_CALL_CONST | TCG_CALL_PURE))) {
--
1.7.10.4
[Qemu-devel] [PATCH v2 4/9] tcg/optimize: simplify and r, a, 0 cases
and r, a, 0 is equivalent to a movi r, 0.
Reviewed-by: Richard Henderson r...@twiddle.net
Signed-off-by: Aurelien Jarno aurel...@aurel32.net
---
tcg/optimize.c |1 +
1 file changed, 1 insertion(+)
diff --git a/tcg/optimize.c b/tcg/optimize.c
index 0db849e..c12cb2b 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -360,6 +360,7 @@ static TCGArg *tcg_constant_folding(TCGContext *s, uint16_t
*tcg_opc_ptr,
/* Simplify expression for op r, a, 0 = movi r, 0 cases */
switch (op) {
+CASE_OP_32_64(and):
CASE_OP_32_64(mul):
if ((temps[args[2]].state == TCG_TEMP_CONST
temps[args[2]].val == 0)) {
--
1.7.10.4
[Qemu-devel] [PATCH v2 8/9] tcg/optimize: add constant folding for brcond
Reviewed-by: Richard Henderson r...@twiddle.net
Signed-off-by: Aurelien Jarno aurel...@aurel32.net
---
tcg/optimize.c | 27 ++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/tcg/optimize.c b/tcg/optimize.c
index 1cb1f36..156e8d9 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -603,6 +603,32 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
uint16_t *tcg_opc_ptr,
args += 4;
break;
}
+CASE_OP_32_64(brcond):
+if (temps[args[0]].state == TCG_TEMP_CONST
+ temps[args[1]].state == TCG_TEMP_CONST) {
+if (do_constant_folding_cond(op, temps[args[0]].val,
+ temps[args[1]].val, args[2])) {
+memset(temps, 0, nb_temps * sizeof(struct tcg_temp_info));
+gen_opc_buf[op_index] = INDEX_op_br;
+gen_args[0] = args[3];
+gen_args += 1;
+args += 4;
+} else {
+gen_opc_buf[op_index] = INDEX_op_nop;
+args += 4;
+}
+break;
+} else {
+memset(temps, 0, nb_temps * sizeof(struct tcg_temp_info));
+reset_temp(args[0], nb_temps, nb_globals);
+gen_args[0] = args[0];
+gen_args[1] = args[1];
+gen_args[2] = args[2];
+gen_args[3] = args[3];
+gen_args += 4;
+args += 4;
+break;
+}
case INDEX_op_call:
nb_call_args = (args[0] 16) + (args[0] 0x);
if (!(args[nb_call_args + 1] (TCG_CALL_CONST | TCG_CALL_PURE))) {
@@ -624,7 +650,6 @@ static TCGArg *tcg_constant_folding(TCGContext *s, uint16_t
*tcg_opc_ptr,
case INDEX_op_set_label:
case INDEX_op_jmp:
case INDEX_op_br:
-CASE_OP_32_64(brcond):
memset(temps, 0, nb_temps * sizeof(struct tcg_temp_info));
for (i = 0; i def-nb_args; i++) {
*gen_args = *args;
--
1.7.10.4
[Qemu-devel] [PATCH v2 6/9] tcg/optimize: swap brcond/setcond arguments when possible
brcond and setcond ops are not commutative, but it's easy to compute the
new condition after swapping the arguments. Try to always put the constant
argument in second position like for commutative ops, to help backends to
generate better code.
Reviewed-by: Richard Henderson r...@twiddle.net
Signed-off-by: Aurelien Jarno aurel...@aurel32.net
---
tcg/optimize.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/tcg/optimize.c b/tcg/optimize.c
index 1698ba3..7debc8a 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -318,6 +318,24 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
uint16_t *tcg_opc_ptr,
args[2] = tmp;
}
break;
+CASE_OP_32_64(brcond):
+if (temps[args[0]].state == TCG_TEMP_CONST
+ temps[args[1]].state != TCG_TEMP_CONST) {
+tmp = args[0];
+args[0] = args[1];
+args[1] = tmp;
+args[2] = tcg_swap_cond(args[2]);
+}
+break;
+CASE_OP_32_64(setcond):
+if (temps[args[1]].state == TCG_TEMP_CONST
+ temps[args[2]].state != TCG_TEMP_CONST) {
+tmp = args[1];
+args[1] = args[2];
+args[2] = tmp;
+args[3] = tcg_swap_cond(args[3]);
+}
+break;
default:
break;
}
--
1.7.10.4
[Qemu-devel] [PATCH v2 0/9] Improve TCG optimizer
This patch series improves the TCG optimizer, based on patterns found while executing various guest. The brcond ad setcond constant folding are useful especially useful when they are used to avoid some argument values (e.g. division by 0), and thus can be optimized when this argument is a constant. This bring around 0.5% improvement on openssl like benchmarks. Modifications between V1 and V2 following feedback I got: - In the first patch, account for the liveness analysis time and optimizing pass time separately - Fixed swith/break in patch 7 to correctly throw an error - Added patch 9 to make the code more readable Other patches are unmodified. Aurelien Jarno (9): tcg: improve profiler tcg/optimize: split expression simplification tcg/optimize: simplify or/xor r, a, 0 cases tcg/optimize: simplify and r, a, 0 cases tcg/optimize: simplify shift/rot r, 0, a = movi r, 0 cases tcg/optimize: swap brcond/setcond arguments when possible tcg/optimize: add constant folding for setcond tcg/optimize: add constant folding for brcond tcg/optimize: fix if/else/break coding style tcg/optimize.c | 179 +++- tcg/tcg.c | 12 +++- tcg/tcg.h |1 + 3 files changed, 175 insertions(+), 17 deletions(-) -- 1.7.10.4
[Qemu-devel] [PATCH v2 1/9] tcg: improve profiler
Now that there are two passes of optimization (optimize.c, liveness)
there is no point of outputing the statistics of the liveness part
only. Update the code to take into account both optimizations.
Signed-off-by: Aurelien Jarno aurel...@aurel32.net
---
tcg/tcg.c | 12 +++-
tcg/tcg.h |1 +
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 8386b70..a4e7f42 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -2059,22 +2059,29 @@ static inline int tcg_gen_code_common(TCGContext *s,
uint8_t *gen_code_buf,
}
#endif
+#ifdef CONFIG_PROFILER
+s-opt_time -= profile_getclock();
+#endif
+
#ifdef USE_TCG_OPTIMIZATIONS
gen_opparam_ptr =
tcg_optimize(s, gen_opc_ptr, gen_opparam_buf, tcg_op_defs);
#endif
#ifdef CONFIG_PROFILER
+s-opt_time += profile_getclock();
s-la_time -= profile_getclock();
#endif
+
tcg_liveness_analysis(s);
+
#ifdef CONFIG_PROFILER
s-la_time += profile_getclock();
#endif
#ifdef DEBUG_DISAS
if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP_OPT))) {
-qemu_log(OP after liveness analysis:\n);
+qemu_log(OP after optimization and liveness analysis:\n);
tcg_dump_ops(s);
qemu_log(\n);
}
@@ -2241,6 +2248,9 @@ void tcg_dump_info(FILE *f, fprintf_function cpu_fprintf)
(double)s-interm_time / tot * 100.0);
cpu_fprintf(f, gen_code time %0.1f%%\n,
(double)s-code_time / tot * 100.0);
+cpu_fprintf(f, optim./code time%0.1f%%\n,
+(double)s-opt_time / (s-code_time ? s-code_time : 1)
+* 100.0);
cpu_fprintf(f, liveness/code time %0.1f%%\n,
(double)s-la_time / (s-code_time ? s-code_time : 1) *
100.0);
cpu_fprintf(f, cpu_restore count % PRId64 \n,
diff --git a/tcg/tcg.h b/tcg/tcg.h
index d710694..7a72729 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -382,6 +382,7 @@ struct TCGContext {
int64_t interm_time;
int64_t code_time;
int64_t la_time;
+int64_t opt_time;
int64_t restore_count;
int64_t restore_time;
#endif
--
1.7.10.4
[Qemu-devel] [PATCH v2] qom: Reject attempts to add a property that already exists
Reject attempts to add a property to an object if one of
that name already exists. This is always a bug in the caller;
this is merely diagnosing it gracefully rather than behaving
oddly later.
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
Changes v1-v2:
* use abort() rather than assert(0), as suggested by Paolo
* make the diagnostic message a little more helpful by
including the type name, and adding '' around names
* the patches fixing bugs which this patch makes fatal errors
have both now been committed to master, so there's no
barrier to committing it now
qom/object.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/qom/object.c b/qom/object.c
index e3e9242..3da4c0e 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -620,7 +620,18 @@ void object_property_add(Object *obj, const char *name,
const char *type,
ObjectPropertyRelease *release,
void *opaque, Error **errp)
{
-ObjectProperty *prop = g_malloc0(sizeof(*prop));
+ObjectProperty *prop;
+
+QTAILQ_FOREACH(prop, obj-properties, node) {
+if (strcmp(prop-name, name) == 0) {
+/* This is always a bug in the caller */
+fprintf(stderr, attempt to add duplicate property '%s'
+ to object (type '%s')\n, name,
object_get_typename(obj));
+abort();
+}
+}
+
+prop = g_malloc0(sizeof(*prop));
prop-name = g_strdup(name);
prop-type = g_strdup(type);
--
1.7.9.5
[Qemu-devel] [PATCH v2 3/9] tcg/optimize: simplify or/xor r, a, 0 cases
or/xor r, a, 0 is equivalent to a mov r, a.
Reviewed-by: Richard Henderson r...@twiddle.net
Signed-off-by: Aurelien Jarno aurel...@aurel32.net
---
tcg/optimize.c |2 ++
1 file changed, 2 insertions(+)
diff --git a/tcg/optimize.c b/tcg/optimize.c
index 63f970d..0db849e 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -331,6 +331,8 @@ static TCGArg *tcg_constant_folding(TCGContext *s, uint16_t
*tcg_opc_ptr,
CASE_OP_32_64(sar):
CASE_OP_32_64(rotl):
CASE_OP_32_64(rotr):
+CASE_OP_32_64(or):
+CASE_OP_32_64(xor):
if (temps[args[1]].state == TCG_TEMP_CONST) {
/* Proceed with possible constant folding. */
break;
--
1.7.10.4
Re: [Qemu-devel] [PATCH] fix entry pointer for ELF kernels loaded with -kernel option
On Wed, Sep 05, 2012 at 03:11:13PM +0200, Kevin Wolf wrote:
Am 05.09.2012 14:56, schrieb Henning Schild:
Find a hopefully proper patch attached. Take it or leave it.
Signed-off-by: Henning Schild henn...@hennsch.de
Reviewed-by: Kevin Wolf kw...@redhat.com
Aurelien, I think in the past you committed some changes in this area.
Does this look good to you and can you get it committed?
Thanks, committed.
---
hw/elf_ops.h | 11 +++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/hw/elf_ops.h b/hw/elf_ops.h
index fa65ce2..731a983 100644
--- a/hw/elf_ops.h
+++ b/hw/elf_ops.h
@@ -269,6 +269,17 @@ static int glue(load_elf, SZ)(const char *name,
int fd,
addr = ph-p_paddr;
}
+/* the entry pointer in the ELF header is a virtual
+ * address, if the text segments paddr and vaddr differ
+ * we need to adjust the entry */
+if (pentry !translate_fn
+ph-p_vaddr != ph-p_paddr
+ehdr.e_entry = ph-p_vaddr
+ehdr.e_entry ph-p_vaddr + ph-p_filesz
+ph-p_flags PF_X) {
+*pentry = ehdr.e_entry - ph-p_vaddr + ph-p_paddr;
+}
+
snprintf(label, sizeof(label), phdr #%d: %s, i, name);
rom_add_blob_fixed(label, data, mem_size, addr);
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH v2] qom: Reject attempts to add a property that already exists
Am 07.09.2012 15:55, schrieb Peter Maydell: Reject attempts to add a property to an object if one of that name already exists. This is always a bug in the caller; this is merely diagnosing it gracefully rather than behaving oddly later. Signed-off-by: Peter Maydell peter.mayd...@linaro.org Looks fine to me, Reviewed-by: Andreas Färber afaer...@suse.de /-F -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 3/7] block: raw-posix image file reopen
On 09/07/2012 06:40 AM, Kevin Wolf wrote:
Am 06.09.2012 17:34, schrieb Corey Bryant:
On 09/06/2012 05:23 AM, Kevin Wolf wrote:
Am 05.09.2012 18:43, schrieb Jeff Cody:
+}
+
+int fcntl_flags = O_APPEND | O_ASYNC | O_NONBLOCK;
+#ifdef O_NOATIME
+fcntl_flags |= O_NOATIME;
+#endif
+if ((raw_s-open_flags ~fcntl_flags) == (s-open_flags ~fcntl_flags)) {
+/* dup the original fd */
+/* TODO: use qemu fcntl wrapper */
+raw_s-fd = fcntl(s-fd, F_DUPFD_CLOEXEC, 0);
+if (raw_s-fd == -1) {
+ret = -1;
+goto error;
+}
+ret = fcntl_setfl(raw_s-fd, raw_s-open_flags);
+} else {
+raw_s-fd = qemu_open(state-bs-filename, raw_s-open_flags, 0644);
+if (raw_s-fd == -1) {
+ret = -1;
+}
Ignoring this part for now, with qemu_dup_flags() it's going to look a
bit different. In particular, I'm hoping that we don't get a second
fcntl_flags enumeration here, but can just fall back to qemu_open()
whenever qemu_dup_flags() fails.
That will require modification to qemu_dup_flags()... I believe
qemu_dup_flags() silently filters out fcntl incompatible flags.
Maybe it would be best to create a small helper function in osdep.c, that
fetches the fcntl_flags. Then qemu_dup_flags() and this function would
use the same helper to fetch fcntl_flags. The results of that would
determine if we call qemu_dup_flags() or qemu_open().
Although, I do think it makes sense to always try qemu_open() if
qemu_dup_flags() fails for some reason.
I'm curious why you can't always call qemu_open().
I believe the original reason was that qemu_open() is more likely to
fail, for example if the image file has been renamed/moved/deleted since
the first open. You could still use fcntl() on an existing file
descriptor, but reopening would fail.
Some things to consider so that fd passing doesn't break when a reopen
occurs. Mainly all the concerns revolve around how fd passing keeps
track of references to fd sets (note: adding and removing fd set
references is all done in qemu_open and qemu_close).
* When reopening, qemu_open needs to be called before qemu_close. This
will prevent the reference list for an fdset from becoming empty. If
qemu_close is called before qemu_open, the reference list can become
empty, and the fdset could be cleaned up before the qemu_open. Then
qemu_open would fail.
Will automatically be right when we properly implement transactional
semantics.
* qemu_open/qemu_close need to be used rather than open/close so that
the references for fd passing are properly accounted for.
Congratulations, you've just discovered a bug in Jeff's patches. It was
a good idea to CC you. ;-)
* I don't think you want to call qemu_dup_flags directly since it
doesn't update the reference list for fd passing. Only qemu_open and
qemu_close update the reference list.
That's a good point, too. So probably a small wrapper that just updates
the reference list in addition?
You could do that. And yes you'd have to add code to add the new dup fd
to an fdset's dup_fds list if in fact the fd that you dup'd was a member
of an fdset's dup_fds list (see how qemu_close() and qemu_open() do this).
But wouldn't it be easier to just go through qemu_close() then
qemu_open() to perform the reopen? Then you don't have to add this
extra code to account for fd passing.
--
Regards,
Corey
If we can modify qemu_dup_flags() to fail if it can't provide the right
set of flags, then I think we should do it - and I think we can. Even
for the existing cases with fd passing it shouldn't break anything, but
only add an additional safety check.
And if touching the function motivates Corey to write some fd passing
test cases so that you can't break it, even better. ;-)
:) Sorry, I do plan to do this soon. I've just been side-tracked with
some other things.
No problem, it was just such a great opportunity to remind you. ;-)
Kevin
Re: [Qemu-devel] [PATCH 02/21] target-s390x: split FPU ops
Am 07.09.2012 06:26, schrieb Alexander Graf: Quoting Richard Henderson r...@twiddle.net: On 09/06/2012 11:42 AM, Alexander Graf wrote: Richard, while at it, could you please check the s390x tcg target? Running any target on there seems to break in the TLB code for me. I did successfully run a simple linux-user test directly off blue's patch set. It exercised a bit of fp and system calls (printf). I don't have a system environment set up at the moment... Ah, I am referring to s390x host code. Running qemu-system-x86_64 on s390x is what breaks for me. If, e.g., arm works on master that might rather point to tcg/s390x/ CONFIG_PASS_AREG0 mode. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] [Bug 1044727] Re: -kernel does not work for multiboot ELF kernels
** Changed in: qemu Status: New = Fix Committed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1044727 Title: -kernel does not work for multiboot ELF kernels Status in QEMU: Fix Committed Bug description: The multiboot header of a kernel image can contain the entry point and memory segment information. If it does not the kernel should have an ELF header that describes the memory segments and contains the entry point. http://www.gnu.org/software/grub/manual/multiboot/multiboot.html #Header-layout I have such a multiboot ELF kernel that can be loaded fine with grub and grub2 but not with the qemu -kernel flag. According to the ELF spec the entry field in the ELF header should contain the virtual address of the multiboot entry code. Qemu sets up the memory regions using the paddr fields from the ELF sections and then tries to start the kernel using the virtual entry address. This will fail with qemu: fatal: Trying to execute code outside RAM or ROM I wrote a simple kernel that can be used to reproduce this bug. Get this archive http://os.inf.tu-dresden.de/~hschild/asmkernel.tar.gz It contains the simple kernel, its source, plus grub and grub2 binaries to boot the kernel in qemu. The HOWTO file contains the command lines you should be using to rebuild the kernel and boot it an bare qemu or with one of the two bootloaders. Find a patch that will fix this issue here: http://os.inf.tu-dresden.de/~hschild/qemu-fix-elf-entry.patch To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1044727/+subscriptions
[Qemu-devel] [Bug 948675] Re: QEMU is crashing when called with -vga none
** Changed in: qemu Status: Fix Committed = Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/948675 Title: QEMU is crashing when called with -vga none Status in QEMU: Fix Released Status in “qemu-kvm” package in Ubuntu: Invalid Bug description: QEMU is crashing when called with -vga none. This regression was inserted in e5ad936b0fd7dfd7fd7908be6f9f1ca88f63b96b. QEMU line: /home/fidencio/dev/bin/qemu-system-x86_64 -enable-kvm -m 1024 -kernel /home/fidencio/src/linux-2.6/arch/x86_64/boot/bzImage -append root=nfs rw nfsrootdebug console=ttyS0 ip=192.168.122.2:192.168.122.1:192.168.122.1:255.255.255.0 nfsroot=192.168.122.1:/home/fidencio/fedora14-minimal -device e1000,vlan=0 -serial stdio -net tap,script=/home/fidencio/dev/etc/qemu-ifup -vga none Backtrace: #0 0x557ac976 in is_romd (pd=2048) at /home/fidencio/src/qemu/exec.c:2110 #1 0x557ac9e3 in is_ram_rom_romd (pd=804864) at /home/fidencio/src/qemu/exec.c:2115 #2 0x557ad05a in cpu_register_physical_memory_log (section= 0x72daf6f0, readable=true, readonly=false) at /home/fidencio/src/qemu/exec.c:2587 #3 0x557e4d47 in as_memory_range_add (as=0x55c34980, fr= 0x7fffec002950) at /home/fidencio/src/qemu/memory.c:317 #4 0x557e6b49 in address_space_update_topology_pass (as= 0x55c34980, old_view=..., new_view=..., adding=true) at /home/fidencio/src/qemu/memory.c:763 #5 0x557e6c3f in address_space_update_topology (as=0x55c34980) at /home/fidencio/src/qemu/memory.c:779 #6 0x557e6d0c in memory_region_update_topology (mr=0x5646d2c0) at /home/fidencio/src/qemu/memory.c:798 #7 0x557e8e16 in memory_region_add_subregion_common (mr= 0x5646d2c0, offset=792576, subregion=0x564a6130) at /home/fidencio/src/qemu/memory.c:1352 #8 0x557e8ede in memory_region_add_subregion_overlap (mr= 0x5646d2c0, offset=792576, subregion=0x564a6130, priority=1000) at /home/fidencio/src/qemu/memory.c:1372 #9 0x557dfebe in vapic_map_rom_writable (s=0x564a3d30) at /home/fidencio/src/qemu/hw/kvmvapic.c:587 #10 0x557dff06 in vapic_prepare (s=0x564a3d30) at /home/fidencio/src/qemu/hw/kvmvapic.c:593 #11 0x557e0001 in vapic_write (opaque=0x564a3d30, addr=0, data=32, size=2) at /home/fidencio/src/qemu/hw/kvmvapic.c:632 #12 0x557e4b84 in memory_region_write_accessor (opaque=0x564a6068, addr=0, value=0x72dafb00, size=2, shift=0, mask=65535) at /home/fidencio/src/qemu/memory.c:274 #13 0x557e4c66 in access_with_adjusted_size (addr=0, value= 0x72dafb00, size=2, access_size_min=1, access_size_max=4, access= 0x557e4b0c memory_region_write_accessor, opaque=0x564a6068) at /home/fidencio/src/qemu/memory.c:304 #14 0x557e5412 in memory_region_iorange_write (iorange=0x564a60b0, offset=0, width=2, data=32) at /home/fidencio/src/qemu/memory.c:440 #15 0x557d0ab6 in ioport_writew_thunk (opaque=0x564a60b0, addr= 126, data=32) at /home/fidencio/src/qemu/ioport.c:218 #16 0x557d0411 in ioport_write (index=1, address=126, data=32) at /home/fidencio/src/qemu/ioport.c:82 #17 0x557d0f3d in cpu_outw (addr=126, val=32) at /home/fidencio/src/qemu/ioport.c:281 #18 0x557d537c in kvm_handle_io (port=126, data=0x77ff4000, direction=1, size=2, count=1) at /home/fidencio/src/qemu/kvm-all.c:1015 #19 0x557d594a in kvm_cpu_exec (env=0x56492f20) at /home/fidencio/src/qemu/kvm-all.c:1160 #20 0x557a5d69 in qemu_kvm_cpu_thread_fn (arg=0x56492f20) at /home/fidencio/src/qemu/cpus.c:733 #21 0x7651dd90 in start_thread (arg=0x72db0700) at pthread_create.c:309 #22 0x7578148d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:115 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/948675/+subscriptions
[Qemu-devel] [Bug 584121] Re: migration always fails on 32bit qemu-kvm-0.12+ (sigsegv)
** Changed in: qemu Status: Fix Committed = Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/584121 Title: migration always fails on 32bit qemu-kvm-0.12+ (sigsegv) Status in QEMU: Fix Released Bug description: On a 32bit host (or when running 32bit userspace on 64bit host), migration always fails with a crash of qemu-kvm process. See http://marc.info/?l=kvmm=127351472231666 for more information. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/584121/+subscriptions
Re: [Qemu-devel] [PATCH 18/21] target-cris: switch to AREG0 free mode
On Fri, Sep 07, 2012 at 04:18:41PM +0200, Aurelien Jarno wrote: On Sun, Sep 02, 2012 at 05:33:47PM +, Blue Swirl wrote: Add an explicit CPUState parameter instead of relying on AREG0 and switch to AREG0 free mode. Signed-off-by: Blue Swirl blauwir...@gmail.com --- configure |2 +- target-cris/Makefile.objs |2 - target-cris/helper.c|4 +- target-cris/helper.h| 34 target-cris/op_helper.c | 89 +-- target-cris/translate.c | 50 --- target-cris/translate_v10.c | 22 +- 7 files changed, 101 insertions(+), 102 deletions(-) diff --git a/configure b/configure index e464d2f..d760e07 100755 --- a/configure +++ b/configure @@ -3829,7 +3829,7 @@ symlink $source_path/Makefile.target $target_dir/Makefile case $target_arch2 in - alpha | arm* | i386 | lm32 | m68k | microblaze* | or32 | s390x | sparc* | unicore32 | x86_64 | xtensa* | ppc*) + alpha | arm* | cris | i386 | lm32 | m68k | microblaze* | or32 | s390x | sparc* | unicore32 | x86_64 | xtensa* | ppc*) echo CONFIG_TCG_PASS_AREG0=y $config_target_mak ;; esac diff --git a/target-cris/Makefile.objs b/target-cris/Makefile.objs index 4b09e8c..afb87bc 100644 --- a/target-cris/Makefile.objs +++ b/target-cris/Makefile.objs @@ -1,4 +1,2 @@ obj-y += translate.o op_helper.o helper.o cpu.o obj-$(CONFIG_SOFTMMU) += mmu.o machine.o - -$(obj)/op_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS) diff --git a/target-cris/helper.c b/target-cris/helper.c index bfbc29e..1bdb7e2 100644 --- a/target-cris/helper.c +++ b/target-cris/helper.c @@ -151,7 +151,7 @@ static void do_interruptv10(CPUCRISState *env) } /* Now that we are in kernel mode, load the handlers address. */ - env-pc = ldl_code(env-pregs[PR_EBP] + ex_vec * 4); +env-pc = cpu_ldl_code(env, env-pregs[PR_EBP] + ex_vec * 4); env-locked_irq = 1; env-pregs[PR_CCS] |= F_FLAG_V10; /* set F. */ @@ -233,7 +233,7 @@ void do_interrupt(CPUCRISState *env) /* Now that we are in kernel mode, load the handlers address. This load may not fault, real hw leaves that behaviour as undefined. */ - env-pc = ldl_code(env-pregs[PR_EBP] + ex_vec * 4); +env-pc = cpu_ldl_code(env, env-pregs[PR_EBP] + ex_vec * 4); /* Clear the excption_index to avoid spurios hw_aborts for recursive bus faults. */ diff --git a/target-cris/helper.h b/target-cris/helper.h index 093063a..b575524 100644 --- a/target-cris/helper.h +++ b/target-cris/helper.h @@ -1,26 +1,26 @@ #include def-helper.h -DEF_HELPER_1(raise_exception, void, i32) -DEF_HELPER_1(tlb_flush_pid, void, i32) -DEF_HELPER_1(spc_write, void, i32) +DEF_HELPER_2(raise_exception, void, env, i32) +DEF_HELPER_2(tlb_flush_pid, void, env, i32) +DEF_HELPER_2(spc_write, void, env, i32) DEF_HELPER_3(dump, void, i32, i32, i32) -DEF_HELPER_0(rfe, void); -DEF_HELPER_0(rfn, void); +DEF_HELPER_1(rfe, void, env); +DEF_HELPER_1(rfn, void, env); -DEF_HELPER_2(movl_sreg_reg, void, i32, i32) -DEF_HELPER_2(movl_reg_sreg, void, i32, i32) +DEF_HELPER_3(movl_sreg_reg, void, env, i32, i32) +DEF_HELPER_3(movl_reg_sreg, void, env, i32, i32) DEF_HELPER_FLAGS_1(lz, TCG_CALL_PURE, i32, i32); -DEF_HELPER_FLAGS_3(btst, TCG_CALL_PURE, i32, i32, i32, i32); +DEF_HELPER_FLAGS_4(btst, TCG_CALL_PURE, i32, env, i32, i32, i32); -DEF_HELPER_FLAGS_3(evaluate_flags_muls, TCG_CALL_PURE, i32, i32, i32, i32) -DEF_HELPER_FLAGS_3(evaluate_flags_mulu, TCG_CALL_PURE, i32, i32, i32, i32) -DEF_HELPER_FLAGS_4(evaluate_flags_mcp, TCG_CALL_PURE, i32, i32, i32, i32, i32) -DEF_HELPER_FLAGS_4(evaluate_flags_alu_4, TCG_CALL_PURE, i32, i32, i32, i32, i32) -DEF_HELPER_FLAGS_4(evaluate_flags_sub_4, TCG_CALL_PURE, i32, i32, i32, i32, i32) -DEF_HELPER_FLAGS_2(evaluate_flags_move_4, TCG_CALL_PURE, i32, i32, i32) -DEF_HELPER_FLAGS_2(evaluate_flags_move_2, TCG_CALL_PURE, i32, i32, i32) -DEF_HELPER_0(evaluate_flags, void) -DEF_HELPER_0(top_evaluate_flags, void) +DEF_HELPER_FLAGS_4(evaluate_flags_muls, TCG_CALL_PURE, i32, env, i32, i32, i32) +DEF_HELPER_FLAGS_4(evaluate_flags_mulu, TCG_CALL_PURE, i32, env, i32, i32, i32) +DEF_HELPER_FLAGS_5(evaluate_flags_mcp, TCG_CALL_PURE, i32, env, i32, i32, i32, i32) +DEF_HELPER_FLAGS_5(evaluate_flags_alu_4, TCG_CALL_PURE, i32, env, i32, i32, i32, i32) +DEF_HELPER_FLAGS_5(evaluate_flags_sub_4, TCG_CALL_PURE, i32, env, i32, i32, i32, i32) +DEF_HELPER_FLAGS_3(evaluate_flags_move_4, TCG_CALL_PURE, i32, env, i32, i32) +DEF_HELPER_FLAGS_3(evaluate_flags_move_2, TCG_CALL_PURE, i32, env, i32, i32) +DEF_HELPER_1(evaluate_flags, void, env) +DEF_HELPER_1(top_evaluate_flags, void, env) #include def-helper.h diff --git a/target-cris/op_helper.c
Re: [Qemu-devel] [PATCH] configure: fix seccomp check
Am 06.09.2012 22:40, schrieb Yann E. MORIN: Currently, if libseccomp is missing but the user explicitly requested seccomp support using --enable-seccomp, configure silently ignores the situation and disables seccomp support. This is unlike all other tests that explicitly fail in such situation. Fix that. Signed-off-by: Yann E. MORIN yann.morin.1...@free.fr Reviewed-by: Andreas Färber afaer...@suse.de Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] [Bug 1022331] Re: -cpu ? causes confusion when directory has 1-character length filenames
** Changed in: qemu Status: Fix Committed = Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1022331 Title: -cpu ? causes confusion when directory has 1-character length filenames Status in QEMU: Fix Released Bug description: When user is in a directory with 1-character long filenames, parameter -cpu ? causes shell to expand ? into filename, which can cause a very confused user. One solution would be to replace/add alias to -cpu ?, for example -cpulist To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1022331/+subscriptions
Re: [Qemu-devel] [PATCH] musicpal: Fix flash mapping
On 7 September 2012 00:03, Jan Kiszka jan.kis...@web.de wrote: The old arithmetic assumed 32 physical address bits which is no longer true for ARM since 3cc0cd61f4. Signed-off-by: Jan Kiszka jan.kis...@web.de --- hw/musicpal.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/musicpal.c b/hw/musicpal.c index ad725b5..10c2c16 100644 --- a/hw/musicpal.c +++ b/hw/musicpal.c @@ -1583,7 +1583,7 @@ static void musicpal_init(ram_addr_t ram_size, * image is smaller than 32 MB. */ #ifdef TARGET_WORDS_BIGENDIAN -pflash_cfi02_register(0-MP_FLASH_SIZE_MAX, NULL, +pflash_cfi02_register(0x1-MP_FLASH_SIZE_MAX, NULL, I don't think this will compile on a 32 bit system, will it? You probably want an ULL suffix. -- PMM
Re: [Qemu-devel] [PATCH 02/21] target-s390x: split FPU ops
On Fri, Sep 07, 2012 at 04:30:51PM +0200, Andreas Färber wrote: Am 07.09.2012 06:26, schrieb Alexander Graf: Quoting Richard Henderson r...@twiddle.net: On 09/06/2012 11:42 AM, Alexander Graf wrote: Richard, while at it, could you please check the s390x tcg target? Running any target on there seems to break in the TLB code for me. I did successfully run a simple linux-user test directly off blue's patch set. It exercised a bit of fp and system calls (printf). I don't have a system environment set up at the moment... Ah, I am referring to s390x host code. Running qemu-system-x86_64 on s390x is what breaks for me. If, e.g., arm works on master that might rather point to tcg/s390x/ CONFIG_PASS_AREG0 mode. This is likely the case. The register shift code in CONFIG_PASS_AREG0 case uses 3 registers for stores and 4 for loads. It should be the reverse. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
[Qemu-devel] [Bug 957622] Re: kvm -kernel with grub multiboot kernel dumps core or exits
** Changed in: qemu Status: Fix Committed = Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/957622 Title: kvm -kernel with grub multiboot kernel dumps core or exits Status in QEMU: Fix Released Status in “qemu-kvm” package in Ubuntu: Fix Released Bug description: I attempted to use kvm -kernel with a grub multiboot image, specifically grub-maverick-20100729.img at [1]. That file was built using [2] $ url=http://bazaar.launchpad.net/~ubuntu-on-ec2/vmbuilder/automated-ec2-builds/download/head:/grubmaverick20100729-20100729071944-bevge631maio9jpl-2/grub-maverick-20100729.img; $ wget $url -O grub-maverick-20100729.img $ qemu-img create -f qcow2 disk.img 1G $ kvm -curses -kernel grub-maverick-20100729.img -drive file=disk.img,if=virtio This process works fine on oneiric and you will see a curses interface, and some output of grub looking for a image to boot. On my laptop (with kvm support), I saw: $ kvm -curses -kernel grub-maverick-20100729.img -drive file=disk.img,if=virtio; fread() failed $ echo $? 1 On a kvm guest (via openstack instance), it crashed differently: $ kvm -curses -kernel grub-maverick-20100729.img -drive file=disk.img,if=virtio Could not access KVM kernel module: No such file or directory failed to initialize KVM: No such file or directory Back to tcg accelerator. GLib-ERROR **: /build/buildd/glib2.0-2.31.20/./glib/gmem.c:165: failed to allocate 4293918720 bytes Trace/breakpoint trap (core dumped) Just for a test, I tried loading kvm-amd, got nested kvm virtualization, but the instance fails the same way. -- [1] http://bazaar.launchpad.net/~ubuntu-on-ec2/vmbuilder/automated-ec2-builds/files/head:/loaders/ [2] http://bazaar.launchpad.net/~ubuntu-on-ec2/vmbuilder/automated-ec2-builds/view/head:/mk-image-mb-loader ProblemType: Bug DistroRelease: Ubuntu 12.04 Package: kvm (not installed) ProcVersionSignature: User Name 3.2.0-18.29-virtual 3.2.9 Uname: Linux 3.2.0-18-virtual x86_64 ApportVersion: 1.94.1-0ubuntu2 Architecture: amd64 CurrentDmesg: [27230.320857] init: qemu-kvm pre-start process (8659) terminated with status 1 [27230.361904] init: qemu-kvm post-stop process (8664) terminated with status 1 [27249.426836] kvm[9021] trap int3 ip:7f44c2bbc13b sp:7fff447e1120 error:0 [27263.380598] kvm[9283] trap int3 ip:7f3fba9f713b sp:7fff8b55d1a0 error:0 Date: Sat Mar 17 01:48:13 2012 Ec2AMI: ami- Ec2AMIManifest: FIXME Ec2AvailabilityZone: nova Ec2InstanceType: m1.small Ec2Kernel: unavailable Ec2Ramdisk: unavailable KvmCmdLine: Error: command ['ps', '-C', 'kvm', '-F'] failed with exit code 1: UIDPID PPID CSZ RSS PSR STIME TTY TIME CMD Lsusb: Bus 001 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub Bus 001 Device 002: ID 0627:0001 Adomax Technology Co., Ltd MachineType: Bochs Bochs ProcEnviron: TERM=screen PATH=(custom, user) LANG=en_US.UTF-8 SHELL=/bin/bash ProcKernelCmdLine: BOOT_IMAGE=/boot/vmlinuz-3.2.0-18-virtual root=LABEL=cloudimg-rootfs ro console=ttyS0 ProcModules: acpiphp 24231 0 - Live 0x floppy 70365 0 - Live 0x psmouse 87603 0 - Live 0x serio_raw 13211 0 - Live 0x virtio_balloon 13108 0 - Live 0x SourcePackage: qemu-kvm UpgradeStatus: No upgrade log present (probably fresh install) dmi.bios.date: 01/01/2007 dmi.bios.vendor: Bochs dmi.bios.version: Bochs dmi.chassis.type: 1 dmi.chassis.vendor: Bochs dmi.modalias: dmi:bvnBochs:bvrBochs:bd01/01/2007:svnBochs:pnBochs:pvr:cvnBochs:ct1:cvr: dmi.product.name: Bochs dmi.sys.vendor: Bochs To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/957622/+subscriptions
[Qemu-devel] [Bug 962880] Re: having a tr_TR.UTF-8 locale creates problems during compile
** Changed in: qemu Status: Fix Committed = Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/962880 Title: having a tr_TR.UTF-8 locale creates problems during compile Status in QEMU: Fix Released Bug description: Default locale; /opt/test/qemu-1.0.1# locale LANG=tr_TR.UTF-8 LC_CTYPE=tr_TR.UTF-8 ... LC_IDENTIFICATION=tr_TR.UTF-8 LC_ALL= -- ./configure make . . . /opt/test/qemu-1.0.1/vl.c: In function 'main': /opt/test/qemu-1.0.1/vl.c:2248: hata: 'CONFIG_QEMU_CONFDIR' bildirilmemiş (bu işlevde ilk kullanımı) /opt/test/qemu-1.0.1/vl.c:2248: hata: (Bildirilmemiş her betimleyici görüldüğü her işlev /opt/test/qemu-1.0.1/vl.c:2248: hata: için sadece bir kez raporlanır.) /opt/test/qemu-1.0.1/vl.c:2248: hata: expected ')' before string constant /opt/test/qemu-1.0.1/vl.c:3090: hata: 'CONFIG_QEMU_DATADIR' bildirilmemiş (bu işlevde ilk kullanımı) make[1]: *** [vl.o] Hata 1 make: *** [subdir-libhw64] Hata 2 -- if we examine the config-host.h (look at the i characters) #define CONFIG_QEMU_PREFiX /usr/local #define CONFIG_QEMU_BiNDiR /usr/local/bin #define CONFIG_QEMU_LiBDiR /usr/local/lib #define CONFIG_QEMU_iNCLUDEDiR /usr/local/include #define CONFIG_QEMU_MANDiR /usr/local/share/man #define CONFIG_QEMU_DATADiR /usr/local/share/qemu #define CONFIG_QEMU_SYSCONFDiR /usr/local/etc #define CONFIG_QEMU_DOCDiR /usr/local/share/doc/qemu #define CONFIG_QEMU_CONFDiR /usr/local/etc/qemu --- changing LC_ALL and LC_LANG to POSIX (C) solves the problem. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/962880/+subscriptions
Re: [Qemu-devel] [PATCH 18/21] target-cris: switch to AREG0 free mode
On Fri, Sep 07, 2012 at 04:40:03PM +0200, Edgar E. Iglesias wrote: On Fri, Sep 07, 2012 at 04:18:41PM +0200, Aurelien Jarno wrote: On Sun, Sep 02, 2012 at 05:33:47PM +, Blue Swirl wrote: Add an explicit CPUState parameter instead of relying on AREG0 and switch to AREG0 free mode. Signed-off-by: Blue Swirl blauwir...@gmail.com --- configure |2 +- target-cris/Makefile.objs |2 - target-cris/helper.c|4 +- target-cris/helper.h| 34 target-cris/op_helper.c | 89 +-- target-cris/translate.c | 50 --- target-cris/translate_v10.c | 22 +- 7 files changed, 101 insertions(+), 102 deletions(-) diff --git a/configure b/configure index e464d2f..d760e07 100755 --- a/configure +++ b/configure @@ -3829,7 +3829,7 @@ symlink $source_path/Makefile.target $target_dir/Makefile case $target_arch2 in - alpha | arm* | i386 | lm32 | m68k | microblaze* | or32 | s390x | sparc* | unicore32 | x86_64 | xtensa* | ppc*) + alpha | arm* | cris | i386 | lm32 | m68k | microblaze* | or32 | s390x | sparc* | unicore32 | x86_64 | xtensa* | ppc*) echo CONFIG_TCG_PASS_AREG0=y $config_target_mak ;; esac diff --git a/target-cris/Makefile.objs b/target-cris/Makefile.objs index 4b09e8c..afb87bc 100644 --- a/target-cris/Makefile.objs +++ b/target-cris/Makefile.objs @@ -1,4 +1,2 @@ obj-y += translate.o op_helper.o helper.o cpu.o obj-$(CONFIG_SOFTMMU) += mmu.o machine.o - -$(obj)/op_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS) diff --git a/target-cris/helper.c b/target-cris/helper.c index bfbc29e..1bdb7e2 100644 --- a/target-cris/helper.c +++ b/target-cris/helper.c @@ -151,7 +151,7 @@ static void do_interruptv10(CPUCRISState *env) } /* Now that we are in kernel mode, load the handlers address. */ - env-pc = ldl_code(env-pregs[PR_EBP] + ex_vec * 4); +env-pc = cpu_ldl_code(env, env-pregs[PR_EBP] + ex_vec * 4); env-locked_irq = 1; env-pregs[PR_CCS] |= F_FLAG_V10; /* set F. */ @@ -233,7 +233,7 @@ void do_interrupt(CPUCRISState *env) /* Now that we are in kernel mode, load the handlers address. This load may not fault, real hw leaves that behaviour as undefined. */ - env-pc = ldl_code(env-pregs[PR_EBP] + ex_vec * 4); +env-pc = cpu_ldl_code(env, env-pregs[PR_EBP] + ex_vec * 4); /* Clear the excption_index to avoid spurios hw_aborts for recursive bus faults. */ diff --git a/target-cris/helper.h b/target-cris/helper.h index 093063a..b575524 100644 --- a/target-cris/helper.h +++ b/target-cris/helper.h @@ -1,26 +1,26 @@ #include def-helper.h -DEF_HELPER_1(raise_exception, void, i32) -DEF_HELPER_1(tlb_flush_pid, void, i32) -DEF_HELPER_1(spc_write, void, i32) +DEF_HELPER_2(raise_exception, void, env, i32) +DEF_HELPER_2(tlb_flush_pid, void, env, i32) +DEF_HELPER_2(spc_write, void, env, i32) DEF_HELPER_3(dump, void, i32, i32, i32) -DEF_HELPER_0(rfe, void); -DEF_HELPER_0(rfn, void); +DEF_HELPER_1(rfe, void, env); +DEF_HELPER_1(rfn, void, env); -DEF_HELPER_2(movl_sreg_reg, void, i32, i32) -DEF_HELPER_2(movl_reg_sreg, void, i32, i32) +DEF_HELPER_3(movl_sreg_reg, void, env, i32, i32) +DEF_HELPER_3(movl_reg_sreg, void, env, i32, i32) DEF_HELPER_FLAGS_1(lz, TCG_CALL_PURE, i32, i32); -DEF_HELPER_FLAGS_3(btst, TCG_CALL_PURE, i32, i32, i32, i32); +DEF_HELPER_FLAGS_4(btst, TCG_CALL_PURE, i32, env, i32, i32, i32); -DEF_HELPER_FLAGS_3(evaluate_flags_muls, TCG_CALL_PURE, i32, i32, i32, i32) -DEF_HELPER_FLAGS_3(evaluate_flags_mulu, TCG_CALL_PURE, i32, i32, i32, i32) -DEF_HELPER_FLAGS_4(evaluate_flags_mcp, TCG_CALL_PURE, i32, i32, i32, i32, i32) -DEF_HELPER_FLAGS_4(evaluate_flags_alu_4, TCG_CALL_PURE, i32, i32, i32, i32, i32) -DEF_HELPER_FLAGS_4(evaluate_flags_sub_4, TCG_CALL_PURE, i32, i32, i32, i32, i32) -DEF_HELPER_FLAGS_2(evaluate_flags_move_4, TCG_CALL_PURE, i32, i32, i32) -DEF_HELPER_FLAGS_2(evaluate_flags_move_2, TCG_CALL_PURE, i32, i32, i32) -DEF_HELPER_0(evaluate_flags, void) -DEF_HELPER_0(top_evaluate_flags, void) +DEF_HELPER_FLAGS_4(evaluate_flags_muls, TCG_CALL_PURE, i32, env, i32, i32, i32) +DEF_HELPER_FLAGS_4(evaluate_flags_mulu, TCG_CALL_PURE, i32, env, i32, i32, i32) +DEF_HELPER_FLAGS_5(evaluate_flags_mcp, TCG_CALL_PURE, i32, env, i32, i32, i32, i32) +DEF_HELPER_FLAGS_5(evaluate_flags_alu_4, TCG_CALL_PURE, i32, env, i32, i32, i32, i32) +DEF_HELPER_FLAGS_5(evaluate_flags_sub_4, TCG_CALL_PURE, i32, env, i32, i32, i32, i32) +DEF_HELPER_FLAGS_3(evaluate_flags_move_4, TCG_CALL_PURE, i32, env, i32, i32) +DEF_HELPER_FLAGS_3(evaluate_flags_move_2, TCG_CALL_PURE, i32, env, i32,
Re: [Qemu-devel] qemu 1.2 : lsi controller + scsi-block don't boot.
Il 07/09/2012 14:35, Alexandre DERUMIER ha scritto: I'm trying to boot scsi-block device with lsi controller, and it doesn't boot. (don't find devices). lsi + scsi-block : don't boot lsi + scsi-hd : boot virtio-scsi + scsi-block : boot The LSI driver in SeaBIOS is really a best effort driver, it's not fun to debug it either... just don't use it, pick up the virtio-scsi backport that is in CentOS and lobby your distro to include it... Paolo
Re: [Qemu-devel] qemu 1.2 : lsi controller + scsi-block don't boot.
Thanks, But Why does it work with lsi + scsi-hd and not scsi-block? For now I'll use scsi-hd for these (very old) guests, it's not a problem. - Mail original - De: Paolo Bonzini pbonz...@redhat.com À: Alexandre DERUMIER aderum...@odiso.com Cc: qemu-devel@nongnu.org Envoyé: Vendredi 7 Septembre 2012 16:48:39 Objet: Re: qemu 1.2 : lsi controller + scsi-block don't boot. Il 07/09/2012 14:35, Alexandre DERUMIER ha scritto: I'm trying to boot scsi-block device with lsi controller, and it doesn't boot. (don't find devices). lsi + scsi-block : don't boot lsi + scsi-hd : boot virtio-scsi + scsi-block : boot The LSI driver in SeaBIOS is really a best effort driver, it's not fun to debug it either... just don't use it, pick up the virtio-scsi backport that is in CentOS and lobby your distro to include it... Paolo
Re: [Qemu-devel] [PATCH] musicpal: Fix flash mapping
On 2012-09-07 16:41, Peter Maydell wrote: On 7 September 2012 00:03, Jan Kiszka jan.kis...@web.de wrote: The old arithmetic assumed 32 physical address bits which is no longer true for ARM since 3cc0cd61f4. Signed-off-by: Jan Kiszka jan.kis...@web.de --- hw/musicpal.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/musicpal.c b/hw/musicpal.c index ad725b5..10c2c16 100644 --- a/hw/musicpal.c +++ b/hw/musicpal.c @@ -1583,7 +1583,7 @@ static void musicpal_init(ram_addr_t ram_size, * image is smaller than 32 MB. */ #ifdef TARGET_WORDS_BIGENDIAN -pflash_cfi02_register(0-MP_FLASH_SIZE_MAX, NULL, +pflash_cfi02_register(0x1-MP_FLASH_SIZE_MAX, NULL, I don't think this will compile on a 32 bit system, will it? You probably want an ULL suffix. It does as the result always fits in 32 bits. But I can add that if you prefer. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH ] lan9118: fix multicast filtering
On Thu, Aug 23, 2012 at 05:39:39PM +0200, Aurelien Jarno wrote:
The lan9118 emulation tries to compute the multicast index by calling
directly the crc32() function from zlib, but fails to get the correct
result.
Use the common compute_mcast_idx() function instead, which gives the
correct result. This fixes IPv6 support.
Signed-off-by: Aurelien Jarno aurel...@aurel32.net
---
hw/lan9118.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/lan9118.c b/hw/lan9118.c
index ff0a50b..ceaf96f 100644
--- a/hw/lan9118.c
+++ b/hw/lan9118.c
@@ -500,7 +500,7 @@ static int lan9118_filter(lan9118_state *s, const uint8_t
*addr)
}
} else {
/* Hash matching */
-hash = (crc32(~0, addr, 6) 26);
+hash = compute_mcast_idx(addr);
if (hash 0x20) {
return (s-mac_hashh (hash 0x1f)) 1;
} else {
Ping?
For the record the Linux kernel uses the ether_crc() function for
smsc911x.c, but also for 8139cp.c, 8139too.c and ethoc.c, which use
compute_mcast_idx() on the QEMU side.
To test it, just run this machine with a Linux kernel with IPv6 support
on an IPv6-enabled network with router advertisement, it should get an
IPv6 address automatically. It doesn't without this patch.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH ] lan9118: fix multicast filtering
On 7 September 2012 15:56, Aurelien Jarno aurel...@aurel32.net wrote:
On Thu, Aug 23, 2012 at 05:39:39PM +0200, Aurelien Jarno wrote:
The lan9118 emulation tries to compute the multicast index by calling
directly the crc32() function from zlib, but fails to get the correct
result.
Use the common compute_mcast_idx() function instead, which gives the
correct result. This fixes IPv6 support.
Signed-off-by: Aurelien Jarno aurel...@aurel32.net
---
hw/lan9118.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/lan9118.c b/hw/lan9118.c
index ff0a50b..ceaf96f 100644
--- a/hw/lan9118.c
+++ b/hw/lan9118.c
@@ -500,7 +500,7 @@ static int lan9118_filter(lan9118_state *s, const
uint8_t *addr)
}
} else {
/* Hash matching */
-hash = (crc32(~0, addr, 6) 26);
+hash = compute_mcast_idx(addr);
if (hash 0x20) {
return (s-mac_hashh (hash 0x1f)) 1;
} else {
Ping?
For the record the Linux kernel uses the ether_crc() function for
smsc911x.c, but also for 8139cp.c, 8139too.c and ethoc.c, which use
compute_mcast_idx() on the QEMU side.
Looks ok to me. I did check the data sheet, which helpfully doesn't
say exactly what the CRC function is, and also the zlib docs (which
suggest we should use something that isn't what we were doing here).
So I guess
Reviewed-by: Peter Maydell peter.mayd...@linaro.org
Happy for you to commit directly or I can put it in arm-devs.next
if you prefer.
-- PMM
Re: [Qemu-devel] [PATCH v6 2/2] block: Support GlusterFS as a QEMU block backend
On Thu, Sep 06, 2012 at 12:29:30PM +0200, Kevin Wolf wrote:
Am 06.09.2012 12:18, schrieb Paolo Bonzini:
Il 06/09/2012 12:07, Kevin Wolf ha scritto:
The AIOCB is already invalid at the time the callback is entered, so we
could release it before the call. However, not all implementation of
AIO are ready for that and I'm not really in the mood for large scale
refactoring...
But the way, what I'd really want to see in the end is to get rid of
qemu_aio_flush() and replace it by .bdrv_drain() callbacks in each
BlockDriver. The way we're doing it today is a layering violation.
That's quite difficult. Completion of an I/O operation can trigger
another I/O operation on another block device, and so on until we go
back to the first device (think of a hypothetical RAID-5 device).
You always have a tree of BDSes, and children should only ever trigger
completion of I/O operations in their parents. Am I missing anything?
Doesn't change anything about this problem, though. So the options that
we have are:
1. Delay the callback using a BH. Doing this in each driver is ugly.
But is there actually more than one possible callback in today's
coroutine world? I only see bdrv_co_io_em_complete(), which could
reenter the coroutine from a BH.
Easy and safe, but it feels a bit like a timebomb. Also, I'm not
entirely sure of _why_ the bottom half works. :)
Hm, safe and time bomb is contradictory in my book. :-)
The bottom half work because we're not reentering the qcow2_create
coroutine immediately, so the gluster AIO callback can complete all of
its cleanup work without being interrupted by code that might wait on
this particular request and create a deadlock this way.
2. Delay the callback by just calling it later when the cleanup has
been completed and .io_flush() can return 0. You say that it's hard
to implement for some drivers, except if the AIOCB are leaked until
the end of functions like qcow2_create().
... which is what we do in posix-aio-compat.c; nobody screamed so far.
True. Would be easy to fix in posix-aio-compat, though, or can a
callback expect that the AIOCB is still valid?
Not really hard, it just has to be assessed for each driver separately.
We can just do it in gluster and refactor it later.
Okay, so let's keep it as an option for now.
I tried this approach (option 2) in gluster and I was able to go past the hang
I was seeing earlier, but this causes other problems.
Let me restate what I am doing so that you could tell me if I am indeed
following the option 2 you mention above. I am doing the cleanup first
(qemu_aio_count-- and releasing the AIOCB) before calling the callback at
the end.
static void qemu_gluster_complete_aio(GlusterAIOCB *acb, BDRVGlusterState *s)
{
int ret;
bool *finished = acb-finished;
BlockDriverCompletionFunc *cb = acb-common.cb;
void *opaque = acb-common.opaque;
if (!acb-ret || acb-ret == acb-size) {
ret = 0; /* Success */
} else if (acb-ret 0) {
ret = acb-ret; /* Read/Write failed */
} else {
ret = -EIO; /* Partial read/write - fail it */
}
s-qemu_aio_count--;
qemu_aio_release(acb);
cb(opaque, ret);
if (finished) {
*finished = true;
}
}
static void qemu_gluster_aio_event_reader(void *opaque)
{
BDRVGlusterState *s = opaque;
ssize_t ret;
do {
char *p = (char *)s-event_acb;
ret = read(s-fds[GLUSTER_FD_READ], p + s-event_reader_pos,
sizeof(s-event_acb) - s-event_reader_pos);
if (ret 0) {
s-event_reader_pos += ret;
if (s-event_reader_pos == sizeof(s-event_acb)) {
s-event_reader_pos = 0;
qemu_gluster_complete_aio(s-event_acb, s);
//s-qemu_aio_count--;
}
}
} while (ret 0 errno == EINTR);
}
qemu_gluster_aio_event_reader() is the node-io_read in qemu_aio_wait().
qemu_aio_wait() calls node-io_read() which calls qemu_gluster_complete_aio().
Before we return back to qemu_aio_wait(), many other things happen:
bdrv_close() gets called from qcow2_create2()
This closes the gluster connection, closes the pipe, does
qemu_set_fd_hander(read_pipe_fd, NULL, NULL, NULL, NULL), which results
in the AioHandler node being deleted from aio_handlers list.
Now qemu_gluster_aio_event_reader (node-io_read) which was called from
qemu_aio_wait() finally completes and goes ahead and accesses node
which has already been deleted. This causes segfault.
So I think the option 1 (scheduling a BH from node-io_read) would
be better for gluster.
Regards,
Bharata.
[Qemu-devel] [PATCH] pflash_cfi01: fix vendor specific extended query
pflash_cfi01 announces a version number of 1.1, which implies Protection Register Information and Burst Read information sections, which are not provided. Decrease the version number to 1.0 so that only the Protection Register Information section is needed. Set the number of protection fields (0x3f) to 0x01, as 0x00 means 256 protections field, which makes the CFI table bigger than the current implementation, causing some kernels to fail to read it. Signed-off-by: Aurelien Jarno aurel...@aurel32.net --- hw/pflash_cfi01.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/pflash_cfi01.c b/hw/pflash_cfi01.c index d1c7423..d56b51a 100644 --- a/hw/pflash_cfi01.c +++ b/hw/pflash_cfi01.c @@ -711,7 +711,7 @@ pflash_t *pflash_cfi01_register(target_phys_addr_t base, pfl-cfi_table[0x33] = 'I'; pfl-cfi_table[0x34] = '1'; -pfl-cfi_table[0x35] = '1'; +pfl-cfi_table[0x35] = '0'; pfl-cfi_table[0x36] = 0x00; pfl-cfi_table[0x37] = 0x00; @@ -723,6 +723,8 @@ pflash_t *pflash_cfi01_register(target_phys_addr_t base, pfl-cfi_table[0x3b] = 0x00; pfl-cfi_table[0x3c] = 0x00; +pfl-cfi_table[0x3f] = 0x01; /* Number of protection fields */ + return pfl; } -- 1.7.10.4
[Qemu-devel] [PATCH] target-sparc: fix fcmp{s, d, q} instructions wrt exception
fcmp{s,d,q} instructions are supposed to ignore quiet NaN (contrary to
the fcmpe{s,d,q} instructions), but the current code is wrongly setting
the NV exception in that case. Moreover the current code is duplicated:
first the arguments are checked for NaN to generate an exception, and
later in case the comparison is unordered (which can only happens if one
of the argument is a NaN), the same check is done to generate an
exception.
Fix that by calling clear_float_exceptions() followed by
check_ieee_exceptions() as for the other floating point instructions.
Use the _compare_quiet functions for fcmp{s,d,q} and the _compare ones
for fcmpe{s,d,q}. Simplify the flag setting by not clearing a flag that
is set the line just below.
This fix allows the math glibc testsuite to pass.
Cc: Blue Swirl blauwir...@gmail.com
Signed-off-by: Aurelien Jarno aurel...@aurel32.net
---
target-sparc/fop_helper.c | 67 ++---
1 file changed, 27 insertions(+), 40 deletions(-)
diff --git a/target-sparc/fop_helper.c b/target-sparc/fop_helper.c
index 9c64ef8..f4b62a5 100644
--- a/target-sparc/fop_helper.c
+++ b/target-sparc/fop_helper.c
@@ -334,34 +334,28 @@ void helper_fsqrtq(CPUSPARCState *env)
}
#define GEN_FCMP(name, size, reg1, reg2, FS, E) \
-void glue(helper_, name) (CPUSPARCState *env)\
+void glue(helper_, name) (CPUSPARCState *env) \
{ \
-env-fsr = FSR_FTT_NMASK; \
-if (E (glue(size, _is_any_nan)(reg1) || \
- glue(size, _is_any_nan)(reg2)) \
-(env-fsr FSR_NVM)) { \
-env-fsr |= FSR_NVC;\
-env-fsr |= FSR_FTT_IEEE_EXCP; \
-helper_raise_exception(env, TT_FP_EXCP);\
+int ret;\
+clear_float_exceptions(env);\
+if (E) {\
+ret = glue(size, _compare)(reg1, reg2, env-fp_status);\
+} else {\
+ret = glue(size, _compare_quiet)(reg1, reg2,\
+ env-fp_status); \
} \
-switch (glue(size, _compare) (reg1, reg2, env-fp_status)) { \
+check_ieee_exceptions(env); \
+switch (ret) { \
case float_relation_unordered: \
-if ((env-fsr FSR_NVM)) { \
-env-fsr |= FSR_NVC;\
-env-fsr |= FSR_FTT_IEEE_EXCP; \
-helper_raise_exception(env, TT_FP_EXCP);\
-} else {\
-env-fsr = ~((FSR_FCC1 | FSR_FCC0) FS); \
-env-fsr |= (FSR_FCC1 | FSR_FCC0) FS;\
-env-fsr |= FSR_NVA;\
-} \
+env-fsr |= (FSR_FCC1 | FSR_FCC0) FS;\
+env-fsr |= FSR_NVA;\
break; \
case float_relation_less: \
-env-fsr = ~((FSR_FCC1 | FSR_FCC0) FS); \
+env-fsr = ~(FSR_FCC1) FS; \
env-fsr |= FSR_FCC0 FS; \
break; \
case float_relation_greater:\
-env-fsr = ~((FSR_FCC1 | FSR_FCC0) FS); \
+env-fsr = ~(FSR_FCC0) FS; \
env-fsr |= FSR_FCC1 FS; \
break; \
default:\
@@ -370,34 +364,27 @@ void helper_fsqrtq(CPUSPARCState *env)
} \
}
#define GEN_FCMP_T(name, size, FS, E) \
-void glue(helper_, name)(CPUSPARCState *env, size src1, size src2) \
+void glue(helper_, name)(CPUSPARCState *env, size src1,
Re: [Qemu-devel] [RFC v2 PATCH 1/6] block: add support functions for live commit, to find and delete images.
On 09/07/2012 06:19 AM, Kevin Wolf wrote:
Am 06.09.2012 16:59, schrieb Jeff Cody:
On 09/06/2012 09:23 AM, Kevin Wolf wrote:
Am 30.08.2012 20:47, schrieb Jeff Cody:
Add bdrv_find_child(), and bdrv_delete_intermediate().
bdrv_find_child(): given 'bs' and the active (topmost) BDS of an image
chain,
find the image that is the immediate top of 'bs'
bdrv_delete_intermediate():
Given 3 BDS (active, top, base), delete images above
base up to and including top, and set base to be the
parent of top's child node.
E.g., this converts:
bottom - base - intermediate - top - active
to
bottom - base - active
where top == active is permitted, although active
will not be deleted.
Signed-off-by: Jeff Cody jc...@redhat.com
At first, when just reading the function name, I thought this would
actually delete the image file. Of course, it only removes it from the
backing file chain, but leaves the image file around. I don't have a
good suggestion, but if someone has a better name, I think we should
change it.
Hmm, the naming seems consistent with bdrv_delete(), which does not
actually delete the image files either (and, that is essentially what
this does... calls bdrv_delete(), on the intermediate images).
However, here are some other name proposals:
* bdrv_disconnect_intermediate()
* bdrv_drop_intermediate()
* bdrv_shorten_chain()
bdrv_drop_intermediate() sounds good to me.
+
+typedef struct BlkIntermediateStates {
+BlockDriverState *bs;
+QSIMPLEQ_ENTRY(BlkIntermediateStates) entry;
+} BlkIntermediateStates;
+
+
+/* deletes images above 'base' up to and including 'top', and sets the
image
+ * above 'top' to have base as its backing file.
+ *
+ * E.g., this will convert the following chain:
+ * bottom - base - intermediate - top - active
+ *
+ * to
+ *
+ * bottom - base - active
+ *
+ * It is allowed for bottom==base, in which case it converts:
+ *
+ * base - intermediate - top - active
+ *
+ * to
+ *
+ * base - active
+ *
+ * It is also allowed for top==active, except in that case active is not
+ * deleted:
Hm, makes the interface inconsistent. Shouldn't you be using top ==
intermediate and it would work without any special casing?
To remain consistent, maybe we should define it as an error if
top==active, and return error in that case? The caller can be
responsible for checking for that - if the caller wants to merge down
the active layer, there are additional steps to be taken anyway.
Yes, why not.
And we can always revisit when implementing the additional functionality.
+/* we could not find the image above 'top', this is an error */
+goto exit;
+}
+
+/* if the active and top image passed in are the same, then we
+ * can't delete the active, so we start one below
+ */
+intermediate = (active == top) ? active-backing_hd : top;
Aha. So intermediate is used to undo the special case. Now we're always
on the last image to be deleted.
This is equivalent to an unconditional new_top_bs-backing_hd.
How about changing this to use the simpler unconditional version?
Sure - since active == top is now an error, there is no reason for the
more complicated logic. And at this point, the statement
(new_top_bs-backing_hd == top) should always be true.
Kevin
Re: [Qemu-devel] [PATCH] musicpal: Fix flash mapping
On 7 September 2012 15:53, Jan Kiszka jan.kis...@web.de wrote: On 2012-09-07 16:41, Peter Maydell wrote: On 7 September 2012 00:03, Jan Kiszka jan.kis...@web.de wrote: +pflash_cfi02_register(0x1-MP_FLASH_SIZE_MAX, NULL, I don't think this will compile on a 32 bit system, will it? You probably want an ULL suffix. It does as the result always fits in 32 bits. But I can add that if you prefer. I think I had a misconception of this bit of the C standard. C will pick a type big enough to fit the constant value (which will in this case be a 64 bit type of some kind), even without an ULL suffix. So you're right, it's OK. Reviewed-by: Peter Maydell peter.mayd...@linaro.org -- PMM
[Qemu-devel] [PATCH 2/3] g3beige: add a video card only when requested
The g3beige machine always add a video card, even when the -vga none
is passed. Fix that by checking if it is enabled or not before
instanciating it.
Cc: Alexander Graf ag...@suse.de
Signed-off-by: Aurelien Jarno aurel...@aurel32.net
---
hw/ppc_oldworld.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/ppc_oldworld.c b/hw/ppc_oldworld.c
index 1dcd8a6..363b0e5 100644
--- a/hw/ppc_oldworld.c
+++ b/hw/ppc_oldworld.c
@@ -250,7 +250,9 @@ static void ppc_heathrow_init (ram_addr_t ram_size,
pci_bus = pci_grackle_init(0xfec0, pic,
get_system_memory(),
get_system_io());
-pci_vga_init(pci_bus);
+if (std_vga_enabled) {
+pci_vga_init(pci_bus);
+}
escc_mem = escc_init(0, pic[0x0f], pic[0x10], serial_hds[0],
serial_hds[1], ESCC_CLOCK, 4);
--
1.7.10.4
[Qemu-devel] [PATCH 1/3] sun4u: add a video card only when requested
The sun4u machine always add a video card, even when -vga none is
passed. Fix that by checking if it is enabled or not before
instanciating it.
Cc: Blue Swirl blauwir...@gmail.com
Signed-off-by: Aurelien Jarno aurel...@aurel32.net
---
hw/sun4u.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/sun4u.c b/hw/sun4u.c
index 07cd042..c6bf6eb 100644
--- a/hw/sun4u.c
+++ b/hw/sun4u.c
@@ -821,7 +821,9 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
ivec_irqs = qemu_allocate_irqs(cpu_set_ivec_irq, env, IVEC_MAX);
pci_bus = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE, ivec_irqs,
pci_bus2,
pci_bus3, pbm_irqs);
-pci_vga_init(pci_bus);
+if (std_vga_enabled) {
+pci_vga_init(pci_bus);
+}
// XXX Should be pci_bus3
isa_bus = pci_ebus_init(pci_bus, -1, pbm_irqs);
--
1.7.10.4
[Qemu-devel] [PATCH 3/3] mac99: add a video card only when requested
The mac99 machine always add a video card, even when the -vga none
is passed. Fix that by checking if it is enabled or not before
instanciating it.
Cc: Alexander Graf ag...@suse.de
Signed-off-by: Aurelien Jarno aurel...@aurel32.net
---
hw/ppc_newworld.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c
index e95cfe8..6db8b3a 100644
--- a/hw/ppc_newworld.c
+++ b/hw/ppc_newworld.c
@@ -330,7 +330,9 @@ static void ppc_core99_init (ram_addr_t ram_size,
machine_arch = ARCH_MAC99;
}
/* init basic PC hardware */
-pci_vga_init(pci_bus);
+if (std_vga_enabled) {
+pci_vga_init(pci_bus);
+}
escc_mem = escc_init(0, pic[0x25], pic[0x24],
serial_hds[0], serial_hds[1], ESCC_CLOCK, 4);
--
1.7.10.4
Re: [Qemu-devel] [PATCH ] lan9118: fix multicast filtering
On Fri, Sep 07, 2012 at 04:04:16PM +0100, Peter Maydell wrote:
On 7 September 2012 15:56, Aurelien Jarno aurel...@aurel32.net wrote:
On Thu, Aug 23, 2012 at 05:39:39PM +0200, Aurelien Jarno wrote:
The lan9118 emulation tries to compute the multicast index by calling
directly the crc32() function from zlib, but fails to get the correct
result.
Use the common compute_mcast_idx() function instead, which gives the
correct result. This fixes IPv6 support.
Signed-off-by: Aurelien Jarno aurel...@aurel32.net
---
hw/lan9118.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/lan9118.c b/hw/lan9118.c
index ff0a50b..ceaf96f 100644
--- a/hw/lan9118.c
+++ b/hw/lan9118.c
@@ -500,7 +500,7 @@ static int lan9118_filter(lan9118_state *s, const
uint8_t *addr)
}
} else {
/* Hash matching */
-hash = (crc32(~0, addr, 6) 26);
+hash = compute_mcast_idx(addr);
if (hash 0x20) {
return (s-mac_hashh (hash 0x1f)) 1;
} else {
Ping?
For the record the Linux kernel uses the ether_crc() function for
smsc911x.c, but also for 8139cp.c, 8139too.c and ethoc.c, which use
compute_mcast_idx() on the QEMU side.
Looks ok to me. I did check the data sheet, which helpfully doesn't
say exactly what the CRC function is, and also the zlib docs (which
suggest we should use something that isn't what we were doing here).
So I guess
Reviewed-by: Peter Maydell peter.mayd...@linaro.org
Happy for you to commit directly or I can put it in arm-devs.next
if you prefer.
Thanks for the review, I have applied it.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
[Qemu-devel] ping Re: [RFC PATCH 00/13] Embedded NBD server
Il 27/08/2012 17:00, Paolo Bonzini ha scritto: The part where I need a second opinion and/or ack is patch 12 and 13. They fix the case of a disk being unplugged while NBD export is active. To do this I add a NotifierList to a BlockDriverState. Does this look okay, or is it too ad hoc? Ping... Kevin/Stefan, could you look at just these two patches: http://permalink.gmane.org/gmane.comp.emulators.qemu/167411 [12/13] block: add close notifiers http://permalink.gmane.org/gmane.comp.emulators.qemu/167410 [13/13] nbd: add notifier to close exports when the image is closed and if you need some context: http://permalink.gmane.org/gmane.comp.emulators.qemu/167400 [09/13] qmp: add NBD server commands Everything else is totally uninteresting. Paolo
Re: [Qemu-devel] [RFC v2 PATCH 2/6] block: add live block commit functionality
On 09/06/2012 05:16 PM, Eric Blake wrote: On 09/06/2012 02:37 PM, Jeff Cody wrote: On 09/06/2012 10:00 AM, Kevin Wolf wrote: Am 30.08.2012 20:47, schrieb Jeff Cody: This adds the live commit coroutine. This iteration focuses on the commit only below the active layer, and not the active layer itself. The behaviour is similar to block streaming; the sectors are walked through, and anything that exists above 'base' is committed back down into base. At the end, intermediate images are deleted, and the chain stitched together. Images are restored to their original open flags upon completion. What should we do with backing files that are smaller than the image to commit? In this version, data is copied up to the size of the backing file, and then we get -EIO from bdrv_co_do_writev(). We could leave it like that, and let it receive -EIO in that case. Alternatively, we could try and determine before the commit if the data will fit in the base, and return -ENOSPC if not. Neither sounds appealing. Why can't we first try to resize the base to the new data size being committed, and only fall back to -ENOSPC or -EIO if the resize fails? OK - we will attempt to resize the base, and return the appropriate error on failure or if unsupported for the format.
Re: [Qemu-devel] [PATCH 4/5] virtio-scsi: Add start/stop functionality for vhost-scsi
Il 07/09/2012 08:48, Nicholas A. Bellinger ha scritto: Cc: Stefan Hajnoczi stefa...@linux.vnet.ibm.com Cc: Zhi Yong Wu wu...@linux.vnet.ibm.com Cc: Michael S. Tsirkin m...@redhat.com Cc: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org --- hw/virtio-pci.c |2 ++ hw/virtio-scsi.c | 49 + hw/virtio-scsi.h |1 + 3 files changed, 52 insertions(+), 0 deletions(-) Please create a completely separate device vhost-scsi-pci instead (or virtio-scsi-tcm-pci, or something like that). It is used completely differently from virtio-scsi-pci, it does not make sense to conflate the two. Paolo
[Qemu-devel] Linux KVM, Windows 7 guest choppy sound
Hi, I apologize if this isn't the right venue for this message, but this mailing list seems a bit more active than qemu-discuss. Background: I am running OpenSUSE 12.1. I fixed audio issues in VM guests by setting the following in qemu.conf: vnc_allow_host_audio = 1 I also set user= and group= to allow qemu-kvm to run as the same user as I am logged in as. This allowed qemu-kvm to send audio to pulseaudio. My issue: I am using the ICH6 virtual audio driver in my VMs. In my Linux VMs, the sound works perfectly without any issues. In my Windows 7 VM, the sound works with the exception of static and choppiness in the audio. Has anyone else seen this or have any ideas for a fix? Thanks! Regards Erik
Re: [Qemu-devel] ping Re: [RFC PATCH 00/13] Embedded NBD server
Am 07.09.2012 17:50, schrieb Paolo Bonzini: Il 27/08/2012 17:00, Paolo Bonzini ha scritto: The part where I need a second opinion and/or ack is patch 12 and 13. They fix the case of a disk being unplugged while NBD export is active. To do this I add a NotifierList to a BlockDriverState. Does this look okay, or is it too ad hoc? Ping... Kevin/Stefan, could you look at just these two patches: http://permalink.gmane.org/gmane.comp.emulators.qemu/167411 [12/13] block: add close notifiers http://permalink.gmane.org/gmane.comp.emulators.qemu/167410 [13/13] nbd: add notifier to close exports when the image is closed and if you need some context: http://permalink.gmane.org/gmane.comp.emulators.qemu/167400 [09/13] qmp: add NBD server commands Everything else is totally uninteresting. I was planning to review it in more detail next week, but I just had a quick look. I'm not sure if automatically shutting down the NBD server when the guest stops using it is always right (for removable media it could even be an eject from the guest), but introducing a notifier list doesn't look too bad. We can probably use it for other things that are currently hardcoded in bdrv_close() with some if statements, like disabling I/O throttling, cancelling a block job, etc. Kevin
[Qemu-devel] [RFC v2] Migration thread
Hi here is v2 of the migration thread series. There is still some issues with locking in the error paths (they are at 54 patches now). Changes from v1: - migration stats series are included - migration bitmap sync trace-events to know how long it takes - file-last_error use almost removed reworked functions to return real error codes and work with that. Some more work needed here. - new savevm for live migration pending method. see last commit for details. Please test and comment. Later, Juan. The following changes since commit 6e4c0d1f03d6ab407509c32fab7cb4b8230f57ff: hw/pl110: Fix spelling of 'palette' (2012-09-06 17:04:33 +0200) are available in the git repository at: http://repo.or.cz/r/qemu/quintela.git migration-thread-v2 for you to fetch changes up to 688feac0fbc287920dff537ed13fb8483c064f7f: savem: Add calculating a new save_live migration method: pending (2012-09-07 14:00:35 +0200) Juan Quintela (49): buffered_file: g_realloc() can't fail fix migration sync migration: store end_time in a local variable migration: print total downtime for final phase of migration migration: rename expected_time to expected_downtime migration: export migrate_get_current() migration: print expected downtime in info migrate savevm: Factorize ram globals reset in its own function ram: introduce migration_bitmap_set_dirty() ram: Introduce migration_bitmap_test_and_reset_dirty() ram: Export last_ram_offset() ram: introduce migration_bitmap_sync() ram: create trace event for migration sync bitmap Separate migration bitmap migration: Add dirty_pages_rate to query migrate output buffered_file: rename opaque to migration_state buffered_file: opaque is MigrationState buffered_file: unfold migrate_fd_put_buffer buffered_file: unfold migrate_fd_put_ready buffered_file: unfold migrate_fd_put_buffer buffered_file: unfold migrate_fd_put_buffer buffered_file: We can access directly to bandwidth_limit buffered_file: callers of buffered_flush() already check for errors buffered_file: make buffered_flush return the error code migration: make migrate_fd_wait_for_unfreeze() return errors savevm: unexport qemu_fflush viritio-net: use qemu_get_buffer() in a temp buffer savevm: Remove qemu_fseek() savevm: make qemu_fflush() return an error code savevm: unfold qemu_fclose_internal() savevm: unexport qemu_ftell() savevm: make qemu_fill_buffer() be consistent savevm: Only qemu_fflush() can generate errors buffered_file: buffered_put_buffer() don't need to set last_error block-migration: make flush_blks() return errors block-migration: Switch meaning of return value block-migration: handle errors with the return codes correctly savevm: un-export qemu_file_set_error() savevm: make qemu_file_put_notify() return errors buffered_file: Move from using a timer to use a thread migration: make qemu_fopen_ops_buffered() return void migration: stop all cpus correctly migration: make writes blocking migration: remove unfreeze logic migration: take finer locking buffered_file: Unfold the trick to restart generating migration data buffered_file: don't flush on put buffer buffered_file: unfold buffered_append in buffered_put_buffer savem: Add calculating a new save_live migration method: pending Paolo Bonzini (2): split MRU ram list BufferedFile: append, then flush Umesh Deshpande (2): add a version number to ram_list protect the ramlist with a separate mutex arch_init.c | 174 block-migration.c | 100 + buffered_file.c | 213 +- buffered_file.h | 12 +-- cpu-all.h | 17 - exec-obsolete.h | 10 --- exec.c| 45 ++-- hmp.c | 12 +++ hw/virtio-net.c | 4 +- migration-exec.c | 2 - migration-fd.c| 6 -- migration-tcp.c | 2 +- migration-unix.c | 2 - migration.c | 151 +++--- migration.h | 10 +++ qapi-schema.json | 18 - qemu-file.h | 11 --- qmp-commands.hx | 9 +++ savevm.c | 144 ++-- sysemu.h | 1 + trace-events | 4 + vmstate.h | 1 + 22 files changed, 498 insertions(+), 450 deletions(-)
Re: [Qemu-devel] [RFC v2 PATCH 3/6] blockdev: rename block_stream_cb to a generic block_job_cb
Il 30/08/2012 20:47, Jeff Cody ha scritto:
@@ -53,6 +53,8 @@ static const int if_max_devs[IF_COUNT] = {
[IF_SCSI] = 7,
};
+static void block_job_cb(void *opaque, int ret);
+
/*
* We automatically delete the drive when a device using it gets
* unplugged. Questionable feature, but we can't just drop it.
Can you avoid the forward declaration?
Paolo
Re: [Qemu-devel] [RFC v2] Migration thread
Il 07/09/2012 18:23, Juan Quintela ha scritto: Hi here is v2 of the migration thread series. There is still some issues with locking in the error paths (they are at 54 patches now). Changes from v1: - migration stats series are included - migration bitmap sync trace-events to know how long it takes - file-last_error use almost removed reworked functions to return real error codes and work with that. Some more work needed here. - new savevm for live migration pending method. see last commit for details. Can you start factoring out any cleanup that can be applied independently? Paolo Please test and comment. Later, Juan. The following changes since commit 6e4c0d1f03d6ab407509c32fab7cb4b8230f57ff: hw/pl110: Fix spelling of 'palette' (2012-09-06 17:04:33 +0200) are available in the git repository at: http://repo.or.cz/r/qemu/quintela.git migration-thread-v2 for you to fetch changes up to 688feac0fbc287920dff537ed13fb8483c064f7f: savem: Add calculating a new save_live migration method: pending (2012-09-07 14:00:35 +0200) Juan Quintela (49): buffered_file: g_realloc() can't fail fix migration sync migration: store end_time in a local variable migration: print total downtime for final phase of migration migration: rename expected_time to expected_downtime migration: export migrate_get_current() migration: print expected downtime in info migrate savevm: Factorize ram globals reset in its own function ram: introduce migration_bitmap_set_dirty() ram: Introduce migration_bitmap_test_and_reset_dirty() ram: Export last_ram_offset() ram: introduce migration_bitmap_sync() ram: create trace event for migration sync bitmap Separate migration bitmap migration: Add dirty_pages_rate to query migrate output buffered_file: rename opaque to migration_state buffered_file: opaque is MigrationState buffered_file: unfold migrate_fd_put_buffer buffered_file: unfold migrate_fd_put_ready buffered_file: unfold migrate_fd_put_buffer buffered_file: unfold migrate_fd_put_buffer buffered_file: We can access directly to bandwidth_limit buffered_file: callers of buffered_flush() already check for errors buffered_file: make buffered_flush return the error code migration: make migrate_fd_wait_for_unfreeze() return errors savevm: unexport qemu_fflush viritio-net: use qemu_get_buffer() in a temp buffer savevm: Remove qemu_fseek() savevm: make qemu_fflush() return an error code savevm: unfold qemu_fclose_internal() savevm: unexport qemu_ftell() savevm: make qemu_fill_buffer() be consistent savevm: Only qemu_fflush() can generate errors buffered_file: buffered_put_buffer() don't need to set last_error block-migration: make flush_blks() return errors block-migration: Switch meaning of return value block-migration: handle errors with the return codes correctly savevm: un-export qemu_file_set_error() savevm: make qemu_file_put_notify() return errors buffered_file: Move from using a timer to use a thread migration: make qemu_fopen_ops_buffered() return void migration: stop all cpus correctly migration: make writes blocking migration: remove unfreeze logic migration: take finer locking buffered_file: Unfold the trick to restart generating migration data buffered_file: don't flush on put buffer buffered_file: unfold buffered_append in buffered_put_buffer savem: Add calculating a new save_live migration method: pending Paolo Bonzini (2): split MRU ram list BufferedFile: append, then flush Umesh Deshpande (2): add a version number to ram_list protect the ramlist with a separate mutex arch_init.c | 174 block-migration.c | 100 + buffered_file.c | 213 +- buffered_file.h | 12 +-- cpu-all.h | 17 - exec-obsolete.h | 10 --- exec.c| 45 ++-- hmp.c | 12 +++ hw/virtio-net.c | 4 +- migration-exec.c | 2 - migration-fd.c| 6 -- migration-tcp.c | 2 +- migration-unix.c | 2 - migration.c | 151 +++--- migration.h | 10 +++ qapi-schema.json | 18 - qemu-file.h | 11 --- qmp-commands.hx | 9 +++ savevm.c | 144 ++-- sysemu.h | 1 + trace-events | 4 + vmstate.h | 1 + 22 files changed, 498 insertions(+), 450 deletions(-)
Re: [Qemu-devel] [RFC v2 PATCH 3/6] blockdev: rename block_stream_cb to a generic block_job_cb
Il 07/09/2012 19:04, Jeff Cody ha scritto:
On 09/07/2012 12:27 PM, Paolo Bonzini wrote:
Il 30/08/2012 20:47, Jeff Cody ha scritto:
@@ -53,6 +53,8 @@ static const int if_max_devs[IF_COUNT] = {
[IF_SCSI] = 7,
};
+static void block_job_cb(void *opaque, int ret);
+
/*
* We automatically delete the drive when a device using it gets
* unplugged. Questionable feature, but we can't just drop it.
Can you avoid the forward declaration?
Paolo
Yes, sure - honestly, I added this patch in, but I assumed that the
similar patch of yours to support mirroring would go in first, making
this patch moot.
I now took this patch of yours in my tree (minus the forward
declaration), so...
Paolo
Re: [Qemu-devel] [PULL for usb-next]: Add support for live-migration to usb-redir
Hi, On 09/07/2012 01:47 PM, Hans de Goede wrote: Hi Gerd, I'm very happy to present to you a pull-request for usb-redir live-migration support. I've tested this combined with Spice seamless migration, and it can successful: 1) migrate a vm while running dd if=/dev/zero of=/dev/sdb1 bs=32K inside the guest with sdb being a redirect USB-2 mass storage device. 2) migrate a vm while running camorama inside the vm showing a 720p video from a redirected USB-2 webcam at 30 fps! Note this is based on usb-next rather then master / usb.62, since one of my patches would otherwise conflict with your recent ehci changes. The following changes since commit a44fd2e0c66b2276f586948702e5ebc7136fdb73: usb-host: allow emulated (non-async) control requests without USBPacket (2012-09-06 12:03:41 +0200) are available in the git repository at: git://people.freedesktop.org/~jwrdegoede/qemu usb-for-gerd for you to fetch changes up to 5f5f0f1eaa29ec1cb07fc906acf917d5648b3bcf: usb-redir: Add chardev open / close debug logging (2012-09-07 13:44:49 +0200) Hans de Goede (9): ehci: Don't set seen to 0 when removing unseen queue-heads ehci: Walk async schedule before and after migration ehci: Don't process too much frames in 1 timer tick usb: Migrate over device speed and speedmask Hmm, thinking more about this, this one is only necessary for usb-redir, since for normal devices both get set from the descriptors of the device + the port speedmask, which are const from a migration pov, so no need to migrate them. And usb-redir itself can re-construct them in its post_load function since it migrates over the usb-redir device_info struct already. So let me respin this patch set, dropping the above patch... usb-redir: Change cancelled packet code into a generic packet-id queue usb-redir: Add an already_in_flight packet-id queue usb-redir: Store max_packet_size in endp_data usb-redir: Add support for migration and instead set speed and speedmask from this patch. usb-redir: Add chardev open / close debug logging hw/usb.h | 4 +- hw/usb/bus.c | 2 + hw/usb/hcd-ehci.c | 61 ++- hw/usb/redirect.c | 482 ++ 4 files changed, 508 insertions(+), 41 deletions(-) Regards, Hans
Re: [Qemu-devel] [Qemu-ppc] [PATCH: RFC] Adding BAR0 for e500 PCI controller
On 09/07/2012 03:08 AM, Alexander Graf wrote: On 07.09.2012, at 01:15, Scott Wood scottw...@freescale.com wrote: On 09/03/2012 01:44 AM, Bhushan Bharat-R65777 wrote: -Original Message- From: Wood Scott-B07421 Sent: Wednesday, August 15, 2012 6:59 AM To: Bhushan Bharat-R65777 Cc: qemu-devel@nongnu.org; qemu-...@nongnu.org; ag...@suse.de; Bhushan Bharat- R65777 Subject: Re: [Qemu-ppc] [PATCH: RFC] Adding BAR0 for e500 PCI controller On 08/14/2012 07:50 AM, Bharat Bhushan wrote: PCI Root complex have TYPE-1 configuration header while PCI endpoint have type-0 configuration header. The type-1 configuration header have a BAR (BAR0). In Freescale PCI controller BAR0 is used for mapping pci address space to CCSR address space. This can used for 2 purposes: 1) for MSI interrupt generation 2) Allow CCSR registers access when configured as PCI endpoint, which I am not sure is a use case with QEMU-KVM guest. What I observed is that when guest read the size of BAR0 of host controller configuration header (TYPE1 header) then it always reads it as 0. When looking into the QEMU hw/ppce500_pci.c, I do not find the PCI controller device registering BAR0. I do not find any other controller also doing so may they do not use BAR0. There are two issues when BAR0 is not there (which I can think of): 1) There should be BAR0 emulated for PCI Root comaplex (TYPE1 header) and when reading the size of BAR0, it should give size as per real h/w. 2) Do we need this BAR0 inbound address translation? When BAR0 is of non-zero size then it will be configured for PCI address space to local address(CCSR) space translation on inbound access. The primary use case is for MSI interrupt generation. The device is configured with a address offsets in PCI address space, which will be translated to MSI interrupt generation MPIC registers. Currently I do not understand the MSI interrupt generation mechanism in QEMU and also IIRC we do not use QEMU MSI interrupt mechanism on e500 guest machines. But this BAR0 will be used when using MSI on e500. This patch is only trying to address #1, right? I don't see any connection from this BAR to CCSR. +memory_region_init_io(h-bar0, pci_host_conf_be_ops, h, + PCIHOST-bar0, 0x100); 0x0100 is correct for e500mc-based systems, but it should be 0x0010 for e500v2-based systems. Scott, Currently we have a generic e500 machine which have CCSR size 0x0010 (MPC8544_CCSRBAR_SIZE). We do not have e500mc and e500v2 machines. So should we make this 0x0010 as per generic e500 machine? Yes, but structure it so that board code decides the size, not the PCI code. Can we somehow pass this via qdev/varargs from machine emulation code (hw/ppc/e500.c) ? Possibly, though it may not be the best idea to express every single aspect of intercomponent integration via qdev -- maybe that's best left for things that are reasonably user-tweakable. If CCSR size is user tweakable, it would be somewhere other than the PCI controller. It depends. Qdev properties are basically object constructor parameters. So if you were weiting C++ code and would have a constructor that gets the size as argument, it would end up being modeled as qdev property. If however actual functionality differs, thus you would in OO speech create a subclass / child class, then you are better off creating a new device struct. In this case, I'm not sure. They are different devices really, but are close enough that the differences could be expressed through qdev properties. I wasn't suggesting that they be different devices. I was suggesting that this isn't a property of the PCI controller, but rather of some other entity to which the PCI controller connects. So maybe a reference to the associated CCSR object would be a qdev parameter, but not the size of that CCSR. -Scott
[Qemu-devel] [PULL for usb-next]: Add support for live-migration to usb-redir (v2)
Hi Gerd, I'm very happy to present to you a pull-request for usb-redir live-migration support. I've tested this combined with Spice seamless migration, and it can successful: 1) migrate a vm while running dd if=/dev/zero of=/dev/sdb1 bs=32K inside the guest with sdb being a redirect USB-2 mass storage device. 2) migrate a vm while running camorama inside the vm showing a 720p video from a redirected USB-2 webcam at 30 fps! Note this is based on usb-next rather then master / usb.62, since one of my patches would otherwise conflict with your recent ehci changes. Changes in v2: - As discussed drop the usb: Migrate over device speed and speedmask patch The following changes since commit a44fd2e0c66b2276f586948702e5ebc7136fdb73: usb-host: allow emulated (non-async) control requests without USBPacket (2012-09-06 12:03:41 +0200) are available in the git repository at: git://people.freedesktop.org/~jwrdegoede/qemu usb-for-gerd for you to fetch changes up to 6ef0b771704ca898a44c1f9ea41ab98590fc2e84: usb-redir: Add chardev open / close debug logging (2012-09-07 21:27:16 +0200) Hans de Goede (8): ehci: Don't set seen to 0 when removing unseen queue-heads ehci: Walk async schedule before and after migration ehci: Don't process too much frames in 1 timer tick usb-redir: Change cancelled packet code into a generic packet-id queue usb-redir: Add an already_in_flight packet-id queue usb-redir: Store max_packet_size in endp_data usb-redir: Add support for migration usb-redir: Add chardev open / close debug logging hw/usb/hcd-ehci.c | 61 ++- hw/usb/redirect.c | 500 ++ 2 files changed, 522 insertions(+), 39 deletions(-) Thanks Regards, Hans
[Qemu-devel] [PATCH] qxl: Ignore set_client_capabilities pre/post migrate
The recent introduction of set_client_capabilities has broken
(seamless) migration by trying to call qxl_send_events pre (seamless
incoming) and post (*) migration, triggering the following assert:
qxl_send_events: Assertion `qemu_spice_display_is_running(d-ssd)' failed.
The solution is easy, pre migration the guest will have already received
the client caps on the migration source side, and post migration there no
longer is a guest, so we can simply ignore the set_client_capabilities call
in both those scenarios.
*) Post migration, so not fatal for to the migration itself, but still a crash
Signed-off-by: Hans de Goede hdego...@redhat.com
---
hw/qxl.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/hw/qxl.c b/hw/qxl.c
index 045432e..1b400f1 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -953,6 +953,11 @@ static void interface_set_client_capabilities(QXLInstance
*sin,
{
PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
+if (runstate_check(RUN_STATE_INMIGRATE) ||
+runstate_check(RUN_STATE_POSTMIGRATE)) {
+return;
+}
+
qxl-shadow_rom.client_present = client_present;
memcpy(qxl-shadow_rom.client_capabilities, caps, sizeof(caps));
qxl-rom-client_present = client_present;
--
1.7.12
