Re: [Qemu-devel] [PATCH] migration: flush the bdrv before stopping VM

2015-03-20 Thread Li, Liang Z
> >> > Right now, we don't have an interface to detect that cases and got
> >> > back to the iterative stage.
> >>
> >> How about go back to the iterative stage when detect that the
> >> pending_size is larger Than max_size, like this:
> >>
> >> +/* do flush here is aimed to shorten the VM downtime,
> >> + * bdrv_flush_all is a time consuming operation
> >> + * when the guest has done some file writing */
> >> +bdrv_flush_all();
> >> +pending_size = qemu_savevm_state_pending(s->file, 
> >> max_size);
> >> +if (pending_size && pending_size >= max_size) {
> >> +qemu_mutex_unlock_iothread();
> >> +continue;
> >> +}
> >>   ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
> >>   if (ret >= 0) {
> >>   qemu_file_set_rate_limit(s->file, INT64_MAX);
> >>
> >> and this is quite simple.
> >
> > Yes, but it is too simple. If you hold all the locks during
> > bdrv_flush_all(), your VM will effectively stop as soon as it performs
> > the next I/O access, so you don't win much. And you still don't have a
> > timeout for cases where the flush takes really long.
> 
> This is probably better than what we had now (basically we are "meassuring"
> after bdrv_flush_all how much the amount of dirty memory has changed,
> and return to iterative stage if it took too much.  A timeout would be better
> anyways.  And an interface te start the synchronization sooner
> asynchronously would be also good.
> 
> Notice that my understanding is that any proper fix for this is 2.4 material.

Then, how to deal with this issue in 2.3, leave it here? or make an incomplete 
fix like I do above?

Liang

> Thanks, Juan.



Re: [Qemu-devel] [PATCH V4 06/19] virtio-ccw: using VIRTIO_NO_VECTOR instead of 0 for invalid virtqueue

2015-03-20 Thread Cornelia Huck
On Wed, 18 Mar 2015 14:08:56 +0100
"Michael S. Tsirkin"  wrote:

> On Wed, Mar 18, 2015 at 05:34:56PM +0800, Jason Wang wrote:
> > There's no need to use vector 0 for invalid virtqueue. So this patch
> > changes to use VIRTIO_NO_VECTOR instead.
> > 
> > Cc: Michael S. Tsirkin 
> > Cc: Cornelia Huck 
> > CC: Christian Borntraeger 
> > Cc: Richard Henderson 
> > Cc: Alexander Graf 
> > Signed-off-by: Jason Wang 
> 
> I don't know what does this actually do.
> Cornelia?

I actually have the same patch somewhere in my queue. The point here is
that 0 is plain wrong (it's a valid queue), while VIRTIO_NO_VECTOR is
most certainly no valid queue.

> 
> > ---
> >  hw/s390x/virtio-ccw.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> > index 130535c..c8b87aa 100644
> > --- a/hw/s390x/virtio-ccw.c
> > +++ b/hw/s390x/virtio-ccw.c
> > @@ -281,7 +281,7 @@ static int virtio_ccw_set_vqs(SubchDev *sch, uint64_t 
> > addr, uint32_t align,
> >  
> >  virtio_queue_set_addr(vdev, index, addr);
> >  if (!addr) {
> > -virtio_queue_set_vector(vdev, index, 0);
> > +virtio_queue_set_vector(vdev, index, VIRTIO_NO_VECTOR);
> >  } else {
> >  /* Fail if we don't have a big enough queue. */
> >  /* TODO: Add interface to handle vring.num changing */
> 
> Right below this, we have
> /* tell notify handler in case of config change */
> vdev->config_vector = VIRTIO_PCI_QUEUE_MAX;
> 
> which also does not seem to make sense.

Basically we have:

- at most 64 virtqueues with their own indicators (always 64 indicator
bits when using classic I/O interrupts, up to 64 indicator bits when
using adapter interrupts)
- another indicator bit for configuration changes (bit 0 of the
secondary indicator bits)

That way, the configuration change indicator is always one bit behind
the last possible queue indicator.

> 
> These changes need some testing though.

My identical patch seemed to work for me.




Re: [Qemu-devel] [PATCH v2 3/7] pci: Remove unused function ich9_d2pbr_init()

2015-03-20 Thread Thomas Huth
On Wed, 18 Mar 2015 14:20:41 +0100
"Michael S. Tsirkin"  wrote:

> On Sat, Mar 14, 2015 at 07:19:30AM +0100, Thomas Huth wrote:
> > The function ich9_d2pbr_init() is completely unused and
> > thus can be deleted.
> > 
> > Signed-off-by: Thomas Huth 
> > Cc: Michael S. Tsirkin 
> 
> Reviewed-by: Michael S. Tsirkin 
> 
> I assume all this will go in for 2.4 through the trivial tree?

The patch that removes unused functions from the migration code has
already been picked up in the migration tree ... so I also don't mind
whether you want to take this through the pci tree or not. Just if
the patches are not picked up through other trees, they should finally
go through trivial, I think.

 Thomas




Re: [Qemu-devel] [PATCH 1/3] block: add bdrv_get_device_or_node_name()

2015-03-20 Thread Markus Armbruster
Alberto Garcia  writes:

> This function gets the device name associated with a BlockDriverState,
> or its node name if the device name is empty.
>
> Signed-off-by: Alberto Garcia 
> ---
>  block.c   | 5 +
>  block/quorum.c| 5 +
>  include/block/block.h | 1 +
>  3 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/block.c b/block.c
> index 0fe97de..af284e3 100644
> --- a/block.c
> +++ b/block.c
> @@ -3920,6 +3920,11 @@ const char *bdrv_get_device_name(const 
> BlockDriverState *bs)
>  return bs->blk ? blk_name(bs->blk) : "";
>  }
>  
> +const char *bdrv_get_device_or_node_name(const BlockDriverState *bs)
> +{
> +return bs->blk ? blk_name(bs->blk) : bs->node_name;
> +}
> +

Does this have uses beyond identifying @bs to the user?

>  int bdrv_get_flags(BlockDriverState *bs)
>  {
>  return bs->open_flags;
> diff --git a/block/quorum.c b/block/quorum.c
> index 437b122..f91ef75 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -226,10 +226,7 @@ static void quorum_report_bad(QuorumAIOCB *acb, char 
> *node_name, int ret)
>  
>  static void quorum_report_failure(QuorumAIOCB *acb)
>  {
> -const char *reference = bdrv_get_device_name(acb->common.bs)[0] ?
> -bdrv_get_device_name(acb->common.bs) :
> -acb->common.bs->node_name;
> -
> +const char *reference = bdrv_get_device_or_node_name(acb->common.bs);
>  qapi_event_send_quorum_failure(reference, acb->sector_num,
> acb->nb_sectors, &error_abort);
>  }

Preexisting: what if reference is null?

event.json suggests it can't be null:

##
# @QUORUM_FAILURE
#
# Emitted by the Quorum block driver if it fails to establish a quorum
#
# @reference: device name if defined else node name
#
# @sector-num: number of the first sector of the failed read operation
#
# @sectors-count: failed read operation sector count
#
# Since: 2.0
##
{ 'event': 'QUORUM_FAILURE',
  'data': { 'reference': 'str', 'sector-num': 'int', 'sectors-count': 'int' 
} }

> diff --git a/include/block/block.h b/include/block/block.h
> index 4c57d63..b285e0d 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -398,6 +398,7 @@ void bdrv_iterate_format(void (*it)(void *opaque, const 
> char *name),
>   void *opaque);
>  const char *bdrv_get_node_name(const BlockDriverState *bs);
>  const char *bdrv_get_device_name(const BlockDriverState *bs);
> +const char *bdrv_get_device_or_node_name(const BlockDriverState *bs);
>  int bdrv_get_flags(BlockDriverState *bs);
>  int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num,
>const uint8_t *buf, int nb_sectors);



Re: [Qemu-devel] [PATCH V6 for-2.3 00/26] hw/pc: implement multiple primary busses for pc machines

2015-03-20 Thread Gerd Hoffmann
  Hi,

>  - You are more than welcome to try using:
>-bios 

Not needed any more, the seabios-1.8.1 release in master should work
just fine.

cheers,
  Gerd





Re: [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages

2015-03-20 Thread Markus Armbruster
Alberto Garcia  writes:

> There are several error messages that identify a BlockDriverState by
> its device name. However those errors can be produced in nodes that
> don't have a device name associated.
>
> In those cases we should use bdrv_get_device_or_node_name() to fall
> back to the node name and produce a more meaningful message.
>
> Signed-off-by: Alberto Garcia 
> ---
>  block.c   | 13 +++--
>  block/qcow.c  |  4 ++--
>  block/qcow2.c |  2 +-
>  block/qed.c   |  2 +-
>  block/vdi.c   |  2 +-
>  block/vhdx.c  |  2 +-
>  block/vmdk.c  |  4 ++--
>  block/vpc.c   |  2 +-
>  block/vvfat.c |  3 ++-
>  9 files changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/block.c b/block.c
> index af284e3..655d9aa 100644
> --- a/block.c
> +++ b/block.c
> @@ -1225,7 +1225,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs, 
> BlockDriverState *backing_hd)
>  } else if (backing_hd) {
>  error_setg(&bs->backing_blocker,
> "device is used as backing hd of '%s'",
> -   bdrv_get_device_name(bs));
> +   bdrv_get_device_or_node_name(bs));
>  }
>  
>  bs->backing_hd = backing_hd;

Before:

* if @bs belongs to a BB with name N, print "backing hd of 'N'"
* else print "backing hd of ''" (not nice, but works)

After: 

* if @bs belongs to a BB with name N, print "backing hd of 'N'" (no change)
* else if @bs has a node name NN, print "backing hd of 'NN'" (improvement)
* else print a null pointer, which crashes on some systems (bug)

If you want your bdrv_get_device_or_node_name() serve as drop-in
replacement for bdrv_get_device_name(), it must not return null.

In my opinion, such a drop-in replacement can only be a stop gap.  We
need to review every error message and figure out how to update it for
node names.  We should've done it back when we added node names.

Your misuse of bdrv_get_device_or_node_name() shows misuse is likely.
As long as that's the case, it needs a function comment.

[...]



Re: [Qemu-devel] [PATCH 1/3] block: add bdrv_get_device_or_node_name()

2015-03-20 Thread Alberto Garcia
On Fri, Mar 20, 2015 at 08:40:32AM +0100, Markus Armbruster wrote:

> > +const char *bdrv_get_device_or_node_name(const BlockDriverState *bs)
> > +{
> > +return bs->blk ? blk_name(bs->blk) : bs->node_name;
> > +}
> > +
> 
> Does this have uses beyond identifying @bs to the user?

None that I can think of, although apart from error messages it can
also be used in data types, like the cases of BLOCK_IMAGE_CORRUPTED
and BlockJobInfo that are being discussed:

   https://lists.gnu.org/archive/html/qemu-devel/2015-03/msg03651.html

> >  static void quorum_report_failure(QuorumAIOCB *acb)
> >  {
> > -const char *reference = bdrv_get_device_name(acb->common.bs)[0] ?
> > -bdrv_get_device_name(acb->common.bs) :
> > -acb->common.bs->node_name;
> > -
> > +const char *reference = bdrv_get_device_or_node_name(acb->common.bs);
> >  qapi_event_send_quorum_failure(reference, acb->sector_num,
> > acb->nb_sectors, &error_abort);
> >  }
> 
> Preexisting: what if reference is null?

It can't happen, both the device and the node name strings are
guaranteed to be non-null. The latter is actually a static string so
there's no chance bs->node_name returns a null pointer.

Berto



Re: [Qemu-devel] [PATCH v5 10/13] blockdev: Keep track of monitor-owned BDS

2015-03-20 Thread Markus Armbruster
Eric Blake  writes:

> On 03/03/2015 01:13 PM, Max Reitz wrote:
>> Signed-off-by: Max Reitz 
>> ---
>>  block.c|  2 ++
>>  blockdev.c | 18 ++
>>  include/block/block_int.h  |  4 
>>  stubs/Makefile.objs|  1 +
>>  stubs/blockdev-close-all-bdrv-states.c |  5 +
>>  5 files changed, 30 insertions(+)
>>  create mode 100644 stubs/blockdev-close-all-bdrv-states.c
>
> Again, might be nice for the commit message to document why adding this
> is useful, but doesn't affect the code.
>
> Reviewed-by: Eric Blake 

Might be nice?  This absolutely needs an explanation, in the code!

Why do we need a separate list of "monitor-owned BDS"?

What makes a BDS "monitor-owned"?

What are the invariants governing relations among bdrv_states,
graph_bdrv_states (what a horrible name) and blk_backends?



[Qemu-devel] [PULL for-2.3 v2 00/13] usb patch queue

2015-03-20 Thread Gerd Hoffmann
  Hi,

Here is the usb patch queue for 2.3-rc1.  Grow a bit larger than I'd
like it to be.  But I've somehow missed I had patches lingering in the
usb queue while preparing the -rc0 pulls, and there also have been some
bugfixes coming in pretty late.

New in v2: added missing signed-off.  No code changes, so I don't spam
the list with a repost of the patches.

please pull,
  Gerd

The following changes since commit 5a4992834daec85c3913654903fb9f4f954e585a:

  Merge remote-tracking branch 'remotes/armbru/tags/pull-cov-model-2015-03-17' 
into staging (2015-03-17 11:43:00 +)

are available in the git repository at:


  git://git.kraxel.org/qemu tags/pull-usb-20150320-1

for you to fetch changes up to 4e289b1b62c8e271e3400317b4c3d98909093bc4:

  ehci: fix segfault when hot-unplugging ehci controller (2015-03-20 08:50:12 
+0100)


usb: bugfix collection.


Gonglei (3):
  uhci: fix segfault when hot-unplugging uhci controller
  ohci: fix resource cleanup leak
  ehci: fix segfault when hot-unplugging ehci controller

Markus Armbruster (9):
  usb: Propagate errors through usb_register_companion()
  usb: Improve companion configuration error messages
  ohci: Complete conversion to realize
  uhci: Convert to realize
  monitor: Drop dead QMP check from monitor_read_password()
  monitor: Plug memory leak in monitor_read_bdrv_key_start()
  monitor usb: Inline monitor_read_bdrv_key_start()'s first part
  usb/dev-storage: Fix QMP device_add missing encryption key failure
  usb/dev-storage: Avoid qerror_report_err() outside QMP handlers

Thomas Huth (1):
  hw/usb: Include USB files only if necessary

 default-configs/arm-softmmu.mak |  1 +
 default-configs/usb.mak |  1 +
 hw/usb/Makefile.objs|  8 +++---
 hw/usb/bus.c| 27 +-
 hw/usb/dev-storage.c| 30 
 hw/usb/hcd-ehci-pci.c   | 10 +++
 hw/usb/hcd-ehci-sysbus.c| 10 +++
 hw/usb/hcd-ehci.c   | 31 -
 hw/usb/hcd-ehci.h   |  1 +
 hw/usb/hcd-ohci.c   | 62 -
 hw/usb/hcd-uhci.c   | 38 +
 include/hw/usb.h| 12 
 monitor.c   | 32 -
 13 files changed, 152 insertions(+), 111 deletions(-)



Re: [Qemu-devel] [PATCH] virtio: move sanity checks to ifdef DEBUG

2015-03-20 Thread Cornelia Huck
On Wed, 18 Mar 2015 12:42:46 +0100
"Michael S. Tsirkin"  wrote:

> All that happens when virtqueue_fill is invoked incorrectly is that we
> corrupt guest memory, so this check is not a security measure.
> Move the check to ifdef DEBUG to make sure we don't introduce new
> crashes close to release.

I don't really disagree with this patch, but is anybody actually making
regular runs with VIRTIO_DEBUG activated?

> Array scans aren't free either, so it's a good idea from performance
> point of view, too.
> 
> Cc: Rusty Russell 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  hw/virtio/virtio.c | 15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 27429c2..37fb2ee 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -243,15 +243,22 @@ int virtio_queue_empty(VirtQueue *vq)
>  void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
>  unsigned int len_written, unsigned int idx)
>  {
> -unsigned int offset, tot_wlen;
> +unsigned int offset;
>  int i;
> 
>  trace_virtqueue_fill(vq, elem, len_written, idx);
> 
> -for (tot_wlen = i = 0; i < elem->in_num; i++) {
> -tot_wlen += elem->in_sg[i].iov_len;
> +#ifdef DEBUG_VIRTIO
> +{
> +/* Check that len_written is <= the writable length. */
> +unsigned int tot_wlen;
> +
> +for (tot_wlen = i = 0; i < elem->in_num; i++) {
> +tot_wlen += elem->in_sg[i].iov_len;
> +}
> +assert(len_written <= tot_wlen);
>  }
> -assert(len_written <= tot_wlen);
> +#endif
> 
>  offset = 0;
>  for (i = 0; i < elem->in_num; i++) {




Re: [Qemu-devel] [PULL 02/29] s390x/ipl: support diagnose 308 subcodes 5 and 6

2015-03-20 Thread Christian Borntraeger
Am 19.03.2015 um 10:31 schrieb Paolo Bonzini:
> 
> 
> On 18/02/2015 21:21, Christian Borntraeger wrote:
>> From: Fan Zhang 
>>
>> To support dynamically updating the IPL device from inside the KVM
>> guest on the s390 platform, DIAG 308 instruction is intercepted
>> in QEMU to handle the request.
>>
>> Subcode 5 allows to specify a new boot device, which is saved for
>> later in the s390_ipl device. This also allows to switch from an
>> external kernel to a boot device.
>>
>> Subcode 6 retrieves boot device configuration that has been previously
>> set.
>>
>> Reviewed-by: Cornelia Huck 
>> Reviewed-by: David Hildenbrand 
>> Acked-by: Christian Borntraeger 
>> Signed-off-by: Jens Freimann 
>> Signed-off-by: Fan Zhang 
>> Signed-off-by: Christian Borntraeger 
>> ---
>>  hw/s390x/ipl.c | 87 
>> ++
>>  hw/s390x/ipl.h | 24 +
>>  hw/s390x/s390-virtio.c |  2 ++
>>  target-s390x/misc_helper.c | 33 --
>>  4 files changed, 129 insertions(+), 17 deletions(-)
>>  create mode 100644 hw/s390x/ipl.h
>>
>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>> index 4014a6a..231713d 100644
>> --- a/hw/s390x/ipl.c
>> +++ b/hw/s390x/ipl.c
>> @@ -18,6 +18,7 @@
>>  #include "hw/sysbus.h"
>>  #include "hw/s390x/virtio-ccw.h"
>>  #include "hw/s390x/css.h"
>> +#include "ipl.h"
>>  
>>  #define KERN_IMAGE_START0x01UL
>>  #define KERN_PARM_AREA  0x010480UL
>> @@ -52,12 +53,17 @@ typedef struct S390IPLState {
>>  uint64_t start_addr;
>>  uint64_t bios_start_addr;
>>  bool enforce_bios;
>> +IplParameterBlock iplb;
>> +bool iplb_valid;
>>  
>>  /*< public >*/
>>  char *kernel;
>>  char *initrd;
>>  char *cmdline;
>>  char *firmware;
>> +uint8_t cssid;
>> +uint8_t ssid;
>> +uint16_t devno;
>>  } S390IPLState;
>>  
>>  
>> @@ -164,6 +170,69 @@ static Property s390_ipl_properties[] = {
>>  DEFINE_PROP_END_OF_LIST(),
>>  };
>>  
>> +/*
>> + * In addition to updating the iplstate, this function returns:
>> + * - 0 if system was ipled with external kernel
>> + * - -1 if no valid boot device was found
>> + * - ccw id of the boot device otherwise
>> + */
>> +static uint64_t s390_update_iplstate(CPUS390XState *env, S390IPLState *ipl)
> 
> This should probably return uint32_t, because otherwise...
> 
>> +return ipl->cssid << 24 | ipl->ssid << 16 | ipl->devno;
> 
> ... a cssid above 127 results in the bits of the high word all set to 1.
> 
> The reason is that, even though ipl->cssid is a uint8_t, before the
> shift it is extended to int.  Then the return statement does a sign
> extension from int to uint64_t.
> 
> Paolo

You are right. The good news is, that it does not matter, as the s390-ccw
bios will shift and mask to get the devno and the ssid. It ignores the cssid
because it does not enable the multiple channel subsystem feature (in the
same way as no other operating system on z) and it ignores the upper
32 bits if the value is != -1.

We will fix that anyway, of course.

Christian
> 
>> +}
>> +
>> +int s390_ipl_update_diag308(IplParameterBlock *iplb)
>> +{
>> +S390IPLState *ipl;
>> +
>> +ipl = S390_IPL(object_resolve_path(TYPE_S390_IPL, NULL));
>> +if (ipl) {
>> +ipl->iplb = *iplb;
>> +ipl->iplb_valid = true;
>> +return 0;
>> +}
>> +return -1;
>> +}
>> +
>> +IplParameterBlock *s390_ipl_get_iplb(void)
>> +{
>> +S390IPLState *ipl;
>> +
>> +ipl = S390_IPL(object_resolve_path(TYPE_S390_IPL, NULL));
>> +if (!ipl || !ipl->iplb_valid) {
>> +return NULL;
>> +}
>> +return &ipl->iplb;
>> +}
>> +
>>  static void s390_ipl_reset(DeviceState *dev)
>>  {
>>  S390IPLState *ipl = S390_IPL(dev);
>> @@ -173,21 +242,9 @@ static void s390_ipl_reset(DeviceState *dev)
>>  env->psw.addr = ipl->start_addr;
>>  env->psw.mask = IPL_PSW_MASK;
>>  
>> -if (!ipl->kernel) {
>> -/* Tell firmware, if there is a preferred boot device */
>> -env->regs[7] = -1;
>> -DeviceState *dev_st = get_boot_device(0);
>> -if (dev_st) {
>> -VirtioCcwDevice *ccw_dev = (VirtioCcwDevice *) 
>> object_dynamic_cast(
>> -OBJECT(qdev_get_parent_bus(dev_st)->parent),
>> -TYPE_VIRTIO_CCW_DEVICE);
>> -
>> -if (ccw_dev) {
>> -env->regs[7] = ccw_dev->sch->cssid << 24 |
>> -   ccw_dev->sch->ssid << 16 |
>> -   ccw_dev->sch->devno;
>> -}
>> -}
>> +if (!ipl->kernel || ipl->iplb_valid) {
>> +env->psw.addr = ipl->bios_start_addr;
>> +env->regs[7] = s390_update_iplstate(env, ipl);
>>  }
>>  
>>  s390_cpu_set_state(CPU_STATE_OPERATING, cpu);
>> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
>> new file mode 100644
>> index 000..f1d082f
>> --- /dev/null
>> +++ b/hw/s390x/ipl.h
>> @@ -0,0 +1,24 @@
>> +/*
>> + * s390 IPL device
>

Re: [Qemu-devel] [PATCH V6 for-2.3 00/26] hw/pc: implement multiple primary busses for pc machines

2015-03-20 Thread Marcel Apfelbaum

On 03/20/2015 09:44 AM, Gerd Hoffmann wrote:

   Hi,


  - You are more than welcome to try using:
-bios 


Not needed any more, the seabios-1.8.1 release in master should work
just fine.

Good to know, thanks!
Marcel



cheers,
   Gerd







Re: [Qemu-devel] [PATCH V6 for-2.3 00/26] hw/pc: implement multiple primary busses for pc machines

2015-03-20 Thread Marcel Apfelbaum

On 03/19/2015 11:45 PM, Paolo Bonzini wrote:



On 19/03/2015 19:52, Marcel Apfelbaum wrote:

  hw/pci-host/piix.c  | 128 -
  hw/pci-host/ppce500.c   |   1 +
  hw/pci-host/q35.c   |   5 +
  hw/pci-host/uninorth.c  |   1 +
  hw/pci/Makefile.objs|   2 +-
  hw/pci/pci.c| 493 +-
  hw/pci/pci_bus.c| 517 
  hw/pci/pci_host.c   |  42 +++
  hw/ppc/ppc4xx_pci.c |   1 +
  hw/scsi/megasas.c   |   1 +
  hw/sh4/r2d.c|   1 +
  hw/sh4/sh_pci.c |   1 +
  hw/vfio/pci.c   |   1 +
  hw/xen/xen_pt.c |   1 +
  include/hw/acpi/aml-build.h |   8 +
  include/hw/i386/pc.h|   8 +-
  include/hw/pci/pci.h|   6 +-
  include/hw/pci/pci_bus.h|  35 +++
  include/hw/pci/pci_host.h   |  34 +++
  include/sysemu/sysemu.h |   1 +
  32 files changed, 1508 insertions(+), 510 deletions(-)


I think this should not be committed to 2.3 after -rc0.

While I agree that looking at the above the series do not
fit for 2.3, I still think the risk is not big because:
- The biggest amount of modifications are in pci_bus/pci_host
  and *is only*  a movement of code into a new file,
  no changes at all (This cover almost half of the series changes)
- The series does not intervene with usual process, meaning if
  pxb is not used, it will work exactly like before
- The bios already supports pxb for this release, is a pity to wait
  for another QEMU version.
- The series were in mailing list for more than a month before rc,
  but the real review process started at rc time.
  (begging the question what a non-maintainer must do to get its code in time)
- The series is mature, rebased on the latest QEMU
  and was reviewed in depth and tested by the community.

The above being said, I'll agree with whatever the maintainers will decide.

Thanks,
Marcel



Paolo






Re: [Qemu-devel] [PATCH] spapr_pci: Fix unsafe signed/unsigned comparisons

2015-03-20 Thread Markus Armbruster
David Gibson  writes:

> spapr_pci.c contains a number of expressions of the form (uval == -1) or
> (uval != -1), where 'uval' is an unsigned value.
>
> This mostly works in practice, because as long as the width of uval is
> greater or equal than that of (int), the -1 will be promoted to the
> unsigned type, which is the expected outcome.
>
> However, at least for the cases where uval is uint32_t, this would break
> on platforms where sizeof(int) > 4 (and a few such do exist), because then
> the uint32_t value would be promoted to the larger int type, and never be
> equal to -1.

We may not care for portability to such systems.  However, a comparison
between signed and unsigned values still makes careful readers pause to
consider width.

gcc can warn (-Wsign-compare), but we don't enable this warning,
probably because we'd get too many of them.

> This patch fixes these errors.  The fixes for the (uint32_t) cases are
> necessary as described above.  I've made similar fixes to (uint64_t) and
> (hwaddr) cases.  Those are strictly theoretical, since I don't know of any
> platforms where sizeof(int) > 8, but hey, it's not that hard so we might
> as well be strictly C standard compliant.

It fixes all -Wsign-compare warnings from this file (but not from
included headers, but that's outside the scope of this patch).

> Reported-by: Markus Armbruster 
> Signed-off-by: David Gibson 

Reviewed-by: Markus Armbruster 



Re: [Qemu-devel] [PATCH 2/2] i6300esb: Fix signed integer overflow

2015-03-20 Thread Richard W.M. Jones
On Fri, Mar 20, 2015 at 02:11:56PM +1100, David Gibson wrote:
> If the guest programs a sufficiently large timeout value an integer
> overflow can occur in i6300esb_restart_timer().  e.g. if the maximum
> possible timer preload value of 0xf is programmed then we end up with
> the calculation:
> 
> timeout = get_ticks_per_sec() * (0xf << 15) / 3300;
> 
> get_ticks_per_sec() returns 10 (10^9) giving:
> 
>  10^9 * (0xf * 2^15) == 0x1dcd632329b00 (65 bits)
> 
> Obviously the division by 33MHz brings it back under 64-bits, but the
> overflow has already occurred.
> 
> Since signed integer overflow has undefined behaviour in C, in theory this
> could be arbitrarily bad.  In practice, the overflowed value wraps around
> to something negative, causing the watchdog to immediately expire, killing
> the guest, which is still fairly bad.
> 
> The bug can be triggered by running a Linux guest, loading the i6300esb
> driver with parameter "heartbeat=2046" and opening /dev/watchdog.  The
> watchdog will trigger as soon as the device is opened.
> 
> This patch corrects the problem by using an __int128_t temporary.  With
> suitable rearrangement of the calculations, I expect it would be possible
> to avoid the __int128_t.  But since we already use __int128_t extensively
> in the memory region code, and this is not a hot path, the super-wide
> integer seems like the simplest approach.
> 
> Signed-off-by: David Gibson 
> ---
>  hw/watchdog/wdt_i6300esb.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/watchdog/wdt_i6300esb.c b/hw/watchdog/wdt_i6300esb.c
> index e694fa9..11728af 100644
> --- a/hw/watchdog/wdt_i6300esb.c
> +++ b/hw/watchdog/wdt_i6300esb.c
> @@ -125,8 +125,14 @@ static void i6300esb_restart_timer(I6300State *d, int 
> stage)
>  else
>  timeout <<= 5;
>  
> -/* Get the timeout in units of ticks_per_sec. */
> -timeout = get_ticks_per_sec() * timeout / 3300;
> +/* Get the timeout in units of ticks_per_sec.
> + *
> + * ticks_per_sec is typically 10^9 == 0x3B9ACA00 (30 bits), with
> + * 20 bits of user supplied preload, and 15 bits of scale, the
> + * multiply here can exceed 64-bits, before we divide by 33MHz, so
> + * we use a 128-bit temporary
> + */
> +timeout = (__int128_t)get_ticks_per_sec() * timeout / 3300;
>  
>  i6300esb_debug("stage %d, timeout %" PRIi64 "\n", d->stage, timeout);
>  
> -- 
> 2.1.0

Reviewed-by: Richard W.M. Jones 

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/



Re: [Qemu-devel] [PATCH 1/3] block: add bdrv_get_device_or_node_name()

2015-03-20 Thread Markus Armbruster
Alberto Garcia  writes:

> On Fri, Mar 20, 2015 at 08:40:32AM +0100, Markus Armbruster wrote:
>
>> > +const char *bdrv_get_device_or_node_name(const BlockDriverState *bs)
>> > +{
>> > +return bs->blk ? blk_name(bs->blk) : bs->node_name;
>> > +}
>> > +
>> 
>> Does this have uses beyond identifying @bs to the user?
>
> None that I can think of, although apart from error messages it can
> also be used in data types, like the cases of BLOCK_IMAGE_CORRUPTED
> and BlockJobInfo that are being discussed:
>
>https://lists.gnu.org/archive/html/qemu-devel/2015-03/msg03651.html

Workable because BB and BDS names share a common name space.  But is it
a good idea?  No need to discuss this here if it's already discussed
elsewhere.

>> >  static void quorum_report_failure(QuorumAIOCB *acb)
>> >  {
>> > -const char *reference = bdrv_get_device_name(acb->common.bs)[0] ?
>> > -bdrv_get_device_name(acb->common.bs) :
>> > -acb->common.bs->node_name;
>> > -
>> > +const char *reference = bdrv_get_device_or_node_name(acb->common.bs);
>> >  qapi_event_send_quorum_failure(reference, acb->sector_num,
>> > acb->nb_sectors, &error_abort);
>> >  }
>> 
>> Preexisting: what if reference is null?
>
> It can't happen, both the device and the node name strings are
> guaranteed to be non-null. The latter is actually a static string so
> there's no chance bs->node_name returns a null pointer.

You're right, I missed the fact that BDS member node_name is an array.

Suggest to add a function comment to bdrv_get_device_or_node_name()
explaining intended use, and that it never returns null.



[Qemu-devel] [PATCH] vhost: logs sharing

2015-03-20 Thread Jason Wang
Currently we allocate one vhost log per vhost device. This is sub
optimal when:

- Guest has several device with vhost as backend
- Guest has multiqueue devices

In the above cases, we can avoid the memory allocation by sharing a
single vhost log among all the vhost devices. This is done by
introducing a global list for vhost logs and counting the reference
the usage.

Tested by doing scp during migration for a 2 queues virtio-net-pci.

Cc: Michael S. Tsirkin 
Signed-off-by: Jason Wang 
---
 hw/virtio/vhost.c | 67 ++-
 include/hw/virtio/vhost.h |  9 ++-
 2 files changed, 63 insertions(+), 13 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 5a12861..21d60cf 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -22,15 +22,20 @@
 #include "hw/virtio/virtio-bus.h"
 #include "migration/migration.h"
 
+static QLIST_HEAD(, vhost_log) vhost_logs =
+   QLIST_HEAD_INITIALIZER(vhost_logs);
+
 static void vhost_dev_sync_region(struct vhost_dev *dev,
   MemoryRegionSection *section,
   uint64_t mfirst, uint64_t mlast,
   uint64_t rfirst, uint64_t rlast)
 {
+vhost_log_chunk_t *log = dev->log->log;
+
 uint64_t start = MAX(mfirst, rfirst);
 uint64_t end = MIN(mlast, rlast);
-vhost_log_chunk_t *from = dev->log + start / VHOST_LOG_CHUNK;
-vhost_log_chunk_t *to = dev->log + end / VHOST_LOG_CHUNK + 1;
+vhost_log_chunk_t *from = log + start / VHOST_LOG_CHUNK;
+vhost_log_chunk_t *to = log + end / VHOST_LOG_CHUNK + 1;
 uint64_t addr = (start / VHOST_LOG_CHUNK) * VHOST_LOG_CHUNK;
 
 if (end < start) {
@@ -280,22 +285,55 @@ static uint64_t vhost_get_log_size(struct vhost_dev *dev)
 }
 return log_size;
 }
+static struct vhost_log *vhost_log_alloc(uint64_t size)
+{
+struct vhost_log *log = g_malloc0(sizeof *log + size * 
sizeof(*(log->log)));
+
+log->size = size;
+log->refcnt = 1;
+QLIST_INSERT_HEAD(&vhost_logs, log, node);
+
+return log;
+}
+
+static struct vhost_log *vhost_log_get(uint64_t size)
+{
+struct vhost_log *log;
+
+QLIST_FOREACH(log, &vhost_logs, node) {
+if (log->size == size) {
+++log->refcnt;
+return log;
+}
+}
+return vhost_log_alloc(size);
+}
+
+static void vhost_log_put(struct vhost_log *log)
+{
+if (!log)
+return;
+
+--log->refcnt;
+if (log->refcnt == 0) {
+QLIST_REMOVE(log, node);
+g_free(log);
+}
+}
 
 static inline void vhost_dev_log_resize(struct vhost_dev* dev, uint64_t size)
 {
-vhost_log_chunk_t *log;
-uint64_t log_base;
+struct vhost_log *log = vhost_log_get(size);
+uint64_t log_base = (uint64_t)(unsigned long)log->log;
 int r;
 
-log = g_malloc0(size * sizeof *log);
-log_base = (uint64_t)(unsigned long)log;
 r = dev->vhost_ops->vhost_call(dev, VHOST_SET_LOG_BASE, &log_base);
 assert(r >= 0);
 /* Sync only the range covered by the old log */
 if (dev->log_size) {
 vhost_log_sync_range(dev, 0, dev->log_size * VHOST_LOG_CHUNK - 1);
 }
-g_free(dev->log);
+vhost_log_put(dev->log);
 dev->log = log;
 dev->log_size = size;
 }
@@ -601,7 +639,7 @@ static int vhost_migration_log(MemoryListener *listener, 
int enable)
 if (r < 0) {
 return r;
 }
-g_free(dev->log);
+vhost_log_put(dev->log);
 dev->log = NULL;
 dev->log_size = 0;
 } else {
@@ -1058,9 +1096,11 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice 
*vdev)
 
 if (hdev->log_enabled) {
 hdev->log_size = vhost_get_log_size(hdev);
-hdev->log = hdev->log_size ?
-g_malloc0(hdev->log_size * sizeof *hdev->log) : NULL;
-r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_LOG_BASE, hdev->log);
+if (hdev->log_size) {
+hdev->log = vhost_log_get(hdev->log_size);
+}
+r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_LOG_BASE,
+hdev->log_size ? hdev->log->log : 
NULL);
 if (r < 0) {
 r = -errno;
 goto fail_log;
@@ -1069,6 +1109,9 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice 
*vdev)
 
 return 0;
 fail_log:
+if (hdev->log_size) {
+vhost_log_put(hdev->log);
+}
 fail_vq:
 while (--i >= 0) {
 vhost_virtqueue_stop(hdev,
@@ -1098,7 +1141,7 @@ void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice 
*vdev)
 vhost_log_sync_range(hdev, 0, ~0x0ull);
 
 hdev->started = false;
-g_free(hdev->log);
+vhost_log_put(hdev->log);
 hdev->log = NULL;
 hdev->log_size = 0;
 }
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index d5593d1..57dd849 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -28,6 +28,13 @@ typedef unsigned long vhost_log_chunk_t;
 #define VHOST_LO

Re: [Qemu-devel] [PATCH 1/2] i6300esb: Correct endiannness

2015-03-20 Thread Richard W.M. Jones
On Fri, Mar 20, 2015 at 02:11:55PM +1100, David Gibson wrote:
> The IO operations for the i6300esb watchdog timer are marked as
> DEVICE_NATIVE_ENDIAN.  This is not correct, and - as a PCI device - should
> be DEVICE_LITTLE_ENDIAN.
> 
> This allows i6300esb to work on ppc targets (yes, using an Intel ICH
> derived device on ppc is a bit odd, but the driver exists on the guest
> and there's no more obviously suitable watchdog device).
> 
> Signed-off-by: David Gibson 
> ---
>  hw/watchdog/wdt_i6300esb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/watchdog/wdt_i6300esb.c b/hw/watchdog/wdt_i6300esb.c
> index b2d158f..e694fa9 100644
> --- a/hw/watchdog/wdt_i6300esb.c
> +++ b/hw/watchdog/wdt_i6300esb.c
> @@ -369,7 +369,7 @@ static const MemoryRegionOps i6300esb_ops = {
>  i6300esb_mem_writel,
>  },
>  },
> -.endianness = DEVICE_NATIVE_ENDIAN,
> +.endianness = DEVICE_LITTLE_ENDIAN,
>  };
>  
>  static const VMStateDescription vmstate_i6300esb = {

Reviewed-by: Richard W.M. Jones 

 - - -

For your amusement, the physical device only ever existed as part of
an Intel "Enterprise SouthBridge" (hence ESB).  I highly doubt it was
ever connected to anything that was not an Intel x86 processor.  There
is a photo of one here:
http://www.tomshardware.com/reviews/amd,889-18.html [requires javascript]

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org



Re: [Qemu-devel] [PATCH 2/2] i6300esb: Fix signed integer overflow

2015-03-20 Thread Paolo Bonzini


On 20/03/2015 04:11, David Gibson wrote:
> If the guest programs a sufficiently large timeout value an integer
> overflow can occur in i6300esb_restart_timer().  e.g. if the maximum
> possible timer preload value of 0xf is programmed then we end up with
> the calculation:
> 
> timeout = get_ticks_per_sec() * (0xf << 15) / 3300;
> 
> get_ticks_per_sec() returns 10 (10^9) giving:
> 
>  10^9 * (0xf * 2^15) == 0x1dcd632329b00 (65 bits)
> 
> Obviously the division by 33MHz brings it back under 64-bits, but the
> overflow has already occurred.
> 
> Since signed integer overflow has undefined behaviour in C, in theory this
> could be arbitrarily bad.  In practice, the overflowed value wraps around
> to something negative, causing the watchdog to immediately expire, killing
> the guest, which is still fairly bad.
> 
> The bug can be triggered by running a Linux guest, loading the i6300esb
> driver with parameter "heartbeat=2046" and opening /dev/watchdog.  The
> watchdog will trigger as soon as the device is opened.
> 
> This patch corrects the problem by using an __int128_t temporary.  With
> suitable rearrangement of the calculations, I expect it would be possible
> to avoid the __int128_t.  But since we already use __int128_t extensively
> in the memory region code, and this is not a hot path, the super-wide
> integer seems like the simplest approach.

We don't use __int128_t, we use the Int128 struct---which however
doesn't have a multiplication function.  __int128_t is not available on
32-bit machines, and is only used under #ifdef CONFIG_INT128.

Instead, you can use muldiv64 which has exactly this purpose.

Paolo

> Signed-off-by: David Gibson 
> ---
>  hw/watchdog/wdt_i6300esb.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/watchdog/wdt_i6300esb.c b/hw/watchdog/wdt_i6300esb.c
> index e694fa9..11728af 100644
> --- a/hw/watchdog/wdt_i6300esb.c
> +++ b/hw/watchdog/wdt_i6300esb.c
> @@ -125,8 +125,14 @@ static void i6300esb_restart_timer(I6300State *d, int 
> stage)
>  else
>  timeout <<= 5;
>  
> -/* Get the timeout in units of ticks_per_sec. */
> -timeout = get_ticks_per_sec() * timeout / 3300;
> +/* Get the timeout in units of ticks_per_sec.
> + *
> + * ticks_per_sec is typically 10^9 == 0x3B9ACA00 (30 bits), with
> + * 20 bits of user supplied preload, and 15 bits of scale, the
> + * multiply here can exceed 64-bits, before we divide by 33MHz, so
> + * we use a 128-bit temporary
> + */
> +timeout = (__int128_t)get_ticks_per_sec() * timeout / 3300;
>  
>  i6300esb_debug("stage %d, timeout %" PRIi64 "\n", d->stage, timeout);
>  
> 



Re: [Qemu-devel] [PATCH 3/3] block: allow BLOCK_IMAGE_CORRUPTED to have a node name

2015-03-20 Thread Alberto Garcia
On Thu, Mar 19, 2015 at 05:14:24PM -0600, Eric Blake wrote:

> So the more I think about it, the more I'd like for this event to
> output both 'device' (mandatory, with an empty string if we can't
> easily tie the BDS to a single device) and 'node' (where 'node' can
> be optional, and omitted if the BDS does not have a node name [if we
> ever add generated node names, then we could make 'node' mandatory
> at that time]).

Ok, I will change that.

On a related note, the QUORUM_FAILURE event does have this mixed field
already, called "reference", which is the device name if present, else
the node name.

But I'm not planning to change it unless you think it's a good idea.
That would be a thing for a different patch anyway.

Berto



Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind

2015-03-20 Thread Ian Campbell
On Fri, 2015-03-20 at 09:04 +0800, Chen, Tiejun wrote:
> Refactor again,
> 
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 8599a6a..05c8916 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -409,6 +409,23 @@ static char *dm_spice_options(libxl__gc *gc,
>   return opt;
>   }
> 
> +static enum libxl_gfx_passthru_kind
> +libxl__detect_gfx_passthru_kind(libxl__gc *gc,
> +const libxl_domain_config *guest_config)
> +{
> +const libxl_domain_build_info *b_info = &guest_config->b_info;
> +
> +if (b_info->u.hvm.gfx_passthru_kind != LIBXL_GFX_PASSTHRU_KIND_DEFAULT)
> +return b_info->u.hvm.gfx_passthru_kind;
> +
> +if (libxl__is_igd_vga_passthru(gc, guest_config)) {
> +return LIBXL_GFX_PASSTHRU_KIND_IGD;
> +}
> +
> +LOG(ERROR, "Unable to detect graphics passthru kind");
> +return LIBXL_GFX_PASSTHRU_KIND_DEFAULT;
> +}
> +
>   static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>   const char *dm, int guest_domid,
>   const libxl_domain_config 
> *guest_config,
> @@ -757,6 +771,21 @@ static char ** 
> libxl__build_device_model_args_new(libxl__gc *gc,
>   machinearg, max_ram_below_4g);
>   }
>   }
> +
> +if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
> +enum libxl_gfx_passthru_kind gfx_passthru_kind =
> +libxl__detect_gfx_passthru_kind(gc, 
> guest_config);
> +switch (gfx_passthru_kind) {
> +case LIBXL_GFX_PASSTHRU_KIND_IGD:
> +machinearg = GCSPRINTF("%s,igd-passthru=on", machinearg);
> +break;
> +case LIBXL_GFX_PASSTHRU_KIND_DEFAULT:
> +LOG(ERROR, "unable to detect required gfx_passthru_kind");

In this case you will now have logged twice. I'd suggest logging only
here and not in the helper.

> +default:

And this case is subtly different to LIBXL_GFX_PASSTHRU_KIND_DEFAULT
since it would indicate some sort of corruption. So, I would drop the
logging on failure in libxl__detect_gfx_passthru_kind and here do:
   case LIBXL_GFX_PASSTHRU_KIND_DEFAULT:
   LOG(ERROR, "unable to detect required gfx_passthru_kind");
   return NULL;
   default:
   LOG(ERROR, "invalid value for gfx_passthru_kind");
   return NULL;

Ian




Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind

2015-03-20 Thread Chen, Tiejun

+case LIBXL_GFX_PASSTHRU_KIND_DEFAULT:
+LOG(ERROR, "unable to detect required gfx_passthru_kind");


In this case you will now have logged twice. I'd suggest logging only
here and not in the helper.


+default:


And this case is subtly different to LIBXL_GFX_PASSTHRU_KIND_DEFAULT
since it would indicate some sort of corruption. So, I would drop the
logging on failure in libxl__detect_gfx_passthru_kind and here do:
case LIBXL_GFX_PASSTHRU_KIND_DEFAULT:
LOG(ERROR, "unable to detect required gfx_passthru_kind");
return NULL;
default:
LOG(ERROR, "invalid value for gfx_passthru_kind");
return NULL;




Okay, I update this patch. If this is fine to you I'd like to send this 
whole series.


---
 tools/libxl/libxl.h  |  6 ++
 tools/libxl/libxl_dm.c   | 36 +---
 tools/libxl/libxl_internal.h |  4 
 tools/libxl/libxl_types.idl  |  6 ++
 tools/libxl/xl_cmdimpl.c | 23 +--
 5 files changed, 70 insertions(+), 5 deletions(-)

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 6bbc52d..62b3ae5 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -720,6 +720,12 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, 
libxl_mac *src);

 #define LIBXL_HAVE_PSR_MBM 1
 #endif

+/*
+ * libxl_domain_build_info has the u.hvm.gfx_passthru_kind field and
+ * the libxl_gfx_passthru_kind enumeration is defined.
+*/
+#define LIBXL_HAVE_GFX_PASSTHRU_KIND
+
 typedef char **libxl_string_list;
 void libxl_string_list_dispose(libxl_string_list *sl);
 int libxl_string_list_length(const libxl_string_list *sl);
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 8599a6a..bf72103 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -409,6 +409,22 @@ static char *dm_spice_options(libxl__gc *gc,
 return opt;
 }

+static enum libxl_gfx_passthru_kind
+libxl__detect_gfx_passthru_kind(libxl__gc *gc,
+const libxl_domain_config *guest_config)
+{
+const libxl_domain_build_info *b_info = &guest_config->b_info;
+
+if (b_info->u.hvm.gfx_passthru_kind != LIBXL_GFX_PASSTHRU_KIND_DEFAULT)
+return b_info->u.hvm.gfx_passthru_kind;
+
+if (libxl__is_igd_vga_passthru(gc, guest_config)) {
+return LIBXL_GFX_PASSTHRU_KIND_IGD;
+}
+
+return LIBXL_GFX_PASSTHRU_KIND_DEFAULT;
+}
+
 static char ** libxl__build_device_model_args_new(libxl__gc *gc,
 const char *dm, int guest_domid,
 const libxl_domain_config 
*guest_config,
@@ -710,9 +726,6 @@ static char ** 
libxl__build_device_model_args_new(libxl__gc *gc,

 flexarray_append(dm_args, "-net");
 flexarray_append(dm_args, "none");
 }
-if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
-flexarray_append(dm_args, "-gfx_passthru");
-}
 } else {
 if (!sdl && !vnc) {
 flexarray_append(dm_args, "-nographic");
@@ -757,6 +770,23 @@ static char ** 
libxl__build_device_model_args_new(libxl__gc *gc,

 machinearg, max_ram_below_4g);
 }
 }
