[Qemu-devel] [PATCH v3] char: restore read callback on a reattached (hotplug) chardev

2013-12-11 Thread Gal Hammer
Fix a bug that was introduced in commit 386a5a1e. A removal of a device
set the chr handlers to NULL. However when the device is plugged back,
its read callback is not restored so data can't be transftered from the
host to the guest (e.g. via the virtio-serial port).

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

Signed-off-by: Gal Hammer 

V3: - fix a typo in comment.
- move the revision history after the "signed-off-by" tag.

V2: - do not call chr_update_read_handler on device removal.
- add asserts to verify chr_update_read_handler is not called
  with an assigned fd_in_tag to prevent fd leaks.
- update fd and udp backends' chr_update_read_handler function
  so it won't remove fd_in to prevent a double release.
---
 qemu-char.c | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index e00f84c..69649f0 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -213,7 +213,7 @@ void qemu_chr_add_handlers(CharDriverState *s,
 s->chr_read = fd_read;
 s->chr_event = fd_event;
 s->handler_opaque = opaque;
-if (s->chr_update_read_handler)
+if (fe_open && s->chr_update_read_handler)
 s->chr_update_read_handler(s);
 
 if (!s->explicit_fe_open) {
@@ -870,6 +870,7 @@ static void fd_chr_update_read_handler(CharDriverState *chr)
 {
 FDCharDriver *s = chr->opaque;
 
+assert(!chr->fd_in_tag);
 remove_fd_in_watch(chr);
 if (s->fd_in) {
 chr->fd_in_tag = io_add_watch_poll(s->fd_in, fd_chr_read_poll,
@@ -2228,7 +2229,7 @@ static void udp_chr_update_read_handler(CharDriverState 
*chr)
 {
 NetCharDriver *s = chr->opaque;
 
-remove_fd_in_watch(chr);
+assert(!chr->fd_in_tag);
 if (s->chan) {
 chr->fd_in_tag = io_add_watch_poll(s->chan, udp_chr_read_poll,
udp_chr_read, chr);
@@ -2510,6 +2511,17 @@ static void tcp_chr_connect(void *opaque)
 qemu_chr_be_generic_open(chr);
 }
 
+static void tcp_chr_update_read_handler(CharDriverState *chr)
+{
+TCPCharDriver *s = chr->opaque;
+
+assert(!chr->fd_in_tag);
+if (s->chan && !chr->fd_in_tag) {
+chr->fd_in_tag = io_add_watch_poll(s->chan, tcp_chr_read_poll,
+   tcp_chr_read, chr);
+}
+}
+
 #define IACSET(x,a,b,c) x[0] = a; x[1] = b; x[2] = c;
 static void tcp_chr_telnet_init(int fd)
 {
@@ -2665,6 +2677,7 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, 
bool do_nodelay,
 chr->get_msgfd = tcp_get_msgfd;
 chr->chr_add_client = tcp_chr_add_client;
 chr->chr_add_watch = tcp_chr_add_watch;
+chr->chr_update_read_handler = tcp_chr_update_read_handler;
 /* be isn't opened until we get a connection */
 chr->explicit_be_open = true;
 
-- 
1.8.1.4




Re: [Qemu-devel] [PATCH 01/22] block: Move initialisation of BlockLimits to bdrv_refresh_limits()

2013-12-11 Thread Thomas Huth
On Thu, 12 Dec 2013 10:57:49 +0800
Wenchao Xia  wrote:

> 
> >> +static int bdrv_refresh_limits(BlockDriverState *bs)
> >> +{
> >> +BlockDriver *drv = bs->drv;
> >> +
> >> +memset(&bs->bl, 0, sizeof(bs->bl));
> >> +
> >> +if (!drv) {
> >> +return 0;
> >> +} else if (drv->bdrv_refresh_limits) {
> >> +return drv->bdrv_refresh_limits(bs);
> >> +}
> >> +
> >> +return 0;
> >  It seems this line can be removed.
> > 
>   I missed the "else if", then the patch is OK.
 
But it could also be written in a shorter way:

if (drv && drv->bdrv_refresh_limits) {
return drv->bdrv_refresh_limits(bs);
}

return 0;

 Regards,
  Thomas




Re: [Qemu-devel] [PATCH v3 0/6] Add error_abort and associated cleanups

2013-12-11 Thread Markus Armbruster
Peter Crosthwaite  writes:

> On Wed, Dec 11, 2013 at 11:57 PM, Markus Armbruster  wrote:
>> Peter Crosthwaite  writes:
>>
>>> Following our discussion RE self asserting API calls, here is a spin of
>>> my proposal. This series obsoletes the need for _nofail variants for
>>> Error ** accepting APIs. Is also greatly reduces the verbosity of calls
>>> sites that are currently asserting against errors.
>>>
>>> Patch 1 is the main event - addition of error_abort. The following
>>> patches then cleanup uses of _nofail and assert_no_error().
>>>
>>> To give it a smoke test, I introduce a (critical) bug into QOM:
>> [...]
>>
>> Reviewed-by: Markus Armbruster 
>>
>
> Thanks,
>
> Do you have a suggested patch queue or person this should go via?

Luiz (cc'ed) has taken error work in the past.  Luiz, what about this
series?



Re: [Qemu-devel] [PATCH 07/22] block: rename buffer_alignment to guest_block_size

2013-12-11 Thread Wenchao Xia
Reviewed-by: Wenchao Xia 




Re: [Qemu-devel] [PATCH 06/22] block: Don't use guest sector size for qemu_blockalign()

2013-12-11 Thread Wenchao Xia
Reviewed-by: Wenchao Xia 




Re: [Qemu-devel] [PATCH v3 04/21] qapi: extend qdict_flatten() for QLists

2013-12-11 Thread Fam Zheng

On 2013年12月12日 02:10, Max Reitz wrote:

Reversing qdict_array_split(), qdict_flatten() should flatten QLists as
well by interpreting them as QDicts where every entry's key is its
index.

This allows bringing QDicts with QLists from QMP commands to the same
form as they would be given as command-line options, thereby allowing
them to be parsed the same way.

Signed-off-by: Max Reitz 
---
  qobject/qdict.c | 45 +
  1 file changed, 41 insertions(+), 4 deletions(-)

diff --git a/qobject/qdict.c b/qobject/qdict.c
index fca1902..d7ce4b3 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -477,7 +477,40 @@ static void qdict_destroy_obj(QObject *obj)
  g_free(qdict);
  }

-static void qdict_do_flatten(QDict *qdict, QDict *target, const char *prefix)
+static void qdict_flatten_qdict(QDict *qdict, QDict *target,
+const char *prefix);
+
+static void qdict_flatten_qlist(QList *qlist, QDict *target, const char 
*prefix)
+{
+QObject *value;
+const QListEntry *entry;
+char *new_key;
+int i;
+
+/* This function is never called with prefix == NULL, i.e., it is always
+ * called from within qdict_flatten_q(list|dict)(). Therefore, it does not
+ * need to remove list entries during the iteration (the whole list will be
+ * deleted eventually anyway from qdict_flatten_qdict()). Also, prefix can
+ * never be NULL. */
+
+entry = qlist_first(qlist);
+
+for (i = 0; entry; entry = qlist_next(entry), i++) {
+value = qlist_entry_obj(entry);
+
+qobject_incref(value);
+new_key = g_strdup_printf("%s.%i", prefix, i);
+qdict_put_obj(target, new_key, value);
+
+if (qobject_type(value) == QTYPE_QDICT) {
+qdict_flatten_qdict(qobject_to_qdict(value), target, new_key);
+} else {
+qdict_flatten_qlist(qobject_to_qlist(value), target, new_key);
+}
+}
+}
+
+static void qdict_flatten_qdict(QDict *qdict, QDict *target, const char 
*prefix)


Sorry for replying twice on the same patch. But it would be nice if you 
could add a few comments on this function with some examples, as you did 
with qdict_array_split, I appreciated that. :)


Fam




Re: [Qemu-devel] [PATCH 01/22] block: Move initialisation of BlockLimits to bdrv_refresh_limits()

2013-12-11 Thread Peter Lieven
Am 11.12.2013 22:08, schrieb Kevin Wolf:
> This function separates filling the BlockLimits from bdrv_open(), which
> allows it to call it from other operations which may change the limits
> (e.g. modifications to the backing file chain or bdrv_reopen)
>
> Signed-off-by: Kevin Wolf 
> ---
>  block.c   | 21 
>  block/iscsi.c | 87 
> ---
>  block/qcow2.c | 11 +-
>  block/qed.c   | 11 +-
>  block/vmdk.c  | 22 +---
>  include/block/block_int.h |  2 ++
>  6 files changed, 113 insertions(+), 41 deletions(-)
>
> diff --git a/block.c b/block.c
> index 13f001a..c32d856 100644
> --- a/block.c
> +++ b/block.c
> @@ -479,6 +479,21 @@ int bdrv_create_file(const char* filename, 
> QEMUOptionParameter *options,
>  return ret;
>  }
>  
> +static int bdrv_refresh_limits(BlockDriverState *bs)
> +{
> +BlockDriver *drv = bs->drv;
> +
> +memset(&bs->bl, 0, sizeof(bs->bl));
> +
> +if (!drv) {
> +return 0;
> +} else if (drv->bdrv_refresh_limits) {
> +return drv->bdrv_refresh_limits(bs);
> +}
> +
> +return 0;
> +}
> +
>  /*
>   * Create a uniquely-named empty temporary file.
>   * Return 0 upon success, otherwise a negative errno value.
> @@ -833,6 +848,8 @@ static int bdrv_open_common(BlockDriverState *bs, 
> BlockDriverState *file,
>  goto free_and_fail;
>  }
>  
> +bdrv_refresh_limits(bs);
> +
>  #ifndef _WIN32
>  if (bs->is_temporary) {
>  assert(bs->filename[0] != '\0');
> @@ -1018,6 +1035,10 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict 
> *options, Error **errp)
>  }
>  pstrcpy(bs->backing_file, sizeof(bs->backing_file),
>  bs->backing_hd->file->filename);
> +
> +/* Recalculate the BlockLimits with the backing file */
> +bdrv_refresh_limits(bs);
> +
>  return 0;
>  }
>  
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 829d444..f3ded8c 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1425,6 +1425,56 @@ static int iscsi_open(BlockDriverState *bs, QDict 
> *options, int flags,
>  task = NULL;
>  }
>  
> +#if defined(LIBISCSI_FEATURE_NOP_COUNTER)
> +/* Set up a timer for sending out iSCSI NOPs */
> +iscsilun->nop_timer = timer_new_ms(QEMU_CLOCK_REALTIME, 
> iscsi_nop_timed_event, iscsilun);
> +timer_mod(iscsilun->nop_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + 
> NOP_INTERVAL);
> +#endif
> +
> +out:
> +qemu_opts_del(opts);
> +if (initiator_name != NULL) {
> +g_free(initiator_name);
> +}
> +if (iscsi_url != NULL) {
> +iscsi_destroy_url(iscsi_url);
> +}
> +if (task != NULL) {
> +scsi_free_scsi_task(task);
> +}
> +
> +if (ret) {
> +if (iscsi != NULL) {
> +iscsi_destroy_context(iscsi);
> +}
> +memset(iscsilun, 0, sizeof(IscsiLun));
> +}
> +return ret;
> +}
> +
> +static void iscsi_close(BlockDriverState *bs)
> +{
> +IscsiLun *iscsilun = bs->opaque;
> +struct iscsi_context *iscsi = iscsilun->iscsi;
> +
> +if (iscsilun->nop_timer) {
> +timer_del(iscsilun->nop_timer);
> +timer_free(iscsilun->nop_timer);
> +}
> +qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), NULL, NULL, NULL);
> +iscsi_destroy_context(iscsi);
> +g_free(iscsilun->zeroblock);
> +memset(iscsilun, 0, sizeof(IscsiLun));
> +}
> +
> +static int iscsi_refresh_limits(BlockDriverState *bs)
> +{
> +IscsiLun *iscsilun = bs->opaque;
> +struct scsi_task *task = NULL;
> +int ret;
> +
> +memset(&bs->bl, 0, sizeof(bs->bl));
> +
you do that memset in bdrv_refresh_limits already.
>  if (iscsilun->lbp.lbpu || iscsilun->lbp.lbpws) {
>  struct scsi_inquiry_block_limits *inq_bl;
>  task = iscsi_do_inquiry(iscsilun->iscsi, iscsilun->lun, 1,
> @@ -1462,48 +1512,14 @@ static int iscsi_open(BlockDriverState *bs, QDict 
> *options, int flags,

iscsi_do_inquiry does a sync call to request the pages from the target.
you can only do that here if you can guarantee that iscsi_refresh_limits will
be called with no pending requests in-flight otherwise this will end in a mess.

otherwise i would propose to leave the inquiry call ins iscsi_open and just fill
the bs->bl struc tin iscsi_refresh_limits. the problem is if we make this async
we have to handle all possible I/O callbacks whiile in iscsi_refresh_limits.
i do not see how an iscsi target can change its limits while a target is 
mounted.

Peter

>   iscsilun);
>  }
>  
> -#if defined(LIBISCSI_FEATURE_NOP_COUNTER)
> -/* Set up a timer for sending out iSCSI NOPs */
> -iscsilun->nop_timer = timer_new_ms(QEMU_CLOCK_REALTIME, 
> iscsi_nop_timed_event, iscsilun);
> -timer_mod(iscsilun->nop_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + 
> NOP_INTERVAL);
> -#endif
> -
> +ret = 0;
>  out:
> -qemu_opts_del(opts);
> - 

Re: [Qemu-devel] [PATCH v3 04/21] qapi: extend qdict_flatten() for QLists

2013-12-11 Thread Fam Zheng

On 2013年12月12日 02:10, Max Reitz wrote:

Reversing qdict_array_split(), qdict_flatten() should flatten QLists as
well by interpreting them as QDicts where every entry's key is its
index.

This allows bringing QDicts with QLists from QMP commands to the same
form as they would be given as command-line options, thereby allowing
them to be parsed the same way.

Signed-off-by: Max Reitz 
---


The logic looks fine, just a few questions about assertions below.


  qobject/qdict.c | 45 +
  1 file changed, 41 insertions(+), 4 deletions(-)

diff --git a/qobject/qdict.c b/qobject/qdict.c
index fca1902..d7ce4b3 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -477,7 +477,40 @@ static void qdict_destroy_obj(QObject *obj)
  g_free(qdict);
  }

-static void qdict_do_flatten(QDict *qdict, QDict *target, const char *prefix)
+static void qdict_flatten_qdict(QDict *qdict, QDict *target,
+const char *prefix);
+
+static void qdict_flatten_qlist(QList *qlist, QDict *target, const char 
*prefix)
+{
+QObject *value;
+const QListEntry *entry;
+char *new_key;
+int i;
+
+/* This function is never called with prefix == NULL, i.e., it is always
+ * called from within qdict_flatten_q(list|dict)(). Therefore, it does not
+ * need to remove list entries during the iteration (the whole list will be
+ * deleted eventually anyway from qdict_flatten_qdict()). Also, prefix can
+ * never be NULL. */


Then maybe assert this?


+
+entry = qlist_first(qlist);
+
+for (i = 0; entry; entry = qlist_next(entry), i++) {
+value = qlist_entry_obj(entry);
+
+qobject_incref(value);
+new_key = g_strdup_printf("%s.%i", prefix, i);
+qdict_put_obj(target, new_key, value);
+
+if (qobject_type(value) == QTYPE_QDICT) {
+qdict_flatten_qdict(qobject_to_qdict(value), target, new_key);
+} else {


And maybe assert the type are not something other than dict and list?



+qdict_flatten_qlist(qobject_to_qlist(value), target, new_key);
+}
+}
+}
+
+static void qdict_flatten_qdict(QDict *qdict, QDict *target, const char 
*prefix)
  {
  QObject *value;
  const QDictEntry *entry, *next;
@@ -500,8 +533,12 @@ static void qdict_do_flatten(QDict *qdict, QDict *target, 
const char *prefix)
  if (qobject_type(value) == QTYPE_QDICT) {
  /* Entries of QDicts are processed recursively, the QDict object
   * itself disappears. */
-qdict_do_flatten(qobject_to_qdict(value), target,
- new_key ? new_key : entry->key);
+qdict_flatten_qdict(qobject_to_qdict(value), target,
+new_key ? new_key : entry->key);
+delete = true;
+} else if (qobject_type(value) == QTYPE_QLIST) {
+qdict_flatten_qlist(qobject_to_qlist(value), target,
+new_key ? new_key : entry->key);
  delete = true;
  } else if (prefix) {
  /* All other objects are moved to the target unchanged. */
@@ -531,7 +568,7 @@ static void qdict_do_flatten(QDict *qdict, QDict *target, 
const char *prefix)
   */
  void qdict_flatten(QDict *qdict)
  {
-qdict_do_flatten(qdict, qdict, NULL);
+qdict_flatten_qdict(qdict, qdict, NULL);


If there was an assumption that the top level is a qdict, also assert 
that here?


Thanks,

Fam




Re: [Qemu-devel] [PATCH 00/22] block: Support for 512b-on-4k emulation

2013-12-11 Thread Peter Lieven
Should it be possible to boot from a 4K-native drive with this series?
If yes, I will run some test with some new iSCSI arrays we got for testing
they can export 4k blocksize LUNs.

Anyway, can you please include the patch from Paolo which sets
the bs->request_alignment = iscsilun->block_size in iscsi_open.
It should be a one-liner.

Peter

Am 11.12.2013 22:08, schrieb Kevin Wolf:
> This patch series adds code to the block layer that allows performing
> I/O requests in smaller granularities than required by the host backend
> (most importantly, O_DIRECT restrictions). It achieves this for reads
> by rounding the request to host-side block boundary, and for writes by
> performing a read-modify-write cycle (and serialising requests
> touching the same block so that the RMW doesn't write back stale data).
>
> Originally I intended to reuse a lot of code from Paolo's previous
> patch series, however as I tried to integrate pread/pwrite, which
> already do a very similar thing (except for considering concurrency),
> and because I wanted to implement zero-copy, most of this series ended
> up being new code.
>
> Zero-copy is possible in a common case because while XFS defauls to a
> 4k sector size and therefore 4k on-disk O_DIRECT alignment for 512E
> disks, it still only has a 512 byte memory alignment requirement.
> (Unfortunately the XFS_IOC_DIOINFO ioctl claims 4k even for memory, but
> we know that the value is wrong and can probe it.)
>
>
> Changes since RFC:
> - Moved opt_mem_alignment into BlockLimits [Paolo]
>
> - Changed BlockLimits in turn to work a bit more like the
>   .bdrv_opt_mem_align() callback of the RFC; allows updating the
>   BlockLimits later when the chain changes or bdrv_reopen() toggles
>   O_DIRECT
>
> - Fixed a typo in a commit message [Eric]
>
>
> Kevin Wolf (20):
>   block: Move initialisation of BlockLimits to bdrv_refresh_limits()
>   block: Inherit opt_transfer_length
>   block: Update BlockLimits when they might have changed
>   qemu_memalign: Allow small alignments
>   block: Detect unaligned length in bdrv_qiov_is_aligned()
>   block: Don't use guest sector size for qemu_blockalign()
>   block: Introduce bdrv_aligned_preadv()
>   block: Introduce bdrv_co_do_preadv()
>   block: Introduce bdrv_aligned_pwritev()
>   block: write: Handle COR dependency after I/O throttling
>   block: Introduce bdrv_co_do_pwritev()
>   block: Switch BdrvTrackedRequest to byte granularity
>   block: Allow waiting for overlapping requests between begin/end
>   block: Make zero-after-EOF work with larger alignment
>   block: Generalise and optimise COR serialisation
>   block: Make overlap range for serialisation dynamic
>   block: Align requests in bdrv_co_do_pwritev()
>   block: Change coroutine wrapper to byte granularity
>   block: Make bdrv_pread() a bdrv_prwv_co() wrapper
>   block: Make bdrv_pwrite() a bdrv_prwv_co() wrapper
>
> Paolo Bonzini (2):
>   block: rename buffer_alignment to guest_block_size
>   raw: Probe required direct I/O alignment
>
>  block.c   | 602 
> +++---
>  block/backup.c|   7 +-
>  block/iscsi.c |  87 ---
>  block/qcow2.c |  11 +-
>  block/qed.c   |  11 +-
>  block/raw-posix.c | 102 ++--
>  block/raw-win32.c |  41 
>  block/stream.c|   2 +
>  block/vmdk.c  |  22 +-
>  hw/block/virtio-blk.c |   2 +-
>  hw/ide/core.c |   2 +-
>  hw/scsi/scsi-disk.c   |   2 +-
>  hw/scsi/scsi-generic.c|   2 +-
>  include/block/block.h |   6 +-
>  include/block/block_int.h |  25 +-
>  util/oslib-posix.c|   5 +
>  16 files changed, 666 insertions(+), 263 deletions(-)
>




Re: [Qemu-devel] [PATCH v2 0/4] X86/KVM: enable Intel MPX for KVM

2013-12-11 Thread Liu, Jinsong
Paolo Bonzini wrote:
> Il 11/12/2013 09:31, Liu, Jinsong ha scritto:
>> Paolo, comments for version 2?
> 
> I think I commented that it's fine, I'm just waiting for a rebase on
> top of the generic patches.
> 
> Paolo
> 

Thanks! common MPX definiation patches have been checked in tip tree (both 
Qiaowei and I use that definiations):
http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?id=191f57c137bcce0e3e9313acb77b2f114d15afbb
http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?id=e7d820a5e549b3eb6c3f9467507566565646a669

Jinsong

>> 
>> Liu, Jinsong wrote:
>>> These patches are version 2 to enalbe Intel MPX for KVM.
>>> 
>>> Version 1:
>>>   * Add some Intel MPX definiation
>>>   * Fix a cpuid(0x0d, 0) exposing bug, dynamic per XCR0 features
>>> enable/disable 
>>>   * vmx and msr handle for MPX support at KVM
>>>   * enalbe MPX feature for guest
>>> 
>>> Version 2:
>>>   * remove generic MPX definiation, kernel side has add the
>>> definiation 
>>>   * add MSR_IA32_BNDCFGS to msrs_to_save
>>> 
>>> Thanks,
>>> Jinsong
>>> 
>>> Liu Jinsong (4):
>>>   KVM/X86: Fix xsave cpuid exposing bug
>>>   KVM/X86: Intel MPX vmx and msr handle
>>>   KVM/X86: add MSR_IA32_BNDCFGS to msrs_to_save
>>>   KVM/X86: Enable Intel MPX for guest.
>>> 
>>>  arch/x86/include/asm/vmx.h|4 
>>>  arch/x86/include/asm/xsave.h  |2 ++
>>>  arch/x86/include/uapi/asm/msr-index.h |1 +
>>>  arch/x86/kvm/cpuid.c  |8 
>>>  arch/x86/kvm/vmx.c|   18 --
>>>  arch/x86/kvm/x86.c|   12 +---
>>>  arch/x86/kvm/x86.h|3 ++-
>>>  7 files changed, 38 insertions(+), 10 deletions(-)




Re: [Qemu-devel] save compiled qemu traces.

2013-12-11 Thread Xin Tong
On Thu, Dec 12, 2013 at 1:07 PM, Xin Tong  wrote:
> see questions below.
>
> On Tue, Dec 10, 2013 at 12:25 AM, Alex Bennée  wrote:
>>
>> trent.t...@gmail.com writes:
>>
>>> Does anyone have profiles on how much time QEMU spends in translating
>>> instructions. QEMU does not have a baseline interpreter nor does it
>>> translate on trace-granularity.  so i imagine QEMU must spend quite a bit
>>> of time translating instructions.
>>
>> Not as much as you'd think. The translation stage isn't very complex and
>> blocks only get translated once (modulo exceptions and self modifying
>> code). If you run perf on your task you should see most of the time is
>> spent in the generated code - if not please send the test case to the
>> list.
>
> I took a profile running speccpu2006 403.gcc with test input on a
> intel xeon machine. we only spent 44.76% of the time in the code cache
> (i.e. 13M ticks in the code cache), while 40.97% of the time is spent
> in the qemu-system-x86_64. some of the hot functions in
> qemu-system-x86_64 are listed below.
>
> *you are right* we do not spend much time in translation routines.
> instead we spend significant amount of time in address translation
> code.
>
> CPU_CLK_UNHALTED % Symbol/Functions
> 1340512 100.00 anon (tgid:7106 range:0x7f97815ca000-0x7f979a692000)
>
>
> CPU_CLK_UNHALTED % Symbol/Functions
> 314655   25.64 address_space_translate_internal
> 308942   25.18 cpu_x86_exec
> 128922   10.51 ldq_phys
> 92345   7.53 cpu_x86_handle_mmu_fault
> 62456   5.09 tlb_set_page
> 49332   4.02 memory_region_is_ram
> 31055   2.53 helper_le_ldq_mmu
> 22048   1.80 memory_region_get_ram_addr
> 19223   1.57 memory_region_section_get_iotlb
> 15873   1.29 tcg_optimize
> 14526   1.18 get_page_addr_code
> 12601   1.03 memory_region_get_ram_ptr

However, being able to reuse cached blocks based on content in QEMU
maybe a step towards reusing translated blocks across multiple
invocations of QEMU.
>
> Xin
>
>
>>
>> I suspect the more useful statistic would be getting a break down of the
>> translation blocks and seeing which ones are the most heavily used and
>> examining if QEMU has done as good a job as it can of translating them.
>>
>>> Is it possible for QEMU to obviate some of the translations by attaching a
>>> signature (e.g. a hash) with every translated basic block and try to reuse
>>> translated basic block based on the signature as much as possible ? Reuses
>>> can be a result of rerunning programs or same libraries statically linked
>>> to programs.
>>
>> Your right a translation cache *could* save some translation time,
>> especially if you end up translating the same program over and over
>> again. Having said that you might find the cost of computing the
>> checksum obviates any speed-up from skipping the translation. After all
>> QEMU only needs to look at each subject instruction once normally.
>>
>> Using QEMU  linux-user for cross building would be the obvious pain
>> point. However as the usual use case is building for embedded platforms
>> most users are just happy to fully utilise their 80-core build machines
>> in preference to having a farm of slow embedded processors.
>>
>>> This could end up saving some translation time.
>>
>> I think you would need to do some performance analysis and come up with
>> some numbers before you made that assumption.
>>
>> Cheers,
>>
>> --
>> Alex Bennée
>> QEMU/KVM Hacker for Linaro
>>



Re: [Qemu-devel] [PATCH V4 5/9] qapi script: use same function to generate enum string

2013-12-11 Thread Eric Blake
On 12/10/2013 10:48 PM, Wenchao Xia wrote:
> Prior to this patch, qapi-visit.py used custom code to generate enum
> names used for handling a qapi union. Fix it to instead reuse common
> code, with identical generated results, and allowing future updates to
> generation to only need to touch one place.
> 
> Signed-off-by: Wenchao Xia 
> ---
>  scripts/qapi-types.py |6 +++---
>  scripts/qapi-visit.py |   21 +++--
>  scripts/qapi.py   |   15 +++
>  3 files changed, 29 insertions(+), 13 deletions(-)

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] save compiled qemu traces.

2013-12-11 Thread Xin Tong
see questions below.

On Tue, Dec 10, 2013 at 12:25 AM, Alex Bennée  wrote:
>
> trent.t...@gmail.com writes:
>
>> Does anyone have profiles on how much time QEMU spends in translating
>> instructions. QEMU does not have a baseline interpreter nor does it
>> translate on trace-granularity.  so i imagine QEMU must spend quite a bit
>> of time translating instructions.
>
> Not as much as you'd think. The translation stage isn't very complex and
> blocks only get translated once (modulo exceptions and self modifying
> code). If you run perf on your task you should see most of the time is
> spent in the generated code - if not please send the test case to the
> list.

I took a profile running speccpu2006 403.gcc with test input on a
intel xeon machine. we only spent 44.76% of the time in the code cache
(i.e. 13M ticks in the code cache), while 40.97% of the time is spent
in the qemu-system-x86_64. some of the hot functions in
qemu-system-x86_64 are listed below.

*you are right* we do not spend much time in translation routines.
instead we spend significant amount of time in address translation
code.

CPU_CLK_UNHALTED % Symbol/Functions
1340512 100.00 anon (tgid:7106 range:0x7f97815ca000-0x7f979a692000)


CPU_CLK_UNHALTED % Symbol/Functions
314655   25.64 address_space_translate_internal
308942   25.18 cpu_x86_exec
128922   10.51 ldq_phys
92345   7.53 cpu_x86_handle_mmu_fault
62456   5.09 tlb_set_page
49332   4.02 memory_region_is_ram
31055   2.53 helper_le_ldq_mmu
22048   1.80 memory_region_get_ram_addr
19223   1.57 memory_region_section_get_iotlb
15873   1.29 tcg_optimize
14526   1.18 get_page_addr_code
12601   1.03 memory_region_get_ram_ptr

Xin


>
> I suspect the more useful statistic would be getting a break down of the
> translation blocks and seeing which ones are the most heavily used and
> examining if QEMU has done as good a job as it can of translating them.
>
>> Is it possible for QEMU to obviate some of the translations by attaching a
>> signature (e.g. a hash) with every translated basic block and try to reuse
>> translated basic block based on the signature as much as possible ? Reuses
>> can be a result of rerunning programs or same libraries statically linked
>> to programs.
>
> Your right a translation cache *could* save some translation time,
> especially if you end up translating the same program over and over
> again. Having said that you might find the cost of computing the
> checksum obviates any speed-up from skipping the translation. After all
> QEMU only needs to look at each subject instruction once normally.
>
> Using QEMU  linux-user for cross building would be the obvious pain
> point. However as the usual use case is building for embedded platforms
> most users are just happy to fully utilise their 80-core build machines
> in preference to having a farm of slow embedded processors.
>
>> This could end up saving some translation time.
>
> I think you would need to do some performance analysis and come up with
> some numbers before you made that assumption.
>
> Cheers,
>
> --
> Alex Bennée
> QEMU/KVM Hacker for Linaro
>



Re: [Qemu-devel] [PATCH v6 00/10] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD

2013-12-11 Thread Ian Main
On Wed, Dec 11, 2013 at 10:46:49AM +0800, Fam Zheng wrote:
> On 2013年11月28日 16:39, Fam Zheng wrote:
> >This series adds for point-in-time snapshot NBD exporting based on
> >blockdev-backup (variant of drive-backup with existing device as target).
> >
> >We get a thin point-in-time snapshot by COW mechanism of drive-backup, and
> >export it through built in NBD server. The steps are as below:
> >
> >  1. (SHELL) qemu-img create -f qcow2 BACKUP.qcow2 
> >
> > (Alternatively we can use -o backing_file=RUNNING-VM.img to omit 
> > explicitly
> > providing the size by ourselves, but it's risky because 
> > RUNNING-VM.qcow2 is
> > used r/w by guest. Whether or not setting backing file in the image file
> > doesn't matter, as we are going to override the backing hd in the next
> > step)
> >
> >  2. (QMP) blockdev-add backing=source-drive file.driver=file 
> > file.filename=BACKUP.qcow2 id=target0 if=none driver=qcow2
> >
> > (where ide0-hd0 is the running BlockDriverState name for
> > RUNNING-VM.img. This patch implements "backing=" option to override
> > backing_hd for added drive)
> >
> >  3. (QMP) blockdev-backup device=source-drive sync=none target=target0
> >
> > (this is the QMP command introduced by this series, which use a named
> > device as target of drive-backup)
> >
> >  4. (QMP) nbd-server-add device=target0
> >
> >When image fleecing done:
> >
> >  1. (QMP) block-job-complete device=ide0-hd0
> >
> >  2. (HMP) drive_del target0
> >
> >  3. (SHELL) rm BACKUP.qcow2
> >
> >v6: Address Paolo's comments, (except for bitmask):
> > - Add blocker for all backing_hd references, a relatively big change, 
> > some
> >   patches are reordered.
> > - Introduce a few other necessary patches.
> > - Move two snapshot checks into bdrv_snapshot_*.
> >
> > The interface is unchanged.
> >
> 
> Hi,
> 
> Based on the size of change, this series needs some more review
> before merging. And I'd like to know if there is any concern or
> objection with op_blocker introduced here. I would like to base my
> next series (incremental backup with dirty bitmap) on it.
> 
> Any more reviews/comments?

I really like it.  Just did some more testing and this seems to have
solved the issues from before.. I suspect this will resolve some issues
with other commands and will be a useful framework for the future.

I love this:

(qemu) drive_del target0
block device is in use by block job: backup
(qemu) drive_del target1
block device is in use by block job: backup
(qemu) drive_del ide0-hd0
device is used as backing hd of 'target0'

$ python qmp --path=/home/imain/qmp-sock eject --device=target0

Exception: block device is in use by block job: backup

I've started working on a backup/snapshot API for libvirt already.

Ian




Re: [Qemu-devel] [PATCH 05/22] block: Detect unaligned length in bdrv_qiov_is_aligned()

2013-12-11 Thread Wenchao Xia
Reviewed-by: Wenchao Xia 




Re: [Qemu-devel] [PATCH 04/22] qemu_memalign: Allow small alignments

2013-12-11 Thread Wenchao Xia
Reviewed-by: Wenchao Xia 




Re: [Qemu-devel] [PATCH 03/22] block: Update BlockLimits when they might have changed

2013-12-11 Thread Wenchao Xia
Reviewed-by: Wenchao Xia 




Re: [Qemu-devel] [PATCH 02/22] block: Inherit opt_transfer_length

2013-12-11 Thread Wenchao Xia
Reviewed-by: Wenchao Xia 




Re: [Qemu-devel] [PATCH 01/22] block: Move initialisation of BlockLimits to bdrv_refresh_limits()

2013-12-11 Thread Wenchao Xia

>> +static int bdrv_refresh_limits(BlockDriverState *bs)
>> +{
>> +BlockDriver *drv = bs->drv;
>> +
>> +memset(&bs->bl, 0, sizeof(bs->bl));
>> +
>> +if (!drv) {
>> +return 0;
>> +} else if (drv->bdrv_refresh_limits) {
>> +return drv->bdrv_refresh_limits(bs);
>> +}
>> +
>> +return 0;
>  It seems this line can be removed.
> 
  I missed the "else if", then the patch is OK.




Re: [Qemu-devel] [PATCH RFC 2/2] rng-egd: introduce a parameter to set buffer size

2013-12-11 Thread Amos Kong
On Tue, Dec 10, 2013 at 09:58:19AM -0700, Eric Blake wrote:
> On 12/09/2013 07:10 AM, Amos Kong wrote:

Hi Eric,

> > This patch makes the buffer size configurable, the max
> > buffer size is 65536.
> > 
> >  -object rng-egd,chardev=chr0,id=rng0,buf_size=1024
> > 
> > Signed-off-by: Amos Kong 
> > ---
> >  backends/rng-egd.c | 24 +---
> >  1 file changed, 21 insertions(+), 3 deletions(-)
> 
> > @@ -281,6 +298,7 @@ static void rng_egd_init(Object *obj)
> >  object_property_add_str(obj, "chardev",
> >  rng_egd_get_chardev, rng_egd_set_chardev,
> >  NULL);
> > +object_property_add_str(obj, "buf_size", NULL, rng_egd_set_buf_size, 
> > NULL);
> >  }
> 
> Will libvirt ever need to set this property?  And is it easily
> discoverable whether qemu supports or lacks this property?

This is ajust a RFC patch, we might fix the performance with other
method. If it won't cost too much memory for buf, we can remove
this parameter.

In this patch, we have a default buffer size.
 
> -- 
> Eric Blake   eblake redhat com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org



-- 
Amos.


pgpdVsHT5OVU1.pgp
Description: PGP signature


Re: [Qemu-devel] [PATCH 01/22] block: Move initialisation of BlockLimits to bdrv_refresh_limits()

2013-12-11 Thread Wenchao Xia
于 2013/12/12 5:08, Kevin Wolf 写道:
> This function separates filling the BlockLimits from bdrv_open(), which
> allows it to call it from other operations which may change the limits
> (e.g. modifications to the backing file chain or bdrv_reopen)
> 
> Signed-off-by: Kevin Wolf 
> ---
>   block.c   | 21 
>   block/iscsi.c | 87 
> ---
>   block/qcow2.c | 11 +-
>   block/qed.c   | 11 +-
>   block/vmdk.c  | 22 +---
>   include/block/block_int.h |  2 ++
>   6 files changed, 113 insertions(+), 41 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 13f001a..c32d856 100644
> --- a/block.c
> +++ b/block.c
> @@ -479,6 +479,21 @@ int bdrv_create_file(const char* filename, 
> QEMUOptionParameter *options,
>   return ret;
>   }
> 
> +static int bdrv_refresh_limits(BlockDriverState *bs)
> +{
> +BlockDriver *drv = bs->drv;
> +
> +memset(&bs->bl, 0, sizeof(bs->bl));
> +
> +if (!drv) {
> +return 0;
> +} else if (drv->bdrv_refresh_limits) {
> +return drv->bdrv_refresh_limits(bs);
> +}
> +
> +return 0;
It seems this line can be removed.

> +}
> +
>   /*
>* Create a uniquely-named empty temporary file.
>* Return 0 upon success, otherwise a negative errno value.
> @@ -833,6 +848,8 @@ static int bdrv_open_common(BlockDriverState *bs, 
> BlockDriverState *file,
>   goto free_and_fail;
>   }
> 
> +bdrv_refresh_limits(bs);
> +
>   #ifndef _WIN32
>   if (bs->is_temporary) {
>   assert(bs->filename[0] != '\0');
> @@ -1018,6 +1035,10 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict 
> *options, Error **errp)
>   }
>   pstrcpy(bs->backing_file, sizeof(bs->backing_file),
>   bs->backing_hd->file->filename);
> +
> +/* Recalculate the BlockLimits with the backing file */
> +bdrv_refresh_limits(bs);
> +
>   return 0;
>   }
> 
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 829d444..f3ded8c 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1425,6 +1425,56 @@ static int iscsi_open(BlockDriverState *bs, QDict 
> *options, int flags,
>   task = NULL;
>   }
> 
> +#if defined(LIBISCSI_FEATURE_NOP_COUNTER)
> +/* Set up a timer for sending out iSCSI NOPs */
> +iscsilun->nop_timer = timer_new_ms(QEMU_CLOCK_REALTIME, 
> iscsi_nop_timed_event, iscsilun);
> +timer_mod(iscsilun->nop_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + 
> NOP_INTERVAL);
> +#endif
> +
> +out:
> +qemu_opts_del(opts);
> +if (initiator_name != NULL) {
> +g_free(initiator_name);
> +}
> +if (iscsi_url != NULL) {
> +iscsi_destroy_url(iscsi_url);
> +}
> +if (task != NULL) {
> +scsi_free_scsi_task(task);
> +}
> +
> +if (ret) {
> +if (iscsi != NULL) {
> +iscsi_destroy_context(iscsi);
> +}
> +memset(iscsilun, 0, sizeof(IscsiLun));
> +}
> +return ret;
> +}
> +
> +static void iscsi_close(BlockDriverState *bs)
> +{
> +IscsiLun *iscsilun = bs->opaque;
> +struct iscsi_context *iscsi = iscsilun->iscsi;
> +
> +if (iscsilun->nop_timer) {
> +timer_del(iscsilun->nop_timer);
> +timer_free(iscsilun->nop_timer);
> +}
> +qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), NULL, NULL, NULL);
> +iscsi_destroy_context(iscsi);
> +g_free(iscsilun->zeroblock);
> +memset(iscsilun, 0, sizeof(IscsiLun));
> +}
> +
> +static int iscsi_refresh_limits(BlockDriverState *bs)
> +{
> +IscsiLun *iscsilun = bs->opaque;
> +struct scsi_task *task = NULL;
> +int ret;
> +
> +memset(&bs->bl, 0, sizeof(bs->bl));
> +
>   if (iscsilun->lbp.lbpu || iscsilun->lbp.lbpws) {
>   struct scsi_inquiry_block_limits *inq_bl;
>   task = iscsi_do_inquiry(iscsilun->iscsi, iscsilun->lun, 1,
> @@ -1462,48 +1512,14 @@ static int iscsi_open(BlockDriverState *bs, QDict 
> *options, int flags,
>iscsilun);
>   }
> 
> -#if defined(LIBISCSI_FEATURE_NOP_COUNTER)
> -/* Set up a timer for sending out iSCSI NOPs */
> -iscsilun->nop_timer = timer_new_ms(QEMU_CLOCK_REALTIME, 
> iscsi_nop_timed_event, iscsilun);
> -timer_mod(iscsilun->nop_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + 
> NOP_INTERVAL);
> -#endif
> -
> +ret = 0;
>   out:
> -qemu_opts_del(opts);
> -if (initiator_name != NULL) {
> -g_free(initiator_name);
> -}
> -if (iscsi_url != NULL) {
> -iscsi_destroy_url(iscsi_url);
> -}
>   if (task != NULL) {
>   scsi_free_scsi_task(task);
>   }
> -
> -if (ret) {
> -if (iscsi != NULL) {
> -iscsi_destroy_context(iscsi);
> -}
> -memset(iscsilun, 0, sizeof(IscsiLun));
> -}
>   return ret;
>   }
> 
> -static void iscsi_close(BlockDriverState *bs)
> -{
> -IscsiLun *iscsilun = bs->opaque;
> -struct iscsi_co

Re: [Qemu-devel] [PATCH] target-i386: clear guest TSC on reset

2013-12-11 Thread Fernando Luis Vázquez Cao

On 12/09/2013 05:50 PM, Fernando Luis Vázquez Cao wrote:

On 12/06/2013 11:22 PM, Marcelo Tosatti wrote:
On Fri, Dec 06, 2013 at 05:24:18PM +0900, Fernando Luis Vázquez Cao 
wrote:

I also wanted to make sure that the initialization that we do
in kvm_arch_vcpu_postcreate on power up and the subsequent
TSC writeback work well together, but I didn't have time to
test it (reading the code, I would say that the TSC generation
counter may end up being increased a few times but the TSCs
would eventually converge).

A basic test should be fine, because the matching code is in use
today.


I applied my two patches to QEMU and I did some testing
with SMP guests (4 VCPUs).


By the way, do you want me to merge the two patches I sent into
one and resend?

Thanks,
Fernando



When the host's TSC is stable all the TSC offsets are matched
as expected:

   [ 4544.779699] kvm: VCPU 0 new tsc generation 1, clock 0
   [ 4544.799691] kvm: VCPU 1 matched tsc offset for 0
   [ 4544.835687] kvm: VCPU 2 matched tsc offset for 0
   [ 4544.882229] kvm: VCPU 3 matched tsc offset for 0
   [ 4544.983740] kvm: VCPU 0 matched tsc offset for 0
   [ 4545.015727] kvm: VCPU 1 matched tsc offset for 0
   [ 4545.031762] kvm: VCPU 2 matched tsc offset for 0
   [ 4545.043756] kvm: VCPU 3 matched tsc offset for 0
   [ 4545.382113] kvm: VCPU 0 matched tsc offset for 0
   [ 4545.382138] kvm: VCPU 1 matched tsc offset for 0
   [ 4545.382155] kvm: VCPU 2 matched tsc offset for 0
   [ 4545.382171] kvm: VCPU 3 matched tsc offset for 0

Regarding unstable TSC hosts, I created one by executing

   TSCBSP=`rdmsr -d -p 0 16`; sleep 1s; wrmsr -p 0 16 $TSCBSP

as you suggested. After doing this KVM will adjust the TSC
offsets to make up for the deltas on the host side:

   [ 5232.172074] kvm: VCPU 0 new tsc generation 1, clock 0
   [ 5232.180759] kvm: SMP vm created on host with unstable TSC; guest 
TSC will not be reliable

   [ 5232.204069] kvm: VCPU 1 adjusted tsc offset by 105344
   [ 5232.268070] kvm: VCPU 2 adjusted tsc offset by 210721
   [ 5232.332066] kvm: VCPU 3 adjusted tsc offset by 210708
   [ 5232.444127] kvm: VCPU 0 adjusted tsc offset by 368959
   [ 5232.458448] kvm: VCPU 1 adjusted tsc offset by 47158
   [ 5232.470400] kvm: VCPU 2 adjusted tsc offset by 39352
   [ 5232.482359] kvm: VCPU 3 adjusted tsc offset by 39371
   [ 5232.875343] kvm: VCPU 0 adjusted tsc offset by 1293878
   [ 5232.875371] kvm: VCPU 1 adjusted tsc offset by 121
   [ 5232.875392] kvm: VCPU 2 adjusted tsc offset by 69
   [ 5232.875412] kvm: VCPU 3 adjusted tsc offset by 62

But despite KVM's efforts on the guest side the check in
check_tsc_sync_source() fails and the TSC is marked unstable:

   tsc: Marking TSC unstable due to check_tsc_sync_source failed

By the way, without my patches applied to QEMU the end result
is the same:

   [  266.300068] kvm: VCPU 0 new tsc generation 1, clock 0
   [  266.308746] kvm: SMP vm created on host with unstable TSC; guest 
TSC will not be reliable

   [  266.332067] kvm: VCPU 1 adjusted tsc offset by 105354
   [  266.396065] kvm: VCPU 2 adjusted tsc offset by 210714
   [  266.428273] kvm: VCPU 3 adjusted tsc offset by 106045

In other words, things seem to be working as expected.

- Fernando




Re: [Qemu-devel] Postcopy codebase status?

2013-12-11 Thread Isaku Yamahata
On Wed, Dec 11, 2013 at 03:37:18PM +,
"Dr. David Alan Gilbert"  wrote:

> Hi Isaku, Benoit,
>   I'm starting to look at postcopy and was looking back through
> the mailing list at your code and comments and just wondered if there
> were any more updates.
> 
>   Isaku: The last code drop to the list I found was
> http://lists.gnu.org/archive/html/qemu-devel/2012-10/msg05195.html
> but I see you're still working on your git repo, do you intend
> to make another post of that ?

Not anytime soon.
As you are aware, https://github.com/yamahata/qemu is still alive.


>   Benoit: I saw your slides and blog post about your work,
> but can't find your code - http://www.hecatonchire.com/ won't resolve
> for me.
> 
> I'll be looking at wiring up Andrea's USERFAULT/remap_anon_pages ideas.

Please keep me on Cc. The issue is how to handle fault in kvm kernel module.

thnaks,


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

-- 
Isaku Yamahata 



[Qemu-devel] [PATCH v2] tcg/i386: fix a comment

2013-12-11 Thread Aurelien Jarno
The comments apply to 8-bit stores, not 8-byte stores.

Cc: Richard Henderson 
Signed-off-by: Aurelien Jarno 
---
 tcg/i386/tcg-target.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
index 7ac8e45..495b901 100644
--- a/tcg/i386/tcg-target.c
+++ b/tcg/i386/tcg-target.c
@@ -1486,7 +1486,7 @@ static void tcg_out_qemu_st_direct(TCGContext *s, TCGReg 
datalo, TCGReg datahi,
 
 switch (memop & MO_SIZE) {
 case MO_8:
-/* In 32-bit mode, 8-byte stores can only happen from [abcd]x.
+/* In 32-bit mode, 8-bit stores can only happen from [abcd]x.
Use the scratch register if necessary.  */
 if (TCG_TARGET_REG_BITS == 32 && datalo >= 4) {
 tcg_out_mov(s, TCG_TYPE_I32, scratch, datalo);
-- 
1.7.10.4




Re: [Qemu-devel] [PATCH v10 5/5] hw/arm: add cubieboard support

2013-12-11 Thread Li Guang

Peter Crosthwaite wrote:

On Wed, Dec 11, 2013 at 8:31 PM, Peter Maydell  wrote:
   

On 11 December 2013 10:24, Peter Crosthwaite
  wrote:
 

On Wed, Dec 11, 2013 at 7:56 PM, Peter Maydell  wrote:
   

On 11 December 2013 05:59, Peter Crosthwaite
  wrote:
 

On Mon, Dec 9, 2013 at 10:10 AM, liguang  wrote:
   

Signed-off-by: liguang
 

Acked-by: Peter Crosthwaite
   

Why Acked-by rather than Reviewed-by ?

 

Not 100% myself on the new QOM styles and standards around boards and
SoC. But it is reviewed by me to the best of my knowledge. If that is
enough, please feel free to promote to Reviewed-by.
   

I'd call that Reviewed-by, yes. Acked-by is just "I don't object to this"
which is a sufficiently weak statement that it's not often used...

 

Ok,

Liguang, please drop the acks on p4 and p5 and replace by Reviewed-by
on next spin.

Reviewed-by: Peter Crosthwaite
   


Ok, thanks!



[Qemu-devel] issue with vgabios lfb and virtio vga

2013-12-11 Thread Dave Airlie
Hi Gerd,

so I have a bit of a conflict I'm not sure how best to handle between
how vgabios.c locates its LFB and how virtio requires the BAR get laid
out.

So virtio-pci requires the BARs are laid out for MSI support with the
config i/o ports at BAR 0, and MSI BAR at BAR 1.

Now the vgabios.c does a check of bar 0 and bar 1 to see if they are
0xfff1 masked, this protects against the the i/o bar but fails to
protect against the LFB one as PCI BARs don't encode the size just the
base address, and a 4k BAR can be aligned to a larger size.

So even if I expand the vgabios.c current code to check for a
potential BAR2 it'll fail if the BAR1 gets aligned to something better
than the 4k is requires.

The correct way to size BARs is to store the old value, write
0x and see how many bits stick, then write back the old value.

Any ideas? I seem to remember vgabios.c had a hack in the past for
vmware, but I'm not sure.

Dave.



Re: [Qemu-devel] [PATCH 3/5] qom: catch errors in object_property_add_child

2013-12-11 Thread Eric Blake
On 12/10/2013 10:00 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini 
> ---
>  qom/object.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)

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 2/5] qom: fix leak for objects created with -object

2013-12-11 Thread Eric Blake
On 12/10/2013 10:00 AM, Paolo Bonzini wrote:
> The object must be unref-ed when its variable goes out of scope.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  vl.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

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 1/5] rng: initialize file descriptor to -1

2013-12-11 Thread Eric Blake
On 12/10/2013 10:00 AM, Paolo Bonzini wrote:
> The file descriptor is never initialized to -1, which makes rng-random
> close stdin if an object is created and immediately destroyed.  If we
> change it to -1, we also need to protect qemu_set_fd_handler from
> receiving a bogus file descriptor.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  backends/rng-random.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

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] inet_listen_opts: add error checking

2013-12-11 Thread Eric Blake
On 12/11/2013 05:00 AM, Gerd Hoffmann wrote:
> Don't use atoi() function which doesn't detect errors, switch to
> strtol and error out on failures.  Also add a range check while
> being at it.
> 
> Signed-off-by: Gerd Hoffmann 
> ---

>  /* lookup */
> -if (port_offset)
> -snprintf(port, sizeof(port), "%d", atoi(port) + port_offset);
> +if (port_offset) {
> +int baseport;
> +errno = 0;
> +baseport = strtol(port, NULL, 10);

Fail #1: Silent truncation on 64-bit platforms.  If the user passed
0x1 you will see baseport==0 with no error indication (and
doesn't make it any nicer that a 32-bit platform will do what you wanted
- platform-dependent bugs are nasty).  :(

You need to assign the results of strtol() into a long, then do range
checking before truncating to int.

> +if (errno != 0) {
> +error_setg(errp, "can't convert to a number: %s", port);
> +return -1;
> +}

Fail #2: You forgot to do validation that a number was actually parsed.
 We hate atoi because atoi("a") is happily 0, but your use of strtol()
still has that bug.  POSIX states that implementations are allowed to
fail with EINVAL when parsing "a", but this failure mode is not required
to give an errno diagnostic.  Your code would reject "a" on glibc, but
accept it on other platforms (system-dependent bugs are nasty).  The
only portable way to ensure that a number was actually parsed and that
there is no garbage in the suffix is to pass in the address of a char*
in the second argument, then validate it against your string to ensure
that enough information was parsed and any suffix is expected.

The rest of this email is generic, and not specifically directed at you
or your patch:


WHY is strtol() such a PAINFUL interface to use correctly?  And WHY
can't qemu copy libvirt's lead of writing a SANE wrapper function, and
then mandating that the rest of the code base use the sane wrapper
instead of strtol()?


-- 
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 v3 0/6] Add error_abort and associated cleanups

2013-12-11 Thread Peter Crosthwaite
On Wed, Dec 11, 2013 at 11:57 PM, Markus Armbruster  wrote:
> Peter Crosthwaite  writes:
>
>> Following our discussion RE self asserting API calls, here is a spin of
>> my proposal. This series obsoletes the need for _nofail variants for
>> Error ** accepting APIs. Is also greatly reduces the verbosity of calls
>> sites that are currently asserting against errors.
>>
>> Patch 1 is the main event - addition of error_abort. The following
>> patches then cleanup uses of _nofail and assert_no_error().
>>
>> To give it a smoke test, I introduce a (critical) bug into QOM:
> [...]
>
> Reviewed-by: Markus Armbruster 
>

Thanks,

Do you have a suggested patch queue or person this should go via?

Regards,
Peter



[Qemu-devel] [PATCH 16/22] block: Make zero-after-EOF work with larger alignment

2013-12-11 Thread Kevin Wolf
Odd file sizes could make bdrv_aligned_preadv() shorten the request in
non-aligned ways. Fix it by rounding to the required alignment instead
of 512 bytes.

Signed-off-by: Kevin Wolf 
---
 block.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index b4f6ead..6dddb7c 100644
--- a/block.c
+++ b/block.c
@@ -2725,7 +2725,7 @@ err:
  */
 static int coroutine_fn bdrv_aligned_preadv(BlockDriverState *bs,
 BdrvTrackedRequest *req, int64_t offset, unsigned int bytes,
-QEMUIOVector *qiov, int flags)
+int64_t align, QEMUIOVector *qiov, int flags)
 {
 BlockDriver *drv = bs->drv;
 int ret;
@@ -2773,7 +2773,7 @@ static int coroutine_fn 
bdrv_aligned_preadv(BlockDriverState *bs,
 }
 
 total_sectors = DIV_ROUND_UP(len, BDRV_SECTOR_SIZE);
-max_nb_sectors = MAX(0, total_sectors - sector_num);
+max_nb_sectors = MAX(0, ROUND_UP(total_sectors - sector_num, align));
 if (max_nb_sectors > 0) {
 ret = drv->bdrv_co_readv(bs, sector_num,
  MIN(nb_sectors, max_nb_sectors), qiov);
@@ -2858,7 +2858,7 @@ static int coroutine_fn 
bdrv_co_do_preadv(BlockDriverState *bs,
 }
 
 tracked_request_begin(&req, bs, offset, bytes, false);
-ret = bdrv_aligned_preadv(bs, &req, offset, bytes,
+ret = bdrv_aligned_preadv(bs, &req, offset, bytes, align,
   use_local_qiov ? &local_qiov : qiov,
   flags);
 tracked_request_end(&req);
-- 
1.8.1.4




[Qemu-devel] [PATCH 06/22] block: Don't use guest sector size for qemu_blockalign()

2013-12-11 Thread Kevin Wolf
bs->buffer_alignment is set by the device emulation and contains the
logical block size of the guest device. This isn't something that the
block layer should know, and even less something to use for determining
the right alignment of buffers to be used for the host.

The new BlockLimits field opt_mem_alignment tells the qemu block layer
the optimal alignment to be used so that no bounce buffer must be used
in the driver.

This patch may change the buffer alignment from 4k to 512 for all
callers that used qemu_blockalign() with the top-level image format
BlockDriverState. The value was never propagated to other levels in the
tree, so in particular raw-posix never required anything else than 512.

While on disks with 4k sectors direct I/O requires a 4k alignment,
memory may still be okay when aligned to 512 byte boundaries. This is
what must have happened in practice, because otherwise this would
already have failed earlier. Therefore I don't expect regressions even
with this intermediate state. Later, raw-posix can implement the hook
and expose a different memory alignment requirement.

Signed-off-by: Kevin Wolf 
---
 block.c   | 23 ---
 include/block/block.h |  3 +++
 include/block/block_int.h |  3 +++
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 62c5a75..d50a4e4 100644
--- a/block.c
+++ b/block.c
@@ -214,6 +214,16 @@ static void bdrv_io_limits_intercept(BlockDriverState *bs,
 qemu_co_queue_next(&bs->throttled_reqs[is_write]);
 }
 
+size_t bdrv_opt_mem_align(BlockDriverState *bs)
+{
+if (!bs || !bs->drv) {
+/* 4k should be on the safe side */
+return 4096;
+}
+
+return bs->bl.opt_mem_alignment;
+}
+
 /* check if the path starts with ":" */
 static int path_has_protocol(const char *path)
 {
@@ -493,6 +503,9 @@ int bdrv_refresh_limits(BlockDriverState *bs)
 if (bs->file) {
 bdrv_refresh_limits(bs->file);
 bs->bl.opt_transfer_length = bs->file->bl.opt_transfer_length;
+bs->bl.opt_mem_alignment = bs->file->bl.opt_mem_alignment;
+} else {
+bs->bl.opt_mem_alignment = 512;
 }
 
 if (bs->backing_hd) {
@@ -500,6 +513,9 @@ int bdrv_refresh_limits(BlockDriverState *bs)
 bs->bl.opt_transfer_length =
 MAX(bs->bl.opt_transfer_length,
 bs->backing_hd->bl.opt_transfer_length);
+bs->bl.opt_mem_alignment =
+MAX(bs->bl.opt_mem_alignment,
+bs->backing_hd->bl.opt_mem_alignment);
 }
 
 /* Then let the driver override it */
@@ -4547,7 +4563,7 @@ void bdrv_set_buffer_alignment(BlockDriverState *bs, int 
align)
 
 void *qemu_blockalign(BlockDriverState *bs, size_t size)
 {
-return qemu_memalign((bs && bs->buffer_alignment) ? bs->buffer_alignment : 
512, size);
+return qemu_memalign(bdrv_opt_mem_align(bs), size);
 }
 
 /*
@@ -4556,12 +4572,13 @@ void *qemu_blockalign(BlockDriverState *bs, size_t size)
 bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov)
 {
 int i;
+size_t alignment = bdrv_opt_mem_align(bs);
 
 for (i = 0; i < qiov->niov; i++) {
-if ((uintptr_t) qiov->iov[i].iov_base % bs->buffer_alignment) {
+if ((uintptr_t) qiov->iov[i].iov_base % alignment) {
 return false;
 }
-if (qiov->iov[i].iov_len % bs->buffer_alignment) {
+if (qiov->iov[i].iov_len % alignment) {
 return false;
 }
 }
diff --git a/include/block/block.h b/include/block/block.h
index 64fb319..cf63ee2 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -419,6 +419,9 @@ void bdrv_img_create(const char *filename, const char *fmt,
  char *options, uint64_t img_size, int flags,
  Error **errp, bool quiet);
 
+/* Returns the alignment in bytes that is required so that no bounce buffer
+ * is required throughout the stack */
+size_t bdrv_opt_mem_align(BlockDriverState *bs);
 void bdrv_set_buffer_alignment(BlockDriverState *bs, int align);
 void *qemu_blockalign(BlockDriverState *bs, size_t size);
 bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index c49fa6b..6ffae7c 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -252,6 +252,9 @@ typedef struct BlockLimits {
 
 /* optimal transfer length in sectors */
 int opt_transfer_length;
+
+/* memory alignment so that no bounce buffer is needed */
+size_t opt_mem_alignment;
 } BlockLimits;
 
 /*
-- 
1.8.1.4




[Qemu-devel] [PATCH 01/22] block: Move initialisation of BlockLimits to bdrv_refresh_limits()

2013-12-11 Thread Kevin Wolf
This function separates filling the BlockLimits from bdrv_open(), which
allows it to call it from other operations which may change the limits
(e.g. modifications to the backing file chain or bdrv_reopen)

Signed-off-by: Kevin Wolf 
---
 block.c   | 21 
 block/iscsi.c | 87 ---
 block/qcow2.c | 11 +-
 block/qed.c   | 11 +-
 block/vmdk.c  | 22 +---
 include/block/block_int.h |  2 ++
 6 files changed, 113 insertions(+), 41 deletions(-)

diff --git a/block.c b/block.c
index 13f001a..c32d856 100644
--- a/block.c
+++ b/block.c
@@ -479,6 +479,21 @@ int bdrv_create_file(const char* filename, 
QEMUOptionParameter *options,
 return ret;
 }
 
+static int bdrv_refresh_limits(BlockDriverState *bs)
+{
+BlockDriver *drv = bs->drv;
+
+memset(&bs->bl, 0, sizeof(bs->bl));
+
+if (!drv) {
+return 0;
+} else if (drv->bdrv_refresh_limits) {
+return drv->bdrv_refresh_limits(bs);
+}
+
+return 0;
+}
+
 /*
  * Create a uniquely-named empty temporary file.
  * Return 0 upon success, otherwise a negative errno value.
@@ -833,6 +848,8 @@ static int bdrv_open_common(BlockDriverState *bs, 
BlockDriverState *file,
 goto free_and_fail;
 }
 
+bdrv_refresh_limits(bs);
+
 #ifndef _WIN32
 if (bs->is_temporary) {
 assert(bs->filename[0] != '\0');
@@ -1018,6 +1035,10 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict 
*options, Error **errp)
 }
 pstrcpy(bs->backing_file, sizeof(bs->backing_file),
 bs->backing_hd->file->filename);
+
+/* Recalculate the BlockLimits with the backing file */
+bdrv_refresh_limits(bs);
+
 return 0;
 }
 
diff --git a/block/iscsi.c b/block/iscsi.c
index 829d444..f3ded8c 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1425,6 +1425,56 @@ static int iscsi_open(BlockDriverState *bs, QDict 
*options, int flags,
 task = NULL;
 }
 
+#if defined(LIBISCSI_FEATURE_NOP_COUNTER)
+/* Set up a timer for sending out iSCSI NOPs */
+iscsilun->nop_timer = timer_new_ms(QEMU_CLOCK_REALTIME, 
iscsi_nop_timed_event, iscsilun);
+timer_mod(iscsilun->nop_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + 
NOP_INTERVAL);
+#endif
+
+out:
+qemu_opts_del(opts);
+if (initiator_name != NULL) {
+g_free(initiator_name);
+}
+if (iscsi_url != NULL) {
+iscsi_destroy_url(iscsi_url);
+}
+if (task != NULL) {
+scsi_free_scsi_task(task);
+}
+
+if (ret) {
+if (iscsi != NULL) {
+iscsi_destroy_context(iscsi);
+}
+memset(iscsilun, 0, sizeof(IscsiLun));
+}
+return ret;
+}
+
+static void iscsi_close(BlockDriverState *bs)
+{
+IscsiLun *iscsilun = bs->opaque;
+struct iscsi_context *iscsi = iscsilun->iscsi;
+
+if (iscsilun->nop_timer) {
+timer_del(iscsilun->nop_timer);
+timer_free(iscsilun->nop_timer);
+}
+qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), NULL, NULL, NULL);
+iscsi_destroy_context(iscsi);
+g_free(iscsilun->zeroblock);
+memset(iscsilun, 0, sizeof(IscsiLun));
+}
+
+static int iscsi_refresh_limits(BlockDriverState *bs)
+{
+IscsiLun *iscsilun = bs->opaque;
+struct scsi_task *task = NULL;
+int ret;
+
+memset(&bs->bl, 0, sizeof(bs->bl));
+
 if (iscsilun->lbp.lbpu || iscsilun->lbp.lbpws) {
 struct scsi_inquiry_block_limits *inq_bl;
 task = iscsi_do_inquiry(iscsilun->iscsi, iscsilun->lun, 1,
@@ -1462,48 +1512,14 @@ static int iscsi_open(BlockDriverState *bs, QDict 
*options, int flags,
  iscsilun);
 }
 
-#if defined(LIBISCSI_FEATURE_NOP_COUNTER)
-/* Set up a timer for sending out iSCSI NOPs */
-iscsilun->nop_timer = timer_new_ms(QEMU_CLOCK_REALTIME, 
iscsi_nop_timed_event, iscsilun);
-timer_mod(iscsilun->nop_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + 
NOP_INTERVAL);
-#endif
-
+ret = 0;
 out:
-qemu_opts_del(opts);
-if (initiator_name != NULL) {
-g_free(initiator_name);
-}
-if (iscsi_url != NULL) {
-iscsi_destroy_url(iscsi_url);
-}
 if (task != NULL) {
 scsi_free_scsi_task(task);
 }
-
-if (ret) {
-if (iscsi != NULL) {
-iscsi_destroy_context(iscsi);
-}
-memset(iscsilun, 0, sizeof(IscsiLun));
-}
 return ret;
 }
 
-static void iscsi_close(BlockDriverState *bs)
-{
-IscsiLun *iscsilun = bs->opaque;
-struct iscsi_context *iscsi = iscsilun->iscsi;
-
-if (iscsilun->nop_timer) {
-timer_del(iscsilun->nop_timer);
-timer_free(iscsilun->nop_timer);
-}
-qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), NULL, NULL, NULL);
-iscsi_destroy_context(iscsi);
-g_free(iscsilun->zeroblock);
-memset(iscsilun, 0, sizeof(IscsiLun));
-}
-
 static int iscsi_truncate(BlockDriverState *bs, int64_t offset)
 {
 IscsiLun *iscsilun = b

[Qemu-devel] [PATCH v2 1/8] target-arm: A64: add support for ld/st pair

2013-12-11 Thread Peter Maydell
From: Alex Bennée 

This patch support the basic load and store pair instructions and
includes the generic helper functions:

  * do_gpr_st()
  * do_fp_st()
  * do_gpr_ld()
  * do_fp_ld()
  * read_cpu_reg_sp()
  * gen_check_sp_alignment()

The last function gen_check_sp_alignment() is a NULL op currently but
put in place to make it easy to add SP alignment checking later.

Signed-off-by: Alex Bennée 
Signed-off-by: Peter Maydell 

---

Changes
  - merge ldp/stp together
  - add read_cpu_reg_sp() and use
  - only reference registers as needed
  - use common tcg ops for gpr/fp loads/store
---
 target-arm/translate-a64.c | 268 -
 1 file changed, 263 insertions(+), 5 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 0a76130..018b3ee 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -99,6 +99,15 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
 cpu_fprintf(f, "\n");
 }
 
+static int get_mem_index(DisasContext *s)
+{
+#ifdef CONFIG_USER_ONLY
+return 1;
+#else
+return s->user;
+#endif
+}
+
 void gen_a64_set_pc_im(uint64_t val)
 {
 tcg_gen_movi_i64(cpu_pc, val);
@@ -250,6 +259,17 @@ static TCGv_i64 read_cpu_reg(DisasContext *s, int reg, int 
sf)
 return v;
 }
 
+static TCGv_i64 read_cpu_reg_sp(DisasContext *s, int reg, int sf)
+{
+TCGv_i64 v = new_tmp_a64(s);
+if (sf) {
+tcg_gen_mov_i64(v, cpu_X[reg]);
+} else {
+tcg_gen_ext32u_i64(v, cpu_X[reg]);
+}
+return v;
+}
+
 /* Set ZF and NF based on a 64 bit result. This is alas fiddlier
  * than the 32 bit equivalent.
  */
@@ -278,6 +298,124 @@ static inline void gen_logic_CC(int sf, TCGv_i64 result)
 }
 
 /*
+ * Load/Store generators
+ */
+
+/*
+ *  Store from GPR register to memory
+ */
+static void do_gpr_st(DisasContext *s, TCGv_i64 source,
+  TCGv_i64 tcg_addr, int size)
+{
+g_assert(size <= 3);
+tcg_gen_qemu_st_i64(source, tcg_addr, get_mem_index(s), MO_TE + size);
+}
+
+/*
+ * Load from memory to GPR register
+ */
+static void do_gpr_ld(DisasContext *s, TCGv_i64 dest, TCGv_i64 tcg_addr,
+  int size, bool is_signed, bool extend)
+{
+TCGMemOp memop =  MO_TE + size;
+
+g_assert(size <= 3);
+
+if (is_signed) {
+memop += MO_SIGN;
+}
+
+tcg_gen_qemu_ld_i64(dest, tcg_addr, get_mem_index(s), memop);
+
+if (extend && is_signed) {
+g_assert(size < 3);
+tcg_gen_ext32u_i64(dest, dest);
+}
+}
+
+/*
+ * Store from FP register to memory
+ */
+static void do_fp_st(DisasContext *s, int srcidx, TCGv_i64 tcg_addr, int size)
+{
+/* This writes the bottom N bits of a 128 bit wide vector to memory */
+int freg_offs = offsetof(CPUARMState, vfp.regs[srcidx * 2]);
+TCGv_i64 tmp = tcg_temp_new_i64();
+
+if (size < 4) {
+switch (size) {
+case 0:
+tcg_gen_ld8u_i64(tmp, cpu_env, freg_offs);
+break;
+case 1:
+tcg_gen_ld16u_i64(tmp, cpu_env, freg_offs);
+break;
+case 2:
+tcg_gen_ld32u_i64(tmp, cpu_env, freg_offs);
+break;
+case 3:
+tcg_gen_ld_i64(tmp, cpu_env, freg_offs);
+break;
+}
+tcg_gen_qemu_st_i64(tmp, tcg_addr, get_mem_index(s), MO_TE + size);
+} else {
+TCGv_i64 tcg_hiaddr = tcg_temp_new_i64();
+tcg_gen_ld_i64(tmp, cpu_env, freg_offs);
+tcg_gen_qemu_st_i64(tmp, tcg_addr, get_mem_index(s), MO_TEQ);
+tcg_gen_qemu_st64(tmp, tcg_addr, get_mem_index(s));
+tcg_gen_ld_i64(tmp, cpu_env, freg_offs + sizeof(float64));
+tcg_gen_addi_i64(tcg_hiaddr, tcg_addr, 8);
+tcg_gen_qemu_st_i64(tmp, tcg_hiaddr, get_mem_index(s), MO_TEQ);
+tcg_temp_free_i64(tcg_hiaddr);
+}
+
+tcg_temp_free_i64(tmp);
+}
+
+/* Load from memory to FP register */
+static void do_fp_ld(DisasContext *s, int destidx, TCGv_i64 tcg_addr, int size)
+{
+/* This always zero-extends and writes to a full 128 bit wide vector */
+int freg_offs = offsetof(CPUARMState, vfp.regs[destidx * 2]);
+TCGv_i64 tmplo = tcg_temp_new_i64();
+TCGv_i64 tmphi;
+
+if (size < 4) {
+TCGMemOp memop =  MO_TE + size;
+tmphi = tcg_const_i64(0);
+tcg_gen_qemu_ld_i64(tmplo, tcg_addr, get_mem_index(s), memop);
+} else {
+TCGv_i64 tcg_hiaddr;
+tmphi = tcg_temp_new_i64();
+tcg_hiaddr = tcg_temp_new_i64();
+
+tcg_gen_qemu_ld_i64(tmplo, tcg_addr, get_mem_index(s), MO_TEQ);
+tcg_gen_addi_i64(tcg_hiaddr, tcg_addr, 8);
+tcg_gen_qemu_ld_i64(tmphi, tcg_hiaddr, get_mem_index(s), MO_TEQ);
+tcg_temp_free_i64(tcg_hiaddr);
+}
+
+tcg_gen_st_i64(tmplo, cpu_env, freg_offs);
+tcg_gen_st_i64(tmphi, cpu_env, freg_offs + sizeof(float64));
+
+tcg_temp_free_i64(tmplo);
+tcg_temp_free_i64(tmphi);
+}
+
+static inline void gen_check_sp_alignment(DisasContext *s)
+{
+

[Qemu-devel] [PATCH v2 6/8] target-arm: A64: add support for move wide instructions

2013-12-11 Thread Peter Maydell
From: Alex Bennée 

This patch adds emulation for the mov wide instructions
(MOVN, MOVZ, MOVK).

Signed-off-by: Alex Bennée 
Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
---
 target-arm/translate-a64.c | 51 --
 1 file changed, 49 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 59393f3..4a6252e 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1626,10 +1626,57 @@ static void disas_logic_imm(DisasContext *s, uint32_t 
insn)
 }
 }
 
-/* Move wide (immediate) */
+/*
+ * C3.4.5 Move wide (immediate)
+ *
+ *  31 30 29 28 23 22 21 20 5 40
+ * +--+-+-+-++--+
+ * |sf| opc | 1 0 0 1 0 1 |  hw |  imm16 |  Rd  |
+ * +--+-+-+-++--+
+ *
+ * sf: 0 -> 32 bit, 1 -> 64 bit
+ * opc: 00 -> N, 10 -> Z, 11 -> K
+ * hw: shift/16 (0,16, and sf only 32, 48)
+ */
 static void disas_movw_imm(DisasContext *s, uint32_t insn)
 {
-unsupported_encoding(s, insn);
+int rd = extract32(insn, 0, 5);
+uint64_t imm = extract32(insn, 5, 16);
+int sf = extract32(insn, 31, 1);
+int opc = extract32(insn, 29, 2);
+int pos = extract32(insn, 21, 2) << 4;
+TCGv_i64 tcg_rd = cpu_reg(s, rd);
+TCGv_i64 tcg_imm;
+
+if (!sf && (pos >= 32)) {
+unallocated_encoding(s);
+return;
+}
+
+switch (opc) {
+case 0: /* MOVN */
+case 2: /* MOVZ */
+imm <<= pos;
+if (opc == 0) {
+imm = ~imm;
+}
+if (!sf) {
+imm &= 0xu;
+}
+tcg_gen_movi_i64(tcg_rd, imm);
+break;
+case 3: /* MOVK */
+tcg_imm = tcg_const_i64(imm);
+tcg_gen_deposit_i64(tcg_rd, tcg_rd, tcg_imm, pos, 16);
+tcg_temp_free_i64(tcg_imm);
+if (!sf) {
+tcg_gen_ext32u_i64(tcg_rd, tcg_rd);
+}
+break;
+default:
+unallocated_encoding(s);
+break;
+}
 }
 
 /* C3.4.2 Bitfield
-- 
1.8.5




[Qemu-devel] [PATCH v2 5/8] target-arm: A64: add support for add, addi, sub, subi

2013-12-11 Thread Peter Maydell
From: Alex Bennée 

Implement the non-carry forms of addition and subtraction
(immediate, extended register and shifted register).
This includes the code to calculate NZCV if the instruction
calls for setting the flags.

Signed-off-by: Alex Bennée 
Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 

---
Changes:
  - remove pointless mov op (we already have a copy)
---
 target-arm/translate-a64.c | 299 +++--
 1 file changed, 289 insertions(+), 10 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 791555d..59393f3 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -297,6 +297,102 @@ static inline void gen_logic_CC(int sf, TCGv_i64 result)
 tcg_gen_movi_i32(cpu_VF, 0);
 }
 
+/* dest = T0 + T1; compute C, N, V and Z flags */
+static void gen_add_CC(int sf, TCGv_i64 dest, TCGv_i64 t0, TCGv_i64 t1)
+{
+if (sf) {
+TCGv_i64 result, flag, tmp;
+result = tcg_temp_new_i64();
+flag = tcg_temp_new_i64();
+tmp = tcg_temp_new_i64();
+
+tcg_gen_movi_i64(tmp, 0);
+tcg_gen_add2_i64(result, flag, t0, tmp, t1, tmp);
+
+tcg_gen_trunc_i64_i32(cpu_CF, flag);
+
+gen_set_NZ64(result);
+
+tcg_gen_xor_i64(flag, result, t0);
+tcg_gen_xor_i64(tmp, t0, t1);
+tcg_gen_andc_i64(flag, flag, tmp);
+tcg_temp_free_i64(tmp);
+tcg_gen_shri_i64(flag, flag, 32);
+tcg_gen_trunc_i64_i32(cpu_VF, flag);
+
+tcg_gen_mov_i64(dest, result);
+tcg_temp_free_i64(result);
+tcg_temp_free_i64(flag);
+} else {
+/* 32 bit arithmetic */
+TCGv_i32 t0_32 = tcg_temp_new_i32();
+TCGv_i32 t1_32 = tcg_temp_new_i32();
+TCGv_i32 tmp = tcg_temp_new_i32();
+
+tcg_gen_movi_i32(tmp, 0);
+tcg_gen_trunc_i64_i32(t0_32, t0);
+tcg_gen_trunc_i64_i32(t1_32, t1);
+tcg_gen_add2_i32(cpu_NF, cpu_CF, t0_32, tmp, t1_32, tmp);
+tcg_gen_mov_i32(cpu_ZF, cpu_NF);
+tcg_gen_xor_i32(cpu_VF, cpu_NF, t0_32);
+tcg_gen_xor_i32(tmp, t0_32, t1_32);
+tcg_gen_andc_i32(cpu_VF, cpu_VF, tmp);
+tcg_gen_extu_i32_i64(dest, cpu_NF);
+
+tcg_temp_free_i32(tmp);
+tcg_temp_free_i32(t0_32);
+tcg_temp_free_i32(t1_32);
+}
+}
+
+/* dest = T0 - T1; compute C, N, V and Z flags */
+static void gen_sub_CC(int sf, TCGv_i64 dest, TCGv_i64 t0, TCGv_i64 t1)
+{
+if (sf) {
+/* 64 bit arithmetic */
+TCGv_i64 result, flag, tmp;
+
+result = tcg_temp_new_i64();
+flag = tcg_temp_new_i64();
+tcg_gen_sub_i64(result, t0, t1);
+
+gen_set_NZ64(result);
+
+tcg_gen_setcond_i64(TCG_COND_GEU, flag, t0, t1);
+tcg_gen_trunc_i64_i32(cpu_CF, flag);
+
+tcg_gen_xor_i64(flag, result, t0);
+tmp = tcg_temp_new_i64();
+tcg_gen_xor_i64(tmp, t0, t1);
+tcg_gen_and_i64(flag, flag, tmp);
+tcg_temp_free_i64(tmp);
+tcg_gen_shri_i64(flag, flag, 32);
+tcg_gen_trunc_i64_i32(cpu_VF, flag);
+tcg_gen_mov_i64(dest, result);
+tcg_temp_free_i64(flag);
+tcg_temp_free_i64(result);
+} else {
+/* 32 bit arithmetic */
+TCGv_i32 t0_32 = tcg_temp_new_i32();
+TCGv_i32 t1_32 = tcg_temp_new_i32();
+TCGv_i32 tmp;
+
+tcg_gen_trunc_i64_i32(t0_32, t0);
+tcg_gen_trunc_i64_i32(t1_32, t1);
+tcg_gen_sub_i32(cpu_NF, t0_32, t1_32);
+tcg_gen_mov_i32(cpu_ZF, cpu_NF);
+tcg_gen_setcond_i32(TCG_COND_GEU, cpu_CF, t0_32, t1_32);
+tcg_gen_xor_i32(cpu_VF, cpu_NF, t0_32);
+tmp = tcg_temp_new_i32();
+tcg_gen_xor_i32(tmp, t0_32, t1_32);
+tcg_temp_free_i32(t0_32);
+tcg_temp_free_i32(t1_32);
+tcg_gen_and_i32(cpu_VF, cpu_VF, tmp);
+tcg_temp_free_i32(tmp);
+tcg_gen_extu_i32_i64(dest, cpu_NF);
+}
+}
+
 /*
  * Load/Store generators
  */
@@ -1311,10 +1407,67 @@ static void disas_pc_rel_adr(DisasContext *s, uint32_t 
insn)
 tcg_gen_movi_i64(cpu_reg(s, rd), base + offset);
 }
 
