Re: [Qemu-devel] [PATCH 0/2] sd: sdhci: correct transfer mode register usage

[P J P]

+-- On Tue, 31 Jan 2017, P J P wrote --+
| In SDHCI emulation, the 'Block Count Enable' bit of the Transfer Mode
| register is used to control 's->blkcnt' value. One, this bit is not
| relevant in single block transfers. Second, Transfer Mode register
| value could be set such that 's->blkcnt' would not see an update
| during multi block transfers. Thus leading to an infinite loop.
| 
| This patch set attempts to correct 'Block Count Enable' bit usage.
| 
| Thank you.
| --
| Prasad J Pandit (2):
|   sd: sdhci: check transfer mode register in multi block transfer
|   sd: sdhci: block count enable not relevant in single block transfer

Ping...!
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH 2/2] savevm: Add new helpers to process the different stages of loadvm/savevm

[Hailiang Zhang]

On 2017/1/31 18:19, Juan Quintela wrote:

"Dr. David Alan Gilbert"  wrote:

* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:

There are several stages during loadvm/savevm process. In different stage,
migration incoming processes different types of sections.
We want to control these stages more accuracy, it will benefit COLO
performance, we don't have to save type of QEMU_VM_SECTION_START
sections everytime while do checkpoint, besides, we want to separate
the process of saving/loading memory and devices state.

So we add three new helper functions: qemu_loadvm_state_begin(),
qemu_load_device_state() and qemu_savevm_live_state() to achieve
different process during migration.

Signed-off-by: zhanghailiang 
Signed-off-by: Li Zhijian 
Reviewed-by: Dr. David Alan Gilbert 


I don't think this one can go in without the patch that follows which
uses these functions; we don't normally add functions
without the patch that uses them.


Agreed.  If you want to add functions, you need new code that use them,
or make old code use them.  It is up to you.



Got it, thanks, I'll merge this series with the one where we use them.


Thanks, Juan.

.

Re: [Qemu-devel] [PATCH 1/2] savevm: split save/find loadvm_handlers entry into two helper functions

[Hailiang Zhang]

On 2017/1/31 18:04, Dr. David Alan Gilbert wrote:

* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:

COLO's checkpoint process is based on migration process,
everytime we do checkpoint we will repeat the process of savevm and loadvm.

So we will call qemu_loadvm_section_start_full() repeatedly, It will
add all migration sections information into loadvm_handlers list everytime,
which will lead to memory leak.

To fix it, we split the process of saving and finding section entry into two
helper functions, we will check if section info was exist in loadvm_handlers
list before save it.

This modifications have no side effect for normal migration.

Signed-off-by: zhanghailiang 
Reviewed-by: Dr. David Alan Gilbert 
---
  migration/savevm.c | 55 +++---
  1 file changed, 40 insertions(+), 15 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index f9c06e9..92b3d6c 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1805,6 +1805,37 @@ void loadvm_free_handlers(MigrationIncomingState *mis)
  }
  }

+static LoadStateEntry *loadvm_save_section_entry(MigrationIncomingState *mis,


Can you change that to loadvm_add_section_entry please;  it's a bit
confusing using 'save' in a function name that's part of the 'load' path.



OK, will fix it in next version, thanks.


Dave


+ SaveStateEntry *se,
+ uint32_t section_id,
+ uint32_t version_id)
+{
+LoadStateEntry *le;
+
+/* Add entry */
+le = g_malloc0(sizeof(*le));
+
+le->se = se;
+le->section_id = section_id;
+le->version_id = version_id;
+QLIST_INSERT_HEAD(>loadvm_handlers, le, entry);
+return le;
+}
+
+static LoadStateEntry *loadvm_find_section_entry(MigrationIncomingState *mis,
+ uint32_t section_id)
+{
+LoadStateEntry *le;
+
+QLIST_FOREACH(le, >loadvm_handlers, entry) {
+if (le->section_id == section_id) {
+break;
+}
+}
+
+return le;
+}
+
  static int
  qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis)
  {
@@ -1847,15 +1878,12 @@ qemu_loadvm_section_start_full(QEMUFile *f, 
MigrationIncomingState *mis)
  return -EINVAL;
  }

-/* Add entry */
-le = g_malloc0(sizeof(*le));
-
-le->se = se;
-le->section_id = section_id;
-le->version_id = version_id;
-QLIST_INSERT_HEAD(>loadvm_handlers, le, entry);
-
-ret = vmstate_load(f, le->se, le->version_id);
+ /* Check if we have saved this section info before, if not, save it */
+le = loadvm_find_section_entry(mis, section_id);
+if (!le) {
+le = loadvm_save_section_entry(mis, se, section_id, version_id);
+}
+ret = vmstate_load(f, se, version_id);
  if (ret < 0) {
  error_report("error while loading state for instance 0x%x of"
   " device '%s'", instance_id, idstr);
@@ -1878,12 +1906,9 @@ qemu_loadvm_section_part_end(QEMUFile *f, 
MigrationIncomingState *mis)
  section_id = qemu_get_be32(f);

  trace_qemu_loadvm_state_section_partend(section_id);
-QLIST_FOREACH(le, >loadvm_handlers, entry) {
-if (le->section_id == section_id) {
-break;
-}
-}
-if (le == NULL) {
+
+le = loadvm_find_section_entry(mis, section_id);
+if (!le) {
  error_report("Unknown savevm section %d", section_id);
  return -EINVAL;
  }
--
1.8.3.1



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

.

Re: [Qemu-devel] [PATCH 2/2] savevm: Add new helpers to process the different stages of loadvm/savevm

[Hailiang Zhang]

On 2017/1/31 18:05, Dr. David Alan Gilbert wrote:

* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:

There are several stages during loadvm/savevm process. In different stage,
migration incoming processes different types of sections.
We want to control these stages more accuracy, it will benefit COLO
performance, we don't have to save type of QEMU_VM_SECTION_START
sections everytime while do checkpoint, besides, we want to separate
the process of saving/loading memory and devices state.

So we add three new helper functions: qemu_loadvm_state_begin(),
qemu_load_device_state() and qemu_savevm_live_state() to achieve
different process during migration.

Signed-off-by: zhanghailiang 
Signed-off-by: Li Zhijian 
Reviewed-by: Dr. David Alan Gilbert 


I don't think this one can go in without the patch that follows which
uses these functions; we don't normally add functions
without the patch that uses them.



Yes, this series is prerequisite for the later COLO optimization,
that will be a bigger series, that's why i picked these two patches,
but since it is irrational, I'll post these series with the later one
where we use this functions. Thanks. :)


Dave


---
  include/sysemu/sysemu.h |  4 
  migration/savevm.c  | 41 +
  2 files changed, 45 insertions(+)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index ff8ffb5..d9214bf 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -126,7 +126,11 @@ void qemu_savevm_send_postcopy_ram_discard(QEMUFile *f, 
const char *name,
 uint64_t *start_list,
 uint64_t *length_list);

+void qemu_savevm_live_state(QEMUFile *f);
+
  int qemu_loadvm_state(QEMUFile *f);
+int qemu_loadvm_state_begin(QEMUFile *f);
+int qemu_load_device_state(QEMUFile *f);

  extern int autostart;

diff --git a/migration/savevm.c b/migration/savevm.c
index 92b3d6c..04dee4f 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1264,6 +1264,13 @@ done:
  return ret;
  }

+void qemu_savevm_live_state(QEMUFile *f)
+{
+/* save QEMU_VM_SECTION_END section */
+qemu_savevm_state_complete_precopy(f, true);
+qemu_put_byte(f, QEMU_VM_EOF);
+}
+
  static int qemu_save_device_state(QEMUFile *f)
  {
  SaveStateEntry *se;
@@ -2064,6 +2071,40 @@ int qemu_loadvm_state(QEMUFile *f)
  return ret;
  }

