date:20110401

[Qemu-devel] Re: [PATCH] w32: Fix compilation of new code

2011-04-01 Thread Paolo Bonzini


On 03/31/2011 10:58 PM, Stefan Weil wrote:

Some recently added new code did not compile for w32 targets.

The functions qemu_iohandler_fill and qemu_iohandler_poll need
data type fd_set which is declared in winsock2.h for w32 targets.

Moving the functions from qemu-common.h to qemu_socket.h fixes
compilations for w32 without adding a new include file to qemu-common.h.

Cc: Paolo Bonzini
Cc: Anthony Liguori
Signed-off-by: Stefan Weil
---
  iohandler.c   |1 +
  qemu-common.h |3 ---
  qemu_socket.h |4 
  3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/iohandler.c b/iohandler.c
index 2b82421..bf1ba5c 100644
--- a/iohandler.c
+++ b/iohandler.c
@@ -26,6 +26,7 @@
  #include "qemu-common.h"
  #include "qemu-char.h"
  #include "qemu-queue.h"
+#include "qemu_socket.h"

  #ifndef _WIN32
  #include
diff --git a/qemu-common.h b/qemu-common.h
index 8ecb488..eb2c96a 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -228,9 +228,6 @@ typedef void IOReadHandler(void *opaque, const uint8_t 
*buf, int size);
  typedef int IOCanReadHandler(void *opaque);
  typedef void IOHandler(void *opaque);

-void qemu_iohandler_fill(int *pnfds, fd_set *readfds, fd_set *writefds, fd_set 
*xfds);
-void qemu_iohandler_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds, int 
rc);
-
  struct ParallelIOArg {
  void *buffer;
  int count;
diff --git a/qemu_socket.h b/qemu_socket.h
index 180e4db..4a6ef82 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -32,6 +32,10 @@ int inet_aton(const char *cp, struct in_addr *ia);

  #include "qemu-option.h"

+/* Functions from iohandler.c. */
+void qemu_iohandler_fill(int *pnfds, fd_set *readfds, fd_set *writefds, fd_set 
*xfds);
+void qemu_iohandler_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds, int 
rc);
+
  /* misc helpers */
  int qemu_socket(int domain, int type, int protocol);
  int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen);


Acked-by: Paolo Bonzini 

Paolo

Re: [Qemu-devel] [PATCH RFC] vga: flag vga ram for notifiers

2011-04-01 Thread Peter Maydell

On 31 March 2011 22:38, Michael S. Tsirkin  wrote:
> On Thu, Mar 31, 2011 at 10:32:11PM +0100, Peter Maydell wrote:
>> On 31 March 2011 21:23, Anthony Liguori  wrote:
>> > Maybe the right approach here is to just use a virtio specific API and
>> > register RAM as register_virtio_dma_area().
>>
>> That seems like a clearer API, yes. I think it makes it much more
>> obvious what it's trying to achieve.

> Maybe register_dma_area - its' not 100% virtio specific.

Presumably it is specific to virtualisation-aware devices
though? Guest DMA has to just work to all the locations you
can DMA to/from on hardware, right?

-- PMM

[Qemu-devel] Re: [PATCH V3] floppy: save and restore DIR register

2011-04-01 Thread Paolo Bonzini


On 04/01/2011 08:22 AM, Jason Wang wrote:

+
+if (drive->bs == NULL) {
+return 1;


Is it okay to return 1 here?  Have you tested the case when both the 
source and the target drives have no floppy?


(The "media_changed == 2" in my sample code was basically a way to save 
the "media present" state of the drive on the source).



+} else {
+drive->bs->media_changed = drive->media_changed;
+return 0;
+}


The code is also missing the case of a pc-0.14 machine.  The problem 
here is that the pc-0.15 machine hasn't been created yet, you get the 
honor. :(


Paolo

[Qemu-devel] Re: [RFC PATCH] qed: add support for Copy-on-Read

2011-04-01 Thread Stefan Hajnoczi

On Wed, Mar 30, 2011 at 08:08:34PM -0500, Anthony Liguori wrote:
> When creating an image using qemu-img, just pass '-o copy_on_read' and then
> whenever QED reads from a backing file, it will write the block to the QED
> file after the read completes ensuring that you only fetch from the backing
> device once.
> 
> This is very useful for streaming images over a slow connection.
> 
> This isn't ready for merge yet as it's not playing nice with synchronize I/O.

What is the issue here?  Streaming had issues with aio contexts and
synchronous I/O emulation but copy-on-read by itself looks safe to me.

> I think it's fairly easy to do the same thing in qcow2 by just hooking adding
> some logic after bdrv_aio_write() to call back into qcow2 with a synchronous
> I/O write in the backing file case.   Thoughts on whether that would actually
> work?

Why not do the follow-up .bdrv_aio_writev() for qcow2 too?  I don't see
a reason to do it synchronously.

Stefan

[Qemu-devel] Re: [RFC][PATCH v1 10/12] guest agent: qemu-ga daemon

2011-04-01 Thread Jes Sorensen

On 03/25/11 20:47, Michael Roth wrote:
> This is the actual guest daemon, it listens for requests over a
> virtio-serial/isa-serial/unix socket channel and routes them through
> to dispatch routines, and writes the results back to the channel in
> a manner similar to Qmp.
> 
> This is currently horribly broken, only the unix-listen channel method
> is working at the moment (likely due to mis-use of gio channel
> interfaces), and the code is in overall rough shape.
> 
> Signed-off-by: Michael Roth 
> ---
>  qemu-ga.c |  522 
> +
>  1 files changed, 522 insertions(+), 0 deletions(-)
>  create mode 100644 qemu-ga.c
> 
> diff --git a/qemu-ga.c b/qemu-ga.c
> new file mode 100644
> index 000..435a1fc
> --- /dev/null
> +++ b/qemu-ga.c
> @@ -0,0 +1,522 @@
> +/*
> + * QEMU Guest Agent
> + *
> + * Copyright IBM Corp. 2011
> + *
> + * Authors:
> + *  Adam Litke
> + *  Michael Roth  
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "qemu_socket.h"
> +#include "json-streamer.h"
> +#include "json-parser.h"
> +#include "guest-agent.h"
> +
> +#define QGA_VERSION "1.0"
> +#define QGA_GUEST_PATH_VIRTIO_DEFAULT "/dev/virtio-ports/va"
> +#define QGA_PIDFILE_DEFAULT "/var/run/qemu-va.pid"
> +#define QGA_BAUDRATE_DEFAULT B38400 /* for isa-serial channels */
> +
> +bool verbose_enabled = false;
> +
> +typedef struct GAState {
> +bool active;
> +int session_id;
> +const char *proxy_path;
> +JSONMessageParser parser;
> +GMainLoop *main_loop;
> +guint conn_id;
> +GSocket *conn_sock;
> +GIOChannel *conn_channel;
> +guint listen_id;
> +GSocket *listen_sock;
> +GIOChannel *listen_channel;
> +const char *path;
> +const char *method;
> +} GAState;
> +
> +static void usage(const char *cmd)
> +{
> +printf(
> +"Usage: %s -c \n"
> +"QEMU virtagent guest agent %s\n"
> +"\n"
> +"  -c, --channel channel method: one of unix-connect, virtio-serial, 
> or\n"
> +"isa-serial\n"
> +"  -p, --pathchannel path\n"
> +"  -v, --verbose display extra debugging information\n"
> +"  -d, --daemonize   become a daemon\n"
> +"  -h, --helpdisplay this help and exit\n"
> +"\n"
> +"Report bugs to \n"
> +, cmd, QGA_VERSION);
> +}
> +
> +static void conn_channel_close(GAState *s);
> +
> +static void become_daemon(void)
> +{
> +pid_t pid, sid;
> +int pidfd;
> +char *pidstr;
> +
> +pid = fork();
> +if (pid < 0)
> +exit(EXIT_FAILURE);

There's a pile of missing braces in this file - please go through it and
fix them before the next version.

Cheers,
Jes

[Qemu-devel] Re: [PATCH V3] floppy: save and restore DIR register

2011-04-01 Thread Jason Wang

Paolo Bonzini writes:
 > On 04/01/2011 08:22 AM, Jason Wang wrote:
 > > +
 > > +if (drive->bs == NULL) {
 > > +return 1;
 > 
 > Is it okay to return 1 here?  Have you tested the case when both the 
 > source and the target drives have no floppy?
 > 

Thanks for the reminding, and it could be fixed by put all pre/post callbacks
into the subsections.

 > (The "media_changed == 2" in my sample code was basically a way to save 
 > the "media present" state of the drive on the source).
 > 

Right, but it would make subsection saving be the common case (consider most of
the vm may just have one floppy but we have two drives). A better solution 
maybe:

1 Set default_migration_media_changed be 0 for 0.15 and 1 for elder
2 Unconditiaonlly send subsection when it was 0, and do not send subsection when
it was 1
3 Set media_changed to default_migration_media_changed in pre_load()
4 Let all pre/post to be in subsection

After those, we can make sure the migration between 0.15 could get correct
media_changed, also make sure the seamless migration between 0.15 and older
machine types.

Any suggestions?

 > > +} else {
 > > +drive->bs->media_changed = drive->media_changed;
 > > +return 0;
 > > +}
 > 
 > The code is also missing the case of a pc-0.14 machine.  The problem 
 > here is that the pc-0.15 machine hasn't been created yet, you get the 
 > honor. :(
 > 

Would create such one :)

 > Paolo

Re: [Qemu-devel] [RFC PATCH] qed: add support for Copy-on-Read

2011-04-01 Thread Kevin Wolf

Am 31.03.2011 03:08, schrieb Anthony Liguori:
> When creating an image using qemu-img, just pass '-o copy_on_read' and then
> whenever QED reads from a backing file, it will write the block to the QED
> file after the read completes ensuring that you only fetch from the backing
> device once.

As you probably know, I don't agree with the interface. Copy on read
should be first and foremost a runtime option. It's okay to fetch the
default for this option from the image file, but it's not the right
primary interface.

> This is very useful for streaming images over a slow connection.
> 
> This isn't ready for merge yet as it's not playing nice with synchronize I/O.

Like Stefan I'm not sure what you mean here.

One problem that I see is that if you have a concurrent write request
from the guest, the COR write request may overwrite the guest's request,
which is obviously wrong. Is this what you mean?

> I think it's fairly easy to do the same thing in qcow2 by just hooking adding
> some logic after bdrv_aio_write() to call back into qcow2 with a synchronous
> I/O write in the backing file case.   Thoughts on whether that would actually
> work?

I can't see a reason why it wouldn't work. Except that it has the same
problem that I described above. A synchronous write isn't going to solve
this, you'd additionally need a qemu_aio_flush() if you want to avoid
proper locking. But that's really the same for QED and qcow2.

Kevin

Re: [Qemu-devel] [PATCH RFC] vga: flag vga ram for notifiers

2011-04-01 Thread Michael S. Tsirkin

On Fri, Apr 01, 2011 at 08:12:48AM +0100, Peter Maydell wrote:
> On 31 March 2011 22:38, Michael S. Tsirkin  wrote:
> > On Thu, Mar 31, 2011 at 10:32:11PM +0100, Peter Maydell wrote:
> >> On 31 March 2011 21:23, Anthony Liguori  wrote:
> >> > Maybe the right approach here is to just use a virtio specific API and
> >> > register RAM as register_virtio_dma_area().
> >>
> >> That seems like a clearer API, yes. I think it makes it much more
> >> obvious what it's trying to achieve.
> 
> > Maybe register_dma_area - its' not 100% virtio specific.
> 
> Presumably it is specific to virtualisation-aware devices
> though? Guest DMA has to just work to all the locations you
> can DMA to/from on hardware, right?
> 
> -- PMM

I guess so, yes.  But it might not be possible e.g. for a PCI device
to DMA into an ISA device.

-- 
MST

[Qemu-devel] Re: [PATCH V3] floppy: save and restore DIR register

2011-04-01 Thread Paolo Bonzini

On 04/01/2011 12:03 PM, Jason Wang wrote:

Paolo Bonzini writes:
  >  On 04/01/2011 08:22 AM, Jason Wang wrote:
  >  >  +
  >  >  +if (drive->bs == NULL) {
  >  >  +return 1;
  >
  >  Is it okay to return 1 here?  Have you tested the case when both the
  >  source and the target drives have no floppy?
  >

Thanks for the reminding, and it could be fixed by put all pre/post callbacks
into the subsections.

Not sure that works, post callbacks are not called for subsections that 
are not needed.

  >  (The "media_changed == 2" in my sample code was basically a way to save
  >  the "media present" state of the drive on the source).

Right, but it would make subsection saving be the common case (consider most of
the vm may just have one floppy but we have two drives).

Hmm, right, there's 2 bits involved---media present and media changed.

1 Set default_migration_media_changed be 0 for 0.15 and 1 for elder
2 Unconditionally send subsection when it was 0, and do not send subsection when
it was 1

Which means, never send it for pc-0.14 and always for pc-0.15.  At this 
point I'm starting to think that this new-to-old migration business is a 
lost cause, and it's easier to bump the version number and just default 
the field to 1 if the version is old. :(

The problem here is that you're solving a different problem than what 
subsections were supposed to help with.  Subsections help with "the 
value that old versions used is usually but not always correct".  Here 
you have "the value that old versions used is usually *in*correct, but 
nobody noticed so far".

Michael/Amit, as you were the ones proposing stronger new-to-old 
support, can you read the thread and see if you have any ideas?

Paolo

[Qemu-devel] [PATCH v7 4/4] piix_pci: load path clean up

2011-04-01 Thread Isaku Yamahata

The previous patch didn't change the behavior when load,
it resulted in ugly code. This patch cleans it up.

With this patch, pic irq lines are manipulated when loaded.
It is expected that it won't change the behaviour because
the interrupts are level: at the moment e.g. pci devices already
reassert interrupts on load.

Test:
- rung linux as guest and use flooding ping (ping -f) to host
  in order to trigger interrupts for e1000 emulated.
- savevm/loadvm and see guest kept running after loadvm.

To be honest, I'm not sure that ping -f caused enough interrupts
because Linux e1000 driver supports NAPI.
TODO: test more OSes, stress test with save/load, live-migration

Signed-off-by: Isaku Yamahata 
---
Changes v3 -> v4:
- newly introduced
---
 hw/piix_pci.c |   12 
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 7ffb821..5f0d92f 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -281,8 +281,7 @@ static void piix3_set_irq_pic(PIIX3State *piix3, int 
pic_irq)
  (pic_irq * PIIX_NUM_PIRQS;
 }
 
-static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level,
-bool propagate)
+static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level)
 {
 int pic_irq;
 uint64_t mask;
@@ -296,15 +295,13 @@ static void piix3_set_irq_level(PIIX3State *piix3, int 
pirq, int level,
 piix3->pic_levels &= ~mask;
 piix3->pic_levels |= mask * !!level;
 
-if (propagate) {
-piix3_set_irq_pic(piix3, pic_irq);
-}
+piix3_set_irq_pic(piix3, pic_irq);
 }
 
 static void piix3_set_irq(void *opaque, int pirq, int level)
 {
 PIIX3State *piix3 = opaque;
-piix3_set_irq_level(piix3, pirq, level, true);
+piix3_set_irq_level(piix3, pirq, level);
 }
 
 /* irq routing is changed. so rebuild bitmap */
@@ -315,8 +312,7 @@ static void piix3_update_irq_levels(PIIX3State *piix3)
 piix3->pic_levels = 0;
 for (pirq = 0; pirq < PIIX_NUM_PIRQS; pirq++) {
 piix3_set_irq_level(piix3, pirq,
-pci_bus_get_irq_level(piix3->dev.bus, pirq),
-false);
+pci_bus_get_irq_level(piix3->dev.bus, pirq));
 }
 }
 
-- 
1.7.1.1

[Qemu-devel] [PATCH v7 1/4] pci: add accessor function to get irq levels

2011-04-01 Thread Isaku Yamahata

Introduce accessor function to know INTx levels.
It will be used later by q35.
Although piix_pci tracks the intx line levels, it can be eliminated
by this helper function.

Cc: Michael S. Tsirkin 
Signed-off-by: Isaku Yamahata 
---
 hw/pci.c |7 +++
 hw/pci.h |1 +
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index 6b577e1..3ee4871 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -126,6 +126,13 @@ static void pci_change_irq_level(PCIDevice *pci_dev, int 
irq_num, int change)
 bus->set_irq(bus->irq_opaque, irq_num, bus->irq_count[irq_num] != 0);
 }
 