-/* Add/subtract (immediate) */
+/*
+ * C3.4.1 Add/subtract (immediate)
+ *
+ *  31 30 29 28   24 23 22 21 10 9   5 4   0
+ * +--+--+--+---+-+-+-+-+
+ * |sf|op| S| 1 0 0 0 1 |shift|imm12|  Rn | Rd  |
+ * +--+--+--+---+-+-+-+-+
+ *
+ *sf: 0 -> 32bit, 1 -> 64bit
+ *op: 0 -> add  , 1 -> sub
+ * S: 1 -> set flags
+ * shift: 00 -> LSL imm by 0, 01 -> LSL imm by 12
+ */
 static void disas_add_sub_imm(DisasContext *s, uint32_t insn)
 {
-unsupported_encoding(s, insn);
+int rd = extract32(insn, 0, 5);
+int rn = extract32(insn, 5, 5);
+uint64_t imm = extract32(insn, 10, 12);
+int shift = extract32(insn, 22, 2);
+bool setflags = extract32(insn, 29, 1);
+bool sub_op = extract32(insn, 30, 1);
+bool is_64bit = extract32(insn, 31, 1);
+
+TCG

[Qemu-devel] [PATCH v2 0/8] target-arm: A64 decoder set 3: loads, stores, misc integer

2013-12-11 Thread Peter Maydell
Here's version 2 of the third set of A64 decoder patches
(loads, stores, misc integer).

Changes v1->v2:
 * merged ldp and stp into one function/patch
 * minor cleanup as per RTH review
 * use the new tcg ops for guest load/store
 * catch the missing UNALLOCATED cases for load/store
 * add missing returns after unallocated_encoding() calls
   in vector load/store decode
 * use tcg ops for mul[su]h

thanks
-- PMM

Alex Bennée (6):
  target-arm: A64: add support for ld/st pair
  target-arm: A64: add support for ld/st unsigned imm
  target-arm: A64: add support for ld/st with reg offset
  target-arm: A64: add support for ld/st with index
  target-arm: A64: add support for add, addi, sub, subi
  target-arm: A64: add support for move wide instructions

Alexander Graf (2):
  target-arm: A64: add support for 3 src data proc insns
  target-arm: A64: implement SVC, BRK

 target-arm/translate-a64.c | 1114 +++-
 1 file changed, 1092 insertions(+), 22 deletions(-)

-- 
1.8.5




[Qemu-devel] [PATCH v2 8/8] target-arm: A64: implement SVC, BRK

2013-12-11 Thread Peter Maydell
From: Alexander Graf 

Add decoding for the exception generating instructions, and implement
SVC (syscalls) and BRK (software breakpoint).

Signed-off-by: Alexander Graf 
Signed-off-by: Alex Bennée 
Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
---
 target-arm/translate-a64.c | 51 --
 1 file changed, 49 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 019a6ed..df4409b 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -806,10 +806,57 @@ static void disas_system(DisasContext *s, uint32_t insn)
 }
 }
 
