[Qemu-devel] Re: [PATCH v4 03/10] x86: Extend validity of cpu_is_bsp
On Thu, Mar 04, 2010 at 02:03:54PM +0200, Gleb Natapov wrote: > > BTW, do real systems allow to hot plug BSP as well? Or how is the case > > handled when you unplug the BSP and then reboot the box? > > > Did you mean hot unplug BSP? OS determines what CPU is BSP by checking > BSP bit in APIC base register. My guess is that there is some pin on CPU > which value is mirrored as BSP bit in APIC base register. Board may have > some logic to check what sockets are populated and chose one of them as > BSP by pulling its pin up. But this is only guess. > Actually this is much more simple: SDM 8.4.1: The MP initialization protocol defines two classes of processors: the bootstrap processor (BSP) and the application processors (APs). Following a power-up or RESET of an MP system, system hardware dynamically selects one of the processors on the system bus as the BSP. The remaining processors are designated as APs. And by "hardware" they mean CPUs themselves over apic BUS. -- Gleb.
[Qemu-devel] virtio block device and sysfs
Hi, I am looking to get in touch with somebody who knows more about the connection between host configuration, qemu, kvm, and the virtio block device driver guest side than I know. My goal is to have a possibility to give a "speaking" name to any block device handed into a guest instance by the host. That name should be visible inside the guest, just as a LV is visible with its name in the system running the LVM. For example I would like to say on the qemu or kvm command line '-drive file=some-file,label=some-label,if=virtio', and have the string "some-label" show up somewhere in /sys/block in the guest, much as /sys/block/sda/device/model shows the hardware vendor and type for a standard SATA disk. The guest could then handle the information passed into it by the host with udev rules, allowing fstab constructs like "mount /dev/virtio/block/by-label/some-label as /usr" Since I don't have pretty much clue about kernel programming, I guess that one would need to have qemu/kvm support for the additional label to be passed to the emulated/virtualized guest, and a modification to the guest kernel virtio driver which needs to accept the information passed by the host and to generate the appropriate entry in /sys. Am I correct in my assumption? Can you say who I need to talk to to get advice about how to implement this (or to have it implemented, if it's easy enough)? Any hints will be appreciated. Greetings Marc -- - Marc Haber | "I don't trust Computers. They | Mailadresse im Header Mannheim, Germany | lose things."Winona Ryder | Fon: *49 621 72739834 Nordisch by Nature | How to make an American Quilt | Fax: *49 3221 2323190
Re: [Qemu-devel] [PATCH] Fix curses interaction with keymaps
On Sun, Feb 28, 2010 at 09:03:00PM +0100, Samuel Thibault wrote:
> Hello,
>
> The combination of keymap support (-k option) and curses is currently
> very broken. The patch below fixes it by first extending keymap support
> to interpret the shift, ctrl, altgr and addupper keywords in keymaps,
> and to fix curses into properly using keymaps.
>
> Samuel
>
> Signed-off-by: Samuel Thibault
>
> commit 36f0635cb65e1735a7e231d609da98efcda756c5
> Author: Samuel Thibault
> Date: Sun Feb 28 20:48:39 2010 +0100
>
> Fix curses with -k option
Thanks, applied.
> diff --git a/curses.c b/curses.c
> index 3ce12b9..4b5beac 100644
> --- a/curses.c
> +++ b/curses.c
> @@ -159,11 +159,10 @@ static void curses_cursor_position(DisplayState *ds,
> int x, int y)
> #include "curses_keys.h"
>
> static kbd_layout_t *kbd_layout = NULL;
> -static int keycode2keysym[CURSES_KEYS];
>
> static void curses_refresh(DisplayState *ds)
> {
> -int chr, nextchr, keysym, keycode;
> +int chr, nextchr, keysym, keycode, keycode_alt;
>
> if (invalidate) {
> clear();
> @@ -204,43 +203,58 @@ static void curses_refresh(DisplayState *ds)
> #endif
>
> keycode = curses2keycode[chr];
> -if (keycode == -1)
> -continue;
> +keycode_alt = 0;
>
> /* alt key */
> if (keycode == 1) {
> nextchr = getch();
>
> if (nextchr != ERR) {
> +chr = nextchr;
> +keycode_alt = ALT;
> keycode = curses2keycode[nextchr];
> nextchr = ERR;
> -if (keycode == -1)
> -continue;
>
> -keycode |= ALT;
> +if (keycode != -1) {
> +keycode |= ALT;
>
> -/* process keys reserved for qemu */
> -if (keycode >= QEMU_KEY_CONSOLE0 &&
> -keycode < QEMU_KEY_CONSOLE0 + 9) {
> -erase();
> -wnoutrefresh(stdscr);
> -console_select(keycode - QEMU_KEY_CONSOLE0);
> +/* process keys reserved for qemu */
> +if (keycode >= QEMU_KEY_CONSOLE0 &&
> +keycode < QEMU_KEY_CONSOLE0 + 9) {
> +erase();
> +wnoutrefresh(stdscr);
> +console_select(keycode - QEMU_KEY_CONSOLE0);
>
> -invalidate = 1;
> -continue;
> +invalidate = 1;
> +continue;
> +}
> }
> }
> }
>
> -if (kbd_layout && !(keycode & GREY)) {
> -keysym = keycode2keysym[keycode & KEY_MASK];
> -if (keysym == -1)
> -keysym = chr;
> +if (kbd_layout) {
> +keysym = -1;
> +if (chr < CURSES_KEYS)
> +keysym = curses2keysym[chr];
> +
> +if (keysym == -1) {
> +if (chr < ' ')
> +keysym = (chr + '@' - 'A' + 'a') | KEYSYM_CNTRL;
> +else
> +keysym = chr;
> +}
>
> -keycode &= ~KEY_MASK;
> -keycode |= keysym2scancode(kbd_layout, keysym);
> +keycode = keysym2scancode(kbd_layout, keysym & KEYSYM_MASK);
> +if (keycode == 0)
> +continue;
> +
> +keycode |= (keysym & ~KEYSYM_MASK) >> 16;
> +keycode |= keycode_alt;
> }
>
> +if (keycode == -1)
> +continue;
> +
> if (is_graphic_console()) {
> /* since terminals don't know about key press and release
> * events, we need to emit both for each key received */
> @@ -250,12 +264,20 @@ static void curses_refresh(DisplayState *ds)
> kbd_put_keycode(CNTRL_CODE);
> if (keycode & ALT)
> kbd_put_keycode(ALT_CODE);
> +if (keycode & ALTGR) {
> +kbd_put_keycode(SCANCODE_EMUL0);
> +kbd_put_keycode(ALT_CODE);
> +}
> if (keycode & GREY)
> kbd_put_keycode(GREY_CODE);
> kbd_put_keycode(keycode & KEY_MASK);
> if (keycode & GREY)
> kbd_put_keycode(GREY_CODE);
> kbd_put_keycode((keycode & KEY_MASK) | KEY_RELEASE);
> +if (keycode & ALTGR) {
> +kbd_put_keycode(SCANCODE_EMUL0);
> +kbd_put_keycode(ALT_CODE | KEY_RELEASE);
> +}
> if (keycode & ALT)
> kbd_put_keycode(ALT_CODE | KEY_RELEASE);
> if (keycode & CNTRL)
> @@ -263,7 +285,7 @@ static void curses_refresh(DisplayState *ds)
> if (keycode & SHIFT)
> kbd_put_keycode(SHIFT_CODE | KEY_RELEASE);
> } else {
> -keysym = curses2keysym[chr];
> +key
Re: [Qemu-devel] [PATCH] use absolute URLs for .gitmodules
On Fri, Mar 05, 2010 at 09:08:04AM +0100, Paolo Bonzini wrote: > The relative URLs do not work when cloning a fork of qemu or when > cloning from the Savannah URL. > > Signed-off-by: Paolo Bonzini > > Signed-off-by: Paolo Bonzini > --- > .gitmodules |4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/.gitmodules b/.gitmodules > index dd4745e..5217ce7 100644 > --- a/.gitmodules > +++ b/.gitmodules > @@ -1,6 +1,6 @@ > [submodule "roms/vgabios"] > path = roms/vgabios > - url = ../vgabios.git > + url = git://git.qemu.org/vgabios.git/ > [submodule "roms/seabios"] > path = roms/seabios > - url = ../seabios.git > + url = git://git.qemu.org/seabios.git/ Is the slash at the end of the URL really wanted? -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH] scsi: update comment on the standards revision
On Thu, Mar 04, 2010 at 02:45:44PM +0100, Christoph Hellwig wrote: > Signed-off-by: Christoph Hellwig Thanks, applied. > Index: qemu/hw/scsi-disk.c > === > --- qemu.orig/hw/scsi-disk.c 2010-03-04 14:39:43.699022260 +0100 > +++ qemu/hw/scsi-disk.c 2010-03-04 14:41:26.768255602 +0100 > @@ -462,8 +462,12 @@ static int scsi_disk_emulate_inquiry(SCS > } > memcpy(&outbuf[8], "QEMU", 8); > memcpy(&outbuf[32], s->version ? s->version : QEMU_VERSION, 4); > -/* Identify device as SCSI-3 rev 1. > - Some later commands are also implemented. */ > +/* > + * We claim conformance to SPC-3, which is required for guests > + * to ask for modern features like READ CAPACITY(16) or the > + * block characteristics VPD page by default. Not all of SPC-3 > + * is actually implemented, but we're good enough. > + */ > outbuf[2] = 5; > outbuf[3] = 2; /* Format 2 */ > > > > -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH] qemu-nbd: Fix wrong description in qemu-nbd.texi
On Thu, Mar 04, 2010 at 12:18:43AM +0900, Ryota Ozaki wrote:
> -c option needs argument but it's missing now.
> This patch fixes it.
>
> Signed-off-by: Ryota Ozaki
Thanks, applied.
> ---
> qemu-nbd.texi |4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-nbd.texi b/qemu-nbd.texi
> index ef13b34..44996cc 100644
> --- a/qemu-nbd.texi
> +++ b/qemu-nbd.texi
> @@ -30,8 +30,8 @@ Export Qemu disk image using NBD protocol.
>use snapshot file
> @item -n, --nocache
>disable host cache
> -...@item -c, --connect
> - connect FILE to NBD device DEV
> +...@item -c, --conne...@var{dev}
> + connect @var{filename} to NBD device @var{dev}
> @item -d, --disconnect
>disconnect the specified device
> @item -e, --shar...@var{num}
> --
> 1.6.5.2
>
>
>
>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH] Build usb-ohci for PCs
On Wed, Mar 03, 2010 at 04:02:25PM +0100, Kevin Wolf wrote: > The OHCI emulation isn't obviously broken and there are people who want to use > it. Let's build it by default so that it can be enabled via -device. > > Signed-off-by: Kevin Wolf Thanks, applied. > --- > default-configs/i386-softmmu.mak |2 ++ > default-configs/x86_64-softmmu.mak |2 ++ > 2 files changed, 4 insertions(+), 0 deletions(-) > > diff --git a/default-configs/i386-softmmu.mak > b/default-configs/i386-softmmu.mak > index 15586a0..9e6d565 100644 > --- a/default-configs/i386-softmmu.mak > +++ b/default-configs/i386-softmmu.mak > @@ -1 +1,3 @@ > # Default configuration for i386-softmmu > + > +CONFIG_USB_OHCI=y > diff --git a/default-configs/x86_64-softmmu.mak > b/default-configs/x86_64-softmmu.mak > index ec98af2..e0f7fc0 100644 > --- a/default-configs/x86_64-softmmu.mak > +++ b/default-configs/x86_64-softmmu.mak > @@ -1 +1,3 @@ > # Default configuration for x86_64-softmmu > + > +CONFIG_USB_OHCI=y > -- > 1.6.6.1 > > > > -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH 2/2] ppc440_bamboo: Disable new virtio-serial features for 0.12 machine type
On Thu, Feb 25, 2010 at 07:26:12PM +0530, Amit Shah wrote:
> Disable the MULTIPORT feature and MSI vectors for the 0.12 machine
> types; those features are added only for 0.13 onwards.
>
> Signed-off-by: Amit Shah
Thanks, applied.
> ---
> hw/ppc440_bamboo.c | 12
> 1 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/hw/ppc440_bamboo.c b/hw/ppc440_bamboo.c
> index aa9f594..db96f14 100644
> --- a/hw/ppc440_bamboo.c
> +++ b/hw/ppc440_bamboo.c
> @@ -183,6 +183,18 @@ static QEMUMachine bamboo_machine_v0_12 = {
> .name = "bamboo-0.12",
> .desc = "bamboo",
> .init = bamboo_init,
> +.compat_props = (GlobalProperty[]) {
> +{
> +.driver = "virtio-serial-pci",
> +.property = "max_nr_ports",
> +.value= stringify(1),
> +},{
> +.driver = "virtio-serial-pci",
> +.property = "vectors",
> +.value= stringify(0),
> +},
> +{ /* end of list */ }
> +},
> };
>
> static void bamboo_machine_init(void)
> --
> 1.6.2.5
>
>
>
>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH 1/2] ppc440_bamboo: Add 0.12 and 0.13 machine types for backward compat
On Thu, Feb 25, 2010 at 07:26:11PM +0530, Amit Shah wrote:
> Add a 0.12 machine type for compatibility with older versions. Mark the
> default one as 0.13.
>
> Signed-off-by: Amit Shah
Thanks, applied.
> ---
> hw/ppc440_bamboo.c | 11 ++-
> 1 files changed, 10 insertions(+), 1 deletions(-)
>
> diff --git a/hw/ppc440_bamboo.c b/hw/ppc440_bamboo.c
> index 1ab9872..aa9f594 100644
> --- a/hw/ppc440_bamboo.c
> +++ b/hw/ppc440_bamboo.c
> @@ -172,7 +172,15 @@ static void bamboo_init(ram_addr_t ram_size,
> }
>
> static QEMUMachine bamboo_machine = {
> -.name = "bamboo",
> +.name = "bamboo-0.13",
> +.alias = "bamboo",
> +.desc = "bamboo",
> +.init = bamboo_init,
> +.is_default = 1,
> +};
> +
> +static QEMUMachine bamboo_machine_v0_12 = {
> +.name = "bamboo-0.12",
> .desc = "bamboo",
> .init = bamboo_init,
> };
> @@ -180,6 +188,7 @@ static QEMUMachine bamboo_machine = {
> static void bamboo_machine_init(void)
> {
> qemu_register_machine(&bamboo_machine);
> +qemu_register_machine(&bamboo_machine_v0_12);
> }
>
> machine_init(bamboo_machine_init);
> --
> 1.6.2.5
>
>
>
>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH 1/3] s390-virtio: Fix compile error for virtio-block init
On Thu, Feb 25, 2010 at 07:15:18PM +0530, Amit Shah wrote:
> Commit 428c149b0be790b440e1cbee185b152cdb22feec modified the argument
> that virtio_blk_init takes. Update the s390 bus code that calls this
> function.
>
> Signed-off-by: Amit Shah
> CC: Christoph Hellwig
Thanks, applied.
> ---
> hw/s390-virtio-bus.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
> index fa0a74f..9fc01e9 100644
> --- a/hw/s390-virtio-bus.c
> +++ b/hw/s390-virtio-bus.c
> @@ -123,7 +123,7 @@ static int s390_virtio_blk_init(VirtIOS390Device *dev)
> {
> VirtIODevice *vdev;
>
> -vdev = virtio_blk_init((DeviceState *)dev, dev->block.dinfo);
> +vdev = virtio_blk_init((DeviceState *)dev, &dev->block);
> if (!vdev) {
> return -1;
> }
> --
> 1.6.2.5
>
>
>
>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH] json-parser: Fix segfault on malformed input
On Wed, Feb 24, 2010 at 04:17:58PM +0100, Kevin Wolf wrote:
> If the parser fails to parse the key in parse_pair, it will access a NULL
> pointer. A simple way to trigger this is sending {foo} via QMP. This patch
> turns the segfault into a syntax error reply.
>
> Signed-off-by: Kevin Wolf
Thanks, applied.
> ---
> json-parser.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/json-parser.c b/json-parser.c
> index f3debcb..579928f 100644
> --- a/json-parser.c
> +++ b/json-parser.c
> @@ -264,7 +264,7 @@ static int parse_pair(JSONParserContext *ctxt, QDict
> *dict, QList **tokens, va_l
>
> peek = qlist_peek(working);
> key = parse_value(ctxt, &working, ap);
> -if (qobject_type(key) != QTYPE_QSTRING) {
> +if (!key || qobject_type(key) != QTYPE_QSTRING) {
> parse_error(ctxt, peek, "key is not a string in object");
> goto out;
> }
> --
> 1.6.6.1
>
>
>
>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH] Fix segfault with ram_size > 4095M without kvm
On Thu, Mar 04, 2010 at 03:34:34PM -0600, Ryan Harper wrote:
> * Aurelien Jarno [2010-03-04 15:27]:
> > On Tue, Feb 23, 2010 at 06:02:15PM +0100, Aurelien Jarno wrote:
> > > Ryan Harper a écrit :
> > > > Currently, x86_64-softmmu qemu segfaults when trying to use > 4095M
> > > > memsize.
> > > > This patch adds a simple check and error message (much like the 2047
> > > > limit on
> > > > 32-bit hosts) on ram_size in the control path after we determine we're
> > > > not using kvm
> > > >
> > > > Upstream qemu-kvm is affected if using the -no-kvm option; this patch
> > > > address
> > > > the segfault there as well.
> > >
> > > It looks like workarounding the real bug. At some point both
> > > i386-softmmu (via PAE) and x86_64-softmmu were able to support > 4GB of
> > > memory. I remember adding the support long time ago, and testing it with
> > > 32GB of emulated RAM.
> >
> > I have looked into that, and actually one patch to get full support for
> > > 4GB of memory was not merged:
>
> Thanks for looking into this.
>
> >
> > diff --git a/exec.c b/exec.c
> > index 8389c54..b0bb058 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -166,7 +166,7 @@ typedef struct PhysPageDesc {
> > */
> > #define L1_BITS (TARGET_VIRT_ADDR_SPACE_BITS - L2_BITS - TARGET_PAGE_BITS)
> > #else
> > -#define L1_BITS (32 - L2_BITS - TARGET_PAGE_BITS)
> > +#define L1_BITS (TARGET_PHYS_ADDR_SPACE_BITS - L2_BITS - TARGET_PAGE_BITS)
> > #endif
> >
> > #define L1_SIZE (1 << L1_BITS)
> >
> > While this patch is acceptable for qemu i386, it creates a big L1 table
> > for x86_64 or other 64-bit architectures, resulting in huge memory
> > overhead.
> >
> > The recent multilevel tables patches from Richard Henderson should fix
> > the problem for HEAD (I haven't found time to look at them in details).
> >
> > As this is not something we really want to backport, your patch makes
> > sense in stable-0.12.
>
> Anthony, do you want me to resend and rebase against 0.12-stable?
>
The patch applies correctly on stable-0.12. I have just applied it.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
[Qemu-devel] Re: [PATCHv4 05/12] virtio: add APIs for queue fields
On Fri, Mar 05, 2010 at 05:40:18PM +0530, Amit Shah wrote:
> On (Wed) Mar 03 2010 [19:16:09], Michael S. Tsirkin wrote:
> > vhost needs physical addresses for ring and other queue fields,
> > so add APIs for these.
>
> Already discussed on IRC, but mentioning here so that it doesn't get
> lost:
>
> > +target_phys_addr_t virtio_queue_get_desc(VirtIODevice *vdev, int n)
> > +{
> > +return vdev->vq[n].vring.desc;
> > +}
> > +
> > +target_phys_addr_t virtio_queue_get_avail(VirtIODevice *vdev, int n)
> > +{
> > +return vdev->vq[n].vring.avail;
> > +}
> > +
> > +target_phys_addr_t virtio_queue_get_used(VirtIODevice *vdev, int n)
> > +{
> > +return vdev->vq[n].vring.used;
> > +}
> > +
> > +target_phys_addr_t virtio_queue_get_ring(VirtIODevice *vdev, int n)
> > +{
> > +return vdev->vq[n].vring.desc;
> > +}
>
> All these functions return the start address of these fields; any better
> way to name them?
>
> eg, just by looking at, eg, 'virtio_queue_get_used()', I'd expect that
> the function returns the number of used buffers in the ring, not the
> start address of the used buffers.
>
> > +target_phys_addr_t virtio_queue_get_desc_size(VirtIODevice *vdev, int n)
> > +{
> > +return sizeof(VRingDesc) * vdev->vq[n].vring.num;
> > +}
> > +
> > +target_phys_addr_t virtio_queue_get_avail_size(VirtIODevice *vdev, int n)
> > +{
> > +return offsetof(VRingAvail, ring) +
> > +sizeof(u_int64_t) * vdev->vq[n].vring.num;
> > +}
> > +
> > +target_phys_addr_t virtio_queue_get_used_size(VirtIODevice *vdev, int n)
> > +{
> > +return offsetof(VRingUsed, ring) +
> > +sizeof(VRingUsedElem) * vdev->vq[n].vring.num;
> > +}
> > +
> > +
>
> Extra newline
>
> > +target_phys_addr_t virtio_queue_get_ring_size(VirtIODevice *vdev, int n)
> > +{
> > +return vdev->vq[n].vring.used - vdev->vq[n].vring.desc +
> > + virtio_queue_get_used_size(vdev, n);
> > +}
> > +
> > +uint16_t virtio_queue_last_avail_idx(VirtIODevice *vdev, int n)
> > +{
> > +return vdev->vq[n].last_avail_idx;
> > +}
> > +
> > +void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t
> > idx)
> > +{
> > +vdev->vq[n].last_avail_idx = idx;
> > +}
>
> virtio_queue_last_avail_idx() does make sense, but since you have a
> 'set_last_avail_idx', better name the previous one 'get_..'?
>
> > +VirtQueue *virtio_queue(VirtIODevice *vdev, int n)
> > +{
> > +return vdev->vq + n;
> > +}
>
> This really doesn't mean anything; I suggest virtio_queue_get_vq().
>
> > +EventNotifier *virtio_queue_guest_notifier(VirtQueue *vq)
> > +{
> > +return &vq->guest_notifier;
> > +}
> > +EventNotifier *virtio_queue_host_notifier(VirtQueue *vq)
> > +{
> > +return &vq->host_notifier;
> > +}
>
> Why drop the 'get_' for these functions?
>
> virtio_queue_get_guest_notifier()
> and
> virtio_queue_get_host_notifier()
>
> might be better.
>
> Amit
Not sure we want 'get' all around, but I'll think about the names, thanks!
--
MST
[Qemu-devel] Re: [PATCHv4 05/12] virtio: add APIs for queue fields
On Fri, Mar 05, 2010 at 06:38:35PM +0530, Amit Shah wrote:
> On (Wed) Mar 03 2010 [19:16:09], Michael S. Tsirkin wrote:
> > +
> > +EventNotifier *virtio_queue_guest_notifier(VirtQueue *vq)
> > +{
> > +return &vq->guest_notifier;
> > +}
> > +EventNotifier *virtio_queue_host_notifier(VirtQueue *vq)
> > +{
> > +return &vq->host_notifier;
> > +}
>
> Where are these host_notifier and guest_notifier fields set? Am I
> completely missing it?
>
> Amit
What do you mean by "set"?
You get a pointer, you can e.g. init it.
[Qemu-devel] Re: [PATCHv4 09/12] vhost: vhost net support
On Fri, Mar 05, 2010 at 11:49:11PM +0530, Amit Shah wrote:
> On (Wed) Mar 03 2010 [19:16:35], Michael S. Tsirkin wrote:
>
> > +static int vhost_virtqueue_init(struct vhost_dev *dev,
> > +struct VirtIODevice *vdev,
> > +struct vhost_virtqueue *vq,
> > +unsigned idx)
> > +{
> > +target_phys_addr_t s, l, a;
> > +int r;
> > +struct vhost_vring_file file = {
> > +.index = idx,
> > +};
> > +struct vhost_vring_state state = {
> > +.index = idx,
> > +};
> > +struct VirtQueue *q = virtio_queue(vdev, idx);
>
> Why depart from using 'vq' for VirtQueue? Why not use 'hvq' for
> vhost_virtqueue? That'll make reading through this code easier... Also,
> 'hvdev' for vhost_dev will be apt as well.
I'll think of better names, thanks.
> > +vq->num = state.num = virtio_queue_get_num(vdev, idx);
>
> I think this should be named 'virtio_queue_get_vq_num' for clarity.
This is the name upstream. If you like, send a patch to rename it :).
> > +r = ioctl(dev->control, VHOST_SET_VRING_NUM, &state);
> > +if (r) {
> > +return -errno;
> > +}
> > +
> > +state.num = virtio_queue_last_avail_idx(vdev, idx);
> > +r = ioctl(dev->control, VHOST_SET_VRING_BASE, &state);
> > +if (r) {
> > +return -errno;
> > +}
> > +
> > +s = l = virtio_queue_get_desc_size(vdev, idx);
> > +a = virtio_queue_get_desc(vdev, idx);
> > +vq->desc = cpu_physical_memory_map(a, &l, 0);
> > +if (!vq->desc || l != s) {
> > +r = -ENOMEM;
> > +goto fail_alloc_desc;
> > +}
> > +s = l = virtio_queue_get_avail_size(vdev, idx);
> > +a = virtio_queue_get_avail(vdev, idx);
> > +vq->avail = cpu_physical_memory_map(a, &l, 0);
> > +if (!vq->avail || l != s) {
> > +r = -ENOMEM;
> > +goto fail_alloc_avail;
> > +}
> > +vq->used_size = s = l = virtio_queue_get_used_size(vdev, idx);
> > +vq->used_phys = a = virtio_queue_get_used(vdev, idx);
> > +vq->used = cpu_physical_memory_map(a, &l, 1);
> > +if (!vq->used || l != s) {
> > +r = -ENOMEM;
> > +goto fail_alloc_used;
> > +}
> > +
> > +vq->ring_size = s = l = virtio_queue_get_ring_size(vdev, idx);
> > +vq->ring_phys = a = virtio_queue_get_ring(vdev, idx);
> > +vq->ring = cpu_physical_memory_map(a, &l, 1);
> > +if (!vq->ring || l != s) {
> > +r = -ENOMEM;
> > +goto fail_alloc_ring;
> > +}
> > +
> > +r = vhost_virtqueue_set_addr(dev, vq, idx, dev->log_enabled);
> > +if (r < 0) {
> > +r = -errno;
> > +goto fail_alloc;
> > +}
> > +if (!vdev->binding->guest_notifier || !vdev->binding->host_notifier) {
> > +fprintf(stderr, "binding does not support irqfd/queuefd\n");
> > +r = -ENOSYS;
> > +goto fail_alloc;
> > +}
>
> This could be checked much earlier on in the function; so that we avoid
> doing all that stuff above and the cleanup.
>
> Amit
Whatever order we put checks in, we'll have to undo stuff
done beforehand on error.
--
MST
Re: [Qemu-devel] [PATCH] sh4 linux-user: Save/restore fpu registers to signal context.
On Thu, Feb 18, 2010 at 12:46:45AM +0900, takas...@ops.dti.ne.jp wrote:
> As "todo" comment in source code.
> And modify restore_sigcontext() to have three args as kernel's does.
>
> Signed-off-by: Takashi YOSHII
Thanks, applied.
> ---
> linux-user/signal.c | 27 +++
> 1 files changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index b0faf2e..8697511 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -2812,6 +2812,7 @@ static int setup_sigcontext(struct target_sigcontext
> *sc,
> CPUState *regs, unsigned long mask)
> {
> int err = 0;
> +int i;
>
> #define COPY(x) err |= __put_user(regs->x, &sc->sc_##x)
> COPY(gregs[0]); COPY(gregs[1]);
> @@ -2827,7 +2828,10 @@ static int setup_sigcontext(struct target_sigcontext
> *sc,
> COPY(sr); COPY(pc);
> #undef COPY
>
> -/* todo: save FPU registers here */
> +for (i=0; i<16; i++)
> +err |= __put_user(regs->fregs[i], &sc->sc_fpregs[i]);
> +err |= __put_user(regs->fpscr, &sc->sc_fpscr);
> +err |= __put_user(regs->fpul, &sc->sc_fpul);
>
> /* non-iBCS2 extensions.. */
> err |= __put_user(mask, &sc->oldmask);
> @@ -2835,10 +2839,11 @@ static int setup_sigcontext(struct target_sigcontext
> *sc,
> return err;
> }
>
> -static int restore_sigcontext(CPUState *regs,
> - struct target_sigcontext *sc)
> +static int restore_sigcontext(CPUState *regs, struct target_sigcontext *sc,
> + target_ulong *r0_p)
> {
> unsigned int err = 0;
> +int i;
>
> #define COPY(x) err |= __get_user(regs->x, &sc->sc_##x)
> COPY(gregs[1]);
> @@ -2854,9 +2859,13 @@ static int restore_sigcontext(CPUState *regs,
> COPY(sr); COPY(pc);
> #undef COPY
>
> -/* todo: restore FPU registers here */
> +for (i=0; i<16; i++)
> +err |= __get_user(regs->fregs[i], &sc->sc_fpregs[i]);
> +err |= __get_user(regs->fpscr, &sc->sc_fpscr);
> +err |= __get_user(regs->fpul, &sc->sc_fpul);
>
> regs->tra = -1; /* disable syscall checks */
> +err |= __get_user(*r0_p, &sc->sc_gregs[0]);
> return err;
> }
>
> @@ -2980,6 +2989,7 @@ long do_sigreturn(CPUState *regs)
> abi_ulong frame_addr;
> sigset_t blocked;
> target_sigset_t target_set;
> +target_ulong r0;
> int i;
> int err = 0;
>
> @@ -3001,11 +3011,11 @@ long do_sigreturn(CPUState *regs)
> target_to_host_sigset_internal(&blocked, &target_set);
> sigprocmask(SIG_SETMASK, &blocked, NULL);
>
> -if (restore_sigcontext(regs, &frame->sc))
> +if (restore_sigcontext(regs, &frame->sc, &r0))
> goto badframe;
>
> unlock_user_struct(frame, frame_addr, 0);
> -return regs->gregs[0];
> +return r0;
>
> badframe:
> unlock_user_struct(frame, frame_addr, 0);
> @@ -3018,6 +3028,7 @@ long do_rt_sigreturn(CPUState *regs)
> struct target_rt_sigframe *frame;
> abi_ulong frame_addr;
> sigset_t blocked;
> +target_ulong r0;
>
> #if defined(DEBUG_SIGNAL)
> fprintf(stderr, "do_rt_sigreturn\n");
> @@ -3029,7 +3040,7 @@ long do_rt_sigreturn(CPUState *regs)
> target_to_host_sigset(&blocked, &frame->uc.uc_sigmask);
> sigprocmask(SIG_SETMASK, &blocked, NULL);
>
> -if (restore_sigcontext(regs, &frame->uc.uc_mcontext))
> +if (restore_sigcontext(regs, &frame->uc.uc_mcontext, &r0))
> goto badframe;
>
> if (do_sigaltstack(frame_addr +
> @@ -3038,7 +3049,7 @@ long do_rt_sigreturn(CPUState *regs)
> goto badframe;
>
> unlock_user_struct(frame, frame_addr, 0);
> -return regs->gregs[0];
> +return r0;
>
> badframe:
> unlock_user_struct(frame, frame_addr, 0);
> --
> 1.6.5
>
>
>
>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH 05/13] cpuid: add missing CPUID feature flag names
On Tue, Feb 02, 2010 at 11:08:13AM +0100, Andre Przywara wrote:
> Some CPUID feature flags had no string value, so they could not be
> switched on or off from the command line.
> Add names for the missing ones mentioned in the current public CPUID
> specification from both Intel and AMD. Those only mentioned in the
> Linux kernel source I put as comments.
>
> Signed-off-by: Andre Przywara
Acked-by: Aurelien Jarno
> ---
> target-i386/cpuid.c | 15 ---
> 1 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
> index 0238718..19d58e1 100644
> --- a/target-i386/cpuid.c
> +++ b/target-i386/cpuid.c
> @@ -52,11 +52,11 @@ static const char *feature_name[] = {
> "fxsr", "sse", "sse2", "ss", "ht" /* Intel htt */, "tm", "ia64", "pbe",
> };
> static const char *ext_feature_name[] = {
> -"pni" /* Intel,AMD sse3 */, NULL, NULL, "monitor",
> -"ds_cpl", "vmx", NULL /* Linux smx */, "est",
> -"tm2", "ssse3", "cid", NULL, NULL, "cx16", "xtpr", NULL,
> -NULL, NULL, "dca", NULL, NULL, NULL, NULL, "popcnt",
> -NULL, NULL, NULL, NULL, NULL, NULL, NULL, "hypervisor",
> +"pni" /* Intel,AMD sse3 */, "pclmuldq", "dtes64", "monitor",
> +"ds_cpl", "vmx", "smx", "est",
> +"tm2", "ssse3", "cid", NULL, NULL /* FMA */, "cx16", "xtpr", "pdcm",
> +NULL, NULL, "dca", "sse4_1", "sse4_2", "x2apic", "movbe", "popcnt",
> +NULL, "aes", "xsave", "osxsave", NULL /* AVX */, NULL, NULL,
> "hypervisor",
> };
> static const char *ext2_feature_name[] = {
> "fpu", "vme", "de", "pse", "tsc", "msr", "pae", "mce",
> @@ -71,8 +71,9 @@ static const char *ext3_feature_name[] = {
> "lahf_lm" /* AMD LahfSahf */, "cmp_legacy",
> "svm", "extapic" /* AMD ExtApicSpace */,
> "cr8legacy" /* AMD AltMovCr8 */, "abm", "sse4a", "misalignsse",
> -"3dnowprefetch", "osvw", NULL /* Linux ibs */, NULL, "skinit", "wdt",
> NULL, NULL,
> -NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> +"3dnowprefetch", "osvw", "ibs", NULL /* SSE-5 */,
> +"skinit", "wdt", NULL, NULL,
> +NULL, NULL, NULL, "nodeid_msr", NULL, NULL, NULL, NULL,
> NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> };
>
> --
> 1.6.4
>
>
>
>
>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH 03/13] cpuid: moved host_cpuid function and remove prototype
On Tue, Feb 02, 2010 at 11:08:11AM +0100, Andre Przywara wrote:
> the host_cpuid function was located at the end of the file and had
> a prototype before it's first use. Move it up and remove the
> prototype.
>
> Signed-off-by: Andre Przywara
Acked-by: Aurelien Jarno
> ---
> target-i386/cpuid.c | 70 --
> 1 files changed, 34 insertions(+), 36 deletions(-)
>
> diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
> index 0a17020..cc080f4 100644
> --- a/target-i386/cpuid.c
> +++ b/target-i386/cpuid.c
> @@ -338,8 +338,40 @@ static x86_def_t x86_defs[] = {
> },
> };
>
> -static void host_cpuid(uint32_t function, uint32_t count, uint32_t *eax,
> - uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
> +static void host_cpuid(uint32_t function, uint32_t count,
> + uint32_t *eax, uint32_t *ebx,
> + uint32_t *ecx, uint32_t *edx)
> +{
> +#if defined(CONFIG_KVM)
> +uint32_t vec[4];
> +
> +#ifdef __x86_64__
> +asm volatile("cpuid"
> + : "=a"(vec[0]), "=b"(vec[1]),
> + "=c"(vec[2]), "=d"(vec[3])
> + : "0"(function), "c"(count) : "cc");
> +#else
> +asm volatile("pusha \n\t"
> + "cpuid \n\t"
> + "mov %%eax, 0(%2) \n\t"
> + "mov %%ebx, 4(%2) \n\t"
> + "mov %%ecx, 8(%2) \n\t"
> + "mov %%edx, 12(%2) \n\t"
> + "popa"
> + : : "a"(function), "c"(count), "S"(vec)
> + : "memory", "cc");
> +#endif
> +
> +if (eax)
> + *eax = vec[0];
> +if (ebx)
> + *ebx = vec[1];
> +if (ecx)
> + *ecx = vec[2];
> +if (edx)
> + *edx = vec[3];
> +#endif
> +}
>
> static int cpu_x86_fill_model_id(char *str)
> {
> @@ -578,40 +610,6 @@ int cpu_x86_register (CPUX86State *env, const char
> *cpu_model)
> return 0;
> }
>
> -static void host_cpuid(uint32_t function, uint32_t count,
> - uint32_t *eax, uint32_t *ebx,
> - uint32_t *ecx, uint32_t *edx)
> -{
> -#if defined(CONFIG_KVM)
> -uint32_t vec[4];
> -
> -#ifdef __x86_64__
> -asm volatile("cpuid"
> - : "=a"(vec[0]), "=b"(vec[1]),
> - "=c"(vec[2]), "=d"(vec[3])
> - : "0"(function), "c"(count) : "cc");
> -#else
> -asm volatile("pusha \n\t"
> - "cpuid \n\t"
> - "mov %%eax, 0(%2) \n\t"
> - "mov %%ebx, 4(%2) \n\t"
> - "mov %%ecx, 8(%2) \n\t"
> - "mov %%edx, 12(%2) \n\t"
> - "popa"
> - : : "a"(function), "c"(count), "S"(vec)
> - : "memory", "cc");
> -#endif
> -
> -if (eax)
> - *eax = vec[0];
> -if (ebx)
> - *ebx = vec[1];
> -if (ecx)
> - *ecx = vec[2];
> -if (edx)
> - *edx = vec[3];
> -#endif
> -}
>
> static void get_cpuid_vendor(CPUX86State *env, uint32_t *ebx,
> uint32_t *ecx, uint32_t *edx)
> --
> 1.6.4
>
>
>
>
>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH 02/13] cpuid: replace magic number with named constant
On Tue, Feb 02, 2010 at 11:08:10AM +0100, Andre Przywara wrote:
> CPUID leaf Fn8000_0001.EDX contains a copy of many Fn_0001.EDX bits.
> Define a name for the mask to improve readability and avoid typos.
>
> Signed-off-by: Andre Przywara
Acked-by: Aurelien Jarno
> ---
> target-i386/cpuid.c | 11 ++-
> 1 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
> index aaa14ba..0a17020 100644
> --- a/target-i386/cpuid.c
> +++ b/target-i386/cpuid.c
> @@ -130,6 +130,7 @@ typedef struct x86_def_t {
>CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_PGE | CPUID_CMOV | \
>CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
>CPUID_PAE | CPUID_SEP | CPUID_APIC)
> +#define EXT2_FEATURE_MASK 0x0183F3FF
> static x86_def_t x86_defs[] = {
> #ifdef TARGET_X86_64
> {
> @@ -147,7 +148,7 @@ static x86_def_t x86_defs[] = {
> /* this feature is needed for Solaris and isn't fully implemented */
> CPUID_PSE36,
> .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_CX16 | CPUID_EXT_POPCNT,
> -.ext2_features = (PPRO_FEATURES & 0x0183F3FF) |
> +.ext2_features = (PPRO_FEATURES & EXT2_FEATURE_MASK) |
> CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
> .ext3_features = CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM |
> CPUID_EXT3_ABM | CPUID_EXT3_SSE4A,
> @@ -170,7 +171,7 @@ static x86_def_t x86_defs[] = {
> .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_CX16 |
> CPUID_EXT_POPCNT,
> /* Missing: CPUID_EXT2_PDPE1GB, CPUID_EXT2_RDTSCP */
> -.ext2_features = (PPRO_FEATURES & 0x0183F3FF) |
> +.ext2_features = (PPRO_FEATURES & EXT2_FEATURE_MASK) |
> CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX |
> CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT | CPUID_EXT2_MMXEXT |
> CPUID_EXT2_FFXSR,
> @@ -220,7 +221,7 @@ static x86_def_t x86_defs[] = {
> /* Missing: CPUID_EXT_POPCNT, CPUID_EXT_MONITOR */
> .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_CX16,
> /* Missing: CPUID_EXT2_PDPE1GB, CPUID_EXT2_RDTSCP */
> -.ext2_features = (PPRO_FEATURES & 0x0183F3FF) |
> +.ext2_features = (PPRO_FEATURES & EXT2_FEATURE_MASK) |
> CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
> /* Missing: CPUID_EXT3_LAHF_LM, CPUID_EXT3_CMP_LEG,
> CPUID_EXT3_EXTAPIC,
> CPUID_EXT3_CR8LEG, CPUID_EXT3_ABM, CPUID_EXT3_SSE4A,
> @@ -308,7 +309,7 @@ static x86_def_t x86_defs[] = {
> .stepping = 3,
> .features = PPRO_FEATURES | CPUID_PSE36 | CPUID_VME |
> CPUID_MTRR | CPUID_MCA,
> -.ext2_features = (PPRO_FEATURES & 0x0183F3FF) | CPUID_EXT2_MMXEXT |
> +.ext2_features = (PPRO_FEATURES & EXT2_FEATURE_MASK) |
> CPUID_EXT2_MMXEXT |
>CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT,
> .xlevel = 0x8008,
> /* XXX: put another string ? */
> @@ -330,7 +331,7 @@ static x86_def_t x86_defs[] = {
> CPUID_EXT_SSE3 /* PNI */ | CPUID_EXT_SSSE3,
> /* Missing: CPUID_EXT_DSCPL | CPUID_EXT_EST |
> * CPUID_EXT_TM2 | CPUID_EXT_XTPR */
> -.ext2_features = (PPRO_FEATURES & 0x0183F3FF) | CPUID_EXT2_NX,
> +.ext2_features = (PPRO_FEATURES & EXT2_FEATURE_MASK) | CPUID_EXT2_NX,
> /* Missing: .ext3_features = CPUID_EXT3_LAHF_LM */
> .xlevel = 0x800A,
> .model_id = "Intel(R) Atom(TM) CPU N270 @ 1.60GHz",
> --
> 1.6.4
>
>
>
>
>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [RFC][PATCH] x86_64: Fix long jumps/calls in long mode with REX.W set
On Thu, Mar 04, 2010 at 04:03:33PM +0300, malc wrote: > On Thu, 4 Mar 2010, malc wrote: > > That's not enough, later on there's a bunch of operations assuming > 32bit width... > After discussing with malc, it happens that this patch is actually correct and enough. I have just committed it. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] qustion about x86 sse insn "lddqu"
On Thu, Dec 03, 2009 at 01:29:23PM +0800, Hui Zhu wrote: > Hi, > > In qemu 0.11.0, it handle lddqu as: > case 0x3f0: /* lddqu */ > if (mod == 3) > goto illegal_op; > gen_lea_modrm(s, modrm, ®_addr, &offset_addr); > gen_sto_env_A0(s->mem_index, offsetof(CPUX86State,xmm_regs[reg])); > break; > It st the value of xmm[reg] to address A0, right? > > But in intel doc about this insn: > LDDQU—Load Unaligned Integer 128 Bits > The instruction is functionally similar to MOVDQU xmm, m128 for loading from > memory. That is: 16 bytes of data starting at an address specified by the > source > memory operand (second operand) are fetched from memory and placed in > a destination > register (first operand). The source operand need not be aligned on a 16-byte > boundary. Up to 32 bytes may be loaded from memory; this is implementation > dependent. > > Did I miss something? Or this code have some bug? > The patch is indeed wrong, I have just committed a patch to fix the problem. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
[Qemu-devel] [PATCH] target-i386: fix lddqu SSE instruction
This instruction load data from memory to register and not the reverse. Signed-off-by: Aurelien Jarno --- target-i386/translate.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/target-i386/translate.c b/target-i386/translate.c index a597e80..525a83b 100644 --- a/target-i386/translate.c +++ b/target-i386/translate.c @@ -3167,7 +3167,7 @@ static void gen_sse(DisasContext *s, int b, target_ulong pc_start, int rex_r) if (mod == 3) goto illegal_op; gen_lea_modrm(s, modrm, ®_addr, &offset_addr); -gen_sto_env_A0(s->mem_index, offsetof(CPUX86State,xmm_regs[reg])); +gen_ldo_env_A0(s->mem_index, offsetof(CPUX86State,xmm_regs[reg])); break; case 0x22b: /* movntss */ case 0x32b: /* movntsd */ -- 1.7.0
Re: [Qemu-devel] [PATCH] EHCI support - device recognized, but no data
Am Freitag, 5. März 2010 04:17 schrieb David S. Ahern: > Jan: > > I spent some more time on the EHCI support today. With the attached > patch (delta to the patch from yesterday) a USB key is recognized within > the guest (FC-12): Looks very nice. Actually I could access the file system on my USB stick. I have pushed your patch along with some additional minor changes to git://repo.or.cz/qemu/kevin.git ehci. Jan, you might want to pull from there. Kevin
Re: [Qemu-devel] i386 emulation bug: mov reg, [addr]
On Tue, Dec 15, 2009 at 07:48:53PM +0100, Clemens Kolbitsch wrote: > Hi list, > > I'm experiencing a strange emulation bug with the op-code below. The > instruction raises a segfault in the application (running on the guest), > however, if I enable KVM to run the exact same application, no segfault is > raised. > > 0x0080023b: 8b 04 65 11 22 33 44mov regEAX, [0x44332211] > > where "11 22 33 44" is just some address. According to gdb (on a 32bit little- > endian machine), this instruction can be disassembled as a "mov address to > reg-eax". > > I have added some debugging code to the disas_insn function in translate.c to > find out that the code is disassembled to the following blocks: > > (NOTE: this debugging comes from an old qemu version where the old TB-style > code was still used. HOWEVER, the same bug is still happening when used on > the > 0.11.0 source branch). > > 0x0080023b: disassemble 7 bytes (to 0x00800242) > 0x001: movl_A0_im 0x44332211 > 0x002: addl_A0_ESP_s1 > 0x003: ldl_user_T0_A0 > 0x004: movl_EAX_T0 > > So, as you can see, everything seems correct, but there is an additional > (second) TB that messes everything up. In fact, the segfault happens because > whatever is in ESP (shifted by one) is added to the address (which might then > not be a valid address). > > As I said, the code might crash in old versions of Qemu just like in the > 0.11.0 source branch and works fine if I use KVM (because the user code is > not > emulated of course). > > Since this is such a fundamental problem, I don't quite understand how this > could stay hidden so long... or maybe there is an error on my side :-/ > > Any help on this is greatly appreciated!! I have just noticed the problem is not yet fixed, even if Jamie proposed a patch in English. I have built a testcase (see below) and I have just sent a patch to the mailing list. Compile with: gcc -static -nostartfiles -m32 -o test test.S .data msg_addr: .long msg0 msg0: .ascii "Hello World\n" msg1: .text .globl _start _start: mov $4, %eax mov $1, %ebx .byte 0x8b .byte 0x0c .byte 0x65 .long msg_addr mov $(msg1-msg0), %edx int $0x80 mov $1, %eax int $0x80 -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
[Qemu-devel] [PATCH] target-i386: fix SIB decoding with index = 4
A SIB byte with an index of 4 means "no scaled index", even if the scale
value is not 0. In 64-bit mode, if REX.X is used, an index of 4 selects
%r12. This is correctly handled by the computation of the index variable,
which includes the index bits, and also the REX.X prefix:
index = ((code >> 3) & 7) | REX_X(s);
Thanks to Avi Kivity, Jamie Lokier and Malc for the analysis of the
problem and the initial patch.
Signed-off-by: Aurelien Jarno
---
target-i386/translate.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/target-i386/translate.c b/target-i386/translate.c
index a597e80..dc6f511 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -2047,8 +2047,8 @@ static void gen_lea_modrm(DisasContext *s, int modrm, int
*reg_ptr, int *offset_
gen_op_movl_A0_im(disp);
}
}
-/* XXX: index == 4 is always invalid */
-if (havesib && (index != 4 || scale != 0)) {
+/* index == 4 means no index */
+if (havesib && (index != 4)) {
#ifdef TARGET_X86_64
if (s->aflag == 2) {
gen_op_addq_A0_reg_sN(scale, index);
--
1.7.0
[Qemu-devel] Re: [PATCH] EHCI support - device recognized, but no data
David S. Ahern wrote: > Jan: > > I spent some more time on the EHCI support today. With the attached > patch (delta to the patch from yesterday) a USB key is recognized within > the guest (FC-12): > > scsi 2:0:0:0: Direct-Access Kingston DT HyperXPMAP PQ: 0 > ANSI: 0 CCS > sd 2:0:0:0: Attached scsi generic sg1 type 0 > sd 2:0:0:0: [sda] 15646720 512-byte logical blocks: (8.01 GB/7.46 GiB) > sd 2:0:0:0: [sda] Write Protect is off > sd 2:0:0:0: [sda] Mode Sense: 23 00 00 00 > sd 2:0:0:0: [sda] Assuming drive cache: write through > sd 2:0:0:0: [sda] Assuming drive cache: write through > sda: sda1 sda2 > sd 2:0:0:0: [sda] Assuming drive cache: write through > sd 2:0:0:0: [sda] Attached SCSI removable disk > > > Unfortunately, fdisk fails to show any data. I'll be out for a few days, > so I won't get back to this until next week. > Cool! Actually, I'm more lucky: I can access the flash of my mobile as a mass storage. Performance (linear reading) is half of native if debugging is disabled (0.5 vs. 1 MByte/s). But one try also caused an "USB stall". Patch committed. I will also look for some cleanups (build breaks for non-x86 targets) and more tests soon. Jan signature.asc Description: OpenPGP digital signature
[Qemu-devel] QEMU license problem (was [PATCH v3] Drop --whole-archive and static libraries)
Blue Swirl schrieb:
> Thanks, applied.
>
>
> On Wed, Jan 6, 2010 at 7:24 PM, Andreas Färber
> wrote:
>> From: Andreas Färber
>>
>> Juan has contributed a cool Makefile infrastructure that enables us
>> to drop
>> static libraries completely:
>>
>> Move shared obj-y definitions to Makefile.objs, prefixed
>> {common-,hw-,user-},
>> and link those object files directly into the executables.
>>
>> Replace HWLIB by HWDIR, specifying only the directory.
>>
>> Drop --whole-archive and ARLIBS in Makefiles and configure.
>>
>> Drop GENERATED_HEADERS dependency in rules.mak, since this rebuilds all
>> common objects after generating a target-specific header; add dependency
>> rules to Makefile and Makefile.target instead.
>>
>> v2:
>> - Don't try to include /config.mak for user emulators
>> - Changes to user object paths ("Quickfix for libuser.a drop") were
>> obsoleted
>> by "user_only: compile everything with -fpie" (Kirill A. Shutemov)
>>
>> v3:
>> - Fix dependency modelling for tools
>> - Remove comment on GENERATED_HEADERS obsoleted by this patch
>>
>> Signed-off-by: Andreas Färber
>> Cc: Blue Swirl
>> Cc: Palle Lyckegaard
>> Cc: Ben Taylor
>> Cc: Juan Quintela
>> Cc: Kirill A. Shutemov
>> Cc: Paolo Bonzini
>> ---
>> Makefile| 138 +---
>> Makefile.hw | 33 +---
>> Makefile.objs | 155
>> +++
>> Makefile.target | 33 +---
>> Makefile.user |9 +---
>> configure | 34 +
>> rules.mak |4 +-
>> 7 files changed, 202 insertions(+), 204 deletions(-)
>> create mode 100644 Makefile.objs
>>
Removing libqemu.a was technically ok, but throws a license problem:
"In particular, the QEMU virtual CPU core library (libqemu.a) is
released under the GNU Lesser General Public License."
Without libqemu.a, this part of QEMU's license no longer works.
I think the best solution would be to add a rule for libqemu.a
which allows users to build this static library (make libqemu.a).
libqemu.a is also still needed for tests/qruncom.
Regards,
Stefan Weil
Re: [Qemu-devel] [PATCH] tcg: add div/rem 32-bit helpers
On 3/6/10, Aurelien Jarno wrote:
> On Sat, Mar 06, 2010 at 09:15:38AM +0200, Blue Swirl wrote:
> > On 3/4/10, Aurelien Jarno wrote:
> > > Some targets like ARM would benefit to use 32-bit helpers for
> > > div/rem/divu/remu.
> > >
> > > Create a #define for div2 so that targets can select between
> > > div, div2 and helper implementation. Use the helper version if none
> > > of the #define are present.
> > >
> > > Signed-off-by: Aurelien Jarno
> > > ---
> > > tcg-runtime.c | 24 +++
> > > tcg/arm/tcg-target.h|1 +
> > > tcg/hppa/tcg-target.h |1 +
> > > tcg/i386/tcg-target.h |1 +
> > > tcg/tcg-op.h| 57
> +-
> > > tcg/tcg-runtime.h |5
> > > tcg/x86_64/tcg-target.h |2 +
> > > 7 files changed, 89 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/tcg-runtime.c b/tcg-runtime.c
> > > index 219cade..abfc364 100644
> > > --- a/tcg-runtime.c
> > > +++ b/tcg-runtime.c
> > > @@ -25,6 +25,30 @@
> > >
> > > #include "tcg/tcg-runtime.h"
> > >
> > > +/* 32-bit helpers */
> > > +
> > > +int32_t tcg_helper_div_i32(int32_t arg1, int32_t arg2)
> > > +{
> > > +return arg1 / arg2;
> > > +}
> > > +
> > > +int32_t tcg_helper_rem_i32(int32_t arg1, int32_t arg2)
> > > +{
> > > +return arg1 % arg2;
> > > +}
> > > +
> > > +uint32_t tcg_helper_divu_i32(uint32_t arg1, uint32_t arg2)
> > > +{
> > > +return arg1 / arg2;
> > > +}
> > > +
> > > +uint32_t tcg_helper_remu_i32(uint32_t arg1, uint32_t arg2)
> > > +{
> > > +return arg1 % arg2;
> > > +}
> > > +
> > > +/* 64-bit helpers */
> > > +
> > > int64_t tcg_helper_shl_i64(int64_t arg1, int64_t arg2)
> > > {
> > > return arg1 << arg2;
> > > diff --git a/tcg/arm/tcg-target.h b/tcg/arm/tcg-target.h
> > > index 4cad967..4b2b0be 100644
> > > --- a/tcg/arm/tcg-target.h
> > > +++ b/tcg/arm/tcg-target.h
> > > @@ -56,6 +56,7 @@ enum {
> > > #define TCG_TARGET_CALL_STACK_OFFSET 0
> > >
> > > /* optional instructions */
> > > +#define TCG_TARGET_HAS_div2_i32
> >
> > Please also add commented out #defines for targets that do not
> > implement div2 yet.
>
>
> I don't really see the point. The targets should implement either
> div/rem or div2, depending on their instruction set. Implementing div2
> on a targets which have separate instructions to compute div and rem
> will impact the performances negatively.
On second thought, fully agree.
> > > #define TCG_TARGET_HAS_ext8s_i32
> > > #define TCG_TARGET_HAS_ext16s_i32
> > > // #define TCG_TARGET_HAS_ext8u_i32
> > > diff --git a/tcg/hppa/tcg-target.h b/tcg/hppa/tcg-target.h
> > > index 7ab6f0c..fa39bfc 100644
> > > --- a/tcg/hppa/tcg-target.h
> > > +++ b/tcg/hppa/tcg-target.h
> > > @@ -75,6 +75,7 @@ enum {
> > > #define TCG_TARGET_STACK_GROWSUP
> > >
> > > /* optional instructions */
> > > +#define TCG_TARGET_HAS_div2_i32
> > > //#define TCG_TARGET_HAS_ext8s_i32
> > > //#define TCG_TARGET_HAS_ext16s_i32
> > > //#define TCG_TARGET_HAS_bswap16_i32
> > > diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h
> > > index f97034c..e994fd5 100644
> > > --- a/tcg/i386/tcg-target.h
> > > +++ b/tcg/i386/tcg-target.h
> > > @@ -45,6 +45,7 @@ enum {
> > > #define TCG_TARGET_CALL_STACK_OFFSET 0
> > >
> > > /* optional instructions */
> > > +#define TCG_TARGET_HAS_div2_i32
> > > #define TCG_TARGET_HAS_rot_i32
> > > #define TCG_TARGET_HAS_ext8s_i32
> > > #define TCG_TARGET_HAS_ext16s_i32
> > > diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h
> > > index 6ae1760..c71e1a8 100644
> > > --- a/tcg/tcg-op.h
> > > +++ b/tcg/tcg-op.h
> > > @@ -365,6 +365,19 @@ static inline void tcg_gen_helperN(void *func, int
> flags, int sizemask,
> > > }
> > >
> > > /* FIXME: Should this be pure? */
> > > +static inline void tcg_gen_helper32(void *func, TCGv_i32 ret,
> > > +TCGv_i32 a, TCGv_i32 b)
> > > +{
> > > +TCGv_ptr fn;
> > > +TCGArg args[2];
> > > +fn = tcg_const_ptr((tcg_target_long)func);
> > > +args[0] = GET_TCGV_I32(a);
> > > +args[1] = GET_TCGV_I32(b);
> > > +tcg_gen_callN(&tcg_ctx, fn, 0, 0, GET_TCGV_I32(ret), 2, args);
> > > +tcg_temp_free_ptr(fn);
> > > +}
> > > +
> > > +/* FIXME: Should this be pure? */
> > > static inline void tcg_gen_helper64(void *func, TCGv_i64 ret,
> > > TCGv_i64 a, TCGv_i64 b)
> > > {
> > > @@ -635,7 +648,7 @@ static inline void tcg_gen_remu_i32(TCGv_i32 ret,
> TCGv_i32 arg1, TCGv_i32 arg2)
> > > {
> > > tcg_gen_op3_i32(INDEX_op_remu_i32, ret, arg1, arg2);
> > > }
> > > -#else
> > > +#elif defined(TCG_TARGET_HAS_div2_i32)
> > > static inline void tcg_gen_div_i32(TCGv_i32 ret, TCGv_i32 arg1,
> TCGv_i32 arg2)
> > > {
> > > T
Re: [Qemu-devel] [PATCH] tcg: add div/rem 32-bit helpers
On Sat, 6 Mar 2010, Aurelien Jarno wrote:
> On Sat, Mar 06, 2010 at 09:15:38AM +0200, Blue Swirl wrote:
> > On 3/4/10, Aurelien Jarno wrote:
> > > Some targets like ARM would benefit to use 32-bit helpers for
> > > div/rem/divu/remu.
> > >
> > > Create a #define for div2 so that targets can select between
> > > div, div2 and helper implementation. Use the helper version if none
> > > of the #define are present.
> > >
> > > Signed-off-by: Aurelien Jarno
> > > ---
> > > tcg-runtime.c | 24 +++
> > > tcg/arm/tcg-target.h|1 +
> > > tcg/hppa/tcg-target.h |1 +
> > > tcg/i386/tcg-target.h |1 +
> > > tcg/tcg-op.h| 57
> > > +-
> > > tcg/tcg-runtime.h |5
> > > tcg/x86_64/tcg-target.h |2 +
> > > 7 files changed, 89 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/tcg-runtime.c b/tcg-runtime.c
> > > index 219cade..abfc364 100644
> > > --- a/tcg-runtime.c
> > > +++ b/tcg-runtime.c
> > > @@ -25,6 +25,30 @@
> > >
> > > #include "tcg/tcg-runtime.h"
> > >
> > > +/* 32-bit helpers */
> > > +
> > > +int32_t tcg_helper_div_i32(int32_t arg1, int32_t arg2)
> > > +{
> > > +return arg1 / arg2;
> > > +}
> > > +
> > > +int32_t tcg_helper_rem_i32(int32_t arg1, int32_t arg2)
> > > +{
> > > +return arg1 % arg2;
> > > +}
> > > +
> > > +uint32_t tcg_helper_divu_i32(uint32_t arg1, uint32_t arg2)
> > > +{
> > > +return arg1 / arg2;
> > > +}
> > > +
> > > +uint32_t tcg_helper_remu_i32(uint32_t arg1, uint32_t arg2)
> > > +{
> > > +return arg1 % arg2;
> > > +}
> > > +
> > > +/* 64-bit helpers */
> > > +
> > > int64_t tcg_helper_shl_i64(int64_t arg1, int64_t arg2)
> > > {
> > > return arg1 << arg2;
> > > diff --git a/tcg/arm/tcg-target.h b/tcg/arm/tcg-target.h
> > > index 4cad967..4b2b0be 100644
> > > --- a/tcg/arm/tcg-target.h
> > > +++ b/tcg/arm/tcg-target.h
> > > @@ -56,6 +56,7 @@ enum {
> > > #define TCG_TARGET_CALL_STACK_OFFSET 0
> > >
> > > /* optional instructions */
> > > +#define TCG_TARGET_HAS_div2_i32
> >
> > Please also add commented out #defines for targets that do not
> > implement div2 yet.
>
> I don't really see the point. The targets should implement either
> div/rem or div2, depending on their instruction set. Implementing div2
> on a targets which have separate instructions to compute div and rem
> will impact the performances negatively.
Seconded.
>
> > > #define TCG_TARGET_HAS_ext8s_i32
> > > #define TCG_TARGET_HAS_ext16s_i32
> > > // #define TCG_TARGET_HAS_ext8u_i32
> > > diff --git a/tcg/hppa/tcg-target.h b/tcg/hppa/tcg-target.h
> > > index 7ab6f0c..fa39bfc 100644
> > > --- a/tcg/hppa/tcg-target.h
> > > +++ b/tcg/hppa/tcg-target.h
> > > @@ -75,6 +75,7 @@ enum {
> > > #define TCG_TARGET_STACK_GROWSUP
> > >
> > > /* optional instructions */
> > > +#define TCG_TARGET_HAS_div2_i32
> > > //#define TCG_TARGET_HAS_ext8s_i32
> > > //#define TCG_TARGET_HAS_ext16s_i32
> > > //#define TCG_TARGET_HAS_bswap16_i32
> > > diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h
> > > index f97034c..e994fd5 100644
> > > --- a/tcg/i386/tcg-target.h
> > > +++ b/tcg/i386/tcg-target.h
> > > @@ -45,6 +45,7 @@ enum {
> > > #define TCG_TARGET_CALL_STACK_OFFSET 0
> > >
> > > /* optional instructions */
> > > +#define TCG_TARGET_HAS_div2_i32
> > > #define TCG_TARGET_HAS_rot_i32
> > > #define TCG_TARGET_HAS_ext8s_i32
> > > #define TCG_TARGET_HAS_ext16s_i32
> > > diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h
> > > index 6ae1760..c71e1a8 100644
> > > --- a/tcg/tcg-op.h
> > > +++ b/tcg/tcg-op.h
> > > @@ -365,6 +365,19 @@ static inline void tcg_gen_helperN(void *func, int
> > > flags, int sizemask,
> > > }
> > >
> > > /* FIXME: Should this be pure? */
> > > +static inline void tcg_gen_helper32(void *func, TCGv_i32 ret,
> > > +TCGv_i32 a, TCGv_i32 b)
> > > +{
> > > +TCGv_ptr fn;
> > > +TCGArg args[2];
> > > +fn = tcg_const_ptr((tcg_target_long)func);
> > > +args[0] = GET_TCGV_I32(a);
> > > +args[1] = GET_TCGV_I32(b);
> > > +tcg_gen_callN(&tcg_ctx, fn, 0, 0, GET_TCGV_I32(ret), 2, args);
> > > +tcg_temp_free_ptr(fn);
> > > +}
> > > +
> > > +/* FIXME: Should this be pure? */
> > > static inline void tcg_gen_helper64(void *func, TCGv_i64 ret,
> > > TCGv_i64 a, TCGv_i64 b)
> > > {
> > > @@ -635,7 +648,7 @@ static inline void tcg_gen_remu_i32(TCGv_i32 ret,
> > > TCGv_i32 arg1, TCGv_i32 arg2)
> > > {
> > > tcg_gen_op3_i32(INDEX_op_remu_i32, ret, arg1, arg2);
> > > }
> > > -#else
> > > +#elif defined(TCG_TARGET_HAS_div2_i32)
> > > static inline void tcg_gen_div_i32(TCGv_i32 ret, TCGv_i32 arg1,
> > > TCGv_i32 arg2)
> > > {
> > > TCGv_i32 t0;
> > > @@ -671,6 +684,26 @@ static inline void tcg_gen_remu_i32(TCGv_i32 ret,
> > > TCGv_i32 arg1, TCGv_i32 a
Re: [Qemu-devel] [PATCH] tcg: add div/rem 32-bit helpers
On Sat, Mar 06, 2010 at 09:15:38AM +0200, Blue Swirl wrote:
> On 3/4/10, Aurelien Jarno wrote:
> > Some targets like ARM would benefit to use 32-bit helpers for
> > div/rem/divu/remu.
> >
> > Create a #define for div2 so that targets can select between
> > div, div2 and helper implementation. Use the helper version if none
> > of the #define are present.
> >
> > Signed-off-by: Aurelien Jarno
> > ---
> > tcg-runtime.c | 24 +++
> > tcg/arm/tcg-target.h|1 +
> > tcg/hppa/tcg-target.h |1 +
> > tcg/i386/tcg-target.h |1 +
> > tcg/tcg-op.h| 57
> > +-
> > tcg/tcg-runtime.h |5
> > tcg/x86_64/tcg-target.h |2 +
> > 7 files changed, 89 insertions(+), 2 deletions(-)
> >
> > diff --git a/tcg-runtime.c b/tcg-runtime.c
> > index 219cade..abfc364 100644
> > --- a/tcg-runtime.c
> > +++ b/tcg-runtime.c
> > @@ -25,6 +25,30 @@
> >
> > #include "tcg/tcg-runtime.h"
> >
> > +/* 32-bit helpers */
> > +
> > +int32_t tcg_helper_div_i32(int32_t arg1, int32_t arg2)
> > +{
> > +return arg1 / arg2;
> > +}
> > +
> > +int32_t tcg_helper_rem_i32(int32_t arg1, int32_t arg2)
> > +{
> > +return arg1 % arg2;
> > +}
> > +
> > +uint32_t tcg_helper_divu_i32(uint32_t arg1, uint32_t arg2)
> > +{
> > +return arg1 / arg2;
> > +}
> > +
> > +uint32_t tcg_helper_remu_i32(uint32_t arg1, uint32_t arg2)
> > +{
> > +return arg1 % arg2;
> > +}
> > +
> > +/* 64-bit helpers */
> > +
> > int64_t tcg_helper_shl_i64(int64_t arg1, int64_t arg2)
> > {
> > return arg1 << arg2;
> > diff --git a/tcg/arm/tcg-target.h b/tcg/arm/tcg-target.h
> > index 4cad967..4b2b0be 100644
> > --- a/tcg/arm/tcg-target.h
> > +++ b/tcg/arm/tcg-target.h
> > @@ -56,6 +56,7 @@ enum {
> > #define TCG_TARGET_CALL_STACK_OFFSET 0
> >
> > /* optional instructions */
> > +#define TCG_TARGET_HAS_div2_i32
>
> Please also add commented out #defines for targets that do not
> implement div2 yet.
I don't really see the point. The targets should implement either
div/rem or div2, depending on their instruction set. Implementing div2
on a targets which have separate instructions to compute div and rem
will impact the performances negatively.
> > #define TCG_TARGET_HAS_ext8s_i32
> > #define TCG_TARGET_HAS_ext16s_i32
> > // #define TCG_TARGET_HAS_ext8u_i32
> > diff --git a/tcg/hppa/tcg-target.h b/tcg/hppa/tcg-target.h
> > index 7ab6f0c..fa39bfc 100644
> > --- a/tcg/hppa/tcg-target.h
> > +++ b/tcg/hppa/tcg-target.h
> > @@ -75,6 +75,7 @@ enum {
> > #define TCG_TARGET_STACK_GROWSUP
> >
> > /* optional instructions */
> > +#define TCG_TARGET_HAS_div2_i32
> > //#define TCG_TARGET_HAS_ext8s_i32
> > //#define TCG_TARGET_HAS_ext16s_i32
> > //#define TCG_TARGET_HAS_bswap16_i32
> > diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h
> > index f97034c..e994fd5 100644
> > --- a/tcg/i386/tcg-target.h
> > +++ b/tcg/i386/tcg-target.h
> > @@ -45,6 +45,7 @@ enum {
> > #define TCG_TARGET_CALL_STACK_OFFSET 0
> >
> > /* optional instructions */
> > +#define TCG_TARGET_HAS_div2_i32
> > #define TCG_TARGET_HAS_rot_i32
> > #define TCG_TARGET_HAS_ext8s_i32
> > #define TCG_TARGET_HAS_ext16s_i32
> > diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h
> > index 6ae1760..c71e1a8 100644
> > --- a/tcg/tcg-op.h
> > +++ b/tcg/tcg-op.h
> > @@ -365,6 +365,19 @@ static inline void tcg_gen_helperN(void *func, int
> > flags, int sizemask,
> > }
> >
> > /* FIXME: Should this be pure? */
> > +static inline void tcg_gen_helper32(void *func, TCGv_i32 ret,
> > +TCGv_i32 a, TCGv_i32 b)
> > +{
> > +TCGv_ptr fn;
> > +TCGArg args[2];
> > +fn = tcg_const_ptr((tcg_target_long)func);
> > +args[0] = GET_TCGV_I32(a);
> > +args[1] = GET_TCGV_I32(b);
> > +tcg_gen_callN(&tcg_ctx, fn, 0, 0, GET_TCGV_I32(ret), 2, args);
> > +tcg_temp_free_ptr(fn);
> > +}
> > +
> > +/* FIXME: Should this be pure? */
> > static inline void tcg_gen_helper64(void *func, TCGv_i64 ret,
> > TCGv_i64 a, TCGv_i64 b)
> > {
> > @@ -635,7 +648,7 @@ static inline void tcg_gen_remu_i32(TCGv_i32 ret,
> > TCGv_i32 arg1, TCGv_i32 arg2)
> > {
> > tcg_gen_op3_i32(INDEX_op_remu_i32, ret, arg1, arg2);
> > }
> > -#else
> > +#elif defined(TCG_TARGET_HAS_div2_i32)
> > static inline void tcg_gen_div_i32(TCGv_i32 ret, TCGv_i32 arg1, TCGv_i32
> > arg2)
> > {
> > TCGv_i32 t0;
> > @@ -671,6 +684,26 @@ static inline void tcg_gen_remu_i32(TCGv_i32 ret,
> > TCGv_i32 arg1, TCGv_i32 arg2)
> > tcg_gen_op5_i32(INDEX_op_divu2_i32, t0, ret, arg1, t0, arg2);
> > tcg_temp_free_i32(t0);
> > }
> > +#else
> > +static inline void tcg_gen_div_i32(TCGv_i32 ret, TCGv_i32 arg1, TCGv_i32
> > arg2)
> > +{
> > +tcg_gen_helper32(tcg_helper_div_i32, ret, arg1, arg2);
> > +}
> > +
> > +static inline void tc
Re: [Qemu-devel] Re: Another VNC crash, qemu-kvm-0.12.3
Alexander Graf writes:
> On 05.03.2010, at 17:52, Chris Webb wrote:
>
> > Of course, if the screen width or height is 1, it doesn't really matter what
> > the value of the mouse position for the click is, so something as simple as
> >
> > diff --git a/vnc.c b/vnc.c
> > --- a/vnc.c
> > +++ b/vnc.c
> > @@ -1421,8 +1421,10 @@
> > dz = 1;
> >
> > if (vs->absolute) {
> > -kbd_mouse_event(x * 0x7FFF / (ds_get_width(vs->ds) - 1),
> > -y * 0x7FFF / (ds_get_height(vs->ds) - 1),
> > +kbd_mouse_event(ds_get_width(vs->ds) > 1 ?
> > + x * 0x7FFF / (ds_get_width(vs->ds) - 1) : 0x4000,
> > +ds_get_height(vs->ds) > 1 ?
> > + y * 0x7FFF / (ds_get_height(vs->ds) - 1) :
> > 0x4000,
> > dz, buttons);
> > } else if (vnc_has_feature(vs, VNC_FEATURE_POINTER_TYPE_CHANGE)) {
> > x -= 0x7FFF;
> >
> > will fix the symptom: the division by zero. The underlying cause of a 9x1
> > display surface is a bit mysterious though.
>
> Is it? When booting the screen gets resized to something like 9x1 for a
> few ms. Try putting debug code in the resize callback - you'll see it.
Ah, okay. In that case, this patch could well be the correct fix rather than
just a work-around. I'll have a look for any other places in vnc.c that
might do a similar division-by-zero for small screen sizes at the same
point.
Best wishes,
Chris.