+int pci_bus_get_irq_level(PCIBus *bus, int irq_num)
+{
+assert(irq_num >= 0);
+assert(irq_num < bus->nirq);
+return !!bus->irq_count[irq_num];
+}
+
 /* Update interrupt status bit in config space on interrupt
  * state change. */
 static void pci_update_irq_status(PCIDevice *dev)
diff --git a/hw/pci.h b/hw/pci.h
index 52ee8c9..a5f875d 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -234,6 +234,7 @@ void pci_bus_new_inplace(PCIBus *bus, DeviceState *parent,
 PCIBus *pci_bus_new(DeviceState *parent, const char *name, uint8_t devfn_min);
 void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
   void *irq_opaque, int nirq);
+int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
 void pci_bus_hotplug(PCIBus *bus, pci_hotplug_fn hotplug, DeviceState *dev);
 PCIBus *pci_register_bus(DeviceState *parent, const char *name,
  pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
-- 
1.7.1.1

[Qemu-devel] [PATCH v7 2/4] piix_pci: eliminate PIIX3State::pci_irq_levels

2011-04-01 Thread Isaku Yamahata

PIIX3State::pci_irq_levels are redundant which is already tracked by
PCIBus layer. So eliminate them.

Cc: Juan Quintela 
Cc: Michael S. Tsirkin 
Signed-off-by: Isaku Yamahata 
---
Changes v3 -> v4:
- use PCI_NUM_PINS instead of magic number 4

Changes v2 -> v3:
- rename member s/dummy_for_save_load_compat/pci_irq_levels_vmstate/g
---
 hw/piix_pci.c |   38 +++---
 1 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 358da58..35e420c 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -37,10 +37,14 @@
 
 typedef PCIHostState I440FXState;
 
+#define PIIX_NUM_PIRQS  4ULL/* PIRQ[A-D] */
+
 typedef struct PIIX3State {
 PCIDevice dev;
-int pci_irq_levels[4];
 qemu_irq *pic;
+
+/* This member isn't used. Just for save/load compatibility */
+int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
 } PIIX3State;
 
 struct PCII440FXState {
@@ -162,9 +166,11 @@ static int i440fx_load_old(QEMUFile* f, void *opaque, int 
version_id)
 i440fx_update_memory_mappings(d);
 qemu_get_8s(f, &d->smm_enabled);
 
-if (version_id == 2)
-for (i = 0; i < 4; i++)
-d->piix3->pci_irq_levels[i] = qemu_get_be32(f);
+if (version_id == 2) {
+for (i = 0; i < PIIX_NUM_PIRQS; i++) {
+qemu_get_be32(f); /* dummy load for compatibility */
+}
+}
 
 return 0;
 }
@@ -236,7 +242,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int 
*piix3_devfn, qemu_irq *
 piix3 = DO_UPCAST(PIIX3State, dev,
   pci_create_simple_multifunction(b, -1, true, "PIIX3"));
 piix3->pic = pic;
-pci_bus_irqs(b, piix3_set_irq, pci_slot_get_pirq, piix3, 4);
+pci_bus_irqs(b, piix3_set_irq, pci_slot_get_pirq, piix3, PIIX_NUM_PIRQS);
 (*pi440fx_state)->piix3 = piix3;
 
 *piix3_devfn = piix3->dev.devfn;
@@ -256,8 +262,6 @@ static void piix3_set_irq(void *opaque, int irq_num, int 
level)
 int i, pic_irq, pic_level;
 PIIX3State *piix3 = opaque;
 
-piix3->pci_irq_levels[irq_num] = level;
-
 /* now we change the pic irq level according to the piix irq mappings */
 /* XXX: optimize */
 pic_irq = piix3->dev.config[0x60 + irq_num];
@@ -266,8 +270,9 @@ static void piix3_set_irq(void *opaque, int irq_num, int 
level)
to it */
 pic_level = 0;
 for (i = 0; i < 4; i++) {
-if (pic_irq == piix3->dev.config[0x60 + i])
-pic_level |= piix3->pci_irq_levels[i];
+if (pic_irq == piix3->dev.config[0x60 + i]) {
+pic_level |= pci_bus_get_irq_level(piix3->dev.bus, i);
+}
 }
 qemu_set_irq(piix3->pic[pic_irq], pic_level);
 }
@@ -309,8 +314,17 @@ static void piix3_reset(void *opaque)
 pci_conf[0xab] = 0x00;
 pci_conf[0xac] = 0x00;
 pci_conf[0xae] = 0x00;
+}
 
