Attention! Votre Compte a été limité !!!!

[Skype Service]

Title: Attention! Votre Compte  a été limité !







	
		
			

	
		
			

	
		
			

	
		
			


	
		
			

	
		
			

	
		
			

	
		
			

	
	
		
			
			
			
			

		
		
			
			 
		
		
			
			

	
	
	
		
			
			
			

	Dear Skype Member:
	
	
	As part of our security
 measures, we regularly screen activity in the Skype system.We recently 
contacted you after noticing an issue on your 
account
	
This is the Last reminder to log in to Skype as soon 
as possible. Once 
you log in, you will be provided with steps to restore your
 account 
access.We appreciate your understanding as we work to 
ensure 
account safety.
	
 
	
		
		
	
	
		
			

 


	
		
		
	
	
		
		
		
		
		Click here
	
	


 
			
		
	
 
	

			
			

	 
	 
	We thank you for your prompt attention to this matter. Please understand that this is a security measure intended to help protect you and your account. We apologise for any inconvenience. 
	
	Sincerely,
	Skype Account Review Department

			
			
			
		
	
	
	


			
		
		
			
			

	
	
	

			
			
		
		
			
			

	
	
	
		
			
			

	
	My account 

			
			
			
			
			

	

[Qemu-devel] HDD problem with Xilinx virtex-ml507 board

[Lê Đức Tài]

Hi,
I have a problem when emulating virtex-ml507.
Loop device can not be mounted as hda, no partitions are listed.

My environment is as following:

1. Qemu 0.14.1 build with libfdt support.

2. Kernel download from Xilinx Git server.
config is customized from 44x/virtex_defconfig with enable some option to 
make kernel can automount filesystem:
Under device drivers--->block devices:
CONFIG_BLK_DEV_LOOP
CONFIG_BLK_DEV_RAM
CONFIG_BLK_DEV_NBD
Under device drivers--->ATA/ATAPI/MFM/RLL support 
CONFIG_IDE_GD
CONFIG_IDE_GD_ATA
CONFIG_BLK_DEV_GENERIC
CONFIG_BLK_DEV_PIIX
Under file systems:
CONFIG_AUTOFS_FS
CONFIG_AUTOFS4_FS
Also the ext2, ext3  file systems are supported

3. RFS is busybox

Run Qemu:
$ qemu-system-ppc -M virtex-ml507 -kernel vmlinux -m 256 -hda rootfs_ppc.ext2 
-append "console=ttyS0 root=/dev/hda" -nographic
Output:
[0.755969] NET: Registered protocol family 17
[0.760477] hd: no drives specified - use hd=cyl,head,sectors on kernel 
command line
[0.782726] Root-NFS: no NFS server address
[0.782932] VFS: Unable to mount root fs via NFS, trying floppy.
[0.790541] VFS: Cannot open root device "hda" or unknown-block(2,0)
[0.792438] Please append a correct "root=" boot option; here are the 
available partitions:
[0.795265] Kernel panic - not syncing: VFS: Unable to mount root fs on 
unknown-block(2,0)

Can you help me for this problem?
Thank.

TaiLD

[Qemu-devel] [PATCH 2/2] qemu-io: Fix if scoping bug

[Devin Nakamura]

Fix a bug caused by lack of braces in if statement

Signed-off-by: Devin Nakamura 
---
 qemu-io.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/qemu-io.c b/qemu-io.c
index 53adb76..1c4f684 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -433,12 +433,12 @@ static int read_f(int argc, char **argv)
 return 0;
 }

-if (!pflag)
+if (!pflag){
 if (offset & 0x1ff) {
 printf("offset %" PRId64 " is not sector aligned\n",
offset);
 return 0;
-
+}
 if (count & 0x1ff) {
 printf("count %d is not sector aligned\n",
count);
-- 
1.7.6.rc1

[Qemu-devel] [PATCH] Replaced tabs with spaces in block.h and block_int.h

[Devin Nakamura]

Signed-off-by: Devin Nakamura 
---
 block.h |6 +++---
 block_int.h |4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/block.h b/block.h
index da7d39c..859d1d9 100644
--- a/block.h
+++ b/block.h
@@ -110,7 +110,7 @@ int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res);
 typedef struct BlockDriverAIOCB BlockDriverAIOCB;
 typedef void BlockDriverCompletionFunc(void *opaque, int ret);
 typedef void BlockDriverDirtyHandler(BlockDriverState *bs, int64_t sector,
-int sector_num);
+ int sector_num);
 BlockDriverAIOCB *bdrv_aio_readv(BlockDriverState *bs, int64_t sector_num,
  QEMUIOVector *iov, int nb_sectors,
  BlockDriverCompletionFunc *cb, void *opaque);
@@ -118,7 +118,7 @@ BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState
*bs, int64_t sector_num,
   QEMUIOVector *iov, int nb_sectors,
   BlockDriverCompletionFunc *cb, void *opaque);
 BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs,
-BlockDriverCompletionFunc *cb, void *opaque);
+ BlockDriverCompletionFunc *cb, void *opaque);
 void bdrv_aio_cancel(BlockDriverAIOCB *acb);

 typedef struct BlockRequest {
@@ -150,7 +150,7 @@ void bdrv_close_all(void);
 int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors);
 int bdrv_has_zero_init(BlockDriverState *bs);
 int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
-   int *pnum);
+  int *pnum);

 #define BIOS_ATA_TRANSLATION_AUTO   0
 #define BIOS_ATA_TRANSLATION_NONE   1
diff --git a/block_int.h b/block_int.h
index eefbd34..5ffc1a9 100644
--- a/block_int.h
+++ b/block_int.h
@@ -213,8 +213,8 @@ struct BlockDriverState {
 void *private;
 };

-#define CHANGE_MEDIA   0x01
-#define CHANGE_SIZE0x02
+#define CHANGE_MEDIA0x01
+#define CHANGE_SIZE 0x02

 struct BlockDriverAIOCB {
 AIOPool *pool;
-- 
1.7.6.rc1

[Qemu-devel] buildbot failure in qemu on pci_x86_64_debian_5_0

[qemu]

The Buildbot has detected a new failure on builder pci_x86_64_debian_5_0 while 
building qemu.
Full details are available at:
 http://buildbot.b1-systems.de/qemu/builders/pci_x86_64_debian_5_0/builds/0

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

Buildslave for this Build: yuzuki

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

BUILD FAILED: failed compile

sincerely,
 -The Buildbot

[Qemu-devel] buildbot failure in qemu on pci_i386_debian_5_0

[qemu]

The Buildbot has detected a new failure on builder pci_i386_debian_5_0 while 
building qemu.
Full details are available at:
 http://buildbot.b1-systems.de/qemu/builders/pci_i386_debian_5_0/builds/0

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

Buildslave for this Build: yuzuki

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

BUILD FAILED: failed compile

sincerely,
 -The Buildbot

Re: [Qemu-devel] [PATCH] v6 revamp acpitable parsing and allow to specify complete (headerful) table

[Michael Tokarev]

I've given up on this one.  Personally I don't need
this stuff for my win7 guests since I can hack either
bios or the O/S loader to include all the necessary
verifications for the win7 activation to work.  I
tried to make this process to be legal (no hacks
or "cracks" needed) and easy for others, but since
noone is interested in this after 6 versions and 5
resends, I wont continue - what I am trying to achieve
by pushing this so hard, after all?

Thanks to everyone who gave (mostly code style) comments -
at least I know the changes has been read by someone.

Frustrated,
 /mjt

12.05.2011 18:44, Michael Tokarev wrote:
> This patch almost rewrites acpi_table_add() function
> (but still leaves it using old get_param_value() interface).
> The result is that it's now possible to specify whole table
> (together with a header) in an external file, instead of just
> data portion, with a new file= parameter, but at the same time
> it's still possible to specify header fields as before.
> 
> Now with the checkpatch.pl formatting fixes, thanks to
> Stefan Hajnoczi for suggestions, with changes from
> Isaku Yamahata, and with my further refinements.
> 
> v5: rediffed against current qemu/master.
> v6: fix one "} else {" coding style defect (noted by Blue Swirl)
> 
> Signed-off-by: Michael Tokarev 
> ---
>  hw/acpi.c   |  292 
> ---
>  qemu-options.hx |7 +-
>  2 files changed, 175 insertions(+), 124 deletions(-)
> 
> diff --git a/hw/acpi.c b/hw/acpi.c
> index ad40fb4..4316189 100644
> --- a/hw/acpi.c
> +++ b/hw/acpi.c
> @@ -22,17 +22,29 @@
>  
>  struct acpi_table_header
>  {
> -char signature [4];/* ACPI signature (4 ASCII characters) */
> +uint16_t _length; /* our length, not actual part of the hdr */
> +  /* XXX why we have 2 length fields here? */
> +char sig[4];  /* ACPI signature (4 ASCII characters) */
>  uint32_t length;  /* Length of table, in bytes, including header 
> */
>  uint8_t revision; /* ACPI Specification minor version # */
>  uint8_t checksum; /* To make sum of entire table == 0 */
> -char oem_id [6];   /* OEM identification */
> -char oem_table_id [8]; /* OEM table identification */
> +char oem_id[6];   /* OEM identification */
> +char oem_table_id[8]; /* OEM table identification */
>  uint32_t oem_revision;/* OEM revision number */
> -char asl_compiler_id [4]; /* ASL compiler vendor ID */
> +char asl_compiler_id[4];  /* ASL compiler vendor ID */
>  uint32_t asl_compiler_revision; /* ASL compiler revision number */
>  } __attribute__((packed));
>  
> +#define ACPI_TABLE_HDR_SIZE sizeof(struct acpi_table_header)
> +#define ACPI_TABLE_PFX_SIZE sizeof(uint16_t)  /* size of the extra prefix */
> +
> +static const char dfl_hdr[ACPI_TABLE_HDR_SIZE] =
> +"\0\0"   /* fake _length (2) */
> +"QEMU\0\0\0\0\1\0"   /* sig (4), len(4), revno (1), csum (1) */
> +"QEMUQEQEMUQEMU\1\0\0\0" /* OEM id (6), table (8), revno (4) */
> +"QEMU\1\0\0\0"   /* ASL compiler ID (4), version (4) */
> +;
> +
>  char *acpi_tables;
>  size_t acpi_tables_len;
>  
> @@ -45,158 +57,192 @@ static int acpi_checksum(const uint8_t *data, int len)
>  return (-sum) & 0xff;
>  }
>  
> +/* like strncpy() but zero-fills the tail of destination */
> +static void strzcpy(char *dst, const char *src, size_t size)
> +{
> +size_t len = strlen(src);
> +if (len >= size) {
> +len = size;
> +} else {
> +  memset(dst + len, 0, size - len);
> +}
> +memcpy(dst, src, len);
> +}
> +
> +/* XXX fixme: this function uses obsolete argument parsing interface */
>  int acpi_table_add(const char *t)
>  {
> -static const char *dfl_id = "QEMUQEMU";
>  char buf[1024], *p, *f;
> -struct acpi_table_header acpi_hdr;
>  unsigned long val;
> -uint32_t length;
> -struct acpi_table_header *acpi_hdr_p;
> -size_t off;
> +size_t len, start, allen;
> +bool has_header;
> +int changed;
> +int r;
> +struct acpi_table_header hdr;
> +
> +r = 0;
> +r |= get_param_value(buf, sizeof(buf), "data", t) ? 1 : 0;
> +r |= get_param_value(buf, sizeof(buf), "file", t) ? 2 : 0;
> +switch (r) {
> +case 0:
> +buf[0] = '\0';
> +case 1:
> +has_header = false;
> +break;
> +case 2:
> +has_header = true;
> +break;
> +default:
> +fprintf(stderr, "acpitable: both data and file are specified\n");
> +return -1;
> +}
> +
> +if (!acpi_tables) {
> +allen = sizeof(uint16_t);
> +acpi_tables = qemu_mallocz(allen);
> +}
> +else {
> +allen = acpi_tables_len;
> +}
> +
> +start = allen;
> +acpi_tables = qemu_realloc(acpi_tables, start + ACPI_TABLE_HDR_SIZE);
> +allen += has_header ? ACPI_TABLE_PFX_SIZE : ACPI_TABLE_HDR_SIZE;
> +
> +/

[Qemu-devel] semantics of "-cpu host" and "check"/"enforce"

[Eduardo Habkost]

Hi,

While checking the cpu model code, I don't think I understand fully what
is supposed to be the right semantics for '-cpu host' on qemu-kvm, and
what exactly we are aiming to.

Maybe this was already discussed before, but I failed to find any
additional information except for the original '-cpu host' patch
submission.

We have 3 sets of cpu features that may or may not be included in
'-cpu host':

A) Features that are supported by the host and that KVM can already
   emulate, or don't need KVM support to be used;
B) Features that may be not supported by the host but can be emulated by
   KVM (e.g. the SVM features, or x2apic);
C) Features that are supported by the host but KVM can't emulate.
   Divided in:
   C1) features we can't emulate and we know about it (e.g. dtes64)[1]
   C2) features we possibly can't emulate but we don't even know about it
   (e.g. features added to recent CPUs).

It seems obvious that all the features in group A must always be
included in '-cpu host', but what about features in the B or C groups?


About group B: it looks like we are not being consistent. For example,
svm_features has every bit enabled when using '-cpu host' even if the
host doesn't support them; in other cases (e.g. x2apic), it is not
enabled by '-cpu host' unless the host already supports it.

Shouldn't we aim for consistency here and choose one of both approaches?
Maybe we want two different model names or options, to differentiate (A)
and (A+B)?  (maybe something like "host" and "host,+all"?)


About group C: If the C group is not empty and 'enforce' is set in the
command-line, should we try to enable the feature and consider the
missing feature a failure condition, or simply avoid enabling the
feature?


Current semantics of '-cpu host' seems to be: A + all svm features. That
means that only part of B is included (all emulated svm features are in,
but x2apic is out); group C seems to be excluded entirely (by
whitelisting in the kvm kernel code), but the disabled features don't
trigger "enforce" errors. Is that correct?


[1] And 3dnow? Why is 3dnow always disabled on qemu-kvm.git/master, at
cpu_x86_cpuid()?

-- 
Eduardo

Re: [Qemu-devel] [PATCH] doc: Minor typo fix.

[Edgar E. Iglesias]

On Thu, Jun 09, 2011 at 07:50:43AM +1000, Brad Hards wrote:
> Thanks to agraf_, stefanha and Snader_LB for their IRC assistance.
> 
> Thanks to Markus Armbruster and Alexander Graf (again) for their
> assistance with the second version of this patch. No patch is too
> simple to test...
> 
> Signed-off-by: Brad Hards 

Applied, thanks.


> ---
>  qemu-options.hx |4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 82e085a..88e7eaa 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1161,9 +1161,9 @@ Specify the guest-visible address of the host. Default 
> is the 2nd IP in the
>  guest network, i.e. x.x.x.2.
>  
>  @item restrict=y|yes|n|no
> -If this options is enabled, the guest will be isolated, i.e. it will not be
> +If this option is enabled, the guest will be isolated, i.e. it will not be
>  able to contact the host and no guest IP packets will be routed over the host
> -to the outside. This option does not affect explicitly set forwarding rule.
> +to the outside. This option does not affect any explicitly set forwarding 
> rules.
>  
>  @item hostname=@var{name}
>  Specifies the client hostname reported by the builtin DHCP server.
> -- 
> 1.7.4.1

Re: [Qemu-devel] [PATCH] sigfd: use pthread_sigmask

[Edgar E. Iglesias]

On Thu, Jun 09, 2011 at 12:55:37AM +0200, Alexander Graf wrote:
> Qemu uses signalfd to figure out, if a signal occured without the need
> to actually receive the signal. Instead, it can read from the fd to receive
> its news.
> 
> Now, we obviously don't always have signalfd around. Especially not on
> non-Linux systems. So what we do there is that we create a new thread,
> block that thread on all signals and simply call sigwait to wait for a
> signal we're interested in to occur.
> 
> This all sounds great, but what we're really doing is:
> 
> sigset_t all;
> 
> sigfillset(&all);
> sigprocmask(SIG_BLOCK, &all, NULL);
> 
> which - on Darwin - blocks all signals on the current _process_, not only
> on the current thread. To block signals on the thread, we can use
> pthread_sigmask().
> 
> This patch does that, assuming that my above analysis is correct, and thus
> renders Qemu useable on Darwin again.


Applied, thanks all.

Cheers


> 
> Reported-by: Andreas Färber 
> CC: Paolo Bonzini 
> CC: Jan Kiszka 
> CC: Anthony Liguori 
> Signed-off-by: Alexander Graf 
> ---
>  compatfd.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/compatfd.c b/compatfd.c
> index bd377c4..41586ce 100644
> --- a/compatfd.c
> +++ b/compatfd.c
> @@ -29,7 +29,7 @@ static void *sigwait_compat(void *opaque)
>  sigset_t all;
>  
>  sigfillset(&all);
> -sigprocmask(SIG_BLOCK, &all, NULL);
> +pthread_sigmask(SIG_BLOCK, &all, NULL);
>  
>  while (1) {
>  int sig;
> -- 
> 1.7.1

Re: [Qemu-devel] [PATCH] hw/9pfs: Fix segfault on arch that doesn't support VirtFS

[Edgar E. Iglesias]

On Fri, Jun 10, 2011 at 07:13:12PM +0530, Aneesh Kumar K.V wrote:
> Signed-off-by: Aneesh Kumar K.V 

Thanks for the quick response earlier today, I had already applied
your first patch :)