+
+if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
+enum libxl_gfx_passthru_kind gfx_passthru_kind =
+libxl__detect_gfx_passthru_kind(gc, 
guest_config);

+switch (gfx_passthru_kind) {
+case LIBXL_GFX_PASSTHRU_KIND_IGD:
+machinearg = GCSPRINTF("%s,igd-passthru=on", machinearg);
+break;
+case LIBXL_GFX_PASSTHRU_KIND_DEFAULT:
+LOG(ERROR, "unable to detect required gfx_passthru_kind");
+return NULL;
+default:
+LOG(ERROR, "invalid value for gfx_passthru_kind");
+return NULL;
+}
+}
+
 flexarray_append(dm_args, machinearg);
 for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; 
i++)

 flexarray_append(dm_args, b_info->extra_hvm[i]);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index c97c62d..26a01cb 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3632,6 +3632,10 @@ static inline void 
libxl__update_config_vtpm(libxl__gc *gc,

  */
 void libxl__bitmap_copy_best_effort(libxl__gc *gc, libxl_bitmap *dptr,
 const libxl_bitmap *sptr);
+
+bool libxl__is_igd_vga_passthru(libxl__gc *gc,
+const libxl_domain_config *d_config);
+
 #endif

 /*
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 47af340..76642a8 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -140,6 +140,11 @@ libxl_tsc_mode = Enumeration("tsc_mode", [
 (3, "native_parav

Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind

2015-03-20 Thread Ian Campbell
On Fri, 2015-03-20 at 18:08 +0800, Chen, Tiejun wrote:
> +if (!xlu_cfg_get_string(config, "gfx_passthru_kind", &buf, 0)) {
> +if (libxl_gfx_passthru_kind_from_string(buf,
> + 
> &b_info->u.hvm.gfx_passthru_kind)) {
> +fprintf(stderr,
> +"ERROR: invalid value \"%s\" for 
> \"gfx_passthru_kind\"\n",
> +buf);
> +exit (1);
> +}
> +}

This unnecessary bit seems to have crept back in.

Don't forget to update the manpages if you haven't already.





Re: [Qemu-devel] [PATCH] object_del: Prevent removing an in-use memory backend object

2015-03-20 Thread Igor Mammedov
On Fri, 20 Mar 2015 12:14:58 +0800
Lin Ma  wrote:

> showing a memory device whose memdev is removed leads an assert:
> 
> (qemu) object_add memory-backend-ram,id=ram0,size=128M
> (qemu) device_add pc-dimm,id=d0,memdev=ram0
> (qemu) object_del ram0
> (qemu) info memory-devices
> **
> ERROR:qom/object.c:1274:object_get_canonical_path_component:\
> assertion failed: (obj->parent != NULL)
> Aborted
> 
> The patch prevents removing an in-use mem backend and outputs an error msg.
> 
> Signed-off-by: Lin Ma 
> ---
>  qmp.c | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/qmp.c b/qmp.c
> index c479e77..0086e2d 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -704,6 +704,7 @@ void qmp_object_del(const char *id, Error **errp)
>  {
>  Object *container;
>  Object *obj;
> +const char *typename;
>  
>  container = container_get(object_get_root(), "/objects");
>  obj = object_resolve_path_component(container, id);
> @@ -711,6 +712,19 @@ void qmp_object_del(const char *id, Error **errp)
>  error_setg(errp, "object id not found");
>  return;
>  }
> +
> +typename = object_class_get_name(object_class_get_parent(\
> +object_get_class(OBJECT(obj;
> +if (strcmp(typename, TYPE_MEMORY_BACKEND) == 0 ) {
> +MemoryRegion *mr;
> +mr = host_memory_backend_get_memory(MEMORY_BACKEND(obj), errp);
> +if (memory_region_is_mapped(mr)) {
> +char *path = object_get_canonical_path_component(obj);
> +error_setg(errp, "memdev %s is in used.", path);
> +g_free(path);
> +return;
> +}
> +}
How about making it more generic?
i.e
  if (!obj_class->can_be_deleted(obj))
 error out

>  object_unparent(obj);
>  }
>  




[Qemu-devel] [PATCH 3/3] block: add 'node-name' field to BLOCK_IMAGE_CORRUPTED

2015-03-20 Thread Alberto Garcia
Since this event can occur in nodes that cannot have a device name
associated, include also a field with the node name.

Signed-off-by: Alberto Garcia 
---
 block/qcow2.c   |  8 ++--
 docs/qmp/qmp-events.txt | 16 +---
 qapi/block-core.json| 13 -
 3 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 168006b..e7c78f1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2809,6 +2809,7 @@ void qcow2_signal_corruption(BlockDriverState *bs, bool 
fatal, int64_t offset,
  int64_t size, const char *message_format, ...)
 {
 BDRVQcowState *s = bs->opaque;
+const char *node_name;
 char *message;
 va_list ap;
 
@@ -2832,8 +2833,11 @@ void qcow2_signal_corruption(BlockDriverState *bs, bool 
fatal, int64_t offset,
 "corruption events will be suppressed\n", message);
 }
 
-qapi_event_send_block_image_corrupted(bdrv_get_device_name(bs), message,
-  offset >= 0, offset, size >= 0, size,
+node_name = bdrv_get_node_name(bs);
+qapi_event_send_block_image_corrupted(bdrv_get_device_name(bs),
+  *node_name != '\0', node_name,
+  message, offset >= 0, offset,
+  size >= 0, size,
   fatal, &error_abort);
 g_free(message);
 
diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
index d759d19..ed1d0a5 100644
--- a/docs/qmp/qmp-events.txt
+++ b/docs/qmp/qmp-events.txt
@@ -35,17 +35,19 @@ Emitted when a disk image is being marked corrupt.
 
 Data:
 
-- "device": Device name (json-string)
-- "msg":Informative message (e.g., reason for the corruption) (json-string)
-- "offset": If the corruption resulted from an image access, this is the access
-offset into the image (json-int)
-- "size":   If the corruption resulted from an image access, this is the access
-size (json-int)
+- "device":Device name (json-string)
+- "node-name": Node name, if it's present (json-string)
+- "msg":   Informative message (e.g., reason for the corruption)
+   (json-string)
+- "offset":If the corruption resulted from an image access, this
+   is the access offset into the image (json-int)
+- "size":  If the corruption resulted from an image access, this
+   is the access size (json-int)
 
 Example:
 
 { "event": "BLOCK_IMAGE_CORRUPTED",
-"data": { "device": "ide0-hd0",
+"data": { "device": "ide0-hd0", "node-name": "node0",
 "msg": "Prevented active L1 table overwrite", "offset": 196608,
 "size": 65536 },
 "timestamp": { "seconds": 1378126126, "microseconds": 966463 } }
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 42c8850..e650168 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1753,6 +1753,8 @@
 #
 # @device: device name
 #
+# @node-name: #optional node name (Since: 2.3)
+#
 # @msg: informative message for human consumption, such as the kind of
 #   corruption being detected. It should not be parsed by machine as it is
 #   not guaranteed to be stable
@@ -1770,11 +1772,12 @@
 # Since: 1.7
 ##
 { 'event': 'BLOCK_IMAGE_CORRUPTED',
-  'data': { 'device' : 'str',
-'msg': 'str',
-'*offset': 'int',
-'*size'  : 'int',
-'fatal'  : 'bool' } }
+  'data': { 'device' : 'str',
+'*node-name' : 'str',
+'msg': 'str',
+'*offset': 'int',
+'*size'  : 'int',
+'fatal'  : 'bool' } }
 
 ##
 # @BLOCK_IO_ERROR
-- 
2.1.4




Re: [Qemu-devel] [PATCH V4 09/19] virito: introduce bus specific queue limit

2015-03-20 Thread Cornelia Huck
On Wed, 18 Mar 2015 17:34:59 +0800
Jason Wang  wrote:

Typo in subject: s/virito/virtio/

> This patch introduces a bus specific queue limitation. It will be
> useful for increasing the limit for one of the bus without disturbing
> other buses.
> 
> Cc: Michael S. Tsirkin 
> Cc: Alexander Graf 
> Cc: Richard Henderson 
> Cc: Cornelia Huck 
> Cc: Christian Borntraeger 
> Cc: Paolo Bonzini 
> Signed-off-by: Jason Wang 
> ---
>  hw/char/virtio-serial-bus.c|  2 +-
>  hw/net/virtio-net.c|  4 ++--
>  hw/s390x/s390-virtio-bus.c |  1 +
>  hw/s390x/virtio-ccw.c  |  1 +
>  hw/scsi/virtio-scsi.c  |  4 ++--
>  hw/virtio/virtio-mmio.c|  1 +
>  hw/virtio/virtio-pci.c |  1 +
>  hw/virtio/virtio.c | 40 +---
>  include/hw/virtio/virtio-bus.h |  1 +
>  include/hw/virtio/virtio.h |  1 +
>  10 files changed, 36 insertions(+), 20 deletions(-)

> diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
> index 047c963..2b41e32 100644
> --- a/hw/s390x/s390-virtio-bus.c
> +++ b/hw/s390x/s390-virtio-bus.c
> @@ -715,6 +715,7 @@ static void virtio_s390_bus_class_init(ObjectClass 
> *klass, void *data)
>  bus_class->max_dev = 1;
>  k->notify = virtio_s390_notify;
>  k->get_features = virtio_s390_get_features;
> +k->queue_max = VIRTIO_PCI_QUEUE_MAX;

I'm wondering whether we should initialize queue_max to smth like
VIRTIO_QUEUE_MAX_DEFAULT (== 64) in virtio-bus.c instead and have the
individual classes override that value once they want to use a
different value?

But I'm fine with this patch as it stands as well.

>  }




[Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages

2015-03-20 Thread Alberto Garcia
There are several error messages that identify a BlockDriverState by
its device name. However those errors can be produced in nodes that
don't have a device name associated.

In those cases we should use bdrv_get_device_or_node_name() to fall
back to the node name and produce a more meaningful message. The
messages are also updated to use the more generic term 'node' instead
of 'device'.

Signed-off-by: Alberto Garcia 
---
 block.c   | 21 +++--
 block/qcow.c  |  4 ++--
 block/qcow2.c |  2 +-
 block/qed.c   |  2 +-
 block/vdi.c   |  2 +-
 block/vhdx.c  |  2 +-
 block/vmdk.c  |  4 ++--
 block/vpc.c   |  2 +-
 block/vvfat.c |  3 ++-
 blockdev.c|  4 ++--
 include/qapi/qmp/qerror.h |  8 
 11 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/block.c b/block.c
index 98b90b4..8247f0a 100644
--- a/block.c
+++ b/block.c
@@ -1224,8 +1224,8 @@ void bdrv_set_backing_hd(BlockDriverState *bs, 
BlockDriverState *backing_hd)
 bdrv_op_unblock_all(bs->backing_hd, bs->backing_blocker);
 } else if (backing_hd) {
 error_setg(&bs->backing_blocker,
-   "device is used as backing hd of '%s'",
-   bdrv_get_device_name(bs));
+   "node is used as backing hd of '%s'",
+   bdrv_get_device_or_node_name(bs));
 }
 
 bs->backing_hd = backing_hd;
@@ -1812,8 +1812,8 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, 
BlockReopenQueue *queue,
  * to r/w */
 if (!(reopen_state->bs->open_flags & BDRV_O_ALLOW_RDWR) &&
 reopen_state->flags & BDRV_O_RDWR) {
-error_set(errp, QERR_DEVICE_IS_READ_ONLY,
-  bdrv_get_device_name(reopen_state->bs));
+error_set(errp, QERR_NODE_IS_READ_ONLY,
+  bdrv_get_device_or_node_name(reopen_state->bs));
 goto error;
 }
 
@@ -1840,7 +1840,8 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, 
BlockReopenQueue *queue,
 /* It is currently mandatory to have a bdrv_reopen_prepare()
  * handler for each supported drv. */
 error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
-  drv->format_name, bdrv_get_device_name(reopen_state->bs),
+  drv->format_name,
+  bdrv_get_device_or_node_name(reopen_state->bs),
  "reopening of file");
 ret = -1;
 goto error;
@@ -3764,8 +3765,8 @@ void bdrv_add_key(BlockDriverState *bs, const char *key, 
Error **errp)
 {
 if (key) {
 if (!bdrv_is_encrypted(bs)) {
-error_setg(errp, "Device '%s' is not encrypted",
-  bdrv_get_device_name(bs));
+error_setg(errp, "Node '%s' is not encrypted",
+  bdrv_get_device_or_node_name(bs));
 } else if (bdrv_set_key(bs, key) < 0) {
 error_set(errp, QERR_INVALID_PASSWORD);
 }
@@ -3773,7 +3774,7 @@ void bdrv_add_key(BlockDriverState *bs, const char *key, 
Error **errp)
 if (bdrv_key_required(bs)) {
 error_set(errp, ERROR_CLASS_DEVICE_ENCRYPTED,
   "'%s' (%s) is encrypted",
-  bdrv_get_device_name(bs),
+  bdrv_get_device_or_node_name(bs),
   bdrv_get_encrypted_filename(bs));
 }
 }
@@ -5548,8 +5549,8 @@ bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType 
op, Error **errp)
 if (!QLIST_EMPTY(&bs->op_blockers[op])) {
 blocker = QLIST_FIRST(&bs->op_blockers[op]);
 if (errp) {
-error_setg(errp, "Device '%s' is busy: %s",
-   bdrv_get_device_name(bs),
+error_setg(errp, "Node '%s' is busy: %s",
+   bdrv_get_device_or_node_name(bs),
error_get_pretty(blocker->reason));
 }
 return true;
diff --git a/block/qcow.c b/block/qcow.c
index 0558969..d4287ca 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -124,7 +124,7 @@ static int qcow_open(BlockDriverState *bs, QDict *options, 
int flags,
 snprintf(version, sizeof(version), "QCOW version %" PRIu32,
  header.version);
 error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
-  bdrv_get_device_name(bs), "qcow", version);
+  bdrv_get_device_or_node_name(bs), "qcow", version);
 ret = -ENOTSUP;
 goto fail;
 }
@@ -231,7 +231,7 @@ static int qcow_open(BlockDriverState *bs, QDict *options, 
int flags,
 /* Disable migration when qcow images are used */
 error_set(&s->migration_blocker,
   QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
-  "qcow", bdrv_get_device_name(bs), "live migration");
+  "qcow", bdrv_get_device_or_node_name(bs), "live migration");
 migrate_add_blocker(s->migration_blocker);
 
 qemu_co_mutex_init(&s->lock);
diff 

[Qemu-devel] [PATCH v2 0/3] Add bdrv_get_device_or_node_name()

2015-03-20 Thread Alberto Garcia
Second version of the series.

v2:
- bdrv_get_device_or_node_name() includes a comment explaining its
  usage.
