Re: [Qemu-devel] [RFC PATCH 0/8] Remove stub mon-protocol-event for block

[Wenchao Xia]

于 2013/9/16 22:27, Wenchao Xia 写道:

于 2013/9/16 18:02, Paolo Bonzini 写道:

Il 16/09/2013 06:59, Wenchao Xia ha scritto:

于 2013/9/12 17:31, Paolo Bonzini 写道:

Il 12/09/2013 11:15, Wenchao Xia ha scritto:

This series will remove the usage of symbols of mon-protocol-event in
qemu-img, qemu-nbd and qemu-io, in short remove the connetion for 
block

layer.

Background:
I am tring to decouple block layer code with other unnnessary
components,
and in ./stub there many symbols that qemu-img linked as fake
implemtion.
As a first step, I am decouple monitor with block layer code, this is
the
first part of it.
There are still other stub symbols for monitor, which will be
solved later.
It seems error handlering is also link with those symbols, and will
adjust
that.

Wenchao Xia (8):
1 block: use type MonitorEvent directly
2 block: do not include monitor.h in block.c
3 qapi: move MonitorEvent define
4 qapi: rename MonitorEvent to QEvent
5 block: add a callback layer for common functions
6 block: replace monitor_protocol_event() with callback
7 block: do not include monitor.h
7 stubs: remove mon-protocol-event.o in stub obj

   block.c|   22 ++
   block/qcow2-refcount.c |4 +++-
   blockjob.c |   10 --
   include/block/block.h  |   12 
   include/block/block_int.h  |3 +--
   include/monitor/monitor.h  |   40
++--
   include/qapi/qmp/qevent.h  |   41
+
   include/qapi/qmp/types.h   |1 +
   monitor.c  |   12 ++--
   stubs/Makefile.objs|1 -
   stubs/mon-protocol-event.c |2 +-
   tests/Makefile |3 ++-
   ui/vnc.c   |2 +-
   vl.c   |4 
   14 files changed, 100 insertions(+), 57 deletions(-)
   create mode 100644 include/qapi/qmp/qevent.h

Patches 1-4 look good.  I'm not sure of the advantage of the last 
four,

however.  The ugly part of monitor_protocol_event is not really the
stub, but the dependency on QObject.

I think replacing QObject with QAPI types is another issue we could
improve. The last
foure patches tries decouple monitor code with block layer code from
build time.
The files in ./stubs is needed since some symbols are needed in some
program which don't
need it really, and I think, that tips the code are not organized very
well in .c file level.

That may very well be the case.  However, picking a function, replacing
it with a function pointer, and throwing it into a struct, will not
improve the structure at the .c file level very much.

It does not abstract anything.  In order to provide a useful
abstractions, the questions to answer are:

1) If a library wanted to provide a callback for events, would the
current design (using QObject) be the right thing to do?  (If you change
the design, it might happen that the stub goes away naturally and that
  I think it would not goes away even QAPI is used. The ./stubs is a 
symbol
issue at link time, so whenever block layer use a symbol belong to 
monitor component,

the stubs would be needed.
  In my opinion, there could be two steps: remove this symbol in block 
code,

replace it with QAPI in monitor code.


the work in these patches does nothing except obscure the history).

2) Are there other services that the block layer could desire from the
monitor?


  I have a draft makefile which link the core block code, it
shows that only event and error printf(include mon_is_qmp) service
are used by block code now. Those symbols basically are listed in 
./stubs.



3) Could any tool (e.g. qemu-io) desire to implement
monitor_protocol_event?  If so, what other services could it provide
that the QEMU monitor provides?


  It seems tools do not need event service now, they link with stubs.


After removing ./stubs, the code will be more clear

If done in the right way, yes.  If done in the wrong way, the code will
be less clear.

Paolo






Hi Paolo,
  This series tries to find a mechanism, which can avoid link with unused
symbols. Assume QObject was replaced with other design, such as QAPI, it
will still call monitor funcions to emit the event, so it will still require
resolving that function in linking of qemu-img.
  Do you have some suggestion for it?

Re: [Qemu-devel] [RFC PATCH 0/8] Remove stub mon-protocol-event for block

[Paolo Bonzini]

Il 23/09/2013 21:06, Wenchao Xia ha scritto:
>>
> 
> Hi Paolo,
>   This series tries to find a mechanism, which can avoid link with unused
> symbols. Assume QObject was replaced with other design, such as QAPI, it
> will still call monitor funcions to emit the event, so it will still
> require
> resolving that function in linking of qemu-img.
>   Do you have some suggestion for it?

No, I don't.  Still, it seems to me that (for now) this series does not
solve any real problem.

Paolo

Re: [Qemu-devel] [PATCH 13/17] blockdev: Remove IF_* check for read-only blockdev_init

[Max Reitz]

On 2013-09-20 13:54, Kevin Wolf wrote:

IF_NONE allows read-only, which makes forbidding it in this place
for other types pretty much pointless.

Instead, make sure that all devices for which the check would have
errored out check in their init function that they don't get a read-only
BlockDriverState. This catches even cases where IF_NONE and -device is
used.