-/* Exception generation */
+/* C3.2.3 Exception generation
+ *
+ *  31 24 23 21 20 5 4   2 1  0
+ * +-+-++-++
+ * | 1 1 0 1 0 1 0 0 | opc |  imm16 | op2 | LL |
+ * +---++--+
+ */
 static void disas_exc(DisasContext *s, uint32_t insn)
 {
-unsupported_encoding(s, insn);
+int opc = extract32(insn, 21, 3);
+int op2_ll = extract32(insn, 0, 5);
+
+switch (opc) {
+case 0:
+/* SVC, HVC, SMC; since we don't support the Virtualization
+ * or TrustZone extensions these all UNDEF except SVC.
+ */
+if (op2_ll != 1) {
+unallocated_encoding(s);
+break;
+}
+gen_exception_insn(s, 0, EXCP_SWI);
+break;
+case 1:
+if (op2_ll != 0) {
+unallocated_encoding(s);
+break;
+}
+/* BRK */
+gen_exception_insn(s, 0, EXCP_BKPT);
+break;
+case 2:
+if (op2_ll != 0) {
+unallocated_encoding(s);
+break;
+}
+/* HLT */
+unsupported_encoding(s, insn);
+break;
+case 5:
+if (op2_ll < 1 || op2_ll > 3) {
+unallocated_encoding(s);
+break;
+}
+/* DCPS1, DCPS2, DCPS3 */
+unsupported_encoding(s, insn);
+break;
+default:
+unallocated_encoding(s);
+break;
+}
 }
 
 /* C3.2.7 Unconditional branch (register)
-- 
1.8.5




[Qemu-devel] [PATCH v2 4/8] target-arm: A64: add support for ld/st with index

2013-12-11 Thread Peter Maydell
From: Alex Bennée 

This adds support for the pre/post-index ld/st forms with immediate
offsets as well as the un-scaled immediate form (which are all
variations on the same 9-bit immediate instruction form).

Signed-off-by: Alex Bennée 
Signed-off-by: Peter Maydell 
---
 target-arm/translate-a64.c | 125 -
 1 file changed, 124 insertions(+), 1 deletion(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 1ea3c0e..791555d 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -933,6 +933,110 @@ static void handle_ldst_pair(DisasContext *s, uint32_t 
insn)
 }
 
 /*
+ * C3.3.8 Load/store (immediate post-indexed)
+ * C3.3.9 Load/store (immediate pre-indexed)
+ * C3.3.12 Load/store (unscaled immediate)
+ *
+ * 31 30 29   27  26 25 24 23 22 21  2012 11 10 95 40
+ * ++---+---+-+-+---++-+--+--+
+ * |size| 1 1 1 | V | 0 0 | opc | 0 |  imm9  | idx |  Rn  |  Rt  |
+ * ++---+---+-+-+---++-+--+--+
+ *
+ * idx = 01 -> post-indexed, 11 pre-indexed, 00 unscaled imm. (no writeback)
+ * V = 0 -> non-vector
+ * size: 00 -> 8 bit, 01 -> 16 bit, 10 -> 32 bit, 11 -> 64bit
+ * opc: 00 -> store, 01 -> loadu, 10 -> loads 64, 11 -> loads 32
+ */
+static void handle_ldst_reg_imm9(DisasContext *s, uint32_t insn)
+{
+int rt = extract32(insn, 0, 5);
+int rn = extract32(insn, 5, 5);
+int imm9 = sextract32(insn, 12, 9);
+int opc = extract32(insn, 22, 2);
+int size = extract32(insn, 30, 2);
+int idx = extract32(insn, 10, 2);
+bool is_signed = false;
+bool is_store = false;
+bool is_extended = false;
+bool is_vector = extract32(insn, 26, 1);
+bool post_index;
+bool writeback;
+
+TCGv_i64 tcg_addr;
+
+if (is_vector) {
+size |= (opc & 2) << 1;
+if (size > 4) {
+unallocated_encoding(s);
+return;
+}
+is_store = ((opc & 1) == 0);
+} else {
+if (size == 3 && opc == 2) {
+/* PRFM - prefetch */
+return;
+}
+if (opc == 3 && size > 1) {
+unallocated_encoding(s);
+return;
+}
+is_store = (opc == 0);
+is_signed = opc & (1<<1);
+is_extended = (size < 3) && (opc & 1);
+}
+
+switch (idx) {
+case 0:
+post_index = false;
+writeback = false;
+break;
+case 1:
+post_index = true;
+writeback = true;
+break;
+case 3:
+post_index = false;
+writeback = true;
+break;
+case 2:
+g_assert(false);
+break;
+}
+
+if (rn == 31) {
+gen_check_sp_alignment(s);
+}
+tcg_addr = read_cpu_reg_sp(s, rn, 1);
+
+if (!post_index) {
+tcg_gen_addi_i64(tcg_addr, tcg_addr, imm9);
+}
+
+if (is_vector) {
+if (is_store) {
+do_fp_st(s, rt, tcg_addr, size);
+} else {
+do_fp_ld(s, rt, tcg_addr, size);
+}
+} else {
+TCGv_i64 tcg_rt = cpu_reg(s, rt);
+if (is_store) {
+do_gpr_st(s, tcg_rt, tcg_addr, size);
+} else {
+do_gpr_ld(s, tcg_rt, tcg_addr, size, is_signed, is_extended);
+}
+}
+
+if (writeback) {
+TCGv_i64 tcg_rn = cpu_reg_sp(s, rn);
+if (post_index) {
+tcg_gen_addi_i64(tcg_addr, tcg_addr, imm9);
+}
+tcg_gen_mov_i64(tcg_rn, tcg_addr);
+}
+}
+
+/*
  * C3.3.10 Load/store (register offset)
  *
  * 31 30 29   27  26 25 24 23 22 21  20  16 15 13 12 11 10 9  5 4  0
@@ -1099,6 +1203,25 @@ static void handle_ldst_reg_unsigned_imm(DisasContext 
*s, uint32_t insn)
 }
 }
 
+/* Load/store register (immediate forms) */
+static void disas_ldst_reg_imm(DisasContext *s, uint32_t insn)
+{
+switch (extract32(insn, 10, 2)) {
+case 0: case 1: case 3:
+/* Load/store register (unscaled immediate) */
+/* Load/store immediate pre/post-indexed */
+handle_ldst_reg_imm9(s, insn);
+break;
+case 2:
+/* Load/store register unprivileged */
+unsupported_encoding(s, insn);
+break;
+default:
+unallocated_encoding(s);
+break;
+}
+}
+
 /* Load/store register (all forms) */
 static void disas_ldst_reg(DisasContext *s, uint32_t insn)
 {
@@ -1107,7 +1230,7 @@ static void disas_ldst_reg(DisasContext *s, uint32_t insn)
 if (extract32(insn, 21, 1) == 1 && extract32(insn, 10, 2) == 2) {
 handle_ldst_reg_roffset(s, insn);
 } else {
-unsupported_encoding(s, insn);
+disas_ldst_reg_imm(s, insn);
 }
 break;
 case 1:
-- 
1.8.5




[Qemu-devel] [PATCH v2 3/8] target-arm: A64: add support for ld/st with reg offset

2013-12-11 Thread Peter Maydell
From: Alex Bennée 

This adds support for the load/store forms using a register offset.

Signed-off-by: Alex Bennée 
Signed-off-by: Peter Maydell 
---
 target-arm/translate-a64.c | 144 -
 1 file changed, 143 insertions(+), 1 deletion(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 7d3f432..1ea3c0e 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -402,6 +402,54 @@ static void do_fp_ld(DisasContext *s, int destidx, 
TCGv_i64 tcg_addr, int size)
 tcg_temp_free_i64(tmphi);
 }
 
+/*
+ * This utility function is for doing register extension with an
+ * optional shift. You will likely want to pass a temporary for the
+ * destination register. See DecodeRegExtend() in the ARM ARM.
+ */
+static void ext_and_shift_reg(TCGv_i64 tcg_out, TCGv_i64 tcg_in,
+  int option, unsigned int shift)
+{
+int extsize = extract32(option, 0, 2);
+bool is_signed = extract32(option, 2, 1);
+
+if (is_signed) {
+switch (extsize) {
+case 0:
+tcg_gen_ext8s_i64(tcg_out, tcg_in);
+break;
+case 1:
+tcg_gen_ext16s_i64(tcg_out, tcg_in);
+break;
+case 2:
+tcg_gen_ext32s_i64(tcg_out, tcg_in);
+break;
+case 3:
+tcg_gen_mov_i64(tcg_out, tcg_in);
+break;
+}
+} else {
+switch (extsize) {
+case 0:
+tcg_gen_ext8u_i64(tcg_out, tcg_in);
+break;
+case 1:
+tcg_gen_ext16u_i64(tcg_out, tcg_in);
+break;
+case 2:
+tcg_gen_ext32u_i64(tcg_out, tcg_in);
+break;
+case 3:
+tcg_gen_mov_i64(tcg_out, tcg_in);
+break;
+}
+}
+
+if (shift) {
+tcg_gen_shli_i64(tcg_out, tcg_out, shift);
+}
+}
+
 static inline void gen_check_sp_alignment(DisasContext *s)
 {
 /* The AArch64 architecture mandates that (if enabled via PSTATE
@@ -885,6 +933,96 @@ static void handle_ldst_pair(DisasContext *s, uint32_t 
insn)
 }
 
 /*
+ * C3.3.10 Load/store (register offset)
+ *
+ * 31 30 29   27  26 25 24 23 22 21  20  16 15 13 12 11 10 9  5 4  0
+ * ++---+---+-+-+---+--+-+--+-+++
+ * |size| 1 1 1 | V | 0 0 | opc | 1 |  Rm  | opt | S| 1 0 | Rn | Rt |
+ * ++---+---+-+-+---+--+-+--+-+++
+ *
+ * For non-vector:
+ *   size: 00-> byte, 01 -> 16 bit, 10 -> 32bit, 11 -> 64bit
+ *   opc: 00 -> store, 01 -> loadu, 10 -> loads 64, 11 -> loads 32
+ * For vector:
+ *   size is opc<1>:size<1:0> so 100 -> 128 bit; 110 and 111 unallocated
+ *   opc<0>: 0 -> store, 1 -> load
+ * V: 1 -> vector/simd
+ * opt: extend encoding (see DecodeRegExtend)
+ * S: if S=1 then scale (essentially index by sizeof(size))
+ * Rt: register to transfer into/out of
+ * Rn: address register or SP for base
+ * Rm: offset register or ZR for offset
+ */
+static void handle_ldst_reg_roffset(DisasContext *s, uint32_t insn)
+{
+int rt = extract32(insn, 0, 5);
+int rn = extract32(insn, 5, 5);
+int shift = extract32(insn, 12, 1);
+int rm = extract32(insn, 16, 5);
+int opc = extract32(insn, 22, 2);
+int opt = extract32(insn, 13, 3);
+int size = extract32(insn, 30, 2);
+bool is_signed = false;
+bool is_store = false;
+bool is_extended = false;
+bool is_vector = extract32(insn, 26, 1);
+
+TCGv_i64 tcg_rm;
+TCGv_i64 tcg_addr;
+
+if (extract32(opt, 1, 1) == 0) {
+unallocated_encoding(s);
+return;
+}
+
+if (is_vector) {
+size |= (opc & 2) << 1;
+if (size > 4) {
+unallocated_encoding(s);
+return;
+}
+is_store = ((opc & 1) == 0);
+} else {
+if (size == 3 && opc == 2) {
+/* PRFM - prefetch */
+return;
+}
+if (opc == 3 && size > 1) {
+unallocated_encoding(s);
+return;
+}
+is_store = (opc == 0);
+is_signed = opc & (1<<1);
+is_extended = (size < 3) && (opc & 1);
+}
+
+if (rn == 31) {
+gen_check_sp_alignment(s);
+}
+tcg_addr = read_cpu_reg_sp(s, rn, 1);
+
+tcg_rm = read_cpu_reg(s, rm, 1);
+ext_and_shift_reg(tcg_rm, tcg_rm, opt, shift ? size : 0);
+
+tcg_gen_add_i64(tcg_addr, tcg_addr, tcg_rm);
+
+if (is_vector) {
+if (is_store) {
+do_fp_st(s, rt, tcg_addr, size);
+} else {
+do_fp_ld(s, rt, tcg_addr, size);
+}
+} else {
+TCGv_i64 tcg_rt = cpu_reg(s, rt);
+if (is_store) {
+do_gpr_st(s, tcg_rt, tcg_addr, size);
+} else {
+do_gpr_ld(s, tcg_rt, tcg_addr, size, is_signed, is_extended);
+}
+}
+}
+
+/*
  * C3.3.13 Load/store (unsigned immediate)
  *
  * 31 30 29   27  26 25 24 23 22 2110 9 5
@@ -966,7 +1104,11 @@ static void disas_ldst_

[Qemu-devel] [PATCH v2 2/8] target-arm: A64: add support for ld/st unsigned imm

2013-12-11 Thread Peter Maydell
From: Alex Bennée 

This adds support for the forms of ld/st with a 12 bit
unsigned immediate offset.

Signed-off-by: Alex Bennée 
Signed-off-by: Peter Maydell 

---

Changes
   - split off cpu_read_reg_sp for earlier use
---
 target-arm/translate-a64.c | 89 +-
 1 file changed, 88 insertions(+), 1 deletion(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 018b3ee..7d3f432 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -884,10 +884,97 @@ static void handle_ldst_pair(DisasContext *s, uint32_t 
insn)
 }
 }
 
+/*
+ * C3.3.13 Load/store (unsigned immediate)
+ *
+ * 31 30 29   27  26 25 24 23 22 2110 9 5
+ * ++---+---+-+-++---+--+
+ * |size| 1 1 1 | V | 0 1 | opc |   imm12|  Rn   |  Rt  |
+ * ++---+---+-+-++---+--+
+ *
+ * For non-vector:
+ *   size: 00-> byte, 01 -> 16 bit, 10 -> 32bit, 11 -> 64bit
+ *   opc: 00 -> store, 01 -> loadu, 10 -> loads 64, 11 -> loads 32
+ * For vector:
+ *   size is opc<1>:size<1:0> so 100 -> 128 bit; 110 and 111 unallocated
+ *   opc<0>: 0 -> store, 1 -> load
+ * Rn: base address register (inc SP)
+ * Rt: target register
+ */
+static void handle_ldst_reg_unsigned_imm(DisasContext *s, uint32_t insn)
+{
+int rt = extract32(insn, 0, 5);
+int rn = extract32(insn, 5, 5);
+unsigned int imm12 = extract32(insn, 10, 12);
+bool is_vector = extract32(insn, 26, 1);
+int size = extract32(insn, 30, 2);
+int opc = extract32(insn, 22, 2);
+unsigned int offset;
+
+TCGv_i64 tcg_addr;
+
+bool is_store;
+bool is_signed = false;
+bool is_extended = false;
+
+if (is_vector) {
+size |= (opc & 2) << 1;
+if (size > 4) {
+unallocated_encoding(s);
+return;
+}
+is_store = ((opc & 1) == 0);
+} else {
+if (size == 3 && opc == 2) {
+/* PRFM - prefetch */
+return;
+}
+if (opc == 3 && size > 1) {
+unallocated_encoding(s);
+return;
+}
+is_store = (opc == 0);
+is_signed = opc & (1<<1);
+is_extended = (size < 3) && (opc & 1);
+}
+
+if (rn == 31) {
+gen_check_sp_alignment(s);
+}
+tcg_addr = read_cpu_reg_sp(s, rn, 1);
+offset = imm12 << size;
+tcg_gen_addi_i64(tcg_addr, tcg_addr, offset);
+
+if (is_vector) {
+if (is_store) {
+do_fp_st(s, rt, tcg_addr, size);
+} else {
+do_fp_ld(s, rt, tcg_addr, size);
+}
+} else {
+TCGv_i64 tcg_rt = cpu_reg(s, rt);
+if (is_store) {
+do_gpr_st(s, tcg_rt, tcg_addr, size);
+} else {
+do_gpr_ld(s, tcg_rt, tcg_addr, size, is_signed, is_extended);
+}
+}
+}
+
 /* Load/store register (all forms) */
 static void disas_ldst_reg(DisasContext *s, uint32_t insn)
 {
-unsupported_encoding(s, insn);
+switch (extract32(insn, 24, 2)) {
+case 0:
+unsupported_encoding(s, insn);
+break;
+case 1:
+handle_ldst_reg_unsigned_imm(s, insn);
+break;
+default:
+unallocated_encoding(s);
+break;
+}
 }
 
 /* AdvSIMD load/store multiple structures */
-- 
1.8.5




[Qemu-devel] [PATCH v2 7/8] target-arm: A64: add support for 3 src data proc insns

2013-12-11 Thread Peter Maydell
From: Alexander Graf 

This patch adds emulation for the "Data-processing (3 source)"
family of instructions, namely MADD, MSUB, SMADDL, SMSUBL, SMULH,
UMADDL, UMSUBL, UMULH.

Signed-off-by: Alexander Graf 
Signed-off-by: Alex Bennée 
Signed-off-by: Peter Maydell 

---
Changes:
  - Use tcg ops for mul[su]h (rh comment)
---
 target-arm/translate-a64.c | 91 +-
 1 file changed, 89 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 4a6252e..019a6ed 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -2110,10 +2110,97 @@ static void handle_add_sub_reg(DisasContext *s, 
uint32_t insn)
 tcg_temp_free_i64(tcg_result);
 }
 
-/* Data-processing (3 source) */
+/* C3.5.9 Data-processing (3 source)
+
+   31 30  29 28   24 23 21  20  16  15  14  10 95 40
+  +--+--+---+--+--++--+--+--+
+  |sf| op54 | 1 1 0 1 1 | op31 |  Rm  | o0 |  Ra  |  Rn  |  Rd  |
+  +--+--+---+--+--++--+--+--+
+
+ */
 static void disas_data_proc_3src(DisasContext *s, uint32_t insn)
 {
-unsupported_encoding(s, insn);
+int rd = extract32(insn, 0, 5);
+int rn = extract32(insn, 5, 5);
+int ra = extract32(insn, 10, 5);
+int rm = extract32(insn, 16, 5);
+int op_id = (extract32(insn, 29, 3) << 4) |
+(extract32(insn, 21, 3) << 1) |
+extract32(insn, 15, 1);
+bool is_32bit = !extract32(insn, 31, 1);
+bool is_sub = extract32(op_id, 0, 1);
+bool is_high = extract32(op_id, 2, 1);
+bool is_signed = false;
+TCGv_i64 tcg_op1;
+TCGv_i64 tcg_op2;
+TCGv_i64 tcg_tmp;
+
+/* Note that op_id is sf:op54:op31:o0 so it includes the 32/64 size flag */
+switch (op_id) {
+case 0x42: /* SMADDL */
+case 0x43: /* SMSUBL */
+case 0x44: /* SMULH */
+is_signed = true;
+break;
+case 0x0: /* MADD (32bit) */
+case 0x1: /* MSUB (32bit) */
+case 0x40: /* MADD (64bit) */
+case 0x41: /* MSUB (64bit) */
+case 0x4a: /* UMADDL */
+case 0x4b: /* UMSUBL */
+case 0x4c: /* UMULH */
+break;
+default:
+unallocated_encoding(s);
+}
+
+if (is_high) {
+TCGv_i64 low_bits = tcg_temp_new_i64(); /* low bits discarded */
+TCGv_i64 tcg_rd = cpu_reg(s, rd);
+TCGv_i64 tcg_rn = cpu_reg(s, rn);
+TCGv_i64 tcg_rm = cpu_reg(s, rm);
+
+if (is_signed) {
+tcg_gen_muls2_i64(low_bits, tcg_rd, tcg_rn, tcg_rm);
+} else {
+tcg_gen_mulu2_i64(low_bits, tcg_rd, tcg_rn, tcg_rm);
+}
+
+tcg_temp_free(low_bits);
+return;
+}
+
+tcg_op1 = tcg_temp_new_i64();
+tcg_op2 = tcg_temp_new_i64();
+tcg_tmp = tcg_temp_new_i64();
+
+if (op_id < 0x42) {
+tcg_gen_mov_i64(tcg_op1, cpu_reg(s, rn));
+tcg_gen_mov_i64(tcg_op2, cpu_reg(s, rm));
+} else {
+if (is_signed) {
+tcg_gen_ext32s_i64(tcg_op1, cpu_reg(s, rn));
+tcg_gen_ext32s_i64(tcg_op2, cpu_reg(s, rm));
+} else {
+tcg_gen_ext32u_i64(tcg_op1, cpu_reg(s, rn));
+tcg_gen_ext32u_i64(tcg_op2, cpu_reg(s, rm));
+}
+}
+
+tcg_gen_mul_i64(tcg_tmp, tcg_op1, tcg_op2);
+if (is_sub) {
+tcg_gen_sub_i64(cpu_reg(s, rd), cpu_reg(s, ra), tcg_tmp);
+} else {
+tcg_gen_add_i64(cpu_reg(s, rd), cpu_reg(s, ra), tcg_tmp);
+}
+
+if (is_32bit) {
+tcg_gen_ext32u_i64(cpu_reg(s, rd), cpu_reg(s, rd));
+}
+
+tcg_temp_free_i64(tcg_op1);
+tcg_temp_free_i64(tcg_op2);
+tcg_temp_free_i64(tcg_tmp);
 }
 
 /* Add/subtract (with carry) */
-- 
1.8.5




Re: [Qemu-devel] [PATCH 4/9] target-arm: A64: add support for ld/st with reg offset

2013-12-11 Thread Alex Bennée

r...@twiddle.net writes:

> On 12/10/2013 06:16 AM, Alex Bennée wrote:
>> However my preference unless there is a strong objection would be to
>> clean that up in later patches. For one thing the more instructions each
>> patch handles the longer it takes to run the instruction validation on
>> the rather slow models to have good coverage of the decoder!
>
> That'd be ok by me.

I had a play trying to see what pulling out the common parts of decode
and unallocated handling based on the ARM ARM pseudo-code into a
separate function would look like. Unfortunately what I ended up
with was a horrible function full of pass-by-reference parameters and a
not particularly cleaner or shorter call-sites in each handler.

I did briefly consider if I could construct a macro which would make for
less duplicated code but suspect that won't help in the long run.

This is certainly something I think that requires more thought. In the
meantime I've addresses your other review comments and Peter should be
pushing a new set of patches soon.


Cheers,

--
Alex Bennée
QEMU/KVM Hacker for Linaro




[Qemu-devel] [PATCH 14/22] block: Switch BdrvTrackedRequest to byte granularity

2013-12-11 Thread Kevin Wolf
Signed-off-by: Kevin Wolf 
---
 block.c   | 52 +++
 block/backup.c|  7 ++-
 include/block/block_int.h |  4 ++--
 3 files changed, 42 insertions(+), 21 deletions(-)

diff --git a/block.c b/block.c
index a80db2e..fa888d9 100644
--- a/block.c
+++ b/block.c
@@ -2037,13 +2037,13 @@ static void tracked_request_end(BdrvTrackedRequest *req)
  */
 static void tracked_request_begin(BdrvTrackedRequest *req,
   BlockDriverState *bs,
-  int64_t sector_num,
-  int nb_sectors, bool is_write)
+  int64_t offset,
+  unsigned int bytes, bool is_write)
 {
 *req = (BdrvTrackedRequest){
 .bs = bs,
-.sector_num = sector_num,
-.nb_sectors = nb_sectors,
+.offset = offset,
+.bytes = bytes,
 .is_write = is_write,
 .co = qemu_coroutine_self(),
 };
@@ -2074,25 +2074,43 @@ void bdrv_round_to_clusters(BlockDriverState *bs,
 }
 }
 