Cheers


> ---
>  fsdev/qemu-fsdev-dummy.c |8 
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/fsdev/qemu-fsdev-dummy.c b/fsdev/qemu-fsdev-dummy.c
> index 619e163..4e700dd 100644
> --- a/fsdev/qemu-fsdev-dummy.c
> +++ b/fsdev/qemu-fsdev-dummy.c
> @@ -13,8 +13,16 @@
>  #include 
>  #include 
>  #include "qemu-fsdev.h"
> +#include "qemu-config.h"
>  
>  int qemu_fsdev_add(QemuOpts *opts)
>  {
>  return 0;
>  }
> +
> +static void fsdev_register_config(void)
> +{
> +qemu_add_opts(&qemu_fsdev_opts);
> +qemu_add_opts(&qemu_virtfs_opts);
> +}
> +machine_init(fsdev_register_config);
> -- 
> 1.7.4.1
> 
> 

Re: [Qemu-devel] [PULL 00/26] Alpha system emulation, v5

[Edgar E. Iglesias]

On Wed, Jun 08, 2011 at 12:10:39PM -0700, Richard Henderson wrote:
> Ping^3.  Anyone?  Bueller?  Bueller?
> 
> r~
> 
> On 06/02/2011 07:56 AM, Richard Henderson wrote:
> > Ping^2.
> > 
> > r~
> > 
> > On 05/27/2011 12:55 PM, Richard Henderson wrote:
> >> Ping?
> >>
> >>
> >> r~
> >>
> >> On 05/23/2011 01:28 PM, Richard Henderson wrote:
> >>> Changes from v4 -> v5
> >>>
> >>>   * Claim official ownership of the Alpha port, rather
> >>> than leave it as "unmaintained".
> >>>
> >>>   * Drop all the patches in hw/ for now.  While they're necessary
> >>> to actually make the port work, these are the subset of the whole
> >>> patchset for which I'm confident I'm doing the Right Thing and
> >>> don't really need patch review.
> >>>
> >>> No mistake, patch review is still welcome but no one has posted
> >>> *anything* substantive for v1->v4.
> >>>
> >>> Please pull.
> >>>


Thanks, I've pulled your changes.

Cheers

[Qemu-devel] [PATCH v2] block/rbd: Remove unused local variable

[Stefan Weil]

Variable 'snap' is assigned a value that is never used.
Remove snap and the related code.

v2:
  The unused variable which was in function rbd_open is now in function
  qemu_rbd_create, so the patch needed an update.

Cc: Christian Brunner 
Cc: Josh Durgin 
Cc: Kevin Wolf 
Signed-off-by: Stefan Weil 
---
 block/rbd.c |4 
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index bdc448a..d5659cd 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -227,7 +227,6 @@ static int qemu_rbd_create(const char *filename, 
QEMUOptionParameter *options)
 char name[RBD_MAX_IMAGE_NAME_SIZE];
 char snap_buf[RBD_MAX_SNAP_NAME_SIZE];
 char conf[RBD_MAX_CONF_SIZE];
-char *snap = NULL;
 rados_t cluster;
 rados_ioctx_t io_ctx;
 int ret;
@@ -238,9 +237,6 @@ static int qemu_rbd_create(const char *filename, 
QEMUOptionParameter *options)
conf, sizeof(conf)) < 0) {
 return -EINVAL;
 }
-if (snap_buf[0] != '\0') {
-snap = snap_buf;
-}
 
 /* Read out options */
 while (options && options->name) {
-- 
1.7.2.5

Re: [Qemu-devel] [PATCH 1/3] lsi: Fix unused-but-set-variable warning

[Richard Henderson]

On 06/10/2011 09:36 AM, Peter Maydell wrote:
> Stefan, if you're picking up gcc warning fixes for the trivial
> tree, how about this one?
> 
> (I think the other gcc warning fixes are either:
>  * in the most recent usb pull request
>  * linux-user
>  * target-alpha

Speaking of, anyone willing to pull and/or look at my target-alpha patchset?
I've been re-posting and/or pinging it for more than 2 months now with no
more than one spelling error as feedback...


r~

Re: [Qemu-devel] [PATCH-v3 2/2] kvm: Fix unused-but-set-variable warning

[Stefan Hajnoczi]

On Tue, May 31, 2011 at 09:53:49AM +0200, Christophe Fergeau wrote:
> Based on a patch from Hans de Goede 
> 
> This warning is new in gcc 4.6.
> 
> Acked-by: Amit Shah 
> Signed-off-by: Christophe Fergeau 
> ---
>  target-i386/kvm.c |3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)

Thanks, applied to the trivial patches tree:
http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/trivial-patches

Stefan

Re: [Qemu-devel] [PATCH 1/3] lsi: Fix unused-but-set-variable warning

[Stefan Hajnoczi]

On Wed, Jun 01, 2011 at 02:56:30PM +0200, Christophe Fergeau wrote:
> This warning is new in gcc 4.6.
> 
> Signed-off-by: Christophe Fergeau 
> ---
>  hw/lsi53c895a.c |2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)

Thanks, applied to the trivial patches tree:
http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/trivial-patches

Stefan

Re: [Qemu-devel] [PATCH 09/12] kvm: ppc: Drop KVM_CAP build dependencies

[Eduardo Habkost]

On Wed, Jun 08, 2011 at 04:11:03PM +0200, Jan Kiszka wrote:

> @@ -217,7 +209,6 @@ int kvm_arch_get_registers(CPUState *env)
>  return ret;
>  }
>  
> -#ifdef KVM_CAP_PPC_BOOKE_SREGS
>  if (sregs.u.e.features & KVM_SREGS_E_BASE) {
>  env->spr[SPR_BOOKE_CSRR0] = sregs.u.e.csrr0;
>  env->spr[SPR_BOOKE_CSRR1] = sregs.u.e.csrr1;

Which tree/commit-id did you use as base for this series? This chunk
doesn't apply on uq/master (patch 11/12, on the other hand, doesn't
apply against qemu.git/master, only uq/master).

After some patch hunting, I found out that the series apply cleanly if I
apply it against a manual merge of uq/master and qemu.git/master, but
maybe there is a branch that already had all the dependencies, that I
didn't know about?

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 0/6] Implement constant folding and copy propagation in TCG

[Richard Henderson]

On 06/09/2011 03:45 AM, Kirill Batuzov wrote:
> Changes:
> v1 -> v2
>  - State and Vals arrays merged to an array of structures.
>  - Added reference counting of temp's copies. This helps to reset temp's state
>faster in most cases.
>  - Do not make copy propagation through operations with TCG_OPF_CALL_CLOBBER 
> or
>TCG_OPF_SIDE_EFFECTS flag.
>  - Split some expression simplifications into independent switch.
>  - Let compiler handle signed shifts and sign/zero extends in it's
>implementation defined way.
> 
> Kirill Batuzov (6):
>   Add TCG optimizations stub
>   Add copy and constant propagation.
>   Do constant folding for basic arithmetic operations.
>   Do constant folding for boolean operations.
>   Do constant folding for shift operations.
>   Do constant folding for unary operations.
> 
>  Makefile.target |2 +-
>  tcg/optimize.c  |  633 
> +++
>  tcg/tcg.c   |6 +
>  tcg/tcg.h   |3 +
>  4 files changed, 643 insertions(+), 1 deletions(-)
>  create mode 100644 tcg/optimize.c
> 

FWIW, a patch that implements most of the suggestions that I made
to the indivual patches in this thread, including a major restructure
of the body of the optimization to avoid code duplication.

I havn't bothered to break it up for now, but at least you can see 
what I was talking about.


r~

---
>From 2b49e328d3d2461f97f0b6802e0c8e88e0165b1c Mon Sep 17 00:00:00 2001
From: Richard Henderson 
Date: Fri, 10 Jun 2011 10:08:17 -0700
Subject: [PATCH] Re-org tcg optimizer.

1: Put equivalence class members in a double-linked list.
2: Use glue to handle 32+64-bit opcodes at the same time.
3: Tidy duplicate code sequences inside tcg_constant_folding.
---
 tcg/optimize.c |  822 +---
 1 files changed, 419 insertions(+), 403 deletions(-)

diff --git a/tcg/optimize.c b/tcg/optimize.c
index 2cdcc29..9768043 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -32,63 +32,124 @@
 #include "tcg-op.h"
 
 typedef enum {
-TCG_TEMP_UNDEF = 0,
+TCG_TEMP_VAR = 0,
 TCG_TEMP_CONST,
 TCG_TEMP_COPY,
-TCG_TEMP_ANY
 } tcg_temp_state;
 
 struct tcg_temp_info {
 tcg_temp_state state;
-uint16_t num_copies;
+uint16_t prev_copy;
+uint16_t next_copy;
 tcg_target_ulong val;
 };
 