- The error messages have been updated to say 'node' instead of
  'device' where appropriate.
- The BLOCK_IMAGE_CORRUPTED event has a new 'node-name' field.

Regards,

Berto

Alberto Garcia (3):
  block: add bdrv_get_device_or_node_name()
  block: use bdrv_get_device_or_node_name() in error messages
  block: add 'node-name' field to BLOCK_IMAGE_CORRUPTED

 block.c   | 30 --
 block/qcow.c  |  4 ++--
 block/qcow2.c | 10 +++---
 block/qed.c   |  2 +-
 block/quorum.c|  5 +
 block/vdi.c   |  2 +-
 block/vhdx.c  |  2 +-
 block/vmdk.c  |  4 ++--
 block/vpc.c   |  2 +-
 block/vvfat.c |  3 ++-
 blockdev.c|  4 ++--
 docs/qmp/qmp-events.txt   | 16 +---
 include/block/block.h |  1 +
 include/qapi/qmp/qerror.h |  8 
 qapi/block-core.json  | 13 -
 15 files changed, 62 insertions(+), 44 deletions(-)

-- 
2.1.4




[Qemu-devel] [PATCH 1/3] block: add bdrv_get_device_or_node_name()

2015-03-20 Thread Alberto Garcia
This function gets the device name associated with a BlockDriverState,
or its node name if the device name is empty.

Signed-off-by: Alberto Garcia 
---
 block.c   | 9 +
 block/quorum.c| 5 +
 include/block/block.h | 1 +
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/block.c b/block.c
index 0fe97de..98b90b4 100644
--- a/block.c
+++ b/block.c
@@ -3920,6 +3920,15 @@ const char *bdrv_get_device_name(const BlockDriverState 
*bs)
 return bs->blk ? blk_name(bs->blk) : "";
 }
 
+/* This can be used to identify nodes that might not have a device
+ * name associated. Since node and device names live in the same
+ * namespace, the result is unambiguous. The exception is if both are
+ * absent, then this returns an empty (non-null) string. */
+const char *bdrv_get_device_or_node_name(const BlockDriverState *bs)
+{
+return bs->blk ? blk_name(bs->blk) : bs->node_name;
+}
+
 int bdrv_get_flags(BlockDriverState *bs)
 {
 return bs->open_flags;
diff --git a/block/quorum.c b/block/quorum.c
index 437b122..f91ef75 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -226,10 +226,7 @@ static void quorum_report_bad(QuorumAIOCB *acb, char 
*node_name, int ret)
 
 static void quorum_report_failure(QuorumAIOCB *acb)
 {
-const char *reference = bdrv_get_device_name(acb->common.bs)[0] ?
-bdrv_get_device_name(acb->common.bs) :
-acb->common.bs->node_name;
-
+const char *reference = bdrv_get_device_or_node_name(acb->common.bs);
 qapi_event_send_quorum_failure(reference, acb->sector_num,
acb->nb_sectors, &error_abort);
 }
diff --git a/include/block/block.h b/include/block/block.h
index 4c57d63..b285e0d 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -398,6 +398,7 @@ void bdrv_iterate_format(void (*it)(void *opaque, const 
char *name),
  void *opaque);
 const char *bdrv_get_node_name(const BlockDriverState *bs);
 const char *bdrv_get_device_name(const BlockDriverState *bs);
+const char *bdrv_get_device_or_node_name(const BlockDriverState *bs);
 int bdrv_get_flags(BlockDriverState *bs);
 int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num,
   const uint8_t *buf, int nb_sectors);
-- 
2.1.4




Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind

2015-03-20 Thread Chen, Tiejun

On 2015/3/20 18:11, Ian Campbell wrote:

On Fri, 2015-03-20 at 18:08 +0800, Chen, Tiejun wrote:

+if (!xlu_cfg_get_string(config, "gfx_passthru_kind", &buf, 0)) {
+if (libxl_gfx_passthru_kind_from_string(buf,
+
&b_info->u.hvm.gfx_passthru_kind)) {
+fprintf(stderr,
+"ERROR: invalid value \"%s\" for
\"gfx_passthru_kind\"\n",
+buf);
+exit (1);
+}
+}


This unnecessary bit seems to have crept back in.


Sorry to hit this again when I address other comments.



Don't forget to update the manpages if you haven't already.



Looks I need to update docs/man/xl.cfg.pod.5.

Thanks
Tiejun



Re: [Qemu-devel] [PATCH v3 for-2.3 0/2] numa: Fix default VCPUs to node mapping

2015-03-20 Thread Igor Mammedov
On Thu, 19 Mar 2015 17:09:20 +
Igor Mammedov  wrote:

> Changes since v2:
>  - fix spelling errors
>  - split out PC hunks itno a separate patch
> 
> Igor Mammedov (2):
>   numa: introduce machine callback for VCPU to node mapping
>   pc: fix default VCPU to NUMA node mapping
> 
>  hw/i386/pc.c  |  9 +
>  include/hw/boards.h   |  5 +
>  include/sysemu/numa.h |  3 ++-
>  numa.c| 18 +-
>  vl.c  |  2 +-
>  5 files changed, 30 insertions(+), 7 deletions(-)
> 

CCing stable which I've forgotten to do earlier



Re: [Qemu-devel] [PULL for-2.3 v2 00/13] usb patch queue

2015-03-20 Thread Peter Maydell
On 20 March 2015 at 08:11, Gerd Hoffmann  wrote:
>   Hi,
>
> Here is the usb patch queue for 2.3-rc1.  Grow a bit larger than I'd
> like it to be.  But I've somehow missed I had patches lingering in the
> usb queue while preparing the -rc0 pulls, and there also have been some
> bugfixes coming in pretty late.
>
> New in v2: added missing signed-off.  No code changes, so I don't spam
> the list with a repost of the patches.
>
> please pull,
>   Gerd
>
> The following changes since commit 5a4992834daec85c3913654903fb9f4f954e585a:
>
>   Merge remote-tracking branch 
> 'remotes/armbru/tags/pull-cov-model-2015-03-17' into staging (2015-03-17 
> 11:43:00 +)
>
> are available in the git repository at:
>
>
>   git://git.kraxel.org/qemu tags/pull-usb-20150320-1
>
> for you to fetch changes up to 4e289b1b62c8e271e3400317b4c3d98909093bc4:
>
>   ehci: fix segfault when hot-unplugging ehci controller (2015-03-20 08:50:12 
> +0100)
>
> 
> usb: bugfix collection.
>
> 

Applied, thanks.

-- PMM



Re: [Qemu-devel] [PATCH v4 5/5] Qemu-Xen-vTPM: QEMU machine class is initialized before tpm_init()

2015-03-20 Thread Stefan Berger

On 03/10/2015 08:14 AM, Quan Xu wrote:

make sure QEMU machine class is initialized and QEMU has registered
Xen stubdom vTPM driver when call tpm_init()

Signed-off-by: Quan Xu 
---
  vl.c | 17 +++--
  1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/vl.c b/vl.c
index f6b3546..0bbdaa1 100644
--- a/vl.c
+++ b/vl.c
@@ -4114,12 +4114,6 @@ int main(int argc, char **argv, char **envp)
  exit(1);
  }

-#ifdef CONFIG_TPM
-if (tpm_init() < 0) {
-exit(1);
-}
-#endif
-
  /* init the bluetooth world */
  if (foreach_device_config(DEV_BT, bt_parse))
  exit(1);
@@ -4225,6 +4219,17 @@ int main(int argc, char **argv, char **envp)
  exit(1);
  }

+/*
+ * For compatible with Xen stubdom vTPM driver, make
+ * sure QEMU machine class is initialized and QEMU has
+ * registered Xen stubdom vTPM driver.
+ */
+#ifdef CONFIG_TPM
+if (tpm_init() < 0) {
+exit(1);
+}
+#endif
+
  /* init generic devices */
  if (qemu_opts_foreach(qemu_find_opts("device"), device_init_func, NULL, 
1) != 0)
  exit(1);

Reviewed-by: Stefan Berger 




Re: [Qemu-devel] [PATCH V4 10/19] virtio-ccw: introduce ccw specific queue limit

2015-03-20 Thread Cornelia Huck
On Wed, 18 Mar 2015 17:35:00 +0800
Jason Wang  wrote:

> Instead of depending on marco, using a bus specific limit.

You should probably also mention the adapter routes change in your
changelog:

"Also make it clear that the number of gsis per I/O adapter is not
directly depending on the number of virtio queues, but rather the other
way around."

> 
> Cc: Alexander Graf 
> Cc: Cornelia Huck 
> Cc: Christian Borntraeger 
> Cc: Richard Henderson 
> Signed-off-by: Jason Wang 
> ---
>  hw/s390x/s390-virtio-ccw.c   |  7 +--
>  hw/s390x/virtio-ccw.c| 19 ---
>  include/hw/s390x/s390_flic.h |  4 +++-
>  3 files changed, 20 insertions(+), 10 deletions(-)
> 

Looks good to me.




Re: [Qemu-devel] [PATCH V4 11/19] virtio-s390: switch to bus specific queue limit

2015-03-20 Thread Cornelia Huck
On Wed, 18 Mar 2015 17:35:01 +0800
Jason Wang  wrote:

> Instead of depending on marco, switch to use a bus specific queue
> limit.
> 
> Cc: Alexander Graf 
> Cc: Richard Henderson 
> Cc: Christian Borntraeger 
> Cc: Cornelia Huck 
> Signed-off-by: Jason Wang 
> ---
>  hw/s390x/s390-virtio-bus.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)

Looks good to me.




Re: [Qemu-devel] [PATCH] spapr_pci: Fix unsafe signed/unsigned comparisons

2015-03-20 Thread Alexander Graf


On 19.03.15 05:14, David Gibson wrote:
> spapr_pci.c contains a number of expressions of the form (uval == -1) or
> (uval != -1), where 'uval' is an unsigned value.
> 
> This mostly works in practice, because as long as the width of uval is
> greater or equal than that of (int), the -1 will be promoted to the
> unsigned type, which is the expected outcome.
> 
> However, at least for the cases where uval is uint32_t, this would break
> on platforms where sizeof(int) > 4 (and a few such do exist), because then
> the uint32_t value would be promoted to the larger int type, and never be
> equal to -1.
> 
> This patch fixes these errors.  The fixes for the (uint32_t) cases are
> necessary as described above.  I've made similar fixes to (uint64_t) and
> (hwaddr) cases.  Those are strictly theoretical, since I don't know of any
> platforms where sizeof(int) > 8, but hey, it's not that hard so we might
> as well be strictly C standard compliant.
> 
> Reported-by: Markus Armbruster 
> Signed-off-by: David Gibson 

Thanks, applied to ppc-next-2.4.


Alex



Re: [Qemu-devel] [PATCH V4 14/19] virtio: introduce vector to virtqueues mapping

2015-03-20 Thread Cornelia Huck
On Wed, 18 Mar 2015 17:35:04 +0800
Jason Wang  wrote:

> Currently we will try to traverse all virtqueues to find a subset that
> using a specific vector. This is sub optimal when we will support
> hundreds or even thousands of virtqueues. So this patch introduces a
> method which could be used by transport to get all virtqueues that
> using a same vector. This is done through QLISTs and the number of
> QLISTs was queried through a transport specific method. When guest
> setting vectors, the virtqueue will be linked and helpers for traverse
> the list was also introduced.
> 
> The first user will be virtio pci which will use this to speed up
> MSI-X masking and unmasking handling.
> 
> Cc: Michael S. Tsirkin 
> Signed-off-by: Jason Wang 
> ---
>  hw/virtio/virtio-pci.c |  8 
>  hw/virtio/virtio.c | 32 ++--
>  include/hw/virtio/virtio-bus.h |  1 +
>  include/hw/virtio/virtio.h |  3 +++
>  4 files changed, 42 insertions(+), 2 deletions(-)

I'm still not too happy introducing this overhead for all devices when
only pci will make use of it.

Could we perhaps make the queue handling dependant on whether the
transport actually provides the query_nvectors callback?




Re: [Qemu-devel] [Xen-devel] [PATCH] SeaBios/vTPM: Enable Xen stubdom vTPM for HVM virtual machine

2015-03-20 Thread Stefan Berger

On 03/19/2015 08:56 AM, Ian Campbell wrote:

On Tue, 2015-03-10 at 08:16 -0400, Quan Xu wrote:

@@ -151,6 +152,8 @@ device_hardware_setup(void)
  esp_scsi_setup();
  megasas_setup();
  pvscsi_setup();
+if (runningOnXen())
+vtpm4hvm_setup();

Is there anything which is actually Xen specific about the driver in
tpm.[ch]? Would it be better to just probe for it, perhaps gates by a
Kconfig option which enables TPM support.


I also think the probing should be done. That code can also be recycled 
from what I posted earlier. It's gated by a Kconfig option, so it 
doesn't fill up the 128k ROM.


Stefan



And following that train of thought I think you could reasonable drop
"4hvm" from the name. And possibly even the leading "v", since I suppose
seabios shouldn't really care if the tpm is emulated or real so long as
it looks like a real tpm.

Ian.






Re: [Qemu-devel] [Xen-devel] [PATCH] SeaBios/vTPM: Enable Xen stubdom vTPM for HVM virtual machine

2015-03-20 Thread Stefan Berger

On 03/19/2015 09:35 PM, Xu, Quan wrote:



-Original Message-
From: Ian Campbell [mailto:ian.campb...@citrix.com]
Sent: Thursday, March 19, 2015 8:57 PM
To: Xu, Quan
Cc: ke...@koconnor.net; stef...@linux.vnet.ibm.com; xen-de...@lists.xen.org;
qemu-devel@nongnu.org; stefano.stabell...@eu.citrix.com
Subject: Re: [Xen-devel] [PATCH] SeaBios/vTPM: Enable Xen stubdom vTPM for
HVM virtual machine

On Tue, 2015-03-10 at 08:16 -0400, Quan Xu wrote:

@@ -151,6 +152,8 @@ device_hardware_setup(void)
  esp_scsi_setup();
  megasas_setup();
  pvscsi_setup();
+if (runningOnXen())
+vtpm4hvm_setup();

Is there anything which is actually Xen specific about the driver in tpm.[ch]?
Would it be better to just probe for it, perhaps gates by a Kconfig option which
enables TPM support.

And following that train of thought I think you could reasonable drop "4hvm"
from the name. And possibly even the leading "v", since I suppose seabios
shouldn't really care if the tpm is emulated or real so long as it looks like a 
real
tpm.

Ian.

Thanks for your review. Make sense.

Quan


From previously posted patches you should be able to take 1/8 with the 
driver:


http://www.seabios.org/pipermail/seabios/2014-July/008179.html

2/8 added ACPI support, but this is not necessary; it contained the 
probing parts, which could be merged into 1.


3/8 was doing TPM initialization, so that should be recyclable as well: 
http://www.seabios.org/pipermail/seabios/2014-July/008180.html


I have a set of patches that removes the ACPI part in 2/8, though didn't 
post it.


   Stefan




Re: [Qemu-devel] [PULL 0/6] NUMA queue 2015-03-19

2015-03-20 Thread Peter Maydell
On 19 March 2015 at 19:26, Eduardo Habkost  wrote:
> The following changes since commit 3e5f6234b4f45a11b7c357dde2d6da36641bc6f6:
>
>   Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging 
> (2015-03-19 17:47:08 +)
>
> are available in the git repository at:
>
>   https://github.com/ehabkost/qemu.git tags/work/numa-verify-cpus-pull-request
>
> for you to fetch changes up to 549fc54b8cfe16a475d8f6b8f838e53b45452b4a:
>
>   numa: Print warning if no node is assigned to a CPU (2015-03-19 16:20:15 
> -0300)
>
> 
> NUMA queue 2015-03-19
>
> 
>

Applied, thanks.

-- PMM



Re: [Qemu-devel] [PATCH v5 20/45] Modify savevm handlers for postcopy

2015-03-20 Thread Dr. David Alan Gilbert
* David Gibson (da...@gibson.dropbear.id.au) wrote:
> On Fri, Mar 13, 2015 at 10:19:54AM +, Dr. David Alan Gilbert wrote:
> > * David Gibson (da...@gibson.dropbear.id.au) wrote:
> > > On Wed, Feb 25, 2015 at 04:51:43PM +, Dr. David Alan Gilbert (git) 
> > > wrote:
> > > > From: "Dr. David Alan Gilbert" 
> > > > 
> > > > Modify save_live_pending to return separate postcopiable and
> > > > non-postcopiable counts.
> > > > 
> > > > Add 'can_postcopy' to allow a device to state if it can postcopy
> > > 
> > > What's the purpose of the can_postcopy callback?  There are no callers
> > > in this patch - is it still necessary with the change to
> > > save_live_pending?
> > 
> > The patch 'qemu_savevm_state_complete: Postcopy changes' uses
> > it in qemu_savevm_state_postcopy_complete and qemu_savevm_state_complete
> > to decide which devices must be completed at that point.
> 
> Couldn't they check for non-zero postcopiable state from
> save_live_pending instead?

That would be a bit weird.

At the moment for each device we call the:
   save_live_setup method (from qemu_savevm_state_begin)

   0...multiple times we call:
   save_live_pending
   save_live_iterate

   and then we always call
   save_live_complete


To my mind we have to call save_live_complete for any device
that we've called save_live_setup on (maybe it allocated something
in _setup that it clears up in _complete).

save_live_pending could perfectly well return 0 remaining at the end of
the migrate for our device, and thus if we used that then we wouldn't
call save_live_complete.

Dave

> 
> -- 
> David Gibson  | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au| minimalist, thank you.  NOT _the_ 
> _other_
>   | _way_ _around_!
> http://www.ozlabs.org/~dgibson


--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [PATCH v5 10/13] blockdev: Keep track of monitor-owned BDS

2015-03-20 Thread Max Reitz

On 2015-03-20 at 04:04, Markus Armbruster wrote:

Eric Blake  writes:


On 03/03/2015 01:13 PM, Max Reitz wrote:

Signed-off-by: Max Reitz 
---
  block.c|  2 ++
  blockdev.c | 18 ++
  include/block/block_int.h  |  4 
  stubs/Makefile.objs|  1 +
  stubs/blockdev-close-all-bdrv-states.c |  5 +
  5 files changed, 30 insertions(+)
  create mode 100644 stubs/blockdev-close-all-bdrv-states.c

Again, might be nice for the commit message to document why adding this
is useful, but doesn't affect the code.

Reviewed-by: Eric Blake 

Might be nice?  This absolutely needs an explanation, in the code!

Why do we need a separate list of "monitor-owned BDS"?


I thought it self-evident, but you're right, of course: The concept 
being that the monitor can hold references to BDS ("owning them", see 
below), of course it should keep track of those references (which I 
thought didn't need an explanation, because if you hold a reference, 
well, you better actually hold one).



What makes a BDS "monitor-owned"?


Being created explicitly.

BDS created implicitly because a BB was created (-drive, blockdev-add 
with @id; in these cases, the BBs are monitor-owned) or that are not the 
root of a BDS tree (thus created implicitly in that tree) are not 
monitor-owned. All BDS created explicitly (blockdev-add without @id) are 
monitor-owned.



What are the invariants governing relations among bdrv_states,
graph_bdrv_states (what a horrible name) and blk_backends?


bdrv_states is removed in the follow-up series. I think it's legacy 
cruft which we needed at a time where we did not have BlockBackends: It 
tracks all the BDSs with a BB, basically (a BDS is inserted there in 
bdrv_new_root() which is only called from blk_new_with_bs()). Therefore, 
we don't need it, as we have a list of all the BBs already.


graph_bdrv_states tracks all BDSs with a node name. We need that because 
we want to find BDSs by node name.


blk_backends, which supersedes bdrv_states, basically, will, after the 
follow-up, contain every single BlockBackend there is. It will be 
different from monitor_block_backends (follow-up...) in that the latter 
only contains the monitor-owned BBs.


Why we need that difference is a question for the follow-up. However, I 
can answer it here, too: We do have some operations that should actually 
be run over all BlockBackends, like blk_name_taken(), blk_drain_all(), 
blk_flush_all(), and so on. However, all the BBs that are not 
monitor-owned should not be visible to the user (the monitor!), so for 
operations like blk_by_name() or qmp_query_blockstats(), only the 
monitor-owned BBs should be considered.


What makes a BB monitor-owned? It's monitor-owned if it has been created 
using -drive or blockdev-add. Are there any other BBs right now? Except 
for qemu-{img,io,nbd}, there is only xen_disk.c, so practically, no.


What makes a BB monitor-unowned? Deleting its reference through a 
monitor command while there are still other users (in theory that would 
be NBD, block jobs, etc., although I don't know whether that actually 
works that way right now). So it is probably possible to have 
non-monitor-owned BBs, but those should not be visible to the user. 
However, we need to consider them inside of the block layer whenever we 
want to do something for actually all the BBs.