+int qemu_loadvm_state_begin(QEMUFile *f)
+{
+MigrationIncomingState *mis = migration_incoming_get_current();
+Error *local_err = NULL;
+int ret;
+
+if (qemu_savevm_state_blocked(_err)) {
+error_report_err(local_err);
+return -EINVAL;
+}
+/* Load QEMU_VM_SECTION_START section */
+ret = qemu_loadvm_state_main(f, mis);
+if (ret < 0) {
+error_report("Failed to loadvm begin work: %d", ret);
+}
+return ret;
+}
+
+int qemu_load_device_state(QEMUFile *f)
+{
+MigrationIncomingState *mis = migration_incoming_get_current();
+int ret;
+
+/* Load QEMU_VM_SECTION_FULL section */
+ret = qemu_loadvm_state_main(f, mis);
+if (ret < 0) {
+error_report("Failed to load device state: %d", ret);
+return ret;
+}
+
+cpu_synchronize_all_post_init();
+return 0;
+}
+
  void hmp_savevm(Monitor *mon, const QDict *qdict)
  {
  BlockDriverState *bs, *bs1;
--
1.8.3.1



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

.

Re: [Qemu-devel] [RFC PATCH] linux-user: Add signal handling for x86_64

[Pranith Kumar]

Hi Laurent,

Thanks for the review.

Laurent Vivier writes:

> Le 25/01/2017 à 01:10, Pranith Kumar a écrit :
>> Adopted from a previous patch posting:
>> https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg02079.html
>> 
>> CC: Allan Wirth 
>> CC: Peter Maydell 
>> Signed-off-by: Pranith Kumar 
>> ---
>>  linux-user/signal.c  | 264 
>> ---
>>  target/i386/cpu.h|   2 +
>>  target/i386/fpu_helper.c |  12 +++
>>  3 files changed, 242 insertions(+), 36 deletions(-)
>> 
>> diff --git a/linux-user/signal.c b/linux-user/signal.c
>> index 0a5bb4e26b..0248621d66 100644
>> --- a/linux-user/signal.c
>> +++ b/linux-user/signal.c
>> @@ -253,8 +253,7 @@ int do_sigprocmask(int how, const sigset_t *set, 
>> sigset_t *oldset)
>>  return 0;
>>  }
>>  
>> -#if !defined(TARGET_OPENRISC) && !defined(TARGET_UNICORE32) && \
>> -!defined(TARGET_X86_64)
>> +#if !defined(TARGET_OPENRISC) && !defined(TARGET_UNICORE32)
>>  /* Just set the guest's signal mask to the specified value; the
>>   * caller is assumed to have called block_signals() already.
>>   */
>> @@ -512,7 +511,7 @@ void signal_init(void)
>>  }
>>  }
>>  
>> -#if !(defined(TARGET_X86_64) || defined(TARGET_UNICORE32))
>> +#ifndef TARGET_UNICORE32
>>  /* Force a synchronously taken signal. The kernel force_sig() function
>>   * also forces the signal to "not blocked, not ignored", but for QEMU
>>   * that work is done in process_pending_signals().
>> @@ -819,9 +818,8 @@ int do_sigaction(int sig, const struct target_sigaction 
>> *act,
>>  return ret;
>>  }
>>  
>> -#if defined(TARGET_I386) && TARGET_ABI_BITS == 32
>> -
>> -/* from the Linux kernel */
>> +#if defined(TARGET_I386)
>> +/* from the Linux kernel - /arch/x86/include/uapi/asm/sigcontext.h */
>>  
>>  struct target_fpreg {
>>  uint16_t significand[4];
>> @@ -835,7 +833,7 @@ struct target_fpxreg {
>>  };
>>  
>>  struct target_xmmreg {
>> -abi_ulong element[4];
>> +uint32_t element[4];
>>  };
>>  
>>  struct target_fpstate {
>> @@ -860,33 +858,117 @@ struct target_fpstate {
>>  abi_ulong padding[56];
>>  };
>
> I think you should remove the definition of the target_fpstate structure
> as you overwrite it with #define below:
> ...
>> +
>> +#ifndef TARGET_X86_64
>> +# define target_fpstate target_fpstate_32
>> +#else
>> +# define target_fpstate target_fpstate_64
>> +#endif
>> +

Good catch. I'll do that.

> ...
>> @@ -959,12 +1052,49 @@ static void setup_sigcontext(struct target_sigcontext 
>> *sc,
>>  /* non-iBCS2 extensions.. */
>>  __put_user(mask, >oldmask);
>>  __put_user(env->cr[2], >cr2);
>> +#else
>> +__put_user(env->regs[8], >r8);
>> +__put_user(env->regs[9], >r9);
>> +__put_user(env->regs[10], >r10);
>> +__put_user(env->regs[11], >r11);
>> +__put_user(env->regs[12], >r12);
>> +__put_user(env->regs[13], >r13);
>> +__put_user(env->regs[14], >r14);
>> +__put_user(env->regs[15], >r15);
>> +
>> +__put_user(env->regs[R_EDI], >rdi);
>> +__put_user(env->regs[R_ESI], >rsi);
>> +__put_user(env->regs[R_EBP], >rbp);
>> +__put_user(env->regs[R_EBX], >rbx);
>> +__put_user(env->regs[R_EDX], >rdx);
>> +__put_user(env->regs[R_EAX], >rax);
>> +__put_user(env->regs[R_ECX], >rcx);
>> +__put_user(env->regs[R_ESP], >rsp);
>> +__put_user(env->eip, >rip);
>> +
>> +__put_user(env->eflags, >eflags);
>> +
>> +__put_user(env->segs[R_CS].selector, >cs);
>> +__put_user((uint16_t)0, >gs);
>> +__put_user((uint16_t)0, >fs);
>> +__put_user(env->segs[R_SS].selector, >ss);
>> +
>> +__put_user(env->error_code, >err);
>> +__put_user(cs->exception_index, >trapno);
>> +__put_user(mask, >oldmask);
>> +__put_user(env->cr[2], >cr2);
>> +
>> +/* fpstate_addr must be 16 byte aligned for fxsave */
>> +assert(!(fpstate_addr & 0xf));
>> +
>> +cpu_x86_fxsave(env, fpstate_addr);
>> +__put_user(fpstate_addr, >fpstate);
>> +#endif
>
> This part would be more readable if the registers were in the same order
> as  in the kernel function setup_sigcontext().

OK, noted this. I'll make the change.

> ...
>> +if (info) {
>> +tswap_siginfo(>info, info);
>> +}
>
> kernel checks "ksig->ka.sa.sa_flags & SA_SIGINFO" to know if there is
> siginfo structure.
>

OK. I'll do the same as what the kernel is doing.

> ...
>>  /* Set up registers for signal handler */
>>  env->regs[R_ESP] = frame_addr;
>> +env->regs[R_EAX] = 0;
>> +env->regs[R_EDI] = sig;
>> +env->regs[R_ESI] = (unsigned long)>info;
>> +env->regs[R_EDX] = (unsigned long)>uc;
>>  env->eip = ka->_sa_handler;
>
> In kernel, 32bit handler seems to not use the same registers as 64bit
> handler, for instance ax is sig, info is dx and uc is cx.
>

Hmm.. how do I reproduce the same here? Should I check if we are in 32-bit 
environment?

> ...
>> @@ -6181,11 +6371,13 @@ static void 

Re: [Qemu-devel] Non-flat command line option argument syntax

[Markus Armbruster]

Markus Armbruster  writes:

[...]
> === Structured values ===
>
> The dotted key convention messes with KEY syntax to permit structured
> values.  Works, but the more conventional way to support structured
> values is a syntax for structured values.  
>
> An obvious one is to use { KEY=VALUE, ...} for objects, and [ VALUE,
> ... ] for arrays.  Looks like this:
>
> -drive 'driver=quorum,
> child=[{ driver=file, filename=disk1.img },
>{ driver=host_device, filename=/dev/sdb },
>{ driver=nbd, host=localhost } ]'
>
> Again, lines broken and indented for legibility; you need to join them
> for actual use.
>
> There's a syntactic catch, though: a value of the form [ ... ] can
> either be an array or a string.  Which one it is depends on the type of
> the key.  To parse this syntax, you need to know the types, unlike JSON
> or traditional QemuOpts.  Unless we outlaw strings starting with '{' or
> '[', which feels impractical.

The parser having to know the types obviously precludes a design where a
simple parser feeds into a type-aware visitor, which is how the JSON ->
QObject -> QAPI pipeline works.

Perhaps less obviously, it even precludes a design where the parser is
derived from the QAPI schema to make it type-aware: the 'any' type
spoils that.

Consider -object/object-add and -device/device_add.  These take a few
statically known parameters such as "id".  Any additional parameters get
collected in a dictionary.  The schema represents it as value of type
'any'.  The code using this dictionary value matches its entries against
QOM properties.

To parse these additional parameters, knowing the (compile-time static)
schema isn't sufficient.  You also need to know the type of the QOM
property they'll get matched to at run-time.

This is a special case.  For the general case, you need to know how code
is going to use the value.  You generally can't know that until it's
done, so we'd have to parse lazily.  Thanks, but no thanks.

> But wait, there's another syntactic catch: in traditional QemuOpts, a
> value ends at the next unescaped ',' or '\0'.  Inside an object, it now
> also ends at the next unescaped '}', and inside an array, at the next
> unescaped ']'.  Or perhaps at the next space (the example above assumes
> it does).  That means we either have to provide a way to escape '}', ']'
> and space, or find another way to delimit string values, say require '"'
> around strings whenever the string contains "funny" characters.
>
> So, if escaped ',' wasn't ugly and confusing enough for you...
>
> === Comparison ===
>
> In my opinion, dotted keys are weird and ugly, but at least they don't
> add to the quoting mess.  Structured values look better, except when
> they do add to the quoting mess.
>
> I'm having a hard time deciding which one I like less :)
>
> Opinions?  Other ideas?
>
>
>
>
> [1] [PATCH v14 00/21] QAPI/QOM work for non-scalar object properties
> (actually v15)
> Message-Id: <1475246744-29302-1-git-send-email-berra...@redhat.com>
> http://lists.gnu.org/archive/html/qemu-devel/2016-09/msg08238.html
>
> [2] [RFC PATCH] block: Crude initial implementation of -blockdev
> Message-Id: <1485968933-9162-1-git-send-email-arm...@redhat.com>
> http://lists.gnu.org/archive/html/qemu-devel/2017-02/msg00182.html
>
> [3] Message-ID: <87h989ncse@dusky.pond.sub.org>
> http://lists.gnu.org/archive/html/qemu-devel/2016-10/msg04046.html

Re: [Qemu-devel] [PATCH RFC] migration: set cpu throttle value by workload

[Chao Fan]

On Fri, Jan 27, 2017 at 12:07:27PM +, Dr. David Alan Gilbert wrote:
>* Chao Fan (fanc.f...@cn.fujitsu.com) wrote:
>> Hi all,
>> 
>> This is a test for this RFC patch.
>> 
>> Start vm as following:
>> cmdline="./x86_64-softmmu/qemu-system-x86_64 -m 2560 \
>> -drive if=none,file=/nfs/img/fedora.qcow2,format=qcow2,id=foo \
>> -netdev tap,id=hn0,queues=1 \
>> -device virtio-net-pci,id=net-pci0,netdev=hn0 \
>> -device virtio-blk,drive=foo \
>> -enable-kvm -M pc -cpu host \
>> -vnc :3 \
>> -monitor stdio"
>> 
>> Continue running benchmark program named himeno[*](modified base on
>> original source). The code is in the attach file, make it in MIDDLE.
>> It costs much cpu calculation and memory. Then migrate the guest.
>> The source host and target host are in one switch.
>> 
>> "before" means the upstream version, "after" means applying this patch.
>> "idpr" means "inst_dirty_pages_rate", a new variable in this RFC PATCH.
>> "count" is "dirty sync count" in "info migrate".
>> "time" is "total time" in "info migrate".
>> "ct pct" is "cpu throttle percentage" in "info migrate".
>> 
>>  
>> | |before|after| 
>> |-|--|-| 
>> |count|time(s)|ct pct|time(s)| idpr |ct pct| 
>> |-|---|--|---|--|--| 
>> |  1  |3  |   0  |4  |   x  |   0  | 
>> |  2  |   53  |   0  |   53  | 14237|   0  | 
>> |  3  |   97  |   0  |   95  |  3142|   0  | 
>> |  4  |  109  |   0  |  105  | 11085|   0  | 
>> |  5  |  117  |   0  |  113  | 12894|   0  | 
>> |  6  |  125  |  20  |  121  | 13549|  67  | 
>> |  7  |  133  |  20  |  130  | 13550|  67  | 
>> |  8  |  141  |  20  |  136  | 13587|  67  | 
>> |  9  |  149  |  30  |  144  | 13553|  99  | 
>> | 10  |  156  |  30  |  152  |  1474|  99  |  
>> | 11  |  164  |  30  |  152  |  1706|  99  |  
>> | 12  |  172  |  40  |  153  |   0  |  99  |  
>> | 13  |  180  |  40  |  153  |   0  |   x  |  
>> | 14  |  188  |  40  |-|
>> | 15  |  195  |  50  |  completed  |  
>> | 16  |  203  |  50  | |  
>> | 17  |  211  |  50  | |  
>> | 18  |  219  |  60  | |  
>> | 19  |  227  |  60  | |  
>> | 20  |  235  |  60  | |  
>> | 21  |  242  |  70  | |  
>> | 22  |  250  |  70  | |  
>> | 23  |  258  |  70  | |  
>> | 24  |  266  |  80  | |  
>> | 25  |  274  |  80  | |  
>> | 26  |  281  |  80  | |  
>> | 27  |  289  |  90  | |  
>> | 28  |  297  |  90  | |  
>> | 29  |  305  |  90  | |  
>> | 30  |  315  |  99  | |  
>> | 31  |  320  |  99  | |  
>> | 32  |  320  |  99  | |  
>> | 33  |  321  |  99  | |  
>> | 34  |  321  |  99  | |  
>> || |
>> |completed   | |
>> 
>> 
>> And the "info migrate" when completed:
>> 
>> before:
>> capabilities: xbzrle: off rdma-pin-all: off auto-converge: on
>> zero-blocks: off compress: off events: off postcopy-ram: off x-colo: off 
>> Migration status: completed
>> total time: 321091 milliseconds
>> downtime: 573 milliseconds
>> setup: 40 milliseconds
>> transferred ram: 10509346 kbytes
>> throughput: 268.13 mbps
>> remaining ram: 0 kbytes
>> total ram: 2638664 kbytes
>> duplicate: 362439 pages
>> skipped: 0 pages
>> normal: 2621414 pages
>> normal bytes: 10485656 kbytes
>> dirty sync count: 34
>> 
>> after:
>> capabilities: xbzrle: off rdma-pin-all: off auto-converge: on
>> zero-blocks: off compress: off events: off postcopy-ram: off x-colo: off 
>> Migration status: completed
>> total time: 152652 milliseconds
>> downtime: 290 milliseconds
>> setup: 47 milliseconds
>> transferred ram: 4997452 kbytes
>> throughput: 268.20 mbps
>> remaining ram: 0 kbytes
>> total ram: 2638664 kbytes
>> duplicate: 359598 pages
>> skipped: 0 pages
>> normal: 1246136 pages
>> normal bytes: 4984544 kbytes
>> dirty sync count: 13
>> 
>> It's clear that the total time is much better(321s VS 153s).
>> The guest began cpu throttle in the 6th dirty sync. But at this time,
>> the dirty pages born too much in this guest. So the default
>> cpu throttle percentage(20 and 10) is too small for this condition. I
>> just use (inst_dirty_pages_rate / 200) to calculate the cpu throttle
>> value. This is just an adhoc algorithm, not supported by any theories. 
>> 
>> Of course on the other hand, the cpu throttle percentage is higher, the
>> guest runs more slowly. But in the result, after applying this patch,
>> the guest spend 23s with the cpu throttle percentage is 67 (total time
>> from 121 to 144), and 9s with cpu throttle percentage is 99 (total time
>> 

Re: [Qemu-devel] Non-flat command line option argument syntax

[Markus Armbruster]

Fam Zheng  writes:

> On Sat, 02/04 14:35, Markus Armbruster wrote:
>> Fam Zheng  writes:
>> 
>> > On Thu, 02/02 20:42, Markus Armbruster wrote:
>> >> === Comparison ===
>> >> 
>> >> In my opinion, dotted keys are weird and ugly, but at least they don't
>> >> add to the quoting mess.  Structured values look better, except when
>> >> they do add to the quoting mess.
>> >> 
>> >> I'm having a hard time deciding which one I like less :)
>> >> 
>> >> Opinions?  Other ideas?
>> >
>> > Here's my poor attempt:
>> >
>> > The dotted syntax, as the simpler of two, can cover everyday use very 
>> > well.  If
>> > we introduce an "@reference" extension to it which can help the 
>> > expresiveness,
>> > we can have a hybrid solution. It's not the cleanest interface and syntax, 
>> > but
>> > escaping, nesting and quoting can all be divide-and-conqured in their 
>> > optimal way.
>> > What I'm imagining is something like:
>> >
>> > -json "id=children0,text=[
>> > { 'driver': 'null-co://' },
>> > { 'driver': 'null-co://' },
>> > { 'driver': 'null-co://' }
>> > ]" \
>> > -dot \
>> >   
>> > id=quorum0,driver=quorum,read-pattern=fifo,vote-threshold=1,children=@children0
>> >  \
>> > -drive if=virtio,id=primary-disk0,driver=qcow2,file=@quorum0
>> >
>> > IOW "-json" and "-dot" define options that is intended to be referenced 
>> > from
>> > other dotted keys (quorum0 uses children0, and in turn primary-disk0 uses
>> > quorum0).
>> >
>> > Note: "-dot" here could be replaced with a -blockdev in this specific case 
>> > but
>> > I'm demostrating it just in case it is useful generically.
>> >
>> > Fam
>> 
>> KEY=@REF for references creates the same issue as KEY=[VALUE,...] for
>> arrays: you need to know the type of KEY to decide whether @REF is a
>> reference or a string, unless we outlaw strings starting with '@'.
>> 
>> Not a show-stopper, but it does preclude a design where a simple parser
>> feeds into a type-aware visitor, which is how the JSON -> QObject ->
>> QAPI pipeline works.
>> 
>> If you get creative in the VALUE part, you complicate the parser and/or
>> need to add quoting.
>> 
>> If you get creative in the KEY part, you need to restrict valid names.
>
> How about KEY@=REF?

Requires outlawing KEYs ending with '@'.  QAPI outlaws such names
already.  QOM doesn't, but I figure no such names exist so far.

[Qemu-devel] Query for PCI burst support

[Gaurav Sharma]

Is it possible to generate a PCIe burst of greater than 2dw for any MMIO.
I see we have limit of max 2dw operation for any MemoryRegion defined by
the max access size.

--Thanks

[Qemu-devel] [PATCH v3] softfloat: Add round-to-odd rounding mode

[Bharata B Rao]

Power ISA 3.0 introduces a few quadruple precision floating point
instructions that support round-to-odd rounding mode. The
round-to-odd mode is explained as under:

Let Z be the intermediate arithmetic result or the operand of a convert
operation. If Z can be represented exactly in the target format, the
result is Z. Otherwise the result is either Z1 or Z2 whichever is odd.
Here Z1 and Z2 are the next larger and smaller numbers representable
in the target format respectively.

Signed-off-by: Bharata B Rao 
Reviewed-by: Peter Maydell 
---
Tested the corner cases for round-to-odd and the default round-to-nearest-even
by comparing the results of QEMU with a known good implementation of
PowerPC ISA 3.0 for the following instructions: xsaddqp[o], xsdivqp[o],
xsmulqp[o] and xscvqpdp[o].

Changes in v3:

- Limited the recalculation of roundIncrement in 64bit rounding only
  to round-to-odd case and hence dropped the separate patch which
  did this for all rounding modes earlier. (Peter Maydell)
- Resorted to QEMU codying style in softfloat.c

v2: https://www.mail-archive.com/qemu-devel@nongnu.org/msg425741.html

 fpu/softfloat.c | 21 -
 include/fpu/softfloat.h |  2 ++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index c295f31..5ccba76 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -623,6 +623,9 @@ static float64 roundAndPackFloat64(flag zSign, int zExp, 
uint64_t zSig,
 case float_round_down:
 roundIncrement = zSign ? 0x3ff : 0;
 break;
+case float_round_to_odd:
+roundIncrement = (zSig & 0x400) ? 0 : 0x3ff;
+break;
 default:
 abort();
 }
@@ -632,8 +635,10 @@ static float64 roundAndPackFloat64(flag zSign, int zExp, 
uint64_t zSig,
  || (( zExp == 0x7FD )
   && ( (int64_t) ( zSig + roundIncrement ) < 0 ) )
) {
+bool overflow_to_inf = roundingMode != float_round_to_odd &&
+   roundIncrement != 0;
 float_raise(float_flag_overflow | float_flag_inexact, status);
-return packFloat64( zSign, 0x7FF, - ( roundIncrement == 0 ));
+return packFloat64(zSign, 0x7FF, -(!overflow_to_inf));
 }
 if ( zExp < 0 ) {
 if (status->flush_to_zero) {
@@ -651,6 +656,13 @@ static float64 roundAndPackFloat64(flag zSign, int zExp, 
uint64_t zSig,
 if (isTiny && roundBits) {
 float_raise(float_flag_underflow, status);
 }
+if (roundingMode == float_round_to_odd) {
+/*
+ * For round-to-odd case, the roundIncrement depends on
+ * zSig which just changed.
+ */
+roundIncrement = (zSig & 0x400) ? 0 : 0x3ff;
+}
 }
 }
 if (roundBits) {
@@ -1149,6 +1161,9 @@ static float128 roundAndPackFloat128(flag zSign, int32_t 
zExp,
 case float_round_down:
 increment = zSign && zSig2;
 break;
+case float_round_to_odd:
+increment = !(zSig1 & 0x1) && zSig2;
+break;
 default:
 abort();
 }
@@ -1168,6 +1183,7 @@ static float128 roundAndPackFloat128(flag zSign, int32_t 
zExp,
 if (( roundingMode == float_round_to_zero )
  || ( zSign && ( roundingMode == float_round_up ) )
  || ( ! zSign && ( roundingMode == float_round_down ) )
+ || (roundingMode == float_round_to_odd)
) {
 return
 packFloat128(
@@ -1215,6 +1231,9 @@ static float128 roundAndPackFloat128(flag zSign, int32_t 
zExp,
 case float_round_down:
 increment = zSign && zSig2;
 break;
+case float_round_to_odd:
+increment = !(zSig1 & 0x1) && zSig2;
+break;
 default:
 abort();
 }
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index 842ec6b..8a39028 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -180,6 +180,8 @@ enum {
 float_round_up   = 2,
 float_round_to_zero  = 3,
 float_round_ties_away= 4,
+/* Not an IEEE rounding mode: round to the closest odd mantissa value */
+float_round_to_odd   = 5,
 };
 
 /*
-- 
2.7.4

Re: [Qemu-devel] [PATCH v12 03/16] qemu-io: Set "share-rw" flag together with read-only

[Fam Zheng]

On Sun, 02/05 22:52, Max Reitz wrote:
> On 23.01.2017 13:30, Fam Zheng wrote:
> > qemu-io is a low level tool to read or modify guest visible data, which
> > implies the user knows very well what is being done. Allowing reading
> > from a locked image is harmless in most cases, so do it.
> > 
> > Signed-off-by: Fam Zheng 
> > ---
> >  qemu-io.c | 2 ++
> >  1 file changed, 2 insertions(+)
> 
> What about open_f()?

Right, should do the same.

Fam

[Qemu-devel] [Bug 498417] Re: cache=writeback on disk image doesn't do write-back

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  cache=writeback on disk image doesn't do write-back

Status in QEMU:
  Expired

Bug description:
  I noticed that qemu seems to have poor disk performance.  Here's a
  test that has miserable performance but which should be really fast:

  - Configure qemu to use the disk image with cache=writeback
  - Configure a 2GiB Linux VM on an 8GiB Linux host
  - In the VM, write a 4GiB file (dd if=/dev/zero of=/tmp/x bs=4K count=1M)
  - In the VM, read it back (dd if=/tmp/x of=/dev/null bs=4K count=1M)

  With writeback, the whole file should end up in the host pagecache.
  So when I read it back, there should be no I/O to the real disk, and
  it should be really fast.  Instead, I see disk activity through the
  duration of the test, and the performance is roughly the native hard
  disk throughput (somewhat slower).

  I'm using version 0.11.1, and this is my command line:

  qemu-system-x86_64 -drive
  cache=writeback,index=0,media=disk,file=ubuntu.img -k en-us -m 2048
  -smp 2 -vnc :3100 -usbdevice tablet -enable-kvm &

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

[Qemu-devel] [Bug 584516] Re: opensuse 11.2 guest hangs after live migration with clocksource=kvm-clock

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  opensuse 11.2 guest hangs after live migration with clocksource=kvm-
  clock

Status in QEMU:
  Expired

Bug description:
  i would like to debug a problem that I encountered some time ago with 
opensuse 11.2 and also
  with Ubuntu (karmic/lucid).

  If I run an opensuse guest 64-bit and do not touch the clocksource settings 
the guest almost
  everytime hangs after live migration at:

  (gdb) thread apply all bt

  Thread 2 (Thread 0x7f846782a950 (LWP 27356)):
  #0  0x7f8467d24cd7 in ioctl () from /lib/libc.so.6
  #1  0x0042b945 in kvm_run (env=0x2468170)
at /usr/src/qemu-kvm-0.12.4/qemu-kvm.c:921
  #2  0x0042cea2 in kvm_cpu_exec (env=0x2468170)
at /usr/src/qemu-kvm-0.12.4/qemu-kvm.c:1651
  #3  0x0042d62c in kvm_main_loop_cpu (env=0x2468170)
at /usr/src/qemu-kvm-0.12.4/qemu-kvm.c:1893
  #4  0x0042d76d in ap_main_loop (_env=0x2468170)
at /usr/src/qemu-kvm-0.12.4/qemu-kvm.c:1943
  #5  0x7f8468caa3ba in start_thread () from /lib/libpthread.so.0
  #6  0x7f8467d2cfcd in clone () from /lib/libc.so.6
  #7  0x in ?? ()

  Thread 1 (Thread 0x7f84692d96f0 (LWP 27353)):
  #0  0x7f8467d25742 in select () from /lib/libc.so.6
  #1  0x0040c25a in main_loop_wait (timeout=1000)
at /usr/src/qemu-kvm-0.12.4/vl.c:3994
  #2  0x0042dcf1 in kvm_main_loop ()
at /usr/src/qemu-kvm-0.12.4/qemu-kvm.c:2126
  #3  0x0040c98c in main_loop () at /usr/src/qemu-kvm-0.12.4/vl.c:4212
  #4  0x0041054b in main (argc=31, argv=0x7fffa91351c8,
envp=0x7fffa91352c8) at /usr/src/qemu-kvm-0.12.4/vl.c:6252

  If I run the same guest with kernel parameter clocksource=acpi_pm, the
  migration succeeds reliably.

  The hosts runs:
  /kernel: /2.6.33.3, /bin: /qemu-kvm-0.12.4, /mod: /2.6.33.3

  I invoke qemu-kvm with:
  /usr/bin/qemu-kvm-0.12.4  -net none  -drive 
file=/dev/sdb,if=ide,boot=on,cache=none,aio=native  -m 1024 -cpu 
qemu64,model_id='Intel(R) Xeon(R) CPU   E5430  @ 2.66GHz'  -monitor 
tcp:0:4001,server,nowait -vnc :1 -name 'test'  -boot order=dc,menu=on  -k de  
-pidfile /var/run/qemu/vm-149.pid  -mem-path /hugepages -mem-prealloc  -rtc 
base=utc,clock=vm -usb -usbdevice tablet

  The Guest is:
  OpenSuse 11.2 64-bit with Kernel 2.6.31.5-0.1-desktop #1 SMP PREEMPT 
2009-10-26 15:49:03 +0100 x86_64
  The clocksource automatically choosen is kvm-clock.

  Feedback appreciated. I have observed the same problem with 0.12.2 and
  also with old binaries provided by Ubuntu Karmic (kvm-88).

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

Re: [Qemu-devel] [PATCH] ui/vnc: Drop unused vnc_has_job() and vnc_jobs_clear()

[Gonglei]

On 2017/2/4 2:24, Peter Maydell wrote:
> The functions vnc_has_job() and vnc_jobs_clear() are
> never used; remove them.
> 
> Signed-off-by: Peter Maydell 
> ---
> 
> I last had a go at this five years ago:
> http://patchwork.ozlabs.org/patch/192335/
> 
> Since then vnc_stop_worker_thread() got removed anyway in
> commit 09526058d, but the vnc_jobs_clear() function that
> it was the only caller of got left behind.
> 
> vnc_has_job() has never been used by anybody, and it's
> impossible to use in a non-racy way, so drop it too.
> 
>  ui/vnc-jobs.h |  2 --
>  ui/vnc-jobs.c | 23 ---
>  2 files changed, 25 deletions(-)

Reviewed-by: Gonglei 

[Qemu-devel] [Bug 1662050] Re: qemu-img convert a overlay qcow2 image into a entire image

[wayen]

** Summary changed:

- qemu-img convert a overlay qcow2 image into a complete image
+ qemu-img convert a overlay qcow2 image into a entire image

** Description changed:

  I have a base image file "base.qcow2" and a delta qcow2 image file
  "delta.qcow2" whose backing file is "base.qcow2".
  
  Now I use qemu-img to convert "delta.qcow2" and will get a new image
- file "new.qcow2" which is complete and equivalent to combination of
+ file "new.qcow2" which is entire and equivalent to combination of
  "base.qcow2" and "delta.qcow2".
  
  In fact, i just want to convert the delta qcow2 image file and get a new
  delta overlay qcow2 image file. So the "new.qcow2" is not what i want. I
  have to admit that this is not bug. Could you please take this as a new
  feature and enable qemu-img to convert images in-place?

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

Title:
  qemu-img convert a overlay qcow2 image into a entire image

Status in QEMU:
  New

Bug description:
  I have a base image file "base.qcow2" and a delta qcow2 image file
  "delta.qcow2" whose backing file is "base.qcow2".

  Now I use qemu-img to convert "delta.qcow2" and will get a new image
  file "new.qcow2" which is entire and equivalent to combination of
  "base.qcow2" and "delta.qcow2".

  In fact, i just want to convert the delta qcow2 image file and get a
  new delta overlay qcow2 image file. So the "new.qcow2" is not what i
  want. I have to admit that this is not bug. Could you please take this
  as a new feature and enable qemu-img to convert images in-place?

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

Re: [Qemu-devel] [PATCH] virtio-gpu: fix memory leak in set scanout

[Li Qiang]

Hello Gerd,

Ping.

2017-01-23 17:09 GMT+08:00 Marc-André Lureau :

>
>
> On Sun, Jan 22, 2017 at 11:43 AM Li Qiang  wrote:
>
>> From: Li Qiang 
>>
>> In virtio_gpu_set_scanout function, when creating the 'rect'
>> its refcount is set to 2, by pixman_image_create_bits and
>> qemu_create_displaysurface_pixman function. This can lead
>> a memory leak issues. This patch avoid this issue.
>>
>> Signed-off-by: Li Qiang 
>>
>
>
> Reviewed-by: Marc-André Lureau 
>
>
>
>> ---
>>  hw/display/virtio-gpu.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
>> index 7a15c61..4aecea3 100644
>> --- a/hw/display/virtio-gpu.c
>> +++ b/hw/display/virtio-gpu.c
>> @@ -608,6 +608,7 @@ static void virtio_gpu_set_scanout(VirtIOGPU *g,
>>  cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
>>  return;
>>  }
>> +pixman_image_unref(rect);
>>  dpy_gfx_replace_surface(g->scanout[ss.scanout_id].con,
>> scanout->ds);
>>  }
>>
>> --
>> 1.8.3.1
>>
>>
>> --
> Marc-André Lureau
>

[Qemu-devel] [Bug 1662050] Re: qemu-img convert a delta qcow2 image into a complete image

[wayen]

** Summary changed:

- qemu-img convert a delta qcow2 image into a complete image
+ qemu-img convert a overlay qcow2 image into a complete image

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

Title:
  qemu-img convert a overlay qcow2 image into a complete image

Status in QEMU:
  New

Bug description:
  I have a base image file "base.qcow2" and a delta qcow2 image file
  "delta.qcow2" whose backing file is "base.qcow2".

  Now I use qemu-img to convert "delta.qcow2" and will get a new image
  file "new.qcow2" which is complete and equivalent to combination of
  "base.qcow2" and "delta.qcow2".

  In fact, i just want to convert the delta qcow2 image file and get a
  new delta overlay qcow2 image file. So the "new.qcow2" is not what i
  want. I have to admit that this is not bug. Could you please take this
  as a new feature and enable qemu-img to convert images in-place?

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

[Qemu-devel] [Bug 1662050] [NEW] qemu-img convert a delta qcow2 image into a complete image

[wayen]

Public bug reported:

I have a base image file "base.qcow2" and a delta qcow2 image file
"delta.qcow2" whose backing file is "base.qcow2".

Now I use qemu-img to convert "delta.qcow2" and will get a new image
file "new.qcow2" which is complete and equivalent to combination of
"base.qcow2" and "delta.qcow2".

In fact, i just want to convert the delta qcow2 image file and get a new
delta overlay qcow2 image file. So the "new.qcow2" is not what i want. I
have to admit that this is not bug. Could you please take this as a new
feature and enable qemu-img to convert images in-place?

** Affects: qemu
 Importance: Undecided
 Status: New


** Tags: feature

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

Title:
  qemu-img convert a delta qcow2 image into a complete image

Status in QEMU:
  New

Bug description:
  I have a base image file "base.qcow2" and a delta qcow2 image file
  "delta.qcow2" whose backing file is "base.qcow2".

  Now I use qemu-img to convert "delta.qcow2" and will get a new image
  file "new.qcow2" which is complete and equivalent to combination of
  "base.qcow2" and "delta.qcow2".

  In fact, i just want to convert the delta qcow2 image file and get a
  new delta overlay qcow2 image file. So the "new.qcow2" is not what i
  want. I have to admit that this is not bug. Could you please take this
  as a new feature and enable qemu-img to convert images in-place?

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

Re: [Qemu-devel] [PATCH 0/4] POWER9 TCG enablements - part 13

[David Gibson]

On Fri, Feb 03, 2017 at 08:01:13PM -0200, Jose Ricardo Ziviani wrote:
> This set contains 4 new instructions
> 
> xsrqpi[x] - VSX Scalar Round to Quad-Precision Integer
> xsrqpxp - VSX Scalar Round Quad-Precision to Double-Extended Precision
> xssqrtqp - VSX Scalar Square Root Quad-Precision
> xssubqp - VSX Scalar Subtract Quad-Precision
> 
> Note:
>  - xssqrtqpo and xssubqpo will be implemented when round-to-odd is
> ready.

Applied to ppc-for-2.9.

Note that I haven't reviewed these particularly closely.  Since
they're new instructions, I figure it won't cause a regression, even
if they're buggy.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v6 02/18] vfio: introduce vfio_get_vaddr()

[David Gibson]

On Fri, Feb 03, 2017 at 04:22:28PM +0800, Peter Xu wrote:
> A cleanup for vfio_iommu_map_notify(). Now we will fetch vaddr even if
> the operation is unmap, but it won't hurt much.
> 
> One thing to mention is that we need the RCU read lock to protect the
> whole translation and map/unmap procedure.
> 
> Signed-off-by: Peter Xu 

Reviewed-by: David Gibson 

> ---
>  hw/vfio/common.c | 65 
> +++-
>  1 file changed, 45 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 174f351..42c4790 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -294,54 +294,79 @@ static bool 
> vfio_listener_skipped_section(MemoryRegionSection *section)
> section->offset_within_address_space & (1ULL << 63);
>  }
>  
> -static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
> +/* Called with rcu_read_lock held.  */
> +static bool vfio_get_vaddr(IOMMUTLBEntry *iotlb, void **vaddr,
> +   bool *read_only)
>  {
> -VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n);
> -VFIOContainer *container = giommu->container;
> -hwaddr iova = iotlb->iova + giommu->iommu_offset;
>  MemoryRegion *mr;
>  hwaddr xlat;
>  hwaddr len = iotlb->addr_mask + 1;
> -void *vaddr;
> -int ret;
> -
> -trace_vfio_iommu_map_notify(iotlb->perm == IOMMU_NONE ? "UNMAP" : "MAP",
> -iova, iova + iotlb->addr_mask);
> -
> -if (iotlb->target_as != _space_memory) {
> -error_report("Wrong target AS \"%s\", only system memory is allowed",
> - iotlb->target_as->name ? iotlb->target_as->name : 
> "none");
> -return;
> -}
> +bool writable = iotlb->perm & IOMMU_WO;
>  
>  /*
>   * The IOMMU TLB entry we have just covers translation through
>   * this IOMMU to its immediate target.  We need to translate
>   * it the rest of the way through to memory.
>   */
> -rcu_read_lock();
>  mr = address_space_translate(_space_memory,
>   iotlb->translated_addr,
> - , , iotlb->perm & IOMMU_WO);
> + , , writable);
>  if (!memory_region_is_ram(mr)) {
>  error_report("iommu map to non memory area %"HWADDR_PRIx"",
>   xlat);
> -goto out;
> +return false;
>  }
> +
>  /*
>   * Translation truncates length to the IOMMU page size,
>   * check that it did not truncate too much.
>   */
>  if (len & iotlb->addr_mask) {
>  error_report("iommu has granularity incompatible with target AS");
> +return false;
> +}
> +
> +*vaddr = memory_region_get_ram_ptr(mr) + xlat;
> +*read_only = !writable || mr->readonly;
> +
> +return true;
> +}
> +
> +static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
> +{
> +VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n);
> +VFIOContainer *container = giommu->container;
> +hwaddr iova = iotlb->iova + giommu->iommu_offset;
> +bool read_only;
> +void *vaddr;
> +int ret;
> +
> +trace_vfio_iommu_map_notify(iotlb->perm == IOMMU_NONE ? "UNMAP" : "MAP",
> +iova, iova + iotlb->addr_mask);
> +
> +if (iotlb->target_as != _space_memory) {
> +error_report("Wrong target AS \"%s\", only system memory is allowed",
> + iotlb->target_as->name ? iotlb->target_as->name : 
> "none");
> +return;
> +}
> +
> +rcu_read_lock();
> +
> +if (!vfio_get_vaddr(iotlb, , _only)) {
>  goto out;
>  }
>  
>  if ((iotlb->perm & IOMMU_RW) != IOMMU_NONE) {
> -vaddr = memory_region_get_ram_ptr(mr) + xlat;
> +/*
> + * vaddr is only valid until rcu_read_unlock(). But after
> + * vfio_dma_map has set up the mapping the pages will be
> + * pinned by the kernel. This makes sure that the RAM backend
> + * of vaddr will always be there, even if the memory object is
> + * destroyed and its backing memory munmap-ed.
> + */
>  ret = vfio_dma_map(container, iova,
> iotlb->addr_mask + 1, vaddr,
> -   !(iotlb->perm & IOMMU_WO) || mr->readonly);
> +   read_only);
>  if (ret) {
>  error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", "
>   "0x%"HWADDR_PRIx", %p) = %d (%m)",

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v16 1/2] virtio-crypto: Add virtio crypto device specification

[Gonglei (Arei)]

Hi Stefan,

I'm just back from Chinese New year's holiday.


>
> Subject: Re: [Qemu-devel] [PATCH v16 1/2] virtio-crypto: Add virtio crypto
> device specification
> 
> On Wed, Jan 18, 2017 at 04:22:36PM +0800, Gonglei wrote:
> > The virtio crypto device is a virtual crypto device (ie. hardware
> > crypto accelerator card). Currently, the virtio crypto device provides
> > the following crypto services: CIPHER, MAC, HASH, and AEAD.
> >
> > In this patch, CIPHER, MAC, HASH, AEAD services are introduced.
> >
> > VIRTIO-153
> >
> > Signed-off-by: Gonglei 
> > CC: Michael S. Tsirkin 
> > CC: Cornelia Huck 
> > CC: Stefan Hajnoczi 
> > CC: Lingli Deng 
> > CC: Jani Kokkonen 
> > CC: Ola Liljedahl 
> > CC: Varun Sethi 
> > CC: Zeng Xin 
> > CC: Keating Brian 
> > CC: Ma Liang J 
> > CC: Griffin John 
> > CC: Mihai Claudiu Caraman 
> > ---
> >  content.tex   |2 +
> >  virtio-crypto.tex | 1245
> +
> >  2 files changed, 1247 insertions(+)
> >  create mode 100644 virtio-crypto.tex
> 
> I'm happy with the device overall.  See specific comments below.
> 
> Reviewed-by: Stefan Hajnoczi 
> 
Thanks!

> >
> > diff --git a/content.tex b/content.tex
> > index 4b45678..ab75f78 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -5750,6 +5750,8 @@ descriptor for the \field{sense_len},
> \field{residual},
> >  \field{status_qualifier}, \field{status}, \field{response} and
> >  \field{sense} fields.
> >
> > +\input{virtio-crypto.tex}
> > +
> >  \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
> >
> >  Currently there are three device-independent feature bits defined:
> > diff --git a/virtio-crypto.tex b/virtio-crypto.tex
> > new file mode 100644
> > index 000..732cd30
> > --- /dev/null
> > +++ b/virtio-crypto.tex
> > @@ -0,0 +1,1245 @@
> > +\section{Crypto Device}\label{sec:Device Types / Crypto Device}
> > +
> > +The virtio crypto device is a virtual cryptography device as well as a 
> > kind of
> > +virtual hardware accelerator for virtual machines. The encryption and
> > +decryption requests are placed in any of the data active queues and are
> ultimately handled by the
> > +backend crypto accelerators. The second kind of queue is the control queue
> used to create
> > +or destroy sessions for symmetric algorithms and will control some
> advanced
> > +features in the future. The virtio crypto device provides the following 
> > crypto
> > +services: CIPHER, MAC, HASH, and AEAD.
> > +
> > +
> > +\subsection{Device ID}\label{sec:Device Types / Crypto Device / Device ID}
> > +
> > +20
> > +
> > +\subsection{Virtqueues}\label{sec:Device Types / Crypto Device /
> Virtqueues}
> > +
> > +\begin{description}
> > +\item[0] dataq1
> > +\item[\ldots]
> > +\item[N-1] dataqN
> > +\item[N] controlq
> > +\end{description}
> > +
> > +N is set by \field{max_dataqueues}.
> > +
> > +\subsection{Feature bits}\label{sec:Device Types / Crypto Device / Feature
> bits}
> > +
> > +VIRTIO_CRYPTO_F_NON_SESSION_MODE (0) non-session mode is available.
> > +VIRTIO_CRYPTO_F_CIPHER_NON_SESSION_MODE (1) non-session mode is
> available for CIPHER service.
> > +VIRTIO_CRYPTO_F_HASH_NON_SESSION_MODE (2) non-session mode is
> available for HASH service.
> > +VIRTIO_CRYPTO_F_MAC_NON_SESSION_MODE (3) non-session mode is
> available for MAC service.
> > +VIRTIO_CRYPTO_F_AEAD_NON_SESSION_MODE (4) non-session mode is
> available for AEAD service.
> 
> Bits 1-4 require bit 0.  Is bit 0 necessary at all?  Or may bits 1-4 can
> be eliminated in favor of just bit 0.
> 
I tried to explain why those bits are necessary in below thread: 

https://www.mail-archive.com/qemu-devel@nongnu.org/msg423072.html


> > +
> > +\subsubsection{Feature bit requirements}\label{sec:Device Types / Crypto
> Device / Feature bits}
> > +
> > +Some crypto feature bits require other crypto feature bits
> > +(see \ref{drivernormative:Basic Facilities of a Virtio Device / Feature 
> > Bits}):
> > +
> > +\begin{description}
> > +\item[VIRTIO_CRYPTO_F_CIPHER_NON_SESSION_MODE] Requires
> VIRTIO_CRYPTO_F_NON_SESSION_MODE.
> > +\item[VIRTIO_CRYPTO_F_HASH_NON_SESSION_MODE] Requires
> VIRTIO_CRYPTO_F_NON_SESSION_MODE.
> > +\item[VIRTIO_CRYPTO_F_MAC_NON_SESSION_MODE] Requires
> VIRTIO_CRYPTO_F_NON_SESSION_MODE.
> > +\item[VIRTIO_CRYPTO_F_AEAD_NON_SESSION_MODE] Requires
> VIRTIO_CRYPTO_F_NON_SESSION_MODE.
> > +\end{description}
> > +
> > +\subsection{Device configuration layout}\label{sec:Device Types / Crypto
> Device / Device configuration layout}
> > +
> > +The following driver-read-only configuration fields are defined:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_config {
> > +le32 

Re: [Qemu-devel] [PATCH v16 2/2] virtio-crypto: Add conformance clauses

[Gonglei (Arei)]

>
> Subject: Re: [Qemu-devel] [PATCH v16 2/2] virtio-crypto: Add conformance
> clauses
> 
> On Wed, Jan 18, 2017 at 04:22:37PM +0800, Gonglei wrote:
> > Add the conformance targets and clauses for
> > virtio-crypto device.
> >
> > Signed-off-by: Gonglei 
> > ---
> >  conformance.tex | 30 ++
> >  1 file changed, 30 insertions(+)
> 
> Reviewed-by: Stefan Hajnoczi 

Thanks!

-Gonglei

Re: [Qemu-devel] [PULL 0/3] isa-cleanup queue 20170206

[no-reply]

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PULL 0/3] isa-cleanup queue 20170206
Message-id: 20170206013927.13693-1-da...@gibson.dropbear.id.au

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]  
patchew/1486110164-13797-1-git-send-email-pet...@redhat.com -> 
patchew/1486110164-13797-1-git-send-email-pet...@redhat.com
 - [tag update]  
patchew/1486144722-6021-1-git-send-email-peter.mayd...@linaro.org -> 
patchew/1486144722-6021-1-git-send-email-peter.mayd...@linaro.org
 - [tag update]  
patchew/1486148371-11692-1-git-send-email-peter.mayd...@linaro.org -> 
patchew/1486148371-11692-1-git-send-email-peter.mayd...@linaro.org
 - [tag update]  
patchew/1486249533-5260-1-git-send-email-peter.mayd...@linaro.org -> 
patchew/1486249533-5260-1-git-send-email-peter.mayd...@linaro.org
 - [tag update]  patchew/20170123123056.30383-1-f...@redhat.com -> 
patchew/20170123123056.30383-1-f...@redhat.com
 - [tag update]  patchew/20170203170729.12399-1-pbonz...@redhat.com -> 
patchew/20170203170729.12399-1-pbonz...@redhat.com
 * [new tag] patchew/20170206013927.13693-1-da...@gibson.dropbear.id.au 
-> patchew/20170206013927.13693-1-da...@gibson.dropbear.id.au
Switched to a new branch 'test'
55da952 Split ISA and sysbus versions of m48t59 device
b58cd33 Allow ISA bus to be configured out
0782c85 Split serial-isa into its own config option

=== OUTPUT BEGIN ===
Checking PATCH 1/3: Split serial-isa into its own config option...
Checking PATCH 2/3: Allow ISA bus to be configured out...
Checking PATCH 3/3: Split ISA and sysbus versions of m48t59 device...
ERROR: do not use C99 // comments
#67: FILE: hw/timer/m48t59-internal.h:28:
+//#define DEBUG_NVRAM

total: 1 errors, 0 warnings, 614 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org

[Qemu-devel] [PULL 3/3] Split ISA and sysbus versions of m48t59 device

[David Gibson]

The m48t59 device supports both ISA and direct sysbus attached versions of
the device in the one .c file.  This can be awkward for some embedded
machine types which need the sysbus M48T59, but don't want to pull in the
ISA bus code and its other dependencies.

Therefore, this patch splits out the code for the ISA attached M48T59 into
its own C file.  It will be built when both CONFIG_M48T59 and
CONFIG_ISA_BUS are enabled.

Signed-off-by: David Gibson 
Reviewed-by: Edgar E. Iglesias 
Reviewed-by: Michael S. Tsirkin 
---
 hw/timer/Makefile.objs |   3 +
 hw/timer/m48t59-internal.h |  82 
 hw/timer/m48t59-isa.c  | 181 +++
 hw/timer/m48t59.c  | 228 -
 4 files changed, 284 insertions(+), 210 deletions(-)
 create mode 100644 hw/timer/m48t59-internal.h
 create mode 100644 hw/timer/m48t59-isa.c

diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
index 71994f2..fc99668 100644
--- a/hw/timer/Makefile.objs
+++ b/hw/timer/Makefile.objs
@@ -6,6 +6,9 @@ common-obj-$(CONFIG_DS1338) += ds1338.o
 common-obj-$(CONFIG_HPET) += hpet.o
 common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o
 common-obj-$(CONFIG_M48T59) += m48t59.o
+ifeq ($(CONFIG_ISA_BUS),y)
+common-obj-$(CONFIG_M48T59) += m48t59-isa.o
+endif
 common-obj-$(CONFIG_PL031) += pl031.o
 common-obj-$(CONFIG_PUV3) += puv3_ost.o
 common-obj-$(CONFIG_TWL92230) += twl92230.o
diff --git a/hw/timer/m48t59-internal.h b/hw/timer/m48t59-internal.h
new file mode 100644
index 000..32ae957
--- /dev/null
+++ b/hw/timer/m48t59-internal.h
@@ -0,0 +1,82 @@
+/*
+ * QEMU M48T59 and M48T08 NVRAM emulation (common header)
+ *
+ * Copyright (c) 2003-2005, 2007 Jocelyn Mayer
+ * Copyright (c) 2013 Hervé Poussineau
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#ifndef HW_M48T59_INTERNAL_H
+#define HW_M48T59_INTERNAL_H 1
+
+//#define DEBUG_NVRAM
+
+#if defined(DEBUG_NVRAM)
+#define NVRAM_PRINTF(fmt, ...) do { printf(fmt , ## __VA_ARGS__); } while (0)
+#else
+#define NVRAM_PRINTF(fmt, ...) do { } while (0)
+#endif
+
+/*
+ * The M48T02, M48T08 and M48T59 chips are very similar. The newer '59 has
+ * alarm and a watchdog timer and related control registers. In the
+ * PPC platform there is also a nvram lock function.
+ */
+
+typedef struct M48txxInfo {
+const char *bus_name;
+uint32_t model; /* 2 = m48t02, 8 = m48t08, 59 = m48t59 */
+uint32_t size;
+} M48txxInfo;
+
+typedef struct M48t59State {
+/* Hardware parameters */
+qemu_irq IRQ;
+MemoryRegion iomem;
+uint32_t size;
+int32_t base_year;
+/* RTC management */
+time_t   time_offset;
+time_t   stop_time;
+/* Alarm & watchdog */
+struct tm alarm;
+QEMUTimer *alrm_timer;
+QEMUTimer *wd_timer;
+/* NVRAM storage */
+uint8_t *buffer;
+/* Model parameters */
+uint32_t model; /* 2 = m48t02, 8 = m48t08, 59 = m48t59 */
+/* NVRAM storage */
+uint16_t addr;
+uint8_t  lock;
+} M48t59State;
+
+uint32_t m48t59_read(M48t59State *NVRAM, uint32_t addr);
+void m48t59_write(M48t59State *NVRAM, uint32_t addr, uint32_t val);
+void m48t59_reset_common(M48t59State *NVRAM);
+void m48t59_realize_common(M48t59State *s, Error **errp);
+
+static inline void m48t59_toggle_lock(M48t59State *NVRAM, int lock)
+{
+NVRAM->lock ^= 1 << lock;
+}
+
+extern const MemoryRegionOps m48t59_io_ops;
+
+#endif /* HW_M48T59_INTERNAL_H */
diff --git a/hw/timer/m48t59-isa.c b/hw/timer/m48t59-isa.c
new file mode 100644
index 000..ea1ba70
--- /dev/null
+++ b/hw/timer/m48t59-isa.c
@@ -0,0 +1,181 @@
+/*
+ * QEMU M48T59 and M48T08 NVRAM emulation (ISA bus interface
+ *
+ * Copyright (c) 2003-2005, 2007 Jocelyn Mayer
+ * Copyright (c) 2013 Hervé Poussineau
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated 

[Qemu-devel] [PULL 1/3] Split serial-isa into its own config option

[David Gibson]

At present, the core device model code for 8250-like serial ports
(serial.c) and the code for serial ports attached to ISA-style legacy IO
(serial-isa.c) are both controlled by the CONFIG_SERIAL variable.

There are lots and lots of embedded platforms that have 8250-like serial
ports but have never had anything resembling ISA legacy IO.  Therefore,
split serial-isa into its own CONFIG_SERIAL_ISA option so it can be
disabled for platforms where it's not appropriate.

For now, I enabled CONFIG_SERIAL_ISA in every default-config where
CONFIG_SERIAL is enabled, excepting microblaze, or32, and xtensa.  As best
as I can tell, those platforms never used legacy ISA, and also don't
include PCI support (which would allow connection of a PCI->ISA bridge
and/or a southbridge including legacy ISA serial ports).

Signed-off-by: David Gibson 
Reviewed-by: Thomas Huth 
Acked-by: Christian Borntraeger 
Reviewed-by: Markus Armbruster 
Reviewed-by: Edgar E. Iglesias 
Reviewed-by: Michael S. Tsirkin 
---
 default-configs/alpha-softmmu.mak   | 1 +
 default-configs/arm-softmmu.mak | 1 +
 default-configs/i386-softmmu.mak| 1 +
 default-configs/mips-softmmu-common.mak | 1 +
 default-configs/moxie-softmmu.mak   | 1 +
 default-configs/pci.mak | 1 +
 default-configs/ppc-softmmu.mak | 1 +
 default-configs/ppc64-softmmu.mak   | 1 +
 default-configs/ppcemb-softmmu.mak  | 1 +
 default-configs/sh4-softmmu.mak | 1 +
 default-configs/sh4eb-softmmu.mak   | 1 +
 default-configs/sparc64-softmmu.mak | 1 +
 default-configs/x86_64-softmmu.mak  | 1 +
 hw/char/Makefile.objs   | 3 ++-
 14 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/default-configs/alpha-softmmu.mak 
b/default-configs/alpha-softmmu.mak
index 7f6161e..e0d75e3 100644
--- a/default-configs/alpha-softmmu.mak
+++ b/default-configs/alpha-softmmu.mak
@@ -3,6 +3,7 @@
 include pci.mak
 include usb.mak
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_I8254=y
 CONFIG_PCKBD=y
 CONFIG_VGA_CIRRUS=y
diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 6f2a180..824fa71 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -6,6 +6,7 @@ CONFIG_VGA=y
 CONFIG_NAND=y
 CONFIG_ECC=y
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_PTIMER=y
 CONFIG_SD=y
 CONFIG_MAX7310=y
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 384cefb..48b07a4 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -15,6 +15,7 @@ CONFIG_IPMI_EXTERN=y
 CONFIG_ISA_IPMI_KCS=y
 CONFIG_ISA_IPMI_BT=y
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_PARALLEL=y
 CONFIG_I8254=y
 CONFIG_PCSPK=y
diff --git a/default-configs/mips-softmmu-common.mak 
b/default-configs/mips-softmmu-common.mak
index f0676f5..7d8f5db 100644
--- a/default-configs/mips-softmmu-common.mak
+++ b/default-configs/mips-softmmu-common.mak
@@ -9,6 +9,7 @@ CONFIG_VGA_ISA_MM=y
 CONFIG_VGA_CIRRUS=y
 CONFIG_VMWARE_VGA=y
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_PARALLEL=y
 CONFIG_I8254=y
 CONFIG_PCSPK=y
diff --git a/default-configs/moxie-softmmu.mak 
b/default-configs/moxie-softmmu.mak
index 1a95476..7e22863 100644
--- a/default-configs/moxie-softmmu.mak
+++ b/default-configs/moxie-softmmu.mak
@@ -2,4 +2,5 @@
 
 CONFIG_MC146818RTC=y
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_VGA=y
diff --git a/default-configs/pci.mak b/default-configs/pci.mak
index fff7ce3..d8d6548 100644
--- a/default-configs/pci.mak
+++ b/default-configs/pci.mak
@@ -27,6 +27,7 @@ CONFIG_AHCI=y
 CONFIG_ESP=y
 CONFIG_ESP_PCI=y
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_SERIAL_PCI=y
 CONFIG_IPACK=y
 CONFIG_WDT_IB6300ESB=y
diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
index 7dd004e..09c1d45 100644
--- a/default-configs/ppc-softmmu.mak
+++ b/default-configs/ppc-softmmu.mak
@@ -46,6 +46,7 @@ CONFIG_PLATFORM_BUS=y
 CONFIG_ETSEC=y
 CONFIG_LIBDECNUMBER=y
 # For PReP
+CONFIG_SERIAL_ISA=y
 CONFIG_MC146818RTC=y
 CONFIG_ISA_TESTDEV=y
 CONFIG_RS6000_MC=y
diff --git a/default-configs/ppc64-softmmu.mak 
b/default-configs/ppc64-softmmu.mak
index 9ae6563..05c8335 100644
--- a/default-configs/ppc64-softmmu.mak
+++ b/default-configs/ppc64-softmmu.mak
@@ -52,6 +52,7 @@ CONFIG_XICS=$(CONFIG_PSERIES)
 CONFIG_XICS_SPAPR=$(CONFIG_PSERIES)
 CONFIG_XICS_KVM=$(and $(CONFIG_PSERIES),$(CONFIG_KVM))
 # For PReP
+CONFIG_SERIAL_ISA=y
 CONFIG_MC146818RTC=y
 CONFIG_ISA_TESTDEV=y
 CONFIG_MEM_HOTPLUG=y
diff --git a/default-configs/ppcemb-softmmu.mak 
b/default-configs/ppcemb-softmmu.mak
index 54acc4d..7f56004 100644
--- a/default-configs/ppcemb-softmmu.mak
+++ b/default-configs/ppcemb-softmmu.mak
@@ -5,6 +5,7 @@ include sound.mak
 include usb.mak
 CONFIG_M48T59=y
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_I8257=y
 CONFIG_OPENPIC=y
 

Re: [Qemu-devel] [PATCH v6 01/18] vfio: trace map/unmap for notify as well

[David Gibson]

On Fri, Feb 03, 2017 at 04:22:27PM +0800, Peter Xu wrote:
> We traces its range, but we don't know whether it's a MAP/UNMAP. Let's
> dump it as well.
> 
> Acked-by: Alex Williamson 
> Signed-off-by: Peter Xu 

Reviewed-by: David Gibson 

> ---
>  hw/vfio/common.c | 3 ++-
>  hw/vfio/trace-events | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 801578b..174f351 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -305,7 +305,8 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, 
> IOMMUTLBEntry *iotlb)
>  void *vaddr;
>  int ret;
>  
> -trace_vfio_iommu_map_notify(iova, iova + iotlb->addr_mask);
> +trace_vfio_iommu_map_notify(iotlb->perm == IOMMU_NONE ? "UNMAP" : "MAP",
> +iova, iova + iotlb->addr_mask);
>  
>  if (iotlb->target_as != _space_memory) {
>  error_report("Wrong target AS \"%s\", only system memory is allowed",
> diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
> index 8de8281..2561c6d 100644
> --- a/hw/vfio/trace-events
> +++ b/hw/vfio/trace-events
> @@ -84,7 +84,7 @@ vfio_pci_igd_lpc_bridge_enabled(const char *name) "%s"
>  # hw/vfio/common.c
>  vfio_region_write(const char *name, int index, uint64_t addr, uint64_t data, 
> unsigned size) " (%s:region%d+0x%"PRIx64", 0x%"PRIx64 ", %d)"
>  vfio_region_read(char *name, int index, uint64_t addr, unsigned size, 
> uint64_t data) " (%s:region%d+0x%"PRIx64", %d) = 0x%"PRIx64
> -vfio_iommu_map_notify(uint64_t iova_start, uint64_t iova_end) "iommu map @ 
> %"PRIx64" - %"PRIx64
> +vfio_iommu_map_notify(const char *op, uint64_t iova_start, uint64_t 
> iova_end) "iommu %s @ %"PRIx64" - %"PRIx64
>  vfio_listener_region_add_skip(uint64_t start, uint64_t end) "SKIPPING 
> region_add %"PRIx64" - %"PRIx64
>  vfio_listener_region_add_iommu(uint64_t start, uint64_t end) "region_add 
> [iommu] %"PRIx64" - %"PRIx64
>  vfio_listener_region_add_ram(uint64_t iova_start, uint64_t iova_end, void 
> *vaddr) "region_add [ram] %"PRIx64" - %"PRIx64" [%p]"

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


signature.asc
Description: PGP signature

[Qemu-devel] [PULL 2/3] Allow ISA bus to be configured out

[David Gibson]

Currently, the code to handle the legacy ISA bus is always included in
qemu.  However there are lots of platforms that don't include ISA legacy
devies, and quite a few that have never used ISA legacy devices at all.

This patch allows the ISA bus code to be disabled in the configuration for
platforms where it doesn't make sense.

For now, the default configs are adjusted to include ISA on all platforms
including PCI: anything with PCI can at least in principle add an i82378
PCI->ISA bridge.  Also, CONFIG_IDE_CORE which is already in pci.mak
requires ISA support.

We also explicitly enable ISA on some other non-PCI platforms which include
ISA devices: moxie, sparc and unicore32.  We may want to pare this down in
future.

The platforms that will lose ISA by default are: cris, lm32, microblazeel,
microblaze, openrisc, s390x, tricore, xtensaeb, xtensa.  As far as I can
tell none of these ever used ISA.

Signed-off-by: David Gibson 
Acked-by: Michael S. Tsirkin 
Reviewed-by: Markus Armbruster 
Reviewed-by: Edgar E. Iglesias 
Reviewed-by: Michael S. Tsirkin 
---
 default-configs/moxie-softmmu.mak | 1 +
 default-configs/pci.mak   | 2 ++
 default-configs/sparc-softmmu.mak | 1 +
 default-configs/unicore32-softmmu.mak | 1 +
 hw/isa/Makefile.objs  | 2 +-
 5 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/default-configs/moxie-softmmu.mak 
b/default-configs/moxie-softmmu.mak
index 7e22863..e00d099 100644
--- a/default-configs/moxie-softmmu.mak
+++ b/default-configs/moxie-softmmu.mak
@@ -1,5 +1,6 @@
 # Default configuration for moxie-softmmu
 
+CONFIG_ISA_BUS=y
 CONFIG_MC146818RTC=y
 CONFIG_SERIAL=y
 CONFIG_SERIAL_ISA=y
diff --git a/default-configs/pci.mak b/default-configs/pci.mak
index d8d6548..60dc651 100644
--- a/default-configs/pci.mak
+++ b/default-configs/pci.mak
@@ -1,4 +1,6 @@
 CONFIG_PCI=y
+# For now, CONFIG_IDE_CORE requires ISA, so we enable it here
+CONFIG_ISA_BUS=y
 CONFIG_VIRTIO_PCI=y
 CONFIG_VIRTIO=y
 CONFIG_USB_UHCI=y
diff --git a/default-configs/sparc-softmmu.mak 
b/default-configs/sparc-softmmu.mak
index ab796b3..004b0f4 100644
--- a/default-configs/sparc-softmmu.mak
+++ b/default-configs/sparc-softmmu.mak
@@ -1,5 +1,6 @@
 # Default configuration for sparc-softmmu
 
+CONFIG_ISA_BUS=y
 CONFIG_ECC=y
 CONFIG_ESP=y
 CONFIG_ESCC=y
diff --git a/default-configs/unicore32-softmmu.mak 
b/default-configs/unicore32-softmmu.mak
index de38577..5f6c4a8 100644
--- a/default-configs/unicore32-softmmu.mak
+++ b/default-configs/unicore32-softmmu.mak
@@ -1,4 +1,5 @@
 # Default configuration for unicore32-softmmu
+CONFIG_ISA_BUS=y
 CONFIG_PUV3=y
 CONFIG_PTIMER=y
 CONFIG_PCKBD=y
diff --git a/hw/isa/Makefile.objs b/hw/isa/Makefile.objs
index 9164556..fb37c55 100644
--- a/hw/isa/Makefile.objs
+++ b/hw/isa/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-y += isa-bus.o
+common-obj-$(CONFIG_ISA_BUS) += isa-bus.o
 common-obj-$(CONFIG_APM) += apm.o
 common-obj-$(CONFIG_I82378) += i82378.o
 common-obj-$(CONFIG_PC87312) += pc87312.o
-- 
2.9.3

[Qemu-devel] [PULL 0/3] isa-cleanup queue 20170206

[David Gibson]

The following changes since commit a951316b8a5c3c63254f20a826afeed940dd4cba:

  Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into 
staging (2017-02-03 14:41:49 +)

are available in the git repository at:

  git://github.com/dgibson/qemu.git tags/isa-cleanup-20170206

for you to fetch changes up to c124c4d13bcb19fc866e7f6de075f906fca6af4a:

  Split ISA and sysbus versions of m48t59 device (2017-02-06 12:33:21 +1100)


Allow ISA to be disabled on some platforms (v3)

This makes some cleanups that are a start on allowing ISA to be
compiled out for platforms which don't use it.

I posted this series last November, and it collected a number of R-bs
and no apparent objections.  So, I've now rebased it (trivially) and
am sending a pull request in the hopes of merge.  A lot of the pieces
here don't have a clear maintainer, so I'm sending it directly to
Peter.

Notes:
  * Patch 3/3 triggers a style warning, but that's just because I'm
moving a C++ // comment verbatim from one file to another

Changes since v2:
  * Trivial rebase

Changes since v1:
  * Fixed some silly compile errors in 3/3 exposed by some
changes in other headers


David Gibson (3):
  Split serial-isa into its own config option
  Allow ISA bus to be configured out
  Split ISA and sysbus versions of m48t59 device

 default-configs/alpha-softmmu.mak   |   1 +
 default-configs/arm-softmmu.mak |   1 +
 default-configs/i386-softmmu.mak|   1 +
 default-configs/mips-softmmu-common.mak |   1 +
 default-configs/moxie-softmmu.mak   |   2 +
 default-configs/pci.mak |   3 +
 default-configs/ppc-softmmu.mak |   1 +
 default-configs/ppc64-softmmu.mak   |   1 +
 default-configs/ppcemb-softmmu.mak  |   1 +
 default-configs/sh4-softmmu.mak |   1 +
 default-configs/sh4eb-softmmu.mak   |   1 +
 default-configs/sparc-softmmu.mak   |   1 +
 default-configs/sparc64-softmmu.mak |   1 +
 default-configs/unicore32-softmmu.mak   |   1 +
 default-configs/x86_64-softmmu.mak  |   1 +
 hw/char/Makefile.objs   |   3 +-
 hw/isa/Makefile.objs|   2 +-
 hw/timer/Makefile.objs  |   3 +
 hw/timer/m48t59-internal.h  |  82 
 hw/timer/m48t59-isa.c   | 181 +
 hw/timer/m48t59.c   | 228 +++-
 21 files changed, 305 insertions(+), 212 deletions(-)
 create mode 100644 hw/timer/m48t59-internal.h
 create mode 100644 hw/timer/m48t59-isa.c

Re: [Qemu-devel] [PATCH 0/3] spapr: fix cpu core hotunplug call flow

[David Gibson]

On Thu, Feb 02, 2017 at 04:02:32PM +0100, Igor Mammedov wrote:
> Make cpu core hotunplug use the same (expected)
> unplug_request/unplug call flow like spapr memory unplug
> and mem/cpu unplug on x86.
> 
> While doing it separate internal cpu core handling and
> machine wiring code and move machine related callbacks
> close to related machine code in spapr.c.
> 
> Series applies on top of tags/ppc-for-2.9-20170202 pull req.
> 
> repo for testing:
>   g...@github.com:imammedo/qemu.git spapr_cpu_unplug_cleanup
> 
> CC: David Gibson  (supporter:sPAPR)
> CC: Alexander Graf  (supporter:sPAPR)
> CC: qemu-...@nongnu.org 
> CC: Bharata B Rao 

Merged to ppc-for-2.9.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [Qemu-ppc] [PATCH 0/4] POWER9 TCG enablements - part 13

[David Gibson]

On Fri, Feb 03, 2017 at 11:29:37PM -0200, jos...@linux.vnet.ibm.com wrote:
> On Fri, Feb 03, 2017 at 02:11:31PM -0800, no-re...@patchew.org wrote:
> > Hi,
> > 
> > Your series seems to have some coding style problems. See output below for
> > more information:
> > 
> > Type: series
> > Subject: [Qemu-devel] [PATCH 0/4] POWER9 TCG enablements - part 13
> > Message-id: 1486159277-25949-1-git-send-email-jos...@linux.vnet.ibm.com
> > 
> > === TEST SCRIPT BEGIN ===
> > #!/bin/bash
> > 
> > BASE=base
> > n=1
> > total=$(git log --oneline $BASE.. | wc -l)
> > failed=0
> > 
> > # Useful git options
> > git config --local diff.renamelimit 0
> > git config --local diff.renames True
> > 
> > commits="$(git log --format=%H --reverse $BASE..)"
> > for c in $commits; do
> > echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
> > if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; 
> > then
> > failed=1
> > echo
> > fi
> > n=$((n+1))
> > done
> > 
> > exit $failed
> > === TEST SCRIPT END ===
> > 
> > Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> > From https://github.com/patchew-project/qemu
> >  * [new tag] 
> > patchew/1486159277-25949-1-git-send-email-jos...@linux.vnet.ibm.com -> 
> > patchew/1486159277-25949-1-git-send-email-jos...@linux.vnet.ibm.com
> >  - [tag update]  patchew/20170203120254.15062-1-berra...@redhat.com -> 
> > patchew/20170203120254.15062-1-berra...@redhat.com
> > Switched to a new branch 'test'
> > 52f618a ppc: implement xssubqp instruction
> > f7dfdbf ppc: implement xssqrtqp instruction
> > 4d895a4 ppc: implement xsrqpxp instruction
> > 5d49c07 ppc: implement xsrqpi[x] instruction
> > 
> > === OUTPUT BEGIN ===
> > Checking PATCH 1/4: ppc: implement xsrqpi[x] instruction...
> > ERROR: Macros with complex values should be enclosed in parenthesis
> > #125: FILE: target/ppc/translate/vsx-ops.inc.c:106:
> > +#define GEN_VSX_Z23FORM_300(name, opc2, opc3, opc4, inval) \
> > +GEN_VSX_XFORM_300_EO(name, opc2, opc3 | 0x00, opc4 | 0x0, inval), \
> > +GEN_VSX_XFORM_300_EO(name, opc2, opc3 | 0x08, opc4 | 0x0, inval), \
> > +GEN_VSX_XFORM_300_EO(name, opc2, opc3 | 0x10, opc4 | 0x0, inval), \
> > +GEN_VSX_XFORM_300_EO(name, opc2, opc3 | 0x18, opc4 | 0x0, inval), \
> > +GEN_VSX_XFORM_300_EO(name, opc2, opc3 | 0x00, opc4 | 0x1, inval), \
> > +GEN_VSX_XFORM_300_EO(name, opc2, opc3 | 0x08, opc4 | 0x1, inval), \
> > +GEN_VSX_XFORM_300_EO(name, opc2, opc3 | 0x10, opc4 | 0x1, inval), \
> > +GEN_VSX_XFORM_300_EO(name, opc2, opc3 | 0x18, opc4 | 0x1, inval)
> > 
> 
> I tried to improve it but this style is used everywhere

Yeah, that's ok.  These instruction generation macros trigger known
false positives in checkpatch.  I think it thinks these are macro
definitions, rather than macro invocations.

> 
> > total: 1 errors, 0 warnings, 103 lines checked
> > 
> > Your patch has style problems, please review.  If any of these errors
> > are false positives report them to the maintainer, see
> > CHECKPATCH in MAINTAINERS.
> > 
> > Checking PATCH 2/4: ppc: implement xsrqpxp instruction...
> > Checking PATCH 3/4: ppc: implement xssqrtqp instruction...
> > Checking PATCH 4/4: ppc: implement xssubqp instruction...
> > === OUTPUT END ===
> > 
> > Test command exited with code: 1
> > 
> > 
> > ---
> > Email generated automatically by Patchew [http://patchew.org/].
> > Please send your feedback to patchew-de...@freelists.org
> 

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v12 00/16] block: Image locking series

[Max Reitz]

On 23.01.2017 13:30, Fam Zheng wrote:
> v12: Fix test cases on old systems that doesn't have F_OFD_SETLK, such as RHEL
>  6. [Patchew]
>  Trim the commit message of patch 15 to avoid bitrotting.
> 
> v11: Move lock bytes from 1-2 to 0x10-0x12. [Daniel]
> 
> v10: While we still don't have comprehensive propagation mechanism that will 
> be
> provided by new op blocker system for "permissive modes", the locking enabled
> by default is regardlessly useful and long overdue. So I think we should merge
> this for 2.9 and build user options on top later when the op blocker API
> settles.

Reasonable, but it's worth noting that on my to-review list op blockers
actually have priority over this for 2.9...

(Or rather, they would have if there was a series posted.)

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v12 05/16] block: Set "share-rw" flag in drive-backup when sync=none

[Max Reitz]

On 23.01.2017 13:30, Fam Zheng wrote:
> In this case we may open the source's backing image chain multiple
> times. Setting share flag means the new open won't try to acquire or
> check any lock, once we implement image locking.
> 
> Signed-off-by: Fam Zheng 
> ---
>  blockdev.c | 3 +++
>  1 file changed, 3 insertions(+)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v12 04/16] qemu-img: Set "share-rw" flag in read-only commands

[Max Reitz]

On 23.01.2017 13:30, Fam Zheng wrote:
> Checking the status of an image when it is being used by guest is
> usually useful, and there is no risk of corrupting data, so don't let
> the upcoming image locking feature limit this use case.
> 
> Signed-off-by: Fam Zheng 
> ---
>  qemu-img.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v12 03/16] qemu-io: Set "share-rw" flag together with read-only

[Max Reitz]

On 23.01.2017 13:30, Fam Zheng wrote:
> qemu-io is a low level tool to read or modify guest visible data, which
> implies the user knows very well what is being done. Allowing reading
> from a locked image is harmless in most cases, so do it.
> 
> Signed-off-by: Fam Zheng 
> ---
>  qemu-io.c | 2 ++
>  1 file changed, 2 insertions(+)

What about open_f()?

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v12 02/16] block: Define BDRV_O_SHARE_RW

[Max Reitz]

On 23.01.2017 13:30, Fam Zheng wrote:
> Signed-off-by: Fam Zheng 
> ---
>  include/block/block.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/block/block.h b/include/block/block.h
> index 8b0dcda..243839d 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -97,6 +97,8 @@ typedef struct HDGeometry {
>select an appropriate protocol driver,
>ignoring the format layer */
>  #define BDRV_O_NO_IO   0x1 /* don't initialize for I/O */
> +#define BDRV_O_SHARE_RW0x2 /* accept shared read-write from other 
> users
> +  of the image */
>  
>  #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_NO_FLUSH)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v12 01/16] osdep: Add qemu_lock_fd and qemu_unlock_fd

[Max Reitz]

On 23.01.2017 13:30, Fam Zheng wrote:
> They are wrappers of POSIX fcntl "file private locking", with a
> convenient "try lock" wrapper implemented with F_OFD_GETLK.
> 
> Signed-off-by: Fam Zheng 
> ---
>  include/qemu/osdep.h |  3 +++
>  util/osdep.c | 48 
>  2 files changed, 51 insertions(+)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [Qemu-block] Non-flat command line option argument syntax

[Max Reitz]

On 04.02.2017 10:51, Markus Armbruster wrote:
> "Richard W.M. Jones"  writes:
> 
>> On Thu, Feb 02, 2017 at 08:42:33PM +0100, Markus Armbruster wrote:
>>> There's also the -drive file=json:... syntax.  It's a bad fit for
>>> QemuOpts, because QemuOpts and JSON fight for the comma.  I'd show you
>>> if I could get it to work.
>>
>> I think this refers to the json: syntax for qemu URIs?
> 
> The json: pseudo-protocol, i.e. "file names" starting with a "json:"
> protocol prefix.  I think we mean the same.
> 
>> Anyway we're using this in virt-v2v
>> (https://github.com/libguestfs/libguestfs/blob/master/v2v/input_libvirt_xen_ssh.ml)
>>
>> It's very useful, please try not to break it!
> 
> We're not going to take the json: pseudo-protocol away.  Perhaps we'll
> deprecate it at some point, but even then it'll stay for a sufficiently
> generous grace period.

It should be noted that there are no current plans to deprecate it. It
won't be superseded by command line changes, considering that it was
originally not introduced to make configuration on the command line more
beautiful but to allow e.g. backing file strings to contain more than
just a filename.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [Qemu-trivial] [PATCH] test-vmstate: remove yield_until_fd_readable

[Laurent Vivier]

Le 03/02/2017 à 18:07, Paolo Bonzini a écrit :
> The function is not needed anymore now that migration is built on
> top of QIOChannel.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  tests/test-vmstate.c | 11 ---
>  1 file changed, 11 deletions(-)
> 
> diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
> index 9d87faf..0c2af4d 100644
> --- a/tests/test-vmstate.c
> +++ b/tests/test-vmstate.c
> @@ -33,17 +33,6 @@
>  static char temp_file[] = "/tmp/vmst.test.XX";
>  static int temp_fd;
>  
> -/* Fake yield_until_fd_readable() implementation so we don't have to pull the
> - * coroutine code as dependency.
> - */
> -void yield_until_fd_readable(int fd)
> -{
> -fd_set fds;
> -FD_ZERO();
> -FD_SET(fd, );
> -select(fd + 1, , NULL, NULL, NULL);
> -}
> -
>  
>  /* Duplicate temp_fd and seek to the beginning of the file */
>  static QEMUFile *open_test_file(bool write)
> 

Tested-by: Laurent Vivier 

Re: [Qemu-devel] [Qemu-trivial] [PATCH] softfloat: Use correct type in float64_to_uint64_round_to_zero()

[Laurent Vivier]

Le 03/02/2017 à 19:59, Peter Maydell a écrit :
> In float64_to_uint64_round_to_zero() a typo meant that we were
> taking the uint64_t return value from float64_to_uint64() and
> putting it into an int64_t variable before returning it as
> uint64_t again. Use uint64_t instead of pointlessly casting it
> back and forth to int64_t.
> 
> Signed-off-by: Peter Maydell 

Reviewed-by: Laurent Vivier 

> ---
> Spotted while reading the code...
> 
>  fpu/softfloat.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index c295f31..218b375 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -7386,7 +7386,7 @@ uint64_t float64_to_uint64_round_to_zero(float64 a, 
> float_status *status)
>  {
>  signed char current_rounding_mode = status->float_rounding_mode;
>  set_float_rounding_mode(float_round_to_zero, status);
> -int64_t v = float64_to_uint64(a, status);
> +uint64_t v = float64_to_uint64(a, status);
>  set_float_rounding_mode(current_rounding_mode, status);
>  return v;
>  }
> 

Re: [Qemu-devel] [Qemu-trivial] [PATCH] CODING_STYLE: Mention preferred comment form

[Laurent Vivier]

Le 03/02/2017 à 18:58, Peter Maydell a écrit :
> Our defacto coding style strongly prefers /* */ style comments
> over the single-line // style, and checkpatch enforces this,
> but we don't actually document this. Mention it in CODING_STYLE.
> 
> Suggested-by: Thomas Huth 
> Signed-off-by: Peter Maydell 
> ---
>  CODING_STYLE | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/CODING_STYLE b/CODING_STYLE
> index f53180b..2fa0c0b 100644
> --- a/CODING_STYLE
> +++ b/CODING_STYLE
> @@ -116,3 +116,10 @@ if (a == 1) {
>  Rationale: Yoda conditions (as in 'if (1 == a)') are awkward to read.
>  Besides, good compilers already warn users when '==' is mis-typed as '=',
>  even when the constant is on the right.
> +
> +7. Comment style
> +
> +We use traditional C-style /* */ comments and avoid // comments.
> +
> +Rationale: The // form is valid in C99, so this is purely a matter of
> +consistency of style. The checkpatch script will warn you about this.
> 

Reviewed-by: Laurent Vivier 

Re: [Qemu-devel] [PATCH V2 01/10] qemu-clk: introduce qemu-clk qom object

[Cédric Le Goater]

ello Frederic,

On 01/26/2017 10:47 AM, fred.kon...@greensocs.com wrote:
> From: KONRAD Frederic 
> 
> This introduces qemu-clk qom object.
> 
> Signed-off-by: KONRAD Frederic 
> ---
>  Makefile.objs |  1 +
>  include/qemu/qemu-clock.h | 40 +
>  qemu-clock.c  | 50 
> +++
>  3 files changed, 91 insertions(+)
>  create mode 100644 include/qemu/qemu-clock.h
>  create mode 100644 qemu-clock.c
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index 01cef86..de0219d 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -78,6 +78,7 @@ common-obj-y += backends/
>  common-obj-$(CONFIG_SECCOMP) += qemu-seccomp.o
>  
>  common-obj-$(CONFIG_FDT) += device_tree.o
> +common-obj-y += qemu-clock.o
>  
>  ##
>  # qapi
> diff --git a/include/qemu/qemu-clock.h b/include/qemu/qemu-clock.h
> new file mode 100644
> index 000..e7acd68
> --- /dev/null
> +++ b/include/qemu/qemu-clock.h
> @@ -0,0 +1,40 @@
> +/*
> + * QEMU Clock
> + *
> + *  Copyright (C) 2016 : GreenSocs Ltd
> + *  http://www.greensocs.com/ , email: i...@greensocs.com
> + *
> + *  Frederic Konrad 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see .

May be we could shorten the GPL v2 statement to something like :
 
 * This code is licensed under the GPL version 2 or later. See the
 * COPYING file in the top-level directory.

I haven't been very good at that my self, but we could save 
quite a few lines in qemu if files were not repeating the 
License statement.


> + *
> + */
> +
> +#ifndef QEMU_CLOCK_H
> +#define QEMU_CLOCK_H
> +
> +#include "qemu/osdep.h"
> +#include "qom/object.h"
> +
> +#define TYPE_CLOCK "qemu-clk"
> +#define QEMU_CLOCK(obj) OBJECT_CHECK(struct qemu_clk, (obj), TYPE_CLOCK)
> +
> +typedef struct qemu_clk {
> +/*< private >*/
> +Object parent_obj;
> +} *qemu_clk;
>

CODING_STYLE says that we should use CamelCase for typedef. Also, I don't 
think the '*' is required. What's the idea ? 

> +#endif /* QEMU_CLOCK_H */
> +
> +
> diff --git a/qemu-clock.c b/qemu-clock.c
> new file mode 100644
> index 000..ceea98d
> --- /dev/null
> +++ b/qemu-clock.c
> @@ -0,0 +1,50 @@
> +/*
> + * QEMU Clock
> + *
> + *  Copyright (C) 2016 : GreenSocs Ltd
> + *  http://www.greensocs.com/ , email: i...@greensocs.com
> + *
> + *  Frederic Konrad 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see .
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/qemu-clock.h"
> +#include "hw/hw.h"
> +#include "qemu/log.h"
> +
> +#ifndef DEBUG_QEMU_CLOCK
> +#define DEBUG_QEMU_CLOCK 0
> +#endif
> +
> +#define DPRINTF(fmt, args...) do {   
> \
> +if (DEBUG_QEMU_CLOCK) {  
> \
> +qemu_log("%s: " fmt, __func__, ## args); 
> \
> +}
> \
> +} while (0);

I think the trace framework should be used instead.

Thanks

C.

> +static const TypeInfo qemu_clk_info = {
> +.name  = TYPE_CLOCK,
> +.parent= TYPE_OBJECT,
> +.instance_size = sizeof(struct qemu_clk),
> +};
> +
> +static void qemu_clk_register_types(void)
> +{
> +type_register_static(_clk_info);
> +}
> +
> +type_init(qemu_clk_register_types);
> 

Re: [Qemu-devel] [PATCH] linux-user: Use correct types in load_symbols()

[Laurent Vivier]

Le 05/02/2017 à 00:05, Peter Maydell a écrit :
> Coverity doesn't like the code in load_symbols() which assumes
> it can use 'int' for a variable that might hold an offset into
> the guest ELF file, because in a 64-bit guest that could
> overflow. Guest binaries with 2GB sections aren't very likely
> and this isn't a security issue because we fully trust the
> guest linux-user binary anyway, but we might as well use the
> right types, which will placate Coverity. Use uint64_t to
> hold section sizes, and bail out if the symbol table is too
> large rather than just overflowing an int.
> 
> (Coverity issue CID1005776)
> 
> Signed-off-by: Peter Maydell 

Reviewed-by: Laurent Vivier 

> ---
>  linux-user/elfload.c | 22 +++---
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index c66cbbe..f4c7b0c 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -2263,6 +2263,7 @@ static int symcmp(const void *s0, const void *s1)
>  static void load_symbols(struct elfhdr *hdr, int fd, abi_ulong load_bias)
>  {
>  int i, shnum, nsyms, sym_idx = 0, str_idx = 0;
> +uint64_t segsz;
>  struct elf_shdr *shdr;
>  char *strings = NULL;
>  struct syminfo *s = NULL;
> @@ -2294,19 +2295,26 @@ static void load_symbols(struct elfhdr *hdr, int fd, 
> abi_ulong load_bias)
>  goto give_up;
>  }
>  
> -i = shdr[str_idx].sh_size;
> -s->disas_strtab = strings = g_try_malloc(i);
> -if (!strings || pread(fd, strings, i, shdr[str_idx].sh_offset) != i) {
> +segsz = shdr[str_idx].sh_size;
> +s->disas_strtab = strings = g_try_malloc(segsz);
> +if (!strings ||
> +pread(fd, strings, segsz, shdr[str_idx].sh_offset) != segsz) {
>  goto give_up;
>  }
>  
> -i = shdr[sym_idx].sh_size;
> -syms = g_try_malloc(i);
> -if (!syms || pread(fd, syms, i, shdr[sym_idx].sh_offset) != i) {
> +segsz = shdr[sym_idx].sh_size;
> +syms = g_try_malloc(segsz);
> +if (!syms || pread(fd, syms, segsz, shdr[sym_idx].sh_offset) != segsz) {
>  goto give_up;
>  }
>  
> -nsyms = i / sizeof(struct elf_sym);
> +if (segsz / sizeof(struct elf_sym) > INT_MAX) {
> +/* Implausibly large symbol table: give up rather than ploughing
> + * on with the number of symbols calculation overflowing
> + */
> +goto give_up;
> +}
> +nsyms = segsz / sizeof(struct elf_sym);
>  for (i = 0; i < nsyms; ) {
>  bswap_sym(syms + i);
>  /* Throw away entries which we do not need.  */
> 

[Qemu-devel] [RESEND PATCH 2/9] hw/dma: QOM'ify sparc32_dma.c

[xiaoqiang zhao]

Drop the old SysBus init function and use instance_init
and an realize function

Signed-off-by: xiaoqiang zhao 
---
 hw/dma/sparc32_dma.c | 25 +++--
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index 9d545e412e..ef4c6a93c8 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -270,23 +270,28 @@ static const VMStateDescription vmstate_dma = {
 }
 };
 
-static int sparc32_dma_init1(SysBusDevice *sbd)
+static void sparc32_dma_init(Object *obj)
 {
-DeviceState *dev = DEVICE(sbd);
-DMAState *s = SPARC32_DMA(dev);
-int reg_size;
+DeviceState *dev = DEVICE(obj);
+DMAState *s = SPARC32_DMA(obj);
+SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
 
 sysbus_init_irq(sbd, >irq);
 
-reg_size = s->is_ledma ? DMA_ETH_SIZE : DMA_SIZE;
-memory_region_init_io(>iomem, OBJECT(s), _mem_ops, s,
-  "dma", reg_size);
 sysbus_init_mmio(sbd, >iomem);
 
 qdev_init_gpio_in(dev, dma_set_irq, 1);
 qdev_init_gpio_out(dev, s->gpio, 2);
+}
 
-return 0;
+static void sparc32_dma_realize(DeviceState *dev, Error **errp)
+{
+DMAState *s = SPARC32_DMA(dev);
+int reg_size;
+
+reg_size = s->is_ledma ? DMA_ETH_SIZE : DMA_SIZE;
+memory_region_init_io(>iomem, OBJECT(dev), _mem_ops, s,
+  "dma", reg_size);
 }
 
 static Property sparc32_dma_properties[] = {
@@ -298,12 +303,11 @@ static Property sparc32_dma_properties[] = {
 static void sparc32_dma_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
-SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-k->init = sparc32_dma_init1;
 dc->reset = dma_reset;
 dc->vmsd = _dma;
 dc->props = sparc32_dma_properties;
+dc->realize = sparc32_dma_realize;
 /* Reason: pointer property "iommu_opaque" */
 dc->cannot_instantiate_with_device_add_yet = true;
 }
@@ -312,6 +316,7 @@ static const TypeInfo sparc32_dma_info = {
 .name  = TYPE_SPARC32_DMA,
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(DMAState),
+.instance_init = sparc32_dma_init,
 .class_init= sparc32_dma_class_init,
 };
 
-- 
2.11.0

[Qemu-devel] [RESEND PATCH 3/9] hw/dma: QOM'ify sun4m_iommu.c

[xiaoqiang zhao]

Drop the old SysBus init function and use instance_init

Signed-off-by: xiaoqiang zhao 
---
 hw/dma/sun4m_iommu.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/hw/dma/sun4m_iommu.c b/hw/dma/sun4m_iommu.c
index b3cbc54c23..335ef63cbc 100644
--- a/hw/dma/sun4m_iommu.c
+++ b/hw/dma/sun4m_iommu.c
@@ -349,17 +349,16 @@ static void iommu_reset(DeviceState *d)
 s->regs[IOMMU_MASK_ID] = IOMMU_TS_MASK;
 }
 
-static int iommu_init1(SysBusDevice *dev)
+static void iommu_init(Object *obj)
 {
-IOMMUState *s = SUN4M_IOMMU(dev);
+IOMMUState *s = SUN4M_IOMMU(obj);
+SysBusDevice *dev = SYS_BUS_DEVICE(obj);
 
 sysbus_init_irq(dev, >irq);
 
-memory_region_init_io(>iomem, OBJECT(s), _mem_ops, s, "iommu",
+memory_region_init_io(>iomem, obj, _mem_ops, s, "iommu",
   IOMMU_NREGS * sizeof(uint32_t));
 sysbus_init_mmio(dev, >iomem);
-
-return 0;
 }
 
 static Property iommu_properties[] = {
@@ -370,9 +369,7 @@ static Property iommu_properties[] = {
 static void iommu_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
-SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-k->init = iommu_init1;
 dc->reset = iommu_reset;
 dc->vmsd = _iommu;
 dc->props = iommu_properties;
@@ -382,6 +379,7 @@ static const TypeInfo iommu_info = {
 .name  = TYPE_SUN4M_IOMMU,
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(IOMMUState),
+.instance_init = iommu_init,
 .class_init= iommu_class_init,
 };
 
-- 
2.11.0

[Qemu-devel] [RESEND PATCH 4/9] hw/misc: QOM'ify slavio_misc.c

[xiaoqiang zhao]

Drop the old SysBus init function and use instance_init

Signed-off-by: xiaoqiang zhao 
---
 hw/misc/slavio_misc.c | 43 +--
 1 file changed, 17 insertions(+), 26 deletions(-)

diff --git a/hw/misc/slavio_misc.c b/hw/misc/slavio_misc.c
index edd5de0702..e1a706e828 100644
--- a/hw/misc/slavio_misc.c
+++ b/hw/misc/slavio_misc.c
@@ -414,76 +414,73 @@ static const VMStateDescription vmstate_misc = {
 }
 };
 
-static int apc_init1(SysBusDevice *dev)
+static void apc_init(Object *obj)
 {
-APCState *s = APC(dev);
+APCState *s = APC(obj);
+SysBusDevice *dev = SYS_BUS_DEVICE(obj);
 
 sysbus_init_irq(dev, >cpu_halt);
 
 /* Power management (APC) XXX: not a Slavio device */
-memory_region_init_io(>iomem, OBJECT(s), _mem_ops, s,
+memory_region_init_io(>iomem, obj, _mem_ops, s,
   "apc", MISC_SIZE);
 sysbus_init_mmio(dev, >iomem);
-return 0;
 }
 
-static int slavio_misc_init1(SysBusDevice *sbd)
+static void slavio_misc_init(Object *obj)
 {
-DeviceState *dev = DEVICE(sbd);
-MiscState *s = SLAVIO_MISC(dev);
+DeviceState *dev = DEVICE(obj);
+MiscState *s = SLAVIO_MISC(obj);
+SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
 
 sysbus_init_irq(sbd, >irq);
 sysbus_init_irq(sbd, >fdc_tc);
 
 /* 8 bit registers */
 /* Slavio control */
-memory_region_init_io(>cfg_iomem, OBJECT(s), _cfg_mem_ops, s,
+memory_region_init_io(>cfg_iomem, obj, _cfg_mem_ops, s,
   "configuration", MISC_SIZE);
 sysbus_init_mmio(sbd, >cfg_iomem);
 
 /* Diagnostics */
-memory_region_init_io(>diag_iomem, OBJECT(s), _diag_mem_ops, s,
+memory_region_init_io(>diag_iomem, obj, _diag_mem_ops, s,
   "diagnostic", MISC_SIZE);
 sysbus_init_mmio(sbd, >diag_iomem);
 
 /* Modem control */
-memory_region_init_io(>mdm_iomem, OBJECT(s), _mdm_mem_ops, s,
+memory_region_init_io(>mdm_iomem, obj, _mdm_mem_ops, s,
   "modem", MISC_SIZE);
 sysbus_init_mmio(sbd, >mdm_iomem);
 
 /* 16 bit registers */
 /* ss600mp diag LEDs */
-memory_region_init_io(>led_iomem, OBJECT(s), _led_mem_ops, s,
+memory_region_init_io(>led_iomem, obj, _led_mem_ops, s,
   "leds", LED_SIZE);
 sysbus_init_mmio(sbd, >led_iomem);
 
 /* 32 bit registers */
 /* System control */
-memory_region_init_io(>sysctrl_iomem, OBJECT(s), 
_sysctrl_mem_ops, s,
+memory_region_init_io(>sysctrl_iomem, obj, _sysctrl_mem_ops, s,
   "system-control", SYSCTRL_SIZE);
 sysbus_init_mmio(sbd, >sysctrl_iomem);
 
 /* AUX 1 (Misc System Functions) */
-memory_region_init_io(>aux1_iomem, OBJECT(s), _aux1_mem_ops, s,
+memory_region_init_io(>aux1_iomem, obj, _aux1_mem_ops, s,
   "misc-system-functions", MISC_SIZE);
 sysbus_init_mmio(sbd, >aux1_iomem);
 
 /* AUX 2 (Software Powerdown Control) */
-memory_region_init_io(>aux2_iomem, OBJECT(s), _aux2_mem_ops, s,
+memory_region_init_io(>aux2_iomem, obj, _aux2_mem_ops, s,
   "software-powerdown-control", MISC_SIZE);
 sysbus_init_mmio(sbd, >aux2_iomem);
 
 qdev_init_gpio_in(dev, slavio_set_power_fail, 1);
-
-return 0;
 }
 
 static void slavio_misc_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
-SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-k->init = slavio_misc_init1;
 dc->reset = slavio_misc_reset;
 dc->vmsd = _misc;
 }
@@ -492,21 +489,15 @@ static const TypeInfo slavio_misc_info = {
 .name  = TYPE_SLAVIO_MISC,
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(MiscState),
+.instance_init = slavio_misc_init,
 .class_init= slavio_misc_class_init,
 };
 
-static void apc_class_init(ObjectClass *klass, void *data)
-{
-SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
-
-k->init = apc_init1;
-}
-
 static const TypeInfo apc_info = {
 .name  = TYPE_APC,
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(MiscState),
-.class_init= apc_class_init,
+.instance_init = apc_init,
 };
 
 static void slavio_misc_register_types(void)
-- 
2.11.0

[Qemu-devel] [RESEND PATCH 5/9] hw/timer: QOM'ify m48txx_sysbus (pass 1)

[xiaoqiang zhao]

* split the old SysBus init function into an instance_init
  and a Device realize function
* use DeviceClass::realize instead of SysBusDeviceClass::init

Signed-off-by: xiaoqiang zhao 
---
 hw/timer/m48t59.c | 35 ++-
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c
index e46ca88391..39e425e950 100644
--- a/hw/timer/m48t59.c
+++ b/hw/timer/m48t59.c
@@ -765,30 +765,31 @@ static void m48t59_isa_realize(DeviceState *dev, Error 
**errp)
 }
 }
 
-static int m48t59_init1(SysBusDevice *dev)
+static void m48t59_init1(Object *obj)
 {
-M48txxSysBusDeviceClass *u = M48TXX_SYS_BUS_GET_CLASS(dev);
-M48txxSysBusState *d = M48TXX_SYS_BUS(dev);
-Object *o = OBJECT(dev);
+M48txxSysBusDeviceClass *u = M48TXX_SYS_BUS_GET_CLASS(obj);
+M48txxSysBusState *d = M48TXX_SYS_BUS(obj);
+SysBusDevice *dev = SYS_BUS_DEVICE(obj);
 M48t59State *s = >state;
-Error *err = NULL;
 
 s->model = u->info.model;
 s->size = u->info.size;
 sysbus_init_irq(dev, >IRQ);
 
-memory_region_init_io(>iomem, o, _ops, s, "m48t59.nvram",
+memory_region_init_io(>iomem, obj, _ops, s, "m48t59.nvram",
   s->size);
-memory_region_init_io(>io, o, _io_ops, s, "m48t59", 4);
-sysbus_init_mmio(dev, >iomem);
-sysbus_init_mmio(dev, >io);
-m48t59_realize_common(s, );
-if (err != NULL) {
-error_free(err);
-return -1;
-}
+memory_region_init_io(>io, obj, _io_ops, s, "m48t59", 4);
+}
 
-return 0;
+static void m48t59_realize(DeviceState *dev, Error **errp)
+{
+M48txxSysBusState *d = M48TXX_SYS_BUS(dev);
+M48t59State *s = >state;
+SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+
+sysbus_init_mmio(sbd, >iomem);
+sysbus_init_mmio(sbd, >io);
+m48t59_realize_common(s, errp);
 }
 
 static uint32_t m48txx_isa_read(Nvram *obj, uint32_t addr)
@@ -862,10 +863,9 @@ static Property m48t59_sysbus_properties[] = {
 static void m48txx_sysbus_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
-SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 NvramClass *nc = NVRAM_CLASS(klass);
 
-k->init = m48t59_init1;
+dc->realize = m48t59_realize;
 dc->reset = m48t59_reset_sysbus;
 dc->props = m48t59_sysbus_properties;
 nc->read = m48txx_sysbus_read;
@@ -891,6 +891,7 @@ static const TypeInfo m48txx_sysbus_type_info = {
 .name = TYPE_M48TXX_SYS_BUS,
 .parent = TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(M48txxSysBusState),
+.instance_init = m48t59_init1,
 .abstract = true,
 .class_init = m48txx_sysbus_class_init,
 .interfaces = (InterfaceInfo[]) {
-- 
2.11.0

[Qemu-devel] [RESEND PATCH 7/9] hw/timer: QOM'ify slavio_timer

[xiaoqiang zhao]

rename slavio_timer_init1 to slavio_timer_init and assign
it to slavio_timer_info.instance_init, then we drop the
SysBusDeviceClass::init

Signed-off-by: xiaoqiang zhao 
---
 hw/timer/slavio_timer.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/hw/timer/slavio_timer.c b/hw/timer/slavio_timer.c
index bfee1f3027..a8cc9c0148 100644
--- a/hw/timer/slavio_timer.c
+++ b/hw/timer/slavio_timer.c
@@ -373,9 +373,10 @@ static void slavio_timer_reset(DeviceState *d)
 s->cputimer_mode = 0;
 }
 
-static int slavio_timer_init1(SysBusDevice *dev)
+static void slavio_timer_init(Object *obj)
 {
-SLAVIO_TIMERState *s = SLAVIO_TIMER(dev);
+SLAVIO_TIMERState *s = SLAVIO_TIMER(obj);
+SysBusDevice *dev = SYS_BUS_DEVICE(obj);
 QEMUBH *bh;
 unsigned int i;
 TimerContext *tc;
@@ -394,14 +395,12 @@ static int slavio_timer_init1(SysBusDevice *dev)
 
 size = i == 0 ? SYS_TIMER_SIZE : CPU_TIMER_SIZE;
 snprintf(timer_name, sizeof(timer_name), "timer-%i", i);
-memory_region_init_io(>iomem, OBJECT(s), _timer_mem_ops, tc,
+memory_region_init_io(>iomem, obj, _timer_mem_ops, tc,
   timer_name, size);
 sysbus_init_mmio(dev, >iomem);
 
 sysbus_init_irq(dev, >cputimer[i].irq);
 }
-
-return 0;
 }
 
 static Property slavio_timer_properties[] = {
@@ -412,9 +411,7 @@ static Property slavio_timer_properties[] = {
 static void slavio_timer_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
-SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-k->init = slavio_timer_init1;
 dc->reset = slavio_timer_reset;
 dc->vmsd = _slavio_timer;
 dc->props = slavio_timer_properties;
@@ -424,6 +421,7 @@ static const TypeInfo slavio_timer_info = {
 .name  = TYPE_SLAVIO_TIMER,
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(SLAVIO_TIMERState),
+.instance_init = slavio_timer_init,
 .class_init= slavio_timer_class_init,
 };
 
-- 
2.11.0

[Qemu-devel] [RESEND PATCH 9/9] hw/sparc64: QOM'ify sun4u.c

[xiaoqiang zhao]

Drop the old SysBusDeviceClass::init and use instance_init
or DeviceClass::realize instead

Signed-off-by: xiaoqiang zhao 
---
 hw/sparc64/sun4u.c | 20 +---
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index d1a6bca873..5182be2e1f 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -329,16 +329,16 @@ static void prom_init(hwaddr addr, const char *bios_name)
 }
 }
 
-static int prom_init1(SysBusDevice *dev)
+static void prom_init1(Object *obj)
 {
-PROMState *s = OPENPROM(dev);
+PROMState *s = OPENPROM(obj);
+SysBusDevice *dev = SYS_BUS_DEVICE(obj);
 
-memory_region_init_ram(>prom, OBJECT(s), "sun4u.prom", PROM_SIZE_MAX,
+memory_region_init_ram(>prom, obj, "sun4u.prom", PROM_SIZE_MAX,
_fatal);
 vmstate_register_ram_global(>prom);
 memory_region_set_readonly(>prom, true);
 sysbus_init_mmio(dev, >prom);
-return 0;
 }
 
 static Property prom_properties[] = {
@@ -348,9 +348,7 @@ static Property prom_properties[] = {
 static void prom_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
-SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-k->init = prom_init1;
 dc->props = prom_properties;
 }
 
@@ -359,6 +357,7 @@ static const TypeInfo prom_info = {
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(PROMState),
 .class_init= prom_class_init,
+.instance_init = prom_init1,
 };
 
 
@@ -373,15 +372,15 @@ typedef struct RamDevice {
 } RamDevice;
 
 /* System RAM */
-static int ram_init1(SysBusDevice *dev)
+static void ram_realize(DeviceState *dev, Error **errp)
 {
 RamDevice *d = SUN4U_RAM(dev);
+SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
 
 memory_region_init_ram(>ram, OBJECT(d), "sun4u.ram", d->size,
_fatal);
 vmstate_register_ram_global(>ram);
-sysbus_init_mmio(dev, >ram);
-return 0;
+sysbus_init_mmio(sbd, >ram);
 }
 
 static void ram_init(hwaddr addr, ram_addr_t RAM_size)
@@ -409,9 +408,8 @@ static Property ram_properties[] = {
 static void ram_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
-SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-k->init = ram_init1;
+dc->realize = ram_realize;
 dc->props = ram_properties;
 }
 
-- 
2.11.0

[Qemu-devel] [RESEND PATCH 6/9] hw/timer: QOM'ify m48txx_sysbus (pass 2)

[xiaoqiang zhao]

assign DeviceClass::vmsd instead of using vmstate_register function

Signed-off-by: xiaoqiang zhao 
---
 hw/timer/m48t59.c | 26 --
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c
index 39e425e950..6afcce4669 100644
--- a/hw/timer/m48t59.c
+++ b/hw/timer/m48t59.c
@@ -639,6 +639,28 @@ static const VMStateDescription vmstate_m48t59 = {
 }
 };
 
+static const VMStateDescription vmstate_m48t59_isa = {
+.name = "m48t59",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_STRUCT(state, M48txxISAState, 0,
+   vmstate_m48t59, M48t59State),
+VMSTATE_END_OF_LIST()
+}
+};
+
+static const VMStateDescription vmstate_m48t59_sys_bus = {
+.name = "m48t59",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_STRUCT(state, M48txxSysBusState, 0,
+   vmstate_m48t59, M48t59State),
+VMSTATE_END_OF_LIST()
+}
+};
+
 static void m48t59_reset_common(M48t59State *NVRAM)
 {
 NVRAM->addr = 0;
@@ -744,8 +766,6 @@ static void m48t59_realize_common(M48t59State *s, Error 
**errp)
 s->wd_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, _cb, s);
 }
 qemu_get_timedate(>alarm, 0);
-
-vmstate_register(NULL, -1, _m48t59, s);
 }
 
 static void m48t59_isa_realize(DeviceState *dev, Error **errp)
@@ -824,6 +844,7 @@ static void m48txx_isa_class_init(ObjectClass *klass, void 
*data)
 dc->realize = m48t59_isa_realize;
 dc->reset = m48t59_reset_isa;
 dc->props = m48t59_isa_properties;
+dc->vmsd = _m48t59_isa;
 nc->read = m48txx_isa_read;
 nc->write = m48txx_isa_write;
 nc->toggle_lock = m48txx_isa_toggle_lock;
@@ -868,6 +889,7 @@ static void m48txx_sysbus_class_init(ObjectClass *klass, 
void *data)
 dc->realize = m48t59_realize;
 dc->reset = m48t59_reset_sysbus;
 dc->props = m48t59_sysbus_properties;
+dc->vmsd = _m48t59_sys_bus;
 nc->read = m48txx_sysbus_read;
 nc->write = m48txx_sysbus_write;
 nc->toggle_lock = m48txx_sysbus_toggle_lock;
-- 
2.11.0

[Qemu-devel] [RESEND PATCH 0/9] QOM'ify work for sparc

[xiaoqiang zhao]

Rebased on the latest master.

This patch set aims for QOM'ifying code relate with sparc.
It is part of my QOM'ify work of qemu code base.

xiaoqiang zhao (9):
  hw/misc: QOM'ify eccmemctl.c
  hw/dma: QOM'ify sparc32_dma.c
  hw/dma: QOM'ify sun4m_iommu.c
  hw/misc: QOM'ify slavio_misc.c
  hw/timer: QOM'ify m48txx_sysbus (pass 1)
  hw/timer: QOM'ify m48txx_sysbus (pass 2)
  hw/timer: QOM'ify slavio_timer
  hw/sparc: QOM'ify sun4m.c
  hw/sparc64: QOM'ify sun4u.c

 hw/dma/sparc32_dma.c| 25 
 hw/dma/sun4m_iommu.c| 12 --
 hw/misc/eccmemctl.c | 25 
 hw/misc/slavio_misc.c   | 43 ++
 hw/sparc/sun4m.c| 54 +++
 hw/sparc64/sun4u.c  | 20 
 hw/timer/m48t59.c   | 61 ++---
 hw/timer/slavio_timer.c | 12 --
 8 files changed, 128 insertions(+), 124 deletions(-)

-- 
2.11.0

[Qemu-devel] [RESEND PATCH 1/9] hw/misc: QOM'ify eccmemctl.c

[xiaoqiang zhao]

* Split the old SysBus init into an instance_init and a
  DeviceClass::realize function
* Drop the old SysBus init function and use instance_init

Signed-off-by: xiaoqiang zhao 
---
 hw/misc/eccmemctl.c | 25 -
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/hw/misc/eccmemctl.c b/hw/misc/eccmemctl.c
index a0071f3eae..bb7cc52b5e 100644
--- a/hw/misc/eccmemctl.c
+++ b/hw/misc/eccmemctl.c
@@ -295,22 +295,29 @@ static void ecc_reset(DeviceState *d)
 s->regs[ECC_ECR1] = 0;
 }
 
-static int ecc_init1(SysBusDevice *dev)
+static void ecc_init(Object *obj)
 {
-ECCState *s = ECC_MEMCTL(dev);
+ECCState *s = ECC_MEMCTL(obj);
+SysBusDevice *dev = SYS_BUS_DEVICE(obj);
 
 sysbus_init_irq(dev, >irq);
-s->regs[0] = s->version;
-memory_region_init_io(>iomem, OBJECT(dev), _mem_ops, s, "ecc", 
ECC_SIZE);
+
+memory_region_init_io(>iomem, obj, _mem_ops, s, "ecc", ECC_SIZE);
 sysbus_init_mmio(dev, >iomem);
+}
+
+static void ecc_realize(DeviceState *dev, Error **errp)
+{
+ECCState *s = ECC_MEMCTL(dev);
+SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+
+s->regs[0] = s->version;
 
 if (s->version == ECC_MCC) { // SS-600MP only
 memory_region_init_io(>iomem_diag, OBJECT(dev), _diag_mem_ops, 
s,
   "ecc.diag", ECC_DIAG_SIZE);
-sysbus_init_mmio(dev, >iomem_diag);
+sysbus_init_mmio(sbd, >iomem_diag);
 }
-
-return 0;
 }
 
 static Property ecc_properties[] = {
@@ -321,9 +328,8 @@ static Property ecc_properties[] = {
 static void ecc_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
-SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-k->init = ecc_init1;
+dc->realize = ecc_realize;
 dc->reset = ecc_reset;
 dc->vmsd = _ecc;
 dc->props = ecc_properties;
@@ -333,6 +339,7 @@ static const TypeInfo ecc_info = {
 .name  = TYPE_ECC_MEMCTL,
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(ECCState),
+.instance_init = ecc_init,
 .class_init= ecc_class_init,
 };
 
-- 
2.11.0

[Qemu-devel] [RESEND PATCH 8/9] hw/sparc: QOM'ify sun4m.c

[xiaoqiang zhao]

Drop the old SysBusDeviceClass::init and use instance_init
or DeviceClass::realize instead

Signed-off-by: xiaoqiang zhao 
---
 hw/sparc/sun4m.c | 54 +++---
 1 file changed, 19 insertions(+), 35 deletions(-)

diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index f5b6efddf8..352ca0bb4c 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -584,30 +584,23 @@ typedef struct IDRegState {
 MemoryRegion mem;
 } IDRegState;
 
-static int idreg_init1(SysBusDevice *dev)
+static void idreg_init1(Object *obj)
 {
-IDRegState *s = MACIO_ID_REGISTER(dev);
+IDRegState *s = MACIO_ID_REGISTER(obj);
+SysBusDevice *dev = SYS_BUS_DEVICE(obj);
 
-memory_region_init_ram(>mem, OBJECT(s),
+memory_region_init_ram(>mem, obj,
"sun4m.idreg", sizeof(idreg_data), _fatal);
 vmstate_register_ram_global(>mem);
 memory_region_set_readonly(>mem, true);
 sysbus_init_mmio(dev, >mem);
-return 0;
-}
-
-static void idreg_class_init(ObjectClass *klass, void *data)
-{
-SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
-
-k->init = idreg_init1;
 }
 
 static const TypeInfo idreg_info = {
 .name  = TYPE_MACIO_ID_REGISTER,
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(IDRegState),
-.class_init= idreg_class_init,
+.instance_init = idreg_init1,
 };
 
 #define TYPE_TCX_AFX "tcx_afx"
@@ -632,28 +625,21 @@ static void afx_init(hwaddr addr)
 sysbus_mmio_map(s, 0, addr);
 }
 
-static int afx_init1(SysBusDevice *dev)
+static void afx_init1(Object *obj)
 {
-AFXState *s = TCX_AFX(dev);
+AFXState *s = TCX_AFX(obj);
+SysBusDevice *dev = SYS_BUS_DEVICE(obj);
 
-memory_region_init_ram(>mem, OBJECT(s), "sun4m.afx", 4, _fatal);
+memory_region_init_ram(>mem, obj, "sun4m.afx", 4, _fatal);
 vmstate_register_ram_global(>mem);
 sysbus_init_mmio(dev, >mem);
-return 0;
-}
-
-static void afx_class_init(ObjectClass *klass, void *data)
-{
-SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
-
-k->init = afx_init1;
 }
 
 static const TypeInfo afx_info = {
 .name  = TYPE_TCX_AFX,
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(AFXState),
-.class_init= afx_class_init,
+.instance_init = afx_init1,
 };
 
 #define TYPE_OPENPROM "openprom"
@@ -706,16 +692,16 @@ static void prom_init(hwaddr addr, const char *bios_name)
 }
 }
 
-static int prom_init1(SysBusDevice *dev)
+static void prom_init1(Object *obj)
 {
-PROMState *s = OPENPROM(dev);
+PROMState *s = OPENPROM(obj);
+SysBusDevice *dev = SYS_BUS_DEVICE(obj);
 
-memory_region_init_ram(>prom, OBJECT(s), "sun4m.prom", PROM_SIZE_MAX,
+memory_region_init_ram(>prom, obj, "sun4m.prom", PROM_SIZE_MAX,
_fatal);
 vmstate_register_ram_global(>prom);
 memory_region_set_readonly(>prom, true);
 sysbus_init_mmio(dev, >prom);
-return 0;
 }
 
 static Property prom_properties[] = {
@@ -725,9 +711,7 @@ static Property prom_properties[] = {
 static void prom_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
-SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-k->init = prom_init1;
 dc->props = prom_properties;
 }
 
@@ -736,6 +720,7 @@ static const TypeInfo prom_info = {
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(PROMState),
 .class_init= prom_class_init,
+.instance_init = prom_init1,
 };
 
 #define TYPE_SUN4M_MEMORY "memory"
@@ -749,14 +734,14 @@ typedef struct RamDevice {
 } RamDevice;
 
 /* System RAM */
-static int ram_init1(SysBusDevice *dev)
+static void ram_realize(DeviceState *dev, Error **errp)
 {
 RamDevice *d = SUN4M_RAM(dev);
+SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
 
 memory_region_allocate_system_memory(>ram, OBJECT(d), "sun4m.ram",
  d->size);
-sysbus_init_mmio(dev, >ram);
-return 0;
+sysbus_init_mmio(sbd, >ram);
 }
 
 static void ram_init(hwaddr addr, ram_addr_t RAM_size,
@@ -792,9 +777,8 @@ static Property ram_properties[] = {
 static void ram_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
-SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-k->init = ram_init1;
+dc->realize = ram_realize;
 dc->props = ram_properties;
 }
 
-- 
2.11.0

[Qemu-devel] [PATCH v5 08/10] PC: Support dynamic sysbus on pc_i440fx

[ben]

From: Ben Warren 

This allows pc_i440fx-based machines to add new devices such as
VM Generation ID directly to the sysbus.

Signed-off-by: Ben Warren 
---
 hw/i386/pc_piix.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 9f102aa..c8ad99c 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -435,6 +435,7 @@ static void pc_i440fx_machine_options(MachineClass *m)
 m->hot_add_cpu = pc_hot_add_cpu;
 m->default_machine_opts = "firmware=bios-256k.bin";
 m->default_display = "std";
+m->has_dynamic_sysbus = true;
 }
 
 static void pc_i440fx_2_9_machine_options(MachineClass *m)
-- 
2.7.4

[Qemu-devel] [PATCH v5 09/10] tests: Move reusable ACPI macros into a new header file

[ben]

From: Ben Warren 

Also usable by upcoming VM Generation ID tests

Signed-off-by: Ben Warren 
---
 tests/acpi-utils.h   | 75 
 tests/bios-tables-test.c | 72 +-
 2 files changed, 76 insertions(+), 71 deletions(-)
 create mode 100644 tests/acpi-utils.h

diff --git a/tests/acpi-utils.h b/tests/acpi-utils.h
new file mode 100644
index 000..d5e5eff
--- /dev/null
+++ b/tests/acpi-utils.h
@@ -0,0 +1,75 @@
+#ifndef TEST_ACPI_UTILS_H
+#define TEST_ACPI_UTILS_H
+
+/* DSDT and SSDTs format */
+typedef struct {
+AcpiTableHeader header;
+gchar *aml;/* aml bytecode from guest */
+gsize aml_len;
+gchar *aml_file;
+gchar *asl;/* asl code generated from aml */
+gsize asl_len;
+gchar *asl_file;
+bool tmp_files_retain;   /* do not delete the temp asl/aml */
+} QEMU_PACKED AcpiSdtTable;
+
+#define ACPI_READ_FIELD(field, addr)   \
+do {   \
+switch (sizeof(field)) {   \
+case 1:\
+field = readb(addr);   \
+break; \
+case 2:\
+field = readw(addr);   \
+break; \
+case 4:\
+field = readl(addr);   \
+break; \
+case 8:\
+field = readq(addr);   \
+break; \
+default:   \
+g_assert(false);   \
+}  \
+addr += sizeof(field);  \
+} while (0);
+
+#define ACPI_READ_ARRAY_PTR(arr, length, addr)  \
+do {\
+int idx;\
+for (idx = 0; idx < length; ++idx) {\
+ACPI_READ_FIELD(arr[idx], addr);\
+}   \
+} while (0);
+
+#define ACPI_READ_ARRAY(arr, addr)   \
+ACPI_READ_ARRAY_PTR(arr, sizeof(arr) / sizeof(arr[0]), addr)
+
+#define ACPI_READ_TABLE_HEADER(table, addr)  \
+do { \
+ACPI_READ_FIELD((table)->signature, addr);   \
+ACPI_READ_FIELD((table)->length, addr);  \
+ACPI_READ_FIELD((table)->revision, addr);\
+ACPI_READ_FIELD((table)->checksum, addr);\
+ACPI_READ_ARRAY((table)->oem_id, addr);  \
+ACPI_READ_ARRAY((table)->oem_table_id, addr);\
+ACPI_READ_FIELD((table)->oem_revision, addr);\
+ACPI_READ_ARRAY((table)->asl_compiler_id, addr); \
+ACPI_READ_FIELD((table)->asl_compiler_revision, addr);   \
+} while (0);
+
+#define ACPI_ASSERT_CMP(actual, expected) do { \
+uint32_t ACPI_ASSERT_CMP_le = cpu_to_le32(actual); \
+char ACPI_ASSERT_CMP_str[5] = {}; \
+memcpy(ACPI_ASSERT_CMP_str, _ASSERT_CMP_le, 4); \
+g_assert_cmpstr(ACPI_ASSERT_CMP_str, ==, expected); \
+} while (0)
+
+#define ACPI_ASSERT_CMP64(actual, expected) do { \
+uint64_t ACPI_ASSERT_CMP_le = cpu_to_le64(actual); \
+char ACPI_ASSERT_CMP_str[9] = {}; \
+memcpy(ACPI_ASSERT_CMP_str, _ASSERT_CMP_le, 8); \
+g_assert_cmpstr(ACPI_ASSERT_CMP_str, ==, expected); \
+} while (0)
+
+#endif  /* TEST_ACPI_UTILS_H */
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 5404805..c642f7f 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -17,6 +17,7 @@
 #include "hw/acpi/acpi-defs.h"
 #include "hw/smbios/smbios.h"
 #include "qemu/bitmap.h"
+#include "acpi-utils.h"
 #include "boot-sector.h"
 
 #define MACHINE_PC "pc"
@@ -24,18 +25,6 @@
 
 #define ACPI_REBUILD_EXPECTED_AML "TEST_ACPI_REBUILD_AML"
 
-/* DSDT and SSDTs format */
-typedef struct {
-AcpiTableHeader header;
-gchar *aml;/* aml bytecode from guest */
-gsize aml_len;
-gchar *aml_file;
-gchar *asl;/* asl code generated from aml */
-gsize asl_len;
-gchar *asl_file;
-bool tmp_files_retain;   /* do not delete the temp asl/aml */
-} QEMU_PACKED AcpiSdtTable;
-
 typedef struct {
 const char *machine;
 const char *variant;
@@ -53,65 +42,6 @@ typedef struct {
 int required_struct_types_len;
 } test_data;
 
-#define ACPI_READ_FIELD(field, addr)   \
-do {   \
-switch (sizeof(field)) {   \
-case 1:\
-field = readb(addr);   \
-   

[Qemu-devel] [PATCH v5 00/10] Add support for VM Generation ID

[ben]

From: Ben Warren 

This patch set adds support for passing a GUID to Windows guests.  It is a
re-implementation of previous patch sets written by Igor Mammedov et al, but
this time passing the GUID data as a fw_cfg blob.

This patch set has dependencies on new guest functionality, in particular the
support for a new linker-loader command and the ability to write back data
to QEMU over a DMA link.  Work is in flight in both SeaBIOS and OVMF to support 
this.

v4->v5:
- Added significantly more detail to the documentation.
- Replaced the previously-implemented linker-loader command with a new one:
  "write pointer".  This allows writing the guest address of a fw_cfg blob 
back
  to an arbitrary offset in a writeable fw_cfg file visible to QEMU.  This 
will
  require support in SeaBIOS and OVMF (ongoing).
- Fixed endianness issues throughout.
- Several styling cleanups.

v3->v4:
- Rebased to top of tree.
- Re-added document patch that was accidentally dropped from the last 
revision.
- Added VMState functionality so that VGIA is restored properly.
- Added Unit tests
v2->v3:
- Added second writeable fw_cfg for storing the VM Generaiton ID
  address.  This uses a new linker-loader command for instructing the
  guest to write back the allocated address.  A patch for SeaBIOS has been
  submitted 
(https://www.seabios.org/pipermail/seabios/2017-January/011079.html)
  and the resulting binary will need to be pulled into QEMU once accepted.
- Setting VM Generation ID by command line or qmp/hmp now accepts an "auto"
  value, whereby QEMU generates a random GUID.
- Incorporated review comments from v2 mainly around code styling and AML 
syntax
- Changed to use the E05 ACPI event instead of E00
v1->v2:
- Removed "changed" boolean parameter as it is unneeded
- Added ACPI Notify logic
- Style changes to pass checkpatch.pl
- Added support for dynamic sysbus to pc_piix boards


Ben Warren (8):
  ACPI: Add a function for building named qword entries
  linker-loader: Add new 'write pointer' command
  docs: VM Generation ID device description
  ACPI: Add vmgenid storage entries to the build tables
  ACPI: Add Virtual Machine Generation ID support
  PC: Support dynamic sysbus on pc_i440fx
  tests: Move reusable ACPI macros into a new header file
  tests: Add unit tests for the VM Generation ID feature

Igor Mammedov (2):
  qmp/hmp: add query-vm-generation-id and 'info vm-generation-id'
commands
  qmp/hmp: add set-vm-generation-id commands

 default-configs/i386-softmmu.mak |   1 +
 default-configs/x86_64-softmmu.mak   |   1 +
 docs/specs/vmgenid.txt   | 239 +++
 hmp-commands-info.hx |  13 ++
 hmp-commands.hx  |  13 ++
 hmp.c|  21 +++
 hmp.h|   2 +
 hw/acpi/Makefile.objs|   1 +
 hw/acpi/aml-build.c  |  39 +-
 hw/acpi/bios-linker-loader.c |  35 +++--
 hw/acpi/nvdimm.c |   2 +-
 hw/acpi/vmgenid.c| 238 ++
 hw/arm/virt-acpi-build.c |   4 +-
 hw/i386/acpi-build.c |  18 ++-
 hw/i386/pc_piix.c|   1 +
 include/hw/acpi/acpi_dev_interface.h |   1 +
 include/hw/acpi/aml-build.h  |   5 +
 include/hw/acpi/bios-linker-loader.h |   3 +-
 include/hw/acpi/vmgenid.h|  37 ++
 qapi-schema.json |  31 +
 stubs/Makefile.objs  |   1 +
 stubs/vmgenid.c  |  14 ++
 tests/Makefile.include   |   2 +
 tests/acpi-utils.h   |  75 +++
 tests/bios-tables-test.c |  72 +--
 tests/vmgenid-test.c | 184 +++
 26 files changed, 962 insertions(+), 91 deletions(-)
 create mode 100644 docs/specs/vmgenid.txt
 create mode 100644 hw/acpi/vmgenid.c
 create mode 100644 include/hw/acpi/vmgenid.h
 create mode 100644 stubs/vmgenid.c
 create mode 100644 tests/acpi-utils.h
 create mode 100644 tests/vmgenid-test.c

-- 
2.7.4

[Qemu-devel] [PATCH v5 07/10] qmp/hmp: add set-vm-generation-id commands

[ben]

From: Igor Mammedov 

Add set-vm-generation-id command to set Virtual Machine
Generation ID counter.

QMP command example:
{ "execute": "set-vm-generation-id",
  "arguments": {
  "guid": "324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
  }
}

HMP command example:
set-vm-generation-id guid=324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87

Signed-off-by: Igor Mammedov 
Reviewed-by: Eric Blake 
Signed-off-by: Ben Warren 
---
 hmp-commands.hx   | 13 +
 hmp.c | 12 
 hmp.h |  1 +
 hw/acpi/vmgenid.c | 12 
 qapi-schema.json  | 11 +++
 stubs/vmgenid.c   |  6 ++
 6 files changed, 55 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8819281..56744aa 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1775,5 +1775,18 @@ ETEXI
 },
 
 STEXI
+@item set-vm-generation-id @var{uuid}
+Set Virtual Machine Generation ID counter to @var{guid}
+ETEXI
+
+{
+.name   = "set-vm-generation-id",
+.args_type  = "guid:s",
+.params = "guid",
+.help   = "Set Virtual Machine Generation ID counter",
+.cmd = hmp_set_vm_generation_id,
+},
+
+STEXI
 @end table
 ETEXI
diff --git a/hmp.c b/hmp.c
index 535613d..39c8965 100644
--- a/hmp.c
+++ b/hmp.c
@@ -2574,3 +2574,15 @@ void hmp_info_vm_generation_id(Monitor *mon, const QDict 
*qdict)
 }
 qapi_free_GuidInfo(info);
 }
+
+void hmp_set_vm_generation_id(Monitor *mon, const QDict *qdict)
+{
+Error *errp = NULL;
+const char *guid = qdict_get_str(qdict, "guid");
+
+qmp_set_vm_generation_id(guid, );
+if (errp) {
+hmp_handle_error(mon, );
+return;
+}
+}
diff --git a/hmp.h b/hmp.h
index 799fd37..e0ac1e8 100644
--- a/hmp.h
+++ b/hmp.h
@@ -138,5 +138,6 @@ void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict 
*qdict);
 void hmp_info_dump(Monitor *mon, const QDict *qdict);
 void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict);
 void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict);
+void hmp_set_vm_generation_id(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c
index e148051..af8b35c 100644
--- a/hw/acpi/vmgenid.c
+++ b/hw/acpi/vmgenid.c
@@ -224,3 +224,15 @@ GuidInfo *qmp_query_vm_generation_id(Error **errp)
 info->guid = qemu_uuid_unparse_strdup();
 return info;
 }
+
+void qmp_set_vm_generation_id(const char *guid, Error **errp)
+{
+Object *obj = find_vmgenid_dev(errp);
+
+if (!obj) {
+return;
+}
+
+object_property_set_str(obj, guid, VMGENID_GUID, errp);
+return;
+}
diff --git a/qapi-schema.json b/qapi-schema.json
index 384a7f3..ed2657d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -6051,3 +6051,14 @@
 # Since 2.9
 ##
 { 'command': 'query-vm-generation-id', 'returns': 'GuidInfo' }
+
+##
+# @set-vm-generation-id:
+#
+# Set Virtual Machine Generation ID
+#
+# @guid: new GUID to set as Virtual Machine Generation ID
+#
+# Since 2.9
+##
+{ 'command': 'set-vm-generation-id', 'data': {'guid': 'str'} }
diff --git a/stubs/vmgenid.c b/stubs/vmgenid.c
index 8c448ac..d25d41b 100644
--- a/stubs/vmgenid.c
+++ b/stubs/vmgenid.c
@@ -6,3 +6,9 @@ GuidInfo *qmp_query_vm_generation_id(Error **errp)
 error_setg(errp, "this command is not currently supported");
 return NULL;
 }
+
+void qmp_set_vm_generation_id(const char *guid, Error **errp)
+{
+error_setg(errp, "this command is not currently supported");
+return;
+}
-- 
2.7.4

[Qemu-devel] [PATCH v5 06/10] qmp/hmp: add query-vm-generation-id and 'info vm-generation-id' commands

[ben]

From: Igor Mammedov 

Add commands to query Virtual Machine Generation ID counter.

QMP command example:
{ "execute": "query-vm-generation-id" }

HMP command example:
info vm-generation-id

Signed-off-by: Igor Mammedov 
Reviewed-by: Eric Blake 
Signed-off-by: Ben Warren 
---
 hmp-commands-info.hx | 13 +
 hmp.c|  9 +
 hmp.h|  1 +
 hw/acpi/vmgenid.c| 20 
 qapi-schema.json | 20 
 stubs/Makefile.objs  |  1 +
 stubs/vmgenid.c  |  8 
 7 files changed, 72 insertions(+)
 create mode 100644 stubs/vmgenid.c

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index b0f35e6..f3df793 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -802,6 +802,19 @@ Show information about hotpluggable CPUs
 ETEXI
 
 STEXI
+@item info vm-generation-id
+Show Virtual Machine Generation ID
+ETEXI
+
+{
+.name   = "vm-generation-id",
+.args_type  = "",
+.params = "",
+.help   = "Show Virtual Machine Generation ID",
+.cmd = hmp_info_vm_generation_id,
+},
+
+STEXI
 @end table
 ETEXI
 
diff --git a/hmp.c b/hmp.c
index 2bc4f06..535613d 100644
--- a/hmp.c
+++ b/hmp.c
@@ -2565,3 +2565,12 @@ void hmp_hotpluggable_cpus(Monitor *mon, const QDict 
*qdict)
 
 qapi_free_HotpluggableCPUList(saved);
 }
+
+void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict)
+{
+GuidInfo *info = qmp_query_vm_generation_id(NULL);
+if (info) {
+monitor_printf(mon, "%s\n", info->guid);
+}
+qapi_free_GuidInfo(info);
+}
diff --git a/hmp.h b/hmp.h
index 05daf7c..799fd37 100644
--- a/hmp.h
+++ b/hmp.h
@@ -137,5 +137,6 @@ void hmp_rocker_of_dpa_flows(Monitor *mon, const QDict 
*qdict);
 void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict *qdict);
 void hmp_info_dump(Monitor *mon, const QDict *qdict);
 void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict);
+void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c
index 6c9ecfd..e148051 100644
--- a/hw/acpi/vmgenid.c
+++ b/hw/acpi/vmgenid.c
@@ -204,3 +204,23 @@ static void vmgenid_register_types(void)
 }
 
 type_init(vmgenid_register_types)
+
+GuidInfo *qmp_query_vm_generation_id(Error **errp)
+{
+GuidInfo *info;
+VmGenIdState *vdev;
+QemuUUID guid;
+Object *obj = find_vmgenid_dev(errp);
+
+if (!obj) {
+return NULL;
+}
+vdev = VMGENID(obj);
+/* Convert GUID back to big-endian before displaying */
+memcpy(, >guid, sizeof(guid));
+qemu_uuid_bswap();
+
+info = g_malloc0(sizeof(*info));
+info->guid = qemu_uuid_unparse_strdup();
+return info;
+}
diff --git a/qapi-schema.json b/qapi-schema.json
index cbdffdd..384a7f3 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -6031,3 +6031,23 @@
 #
 ##
 { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
+
+##
+# @GuidInfo:
+#
+# GUID information.
+#
+# @guid: the globally unique identifier
+#
+# Since: 2.9
+##
+{ 'struct': 'GuidInfo', 'data': {'guid': 'str'} }
+
+##
+# @query-vm-generation-id:
+#
+# Show Virtual Machine Generation ID
+#
+# Since 2.9
+##
+{ 'command': 'query-vm-generation-id', 'returns': 'GuidInfo' }
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index a187295..0bffca6 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -35,3 +35,4 @@ stub-obj-y += qmp_pc_dimm_device_list.o
 stub-obj-y += target-monitor-defs.o
 stub-obj-y += target-get-monitor-def.o
 stub-obj-y += pc_madt_cpu_entry.o
+stub-obj-y += vmgenid.o
diff --git a/stubs/vmgenid.c b/stubs/vmgenid.c
new file mode 100644
index 000..8c448ac
--- /dev/null
+++ b/stubs/vmgenid.c
@@ -0,0 +1,8 @@
+#include "qemu/osdep.h"
+#include "qmp-commands.h"
+
+GuidInfo *qmp_query_vm_generation_id(Error **errp)
+{
+error_setg(errp, "this command is not currently supported");
+return NULL;
+}
-- 
2.7.4

[Qemu-devel] [PATCH v5 04/10] ACPI: Add vmgenid storage entries to the build tables

[ben]

From: Ben Warren 

This allows them to be centrally initialized and destroyed

Signed-off-by: Ben Warren 
---
 hw/acpi/aml-build.c | 2 ++
 include/hw/acpi/aml-build.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 03b6c6c..0f39eaf 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1594,6 +1594,7 @@ void acpi_build_tables_init(AcpiBuildTables *tables)
 tables->rsdp = g_array_new(false, true /* clear */, 1);
 tables->table_data = g_array_new(false, true /* clear */, 1);
 tables->tcpalog = g_array_new(false, true /* clear */, 1);
+tables->vmgenid = g_array_new(false, true /* clear */, 1);
 tables->linker = bios_linker_loader_init();
 }
 
@@ -1603,6 +1604,7 @@ void acpi_build_tables_cleanup(AcpiBuildTables *tables, 
bool mfre)
 g_array_free(tables->rsdp, true);
 g_array_free(tables->table_data, true);
 g_array_free(tables->tcpalog, mfre);
+g_array_free(tables->vmgenid, mfre);
 }
 
 /* Build rsdt table */
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index dbf63cf..6f4e239 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -210,6 +210,7 @@ struct AcpiBuildTables {
 GArray *table_data;
 GArray *rsdp;
 GArray *tcpalog;
+GArray *vmgenid;
 BIOSLinker *linker;
 } AcpiBuildTables;
 
-- 
2.7.4

[Qemu-devel] [PATCH v5 10/10] tests: Add unit tests for the VM Generation ID feature

[ben]

From: Ben Warren 

The following tests are implemented:
* test that a GUID passed in by command line is propagated to the guest.
* test that changing the GUID at runtime via the monitor is reflected in
  the guest.
* test that the "auto" argument to the GUID generates a different, and
  correct GUID as seen by the guest.

  This patch is loosely based on a previous patch from:
  Gal Hammer   and Igor Mammedov 

Signed-off-by: Ben Warren 
---
 tests/Makefile.include |   2 +
 tests/vmgenid-test.c   | 184 +
 2 files changed, 186 insertions(+)
 create mode 100644 tests/vmgenid-test.c

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 634394a..ca4b3f7 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -241,6 +241,7 @@ check-qtest-i386-y += tests/usb-hcd-xhci-test$(EXESUF)
 gcov-files-i386-y += hw/usb/hcd-xhci.c
 check-qtest-i386-y += tests/pc-cpu-test$(EXESUF)
 check-qtest-i386-y += tests/q35-test$(EXESUF)
+check-qtest-i386-y += tests/vmgenid-test$(EXESUF)
 gcov-files-i386-y += hw/pci-host/q35.c
 check-qtest-i386-$(CONFIG_VHOST_NET_TEST_i386) += 
tests/vhost-user-test$(EXESUF)
 ifeq ($(CONFIG_VHOST_NET_TEST_i386),)
@@ -726,6 +727,7 @@ tests/ivshmem-test$(EXESUF): tests/ivshmem-test.o 
contrib/ivshmem-server/ivshmem
 tests/vhost-user-bridge$(EXESUF): tests/vhost-user-bridge.o 
contrib/libvhost-user/libvhost-user.o $(test-util-obj-y)
 tests/test-uuid$(EXESUF): tests/test-uuid.o $(test-util-obj-y)
 tests/test-arm-mptimer$(EXESUF): tests/test-arm-mptimer.o
+tests/vmgenid-test$(EXESUF): tests/vmgenid-test.o
 
 tests/migration/stress$(EXESUF): tests/migration/stress.o
$(call quiet-command, $(LINKPROG) -static -O3 $(PTHREAD_LIB) -o $@ $< 
,"LINK","$(TARGET_DIR)$@")
diff --git a/tests/vmgenid-test.c b/tests/vmgenid-test.c
new file mode 100644
index 000..0f5ba07
--- /dev/null
+++ b/tests/vmgenid-test.c
@@ -0,0 +1,184 @@
+/*
+ * QTest testcase for VM Generation ID
+ *
+ * Copyright (c) 2016 Red Hat, Inc.
+ * Copyright (c) 2017 Skyport Systems
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include 
+#include 
+#include 
+#include "qemu/osdep.h"
+#include "qemu/bitmap.h"
+#include "qemu/uuid.h"
+#include "hw/acpi/acpi-defs.h"
+#include "acpi-utils.h"
+#include "libqtest.h"
+
+#define VGID_GUID "324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
+
+static uint64_t vgia;
+
+typedef struct {
+AcpiTableHeader header;
+gchar name_op;
+gchar vgia[4];
+gchar val_op;
+uint64_t vgia_val;
+} QEMU_PACKED VgidTable;
+
+static uint64_t find_vgia(void)
+{
+uint32_t off;
+AcpiRsdpDescriptor rsdp_table;
+uint32_t rsdt;
+AcpiRsdtDescriptorRev1 rsdt_table;
+int tables_nr;
+uint32_t *tables;
+AcpiTableHeader ssdt_table;
+VgidTable vgid_table;
+int i;
+
+/* First, find the RSDP */
+for (off = 0xf; off < 0x10; off += 0x10) {
+uint8_t sig[] = "RSD PTR ";
+
+for (i = 0; i < sizeof sig - 1; ++i) {
+sig[i] = readb(off + i);
+}
+
+if (!memcmp(sig, "RSD PTR ", sizeof sig)) {
+break;
+}
+}
+g_assert_cmphex(off, <, 0x10);
+
+/* Parse the RSDP header so we can find the RSDT */
+ACPI_READ_FIELD(rsdp_table.signature, off);
+ACPI_ASSERT_CMP64(rsdp_table.signature, "RSD PTR ");
+
+ACPI_READ_FIELD(rsdp_table.checksum, off);
+ACPI_READ_ARRAY(rsdp_table.oem_id, off);
+ACPI_READ_FIELD(rsdp_table.revision, off);
+ACPI_READ_FIELD(rsdp_table.rsdt_physical_address, off);
+
+rsdt = rsdp_table.rsdt_physical_address;
+/* read the header */
+ACPI_READ_TABLE_HEADER(_table, rsdt);
+ACPI_ASSERT_CMP(rsdt_table.signature, "RSDT");
+
+/* compute the table entries in rsdt */
+tables_nr = (rsdt_table.length - sizeof(AcpiRsdtDescriptorRev1)) /
+sizeof(uint32_t);
+g_assert_cmpint(tables_nr, >, 0);
+
+/* get the addresses of the tables pointed by rsdt */
+tables = g_new0(uint32_t, tables_nr);
+ACPI_READ_ARRAY_PTR(tables, tables_nr, rsdt);
+
+for (i = 0; i < tables_nr; i++) {
+ACPI_READ_TABLE_HEADER(_table, tables[i]);
+if (!strncmp((char *)ssdt_table.oem_table_id, "VMGENID", 7)) {
+/* the first entry in the table should be VGIA
+ * That's all we need */
+ACPI_READ_FIELD(vgid_table.name_op, tables[i]);
+g_assert(vgid_table.name_op == 0x08);  /* name */
+ACPI_READ_ARRAY(vgid_table.vgia, tables[i]);
+g_assert(memcmp(vgid_table.vgia, "VGIA", 4) == 0);
+ACPI_READ_FIELD(vgid_table.val_op, tables[i]);
+g_assert(vgid_table.val_op == 0x0E);  /* qword */
+ACPI_READ_FIELD(vgid_table.vgia_val, tables[i]);
+return vgid_table.vgia_val;
+}
+}
+

[Qemu-devel] [PATCH v5 05/10] ACPI: Add Virtual Machine Generation ID support

[ben]

From: Ben Warren 

This implements the VM Generation ID feature by passing a 128-bit
GUID to the guest via a fw_cfg blob.
Any time the GUID changes, an ACPI notify event is sent to the guest

The user interface is a simple device with one parameter:
 - guid (string, must be "auto" or in UUID format
   ----)

Signed-off-by: Ben Warren 
---
 default-configs/i386-softmmu.mak |   1 +
 default-configs/x86_64-softmmu.mak   |   1 +
 hw/acpi/Makefile.objs|   1 +
 hw/acpi/vmgenid.c| 206 +++
 hw/i386/acpi-build.c |  10 ++
 include/hw/acpi/acpi_dev_interface.h |   1 +
 include/hw/acpi/vmgenid.h|  37 +++
 7 files changed, 257 insertions(+)
 create mode 100644 hw/acpi/vmgenid.c
 create mode 100644 include/hw/acpi/vmgenid.h

diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 384cefb..1a43542 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -58,3 +58,4 @@ CONFIG_I82801B11=y
 CONFIG_SMBIOS=y
 CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM)
 CONFIG_PXB=y
+CONFIG_ACPI_VMGENID=y
diff --git a/default-configs/x86_64-softmmu.mak 
b/default-configs/x86_64-softmmu.mak
index 491a191..aee8b08 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -58,3 +58,4 @@ CONFIG_I82801B11=y
 CONFIG_SMBIOS=y
 CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM)
 CONFIG_PXB=y
+CONFIG_ACPI_VMGENID=y
diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
index 6acf798..11c35bc 100644
--- a/hw/acpi/Makefile.objs
+++ b/hw/acpi/Makefile.objs
@@ -5,6 +5,7 @@ common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
 common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
 common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu.o
 common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
+common-obj-$(CONFIG_ACPI_VMGENID) += vmgenid.o
 common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o
 
 common-obj-y += acpi_interface.o
diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c
new file mode 100644
index 000..6c9ecfd
--- /dev/null
+++ b/hw/acpi/vmgenid.c
@@ -0,0 +1,206 @@
+/*
+ *  Virtual Machine Generation ID Device
+ *
+ *  Copyright (C) 2017 Skyport Systems.
+ *
+ *  Author: Ben Warren 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qmp-commands.h"
+#include "hw/acpi/acpi.h"
+#include "hw/acpi/aml-build.h"
+#include "hw/acpi/vmgenid.h"
+#include "hw/nvram/fw_cfg.h"
+#include "sysemu/sysemu.h"
+
+void vmgenid_build_acpi(GArray *table_data, GArray *guid, BIOSLinker *linker)
+{
+Object *obj;
+VmGenIdState *s;
+Aml *ssdt, *dev, *scope, *method, *addr, *if_ctx;
+uint32_t vgia_offset;
+
+obj = find_vmgenid_dev(NULL);
+assert(obj);
+s = VMGENID(obj);
+
+/* Fill in the GUID values */
+if (guid->len != VMGENID_FW_CFG_SIZE) {
+g_array_set_size(guid, VMGENID_FW_CFG_SIZE);
+}
+g_array_insert_vals(guid, VMGENID_GUID_OFFSET, s->guid.data, 16);
+
+/* Put this in a separate SSDT table */
+ssdt = init_aml_allocator();
+
+/* Reserve space for header */
+acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
+
+/* Storage for the GUID address */
+vgia_offset = table_data->len +
+build_append_named_dword(ssdt->buf, "VGIA");
+scope = aml_scope("\\_SB");
+dev = aml_device("VGEN");
+aml_append(dev, aml_name_decl("_HID", aml_string("QEMUVGID")));
+aml_append(dev, aml_name_decl("_CID", aml_string("VM_Gen_Counter")));
+aml_append(dev, aml_name_decl("_DDN", aml_string("VM_Gen_Counter")));
+
+/* Simple status method to check that address is linked and non-zero */
+method = aml_method("_STA", 0, AML_NOTSERIALIZED);
+addr = aml_local(0);
+aml_append(method, aml_store(aml_int(0xf), addr));
+if_ctx = aml_if(aml_equal(aml_name("VGIA"), aml_int(0)));
+aml_append(if_ctx, aml_store(aml_int(0), addr));
+aml_append(method, if_ctx);
+aml_append(method, aml_return(addr));
+aml_append(dev, method);
+
+/* the ADDR method returns two 32-bit words representing the lower and
+ * upper halves * of the physical address of the fw_cfg blob
+ * (holding the GUID) */
+method = aml_method("ADDR", 0, AML_NOTSERIALIZED);
+
+addr = aml_local(0);
+aml_append(method, aml_store(aml_package(2), addr));
+
+aml_append(method, aml_store(aml_add(aml_name("VGIA"),
+ aml_int(VMGENID_GUID_OFFSET), NULL),
+ aml_index(addr, aml_int(0;
+aml_append(method, aml_store(aml_int(0), aml_index(addr, aml_int(1;
+aml_append(method, aml_return(addr));
+
+aml_append(dev, method);
+aml_append(scope, dev);
+aml_append(ssdt, scope);
+
+/* attach an ACPI 

[Qemu-devel] [PATCH v5 02/10] linker-loader: Add new 'write pointer' command

[ben]

From: Ben Warren 

This adds to the existing 'add pointer' functionality in that it
instructs the guest (BIOS or UEFI) to not patch memory but to instead
write the changes back to QEMU via a writeable fw_cfg file.

Signed-off-by: Ben Warren 
---
 hw/acpi/aml-build.c  |  2 +-
 hw/acpi/bios-linker-loader.c | 35 ---
 hw/acpi/nvdimm.c |  2 +-
 hw/arm/virt-acpi-build.c |  4 ++--
 hw/i386/acpi-build.c |  8 
 include/hw/acpi/bios-linker-loader.h |  3 ++-
 6 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 9fc54c9..03b6c6c 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1626,7 +1626,7 @@ build_rsdt(GArray *table_data, BIOSLinker *linker, GArray 
*table_offsets,
 /* rsdt->table_offset_entry to be filled by Guest linker */
 bios_linker_loader_add_pointer(linker,
 ACPI_BUILD_TABLE_FILE, rsdt_entry_offset, rsdt_entry_size,
-ACPI_BUILD_TABLE_FILE, ref_tbl_offset);
+ACPI_BUILD_TABLE_FILE, ref_tbl_offset, false);
 }
 build_header(linker, table_data,
  (void *)rsdt, "RSDT", rsdt_len, 1, oem_id, oem_table_id);
diff --git a/hw/acpi/bios-linker-loader.c b/hw/acpi/bios-linker-loader.c
index d963ebe..e46bc29 100644
--- a/hw/acpi/bios-linker-loader.c
+++ b/hw/acpi/bios-linker-loader.c
@@ -52,10 +52,13 @@ struct BiosLinkerLoaderEntry {
 } alloc;
 
 /*
- * COMMAND_ADD_POINTER - patch the table (originating from
- * @dest_file) at @pointer.offset, by adding a pointer to the table
+ * COMMAND_ADD_POINTER &
+ * COMMAND_WRITE_POINTER - patch guest memory (originating from
+ * @dest_file) at @pointer.offset, by adding a pointer to the memory
  * originating from @src_file. 1,2,4 or 8 byte unsigned
  * addition is used depending on @pointer.size.
+ * Instead of patching memory, COMMAND_WRITE_POINTER writes the changes
+ * to @dest_file in QEMU via fw_cfg DMA.
  */
 struct {
 char dest_file[BIOS_LINKER_LOADER_FILESZ];
@@ -85,9 +88,10 @@ struct BiosLinkerLoaderEntry {
 typedef struct BiosLinkerLoaderEntry BiosLinkerLoaderEntry;
 
 enum {
-BIOS_LINKER_LOADER_COMMAND_ALLOCATE = 0x1,
-BIOS_LINKER_LOADER_COMMAND_ADD_POINTER  = 0x2,
-BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM = 0x3,
+BIOS_LINKER_LOADER_COMMAND_ALLOCATE  = 0x1,
+BIOS_LINKER_LOADER_COMMAND_ADD_POINTER   = 0x2,
+BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM  = 0x3,
+BIOS_LINKER_LOADER_COMMAND_WRITE_POINTER = 0x4,
 };
 
 enum {
@@ -242,13 +246,15 @@ void bios_linker_loader_add_checksum(BIOSLinker *linker, 
const char *file_name,
  * @src_offset: location within source file blob to which
  *  @dest_file+@dst_patched_offset will point to after
  *  firmware's executed ADD_POINTER command
+ * @write_back: guest should write change contents back to QEMU after patching
  */
 void bios_linker_loader_add_pointer(BIOSLinker *linker,
 const char *dest_file,
 uint32_t dst_patched_offset,
 uint8_t dst_patched_size,
 const char *src_file,
-uint32_t src_offset)
+uint32_t src_offset,
+bool write_back)
 {
 uint64_t le_src_offset;
 BiosLinkerLoaderEntry entry;
@@ -257,8 +263,11 @@ void bios_linker_loader_add_pointer(BIOSLinker *linker,
 const BiosLinkerFileEntry *source_file =
 bios_linker_find_file(linker, src_file);
 
-assert(dst_patched_offset < dst_file->blob->len);
-assert(dst_patched_offset + dst_patched_size <= dst_file->blob->len);
+/* dst_file need not exist if writing back */
+if (!write_back) {
+assert(dst_patched_offset < dst_file->blob->len);
+assert(dst_patched_offset + dst_patched_size <= dst_file->blob->len);
+}
 assert(src_offset < source_file->blob->len);
 
 memset(, 0, sizeof entry);
@@ -266,15 +275,19 @@ void bios_linker_loader_add_pointer(BIOSLinker *linker,
 sizeof entry.pointer.dest_file - 1);
 strncpy(entry.pointer.src_file, src_file,
 sizeof entry.pointer.src_file - 1);
-entry.command = cpu_to_le32(BIOS_LINKER_LOADER_COMMAND_ADD_POINTER);
+entry.command = cpu_to_le32(write_back ?
+BIOS_LINKER_LOADER_COMMAND_WRITE_POINTER :
+BIOS_LINKER_LOADER_COMMAND_ADD_POINTER);
 entry.pointer.offset = cpu_to_le32(dst_patched_offset);
 entry.pointer.size = dst_patched_size;
 assert(dst_patched_size == 1 || dst_patched_size == 2 ||
dst_patched_size == 4 || 

[Qemu-devel] [PATCH v5 03/10] docs: VM Generation ID device description

[ben]

From: Ben Warren 

This patch is based off an earlier version by
Gal Hammer (gham...@redhat.com)

Requirements section, ASCII diagrams and overall help
provided by Laszlo Ersek (ler...@redhat.com)

Signed-off-by: Gal Hammer 
Signed-off-by: Ben Warren 
---
 docs/specs/vmgenid.txt | 239 +
 1 file changed, 239 insertions(+)
 create mode 100644 docs/specs/vmgenid.txt

diff --git a/docs/specs/vmgenid.txt b/docs/specs/vmgenid.txt
new file mode 100644
index 000..d36f8bd
--- /dev/null
+++ b/docs/specs/vmgenid.txt
@@ -0,0 +1,239 @@
+VIRTUAL MACHINE GENERATION ID
+=
+
+Copyright (C) 2016 Red Hat, Inc.
+Copyright (C) 2017 Skyport Systems, Inc.
+
+This work is licensed under the terms of the GNU GPL, version 2 or later.
+See the COPYING file in the top-level directory.
+
+===
+
+The VM generation ID (vmgenid) device is an emulated device which
+exposes a 128-bit, cryptographically random, integer value identifier,
+referred to as a Globally Unique Identifier, or GUID.
+
+This allows management applications (e.g. libvirt) to notify the guest
+operating system when the virtual machine is executed with a different
+configuration (e.g. snapshot execution or creation from a template).  The
+guest operating system notices the change, and is then able to react as
+appropriate by marking its copies of distributed databases as dirty,
+re-initializing its random number generator etc.
+
+
+Requirements
+
+
+These requirements are extracted from the "How to implement virtual machine
+generation ID support in a virtualization platform" section of the
+specification, dated August 1, 2012.
+
+
+The document may be found on the web at:
+  http://go.microsoft.com/fwlink/?LinkId=260709
+
+R1a. The generation ID shall live in an 8-byte aligned buffer.
+
+R1b. The buffer holding the generation ID shall be in guest RAM, ROM, or device
+ MMIO range.
+
+R1c. The buffer holding the generation ID shall be kept separate from areas
+ used by the operating system.
+
+R1d. The buffer shall not be covered by an AddressRangeMemory or
+ AddressRangeACPI entry in the E820 or UEFI memory map.
+
+R1e. The generation ID shall not live in a page frame that could be mapped with
+ caching disabled. (In other words, regardless of whether the generation ID
+ lives in RAM, ROM or MMIO, it shall only be mapped as cacheable.)
+
+R2 to R5. [These AML requirements are isolated well enough in the Microsoft
+  specification for us to simply refer to them here.]
+
+R6. The hypervisor shall expose a _HID (hardware identifier) object in the
+VMGenId device's scope that is unique to the hypervisor vendor.
+
+
+QEMU Implementation
+---
+
+The above-mentioned specification does not dictate which ACPI descriptor table
+will contain the VM Generation ID device.  Other implementations (Hyper-V and
+Xen) put it in the main descriptor table (Differentiated System Description
+Table or DSDT).  For ease of debugging and implementation, we have decided to
+put it in its own Secondary System Description Table, or SSDT.
+
+The following is a dump of the contents from a running system:
+
+# iasl -p ./SSDT -d /sys/firmware/acpi/tables/SSDT
+
+Intel ACPI Component Architecture
+ASL+ Optimizing Compiler version 20150717-64
+Copyright (c) 2000 - 2015 Intel Corporation
+
+Reading ACPI table from file /sys/firmware/acpi/tables/SSDT - Length
+0198 (0xC6)
+ACPI: SSDT 0x C6 (v01 BOCHS  VMGENID  0001 BXPC
+0001)
+Acpi table [SSDT] successfully installed and loaded
+Pass 1 parse of [SSDT]
+Pass 2 parse of [SSDT]
+Parsing Deferred Opcodes (Methods/Buffers/Packages/Regions)
+
+Parsing completed
+Disassembly completed
+ASL Output:./SSDT.dsl - 1631 bytes
+# cat SSDT.dsl
+/*
+ * Intel ACPI Component Architecture
+ * AML/ASL+ Disassembler version 20150717-64
+ * Copyright (c) 2000 - 2015 Intel Corporation
+ *
+ * Disassembling to symbolic ASL+ operators
+ *
+ * Disassembly of /sys/firmware/acpi/tables/SSDT, Sun Feb  5 00:19:37 2017
+ *
+ * Original Table Header:
+ * Signature"SSDT"
+ * Length   0x00CA (202)
+ * Revision 0x01
+ * Checksum 0x4B
+ * OEM ID   "BOCHS "
+ * OEM Table ID "VMGENID"
+ * OEM Revision 0x0001 (1)
+ * Compiler ID  "BXPC"
+ * Compiler Version 0x0001 (1)
+ */
+DefinitionBlock ("/sys/firmware/acpi/tables/SSDT.aml", "SSDT", 1, "BOCHS ",
+"VMGENID", 0x0001)
+{
+Name (VGIA, 0x07FFF000)
+Scope (\_SB)
+{
+Device (VGEN)
+{
+Name (_HID, "QEMUVGID")  // _HID: Hardware ID
+Name (_CID, "VM_Gen_Counter")  // _CID: Compatible ID
+Name (_DDN, "VM_Gen_Counter")  // _DDN: DOS Device Name
+Method (_STA, 0, NotSerialized)  // _STA: Status
+{
+

[Qemu-devel] [PATCH v5 01/10] ACPI: Add a function for building named qword entries

[ben]

From: Ben Warren 

This is initially used to patch a 64-bit address into
the VM Generation ID SSDT

Signed-off-by: Ben Warren 
---
 hw/acpi/aml-build.c | 35 +++
 include/hw/acpi/aml-build.h |  4 
 2 files changed, 39 insertions(+)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index b2a1e40..9fc54c9 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -264,6 +264,9 @@ static void build_append_int(GArray *table, uint64_t value)
  * Warning: runtime patching is best avoided. Only use this as
  * a replacement for DataTableRegion (for guests that don't
  * support it).
+ *
+ * ACPI 5.0: 19.2.5
+ * 32-bit integers are supported beginning with ACPI 1.0
  */
 int
 build_append_named_dword(GArray *array, const char *name_format, ...)
@@ -285,6 +288,38 @@ build_append_named_dword(GArray *array, const char 
*name_format, ...)
 return offset;
 }
 
+/*
+ * Build NAME(, 0x) where 0x
+ * is encoded as a qword, and return the offset to 0x
+ * for runtime patching.
+ *
+ * Warning: runtime patching is best avoided. Only use this as
+ * a replacement for DataTableRegion (for guests that don't
+ * support it).
+ *
+ * ACPI 5.0: 19.2.5
+ * 64-bit integers are supported beginning with ACPI 2.0
+ */
+int
+build_append_named_qword(GArray *array, const char *name_format, ...)
+{
+int offset;
+va_list ap;
+
+build_append_byte(array, 0x08); /* NameOp */
+va_start(ap, name_format);
+build_append_namestringv(array, name_format, ap);
+va_end(ap);
+
+build_append_byte(array, 0x0E); /* QWordPrefix */
+
+offset = array->len;
+build_append_int_noprefix(array, 0x, 8);
+assert(array->len == offset + 8);
+
+return offset;
+}
+
 static GPtrArray *alloc_list;
 
 static Aml *aml_alloc(void)
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 559326c..dbf63cf 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -385,6 +385,10 @@ int
 build_append_named_dword(GArray *array, const char *name_format, ...)
 GCC_FMT_ATTR(2, 3);
 
+int
+build_append_named_qword(GArray *array, const char *name_format, ...)
+GCC_FMT_ATTR(2, 3);
+
 void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
uint64_t len, int node, MemoryAffinityFlags flags);
 
-- 
2.7.4

Re: [Qemu-devel] [Bug 1661386] Re: Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed

[Matwey V. Kornilov]

2017-02-03 22:51 GMT+03:00 Dr. David Alan Gilbert :
> Ah well that is a bit better; you see now it's failing in kvm_**get**_msrs 
> rather
> than put; so the question is which of the two changes made it survive 
> kvm_put_msrs
>
> I'd hoped that the flags in (2) would have turned off the CPU flag and
> thus made it go in both of them.
>
> kvm_msr_entry_add: @103 index=20f value=0
> qemu-system-x86_64: /home/matwey/lab/qemu/target/i386/kvm.c:2218:
> kvm_get_msrs: Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed.
>
> 1) Was it the steal time or the pmu change that made it flip over to the 
> get_msrs?

It was has_msr_architectural_pmu.

> 2) Can you get it to flip over to the get_msrs with the flag rather
than the code change?

Only using code change.

>
> Dave
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1661386
>
> Title:
>   Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed
>
> Status in QEMU:
>   New
>
> Bug description:
>   Hello,
>
>
>   I see the following when try to run qemu from master as the following:
>
>   # ./x86_64-softmmu/qemu-system-x86_64 --version
>   QEMU emulator version 2.8.50 (v2.8.0-1006-g4e9f524)
>   Copyright (c) 2003-2016 Fabrice Bellard and the QEMU Project developers
>   # ./x86_64-softmmu/qemu-system-x86_64 -machine accel=kvm -nodefaults
>   -no-reboot -nographic -cpu host -vga none  -kernel .build.kernel.kvm
>   -initrd .build.initrd.kvm -append 'panic=1 no-kvmclock console=ttyS0
>   loglevel=7' -m 1024 -serial stdio
>   qemu-system-x86_64: /home/matwey/lab/qemu/target/i386/kvm.c:1849:
>   kvm_put_msrs: Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed.
>
>   First broken commit has been bisected:
>
>   commit 48e1a45c3166d659f781171a47dabf4a187ed7a5
>   Author: Paolo Bonzini 
>   Date:   Wed Mar 30 22:55:29 2016 +0200
>
>   target-i386: assert that KVM_GET/SET_MSRS can set all requested MSRs
>
>   This would have caught the bug in the previous patch.
>
>   Signed-off-by: Paolo Bonzini 
>
>   My cpuinfo is the following:
>
>   processor   : 0
>   vendor_id   : GenuineIntel
>   cpu family  : 6
>   model   : 44
>   model name  : Intel(R) Xeon(R) CPU   X5675  @ 3.07GHz
>   stepping: 2
>   microcode   : 0x14
>   cpu MHz : 3066.775
>   cache size  : 12288 KB
>   physical id : 0
>   siblings: 2
>   core id : 0
>   cpu cores   : 2
>   apicid  : 0
>   initial apicid  : 0
>   fpu : yes
>   fpu_exception   : yes
>   cpuid level : 11
>   wp  : yes
>   flags   : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca 
> cmov pat pse36 clflush dts mmx fxsr sse sse2 ss ht syscall nx rdtscp lm 
> constant_tsc arch_perfmon pebs bts nopl xtopology tsc_reliable nonstop_tsc 
> aperfmperf pni pclmulqdq vmx ssse3 cx16 sse4_1 sse4_2 popcnt aes hypervisor 
> lahf_lm ida arat epb dtherm tpr_shadow vnmi ept vpid
>   bugs:
>   bogomips: 6133.55
>   clflush size: 64
>   cache_alignment : 64
>   address sizes   : 40 bits physical, 48 bits virtual
>   power management:
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/qemu/+bug/1661386/+subscriptions


-- 
With best regards,
Matwey V. Kornilov
http://blog.matwey.name
xmpp://0x2...@jabber.ru

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

Title:
  Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed

Status in QEMU:
  New

Bug description:
  Hello,

  
  I see the following when try to run qemu from master as the following:

  # ./x86_64-softmmu/qemu-system-x86_64 --version
  QEMU emulator version 2.8.50 (v2.8.0-1006-g4e9f524)
  Copyright (c) 2003-2016 Fabrice Bellard and the QEMU Project developers
  # ./x86_64-softmmu/qemu-system-x86_64 -machine accel=kvm -nodefaults
  -no-reboot -nographic -cpu host -vga none  -kernel .build.kernel.kvm
  -initrd .build.initrd.kvm -append 'panic=1 no-kvmclock console=ttyS0
  loglevel=7' -m 1024 -serial stdio
  qemu-system-x86_64: /home/matwey/lab/qemu/target/i386/kvm.c:1849:
  kvm_put_msrs: Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed.

  First broken commit has been bisected:

  commit 48e1a45c3166d659f781171a47dabf4a187ed7a5
  Author: Paolo Bonzini 
  Date:   Wed Mar 30 22:55:29 2016 +0200

  target-i386: assert that KVM_GET/SET_MSRS can set all requested MSRs
  
  This would have caught the bug in the previous patch.
  
  Signed-off-by: Paolo Bonzini 

  My cpuinfo is the following:

  processor   : 0
  vendor_id   : GenuineIntel
  cpu family  : 6
  model   : 44
  model name  : Intel(R) Xeon(R) CPU   X5675  @ 3.07GHz
  stepping: 2
  microcode   : 0x14
  cpu MHz : 3066.775