-/* Reset TEMP's state to TCG_TEMP_ANY.  If TEMP was a representative of some
-   class of equivalent temp's, a new representative should be chosen in this
-   class. */
-static void reset_temp(struct tcg_temp_info *temps, TCGArg temp, int nb_temps,
-   int nb_globals)
+/* Array TEMPS has an element for each temp.
+   If this temp holds a constant then its value is kept in .VAL.
+   If this temp is a copy of other ones then this equivalence class'
+   representative is kept in .VAL.  */
+struct tcg_temp_info temps[TCG_MAX_TEMPS];
+
+/* Avoid some of the rampant if-deffery related to 32 vs 64-bit operations.  */
+#if TCG_TARGET_REG_BITS == 64
+#define CASE_OP_32_64(x)\
+glue(glue(case INDEX_op_, x), _i32):\
+glue(glue(case INDEX_op_, x), _i64)
+#else
+#define CASE_OP_32_64(x)\
+glue(glue(case INDEX_op_, x), _i32)
+#endif
+
+
+/* Reset all temporaries to TCG_TEMP_VAR.  */
+static void reset_all_temps(int nb_temps)
 {
 int i;
-TCGArg new_base;
-if (temps[temp].state == TCG_TEMP_COPY) {
-temps[temps[temp].val].num_copies--;
+for (i = 0; i < nb_temps; ++i) {
+temps[i].state = TCG_TEMP_VAR;
+temps[i].prev_copy = i;
+temps[i].next_copy = i;
 }
-if (temps[temp].num_copies > 0) {
-new_base = (TCGArg)-1;
-for (i = nb_globals; i < nb_temps; i++) {
-if (temps[i].state == TCG_TEMP_COPY && temps[i].val == temp) {
-if (new_base == ((TCGArg)-1)) {
-new_base = (TCGArg)i;
-temps[i].state = TCG_TEMP_ANY;
-temps[i].num_copies = 0;
-} else {
-temps[i].val = new_base;
-temps[new_base].num_copies++;
-}
+}
+
+/* Reset T's state to TCG_TEMP_VAR.  If T was a representative of some
+   class of equivalent temp's, a new representative should be chosen
+   in this class. */
+static void reset_temp(int t)
+{
+int prev = temps[t].prev_copy;
+int next = temps[t].next_copy;
+
+switch (temps[t].state) {
+case TCG_TEMP_VAR:
+/* If next (and prev) equals temp, that means there are no copies.
+   Otherwise, promote NEXT to be the new representative of the
+   equivalence class.  */
+if (next != t) {
+int i;
+temps[next].state = TCG_TEMP_VAR;
+for (i = temps[next].next_copy; i != t; i = temps[i].next_copy) {
+temps[i].val = next;
 }
 }
-for (i = 0; i < nb_globals; i++) {
-if (temps[i].stat

Re: [Qemu-devel] [PATCH v2 5/6] Do constant folding for shift operations.

[Richard Henderson]

On 06/09/2011 03:45 AM, Kirill Batuzov wrote:
> +case INDEX_op_shl_i32:
> +#if TCG_TARGET_REG_BITS == 64
> +y &= 0x;
> +case INDEX_op_shl_i64:
> +#endif
> +return x << y;
> +
> +case INDEX_op_shr_i32:
> +#if TCG_TARGET_REG_BITS == 64
> +x &= 0x;
> +y &= 0x;
> +case INDEX_op_shr_i64:
> +#endif
> +/* Assuming TCGArg to be unsigned */
> +return x >> y;

Don't assume when you've got a uint64_t type readily available.

> +case INDEX_op_sar_i32:
> +#if TCG_TARGET_REG_BITS == 64
> +x &= 0x;
> +y &= 0x;
> +#endif
> +return (int32_t)x >> (int32_t)y;

Masks are redundant with the casts.

> +case INDEX_op_rotr_i32:
> +#if TCG_TARGET_REG_BITS == 64
> +x &= 0x;
> +y &= 0x;
> +#endif
> +x = (x << (32 - y)) | (x >> y);

Have you looked to see if this gets recognized as a rotate
by the compiler?  I suspect that it will if you use a cast
to uint32_t here, but not if it is left as a 64-bit TCGArg.

> +#if TCG_TARGET_REG_BITS == 64
> +case INDEX_op_rotl_i64:
> +x = (x << y) | (x >> (64 - y));
> +return x;
> +#endif

Likewise it's probably best to cast to uint64_t here.


r~

Re: [Qemu-devel] [PATCH v2 6/6] Do constant folding for unary operations.

[Richard Henderson]

On 06/09/2011 03:45 AM, Kirill Batuzov wrote:
> +case INDEX_op_ext8s_i32:
> +return (int32_t)(int8_t)x;
> +
> +case INDEX_op_ext16s_i32:
> +return (int32_t)(int16_t)x;

No need to cast back to a 32-bit type.  They'll be
extended properly for the return type which is TCGArg.
And if you drop these intermediate casts, you can
merge the 32 and 64-bit copies.


r~

Re: [Qemu-devel] [PATCH v2 3/6] Do constant folding for basic arithmetic operations.

[Richard Henderson]

On 06/09/2011 03:45 AM, Kirill Batuzov wrote:
> +static int op_to_mov(int op)
> +{
> +if (op_bits(op) == 32) {
> +return INDEX_op_mov_i32;
> +}
> +#if TCG_TARGET_REG_BITS == 64
> +if (op_bits(op) == 64) {
> +return INDEX_op_mov_i64;
> +}
> +#endif
> +tcg_abort();
> +}

Again, switch not two calls.

> +static TCGArg do_constant_folding(int op, TCGArg x, TCGArg y)
> +{
> +TCGArg res = do_constant_folding_2(op, x, y);
> +#if TCG_TARGET_REG_BITS == 64
> +if (op_bits(op) == 32) {
> +res &= 0x;

Strictly speaking, this isn't required.  The top bits of any
constant are considered garbage to the code generators.  C.f.
the code in tcg_out_movi for any 64-bit host.

That said, only x86_64 and s390 get this right for the constraints.
x86_64 by being able to use 'i' to accept all constants for 32-bit
operations, and s390 by using 'W' as a modifier to force 32-bit
comparison in tcg_target_const_match.

So it's probably best to keep this for now.

> +/* Simplify expression if possible. */
> +switch (op) {
> +case INDEX_op_add_i32:
> +#if TCG_TARGET_REG_BITS == 64
> +case INDEX_op_add_i64:
> +#endif
> +if (temps[args[1]].state == TCG_TEMP_CONST) {
> +/* 0 + x == x + 0 */
> +tmp = args[1];
> +args[1] = args[2];
> +args[2] = tmp;
> +}

Probably best to break this out into another switch so that
you can handle all of the commutative operations.

> +/* Fallthrough */
> +case INDEX_op_sub_i32:
> +#if TCG_TARGET_REG_BITS == 64
> +case INDEX_op_sub_i64:
> +#endif
> +if (temps[args[1]].state == TCG_TEMP_CONST) {
> +/* Proceed with possible constant folding. */
> +break;
> +}
> +if (temps[args[2]].state == TCG_TEMP_CONST
> +&& temps[args[2]].val == 0) {
> +if ((temps[args[0]].state == TCG_TEMP_COPY
> +&& temps[args[0]].val == args[1])
> +|| args[0] == args[1]) {
> +args += 3;
> +gen_opc_buf[op_index] = INDEX_op_nop;
> +} else {
> +reset_temp(temps, args[0], nb_temps, nb_globals);
> +if (args[1] >= s->nb_globals) {
> +temps[args[0]].state = TCG_TEMP_COPY;
> +temps[args[0]].val = args[1];
> +temps[args[1]].num_copies++;
> +}
> +gen_opc_buf[op_index] = op_to_mov(op);
> +gen_args[0] = args[0];
> +gen_args[1] = args[1];
> +gen_args += 2;
> +args += 3;
> +}
> +continue;
> +}
> +break;
> +case INDEX_op_mul_i32:
> +#if TCG_TARGET_REG_BITS == 64
> +case INDEX_op_mul_i64:
> +#endif
> +if ((temps[args[1]].state == TCG_TEMP_CONST
> + && temps[args[1]].val == 0)
> +|| (temps[args[2]].state == TCG_TEMP_CONST
> +&& temps[args[2]].val == 0)) {
> +reset_temp(temps, args[0], nb_temps, nb_globals);
> +temps[args[0]].state = TCG_TEMP_CONST;
> +temps[args[0]].val = 0;
> +assert(temps[args[0]].num_copies == 0);
> +gen_opc_buf[op_index] = op_to_movi(op);
> +gen_args[0] = args[0];
> +gen_args[1] = 0;
> +args += 3;
> +gen_args += 2;
> +continue;
> +}

This is *way* too much code duplication, particularly with the
same code sequences already existing for mov and movi and more
to come with the logicals.


r~

Re: [Qemu-devel] [PATCH v2 2/6] Add copy and constant propagation.

[Richard Henderson]

On 06/09/2011 03:45 AM, Kirill Batuzov wrote:
> Make tcg_constant_folding do copy and constant propagation. It is a
> preparational work before actual constant folding.
> 
> Signed-off-by: Kirill Batuzov 
> ---
>  tcg/optimize.c |  161 
> 
>  1 files changed, 161 insertions(+), 0 deletions(-)
> 
> diff --git a/tcg/optimize.c b/tcg/optimize.c
> index 5daf131..7996798 100644
> --- a/tcg/optimize.c
> +++ b/tcg/optimize.c
> @@ -31,23 +31,178 @@
>  #include "qemu-common.h"
>  #include "tcg-op.h"
>  
> +typedef enum {
> +TCG_TEMP_UNDEF = 0,
> +TCG_TEMP_CONST,
> +TCG_TEMP_COPY,
> +TCG_TEMP_ANY
> +} tcg_temp_state;

Coding conventions request StudlyCaps, fwiw.

> +
> +struct tcg_temp_info {
> +tcg_temp_state state;
> +uint16_t num_copies;
> +tcg_target_ulong val;
> +};
> +
> +/* Reset TEMP's state to TCG_TEMP_ANY.  If TEMP was a representative of some
> +   class of equivalent temp's, a new representative should be chosen in this
> +   class. */
> +static void reset_temp(struct tcg_temp_info *temps, TCGArg temp, int 
> nb_temps,
> +   int nb_globals)
> +{
> +int i;
> +TCGArg new_base;
> +if (temps[temp].state == TCG_TEMP_COPY) {
> +temps[temps[temp].val].num_copies--;
> +}
> +if (temps[temp].num_copies > 0) {
> +new_base = (TCGArg)-1;
> +for (i = nb_globals; i < nb_temps; i++) {

I think it's probably better to place the elements of the equiv class into
a double-linked circular list, rather than a mere num_copies that forces
you to iterate over nb_temps to remove an element.  E.g.

  struct tcg_temp_info {
tcg_temp_state state;
uint16_t prev_copy;
uint16_t next_copy;
tcg_target_ulong val;
  };

This makes elements easy to remove, and easy to choose a new representative.

> +static int op_bits(int op)
> +{
> +switch (op) {
> +case INDEX_op_mov_i32:
> +return 32;
> +#if TCG_TARGET_REG_BITS == 64
> +case INDEX_op_mov_i64:
> +return 64;
> +#endif
> +default:
> +fprintf(stderr, "Unrecognized operation %d in op_bits.\n", op);
> +tcg_abort();
> +}
> +}

I think we would be well-served to move this to a property of the opcode,
much the same way as TCG_OPF_CALL_CLOBBER et al.  I would not, of course,
suggest to block this patch series on that cleanup.

> +static int op_to_movi(int op)
> +{
> +if (op_bits(op) == 32) {
> +return INDEX_op_movi_i32;
> +}
> +#if TCG_TARGET_REG_BITS == 64
> +if (op_bits(op) == 64) {
> +return INDEX_op_movi_i64;
> +}
> +#endif
> +tcg_abort();
> +}

This should use a switch, not two calls to a function.

> +struct tcg_temp_info temps[TCG_MAX_TEMPS];

Given that gen_opc_buf is static, I see no reason not to make this a
static variable as well.  Put it at file scope so you don't have to
pass it as arguments to subroutines.

> +/* Do copy propagation */
> +if (!(def->flags & (TCG_OPF_CALL_CLOBBER | TCG_OPF_SIDE_EFFECTS))) {

Why are you suppressing copy propagation in this way?  I see no reason for it.

> +assert(op != INDEX_op_call);
> +for (i = def->nb_oargs; i < def->nb_oargs + def->nb_iargs; i++) {
> +if (temps[args[i]].state == TCG_TEMP_COPY
> +&& !(def->args_ct[i].ct & TCG_CT_IALIAS)

>From looking at only ARM translator output, you might suppose that we've
already performed matching constraint substitution.  But you'd be wrong.

I realize that at present you get a smidge smaller code by suppressing
substitution around matching constraints, but that's only because our
copy propagation is incomplete.  From a pure theoretic stance, I think
this line is wrong.  E.g.

 With IALIAS suppression Without
  0x40802e50  0x40802e50
 mov_i32 tmp5,r10|   nopn $0x2,$0x2
 movi_i32 tmp6,$0x40802e58   movi_i32 tmp6,$0x40802e58
 add_i32 tmp6,tmp5,tmp6  |   add_i32 tmp6,r10,tmp6
 mov_i32 r10,tmp6mov_i32 r10,tmp6

I'll claim that that the right-hand column is better, even though it
currently forces the generation of an LEA insn instead of an ADD.

What's needed to fix this is either (1) a much better register allocator
or (2) a reverse copy-propagation pass that pushes global variables up
into outputs containing temporaries.

> +&& (def->args_ct[i].ct & TCG_CT_REG)) {

This line looks redundant.  Don't we assert this property
in tcg_add_target_add_op_defs?  Certainly elsewhere we expect all
arguments to be able to accept a register...

> +case INDEX_op_mov_i32:
> +#if TCG_TARGET_REG_BITS == 64
> +case INDEX_op_mov_i64:
> +#endif

I suggest we take a page from tcg/sparc/tcg-target.c:

#if TCG_TARGET_REG_BITS == 64
#define CASE_OP_32_64(x)\
glue(glue(case INDEX_op_, x), _i32):\
glue(glue(case INDEX

Re: [Qemu-devel] [PATCH 12/14] linux-user: syscall should use sanitized arg1

[Peter Maydell]

On 2 June 2011 12:53, Juan Quintela  wrote:
> Looking at the other architectures, we should be using "how" not "arg1".
>
> Signed-off-by: Juan Quintela 

OK as far as it goes, but I think we should also change the
  int how = arg1;

to just 'int how;' while we're cleaning up this chunk of code.

-- PMM

Re: [Qemu-devel] [PATCH 05/14] linuxload: id_change was a write only variable

[Peter Maydell]

On 2 June 2011 12:53, Juan Quintela  wrote:
>
> Signed-off-by: Juan Quintela 

Reviewed-by: Peter Maydell 

It does seem a bit odd that we were carefully calculating
this flag and then ignoring it, but I guess we just have
to treat the reason as lost in the mists of time (as you say,
according to the git history it has always been this way..)

-- PMM

Re: [Qemu-devel] [PATCH 08/14] syscall: really return ret code

[Peter Maydell]

On 2 June 2011 12:53, Juan Quintela  wrote:
> We assign ret with the error code, but then return 0 unconditionally.
>
> Signed-off-by: Juan Quintela 

Reviewed-by: Peter Maydell 

-- PMM

Re: [Qemu-devel] [PATCH 1/3] lsi: Fix unused-but-set-variable warning

[Peter Maydell]

Stefan, if you're picking up gcc warning fixes for the trivial
tree, how about this one?

(I think the other gcc warning fixes are either:
 * in the most recent usb pull request
 * linux-user
 * target-alpha
which all have a clear owner/interested person.)

Reviewed-by: Peter Maydell 

-- PMM

On 1 June 2011 15:19, Paolo Bonzini  wrote:
> On 06/01/2011 02:56 PM, Christophe Fergeau wrote:
>>
>> This warning is new in gcc 4.6.
>>
>> Signed-off-by: Christophe Fergeau
>> ---
>>  hw/lsi53c895a.c |    2 --
>>  1 files changed, 0 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
>> index 83084b6..90c6cbc 100644
>> --- a/hw/lsi53c895a.c
>> +++ b/hw/lsi53c895a.c
>> @@ -889,7 +889,6 @@ static void lsi_do_msgout(LSIState *s)
>>      uint8_t msg;
>>      int len;
>>      uint32_t current_tag;
>> -    SCSIDevice *current_dev;
>>      lsi_request *current_req, *p, *p_next;
>>      int id;
>>
>> @@ -901,7 +900,6 @@ static void lsi_do_msgout(LSIState *s)
>>          current_req = lsi_find_by_tag(s, current_tag);
>>      }
>>      id = (current_tag>>  8)&  0xf;
>> -    current_dev = s->bus.devs[id];
>>
>>      DPRINTF("MSG out len=%d\n", s->dbc);
>>      while (s->dbc) {
>
> Acked-by: Paolo Bonzini 

Re: [Qemu-devel] [PATCH RFC 0/3] basic support for composing sysbus devices

[Andreas Färber]

Am 10.06.2011 um 14:51 schrieb Anthony Liguori:

The trouble is that I don't think we have a reasonable way to refer  
to properties of other devices and we don't have names for all  
devices.  I think if we fix the later problem, the former problem  
becomes easier.


For the former issue I sent a patch "qdev: Add helpers for reading  
properties" yesterday:


http://patchwork.ozlabs.org/patch/99536/

That allows to access properties of arbitrary DeviceState by name,  
e.g., qdev_prop_get_uint32(dev, "irq").


It currently depends on the open question of how to model bool  
properties but I can reorder the two if needed.


Andreas

Re: [Qemu-devel] [PATCH 0/3] block: Avoid direct AIO callback

[Kevin Wolf]

Am 10.06.2011 17:32, schrieb Luiz Capitulino:
> On Tue,  7 Jun 2011 16:18:30 +0200
> Kevin Wolf  wrote:
> 
>> This series fixes some cases of block drivers calling AIO callbacks too 
>> early.
>> It fixes the IDE assertion failure reported by Luiz (in error cases, the DMA
>> status, including acb, could first be reset in the callback and only then be
>> set by the caller, resulting in a dangling acb and wrong status register 
>> value).
> 
> This fixes the reported bug, thanks.
> 
> I know this is a different subject, but I'm still unable to use the host cdrom
> if the -snapshot flag is passed, I think the idea of ignoring the flag for a
> read-only device would fix this, no?

Yes, it would. But as we discussed it would have other implications that
I wouldn't feel comfortable about (surprising semantics of 'commit'
would be one).

Passing things like bdrv_eject() or bdrv_is_inserted() to the backing
file still sounds like a cleaner approach, but probably isn't as easy.

Kevin

Re: [Qemu-devel] [PATCH RFC 0/3] basic support for composing sysbus devices

[Anthony Liguori]

On 06/10/2011 09:59 AM, Markus Armbruster wrote:

Anthony Liguori  writes:


On 06/10/2011 03:13 AM, Markus Armbruster wrote:

Jan Kiszka   writes:

Resource management, e.g. IRQs. That will be useful for other types of
buses as well.


A device should be able to say "I need to be connected to an IRQ line".
Feels generic to me.


More specifically, a device has input IRQs.  A device has no idea what
number the IRQ is tied to.

Devices may also have output IRQs.  At the qdev layer, we should be
able to connect an arbitrary output IRQ to an arbitrary input IRQ.

So the crux of the problem is that:

  -device isa-serial,id=serial,irq=3

Is very wrong.  It ought to look something more like

  -device piix3,id=piix3 -device isa-serial,id=serial,irq=piix3.irq[3]


As Jan pointed out, ISA is a counter-example: your "very wrong" claim is
actually wrong there :)


The wrongness comes from the fact that "irq=3" assumes that somehow the 
ISA device can get an IRQ from an index.


If instead the example was:

 -device piix3,id=isa0
 -device isa-serial,id=serial,irq=3,bus=isa0

With the implication isa-serial could use the "3" index to get an IRQ 
from it's bus at realize, like:


void isa_serial_initfn()
{
ISAController *bus = qdev_get_isa_controller(dev, "bus");
connect_irq(dev->irq, bus->irq[qdev_get_int(dev, "irq")]);
}

That is at least conceptually okay but I don't think it's best.  If you 
want to go this route, I'd actually model DIP switches or ISA plug and 
play.  Usually devices would only allow you to choose between two of the 
5 IRQs available.


But personally, the modelling I mentioned in another part of the thread 
where you connect the IRQs directly to the device seems like a better 
compromise to me.



An ISA device is always connected to all the ISA bus's interrupt lines.
Device configuration determines how the device uses these lines.


That's 100% correct.  But the "ISA bus" is just a grouping of interfaces 
to another device.  We shouldn't treat that as any more special than any 
other type of connection between devices.



The old (non-MSI) PCI interrupts are similar, I think.

Of course, "ISA IRQ 4" can be anything, depending on how the device
providing the ISA bus is wired up.


IRQ forwarding becomes very easy in this model because your composite
qdev device has a set of input IRQs and then routes the input IRQs to
the appropriate input IRQs of the sub devices.

The trouble is that I don't think we have a reasonable way to refer to
properties of other devices and we don't have names for all devices.
I think if we fix the later problem, the former problem becomes
easier.


We already connect devices to other resources, such as block, char and
network host parts.  The way we do it may not be "pure", but it works:
we define the plug as property, and connect it to its socket by saying
PROP-NAME=SOCK-NAME.  Note that the connection is made by core qdev;
device model code is oblivious of it.  It just uses it.


Yes.  The problem with this is namespaces.  What you are referring to as 
"SOCK-NAME" is interpreted based on the property type.  Ideally, we'd 
have a single namespace for everything.



  They can be replaced by having fixed "slots".
For instance, if you had a way of having a PCIDevice * property, the
I440FX could have 32 PCIDevice * properties.  That's how you would add
to a bus (and notice that it conveniently solves bus addressing in a
robust fashion).


Assumes that bus addresses are isomorphic to [0..N], doesn't it?

Not really the case for USB: we use a string of the form %d(.%d)* there.


Couple points there.

1) I didn't mean to suggest that the name "irq[3]" implies anything.  I 
sort of thing it should be just treated as a string.  It could as well 
be "irq[foo]".  I'm not 100% here though.


2) USB addressing is expressing an implicit topology of HUBs.  I think 
it's better to model those HUBs explicitly and leave convenience to 
syntactic sugar.  For instance:


 -device usb-uhci,id=uhci0,ports=4,port0=hub0
 -device usb-hub,id=hub0,ports=4,port1=tablet0
 -device usb-tablet,id=tablet0

Which is the expansion of:

 -tablet address=0.1


Not my idea.

A bus is just a standardized container for slots.  A single device can
provide multiple buses.  Killing buses just unwraps the slots.  You lose
the grouping.


The grouping is still there by virtue of the fact that the slots are all 
part of the same device.



What exactly is so very wrong about buses that they need to die?


They force a device tree.  The device model shouldn't be a tree, but a 
directed graph.  It's perfectly fine to have a type called PCIBus that 
I440FX extends, but qdev shouldn't have explicit knowledge of something 
called a "bus" IMHO.  Doing this forces a limited mechanism of 
connecting devices because it creates an artificial tree (by implying a 
parent/child relationship).  It makes composition difficult if not 
impossible.


Regards,

Anthony Liguori


Honest, non-rhetorical 

Re: [Qemu-devel] gtester questions/issues

[Michael Roth]

On 06/10/2011 09:55 AM, Luiz Capitulino wrote:

On Thu, 09 Jun 2011 18:04:44 -0500
Michael Roth  wrote:


On 06/09/2011 03:02 PM, Luiz Capitulino wrote:

On Thu, 09 Jun 2011 14:04:37 -0500
Anthony Liguori   wrote:


On 06/09/2011 01:47 PM, Luiz Capitulino wrote:


I've started writing some tests with the glib test framework (used by the qapi
patches) but am facing some issues that doesn't seem to exist with check (our
current framework).

Of course that it's possible that I'm missing something, in this case pointers
are welcome, but I must admit that my first impression wasn't positive.

1. Catching test abortion

By default check runs each test on a separate process, this way it's able to
catch any kind of abortion (such as an invalid pointer deference) and it
prints a very developer friendly message:

Running suite(s): Memory module test suite
0%: Checks: 1, Failures: 0, Errors: 1
check-memory.c:20:E:Memory API:test_read_write_byte_simple:33: (after this 
point) Received signal 11 (Segmentation fault)

The glib suite doesn't seem to do that, at least not by default, so this is
what you get on an invalid pointer:

~/src/qmp-unstable/build (qapi-review)/ ./test-visiter2
/qapi/visitor/input/int: Segmentation fault (core dumped)
~/src/qmp-unstable/build (qapi-review)/

Is it possible to have check's functionality someway? I read about the
g_test_trap_fork() function, but one would have to use it manually in
each test case, this is a no-no.


I think this is a personal preference thing.  I think having fork() be
optional is great because it makes it easier to use common state for
multiple test cases.


Coupling test-cases like this is almost always a bad thing. Test-cases have
to be independent from each other so that they can be run and debugged
individually, also a failing test won't bring the whole suite down, as this
makes a failing report useless.

That said, you can still do this sharing without sacrificing essential features.
Like disabling the fork mode altogether or subdividing test cases.

Anyway, If there's a non-ultra cumbersome way to use g_test_trap_fork() (or any
other workaround) to catch segfaults and abortions, then fine. Otherwise I
consider this a blocker, as any code we're going to test in qemu can possibly
crash. This is really a very basic feature that a C unit-test framework can
offer.



You kind of get the desired behavior if you run the test via something like:

gtester -k -o test.xml test-visiter

The gtester utility will log the return code after a test bombs, then
restart and skip to the test following the one that bombed. And I'm sure
gtester-report can process the resulting test.xml in manner similar to
check...


Ok, that makes the problem less worse and I agree it's possible to cook
a workaround for it. But IMO, glib's test framework is flawed. You just
can't require developers to run two additional utilities and dump xml so
that they can know a particular test exploded.

The argument that qemu will be linked against glib is a valid one. But I
really think we're changing for the worse here, and this can compromise
all the plans on focusing on more unit-tests. What's the point in investing
time in writing and maintaining unit-tests if they can get as difficult
as the VM itself to be debugged?



The unit tests would only be slightly more difficult to debug if used 
interactively. You'd still be able to tell what test failed, you just 
wouldn't see failure beyond that without some extra work. So long as we 
don't carry unit test failures for extended periods this shouldn't hurt 
anyone's workflow too much. I agree there appears to be a little bit of 
a trade-off, but being able to do a make check by default on every build 
is a big gain.


And for those automated tests the gtester utilities should make it easy 
to work around. The XML seems sane enough.



unfortunately it appears to be broken for me on Ubuntu 10.04 so
here's the raw XML dump for reference:


Yes, there's this one too and the memory leak.






  
  R02S13c4d9e6d35c23e8dd988917863a66b1
  
0.000346

  
  
0.00

  
  0.015056


  
  R02S7acda18e321c5a41ccaee4f524877343
  
  
  

ERROR:/home/mdroth/w/qemu2.git/test-visiter.c:312:test_epic_fail2: 
assertion
failed: (false)
0.00

  
  0.006355


  
  R02S73a208dd8f1b127c23b6a7883df9b78f
  
  
  
  
0.000318

  
  
0.36

  
  
0.59

  
  0.008079



XML or HTML...it's not pretty, but we can make use of it for automated
tests. And for interactive use I don't think it's as much a problem
since that'll for the most part be developers making sure they didn't
break any tests before committing, or working on failures picked up by
automated runs: not a big deal in those cases if the unit tests stop at
the first ab

Re: [Qemu-devel] [PATCH 0/3] block: Avoid direct AIO callback

[Luiz Capitulino]

On Tue,  7 Jun 2011 16:18:30 +0200
Kevin Wolf  wrote:

> This series fixes some cases of block drivers calling AIO callbacks too early.
> It fixes the IDE assertion failure reported by Luiz (in error cases, the DMA
> status, including acb, could first be reset in the callback and only then be
> set by the caller, resulting in a dangling acb and wrong status register 
> value).

This fixes the reported bug, thanks.

I know this is a different subject, but I'm still unable to use the host cdrom
if the -snapshot flag is passed, I think the idea of ignoring the flag for a
read-only device would fix this, no?

> 
> Kevin Wolf (3):
>   qcow2: Avoid direct AIO callback
>   qcow: Avoid direct AIO callback
>   vdi: Avoid direct AIO callback
> 
>  block/qcow.c  |   58 +++-
>  block/qcow2.c |   39 +
>  block/vdi.c   |   40 +++---
>  3 files changed, 122 insertions(+), 15 deletions(-)
> 

Re: [Qemu-devel] [PATCH 06/14] flatload: end_code was only used in a debug message

[Peter Maydell]

On 2 June 2011 12:53, Juan Quintela  wrote:
> -    end_code = textpos + text_len;
>
>     DBG_FLT("%s %s: TEXT=%x-%x DATA=%x-%x BSS=%x-%x\n",
>             id ? "Lib" : "Load", bprm->filename,
> -            (int) start_code, (int) end_code,
> +            (int) start_code, (int) textpos + text_lon,

Typo here, should be "text_len", not "text_lon".

-- PMM

Re: [Qemu-devel] QEMU 9pfs intentionally returning short reads ?

[Venkateswararao Jujjuri]

On 06/10/2011 05:20 AM, Daniel P. Berrange wrote:

On Fri, Jun 10, 2011 at 05:36:13PM +0530, Aneesh Kumar K.V wrote:

On Fri, 10 Jun 2011 11:33:05 +0100, "Daniel P. Berrange"  
wrote:

I've been doing some work trying to run QEMU guests with a root filesystem
exported from the host using virtio 9pfs. One of the issues that I have
discovered is that the 9p FS running on QEMU appears to cap all reads at
4096 bytes[1]. Any larger read will return only partial data for plain
files.


But we should loop in kernel, requesting for multiple 9p request.

kernel does

size = fid->iounit ? fid->iounit : fid->clnt->msize - P9_IOHDRSZ;
if (count>  size)
ret = v9fs_file_readn(filp, NULL, udata, count, *offset);
else
ret = p9_client_read(fid, NULL, udata, *offset, count);

and v9fs_file_readn() does..

do {
n = p9_client_read(fid, data, udata, offset, count);
if (n<= 0)
break;

if (data)
data += n;
if (udata)
udata += n;

offset += n;
count -= n;
total += n;
} while (count>  0&&  n == size);


I also did an strace of simple test and i see

open("test", O_RDONLY)  = 3
read(3, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 
8192) = 8192

In my test I did

#  strace -e trace=read,open perl -e 'open FOO, "/usr/share/X11/XKeysymDB"; 
sysread FOO, $foo, 8192'
open("/usr/share/X11/XKeysymDB", O_RDONLY) = 3
read(3, "! Copyright 1993 Massachusetts I"..., 8192) = 4096

Perhaps there is a guest kernel driver difference ? I'm using

   2.6.35.13-91.fc14.x86_64
The default msize is 8k; Because of iounit the net layer need to send 
the IO in the chunks of 4k
8k - header size faults into 4k; But never the less the fs layer should 
loop.


There was an issue with read; and that bug is fixed by
commit 97e8442b0971ea6be9a495b3d03402985cfe5d6a
Author: M. Mohan Kumar 
Date:   Fri Jun 4 11:59:07 2010 +

9p: Make use of iounit for read/write

Change the v9fs_file_readn function to limit the maximum transfer size
based on the iounit or msize.

Also remove the redundant check for limiting the transfer size in
v9fs_file_write. This check is done by p9_client_write.

Signed-off-by: M. Mohan Kumar 
Signed-off-by: Eric Van Hensbergen 



Thanks,
JV

Regards,
Daniel

Re: [Qemu-devel] gtester questions/issues

[Luiz Capitulino]

On Fri, 10 Jun 2011 10:05:17 -0500
Anthony Liguori  wrote:

> On 06/10/2011 09:55 AM, Luiz Capitulino wrote:
> > On Thu, 09 Jun 2011 18:04:44 -0500
> >> You kind of get the desired behavior if you run the test via something 
> >> like:
> >>
> >> gtester -k -o test.xml test-visiter
> >>
> >> The gtester utility will log the return code after a test bombs, then
> >> restart and skip to the test following the one that bombed. And I'm sure
> >> gtester-report can process the resulting test.xml in manner similar to
> >> check...
> >
> > Ok, that makes the problem less worse and I agree it's possible to cook
> > a workaround for it. But IMO, glib's test framework is flawed. You just
> > can't require developers to run two additional utilities and dump xml so
> > that they can know a particular test exploded.
> 
> It all happens automagically during make check.  I don't understand what 
> the problem here is.

That's the "I agree it's possible to cook a workaround for it" part. But of
course that we have to fix gtester-report first. It doesn't work today and it
only knows how to dump HTML.

Re: [Qemu-devel] gtester questions/issues

[Anthony Liguori]

On 06/10/2011 09:55 AM, Luiz Capitulino wrote:

On Thu, 09 Jun 2011 18:04:44 -0500

You kind of get the desired behavior if you run the test via something like:

gtester -k -o test.xml test-visiter

The gtester utility will log the return code after a test bombs, then
restart and skip to the test following the one that bombed. And I'm sure
gtester-report can process the resulting test.xml in manner similar to
check...


Ok, that makes the problem less worse and I agree it's possible to cook
a workaround for it. But IMO, glib's test framework is flawed. You just
can't require developers to run two additional utilities and dump xml so
that they can know a particular test exploded.


It all happens automagically during make check.  I don't understand what 
the problem here is.


Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH RFC 0/3] basic support for composing sysbus devices

[Markus Armbruster]

Anthony Liguori  writes:

> On 06/10/2011 03:13 AM, Markus Armbruster wrote:
>> Jan Kiszka  writes:
>>> Resource management, e.g. IRQs. That will be useful for other types of
>>> buses as well.
>>
>> A device should be able to say "I need to be connected to an IRQ line".
>> Feels generic to me.
>
> More specifically, a device has input IRQs.  A device has no idea what
> number the IRQ is tied to.
>
> Devices may also have output IRQs.  At the qdev layer, we should be
> able to connect an arbitrary output IRQ to an arbitrary input IRQ.
>
> So the crux of the problem is that:
>
>  -device isa-serial,id=serial,irq=3
>
> Is very wrong.  It ought to look something more like
>
>  -device piix3,id=piix3 -device isa-serial,id=serial,irq=piix3.irq[3]

As Jan pointed out, ISA is a counter-example: your "very wrong" claim is
actually wrong there :)

An ISA device is always connected to all the ISA bus's interrupt lines.
Device configuration determines how the device uses these lines.

The old (non-MSI) PCI interrupts are similar, I think.

Of course, "ISA IRQ 4" can be anything, depending on how the device
providing the ISA bus is wired up.

> IRQ forwarding becomes very easy in this model because your composite
> qdev device has a set of input IRQs and then routes the input IRQs to
> the appropriate input IRQs of the sub devices.
>
> The trouble is that I don't think we have a reasonable way to refer to
> properties of other devices and we don't have names for all devices.
> I think if we fix the later problem, the former problem becomes
> easier.

We already connect devices to other resources, such as block, char and
network host parts.  The way we do it may not be "pure", but it works:
we define the plug as property, and connect it to its socket by saying
PROP-NAME=SOCK-NAME.  Note that the connection is made by core qdev;
device model code is oblivious of it.  It just uses it.

[...]
>> I doubt all resources are as generic as IRQ lines, but that's okay.
>
> They pretty much are.  We're really just talking about referring to
> subcomponents of a device.  That's what's lacking today.
>
>> Device component composition is not (yet?) covered by qdev.  Of course
>> we compose anyway, in various ad hoc ways.
>
> Busses need to die.

Well, I wish I was as certain as you about about what is very wrong,
what needs to die, and what needs to be done instead.

>  They can be replaced by having fixed "slots".
> For instance, if you had a way of having a PCIDevice * property, the
> I440FX could have 32 PCIDevice * properties.  That's how you would add
> to a bus (and notice that it conveniently solves bus addressing in a
> robust fashion).

Assumes that bus addresses are isomorphic to [0..N], doesn't it?

Not really the case for USB: we use a string of the form %d(.%d)* there.
Not my idea.

A bus is just a standardized container for slots.  A single device can
provide multiple buses.  Killing buses just unwraps the slots.  You lose
the grouping.

What exactly is so very wrong about buses that they need to die?
Honest, non-rhetorical question.

Re: [Qemu-devel] gtester questions/issues

[Luiz Capitulino]

On Thu, 09 Jun 2011 18:04:44 -0500
Michael Roth  wrote:

> On 06/09/2011 03:02 PM, Luiz Capitulino wrote:
> > On Thu, 09 Jun 2011 14:04:37 -0500
> > Anthony Liguori  wrote:
> >
> >> On 06/09/2011 01:47 PM, Luiz Capitulino wrote:
> >>>
> >>> I've started writing some tests with the glib test framework (used by the 
> >>> qapi
> >>> patches) but am facing some issues that doesn't seem to exist with check 
> >>> (our
> >>> current framework).
> >>>
> >>> Of course that it's possible that I'm missing something, in this case 
> >>> pointers
> >>> are welcome, but I must admit that my first impression wasn't positive.
> >>>
> >>> 1. Catching test abortion
> >>>
> >>> By default check runs each test on a separate process, this way it's able 
> >>> to
> >>> catch any kind of abortion (such as an invalid pointer deference) and it
> >>> prints a very developer friendly message:
> >>>
> >>>Running suite(s): Memory module test suite
> >>>0%: Checks: 1, Failures: 0, Errors: 1
> >>>check-memory.c:20:E:Memory API:test_read_write_byte_simple:33: (after 
> >>> this point) Received signal 11 (Segmentation fault)
> >>>
> >>> The glib suite doesn't seem to do that, at least not by default, so this 
> >>> is
> >>> what you get on an invalid pointer:
> >>>
> >>>~/src/qmp-unstable/build (qapi-review)/ ./test-visiter2
> >>>/qapi/visitor/input/int: Segmentation fault (core dumped)
> >>>~/src/qmp-unstable/build (qapi-review)/
> >>>
> >>> Is it possible to have check's functionality someway? I read about the
> >>> g_test_trap_fork() function, but one would have to use it manually in
> >>> each test case, this is a no-no.
> >>
> >> I think this is a personal preference thing.  I think having fork() be
> >> optional is great because it makes it easier to use common state for
> >> multiple test cases.
> >
> > Coupling test-cases like this is almost always a bad thing. Test-cases have
> > to be independent from each other so that they can be run and debugged
> > individually, also a failing test won't bring the whole suite down, as this
> > makes a failing report useless.
> >
> > That said, you can still do this sharing without sacrificing essential 
> > features.
> > Like disabling the fork mode altogether or subdividing test cases.
> >
> > Anyway, If there's a non-ultra cumbersome way to use g_test_trap_fork() (or 
> > any
> > other workaround) to catch segfaults and abortions, then fine. Otherwise I
> > consider this a blocker, as any code we're going to test in qemu can 
> > possibly
> > crash. This is really a very basic feature that a C unit-test framework can
> > offer.
> >
> 
> You kind of get the desired behavior if you run the test via something like:
> 
> gtester -k -o test.xml test-visiter
> 
> The gtester utility will log the return code after a test bombs, then 
> restart and skip to the test following the one that bombed. And I'm sure 
> gtester-report can process the resulting test.xml in manner similar to 
> check...

Ok, that makes the problem less worse and I agree it's possible to cook
a workaround for it. But IMO, glib's test framework is flawed. You just
can't require developers to run two additional utilities and dump xml so
that they can know a particular test exploded.

The argument that qemu will be linked against glib is a valid one. But I
really think we're changing for the worse here, and this can compromise
all the plans on focusing on more unit-tests. What's the point in investing
time in writing and maintaining unit-tests if they can get as difficult
as the VM itself to be debugged?

> unfortunately it appears to be broken for me on Ubuntu 10.04 so 
> here's the raw XML dump for reference:

Yes, there's this one too and the memory leak.

> 
> 
> 
>
>  
>  R02S13c4d9e6d35c23e8dd988917863a66b1
>  
>0.000346
>
>  
>  
>0.00
>
>  
>  0.015056
>
>
>  
>  R02S7acda18e321c5a41ccaee4f524877343
>  
>  
>  
>  
> ERROR:/home/mdroth/w/qemu2.git/test-visiter.c:312:test_epic_fail2: 
> assertion 
> failed: (false)
>0.00
>
>  
>  0.006355
>
>
>  
>  R02S73a208dd8f1b127c23b6a7883df9b78f
>  
>  
>  
>  
>0.000318
>
>  
>  
>0.36
>
>  
>  
>0.59
>
>  
>  0.008079
>
> 
> 
> XML or HTML...it's not pretty, but we can make use of it for automated 
> tests. And for interactive use I don't think it's as much a problem 
> since that'll for the most part be developers making sure they didn't 
> break any tests before committing, or working on failures picked up by 
> automated runs: not a big deal in those cases if the unit tests stop at 
> the first abort.

I hope you're not saying we're going to live with an XML output. I don't even
consider having to read XML as test output. I'm under the assumption that
we'll get this fixed in glib.

Re: [Qemu-devel] [PATCH RFC 0/3] basic support for composing sysbus devices

[Anthony Liguori]

On 06/10/2011 09:22 AM, Markus Armbruster wrote:

Peter Maydell  writes:


But I think that's a non-typical case compared to the usual one
of "these wires are just hardwired this way by the machine".


IIRC, the PCI bus also provides a number of IRQ lines for the device to
tickle (INTA#..INTD#).  There's rarely a need to configure their use,
though.


Right.  So a PCI bus has 4 Pins (ignoring MSI) that a device can connect to.

This is a case where Pin is the wrong interface to model because the 
device decides which LNK to use.  We don't want to model every single 
wire with a 32-bit PCI bus such that we have to make dozens of 
connections.  We want to collapse all into a single higher level interface:


So you want something like:

 -device i440fx,id=pcibus,slot[3]=blk0
 -device virtio-pci-blk,id=blk0,bus=pcibus

And then when the virtio-pci-blk device is "realized" (qdev_initfn), it 
should use it's bus property and connect to the appropriate LNKs.


You could do the same for ISA, but since it's so common to have DIP 
switches on non-PnP ISA devices, and we call things ISA that aren't 
strictly ISA (yeah, yeah, let's not have this discussion again), 
actually wiring up the IRQs by hand instead of having it negotiated 
through the bus interface makes sense.


But note that the use of "bus" here is just a name.  It does not 
implement the notion of bus that qdev does and says nothing about device 
hierarchy.


Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH RFC 0/3] basic support for composing sysbus devices

[Anthony Liguori]

On 06/10/2011 09:18 AM, Jan Kiszka wrote:

On 2011-06-10 16:12, Anthony Liguori wrote:

On 06/10/2011 08:43 AM, Jan Kiszka wrote:
   -device piix3,id=piix3 -device
isa-serial,id=serial,irq[3]=piix3.irq[3],irq[4]=piix3.irq[4],...

But I don't think we benefit from modelling it this correctly.  The
point is that the infrastructure could handle it though.


I don't see the point of 'piix3.' in your IRQ assignment, though. It is
redundant to the bus assignment


Bus assignment is the problem.

IRQ routing cannot be tied strictly to buses if we want to be able to do 
composition.


You could do clever things syntactically but the general mechanism is 
needed is the ability to wire things in an arbitrary fashion.


Regards,

Anthony Liguori

 - as the ISA bus also routes interrupts.

There are surely also buses that don't. Still, interrupt routing via the
bus should be possible (as it avoids boilerplate code or configuration
redundancy).

Jan

Re: [Qemu-devel] virtio scsi host draft specification, v3

[Paolo Bonzini]

> If requests are placed on arbitrary queues you'll inevitably run on
> locking issues to ensure strict request ordering.
> I would add here:
> 
> If a device uses more than one queue it is the responsibility of the
> device to ensure strict request ordering.

Applied with s/device/guest/g.

> Please do not rely in bus/target/lun here. These are leftovers from
> parallel SCSI and do not have any meaning on modern SCSI
> implementation (eg FC or SAS). Rephrase that to
> 
> The lun field is the Logical Unit Number as defined in SAM.

Ok.

> >  The status byte is written by the device to be the SCSI status
> >  code.
>
> ?? I doubt that exists. Make that:
> 
> The status byte is written by the device to be the status code as
> defined in SAM.

Ok.

> >  The response byte is written by the device to be one of the
> >  following:
> >
> >  - VIRTIO_SCSI_S_OK when the request was completed and the
> >  status byte
> >is filled with a SCSI status code (not necessarily "GOOD").
> >
> >  - VIRTIO_SCSI_S_UNDERRUN if the content of the CDB requires
> >  transferring
> >more data than is available in the data buffers.
> >
> >  - VIRTIO_SCSI_S_ABORTED if the request was cancelled due to a
> >  reset
> >or another task management function.
> >
> >  - VIRTIO_SCSI_S_FAILURE for other host or guest error. In
> >  particular,
> >if neither dataout nor datain is empty, and the
> >VIRTIO_SCSI_F_INOUT
> >feature has not been negotiated, the request will be
> >immediately
> >returned with a response equal to VIRTIO_SCSI_S_FAILURE.
> >
> And, of course:
> 
> VIRTIO_SCSI_S_DISCONNECT if the request could not be processed due
> to a communication failure (eg device was removed or could not be
> reached).

Ok.

> This specification implies a strict one-to-one mapping between host
> and target. IE there is no way of specifying more than one target
> per host.

Actually no, the intention is to use hierarchical LUNs to support
more than one target per host.

Paolo

Re: [Qemu-devel] [PATCH RFC 0/3] basic support for composing sysbus devices

[Anthony Liguori]

On 06/10/2011 08:50 AM, Peter Maydell wrote:

On 10 June 2011 14:43, Jan Kiszka  wrote:

On 2011-06-10 15:10, Peter Maydell wrote:

This makes the wiring of this signal look like a property of the
isa-serial device, which is a bit odd, since it's just as much
a property of the piix3. Actually it's neither, it's a property
of the machine model, and you might actually want a syntax a bit
more like:

  piix3 = piix3(property=value, property=value...);
  serial = isa-serial(property=value...);
  connect(serial.irq, piix3.irq[3]);


In fact, in the ISA case, it is a device property: The device, and only
the device decides which IRQ to use - from the bus it is attached to. So
attaching an ISA device to the bus of an ISA bridge like the PIIX3 and
selecting local IRQ 3 are the steps we can already express today.


Ah, in that case Anthony's suggestion of
   -device piix3,id=piix3 -device isa-serial,id=serial,irq=piix3.irq[3]
wrong in a different way -- the isa-serial shouldn't care
what other device is providing the ISA bus it is sitting on,


If it really makes you feel bad, you could also do:

 -device piix3,id=piix3
 -device wire,id=wire,in=piix3.irq[3]
 -device isa-serial,id=serial,irq=wire.out

Regards,

Anthony Liguori


it just has a property of which ISA irq line it is using
(and rely on an isa bus abstraction to wire things up at
the machine model level). [As you say, this works now.]

But I think that's a non-typical case compared to the usual one
of "these wires are just hardwired this way by the machine".

-- PMM

Re: [Qemu-devel] [PATCH RFC 0/3] basic support for composing sysbus devices

[Markus Armbruster]

Peter Maydell  writes:

> On 10 June 2011 14:43, Jan Kiszka  wrote:
>> On 2011-06-10 15:10, Peter Maydell wrote:
>>> This makes the wiring of this signal look like a property of the
>>> isa-serial device, which is a bit odd, since it's just as much
>>> a property of the piix3. Actually it's neither, it's a property
>>> of the machine model, and you might actually want a syntax a bit
>>> more like:
>>>
>>>  piix3 = piix3(property=value, property=value...);
>>>  serial = isa-serial(property=value...);
>>>  connect(serial.irq, piix3.irq[3]);
>>
>> In fact, in the ISA case, it is a device property: The device, and only
>> the device decides which IRQ to use - from the bus it is attached to. So
>> attaching an ISA device to the bus of an ISA bridge like the PIIX3 and
>> selecting local IRQ 3 are the steps we can already express today.
>
> Ah, in that case Anthony's suggestion of
>   -device piix3,id=piix3 -device isa-serial,id=serial,irq=piix3.irq[3]
> wrong in a different way -- the isa-serial shouldn't care
> what other device is providing the ISA bus it is sitting on,
> it just has a property of which ISA irq line it is using
> (and rely on an isa bus abstraction to wire things up at
> the machine model level). [As you say, this works now.]
>
> But I think that's a non-typical case compared to the usual one
> of "these wires are just hardwired this way by the machine".

IIRC, the PCI bus also provides a number of IRQ lines for the device to
tickle (INTA#..INTD#).  There's rarely a need to configure their use,
though.

Re: [Qemu-devel] [PATCH RFC 0/3] basic support for composing sysbus devices

[Jan Kiszka]

On 2011-06-10 16:12, Anthony Liguori wrote:
> On 06/10/2011 08:43 AM, Jan Kiszka wrote:
>> On 2011-06-10 15:10, Peter Maydell wrote:
>>> On 10 June 2011 13:51, Anthony Liguori  wrote:
 On 06/10/2011 03:13 AM, Markus Armbruster wrote:
>
> Jan Kiszka   writes:
>>
>> Resource management, e.g. IRQs. That will be useful for other types of
>> buses as well.
>
> A device should be able to say "I need to be connected to an IRQ line".
> Feels generic to me.

 More specifically, a device has input IRQs.  A device has no idea what
 number the IRQ is tied to.
>>
>> Generally true. Two exceptions still require to make the path explorable
>> / transfer information in the reverse direction: IRQ de-coalescing and
>> physical device assignment. That's something a new generic API should
>> encapsulate so that we can stop peaking into the machine details.
>>

 Devices may also have output IRQs.  At the qdev layer, we should be able to
 connect an arbitrary output IRQ to an arbitrary input IRQ.
>>>
>>> Actually, devices have input and output I/O signals (GPIOs, if you like).
>>> A subset of these are IRQs. We already have some APIs in QEMU which
>>> claim to be dealing with 'irq's but actually are just for wiring
>>> up generic signals; I'd rather we didn't proliferate that terminology
>>> confusion if possible. (And a single GPIO wire is just one kind of
>>> thing you might want to link between two devices, obviously. MMIO is
>>> another.)
>>>
 So the crux of the problem is that:

   -device isa-serial,id=serial,irq=3

 Is very wrong.  It ought to look something more like

   -device piix3,id=piix3 -device isa-serial,id=serial,irq=piix3.irq[3]
>>>
>>> This makes the wiring of this signal look like a property of the
>>> isa-serial device, which is a bit odd, since it's just as much
>>> a property of the piix3. Actually it's neither, it's a property
>>> of the machine model, and you might actually want a syntax a bit
>>> more like:
>>>
>>>   piix3 = piix3(property=value, property=value...);
>>>   serial = isa-serial(property=value...);
>>>   connect(serial.irq, piix3.irq[3]);
>>
>> In fact, in the ISA case, it is a device property: The device, and only
>> the device decides which IRQ to use - from the bus it is attached to. So
>> attaching an ISA device to the bus of an ISA bridge like the PIIX3 and
>> selecting local IRQ 3 are the steps we can already express today.
> 
> If you really want to be pedantic, each ISA device has 5 input Pins that 
> are supposed to correspond to IRQ 3, IRQ 4, IRQ 5, IRQ 6, and IRQ 7.
> 
> This could easily be modelled by doing the following:
> 
>   -device piix3,id=piix3 -device 
> isa-serial,id=serial,irq[3]=piix3.irq[3],irq[4]=piix3.irq[4],...
> 
> But I don't think we benefit from modelling it this correctly.  The 
> point is that the infrastructure could handle it though.

I don't see the point of 'piix3.' in your IRQ assignment, though. It is
redundant to the bus assignment - as the ISA bus also routes interrupts.
There are surely also buses that don't. Still, interrupt routing via the
bus should be possible (as it avoids boilerplate code or configuration
redundancy).

Jan

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

Re: [Qemu-devel] [PATCH RFC 0/3] basic support for composing sysbus devices

[Anthony Liguori]

On 06/10/2011 08:43 AM, Jan Kiszka wrote:

On 2011-06-10 15:10, Peter Maydell wrote:

On 10 June 2011 13:51, Anthony Liguori  wrote:

On 06/10/2011 03:13 AM, Markus Armbruster wrote:


Jan Kiszka   writes:


Resource management, e.g. IRQs. That will be useful for other types of
buses as well.


A device should be able to say "I need to be connected to an IRQ line".
Feels generic to me.


More specifically, a device has input IRQs.  A device has no idea what
number the IRQ is tied to.


Generally true. Two exceptions still require to make the path explorable
/ transfer information in the reverse direction: IRQ de-coalescing and
physical device assignment. That's something a new generic API should
encapsulate so that we can stop peaking into the machine details.



Devices may also have output IRQs.  At the qdev layer, we should be able to
connect an arbitrary output IRQ to an arbitrary input IRQ.


Actually, devices have input and output I/O signals (GPIOs, if you like).
A subset of these are IRQs. We already have some APIs in QEMU which
claim to be dealing with 'irq's but actually are just for wiring
up generic signals; I'd rather we didn't proliferate that terminology
confusion if possible. (And a single GPIO wire is just one kind of
thing you might want to link between two devices, obviously. MMIO is
another.)


So the crux of the problem is that:

  -device isa-serial,id=serial,irq=3

Is very wrong.  It ought to look something more like

  -device piix3,id=piix3 -device isa-serial,id=serial,irq=piix3.irq[3]


This makes the wiring of this signal look like a property of the
isa-serial device, which is a bit odd, since it's just as much
a property of the piix3. Actually it's neither, it's a property
of the machine model, and you might actually want a syntax a bit
more like:

  piix3 = piix3(property=value, property=value...);
  serial = isa-serial(property=value...);
  connect(serial.irq, piix3.irq[3]);


In fact, in the ISA case, it is a device property: The device, and only
the device decides which IRQ to use - from the bus it is attached to. So
attaching an ISA device to the bus of an ISA bridge like the PIIX3 and
selecting local IRQ 3 are the steps we can already express today.


If you really want to be pedantic, each ISA device has 5 input Pins that 
are supposed to correspond to IRQ 3, IRQ 4, IRQ 5, IRQ 6, and IRQ 7.


This could easily be modelled by doing the following:

 -device piix3,id=piix3 -device 
isa-serial,id=serial,irq[3]=piix3.irq[3],irq[4]=piix3.irq[4],...


But I don't think we benefit from modelling it this correctly.  The 
point is that the infrastructure could handle it though.


Regards,

Anthony Liguori



Jan

Re: [Qemu-devel] [PATCH RFC 0/3] basic support for composing sysbus devices

[Anthony Liguori]

On 06/10/2011 08:10 AM, Peter Maydell wrote:

On 10 June 2011 13:51, Anthony Liguori  wrote:

On 06/10/2011 03:13 AM, Markus Armbruster wrote:


Jan Kiszkawrites:


Resource management, e.g. IRQs. That will be useful for other types of
buses as well.


A device should be able to say "I need to be connected to an IRQ line".
Feels generic to me.


More specifically, a device has input IRQs.  A device has no idea what
number the IRQ is tied to.

Devices may also have output IRQs.  At the qdev layer, we should be able to
connect an arbitrary output IRQ to an arbitrary input IRQ.


Actually, devices have input and output I/O signals (GPIOs, if you like).


Yes, I prefer the term Pin but since the discussion was using IRQs, I 
didn't want to rock the boat ;-)



A subset of these are IRQs. We already have some APIs in QEMU which
claim to be dealing with 'irq's but actually are just for wiring
up generic signals; I'd rather we didn't proliferate that terminology
confusion if possible. (And a single GPIO wire is just one kind of
thing you might want to link between two devices, obviously. MMIO is
another.)


Forget about MMIO.  From a device perspective, MMIO is an extremely high 
level concept.  MMIO almost always involves some higher level interface 
(like PCI, ISA, etc.).


That's really the interface that you want to model.  Sysbus ends up 
being treated as a generic bus in-lieu of implementing machine specific 
buses (probably because they're proprietary and undocumented).  That's 
sort of okay but we should treat that not as the parent bus of 
everything but as a generic bus type.



So the crux of the problem is that:

  -device isa-serial,id=serial,irq=3

Is very wrong.  It ought to look something more like

  -device piix3,id=piix3 -device isa-serial,id=serial,irq=piix3.irq[3]


This makes the wiring of this signal look like a property of the
isa-serial device, which is a bit odd, since it's just as much
a property of the piix3. Actually it's neither, it's a property
of the machine model, and you might actually want a syntax a bit
more like:


Think of it like a graph with directed paths.  The property is the end 
point of the connection.


But the key point is: the connections of the graph *IS* the machine model.



  piix3 = piix3(property=value, property=value...);
  serial = isa-serial(property=value...);
  connect(serial.irq, piix3.irq[3]);


You can use a different verb but essentially, it's still:

serial.irq OPERATOR piix3.irq[3]

Picking a direction and using assignment is convenient syntactically.

Regards,

Anthony Liguori



(in some mythical stitching language, which I think makes much
more sense than command line switches anyway.)

-- PMM

Re: [Qemu-devel] [PATCH 3/3] ide: add TRIM support

[Kevin Wolf]

Am 19.05.2011 10:58, schrieb Christoph Hellwig:
> Add support for TRIM sub function of the data set management command,
> and wire it up to the qemu discard infrastructure.
> 
> Signed-off-by: Christoph Hellwig 

> Index: qemu/hw/ide/pci.c
> ===
> --- qemu.orig/hw/ide/pci.c2011-05-18 20:28:17.153625872 +0200
> +++ qemu/hw/ide/pci.c 2011-05-18 20:33:06.102141553 +0200
> @@ -205,6 +205,9 @@ static void bmdma_restart_bh(void *opaqu
>  }
>  } else if (bm->status & BM_STATUS_RETRY_FLUSH) {
>  ide_flush_cache(bmdma_active_if(bm));
> +} else if (bm->status & BM_STATUS_RETRY_TRIM) {
> +bm->status &= ~BM_STATUS_RETRY_TRIM;
> +bmdma_restart_dma(bm, IDE_DMA_TRIM);
>  }
>  }

Just noticed that this is wrong. BM_STATUS_DMA_RETRY is always set at
the same time, so this is dead code.

Kevin

Re: [Qemu-devel] [PATCH RFC 0/3] basic support for composing sysbus devices

[Jan Kiszka]

On 2011-06-10 15:10, Peter Maydell wrote:
> On 10 June 2011 13:51, Anthony Liguori  wrote:
>> On 06/10/2011 03:13 AM, Markus Armbruster wrote:
>>>
>>> Jan Kiszka  writes:

 Resource management, e.g. IRQs. That will be useful for other types of
 buses as well.
>>>
>>> A device should be able to say "I need to be connected to an IRQ line".
>>> Feels generic to me.
>>
>> More specifically, a device has input IRQs.  A device has no idea what
>> number the IRQ is tied to.

Generally true. Two exceptions still require to make the path explorable
/ transfer information in the reverse direction: IRQ de-coalescing and
physical device assignment. That's something a new generic API should
encapsulate so that we can stop peaking into the machine details.

>>
>> Devices may also have output IRQs.  At the qdev layer, we should be able to
>> connect an arbitrary output IRQ to an arbitrary input IRQ.
> 
> Actually, devices have input and output I/O signals (GPIOs, if you like).
> A subset of these are IRQs. We already have some APIs in QEMU which
> claim to be dealing with 'irq's but actually are just for wiring
> up generic signals; I'd rather we didn't proliferate that terminology
> confusion if possible. (And a single GPIO wire is just one kind of
> thing you might want to link between two devices, obviously. MMIO is
> another.)
> 
>> So the crux of the problem is that:
>>
>>  -device isa-serial,id=serial,irq=3
>>
>> Is very wrong.  It ought to look something more like
>>
>>  -device piix3,id=piix3 -device isa-serial,id=serial,irq=piix3.irq[3]
> 
> This makes the wiring of this signal look like a property of the
> isa-serial device, which is a bit odd, since it's just as much
> a property of the piix3. Actually it's neither, it's a property
> of the machine model, and you might actually want a syntax a bit
> more like:
> 
>  piix3 = piix3(property=value, property=value...);
>  serial = isa-serial(property=value...);
>  connect(serial.irq, piix3.irq[3]);

In fact, in the ISA case, it is a device property: The device, and only
the device decides which IRQ to use - from the bus it is attached to. So
attaching an ISA device to the bus of an ISA bridge like the PIIX3 and
selecting local IRQ 3 are the steps we can already express today.

Jan

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

Re: [Qemu-devel] [PATCH RFC 0/3] basic support for composing sysbus devices

[Peter Maydell]

On 10 June 2011 14:43, Jan Kiszka  wrote:
> On 2011-06-10 15:10, Peter Maydell wrote:
>> This makes the wiring of this signal look like a property of the
>> isa-serial device, which is a bit odd, since it's just as much
>> a property of the piix3. Actually it's neither, it's a property
>> of the machine model, and you might actually want a syntax a bit
>> more like:
>>
>>  piix3 = piix3(property=value, property=value...);
>>  serial = isa-serial(property=value...);
>>  connect(serial.irq, piix3.irq[3]);
>
> In fact, in the ISA case, it is a device property: The device, and only
> the device decides which IRQ to use - from the bus it is attached to. So
> attaching an ISA device to the bus of an ISA bridge like the PIIX3 and
> selecting local IRQ 3 are the steps we can already express today.

Ah, in that case Anthony's suggestion of
  -device piix3,id=piix3 -device isa-serial,id=serial,irq=piix3.irq[3]
wrong in a different way -- the isa-serial shouldn't care
what other device is providing the ISA bus it is sitting on,
it just has a property of which ISA irq line it is using
(and rely on an isa bus abstraction to wire things up at
the machine model level). [As you say, this works now.]

But I think that's a non-typical case compared to the usual one
of "these wires are just hardwired this way by the machine".

-- PMM

[Qemu-devel] [PATCH] hw/9pfs: Fix segfault on arch that doesn't support VirtFS

[Aneesh Kumar K.V]

Signed-off-by: Aneesh Kumar K.V 
---
 fsdev/qemu-fsdev-dummy.c |8 
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/fsdev/qemu-fsdev-dummy.c b/fsdev/qemu-fsdev-dummy.c
index 619e163..4e700dd 100644
--- a/fsdev/qemu-fsdev-dummy.c
+++ b/fsdev/qemu-fsdev-dummy.c
@@ -13,8 +13,16 @@
 #include 
 #include 
 #include "qemu-fsdev.h"
+#include "qemu-config.h"
 
 int qemu_fsdev_add(QemuOpts *opts)
 {
 return 0;
 }
+
+static void fsdev_register_config(void)
+{
+qemu_add_opts(&qemu_fsdev_opts);
+qemu_add_opts(&qemu_virtfs_opts);
+}
+machine_init(fsdev_register_config);
-- 
1.7.4.1

Re: [Qemu-devel] [PATCH RFC 0/3] basic support for composing sysbus devices

[Peter Maydell]

On 10 June 2011 13:51, Anthony Liguori  wrote:
> On 06/10/2011 03:13 AM, Markus Armbruster wrote:
>>
>> Jan Kiszka  writes:
>>>
>>> Resource management, e.g. IRQs. That will be useful for other types of
>>> buses as well.
>>
>> A device should be able to say "I need to be connected to an IRQ line".
>> Feels generic to me.
>
> More specifically, a device has input IRQs.  A device has no idea what
> number the IRQ is tied to.
>
> Devices may also have output IRQs.  At the qdev layer, we should be able to
> connect an arbitrary output IRQ to an arbitrary input IRQ.

Actually, devices have input and output I/O signals (GPIOs, if you like).
A subset of these are IRQs. We already have some APIs in QEMU which
claim to be dealing with 'irq's but actually are just for wiring
up generic signals; I'd rather we didn't proliferate that terminology
confusion if possible. (And a single GPIO wire is just one kind of
thing you might want to link between two devices, obviously. MMIO is
another.)

> So the crux of the problem is that:
>
>  -device isa-serial,id=serial,irq=3
>
> Is very wrong.  It ought to look something more like
>
>  -device piix3,id=piix3 -device isa-serial,id=serial,irq=piix3.irq[3]

This makes the wiring of this signal look like a property of the
isa-serial device, which is a bit odd, since it's just as much
a property of the piix3. Actually it's neither, it's a property
of the machine model, and you might actually want a syntax a bit
more like:

 piix3 = piix3(property=value, property=value...);
 serial = isa-serial(property=value...);
 connect(serial.irq, piix3.irq[3]);

(in some mythical stitching language, which I think makes much
more sense than command line switches anyway.)

-- PMM

Re: [Qemu-devel] fsdev - broken qemu-system-?

[Edgar E. Iglesias]

On Fri, Jun 10, 2011 at 05:47:05PM +0530, Aneesh Kumar K.V wrote:
> On Fri, 10 Jun 2011 12:12:33 +0200, "Edgar E. Iglesias" 
>  wrote:
> > At least CRIS, Microblaze and lm32 are broken on latest git. Things fail 
> > with
> > the following message:
> > qemu-system-cris: there is no option group "fsdev"
> > 
> > Under GDB I see a segfault...
> > 
> > % gdb --args ~/src/c/qemu/git/build-qemu/cris-softmmu/qemu-system-cris -M 
> > axis-dev88 -kernel kimage -serial stdio



> > envp=) at /home/edgar/src/c/qemu/git/qemu/vl.c:3015
> > (gdb) q
> > A debugging session is active.
> > 
> > Inferior 1 [process 1473] will be killed.
> > 
> 
> Can you try this patch 

It works, thanks!

Cheers

> 
> diff --git a/fsdev/qemu-fsdev-dummy.c b/fsdev/qemu-fsdev-dummy.c
> index 619e163..4e700dd 100644
> --- a/fsdev/qemu-fsdev-dummy.c
> +++ b/fsdev/qemu-fsdev-dummy.c
> @@ -13,8 +13,16 @@
>  #include 
>  #include 
>  #include "qemu-fsdev.h"
> +#include "qemu-config.h"
>  
>  int qemu_fsdev_add(QemuOpts *opts)
>  {
>  return 0;
>  }
> +
> +static void fsdev_register_config(void)
> +{
> +qemu_add_opts(&qemu_fsdev_opts);
> +qemu_add_opts(&qemu_virtfs_opts);
> +}
> +machine_init(fsdev_register_config);

Re: [Qemu-devel] virtio scsi host draft specification, v3

[Hannes Reinecke]

On 06/07/2011 03:43 PM, Paolo Bonzini wrote:

Hi all,

after some preliminary discussion on the QEMU mailing list, I present a
draft specification for a virtio-based SCSI host (controller, HBA, you
name it).

The virtio SCSI host is the basis of an alternative storage stack for
KVM. This stack would overcome several limitations of the current
solution, virtio-blk:

1) scalability limitations: virtio-blk-over-PCI puts a strong upper
limit on the number of devices that can be added to a guest. Common
configurations have a limit of ~30 devices. While this can be worked
around by implementing a PCI-to-PCI bridge, or by using multifunction
virtio-blk devices, these solutions either have not been implemented
yet, or introduce management restrictions. On the other hand, the SCSI
architecture is well known for its scalability and virtio-scsi supports
advanced feature such as multiqueueing.

2) limited flexibility: virtio-blk does not support all possible storage
scenarios. For example, it does not allow SCSI passthrough or persistent
reservations. In principle, virtio-scsi provides anything that the
underlying SCSI target (be it physical storage, iSCSI or the in-kernel
target) supports.

3) limited extensibility: over the time, many features have been added
to virtio-blk. Each such change requires modifications to the virtio
specification, to the guest drivers, and to the device model in the
host. The virtio-scsi spec has been written to follow SAM conventions,
and exposing new features to the guest will only require changes to the
host's SCSI target implementation.


Comments are welcome.

Paolo

--->8 ---


Virtio SCSI Host Device Spec


The virtio SCSI host device groups together one or more simple virtual
devices (ie. disk), and allows communicating to these devices using the
SCSI protocol.  An instance of the device represents a SCSI host with
possibly many buses, targets and LUN attached.

The virtio SCSI device services two kinds of requests:

- command requests for a logical unit;

- task management functions related to a logical unit, target or
command.

The device is also able to send out notifications about added
and removed logical units.

v1:
 First public version

v2:
 Merged all virtqueues into one, removed separate TARGET fields

v3:
 Added configuration information and reworked descriptor structure.
 Added back multiqueue on Avi's request, while still leaving TARGET
 fields out.  Added dummy event and clarified some aspects of the
 event protocol.  First version sent to a wider audience (linux-kernel
 and virtio lists).

Configuration
-

Subsystem Device ID
 TBD

Virtqueues
 0:controlq
 1:eventq
 2..n:request queues

Feature bits
 VIRTIO_SCSI_F_INOUT (0) - Whether a single request can include both
 read-only and write-only data buffers.

Device configuration layout
 struct virtio_scsi_config {
 u32 num_queues;
 u32 event_info_size;
 u32 sense_size;
 u32 cdb_size;
 }

 num_queues is the total number of virtqueues exposed by the
 device.  The driver is free to use only one request queue, or
 it can use more to achieve better performance.

 event_info_size is the maximum size that the device will fill
 for buffers that the driver places in the eventq.  The
 driver should always put buffers at least of this size.

 sense_size is the maximum size of the sense data that the device
 will write.  The default value is written by the device and
 will always be 96, but the driver can modify it.

 cdb_size is the maximum size of the CBD that the driver
 will write.  The default value is written by the device and
 will always be 32, but the driver can likewise modify it.

Device initialization
-

The initialization routine should first of all discover the device's
virtqueues.

The driver should then place at least a buffer in the eventq.
Buffers returned by the device on the eventq may be referred
to as "events" in the rest of the document.

The driver can immediately issue requests (for example, INQUIRY or
REPORT LUNS) or task management functions (for example, I_T RESET).

Device operation: request queues


The driver queues requests to an arbitrary request queue, and they are
used by the device on that same queue.


What about request ordering?
If requests are placed on arbitrary queues you'll inevitably run on 
locking issues to ensure strict request ordering.

I would add here:

If a device uses more than one queue it is the responsibility of the 
device to ensure strict request ordering.



Requests have the following format:

 struct virtio_scsi_req_cmd {
 u8 lun[8];
 u64 id;
 u8 task_attr;
 u8 prio;
 u8 crn;
 char cdb[cdb_size];
 char dataout[];

 

Re: [Qemu-devel] [PATCH RFC 0/3] basic support for composing sysbus devices

[Anthony Liguori]

On 06/10/2011 03:13 AM, Markus Armbruster wrote:

Jan Kiszka  writes:

Resource management, e.g. IRQs. That will be useful for other types of
buses as well.


A device should be able to say "I need to be connected to an IRQ line".
Feels generic to me.


More specifically, a device has input IRQs.  A device has no idea what 
number the IRQ is tied to.


Devices may also have output IRQs.  At the qdev layer, we should be able 
to connect an arbitrary output IRQ to an arbitrary input IRQ.


So the crux of the problem is that:

 -device isa-serial,id=serial,irq=3

Is very wrong.  It ought to look something more like

 -device piix3,id=piix3 -device isa-serial,id=serial,irq=piix3.irq[3]

IRQ forwarding becomes very easy in this model because your composite 
qdev device has a set of input IRQs and then routes the input IRQs to 
the appropriate input IRQs of the sub devices.


The trouble is that I don't think we have a reasonable way to refer to 
properties of other devices and we don't have names for all devices.  I 
think if we fix the later problem, the former problem becomes easier.



Connecting the two is configuration.  Requires a suitable naming scheme
for IRQ lines.  Naming might have to include bus-specific sugar for
user-friendliness.  For instance, I'd rather express "isa-serial uses
ISA interrupt 4" as "irq=4" than as something like
"irq=PIIX3/isa.0:irq.4".


That's just syntactic sugar.  It can live at a higher level than the 
qdev API.



I doubt all resources are as generic as IRQ lines, but that's okay.


They pretty much are.  We're really just talking about referring to 
subcomponents of a device.  That's what's lacking today.



Device component composition is not (yet?) covered by qdev.  Of course
we compose anyway, in various ad hoc ways.


Busses need to die.  They can be replaced by having fixed "slots".  For 
instance, if you had a way of having a PCIDevice * property, the I440FX 
could have 32 PCIDevice * properties.  That's how you would add to a bus 
(and notice that it conveniently solves bus addressing in a robust fashion).


Regards,

Anthony Liguori


One way is to put the components' state structs into the device's state
struct, and define suitable wrappers.  For instance, we have qdevs
"sysbus-fdc" and "isa-fdc".  They both contain the FDC proper as a
component: FDCtrlSysBus and FDCtrlISABus contain a FDCtrl member.
Simple enough.

A different way to adapt the same component to different buses can be
found in virtio: VirtIOPCIProxy and VirtIOS390Device both contain
pointers to VirtIODevice.  I found that one quite awkward to work with.

Yet another way can be found in usb-storage.  usb-storage expands into
two qdevs connected by a qbus: it provides a SCSI bus, and automatically
creates a single scsi-disk on that bus.  One of those tricks that look
cute initially, then create no end of trouble.

I figure a "qdevy" composition mechanism would be useful.  Needs
thought.

Re: [Qemu-devel] QEMU 9pfs intentionally returning short reads ?

[Daniel P. Berrange]

On Fri, Jun 10, 2011 at 05:36:13PM +0530, Aneesh Kumar K.V wrote:
> On Fri, 10 Jun 2011 11:33:05 +0100, "Daniel P. Berrange" 
>  wrote:
> > I've been doing some work trying to run QEMU guests with a root filesystem
> > exported from the host using virtio 9pfs. One of the issues that I have
> > discovered is that the 9p FS running on QEMU appears to cap all reads at
> > 4096 bytes[1]. Any larger read will return only partial data for plain
> > files.
> > 
> 
> But we should loop in kernel, requesting for multiple 9p request. 
> 
> kernel does
> 
>   size = fid->iounit ? fid->iounit : fid->clnt->msize - P9_IOHDRSZ;
>   if (count > size)
>   ret = v9fs_file_readn(filp, NULL, udata, count, *offset);
>   else
>   ret = p9_client_read(fid, NULL, udata, *offset, count);
> 
> and v9fs_file_readn() does..
> 
>   do {
>   n = p9_client_read(fid, data, udata, offset, count);
>   if (n <= 0)
>   break;
> 
>   if (data)
>   data += n;
>   if (udata)
>   udata += n;
> 
>   offset += n;
>   count -= n;
>   total += n;
>   } while (count > 0 && n == size);
> 
> 
> I also did an strace of simple test and i see
> 
> open("test", O_RDONLY)  = 3
> read(3, 
> "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 8192) 
> = 8192

In my test I did

#  strace -e trace=read,open perl -e 'open FOO, "/usr/share/X11/XKeysymDB"; 
sysread FOO, $foo, 8192'
open("/usr/share/X11/XKeysymDB", O_RDONLY) = 3
read(3, "! Copyright 1993 Massachusetts I"..., 8192) = 4096

Perhaps there is a guest kernel driver difference ? I'm using

  2.6.35.13-91.fc14.x86_64

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-v3 2/2] kvm: Fix unused-but-set-variable warning

[Stefan Hajnoczi]

On Fri, Jun 10, 2011 at 11:43 AM, Christophe Fergeau
 wrote:
> Ping?

Let me add these patches to the trivial-patches tree since they
haven't been picked up.

Stefan

Re: [Qemu-devel] fsdev - broken qemu-system-?

[Aneesh Kumar K.V]

On Fri, 10 Jun 2011 12:12:33 +0200, "Edgar E. Iglesias" 
 wrote:
> At least CRIS, Microblaze and lm32 are broken on latest git. Things fail with
> the following message:
> qemu-system-cris: there is no option group "fsdev"
> 
> Under GDB I see a segfault...
> 
> % gdb --args ~/src/c/qemu/git/build-qemu/cris-softmmu/qemu-system-cris -M 
> axis-dev88 -kernel kimage -serial stdio
> GNU gdb (Gentoo 7.2 p1) 7.2
> Copyright (C) 2010 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later 
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
> and "show warranty" for details.
> This GDB was configured as "x86_64-pc-linux-gnu".
> For bug reporting instructions, please see:
> ...
> Reading symbols from 
> /home/edgar/src/c/qemu/git/build-qemu/cris-softmmu/qemu-system-cris...done.
> (gdb) run
> Starting program: 
> /home/edgar/src/c/qemu/git/build-qemu/cris-softmmu/qemu-system-cris -M 
> axis-dev88 -kernel kimage -serial stdio
> [Thread debugging using libthread_db enabled]
> qemu-system-cris: there is no option group "fsdev"
> 
> Program received signal SIGSEGV, Segmentation fault.
> qemu_opts_foreach (list=0x0, func=0x4fd210 , opaque=0x0, 
> abort_on_failure=1) at /home/edgar/src/c/qemu/git/qemu/qemu-option.c:969
> 969 QTAILQ_FOREACH(opts, &list->head, next) {
> (gdb) bt
> #0  qemu_opts_foreach (list=0x0, func=0x4fd210 , opaque=0x0, 
> abort_on_failure=1) at /home/edgar/src/c/qemu/git/qemu/qemu-option.c:969
> #1  0x004ff05d in main (argc=7, argv=0x7fffdc28, 
> envp=) at /home/edgar/src/c/qemu/git/qemu/vl.c:3015
> (gdb) q
> A debugging session is active.
> 
> Inferior 1 [process 1473] will be killed.
> 

Can you try this patch 

diff --git a/fsdev/qemu-fsdev-dummy.c b/fsdev/qemu-fsdev-dummy.c
index 619e163..4e700dd 100644
--- a/fsdev/qemu-fsdev-dummy.c
+++ b/fsdev/qemu-fsdev-dummy.c
@@ -13,8 +13,16 @@
 #include 
 #include 
 #include "qemu-fsdev.h"
+#include "qemu-config.h"
 
 int qemu_fsdev_add(QemuOpts *opts)
 {
 return 0;
 }
+
+static void fsdev_register_config(void)
+{
+qemu_add_opts(&qemu_fsdev_opts);
+qemu_add_opts(&qemu_virtfs_opts);
+}
+machine_init(fsdev_register_config);

Re: [Qemu-devel] virtio scsi host draft specification, v3

[Paolo Bonzini]

On 06/10/2011 02:14 PM, Stefan Hajnoczi wrote:

Paolo, I'll switch the Linux guest LLD and QEMU virtio-scsi skeleton
that I have to comply with the spec.  Does this sound good or did you
want to write these from scratch?


Why should I want to write things from scratch? :)  Just send me again a 
pointer to your git tree, I'll make sure to add it as a remote this time 
(private mail will do).


Thanks,

Paolo

Re: [Qemu-devel] virtio scsi host draft specification, v3

[Stefan Hajnoczi]

On Fri, Jun 10, 2011 at 12:33 PM, Rusty Russell  wrote:
> On Thu, 09 Jun 2011 08:59:27 +0200, Paolo Bonzini  wrote:
>> On 06/09/2011 01:28 AM, Rusty Russell wrote:
>> >> >  after some preliminary discussion on the QEMU mailing list, I present a
>> >> >  draft specification for a virtio-based SCSI host (controller, HBA, you
>> >> >  name it).
>> >
>> > OK, I'm impressed.  This is very well written and it doesn't make any of
>> > the obvious mistakes wrt. virtio.
>>
>> Thanks very much, and thanks to those who corrected my early mistakes.
>>
>> > I assume you have an implementation, as well?
>>
>> Unfortunately not; "we're working on it", which means I should start in
>> July when I come back from vacation.
>>
>> Do you prefer to wait for one before I make a patch to the LyX source?
>> In the meanwhile, can you reserve a subsystem ID for me?
>>
>> Paolo
>
> Sure, you can have the next subsystem ID.
>
> It's a pain to patch once it's in LyX, so let's get the implementation
> base on what you posted here an see how much it changes first...

Paolo, I'll switch the Linux guest LLD and QEMU virtio-scsi skeleton
that I have to comply with the spec.  Does this sound good or did you
want to write these from scratch?

Stefan

Re: [Qemu-devel] [PATCH] coroutine: Implement coroutines using gthread

[Stefan Hajnoczi]

On Fri, Jun 10, 2011 at 12:09 PM, Aneesh Kumar K.V
 wrote:
> On Fri, 10 Jun 2011 11:24:20 +0100, Stefan Hajnoczi  
> wrote:
>> On Thu, Jun 9, 2011 at 6:41 PM, Aneesh Kumar K.V
>>  wrote:
>> > On platforms that doesn't support makecontext use gthread
>> > based coroutine implementation.
>> >
>> > Signed-off-by: Aneesh Kumar K.V 
>> > ---
>> >
>> > NOTE: Tested on linux with force compliation of coroutine-gthread.c
>> >
>> >  Makefile.objs       |    5 ++
>> >  configure           |   18 +
>> >  coroutine-gthread.c |  172 
>> > +++
>> >  3 files changed, 195 insertions(+), 0 deletions(-)
>> >  create mode 100644 coroutine-gthread.c
>> >
>> > diff --git a/Makefile.objs b/Makefile.objs
>> > index 0f1d7df..d354d3c 100644
>> > --- a/Makefile.objs
>> > +++ b/Makefile.objs
>> > @@ -13,9 +13,14 @@ oslib-obj-$(CONFIG_POSIX) += oslib-posix.o 
>> > qemu-thread-posix.o
>> >  ###
>> >  # coroutines
>> >  coroutine-obj-y = qemu-coroutine.o qemu-coroutine-lock.o
>> > +ifeq ($(CONFIG_UCONTEXT_COROUTINE),y)
>> >  coroutine-obj-$(CONFIG_POSIX) += coroutine-ucontext.o
>> > +else
>> > +coroutine-obj-$(CONFIG_POSIX) += coroutine-gthread.o
>> > +endif
>> >  coroutine-obj-$(CONFIG_WIN32) += coroutine-win32.o
>> >
>> > +
>> >  ###
>> >  # block-obj-y is code used by both qemu system emulation and qemu-img
>> >
>> > diff --git a/configure b/configure
>> > index 980914a..529d8c4 100755
>> > --- a/configure
>> > +++ b/configure
>> > @@ -2568,6 +2568,20 @@ if test "$trace_backend" = "dtrace"; then
>> >  fi
>> >
>> >  ##
>> > +# check if we have makecontext
>> > +
>> > +ucontext_coroutine=no
>> > +cat > $TMPC << EOF
>> > +#include 
>> > +int main(void) { makecontext(0, 0, 0); }
>> > +EOF
>> > +if compile_prog "" "" ; then
>> > +    ucontext_coroutine=yes
>> > +fi
>> > +
>> > +
>> > +
>> > +##
>> >  # End of CC checks
>> >  # After here, no more $cc or $ld runs
>> >
>> > @@ -3031,6 +3045,10 @@ if test "$rbd" = "yes" ; then
>> >   echo "CONFIG_RBD=y" >> $config_host_mak
>> >  fi
>> >
>> > +if test "$ucontext_coroutine" = "yes" ; then
>> > +  echo "CONFIG_UCONTEXT_COROUTINE=y" >> $config_host_mak
>> > +fi
>> > +
>> >  # USB host support
>> >  case "$usb" in
>> >  linux)
>> > diff --git a/coroutine-gthread.c b/coroutine-gthread.c
>> > new file mode 100644
>> > index 000..37e5a16
>> > --- /dev/null
>> > +++ b/coroutine-gthread.c
>> > @@ -0,0 +1,172 @@
>> > +/*
>>
>> A summary of the purpose of this file would be nice.
>>
>> > + *
>> > + * Copyright (C) 2006  Anthony Liguori 
>> > + * Copyright (C) 2011  Aneesh Kumar K.V 
>> > + *
>> > + * This library is free software; you can redistribute it and/or
>> > + * modify it under the terms of the GNU Lesser General Public
>> > + * License as published by the Free Software Foundation; either
>> > + * version 2.0 of the License, or (at your option) any later version.
>> > + *
>> > + * This library is distributed in the hope that it will be useful,
>> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> > + * Lesser General Public License for more details.
>> > + *
>> > + * You should have received a copy of the GNU Lesser General Public
>> > + * License along with this library; if not, write to the Free Software
>> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  
>> > 02110-1301 USA
>>
>> The URL is preferred over the postal address:
>>
>>  * You should have received a copy of the GNU Lesser General Public
>>  * License along with this library; if not, see
>>  .
>
> inherited that header from gtk-vnc version of original file
>>
>> > + */
>> > +
>> > +#include "qemu-common.h"
>> > +#include "qemu-coroutine-int.h"
>> > +#include 
>
> Will fix.
>
>>
>> It's good practice to include system headers first unless you are
>> explicitly setting a magic #define to affect the system header.
>>
>> > +
>> > +typedef struct {
>> > +    Coroutine qemu_co;
>> > +    GThread *thread;
>> > +    gboolean runnable;
>> > +} CoroutineGthread;
>> > +
>> > +typedef struct {
>> > +    /** Currently executing coroutine */
>> > +    CoroutineGthread *current;
>> > +
>> > +    /** The default coroutine */
>> > +    CoroutineGthread leader;
>> > +} CoroutineThreadState;
>> > +
>> > +static GCond *run_cond;
>> > +static GMutex *run_lock;
>> > +static pthread_key_t thread_state_key;
>>
>> Why pthread_key_t instead of GStaticPrivate?
>
> No specific reason other than I was not aware of GStaticPrivate.

It would be worth using that to avoid mixing GThread and pthreads
unnecessarily.  I still need to look at the rest of this patch so
don't worry about sending v2 yet.

Stefan

Re: [Qemu-devel] QEMU 9pfs intentionally returning short reads ?

[Aneesh Kumar K.V]

On Fri, 10 Jun 2011 11:33:05 +0100, "Daniel P. Berrange"  
wrote:
> I've been doing some work trying to run QEMU guests with a root filesystem
> exported from the host using virtio 9pfs. One of the issues that I have
> discovered is that the 9p FS running on QEMU appears to cap all reads at
> 4096 bytes[1]. Any larger read will return only partial data for plain
> files.
> 

But we should loop in kernel, requesting for multiple 9p request. 

kernel does

size = fid->iounit ? fid->iounit : fid->clnt->msize - P9_IOHDRSZ;
if (count > size)
ret = v9fs_file_readn(filp, NULL, udata, count, *offset);
else
ret = p9_client_read(fid, NULL, udata, *offset, count);

and v9fs_file_readn() does..

do {
n = p9_client_read(fid, data, udata, offset, count);
if (n <= 0)
break;

if (data)
data += n;
if (udata)
udata += n;

offset += n;
count -= n;
total += n;
} while (count > 0 && n == size);


I also did an strace of simple test and i see

open("test", O_RDONLY)  = 3
read(3, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 
8192) = 8192

-aneesh

Re: [Qemu-devel] [PATCH 3/3] ide: add TRIM support

[Kevin Wolf]

Am 19.05.2011 10:58, schrieb Christoph Hellwig:
> Add support for TRIM sub function of the data set management command,
> and wire it up to the qemu discard infrastructure.
> 
> Signed-off-by: Christoph Hellwig 
> 
> Index: qemu/hw/ide/core.c
> ===
> --- qemu.orig/hw/ide/core.c   2011-05-18 20:30:20.855172933 +0200
> +++ qemu/hw/ide/core.c2011-05-18 20:32:26.237625024 +0200
> @@ -124,6 +124,9 @@ static void ide_identify(IDEState *s)
>  put_le16(p + 66, 120);
>  put_le16(p + 67, 120);
>  put_le16(p + 68, 120);
> +if (dev && dev->conf.discard_granularity) {
> +put_le16(p + 69, (1 << 14)); /* determinate TRIM behavior */
> +}
>  
>  if (s->ncq_queues) {
>  put_le16(p + 75, s->ncq_queues - 1);
> @@ -157,6 +160,9 @@ static void ide_identify(IDEState *s)
>  dev = s->unit ? s->bus->slave : s->bus->master;
>  if (dev && dev->conf.physical_block_size)
>  put_le16(p + 106, 0x6000 | get_physical_block_exp(&dev->conf));
> +if (dev && dev->conf.discard_granularity) {
> +put_le16(p + 169, 1); /* TRIM support */
> +}
>  
>  memcpy(s->identify_data, p, sizeof(s->identify_data));
>  s->identify_set = 1;
> @@ -299,6 +305,72 @@ static void ide_set_signature(IDEState *
>  }
>  }
>  
> +typedef struct TrimAIOCB {
> +BlockDriverAIOCB common;
> +QEMUBH *bh;
> +int ret;
> +} TrimAIOCB;
> +
> +static void trim_aio_cancel(BlockDriverAIOCB *acb)
> +{
> +TrimAIOCB *iocb = container_of(acb, TrimAIOCB, common);
> +
> +qemu_bh_delete(iocb->bh);
> +iocb->bh = NULL;
> +qemu_aio_release(iocb);
> +}
> +
> +static AIOPool trim_aio_pool = {
> +.aiocb_size = sizeof(TrimAIOCB),
> +.cancel = trim_aio_cancel,
> +};
> +
> +static void ide_trim_bh_cb(void *opaque)
> +{
> +TrimAIOCB *iocb = opaque;
> +
> +iocb->common.cb(iocb->common.opaque, iocb->ret);
> +
> +qemu_bh_delete(iocb->bh);
> +iocb->bh = NULL;
> +
> +qemu_aio_release(iocb);
> +}
> +
> +BlockDriverAIOCB *ide_issue_trim(BlockDriverState *bs,
> +int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
> +BlockDriverCompletionFunc *cb, void *opaque)
> +{
> +TrimAIOCB *iocb;
> +int i, j, ret;
> +
> +iocb = qemu_aio_get(&trim_aio_pool, bs, cb, opaque);
> +iocb->bh = qemu_bh_new(ide_trim_bh_cb, iocb);
> +iocb->ret = 0;
> +
> +for (j = 0; j < qiov->niov; j++) {
> +uint64_t *buffer = qiov->iov[j].iov_base;
> +
> +for (i = 0; i < qiov->iov[j].iov_len / 8; i++) {
> +/* 6-byte LBA + 2-byte range per entry */
> +uint64_t entry = le64_to_cpu(buffer[i]);
> +uint64_t sector = entry & 0xULL;
> +uint16_t count = entry >> 48;
> +
> +if (count == 0)
> +break;
> +
> +ret = bdrv_discard(bs, sector * 512, count * 512);

Hm... bdrv_discard wants sector numbers instead of bytes, doesn't it?

If you agree, I'll send out and apply a fixed and rebased version.

Kevin

Re: [Qemu-devel] virtio scsi host draft specification, v3

[Rusty Russell]

On Thu, 09 Jun 2011 08:59:27 +0200, Paolo Bonzini  wrote:
> On 06/09/2011 01:28 AM, Rusty Russell wrote:
> >> >  after some preliminary discussion on the QEMU mailing list, I present a
> >> >  draft specification for a virtio-based SCSI host (controller, HBA, you
> >> >  name it).
> >
> > OK, I'm impressed.  This is very well written and it doesn't make any of
> > the obvious mistakes wrt. virtio.
> 
> Thanks very much, and thanks to those who corrected my early mistakes.
> 
> > I assume you have an implementation, as well?
> 
> Unfortunately not; "we're working on it", which means I should start in 
> July when I come back from vacation.
> 
> Do you prefer to wait for one before I make a patch to the LyX source? 
> In the meanwhile, can you reserve a subsystem ID for me?
> 
> Paolo

Sure, you can have the next subsystem ID.

It's a pain to patch once it's in LyX, so let's get the implementation
base on what you posted here an see how much it changes first...

Cheers,
Rusty.

Re: [Qemu-devel] [PATCH] coroutine: Implement coroutines using gthread

[Aneesh Kumar K.V]

On Fri, 10 Jun 2011 11:24:20 +0100, Stefan Hajnoczi  wrote:
> On Thu, Jun 9, 2011 at 6:41 PM, Aneesh Kumar K.V
>  wrote:
> > On platforms that doesn't support makecontext use gthread
> > based coroutine implementation.
> >
> > Signed-off-by: Aneesh Kumar K.V 
> > ---
> >
> > NOTE: Tested on linux with force compliation of coroutine-gthread.c
> >
> >  Makefile.objs       |    5 ++
> >  configure           |   18 +
> >  coroutine-gthread.c |  172 
> > +++
> >  3 files changed, 195 insertions(+), 0 deletions(-)
> >  create mode 100644 coroutine-gthread.c
> >
> > diff --git a/Makefile.objs b/Makefile.objs
> > index 0f1d7df..d354d3c 100644
> > --- a/Makefile.objs
> > +++ b/Makefile.objs
> > @@ -13,9 +13,14 @@ oslib-obj-$(CONFIG_POSIX) += oslib-posix.o 
> > qemu-thread-posix.o
> >  ###
> >  # coroutines
> >  coroutine-obj-y = qemu-coroutine.o qemu-coroutine-lock.o
> > +ifeq ($(CONFIG_UCONTEXT_COROUTINE),y)
> >  coroutine-obj-$(CONFIG_POSIX) += coroutine-ucontext.o
> > +else
> > +coroutine-obj-$(CONFIG_POSIX) += coroutine-gthread.o
> > +endif
> >  coroutine-obj-$(CONFIG_WIN32) += coroutine-win32.o
> >
> > +
> >  ###
> >  # block-obj-y is code used by both qemu system emulation and qemu-img
> >
> > diff --git a/configure b/configure
> > index 980914a..529d8c4 100755
> > --- a/configure
> > +++ b/configure
> > @@ -2568,6 +2568,20 @@ if test "$trace_backend" = "dtrace"; then
> >  fi
> >
> >  ##
> > +# check if we have makecontext
> > +
> > +ucontext_coroutine=no
> > +cat > $TMPC << EOF
> > +#include 
> > +int main(void) { makecontext(0, 0, 0); }
> > +EOF
> > +if compile_prog "" "" ; then
> > +    ucontext_coroutine=yes
> > +fi
> > +
> > +
> > +
> > +##
> >  # End of CC checks
> >  # After here, no more $cc or $ld runs
> >
> > @@ -3031,6 +3045,10 @@ if test "$rbd" = "yes" ; then
> >   echo "CONFIG_RBD=y" >> $config_host_mak
> >  fi
> >
> > +if test "$ucontext_coroutine" = "yes" ; then
> > +  echo "CONFIG_UCONTEXT_COROUTINE=y" >> $config_host_mak
> > +fi
> > +
> >  # USB host support
> >  case "$usb" in
> >  linux)
> > diff --git a/coroutine-gthread.c b/coroutine-gthread.c
> > new file mode 100644
> > index 000..37e5a16
> > --- /dev/null
> > +++ b/coroutine-gthread.c
> > @@ -0,0 +1,172 @@
> > +/*
> 
> A summary of the purpose of this file would be nice.
> 
> > + *
> > + * Copyright (C) 2006  Anthony Liguori 
> > + * Copyright (C) 2011  Aneesh Kumar K.V 
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.0 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  
> > 02110-1301 USA
> 
> The URL is preferred over the postal address:
> 
>  * You should have received a copy of the GNU Lesser General Public
>  * License along with this library; if not, see
>  .

inherited that header from gtk-vnc version of original file
> 
> > + */
> > +
> > +#include "qemu-common.h"
> > +#include "qemu-coroutine-int.h"
> > +#include 

Will fix.

> 
> It's good practice to include system headers first unless you are
> explicitly setting a magic #define to affect the system header.
> 
> > +
> > +typedef struct {
> > +    Coroutine qemu_co;
> > +    GThread *thread;
> > +    gboolean runnable;
> > +} CoroutineGthread;
> > +
> > +typedef struct {
> > +    /** Currently executing coroutine */
> > +    CoroutineGthread *current;
> > +
> > +    /** The default coroutine */
> > +    CoroutineGthread leader;
> > +} CoroutineThreadState;
> > +
> > +static GCond *run_cond;
> > +static GMutex *run_lock;
> > +static pthread_key_t thread_state_key;
> 
> Why pthread_key_t instead of GStaticPrivate?

No specific reason other than I was not aware of GStaticPrivate. 

> 
> I've followed the main control flow once but am going to review in
> more detail to make sure.
> 

Thanks.
-aneesh

Re: [Qemu-devel] [PATCH] Replaced tabs with spaces in block.h and block_int.h

[Kevin Wolf]

Am 09.06.2011 07:06, schrieb Devin Nakamura:
> Signed-off-by: Devin Nakamura 
> ---
>  block.h |6 +++---
>  block_int.h |4 ++--
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/block.h b/block.h
> index da7d39c..859d1d9 100644
> --- a/block.h
> +++ b/block.h
> @@ -110,7 +110,7 @@ int bdrv_check(BlockDriverState *bs, BdrvCheckResult 
> *res);
>  typedef struct BlockDriverAIOCB BlockDriverAIOCB;
>  typedef void BlockDriverCompletionFunc(void *opaque, int ret);
>  typedef void BlockDriverDirtyHandler(BlockDriverState *bs, int64_t sector,
> -  int sector_num);
> + int sector_num);
>  BlockDriverAIOCB *bdrv_aio_readv(BlockDriverState *bs, int64_t sector_num,
>   QEMUIOVector *iov, int nb_sectors,
>   BlockDriverCompletionFunc *cb, void 
> *opaque);
> @@ -118,7 +118,7 @@ BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState
> *bs, int64_t sector_num,

The patch is corrupted by this line wrap. This won't happen if you use
git send-email. Stefan posted his config for GMail recently, maybe it
helps you:

> The GMail web interface *always* wraps lines, there doesn't seem to be
> a way to disable that.  If you send it through GMail's SMTP servers
> with git-send-email(1) you'll be fine.
> 
> Here is my .gitconfig:
> [sendemail]
> smtpserver = smtp.gmail.com
> smtpserverport = 587
> smtpencryption = tls
> smtpuser = stefa...@gmail.com
> chainreplyto = false

Kevin

Re: [Qemu-devel] [PATCH-v3 2/2] kvm: Fix unused-but-set-variable warning

[Christophe Fergeau]

Ping?

On Tue, May 31, 2011 at 09:53:49AM +0200, Christophe Fergeau wrote:
> Based on a patch from Hans de Goede 
> 
> This warning is new in gcc 4.6.
> 
> Acked-by: Amit Shah 
> Signed-off-by: Christophe Fergeau 
> ---
>  target-i386/kvm.c |3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index faedc6c..58a70bc 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -970,7 +970,7 @@ static int kvm_get_xsave(CPUState *env)
>  #ifdef KVM_CAP_XSAVE
>  struct kvm_xsave* xsave;
>  int ret, i;
> -uint16_t cwd, swd, twd, fop;
> +uint16_t cwd, swd, twd;
>  
>  if (!kvm_has_xsave()) {
>  return kvm_get_fpu(env);
> @@ -986,7 +986,6 @@ static int kvm_get_xsave(CPUState *env)
>  cwd = (uint16_t)xsave->region[0];
>  swd = (uint16_t)(xsave->region[0] >> 16);
>  twd = (uint16_t)xsave->region[1];
> -fop = (uint16_t)(xsave->region[1] >> 16);
>  env->fpstt = (swd >> 11) & 7;
>  env->fpus = swd;
>  env->fpuc = cwd;
> -- 
> 1.7.5.2
> 
> 


pgp908Z7e0qyc.pgp
Description: PGP signature

[Qemu-devel] QEMU 9pfs intentionally returning short reads ?

[Daniel P. Berrange]

I've been doing some work trying to run QEMU guests with a root filesystem
exported from the host using virtio 9pfs. One of the issues that I have
discovered is that the 9p FS running on QEMU appears to cap all reads at
4096 bytes[1]. Any larger read will return only partial data for plain
files.

Now this is allowed by POSIX, but in practice most filesystems will always
return the full amount of requested data for plain files, so this surprises
some (buggy) applications. In particular the 9pfs behaviour has broken Xorg's
xkbcomp program (well anything calling XrmGetFileDatabase) which results i
a non-functional keyboard for the guest X server. I filed a BZ against
libX11 to get this fixed:

  https://bugzilla.redhat.com/show_bug.cgi?id=712190

I'm wondering if this behaviour of QEMU is actually intentional though ?

This short read behaviour appears to be a result of this change from last
year:

  commit 5e94c103a0321ca47deb81643839c44a03047416
  Author: M. Mohan Kumar 
  Date:   Wed Jun 9 19:14:28 2010 +0530

virtio-9p: Compute iounit based on host filesystem block size

Compute iounit based on the host filesystem block size and pass it to
client with open/create response. Also return iounit as statfs's f_bsize
for optimal block size transfers.

Signed-off-by: M. Mohan Kumar 
Reviewd-by: Sripathi Kodi 
Signed-off-by: Venkateswararao Jujjuri 


Before that change, a read of 8192 bytes, returns 8192 bytes, after that
any read will return a max of 4096 bytes[1].

If this is intentional, then it is something for users/developers to
watch out for if running into wierd I/O errors with apps in QEMU guests
with 9pfs. It wouldn't surprise me if more apps were not correctly coping
with short reads on plain files.

Daniel

[1] It isn't strictly a 4096 byte limit. It is actually limited to the value
of the 'f_bsize' field of statfs() on the filesystem being passed through.
-- 
|: 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] coroutine: Implement coroutines using gthread

[Stefan Hajnoczi]

On Thu, Jun 9, 2011 at 6:41 PM, Aneesh Kumar K.V
 wrote:
> On platforms that doesn't support makecontext use gthread
> based coroutine implementation.
>
> Signed-off-by: Aneesh Kumar K.V 
> ---
>
> NOTE: Tested on linux with force compliation of coroutine-gthread.c
>
>  Makefile.objs       |    5 ++
>  configure           |   18 +
>  coroutine-gthread.c |  172 
> +++
>  3 files changed, 195 insertions(+), 0 deletions(-)
>  create mode 100644 coroutine-gthread.c
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 0f1d7df..d354d3c 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -13,9 +13,14 @@ oslib-obj-$(CONFIG_POSIX) += oslib-posix.o 
> qemu-thread-posix.o
>  ###
>  # coroutines
>  coroutine-obj-y = qemu-coroutine.o qemu-coroutine-lock.o
> +ifeq ($(CONFIG_UCONTEXT_COROUTINE),y)
>  coroutine-obj-$(CONFIG_POSIX) += coroutine-ucontext.o
> +else
> +coroutine-obj-$(CONFIG_POSIX) += coroutine-gthread.o
> +endif
>  coroutine-obj-$(CONFIG_WIN32) += coroutine-win32.o
>
> +
>  ###
>  # block-obj-y is code used by both qemu system emulation and qemu-img
>
> diff --git a/configure b/configure
> index 980914a..529d8c4 100755
> --- a/configure
> +++ b/configure
> @@ -2568,6 +2568,20 @@ if test "$trace_backend" = "dtrace"; then
>  fi
>
>  ##
> +# check if we have makecontext
> +
> +ucontext_coroutine=no
> +cat > $TMPC << EOF
> +#include 
> +int main(void) { makecontext(0, 0, 0); }
> +EOF
> +if compile_prog "" "" ; then
> +    ucontext_coroutine=yes
> +fi
> +
> +
> +
> +##
>  # End of CC checks
>  # After here, no more $cc or $ld runs
>
> @@ -3031,6 +3045,10 @@ if test "$rbd" = "yes" ; then
>   echo "CONFIG_RBD=y" >> $config_host_mak
>  fi
>
> +if test "$ucontext_coroutine" = "yes" ; then
> +  echo "CONFIG_UCONTEXT_COROUTINE=y" >> $config_host_mak
> +fi
> +
>  # USB host support
>  case "$usb" in
>  linux)
> diff --git a/coroutine-gthread.c b/coroutine-gthread.c
> new file mode 100644
> index 000..37e5a16
> --- /dev/null
> +++ b/coroutine-gthread.c
> @@ -0,0 +1,172 @@
> +/*

A summary of the purpose of this file would be nice.

> + *
> + * Copyright (C) 2006  Anthony Liguori 
> + * Copyright (C) 2011  Aneesh Kumar K.V 
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.0 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 
> USA

The URL is preferred over the postal address:

 * You should have received a copy of the GNU Lesser General Public
 * License along with this library; if not, see .

> + */
> +
> +#include "qemu-common.h"
> +#include "qemu-coroutine-int.h"
> +#include 

It's good practice to include system headers first unless you are
explicitly setting a magic #define to affect the system header.

> +
> +typedef struct {
> +    Coroutine qemu_co;
> +    GThread *thread;
> +    gboolean runnable;
> +} CoroutineGthread;
> +
> +typedef struct {
> +    /** Currently executing coroutine */
> +    CoroutineGthread *current;
> +
> +    /** The default coroutine */
> +    CoroutineGthread leader;
> +} CoroutineThreadState;
> +
> +static GCond *run_cond;
> +static GMutex *run_lock;
> +static pthread_key_t thread_state_key;

Why pthread_key_t instead of GStaticPrivate?

I've followed the main control flow once but am going to review in
more detail to make sure.

Stefan

[Qemu-devel] fsdev - broken qemu-system-?

[Edgar E. Iglesias]

At least CRIS, Microblaze and lm32 are broken on latest git. Things fail with
the following message:
qemu-system-cris: there is no option group "fsdev"

Under GDB I see a segfault...

% gdb --args ~/src/c/qemu/git/build-qemu/cris-softmmu/qemu-system-cris -M 
axis-dev88 -kernel kimage -serial stdio
GNU gdb (Gentoo 7.2 p1) 7.2
Copyright (C) 2010 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
For bug reporting instructions, please see:
...
Reading symbols from 
/home/edgar/src/c/qemu/git/build-qemu/cris-softmmu/qemu-system-cris...done.
(gdb) run
Starting program: 
/home/edgar/src/c/qemu/git/build-qemu/cris-softmmu/qemu-system-cris -M 
axis-dev88 -kernel kimage -serial stdio
[Thread debugging using libthread_db enabled]
qemu-system-cris: there is no option group "fsdev"

Program received signal SIGSEGV, Segmentation fault.
qemu_opts_foreach (list=0x0, func=0x4fd210 , opaque=0x0, 
abort_on_failure=1) at /home/edgar/src/c/qemu/git/qemu/qemu-option.c:969
969 QTAILQ_FOREACH(opts, &list->head, next) {
(gdb) bt
#0  qemu_opts_foreach (list=0x0, func=0x4fd210 , opaque=0x0, 
abort_on_failure=1) at /home/edgar/src/c/qemu/git/qemu/qemu-option.c:969
#1  0x004ff05d in main (argc=7, argv=0x7fffdc28, 
envp=) at /home/edgar/src/c/qemu/git/qemu/vl.c:3015
(gdb) q
A debugging session is active.

Inferior 1 [process 1473] will be killed.

Quit anyway? (y or n) y

Re: [Qemu-devel] [PATCH] [trivial] print meaningful error message in case of --disable-vhost-net

[Stefan Hajnoczi]

On Thu, Jun 9, 2011 at 9:55 PM, Michael Tokarev  wrote:
> When qemu gets compiled without support of vhost-net, any attempt
> to use it fails with a very clear error message:
>
>  qemu-system-x86_64: -netdev ...,vhost=on: vhost-net requested but could not 
> be initialized
>
> there's absolutely no reason given _why_ it coult not be
> initialized, and even strace'ing the process in question
> does not reveal any errors.  So print a message telling
> what's going on.
>
> Signed-off-by: Michael Tokarev 
> ---
>  hw/vhost_net.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)

This is useful.  CCed Michael Tsirkin.

Stefan

Re: [Qemu-devel] [PATCH RFC 0/3] basic support for composing sysbus devices

[Markus Armbruster]

Jan Kiszka  writes:

> On 2011-06-09 18:40, Markus Armbruster wrote:
>> Jan Kiszka  writes:
>> 
>>> On 2011-06-08 13:33, Peter Maydell wrote:
 At the moment you can't really implement one sysbus device by saying
 that it's composed of a set of other sysbus devices. This patch adds
 new functions sysbus_pass_mmio() and sysbus_pass_one_irq() which
 allow a sysbus device to delegate an MMIO or IRQ to another sysbus
 device (The approach is inspired by the existing sysbus_pass_irq()
 which lets a sysbus device delegate all its IRQs at once).

 This works; the most obvious deficiency is that the subcomponent
 device will still appear as its own device on the bus.

 So: is this a reasonable solution to the problem, or an unacceptable
 hack? Comments welcome :-)
>>>
>>> Sounds more like a little hack. :)
>>>
>>> The relationships should be expressed via qdev, not yet another
>>> sysbus-specific extension. Generally, many services of sysbus should
>>> rather be generic qdev things.
>> 
>> Examples?
>
> Resource management, e.g. IRQs. That will be useful for other types of
> buses as well.