Side note: As far as I know, we currently only have drive_del to drop 
the monitor reference to a BB. What happens there before the follow-up 
is the BB is made anonymous and removed from blk_backends 
(blk_hide_on_behalf_of_hmp_drive_del()) and the root BDS from 
bdrv_states (it's really redundant), so before that series, blk_backends 
is basically the list of monitor-owned BBs (which I don't think is 
right, it should contain all BBs, and we need a separate list for the 
monitor-owned ones).


I hope I was able to explain myself.

Max



Re: [Qemu-devel] [PATCH 1/3] block: add bdrv_get_device_or_node_name()

2015-03-20 Thread Max Reitz

On 2015-03-20 at 06:19, Alberto Garcia wrote:

This function gets the device name associated with a BlockDriverState,
or its node name if the device name is empty.

Signed-off-by: Alberto Garcia 
---
  block.c   | 9 +
  block/quorum.c| 5 +
  include/block/block.h | 1 +
  3 files changed, 11 insertions(+), 4 deletions(-)


Reviewed-by: Max Reitz 



[Qemu-devel] [PATCH RFC for-2.3 0/1] block: New command line option --no-format-probing

2015-03-20 Thread Markus Armbruster
First of all, my apologies for being so late with this.  I realized
part way through the current development cycle that I couldn't do both
the error work and my half of the block probing work we discussed back
in November, so I punted the latter to the next cycle, missing the one
little feature I quite obviously could do.

Why is this "RFC for-2.3"?  The patch is simple, and quite obviously
does nothing unless you run with --no-format-probing.  If libvirt
wants it in 2.3, then I think we should consider it even at this late
stage.  If libvirt doesn't want it, or won't try to make use of it for
another few months, I'm happy to drop the patch now and revisit the
larger topic in the next cycle.

I readily admit --no-format-probing is fugly.  Better ideas are
welcome.

Markus Armbruster (1):
  block: New command line option --no-format-probing

 block.c   |  9 -
 include/block/block.h |  2 +-
 qemu-options.hx   | 12 
 vl.c  |  6 +-
 4 files changed, 26 insertions(+), 3 deletions(-)

-- 
1.9.3




[Qemu-devel] [PATCH RFC for-2.3 1/1] block: New command line option --no-format-probing

2015-03-20 Thread Markus Armbruster
Probing is convenient, but probing untrusted raw images is insecure
(CVE-2008-2004).  To avoid it, users should always specify raw format
explicitly.  This isn't trivial, and even sophisticated users have
gotten it wrong (libvirt CVE-2010-2237, CVE-2010-2238, CVE-2010-2239,
plus more recent variations of the theme that didn't get CVEs because
they were caught before they could hurt users).

Disabling probing entirely is a (hamfisted) way to ensure you always
specify the format.

Unfortunately, the new option is not available with -readconfig.
There's no obvious option group to take it.  I think we could use a
"miscellaneous" option group.

Signed-off-by: Markus Armbruster 
---
 block.c   |  9 -
 include/block/block.h |  2 +-
 qemu-options.hx   | 12 
 vl.c  |  6 +-
 4 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 0fe97de..5865309 100644
--- a/block.c
+++ b/block.c
@@ -103,6 +103,7 @@ static void bdrv_reset_dirty(BlockDriverState *bs, int64_t 
cur_sector,
  int nr_sectors);
 /* If non-zero, use only whitelisted block drivers */
 static int use_bdrv_whitelist;
+static bool bdrv_image_probing_disabled;
 
 #ifdef _WIN32
 static int is_windows_drive_prefix(const char *filename)
@@ -751,6 +752,11 @@ static int find_image_format(BlockDriverState *bs, const 
char *filename,
 return ret;
 }
 
+if (bdrv_image_probing_disabled) {
+error_setg(errp, "Format not specified and image probing disabled");
+return -EINVAL;
+}
+
 ret = bdrv_pread(bs, 0, buf, sizeof(buf));
 if (ret < 0) {
 error_setg_errno(errp, -ret, "Could not read image for determining its 
"
@@ -4909,9 +4915,10 @@ void bdrv_init(void)
 module_call_init(MODULE_INIT_BLOCK);
 }
 
-void bdrv_init_with_whitelist(void)
+void bdrv_init_with_whitelist(bool no_format_probing)
 {
 use_bdrv_whitelist = 1;
+bdrv_image_probing_disabled = no_format_probing;
 bdrv_init();
 }
 
diff --git a/include/block/block.h b/include/block/block.h
index 4c57d63..b5a8b23 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -177,7 +177,7 @@ void bdrv_io_limits_enable(BlockDriverState *bs);
 void bdrv_io_limits_disable(BlockDriverState *bs);
 
 void bdrv_init(void);
-void bdrv_init_with_whitelist(void);
+void bdrv_init_with_whitelist(bool no_format_probing);
 BlockDriver *bdrv_find_protocol(const char *filename,
 bool allow_protocol_prefix,
 Error **errp);
diff --git a/qemu-options.hx b/qemu-options.hx
index 319d971..8aa4d7b 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -963,6 +963,18 @@ STEXI
 Disable SDL window close capability.
 ETEXI
 
+DEF("no-format-probing", 0, QEMU_OPTION_no_format_probing,
+"-no-format-probing\n"
+"disable block image format probing\n", QEMU_ARCH_ALL)
+STEXI
+@item -no-format-probing
+@findex -no-format-probing
+Disable block image format probing.  Probing is convenient, but
+probing untrusted raw images is insecure.  To avoid it, always specify
+raw format explicitly.  Disabling probing entirely is a (hamfisted)
+way to ensure you do.
+ETEXI
+
 DEF("sdl", 0, QEMU_OPTION_sdl,
 "-sdlenable SDL\n", QEMU_ARCH_ALL)
 STEXI
diff --git a/vl.c b/vl.c
index 75ec292..94d5e15 100644
--- a/vl.c
+++ b/vl.c
@@ -2754,6 +2754,7 @@ int main(int argc, char **argv, char **envp)
 #endif
 bool defconfig = true;
 bool userconfig = true;
+bool no_format_probing = false;
 const char *log_mask = NULL;
 const char *log_file = NULL;
 GMemVTable mem_trace = {
@@ -2823,7 +2824,7 @@ int main(int argc, char **argv, char **envp)
 
 nb_nics = 0;
 
-bdrv_init_with_whitelist();
+bdrv_init_with_whitelist(no_format_probing);
 
 autostart = 1;
 
@@ -3381,6 +3382,9 @@ int main(int argc, char **argv, char **envp)
 case QEMU_OPTION_no_quit:
 no_quit = 1;
 break;
+case QEMU_OPTION_no_format_probing:
+no_format_probing = true;
+break;
 case QEMU_OPTION_sdl:
 #ifdef CONFIG_SDL
 display_type = DT_SDL;
-- 
1.9.3




Re: [Qemu-devel] [PULL 0/2] X86 queue 2015-03-19

2015-03-20 Thread Peter Maydell
On 19 March 2015 at 19:40, Eduardo Habkost  wrote:
> The following changes since commit 3e5f6234b4f45a11b7c357dde2d6da36641bc6f6:
>
>   Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging 
> (2015-03-19 17:47:08 +)
>
> are available in the git repository at:
>
>   https://github.com/ehabkost/qemu.git tags/x86-pull-request
>
> for you to fetch changes up to a356850b80b3d13b2ef737dad2acb05e6da03753:
>
>   target-i386: Haswell-noTSX and Broadwell-noTSX (2015-03-19 16:35:14 -0300)
>
> 
> X86 queue 2015-03-19
>
> 

Applied, thanks.

-- PMM



Re: [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages

2015-03-20 Thread Max Reitz

On 2015-03-20 at 06:19, Alberto Garcia wrote:

There are several error messages that identify a BlockDriverState by
its device name. However those errors can be produced in nodes that
don't have a device name associated.

In those cases we should use bdrv_get_device_or_node_name() to fall
back to the node name and produce a more meaningful message. The
messages are also updated to use the more generic term 'node' instead
of 'device'.

Signed-off-by: Alberto Garcia 
---
  block.c   | 21 +++--
  block/qcow.c  |  4 ++--
  block/qcow2.c |  2 +-
  block/qed.c   |  2 +-
  block/vdi.c   |  2 +-
  block/vhdx.c  |  2 +-
  block/vmdk.c  |  4 ++--
  block/vpc.c   |  2 +-
  block/vvfat.c |  3 ++-
  blockdev.c|  4 ++--
  include/qapi/qmp/qerror.h |  8 
  11 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/block.c b/block.c
index 98b90b4..8247f0a 100644
--- a/block.c
+++ b/block.c
@@ -1224,8 +1224,8 @@ void bdrv_set_backing_hd(BlockDriverState *bs, 
BlockDriverState *backing_hd)
  bdrv_op_unblock_all(bs->backing_hd, bs->backing_blocker);
  } else if (backing_hd) {
  error_setg(&bs->backing_blocker,
-   "device is used as backing hd of '%s'",
-   bdrv_get_device_name(bs));
+   "node is used as backing hd of '%s'",
+   bdrv_get_device_or_node_name(bs));


Actually, the change of "device" to "node" here is independent of the 
use of bdrv_get_device_or_node_name(). But it's correct anyway.



  }
  
  bs->backing_hd = backing_hd;

@@ -1812,8 +1812,8 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, 
BlockReopenQueue *queue,
   * to r/w */
  if (!(reopen_state->bs->open_flags & BDRV_O_ALLOW_RDWR) &&
  reopen_state->flags & BDRV_O_RDWR) {
-error_set(errp, QERR_DEVICE_IS_READ_ONLY,
-  bdrv_get_device_name(reopen_state->bs));
+error_set(errp, QERR_NODE_IS_READ_ONLY,
+  bdrv_get_device_or_node_name(reopen_state->bs));


I think Markus would be happier with just removing the macro.

(Make it error_setg(errp, "Node '%s' is read only", 
bdrv_get_device_or_node_name(reopen_state->bs)) and remove the 
QERR_DEVICE_IS_READ_ONLY macro from qerror.h)


[snip]


diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 57a62d4..46cad14 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -38,7 +38,7 @@ void qerror_report_err(Error *err);
  ERROR_CLASS_GENERIC_ERROR, "Base '%s' not found"
  
  #define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \

-ERROR_CLASS_GENERIC_ERROR, "Block format '%s' used by device '%s' does not 
support feature '%s'"
+ERROR_CLASS_GENERIC_ERROR, "Block format '%s' used by node '%s' does not 
support feature '%s'"


Preexisting, but first: This line has over 80 characters.

Second: This seems like a good opportunity to drop this macro and 
replace its occurrences by error_setg(errp, "Block format...") and the like.



   #define QERR_BLOCK_JOB_NOT_READY \
  ERROR_CLASS_GENERIC_ERROR, "The active block job for device '%s' cannot be 
completed"
@@ -58,9 +58,6 @@ void qerror_report_err(Error *err);
  #define QERR_DEVICE_IN_USE \
  ERROR_CLASS_GENERIC_ERROR, "Device '%s' is in use"
  
-#define QERR_DEVICE_IS_READ_ONLY \

-ERROR_CLASS_GENERIC_ERROR, "Device '%s' is read only"
-
  #define QERR_DEVICE_NO_HOTPLUG \
  ERROR_CLASS_GENERIC_ERROR, "Device '%s' does not support hotplugging"
  
@@ -103,6 +100,9 @@ void qerror_report_err(Error *err);

  #define QERR_MISSING_PARAMETER \
  ERROR_CLASS_GENERIC_ERROR, "Parameter '%s' is missing"
  
+#define QERR_NODE_IS_READ_ONLY \

+ERROR_CLASS_GENERIC_ERROR, "Node '%s' is read only"
+
  #define QERR_PERMISSION_DENIED \
  ERROR_CLASS_GENERIC_ERROR, "Insufficient permission to perform this 
operation"


As said above, the same goes for this macro.

I'm not so ardent about this, so if you want to keep the macros, it's 
fine with me; but the overly long line need to be fixed (only the one 
you're touching, not necessarily the rest, because I guess they'll be 
dropped in the future anyway).


Markus?

Max



Re: [Qemu-devel] [PATCH 3/3] block: add 'node-name' field to BLOCK_IMAGE_CORRUPTED

2015-03-20 Thread Max Reitz

On 2015-03-20 at 06:19, Alberto Garcia wrote:

Since this event can occur in nodes that cannot have a device name
associated, include also a field with the node name.

Signed-off-by: Alberto Garcia 
---
  block/qcow2.c   |  8 ++--
  docs/qmp/qmp-events.txt | 16 +---
  qapi/block-core.json| 13 -
  3 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 168006b..e7c78f1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2809,6 +2809,7 @@ void qcow2_signal_corruption(BlockDriverState *bs, bool 
fatal, int64_t offset,
   int64_t size, const char *message_format, ...)
  {
  BDRVQcowState *s = bs->opaque;
+const char *node_name;
  char *message;
  va_list ap;
  
@@ -2832,8 +2833,11 @@ void qcow2_signal_corruption(BlockDriverState *bs, bool fatal, int64_t offset,

  "corruption events will be suppressed\n", message);
  }
  
-qapi_event_send_block_image_corrupted(bdrv_get_device_name(bs), message,

-  offset >= 0, offset, size >= 0, size,
+node_name = bdrv_get_node_name(bs);
+qapi_event_send_block_image_corrupted(bdrv_get_device_name(bs),
+  *node_name != '\0', node_name,
+  message, offset >= 0, offset,
+  size >= 0, size,
fatal, &error_abort);
  g_free(message);
  
diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt

index d759d19..ed1d0a5 100644
--- a/docs/qmp/qmp-events.txt
+++ b/docs/qmp/qmp-events.txt
@@ -35,17 +35,19 @@ Emitted when a disk image is being marked corrupt.
  
  Data:
  
-- "device": Device name (json-string)

-- "msg":Informative message (e.g., reason for the corruption) (json-string)
-- "offset": If the corruption resulted from an image access, this is the access
-offset into the image (json-int)
-- "size":   If the corruption resulted from an image access, this is the access
-size (json-int)
+- "device":Device name (json-string)
+- "node-name": Node name, if it's present (json-string)
+- "msg":   Informative message (e.g., reason for the corruption)
+   (json-string)
+- "offset":If the corruption resulted from an image access, this
+   is the access offset into the image (json-int)
+- "size":  If the corruption resulted from an image access, this
+   is the access size (json-int)
  
  Example:
  
  { "event": "BLOCK_IMAGE_CORRUPTED",

-"data": { "device": "ide0-hd0",
+"data": { "device": "ide0-hd0", "node-name": "node0",
  "msg": "Prevented active L1 table overwrite", "offset": 196608,
  "size": 65536 },
  "timestamp": { "seconds": 1378126126, "microseconds": 966463 } }
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 42c8850..e650168 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1753,6 +1753,8 @@
  #
  # @device: device name
  #
+# @node-name: #optional node name (Since: 2.3)


*2.4

With that fixed:

Reviewed-by: Max Reitz 


+#
  # @msg: informative message for human consumption, such as the kind of
  #   corruption being detected. It should not be parsed by machine as it is
  #   not guaranteed to be stable
@@ -1770,11 +1772,12 @@
  # Since: 1.7
  ##
  { 'event': 'BLOCK_IMAGE_CORRUPTED',
-  'data': { 'device' : 'str',
-'msg': 'str',
-'*offset': 'int',
-'*size'  : 'int',
-'fatal'  : 'bool' } }
+  'data': { 'device' : 'str',
+'*node-name' : 'str',
+'msg': 'str',
+'*offset': 'int',
+'*size'  : 'int',
+'fatal'  : 'bool' } }
  
  ##

  # @BLOCK_IO_ERROR





[Qemu-devel] [PATCH] block: Document blockdev-add's immaturity

2015-03-20 Thread Markus Armbruster
Signed-off-by: Markus Armbruster 
---
 qapi/block-core.json | 4 
 qmp-commands.hx  | 4 
 2 files changed, 8 insertions(+)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index f525b04..7873084 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1721,6 +1721,10 @@
 #
 # Creates a new block device.
 #
+# This command is still a work in progress.  It doesn't support all
+# block drivers, it lacks a matching blockdev-del, and more.  Stay
+# away from it unless you want to help with its development.
+#
 # @options: block device options for the new device
 #
 # Since: 1.7
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 7f68760..3a42ad0 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3636,6 +3636,10 @@ blockdev-add
 
 Add a block device.
 
+This command is still a work in progress.  It doesn't support all
+block drivers, it lacks a matching blockdev-del, and more.  Stay away
+from it unless you want to help with its development.
+
 Arguments:
 
 - "options": block driver options
-- 
1.9.3




[Qemu-devel] [PATCH] target-tricore: fix DVINIT_HU/BU calculating overflow before result

2015-03-20 Thread Bastian Koppelmann
dvinit_hu/bu for ISA v1.3 calculate the higher part of the result, that is 
needed
for the overflow bits, after calculating the overflow bits.

Signed-off-by: Bastian Koppelmann 
---
 target-tricore/translate.c | 29 +
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index 0b7cf06..d3680c3 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -6240,7 +6240,7 @@ static void decode_rr_divide(CPUTriCoreState *env, 
DisasContext *ctx)
 uint32_t op2;
 int r1, r2, r3;
 
-TCGv temp, temp2;
+TCGv temp, temp2, temp3;
 
 op2 = MASK_OP_RR_OP2(ctx->opcode);
 r3 = MASK_OP_RR_D(ctx->opcode);
@@ -6261,14 +6261,17 @@ static void decode_rr_divide(CPUTriCoreState *env, 
DisasContext *ctx)
 case OPC2_32_RR_DVINIT_BU:
 temp = tcg_temp_new();
 temp2 = tcg_temp_new();
+temp3 = tcg_temp_new();
+
+tcg_gen_shri_tl(temp3, cpu_gpr_d[r1], 8);
 /* reset av */
 tcg_gen_movi_tl(cpu_PSW_AV, 0);
 if (!tricore_feature(env, TRICORE_FEATURE_131)) {
 /* overflow = (abs(D[r3+1]) >= abs(D[r2])) */
-tcg_gen_neg_tl(temp, cpu_gpr_d[r3+1]);
+tcg_gen_neg_tl(temp, temp3);
 /* use cpu_PSW_AV to compare against 0 */
-tcg_gen_movcond_tl(TCG_COND_LT, temp, cpu_gpr_d[r3+1], cpu_PSW_AV,
-   temp, cpu_gpr_d[r3+1]);
+tcg_gen_movcond_tl(TCG_COND_LT, temp, temp3, cpu_PSW_AV,
+   temp, temp3);
 tcg_gen_neg_tl(temp2, cpu_gpr_d[r2]);
 tcg_gen_movcond_tl(TCG_COND_LT, temp2, cpu_gpr_d[r2], cpu_PSW_AV,
temp2, cpu_gpr_d[r2]);
@@ -6281,9 +6284,8 @@ static void decode_rr_divide(CPUTriCoreState *env, 
DisasContext *ctx)
 /* sv */
 tcg_gen_or_tl(cpu_PSW_SV, cpu_PSW_SV, cpu_PSW_V);
 /* write result */
-tcg_gen_shri_tl(temp, cpu_gpr_d[r1], 8);
 tcg_gen_shli_tl(cpu_gpr_d[r3], cpu_gpr_d[r1], 24);
-tcg_gen_mov_tl(cpu_gpr_d[r3+1], temp);
+tcg_gen_mov_tl(cpu_gpr_d[r3+1], temp3);
 
 tcg_temp_free(temp);
 tcg_temp_free(temp2);
@@ -6295,14 +6297,17 @@ static void decode_rr_divide(CPUTriCoreState *env, 
DisasContext *ctx)
 case OPC2_32_RR_DVINIT_HU:
 temp = tcg_temp_new();
 temp2 = tcg_temp_new();
+temp3 = tcg_temp_new();
+
+tcg_gen_shri_tl(temp3, cpu_gpr_d[r1], 16);
 /* reset av */
 tcg_gen_movi_tl(cpu_PSW_AV, 0);
 if (!tricore_feature(env, TRICORE_FEATURE_131)) {
 /* overflow = (abs(D[r3+1]) >= abs(D[r2])) */
-tcg_gen_neg_tl(temp, cpu_gpr_d[r3+1]);
+tcg_gen_neg_tl(temp, temp3);
 /* use cpu_PSW_AV to compare against 0 */
-tcg_gen_movcond_tl(TCG_COND_LT, temp, cpu_gpr_d[r3+1], cpu_PSW_AV,
-   temp, cpu_gpr_d[r3+1]);
+tcg_gen_movcond_tl(TCG_COND_LT, temp, temp3, cpu_PSW_AV,
+   temp, temp3);
 tcg_gen_neg_tl(temp2, cpu_gpr_d[r2]);
 tcg_gen_movcond_tl(TCG_COND_LT, temp2, cpu_gpr_d[r2], cpu_PSW_AV,
temp2, cpu_gpr_d[r2]);
@@ -6315,11 +6320,11 @@ static void decode_rr_divide(CPUTriCoreState *env, 
DisasContext *ctx)
 /* sv */
 tcg_gen_or_tl(cpu_PSW_SV, cpu_PSW_SV, cpu_PSW_V);
 /* write result */
-tcg_gen_mov_tl(temp, cpu_gpr_d[r1]);
-tcg_gen_shri_tl(cpu_gpr_d[r3+1], temp, 16);
-tcg_gen_shli_tl(cpu_gpr_d[r3], temp, 16);
+tcg_gen_mov_tl(cpu_gpr_d[r3+1], temp3);
+tcg_gen_shli_tl(cpu_gpr_d[r3], cpu_gpr_d[r1], 16);
 tcg_temp_free(temp);
 tcg_temp_free(temp2);
+tcg_temp_free(temp3);
 break;
 case OPC2_32_RR_DVINIT:
 temp = tcg_temp_new();
-- 
2.3.3




Re: [Qemu-devel] [PATCH] block: Document blockdev-add's immaturity

2015-03-20 Thread Markus Armbruster
Forgot to stick for-2.3 into the subject.



Re: [Qemu-devel] [ipxe-devel] Emulation failure on booting/rebooting VMs

2015-03-20 Thread Robin Smidsrød
On 19.03.2015 19:31, Mohammed Gamal wrote:
> 
> On Wed, Mar 18, 2015 at 7:09 PM, Kevin O'Connor  > wrote:
> 
> On Wed, Mar 18, 2015 at 06:36:48PM +0100, Mohammed Gamal wrote:
> > Hi,
> > I've been sporadically getting my KVM virtual machines crashing
> with this
> > message while they're booting
> >
> > KVM internal error. Suberror: 1
> > emulation failure
> > EAX= EBX= ECX= EDX=00600f12
> > ESI= EDI= EBP= ESP=fffa
> > EIP=ff53 EFL=0006 [-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0
> > ES =   9300
> > CS =f000 000f  9b00
> > SS =   9200
> > DS =   9300
> > FS =   9300
> > GS =   9300
> > LDT=   8200
> > TR =   8300
> > GDT=  
> > IDT=  
> > CR0=6010 CR2= CR3= CR4=
> > DR0= DR1= DR2=
> > DR3=
> > DR6=0ff0 DR7=0400
> > EFER=
> > Code=74 65 61 6d 2e 00 66 68 5f 55 00 00 e9 c3 f8 fb f4 cb 66 c3
>  66 68
> > ff e6 00 00 e9 8b b1 66 55 66 57 66 56 66 53 66 89 c1 66 89 d6 a8
> 07 75 23
> > 66 0f
> >
> > I am running qemu 1.2, seabios 1.7.3 and ipxe (1.0.0-f6840ba) and
> the host
> > CPU is AMD Opteron 6386 SE running kernel 3.4.71.
> 
> To debug seabios issues, we need the full log - see:
>   http://www.seabios.org/Debugging
> 
> But, your software versions are a bit old which makes diagnosing any
> issues hard.  I suggest updating to the latest software and seeing if
> the problem is still present.
> 
> -Kevin
> 
> 
> Unfortunately I can't change that setup. The problem is also rather
> sporadic so I have no other means to reproduce it and debug it.
> I tried looking into the seabios code but it looks like the instruction
> pointer is pointing at the default interrupt vector table entry
> 'entry_iret_official'. I presume this means that was triggered with an
> interrupt of some sort? The other register values don't seem to provide
> any clues nevertheless.

I thought maybe this commit was relevant,
https://git.ipxe.org/wimboot.git/commitdiff/1f11d05addc7aab9097b59e07680a0d1c05e34b3,
but noticed you're not using wimboot at all...

-- Robin





Re: [Qemu-devel] [PATCH RFC for-2.3 1/1] block: New command line option --no-format-probing

2015-03-20 Thread Max Reitz

On 2015-03-20 at 09:05, Markus Armbruster wrote:

Probing is convenient, but probing untrusted raw images is insecure
(CVE-2008-2004).  To avoid it, users should always specify raw format
explicitly.  This isn't trivial, and even sophisticated users have
gotten it wrong (libvirt CVE-2010-2237, CVE-2010-2238, CVE-2010-2239,
plus more recent variations of the theme that didn't get CVEs because
they were caught before they could hurt users).

Disabling probing entirely is a (hamfisted) way to ensure you always
specify the format.

Unfortunately, the new option is not available with -readconfig.
There's no obvious option group to take it.  I think we could use a
"miscellaneous" option group.

Signed-off-by: Markus Armbruster 
---
  block.c   |  9 -
  include/block/block.h |  2 +-
  qemu-options.hx   | 12 
  vl.c  |  6 +-
  4 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 0fe97de..5865309 100644
--- a/block.c
+++ b/block.c
@@ -103,6 +103,7 @@ static void bdrv_reset_dirty(BlockDriverState *bs, int64_t 
cur_sector,
   int nr_sectors);
  /* If non-zero, use only whitelisted block drivers */
  static int use_bdrv_whitelist;
+static bool bdrv_image_probing_disabled;
  
  #ifdef _WIN32

  static int is_windows_drive_prefix(const char *filename)
@@ -751,6 +752,11 @@ static int find_image_format(BlockDriverState *bs, const 
char *filename,
  return ret;
  }
  
+if (bdrv_image_probing_disabled) {

+error_setg(errp, "Format not specified and image probing disabled");
+return -EINVAL;
+}
+
  ret = bdrv_pread(bs, 0, buf, sizeof(buf));
  if (ret < 0) {
  error_setg_errno(errp, -ret, "Could not read image for determining its 
"
@@ -4909,9 +4915,10 @@ void bdrv_init(void)
  module_call_init(MODULE_INIT_BLOCK);
  }
  
-void bdrv_init_with_whitelist(void)

+void bdrv_init_with_whitelist(bool no_format_probing)
  {
  use_bdrv_whitelist = 1;
+bdrv_image_probing_disabled = no_format_probing;
  bdrv_init();
  }
  
diff --git a/include/block/block.h b/include/block/block.h

index 4c57d63..b5a8b23 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -177,7 +177,7 @@ void bdrv_io_limits_enable(BlockDriverState *bs);
  void bdrv_io_limits_disable(BlockDriverState *bs);
  
  void bdrv_init(void);

-void bdrv_init_with_whitelist(void);
+void bdrv_init_with_whitelist(bool no_format_probing);
  BlockDriver *bdrv_find_protocol(const char *filename,
  bool allow_protocol_prefix,
  Error **errp);
diff --git a/qemu-options.hx b/qemu-options.hx
index 319d971..8aa4d7b 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -963,6 +963,18 @@ STEXI
  Disable SDL window close capability.
  ETEXI
  
+DEF("no-format-probing", 0, QEMU_OPTION_no_format_probing,

+"-no-format-probing\n"
+"disable block image format probing\n", QEMU_ARCH_ALL)
+STEXI
+@item -no-format-probing
+@findex -no-format-probing
+Disable block image format probing.  Probing is convenient, but
+probing untrusted raw images is insecure.  To avoid it, always specify
+raw format explicitly.  Disabling probing entirely is a (hamfisted)
+way to ensure you do.
+ETEXI
+
  DEF("sdl", 0, QEMU_OPTION_sdl,
  "-sdlenable SDL\n", QEMU_ARCH_ALL)
  STEXI
diff --git a/vl.c b/vl.c
index 75ec292..94d5e15 100644
--- a/vl.c
+++ b/vl.c
@@ -2754,6 +2754,7 @@ int main(int argc, char **argv, char **envp)
  #endif
  bool defconfig = true;
  bool userconfig = true;
+bool no_format_probing = false;
  const char *log_mask = NULL;
  const char *log_file = NULL;
  GMemVTable mem_trace = {
@@ -2823,7 +2824,7 @@ int main(int argc, char **argv, char **envp)
  
  nb_nics = 0;
  
-bdrv_init_with_whitelist();

+bdrv_init_with_whitelist(no_format_probing);
  
  autostart = 1;
  
@@ -3381,6 +3382,9 @@ int main(int argc, char **argv, char **envp)

  case QEMU_OPTION_no_quit:
  no_quit = 1;
  break;
+case QEMU_OPTION_no_format_probing:
+no_format_probing = true;
+break;
  case QEMU_OPTION_sdl:
  #ifdef CONFIG_SDL
  display_type = DT_SDL;


You're setting no_format_probing after you're using it, so it doesn't 
work very well. :-)


Max



Re: [Qemu-devel] [PATCH RFC for-2.3 1/1] block: New command line option --no-format-probing

2015-03-20 Thread Markus Armbruster
Max Reitz  writes:

> On 2015-03-20 at 09:05, Markus Armbruster wrote:
>> Probing is convenient, but probing untrusted raw images is insecure
>> (CVE-2008-2004).  To avoid it, users should always specify raw format
>> explicitly.  This isn't trivial, and even sophisticated users have
>> gotten it wrong (libvirt CVE-2010-2237, CVE-2010-2238, CVE-2010-2239,
>> plus more recent variations of the theme that didn't get CVEs because
>> they were caught before they could hurt users).
>>
>> Disabling probing entirely is a (hamfisted) way to ensure you always
>> specify the format.
>>
>> Unfortunately, the new option is not available with -readconfig.
>> There's no obvious option group to take it.  I think we could use a
>> "miscellaneous" option group.
>>
>> Signed-off-by: Markus Armbruster 
>> ---
>>   block.c   |  9 -
>>   include/block/block.h |  2 +-
>>   qemu-options.hx   | 12 
>>   vl.c  |  6 +-
>>   4 files changed, 26 insertions(+), 3 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 0fe97de..5865309 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -103,6 +103,7 @@ static void bdrv_reset_dirty(BlockDriverState *bs, 
>> int64_t cur_sector,
>>int nr_sectors);
>>   /* If non-zero, use only whitelisted block drivers */
>>   static int use_bdrv_whitelist;
>> +static bool bdrv_image_probing_disabled;
>> #ifdef _WIN32
>>   static int is_windows_drive_prefix(const char *filename)
>> @@ -751,6 +752,11 @@ static int find_image_format(BlockDriverState *bs, 
>> const char *filename,
>>   return ret;
>>   }
>>   +if (bdrv_image_probing_disabled) {
>> +error_setg(errp, "Format not specified and image probing disabled");
>> +return -EINVAL;
>> +}
>> +
>>   ret = bdrv_pread(bs, 0, buf, sizeof(buf));
>>   if (ret < 0) {
>>   error_setg_errno(errp, -ret, "Could not read image for determining 
>> its "
>> @@ -4909,9 +4915,10 @@ void bdrv_init(void)
>>   module_call_init(MODULE_INIT_BLOCK);
>>   }
>>   -void bdrv_init_with_whitelist(void)
>> +void bdrv_init_with_whitelist(bool no_format_probing)
>>   {
>>   use_bdrv_whitelist = 1;
>> +bdrv_image_probing_disabled = no_format_probing;
>>   bdrv_init();
>>   }
>>   diff --git a/include/block/block.h b/include/block/block.h
>> index 4c57d63..b5a8b23 100644
>> --- a/include/block/block.h
>> +++ b/include/block/block.h
>> @@ -177,7 +177,7 @@ void bdrv_io_limits_enable(BlockDriverState *bs);
>>   void bdrv_io_limits_disable(BlockDriverState *bs);
>> void bdrv_init(void);
>> -void bdrv_init_with_whitelist(void);
>> +void bdrv_init_with_whitelist(bool no_format_probing);
>>   BlockDriver *bdrv_find_protocol(const char *filename,
>>   bool allow_protocol_prefix,
>>   Error **errp);
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 319d971..8aa4d7b 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -963,6 +963,18 @@ STEXI
>>   Disable SDL window close capability.
>>   ETEXI
>>   +DEF("no-format-probing", 0, QEMU_OPTION_no_format_probing,
>> +"-no-format-probing\n"
>> +"disable block image format probing\n", QEMU_ARCH_ALL)
>> +STEXI
>> +@item -no-format-probing
>> +@findex -no-format-probing
>> +Disable block image format probing.  Probing is convenient, but
>> +probing untrusted raw images is insecure.  To avoid it, always specify
>> +raw format explicitly.  Disabling probing entirely is a (hamfisted)
>> +way to ensure you do.
>> +ETEXI
>> +
>>   DEF("sdl", 0, QEMU_OPTION_sdl,
>>   "-sdlenable SDL\n", QEMU_ARCH_ALL)
>>   STEXI
>> diff --git a/vl.c b/vl.c
>> index 75ec292..94d5e15 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2754,6 +2754,7 @@ int main(int argc, char **argv, char **envp)
>>   #endif
>>   bool defconfig = true;
>>   bool userconfig = true;
>> +bool no_format_probing = false;
>>   const char *log_mask = NULL;
>>   const char *log_file = NULL;
>>   GMemVTable mem_trace = {
>> @@ -2823,7 +2824,7 @@ int main(int argc, char **argv, char **envp)
>> nb_nics = 0;
>>   -bdrv_init_with_whitelist();
>> +bdrv_init_with_whitelist(no_format_probing);
>> autostart = 1;
>>   @@ -3381,6 +3382,9 @@ int main(int argc, char **argv, char **envp)
>>   case QEMU_OPTION_no_quit:
>>   no_quit = 1;
>>   break;
>> +case QEMU_OPTION_no_format_probing:
>> +no_format_probing = true;
>> +break;
>>   case QEMU_OPTION_sdl:
>>   #ifdef CONFIG_SDL
>>   display_type = DT_SDL;
>
> You're setting no_format_probing after you're using it, so it doesn't
> work very well. :-)

I really should not test stuff on a hurried Friday afternoon...

I'd appreciate opinions on whether this is wanted for 2.3.  If it is,
I'll post a version that actually works.



Re: [Qemu-devel] [PATCH RFC for-2.3 1/1] block: New command line option --no-format-probing

2015-03-20 Thread Max Reitz

On 2015-03-20 at 09:48, Markus Armbruster wrote:

Max Reitz  writes:


On 2015-03-20 at 09:05, Markus Armbruster wrote:

Probing is convenient, but probing untrusted raw images is insecure
(CVE-2008-2004).  To avoid it, users should always specify raw format
explicitly.  This isn't trivial, and even sophisticated users have
gotten it wrong (libvirt CVE-2010-2237, CVE-2010-2238, CVE-2010-2239,
plus more recent variations of the theme that didn't get CVEs because
they were caught before they could hurt users).

Disabling probing entirely is a (hamfisted) way to ensure you always
specify the format.

Unfortunately, the new option is not available with -readconfig.
There's no obvious option group to take it.  I think we could use a
"miscellaneous" option group.

Signed-off-by: Markus Armbruster 
---
   block.c   |  9 -
   include/block/block.h |  2 +-
   qemu-options.hx   | 12 
   vl.c  |  6 +-
   4 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 0fe97de..5865309 100644
--- a/block.c
+++ b/block.c
@@ -103,6 +103,7 @@ static void bdrv_reset_dirty(BlockDriverState *bs, int64_t 
cur_sector,
int nr_sectors);
   /* If non-zero, use only whitelisted block drivers */
   static int use_bdrv_whitelist;
+static bool bdrv_image_probing_disabled;
 #ifdef _WIN32
   static int is_windows_drive_prefix(const char *filename)
@@ -751,6 +752,11 @@ static int find_image_format(BlockDriverState *bs, const 
char *filename,
   return ret;
   }
   +if (bdrv_image_probing_disabled) {
+error_setg(errp, "Format not specified and image probing disabled");
+return -EINVAL;
+}
+
   ret = bdrv_pread(bs, 0, buf, sizeof(buf));
   if (ret < 0) {
   error_setg_errno(errp, -ret, "Could not read image for determining its 
"
@@ -4909,9 +4915,10 @@ void bdrv_init(void)
   module_call_init(MODULE_INIT_BLOCK);
   }
   -void bdrv_init_with_whitelist(void)
+void bdrv_init_with_whitelist(bool no_format_probing)
   {
   use_bdrv_whitelist = 1;
+bdrv_image_probing_disabled = no_format_probing;
   bdrv_init();
   }
   diff --git a/include/block/block.h b/include/block/block.h
index 4c57d63..b5a8b23 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -177,7 +177,7 @@ void bdrv_io_limits_enable(BlockDriverState *bs);
   void bdrv_io_limits_disable(BlockDriverState *bs);
 void bdrv_init(void);
-void bdrv_init_with_whitelist(void);
+void bdrv_init_with_whitelist(bool no_format_probing);
   BlockDriver *bdrv_find_protocol(const char *filename,
   bool allow_protocol_prefix,
   Error **errp);
diff --git a/qemu-options.hx b/qemu-options.hx
index 319d971..8aa4d7b 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -963,6 +963,18 @@ STEXI
   Disable SDL window close capability.
   ETEXI
   +DEF("no-format-probing", 0, QEMU_OPTION_no_format_probing,
+"-no-format-probing\n"
+"disable block image format probing\n", QEMU_ARCH_ALL)
+STEXI
+@item -no-format-probing
+@findex -no-format-probing
+Disable block image format probing.  Probing is convenient, but
+probing untrusted raw images is insecure.  To avoid it, always specify
+raw format explicitly.  Disabling probing entirely is a (hamfisted)
+way to ensure you do.
+ETEXI
+
   DEF("sdl", 0, QEMU_OPTION_sdl,
   "-sdlenable SDL\n", QEMU_ARCH_ALL)
   STEXI
diff --git a/vl.c b/vl.c
index 75ec292..94d5e15 100644
--- a/vl.c
+++ b/vl.c
@@ -2754,6 +2754,7 @@ int main(int argc, char **argv, char **envp)
   #endif
   bool defconfig = true;
   bool userconfig = true;
+bool no_format_probing = false;
   const char *log_mask = NULL;
   const char *log_file = NULL;
   GMemVTable mem_trace = {
@@ -2823,7 +2824,7 @@ int main(int argc, char **argv, char **envp)
 nb_nics = 0;
   -bdrv_init_with_whitelist();
+bdrv_init_with_whitelist(no_format_probing);
 autostart = 1;
   @@ -3381,6 +3382,9 @@ int main(int argc, char **argv, char **envp)
   case QEMU_OPTION_no_quit:
   no_quit = 1;
   break;
+case QEMU_OPTION_no_format_probing:
+no_format_probing = true;
+break;
   case QEMU_OPTION_sdl:
   #ifdef CONFIG_SDL
   display_type = DT_SDL;

You're setting no_format_probing after you're using it, so it doesn't
work very well. :-)

I really should not test stuff on a hurried Friday afternoon...

I'd appreciate opinions on whether this is wanted for 2.3.  If it is,
I'll post a version that actually works.


I don't have any objections because it won't break anything. But I guess 
it'll be mostly up to whether Eric thinks that we'll need it right now.


Max



Re: [Qemu-devel] [for-2.3 PATCH] block: Document blockdev-add's immaturity

2015-03-20 Thread Eric Blake
On 03/20/2015 07:32 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster 
> ---
>  qapi/block-core.json | 4 
>  qmp-commands.hx  | 4 
>  2 files changed, 8 insertions(+)

Never hurts to document things like this, even after freeze :)

Reviewed-by: Eric Blake 


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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH RFC for-2.3 1/1] block: New command line option --no-format-probing

2015-03-20 Thread Eric Blake
On 03/20/2015 07:49 AM, Max Reitz wrote:

 Disabling probing entirely is a (hamfisted) way to ensure you always
 specify the format.


>>
>> I'd appreciate opinions on whether this is wanted for 2.3.  If it is,
>> I'll post a version that actually works.
> 
> I don't have any objections because it won't break anything. But I guess
> it'll be mostly up to whether Eric thinks that we'll need it right now.

I'm totally in favor of the idea; it has no drawback to users that don't
add it to the command line (so no chance of regression to existing
command lines), and allows users that care to ensure that they are being
secure.  And I can argue that this is a bug fix rather than a feature
and therefore appropriate even in freeze (even though the bug has been a
long-standing security hole, rather than a recent regression).

If (a working version of) this makes it in 2.3, libvirt WILL use it in
the next release.  It will take me less than 5 minutes to write up the
libvirt patch, as long as the new option is advertised via
query-command-line-options (which means that QMP introspection of the
new option is a must for v2 :)

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages

2015-03-20 Thread Markus Armbruster
Max Reitz  writes:

> On 2015-03-20 at 06:19, Alberto Garcia wrote:
>> There are several error messages that identify a BlockDriverState by
>> its device name. However those errors can be produced in nodes that
>> don't have a device name associated.
>>
>> In those cases we should use bdrv_get_device_or_node_name() to fall
>> back to the node name and produce a more meaningful message. The
>> messages are also updated to use the more generic term 'node' instead
>> of 'device'.
>>
>> Signed-off-by: Alberto Garcia 
>> ---
>>   block.c   | 21 +++--
>>   block/qcow.c  |  4 ++--
>>   block/qcow2.c |  2 +-
>>   block/qed.c   |  2 +-
>>   block/vdi.c   |  2 +-
>>   block/vhdx.c  |  2 +-
>>   block/vmdk.c  |  4 ++--
>>   block/vpc.c   |  2 +-
>>   block/vvfat.c |  3 ++-
>>   blockdev.c|  4 ++--
>>   include/qapi/qmp/qerror.h |  8 
>>   11 files changed, 28 insertions(+), 26 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 98b90b4..8247f0a 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -1224,8 +1224,8 @@ void bdrv_set_backing_hd(BlockDriverState *bs, 
>> BlockDriverState *backing_hd)
>>   bdrv_op_unblock_all(bs->backing_hd, bs->backing_blocker);
>>   } else if (backing_hd) {
>>   error_setg(&bs->backing_blocker,
>> -   "device is used as backing hd of '%s'",
>> -   bdrv_get_device_name(bs));
>> +   "node is used as backing hd of '%s'",
>> +   bdrv_get_device_or_node_name(bs));
>
> Actually, the change of "device" to "node" here is independent of the
> use of bdrv_get_device_or_node_name(). But it's correct anyway.
>
>>   }
>> bs->backing_hd = backing_hd;
>> @@ -1812,8 +1812,8 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, 
>> BlockReopenQueue *queue,
>>* to r/w */
>>   if (!(reopen_state->bs->open_flags & BDRV_O_ALLOW_RDWR) &&
>>   reopen_state->flags & BDRV_O_RDWR) {
>> -error_set(errp, QERR_DEVICE_IS_READ_ONLY,
>> -  bdrv_get_device_name(reopen_state->bs));
>> +error_set(errp, QERR_NODE_IS_READ_ONLY,
>> +  bdrv_get_device_or_node_name(reopen_state->bs));
>
> I think Markus would be happier with just removing the macro.
>
> (Make it error_setg(errp, "Node '%s' is read only",
> bdrv_get_device_or_node_name(reopen_state->bs)) and remove the
> QERR_DEVICE_IS_READ_ONLY macro from qerror.h)
>
> [snip]
>
>> diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
>> index 57a62d4..46cad14 100644
>> --- a/include/qapi/qmp/qerror.h
>> +++ b/include/qapi/qmp/qerror.h
>> @@ -38,7 +38,7 @@ void qerror_report_err(Error *err);
>>   ERROR_CLASS_GENERIC_ERROR, "Base '%s' not found"
>> #define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \
>> -ERROR_CLASS_GENERIC_ERROR, "Block format '%s' used by device '%s' does 
>> not support feature '%s'"
>> +ERROR_CLASS_GENERIC_ERROR, "Block format '%s' used by node '%s' does 
>> not support feature '%s'"
>
> Preexisting, but first: This line has over 80 characters.
>
> Second: This seems like a good opportunity to drop this macro and
> replace its occurrences by error_setg(errp, "Block format...") and the
> like.
>
>>#define QERR_BLOCK_JOB_NOT_READY \
>>   ERROR_CLASS_GENERIC_ERROR, "The active block job for device '%s' 
>> cannot be completed"
>> @@ -58,9 +58,6 @@ void qerror_report_err(Error *err);
>>   #define QERR_DEVICE_IN_USE \
>>   ERROR_CLASS_GENERIC_ERROR, "Device '%s' is in use"
>>   -#define QERR_DEVICE_IS_READ_ONLY \
>> -ERROR_CLASS_GENERIC_ERROR, "Device '%s' is read only"
>> -
>>   #define QERR_DEVICE_NO_HOTPLUG \
>>   ERROR_CLASS_GENERIC_ERROR, "Device '%s' does not support hotplugging"
>>   @@ -103,6 +100,9 @@ void qerror_report_err(Error *err);
>>   #define QERR_MISSING_PARAMETER \
>>   ERROR_CLASS_GENERIC_ERROR, "Parameter '%s' is missing"
>>   +#define QERR_NODE_IS_READ_ONLY \
>> +ERROR_CLASS_GENERIC_ERROR, "Node '%s' is read only"
>> +
>>   #define QERR_PERMISSION_DENIED \
>>   ERROR_CLASS_GENERIC_ERROR, "Insufficient permission to perform this 
>> operation"
>
> As said above, the same goes for this macro.
>
> I'm not so ardent about this, so if you want to keep the macros, it's
> fine with me; but the overly long line need to be fixed (only the one
> you're touching, not necessarily the rest, because I guess they'll be
> dropped in the future anyway).
>
> Markus?

I've been chipping away at qerror.h for a while.  I think I can empty it
out in the next development cycle.  So every new QERR_ macro you add
I'll have to replace again.  Using literal strings instead will save me
the trouble.

If you find yourself using the same error string in many places, chances
are that some of these places do the same thing.  Consider factoring
out.

Other than that, I wouldn't worry about duplicating er

Re: [Qemu-devel] [PATCH RFC for-2.3 0/1] block: New command line option --no-format-probing

2015-03-20 Thread Eric Blake
On 03/20/2015 07:05 AM, Markus Armbruster wrote:
> First of all, my apologies for being so late with this.  I realized
> part way through the current development cycle that I couldn't do both
> the error work and my half of the block probing work we discussed back
> in November, so I punted the latter to the next cycle, missing the one
> little feature I quite obviously could do.
> 
> Why is this "RFC for-2.3"?  The patch is simple, and quite obviously
> does nothing unless you run with --no-format-probing.  If libvirt
> wants it in 2.3, then I think we should consider it even at this late
> stage.  If libvirt doesn't want it, or won't try to make use of it for
> another few months, I'm happy to drop the patch now and revisit the
> larger topic in the next cycle.
> 
> I readily admit --no-format-probing is fugly.  Better ideas are
> welcome.

So, is it spelled '--no-format-probing', or is it spelled
'-no-format-probing'?  It is in the same category as other flag
parameters like '-enable-fips' or '-no-user-config'.

> 
> Markus Armbruster (1):
>   block: New command line option --no-format-probing
> 
>  block.c   |  9 -
>  include/block/block.h |  2 +-
>  qemu-options.hx   | 12 
>  vl.c  |  6 +-
>  4 files changed, 26 insertions(+), 3 deletions(-)
> 

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



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH v2 0/6] virtio: add input device

2015-03-20 Thread Gerd Hoffmann
  Hi,

This patch series adds virtio input devices.  It's basically sending
linux evdev events over virtio.  There is support for emulated hid
devices (i.e. send usual input to virtio device instead of usb or ps2
device) and for evdev device pass-through.

This depends on mst's virtio-1.0 patches.

Changes in v2:
 * sync header files from linux & use them.
 * added virtio_input_devids struct, pass device ids from host
   (requested on guest driver review).

Guest driver:
  https://www.kraxel.org/cgit/linux/log/?h=virtio-input

Specification:
  https://www.kraxel.org/cgit/virtio-spec/log/?h=virtio-input
  https://www.kraxel.org/virtio/virtio-v1.0-csprd03-virtio-input.html#x1-2640007

Gerd Hoffmann (6):
  pci: add  PCI_CLASS_INPUT_*
  virtio-input: add virtio_input.h
  virtio-input: add linux/input.h
  virtio-input: core code & base class
  virtio-input: emulated devices
  virtio-input: evdev passthrough

 hw/input/Makefile.objs|6 +
 hw/input/virtio-input-hid.c   |  502 +++
 hw/input/virtio-input-host.c  |  182 
 hw/input/virtio-input.c   |  282 ++
 hw/virtio/virtio-pci.c|  150 
 hw/virtio/virtio-pci.h|   37 +
 include/hw/pci/pci_ids.h  |7 +
 include/hw/virtio/virtio-input.h  |  117 +++
 include/hw/virtio/virtio.h|1 +
 include/standard-headers/linux/input.h| 1198 +
 include/standard-headers/linux/virtio_ids.h   |1 +
 include/standard-headers/linux/virtio_input.h |   75 ++
 scripts/update-linux-headers.sh   |4 +-
 13 files changed, 2561 insertions(+), 1 deletion(-)
 create mode 100644 hw/input/virtio-input-hid.c
 create mode 100644 hw/input/virtio-input-host.c
 create mode 100644 hw/input/virtio-input.c
 create mode 100644 include/hw/virtio/virtio-input.h
 create mode 100644 include/standard-headers/linux/input.h
 create mode 100644 include/standard-headers/linux/virtio_input.h

-- 
1.8.3.1




[Qemu-devel] [PATCH v2 4/6] virtio-input: core code & base class

2015-03-20 Thread Gerd Hoffmann
This patch adds virtio-input support to qemu.  It brings a abstract
base class providing core support, other classes can build on it to
actually implement input devices.

virtio-input basically sends linux input layer events (evdev) over
virtio.

Signed-off-by: Gerd Hoffmann 
---
 hw/input/Makefile.objs   |   4 +
 hw/input/virtio-input.c  | 282 +++
 hw/virtio/virtio-pci.c   |  36 +
 hw/virtio/virtio-pci.h   |  14 ++
 include/hw/virtio/virtio-input.h |  83 
 include/hw/virtio/virtio.h   |   1 +
 6 files changed, 420 insertions(+)
 create mode 100644 hw/input/virtio-input.c
 create mode 100644 include/hw/virtio/virtio-input.h

diff --git a/hw/input/Makefile.objs b/hw/input/Makefile.objs
index e8c80b9..ee8bba9 100644
--- a/hw/input/Makefile.objs
+++ b/hw/input/Makefile.objs
@@ -8,6 +8,10 @@ common-obj-$(CONFIG_STELLARIS_INPUT) += stellaris_input.o
 common-obj-$(CONFIG_TSC2005) += tsc2005.o
 common-obj-$(CONFIG_VMMOUSE) += vmmouse.o
 
+ifeq ($(CONFIG_LINUX),y)
+common-obj-$(CONFIG_VIRTIO) += virtio-input.o
+endif
+
 obj-$(CONFIG_MILKYMIST) += milkymist-softusb.o
 obj-$(CONFIG_PXA2XX) += pxa2xx_keypad.o
 obj-$(CONFIG_TSC210X) += tsc210x.o
diff --git a/hw/input/virtio-input.c b/hw/input/virtio-input.c
new file mode 100644
index 000..17cbe97
--- /dev/null
+++ b/hw/input/virtio-input.c
@@ -0,0 +1,282 @@
+/*
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.  See the COPYING file in the
+ * top-level directory.
+ */
+
+#include "qemu/iov.h"
+
+#include "hw/qdev.h"
+#include "hw/virtio/virtio.h"
+#include "hw/virtio/virtio-input.h"
+
+#include "standard-headers/linux/input.h"
+
+/* - */
+
+void virtio_input_send(VirtIOInput *vinput, virtio_input_event *event)
+{
+VirtQueueElement elem;
+unsigned have, need;
+int i, len;
+
+/* queue up events ... */
+if (vinput->qindex == vinput->qsize) {
+vinput->qsize++;
+vinput->queue = realloc(vinput->queue, vinput->qsize *
+sizeof(virtio_input_event));
+}
+vinput->queue[vinput->qindex++] = *event;
+
+/* ... until we see a report sync ... */
+if (event->type != cpu_to_le16(EV_SYN) ||
+event->code != cpu_to_le16(SYN_REPORT)) {
+return;
+}
+
+/* ... then check available space ... */
+need = sizeof(virtio_input_event) * vinput->qindex;
+virtqueue_get_avail_bytes(vinput->evt, &have, NULL, need, 0);
+if (have < need) {
+vinput->qindex = 0;
+fprintf(stderr, "%s: ENOSPC in vq, dropping events\n", __func__);
+return;
+}
+
+/* ... and finally pass them to the guest */
+for (i = 0; i < vinput->qindex; i++) {
+if (!virtqueue_pop(vinput->evt, &elem)) {
+/* should not happen, we've checked for space beforehand */
+fprintf(stderr, "%s: Huh?  No vq elem available ...\n", __func__);
+return;
+}
+len = iov_from_buf(elem.in_sg, elem.in_num,
+   0, vinput->queue+i, sizeof(virtio_input_event));
+virtqueue_push(vinput->evt, &elem, len);
+}
+virtio_notify(VIRTIO_DEVICE(vinput), vinput->evt);
+vinput->qindex = 0;
+}
+
+static void virtio_input_handle_evt(VirtIODevice *vdev, VirtQueue *vq)
+{
+/* nothing */
+}
+
+static void virtio_input_handle_sts(VirtIODevice *vdev, VirtQueue *vq)
+{
+VirtIOInputClass *vic = VIRTIO_INPUT_GET_CLASS(vdev);
+VirtIOInput *vinput = VIRTIO_INPUT(vdev);
+virtio_input_event event;
+VirtQueueElement elem;
+int len;
+
+while (virtqueue_pop(vinput->sts, &elem)) {
+memset(&event, 0, sizeof(event));
+len = iov_to_buf(elem.out_sg, elem.out_num,
+ 0, &event, sizeof(event));
+if (vic->handle_status) {
+vic->handle_status(vinput, &event);
+}
+virtqueue_push(vinput->sts, &elem, len);
+}
+virtio_notify(vdev, vinput->sts);
+}
+
+static virtio_input_config *virtio_input_find_config(VirtIOInput *vinput,
+ uint8_t select,
+ uint8_t subsel)
+{
+VirtIOInputConfig *cfg;
+
+QTAILQ_FOREACH(cfg, &vinput->cfg_list, node) {
+if (select == cfg->config.select &&
+subsel == cfg->config.subsel) {
+return &cfg->config;
+}
+}
+return NULL;
+}
+
+void virtio_input_add_config(VirtIOInput *vinput,
+ virtio_input_config *config)
+{
+VirtIOInputConfig *cfg;
+
+if (virtio_input_find_config(vinput, config->select, config->subsel)) {
+/* should not happen */
+fprintf(stderr, "%s: duplicate config: %d/%d\n",
+__func__, config->select, config->subsel);
+abort();
+}
+
+cfg = g_new0(VirtIOInputConfig

[Qemu-devel] [PATCH v2 1/6] pci: add PCI_CLASS_INPUT_*

2015-03-20 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann 
---
 include/hw/pci/pci_ids.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h
index d7be386..dfccefc 100644
--- a/include/hw/pci/pci_ids.h
+++ b/include/hw/pci/pci_ids.h
@@ -46,6 +46,13 @@
 #define PCI_CLASS_COMMUNICATION_SERIAL   0x0700
 #define PCI_CLASS_COMMUNICATION_OTHER0x0780
 
+#define PCI_CLASS_INPUT_KEYBOARD 0x0900
+#define PCI_CLASS_INPUT_PEN  0x0901
+#define PCI_CLASS_INPUT_MOUSE0x0902
+#define PCI_CLASS_INPUT_SCANNER  0x0903
+#define PCI_CLASS_INPUT_GAMEPORT 0x0904
+#define PCI_CLASS_INPUT_OTHER0x0980
+
 #define PCI_CLASS_PROCESSOR_CO   0x0b40
 #define PCI_CLASS_PROCESSOR_POWERPC  0x0b20
 
-- 
1.8.3.1




[Qemu-devel] [PATCH v2 6/6] virtio-input: evdev passthrough

2015-03-20 Thread Gerd Hoffmann
This allows to assign host input devices to the guest:

qemu -device virto-input-host-pci,evdev=/dev/input/event

The guest gets exclusive access to the input device, so be careful
with assigning the keyboard if you have only one connected to your
machine.

Signed-off-by: Gerd Hoffmann 
---
 hw/input/Makefile.objs   |   1 +
 hw/input/virtio-input-host.c | 182 +++
 hw/virtio/virtio-pci.c   |  30 +++
 hw/virtio/virtio-pci.h   |  10 +++
 include/hw/virtio/virtio-input.h |  13 +++
 5 files changed, 236 insertions(+)
 create mode 100644 hw/input/virtio-input-host.c

diff --git a/hw/input/Makefile.objs b/hw/input/Makefile.objs
index 0dae710..624ba7e 100644
--- a/hw/input/Makefile.objs
+++ b/hw/input/Makefile.objs
@@ -11,6 +11,7 @@ common-obj-$(CONFIG_VMMOUSE) += vmmouse.o
 ifeq ($(CONFIG_LINUX),y)
 common-obj-$(CONFIG_VIRTIO) += virtio-input.o
 common-obj-$(CONFIG_VIRTIO) += virtio-input-hid.o
+common-obj-$(CONFIG_VIRTIO) += virtio-input-host.o
 endif
 
 obj-$(CONFIG_MILKYMIST) += milkymist-softusb.o
diff --git a/hw/input/virtio-input-host.c b/hw/input/virtio-input-host.c
new file mode 100644
index 000..b16cc4c
--- /dev/null
+++ b/hw/input/virtio-input-host.c
@@ -0,0 +1,182 @@
+/*
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.  See the COPYING file in the
+ * top-level directory.
+ */
+
+#include "qemu-common.h"
+#include "qemu/sockets.h"
+
+#include "hw/qdev.h"
+#include "hw/virtio/virtio.h"
+#include "hw/virtio/virtio-input.h"
+
+#include "standard-headers/linux/input.h"
+
+/* - */
+
+static struct virtio_input_config virtio_input_host_config[] = {
+{ /* empty list */ },
+};
+
+static void virtio_input_host_event(void *opaque)
+{
+VirtIOInputHost *vih = opaque;
+VirtIOInput *vinput = VIRTIO_INPUT(vih);
+struct virtio_input_event virtio;
+struct input_event evdev;
+int rc;
+
+for (;;) {
+rc = read(vih->fd, &evdev, sizeof(evdev));
+if (rc != sizeof(evdev)) {
+break;
+}
+
+virtio.type  = cpu_to_le16(evdev.type);
+virtio.code  = cpu_to_le16(evdev.code);
+virtio.value = cpu_to_le32(evdev.value);
+virtio_input_send(vinput, &virtio);
+}
+}
+
+static void virtio_input_bits_config(VirtIOInputHost *vih,
+ int type, int count)
+{
+virtio_input_config bits;
+int rc, i, size = 0;
+
+memset(&bits, 0, sizeof(bits));
+rc = ioctl(vih->fd, EVIOCGBIT(type, count/8), bits.u.bitmap);
+if (rc < 0) {
+return;
+}
+
+for (i = 0; i < count/8; i++) {
+if (bits.u.bitmap[i]) {
+size = i+1;
+}
+}
+if (size == 0) {
+return;
+}
+
+bits.select = VIRTIO_INPUT_CFG_EV_BITS;
+bits.subsel = type;
+bits.size   = size;
+virtio_input_add_config(VIRTIO_INPUT(vih), &bits);
+}
+
+static void virtio_input_host_realize(DeviceState *dev, Error **errp)
+{
+VirtIOInputHost *vih = VIRTIO_INPUT_HOST(dev);
+VirtIOInput *vinput = VIRTIO_INPUT(dev);
+virtio_input_config id;
+struct input_id ids;
+int rc, ver;
+
+if (!vih->evdev) {
+error_setg(errp, "evdev property is required");
+return;
+}
+
+vih->fd = open(vih->evdev, O_RDWR);
+if (vih->fd < 0)  {
+error_setg_file_open(errp, errno, vih->evdev);
+return;
+}
+qemu_set_nonblock(vih->fd);
+
+rc = ioctl(vih->fd, EVIOCGVERSION, &ver);
+if (rc < 0) {
+error_setg(errp, "%s: is not an evdev device", vih->evdev);
+goto err_close;
+}
+
+rc = ioctl(vih->fd, EVIOCGRAB, 1);
+if (rc < 0) {
+error_setg_errno(errp, errno, "%s: failed to get exclusive access",
+ vih->evdev);
+goto err_close;
+}
+
+memset(&id, 0, sizeof(id));
+ioctl(vih->fd, EVIOCGNAME(sizeof(id.u.string)-1), id.u.string);
+id.select = VIRTIO_INPUT_CFG_ID_NAME;
+id.size = strlen(id.u.string);
+virtio_input_add_config(vinput, &id);
+
+if (ioctl(vih->fd, EVIOCGID, &ids) == 0) {
+memset(&id, 0, sizeof(id));
+id.select = VIRTIO_INPUT_CFG_ID_DEVIDS;
+id.size = sizeof(struct virtio_input_devids);
+id.u.ids.bustype = cpu_to_le16(ids.bustype);
+id.u.ids.vendor  = cpu_to_le16(ids.vendor);
+id.u.ids.product = cpu_to_le16(ids.product);
+id.u.ids.version = cpu_to_le16(ids.version);
+virtio_input_add_config(vinput, &id);
+}
+
+virtio_input_bits_config(vih, EV_KEY, KEY_CNT);
+virtio_input_bits_config(vih, EV_REL, REL_CNT);
+virtio_input_bits_config(vih, EV_ABS, ABS_CNT);
+virtio_input_bits_config(vih, EV_MSC, MSC_CNT);
+virtio_input_bits_config(vih, EV_SW,  SW_CNT);
+
+qemu_set_fd_handler(vih->fd, virtio_input_host_event, NULL, vih);
+return;
+
+err_close:
+close(vih->fd);
+

[Qemu-devel] [PATCH v2 2/6] virtio-input: add virtio_input.h

2015-03-20 Thread Gerd Hoffmann
Using scripts/update-linux-headers.sh with linux-input kernel branch.

Signed-off-by: Gerd Hoffmann 
---
 include/standard-headers/linux/virtio_ids.h   |  1 +
 include/standard-headers/linux/virtio_input.h | 75 +++
 2 files changed, 76 insertions(+)
 create mode 100644 include/standard-headers/linux/virtio_input.h

diff --git a/include/standard-headers/linux/virtio_ids.h 
b/include/standard-headers/linux/virtio_ids.h
index 284fc3a..5f60aa4 100644
--- a/include/standard-headers/linux/virtio_ids.h
+++ b/include/standard-headers/linux/virtio_ids.h
@@ -39,5 +39,6 @@
 #define VIRTIO_ID_9P   9 /* 9p virtio console */
 #define VIRTIO_ID_RPROC_SERIAL 11 /* virtio remoteproc serial link */
 #define VIRTIO_ID_CAIF12 /* Virtio caif */
+#define VIRTIO_ID_INPUT18 /* virtio input */
 
 #endif /* _LINUX_VIRTIO_IDS_H */
diff --git a/include/standard-headers/linux/virtio_input.h 
b/include/standard-headers/linux/virtio_input.h
new file mode 100644
index 000..8c91bab
--- /dev/null
+++ b/include/standard-headers/linux/virtio_input.h
@@ -0,0 +1,75 @@
+#ifndef _LINUX_VIRTIO_INPUT_H
+#define _LINUX_VIRTIO_INPUT_H
+/* This header is BSD licensed so anyone can use the definitions to implement
+ * compatible drivers/servers.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of IBM nor the names of its contributors
+ *may be used to endorse or promote products derived from this software
+ *without specific prior written permission.
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL IBM OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE. */
+#include "standard-headers/linux/virtio_ids.h"
+#include "standard-headers/linux/virtio_config.h"
+
+enum virtio_input_config_select {
+   VIRTIO_INPUT_CFG_UNSET  = 0x00,
+   VIRTIO_INPUT_CFG_ID_NAME= 0x01,
+   VIRTIO_INPUT_CFG_ID_SERIAL  = 0x02,
+   VIRTIO_INPUT_CFG_ID_DEVIDS  = 0x03,
+   VIRTIO_INPUT_CFG_PROP_BITS  = 0x10,
+   VIRTIO_INPUT_CFG_EV_BITS= 0x11,
+   VIRTIO_INPUT_CFG_ABS_INFO   = 0x12,
+};
+
+struct virtio_input_absinfo {
+   uint32_t  min;
+   uint32_t  max;
+   uint32_t  fuzz;
+   uint32_t  flat;
+};
+
+struct virtio_input_devids {
+   uint16_t  bustype;
+   uint16_t  vendor;
+   uint16_t  product;
+   uint16_t  version;
+};
+
+struct virtio_input_config {
+   uint8_tselect;
+   uint8_tsubsel;
+   uint8_tsize;
+   uint8_treserved;
+   union {
+   char string[128];
+   uint8_t bitmap[128];
+   struct virtio_input_absinfo abs;
+   struct virtio_input_devids ids;
+   } u;
+};
+
+struct virtio_input_event {
+   uint16_t type;
+   uint16_t code;
+   uint32_t value;
+};
+
+#endif /* _LINUX_VIRTIO_INPUT_H */
-- 
1.8.3.1




[Qemu-devel] [PATCH v2 3/6] virtio-input: add linux/input.h

2015-03-20 Thread Gerd Hoffmann
Linux input layer (evdev) header file.

Signed-off-by: Gerd Hoffmann 
---
 include/standard-headers/linux/input.h | 1198 
 scripts/update-linux-headers.sh|4 +-
 2 files changed, 1201 insertions(+), 1 deletion(-)
 create mode 100644 include/standard-headers/linux/input.h

diff --git a/include/standard-headers/linux/input.h 
b/include/standard-headers/linux/input.h
new file mode 100644
index 000..b94d365
--- /dev/null
+++ b/include/standard-headers/linux/input.h
@@ -0,0 +1,1198 @@
+/*
+ * Copyright (c) 1999-2002 Vojtech Pavlik
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+#ifndef _INPUT_H
+#define _INPUT_H
+
+
+#include 
+#include 
+#include 
+#include "standard-headers/linux/types.h"
+
+
+/*
+ * The event structure itself
+ */
+
+struct input_event {
+   struct timeval time;
+   uint16_t type;
+   uint16_t code;
+   int32_t value;
+};
+
+/*
+ * Protocol version.
+ */
+
+#define EV_VERSION 0x010001
+
+/*
+ * IOCTLs (0x00 - 0x7f)
+ */
+
+struct input_id {
+   uint16_t bustype;
+   uint16_t vendor;
+   uint16_t product;
+   uint16_t version;
+};
+
+/**
+ * struct input_absinfo - used by EVIOCGABS/EVIOCSABS ioctls
+ * @value: latest reported value for the axis.
+ * @minimum: specifies minimum value for the axis.
+ * @maximum: specifies maximum value for the axis.
+ * @fuzz: specifies fuzz value that is used to filter noise from
+ * the event stream.
+ * @flat: values that are within this value will be discarded by
+ * joydev interface and reported as 0 instead.
+ * @resolution: specifies resolution for the values reported for
+ * the axis.
+ *
+ * Note that input core does not clamp reported values to the
+ * [minimum, maximum] limits, such task is left to userspace.
+ *
+ * Resolution for main axes (ABS_X, ABS_Y, ABS_Z) is reported in
+ * units per millimeter (units/mm), resolution for rotational axes
+ * (ABS_RX, ABS_RY, ABS_RZ) is reported in units per radian.
+ */
+struct input_absinfo {
+   int32_t value;
+   int32_t minimum;
+   int32_t maximum;
+   int32_t fuzz;
+   int32_t flat;
+   int32_t resolution;
+};
+
+/**
+ * struct input_keymap_entry - used by EVIOCGKEYCODE/EVIOCSKEYCODE ioctls
+ * @scancode: scancode represented in machine-endian form.
+ * @len: length of the scancode that resides in @scancode buffer.
+ * @index: index in the keymap, may be used instead of scancode
+ * @flags: allows to specify how kernel should handle the request. For
+ * example, setting INPUT_KEYMAP_BY_INDEX flag indicates that kernel
+ * should perform lookup in keymap by @index instead of @scancode
+ * @keycode: key code assigned to this scancode
+ *
+ * The structure is used to retrieve and modify keymap data. Users have
+ * option of performing lookup either by @scancode itself or by @index
+ * in keymap entry. EVIOCGKEYCODE will also return scancode or index
+ * (depending on which element was used to perform lookup).
+ */
+struct input_keymap_entry {
+#define INPUT_KEYMAP_BY_INDEX  (1 << 0)
+   uint8_t  flags;
+   uint8_t  len;
+   uint16_t index;
+   uint32_t keycode;
+   uint8_t  scancode[32];
+};
+
+#define EVIOCGVERSION  _IOR('E', 0x01, int)/* get 
driver version */
+#define EVIOCGID   _IOR('E', 0x02, struct input_id)/* get 
device ID */
+#define EVIOCGREP  _IOR('E', 0x03, unsigned int[2])/* get 
repeat settings */
+#define EVIOCSREP  _IOW('E', 0x03, unsigned int[2])/* set 
repeat settings */
+
+#define EVIOCGKEYCODE  _IOR('E', 0x04, unsigned int[2])/* get 
keycode */
+#define EVIOCGKEYCODE_V2   _IOR('E', 0x04, struct input_keymap_entry)
+#define EVIOCSKEYCODE  _IOW('E', 0x04, unsigned int[2])/* set 
keycode */
+#define EVIOCSKEYCODE_V2   _IOW('E', 0x04, struct input_keymap_entry)
+
+#define EVIOCGNAME(len)_IOC(_IOC_READ, 'E', 0x06, len) 
/* get device name */
+#define EVIOCGPHYS(len)_IOC(_IOC_READ, 'E', 0x07, len) 
/* get physical location */
+#define EVIOCGUNIQ(len)_IOC(_IOC_READ, 'E', 0x08, len) 
/* get unique identifier */
+#define EVIOCGPROP(len)_IOC(_IOC_READ, 'E', 0x09, len) 
/* get device properties */
+
+/**
+ * EVIOCGMTSLOTS(len) - get MT slot values
+ * @len: size of the data buffer in bytes
+ *
+ * The ioctl buffer argument should be binary equivalent to
+ *
+ * struct input_mt_request_layout {
+ * uint32_t code;
+ * int32_t values[num_slots];
+ * };
+ *
+ * where num_slots is the (arbitrary) number of MT slots to extract.
+ *
+ * The ioctl size argument (len) is the size of the buffer, which
+ * should satisfy len = (num_slots + 1) * sizeof(int32_t).  If len is
+ * too 

[Qemu-devel] [PATCH v2 5/6] virtio-input: emulated devices

2015-03-20 Thread Gerd Hoffmann
This patch adds the virtio-input-hid base class and
virtio-{keyboard,mouse,tablet} subclasses building on the base class.
They are hooked up to the qemu input core and deliver input events
to the guest like all other hid devices (ps/2 kbd, usb tablet, ...).

Using them is as simple as adding "-device virtio-tablet-pci" to your
command line.  If you want add multiple devices but don't want waste
a pci slot for each you can compose a multifunction device this way:

qemu -device virtio-keyboard-pci,addr=0d.0,multifunction=on \
 -device virtio-tablet-pci,addr=0d.1,multifunction=on

Signed-off-by: Gerd Hoffmann 
---
 hw/input/Makefile.objs   |   1 +
 hw/input/virtio-input-hid.c  | 502 +++
 hw/virtio/virtio-pci.c   |  84 +++
 hw/virtio/virtio-pci.h   |  13 +
 include/hw/virtio/virtio-input.h |  21 ++
 5 files changed, 621 insertions(+)
 create mode 100644 hw/input/virtio-input-hid.c

diff --git a/hw/input/Makefile.objs b/hw/input/Makefile.objs
index ee8bba9..0dae710 100644
--- a/hw/input/Makefile.objs
+++ b/hw/input/Makefile.objs
@@ -10,6 +10,7 @@ common-obj-$(CONFIG_VMMOUSE) += vmmouse.o
 
 ifeq ($(CONFIG_LINUX),y)
 common-obj-$(CONFIG_VIRTIO) += virtio-input.o
+common-obj-$(CONFIG_VIRTIO) += virtio-input-hid.o
 endif
 
 obj-$(CONFIG_MILKYMIST) += milkymist-softusb.o
diff --git a/hw/input/virtio-input-hid.c b/hw/input/virtio-input-hid.c
new file mode 100644
index 000..32cc94a
--- /dev/null
+++ b/hw/input/virtio-input-hid.c
@@ -0,0 +1,502 @@
+/*
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.  See the COPYING file in the
+ * top-level directory.
+ */
+
+#include "qemu/iov.h"
+
+#include "hw/qdev.h"
+#include "hw/virtio/virtio.h"
+#include "hw/virtio/virtio-input.h"
+
+#undef CONFIG_CURSES
+#include "ui/console.h"
+
+#include "standard-headers/linux/input.h"
+
+#define VIRTIO_ID_NAME_KEYBOARD "QEMU Virtio Keyboard"
+#define VIRTIO_ID_NAME_MOUSE"QEMU Virtio Mouse"
+#define VIRTIO_ID_NAME_TABLET   "QEMU Virtio Tablet"
+
+/* - */
+
+static const unsigned int keymap_qcode[Q_KEY_CODE_MAX] = {
+[Q_KEY_CODE_ESC] = KEY_ESC,
+[Q_KEY_CODE_1]   = KEY_1,
+[Q_KEY_CODE_2]   = KEY_2,
+[Q_KEY_CODE_3]   = KEY_3,
+[Q_KEY_CODE_4]   = KEY_4,
+[Q_KEY_CODE_5]   = KEY_5,
+[Q_KEY_CODE_6]   = KEY_6,
+[Q_KEY_CODE_7]   = KEY_7,
+[Q_KEY_CODE_8]   = KEY_8,
+[Q_KEY_CODE_9]   = KEY_9,
+[Q_KEY_CODE_0]   = KEY_0,
+[Q_KEY_CODE_MINUS]   = KEY_MINUS,
+[Q_KEY_CODE_EQUAL]   = KEY_EQUAL,
+[Q_KEY_CODE_BACKSPACE]   = KEY_BACKSPACE,
+
+[Q_KEY_CODE_TAB] = KEY_TAB,
+[Q_KEY_CODE_Q]   = KEY_Q,
+[Q_KEY_CODE_W]   = KEY_W,
+[Q_KEY_CODE_E]   = KEY_E,
+[Q_KEY_CODE_R]   = KEY_R,
+[Q_KEY_CODE_T]   = KEY_T,
+[Q_KEY_CODE_Y]   = KEY_Y,
+[Q_KEY_CODE_U]   = KEY_U,
+[Q_KEY_CODE_I]   = KEY_I,
+[Q_KEY_CODE_O]   = KEY_O,
+[Q_KEY_CODE_P]   = KEY_P,
+[Q_KEY_CODE_BRACKET_LEFT]= KEY_LEFTBRACE,
+[Q_KEY_CODE_BRACKET_RIGHT]   = KEY_RIGHTBRACE,
+[Q_KEY_CODE_RET] = KEY_ENTER,
+
+[Q_KEY_CODE_CTRL]= KEY_LEFTCTRL,
+[Q_KEY_CODE_A]   = KEY_A,
+[Q_KEY_CODE_S]   = KEY_S,
+[Q_KEY_CODE_D]   = KEY_D,
+[Q_KEY_CODE_F]   = KEY_F,
+[Q_KEY_CODE_G]   = KEY_G,
+[Q_KEY_CODE_H]   = KEY_H,
+[Q_KEY_CODE_J]   = KEY_J,
+[Q_KEY_CODE_K]   = KEY_K,
+[Q_KEY_CODE_L]   = KEY_L,
+[Q_KEY_CODE_SEMICOLON]   = KEY_SEMICOLON,
+[Q_KEY_CODE_APOSTROPHE]  = KEY_APOSTROPHE,
+[Q_KEY_CODE_GRAVE_ACCENT]= KEY_GRAVE,
+
+[Q_KEY_CODE_SHIFT]   = KEY_LEFTSHIFT,
+[Q_KEY_CODE_BACKSLASH]   = KEY_BACKSLASH,
+[Q_KEY_CODE_LESS]= KEY_102ND,
+[Q_KEY_CODE_Z]   = KEY_Z,
+[Q_KEY_CODE_X]   = KEY_X,
+[Q_KEY_CODE_C]   = KEY_C,
+[Q_KEY_CODE_V]   = KEY_V,
+[Q_KEY_CODE_B]   = KEY_B,
+[Q_KEY_CODE_N]   = KEY_N,
+[Q_KEY_CODE_M]   = KEY_M,
+[Q_KEY_CODE_COMMA]   = KEY_COMMA,
+[Q_KEY_CODE_DOT] = KEY_DOT,
+[Q_KEY_CODE_SLASH]   = KEY_SLASH,
+[Q_KEY_CODE_SHIFT_R] = KEY_RIGHTSHIFT,
+
+[Q_KEY_CODE_ALT] = KEY_LEFTALT,
+[Q_KEY_CODE_SPC] 

[Qemu-devel] [PATCH] mips_malta: use compat props to avoid loading efi-pcnet.rom

2015-03-20 Thread Leon Alrae
Currently qemu-system-mips aborts if it fails to find efi-pcnet.rom file which
does not make sense. NIC on Malta board should not require x86 firmware.

Reported-by: Maciej W. Rozycki 
Suggested-by: Gerd Hoffmann 
Signed-off-by: Leon Alrae 
---
Hi,

This fixes a problem reported some time ago:
http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg01870.html

Regards,
Leon
---
 hw/mips/mips_malta.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 533b2e6..b857941 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1212,6 +1212,14 @@ static QEMUMachine mips_malta_machine = {
 .init = mips_malta_init,
 .max_cpus = 16,
 .is_default = 1,
+.compat_props = (GlobalProperty[]) {
+{
+.driver   = TYPE_PCI_DEVICE,
+.property = "romfile",
+.value= "",
+},
+{ /* end of list */ }
+},
 };
 
 static void mips_malta_register_types(void)
-- 
2.1.0




[Qemu-devel] [RFC PATCH] qemu: enforce no format probing when possible

2015-03-20 Thread Eric Blake
qemu 2.3 added an option, with this documentation:

| Probing is convenient, but probing untrusted raw images is insecure
| (CVE-2008-2004).  To avoid it, users should always specify raw format
| explicitly.  This isn't trivial, and even sophisticated users have
| gotten it wrong (libvirt CVE-2010-2237, CVE-2010-2238, CVE-2010-2239,
| plus more recent variations of the theme that didn't get CVEs because
| they were caught before they could hurt users).
|
| Disabling probing entirely is a (hamfisted) way to ensure you always
| specify the format.

Use it when libvirt is configured to avoid probing, to ensure we
catch libvirt failures on avoiding probing before such bugs can
escalate into another CVE.

* src/qemu/qemu_capabilities.h (QEMU_CAPS_NO_FORMAT_PROBING): New
capability bit.
* src/qemu/qemu_capabilities.c (virQEMUCapsCommandLine): Set it.
* src/qemu/qemu_command.c (qemuBuildCommandLine): Use it.

Signed-off-by: Eric Blake 
CC: qemu-devel@nongnu.org
---

RFC because I need to enhance the libvirt testsuite to prove we set
the option, and because the qemu side has not been committed yet
(and may therefore change the final spelling for the new option).

 src/qemu/qemu_capabilities.c | 7 +--
 src/qemu/qemu_capabilities.h | 3 ++-
 src/qemu/qemu_command.c  | 3 +++
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index ccf22f0..b452a75 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1,7 +1,7 @@
 /*
  * qemu_capabilities.c: QEMU capabilities generation
  *
- * Copyright (C) 2006-2014 Red Hat, Inc.
+ * Copyright (C) 2006-2015 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -279,6 +279,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "qxl.vgamem_mb",
   "qxl-vga.vgamem_mb",
   "pc-dimm",
+
+  "no-format-probing", /* 185 */
 );


@@ -2514,7 +2516,8 @@ static struct virQEMUCapsCommandLineProps 
virQEMUCapsCommandLine[] = {
 { "spice", "disable-agent-file-xfer", QEMU_CAPS_SPICE_FILE_XFER_DISABLE },
 { "msg", "timestamp", QEMU_CAPS_MSG_TIMESTAMP },
 { "numa", NULL, QEMU_CAPS_NUMA },
-{ "drive", "throttling.bps-total-max", QEMU_CAPS_DRIVE_IOTUNE_MAX},
+{ "drive", "throttling.bps-total-max", QEMU_CAPS_DRIVE_IOTUNE_MAX },
+{ "no-format-probing", NULL, QEMU_CAPS_NO_FORMAT_PROBING },
 };

 static int
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index c7b1ac7..2cdcd81 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -1,7 +1,7 @@
 /*
  * qemu_capabilities.h: QEMU capabilities generation
  *
- * Copyright (C) 2006-2014 Red Hat, Inc.
+ * Copyright (C) 2006-2015 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -224,6 +224,7 @@ typedef enum {
 QEMU_CAPS_QXL_VGAMEM = 182, /* -device qxl.vgamem_mb */
 QEMU_CAPS_QXL_VGA_VGAMEM = 183, /* -device qxl-vga.vgamem_mb */
 QEMU_CAPS_DEVICE_PC_DIMM = 184, /* pc-dimm device */
+QEMU_CAPS_NO_FORMAT_PROBING  = 185, /* -no-format-probing */

 QEMU_CAPS_LAST,   /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 63d43d4..1085639 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8672,6 +8672,9 @@ qemuBuildCommandLine(virConnectPtr conn,
 virCommandAddArg(cmd, "-nodefconfig");
 virCommandAddArg(cmd, "-nodefaults");
 }
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_FORMAT_PROBING) &&
+!cfg->allowDiskFormatProbing)
+virCommandAddArg(cmd, "-no-format-probing");

 /* Serial graphics adapter */
 if (def->os.bios.useserial == VIR_TRISTATE_BOOL_YES) {
-- 
2.1.0




Re: [Qemu-devel] [PATCH RFC for-2.3 1/1] block: New command line option --no-format-probing

2015-03-20 Thread Markus Armbruster
Eric Blake  writes:

> On 03/20/2015 07:49 AM, Max Reitz wrote:
>
> Disabling probing entirely is a (hamfisted) way to ensure you always
> specify the format.
>
>
>>>
>>> I'd appreciate opinions on whether this is wanted for 2.3.  If it is,
>>> I'll post a version that actually works.
>> 
>> I don't have any objections because it won't break anything. But I guess
>> it'll be mostly up to whether Eric thinks that we'll need it right now.
>
> I'm totally in favor of the idea; it has no drawback to users that don't
> add it to the command line (so no chance of regression to existing
> command lines), and allows users that care to ensure that they are being
> secure.  And I can argue that this is a bug fix rather than a feature
> and therefore appropriate even in freeze (even though the bug has been a
> long-standing security hole, rather than a recent regression).
>
> If (a working version of) this makes it in 2.3, libvirt WILL use it in
> the next release.  It will take me less than 5 minutes to write up the
> libvirt patch, as long as the new option is advertised via
> query-command-line-options (which means that QMP introspection of the
> new option is a must for v2 :)

query-command-line-options covers only options using QemuOpts.  Fixing
that defect isn't in the cards for 2.3, which means I'll have to
implement it with QemuOpts.

Ways to do that:

* Stick it into an existing QemuOpts option.  Is there one that fits?
  --machine doesn't really fit.

* Create a new QemuOpts option for miscellaneous settings.  Would --misc
  format-probing=off be too ugly?  Got a better name than --misc?

  Existing miscellaneous non-QemeOpts options could then (in 2.4!)
  become sugar for something in this option group, thus become available
  with -readconfig.

Thoughts?



Re: [Qemu-devel] [PATCH RFC for-2.3 0/1] block: New command line option --no-format-probing

2015-03-20 Thread Markus Armbruster
Eric Blake  writes:

> On 03/20/2015 07:05 AM, Markus Armbruster wrote:
>> First of all, my apologies for being so late with this.  I realized
>> part way through the current development cycle that I couldn't do both
>> the error work and my half of the block probing work we discussed back
>> in November, so I punted the latter to the next cycle, missing the one
>> little feature I quite obviously could do.
>> 
>> Why is this "RFC for-2.3"?  The patch is simple, and quite obviously
>> does nothing unless you run with --no-format-probing.  If libvirt
>> wants it in 2.3, then I think we should consider it even at this late
>> stage.  If libvirt doesn't want it, or won't try to make use of it for
>> another few months, I'm happy to drop the patch now and revisit the
>> larger topic in the next cycle.
>> 
>> I readily admit --no-format-probing is fugly.  Better ideas are
>> welcome.
>
> So, is it spelled '--no-format-probing', or is it spelled
> '-no-format-probing'?  It is in the same category as other flag
> parameters like '-enable-fips' or '-no-user-config'.

Any QEMU option can be spelled both as -NAME and as --NAME.

Rolling your very own command line parser is foolish, but the only way
to stay design-flaw-compatible.



Re: [Qemu-devel] [PATCH RFC for-2.3 1/1] block: New command line option --no-format-probing

2015-03-20 Thread Eric Blake
On 03/20/2015 08:19 AM, Markus Armbruster wrote:

>> If (a working version of) this makes it in 2.3, libvirt WILL use it in
>> the next release.  It will take me less than 5 minutes to write up the
>> libvirt patch, as long as the new option is advertised via
>> query-command-line-options (which means that QMP introspection of the
>> new option is a must for v2 :)

The real libvirt requirement is that the new option is accessible via
_some_ QMP command (doesn't have to be query-command-line-options, but
that is the most obvious).

> 
> query-command-line-options covers only options using QemuOpts.  Fixing
> that defect isn't in the cards for 2.3, which means I'll have to
> implement it with QemuOpts.

Makes sense, and is certainly easier than figuring out an alternative
QMP probing mechanism.

> 
> Ways to do that:
> 
> * Stick it into an existing QemuOpts option.  Is there one that fits?
>   --machine doesn't really fit.

Mostly agree; --machine is more for what the guest sees, while this has
no impact on the guest ABI.  On the other hand, things  like -machine
accel=[tcg|kvm] aren't necessarily guest visible either, so -machine has
tended to be a catch-all for other things.  But that doesn't mean it
should continue to be one.

> 
> * Create a new QemuOpts option for miscellaneous settings.  Would --misc
>   format-probing=off be too ugly?  Got a better name than --misc?

The only clients using the new option will be machine generated, so I
don't think ugly appearance is a show-stopper.  And -misc really does
sound like it is the most amenable to adding other random flags in the
future, so I can live with that name.

Unless anyone else has a better idea, '-misc format-probing=off' has my
vote and works for libvirt; it is only a two-line change to my libvirt
RFC patch:

diff --git i/src/qemu/qemu_capabilities.c w/src/qemu/qemu_capabilities.c
index b452a75..f7a46c1 100644
--- i/src/qemu/qemu_capabilities.c
+++ w/src/qemu/qemu_capabilities.c
@@ -2517,7 +2517,7 @@ static struct virQEMUCapsCommandLineProps
virQEMUCapsCommandLine[] = {
 { "msg", "timestamp", QEMU_CAPS_MSG_TIMESTAMP },
 { "numa", NULL, QEMU_CAPS_NUMA },
 { "drive", "throttling.bps-total-max", QEMU_CAPS_DRIVE_IOTUNE_MAX },
-{ "no-format-probing", NULL, QEMU_CAPS_NO_FORMAT_PROBING },
+{ "misc", "format-probing", QEMU_CAPS_NO_FORMAT_PROBING },
 };

 static int
diff --git i/src/qemu/qemu_command.c w/src/qemu/qemu_command.c
index 1085639..08f6560 100644
--- i/src/qemu/qemu_command.c
+++ w/src/qemu/qemu_command.c
@@ -8674,7 +8674,7 @@ qemuBuildCommandLine(virConnectPtr conn,
 }
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_FORMAT_PROBING) &&
 !cfg->allowDiskFormatProbing)
-virCommandAddArg(cmd, "-no-format-probing");
+virCommandAddArgList(cmd, "-misc", "format-probing=off", NULL);

 /* Serial graphics adapter */
 if (def->os.bios.useserial == VIR_TRISTATE_BOOL_YES) {


> 
>   Existing miscellaneous non-QemeOpts options could then (in 2.4!)
>   become sugar for something in this option group, thus become available
>   with -readconfig.

Nice way to plan for future cleanups.  I also agree that while this one
option about format-probing is reasonable to get into 2.3, that any
other sugar cleanups are better deferred until after the freeze.

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



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH v3 0/3] Add bdrv_get_device_or_node_name()

2015-03-20 Thread Alberto Garcia
One more try.

v3:
- The node-name field in BLOCK_IMAGE_CORRUPTED is now Since: 2.4
- Remove the QERR_ macros instead of updating them. The text message
  is adapted to each case where applicable, and 'device' is renamed to
  'node' only where it makes sense.

v2:
- bdrv_get_device_or_node_name() includes a comment explaining its
  usage.
- The error messages have been updated to say 'node' instead of
  'device' where appropriate.
- The BLOCK_IMAGE_CORRUPTED event has a new 'node-name' field.

Regards,

Berto

Alberto Garcia (3):
  block: add bdrv_get_device_or_node_name()
  block: use bdrv_get_device_or_node_name() in error messages
  block: add 'node-name' field to BLOCK_IMAGE_CORRUPTED

 block.c   | 33 +
 block/qcow.c  |  8 
 block/qcow2.c | 10 +++---
 block/qed.c   |  2 +-
 block/quorum.c|  5 +
 block/snapshot.c  | 12 ++--
 block/vdi.c   |  6 +++---
 block/vhdx.c  |  6 +++---
 block/vmdk.c  |  8 
 block/vpc.c   |  6 +++---
 block/vvfat.c |  7 ---
 blockdev.c|  9 +
 docs/qmp/qmp-events.txt   | 16 +---
 include/block/block.h |  1 +
 include/qapi/qmp/qerror.h |  6 --
 qapi/block-core.json  | 13 -
 16 files changed, 80 insertions(+), 68 deletions(-)

-- 
2.1.4




[Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages

2015-03-20 Thread Alberto Garcia
There are several error messages that identify a BlockDriverState by
its device name. However those errors can be produced in nodes that
don't have a device name associated.

In those cases we should use bdrv_get_device_or_node_name() to fall
back to the node name and produce a more meaningful message. The
messages are also updated to use the more generic term 'node' instead
of 'device'.

Signed-off-by: Alberto Garcia 
---
 block.c   | 24 
 block/qcow.c  |  8 
 block/qcow2.c |  2 +-
 block/qed.c   |  2 +-
 block/snapshot.c  | 12 ++--
 block/vdi.c   |  6 +++---
 block/vhdx.c  |  6 +++---
 block/vmdk.c  |  8 
 block/vpc.c   |  6 +++---
 block/vvfat.c |  7 ---
 blockdev.c|  9 +
 include/qapi/qmp/qerror.h |  6 --
 12 files changed, 46 insertions(+), 50 deletions(-)

diff --git a/block.c b/block.c
index 98b90b4..737ab68 100644
--- a/block.c
+++ b/block.c
@@ -1224,8 +1224,8 @@ void bdrv_set_backing_hd(BlockDriverState *bs, 
BlockDriverState *backing_hd)
 bdrv_op_unblock_all(bs->backing_hd, bs->backing_blocker);
 } else if (backing_hd) {
 error_setg(&bs->backing_blocker,
-   "device is used as backing hd of '%s'",
-   bdrv_get_device_name(bs));
+   "node is used as backing hd of '%s'",
+   bdrv_get_device_or_node_name(bs));
 }
 
 bs->backing_hd = backing_hd;
@@ -1812,8 +1812,8 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, 
BlockReopenQueue *queue,
  * to r/w */
 if (!(reopen_state->bs->open_flags & BDRV_O_ALLOW_RDWR) &&
 reopen_state->flags & BDRV_O_RDWR) {
-error_set(errp, QERR_DEVICE_IS_READ_ONLY,
-  bdrv_get_device_name(reopen_state->bs));
+error_setg(errp, "Node '%s' is read only",
+   bdrv_get_device_or_node_name(reopen_state->bs));
 goto error;
 }
 
@@ -1839,9 +1839,9 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, 
BlockReopenQueue *queue,
 } else {
 /* It is currently mandatory to have a bdrv_reopen_prepare()
  * handler for each supported drv. */
-error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
-  drv->format_name, bdrv_get_device_name(reopen_state->bs),
- "reopening of file");
+error_setg(errp, "Block format '%s' used by node '%s' "
+   "does not support reopening files", drv->format_name,
+   bdrv_get_device_or_node_name(reopen_state->bs));
 ret = -1;
 goto error;
 }
@@ -3764,8 +3764,8 @@ void bdrv_add_key(BlockDriverState *bs, const char *key, 
Error **errp)
 {
 if (key) {
 if (!bdrv_is_encrypted(bs)) {
-error_setg(errp, "Device '%s' is not encrypted",
-  bdrv_get_device_name(bs));
+error_setg(errp, "Node '%s' is not encrypted",
+  bdrv_get_device_or_node_name(bs));
 } else if (bdrv_set_key(bs, key) < 0) {
 error_set(errp, QERR_INVALID_PASSWORD);
 }
@@ -3773,7 +3773,7 @@ void bdrv_add_key(BlockDriverState *bs, const char *key, 
Error **errp)
 if (bdrv_key_required(bs)) {
 error_set(errp, ERROR_CLASS_DEVICE_ENCRYPTED,
   "'%s' (%s) is encrypted",
-  bdrv_get_device_name(bs),
+  bdrv_get_device_or_node_name(bs),
   bdrv_get_encrypted_filename(bs));
 }
 }
@@ -5548,8 +5548,8 @@ bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType 
op, Error **errp)
 if (!QLIST_EMPTY(&bs->op_blockers[op])) {
 blocker = QLIST_FIRST(&bs->op_blockers[op]);
 if (errp) {
-error_setg(errp, "Device '%s' is busy: %s",
-   bdrv_get_device_name(bs),
+error_setg(errp, "Node '%s' is busy: %s",
+   bdrv_get_device_or_node_name(bs),
error_get_pretty(blocker->reason));
 }
 return true;
diff --git a/block/qcow.c b/block/qcow.c
index 0558969..ab89328 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -124,7 +124,7 @@ static int qcow_open(BlockDriverState *bs, QDict *options, 
int flags,
 snprintf(version, sizeof(version), "QCOW version %" PRIu32,
  header.version);
 error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
-  bdrv_get_device_name(bs), "qcow", version);
+  bdrv_get_device_or_node_name(bs), "qcow", version);
 ret = -ENOTSUP;
 goto fail;
 }
@@ -229,9 +229,9 @@ static int qcow_open(BlockDriverState *bs, QDict *options, 
int flags,
 }
 
 /* Disable migration when qcow images are used */
-error_set(&s->migration_blocker,
-  QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
-  "qcow"

Re: [Qemu-devel] [PATCH v2 4/5] fw_cfg: prohibit insertion of duplicate fw_cfg file names

2015-03-20 Thread Gabriel L. Somlo
On Fri, Mar 20, 2015 at 07:51:06AM +0100, Laszlo Ersek wrote:
> Here's an idea I had this morning.
> 
> This series gives equal rank to fw_cfg file names that originate
> internally and those that come from the user, via the command line.
> 
> That means that whenever qemu developers want to introduce a new fw_cfg
> file, they can never be sure that the new name will not conflict with
> something a user has already been passing in via the command line, for
> whatever purposes. (Because, well, that's the goal of this patchset, to
> empower the user to pass in fw_cfg files independently of qemu developers.)
> 
> This looks brittle. How about:
> 
> (a) advising users in the docs txt *and in the manual* to use some kind
> of fw_cfg file name prefix, like "usr/" or "opt/", and then steering
> clear of such prefixes in qemu, as far as developers are concerned. Or,
> 
> (b) automatically prepending "opt/" or "usr/" to all fw_cfg file names
> that come via -fw_cfg (equiv. via [fw_cfg] in the config file), and, for
> developers, steering clear of those prefixes in qemu's source.
> 
> The C standard and the POSIX standard define lists of identifier
> prefixes (well, patterns) that are reserved for various uses. If a
> program violates that, it might not compile on some platform, or with
> the next release of the compiler on the same platform etc. I think we
> should posit something like this.
> 
> Personally I vote (a). Document it, but don't enforce it.
> 
> (Assuming that a user-specified fw_cfg file gains traction, and becomes
> popular to the point that qemu wants to expose it itself, then qemu can
> just generate the same file with (eg.) an "etc/" prefix. And then
> firmware (or other guest code) can start looking for the file under both
> prefixes, and give priority to... well, that's another policy question;
> but we're talking mechanism thus far. :))
> 
> Thoughts about (a) vs. (b) vs. neither?

You're basically saying it'd be nice to keep user-provided commandline
blobs in a separate *namespace* from those inserted programmatically
by QEMU, and I definitely agree!

My inner control freak's gut reaction is to vote (b), but I'm sure
theres lots of facets of the issue I haven't considered, so I could
easily be talked out of that selection :)

Either way, this would go in with patch 5/5 (where the command line
option is being added), so everything up to that point (including
generating an error on dupe fwcfg file names) should probably stay
the way it is...

Thanks much,
--Gabriel



[Qemu-devel] [PATCH 3/3] block: add 'node-name' field to BLOCK_IMAGE_CORRUPTED

2015-03-20 Thread Alberto Garcia
Since this event can occur in nodes that cannot have a device name
associated, include also a field with the node name.

Signed-off-by: Alberto Garcia 
---
 block/qcow2.c   |  8 ++--
 docs/qmp/qmp-events.txt | 16 +---
 qapi/block-core.json| 13 -
 3 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 168006b..e7c78f1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2809,6 +2809,7 @@ void qcow2_signal_corruption(BlockDriverState *bs, bool 
fatal, int64_t offset,
  int64_t size, const char *message_format, ...)
 {
 BDRVQcowState *s = bs->opaque;
+const char *node_name;
 char *message;
 va_list ap;
 
@@ -2832,8 +2833,11 @@ void qcow2_signal_corruption(BlockDriverState *bs, bool 
fatal, int64_t offset,
 "corruption events will be suppressed\n", message);
 }
 
-qapi_event_send_block_image_corrupted(bdrv_get_device_name(bs), message,
-  offset >= 0, offset, size >= 0, size,
+node_name = bdrv_get_node_name(bs);
+qapi_event_send_block_image_corrupted(bdrv_get_device_name(bs),
+  *node_name != '\0', node_name,
+  message, offset >= 0, offset,
+  size >= 0, size,
   fatal, &error_abort);
 g_free(message);
 
diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
index d759d19..ed1d0a5 100644
--- a/docs/qmp/qmp-events.txt
+++ b/docs/qmp/qmp-events.txt
@@ -35,17 +35,19 @@ Emitted when a disk image is being marked corrupt.
 
 Data:
 
-- "device": Device name (json-string)
-- "msg":Informative message (e.g., reason for the corruption) (json-string)
-- "offset": If the corruption resulted from an image access, this is the access
-offset into the image (json-int)
-- "size":   If the corruption resulted from an image access, this is the access
-size (json-int)
+- "device":Device name (json-string)
+- "node-name": Node name, if it's present (json-string)
+- "msg":   Informative message (e.g., reason for the corruption)
+   (json-string)
+- "offset":If the corruption resulted from an image access, this
+   is the access offset into the image (json-int)
+- "size":  If the corruption resulted from an image access, this
+   is the access size (json-int)
 
 Example:
 
 { "event": "BLOCK_IMAGE_CORRUPTED",
-"data": { "device": "ide0-hd0",
+"data": { "device": "ide0-hd0", "node-name": "node0",
 "msg": "Prevented active L1 table overwrite", "offset": 196608,
 "size": 65536 },
 "timestamp": { "seconds": 1378126126, "microseconds": 966463 } }
diff --git a/qapi/block-core.json b/qapi/block-core.json
index f525b04..2a40b73 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1752,6 +1752,8 @@
 #
 # @device: device name
 #
+# @node-name: #optional node name (Since: 2.4)
+#
 # @msg: informative message for human consumption, such as the kind of
 #   corruption being detected. It should not be parsed by machine as it is
 #   not guaranteed to be stable
@@ -1769,11 +1771,12 @@
 # Since: 1.7
 ##
 { 'event': 'BLOCK_IMAGE_CORRUPTED',
-  'data': { 'device' : 'str',
-'msg': 'str',
-'*offset': 'int',
-'*size'  : 'int',
-'fatal'  : 'bool' } }
+  'data': { 'device' : 'str',
+'*node-name' : 'str',
+'msg': 'str',
+'*offset': 'int',
+'*size'  : 'int',
+'fatal'  : 'bool' } }
 
 ##
 # @BLOCK_IO_ERROR
-- 
2.1.4




[Qemu-devel] [PATCH 1/3] block: add bdrv_get_device_or_node_name()

2015-03-20 Thread Alberto Garcia
This function gets the device name associated with a BlockDriverState,
or its node name if the device name is empty.

Signed-off-by: Alberto Garcia 
---
 block.c   | 9 +
 block/quorum.c| 5 +
 include/block/block.h | 1 +
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/block.c b/block.c
index 0fe97de..98b90b4 100644
--- a/block.c
+++ b/block.c
@@ -3920,6 +3920,15 @@ const char *bdrv_get_device_name(const BlockDriverState 
*bs)
 return bs->blk ? blk_name(bs->blk) : "";
 }
 
+/* This can be used to identify nodes that might not have a device
+ * name associated. Since node and device names live in the same
+ * namespace, the result is unambiguous. The exception is if both are
+ * absent, then this returns an empty (non-null) string. */
+const char *bdrv_get_device_or_node_name(const BlockDriverState *bs)
+{
+return bs->blk ? blk_name(bs->blk) : bs->node_name;
+}
+
 int bdrv_get_flags(BlockDriverState *bs)
 {
 return bs->open_flags;
diff --git a/block/quorum.c b/block/quorum.c
index 437b122..f91ef75 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -226,10 +226,7 @@ static void quorum_report_bad(QuorumAIOCB *acb, char 
*node_name, int ret)
 
 static void quorum_report_failure(QuorumAIOCB *acb)
 {
-const char *reference = bdrv_get_device_name(acb->common.bs)[0] ?
-bdrv_get_device_name(acb->common.bs) :
-acb->common.bs->node_name;
-
+const char *reference = bdrv_get_device_or_node_name(acb->common.bs);
 qapi_event_send_quorum_failure(reference, acb->sector_num,
acb->nb_sectors, &error_abort);
 }
diff --git a/include/block/block.h b/include/block/block.h
index 4c57d63..b285e0d 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -398,6 +398,7 @@ void bdrv_iterate_format(void (*it)(void *opaque, const 
char *name),
  void *opaque);
 const char *bdrv_get_node_name(const BlockDriverState *bs);
 const char *bdrv_get_device_name(const BlockDriverState *bs);
+const char *bdrv_get_device_or_node_name(const BlockDriverState *bs);
 int bdrv_get_flags(BlockDriverState *bs);
 int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num,
   const uint8_t *buf, int nb_sectors);
-- 
2.1.4




Re: [Qemu-devel] [PATCH v3 for-2.3 0/2] numa: Fix default VCPUs to node mapping

2015-03-20 Thread Eduardo Habkost
On Fri, Mar 20, 2015 at 11:24:14AM +0100, Igor Mammedov wrote:
> On Thu, 19 Mar 2015 17:09:20 +
> Igor Mammedov  wrote:
> 
> > Changes since v2:
> >  - fix spelling errors
> >  - split out PC hunks itno a separate patch
> > 
> > Igor Mammedov (2):
> >   numa: introduce machine callback for VCPU to node mapping
> >   pc: fix default VCPU to NUMA node mapping
> > 
> >  hw/i386/pc.c  |  9 +
> >  include/hw/boards.h   |  5 +
> >  include/sysemu/numa.h |  3 ++-
> >  numa.c| 18 +-
> >  vl.c  |  2 +-
> >  5 files changed, 30 insertions(+), 7 deletions(-)
> > 
> 
> CCing stable which I've forgotten to do earlier

Why stable? This can be easily worked around by explicitly setting the
CPUs for each node. I see this as an usbility improvement, not a bug
fix. And it breaks compatibility.

-- 
Eduardo



Re: [Qemu-devel] [PATCH v3 for-2.3 0/2] numa: Fix default VCPUs to node mapping

2015-03-20 Thread Igor Mammedov
On Fri, 20 Mar 2015 11:52:46 -0300
Eduardo Habkost  wrote:

> On Fri, Mar 20, 2015 at 11:24:14AM +0100, Igor Mammedov wrote:
> > On Thu, 19 Mar 2015 17:09:20 +
> > Igor Mammedov  wrote:
> > 
> > > Changes since v2:
> > >  - fix spelling errors
> > >  - split out PC hunks itno a separate patch
> > > 
> > > Igor Mammedov (2):
> > >   numa: introduce machine callback for VCPU to node mapping
> > >   pc: fix default VCPU to NUMA node mapping
> > > 
> > >  hw/i386/pc.c  |  9 +
> > >  include/hw/boards.h   |  5 +
> > >  include/sysemu/numa.h |  3 ++-
> > >  numa.c| 18 +-
> > >  vl.c  |  2 +-
> > >  5 files changed, 30 insertions(+), 7 deletions(-)
> > > 
> > 
> > CCing stable which I've forgotten to do earlier
> 
> Why stable? This can be easily worked around by explicitly setting the
> CPUs for each node. I see this as an usbility improvement, not a bug
> fix. And it breaks compatibility.
For me if guest hangs due to incorrect topology provided by QEMU, it's a bug.
I'm not sure about what compatibility it breaks though,
but I won't insist on making downstream (2.2 based) live easier
in this case since there is workaround.






Re: [Qemu-devel] [PATCH] e1000: work around win 8.0 boot hang

2015-03-20 Thread Wei Huang


On 02/24/2015 05:46 AM, Stefan Hajnoczi wrote:
> On Tue, Feb 24, 2015 at 11:35 AM, Stefan Hajnoczi  wrote:
>> On Thu, Feb 19, 2015 at 08:24:19PM +0100, Radim Krčmář wrote:
>>> Window 8.0 driver has a particular behavior for a small time frame after
>>> it enables rx interrupts:  the interrupt handler never clears
>>> E1000_ICR_RXT0.  The handler does this something like this:
>>>   set_imc(-1)   (1) disable all interrupts
>>>   val = read_icr()  (2) clear ICR
>>>   handled = magic(val)  (3) do nothing to E1000_ICR_RXT0
>>>   set_ics(val & ~handled)   (4) set unhandled interrupts back to ICR
>>>   set_ims(157)  (5) enable some interrupts
>>>
>>> so if we started with RXT0, then every time the handler re-enables e1000
>>> interrupts, it receives one.  This likely wouldn't matter in real
>>> hardware, because it is slow enough to make some progress between
>>> interrupts, but KVM instantly interrupts it, and boot hangs.
>>> (If we have multiple VCPUs, the interrupt gets load-balanced and
>>>  everything is fine.)
>>>
>>> I haven't found any problem in earlier phase of initialization and
>>> windows writes 0 to RADV and RDTR, so some workaround looks like the
>>> only way if we want to support win8.0 on uniprocessors.  (I vote NO.)
>>>
>>> This workaround uses the fact that a constant is cleared from ICR and
>>> later set back to it.  After detecting this situation, we reuse the
>>> mitigation framework to inject an interrupt 10 microseconds later.
>>> (It's not exactly 10 microseconds, to keep the existing logic intact.)
>>>
>>> The detection is done by checking at (1), (2), and (5).  (2) and (5)
>>> require that the only bit in ICR is RXT0.  We could also check at (4),
>>> and on writes to any other register, but it would most likely only add
>>> more useless code, because normal operations shouldn't behave like that
>>> anyway.  (An OS that deliberately keeps bits in ICR to notify itself
>>> that there are more packets, or for more creative reasons, is nothing we
>>> should care about.)
>>>
>>> Signed-off-by: Radim Krčmář 
>>> ---
>>>  The patch is still untested -- it only approximates the behavior of RHEL
>>>  patches that worked, I'll try to get a reproducer ...
>>>
>>>  hw/net/e1000.c | 29 ++---
>>>  1 file changed, 22 insertions(+), 7 deletions(-)
>>
>> Hi Alex,
>> I've CCed you in case you have any advice regarding QEMU's e1000
>> emulation.  It seems Windows 8 gets itself into a kind of interrupt
>> storm and a workaround in QEMU will be necessary.
>>
>> Any thoughts?
> 
> Okay, I guess Alex has changed jobs since the email has bounced.  Too
> bad, it was worth a shot.
> 
> Regarding the workaround, I'm okay with it.  It's a hack for sure but
> what other option do we have?
> 
I wasn't able to reproduce this problem with upstream QEMU. According to
Radim, this bug requires a very subtle timing during guest installation.
So probably my testing didn't hit the right timing. Additionally our QE
confirmed that this patch fixed a Win8 installation issue that were seen
on in-house QEMU (e.g. qemu-kvm-rhev). With that, I am OK with this
patch. The only thing left is to fix the compilation in this patch (as
Radim pointed out). Anyway,

Reviewed-by: Wei Huang 

Thanks,
-Wei

> Stefan
> 



Re: [Qemu-devel] [PATCH] target-tricore: fix DVINIT_HU/BU calculating overflow before result

2015-03-20 Thread Bastian Koppelmann

On 03/20/2015 02:34 PM, Bastian Koppelmann wrote:

dvinit_hu/bu for ISA v1.3 calculate the higher part of the result, that is 
needed
for the overflow bits, after calculating the overflow bits.

Signed-off-by: Bastian Koppelmann 
---
  target-tricore/translate.c | 29 +
  1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index 0b7cf06..d3680c3 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -6240,7 +6240,7 @@ static void decode_rr_divide(CPUTriCoreState *env, 
DisasContext *ctx)
  uint32_t op2;
  int r1, r2, r3;
  
-TCGv temp, temp2;

+TCGv temp, temp2, temp3;
  
  op2 = MASK_OP_RR_OP2(ctx->opcode);

  r3 = MASK_OP_RR_D(ctx->opcode);
@@ -6261,14 +6261,17 @@ static void decode_rr_divide(CPUTriCoreState *env, 
DisasContext *ctx)
  case OPC2_32_RR_DVINIT_BU:
  temp = tcg_temp_new();
  temp2 = tcg_temp_new();
+temp3 = tcg_temp_new();
+
+tcg_gen_shri_tl(temp3, cpu_gpr_d[r1], 8);
  /* reset av */
  tcg_gen_movi_tl(cpu_PSW_AV, 0);
  if (!tricore_feature(env, TRICORE_FEATURE_131)) {
  /* overflow = (abs(D[r3+1]) >= abs(D[r2])) */
-tcg_gen_neg_tl(temp, cpu_gpr_d[r3+1]);
+tcg_gen_neg_tl(temp, temp3);
  /* use cpu_PSW_AV to compare against 0 */
-tcg_gen_movcond_tl(TCG_COND_LT, temp, cpu_gpr_d[r3+1], cpu_PSW_AV,
-   temp, cpu_gpr_d[r3+1]);
+tcg_gen_movcond_tl(TCG_COND_LT, temp, temp3, cpu_PSW_AV,
+   temp, temp3);
  tcg_gen_neg_tl(temp2, cpu_gpr_d[r2]);
  tcg_gen_movcond_tl(TCG_COND_LT, temp2, cpu_gpr_d[r2], cpu_PSW_AV,
 temp2, cpu_gpr_d[r2]);
@@ -6281,9 +6284,8 @@ static void decode_rr_divide(CPUTriCoreState *env, 
DisasContext *ctx)
  /* sv */
  tcg_gen_or_tl(cpu_PSW_SV, cpu_PSW_SV, cpu_PSW_V);
  /* write result */
-tcg_gen_shri_tl(temp, cpu_gpr_d[r1], 8);
  tcg_gen_shli_tl(cpu_gpr_d[r3], cpu_gpr_d[r1], 24);
-tcg_gen_mov_tl(cpu_gpr_d[r3+1], temp);
+tcg_gen_mov_tl(cpu_gpr_d[r3+1], temp3);
  
  tcg_temp_free(temp);

  tcg_temp_free(temp2);

Mh, I forgot to free temp3 here. I'll change that after I applied it to 
my tricore-next branch.


Cheers,
Bastian



[Qemu-devel] [PATCH 0/6 v7] tilegx: Firstly add tilegx feature for linux-user

2015-03-20 Thread Chen Gang
After load elf64 binary, qemu tilegx can finish executing the first
system call (uname) successfully in  _dl_discover_osversion(), and
return to __libc_start_main().

Chen Gang (6):
  target-tilegx: Firstly add TILE-Gx with minimized features
  linux-user: tilegx: Firstly add architecture related features
  linux-user: tilegx: Add target features support within qemu
  linux-user: Support tilegx architecture in syscall
  linux-user: Support tilegx architecture in linux-user
  linux-user/syscall.c: conditionalize syscalls which are not defined in
tilegx

 configure |3 +
 default-configs/tilegx-linux-user.mak |1 +
 include/elf.h |2 +
 linux-user/elfload.c  |   23 +
 linux-user/main.c |   86 ++
 linux-user/syscall.c  |   50 +-
 linux-user/syscall_defs.h |   38 +-
 linux-user/tilegx/syscall.h   |   80 ++
 linux-user/tilegx/syscall_nr.h|  278 ++
 linux-user/tilegx/target_cpu.h|   35 +
 linux-user/tilegx/target_signal.h |   28 +
 linux-user/tilegx/target_structs.h|   48 +
 linux-user/tilegx/termbits.h  |  285 ++
 target-tilegx/Makefile.objs   |1 +
 target-tilegx/cpu-qom.h   |   73 ++
 target-tilegx/cpu.c   |  149 +++
 target-tilegx/cpu.h   |   94 ++
 target-tilegx/helper.c|   31 +
 target-tilegx/helper.h|1 +
 target-tilegx/opcode_tilegx.h | 1406 ++
 target-tilegx/translate.c | 1764 +
 21 files changed, 4471 insertions(+), 5 deletions(-)
 create mode 100644 default-configs/tilegx-linux-user.mak
 create mode 100644 linux-user/tilegx/syscall.h
 create mode 100644 linux-user/tilegx/syscall_nr.h
 create mode 100644 linux-user/tilegx/target_cpu.h
 create mode 100644 linux-user/tilegx/target_signal.h
 create mode 100644 linux-user/tilegx/target_structs.h
 create mode 100644 linux-user/tilegx/termbits.h
 create mode 100644 target-tilegx/Makefile.objs
 create mode 100644 target-tilegx/cpu-qom.h
 create mode 100644 target-tilegx/cpu.c
 create mode 100644 target-tilegx/cpu.h
 create mode 100644 target-tilegx/helper.c
 create mode 100644 target-tilegx/helper.h
 create mode 100644 target-tilegx/opcode_tilegx.h
 create mode 100644 target-tilegx/translate.c

-- 
1.9.3



[Qemu-devel] [PATCH 1/6 v7] target-tilegx: Firstly add TILE-Gx with minimized features

2015-03-20 Thread Chen Gang
It is the configure and build system support for TILE-Gx (tilegx will be
used in configure and real sub-directory name).

At present, it is linux-user only, and can finish the first system call
(uname) execution in __libc_start_main().

Signed-off-by: Chen Gang 
---
 configure |3 +
 default-configs/tilegx-linux-user.mak |1 +
 target-tilegx/Makefile.objs   |1 +
 target-tilegx/cpu-qom.h   |   73 ++
 target-tilegx/cpu.c   |  149 +++
 target-tilegx/cpu.h   |   94 ++
 target-tilegx/helper.c|   31 +
 target-tilegx/helper.h|1 +
 target-tilegx/opcode_tilegx.h | 1406 ++
 target-tilegx/translate.c | 1764 +
 10 files changed, 3523 insertions(+)
 create mode 100644 default-configs/tilegx-linux-user.mak
 create mode 100644 target-tilegx/Makefile.objs
 create mode 100644 target-tilegx/cpu-qom.h
 create mode 100644 target-tilegx/cpu.c
 create mode 100644 target-tilegx/cpu.h
 create mode 100644 target-tilegx/helper.c
 create mode 100644 target-tilegx/helper.h
 create mode 100644 target-tilegx/opcode_tilegx.h
 create mode 100644 target-tilegx/translate.c

diff --git a/configure b/configure
index 589798e..a8704ec 100755
--- a/configure
+++ b/configure
@@ -5215,6 +5215,9 @@ case "$target_name" in
   s390x)
 gdb_xml_files="s390x-core64.xml s390-acr.xml s390-fpr.xml"
   ;;
+  tilegx)
+TARGET_ARCH=tilegx
+  ;;
   unicore32)
   ;;
   xtensa|xtensaeb)
diff --git a/default-configs/tilegx-linux-user.mak 
b/default-configs/tilegx-linux-user.mak
new file mode 100644
index 000..3e47493
--- /dev/null
+++ b/default-configs/tilegx-linux-user.mak
@@ -0,0 +1 @@
+# Default configuration for tilegx-linux-user
diff --git a/target-tilegx/Makefile.objs b/target-tilegx/Makefile.objs
new file mode 100644
index 000..8b3dc76
--- /dev/null
+++ b/target-tilegx/Makefile.objs
@@ -0,0 +1 @@
+obj-y += cpu.o translate.o helper.o
diff --git a/target-tilegx/cpu-qom.h b/target-tilegx/cpu-qom.h
new file mode 100644
index 000..5615c3b
--- /dev/null
+++ b/target-tilegx/cpu-qom.h
@@ -0,0 +1,73 @@
+/*
+ * QEMU TILE-Gx CPU
+ *
+ * Copyright (c) 2015 Chen Gang
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see
+ * 
+ */
+#ifndef QEMU_TILEGX_CPU_QOM_H
+#define QEMU_TILEGX_CPU_QOM_H
+
+#include "qom/cpu.h"
+
+#define TYPE_TILEGX_CPU "tilegx-cpu"
+
+#define TILEGX_CPU_CLASS(klass) \
+OBJECT_CLASS_CHECK(TileGXCPUClass, (klass), TYPE_TILEGX_CPU)
+#define TILEGX_CPU(obj) \
+OBJECT_CHECK(TileGXCPU, (obj), TYPE_TILEGX_CPU)
+#define TILEGX_CPU_GET_CLASS(obj) \
+OBJECT_GET_CLASS(TileGXCPUClass, (obj), TYPE_TILEGX_CPU)
+
+/**
+ * TileGXCPUClass:
+ * @parent_realize: The parent class' realize handler.
+ * @parent_reset: The parent class' reset handler.
+ *
+ * A Tile-Gx CPU model.
+ */
+typedef struct TileGXCPUClass {
+/*< private >*/
+CPUClass parent_class;
+/*< public >*/
+
+DeviceRealize parent_realize;
+void (*parent_reset)(CPUState *cpu);
+} TileGXCPUClass;
+
+/**
+ * TileGXCPU:
+ * @env: #CPUTLGState
+ *
+ * A Tile-GX CPU.
+ */
+typedef struct TileGXCPU {
+/*< private >*/
+CPUState parent_obj;
+/*< public >*/
+
+CPUTLGState env;
+} TileGXCPU;
+
+static inline TileGXCPU *tilegx_env_get_cpu(CPUTLGState *env)
+{
+return container_of(env, TileGXCPU, env);
+}
+
+#define ENV_GET_CPU(e) CPU(tilegx_env_get_cpu(e))
+
+#define ENV_OFFSET offsetof(TileGXCPU, env)
+
+#endif
diff --git a/target-tilegx/cpu.c b/target-tilegx/cpu.c
new file mode 100644
index 000..8255fdc
--- /dev/null
+++ b/target-tilegx/cpu.c
@@ -0,0 +1,149 @@
+/*
+ * QEMU TILE-Gx CPU
+ *
+ *  Copyright (c) 2015 Chen Gang
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public

[Qemu-devel] [PATCH 2/6 v7] linux-user: tilegx: Firstly add architecture related features

2015-03-20 Thread Chen Gang
They are based on Linux kernel tilegx architecture for 64 bit binary,
also based on tilegx ABI reference document.

Signed-off-by: Chen Gang 
---
 linux-user/tilegx/syscall.h|  80 
 linux-user/tilegx/syscall_nr.h | 278 
 linux-user/tilegx/termbits.h   | 285 +
 3 files changed, 643 insertions(+)
 create mode 100644 linux-user/tilegx/syscall.h
 create mode 100644 linux-user/tilegx/syscall_nr.h
 create mode 100644 linux-user/tilegx/termbits.h

diff --git a/linux-user/tilegx/syscall.h b/linux-user/tilegx/syscall.h
new file mode 100644
index 000..561e158
--- /dev/null
+++ b/linux-user/tilegx/syscall.h
@@ -0,0 +1,80 @@
+#ifndef TILEGX_SYSCALLS_H
+#define TILEGX_SYSCALLS_H
+
+#define UNAME_MACHINE "tilegx"
+#define UNAME_MINIMUM_RELEASE "3.19"
+
+/* We use tilegx to keep things similar to the kernel sources.  */
+typedef uint64_t tilegx_reg_t;
+
+struct target_pt_regs {
+
+/* Can be as parameters */
+tilegx_reg_t r0;  /* Also for return value, both function and system call 
*/
+tilegx_reg_t r1;
+tilegx_reg_t r2;
+tilegx_reg_t r3;
+tilegx_reg_t r4;
+tilegx_reg_t r5;
+tilegx_reg_t r6;
+tilegx_reg_t r7;
+tilegx_reg_t r8;
+tilegx_reg_t r9;
+
+/* Normal using, caller saved */
+tilegx_reg_t r10;  /* Also for system call */
+tilegx_reg_t r11;
+tilegx_reg_t r12;
+tilegx_reg_t r13;
+tilegx_reg_t r14;
+tilegx_reg_t r15;
+tilegx_reg_t r16;
+tilegx_reg_t r17;
+tilegx_reg_t r18;
+tilegx_reg_t r19;
+tilegx_reg_t r20;
+tilegx_reg_t r21;
+tilegx_reg_t r22;
+tilegx_reg_t r23;
+tilegx_reg_t r24;
+tilegx_reg_t r25;
+tilegx_reg_t r26;
+tilegx_reg_t r27;
+tilegx_reg_t r28;
+tilegx_reg_t r29;
+
+/* Normal using, callee saved */
+tilegx_reg_t r30;
+tilegx_reg_t r31;
+tilegx_reg_t r32;
+tilegx_reg_t r33;
+tilegx_reg_t r34;
+tilegx_reg_t r35;
+tilegx_reg_t r36;
+tilegx_reg_t r37;
+tilegx_reg_t r38;
+tilegx_reg_t r39;
+tilegx_reg_t r40;
+tilegx_reg_t r41;
+tilegx_reg_t r42;
+tilegx_reg_t r43;
+tilegx_reg_t r44;
+tilegx_reg_t r45;
+tilegx_reg_t r46;
+tilegx_reg_t r47;
+tilegx_reg_t r48;
+tilegx_reg_t r49;
+tilegx_reg_t r50;
+tilegx_reg_t r51;
+
+/* Control using */
+tilegx_reg_t r52;/* optional frame pointer */
+tilegx_reg_t tp; /* thread-local data */
+tilegx_reg_t sp; /* stack pointer */
+tilegx_reg_t lr; /* lr pointer */
+};
+
+#define TARGET_MLOCKALL_MCL_CURRENT 1
+#define TARGET_MLOCKALL_MCL_FUTURE  2
+
+#endif
diff --git a/linux-user/tilegx/syscall_nr.h b/linux-user/tilegx/syscall_nr.h
new file mode 100644
index 000..8121154
--- /dev/null
+++ b/linux-user/tilegx/syscall_nr.h
@@ -0,0 +1,278 @@
+#ifndef TILEGX_SYSCALL_NR
+#define TILEGX_SYSCALL_NR
+
+/*
+ * Copy from linux kernel asm-generic/unistd.h, which tilegx uses.
+ */
+#define TARGET_NR_io_setup  0
+#define TARGET_NR_io_destroy1
+#define TARGET_NR_io_submit 2
+#define TARGET_NR_io_cancel 3
+#define TARGET_NR_io_getevents  4
+#define TARGET_NR_setxattr  5
+#define TARGET_NR_lsetxattr 6
+#define TARGET_NR_fsetxattr 7
+#define TARGET_NR_getxattr  8
+#define TARGET_NR_lgetxattr 9
+#define TARGET_NR_fgetxattr 10
+#define TARGET_NR_listxattr 11
+#define TARGET_NR_llistxattr12
+#define TARGET_NR_flistxattr13
+#define TARGET_NR_removexattr   14
+#define TARGET_NR_lremovexattr  15
+#define TARGET_NR_fremovexattr  16
+#define TARGET_NR_getcwd17
+#define TARGET_NR_lookup_dcookie18
+#define TARGET_NR_eventfd2  19
+#define TARGET_NR_epoll_create1 20
+#define TARGET_NR_epoll_ctl 21
+#define TARGET_NR_epoll_pwait   22
+#define TARGET_NR_dup   23
+#define TARGET_NR_dup3  24
+#define TARGET_NR_fcntl 25
+#define TARGET_NR_inotify_init1 26
+#define TARGET_NR_inotify_add_watch 27
+#define TARGET_NR_inotify_rm_watch  28
+#define TARGET_NR_ioctl 29
+#define TARGET_NR_ioprio_set30
+#define TARGET_NR_ioprio_get31
+#define TARGET_NR_flock 32
+#define TARGET_NR_mknodat   33
+#define TARGET_NR_mkdirat   34
+#define TARGET_NR_unlinkat  35
+#define TARGET_NR_symlinkat 36
+#define TARGET_NR_linkat37
+#define TARGET_NR_renameat   

[Qemu-devel] [PATCH 3/6 v7] linux-user: tilegx: Add target features support within qemu

2015-03-20 Thread Chen Gang
They are for target features within qemu which independent from outside.

Signed-off-by: Chen Gang 
---
 linux-user/tilegx/target_cpu.h | 35 +++
 linux-user/tilegx/target_signal.h  | 28 ++
 linux-user/tilegx/target_structs.h | 48 ++
 3 files changed, 111 insertions(+)
 create mode 100644 linux-user/tilegx/target_cpu.h
 create mode 100644 linux-user/tilegx/target_signal.h
 create mode 100644 linux-user/tilegx/target_structs.h

diff --git a/linux-user/tilegx/target_cpu.h b/linux-user/tilegx/target_cpu.h
new file mode 100644
index 000..c96e81d
--- /dev/null
+++ b/linux-user/tilegx/target_cpu.h
@@ -0,0 +1,35 @@
+/*
+ * TILE-Gx specific CPU ABI and functions for linux-user
+ *
+ * Copyright (c) 2015 Chen Gang
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ */
+#ifndef TARGET_CPU_H
+#define TARGET_CPU_H
+
+static inline void cpu_clone_regs(CPUTLGState *env, target_ulong newsp)
+{
+if (newsp) {
+env->regs[TILEGX_R_SP] = newsp;
+}
+env->regs[TILEGX_R_RE] = 0;
+}
+
+static inline void cpu_set_tls(CPUTLGState *env, target_ulong newtls)
+{
+env->regs[TILEGX_R_TP] = newtls;
+}
+
+#endif
diff --git a/linux-user/tilegx/target_signal.h 
b/linux-user/tilegx/target_signal.h
new file mode 100644
index 000..fbab216
--- /dev/null
+++ b/linux-user/tilegx/target_signal.h
@@ -0,0 +1,28 @@
+#ifndef TARGET_SIGNAL_H
+#define TARGET_SIGNAL_H
+
+#include "cpu.h"
+
+/* this struct defines a stack used during syscall handling */
+
+typedef struct target_sigaltstack {
+abi_ulong ss_sp;
+abi_ulong ss_size;
+abi_long ss_flags;
+} target_stack_t;
+
+/*
+ * sigaltstack controls
+ */
+#define TARGET_SS_ONSTACK 1
+#define TARGET_SS_DISABLE 2
+
+#define TARGET_MINSIGSTKSZ2048
+#define TARGET_SIGSTKSZ   8192
+
+static inline abi_ulong get_sp_from_cpustate(CPUTLGState *state)
+{
+return state->regs[TILEGX_R_SP];
+}
+
+#endif /* TARGET_SIGNAL_H */
diff --git a/linux-user/tilegx/target_structs.h 
b/linux-user/tilegx/target_structs.h
new file mode 100644
index 000..13a1505
--- /dev/null
+++ b/linux-user/tilegx/target_structs.h
@@ -0,0 +1,48 @@
+/*
+ * TILE-Gx specific structures for linux-user
+ *
+ * Copyright (c) 2015 Chen Gang
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ */
+#ifndef TARGET_STRUCTS_H
+#define TARGET_STRUCTS_H
+
+struct target_ipc_perm {
+abi_int __key;  /* Key.  */
+abi_uint uid;   /* Owner's user ID.  */
+abi_uint gid;   /* Owner's group ID.  */
+abi_uint cuid;  /* Creator's user ID.  */
+abi_uint cgid;  /* Creator's group ID.  */
+abi_uint mode;/* Read/write permission.  */
+abi_ushort __seq;   /* Sequence number.  */
+abi_ushort __pad2;
+abi_ulong __unused1;
+abi_ulong __unused2;
+};
+
+struct target_shmid_ds {
+struct target_ipc_perm shm_perm;/* operation permission struct */
+abi_long shm_segsz; /* size of segment in bytes */
+abi_ulong shm_atime;/* time of last shmat() */
+abi_ulong shm_dtime;/* time of last shmdt() */
+abi_ulong shm_ctime;/* time of last change by shmctl() */
+abi_int shm_cpid;   /* pid of creator */
+abi_int shm_lpid;   /* pid of last shmop */
+abi_ulong shm_nattch;   /* number of current attaches */
+abi_ulong __unused4;
+abi_ulong __unused5;
+};
+
+#endif
-- 
1.9.3



[Qemu-devel] [PATCH 4/6 v7] linux-user: Support tilegx architecture in syscall

2015-03-20 Thread Chen Gang
Add tilegx architecture in "syscall_defs.h", all related features (ioctrl,
and stat) are based on Linux kernel tilegx 64-bit implementation.

Signed-off-by: Chen Gang 
---
 linux-user/syscall_defs.h | 38 ++
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index edd5f3c..023f4b5 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -64,8 +64,9 @@
 #endif
 
 #if defined(TARGET_I386) || defined(TARGET_ARM) || defined(TARGET_SH4) \
-|| defined(TARGET_M68K) || defined(TARGET_CRIS) || 
defined(TARGET_UNICORE32) \
-|| defined(TARGET_S390X) || defined(TARGET_OPENRISC)
+|| defined(TARGET_M68K) || defined(TARGET_CRIS) \
+|| defined(TARGET_UNICORE32) || defined(TARGET_S390X) \
+|| defined(TARGET_OPENRISC) || defined(TARGET_TILEGX)
 
 #define TARGET_IOC_SIZEBITS14
 #define TARGET_IOC_DIRBITS 2
@@ -365,7 +366,8 @@ int do_sigaction(int sig, const struct target_sigaction 
*act,
 || defined(TARGET_PPC) || defined(TARGET_MIPS) || defined(TARGET_SH4) \
 || defined(TARGET_M68K) || defined(TARGET_ALPHA) || defined(TARGET_CRIS) \
 || defined(TARGET_MICROBLAZE) || defined(TARGET_UNICORE32) \
-|| defined(TARGET_S390X) || defined(TARGET_OPENRISC)
+|| defined(TARGET_S390X) || defined(TARGET_OPENRISC) \
+|| defined(TARGET_TILEGX)
 
 #if defined(TARGET_SPARC)
 #define TARGET_SA_NOCLDSTOP8u
@@ -1922,6 +1924,32 @@ struct target_stat64 {
 unsigned int __unused5;
 };
 
+#elif defined(TARGET_TILEGX)
+
+/* Copy from Linux kernel "uapi/asm-generic/stat.h" */
+struct target_stat {
+abi_ulong st_dev;   /* Device.  */
+abi_ulong st_ino;   /* File serial number.  */
+unsigned int st_mode;   /* File mode.  */
+unsigned int st_nlink;  /* Link count.  */
+unsigned int st_uid;/* User ID of the file's owner.  */
+unsigned int st_gid;/* Group ID of the file's group. */
+abi_ulong st_rdev;  /* Device number, if device.  */
+abi_ulong __pad1;
+abi_long  st_size;  /* Size of file, in bytes.  */
+int st_blksize; /* Optimal block size for I/O.  */
+int __pad2;
+abi_long st_blocks; /* Number 512-byte blocks allocated. */
+abi_long target_st_atime;   /* Time of last access.  */
+abi_ulong target_st_atime_nsec;
+abi_long target_st_mtime;   /* Time of last modification.  */
+abi_ulong target_st_mtime_nsec;
+abi_long target_st_ctime;   /* Time of last status change.  */
+abi_ulong target_st_ctime_nsec;
+unsigned int __unused4;
+unsigned int __unused5;
+};
+
 #else
 #error unsupported CPU
 #endif
@@ -2264,7 +2292,9 @@ struct target_flock {
 struct target_flock64 {
short  l_type;
short  l_whence;
-#if defined(TARGET_PPC) || defined(TARGET_X86_64) || defined(TARGET_MIPS) || 
defined(TARGET_SPARC) || defined(TARGET_HPPA) || defined (TARGET_MICROBLAZE)
+#if defined(TARGET_PPC) || defined(TARGET_X86_64) || defined(TARGET_MIPS) \
+|| defined(TARGET_SPARC) || defined(TARGET_HPPA) \
+|| defined(TARGET_MICROBLAZE) || defined(TARGET_TILEGX)
 int __pad;
 #endif
unsigned long long l_start;
-- 
1.9.3




[Qemu-devel] [PATCH 5/6 v7] linux-user: Support tilegx architecture in linux-user

2015-03-20 Thread Chen Gang
Add main working flow feature, system call processing feature, and elf64
tilegx binary loading feature, based on Linux kernel tilegx 64-bit
implementation.

Signed-off-by: Chen Gang 
---
 include/elf.h|  2 ++
 linux-user/elfload.c | 23 ++
 linux-user/main.c| 86 
 3 files changed, 111 insertions(+)

diff --git a/include/elf.h b/include/elf.h
index 3e75f05..154144e 100644
--- a/include/elf.h
+++ b/include/elf.h
@@ -133,6 +133,8 @@ typedef int64_t  Elf64_Sxword;
 
 #define EM_AARCH64  183
 
+#define EM_TILEGX   191 /* TILE-Gx */
+
 /* This is the info that is needed to parse the dynamic section of the file */
 #define DT_NULL0
 #define DT_NEEDED  1
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 399c021..2571cb8 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1189,6 +1189,29 @@ static inline void init_thread(struct target_pt_regs 
*regs, struct image_info *i
 
 #endif /* TARGET_S390X */
 
+#ifdef TARGET_TILEGX
+
+/* 42 bits real used address, a half for user mode */
+#define ELF_START_MMAP (0x00200ULL)
+
+#define elf_check_arch(x) ((x) == EM_TILEGX)
+
+#define ELF_CLASS   ELFCLASS64
+#define ELF_DATAELFDATA2LSB
+#define ELF_ARCHEM_TILEGX
+
+static inline void init_thread(struct target_pt_regs *regs,
+   struct image_info *infop)
+{
+regs->lr = infop->entry;
+regs->sp = infop->start_stack;
+
+}
+
+#define ELF_EXEC_PAGESIZE65536 /* TILE-Gx page size is 64KB */
+
+#endif /* TARGET_TILEGX */
+
 #ifndef ELF_PLATFORM
 #define ELF_PLATFORM (NULL)
 #endif
diff --git a/linux-user/main.c b/linux-user/main.c
index 6e446de..ecfc80b 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3418,6 +3418,32 @@ void cpu_loop(CPUS390XState *env)
 
 #endif /* TARGET_S390X */
 
+#ifdef TARGET_TILEGX
+void cpu_loop(CPUTLGState *env)
+{
+CPUState *cs = CPU(tilegx_env_get_cpu(env));
+int trapnr;
+
+while (1) {
+cpu_exec_start(cs);
+trapnr = cpu_tilegx_exec(env);
+cpu_exec_end(cs);
+switch (trapnr) {
+case TILEGX_EXCP_SYSCALL:
+env->regs[TILEGX_R_RE] = do_syscall(env, env->regs[TILEGX_R_NR],
+env->regs[0], env->regs[1],
+env->regs[2], env->regs[3],
+env->regs[4], env->regs[5],
+env->regs[6], env->regs[7]);
+break;
+default:
+exit(-1);
+}
+process_pending_signals(env);
+}
+}
+#endif
+
 THREAD CPUState *thread_cpu;
 
 void task_settid(TaskState *ts)
@@ -4392,6 +4418,66 @@ int main(int argc, char **argv, char **envp)
 env->psw.mask = regs->psw.mask;
 env->psw.addr = regs->psw.addr;
 }
+#elif defined(TARGET_TILEGX)
+{
+env->regs[0] = regs->r0;
+env->regs[1] = regs->r1;
+env->regs[2] = regs->r2;
+env->regs[3] = regs->r3;
+env->regs[4] = regs->r4;
+env->regs[5] = regs->r5;
+env->regs[6] = regs->r6;
+env->regs[7] = regs->r7;
+env->regs[8] = regs->r8;
+env->regs[9] = regs->r9;
+env->regs[10] = regs->r10;
+env->regs[11] = regs->r11;
+env->regs[12] = regs->r12;
+env->regs[13] = regs->r13;
+env->regs[14] = regs->r14;
+env->regs[15] = regs->r15;
+env->regs[16] = regs->r16;
+env->regs[17] = regs->r17;
+env->regs[18] = regs->r18;
+env->regs[19] = regs->r19;
+env->regs[20] = regs->r20;
+env->regs[21] = regs->r21;
+env->regs[22] = regs->r22;
+env->regs[23] = regs->r23;
+env->regs[24] = regs->r24;
+env->regs[25] = regs->r25;
+env->regs[26] = regs->r26;
+env->regs[27] = regs->r27;
+env->regs[28] = regs->r28;
+env->regs[29] = regs->r29;
+env->regs[30] = regs->r30;
+env->regs[31] = regs->r31;
+env->regs[32] = regs->r32;
+env->regs[33] = regs->r33;
+env->regs[34] = regs->r34;
+env->regs[35] = regs->r35;
+env->regs[36] = regs->r36;
+env->regs[37] = regs->r37;
+env->regs[38] = regs->r38;
+env->regs[39] = regs->r39;
+env->regs[40] = regs->r40;
+env->regs[41] = regs->r41;
+env->regs[42] = regs->r42;
+env->regs[43] = regs->r43;
+env->regs[44] = regs->r44;
+env->regs[45] = regs->r45;
+env->regs[46] = regs->r46;
+env->regs[47] = regs->r47;
+env->regs[48] = regs->r48;
+env->regs[49] = regs->r49;
+env->regs[50] = regs->r50;
+env->regs[51] = regs->r51;
+env->regs[52] = regs->r52; /* TILEGX_R_BP */
+env->regs[53] = regs->tp;  /* TILEGX_R_TP */
+env->regs[54] = regs->sp;  /* TILEGX_R_SP */
+env->regs[5

[Qemu-devel] [PATCH 6/6 v7] linux-user/syscall.c: conditionalize syscalls which are not defined in tilegx

2015-03-20 Thread Chen Gang
For tilegx, several syscall macros are not supported, so switch them to
avoid building break.

Signed-off-by: Chen Gang 
---
 linux-user/syscall.c | 50 +-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 5720195..d1a00ad 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -213,7 +213,7 @@ static int gettid(void) {
 return -ENOSYS;
 }
 #endif
-#ifdef __NR_getdents
+#if defined(TARGET_NR_getdents) && defined(__NR_getdents)
 _syscall3(int, sys_getdents, uint, fd, struct linux_dirent *, dirp, uint, 
count);
 #endif
 #if !defined(__NR_getdents) || \
@@ -5580,6 +5580,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 ret = get_errno(write(arg1, p, arg3));
 unlock_user(p, arg2, 0);
 break;
+#ifdef TARGET_NR_open
 case TARGET_NR_open:
 if (!(p = lock_user_string(arg1)))
 goto efault;
@@ -5588,6 +5589,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
   arg3));
 unlock_user(p, arg1, 0);
 break;
+#endif
 case TARGET_NR_openat:
 if (!(p = lock_user_string(arg2)))
 goto efault;
@@ -5602,9 +5604,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 case TARGET_NR_brk:
 ret = do_brk(arg1);
 break;
+#ifdef TARGET_NR_fork
 case TARGET_NR_fork:
 ret = get_errno(do_fork(cpu_env, SIGCHLD, 0, 0, 0, 0));
 break;
+#endif
 #ifdef TARGET_NR_waitpid
 case TARGET_NR_waitpid:
 {
@@ -5639,6 +5643,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 unlock_user(p, arg1, 0);
 break;
 #endif
+#ifdef TARGET_NR_link
 case TARGET_NR_link:
 {
 void * p2;
@@ -5652,6 +5657,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 unlock_user(p, arg1, 0);
 }
 break;
+#endif
 #if defined(TARGET_NR_linkat)
 case TARGET_NR_linkat:
 {
@@ -5669,12 +5675,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 }
 break;
 #endif
+#ifdef TARGET_NR_unlink
 case TARGET_NR_unlink:
 if (!(p = lock_user_string(arg1)))
 goto efault;
 ret = get_errno(unlink(p));
 unlock_user(p, arg1, 0);
 break;
+#endif
 #if defined(TARGET_NR_unlinkat)
 case TARGET_NR_unlinkat:
 if (!(p = lock_user_string(arg2)))
@@ -5791,12 +5799,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 }
 break;
 #endif
+#ifdef TARGET_NR_mknod
 case TARGET_NR_mknod:
 if (!(p = lock_user_string(arg1)))
 goto efault;
 ret = get_errno(mknod(p, arg2, arg3));
 unlock_user(p, arg1, 0);
 break;
+#endif
 #if defined(TARGET_NR_mknodat)
 case TARGET_NR_mknodat:
 if (!(p = lock_user_string(arg2)))
@@ -5805,12 +5815,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 unlock_user(p, arg2, 0);
 break;
 #endif
+#ifdef TARGET_NR_chmod
 case TARGET_NR_chmod:
 if (!(p = lock_user_string(arg1)))
 goto efault;
 ret = get_errno(chmod(p, arg2));
 unlock_user(p, arg1, 0);
 break;
+#endif
 #ifdef TARGET_NR_break
 case TARGET_NR_break:
 goto unimplemented;
@@ -5945,6 +5957,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 }
 break;
 #endif
+#ifdef TARGET_NR_utimes
 case TARGET_NR_utimes:
 {
 struct timeval *tvp, tv[2];
@@ -5963,6 +5976,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 unlock_user(p, arg1, 0);
 }
 break;
+#endif
 #if defined(TARGET_NR_futimesat)
 case TARGET_NR_futimesat:
 {
@@ -5991,12 +6005,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 case TARGET_NR_gtty:
 goto unimplemented;
 #endif
+#ifdef TARGET_NR_access
 case TARGET_NR_access:
 if (!(p = lock_user_string(arg1)))
 goto efault;
 ret = get_errno(access(path(p), arg2));
 unlock_user(p, arg1, 0);
 break;
+#endif
 #if defined(TARGET_NR_faccessat) && defined(__NR_faccessat)
 case TARGET_NR_faccessat:
 if (!(p = lock_user_string(arg2)))
@@ -6021,6 +6037,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 case TARGET_NR_kill:
 ret = get_errno(kill(arg1, target_to_host_signal(arg2)));
 break;
+#ifdef TARGET_NR_rename
 case TARGET_NR_rename:
 {
 void *p2;
@@ -6034,6 +6051,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 unlock_user(p, arg1, 0);
 }
 break;
+#endif
 #if defined(TARGET_NR_renameat)
 case TARGET_NR_renameat:
 {
@@ -6049,12 +6067,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 }
 break;
 #endif
+#ifdef TARG

[Qemu-devel] [for-2.3 PATCH] gitignore: Ignore more .pod files.

2015-03-20 Thread Eric Blake
kvm_stat.{1,pod} started showing up as untracked files in my
directory, and I nearly accidentally merged them into a commit
with my usual habit of 'git add .'.  Rather than naming every
such generated doc file, just exclude the pattern.

Signed-off-by: Eric Blake 
CC: qemu-triv...@nongnu.org
---
 .gitignore | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/.gitignore b/.gitignore
index e32a584..da5413d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -37,14 +37,8 @@
 /qemu-tech.html
 /qemu-doc.info
 /qemu-tech.info
-/qemu.1
-/qemu.pod
-/qemu-img.1
-/qemu-img.pod
 /qemu-img
 /qemu-nbd
-/qemu-nbd.8
-/qemu-nbd.pod
 /qemu-options.def
 /qemu-options.texi
 /qemu-img-cmds.texi
@@ -56,8 +50,7 @@
 /qmp-commands.txt
 /vscclient
 /fsdev/virtfs-proxy-helper
-/fsdev/virtfs-proxy-helper.1
-/fsdev/virtfs-proxy-helper.pod
+*.1
 *.a
 *.aux
 *.cp
@@ -70,6 +63,7 @@
 *.ky
 *.log
 *.pdf
+*.pod
 *.cps
 *.fns
 *.kys
-- 
2.1.0




Re: [Qemu-devel] [for-2.3 PATCH] gitignore: Ignore more .pod files.

2015-03-20 Thread Eric Blake
On 03/20/2015 10:26 AM, Eric Blake wrote:
> kvm_stat.{1,pod} started showing up as untracked files in my
> directory, and I nearly accidentally merged them into a commit
> with my usual habit of 'git add .'.  Rather than naming every
> such generated doc file, just exclude the pattern.
> 
> Signed-off-by: Eric Blake 
> CC: qemu-triv...@nongnu.org
> ---

> -/qemu-nbd.8

> +*.1

Blech. Serves me right for not saving my buffer before hitting send.  v2
coming up that doesn't leak qemu-nbd.8 (or any other man page we might
add outside of section 1).

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



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [for-2.3 PATCH v2] gitignore: Ignore more .pod files.

2015-03-20 Thread Eric Blake
kvm_stat.{1,pod} started showing up as untracked files in my
directory, and I nearly accidentally merged them into a commit
with my usual habit of 'git add .'.  Rather than spelling out
each such file, just ignore the entire pattern.

Signed-off-by: Eric Blake 
CC: qemu-triv...@nongnu.org
---

v2: fix man page pattern

 .gitignore | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/.gitignore b/.gitignore
index e32a584..aed0e1f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -37,14 +37,8 @@
 /qemu-tech.html
 /qemu-doc.info
 /qemu-tech.info
-/qemu.1
-/qemu.pod
-/qemu-img.1
-/qemu-img.pod
 /qemu-img
 /qemu-nbd
-/qemu-nbd.8
-/qemu-nbd.pod
 /qemu-options.def
 /qemu-options.texi
 /qemu-img-cmds.texi
@@ -56,8 +50,7 @@
 /qmp-commands.txt
 /vscclient
 /fsdev/virtfs-proxy-helper
-/fsdev/virtfs-proxy-helper.1
-/fsdev/virtfs-proxy-helper.pod
+*.[1-9]
 *.a
 *.aux
 *.cp
@@ -70,6 +63,7 @@
 *.ky
 *.log
 *.pdf
+*.pod
 *.cps
 *.fns
 *.kys
-- 
2.1.0




[Qemu-devel] [PATCH] Probe via #define check for FreeBSD, NetBSD and DragonFly BSD

2015-03-20 Thread Wei Liu
Signed-off-by: Wei Liu 
---
 configure | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/configure b/configure
index 589798e..ceacd81 100755
--- a/configure
+++ b/configure
@@ -441,6 +441,12 @@ elif check_define _WIN32 ; then
   targetos='MINGW32'
 elif check_define __OpenBSD__ ; then
   targetos='OpenBSD'
+elif check_define __FreeBSD__ ; then
+  targetos='FreeBSD'
+elif check_define __DragonFly__ ; then
+  targetos='DragonFly'
+elif check_define __NetBSD__ ; then
+  targetos='NetBSD'
 elif check_define __sun__ ; then
   targetos='SunOS'
 elif check_define __HAIKU__ ; then
-- 
1.9.1




Re: [Qemu-devel] [for-2.3 PATCH v2] gitignore: Ignore more .pod files.

2015-03-20 Thread Stefan Weil

Am 20.03.2015 um 17:30 schrieb Eric Blake:

kvm_stat.{1,pod} started showing up as untracked files in my
directory, and I nearly accidentally merged them into a commit
with my usual habit of 'git add .'.  Rather than spelling out
each such file, just ignore the entire pattern.

Signed-off-by: Eric Blake 
CC: qemu-triv...@nongnu.org
---

v2: fix man page pattern

  .gitignore | 10 ++
  1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/.gitignore b/.gitignore
index e32a584..aed0e1f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -37,14 +37,8 @@
  /qemu-tech.html
  /qemu-doc.info
  /qemu-tech.info
-/qemu.1
-/qemu.pod
-/qemu-img.1
-/qemu-img.pod
  /qemu-img
  /qemu-nbd
-/qemu-nbd.8
-/qemu-nbd.pod
  /qemu-options.def
  /qemu-options.texi
  /qemu-img-cmds.texi
@@ -56,8 +50,7 @@
  /qmp-commands.txt
  /vscclient
  /fsdev/virtfs-proxy-helper
-/fsdev/virtfs-proxy-helper.1
-/fsdev/virtfs-proxy-helper.pod
+*.[1-9]
  *.a
  *.aux
  *.cp
@@ -70,6 +63,7 @@
  *.ky
  *.log
  *.pdf
+*.pod
  *.cps
  *.fns
  *.kys


Reviewed-by: Stefan Weil 

What about only supporting out-of-tree builds for future versions of QEMU?
.gitignore could be reduced to a few lines then.




Re: [Qemu-devel] [PATCH 1/6 v6] target-tilegx: Firstly add TILE-Gx with minimized features

2015-03-20 Thread Richard Henderson
On 03/18/2015 07:04 PM, Chen Gang wrote:
> For me, I am not quite sure about it, the related functional description
> is:
> 
>   rf[Dest] = signExtend32 ((int32_t) rf[SrcA] * (int32_t) rf[SrcB]);
> 
> Do you mean it is equal to:
> 
>   rf[Dest] = signExtend32 (rf[SrcA] * rf[SrcB]);

Yes, of course.

Think about how multiplication works.  Bit N of the input cannot affect any
output bit in [N-1,0].  Thus the first sign-extend cannot affect the output.


r~



[Qemu-devel] [PATCH] acpi: Add missing GCC_FMT_ATTR to local function

2015-03-20 Thread Stefan Weil
This fixes these gcc warnings (not enabled in default build):

hw/acpi/aml-build.c:83:5: warning:
 function might be possible candidate for ‘gnu_printf’ format attribute 
[-Wsuggest-attribute=format]
hw/acpi/aml-build.c:88:5: warning:
 function might be possible candidate for ‘gnu_printf’ format attribute 
[-Wsuggest-attribute=format]

Cc: Michael S. Tsirkin 
Signed-off-by: Stefan Weil 
---
 hw/acpi/aml-build.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 6242908..d7945f6 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -68,7 +68,7 @@ build_append_nameseg(GArray *array, const char *seg)
 g_array_append_vals(array, "", ACPI_NAMESEG_LEN - len);
 }
 
-static void
+static void GCC_FMT_ATTR(2, 0)
 build_append_namestringv(GArray *array, const char *format, va_list ap)
 {
 /* It would be nicer to use g_string_vprintf but it's only there in 2.22 */
-- 
2.1.4




Re: [Qemu-devel] [PATCH for-2.3 0/4] ahci: fix big endian PIO failures

2015-03-20 Thread Andreas Färber
Am 20.03.2015 um 01:24 schrieb John Snow:
> Two issues were unearthed from ahci-test on ppc64:
> 
> (1) The ahci_populate_sglist function had endian issues,
> which is only likely to impact PIO transfers for buffers
> greater than one sector, and
> 
> (2) multiple-sector PIO which I attempted to repair in
> 36334faf has been broken for years. ahci-test didn't catch
> this because it used a pattern that was identical for each
> sector.
> 
> So the pattern has been corrected and the underlying issue
> fixed.
> 
> This should clear up the test failures (properly) for ppc64.
> 
> John Snow (4):
>   ide: fix cmd_write_pio when nsectors > 1
>   ide: fix cmd_read_pio when nsectors > 1
>   ahci: Fix sglist offset manipulation for BE machines
>   ahci-test: improve rw buffer patterns

Confirming that it resolves the observed failure,

Tested-by: Andreas Färber 

Thanks for the quick fix,

Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)



Re: [Qemu-devel] GSoC Proposal: ARM Virtualization Extensions

2015-03-20 Thread Peter Maydell
On 20 March 2015 at 17:25, Merten Sach  wrote:
> I'm interested in participating in this years edition of Google Summer of 
> Code.

> Initially I wanted to propose the implementation of ARMv7
> virtualization extensions. I know this is not supported at the moment.
> Also based on the documentation I found I thought it is still untouched.
> However, when I looked at git log I saw that there is some preparation
> to include EL2 support (LPAE, nested paging preparation, etc).

Yes, we have put in some initial foundational changes but
actual support is still missing. Edgar, can you remind me
whether you have some pending patches for EL2 support still?

> Is there demand for a GSoC project to further ARM virtualization
> extension support?

We would certainly like to see virtualization emulation
supported in upstream QEMU. I think my main areas of
concern would be:
 * working with any existing code that Edgar has up his sleeve
 * having a project plan that divides the work up into small
   but coherent chunks that can be landed upstream incrementally
   (to avoid the failure mode of "work all summer but don't
   get it finished, and so don't have anything concrete
   to show for it at the end")

thanks
-- PMM



Re: [Qemu-devel] [PATCH 1/6 v7] target-tilegx: Firstly add TILE-Gx with minimized features

2015-03-20 Thread Richard Henderson
On 03/20/2015 08:25 AM, Chen Gang wrote:
> +/*
> + * The related functional description for bfextu in isa document:
> + *
> + * uint64_t mask = 0;
> + * mask = (-1ULL) ^ ((-1ULL << ((BFEnd - BFStart) & 63)) << 1);
> + * uint64_t rot_src = (((uint64_t) rf[SrcA]) >> BFStart)
> + *| (rf[SrcA] << (64 - BFStart));
> + * rf[Dest] = rot_src & mask;
> + */
> +static void gen_bfextu(struct DisasContext *dc, uint8_t rdst, uint8_t rsrc,
> +   int8_t start, int8_t end)
> +{
> +uint64_t mask = (-1ULL) ^ ((-1ULL << ((end - start) & 63)) << 1);
> +TCGv tmp = dest_gr(dc, rdst);
> +
> +qemu_log_mask(CPU_LOG_TB_IN_ASM, "bfextu r%d, r%d, %d, %d\n",
> +  rdst, rsrc, start, end);
> +
> +tcg_gen_rotli_i64(tmp, load_gr(dc, rsrc), start);

Wrong direction rotate: rotri.

> +static void decode_rrr_1_opcode_y0(struct DisasContext *dc,
> +   tilegx_bundle_bits bundle)
> +{
> +switch (get_RRROpcodeExtension_Y0(bundle)) {
> +case UNARY_RRR_1_OPCODE_Y0:
> +switch (get_UnaryOpcodeExtension_Y0(bundle)) {
> +case NOP_UNARY_OPCODE_Y0:
> +case  FNOP_UNARY_OPCODE_Y0:
> +if (!get_SrcA_Y0(bundle) && !get_Dest_Y0(bundle)) {
> +gen_fnop();
> +return;
> +}
> +break;
> +case CNTLZ_UNARY_OPCODE_Y0:
> +case CNTTZ_UNARY_OPCODE_Y0:
> +case FSINGLE_PACK1_UNARY_OPCODE_Y0:
> +case PCNT_UNARY_OPCODE_Y0:
> +case REVBITS_UNARY_OPCODE_Y0:
> +case REVBYTES_UNARY_OPCODE_Y0:
> +case TBLIDXB0_UNARY_OPCODE_Y0:
> +case TBLIDXB1_UNARY_OPCODE_Y0:
> +case TBLIDXB2_UNARY_OPCODE_Y0:
> +case TBLIDXB3_UNARY_OPCODE_Y0:
> +default:
> +break;
> +}
> +break;
> +case SHL1ADD_RRR_1_OPCODE_Y0:
> +case SHL2ADD_RRR_1_OPCODE_Y0:
> +case SHL3ADD_RRR_1_OPCODE_Y0:
> +case RRR_1_OPCODE_Y0:

RRR_1_OPCODE_Y0 doesn't belong.  It's the main opcode that brought us here.

> +default:
> +break;
> +}
> +
> +qemu_log_mask(LOG_UNIMP, "UNIMP rrr_1_opcode_y0, [" TARGET_FMT_lx "]\n",
> +  (uint64_t)bundle);

Eh, TARGET_FMT_lx is tied to TARGET_LONG_BITS, and you're not using target_long
but uint64_t.  I think it would be better to use "%016" PRIx64 directly, or
create your own macro for this file.


r~



Re: [Qemu-devel] [PATCH 1/6 v7] target-tilegx: Firstly add TILE-Gx with minimized features

2015-03-20 Thread Peter Maydell
On 20 March 2015 at 15:25, Chen Gang  wrote:
> It is the configure and build system support for TILE-Gx (tilegx will be
> used in configure and real sub-directory name).
>
> At present, it is linux-user only, and can finish the first system call
> (uname) execution in __libc_start_main().
>
> Signed-off-by: Chen Gang 
> ---
>  configure |3 +
>  default-configs/tilegx-linux-user.mak |1 +
>  target-tilegx/Makefile.objs   |1 +
>  target-tilegx/cpu-qom.h   |   73 ++
>  target-tilegx/cpu.c   |  149 +++
>  target-tilegx/cpu.h   |   94 ++
>  target-tilegx/helper.c|   31 +
>  target-tilegx/helper.h|1 +
>  target-tilegx/opcode_tilegx.h | 1406 ++
>  target-tilegx/translate.c | 1764 
> +
>  10 files changed, 3523 insertions(+)

If you have any patch which has a diffstat like this then
you *must* split it up into separate patches. Even
1000 lines is really too long to be easily
reviewable, and this patch is now over three times
that size! Most patches should be 250 lines or
less, really, unless they're very mechanical.

-- PMM



Re: [Qemu-devel] [PATCH V6 for-2.3 00/26] hw/pc: implement multiple primary busses for pc machines

2015-03-20 Thread Paolo Bonzini


On 20/03/2015 09:37, Marcel Apfelbaum wrote:
>> I think this should not be committed to 2.3 after -rc0.
> While I agree that looking at the above the series do not
> fit for 2.3, I still think the risk is not big because:
> - The biggest amount of modifications are in pci_bus/pci_host
>   and *is only*  a movement of code into a new file,
>   no changes at all (This cover almost half of the series changes)
> - The series does not intervene with usual process, meaning if
>   pxb is not used, it will work exactly like before
> - The bios already supports pxb for this release, is a pity to wait
>   for another QEMU version.
> - The series were in mailing list for more than a month before rc,
>   but the real review process started at rc time.
>   (begging the question what a non-maintainer must do to get its code in
> time)
> - The series is mature, rebased on the latest QEMU
>   and was reviewed in depth and tested by the community.

I understand, but you have to draw a line somewhere.  The easiest place
to draw the line is 2.3.0-rc0.

Maintainers can already prepare branches with patches destined to 2.4,
and these 26 can be included there.  As far as you are concerned, if the
code is ready you can stop worrying about it.

The problem isn't non-maintainer vs. maintainer.  There was a dependency
on Igor's series, which took some time to get in---partly because it was
included in a 100-patch pull request, which isn't exactly the preferred
way to send pull request in absence of a serious effort on continuous
integration of maintainer branches.

Paolo



Re: [Qemu-devel] [PATCH v2 5/5] fw_cfg: insert fw_cfg file blobs via qemu cmdline

2015-03-20 Thread Gabriel L. Somlo
On Thu, Mar 19, 2015 at 06:38:53PM +0100, Laszlo Ersek wrote:
> On 03/19/15 01:18, Gabriel L. Somlo wrote:
> > Allow user supplied files to be inserted into the fw_cfg
> > device before starting the guest. Since fw_cfg_add_file()
> > already disallows duplicate fw_cfg file names, qemu will
> > exit with an error message if the user supplies multiple
> > blobs with the same fw_cfg file name, or if a blob name
> > collides with a fw_cfg name programmatically added from
> > within the QEMU source code.
> > 
> > Signed-off-by: Gabriel Somlo 
> > ---
> 
> Of course, I guess, you should wrap your qemu_opts_foreach() loop with a
> check to see if the machine in question actually has fw_cfg. If it
> doesn't, then you might want to skip the loop, or even exit with an error.
> 
> H... that's messy, again. fw_cfg is built into the qemu binary only
> if you have CONFIG_SOFTMMU. I guess something like this should work:
> 
> #ifdef CONFIG_SOFTMMU
> /* okay, fw_cfg_find() is linked into the binary */
> 
> if (fw_cfg_find()) {
> /* okay, the board has set up fw_cfg in its MachineClass init
>  * function
>  */
> 
> if (qemu_opts_foreach(qemu_find_opts("fw_cfg"), parse_fw_cfg,
>   NULL, 1)) {
> exit(1);
> }
> }
> #endif
> 
> You should definitely wait & see what Markus & Gerd think about the above.

Thanks much Gerd, Markus, and Laszlo for the "QemuOpts 101" crash course!

With this change, the command-line options patch no longer has to
touch fw_cfg.c at all, which is pretty cool, IMHO.

Below is "version 2.5" of the command-line fw_cfg patch only. I'll
send out a v3 of the series once we're mostly happy with this one in
particular.

I didn't use #ifdef around fw_cfg_find() -- I think the underlying
object_resolve_path() will return NULL whether fw_cfg support isn't
compiled in or if the fw_cfg device hasn't been initialized, so
that should not be necessary [*].

[*] Oh, wait. Right now, fw_cfg_find() is a function defined inside
fw_cfg.c, so if that won't get compiled, we lose... How about turning
it into a macro or inline function in fw_cfg.h instead ? Something like

static inline FWCfgState *fw_cfg_find(void)
{
return FW_CFG(object_resolve_path(FW_CFG_PATH, NULL));
}

should work nicely -- any thoughts ?

The only remaining issue in my mind is what to do about the issue
Laszlo raised, of whether we should force user-provided fw_cfg files
into a separate "namespace" from those inserted programmatically.

Thanks much,
  Gabriel


diff --git a/qemu-options.hx b/qemu-options.hx
index 319d971..138b9cd 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2668,6 +2668,17 @@ STEXI
 @table @option
 ETEXI
 
+DEF("fw_cfg", HAS_ARG, QEMU_OPTION_fwcfg,
+"-fw_cfg name=,file=\n"
+"add named fw_cfg entry from file\n",
+QEMU_ARCH_ALL)
+STEXI
+@item -fw_cfg name=@var{name},file=@var{file}
+@findex -fw_cfg
+Add named fw_cfg entry from file. @var{name} determines the name of
+the entry in the fw_cfg file directory exposed to the guest.
+ETEXI
+
 DEF("serial", HAS_ARG, QEMU_OPTION_serial, \
 "-serial dev redirect the serial port to char device 'dev'\n",
 QEMU_ARCH_ALL)
diff --git a/vl.c b/vl.c
index 75ec292..b8e5e4c 100644
--- a/vl.c
+++ b/vl.c
@@ -490,6 +490,25 @@ static QemuOptsList qemu_semihosting_config_opts = {
 },
 };
 
+static QemuOptsList qemu_fw_cfg_opts = {
+.name = "fw_cfg",
+.implied_opt_name = "name",
+.head = QTAILQ_HEAD_INITIALIZER(qemu_fw_cfg_opts.head),
+.desc = {
+{
+.name = "name",
+.type = QEMU_OPT_STRING,
+.help = "Sets the fw_cfg name of the blob to be inserted",
+}, {
+.name = "file",
+.type = QEMU_OPT_STRING,
+.help = "Sets the name of the file from which\n"
+"the fw_cfg blob will be loaded",
+},
+{ /* end of list */ }
+},
+};
+
 /**
  * Get machine options
  *
@@ -2118,6 +2137,34 @@ char *qemu_find_file(int type, const char *name)
 return NULL;
 }
 
+static int parse_fw_cfg(QemuOpts *opts, void *opaque)
+{
+gchar *buf;
+size_t size;
+const char *name, *file;
+
+if (opaque == NULL) {
+return 0;
+}
+
+name = qemu_opt_get(opts, "name");
+file = qemu_opt_get(opts, "file");
+if (name == NULL || *name == '\0' || file == NULL || *file == '\0') {
+error_report("invalid argument value");
+return -1;
+}
+if (strlen(name) > FW_CFG_MAX_FILE_PATH - 1) {
+error_report("name too long (max. %d char)", FW_CFG_MAX_FILE_PATH - 1);
+return -1;
+}
+if (!g_file_get_contents(file, &buf, &size, NULL)) {
+error_report("can't load %s", file);
+return -1;
+}
+fw_cfg_add_file((FWCfgState *)opaque, name, buf, size);
+return 0;
+}
+
 static int device_help_func(QemuOpts *opts, void *opaque)
 {
 return qdev_device_help(opt

  1   2   >