[Qemu-devel] [PATCHv7 0/3] qemu: properties for feature compatibility
Here's what I came up with for solving the problem of differences in features in virtio between 0.12 and 0.11. This also enables migration between different backends, e.g. between host where tap supports virtio net header and where it does not: management just needs to set features appropriately. Compat bits are yet to be set, will be done when this patch is applied. changes since v6: - fix arm-softmmu build Changes since v5: - added patch to rename features-guest_features Changes since v4: - Save bit offset in a separate field, offset is in bytes Changes since RFC v2: - we already have a flag to control write cache feature, let's not add another one Changes since RFC: - add symbolic names for properties - only optional features can be changed Acked-by: Gerd Hoffmann kra...@redhat.com Michael S. Tsirkin (3): qdev: add bit property type virtio: rename features - guest_features virtio: add features as qdev properties hw/qdev-properties.c | 62 - hw/qdev.h| 11 + hw/s390-virtio-bus.c | 14 --- hw/s390-virtio-bus.h |1 + hw/syborg_virtio.c | 17 - hw/virtio-balloon.c |4 +- hw/virtio-blk.c |6 + hw/virtio-blk.h |8 ++ hw/virtio-console.c |4 +- hw/virtio-net.c | 49 +-- hw/virtio-net.h | 20 hw/virtio-pci.c | 29 +++ hw/virtio.c | 10 hw/virtio.h |9 +- 14 files changed, 172 insertions(+), 72 deletions(-)
[Qemu-devel] [PATCHv7 1/3] qdev: add bit property type
This adds bit property type, which is a boolean stored in a 32 bit integer field, with legal values on and off. Will be used by virtio for feature bits. Signed-off-by: Michael S. Tsirkin m...@redhat.com Acked-by: Gerd Hoffmann kra...@redhat.com --- hw/qdev-properties.c | 62 - hw/qdev.h| 11 + 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 217ddc0..9e123ae 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -9,6 +9,59 @@ void *qdev_get_prop_ptr(DeviceState *dev, Property *prop) return ptr; } +static uint32_t qdev_get_prop_mask(Property *prop) +{ +assert(prop-info-type == PROP_TYPE_BIT); +return 0x1 prop-bitnr; +} + +static void bit_prop_set(DeviceState *dev, Property *props, bool val) +{ +uint32_t *p = qdev_get_prop_ptr(dev, props); +uint32_t mask = qdev_get_prop_mask(props); +if (val) +*p |= ~mask; +else +*p = ~mask; +} + +static void qdev_prop_cpy(DeviceState *dev, Property *props, void *src) +{ +if (props-info-type == PROP_TYPE_BIT) { +bool *defval = src; +bit_prop_set(dev, props, *defval); +} else { +char *dst = qdev_get_prop_ptr(dev, props); +memcpy(dst, src, props-info-size); +} +} + +/* Bit */ +static int parse_bit(DeviceState *dev, Property *prop, const char *str) +{ +if (!strncasecmp(str, on, 2)) +bit_prop_set(dev, prop, true); +else if (!strncasecmp(str, off, 3)) +bit_prop_set(dev, prop, false); +else +return -1; +return 0; +} + +static int print_bit(DeviceState *dev, Property *prop, char *dest, size_t len) +{ +uint8_t *p = qdev_get_prop_ptr(dev, prop); +return snprintf(dest, len, (*p qdev_get_prop_mask(prop)) ? on : off); +} + +PropertyInfo qdev_prop_bit = { +.name = on/off, +.type = PROP_TYPE_BIT, +.size = sizeof(uint32_t), +.parse = parse_bit, +.print = print_bit, +}; + /* --- 8bit integer --- */ static int parse_uint8(DeviceState *dev, Property *prop, const char *str) @@ -511,7 +564,6 @@ int qdev_prop_parse(DeviceState *dev, const char *name, const char *value) void qdev_prop_set(DeviceState *dev, const char *name, void *src, enum PropertyType type) { Property *prop; -void *dst; prop = qdev_prop_find(dev, name); if (!prop) { @@ -524,8 +576,7 @@ void qdev_prop_set(DeviceState *dev, const char *name, void *src, enum PropertyT __FUNCTION__, dev-info-name, name); abort(); } -dst = qdev_get_prop_ptr(dev, prop); -memcpy(dst, src, prop-info-size); +qdev_prop_cpy(dev, prop, src); } void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value) @@ -585,14 +636,11 @@ void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value) void qdev_prop_set_defaults(DeviceState *dev, Property *props) { -char *dst; - if (!props) return; while (props-name) { if (props-defval) { -dst = qdev_get_prop_ptr(dev, props); -memcpy(dst, props-defval, props-info-size); +qdev_prop_cpy(dev, props, props-defval); } props++; } diff --git a/hw/qdev.h b/hw/qdev.h index bbcdba1..07b9603 100644 --- a/hw/qdev.h +++ b/hw/qdev.h @@ -64,6 +64,7 @@ struct Property { const char *name; PropertyInfo *info; int offset; +int bitnr; void *defval; }; @@ -82,6 +83,7 @@ enum PropertyType { PROP_TYPE_NETDEV, PROP_TYPE_VLAN, PROP_TYPE_PTR, +PROP_TYPE_BIT, }; struct PropertyInfo { @@ -173,6 +175,7 @@ void do_device_del(Monitor *mon, const QDict *qdict); /*** qdev-properties.c ***/ +extern PropertyInfo qdev_prop_bit; extern PropertyInfo qdev_prop_uint8; extern PropertyInfo qdev_prop_uint16; extern PropertyInfo qdev_prop_uint32; @@ -202,6 +205,14 @@ extern PropertyInfo qdev_prop_pci_devfn; + type_check(_type,typeof_field(_state, _field)), \ .defval= (_type[]) { _defval }, \ } +#define DEFINE_PROP_BIT(_name, _state, _field, _bit, _defval) { \ +.name = (_name),\ +.info = (qdev_prop_bit), \ +.bitnr= (_bit), \ +.offset= offsetof(_state, _field)\ ++ type_check(uint32_t,typeof_field(_state, _field)), \ +.defval= (bool[]) { (_defval) }, \ +} #define DEFINE_PROP_UINT8(_n, _s, _f, _d) \ DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint8, uint8_t) -- 1.6.6.rc1.43.gf55cc
[Qemu-devel] [PATCHv7 2/3] virtio: rename features - guest_features
Rename features-guest_features. This is what they are, avoid confusion with host features which we also need to keep around. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- hw/s390-virtio-bus.c |2 +- hw/syborg_virtio.c |4 ++-- hw/virtio-net.c | 10 +- hw/virtio-pci.c |4 ++-- hw/virtio.c |8 hw/virtio.h |2 +- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c index dc154ed..6c0da11 100644 --- a/hw/s390-virtio-bus.c +++ b/hw/s390-virtio-bus.c @@ -251,7 +251,7 @@ void s390_virtio_device_update_status(VirtIOS390Device *dev) if (vdev-set_features) { vdev-set_features(vdev, features); } -vdev-features = features; +vdev-guest_features = features; } VirtIOS390Device *s390_virtio_bus_console(VirtIOS390Bus *bus) diff --git a/hw/syborg_virtio.c b/hw/syborg_virtio.c index a84206a..fe6fc23 100644 --- a/hw/syborg_virtio.c +++ b/hw/syborg_virtio.c @@ -90,7 +90,7 @@ static uint32_t syborg_virtio_readl(void *opaque, target_phys_addr_t offset) ret |= vdev-binding-get_features(s); break; case SYBORG_VIRTIO_GUEST_FEATURES: -ret = vdev-features; +ret = vdev-guest_features; break; case SYBORG_VIRTIO_QUEUE_BASE: ret = virtio_queue_get_addr(vdev, vdev-queue_sel); @@ -132,7 +132,7 @@ static void syborg_virtio_writel(void *opaque, target_phys_addr_t offset, case SYBORG_VIRTIO_GUEST_FEATURES: if (vdev-set_features) vdev-set_features(vdev, value); -vdev-features = value; +vdev-guest_features = value; break; case SYBORG_VIRTIO_QUEUE_BASE: if (value == 0) diff --git a/hw/virtio-net.c b/hw/virtio-net.c index 2f201ff..ab20a33 100644 --- a/hw/virtio-net.c +++ b/hw/virtio-net.c @@ -768,11 +768,11 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id) if (n-has_vnet_hdr) { tap_using_vnet_hdr(n-nic-nc.peer, 1); tap_set_offload(n-nic-nc.peer, -(n-vdev.features VIRTIO_NET_F_GUEST_CSUM) 1, -(n-vdev.features VIRTIO_NET_F_GUEST_TSO4) 1, -(n-vdev.features VIRTIO_NET_F_GUEST_TSO6) 1, -(n-vdev.features VIRTIO_NET_F_GUEST_ECN) 1, -(n-vdev.features VIRTIO_NET_F_GUEST_UFO) 1); +(n-vdev.guest_features VIRTIO_NET_F_GUEST_CSUM) 1, +(n-vdev.guest_features VIRTIO_NET_F_GUEST_TSO4) 1, +(n-vdev.guest_features VIRTIO_NET_F_GUEST_TSO6) 1, +(n-vdev.guest_features VIRTIO_NET_F_GUEST_ECN) 1, +(n-vdev.guest_features VIRTIO_NET_F_GUEST_UFO) 1); } } diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index 62b46bd..4e1d5e1 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -181,7 +181,7 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val) } if (vdev-set_features) vdev-set_features(vdev, val); -vdev-features = val; +vdev-guest_features = val; break; case VIRTIO_PCI_QUEUE_PFN: pa = (target_phys_addr_t)val VIRTIO_PCI_QUEUE_ADDR_SHIFT; @@ -239,7 +239,7 @@ static uint32_t virtio_ioport_read(VirtIOPCIProxy *proxy, uint32_t addr) ret |= vdev-binding-get_features(proxy); break; case VIRTIO_PCI_GUEST_FEATURES: -ret = vdev-features; +ret = vdev-guest_features; break; case VIRTIO_PCI_QUEUE_PFN: ret = virtio_queue_get_addr(vdev, vdev-queue_sel) diff --git a/hw/virtio.c b/hw/virtio.c index bb78ca8..f01548e 100644 --- a/hw/virtio.c +++ b/hw/virtio.c @@ -441,7 +441,7 @@ void virtio_reset(void *opaque) if (vdev-reset) vdev-reset(vdev); -vdev-features = 0; +vdev-guest_features = 0; vdev-queue_sel = 0; vdev-status = 0; vdev-isr = 0; @@ -596,7 +596,7 @@ void virtio_notify(VirtIODevice *vdev, VirtQueue *vq) mb(); /* Always notify when queue is empty (when feature acknowledge) */ if ((vring_avail_flags(vq) VRING_AVAIL_F_NO_INTERRUPT) -(!(vdev-features (1 VIRTIO_F_NOTIFY_ON_EMPTY)) || +(!(vdev-guest_features (1 VIRTIO_F_NOTIFY_ON_EMPTY)) || (vq-inuse || vring_avail_idx(vq) != vq-last_avail_idx))) return; @@ -623,7 +623,7 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) qemu_put_8s(f, vdev-status); qemu_put_8s(f, vdev-isr); qemu_put_be16s(f, vdev-queue_sel); -qemu_put_be32s(f, vdev-features); +qemu_put_be32s(f, vdev-guest_features); qemu_put_be32(f, vdev-config_len); qemu_put_buffer(f, vdev-config, vdev-config_len); @@ -668,7 +668,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f) features, supported_features); return -1; } -vdev-features =
[Qemu-devel] [PATCHv7 3/3] virtio: add features as qdev properties
Add feature bits as properties to virtio. This makes it possible to e.g. define machine without indirect buffer support, which is required for 0.10 compatibility, or without hardware checksum support, which is required for 0.11 compatibility. Since default values for optional features are now set by qdev, get_features callback has been modified: it sets non-optional bits, and clears bits not supported by host. Signed-off-by: Michael S. Tsirkin m...@redhat.com Acked-by: Gerd Hoffmann kra...@redhat.com --- hw/s390-virtio-bus.c | 12 +--- hw/s390-virtio-bus.h |1 + hw/syborg_virtio.c | 13 - hw/virtio-balloon.c |4 ++-- hw/virtio-blk.c |6 +- hw/virtio-blk.h |8 hw/virtio-console.c |4 ++-- hw/virtio-net.c | 39 --- hw/virtio-net.h | 20 hw/virtio-pci.c | 25 + hw/virtio.c |2 +- hw/virtio.h |7 ++- 12 files changed, 91 insertions(+), 50 deletions(-) diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c index 6c0da11..980e7eb 100644 --- a/hw/s390-virtio-bus.c +++ b/hw/s390-virtio-bus.c @@ -101,6 +101,7 @@ static int s390_virtio_device_init(VirtIOS390Device *dev, VirtIODevice *vdev) bus-dev_offs += dev_len; virtio_bind_device(vdev, virtio_s390_bindings, dev); +dev-host_features = vdev-get_features(vdev, dev-host_features); s390_virtio_device_sync(dev); return 0; @@ -222,9 +223,7 @@ static void s390_virtio_device_sync(VirtIOS390Device *dev) cur_offs += num_vq * VIRTIO_VQCONFIG_LEN; /* Sync feature bitmap */ -if (dev-vdev-get_features) { -stl_phys(cur_offs, dev-vdev-get_features(dev-vdev)); -} +stl_phys(cur_offs, dev-host_features); dev-feat_offs = cur_offs + dev-feat_len; cur_offs += dev-feat_len * 2; @@ -310,10 +309,17 @@ static void virtio_s390_notify(void *opaque, uint16_t vector) kvm_s390_virtio_irq(s390_cpu_addr2state(0), 0, token); } +static unsigned virtio_s390_get_features(void *opaque) +{ +VirtIOS390Device *dev = (VirtIOS390Device*)opaque; +return dev-host_features; +} + / S390 Virtio Bus Device Descriptions ***/ static const VirtIOBindings virtio_s390_bindings = { .notify = virtio_s390_notify, +.get_features = virtio_s390_get_features, }; static VirtIOS390DeviceInfo s390_virtio_net = { diff --git a/hw/s390-virtio-bus.h b/hw/s390-virtio-bus.h index ef36714..8ae2065 100644 --- a/hw/s390-virtio-bus.h +++ b/hw/s390-virtio-bus.h @@ -40,6 +40,7 @@ typedef struct VirtIOS390Device { VirtIODevice *vdev; DriveInfo *dinfo; NICConf nic; +uint32_t host_features; } VirtIOS390Device; typedef struct VirtIOS390Bus { diff --git a/hw/syborg_virtio.c b/hw/syborg_virtio.c index fe6fc23..65239a0 100644 --- a/hw/syborg_virtio.c +++ b/hw/syborg_virtio.c @@ -25,6 +25,7 @@ #include syborg.h #include sysbus.h #include virtio.h +#include virtio-net.h #include sysemu.h //#define DEBUG_SYBORG_VIRTIO @@ -66,6 +67,7 @@ typedef struct { uint32_t int_enable; uint32_t id; NICConf nic; +uint32_t host_features; } SyborgVirtIOProxy; static uint32_t syborg_virtio_readl(void *opaque, target_phys_addr_t offset) @@ -86,8 +88,7 @@ static uint32_t syborg_virtio_readl(void *opaque, target_phys_addr_t offset) ret = s-id; break; case SYBORG_VIRTIO_HOST_FEATURES: -ret = vdev-get_features(vdev); -ret |= vdev-binding-get_features(s); +ret = s-host_features; break; case SYBORG_VIRTIO_GUEST_FEATURES: ret = vdev-guest_features; @@ -244,9 +245,8 @@ static void syborg_virtio_update_irq(void *opaque, uint16_t vector) static unsigned syborg_virtio_get_features(void *opaque) { -unsigned ret = 0; -ret |= (1 VIRTIO_F_NOTIFY_ON_EMPTY); -return ret; +SyborgVirtIOProxy *proxy = opaque; +return proxy-host_features; } static VirtIOBindings syborg_virtio_bindings = { @@ -272,6 +272,8 @@ static int syborg_virtio_init(SyborgVirtIOProxy *proxy, VirtIODevice *vdev) qemu_register_reset(virtio_reset, vdev); virtio_bind_device(vdev, syborg_virtio_bindings, proxy); +proxy-host_features |= (0x1 VIRTIO_F_NOTIFY_ON_EMPTY); +proxy-host_features = vdev-get_features(vdev, proxy-host_features); return 0; } @@ -292,6 +294,7 @@ static SysBusDeviceInfo syborg_virtio_net_info = { .qdev.size = sizeof(SyborgVirtIOProxy), .qdev.props = (Property[]) { DEFINE_NIC_PROPERTIES(SyborgVirtIOProxy, nic), +DEFINE_VIRTIO_NET_FEATURES(SyborgVirtIOProxy, host_features), DEFINE_PROP_END_OF_LIST(), } }; diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c index cfd3b41..e17880f 100644 --- a/hw/virtio-balloon.c +++ b/hw/virtio-balloon.c @@ -124,9 +124,9 @@ static void virtio_balloon_set_config(VirtIODevice *vdev, dev-actual =
Re: [Qemu-devel] Re: [PATCH v2] virtio-blk physical block size
On 01/08/2010 05:40 PM, Christoph Hellwig wrote: Maybe we should pick one on image creation and then stick to it. For an image format we could write down this information in the image, but for a raw images that's impossible. The management system should remember it (like it remembers which images belong to which guest, and how to expose them). -- error compiling committee.c: too many arguments to function
[Qemu-devel] New kvm-related qemu patch queue
In order to improve qemu.git kvm integration quality wrt performance, features, and reliability Marcelo and I will begin to maintain a patch queue based on qemu.git containing kvm-related patches. We will review and apply patches to this queue, test them using the same test suite that is used for qemu-kvm.git, and regularly submit them for inclusion in qemu.git, mimicking the relationship between kvm.git and Linus' linux-2.6.git. One of the problems of qemu.git kvm support is that it is a clean reimplementation, and thus some of the nuances that were carefully ironed out in qemu-kvm.git are lost. To that end, we would like to change the process of adding features as follows: - first, the feature in qemu-kvm.git master is morphed to a form suitable for merging into qemu.git - when that has been accomplished, the feature is broken into patches and merged into the patch queue This process ensures that important details are not lost: the morphing step attempts to preserve all the nuances, and it is much easier to spot a behaviour change in a review than to spot a missing behaviour in a new feature. This was successfully used to merge i386 and x86_64 arch support in Linux. Some qemu-kvm.git features (primarily support for very old kernels) can be dropped, simplifying the convergence process. Over time, qemu-kvm.git master will converge with qemu.git master and all that will remain is the patch queue. Features in qemu-kvm.git not present in qemu.git include: - smp support - in-kernel irqchip - in-kernel pit - tpr patching - device assignment - kvm paravirtualization - boot=on (extboot, or a real option rom) - test suite - ia64 - signalfd support Many others are implemented differently in the two trees. The patch queue will appear as a branch 'uq/master' (for 'upstream queue); if necessary branches for the stable series will also be created. In order to get a kvm feature into qemu.git, please observe the following process: - post a patch series against qemu-kvm.git/master that implements the feature, or changes an existing feature to use qemu.git infrastructure - once that's merged, post a patch series against qemu-kvm.git/uq/master that brings the same code into qemu.git. The new branch will be subject to the same automatic testing that qemu-kvm.git is. Please post patches to both qemu-devel and kvm mailing lists. This won't work for all features, so we'll have to feel our way and make decisions on a case by case basis. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
[Qemu-devel] Re: New kvm-related qemu patch queue
On Sun, Jan 10, 2010 at 02:02:27PM +0200, Avi Kivity wrote: In order to improve qemu.git kvm integration quality wrt performance, features, and reliability Marcelo and I will begin to maintain a patch queue based on qemu.git containing kvm-related patches. We will review and apply patches to this queue, test them using the same test suite that is used for qemu-kvm.git, and regularly submit them for inclusion in qemu.git, mimicking the relationship between kvm.git and Linus' linux-2.6.git. One of the problems of qemu.git kvm support is that it is a clean reimplementation, and thus some of the nuances that were carefully ironed out in qemu-kvm.git are lost. To that end, we would like to change the process of adding features as follows: - first, the feature in qemu-kvm.git master is morphed to a form suitable for merging into qemu.git - when that has been accomplished, the feature is broken into patches and merged into the patch queue If there is the same feature in qemu-kvm.git and qemu.git whom is morphed to whom and who is merged where? I am confused. We have much of duplicated code between qemu-kvm.git and qemu.git it is often almost, but not exactly the same. Is it really productive to set rules who should be morphed and how at this point? This process ensures that important details are not lost: the morphing step attempts to preserve all the nuances, and it is much easier to spot a behaviour change in a review than to spot a missing behaviour in a new feature. This was successfully used to merge i386 and x86_64 arch support in Linux. Some qemu-kvm.git features (primarily support for very old kernels) can be dropped, simplifying the convergence process. Over time, qemu-kvm.git master will converge with qemu.git master and all that will remain is the patch queue. Features in qemu-kvm.git not present in qemu.git include: - smp support - in-kernel irqchip - in-kernel pit - tpr patching - device assignment - kvm paravirtualization - boot=on (extboot, or a real option rom) - test suite - ia64 - signalfd support Many others are implemented differently in the two trees. The patch queue will appear as a branch 'uq/master' (for 'upstream queue); if necessary branches for the stable series will also be created. In order to get a kvm feature into qemu.git, please observe the following process: - post a patch series against qemu-kvm.git/master that implements the feature, or changes an existing feature to use qemu.git infrastructure It was other way around till last week. Why change? - once that's merged, post a patch series against qemu-kvm.git/uq/master that brings the same code into qemu.git. The new branch will be subject to the same automatic testing that qemu-kvm.git is. Please post patches to both qemu-devel and kvm mailing lists. This won't work for all features, so we'll have to feel our way and make decisions on a case by case basis. -- Gleb.
[Qemu-devel] Re: New kvm-related qemu patch queue
On 01/10/2010 04:28 PM, Gleb Natapov wrote: On Sun, Jan 10, 2010 at 02:02:27PM +0200, Avi Kivity wrote: In order to improve qemu.git kvm integration quality wrt performance, features, and reliability Marcelo and I will begin to maintain a patch queue based on qemu.git containing kvm-related patches. We will review and apply patches to this queue, test them using the same test suite that is used for qemu-kvm.git, and regularly submit them for inclusion in qemu.git, mimicking the relationship between kvm.git and Linus' linux-2.6.git. One of the problems of qemu.git kvm support is that it is a clean reimplementation, and thus some of the nuances that were carefully ironed out in qemu-kvm.git are lost. To that end, we would like to change the process of adding features as follows: - first, the feature in qemu-kvm.git master is morphed to a form suitable for merging into qemu.git - when that has been accomplished, the feature is broken into patches and merged into the patch queue If there is the same feature in qemu-kvm.git and qemu.git whom is morphed to whom and who is merged where? I am confused. We have much of duplicated code between qemu-kvm.git and qemu.git it is often almost, but not exactly the same. Is it really productive to set rules who should be morphed and how at this point? If the feature is already in both, then morph qemu-kvm.git into what is already in qemu.git. Hopefully anything missing in qemu.git will be discovered while making the changes. In order to get a kvm feature into qemu.git, please observe the following process: - post a patch series against qemu-kvm.git/master that implements the feature, or changes an existing feature to use qemu.git infrastructure It was other way around till last week. Why change? There were a lot of regressions. -- error compiling committee.c: too many arguments to function
[Qemu-devel] Re: New kvm-related qemu patch queue
On 01/10/2010 04:49 PM, Gleb Natapov wrote: If the feature is already in both, then morph qemu-kvm.git into what is already in qemu.git. Hopefully anything missing in qemu.git will be discovered while making the changes. What about bugs that are present only in qemu.git? Like this: http://patchwork.ozlabs.org/patch/42298/. Should it go through qemu-kvm.git/uq/master? Yes. So there is a central place for kvm patches, and so they see autotest. What about this one: http://patchwork.ozlabs.org/patch/42447/ should it be postponed untill qemu.git and qemu-kvm.git converge on using the same cpuid infrastructure, or add similar functionality to qemu-kvm to, or add new cpu flags to qemu-kvm only and when cpuid code converge qemu.git will have it too? Best to make qemu-kvm.git and qemu.git converge first. Duplicating the patch extends the problem. Of course if something is urgent we can bypass the process. -- error compiling committee.c: too many arguments to function
[Qemu-devel] Re: [ kvm-Bugs-2907597 ] qemu vnc server clips at 2560x1600
On 01/10/2010 06:26 PM, SourceForge.net wrote: Initial Comment: So I am running using the VESA driver to run an Ubuntu 9.10 guest at 2560x1600 (I had to modify the xserver-video-vesa package to remove an internal screen limit of 2048x2048 in the xorg vesa driver) and everything works great except that the qemu vnc server appears to clip at this resolution. The problem goes away if I run 1900x1200 and it doesn't change if I run 16bit depth or 24bit depth. I have attached two screenshots, the first is vncing directly into qemu (which exhibits the problem) and the second is vncing to a vnc server I have running in the guest which doesn't have the problem. I poked around in vnc.c and couldn't see any limits but I feel like its a buffer limit of some kind. Also if you look very closely at the first image you can see that the first row is drawn correctly all the way across but subsequent rows are not. If you need more information doesn't hesitate to ask. Anthony, can you take a look at this? Seems like a serious issue, could find nothing obvious in vnc.c. -- Comment By: Avi Kivity (avik) Date: 2010-01-10 18:26 Message: Confirmed with vncviewer and vinagre, so unlikely to be a client problem. -- Comment By: Avi Kivity (avik) Date: 2010-01-10 16:17 Message: The cropping is 512 pixels wide, the correct area is 2048 pixels wide, so it does look like some kind of limit. -- Comment By: SourceForge Robot (sf-robot) Date: 2009-12-28 04:20 Message: This Tracker item was closed automatically by the system. It was previously set to a Pending status, and the original submitter did not respond within 14 days (the time period specified by the administrator of this Tracker). -- Comment By: Matthew Colyer (mcsoccer) Date: 2009-12-17 05:18 Message: The blocks do not show up when run in SDL mode. So I believe the problem is still somehow related to the VNC server. -- Comment By: Avi Kivity (avik) Date: 2009-12-13 12:29 Message: Does it work well with SDL? Maybe the problem is not vnc related. -- You can respond by visiting: https://sourceforge.net/tracker/?func=detailatid=893831aid=2907597group_id=180599 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- error compiling committee.c: too many arguments to function
[Qemu-devel] Re: [PATCH 1/6] Make config space accessor host bus trapable
On Sun, Jan 3, 2010 at 7:18 PM, Blue Swirl blauwir...@gmail.com wrote: On Sun, Jan 3, 2010 at 6:06 PM, Michael S. Tsirkin m...@redhat.com wrote: On Sun, Jan 03, 2010 at 06:50:15PM +0100, Alexander Graf wrote: On 03.01.2010, at 18:44, Michael S. Tsirkin wrote: On Sun, Jan 03, 2010 at 06:40:52PM +0100, Alexander Graf wrote: On 03.01.2010, at 18:29, Michael S. Tsirkin wrote: On Sun, Jan 03, 2010 at 05:09:32PM +0100, Alexander Graf wrote: On 03.01.2010, at 16:45, Michael S. Tsirkin wrote: On Sun, Jan 03, 2010 at 02:50:45AM +0100, Alexander Graf wrote: Different host buses may have different layouts for config space accessors. The Mac U3 for example uses the following define to access Type 0 (directly attached) devices: #define MACRISC_CFA0(devfn, off) \ ((1 (unsigned int)PCI_SLOT(dev_fn)) \ | (((unsigned int)PCI_FUNC(dev_fn)) 8) \ | (((unsigned int)(off)) 0xFCUL)) Obviously, this is different from the encoding we interpret in qemu. In order to let host controllers take some influence there, we can just introduce a converter function that takes whatever accessor we have and makes a qemu understandable one out of it. This patch is the groundwork for Uninorth PCI config space fixes. Signed-off-by: Alexander Graf ag...@suse.de CC: Michael S. Tsirkin m...@redhat.com Thanks! It always bothered me that the code in pci_host uses x86 encoding and other architectures are converted to x86. As long as we are adding redirection, maybe we should get rid of this, and make get_config_reg return register and devfn separately, so we don't need to encode/decode back and forth? Hmm, when touching code I'm not 100% sure what I'm doing I usually try to build on stuff that is there already. That's exactly what happened here. I'm personally not against defining the x86 format as qemu default. That way everyone knows what to deal with. I'm not sure if PCI defines anything that couldn't be represented by the x86 encoding in 32 bits. I actually doubt it. So it seems like a pretty good fit, especially given that all the other code is already in place. OTOH if we are after a quick fix, won't it be simpler to have unin_pci simply use its own routines instead of pci_host_conf_register_mmio etc? Hm, I figured this is less work :-). Let me explain the idea: we have: static void pci_host_config_writel_ioport(void *opaque, uint32_t addr, uint32_t val) { PCIHostState *s = opaque; PCI_DPRINTF(%s addr %PRIx32 val %PRIx32\n, __func__, addr, val); s-config_reg = val; } static uint32_t pci_host_config_readl_ioport(void *opaque, uint32_t addr) { PCIHostState *s = opaque; uint32_t val = s-config_reg; PCI_DPRINTF(%s addr %PRIx32 val %PRIx32\n, __func__, addr, val); return val; } void pci_host_conf_register_ioport(pio_addr_t ioport, PCIHostState *s) { register_ioport_write(ioport, 4, 4, pci_host_config_writel_ioport, s); register_ioport_read(ioport, 4, 4, pci_host_config_readl_ioport, s); } If instead of a simple config_reg = val we translate to pc format here, the rest will work. No? Well, that'd mean I'd have to implement a config_reg read function and do the conversion backwards again there. Or I could just save it off in the unin state ... hmm ... Right. Either way, let's better get this done right. I'd rather want to have a proper framework in place in case someone else stumbles across something similar. Alex There's already ugliness with swap/noswap versions in there. Anything that claims to be a proper framework would need to address that as well IMO. Hm, so you'd rather wait for 5 years to have an all-in-one rework than incrementally improving the existing code? Not really, incremental improvements are good. But what you posted does not seem to clean up most code, what it really does is fixes ppc unin_pci. My feeling was since there's only one user for now maybe it is better to just have device specific code rather than complicate generic code. Makes sense? We need to see several users before we add yet another level of indirection. If there is a level of indirection that solves the swap/noswap ugliness as well that would show this is a good abstraction. I think I even know what a good abstraction would be: decode address on write and keep it in decoded form. I think Sparc64 PBM configuration cycles are also a bit different. It's described in UltraSPARC User's Manual 805-0087, p.325. Currently both QEMU and OpenBIOS just use something common, but as soon as Linux boot gets so far as to probe PCI bus, we'd have to fix that. That time is very close, with latest QEMU and OpenBIOS, PCI probe starts (with GDB tricks for calibrate_delay, which
Re: [Qemu-devel] [PATCH] Workaround for broken OSS_GETVERSION on FreeBSD, part two
On Sun, Jan 10, 2010 at 01:40:18AM +0300, malc wrote: On Sat, 9 Jan 2010, Juergen Lock wrote: Turns out on those versions of FreeBSD (= 7.x) that know OSS_GETVERSION the ioctl doesn't actually work yet (except in the Linuxolator), so if building on FreeBSD fall back to using SOUND_VERSION as defined in sys/soundcard.h (which atm is 0x04) if the ioctl is defined but fails. I've changed the code in the meantime, so this wont apply. Also this is wrong version is dynamic by nature, if someone runs qemu on FreeBSD where SOUND_VERSION is less than 4, things will get ugly. Yes but running packages on an older FreeBSD version than they were built on isn't really supported (read: won't work, especially if its an older major branch too like in this case.) On top of that, SOUND_VERSION was bumped to 0x04 for FreeBSD 7.0 already (the commit is from 2006), and FreeBSD 6 is nothing more than a legacy branch now that I think few ppl still use (or at least for things like qemu), the last release was FreeBSD 8.0... In the new code i mentioned one can opt for not considering POLICY failure a hard error and proceed with a SETFRAGMENT path, which is what i would prefer to see on FreeBSD. Anyway, you are the qemu audio maintainer, so if thats really what you prefer, fine with me. :) Cheers, Juergen
[Qemu-devel] mulu2 not implemented for tcg target sparc
Hi, The tcg mulu2 operation is apparently missing for the sparc target (tcg/sparc/tcg-target.c function tcg_out_op()) Is anyone else working on implementing the missing mulu2 operation? Regards Palle
Re: [Qemu-devel] mulu2 not implemented for tcg target sparc
On Sun, Jan 10, 2010 at 8:20 PM, Palle Lyckegaard pa...@lyckegaard.dk wrote: Hi, The tcg mulu2 operation is apparently missing for the sparc target (tcg/sparc/tcg-target.c function tcg_out_op()) Is anyone else working on implementing the missing mulu2 operation? Is it needed somewhere?
Re: [Qemu-devel] mulu2 not implemented for tcg target sparc
On Sun, Jan 10, 2010 at 9:46 PM, Blue Swirl blauwir...@gmail.com wrote: On Sun, Jan 10, 2010 at 8:20 PM, Palle Lyckegaard pa...@lyckegaard.dk wrote: Hi, The tcg mulu2 operation is apparently missing for the sparc target (tcg/sparc/tcg-target.c function tcg_out_op()) Is anyone else working on implementing the missing mulu2 operation? Is it needed somewhere? It will be generated for instance for some muls when the target is i386 (not even x86_64) on a 32-bit host. Laurent
Re: [Qemu-devel] mulu2 not implemented for tcg target sparc
On Sun, Jan 10, 2010 at 8:50 PM, Laurent Desnogues laurent.desnog...@gmail.com wrote: On Sun, Jan 10, 2010 at 9:46 PM, Blue Swirl blauwir...@gmail.com wrote: On Sun, Jan 10, 2010 at 8:20 PM, Palle Lyckegaard pa...@lyckegaard.dk wrote: Hi, The tcg mulu2 operation is apparently missing for the sparc target (tcg/sparc/tcg-target.c function tcg_out_op()) Is anyone else working on implementing the missing mulu2 operation? Is it needed somewhere? It will be generated for instance for some muls when the target is i386 (not even x86_64) on a 32-bit host. In that case mulu2 support would be useful for Sparc32 host too. Palle, are you planning to dig into this?
Re: [Qemu-devel] mulu2 not implemented for tcg target sparc
On Sun, 10 Jan 2010, Blue Swirl wrote: Is it needed somewhere? I was trying to run qemu-system-mips with a NetBSD malta kernel that generates a MIPS mult operation. Tracing the code through tcg points at a missing mulu2 opreration for sparc. It will be generated for instance for some muls when the target is i386 (not even x86_64) on a 32-bit host. In that case mulu2 support would be useful for Sparc32 host too. Palle, are you planning to dig into this? Yes I will try even if my tcg and SPARC assembly skills and very limited (so far...) Regards Palle
[Qemu-devel] [RFC][PATCH] performance improvement for windows guests, running on top of virtio block device
The following patch allows us to improve Windows virtio block driver performance on small size requests. Additionally, it leads to reducing of cpu usage on write IOs repository: /home/vadimr/work/win7/qemu branch: master commit 68290c4e9c96f345d544ca5d2b89f27a1e67e27a Author: Vadim Rozenfeld vad...@localhost.localdomain Date: Mon Jan 11 09:00:21 2010 +0200 [RFC][PATCH] small IOs performance for windows guests, running on top of virtio block device diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c index a2f0639..0e3a8d5 100644 --- a/hw/virtio-blk.c +++ b/hw/virtio-blk.c @@ -28,6 +28,7 @@ typedef struct VirtIOBlock char serial_str[BLOCK_SERIAL_STRLEN + 1]; QEMUBH *bh; size_t config_size; +unsigned int pending; } VirtIOBlock; static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev) @@ -87,6 +88,8 @@ typedef struct VirtIOBlockReq struct VirtIOBlockReq *next; } VirtIOBlockReq; +static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq); + static void virtio_blk_req_complete(VirtIOBlockReq *req, int status) { VirtIOBlock *s = req-dev; @@ -95,6 +98,11 @@ static void virtio_blk_req_complete(VirtIOBlockReq *req, int status) virtqueue_push(s-vq, req-elem, req-qiov.size + sizeof(*req-in)); virtio_notify(s-vdev, s-vq); +if(--s-pending == 0) { +virtio_queue_set_notification(s-vq, 1); +virtio_blk_handle_output(s-vdev, s-vq); +} + qemu_free(req); } @@ -340,6 +348,9 @@ static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq) exit(1); } +if(++s-pending == 1) +virtio_queue_set_notification(s-vq, 0); + req-out = (void *)req-elem.out_sg[0].iov_base; req-in = (void *)req-elem.in_sg[req-elem.in_num - 1].iov_base;
[Qemu-devel] Re: New kvm-related qemu patch queue
On Sun, Jan 10, 2010 at 04:52:48PM +0200, Avi Kivity wrote: On 01/10/2010 04:49 PM, Gleb Natapov wrote: If the feature is already in both, then morph qemu-kvm.git into what is already in qemu.git. Hopefully anything missing in qemu.git will be discovered while making the changes. What about bugs that are present only in qemu.git? Like this: http://patchwork.ozlabs.org/patch/42298/. Should it go through qemu-kvm.git/uq/master? Yes. So there is a central place for kvm patches, and so they see autotest. So I assume you'll take it then. What about this one: http://patchwork.ozlabs.org/patch/42447/ should it be postponed untill qemu.git and qemu-kvm.git converge on using the same cpuid infrastructure, or add similar functionality to qemu-kvm to, or add new cpu flags to qemu-kvm only and when cpuid code converge qemu.git will have it too? Best to make qemu-kvm.git and qemu.git converge first. Duplicating the patch extends the problem. Of course if something is urgent we can bypass the process. Actually, looking into it, this patch is needed for qemu-kvm.git and qemu.git to converge. For qemu-kvm to use kvm -cpu flags the patch below should be applied on top of the previous patch. Signed-off-by: Gleb Natapov g...@redhat.com diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c index 82e362c..b3c371c 100644 --- a/qemu-kvm-x86.c +++ b/qemu-kvm-x86.c @@ -1308,7 +1308,7 @@ int kvm_arch_init_vcpu(CPUState *cenv) pv_ent = cpuid_ent[cpuid_nent++]; memset(pv_ent, 0, sizeof(*pv_ent)); pv_ent-function = KVM_CPUID_FEATURES; -pv_ent-eax = get_para_features(kvm_context); +pv_ent-eax = cenv-cpuid_kvm_features get_para_features(kvm_context); #endif kvm_trim_features(cenv-cpuid_features, -- Gleb.