A device should be able to say "I need to be connected to an IRQ line".
Feels generic to me.

A device or bus should be able to offer IRQ lines.  Feels generic, too.

Connecting the two is configuration.  Requires a suitable naming scheme
for IRQ lines.  Naming might have to include bus-specific sugar for
user-friendliness.  For instance, I'd rather express "isa-serial uses
ISA interrupt 4" as "irq=4" than as something like
"irq=PIIX3/isa.0:irq.4".

I doubt all resources are as generic as IRQ lines, but that's okay.

>>> Is there anything that today prevents creating a local bus and attaching
>>> the component devices to that? If it's multi-bus support, that should to
>>> be added anyway. Passing-through of MMIO and IRQs is still a worthwhile
>>> generic service, then probably qbus associated.
>> 
>> Do you mean making the container device a sysbus-sysbus-bridge, then
>> hanging the component devices off the inner sysbus?
>
> And how to apply this concept on a composed PCI device e.g.?

Make the PCI device provide a sysbus for its components?  Dunno...

See also thread "[RFC v4 00/12] ISA reconfigurability v4".

> Maybe we could define something like Linux' struct resource + a set of
> helper services for it.

Device component composition is not (yet?) covered by qdev.  Of course
we compose anyway, in various ad hoc ways.

One way is to put the components' state structs into the device's state
struct, and define suitable wrappers.  For instance, we have qdevs
"sysbus-fdc" and "isa-fdc".  They both contain the FDC proper as a
component: FDCtrlSysBus and FDCtrlISABus contain a FDCtrl member.
Simple enough.