+static void round_bytes_to_clusters(BlockDriverState *bs,
+int64_t offset, unsigned int bytes,
+int64_t *cluster_offset,
+unsigned int *cluster_bytes)
+{
+BlockDriverInfo bdi;
+
+if (bdrv_get_info(bs, &bdi) < 0 || bdi.cluster_size == 0) {
+*cluster_offset = offset;
+*cluster_bytes = bytes;
+} else {
+*cluster_offset = QEMU_ALIGN_DOWN(offset, bdi.cluster_size);
+*cluster_bytes = QEMU_ALIGN_UP(offset - *cluster_offset + bytes,
+   bdi.cluster_size);
+}
+}
+
 static bool tracked_request_overlaps(BdrvTrackedRequest *req,
- int64_t sector_num, int nb_sectors) {
+ int64_t offset, int bytes)
+{
 /*    */
-if (sector_num >= req->sector_num + req->nb_sectors) {
+if (offset >= req->offset + req->bytes) {
 return false;
 }
 /*    */
-if (req->sector_num >= sector_num + nb_sectors) {
+if (req->offset >= offset + bytes) {
 return false;
 }
 return true;
 }
 
 static void coroutine_fn wait_for_overlapping_requests(BlockDriverState *bs,
-int64_t sector_num, int nb_sectors)
+int64_t offset, unsigned int bytes)
 {
 BdrvTrackedRequest *req;
-int64_t cluster_sector_num;
-int cluster_nb_sectors;
+int64_t cluster_offset;
+unsigned int cluster_bytes;
 bool retry;
 
 /* If we touch the same cluster it counts as an overlap.  This guarantees
@@ -2101,14 +2119,12 @@ static void coroutine_fn 
wait_for_overlapping_requests(BlockDriverState *bs,
  * CoR read and write operations are atomic and guest writes cannot
  * interleave between them.
  */
-bdrv_round_to_clusters(bs, sector_num, nb_sectors,
-   &cluster_sector_num, &cluster_nb_sectors);
+round_bytes_to_clusters(bs, offset, bytes, &cluster_offset, 
&cluster_bytes);
 
 do {
 retry = false;
 QLIST_FOREACH(req, &bs->tracked_requests, list) {
-if (tracked_request_overlaps(req, cluster_sector_num,
- cluster_nb_sectors)) {
+if (tracked_request_overlaps(req, cluster_offset, cluster_bytes)) {
 /* Hitting this means there was a reentrant request, for
  * example, a block driver issuing nested requests.  This must
  * never happen since it means deadlock.
@@ -2723,10 +2739,10 @@ static int coroutine_fn 
bdrv_aligned_preadv(BlockDriverState *bs,
 }
 
 if (bs->copy_on_read_in_flight) {
-wait_for_overlapping_requests(bs, sector_num, nb_sectors);
+wait_for_overlapping_requests(bs, offset, bytes);
 }
 
-tracked_request_begin(&req, bs, sector_num, nb_sectors, false);
+tracked_request_begin(&req, bs, offset, bytes, false);
 
 if (flags & BDRV_REQ_COPY_ON_READ) {
 int pnum;
@@ -2974,10 +2990,10 @@ static int coroutine_fn 
bdrv_aligned_pwritev(BlockDriverState *bs,
 assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
 
 if (bs->copy_on_read_in_flight) {
-wait_for_overlapping_requests(bs, sector_num, nb_sectors);
+wait_for_overlapping_requests(bs, offset, bytes);
 }
 
-tracked_request_begin(&req, bs, sector_num, nb_sectors, true);
+tracked_request_begin(&req, bs, offset, bytes, true);
 
 ret = notifier_with_return_list_notify(&bs->before_write_notifiers, &req);
 
diff --git a/block/backup.c b/block/backup.c
index 0198514..15a2e55 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -181,8 +181,13 @@ static int coroutine_fn backup_before_write_notify(
 void *opaque)
 {
 BdrvTrackedRequest *req = opaque;
+int64_t sector_num = req->offset >> BDRV_SECTOR

Re: [Qemu-devel] [SeaBIOS] [BUG] Windows 7 fails to start with new vgabios binaries

2013-12-11 Thread Kevin O'Connor
On Tue, Dec 10, 2013 at 04:55:36AM -0500, Gal Hammer wrote:
> Kevin,
> 
> The patch was verified and it solves the problem on Windows 7 official 
> release as well.

Thanks.

I pushed this patch to seabios git master.

-Kevin


> 
> - Original Message -
> From: "Kevin O'Connor" 
> To: "Gerd Hoffmann" 
> Cc: "Gal Hammer" , "seabios" , 
> qemu-devel@nongnu.org, "Julian Pidancet" 
> Sent: Tuesday, December 10, 2013 12:12:51 AM
> Subject: Re: [SeaBIOS] [BUG] Windows 7 fails to start with new vgabios 
> binaries
> 
> On Mon, Dec 09, 2013 at 10:57:25AM -0500, Kevin O'Connor wrote:
> > On Mon, Dec 09, 2013 at 02:20:59PM +0100, Gerd Hoffmann wrote:
> > > On Mo, 2013-12-09 at 14:18 +0200, Gal Hammer wrote:
> > > > A Windows 7 (32-bit) VM running with QXL device fails to start with the 
> > > > new updated vgabios binaries (commit 
> > > Tracked down to the new vgabios stack switching.  With
> > > CONFIG_VGA_ALLOCATE_EXTRA_STACK=n everything is fine again.
> > 
> > Ughh.  WinXP doesn't have the problem, but I can also reproduce on
> > Win7 beta.
> > 
> > I'll change the default for CONFIG_VGA_ALLOCATE_EXTRA_STACK to off.
> 
> I tracked this down further.  The problem is the Windows x86 emulator
> doesn't correctly handle memory accesses relative to the %esp
> register.  Julian reported this some time back and we worked around it
> then.  However, the recent "extra stack" assembler code inserted a few
> of these instructions.
> 
> I think the proper fix is to leave CONFIG_VGA_ALLOCATE_EXTRA_STACK on
> and use slightly different assembler so as not to aggravate win7.
> 
> I tested the seabios patch below and my win7 beta now boots okay.
> 
> -Kevin
> 
> 
> --- a/vgasrc/vgaentry.S
> +++ b/vgasrc/vgaentry.S
> @@ -97,12 +97,9 @@ entry_10_extrastack:
>  movl %ecx, BREGS_ecx(%eax)
>  movw %es, BREGS_es(%eax)
>  movl %esp, BREGS_size+0(%eax)
> -movzwl %sp, %esp
>  movw %ss, BREGS_size+4(%eax)
> -movl (%esp), %edx
> -movl %edx, BREGS_code(%eax)
> -movw 4(%esp), %dx
> -movw %dx, BREGS_flags(%eax)
> +popl BREGS_code(%eax)
> +popw BREGS_flags(%eax)
>  
>  movw %ds, %dx   // Setup %ss/%esp and call function
>  movw %dx, %ss



[Qemu-devel] [PATCH 15/22] block: Allow waiting for overlapping requests between begin/end

2013-12-11 Thread Kevin Wolf
Previously, it was not possible to use wait_for_overlapping_requests()
between tracked_request_begin()/end() because it would wait for itself.

Ignore the current request in the overlap check and run more of the
bdrv_co_do_preadv/pwritev code with a BdrvTrackedRequest present.

Signed-off-by: Kevin Wolf 
---
 block.c | 38 --
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/block.c b/block.c
index fa888d9..b4f6ead 100644
--- a/block.c
+++ b/block.c
@@ -2106,7 +2106,7 @@ static bool tracked_request_overlaps(BdrvTrackedRequest 
*req,
 }
 
 static void coroutine_fn wait_for_overlapping_requests(BlockDriverState *bs,
-int64_t offset, unsigned int bytes)
+BdrvTrackedRequest *self, int64_t offset, unsigned int bytes)
 {
 BdrvTrackedRequest *req;
 int64_t cluster_offset;
@@ -2124,6 +2124,9 @@ static void coroutine_fn 
wait_for_overlapping_requests(BlockDriverState *bs,
 do {
 retry = false;
 QLIST_FOREACH(req, &bs->tracked_requests, list) {
+if (req == self) {
+continue;
+}
 if (tracked_request_overlaps(req, cluster_offset, cluster_bytes)) {
 /* Hitting this means there was a reentrant request, for
  * example, a block driver issuing nested requests.  This must
@@ -2721,10 +2724,10 @@ err:
  * implemented by the caller.
  */
 static int coroutine_fn bdrv_aligned_preadv(BlockDriverState *bs,
-int64_t offset, unsigned int bytes, QEMUIOVector *qiov, int flags)
+BdrvTrackedRequest *req, int64_t offset, unsigned int bytes,
+QEMUIOVector *qiov, int flags)
 {
 BlockDriver *drv = bs->drv;
-BdrvTrackedRequest req;
 int ret;
 
 int64_t sector_num = offset >> BDRV_SECTOR_BITS;
@@ -2739,11 +2742,9 @@ static int coroutine_fn 
bdrv_aligned_preadv(BlockDriverState *bs,
 }
 
 if (bs->copy_on_read_in_flight) {
-wait_for_overlapping_requests(bs, offset, bytes);
+wait_for_overlapping_requests(bs, req, offset, bytes);
 }
 
-tracked_request_begin(&req, bs, offset, bytes, false);
-
 if (flags & BDRV_REQ_COPY_ON_READ) {
 int pnum;
 
@@ -2790,8 +2791,6 @@ static int coroutine_fn 
bdrv_aligned_preadv(BlockDriverState *bs,
 }
 
 out:
-tracked_request_end(&req);
-
 if (flags & BDRV_REQ_COPY_ON_READ) {
 bs->copy_on_read_in_flight--;
 }
@@ -2807,6 +2806,8 @@ static int coroutine_fn 
bdrv_co_do_preadv(BlockDriverState *bs,
 BdrvRequestFlags flags)
 {
 BlockDriver *drv = bs->drv;
+BdrvTrackedRequest req;
+
 /* TODO Lift BDRV_SECTOR_SIZE restriction in BlockDriver interface */
 uint64_t align = MAX(BDRV_SECTOR_SIZE, bs->request_alignment);
 uint8_t *head_buf = NULL;
@@ -2856,9 +2857,11 @@ static int coroutine_fn 
bdrv_co_do_preadv(BlockDriverState *bs,
 bytes = ROUND_UP(bytes, align);
 }
 
-ret = bdrv_aligned_preadv(bs, offset, bytes,
+tracked_request_begin(&req, bs, offset, bytes, false);
+ret = bdrv_aligned_preadv(bs, &req, offset, bytes,
   use_local_qiov ? &local_qiov : qiov,
   flags);
+tracked_request_end(&req);
 
 if (use_local_qiov) {
 qemu_iovec_destroy(&local_qiov);
@@ -2977,10 +2980,10 @@ static int coroutine_fn 
bdrv_co_do_write_zeroes(BlockDriverState *bs,
  * Forwards an already correctly aligned write request to the BlockDriver.
  */
 static int coroutine_fn bdrv_aligned_pwritev(BlockDriverState *bs,
-int64_t offset, unsigned int bytes, QEMUIOVector *qiov, int flags)
+BdrvTrackedRequest *req, int64_t offset, unsigned int bytes,
+QEMUIOVector *qiov, int flags)
 {
 BlockDriver *drv = bs->drv;
-BdrvTrackedRequest req;
 int ret;
 
 int64_t sector_num = offset >> BDRV_SECTOR_BITS;
@@ -2990,12 +2993,10 @@ static int coroutine_fn 
bdrv_aligned_pwritev(BlockDriverState *bs,
 assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
 
 if (bs->copy_on_read_in_flight) {
-wait_for_overlapping_requests(bs, offset, bytes);
+wait_for_overlapping_requests(bs, req, offset, bytes);
 }
 
-tracked_request_begin(&req, bs, offset, bytes, true);
-
-ret = notifier_with_return_list_notify(&bs->before_write_notifiers, &req);
+ret = notifier_with_return_list_notify(&bs->before_write_notifiers, req);
 
 if (ret < 0) {
 /* Do nothing, write notifier decided to fail this request */
@@ -3018,8 +3019,6 @@ static int coroutine_fn 
bdrv_aligned_pwritev(BlockDriverState *bs,
 bs->total_sectors = MAX(bs->total_sectors, sector_num + nb_sectors);
 }
 
-tracked_request_end(&req);
-
 return ret;
 }
 
@@ -3030,6 +3029,7 @@ static int coroutine_fn 
bdrv_co_do_pwritev(BlockDriverState *bs,
 int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
 BdrvRequestFlags flags)
 {
+BdrvTrackedRequest req;
 int ret;
 
 if (!bs->drv) {
@@ -3047,7 +3047,9 @@ static int coroutine_fn 
bdrv

Re: [Qemu-devel] [RFC PATCH 04/14] NUMA: convert -numa option to use OptsVisitor

2013-12-11 Thread Paolo Bonzini
Il 11/12/2013 19:51, Eric Blake ha scritto:
>> > +{ 'union': 'NumaOptions',
>> > +  'data': {
>> > +'node': 'NumaNodeOptions' }}
> Why do we need a union, if there's no alternative, and since nothing
> else in the series adds an alternative?

Because these structures are used to parse command-line options and
follow somewhat special rules.  The "node" in "-numa node,..." is
present for forwards-extensibility.  Another alternative, "mem", was
added in previous versions of the NUMA policy patches but is made
obsolete by Igor's memory object.

>> > +
>> > +##
>> > +# @NumaNodeOptions
>> > +#
>> > +# Create a guest NUMA node. (for OptsVisitor)
>> > +#
>> > +# @nodeid: #optional NUMA node ID
>> > +#
>> > +# @cpus: #optional VCPUs belong to this node
> What are the defaults if these fields are omitted?

Wanlong, can you fill this out when you resubmit?

>> > +#
>> > +# @mem: #optional memory size of this node
> In bytes?  Why is this field a string instead of an integer?

It was like this because it is a command-line option and thus is never
used via QMP.  However, it can and should indeed be a 'size', handled
like Igor did for '-m mem=' in patch 8 of the series.

Paolo

>> > +#
>> > +# Since: 2.0
>> > +##
>> > +{ 'type': 'NumaNodeOptions',
>> > +  'data': {
>> > +   '*nodeid': 'uint16',
>> > +   '*cpus':   ['uint16'],
>> > +   '*mem':'str' }}




[Qemu-devel] [PATCH 17/22] block: Generalise and optimise COR serialisation

2013-12-11 Thread Kevin Wolf
Change the API so that specific requests can be marked serialising. Only
these requests are checked for overlaps then.

This means that during a Copy on Read operation, not all requests
overlapping other requests are serialised any more, but only those that
actually overlap with the specific COR request.

Also remove COR from function and variable names because this
functionality can be useful in other contexts.

Signed-off-by: Kevin Wolf 
---
 block.c   | 40 +---
 include/block/block_int.h |  5 +++--
 2 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/block.c b/block.c
index 6dddb7c..40be327 100644
--- a/block.c
+++ b/block.c
@@ -2028,6 +2028,10 @@ int bdrv_commit_all(void)
  */
 static void tracked_request_end(BdrvTrackedRequest *req)
 {
+if (req->serialising) {
+req->bs->serialising_in_flight--;
+}
+
 QLIST_REMOVE(req, list);
 qemu_co_queue_restart_all(&req->wait_queue);
 }
@@ -2046,6 +2050,7 @@ static void tracked_request_begin(BdrvTrackedRequest *req,
 .bytes = bytes,
 .is_write = is_write,
 .co = qemu_coroutine_self(),
+.serialising = false,
 };
 
 qemu_co_queue_init(&req->wait_queue);
@@ -2053,6 +2058,14 @@ static void tracked_request_begin(BdrvTrackedRequest 
*req,
 QLIST_INSERT_HEAD(&bs->tracked_requests, req, list);
 }
 
+static void mark_request_serialising(BdrvTrackedRequest *req)
+{
+if (!req->serialising) {
+req->bs->serialising_in_flight++;
+req->serialising = true;
+}
+}
+
 /**
  * Round a region to cluster boundaries
  */
@@ -2105,26 +2118,31 @@ static bool tracked_request_overlaps(BdrvTrackedRequest 
*req,
 return true;
 }
 
-static void coroutine_fn wait_for_overlapping_requests(BlockDriverState *bs,
-BdrvTrackedRequest *self, int64_t offset, unsigned int bytes)
+static void coroutine_fn wait_serialising_requests(BdrvTrackedRequest *self)
 {
+BlockDriverState *bs = self->bs;
 BdrvTrackedRequest *req;
 int64_t cluster_offset;
 unsigned int cluster_bytes;
 bool retry;
 
+if (!bs->serialising_in_flight) {
+return;
+}
+
 /* If we touch the same cluster it counts as an overlap.  This guarantees
  * that allocating writes will be serialized and not race with each other
  * for the same cluster.  For example, in copy-on-read it ensures that the
  * CoR read and write operations are atomic and guest writes cannot
  * interleave between them.
  */
-round_bytes_to_clusters(bs, offset, bytes, &cluster_offset, 
&cluster_bytes);
+round_bytes_to_clusters(bs, self->offset, self->bytes,
+&cluster_offset, &cluster_bytes);
 
 do {
 retry = false;
 QLIST_FOREACH(req, &bs->tracked_requests, list) {
-if (req == self) {
+if (req == self || (!req->serialising && !self->serialising)) {
 continue;
 }
 if (tracked_request_overlaps(req, cluster_offset, cluster_bytes)) {
@@ -2738,12 +2756,10 @@ static int coroutine_fn 
bdrv_aligned_preadv(BlockDriverState *bs,
 
 /* Handle Copy on Read and associated serialisation */
 if (flags & BDRV_REQ_COPY_ON_READ) {
-bs->copy_on_read_in_flight++;
+mark_request_serialising(req);
 }
 
-if (bs->copy_on_read_in_flight) {
-wait_for_overlapping_requests(bs, req, offset, bytes);
-}
+wait_serialising_requests(req);
 
 if (flags & BDRV_REQ_COPY_ON_READ) {
 int pnum;
@@ -2791,10 +2807,6 @@ static int coroutine_fn 
bdrv_aligned_preadv(BlockDriverState *bs,
 }
 
 out:
-if (flags & BDRV_REQ_COPY_ON_READ) {
-bs->copy_on_read_in_flight--;
-}
-
 return ret;
 }
 
@@ -2992,9 +3004,7 @@ static int coroutine_fn 
bdrv_aligned_pwritev(BlockDriverState *bs,
 assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
 assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
 
-if (bs->copy_on_read_in_flight) {
-wait_for_overlapping_requests(bs, req, offset, bytes);
-}
+wait_serialising_requests(req);
 
 ret = notifier_with_return_list_notify(&bs->before_write_notifiers, req);
 
diff --git a/include/block/block_int.h b/include/block/block_int.h
index a11e5c9..b00402b 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -60,6 +60,7 @@ typedef struct BdrvTrackedRequest {
 int64_t offset;
 unsigned int bytes;
 bool is_write;
+bool serialising;
 QLIST_ENTRY(BdrvTrackedRequest) list;
 Coroutine *co; /* owner, used for deadlock detection */
 CoQueue wait_queue; /* coroutines blocked on this request */
@@ -296,8 +297,8 @@ struct BlockDriverState {
 /* Callback before write request is processed */
 NotifierWithReturnList before_write_notifiers;
 
-/* number of in-flight copy-on-read requests */
-unsigned int copy_on_read_in_flight;
+/* number of in-flight serialising requests */
+unsigned int serialis

[Qemu-devel] [PATCH 18/22] block: Make overlap range for serialisation dynamic

2013-12-11 Thread Kevin Wolf
Copy on Read wants to serialise with all requests touching the same
cluster, so wait_serialising_requests() rounded to cluster boundaries.
Other users like alignment RMW will have different requirements, though
(requests touching the same sector), so make it dynamic.

Signed-off-by: Kevin Wolf 
---
 block.c   | 51 +++
 include/block/block_int.h |  4 
 2 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/block.c b/block.c
index 40be327..6630b41 100644
--- a/block.c
+++ b/block.c
@@ -2058,12 +2058,19 @@ static void tracked_request_begin(BdrvTrackedRequest 
*req,
 QLIST_INSERT_HEAD(&bs->tracked_requests, req, list);
 }
 
-static void mark_request_serialising(BdrvTrackedRequest *req)
+static void mark_request_serialising(BdrvTrackedRequest *req, size_t align)
 {
+int64_t overlap_offset = req->offset & ~(align - 1);
+int overlap_bytes = ROUND_UP(req->offset + req->bytes, align)
+  - req->overlap_offset;
+
 if (!req->serialising) {
 req->bs->serialising_in_flight++;
 req->serialising = true;
 }
+
+req->overlap_offset = MIN(req->overlap_offset, overlap_offset);
+req->overlap_bytes = MAX(req->overlap_bytes, overlap_bytes);
 }
 
 /**
@@ -2087,20 +2094,16 @@ void bdrv_round_to_clusters(BlockDriverState *bs,
 }
 }
 
-static void round_bytes_to_clusters(BlockDriverState *bs,
-int64_t offset, unsigned int bytes,
-int64_t *cluster_offset,
-unsigned int *cluster_bytes)
+static int bdrv_get_cluster_size(BlockDriverState *bs)
 {
 BlockDriverInfo bdi;
+int ret;
 
-if (bdrv_get_info(bs, &bdi) < 0 || bdi.cluster_size == 0) {
-*cluster_offset = offset;
-*cluster_bytes = bytes;
+ret = bdrv_get_info(bs, &bdi);
+if (ret < 0 || bdi.cluster_size == 0) {
+return bs->request_alignment;
 } else {
-*cluster_offset = QEMU_ALIGN_DOWN(offset, bdi.cluster_size);
-*cluster_bytes = QEMU_ALIGN_UP(offset - *cluster_offset + bytes,
-   bdi.cluster_size);
+return bdi.cluster_size;
 }
 }
 
@@ -2108,11 +2111,11 @@ static bool tracked_request_overlaps(BdrvTrackedRequest 
*req,
  int64_t offset, int bytes)
 {
 /*    */
-if (offset >= req->offset + req->bytes) {
+if (offset >= req->overlap_offset + req->overlap_bytes) {
 return false;
 }
 /*    */
-if (req->offset >= offset + bytes) {
+if (req->overlap_offset >= offset + bytes) {
 return false;
 }
 return true;
@@ -2122,30 +2125,21 @@ static void coroutine_fn 
wait_serialising_requests(BdrvTrackedRequest *self)
 {
 BlockDriverState *bs = self->bs;
 BdrvTrackedRequest *req;
-int64_t cluster_offset;
-unsigned int cluster_bytes;
 bool retry;
 
 if (!bs->serialising_in_flight) {
 return;
 }
 
-/* If we touch the same cluster it counts as an overlap.  This guarantees
- * that allocating writes will be serialized and not race with each other
- * for the same cluster.  For example, in copy-on-read it ensures that the
- * CoR read and write operations are atomic and guest writes cannot
- * interleave between them.
- */
-round_bytes_to_clusters(bs, self->offset, self->bytes,
-&cluster_offset, &cluster_bytes);
-
 do {
 retry = false;
 QLIST_FOREACH(req, &bs->tracked_requests, list) {
 if (req == self || (!req->serialising && !self->serialising)) {
 continue;
 }
-if (tracked_request_overlaps(req, cluster_offset, cluster_bytes)) {
+if (tracked_request_overlaps(req, self->overlap_offset,
+ self->overlap_bytes))
+{
 /* Hitting this means there was a reentrant request, for
  * example, a block driver issuing nested requests.  This must
  * never happen since it means deadlock.
@@ -2756,7 +2750,12 @@ static int coroutine_fn 
bdrv_aligned_preadv(BlockDriverState *bs,
 
 /* Handle Copy on Read and associated serialisation */
 if (flags & BDRV_REQ_COPY_ON_READ) {
-mark_request_serialising(req);
+/* If we touch the same cluster it counts as an overlap.  This
+ * guarantees that allocating writes will be serialized and not race
+ * with each other for the same cluster.  For example, in copy-on-read
+ * it ensures that the CoR read and write operations are atomic and
+ * guest writes cannot interleave between them. */
+mark_request_serialising(req, bdrv_get_cluster_size(bs));
 }
 
 wait_serialising_requests(req);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index b00402b..1aee02b 1006

Re: [Qemu-devel] [V2 PATCH 02/18] target-ppc: Add ISA2.06 bpermd Instruction

2013-12-11 Thread Richard Henderson
On 12/11/2013 11:16 AM, Tom Musta wrote:
> This patch adds the Bit Permute Doubleword (bpermd) instruction,
> which was introduced in Power ISA 2.06 as part of the base 64-bit
> architecture.
> 
> V2: Addressing stylistic comments from Richard Henderson.
> 
> Signed-off-by: Tom Musta 
> ---
>  target-ppc/helper.h |1 +
>  target-ppc/int_helper.c |   20 
>  target-ppc/translate.c  |   10 ++
>  3 files changed, 31 insertions(+), 0 deletions(-)

Reviewed-by: Richard Henderson 


r~



[Qemu-devel] [PATCH 20/22] block: Change coroutine wrapper to byte granularity

2013-12-11 Thread Kevin Wolf
Signed-off-by: Kevin Wolf 
---
 block.c | 48 ++--
 1 file changed, 26 insertions(+), 22 deletions(-)

diff --git a/block.c b/block.c
index 98ea723..5e2e092 100644
--- a/block.c
+++ b/block.c
@@ -69,11 +69,11 @@ static int coroutine_fn bdrv_co_readv_em(BlockDriverState 
*bs,
 static int coroutine_fn bdrv_co_writev_em(BlockDriverState *bs,
  int64_t sector_num, int nb_sectors,
  QEMUIOVector *iov);
-static int coroutine_fn bdrv_co_do_readv(BlockDriverState *bs,
-int64_t sector_num, int nb_sectors, QEMUIOVector *qiov,
+static int coroutine_fn bdrv_co_do_preadv(BlockDriverState *bs,
+int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
 BdrvRequestFlags flags);
-static int coroutine_fn bdrv_co_do_writev(BlockDriverState *bs,
-int64_t sector_num, int nb_sectors, QEMUIOVector *qiov,
+static int coroutine_fn bdrv_co_do_pwritev(BlockDriverState *bs,
+int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
 BdrvRequestFlags flags);
 static BlockDriverAIOCB *bdrv_co_aio_rw_vector(BlockDriverState *bs,
int64_t sector_num,
@@ -2361,8 +2361,7 @@ static int bdrv_check_request(BlockDriverState *bs, 
int64_t sector_num,
 
 typedef struct RwCo {
 BlockDriverState *bs;
-int64_t sector_num;
-int nb_sectors;
+int64_t offset;
 QEMUIOVector *qiov;
 bool is_write;
 int ret;
@@ -2374,34 +2373,32 @@ static void coroutine_fn bdrv_rw_co_entry(void *opaque)
 RwCo *rwco = opaque;
 
 if (!rwco->is_write) {
-rwco->ret = bdrv_co_do_readv(rwco->bs, rwco->sector_num,
- rwco->nb_sectors, rwco->qiov,
- rwco->flags);
-} else {
-rwco->ret = bdrv_co_do_writev(rwco->bs, rwco->sector_num,
-  rwco->nb_sectors, rwco->qiov,
+rwco->ret = bdrv_co_do_preadv(rwco->bs, rwco->offset,
+  rwco->qiov->size, rwco->qiov,
   rwco->flags);
+} else {
+rwco->ret = bdrv_co_do_pwritev(rwco->bs, rwco->offset,
+   rwco->qiov->size, rwco->qiov,
+   rwco->flags);
 }
 }
 
 /*
  * Process a vectored synchronous request using coroutines
  */
-static int bdrv_rwv_co(BlockDriverState *bs, int64_t sector_num,
-   QEMUIOVector *qiov, bool is_write,
-   BdrvRequestFlags flags)
+static int bdrv_prwv_co(BlockDriverState *bs, int64_t offset,
+QEMUIOVector *qiov, bool is_write,
+BdrvRequestFlags flags)
 {
 Coroutine *co;
 RwCo rwco = {
 .bs = bs,
-.sector_num = sector_num,
-.nb_sectors = qiov->size >> BDRV_SECTOR_BITS,
+.offset = offset,
 .qiov = qiov,
 .is_write = is_write,
 .ret = NOT_DONE,
 .flags = flags,
 };
-assert((qiov->size & (BDRV_SECTOR_SIZE - 1)) == 0);
 
 /**
  * In sync call context, when the vcpu is blocked, this throttling timer
@@ -2440,7 +2437,8 @@ static int bdrv_rw_co(BlockDriverState *bs, int64_t 
sector_num, uint8_t *buf,
 };
 
 qemu_iovec_init_external(&qiov, &iov, 1);
-return bdrv_rwv_co(bs, sector_num, &qiov, is_write, flags);
+return bdrv_prwv_co(bs, sector_num << BDRV_SECTOR_BITS,
+&qiov, is_write, flags);
 }
 
 /* return < 0 if error. See bdrv_write() for the return codes */
@@ -2478,7 +2476,7 @@ int bdrv_write(BlockDriverState *bs, int64_t sector_num,
 
 int bdrv_writev(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov)
 {
-return bdrv_rwv_co(bs, sector_num, qiov, true, 0);
+return bdrv_prwv_co(bs, sector_num << BDRV_SECTOR_BITS, qiov, true, 0);
 }
 
 int bdrv_write_zeroes(BlockDriverState *bs, int64_t sector_num,
@@ -4586,9 +4584,15 @@ int bdrv_flush(BlockDriverState *bs)
 return rwco.ret;
 }
 
+typedef struct DiscardCo {
+BlockDriverState *bs;
+int64_t sector_num;
+int nb_sectors;
+int ret;
+} DiscardCo;
 static void coroutine_fn bdrv_discard_co_entry(void *opaque)
 {
-RwCo *rwco = opaque;
+DiscardCo *rwco = opaque;
 
 rwco->ret = bdrv_co_discard(rwco->bs, rwco->sector_num, rwco->nb_sectors);
 }
@@ -4672,7 +4676,7 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, 
int64_t sector_num,
 int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
 {
 Coroutine *co;
-RwCo rwco = {
+DiscardCo rwco = {
 .bs = bs,
 .sector_num = sector_num,
 .nb_sectors = nb_sectors,
-- 
1.8.1.4




[Qemu-devel] [PATCH 11/22] block: Introduce bdrv_aligned_pwritev()

2013-12-11 Thread Kevin Wolf
This separates the part of bdrv_co_do_writev() that needs to happen
before the request is modified to match the backend alignment, and a
part that needs to be executed afterwards and passes the request to the
BlockDriver.

Signed-off-by: Kevin Wolf 
---
 block.c | 62 +-
 1 file changed, 41 insertions(+), 21 deletions(-)

diff --git a/block.c b/block.c
index 7812d8f..e26e31c 100644
--- a/block.c
+++ b/block.c
@@ -2958,34 +2958,20 @@ static int coroutine_fn 
bdrv_co_do_write_zeroes(BlockDriverState *bs,
 }
 
 /*
- * Handle a write request in coroutine context
+ * Forwards an already correctly aligned write request to the BlockDriver.
  */
-static int coroutine_fn bdrv_co_do_writev(BlockDriverState *bs,
-int64_t sector_num, int nb_sectors, QEMUIOVector *qiov,
-BdrvRequestFlags flags)
+static int coroutine_fn bdrv_aligned_pwritev(BlockDriverState *bs,
+int64_t offset, unsigned int bytes, QEMUIOVector *qiov, int flags)
 {
 BlockDriver *drv = bs->drv;
 BdrvTrackedRequest req;
 int ret;
 
-if (!bs->drv) {
-return -ENOMEDIUM;
-}
-if (bs->read_only) {
-return -EACCES;
-}
-if (bdrv_check_request(bs, sector_num, nb_sectors)) {
-return -EIO;
-}
-
-if (bs->copy_on_read_in_flight) {
-wait_for_overlapping_requests(bs, sector_num, nb_sectors);
-}
+int64_t sector_num = offset >> BDRV_SECTOR_BITS;
+unsigned int nb_sectors = bytes >> BDRV_SECTOR_BITS;
 
-/* throttling disk I/O */
-if (bs->io_limits_enabled) {
-bdrv_io_limits_intercept(bs, nb_sectors, true);
-}
+assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
+assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
 
 tracked_request_begin(&req, bs, sector_num, nb_sectors, true);
 
@@ -3017,6 +3003,40 @@ static int coroutine_fn 
bdrv_co_do_writev(BlockDriverState *bs,
 return ret;
 }
 
+/*
+ * Handle a write request in coroutine context
+ */
+static int coroutine_fn bdrv_co_do_writev(BlockDriverState *bs,
+int64_t sector_num, int nb_sectors, QEMUIOVector *qiov,
+BdrvRequestFlags flags)
+{
+int ret;
+
+if (!bs->drv) {
+return -ENOMEDIUM;
+}
+if (bs->read_only) {
+return -EACCES;
+}
+if (bdrv_check_request(bs, sector_num, nb_sectors)) {
+return -EIO;
+}
+
+if (bs->copy_on_read_in_flight) {
+wait_for_overlapping_requests(bs, sector_num, nb_sectors);
+}
+
+/* throttling disk I/O */
+if (bs->io_limits_enabled) {
+bdrv_io_limits_intercept(bs, nb_sectors, true);
+}
+
+ret = bdrv_aligned_pwritev(bs, sector_num << BDRV_SECTOR_BITS,
+   nb_sectors << BDRV_SECTOR_BITS, qiov, flags);
+
+return ret;
+}
+
 int coroutine_fn bdrv_co_writev(BlockDriverState *bs, int64_t sector_num,
 int nb_sectors, QEMUIOVector *qiov)
 {
-- 
1.8.1.4




[Qemu-devel] [PATCH 13/22] block: Introduce bdrv_co_do_pwritev()

2013-12-11 Thread Kevin Wolf
This is going to become the bdrv_co_do_preadv() equivalent for writes.
In this patch, however, just a function taking byte offsets is created,
it doesn't align anything yet.

Signed-off-by: Kevin Wolf 
---
 block.c | 23 +--
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/block.c b/block.c
index 385fb8a..a80db2e 100644
--- a/block.c
+++ b/block.c
@@ -3010,8 +3010,8 @@ static int coroutine_fn 
bdrv_aligned_pwritev(BlockDriverState *bs,
 /*
  * Handle a write request in coroutine context
  */
-static int coroutine_fn bdrv_co_do_writev(BlockDriverState *bs,
-int64_t sector_num, int nb_sectors, QEMUIOVector *qiov,
+static int coroutine_fn bdrv_co_do_pwritev(BlockDriverState *bs,
+int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
 BdrvRequestFlags flags)
 {
 int ret;
@@ -3022,21 +3022,32 @@ static int coroutine_fn 
bdrv_co_do_writev(BlockDriverState *bs,
 if (bs->read_only) {
 return -EACCES;
 }
-if (bdrv_check_request(bs, sector_num, nb_sectors)) {
+if (bdrv_check_byte_request(bs, offset, bytes)) {
 return -EIO;
 }
 
 /* throttling disk I/O */
 if (bs->io_limits_enabled) {
-bdrv_io_limits_intercept(bs, nb_sectors, true);
+bdrv_io_limits_intercept(bs, bytes << BDRV_SECTOR_BITS, true);
 }
 
-ret = bdrv_aligned_pwritev(bs, sector_num << BDRV_SECTOR_BITS,
-   nb_sectors << BDRV_SECTOR_BITS, qiov, flags);
+ret = bdrv_aligned_pwritev(bs, offset, bytes, qiov, flags);
 
 return ret;
 }
 
+static int coroutine_fn bdrv_co_do_writev(BlockDriverState *bs,
+int64_t sector_num, int nb_sectors, QEMUIOVector *qiov,
+BdrvRequestFlags flags)
+{
+if (nb_sectors < 0 || nb_sectors > (UINT_MAX >> BDRV_SECTOR_BITS)) {
+return -EINVAL;
+}
+
+return bdrv_co_do_pwritev(bs, sector_num << BDRV_SECTOR_BITS,
+  nb_sectors << BDRV_SECTOR_BITS, qiov, flags);
+}
+
 int coroutine_fn bdrv_co_writev(BlockDriverState *bs, int64_t sector_num,
 int nb_sectors, QEMUIOVector *qiov)
 {
-- 
1.8.1.4




[Qemu-devel] [PATCH 21/22] block: Make bdrv_pread() a bdrv_prwv_co() wrapper

2013-12-11 Thread Kevin Wolf
Instead of implementing the alignment adjustment here, use the now
existing functionality of bdrv_co_do_preadv().

Signed-off-by: Kevin Wolf 
---
 block.c | 49 +
 1 file changed, 13 insertions(+), 36 deletions(-)

diff --git a/block.c b/block.c
index 5e2e092..e8653e3 100644
--- a/block.c
+++ b/block.c
@@ -2523,49 +2523,26 @@ int bdrv_make_zero(BlockDriverState *bs, 
BdrvRequestFlags flags)
 }
 }
 
-int bdrv_pread(BlockDriverState *bs, int64_t offset,
-   void *buf, int count1)
+int bdrv_pread(BlockDriverState *bs, int64_t offset, void *buf, int bytes)
 {
-uint8_t tmp_buf[BDRV_SECTOR_SIZE];
-int len, nb_sectors, count;
-int64_t sector_num;
+QEMUIOVector qiov;
+struct iovec iov = {
+.iov_base = (void *)buf,
+.iov_len = bytes,
+};
 int ret;
 
-count = count1;
-/* first read to align to sector start */
-len = (BDRV_SECTOR_SIZE - offset) & (BDRV_SECTOR_SIZE - 1);
-if (len > count)
-len = count;
-sector_num = offset >> BDRV_SECTOR_BITS;
-if (len > 0) {
-if ((ret = bdrv_read(bs, sector_num, tmp_buf, 1)) < 0)
-return ret;
-memcpy(buf, tmp_buf + (offset & (BDRV_SECTOR_SIZE - 1)), len);
-count -= len;
-if (count == 0)
-return count1;
-sector_num++;
-buf += len;
+if (bytes < 0) {
+return -EINVAL;
 }
 
-/* read the sectors "in place" */
-nb_sectors = count >> BDRV_SECTOR_BITS;
-if (nb_sectors > 0) {
-if ((ret = bdrv_read(bs, sector_num, buf, nb_sectors)) < 0)
-return ret;
-sector_num += nb_sectors;
-len = nb_sectors << BDRV_SECTOR_BITS;
-buf += len;
-count -= len;
+qemu_iovec_init_external(&qiov, &iov, 1);
+ret = bdrv_prwv_co(bs, offset, &qiov, false, 0);
+if (ret < 0) {
+return ret;
 }
 
-/* add data from the last sector */
-if (count > 0) {
-if ((ret = bdrv_read(bs, sector_num, tmp_buf, 1)) < 0)
-return ret;
-memcpy(buf, tmp_buf, count);
-}
-return count1;
+return bytes;
 }
 
 int bdrv_pwritev(BlockDriverState *bs, int64_t offset, QEMUIOVector *qiov)
-- 
1.8.1.4




[Qemu-devel] [PATCH 07/22] block: rename buffer_alignment to guest_block_size

2013-12-11 Thread Kevin Wolf
From: Paolo Bonzini 

The alignment field is now set to the value that is promised to the
guest, rather than required by the host.  The next patches will make
QEMU aware of the host-provided values, so make this clear.

The alignment is also not about memory buffers, but about the sectors on
the disk, change the documentation of the field.

At this point, the field is set by the device emulation, but completely
ignored by the block layer.

Signed-off-by: Paolo Bonzini 
Signed-off-by: Kevin Wolf 
---
 block.c   | 10 +-
 hw/block/virtio-blk.c |  2 +-
 hw/ide/core.c |  2 +-
 hw/scsi/scsi-disk.c   |  2 +-
 hw/scsi/scsi-generic.c|  2 +-
 include/block/block.h |  2 +-
 include/block/block_int.h |  4 ++--
 7 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/block.c b/block.c
index d50a4e4..3504d17 100644
--- a/block.c
+++ b/block.c
@@ -812,7 +812,7 @@ static int bdrv_open_common(BlockDriverState *bs, 
BlockDriverState *file,
 }
 
 bs->open_flags = flags;
-bs->buffer_alignment = 512;
+bs->guest_block_size = 512;
 bs->zero_beyond_eof = true;
 open_flags = bdrv_open_flags(bs, flags);
 bs->read_only = !(open_flags & BDRV_O_RDWR);
@@ -1648,7 +1648,7 @@ static void bdrv_move_feature_fields(BlockDriverState 
*bs_dest,
 bs_dest->dev_ops= bs_src->dev_ops;
 bs_dest->dev_opaque = bs_src->dev_opaque;
 bs_dest->dev= bs_src->dev;
-bs_dest->buffer_alignment   = bs_src->buffer_alignment;
+bs_dest->guest_block_size   = bs_src->guest_block_size;
 bs_dest->copy_on_read   = bs_src->copy_on_read;
 
 bs_dest->enable_write_cache = bs_src->enable_write_cache;
@@ -1800,7 +1800,7 @@ void bdrv_detach_dev(BlockDriverState *bs, void *dev)
 bs->dev = NULL;
 bs->dev_ops = NULL;
 bs->dev_opaque = NULL;
-bs->buffer_alignment = 512;
+bs->guest_block_size = 512;
 }
 
 /* TODO change to return DeviceState * when all users are qdevified */
@@ -4556,9 +4556,9 @@ BlockDriverAIOCB *bdrv_aio_ioctl(BlockDriverState *bs,
 return NULL;
 }
 
-void bdrv_set_buffer_alignment(BlockDriverState *bs, int align)
+void bdrv_set_guest_block_size(BlockDriverState *bs, int align)
 {
-bs->buffer_alignment = align;
+bs->guest_block_size = align;
 }
 
 void *qemu_blockalign(BlockDriverState *bs, size_t size)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 13f6d82..323e9ec 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -720,7 +720,7 @@ static int virtio_blk_device_init(VirtIODevice *vdev)
 register_savevm(qdev, "virtio-blk", virtio_blk_id++, 2,
 virtio_blk_save, virtio_blk_load, s);
 bdrv_set_dev_ops(s->bs, &virtio_block_ops, s);
-bdrv_set_buffer_alignment(s->bs, s->conf->logical_block_size);
+bdrv_set_guest_block_size(s->bs, s->conf->logical_block_size);
 
 bdrv_iostatus_enable(s->bs);
 
diff --git a/hw/ide/core.c b/hw/ide/core.c
index e1f4c33..036cd4a 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2103,7 +2103,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, 
IDEDriveKind kind,
 s->smart_selftest_count = 0;
 if (kind == IDE_CD) {
 bdrv_set_dev_ops(bs, &ide_cd_block_ops, s);
-bdrv_set_buffer_alignment(bs, 2048);
+bdrv_set_guest_block_size(bs, 2048);
 } else {
 if (!bdrv_is_inserted(s->bs)) {
 error_report("Device needs media, but drive is empty");
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index efadfc0..6cf6040 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2225,7 +2225,7 @@ static int scsi_initfn(SCSIDevice *dev)
 } else {
 bdrv_set_dev_ops(s->qdev.conf.bs, &scsi_disk_block_ops, s);
 }
-bdrv_set_buffer_alignment(s->qdev.conf.bs, s->qdev.blocksize);
+bdrv_set_guest_block_size(s->qdev.conf.bs, s->qdev.blocksize);
 
 bdrv_iostatus_enable(s->qdev.conf.bs);
 add_boot_device_path(s->qdev.conf.bootindex, &dev->qdev, NULL);
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 8f195be..f08b64e 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -210,7 +210,7 @@ static void scsi_read_complete(void * opaque, int ret)
 s->blocksize = ldl_be_p(&r->buf[8]);
 s->max_lba = ldq_be_p(&r->buf[0]);
 }
-bdrv_set_buffer_alignment(s->conf.bs, s->blocksize);
+bdrv_set_guest_block_size(s->conf.bs, s->blocksize);
 
 scsi_req_data(&r->req, len);
 if (!r->req.io_canceled) {
diff --git a/include/block/block.h b/include/block/block.h
index cf63ee2..05252d5 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -422,7 +422,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
 /* Returns the alignment in bytes that is required so that no bounce buffer
  * is required throughout the stack */
 size_t bdrv_opt_mem_align(BlockDriverState *bs);
-void bdrv_set_buffer_alignment(BlockDriverState *bs, i

[Qemu-devel] [PATCH 19/22] block: Align requests in bdrv_co_do_pwritev()

2013-12-11 Thread Kevin Wolf
This patch changes bdrv_co_do_pwritev() to actually be what its name
promises. If requests aren't properly aligned, it performs a RMW.

Requests touching the same block are serialised against the RMW request.
Further optimisation of this is possible by differentiating types of
requests (concurrent reads should actually be okay here).

Signed-off-by: Kevin Wolf 
---
 block.c | 86 -
 1 file changed, 85 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 6630b41..98ea723 100644
--- a/block.c
+++ b/block.c
@@ -3039,6 +3039,12 @@ static int coroutine_fn 
bdrv_co_do_pwritev(BlockDriverState *bs,
 BdrvRequestFlags flags)
 {
 BdrvTrackedRequest req;
+/* TODO Lift BDRV_SECTOR_SIZE restriction in BlockDriver interface */
+uint64_t align = MAX(BDRV_SECTOR_SIZE, bs->request_alignment);
+uint8_t *head_buf = NULL;
+uint8_t *tail_buf = NULL;
+QEMUIOVector local_qiov;
+bool use_local_qiov = false;
 int ret;
 
 if (!bs->drv) {
@@ -3056,10 +3062,88 @@ static int coroutine_fn 
bdrv_co_do_pwritev(BlockDriverState *bs,
 bdrv_io_limits_intercept(bs, bytes << BDRV_SECTOR_BITS, true);
 }
 
+/*
+ * Align write if necessary by performing a read-modify-write cycle.
+ * Pad qiov with the read parts and be sure to have a tracked request not
+ * only for bdrv_aligned_pwritev, but also for the reads of the RMW cycle.
+ */
 tracked_request_begin(&req, bs, offset, bytes, true);
-ret = bdrv_aligned_pwritev(bs, &req, offset, bytes, qiov, flags);
+
+if (offset & (align - 1)) {
+QEMUIOVector head_qiov;
+struct iovec head_iov;
+
+mark_request_serialising(&req, align);
+wait_serialising_requests(&req);
+
+head_buf = qemu_blockalign(bs, align);
+head_iov = (struct iovec) {
+.iov_base   = head_buf,
+.iov_len= align,
+};
+qemu_iovec_init_external(&head_qiov, &head_iov, 1);
+
+ret = bdrv_aligned_preadv(bs, &req, offset & ~(align - 1), align,
+  align, &head_qiov, 0);
+if (ret < 0) {
+goto fail;
+}
+
+qemu_iovec_init(&local_qiov, qiov->niov + 2);
+qemu_iovec_add(&local_qiov, head_buf, offset & (align - 1));
+qemu_iovec_concat(&local_qiov, qiov, 0, qiov->size);
+use_local_qiov = true;
+
+bytes += offset & (align - 1);
+offset = offset & ~(align - 1);
+}
+
+if ((offset + bytes) & (align - 1)) {
+QEMUIOVector tail_qiov;
+struct iovec tail_iov;
+size_t tail_bytes;
+
+mark_request_serialising(&req, align);
+wait_serialising_requests(&req);
+
+tail_buf = qemu_blockalign(bs, align);
+tail_iov = (struct iovec) {
+.iov_base   = tail_buf,
+.iov_len= align,
+};
+qemu_iovec_init_external(&tail_qiov, &tail_iov, 1);
+
+ret = bdrv_aligned_preadv(bs, &req, (offset + bytes) & ~(align - 1), 
align,
+  align, &tail_qiov, 0);
+if (ret < 0) {
+goto fail;
+}
+
+if (!use_local_qiov) {
+qemu_iovec_init(&local_qiov, qiov->niov + 1);
+qemu_iovec_concat(&local_qiov, qiov, 0, qiov->size);
+use_local_qiov = true;
+}
+
+tail_bytes = (offset + bytes) & (align - 1);
+qemu_iovec_add(&local_qiov, tail_buf + tail_bytes, align - tail_bytes);
+
+bytes = ROUND_UP(bytes, align);
+}
+
+ret = bdrv_aligned_pwritev(bs, &req, offset, bytes,
+   use_local_qiov ? &local_qiov : qiov,
+   flags);
+
+fail:
 tracked_request_end(&req);
 
+if (use_local_qiov) {
+qemu_iovec_destroy(&local_qiov);
+qemu_vfree(head_buf);
+qemu_vfree(tail_buf);
+}
+
 return ret;
 }
 
-- 
1.8.1.4




[Qemu-devel] [PATCH 09/22] block: Introduce bdrv_aligned_preadv()

2013-12-11 Thread Kevin Wolf
This separates the part of bdrv_co_do_readv() that needs to happen
before the request is modified to match the backend alignment, and a
part that needs to be executed afterwards and passes the request to the
BlockDriver.

Signed-off-by: Kevin Wolf 
---
 block.c | 61 +++--
 1 file changed, 43 insertions(+), 18 deletions(-)

diff --git a/block.c b/block.c
index b86e754..cbc9f6d 100644
--- a/block.c
+++ b/block.c
@@ -2700,26 +2700,24 @@ err:
 }
 
 /*
- * Handle a read request in coroutine context
+ * Forwards an already correctly aligned request to the BlockDriver. This
+ * handles copy on read and zeroing after EOF; any other features must be
+ * implemented by the caller.
  */
-static int coroutine_fn bdrv_co_do_readv(BlockDriverState *bs,
-int64_t sector_num, int nb_sectors, QEMUIOVector *qiov,
-BdrvRequestFlags flags)
+static int coroutine_fn bdrv_aligned_preadv(BlockDriverState *bs,
+int64_t offset, unsigned int bytes, QEMUIOVector *qiov, int flags)
 {
 BlockDriver *drv = bs->drv;
 BdrvTrackedRequest req;
 int ret;
 
-if (!drv) {
-return -ENOMEDIUM;
-}
-if (bdrv_check_request(bs, sector_num, nb_sectors)) {
-return -EIO;
-}
+int64_t sector_num = offset >> BDRV_SECTOR_BITS;
+unsigned int nb_sectors = bytes >> BDRV_SECTOR_BITS;
 
-if (bs->copy_on_read) {
-flags |= BDRV_REQ_COPY_ON_READ;
-}
+assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
+assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
+
+/* Handle Copy on Read and associated serialisation */
 if (flags & BDRV_REQ_COPY_ON_READ) {
 bs->copy_on_read_in_flight++;
 }
@@ -2728,11 +2726,6 @@ static int coroutine_fn 
bdrv_co_do_readv(BlockDriverState *bs,
 wait_for_overlapping_requests(bs, sector_num, nb_sectors);
 }
 
-/* throttling disk I/O */
-if (bs->io_limits_enabled) {
-bdrv_io_limits_intercept(bs, nb_sectors, false);
-}
-
 tracked_request_begin(&req, bs, sector_num, nb_sectors, false);
 
 if (flags & BDRV_REQ_COPY_ON_READ) {
@@ -2749,6 +2742,7 @@ static int coroutine_fn bdrv_co_do_readv(BlockDriverState 
*bs,
 }
 }
 
+/* Forward the request to the BlockDriver */
 if (!(bs->zero_beyond_eof && bs->growable)) {
 ret = drv->bdrv_co_readv(bs, sector_num, nb_sectors, qiov);
 } else {
@@ -2789,6 +2783,37 @@ out:
 return ret;
 }
 
+/*
+ * Handle a read request in coroutine context
+ */
+static int coroutine_fn bdrv_co_do_readv(BlockDriverState *bs,
+int64_t sector_num, int nb_sectors, QEMUIOVector *qiov,
+BdrvRequestFlags flags)
+{
+BlockDriver *drv = bs->drv;
+int ret;
+
+if (!drv) {
+return -ENOMEDIUM;
+}
+if (bdrv_check_request(bs, sector_num, nb_sectors)) {
+return -EIO;
+}
+
+if (bs->copy_on_read) {
+flags |= BDRV_REQ_COPY_ON_READ;
+}
+
+/* throttling disk I/O */
+if (bs->io_limits_enabled) {
+bdrv_io_limits_intercept(bs, nb_sectors, false);
+}
+
+ret = bdrv_aligned_preadv(bs, sector_num << BDRV_SECTOR_BITS,
+ nb_sectors << BDRV_SECTOR_BITS, qiov, flags);
+return ret;
+}
+
 int coroutine_fn bdrv_co_readv(BlockDriverState *bs, int64_t sector_num,
 int nb_sectors, QEMUIOVector *qiov)
 {
-- 
1.8.1.4




[Qemu-devel] [PATCH 22/22] block: Make bdrv_pwrite() a bdrv_prwv_co() wrapper

2013-12-11 Thread Kevin Wolf
Instead of implementing the alignment adjustment here, use the now
existing functionality of bdrv_co_do_pwritev().

Signed-off-by: Kevin Wolf 
---
 block.c | 64 +---
 1 file changed, 9 insertions(+), 55 deletions(-)

diff --git a/block.c b/block.c
index e8653e3..4ab40f8 100644
--- a/block.c
+++ b/block.c
@@ -2474,11 +2474,6 @@ int bdrv_write(BlockDriverState *bs, int64_t sector_num,
 return bdrv_rw_co(bs, sector_num, (uint8_t *)buf, nb_sectors, true, 0);
 }
 
-int bdrv_writev(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov)
-{
-return bdrv_prwv_co(bs, sector_num << BDRV_SECTOR_BITS, qiov, true, 0);
-}
-
 int bdrv_write_zeroes(BlockDriverState *bs, int64_t sector_num,
   int nb_sectors, BdrvRequestFlags flags)
 {
@@ -2547,70 +2542,29 @@ int bdrv_pread(BlockDriverState *bs, int64_t offset, 
void *buf, int bytes)
 
 int bdrv_pwritev(BlockDriverState *bs, int64_t offset, QEMUIOVector *qiov)
 {
-uint8_t tmp_buf[BDRV_SECTOR_SIZE];
-int len, nb_sectors, count;
-int64_t sector_num;
 int ret;
 
-count = qiov->size;
-
-/* first write to align to sector start */
-len = (BDRV_SECTOR_SIZE - offset) & (BDRV_SECTOR_SIZE - 1);
-if (len > count)
-len = count;
-sector_num = offset >> BDRV_SECTOR_BITS;
-if (len > 0) {
-if ((ret = bdrv_read(bs, sector_num, tmp_buf, 1)) < 0)
-return ret;
-qemu_iovec_to_buf(qiov, 0, tmp_buf + (offset & (BDRV_SECTOR_SIZE - 1)),
-  len);
-if ((ret = bdrv_write(bs, sector_num, tmp_buf, 1)) < 0)
-return ret;
-count -= len;
-if (count == 0)
-return qiov->size;
-sector_num++;
-}
-
-/* write the sectors "in place" */
-nb_sectors = count >> BDRV_SECTOR_BITS;
-if (nb_sectors > 0) {
-QEMUIOVector qiov_inplace;
-
-qemu_iovec_init(&qiov_inplace, qiov->niov);
-qemu_iovec_concat(&qiov_inplace, qiov, len,
-  nb_sectors << BDRV_SECTOR_BITS);
-ret = bdrv_writev(bs, sector_num, &qiov_inplace);
-qemu_iovec_destroy(&qiov_inplace);
-if (ret < 0) {
-return ret;
-}
-
-sector_num += nb_sectors;
-len = nb_sectors << BDRV_SECTOR_BITS;
-count -= len;
+ret = bdrv_prwv_co(bs, offset, qiov, true, 0);
+if (ret < 0) {
+return ret;
 }
 
-/* add data from the last sector */
-if (count > 0) {
-if ((ret = bdrv_read(bs, sector_num, tmp_buf, 1)) < 0)
-return ret;
-qemu_iovec_to_buf(qiov, qiov->size - count, tmp_buf, count);
-if ((ret = bdrv_write(bs, sector_num, tmp_buf, 1)) < 0)
-return ret;
-}
 return qiov->size;
 }
 
 int bdrv_pwrite(BlockDriverState *bs, int64_t offset,
-const void *buf, int count1)
+const void *buf, int bytes)
 {
 QEMUIOVector qiov;
 struct iovec iov = {
 .iov_base   = (void *) buf,
-.iov_len= count1,
+.iov_len= bytes,
 };
 
+if (bytes < 0) {
+return -EINVAL;
+}
+
 qemu_iovec_init_external(&qiov, &iov, 1);
 return bdrv_pwritev(bs, offset, &qiov);
 }
-- 
1.8.1.4




[Qemu-devel] [PATCH 12/22] block: write: Handle COR dependency after I/O throttling

2013-12-11 Thread Kevin Wolf
First waiting for all COR requests to complete and calling the
throttling function afterwards means that the request could be delayed
and we still need to wait for the COR request even if it was issued only
after the throttled write request.

Signed-off-by: Kevin Wolf 
---
 block.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block.c b/block.c
index e26e31c..385fb8a 100644
--- a/block.c
+++ b/block.c
@@ -2973,6 +2973,10 @@ static int coroutine_fn 
bdrv_aligned_pwritev(BlockDriverState *bs,
 assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
 assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
 
+if (bs->copy_on_read_in_flight) {
+wait_for_overlapping_requests(bs, sector_num, nb_sectors);
+}
+
 tracked_request_begin(&req, bs, sector_num, nb_sectors, true);
 
 ret = notifier_with_return_list_notify(&bs->before_write_notifiers, &req);
@@ -3022,10 +3026,6 @@ static int coroutine_fn 
bdrv_co_do_writev(BlockDriverState *bs,
 return -EIO;
 }
 
-if (bs->copy_on_read_in_flight) {
-wait_for_overlapping_requests(bs, sector_num, nb_sectors);
-}
-
 /* throttling disk I/O */
 if (bs->io_limits_enabled) {
 bdrv_io_limits_intercept(bs, nb_sectors, true);
-- 
1.8.1.4




[Qemu-devel] [PATCH 04/22] qemu_memalign: Allow small alignments

2013-12-11 Thread Kevin Wolf
The functions used by qemu_memalign() require an alignment that is at
least sizeof(void*). Adjust it if it is too small.

Signed-off-by: Kevin Wolf 
---
 util/oslib-posix.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index e00a44c..54f8932 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -85,6 +85,11 @@ void *qemu_oom_check(void *ptr)
 void *qemu_memalign(size_t alignment, size_t size)
 {
 void *ptr;
+
+if (alignment < sizeof(void*)) {
+alignment = sizeof(void*);
+}
+
 #if defined(_POSIX_C_SOURCE) && !defined(__sun__)
 int ret;
 ret = posix_memalign(&ptr, alignment, size);
-- 
1.8.1.4




[Qemu-devel] [PATCH 00/22] block: Support for 512b-on-4k emulation

2013-12-11 Thread Kevin Wolf
This patch series adds code to the block layer that allows performing
I/O requests in smaller granularities than required by the host backend
(most importantly, O_DIRECT restrictions). It achieves this for reads
by rounding the request to host-side block boundary, and for writes by
performing a read-modify-write cycle (and serialising requests
touching the same block so that the RMW doesn't write back stale data).

Originally I intended to reuse a lot of code from Paolo's previous
patch series, however as I tried to integrate pread/pwrite, which
already do a very similar thing (except for considering concurrency),
and because I wanted to implement zero-copy, most of this series ended
up being new code.

Zero-copy is possible in a common case because while XFS defauls to a
4k sector size and therefore 4k on-disk O_DIRECT alignment for 512E
disks, it still only has a 512 byte memory alignment requirement.
(Unfortunately the XFS_IOC_DIOINFO ioctl claims 4k even for memory, but
we know that the value is wrong and can probe it.)


Changes since RFC:
- Moved opt_mem_alignment into BlockLimits [Paolo]

- Changed BlockLimits in turn to work a bit more like the
  .bdrv_opt_mem_align() callback of the RFC; allows updating the
  BlockLimits later when the chain changes or bdrv_reopen() toggles
  O_DIRECT

- Fixed a typo in a commit message [Eric]


Kevin Wolf (20):
  block: Move initialisation of BlockLimits to bdrv_refresh_limits()
  block: Inherit opt_transfer_length
  block: Update BlockLimits when they might have changed
  qemu_memalign: Allow small alignments
  block: Detect unaligned length in bdrv_qiov_is_aligned()
  block: Don't use guest sector size for qemu_blockalign()
  block: Introduce bdrv_aligned_preadv()
  block: Introduce bdrv_co_do_preadv()
  block: Introduce bdrv_aligned_pwritev()
  block: write: Handle COR dependency after I/O throttling
  block: Introduce bdrv_co_do_pwritev()
  block: Switch BdrvTrackedRequest to byte granularity
  block: Allow waiting for overlapping requests between begin/end
  block: Make zero-after-EOF work with larger alignment
  block: Generalise and optimise COR serialisation
  block: Make overlap range for serialisation dynamic
  block: Align requests in bdrv_co_do_pwritev()
  block: Change coroutine wrapper to byte granularity
  block: Make bdrv_pread() a bdrv_prwv_co() wrapper
  block: Make bdrv_pwrite() a bdrv_prwv_co() wrapper

Paolo Bonzini (2):
  block: rename buffer_alignment to guest_block_size
  raw: Probe required direct I/O alignment

 block.c   | 602 +++---
 block/backup.c|   7 +-
 block/iscsi.c |  87 ---
 block/qcow2.c |  11 +-
 block/qed.c   |  11 +-
 block/raw-posix.c | 102 ++--
 block/raw-win32.c |  41 
 block/stream.c|   2 +
 block/vmdk.c  |  22 +-
 hw/block/virtio-blk.c |   2 +-
 hw/ide/core.c |   2 +-
 hw/scsi/scsi-disk.c   |   2 +-
 hw/scsi/scsi-generic.c|   2 +-
 include/block/block.h |   6 +-
 include/block/block_int.h |  25 +-
 util/oslib-posix.c|   5 +
 16 files changed, 666 insertions(+), 263 deletions(-)

-- 
1.8.1.4




[Qemu-devel] [PATCH 10/22] block: Introduce bdrv_co_do_preadv()

2013-12-11 Thread Kevin Wolf
Similar to bdrv_pread(), which aligns byte-aligned request to 512 byte
sectors, bdrv_co_do_preadv() takes a byte-aligned request and aligns it
to the alignment specified in bs->request_alignment.

Signed-off-by: Kevin Wolf 
---
 block.c | 63 +--
 1 file changed, 57 insertions(+), 6 deletions(-)

diff --git a/block.c b/block.c
index cbc9f6d..7812d8f 100644
--- a/block.c
+++ b/block.c
@@ -2786,17 +2786,23 @@ out:
 /*
  * Handle a read request in coroutine context
  */
-static int coroutine_fn bdrv_co_do_readv(BlockDriverState *bs,
-int64_t sector_num, int nb_sectors, QEMUIOVector *qiov,
+static int coroutine_fn bdrv_co_do_preadv(BlockDriverState *bs,
+int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
 BdrvRequestFlags flags)
 {
 BlockDriver *drv = bs->drv;
+/* TODO Lift BDRV_SECTOR_SIZE restriction in BlockDriver interface */
+uint64_t align = MAX(BDRV_SECTOR_SIZE, bs->request_alignment);
+uint8_t *head_buf = NULL;
+uint8_t *tail_buf = NULL;
+QEMUIOVector local_qiov;
+bool use_local_qiov = false;
 int ret;
 
 if (!drv) {
 return -ENOMEDIUM;
 }
-if (bdrv_check_request(bs, sector_num, nb_sectors)) {
+if (bdrv_check_byte_request(bs, offset, bytes)) {
 return -EIO;
 }
 
@@ -2806,14 +2812,59 @@ static int coroutine_fn 
bdrv_co_do_readv(BlockDriverState *bs,
 
 /* throttling disk I/O */
 if (bs->io_limits_enabled) {
-bdrv_io_limits_intercept(bs, nb_sectors, false);
+bdrv_io_limits_intercept(bs, bytes >> BDRV_SECTOR_BITS, false);
+}
+
+/* Align read if necessary by padding qiov */
+if (offset & (align - 1)) {
+head_buf = qemu_blockalign(bs, align);
+qemu_iovec_init(&local_qiov, qiov->niov + 2);
+qemu_iovec_add(&local_qiov, head_buf, offset & (align - 1));
+qemu_iovec_concat(&local_qiov, qiov, 0, qiov->size);
+use_local_qiov = true;
+
+bytes += offset & (align - 1);
+offset = offset & ~(align - 1);
+}
+
+if ((offset + bytes) & (align - 1)) {
+if (!use_local_qiov) {
+qemu_iovec_init(&local_qiov, qiov->niov + 1);
+qemu_iovec_concat(&local_qiov, qiov, 0, qiov->size);
+use_local_qiov = true;
+}
+tail_buf = qemu_blockalign(bs, align);
+qemu_iovec_add(&local_qiov, tail_buf,
+   align - ((offset + bytes) & (align - 1)));
+
+bytes = ROUND_UP(bytes, align);
+}
+
+ret = bdrv_aligned_preadv(bs, offset, bytes,
+  use_local_qiov ? &local_qiov : qiov,
+  flags);
+
+if (use_local_qiov) {
+qemu_iovec_destroy(&local_qiov);
+qemu_vfree(head_buf);
+qemu_vfree(tail_buf);
 }
 
-ret = bdrv_aligned_preadv(bs, sector_num << BDRV_SECTOR_BITS,
- nb_sectors << BDRV_SECTOR_BITS, qiov, flags);
 return ret;
 }
 
+static int coroutine_fn bdrv_co_do_readv(BlockDriverState *bs,
+int64_t sector_num, int nb_sectors, QEMUIOVector *qiov,
+BdrvRequestFlags flags)
+{
+if (nb_sectors < 0 || nb_sectors > (UINT_MAX >> BDRV_SECTOR_BITS)) {
+return -EINVAL;
+}
+
+return bdrv_co_do_preadv(bs, sector_num << BDRV_SECTOR_BITS,
+ nb_sectors << BDRV_SECTOR_BITS, qiov, flags);
+}
+
 int coroutine_fn bdrv_co_readv(BlockDriverState *bs, int64_t sector_num,
 int nb_sectors, QEMUIOVector *qiov)
 {
-- 
1.8.1.4




[Qemu-devel] [PATCH 08/22] raw: Probe required direct I/O alignment

2013-12-11 Thread Kevin Wolf
From: Paolo Bonzini 

Add a bs->request_alignment field that contains the required
offset/length alignment for I/O requests and fill it in the raw block
drivers. Use ioctls if possible, else see what alignment it takes for
O_DIRECT to succeed.

While at it, also expose the memory alignment requirements, which may be
(and in practice are) different from the disk alignment requirements.

Signed-off-by: Paolo Bonzini 
Signed-off-by: Kevin Wolf 
---
 block.c   |   3 ++
 block/raw-posix.c | 102 ++
 block/raw-win32.c |  41 +++
 include/block/block_int.h |   3 ++
 4 files changed, 132 insertions(+), 17 deletions(-)

diff --git a/block.c b/block.c
index 3504d17..b86e754 100644
--- a/block.c
+++ b/block.c
@@ -813,6 +813,7 @@ static int bdrv_open_common(BlockDriverState *bs, 
BlockDriverState *file,
 
 bs->open_flags = flags;
 bs->guest_block_size = 512;
+bs->request_alignment = 512;
 bs->zero_beyond_eof = true;
 open_flags = bdrv_open_flags(bs, flags);
 bs->read_only = !(open_flags & BDRV_O_RDWR);
@@ -881,6 +882,8 @@ static int bdrv_open_common(BlockDriverState *bs, 
BlockDriverState *file,
 }
 
 bdrv_refresh_limits(bs);
+assert(bdrv_opt_mem_align(bs) != 0);
+assert(bs->request_alignment != 0);
 
 #ifndef _WIN32
 if (bs->is_temporary) {
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 10c6b34..e8e75a7 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -127,6 +127,8 @@ typedef struct BDRVRawState {
 int fd;
 int type;
 int open_flags;
+size_t buf_align;
+
 #if defined(__linux__)
 /* linux floppy specific */
 int64_t fd_open_time;
@@ -213,6 +215,76 @@ static int raw_normalize_devicepath(const char **filename)
 }
 #endif
 
+static void raw_probe_alignment(BlockDriverState *bs)
+{
+BDRVRawState *s = bs->opaque;
+char *buf;
+unsigned int sector_size;
+
+/* For /dev/sg devices the alignment is not really used.
+   With buffered I/O, we don't have any restrictions. */
+if (bs->sg || !(s->open_flags & O_DIRECT)) {
+bs->request_alignment = 1;
+s->buf_align = 1;
+return;
+}
+
+/* Try a few ioctls to get the right size */
+bs->request_alignment = 0;
+s->buf_align = 0;
+
+#ifdef BLKSSZGET
+if (ioctl(s->fd, BLKSSZGET, §or_size) >= 0) {
+bs->request_alignment = sector_size;
+}
+#endif
+#ifdef DKIOCGETBLOCKSIZE
+if (ioctl(s->fd, DKIOCGETBLOCKSIZE, §or_size) >= 0) {
+bs->request_alignment = sector_size;
+}
+#endif
+#ifdef DIOCGSECTORSIZE
+if (ioctl(s->fd, DIOCGSECTORSIZE, §or_size) >= 0) {
+bs->request_alignment = sector_size;
+}
+#endif
+#ifdef CONFIG_XFS
+if (s->is_xfs) {
+struct dioattr da;
+if (xfsctl(NULL, s->fd, XFS_IOC_DIOINFO, &da) >= 0) {
+bs->request_alignment = da.d_miniosz;
+/* The kernel returns wrong information for d_mem */
+/* s->buf_align = da.d_mem; */
+}
+}
+#endif
+
+/* If we could not get the sizes so far, we can only guess them */
+if (!s->buf_align) {
+size_t align;
+buf = qemu_memalign(MAX_BLOCKSIZE, 2 * MAX_BLOCKSIZE);
+for (align = 512; align <= MAX_BLOCKSIZE; align <<= 1) {
+if (pread(s->fd, buf + align, MAX_BLOCKSIZE, 0) >= 0) {
+s->buf_align = align;
+break;
+}
+}
+qemu_vfree(buf);
+}
+
+if (!bs->request_alignment) {
+size_t align;
+buf = qemu_memalign(s->buf_align, MAX_BLOCKSIZE);
+for (align = 512; align <= MAX_BLOCKSIZE; align <<= 1) {
+if (pread(s->fd, buf, align, 0) >= 0) {
+bs->request_alignment = align;
+break;
+}
+}
+qemu_vfree(buf);
+}
+}
+
 static void raw_parse_flags(int bdrv_flags, int *open_flags)
 {
 assert(open_flags != NULL);
@@ -463,7 +535,6 @@ static int raw_reopen_prepare(BDRVReopenState *state,
 return ret;
 }
 
-
 static void raw_reopen_commit(BDRVReopenState *state)
 {
 BDRVRawReopenState *raw_s = state->opaque;
@@ -499,23 +570,15 @@ static void raw_reopen_abort(BDRVReopenState *state)
 state->opaque = NULL;
 }
 
+static int raw_refresh_limits(BlockDriverState *bs)
+{
+BDRVRawState *s = bs->opaque;
 
-/* XXX: use host sector size if necessary with:
-#ifdef DIOCGSECTORSIZE
-{
-unsigned int sectorsize = 512;
-if (!ioctl(fd, DIOCGSECTORSIZE, §orsize) &&
-sectorsize > bufsize)
-bufsize = sectorsize;
-}
-#endif
-#ifdef CONFIG_COCOA
-uint32_t blockSize = 512;
-if ( !ioctl( fd, DKIOCGETBLOCKSIZE, &blockSize ) && blockSize > 
bufsize) {
-bufsize = blockSize;
-}
-#endif
-*/
+raw_probe_alignment(bs);
+bs->bl.opt_mem_alignment = s->buf_align;
+
+return 0;
+}
 
 static ssize_t handle_aiocb_ioctl(Raw

[Qemu-devel] [PATCH 03/22] block: Update BlockLimits when they might have changed

2013-12-11 Thread Kevin Wolf
When reopening with different flags, or when backing files disappear
from the chain, the limits may change. Make sure they get updated in
these cases.

Signed-off-by: Kevin Wolf 
---
 block.c   | 5 -
 block/stream.c| 2 ++
 include/block/block.h | 1 +
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 79a325f..8eae359 100644
--- a/block.c
+++ b/block.c
@@ -479,7 +479,7 @@ int bdrv_create_file(const char* filename, 
QEMUOptionParameter *options,
 return ret;
 }
 
-static int bdrv_refresh_limits(BlockDriverState *bs)
+int bdrv_refresh_limits(BlockDriverState *bs)
 {
 BlockDriver *drv = bs->drv;
 
@@ -1464,6 +1464,8 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state)
 reopen_state->bs->enable_write_cache = !!(reopen_state->flags &
   BDRV_O_CACHE_WB);
 reopen_state->bs->read_only = !(reopen_state->flags & BDRV_O_RDWR);
+
+bdrv_refresh_limits(reopen_state->bs);
 }
 
 /*
@@ -2261,6 +2263,7 @@ int bdrv_drop_intermediate(BlockDriverState *active, 
BlockDriverState *top,
 }
 new_top_bs->backing_hd = base_bs;
 
+bdrv_refresh_limits(new_top_bs);
 
 QSIMPLEQ_FOREACH_SAFE(intermediate_state, &states_to_delete, entry, next) {
 /* so that bdrv_close() does not recursively close the chain */
diff --git a/block/stream.c b/block/stream.c
index 46bec7d..dd0b4ac 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -75,6 +75,8 @@ static void close_unused_images(BlockDriverState *top, 
BlockDriverState *base,
 unused->backing_hd = NULL;
 bdrv_unref(unused);
 }
+
+bdrv_refresh_limits(top);
 }
 
 static void coroutine_fn stream_run(void *opaque)
diff --git a/include/block/block.h b/include/block/block.h
index 36efaea..64fb319 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -249,6 +249,7 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset);
 int64_t bdrv_getlength(BlockDriverState *bs);
 int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
 void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
+int bdrv_refresh_limits(BlockDriverState *bs);
 int bdrv_commit(BlockDriverState *bs);
 int bdrv_commit_all(void);
 int bdrv_change_backing_file(BlockDriverState *bs,
-- 
1.8.1.4




[Qemu-devel] [PATCH 05/22] block: Detect unaligned length in bdrv_qiov_is_aligned()

2013-12-11 Thread Kevin Wolf
For an O_DIRECT request to succeed, it's not only necessary that all
base addresses in the qiov are aligned, but also that each length in it
is aligned.

Signed-off-by: Kevin Wolf 
---
 block.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block.c b/block.c
index 8eae359..62c5a75 100644
--- a/block.c
+++ b/block.c
@@ -4561,6 +4561,9 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, 
QEMUIOVector *qiov)
 if ((uintptr_t) qiov->iov[i].iov_base % bs->buffer_alignment) {
 return false;
 }
+if (qiov->iov[i].iov_len % bs->buffer_alignment) {
+return false;
+}
 }
 
 return true;
-- 
1.8.1.4




[Qemu-devel] [PATCH 02/22] block: Inherit opt_transfer_length

2013-12-11 Thread Kevin Wolf
When there is a format driver between the backend, it's not guaranteed
that exposing the opt_transfer_length for the format driver results in
the optimal requests (because of fragmentation etc.), but it can't make
things worse, so let's just do it.

Signed-off-by: Kevin Wolf 
---
 block.c | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index c32d856..79a325f 100644
--- a/block.c
+++ b/block.c
@@ -487,7 +487,23 @@ static int bdrv_refresh_limits(BlockDriverState *bs)
 
 if (!drv) {
 return 0;
-} else if (drv->bdrv_refresh_limits) {
+}
+
+/* Take some limits from the children as a default */
+if (bs->file) {
+bdrv_refresh_limits(bs->file);
+bs->bl.opt_transfer_length = bs->file->bl.opt_transfer_length;
+}
+
+if (bs->backing_hd) {
+bdrv_refresh_limits(bs->backing_hd);
+bs->bl.opt_transfer_length =
+MAX(bs->bl.opt_transfer_length,
+bs->backing_hd->bl.opt_transfer_length);
+}
+
+/* Then let the driver override it */
+if (drv->bdrv_refresh_limits) {
 return drv->bdrv_refresh_limits(bs);
 }
 
-- 
1.8.1.4




[Qemu-devel] [PATCH] target-m68k: Replace qemu_assert by hw_error

2013-12-11 Thread Stefan Weil
hw_error is already used for target-arm and target-s390x.
Using it for target-m68k fixes this compiler warning with Darwin because
hw_error is declared with QEMU_NORETURN:

target-m68k/translate.c:671:13: warning:
 variable 'offset' is used uninitialized whenever switch default is taken
 [-Wsometimes-uninitialized]

Signed-off-by: Stefan Weil 
---
 target-m68k/translate.c |   28 +---
 1 file changed, 9 insertions(+), 19 deletions(-)

diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index f54b94a..259788e 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -110,14 +110,6 @@ void m68k_tcg_init(void)
 store_dummy = tcg_global_mem_new(TCG_AREG0, -8, "NULL");
 }
 
-static inline void qemu_assert(int cond, const char *msg)
-{
-if (!cond) {
-fprintf (stderr, "badness: %s\n", msg);
-abort();
-}
-}
-
 /* internal defines */
 typedef struct DisasContext {
 CPUM68KState *env;
@@ -199,7 +191,7 @@ static inline TCGv gen_load(DisasContext * s, int opsize, 
TCGv addr, int sign)
 tcg_gen_qemu_ld32u(tmp, addr, index);
 break;
 default:
-qemu_assert(0, "bad load size");
+hw_error("%s: Bad load size\n", __func__);
 }
 gen_throws_exception = gen_last_qop;
 return tmp;
@@ -233,7 +225,7 @@ static inline void gen_store(DisasContext *s, int opsize, 
TCGv addr, TCGv val)
 tcg_gen_qemu_st32(val, addr, index);
 break;
 default:
-qemu_assert(0, "bad store size");
+hw_error("%s: Bad store size\n", __func__);
 }
 gen_throws_exception = gen_last_qop;
 }
@@ -437,8 +429,7 @@ static inline int opsize_bytes(int opsize)
 case OS_SINGLE: return 4;
 case OS_DOUBLE: return 8;
 default:
-qemu_assert(0, "bad operand size");
-return 0;
+hw_error("%s: Bad operand size\n", __func__);
 }
 }
 
@@ -465,8 +456,7 @@ static void gen_partset_reg(int opsize, TCGv reg, TCGv val)
 tcg_gen_mov_i32(reg, val);
 break;
 default:
-qemu_assert(0, "Bad operand size");
-break;
+hw_error("%s: Bad operand size\n", __func__);
 }
 }
 
@@ -495,7 +485,7 @@ static inline TCGv gen_extend(TCGv val, int opsize, int 
sign)
 tmp = val;
 break;
 default:
-qemu_assert(0, "Bad operand size");
+hw_error("%s: Bad operand size\n", __func__);
 }
 return tmp;
 }
@@ -669,7 +659,7 @@ static TCGv gen_ea(CPUM68KState *env, DisasContext *s, 
uint16_t insn,
 offset = read_im32(env, s);
 break;
 default:
-qemu_assert(0, "Bad immediate operand");
+hw_error("%s: Bad immediate operand\n", __func__);
 }
 return tcg_const_i32(offset);
 default:
@@ -2092,7 +2082,7 @@ DISAS_INSN(wdebug)
 return;
 }
 /* TODO: Implement wdebug.  */
-qemu_assert(0, "WDEBUG not implemented");
+hw_error("%s: Not implemented\n", __func__);
 }
 
 DISAS_INSN(trap)
@@ -2467,13 +2457,13 @@ DISAS_INSN(fbcc)
 DISAS_INSN(frestore)
 {
 /* TODO: Implement frestore.  */
-qemu_assert(0, "FRESTORE not implemented");
+hw_error("%s: Not implemented\n", __func__);
 }
 
 DISAS_INSN(fsave)
 {
 /* TODO: Implement fsave.  */
-qemu_assert(0, "FSAVE not implemented");
+hw_error("%s: Not implemented\n", __func__);
 }
 
 static inline TCGv gen_mac_extract_word(DisasContext *s, TCGv val, int upper)
-- 
1.7.10.4




[Qemu-devel] A question about virtio inside qemu

2013-12-11 Thread Yaodong Yang
Hi community,I have a quick question about the virtio inside qemu. When the user application sends to a specific virtual disk a large number of read requests in a very short time, where should these requests be queued? Inside the virtqueue? virtqueue available ring or the underlying bdrv_read_aio()?Currently, I run one guest vm with two virtual disks (one for ubuntu system, the other is to serve the data access.) When the system is running, I can check there are for instance 50 inflight requests to read or write the system virtual disk. However, the inflight requests to the data virtual disk is always one. Which is not reasonable, because inside the vm we calculated the avery response time for each request is much larger than the time interval between two adjacent requests.My ultimate goal is to get the number of in-flight requests for a certain virtual disk. Could someone give me some hints?Thanks a lot! I appreciate your help!Yaodong  

Re: [Qemu-devel] [V2 PATCH 11/18] softfloat: Fix float64_to_uint32

2013-12-11 Thread Tom Musta
On 12/11/2013 1:53 PM, Peter Maydell wrote:
> On 11 December 2013 19:16, Tom Musta  wrote:
>>  uint32 float64_to_uint32( float64 a STATUS_PARAM )
>>  {
>> -int64_t v;
>> +uint64_t v;
>>  uint32 res;
>>
>> -v = float64_to_int64(a STATUS_VAR);
>> -if (v < 0) {
>> -res = 0;
>> -float_raise( float_flag_invalid STATUS_VAR);
>> -} else if (v > 0x) {
>> +v = float64_to_uint64(a STATUS_VAR);
>> +if (v > 0x) {
>>  res = 0x;
>> +STATUS(float_exception_flags) &= ~float_flag_inexact;
> 
> The IEEE exception flags are cumulative (ie never get cleared
> except by guest program explicit request); this change means
> that if a previous operation raised the inexact flag you've just
> lost that.
> 
> thanks
> -- PMM
> 
Thank you, Peter.  I will fix.




[Qemu-devel] [PATCH] block/vvfat: Fix compiler warnings for OpenBSD

2013-12-11 Thread Stefan Weil
The buildbot shows these compiler warnings:

block/vvfat.c: In function 'create_short_and_long_name':
block/vvfat.c:620: warning: array size (8) smaller than bound length (11)
block/vvfat.c:620: warning: array size (8) smaller than bound length (11)
block/vvfat.c:635: warning: array size (8) smaller than bound length (11)
block/vvfat.c:635: warning: array size (8) smaller than bound length (11)

They are caused by tricky code where 8 characters for the name are followed
by 3 characters for the extension, and some operations touch both name and
extension.

Using an 11 character name which includes the extension fixes the compiler
warning, satisfies cppcheck, valgrind and maybe other static and dynamic
code checkers, and even simplifies some parts of the code.

Signed-off-by: Stefan Weil 
---
 block/vvfat.c |   43 ++-
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index 3ddaa0b..1abb8ad 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -266,8 +266,7 @@ typedef struct mbr_t {
 } QEMU_PACKED mbr_t;
 
 typedef struct direntry_t {
-uint8_t name[8];
-uint8_t extension[3];
+uint8_t name[8 + 3];
 uint8_t attributes;
 uint8_t reserved[2];
 uint16_t ctime;
@@ -518,11 +517,9 @@ static inline uint8_t fat_chksum(const direntry_t* entry)
 uint8_t chksum=0;
 int i;
 
-for(i=0;i<11;i++) {
-unsigned char c;
-
-c = (i < 8) ? entry->name[i] : entry->extension[i-8];
-chksum=(((chksum&0xfe)>>1)|((chksum&0x01)?0x80:0)) + c;
+for (i = 0; i < ARRAY_SIZE(entry->name); i++) {
+chksum = (((chksum & 0xfe) >> 1) |
+  ((chksum & 0x01) ? 0x80 : 0)) + entry->name[i];
 }
 
 return chksum;
@@ -617,7 +614,7 @@ static inline direntry_t* 
create_short_and_long_name(BDRVVVFATState* s,
 
 if(is_dot) {
entry=array_get_next(&(s->directory));
-   memset(entry->name,0x20,11);
+memset(entry->name, 0x20, sizeof(entry->name));
memcpy(entry->name,filename,strlen(filename));
return entry;
 }
@@ -632,12 +629,14 @@ static inline direntry_t* 
create_short_and_long_name(BDRVVVFATState* s,
i = 8;
 
 entry=array_get_next(&(s->directory));
-memset(entry->name,0x20,11);
+memset(entry->name, 0x20, sizeof(entry->name));
 memcpy(entry->name, filename, i);
 
-if(j > 0)
-   for (i = 0; i < 3 && filename[j+1+i]; i++)
-   entry->extension[i] = filename[j+1+i];
+if (j > 0) {
+for (i = 0; i < 3 && filename[j + 1 + i]; i++) {
+entry->name[8 + i] = filename[j + 1 + i];
+}
+}
 
 /* upcase & remove unwanted characters */
 for(i=10;i>=0;i--) {
@@ -861,8 +860,7 @@ static int init_directories(BDRVVVFATState* s,
 {
direntry_t* entry=array_get_next(&(s->directory));
entry->attributes=0x28; /* archive | volume label */
-   memcpy(entry->name,"QEMU VVF",8);
-   memcpy(entry->extension,"AT ",3);
+memcpy(entry->name, "QEMU VVFAT ", sizeof(entry->name));
 }
 
 /* Now build FAT, and write back information into directory */
@@ -1591,17 +1589,20 @@ static int parse_short_name(BDRVVVFATState* s,
lfn->name[i] = direntry->name[i];
 }
 
-for (j = 2; j >= 0 && direntry->extension[j] == ' '; j--);
+for (j = 2; j >= 0 && direntry->name[8 + j] == ' '; j--) {
+}
 if (j >= 0) {
lfn->name[i++] = '.';
lfn->name[i + j + 1] = '\0';
for (;j >= 0; j--) {
-   if (direntry->extension[j] <= ' ' || direntry->extension[j] > 0x7f)
-   return -2;
-   else if (s->downcase_short_names)
-   lfn->name[i + j] = qemu_tolower(direntry->extension[j]);
-   else
-   lfn->name[i + j] = direntry->extension[j];
+uint8_t c = direntry->name[8 + j];
+if (c <= ' ' || c > 0x7f) {
+return -2;
+} else if (s->downcase_short_names) {
+lfn->name[i + j] = qemu_tolower(c);
+} else {
+lfn->name[i + j] = c;
+}
}
 } else
lfn->name[i + j + 1] = '\0';
-- 
1.7.10.4




[Qemu-devel] [PATCH v3 07/21] blkdebug: Use command-line in read_config()

2013-12-11 Thread Max Reitz
Use qemu_config_parse_qdict() to parse the command-line options in
addition to the config file.

Signed-off-by: Max Reitz 
---
 block/blkdebug.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index f7c898f..584b1d6 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -271,11 +271,13 @@ static void remove_rule(BlkdebugRule *rule)
 g_free(rule);
 }
 
-static int read_config(BDRVBlkdebugState *s, const char *filename, Error 
**errp)
+static int read_config(BDRVBlkdebugState *s, const char *filename,
+   QDict *options, Error **errp)
 {
 FILE *f = NULL;
 int ret;
 struct add_rule_data d;
+Error *local_err = NULL;
 
 if (filename) {
 f = fopen(filename, "r");
@@ -292,6 +294,13 @@ static int read_config(BDRVBlkdebugState *s, const char 
*filename, Error **errp)
 }
 }
 
+qemu_config_parse_qdict(options, config_groups, &local_err);
+if (error_is_set(&local_err)) {
+error_propagate(errp, local_err);
+ret = -EINVAL;
+goto fail;
+}
+
 d.s = s;
 d.action = ACTION_INJECT_ERROR;
 qemu_opts_foreach(&inject_error_opts, add_rule, &d, 0);
@@ -376,9 +385,9 @@ static int blkdebug_open(BlockDriverState *bs, QDict 
*options, int flags,
 goto fail;
 }
 
-/* Read rules from config file */
+/* Read rules from config file or command line options */
 config = qemu_opt_get(opts, "config");
-ret = read_config(s, config, errp);
+ret = read_config(s, config, options, errp);
 if (ret) {
 goto fail;
 }
-- 
1.8.5.1




[Qemu-devel] [V2 PATCH 02/18] target-ppc: Add ISA2.06 bpermd Instruction

2013-12-11 Thread Tom Musta
This patch adds the Bit Permute Doubleword (bpermd) instruction,
which was introduced in Power ISA 2.06 as part of the base 64-bit
architecture.

V2: Addressing stylistic comments from Richard Henderson.

Signed-off-by: Tom Musta 
---
 target-ppc/helper.h |1 +
 target-ppc/int_helper.c |   20 
 target-ppc/translate.c  |   10 ++
 3 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 6250eba..1ec9c65 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -41,6 +41,7 @@ DEF_HELPER_3(sraw, tl, env, tl, tl)
 #if defined(TARGET_PPC64)
 DEF_HELPER_FLAGS_1(cntlzd, TCG_CALL_NO_RWG_SE, tl, tl)
 DEF_HELPER_FLAGS_1(popcntd, TCG_CALL_NO_RWG_SE, tl, tl)
+DEF_HELPER_3(bpermd, i64, env, i64, i64)
 DEF_HELPER_3(srad, tl, env, tl, tl)
 #endif
 
diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index e50bdd2..abc69a7 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -53,6 +53,26 @@ target_ulong helper_cntlzd(target_ulong t)
 }
 #endif
 
+#if defined(TARGET_PPC64)
+
+uint64_t helper_bpermd(CPUPPCState *env, uint64_t rs, uint64_t rb)
+{
+int i;
+uint64_t ra = 0;
+
+for (i = 0; i < 8; i++) {
+int index = (rs >> (i*8)) & 0xFF;
+if (index < 64) {
+if (rb & (1ul << (63-index))) {
+ra |= 1 << i;
+}
+}
+}
+return ra;
+}
+
+#endif
+
 target_ulong helper_cmpb(target_ulong rs, target_ulong rb)
 {
 target_ulong mask = 0xff;
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 1f7e499..0d39de2 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -1525,6 +1525,15 @@ static void gen_prtyd(DisasContext *ctx)
 #endif
 
 #if defined(TARGET_PPC64)
+/* bpermd */
+static void gen_bpermd(DisasContext *ctx)
+{
+gen_helper_bpermd(cpu_gpr[rA(ctx->opcode)], cpu_env,
+  cpu_gpr[rS(ctx->opcode)], cpu_gpr[rB(ctx->opcode)]);
+}
+#endif
+
+#if defined(TARGET_PPC64)
 /* extsw & extsw. */
 GEN_LOGICAL1(extsw, tcg_gen_ext32s_tl, 0x1E, PPC_64B);
 
@@ -9322,6 +9331,7 @@ GEN_HANDLER_E(prtyw, 0x1F, 0x1A, 0x04, 0xF801, 
PPC_NONE, PPC2_ISA205),
 GEN_HANDLER(popcntd, 0x1F, 0x1A, 0x0F, 0xF801, PPC_POPCNTWD),
 GEN_HANDLER(cntlzd, 0x1F, 0x1A, 0x01, 0x, PPC_64B),
 GEN_HANDLER_E(prtyd, 0x1F, 0x1A, 0x05, 0xF801, PPC_NONE, PPC2_ISA205),
+GEN_HANDLER_E(bpermd, 0x1F, 0x1C, 0x07, 0x0001, PPC_NONE, PPC2_ISA206),
 #endif
 GEN_HANDLER(rlwimi, 0x14, 0xFF, 0xFF, 0x, PPC_INTEGER),
 GEN_HANDLER(rlwinm, 0x15, 0xFF, 0xFF, 0x, PPC_INTEGER),
-- 
1.7.1




[Qemu-devel] [V2 PATCH 09/18] softfloat: Fix Handling of Small Negatives in float64_to_uint64

2013-12-11 Thread Tom Musta
The float64_to_uint64 routine exits early for all negative numbers.
While the integer result is always correctly returned as 0, the
exception flags are also always set to float_flag_invalid.  This
is incorrect for those cases where a small negative number (-1 < x < 0)
rounds to zero.  In such a case, the flag should be reported as
inexact.

The following patch allows these small numbers to flow through the
rounding and packing code.

Some interesting test patterns are:

(1) BC6AEEBA7F390215 / -0x1.aeeba7f390215p-57, round to nearest
even should round up to zero (inexact)

(2) A66A44F252C9AAAC /-0x1.a44f252c9aaacp-409 round up should round
up to zero (inexact)

(3) B4692F3AFFAF2716 / -0x1.92f3affaf2716p-185 round down should
be invalid.

Signed-off-by: Tom Musta 
---
 fpu/softfloat.c |   41 +
 1 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index cb03dca..3d7a8ff 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -161,7 +161,6 @@ static int32 roundAndPackInt32( flag zSign, uint64_t absZ 
STATUS_PARAM)
 | exception is raised and the largest positive or negative integer is
 | returned.
 **/
-
 static int64 roundAndPackInt64( flag zSign, uint64_t absZ0, uint64_t absZ1 
STATUS_PARAM)
 {
 int8 roundingMode;
@@ -213,20 +212,24 @@ static int64 roundAndPackInt64( flag zSign, uint64_t 
absZ0, uint64_t absZ1 STATU
 | exception is raised and the largest unsigned integer is returned.
 **/
 
-static int64 roundAndPackUint64(uint64_t absZ0, uint64_t absZ1 STATUS_PARAM)
+static int64 roundAndPackUint64(flag zSign, uint64_t absZ0,
+uint64_t absZ1 STATUS_PARAM)
 {
 int8 roundingMode;
 flag roundNearestEven, increment;
-int64_t z;
 
 roundingMode = STATUS(float_rounding_mode);
 roundNearestEven = (roundingMode == float_round_nearest_even);
-increment = ((int64_t) absZ1 < 0);
+increment = ((int64_t)absZ1 < 0);
 if (!roundNearestEven) {
 if (roundingMode == float_round_to_zero) {
 increment = 0;
-} else {
-increment = (roundingMode == float_round_up) && absZ1;
+} else if (absZ1) {
+if (zSign) {
+increment = (roundingMode == float_round_down) && absZ1;
+} else {
+increment = (roundingMode == float_round_up) && absZ1;
+}
 }
 }
 if (increment) {
@@ -237,11 +240,16 @@ static int64 roundAndPackUint64(uint64_t absZ0, uint64_t 
absZ1 STATUS_PARAM)
 }
 absZ0 &= ~(((uint64_t)(absZ1<<1) == 0) & roundNearestEven);
 }
-z = absZ0;
+
+if (zSign && absZ0) {
+float_raise(float_flag_invalid STATUS_VAR);
+return 0;
+}
+
 if (absZ1) {
 STATUS(float_exception_flags) |= float_flag_inexact;
 }
-return z;
+return absZ0;
 }
 
 /*
@@ -1590,7 +1598,7 @@ uint64 float32_to_uint64(float32 a STATUS_PARAM)
 aSig64 = aSig;
 aSig64 <<= 40;
 shift64ExtraRightJamming(aSig64, 0, shiftCount, &aSig64, &aSigExtra);
-return roundAndPackUint64(aSig64, aSigExtra STATUS_VAR);
+return roundAndPackUint64(aSign, aSig64, aSigExtra STATUS_VAR);
 }
 
 /*
@@ -6643,12 +6651,8 @@ uint64_t float64_to_uint64(float64 a STATUS_PARAM)
 aSig = extractFloat64Frac(a);
 aExp = extractFloat64Exp(a);
 aSign = extractFloat64Sign(a);
-if (aSign) {
-if (aExp) {
-float_raise(float_flag_invalid STATUS_VAR);
-} else if (aSig) { /* negative denormalized */
-float_raise(float_flag_inexact STATUS_VAR);
-}
+if (aSign && (aExp > 1022)) {
+float_raise(float_flag_invalid STATUS_VAR);
 return 0;
 }
 if (aExp) {
@@ -6657,10 +6661,7 @@ uint64_t float64_to_uint64(float64 a STATUS_PARAM)
 shiftCount = 0x433 - aExp;
 if (shiftCount <= 0) {
 if (0x43E < aExp) {
-if ((aSig != LIT64(0x0010)) ||
- (aExp == 0x7FF)) {
-float_raise(float_flag_invalid STATUS_VAR);
-}
+float_raise(float_flag_invalid STATUS_VAR);
 return LIT64(0x);
 }
 aSigExtra = 0;
@@ -6668,7 +6669,7 @@ uint64_t float64_to_uint64(float64 a STATUS_PARAM)
 } else {
 shift64ExtraRightJamming(aSig, 0, shiftCount, &aSig, &aSigExtra);
 }
-return roundAndPackUint64(aSig, aSigExtra STATUS_VAR);
+return roundAndPackUint64(aSign, aSig, aSigExtra STATUS_VAR);
 }
 
 
-- 
1.7.1




[Qemu-devel] [PULL 21/28] hpet: inverse polarity when pin above ISA_NUM_IRQS

2013-12-11 Thread Michael S. Tsirkin
From: Liu Ping Fan 

According to hpet spec, hpet irq is high active. But according to
ICH spec, there is inversion before the input of ioapic. So the OS
will expect low active on this IRQ line. (On bare metal, if OS driver
claims high active on this line, spurious irq is generated)

We fold the emulation of this inversion inside the hpet logic.

Signed-off-by: Liu Ping Fan 
Reviewed-by: Paolo Bonzini 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/timer/hpet.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index 2eb75ea..0aee2c1 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -198,13 +198,23 @@ static void update_irq(struct HPETTimer *timer, int set)
 if (!set || !timer_enabled(timer) || !hpet_enabled(timer->state)) {
 s->isr &= ~mask;
 if (!timer_fsb_route(timer)) {
-qemu_irq_lower(s->irqs[route]);
+/* fold the ICH PIRQ# pin's internal inversion logic into hpet */
+if (route >= ISA_NUM_IRQS) {
+qemu_irq_raise(s->irqs[route]);
+} else {
+qemu_irq_lower(s->irqs[route]);
+}
 }
 } else if (timer_fsb_route(timer)) {
 stl_le_phys(timer->fsb >> 32, timer->fsb & 0x);
 } else if (timer->config & HPET_TN_TYPE_LEVEL) {
 s->isr |= mask;
-qemu_irq_raise(s->irqs[route]);
+/* fold the ICH PIRQ# pin's internal inversion logic into hpet */
+if (route >= ISA_NUM_IRQS) {
+qemu_irq_lower(s->irqs[route]);
+} else {
+qemu_irq_raise(s->irqs[route]);
+}
 } else {
 s->isr &= ~mask;
 qemu_irq_pulse(s->irqs[route]);
-- 
MST




[Qemu-devel] [PATCH 1/5] target-s390: Convert to new qemu_ld/st opcodes

2013-12-11 Thread Richard Henderson
Simplistic search and replace only.  Cleanups to follow.

Signed-off-by: Richard Henderson 
---
 target-s390x/translate.c | 154 +++
 1 file changed, 77 insertions(+), 77 deletions(-)

diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index bc99a37..8fc8259 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -1754,20 +1754,20 @@ static ExitStatus op_clc(DisasContext *s, DisasOps *o)
 
 switch (l + 1) {
 case 1:
-tcg_gen_qemu_ld8u(cc_src, o->addr1, get_mem_index(s));
-tcg_gen_qemu_ld8u(cc_dst, o->in2, get_mem_index(s));
+tcg_gen_qemu_ld_i64(cc_src, o->addr1, get_mem_index(s), MO_UB);
+tcg_gen_qemu_ld_i64(cc_dst, o->in2, get_mem_index(s), MO_UB);
 break;
 case 2:
-tcg_gen_qemu_ld16u(cc_src, o->addr1, get_mem_index(s));
-tcg_gen_qemu_ld16u(cc_dst, o->in2, get_mem_index(s));
+tcg_gen_qemu_ld_i64(cc_src, o->addr1, get_mem_index(s), MO_BEUW);
+tcg_gen_qemu_ld_i64(cc_dst, o->in2, get_mem_index(s), MO_BEUW);
 break;
 case 4:
-tcg_gen_qemu_ld32u(cc_src, o->addr1, get_mem_index(s));
-tcg_gen_qemu_ld32u(cc_dst, o->in2, get_mem_index(s));
+tcg_gen_qemu_ld_i64(cc_src, o->addr1, get_mem_index(s), MO_BEUL);
+tcg_gen_qemu_ld_i64(cc_dst, o->in2, get_mem_index(s), MO_BEUL);
 break;
 case 8:
-tcg_gen_qemu_ld64(cc_src, o->addr1, get_mem_index(s));
-tcg_gen_qemu_ld64(cc_dst, o->in2, get_mem_index(s));
+tcg_gen_qemu_ld_i64(cc_src, o->addr1, get_mem_index(s), MO_BEQ);
+tcg_gen_qemu_ld_i64(cc_dst, o->in2, get_mem_index(s), MO_BEQ);
 break;
 default:
 potential_page_fault(s);
@@ -1841,9 +1841,9 @@ static ExitStatus op_cs(DisasContext *s, DisasOps *o)
means that R1 is equal to the memory in all conditions.  */
 addr = get_address(s, 0, b2, d2);
 if (is_64) {
-tcg_gen_qemu_ld64(o->out, addr, get_mem_index(s));
+tcg_gen_qemu_ld_i64(o->out, addr, get_mem_index(s), MO_BEQ);
 } else {
-tcg_gen_qemu_ld32u(o->out, addr, get_mem_index(s));
+tcg_gen_qemu_ld_i64(o->out, addr, get_mem_index(s), MO_BEUL);
 }
 
 /* Are the memory and expected values (un)equal?  Note that this setcond
@@ -1859,9 +1859,9 @@ static ExitStatus op_cs(DisasContext *s, DisasOps *o)
 mem = tcg_temp_new_i64();
 tcg_gen_movcond_i64(TCG_COND_EQ, mem, cc, z, o->in1, o->out);
 if (is_64) {
-tcg_gen_qemu_st64(mem, addr, get_mem_index(s));
+tcg_gen_qemu_st_i64(mem, addr, get_mem_index(s), MO_BEQ);
 } else {
-tcg_gen_qemu_st32(mem, addr, get_mem_index(s));
+tcg_gen_qemu_st_i64(mem, addr, get_mem_index(s), MO_BEUL);
 }
 tcg_temp_free_i64(z);
 tcg_temp_free_i64(mem);
@@ -1891,8 +1891,8 @@ static ExitStatus op_cdsg(DisasContext *s, DisasOps *o)
 outh = tcg_temp_new_i64();
 outl = tcg_temp_new_i64();
 
-tcg_gen_qemu_ld64(outh, addrh, get_mem_index(s));
-tcg_gen_qemu_ld64(outl, addrl, get_mem_index(s));
+tcg_gen_qemu_ld_i64(outh, addrh, get_mem_index(s), MO_BEQ);
+tcg_gen_qemu_ld_i64(outl, addrl, get_mem_index(s), MO_BEQ);
 
 /* Fold the double-word compare with arithmetic.  */
 cc = tcg_temp_new_i64();
@@ -1909,8 +1909,8 @@ static ExitStatus op_cdsg(DisasContext *s, DisasOps *o)
 tcg_gen_movcond_i64(TCG_COND_EQ, meml, cc, z, regs[r3 + 1], outl);
 tcg_temp_free_i64(z);
 
-tcg_gen_qemu_st64(memh, addrh, get_mem_index(s));
-tcg_gen_qemu_st64(meml, addrl, get_mem_index(s));
+tcg_gen_qemu_st_i64(memh, addrh, get_mem_index(s), MO_BEQ);
+tcg_gen_qemu_st_i64(meml, addrl, get_mem_index(s), MO_BEQ);
 tcg_temp_free_i64(memh);
 tcg_temp_free_i64(meml);
 tcg_temp_free_i64(addrh);
@@ -1946,7 +1946,7 @@ static ExitStatus op_cvd(DisasContext *s, DisasOps *o)
 tcg_gen_trunc_i64_i32(t2, o->in1);
 gen_helper_cvd(t1, t2);
 tcg_temp_free_i32(t2);
-tcg_gen_qemu_st64(t1, o->in2, get_mem_index(s));
+tcg_gen_qemu_st_i64(t1, o->in2, get_mem_index(s), MO_BEQ);
 tcg_temp_free_i64(t1);
 return NO_EXIT;
 }
@@ -2110,7 +2110,7 @@ static ExitStatus op_icm(DisasContext *s, DisasOps *o)
 switch (m3) {
 case 0xf:
 /* Effectively a 32-bit load.  */
-tcg_gen_qemu_ld32u(tmp, o->in2, get_mem_index(s));
+tcg_gen_qemu_ld_i64(tmp, o->in2, get_mem_index(s), MO_BEUL);
 len = 32;
 goto one_insert;
 
@@ -2118,7 +2118,7 @@ static ExitStatus op_icm(DisasContext *s, DisasOps *o)
 case 0x6:
 case 0x3:
 /* Effectively a 16-bit load.  */
-tcg_gen_qemu_ld16u(tmp, o->in2, get_mem_index(s));
+tcg_gen_qemu_ld_i64(tmp, o->in2, get_mem_index(s), MO_BEUW);
 len = 16;
 goto one_insert;
 
@@ -2127,7 +2127,7 @@ static ExitStatus op_icm(DisasContext *s, DisasOps *o)
 case 0x2:
 case 0x1:
 /* Effectively an 8-bit load.  */
-tcg_gen_qemu_ld8u(tmp, o->

[Qemu-devel] [V2 PATCH 05/18] target-ppc: Add ISA 2.06 divwe[u][o] Instructions

2013-12-11 Thread Tom Musta
This patch addes the Signed and Unsigned  Divide Word Extended
instructions which were introduced in Power ISA 2.06.

V2: Eliminating extraneous code in the overflow case per comments
from Richard Henderson.  Fixed corner case bug in divweu (check
for (RA) >= (RB)).

Signed-off-by: Tom Musta 
---
 target-ppc/translate.c |   70 
 1 files changed, 70 insertions(+), 0 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index b274a15..3344fa9 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -984,6 +984,72 @@ GEN_INT_ARITH_DIVW(divwuo, 0x1E, 0, 1);
 /* divw  divw.  divwo  divwo.   */
 GEN_INT_ARITH_DIVW(divw, 0x0F, 1, 0);
 GEN_INT_ARITH_DIVW(divwo, 0x1F, 1, 1);
+
+#define GEN_DIVWE(op, signed, compute_ov) \
+static void gen_##op(DisasContext *ctx)   \
+{ \
+/* Need to use local temps because of the branches */ \
+TCGv ra = tcg_temp_local_new();   \
+TCGv rb = tcg_temp_local_new();   \
+int lbl_ov = gen_new_label(); \
+int lbl_rc = gen_new_label(); \
+  \
+if (signed) { \
+TCGv tmp0;\
+/* divide by zero ? */\
+tcg_gen_ext32s_i64(rb, cpu_gpr[rB(ctx->opcode)]); \
+tcg_gen_brcondi_i64(TCG_COND_EQ, rb, 0, lbl_ov);  \
+tcg_gen_shli_i64(ra, cpu_gpr[rA(ctx->opcode)], 32);   \
+/* check for MIN div -1 */\
+int l3 = gen_new_label(); \
+tcg_gen_brcondi_i64(TCG_COND_NE, rb, -1l, l3);\
+tcg_gen_brcondi_i64(TCG_COND_EQ, ra, INT64_MIN, lbl_ov);  \
+gen_set_label(l3);\
+tcg_gen_div_i64(cpu_gpr[rD(ctx->opcode)], ra, rb);\
+tmp0 = tcg_temp_local_new();  \
+/* does the result fit in 32 bits? */ \
+tcg_gen_ext32s_i64(tmp0, cpu_gpr[rD(ctx->opcode)]);   \
+tcg_gen_brcond_i64(TCG_COND_NE, cpu_gpr[rD(ctx->opcode)], tmp0,   \
+   lbl_ov);   \
+tcg_temp_free(tmp0);  \
+} else { /* unsigned */   \
+/* divide by zero ? */\
+tcg_gen_ext32u_i64(rb, cpu_gpr[rB(ctx->opcode)]); \
+tcg_gen_brcondi_i64(TCG_COND_EQ, rb, 0, lbl_ov);  \
+/* is ra[32:63] >= rb[32:63] ? */ \
+tcg_gen_ext32u_i64(ra, cpu_gpr[rA(ctx->opcode)]); \
+tcg_gen_brcond_i64(TCG_COND_GEU, ra, rb, lbl_ov); \
+tcg_gen_shli_i64(ra, cpu_gpr[rA(ctx->opcode)], 32);   \
+tcg_gen_divu_i64(cpu_gpr[rD(ctx->opcode)], ra, rb);   \
+} \
+  \
+if (compute_ov) { \
+tcg_gen_movi_tl(cpu_ov, 0);   \
+} \
+tcg_gen_br(lbl_rc);   \
+  \
+gen_set_label(lbl_ov); /* overflow handling */\
+tcg_gen_movi_i64(cpu_gpr[rD(ctx->opcode)], 0);\
+  \
+if (compute_ov) { \
+tcg_gen_movi_tl(cpu_ov, 1);   \
+tcg_gen_movi_tl(cpu_so, 1);   \
+} \
+  \
+gen_set_label(lbl_rc);\
+if (unlikely(Rc(ctx->opco

Re: [Qemu-devel] [PATCH 5/11 v3 FIXED] qdev: add "hotplugable" property to Device

2013-12-11 Thread Markus Armbruster
Please spell it "pluggable", both in C identifiers and strings.



[Qemu-devel] [PATCH v3 05/21] qemu-option: Add qemu_config_parse_qdict()

2013-12-11 Thread Max Reitz
This function basically parses command-line options given as a QDict
replacing a config file.

For instance, the QDict {"section.opt1": 42, "section.opt2": 23}
corresponds to the config file:

[section]
opt1 = 42
opt2 = 23

It is possible to specify multiple sections and also multiple sections
of the same type. On the command line, this looks like the following:

inject-error.0.event=reftable_load,\
inject-error.1.event=l2_load,\
set-state.event=l1_update

This would correspond to the following config file:

[inject-error "inject-error.0"]
event = reftable_load

[inject-error "inject-error.1"]
event = l2_load

[set-state]
event = l1_update

Signed-off-by: Max Reitz 
---
 include/qemu/config-file.h |  6 +++
 util/qemu-config.c | 95 ++
 2 files changed, 101 insertions(+)

diff --git a/include/qemu/config-file.h b/include/qemu/config-file.h
index 508428f..dbd97c4 100644
--- a/include/qemu/config-file.h
+++ b/include/qemu/config-file.h
@@ -4,6 +4,7 @@
 #include 
 #include "qemu/option.h"
 #include "qapi/error.h"
+#include "qapi/qmp/qdict.h"
 
 QemuOptsList *qemu_find_opts(const char *group);
 QemuOptsList *qemu_find_opts_err(const char *group, Error **errp);
@@ -18,6 +19,11 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const 
char *fname);
 
 int qemu_read_config_file(const char *filename);
 
+/* Parse QDict options as a replacement for a config file (allowing multiple
+   enumerated (0..(n-1)) configuration "sections") */
+void qemu_config_parse_qdict(QDict *options, QemuOptsList **lists,
+ Error **errp);
+
 /* Read default QEMU config files
  */
 int qemu_read_default_config_files(bool userconfig);
diff --git a/util/qemu-config.c b/util/qemu-config.c
index 04da942..12178d4 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -356,3 +356,98 @@ int qemu_read_config_file(const char *filename)
 return -EINVAL;
 }
 }
+
+static void config_parse_qdict_section(QDict *options, QemuOptsList *opts,
+   Error **errp)
+{
+QemuOpts *subopts;
+QDict *subqdict;
+QList *list = NULL;
+Error *local_err = NULL;
+size_t orig_size, enum_size;
+char *prefix;
+
+prefix = g_strdup_printf("%s.", opts->name);
+qdict_extract_subqdict(options, &subqdict, prefix);
+g_free(prefix);
+orig_size = qdict_size(subqdict);
+if (!orig_size) {
+goto out;
+}
+
+subopts = qemu_opts_create_nofail(opts);
+qemu_opts_absorb_qdict(subopts, subqdict, &local_err);
+if (error_is_set(&local_err)) {
+error_propagate(errp, local_err);
+goto out;
+}
+
+enum_size = qdict_size(subqdict);
+if (enum_size < orig_size && enum_size) {
+error_setg(errp, "Unknown option '%s' for [%s]",
+   qdict_first(subqdict)->key, opts->name);
+goto out;
+}
+
+if (enum_size) {
+/* Multiple, enumerated sections */
+QListEntry *list_entry;
+unsigned i = 0;
+
+/* Not required anymore */
+qemu_opts_del(subopts);
+
+qdict_array_split(subqdict, &list);
+if (qdict_size(subqdict)) {
+error_setg(errp, "Unused option '%s' for [%s]",
+   qdict_first(subqdict)->key, opts->name);
+goto out;
+}
+
+QLIST_FOREACH_ENTRY(list, list_entry) {
+QDict *section = qobject_to_qdict(qlist_entry_obj(list_entry));
+char *opt_name;
+
+opt_name = g_strdup_printf("%s.%u", opts->name, i++);
+subopts = qemu_opts_create(opts, opt_name, 1, &local_err);
+g_free(opt_name);
+if (error_is_set(&local_err)) {
+error_propagate(errp, local_err);
+goto out;
+}
+
+qemu_opts_absorb_qdict(subopts, section, &local_err);
+if (error_is_set(&local_err)) {
+error_propagate(errp, local_err);
+qemu_opts_del(subopts);
+goto out;
+}
+
+if (qdict_size(section)) {
+error_setg(errp, "[%s] section doesn't support the option 
'%s'",
+   opts->name, qdict_first(section)->key);
+qemu_opts_del(subopts);
+goto out;
+}
+}
+}
+
+out:
+QDECREF(subqdict);
+QDECREF(list);
+}
+
+void qemu_config_parse_qdict(QDict *options, QemuOptsList **lists,
+ Error **errp)
+{
+int i;
+Error *local_err = NULL;
+
+for (i = 0; lists[i]; i++) {
+config_parse_qdict_section(options, lists[i], &local_err);
+if (error_is_set(&local_err)) {
+error_propagate(errp, local_err);
+return;
+}
+}
+}
-- 
1.8.5.1




Re: [Qemu-devel] [V2 PATCH 12/18] softfloat: Fix float64_to_uint32_round_to_zero

2013-12-11 Thread Peter Maydell
On 11 December 2013 19:16, Tom Musta  wrote:
> The float64_to_uint32_round_to_zero routine is incorrect.
>
> For example, the following test pattern:
>
> 425F81378DC0CD1F / 0x1.f81378dc0cd1fp+38
>
> will erroneously set the inexact flag.
>
> This patch re-implements the routine to temporarily force the
> rounding mode and use the float64_to_uint32 routine.

>  uint32 float64_to_uint32_round_to_zero( float64 a STATUS_PARAM )
>  {
> -int64_t v;
> +uint64_t v;
>  uint32 res;
>
> -v = float64_to_int64_round_to_zero(a STATUS_VAR);
> -if (v < 0) {
> -res = 0;
> -float_raise( float_flag_invalid STATUS_VAR);
> -} else if (v > 0x) {
> +v = float64_to_uint64_round_to_zero(a STATUS_VAR);
> +if (v > 0x) {
>  res = 0x;
> +STATUS(float_exception_flags) &= ~float_flag_inexact;
>  float_raise( float_flag_invalid STATUS_VAR);
>  } else {
>  res = v;

The patch doesn't seem to be doing what the commit message
says it does?

thanks
-- PMM



[Qemu-devel] [V2 PATCH 17/18] target-ppc: Enable frsqrtes on Power7 and Power8

2013-12-11 Thread Tom Musta
The frsqrtes instruction was introduced prior to ISA 2.06 and is
support on both the Power7 and Power8 processors.  However, this
instruction is handled as illegal in the current QEMU emulation
machines.  This patch enables the existing implemention of frsqrtes
in the P7 and P8 machines.

Signed-off-by: Tom Musta 
---
 target-ppc/translate_init.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 7bb9bbc..ec65bf4 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -7227,6 +7227,7 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
 pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
+   PPC_FLOAT_FRSQRTES |
PPC_FLOAT_STFIWX |
PPC_FLOAT_EXT |
PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
@@ -7265,6 +7266,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
 pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
+   PPC_FLOAT_FRSQRTES |
PPC_FLOAT_STFIWX |
PPC_FLOAT_EXT |
PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
-- 
1.7.1




Re: [Qemu-devel] [V2 PATCH 11/18] softfloat: Fix float64_to_uint32

2013-12-11 Thread Peter Maydell
On 11 December 2013 19:16, Tom Musta  wrote:
>  uint32 float64_to_uint32( float64 a STATUS_PARAM )
>  {
> -int64_t v;
> +uint64_t v;
>  uint32 res;
>
> -v = float64_to_int64(a STATUS_VAR);
> -if (v < 0) {
> -res = 0;
> -float_raise( float_flag_invalid STATUS_VAR);
> -} else if (v > 0x) {
> +v = float64_to_uint64(a STATUS_VAR);
> +if (v > 0x) {
>  res = 0x;
> +STATUS(float_exception_flags) &= ~float_flag_inexact;

The IEEE exception flags are cumulative (ie never get cleared
except by guest program explicit request); this change means
that if a previous operation raised the inexact flag you've just
lost that.

thanks
-- PMM



Re: [Qemu-devel] [PATCH] [PATCH RFC v4] qemu-monitor: HMP cpu-add wrapper

2013-12-11 Thread Jason J. Herne

On 12/11/2013 01:24 PM, Jason J. Herne wrote:

I forgot to modify the subject line to remove "[PATCH RFC v4]".  My 
apologies!


--
-- Jason J. Herne (jjhe...@linux.vnet.ibm.com)




[Qemu-devel] [PATCH 2/5] target-s390: Simplify op_clc

2013-12-11 Thread Richard Henderson
New opcodes can unify 4 different code paths.

Signed-off-by: Richard Henderson 
---
 target-s390x/translate.c | 30 +-
 1 file changed, 9 insertions(+), 21 deletions(-)

diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 8fc8259..8f8567e 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -1750,34 +1750,22 @@ static ExitStatus op_cksm(DisasContext *s, DisasOps *o)
 static ExitStatus op_clc(DisasContext *s, DisasOps *o)
 {
 int l = get_field(s->fields, l1);
-TCGv_i32 vl;
 
-switch (l + 1) {
-case 1:
-tcg_gen_qemu_ld_i64(cc_src, o->addr1, get_mem_index(s), MO_UB);
-tcg_gen_qemu_ld_i64(cc_dst, o->in2, get_mem_index(s), MO_UB);
-break;
-case 2:
-tcg_gen_qemu_ld_i64(cc_src, o->addr1, get_mem_index(s), MO_BEUW);
-tcg_gen_qemu_ld_i64(cc_dst, o->in2, get_mem_index(s), MO_BEUW);
-break;
-case 4:
-tcg_gen_qemu_ld_i64(cc_src, o->addr1, get_mem_index(s), MO_BEUL);
-tcg_gen_qemu_ld_i64(cc_dst, o->in2, get_mem_index(s), MO_BEUL);
-break;
-case 8:
-tcg_gen_qemu_ld_i64(cc_src, o->addr1, get_mem_index(s), MO_BEQ);
-tcg_gen_qemu_ld_i64(cc_dst, o->in2, get_mem_index(s), MO_BEQ);
-break;
-default:
+if (l + 1 <= 8) {
+TCGMemOp mop = MO_BE + ctz32(l + 1);
+
+tcg_gen_qemu_ld_i64(cc_src, o->addr1, get_mem_index(s), mop);
+tcg_gen_qemu_ld_i64(cc_dst, o->in2, get_mem_index(s), mop);
+gen_op_update2_cc_i64(s, CC_OP_LTUGTU_64, cc_src, cc_dst);
+} else {
+TCGv_i32 vl;
+
 potential_page_fault(s);
 vl = tcg_const_i32(l);
 gen_helper_clc(cc_op, cpu_env, vl, o->addr1, o->in2);
 tcg_temp_free_i32(vl);
 set_cc_static(s);
-return NO_EXIT;
 }
-gen_op_update2_cc_i64(s, CC_OP_LTUGTU_64, cc_src, cc_dst);
 return NO_EXIT;
 }
 
-- 
1.8.3.1




Re: [Qemu-devel] [V2 PATCH 00/18] target-ppc: Base ISA V2.06 for Power7/Power8

2013-12-11 Thread Peter Maydell
On 11 December 2013 19:42, Tom Musta  wrote:
> On 12/11/2013 1:40 PM, Peter Maydell wrote:
>> All patches which touch fpu/ need to come with a statement
>> (in the cover letter is fine) that the changes are licensed under
>> both the softfloat-2a and -2b licenses [or under some license
>> compatible with both, I guess]. (We're trying to relicense that
>> area of the codebase at the moment.)
>
> Aghhh   I should have remembered that.  Sucks that this stuff
> can't be consistent with the rest of the code.

A reply here will do, you don't need to respin just for this.

thanks
-- PMM



[Qemu-devel] [PATCH 3/5] target-s390: Simplify op_cs, op_soc, op_stm

2013-12-11 Thread Richard Henderson
Unifying 2 different code paths each.

Signed-off-by: Richard Henderson 
---
 target-s390x/translate.c | 28 +++-
 1 file changed, 7 insertions(+), 21 deletions(-)

diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 8f8567e..3e88c23 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -1818,7 +1818,7 @@ static ExitStatus op_cs(DisasContext *s, DisasOps *o)
 /* FIXME: needs an atomic solution for CONFIG_USER_ONLY.  */
 int d2 = get_field(s->fields, d2);
 int b2 = get_field(s->fields, b2);
-int is_64 = s->insn->data;
+TCGMemOp mop = s->insn->data ? MO_BEQ : MO_BEUL;
 TCGv_i64 addr, mem, cc, z;
 
 /* Note that in1 = R3 (new value) and
@@ -1828,11 +1828,7 @@ static ExitStatus op_cs(DisasContext *s, DisasOps *o)
about moving the memory to R1 on inequality, if we include equality it
means that R1 is equal to the memory in all conditions.  */
 addr = get_address(s, 0, b2, d2);
-if (is_64) {
-tcg_gen_qemu_ld_i64(o->out, addr, get_mem_index(s), MO_BEQ);
-} else {
-tcg_gen_qemu_ld_i64(o->out, addr, get_mem_index(s), MO_BEUL);
-}
+tcg_gen_qemu_ld_i64(o->out, addr, get_mem_index(s), mop);
 
 /* Are the memory and expected values (un)equal?  Note that this setcond
produces the output CC value, thus the NE sense of the test.  */
@@ -1846,11 +1842,7 @@ static ExitStatus op_cs(DisasContext *s, DisasOps *o)
 z = tcg_const_i64(0);
 mem = tcg_temp_new_i64();
 tcg_gen_movcond_i64(TCG_COND_EQ, mem, cc, z, o->in1, o->out);
-if (is_64) {
-tcg_gen_qemu_st_i64(mem, addr, get_mem_index(s), MO_BEQ);
-} else {
-tcg_gen_qemu_st_i64(mem, addr, get_mem_index(s), MO_BEUL);
-}
+tcg_gen_qemu_st_i64(mem, addr, get_mem_index(s), mop);
 tcg_temp_free_i64(z);
 tcg_temp_free_i64(mem);
 tcg_temp_free_i64(addr);
@@ -2986,6 +2978,7 @@ static ExitStatus op_sigp(DisasContext *s, DisasOps *o)
 
 static ExitStatus op_soc(DisasContext *s, DisasOps *o)
 {
+TCGMemOp mop = s->insn->data ? MO_BEQ : MO_BEUL;
 DisasCompare c;
 TCGv_i64 a;
 int lab, r1;
@@ -3002,11 +2995,7 @@ static ExitStatus op_soc(DisasContext *s, DisasOps *o)
 
 r1 = get_field(s->fields, r1);
 a = get_address(s, 0, get_field(s->fields, b2), get_field(s->fields, d2));
-if (s->insn->data) {
-tcg_gen_qemu_st_i64(regs[r1], a, get_mem_index(s), MO_BEQ);
-} else {
-tcg_gen_qemu_st_i64(regs[r1], a, get_mem_index(s), MO_BEUL);
-}
+tcg_gen_qemu_st_i64(regs[r1], a, get_mem_index(s), mop);
 tcg_temp_free_i64(a);
 
 gen_set_label(lab);
@@ -3387,14 +3376,11 @@ static ExitStatus op_stm(DisasContext *s, DisasOps *o)
 int r1 = get_field(s->fields, r1);
 int r3 = get_field(s->fields, r3);
 int size = s->insn->data;
+TCGMemOp mop = size == 8 ? MO_BEQ : MO_BEUL;
 TCGv_i64 tsize = tcg_const_i64(size);
 
 while (1) {
-if (size == 8) {
-tcg_gen_qemu_st_i64(regs[r1], o->in2, get_mem_index(s), MO_BEQ);
-} else {
-tcg_gen_qemu_st_i64(regs[r1], o->in2, get_mem_index(s), MO_BEUL);
-}
+tcg_gen_qemu_st_i64(regs[r1], o->in2, get_mem_index(s), mop);
 if (r1 == r3) {
 break;
 }
-- 
1.8.3.1




[Qemu-devel] [PULL 06/28] pci: fix address space size for bridge

2013-12-11 Thread Michael S. Tsirkin
Address space size for bridge should be full 64 bit,
so we should use UINT64_MAX not INT64_MAX as it's size.

Signed-off-by: Michael S. Tsirkin 
---
 hw/pci/pci_bridge.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index 290abab..f72872e 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -372,7 +372,7 @@ int pci_bridge_initfn(PCIDevice *dev, const char *typename)
 sec_bus->parent_dev = dev;
 sec_bus->map_irq = br->map_irq ? br->map_irq : pci_swizzle_map_irq_fn;
 sec_bus->address_space_mem = &br->address_space_mem;
-memory_region_init(&br->address_space_mem, OBJECT(br), "pci_bridge_pci", 
INT64_MAX);
+memory_region_init(&br->address_space_mem, OBJECT(br), "pci_bridge_pci", 
UINT64_MAX);
 sec_bus->address_space_io = &br->address_space_io;
 memory_region_init(&br->address_space_io, OBJECT(br), "pci_bridge_io", 
65536);
 br->windows = pci_bridge_region_init(br);
-- 
MST




Re: [Qemu-devel] [V2 PATCH 00/18] target-ppc: Base ISA V2.06 for Power7/Power8

2013-12-11 Thread Tom Musta
On 12/11/2013 1:40 PM, Peter Maydell wrote:
> On 11 December 2013 19:16, Tom Musta  wrote:
>> Additionally, some bugs in the common floating point library (softfloat)
>> are fixed.
> 
>>  fpu/softfloat.c |   73 +--
> 
> All patches which touch fpu/ need to come with a statement
> (in the cover letter is fine) that the changes are licensed under
> both the softfloat-2a and -2b licenses [or under some license
> compatible with both, I guess]. (We're trying to relicense that
> area of the codebase at the moment.)
> 
> thanks
> -- PMM
> 

Aghhh   I should have remembered that.  Sucks that this stuff
can't be consistent with the rest of the code.

V3 forthcoming.




[Qemu-devel] [PULL 26/28] acpi unit-test: adjust the test data structure for better handling

2013-12-11 Thread Michael S. Tsirkin
From: Marcel Apfelbaum 

Ensure more then one instance of test_data may exist
at a given time. It will help to compare different
acpi table versions.

Signed-off-by: Marcel Apfelbaum 
Signed-off-by: Michael S. Tsirkin 
---
 tests/acpi-test.c | 55 ---
 1 file changed, 32 insertions(+), 23 deletions(-)

diff --git a/tests/acpi-test.c b/tests/acpi-test.c
index 43775cd..ca83b1d 100644
--- a/tests/acpi-test.c
+++ b/tests/acpi-test.c
@@ -34,7 +34,7 @@ typedef struct {
 uint32_t *rsdt_tables_addr;
 int rsdt_tables_nr;
 AcpiSdtTable dsdt_table;
-AcpiSdtTable *ssdt_tables;
+GArray *ssdt_tables;
 } test_data;
 
 #define LOW(x) ((x) & 0xff)
@@ -118,6 +118,18 @@ static uint8_t boot_sector[0x200] = {
 
 static const char *disk = "tests/acpi-test-disk.raw";
 
+static void free_test_data(test_data *data)
+{
+int i;
+
+g_free(data->rsdt_tables_addr);
+for (i = 0; i < data->ssdt_tables->len; ++i) {
+g_free(g_array_index(data->ssdt_tables, AcpiSdtTable, i).aml);
+}
+g_array_free(data->ssdt_tables, false);
+g_free(data->dsdt_table.aml);
+}
+
 static uint8_t acpi_checksum(const uint8_t *data, int len)
 {
 int i;
@@ -295,30 +307,30 @@ static void test_acpi_dsdt_table(test_data *data)
 
 static void test_acpi_ssdt_tables(test_data *data)
 {
-AcpiSdtTable *ssdt_tables;
+GArray *ssdt_tables;
 int ssdt_tables_nr = data->rsdt_tables_nr - 1; /* fadt is first */
 int i;
 
-ssdt_tables = g_new0(AcpiSdtTable, ssdt_tables_nr);
+ssdt_tables = g_array_sized_new(false, true, sizeof(AcpiSdtTable),
+ssdt_tables_nr);
 for (i = 0; i < ssdt_tables_nr; i++) {
-AcpiSdtTable *ssdt_table = &ssdt_tables[i];
+AcpiSdtTable ssdt_table;
 uint32_t addr = data->rsdt_tables_addr[i + 1]; /* fadt is first */
-
-test_dst_table(ssdt_table, addr);
+test_dst_table(&ssdt_table, addr);
+g_array_append_val(ssdt_tables, ssdt_table);
 }
 data->ssdt_tables = ssdt_tables;
 }
 
-static void test_acpi_one(const char *params)
+static void test_acpi_one(const char *params, test_data *data)
 {
 char *args;
 uint8_t signature_low;
 uint8_t signature_high;
 uint16_t signature;
 int i;
-test_data data;
 
-memset(&data, 0, sizeof(data));
+memset(data, 0, sizeof(*data));
 args = g_strdup_printf("-net none -display none %s %s",
params ? params : "", disk);
 qtest_start(args);
@@ -342,20 +354,13 @@ static void test_acpi_one(const char *params)
 }
 g_assert_cmphex(signature, ==, SIGNATURE);
 
-test_acpi_rsdp_address(&data);
-test_acpi_rsdp_table(&data);
-test_acpi_rsdt_table(&data);
-test_acpi_fadt_table(&data);
+test_acpi_rsdp_address(data);
+test_acpi_rsdp_table(data);
+test_acpi_rsdt_table(data);
+test_acpi_fadt_table(data);
 test_acpi_facs_table(data);
-test_acpi_dsdt_table(&data);
-test_acpi_ssdt_tables(&data);
-
-g_free(data.rsdt_tables_addr);
-for (i = 0; i < (data.rsdt_tables_nr - 1); ++i) {
-g_free(data.ssdt_tables[i].aml);
-}
-g_free(data.ssdt_tables);
-g_free(data.dsdt_table.aml);
+test_acpi_dsdt_table(data);
+test_acpi_ssdt_tables(data);
 
 qtest_quit(global_qtest);
 g_free(args);
@@ -363,10 +368,14 @@ static void test_acpi_one(const char *params)
 
 static void test_acpi_tcg(void)
 {
+test_data data;
+
 /* Supplying -machine accel argument overrides the default (qtest).
  * This is to make guest actually run.
  */
-test_acpi_one("-machine accel=tcg");
+test_acpi_one("-machine accel=tcg", &data);
+
+free_test_data(&data);
 }
 
 int main(int argc, char *argv[])
-- 
MST




Re: [Qemu-devel] [V2 PATCH 00/18] target-ppc: Base ISA V2.06 for Power7/Power8

2013-12-11 Thread Peter Maydell
On 11 December 2013 19:16, Tom Musta  wrote:
> Additionally, some bugs in the common floating point library (softfloat)
> are fixed.

>  fpu/softfloat.c |   73 +--

All patches which touch fpu/ need to come with a statement
(in the cover letter is fine) that the changes are licensed under
both the softfloat-2a and -2b licenses [or under some license
compatible with both, I guess]. (We're trying to relicense that
area of the codebase at the moment.)

thanks
-- PMM



[Qemu-devel] [PULL 19/28] ACPI DSDT: Make control method `IQCR` serialized

2013-12-11 Thread Michael S. Tsirkin
Forward-port the following commit from seabios:

commit 995bbeef78b338370f426bf8d0399038c3fa259c
Author: Paul Menzel 
Date:   Thu Oct 3 11:30:52 2013 +0200

The ASL Optimizing Compiler version 20130823-32 [Sep 11 2013] issues the
following warning.

$ make
[…]
  Compiling IASL out/src/fw/acpi-dsdt.hex
out/src/fw/acpi-dsdt.dsl.i360: Method(IQCR, 1, 
NotSerialized) {
Remark   2120 - ^ Control 
Method should be made Serialized (due to creation of named objects within)
[…]
ASL Input: out/src/fw/acpi-dsdt.dsl.i - 475 lines, 19181 bytes, 
316 keywords
AML Output:out/src/fw/acpi-dsdt.aml - 4407 bytes, 159 named 
objects, 157 executable opcodes
Listing File:  out/src/fw/acpi-dsdt.lst - 143715 bytes
Hex Dump:  out/src/fw/acpi-dsdt.hex - 41661 bytes

Compilation complete. 0 Errors, 0 Warnings, 1 Remarks, 246 
Optimizations
[…]

After changing the parameter from `NotSerialized` to `Serialized`, the
remark is indeed gone and there is no size change.

The remark was added in ACPICA version 20130517 [1] and gives the
following explanation.

If a thread blocks within the method for any reason, and another 
thread
enters the method, the method will fail because an attempt will be
made to create the same (named) object twice.

In this case, issue a remark that the method should be marked
serialized. ACPICA BZ 909.

[1] 
https://github.com/acpica/acpica/commit/ba84d0fc18ba910a47a3f71c68a43543c06e6831

Signed-off-by: Paul Menzel 

Reported-by: Marcel Apfelbaum 
Tested-by: Marcel Apfelbaum 
Signed-off-by: Michael S. Tsirkin 
---
 hw/i386/acpi-dsdt.dsl   | 2 +-
 hw/i386/acpi-dsdt.hex.generated | 4 ++--
 hw/i386/q35-acpi-dsdt.dsl   | 2 +-
 hw/i386/q35-acpi-dsdt.hex.generated | 4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/i386/acpi-dsdt.dsl b/hw/i386/acpi-dsdt.dsl
index 90efce0..a377424 100644
--- a/hw/i386/acpi-dsdt.dsl
+++ b/hw/i386/acpi-dsdt.dsl
@@ -235,7 +235,7 @@ DefinitionBlock (
 }
 Return (0x0B)
 }
-Method(IQCR, 1, NotSerialized) {
+Method(IQCR, 1, Serialized) {
 // _CRS method - get current settings
 Name(PRR0, ResourceTemplate() {
 Interrupt(, Level, ActiveHigh, Shared) { 0 }
diff --git a/hw/i386/acpi-dsdt.hex.generated b/hw/i386/acpi-dsdt.hex.generated
index 2c01107..f8bd4ea 100644
--- a/hw/i386/acpi-dsdt.hex.generated
+++ b/hw/i386/acpi-dsdt.hex.generated
@@ -8,7 +8,7 @@ static unsigned char AcpiDsdtAmlCode[] = {
 0x0,
 0x0,
 0x1,
-0xe0,
+0xd8,
 0x42,
 0x58,
 0x50,
@@ -3379,7 +3379,7 @@ static unsigned char AcpiDsdtAmlCode[] = {
 0x51,
 0x43,
 0x52,
-0x1,
+0x9,
 0x8,
 0x50,
 0x52,
diff --git a/hw/i386/q35-acpi-dsdt.dsl b/hw/i386/q35-acpi-dsdt.dsl
index 21c89b0..575c5d7 100644
--- a/hw/i386/q35-acpi-dsdt.dsl
+++ b/hw/i386/q35-acpi-dsdt.dsl
@@ -333,7 +333,7 @@ DefinitionBlock (
 }
 Return (0x0B)
 }
-Method(IQCR, 1, NotSerialized) {
+Method(IQCR, 1, Serialized) {
 // _CRS method - get current settings
 Name(PRR0, ResourceTemplate() {
 Interrupt(, Level, ActiveHigh, Shared) { 0 }
diff --git a/hw/i386/q35-acpi-dsdt.hex.generated 
b/hw/i386/q35-acpi-dsdt.hex.generated
index 32c16ff..111ad3e 100644
--- a/hw/i386/q35-acpi-dsdt.hex.generated
+++ b/hw/i386/q35-acpi-dsdt.hex.generated
@@ -8,7 +8,7 @@ static unsigned char Q35AcpiDsdtAmlCode[] = {
 0x0,
 0x0,
 0x1,
-0x6,
+0xfe,
 0x42,
 0x58,
 0x50,
@@ -5338,7 +5338,7 @@ static unsigned char Q35AcpiDsdtAmlCode[] = {
 0x51,
 0x43,
 0x52,
-0x1,
+0x9,
 0x8,
 0x50,
 0x52,
-- 
MST




Re: [Qemu-devel] [PATCH v3 4/4] tcg/optimize: add known-zero bits compute for load ops

2013-12-11 Thread Richard Henderson
On 12/11/2013 06:13 AM, Aurelien Jarno wrote:
> Cc: Richard Henderson 
> Cc: Paolo Bonzini 
> Signed-off-by: Aurelien Jarno 
> ---
>  tcg/optimize.c |   33 +
>  1 file changed, 33 insertions(+)

Reviewed-by: Richard Henderson 


r~



[Qemu-devel] [PATCH 0/5] target-s390: Use the new qemu_ld/st opcodes

2013-12-11 Thread Richard Henderson
The first patch is purely mechanical.  The subsequent patches
tidy things up as allowed by the new interfaces.


r~


Richard Henderson (5):
  target-s390: Convert to new qemu_ld/st opcodes
  target-s390: Simplify op_clc
  target-s390: Simplify op_cs, op_soc, op_stm
  target-s390: Simplify op_icm, op_stcm
  target-s390: Use little-endian ops for LOAD/STORE REVERSED

 target-s390x/insn-data.def |  12 +--
 target-s390x/translate.c   | 251 ++---
 2 files changed, 130 insertions(+), 133 deletions(-)

-- 
1.8.3.1




[Qemu-devel] [PATCH 4/5] target-s390: Simplify op_icm, op_stcm

2013-12-11 Thread Richard Henderson
Loads and stores can now be shared, along with the surrounding code.

Signed-off-by: Richard Henderson 
---
 target-s390x/translate.c | 27 ---
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 3e88c23..aa7d351 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -2085,12 +2085,13 @@ static ExitStatus op_icm(DisasContext *s, DisasOps *o)
 int m3 = get_field(s->fields, m3);
 int pos, len, base = s->insn->data;
 TCGv_i64 tmp = tcg_temp_new_i64();
+TCGMemOp mop;
 uint64_t ccm;
 
 switch (m3) {
 case 0xf:
 /* Effectively a 32-bit load.  */
-tcg_gen_qemu_ld_i64(tmp, o->in2, get_mem_index(s), MO_BEUL);
+mop = MO_BEUL;
 len = 32;
 goto one_insert;
 
@@ -2098,7 +2099,7 @@ static ExitStatus op_icm(DisasContext *s, DisasOps *o)
 case 0x6:
 case 0x3:
 /* Effectively a 16-bit load.  */
-tcg_gen_qemu_ld_i64(tmp, o->in2, get_mem_index(s), MO_BEUW);
+mop = MO_BEUW;
 len = 16;
 goto one_insert;
 
@@ -2107,11 +2108,12 @@ static ExitStatus op_icm(DisasContext *s, DisasOps *o)
 case 0x2:
 case 0x1:
 /* Effectively an 8-bit load.  */
-tcg_gen_qemu_ld_i64(tmp, o->in2, get_mem_index(s), MO_UB);
+mop = MO_UB;
 len = 8;
 goto one_insert;
 
 one_insert:
+tcg_gen_qemu_ld_i64(tmp, o->in2, get_mem_index(s), mop);
 pos = base + ctz32(m3) * 8;
 tcg_gen_deposit_i64(o->out, o->out, tmp, pos, len);
 ccm = ((1ull << len) - 1) << pos;
@@ -3327,30 +3329,33 @@ static ExitStatus op_stcm(DisasContext *s, DisasOps *o)
 int m3 = get_field(s->fields, m3);
 int pos, base = s->insn->data;
 TCGv_i64 tmp = tcg_temp_new_i64();
+TCGMemOp mop;
 
-pos = base + ctz32(m3) * 8;
 switch (m3) {
 case 0xf:
 /* Effectively a 32-bit store.  */
-tcg_gen_shri_i64(tmp, o->in1, pos);
-tcg_gen_qemu_st_i64(tmp, o->in2, get_mem_index(s), MO_BEUL);
-break;
+mop = MO_BEUL;
+goto one_store;
 
 case 0xc:
 case 0x6:
 case 0x3:
 /* Effectively a 16-bit store.  */
-tcg_gen_shri_i64(tmp, o->in1, pos);
-tcg_gen_qemu_st_i64(tmp, o->in2, get_mem_index(s), MO_BEUW);
-break;
+mop = MO_BEUW;
+goto one_store;
 
 case 0x8:
 case 0x4:
 case 0x2:
 case 0x1:
 /* Effectively an 8-bit store.  */
+mop = MO_UB;
+goto one_store;
+
+one_store:
+pos = base + ctz32(m3) * 8;
 tcg_gen_shri_i64(tmp, o->in1, pos);
-tcg_gen_qemu_st_i64(tmp, o->in2, get_mem_index(s), MO_UB);
+tcg_gen_qemu_st_i64(tmp, o->in2, get_mem_index(s), mop);
 break;
 
 default:
-- 
1.8.3.1




[Qemu-devel] [V2 PATCH 03/18] target-ppc: Add ISA2.06 divdeu[o] Instructions

2013-12-11 Thread Tom Musta
This patch adds the Divide Doubleword Extended Unsigned
instructions.  This instruction requires dividing a 128-bit
value by a 64 bit value.  Since 128 bit integer division is
not supported in TCG, a helper is used, providing a
repeated difference algorithm.

V2: Moved the 128-bit divide routine into host-utils per Richard
Henderson's suggestion.

Signed-off-by: Tom Musta 
---
 include/qemu/host-utils.h |   14 ++
 target-ppc/helper.h   |1 +
 target-ppc/int_helper.c   |   27 +++
 target-ppc/translate.c|   20 
 util/host-utils.c |   38 ++
 5 files changed, 100 insertions(+), 0 deletions(-)

diff --git a/include/qemu/host-utils.h b/include/qemu/host-utils.h
index 0f688c1..0ca187d 100644
--- a/include/qemu/host-utils.h
+++ b/include/qemu/host-utils.h
@@ -44,9 +44,23 @@ static inline void muls64(uint64_t *plow, uint64_t *phigh,
 *plow = r;
 *phigh = r >> 64;
 }
+
+static inline int divu128(uint64_t *plow, uint64_t *phigh, uint64_t divisor)
+{
+if (divisor == 0) {
+return 1;
+} else {
+__uint128_t dividend = ((__uint128_t)*phigh << 64) | *plow;
+__uint128_t result = dividend / divisor;
+*plow = result;
+*phigh = dividend % divisor;
+return result > UINT64_MAX;
+}
+}
 #else
 void muls64(uint64_t *phigh, uint64_t *plow, int64_t a, int64_t b);
 void mulu64(uint64_t *phigh, uint64_t *plow, uint64_t a, uint64_t b);
+int divu128(uint64_t *plow, uint64_t *phigh, uint64_t divisor);
 #endif
 
 /**
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 1ec9c65..3eff4df 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -31,6 +31,7 @@ DEF_HELPER_5(lscbx, tl, env, tl, i32, i32, i32)
 
 #if defined(TARGET_PPC64)
 DEF_HELPER_3(mulldo, i64, env, i64, i64)
+DEF_HELPER_4(divdeu, i64, env, i64, i64, i32)
 #endif
 
 DEF_HELPER_FLAGS_1(cntlzw, TCG_CALL_NO_RWG_SE, tl, tl)
diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index abc69a7..d6dcac9 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -41,6 +41,33 @@ uint64_t helper_mulldo(CPUPPCState *env, uint64_t arg1, 
uint64_t arg2)
 }
 #endif
 
+#if defined(TARGET_PPC64)
+
+uint64_t helper_divdeu(CPUPPCState *env, uint64_t ra, uint64_t rb, uint32_t oe)
+{
+uint64_t rt = 0;
+int overflow = 0;
+
+overflow = divu128(&rt, &ra, rb);
+
+if (unlikely(overflow)) {
+rt = 0; /* Undefined */
+}
+
+if (oe) {
+if (unlikely(overflow)) {
+env->so = env->ov = 1;
+} else {
+env->ov = 0;
+}
+}
+
+return rt;
+}
+
+#endif
+
+
 target_ulong helper_cntlzw(target_ulong t)
 {
 return clz32(t);
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 0d39de2..7a51c6d 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -1032,6 +1032,23 @@ GEN_INT_ARITH_DIVD(divduo, 0x1E, 0, 1);
 /* divw  divw.  divwo  divwo.   */
 GEN_INT_ARITH_DIVD(divd, 0x0F, 1, 0);
 GEN_INT_ARITH_DIVD(divdo, 0x1F, 1, 1);
+
+/* divdeu[o][.] */
+#define GEN_DIVDE(name, hlpr, compute_ov) \
+static void gen_##name(DisasContext *ctx) \
+{ \
+TCGv_i32 t0 = tcg_const_i32(compute_ov);  \
+gen_helper_##hlpr(cpu_gpr[rD(ctx->opcode)], cpu_env,  \
+ cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)], t0); \
+tcg_temp_free_i32(t0);\
+if (unlikely(Rc(ctx->opcode) != 0)) { \
+gen_set_Rc0(ctx, cpu_gpr[rD(ctx->opcode)]);   \
+} \
+}
+
+GEN_DIVDE(divdeu, divdeu, 0);
+GEN_DIVDE(divdeuo, divdeu, 1);
+
 #endif
 
 /* mulhw  mulhw. */
@@ -9594,6 +9611,9 @@ GEN_INT_ARITH_DIVD(divduo, 0x1E, 0, 1),
 GEN_INT_ARITH_DIVD(divd, 0x0F, 1, 0),
 GEN_INT_ARITH_DIVD(divdo, 0x1F, 1, 1),
 
+GEN_HANDLER_E(divdeu, 0x1F, 0x09, 0x0C, 0x, PPC_NONE, PPC2_ISA206),
+GEN_HANDLER_E(divdeuo, 0x1F, 0x09, 0x1C, 0x, PPC_NONE, PPC2_ISA206),
+
 #undef GEN_INT_ARITH_MUL_HELPER
 #define GEN_INT_ARITH_MUL_HELPER(name, opc3)  \
 GEN_HANDLER(name, 0x1F, 0x09, opc3, 0x, PPC_64B)
diff --git a/util/host-utils.c b/util/host-utils.c
index f0784d6..b6f7a6e 100644
--- a/util/host-utils.c
+++ b/util/host-utils.c
@@ -86,4 +86,42 @@ void muls64 (uint64_t *plow, uint64_t *phigh, int64_t a, 
int64_t b)
 }
 *phigh = rh;
 }
+
+/* Unsigned 128x64 division.  Returns 1 if overflow (divide by zero or */
+/* quotient exceeds 64 bits).  Otherwise returns quotient via plow and */
+/* remainder via phigh. */
+int divu128(uint64_t *plow, uint64_t *phigh, uint64_t divisor)
+{
+uint64_t dhi =

[Qemu-devel] [PATCH 5/5] target-s390: Use little-endian ops for LOAD/STORE REVERSED

2013-12-11 Thread Richard Henderson
We don't need separate bswap with the new qemu_ld/st opcodes.

Signed-off-by: Richard Henderson 
---
 target-s390x/insn-data.def | 12 +-
 target-s390x/translate.c   | 58 ++
 2 files changed, 44 insertions(+), 26 deletions(-)

diff --git a/target-s390x/insn-data.def b/target-s390x/insn-data.def
index b42ebb6..dade9ba 100644
--- a/target-s390x/insn-data.def
+++ b/target-s390x/insn-data.def
@@ -423,9 +423,9 @@
 /* LOAD REVERSED */
 C(0xb91f, LRVR,RRE,   Z,   0, r2_32u, new, r1_32, rev32, 0)
 C(0xb90f, LRVGR,   RRE,   Z,   0, r2_o, r1, 0, rev64, 0)
-C(0xe31f, LRVH,RXY_a, Z,   0, m2_16u, new, r1_16, rev16, 0)
-C(0xe31e, LRV, RXY_a, Z,   0, m2_32u, new, r1_32, rev32, 0)
-C(0xe30f, LRVG,RXY_a, Z,   0, m2_64, r1, 0, rev64, 0)
+C(0xe31f, LRVH,RXY_a, Z,   0, a2, new, r1_16, ld16r, 0)
+C(0xe31e, LRV, RXY_a, Z,   0, a2, new, r1_32, ld32r, 0)
+C(0xe30f, LRVG,RXY_a, Z,   0, a2, r1, 0, ld64r, 0)
 /* LOAD ZERO */
 C(0xb374, LZER,RRE,   Z,   0, 0, 0, e1, zero, 0)
 C(0xb375, LZDR,RRE,   Z,   0, 0, 0, f1, zero, 0)
@@ -635,9 +635,9 @@
 D(0xebf3, STOC,RSY_b, LOC, 0, 0, 0, 0, soc, 0, 0)
 D(0xebe3, STOCG,   RSY_b, LOC, 0, 0, 0, 0, soc, 0, 1)
 /* STORE REVERSED */
-C(0xe33f, STRVH,   RXY_a, Z,   la2, r1_16u, new, m1_16, rev16, 0)
-C(0xe33e, STRV,RXY_a, Z,   la2, r1_32u, new, m1_32, rev32, 0)
-C(0xe32f, STRVG,   RXY_a, Z,   la2, r1_o, new, m1_64, rev64, 0)
+C(0xe33f, STRVH,   RXY_a, Z,   r1_o, a2, 0, 0, st16r, 0)
+C(0xe33e, STRV,RXY_a, Z,   r1_o, a2, 0, 0, st32r, 0)
+C(0xe32f, STRVG,   RXY_a, Z,   r1_o, a2, 0, 0, st64r, 0)
 
 /* STORE FPC */
 C(0xb29c, STFPC,   S, Z,   0, a2, new, m2_32, efpc, 0)
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index aa7d351..f1c000d 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -2271,6 +2271,24 @@ static ExitStatus op_ld64(DisasContext *s, DisasOps *o)
 return NO_EXIT;
 }
 
+static ExitStatus op_ld16r(DisasContext *s, DisasOps *o)
+{
+tcg_gen_qemu_ld_i64(o->out, o->in2, get_mem_index(s), MO_LEUW);
+return NO_EXIT;
+}
+
+static ExitStatus op_ld32r(DisasContext *s, DisasOps *o)
+{
+tcg_gen_qemu_ld_i64(o->out, o->in2, get_mem_index(s), MO_LEUL);
+return NO_EXIT;
+}
+
+static ExitStatus op_ld64r(DisasContext *s, DisasOps *o)
+{
+tcg_gen_qemu_ld_i64(o->out, o->in2, get_mem_index(s), MO_LEQ);
+return NO_EXIT;
+}
+
 static ExitStatus op_loc(DisasContext *s, DisasOps *o)
 {
 DisasCompare c;
@@ -2855,12 +2873,6 @@ static ExitStatus op_rosbg(DisasContext *s, DisasOps *o)
 return NO_EXIT;
 }
 
-static ExitStatus op_rev16(DisasContext *s, DisasOps *o)
-{
-tcg_gen_bswap16_i64(o->out, o->in2);
-return NO_EXIT;
-}
-
 static ExitStatus op_rev32(DisasContext *s, DisasOps *o)
 {
 tcg_gen_bswap32_i64(o->out, o->in2);
@@ -3313,6 +3325,24 @@ static ExitStatus op_st64(DisasContext *s, DisasOps *o)
 return NO_EXIT;
 }
 
+static ExitStatus op_st16r(DisasContext *s, DisasOps *o)
+{
+tcg_gen_qemu_st_i64(o->in1, o->in2, get_mem_index(s), MO_LEUW);
+return NO_EXIT;
+}
+
+static ExitStatus op_st32r(DisasContext *s, DisasOps *o)
+{
+tcg_gen_qemu_st_i64(o->in1, o->in2, get_mem_index(s), MO_LEUL);
+return NO_EXIT;
+}
+
+static ExitStatus op_st64r(DisasContext *s, DisasOps *o)
+{
+tcg_gen_qemu_st_i64(o->in1, o->in2, get_mem_index(s), MO_LEQ);
+return NO_EXIT;
+}
+
 static ExitStatus op_stam(DisasContext *s, DisasOps *o)
 {
 TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1));
@@ -4089,12 +4119,14 @@ static void in1_la1(DisasContext *s, DisasFields *f, 
DisasOps *o)
 }
 #define SPEC_in1_la1 0
 
+#ifndef CONFIG_USER_ONLY
 static void in1_la2(DisasContext *s, DisasFields *f, DisasOps *o)
 {
 int x2 = have_field(f, x2) ? get_field(f, x2) : 0;
 o->addr1 = get_address(s, x2, get_field(f, b2), get_field(f, d2));
 }
 #define SPEC_in1_la2 0
+#endif
 
 static void in1_m1_8u(DisasContext *s, DisasFields *f, DisasOps *o)
 {
@@ -4154,13 +4186,6 @@ static void in2_r1_o(DisasContext *s, DisasFields *f, 
DisasOps *o)
 }
 #define SPEC_in2_r1_o 0
 
-static void in2_r1_16u(DisasContext *s, DisasFields *f, DisasOps *o)
-{
-o->in2 = tcg_temp_new_i64();
-tcg_gen_ext16u_i64(o->in2, regs[get_field(f, r1)]);
-}
-#define SPEC_in2_r1_16u 0
-
 static void in2_r1_32u(DisasContext *s, DisasFields *f, DisasOps *o)
 {
 o->in2 = tcg_temp_new_i64();
@@ -4313,13 +4338,6 @@ static void in2_m2_16s(DisasContext *s, DisasFields *f, 
DisasOps *o)
 }
 #define SPEC_in2_m2_16s 0
 
-static void in2_m2_16u(DisasContext *s, DisasFields *f, DisasOps *o)
-{
-in2_a2(s, f, o);
-tcg_gen_qemu_ld_i64(o->in2, o->in2, get_mem_index(s), MO_BEUW);
-}
-#define SPEC_in2_m2_16u 0
-
 static void in2_m2_32s(DisasContext *s, DisasFields *f, DisasOps *o)
 {
 in2_a2(s, f, o);
-- 
1.8.3.1




[Qemu-devel] [V2 PATCH 07/18] target-ppc: Add ISA 2.06 stbcx. and sthcx. Instructions

2013-12-11 Thread Tom Musta
This patch adds the byte and halfword variants of the Store Conditional
instructions.   A common macro is introduced and the existing implementations
of stwcx. and stdcx. are re-implemented using this macro.

V2: Re-implemented gen_conditional_store() and STCX macro per comments
from Richard.

Signed-off-by: Tom Musta 
---
 target-ppc/translate.c |   88 ++-
 1 files changed, 41 insertions(+), 47 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index c3d0ebe..27eef84 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -3242,8 +3242,8 @@ LARX(lwarx, 4, ld32u);
 
 
 #if defined(CONFIG_USER_ONLY)
-static void gen_conditional_store (DisasContext *ctx, TCGv EA,
-   int reg, int size)
+static void gen_conditional_store(DisasContext *ctx, TCGv EA,
+  int reg, int size)
 {
 TCGv t0 = tcg_temp_new();
 uint32_t save_exception = ctx->exception;
@@ -3257,62 +3257,54 @@ static void gen_conditional_store (DisasContext *ctx, 
TCGv EA,
 gen_exception(ctx, POWERPC_EXCP_STCX);
 ctx->exception = save_exception;
 }
-#endif
-
-/* stwcx. */
-static void gen_stwcx_(DisasContext *ctx)
-{
-TCGv t0;
-gen_set_access_type(ctx, ACCESS_RES);
-t0 = tcg_temp_local_new();
-gen_addr_reg_index(ctx, t0);
-gen_check_align(ctx, t0, 0x03);
-#if defined(CONFIG_USER_ONLY)
-gen_conditional_store(ctx, t0, rS(ctx->opcode), 4);
 #else
-{
-int l1;
+static void gen_conditional_store(DisasContext *ctx, TCGv EA,
+  int reg, int size)
+{
+int l1;
 
-tcg_gen_trunc_tl_i32(cpu_crf[0], cpu_so);
-l1 = gen_new_label();
-tcg_gen_brcond_tl(TCG_COND_NE, t0, cpu_reserve, l1);
-tcg_gen_ori_i32(cpu_crf[0], cpu_crf[0], 1 << CRF_EQ);
-gen_qemu_st32(ctx, cpu_gpr[rS(ctx->opcode)], t0);
-gen_set_label(l1);
-tcg_gen_movi_tl(cpu_reserve, -1);
+tcg_gen_trunc_tl_i32(cpu_crf[0], cpu_so);
+l1 = gen_new_label();
+tcg_gen_brcond_tl(TCG_COND_NE, EA, cpu_reserve, l1);
+tcg_gen_ori_i32(cpu_crf[0], cpu_crf[0], 1 << CRF_EQ);
+if (size == 8) {
+gen_qemu_st64(ctx, cpu_gpr[reg], EA);
+} else if (size == 4) {
+gen_qemu_st32(ctx, cpu_gpr[reg], EA);
+} else if (size == 2) {
+gen_qemu_st16(ctx, cpu_gpr[reg], EA);
+} else {
+gen_qemu_st8(ctx, cpu_gpr[reg], EA);
 }
+gen_set_label(l1);
+tcg_gen_movi_tl(cpu_reserve, -1);
+}
 #endif
-tcg_temp_free(t0);
+
+#define STCX(name, len)   \
+static void gen_##name(DisasContext *ctx) \
+{ \
+TCGv t0;  \
+gen_set_access_type(ctx, ACCESS_RES); \
+t0 = tcg_temp_local_new();\
+gen_addr_reg_index(ctx, t0);  \
+if (len > 1) {\
+gen_check_align(ctx, t0, (len)-1);\
+} \
+gen_conditional_store(ctx, t0, rS(ctx->opcode), len); \
+tcg_temp_free(t0);\
 }
 
+STCX(stbcx_, 1);
+STCX(sthcx_, 2);
+STCX(stwcx_, 4);
+
 #if defined(TARGET_PPC64)
 /* ldarx */
 LARX(ldarx, 8, ld64);
 
 /* stdcx. */
-static void gen_stdcx_(DisasContext *ctx)
-{
-TCGv t0;
-gen_set_access_type(ctx, ACCESS_RES);
-t0 = tcg_temp_local_new();
-gen_addr_reg_index(ctx, t0);
-gen_check_align(ctx, t0, 0x07);
-#if defined(CONFIG_USER_ONLY)
-gen_conditional_store(ctx, t0, rS(ctx->opcode), 8);
-#else
-{
-int l1;
-tcg_gen_trunc_tl_i32(cpu_crf[0], cpu_so);
-l1 = gen_new_label();
-tcg_gen_brcond_tl(TCG_COND_NE, t0, cpu_reserve, l1);
-tcg_gen_ori_i32(cpu_crf[0], cpu_crf[0], 1 << CRF_EQ);
-gen_qemu_st64(ctx, cpu_gpr[rS(ctx->opcode)], t0);
-gen_set_label(l1);
-tcg_gen_movi_tl(cpu_reserve, -1);
-}
-#endif
-tcg_temp_free(t0);
-}
+STCX(stdcx_, 8);
 #endif /* defined(TARGET_PPC64) */
 
 /* sync */
@@ -9460,6 +9452,8 @@ GEN_HANDLER(isync, 0x13, 0x16, 0x04, 0x03FFF801, PPC_MEM),
 GEN_HANDLER_E(lbarx, 0x1F, 0x14, 0x01, 0x, PPC_NONE, PPC2_ISA206),
 GEN_HANDLER_E(lharx, 0x1F, 0x14, 0x03, 0x, PPC_NONE, PPC2_ISA206),
 GEN_HANDLER(lwarx, 0x1F, 0x14, 0x00, 0x, PPC_RES),
+GEN_HANDLER_E(stbcx_, 0x1F, 0x16, 0x15, 0x, PPC_NONE, PPC2_ISA206),
+GEN_HANDLER_E(sthcx_, 0x1F, 0x16, 0x16, 0x, PPC_NONE, PPC2_ISA206),
 GEN_HANDLER2(stwcx_, "stwcx.", 0x1F, 0x16, 0x04, 0x, PPC_RES),
 #if defined(TARGET_PPC64)
 GEN_HANDLER(ldarx, 0x1F, 0x14, 0x02, 0x, PPC_64B),
-- 
1.7.1




[Qemu-devel] [V2 PATCH 15/18] target-ppc: Add ISA 2.06 ftdiv Instruction

2013-12-11 Thread Tom Musta
This patch adds the Floating Point Test for Divide instruction which
was introduced in Power ISA 2.06.

Signed-off-by: Tom Musta 
---
 target-ppc/fpu_helper.c |   56 ++
 target-ppc/helper.h |2 +
 target-ppc/translate.c  |   17 ++
 3 files changed, 65 insertions(+), 10 deletions(-)

diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c
index 981f002..82ff0db 100644
--- a/target-ppc/fpu_helper.c
+++ b/target-ppc/fpu_helper.c
@@ -50,6 +50,16 @@ static inline int isden(float64 d)
 return ((u.ll >> 52) & 0x7FF) == 0;
 }
 
+static inline int ppc_float32_get_unbiased_exp(float32 f)
+{
+return ((f >> 23) & 0xFF) - 127;
+}
+
+static inline int ppc_float64_get_unbiased_exp(float64 f)
+{
+return ((f >> 52) & 0x7FF) - 1023;
+}
+
 uint32_t helper_compute_fprf(CPUPPCState *env, uint64_t arg, uint32_t set_fprf)
 {
 CPU_DoubleU farg;
@@ -988,6 +998,42 @@ uint64_t helper_fsel(CPUPPCState *env, uint64_t arg1, 
uint64_t arg2,
 }
 }
 
+void helper_ftdiv(CPUPPCState *env, uint32_t bf, uint64_t fra, uint64_t frb)
+{
+int fe_flag = 0;
+int fg_flag = 0;
+
+if (unlikely(float64_is_infinity(fra) ||
+ float64_is_infinity(frb) ||
+ float64_is_zero(frb))) {
+fe_flag = 1;
+fg_flag = 1;
+} else {
+int e_a = ppc_float64_get_unbiased_exp(fra);
+int e_b = ppc_float64_get_unbiased_exp(frb);
+
+if (unlikely(float64_is_any_nan(fra) ||
+ float64_is_any_nan(frb))) {
+fe_flag = 1;
+} else if ((e_b <= -1022) || (e_b >= 1021)) {
+fe_flag = 1;
+} else if (!float64_is_zero(fra) &&
+   (((e_a - e_b) >= 1023) ||
+((e_a - e_b) <= -1021) ||
+(e_a <= -970))) {
+fe_flag = 1;
+}
+
+if (unlikely(float64_is_zero_or_denormal(frb))) {
+/* XB is not zero because of the above check and */
+/* so must be denormalized.  */
+fg_flag = 1;
+}
+}
+
+env->crf[bf] = 0x8 | (fg_flag ? 4 : 0) | (fe_flag ? 2 : 0);
+}
+
 void helper_fcmpu(CPUPPCState *env, uint64_t arg1, uint64_t arg2,
   uint32_t crfD)
 {
@@ -2016,16 +2062,6 @@ VSX_RSQRTE(xsrsqrtesp, 1, float64, f64, 1, 1)
 VSX_RSQRTE(xvrsqrtedp, 2, float64, f64, 0, 0)
 VSX_RSQRTE(xvrsqrtesp, 4, float32, f32, 0, 0)
 
-static inline int ppc_float32_get_unbiased_exp(float32 f)
-{
-return ((f >> 23) & 0xFF) - 127;
-}
-
-static inline int ppc_float64_get_unbiased_exp(float64 f)
-{
-return ((f >> 52) & 0x7FF) - 1023;
-}
-
 /* VSX_TDIV - VSX floating point test for divide
  *   op- instruction mnemonic
  *   nels  - number of elements (1, 2 or 4)
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 19b2f6b..5f5d3f6 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -97,6 +97,8 @@ DEF_HELPER_2(fres, i64, env, i64)
 DEF_HELPER_2(frsqrte, i64, env, i64)
 DEF_HELPER_4(fsel, i64, env, i64, i64, i64)
 
+DEF_HELPER_4(ftdiv, void, env, i32, i64, i64)
+
 #define dh_alias_avr ptr
 #define dh_ctype_avr ppc_avr_t *
 #define dh_is_signed_avr dh_is_signed_ptr
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 9f0c682..403e274 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -2299,6 +2299,22 @@ GEN_FLOAT_B(rip, 0x08, 0x0E, 1, PPC_FLOAT_EXT);
 /* frim */
 GEN_FLOAT_B(rim, 0x08, 0x0F, 1, PPC_FLOAT_EXT);
 
+static void gen_ftdiv(DisasContext *ctx)
+{
+TCGv_i32 bf;
+if (unlikely(!ctx->fpu_enabled)) {
+gen_exception(ctx, POWERPC_EXCP_FPU);
+return;
+}
+/* NIP cannot be restored if the memory exception comes from an helper */
+gen_update_nip(ctx, ctx->nip - 4);
+bf = tcg_const_i32(crfD(ctx->opcode));
+gen_helper_ftdiv(cpu_env, bf, cpu_fpr[rA(ctx->opcode)],
+ cpu_fpr[rB(ctx->opcode)]);
+}
+
+
+
 /*** Floating-Point compare***/
 
 /* fcmpo */
@@ -9801,6 +9817,7 @@ GEN_FLOAT_ACB(madd, 0x1D, 1, PPC_FLOAT),
 GEN_FLOAT_ACB(msub, 0x1C, 1, PPC_FLOAT),
 GEN_FLOAT_ACB(nmadd, 0x1F, 1, PPC_FLOAT),
 GEN_FLOAT_ACB(nmsub, 0x1E, 1, PPC_FLOAT),
+GEN_HANDLER_E(ftdiv, 0x3F, 0x00, 0x04, 1, PPC_NONE, PPC2_ISA206),
 GEN_FLOAT_B(ctiw, 0x0E, 0x00, 0, PPC_FLOAT),
 GEN_HANDLER_E(fctiwu, 0x3F, 0x0E, 0x04, 0x, PPC_NONE, PPC2_ISA206),
 GEN_FLOAT_B(ctiwz, 0x0F, 0x00, 0, PPC_FLOAT),
-- 
1.7.1




[Qemu-devel] [V2 PATCH 18/18] target-ppc: Add ISA2.06 lfiwzx Instruction

2013-12-11 Thread Tom Musta
This patch adds the Load Floating Point as Integer Word and
Zero Indexed (lfiwzx) instruction which was introduced in
Power ISA 2.06.

Signed-off-by: Tom Musta 
---
 target-ppc/translate.c |   15 +++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 0833606..cf1fb1b 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -3524,6 +3524,20 @@ static void gen_lfiwax(DisasContext *ctx)
 tcg_temp_free(t0);
 }
 
+/* lfiwzx */
+static void gen_lfiwzx(DisasContext *ctx)
+{
+TCGv EA;
+if (unlikely(!ctx->fpu_enabled)) {
+gen_exception(ctx, POWERPC_EXCP_FPU);
+return;
+}
+gen_set_access_type(ctx, ACCESS_FLOAT);
+EA = tcg_temp_new();
+gen_addr_reg_index(ctx, EA);
+gen_qemu_ld32u(ctx, cpu_fpr[rD(ctx->opcode)], EA);
+tcg_temp_free(EA);
+}
 /*** Floating-point store  ***/
 #define GEN_STF(name, stop, opc, type)\
 static void glue(gen_, name)(DisasContext *ctx)
   \
@@ -9937,6 +9951,7 @@ GEN_LDXF(name, ldop, 0x17, op | 0x00, type)
 GEN_LDFS(lfd, ld64, 0x12, PPC_FLOAT)
 GEN_LDFS(lfs, ld32fs, 0x10, PPC_FLOAT)
 GEN_HANDLER_E(lfiwax, 0x1f, 0x17, 0x1a, 0x0001, PPC_NONE, PPC2_ISA205),
+GEN_HANDLER_E(lfiwzx, 0x1f, 0x17, 0x1b, 0x0001, PPC_NONE, PPC2_ISA206),
 GEN_HANDLER_E(lfdp, 0x39, 0xFF, 0xFF, 0x0023, PPC_NONE, PPC2_ISA205),
 GEN_HANDLER_E(lfdpx, 0x1F, 0x17, 0x18, 0x0021, PPC_NONE, PPC2_ISA205),
 
-- 
1.7.1




[Qemu-devel] [V2 PATCH 11/18] softfloat: Fix float64_to_uint32

2013-12-11 Thread Tom Musta
The float64_to_uint32 has several flaws:

 - for numbers between 2**32 and 2**64, the inexact exception flag
   may get incorrectly set.  In this case, only the invalid flag
   should be set.

   test pattern: 425F81378DC0CD1F / 0x1.f81378dc0cd1fp+38

 - for numbers between 2**63 and 2**64, incorrect results may
   be produced:

   test pattern: 43EAAF73F1F0B8BD / 0x1.aaf73f1f0b8bdp+63

This patch re-implements float64_to_uint32 to re-use the
float64_to_uint64 routine (instead of float64_to_int64).  For the
saturation case, the inexact bit is explicitly cleared before raising
the invalid flag.

Signed-off-by: Tom Musta 
---
 fpu/softfloat.c |   10 --
 1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 1003e59..6a8b6f5 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -6578,15 +6578,13 @@ uint_fast16_t float32_to_uint16_round_to_zero(float32 a 
STATUS_PARAM)
 
 uint32 float64_to_uint32( float64 a STATUS_PARAM )
 {
-int64_t v;
+uint64_t v;
 uint32 res;
 
-v = float64_to_int64(a STATUS_VAR);
-if (v < 0) {
-res = 0;
-float_raise( float_flag_invalid STATUS_VAR);
-} else if (v > 0x) {
+v = float64_to_uint64(a STATUS_VAR);
+if (v > 0x) {
 res = 0x;
+STATUS(float_exception_flags) &= ~float_flag_inexact;
 float_raise( float_flag_invalid STATUS_VAR);
 } else {
 res = v;
-- 
1.7.1




  1   2   3   4   >