[Qemu-devel] [PATCH] readconfig: add emulation of /dev/fd/ to platforms that that lacks this API

2012-03-02 Thread Ronnie Sahlberg

Resending the patch with the strncmp() bug fixed.

Its been a long long week when you mess up something as simple as this.


regards
ronnie sahlberg




[Qemu-devel] [PATCH] READCONFIG: allow /dev/fd/ the file to use for --readconfig

2012-03-02 Thread Ronnie Sahlberg
On many platforms /dev/fd/ is used to refer to open filedescriptor  of 
the running process.
If we fail to open the provided filename and if the filename starts with 
'/dev/fd/' then assume this is a platform which do not support these special 
files.
In that case read out the descriptor value from the provided string and fdopen 
it.

This allows to use /dev/fd/ syntax on all platforms, those that do provide 
this indeface in /dev  and an emulation of this for the platforms that do not 
provide /dev/fd

Signed-off-by: Ronnie Sahlberg 
---
 qemu-config.c   |   16 +++-
 qemu-options.hx |3 ++-
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/qemu-config.c b/qemu-config.c
index 7d9da78..96e7497 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -814,9 +814,23 @@ out:
 
 int qemu_read_config_file(const char *filename)
 {
-FILE *f = fopen(filename, "r");
+FILE *f;
 int ret;
 
+f = fopen(filename, "r");
+
+if (f == NULL && !strncmp(filename, "/dev/fd/", 8)) {
+int fd;
+char *ptr = NULL;
+
+errno = 0;
+fd = strtol(filename + 8, &ptr, 10);
+if (errno != 0 || ptr == filename + 8 || *ptr != '\0') {
+return -EINVAL;
+}
+f = fdopen(fd, "r");
+}
+
 if (f == NULL) {
 return -errno;
 }
diff --git a/qemu-options.hx b/qemu-options.hx
index b129996..c70c6e7 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2650,7 +2650,8 @@ DEF("readconfig", HAS_ARG, QEMU_OPTION_readconfig,
 STEXI
 @item -readconfig @var{file}
 @findex -readconfig
-Read device configuration from @var{file}.
+Read device configuration from @var{file}. Use '/dev/fd/' as filename
+to read from an existing, inherited, filedesriptor.
 ETEXI
 DEF("writeconfig", HAS_ARG, QEMU_OPTION_writeconfig,
 "-writeconfig \n"
-- 
1.7.3.1




[Qemu-devel] [PATCH] READCONFIG: allow /dev/fd/ the file to use for --readconfig

2012-03-02 Thread Ronnie Sahlberg
On many platforms /dev/fd/ ise used to refer to open filedescriptor  of 
the running process.
If we fail to open the provided filename and if the filename starts with 
'/dev/fd/' then assume this is a platform which do not support these special 
files.
In that case read out the descriptor value from the provided string and fdopen 
it.

This allows to use /dev/fd/ syntax on all platforms, those that do provide 
this indeface in /dev  and an emulation of this for the platforms that do not 
provide /dev/fd

Signed-off-by: Ronnie Sahlberg 
---
 qemu-config.c   |   16 +++-
 qemu-options.hx |3 ++-
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/qemu-config.c b/qemu-config.c
index 7d9da78..7abc08d 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -814,9 +814,23 @@ out:
 
 int qemu_read_config_file(const char *filename)
 {
-FILE *f = fopen(filename, "r");
+FILE *f;
 int ret;
 
+f = fopen(filename, "r");
+
+if (f == NULL && strncmp(filename, "/dev/fd/", 8)) {
+int fd;
+char *ptr = NULL;
+
+errno = 0;
+fd = strtol(filename + 8, &ptr, 10);
+if (errno != 0 || ptr == filename + 8 || *ptr != '\0') {
+return -EINVAL;
+}
+f = fdopen(fd, "r");
+}
+
 if (f == NULL) {
 return -errno;
 }
diff --git a/qemu-options.hx b/qemu-options.hx
index b129996..c70c6e7 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2650,7 +2650,8 @@ DEF("readconfig", HAS_ARG, QEMU_OPTION_readconfig,
 STEXI
 @item -readconfig @var{file}
 @findex -readconfig
-Read device configuration from @var{file}.
+Read device configuration from @var{file}. Use '/dev/fd/' as filename
+to read from an existing, inherited, filedesriptor.
 ETEXI
 DEF("writeconfig", HAS_ARG, QEMU_OPTION_writeconfig,
 "-writeconfig \n"
-- 
1.7.3.1




[Qemu-devel] [PATCH] readconfig: add emulation of /dev/fd/ to platforms that that lacks this API

2012-03-02 Thread Ronnie Sahlberg

Please find a patch to -readconfig.

On many platforms -readconfig /dev/fd/ can be used to read from an already 
opened and inherited filedescriptor .

On platforms that do not natively provide /dev/fd/ 
add emulation of this by checking if the ofiginal fopen(path) failed, then IF
the path starts with /dev/fd/ then try to read the descriptorvalue and fdopen() 
it.


It means that we can use -readconfig /dev/fd/ on all platforms. On those 
that provide a /dev/fd we just open that file. On those that do not we emulate 
it.


regards
ronnie sahlberg




Re: [Qemu-devel] QEMU desired libiscsi.so clashes with libiscsi.so from iscsi-initiator-utils

2012-03-02 Thread ronnie sahlberg
Yes,

Very unfortuante since libiscsi is such a nice name for a
multiplatform library what even works on win32 :-(

I have so renamed it to libiscsiclient  and sent a patch to qemu to
this list to use -liscsiclient instead of -liscsi



tarballs  can be found at
https://github.com/sahlberg/libiscsi/downloads


That resolves all issues you are concerned about ?


regards
ronnie sahlberg

On Tue, Feb 14, 2012 at 12:24 AM, Daniel P. Berrange
 wrote:
> I was investigating how to build latest QEMU with the iSCSI block driver
> enabled. I saw that configure wanted a libiscsi.so, so I installed that
> library from Fedora RPMs via the iscsi-initiator-utils package, but it
> still wouldn't build.
>
> After further investigation, I find that QEMU in fact wants a completely
> different, unlreated libiscsi.so library:
>
>  https://github.com/sahlberg/libiscsi
>
> Obviously we have a problem here because we can't have two different
> libraries called libiscsi.so installed at the same time.
>
> Since iscsi-initiator-utils is a standard Linux distro package whose usage
> of libiscsi.so predates this github project, it seems that to resolve this
> it will be neccessary to rename the latter. eg perhaps libiscsi-client.so ?
>
> The followup question is where to find actual libiscsi releases to package
> up for OS distros ? It is not very desirable to just package GIT snapshots.
>
> Regards,
> Daniel
> --
> |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-             http://virt-manager.org :|
> |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|



[Qemu-devel] [PATCH] Change library from libiscsi to libiscsiclient to match library rename in libiscsi

2012-03-02 Thread Ronnie Sahlberg

Signed-off-by: Ronnie Sahlberg 
---
 configure |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index fb0e18e..294c0c1 100755
--- a/configure
+++ b/configure
@@ -2503,9 +2503,9 @@ if test "$libiscsi" != "no" ; then
 #include 
 int main(void) { iscsi_create_context(""); return 0; }
 EOF
-  if compile_prog "-Werror" "-liscsi" ; then
+  if compile_prog "-Werror" "-liscsiclient" ; then
 libiscsi="yes"
-LIBS="$LIBS -liscsi"
+LIBS="$LIBS -liscsiclient"
   else
 if test "$libiscsi" = "yes" ; then
   feature_not_found "libiscsi"
-- 
1.7.3.1




[Qemu-devel] [PATCH] Rename libiscsi to libiscsiclient

2012-03-02 Thread Ronnie Sahlberg

Please find a patch that changes the name of the iscsi library from libiscsi to 
libiscsiclient

While libiscsi is a very nice name that works on all platforms, including 
win32, it clashes with a linux library. 



regards
ronnie sahlberg




[Qemu-devel] Problems on running vxworks on KVM with x86_64!

2012-03-02 Thread GaoYi
Hi Bill,

   I am trying to run vxworks on KVM with X86_64 machine. I've downloaded
your binary Vxworks Image from

http://lists.gnu.org/archive/html/qemu-devel/2009-06/msg00401.html
and ran it by: *qemu -fda vxworks.img*. Unfortunately, I could not bootup
the vxworks. The vncviewer shows that:

  Boot from floppy disk
  v1.6 
and quick restarts again and again.
   So what may be the problem? I would be much appreciated if you could
describe your KVM version, configuration or something related.
   Thanks,

Yi


Re: [Qemu-devel] IRQ number, interrupt number, interrupt line & GPIO[in/out]

2012-03-02 Thread Zhi Yong Wu
On Sat, Mar 3, 2012 at 12:41 AM, Peter Maydell  wrote:
> On 2 March 2012 16:01, Anthony Liguori  wrote:
>> On 03/02/2012 06:38 AM, Zhi Yong Wu wrote:
>>> Can anyone explain their relationship and difference among them?  It
>>> is very appreciated if you can make some comments. thanks.
>>
>>
>> IRQ == interrupt.
>>
>> GPIO is just another name for an input or output pin on a chip which could
>> be a IRQ line.
>
> The other point to note here is that there are two different sets
> of terminology:
> (1) real world terminology, which is roughly what Anthony is describing
> (2) QEMU qdev and sysbus terms, which don't necessarily always map
> quite cleanly to (1)
Yeah, i also found that they don't cleanly map to real world
teminology. so many interrupt concepts make me get confused.

By the way, i have one question about MSI[-X].
How does Root Complex know if one write transaction is used for
MSI[-X]. When it recieves this transaction request, how it convert it
to one interrupt which CPU can identify?

>
> In particular the QEMU type 'qemu_irq' is really just an arbitrary signal
> (and would be better named GPIO) -- it is not always used for an
> actual interrupt line.
Can you elaborate this?
>
> -- PMM



-- 
Regards,

Zhi Yong Wu



Re: [Qemu-devel] IRQ number, interrupt number, interrupt line & GPIO[in/out]

2012-03-02 Thread Zhi Yong Wu
On Fri, Mar 2, 2012 at 11:12 PM, ���f任  wrote:
>> Can anyone explain their relationship and difference among them?  It
>> is very appreciated if you can make some comments. thanks.
>
>  I think IRQ number, interrupt number are quite similar things. You can
> check PIC [1] first, especially 8259A [2]. When a device raise an interrupt,
> the interrupt is delivered to CPU through PIC. Each device attaches itself
> to one of PIC's pins. Thus, when we say the IRQ number of device X is Y,
> it means device X attaches itself to PIC's pin Y. PIC will deliver the highest
> priority interrupt to the CPU. The term "interrupt line" might appear in PCI
> context [3]. BIOS usually uses interrupt line to represent what PIC pin the
> device attatches to. Note that PIC (Programmable Interrupt Controller) and
> PCI (Peripheral Component Interconnect) are different things.
>
>  GPIO mostly is used on SoC. It depends on the vendor how to use GPIO. One
> of GPIO capabilities is similar to PIC, I guess. But I leave this to SoC
> experts. I strongly recommend the book [5] if you want to learn how things
> work.
thanks a lot for your help, they are very in theory.:). actually these
concepts all exist in QEMU. I would like to know how they work
together.
>
> HTH,
> chenwj
>
> [1] http://en.wikipedia.org/wiki/Programmable_Interrupt_Controller
> [2] http://en.wikipedia.org/wiki/Intel_8259
> [3] http://en.wikipedia.org/wiki/Conventional_PCI
> [4] http://en.wikipedia.org/wiki/General_Purpose_Input/Output
> [5] 系统虚拟化:原理与实现
>
> --
> Wei-Ren Chen (���f任)
> Computer Systems Lab, Institute of Information Science,
> Academia Sinica, Taiwan (R.O.C.)
> Tel:886-2-2788-3799 #1667
> Homepage: http://people.cs.nctu.edu.tw/~chenwj



-- 
Regards,

Zhi Yong Wu



Re: [Qemu-devel] IRQ number, interrupt number, interrupt line & GPIO[in/out]

2012-03-02 Thread Zhi Yong Wu
HI, anthony.

On Sat, Mar 3, 2012 at 12:01 AM, Anthony Liguori  wrote:
> Hi Zhi Yong,
>
>
> On 03/02/2012 06:38 AM, Zhi Yong Wu wrote:
>>
>> HI,
>>
>> Can anyone explain their relationship and difference among them?  It
>> is very appreciated if you can make some comments. thanks.
>
>
> IRQ == interrupt.
>
> GPIO is just another name for an input or output pin on a chip which could
> be a IRQ line.
>
> Interrupt controllers can receive interrupts from one or more devices.
>  Usually, the input pins on an interrupt controller can be numbered
> sequentially.  When we say that the first UART is on IRQ number 3, what that
> really means is that the IRQ output pin on the UART chip is connected to pin
> number 3 on the interrupt controller with a wire.
>
> But there never is a single interrupt controller in a real system.  For
> instance, a PCI bus has it's own interrupt controller that has four input
> pins (called LNKs) that are oddly labeled A, B, C, D.
thanks, i got what they mean.
>
> For the I440FX PCI bus, those four input pins are mapped to two IRQs which
> are then connected to the I/O APIC.
What do two IRQs mean here? how will those four input pins be mapped
to two IRQs? For example, 8259 interrupt controller is connected to
I/O APIC.
How will these two IRQs be connected to I/O APIC? i think that those
four input pins are connected to I/O APIC with wire, right?
Can you elaborate them if you are available?
>
> Regards,
>
> Anthony Liguori



-- 
Regards,

Zhi Yong Wu



Re: [Qemu-devel] [PATCH] libcacard: Fix compilation with gcc-4.7

2012-03-02 Thread Brad Smith

On 02/03/12 10:49 AM, Hans de Goede wrote:

VCARD_ATR_PREFIX is used as part of an array initializer so it should
not have () around it, so far this happened to work, but gcc-4.7 does
not like it.


This recent commit..

libcacard: fix reported ATR length

Broke the build on my OpenBSD (gcc 4.2.1) buildbot which was due to the
change to this macro.

vcard_emul_nss.c:528: warning: left-hand operand of comma expression has 
no effect
vcard_emul_nss.c:528: warning: left-hand operand of comma expression has 
no effect
vcard_emul_nss.c:528: warning: left-hand operand of comma expression has 
no effect
vcard_emul_nss.c:528: warning: left-hand operand of comma expression has 
no effect
vcard_emul_nss.c:528: warning: left-hand operand of comma expression has 
no effect
vcard_emul_nss.c:528: warning: left-hand operand of comma expression has 
no effect
vcard_emul_nss.c:528: warning: left-hand operand of comma expression has 
no effect
vcard_emul_nss.c:528: warning: left-hand operand of comma expression has 
no effect
vcard_emul_nss.c:528: warning: left-hand operand of comma expression has 
no effect

vcard_emul_nss.c:528: error: initializer element is not constant
vcard_emul_nss.c:528: error: (near initialization for 'nss_atr[0]')
gmake[1]: *** [vcard_emul_nss.o] Error 1


Signed-off-by: Hans de Goede
---
  libcacard/vcardt.h |4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libcacard/vcardt.h b/libcacard/vcardt.h
index d4d8e2e..d3e9522 100644
--- a/libcacard/vcardt.h
+++ b/libcacard/vcardt.h
@@ -26,8 +26,8 @@ typedef struct VCardEmulStruct VCardEmul;
  #define MAX_CHANNEL 4

  /* create an ATR with appropriate historical bytes */
-#define VCARD_ATR_PREFIX(size) (0x3b, 0x68+(size), 0x00, 0xff, \
-   'V', 'C', 'A', 'R', 'D', '_')
+#define VCARD_ATR_PREFIX(size) 0x3b, 0x68+(size), 0x00, 0xff, \
+   'V', 'C', 'A', 'R', 'D', '_'


  typedef enum {


--
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.




Re: [Qemu-devel] ARM brk bug

2012-03-02 Thread Alexander Graf

On 02.03.2012, at 18:49, Peter Maydell wrote:

> On 27 February 2012 15:16, Bernhard M. Wiedemann  wrote:
>> I found that running a debian arm5 bash with qemu runs into varying
>> problems with -R but works without.
> 
> So I had a look at this this afternoon, and what seems to be happening
> is that with -R, the call to target_mmap() in elfload.c:setup_arg_pages()
> (which creates the stack) is putting the stack immediately after the
> bash BSS segment in the address space. This means that brk() will
> never be able to expand, and it looks like something in either bash
> or libc's locale code isn't correctly handling the failure, so we
> crash. (The segfault is from a strlen(NULL) from setlocale() I think.)
> 
> We should probably try to put the stack somewhere more sensible than
> where it currently ends up...

Yikes - I just realized that git on armv7 also breaks due to this:

  
https://build.opensuse.org/package/live_build_log?arch=armv7l&package=git&project=openSUSE%3AFactory%3AARM&repository=standard

So how could we go with this? Give target_mmap a hint that we want to map at 
some randomly defined address rather than let it to its automatic thing?


Alex




[Qemu-devel] buildbot failure in qemu on block_openbsd_current

2012-03-02 Thread qemu
The Buildbot has detected a new failure on builder block_openbsd_current while 
building qemu.
Full details are available at:
 http://buildbot.b1-systems.de/qemu/builders/block_openbsd_current/builds/154

Buildbot URL: http://buildbot.b1-systems.de/qemu/

Buildslave for this Build: brad_openbsd_current

Build Reason: The Nightly scheduler named 'nightly_block' triggered this build
Build Source Stamp: [branch block] HEAD
Blamelist: 

BUILD FAILED: failed compile

sincerely,
 -The Buildbot



[Qemu-devel] buildbot failure in qemu on block_mingw32

2012-03-02 Thread qemu
The Buildbot has detected a new failure on builder block_mingw32 while building 
qemu.
Full details are available at:
 http://buildbot.b1-systems.de/qemu/builders/block_mingw32/builds/144

Buildbot URL: http://buildbot.b1-systems.de/qemu/

Buildslave for this Build: kraxel_rhel61

Build Reason: The Nightly scheduler named 'nightly_block' triggered this build
Build Source Stamp: [branch block] HEAD
Blamelist: 

BUILD FAILED: failed compile

sincerely,
 -The Buildbot



[Qemu-devel] buildbot failure in qemu on default_openbsd_4.9

2012-03-02 Thread qemu
The Buildbot has detected a new failure on builder default_openbsd_4.9 while 
building qemu.
Full details are available at:
 http://buildbot.b1-systems.de/qemu/builders/default_openbsd_4.9/builds/195

Buildbot URL: http://buildbot.b1-systems.de/qemu/

Buildslave for this Build: kraxel_openbsd49

Build Reason: The Nightly scheduler named 'nightly_default' triggered this build
Build Source Stamp: [branch master] HEAD
Blamelist: 

BUILD FAILED: failed compile

sincerely,
 -The Buildbot



[Qemu-devel] [PATCH 1/6] w64: Fix size of ram_addr_t

2012-03-02 Thread Stefan Weil
ram_addr_t must be large enough to address any address of the host.

For hosts with sizeof(unsigned long) == sizeof(void *), this patch
changes nothing. All currently supported hosts fall into this category.

For w64 hosts, sizeof(unsigned long) is 4 while sizeof(void *) is 8,
so the use of uintptr_t is needed.

Signed-off-by: Stefan Weil 
---
 cpu-common.h |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/cpu-common.h b/cpu-common.h
index a40c57d..dca5175 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -28,9 +28,9 @@ typedef uint64_t ram_addr_t;
 #  define RAM_ADDR_MAX UINT64_MAX
 #  define RAM_ADDR_FMT "%" PRIx64
 #else
-typedef unsigned long ram_addr_t;
-#  define RAM_ADDR_MAX ULONG_MAX
-#  define RAM_ADDR_FMT "%lx"
+typedef uintptr_t ram_addr_t;
+#  define RAM_ADDR_MAX UINTPTR_MAX
+#  define RAM_ADDR_FMT "%" PRIxPTR
 #endif
 
 /* memory API */
-- 
1.7.9




Re: [Qemu-devel] [PATCH 5/6] cache-utils: Change data type of parameters for flush_icache_range

2012-03-02 Thread Alexander Graf

On 02.03.2012, at 23:30, Stefan Weil wrote:

> The TCG targets i386 and tci needed a change of the function
> prototype for w64.
> 
> This change is currently not needed here, but it can be applied
> to avoid code differences.

Both 4 and 5 look reasonable to me.

Alex




[Qemu-devel] [PATCH 0/6] w64: Improve compilation with MinGW-w64

2012-03-02 Thread Stefan Weil
These patches are a step towards full 64 bit support for w64.
The patches 4 and 5 are optional.

Please apply this series.

Thanks,
Stefan Weil

[PATCH 1/6] w64: Fix size of ram_addr_t
[PATCH 2/6] tcg: Rearrange definitions and include statements
[PATCH 3/6] w64: Fix data type of parameters for flush_icache_range
[PATCH 4/6] w64: Change data type of parameters for
[PATCH 5/6] cache-utils: Change data type of parameters for
[PATCH 6/6] w64: fix type casts when calling flush_icache_range



[Qemu-devel] [PATCH 6/6] w64: fix type casts when calling flush_icache_range

2012-03-02 Thread Stefan Weil
Signed-off-by: Stefan Weil 
---
 tcg/tcg.c |9 +
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 351a0a3..cd2db3c 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -253,8 +253,8 @@ void tcg_prologue_init(TCGContext *s)
 s->code_buf = code_gen_prologue;
 s->code_ptr = s->code_buf;
 tcg_target_qemu_prologue(s);
-flush_icache_range((unsigned long)s->code_buf, 
-   (unsigned long)s->code_ptr);
+flush_icache_range((tcg_target_ulong)s->code_buf,
+   (tcg_target_ulong)s->code_ptr);
 }
 
 void tcg_set_frame(TCGContext *s, int reg,
@@ -2176,8 +2176,9 @@ int tcg_gen_code(TCGContext *s, uint8_t *gen_code_buf)
 tcg_gen_code_common(s, gen_code_buf, -1);
 
 /* flush instruction cache */
-flush_icache_range((unsigned long)gen_code_buf, 
-   (unsigned long)s->code_ptr);
+flush_icache_range((tcg_target_ulong)gen_code_buf,
+   (tcg_target_ulong)s->code_ptr);
+
 return s->code_ptr -  gen_code_buf;
 }
 
-- 
1.7.9




[Qemu-devel] [PATCH 3/6] w64: Fix data type of parameters for flush_icache_range

2012-03-02 Thread Stefan Weil
flush_icache_range takes two address parameters which must be large
enough to address any address of the host.

For hosts with sizeof(unsigned long) == sizeof(void *), this patch
changes nothing. All currently supported hosts fall into this category.

For w64 hosts, sizeof(unsigned long) is 4 while sizeof(void *) is 8,
so the use of tcg_target_ulong is needed for i386 and tci (the tcg
targets which work with w64).

Signed-off-by: Stefan Weil 
---
 tcg/i386/tcg-target.h |3 ++-
 tcg/tci/tcg-target.h  |3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h
index adbb036..c3cfe05 100644
--- a/tcg/i386/tcg-target.h
+++ b/tcg/i386/tcg-target.h
@@ -123,6 +123,7 @@ typedef enum {
 # define TCG_AREG0 TCG_REG_EBP
 #endif
 
-static inline void flush_icache_range(unsigned long start, unsigned long stop)
+static inline void flush_icache_range(tcg_target_ulong start,
+  tcg_target_ulong stop)
 {
 }
diff --git a/tcg/tci/tcg-target.h b/tcg/tci/tcg-target.h
index 03e0fd1..81fcc0f 100644
--- a/tcg/tci/tcg-target.h
+++ b/tcg/tci/tcg-target.h
@@ -157,7 +157,8 @@ void tci_disas(uint8_t opc);
 unsigned long tcg_qemu_tb_exec(CPUState *env, uint8_t *tb_ptr);
 #define tcg_qemu_tb_exec tcg_qemu_tb_exec
 
-static inline void flush_icache_range(unsigned long start, unsigned long stop)
+static inline void flush_icache_range(tcg_target_ulong start,
+  tcg_target_ulong stop)
 {
 }
 
-- 
1.7.9




[Qemu-devel] [PATCH 4/6] w64: Change data type of parameters for flush_icache_range

2012-03-02 Thread Stefan Weil
The TCG targets i386 and tci needed a change of the function
prototype for w64.

This change is currently not needed for the other TCG targets,
but it can be applied to avoid code differences.

Cc: Blue Swirl 
Cc: Andrzej Zaborowski 
Cc: Richard Henderson 
Cc: Aurelien Jarno 
Cc: Alexander Graf 
Signed-off-by: Stefan Weil 
---
 tcg/arm/tcg-target.h   |3 ++-
 tcg/hppa/tcg-target.h  |4 +++-
 tcg/ia64/tcg-target.h  |3 ++-
 tcg/mips/tcg-target.h  |3 ++-
 tcg/s390/tcg-target.h  |3 ++-
 tcg/sparc/tcg-target.h |3 ++-
 6 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/tcg/arm/tcg-target.h b/tcg/arm/tcg-target.h
index 0035b47..f90b834 100644
--- a/tcg/arm/tcg-target.h
+++ b/tcg/arm/tcg-target.h
@@ -81,7 +81,8 @@ enum {
 TCG_AREG0 = TCG_REG_R6,
 };
 
-static inline void flush_icache_range(unsigned long start, unsigned long stop)
+static inline void flush_icache_range(tcg_target_ulong start,
+  tcg_target_ulong stop)
 {
 #if QEMU_GNUC_PREREQ(4, 1)
 __builtin___clear_cache((char *) start, (char *) stop);
diff --git a/tcg/hppa/tcg-target.h b/tcg/hppa/tcg-target.h
index 7f3c4cc..d4bf6fe 100644
--- a/tcg/hppa/tcg-target.h
+++ b/tcg/hppa/tcg-target.h
@@ -107,7 +107,9 @@ typedef enum {
 /* Note: must be synced with dyngen-exec.h */
 #define TCG_AREG0 TCG_REG_R17
 
-static inline void flush_icache_range(unsigned long start, unsigned long stop)
+
+static inline void flush_icache_range(tcg_target_ulong start,
+  tcg_target_ulong stop)
 {
 start &= ~31;
 while (start <= stop) {
diff --git a/tcg/ia64/tcg-target.h b/tcg/ia64/tcg-target.h
index c388089..0631b9f 100644
--- a/tcg/ia64/tcg-target.h
+++ b/tcg/ia64/tcg-target.h
@@ -146,7 +146,8 @@ typedef enum {
 /* Guest base is supported */
 #define TCG_TARGET_HAS_GUEST_BASE
 
-static inline void flush_icache_range(unsigned long start, unsigned long stop)
+static inline void flush_icache_range(tcg_target_ulong start,
+  tcg_target_ulong stop)
 {
 start = start & ~(32UL - 1UL);
 stop = (stop + (32UL - 1UL)) & ~(32UL - 1UL);
diff --git a/tcg/mips/tcg-target.h b/tcg/mips/tcg-target.h
index 477bc38..d3c804d 100644
--- a/tcg/mips/tcg-target.h
+++ b/tcg/mips/tcg-target.h
@@ -108,7 +108,8 @@ typedef enum {
 #include 
 #endif
 
-static inline void flush_icache_range(unsigned long start, unsigned long stop)
+static inline void flush_icache_range(tcg_target_ulong start,
+  tcg_target_ulong stop)
 {
 cacheflush ((void *)start, stop-start, ICACHE);
 }
diff --git a/tcg/s390/tcg-target.h b/tcg/s390/tcg-target.h
index e4cd641..d12f90b 100644
--- a/tcg/s390/tcg-target.h
+++ b/tcg/s390/tcg-target.h
@@ -100,6 +100,7 @@ enum {
 TCG_AREG0 = TCG_REG_R10,
 };
 
-static inline void flush_icache_range(unsigned long start, unsigned long stop)
+static inline void flush_icache_range(tcg_target_ulong start,
+  tcg_target_ulong stop)
 {
 }
diff --git a/tcg/sparc/tcg-target.h b/tcg/sparc/tcg-target.h
index c3fe131..ee2274d 100644
--- a/tcg/sparc/tcg-target.h
+++ b/tcg/sparc/tcg-target.h
@@ -134,7 +134,8 @@ typedef enum {
 #define TCG_AREG0 TCG_REG_G6
 #endif
 
-static inline void flush_icache_range(unsigned long start, unsigned long stop)
+static inline void flush_icache_range(tcg_target_ulong start,
+  tcg_target_ulong stop)
 {
 unsigned long p;
 
-- 
1.7.9




[Qemu-devel] [PATCH 5/6] cache-utils: Change data type of parameters for flush_icache_range

2012-03-02 Thread Stefan Weil
The TCG targets i386 and tci needed a change of the function
prototype for w64.

This change is currently not needed here, but it can be applied
to avoid code differences.

Cc: Alexander Graf 
Signed-off-by: Stefan Weil 
---
 cache-utils.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/cache-utils.h b/cache-utils.h
index 0b65907..04a6e2e 100644
--- a/cache-utils.h
+++ b/cache-utils.h
@@ -12,7 +12,7 @@ extern struct qemu_cache_conf qemu_cache_conf;
 void qemu_cache_utils_init(char **envp);
 
 /* mildly adjusted code from tcg-dyngen.c */
-static inline void flush_icache_range(unsigned long start, unsigned long stop)
+static inline void flush_icache_range(uintptr_t start, uintptr_t stop)
 {
 unsigned long p, start1, stop1;
 unsigned long dsize = qemu_cache_conf.dcache_bsize;
-- 
1.7.9




[Qemu-devel] [PATCH 2/6] tcg: Rearrange definitions and include statements

2012-03-02 Thread Stefan Weil
This change makes tcg_target_ulong available in tcg-target.h.

Signed-off-by: Stefan Weil 
---
 tcg/tcg.h |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tcg/tcg.h b/tcg/tcg.h
index 5c28239..cc223ea 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -32,9 +32,6 @@
 # error Unknown pointer size for tcg target
 #endif
 
-#include "tcg-target.h"
-#include "tcg-runtime.h"
-
 #if TCG_TARGET_REG_BITS == 32
 typedef int32_t tcg_target_long;
 typedef uint32_t tcg_target_ulong;
@@ -49,6 +46,9 @@ typedef uint64_t tcg_target_ulong;
 #error unsupported
 #endif
 
+#include "tcg-target.h"
+#include "tcg-runtime.h"
+
 #if TCG_TARGET_NB_REGS <= 32
 typedef uint32_t TCGRegSet;
 #elif TCG_TARGET_NB_REGS <= 64
-- 
1.7.9




Re: [Qemu-devel] [PATCH 03/10] qtest: add C version of test infrastructure

2012-03-02 Thread Eduardo Habkost
On Sat, Feb 25, 2012 at 01:42:42PM -0600, Anthony Liguori wrote:
> This also includes a qtest wrapper script to make it easier to launch qtest
> tests directly.
> 
> Signed-off-by: Anthony Liguori 
> ---
>  scripts/qtest|5 +
>  tests/Makefile   |2 +
>  tests/libqtest.c |  334 
> ++
>  tests/libqtest.h |   63 ++
>  4 files changed, 404 insertions(+), 0 deletions(-)
>  create mode 100644 scripts/qtest
>  create mode 100644 tests/libqtest.c
>  create mode 100644 tests/libqtest.h
> 
> diff --git a/scripts/qtest b/scripts/qtest
> new file mode 100644
> index 000..5cff3d4
> --- /dev/null
> +++ b/scripts/qtest

The script is missing the executable bit. When trying to run 'make check' after
applying this series, I get:

Running 'tests/rtc-test' with qemu-system-x86_64...
/bin/sh: line 3: /home/ehabkost/pessoal/proj/virt/qemu/scripts/qtest: 
Permission denied
make: *** [check-qtest-x86_64] Error 126

-- 
Eduardo



Re: [Qemu-devel] [RFC] fix crashes with vmware driver

2012-03-02 Thread Ryan Harper
* Serge Hallyn  [2012-03-02 15:13]:
> Hi,
> 
> I don't know where the best place to catch this would be, but
> with vnc and vmware_vga it's possible to get set_bit called on
> a negative index, crashing qemu.  See
> 
> https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/918791
> 
> for details.  This patch prevents that.  It's possible this
> should be caught earlier, but this patch works for me.
> 
> Signed-off-by: Serge Hallyn 
> ---
>  vmware_vga.c |   16 
>  1 file changed, 16 insertions(+)
> 
> Index: qemu-kvm-1.0+noroms/hw/vmware_vga.c
> ===
> --- qemu-kvm-1.0+noroms.orig/hw/vmware_vga.c  2012-03-01 16:19:23.280571798 
> -0600
> +++ qemu-kvm-1.0+noroms/hw/vmware_vga.c   2012-03-01 16:27:27.910975006 
> -0600
> @@ -298,6 +298,22 @@
>  uint8_t *src;
>  uint8_t *dst;
> 
> +if (x < 0) {
> +fprintf(stderr, "%s: update x was < 0 (%d, w %d)\n",
> +__FUNCTION__, x, w);
> +w += x;
> + if (w < 0)
> + return;
> +x = 0;
> +}
> +if (y < 0) {
> +fprintf(stderr, "%s: update y was < 0 (%d, h %d)\n",
> +__FUNCTION__, y, h);
> +h += y;
> + if (h < 0)
> + return;
> +y = 0;
> +}

Looks like it has mixed spaces and tabs.

CODING_STYLE wants {} on all if's 



>  if (x + w > s->width) {
>  fprintf(stderr, "%s: update width too large x: %d, w: %d\n",
>  __FUNCTION__, x, w);

-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ry...@us.ibm.com




[Qemu-devel] [PATCH 08/13] usb-ehci: Fix cerr tracking

2012-03-02 Thread Hans de Goede
cerr should only be decremented on errors which cause XactErr to be set, and
when that happens the failing transaction should be retried until cerr reaches
0 and only then should USBSTS_ERRINT be set (and inactive cleared and
USBSTS_INT set if requested).

Since we don't have any hardware level errors (and in case of redirection
the real hardware has already retried), re-trying makes no sense, so
immediately set cerr to 0 on errors which set XactErr.

Signed-off-by: Hans de Goede 
---
 hw/usb-ehci.c |   19 ++-
 1 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 507e4a8..2685adc 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -1291,7 +1291,7 @@ static void ehci_async_complete_packet(USBPort *port, 
USBPacket *packet)
 
 static void ehci_execute_complete(EHCIQueue *q)
 {
-int c_err, reload;
+int reload;
 
 assert(q->async != EHCI_ASYNC_INFLIGHT);
 q->async = EHCI_ASYNC_NONE;
@@ -1300,15 +1300,10 @@ static void ehci_execute_complete(EHCIQueue *q)
 q->qhaddr, q->qh.next, q->qtdaddr, q->usb_status);
 
 if (q->usb_status < 0) {
-err:
-/* TO-DO: put this is in a function that can be invoked below as well 
*/
-c_err = get_field(q->qh.token, QTD_TOKEN_CERR);
-c_err--;
-set_field(&q->qh.token, c_err, QTD_TOKEN_CERR);
-
 switch(q->usb_status) {
 case USB_RET_NODEV:
 q->qh.token |= (QTD_TOKEN_HALT | QTD_TOKEN_XACTERR);
+set_field(&q->qh.token, 0, QTD_TOKEN_CERR);
 ehci_record_interrupt(q->ehci, USBSTS_ERRINT);
 break;
 case USB_RET_STALL:
@@ -1336,15 +1331,13 @@ err:
 assert(0);
 break;
 }
+} else if ((q->usb_status > q->tbytes) && (q->pid == USB_TOKEN_IN)) {
+q->usb_status = USB_RET_BABBLE;
+q->qh.token |= (QTD_TOKEN_HALT | QTD_TOKEN_BABBLE);
+ehci_record_interrupt(q->ehci, USBSTS_ERRINT);
 } else {
-// DPRINTF("Short packet condition\n");
 // TODO check 4.12 for splits
 
-if ((q->usb_status > q->tbytes) && (q->pid == USB_TOKEN_IN)) {
-q->usb_status = USB_RET_BABBLE;
-goto err;
-}
-
 if (q->tbytes && q->pid == USB_TOKEN_IN) {
 q->tbytes -= q->usb_status;
 } else {
-- 
1.7.7.6




[Qemu-devel] [RFC] fix crashes with vmware driver

2012-03-02 Thread Serge Hallyn
Hi,

I don't know where the best place to catch this would be, but
with vnc and vmware_vga it's possible to get set_bit called on
a negative index, crashing qemu.  See

https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/918791

for details.  This patch prevents that.  It's possible this
should be caught earlier, but this patch works for me.

Signed-off-by: Serge Hallyn 
---
 vmware_vga.c |   16 
 1 file changed, 16 insertions(+)

Index: qemu-kvm-1.0+noroms/hw/vmware_vga.c
===
--- qemu-kvm-1.0+noroms.orig/hw/vmware_vga.c2012-03-01 16:19:23.280571798 
-0600
+++ qemu-kvm-1.0+noroms/hw/vmware_vga.c 2012-03-01 16:27:27.910975006 -0600
@@ -298,6 +298,22 @@
 uint8_t *src;
 uint8_t *dst;
 
+if (x < 0) {
+fprintf(stderr, "%s: update x was < 0 (%d, w %d)\n",
+__FUNCTION__, x, w);
+w += x;
+   if (w < 0)
+   return;
+x = 0;
+}
+if (y < 0) {
+fprintf(stderr, "%s: update y was < 0 (%d, h %d)\n",
+__FUNCTION__, y, h);
+h += y;
+   if (h < 0)
+   return;
+y = 0;
+}
 if (x + w > s->width) {
 fprintf(stderr, "%s: update width too large x: %d, w: %d\n",
 __FUNCTION__, x, w);



[Qemu-devel] [PATCH 09/13] usb-ehci: Remove dead nakcnt code

2012-03-02 Thread Hans de Goede
This patch removes 2 bits of dead nakcnt code:

1) usb_ehci_execute calls ehci_qh_do_overlay which does:
nakcnt = reload;
and then has a block of code which is conditional on:
if (reload && !nakcnt) {
which ofcourse is never true now as nakcnt == reload.

2) ehci_state_fetchqh does:
nakcnt = reload;
but before nakcnt is ever used ehci_state_fetchqh is always followed
by a ehci_qh_do_overlay call which also does:
nakcnt = reload;
So doing this from ehci_state_fetchqh is redundant.

Signed-off-by: Hans de Goede 
---
 hw/usb-ehci.c |   20 
 1 files changed, 0 insertions(+), 20 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 2685adc..07bcd1f 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -1615,7 +1615,6 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int 
async)
 {
 uint32_t entry;
 EHCIQueue *q;
-int reload;
 
 entry = ehci_get_fetch_addr(ehci, async);
 q = ehci_find_queue_by_qh(ehci, entry, async);
@@ -1673,11 +1672,6 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, 
int async)
 }
 #endif
 
-reload = get_field(q->qh.epchar, QH_EPCHAR_RL);
-if (reload) {
-set_field(&q->qh.altnext_qtd, reload, QH_ALTNEXT_NAKCNT);
-}
-
 if (q->qh.token & QTD_TOKEN_HALT) {
 ehci_set_state(ehci, async, EST_HORIZONTALQH);
 
@@ -1837,25 +1831,11 @@ static void ehci_flush_qh(EHCIQueue *q)
 static int ehci_state_execute(EHCIQueue *q, int async)
 {
 int again = 0;
-int reload, nakcnt;
-int smask;
 
 if (ehci_qh_do_overlay(q) != 0) {
 return -1;
 }
 
-smask = get_field(q->qh.epcap, QH_EPCAP_SMASK);
-
-if (!smask) {
-reload = get_field(q->qh.epchar, QH_EPCHAR_RL);
-nakcnt = get_field(q->qh.altnext_qtd, QH_ALTNEXT_NAKCNT);
-if (reload && !nakcnt) {
-ehci_set_state(q->ehci, async, EST_HORIZONTALQH);
-again = 1;
-goto out;
-}
-}
-
 // TODO verify enough time remains in the uframe as in 4.4.1.1
 // TODO write back ptr to async list when done or out of time
 // TODO Windows does not seem to ever set the MULT field
-- 
1.7.7.6




[Qemu-devel] [PATCH 07/13] usb-ehci: Any packet completion except for NAK should set the interrupt

2012-03-02 Thread Hans de Goede
As clearly stated in the 2.3.2 of the EHCI spec, any time USBERRINT get
sets then if the td has its IOC bit set USBINT should be set as well.

This means that for any status except for USB_RET_NAK we should set
USBINT if the IOC bit is set.

Signed-off-by: Hans de Goede 
---
 hw/usb-ehci.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index d386b84..507e4a8 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -1360,7 +1360,7 @@ err:
 q->qh.token ^= QTD_TOKEN_DTOGGLE;
 q->qh.token &= ~QTD_TOKEN_ACTIVE;
 
-if ((q->usb_status >= 0) && (q->qh.token & QTD_TOKEN_IOC)) {
+if ((q->usb_status != USB_RET_NAK) && (q->qh.token & QTD_TOKEN_IOC)) {
 ehci_record_interrupt(q->ehci, USBSTS_INT);
 }
 }
-- 
1.7.7.6




[Qemu-devel] [PATCH 12/13] usb: return BABBLE rather then NAK when we receive too much data

2012-03-02 Thread Hans de Goede
Signed-off-by: Hans de Goede 
---
 usb-linux.c |8 +++-
 usb-redir.c |4 ++--
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/usb-linux.c b/usb-linux.c
index 47994f3..38df9e6 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -364,6 +364,10 @@ static void async_complete(void *opaque)
 p->result = USB_RET_STALL;
 break;
 
+case -EOVERFLOW:
+p->result = USB_RET_BABBLE;
+break;
+
 default:
 p->result = USB_RET_NAK;
 break;
@@ -722,6 +726,8 @@ static int urb_status_to_usb_ret(int status)
 switch (status) {
 case -EPIPE:
 return USB_RET_STALL;
+case -EOVERFLOW:
+return USB_RET_BABBLE;
 default:
 return USB_RET_NAK;
 }
@@ -759,7 +765,7 @@ static int usb_host_handle_iso_data(USBHostDevice *s, 
USBPacket *p, int in)
 } else if (aurb[i].urb.iso_frame_desc[j].actual_length
> p->iov.size) {
 printf("husb: received iso data is larger then packet\n");
-len = USB_RET_NAK;
+len = USB_RET_BABBLE;
 /* All good copy data over */
 } else {
 len = aurb[i].urb.iso_frame_desc[j].actual_length;
diff --git a/usb-redir.c b/usb-redir.c
index ef9fa96..4c8e3e3 100644
--- a/usb-redir.c
+++ b/usb-redir.c
@@ -457,7 +457,7 @@ static int usbredir_handle_iso_data(USBRedirDevice *dev, 
USBPacket *p,
 ERROR("received iso data is larger then packet ep %02X (%d > 
%d)\n",
   ep, len, (int)p->iov.size);
 bufp_free(dev, isop, ep);
-return USB_RET_NAK;
+return USB_RET_BABBLE;
 }
 usb_packet_copy(p, isop->data, len);
 bufp_free(dev, isop, ep);
@@ -576,7 +576,7 @@ static int usbredir_handle_interrupt_data(USBRedirDevice 
*dev,
 if (len > p->iov.size) {
 ERROR("received int data is larger then packet ep %02X\n", ep);
 bufp_free(dev, intp, ep);
-return USB_RET_NAK;
+return USB_RET_BABBLE;
 }
 usb_packet_copy(p, intp->data, len);
 bufp_free(dev, intp, ep);
-- 
1.7.7.6




Re: [Qemu-devel] [PATCH v2 0/4] slirp: Fix for requeuing crash, cleanups

2012-03-02 Thread Stefan Weil

Am 02.03.2012 19:57, schrieb Jan Kiszka:

Well, this requeuing bug seems to have a long breath. Previous attempts
to fix it (mine included) neglected the fact that we need to walk the
queue of pending packets, not just restart from the beginning after a
requeue. This version should get it Right(TM).

This also comes with a fix for resource cleanups on slirp shutdown. At
least valgrind is happy now.

Changes in v2:
- fixed corner case of session list walk that Stefan Weil reported

CC: Fabien Chouteau 
CC: Michael S. Tsirkin 
CC: Stefan Weil 
CC: Zhi Yong Wu 

Jan Kiszka (4):
slirp: Keep next_m always valid
slirp: Fix queue walking in if_start
slirp: Remove unneeded if_queued
slirp: Cleanup resources on instance removal

slirp/if.c | 64 +
slirp/ip_icmp.c | 7 ++
slirp/ip_icmp.h | 1 +
slirp/ip_input.c | 7 ++
slirp/mbuf.c | 21 +
slirp/mbuf.h | 1 +
slirp/slirp.c | 10 +++-
slirp/slirp.h | 3 +-
slirp/tcp_subr.c | 7 ++
slirp/udp.c | 8 ++
slirp/udp.h | 1 +
11 files changed, 94 insertions(+), 36 deletions(-)


Hi Jan,

this is what I get with your new patch series.

Regards,
Stefan


Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffe9bf0700 (LWP 5863)]
0x557781bf in slirp_remque (a=0x569916b0) at 
/home/stefan/src/qemu/repo.or.cz/qemu/ar7/slirp/misc.c:39
39((struct quehead *)(element->qh_rlink))->qh_link = 
element->qh_link;

(gdb) i s
#0  0x557781bf in slirp_remque (a=0x569916b0) at 
/home/stefan/src/qemu/repo.or.cz/qemu/ar7/slirp/misc.c:39
#1  0x55777b00 in m_get (slirp=0x562bdb80) at 
/home/stefan/src/qemu/repo.or.cz/qemu/ar7/slirp/mbuf.c:81
#2  0x5577abdf in slirp_input (slirp=0x562bdb80, 
pkt=0x56305d58 "RU\n", pkt_len=54) at 
/home/stefan/src/qemu/repo.or.cz/qemu/ar7/slirp/slirp.c:673
#3  0x55730f8b in net_slirp_receive (nc=0x562bd950, 
buf=0x56305d58 "RU\n", size=54) at 
/home/stefan/src/qemu/repo.or.cz/qemu/ar7/net/slirp.c:116
#4  0x5572dc11 in qemu_vlan_deliver_packet 
(sender=0x563074c0, flags=0, buf=0x56305d58 "RU\n", size=54, 
opaque=0x562bd8b0)

at /home/stefan/src/qemu/repo.or.cz/qemu/ar7/net.c:451
#5  0x55730938 in qemu_net_queue_deliver (queue=0x562bd8f0, 
sender=0x563074c0, flags=0, data=0x56305d58 "RU\n", size=54)

at /home/stefan/src/qemu/repo.or.cz/qemu/ar7/net/queue.c:154
#6  0x55730a78 in qemu_net_queue_send (queue=0x562bd8f0, 
sender=0x563074c0, flags=0, data=0x56305d58 "RU\n", size=54, 
sent_cb=0)

at /home/stefan/src/qemu/repo.or.cz/qemu/ar7/net/queue.c:188
#7  0x5572de30 in qemu_send_packet_async_with_flags 
(sender=0x563074c0, flags=0, buf=0x56305d58 "RU\n", size=54, 
sent_cb=0)

at /home/stefan/src/qemu/repo.or.cz/qemu/ar7/net.c:519
#8  0x5572de8b in qemu_send_packet_async (sender=0x563074c0, 
buf=0x56305d58 "RU\n", size=54, sent_cb=0) at 
/home/stefan/src/qemu/repo.or.cz/qemu/ar7/net.c:526
#9  0x5572dedb in qemu_send_packet (vc=0x563074c0, 
buf=0x56305d58 "RU\n", size=54) at 
/home/stefan/src/qemu/repo.or.cz/qemu/ar7/net.c:532
#10 0x556e9daa in pcnet_transmit (s=0x56305af8) at 
/home/stefan/src/qemu/repo.or.cz/qemu/ar7/hw/pcnet.c:1258
#11 0x556ea0fd in pcnet_poll_timer (opaque=0x56305af8) at 
/home/stefan/src/qemu/repo.or.cz/qemu/ar7/hw/pcnet.c:1321
#12 0x556ea8e9 in pcnet_ioport_writew (opaque=0x56305af8, 
addr=18, val=0) at /home/stefan/src/qemu/repo.or.cz/qemu/ar7/hw/pcnet.c:1571
#13 0x556e62b3 in pcnet_ioport_write (opaque=0x56305af8, 
addr=18, data=0, size=2) at 
/home/stefan/src/qemu/repo.or.cz/qemu/ar7/hw/pcnet-pci.c:120
#14 0x55801c8b in memory_region_write_accessor 
(opaque=0x56306d80, addr=18, value=0x7fffe9bef690, size=2, shift=0, 
mask=65535)

at /home/stefan/src/qemu/repo.or.cz/qemu/ar7/memory.c:329
#15 0x55801d6d in access_with_adjusted_size (addr=18, 
value=0x7fffe9bef690, size=2, access_size_min=1, access_size_max=4,
access=0x55801c13 , 
opaque=0x56306d80) at 
/home/stefan/src/qemu/repo.or.cz/qemu/ar7/memory.c:359
#16 0x5580217d in memory_region_iorange_write 
(iorange=0x56306dc0, offset=18, width=2, data=0) at 
/home/stefan/src/qemu/repo.or.cz/qemu/ar7/memory.c:428
#17 0x557fb41c in ioport_writew_thunk (opaque=0x56306dc0, 
addr=4146, data=0) at /home/stefan/src/qemu/repo.or.cz/qemu/ar7/ioport.c:218
#18 0x557facb5 in ioport_write (index=1, address=4146, data=0) 
at /home/stefan/src/qemu/repo.or.cz/qemu/ar7/ioport.c:82
#19 0x557fb8a3 in cpu_outw (addr=4146, val=0) at 
/home/stefan/src/qemu/repo.or.cz/qemu/ar7/ioport.c:281
#20 0x556c7ae4 in isa_mmio_writew (opaque=0x0, addr=4146, val=0) 
at /home/stefan/src/qemu/repo.or.cz/qemu/ar7/hw/isa_mmio.c:38
#21 0x5580477f in memory_region_dispatch_write 
(mr=0x562ffc38, addr=41

[Qemu-devel] [PATCH 05/13] usb-ehci: Drop cached qhs when the doorbell gets rung

2012-03-02 Thread Hans de Goede
The purpose of the IAAD bit / the doorbell is to make the ehci controller
forget about cached qhs, this is mainly used when cancelling transactions,
the qh is unlinked from the async schedule and then the doorbell gets rung,
once the doorbell is acked by the controller the hcd knows that the qh is
no longer in use and that it can do something else with the memory, such
as re-use it for a new qh! But we keep our struct representing this qh around
for circa 250 ms. This allows for a (mightily large) race window where the
following could happen:
-hcd submits a qh at address 0xdeadbeef
-our ehci code sees the qh, sends a request to a usb-device, gets a result
 of USB_RET_ASYNC, sets the async_state of the qh to EHCI_ASYNC_INFLIGHT
-hcd unlinks the qh at address 0xdeadbeef
-hcd rings the doorbell, wait for us to ack it
-hcd re-uses the qh at address 0xdeadbeef
-our ehci code sees the qh, looks in the async_queue, sees there already is
 a qh at address 0xdeadbeef there with async_state of EHCI_ASYNC_INFLIGHT,
 does nothing
-the *original* (which the hcd thinks it has cancelled) transaction finishes
-our ehci code sees the qh on yet another pass through the async list,
 looks in the async_queue, sees there already is a qh at address 0xdeadbeef
 there with async_state of EHCI_ASYNC_COMPLETED, and finished the transaction
 with the results of the *original* transaction.

Not good (tm), this patch fixes this race by removing all qhs which have not
been seen during the last cycle through the async list immidiately when the
doorbell is rung.

Note this patch does not fix any actually observed problem, but upon
reading of the EHCI spec it became apparent to me that the above race could
happen and the usb-ehci behavior from before this patch is not good.

Signed-off-by: Hans de Goede 
---
 hw/usb-ehci.c |   33 +
 1 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index d384fcc..b349003 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -697,7 +697,7 @@ static EHCIQueue *ehci_find_queue_by_qh(EHCIState *ehci, 
uint32_t addr,
 return NULL;
 }
 
-static void ehci_queues_rip_unused(EHCIState *ehci, int async)
+static void ehci_queues_rip_unused(EHCIState *ehci, int async, int flush)
 {
 EHCIQueueHead *head = async ? &ehci->aqueues : &ehci->pqueues;
 EHCIQueue *q, *tmp;
@@ -708,7 +708,7 @@ static void ehci_queues_rip_unused(EHCIState *ehci, int 
async)
 q->ts = ehci->last_run_ns;
 continue;
 }
-if (ehci->last_run_ns < q->ts + 25000) {
+if (!flush && ehci->last_run_ns < q->ts + 25000) {
 /* allow 0.25 sec idle */
 continue;
 }
@@ -1537,7 +1537,7 @@ static int ehci_state_waitlisthead(EHCIState *ehci,  int 
async)
 ehci_set_usbsts(ehci, USBSTS_REC);
 }
 
-ehci_queues_rip_unused(ehci, async);
+ehci_queues_rip_unused(ehci, async, 0);
 
 /*  Find the head of the list (4.9.1.1) */
 for(i = 0; i < MAX_QH; i++) {
@@ -2093,18 +2093,7 @@ static void ehci_advance_async_state(EHCIState *ehci)
 break;
 }
 
-/* If the doorbell is set, the guest wants to make a change to the
- * schedule. The host controller needs to release cached data.
- * (section 4.8.2)
- */
-if (ehci->usbcmd & USBCMD_IAAD) {
-DPRINTF("ASYNC: doorbell request acknowledged\n");
-ehci->usbcmd &= ~USBCMD_IAAD;
-ehci_set_interrupt(ehci, USBSTS_IAA);
-break;
-}
-
-/* make sure guest has acknowledged */
+/* make sure guest has acknowledged the doorbell interrupt */
 /* TO-DO: is this really needed? */
 if (ehci->usbsts & USBSTS_IAA) {
 DPRINTF("IAA status bit still set.\n");
@@ -2118,6 +2107,18 @@ static void ehci_advance_async_state(EHCIState *ehci)
 
 ehci_set_state(ehci, async, EST_WAITLISTHEAD);
 ehci_advance_state(ehci, async);
+
+/* If the doorbell is set, the guest wants to make a change to the
+ * schedule. The host controller needs to release cached data.
+ * (section 4.8.2)
+ */
+if (ehci->usbcmd & USBCMD_IAAD) {
+/* Remove all unseen qhs from the async qhs queue */
+ehci_queues_rip_unused(ehci, async, 1);
+DPRINTF("ASYNC: doorbell request acknowledged\n");
+ehci->usbcmd &= ~USBCMD_IAAD;
+ehci_set_interrupt(ehci, USBSTS_IAA);
+}
 break;
 
 default:
@@ -2167,7 +2168,7 @@ static void ehci_advance_periodic_state(EHCIState *ehci)
 ehci_set_fetch_addr(ehci, async,entry);
 ehci_set_state(ehci, async, EST_FETCHENTRY);
 ehci_advance_state(ehci, async);
-ehci_queues_rip_unused(ehci, async);
+ehci_queues_rip_unused(ehci, async, 0);
 break;
 
 default:
-- 
1.7.7.6




[Qemu-devel] [PATCH v1 1/1] mips: properly compute hflags and fcr0 on cpu reset

2012-03-02 Thread Meador Inge
Currently 'cpu_reset' doesn't fully compute all of the needed
HFLAGs and fails to setup fcr0 after clearing the CPU state.
This can cause instruction exceptions.  For example, using
'madd.d' on machines that should support it is kindly greeted
with:

qemu: uncaught target signal 4 (Illegal instruction) - core dumped
Illegal instruction (core dumped)

because fcr0 is bogus and MIPS_HFLAG_COP1X is not correcly set in hflags.

This is fixed by modifying 'cpu_reset' to use 'compute_hflags' and
initializing 'fcr0' from the current CPU model.

Signed-off-by: Maciej W. Rozycki 
Signed-off-by: Nathan Froyd 
Signed-off-by: Meador Inge 
---
 target-mips/cpu.h   |   49 +++
 target-mips/op_helper.c |   49 ---
 target-mips/translate.c |   17 +++
 3 files changed, 53 insertions(+), 62 deletions(-)

diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 71cb4e8..fc65348 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -737,4 +737,53 @@ static inline void cpu_pc_from_tb(CPUState *env, 
TranslationBlock *tb)
 env->hflags |= tb->flags & MIPS_HFLAG_BMASK;
 }
 
+static inline void compute_hflags(CPUState *env)
+{
+env->hflags &= ~(MIPS_HFLAG_COP1X | MIPS_HFLAG_64 | MIPS_HFLAG_CP0 |
+ MIPS_HFLAG_F64 | MIPS_HFLAG_FPU | MIPS_HFLAG_KSU |
+ MIPS_HFLAG_UX);
+if (!(env->CP0_Status & (1 << CP0St_EXL)) &&
+!(env->CP0_Status & (1 << CP0St_ERL)) &&
+!(env->hflags & MIPS_HFLAG_DM)) {
+env->hflags |= (env->CP0_Status >> CP0St_KSU) & MIPS_HFLAG_KSU;
+}
+#if defined(TARGET_MIPS64)
+if (((env->hflags & MIPS_HFLAG_KSU) != MIPS_HFLAG_UM) ||
+(env->CP0_Status & (1 << CP0St_PX)) ||
+(env->CP0_Status & (1 << CP0St_UX))) {
+env->hflags |= MIPS_HFLAG_64;
+}
+if (env->CP0_Status & (1 << CP0St_UX)) {
+env->hflags |= MIPS_HFLAG_UX;
+}
+#endif
+if ((env->CP0_Status & (1 << CP0St_CU0)) ||
+!(env->hflags & MIPS_HFLAG_KSU)) {
+env->hflags |= MIPS_HFLAG_CP0;
+}
+if (env->CP0_Status & (1 << CP0St_CU1)) {
+env->hflags |= MIPS_HFLAG_FPU;
+}
+if (env->CP0_Status & (1 << CP0St_FR)) {
+env->hflags |= MIPS_HFLAG_F64;
+}
+if (env->insn_flags & ISA_MIPS32R2) {
+if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
+env->hflags |= MIPS_HFLAG_COP1X;
+}
+} else if (env->insn_flags & ISA_MIPS32) {
+if (env->hflags & MIPS_HFLAG_64) {
+env->hflags |= MIPS_HFLAG_COP1X;
+}
+} else if (env->insn_flags & ISA_MIPS4) {
+/* All supported MIPS IV CPUs use the XX (CU3) to enable
+   and disable the MIPS IV extensions to the MIPS III ISA.
+   Some other MIPS IV CPUs ignore the bit, so the check here
+   would be too restrictive for them.  */
+if (env->CP0_Status & (1 << CP0St_CU3)) {
+env->hflags |= MIPS_HFLAG_COP1X;
+}
+}
+}
+
 #endif /* !defined (__MIPS_CPU_H__) */
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index c51b9cb..1f1c736 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -32,55 +32,6 @@
 static inline void cpu_mips_tlb_flush (CPUState *env, int flush_global);
 #endif
 
-static inline void compute_hflags(CPUState *env)
-{
-env->hflags &= ~(MIPS_HFLAG_COP1X | MIPS_HFLAG_64 | MIPS_HFLAG_CP0 |
- MIPS_HFLAG_F64 | MIPS_HFLAG_FPU | MIPS_HFLAG_KSU |
- MIPS_HFLAG_UX);
-if (!(env->CP0_Status & (1 << CP0St_EXL)) &&
-!(env->CP0_Status & (1 << CP0St_ERL)) &&
-!(env->hflags & MIPS_HFLAG_DM)) {
-env->hflags |= (env->CP0_Status >> CP0St_KSU) & MIPS_HFLAG_KSU;
-}
-#if defined(TARGET_MIPS64)
-if (((env->hflags & MIPS_HFLAG_KSU) != MIPS_HFLAG_UM) ||
-(env->CP0_Status & (1 << CP0St_PX)) ||
-(env->CP0_Status & (1 << CP0St_UX))) {
-env->hflags |= MIPS_HFLAG_64;
-}
-if (env->CP0_Status & (1 << CP0St_UX)) {
-env->hflags |= MIPS_HFLAG_UX;
-}
-#endif
-if ((env->CP0_Status & (1 << CP0St_CU0)) ||
-!(env->hflags & MIPS_HFLAG_KSU)) {
-env->hflags |= MIPS_HFLAG_CP0;
-}
-if (env->CP0_Status & (1 << CP0St_CU1)) {
-env->hflags |= MIPS_HFLAG_FPU;
-}
-if (env->CP0_Status & (1 << CP0St_FR)) {
-env->hflags |= MIPS_HFLAG_F64;
-}
-if (env->insn_flags & ISA_MIPS32R2) {
-if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
-env->hflags |= MIPS_HFLAG_COP1X;
-}
-} else if (env->insn_flags & ISA_MIPS32) {
-if (env->hflags & MIPS_HFLAG_64) {
-env->hflags |= MIPS_HFLAG_COP1X;
-}
-} else if (env->insn_flags & ISA_MIPS4) {
-/* All supported MIPS IV CPUs use the XX (CU3) to enable
-   and disable the MIPS IV extensions to the MIPS III ISA.
-   Some other MIPS IV CPUs ignore the bit, so the check here
-

[Qemu-devel] usb patches [v2]

2012-03-02 Thread Hans de Goede
Hi,

Here is a series of usb patches against current qemu master, mostly
consisting of a whole bunch of small ehci fixes, plus some redirection
fixes.

Changes in v2:
Due to some reshuffling of patches before sending [PATCH 04/13] "usb-ehci:
always call ehci_queues_rip_unused for period queues" ended up passing in the
flush flag of ehci_queues_rip_unused, which does not get added to it until
the next patch. This version fixes this so that all intermediate steps
compile.

Regards,

Hans



[Qemu-devel] [PATCH 10/13] usb-ehci: Fix and simplify nakcnt handling

2012-03-02 Thread Hans de Goede
The nakcnt code in ehci_execute_complete() marked transactions as finished
when a packet completed with a result of USB_RET_NAK, but USB_RET_NAK
means that the device cannot receive / send data at that time and that
the transaction should be retried later, which is also what the usb-uhci
and usb-ohci code does.

Note that there already was some special code in place to handle this
for interrupt endpoints in the form of doing a return from
ehci_execute_complete() when reload == 0, but that for bulk transactions
this was not handled correctly (where as for example the usb-ccid device does
return USB_RET_NAK for bulk packets).

Besides that the code in ehci_execute_complete() decrement nakcnt by 1
on a packet result of USB_RET_NAK, but
-since the transaction got marked as finished,
 nakcnt would never be decremented again
-there is no code checking for nakcnt becoming 0
-there is no use in re-trying the transaction within the same usb frame /
 usb-ehci frame-timer call, since the status of emulated devices won't change
 as long as the usb-ehci frame-timer is running
So we should simply set the nakcnt to 0 when we get a USB_RET_NAK, thus
claiming that we've tried reload times (or as many times as possible if
reload is 0).

Besides the code in ehci_execute_complete() handling USB_RET_NAK there
was also code handling it in ehci_state_executing(), which calls
ehci_execute_complete(), and then does its own handling on top of the handling
in ehci_execute_complete(), this code would decrement nakcnt *again* (if not
already 0), or restore the reload value (which was never changed) on success.

Since the double decrement was wrong to begin with, and is no longer needed
now that we set nakcnt directly to 0 on USB_RET_NAK, and the restore of reload
is not needed either, this patch simply removes all nakcnt handling from
ehci_state_executing().

Signed-off-by: Hans de Goede 
---
 hw/usb-ehci.c |   32 
 1 files changed, 4 insertions(+), 28 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 07bcd1f..9197298 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -1291,8 +1291,6 @@ static void ehci_async_complete_packet(USBPort *port, 
USBPacket *packet)
 
 static void ehci_execute_complete(EHCIQueue *q)
 {
-int reload;
-
 assert(q->async != EHCI_ASYNC_INFLIGHT);
 q->async = EHCI_ASYNC_NONE;
 
@@ -1311,16 +1309,8 @@ static void ehci_execute_complete(EHCIQueue *q)
 ehci_record_interrupt(q->ehci, USBSTS_ERRINT);
 break;
 case USB_RET_NAK:
-/* 4.10.3 */
-reload = get_field(q->qh.epchar, QH_EPCHAR_RL);
-if ((q->pid == USB_TOKEN_IN) && reload) {
-int nakcnt = get_field(q->qh.altnext_qtd, QH_ALTNEXT_NAKCNT);
-nakcnt--;
-set_field(&q->qh.altnext_qtd, nakcnt, QH_ALTNEXT_NAKCNT);
-} else if (!reload) {
-return;
-}
-break;
+set_field(&q->qh.altnext_qtd, 0, QH_ALTNEXT_NAKCNT);
+return; /* We're not done yet with this transaction */
 case USB_RET_BABBLE:
 q->qh.token |= (QTD_TOKEN_HALT | QTD_TOKEN_BABBLE);
 ehci_record_interrupt(q->ehci, USBSTS_ERRINT);
@@ -1353,7 +1343,7 @@ static void ehci_execute_complete(EHCIQueue *q)
 q->qh.token ^= QTD_TOKEN_DTOGGLE;
 q->qh.token &= ~QTD_TOKEN_ACTIVE;
 
-if ((q->usb_status != USB_RET_NAK) && (q->qh.token & QTD_TOKEN_IOC)) {
+if (q->qh.token & QTD_TOKEN_IOC) {
 ehci_record_interrupt(q->ehci, USBSTS_INT);
 }
 }
@@ -1877,7 +1867,6 @@ out:
 static int ehci_state_executing(EHCIQueue *q, int async)
 {
 int again = 0;
-int reload, nakcnt;
 
 ehci_execute_complete(q);
 if (q->usb_status == USB_RET_ASYNC) {
@@ -1897,21 +1886,8 @@ static int ehci_state_executing(EHCIQueue *q, int async)
 // counter decrements to 0
 }
 
-reload = get_field(q->qh.epchar, QH_EPCHAR_RL);
-if (reload) {
-nakcnt = get_field(q->qh.altnext_qtd, QH_ALTNEXT_NAKCNT);
-if (q->usb_status == USB_RET_NAK) {
-if (nakcnt) {
-nakcnt--;
-}
-} else {
-nakcnt = reload;
-}
-set_field(&q->qh.altnext_qtd, nakcnt, QH_ALTNEXT_NAKCNT);
-}
-
 /* 4.10.5 */
-if ((q->usb_status == USB_RET_NAK) || (q->qh.token & QTD_TOKEN_ACTIVE)) {
+if (q->usb_status == USB_RET_NAK) {
 ehci_set_state(q->ehci, async, EST_HORIZONTALQH);
 } else {
 ehci_set_state(q->ehci, async, EST_WRITEBACK);
-- 
1.7.7.6




[Qemu-devel] [PATCH 01/13] usb-redir: Set ep type and interface

2012-03-02 Thread Hans de Goede
Since we don't use usb_desc.c we need to do this ourselves. This fixes
iso transfers no longer working for USB 2 devices due to the ep->type
check in ehci.c

Signed-off-by: Hans de Goede 
---
 usb-redir.c |8 
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/usb-redir.c b/usb-redir.c
index fc58a0e..ef9fa96 100644
--- a/usb-redir.c
+++ b/usb-redir.c
@@ -1149,6 +1149,7 @@ static void usbredir_device_disconnect(void *priv)
 for (i = 0; i < MAX_ENDPOINTS; i++) {
 QTAILQ_INIT(&dev->endpoint[i].bufpq);
 }
+usb_ep_init(&dev->dev);
 dev->interface_info.interface_count = 0;
 }
 
@@ -1175,6 +1176,7 @@ static void usbredir_ep_info(void *priv,
 struct usb_redir_ep_info_header *ep_info)
 {
 USBRedirDevice *dev = priv;
+struct USBEndpoint *usb_ep;
 int i;
 
 for (i = 0; i < MAX_ENDPOINTS; i++) {
@@ -1199,7 +1201,13 @@ static void usbredir_ep_info(void *priv,
 default:
 ERROR("Received invalid endpoint type\n");
 usbredir_device_disconnect(dev);
+return;
 }
+usb_ep = usb_ep_get(&dev->dev,
+(i & 0x10) ? USB_TOKEN_IN : USB_TOKEN_OUT,
+i & 0x0f);
+usb_ep->type = dev->endpoint[i].type;
+usb_ep->ifnum = dev->endpoint[i].interface;
 }
 }
 
-- 
1.7.7.6




[Qemu-devel] [PATCH 11/13] usb-ehci: Cleanup itd error handling

2012-03-02 Thread Hans de Goede
All error statuses except for NAK are handled in a switch case, move the
handling of NAK into the same switch case.

Signed-off-by: Hans de Goede 
---
 hw/usb-ehci.c |   28 ++--
 1 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 9197298..825fcc0 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -1466,20 +1466,7 @@ static int ehci_process_itd(EHCIState *ehci,
 }
 qemu_sglist_destroy(&ehci->isgl);
 
-if (ret == USB_RET_NAK) {
-/* no data for us, so do a zero-length transfer */
-ret = 0;
-}
-
-if (ret >= 0) {
-if (!dir) {
-/* OUT */
-set_field(&itd->transact[i], len - ret, ITD_XACT_LENGTH);
-} else {
-/* IN */
-set_field(&itd->transact[i], ret, ITD_XACT_LENGTH);
-}
-} else {
+if (ret < 0) {
 switch (ret) {
 default:
 fprintf(stderr, "Unexpected iso usb result: %d\n", ret);
@@ -1495,6 +1482,19 @@ static int ehci_process_itd(EHCIState *ehci,
 itd->transact[i] |= ITD_XACT_BABBLE;
 ehci_record_interrupt(ehci, USBSTS_ERRINT);
 break;
+case USB_RET_NAK:
+/* no data for us, so do a zero-length transfer */
+ret = 0;
+break;
+}
+}
+if (ret >= 0) {
+if (!dir) {
+/* OUT */
+set_field(&itd->transact[i], len - ret, ITD_XACT_LENGTH);
+} else {
+/* IN */
+set_field(&itd->transact[i], ret, ITD_XACT_LENGTH);
 }
 }
 if (itd->transact[i] & ITD_XACT_IOC) {
-- 
1.7.7.6




[Qemu-devel] [PATCH 13/13] usb: add USB_RET_IOERROR

2012-03-02 Thread Hans de Goede
We already have USB_RET_NAK, but that means that a device does not want
to send/receive right now. But with host / network redirection we can
actually have a transaction fail due to some io error, rather then ie
the device just not having any data atm.

This patch adds a new error code named USB_RET_IOERROR for this, and uses
it were appropriate.

Notes:
-Currently all usb-controllers handle this the same as NODEV, but that
 may change in the future, OHCI could indicate a CRC error instead for example.
-This patch does not touch hw/usb-musb.c, that is because the code in there
 handles STALL and NAK specially and has a if status < 0 generic catch all
 for all other errors

Signed-off-by: Hans de Goede 
---
 hw/usb-ehci.c |2 ++
 hw/usb-ohci.c |2 ++
 hw/usb-uhci.c |1 +
 hw/usb.h  |   11 ++-
 usb-linux.c   |4 ++--
 usb-redir.c   |9 ++---
 6 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 825fcc0..df742f7 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -1299,6 +1299,7 @@ static void ehci_execute_complete(EHCIQueue *q)
 
 if (q->usb_status < 0) {
 switch(q->usb_status) {
+case USB_RET_IOERROR:
 case USB_RET_NODEV:
 q->qh.token |= (QTD_TOKEN_HALT | QTD_TOKEN_XACTERR);
 set_field(&q->qh.token, 0, QTD_TOKEN_CERR);
@@ -1471,6 +1472,7 @@ static int ehci_process_itd(EHCIState *ehci,
 default:
 fprintf(stderr, "Unexpected iso usb result: %d\n", ret);
 /* Fall through */
+case USB_RET_IOERROR:
 case USB_RET_NODEV:
 /* 3.3.2: XACTERR is only allowed on IN transactions */
 if (dir) {
diff --git a/hw/usb-ohci.c b/hw/usb-ohci.c
index 7aa19fe..20aaa74 100644
--- a/hw/usb-ohci.c
+++ b/hw/usb-ohci.c
@@ -837,6 +837,7 @@ static int ohci_service_iso_td(OHCIState *ohci, struct 
ohci_ed *ed,
 OHCI_CC_DATAUNDERRUN);
 } else {
 switch (ret) {
+case USB_RET_IOERROR:
 case USB_RET_NODEV:
 OHCI_SET_BM(iso_td.offset[relative_frame_number], TD_PSW_CC,
 OHCI_CC_DEVICENOTRESPONDING);
@@ -1052,6 +1053,7 @@ static int ohci_service_td(OHCIState *ohci, struct 
ohci_ed *ed)
 OHCI_SET_BM(td.flags, TD_CC, OHCI_CC_DATAUNDERRUN);
 } else {
 switch (ret) {
+case USB_RET_IOERROR:
 case USB_RET_NODEV:
 OHCI_SET_BM(td.flags, TD_CC, OHCI_CC_DEVICENOTRESPONDING);
 case USB_RET_NAK:
diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
index 70e3881..2c6ed38 100644
--- a/hw/usb-uhci.c
+++ b/hw/usb-uhci.c
@@ -765,6 +765,7 @@ out:
 break;
return 1;
 
+case USB_RET_IOERROR:
 case USB_RET_NODEV:
 default:
break;
diff --git a/hw/usb.h b/hw/usb.h
index 8e83697..1a30ebb 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -39,11 +39,12 @@
 #define USB_TOKEN_IN0x69 /* device -> host */
 #define USB_TOKEN_OUT   0xe1 /* host -> device */
 
-#define USB_RET_NODEV  (-1)
-#define USB_RET_NAK(-2)
-#define USB_RET_STALL  (-3)
-#define USB_RET_BABBLE (-4)
-#define USB_RET_ASYNC  (-5)
+#define USB_RET_NODEV   (-1)
+#define USB_RET_NAK (-2)
+#define USB_RET_STALL   (-3)
+#define USB_RET_BABBLE  (-4)
+#define USB_RET_IOERROR (-5)
+#define USB_RET_ASYNC   (-6)
 
 #define USB_SPEED_LOW   0
 #define USB_SPEED_FULL  1
diff --git a/usb-linux.c b/usb-linux.c
index 38df9e6..050ea7a 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -369,7 +369,7 @@ static void async_complete(void *opaque)
 break;
 
 default:
-p->result = USB_RET_NAK;
+p->result = USB_RET_IOERROR;
 break;
 }
 
@@ -729,7 +729,7 @@ static int urb_status_to_usb_ret(int status)
 case -EOVERFLOW:
 return USB_RET_BABBLE;
 default:
-return USB_RET_NAK;
+return USB_RET_IOERROR;
 }
 }
 
diff --git a/usb-redir.c b/usb-redir.c
index 4c8e3e3..59e9327 100644
--- a/usb-redir.c
+++ b/usb-redir.c
@@ -441,7 +441,7 @@ static int usbredir_handle_iso_data(USBRedirDevice *dev, 
USBPacket *p,
 /* Check iso_error for stream errors, otherwise its an underrun */
 status = dev->endpoint[EP2I(ep)].iso_error;
 dev->endpoint[EP2I(ep)].iso_error = 0;
-return status ? USB_RET_NAK : 0;
+return status ? USB_RET_IOERROR : 0;
 }
 DPRINTF2("iso-token-in ep %02X status %d len %d queue-size: %d\n", ep,
  isop->status, isop->len, dev->endpoint[EP2I(ep)].bufpq_size);
@@ -449,7 +449,7 @@ static int usbredir_handle_iso_data(USBRedirDevice *dev, 
USBPacket *p,
 status = isop->status;
 if (status != usb_redir_success) {
 bufp_free(dev, isop, ep);
-return USB_RET_NAK;
+return USB_RET_IOERROR;
 }
 
 len

[Qemu-devel] [PATCH 04/13] usb-ehci: always call ehci_queues_rip_unused for period queues

2012-03-02 Thread Hans de Goede
Before this patch USB 2 devices with interrupt endpoints were not working
properly. The problem is that to avoid loops we stop processing as soon
as we encounter a queue-head (qh) we've already seen since qhs can be linked
in a circular fashion, this is tracked by the seen flag in our qh struct.

The resetting of the seen flag is done from ehci_queues_rip_unused which
before this patch was only called when executing the statemachine for the
async schedule.

But packets for interrupt endpoints are part of the periodic schedule! So what
would happen is that when there were no ctrl or bulk packets for a USB 2
device with an interrupt endpoint, the async schedule would become non
active, then ehci_queues_rip_unused would no longer get called and when
processing the qhs for the interrupt endpoints from the periodic schedule
their seen bit would still be 1 and they would be skipped.

Signed-off-by: Hans de Goede 
---
 hw/usb-ehci.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 840022d..d384fcc 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -2167,6 +2167,7 @@ static void ehci_advance_periodic_state(EHCIState *ehci)
 ehci_set_fetch_addr(ehci, async,entry);
 ehci_set_state(ehci, async, EST_FETCHENTRY);
 ehci_advance_state(ehci, async);
+ehci_queues_rip_unused(ehci, async);
 break;
 
 default:
-- 
1.7.7.6




[Qemu-devel] [PATCH 03/13] usb-ehci: split our qh queue into async and periodic queues

2012-03-02 Thread Hans de Goede
qhs can be part of both the async and the periodic schedule, as is shown
in later patches in this series it is useful to keep track of the qhs on
a per schedule basis.

Signed-off-by: Hans de Goede 
---
 hw/usb-ehci.c |   62 ++---
 1 files changed, 37 insertions(+), 25 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index d41b80e..840022d 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -347,7 +347,6 @@ enum async_state {
 struct EHCIQueue {
 EHCIState *ehci;
 QTAILQ_ENTRY(EHCIQueue) next;
-bool async_schedule;
 uint32_t seen;
 uint64_t ts;
 
@@ -367,6 +366,8 @@ struct EHCIQueue {
 int usb_status;
 };
 
+typedef QTAILQ_HEAD(EHCIQueueHead, EHCIQueue) EHCIQueueHead;
+
 struct EHCIState {
 PCIDevice dev;
 USBBus bus;
@@ -410,7 +411,8 @@ struct EHCIState {
 USBPort ports[NB_PORTS];
 USBPort *companion_ports[NB_PORTS];
 uint32_t usbsts_pending;
-QTAILQ_HEAD(, EHCIQueue) queues;
+EHCIQueueHead aqueues;
+EHCIQueueHead pqueues;
 
 uint32_t a_fetch_addr;   // which address to look at next
 uint32_t p_fetch_addr;   // which address to look at next
@@ -660,31 +662,34 @@ static void ehci_trace_sitd(EHCIState *s, 
target_phys_addr_t addr,
 
 static EHCIQueue *ehci_alloc_queue(EHCIState *ehci, int async)
 {
+EHCIQueueHead *head = async ? &ehci->aqueues : &ehci->pqueues;
 EHCIQueue *q;
 
 q = g_malloc0(sizeof(*q));
 q->ehci = ehci;
-q->async_schedule = async;
-QTAILQ_INSERT_HEAD(&ehci->queues, q, next);
+QTAILQ_INSERT_HEAD(head, q, next);
 trace_usb_ehci_queue_action(q, "alloc");
 return q;
 }
 
-static void ehci_free_queue(EHCIQueue *q)
+static void ehci_free_queue(EHCIQueue *q, int async)
 {
+EHCIQueueHead *head = async ? &q->ehci->aqueues : &q->ehci->pqueues;
 trace_usb_ehci_queue_action(q, "free");
 if (q->async == EHCI_ASYNC_INFLIGHT) {
 usb_cancel_packet(&q->packet);
 }
-QTAILQ_REMOVE(&q->ehci->queues, q, next);
+QTAILQ_REMOVE(head, q, next);
 g_free(q);
 }
 
-static EHCIQueue *ehci_find_queue_by_qh(EHCIState *ehci, uint32_t addr)
+static EHCIQueue *ehci_find_queue_by_qh(EHCIState *ehci, uint32_t addr,
+int async)
 {
+EHCIQueueHead *head = async ? &ehci->aqueues : &ehci->pqueues;
 EHCIQueue *q;
 
-QTAILQ_FOREACH(q, &ehci->queues, next) {
+QTAILQ_FOREACH(q, head, next) {
 if (addr == q->qhaddr) {
 return q;
 }
@@ -692,11 +697,12 @@ static EHCIQueue *ehci_find_queue_by_qh(EHCIState *ehci, 
uint32_t addr)
 return NULL;
 }
 
-static void ehci_queues_rip_unused(EHCIState *ehci)
+static void ehci_queues_rip_unused(EHCIState *ehci, int async)
 {
+EHCIQueueHead *head = async ? &ehci->aqueues : &ehci->pqueues;
 EHCIQueue *q, *tmp;
 
-QTAILQ_FOREACH_SAFE(q, &ehci->queues, next, tmp) {
+QTAILQ_FOREACH_SAFE(q, head, next, tmp) {
 if (q->seen) {
 q->seen = 0;
 q->ts = ehci->last_run_ns;
@@ -706,29 +712,31 @@ static void ehci_queues_rip_unused(EHCIState *ehci)
 /* allow 0.25 sec idle */
 continue;
 }
-ehci_free_queue(q);
+ehci_free_queue(q, async);
 }
 }
 
-static void ehci_queues_rip_device(EHCIState *ehci, USBDevice *dev)
+static void ehci_queues_rip_device(EHCIState *ehci, USBDevice *dev, int async)
 {
+EHCIQueueHead *head = async ? &ehci->aqueues : &ehci->pqueues;
 EHCIQueue *q, *tmp;
 
-QTAILQ_FOREACH_SAFE(q, &ehci->queues, next, tmp) {
+QTAILQ_FOREACH_SAFE(q, head, next, tmp) {
 if (!usb_packet_is_inflight(&q->packet) ||
 q->packet.ep->dev != dev) {
 continue;
 }
-ehci_free_queue(q);
+ehci_free_queue(q, async);
 }
 }
 
-static void ehci_queues_rip_all(EHCIState *ehci)
+static void ehci_queues_rip_all(EHCIState *ehci, int async)
 {
+EHCIQueueHead *head = async ? &ehci->aqueues : &ehci->pqueues;
 EHCIQueue *q, *tmp;
 
-QTAILQ_FOREACH_SAFE(q, &ehci->queues, next, tmp) {
-ehci_free_queue(q);
+QTAILQ_FOREACH_SAFE(q, head, next, tmp) {
+ehci_free_queue(q, async);
 }
 }
 
@@ -773,7 +781,8 @@ static void ehci_detach(USBPort *port)
 return;
 }
 
-ehci_queues_rip_device(s, port->dev);
+ehci_queues_rip_device(s, port->dev, 0);
+ehci_queues_rip_device(s, port->dev, 1);
 
 *portsc &= ~(PORTSC_CONNECT|PORTSC_PED);
 *portsc |= PORTSC_CSC;
@@ -793,7 +802,8 @@ static void ehci_child_detach(USBPort *port, USBDevice 
*child)
 return;
 }
 
-ehci_queues_rip_device(s, child);
+ehci_queues_rip_device(s, child, 0);
+ehci_queues_rip_device(s, child, 1);
 }
 
 static void ehci_wakeup(USBPort *port)
@@ -911,7 +921,8 @@ static void ehci_reset(void *opaque)
 usb_device_reset(devs[i]);
 }
 }
-ehci_queues_rip_all(s);
+ehci_queues_rip_all(s, 0);
+ehci_queues_rip_all(s, 1);
  

[Qemu-devel] [PATCH 06/13] usb-ehci: Rip the queues when the async or period schedule is halted

2012-03-02 Thread Hans de Goede
Signed-off-by: Hans de Goede 
---
 hw/usb-ehci.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index b349003..d386b84 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -1076,7 +1076,8 @@ static void ehci_mem_writel(void *ptr, target_phys_addr_t 
addr, uint32_t val)
 
 if (!(val & USBCMD_RUNSTOP) && (s->usbcmd & USBCMD_RUNSTOP)) {
 qemu_del_timer(s->frame_timer);
-// TODO - should finish out some stuff before setting halt
+ehci_queues_rip_all(s, 0);
+ehci_queues_rip_all(s, 1);
 ehci_set_usbsts(s, USBSTS_HALT);
 }
 
@@ -2088,6 +2089,7 @@ static void ehci_advance_async_state(EHCIState *ehci)
 
 case EST_ACTIVE:
 if ( !(ehci->usbcmd & USBCMD_ASE)) {
+ehci_queues_rip_all(ehci, async);
 ehci_clear_usbsts(ehci, USBSTS_ASS);
 ehci_set_state(ehci, async, EST_INACTIVE);
 break;
@@ -2148,6 +2150,7 @@ static void ehci_advance_periodic_state(EHCIState *ehci)
 
 case EST_ACTIVE:
 if ( !(ehci->frindex & 7) && !(ehci->usbcmd & USBCMD_PSE)) {
+ehci_queues_rip_all(ehci, async);
 ehci_clear_usbsts(ehci, USBSTS_PSS);
 ehci_set_state(ehci, async, EST_INACTIVE);
 break;
-- 
1.7.7.6




[Qemu-devel] [PATCH 02/13] usb-ehci: Never follow table entries with the T-bit set

2012-03-02 Thread Hans de Goede
Before this patch the T-bit was not checked in 2 places, while it should be.

Once we properly check the T-bit everywhere we no longer need the weird
entry < 0x1000 and entry > 0x1000 checks, so this patch removes them.

Signed-off-by: Hans de Goede 
---
 hw/usb-ehci.c |   10 --
 1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index afc8ccf..d41b80e 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -1568,8 +1568,7 @@ static int ehci_state_fetchentry(EHCIState *ehci, int 
async)
 int again = 0;
 uint32_t entry = ehci_get_fetch_addr(ehci, async);
 
-if (entry < 0x1000) {
-DPRINTF("fetchentry: entry invalid (0x%08x)\n", entry);
+if (NLPTR_TBIT(entry)) {
 ehci_set_state(ehci, async, EST_ACTIVE);
 goto out;
 }
@@ -1677,7 +1676,8 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int 
async)
 if (q->qh.token & QTD_TOKEN_HALT) {
 ehci_set_state(ehci, async, EST_HORIZONTALQH);
 
-} else if ((q->qh.token & QTD_TOKEN_ACTIVE) && (q->qh.current_qtd > 
0x1000)) {
+} else if ((q->qh.token & QTD_TOKEN_ACTIVE) &&
+   (NLPTR_TBIT(q->qh.current_qtd) == 0)) {
 q->qtdaddr = q->qh.current_qtd;
 ehci_set_state(ehci, async, EST_FETCHQTD);
 
@@ -1756,7 +1756,6 @@ static int ehci_state_advqueue(EHCIQueue *q, int async)
  * want data and alt-next qTD is valid
  */
 if (((q->qh.token & QTD_TOKEN_TBYTES_MASK) != 0) &&
-(q->qh.altnext_qtd > 0x1000) &&
 (NLPTR_TBIT(q->qh.altnext_qtd) == 0)) {
 q->qtdaddr = q->qh.altnext_qtd;
 ehci_set_state(q->ehci, async, EST_FETCHQTD);
@@ -1764,8 +1763,7 @@ static int ehci_state_advqueue(EHCIQueue *q, int async)
 /*
  *  next qTD is valid
  */
-} else if ((q->qh.next_qtd > 0x1000) &&
-   (NLPTR_TBIT(q->qh.next_qtd) == 0)) {
+} else if (NLPTR_TBIT(q->qh.next_qtd) == 0) {
 q->qtdaddr = q->qh.next_qtd;
 ehci_set_state(q->ehci, async, EST_FETCHQTD);
 
-- 
1.7.7.6




[Qemu-devel] [PATCH v3 1/4] i8254: Factor out base class for KVM reuse

2012-03-02 Thread Jan Kiszka
Applying the concept used for the *PICs once again: establish a base
class for the i8254 that can be used both by the current user space
emulation and the upcoming KVM in-kernel version. We share most of the
public interface of the i8254, specifically to the pcspk, vmstate, reset
and certain init parts.

Signed-off-by: Jan Kiszka 
---
 Makefile.objs   |2 +-
 hw/i8254.c  |  269 +---
 hw/i8254_common.c   |  307 +++
 hw/i8254_internal.h |   85 ++
 4 files changed, 424 insertions(+), 239 deletions(-)
 create mode 100644 hw/i8254_common.c
 create mode 100644 hw/i8254_internal.h

diff --git a/Makefile.objs b/Makefile.objs
index 808de6a..b39d76c 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -210,7 +210,7 @@ hw-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
 
 hw-obj-$(CONFIG_SERIAL) += serial.o
 hw-obj-$(CONFIG_PARALLEL) += parallel.o
-hw-obj-$(CONFIG_I8254) += i8254.o
+hw-obj-$(CONFIG_I8254) += i8254_common.o i8254.o
 hw-obj-$(CONFIG_PCSPK) += pcspk.o
 hw-obj-$(CONFIG_PCKBD) += pckbd.o
 hw-obj-$(CONFIG_USB_UHCI) += usb-uhci.o
diff --git a/hw/i8254.c b/hw/i8254.c
index f30396a..9fde344 100644
--- a/hw/i8254.c
+++ b/hw/i8254.c
@@ -26,6 +26,7 @@
 #include "isa.h"
 #include "qemu-timer.h"
 #include "i8254.h"
+#include "i8254_internal.h"
 
 //#define DEBUG_PIT
 
@@ -34,34 +35,6 @@
 #define RW_STATE_WORD0 3
 #define RW_STATE_WORD1 4
 
-typedef struct PITChannelState {
-int count; /* can be 65536 */
-uint16_t latched_count;
-uint8_t count_latched;
-uint8_t status_latched;
-uint8_t status;
-uint8_t read_state;
-uint8_t write_state;
-uint8_t write_latch;
-uint8_t rw_mode;
-uint8_t mode;
-uint8_t bcd; /* not supported */
-uint8_t gate; /* timer start */
-int64_t count_load_time;
-/* irq handling */
-int64_t next_transition_time;
-QEMUTimer *irq_timer;
-qemu_irq irq;
-uint32_t irq_disabled;
-} PITChannelState;
-
-typedef struct PITState {
-ISADevice dev;
-MemoryRegion ioports;
-uint32_t iobase;
-PITChannelState channels[3];
-} PITState;
-
 static void pit_irq_timer_update(PITChannelState *s, int64_t current_time);
 
 static int pit_get_count(PITChannelState *s)
@@ -89,99 +62,11 @@ static int pit_get_count(PITChannelState *s)
 return counter;
 }
 
-/* get pit output bit */
-static int pit_get_out(PITChannelState *s, int64_t current_time)
-{
-uint64_t d;
-int out;
-
-d = muldiv64(current_time - s->count_load_time, PIT_FREQ,
- get_ticks_per_sec());
-switch(s->mode) {
-default:
-case 0:
-out = (d >= s->count);
-break;
-case 1:
-out = (d < s->count);
-break;
-case 2:
-if ((d % s->count) == 0 && d != 0)
-out = 1;
-else
-out = 0;
-break;
-case 3:
-out = (d % s->count) < ((s->count + 1) >> 1);
-break;
-case 4:
-case 5:
-out = (d == s->count);
-break;
-}
-return out;
-}
-
-/* return -1 if no transition will occur.  */
-static int64_t pit_get_next_transition_time(PITChannelState *s,
-int64_t current_time)
-{
-uint64_t d, next_time, base;
-int period2;
-
-d = muldiv64(current_time - s->count_load_time, PIT_FREQ,
- get_ticks_per_sec());
-switch(s->mode) {
-default:
-case 0:
-case 1:
-if (d < s->count)
-next_time = s->count;
-else
-return -1;
-break;
-case 2:
-base = (d / s->count) * s->count;
-if ((d - base) == 0 && d != 0)
-next_time = base + s->count;
-else
-next_time = base + s->count + 1;
-break;
-case 3:
-base = (d / s->count) * s->count;
-period2 = ((s->count + 1) >> 1);
-if ((d - base) < period2)
-next_time = base + period2;
-else
-next_time = base + s->count;
-break;
-case 4:
-case 5:
-if (d < s->count)
-next_time = s->count;
-else if (d == s->count)
-next_time = s->count + 1;
-else
-return -1;
-break;
-}
-/* convert to timer units */
-next_time = s->count_load_time + muldiv64(next_time, get_ticks_per_sec(),
-  PIT_FREQ);
-/* fix potential rounding problems */
-/* XXX: better solution: use a clock at PIT_FREQ Hz */
-if (next_time <= current_time)
-next_time = current_time + 1;
-return next_time;
-}
-
 /* val must be 0 or 1 */
-void pit_set_gate(ISADevice *dev, int channel, int val)
+static void pit_set_channel_gate(PITCommonState *s, PITChannelState *sc,
+ int val)
 {
-PITState *pit = DO_UPCAST(PITState, dev, dev);
-PITChannelState *s = &pit->channels[channel];
-
-switch(s->mode) {
+switch

[Qemu-devel] [PATCH v3 3/4] kvm: Add kvm_has_pit_state2 helper

2012-03-02 Thread Jan Kiszka
To be used for in-kernel PIT emulation.

Signed-off-by: Jan Kiszka 
---
 kvm-all.c  |   10 ++
 kvm-stub.c |5 +
 kvm.h  |1 +
 3 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 77eadf6..278085f 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -76,6 +76,7 @@ struct KVMState
 struct kvm_sw_breakpoint_head kvm_sw_breakpoints;
 #endif
 int pit_in_kernel;
+int pit_state2;
 int xsave, xcrs;
 int many_ioeventfds;
 int irqchip_inject_ioctl;
@@ -1058,6 +1059,10 @@ int kvm_init(void)
 s->xcrs = kvm_check_extension(s, KVM_CAP_XCRS);
 #endif
 
+#ifdef KVM_CAP_PIT_STATE2
+s->pit_state2 = kvm_check_extension(s, KVM_CAP_PIT_STATE2);
+#endif
+
 ret = kvm_arch_init(s);
 if (ret < 0) {
 goto err;
@@ -1390,6 +1395,11 @@ int kvm_has_xcrs(void)
 return kvm_state->xcrs;
 }
 
+int kvm_has_pit_state2(void)
+{
+return kvm_state->pit_state2;
+}
+
 int kvm_has_many_ioeventfds(void)
 {
 if (!kvm_enabled()) {
diff --git a/kvm-stub.c b/kvm-stub.c
index f63a0d2..1f1c686 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -78,6 +78,11 @@ int kvm_allows_irq0_override(void)
 return 1;
 }
 
+int kvm_has_pit_state2(void)
+{
+return 0;
+}
+
 void kvm_setup_guest_memory(void *start, size_t size)
 {
 }
diff --git a/kvm.h b/kvm.h
index f9f1dc8..8ef4476 100644
--- a/kvm.h
+++ b/kvm.h
@@ -54,6 +54,7 @@ int kvm_has_robust_singlestep(void);
 int kvm_has_debugregs(void);
 int kvm_has_xsave(void);
 int kvm_has_xcrs(void);
+int kvm_has_pit_state2(void);
 int kvm_has_many_ioeventfds(void);
 int kvm_has_gsi_routing(void);
 
-- 
1.7.3.4




[Qemu-devel] [PATCH v3 4/4] kvm: x86: Add user space part for in-kernel i8254

2012-03-02 Thread Jan Kiszka
This provides the required user space stubs to enable the in-kernel
i8254 emulation of KVM.

The in-kernel model supports lost tick compensation according to the
"delay" policy. This is enabled by default and can be switched off via a
device property.

Depending on the feature set of the host kernel (before 2.6.32), we may
have to disable the HPET or lack sound output from the PC speaker.

Signed-off-by: Jan Kiszka 
---
 Makefile.target |2 +-
 hw/i8254.h  |   11 +++
 hw/kvm/i8254.c  |  254 +++
 hw/pc.c |   14 +++-
 4 files changed, 278 insertions(+), 3 deletions(-)
 create mode 100644 hw/kvm/i8254.c

diff --git a/Makefile.target b/Makefile.target
index 692c9d7..1a7006a 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -244,7 +244,7 @@ obj-i386-y += pci-hotplug.o smbios.o wdt_ib700.o
 obj-i386-y += debugcon.o multiboot.o
 obj-i386-y += pc_piix.o
 obj-i386-y += pc_sysfw.o
-obj-i386-$(CONFIG_KVM) += kvm/clock.o kvm/apic.o kvm/i8259.o kvm/ioapic.o
+obj-i386-$(CONFIG_KVM) += kvm/clock.o kvm/apic.o kvm/i8259.o kvm/ioapic.o 
kvm/i8254.o
 obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
 
 # shared objects
diff --git a/hw/i8254.h b/hw/i8254.h
index a1d2e98..ba6b598 100644
--- a/hw/i8254.h
+++ b/hw/i8254.h
@@ -51,6 +51,17 @@ static inline ISADevice *pit_init(ISABus *bus, int base, int 
isa_irq,
 return dev;
 }
 
+static inline ISADevice *kvm_pit_init(ISABus *bus, int base)
+{
+ISADevice *dev;
+
+dev = isa_create(bus, "kvm-pit");
+qdev_prop_set_uint32(&dev->qdev, "iobase", base);
+qdev_init_nofail(&dev->qdev);
+
+return dev;
+}
+
 void pit_set_gate(ISADevice *dev, int channel, int val);
 void pit_get_channel_info(ISADevice *dev, int channel, PITChannelInfo *info);
 
diff --git a/hw/kvm/i8254.c b/hw/kvm/i8254.c
new file mode 100644
index 000..bb5fe07
--- /dev/null
+++ b/hw/kvm/i8254.c
@@ -0,0 +1,254 @@
+/*
+ * KVM in-kernel PIT (i8254) support
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ * Copyright (c) 2012  Jan Kiszka, Siemens AG
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "qemu-timer.h"
+#include "hw/i8254.h"
+#include "hw/i8254_internal.h"
+#include "kvm.h"
+
+#define KVM_PIT_REINJECT_BIT 0
+
+typedef struct KVMPITState {
+PITCommonState pit;
+LostTickPolicy lost_tick_policy;
+} KVMPITState;
+
+static void kvm_pit_get(PITCommonState *s)
+{
+struct kvm_pit_state2 kpit;
+struct kvm_pit_channel_state *kchan;
+struct PITChannelState *sc;
+int i, ret;
+
+if (kvm_has_pit_state2()) {
+ret = kvm_vm_ioctl(kvm_state, KVM_GET_PIT2, &kpit);
+if (ret < 0) {
+fprintf(stderr, "KVM_GET_PIT2 failed: %s\n", strerror(ret));
+abort();
+}
+s->channels[0].irq_disabled = kpit.flags & KVM_PIT_FLAGS_HPET_LEGACY;
+} else {
+/*
+ * kvm_pit_state2 is superset of kvm_pit_state struct,
+ * so we can use it for KVM_GET_PIT as well.
+ */
+ret = kvm_vm_ioctl(kvm_state, KVM_GET_PIT, &kpit);
+if (ret < 0) {
+fprintf(stderr, "KVM_GET_PIT failed: %s\n", strerror(ret));
+abort();
+}
+}
+for (i = 0; i < 3; i++) {
+kchan = &kpit.channels[i];
+sc = &s->channels[i];
+sc->count = kchan->count;
+sc->latched_count = kchan->latched_count;
+sc->count_latched = kchan->count_latched;
+sc->status_latched = kchan->status_latched;
+sc->status = kchan->status;
+sc->read_state = kchan->read_state;
+sc->write_state = kchan->write_state;
+sc->write_latch = kchan->write_latch;
+sc->rw_mode = kchan->rw_mode;
+sc->mode = kchan->mode;
+sc->bcd = kchan->bcd;
+sc->gate = kchan->gate;
+sc->count_load_time = kchan->count_load_time;
+}
+
+sc = &s->channels[0];
+sc->next_transition_time =
+pit_get_next_tr

[Qemu-devel] [PATCH v3 2/4] i8254: Open-code timer restore

2012-03-02 Thread Jan Kiszka
Same as for the APIC: To enable migration between accelerated and
non-accelerated models, we need to arm the channel 0 timer only inside
the emulated PIT model. The common code just saves/restores that timer
to the the next_transition_time field.

Signed-off-by: Jan Kiszka 
---
 hw/i8254.c|   12 
 hw/i8254_common.c |   10 +++---
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/hw/i8254.c b/hw/i8254.c
index 9fde344..77bd5e8 100644
--- a/hw/i8254.c
+++ b/hw/i8254.c
@@ -300,6 +300,17 @@ static const MemoryRegionOps pit_ioport_ops = {
 .old_portio = pit_portio
 };
 
+static void pit_post_load(PITCommonState *s)
+{
+PITChannelState *sc = &s->channels[0];
+
+if (sc->next_transition_time != -1) {
+qemu_mod_timer(sc->irq_timer, sc->next_transition_time);
+} else {
+qemu_del_timer(sc->irq_timer);
+}
+}
+
 static int pit_initfn(PITCommonState *pit)
 {
 PITChannelState *s;
@@ -329,6 +340,7 @@ static void pit_class_initfn(ObjectClass *klass, void *data)
 k->init = pit_initfn;
 k->set_channel_gate = pit_set_channel_gate;
 k->get_channel_info = pit_get_channel_info_common;
+k->post_load = pit_post_load;
 dc->reset = pit_reset;
 dc->props = pit_properties;
 }
diff --git a/hw/i8254_common.c b/hw/i8254_common.c
index 6373e87..a03d7cd 100644
--- a/hw/i8254_common.c
+++ b/hw/i8254_common.c
@@ -211,6 +211,7 @@ static const VMStateDescription vmstate_pit_channel = {
 static int pit_load_old(QEMUFile *f, void *opaque, int version_id)
 {
 PITCommonState *pit = opaque;
+PITCommonClass *c = PIT_COMMON_GET_CLASS(pit);
 PITChannelState *s;
 int i;
 
@@ -234,11 +235,13 @@ static int pit_load_old(QEMUFile *f, void *opaque, int 
version_id)
 qemu_get_8s(f, &s->gate);
 s->count_load_time = qemu_get_be64(f);
 s->irq_disabled = 0;
-if (s->irq_timer) {
+if (i == 0) {
 s->next_transition_time = qemu_get_be64(f);
-qemu_get_timer(f, s->irq_timer);
 }
 }
+if (c->post_load) {
+c->post_load(pit);
+}
 return 0;
 }
 
@@ -275,7 +278,8 @@ static const VMStateDescription vmstate_pit_common = {
 VMSTATE_UINT32_V(channels[0].irq_disabled, PITCommonState, 3),
 VMSTATE_STRUCT_ARRAY(channels, PITCommonState, 3, 2,
  vmstate_pit_channel, PITChannelState),
-VMSTATE_TIMER(channels[0].irq_timer, PITCommonState),
+VMSTATE_INT64(channels[0].next_transition_time,
+  PITCommonState), /* formerly irq_timer */
 VMSTATE_END_OF_LIST()
 }
 };
-- 
1.7.3.4




[Qemu-devel] [PATCH v3 0/4] uq/master: Introduce KVM PIT support

2012-03-02 Thread Jan Kiszka
This adds another piece of qemu-kvm to upstream: The accelerated
in-kernel model of the i8254. It does this in the same fashion as the
interrupt controllers were already introduced. And it even has one bug
less than qemu-kvm: PC speaker output still works with KVM acceleration
enabled.

Changes in v3:
 - Rebased over recent upstream master
 - Aligned license of hw/kvm/i8254.c to the other PIT files

The patches apply on top of upstream commit 88e6c60671, thus require an
uq/master update.

Please merge.

Jan Kiszka (4):
  i8254: Factor out base class for KVM reuse
  i8254: Open-code timer restore
  kvm: Add kvm_has_pit_state2 helper
  kvm: x86: Add user space part for in-kernel i8254

 Makefile.objs   |2 +-
 Makefile.target |2 +-
 hw/i8254.c  |  281 +++---
 hw/i8254.h  |   11 ++
 hw/i8254_common.c   |  311 +++
 hw/i8254_internal.h |   85 ++
 hw/kvm/i8254.c  |  254 +
 hw/pc.c |   14 ++-
 kvm-all.c   |   10 ++
 kvm-stub.c  |5 +
 kvm.h   |1 +
 11 files changed, 734 insertions(+), 242 deletions(-)
 create mode 100644 hw/i8254_common.c
 create mode 100644 hw/i8254_internal.h
 create mode 100644 hw/kvm/i8254.c

-- 
1.7.3.4




[Qemu-devel] [PATCH 09/13] usb-ehci: Remove dead nakcnt code

2012-03-02 Thread Hans de Goede
This patch removes 2 bits of dead nakcnt code:

1) usb_ehci_execute calls ehci_qh_do_overlay which does:
nakcnt = reload;
and then has a block of code which is conditional on:
if (reload && !nakcnt) {
which ofcourse is never true now as nakcnt == reload.

2) ehci_state_fetchqh does:
nakcnt = reload;
but before nakcnt is ever used ehci_state_fetchqh is always followed
by a ehci_qh_do_overlay call which also does:
nakcnt = reload;
So doing this from ehci_state_fetchqh is redundant.

Signed-off-by: Hans de Goede 
---
 hw/usb-ehci.c |   20 
 1 files changed, 0 insertions(+), 20 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 2685adc..07bcd1f 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -1615,7 +1615,6 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int 
async)
 {
 uint32_t entry;
 EHCIQueue *q;
-int reload;
 
 entry = ehci_get_fetch_addr(ehci, async);
 q = ehci_find_queue_by_qh(ehci, entry, async);
@@ -1673,11 +1672,6 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, 
int async)
 }
 #endif
 
-reload = get_field(q->qh.epchar, QH_EPCHAR_RL);
-if (reload) {
-set_field(&q->qh.altnext_qtd, reload, QH_ALTNEXT_NAKCNT);
-}
-
 if (q->qh.token & QTD_TOKEN_HALT) {
 ehci_set_state(ehci, async, EST_HORIZONTALQH);
 
@@ -1837,25 +1831,11 @@ static void ehci_flush_qh(EHCIQueue *q)
 static int ehci_state_execute(EHCIQueue *q, int async)
 {
 int again = 0;
-int reload, nakcnt;
-int smask;
 
 if (ehci_qh_do_overlay(q) != 0) {
 return -1;
 }
 
-smask = get_field(q->qh.epcap, QH_EPCAP_SMASK);
-
-if (!smask) {
-reload = get_field(q->qh.epchar, QH_EPCHAR_RL);
-nakcnt = get_field(q->qh.altnext_qtd, QH_ALTNEXT_NAKCNT);
-if (reload && !nakcnt) {
-ehci_set_state(q->ehci, async, EST_HORIZONTALQH);
-again = 1;
-goto out;
-}
-}
-
 // TODO verify enough time remains in the uframe as in 4.4.1.1
 // TODO write back ptr to async list when done or out of time
 // TODO Windows does not seem to ever set the MULT field
-- 
1.7.7.6




[Qemu-devel] [PATCH 12/13] usb: return BABBLE rather then NAK when we receive too much data

2012-03-02 Thread Hans de Goede
Signed-off-by: Hans de Goede 
---
 usb-linux.c |8 +++-
 usb-redir.c |4 ++--
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/usb-linux.c b/usb-linux.c
index 47994f3..38df9e6 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -364,6 +364,10 @@ static void async_complete(void *opaque)
 p->result = USB_RET_STALL;
 break;
 
+case -EOVERFLOW:
+p->result = USB_RET_BABBLE;
+break;
+
 default:
 p->result = USB_RET_NAK;
 break;
@@ -722,6 +726,8 @@ static int urb_status_to_usb_ret(int status)
 switch (status) {
 case -EPIPE:
 return USB_RET_STALL;
+case -EOVERFLOW:
+return USB_RET_BABBLE;
 default:
 return USB_RET_NAK;
 }
@@ -759,7 +765,7 @@ static int usb_host_handle_iso_data(USBHostDevice *s, 
USBPacket *p, int in)
 } else if (aurb[i].urb.iso_frame_desc[j].actual_length
> p->iov.size) {
 printf("husb: received iso data is larger then packet\n");
-len = USB_RET_NAK;
+len = USB_RET_BABBLE;
 /* All good copy data over */
 } else {
 len = aurb[i].urb.iso_frame_desc[j].actual_length;
diff --git a/usb-redir.c b/usb-redir.c
index ef9fa96..4c8e3e3 100644
--- a/usb-redir.c
+++ b/usb-redir.c
@@ -457,7 +457,7 @@ static int usbredir_handle_iso_data(USBRedirDevice *dev, 
USBPacket *p,
 ERROR("received iso data is larger then packet ep %02X (%d > 
%d)\n",
   ep, len, (int)p->iov.size);
 bufp_free(dev, isop, ep);
-return USB_RET_NAK;
+return USB_RET_BABBLE;
 }
 usb_packet_copy(p, isop->data, len);
 bufp_free(dev, isop, ep);
@@ -576,7 +576,7 @@ static int usbredir_handle_interrupt_data(USBRedirDevice 
*dev,
 if (len > p->iov.size) {
 ERROR("received int data is larger then packet ep %02X\n", ep);
 bufp_free(dev, intp, ep);
-return USB_RET_NAK;
+return USB_RET_BABBLE;
 }
 usb_packet_copy(p, intp->data, len);
 bufp_free(dev, intp, ep);
-- 
1.7.7.6




[Qemu-devel] [PATCH 11/13] usb-ehci: Cleanup itd error handling

2012-03-02 Thread Hans de Goede
All error statuses except for NAK are handled in a switch case, move the
handling of NAK into the same switch case.

Signed-off-by: Hans de Goede 
---
 hw/usb-ehci.c |   28 ++--
 1 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 9197298..825fcc0 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -1466,20 +1466,7 @@ static int ehci_process_itd(EHCIState *ehci,
 }
 qemu_sglist_destroy(&ehci->isgl);
 
-if (ret == USB_RET_NAK) {
-/* no data for us, so do a zero-length transfer */
-ret = 0;
-}
-
-if (ret >= 0) {
-if (!dir) {
-/* OUT */
-set_field(&itd->transact[i], len - ret, ITD_XACT_LENGTH);
-} else {
-/* IN */
-set_field(&itd->transact[i], ret, ITD_XACT_LENGTH);
-}
-} else {
+if (ret < 0) {
 switch (ret) {
 default:
 fprintf(stderr, "Unexpected iso usb result: %d\n", ret);
@@ -1495,6 +1482,19 @@ static int ehci_process_itd(EHCIState *ehci,
 itd->transact[i] |= ITD_XACT_BABBLE;
 ehci_record_interrupt(ehci, USBSTS_ERRINT);
 break;
+case USB_RET_NAK:
+/* no data for us, so do a zero-length transfer */
+ret = 0;
+break;
+}
+}
+if (ret >= 0) {
+if (!dir) {
+/* OUT */
+set_field(&itd->transact[i], len - ret, ITD_XACT_LENGTH);
+} else {
+/* IN */
+set_field(&itd->transact[i], ret, ITD_XACT_LENGTH);
 }
 }
 if (itd->transact[i] & ITD_XACT_IOC) {
-- 
1.7.7.6




[Qemu-devel] [PATCH 04/13] usb-ehci: always call ehci_queues_rip_unused for period queues

2012-03-02 Thread Hans de Goede
Before this patch USB 2 devices with interrupt endpoints were not working
properly. The problem is that to avoid loops we stop processing as soon
as we encounter a queue-head (qh) we've already seen since qhs can be linked
in a circular fashion, this is tracked by the seen flag in our qh struct.

The resetting of the seen flag is done from ehci_queues_rip_unused which
before this patch was only called when executing the statemachine for the
async schedule.

But packets for interrupt endpoints are part of the periodic schedule! So what
would happen is that when there were no ctrl or bulk packets for a USB 2
device with an interrupt endpoint, the async schedule would become non
active, then ehci_queues_rip_unused would no longer get called and when
processing the qhs for the interrupt endpoints from the periodic schedule
their seen bit would still be 1 and they would be skipped.

Signed-off-by: Hans de Goede 
---
 hw/usb-ehci.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 840022d..d0d0144 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -2167,6 +2167,7 @@ static void ehci_advance_periodic_state(EHCIState *ehci)
 ehci_set_fetch_addr(ehci, async,entry);
 ehci_set_state(ehci, async, EST_FETCHENTRY);
 ehci_advance_state(ehci, async);
+ehci_queues_rip_unused(ehci, async, 0);
 break;
 
 default:
-- 
1.7.7.6




[Qemu-devel] [PATCH 03/13] usb-ehci: split our qh queue into async and periodic queues

2012-03-02 Thread Hans de Goede
qhs can be part of both the async and the periodic schedule, as is shown
in later patches in this series it is useful to keep track of the qhs on
a per schedule basis.

Signed-off-by: Hans de Goede 
---
 hw/usb-ehci.c |   62 ++---
 1 files changed, 37 insertions(+), 25 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index d41b80e..840022d 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -347,7 +347,6 @@ enum async_state {
 struct EHCIQueue {
 EHCIState *ehci;
 QTAILQ_ENTRY(EHCIQueue) next;
-bool async_schedule;
 uint32_t seen;
 uint64_t ts;
 
@@ -367,6 +366,8 @@ struct EHCIQueue {
 int usb_status;
 };
 
+typedef QTAILQ_HEAD(EHCIQueueHead, EHCIQueue) EHCIQueueHead;
+
 struct EHCIState {
 PCIDevice dev;
 USBBus bus;
@@ -410,7 +411,8 @@ struct EHCIState {
 USBPort ports[NB_PORTS];
 USBPort *companion_ports[NB_PORTS];
 uint32_t usbsts_pending;
-QTAILQ_HEAD(, EHCIQueue) queues;
+EHCIQueueHead aqueues;
+EHCIQueueHead pqueues;
 
 uint32_t a_fetch_addr;   // which address to look at next
 uint32_t p_fetch_addr;   // which address to look at next
@@ -660,31 +662,34 @@ static void ehci_trace_sitd(EHCIState *s, 
target_phys_addr_t addr,
 
 static EHCIQueue *ehci_alloc_queue(EHCIState *ehci, int async)
 {
+EHCIQueueHead *head = async ? &ehci->aqueues : &ehci->pqueues;
 EHCIQueue *q;
 
 q = g_malloc0(sizeof(*q));
 q->ehci = ehci;
-q->async_schedule = async;
-QTAILQ_INSERT_HEAD(&ehci->queues, q, next);
+QTAILQ_INSERT_HEAD(head, q, next);
 trace_usb_ehci_queue_action(q, "alloc");
 return q;
 }
 
-static void ehci_free_queue(EHCIQueue *q)
+static void ehci_free_queue(EHCIQueue *q, int async)
 {
+EHCIQueueHead *head = async ? &q->ehci->aqueues : &q->ehci->pqueues;
 trace_usb_ehci_queue_action(q, "free");
 if (q->async == EHCI_ASYNC_INFLIGHT) {
 usb_cancel_packet(&q->packet);
 }
-QTAILQ_REMOVE(&q->ehci->queues, q, next);
+QTAILQ_REMOVE(head, q, next);
 g_free(q);
 }
 
-static EHCIQueue *ehci_find_queue_by_qh(EHCIState *ehci, uint32_t addr)
+static EHCIQueue *ehci_find_queue_by_qh(EHCIState *ehci, uint32_t addr,
+int async)
 {
+EHCIQueueHead *head = async ? &ehci->aqueues : &ehci->pqueues;
 EHCIQueue *q;
 
-QTAILQ_FOREACH(q, &ehci->queues, next) {
+QTAILQ_FOREACH(q, head, next) {
 if (addr == q->qhaddr) {
 return q;
 }
@@ -692,11 +697,12 @@ static EHCIQueue *ehci_find_queue_by_qh(EHCIState *ehci, 
uint32_t addr)
 return NULL;
 }
 
-static void ehci_queues_rip_unused(EHCIState *ehci)
+static void ehci_queues_rip_unused(EHCIState *ehci, int async)
 {
+EHCIQueueHead *head = async ? &ehci->aqueues : &ehci->pqueues;
 EHCIQueue *q, *tmp;
 
-QTAILQ_FOREACH_SAFE(q, &ehci->queues, next, tmp) {
+QTAILQ_FOREACH_SAFE(q, head, next, tmp) {
 if (q->seen) {
 q->seen = 0;
 q->ts = ehci->last_run_ns;
@@ -706,29 +712,31 @@ static void ehci_queues_rip_unused(EHCIState *ehci)
 /* allow 0.25 sec idle */
 continue;
 }
-ehci_free_queue(q);
+ehci_free_queue(q, async);
 }
 }
 
-static void ehci_queues_rip_device(EHCIState *ehci, USBDevice *dev)
+static void ehci_queues_rip_device(EHCIState *ehci, USBDevice *dev, int async)
 {
+EHCIQueueHead *head = async ? &ehci->aqueues : &ehci->pqueues;
 EHCIQueue *q, *tmp;
 
-QTAILQ_FOREACH_SAFE(q, &ehci->queues, next, tmp) {
+QTAILQ_FOREACH_SAFE(q, head, next, tmp) {
 if (!usb_packet_is_inflight(&q->packet) ||
 q->packet.ep->dev != dev) {
 continue;
 }
-ehci_free_queue(q);
+ehci_free_queue(q, async);
 }
 }
 
-static void ehci_queues_rip_all(EHCIState *ehci)
+static void ehci_queues_rip_all(EHCIState *ehci, int async)
 {
+EHCIQueueHead *head = async ? &ehci->aqueues : &ehci->pqueues;
 EHCIQueue *q, *tmp;
 
-QTAILQ_FOREACH_SAFE(q, &ehci->queues, next, tmp) {
-ehci_free_queue(q);
+QTAILQ_FOREACH_SAFE(q, head, next, tmp) {
+ehci_free_queue(q, async);
 }
 }
 
@@ -773,7 +781,8 @@ static void ehci_detach(USBPort *port)
 return;
 }
 
-ehci_queues_rip_device(s, port->dev);
+ehci_queues_rip_device(s, port->dev, 0);
+ehci_queues_rip_device(s, port->dev, 1);
 
 *portsc &= ~(PORTSC_CONNECT|PORTSC_PED);
 *portsc |= PORTSC_CSC;
@@ -793,7 +802,8 @@ static void ehci_child_detach(USBPort *port, USBDevice 
*child)
 return;
 }
 
-ehci_queues_rip_device(s, child);
+ehci_queues_rip_device(s, child, 0);
+ehci_queues_rip_device(s, child, 1);
 }
 
 static void ehci_wakeup(USBPort *port)
@@ -911,7 +921,8 @@ static void ehci_reset(void *opaque)
 usb_device_reset(devs[i]);
 }
 }
-ehci_queues_rip_all(s);
+ehci_queues_rip_all(s, 0);
+ehci_queues_rip_all(s, 1);
  

[Qemu-devel] [PATCH 05/13] usb-ehci: Drop cached qhs when the doorbell gets rung

2012-03-02 Thread Hans de Goede
The purpose of the IAAD bit / the doorbell is to make the ehci controller
forget about cached qhs, this is mainly used when cancelling transactions,
the qh is unlinked from the async schedule and then the doorbell gets rung,
once the doorbell is acked by the controller the hcd knows that the qh is
no longer in use and that it can do something else with the memory, such
as re-use it for a new qh! But we keep our struct representing this qh around
for circa 250 ms. This allows for a (mightily large) race window where the
following could happen:
-hcd submits a qh at address 0xdeadbeef
-our ehci code sees the qh, sends a request to a usb-device, gets a result
 of USB_RET_ASYNC, sets the async_state of the qh to EHCI_ASYNC_INFLIGHT
-hcd unlinks the qh at address 0xdeadbeef
-hcd rings the doorbell, wait for us to ack it
-hcd re-uses the qh at address 0xdeadbeef
-our ehci code sees the qh, looks in the async_queue, sees there already is
 a qh at address 0xdeadbeef there with async_state of EHCI_ASYNC_INFLIGHT,
 does nothing
-the *original* (which the hcd thinks it has cancelled) transaction finishes
-our ehci code sees the qh on yet another pass through the async list,
 looks in the async_queue, sees there already is a qh at address 0xdeadbeef
 there with async_state of EHCI_ASYNC_COMPLETED, and finished the transaction
 with the results of the *original* transaction.

Not good (tm), this patch fixes this race by removing all qhs which have not
been seen during the last cycle through the async list immidiately when the
doorbell is rung.

Note this patch does not fix any actually observed problem, but upon
reading of the EHCI spec it became apparent to me that the above race could
happen and the usb-ehci behavior from before this patch is not good.

Signed-off-by: Hans de Goede 
---
 hw/usb-ehci.c |   31 ---
 1 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index d0d0144..b349003 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -697,7 +697,7 @@ static EHCIQueue *ehci_find_queue_by_qh(EHCIState *ehci, 
uint32_t addr,
 return NULL;
 }
 
-static void ehci_queues_rip_unused(EHCIState *ehci, int async)
+static void ehci_queues_rip_unused(EHCIState *ehci, int async, int flush)
 {
 EHCIQueueHead *head = async ? &ehci->aqueues : &ehci->pqueues;
 EHCIQueue *q, *tmp;
@@ -708,7 +708,7 @@ static void ehci_queues_rip_unused(EHCIState *ehci, int 
async)
 q->ts = ehci->last_run_ns;
 continue;
 }
-if (ehci->last_run_ns < q->ts + 25000) {
+if (!flush && ehci->last_run_ns < q->ts + 25000) {
 /* allow 0.25 sec idle */
 continue;
 }
@@ -1537,7 +1537,7 @@ static int ehci_state_waitlisthead(EHCIState *ehci,  int 
async)
 ehci_set_usbsts(ehci, USBSTS_REC);
 }
 
-ehci_queues_rip_unused(ehci, async);
+ehci_queues_rip_unused(ehci, async, 0);
 
 /*  Find the head of the list (4.9.1.1) */
 for(i = 0; i < MAX_QH; i++) {
@@ -2093,18 +2093,7 @@ static void ehci_advance_async_state(EHCIState *ehci)
 break;
 }
 
-/* If the doorbell is set, the guest wants to make a change to the
- * schedule. The host controller needs to release cached data.
- * (section 4.8.2)
- */
-if (ehci->usbcmd & USBCMD_IAAD) {
-DPRINTF("ASYNC: doorbell request acknowledged\n");
-ehci->usbcmd &= ~USBCMD_IAAD;
-ehci_set_interrupt(ehci, USBSTS_IAA);
-break;
-}
-
-/* make sure guest has acknowledged */
+/* make sure guest has acknowledged the doorbell interrupt */
 /* TO-DO: is this really needed? */
 if (ehci->usbsts & USBSTS_IAA) {
 DPRINTF("IAA status bit still set.\n");
@@ -2118,6 +2107,18 @@ static void ehci_advance_async_state(EHCIState *ehci)
 
 ehci_set_state(ehci, async, EST_WAITLISTHEAD);
 ehci_advance_state(ehci, async);
+
+/* If the doorbell is set, the guest wants to make a change to the
+ * schedule. The host controller needs to release cached data.
+ * (section 4.8.2)
+ */
+if (ehci->usbcmd & USBCMD_IAAD) {
+/* Remove all unseen qhs from the async qhs queue */
+ehci_queues_rip_unused(ehci, async, 1);
+DPRINTF("ASYNC: doorbell request acknowledged\n");
+ehci->usbcmd &= ~USBCMD_IAAD;
+ehci_set_interrupt(ehci, USBSTS_IAA);
+}
 break;
 
 default:
-- 
1.7.7.6




[Qemu-devel] [PATCH v2 2/4] slirp: Fix queue walking in if_start

2012-03-02 Thread Jan Kiszka
Another attempt to get this right: We need to carefully walk both the
fastq and the batchq in if_start while trying to send packets to
possibly not yet resolved hosts on the virtual network.

So far we just requeued a delayed packet where it was and then started
walking the queues from the top again - that couldn't work. Now we pre-
calculate the next packet in the queue so that the current one can
safely be removed if it was sent successfully. We also need to take into
account that the next packet can be from the same session if the current
one was sent and there are no other sessions.

CC: Fabien Chouteau 
CC: Zhi Yong Wu 
CC: Stefan Weil 
Signed-off-by: Jan Kiszka 
---
 slirp/if.c |   46 +++---
 1 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/slirp/if.c b/slirp/if.c
index 14fdef1..6f59620 100644
--- a/slirp/if.c
+++ b/slirp/if.c
@@ -158,26 +158,35 @@ void if_start(Slirp *slirp)
 {
 uint64_t now = qemu_get_clock_ns(rt_clock);
 int requeued = 0;
-bool from_batchq = false;
-struct mbuf *ifm, *ifqt;
+struct mbuf *ifm, *ifm_next, *ifqt;
 
 DEBUG_CALL("if_start");
 
-while (slirp->if_queued) {
+if (slirp->if_fastq.ifq_next != &slirp->if_fastq) {
+ifm_next = slirp->if_fastq.ifq_next;
+} else if (slirp->next_m != &slirp->if_batchq) {
+/* Nothing on fastq, pick up from batchq via next_m */
+ifm_next = slirp->next_m;
+} else {
+ifm_next = NULL;
+}
+
+while (ifm_next) {
 /* check if we can really output */
-if (!slirp_can_output(slirp->opaque))
+if (!slirp_can_output(slirp->opaque)) {
 return;
+}
 
-/*
- * See which queue to get next packet from
- * If there's something in the fastq, select it immediately
- */
-if (slirp->if_fastq.ifq_next != &slirp->if_fastq) {
-ifm = slirp->if_fastq.ifq_next;
-} else {
-/* Nothing on fastq, pick up from batchq via next_m */
-ifm = slirp->next_m;
-from_batchq = true;
+ifm = ifm_next;
+
+ifm_next = ifm->ifq_next;
+if (ifm_next == &slirp->if_fastq) {
+/* No more packets in fastq, switch to batchq */
+ifm_next = slirp->next_m;
+}
+if (ifm_next == &slirp->if_batchq) {
+/* end of batchq */
+ifm_next = NULL;
 }
 
 slirp->if_queued--;
@@ -189,7 +198,7 @@ void if_start(Slirp *slirp)
 continue;
 }
 
-if (from_batchq) {
+if (ifm == slirp->next_m) {
 /* Set which packet to send on next iteration */
 slirp->next_m = ifm->ifq_next;
 }
@@ -202,11 +211,11 @@ void if_start(Slirp *slirp)
 if (ifm->ifs_next != ifm) {
 insque(ifm->ifs_next, ifqt);
 ifs_remque(ifm);
-/* Set next_m if the session packet is now the only one on
- * batchq */
+/* Set next_m and ifm_next if the session packet is now the only
+ * one on batchq */
 if (ifqt == &slirp->if_batchq &&
 slirp->next_m == &slirp->if_batchq) {
-slirp->next_m = ifm->ifs_next;
+slirp->next_m = ifm_next = ifm->ifs_next;
 }
 }
 
@@ -217,7 +226,6 @@ void if_start(Slirp *slirp)
 }
 
 m_free(ifm);
-
 }
 
 slirp->if_queued = requeued;
-- 
1.7.3.4




[Qemu-devel] [PATCH v2 1/4] slirp: Keep next_m always valid

2012-03-02 Thread Jan Kiszka
Make sure that next_m always points to a packet if batchq is non-empty.
This will simplify walking the queues in if_start.

CC: Fabien Chouteau 
CC: Zhi Yong Wu 
CC: Stefan Weil 
Signed-off-by: Jan Kiszka 
---
 slirp/if.c |   22 ++
 1 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/slirp/if.c b/slirp/if.c
index 33f08e1..14fdef1 100644
--- a/slirp/if.c
+++ b/slirp/if.c
@@ -96,8 +96,13 @@ if_output(struct socket *so, struct mbuf *ifm)
ifs_insque(ifm, ifq->ifs_prev);
goto diddit;
}
-   } else
+} else {
ifq = slirp->if_batchq.ifq_prev;
+/* Set next_m if the queue was empty so far */
+if (slirp->next_m == &slirp->if_batchq) {
+slirp->next_m = ifm;
+}
+}
 
/* Create a new doubly linked list for this session */
ifm->ifq_so = so;
@@ -170,13 +175,8 @@ void if_start(Slirp *slirp)
 if (slirp->if_fastq.ifq_next != &slirp->if_fastq) {
 ifm = slirp->if_fastq.ifq_next;
 } else {
-/* Nothing on fastq, see if next_m is valid */
-if (slirp->next_m != &slirp->if_batchq) {
-ifm = slirp->next_m;
-} else {
-ifm = slirp->if_batchq.ifq_next;
-}
-
+/* Nothing on fastq, pick up from batchq via next_m */
+ifm = slirp->next_m;
 from_batchq = true;
 }
 
@@ -202,6 +202,12 @@ void if_start(Slirp *slirp)
 if (ifm->ifs_next != ifm) {
 insque(ifm->ifs_next, ifqt);
 ifs_remque(ifm);
+/* Set next_m if the session packet is now the only one on
+ * batchq */
+if (ifqt == &slirp->if_batchq &&
+slirp->next_m == &slirp->if_batchq) {
+slirp->next_m = ifm->ifs_next;
+}
 }
 
 /* Update so_queued */
-- 
1.7.3.4




[Qemu-devel] [PATCH v2 0/4] slirp: Fix for requeuing crash, cleanups

2012-03-02 Thread Jan Kiszka
Well, this requeuing bug seems to have a long breath. Previous attempts
to fix it (mine included) neglected the fact that we need to walk the
queue of pending packets, not just restart from the beginning after a
requeue. This version should get it Right(TM).

This also comes with a fix for resource cleanups on slirp shutdown. At
least valgrind is happy now.

Changes in v2:
 - fixed corner case of session list walk that Stefan Weil reported

CC: Fabien Chouteau 
CC: Michael S. Tsirkin 
CC: Stefan Weil 
CC: Zhi Yong Wu 

Jan Kiszka (4):
  slirp: Keep next_m always valid
  slirp: Fix queue walking in if_start
  slirp: Remove unneeded if_queued
  slirp: Cleanup resources on instance removal

 slirp/if.c   |   64 +
 slirp/ip_icmp.c  |7 ++
 slirp/ip_icmp.h  |1 +
 slirp/ip_input.c |7 ++
 slirp/mbuf.c |   21 +
 slirp/mbuf.h |1 +
 slirp/slirp.c|   10 +++-
 slirp/slirp.h|3 +-
 slirp/tcp_subr.c |7 ++
 slirp/udp.c  |8 ++
 slirp/udp.h  |1 +
 11 files changed, 94 insertions(+), 36 deletions(-)

-- 
1.7.3.4




[Qemu-devel] [PATCH v2 4/4] slirp: Cleanup resources on instance removal

2012-03-02 Thread Jan Kiszka
Close & free sockets when shutting down a slirp instance, also release
all buffers.

CC: Michael S. Tsirkin 
Signed-off-by: Jan Kiszka 
---
 slirp/ip_icmp.c  |7 +++
 slirp/ip_icmp.h  |1 +
 slirp/ip_input.c |7 +++
 slirp/mbuf.c |   21 +
 slirp/mbuf.h |1 +
 slirp/slirp.c|3 +++
 slirp/slirp.h|2 ++
 slirp/tcp_subr.c |7 +++
 slirp/udp.c  |8 
 slirp/udp.h  |1 +
 10 files changed, 58 insertions(+), 0 deletions(-)

diff --git a/slirp/ip_icmp.c b/slirp/ip_icmp.c
index 5dbf21d..d571fd0 100644
--- a/slirp/ip_icmp.c
+++ b/slirp/ip_icmp.c
@@ -66,6 +66,13 @@ void icmp_init(Slirp *slirp)
 slirp->icmp_last_so = &slirp->icmp;
 }
 
+void icmp_cleanup(Slirp *slirp)
+{
+while (slirp->icmp.so_next != &slirp->icmp) {
+icmp_detach(slirp->icmp.so_next);
+}
+}
+
 static int icmp_send(struct socket *so, struct mbuf *m, int hlen)
 {
 struct ip *ip = mtod(m, struct ip *);
diff --git a/slirp/ip_icmp.h b/slirp/ip_icmp.h
index b3da1f2..1a1af91 100644
--- a/slirp/ip_icmp.h
+++ b/slirp/ip_icmp.h
@@ -154,6 +154,7 @@ struct icmp {
(type) == ICMP_MASKREQ || (type) == ICMP_MASKREPLY)
 
 void icmp_init(Slirp *slirp);
+void icmp_cleanup(Slirp *slirp);
 void icmp_input(struct mbuf *, int);
 void icmp_error(struct mbuf *msrc, u_char type, u_char code, int minsize,
 const char *message);
diff --git a/slirp/ip_input.c b/slirp/ip_input.c
index c7b3eb4..ce24faf 100644
--- a/slirp/ip_input.c
+++ b/slirp/ip_input.c
@@ -61,6 +61,13 @@ ip_init(Slirp *slirp)
 icmp_init(slirp);
 }
 
+void ip_cleanup(Slirp *slirp)
+{
+udp_cleanup(slirp);
+tcp_cleanup(slirp);
+icmp_cleanup(slirp);
+}
+
 /*
  * Ip input routine.  Checksum and byte swap header.  If fragmented
  * try to reassemble.  Process options.  Pass to next level.
diff --git a/slirp/mbuf.c b/slirp/mbuf.c
index c699c75..4fefb04 100644
--- a/slirp/mbuf.c
+++ b/slirp/mbuf.c
@@ -32,6 +32,27 @@ m_init(Slirp *slirp)
 slirp->m_usedlist.m_next = slirp->m_usedlist.m_prev = &slirp->m_usedlist;
 }
 
+void m_cleanup(Slirp *slirp)
+{
+struct mbuf *m, *next;
+
+m = slirp->m_usedlist.m_next;
+while (m != &slirp->m_usedlist) {
+next = m->m_next;
+if (m->m_flags & M_EXT) {
+free(m->m_ext);
+}
+free(m);
+m = next;
+}
+m = slirp->m_freelist.m_next;
+while (m != &slirp->m_freelist) {
+next = m->m_next;
+free(m);
+m = next;
+}
+}
+
 /*
  * Get an mbuf from the free list, if there are none
  * malloc one
diff --git a/slirp/mbuf.h b/slirp/mbuf.h
index 8d7951f..3f3ab09 100644
--- a/slirp/mbuf.h
+++ b/slirp/mbuf.h
@@ -116,6 +116,7 @@ struct mbuf {
 * it rather than putting it on the 
free list */
 
 void m_init(Slirp *);
+void m_cleanup(Slirp *slirp);
 struct mbuf * m_get(Slirp *);
 void m_free(struct mbuf *);
 void m_cat(register struct mbuf *, register struct mbuf *);
diff --git a/slirp/slirp.c b/slirp/slirp.c
index bcffc34..1502830 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -246,6 +246,9 @@ void slirp_cleanup(Slirp *slirp)
 
 unregister_savevm(NULL, "slirp", slirp);
 
+ip_cleanup(slirp);
+m_cleanup(slirp);
+
 g_free(slirp->tftp_prefix);
 g_free(slirp->bootp_filename);
 g_free(slirp);
diff --git a/slirp/slirp.h b/slirp/slirp.h
index 950eccd..013a3b3 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -314,6 +314,7 @@ void if_output(struct socket *, struct mbuf *);
 
 /* ip_input.c */
 void ip_init(Slirp *);
+void ip_cleanup(Slirp *);
 void ip_input(struct mbuf *);
 void ip_slowtimo(Slirp *);
 void ip_stripoptions(register struct mbuf *, struct mbuf *);
@@ -331,6 +332,7 @@ void tcp_setpersist(register struct tcpcb *);
 
 /* tcp_subr.c */
 void tcp_init(Slirp *);
+void tcp_cleanup(Slirp *);
 void tcp_template(struct tcpcb *);
 void tcp_respond(struct tcpcb *, register struct tcpiphdr *, register struct 
mbuf *, tcp_seq, tcp_seq, int);
 struct tcpcb * tcp_newtcpcb(struct socket *);
diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c
index 143a238..6f6585a 100644
--- a/slirp/tcp_subr.c
+++ b/slirp/tcp_subr.c
@@ -55,6 +55,13 @@ tcp_init(Slirp *slirp)
 slirp->tcp_last_so = &slirp->tcb;
 }
 
+void tcp_cleanup(Slirp *slirp)
+{
+while (slirp->tcb.so_next != &slirp->tcb) {
+tcp_close(sototcpcb(slirp->tcb.so_next));
+}
+}
+
 /*
  * Create template to be used to send tcp packets on a connection.
  * Call after host entry created, fills
diff --git a/slirp/udp.c b/slirp/udp.c
index 5b060f3..ced5096 100644
--- a/slirp/udp.c
+++ b/slirp/udp.c
@@ -49,6 +49,14 @@ udp_init(Slirp *slirp)
 slirp->udb.so_next = slirp->udb.so_prev = &slirp->udb;
 slirp->udp_last_so = &slirp->udb;
 }
+
+void udp_cleanup(Slirp *slirp)
+{
+while (slirp->udb.so_next != &slirp->udb) {
+udp_detach(slirp->udb.so_next);
+}
+}
+
 /* m->m_data  points at ip packet header
  * m->m_len   length ip packe

[Qemu-devel] [PATCH v2 3/4] slirp: Remove unneeded if_queued

2012-03-02 Thread Jan Kiszka
There is now a trivial check on entry of if_start for pending packets,
so we can drop the additional tracking via if_queued.

Signed-off-by: Jan Kiszka 
---
 slirp/if.c|8 
 slirp/slirp.c |7 +--
 slirp/slirp.h |1 -
 3 files changed, 1 insertions(+), 15 deletions(-)

diff --git a/slirp/if.c b/slirp/if.c
index 6f59620..fee745a 100644
--- a/slirp/if.c
+++ b/slirp/if.c
@@ -110,8 +110,6 @@ if_output(struct socket *so, struct mbuf *ifm)
insque(ifm, ifq);
 
 diddit:
-   slirp->if_queued++;
-
if (so) {
/* Update *_queued */
so->so_queued++;
@@ -157,7 +155,6 @@ diddit:
 void if_start(Slirp *slirp)
 {
 uint64_t now = qemu_get_clock_ns(rt_clock);
-int requeued = 0;
 struct mbuf *ifm, *ifm_next, *ifqt;
 
 DEBUG_CALL("if_start");
@@ -189,12 +186,9 @@ void if_start(Slirp *slirp)
 ifm_next = NULL;
 }
 
-slirp->if_queued--;
-
 /* Try to send packet unless it already expired */
 if (ifm->expiration_date >= now && !if_encap(slirp, ifm)) {
 /* Packet is delayed due to pending ARP resolution */
-requeued++;
 continue;
 }
 
@@ -227,6 +221,4 @@ void if_start(Slirp *slirp)
 
 m_free(ifm);
 }
-
-slirp->if_queued = requeued;
 }
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 19d69eb..bcffc34 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -581,12 +581,7 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, 
fd_set *xfds,
 }
}
 
-   /*
-* See if we can start outputting
-*/
-   if (slirp->if_queued) {
-   if_start(slirp);
-   }
+if_start(slirp);
 }
 
/* clear global file descriptor sets.
diff --git a/slirp/slirp.h b/slirp/slirp.h
index 28a5c03..950eccd 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -235,7 +235,6 @@ struct Slirp {
 int mbuf_alloced;
 
 /* if states */
-int if_queued;  /* number of packets queued so far */
 struct mbuf if_fastq;   /* fast queue (for interactive data) */
 struct mbuf if_batchq;  /* queue for non-interactive data */
 struct mbuf *next_m;/* pointer to next mbuf to output */
-- 
1.7.3.4




[Qemu-devel] [PATCH 08/13] usb-ehci: Fix cerr tracking

2012-03-02 Thread Hans de Goede
cerr should only be decremented on errors which cause XactErr to be set, and
when that happens the failing transaction should be retried until cerr reaches
0 and only then should USBSTS_ERRINT be set (and inactive cleared and
USBSTS_INT set if requested).

Since we don't have any hardware level errors (and in case of redirection
the real hardware has already retried), re-trying makes no sense, so
immediately set cerr to 0 on errors which set XactErr.

Signed-off-by: Hans de Goede 
---
 hw/usb-ehci.c |   19 ++-
 1 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 507e4a8..2685adc 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -1291,7 +1291,7 @@ static void ehci_async_complete_packet(USBPort *port, 
USBPacket *packet)
 
 static void ehci_execute_complete(EHCIQueue *q)
 {
-int c_err, reload;
+int reload;
 
 assert(q->async != EHCI_ASYNC_INFLIGHT);
 q->async = EHCI_ASYNC_NONE;
@@ -1300,15 +1300,10 @@ static void ehci_execute_complete(EHCIQueue *q)
 q->qhaddr, q->qh.next, q->qtdaddr, q->usb_status);
 
 if (q->usb_status < 0) {
-err:
-/* TO-DO: put this is in a function that can be invoked below as well 
*/
-c_err = get_field(q->qh.token, QTD_TOKEN_CERR);
-c_err--;
-set_field(&q->qh.token, c_err, QTD_TOKEN_CERR);
-
 switch(q->usb_status) {
 case USB_RET_NODEV:
 q->qh.token |= (QTD_TOKEN_HALT | QTD_TOKEN_XACTERR);
+set_field(&q->qh.token, 0, QTD_TOKEN_CERR);
 ehci_record_interrupt(q->ehci, USBSTS_ERRINT);
 break;
 case USB_RET_STALL:
@@ -1336,15 +1331,13 @@ err:
 assert(0);
 break;
 }
+} else if ((q->usb_status > q->tbytes) && (q->pid == USB_TOKEN_IN)) {
+q->usb_status = USB_RET_BABBLE;
+q->qh.token |= (QTD_TOKEN_HALT | QTD_TOKEN_BABBLE);
+ehci_record_interrupt(q->ehci, USBSTS_ERRINT);
 } else {
-// DPRINTF("Short packet condition\n");
 // TODO check 4.12 for splits
 
-if ((q->usb_status > q->tbytes) && (q->pid == USB_TOKEN_IN)) {
-q->usb_status = USB_RET_BABBLE;
-goto err;
-}
-
 if (q->tbytes && q->pid == USB_TOKEN_IN) {
 q->tbytes -= q->usb_status;
 } else {
-- 
1.7.7.6




[Qemu-devel] [Bug 524447] Re: virsh save is very slow

2012-03-02 Thread Jeff Snider
I'll go stand up some vms to test that one out.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/524447

Title:
  virsh save is very slow

Status in libvirt virtualization API:
  Unknown
Status in QEMU:
  Fix Released
Status in “libvirt” package in Ubuntu:
  Invalid
Status in “qemu-kvm” package in Ubuntu:
  Fix Released
Status in “libvirt” source package in Lucid:
  Won't Fix
Status in “qemu-kvm” source package in Lucid:
  Fix Committed
Status in “libvirt” source package in Maverick:
  Won't Fix
Status in “qemu-kvm” source package in Maverick:
  Won't Fix
Status in “qemu-kvm” package in Debian:
  Fix Released

Bug description:
  ==
  SRU Justification:
  1. impact: 'qemu save' is slow
  2. how addressed: a patch upstream fixes the case when a file does not 
announce when it is ready.
  3. patch: see the patch in linked bzr trees
  4. TEST CASE: see comment #4 for a specific recipe
  5. regression potential:  this patch only touches the vm save path.
  ==

  As reported here: http://www.redhat.com/archives/libvir-
  list/2009-December/msg00203.html

  "virsh save" is very slow - it writes the image at around 1MB/sec on
  my test system.

  (I think I saw a bug report for this issue on Fedora's bugzilla, but I
  can't find it now...)

  Confirmed under Karmic.

To manage notifications about this bug go to:
https://bugs.launchpad.net/libvirt/+bug/524447/+subscriptions



[Qemu-devel] [PATCH 1/3] qcow2: Factor out count_cow_clusters

2012-03-02 Thread Kevin Wolf
Signed-off-by: Kevin Wolf 
---
 block/qcow2-cluster.c |   55 -
 1 files changed, 36 insertions(+), 19 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index a791bbe..903454d 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -677,6 +677,41 @@ err:
  }
 
 /*
+ * Returns the number of contiguous clusters that can be used for an allocating
+ * write, but require COW to be performed (this includes yet unallocated space,
+ * which must copy from the backing file)
+ */
+static int count_cow_clusters(BDRVQcowState *s, int nb_clusters,
+uint64_t *l2_table, int l2_index)
+{
+int i = 0;
+uint64_t cluster_offset;
+
+while (i < nb_clusters) {
+i += count_contiguous_clusters(nb_clusters - i, s->cluster_size,
+&l2_table[l2_index], i, 0);
+if ((i >= nb_clusters) || be64_to_cpu(l2_table[l2_index + i])) {
+break;
+}
+
+i += count_contiguous_free_clusters(nb_clusters - i,
+&l2_table[l2_index + i]);
+if (i >= nb_clusters) {
+break;
+}
+
+cluster_offset = be64_to_cpu(l2_table[l2_index + i]);
+
+if ((cluster_offset & QCOW_OFLAG_COPIED) ||
+(cluster_offset & QCOW_OFLAG_COMPRESSED))
+break;
+}
+
+assert(i <= nb_clusters);
+return i;
+}
+
+/*
  * alloc_cluster_offset
  *
  * For a given offset of the disk image, return cluster offset in qcow2 file.
@@ -739,25 +774,7 @@ again:
 
 /* how many available clusters ? */
 
-while (i < nb_clusters) {
-i += count_contiguous_clusters(nb_clusters - i, s->cluster_size,
-&l2_table[l2_index], i, 0);
-if ((i >= nb_clusters) || be64_to_cpu(l2_table[l2_index + i])) {
-break;
-}
-
-i += count_contiguous_free_clusters(nb_clusters - i,
-&l2_table[l2_index + i]);
-if (i >= nb_clusters) {
-break;
-}
-
-cluster_offset = be64_to_cpu(l2_table[l2_index + i]);
-
-if ((cluster_offset & QCOW_OFLAG_COPIED) ||
-(cluster_offset & QCOW_OFLAG_COMPRESSED))
-break;
-}
+i = count_cow_clusters(s, nb_clusters, l2_table, l2_index);
 assert(i <= nb_clusters);
 nb_clusters = i;
 
-- 
1.7.6.5




[Qemu-devel] [RFC PATCH 0/3] qcow2: Save requests during cluster allocation

2012-03-02 Thread Kevin Wolf
When a write request spans both allocated and unallocated clusters, qcow2
splits the request in two parts. This is not necessary if we have sequential
writes: If the unallocated area can be allocated such that in the image file it
is adjacent to the already allocated part, a single request is enough.

Kevin Wolf (3):
  qcow2: Factor out count_cow_clusters
  qcow2: Add qcow2_alloc_clusters_at()
  qcow2: Reduce number of I/O requests

 block/qcow2-cluster.c  |  274 +---
 block/qcow2-refcount.c |   28 +
 block/qcow2.h  |3 +
 trace-events   |1 +
 4 files changed, 222 insertions(+), 84 deletions(-)

-- 
1.7.6.5




[Qemu-devel] [PATCH 3/3] qcow2: Reduce number of I/O requests

2012-03-02 Thread Kevin Wolf
If the first part of a write request is allocated, but the second isn't
and it can be allocated so that the resulting area is contiguous, handle
it at once. This is a common case for sequential writes.

After this patch, alloc_cluster_offset() only checks if the clusters are
already allocated or how many new clusters can be allocated contigouosly.
The actual cluster allocation is split off into a new function
do_alloc_cluster_offset().

Signed-off-by: Kevin Wolf 
---
 block/qcow2-cluster.c |  243 +
 block/qcow2.h |1 +
 trace-events  |1 +
 3 files changed, 168 insertions(+), 77 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 903454d..e0fb907 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -589,7 +589,7 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, 
QCowL2Meta *m)
 BDRVQcowState *s = bs->opaque;
 int i, j = 0, l2_index, ret;
 uint64_t *old_cluster, start_sect, l2_offset, *l2_table;
-uint64_t cluster_offset = m->cluster_offset;
+uint64_t cluster_offset = m->alloc_offset;
 bool cow = false;
 
 trace_qcow2_cluster_link_l2(qemu_coroutine_self(), m->nb_clusters);
@@ -712,12 +712,94 @@ static int count_cow_clusters(BDRVQcowState *s, int 
nb_clusters,
 }
 
 /*
+ * Allocates new clusters for the given guest_offset.
+ *
+ * At most *nb_clusters are allocated, and on return *nb_clusters is updated to
+ * contain the number of clusters that have been allocated and are contiguous
+ * in the image file.
+ *
+ * If *host_offset is non-zero, it specifies the offset in the image file at
+ * which the new clusters must start. *nb_clusters can be 0 on return in this
+ * case if the cluster at host_offset is already in use. If *host_offset is
+ * zero, the clusters can be allocated anywhere in the image file.
+ *
+ * *host_offset is updated to contain the offset into the image file at which
+ * the first allocated cluster starts.
+ *
+ * Return 0 on success and -errno in error cases. -EAGAIN means that the
+ * function has been waiting for another request and the allocation must be
+ * restarted, but the whole request should not be failed.
+ */
+static int do_alloc_cluster_offset(BlockDriverState *bs, uint64_t guest_offset,
+uint64_t *host_offset, unsigned int *nb_clusters, uint64_t *l2_table)
+{
+BDRVQcowState *s = bs->opaque;
+int64_t cluster_offset;
+QCowL2Meta *old_alloc;
+
+trace_qcow2_do_alloc_clusters_offset(qemu_coroutine_self(), guest_offset,
+ *host_offset, *nb_clusters);
+
+/*
+ * Check if there already is an AIO write request in flight which allocates
+ * the same cluster. In this case we need to wait until the previous
+ * request has completed and updated the L2 table accordingly.
+ */
+QLIST_FOREACH(old_alloc, &s->cluster_allocs, next_in_flight) {
+
+uint64_t start = guest_offset >> s->cluster_bits;
+uint64_t end = start + *nb_clusters;
+uint64_t old_start = old_alloc->offset >> s->cluster_bits;
+uint64_t old_end = old_start + old_alloc->nb_clusters;
+
+if (end < old_start || start > old_end) {
+/* No intersection */
+} else {
+if (start < old_start) {
+/* Stop at the start of a running allocation */
+*nb_clusters = old_start - start;
+} else {
+*nb_clusters = 0;
+}
+
+if (*nb_clusters == 0) {
+/* Wait for the dependency to complete. We need to recheck
+ * the free/allocated clusters when we continue. */
+qemu_co_mutex_unlock(&s->lock);
+qemu_co_queue_wait(&old_alloc->dependent_requests);
+qemu_co_mutex_lock(&s->lock);
+return -EAGAIN;
+}
+}
+}
+
+if (!*nb_clusters) {
+abort();
+}
+
+/* Allocate new clusters */
+trace_qcow2_cluster_alloc_phys(qemu_coroutine_self());
+if (*host_offset == 0) {
+cluster_offset = qcow2_alloc_clusters(bs, *nb_clusters * 
s->cluster_size);
+} else {
+cluster_offset = *host_offset;
+*nb_clusters = qcow2_alloc_clusters_at(bs, cluster_offset, 
*nb_clusters);
+}
+
+if (cluster_offset < 0) {
+return cluster_offset;
+}
+*host_offset = cluster_offset;
+return 0;
+}
+
+/*
  * alloc_cluster_offset
  *
- * For a given offset of the disk image, return cluster offset in qcow2 file.
- * If the offset is not found, allocate a new cluster.
+ * For a given offset on the virtual disk, find the cluster offset in qcow2
+ * file. If the offset is not found, allocate a new cluster.
  *
- * If the cluster was already allocated, m->nb_clusters is set to 0,
+ * If the cluster was already allocated, m->nb_clusters is set to 0 and
  * other fields in m are meaningless.
  *
  * If the cluster is newly allocated, m->nb_

[Qemu-devel] [PATCH 2/3] qcow2: Add qcow2_alloc_clusters_at()

2012-03-02 Thread Kevin Wolf
This function allows to allocate clusters at a given offset in the image
file. This is useful if you want to allocate the second part of an area
that must be contiguous.

Signed-off-by: Kevin Wolf 
---
 block/qcow2-refcount.c |   28 
 block/qcow2.h  |2 ++
 2 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 2db2ede..f39928a 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -582,6 +582,34 @@ int64_t qcow2_alloc_clusters(BlockDriverState *bs, int64_t 
size)
 return offset;
 }
 
+int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset,
+int nb_clusters)
+{
+BDRVQcowState *s = bs->opaque;
+uint64_t cluster_index;
+int i, refcount, ret;
+
+/* Check how many clusters there are free */
+cluster_index = offset >> s->cluster_bits;
+for(i = 0; i < nb_clusters; i++) {
+refcount = get_refcount(bs, cluster_index++);
+
+if (refcount < 0) {
+return refcount;
+} else if (refcount != 0) {
+break;
+}
+}
+
+/* And then allocate them */
+ret = update_refcount(bs, offset, i << s->cluster_bits, 1);
+if (ret < 0) {
+return ret;
+}
+
+return i;
+}
+
 /* only used to allocate compressed sectors. We try to allocate
contiguous sectors. size must be <= cluster_size */
 int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
diff --git a/block/qcow2.h b/block/qcow2.h
index fc35838..5129e3e 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -193,6 +193,8 @@ int qcow2_refcount_init(BlockDriverState *bs);
 void qcow2_refcount_close(BlockDriverState *bs);
 
 int64_t qcow2_alloc_clusters(BlockDriverState *bs, int64_t size);
+int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset,
+int nb_clusters);
 int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size);
 void qcow2_free_clusters(BlockDriverState *bs,
 int64_t offset, int64_t size);
-- 
1.7.6.5




[Qemu-devel] [PATCH 13/13] usb: add USB_RET_IOERROR

2012-03-02 Thread Hans de Goede
We already have USB_RET_NAK, but that means that a device does not want
to send/receive right now. But with host / network redirection we can
actually have a transaction fail due to some io error, rather then ie
the device just not having any data atm.

This patch adds a new error code named USB_RET_IOERROR for this, and uses
it were appropriate.

Notes:
-Currently all usb-controllers handle this the same as NODEV, but that
 may change in the future, OHCI could indicate a CRC error instead for example.
-This patch does not touch hw/usb-musb.c, that is because the code in there
 handles STALL and NAK specially and has a if status < 0 generic catch all
 for all other errors

Signed-off-by: Hans de Goede 
---
 hw/usb-ehci.c |2 ++
 hw/usb-ohci.c |2 ++
 hw/usb-uhci.c |1 +
 hw/usb.h  |   11 ++-
 usb-linux.c   |4 ++--
 usb-redir.c   |9 ++---
 6 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 825fcc0..df742f7 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -1299,6 +1299,7 @@ static void ehci_execute_complete(EHCIQueue *q)
 
 if (q->usb_status < 0) {
 switch(q->usb_status) {
+case USB_RET_IOERROR:
 case USB_RET_NODEV:
 q->qh.token |= (QTD_TOKEN_HALT | QTD_TOKEN_XACTERR);
 set_field(&q->qh.token, 0, QTD_TOKEN_CERR);
@@ -1471,6 +1472,7 @@ static int ehci_process_itd(EHCIState *ehci,
 default:
 fprintf(stderr, "Unexpected iso usb result: %d\n", ret);
 /* Fall through */
+case USB_RET_IOERROR:
 case USB_RET_NODEV:
 /* 3.3.2: XACTERR is only allowed on IN transactions */
 if (dir) {
diff --git a/hw/usb-ohci.c b/hw/usb-ohci.c
index 7aa19fe..20aaa74 100644
--- a/hw/usb-ohci.c
+++ b/hw/usb-ohci.c
@@ -837,6 +837,7 @@ static int ohci_service_iso_td(OHCIState *ohci, struct 
ohci_ed *ed,
 OHCI_CC_DATAUNDERRUN);
 } else {
 switch (ret) {
+case USB_RET_IOERROR:
 case USB_RET_NODEV:
 OHCI_SET_BM(iso_td.offset[relative_frame_number], TD_PSW_CC,
 OHCI_CC_DEVICENOTRESPONDING);
@@ -1052,6 +1053,7 @@ static int ohci_service_td(OHCIState *ohci, struct 
ohci_ed *ed)
 OHCI_SET_BM(td.flags, TD_CC, OHCI_CC_DATAUNDERRUN);
 } else {
 switch (ret) {
+case USB_RET_IOERROR:
 case USB_RET_NODEV:
 OHCI_SET_BM(td.flags, TD_CC, OHCI_CC_DEVICENOTRESPONDING);
 case USB_RET_NAK:
diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
index 70e3881..2c6ed38 100644
--- a/hw/usb-uhci.c
+++ b/hw/usb-uhci.c
@@ -765,6 +765,7 @@ out:
 break;
return 1;
 
+case USB_RET_IOERROR:
 case USB_RET_NODEV:
 default:
break;
diff --git a/hw/usb.h b/hw/usb.h
index 8e83697..1a30ebb 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -39,11 +39,12 @@
 #define USB_TOKEN_IN0x69 /* device -> host */
 #define USB_TOKEN_OUT   0xe1 /* host -> device */
 
-#define USB_RET_NODEV  (-1)
-#define USB_RET_NAK(-2)
-#define USB_RET_STALL  (-3)
-#define USB_RET_BABBLE (-4)
-#define USB_RET_ASYNC  (-5)
+#define USB_RET_NODEV   (-1)
+#define USB_RET_NAK (-2)
+#define USB_RET_STALL   (-3)
+#define USB_RET_BABBLE  (-4)
+#define USB_RET_IOERROR (-5)
+#define USB_RET_ASYNC   (-6)
 
 #define USB_SPEED_LOW   0
 #define USB_SPEED_FULL  1
diff --git a/usb-linux.c b/usb-linux.c
index 38df9e6..050ea7a 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -369,7 +369,7 @@ static void async_complete(void *opaque)
 break;
 
 default:
-p->result = USB_RET_NAK;
+p->result = USB_RET_IOERROR;
 break;
 }
 
@@ -729,7 +729,7 @@ static int urb_status_to_usb_ret(int status)
 case -EOVERFLOW:
 return USB_RET_BABBLE;
 default:
-return USB_RET_NAK;
+return USB_RET_IOERROR;
 }
 }
 
diff --git a/usb-redir.c b/usb-redir.c
index 4c8e3e3..59e9327 100644
--- a/usb-redir.c
+++ b/usb-redir.c
@@ -441,7 +441,7 @@ static int usbredir_handle_iso_data(USBRedirDevice *dev, 
USBPacket *p,
 /* Check iso_error for stream errors, otherwise its an underrun */
 status = dev->endpoint[EP2I(ep)].iso_error;
 dev->endpoint[EP2I(ep)].iso_error = 0;
-return status ? USB_RET_NAK : 0;
+return status ? USB_RET_IOERROR : 0;
 }
 DPRINTF2("iso-token-in ep %02X status %d len %d queue-size: %d\n", ep,
  isop->status, isop->len, dev->endpoint[EP2I(ep)].bufpq_size);
@@ -449,7 +449,7 @@ static int usbredir_handle_iso_data(USBRedirDevice *dev, 
USBPacket *p,
 status = isop->status;
 if (status != usb_redir_success) {
 bufp_free(dev, isop, ep);
-return USB_RET_NAK;
+return USB_RET_IOERROR;
 }
 
 len

[Qemu-devel] [PATCH 06/13] usb-ehci: Rip the queues when the async or period schedule is halted

2012-03-02 Thread Hans de Goede
Signed-off-by: Hans de Goede 
---
 hw/usb-ehci.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index b349003..d386b84 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -1076,7 +1076,8 @@ static void ehci_mem_writel(void *ptr, target_phys_addr_t 
addr, uint32_t val)
 
 if (!(val & USBCMD_RUNSTOP) && (s->usbcmd & USBCMD_RUNSTOP)) {
 qemu_del_timer(s->frame_timer);
-// TODO - should finish out some stuff before setting halt
+ehci_queues_rip_all(s, 0);
+ehci_queues_rip_all(s, 1);
 ehci_set_usbsts(s, USBSTS_HALT);
 }
 
@@ -2088,6 +2089,7 @@ static void ehci_advance_async_state(EHCIState *ehci)
 
 case EST_ACTIVE:
 if ( !(ehci->usbcmd & USBCMD_ASE)) {
+ehci_queues_rip_all(ehci, async);
 ehci_clear_usbsts(ehci, USBSTS_ASS);
 ehci_set_state(ehci, async, EST_INACTIVE);
 break;
@@ -2148,6 +2150,7 @@ static void ehci_advance_periodic_state(EHCIState *ehci)
 
 case EST_ACTIVE:
 if ( !(ehci->frindex & 7) && !(ehci->usbcmd & USBCMD_PSE)) {
+ehci_queues_rip_all(ehci, async);
 ehci_clear_usbsts(ehci, USBSTS_PSS);
 ehci_set_state(ehci, async, EST_INACTIVE);
 break;
-- 
1.7.7.6




[Qemu-devel] [PATCH 10/13] usb-ehci: Fix and simplify nakcnt handling

2012-03-02 Thread Hans de Goede
The nakcnt code in ehci_execute_complete() marked transactions as finished
when a packet completed with a result of USB_RET_NAK, but USB_RET_NAK
means that the device cannot receive / send data at that time and that
the transaction should be retried later, which is also what the usb-uhci
and usb-ohci code does.

Note that there already was some special code in place to handle this
for interrupt endpoints in the form of doing a return from
ehci_execute_complete() when reload == 0, but that for bulk transactions
this was not handled correctly (where as for example the usb-ccid device does
return USB_RET_NAK for bulk packets).

Besides that the code in ehci_execute_complete() decrement nakcnt by 1
on a packet result of USB_RET_NAK, but
-since the transaction got marked as finished,
 nakcnt would never be decremented again
-there is no code checking for nakcnt becoming 0
-there is no use in re-trying the transaction within the same usb frame /
 usb-ehci frame-timer call, since the status of emulated devices won't change
 as long as the usb-ehci frame-timer is running
So we should simply set the nakcnt to 0 when we get a USB_RET_NAK, thus
claiming that we've tried reload times (or as many times as possible if
reload is 0).

Besides the code in ehci_execute_complete() handling USB_RET_NAK there
was also code handling it in ehci_state_executing(), which calls
ehci_execute_complete(), and then does its own handling on top of the handling
in ehci_execute_complete(), this code would decrement nakcnt *again* (if not
already 0), or restore the reload value (which was never changed) on success.

Since the double decrement was wrong to begin with, and is no longer needed
now that we set nakcnt directly to 0 on USB_RET_NAK, and the restore of reload
is not needed either, this patch simply removes all nakcnt handling from
ehci_state_executing().

Signed-off-by: Hans de Goede 
---
 hw/usb-ehci.c |   32 
 1 files changed, 4 insertions(+), 28 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 07bcd1f..9197298 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -1291,8 +1291,6 @@ static void ehci_async_complete_packet(USBPort *port, 
USBPacket *packet)
 
 static void ehci_execute_complete(EHCIQueue *q)
 {
-int reload;
-
 assert(q->async != EHCI_ASYNC_INFLIGHT);
 q->async = EHCI_ASYNC_NONE;
 
@@ -1311,16 +1309,8 @@ static void ehci_execute_complete(EHCIQueue *q)
 ehci_record_interrupt(q->ehci, USBSTS_ERRINT);
 break;
 case USB_RET_NAK:
-/* 4.10.3 */
-reload = get_field(q->qh.epchar, QH_EPCHAR_RL);
-if ((q->pid == USB_TOKEN_IN) && reload) {
-int nakcnt = get_field(q->qh.altnext_qtd, QH_ALTNEXT_NAKCNT);
-nakcnt--;
-set_field(&q->qh.altnext_qtd, nakcnt, QH_ALTNEXT_NAKCNT);
-} else if (!reload) {
-return;
-}
-break;
+set_field(&q->qh.altnext_qtd, 0, QH_ALTNEXT_NAKCNT);
+return; /* We're not done yet with this transaction */
 case USB_RET_BABBLE:
 q->qh.token |= (QTD_TOKEN_HALT | QTD_TOKEN_BABBLE);
 ehci_record_interrupt(q->ehci, USBSTS_ERRINT);
@@ -1353,7 +1343,7 @@ static void ehci_execute_complete(EHCIQueue *q)
 q->qh.token ^= QTD_TOKEN_DTOGGLE;
 q->qh.token &= ~QTD_TOKEN_ACTIVE;
 
-if ((q->usb_status != USB_RET_NAK) && (q->qh.token & QTD_TOKEN_IOC)) {
+if (q->qh.token & QTD_TOKEN_IOC) {
 ehci_record_interrupt(q->ehci, USBSTS_INT);
 }
 }
@@ -1877,7 +1867,6 @@ out:
 static int ehci_state_executing(EHCIQueue *q, int async)
 {
 int again = 0;
-int reload, nakcnt;
 
 ehci_execute_complete(q);
 if (q->usb_status == USB_RET_ASYNC) {
@@ -1897,21 +1886,8 @@ static int ehci_state_executing(EHCIQueue *q, int async)
 // counter decrements to 0
 }
 
-reload = get_field(q->qh.epchar, QH_EPCHAR_RL);
-if (reload) {
-nakcnt = get_field(q->qh.altnext_qtd, QH_ALTNEXT_NAKCNT);
-if (q->usb_status == USB_RET_NAK) {
-if (nakcnt) {
-nakcnt--;
-}
-} else {
-nakcnt = reload;
-}
-set_field(&q->qh.altnext_qtd, nakcnt, QH_ALTNEXT_NAKCNT);
-}
-
 /* 4.10.5 */
-if ((q->usb_status == USB_RET_NAK) || (q->qh.token & QTD_TOKEN_ACTIVE)) {
+if (q->usb_status == USB_RET_NAK) {
 ehci_set_state(q->ehci, async, EST_HORIZONTALQH);
 } else {
 ehci_set_state(q->ehci, async, EST_WRITEBACK);
-- 
1.7.7.6




Re: [Qemu-devel] Add support for new image type

2012-03-02 Thread Kai Meyer


From: Paolo Bonzini [pbonz...@redhat.com]
Sent: Thursday, March 01, 2012 11:54 PM
To: Kai Meyer
Cc: Anthony Liguori; Stefan Weil; qemu-devel@nongnu.org; Nate Bushman
Subject: Re: Add support for new image type

Il 01/03/2012 22:14, Kai Meyer ha scritto:
> If we can't use qemu in general use-cases (since we can't push support
> for our images up stream), we could still really benefit from a targeted
> use-cases (like a Rescue Environment.)

It does not matter whether it is upstream or not.

When you distribute your modified QEMU binary, anyone who receives it
has the right to ask you for the complete corresponding source code.

I also suggest that you write a wrapper around your library that exports
the contents as iSCSI or NBD.

Paolo


Well, yes. I was assuming that there was potential for us to be able to 
distribute qemu modifications that would not require us opening up our library. 
The more we look at it, and some past precedence we've experienced, it looks 
like it's not going to happen.

I think I agree that iSCSI is really our only option (legally). Too bad. The 
code was so simple to integrate directly into qemu. I have to give you guys 
kudos again for already having such a simple integration point.

-Kai Meyer


[Qemu-devel] [PATCH 02/13] usb-ehci: Never follow table entries with the T-bit set

2012-03-02 Thread Hans de Goede
Before this patch the T-bit was not checked in 2 places, while it should be.

Once we properly check the T-bit everywhere we no longer need the weird
entry < 0x1000 and entry > 0x1000 checks, so this patch removes them.

Signed-off-by: Hans de Goede 
---
 hw/usb-ehci.c |   10 --
 1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index afc8ccf..d41b80e 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -1568,8 +1568,7 @@ static int ehci_state_fetchentry(EHCIState *ehci, int 
async)
 int again = 0;
 uint32_t entry = ehci_get_fetch_addr(ehci, async);
 
-if (entry < 0x1000) {
-DPRINTF("fetchentry: entry invalid (0x%08x)\n", entry);
+if (NLPTR_TBIT(entry)) {
 ehci_set_state(ehci, async, EST_ACTIVE);
 goto out;
 }
@@ -1677,7 +1676,8 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int 
async)
 if (q->qh.token & QTD_TOKEN_HALT) {
 ehci_set_state(ehci, async, EST_HORIZONTALQH);
 
-} else if ((q->qh.token & QTD_TOKEN_ACTIVE) && (q->qh.current_qtd > 
0x1000)) {
+} else if ((q->qh.token & QTD_TOKEN_ACTIVE) &&
+   (NLPTR_TBIT(q->qh.current_qtd) == 0)) {
 q->qtdaddr = q->qh.current_qtd;
 ehci_set_state(ehci, async, EST_FETCHQTD);
 
@@ -1756,7 +1756,6 @@ static int ehci_state_advqueue(EHCIQueue *q, int async)
  * want data and alt-next qTD is valid
  */
 if (((q->qh.token & QTD_TOKEN_TBYTES_MASK) != 0) &&
-(q->qh.altnext_qtd > 0x1000) &&
 (NLPTR_TBIT(q->qh.altnext_qtd) == 0)) {
 q->qtdaddr = q->qh.altnext_qtd;
 ehci_set_state(q->ehci, async, EST_FETCHQTD);
@@ -1764,8 +1763,7 @@ static int ehci_state_advqueue(EHCIQueue *q, int async)
 /*
  *  next qTD is valid
  */
-} else if ((q->qh.next_qtd > 0x1000) &&
-   (NLPTR_TBIT(q->qh.next_qtd) == 0)) {
+} else if (NLPTR_TBIT(q->qh.next_qtd) == 0) {
 q->qtdaddr = q->qh.next_qtd;
 ehci_set_state(q->ehci, async, EST_FETCHQTD);
 
-- 
1.7.7.6




[Qemu-devel] [PATCH 07/13] usb-ehci: Any packet completion except for NAK should set the interrupt

2012-03-02 Thread Hans de Goede
As clearly stated in the 2.3.2 of the EHCI spec, any time USBERRINT get
sets then if the td has its IOC bit set USBINT should be set as well.

This means that for any status except for USB_RET_NAK we should set
USBINT if the IOC bit is set.

Signed-off-by: Hans de Goede 
---
 hw/usb-ehci.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index d386b84..507e4a8 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -1360,7 +1360,7 @@ err:
 q->qh.token ^= QTD_TOKEN_DTOGGLE;
 q->qh.token &= ~QTD_TOKEN_ACTIVE;
 
-if ((q->usb_status >= 0) && (q->qh.token & QTD_TOKEN_IOC)) {
+if ((q->usb_status != USB_RET_NAK) && (q->qh.token & QTD_TOKEN_IOC)) {
 ehci_record_interrupt(q->ehci, USBSTS_INT);
 }
 }
-- 
1.7.7.6




[Qemu-devel] [PATCH 0/13] usb patches

2012-03-02 Thread Hans de Goede
Hi,

Here is a series of usb patches against current qemu master, mostly
consisting of a whole bunch of small ehci fixes, plus some redirection
fixes.

Regards,

Hans



[Qemu-devel] [PATCH 01/13] usb-redir: Set ep type and interface

2012-03-02 Thread Hans de Goede
Since we don't use usb_desc.c we need to do this ourselves. This fixes
iso transfers no longer working for USB 2 devices due to the ep->type
check in ehci.c

Signed-off-by: Hans de Goede 
---
 usb-redir.c |8 
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/usb-redir.c b/usb-redir.c
index fc58a0e..ef9fa96 100644
--- a/usb-redir.c
+++ b/usb-redir.c
@@ -1149,6 +1149,7 @@ static void usbredir_device_disconnect(void *priv)
 for (i = 0; i < MAX_ENDPOINTS; i++) {
 QTAILQ_INIT(&dev->endpoint[i].bufpq);
 }
+usb_ep_init(&dev->dev);
 dev->interface_info.interface_count = 0;
 }
 
@@ -1175,6 +1176,7 @@ static void usbredir_ep_info(void *priv,
 struct usb_redir_ep_info_header *ep_info)
 {
 USBRedirDevice *dev = priv;
+struct USBEndpoint *usb_ep;
 int i;
 
 for (i = 0; i < MAX_ENDPOINTS; i++) {
@@ -1199,7 +1201,13 @@ static void usbredir_ep_info(void *priv,
 default:
 ERROR("Received invalid endpoint type\n");
 usbredir_device_disconnect(dev);
+return;
 }
+usb_ep = usb_ep_get(&dev->dev,
+(i & 0x10) ? USB_TOKEN_IN : USB_TOKEN_OUT,
+i & 0x0f);
+usb_ep->type = dev->endpoint[i].type;
+usb_ep->ifnum = dev->endpoint[i].interface;
 }
 }
 
-- 
1.7.7.6




[Qemu-devel] [Bug 524447] Re: virsh save is very slow

2012-03-02 Thread Clint Byrum
Ben, yes, sorry I missed the fact that there was already another bug
that needs verification in lucid-proposed. Bug #592010 needs to be
verified, or reverted, before this one can proceed to lucid-updates.
Verification is tricky, since one needs to do a hardy -> lucid upgrade
to verify it.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/524447

Title:
  virsh save is very slow

Status in libvirt virtualization API:
  Unknown
Status in QEMU:
  Fix Released
Status in “libvirt” package in Ubuntu:
  Invalid
Status in “qemu-kvm” package in Ubuntu:
  Fix Released
Status in “libvirt” source package in Lucid:
  Won't Fix
Status in “qemu-kvm” source package in Lucid:
  Fix Committed
Status in “libvirt” source package in Maverick:
  Won't Fix
Status in “qemu-kvm” source package in Maverick:
  Won't Fix
Status in “qemu-kvm” package in Debian:
  Fix Released

Bug description:
  ==
  SRU Justification:
  1. impact: 'qemu save' is slow
  2. how addressed: a patch upstream fixes the case when a file does not 
announce when it is ready.
  3. patch: see the patch in linked bzr trees
  4. TEST CASE: see comment #4 for a specific recipe
  5. regression potential:  this patch only touches the vm save path.
  ==

  As reported here: http://www.redhat.com/archives/libvir-
  list/2009-December/msg00203.html

  "virsh save" is very slow - it writes the image at around 1MB/sec on
  my test system.

  (I think I saw a bug report for this issue on Fedora's bugzilla, but I
  can't find it now...)

  Confirmed under Karmic.

To manage notifications about this bug go to:
https://bugs.launchpad.net/libvirt/+bug/524447/+subscriptions



Re: [Qemu-devel] Has anybody run Vxworks on KVM?

2012-03-02 Thread Brian Vandenberg
On Fri, Mar 2, 2012 at 9:10 AM, GaoYi  wrote:
> Hi all,
>
>I am trying to run Vxworks on KVM under X86_64 platform. Now I can
> startup vxworks without 'no-kvm' option. However, the bootup failed with
> hardware virtualization selected. Has anybody run it successfully?
>
>Any of your suggestions would be appreicated.
>
> Yi

Yi,

  First, sorry for the duplicate message to yourself; I didn't realize
it wasn't being sent to the mailing list.

  I don't know about KVM in particular, but Bill Paul (from Windriver)
got vxworks to run under qemu x86:

http://lists.gnu.org/archive/html/qemu-devel/2009-06/msg00401.html

-Brian



Re: [Qemu-devel] ARM brk bug

2012-03-02 Thread Peter Maydell
On 27 February 2012 15:16, Bernhard M. Wiedemann  wrote:
> I found that running a debian arm5 bash with qemu runs into varying
> problems with -R but works without.

So I had a look at this this afternoon, and what seems to be happening
is that with -R, the call to target_mmap() in elfload.c:setup_arg_pages()
(which creates the stack) is putting the stack immediately after the
bash BSS segment in the address space. This means that brk() will
never be able to expand, and it looks like something in either bash
or libc's locale code isn't correctly handling the failure, so we
crash. (The segfault is from a strlen(NULL) from setlocale() I think.)

We should probably try to put the stack somewhere more sensible than
where it currently ends up...

-- PMM



[Qemu-devel] [Bug 524447] Re: virsh save is very slow

2012-03-02 Thread BenLake
Is something holding up the release to lucid-updates?

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/524447

Title:
  virsh save is very slow

Status in libvirt virtualization API:
  Unknown
Status in QEMU:
  Fix Released
Status in “libvirt” package in Ubuntu:
  Invalid
Status in “qemu-kvm” package in Ubuntu:
  Fix Released
Status in “libvirt” source package in Lucid:
  Won't Fix
Status in “qemu-kvm” source package in Lucid:
  Fix Committed
Status in “libvirt” source package in Maverick:
  Won't Fix
Status in “qemu-kvm” source package in Maverick:
  Won't Fix
Status in “qemu-kvm” package in Debian:
  Fix Released

Bug description:
  ==
  SRU Justification:
  1. impact: 'qemu save' is slow
  2. how addressed: a patch upstream fixes the case when a file does not 
announce when it is ready.
  3. patch: see the patch in linked bzr trees
  4. TEST CASE: see comment #4 for a specific recipe
  5. regression potential:  this patch only touches the vm save path.
  ==

  As reported here: http://www.redhat.com/archives/libvir-
  list/2009-December/msg00203.html

  "virsh save" is very slow - it writes the image at around 1MB/sec on
  my test system.

  (I think I saw a bug report for this issue on Fedora's bugzilla, but I
  can't find it now...)

  Confirmed under Karmic.

To manage notifications about this bug go to:
https://bugs.launchpad.net/libvirt/+bug/524447/+subscriptions



Re: [Qemu-devel] IRQ number, interrupt number, interrupt line & GPIO[in/out]

2012-03-02 Thread Peter Maydell
On 2 March 2012 16:01, Anthony Liguori  wrote:
> On 03/02/2012 06:38 AM, Zhi Yong Wu wrote:
>> Can anyone explain their relationship and difference among them?  It
>> is very appreciated if you can make some comments. thanks.
>
>
> IRQ == interrupt.
>
> GPIO is just another name for an input or output pin on a chip which could
> be a IRQ line.

The other point to note here is that there are two different sets
of terminology:
(1) real world terminology, which is roughly what Anthony is describing
(2) QEMU qdev and sysbus terms, which don't necessarily always map
quite cleanly to (1)

In particular the QEMU type 'qemu_irq' is really just an arbitrary signal
(and would be better named GPIO) -- it is not always used for an
actual interrupt line.

-- PMM



Re: [Qemu-devel] Has anybody run Vxworks on KVM?

2012-03-02 Thread Jan Kiszka
On 2012-03-02 17:10, GaoYi wrote:
> Hi all,
> 
>I am trying to run Vxworks on KVM under X86_64 platform. Now I can
> startup vxworks without 'no-kvm' option. However, the bootup failed with
> hardware virtualization selected. Has anybody run it successfully?
> 
>Any of your suggestions would be appreicated.

According to your private reports, this is most probably a KVM issue of
its x86 emulator. So please post the complete error message qemu[-kvm]
dumps to the console.

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH] spice: set spice uuid and name

2012-03-02 Thread Alon Levy
On Fri, Mar 02, 2012 at 11:15:48AM -0500, Marc-André Lureau wrote:
> 
> 
> - Mensaje original -
> > On Fri, Mar 02, 2012 at 01:49:22PM +0100, Marc-André Lureau wrote:
> > > This allows a Spice client to identify a VM
> >
> > My only problem with this is that if we have a monitor vmcchannel you
> > could issue the command to query that, and much more, without having
> > to
> > add any messages. And adding a monitor channel is really easy - the
> > only
> > requirement being that qemu can handle two monitor users, libvirt and
> > spice.
> 
> Interesting idea, then we would have a Spice "qemu monitor" channel,
> and we would need to do the same job as libvirt-qemu to have a stable
> layer on top. Arguably, we could share their code, but that doesn't seem
> trivial either.

True. It would be simpler if there was a libqemumonitor inside qemu that
libvirt and us used.

> 
> Both approach do not seem incompatible to me.Having a uuid/name Spice API
> can be useful for XSpice or other servers too.
True. Then it makes sense to add this.




Re: [Qemu-devel] [PATCH] spice: set spice uuid and name

2012-03-02 Thread Marc-André Lureau


- Mensaje original -
> On Fri, Mar 02, 2012 at 01:49:22PM +0100, Marc-André Lureau wrote:
> > This allows a Spice client to identify a VM
>
> My only problem with this is that if we have a monitor vmcchannel you
> could issue the command to query that, and much more, without having
> to
> add any messages. And adding a monitor channel is really easy - the
> only
> requirement being that qemu can handle two monitor users, libvirt and
> spice.

Interesting idea, then we would have a Spice "qemu monitor" channel,
and we would need to do the same job as libvirt-qemu to have a stable
layer on top. Arguably, we could share their code, but that doesn't seem
trivial either.

Both approach do not seem incompatible to me.Having a uuid/name Spice API
can be useful for XSpice or other servers too.



[Qemu-devel] Has anybody run Vxworks on KVM?

2012-03-02 Thread GaoYi
Hi all,

   I am trying to run Vxworks on KVM under X86_64 platform. Now I can
startup vxworks without 'no-kvm' option. However, the bootup failed with
hardware virtualization selected. Has anybody run it successfully?

   Any of your suggestions would be appreicated.

Yi


Re: [Qemu-devel] [PATCH] spice: set spice uuid and name

2012-03-02 Thread Alon Levy
On Fri, Mar 02, 2012 at 01:49:22PM +0100, Marc-André Lureau wrote:
> This allows a Spice client to identify a VM

My only problem with this is that if we have a monitor vmcchannel you
could issue the command to query that, and much more, without having to
add any messages. And adding a monitor channel is really easy - the only
requirement being that qemu can handle two monitor users, libvirt and
spice.

> ---
>  ui/spice-core.c |6 ++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index 1308a3d..8472cdd 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -19,6 +19,7 @@
>  #include 
>  
>  #include 
> +#include "sysemu.h"
>  
>  #include "qemu-common.h"
>  #include "qemu-spice.h"
> @@ -699,6 +700,11 @@ void qemu_spice_init(void)
>  
>  qemu_opt_foreach(opts, add_channel, NULL, 0);
>  
> +#if SPICE_SERVER_VERSION >= 0x000a02 /* 0.10.2 */
> +spice_server_set_name(spice_server, qemu_name);
> +spice_server_set_uuid(spice_server, qemu_uuid);
> +#endif
> +
>  if (0 != spice_server_init(spice_server, &core_interface)) {
>  fprintf(stderr, "failed to initialize spice server");
>  exit(1);
> -- 
> 1.7.7.6
> 
> 



Re: [Qemu-devel] IRQ number, interrupt number, interrupt line & GPIO[in/out]

2012-03-02 Thread Anthony Liguori

Hi Zhi Yong,

On 03/02/2012 06:38 AM, Zhi Yong Wu wrote:

HI,

Can anyone explain their relationship and difference among them?  It
is very appreciated if you can make some comments. thanks.


IRQ == interrupt.

GPIO is just another name for an input or output pin on a chip which could be a 
IRQ line.


Interrupt controllers can receive interrupts from one or more devices.  Usually, 
the input pins on an interrupt controller can be numbered sequentially.  When we 
say that the first UART is on IRQ number 3, what that really means is that the 
IRQ output pin on the UART chip is connected to pin number 3 on the interrupt 
controller with a wire.


But there never is a single interrupt controller in a real system.  For 
instance, a PCI bus has it's own interrupt controller that has four input pins 
(called LNKs) that are oddly labeled A, B, C, D.


For the I440FX PCI bus, those four input pins are mapped to two IRQs which are 
then connected to the I/O APIC.


Regards,

Anthony Liguori



[Qemu-devel] [PATCH] libcacard: Fix compilation with gcc-4.7

2012-03-02 Thread Hans de Goede
VCARD_ATR_PREFIX is used as part of an array initializer so it should
not have () around it, so far this happened to work, but gcc-4.7 does
not like it.

Signed-off-by: Hans de Goede 
---
 libcacard/vcardt.h |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libcacard/vcardt.h b/libcacard/vcardt.h
index d4d8e2e..d3e9522 100644
--- a/libcacard/vcardt.h
+++ b/libcacard/vcardt.h
@@ -26,8 +26,8 @@ typedef struct VCardEmulStruct VCardEmul;
 #define MAX_CHANNEL 4
 
 /* create an ATR with appropriate historical bytes */
-#define VCARD_ATR_PREFIX(size) (0x3b, 0x68+(size), 0x00, 0xff, \
-   'V', 'C', 'A', 'R', 'D', '_')
+#define VCARD_ATR_PREFIX(size) 0x3b, 0x68+(size), 0x00, 0xff, \
+   'V', 'C', 'A', 'R', 'D', '_'
 
 
 typedef enum {
-- 
1.7.7.6




Re: [Qemu-devel] unknown keycodes 'macintosh_aliases(qwertz)'

2012-03-02 Thread Daniel P. Berrange
On Fri, Mar 02, 2012 at 12:21:49PM +0100, Jörg Sommer wrote:
> Hi,
> 
> as requested by the qemu software, here's my report that qemu says
> 
> unknown keycodes `macintosh_aliases(qwertz)', please report to 
> qemu-devel@nongnu.org


Thanks for the information, I have posted patches a few weeks back
which should fix this problem:

  https://lists.gnu.org/archive/html/qemu-devel/2012-01/msg02210.html

I will be posted a second version of the patches soon, with a
view to getting this fixed in the next QEMU release

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|



Re: [Qemu-devel] [PATCH 5/5] hw: add Atmel maxtouch touchscreen implementation

2012-03-02 Thread Igor Mitsyanko

On 03/02/2012 06:19 PM, Andreas Färber wrote:

Hi Igor,

Am 02.03.2012 12:35, schrieb Igor Mitsyanko:

diff --git a/Makefile.target b/Makefile.target
index 7968120..05ce652 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -349,6 +349,7 @@ obj-arm-y += exynos4210_gic.o exynos4210_combiner.o 
exynos4210.o
  obj-arm-y += exynos4_boards.o exynos4210_uart.o exynos4210_pwm.o
  obj-arm-y += exynos4210_pmu.o exynos4210_mct.o exynos4210_fimd.o
  obj-arm-y += exynos4210_i2c.o exynos4210_gpio.o
+obj-arm-y += maxtouch.o
  obj-arm-y += arm_l2x0.o
  obj-arm-y += arm_mptimer.o a15mpcore.o
  obj-arm-y += armv7m.o armv7m_nvic.o stellaris.o pl022.o stellaris_enet.o


I don't think this touchscreen device is really ARM-specific code-wise,
is it? I would rather expect this to be hw-obj-$(CONFIG_MAXTOUCH) in
Makefile.objs (so that it could be shared with, e.g., AVR).
default-configs/arm-softmmu.mak would need CONFIG_MAXTOUCH=y then.

Andreas


Sure, that makes a lot of sense.

Originally I was confused by framebuffer.o, bitbang_i2c.o (and a few 
other files) in arm-specific obj-arm-y. It gave me an impression that in 
case if only one target currently makes use of a certain hardware then 
corresponding hardware sources should be compiled only for this target.


--
Mitsyanko Igor
ASWG, Moscow R&D center, Samsung Electronics
email: i.mitsya...@samsung.com




Re: [Qemu-devel] [PATCH 5/5] hw: add Atmel maxtouch touchscreen implementation

2012-03-02 Thread Andreas Färber
Hi Igor,

Am 02.03.2012 12:35, schrieb Igor Mitsyanko:
> diff --git a/Makefile.target b/Makefile.target
> index 7968120..05ce652 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -349,6 +349,7 @@ obj-arm-y += exynos4210_gic.o exynos4210_combiner.o 
> exynos4210.o
>  obj-arm-y += exynos4_boards.o exynos4210_uart.o exynos4210_pwm.o
>  obj-arm-y += exynos4210_pmu.o exynos4210_mct.o exynos4210_fimd.o
>  obj-arm-y += exynos4210_i2c.o exynos4210_gpio.o
> +obj-arm-y += maxtouch.o
>  obj-arm-y += arm_l2x0.o
>  obj-arm-y += arm_mptimer.o a15mpcore.o
>  obj-arm-y += armv7m.o armv7m_nvic.o stellaris.o pl022.o stellaris_enet.o

I don't think this touchscreen device is really ARM-specific code-wise,
is it? I would rather expect this to be hw-obj-$(CONFIG_MAXTOUCH) in
Makefile.objs (so that it could be shared with, e.g., AVR).
default-configs/arm-softmmu.mak would need CONFIG_MAXTOUCH=y then.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH v3 5/7] RTC:Add RTC update-ended interrupt support

2012-03-02 Thread Zhang, Yang Z
> -Original Message-
> From: Paolo Bonzini [mailto:paolo.bonz...@gmail.com] On Behalf Of Paolo
> Bonzini
> Sent: Friday, March 02, 2012 8:14 PM
> To: Zhang, Yang Z
> Cc: qemu-devel@nongnu.org; Jan Kiszka; k...@vger.kernel.org;
> aligu...@us.ibm.com; Marcelo Tosatti
> Subject: Re: [PATCH v3 5/7] RTC:Add RTC update-ended interrupt support
> 
> Il 02/03/2012 08:00, Zhang, Yang Z ha scritto:
> > Use a timer to emulate update cycle. When update cycle ended and UIE
> > is setting, then raise an interrupt. The timer runs only when UF or AF is 
> > cleared.
> 
> Isn't AF=0 the common case when the RTC starts, so the timer always runs until
> the time crosses midnight?

Right. But for server platform, the guest will run always. One day is not a big 
cost. 
And if guest not use the alarm. Then the timer will never run again.
Also, I can set the default value in alarm to 'don't care' mode. And this will 
avoid the timer always runs.

best regards
yang



Re: [Qemu-devel] [PATCH] usb: queue can have async packets too

2012-03-02 Thread Gerd Hoffmann
> But: This + the latest GIT master causes again my problems with the

With or without this one:

commit 5ca2358ac895139e624881c5b3bf3095d3cc4515
Date:   Wed Feb 29 09:11:00 2012 -0600

Merge remote-tracking branch 'kraxel/usb.39' into staging

?

cheers,
  Gerd




Re: [Qemu-devel] [PATCH] spice: set spice uuid and name

2012-03-02 Thread Gerd Hoffmann
On 03/02/12 14:59, Marc-André Lureau wrote:
> On Fri, Mar 2, 2012 at 2:25 PM, Gerd Hoffmann  wrote:
>> On 03/02/12 13:49, Marc-André Lureau wrote:
>>> This allows a Spice client to identify a VM
>>
>> Patch doesn't apply, please rebase.
> 
> It applies here on top of git://git.qemu.org/qemu.git/master:
> 
> commit 88e6c60671df4c8b1b6c1eb8f76950ab1bea0ec2
> Merge: 14655e4 8f6f962
> Author: Anthony Liguori 
> Date:   Thu Mar 1 15:26:55 2012 -0600
> 
> Merge remote-tracking branch 'qemu-kvm/memory/urgent' into staging
> 

No:

rincewind kraxel ~/projects/qemu (tmp)# git am -s
~/Downloads/patches/spice/\[Qemu-devel\]\ \[PATCH\]\ spice\:\ set\
spice\ uuid\ and\ name.eml
Applying: spice: set spice uuid and name
error: patch failed: ui/spice-core.c:699
error: ui/spice-core.c: patch does not apply
Patch failed at 0001 spice: set spice uuid and name
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".

And please wait until the spice-server changes this patch depends on are
committed upstream before resubmitting.

thanks,
  Gerd



[Qemu-devel] [Bug 918791] Re: qemu-kvm dies when using vmvga driver and unity in the guest

2012-03-02 Thread Serge Hallyn
@Jamie,

yes libvirt supports spice.  You do have to set the video section
accordingly.  Here is an example xml file that works for me:


  spice
  524288
  524288
  1
  
hvm

  
  



  
  
  destroy
  restart
  restart
  
/usr/bin/kvm-spice

  
  
  
  


  



  


  


  



  



  
  
  


  



  


-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/918791

Title:
  qemu-kvm dies when using vmvga driver and unity in the guest

Status in QEMU:
  New
Status in “qemu-kvm” package in Ubuntu:
  Confirmed
Status in “xserver-xorg-video-vmware” package in Ubuntu:
  Confirmed
Status in “qemu-kvm” source package in Precise:
  Confirmed
Status in “xserver-xorg-video-vmware” source package in Precise:
  Confirmed

Bug description:
  12.04's qemu-kvm has been unstable for me and Marc Deslauriers and I
  figured out it has something to do with the interaction of qemu-kvm,
  unity and the vmvga driver. This is a regression over qemu-kvm in
  11.10.

  TEST CASE:
  1. start a VM that uses unity (eg, 11.04, 11.10 or 12.04). My tests use 
unity-2d on an amd64 host and amd64 guests
  2. on 11.04 and 11.10, open empathy via the messaging indicator and click 
'Chat'. On 12.04, open empathy via the messaging indicator and click 'Chat', 
close the empathy wizard, move the empathy window over the unity luancher (so 
it autohides), then do 'ctrl+alt+t' to open a terminal

  When the launcher tries to auto(un)hide, qemu-kvm dies with this:
  [10574.958149] do_general_protection: 132 callbacks suppressed
  [10574.958154] kvm[13192] general protection ip:7fab9680ea0f sp:74440148 
error:0 in qemu-system-x86_64[7fab966c4000+2c9000]

  Relevant libvirt xml:
  


  

  If I change to using 'cirrus', then qemu-kvm no longer crashes. Eg:
  



  

  The workaround is therefore to use the cirrus driver instead of vmvga,
  however being able to kill qemu-kvm in this manner is not ideal. Also,
  unfortunately unity-2d does not run with with cirrus driver under
  11.04, so the security and SRU teams are unable to properly test
  updates in GUI applications under unity when using the current 12.04
  qemu-kvm.

  I tried to report this via apport, but apport complained about a CRC
  error, so I could not.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/918791/+subscriptions



Re: [Qemu-devel] [PATCH] spice: set spice uuid and name

2012-03-02 Thread Marc-André Lureau
On Fri, Mar 2, 2012 at 2:25 PM, Gerd Hoffmann  wrote:
> On 03/02/12 13:49, Marc-André Lureau wrote:
>> This allows a Spice client to identify a VM
>
> Patch doesn't apply, please rebase.

It applies here on top of git://git.qemu.org/qemu.git/master:

commit 88e6c60671df4c8b1b6c1eb8f76950ab1bea0ec2
Merge: 14655e4 8f6f962
Author: Anthony Liguori 
Date:   Thu Mar 1 15:26:55 2012 -0600

Merge remote-tracking branch 'qemu-kvm/memory/urgent' into staging

-- 
Marc-André Lureau



[Qemu-devel] [PATCH 6/7] xhci: fix control xfers

2012-03-02 Thread Gerd Hoffmann
Use the new, direct control transfer submission method instead of
bypassing the usb core by calling usb_device_handle_control directly.
The later fails for async control transfers.

This patch gets xhci + usb-host combo going.
---
 hw/usb-xhci.c |   13 +
 1 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/hw/usb-xhci.c b/hw/usb-xhci.c
index fc5b542..8305489 100644
--- a/hw/usb-xhci.c
+++ b/hw/usb-xhci.c
@@ -1470,8 +1470,8 @@ static USBDevice *xhci_find_device(XHCIPort *port, 
uint8_t addr)
 static int xhci_fire_ctl_transfer(XHCIState *xhci, XHCITransfer *xfer)
 {
 XHCITRB *trb_setup, *trb_status;
-uint8_t bmRequestType, bRequest;
-uint16_t wValue, wLength, wIndex;
+uint8_t bmRequestType;
+uint16_t wLength;
 XHCIPort *port;
 USBDevice *dev;
 int ret;
@@ -1508,9 +1508,6 @@ static int xhci_fire_ctl_transfer(XHCIState *xhci, 
XHCITransfer *xfer)
 }
 
 bmRequestType = trb_setup->parameter;
-bRequest = trb_setup->parameter >> 8;
-wValue = trb_setup->parameter >> 16;
-wIndex = trb_setup->parameter >> 32;
 wLength = trb_setup->parameter >> 48;
 
 if (xfer->data && xfer->data_alloced < wLength) {
@@ -1537,12 +1534,12 @@ static int xhci_fire_ctl_transfer(XHCIState *xhci, 
XHCITransfer *xfer)
 xfer->iso_xfer = false;
 
 xhci_setup_packet(xfer, dev);
+xfer->packet.parameter = trb_setup->parameter;
 if (!xfer->in_xfer) {
 xhci_xfer_data(xfer, xfer->data, wLength, 0, 1, 0);
 }
-ret = usb_device_handle_control(dev, &xfer->packet,
-(bmRequestType << 8) | bRequest,
-wValue, wIndex, wLength, xfer->data);
+
+ret = usb_handle_packet(dev, &xfer->packet);
 
 xhci_complete_packet(xfer, ret);
 if (!xfer->running_async && !xfer->running_retry) {
-- 
1.7.1




[Qemu-devel] [PATCH 5/7] usb: add shortcut for control transfers

2012-03-02 Thread Gerd Hoffmann
Add a more direct code path to submit control transfers.  Instead of
feeding three usb packets (setup, data, ack) to usb_handle_packet and
have the do_token_* functions in usb.c poke the control transfer
parameters out of it just submit a single packet carrying the actual
data with the control xfer parameters filled into USBPacket->parameters.

Signed-off-by: Gerd Hoffmann 
---
 hw/usb.c |   59 +++
 hw/usb.h |1 +
 2 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/hw/usb.c b/hw/usb.c
index 800d912..1ec2e90 100644
--- a/hw/usb.c
+++ b/hw/usb.c
@@ -95,6 +95,7 @@ void usb_wakeup(USBEndpoint *ep)
 #define SETUP_STATE_SETUP 1
 #define SETUP_STATE_DATA  2
 #define SETUP_STATE_ACK   3
+#define SETUP_STATE_PARAM 4
 
 static int do_token_setup(USBDevice *s, USBPacket *p)
 {
@@ -226,6 +227,50 @@ static int do_token_out(USBDevice *s, USBPacket *p)
 }
 }
 
+static int do_parameter(USBDevice *s, USBPacket *p)
+{
+int request, value, index;
+int i, ret = 0;
+
+for (i = 0; i < 8; i++) {
+s->setup_buf[i] = p->parameter >> (i*8);
+}
+
+s->setup_state = SETUP_STATE_PARAM;
+s->setup_len   = (s->setup_buf[7] << 8) | s->setup_buf[6];
+s->setup_index = 0;
+
+request = (s->setup_buf[0] << 8) | s->setup_buf[1];
+value   = (s->setup_buf[3] << 8) | s->setup_buf[2];
+index   = (s->setup_buf[5] << 8) | s->setup_buf[4];
+
+if (s->setup_len > sizeof(s->data_buf)) {
+fprintf(stderr,
+"usb_generic_handle_packet: ctrl buffer too small (%d > 
%zu)\n",
+s->setup_len, sizeof(s->data_buf));
+return USB_RET_STALL;
+}
+
+if (p->pid == USB_TOKEN_OUT) {
+usb_packet_copy(p, s->data_buf, s->setup_len);
+}
+
+ret = usb_device_handle_control(s, p, request, value, index,
+s->setup_len, s->data_buf);
+if (ret < 0) {
+return ret;
+}
+
+if (ret < s->setup_len) {
+s->setup_len = ret;
+}
+if (p->pid == USB_TOKEN_IN) {
+usb_packet_copy(p, s->data_buf, s->setup_len);
+}
+
+return ret;
+}
+
 /* ctrl complete function for devices which use usb_generic_handle_packet and
may return USB_RET_ASYNC from their handle_control callback. Device code
which does this *must* call this function instead of the normal
@@ -250,6 +295,16 @@ void usb_generic_async_ctrl_complete(USBDevice *s, 
USBPacket *p)
 p->result = 0;
 break;
 
+case SETUP_STATE_PARAM:
+if (p->result < s->setup_len) {
+s->setup_len = p->result;
+}
+if (p->pid == USB_TOKEN_IN) {
+p->result = 0;
+usb_packet_copy(p, s->data_buf, s->setup_len);
+}
+break;
+
 default:
 break;
 }
@@ -292,6 +347,9 @@ static int usb_process_one(USBPacket *p)
 
 if (p->ep->nr == 0) {
 /* control pipe */
+if (p->parameter) {
+return do_parameter(dev, p);
+}
 switch (p->pid) {
 case USB_TOKEN_SETUP:
 return do_token_setup(dev, p);
@@ -416,6 +474,7 @@ void usb_packet_setup(USBPacket *p, int pid, USBEndpoint 
*ep)
 p->pid = pid;
 p->ep = ep;
 p->result = 0;
+p->parameter = 0;
 qemu_iovec_reset(&p->iov);
 usb_packet_set_state(p, USB_PACKET_SETUP);
 }
diff --git a/hw/usb.h b/hw/usb.h
index 5bcc9b5..24147e9 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -326,6 +326,7 @@ struct USBPacket {
 int pid;
 USBEndpoint *ep;
 QEMUIOVector iov;
+uint64_t parameter; /* control transfers */
 int result; /* transfer length or USB_RET_* status code */
 /* Internal use by the USB layer.  */
 USBPacketState state;
-- 
1.7.1




[Qemu-devel] [PATCH] qcow2: Add some tracing

2012-03-02 Thread Kevin Wolf
Signed-off-by: Kevin Wolf 
---
 block/qcow2-cache.c   |   18 ++
 block/qcow2-cluster.c |   15 ++-
 block/qcow2.c |9 +
 trace-events  |   24 
 4 files changed, 65 insertions(+), 1 deletions(-)

diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index 340a6f2..710d4b1 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -25,6 +25,7 @@
 #include "block_int.h"
 #include "qemu-common.h"
 #include "qcow2.h"
+#include "trace.h"
 
 typedef struct Qcow2CachedTable {
 void*   table;
@@ -100,6 +101,9 @@ static int qcow2_cache_entry_flush(BlockDriverState *bs, 
Qcow2Cache *c, int i)
 return 0;
 }
 
+trace_qcow2_cache_entry_flush(qemu_coroutine_self(),
+  c == s->l2_table_cache, i);
+
 if (c->depends) {
 ret = qcow2_cache_flush_dependency(bs, c);
 } else if (c->depends_on_flush) {
@@ -132,10 +136,13 @@ static int qcow2_cache_entry_flush(BlockDriverState *bs, 
Qcow2Cache *c, int i)
 
 int qcow2_cache_flush(BlockDriverState *bs, Qcow2Cache *c)
 {
+BDRVQcowState *s = bs->opaque;
 int result = 0;
 int ret;
 int i;
 
+trace_qcow2_cache_flush(qemu_coroutine_self(), c == s->l2_table_cache);
+
 for (i = 0; i < c->size; i++) {
 ret = qcow2_cache_entry_flush(bs, c, i);
 if (ret < 0 && result != -ENOSPC) {
@@ -218,6 +225,9 @@ static int qcow2_cache_do_get(BlockDriverState *bs, 
Qcow2Cache *c,
 int i;
 int ret;
 
+trace_qcow2_cache_get(qemu_coroutine_self(), c == s->l2_table_cache,
+  offset, read_from_disk);
+
 /* Check if the table is already cached */
 for (i = 0; i < c->size; i++) {
 if (c->entries[i].offset == offset) {
@@ -227,6 +237,8 @@ static int qcow2_cache_do_get(BlockDriverState *bs, 
Qcow2Cache *c,
 
 /* If not, write a table back and replace it */
 i = qcow2_cache_find_entry_to_replace(c);
+trace_qcow2_cache_get_replace_entry(qemu_coroutine_self(),
+c == s->l2_table_cache, i);
 if (i < 0) {
 return i;
 }
@@ -236,6 +248,8 @@ static int qcow2_cache_do_get(BlockDriverState *bs, 
Qcow2Cache *c,
 return ret;
 }
 
+trace_qcow2_cache_get_read(qemu_coroutine_self(),
+   c == s->l2_table_cache, i);
 c->entries[i].offset = 0;
 if (read_from_disk) {
 if (c == s->l2_table_cache) {
@@ -258,6 +272,10 @@ found:
 c->entries[i].cache_hits++;
 c->entries[i].ref++;
 *table = c->entries[i].table;
+
+trace_qcow2_cache_get_done(qemu_coroutine_self(),
+   c == s->l2_table_cache, i);
+
 return 0;
 }
 
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 07a2e93..a791bbe 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -27,6 +27,7 @@
 #include "qemu-common.h"
 #include "block_int.h"
 #include "block/qcow2.h"
+#include "trace.h"
 
 int qcow2_grow_l1_table(BlockDriverState *bs, int min_size, bool exact_size)
 {
@@ -170,6 +171,8 @@ static int l2_allocate(BlockDriverState *bs, int l1_index, 
uint64_t **table)
 
 old_l2_offset = s->l1_table[l1_index];
 
+trace_qcow2_l2_allocate(bs, l1_index);
+
 /* allocate a new l2 entry */
 
 l2_offset = qcow2_alloc_clusters(bs, s->l2_size * sizeof(uint64_t));
@@ -184,6 +187,7 @@ static int l2_allocate(BlockDriverState *bs, int l1_index, 
uint64_t **table)
 
 /* allocate a new entry in the l2 cache */
 
+trace_qcow2_l2_allocate_get_empty(bs, l1_index);
 ret = qcow2_cache_get_empty(bs, s->l2_table_cache, l2_offset, (void**) 
table);
 if (ret < 0) {
 return ret;
@@ -216,6 +220,7 @@ static int l2_allocate(BlockDriverState *bs, int l1_index, 
uint64_t **table)
 /* write the l2 table to the file */
 BLKDBG_EVENT(bs->file, BLKDBG_L2_ALLOC_WRITE);
 
+trace_qcow2_l2_allocate_write_l2(bs, l1_index);
 qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_table);
 ret = qcow2_cache_flush(bs, s->l2_table_cache);
 if (ret < 0) {
@@ -223,6 +228,7 @@ static int l2_allocate(BlockDriverState *bs, int l1_index, 
uint64_t **table)
 }
 
 /* update the L1 entry */
+trace_qcow2_l2_allocate_write_l1(bs, l1_index);
 s->l1_table[l1_index] = l2_offset | QCOW_OFLAG_COPIED;
 ret = write_l1_entry(bs, l1_index);
 if (ret < 0) {
@@ -230,9 +236,11 @@ static int l2_allocate(BlockDriverState *bs, int l1_index, 
uint64_t **table)
 }
 
 *table = l2_table;
+trace_qcow2_l2_allocate_done(bs, l1_index, 0);
 return 0;
 
 fail:
+trace_qcow2_l2_allocate_done(bs, l1_index, ret);
 qcow2_cache_put(bs, s->l2_table_cache, (void**) table);
 s->l1_table[l1_index] = old_l2_offset;
 return ret;
@@ -584,6 +592,8 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, 
QCowL2Meta *m)
 uint64_t cluster_offset = m->cluster_offset;
 bool cow = false;
 
+trace_qcow2_cluster_link_l2(qemu_coro

Re: [Qemu-devel] [PATCH] add reopen to blockdev-transaction

2012-03-02 Thread Kevin Wolf
Am 02.03.2012 14:25, schrieb Paolo Bonzini:
> Il 02/03/2012 14:00, Kevin Wolf ha scritto:
>> Am 01.03.2012 17:52, schrieb Paolo Bonzini:
> But you can even keep from your first patch the drive-reopen command and
> not make it atomic, that shouldn't be a problem.

 I'm not sure whether it makes sense for a separate drive-reopen or
 whether to just add this to blockdev-transaction (or even both); I can
 make libvirt use whichever color bikeshed we pick.  There's definitely a
 transaction aspect here
>>>
>>> It's not so much atomicity, it's just safety.  The drive-reopen command
>>> must be implemented in a similar way to bdrv_append; it must not do a
>>> close+reopen in the same way as the existing blockdev-snapshot-sync
>>> command, but that's just that blockdev-snapshot-sync was implemented
>>> poorly.
>>
>> For reopen this is a bit harder because you deal with already opened
>> images and you must never have the same image opened twice at the same time.
> 
> This is only for read-write images, and the backing files are read-only,
> so this shouldn't be a problem, no?

Opening an image read-write that is still open read-only may break the
read-only instance.

You can argue that opening an image read-only while a read-write
instance is open can be tolerated if you flushed the image and made sure
no new requests are coming in. This is what happens with live migration.
It's a case that has given us enough headaches that I would not want to
introduce similar behaviour in more cases.

So in short: Regardless of ro/rw, opening images twice is bad. Just
don't do it.

If anything, a possible solution could look like the bdrv_reopen
proposal which already includes prepare/commit/abort functions in the
block driver.

Kevin



[Qemu-devel] [PATCH 2/7] usb: queue can have async packets

2012-03-02 Thread Gerd Hoffmann
This can happen today in case the ->complete() callback queues up the
next packet.  Also we'll support pipelining soon, which allows to have
multiple packets per queue in flight (aka ASYNC) state.

Signed-off-by: Gerd Hoffmann 
---
 hw/usb.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/usb.c b/hw/usb.c
index 57fc5e3..fc41d62 100644
--- a/hw/usb.c
+++ b/hw/usb.c
@@ -356,6 +356,9 @@ void usb_packet_complete(USBDevice *dev, USBPacket *p)
 
 while (!QTAILQ_EMPTY(&ep->queue)) {
 p = QTAILQ_FIRST(&ep->queue);
+if (p->state == USB_PACKET_ASYNC) {
+break;
+}
 assert(p->state == USB_PACKET_QUEUED);
 ret = usb_process_one(p);
 if (ret == USB_RET_ASYNC) {
-- 
1.7.1




Re: [Qemu-devel] [PATCH] add reopen to blockdev-transaction

2012-03-02 Thread Paolo Bonzini
Il 02/03/2012 14:00, Kevin Wolf ha scritto:
> Am 01.03.2012 17:52, schrieb Paolo Bonzini:
 But you can even keep from your first patch the drive-reopen command and
 not make it atomic, that shouldn't be a problem.
>>>
>>> I'm not sure whether it makes sense for a separate drive-reopen or
>>> whether to just add this to blockdev-transaction (or even both); I can
>>> make libvirt use whichever color bikeshed we pick.  There's definitely a
>>> transaction aspect here
>>
>> It's not so much atomicity, it's just safety.  The drive-reopen command
>> must be implemented in a similar way to bdrv_append; it must not do a
>> close+reopen in the same way as the existing blockdev-snapshot-sync
>> command, but that's just that blockdev-snapshot-sync was implemented
>> poorly.
> 
> For reopen this is a bit harder because you deal with already opened
> images and you must never have the same image opened twice at the same time.

This is only for read-write images, and the backing files are read-only,
so this shouldn't be a problem, no?

Hmm, actually there could one problem.  Say you switch from base->old to
base->new; if old is the outcome of a live snapshot operation, base will
still be open read-write.

So perhaps we do need a new method like bdrv_freeze; after bdrv_freeze
you know that bdrv_close will not need to write to disk.  So e.g. for
QED bdrv_freeze will turn off the need-check bit.

Paolo



Re: [Qemu-devel] [PATCH] spice: set spice uuid and name

2012-03-02 Thread Gerd Hoffmann
On 03/02/12 13:49, Marc-André Lureau wrote:
> This allows a Spice client to identify a VM

Patch doesn't apply, please rebase.

cheers,
  Gerd




  1   2   >