A different way to adapt the same component to different buses can be
found in virtio: VirtIOPCIProxy and VirtIOS390Device both contain
pointers to VirtIODevice.  I found that one quite awkward to work with.

Yet another way can be found in usb-storage.  usb-storage expands into
two qdevs connected by a qbus: it provides a SCSI bus, and automatically
creates a single scsi-disk on that bus.  One of those tricks that look
cute initially, then create no end of trouble.

I figure a "qdevy" composition mechanism would be useful.  Needs
thought.

Re: [Qemu-devel] [PATCH] qemu-kvm: fix pulseaudio detection in configure

[Markus Armbruster]

Peter Maydell  writes:

> On 9 June 2011 18:44, Andreas Färber  wrote:
>> Am 09.06.2011 um 17:52 schrieb Marc-Antoine Perennou:
>>> Manually including stddef.h or replacing NULL by 0 or (void*)0 makes it
>>> work.
>>
>> Then please submit a new patch that explicit includes that header with a
>> comment explaining why, so that it doesn't get removed by accident.
>
> I think the original patch is fine. This configure test isn't
> trying to test whether we correctly found some definition of
> NULL, it's testing pulseaudio presence. The test code should
> be the simplest and most stand-alone code that achieves that.
> There's no need to go dragging in extra headers when we don't
> even need to be using NULL here anyhow.
>
> We already have configure tests which use unadorned 0 for
> pointers (eg the pthread test), it's legal C, and it works.
> I think we should call this a trivial patch and just apply it.
>
> Reviewed-by: Peter Maydell 

I agree with your reasoning.