Signed-off-by: Kevin Wolf 
---
  blockdev.c | 6 --
  hw/block/m25p80.c  | 5 +
  hw/block/xen_disk.c| 6 ++
  hw/sd/milkymist-memcard.c  | 4 
  hw/sd/omap_mmc.c   | 8 
  hw/sd/pl181.c  | 4 
  hw/sd/pxa2xx_mmci.c| 4 
  hw/sd/sd.c | 5 +
  hw/sd/sdhci.c  | 4 
  hw/sd/ssi-sd.c | 3 +++
  tests/qemu-iotests/051.out | 5 -
  11 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index bbdae31..ed4ba65 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -529,12 +529,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
  if (media == MEDIA_CDROM) {
  /* CDROM is fine for any interface, don't check.  */
  ro = 1;
-} else if (ro == 1) {
-if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY &&
-type != IF_NONE && type != IF_PFLASH) {
-error_report("read-only not supported by this bus type");
-goto err;
-}
  }
  
  bdrv_flags |= ro ? 0 : BDRV_O_RDWR;

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 8c3b7f0..02a1544 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -624,6 +624,11 @@ static int m25p80_init(SSISlave *ss)
  if (dinfo && dinfo->bdrv) {
  DB_PRINT_L(0, "Binding to IF_MTD drive\n");
  s->bdrv = dinfo->bdrv;
+if (bdrv_is_read_only(s->bdrv)) {
+fprintf(stderr, "Can't use a read-only drive");
+return 1;
+}
+
  /* FIXME: Move to late init */
  if (bdrv_read(s->bdrv, 0, s->storage, DIV_ROUND_UP(s->size,
  BDRV_SECTOR_SIZE))) {
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index f35fc59..828c9d9 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -39,6 +39,7 @@
  #include "hw/xen/xen_backend.h"
  #include "xen_blkif.h"
  #include "sysemu/blockdev.h"
+#include "qemu/error-report.h"
  
  /* - */
  
@@ -829,6 +830,11 @@ static int blk_connect(struct XenDevice *xendev)

  /* setup via qemu cmdline -> already setup for us */
  xen_be_printf(&blkdev->xendev, 2, "get configured bdrv (cmdline 
setup)\n");
  blkdev->bs = blkdev->dinfo->bdrv;
+if (bdrv_is_read_only(blkdev->bs) && !readonly) {
+error_report("Unexpected read-only drive");
+blkdev->bs = NULL;
+return -1;
+}
Just out of curiosity, why do you use error_report here but 
fprintf(stderr) in most other places?


Max


  /* blkdev->bs is not create by us, we get a reference
   * so we can bdrv_unref() unconditionally */
  bdrv_ref(blkdev->bs);
diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
index 42613b3..d1168c9 100644
--- a/hw/sd/milkymist-memcard.c
+++ b/hw/sd/milkymist-memcard.c
@@ -255,6 +255,10 @@ static int milkymist_memcard_init(SysBusDevice *dev)
  
  dinfo = drive_get_next(IF_SD);

  s->card = sd_init(dinfo ? dinfo->bdrv : NULL, false);
+if (s->card == NULL) {
+return -1;
+}
+
  s->enabled = dinfo ? bdrv_is_inserted(dinfo->bdrv) : 0;
  
  memory_region_init_io(&s->regs_region, OBJECT(s), &memcard_mmio_ops, s,

diff --git a/hw/sd/omap_mmc.c b/hw/sd/omap_mmc.c
index bf5d1fb..8c6ab2e 100644
--- a/hw/sd/omap_mmc.c
+++ b/hw/sd/omap_mmc.c
@@ -593,6 +593,10 @@ struct omap_mmc_s *omap_mmc_init(hwaddr base,
  
  /* Instantiate the storage */

  s->card = sd_init(bd, false);
+if (s->card == NULL) {
+fprintf(stderr, "Failed to initialise SD card\n");
+exit(1);
+}
  
  return s;

  }
@@ -618,6 +622,10 @@ struct omap_mmc_s *omap2_mmc_init(struct 
omap_target_agent_s *ta,
  
  /* Instantiate the storage */

  s->card = sd_init(bd, false);
+if (s->card == NULL) {
+fprintf(stderr, "Failed to initialise SD card\n");
+exit(1);
+}
  
  s->cdet = qemu_allocate_irqs(omap_mmc_cover_cb, s, 1)[0];

  sd_set_cb(s->card, NULL, s->cdet);
diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c
index 03875bf..c35896d 100644
--- a/hw/sd/pl181.c
+++ b/hw/sd/pl181.c
@@ -491,6 +491,10 @@ static int pl181_init(SysBusDevice *sbd)
  qdev_init_gpio_out(dev, s->cardstatus, 2);
  dinfo = drive_get_next(IF_SD);
  s->card = sd_init(dinfo ? dinfo->bdrv : NULL, false);
+if (s->card == NULL) {
+return -1;
+}
+
  return 0;
  }
  
diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c

index 90c955f..d720bc2 100644
--- a/hw/sd/px

Re: [Qemu-devel] [PATCH 13/17] blockdev: Remove IF_* check for read-only blockdev_init

[Kevin Wolf]

Am 23.09.2013 um 10:00 hat Max Reitz geschrieben:
> On 2013-09-20 13:54, Kevin Wolf wrote:
> >IF_NONE allows read-only, which makes forbidding it in this place
> >for other types pretty much pointless.
> >
> >Instead, make sure that all devices for which the check would have
> >errored out check in their init function that they don't get a read-only
> >BlockDriverState. This catches even cases where IF_NONE and -device is
> >used.
> >
> >Signed-off-by: Kevin Wolf 
> >---
> >  blockdev.c | 6 --
> >  hw/block/m25p80.c  | 5 +
> >  hw/block/xen_disk.c| 6 ++
> >  hw/sd/milkymist-memcard.c  | 4 
> >  hw/sd/omap_mmc.c   | 8 
> >  hw/sd/pl181.c  | 4 
> >  hw/sd/pxa2xx_mmci.c| 4 
> >  hw/sd/sd.c | 5 +
> >  hw/sd/sdhci.c  | 4 
> >  hw/sd/ssi-sd.c | 3 +++
> >  tests/qemu-iotests/051.out | 5 -
> >  11 files changed, 47 insertions(+), 7 deletions(-)
> >
> >diff --git a/blockdev.c b/blockdev.c
> >index bbdae31..ed4ba65 100644
> >--- a/blockdev.c
> >+++ b/blockdev.c
> >@@ -529,12 +529,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
> >  if (media == MEDIA_CDROM) {
> >  /* CDROM is fine for any interface, don't check.  */
> >  ro = 1;
> >-} else if (ro == 1) {
> >-if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY &&
> >-type != IF_NONE && type != IF_PFLASH) {
> >-error_report("read-only not supported by this bus type");
> >-goto err;
> >-}
> >  }
> >  bdrv_flags |= ro ? 0 : BDRV_O_RDWR;
> >diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> >index 8c3b7f0..02a1544 100644
> >--- a/hw/block/m25p80.c
> >+++ b/hw/block/m25p80.c
> >@@ -624,6 +624,11 @@ static int m25p80_init(SSISlave *ss)
> >  if (dinfo && dinfo->bdrv) {
> >  DB_PRINT_L(0, "Binding to IF_MTD drive\n");
> >  s->bdrv = dinfo->bdrv;
> >+if (bdrv_is_read_only(s->bdrv)) {
> >+fprintf(stderr, "Can't use a read-only drive");
> >+return 1;
> >+}
> >+
> >  /* FIXME: Move to late init */
> >  if (bdrv_read(s->bdrv, 0, s->storage, DIV_ROUND_UP(s->size,
> >  BDRV_SECTOR_SIZE))) {
> >diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> >index f35fc59..828c9d9 100644
> >--- a/hw/block/xen_disk.c
> >+++ b/hw/block/xen_disk.c
> >@@ -39,6 +39,7 @@
> >  #include "hw/xen/xen_backend.h"
> >  #include "xen_blkif.h"
> >  #include "sysemu/blockdev.h"
> >+#include "qemu/error-report.h"
> >  /* - */
> >@@ -829,6 +830,11 @@ static int blk_connect(struct XenDevice *xendev)
> >  /* setup via qemu cmdline -> already setup for us */
> >  xen_be_printf(&blkdev->xendev, 2, "get configured bdrv (cmdline 
> > setup)\n");
> >  blkdev->bs = blkdev->dinfo->bdrv;
> >+if (bdrv_is_read_only(blkdev->bs) && !readonly) {
> >+error_report("Unexpected read-only drive");
> >+blkdev->bs = NULL;
> >+return -1;
> >+}
> Just out of curiosity, why do you use error_report here but
> fprintf(stderr) in most other places?

Variatio delectat. ;-)

I guess fprintf + exit was a common pattern elsewhere, so I used it,
whereas error_report() is my default function. But now that I look at
this file again, maybe it should really be xen_be_printf() here...

Kevin

Re: [Qemu-devel] [PATCH 05/17] blockdev: Separate ID generation from DriveInfo creation

[Max Reitz]

On 2013-09-20 13:54, Kevin Wolf wrote:

blockdev-add shouldn't automatically generate IDs, but will keep most of
the DriveInfo creation code.

Signed-off-by: Kevin Wolf 
---
  blockdev.c| 32 +---
  include/qemu/option.h |  1 +
  util/qemu-option.c|  6 ++
  3 files changed, 24 insertions(+), 15 deletions(-)

Reviewed-by: Max Reitz 

Re: [Qemu-devel] [PATCH 11/17] blockdev: Move bus/unit/index processing to drive_init

[Max Reitz]

On 2013-09-20 13:54, Kevin Wolf wrote:

This requires moving the automatic ID generation at the same time, so
let's do that as well.

Signed-off-by: Kevin Wolf 
---
  blockdev.c | 157 -
  1 file changed, 73 insertions(+), 84 deletions(-)


Reviewed-by: Max Reitz 

Re: [Qemu-devel] [PATCH 12/17] blockdev: Move virtio-blk device creation to drive_init

[Max Reitz]

On 2013-09-20 13:54, Kevin Wolf wrote:

Signed-off-by: Kevin Wolf 
---
  blockdev.c | 54 +++---
  1 file changed, 27 insertions(+), 27 deletions(-)


Aside from return NULL instead of goto fail:

Reviewed-by: Max Reitz 

Re: [Qemu-devel] [PATCH 14/17] qemu-iotests: Check autodel behaviour for device_del

[Max Reitz]

On 2013-09-20 13:54, Kevin Wolf wrote:

Block devices creates with -drive and drive_add should automatically
disappear if the guest device is unplugged. blockdev-add ones shouldn't.

Signed-off-by: Kevin Wolf 
---
  tests/qemu-iotests/064   | 133 +++
  tests/qemu-iotests/064.out   |  80 +++
  tests/qemu-iotests/common.filter |   8 +++
  tests/qemu-iotests/group |   1 +
  4 files changed, 222 insertions(+)
  create mode 100755 tests/qemu-iotests/064
  create mode 100644 tests/qemu-iotests/064.out


With manual CR reconstruction:

Reviewed-by: Max Reitz 

Re: [Qemu-devel] [PATCH 15/17] blockdev: Remove 'media' parameter from blockdev_init()

[Max Reitz]

On 2013-09-20 13:54, Kevin Wolf wrote:

The remaining users shouldn't be there with blockdev-add and are easy to
move to drive_init().

Bonus bug fix: As a side effect, CD-ROM drives can now use block drivers
on the read-only whitelist without explicitly specifying read-only=on,
even if a format is explicitly specified.

Signed-off-by: Kevin Wolf 
---
  blockdev.c | 40 +++-
  1 file changed, 15 insertions(+), 25 deletions(-)


Reviewed-by: Max Reitz 

Re: [Qemu-devel] [PATCH 17/17] blockdev: blockdev_init() error conversion

[Max Reitz]

On 2013-09-20 13:54, Kevin Wolf wrote:

This gives us meaningful error messages for the blockdev-add QMP
command.

Signed-off-by: Kevin Wolf 
---
  blockdev.c | 59 +--
  1 file changed, 33 insertions(+), 26 deletions(-)


Reviewed-by: Max Reitz 

Re: [Qemu-devel] [PATCH 16/17] blockdev: Don't disable COR automatically with blockdev-add

[Max Reitz]

On 2013-09-20 13:54, Kevin Wolf wrote:

If a read-only device is configured with copy-on-read=on, the old code
only prints a warning and automatically disables copy on read. Make it
a real error for blockdev-add.

Signed-off-by: Kevin Wolf 
---
  block.c|  9 +++--
  blockdev.c | 31 +++
  2 files changed, 34 insertions(+), 6 deletions(-)


Reviewed-by: Max Reitz 

Re: [Qemu-devel] [PATCH 0/2] block: fix backing file overriding

[Kevin Wolf]

Am 22.09.2013 um 14:05 hat Fam Zheng geschrieben:
> The backing.file.filename option is not working as expected: if there's also a
> backing file name from the format driver, adding this option fails bdrv_open;
> if there's no backing file name info in the image, "info block" doesn't show
> the overrided file name.
> 
> A test case is updated to catch these issues.

Thanks, applied to the block branch.

Kevin

[Qemu-devel] [PATCH v2] vmdk: fix cluster size check for flat extents

[Fam Zheng]

We use the extent size as cluster size for flat extents (where no L1/L2
table is allocated so it's safe) reuse sector calculating code with
sparse extents.

Don't pass in the cluster size for adding flat extent, just set it to
sectors later, then the cluster size checking will not fail.

The cluster_sectors is changed to int64_t to allow big flat extent.

Without this, flat extent opening is broken:

# qemu-img create -f vmdk -o subformat=monolithicFlat /tmp/a.vmdk 100G
Formatting '/tmp/a.vmdk', fmt=vmdk size=107374182400 compat6=off 
subformat='monolithicFlat' zeroed_grain=off
# qemu-img info /tmp/a.vmdk
image: /tmp/a.vmdk
file format: raw
virtual size: 0 (0 bytes)
disk size: 4.0K

Signed-off-by: Fam Zheng 
---
 block/vmdk.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 96ef1b5..5d56e31 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -105,7 +105,7 @@ typedef struct VmdkExtent {
 uint32_t l2_cache_offsets[L2_CACHE_SIZE];
 uint32_t l2_cache_counts[L2_CACHE_SIZE];
 
-unsigned int cluster_sectors;
+int64_t cluster_sectors;
 } VmdkExtent;
 
 typedef struct BDRVVmdkState {
@@ -424,7 +424,7 @@ static int vmdk_add_extent(BlockDriverState *bs,
 extent->l1_size = l1_size;
 extent->l1_entry_sectors = l2_size * cluster_sectors;
 extent->l2_size = l2_size;
-extent->cluster_sectors = cluster_sectors;
+extent->cluster_sectors = flat ? sectors : cluster_sectors;
 
 if (s->num_extents > 1) {
 extent->end_sector = (*(extent - 1)).end_sector + extent->sectors;
@@ -741,7 +741,7 @@ static int vmdk_parse_extents(const char *desc, 
BlockDriverState *bs,
 VmdkExtent *extent;
 
 ret = vmdk_add_extent(bs, extent_file, true, sectors,
-0, 0, 0, 0, sectors, &extent);
+0, 0, 0, 0, 0, &extent);
 if (ret < 0) {
 return ret;
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] block: introduce BlockDriver.bdrv_needs_filename to enable some drivers.

[Kevin Wolf]

Am 20.09.2013 um 22:24 hat Benoît Canet geschrieben:
> Some drivers will have driver specifics options but no filename.
> This new bool allow the block layer to treat them correctly.
> 
> The first driver of this type will be the quorum driver.
> 
> Signed-off-by: Benoit Canet 
> ---
>  block.c   | 6 --
>  block/bochs.c | 1 +
>  block/cloop.c | 1 +
>  block/cow.c   | 1 +
>  block/dmg.c   | 1 +
>  block/gluster.c   | 4 
>  block/iscsi.c | 1 +
>  block/parallels.c | 1 +
>  block/qcow.c  | 1 +
>  block/qcow2.c | 1 +
>  block/qed.c   | 1 +
>  block/raw-posix.c | 5 +
>  block/raw-win32.c | 2 ++
>  block/raw_bsd.c   | 1 +
>  block/rbd.c   | 1 +
>  block/sheepdog.c  | 3 +++
>  block/vdi.c   | 1 +
>  block/vhdx.c  | 1 +
>  block/vmdk.c  | 1 +
>  block/vpc.c   | 1 +
>  include/block/block_int.h | 1 +
>  21 files changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 827549c..88099a1 100644
> --- a/block.c
> +++ b/block.c
> @@ -760,7 +760,8 @@ static int bdrv_open_common(BlockDriverState *bs, 
> BlockDriverState *file,
>  /* Open the image, either directly or using a protocol */
>  if (drv->bdrv_file_open) {
>  assert(file == NULL);
> -assert(drv->bdrv_parse_filename || filename != NULL);
> +assert(drv->bdrv_parse_filename || !drv->bdrv_needs_filename ||
> +   filename != NULL);

Doesn't drv->bdrv_parse_filename imply !drv->bdrv_needs_filename? (If
I'm not mistaken, it is only checked here anyway because there was no
bdrv_needs_filename yet.) I think it's enough to check the latter.

>  ret = drv->bdrv_file_open(bs, options, open_flags);
>  } else {
>  if (file == NULL) {
> @@ -870,7 +871,8 @@ int bdrv_file_open(BlockDriverState **pbs, const char 
> *filename,
>  goto fail;
>  }
>  qdict_del(options, "filename");
> -} else if (!drv->bdrv_parse_filename && !filename) {
> +} else if (!drv->bdrv_parse_filename && drv->bdrv_needs_filename &&
> +   !filename) {
>  qerror_report(ERROR_CLASS_GENERIC_ERROR,
>"The '%s' block driver requires a file name",
>drv->format_name);
> diff --git a/block/bochs.c b/block/bochs.c
> index d7078c0..72a73aa 100644
> --- a/block/bochs.c
> +++ b/block/bochs.c
> @@ -237,6 +237,7 @@ static void bochs_close(BlockDriverState *bs)
>  static BlockDriver bdrv_bochs = {
>  .format_name = "bochs",
>  .instance_size   = sizeof(BDRVBochsState),
> +.bdrv_needs_filename = true,

This looks wrong.

Format drivers which only implement .bdrv_open() (and not
.bdrv_file_open()) aren't even passed a filename. It would also be wrong
because they can be used above non-file protocols.

Do you get failures if you remove this line from format drivers? If so,
we need to look into that and fix it.

Kevin

Re: [Qemu-devel] [PATCH v4 19/23] piix: APIs for pc guest info

[Igor Mammedov]

On Sun, 22 Sep 2013 16:37:59 +0300
"Michael S. Tsirkin"  wrote:

> This adds APIs that will be used to fill in guest acpi tables.
> Some required information is still lacking in QOM, so we
> fall back on lookups by type and returning explicit types.
> 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  include/hw/acpi/piix4.h |  8 
>  include/hw/i386/pc.h|  1 +
>  hw/acpi/piix4.c | 44 
>  hw/pci-host/piix.c  |  8 
>  4 files changed, 57 insertions(+), 4 deletions(-)
>  create mode 100644 include/hw/acpi/piix4.h
> 
> diff --git a/include/hw/acpi/piix4.h b/include/hw/acpi/piix4.h
> new file mode 100644
> index 000..65e6fd7
> --- /dev/null
> +++ b/include/hw/acpi/piix4.h
> @@ -0,0 +1,8 @@
> +#ifndef HW_ACPI_PIIX4_H
> +#define HW_ACPI_PIIX4_H
> +
> +#include "qemu/typedefs.h"
> +
> +Object *piix4_pm_find(void);
> +
> +#endif
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index f966cef..2a5d996 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -193,6 +193,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int 
> *piix_devfn,
>  MemoryRegion *pci_memory,
>  MemoryRegion *ram_memory);
>  
> +PCIBus *find_i440fx(void);
>  /* piix4.c */
>  extern PCIDevice *piix4_dev;
>  int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn);
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index 4b8c1da..3bcd890 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -29,6 +29,7 @@
>  #include "exec/ioport.h"
>  #include "hw/nvram/fw_cfg.h"
>  #include "exec/address-spaces.h"
> +#include "hw/acpi/piix4.h"
>  
>  //#define DEBUG
>  
> @@ -69,6 +70,8 @@ typedef struct PIIX4PMState {
>  /*< public >*/
>  
>  MemoryRegion io;
> +uint32_t io_base;
> +
>  MemoryRegion io_gpe;
>  MemoryRegion io_pci;
>  MemoryRegion io_cpu;
> @@ -152,14 +155,13 @@ static void apm_ctrl_changed(uint32_t val, void *arg)
>  static void pm_io_space_update(PIIX4PMState *s)
>  {
>  PCIDevice *d = PCI_DEVICE(s);
> -uint32_t pm_io_base;
>  
> -pm_io_base = le32_to_cpu(*(uint32_t *)(d->config + 0x40));
> -pm_io_base &= 0xffc0;
> +s->io_base = le32_to_cpu(*(uint32_t *)(d->config + 0x40));
> +s->io_base &= 0xffc0;
>  
>  memory_region_transaction_begin();
>  memory_region_set_enabled(&s->io, d->config[0x80] & 1);
> -memory_region_set_address(&s->io, pm_io_base);
> +memory_region_set_address(&s->io, s->io_base);
>  memory_region_transaction_commit();
>  }
>  
> @@ -407,6 +409,28 @@ static void piix4_pm_machine_ready(Notifier *n, void 
> *opaque)
>  (memory_region_present(io_as, 0x2f8) ? 0x90 : 0);
>  }
>  
> +static void piix4_pm_add_propeties(PIIX4PMState *s)
> +{
> +static const uint8_t acpi_enable_cmd = ACPI_ENABLE;
> +static const uint8_t acpi_disable_cmd = ACPI_DISABLE;
> +static const uint32_t gpe0_blk = GPE_BASE;
> +static const uint32_t gpe0_blk_len = GPE_LEN;
> +static const uint16_t sci_int = 9;
> +
> +object_property_add_uint8_ptr(OBJECT(s), ACPI_PM_PROP_ACPI_ENABLE_CMD,
> +  &acpi_enable_cmd, NULL);
> +object_property_add_uint8_ptr(OBJECT(s), ACPI_PM_PROP_ACPI_DISABLE_CMD,
> +  &acpi_disable_cmd, NULL);
> +object_property_add_uint32_ptr(OBJECT(s), ACPI_PM_PROP_GPE0_BLK,
> +  &gpe0_blk, NULL);
> +object_property_add_uint32_ptr(OBJECT(s), ACPI_PM_PROP_GPE0_BLK_LEN,
> +  &gpe0_blk_len, NULL);
> +object_property_add_uint16_ptr(OBJECT(s), ACPI_PM_PROP_SCI_INT,
> +  &sci_int, NULL);
> +object_property_add_uint32_ptr(OBJECT(s), ACPI_PM_PROP_PM_IO_BASE,
> +  &s->io_base, NULL);
> +}
> +
>  static int piix4_pm_initfn(PCIDevice *dev)
>  {
>  PIIX4PMState *s = PIIX4_PM(dev);
> @@ -456,9 +480,21 @@ static int piix4_pm_initfn(PCIDevice *dev)
>  
>  piix4_acpi_system_hot_add_init(pci_address_space_io(dev), dev->bus, s);
>  
> +piix4_pm_add_propeties(s);
>  return 0;
>  }
>  
> +Object *piix4_pm_find(void)
> +{
> +bool ambig;
> +Object *o = object_resolve_path_type("", TYPE_PIIX4_PM, &ambig);
> +
> +if (ambig || !o) {
> +return NULL;
> +}
> +return o;
> +}
> +
>  i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
> qemu_irq sci_irq, qemu_irq smi_irq,
> int kvm_enabled, FWCfgState *fw_cfg)
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index c041149..bad3953 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -416,6 +416,14 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
>  return b;
>  }
>  
> +PCIBus *find_i440fx(void)
> +{
> +PCIHostState *s = OBJECT_CHECK(PCIHostState,
> +   object_resolve_path("/machine/i440fx", 
> NULL),

Re: [Qemu-devel] [PATCH] linux-user: Emulate SOCK_CLOEXEC/NONBLOCK if unavailable

[Riku Voipio]

Hi,

On Mon, Sep 16, 2013 at 03:08:06PM +0200, edgar.igles...@gmail.com wrote:
> From: "Edgar E. Iglesias" 
> 
> If the host lacks support for SOCK_CLOEXEC or SOCK_NONBLOCK,
> try to emulate them with fcntl() FD_CLOEXEC and O_NONBLOCK.

Last time emulating CLOEXEC with fcntl was discussed, the idea
was rejected[1]. The whole point of CLOEXEC flag is guarantee
race free open, which when implemented this way it is no longer.

It is better to tell the userspace that atomic operation is not
available and let the userspace app/library to cope with that,
than give the application false sense of safety.

Also, this was 4 years ago, on which platforms is this still
important?

Riku

[1] http://lists.nongnu.org/archive/html/qemu-devel/2009-05/msg00263.html

> Signed-off-by: Edgar E. Iglesias 
> ---
>  linux-user/syscall.c |   48 +---
>  1 file changed, 45 insertions(+), 3 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index c62d875..6aa8cd7 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1701,7 +1701,7 @@ static void unlock_iovec(struct iovec *vec, abi_ulong 
> target_addr,
>  free(vec);
>  }
>  
> -static inline void target_to_host_sock_type(int *type)
> +static inline int target_to_host_sock_type(int *type)
>  {
>  int host_type = 0;
>  int target_type = *type;
> @@ -1718,22 +1718,64 @@ static inline void target_to_host_sock_type(int *type)
>  break;
>  }
>  if (target_type & TARGET_SOCK_CLOEXEC) {
> +#if defined(SOCK_CLOEXEC)
>  host_type |= SOCK_CLOEXEC;
> +#elif !defined(FD_CLOEXEC)
> +return -TARGET_EINVAL;
> +#endif
>  }
>  if (target_type & TARGET_SOCK_NONBLOCK) {
> +#if defined(SOCK_NONBLOCK)
>  host_type |= SOCK_NONBLOCK;
> +#elif !defined(O_NONBLOCK)
> +return -TARGET_EINVAL;
> +#endif
>  }
>  *type = host_type;
> +return 0;
> +}
> +
> +/* Try to emulate socket type flags after socket creation.  */
> +static int sock_flags_fixup(int fd, int target_type)
> +{
> +#if !defined(SOCK_CLOEXEC) && defined(FD_CLOEXEC)
> +if (target_type & TARGET_SOCK_CLOEXEC) {
> +if (fcntl(fd, F_SETFD, FD_CLOEXEC) == -1) {
> +close(fd);
> +return -TARGET_EINVAL;
> +}
> +}
> +#endif
> +#if !defined(SOCK_NONBLOCK) && defined(O_NONBLOCK)
> +if (target_type & TARGET_SOCK_NONBLOCK) {
> +int flags = fcntl(fd, F_GETFL);
> +if (fcntl(fd, F_SETFL, O_NONBLOCK | flags) == -1) {
> +close(fd);
> +return -TARGET_EINVAL;
> +}
> +}
> +#endif
> +return fd;
>  }
>  
>  /* do_socket() Must return target values and target errnos. */
>  static abi_long do_socket(int domain, int type, int protocol)
>  {
> -target_to_host_sock_type(&type);
> +int target_type = type;
> +int ret;
> +
> +ret = target_to_host_sock_type(&type);
> +if (ret) {
> +return ret;
> +}
>  
>  if (domain == PF_NETLINK)
>  return -EAFNOSUPPORT; /* do not NETLINK socket connections possible 
> */
> -return get_errno(socket(domain, type, protocol));
> +ret = get_errno(socket(domain, type, protocol));
> +if (ret >= 0) {
> +ret = sock_flags_fixup(ret, target_type);
> +}
> +return ret;
>  }
>  
>  /* do_bind() Must return target values and target errnos. */
> -- 
> 1.7.10.4

Re: [Qemu-devel] [PATCH v2] vmdk: fix cluster size check for flat extents

[Fam Zheng]

On Mon, 09/23 17:18, Fam Zheng wrote:
> We use the extent size as cluster size for flat extents (where no L1/L2
> table is allocated so it's safe) reuse sector calculating code with
> sparse extents.
> 
> Don't pass in the cluster size for adding flat extent, just set it to
> sectors later, then the cluster size checking will not fail.
> 
> The cluster_sectors is changed to int64_t to allow big flat extent.
> 
> Without this, flat extent opening is broken:
> 
> # qemu-img create -f vmdk -o subformat=monolithicFlat /tmp/a.vmdk 100G
> Formatting '/tmp/a.vmdk', fmt=vmdk size=107374182400 compat6=off 
> subformat='monolithicFlat' zeroed_grain=off
> # qemu-img info /tmp/a.vmdk
> image: /tmp/a.vmdk
> file format: raw
> virtual size: 0 (0 bytes)
> disk size: 4.0K
> 
> Signed-off-by: Fam Zheng 
> ---

When adding a test case, iotests case 059 seems broken now, it will perhaps
take some time to check and I'll post a fix together with test case for this
later.

Fam

[Qemu-devel] [PATCH 2/7] roms: enable parallel builds for 'make lgplvgabios'

[Gerd Hoffmann]

Recurse into vgabios once, adjust dependencies, call make using
$(MAKE) $(MAKEFLAGS) so jobserver mode works.

Signed-off-by: Gerd Hoffmann 
---
 roms/Makefile | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/roms/Makefile b/roms/Makefile
index b646060..6d4330f 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -1,5 +1,6 @@
 
 vgabios_variants := stdvga cirrus vmware qxl
+vgabios_targets  := $(patsubst %,vgabios-%.bin,$(vgabios_variants))
 pxerom_variants  := e1000 eepro100 ne2k_pci pcnet rtl8139 virtio
 
 pxe-rom-e1000efi-rom-e1000: VID := 8086
@@ -49,12 +50,16 @@ seavgabios-%: config.vga.%
make -C seabios out/vgabios.bin
cp seabios/out/vgabios.bin ../pc-bios/vgabios-$*.bin
 
+
 lgplvgabios: $(patsubst %,lgplvgabios-%,$(vgabios_variants))
 
-lgplvgabios-%:
-   make -C vgabios vgabios-$*.bin
+lgplvgabios-%: build-lgplvgabios
cp vgabios/VGABIOS-lgpl-latest.$*.bin ../pc-bios/vgabios-$*.bin
 
+build-lgplvgabios:
+   $(MAKE) $(MAKEFLAGS) -C vgabios $(vgabios_targets)
+
+
 pxerom: $(patsubst %,pxe-rom-%,$(pxerom_variants))
 
 pxe-rom-%: ipxe/src/config/local/general.h
-- 
1.8.3.1

[Qemu-devel] [PATCH 6/7] roms: add rules to build slof

[Gerd Hoffmann]

Add some logic to detect cross compilers.  Add support for "make slof",
which should JustWork[tm] if you are on a ppx64 machine or have a ppc64
cross compiler installed somewhere in your path.

Signed-off-by: Gerd Hoffmann 
---
 roms/Makefile | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/roms/Makefile b/roms/Makefile
index 9672625..5fcc77d 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -18,6 +18,21 @@ pxe-rom-virtio   efi-rom-virtio   : VID := 1af4
 pxe-rom-virtio   efi-rom-virtio   : DID := 1000
 
 #
+# cross compiler auto detection
+#
+path := $(subst :, ,$(PATH))
+system := $(shell uname -s | tr "A-Z" "a-z")
+
+# first find cross binutils in path
+find-cross-ld = $(firstword $(wildcard $(patsubst 
%,%/$(1)-*$(system)*-ld,$(path
+# then check we have cross gcc too
+find-cross-gcc = $(firstword $(wildcard $(patsubst %ld,%gcc,$(call 
find-cross-ld,$(1)
+# finally strip off path + toolname so we get the prefix
+find-cross-prefix = $(subst gcc,,$(notdir $(call find-cross-gcc,$(1
+
+powerpc64_cross_prefix := $(call find-cross-prefix,powerpc64)
+
+#
 # EfiRom utility is shipped with edk2 / tianocore, in BaseTools/
 #
 # We need that to combine multiple images (legacy bios,
@@ -37,6 +52,7 @@ default:
@echo "  pxerom -- update nic roms (bios only)"
@echo "  efirom -- update nic roms (bios+efi, this needs"
@echo "the EfiRom utility from edk2 / tianocore)"
+   @echo "  slof   -- update slof.bin"
 
 bios: config.seabios
sh configure-seabios.sh $<
@@ -90,8 +106,14 @@ ipxe/src/config/local/%: config.ipxe.%
cp $< $@
 
 
+slof:
+   $(MAKE) $(MAKEFLAGS) -C SLOF CROSS=$(powerpc64_cross_prefix) qemu
+   cp SLOF/boot_rom.bin ../pc-bios/slof.bin
+
+
 clean:
rm -rf seabios/.config seabios/out
$(MAKE) $(MAKEFLAGS) -C vgabios clean
rm -f vgabios/VGABIOS-lgpl-latest*
$(MAKE) $(MAKEFLAGS) -C ipxe/src veryclean
+   $(MAKE) $(MAKEFLAGS) -C SLOF clean
-- 
1.8.3.1

[Qemu-devel] [PATCH 1/7] roms: add 'make clean'

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 roms/Makefile | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/roms/Makefile b/roms/Makefile
index 7a228ae..b646060 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -75,3 +75,10 @@ efi-rom-%: ipxe/src/config/local/general.h
 
 ipxe/src/config/local/%: config.ipxe.%
cp $< $@
+
+
+clean:
+   rm -rf seabios/.config seabios/out
+   $(MAKE) $(MAKEFLAGS) -C vgabios clean
+   rm -f vgabios/VGABIOS-lgpl-latest*
+   $(MAKE) $(MAKEFLAGS) -C ipxe/src veryclean
-- 
1.8.3.1

[Qemu-devel] [PATCH 3/7] roms: build lgplvgabios isavga variant

[Gerd Hoffmann]

Add logic to also build+install the isavga vgabios variant.

Signed-off-by: Gerd Hoffmann 
---
 roms/Makefile | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/roms/Makefile b/roms/Makefile
index 6d4330f..11d7837 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -1,6 +1,6 @@
 
-vgabios_variants := stdvga cirrus vmware qxl
-vgabios_targets  := $(patsubst %,vgabios-%.bin,$(vgabios_variants))
+vgabios_variants := stdvga cirrus vmware qxl isavga
+vgabios_targets  := $(subst -isavga,,$(patsubst 
%,vgabios-%.bin,$(vgabios_variants)))
 pxerom_variants  := e1000 eepro100 ne2k_pci pcnet rtl8139 virtio
 
 pxe-rom-e1000efi-rom-e1000: VID := 8086
@@ -53,6 +53,8 @@ seavgabios-%: config.vga.%
 
 lgplvgabios: $(patsubst %,lgplvgabios-%,$(vgabios_variants))
 
+lgplvgabios-isavga: build-lgplvgabios
+   cp vgabios/VGABIOS-lgpl-latest.bin ../pc-bios/vgabios.bin
 lgplvgabios-%: build-lgplvgabios
cp vgabios/VGABIOS-lgpl-latest.$*.bin ../pc-bios/vgabios-$*.bin
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 0/7] roms: build improvements

[Gerd Hoffmann]

  Hi,

Here comes a patch series with a bunch of improvements for rom builds.
It enables parallel builds for some of the roms, and it starts adding
support for cross builds.

cheers,
  Gerd

Gerd Hoffmann (7):
  roms: add 'make clean'
  roms: enable parallel builds for 'make lgplvgabios'
  roms: build lgplvgabios isavga variant
  roms: parallel ipxe builds
  roms: rewrite scripts/refresh-pxe-roms.sh
  roms: add rules to build slof
  roms: enable ipxe cross builds

 roms/Makefile   | 63 ++-
 scripts/refresh-pxe-roms.sh | 80 -
 2 files changed, 60 insertions(+), 83 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 7/7] roms: enable ipxe cross builds

[Gerd Hoffmann]

---
 roms/Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/roms/Makefile b/roms/Makefile
index 5fcc77d..1966f04 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -31,6 +31,7 @@ find-cross-gcc = $(firstword $(wildcard $(patsubst 
%ld,%gcc,$(call find-cross-ld
 find-cross-prefix = $(subst gcc,,$(notdir $(call find-cross-gcc,$(1
 
 powerpc64_cross_prefix := $(call find-cross-prefix,powerpc64)
+x86_64_cross_prefix := $(call find-cross-prefix,x86_64)
 
 #
 # EfiRom utility is shipped with edk2 / tianocore, in BaseTools/
@@ -95,10 +96,12 @@ efi-rom-%: build-pxe-roms build-efi-roms
 
 build-pxe-roms: ipxe/src/config/local/general.h
$(MAKE) $(MAKEFLAGS) -C ipxe/src GITVERSION="" \
+   CROSS_COMPILE=$(x86_64_cross_prefix) \
$(patsubst %,bin/%.rom,$(pxerom_targets))
 
 build-efi-roms: build-pxe-roms ipxe/src/config/local/general.h
$(MAKE) $(MAKEFLAGS) -C ipxe/src GITVERSION="" \
+   CROSS_COMPILE=$(x86_64_cross_prefix) \
$(patsubst %,bin-i386-efi/%.efidrv,$(pxerom_targets)) \
$(patsubst %,bin-x86_64-efi/%.efidrv,$(pxerom_targets))
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 4/7] roms: parallel ipxe builds

[Gerd Hoffmann]

Enable parallel ipxe builds.  Reduce the recursive make calls.  Call
recursive make properly using $(MAKE) $(MAKEFLAGS).

Signed-off-by: Gerd Hoffmann 
---
 roms/Makefile | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/roms/Makefile b/roms/Makefile
index 11d7837..9672625 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -2,6 +2,7 @@
 vgabios_variants := stdvga cirrus vmware qxl isavga
 vgabios_targets  := $(subst -isavga,,$(patsubst 
%,vgabios-%.bin,$(vgabios_variants)))
 pxerom_variants  := e1000 eepro100 ne2k_pci pcnet rtl8139 virtio
+pxerom_targets   := 8086100e 80861209 10500940 10222000 10ec8139 1af41000
 
 pxe-rom-e1000efi-rom-e1000: VID := 8086
 pxe-rom-e1000efi-rom-e1000: DID := 100e
@@ -64,22 +65,27 @@ build-lgplvgabios:
 
 pxerom: $(patsubst %,pxe-rom-%,$(pxerom_variants))
 
-pxe-rom-%: ipxe/src/config/local/general.h
-   make -C ipxe/src bin/$(VID)$(DID).rom
+pxe-rom-%: build-pxe-roms
cp ipxe/src/bin/$(VID)$(DID).rom ../pc-bios/pxe-$*.rom
 
 efirom: $(patsubst %,efi-rom-%,$(pxerom_variants))
 
-efi-rom-%: ipxe/src/config/local/general.h
-   make -C ipxe/src bin/$(VID)$(DID).rom
-   make -C ipxe/src bin-i386-efi/$(VID)$(DID).efidrv
-   make -C ipxe/src bin-x86_64-efi/$(VID)$(DID).efidrv
+efi-rom-%: build-pxe-roms build-efi-roms
$(EFIROM) -f "0x$(VID)" -i "0x$(DID)" -l 0x02 \
-b ipxe/src/bin/$(VID)$(DID).rom \
-ec ipxe/src/bin-i386-efi/$(VID)$(DID).efidrv \
-ec ipxe/src/bin-x86_64-efi/$(VID)$(DID).efidrv \
-o ../pc-bios/efi-$*.rom
 
+build-pxe-roms: ipxe/src/config/local/general.h
+   $(MAKE) $(MAKEFLAGS) -C ipxe/src GITVERSION="" \
+   $(patsubst %,bin/%.rom,$(pxerom_targets))
+
+build-efi-roms: build-pxe-roms ipxe/src/config/local/general.h
+   $(MAKE) $(MAKEFLAGS) -C ipxe/src GITVERSION="" \
+   $(patsubst %,bin-i386-efi/%.efidrv,$(pxerom_targets)) \
+   $(patsubst %,bin-x86_64-efi/%.efidrv,$(pxerom_targets))
+
 ipxe/src/config/local/%: config.ipxe.%
cp $< $@
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 5/7] roms: rewrite scripts/refresh-pxe-roms.sh

[Gerd Hoffmann]

Just use the Makefile in roms/

Signed-off-by: Gerd Hoffmann 
---
 scripts/refresh-pxe-roms.sh | 80 -
 1 file changed, 6 insertions(+), 74 deletions(-)

diff --git a/scripts/refresh-pxe-roms.sh b/scripts/refresh-pxe-roms.sh
index 14d5860..90fc0b3 100755
--- a/scripts/refresh-pxe-roms.sh
+++ b/scripts/refresh-pxe-roms.sh
@@ -21,79 +21,11 @@
 # Usage: Run from root of qemu tree
 # ./scripts/refresh-pxe-roms.sh
 
-QEMU_DIR=$PWD
-ROM_DIR="pc-bios"
-BUILD_DIR="roms/ipxe"
-LOCAL_CONFIG="src/config/local/general.h"
-
-function cleanup ()
-{
-if [ -n "$SAVED_CONFIG" ]; then
-cp "$SAVED_CONFIG" "$BUILD_DIR"/"$LOCAL_CONFIG"
-rm "$SAVED_CONFIG"
-fi
-cd "$QEMU_DIR"
-}
-
-function make_rom ()
-{
-cd "$BUILD_DIR"/src
-
-BUILD_LOG=$(mktemp)
-
-echo Building "$2"...
-make bin/"$1".rom > "$BUILD_LOG" 2>&1
-if [ $? -ne 0 ]; then
-echo Build failed
-tail --lines=100 "$BUILD_LOG"
-rm "$BUILD_LOG"
-cleanup
-exit 1
-fi
-rm "$BUILD_LOG"
-
-cp bin/"$1".rom "$QEMU_DIR"/"$ROM_DIR"/"$2"
-
-cd "$QEMU_DIR"
-}
-
-if [ ! -d "$QEMU_DIR"/"$ROM_DIR" ]; then
-echo "error: can't find $ROM_DIR directory," \
- "run me from the root of the qemu tree"
-exit 1
-fi
-
-if [ ! -d "$BUILD_DIR"/src ]; then
-echo "error: $BUILD_DIR not populated, try:"
-echo "  git submodule init $BUILD_DIR"
-echo "  git submodule update $BUILD_DIR"
-exit 1
-fi
-
-if [ -e "$BUILD_DIR"/"$LOCAL_CONFIG" ]; then
-SAVED_CONFIG=$(mktemp)
-cp "$BUILD_DIR"/"$LOCAL_CONFIG" "$SAVED_CONFIG"
-fi
-
-echo "#undef BANNER_TIMEOUT" > "$BUILD_DIR"/"$LOCAL_CONFIG"
-echo "#define BANNER_TIMEOUT 0" >> "$BUILD_DIR"/"$LOCAL_CONFIG"
-
-IPXE_VERSION=$(cd "$BUILD_DIR" && git describe --tags)
-if [ -z "$IPXE_VERSION" ]; then
-echo "error: unable to retrieve git version"
-cleanup
-exit 1
+targets="pxerom"
+if test -x "$(which EfiRom 2>/dev/null)"; then
+targets="$targets efirom"
 fi
 
-echo "#undef PRODUCT_NAME" >> "$BUILD_DIR"/"$LOCAL_CONFIG"
-echo "#define PRODUCT_NAME \"iPXE $IPXE_VERSION\"" >> 
"$BUILD_DIR"/"$LOCAL_CONFIG"
-
-make_rom 8086100e pxe-e1000.rom
-make_rom 80861209 pxe-eepro100.rom
-make_rom 10500940 pxe-ne2k_pci.rom
-make_rom 10222000 pxe-pcnet.rom
-make_rom 10ec8139 pxe-rtl8139.rom
-make_rom 1af41000 pxe-virtio.rom
-
-echo done
-cleanup
+cd roms
+make -j4 $targets || exit 1
+make clean
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v4 19/23] piix: APIs for pc guest info

[Michael S. Tsirkin]

On Mon, Sep 23, 2013 at 11:27:09AM +0200, Igor Mammedov wrote:
> On Sun, 22 Sep 2013 16:37:59 +0300
> "Michael S. Tsirkin"  wrote:
> 
> > This adds APIs that will be used to fill in guest acpi tables.
> > Some required information is still lacking in QOM, so we
> > fall back on lookups by type and returning explicit types.
> > 
> > Signed-off-by: Michael S. Tsirkin 
> > ---
> >  include/hw/acpi/piix4.h |  8 
> >  include/hw/i386/pc.h|  1 +
> >  hw/acpi/piix4.c | 44 
> >  hw/pci-host/piix.c  |  8 
> >  4 files changed, 57 insertions(+), 4 deletions(-)
> >  create mode 100644 include/hw/acpi/piix4.h
> > 
> > diff --git a/include/hw/acpi/piix4.h b/include/hw/acpi/piix4.h
> > new file mode 100644
> > index 000..65e6fd7
> > --- /dev/null
> > +++ b/include/hw/acpi/piix4.h
> > @@ -0,0 +1,8 @@
> > +#ifndef HW_ACPI_PIIX4_H
> > +#define HW_ACPI_PIIX4_H
> > +
> > +#include "qemu/typedefs.h"
> > +
> > +Object *piix4_pm_find(void);
> > +
> > +#endif
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index f966cef..2a5d996 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -193,6 +193,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int 
> > *piix_devfn,
> >  MemoryRegion *pci_memory,
> >  MemoryRegion *ram_memory);
> >  
> > +PCIBus *find_i440fx(void);
> >  /* piix4.c */
> >  extern PCIDevice *piix4_dev;
> >  int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn);
> > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> > index 4b8c1da..3bcd890 100644
> > --- a/hw/acpi/piix4.c
> > +++ b/hw/acpi/piix4.c
> > @@ -29,6 +29,7 @@
> >  #include "exec/ioport.h"
> >  #include "hw/nvram/fw_cfg.h"
> >  #include "exec/address-spaces.h"
> > +#include "hw/acpi/piix4.h"
> >  
> >  //#define DEBUG
> >  
> > @@ -69,6 +70,8 @@ typedef struct PIIX4PMState {
> >  /*< public >*/
> >  
> >  MemoryRegion io;
> > +uint32_t io_base;
> > +
> >  MemoryRegion io_gpe;
> >  MemoryRegion io_pci;
> >  MemoryRegion io_cpu;
> > @@ -152,14 +155,13 @@ static void apm_ctrl_changed(uint32_t val, void *arg)
> >  static void pm_io_space_update(PIIX4PMState *s)
> >  {
> >  PCIDevice *d = PCI_DEVICE(s);
> > -uint32_t pm_io_base;
> >  
> > -pm_io_base = le32_to_cpu(*(uint32_t *)(d->config + 0x40));
> > -pm_io_base &= 0xffc0;
> > +s->io_base = le32_to_cpu(*(uint32_t *)(d->config + 0x40));
> > +s->io_base &= 0xffc0;
> >  
> >  memory_region_transaction_begin();
> >  memory_region_set_enabled(&s->io, d->config[0x80] & 1);
> > -memory_region_set_address(&s->io, pm_io_base);
> > +memory_region_set_address(&s->io, s->io_base);
> >  memory_region_transaction_commit();
> >  }
> >  
> > @@ -407,6 +409,28 @@ static void piix4_pm_machine_ready(Notifier *n, void 
> > *opaque)
> >  (memory_region_present(io_as, 0x2f8) ? 0x90 : 0);
> >  }
> >  
> > +static void piix4_pm_add_propeties(PIIX4PMState *s)
> > +{
> > +static const uint8_t acpi_enable_cmd = ACPI_ENABLE;
> > +static const uint8_t acpi_disable_cmd = ACPI_DISABLE;
> > +static const uint32_t gpe0_blk = GPE_BASE;
> > +static const uint32_t gpe0_blk_len = GPE_LEN;
> > +static const uint16_t sci_int = 9;
> > +
> > +object_property_add_uint8_ptr(OBJECT(s), ACPI_PM_PROP_ACPI_ENABLE_CMD,
> > +  &acpi_enable_cmd, NULL);
> > +object_property_add_uint8_ptr(OBJECT(s), ACPI_PM_PROP_ACPI_DISABLE_CMD,
> > +  &acpi_disable_cmd, NULL);
> > +object_property_add_uint32_ptr(OBJECT(s), ACPI_PM_PROP_GPE0_BLK,
> > +  &gpe0_blk, NULL);
> > +object_property_add_uint32_ptr(OBJECT(s), ACPI_PM_PROP_GPE0_BLK_LEN,
> > +  &gpe0_blk_len, NULL);
> > +object_property_add_uint16_ptr(OBJECT(s), ACPI_PM_PROP_SCI_INT,
> > +  &sci_int, NULL);
> > +object_property_add_uint32_ptr(OBJECT(s), ACPI_PM_PROP_PM_IO_BASE,
> > +  &s->io_base, NULL);
> > +}
> > +
> >  static int piix4_pm_initfn(PCIDevice *dev)
> >  {
> >  PIIX4PMState *s = PIIX4_PM(dev);
> > @@ -456,9 +480,21 @@ static int piix4_pm_initfn(PCIDevice *dev)
> >  
> >  piix4_acpi_system_hot_add_init(pci_address_space_io(dev), dev->bus, s);
> >  
> > +piix4_pm_add_propeties(s);
> >  return 0;
> >  }
> >  
> > +Object *piix4_pm_find(void)
> > +{
> > +bool ambig;
> > +Object *o = object_resolve_path_type("", TYPE_PIIX4_PM, &ambig);
> > +
> > +if (ambig || !o) {
> > +return NULL;
> > +}
> > +return o;
> > +}
> > +
> >  i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
> > qemu_irq sci_irq, qemu_irq smi_irq,
> > int kvm_enabled, FWCfgState *fw_cfg)
> > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> > index c041149..bad3953 

Re: [Qemu-devel] [PATCH V2 2/4] qemu-nbd: support internal snapshot export

[Paolo Bonzini]

Il 22/09/2013 11:39, Wenchao Xia ha scritto:
> Now it is possible to directly export an internal snapshot, which
> can be used to probe the snapshot's contents without qemu-img
> convert.
> 
> Signed-off-by: Wenchao Xia 
> ---
>  qemu-nbd.c |   54 +-
>  1 files changed, 53 insertions(+), 1 deletions(-)
> 
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index c26c98e..e450d04 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -20,6 +20,7 @@
>  #include "block/block.h"
>  #include "block/nbd.h"
>  #include "qemu/main-loop.h"
> +#include "block/snapshot.h"
>  
>  #include 
>  #include 
> @@ -304,6 +305,23 @@ static void nbd_accept(void *opaque)
>  }
>  }
>  
> +#define SNAPSHOT_OPT_ID "id"
> +#define SNAPSHOT_OPT_NAME   "name"
> +
> +static QEMUOptionParameter snapshot_options[] = {
> +{
> +.name = SNAPSHOT_OPT_ID,
> +.type = OPT_STRING,
> +.help = "snapshot id"
> +},
> +{
> +.name = SNAPSHOT_OPT_NAME,
> +.type = OPT_STRING,
> +.help = "snapshot name"
> +},
> +{ NULL }
> +};

I think whatever mechanism you use here to pick a snapshot id or name
should be implemented in qemu-img too.

Also, I think QEMUOptionParameter is being phased out.

>  int main(int argc, char **argv)
>  {
>  BlockDriverState *bs;
> @@ -315,7 +333,10 @@ int main(int argc, char **argv)
>  char *device = NULL;
>  int port = NBD_DEFAULT_PORT;
>  off_t fd_size;
> -const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:t";
> +QEMUOptionParameter *sn_param = NULL;
> +const QEMUOptionParameter *sn_param_id, *sn_param_name;
> +const char *sn_id = NULL, *sn_name = NULL;
> +const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:";
>  struct option lopt[] = {
>  { "help", 0, NULL, 'h' },
>  { "version", 0, NULL, 'V' },
> @@ -328,6 +349,7 @@ int main(int argc, char **argv)
>  { "connect", 1, NULL, 'c' },
>  { "disconnect", 0, NULL, 'd' },
>  { "snapshot", 0, NULL, 's' },
> +{ "snapshot-load", 1, NULL, 'l' },

Please call this option "load-snapshot".

Paolo

>  { "nocache", 0, NULL, 'n' },
>  { "cache", 1, NULL, QEMU_NBD_OPT_CACHE },
>  #ifdef CONFIG_LINUX_AIO
> @@ -428,6 +450,14 @@ int main(int argc, char **argv)
>  errx(EXIT_FAILURE, "Offset must be positive `%s'", optarg);
>  }
>  break;
> +case 'l':
> +sn_param = parse_option_parameters(optarg,
> +   snapshot_options, sn_param);
> +if (!sn_param) {
> +errx(EXIT_FAILURE,
> + "Invalid snapshot-load options '%s'", optarg);
> +}
> +/* fall through */
>  case 'r':
>  nbdflags |= NBD_FLAG_READ_ONLY;
>  flags &= ~BDRV_O_RDWR;
> @@ -581,6 +611,24 @@ int main(int argc, char **argv)
>  error_get_pretty(local_err));
>  }
>  
> +if (sn_param) {
> +sn_param_id = get_option_parameter(sn_param, SNAPSHOT_OPT_ID);
> +sn_param_name = get_option_parameter(sn_param, SNAPSHOT_OPT_NAME);
> +if (sn_param_id) {
> +sn_id = sn_param_id->value.s;
> +}
> +if (sn_param_name) {
> +sn_name = sn_param_name->value.s;
> +}
> +ret = bdrv_snapshot_load_tmp(bs, sn_id, sn_name, &local_err);
> +if (ret < 0) {
> +errno = -ret;
> +err(EXIT_FAILURE,
> +"Failed to load snapshot, reason:\n%s",
> +error_get_pretty(local_err));
> +}
> +}
> +
>  fd_size = bdrv_getlength(bs);
>  
>  if (partition != -1) {
> @@ -641,6 +689,10 @@ int main(int argc, char **argv)
>  unlink(sockpath);
>  }
>  
> +if (sn_param) {
> +free_option_parameters(sn_param);
> +}
> +
>  if (device) {
>  void *ret;
>  pthread_join(client_thread, &ret);
> 

Re: [Qemu-devel] [PATCH v5 0/5] Do not set SO_REUSEADDR on Windows

[Sebastian Ottlik]

On 18.09.2013 18:58, Stefan Weil wrote:

Am 16.09.2013 17:10, schrieb Sebastian Ottlik:

On 16.09.2013 16:55, Paolo Bonzini wrote:

Il 16/09/2013 16:23, Sebastian Ottlik ha scritto:

- Added the silent flag to socket_set_fast_reuse controlling error
reporting
One location where SO_REUSEADDR was set would report errors if
setting the
option failed. Keeping the reporting code there would be somewhat
unclean, so
I moved it to socket_set_fast_reuse. A side effect of this was
that the error
reporting was added for all locations that now use
socket_set_fast_reuse. Here
a new flag is added to control error reporting, which means this
patchset
won't change QEMU behaviour (except for not setting SO_REUSEADDR
on Windows).

Is there actually a case where setting SO_REUSEADDR could fail?

Paolo

Yes, but its very unlikely. E.g. the first parameter is not a valid
socket.


If failures only happen when something is very wrong (like an invalid
socket id),
an assertion might be better, and we could remove the 'silent' parameter.

Stefan

IMO for debug builds this is a good idea. However, in production use it 
is probably preferable to keep QEMU running, as a failure won't be too 
critical. From a quick grep it looks like NDEBUG is not set so 
assertions wont be removed for non-debug builds. I don't feel acquainted 
enough with the source code to decide about this kind of change in 
functionality, which is why I was waiting so long to reply.

Re: [Qemu-devel] [PATCH 0/4] qom: add helpers for integer properties

[Paolo Bonzini]

Il 22/09/2013 09:16, Michael S. Tsirkin ha scritto:
> Add helper functions for adding read-only properties, that work in the
> case where the value is in memory.
> 
> Michael S. Tsirkin (4):
>   qemu: add Error to typedefs
>   qom: pull in qemu/typedefs
>   qom: cleanup struct Error references
>   qom: add pointer to int property helpers
> 
>  include/qemu/typedefs.h |  1 +
>  include/qom/object.h| 73 
> +++--
>  qom/object.c| 56 +
>  3 files changed, 104 insertions(+), 26 deletions(-)
> 

Reviewed-by: Paolo Bonzini 

[Qemu-devel] [PATCH] hw/pci: completed master-abort emulation

[Marcel Apfelbaum]

This patch is implemented on top of series:
[PATCH v5 0/3] pci: implement upstream master abort protocol

Added "master abort io" background region for PCIBus.

Added "master abort mem" region to catch transactions initiated
by pci devices targeted to unassigned addresses.

Enabled "master abort" regions for PCI-2-PCI bridge's secondary bus.

Set "Received Master Abort" Bit on Status/Secondary Status
register as defined in the PCI Spec.

Signed-off-by: Marcel Apfelbaum 
---
 hw/pci/pci.c | 115 ++-
 hw/pci/pci_bridge.c  |  10 +
 include/hw/pci/pci.h |   3 ++
 include/hw/pci/pci_bus.h |   1 +
 4 files changed, 118 insertions(+), 11 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index d8a1b11..1f4e707 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -283,17 +283,76 @@ const char *pci_root_bus_path(PCIDevice *dev)
 return rootbus->qbus.name;
 }
 
+static PCIDevice *pci_bus_find_host(PCIBus *bus)
+{
+PCIDevice *dev;
+int i;
+
+for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
+dev = bus->devices[i];
+if (dev) {
+PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
+if (pc->class_id == PCI_CLASS_BRIDGE_HOST) {
+return dev;
+}
+}
+}
+
+return NULL;
+}
+
+static void master_abort(void *opaque)
+{
+PCIDevice *master_dev = NULL;
+PCIDeviceClass *pc;
+PCIBus *bus;
+int downstream;
+
+if (object_dynamic_cast(OBJECT(opaque), TYPE_PCI_DEVICE)) {
+master_dev = PCI_DEVICE(opaque);
+bus = master_dev->bus;
+while (!pci_bus_is_root(bus)) {
+master_dev = bus->parent_dev;
+bus = master_dev->bus;
+}
+downstream = 0;
+}
+
+if (object_dynamic_cast(OBJECT(opaque), TYPE_PCI_BUS)) {
+bus = PCI_BUS(opaque);
+if (pci_bus_is_root(bus)) {
+master_dev = pci_bus_find_host(bus);
+} else { /* bus behind a PCI-2-PCI bridge */
+master_dev = bus->parent_dev;
+}
+downstream = 1;
+}
+
+assert(master_dev);
+pc = PCI_DEVICE_GET_CLASS(master_dev);
+
+if (downstream && pc->is_bridge) {
+pci_word_test_and_set_mask(master_dev->config + PCI_SEC_STATUS,
+   PCI_STATUS_REC_MASTER_ABORT);
+} else {
+pci_word_test_and_set_mask(master_dev->config + PCI_STATUS,
+   PCI_STATUS_REC_MASTER_ABORT);
+}
+}
+
 static uint64_t master_abort_mem_read(void *opaque, hwaddr addr, unsigned size)
 {
-   return -1ULL;
+master_abort(opaque);
+return -1ULL;
 }
 
 static void master_abort_mem_write(void *opaque, hwaddr addr, uint64_t val,
unsigned size)
 {
+master_abort(opaque);
 }
 
-static const MemoryRegionOps master_abort_mem_ops = {
+static const MemoryRegionOps master_abort_ops = {
 .read = master_abort_mem_read,
 .write = master_abort_mem_write,
 .endianness = DEVICE_LITTLE_ENDIAN,
@@ -301,6 +360,23 @@ static const MemoryRegionOps master_abort_mem_ops = {
 
 #define MASTER_ABORT_MEM_PRIORITY INT_MIN
 
+void pci_bus_master_abort_init(PCIBus *bus, const char *mem, const char *io)
+{
+memory_region_init_io(&bus->master_abort_mem, OBJECT(bus),
+  &master_abort_ops, bus, mem,
+  memory_region_size(bus->address_space_mem));
+memory_region_add_subregion_overlap(bus->address_space_mem,
+0, &bus->master_abort_mem,
+MASTER_ABORT_MEM_PRIORITY);
+
+memory_region_init_io(&bus->master_abort_io, OBJECT(bus),
+  &master_abort_ops, bus, io,
+  memory_region_size(bus->address_space_io));
+memory_region_add_subregion_overlap(bus->address_space_io,
+0, &bus->master_abort_io,
+MASTER_ABORT_MEM_PRIORITY);
+}
+
 static void pci_bus_init(PCIBus *bus, DeviceState *parent,
  const char *name,
  MemoryRegion *address_space_mem,
@@ -312,13 +388,8 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
 bus->address_space_mem = address_space_mem;
 bus->address_space_io = address_space_io;
 
-
-memory_region_init_io(&bus->master_abort_mem, OBJECT(bus),
-  &master_abort_mem_ops, bus, "pci-master-abort",
-  memory_region_size(bus->address_space_mem));
-memory_region_add_subregion_overlap(bus->address_space_mem,
-0, &bus->master_abort_mem,
-MASTER_ABORT_MEM_PRIORITY);
+pci_bus_master_abort_init(bus, "pci-master-abort-mem",
+  "pci-master-abort-io");
 
 /* host bridge */
 QLIST_INIT(&bus->child);
@@ -840,9 +911,24 @@ static

Re: [Qemu-devel] [PATCH v4 19/23] piix: APIs for pc guest info

[Igor Mammedov]

On Mon, 23 Sep 2013 13:10:49 +0300
"Michael S. Tsirkin"  wrote:

> On Mon, Sep 23, 2013 at 11:27:09AM +0200, Igor Mammedov wrote:
> > On Sun, 22 Sep 2013 16:37:59 +0300
> > "Michael S. Tsirkin"  wrote:
> > 
> > > This adds APIs that will be used to fill in guest acpi tables.
> > > Some required information is still lacking in QOM, so we
> > > fall back on lookups by type and returning explicit types.
> > > 
> > > Signed-off-by: Michael S. Tsirkin 
> > > ---
> > >  include/hw/acpi/piix4.h |  8 
> > >  include/hw/i386/pc.h|  1 +
> > >  hw/acpi/piix4.c | 44 
> > >  hw/pci-host/piix.c  |  8 
> > >  4 files changed, 57 insertions(+), 4 deletions(-)
> > >  create mode 100644 include/hw/acpi/piix4.h
> > > 
> > > diff --git a/include/hw/acpi/piix4.h b/include/hw/acpi/piix4.h
> > > new file mode 100644
> > > index 000..65e6fd7
> > > --- /dev/null
> > > +++ b/include/hw/acpi/piix4.h
> > > @@ -0,0 +1,8 @@
> > > +#ifndef HW_ACPI_PIIX4_H
> > > +#define HW_ACPI_PIIX4_H
> > > +
> > > +#include "qemu/typedefs.h"
> > > +
> > > +Object *piix4_pm_find(void);
> > > +
> > > +#endif
> > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > > index f966cef..2a5d996 100644
> > > --- a/include/hw/i386/pc.h
> > > +++ b/include/hw/i386/pc.h
> > > @@ -193,6 +193,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, 
> > > int *piix_devfn,
> > >  MemoryRegion *pci_memory,
> > >  MemoryRegion *ram_memory);
> > >  
> > > +PCIBus *find_i440fx(void);
> > >  /* piix4.c */
> > >  extern PCIDevice *piix4_dev;
> > >  int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn);
> > > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> > > index 4b8c1da..3bcd890 100644
> > > --- a/hw/acpi/piix4.c
> > > +++ b/hw/acpi/piix4.c
> > > @@ -29,6 +29,7 @@
> > >  #include "exec/ioport.h"
> > >  #include "hw/nvram/fw_cfg.h"
> > >  #include "exec/address-spaces.h"
> > > +#include "hw/acpi/piix4.h"
> > >  
> > >  //#define DEBUG
> > >  
> > > @@ -69,6 +70,8 @@ typedef struct PIIX4PMState {
> > >  /*< public >*/
> > >  
> > >  MemoryRegion io;
> > > +uint32_t io_base;
> > > +
> > >  MemoryRegion io_gpe;
> > >  MemoryRegion io_pci;
> > >  MemoryRegion io_cpu;
> > > @@ -152,14 +155,13 @@ static void apm_ctrl_changed(uint32_t val, void 
> > > *arg)
> > >  static void pm_io_space_update(PIIX4PMState *s)
> > >  {
> > >  PCIDevice *d = PCI_DEVICE(s);
> > > -uint32_t pm_io_base;
> > >  
> > > -pm_io_base = le32_to_cpu(*(uint32_t *)(d->config + 0x40));
> > > -pm_io_base &= 0xffc0;
> > > +s->io_base = le32_to_cpu(*(uint32_t *)(d->config + 0x40));
> > > +s->io_base &= 0xffc0;
> > >  
> > >  memory_region_transaction_begin();
> > >  memory_region_set_enabled(&s->io, d->config[0x80] & 1);
> > > -memory_region_set_address(&s->io, pm_io_base);
> > > +memory_region_set_address(&s->io, s->io_base);
> > >  memory_region_transaction_commit();
> > >  }
> > >  
> > > @@ -407,6 +409,28 @@ static void piix4_pm_machine_ready(Notifier *n, void 
> > > *opaque)
> > >  (memory_region_present(io_as, 0x2f8) ? 0x90 : 0);
> > >  }
> > >  
> > > +static void piix4_pm_add_propeties(PIIX4PMState *s)
> > > +{
> > > +static const uint8_t acpi_enable_cmd = ACPI_ENABLE;
> > > +static const uint8_t acpi_disable_cmd = ACPI_DISABLE;
> > > +static const uint32_t gpe0_blk = GPE_BASE;
> > > +static const uint32_t gpe0_blk_len = GPE_LEN;
> > > +static const uint16_t sci_int = 9;
> > > +
> > > +object_property_add_uint8_ptr(OBJECT(s), 
> > > ACPI_PM_PROP_ACPI_ENABLE_CMD,
> > > +  &acpi_enable_cmd, NULL);
> > > +object_property_add_uint8_ptr(OBJECT(s), 
> > > ACPI_PM_PROP_ACPI_DISABLE_CMD,
> > > +  &acpi_disable_cmd, NULL);
> > > +object_property_add_uint32_ptr(OBJECT(s), ACPI_PM_PROP_GPE0_BLK,
> > > +  &gpe0_blk, NULL);
> > > +object_property_add_uint32_ptr(OBJECT(s), ACPI_PM_PROP_GPE0_BLK_LEN,
> > > +  &gpe0_blk_len, NULL);
> > > +object_property_add_uint16_ptr(OBJECT(s), ACPI_PM_PROP_SCI_INT,
> > > +  &sci_int, NULL);
> > > +object_property_add_uint32_ptr(OBJECT(s), ACPI_PM_PROP_PM_IO_BASE,
> > > +  &s->io_base, NULL);
> > > +}
> > > +
> > >  static int piix4_pm_initfn(PCIDevice *dev)
> > >  {
> > >  PIIX4PMState *s = PIIX4_PM(dev);
> > > @@ -456,9 +480,21 @@ static int piix4_pm_initfn(PCIDevice *dev)
> > >  
> > >  piix4_acpi_system_hot_add_init(pci_address_space_io(dev), dev->bus, 
> > > s);
> > >  
> > > +piix4_pm_add_propeties(s);
> > >  return 0;
> > >  }
> > >  
> > > +Object *piix4_pm_find(void)
> > > +{
> > > +bool ambig;
> > > +Object *o = object_resolve_path_type("", TYPE_PIIX4_PM, &ambig);
> > > +
> > > +if (ambig || !o) {
> > > +  

Re: [Qemu-devel] [PATCH 1/2] ich9: update sci on gpe write

[Hu Tao]

On Thu, Sep 12, 2013 at 05:22:14PM +0200, Igor Mammedov wrote:
> On Wed, 21 Aug 2013 17:04:27 +0800
> Hu Tao  wrote:
> 
> > OSPM may disable the sci by clearing GPEx_BLK EN bit, in the case
> > we have to set sci level to 0 or guest will receive sci interrupts
> > endlessly.
> 
> Could you make a more verbose comment, referring to relevant ACPI spec chapter
> and it would be nice,

See Section 5.6.4, ACPI 5.0, page 221.

>   if you experienced problem with linux guest, to add
> symptoms here as well.

See http://lists.gnu.org/archive/html/qemu-devel/2012-12/msg02711.html.
I guess without this patch the same thing will happen in guest linux
when hotplugging a vcpu.

> 
> commit 633aa0ac did equivalent change to piix4 part, so it's worth to mention
> it here.
> 
> > Signed-off-by: Hu Tao 
> > ---
> >  hw/acpi/ich9.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> > index 3fb443d..8717c15 100644
> > --- a/hw/acpi/ich9.c
> > +++ b/hw/acpi/ich9.c
> > @@ -79,6 +79,8 @@ static void ich9_gpe_writeb(void *opaque, hwaddr addr, 
> > uint64_t val,
> >  {
> >  ICH9LPCPMRegs *pm = opaque;
> >  acpi_gpe_ioport_writeb(&pm->acpi_regs, addr, val);
> > +
> > +pm_update_sci(pm);
> >  }
> >  
> >  static const MemoryRegionOps ich9_gpe_ops = {

Re: [Qemu-devel] [PATCH] hw/pci: completed master-abort emulation

[Michael S. Tsirkin]

On Mon, Sep 23, 2013 at 02:01:17PM +0300, Marcel Apfelbaum wrote:
> This patch is implemented on top of series:
> [PATCH v5 0/3] pci: implement upstream master abort protocol
> 
> Added "master abort io" background region for PCIBus.
> 
> Added "master abort mem" region to catch transactions initiated
> by pci devices targeted to unassigned addresses.
> 
> Enabled "master abort" regions for PCI-2-PCI bridge's secondary bus.
> 
> Set "Received Master Abort" Bit on Status/Secondary Status
> register as defined in the PCI Spec.
> 
> Signed-off-by: Marcel Apfelbaum 

I think it will be easier to review the complete
series, not an incremental patch.

We probably need to think what's the right thing
to do for master abort mode since we do not
emulate SERR. Do we make it writeable at all?

It's a pci only bit:
When the Master-Abort Mode bit is cleared and a posted write transaction
forwarded by the
bridge terminates with a Master-Abort, no error is reported (note that
the Master-Abort bit
is still set). When the Master-Abort Mode bit is set and a posted write
transaction forwarded
by the bridge terminates with a Master-Abort on the destination bus, the
bridge must:
Assert SERR# on the primary interfaceCommand register)
(if enabled by the SERR# Enable bitin the
Set the Signaled System Errorbit in the Command register)
bitin the Status register (if enabled by the SERR# Enable)

one way to do this would be not to set Master-Abort bit even
though spec says we should, and pretend there was no error.

> ---
>  hw/pci/pci.c | 115 
> ++-
>  hw/pci/pci_bridge.c  |  10 +
>  include/hw/pci/pci.h |   3 ++
>  include/hw/pci/pci_bus.h |   1 +
>  4 files changed, 118 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index d8a1b11..1f4e707 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -283,17 +283,76 @@ const char *pci_root_bus_path(PCIDevice *dev)
>  return rootbus->qbus.name;
>  }
>  
> +static PCIDevice *pci_bus_find_host(PCIBus *bus)
> +{
> +PCIDevice *dev;
> +int i;
> +
> +for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
> +dev = bus->devices[i];
> +if (dev) {
> +PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
> +if (pc->class_id == PCI_CLASS_BRIDGE_HOST) {

In fact you can do this easier:
 pci_get_word(dev->config + PCI_CLASS_DEVICE) == PCI_CLASS_BRIDGE_HOST

> +return dev;
> +}
> +}
> +}
> +
> +return NULL;
> +}
> +
> +static void master_abort(void *opaque)
> +{
> +PCIDevice *master_dev = NULL;
> +PCIDeviceClass *pc;
> +PCIBus *bus;
> +int downstream;

bool?

> +
> +if (object_dynamic_cast(OBJECT(opaque), TYPE_PCI_DEVICE)) {

Please don't do dynamic cast just because you can.
You know very well what kind of object you pass
when you create the MR.
So just call the appropriate function.

> +master_dev = PCI_DEVICE(opaque);
> +bus = master_dev->bus;
> +while (!pci_bus_is_root(bus)) {
> +master_dev = bus->parent_dev;
> +bus = master_dev->bus;
> +}
> +downstream = 0;
> +}
> +
> +if (object_dynamic_cast(OBJECT(opaque), TYPE_PCI_BUS)) {
> +bus = PCI_BUS(opaque);
> +if (pci_bus_is_root(bus)) {
> +master_dev = pci_bus_find_host(bus);
> +} else { /* bus behind a PCI-2-PCI bridge */
> +master_dev = bus->parent_dev;
> +}

So the reason for this is that device to device MA
is still not implemented here, correct?
If it was it would be just one instance of it.
I'm not saying this must block this patch, however
there should be a code comment to this effect,
and commit log should mention this explicitly.

> +downstream = 1;
> +}
> +
> +assert(master_dev);
> +pc = PCI_DEVICE_GET_CLASS(master_dev);
> +

There's very little common code between devices and
buses. So just use 2 functions.

> +if (downstream && pc->is_bridge) {
> +pci_word_test_and_set_mask(master_dev->config + PCI_SEC_STATUS,
> +   PCI_STATUS_REC_MASTER_ABORT);
> +} else {
> +pci_word_test_and_set_mask(master_dev->config + PCI_STATUS,
> +   PCI_STATUS_REC_MASTER_ABORT);
> +}
> +}
> +
>  static uint64_t master_abort_mem_read(void *opaque, hwaddr addr, unsigned 
> size)
>  {
> -   return -1ULL;

I didn't notice but this means there were 3 spaces here in previous
patch?

> +master_abort(opaque);
> +return -1ULL;
>  }
>  
>  static void master_abort_mem_write(void *opaque, hwaddr addr, uint64_t val,
> unsigned size)
>  {
> +master_abort(opaque);
>  }
>  
> -static const MemoryRegionOps master_abort_mem_ops = {
> +static const MemoryRegionOps master_abort_ops = {
>  .read 

Re: [Qemu-devel] [PATCH v4 19/23] piix: APIs for pc guest info

[Michael S. Tsirkin]

On Mon, Sep 23, 2013 at 01:14:11PM +0200, Igor Mammedov wrote:
> On Mon, 23 Sep 2013 13:10:49 +0300
> "Michael S. Tsirkin"  wrote:
> 
> > On Mon, Sep 23, 2013 at 11:27:09AM +0200, Igor Mammedov wrote:
> > > On Sun, 22 Sep 2013 16:37:59 +0300
> > > "Michael S. Tsirkin"  wrote:
> > > 
> > > > This adds APIs that will be used to fill in guest acpi tables.
> > > > Some required information is still lacking in QOM, so we
> > > > fall back on lookups by type and returning explicit types.
> > > > 
> > > > Signed-off-by: Michael S. Tsirkin 
> > > > ---
> > > >  include/hw/acpi/piix4.h |  8 
> > > >  include/hw/i386/pc.h|  1 +
> > > >  hw/acpi/piix4.c | 44 
> > > > 
> > > >  hw/pci-host/piix.c  |  8 
> > > >  4 files changed, 57 insertions(+), 4 deletions(-)
> > > >  create mode 100644 include/hw/acpi/piix4.h
> > > > 
> > > > diff --git a/include/hw/acpi/piix4.h b/include/hw/acpi/piix4.h
> > > > new file mode 100644
> > > > index 000..65e6fd7
> > > > --- /dev/null
> > > > +++ b/include/hw/acpi/piix4.h
> > > > @@ -0,0 +1,8 @@
> > > > +#ifndef HW_ACPI_PIIX4_H
> > > > +#define HW_ACPI_PIIX4_H
> > > > +
> > > > +#include "qemu/typedefs.h"
> > > > +
> > > > +Object *piix4_pm_find(void);
> > > > +
> > > > +#endif
> > > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > > > index f966cef..2a5d996 100644
> > > > --- a/include/hw/i386/pc.h
> > > > +++ b/include/hw/i386/pc.h
> > > > @@ -193,6 +193,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, 
> > > > int *piix_devfn,
> > > >  MemoryRegion *pci_memory,
> > > >  MemoryRegion *ram_memory);
> > > >  
> > > > +PCIBus *find_i440fx(void);
> > > >  /* piix4.c */
> > > >  extern PCIDevice *piix4_dev;
> > > >  int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn);
> > > > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> > > > index 4b8c1da..3bcd890 100644
> > > > --- a/hw/acpi/piix4.c
> > > > +++ b/hw/acpi/piix4.c
> > > > @@ -29,6 +29,7 @@
> > > >  #include "exec/ioport.h"
> > > >  #include "hw/nvram/fw_cfg.h"
> > > >  #include "exec/address-spaces.h"
> > > > +#include "hw/acpi/piix4.h"
> > > >  
> > > >  //#define DEBUG
> > > >  
> > > > @@ -69,6 +70,8 @@ typedef struct PIIX4PMState {
> > > >  /*< public >*/
> > > >  
> > > >  MemoryRegion io;
> > > > +uint32_t io_base;
> > > > +
> > > >  MemoryRegion io_gpe;
> > > >  MemoryRegion io_pci;
> > > >  MemoryRegion io_cpu;
> > > > @@ -152,14 +155,13 @@ static void apm_ctrl_changed(uint32_t val, void 
> > > > *arg)
> > > >  static void pm_io_space_update(PIIX4PMState *s)
> > > >  {
> > > >  PCIDevice *d = PCI_DEVICE(s);
> > > > -uint32_t pm_io_base;
> > > >  
> > > > -pm_io_base = le32_to_cpu(*(uint32_t *)(d->config + 0x40));
> > > > -pm_io_base &= 0xffc0;
> > > > +s->io_base = le32_to_cpu(*(uint32_t *)(d->config + 0x40));
> > > > +s->io_base &= 0xffc0;
> > > >  
> > > >  memory_region_transaction_begin();
> > > >  memory_region_set_enabled(&s->io, d->config[0x80] & 1);
> > > > -memory_region_set_address(&s->io, pm_io_base);
> > > > +memory_region_set_address(&s->io, s->io_base);
> > > >  memory_region_transaction_commit();
> > > >  }
> > > >  
> > > > @@ -407,6 +409,28 @@ static void piix4_pm_machine_ready(Notifier *n, 
> > > > void *opaque)
> > > >  (memory_region_present(io_as, 0x2f8) ? 0x90 : 0);
> > > >  }
> > > >  
> > > > +static void piix4_pm_add_propeties(PIIX4PMState *s)
> > > > +{
> > > > +static const uint8_t acpi_enable_cmd = ACPI_ENABLE;
> > > > +static const uint8_t acpi_disable_cmd = ACPI_DISABLE;
> > > > +static const uint32_t gpe0_blk = GPE_BASE;
> > > > +static const uint32_t gpe0_blk_len = GPE_LEN;
> > > > +static const uint16_t sci_int = 9;
> > > > +
> > > > +object_property_add_uint8_ptr(OBJECT(s), 
> > > > ACPI_PM_PROP_ACPI_ENABLE_CMD,
> > > > +  &acpi_enable_cmd, NULL);
> > > > +object_property_add_uint8_ptr(OBJECT(s), 
> > > > ACPI_PM_PROP_ACPI_DISABLE_CMD,
> > > > +  &acpi_disable_cmd, NULL);
> > > > +object_property_add_uint32_ptr(OBJECT(s), ACPI_PM_PROP_GPE0_BLK,
> > > > +  &gpe0_blk, NULL);
> > > > +object_property_add_uint32_ptr(OBJECT(s), 
> > > > ACPI_PM_PROP_GPE0_BLK_LEN,
> > > > +  &gpe0_blk_len, NULL);
> > > > +object_property_add_uint16_ptr(OBJECT(s), ACPI_PM_PROP_SCI_INT,
> > > > +  &sci_int, NULL);
> > > > +object_property_add_uint32_ptr(OBJECT(s), ACPI_PM_PROP_PM_IO_BASE,
> > > > +  &s->io_base, NULL);
> > > > +}
> > > > +
> > > >  static int piix4_pm_initfn(PCIDevice *dev)
> > > >  {
> > > >  PIIX4PMState *s = PIIX4_PM(dev);
> > > > @@ -456,9 +480,21 @@ static int piix4_pm_initfn(PCIDevice *dev)
> > > >  
> > > >  piix4_acpi_system_hot_add_in

Re: [Qemu-devel] [PATCH] linux-user: Emulate SOCK_CLOEXEC/NONBLOCK if unavailable

[Edgar E. Iglesias]

On Mon, Sep 23, 2013 at 12:37:10PM +0300, Riku Voipio wrote:
> Hi,

Hi Riku,

> 
> On Mon, Sep 16, 2013 at 03:08:06PM +0200, edgar.igles...@gmail.com wrote:
> > From: "Edgar E. Iglesias" 
> > 
> > If the host lacks support for SOCK_CLOEXEC or SOCK_NONBLOCK,
> > try to emulate them with fcntl() FD_CLOEXEC and O_NONBLOCK.
> 
> Last time emulating CLOEXEC with fcntl was discussed, the idea
> was rejected[1]. The whole point of CLOEXEC flag is guarantee
> race free open, which when implemented this way it is no longer.
> 
> It is better to tell the userspace that atomic operation is not
> available and let the userspace app/library to cope with that,
> than give the application false sense of safety.

Agreed, I'll send a v2 that EINVALs on lack of SOCK_CLOEXEC.
My primary concern is to avoid the build error of qemu.

Thanks,
Edgar

[Qemu-devel] [PATCH V2] introduce .bdrv_needs_filename

[Benoît Canet]

since v1:
simplify assertion [Kevin]
add .bdrv_needs_filename only to driver to having .bdrv_parse_filename nor
   .bdrv_open [Kevin]
Tested that raw, qed and ssh protocols works fine.

Benoît Canet (1):
  block: introduce BlockDriver.bdrv_needs_filename to enable some
drivers.

 block.c   | 5 +++--
 block/gluster.c   | 4 
 block/iscsi.c | 1 +
 block/raw-posix.c | 5 +
 block/raw-win32.c | 2 ++
 block/rbd.c   | 1 +
 block/sheepdog.c  | 3 +++
 include/block/block_int.h | 1 +
 8 files changed, 20 insertions(+), 2 deletions(-)

-- 
1.8.1.2

[Qemu-devel] [PATCH V2] block: introduce BlockDriver.bdrv_needs_filename to enable some drivers.

[Benoît Canet]

Some drivers will have driver specifics options but no filename.
This new bool allow the block layer to treat them correctly.

The .bdrv_needs_filename is set in drivers not having .bdrv_parse_filename and
not having .bdrv_open.

The first exception to this rule will be the quorum driver.

Signed-off-by: Benoit Canet 
---
 block.c   | 5 +++--
 block/gluster.c   | 4 
 block/iscsi.c | 1 +
 block/raw-posix.c | 5 +
 block/raw-win32.c | 2 ++
 block/rbd.c   | 1 +
 block/sheepdog.c  | 3 +++
 include/block/block_int.h | 1 +
 8 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index 827549c..a4db13d 100644
--- a/block.c
+++ b/block.c
@@ -760,7 +760,7 @@ static int bdrv_open_common(BlockDriverState *bs, 
BlockDriverState *file,
 /* Open the image, either directly or using a protocol */
 if (drv->bdrv_file_open) {
 assert(file == NULL);
-assert(drv->bdrv_parse_filename || filename != NULL);
+assert(!drv->bdrv_needs_filename || filename != NULL);
 ret = drv->bdrv_file_open(bs, options, open_flags);
 } else {
 if (file == NULL) {
@@ -870,7 +870,8 @@ int bdrv_file_open(BlockDriverState **pbs, const char 
*filename,
 goto fail;
 }
 qdict_del(options, "filename");
-} else if (!drv->bdrv_parse_filename && !filename) {
+} else if (!drv->bdrv_parse_filename && drv->bdrv_needs_filename &&
+   !filename) {
 qerror_report(ERROR_CLASS_GENERIC_ERROR,
   "The '%s' block driver requires a file name",
   drv->format_name);
diff --git a/block/gluster.c b/block/gluster.c
index dbb03f4..c124400 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -611,6 +611,7 @@ static BlockDriver bdrv_gluster = {
 .format_name  = "gluster",
 .protocol_name= "gluster",
 .instance_size= sizeof(BDRVGlusterState),
+.bdrv_needs_filename  = true,
 .bdrv_file_open   = qemu_gluster_open,
 .bdrv_close   = qemu_gluster_close,
 .bdrv_create  = qemu_gluster_create,
@@ -631,6 +632,7 @@ static BlockDriver bdrv_gluster_tcp = {
 .format_name  = "gluster",
 .protocol_name= "gluster+tcp",
 .instance_size= sizeof(BDRVGlusterState),
+.bdrv_needs_filename  = true,
 .bdrv_file_open   = qemu_gluster_open,
 .bdrv_close   = qemu_gluster_close,
 .bdrv_create  = qemu_gluster_create,
@@ -651,6 +653,7 @@ static BlockDriver bdrv_gluster_unix = {
 .format_name  = "gluster",
 .protocol_name= "gluster+unix",
 .instance_size= sizeof(BDRVGlusterState),
+.bdrv_needs_filename  = true,
 .bdrv_file_open   = qemu_gluster_open,
 .bdrv_close   = qemu_gluster_close,
 .bdrv_create  = qemu_gluster_create,
@@ -671,6 +674,7 @@ static BlockDriver bdrv_gluster_rdma = {
 .format_name  = "gluster",
 .protocol_name= "gluster+rdma",
 .instance_size= sizeof(BDRVGlusterState),
+.bdrv_needs_filename  = true,
 .bdrv_file_open   = qemu_gluster_open,
 .bdrv_close   = qemu_gluster_close,
 .bdrv_create  = qemu_gluster_create,
diff --git a/block/iscsi.c b/block/iscsi.c
index 813abd8..cbe0c84 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1304,6 +1304,7 @@ static BlockDriver bdrv_iscsi = {
 .protocol_name   = "iscsi",
 
 .instance_size   = sizeof(IscsiLun),
+.bdrv_needs_filename = true,
 .bdrv_file_open  = iscsi_open,
 .bdrv_close  = iscsi_close,
 .bdrv_create = iscsi_create,
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 1b41ea3..7ac6b97 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1198,6 +1198,7 @@ static BlockDriver bdrv_file = {
 .format_name = "file",
 .protocol_name = "file",
 .instance_size = sizeof(BDRVRawState),
+.bdrv_needs_filename = true,
 .bdrv_probe = NULL, /* no probe for protocols */
 .bdrv_file_open = raw_open,
 .bdrv_reopen_prepare = raw_reopen_prepare,
@@ -1538,6 +1539,7 @@ static BlockDriver bdrv_host_device = {
 .format_name= "host_device",
 .protocol_name= "host_device",
 .instance_size  = sizeof(BDRVRawState),
+.bdrv_needs_filename = true,
 .bdrv_probe_device  = hdev_probe_device,
 .bdrv_file_open = hdev_open,
 .bdrv_close = raw_close,
@@ -1662,6 +1664,7 @@ static BlockDriver bdrv_host_floppy = {
 .format_name= "host_floppy",
 .protocol_name  = "host_floppy",
 .instance_size  = sizeof(BDRVRawState),
+.bdrv_needs_filename = true,
 .bdrv_probe_device = fl

Re: [Qemu-devel] [PATCH] linux-user: Fix wrong use of stat instead of stat64 for sparc64

[Riku Voipio]

On Thu, Sep 19, 2013 at 07:31:51PM +0200, Stefan Weil wrote:
> Ping? Are there any more opinions how qemu-sparc64 should be fixed?
> Should we choose Peter's approach (which is good, but with a
> higher risk than my patch)?

I've included it now in my qeu, since it fixes qemu-sparc64 in my
smoketest and causes seemingly no regressions on other platforms.

Riku

Re: [Qemu-devel] [PATCH 5/5] hw: arm_gic_kvm: Add KVM VGIC save/restore logic

[Peter Maydell]

On 23 September 2013 11:14, Christoffer Dall
 wrote:
> On Sat, Sep 21, 2013 at 06:38:19PM +0900, Peter Maydell wrote:
>  (2) How does the arm_gic_kvm.c code detect the underlying host CPU that
>  the kernel read the register from when it returned the value of the
>  register to do the proper translation?  I don't even want to think
>  about how this will work on Big.Little...

That's why the kernel does the translation, not userspace :-)

-- PMM

Re: [Qemu-devel] [PATCH] linux-user: Fix wrong use of stat instead of stat64 for sparc64

[Peter Maydell]

On 23 September 2013 20:57, Riku Voipio  wrote:
> On Thu, Sep 19, 2013 at 07:31:51PM +0200, Stefan Weil wrote:
>> Ping? Are there any more opinions how qemu-sparc64 should be fixed?
>> Should we choose Peter's approach (which is good, but with a
>> higher risk than my patch)?
>
> I've included it now in my qeu, since it fixes qemu-sparc64 in my
> smoketest and causes seemingly no regressions on other platforms.

Yeah, but I bet there are other platforms we're not getting this
right on since the #ifdef here isn't the same as the #ifdef that
decides whether we have a target_stat64 struct or not...

-- PMM

[Qemu-devel] [PATCH v5 2/6] block: Add bdrv_get_specific_info

[Max Reitz]

Add a function for retrieving an ImageInfoSpecific object from a block
driver.

Signed-off-by: Max Reitz 
---
 block.c   | 9 +
 block/qapi.c  | 3 +++
 include/block/block.h | 1 +
 include/block/block_int.h | 1 +
 4 files changed, 14 insertions(+)

diff --git a/block.c b/block.c
index a2ea39a..4360767 100644
--- a/block.c
+++ b/block.c
@@ -3340,6 +3340,15 @@ int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo 
*bdi)
 return drv->bdrv_get_info(bs, bdi);
 }
 
+ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs)
+{
+BlockDriver *drv = bs->drv;
+if (drv && drv->bdrv_get_specific_info) {
+return drv->bdrv_get_specific_info(bs);
+}
+return NULL;
+}
+
 int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t *buf,
   int64_t pos, int size)
 {
diff --git a/block/qapi.c b/block/qapi.c
index 782051c..ab1dd24 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -134,6 +134,9 @@ void bdrv_query_image_info(BlockDriverState *bs,
 info->dirty_flag = bdi.is_dirty;
 info->has_dirty_flag = true;
 }
+info->format_specific = bdrv_get_specific_info(bs);
+info->has_format_specific = info->format_specific != NULL;
+
 backing_filename = bs->backing_file;
 if (backing_filename[0] != '\0') {
 info->backing_filename = g_strdup(backing_filename);
diff --git a/include/block/block.h b/include/block/block.h
index f808550..e265124 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -335,6 +335,7 @@ int bdrv_get_flags(BlockDriverState *bs);
 int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num,
   const uint8_t *buf, int nb_sectors);
 int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi);
+ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs);
 void bdrv_round_to_clusters(BlockDriverState *bs,
 int64_t sector_num, int nb_sectors,
 int64_t *cluster_sector_num,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 3eeb6fe..5de5b0e 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -162,6 +162,7 @@ struct BlockDriver {
 int (*bdrv_snapshot_load_tmp)(BlockDriverState *bs,
   const char *snapshot_name);
 int (*bdrv_get_info)(BlockDriverState *bs, BlockDriverInfo *bdi);
+ImageInfoSpecific *(*bdrv_get_specific_info)(BlockDriverState *bs);
 
 int (*bdrv_save_vmstate)(BlockDriverState *bs, QEMUIOVector *qiov,
  int64_t pos);
-- 
1.8.3.1

[Qemu-devel] [PATCH v5 6/6] qemu-iotests: Additional info from qemu-img info

[Max Reitz]

Add a test for the additional information now provided by qemu-img info
when used on qcow2 images.

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/065 | 72 ++
 tests/qemu-iotests/065.out | 22 ++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 95 insertions(+)
 create mode 100755 tests/qemu-iotests/065
 create mode 100644 tests/qemu-iotests/065.out

diff --git a/tests/qemu-iotests/065 b/tests/qemu-iotests/065
new file mode 100755
index 000..5c56b56
--- /dev/null
+++ b/tests/qemu-iotests/065
@@ -0,0 +1,72 @@
+#!/bin/bash
+#
+# Test for additional information emitted by qemu-img info on qcow2
+# images
+#
+# Copyright (C) 2013 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program 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 General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+# creator
+owner=mre...@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1   # failure is the default!
+
+_cleanup()
+{
+   _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# This tests qocw2-specific low-level functionality
+_supported_fmt qcow2
+_supported_proto generic
+_supported_os Linux
+
+IMG_SIZE=64M
+
+echo
+echo "=== Testing qcow2 image with -o compat=0.10 ==="
+echo
+IMGOPTS="compat=0.10" _make_test_img $IMG_SIZE
+# don't use _img_info, since that function will filter out the
+# additional information we're about to test for
+$QEMU_IMG info "$TEST_IMG" | sed -n '/^Format specific information:$/,$p'
+
+echo
+echo "=== Testing qcow2 image with -o compat=1.1,lazy_refcounts=off ==="
+echo
+IMGOPTS="compat=1.1,lazy_refcounts=off" _make_test_img $IMG_SIZE
+$QEMU_IMG info "$TEST_IMG" | sed -n '/^Format specific information:$/,$p'
+
+echo
+echo "=== Testing qcow2 image with -o compat=1.1,lazy_refcounts=on ==="
+echo
+IMGOPTS="compat=1.1,lazy_refcounts=on" _make_test_img $IMG_SIZE
+$QEMU_IMG info "$TEST_IMG" | sed -n '/^Format specific information:$/,$p'
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/065.out b/tests/qemu-iotests/065.out
new file mode 100644
index 000..4dd7f2b
--- /dev/null
+++ b/tests/qemu-iotests/065.out
@@ -0,0 +1,22 @@
+QA output created by 065
+
+=== Testing qcow2 image with -o compat=0.10 ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+Format specific information:
+compat: 0.10
+
+=== Testing qcow2 image with -o compat=1.1,lazy_refcounts=off ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+Format specific information:
+compat: 1.1
+lazy refcounts: false
+
+=== Testing qcow2 image with -o compat=1.1,lazy_refcounts=on ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+Format specific information:
+compat: 1.1
+lazy refcounts: true
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 1ad02e5..f1a68b0 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -69,3 +69,4 @@
 061 rw auto
 062 rw auto
 063 rw auto
+065 rw auto
-- 
1.8.3.1

[Qemu-devel] [PATCH v5 0/6] Provide additional info through qemu-img info

[Max Reitz]

qemu-img info provides only pretty general information about an image.
For any image format, there might be specific options which cannot be
represented in a universal way; for instance, qcow2 provides the
compatibility and lazy_refcount options whose values are certainly
interesting but currently cannot be output by qemu-img info.

Therefore, this series adds a new ImageInfoSpecific union type to
ImageInfo which may be used by block drivers as a template for new types
dedicated to the specific information they can provide, as well as a
function bdrv_get_specific_info for retrieving this information. It also
adds support to qemu-img info and qemu-io -c info to print the content
of these specific structures.

v5:
 - In consideration of Stefan's argument about bdrv_round_to_cluster
   being called in the I/O path, removed ImageInfoSpecific from
   BlockDriverInfo and introduced a new function bdrv_get_specific_info
   instead; this also removes the need for bdrv_put_info.
   Resulting changes in regard to the single patches:
- patch 2: completely different
- patch 3: qemu-io-cmds.c:info_f now needs to call
  bdrv_get_specific_info in order to receive the new information
- patch 4: moved the creation of ImageInfoSpecific from qcow2_get_info
  (which now remains unmodified) to the new qcow2_get_specific_info
  function; additionally, register this function as
  bdrv_qcow2.bdrv_get_specific_info
 - rebased on Kevin's block branch, renamed test from 064 to 065 (to
   avoid conflict with his blockdev-add series; affects patch 6)
 - patches 1 and 5 remain unmodified

v4:
 - changed dirty "grep -A 42" for grepping all lines until EOF in test
   064 to cleaner "sed -n '//,$p'" (patch 6)
 - rebased on Kevin's block branch (affects line numbers in patches 2, 3
   and 4 as well as the group file change in patch 6)

v3:
 - implemented Fam's remarks:
   - bdrv_get_info already initializes all fields to NULL, no need to do
 this manually (patch 2)
   - implemented bdrv_put_info as a wrapper to
 qapi_free_ImageInfoSpecific, though this may change with further
 extensions to BlockDriverInfo (patch 2)
   - changed one occurence of puts("foo") to printf("foo\n") in order to
 be consistent with the surrounding code (patch 3)
   - other patches (1, 4, 5, 6) remain unmodified

v2:
 - following Eric's recommendation: changed the representation of the
   format specific information from an uninterpreted blobbed string to a
   union of format specific types


Max Reitz (6):
  qapi: Add ImageInfoSpecific type
  block: Add bdrv_get_specific_info
  block/qapi: Human-readable ImageInfoSpecific dump
  qcow2: Add support for ImageInfoSpecific
  qemu-iotests: Discard specific info in _img_info
  qemu-iotests: Additional info from qemu-img info

 block.c  |   9 
 block/qapi.c | 124 +++
 block/qcow2.c|  19 +++
 include/block/block.h|   1 +
 include/block/block_int.h|   1 +
 include/block/qapi.h |   2 +
 qapi-schema.json |  34 +++-
 qemu-io-cmds.c   |   9 
 tests/qemu-iotests/065   |  72 +
 tests/qemu-iotests/065.out   |  22 
 tests/qemu-iotests/common.rc |  19 ++-
 tests/qemu-iotests/group |   1 +
 12 files changed, 311 insertions(+), 2 deletions(-)
 create mode 100755 tests/qemu-iotests/065
 create mode 100644 tests/qemu-iotests/065.out

-- 
1.8.3.1

[Qemu-devel] [PATCH v5 4/6] qcow2: Add support for ImageInfoSpecific

[Max Reitz]

Add a new ImageInfoSpecificQCow2 type as a subtype of ImageInfoSpecific.
This contains the compatibility level as a string and an optional
lazy_refcounts boolean (optional means mandatory for compat >= 1.1 and
not available for compat == 0.10).

Also, add qcow2_get_specific_info, which returns this information.

Signed-off-by: Max Reitz 
---
 block/qcow2.c| 19 +++
 qapi-schema.json | 16 
 2 files changed, 35 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 318d95d..f080a8a 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1810,6 +1810,24 @@ static int qcow2_get_info(BlockDriverState *bs, 
BlockDriverInfo *bdi)
 return 0;
 }
 
+static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs)
+{
+BDRVQcowState *s = bs->opaque;
+ImageInfoSpecific *spec_info = g_new0(ImageInfoSpecific, 1);
+
+spec_info->kind = IMAGE_INFO_SPECIFIC_KIND_QCOW2;
+spec_info->qcow2 = g_new0(ImageInfoSpecificQCow2, 1);
+if (s->qcow_version == 2) {
+spec_info->qcow2->compat = g_strdup("0.10");
+} else if (s->qcow_version == 3) {
+spec_info->qcow2->compat = g_strdup("1.1");
+spec_info->qcow2->lazy_refcounts = s->use_lazy_refcounts;
+spec_info->qcow2->has_lazy_refcounts = true;
+}
+
+return spec_info;
+}
+
 #if 0
 static void dump_refcounts(BlockDriverState *bs)
 {
@@ -2130,6 +2148,7 @@ static BlockDriver bdrv_qcow2 = {
 .bdrv_snapshot_list = qcow2_snapshot_list,
 .bdrv_snapshot_load_tmp = qcow2_snapshot_load_tmp,
 .bdrv_get_info  = qcow2_get_info,
+.bdrv_get_specific_info = qcow2_get_specific_info,
 
 .bdrv_save_vmstate= qcow2_save_vmstate,
 .bdrv_load_vmstate= qcow2_load_vmstate,
diff --git a/qapi-schema.json b/qapi-schema.json
index cbad705..d697da2 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -210,6 +210,21 @@
 'vm-clock-sec': 'int', 'vm-clock-nsec': 'int' } }
 
 ##
+# @ImageInfoSpecificQCow2:
+#
+# @compat: compatibility level
+#
+# @lazy-refcounts: #optional on or off; only valid for compat >= 1.1
+#
+# Since: 1.7
+##
+{ 'type': 'ImageInfoSpecificQCow2',
+  'data': {
+  'compat': 'str',
+  '*lazy-refcounts': 'bool'
+  } }
+
+##
 # @ImageInfoSpecific:
 #
 # A discriminated record of image format specific information structures.
@@ -219,6 +234,7 @@
 
 { 'union': 'ImageInfoSpecific',
   'data': {
+  'qcow2': 'ImageInfoSpecificQCow2'
   } }
 
 ##
-- 
1.8.3.1

[Qemu-devel] [PATCH v5 3/6] block/qapi: Human-readable ImageInfoSpecific dump

[Max Reitz]

Add a function for generically dumping the ImageInfoSpecific information
in a human-readable format to block/qapi.c.

Use this function in bdrv_image_info_dump and qemu-io-cmds.c:info_f to
allow qemu-img info resp. qemu-io -c info to print that format specific
information.

Signed-off-by: Max Reitz 
---
 block/qapi.c | 121 +++
 include/block/qapi.h |   2 +
 qemu-io-cmds.c   |   9 
 3 files changed, 132 insertions(+)

diff --git a/block/qapi.c b/block/qapi.c
index ab1dd24..e7d1591 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -25,6 +25,9 @@
 #include "block/qapi.h"
 #include "block/block_int.h"
 #include "qmp-commands.h"
+#include "qapi-visit.h"
+#include "qapi/qmp-output-visitor.h"
+#include "qapi/qmp/types.h"
 
 /*
  * Returns 0 on success, with *p_list either set to describe snapshot
@@ -426,6 +429,119 @@ void bdrv_snapshot_dump(fprintf_function func_fprintf, 
void *f,
 }
 }
 
+static void dump_qdict(fprintf_function func_fprintf, void *f, int indentation,
+   QDict *dict);
+static void dump_qlist(fprintf_function func_fprintf, void *f, int indentation,
+   QList *list);
+
+static void dump_qobject(fprintf_function func_fprintf, void *f,
+ int comp_indent, QObject *obj)
+{
+switch (qobject_type(obj)) {
+case QTYPE_QINT: {
+QInt *value = qobject_to_qint(obj);
+func_fprintf(f, "%" PRId64, qint_get_int(value));
+break;
+}
+case QTYPE_QSTRING: {
+QString *value = qobject_to_qstring(obj);
+func_fprintf(f, "%s", qstring_get_str(value));
+break;
+}
+case QTYPE_QDICT: {
+QDict *value = qobject_to_qdict(obj);
+dump_qdict(func_fprintf, f, comp_indent, value);
+break;
+}
+case QTYPE_QLIST: {
+QList *value = qobject_to_qlist(obj);
+dump_qlist(func_fprintf, f, comp_indent, value);
+break;
+}
+case QTYPE_QFLOAT: {
+QFloat *value = qobject_to_qfloat(obj);
+func_fprintf(f, "%g", qfloat_get_double(value));
+break;
+}
+case QTYPE_QBOOL: {
+QBool *value = qobject_to_qbool(obj);
+func_fprintf(f, "%s", qbool_get_int(value) ? "true" : "false");
+break;
+}
+case QTYPE_QERROR: {
+QString *value = qerror_human((QError *)obj);
+func_fprintf(f, "%s", qstring_get_str(value));
+break;
+}
+case QTYPE_NONE:
+break;
+case QTYPE_MAX:
+default:
+abort();
+}
+}
+
+static void dump_qlist(fprintf_function func_fprintf, void *f, int indentation,
+   QList *list)
+{
+const QListEntry *entry;
+int i = 0;
+
+for (entry = qlist_first(list); entry; entry = qlist_next(entry), i++) {
+qtype_code type = qobject_type(entry->value);
+bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
+const char *format = composite ? "%*s[%i]:\n" : "%*s[%i]: ";
+
+func_fprintf(f, format, indentation * 4, "", i);
+dump_qobject(func_fprintf, f, indentation + 1, entry->value);
+if (!composite) {
+func_fprintf(f, "\n");
+}
+}
+}
+
+static void dump_qdict(fprintf_function func_fprintf, void *f, int indentation,
+   QDict *dict)
+{
+const QDictEntry *entry;
+
+for (entry = qdict_first(dict); entry; entry = qdict_next(dict, entry)) {
+qtype_code type = qobject_type(entry->value);
+bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
+const char *format = composite ? "%*s%s:\n" : "%*s%s: ";
+char key[strlen(entry->key) + 1];
+int i;
+
+/* replace dashes with spaces in key (variable) names */
+for (i = 0; entry->key[i]; i++) {
+key[i] = entry->key[i] == '-' ? ' ' : entry->key[i];
+}
+key[i] = 0;
+
+func_fprintf(f, format, indentation * 4, "", key);
+dump_qobject(func_fprintf, f, indentation + 1, entry->value);
+if (!composite) {
+func_fprintf(f, "\n");
+}
+}
+}
+
+void bdrv_image_info_specific_dump(fprintf_function func_fprintf, void *f,
+   ImageInfoSpecific *info_spec)
+{
+Error *local_err = NULL;
+QmpOutputVisitor *ov = qmp_output_visitor_new();
+QObject *obj, *data;
+
+visit_type_ImageInfoSpecific(qmp_output_get_visitor(ov), &info_spec, NULL,
+ &local_err);
+obj = qmp_output_get_qobject(ov);
+assert(qobject_type(obj) == QTYPE_QDICT);
+data = qdict_get(qobject_to_qdict(obj), "data");
+dump_qobject(func_fprintf, f, 0, data);
+qmp_output_visitor_cleanup(ov);
+}
+
 void bdrv_image_info_dump(fprintf_function func_fprintf, void *f,
 

Re: [Qemu-devel] [PATCH] hmp: block-stream: fix typo

[Stefan Hajnoczi]

On Fri, Sep 20, 2013 at 08:07:43AM -0500, Anthony Liguori wrote:
> Found this by enabling C++ errors.  The bool and enum arguments
> are mistakenly flipped.
> 
> Signed-off-by: Anthony Liguori 
> ---
>  hmp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hmp.c b/hmp.c
> index 2a90295..5891507 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1163,7 +1163,7 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict)
>  
>  qmp_block_stream(device, base != NULL, base,
>   qdict_haskey(qdict, "speed"), speed,
> - BLOCKDEV_ON_ERROR_REPORT, true, &error);
> + true, BLOCKDEV_ON_ERROR_REPORT, &error);
>  
>  hmp_handle_error(mon, &error);
>  }
> -- 
> 1.8.0

Acked-by: Stefan Hajnoczi 

[Qemu-devel] [PATCH v5 1/6] qapi: Add ImageInfoSpecific type

[Max Reitz]

Add a new type ImageInfoSpecific as a union for image format specific
information in ImageInfo.

Signed-off-by: Max Reitz 
---
 qapi-schema.json | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 145eca8..cbad705 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -210,6 +210,18 @@
 'vm-clock-sec': 'int', 'vm-clock-nsec': 'int' } }
 
 ##
+# @ImageInfoSpecific:
+#
+# A discriminated record of image format specific information structures.
+#
+# Since: 1.7
+##
+
+{ 'union': 'ImageInfoSpecific',
+  'data': {
+  } }
+
+##
 # @ImageInfo:
 #
 # Information about a QEMU image file
@@ -238,6 +250,9 @@
 #
 # @backing-image: #optional info of the backing image (since 1.6)
 #
+# @info-string: #optional string supplying additional format-specific
+# information (since 1.7)
+#
 # Since: 1.3
 #
 ##
@@ -248,7 +263,8 @@
'*cluster-size': 'int', '*encrypted': 'bool',
'*backing-filename': 'str', '*full-backing-filename': 'str',
'*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'],
-   '*backing-image': 'ImageInfo' } }
+   '*backing-image': 'ImageInfo',
+   '*format-specific': 'ImageInfoSpecific' } }
 
 ##
 # @ImageCheck:
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2] block: Fix compiler warning (-Werror=uninitialized)

[Stefan Hajnoczi]

On Sun, Sep 22, 2013 at 08:19:10AM +0200, Stefan Weil wrote:
> The patch fixes a warning from gcc (Debian 4.6.3-14+rpi1) 4.6.3:
> 
> block/stream.c:141:22: error:
> ‘copy’ may be used uninitialized in this function [-Werror=uninitialized]
> 
> This is not a real bug - a better compiler would not complain.
> 
> Now 'copy' has always a defined value, so the check for ret >= 0
> can be removed.
> 
> Signed-off-by: Stefan Weil 
> ---
> 
> v2:
> 
> As noted by Andreas Färber, 'copy' must be set to false after the goto label.
> Thanks for this hint.
> 
> Regards,
> Stefan
> 
>  block/stream.c |5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/block/stream.c b/block/stream.c
> index 078ce4a..45837f4 100644
> --- a/block/stream.c
> +++ b/block/stream.c
> @@ -119,11 +119,12 @@ wait:
>  break;
>  }
>  
> +copy = false;
> +
>  ret = bdrv_is_allocated(bs, sector_num,
>  STREAM_BUFFER_SIZE / BDRV_SECTOR_SIZE, &n);
>  if (ret == 1) {
>  /* Allocated in the top, no need to copy.  */
> -copy = false;
>  } else if (ret >= 0) {
>  /* Copy if allocated in the intermediate images.  Limit to the
>   * known-unallocated area [sector_num, sector_num+n).  */
> @@ -138,7 +139,7 @@ wait:
>  copy = (ret == 1);
>  }
>  trace_stream_one_iteration(s, sector_num, n, ret);
> -if (ret >= 0 && copy) {
> +if (copy) {
>  if (s->common.speed) {
>  delay_ns = ratelimit_calculate_delay(&s->limit, n);
>  if (delay_ns > 0) {
> -- 
> 1.7.10.4

Acked-by: Stefan Hajnoczi 

[Qemu-devel] [PATCH v5 5/6] qemu-iotests: Discard specific info in _img_info

[Max Reitz]

In _img_info, filter out additional information specific to the image
format provided by qemu-img info, since tests designed for multiple
image formats would produce different outputs for every image format
else.

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/common.rc | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 28b39e4..12d8882 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -181,12 +181,29 @@ _check_test_img()
 
 _img_info()
 {
+discard=0
 $QEMU_IMG info "$@" $TEST_IMG 2>&1 | \
 sed -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \
 -e "s#$TEST_DIR#TEST_DIR#g" \
 -e "s#$IMGFMT#IMGFMT#g" \
 -e "/^disk size:/ D" \
--e "/actual-size/ D"
+-e "/actual-size/ D" | \
+while IFS='' read line; do
+if [ "$line" == "Format specific information:" ]; then
+discard=1
+elif [ "`echo "$line" | sed -e 's/^ *//'`" == '"format-specific": 
{' ]; then
+discard=2
+json_indent="`echo "$line" | sed -e 's/^\( *\).*$/\1/'`"
+fi
+if [ $discard == 0 ]; then
+echo "$line"
+elif [ $discard == 1 -a -z "$line" ]; then
+echo
+discard=0
+elif [ $discard == 2 -a "`echo "$line" | sed -e 's/ *$//'`" == 
"${json_indent}}," ]; then
+discard=0
+fi
+done
 }
 
 _get_pids_by_name()
-- 
1.8.3.1

[Qemu-devel] [PATCH v2] linux-user: Handle SOCK_CLOEXEC/NONBLOCK if unavailable on host

[edgar . iglesias]

From: "Edgar E. Iglesias" 

If the host lacks SOCK_CLOEXEC, bail out with -EINVAL.
If the host lacks SOCK_ONONBLOCK, try to emulate it with fcntl()
and O_NONBLOCK.

Signed-off-by: Edgar E. Iglesias 
---
v2: Dont emulate SOCK_CLOEXEC (Riku)

 linux-user/syscall.c |   40 +---
 1 file changed, 37 insertions(+), 3 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index c62d875..69badfa 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1701,7 +1701,7 @@ static void unlock_iovec(struct iovec *vec, abi_ulong 
target_addr,
 free(vec);
 }
 
-static inline void target_to_host_sock_type(int *type)
+static inline int target_to_host_sock_type(int *type)
 {
 int host_type = 0;
 int target_type = *type;
@@ -1718,22 +1718,56 @@ static inline void target_to_host_sock_type(int *type)
 break;
 }
 if (target_type & TARGET_SOCK_CLOEXEC) {
+#if defined(SOCK_CLOEXEC)
 host_type |= SOCK_CLOEXEC;
+#else
+return -TARGET_EINVAL;
+#endif
 }
 if (target_type & TARGET_SOCK_NONBLOCK) {
+#if defined(SOCK_NONBLOCK)
 host_type |= SOCK_NONBLOCK;
+#elif !defined(O_NONBLOCK)
+return -TARGET_EINVAL;
+#endif
 }
 *type = host_type;
+return 0;
+}
+
+/* Try to emulate socket type flags after socket creation.  */
+static int sock_flags_fixup(int fd, int target_type)
+{
+#if !defined(SOCK_NONBLOCK) && defined(O_NONBLOCK)
+if (target_type & TARGET_SOCK_NONBLOCK) {
+int flags = fcntl(fd, F_GETFL);
+if (fcntl(fd, F_SETFL, O_NONBLOCK | flags) == -1) {
+close(fd);
+return -TARGET_EINVAL;
+}
+}
+#endif
+return fd;
 }
 
 /* do_socket() Must return target values and target errnos. */
 static abi_long do_socket(int domain, int type, int protocol)
 {
-target_to_host_sock_type(&type);
+int target_type = type;
+int ret;
+
+ret = target_to_host_sock_type(&type);
+if (ret) {
+return ret;
+}
 
 if (domain == PF_NETLINK)
 return -EAFNOSUPPORT; /* do not NETLINK socket connections possible */
-return get_errno(socket(domain, type, protocol));
+ret = get_errno(socket(domain, type, protocol));
+if (ret >= 0) {
+ret = sock_flags_fixup(ret, target_type);
+}
+return ret;
 }
 
 /* do_bind() Must return target values and target errnos. */
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH] Extend qemu-ga's 'guest-info' command to expose flag 'success-response'

[Luiz Capitulino]

On Sun, 22 Sep 2013 14:50:54 +0800
Mark Wu  wrote:

> Now we have several qemu-ga commands not returning response on success.
> It has been documented in qga/qapi-schema.json already. This patch exposes
> the 'success-response' flag by extending 'guest-info' command. With this
> change, the clients can handle the command response more flexibly.
> 
> Changes:
>   v2: add the notation 'since 1.7' to the option 'success-response'
> (per Eric Blake's comments)
> 
> Signed-off-by: Mark Wu 

Reviewed-by: Luiz Capitulino 

> ---
>  include/qapi/qmp/dispatch.h |  1 +
>  qapi/qmp-registry.c | 13 +
>  qga/commands.c  |  2 ++
>  qga/qapi-schema.json|  5 -
>  4 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
> index 1ce11f5..f08c1dd 100644
> --- a/include/qapi/qmp/dispatch.h
> +++ b/include/qapi/qmp/dispatch.h
> @@ -48,6 +48,7 @@ QObject *qmp_dispatch(QObject *request);
>  void qmp_disable_command(const char *name);
>  void qmp_enable_command(const char *name);
>  bool qmp_command_is_enabled(const char *name);
> +bool qmp_command_has_success_response(const char *name);
>  char **qmp_get_command_list(void);
>  QObject *qmp_build_error_object(Error *errp);
>  
> diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c
> index 28bbbe8..f86c3a5 100644
> --- a/qapi/qmp-registry.c
> +++ b/qapi/qmp-registry.c
> @@ -79,6 +79,19 @@ bool qmp_command_is_enabled(const char *name)
>  return false;
>  }
>  
> +bool qmp_command_has_success_response(const char *name)
> +{
> +QmpCommand *cmd;
> +
> +QTAILQ_FOREACH(cmd, &qmp_commands, node) {
> +if (strcmp(cmd->name, name) == 0) {
> +return cmd->options != QCO_NO_SUCCESS_RESP;
> +}
> +}
> +
> +return false;
> +}
> +
>  char **qmp_get_command_list(void)
>  {
>  QmpCommand *cmd;
> diff --git a/qga/commands.c b/qga/commands.c
> index 528b082..0df910b 100644
> --- a/qga/commands.c
> +++ b/qga/commands.c
> @@ -63,6 +63,8 @@ struct GuestAgentInfo *qmp_guest_info(Error **err)
>  cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
>  cmd_info->name = g_strdup(*cmd_list);
>  cmd_info->enabled = qmp_command_is_enabled(cmd_info->name);
> +cmd_info->success_response =
> +qmp_command_has_success_response(cmd_info->name);
>  
>  cmd_info_list = g_malloc0(sizeof(GuestAgentCommandInfoList));
>  cmd_info_list->value = cmd_info;
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index 7155b7a..9c6a58f 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -141,10 +141,13 @@
>  #
>  # @enabled: whether command is currently enabled by guest admin
>  #
> +# @success-response: whether command returns a response on success
> +#and only available since 1.7
> +#
>  # Since 1.1.0
>  ##
>  { 'type': 'GuestAgentCommandInfo',
> -  'data': { 'name': 'str', 'enabled': 'bool' } }
> +  'data': { 'name': 'str', 'enabled': 'bool', 'success-response': 'bool' } }
>  
>  ##
>  # @GuestAgentInfo

Re: [Qemu-devel] [PATCH v4 12/23] acpi: add rules to compile ASL source

[Paolo Bonzini]

Il 22/09/2013 15:37, Michael S. Tsirkin ha scritto:
> Detect presence of IASL compiler and use it
> to process ASL source. If not there, use pre-compiled
> files in-tree. Add script to update the in-tree files.
> 
> Note: distros are known to silently update iasl
> so detect correct iasl flags for the installed version on each run as
> opposed to at configure time.

This is not any different from a C compiler, which is likely updated
much much more often than iasl.  However, I don't feel strongly at all
about this.

Paolo

Re: [Qemu-devel] [PATCH] hw/pci: completed master-abort emulation

[Marcel Apfelbaum]

On Mon, 2013-09-23 at 14:27 +0300, Michael S. Tsirkin wrote:
> On Mon, Sep 23, 2013 at 02:01:17PM +0300, Marcel Apfelbaum wrote:
> > This patch is implemented on top of series:
> > [PATCH v5 0/3] pci: implement upstream master abort protocol
> > 
> > Added "master abort io" background region for PCIBus.
> > 
> > Added "master abort mem" region to catch transactions initiated
> > by pci devices targeted to unassigned addresses.
> > 
> > Enabled "master abort" regions for PCI-2-PCI bridge's secondary bus.
> > 
> > Set "Received Master Abort" Bit on Status/Secondary Status
> > register as defined in the PCI Spec.
> > 
> > Signed-off-by: Marcel Apfelbaum 
> 
> I think it will be easier to review the complete
> series, not an incremental patch.
Should I combine this with the prev series without the
memory patches?

> 
> We probably need to think what's the right thing
> to do for master abort mode since we do not
> emulate SERR. Do we make it writeable at all?
Master abort mode can be enabled. (Tested)

> 
> It's a pci only bit:
>   When the Master-Abort Mode bit is cleared and a posted write transaction
>   forwarded by the
>   bridge terminates with a Master-Abort, no error is reported (note that
>   the Master-Abort bit
>   is still set). When the Master-Abort Mode bit is set and a posted write
>   transaction forwarded
>   by the bridge terminates with a Master-Abort on the destination bus, the
>   bridge must:
>   Assert SERR# on the primary interfaceCommand register)
>   (if enabled by the SERR# Enable bitin the
>   Set the Signaled System Errorbit in the Command register)
>   bitin the Status register (if enabled by the SERR# Enable)
> 
> one way to do this would be not to set Master-Abort bit even
> though spec says we should, and pretend there was no error.
Maybe we can just not allow to set "Master abort mode"
in BRIDGE_CONTROL register. It is a cleaner way (I think)
considering we don't emulate SERR.

> 
> > ---
> >  hw/pci/pci.c | 115 
> > ++-
> >  hw/pci/pci_bridge.c  |  10 +
> >  include/hw/pci/pci.h |   3 ++
> >  include/hw/pci/pci_bus.h |   1 +
> >  4 files changed, 118 insertions(+), 11 deletions(-)
> > 
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index d8a1b11..1f4e707 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -283,17 +283,76 @@ const char *pci_root_bus_path(PCIDevice *dev)
> >  return rootbus->qbus.name;
> >  }
> >  
> > +static PCIDevice *pci_bus_find_host(PCIBus *bus)
> > +{
> > +PCIDevice *dev;
> > +int i;
> > +
> > +for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
> > +dev = bus->devices[i];
> > +if (dev) {
> > +PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
> > +if (pc->class_id == PCI_CLASS_BRIDGE_HOST) {
> 
> In fact you can do this easier:
>pci_get_word(dev->config + PCI_CLASS_DEVICE) == PCI_CLASS_BRIDGE_HOST
Thanks, I'll change.
> 
> > +return dev;
> > +}
> > +}
> > +}
> > +
> > +return NULL;
> > +}
> > +
> > +static void master_abort(void *opaque)
> > +{
> > +PCIDevice *master_dev = NULL;
> > +PCIDeviceClass *pc;
> > +PCIBus *bus;
> > +int downstream;
> 
> bool?
Yes, I'll change

> > +
> > +if (object_dynamic_cast(OBJECT(opaque), TYPE_PCI_DEVICE)) {
> 
> Please don't do dynamic cast just because you can.
> You know very well what kind of object you pass
> when you create the MR.
> So just call the appropriate function.
I wanted to merge the code for all 3 entities involved:
root PCIBus, PCIBus behind a bridge, and PCIDevice.
The region for merge was to use the same master_abort_mem_ops
because there are the same operations that must be done:
return -1 (0xFFF...) and set master abort received bit. 

> 
> > +master_dev = PCI_DEVICE(opaque);
> > +bus = master_dev->bus;
> > +while (!pci_bus_is_root(bus)) {
> > +master_dev = bus->parent_dev;
> > +bus = master_dev->bus;
> > +}
> > +downstream = 0;
> > +}
> > +
> > +if (object_dynamic_cast(OBJECT(opaque), TYPE_PCI_BUS)) {
> > +bus = PCI_BUS(opaque);
> > +if (pci_bus_is_root(bus)) {
> > +master_dev = pci_bus_find_host(bus);
> > +} else { /* bus behind a PCI-2-PCI bridge */
> > +master_dev = bus->parent_dev;
> > +}
> 
> So the reason for this is that device to device MA
> is still not implemented here, correct?
Nope, the above code differentiate 2 scenarios:
1. the bus is the root bus => look for host bridge
   to update STATUS register
2. the bus is bridge's secondary bus =>
   update bridge STATUS register
"Device 2 Device" is handled implicitly because we have
a background region covering all device's target memory

> If it was it would be just one instance of it.
> I'm not saying this must block this patch, however
> there should be a code comment t

Re: [Qemu-devel] [PATCH v2 3/3] block: qemu-iotests - quote $TEST_IMG* and $TEST_DIR usage

[Stefan Hajnoczi]

On Fri, Sep 20, 2013 at 01:12:40PM -0400, Jeff Cody wrote:
> diff --git a/tests/qemu-iotests/043 b/tests/qemu-iotests/043
> index 478773d..ae7cf27 100755
> --- a/tests/qemu-iotests/043
> +++ b/tests/qemu-iotests/043
> @@ -31,7 +31,7 @@ status=1# failure is the default!
>  _cleanup()
>  {
>  _cleanup_test_img
> -rm -f $TEST_IMG.[123].base
> +rm -f "$TEST_IMG.[123].base"

This changes the semantics:

  $ touch a1 a2 a3
  $ echo a[123]
  a1 a2 a3
  $ echo "a[123]"
  a[123]

Re: [Qemu-devel] [PATCH v4 15/23] i386: add bios linker/loader

[Paolo Bonzini]

Il 22/09/2013 15:37, Michael S. Tsirkin ha scritto:
> This adds a dynamic bios linker/loader.
> This will be used by acpi table generation
> code to:
> - load each table in the appropriate memory segment
> - link tables to each other
> - fix up checksums after said linking
> 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  include/hw/i386/bios-linker-loader.h |  26 ++
>  hw/i386/bios-linker-loader.c | 156 
> +++
>  hw/i386/Makefile.objs|   1 +

Just a naming question...

If this is ACPI-specific (or if there are no other uses in sight), it
should go in include/hw/acpi and hw/acpi, and renamed with s/bios/acpi/ etc.

If this is i386-specific, it should go entirely in hw/i386 and the
headers should be included as "bios-linker-loader.h".

Also, the API is (understandably) bios_linker_*, not
bios_linker_loader_*, with the exception of BiosLinkerLoaderEntry,
BIOS_LINKER_LOADER_FILESZ and perhaps a few others I've missed.  I would
just drop the loader part, and if you go for the BIOS -> ACPI rename,
make the filenames include/hw/acpi/linker.h and hw/acpi/linker.c.

Just post v5 of this patch in any case, I don't need to see the matching
changes in patch 23.

Paolo

>  3 files changed, 183 insertions(+)
>  create mode 100644 include/hw/i386/bios-linker-loader.h
>  create mode 100644 hw/i386/bios-linker-loader.c
> 
> diff --git a/include/hw/i386/bios-linker-loader.h 
> b/include/hw/i386/bios-linker-loader.h
> new file mode 100644
> index 000..18c3868
> --- /dev/null
> +++ b/include/hw/i386/bios-linker-loader.h
> @@ -0,0 +1,26 @@
> +#ifndef BIOS_LINKER_LOADER_H
> +#define BIOS_LINKER_LOADER_H
> +
> +#include 
> +#include 
> +#include 
> +
> +GArray *bios_linker_init(void);
> +
> +void bios_linker_alloc(GArray *linker,
> +   const char *file,
> +   uint32_t alloc_align,
> +   bool alloc_fseg);
> +
> +void bios_linker_add_checksum(GArray *linker, const char *file, void *table,
> +  void *start, unsigned size, uint8_t *checksum);
> +
> +
> +void bios_linker_add_pointer(GArray *linker,
> + const char *dest_file,
> + const char *src_file,
> + GArray *table, void *pointer,
> + uint8_t pointer_size);
> +
> +void *bios_linker_cleanup(GArray *linker);
> +#endif
> diff --git a/hw/i386/bios-linker-loader.c b/hw/i386/bios-linker-loader.c
> new file mode 100644
> index 000..644016b
> --- /dev/null
> +++ b/hw/i386/bios-linker-loader.c
> @@ -0,0 +1,156 @@
> +/* Dynamic linker/loader of ACPI tables
> + *
> + * Copyright (C) 2013 Red Hat Inc
> + *
> + * Author: Michael S. Tsirkin 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> +
> + * This program 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 General Public License for more details.
> +
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see .
> + */
> +
> +#include "hw/i386/bios-linker-loader.h"
> +#include "hw/nvram/fw_cfg.h"
> +
> +#include 
> +#include 
> +#include "qemu/bswap.h"
> +
> +#define BIOS_LINKER_LOADER_FILESZ FW_CFG_MAX_FILE_PATH
> +
> +struct BiosLinkerLoaderEntry {
> +uint32_t command;
> +union {
> +/*
> + * COMMAND_ALLOCATE - allocate a table from @alloc_file
> + * subject to @alloc_align alignment (must be power of 2)
> + * and @alloc_zone (can be HIGH or FSEG) requirements.
> + *
> + * Must appear exactly once for each file, and before
> + * this file is referenced by any other command.
> + */
> +struct {
> +char alloc_file[BIOS_LINKER_LOADER_FILESZ];
> +uint32_t alloc_align;
> +uint8_t alloc_zone;
> +};

I would prefer to have named structs, like

struct {
char file[BIOS_LINKER_LOADER_FILESZ];
uint32_t align;
uint8_t zone;
} alloc;

Also perhaps add QEMU_PACKED here too, just in case someone makes
typedefs out of the structs and forgets it.

> +/*
> + * COMMAND_ADD_POINTER - patch the table (originating from
> + * @dest_file) at @pointer_offset, by adding a pointer to the table
> + * originating from @src_file. 1,2,4 or 8 byte unsigned
> + * addition is used depending on @pointer_size.
> + */
> +struct {
> +char pointer_dest_file[BIOS_LINKER_LOADER_FILESZ];
> +char pointer_src_fil

Re: [Qemu-devel] [PATCH] virtio-net: broken RX filtering logic fixed

[Stefan Hajnoczi]

On Sun, Sep 22, 2013 at 06:09:13PM +0300, Dmitry Fleytman wrote:
> From: Dmitry Fleytman 
> 
> Upon processing of VIRTIO_NET_CTRL_MAC_TABLE_SET command
> multicast list overwrites unicast list in mac_table.
> This leads to broken logic for both unicast and multicast RX filtering.
> 
> Signed-off-by: Dmitry Fleytman 
> ---
>  hw/net/virtio-net.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Thanks, applied to my net-next tree:
https://github.com/stefanha/qemu/commits/net-next

Stefan

Re: [Qemu-devel] [PATCH 02/12] trace+libvirt: start trace processing thread in final child process

[Stefan Hajnoczi]

On Fri, Sep 06, 2013 at 04:06:08PM +0200, Michael Mueller wrote:
> On Wed, 17 Jul 2013 10:08:15 +0800
> Stefan Hajnoczi  wrote:
> 
> > On Tue, Jul 16, 2013 at 02:17:28PM +0200, Michael Mueller wrote:
> > > On Tue, 16 Jul 2013 11:05:11 +0800
> > > Stefan Hajnoczi  wrote:
> > > 
> > > > On Mon, Jul 15, 2013 at 09:41:19PM +0200, Christian Borntraeger wrote:
> > > > > When running with trace backend e.g. "simple" the writer thread
> > > > > needs to be implemented in the same process context as the trace
> > > > > points that will be processed. Under libvirtd control, qemu gets
> > > > > first started in daemonized mode to privide its capabilities.
> > > > > Creating the writer thread in the initial process context then
> > > > > leads to a dead lock because the thread gets termined together with
> > > > > the initial parent. (-daemonize) This results in stale qemu
> > > > > processes. Fix this by deferring trace initialization.
> > > > 
> > > > I don't think this works since trace events will fill up trace_buf[]
> > > > and eventually invoke flush_trace_file().
> > > > 
> > > > At that point we use trace_available_cond and trace_empty_cond, which
> > > > may be NULL in Glib <2.31.0.
> > > > 
> > > > Perhaps this can be made safe by checking trace_writeout_enabled.  It
> > > > will be false before the backend has been initialized.
> > > > 
> > > > Stefan
> > > > 
> > > 
> > > You mean something like this. I'll give it a try:
> > > 
> > > ---
> > >  trace/simple.c |3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > --- a/trace/simple.c
> > > +++ b/trace/simple.c
> > > @@ -53,7 +53,7 @@ static GCond *trace_empty_cond;
> > >  #endif
> > >  
> > >  static bool trace_available;
> > > -static bool trace_writeout_enabled;
> > > +static bool trace_writeout_enabled = false;
> > 
> > static bool is automatically initialized to false.
> > 
> > >  enum {
> > >  TRACE_BUF_LEN = 4096 * 64,
> > > @@ -427,5 +427,6 @@ bool trace_backend_init(const char *even
> > >  atexit(st_flush_trace_buffer);
> > >  trace_backend_init_events(events);
> > >  st_set_trace_file(file);
> > > +trace_writeout_enabled = false;
> > 
> > I was thinking along the lines of trace_record_finish() not calling
> > flush_trace_file() if trace_writeout_enabled is false.
> > 
> > Stefan
> > 
> 
> I just looked into it again and think that it is save the way I suggested, 
> because as long
> trace_backend_init() isn't called, also trace_backend_init_events() hasn't 
> registered any
> events. Thus no trace records will be written and can fill up the trace 
> buffer.

Good point, Michael.

Do you mind resending your latest code rebased onto qemu.git/master?
It's been a while since we discussed these patches and I'd like to make
sure we have a fresh email thread to review and complete the merge.

Stefan

Re: [Qemu-devel] I/O performance degradation with Virtio-Blk-Data-Plane

[Stefan Hajnoczi]

On Thu, Sep 05, 2013 at 10:18:28AM +0900, Jonghwan Choi wrote:

Thanks for posting these details.

Have you tried running x-data-plane=off with vcpu = 8 and how does the
performance compare to x-data-plane=off with vcpu = 1?

> > 1. The fio results so it's clear which cases performed worse and by how
> >much.
> >
> When I set vcpu = 8, read performance is decreased about 25%.
> In my test, when vcpu  = 1, I got a best formance.

Performance with vcpu = 8 is 25% worse than performance with vcpu = 1?

Can you try pinning threads to host CPUs?  See libvirt emulatorpin and
vcpupin attributes:
http://libvirt.org/formatdomain.html#elementsCPUTuning

> > 2. The fio job files.
> > 
> [testglobal]
> description=high_iops
> exec_prerun="echo 3 > /proc/sys/vm/drop_caches"
> group_reporting=1
> rw=read
> direct=1
> ioengine=sync
> bs=4m
> numjobs=1
> size=2048m

A couple of points to check:

1. This test case is synchronous and latency-sensitive, you are not
   benchmarking parallel I/Os so x-data-plane=on is not expected to
   perform any better than x-data-plane=off.

   The point of x-data-plane=on is for smp > 1 guests with parallel I/O
   to scale well.  If both those conditions are not met by the workload
   then I don't expect you to see any gains over x-data-plane=off.

   If you want to try parallel I/Os, I suggest using:

   ioengine=linuxaio
   iodepth=16

2. size=2048m with bs=4m on an SSD drive seems quite small because the
   test would complete quickly.  What is the overall running time of
   this test?

   In order to collect stable results it's usually a good idea for the
   test to run for a couple of minutes (e.g. 2 minutes minimum).
   Otherwise outliers can influence the results too much.

   You may need to increase 'size' or use the 'runtime=2m' option
   instead.

Stefan

Re: [Qemu-devel] [PATCH] scsi: prefer UUID to VM name for the initiator name

[Stefan Hajnoczi]

On Thu, Sep 12, 2013 at 10:51:48AM +0200, Paolo Bonzini wrote:
> Il 12/09/2013 10:38, Stefan Hajnoczi ha scritto:
> > On Tue, Sep 10, 2013 at 06:40:04PM +0200, Paolo Bonzini wrote:
> >> The UUID is unique even across multiple hosts, thus it is
> >> better than a VM name even if it is less user-friendly.
> >>
> >> Signed-off-by: Paolo Bonzini 
> >> ---
> >>  block/iscsi.c   | 23 ---
> >>  include/sysemu/sysemu.h |  2 ++
> >>  stubs/Makefile.objs |  1 +
> >>  stubs/uuid.c| 12 
> >>  4 files changed, 31 insertions(+), 7 deletions(-)
> >>  create mode 100644 stubs/uuid.c
> > 
> > Will changing the initiator name break existing setups/ACLs?
> 
> The username for authentication is separate from the initiator name.
> 
> It could break persistent reservations, but there is a workaround
> (specify the initiator name manually through "-iscsi").  I'll make sure
> to document this in the release changelog.
> 
> Does it look good?

It's already merged but for the record, I'm happy with the patch.

Stefan

Re: [Qemu-devel] [PATCH v4 15/23] i386: add bios linker/loader

[Michael S. Tsirkin]

On Mon, Sep 23, 2013 at 02:48:57PM +0200, Paolo Bonzini wrote:
> Il 22/09/2013 15:37, Michael S. Tsirkin ha scritto:
> > This adds a dynamic bios linker/loader.
> > This will be used by acpi table generation
> > code to:
> > - load each table in the appropriate memory segment
> > - link tables to each other
> > - fix up checksums after said linking
> > 
> > Signed-off-by: Michael S. Tsirkin 
> > ---
> >  include/hw/i386/bios-linker-loader.h |  26 ++
> >  hw/i386/bios-linker-loader.c | 156 
> > +++
> >  hw/i386/Makefile.objs|   1 +
> 
> Just a naming question...
> 
> If this is ACPI-specific (or if there are no other uses in sight), it
> should go in include/hw/acpi and hw/acpi, and renamed with s/bios/acpi/ etc.
> 
> If this is i386-specific, it should go entirely in hw/i386 and the
> headers should be included as "bios-linker-loader.h".

Definitely not ACPI specific.
It's only used on i386 so I put it all under i386.
You think we should replace
#include "hw/i386/bios-linker-loader.h" with
#include "bios-linker-loader.h"?

I don't really care, even though this makes headers
hard to find. What's the benefit?

> Also, the API is (understandably) bios_linker_*, not
> bios_linker_loader_*, with the exception of BiosLinkerLoaderEntry,
> BIOS_LINKER_LOADER_FILESZ and perhaps a few others I've missed.  I would
> just drop the loader part, and if you go for the BIOS -> ACPI rename,
> make the filenames include/hw/acpi/linker.h and hw/acpi/linker.c.
> 
> Just post v5 of this patch in any case, I don't need to see the matching
> changes in patch 23.
> 
> Paolo

It's not easy to find a good name for it.
For now I'll just make all names bios_linker_loader_ consistently.
We can always rename it later, it's not set in stone.

> >  3 files changed, 183 insertions(+)
> >  create mode 100644 include/hw/i386/bios-linker-loader.h
> >  create mode 100644 hw/i386/bios-linker-loader.c
> > 
> > diff --git a/include/hw/i386/bios-linker-loader.h 
> > b/include/hw/i386/bios-linker-loader.h
> > new file mode 100644
> > index 000..18c3868
> > --- /dev/null
> > +++ b/include/hw/i386/bios-linker-loader.h
> > @@ -0,0 +1,26 @@
> > +#ifndef BIOS_LINKER_LOADER_H
> > +#define BIOS_LINKER_LOADER_H
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +GArray *bios_linker_init(void);
> > +
> > +void bios_linker_alloc(GArray *linker,
> > +   const char *file,
> > +   uint32_t alloc_align,
> > +   bool alloc_fseg);
> > +
> > +void bios_linker_add_checksum(GArray *linker, const char *file, void 
> > *table,
> > +  void *start, unsigned size, uint8_t 
> > *checksum);
> > +
> > +
> > +void bios_linker_add_pointer(GArray *linker,
> > + const char *dest_file,
> > + const char *src_file,
> > + GArray *table, void *pointer,
> > + uint8_t pointer_size);
> > +
> > +void *bios_linker_cleanup(GArray *linker);
> > +#endif
> > diff --git a/hw/i386/bios-linker-loader.c b/hw/i386/bios-linker-loader.c
> > new file mode 100644
> > index 000..644016b
> > --- /dev/null
> > +++ b/hw/i386/bios-linker-loader.c
> > @@ -0,0 +1,156 @@
> > +/* Dynamic linker/loader of ACPI tables
> > + *
> > + * Copyright (C) 2013 Red Hat Inc
> > + *
> > + * Author: Michael S. Tsirkin 
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > +
> > + * This program 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 General Public License for more details.
> > +
> > + * You should have received a copy of the GNU General Public License along
> > + * with this program; if not, see .
> > + */
> > +
> > +#include "hw/i386/bios-linker-loader.h"
> > +#include "hw/nvram/fw_cfg.h"
> > +
> > +#include 
> > +#include 
> > +#include "qemu/bswap.h"
> > +
> > +#define BIOS_LINKER_LOADER_FILESZ FW_CFG_MAX_FILE_PATH
> > +
> > +struct BiosLinkerLoaderEntry {
> > +uint32_t command;
> > +union {
> > +/*
> > + * COMMAND_ALLOCATE - allocate a table from @alloc_file
> > + * subject to @alloc_align alignment (must be power of 2)
> > + * and @alloc_zone (can be HIGH or FSEG) requirements.
> > + *
> > + * Must appear exactly once for each file, and before
> > + * this file is referenced by any other command.
> > + */
> > +struct {
> > +char alloc_file[BIOS_LINKER_LOADER_FILESZ];
> > +uint32_t alloc_align;
> > +uint

[Qemu-devel] [PATCH v3 14/18] milkymist: Suppress -kernel/-bios/-drive error for qtest

[Andreas Färber]

Acked-by: Michael Walle 
Signed-off-by: Andreas Färber 
---
 hw/lm32/milkymist.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c
index f1744ec..15053c4 100644
--- a/hw/lm32/milkymist.c
+++ b/hw/lm32/milkymist.c
@@ -21,6 +21,7 @@
 #include "hw/hw.h"
 #include "hw/block/flash.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/qtest.h"
 #include "hw/devices.h"
 #include "hw/boards.h"
 #include "hw/loader.h"
@@ -143,7 +144,7 @@ milkymist_init(QEMUMachineInitArgs *args)
 reset_info->bootstrap_pc = BIOS_OFFSET;
 
 /* if no kernel is given no valid bios rom is a fatal error */
-if (!kernel_filename && !dinfo && !bios_filename) {
+if (!kernel_filename && !dinfo && !bios_filename && !qtest_enabled()) {
 fprintf(stderr, "qemu: could not load Milkymist One bios '%s'\n",
 bios_name);
 exit(1);
-- 
1.8.1.4

[Qemu-devel] [PATCH v3 00/18] qtest: Test all targets

[Andreas Färber]

Hello,

As discussed on the KVM Call, this series extends test coverage to all 16 
targets.
For now it tests that QOM type changes do not lead to QOM cast assertions.

v3 macro'fies and cleans up ppcemb machine test setup.
There was a discussion of dropping usage of qtest_enabled() in favor of never
erroring out in machine inits but Aurélien went ahead to restore mips error
behavior, Markus suggested as alternative to supply dummy blobs and real blobs
per machine seem infeasible. This solution seems the easiest way forward, be it
an interim solution, so I am planning to take this through qom-next before I
pull in more QOM realize conversions on so far untested machines. Similarly
I see it as more important to get test coverage quickly than waiting for a
working QMP query-machines based solution; if we do more than just machine
instantiation, we will need to pass in machine-specific QOM paths anyway.

Regards,
Andreas

v2 -> v3:
* Rebased onto mips and Makefile changes; ppc patches were applied.
* Use macros for machine list traversal.

v1 -> v2:
* gumstix, z2: Avoided conditionalizing use of pflash device in favor of NULL 
bdrv.
* puv3: Limited qtest workaround to a NULL kernel_filename.
* Added error workarounds for milkymist, ppc405, shix and leon3.
* Cleaned up debug output for ppc405 and shix.
* Extended qom-test to cover virtually all machines, including n800 and pc.
* Moved machine names to arrays wherever sensible, to aid with extensibility.
* Adopted error_report() for armv7m, too.

Cc: Anthony Liguori 
Cc: Aurélien Jarno 
Cc: Paolo Bonzini 
Cc: Peter Maydell 

Andreas Färber (18):
  mips_mipssim: Silence BIOS loading warning for qtest
  arm/boot: Turn arm_load_kernel() into no-op for qtest without -kernel
  puv3: Turn puv3_load_kernel() into a no-op for qtest without -kernel
  mainstone: Don't enforce use of -pflash for qtest
  gumstix: Don't enforce use of -pflash for qtest
  z2: Don't enforce use of -pflash for qtest
  palm: Don't enforce loading ROM or kernel for qtest
  omap_sx1: Don't enforce use of kernel or flash for qtest
  exynos4_boards: Silence lack of -smp 2 warning for qtest
  armv7m: Don't enforce use of kernel for qtest
  axis_dev88: Don't enforce use of kernel for qtest
  mcf5208: Don't enforce use of kernel for qtest
  an5206: Don't enforce use of kernel for qtest
  milkymist: Suppress -kernel/-bios/-drive error for qtest
  shix: Drop debug output
  shix: Don't require firmware presence for qtest
  leon3: Don't enforce use of -bios with qtest
  qtest: Prepare QOM machine tests

 hw/arm/armv7m.c |  25 ++---
 hw/arm/boot.c   |   4 +
 hw/arm/exynos4_boards.c |   3 +-
 hw/arm/gumstix.c|  11 ++-
 hw/arm/mainstone.c  |   5 +-
 hw/arm/omap_sx1.c   |   3 +-
 hw/arm/palm.c   |   3 +-
 hw/arm/z2.c |   5 +-
 hw/block/tc58128.c  |  10 +-
 hw/cris/axis_dev88.c|  11 ++-
 hw/lm32/milkymist.c |   3 +-
 hw/m68k/an5206.c|   4 +
 hw/m68k/mcf5208.c   |   4 +
 hw/mips/mips_mipssim.c  |   4 +-
 hw/sh4/shix.c   |  16 +--
 hw/sparc/leon3.c|   3 +-
 hw/unicore32/puv3.c |   4 +
 tests/Makefile  |  26 +
 tests/qom-test.c| 253 
 19 files changed, 353 insertions(+), 44 deletions(-)
 create mode 100644 tests/qom-test.c

-- 
1.8.1.4

[Qemu-devel] [PATCH v3 16/18] shix: Don't require firmware presence for qtest

[Andreas Färber]

Adopt error_report() while at it.

Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Aurelien Jarno 
Signed-off-by: Andreas Färber 
---
 hw/block/tc58128.c | 10 ++
 hw/sh4/shix.c  |  9 +
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/hw/block/tc58128.c b/hw/block/tc58128.c
index a3929d4..728f1c3 100644
--- a/hw/block/tc58128.c
+++ b/hw/block/tc58128.c
@@ -1,6 +1,8 @@
 #include "hw/hw.h"
 #include "hw/sh4/sh.h"
 #include "hw/loader.h"
+#include "sysemu/qtest.h"
+#include "qemu/error-report.h"
 
 #define CE1  0x0100
 #define CE2  0x0200
@@ -36,10 +38,10 @@ static void init_dev(tc58128_dev * dev, const char 
*filename)
/* Load flash image skipping the first block */
ret = load_image(filename, dev->flash_contents + 528 * 32);
if (ret < 0) {
-   fprintf(stderr, "ret=%d\n", ret);
-   fprintf(stderr, "qemu: could not load flash image %s\n",
-   filename);
-   exit(1);
+if (!qtest_enabled()) {
+error_report("Could not load flash image %s", filename);
+exit(1);
+}
} else {
/* Build first block with number of blocks */
blocks = (ret + 528 * 32 - 1) / (528 * 32);
diff --git a/hw/sh4/shix.c b/hw/sh4/shix.c
index f008b98..904a966 100644
--- a/hw/sh4/shix.c
+++ b/hw/sh4/shix.c
@@ -30,9 +30,11 @@
 #include "hw/hw.h"
 #include "hw/sh4/sh.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/qtest.h"
 #include "hw/boards.h"
 #include "hw/loader.h"
 #include "exec/address-spaces.h"
+#include "qemu/error-report.h"
 
 #define BIOS_FILENAME "shix_bios.bin"
 #define BIOS_ADDRESS 0xA000
@@ -72,10 +74,9 @@ static void shix_init(QEMUMachineInitArgs *args)
 if (bios_name == NULL)
 bios_name = BIOS_FILENAME;
 ret = load_image_targphys(bios_name, 0, 0x4000);
-if (ret < 0) { /* Check bios size */
-   fprintf(stderr, "qemu: could not load SHIX bios '%s'\n",
-   bios_name);
-   exit(1);
+if (ret < 0 && !qtest_enabled()) {
+error_report("Could not load SHIX bios '%s'", bios_name);
+exit(1);
 }
 
 /* Register peripherals */
-- 
1.8.1.4

[Qemu-devel] [PATCH v3 02/18] arm/boot: Turn arm_load_kernel() into no-op for qtest without -kernel

[Andreas Färber]

Signed-off-by: Andreas Färber 
---
 hw/arm/boot.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 1e313af..0c3dc5f 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -15,6 +15,7 @@
 #include "hw/loader.h"
 #include "elf.h"
 #include "sysemu/device_tree.h"
+#include "sysemu/qtest.h"
 #include "qemu/config-file.h"
 
 #define KERNEL_ARGS_ADDR 0x100
@@ -354,6 +355,9 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
*info)
 
 /* Load the kernel.  */
 if (!info->kernel_filename) {
+if (qtest_enabled()) {
+return;
+}
 fprintf(stderr, "Kernel image must be specified\n");
 exit(1);
 }
-- 
1.8.1.4

[Qemu-devel] [PATCH v3 07/18] palm: Don't enforce loading ROM or kernel for qtest

[Andreas Färber]

Signed-off-by: Andreas Färber 
---
 hw/arm/palm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/arm/palm.c b/hw/arm/palm.c
index 3e39044..ff6bfab 100644
--- a/hw/arm/palm.c
+++ b/hw/arm/palm.c
@@ -19,6 +19,7 @@
 #include "hw/hw.h"
 #include "audio/audio.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/qtest.h"
 #include "ui/console.h"
 #include "hw/arm/omap.h"
 #include "hw/boards.h"
@@ -255,7 +256,7 @@ static void palmte_init(QEMUMachineInitArgs *args)
 }
 }
 
-if (!rom_loaded && !kernel_filename) {
+if (!rom_loaded && !kernel_filename && !qtest_enabled()) {
 fprintf(stderr, "Kernel or ROM image must be specified\n");
 exit(1);
 }
-- 
1.8.1.4

[Qemu-devel] [PATCH v3 10/18] armv7m: Don't enforce use of kernel for qtest

[Andreas Färber]

Adopt error_report().

Signed-off-by: Andreas Färber 
---
 hw/arm/armv7m.c | 25 ++---
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 89a9015..397e8df 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -11,6 +11,8 @@
 #include "hw/arm/arm.h"
 #include "hw/loader.h"
 #include "elf.h"
+#include "sysemu/qtest.h"
+#include "qemu/error-report.h"
 
 /* Bitbanded IO.  Each word corresponds to a single bit.  */
 
@@ -232,21 +234,22 @@ qemu_irq *armv7m_init(MemoryRegion *address_space_mem,
 big_endian = 0;
 #endif
 
-if (!kernel_filename) {
+if (!kernel_filename && !qtest_enabled()) {
 fprintf(stderr, "Guest image must be specified (using -kernel)\n");
 exit(1);
 }
 
-image_size = load_elf(kernel_filename, NULL, NULL, &entry, &lowaddr,
-  NULL, big_endian, ELF_MACHINE, 1);
-if (image_size < 0) {
-image_size = load_image_targphys(kernel_filename, 0, flash_size);
-   lowaddr = 0;
-}
-if (image_size < 0) {
-fprintf(stderr, "qemu: could not load kernel '%s'\n",
-kernel_filename);
-exit(1);
+if (kernel_filename) {
+image_size = load_elf(kernel_filename, NULL, NULL, &entry, &lowaddr,
+  NULL, big_endian, ELF_MACHINE, 1);
+if (image_size < 0) {
+image_size = load_image_targphys(kernel_filename, 0, flash_size);
+lowaddr = 0;
+}
+if (image_size < 0) {
+error_report("Could not load kernel '%s'", kernel_filename);
+exit(1);
+}
 }
 
 /* Hack to map an additional page of ram at the top of the address
-- 
1.8.1.4

[Qemu-devel] [PATCH v3 03/18] puv3: Turn puv3_load_kernel() into a no-op for qtest without -kernel

[Andreas Färber]

Replacing the assert() with more user-friendly error handling is left
for a follow-up.

Signed-off-by: Andreas Färber 
---
 hw/unicore32/puv3.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/unicore32/puv3.c b/hw/unicore32/puv3.c
index a900061..e05cbc1 100644
--- a/hw/unicore32/puv3.c
+++ b/hw/unicore32/puv3.c
@@ -17,6 +17,7 @@
 #include "hw/boards.h"
 #include "hw/loader.h"
 #include "hw/i386/pc.h"
+#include "sysemu/qtest.h"
 
 #undef DEBUG_PUV3
 #include "hw/unicore32/puv3.h"
@@ -84,6 +85,9 @@ static void puv3_load_kernel(const char *kernel_filename)
 {
 int size;
 
+if (kernel_filename == NULL && qtest_enabled()) {
+return;
+}
 assert(kernel_filename != NULL);
 
 /* only zImage format supported */
-- 
1.8.1.4

[Qemu-devel] [PATCH v3 04/18] mainstone: Don't enforce use of -pflash for qtest

[Andreas Färber]

Simply skip flash setup for now.

Also drop useless debug output.

Signed-off-by: Andreas Färber 
---
 hw/arm/mainstone.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/arm/mainstone.c b/hw/arm/mainstone.c
index b244f7e..9402c84 100644
--- a/hw/arm/mainstone.c
+++ b/hw/arm/mainstone.c
@@ -21,6 +21,7 @@
 #include "sysemu/blockdev.h"
 #include "hw/sysbus.h"
 #include "exec/address-spaces.h"
+#include "sysemu/qtest.h"
 
 /* Device addresses */
 #define MST_FPGA_PHYS  0x0800
@@ -127,6 +128,9 @@ static void mainstone_common_init(MemoryRegion 
*address_space_mem,
 for (i = 0; i < 2; i ++) {
 dinfo = drive_get(IF_PFLASH, 0, i);
 if (!dinfo) {
+if (qtest_enabled()) {
+break;
+}
 fprintf(stderr, "Two flash images must be given with the "
 "'pflash' parameter\n");
 exit(1);
@@ -147,7 +151,6 @@ static void mainstone_common_init(MemoryRegion 
*address_space_mem,
 qdev_get_gpio_in(mpu->gpio, 0));
 
 /* setup keypad */
-printf("map addr %p\n", &map);
 pxa27x_register_keypad(mpu->kp, map, 0xe0);
 
 /* MMC/SD host */
-- 
1.8.1.4

[Qemu-devel] [PATCH v3 09/18] exynos4_boards: Silence lack of -smp 2 warning for qtest

[Andreas Färber]

Signed-off-by: Andreas Färber 
---
 hw/arm/exynos4_boards.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
index 2929f9f..26cedec 100644
--- a/hw/arm/exynos4_boards.c
+++ b/hw/arm/exynos4_boards.c
@@ -22,6 +22,7 @@
  */
 
 #include "sysemu/sysemu.h"
+#include "sysemu/qtest.h"
 #include "hw/sysbus.h"
 #include "net/net.h"
 #include "hw/arm/arm.h"
@@ -96,7 +97,7 @@ static void lan9215_init(uint32_t base, qemu_irq irq)
 static Exynos4210State *exynos4_boards_init_common(QEMUMachineInitArgs *args,
Exynos4BoardType board_type)
 {
-if (smp_cpus != EXYNOS4210_NCPUS) {
+if (smp_cpus != EXYNOS4210_NCPUS && !qtest_enabled()) {
 fprintf(stderr, "%s board supports only %d CPU cores. Ignoring 
smp_cpus"
 " value.\n",
 exynos4_machines[board_type].name,
-- 
1.8.1.4

[Qemu-devel] [PATCH v3 11/18] axis_dev88: Don't enforce use of kernel for qtest

[Andreas Färber]

Signed-off-by: Andreas Färber 
---
 hw/cris/axis_dev88.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/cris/axis_dev88.c b/hw/cris/axis_dev88.c
index 03058d3..5524088 100644
--- a/hw/cris/axis_dev88.c
+++ b/hw/cris/axis_dev88.c
@@ -32,6 +32,7 @@
 #include "boot.h"
 #include "sysemu/blockdev.h"
 #include "exec/address-spaces.h"
+#include "sysemu/qtest.h"
 
 #define D(x)
 #define DNAND(x)
@@ -340,14 +341,14 @@ void axisdev88_init(QEMUMachineInitArgs *args)
  irq[0x14 + i]);
 }
 
-if (!kernel_filename) {
+if (kernel_filename) {
+li.image_filename = kernel_filename;
+li.cmdline = kernel_cmdline;
+cris_load_image(cpu, &li);
+} else if (!qtest_enabled()) {
 fprintf(stderr, "Kernel image must be specified\n");
 exit(1);
 }
-
-li.image_filename = kernel_filename;
-li.cmdline = kernel_cmdline;
-cris_load_image(cpu, &li);
 }
 
 static QEMUMachine axisdev88_machine = {
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v4 12/23] acpi: add rules to compile ASL source

[Laszlo Ersek]

On 09/23/13 15:39, Michael S. Tsirkin wrote:
> On Mon, Sep 23, 2013 at 02:36:41PM +0200, Paolo Bonzini wrote:
>> Il 22/09/2013 15:37, Michael S. Tsirkin ha scritto:
>>> Detect presence of IASL compiler and use it
>>> to process ASL source. If not there, use pre-compiled
>>> files in-tree. Add script to update the in-tree files.
>>>
>>> Note: distros are known to silently update iasl
>>> so detect correct iasl flags for the installed version on each run as
>>> opposed to at configure time.
>>
>> This is not any different from a C compiler, which is likely updated
>> much much more often than iasl.
> 
> Yes but it's not a theoretical issue:
> we did catch iasl changing flags semantics on the fly, once.
> 
> I think compilers don't do this as a norm :)

(Fully tangentially... gcc and clang emit warnings for a new set of code
constructs every other week. When combined with -Werror, it's super
annoying; valid and intentional code can stop to build unexpectedly.
Good thing we have --disable-werror for this.)

Laszlo

[Qemu-devel] [PATCH v3 18/18] qtest: Prepare QOM machine tests

[Andreas Färber]

Instantiate all [*] machines per target, so that they get a bit of test
coverage at all. This has proven helpful during QOM refactorings.

[*] ppcemb target contains some non-working non-embedded machines, and
ppc405 CPUs are not available there either.
i386 and x86_64 do not cover pc*-x.y or xenfv.

Signed-off-by: Andreas Färber 
---
 tests/Makefile   |  26 ++
 tests/qom-test.c | 253 +++
 2 files changed, 279 insertions(+)
 create mode 100644 tests/qom-test.c

diff --git a/tests/Makefile b/tests/Makefile
index 994fef1..53a5c09 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -67,25 +67,50 @@ check-qtest-i386-y += tests/boot-order-test$(EXESUF)
 check-qtest-i386-y += tests/rtc-test$(EXESUF)
 check-qtest-i386-y += tests/i440fx-test$(EXESUF)
 check-qtest-i386-y += tests/fw_cfg-test$(EXESUF)
+check-qtest-i386-y += tests/qom-test$(EXESUF)
 check-qtest-x86_64-y = $(check-qtest-i386-y)
 gcov-files-i386-y += i386-softmmu/hw/mc146818rtc.c
 gcov-files-x86_64-y = $(subst 
i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y))
 check-qtest-mips-y = tests/endianness-test$(EXESUF)
 check-qtest-mips64-y = tests/endianness-test$(EXESUF)
 check-qtest-mips64el-y = tests/endianness-test$(EXESUF)
+check-qtest-mips-y += tests/qom-test$(EXESUF)
+check-qtest-mipsel-y += tests/qom-test$(EXESUF)
+check-qtest-mips64-y += tests/qom-test$(EXESUF)
+check-qtest-mips64el-y += tests/qom-test$(EXESUF)
 check-qtest-ppc-y = tests/endianness-test$(EXESUF)
 check-qtest-ppc64-y = tests/endianness-test$(EXESUF)
 check-qtest-sh4-y = tests/endianness-test$(EXESUF)
 check-qtest-sh4eb-y = tests/endianness-test$(EXESUF)
+check-qtest-sh4-y += tests/qom-test$(EXESUF)
+check-qtest-sh4eb-y += tests/qom-test$(EXESUF)
 check-qtest-sparc64-y = tests/endianness-test$(EXESUF)
 #check-qtest-sparc-y = tests/m48t59-test$(EXESUF)
 #check-qtest-sparc64-y += tests/m48t59-test$(EXESUF)
 gcov-files-sparc-y += hw/m48t59.c
 gcov-files-sparc64-y += hw/m48t59.c
+check-qtest-sparc-y += tests/qom-test$(EXESUF)
+check-qtest-sparc64-y += tests/qom-test$(EXESUF)
 check-qtest-arm-y = tests/tmp105-test$(EXESUF)
 gcov-files-arm-y += hw/tmp105.c
+check-qtest-arm-y += tests/qom-test$(EXESUF)
 check-qtest-ppc-y += tests/boot-order-test$(EXESUF)
 check-qtest-ppc64-y += tests/boot-order-test$(EXESUF)
+check-qtest-ppc-y += tests/qom-test$(EXESUF)
+check-qtest-ppc64-y += tests/qom-test$(EXESUF)
+check-qtest-ppcemb-y += tests/qom-test$(EXESUF)
+check-qtest-alpha-y += tests/qom-test$(EXESUF)
+check-qtest-cris-y += tests/qom-test$(EXESUF)
+check-qtest-lm32-y += tests/qom-test$(EXESUF)
+check-qtest-m68k-y += tests/qom-test$(EXESUF)
+check-qtest-microblaze-y += tests/qom-test$(EXESUF)
+check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
+check-qtest-moxie-y += tests/qom-test$(EXESUF)
+check-qtest-or32-y += tests/qom-test$(EXESUF)
+check-qtest-s390x-y += tests/qom-test$(EXESUF)
+check-qtest-unicore32-y += tests/qom-test$(EXESUF)
+check-qtest-xtensa-y += tests/qom-test$(EXESUF)
+check-qtest-xtensaeb-y = $(check-qtest-xtensa-y)
 
 check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
 comments.json empty.json funny-char.json indented-expr.json \
@@ -174,6 +199,7 @@ tests/boot-order-test$(EXESUF): tests/boot-order-test.o 
$(libqos-obj-y)
 tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
 tests/i440fx-test$(EXESUF): tests/i440fx-test.o $(libqos-pc-obj-y)
 tests/fw_cfg-test$(EXESUF): tests/fw_cfg-test.o $(libqos-pc-obj-y)
+tests/qom-test$(EXESUF): tests/qom-test.o
 tests/qemu-iotests/socket_scm_helper$(EXESUF): 
tests/qemu-iotests/socket_scm_helper.o
 
 # QTest rules
diff --git a/tests/qom-test.c b/tests/qom-test.c
new file mode 100644
index 000..6ed23c5
--- /dev/null
+++ b/tests/qom-test.c
@@ -0,0 +1,253 @@
+/*
+ * QTest testcase for QOM
+ *
+ * Copyright (c) 2013 SUSE LINUX Products GmbH
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "libqtest.h"
+
+#include 
+#include 
+#include "qemu/osdep.h"
+
+static void test_nop(gconstpointer data)
+{
+QTestState *s;
+const char *machine = data;
+char *args;
+
+args = g_strdup_printf("-display none -machine %s", machine);
+s = qtest_start(args);
+if (s) {
+qtest_quit(s);
+}
+g_free(args);
+}
+
+static const char *x86_machines[] = {
+"pc",
+"isapc",
+"q35",
+};
+
+static const char *alpha_machines[] = {
+"clipper",
+};
+
+static const char *arm_machines[] = {
+"integratorcp",
+"versatilepb",
+"versatileab",
+"lm3s811evb",
+"lm3s6965evb",
+"collie",
+"akita",
+"spitz",
+"borzoi",
+"terrier",
+"tosa",
+"cheetah",
+"sx1-v1",
+"sx1",
+"realview-eb",
+"realview-eb-mpcore",
+"realview-pb-a8",
+"realview-pbx-a9",
+"musicpal",
+"mainstone",
+"connex",
+"verdex",
+"z2",
+"n800",
+"n810",
+"kzm",
+"vexpress-

[Qemu-devel] [PATCH v3 12/18] mcf5208: Don't enforce use of kernel for qtest

[Andreas Färber]

Signed-off-by: Andreas Färber 
---
 hw/m68k/mcf5208.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
index fb96fe8..6e30c0b 100644
--- a/hw/m68k/mcf5208.c
+++ b/hw/m68k/mcf5208.c
@@ -10,6 +10,7 @@
 #include "qemu/timer.h"
 #include "hw/ptimer.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/qtest.h"
 #include "net/net.h"
 #include "hw/boards.h"
 #include "hw/loader.h"
@@ -267,6 +268,9 @@ static void mcf5208evb_init(QEMUMachineInitArgs *args)
 
 /* Load kernel.  */
 if (!kernel_filename) {
+if (qtest_enabled()) {
+return;
+}
 fprintf(stderr, "Kernel image must be specified\n");
 exit(1);
 }
-- 
1.8.1.4

[Qemu-devel] [PATCH v3 01/18] mips_mipssim: Silence BIOS loading warning for qtest

[Andreas Färber]

Reviewed-by: Aurelien Jarno 
Signed-off-by: Andreas Färber 
---
 hw/mips/mips_mipssim.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
index 242bab9..239aa6a 100644
--- a/hw/mips/mips_mipssim.c
+++ b/hw/mips/mips_mipssim.c
@@ -38,6 +38,7 @@
 #include "hw/sysbus.h"
 #include "exec/address-spaces.h"
 #include "qemu/error-report.h"
+#include "sysemu/qtest.h"
 
 static struct _loaderparams {
 int ram_size;
@@ -190,7 +191,8 @@ mips_mipssim_init(QEMUMachineInitArgs *args)
 } else {
 bios_size = -1;
 }
-if ((bios_size < 0 || bios_size > BIOS_SIZE) && !kernel_filename) {
+if ((bios_size < 0 || bios_size > BIOS_SIZE) &&
+!kernel_filename && !qtest_enabled()) {
 /* Bail out if we have neither a kernel image nor boot vector code. */
 error_report("Could not load MIPS bios '%s', and no "
  "-kernel argument was specified", filename);
-- 
1.8.1.4

[Qemu-devel] [PATCH v3 13/18] an5206: Don't enforce use of kernel for qtest

[Andreas Färber]

Signed-off-by: Andreas Färber 
---
 hw/m68k/an5206.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/m68k/an5206.c b/hw/m68k/an5206.c
index a8eee44..24f2068 100644
--- a/hw/m68k/an5206.c
+++ b/hw/m68k/an5206.c
@@ -12,6 +12,7 @@
 #include "hw/loader.h"
 #include "elf.h"
 #include "exec/address-spaces.h"
+#include "sysemu/qtest.h"
 
 #define KERNEL_LOAD_ADDR 0x1
 #define AN5206_MBAR_ADDR 0x1000
@@ -62,6 +63,9 @@ static void an5206_init(QEMUMachineInitArgs *args)
 
 /* Load kernel.  */
 if (!kernel_filename) {
+if (qtest_enabled()) {
+return;
+}
 fprintf(stderr, "Kernel image must be specified\n");
 exit(1);
 }
-- 
1.8.1.4

Re: [Qemu-devel] ChrEMU - Virtualization in the Browser

[Stefan Hajnoczi]

On Tue, Sep 10, 2013 at 08:08:22PM -0400, Joey Carlini wrote:
> I managed to get QEMU running on a Crouton install, virtual box not being
> possible with the Chrome OS kermel with the KVM mods required, and even a
> couple distros running. Since I enjoy pain and/or haven't done enough cool
> things to be called a badass dev, I figured, why not try building QEMU into
> a Chrome app, now that packaged apps are a thing, and native client allows
> for C code to run within the browser, letting an entire VM run on a stock
> Chromebook.

QEMU isn't pure C code and effort would be required to make it run under
Native Client.

I've never used Native Client but I think its machine code verifier
checks the application to ensure that control flow is safe.  In other
words, low-level things that QEMU does like code generation or stack
switching are probably not allowed under Native Client since they are
unsafe!

Maybe I'm wrong and it's possible, but the first thing to check is the
constraints that Native Client puts on the application code.

Stefan

Re: [Qemu-devel] [PATCH v4 15/23] i386: add bios linker/loader

[Paolo Bonzini]

Il 23/09/2013 15:36, Michael S. Tsirkin ha scritto:
> On Mon, Sep 23, 2013 at 02:48:57PM +0200, Paolo Bonzini wrote:
>> Il 22/09/2013 15:37, Michael S. Tsirkin ha scritto:
>>> This adds a dynamic bios linker/loader.
>>> This will be used by acpi table generation
>>> code to:
>>> - load each table in the appropriate memory segment
>>> - link tables to each other
>>> - fix up checksums after said linking
>>>
>>> Signed-off-by: Michael S. Tsirkin 
>>> ---
>>>  include/hw/i386/bios-linker-loader.h |  26 ++
>>>  hw/i386/bios-linker-loader.c | 156 
>>> +++
>>>  hw/i386/Makefile.objs|   1 +
>>
>> Just a naming question...
>>
>> If this is ACPI-specific (or if there are no other uses in sight), it
>> should go in include/hw/acpi and hw/acpi, and renamed with s/bios/acpi/ etc.
>>
>> If this is i386-specific, it should go entirely in hw/i386 and the
>> headers should be included as "bios-linker-loader.h".
> 
> Definitely not ACPI specific.
> It's only used on i386 so I put it all under i386.
> You think we should replace
> #include "hw/i386/bios-linker-loader.h" with
> #include "bios-linker-loader.h"?
> 
> I don't really care, even though this makes headers
> hard to find. What's the benefit?

Private headers are outside include/, public headers within it.  Same
conventions as Linux.  But if it's not ACPI-specific, I don't feel too
strongly about it.

> It's not easy to find a good name for it.
> For now I'll just make all names bios_linker_loader_ consistently.
> We can always rename it later, it's not set in stone.

Ok.

>>> +struct BiosLinkerLoaderEntry {
>>> +uint32_t command;
>>> +union {
>>> +/*
>>> + * COMMAND_ALLOCATE - allocate a table from @alloc_file
>>> + * subject to @alloc_align alignment (must be power of 2)
>>> + * and @alloc_zone (can be HIGH or FSEG) requirements.
>>> + *
>>> + * Must appear exactly once for each file, and before
>>> + * this file is referenced by any other command.
>>> + */
>>> +struct {
>>> +char alloc_file[BIOS_LINKER_LOADER_FILESZ];
>>> +uint32_t alloc_align;
>>> +uint8_t alloc_zone;
>>> +};
>>
>> I would prefer to have named structs, like
>>
>> struct {
>> char file[BIOS_LINKER_LOADER_FILESZ];
>> uint32_t align;
>> uint8_t zone;
>> } alloc;
> 
> Why do you prefer this? It just makes code more verbose.

It doesn't, the names were already prefixed with alloc_/pointer_/cksum_.
 All it does is change underscores to periods.

Paolo

>>> +/*
>>> + * COMMAND_ADD_POINTER - patch the table (originating from
>>> + * @dest_file) at @pointer_offset, by adding a pointer to the table
>>> + * originating from @src_file. 1,2,4 or 8 byte unsigned
>>> + * addition is used depending on @pointer_size.
>>> + */
>>> +struct {
>>> +char pointer_dest_file[BIOS_LINKER_LOADER_FILESZ];
>>> +char pointer_src_file[BIOS_LINKER_LOADER_FILESZ];
>>> +uint32_t pointer_offset;
>>> +uint8_t pointer_size;
>>> +};
>>> +
>>> +/*
>>> + * COMMAND_ADD_CHECKSUM - calculate checksum of the range 
>>> specified by
>>> + * @cksum_start and @cksum_length fields,
>>> + * and then add the value at @cksum_offset.
>>> + * Checksum simply sums -X for each byte X in the range
>>> + * using 8-bit math.
>>> + */
>>> +struct {
>>> +char cksum_file[BIOS_LINKER_LOADER_FILESZ];
>>> +uint32_t cksum_offset;
>>> +uint32_t cksum_start;
>>> +uint32_t cksum_length;
>>> +};
>>> +
>>> +/* padding */
>>> +char pad[124];
>>> +};
>>> +} QEMU_PACKED;
>>> +typedef struct BiosLinkerLoaderEntry BiosLinkerLoaderEntry;
>>> +
>>> +enum {
>>> +BIOS_LINKER_LOADER_COMMAND_ALLOCATE = 0x1,
>>> +BIOS_LINKER_LOADER_COMMAND_ADD_POINTER  = 0x2,
>>> +BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM = 0x3,
>>> +};
>>> +
>>> +enum {
>>> +BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH = 0x1,
>>> +BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG = 0x2,
>>> +};
>>> +
>>> +GArray *bios_linker_init(void)
>>> +{
>>> +return g_array_new(false, true /* clear */, 
>>> sizeof(BiosLinkerLoaderEntry));
>>> +}
>>> +
>>> +/* Free linker wrapper and return the linker array. */
>>> +void *bios_linker_cleanup(GArray *linker)
>>> +{
>>> +return g_array_free(linker, false);
>>> +}
>>> +
>>> +void bios_linker_alloc(GArray *linker,
>>> +   const char *file,
>>> +   uint32_t alloc_align,
>>> +   bool alloc_fseg)
>>> +{
>>> +BiosLinkerLoaderEntry entry;
>>> +
>>> +memset(&entry, 0, sizeof entry);
>>> +strncpy(entry.alloc_file, file, sizeof entry.alloc_file - 1);
>>
>> Please use pstrcpy (also below).
> 
> OK.
> 
>>> +entry.c

Re: [Qemu-devel] [PATCH v4 15/23] i386: add bios linker/loader

[Michael S. Tsirkin]

On Mon, Sep 23, 2013 at 03:39:10PM +0200, Paolo Bonzini wrote:
> Il 23/09/2013 15:36, Michael S. Tsirkin ha scritto:
> > On Mon, Sep 23, 2013 at 02:48:57PM +0200, Paolo Bonzini wrote:
> >> Il 22/09/2013 15:37, Michael S. Tsirkin ha scritto:
> >>> This adds a dynamic bios linker/loader.
> >>> This will be used by acpi table generation
> >>> code to:
> >>> - load each table in the appropriate memory segment
> >>> - link tables to each other
> >>> - fix up checksums after said linking
> >>>
> >>> Signed-off-by: Michael S. Tsirkin 
> >>> ---
> >>>  include/hw/i386/bios-linker-loader.h |  26 ++
> >>>  hw/i386/bios-linker-loader.c | 156 
> >>> +++
> >>>  hw/i386/Makefile.objs|   1 +
> >>
> >> Just a naming question...
> >>
> >> If this is ACPI-specific (or if there are no other uses in sight), it
> >> should go in include/hw/acpi and hw/acpi, and renamed with s/bios/acpi/ 
> >> etc.
> >>
> >> If this is i386-specific, it should go entirely in hw/i386 and the
> >> headers should be included as "bios-linker-loader.h".
> > 
> > Definitely not ACPI specific.
> > It's only used on i386 so I put it all under i386.
> > You think we should replace
> > #include "hw/i386/bios-linker-loader.h" with
> > #include "bios-linker-loader.h"?
> > 
> > I don't really care, even though this makes headers
> > hard to find. What's the benefit?
> 
> Private headers are outside include/, public headers within it.

Ah you mean move it outside include/. OK.

>  Same
> conventions as Linux.  But if it's not ACPI-specific, I don't feel too
> strongly about it.
> 
> > It's not easy to find a good name for it.
> > For now I'll just make all names bios_linker_loader_ consistently.
> > We can always rename it later, it's not set in stone.
> 
> Ok.
> 
> >>> +struct BiosLinkerLoaderEntry {
> >>> +uint32_t command;
> >>> +union {
> >>> +/*
> >>> + * COMMAND_ALLOCATE - allocate a table from @alloc_file
> >>> + * subject to @alloc_align alignment (must be power of 2)
> >>> + * and @alloc_zone (can be HIGH or FSEG) requirements.
> >>> + *
> >>> + * Must appear exactly once for each file, and before
> >>> + * this file is referenced by any other command.
> >>> + */
> >>> +struct {
> >>> +char alloc_file[BIOS_LINKER_LOADER_FILESZ];
> >>> +uint32_t alloc_align;
> >>> +uint8_t alloc_zone;
> >>> +};
> >>
> >> I would prefer to have named structs, like
> >>
> >> struct {
> >> char file[BIOS_LINKER_LOADER_FILESZ];
> >> uint32_t align;
> >> uint8_t zone;
> >> } alloc;
> > 
> > Why do you prefer this? It just makes code more verbose.
> 
> It doesn't, the names were already prefixed with alloc_/pointer_/cksum_.
>  All it does is change underscores to periods.
> 
> Paolo

You are right, that's cleaner. I'll do this.
I don't want to sprinkle QEMU_PACKED around though,
it's just there for documentation purposes, so one
place is enough.

> >>> +/*
> >>> + * COMMAND_ADD_POINTER - patch the table (originating from
> >>> + * @dest_file) at @pointer_offset, by adding a pointer to the 
> >>> table
> >>> + * originating from @src_file. 1,2,4 or 8 byte unsigned
> >>> + * addition is used depending on @pointer_size.
> >>> + */
> >>> +struct {
> >>> +char pointer_dest_file[BIOS_LINKER_LOADER_FILESZ];
> >>> +char pointer_src_file[BIOS_LINKER_LOADER_FILESZ];
> >>> +uint32_t pointer_offset;
> >>> +uint8_t pointer_size;
> >>> +};
> >>> +
> >>> +/*
> >>> + * COMMAND_ADD_CHECKSUM - calculate checksum of the range 
> >>> specified by
> >>> + * @cksum_start and @cksum_length fields,
> >>> + * and then add the value at @cksum_offset.
> >>> + * Checksum simply sums -X for each byte X in the range
> >>> + * using 8-bit math.
> >>> + */
> >>> +struct {
> >>> +char cksum_file[BIOS_LINKER_LOADER_FILESZ];
> >>> +uint32_t cksum_offset;
> >>> +uint32_t cksum_start;
> >>> +uint32_t cksum_length;
> >>> +};
> >>> +
> >>> +/* padding */
> >>> +char pad[124];
> >>> +};
> >>> +} QEMU_PACKED;
> >>> +typedef struct BiosLinkerLoaderEntry BiosLinkerLoaderEntry;
> >>> +
> >>> +enum {
> >>> +BIOS_LINKER_LOADER_COMMAND_ALLOCATE = 0x1,
> >>> +BIOS_LINKER_LOADER_COMMAND_ADD_POINTER  = 0x2,
> >>> +BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM = 0x3,
> >>> +};
> >>> +
> >>> +enum {
> >>> +BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH = 0x1,
> >>> +BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG = 0x2,
> >>> +};
> >>> +
> >>> +GArray *bios_linker_init(void)
> >>> +{
> >>> +return g_array_new(false, true /* clear */, 
> >>> sizeof(BiosLinkerLoaderEntry));
> >>> +}
> >>> +
> >>> +/* Free linker wrapper and return the lin

Re: [Qemu-devel] [PATCH v4 12/23] acpi: add rules to compile ASL source

[Michael S. Tsirkin]

On Mon, Sep 23, 2013 at 02:36:41PM +0200, Paolo Bonzini wrote:
> Il 22/09/2013 15:37, Michael S. Tsirkin ha scritto:
> > Detect presence of IASL compiler and use it
> > to process ASL source. If not there, use pre-compiled
> > files in-tree. Add script to update the in-tree files.
> > 
> > Note: distros are known to silently update iasl
> > so detect correct iasl flags for the installed version on each run as
> > opposed to at configure time.
> 
> This is not any different from a C compiler, which is likely updated
> much much more often than iasl.

Yes but it's not a theoretical issue:
we did catch iasl changing flags semantics on the fly, once.

I think compilers don't do this as a norm :)

> However, I don't feel strongly at all
> about this.
> 
> Paolo

Re: [Qemu-devel] [PATCH v4 15/23] i386: add bios linker/loader

[Paolo Bonzini]

Il 23/09/2013 15:47, Michael S. Tsirkin ha scritto:
> > It doesn't, the names were already prefixed with alloc_/pointer_/cksum_.
> >  All it does is change underscores to periods.
> 
> You are right, that's cleaner. I'll do this.
> I don't want to sprinkle QEMU_PACKED around though,
> it's just there for documentation purposes, so one
> place is enough.

Fair enough.  Thanks!

Paolo

Re: [Qemu-devel] [PATCH] Extend qemu-ga's 'guest-info' command to expose flag 'success-response'

[Eric Blake]

On 09/22/2013 12:50 AM, Mark Wu wrote:
> Now we have several qemu-ga commands not returning response on success.
> It has been documented in qga/qapi-schema.json already. This patch exposes
> the 'success-response' flag by extending 'guest-info' command. With this
> change, the clients can handle the command response more flexibly.
> 
> Changes:
>   v2: add the notation 'since 1.7' to the option 'success-response'
> (per Eric Blake's comments)

Typically, the patch changelog should be placed...

> 
> Signed-off-by: Mark Wu 
> ---

...after the --- separator (see
http://wiki.qemu.org/Contribute/SubmitAPatch for more hints).


> +++ b/qga/qapi-schema.json
> @@ -141,10 +141,13 @@
>  #
>  # @enabled: whether command is currently enabled by guest admin
>  #
> +# @success-response: whether command returns a response on success
> +#and only available since 1.7

More typically, this is written:

@success-response: whether command returns a response on
   success (since 1.7)

but as we don't parse the documentation, it doesn't hurt as written.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH 02/10] target-s390: Implement STFLE

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 target-s390x/helper.h  |  1 +
 target-s390x/insn-data.def |  2 ++
 target-s390x/misc_helper.c | 13 +
 target-s390x/translate.c   |  8 
 4 files changed, 24 insertions(+)

diff --git a/target-s390x/helper.h b/target-s390x/helper.h
index 0d80aa0..547b46a 100644
--- a/target-s390x/helper.h
+++ b/target-s390x/helper.h
@@ -85,6 +85,7 @@ DEF_HELPER_FLAGS_5(calc_cc, TCG_CALL_NO_RWG_SE, i32, env, 
i32, i64, i64, i64)
 DEF_HELPER_FLAGS_2(sfpc, TCG_CALL_NO_RWG, void, env, i64)
 DEF_HELPER_FLAGS_2(sfas, TCG_CALL_NO_WG, void, env, i64)
 DEF_HELPER_FLAGS_1(popcnt, TCG_CALL_NO_RWG_SE, i64, i64)
+DEF_HELPER_2(stfle, i32, env, i64)
 
 #ifndef CONFIG_USER_ONLY
 DEF_HELPER_3(servc, i32, env, i64, i64)
diff --git a/target-s390x/insn-data.def b/target-s390x/insn-data.def
index b42ebb6..4b462d4 100644
--- a/target-s390x/insn-data.def
+++ b/target-s390x/insn-data.def
@@ -639,6 +639,8 @@
 C(0xe33e, STRV,RXY_a, Z,   la2, r1_32u, new, m1_32, rev32, 0)
 C(0xe32f, STRVG,   RXY_a, Z,   la2, r1_o, new, m1_64, rev64, 0)
 
+/* STORE FACILITY LIST EXTENDED */
+C(0xb2b0, STFLE,   S,  SFLE,   0, a2, 0, 0, stfle, 0)
 /* STORE FPC */
 C(0xb29c, STFPC,   S, Z,   0, a2, new, m2_32, efpc, 0)
 
diff --git a/target-s390x/misc_helper.c b/target-s390x/misc_helper.c
index 1690907..b6ee429 100644
--- a/target-s390x/misc_helper.c
+++ b/target-s390x/misc_helper.c
@@ -518,3 +518,16 @@ uint32_t HELPER(sigp)(CPUS390XState *env, uint64_t 
order_code, uint32_t r1,
 return cc;
 }
 #endif
+
+uint32_t HELPER(stfle)(CPUS390XState *env, uint64_t addr)
+{
+int max_m1 = (env->facilities[1] != 0);
+int count_m1 = env->regs[0] & 0xff;
+
+cpu_stq_data(env, addr, env->facilities[0]);
+if (count_m1 > 0) {
+cpu_stq_data(env, addr + 8, env->facilities[1]);
+}
+env->regs[0] = max_m1;
+return (count_m1 >= max_m1 ? 0 : 3);
+}
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index d4dc8ea..690b4a0 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -3314,6 +3314,14 @@ static ExitStatus op_stura(DisasContext *s, DisasOps *o)
 }
 #endif
 
+static ExitStatus op_stfle(DisasContext *s, DisasOps *o)
+{
+potential_page_fault(s);
+gen_helper_stfle(cc_op, cpu_env, o->in2);
+set_cc_static(s);
+return NO_EXIT;
+}
+
 static ExitStatus op_st8(DisasContext *s, DisasOps *o)
 {
 tcg_gen_qemu_st8(o->in1, o->in2, get_mem_index(s));
-- 
1.8.1.4

[Qemu-devel] [PATCH 01/10] target-s390: Move facilities bits to env

[Richard Henderson]

Rather than simply hard-coding them in STFL instruction.

Signed-off-by: Richard Henderson 
---
 target-s390x/cpu.c   |  3 +++
 target-s390x/cpu.h   |  1 +
 target-s390x/translate.c | 10 +-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 3c89f8a..ff691df 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -181,6 +181,9 @@ static void s390_cpu_initfn(Object *obj)
 env->cpu_num = cpu_num++;
 env->ext_index = -1;
 
+env->facilities[0] = 0xc000ull;
+env->facilities[1] = 0;
+
 if (tcg_enabled() && !inited) {
 inited = true;
 s390x_translate_init();
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 8be5648..746aec8 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -136,6 +136,7 @@ typedef struct CPUS390XState {
 CPU_COMMON
 
 /* reset does memset(0) up to here */
+uint64_t facilities[2];
 
 int cpu_num;
 uint8_t *storage_keys;
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index afe90eb..d4dc8ea 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -3230,12 +3230,12 @@ static ExitStatus op_spt(DisasContext *s, DisasOps *o)
 
 static ExitStatus op_stfl(DisasContext *s, DisasOps *o)
 {
-TCGv_i64 f, a;
-/* We really ought to have more complete indication of facilities
-   that we implement.  Address this when STFLE is implemented.  */
+TCGv_i64 f = tcg_temp_new_i64();
+TCGv_i64 a = tcg_const_i64(200);
+
 check_privileged(s);
-f = tcg_const_i64(0xc000);
-a = tcg_const_i64(200);
+tcg_gen_ld_i64(f, cpu_env, offsetof(CPUS390XState, facilities[0]));
+tcg_gen_shri_i64(f, f, 32);
 tcg_gen_qemu_st32(f, a, get_mem_index(s));
 tcg_temp_free_i64(f);
 tcg_temp_free_i64(a);
-- 
1.8.1.4

[Qemu-devel] [PATCH 0/9] target-s390 tcg improvements

[Richard Henderson]

With this patch set we can boot the fedora 19 kernel, and make
it all the way to /bin/init.  At which point the process either
hangs or crashes; in either case the kernel winds up with no
runnable processes and spends its time in the idle loop.

The choice of z9-109 for the facilities is because that appears
to be what fedora 19 is targeting as the minimum.

That said, a debian install can make it all the way through to
completion, so the fedora crash/hang must be related to something
in the extra z9-109 insns.



r~


Richard Henderson (10):
  target-s390: Move facilities bits to env
  target-s390: Implement STFLE
  target-s390: Add facilities bits and sets
  target-s390: Raise OPERATION exception for disabled insns
  target-s390: Implement SAM31 and SAM64
  target-s390: Implement EPSW
  target-s390: Fix STIDP
  target-s390: Fix STURA
  target-s390: Implement LURA, LURAG, STURG
  target-s390: Implement ECAG

 target-s390x/cpu.c |  78 ++
 target-s390x/cpu.h |  74 -
 target-s390x/helper.h  |   4 ++
 target-s390x/insn-data.def |  18 +++--
 target-s390x/mem_helper.c  |  18 -
 target-s390x/misc_helper.c |  13 
 target-s390x/translate.c   | 161 -
 7 files changed, 329 insertions(+), 37 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH 06/10] target-s390: Implement EPSW

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 target-s390x/insn-data.def |  2 ++
 target-s390x/translate.c   | 18 ++
 2 files changed, 20 insertions(+)

diff --git a/target-s390x/insn-data.def b/target-s390x/insn-data.def
index c528eb4..48850ff 100644
--- a/target-s390x/insn-data.def
+++ b/target-s390x/insn-data.def
@@ -287,6 +287,8 @@
 C(0xb24f, EAR, RRE,   Z,   0, 0, new, r1_32, ear, 0)
 /* EXTRACT FPC */
 C(0xb38c, EFPC,RRE,   Z,   0, 0, new, r1_32, efpc, 0)
+/* EXTRACT PSW */
+C(0xb98d, EPSW,RRE,   Z,   0, 0, 0, 0, epsw, 0)
 
 /* FIND LEFTMOST ONE */
 C(0xb983, FLOGR,   RRE,   EI,  0, r2_o, r1_P, 0, flogr, 0)
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 6ca8f0b..192d54e 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -2036,6 +2036,24 @@ static ExitStatus op_efpc(DisasContext *s, DisasOps *o)
 return NO_EXIT;
 }
 
+static ExitStatus op_epsw(DisasContext *s, DisasOps *o)
+{
+int r1 = get_field(s->fields, r1);
+int r2 = get_field(s->fields, r2);
+TCGv_i64 t = tcg_temp_new_i64();
+
+/* Note the "subsequently" in the PoO, which implies a defined result
+   if r1 == r2.  Thus we cannot defer these writes to an output hook.  */
+tcg_gen_shri_i64(t, psw_mask, 32);
+store_reg32_i64(r1, t);
+if (r2 != 0) {
+store_reg32_i64(r2, psw_mask);
+}
+
+tcg_temp_free_i64(t);
+return NO_EXIT;
+}
+
 static ExitStatus op_ex(DisasContext *s, DisasOps *o)
 {
 /* ??? Perhaps a better way to implement EXECUTE is to set a bit in
-- 
1.8.1.4

[Qemu-devel] [PATCH 10/10] target-s390: Implement ECAG

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 target-s390x/insn-data.def | 2 ++
 target-s390x/translate.c   | 7 +++
 2 files changed, 9 insertions(+)

diff --git a/target-s390x/insn-data.def b/target-s390x/insn-data.def
index a405f64..d3bc5b1 100644
--- a/target-s390x/insn-data.def
+++ b/target-s390x/insn-data.def
@@ -285,6 +285,8 @@
 
 /* EXTRACT ACCESS */
 C(0xb24f, EAR, RRE,   Z,   0, 0, new, r1_32, ear, 0)
+/* EXTRACT CPU ATTRIBUTE */
+C(0xeb4c, ECAG,RSY_a, GIE, 0, a2, r1, 0, ecag, 0)
 /* EXTRACT FPC */
 C(0xb38c, EFPC,RRE,   Z,   0, 0, new, r1_32, efpc, 0)
 /* EXTRACT PSW */
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index e39305d..119f700 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -2030,6 +2030,13 @@ static ExitStatus op_ear(DisasContext *s, DisasOps *o)
 return NO_EXIT;
 }
 
+static ExitStatus op_ecag(DisasContext *s, DisasOps *o)
+{
+/* No cache information provided.  */
+tcg_gen_movi_i64(o->out, -1);
+return NO_EXIT;
+}
+
 static ExitStatus op_efpc(DisasContext *s, DisasOps *o)
 {
 tcg_gen_ld32u_i64(o->out, cpu_env, offsetof(CPUS390XState, fpc));
-- 
1.8.1.4

[Qemu-devel] [PATCH 03/10] target-s390: Add facilities bits and sets

[Richard Henderson]

Name the facilities bits, collect the set of bits for tcg and the various
real processor revisions.  Update the set of facilities reported for TCG.

Signed-off-by: Richard Henderson 
---
 target-s390x/cpu.c   | 75 +++-
 target-s390x/cpu.h   | 59 +
 target-s390x/translate.c | 49 ---
 3 files changed, 157 insertions(+), 26 deletions(-)

diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index ff691df..01ff49b 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -152,6 +152,70 @@ static void s390_cpu_realizefn(DeviceState *dev, Error 
**errp)
 scc->parent_realize(dev, errp);
 }
 
+#define FAC0_TCG \
+( FAC_BIT(0, FAC_N3) \
+| FAC_BIT(0, FAC_ZARCH) \
+| FAC_BIT(0, FAC_ZARCH_ACTIVE) \
+| FAC_BIT(0, FAC_STFLE) \
+| FAC_BIT(0, FAC_LONG_DISPLACEMENT) \
+| FAC_BIT(0, FAC_LONG_DISPLACEMENT_FAST) \
+| FAC_BIT(0, FAC_EXTENDED_IMMEDIATE) \
+| FAC_BIT(0, FAC_GENERAL_INSTRUCTIONS_EXT) \
+| FAC_BIT(0, FAC_FLOATING_POINT_EXT) \
+| FAC_BIT(0, FAC_FLOATING_POINT_SUPPPORT_ENH))
+/* ??? We may have most of the collection of facilities at bit 45.  */
+
+/* ??? These lists of facilities gleaned from arch/s390/kernel/head.S.  */
+#define FAC0_Z900 \
+( FAC_BIT(0, FAC_N3) \
+| FAC_BIT(0, FAC_ZARCH) \
+| FAC_BIT(0, FAC_ZARCH_ACTIVE))
+
+#define FAC0_Z990 \
+( FAC0_Z900 \
+| FAC_BIT(0, FAC_LONG_DISPLACEMENT) \
+| FAC_BIT(0, FAC_LONG_DISPLACEMENT_FAST))
+
+#define FAC0_Z9_109 \
+( FAC0_Z990 \
+| FAC_BIT(0, FAC_STFLE) \
+| FAC_BIT(0, FAC_EXTENDED_TRANSLATION_2) \
+| FAC_BIT(0, FAC_MESSAGE_SECURITY_ASSIST) \
+| FAC_BIT(0, FAC_LONG_DISPLACEMENT) \
+| FAC_BIT(0, FAC_LONG_DISPLACEMENT_FAST) \
+| FAC_BIT(0, FAC_HFP_MADDSUB) \
+| FAC_BIT(0, FAC_EXTENDED_IMMEDIATE) \
+| FAC_BIT(0, FAC_EXTENDED_TRANSLATION_3) \
+| FAC_BIT(0, FAC_HFP_UNNORMALIZED_EXT) \
+| FAC_BIT(0, FAC_ETF2_ENH) \
+| FAC_BIT(0, FAC_STORE_CLOCK_FAST) \
+| FAC_BIT(0, FAC_ETF3_ENH) \
+| FAC_BIT(0, FAC_EXTRACT_CPU_TIME))
+
+#define FAC0_Z10 \
+( FAC0_Z9_109 \
+| FAC_BIT(0, FAC_COMPARE_AND_SWAP_AND_STORE) \
+| FAC_BIT(0, FAC_COMPARE_AND_SWAP_AND_STORE_2) \
+| FAC_BIT(0, FAC_GENERAL_INSTRUCTIONS_EXT) \
+| FAC_BIT(0, FAC_EXECUTE_EXT) \
+| FAC_BIT(0, FAC_FLOATING_POINT_SUPPPORT_ENH) \
+| FAC_BIT(0, FAC_DFP) \
+| FAC_BIT(0, FAC_PFPO))
+
+#define FAC0_Z196 \
+( FAC0_Z10 \
+| FAC_BIT(0, FAC_FLOATING_POINT_EXT) \
+| FAC_BIT(0, FAC_MULTI_45))
+
+#define FAC0_ZEC12 \
+( FAC0_Z196 \
+| FAC_BIT(0, FAC_DFP_ZONED_CONVERSION) \
+| FAC_BIT(0, FAC_MULTI_49) \
+| FAC_BIT(0, FAC_CONSTRAINT_TRANSACTIONAL_EXE))
+#define FAC1_ZEC12 \
+FAC_BIT(1, FAC_TRANSACTIONAL_EXE)
+
+
 static void s390_cpu_initfn(Object *obj)
 {
 CPUState *cs = CPU(obj);
@@ -181,9 +245,18 @@ static void s390_cpu_initfn(Object *obj)
 env->cpu_num = cpu_num++;
 env->ext_index = -1;
 
-env->facilities[0] = 0xc000ull;
+env->facilities[0] = FAC0_TCG;
 env->facilities[1] = 0;
 
+/* ??? Current distros are targeting Z9-109 as the minimum.  TCG
+   supports most of the Z9-109 facilities but not all.  Sadly, the
+   kernel checks for facilities it doesn't actually need, minor stuff
+   like hex floating point and translation.  For now, include all
+   that the kernel requires we support.  */
+#ifndef CONFIG_USER_ONLY
+env->facilities[0] |= FAC0_Z9_109;
+#endif
+
 if (tcg_enabled() && !inited) {
 inited = true;
 s390x_translate_init();
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 746aec8..a0bafef 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -83,6 +83,65 @@ typedef struct MchkQueue {
 #define KVM_S390_RUNTIME_DIRTY_PARTIAL  1
 #define KVM_S390_RUNTIME_DIRTY_FULL 2
 
+typedef enum {
+FAC_N3  = 0,
+FAC_ZARCH   = 1,
+FAC_ZARCH_ACTIVE= 2,
+FAC_DAT_ENH = 3,
+FAC_ASN_LX_REUSE= 6,
+FAC_STFLE   = 7,
+FAC_ENHANCED_DAT_1  = 8,
+FAC_SENSE_RUNNING_STATUS= 9,
+FAC_CONDITIONAL_SSKE= 10,
+FAC_CONFIGURATION_TOPOLOGY  = 11,
+FAC_IPTE_RANGE  = 13,
+FAC_NONQ_KEY_SETTING= 14,
+FAC_EXTENDED_TRANSLATION_2  = 16,
+FAC_MESSAGE_SECURITY_ASSIST = 17,
+FAC_LONG_DISPLACEMENT   = 18,
+FAC_LONG_DISPLACEMENT_FAST  = 19,
+FAC_HFP_MADDSUB = 20,
+FAC_EXTENDED_IMMEDIATE  = 21,
+FAC_EXTENDED_TRANSLATION_3  = 22,
+FAC_HFP_UNNORMALIZED_EXT= 23,
+FAC_ETF2_ENH= 24,
+FAC_STORE_CLOCK_FAST= 25,
+FAC_PARSING_ENH   

[Qemu-devel] [PATCH 09/10] target-s390: Implement LURA, LURAG, STURG

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 target-s390x/helper.h  |  3 +++
 target-s390x/insn-data.def |  4 
 target-s390x/mem_helper.c  | 16 
 target-s390x/translate.c   | 26 ++
 4 files changed, 49 insertions(+)

diff --git a/target-s390x/helper.h b/target-s390x/helper.h
index 547b46a..20047d2 100644
--- a/target-s390x/helper.h
+++ b/target-s390x/helper.h
@@ -114,7 +114,10 @@ DEF_HELPER_FLAGS_2(sacf, TCG_CALL_NO_WG, void, env, i64)
 DEF_HELPER_FLAGS_3(ipte, TCG_CALL_NO_RWG, void, env, i64, i64)
 DEF_HELPER_FLAGS_1(ptlb, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_2(lra, i64, env, i64)
+DEF_HELPER_FLAGS_2(lura, TCG_CALL_NO_WG, i64, env, i64)
+DEF_HELPER_FLAGS_2(lurag, TCG_CALL_NO_WG, i64, env, i64)
 DEF_HELPER_FLAGS_3(stura, TCG_CALL_NO_WG, void, env, i64, i64)
+DEF_HELPER_FLAGS_3(sturg, TCG_CALL_NO_WG, void, env, i64, i64)
 #endif
 
 #include "exec/def-helper.h"
diff --git a/target-s390x/insn-data.def b/target-s390x/insn-data.def
index 48850ff..a405f64 100644
--- a/target-s390x/insn-data.def
+++ b/target-s390x/insn-data.def
@@ -741,6 +741,9 @@
 C(0xb100, LRA, RX_a,  Z,   0, a2, r1, 0, lra, 0)
 C(0xe313, LRAY,RXY_a, LD,  0, a2, r1, 0, lra, 0)
 C(0xe303, LRAG,RXY_a, Z,   0, a2, r1, 0, lra, 0)
+/* LOAD USING REAL ADDRESS */
+C(0xb24b, LURA,RRE,   Z,   0, r2, new, r1_32, lura, 0)
+C(0xb905, LURAG,   RRE,   Z,   0, r2, r1, 0, lurag, 0)
 /* MOVE TO PRIMARY */
 C(0xda00, MVCP,SS_d,  Z,   la1, a2, 0, 0, mvcp, 0)
 /* MOVE TO SECONDARY */
@@ -798,6 +801,7 @@
 C(0xad00, STOSM,   SI,Z,   la1, 0, 0, 0, stnosm, 0)
 /* STORE USING REAL ADDRESS */
 C(0xb246, STURA,   RRE,   Z,   r1_o, r2_o, 0, 0, stura, 0)
+C(0xb925, STURG,   RRE,   Z,   r1_o, r2_o, 0, 0, sturg, 0)
 /* TEST PROTECTION */
 C(0xe501, TPROT,   SSE,   Z,   la1, a2, 0, 0, tprot, 0)
 
diff --git a/target-s390x/mem_helper.c b/target-s390x/mem_helper.c
index 408836c..9bd5e10 100644
--- a/target-s390x/mem_helper.c
+++ b/target-s390x/mem_helper.c
@@ -1038,12 +1038,28 @@ void HELPER(ptlb)(CPUS390XState *env)
 tlb_flush(env, 1);
 }
 
+/* load using real address */
+uint64_t HELPER(lura)(CPUS390XState *env, uint64_t addr)
+{
+return (uint32_t)ldl_phys(get_address(env, 0, 0, addr));
+}
+
+uint64_t HELPER(lurag)(CPUS390XState *env, uint64_t addr)
+{
+return ldq_phys(get_address(env, 0, 0, addr));
+}
+
 /* store using real address */
 void HELPER(stura)(CPUS390XState *env, uint64_t addr, uint64_t v1)
 {
 stl_phys(get_address(env, 0, 0, addr), (uint32_t)v1);
 }
 
+void HELPER(sturg)(CPUS390XState *env, uint64_t addr, uint64_t v1)
+{
+stq_phys(get_address(env, 0, 0, addr), v1);
+}
+
 /* load real address */
 uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
 {
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 25a6537..e39305d 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -2463,6 +2463,24 @@ static ExitStatus op_lm64(DisasContext *s, DisasOps *o)
 return NO_EXIT;
 }
 
+#ifndef CONFIG_USER_ONLY
+static ExitStatus op_lura(DisasContext *s, DisasOps *o)
+{
+check_privileged(s);
+potential_page_fault(s);
+gen_helper_lura(o->out, cpu_env, o->in2);
+return NO_EXIT;
+}
+
+static ExitStatus op_lurag(DisasContext *s, DisasOps *o)
+{
+check_privileged(s);
+potential_page_fault(s);
+gen_helper_lurag(o->out, cpu_env, o->in2);
+return NO_EXIT;
+}
+#endif
+
 static ExitStatus op_mov2(DisasContext *s, DisasOps *o)
 {
 o->out = o->in2;
@@ -3337,6 +3355,14 @@ static ExitStatus op_stura(DisasContext *s, DisasOps *o)
 gen_helper_stura(cpu_env, o->in2, o->in1);
 return NO_EXIT;
 }
+
+static ExitStatus op_sturg(DisasContext *s, DisasOps *o)
+{
+check_privileged(s);
+potential_page_fault(s);
+gen_helper_sturg(cpu_env, o->in2, o->in1);
+return NO_EXIT;
+}
 #endif
 
 static ExitStatus op_stfle(DisasContext *s, DisasOps *o)
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH] hw/pci: completed master-abort emulation

[Michael S. Tsirkin]

On Mon, Sep 23, 2013 at 03:37:43PM +0300, Marcel Apfelbaum wrote:
> On Mon, 2013-09-23 at 14:27 +0300, Michael S. Tsirkin wrote:
> > On Mon, Sep 23, 2013 at 02:01:17PM +0300, Marcel Apfelbaum wrote:
> > > This patch is implemented on top of series:
> > > [PATCH v5 0/3] pci: implement upstream master abort protocol
> > > 
> > > Added "master abort io" background region for PCIBus.
> > > 
> > > Added "master abort mem" region to catch transactions initiated
> > > by pci devices targeted to unassigned addresses.
> > > 
> > > Enabled "master abort" regions for PCI-2-PCI bridge's secondary bus.
> > > 
> > > Set "Received Master Abort" Bit on Status/Secondary Status
> > > register as defined in the PCI Spec.
> > > 
> > > Signed-off-by: Marcel Apfelbaum 
> > 
> > I think it will be easier to review the complete
> > series, not an incremental patch.
> Should I combine this with the prev series without the
> memory patches?
> 
> > 
> > We probably need to think what's the right thing
> > to do for master abort mode since we do not
> > emulate SERR. Do we make it writeable at all?
> Master abort mode can be enabled. (Tested)
> > 
> > It's a pci only bit:
> > When the Master-Abort Mode bit is cleared and a posted write transaction
> > forwarded by the
> > bridge terminates with a Master-Abort, no error is reported (note that
> > the Master-Abort bit
> > is still set). When the Master-Abort Mode bit is set and a posted write
> > transaction forwarded
> > by the bridge terminates with a Master-Abort on the destination bus, the
> > bridge must:
> > Assert SERR# on the primary interfaceCommand register)
> > (if enabled by the SERR# Enable bitin the
> > Set the Signaled System Errorbit in the Command register)
> > bitin the Status register (if enabled by the SERR# Enable)
> > 
> > one way to do this would be not to set Master-Abort bit even
> > though spec says we should, and pretend there was no error.
> Maybe we can just not allow to set "Master abort mode"
> in BRIDGE_CONTROL register. It is a cleaner way (I think)
> considering we don't emulate SERR.

I'm not sure P2P spec allows this - want to check.
It's the right thing to do for express.

Also pls note that cross-version migration
requires that we keep it writable for old
machine types.

> > 
> > > ---
> > >  hw/pci/pci.c | 115 
> > > ++-
> > >  hw/pci/pci_bridge.c  |  10 +
> > >  include/hw/pci/pci.h |   3 ++
> > >  include/hw/pci/pci_bus.h |   1 +
> > >  4 files changed, 118 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > index d8a1b11..1f4e707 100644
> > > --- a/hw/pci/pci.c
> > > +++ b/hw/pci/pci.c
> > > @@ -283,17 +283,76 @@ const char *pci_root_bus_path(PCIDevice *dev)
> > >  return rootbus->qbus.name;
> > >  }
> > >  
> > > +static PCIDevice *pci_bus_find_host(PCIBus *bus)
> > > +{
> > > +PCIDevice *dev;
> > > +int i;
> > > +
> > > +for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
> > > +dev = bus->devices[i];
> > > +if (dev) {
> > > +PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
> > > +if (pc->class_id == PCI_CLASS_BRIDGE_HOST) {
> > 
> > In fact you can do this easier:
> >  pci_get_word(dev->config + PCI_CLASS_DEVICE) == PCI_CLASS_BRIDGE_HOST
> Thanks, I'll change.
> > 
> > > +return dev;
> > > +}
> > > +}
> > > +}
> > > +
> > > +return NULL;
> > > +}
> > > +
> > > +static void master_abort(void *opaque)
> > > +{
> > > +PCIDevice *master_dev = NULL;
> > > +PCIDeviceClass *pc;
> > > +PCIBus *bus;
> > > +int downstream;
> > 
> > bool?
> Yes, I'll change
> 
> > > +
> > > +if (object_dynamic_cast(OBJECT(opaque), TYPE_PCI_DEVICE)) {
> > 
> > Please don't do dynamic cast just because you can.
> > You know very well what kind of object you pass
> > when you create the MR.
> > So just call the appropriate function.
> I wanted to merge the code for all 3 entities involved:
> root PCIBus, PCIBus behind a bridge, and PCIDevice.
> The region for merge was to use the same master_abort_mem_ops
> because there are the same operations that must be done:
> return -1 (0xFFF...) and set master abort received bit. 

return -1 is not a lot of common code, and MA bit
is in a different place each time.

> > 
> > > +master_dev = PCI_DEVICE(opaque);
> > > +bus = master_dev->bus;
> > > +while (!pci_bus_is_root(bus)) {
> > > +master_dev = bus->parent_dev;
> > > +bus = master_dev->bus;
> > > +}
> > > +downstream = 0;
> > > +}
> > > +
> > > +if (object_dynamic_cast(OBJECT(opaque), TYPE_PCI_BUS)) {
> > > +bus = PCI_BUS(opaque);
> > > +if (pci_bus_is_root(bus)) {
> > > +master_dev = pci_bus_find_host(bus);
> > > +} else { /* bus behind a PCI-2-PCI bridge */
> > > +master_dev = bus->parent_

[Qemu-devel] [PATCH v3 05/18] gumstix: Don't enforce use of -pflash for qtest

[Andreas Färber]

Signed-off-by: Andreas Färber 
---
 hw/arm/gumstix.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/arm/gumstix.c b/hw/arm/gumstix.c
index e97fbbd..aeea172 100644
--- a/hw/arm/gumstix.c
+++ b/hw/arm/gumstix.c
@@ -42,6 +42,7 @@
 #include "hw/boards.h"
 #include "sysemu/blockdev.h"
 #include "exec/address-spaces.h"
+#include "sysemu/qtest.h"
 
 static const int sector_len = 128 * 1024;
 
@@ -58,7 +59,7 @@ static void connex_init(QEMUMachineInitArgs *args)
 cpu = pxa255_init(address_space_mem, connex_ram);
 
 dinfo = drive_get(IF_PFLASH, 0, 0);
-if (!dinfo) {
+if (!dinfo && !qtest_enabled()) {
 fprintf(stderr, "A flash image must be given with the "
 "'pflash' parameter\n");
 exit(1);
@@ -70,7 +71,8 @@ static void connex_init(QEMUMachineInitArgs *args)
 be = 0;
 #endif
 if (!pflash_cfi01_register(0x, NULL, "connext.rom", connex_rom,
-   dinfo->bdrv, sector_len, connex_rom / 
sector_len,
+   dinfo ? dinfo->bdrv : NULL,
+   sector_len, connex_rom / sector_len,
2, 0, 0, 0, 0, be)) {
 fprintf(stderr, "qemu: Error registering flash memory.\n");
 exit(1);
@@ -95,7 +97,7 @@ static void verdex_init(QEMUMachineInitArgs *args)
 cpu = pxa270_init(address_space_mem, verdex_ram, cpu_model ?: "pxa270-c0");
 
 dinfo = drive_get(IF_PFLASH, 0, 0);
-if (!dinfo) {
+if (!dinfo && !qtest_enabled()) {
 fprintf(stderr, "A flash image must be given with the "
 "'pflash' parameter\n");
 exit(1);
@@ -107,7 +109,8 @@ static void verdex_init(QEMUMachineInitArgs *args)
 be = 0;
 #endif
 if (!pflash_cfi01_register(0x, NULL, "verdex.rom", verdex_rom,
-   dinfo->bdrv, sector_len, verdex_rom / 
sector_len,
+   dinfo ? dinfo->bdrv : NULL,
+   sector_len, verdex_rom / sector_len,
2, 0, 0, 0, 0, be)) {
 fprintf(stderr, "qemu: Error registering flash memory.\n");
 exit(1);
-- 
1.8.1.4

[Qemu-devel] [PATCH 08/10] target-s390: Fix STURA

[Richard Henderson]

We were storing 16 bits instead of 32.

Signed-off-by: Richard Henderson 
---
 target-s390x/mem_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-s390x/mem_helper.c b/target-s390x/mem_helper.c
index 1422ae9..408836c 100644
--- a/target-s390x/mem_helper.c
+++ b/target-s390x/mem_helper.c
@@ -1041,7 +1041,7 @@ void HELPER(ptlb)(CPUS390XState *env)
 /* store using real address */
 void HELPER(stura)(CPUS390XState *env, uint64_t addr, uint64_t v1)
 {
-stw_phys(get_address(env, 0, 0, addr), (uint32_t)v1);
+stl_phys(get_address(env, 0, 0, addr), (uint32_t)v1);
 }
 
 /* load real address */
-- 
1.8.1.4

[Qemu-devel] [PATCH 07/10] target-s390: Fix STIDP

[Richard Henderson]

The implementation had been incomplete, as we did not store the
machine type.

Signed-off-by: Richard Henderson 
---
 target-s390x/cpu.c   |  2 ++
 target-s390x/cpu.h   | 14 +-
 target-s390x/translate.c |  2 +-
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 01ff49b..d003dcf 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -257,6 +257,8 @@ static void s390_cpu_initfn(Object *obj)
 env->facilities[0] |= FAC0_Z9_109;
 #endif
 
+env->machine_type = 0x2094;   /* ??? Also Z9-109.  */
+
 if (tcg_enabled() && !inited) {
 inited = true;
 s390x_translate_init();
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index a0bafef..95f9cab 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -197,7 +197,19 @@ typedef struct CPUS390XState {
 /* reset does memset(0) up to here */
 uint64_t facilities[2];
 
-int cpu_num;
+union {
+uint64_t cpuid;
+struct {
+#ifdef HOST_WORDS_BIGENDIAN
+uint32_t cpu_num;
+uint32_t machine_type;
+#else
+uint32_t machine_type;
+uint32_t cpu_num;
+#endif
+};
+};
+
 uint8_t *storage_keys;
 
 uint64_t tod_offset;
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 192d54e..25a6537 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -3242,7 +3242,7 @@ static ExitStatus op_stctl(DisasContext *s, DisasOps *o)
 static ExitStatus op_stidp(DisasContext *s, DisasOps *o)
 {
 check_privileged(s);
-tcg_gen_ld32u_i64(o->out, cpu_env, offsetof(CPUS390XState, cpu_num));
+tcg_gen_ld_i64(o->out, cpu_env, offsetof(CPUS390XState, cpuid));
 return NO_EXIT;
 }
 
-- 
1.8.1.4

[Qemu-devel] [PATCH] tracing: start trace processing thread in final child process

[Christian Borntraeger]

From: Michael Mueller 

When running with trace backend e.g. "simple" the writer thread needs to be
implemented in the same process context as the trace points that will be
processed. Under libvirtd control, qemu gets first started in daemonized
mode to privide its capabilities. Creating the writer thread in the initial
process context then leads to a dead lock because the thread gets termined
together with the initial parent. (-daemonize)

Signed-off-by: Michael Mueller 
Signed-off-by: Christian Borntraeger 
[minor whitespace fixes]
---
 vl.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index 4e709d5..93670c8 100644
--- a/vl.c
+++ b/vl.c
@@ -3863,8 +3863,10 @@ int main(int argc, char **argv, char **envp)
 qemu_set_log(mask);
 }
 
-if (!trace_backend_init(trace_events, trace_file)) {
-exit(1);
+if (!is_daemonized()) {
+if (!trace_backend_init(trace_events, trace_file)) {
+exit(1);
+}
 }
 
 /* If no data_dir is specified then try to find it relative to the
@@ -4358,6 +4360,12 @@ int main(int argc, char **argv, char **envp)
 
 os_setup_post();
 
+if (is_daemonized()) {
+if (!trace_backend_init(trace_events, trace_file)) {
+exit(1);
+}
+}
+
 main_loop();
 bdrv_close_all();
 pause_all_vcpus();
-- 
1.8.3.1

[Qemu-devel] [PATCH] qemu-iotests: Do not execute 052 with -nocache

[Max Reitz]

Test 052 uses qemu-io -s which will result in bdrv_open trying to create
a temporary snapshot file in /tmp. However, since O_DIRECT and tmpfs
do not work well together, disable this test for -nocache.

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/052 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qemu-iotests/052 b/tests/qemu-iotests/052
index 14a5126..49810cb 100755
--- a/tests/qemu-iotests/052
+++ b/tests/qemu-iotests/052
@@ -41,6 +41,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt generic
 _supported_proto generic
 _supported_os Linux
+_unsupported_qemu_io_options --nocache
 
 
 size=128M
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] hw/pci: completed master-abort emulation

[Michael S. Tsirkin]

On Mon, Sep 23, 2013 at 05:43:38PM +0300, Marcel Apfelbaum wrote:
> On Mon, 2013-09-23 at 16:45 +0300, Michael S. Tsirkin wrote:
> > On Mon, Sep 23, 2013 at 03:37:43PM +0300, Marcel Apfelbaum wrote:
> > > On Mon, 2013-09-23 at 14:27 +0300, Michael S. Tsirkin wrote:
> > > > On Mon, Sep 23, 2013 at 02:01:17PM +0300, Marcel Apfelbaum wrote:
> > > > > This patch is implemented on top of series:
> > > > > [PATCH v5 0/3] pci: implement upstream master abort protocol
> > > > > 
> > > > > Added "master abort io" background region for PCIBus.
> > > > > 
> > > > > Added "master abort mem" region to catch transactions initiated
> > > > > by pci devices targeted to unassigned addresses.
> > > > > 
> > > > > Enabled "master abort" regions for PCI-2-PCI bridge's secondary bus.
> > > > > 
> > > > > Set "Received Master Abort" Bit on Status/Secondary Status
> > > > > register as defined in the PCI Spec.
> > > > > 
> > > > > Signed-off-by: Marcel Apfelbaum 
> > > > 
> > > > I think it will be easier to review the complete
> > > > series, not an incremental patch.
> > > Should I combine this with the prev series without the
> > > memory patches?
> > > 
> > > > 
> > > > We probably need to think what's the right thing
> > > > to do for master abort mode since we do not
> > > > emulate SERR. Do we make it writeable at all?
> > > Master abort mode can be enabled. (Tested)
> > > > 
> > > > It's a pci only bit:
> > > > When the Master-Abort Mode bit is cleared and a posted write 
> > > > transaction
> > > > forwarded by the
> > > > bridge terminates with a Master-Abort, no error is reported 
> > > > (note that
> > > > the Master-Abort bit
> > > > is still set). When the Master-Abort Mode bit is set and a 
> > > > posted write
> > > > transaction forwarded
> > > > by the bridge terminates with a Master-Abort on the destination 
> > > > bus, the
> > > > bridge must:
> > > > Assert SERR# on the primary interfaceCommand register)
> > > > (if enabled by the SERR# Enable bitin the
> > > > Set the Signaled System Errorbit in the Command register)
> > > > bitin the Status register (if enabled by the SERR# Enable)
> > > > 
> > > > one way to do this would be not to set Master-Abort bit even
> > > > though spec says we should, and pretend there was no error.
> > > Maybe we can just not allow to set "Master abort mode"
> > > in BRIDGE_CONTROL register. It is a cleaner way (I think)
> > > considering we don't emulate SERR.
> > 
> > I'm not sure P2P spec allows this - want to check.
> I will check this too, thanks
> 
> > It's the right thing to do for express.
> > 
> > Also pls note that cross-version migration
> > requires that we keep it writable for old
> > machine types.
> Yes :(. It seems we need another solution.

Not really. Just set a flag "don't assert master abort"
for old machine types.

> > 
> > > > 
> > > > > ---
> > > > >  hw/pci/pci.c | 115 
> > > > > ++-
> > > > >  hw/pci/pci_bridge.c  |  10 +
> > > > >  include/hw/pci/pci.h |   3 ++
> > > > >  include/hw/pci/pci_bus.h |   1 +
> > > > >  4 files changed, 118 insertions(+), 11 deletions(-)
> > > > > 
> > > > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > > > index d8a1b11..1f4e707 100644
> > > > > --- a/hw/pci/pci.c
> > > > > +++ b/hw/pci/pci.c
> > > > > @@ -283,17 +283,76 @@ const char *pci_root_bus_path(PCIDevice *dev)
> > > > >  return rootbus->qbus.name;
> > > > >  }
> > > > >  
> > > > > +static PCIDevice *pci_bus_find_host(PCIBus *bus)
> > > > > +{
> > > > > +PCIDevice *dev;
> > > > > +int i;
> > > > > +
> > > > > +for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
> > > > > +dev = bus->devices[i];
> > > > > +if (dev) {
> > > > > +PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
> > > > > +if (pc->class_id == PCI_CLASS_BRIDGE_HOST) {
> > > > 
> > > > In fact you can do this easier:
> > > >  pci_get_word(dev->config + PCI_CLASS_DEVICE) == 
> > > > PCI_CLASS_BRIDGE_HOST
> > > Thanks, I'll change.
> > > > 
> > > > > +return dev;
> > > > > +}
> > > > > +}
> > > > > +}
> > > > > +
> > > > > +return NULL;
> > > > > +}
> > > > > +
> > > > > +static void master_abort(void *opaque)
> > > > > +{
> > > > > +PCIDevice *master_dev = NULL;
> > > > > +PCIDeviceClass *pc;
> > > > > +PCIBus *bus;
> > > > > +int downstream;
> > > > 
> > > > bool?
> > > Yes, I'll change
> > > 
> > > > > +
> > > > > +if (object_dynamic_cast(OBJECT(opaque), TYPE_PCI_DEVICE)) {
> > > > 
> > > > Please don't do dynamic cast just because you can.
> > > > You know very well what kind of object you pass
> > > > when you create the MR.
> > > > So just call the appropriate function.
> > > I wanted to merge the code for all 3 entities involved:
> > > root PCIBus, PCIBus behind a bridge, and PCIDevice.
> > > The region for merge wa

Re: [Qemu-devel] [PATCH v4 06/12] xics: convert init() to realize()

[Andreas Färber]

Am 30.08.2013 07:28, schrieb Alexey Kardashevskiy:
> This fixes XICS according new QOM rules.
> 
> This converts ICS's init() callbacks to realize().
> 
> This converts legacy qdev_init_nofail() to property_set(realized).
> 
> Signed-off-by: Alexey Kardashevskiy 

Reviewed-by: Andreas Färber 

Thanks,
Andreas

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

Re: [Qemu-devel] [PATCH] hw/pci: completed master-abort emulation

[Marcel Apfelbaum]

On Mon, 2013-09-23 at 16:45 +0300, Michael S. Tsirkin wrote:
> On Mon, Sep 23, 2013 at 03:37:43PM +0300, Marcel Apfelbaum wrote:
> > On Mon, 2013-09-23 at 14:27 +0300, Michael S. Tsirkin wrote:
> > > On Mon, Sep 23, 2013 at 02:01:17PM +0300, Marcel Apfelbaum wrote:
> > > > This patch is implemented on top of series:
> > > > [PATCH v5 0/3] pci: implement upstream master abort protocol
> > > > 
> > > > Added "master abort io" background region for PCIBus.
> > > > 
> > > > Added "master abort mem" region to catch transactions initiated
> > > > by pci devices targeted to unassigned addresses.
> > > > 
> > > > Enabled "master abort" regions for PCI-2-PCI bridge's secondary bus.
> > > > 
> > > > Set "Received Master Abort" Bit on Status/Secondary Status
> > > > register as defined in the PCI Spec.
> > > > 
> > > > Signed-off-by: Marcel Apfelbaum 
> > > 
> > > I think it will be easier to review the complete
> > > series, not an incremental patch.
> > Should I combine this with the prev series without the
> > memory patches?
> > 
> > > 
> > > We probably need to think what's the right thing
> > > to do for master abort mode since we do not
> > > emulate SERR. Do we make it writeable at all?
> > Master abort mode can be enabled. (Tested)
> > > 
> > > It's a pci only bit:
> > >   When the Master-Abort Mode bit is cleared and a posted write transaction
> > >   forwarded by the
> > >   bridge terminates with a Master-Abort, no error is reported (note that
> > >   the Master-Abort bit
> > >   is still set). When the Master-Abort Mode bit is set and a posted write
> > >   transaction forwarded
> > >   by the bridge terminates with a Master-Abort on the destination bus, the
> > >   bridge must:
> > >   Assert SERR# on the primary interfaceCommand register)
> > >   (if enabled by the SERR# Enable bitin the
> > >   Set the Signaled System Errorbit in the Command register)
> > >   bitin the Status register (if enabled by the SERR# Enable)
> > > 
> > > one way to do this would be not to set Master-Abort bit even
> > > though spec says we should, and pretend there was no error.
> > Maybe we can just not allow to set "Master abort mode"
> > in BRIDGE_CONTROL register. It is a cleaner way (I think)
> > considering we don't emulate SERR.
> 
> I'm not sure P2P spec allows this - want to check.
I will check this too, thanks

> It's the right thing to do for express.
> 
> Also pls note that cross-version migration
> requires that we keep it writable for old
> machine types.
Yes :(. It seems we need another solution.

> 
> > > 
> > > > ---
> > > >  hw/pci/pci.c | 115 
> > > > ++-
> > > >  hw/pci/pci_bridge.c  |  10 +
> > > >  include/hw/pci/pci.h |   3 ++
> > > >  include/hw/pci/pci_bus.h |   1 +
> > > >  4 files changed, 118 insertions(+), 11 deletions(-)
> > > > 
> > > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > > index d8a1b11..1f4e707 100644
> > > > --- a/hw/pci/pci.c
> > > > +++ b/hw/pci/pci.c
> > > > @@ -283,17 +283,76 @@ const char *pci_root_bus_path(PCIDevice *dev)
> > > >  return rootbus->qbus.name;
> > > >  }
> > > >  
> > > > +static PCIDevice *pci_bus_find_host(PCIBus *bus)
> > > > +{
> > > > +PCIDevice *dev;
> > > > +int i;
> > > > +
> > > > +for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
> > > > +dev = bus->devices[i];
> > > > +if (dev) {
> > > > +PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
> > > > +if (pc->class_id == PCI_CLASS_BRIDGE_HOST) {
> > > 
> > > In fact you can do this easier:
> > >pci_get_word(dev->config + PCI_CLASS_DEVICE) == PCI_CLASS_BRIDGE_HOST
> > Thanks, I'll change.
> > > 
> > > > +return dev;
> > > > +}
> > > > +}
> > > > +}
> > > > +
> > > > +return NULL;
> > > > +}
> > > > +
> > > > +static void master_abort(void *opaque)
> > > > +{
> > > > +PCIDevice *master_dev = NULL;
> > > > +PCIDeviceClass *pc;
> > > > +PCIBus *bus;
> > > > +int downstream;
> > > 
> > > bool?
> > Yes, I'll change
> > 
> > > > +
> > > > +if (object_dynamic_cast(OBJECT(opaque), TYPE_PCI_DEVICE)) {
> > > 
> > > Please don't do dynamic cast just because you can.
> > > You know very well what kind of object you pass
> > > when you create the MR.
> > > So just call the appropriate function.
> > I wanted to merge the code for all 3 entities involved:
> > root PCIBus, PCIBus behind a bridge, and PCIDevice.
> > The region for merge was to use the same master_abort_mem_ops
> > because there are the same operations that must be done:
> > return -1 (0xFFF...) and set master abort received bit. 
> 
> return -1 is not a lot of common code, and MA bit
> is in a different place each time.
Ack

> 
> > > 
> > > > +master_dev = PCI_DEVICE(opaque);
> > > > +bus = master_dev->bus;
> > > > +while (!pci_bus_is_root(bus)) {
> > > > +master_dev = bus->parent_dev;
> > > > +bus = master_dev->bus;
> > > >

Re: [Qemu-devel] [PATCH v4 07/12] xics: add missing const specifiers to TypeInfo

[Andreas Färber]

Am 30.08.2013 07:28, schrieb Alexey Kardashevskiy:
> This adds missing const specifiers to ICS and ICP TypeInfo's.
> 
> Signed-off-by: Alexey Kardashevskiy 

Reviewed-by: Andreas Färber 

Andreas

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

[Qemu-devel] [PATCH 05/10] target-s390: Implement SAM31 and SAM64

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 target-s390x/insn-data.def |  8 
 target-s390x/translate.c   | 29 +
 2 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/target-s390x/insn-data.def b/target-s390x/insn-data.def
index 4b462d4..c528eb4 100644
--- a/target-s390x/insn-data.def
+++ b/target-s390x/insn-data.def
@@ -566,6 +566,10 @@
 
 /* SET ACCESS */
 C(0xb24e, SAR, RRE,   Z,   0, r2_o, 0, 0, sar, 0)
+/* SET ADDRESSING MODE */
+/* We only do 32 and 64-bit.  Let SAM24 signal illegal instruction.  */
+C(0x010d, SAM31,   E, Z,   0, 0, 0, 0, sam31, 0)
+C(0x010e, SAM64,   E, Z,   0, 0, 0, 0, sam64, 0)
 /* SET FPC */
 C(0xb384, SFPC,RRE,   Z,   0, r1_o, 0, 0, sfpc, 0)
 /* SET FPC AND SIGNAL */
@@ -745,10 +749,6 @@
 C(0xb22a, RRBE,RRE,   Z,   0, r2_o, 0, 0, rrbe, 0)
 /* SERVICE CALL LOGICAL PROCESSOR (PV hypercall) */
 C(0xb220, SERVC,   RRE,   Z,   r1_o, r2_o, 0, 0, servc, 0)
-/* SET ADDRESSING MODE */
-/* We only do 64-bit, so accept this as a no-op.
-   Let SAM24 and SAM31 signal illegal instruction.  */
-C(0x010e, SAM64,   E, Z,   0, 0, 0, 0, 0, 0)
 /* SET ADDRESS SPACE CONTROL FAST */
 C(0xb279, SACF,S, Z,   0, a2, 0, 0, sacf, 0)
 /* SET CLOCK */
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index c8bbedb..6ca8f0b 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -2912,6 +2912,35 @@ static ExitStatus op_sacf(DisasContext *s, DisasOps *o)
 }
 #endif
 
+static ExitStatus op_sam31(DisasContext *s, DisasOps *o)
+{
+/* Bizzare but true, we check the address of the current insn for the
+   specification exception, not the next to be executed.  Thus the PoO
+   documents that Bad Things Happen at 0x7ffe.  */
+if (s->pc & ~0x7ff) {
+gen_program_exception(s, PGM_SPECIFICATION);
+return EXIT_NORETURN;
+}
+
+tcg_gen_andi_i64(psw_mask, psw_mask, ~PSW_MASK_64);
+tcg_gen_ori_i64(psw_mask, psw_mask, PSW_MASK_32);
+
+/* Always exit the TB, since we (may have) changed execution mode.  */
+s->pc = s->next_pc & 0x7fff;
+update_psw_addr(s);
+return EXIT_PC_UPDATED;
+}
+
+static ExitStatus op_sam64(DisasContext *s, DisasOps *o)
+{
+tcg_gen_ori_i64(psw_mask, psw_mask, PSW_MASK_32 | PSW_MASK_64);
+
+/* Always exit the TB, since we (may have) changed execution mode.  */
+s->pc = s->next_pc;
+update_psw_addr(s);
+return EXIT_PC_UPDATED;
+}
+
 static ExitStatus op_sar(DisasContext *s, DisasOps *o)
 {
 int r1 = get_field(s->fields, r1);
-- 
1.8.1.4

[Qemu-devel] [PATCH v3 15/18] shix: Drop debug output

[Andreas Färber]

Reviewed-by: Aurelien Jarno 
Signed-off-by: Andreas Färber 
---
 hw/sh4/shix.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/hw/sh4/shix.c b/hw/sh4/shix.c
index 1ff37f5..f008b98 100644
--- a/hw/sh4/shix.c
+++ b/hw/sh4/shix.c
@@ -50,7 +50,6 @@ static void shix_init(QEMUMachineInitArgs *args)
 if (!cpu_model)
 cpu_model = "any";
 
-printf("Initializing CPU\n");
 cpu = cpu_sh4_init(cpu_model);
 if (cpu == NULL) {
 fprintf(stderr, "Unable to find CPU definition\n");
@@ -58,16 +57,13 @@ static void shix_init(QEMUMachineInitArgs *args)
 }
 
 /* Allocate memory space */
-printf("Allocating ROM\n");
 memory_region_init_ram(rom, NULL, "shix.rom", 0x4000);
 vmstate_register_ram_global(rom);
 memory_region_set_readonly(rom, true);
 memory_region_add_subregion(sysmem, 0x, rom);
-printf("Allocating SDRAM 1\n");
 memory_region_init_ram(&sdram[0], NULL, "shix.sdram1", 0x0100);
 vmstate_register_ram_global(&sdram[0]);
 memory_region_add_subregion(sysmem, 0x0800, &sdram[0]);
-printf("Allocating SDRAM 2\n");
 memory_region_init_ram(&sdram[1], NULL, "shix.sdram2", 0x0100);
 vmstate_register_ram_global(&sdram[1]);
 memory_region_add_subregion(sysmem, 0x0c00, &sdram[1]);
@@ -75,10 +71,8 @@ static void shix_init(QEMUMachineInitArgs *args)
 /* Load BIOS in 0 (and access it through P2, 0xA000) */
 if (bios_name == NULL)
 bios_name = BIOS_FILENAME;
-printf("%s: load BIOS '%s'\n", __func__, bios_name);
 ret = load_image_targphys(bios_name, 0, 0x4000);
 if (ret < 0) { /* Check bios size */
-   fprintf(stderr, "ret=%d\n", ret);
fprintf(stderr, "qemu: could not load SHIX bios '%s'\n",
bios_name);
exit(1);
@@ -88,7 +82,6 @@ static void shix_init(QEMUMachineInitArgs *args)
 s = sh7750_init(cpu, sysmem);
 /* X Check success */
 tc58128_init(s, "shix_linux_nand.bin", NULL);
-fprintf(stderr, "initialization terminated\n");
 }
 
 static QEMUMachine shix_machine = {
-- 
1.8.1.4