-memset(d->pci_irq_levels, 0, sizeof(d->pci_irq_levels));
+static void piix3_pre_save(void *opaque)
+{
+int i;
+PIIX3State *piix3 = opaque;
+
+for (i = 0; i < ARRAY_SIZE(piix3->pci_irq_levels_vmstate); i++) {
+piix3->pci_irq_levels_vmstate[i] =
+pci_bus_get_irq_level(piix3->dev.bus, i);
+}
 }
 
 static const VMStateDescription vmstate_piix3 = {
@@ -318,9 +332,11 @@ static const VMStateDescription vmstate_piix3 = {
 .version_id = 3,
 .minimum_version_id = 2,
 .minimum_version_id_old = 2,
+.pre_save = piix3_pre_save,
 .fields  = (VMStateField []) {
 VMSTATE_PCI_DEVICE(dev, PIIX3State),
-VMSTATE_INT32_ARRAY_V(pci_irq_levels, PIIX3State, 4, 3),
+VMSTATE_INT32_ARRAY_V(pci_irq_levels_vmstate, PIIX3State,
+  PIIX_NUM_PIRQS, 3),
 VMSTATE_END_OF_LIST()
 }
 };
-- 
1.7.1.1

[Qemu-devel] [PATCH v7 0/4] piix_pci: optimize irq data path

2011-04-01 Thread Isaku Yamahata

Here is v7 which are rebased to pci branch.

I tested this patch series as follows. please see the commit message
for details.
- 3/4 piix_pci: optimize set irq path
  Run linux as guest with 4 e1000 emulated devices.
  And confirmed that each PIRQ[A-D] are able to assert interrupts.
- 4/4 piix_pci: load path clean up
  Run linux as guest with one e1000 emulated device.
  savevm/loadvm guest with runing ping -f in guest.

To be honest, the patch, "4/4 piix_pci: load path clean up", needs more
extensive tests. For example live migration and stress test.
But for now I don't have plan to further testing in near future. Sorry.
So please feel free to pick or drop the last patch.

patch description:
This patch series optimizes irq data path of piix_pci.
So far piix3 tracks each pirq level and checks whether a given pic pins is
asserted by seeing if each pirq is mapped into the pic pin.
This is independent on irq routing, but data path is on slow path.

Given that irq routing is rarely changed and asserting pic pins is on
data path, the path that asserts pic pins should be optimized and
chainging irq routing should be on slow path.
The new behavior with this patch series is to use bitmap which is addressed
by pirq and pic pins with a given irq routing.
When pirq is asserted, the bitmap is set and see if the pic pins is
asserted by checking the bitmaps.
When irq routing is changed, rebuild the bitmap and re-assert pic pins.

Changes v6 -> v7:
- rebased to pci branch
- added how to test to the commit message

Changes v5 -> v6:
- fixed piix3_set_irq_pic()

Changes v4 -> v5:
- typo

Changes v3 -> v4:
- use pirq, pci_intx instead of irq_num in piix_pci.c
- use symbolic constant PIC_NUM_PINS
- introduced new patch 4/4 which cleans up a bit.

Changes v2 -> v3:
- s/dummy_for_save_load_compat/pci_irq_levels_vmstate/g
- move down unused member of pci_irq_levels_vmstate in the structure
  for cache efficiency

Changes v1 -> v2:
- addressed review comments.

Isaku Yamahata (4):
  pci: add accessor function to get irq levels
  piix_pci: eliminate PIIX3State::pci_irq_levels
  piix_pci: optimize set irq path
  piix_pci: load path clean up

 hw/pci.c  |7 +++
 hw/pci.h  |1 +
 hw/piix_pci.c |  129 ++---
 3 files changed, 112 insertions(+), 25 deletions(-)

[Qemu-devel] [PATCH v7 3/4] piix_pci: optimize set irq path

2011-04-01 Thread Isaku Yamahata

optimize irq routing in piix_pic.c which has been a TODO.
So far piix3 tracks each pirq level and checks whether a given pic pins is
asserted by seeing if each pirq is mapped into the pic pin.
This is independent on irq routing, but data path is on slow path.

Given that irq routing is rarely changed and asserting pic pins is on
data path, the path that asserts pic pins should be optimized and
chainging irq routing should be on slow path.
The new behavior with this patch series is to use bitmap which is addressed
by pirq and pic pins with a given irq routing.
When pirq is asserted, the bitmap is set and see if the pic pins is
asserted by checking the bitmaps.
When irq routing is changed, rebuild the bitmap and re-assert pic pins.

test:
- create VM with 4 e1000 nics in different pci slots
  (i.e. fn=0 for each e1000)
  Thus those e1000's INTA are connected to each PIRQ[A-D].
- run linux as guest and saw each devices triggers interrupt
  by seeing /proc/interrupts. And then confirmed that each PIRQ[A-D]
  surely asserted interrupts.
  Because irq 10 and 11 are shared by 4 e1000's, it only one NIC is activated
  with ifconfig ethN up/down when counting interrupts.

Cc: Michael S. Tsirkin 
Signed-off-by: Isaku Yamahata 
---
Changes v4 -> v5:
- fix piix_set_irq_pic()

Changes v3 -> v4:
- replace irq_num with pirq or pci_intx

Changes v1 -> v2:
- some minor clean ups
- commit log message
---
 hw/piix_pci.c |  101 +++-
 1 files changed, 84 insertions(+), 17 deletions(-)

diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 35e420c..7ffb821 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -37,10 +37,27 @@
 
 typedef PCIHostState I440FXState;
 
+#define PIIX_NUM_PIC_IRQS   16  /* i8259 * 2 */
 #define PIIX_NUM_PIRQS  4ULL/* PIRQ[A-D] */
+#define PIIX_PIRQC  0x60
 
 typedef struct PIIX3State {
 PCIDevice dev;
+
+/*
+ * bitmap to track pic levels.
+ * The pic level is the logical OR of all the PCI irqs mapped to it
+ * So one PIC level is tracked by PIIX_NUM_PIRQS bits.
+ *
+ * PIRQ is mapped to PIC pins, we track it by
+ * PIIX_NUM_PIRQS * PIIX_NUM_PIC_IRQS = 64 bits with
+ * pic_irq * PIIX_NUM_PIRQS + pirq
+ */
+#if PIIX_NUM_PIC_IRQS * PIIX_NUM_PIRQS > 64
+#error "unable to encode pic state in 64bit in pic_levels."
+#endif
+uint64_t pic_levels;
+
 qemu_irq *pic;
 
 /* This member isn't used. Just for save/load compatibility */
@@ -59,16 +76,16 @@ struct PCII440FXState {
 #define I440FX_PAM_SIZE 7
 #define I440FX_SMRAM0x72
 
-static void piix3_set_irq(void *opaque, int irq_num, int level);
+static void piix3_set_irq(void *opaque, int pirq, int level);
 
 /* return the global irq number corresponding to a given device irq
pin. We could also use the bus number to have a more precise
mapping. */
-static int pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
+static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
 {
 int slot_addend;
 slot_addend = (pci_dev->devfn >> 3) - 1;
-return (irq_num + slot_addend) & 3;
+return (pci_intx + slot_addend) & 3;
 }
 
 static void update_pam(PCII440FXState *d, uint32_t start, uint32_t end, int r)
@@ -256,25 +273,64 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int 
*piix3_devfn, qemu_irq *
 }
 
 /* PIIX3 PCI to ISA bridge */
+static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
+{
+qemu_set_irq(piix3->pic[pic_irq],
+ !!(piix3->pic_levels &
+(((1UL << PIIX_NUM_PIRQS) - 1) <<
+ (pic_irq * PIIX_NUM_PIRQS;
+}
 
-static void piix3_set_irq(void *opaque, int irq_num, int level)
+static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level,
+bool propagate)
+{
+int pic_irq;
+uint64_t mask;
+
+pic_irq = piix3->dev.config[PIIX_PIRQC + pirq];
+if (pic_irq >= PIIX_NUM_PIC_IRQS) {
+return;
+}
+
+mask = 1ULL << ((pic_irq * PIIX_NUM_PIRQS) + pirq);
+piix3->pic_levels &= ~mask;
+piix3->pic_levels |= mask * !!level;
+
+if (propagate) {
+piix3_set_irq_pic(piix3, pic_irq);
+}
+}
+
+static void piix3_set_irq(void *opaque, int pirq, int level)
 {
-int i, pic_irq, pic_level;
 PIIX3State *piix3 = opaque;
+piix3_set_irq_level(piix3, pirq, level, true);
+}
 
-/* now we change the pic irq level according to the piix irq mappings */
-/* XXX: optimize */
-pic_irq = piix3->dev.config[0x60 + irq_num];
-if (pic_irq < 16) {
-/* The pic level is the logical OR of all the PCI irqs mapped
-   to it */
-pic_level = 0;
-for (i = 0; i < 4; i++) {
-if (pic_irq == piix3->dev.config[0x60 + i]) {
-pic_level |= pci_bus_get_irq_level(piix3->dev.bus, i);
-}
+/* irq routing is changed. so rebuild bitmap */
+static void piix3_update_irq_levels(PIIX3State *piix3)
+{
+int pirq;
+
+pi

[Qemu-devel] Re: [RFC PATCH] qed: add support for Copy-on-Read

2011-04-01 Thread Anthony Liguori

On 04/01/2011 04:42 AM, Stefan Hajnoczi wrote:

On Wed, Mar 30, 2011 at 08:08:34PM -0500, Anthony Liguori wrote:

When creating an image using qemu-img, just pass '-o copy_on_read' and then
whenever QED reads from a backing file, it will write the block to the QED
file after the read completes ensuring that you only fetch from the backing
device once.

This is very useful for streaming images over a slow connection.

This isn't ready for merge yet as it's not playing nice with synchronize I/O.

What is the issue here? Streaming had issues with aio contexts and
synchronous I/O emulation but copy-on-read by itself looks safe to me.

Here's the scenario that fails for me although I'm starting to suspect
block/curl as the real culprit.

qemu-img create -f qed -o copy_on_read -b
http://linux.nssl.noaa.gov/fedora/fedora/linux/releases/14/Fedora/x86_64/iso/Fedora-14-x86_64-DVD.iso
cached_iso.img

qemu -cdrom cache_iso.img -boot d

And I have a patch that does a bunch of synchronous reads of the disk.

I think it's fairly easy to do the same thing in qcow2 by just hooking adding
some logic after bdrv_aio_write() to call back into qcow2 with a synchronous
I/O write in the backing file case. Thoughts on whether that would actually
work?

Why not do the follow-up .bdrv_aio_writev() for qcow2 too? I don't see
a reason to do it synchronously.

Oh, I assumed with coroutines that that would be the preference in
qcow2. If not, AIO is just as good for me :-)

Regards,

Anthony Liguori

Stefan

Re: [Qemu-devel] [RFC PATCH] qed: add support for Copy-on-Read

2011-04-01 Thread Anthony Liguori


On 04/01/2011 06:11 AM, Kevin Wolf wrote:

Am 31.03.2011 03:08, schrieb Anthony Liguori:

When creating an image using qemu-img, just pass '-o copy_on_read' and then
whenever QED reads from a backing file, it will write the block to the QED
file after the read completes ensuring that you only fetch from the backing
device once.

As you probably know, I don't agree with the interface. Copy on read
should be first and foremost a runtime option. It's okay to fetch the
default for this option from the image file, but it's not the right
primary interface.


That's the main reason I posted this.  I wanted to revisit that 
discussion and see if we're any close to having a primary interface for 
this.


I think blockdev is still a ways off.  Would a -drive 
file=image.img,cor=on make sense as an intermediate mechanism?



This is very useful for streaming images over a slow connection.

This isn't ready for merge yet as it's not playing nice with synchronize I/O.

Like Stefan I'm not sure what you mean here.

One problem that I see is that if you have a concurrent write request
from the guest, the COR write request may overwrite the guest's request,
which is obviously wrong. Is this what you mean?


Yes, I think you're right here but I don't think this is the issue.  But 
surely the semantics of a simultaneous read/write are undefined at least 
on the read side.  I guess having the write be undefined is unexpected.


Regards,

Anthony Liguori


Kevin

Re: [Qemu-devel] [RFC PATCH] qed: add support for Copy-on-Read

2011-04-01 Thread Kevin Wolf

Am 01.04.2011 14:36, schrieb Anthony Liguori:
> On 04/01/2011 06:11 AM, Kevin Wolf wrote:
>> Am 31.03.2011 03:08, schrieb Anthony Liguori:
>>> When creating an image using qemu-img, just pass '-o copy_on_read' and then
>>> whenever QED reads from a backing file, it will write the block to the QED
>>> file after the read completes ensuring that you only fetch from the backing
>>> device once.
>> As you probably know, I don't agree with the interface. Copy on read
>> should be first and foremost a runtime option. It's okay to fetch the
>> default for this option from the image file, but it's not the right
>> primary interface.
> 
> That's the main reason I posted this.  I wanted to revisit that 
> discussion and see if we're any close to having a primary interface for 
> this.
> 
> I think blockdev is still a ways off.  Would a -drive 
> file=image.img,cor=on make sense as an intermediate mechanism?

I don't like it much (and I think neither do you), but I agree that
blockdev won't be there tomorrow, so I wouldn't object to it.

>>> This is very useful for streaming images over a slow connection.
>>>
>>> This isn't ready for merge yet as it's not playing nice with synchronize 
>>> I/O.
>> Like Stefan I'm not sure what you mean here.
>>
>> One problem that I see is that if you have a concurrent write request
>> from the guest, the COR write request may overwrite the guest's request,
>> which is obviously wrong. Is this what you mean?
> 
> Yes, I think you're right here but I don't think this is the issue.  But 
> surely the semantics of a simultaneous read/write are undefined at least 
> on the read side.  I guess having the write be undefined is unexpected.

Yes and no. If you have the read and the write to same sector, then it
would be undefined whether the read returns the old or the new data. But
if the write completes successfully, the sector must have been updated.
I doubt that this is something that guests usually do.

However, we're not operating on a sector level, but on a cluster level
here. So the guest could actually access two different sectors and still
get its write request overwritten by the COR.

Kevin

Re: [Qemu-devel] [PATCH v24 00/10] usb-ccid

2011-04-01 Thread Anthony Liguori


On 03/28/2011 11:11 AM, Alon Levy wrote:

This patchset adds three new devices, usb-ccid, ccid-card-passthru and
ccid-card-emulated, providing a CCID bus, a simple passthru protocol
implementing card requiring a client, and a standalone emulated card.

It also introduces a new directory libcaccard with CAC card emulation,
CAC is a type of ISO 7816 smart card.

Tree for pull: git://anongit.freedesktop.org/~alon/qemu usb_ccid.v23


CC libcacard/cac.o
In file included from /home/anthony/git/qemu/libcacard/cac.c:8:
/home/anthony/git/qemu/qemu-common.h:5:25: error: config-host.h: No such 
file or directory

In file included from /home/anthony/git/qemu/libcacard/cac.c:8:
/home/anthony/git/qemu/qemu-common.h:63: error: redefinition of ‘struct 
iovec’

make[1]: *** [cac.o] Error 1
make: *** [subdir-libcacard] Error 2

This is building from a separate directory. Let me know if you can't 
reproduce.


Regards,

Anthony Liguori


v23->v24 changes:
  * libcacard:
   = changed patches: (that need re-review)
* 6 - libcacard: initial commit
* 7 - libcacard: add vscclient
* 5 - ccid: add passthru card device
   * drop "libcacard add passthru" patch, not ready, not used.
   * remove unrequired includes
   * use stderr in vscclient for printing errors
   * cosmetic fixes

v22->v23 changes:
  * libcacard
   * configure fixes: (reported by Stefan Hajnoczi)
* test a = b, not a == b (second isn't portable)
* quote $source_path in case it contains spaces
 - this doesn't really help since there are many other places
   that need similar fixes, not introduced by this patch.

v21->v22 changes:
  * libcacard:
   * fix configure to not link libcacard if nss not found
  (reported by Stefan Hajnoczi)
   * fix vscclient linkage with simpletrace backend
  (reported by Stefan Hajnoczi)
   * card_7816.c: add missing break in ERROR_DATA_NOT_FOUND
  (reported by William van de Velde)

v20->v21 changes:
  * all: cosmetics
  * libcacard, ccid-card-passthru:
   * use qemu-{malloc,free} and qemu-thread, error_report
  * libcacard:
   * split to multiple patches

v19->v20 changes:
  * checkpatch.pl. Here are the remaining errors with explanation:
   * ignored 5 macro errors of the type
"ERROR: Macros with complex values should be enclosed in parenthesis"
because fixing them breaks current code, if it really bothers someone
I can fix it.
* four of them are in libcacard/card_7816t.h:
/* give the subfields a unified look */
..
#define a_cla a_header->ah_cla /* class */
#define a_ins a_header->ah_ins /* instruction */
#define a_p1 a_header->ah_p1   /* parameter 1 */
#define a_p2 a_header->ah_p2   /* parameter 2 */
* and the fifth:
#4946: FILE: libcacard/vcardt.h:31:
+#define VCARD_ATR_PREFIX(size) 0x3b, 0x66+(size), 0x00, 0xff, \
+   'V', 'C', 'A', 'R', 'D', '_'
   * Ignored this warning since I couldn't figure it out, and it's a test
file:
WARNING: externs should be avoided in .c files
#2343: FILE: libcacard/link_test.c:7:
+VCardStatus cac_card_init(const char *flags, VCard *card,

v18-v19 changes:
  * more merges, down to a single digit number of patches.
  * drop enumeration property, use string.
  * rebased (trivial)

v17-v18 changes:
  * merge vscard_common.h patches.
  * actually provide a tree to pull.

v16-v17 changes:
  * merged all the "v15->v16" patches
  * merged some more wherever it was easy (all same file commits).
  * added signed off by to first four patches
  * ccid.h: added copyright, removed underscore in defines, and replaced
  non C89 comments

v15-v16 changes:
  * split vscard_common introducing patch for ease of review
  * sum of commit logs for the v15-v16 commits: (whitespace fixes
 removed for space, see original commit messages in later patches)
   * usb-ccid:
* fix abort on client answer after card remove
* enable migration
* remove side affect code from asserts
* return consistent self-powered state
* mask out reserved bits in ccid_set_parameters
* add missing abRFU in SetParameters (no affect on linux guest)
   * vscard_common.h protocol change:
* VSCMsgInit capabilities and magic
* removed ReaderResponse, will use Error instead with code==VSC_SUCCESS.
* added Flush and FlushComplete, remove Reconnect.
* define VSCARD_MAGIC
* added error code VSC_SUCCESS.
   * ccid-card-passthru
* return correct size
* return error instead of assert if client sent too large ATR
* don't assert if client sent too large a size, but add asserts for indices 
to buffer
* reset vscard_in indices on chardev disconnect
* handle init from client
* error if no chardev supplied
* use ntoh, hton
* eradicate reader_id_t
* remove Reconnect usage (removed from VSCARD protocol)
* send VSC_SUCCESS on card insert/remove and reader add/remove
   * ccid-card-emulated
* fix error reporting in initfn

v14-v15 changes:
  * add patch with --enable-smartcard and --disable-

Re: [Qemu-devel] Re: [PATCH v2] qemu-img: Initial progress printing support

2011-04-01 Thread Jes Sorensen

On 03/31/11 13:49, Stefan Hajnoczi wrote:
> On Thu, Mar 31, 2011 at 12:38 PM, Kevin Wolf  wrote:
>> Am 31.03.2011 13:15, schrieb Jes Sorensen:
>>> On 03/31/11 12:38, Kevin Wolf wrote:
>>> I have been a little reluctant to do this because it will break the ABI
>>> for tools running qemu-img from a GUI etc.
>>
>> That's the reason for the "from a terminal" part. If we check for
>> isatty(), we should handle these cases just fine.
> 
> Yes, I think checking for a tty is enough precaution and allows users
> to get the benefit of the progress bar.  TBH I'd probably forget to
> add -p half the time :).

Ok, this is fine with me - however how do you suggest we offer the
option to disable it on the command line, an additional flag?

Jes

[Qemu-devel] [PATCH 09/10] target-arm: Don't leak TCG temp for UNDEFs in Neon load/store space

2011-04-01 Thread Peter Maydell

Move the allocation and freeing of the TCG temp used for the address for
Neon load/store instructions so that we don't allocate the temporary
until we've done enough decoding to know that the instruction is not
an UNDEF pattern; this avoids leaking the TCG temp in these cases.

Signed-off-by: Peter Maydell 
---
 target-arm/translate.c |8 ++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index e79ea03..527e260 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -3810,7 +3810,6 @@ static int disas_neon_ls_insn(CPUState * env, 
DisasContext *s, uint32_t insn)
 rn = (insn >> 16) & 0xf;
 rm = insn & 0xf;
 load = (insn & (1 << 21)) != 0;
-addr = tcg_temp_new_i32();
 if ((insn & (1 << 23)) == 0) {
 /* Load store all elements.  */
 op = (insn >> 8) & 0xf;
@@ -3822,6 +3821,7 @@ static int disas_neon_ls_insn(CPUState * env, 
DisasContext *s, uint32_t insn)
 spacing = neon_ls_element_type[op].spacing;
 if (size == 3 && (interleave | spacing) != 1)
 return 1;
+addr = tcg_temp_new_i32();
 load_reg_var(s, addr, rn);
 stride = (1 << size) * interleave;
 for (reg = 0; reg < nregs; reg++) {
@@ -3907,6 +3907,7 @@ static int disas_neon_ls_insn(CPUState * env, 
DisasContext *s, uint32_t insn)
 }
 rd += spacing;
 }
+tcg_temp_free_i32(addr);
 stride = nregs * 8;
 } else {
 size = (insn >> 10) & 3;
@@ -3932,6 +3933,7 @@ static int disas_neon_ls_insn(CPUState * env, 
DisasContext *s, uint32_t insn)
 if (nregs == 3 && a == 1) {
 return 1;
 }
+addr = tcg_temp_new_i32();
 load_reg_var(s, addr, rn);
 if (nregs == 1) {
 /* VLD1 to all lanes: bit 5 indicates how many Dregs to write 
*/
@@ -3955,6 +3957,7 @@ static int disas_neon_ls_insn(CPUState * env, 
DisasContext *s, uint32_t insn)
 rd += stride;
 }
 }
+tcg_temp_free_i32(addr);
 stride = (1 << size) * nregs;
 } else {
 /* Single element.  */
@@ -3976,6 +3979,7 @@ static int disas_neon_ls_insn(CPUState * env, 
DisasContext *s, uint32_t insn)
 abort();
 }
 nregs = ((insn >> 8) & 3) + 1;
+addr = tcg_temp_new_i32();
 load_reg_var(s, addr, rn);
 for (reg = 0; reg < nregs; reg++) {
 if (load) {
@@ -4017,10 +4021,10 @@ static int disas_neon_ls_insn(CPUState * env, 
DisasContext *s, uint32_t insn)
 rd += stride;
 tcg_gen_addi_i32(addr, addr, 1 << size);
 }
+tcg_temp_free_i32(addr);
 stride = nregs * (1 << size);
 }
 }
-tcg_temp_free_i32(addr);
 if (rm != 15) {
 TCGv base;
 
-- 
1.7.1

[Qemu-devel] [PATCH 02/10] target-arm/neon_helper.c: Use make_float32/float32_val macros

2011-04-01 Thread Peter Maydell

Use the softfloat make_float32 and float32_val macros to convert between
softfloat's float32 type and raw uint32_t types, rather than private
conversion functions.

Signed-off-by: Peter Maydell 
Reviewed-by: Nathan Froyd 
---
 target-arm/neon_helper.c |   56 ++
 1 files changed, 17 insertions(+), 39 deletions(-)

diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index 97bc1e6..4039036 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -20,29 +20,6 @@
 
 #define NFS (&env->vfp.standard_fp_status)
 
-/* Helper routines to perform bitwise copies between float and int.  */
-static inline float32 vfp_itos(uint32_t i)
-{
-union {
-uint32_t i;
-float32 s;
-} v;
-
-v.i = i;
-return v.s;
-}
-
-static inline uint32_t vfp_stoi(float32 s)
-{
-union {
-uint32_t i;
-float32 s;
-} v;
-
-v.s = s;
-return v.i;
-}
-
 #define NEON_TYPE1(name, type) \
 typedef struct \
 { \
@@ -1795,50 +1772,51 @@ uint32_t HELPER(neon_qneg_s32)(CPUState *env, uint32_t 
x)
 /* NEON Float helpers.  */
 uint32_t HELPER(neon_min_f32)(CPUState *env, uint32_t a, uint32_t b)
 {
-float32 f0 = vfp_itos(a);
-float32 f1 = vfp_itos(b);
+float32 f0 = make_float32(a);
+float32 f1 = make_float32(b);
 return (float32_compare_quiet(f0, f1, NFS) == -1) ? a : b;
 }
 
 uint32_t HELPER(neon_max_f32)(CPUState *env, uint32_t a, uint32_t b)
 {
-float32 f0 = vfp_itos(a);
-float32 f1 = vfp_itos(b);
+float32 f0 = make_float32(a);
+float32 f1 = make_float32(b);
 return (float32_compare_quiet(f0, f1, NFS) == 1) ? a : b;
 }
 
 uint32_t HELPER(neon_abd_f32)(CPUState *env, uint32_t a, uint32_t b)
 {
-float32 f0 = vfp_itos(a);
-float32 f1 = vfp_itos(b);
-return vfp_stoi((float32_compare_quiet(f0, f1, NFS) == 1)
+float32 f0 = make_float32(a);
+float32 f1 = make_float32(b);
+return float32_val((float32_compare_quiet(f0, f1, NFS) == 1)
 ? float32_sub(f0, f1, NFS)
 : float32_sub(f1, f0, NFS));
 }
 
 uint32_t HELPER(neon_add_f32)(CPUState *env, uint32_t a, uint32_t b)
 {
-return vfp_stoi(float32_add(vfp_itos(a), vfp_itos(b), NFS));
+return float32_val(float32_add(make_float32(a), make_float32(b), NFS));
 }
 
 uint32_t HELPER(neon_sub_f32)(CPUState *env, uint32_t a, uint32_t b)
 {
-return vfp_stoi(float32_sub(vfp_itos(a), vfp_itos(b), NFS));
+return float32_val(float32_sub(make_float32(a), make_float32(b), NFS));
 }
 
 uint32_t HELPER(neon_mul_f32)(CPUState *env, uint32_t a, uint32_t b)
 {
-return vfp_stoi(float32_mul(vfp_itos(a), vfp_itos(b), NFS));
+return float32_val(float32_mul(make_float32(a), make_float32(b), NFS));
 }
 
 /* Floating point comparisons produce an integer result.  */
 #define NEON_VOP_FCMP(name, cmp) \
 uint32_t HELPER(neon_##name)(CPUState *env, uint32_t a, uint32_t b) \
 { \
-if (float32_compare_quiet(vfp_itos(a), vfp_itos(b), NFS) cmp 0) \
+if (float32_compare_quiet(make_float32(a), make_float32(b), NFS) cmp 0) { \
 return ~0; \
-else \
+} else { \
 return 0; \
+} \
 }
 
 NEON_VOP_FCMP(ceq_f32, ==)
@@ -1847,15 +1825,15 @@ NEON_VOP_FCMP(cgt_f32, >)
 
 uint32_t HELPER(neon_acge_f32)(CPUState *env, uint32_t a, uint32_t b)
 {
-float32 f0 = float32_abs(vfp_itos(a));
-float32 f1 = float32_abs(vfp_itos(b));
+float32 f0 = float32_abs(make_float32(a));
+float32 f1 = float32_abs(make_float32(b));
 return (float32_compare_quiet(f0, f1,NFS) >= 0) ? ~0 : 0;
 }
 
 uint32_t HELPER(neon_acgt_f32)(CPUState *env, uint32_t a, uint32_t b)
 {
-float32 f0 = float32_abs(vfp_itos(a));
-float32 f1 = float32_abs(vfp_itos(b));
+float32 f0 = float32_abs(make_float32(a));
+float32 f1 = float32_abs(make_float32(b));
 return (float32_compare_quiet(f0, f1, NFS) > 0) ? ~0 : 0;
 }
 
-- 
1.7.1

[Qemu-devel] [PATCH 06/10] softfloat: Add float_min() and float_max() functions

2011-04-01 Thread Peter Maydell

Add min and max operations to softfloat. This allows us to implement
propagation of NaNs and handling of negative zero correctly (unlike
the approach of having target helper routines return one of the operands
based on the result of a comparison op).

Signed-off-by: Peter Maydell 
---
 fpu/softfloat.c |   49 +
 fpu/softfloat.h |4 
 2 files changed, 53 insertions(+), 0 deletions(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 08e4ae0..03fb948 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -6057,6 +6057,55 @@ int float128_compare_quiet( float128 a, float128 b 
STATUS_PARAM )
 return float128_compare_internal(a, b, 1 STATUS_VAR);
 }
 
+/* min() and max() functions. These can't be implemented as
+ * 'compare and pick one input' because that would mishandle
+ * NaNs and +0 vs -0.
+ */
+#define MINMAX(s, nan_exp)  \
+INLINE float ## s float ## s ## _minmax(float ## s a, float ## s b, \
+int ismin STATUS_PARAM )\
+{   \
+flag aSign, bSign;  \
+uint ## s ## _t av, bv; \
+a = float ## s ## _squash_input_denormal(a STATUS_VAR); \
+b = float ## s ## _squash_input_denormal(b STATUS_VAR); \
+if (float ## s ## _is_any_nan(a) || \
+float ## s ## _is_any_nan(b)) { \
+return propagateFloat ## s ## NaN(a, b STATUS_VAR); \
+}   \
+aSign = extractFloat ## s ## Sign(a);   \
+bSign = extractFloat ## s ## Sign(b);   \
+av = float ## s ## _val(a); \
+bv = float ## s ## _val(b); \
+if (aSign != bSign) {   \
+if (ismin) {\
+return aSign ? a : b;   \
+} else {\
+return aSign ? b : a;   \
+}   \
+} else {\
+if (ismin) {\
+return (aSign ^ (av < bv)) ? a : b; \
+} else {\
+return (aSign ^ (av < bv)) ? b : a; \
+}   \
+}   \
+}   \
+\
+float ## s float ## s ## _min(float ## s a, float ## s b STATUS_PARAM)  \
+{   \
+return float ## s ## _minmax(a, b, 1 STATUS_VAR);   \
+}   \
+\
+float ## s float ## s ## _max(float ## s a, float ## s b STATUS_PARAM)  \
+{   \
+return float ## s ## _minmax(a, b, 0 STATUS_VAR);   \
+}
+
+MINMAX(32, 0xff)
+MINMAX(64, 0x7ff)
+
+
 /* Multiply A by 2 raised to the power N.  */
 float32 float32_scalbn( float32 a, int n STATUS_PARAM )
 {
diff --git a/fpu/softfloat.h b/fpu/softfloat.h
index 5d05fa5..90f4250 100644
--- a/fpu/softfloat.h
+++ b/fpu/softfloat.h
@@ -324,6 +324,8 @@ int float32_le_quiet( float32, float32 STATUS_PARAM );
 int float32_lt_quiet( float32, float32 STATUS_PARAM );
 int float32_compare( float32, float32 STATUS_PARAM );
 int float32_compare_quiet( float32, float32 STATUS_PARAM );
+float32 float32_min(float32, float32 STATUS_PARAM);
+float32 float32_max(float32, float32 STATUS_PARAM);
 int float32_is_quiet_nan( float32 );
 int float32_is_signaling_nan( float32 );
 float32 float32_maybe_silence_nan( float32 );
@@ -436,6 +438,8 @@ int float64_le_quiet( float64, float64 STATUS_PARAM );
 int float64_lt_quiet( float64, float64 STATUS_PARAM );
 int float64_compare( float64, float64 STATUS_PARAM );
 int float64_compare_quiet( float64, float64 STATUS_PARAM );
+float64 float64_min(float64, float64 STATUS_PARAM);
+float64 float64_max(float64, float64 STATUS_PARAM);
 int float64_is_quiet_nan( float64 a );
 int float64_is_signaling_nan( float64 );
 float64 float64_m

[Qemu-devel] [PATCH 04/10] target-arm: Fix VCLE.F32 #0, VCLT.F32 #0 NaN handling

2011-04-01 Thread Peter Maydell

Implementing the floating-point versions of VCLE #0 and VCLT #0 by
doing a GT comparison and inverting the result gives the wrong
result if the input is a NaN. Implement as a GT comparison with the
operands swapped instead.

Signed-off-by: Peter Maydell 
---
 target-arm/translate.c |   18 --
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index cf2440e..6ce8b7a 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -5641,25 +5641,31 @@ static int disas_neon_data_insn(CPUState * env, 
DisasContext *s, uint32_t insn)
 gen_neon_rsb(size, tmp, tmp2);
 tcg_temp_free(tmp2);
 break;
-case 24: case 27: /* Float VCGT #0, Float VCLE #0 */
+case 24: /* Float VCGT #0 */
 tmp2 = tcg_const_i32(0);
 gen_helper_neon_cgt_f32(tmp,  cpu_env, tmp, tmp2);
 tcg_temp_free(tmp2);
-if (op == 27)
-tcg_gen_not_i32(tmp, tmp);
 break;
-case 25: case 28: /* Float VCGE #0, Float VCLT #0 */
+case 25: /* Float VCGE #0 */
 tmp2 = tcg_const_i32(0);
 gen_helper_neon_cge_f32(tmp,  cpu_env, tmp, tmp2);
 tcg_temp_free(tmp2);
-if (op == 28)
-tcg_gen_not_i32(tmp, tmp);
 break;
 case 26: /* Float VCEQ #0 */
 tmp2 = tcg_const_i32(0);
 gen_helper_neon_ceq_f32(tmp,  cpu_env, tmp, tmp2);
 tcg_temp_free(tmp2);
 break;
+case 27: /* Float VCLE #0 */
+tmp2 = tcg_const_i32(0);
+gen_helper_neon_cge_f32(tmp,  cpu_env, tmp2, tmp);
+tcg_temp_free(tmp2);
+break;
+case 28: /* Float VCLT #0 */
+tmp2 = tcg_const_i32(0);
+gen_helper_neon_cgt_f32(tmp,  cpu_env, tmp2, tmp);
+tcg_temp_free(tmp2);
+break;
 case 30: /* Float VABS */
 gen_vfp_abs(0);
 break;
-- 
1.7.1

[Qemu-devel] [PATCH 00/10] [PULL] ARM Neon fixes

2011-04-01 Thread Peter Maydell

This is a pull request for a set of patches fixing various minor Neon
problems for ARM targets, which I sent to the list a couple of weeks ago.
A few of them got reviewed-by Nathan, one had some minor discussion which
didn't amount to a request for any change, the rest had no comments.

The softfloat patch includes a trivial change since the original
posted version: the MINMAX macro now has a "uint ## s ## _t" rather than
a "bits ## s" since the bits32/bits64 types were removed in commit bb98fe4.

The following changes since commit a5086f95421e43c7b9e1b28a111aae0be4848117:

  lm32: use lookup table for opcodes (2011-03-31 08:54:05 +0200)

are available in the git repository at:
  git://git.linaro.org/people/pmaydell/qemu-arm.git for-upstream

Peter Maydell (10):
  target-arm: Make Neon helper routines use correct FP status
  target-arm/neon_helper.c: Use make_float32/float32_val macros
  target-arm: Return right result for Neon comparison with NaNs
  target-arm: Fix VCLE.F32 #0, VCLT.F32 #0 NaN handling
  target-arm: Correct ABD's handling of negative zeroes
  softfloat: Add float*_min() and float*_max() functions
  target-arm: Use new softfloat min/max functions for VMAX, VMIN
  target-arm: Fix VLD of single element to all lanes
  target-arm: Don't leak TCG temp for UNDEFs in Neon load/store space
  target-arm/helper.c: For float-int conversion helpers pass ints as ints

 fpu/softfloat.c  |   49 +++
 fpu/softfloat.h  |4 +
 target-arm/helper.c  |  155 -
 target-arm/helpers.h |   82 
 target-arm/neon_helper.c |  103 --
 target-arm/translate.c   |  152 +
 6 files changed, 289 insertions(+), 256 deletions(-)

[Qemu-devel] [PATCH 07/10] target-arm: Use new softfloat min/max functions for VMAX, VMIN

2011-04-01 Thread Peter Maydell

Use the new softfloat min/max functions to implement the Neon VMAX
and VMIN instructions. This allows us to get the right behaviour
for NaN and negative zero.

Signed-off-by: Peter Maydell 
---
 target-arm/neon_helper.c |8 ++--
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index 1905545..19784ab 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -1772,16 +1772,12 @@ uint32_t HELPER(neon_qneg_s32)(CPUState *env, uint32_t 
x)
 /* NEON Float helpers.  */
 uint32_t HELPER(neon_min_f32)(CPUState *env, uint32_t a, uint32_t b)
 {
-float32 f0 = make_float32(a);
-float32 f1 = make_float32(b);
-return (float32_compare_quiet(f0, f1, NFS) == -1) ? a : b;
+return float32_val(float32_min(make_float32(a), make_float32(b), NFS));
 }
 
 uint32_t HELPER(neon_max_f32)(CPUState *env, uint32_t a, uint32_t b)
 {
-float32 f0 = make_float32(a);
-float32 f1 = make_float32(b);
-return (float32_compare_quiet(f0, f1, NFS) == 1) ? a : b;
+return float32_val(float32_max(make_float32(a), make_float32(b), NFS));
 }
 
 uint32_t HELPER(neon_abd_f32)(CPUState *env, uint32_t a, uint32_t b)
-- 
1.7.1

[Qemu-devel] [PATCH 10/10] target-arm/helper.c: For float-int conversion helpers pass ints as ints

2011-04-01 Thread Peter Maydell

Correct the argument and return types for the float<->int conversion helper
functions so that integer arguments and return values are declared as
uint32_t/uint64_t, not float32/float64. This allows us to remove the
hand-rolled functions which were doing bitwise copies between the types
via unions.

Signed-off-by: Peter Maydell 
Reviewed-by: Nathan Froyd 
---
 target-arm/helper.c  |  155 ++
 target-arm/helpers.h |   60 ++--
 2 files changed, 85 insertions(+), 130 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 78f3d39..6788a4c 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2486,135 +2486,90 @@ DO_VFP_cmp(s, float32)
 DO_VFP_cmp(d, float64)
 #undef DO_VFP_cmp
 
-/* Helper routines to perform bitwise copies between float and int.  */
-static inline float32 vfp_itos(uint32_t i)
-{
-union {
-uint32_t i;
-float32 s;
-} v;
-
-v.i = i;
-return v.s;
-}
-
-static inline uint32_t vfp_stoi(float32 s)
-{
-union {
-uint32_t i;
-float32 s;
-} v;
-
-v.s = s;
-return v.i;
-}
-
-static inline float64 vfp_itod(uint64_t i)
-{
-union {
-uint64_t i;
-float64 d;
-} v;
-
-v.i = i;
-return v.d;
-}
-
-static inline uint64_t vfp_dtoi(float64 d)
-{
-union {
-uint64_t i;
-float64 d;
-} v;
-
-v.d = d;
-return v.i;
-}
-
 /* Integer to float conversion.  */
-float32 VFP_HELPER(uito, s)(float32 x, CPUState *env)
+float32 VFP_HELPER(uito, s)(uint32_t x, CPUState *env)
 {
-return uint32_to_float32(vfp_stoi(x), &env->vfp.fp_status);
+return uint32_to_float32(x, &env->vfp.fp_status);
 }
 
-float64 VFP_HELPER(uito, d)(float32 x, CPUState *env)
+float64 VFP_HELPER(uito, d)(uint32_t x, CPUState *env)
 {
-return uint32_to_float64(vfp_stoi(x), &env->vfp.fp_status);
+return uint32_to_float64(x, &env->vfp.fp_status);
 }
 
-float32 VFP_HELPER(sito, s)(float32 x, CPUState *env)
+float32 VFP_HELPER(sito, s)(uint32_t x, CPUState *env)
 {
-return int32_to_float32(vfp_stoi(x), &env->vfp.fp_status);
+return int32_to_float32(x, &env->vfp.fp_status);
 }
 
-float64 VFP_HELPER(sito, d)(float32 x, CPUState *env)
+float64 VFP_HELPER(sito, d)(uint32_t x, CPUState *env)
 {
-return int32_to_float64(vfp_stoi(x), &env->vfp.fp_status);
+return int32_to_float64(x, &env->vfp.fp_status);
 }
 
 /* Float to integer conversion.  */
-float32 VFP_HELPER(toui, s)(float32 x, CPUState *env)
+uint32_t VFP_HELPER(toui, s)(float32 x, CPUState *env)
 {
 if (float32_is_any_nan(x)) {
-return float32_zero;
+return 0;
 }
-return vfp_itos(float32_to_uint32(x, &env->vfp.fp_status));
+return float32_to_uint32(x, &env->vfp.fp_status);
 }
 
-float32 VFP_HELPER(toui, d)(float64 x, CPUState *env)
+uint32_t VFP_HELPER(toui, d)(float64 x, CPUState *env)
 {
 if (float64_is_any_nan(x)) {
-return float32_zero;
+return 0;
 }
-return vfp_itos(float64_to_uint32(x, &env->vfp.fp_status));
+return float64_to_uint32(x, &env->vfp.fp_status);
 }
 
-float32 VFP_HELPER(tosi, s)(float32 x, CPUState *env)
+uint32_t VFP_HELPER(tosi, s)(float32 x, CPUState *env)
 {
 if (float32_is_any_nan(x)) {
-return float32_zero;
+return 0;
 }
-return vfp_itos(float32_to_int32(x, &env->vfp.fp_status));
+return float32_to_int32(x, &env->vfp.fp_status);
 }
 
-float32 VFP_HELPER(tosi, d)(float64 x, CPUState *env)
+uint32_t VFP_HELPER(tosi, d)(float64 x, CPUState *env)
 {
 if (float64_is_any_nan(x)) {
-return float32_zero;
+return 0;
 }
-return vfp_itos(float64_to_int32(x, &env->vfp.fp_status));
+return float64_to_int32(x, &env->vfp.fp_status);
 }
 
-float32 VFP_HELPER(touiz, s)(float32 x, CPUState *env)
+uint32_t VFP_HELPER(touiz, s)(float32 x, CPUState *env)
 {
 if (float32_is_any_nan(x)) {
-return float32_zero;
+return 0;
 }
-return vfp_itos(float32_to_uint32_round_to_zero(x, &env->vfp.fp_status));
+return float32_to_uint32_round_to_zero(x, &env->vfp.fp_status);
 }
 
-float32 VFP_HELPER(touiz, d)(float64 x, CPUState *env)
+uint32_t VFP_HELPER(touiz, d)(float64 x, CPUState *env)
 {
 if (float64_is_any_nan(x)) {
-return float32_zero;
+return 0;
 }
-return vfp_itos(float64_to_uint32_round_to_zero(x, &env->vfp.fp_status));
+return float64_to_uint32_round_to_zero(x, &env->vfp.fp_status);
 }
 
-float32 VFP_HELPER(tosiz, s)(float32 x, CPUState *env)
+uint32_t VFP_HELPER(tosiz, s)(float32 x, CPUState *env)
 {
 if (float32_is_any_nan(x)) {
-return float32_zero;
+return 0;
 }
-return vfp_itos(float32_to_int32_round_to_zero(x, &env->vfp.fp_status));
+return float32_to_int32_round_to_zero(x, &env->vfp.fp_status);
 }
 
-float32 VFP_HELPER(tosiz, d)(float64 x, CPUState *env)
+uint32_t VFP_HELPER(tosiz, d)(float64 x, CPUState *env)
 {
 if (float64_is_any_na

[Qemu-devel] [PATCH 01/10] target-arm: Make Neon helper routines use correct FP status

2011-04-01 Thread Peter Maydell

Make the Neon helper routines use the correct FP status from
the CPUEnv rather than using a dummy static one. This means
they will correctly handle denormals and NaNs and will set
FPSCR exception bits properly.

Signed-off-by: Peter Maydell 
---
 target-arm/helpers.h |   22 +++---
 target-arm/neon_helper.c |   21 ++---
 target-arm/translate.c   |   42 ++
 3 files changed, 43 insertions(+), 42 deletions(-)

diff --git a/target-arm/helpers.h b/target-arm/helpers.h
index bd6977c..e2260b6 100644
--- a/target-arm/helpers.h
+++ b/target-arm/helpers.h
@@ -350,17 +350,17 @@ DEF_HELPER_2(neon_qneg_s8, i32, env, i32)
 DEF_HELPER_2(neon_qneg_s16, i32, env, i32)
 DEF_HELPER_2(neon_qneg_s32, i32, env, i32)
 
-DEF_HELPER_2(neon_min_f32, i32, i32, i32)
-DEF_HELPER_2(neon_max_f32, i32, i32, i32)
-DEF_HELPER_2(neon_abd_f32, i32, i32, i32)
-DEF_HELPER_2(neon_add_f32, i32, i32, i32)
-DEF_HELPER_2(neon_sub_f32, i32, i32, i32)
-DEF_HELPER_2(neon_mul_f32, i32, i32, i32)
-DEF_HELPER_2(neon_ceq_f32, i32, i32, i32)
-DEF_HELPER_2(neon_cge_f32, i32, i32, i32)
-DEF_HELPER_2(neon_cgt_f32, i32, i32, i32)
-DEF_HELPER_2(neon_acge_f32, i32, i32, i32)
-DEF_HELPER_2(neon_acgt_f32, i32, i32, i32)
+DEF_HELPER_3(neon_min_f32, i32, env, i32, i32)
+DEF_HELPER_3(neon_max_f32, i32, env, i32, i32)
+DEF_HELPER_3(neon_abd_f32, i32, env, i32, i32)
+DEF_HELPER_3(neon_add_f32, i32, env, i32, i32)
+DEF_HELPER_3(neon_sub_f32, i32, env, i32, i32)
+DEF_HELPER_3(neon_mul_f32, i32, env, i32, i32)
+DEF_HELPER_3(neon_ceq_f32, i32, env, i32, i32)
+DEF_HELPER_3(neon_cge_f32, i32, env, i32, i32)
+DEF_HELPER_3(neon_cgt_f32, i32, env, i32, i32)
+DEF_HELPER_3(neon_acge_f32, i32, env, i32, i32)
+DEF_HELPER_3(neon_acgt_f32, i32, env, i32, i32)
 
 /* iwmmxt_helper.c */
 DEF_HELPER_2(iwmmxt_maddsq, i64, i64, i64)
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index 002a9c1..97bc1e6 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -18,8 +18,7 @@
 
 #define SET_QC() env->vfp.xregs[ARM_VFP_FPSCR] = CPSR_Q
 
-static float_status neon_float_status;
-#define NFS &neon_float_status
+#define NFS (&env->vfp.standard_fp_status)
 
 /* Helper routines to perform bitwise copies between float and int.  */
 static inline float32 vfp_itos(uint32_t i)
@@ -1794,21 +1793,21 @@ uint32_t HELPER(neon_qneg_s32)(CPUState *env, uint32_t 
x)
 }
 
 /* NEON Float helpers.  */
-uint32_t HELPER(neon_min_f32)(uint32_t a, uint32_t b)
+uint32_t HELPER(neon_min_f32)(CPUState *env, uint32_t a, uint32_t b)
 {
 float32 f0 = vfp_itos(a);
 float32 f1 = vfp_itos(b);
 return (float32_compare_quiet(f0, f1, NFS) == -1) ? a : b;
 }
 
-uint32_t HELPER(neon_max_f32)(uint32_t a, uint32_t b)
+uint32_t HELPER(neon_max_f32)(CPUState *env, uint32_t a, uint32_t b)
 {
 float32 f0 = vfp_itos(a);
 float32 f1 = vfp_itos(b);
 return (float32_compare_quiet(f0, f1, NFS) == 1) ? a : b;
 }
 
-uint32_t HELPER(neon_abd_f32)(uint32_t a, uint32_t b)
+uint32_t HELPER(neon_abd_f32)(CPUState *env, uint32_t a, uint32_t b)
 {
 float32 f0 = vfp_itos(a);
 float32 f1 = vfp_itos(b);
@@ -1817,24 +1816,24 @@ uint32_t HELPER(neon_abd_f32)(uint32_t a, uint32_t b)
 : float32_sub(f1, f0, NFS));
 }
 
-uint32_t HELPER(neon_add_f32)(uint32_t a, uint32_t b)
+uint32_t HELPER(neon_add_f32)(CPUState *env, uint32_t a, uint32_t b)
 {
 return vfp_stoi(float32_add(vfp_itos(a), vfp_itos(b), NFS));
 }
 
-uint32_t HELPER(neon_sub_f32)(uint32_t a, uint32_t b)
+uint32_t HELPER(neon_sub_f32)(CPUState *env, uint32_t a, uint32_t b)
 {
 return vfp_stoi(float32_sub(vfp_itos(a), vfp_itos(b), NFS));
 }
 
-uint32_t HELPER(neon_mul_f32)(uint32_t a, uint32_t b)
+uint32_t HELPER(neon_mul_f32)(CPUState *env, uint32_t a, uint32_t b)
 {
 return vfp_stoi(float32_mul(vfp_itos(a), vfp_itos(b), NFS));
 }
 
 /* Floating point comparisons produce an integer result.  */
 #define NEON_VOP_FCMP(name, cmp) \
-uint32_t HELPER(neon_##name)(uint32_t a, uint32_t b) \
+uint32_t HELPER(neon_##name)(CPUState *env, uint32_t a, uint32_t b) \
 { \
 if (float32_compare_quiet(vfp_itos(a), vfp_itos(b), NFS) cmp 0) \
 return ~0; \
@@ -1846,14 +1845,14 @@ NEON_VOP_FCMP(ceq_f32, ==)
 NEON_VOP_FCMP(cge_f32, >=)
 NEON_VOP_FCMP(cgt_f32, >)
 
-uint32_t HELPER(neon_acge_f32)(uint32_t a, uint32_t b)
+uint32_t HELPER(neon_acge_f32)(CPUState *env, uint32_t a, uint32_t b)
 {
 float32 f0 = float32_abs(vfp_itos(a));
 float32 f1 = float32_abs(vfp_itos(b));
 return (float32_compare_quiet(f0, f1,NFS) >= 0) ? ~0 : 0;
 }
 
-uint32_t HELPER(neon_acgt_f32)(uint32_t a, uint32_t b)
+uint32_t HELPER(neon_acgt_f32)(CPUState *env, uint32_t a, uint32_t b)
 {
 float32 f0 = float32_abs(vfp_itos(a));
 float32 f1 = float32_abs(vfp_itos(b));
diff --git a/target-arm/translate.c b/target-arm/translate.c
index f69912f..cf2440e 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -4519,56 +4519,56 @@ static int d

[Qemu-devel] [PATCH 05/10] target-arm: Correct ABD's handling of negative zeroes

2011-04-01 Thread Peter Maydell

Implement ABD by taking the absolute value of the difference
of the operands (as the ARM ARM specifies) rather than by
flipping the order of the operands to the subtract based
on the results of a comparison. The latter approch gives
the wrong answers for some edge cases like negative zero.

Signed-off-by: Peter Maydell 
---
 target-arm/neon_helper.c |4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index 8eb4cef..1905545 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -1788,9 +1788,7 @@ uint32_t HELPER(neon_abd_f32)(CPUState *env, uint32_t a, 
uint32_t b)
 {
 float32 f0 = make_float32(a);
 float32 f1 = make_float32(b);
-return float32_val((float32_compare_quiet(f0, f1, NFS) == 1)
-? float32_sub(f0, f1, NFS)
-: float32_sub(f1, f0, NFS));
+return float32_val(float32_abs(float32_sub(f0, f1, NFS)));
 }
 
 uint32_t HELPER(neon_add_f32)(CPUState *env, uint32_t a, uint32_t b)
-- 
1.7.1

[Qemu-devel] Re: [PATCH v7 0/4] piix_pci: optimize irq data path

2011-04-01 Thread Michael S. Tsirkin

On Fri, Apr 01, 2011 at 08:43:20PM +0900, Isaku Yamahata wrote:
> Here is v7 which are rebased to pci branch.
> 
> I tested this patch series as follows. please see the commit message
> for details.
> - 3/4 piix_pci: optimize set irq path
>   Run linux as guest with 4 e1000 emulated devices.
>   And confirmed that each PIRQ[A-D] are able to assert interrupts.
> - 4/4 piix_pci: load path clean up
>   Run linux as guest with one e1000 emulated device.
>   savevm/loadvm guest with runing ping -f in guest.
> 
> To be honest, the patch, "4/4 piix_pci: load path clean up", needs more
> extensive tests. For example live migration and stress test.
> But for now I don't have plan to further testing in near future. Sorry.
> So please feel free to pick or drop the last patch.

Applied, I will hit it with autotest.

> patch description:
> This patch series optimizes irq data path of piix_pci.
> So far piix3 tracks each pirq level and checks whether a given pic pins is
> asserted by seeing if each pirq is mapped into the pic pin.
> This is independent on irq routing, but data path is on slow path.
> 
> Given that irq routing is rarely changed and asserting pic pins is on
> data path, the path that asserts pic pins should be optimized and
> chainging irq routing should be on slow path.
> The new behavior with this patch series is to use bitmap which is addressed
> by pirq and pic pins with a given irq routing.
> When pirq is asserted, the bitmap is set and see if the pic pins is
> asserted by checking the bitmaps.
> When irq routing is changed, rebuild the bitmap and re-assert pic pins.
> 
> Changes v6 -> v7:
> - rebased to pci branch
> - added how to test to the commit message
> 
> Changes v5 -> v6:
> - fixed piix3_set_irq_pic()
> 
> Changes v4 -> v5:
> - typo
> 
> Changes v3 -> v4:
> - use pirq, pci_intx instead of irq_num in piix_pci.c
> - use symbolic constant PIC_NUM_PINS
> - introduced new patch 4/4 which cleans up a bit.
> 
> Changes v2 -> v3:
> - s/dummy_for_save_load_compat/pci_irq_levels_vmstate/g
> - move down unused member of pci_irq_levels_vmstate in the structure
>   for cache efficiency
> 
> Changes v1 -> v2:
> - addressed review comments.
> 
> Isaku Yamahata (4):
>   pci: add accessor function to get irq levels
>   piix_pci: eliminate PIIX3State::pci_irq_levels
>   piix_pci: optimize set irq path
>   piix_pci: load path clean up
> 
>  hw/pci.c  |7 +++
>  hw/pci.h  |1 +
>  hw/piix_pci.c |  129 
> ++---
>  3 files changed, 112 insertions(+), 25 deletions(-)

Re: [Qemu-devel] Re: [PATCH v2 3/3] raw-posix: Re-open host CD-ROM after media change

2011-04-01 Thread Stefan Hajnoczi

On Thu, Mar 31, 2011 at 11:05 AM, Kevin Wolf  wrote:
> The other thing is that I'm not sure if everything in raw-posix is
> prepared to deal with a -1 fd. At the very least, I think we'll get
> -EBADF errors instead of the expected -ENOMEDIUM.

Not all of block.c checks for !bdrv_is_inserted() so you are right, I
missed cases where we should return -ENOMEDIUM.

We have a similar scenario with floppy disks where fd_open() can
return leaving s->fd == -1.  I would like to unify these floppy and
CD-ROM open cases.

Stefan

Re: [Qemu-devel] Re: [PATCH v2] qemu-img: Initial progress printing support

2011-04-01 Thread Stefan Hajnoczi

On Fri, Apr 1, 2011 at 2:41 PM, Jes Sorensen  wrote:
> On 03/31/11 13:49, Stefan Hajnoczi wrote:
>> On Thu, Mar 31, 2011 at 12:38 PM, Kevin Wolf  wrote:
>>> Am 31.03.2011 13:15, schrieb Jes Sorensen:
 On 03/31/11 12:38, Kevin Wolf wrote:
 I have been a little reluctant to do this because it will break the ABI
 for tools running qemu-img from a GUI etc.
>>>
>>> That's the reason for the "from a terminal" part. If we check for
>>> isatty(), we should handle these cases just fine.
>>
>> Yes, I think checking for a tty is enough precaution and allows users
>> to get the benefit of the progress bar.  TBH I'd probably forget to
>> add -p half the time :).
>
> Ok, this is fine with me - however how do you suggest we offer the
> option to disable it on the command line, an additional flag?

That would be safest, good idea.

Stefan

[Qemu-devel] [PATCH 03/10] target-arm: Return right result for Neon comparison with NaNs

2011-04-01 Thread Peter Maydell

Fix the helper functions implementing the Neon floating point comparison
ops (VCGE, VCGT, VCEQ, VACGT, VACGE) to return the right answer when
one of the values being compared is a NaN.

Signed-off-by: Peter Maydell 
---
 target-arm/neon_helper.c |   28 ++--
 1 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index 4039036..8eb4cef 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -1809,32 +1809,40 @@ uint32_t HELPER(neon_mul_f32)(CPUState *env, uint32_t 
a, uint32_t b)
 }
 
 /* Floating point comparisons produce an integer result.  */
-#define NEON_VOP_FCMP(name, cmp) \
+#define NEON_VOP_FCMP(name, ok) \
 uint32_t HELPER(neon_##name)(CPUState *env, uint32_t a, uint32_t b) \
 { \
-if (float32_compare_quiet(make_float32(a), make_float32(b), NFS) cmp 0) { \
-return ~0; \
-} else { \
-return 0; \
+switch (float32_compare_quiet(make_float32(a), make_float32(b), NFS)) { \
+ok return ~0; \
+default: return 0; \
 } \
 }
 
-NEON_VOP_FCMP(ceq_f32, ==)
-NEON_VOP_FCMP(cge_f32, >=)
-NEON_VOP_FCMP(cgt_f32, >)
+NEON_VOP_FCMP(ceq_f32, case float_relation_equal:)
+NEON_VOP_FCMP(cge_f32, case float_relation_equal: case float_relation_greater:)
+NEON_VOP_FCMP(cgt_f32, case float_relation_greater:)
 
 uint32_t HELPER(neon_acge_f32)(CPUState *env, uint32_t a, uint32_t b)
 {
 float32 f0 = float32_abs(make_float32(a));
 float32 f1 = float32_abs(make_float32(b));
-return (float32_compare_quiet(f0, f1,NFS) >= 0) ? ~0 : 0;
+switch (float32_compare_quiet(f0, f1, NFS)) {
+case float_relation_equal:
+case float_relation_greater:
+return ~0;
+default:
+return 0;
+}
 }
 
 uint32_t HELPER(neon_acgt_f32)(CPUState *env, uint32_t a, uint32_t b)
 {
 float32 f0 = float32_abs(make_float32(a));
 float32 f1 = float32_abs(make_float32(b));
-return (float32_compare_quiet(f0, f1, NFS) > 0) ? ~0 : 0;
+if (float32_compare_quiet(f0, f1, NFS) == float_relation_greater) {
+return ~0;
+}
+return 0;
 }
 
 #define ELEM(V, N, SIZE) (((V) >> ((N) * (SIZE))) & ((1ull << (SIZE)) - 1))
-- 
1.7.1

[Qemu-devel] [PATCH 08/10] target-arm: Fix VLD of single element to all lanes

2011-04-01 Thread Peter Maydell

Fix several bugs in VLD of single element to all lanes:

The "single element to all lanes" form of VLD1 differs from those for
VLD2, VLD3 and VLD4 in that bit 5 indicates whether the loaded element
should be written to one or two Dregs (rather than being a register
stride). Handle this by special-casing VLD1 rather than trying to
have one loop which deals with both VLD1 and 2/3/4.

Handle VLD4.32 with 16 byte alignment specified, rather than UNDEFfing.

UNDEF for the invalid size and alignment combinations.

Signed-off-by: Peter Maydell 
---
 target-arm/translate.c |   84 +--
 1 files changed, 59 insertions(+), 25 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 6ce8b7a..e79ea03 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -2648,6 +2648,28 @@ static void gen_neon_dup_high16(TCGv var)
 tcg_temp_free_i32(tmp);
 }
 
+static TCGv gen_load_and_replicate(DisasContext *s, TCGv addr, int size)
+{
+/* Load a single Neon element and replicate into a 32 bit TCG reg */
+TCGv tmp;
+switch (size) {
+case 0:
+tmp = gen_ld8u(addr, IS_USER(s));
+gen_neon_dup_u8(tmp, 0);
+break;
+case 1:
+tmp = gen_ld16u(addr, IS_USER(s));
+gen_neon_dup_low16(tmp);
+break;
+case 2:
+tmp = gen_ld32(addr, IS_USER(s));
+break;
+default: /* Avoid compiler warnings.  */
+abort();
+}
+return tmp;
+}
+
 /* Disassemble a VFP instruction.  Returns nonzero if an error occured
(ie. an undefined instruction).  */
 static int disas_vfp_insn(CPUState * env, DisasContext *s, uint32_t insn)
@@ -3890,36 +3912,48 @@ static int disas_neon_ls_insn(CPUState * env, 
DisasContext *s, uint32_t insn)
 size = (insn >> 10) & 3;
 if (size == 3) {
 /* Load single element to all lanes.  */
-if (!load)
+int a = (insn >> 4) & 1;
+if (!load) {
 return 1;
+}
 size = (insn >> 6) & 3;
 nregs = ((insn >> 8) & 3) + 1;
-stride = (insn & (1 << 5)) ? 2 : 1;
-load_reg_var(s, addr, rn);
-for (reg = 0; reg < nregs; reg++) {
-switch (size) {
-case 0:
-tmp = gen_ld8u(addr, IS_USER(s));
-gen_neon_dup_u8(tmp, 0);
-break;
-case 1:
-tmp = gen_ld16u(addr, IS_USER(s));
-gen_neon_dup_low16(tmp);
-break;
-case 2:
-tmp = gen_ld32(addr, IS_USER(s));
-break;
-case 3:
+
+if (size == 3) {
+if (nregs != 4 || a == 0) {
 return 1;
-default: /* Avoid compiler warnings.  */
-abort();
 }
-tcg_gen_addi_i32(addr, addr, 1 << size);
-tmp2 = tcg_temp_new_i32();
-tcg_gen_mov_i32(tmp2, tmp);
-neon_store_reg(rd, 0, tmp2);
-neon_store_reg(rd, 1, tmp);
-rd += stride;
+/* For VLD4 size==3 a == 1 means 32 bits at 16 byte alignment 
*/
+size = 2;
+}
+if (nregs == 1 && a == 1 && size == 0) {
+return 1;
+}
+if (nregs == 3 && a == 1) {
+return 1;
+}
+load_reg_var(s, addr, rn);
+if (nregs == 1) {
+/* VLD1 to all lanes: bit 5 indicates how many Dregs to write 
*/
+tmp = gen_load_and_replicate(s, addr, size);
+tcg_gen_st_i32(tmp, cpu_env, neon_reg_offset(rd, 0));
+tcg_gen_st_i32(tmp, cpu_env, neon_reg_offset(rd, 1));
+if (insn & (1 << 5)) {
+tcg_gen_st_i32(tmp, cpu_env, neon_reg_offset(rd + 1, 0));
+tcg_gen_st_i32(tmp, cpu_env, neon_reg_offset(rd + 1, 1));
+}
+tcg_temp_free_i32(tmp);
+} else {
+/* VLD2/3/4 to all lanes: bit 5 indicates register stride */
+stride = (insn & (1 << 5)) ? 2 : 1;
+for (reg = 0; reg < nregs; reg++) {
+tmp = gen_load_and_replicate(s, addr, size);
+tcg_gen_st_i32(tmp, cpu_env, neon_reg_offset(rd, 0));
+tcg_gen_st_i32(tmp, cpu_env, neon_reg_offset(rd, 1));
+tcg_temp_free_i32(tmp);
+tcg_gen_addi_i32(addr, addr, 1 << size);
+rd += stride;
+}
 }
 stride = (1 << size) * nregs;
 } else {
-- 
1.7.1

1 2 >

1 - 100 of 119 matches

Mail list logo