Re: [Qemu-devel] [RFC PATCH 0/2] qga: add guest-get-os-version for windows

[zhanghailiang]

On 2014/12/17 0:25, Eric Blake wrote:

On 12/16/2014 12:30 AM, zhanghailiang wrote:

Hi,

This patch series add a new guest command 'guest-get-os-version'.
It is now only available for windows guest.


Why not also supply it for Linux guests?  uname() is your friend; it
should be fairly easy to wire up.



Er, this patch is RFC, i don't know if this idea (support get-os-version 
command) is acceptable,
I will add it in next formal version if it is acceptable. ;)



It will return guest's OS version name and type, like bellow:
'{"return":{"name":"Microsoft Windows Server 2012 R2","type":64}}'

Sometimes we need to know guest's OS version info.
(Actually, we need this info when we update guest's applications and drivers
in our project.)


Have you looked into libguestfs' ability to get this information from an
(offline) guest image?  I'm not rejecting this command, but trying to
make sure that it is not duplicating something that can already be done.



I have look into libgustfs codes, and yes, it supports get this info from 
offline guest image,
but unfortunately we don't use libguestfs in our project.
(I also make a simple test to get proudct name by using
"virt-inspector -d redhat-6.4  | virt-inspector --xpath '//product_name'",
it is a little  time consuming (about 12s)).

So, what's your suggestion?

Thanks,
zhanghailiang

Re: [Qemu-devel] [RFC PATCH 2/2] qga: implement qmp_guest_get_os_version for windows

[zhanghailiang]

On 2014/12/17 0:26, Eric Blake wrote:

On 12/16/2014 04:48 AM, Yan Vugenfirer wrote:

+
+if (si.wProcessorArchitecture == PROCESSOR_ARCHITECTURE_AMD64 ||
+si.wProcessorArchitecture == PROCESSOR_ARCHITECTURE_IA64) {


If one of the motivations is to update drivers on the guest - those should be 
treated as deferent architectures.
Why not return string as well (x64, x86, IA64, ARM)?


The architecture should already be known by the host (after all, the
flavor of qemu running the guest should tell you what architecture the
guest would report).



Yes, we can easily get this info from host ;)

Re: [Qemu-devel] [PATCH v2 00/17] ide: rerror and werror support for IDE and AHCI

[Markus Armbruster]

John Snow  writes:

> This series was written mostly by Paolo Bonzini to do two things:
>
> 1. Unify the restart callbacks for ISA, AHCI and BMDMA
> 2. Ensure we can restart a command after migration
>
> Many of the early patches only make much sense considering the
> end-goal of eliminating BMDMA specific restart code to be shared
> with ISA and AHCI codepaths.
>
> Migration for halted commands is fixed for ISA, PCI and AHCI.
> As a consequence, operations halted via rerror=stop or werror=stop
> should be able to be successfully migrated and resumed when using
> ISA, PCI, or AHCI.
>
> This series includes tests for ISA and PCI/BMDMA, but does not
> yet include tests for AHCI, which require some more qtest work
> to be upstreamed first. Regardless, the AHCI tests have been
> written and can be observed at:
> https://github.com/jnsnow/qemu/commits/ahci-devel-latest
>
> See "ahci: add migrate dma test" and "ahci-test: add flush migrate test"
> for the WIP versions of the AHCI test that I used to exercise this
> patchset.

I had to read this twice to even guess how this is related to the stated
subject "ide: rerror and werror support for IDE and AHCI" :)

IDE already supports rerror and werror.  I guess this series fixes
migration bugs that can bite when rerror/werror=stop.  Correct?

Re: [Qemu-devel] [PATCH v4 1/8] fw_cfg: max access size and region size are the same for MMIO data reg

[Alexander Graf]

> Am 17.12.2014 um 08:13 schrieb Laszlo Ersek :
> 
>> On 12/16/14 21:41, Peter Maydell wrote:
>>> On 16 December 2014 at 19:00, Laszlo Ersek  wrote:
>>> The root of this question is what each of
>>> 
>>> enum device_endian {
>>>DEVICE_NATIVE_ENDIAN,
>>>DEVICE_BIG_ENDIAN,
>>>DEVICE_LITTLE_ENDIAN,
>>> };
>>> 
>>> means.
>> 
>> An opening remark: endianness is a horribly confusing topic and
>> support of more than one endianness is even worse. I may have made
>> some inadvertent errors in this reply; if you think so please
>> let me know and I'll have another stab at it.
>> 
>> That said: the device_endian options indicate what a device's
>> internal idea of its endianness is. This is most relevant if
>> a device accepts accesses at wider than byte width
>> (for instance, if you can read a 32-bit value from
>> address offset 0 and also an 8-bit value from offset 3 then
>> how do those values line up? If you read a 32-bit value then
>> which way round is it compared to what the device's io read/write
>> function return?).
>> 
>> NATIVE_ENDIAN means "same order as the CPU's main data bus's
>> natural representation". (Note that this is not necessarily
>> the same as "the endianness the CPU currently has"; on ARM
>> you can flip the CPU between LE and BE at runtime, which
>> is basically inserting a byte-swizzling step between data
>> accesses and the CPU's data bus, which is always LE for ARMv7+.)
>> 
>> Note that RAM accessible to a KVM guest is always NATIVE_ENDIAN
>> (because the guest vcpu reads it directly with the h/w cpu).
>> 
>>> Consider the following call tree, which implements the splitting /
>>> combining of an MMIO read:
>>> 
>>> memory_region_dispatch_read() [memory.c]
>>>  memory_region_dispatch_read1()
>>>access_with_adjusted_size()
>>>  memory_region_big_endian()
>>>  for each byte in the wide access:
>>>memory_region_read_accessor()
>>>  fw_cfg_data_mem_read() [hw/nvram/fw_cfg.c]
>>>fw_cfg_read()
>>>  adjust_endianness()
>>>memory_region_wrong_endianness()
>>>bswapXX()
>>> 
>>> The function access_with_adjusted_size() always iterates over the MMIO
>>> address range in incrementing address order. However, it can calculate
>>> the shift count for memory_region_read_accessor() in two ways.
>>> 
>>> When memory_region_big_endian() returns "true", the shift count
>>> decreases as the MMIO address increases.
>>> 
>>> When memory_region_big_endian() returns "false", the shift count
>>> increases as the MMIO address increases.
>> 
>> Yep, because this is how the device has told us that it thinks
>> of accesses as being put together.
>> 
>> The column in your table "host value" is the 16 bit value read from
>> the device, ie what we have decided (based on device_endian) that
>> it would have returned us if it had supported a 16 bit read directly
>> itself. BE devices compose 16 bit values with the high byte first,
>> LE devices with the low byte first, and native-endian devices
>> in the same order as guest-endianness.
>> 
>>> In memory_region_read_accessor(), the shift count is used for a logical
>>> (ie. numeric) bitwise left shift (<<). That operator works with numeric
>>> values and hides (ie. accounts for) host endianness.
>>> 
>>> Let's consider
>>> - an array of two bytes, [0xaa, 0xbb],
>>> - a uint16_t access made from the guest,
>>> - and all twelve possible cases.
>>> 
>>> In the table below, the "host", "guest" and "device_endian" columns are
>>> input variables. The columns to the right are calculated / derived
>>> values. The arrows above the columns show the data dependencies.
>>> 
>>> After memory_region_dispatch_read1() constructs the value that is
>>> visible in the "host value" column, it is passed to adjust_endianness().
>>> If memory_region_wrong_endianness() returns "true", then the host value
>>> is byte-swapped. The result is then passed to the guest.
>>> 
>>>  
>>> +---+--+
>>>  |  
>>>  |  |
>>>+ 
>>> --+-+
>>>|  |
>>>| | |
>>>  |   |  |
>>>  +--+-+ 
>>>  |   |  |
>>>  | | | || | 
>>>  |   |  |
>>>  |   +---+---+  ++  | | 
>>>  ++---+  |  |
>>>  |   | | |   | | |  

Re: [Qemu-devel] [PATCH v4 1/8] fw_cfg: max access size and region size are the same for MMIO data reg

[Laszlo Ersek]

On 12/17/14 09:28, Alexander Graf wrote:
> 
> 
> 
>> Am 17.12.2014 um 08:13 schrieb Laszlo Ersek :
>>
>>> On 12/16/14 21:41, Peter Maydell wrote:
 On 16 December 2014 at 19:00, Laszlo Ersek  wrote:
 The root of this question is what each of

 enum device_endian {
DEVICE_NATIVE_ENDIAN,
DEVICE_BIG_ENDIAN,
DEVICE_LITTLE_ENDIAN,
 };

 means.
>>>
>>> An opening remark: endianness is a horribly confusing topic and
>>> support of more than one endianness is even worse. I may have made
>>> some inadvertent errors in this reply; if you think so please
>>> let me know and I'll have another stab at it.
>>>
>>> That said: the device_endian options indicate what a device's
>>> internal idea of its endianness is. This is most relevant if
>>> a device accepts accesses at wider than byte width
>>> (for instance, if you can read a 32-bit value from
>>> address offset 0 and also an 8-bit value from offset 3 then
>>> how do those values line up? If you read a 32-bit value then
>>> which way round is it compared to what the device's io read/write
>>> function return?).
>>>
>>> NATIVE_ENDIAN means "same order as the CPU's main data bus's
>>> natural representation". (Note that this is not necessarily
>>> the same as "the endianness the CPU currently has"; on ARM
>>> you can flip the CPU between LE and BE at runtime, which
>>> is basically inserting a byte-swizzling step between data
>>> accesses and the CPU's data bus, which is always LE for ARMv7+.)
>>>
>>> Note that RAM accessible to a KVM guest is always NATIVE_ENDIAN
>>> (because the guest vcpu reads it directly with the h/w cpu).
>>>
 Consider the following call tree, which implements the splitting /
 combining of an MMIO read:

 memory_region_dispatch_read() [memory.c]
  memory_region_dispatch_read1()
access_with_adjusted_size()
  memory_region_big_endian()
  for each byte in the wide access:
memory_region_read_accessor()
  fw_cfg_data_mem_read() [hw/nvram/fw_cfg.c]
fw_cfg_read()
  adjust_endianness()
memory_region_wrong_endianness()
bswapXX()

 The function access_with_adjusted_size() always iterates over the MMIO
 address range in incrementing address order. However, it can calculate
 the shift count for memory_region_read_accessor() in two ways.

 When memory_region_big_endian() returns "true", the shift count
 decreases as the MMIO address increases.

 When memory_region_big_endian() returns "false", the shift count
 increases as the MMIO address increases.
>>>
>>> Yep, because this is how the device has told us that it thinks
>>> of accesses as being put together.
>>>
>>> The column in your table "host value" is the 16 bit value read from
>>> the device, ie what we have decided (based on device_endian) that
>>> it would have returned us if it had supported a 16 bit read directly
>>> itself. BE devices compose 16 bit values with the high byte first,
>>> LE devices with the low byte first, and native-endian devices
>>> in the same order as guest-endianness.
>>>
 In memory_region_read_accessor(), the shift count is used for a logical
 (ie. numeric) bitwise left shift (<<). That operator works with numeric
 values and hides (ie. accounts for) host endianness.

 Let's consider
 - an array of two bytes, [0xaa, 0xbb],
 - a uint16_t access made from the guest,
 - and all twelve possible cases.

 In the table below, the "host", "guest" and "device_endian" columns are
 input variables. The columns to the right are calculated / derived
 values. The arrows above the columns show the data dependencies.

 After memory_region_dispatch_read1() constructs the value that is
 visible in the "host value" column, it is passed to adjust_endianness().
 If memory_region_wrong_endianness() returns "true", then the host value
 is byte-swapped. The result is then passed to the guest.

  
 +---+--+
  | 
   |  |
+ 
 --+-+
|  |
| | |   
   |   |  |
  
 +--+-+ 
  |   |  |
  | | | || 
 |  |   |  |
  |   +---+---+  +---

Re: [Qemu-devel] [PATCH v3 04/10] vnc: switch to QemuOpts, allow multiple servers

[Gerd Hoffmann]

On Mi, 2014-12-17 at 11:46 +0800, Gonglei wrote:
> On 2014/12/16 21:20, Gerd Hoffmann wrote:
> 
> > This patch switches vnc over to QemuOpts, and it (more or less
> > as side effect) allows multiple vnc server instances.
> > 
> > Signed-off-by: Gerd Hoffmann 
> > ---
> >  include/ui/console.h |   4 +-
> >  qmp.c|  15 ++-
> >  ui/vnc.c | 271 
> > ---
> >  vl.c |  42 +++-
> >  4 files changed, 200 insertions(+), 132 deletions(-)
> > 
> 
> Hi, Gerd
> I'm afraid you forgot to update this patch following comments of version 2. :)

Oops, those fixups ended up in the wrong patch (#9).  I'll correct it.

cheers,
  Gerd

Re: [Qemu-devel] [PULL 27/47] icount: set can_do_io outside TB execution

[Pavel Dovgaluk]

> From: Paolo Bonzini [mailto:pbonz...@redhat.com]
> 
> This patch sets can_do_io function to allow reading icount
> within cpu-exec, but outside TB execution.

Will you apply other icount patches that enables/disables using icount in 
particular TBs?

Pavel Dovgalyuk


> 
> Signed-off-by: Pavel Dovgalyuk 
> Signed-off-by: Paolo Bonzini 
> ---
>  cpu-exec.c | 3 +++
>  cpus.c | 3 +++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/cpu-exec.c b/cpu-exec.c
> index 4df9856..cce80f0 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -168,7 +168,9 @@ static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu, 
> uint8_t *tb_ptr)
>  }
>  #endif /* DEBUG_DISAS */
> 
> +cpu->can_do_io = 0;
>  next_tb = tcg_qemu_tb_exec(env, tb_ptr);
> +cpu->can_do_io = 1;
>  trace_exec_tb_exit((void *) (next_tb & ~TB_EXIT_MASK),
> next_tb & TB_EXIT_MASK);
> 
> @@ -543,6 +545,7 @@ int cpu_exec(CPUArchState *env)
>  cpu = current_cpu;
>  env = cpu->env_ptr;
>  cc = CPU_GET_CLASS(cpu);
> +cpu->can_do_io = 1;
>  #ifdef TARGET_I386
>  x86_cpu = X86_CPU(cpu);
>  #endif
> diff --git a/cpus.c b/cpus.c
> index 91119bb..615d4ae 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -935,6 +935,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
>  qemu_thread_get_self(cpu->thread);
>  cpu->thread_id = qemu_get_thread_id();
>  cpu->exception_index = -1;
> +cpu->can_do_io = 1;
>  current_cpu = cpu;
> 
>  r = kvm_init_vcpu(cpu);
> @@ -976,6 +977,7 @@ static void *qemu_dummy_cpu_thread_fn(void *arg)
>  qemu_thread_get_self(cpu->thread);
>  cpu->thread_id = qemu_get_thread_id();
>  cpu->exception_index = -1;
> +cpu->can_do_io = 1;
> 
>  sigemptyset(&waitset);
>  sigaddset(&waitset, SIG_IPI);
> @@ -1019,6 +1021,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
>  cpu->thread_id = qemu_get_thread_id();
>  cpu->created = true;
>  cpu->exception_index = -1;
> +cpu->can_do_io = 1;
>  }
>  qemu_cond_signal(&qemu_cpu_cond);
> 
> --
> 1.8.3.1
> 

Re: [Qemu-devel] [PATCH] qmp: implement a qmp command get_link_status

[Markus Armbruster]

Copying Eric for additional QAPI schema expertise.

Wolfgang Link  writes:

> this qmp command returns the current link state from the given nic
> this is impotent if the set_link failed or get an timeout.

Please start your sentences with a capital letter, and end them with a
period :)

If "link status" was a QOM property, we could fetch it with existing
command qom-get.  It isn't, as far as I can tell.

>
> Signed-off-by: Wolfgang Link 
> ---
>  net/net.c|   26 ++
>  qapi-schema.json |   15 +++
>  qmp-commands.hx  |   22 ++
>  3 files changed, 63 insertions(+)
>
> diff --git a/net/net.c b/net/net.c
> index 7acc162..57c6f1e 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1141,6 +1141,32 @@ void do_info_network(Monitor *mon, const QDict *qdict)
>  }
>  }
>  
> +int64_t qmp_get_link_status(const char *name, Error **errp)
> +{
> +NetClientState *ncs[MAX_QUEUE_NUM];
> +NetClientState *nc;
> +int queues;
> +bool ret;
> +
> +queues = qemu_find_net_clients_except(name, ncs,
> +  NET_CLIENT_OPTIONS_KIND_MAX,
> +  MAX_QUEUE_NUM);

This finds the NetClientState named @name.  The third parameter lets you
exclude a certain kind, but your NET_CLIENT_OPTIONS_KIND_MAX argument
excludes none.  Keep this in mind for further down.

> +
> +if (queues == 0) {
> +error_set(errp, QERR_DEVICE_NOT_FOUND, name);
> +return (int64_t) -1;
> +}
> +
> +nc = ncs[0];
> +ret = ncs[0]->link_down;
> +
> +if (nc->peer->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
> +ret = ncs[0]->peer->link_down;
> +}

Can you explain why examining only the first queue suffices?

> +
> +return (int64_t) ret ? 0 : 1;

Superfluous cast of ret.

> +}
> +
>  void qmp_set_link(const char *name, bool up, Error **errp)
>  {
>  NetClientState *ncs[MAX_QUEUE_NUM];
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 9ffdcf8..c5321e6 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1200,6 +1200,21 @@
>  { 'command': 'inject-nmi' }
>  
>  ##
> +# @get_link_status

We have two informational commands named like "get":
trace-event-set-state and qom-get.  We have many named query-FOO.  But
they usually don't take an argument.  Hmm.

> +#
> +# Get the current link state of the nics or nic.
> +#
> +# @name: name of the nic you get the state of

"nics or nic" (one or more) or "the nic" (just one)?

Doesn't your code above support any network client, not just NICs?

> +#
> +# Return: If link is up 1
> +# If link is down 0
> +# If an error occure an empty string.

A QMP command makes the server send either a success response, of the
type declared in the schema (here: 'returns': 'int'), or an error
response.

If we have nothing special to say about the error response in the
schema's command documentation, we say nothing at all.

Bool is a more natural type then int for the answer to "is the link up?"

We usually make return value objects to facilitate future extensions.
Perhaps not an issue for a command that answers the "is the link up?"
question.  Begs the question whether we should we have a more general
command to query NIC properties.

Should link status be visible in HMP's "info network"?

Stefan, what's the QMP equivalent to "info network"?

> +#
> +# Notes: this is an Proxmox VE extension and not offical part of Qemu.

Really?

> +##
> +{ 'command': 'get_link_status', 'data': {'name': 'str'}, 'returns': 'int'}
> +
> +##
>  # @set_link:
>  #
>  # Sets the link status of a virtual network adapter.
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 718dd92..ecc501a 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -1431,6 +1431,28 @@ Example:
>  EQMP
>  
>  {
> + .name   = "get_link_status",
> + .args_type  = "name:s",
> + .mhandler.cmd_new = qmp_marshal_input_get_link_status,
> +},
> +
> +SQMP
> +get_link_status
> +
> +
> +Get the link status of a network adapter.
> +
> +Arguments:
> +
> +- "name": network device name (json-string)
> +
> +Example:
> +
> +-> { "execute": "set_link", "arguments": { "name": "e1000.0" } }
> +<- { "return": {1} }
> +EQMP
> +
> +{
>  .name   = "set_link",
>  .args_type  = "name:s,up:b",
>  .mhandler.cmd_new = qmp_marshal_input_set_link,

Re: [Qemu-devel] [PATCH v2 02/10] sdl2: rename sdl2_state to sdl2_console, move to header file

[Max Reitz]

On 2014-12-16 at 14:26, Gerd Hoffmann wrote:

Create sdl2.h header file, in preparation for sdl2 code splitup.
Populate it with sdl2_console struct (renamed from sdl2_state).

Signed-off-by: Gerd Hoffmann 
---
  include/ui/sdl2.h | 16 ++
  ui/sdl2.c | 63 ---
  2 files changed, 43 insertions(+), 36 deletions(-)
  create mode 100644 include/ui/sdl2.h


Reviewed-by: Max Reitz 

Re: [Qemu-devel] [PATCH v2 03/10] sdl2: move keyboard input code to new sdl2-input.c

[Max Reitz]

On 2014-12-16 at 14:26, Gerd Hoffmann wrote:

Signed-off-by: Gerd Hoffmann 
---
  include/ui/sdl2.h |   4 +++
  ui/Makefile.objs  |   2 +-
  ui/sdl2-input.c   | 106 ++
  ui/sdl2.c |  75 ++
  4 files changed, 114 insertions(+), 73 deletions(-)
  create mode 100644 ui/sdl2-input.c


Reviewed-by: Max Reitz 

Re: [Qemu-devel] [PATCH v2 04/10] sdl2: turn on keyboard grabs

[Max Reitz]

On 2014-12-16 at 14:26, Gerd Hoffmann wrote:

Makes quite some keys actually go to the guest instead of
being captured by the host window manager.

Signed-off-by: Gerd Hoffmann 
---
  ui/sdl2.c | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)


Reviewed-by: Max Reitz 

Re: [Qemu-devel] [PATCH] QEMU:Change the IRQ number for GPIO Controller

[Amit Tomar]

Ping :)

-Original Message-
From: Tomar Amit-B51888 
Sent: Monday, December 08, 2014 3:03 PM
To: 'qemu-devel@nongnu.org'; 'qemu-...@nongnu.org'
Cc: 'ag...@suse.de'
Subject: [PATCH] QEMU:Change the IRQ number for GPIO Controller

As per RM and following links IRQ 43 is for I2C controller and IRQ 47 is for 
GPIO controller.

https://lists.ozlabs.org/pipermail/linuxppc-dev/2011-January/087924.html
http://lxr.free-electrons.com/source/arch/powerpc/boot/dts/fsl/pq3-gpio-0.dtsi?v=3.4


Signed-off-by: Amit Singh Tomar 
---
 hw/ppc/e500.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c index 2832fc0..2cd69a9 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -65,7 +65,7 @@
 #define MPC8544_UTIL_OFFSET0xeULL
 #define MPC8544_SPIN_BASE  0xEF00ULL
 #define MPC8XXX_GPIO_OFFSET0x000FF000ULL
-#define MPC8XXX_GPIO_IRQ   43
+#define MPC8XXX_GPIO_IRQ   47

 struct boot_info
 {
--
1.7.9.5

Re: [Qemu-devel] [PULL 27/47] icount: set can_do_io outside TB execution

[Paolo Bonzini]

On 17/12/2014 09:53, Pavel Dovgaluk wrote:
> > This patch sets can_do_io function to allow reading icount
> > within cpu-exec, but outside TB execution.
>
> Will you apply other icount patches that enables/disables using icount in 
> particular TBs?

Waiting for an ack for the non-x86 parts, but they're on my radar.

Paolo

Re: [Qemu-devel] [PATCH] QEMU:Change the IRQ number for GPIO Controller

[Alexander Graf]

On 17.12.14 10:18, Amit Tomar wrote:
> Ping :)
> 
> -Original Message-
> From: Tomar Amit-B51888 
> Sent: Monday, December 08, 2014 3:03 PM
> To: 'qemu-devel@nongnu.org'; 'qemu-...@nongnu.org'
> Cc: 'ag...@suse.de'
> Subject: [PATCH] QEMU:Change the IRQ number for GPIO Controller
> 
> As per RM and following links IRQ 43 is for I2C controller and IRQ 47 is for 
> GPIO controller.
> 
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2011-January/087924.html
> http://lxr.free-electrons.com/source/arch/powerpc/boot/dts/fsl/pq3-gpio-0.dtsi?v=3.4
> 
> 
> Signed-off-by: Amit Singh Tomar 

Please change the subject line to something that allows people skimming
through the patches to know that it modifies ppc e500 code.

Otherwise looks good to me.

Alex

Re: [Qemu-devel] [PATCH RFC for-2.3 0/6] qga: add three logical memory hotplug related commands

[zhanghailiang]

Ping again...

Any comments are warmly welcome! Thanks.

On 2014/12/6 14:59, zhanghailiang wrote:

Hi,

This patch series add three guest commands about memory block:
guest-get-memory-blocks, guest-set-memory-blocks, guest-get-memory-block-size.

With these three commands, we can get information about guest's memory block
online/offline status and memory block size (unit of memory online/offline
operation ). Also, we can change guest's memory block status (Logical memory
hotplug/unplug) from host.

zhanghailiang (6):
   qga: introduce three guest memory block commands with stubs
   qga: introduce three help functions for memory block functions
   qga: implement qmp_guest_get_memory_blocks() for Linux with sysfs
   qga: implement qmp_guest_set_memory_blocks() for Linux with sysfs
   qga: implement qmp_guest_get_memory_block_size() for Linux with sysfs
   qga: add memory block command that unsupported to blacklist

  qga/commands-posix.c | 266 ++-
  qga/commands-win32.c |  21 
  qga/qapi-schema.json |  88 +
  3 files changed, 374 insertions(+), 1 deletion(-)

Re: [Qemu-devel] [PATCH v4 1/8] fw_cfg: max access size and region size are the same for MMIO data reg

[Paolo Bonzini]

On 17/12/2014 06:06, Laszlo Ersek wrote:
>> > That said, the standalone selector is used by BE platforms only, so we
>> > know that the standalone selector is always DEVICE_BIG_ENDIAN.
> This series exposes the standalone selector (as MMIO) to ARM guests as
> well; and in "Documentation/devicetree/bindings/arm/fw-cfg.txt" for the
> kernel I'm saying that the selector is little endian.
> 
> Therefore I think that the standalone selector is not only (going to be)
> used by BE platforms (or I don't understand your above statement
> correctly).

It's not going to be used only by BE platforms.  But so far it is used
by BE platforms only.  If you want to keep everything consistent, you
can make it (and the wide datum) BE; and swizzle data in the firmware.
Otherwise, NATIVE_ENDIAN is fine.

> In other words, the standalone selector is NATIVE_ENDIAN, but in the
> description of the *ARM* bindings, we can simply say that it's little
> endian.

Yes.

Paolo

Re: [Qemu-devel] [PATCH v4 1/8] fw_cfg: max access size and region size are the same for MMIO data reg

[Alexander Graf]

On 17.12.14 10:23, Paolo Bonzini wrote:
> 
> 
> On 17/12/2014 06:06, Laszlo Ersek wrote:
 That said, the standalone selector is used by BE platforms only, so we
 know that the standalone selector is always DEVICE_BIG_ENDIAN.
>> This series exposes the standalone selector (as MMIO) to ARM guests as
>> well; and in "Documentation/devicetree/bindings/arm/fw-cfg.txt" for the
>> kernel I'm saying that the selector is little endian.
>>
>> Therefore I think that the standalone selector is not only (going to be)
>> used by BE platforms (or I don't understand your above statement
>> correctly).
> 
> It's not going to be used only by BE platforms.  But so far it is used
> by BE platforms only.  If you want to keep everything consistent, you
> can make it (and the wide datum) BE; and swizzle data in the firmware.
> Otherwise, NATIVE_ENDIAN is fine.

For the sake of keeping myself sane, I would really prefer not to have
NATIVE_ENDIAN. Once you start thinking about little endian guests on PPC
sharing code with little endian guests on x86 your head will start to
explode otherwise.

If code becomes simpler by making the mmio accessors BIG_ENDIAN, that's
fine with me as well.


Alex

Re: [Qemu-devel] [PATCH v3 1/3] qmp: Add command 'blockdev-backup'

[Markus Armbruster]

Fam Zheng  writes:

> Similar to drive-backup, but this command uses a device id as target
> instead of creating/opening an image file.
>
> Also add blocker on target bs, since the target is also a named device
> now.
>
> Add check and report error for bs == target which became possible but is
> an illegal case with introduction of blockdev-backup.
>
> Signed-off-by: Fam Zheng 
> ---
>  block/backup.c   | 28 +++
>  blockdev.c   | 47 +
>  qapi/block-core.json | 54 
> 
>  qmp-commands.hx  | 44 ++
>  4 files changed, 173 insertions(+)
>
> diff --git a/block/backup.c b/block/backup.c
> index 792e655..b944dd4 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -360,6 +360,7 @@ static void coroutine_fn backup_run(void *opaque)
>  hbitmap_free(job->bitmap);
>  
>  bdrv_iostatus_disable(target);
> +bdrv_op_unblock_all(target, job->common.blocker);
>  
>  data = g_malloc(sizeof(*data));
>  data->ret = ret;
> @@ -379,6 +380,11 @@ void backup_start(BlockDriverState *bs, BlockDriverState 
> *target,
>  assert(target);
>  assert(cb);
>  
> +if (bs == target) {
> +error_setg(errp, "Source and target cannot be the same");
> +return;
> +}
> +
>  if ((on_source_error == BLOCKDEV_ON_ERROR_STOP ||
>   on_source_error == BLOCKDEV_ON_ERROR_ENOSPC) &&
>  !bdrv_iostatus_is_enabled(bs)) {
> @@ -386,6 +392,26 @@ void backup_start(BlockDriverState *bs, BlockDriverState 
> *target,
>  return;
>  }
>  
> +if (!bdrv_is_inserted(bs)) {
> +error_setg(errp, "Devie is not inserted: %s",

s/Devie/Device/

> +   bdrv_get_device_name(bs));
> +return;
> +}
> +
> +if (!bdrv_is_inserted(target)) {
> +error_setg(errp, "Devie is not inserted: %s",

Likewise.

> +   bdrv_get_device_name(target));
> +return;
> +}
> +
> +if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
> +return;
> +}
> +
> +if (bdrv_op_is_blocked(target, BLOCK_OP_TYPE_BACKUP_TARGET, errp)) {
> +return;
> +}
> +
>  len = bdrv_getlength(bs);
>  if (len < 0) {
>  error_setg_errno(errp, -len, "unable to get length for '%s'",
> @@ -399,6 +425,8 @@ void backup_start(BlockDriverState *bs, BlockDriverState 
> *target,
>  return;
>  }
>  
> +bdrv_op_block_all(target, job->common.blocker);
> +
>  job->on_source_error = on_source_error;
>  job->on_target_error = on_target_error;
>  job->target = target;
> diff --git a/blockdev.c b/blockdev.c
> index 57910b8..2e5068c 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2156,6 +2156,8 @@ void qmp_drive_backup(const char *device, const char 
> *target,
>  aio_context = bdrv_get_aio_context(bs);
>  aio_context_acquire(aio_context);
>  
> +/* Although backup_run has this check too, we need to use bs->drv below, 
> so
> + * do an early check redundantly. */
>  if (!bdrv_is_inserted(bs)) {
>  error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
>  goto out;
> @@ -2172,6 +2174,7 @@ void qmp_drive_backup(const char *device, const char 
> *target,
>  }
>  }
>  
> +/* Early check to avoid creating target */
>  if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
>  goto out;
>  }
> @@ -2239,6 +2242,50 @@ BlockDeviceInfoList *qmp_query_named_block_nodes(Error 
> **errp)
>  return bdrv_named_nodes_list();
>  }
>  
> +void qmp_blockdev_backup(const char *device, const char *target,
> + enum MirrorSyncMode sync,
> + bool has_speed, int64_t speed,
> + bool has_on_source_error,
> + BlockdevOnError on_source_error,
> + bool has_on_target_error,
> + BlockdevOnError on_target_error,
> + Error **errp)
> +{
> +BlockDriverState *bs;
> +BlockDriverState *target_bs;
> +Error *local_err = NULL;
> +
> +if (!has_speed) {
> +speed = 0;
> +}
> +if (!has_on_source_error) {
> +on_source_error = BLOCKDEV_ON_ERROR_REPORT;
> +}
> +if (!has_on_target_error) {
> +on_target_error = BLOCKDEV_ON_ERROR_REPORT;
> +}
> +
> +bs = bdrv_find(device);
> +if (!bs) {
> +error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> +return;
> +}
> +
> +target_bs = bdrv_find(target);
> +if (!target_bs) {
> +error_set(errp, QERR_DEVICE_NOT_FOUND, target);
> +return;
> +}
> +
> +bdrv_ref(target_bs);
> +backup_start(bs, target_bs, speed, sync, on_source_error, 
> on_target_error,
> + block_job_cb, bs, &local_err);
> +if (local_err != NULL) {
> +bdrv_unref(target_bs);
> +error_

Re: [Qemu-devel] [PATCH v2 00/17] ide: rerror and werror support for IDE and AHCI

[Paolo Bonzini]

On 17/12/2014 09:23, Markus Armbruster wrote:
> I had to read this twice to even guess how this is related to the stated
> subject "ide: rerror and werror support for IDE and AHCI" :)
> 
> IDE already supports rerror and werror.  I guess this series fixes
> migration bugs that can bite when rerror/werror=stop.  Correct?

IDE over PCI supports rerror and werror.  IDE over ISA does not.

This series makes it possible to support it very easily, see patch 13.

Paolo

Re: [Qemu-devel] [PATCH v3 2/3] block: Add blockdev-backup to transaction

[Markus Armbruster]

Fam Zheng  writes:

> BTW add version info for other transaction types.
>
> Signed-off-by: Fam Zheng 
> ---
>  blockdev.c   | 48 
>  qapi-schema.json |  7 +++
>  2 files changed, 55 insertions(+)
>
> diff --git a/blockdev.c b/blockdev.c
> index 2e5068c..6401850 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1495,6 +1495,49 @@ static void drive_backup_abort(BlkTransactionState 
> *common)
>  }
>  }
>  
> +typedef struct BlockdevBackupState {
> +BlkTransactionState common;
> +BlockDriverState *bs;
> +BlockJob *job;
> +} BlockdevBackupState;
> +
> +static void blockdev_backup_prepare(BlkTransactionState *common, Error 
> **errp)
> +{
> +BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, 
> common);
> +BlockdevBackup *backup;
> +Error *local_err = NULL;
> +
> +assert(common->action->kind == TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP);
> +backup = common->action->blockdev_backup;
> +
> +qmp_blockdev_backup(backup->device, backup->target,
> +backup->sync,
> +backup->has_speed, backup->speed,
> +backup->has_on_source_error, backup->on_source_error,
> +backup->has_on_target_error, backup->on_target_error,
> +&local_err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +state->bs = NULL;
> +state->job = NULL;
> +return;
> +}
> +
> +state->bs = bdrv_find(backup->device);
> +state->job = state->bs->job;
> +}
> +
> +static void blockdev_backup_abort(BlkTransactionState *common)
> +{
> +BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, 
> common);
> +BlockDriverState *bs = state->bs;
> +
> +/* Only cancel if it's the job we started */
> +if (bs && bs->job && bs->job == state->job) {
> +block_job_cancel_sync(bs->job);
> +}
> +}
> +
>  static void abort_prepare(BlkTransactionState *common, Error **errp)
>  {
>  error_setg(errp, "Transaction aborted using Abort action");
> @@ -1517,6 +1560,11 @@ static const BdrvActionOps actions[] = {
>  .prepare = drive_backup_prepare,
>  .abort = drive_backup_abort,
>  },
> +[TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP] = {
> +.instance_size = sizeof(BlockdevBackupState),
> +.prepare = blockdev_backup_prepare,
> +.abort = blockdev_backup_abort,
> +},
>  [TRANSACTION_ACTION_KIND_ABORT] = {
>  .instance_size = sizeof(BlkTransactionState),
>  .prepare = abort_prepare,
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 24379ab..5fc6892 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1254,11 +1254,18 @@
>  #
>  # A discriminated record of operations that can be performed with
>  # @transaction.
> +#
> +# Since 1.1
> +# drive-backup since 1.6
> +# abort since 1.6
> +# blockdev-snapshot-internal-sync since 1.7

I'd do these belated doc updates in a separate patch preceding this one.
Up to you.

> +# blockdev-backup since 2.3
>  ##
>  { 'union': 'TransactionAction',
>'data': {
> 'blockdev-snapshot-sync': 'BlockdevSnapshot',
> 'drive-backup': 'DriveBackup',
> +   'blockdev-backup': 'BlockdevBackup',
> 'abort': 'Abort',
> 'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal'
> } }

Re: [Qemu-devel] [PATCH v2 15/17] ahci: add support for restarting non-queued commands

[Paolo Bonzini]

On 17/12/2014 02:36, John Snow wrote:
> + * If an error was there, ad->busy_slot will not be -1.  Restarting
> + * the operation will resume execution of the command list, do
> + * nothing here.

I wrote this comment, and I had to reread it twice to make sense of it.

Perhaps:

* If an error was there, ad->busy_slot will not be -1.  In that
* case, restarting the operation will resume execution of the
* command list.  Otherwise we have to do it here.

Paolo

Re: [Qemu-devel] [PATCH] qmp: implement a qmp command get_link_status

[Eric Blake]

On 12/17/2014 02:02 AM, Markus Armbruster wrote:
> Copying Eric for additional QAPI schema expertise.

Thanks (not sure why I didn't see the original, as I usually notice
emails that touch .json files)

> 
> Wolfgang Link  writes:
> 
>> this qmp command returns the current link state from the given nic
>> this is impotent if the set_link failed or get an timeout.

s/impotent/important/ (very different meanings!)
s/get an timeout/had a timeout/

> 
> Please start your sentences with a capital letter, and end them with a
> period :)
> 
> If "link status" was a QOM property, we could fetch it with existing
> command qom-get.  It isn't, as far as I can tell.

Adding it as a QOM property would make it accessible but tedious to get
to; even better would be adding it to a general query command that can
tell all sorts of things about the network device.  That is, we probably
want some sort of 'query-netdev' that can return link status and more.


>> +
>> +return (int64_t) ret ? 0 : 1;
> 
> Superfluous cast of ret.

Even shorter as:
return !ret;

but why return an integer when a bool will do?


>> +++ b/qapi-schema.json
>> @@ -1200,6 +1200,21 @@
>>  { 'command': 'inject-nmi' }
>>  
>>  ##
>> +# @get_link_status
> 
> We have two informational commands named like "get":
> trace-event-set-state and qom-get.  We have many named query-FOO.  But
> they usually don't take an argument.  Hmm.

Also, new commands should be named with dash '-', not underscore '_'.

> 
>> +#
>> +# Get the current link state of the nics or nic.
>> +#
>> +# @name: name of the nic you get the state of
> 
> "nics or nic" (one or more) or "the nic" (just one)?
> 
> Doesn't your code above support any network client, not just NICs?
> 
>> +#
>> +# Return: If link is up 1
>> +# If link is down 0
>> +# If an error occure an empty string.

s/occure/occurs,/
but as Markus correctly pointed out, that is not how an error is returned.

> 
> A QMP command makes the server send either a success response, of the
> type declared in the schema (here: 'returns': 'int'), or an error
> response.
> 
> If we have nothing special to say about the error response in the
> schema's command documentation, we say nothing at all.
> 
> Bool is a more natural type then int for the answer to "is the link up?"
> 
> We usually make return value objects to facilitate future extensions.
> Perhaps not an issue for a command that answers the "is the link up?"
> question.  Begs the question whether we should we have a more general
> command to query NIC properties.

Yes, we should (if we don't already have it under some other name; a
quick search for query-net didn't find it).

> 
> Should link status be visible in HMP's "info network"?

Yes.

> 
> Stefan, what's the QMP equivalent to "info network"?

Alas, I think this is one place where QMP hasn't caught up, and HMP is
still open-coding things.

> 
>> +#
>> +# Notes: this is an Proxmox VE extension and not offical part of Qemu.

s/offical/official/

> 
> Really?
> 
>> +##
>> +{ 'command': 'get_link_status', 'data': {'name': 'str'}, 'returns': 'int'}

Markus is correct; 'returns':'int' is not extensible, and you should
instead be returning a struct (or better yet, like other query commands,
return an array of structs, where each array member describes a network
device, so that one command learns the state of all devices at once).

>> +SQMP
>> +get_link_status
>> +

Make the --- pad out to the length of the command name.

>> +
>> +Get the link status of a network adapter.
>> +
>> +Arguments:
>> +
>> +- "name": network device name (json-string)
>> +
>> +Example:
>> +
>> +-> { "execute": "set_link", "arguments": { "name": "e1000.0" } }

Example doesn't match the command it is describing (did you mean
"get_link_status" for the command name?)

>> +<- { "return": {1} }

Invalid JSON.  As spec'd by your patch, this would be { "return": 1 };
but ideally you want a struct, as in { "return": { "link": true } }, or
even an array of structs, as in { "return": [ { "device": "net0",
"link": true }, { "device": "net1", "link": false } ] }

>> +EQMP
>> +
>> +{
>>  .name   = "set_link",
>>  .args_type  = "name:s,up:b",
>>  .mhandler.cmd_new = qmp_marshal_input_set_link,
> 
> 

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] Unable to loadvm on qemu-system-ppc -M g3beige (keyboard freeze)

[Mark Cave-Ayland]

Hi all,

Whilst trying to debug a regression, I've found that I can't
successfully execute a savevm/restart/loadvm sequence in qemu-system-ppc
-M g3beige. The loadvm command succeeds after the restart, however the
keyboard remains unresponsive making it impossible to continue from
where I left off.

Steps to reproduce:

1) Launch qemu-system-ppc as follows:

qemu-system-ppc -M g3beige -hda tmp.qcow2 -prom-env 'auto-boot?=false'

2) Switch to the monitor in order to pause the VM and save

(qemu) stop
(qemu) savevm test
(qemu) c

3) Switch back to the VGA display and confirm the keyboard works

4) Close qemu-system-ppc

5) Restart qemu-system-ppc with -loadvm options

qemu-system-ppc -M g3beige -prom-env 'auto-boot?=false' -loadvm test


After the restart the keyboard is frozen and unresponsive. Some
preliminary poking with gdb before and after seems to indicate that the
cuda_receive_packet() functions aren't being called after the restart
indicating that likely some state is missing from the savevm image and
not being restored correctly. Anyone have any ideas as to what might be
going wrong?


ATB,

Mark.

Re: [Qemu-devel] [Qemu-ppc] Unable to loadvm on qemu-system-ppc -M g3beige (keyboard freeze)

[Alexander Graf]

On 17.12.14 11:17, Mark Cave-Ayland wrote:
> Hi all,
> 
> Whilst trying to debug a regression, I've found that I can't
> successfully execute a savevm/restart/loadvm sequence in qemu-system-ppc
> -M g3beige. The loadvm command succeeds after the restart, however the
> keyboard remains unresponsive making it impossible to continue from
> where I left off.
> 
> Steps to reproduce:
> 
> 1) Launch qemu-system-ppc as follows:
> 
> qemu-system-ppc -M g3beige -hda tmp.qcow2 -prom-env 'auto-boot?=false'
> 
> 2) Switch to the monitor in order to pause the VM and save
> 
> (qemu) stop
> (qemu) savevm test
> (qemu) c
> 
> 3) Switch back to the VGA display and confirm the keyboard works
> 
> 4) Close qemu-system-ppc
> 
> 5) Restart qemu-system-ppc with -loadvm options
> 
> qemu-system-ppc -M g3beige -prom-env 'auto-boot?=false' -loadvm test
> 
> 
> After the restart the keyboard is frozen and unresponsive. Some
> preliminary poking with gdb before and after seems to indicate that the
> cuda_receive_packet() functions aren't being called after the restart
> indicating that likely some state is missing from the savevm image and
> not being restored correctly. Anyone have any ideas as to what might be
> going wrong?

I don't think anyone has test the migration code on non-papr ppc for
quite a while. In fact, I'm surprised it's only the keyboard not working :).

You're more than invited to send patches to fix it! :)


Alex

Re: [Qemu-devel] [RFC PATCH] target-arm: protect cpu_exclusive_*.

[Frederic Konrad]

On 16/12/2014 17:37, Peter Maydell wrote:

On 16 December 2014 at 09:13,   wrote:

From: KONRAD Frederic 

This adds a lock to avoid multiple exclusive access at the same time in case of
TCG multithread.

Hi Peter,


This feels to me like it's not really possible to review on
its own, since you can't see how it fits into the design of
the rest of the multithreading support.

true the only thing we observe is that it didn't change anything right now.


The other approach here rather than having a pile of mutexes
in the target-* code would be to have TCG IR support for
"begin critical section"/"end critical section". Then you
could have the main loop ensure that no other CPU is running
at the same time as the critical-section code. (linux-user
already has an ad-hoc implementation of this for the
exclusives.)

-- PMM


What do you mean by TCG IR?

Thanks,
Fred

Re: [Qemu-devel] [RFC PATCH] target-arm: protect cpu_exclusive_*.

[Alexander Graf]

On 17.12.14 11:27, Frederic Konrad wrote:
> On 16/12/2014 17:37, Peter Maydell wrote:
>> On 16 December 2014 at 09:13,   wrote:
>>> From: KONRAD Frederic 
>>>
>>> This adds a lock to avoid multiple exclusive access at the same time
>>> in case of
>>> TCG multithread.
> Hi Peter,
> 
>> This feels to me like it's not really possible to review on
>> its own, since you can't see how it fits into the design of
>> the rest of the multithreading support.
> true the only thing we observe is that it didn't change anything right now.
> 
>> The other approach here rather than having a pile of mutexes
>> in the target-* code would be to have TCG IR support for
>> "begin critical section"/"end critical section". Then you
>> could have the main loop ensure that no other CPU is running
>> at the same time as the critical-section code. (linux-user
>> already has an ad-hoc implementation of this for the
>> exclusives.)
>>
>> -- PMM
>>
> What do you mean by TCG IR?

TCP ops. The nice thing is that TCG could translate those into
transactions if the host supports them as well.


Alex

Re: [Qemu-devel] [RFC PATCH] target-arm: protect cpu_exclusive_*.

[Mark Burton]

> On 17 Dec 2014, at 11:28, Alexander Graf  wrote:
> 
> 
> 
> On 17.12.14 11:27, Frederic Konrad wrote:
>> On 16/12/2014 17:37, Peter Maydell wrote:
>>> On 16 December 2014 at 09:13,   wrote:
 From: KONRAD Frederic 
 
 This adds a lock to avoid multiple exclusive access at the same time
 in case of
 TCG multithread.
>> Hi Peter,
>> 
>>> This feels to me like it's not really possible to review on
>>> its own, since you can't see how it fits into the design of
>>> the rest of the multithreading support.
>> true the only thing we observe is that it didn't change anything right now.
>> 
>>> The other approach here rather than having a pile of mutexes
>>> in the target-* code would be to have TCG IR support for
>>> "begin critical section"/"end critical section". Then you
>>> could have the main loop ensure that no other CPU is running
>>> at the same time as the critical-section code. (linux-user
>>> already has an ad-hoc implementation of this for the
>>> exclusives.)
>>> 
>>> -- PMM
>>> 
>> What do you mean by TCG IR?
> 
> TCP ops. The nice thing is that TCG could translate those into
> transactions if the host supports them as well.
> 

Hows that different in reality from what we have now?
Cheers
Mark.

> 
> Alex


 +44 (0)20 7100 3485 x 210
 +33 (0)5 33 52 01 77x 210

+33 (0)603762104
mark.burton

Re: [Qemu-devel] [PATCH v3 3/3] qemu-iotests: Test blockdev-backup in 055

[Markus Armbruster]

Fam Zheng  writes:

> This applies cases on drive-backup on blockdev-backup, except cases with
> target format and mode.
>
> Also add a case to check source == target.
>
> Signed-off-by: Fam Zheng 
> ---
>  tests/qemu-iotests/055 | 211 
> +
>  tests/qemu-iotests/055.out |   4 +-
>  2 files changed, 176 insertions(+), 39 deletions(-)

I rather like how you changed this from v1.  Diffstat then was
236 insertions(+), 45 deletions(-).

Re: [Qemu-devel] [PATCH v3 1/3] qmp: Add command 'blockdev-backup'

[Fam Zheng]

On Wed, 12/17 10:36, Markus Armbruster wrote:
> Fam Zheng  writes:
> 
> > Similar to drive-backup, but this command uses a device id as target
> > instead of creating/opening an image file.
> >
> > Also add blocker on target bs, since the target is also a named device
> > now.
> >
> > Add check and report error for bs == target which became possible but is
> > an illegal case with introduction of blockdev-backup.
> >
> > Signed-off-by: Fam Zheng 
> > ---
> >  block/backup.c   | 28 +++
> >  blockdev.c   | 47 +
> >  qapi/block-core.json | 54 
> > 
> >  qmp-commands.hx  | 44 ++
> >  4 files changed, 173 insertions(+)
> >
> > diff --git a/block/backup.c b/block/backup.c
> > index 792e655..b944dd4 100644
> > --- a/block/backup.c
> > +++ b/block/backup.c
> > @@ -360,6 +360,7 @@ static void coroutine_fn backup_run(void *opaque)
> >  hbitmap_free(job->bitmap);
> >  
> >  bdrv_iostatus_disable(target);
> > +bdrv_op_unblock_all(target, job->common.blocker);
> >  
> >  data = g_malloc(sizeof(*data));
> >  data->ret = ret;
> > @@ -379,6 +380,11 @@ void backup_start(BlockDriverState *bs, 
> > BlockDriverState *target,
> >  assert(target);
> >  assert(cb);
> >  
> > +if (bs == target) {
> > +error_setg(errp, "Source and target cannot be the same");
> > +return;
> > +}
> > +
> >  if ((on_source_error == BLOCKDEV_ON_ERROR_STOP ||
> >   on_source_error == BLOCKDEV_ON_ERROR_ENOSPC) &&
> >  !bdrv_iostatus_is_enabled(bs)) {
> > @@ -386,6 +392,26 @@ void backup_start(BlockDriverState *bs, 
> > BlockDriverState *target,
> >  return;
> >  }
> >  
> > +if (!bdrv_is_inserted(bs)) {
> > +error_setg(errp, "Devie is not inserted: %s",
> 
> s/Devie/Device/

OK.

> 
> > +   bdrv_get_device_name(bs));
> > +return;
> > +}
> > +
> > +if (!bdrv_is_inserted(target)) {
> > +error_setg(errp, "Devie is not inserted: %s",
> 
> Likewise.

OK.

> 
> > +   bdrv_get_device_name(target));
> > +return;
> > +}
> > +
> > +if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
> > +return;
> > +}
> > +
> > +if (bdrv_op_is_blocked(target, BLOCK_OP_TYPE_BACKUP_TARGET, errp)) {
> > +return;
> > +}
> > +
> >  len = bdrv_getlength(bs);
> >  if (len < 0) {
> >  error_setg_errno(errp, -len, "unable to get length for '%s'",
> > @@ -399,6 +425,8 @@ void backup_start(BlockDriverState *bs, 
> > BlockDriverState *target,
> >  return;
> >  }
> >  
> > +bdrv_op_block_all(target, job->common.blocker);
> > +
> >  job->on_source_error = on_source_error;
> >  job->on_target_error = on_target_error;
> >  job->target = target;
> > diff --git a/blockdev.c b/blockdev.c
> > index 57910b8..2e5068c 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -2156,6 +2156,8 @@ void qmp_drive_backup(const char *device, const char 
> > *target,
> >  aio_context = bdrv_get_aio_context(bs);
> >  aio_context_acquire(aio_context);
> >  
> > +/* Although backup_run has this check too, we need to use bs->drv 
> > below, so
> > + * do an early check redundantly. */
> >  if (!bdrv_is_inserted(bs)) {
> >  error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
> >  goto out;
> > @@ -2172,6 +2174,7 @@ void qmp_drive_backup(const char *device, const char 
> > *target,
> >  }
> >  }
> >  
> > +/* Early check to avoid creating target */
> >  if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
> >  goto out;
> >  }
> > @@ -2239,6 +2242,50 @@ BlockDeviceInfoList 
> > *qmp_query_named_block_nodes(Error **errp)
> >  return bdrv_named_nodes_list();
> >  }
> >  
> > +void qmp_blockdev_backup(const char *device, const char *target,
> > + enum MirrorSyncMode sync,
> > + bool has_speed, int64_t speed,
> > + bool has_on_source_error,
> > + BlockdevOnError on_source_error,
> > + bool has_on_target_error,
> > + BlockdevOnError on_target_error,
> > + Error **errp)
> > +{
> > +BlockDriverState *bs;
> > +BlockDriverState *target_bs;
> > +Error *local_err = NULL;
> > +
> > +if (!has_speed) {
> > +speed = 0;
> > +}
> > +if (!has_on_source_error) {
> > +on_source_error = BLOCKDEV_ON_ERROR_REPORT;
> > +}
> > +if (!has_on_target_error) {
> > +on_target_error = BLOCKDEV_ON_ERROR_REPORT;
> > +}
> > +
> > +bs = bdrv_find(device);
> > +if (!bs) {
> > +error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> > +return;
> > +}
> > +
> > +target_bs = bdrv_find(target);
> > +

Re: [Qemu-devel] [PATCH v3 2/3] block: Add blockdev-backup to transaction

[Fam Zheng]

On Wed, 12/17 10:40, Markus Armbruster wrote:
> Fam Zheng  writes:
> 
> > BTW add version info for other transaction types.
> >
> > Signed-off-by: Fam Zheng 
> > ---
> >  blockdev.c   | 48 
> >  qapi-schema.json |  7 +++
> >  2 files changed, 55 insertions(+)
> >
> > diff --git a/blockdev.c b/blockdev.c
> > index 2e5068c..6401850 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -1495,6 +1495,49 @@ static void drive_backup_abort(BlkTransactionState 
> > *common)
> >  }
> >  }
> >  
> > +typedef struct BlockdevBackupState {
> > +BlkTransactionState common;
> > +BlockDriverState *bs;
> > +BlockJob *job;
> > +} BlockdevBackupState;
> > +
> > +static void blockdev_backup_prepare(BlkTransactionState *common, Error 
> > **errp)
> > +{
> > +BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, 
> > common);
> > +BlockdevBackup *backup;
> > +Error *local_err = NULL;
> > +
> > +assert(common->action->kind == 
> > TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP);
> > +backup = common->action->blockdev_backup;
> > +
> > +qmp_blockdev_backup(backup->device, backup->target,
> > +backup->sync,
> > +backup->has_speed, backup->speed,
> > +backup->has_on_source_error, 
> > backup->on_source_error,
> > +backup->has_on_target_error, 
> > backup->on_target_error,
> > +&local_err);
> > +if (local_err) {
> > +error_propagate(errp, local_err);
> > +state->bs = NULL;
> > +state->job = NULL;
> > +return;
> > +}
> > +
> > +state->bs = bdrv_find(backup->device);
> > +state->job = state->bs->job;
> > +}
> > +
> > +static void blockdev_backup_abort(BlkTransactionState *common)
> > +{
> > +BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, 
> > common);
> > +BlockDriverState *bs = state->bs;
> > +
> > +/* Only cancel if it's the job we started */
> > +if (bs && bs->job && bs->job == state->job) {
> > +block_job_cancel_sync(bs->job);
> > +}
> > +}
> > +
> >  static void abort_prepare(BlkTransactionState *common, Error **errp)
> >  {
> >  error_setg(errp, "Transaction aborted using Abort action");
> > @@ -1517,6 +1560,11 @@ static const BdrvActionOps actions[] = {
> >  .prepare = drive_backup_prepare,
> >  .abort = drive_backup_abort,
> >  },
> > +[TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP] = {
> > +.instance_size = sizeof(BlockdevBackupState),
> > +.prepare = blockdev_backup_prepare,
> > +.abort = blockdev_backup_abort,
> > +},
> >  [TRANSACTION_ACTION_KIND_ABORT] = {
> >  .instance_size = sizeof(BlkTransactionState),
> >  .prepare = abort_prepare,
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 24379ab..5fc6892 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -1254,11 +1254,18 @@
> >  #
> >  # A discriminated record of operations that can be performed with
> >  # @transaction.
> > +#
> > +# Since 1.1
> > +# drive-backup since 1.6
> > +# abort since 1.6
> > +# blockdev-snapshot-internal-sync since 1.7
> 
> I'd do these belated doc updates in a separate patch preceding this one.
> Up to you.

Okay, I remember one previous reviewer said this is fine, but since we are
talking about it, why not split it.

Fam

> 
> > +# blockdev-backup since 2.3
> >  ##
> >  { 'union': 'TransactionAction',
> >'data': {
> > 'blockdev-snapshot-sync': 'BlockdevSnapshot',
> > 'drive-backup': 'DriveBackup',
> > +   'blockdev-backup': 'BlockdevBackup',
> > 'abort': 'Abort',
> > 'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal'
> > } }

Re: [Qemu-devel] [PATCH v3 0/3] qmp: Add "blockdev-backup"

[Markus Armbruster]

Fam Zheng  writes:

> v3: Address Eric's comments on documentation.
> Squashed 3/4 into 2/4.
>
> v2: Address Markus' and Eric's comments:
> Fix qapi schema documentation.
> Fix versioning of transactions.
> Improve test case code by dropping inelegnet bool.
>
> The existing drive-backup command accepts a target file path, but that
> interface provides little flexibility on the properties of target block 
> device,
> compared to what is possible with "blockdev-add", "drive_add" or "-drive".
>
> This is also a building block to allow image fleecing (creating a point in 
> time
> snapshot and export with nbd-server-add).
>
> (For symmetry, blockdev-mirror will be added in a separate series.)

Trivial typos/pastos in PATCH 1, and a question on GenericError.  If you
address these, you can add my R-by.

Re: [Qemu-devel] [RFC PATCH] target-arm: protect cpu_exclusive_*.

[Alexander Graf]

On 17.12.14 11:31, Mark Burton wrote:
> 
>> On 17 Dec 2014, at 11:28, Alexander Graf  wrote:
>>
>>
>>
>> On 17.12.14 11:27, Frederic Konrad wrote:
>>> On 16/12/2014 17:37, Peter Maydell wrote:
 On 16 December 2014 at 09:13,   wrote:
> From: KONRAD Frederic 
>
> This adds a lock to avoid multiple exclusive access at the same time
> in case of
> TCG multithread.
>>> Hi Peter,
>>>
 This feels to me like it's not really possible to review on
 its own, since you can't see how it fits into the design of
 the rest of the multithreading support.
>>> true the only thing we observe is that it didn't change anything right now.
>>>
 The other approach here rather than having a pile of mutexes
 in the target-* code would be to have TCG IR support for
 "begin critical section"/"end critical section". Then you
 could have the main loop ensure that no other CPU is running
 at the same time as the critical-section code. (linux-user
 already has an ad-hoc implementation of this for the
 exclusives.)

 -- PMM

>>> What do you mean by TCG IR?
>>
>> TCP ops. The nice thing is that TCG could translate those into
>> transactions if the host supports them as well.
>>
> 
> Hows that different in reality from what we have now?
> Cheers
> Mark.

The current code can't optimize things in TCG. There's a good chance
your TCG host implementation can have an optimization pass that creates
host cmpxchg instructions or maybe even transaction blocks out of the
critical sections.


Alex

Re: [Qemu-devel] [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1

[Lokesha, Amulya]

Hi Max, Jeff,

We were able to get the qemu patch files downloaded from the qemu patch site -  
https://patchwork.ozlabs.org and were able to apply the patches successfully 
without any errors. With the patches applied, we recompiled the qemu and 
converted the VDK vmdk to vhdx format and uploaded to the SCVMM Server. But it 
failed again with the syntax error as below:

Information (10804)
Unable to import \\TestServer\MSSCVMMLibrary\VHDs\Product-disk1.vhdx because of 
a syntax error in the file.

Please find my comments inline for your questions

Please let us know if there is anything else you need from us.


Thanks,
Amulya


-Original Message-
From: Jeff Cody [mailto:jc...@redhat.com]
Sent: Friday, December 12, 2014 8:48 PM
To: Lokesha, Amulya
Cc: Max Reitz; qemu-devel@nongnu.org; kw...@redhat.com; stefa...@redhat.com
Subject: Re: [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to 
bdrv_has_zero_init_1

On Fri, Dec 12, 2014 at 09:43:16AM -0500, Lokesha, Amulya wrote:
>Hi Max,
> 
> 

Please reply in-line, it makes it easier to follow technical discussions - 
thanks :)

> 
>We applied all the 5 patches from the mail chain I got since the last
>week. Please find attached the patches used by us.
> 
>We were unable to apply the patch3 as it failed with the following 
> error
> 
> 
> 
># patch -p1 < patch3
> 
>patching file block/vhdx.c
> 
>patch:  malformed patch at line 17:  error_setg_errno(errp,
>EINVAL, "Image size too large; max of 64TB"); @@ -1936,7 +1936,9 @@ static
>QemuOptsList vhdx_create_opts = {
> 
>

It looks like however you saved the patch file, it was corrupted.
Looking at your attached patch 3, it split line 9 across 2 lines.
Your patch also has whitespace differences from the patch I sent.

You also attached 5 patches - Why are you using patch 0?  You should only be 
applying patches 1-4.  This should not be causing any actual issues, however.
[Amulya]: First time we applied patches 1 to 4, created VHDX image and deployed 
to HyperV Server, but we got the same error. Then we took a fresh qemu source 
and applied patches 0 to 4 and deployed to HyperV and again got the same syntax 
error.


Are you using git for your qemu version?  If so, 'git am' is the preferred 
method of applying the patches - just save each of the patch emails (the whole 
email should be fine), and run 'git am' on each file.

[Amulya] : No. We don't have a git repository for our team. Could you please 
let us know how to apply these patches without git. What is the difference in 
applying the patch directly and modifying the code directly? Does it have any 
impact?

> 
> 
> 
>Hence, we manually added the patch3 changes and recompiled the qemu. We
>then used the patched qemu-img to convert  our vmdk image to dynamic VHDX
>format. We found that the image created this time had a considerable
>decrease in its size from 50GB to 12GB.
> 

Could you tell me the file size of the VMDK image you were converting?
Is it roughly 12GB as well?
[Amulya] :  No, the vmdk image which we used for conversion is just 1.4GB


>However, when we deployed it into our SCVMM 2012, the import of the VHDX
>image failed with a "syntax error" as below
> 
> 
> 
>Information (10804)
> 
>Unable to import \\Test.com\Library\VHDs\Test-disk1.vhdx because of a
>syntax error in the file.
> 
> 

If you run qemu-img info on Test-disk1.vhdx, what does it say?

[Amulya] :   The following is seen with qemu-img info
# qemu-img info Test-disk1.vhdx
image: Test-disk1.vhdx
file format: vhdx
virtual size: 50G (53687091200 bytes)
disk size: 3.4G
cluster_size: 16777216

The size of our images
# ls -ltrh
total 4.8G
-rw-r--r-- 1 root root 1.4G Dec  9 10:48 Test-disk1.vmdk
-rw-r--r-- 1 root root  12G Dec 12 04:49 Test-disk1.vhdx

> 
>Please let us know if we missed anything.
> 
> 
> 
>Thanks,
> 
>Amulya
> 
> 
>

[...]

Jeff

Re: [Qemu-devel] [Qemu-ppc] Unable to loadvm on qemu-system-ppc -M g3beige (keyboard freeze)

[Mark Cave-Ayland]

On 17/12/14 10:23, Alexander Graf wrote:

>> After the restart the keyboard is frozen and unresponsive. Some
>> preliminary poking with gdb before and after seems to indicate that the
>> cuda_receive_packet() functions aren't being called after the restart
>> indicating that likely some state is missing from the savevm image and
>> not being restored correctly. Anyone have any ideas as to what might be
>> going wrong?
> 
> I don't think anyone has test the migration code on non-papr ppc for
> quite a while. In fact, I'm surprised it's only the keyboard not working :).
> 
> You're more than invited to send patches to fix it! :)

How did I know that would be the response ;) I don't suppose anyone has
a vague memory of the timeframe/QEMU version when a savevm/loadvm was
last working for -M g3beige as a starting point?!


ATB,

Mark.

[Qemu-devel] [PATCH v2 5/5] bootdevice: add Error **errp argument for QEMUBootSetHandler

[arei.gonglei]

From: Gonglei 

We can use it for checking when we change traditional
boot order dynamically and propagate error message
to the monitor.
For x86 architecture, we pass &local_err to set_boot_dev()
when vm startup in pc_coms_init().

Cc: Michael S. Tsirkin 
Cc: Alexander Graf 
Cc: Blue Swirl 
Cc: qemu-...@nongnu.org
Signed-off-by: Gonglei 
---
 bootdevice.c|  5 +
 hw/i386/pc.c| 23 +--
 hw/ppc/mac_newworld.c   |  4 ++--
 hw/ppc/mac_oldworld.c   |  5 ++---
 hw/sparc/sun4m.c|  4 ++--
 hw/sparc64/sun4u.c  |  4 ++--
 include/sysemu/sysemu.h |  4 ++--
 7 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/bootdevice.c b/bootdevice.c
index 9de34ba..5914417 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -63,10 +63,7 @@ void qemu_boot_set(const char *boot_order, Error **errp)
 return;
 }
 
-if (boot_set_handler(boot_set_opaque, boot_order)) {
-error_setg(errp, "setting boot device list failed");
-return;
-}
+boot_set_handler(boot_set_opaque, boot_order, errp);
 }
 
 void validate_bootdevices(const char *devices, Error **errp)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index c0e55a6..6b7fdea 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -282,7 +282,7 @@ static int boot_device2nibble(char boot_device)
 return 0;
 }
 
-static int set_boot_dev(ISADevice *s, const char *boot_device)
+static void set_boot_dev(ISADevice *s, const char *boot_device, Error **errp)
 {
 #define PC_MAX_BOOT_DEVICES 3
 int nbds, bds[3] = { 0, };
@@ -290,25 +290,24 @@ static int set_boot_dev(ISADevice *s, const char 
*boot_device)
 
 nbds = strlen(boot_device);
 if (nbds > PC_MAX_BOOT_DEVICES) {
-error_report("Too many boot devices for PC");
-return(1);
+error_setg(errp, "Too many boot devices for PC");
+return;
 }
 for (i = 0; i < nbds; i++) {
 bds[i] = boot_device2nibble(boot_device[i]);
 if (bds[i] == 0) {
-error_report("Invalid boot device for PC: '%c'",
- boot_device[i]);
-return(1);
+error_setg(errp, "Invalid boot device for PC: '%c'",
+   boot_device[i]);
+return;
 }
 }
 rtc_set_memory(s, 0x3d, (bds[1] << 4) | bds[0]);
 rtc_set_memory(s, 0x38, (bds[2] << 4) | (fd_bootchk ? 0x0 : 0x1));
-return(0);
 }
 
-static int pc_boot_set(void *opaque, const char *boot_device)
+static void pc_boot_set(void *opaque, const char *boot_device, Error **errp)
 {
-return set_boot_dev(opaque, boot_device);
+set_boot_dev(opaque, boot_device, errp);
 }
 
 typedef struct pc_cmos_init_late_arg {
@@ -365,6 +364,7 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t 
above_4g_mem_size,
 FDriveType fd_type[2] = { FDRIVE_DRV_NONE, FDRIVE_DRV_NONE };
 static pc_cmos_init_late_arg arg;
 PCMachineState *pc_machine = PC_MACHINE(machine);
+Error *local_err = NULL;
 
 /* various important CMOS locations needed by PC/Bochs bios */
 
@@ -412,7 +412,10 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t 
above_4g_mem_size,
 object_property_set_link(OBJECT(machine), OBJECT(s),
  "rtc_state", &error_abort);
 
-if (set_boot_dev(s, boot_device)) {
+set_boot_dev(s, boot_device, &local_err);
+if (local_err) {
+error_report("%s", error_get_pretty(local_err));
+error_free(local_err);
 exit(1);
 }
 
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 89aee71..ee1ed8a 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -116,10 +116,10 @@ static const MemoryRegionOps unin_ops = {
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static int fw_cfg_boot_set(void *opaque, const char *boot_device)
+static void fw_cfg_boot_set(void *opaque, const char *boot_device,
+Error **errp)
 {
 fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
-return 0;
 }
 
 static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 32c21a4..15109c2 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -49,13 +49,12 @@
 #define CLOCKFREQ 26600UL
 #define BUSFREQ 6600UL
 
-static int fw_cfg_boot_set(void *opaque, const char *boot_device)
+static void fw_cfg_boot_set(void *opaque, const char *boot_device,
+Error **errp)
 {
 fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
-return 0;
 }
 
-
 static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
 {
 return (addr & 0x0fff) + KERNEL_LOAD_ADDR;
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 8273199..df259ad 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -121,10 +121,10 @@ void DMA_register_channel (int nchan,
 {
 }
 
-static int fw_cfg_boot_set(void *opaque, const char *boot_device)
+static void fw_cfg_boot_set(void *opaque, const c

[Qemu-devel] [PATCH v2 4/5] bootdevice: add validate check for qemu_boot_set()

[arei.gonglei]

From: Gonglei 

Signed-off-by: Gonglei 
---
 bootdevice.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/bootdevice.c b/bootdevice.c
index 7f07507..9de34ba 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -49,12 +49,20 @@ void qemu_register_boot_set(QEMUBootSetHandler *func, void 
*opaque)
 
 void qemu_boot_set(const char *boot_order, Error **errp)
 {
+Error *local_err = NULL;
+
 if (!boot_set_handler) {
 error_setg(errp, "no function defined to set boot device list for"
  " this architecture");
 return;
 }
 
+validate_bootdevices(boot_order, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+
 if (boot_set_handler(boot_set_opaque, boot_order)) {
 error_setg(errp, "setting boot device list failed");
 return;
-- 
1.7.12.4

[Qemu-devel] [PATCH v2 2/5] bootdevice: add Error **errp argument for validate_bootdevices()

[arei.gonglei]

From: Gonglei 

We can use it for checking when we change traditional
boot order dynamically and propagate error message
to the monitor.

Signed-off-by: Gonglei 
---
 bootdevice.c| 10 +-
 include/sysemu/sysemu.h |  2 +-
 vl.c| 15 +--
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/bootdevice.c b/bootdevice.c
index aae4cac..184348e 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -55,7 +55,7 @@ int qemu_boot_set(const char *boot_order)
 return boot_set_handler(boot_set_opaque, boot_order);
 }
 
-void validate_bootdevices(const char *devices)
+void validate_bootdevices(const char *devices, Error **errp)
 {
 /* We just do some generic consistency checks */
 const char *p;
@@ -72,12 +72,12 @@ void validate_bootdevices(const char *devices)
  * features.
  */
 if (*p < 'a' || *p > 'p') {
-fprintf(stderr, "Invalid boot device '%c'\n", *p);
-exit(1);
+error_setg(errp, "Invalid boot device '%c'", *p);
+return;
 }
 if (bitmap & (1 << (*p - 'a'))) {
-fprintf(stderr, "Boot device '%c' was given twice\n", *p);
-exit(1);
+error_setg(errp, "Boot device '%c' was given twice", *p);
+return;
 }
 bitmap |= 1 << (*p - 'a');
 }
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 84798ef..1382d63 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -217,7 +217,7 @@ void device_add_bootindex_property(Object *obj, int32_t 
*bootindex,
const char *name, const char *suffix,
DeviceState *dev, Error **errp);
 void restore_boot_order(void *opaque);
-void validate_bootdevices(const char *devices);
+void validate_bootdevices(const char *devices, Error **errp);
 
 /* handler to set the boot_device order for a specific type of QEMUMachine */
 /* return 0 if success */
diff --git a/vl.c b/vl.c
index f665621..f0cdb63 100644
--- a/vl.c
+++ b/vl.c
@@ -4087,16 +4087,27 @@ int main(int argc, char **argv, char **envp)
 if (opts) {
 char *normal_boot_order;
 const char *order, *once;
+Error *local_err = NULL;
 
 order = qemu_opt_get(opts, "order");
 if (order) {
-validate_bootdevices(order);
+validate_bootdevices(order, &local_err);
+if (local_err) {
+error_report("%s", error_get_pretty(local_err));
+error_free(local_err);
+exit(1);
+}
 boot_order = order;
 }
 
 once = qemu_opt_get(opts, "once");
 if (once) {
-validate_bootdevices(once);
+validate_bootdevices(once, &local_err);
+if (local_err) {
+error_report("%s", error_get_pretty(local_err));
+error_free(local_err);
+exit(1);
+}
 normal_boot_order = g_strdup(boot_order);
 boot_order = once;
 qemu_register_reset(restore_boot_order, normal_boot_order);
-- 
1.7.12.4

[Qemu-devel] [PATCH v2 3/5] bootdevice: add Error **errp argument for qemu_boot_set()

[arei.gonglei]

From: Gonglei 

We can use it for checking when we change traditional
boot order dynamically and propagate error message
to the monitor.

Signed-off-by: Gonglei 
---
 bootdevice.c| 14 ++
 include/sysemu/sysemu.h |  2 +-
 monitor.c   | 14 ++
 3 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/bootdevice.c b/bootdevice.c
index 184348e..7f07507 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -47,12 +47,18 @@ void qemu_register_boot_set(QEMUBootSetHandler *func, void 
*opaque)
 boot_set_opaque = opaque;
 }
 
-int qemu_boot_set(const char *boot_order)
+void qemu_boot_set(const char *boot_order, Error **errp)
 {
 if (!boot_set_handler) {
-return -EINVAL;
+error_setg(errp, "no function defined to set boot device list for"
+ " this architecture");
+return;
+}
+
+if (boot_set_handler(boot_set_opaque, boot_order)) {
+error_setg(errp, "setting boot device list failed");
+return;
 }
-return boot_set_handler(boot_set_opaque, boot_order);
 }
 
 void validate_bootdevices(const char *devices, Error **errp)
@@ -94,7 +100,7 @@ void restore_boot_order(void *opaque)
 return;
 }
 
-qemu_boot_set(normal_boot_order);
+qemu_boot_set(normal_boot_order, NULL);
 
 qemu_unregister_reset(restore_boot_order, normal_boot_order);
 g_free(normal_boot_order);
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 1382d63..ee7fee7 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -223,7 +223,7 @@ void validate_bootdevices(const char *devices, Error 
**errp);
 /* return 0 if success */
 typedef int QEMUBootSetHandler(void *opaque, const char *boot_order);
 void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque);
-int qemu_boot_set(const char *boot_order);
+void qemu_boot_set(const char *boot_order, Error **errp);
 
 QemuOpts *qemu_get_machine_opts(void);
 
diff --git a/monitor.c b/monitor.c
index b37ddda..62e21c5 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1489,17 +1489,15 @@ static void do_ioport_write(Monitor *mon, const QDict 
*qdict)
 
 static void do_boot_set(Monitor *mon, const QDict *qdict)
 {
-int res;
+Error *local_err = NULL;
 const char *bootdevice = qdict_get_str(qdict, "bootdevice");
 
-res = qemu_boot_set(bootdevice);
-if (res == 0) {
-monitor_printf(mon, "boot device list now set to %s\n", bootdevice);
-} else if (res > 0) {
-monitor_printf(mon, "setting boot device list failed\n");
+qemu_boot_set(bootdevice, &local_err);
+if (local_err) {
+monitor_printf(mon, "%s\n", error_get_pretty(local_err));
+error_free(local_err);
 } else {
-monitor_printf(mon, "no function defined to set boot device list for "
-   "this architecture\n");
+monitor_printf(mon, "boot device list now set to %s\n", bootdevice);
 }
 }
 
-- 
1.7.12.4

[Qemu-devel] [PATCH v2 0/5] bootdevice: Refactor and improvement

[arei.gonglei]

From: Gonglei 

Changes of v2:
 Thanks to Peter's report and suggestion.
 - fix 'make check' complaint.
 - fix inapposite using &error_abort in patch 5.

Because of Peter's review comments in a pull request, I start a new
round review about this patch series v2. Please review, Thanks.

Patch 1 just move boot order related code to bootdevice.c.
Patch 2,3,5 add an argument to corresponding functions.
This way, we can propagate the error messages to the caller.
Maybe somebody will say we will remove the legacy boot order
in the future, instead of using bootindex. But at present,
for PPC, the have no way support bootindex, ARM on the flight
(Laszlo Ersek) as far as know.

After this work, we can easily to add QMP command for existing
HMP command 'boot_set' if we have a requirement.


Gonglei (5):
  bootdevice: move code about bootorder from vl.c to bootdevice.c
  bootdevice: add Error **errp argument for validate_bootdevices()
  bootdevice: add Error **errp argument for qemu_boot_set()
  bootdevice: add validate check for qemu_boot_set()
  bootdevice: add Error **errp argument for QEMUBootSetHandler

 bootdevice.c| 73 ++
 hw/i386/pc.c| 23 ---
 hw/ppc/mac_newworld.c   |  4 +--
 hw/ppc/mac_oldworld.c   |  5 ++--
 hw/sparc/sun4m.c|  4 +--
 hw/sparc64/sun4u.c  |  4 +--
 include/hw/hw.h |  6 
 include/sysemu/sysemu.h |  8 +
 monitor.c   | 14 -
 vl.c| 77 +
 10 files changed, 121 insertions(+), 97 deletions(-)

-- 
1.7.12.4

[Qemu-devel] [PATCH v2 1/5] bootdevice: move code about bootorder from vl.c to bootdevice.c

[arei.gonglei]

From: Gonglei 

First, we can downsize vl.c, make it simpler by
little and little. Second, I can maintain those code
and make some improvement.

Cc: Jan Kiszka 
Signed-off-by: Gonglei 
---
 bootdevice.c| 62 +
 include/hw/hw.h |  6 -
 include/sysemu/sysemu.h |  8 +++
 vl.c| 62 -
 4 files changed, 70 insertions(+), 68 deletions(-)

diff --git a/bootdevice.c b/bootdevice.c
index b29970c..aae4cac 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -25,6 +25,7 @@
 #include "sysemu/sysemu.h"
 #include "qapi/visitor.h"
 #include "qemu/error-report.h"
+#include "hw/hw.h"
 
 typedef struct FWBootEntry FWBootEntry;
 
@@ -37,6 +38,67 @@ struct FWBootEntry {
 
 static QTAILQ_HEAD(, FWBootEntry) fw_boot_order =
 QTAILQ_HEAD_INITIALIZER(fw_boot_order);
+static QEMUBootSetHandler *boot_set_handler;
+static void *boot_set_opaque;
+
+void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque)
+{
+boot_set_handler = func;
+boot_set_opaque = opaque;
+}
+
+int qemu_boot_set(const char *boot_order)
+{
+if (!boot_set_handler) {
+return -EINVAL;
+}
+return boot_set_handler(boot_set_opaque, boot_order);
+}
+
+void validate_bootdevices(const char *devices)
+{
+/* We just do some generic consistency checks */
+const char *p;
+int bitmap = 0;
+
+for (p = devices; *p != '\0'; p++) {
+/* Allowed boot devices are:
+ * a-b: floppy disk drives
+ * c-f: IDE disk drives
+ * g-m: machine implementation dependent drives
+ * n-p: network devices
+ * It's up to each machine implementation to check if the given boot
+ * devices match the actual hardware implementation and firmware
+ * features.
+ */
+if (*p < 'a' || *p > 'p') {
+fprintf(stderr, "Invalid boot device '%c'\n", *p);
+exit(1);
+}
+if (bitmap & (1 << (*p - 'a'))) {
+fprintf(stderr, "Boot device '%c' was given twice\n", *p);
+exit(1);
+}
+bitmap |= 1 << (*p - 'a');
+}
+}
+
+void restore_boot_order(void *opaque)
+{
+char *normal_boot_order = opaque;
+static int first = 1;
+
+/* Restore boot order and remove ourselves after the first boot */
+if (first) {
+first = 0;
+return;
+}
+
+qemu_boot_set(normal_boot_order);
+
+qemu_unregister_reset(restore_boot_order, normal_boot_order);
+g_free(normal_boot_order);
+}
 
 void check_boot_index(int32_t bootindex, Error **errp)
 {
diff --git a/include/hw/hw.h b/include/hw/hw.h
index 33bdb92..c78adae 100644
--- a/include/hw/hw.h
+++ b/include/hw/hw.h
@@ -41,12 +41,6 @@ typedef void QEMUResetHandler(void *opaque);
 void qemu_register_reset(QEMUResetHandler *func, void *opaque);
 void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
 
-/* handler to set the boot_device order for a specific type of QEMUMachine */
-/* return 0 if success */
-typedef int QEMUBootSetHandler(void *opaque, const char *boot_order);
-void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque);
-int qemu_boot_set(const char *boot_order);
-
 #ifdef NEED_CPU_H
 #if TARGET_LONG_BITS == 64
 #define VMSTATE_UINTTL_V(_f, _s, _v)  \
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 9fea3bc..84798ef 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -216,6 +216,14 @@ void del_boot_device_path(DeviceState *dev, const char 
*suffix);
 void device_add_bootindex_property(Object *obj, int32_t *bootindex,
const char *name, const char *suffix,
DeviceState *dev, Error **errp);
+void restore_boot_order(void *opaque);
+void validate_bootdevices(const char *devices);
+
+/* handler to set the boot_device order for a specific type of QEMUMachine */
+/* return 0 if success */
+typedef int QEMUBootSetHandler(void *opaque, const char *boot_order);
+void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque);
+int qemu_boot_set(const char *boot_order);
 
 QemuOpts *qemu_get_machine_opts(void);
 
diff --git a/vl.c b/vl.c
index 113e98e..f665621 100644
--- a/vl.c
+++ b/vl.c
@@ -196,9 +196,6 @@ NodeInfo numa_info[MAX_NODES];
 uint8_t qemu_uuid[16];
 bool qemu_uuid_set;
 
-static QEMUBootSetHandler *boot_set_handler;
-static void *boot_set_opaque;
-
 static NotifierList exit_notifiers =
 NOTIFIER_LIST_INITIALIZER(exit_notifiers);
 
@@ -1198,65 +1195,6 @@ static void default_drive(int enable, int snapshot, 
BlockInterfaceType type,
 
 }
 
-void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque)
-{
-boot_set_handler = func;
-boot_set_opaque = opaque;
-}
-
-int qemu_boot_set(const char *boot_order)
-{
-if (!boot_set_handler) {
-return -EINVAL;
-}
-return boot_set_handler(boot_set_opaque, boot_order)

Re: [Qemu-devel] [Qemu-ppc] Unable to loadvm on qemu-system-ppc -M g3beige (keyboard freeze)

[Alexander Graf]

On 17.12.14 11:47, Mark Cave-Ayland wrote:
> On 17/12/14 10:23, Alexander Graf wrote:
> 
>>> After the restart the keyboard is frozen and unresponsive. Some
>>> preliminary poking with gdb before and after seems to indicate that the
>>> cuda_receive_packet() functions aren't being called after the restart
>>> indicating that likely some state is missing from the savevm image and
>>> not being restored correctly. Anyone have any ideas as to what might be
>>> going wrong?
>>
>> I don't think anyone has test the migration code on non-papr ppc for
>> quite a while. In fact, I'm surprised it's only the keyboard not working :).
>>
>> You're more than invited to send patches to fix it! :)
> 
> How did I know that would be the response ;) I don't suppose anyone has
> a vague memory of the timeframe/QEMU version when a savevm/loadvm was
> last working for -M g3beige as a starting point?!

Phew - 0.7.0 maybe? If you say that only CUDA is broken, I don't think
it'd be too hard to fix :). Check that there are VMSTATEs for everything
and maybe retrigger interrupts after migration.


Alex

Re: [Qemu-devel] [RFC PATCH] target-arm: protect cpu_exclusive_*.

[Mark Burton]

> On 17 Dec 2014, at 11:45, Alexander Graf  wrote:
> 
> 
> 
> On 17.12.14 11:31, Mark Burton wrote:
>> 
>>> On 17 Dec 2014, at 11:28, Alexander Graf  wrote:
>>> 
>>> 
>>> 
>>> On 17.12.14 11:27, Frederic Konrad wrote:
 On 16/12/2014 17:37, Peter Maydell wrote:
> On 16 December 2014 at 09:13,   wrote:
>> From: KONRAD Frederic 
>> 
>> This adds a lock to avoid multiple exclusive access at the same time
>> in case of
>> TCG multithread.
 Hi Peter,
 
> This feels to me like it's not really possible to review on
> its own, since you can't see how it fits into the design of
> the rest of the multithreading support.
 true the only thing we observe is that it didn't change anything right now.
 
> The other approach here rather than having a pile of mutexes
> in the target-* code would be to have TCG IR support for
> "begin critical section"/"end critical section". Then you
> could have the main loop ensure that no other CPU is running
> at the same time as the critical-section code. (linux-user
> already has an ad-hoc implementation of this for the
> exclusives.)
> 
> -- PMM
> 
 What do you mean by TCG IR?
>>> 
>>> TCP ops. The nice thing is that TCG could translate those into
>>> transactions if the host supports them as well.
>>> 
>> 
>> Hows that different in reality from what we have now?
>> Cheers
>> Mark.
> 
> The current code can't optimize things in TCG. There's a good chance
> your TCG host implementation can have an optimization pass that creates
> host cmpxchg instructions or maybe even transaction blocks out of the
> critical sections.
> 
> 


Ok - I get it - I see the value - so long as it’s possible to do. It would 
solve a lot of problems...

We were not (yet) trying to fix that, we were simply asking the question, if we 
add these mutex’s - do we have any detrimental impact on anything.
Seems like the answer is that adding the mutex’s is fine - it doesn’t seem to 
have a performance impact or anything. Good.

But - I see what you mean - if we implemented this as an op, then it would be 
much simpler to optimise/fix properly afterwards - and - that “fix” might not 
even need to deal with the whole memory chain issue - maybe….. 

Cheers

Mark.

Re: [Qemu-devel] [RFC PATCH] target-arm: protect cpu_exclusive_*.

[Alexander Graf]

On 17.12.14 12:12, Mark Burton wrote:
> 
>> On 17 Dec 2014, at 11:45, Alexander Graf  wrote:
>>
>>
>>
>> On 17.12.14 11:31, Mark Burton wrote:
>>>
 On 17 Dec 2014, at 11:28, Alexander Graf  wrote:



 On 17.12.14 11:27, Frederic Konrad wrote:
> On 16/12/2014 17:37, Peter Maydell wrote:
>> On 16 December 2014 at 09:13,   wrote:
>>> From: KONRAD Frederic 
>>>
>>> This adds a lock to avoid multiple exclusive access at the same time
>>> in case of
>>> TCG multithread.
> Hi Peter,
>
>> This feels to me like it's not really possible to review on
>> its own, since you can't see how it fits into the design of
>> the rest of the multithreading support.
> true the only thing we observe is that it didn't change anything right 
> now.
>
>> The other approach here rather than having a pile of mutexes
>> in the target-* code would be to have TCG IR support for
>> "begin critical section"/"end critical section". Then you
>> could have the main loop ensure that no other CPU is running
>> at the same time as the critical-section code. (linux-user
>> already has an ad-hoc implementation of this for the
>> exclusives.)
>>
>> -- PMM
>>
> What do you mean by TCG IR?

 TCP ops. The nice thing is that TCG could translate those into
 transactions if the host supports them as well.

>>>
>>> Hows that different in reality from what we have now?
>>> Cheers
>>> Mark.
>>
>> The current code can't optimize things in TCG. There's a good chance
>> your TCG host implementation can have an optimization pass that creates
>> host cmpxchg instructions or maybe even transaction blocks out of the
>> critical sections.
>>
>>
> 
> 
> Ok - I get it - I see the value - so long as it’s possible to do. It would 
> solve a lot of problems...
> 
> We were not (yet) trying to fix that, we were simply asking the question, if 
> we add these mutex’s - do we have any detrimental impact on anything.
> Seems like the answer is that adding the mutex’s is fine - it doesn’t seem to 
> have a performance impact or anything. Good.
> 
> But - I see what you mean - if we implemented this as an op, then it would be 
> much simpler to optimise/fix properly afterwards - and - that “fix” might not 
> even need to deal with the whole memory chain issue - maybe….. 

Yes, especially given that transactional memory is getting pretty common
these days (Haswell had it, POWER8 has it) I think it makes a lot of
sense to just base on its concept in the design here. It's the easiest
way to make parallel memory accesses fast without taking big locks all
the time.

So I think the best way to go forward would be to add transaction_start
and transaction_end opcodes to TCG and implement them as mutex locks
today. When you get the chance to get yourself a machine that supports
actual TM, try to replace them with transaction start/end blocks and
have the normal mutex code as fallback if the transaction fails.


Alex

Re: [Qemu-devel] [RFC PATCH] target-arm: protect cpu_exclusive_*.

[Peter Maydell]

On 17 December 2014 at 11:12, Mark Burton  wrote:
> We were not (yet) trying to fix that, we were simply asking the
> question, if we add these mutex’s - do we have any detrimental impact
> on anything.
> Seems like the answer is that adding the mutex’s is fine - it doesn’t
> seem to have a performance impact or anything. Good.

Personally I dislike the mutex approach because it means that every
target ends up with its own ad-hoc implementation of something that
would be better implemented in the QEMU core support (even if that
core support is simply taking mutexes in the end).

I also note that the linux-user code for handling exclusives does it
by having a region (effectively) where all other CPUs stop executing
*anything*. Do we need that?

PS: you'll find that we already have some ancient and half-broken
support code for the mutex approach (search for 'spinlock_t')...

-- PMM

Re: [Qemu-devel] [Qemu-ppc] Unable to loadvm on qemu-system-ppc -M g3beige (keyboard freeze)

[Peter Maydell]

On 17 December 2014 at 11:11, Alexander Graf  wrote:
> Phew - 0.7.0 maybe? If you say that only CUDA is broken, I don't think
> it'd be too hard to fix :). Check that there are VMSTATEs for everything
> and maybe retrigger interrupts after migration.

It shouldn't be necessary to retrigger anything post-migration:
the devices on both ends should both believe the interrupt is
asserted once they've loaded their state.

Given how long it's been since this was known-working, it's
probably easiest just to compare the VMState structs against
the device structs for every device that might be vaguely
relevant, looking for missing fields.

-- PMM

Re: [Qemu-devel] [RFC PATCH] target-arm: protect cpu_exclusive_*.

[Paolo Bonzini]

On 17/12/2014 12:18, Alexander Graf wrote:
> 
> So I think the best way to go forward would be to add transaction_start
> and transaction_end opcodes to TCG and implement them as mutex locks
> today. When you get the chance to get yourself a machine that supports
> actual TM, try to replace them with transaction start/end blocks and
> have the normal mutex code as fallback if the transaction fails.

Or implement load_locked/store_conditional TCG ops.  They can be
implemented as transactions, hardware ll/sc, or something slow that uses
the MMU.

Paolo

Re: [Qemu-devel] [RFC PATCH] target-arm: protect cpu_exclusive_*.

[Peter Maydell]

On 17 December 2014 at 11:25, Paolo Bonzini  wrote:
>
>
> On 17/12/2014 12:18, Alexander Graf wrote:
>>
>> So I think the best way to go forward would be to add transaction_start
>> and transaction_end opcodes to TCG and implement them as mutex locks
>> today. When you get the chance to get yourself a machine that supports
>> actual TM, try to replace them with transaction start/end blocks and
>> have the normal mutex code as fallback if the transaction fails.
>
> Or implement load_locked/store_conditional TCG ops.  They can be
> implemented as transactions, hardware ll/sc, or something slow that uses
> the MMU.

You'd need to compare the semantics of ll/sc across the various
architectures to find out if there was anything you could
actually meaningfully define as useful TCG op semantics there.

-- PMM

Re: [Qemu-devel] the need for if=none for -drive?

[Markus Armbruster]

Sorry for the slow reply, I missed this one until now.  Adding block
maintainers.

John Snow  writes:

> On 11/02/2014 02:21 AM, Michael Tokarev wrote:
>> All "modern" 2-way drive/device specifications need to explicitly
>> specify if=none for the drive for it to not be used in the default
>> ide bus implicitly.
>
> Which behave like this? I am reasonably sheltered in the IDE and S/ATA
> lands.

-drive's parameter "if" defaults to the board's preferred interface.
if=none was added to shoehorn block devices into the -device world.  As
Michael wrote, you always have to say if=none,id=FOO,... to create
something usable with -device.

>> But how about using some if=unspecified implicitly for all devices
>> which don't specify if=, pick any devices from that list which are
>> referenced from -device, and only add those which left (plus the
>> ones with explicit if=ide) to the default ide bus?
>
> I wouldn't really be opposed to this, since making things more
> explicit and less mysterious with the drive sugar sounds welcome to
> me. I imagine you're trying to avoid the case where if= is omitted and
> QEMU gets to decide what it wants to do in this (highly ambiguous)
> case?
>
>> The change should be strighforward, the only (maybe significant)
>> prob I see is a need to reorder -device/-drive initialization in
>> vl.c.  We might pick them up in drive_check_orphaned() too.  Or,
>> we can walk over -devices when adding -drives with if={unspec,explicit}.
>>
>> (this is a sort of a followup to 6b9e03a4e759876, "qtest/bios-tables:
>> Correct Q35 command line", but ofcourse it's a topic by its own).
>>
>> Thanks,
>>
>> /mjt
>>
>
> It may be a little too late, since it seemed as if defaulting to
> "IF_DEFAULT" always winds up getting mapped to whatever is provided by
> the second argument of drive_new (block_default_type) which usually
> winds up being machine_class->block_default_type -- which for Q35 and
> piix both is IDE. (Implicitly, because it never sets it. interface 0
> is IF_IDE -- which makes the "true default" for if ... IDE, until it
> is overridden.)
>
> Changing this behavior might break more than we fix, perhaps.
>
> I'll leave the masterminding of fixing up the grossly broken drive
> sugar up to Markus, the secret architect of the recent Q35 sugar
> patches :>

IF_DEFAULT is currently used only for the non-option image argument,
-hd[a-d] and -cdrom.  It tells the desugaring function drive_add() not
add an "if" parameter.

Parameter "if" is processed by drive_new().  It defaults to argument
block_default_type.  For -drive, it's the current machine's
block_default_type.  Some machines set IF_SCSI, and two set IF_VIRTIO,
the rest get IF_IDE via zero initialization.

Board initialization code iterates over drives they know, and create
appropriate device models.  Boards never pick up if=none drives.

Anything not picked up by board initialization can be used by -device.
Ideally, these are just the if=none drives, but confusing crap like
"-drive id=foo,if=mtd,file=tmp.qcow2 -device usb-storage,drive=foo" also
works.

Anything not used by -device either triggers an "orphaned drive"
warning, and stays around for possible use by device_add.

Michael proposes to reshuffle this a bit.  Instead of resolving missing
"if" to the machine's block_default_type in drive_new(), keep it
undecided a bit longer.  Give -device first pick.  Anything left over
defaults to the machine's block_default_type as before.

Two remarks.

First, I'm afraid giving -device first pick isn't quite trivial.  The
current code acting on -device creates and connects a device model.
Requires the board to be initialized already.  Either we delay the board
iterating over drives until after -device is done.  Changes failure
modes, need to make sure the error reporting doesn't degrade.  Or we do
an early pass to claim the drives for -device, and actually connect them
later.

Second, while I too find the need for if=none annoying, I'm not sure I
like the amount of extra magic.  Could be too much.  Opinions?

Re: [Qemu-devel] [Qemu-ppc] [PATCH] ppc: do not use get_clock_realtime()

[Alexander Graf]

On 26.11.14 15:01, Paolo Bonzini wrote:
> Use the external qemu-timer API instead.
> 
> Cc: qemu-...@nongnu.org
> Signed-off-by: Paolo Bonzini 

Thanks, applied to ppc-next.


Alex

[Qemu-devel] [PATCH] Force pread64/pwrite64 to return 0 for zero-length buffer

[Ilya Palachev]

According to official standard POSIX.1-2001. pread64 and pwrite64
should return 0 for zero-length buffers as mentioned at

http://pubs.opengroup.org/onlinepubs/009695399/functions/read.html
http://pubs.opengroup.org/onlinepubs/009695399/functions/write.html

Change-Id: Icd66ea29658329fbd5e6461d1def0c78c81d2671
Signed-off-by: Ilya Palachev 
---
 linux-user/syscall.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index a41dd43..a08f5ef 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8127,6 +8127,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_ulong 
arg1,
 arg4 = arg5;
 arg5 = arg6;
 }
+if (!arg3) {
+ret = 0;
+break;
+}
 if (!(p = lock_user(VERIFY_WRITE, arg2, arg3, 0)))
 goto efault;
 ret = get_errno(pread64(arg1, p, arg3, target_offset64(arg4, arg5)));
@@ -8137,6 +8141,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_ulong 
arg1,
 arg4 = arg5;
 arg5 = arg6;
 }
+if (!arg3) {
+ret = 0;
+break;
+}
 if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1)))
 goto efault;
 ret = get_errno(pwrite64(arg1, p, arg3, target_offset64(arg4, arg5)));
-- 
2.1.3

Re: [Qemu-devel] [PATCH] Fix crash on spapr_tce_table_finalize()

[Alexander Graf]

On 08.12.14 03:48, David Gibson wrote:
> spapr_tce_table_finalize() can SEGV if the object was not previously
> realized.  In particular this can be triggered by running
>  qemu-system-ppc -device spapr-tce-table,?
> 
> The basic problem is that we have mismatched initialization versus
> finalization: spapr_tce_table_finalize() is attempting to undo things that
> are done in spapr_tce_table_realize(), not an instance_init function.
> 
> Therefore, replace spapr_tce_table_finalize() with
> spapr_tce_table_unrealize().
> 
> Signed-off-by: David Gibson 

Thanks, applied to ppc-next and added CC stable.


Alex

Re: [Qemu-devel] [PATCH v9 02/10] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove

[John Snow]

On 12/15/2014 03:50 AM, Markus Armbruster wrote:

Stefan Hajnoczi  writes:


On Wed, Dec 10, 2014 at 01:43:47PM +0100, Markus Armbruster wrote:

Stefan Hajnoczi  writes:


On Mon, Dec 01, 2014 at 03:30:08PM -0500, John Snow wrote:

diff --git a/block.c b/block.c
index e5c6ccf..3f27519 100644
--- a/block.c
+++ b/block.c
@@ -5420,6 +5420,25 @@ int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap 
*bitmap, int64_t sector
  }
  }

+#define BDB_MIN_DEF_GRANULARITY 4096
+#define BDB_MAX_DEF_GRANULARITY 65536
+#define BDB_DEFAULT_GRANULARITY BDB_MAX_DEF_GRANULARITY
+
+uint64_t bdrv_dbm_calc_def_granularity(BlockDriverState *bs)


Long names are unwieldy but introducing multiple abbreviations is not a
good solution, it makes the code more confusing (BDB vs dbm).

I would call the function bdrv_get_default_bitmap_granularity().

The constants weren't necessary since the point of this function is to
capture the default value.  No one else should use the constants -
otherwise there is a high probability that they are reimplementing this
logic.  I would just leave them as literals in the code.


diff --git a/blockdev.c b/blockdev.c
index 5651a8e..4d30b09 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1894,6 +1894,60 @@ void qmp_block_set_io_throttle(const char *device, 
int64_t bps, int64_t bps_rd,
  aio_context_release(aio_context);
  }

+void qmp_block_dirty_bitmap_add(const char *device, const char *name,
+bool has_granularity, int64_t granularity,
+Error **errp)
+{
+BlockDriverState *bs;
+
+bs = bdrv_lookup_bs(device, NULL, errp);


Markus: I think we need to support node-name here so dirty bitmaps can
be applied at any node in the graph?


We need to consider node names for all new QMP commands.  Whenever we
add a backend name parameter, like we do for the two new commands in
this patch, we need to decide whether nodes make sense, too.  Do they?

I'm afraid we haven't quite made up our mind what to do when nodes make
sense.

Existing patterns of backend / node name parameters:

1. Backend name only

Parameter is commonly called @device.  Examples: eject,
block_set_io_throttle.

Code uses blk_by_name() or bdrv_find().  The latter needs to be
converted to the former.

2. Backend or node name

2a. Two optional parameters, commonly called @device and @node-name,
of which exactly one must be given.  Example: block_passwd.

Code uses

 bs = bdrv_lookup_bs(has_device ? device : NULL,
 has_node_name ? node_name : NULL,
 &local_err);

which is a roundabout way to say

 bs = bdrv_lookup_bs(device, node_name, &local_err);

2b. Single parameter.  The single example is anonymous union
BlockdevRef.

Code uses

 bs = bdrv_lookup_bs(reference, reference, errp);

If we want to adopt "single parameter" going forward, we need to
decide on a naming convention.  Existing commands are probably stuck
with @device for compatibility.  Do we care for names enough to
improve on that?

A convenience wrapper around bdrv_lookup_bs() to avoid stuttering
name argument could make sense.


Initially only the backend needs dirty bitmap support (this satisfies
the incremental backup use case).

It is possible that future use cases will require node-name.

I'm happy with just allowing the device parameter today and adding the
node-name parameter if needed later.

This conservative approach seems good because IMO we shouldn't add
unused features to the API since they need to be tested and maintained
forever.

So maybe use a device argument with blk_by_name() for now.

In the future switch to bdrv_lookup_bs() with has_device/has_node_name.

If anyone strongly feels we should support node-name from day 1, I'm
okay with that too but there needs to be a test case which actually
exercises that code!


Agree with not adding unused features.

However, we should make up our minds how we want QMP to do backend and
node names in the future.  I see two basic options:

1. Inertia

Keep adding separate arguments for backend name (commonly called
@device) and node name (commonly called @node-name).  If the command
can take only one, make it mandatory.  If it can take either, make it
either/or.

Cements the inconsistency between single parameter in BlockdevRef and
the two either/or parameters elsewhere.

2. Clean up the mess

New commands take a single parameter.  The command accepts backends
or nodes as they make sense for the command.  Acceptable parameter
name needed.

Either existing commands get changed to single parameter (with the
necessary compatibility and discoverability gunk), or they get
replaced by new commands.

I'll analyze how the gunk could be done in a separate message,
hopefully soon.



OK, given the most recent email, it seems as if you would prefer to use 
"@devic

Re: [Qemu-devel] [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1

[Jeff Cody]

On Wed, Dec 17, 2014 at 05:46:32AM -0500, Lokesha, Amulya wrote:
> Hi Max, Jeff,
> 
> We were able to get the qemu patch files downloaded from the qemu patch site 
> -  https://patchwork.ozlabs.org and were able to apply the patches 
> successfully without any errors. With the patches applied, we recompiled the 
> qemu and converted the VDK vmdk to vhdx format and uploaded to the SCVMM 
> Server. But it failed again with the syntax error as below:
> 
> Information (10804)
> Unable to import \\TestServer\MSSCVMMLibrary\VHDs\Product-disk1.vhdx because 
> of a syntax error in the file.
> 
> Please find my comments inline for your questions
> 
> Please let us know if there is anything else you need from us.
> 
>

Amulya,

I will try to test this on Windows Server, and see if I can reproduce
what you are seeing.

-Jeff

> 
> 
> -Original Message-
> From: Jeff Cody [mailto:jc...@redhat.com]
> Sent: Friday, December 12, 2014 8:48 PM
> To: Lokesha, Amulya
> Cc: Max Reitz; qemu-devel@nongnu.org; kw...@redhat.com; stefa...@redhat.com
> Subject: Re: [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to 
> bdrv_has_zero_init_1
> 
> On Fri, Dec 12, 2014 at 09:43:16AM -0500, Lokesha, Amulya wrote:
> >Hi Max,
> > 
> > 
> 
> Please reply in-line, it makes it easier to follow technical discussions - 
> thanks :)
> 
> > 
> >We applied all the 5 patches from the mail chain I got since the last
> >week. Please find attached the patches used by us.
> > 
> >We were unable to apply the patch3 as it failed with the following 
> > error
> > 
> > 
> > 
> ># patch -p1 < patch3
> > 
> >patching file block/vhdx.c
> > 
> >patch:  malformed patch at line 17:  error_setg_errno(errp,
> >EINVAL, "Image size too large; max of 64TB"); @@ -1936,7 +1936,9 @@ 
> > static
> >QemuOptsList vhdx_create_opts = {
> > 
> >
> 
> It looks like however you saved the patch file, it was corrupted.
> Looking at your attached patch 3, it split line 9 across 2 lines.
> Your patch also has whitespace differences from the patch I sent.
> 
> You also attached 5 patches - Why are you using patch 0?  You should only be 
> applying patches 1-4.  This should not be causing any actual issues, however.
> [Amulya]: First time we applied patches 1 to 4, created VHDX image and 
> deployed to HyperV Server, but we got the same error. Then we took a fresh 
> qemu source and applied patches 0 to 4 and deployed to HyperV and again got 
> the same syntax error.
> 
> 
> Are you using git for your qemu version?  If so, 'git am' is the preferred 
> method of applying the patches - just save each of the patch emails (the 
> whole email should be fine), and run 'git am' on each file.
> 
> [Amulya] : No. We don't have a git repository for our team. Could you please 
> let us know how to apply these patches without git. What is the difference in 
> applying the patch directly and modifying the code directly? Does it have any 
> impact?
> 
> > 
> > 
> > 
> >Hence, we manually added the patch3 changes and recompiled the qemu. We
> >then used the patched qemu-img to convert  our vmdk image to dynamic VHDX
> >format. We found that the image created this time had a considerable
> >decrease in its size from 50GB to 12GB.
> > 
> 
> Could you tell me the file size of the VMDK image you were converting?
> Is it roughly 12GB as well?
> [Amulya] :  No, the vmdk image which we used for conversion is just 1.4GB
> 
> 
> >However, when we deployed it into our SCVMM 2012, the import of the VHDX
> >image failed with a "syntax error" as below
> > 
> > 
> > 
> >Information (10804)
> > 
> >Unable to import \\Test.com\Library\VHDs\Test-disk1.vhdx because of a
> >syntax error in the file.
> > 
> > 
> 
> If you run qemu-img info on Test-disk1.vhdx, what does it say?
> 
> [Amulya] :   The following is seen with qemu-img info
> # qemu-img info Test-disk1.vhdx
> image: Test-disk1.vhdx
> file format: vhdx
> virtual size: 50G (53687091200 bytes)
> disk size: 3.4G
> cluster_size: 16777216
> 
> The size of our images
> # ls -ltrh
> total 4.8G
> -rw-r--r-- 1 root root 1.4G Dec  9 10:48 Test-disk1.vmdk
> -rw-r--r-- 1 root root  12G Dec 12 04:49 Test-disk1.vhdx
> 
> > 
> >Please let us know if we missed anything.
> > 
> > 
> > 
> >Thanks,
> > 
> >Amulya
> > 
> > 
> >
> 
> [...]
> 
> Jeff

Re: [Qemu-devel] [PATCH] Force pread64/pwrite64 to return 0 for zero-length buffer

[Peter Maydell]

On 17 December 2014 at 12:02, Ilya Palachev  wrote:
> According to official standard POSIX.1-2001. pread64 and pwrite64
> should return 0 for zero-length buffers as mentioned at
>
> http://pubs.opengroup.org/onlinepubs/009695399/functions/read.html
> http://pubs.opengroup.org/onlinepubs/009695399/functions/write.html
>
> Change-Id: Icd66ea29658329fbd5e6461d1def0c78c81d2671
> Signed-off-by: Ilya Palachev 

If this is a problem, doesn't it apply to more syscalls than
just pread64 and pwrite64 ?

-- PMM

Re: [Qemu-devel] [PATCH v2 1/5] linux-aio: queue requests that cannot be submitted

[Paolo Bonzini]

On 16/12/2014 21:26, Paolo Bonzini wrote:
> 
>> I could reproduce this very well on a random OS image that I had around.
>>  This is raw over XFS over dm-crypt, and the image is about 75% sparse
>> (8.2G used over 35G).  I only get 1-2%, but still it's visible.
>>
>> However I can hardly reproduce it when using a partition directly:
>>
>>  oldnew
>> mean 9.9565 9.9636  (+0.07%)
>> stddev   0.0405 0.0537
>> min  9.871  9.867
>> median   9.973  9.971
>> max  10.01  10.053
>> count20 20
>>
>> I haven't tried removing layers (e.g. fully-allocated XFS image without
>> dm-crypt).
> 
> Could not reproduce it with a fully-allocated XFS image, on the contrary
> the patched QEMU is a bit faster:
> 
> old  new
> mean14.83325 14.82660
> stddev  0.016930 0.010328
> min 14.819   14.818
> max 14.854   14.883
> median  14.8225  14.8255
> count   20   20

Same for 50% sparse XFS image, patched QEMU a bit faster:

old new
Mean8.31285 8.30625
stddev  0.03690 0.0
min 8.277   8.276
max 8.378   8.382
median  8.292   8.2905
count   20  20

[Qemu-devel] [PATCH v5 0/4] qmp: Add "blockdev-backup"

[Fam Zheng]

v5: Address Max's and Markus' comments:
Split patch 1. (Markus)
Fix typos and pastos. (Markus, Max)
Actually acquire aio context. (Max)
Drop unnecessary initialization of fields in blockdev_backup_prepare. (Max)
Add "sync" in the document example.
Add Max's rev-by in patch 4.

The existing drive-backup command accepts a target file path, but that
interface provides little flexibility on the properties of target block device,
compared to what is possible with "blockdev-add", "drive_add" or "-drive".

This is also a building block to allow image fleecing (creating a point in time
snapshot and export with nbd-server-add).

(For symmetry, blockdev-mirror will be added in a separate series.)

Fam


Fam Zheng (4):
  qapi: Comment version info in TransactionAction
  qmp: Add command 'blockdev-backup'
  block: Add blockdev-backup to transaction
  qemu-iotests: Test blockdev-backup in 055

 block/backup.c |  28 ++
 blockdev.c | 133 
 qapi-schema.json   |   8 ++
 qapi/block-core.json   |  54 
 qmp-commands.hx|  42 +
 tests/qemu-iotests/055 | 211 +
 tests/qemu-iotests/055.out |   4 +-
 7 files changed, 441 insertions(+), 39 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH v5 1/4] qapi: Comment version info in TransactionAction

[Fam Zheng]

Signed-off-by: Fam Zheng 
---
 qapi-schema.json | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/qapi-schema.json b/qapi-schema.json
index 563b4ad..47d99cf 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1254,6 +1254,12 @@
 #
 # A discriminated record of operations that can be performed with
 # @transaction.
+#
+# Since 1.1
+#
+# drive-backup since 1.6
+# abort since 1.6
+# blockdev-snapshot-internal-sync since 1.7
 ##
 { 'union': 'TransactionAction',
   'data': {
-- 
1.9.3

[Qemu-devel] [PATCH v5 3/4] block: Add blockdev-backup to transaction

[Fam Zheng]

Signed-off-by: Fam Zheng 
---
 blockdev.c   | 79 
 qapi-schema.json |  2 ++
 2 files changed, 81 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index dbbab1d..9f9ae88 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1559,6 +1559,79 @@ static void drive_backup_clean(BlkTransactionState 
*common)
 }
 }
 
+typedef struct BlockdevBackupState {
+BlkTransactionState common;
+BlockDriverState *bs;
+BlockJob *job;
+AioContext *aio_context;
+} BlockdevBackupState;
+
+static void blockdev_backup_prepare(BlkTransactionState *common, Error **errp)
+{
+BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, 
common);
+BlockdevBackup *backup;
+BlockDriverState *bs, *target;
+Error *local_err = NULL;
+
+assert(common->action->kind == TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP);
+backup = common->action->blockdev_backup;
+
+bs = bdrv_find(backup->device);
+if (!bs) {
+error_set(errp, QERR_DEVICE_NOT_FOUND, backup->device);
+return;
+}
+
+target = bdrv_find(backup->target);
+if (!target) {
+error_set(errp, QERR_DEVICE_NOT_FOUND, backup->target);
+return;
+}
+
+/* AioContext is released in .clean() */
+state->aio_context = bdrv_get_aio_context(bs);
+if (state->aio_context != bdrv_get_aio_context(target)) {
+state->aio_context = NULL;
+error_setg(errp, "Backup between two IO threads is not implemented");
+return;
+}
+aio_context_acquire(state->aio_context);
+
+qmp_blockdev_backup(backup->device, backup->target,
+backup->sync,
+backup->has_speed, backup->speed,
+backup->has_on_source_error, backup->on_source_error,
+backup->has_on_target_error, backup->on_target_error,
+&local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+
+state->bs = bdrv_find(backup->device);
+state->job = state->bs->job;
+}
+
+static void blockdev_backup_abort(BlkTransactionState *common)
+{
+BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, 
common);
+BlockDriverState *bs = state->bs;
+
+/* Only cancel if it's the job we started */
+if (bs && bs->job && bs->job == state->job) {
+block_job_cancel_sync(bs->job);
+}
+}
+
+static void blockdev_backup_clean(BlkTransactionState *common)
+{
+BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, 
common);
+
+if (state->aio_context) {
+aio_context_release(state->aio_context);
+}
+}
+
 static void abort_prepare(BlkTransactionState *common, Error **errp)
 {
 error_setg(errp, "Transaction aborted using Abort action");
@@ -1582,6 +1655,12 @@ static const BdrvActionOps actions[] = {
 .abort = drive_backup_abort,
 .clean = drive_backup_clean,
 },
+[TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP] = {
+.instance_size = sizeof(BlockdevBackupState),
+.prepare = blockdev_backup_prepare,
+.abort = blockdev_backup_abort,
+.clean = blockdev_backup_clean,
+},
 [TRANSACTION_ACTION_KIND_ABORT] = {
 .instance_size = sizeof(BlkTransactionState),
 .prepare = abort_prepare,
diff --git a/qapi-schema.json b/qapi-schema.json
index 47d99cf..fbfc52f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1260,11 +1260,13 @@
 # drive-backup since 1.6
 # abort since 1.6
 # blockdev-snapshot-internal-sync since 1.7
+# blockdev-backup since 2.3
 ##
 { 'union': 'TransactionAction',
   'data': {
'blockdev-snapshot-sync': 'BlockdevSnapshot',
'drive-backup': 'DriveBackup',
+   'blockdev-backup': 'BlockdevBackup',
'abort': 'Abort',
'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal'
} }
-- 
1.9.3

[Qemu-devel] [PATCH v5 4/4] qemu-iotests: Test blockdev-backup in 055

[Fam Zheng]

This applies cases on drive-backup on blockdev-backup, except cases with
target format and mode.

Also add a case to check source == target.

Signed-off-by: Fam Zheng 
Reviewed-by: Max Reitz 
---
 tests/qemu-iotests/055 | 211 +
 tests/qemu-iotests/055.out |   4 +-
 2 files changed, 176 insertions(+), 39 deletions(-)

diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055
index 0872444..e81d4d0 100755
--- a/tests/qemu-iotests/055
+++ b/tests/qemu-iotests/055
@@ -1,8 +1,8 @@
 #!/usr/bin/env python
 #
-# Tests for drive-backup
+# Tests for drive-backup and blockdev-backup
 #
-# Copyright (C) 2013 Red Hat, Inc.
+# Copyright (C) 2013, 2014 Red Hat, Inc.
 #
 # Based on 041.
 #
@@ -27,6 +27,7 @@ from iotests import qemu_img, qemu_io
 
 test_img = os.path.join(iotests.test_dir, 'test.img')
 target_img = os.path.join(iotests.test_dir, 'target.img')
+blockdev_target_img = os.path.join(iotests.test_dir, 'blockdev-target.img')
 
 class TestSingleDrive(iotests.QMPTestCase):
 image_len = 64 * 1024 * 1024 # MB
@@ -38,34 +39,41 @@ class TestSingleDrive(iotests.QMPTestCase):
 qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xd5 1M 32k', test_img)
 qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 32M 124k', test_img)
 qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 67043328 64k', 
test_img)
+qemu_img('create', '-f', iotests.imgfmt, blockdev_target_img, 
str(TestSingleDrive.image_len))
 
-self.vm = iotests.VM().add_drive(test_img)
+self.vm = 
iotests.VM().add_drive(test_img).add_drive(blockdev_target_img)
 self.vm.launch()
 
 def tearDown(self):
 self.vm.shutdown()
 os.remove(test_img)
+os.remove(blockdev_target_img)
 try:
 os.remove(target_img)
 except OSError:
 pass
 
-def test_cancel(self):
+def do_test_cancel(self, cmd, target):
 self.assert_no_active_block_jobs()
 
-result = self.vm.qmp('drive-backup', device='drive0',
- target=target_img, sync='full')
+result = self.vm.qmp(cmd, device='drive0', target=target, sync='full')
 self.assert_qmp(result, 'return', {})
 
 event = self.cancel_and_wait()
 self.assert_qmp(event, 'data/type', 'backup')
 
-def test_pause(self):
+def test_cancel_drive_backup(self):
+self.do_test_cancel('drive-backup', target_img)
+
+def test_cancel_blockdev_backup(self):
+self.do_test_cancel('blockdev-backup', 'drive1')
+
+def do_test_pause(self, cmd, target, image):
 self.assert_no_active_block_jobs()
 
 self.vm.pause_drive('drive0')
-result = self.vm.qmp('drive-backup', device='drive0',
- target=target_img, sync='full')
+result = self.vm.qmp(cmd, device='drive0',
+ target=target, sync='full')
 self.assert_qmp(result, 'return', {})
 
 result = self.vm.qmp('block-job-pause', device='drive0')
@@ -86,14 +94,25 @@ class TestSingleDrive(iotests.QMPTestCase):
 self.wait_until_completed()
 
 self.vm.shutdown()
-self.assertTrue(iotests.compare_images(test_img, target_img),
+self.assertTrue(iotests.compare_images(test_img, image),
 'target image does not match source after backup')
 
+def test_pause_drive_backup(self):
+self.do_test_pause('drive-backup', target_img, target_img)
+
+def test_pause_blockdev_backup(self):
+self.do_test_pause('blockdev-backup', 'drive1', blockdev_target_img)
+
 def test_medium_not_found(self):
 result = self.vm.qmp('drive-backup', device='ide1-cd0',
  target=target_img, sync='full')
 self.assert_qmp(result, 'error/class', 'GenericError')
 
+def test_medium_not_found_blockdev_backup(self):
+result = self.vm.qmp('blockdev-backup', device='ide1-cd0',
+ target='drive1', sync='full')
+self.assert_qmp(result, 'error/class', 'GenericError')
+
 def test_image_not_found(self):
 result = self.vm.qmp('drive-backup', device='drive0',
  target=target_img, sync='full', mode='existing')
@@ -105,31 +124,53 @@ class TestSingleDrive(iotests.QMPTestCase):
  format='spaghetti-noodles')
 self.assert_qmp(result, 'error/class', 'GenericError')
 
-def test_device_not_found(self):
-result = self.vm.qmp('drive-backup', device='nonexistent',
- target=target_img, sync='full')
+def do_test_device_not_found(self, cmd, **args):
+result = self.vm.qmp(cmd, **args)
 self.assert_qmp(result, 'error/class', 'DeviceNotFound')
 
+def test_device_not_found(self):
+self.do_test_device_not_found('drive-backup', device='nonexistent',
+  target=target_img, sync='full')
+
+   

[Qemu-devel] [PATCH v5 2/4] qmp: Add command 'blockdev-backup'

[Fam Zheng]

Similar to drive-backup, but this command uses a device id as target
instead of creating/opening an image file.

Also add blocker on target bs, since the target is also a named device
now.

Add check and report error for bs == target which became possible but is
an illegal case with introduction of blockdev-backup.

Signed-off-by: Fam Zheng 
---
 block/backup.c   | 28 +++
 blockdev.c   | 54 
 qapi/block-core.json | 54 
 qmp-commands.hx  | 42 
 4 files changed, 178 insertions(+)

diff --git a/block/backup.c b/block/backup.c
index 792e655..1c535b1 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -360,6 +360,7 @@ static void coroutine_fn backup_run(void *opaque)
 hbitmap_free(job->bitmap);
 
 bdrv_iostatus_disable(target);
+bdrv_op_unblock_all(target, job->common.blocker);
 
 data = g_malloc(sizeof(*data));
 data->ret = ret;
@@ -379,6 +380,11 @@ void backup_start(BlockDriverState *bs, BlockDriverState 
*target,
 assert(target);
 assert(cb);
 
+if (bs == target) {
+error_setg(errp, "Source and target cannot be the same");
+return;
+}
+
 if ((on_source_error == BLOCKDEV_ON_ERROR_STOP ||
  on_source_error == BLOCKDEV_ON_ERROR_ENOSPC) &&
 !bdrv_iostatus_is_enabled(bs)) {
@@ -386,6 +392,26 @@ void backup_start(BlockDriverState *bs, BlockDriverState 
*target,
 return;
 }
 
+if (!bdrv_is_inserted(bs)) {
+error_setg(errp, "Device is not inserted: %s",
+   bdrv_get_device_name(bs));
+return;
+}
+
+if (!bdrv_is_inserted(target)) {
+error_setg(errp, "Device is not inserted: %s",
+   bdrv_get_device_name(target));
+return;
+}
+
+if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
+return;
+}
+
+if (bdrv_op_is_blocked(target, BLOCK_OP_TYPE_BACKUP_TARGET, errp)) {
+return;
+}
+
 len = bdrv_getlength(bs);
 if (len < 0) {
 error_setg_errno(errp, -len, "unable to get length for '%s'",
@@ -399,6 +425,8 @@ void backup_start(BlockDriverState *bs, BlockDriverState 
*target,
 return;
 }
 
+bdrv_op_block_all(target, job->common.blocker);
+
 job->on_source_error = on_source_error;
 job->on_target_error = on_target_error;
 job->target = target;
diff --git a/blockdev.c b/blockdev.c
index 5651a8e..dbbab1d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2240,6 +2240,8 @@ void qmp_drive_backup(const char *device, const char 
*target,
 aio_context = bdrv_get_aio_context(bs);
 aio_context_acquire(aio_context);
 
+/* Although backup_run has this check too, we need to use bs->drv below, so
+ * do an early check redundantly. */
 if (!bdrv_is_inserted(bs)) {
 error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
 goto out;
@@ -2256,6 +2258,7 @@ void qmp_drive_backup(const char *device, const char 
*target,
 }
 }
 
+/* Early check to avoid creating target */
 if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
 goto out;
 }
@@ -2323,6 +2326,57 @@ BlockDeviceInfoList *qmp_query_named_block_nodes(Error 
**errp)
 return bdrv_named_nodes_list();
 }
 
+void qmp_blockdev_backup(const char *device, const char *target,
+ enum MirrorSyncMode sync,
+ bool has_speed, int64_t speed,
+ bool has_on_source_error,
+ BlockdevOnError on_source_error,
+ bool has_on_target_error,
+ BlockdevOnError on_target_error,
+ Error **errp)
+{
+BlockDriverState *bs;
+BlockDriverState *target_bs;
+Error *local_err = NULL;
+AioContext *aio_context;
+
+if (!has_speed) {
+speed = 0;
+}
+if (!has_on_source_error) {
+on_source_error = BLOCKDEV_ON_ERROR_REPORT;
+}
+if (!has_on_target_error) {
+on_target_error = BLOCKDEV_ON_ERROR_REPORT;
+}
+
+bs = bdrv_find(device);
+if (!bs) {
+error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+return;
+}
+
+aio_context = bdrv_get_aio_context(bs);
+aio_context_acquire(aio_context);
+
+target_bs = bdrv_find(target);
+if (!target_bs) {
+error_set(errp, QERR_DEVICE_NOT_FOUND, target);
+goto out;
+}
+
+bdrv_ref(target_bs);
+bdrv_set_aio_context(target_bs, bdrv_get_aio_context(bs));
+backup_start(bs, target_bs, speed, sync, on_source_error, on_target_error,
+ block_job_cb, bs, &local_err);
+if (local_err != NULL) {
+bdrv_unref(target_bs);
+error_propagate(errp, local_err);
+}
+out:
+aio_context_release(aio_context);
+}
+
 #define DEFAULT_MIRROR_BUF_SIZE   (10 << 20)
 
 void qmp_drive_mirror(const ch

Re: [Qemu-devel] [PATCH] Force pread64/pwrite64 to return 0 for zero-length buffer

[Ilya Palachev]

On 17.12.2014 15:28, Peter Maydell wrote:

If this is a problem, doesn't it apply to more syscalls than
just pread64 and pwrite64 ?


Hi,

We were interested in pwrite64/pread64 only since it caused a failure in 
elfutils (see https://bugzilla.redhat.com/show_bug.cgi?id=1174267).
Of course, it applies also to other syscalls. There are the following 
problems:


- Find what syscalls should be changed (check POSIX specification and 
qemu implementation)


- Whether to change them all with "if" statements as for 
pread64/pwrite64. Or there is some more convenient way?


- Find they how to test them all after the change is made (make check?)

What do you think about that?

Best regards,
Ilya Palachev

Re: [Qemu-devel] [PATCH] pseries: Update SLOF firmware image to 20141202

[Alexander Graf]

On 02.12.14 05:32, Alexey Kardashevskiy wrote:
> The changelog is:
>   > version: update to 20141202
>   > ipv4: Fix send packet across a subnet
>   > pci: scan only type 0 and type 1
>   > usb-xhci: support xhci extended capabilities
>   > Fix term-io-key to also work when stdin has not been set yet
>   > net-snk: llfw startup is using the wrong offset to handler
>   > net-snk: Make call_client_interface() a bit more ABI compliant
>   > net-snk: Remove custom printf version
>   > net-snk: Sanitize our .lds file
>   > net-snk: Avoid type clash for stdin & stdout
>   > net-snk: use socket descriptor in the network stack
>   > net-snk: Remove printk() in favor of printf()
>   > net-snk: Remove redundant prototypes
>   > net-snk: Remove unused timer functions
>   > net-snk: Remove some unused PCI functions
>   > net-snk: Remove module system
>   > net-snk: Remove insmod/rmmod
>   > net-snk: Remove snk_kernel_interface and related definitions
>   > net-snk: Remove pci/vio_config gunk
>   > js2x: Fix build
>   > net-snk: Remoe some now unused "kernel" functions
>   > rtas: Improve error handling in instantiate-rtas
>   > version: update to 20140827
>   > Add private HCALL to inform updated RTAS base and entry
>   > xhci: fix port assignment
> 
> Cc: Nikunj A Dadhania 
> Signed-off-by: Alexey Kardashevskiy 

Thanks, applied to ppc-next.


Alex

Re: [Qemu-devel] [RFC PATCH v6 11/32] From 7abf2f72777958d395cfd01d97fe707cc06152b5 Mon Sep 17 00:00:00 2001

[Alexander Graf]

> On 08.12.2014, at 08:54, Pavel Dovgalyuk  wrote:
> 
> From: Paolo Bonzini 
> 
> Subject: [PATCH] target-ppc: pass DisasContext to SPR generator functions
> 
> Signed-off-by: Paolo Bonzini 
> 
> Signed-off-by: Pavel Dovgalyuk 
> ---


Reviewed-by: Alexander Graf 


Alex

PS: It helps to CC qemu-ppc to get review on PPC patches ;)

Re: [Qemu-devel] [RFC PATCH 0/3] VFIO support for AMBA devices

[Eric Auger]

On 12/17/2014 11:09 AM, Baptiste Reynal wrote:
> The following series add VFIO support for AMBA devices.
> 
> It introduces multiple compatible string support to deal with arm,primecell
> compatible string.
> 
> The VFIOPlatformDevice now checks for this string and performs amba specific
> operations if it is present (change path of the device, add clock in the fd).
> 
> This patch applies on top of http://git.linaro.org/people/eric.auger/qemu.git
> on branch vfio_integ_v9_rc6_official_dynsysbus_v6 for irqfd support.
> 
> TODO: The IRQ forwarding doesn't work.
> 
> The series has been tested using PL330 device, irq forwarding doesn't work.
> (The test hangs on the first IRQ).

Hi Baptiste,

If am not wrong your branch below does not contain the requested kernel
patches for both irqfd and forwarding. Please take the requested pieces
on my linux git:
last official one can be taken at
http://git.linaro.org/people/eric.auger/linux.git (branch 3.18-rc6-v10)
All the requested kernel dependencies are quoted in
http://lists.gnu.org/archive/html/qemu-devel/2014-11/msg04372.html. That
should help ;-)

Best Regards

Eric


> The test performed relies on a special guest kernel, which can be found here
> http://github.com/virtualopensystems/linux-kvm-arm.git on branch
> vfio-platform-v6-guide, with CONFIG_KVM, CONFIG_ARM_SMMU, CONFIG_PL330_DMA
> and CONFIG_VOSYS_DMATEST enabled.
> 
> Run qemu using this command:
> ./qemu-system-arm -M virt -m 512M -enable-kvm \
> -device 
> vfio-pl330,host="2c0a.dma",id=dma,x-irqfd=false,x-forward=false \
> -append "earlyprintk console=ttyAMA0" \
> -kernel zImage -nographic
> 
> Then run DMA test:
> mount -t debugfs none /sys/kernel/debug
> echo 1 > /sys/kernel/debug/vosys_dmatest/start
> 
> Results can be checked using dmesg.
> 
> Baptiste Reynal (3):
>   hw/vfio/sysbus-fdt: generic add_generic_fdt_node
>   hw/vfio: amba device support
>   hw/vfio: add pl330 device support
> 
>  hw/arm/sysbus-fdt.c   | 34 +-
>  hw/vfio/Makefile.objs |  1 +
>  hw/vfio/pl330.c   | 41 +
>  hw/vfio/platform.c| 15 ---
>  include/hw/vfio/vfio-common.h |  1 +
>  include/hw/vfio/vfio-pl330.h  | 26 ++
>  6 files changed, 110 insertions(+), 8 deletions(-)
>  create mode 100644 hw/vfio/pl330.c
>  create mode 100644 include/hw/vfio/vfio-pl330.h
> 

Re: [Qemu-devel] [PATCH 1/1] ich9: add disable_s3, disable_s4, s4_val properties

[Marcel Apfelbaum]

On 12/16/2014 01:23 PM, Amit Shah wrote:

PIIX4 has disable_s3 and disable_s4 properties to enable or disable PM
functions.  Add such properties to the ICH9 chipset as well for the Q35
machine type.

S3 / S4 are not guaranteed to always work (needs work in the guest as
well as QEMU for things to work properly), and disabling advertising of
these features ensures guests don't go into zombie state if something
isn't working right.

The defaults are kept the same as in PIIX4: both S3 and S4 are enabled
by default.

These can be disabled via the cmdline:

   ... -global ICH9-LPC,disable_s3=1 -global ICH9-LPC,disable_s4=1

Maybe we can add them to Q35 Machines instead?
- machine q35,disable_s3=1,disable_s4=1

We have built-in support now.

Thanks,
Marcel



Signed-off-by: Amit Shah 
---
  hw/acpi/ich9.c | 96 ++
  include/hw/acpi/ich9.h |  4 +++
  2 files changed, 100 insertions(+)

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index ea991a3..98828f5 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -269,10 +269,94 @@ static void ich9_pm_set_memory_hotplug_support(Object 
*obj, bool value,
  s->pm.acpi_memory_hotplug.is_enabled = value;
  }

+static void ich9_pm_get_disable_s3(Object *obj, Visitor *v,
+   void *opaque, const char *name,
+   Error **errp)
+{
+ICH9LPCPMRegs *pm = opaque;
+uint8_t value = pm->disable_s3;
+
+visit_type_uint8(v, &value, name, errp);
+}
+
+static void ich9_pm_set_disable_s3(Object *obj, Visitor *v,
+   void *opaque, const char *name,
+   Error **errp)
+{
+ICH9LPCPMRegs *pm = opaque;
+Error *local_err = NULL;
+uint8_t value;
+
+visit_type_uint8(v, &value, name, &local_err);
+if (local_err) {
+goto out;
+}
+pm->disable_s3 = value;
+out:
+error_propagate(errp, local_err);
+}
+
+static void ich9_pm_get_disable_s4(Object *obj, Visitor *v,
+   void *opaque, const char *name,
+   Error **errp)
+{
+ICH9LPCPMRegs *pm = opaque;
+uint8_t value = pm->disable_s4;
+
+visit_type_uint8(v, &value, name, errp);
+}
+
+static void ich9_pm_set_disable_s4(Object *obj, Visitor *v,
+   void *opaque, const char *name,
+   Error **errp)
+{
+ICH9LPCPMRegs *pm = opaque;
+Error *local_err = NULL;
+uint8_t value;
+
+visit_type_uint8(v, &value, name, &local_err);
+if (local_err) {
+goto out;
+}
+pm->disable_s4 = value;
+out:
+error_propagate(errp, local_err);
+}
+
+static void ich9_pm_get_s4_val(Object *obj, Visitor *v,
+   void *opaque, const char *name,
+   Error **errp)
+{
+ICH9LPCPMRegs *pm = opaque;
+uint8_t value = pm->s4_val;
+
+visit_type_uint8(v, &value, name, errp);
+}
+
+static void ich9_pm_set_s4_val(Object *obj, Visitor *v,
+   void *opaque, const char *name,
+   Error **errp)
+{
+ICH9LPCPMRegs *pm = opaque;
+Error *local_err = NULL;
+uint8_t value;
+
+visit_type_uint8(v, &value, name, &local_err);
+if (local_err) {
+goto out;
+}
+pm->s4_val = value;
+out:
+error_propagate(errp, local_err);
+}
+
  void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm, Error **errp)
  {
  static const uint32_t gpe0_len = ICH9_PMIO_GPE0_LEN;
  pm->acpi_memory_hotplug.is_enabled = true;
+pm->disable_s3 = 0;
+pm->disable_s4 = 0;
+pm->s4_val = 2;

  object_property_add_uint32_ptr(obj, ACPI_PM_PROP_PM_IO_BASE,
 &pm->pm_io_base, errp);
@@ -285,6 +369,18 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs 
*pm, Error **errp)
   ich9_pm_get_memory_hotplug_support,
   ich9_pm_set_memory_hotplug_support,
   NULL);
+object_property_add(obj, ACPI_PM_PROP_S3_DISABLED, "uint8",
+ich9_pm_get_disable_s3,
+ich9_pm_set_disable_s3,
+NULL, pm, NULL);
+object_property_add(obj, ACPI_PM_PROP_S4_DISABLED, "uint8",
+ich9_pm_get_disable_s4,
+ich9_pm_set_disable_s4,
+NULL, pm, NULL);
+object_property_add(obj, ACPI_PM_PROP_S4_VAL, "uint8",
+ich9_pm_get_s4_val,
+ich9_pm_set_s4_val,
+NULL, pm, NULL);
  }

  void ich9_pm_device_plug_cb(ICH9LPCPMRegs *pm, DeviceState *dev, Error **errp)
diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
index fe975e6..12d7a7a 100644
--- a/include/hw/acpi/ich9.h
+++ b/include/hw/acpi/ich9.h
@@ -49,6 +49,10 @@ typedef struct ICH9LPCPMRegs {
  AcpiCpuHotplu

Re: [Qemu-devel] [PATCH] Force pread64/pwrite64 to return 0 for zero-length buffer

[Peter Maydell]

On 17 December 2014 at 12:56, Ilya Palachev  wrote:
> We were interested in pwrite64/pread64 only since it caused a failure in
> elfutils (see https://bugzilla.redhat.com/show_bug.cgi?id=1174267).
> Of course, it applies also to other syscalls. There are the following
> problems:
>
> - Find what syscalls should be changed (check POSIX specification and qemu
> implementation)

Well, we should at least fix all the read/write syscalls, so we're
consistent.

> - Whether to change them all with "if" statements as for pread64/pwrite64.
> Or there is some more convenient way?

The other possibility would be to make sure that lock_user/unlock_user
succeeded for zero length. But that would be a pain to validate because
we'd need to check that no other syscall was relying on the "zero length
fails" behaviour. So a specific check and early exit is probably easier
and better.

> - Find they how to test them all after the change is made (make check?)

"make check" doesn't really exercise linux-user. Running the ltp tests
is usually a more comprehensive test:
http://wiki.qemu.org/Testing/LTP

-- PMM

Re: [Qemu-devel] [RFC PATCH 1/3] hw/vfio/sysbus-fdt: generic add_generic_fdt_node

[Eric Auger]

On 12/17/2014 11:09 AM, Baptiste Reynal wrote:
> Modify add_calxeda_midway_xgmac_fdt_node to make it more generic.
> Add multiple compatible strings support.
Hi Baptiste,

Although I understand you may be tempted to transform the Calxeda basic
node creation function into something more generic, we came to the
current result after long discussions with Alex Graf. This generic
function was something released in the past but transformed into
something specific at the end.

Typically the fact the IRQ is edge sensitive or level sensitive may
depend on the device and it would be arbitrary here to choose either in
a "generic" node creation.

So the current trend for new devices is to add another entry in the
add_fdt_node_functions array; genericity target was argued in the past
and rejected.

If you want to do something going in the direction of genericity I would
advise you to investigate using Antonios API ([RFC 0/4] VFIO: PLATFORM:
Return device tree info for a platform device node). This could
typically enable to get the egde/level sensitive characteritics.

> 
> Signed-off-by: Baptiste Reynal 
> ---
>  hw/arm/sysbus-fdt.c | 21 -
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
> index 15bb50c..125ba37 100644
> --- a/hw/arm/sysbus-fdt.c
> +++ b/hw/arm/sysbus-fdt.c
> @@ -58,12 +58,12 @@ typedef struct NodeCreationPair {
>  /* Device Specific Code */
>  
>  /**
> - * add_calxeda_midway_xgmac_fdt_node
> + * add_device_fdt_node
>   *
>   * Generates a very simple node with following properties:
>   * compatible string, regs, interrupts
>   */
> -static int add_calxeda_midway_xgmac_fdt_node(SysBusDevice *sbdev, void 
> *opaque)
> +static int add_generic_fdt_node(SysBusDevice *sbdev, void *opaque)
>  {
>  PlatformBusFdtData *data = opaque;
>  PlatformBusDevice *pbus = data->pbus;
> @@ -80,6 +80,18 @@ static int add_calxeda_midway_xgmac_fdt_node(SysBusDevice 
> *sbdev, void *opaque)
>  VFIODevice *vbasedev = &vdev->vbasedev;
>  Object *obj = OBJECT(sbdev);
>  
> +/*
> + * Process compatible string to deal with multiple strings
> + * (; is replaced by \0)
> + */
> +char *compat = g_strdup(vdev->compat);
> +compat_str_len = strlen(compat) + 1;
> +
> +char *semicolon = compat;
> +while ((semicolon = strchr(semicolon, ';')) != NULL) {
> +*semicolon = '\0';
> +}
why can't you format the string directly in the corresponding VFIO AMBA
device? Formerly we had this problem of conversion since we passed the
format string in qemu command line.

Best Regards

Eric
> +
>  mmio_base = object_property_get_int(obj, "mmio[0]", NULL);
>  
>  nodename = g_strdup_printf("%s/%s@%" PRIx64, parent_node,
> @@ -88,9 +100,8 @@ static int add_calxeda_midway_xgmac_fdt_node(SysBusDevice 
> *sbdev, void *opaque)
>  
>  qemu_fdt_add_subnode(fdt, nodename);
>  
> -compat_str_len = strlen(vdev->compat) + 1;
>  qemu_fdt_setprop(fdt, nodename, "compatible",
> -  vdev->compat, compat_str_len);
> +  compat, compat_str_len);
>  
>  reg_attr = g_new(uint64_t, vbasedev->num_regions*4);
>  
> @@ -140,7 +151,7 @@ fail:
>  
>  /* list of supported dynamic sysbus devices */
>  static const NodeCreationPair add_fdt_node_functions[] = {
> -{TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node},
> +{TYPE_VFIO_CALXEDA_XGMAC, add_generic_fdt_node},
>  {"", NULL}, /*last element*/
>  };
>  
> 

[Qemu-devel] ich6 and ich9 problem using spice with opus enable

[Victor Toso]

Hey,

When connecting to a windows 7 VM with ich6 or ich9 audio devices, I keep
having a click sound every second or so when playing audio files. The
problem doesn't happen with ac97 for instance.
I'm using spice to connect to the VM with OPUS enabled. Looks like that the
audio should be at 48kHz but it is actually 44,1kHz.

This was first reported in spice-devel [0] but it does look like a problem
in the audio device.
Any hints about this problem?

[0]
http://lists.freedesktop.org/archives/spice-devel/2014-November/017995.html

-- 
-
Victor Toso

Re: [Qemu-devel] [RFC PATCH 2/3] hw/vfio: amba device support

[Eric Auger]

On 12/17/2014 11:09 AM, Baptiste Reynal wrote:
> Add VFIO_DEVICE_TYPE_AMBA.
> Differentiate amba and platform devices according to compatible string.
> If the device is amba, clocks description is added in the fd.
> 
> Signed-off-by: Baptiste Reynal 
> ---
>  hw/arm/sysbus-fdt.c   | 11 +++
>  hw/vfio/platform.c| 15 ---
>  include/hw/vfio/vfio-common.h |  1 +
>  3 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
> index 125ba37..41b4c87 100644
> --- a/hw/arm/sysbus-fdt.c
> +++ b/hw/arm/sysbus-fdt.c
> @@ -62,6 +62,8 @@ typedef struct NodeCreationPair {
>   *
>   * Generates a very simple node with following properties:
>   * compatible string, regs, interrupts
> + * And the following properties if it is on an AMBA bus :
> + * clocks, clock-names
>   */
>  static int add_generic_fdt_node(SysBusDevice *sbdev, void *opaque)
>  {
> @@ -103,6 +105,15 @@ static int add_generic_fdt_node(SysBusDevice *sbdev, 
> void *opaque)
>  qemu_fdt_setprop(fdt, nodename, "compatible",
>compat, compat_str_len);
>  
> +if (vbasedev->type == VFIO_DEVICE_TYPE_AMBA) {
> +/* Setup clocks for AMBA device */
> +qemu_fdt_setprop_cells(fdt, nodename, "clocks",
> +qemu_fdt_getprop_cell(fdt, "/apb-pclk", "phandle"));
Shouldn't you test if this clock node exists in the guest device tree?
> +char clock_names[] = "apb_pclk";
> +qemu_fdt_setprop(fdt, nodename, "clock-names", clock_names,
> +sizeof(clock_names));
> +}
> +
>  reg_attr = g_new(uint64_t, vbasedev->num_regions*4);
>  
>  for (i = 0; i < vbasedev->num_regions; i++) {
> diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
> index 39eef08..0a96178 100644
> --- a/hw/vfio/platform.c
> +++ b/hw/vfio/platform.c
> @@ -563,8 +563,13 @@ static int vfio_base_device_init(VFIODevice *vbasedev)
>  }
>  
>  /* Check that the host device exists */
> -snprintf(path, sizeof(path), "/sys/bus/platform/devices/%s/",
> - vbasedev->name);
> +if (vbasedev->type == VFIO_DEVICE_TYPE_AMBA) {
> +snprintf(path, sizeof(path), "/sys/bus/amba/devices/%s/",
> +vbasedev->name);
> +} else {
> +snprintf(path, sizeof(path), "/sys/bus/platform/devices/%s/",
> +vbasedev->name);
> +}
>  
>  if (stat(path, &st) < 0) {
>  error_report("vfio: error: no such host device: %s", path);
> @@ -661,7 +666,11 @@ static void vfio_platform_realize(DeviceState *dev, 
> Error **errp)
>  VFIODevice *vbasedev = &vdev->vbasedev;
>  int i, ret;
>  
> -vbasedev->type = VFIO_DEVICE_TYPE_PLATFORM;
> +if (strstr(vdev->compat, "arm,primecell")) {
> +vbasedev->type = VFIO_DEVICE_TYPE_AMBA;
> +} else {
> +vbasedev->type = VFIO_DEVICE_TYPE_PLATFORM;
> +}
>  vbasedev->ops = &vfio_platform_ops;
The plan is to use VFIO_DEVICE_GET_INFO instead. Alex recently requested
we test the flags. We need a header sync for that.

Best Regards

Eric
>  
>  #ifdef CONFIG_KVM
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 58fd786..2f1b09c 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -49,6 +49,7 @@ extern int vfio_kvm_device_fd;
>  enum {
>  VFIO_DEVICE_TYPE_PCI = 0,
>  VFIO_DEVICE_TYPE_PLATFORM = 1,
> +VFIO_DEVICE_TYPE_AMBA = 2,
>  };
>  
>  typedef struct VFIORegion {
> 

Re: [Qemu-devel] [PATCH 0/6] relicense QEMU softfloat from 2b to to 2a

[Peter Maydell]

On 5 December 2014 at 11:15, Peter Maydell  wrote:
> Hi; this is a ping (with some of the less frequent contributors
> taken off cc), because I'd really like it if we could get this
> issue dealt with (preferably before a flood of new patches to
> softfloat arrive after the tree reopens for 2.3), but this patchset
> needs review (for methodology, of my reimplemented functions, and
> of whether we should avoid the bisection break).

Ping^2 -- we've had some review of the reimplemented functions,
but I would still appreciate review of methodology, opinions on
whether we should squash the revert and reimplement patches
together, etc.

thanks
-- PMM

Re: [Qemu-devel] Can we make monitor commands identify BDS / BB by name consistently? (was: Review of monitor commands identifying BDS / BB by name)

[Kevin Wolf]

Am 16.12.2014 um 19:12 hat Markus Armbruster geschrieben:
> Conscious design decision: Backend (BB) and node (BDS) names share a
> common name space.
> 
> Enables a convenience feature: when a command needs a node, we accept
> either kind of name, and a backend name is resolved to its root node.
> 
> Should not be confused with a command that can work either on a backend
> or on a node.  There, a backend name resolves to the backend, not its
> root node.  Can't point to an example offhand.
> 
> Let's concentrate on the "command needs a node" case.
> 
> As we saw in my review of monitor commands, we have two different
> conventions there.
> 
> * Single name
> 
>   Within BlockdevOptions objects (used by blockdev-add), we use a single
>   string member, with a name that explains its role.  Actually, the
>   member is an anonymous union of string and BlockdevOptions.
> 
>   Example: a BlockdevOptionsGenericFormat object (used for format "raw"
>   and others) has a member @file that may name a backend or a node.
> 
>   Example: a BlockdevOptionsQcow2 object (used for "qcow2"), has a
>   member @file as above, and a member @backing that may again name a
>   backend or a node.
> 
> * Pair of names
> 
>   Elsewhere, command argument objects have a pair of optional members,
>   of which exactly one must be present.  One of them must name a
>   backend, the other must name a node.  The former is commonly called
>   @device, the latter @node-name.
> 
>   Example: block_passwd parameters @device and @node-name.
> 
> I'd very much like some consistency here.
> 
> As Kevin pointed out, you can't easily change BlockdevOptions to the
> "pair of names" convention, because an anonymous union can have only one
> object member, and that's taken by BlockdevOptions.  If you want us to
> adopt the "pair of names" convention, you need to come up with a way to
> use it with BlockdevOptions.
> 
> I want us to adopt the "single name" convention instead.  Therefore, I
> need to come up with a way to use it with the command argument objects
> that currently use "pair of names".  The problems there are
> compatibility and discoverability.
> 
> Four ways come to mind:
> 
> 1. Extend @node-name to accept backend names, deprecate @device
> 
>@node-name becomes mandatory except in deprecated usage.
>Nevertheless, it remains optional in the schema, which is confusing.
> 
>For discovery, you first have to try whether the command accepts
>parameter @node-name.  If no, you have a QEMU predating node names,
>and you need to use @device.  If yes, you need to try whether the
>command accepts a backend name as argument for @node-name.  Involves
>defining a backend.  Awkward.
> 
> 2. Extend @device to accept node names, deprecate @node-name
> 
>@device becomes mandatory except in deprecated usage.  Nevertheless,
>it remains optional in the schema, which is confusing.

Actually, I think we may consider that device used to be mandatory until
recently. Management tools should be able to cope with it. If we make it
mandatory again, we may just look like an older qemu version - would
that be that bad?

>We're stuck with a bad parameter name: @device.
> 
>For discovery, you need to try whether the command accepts a node
>name as argument for @device.  Involves defining a node.  Almost as
>awkward.
> 
> 3. Add a new parameter, deprecate both old ones
> 
>The new parameter is mandatory except in deprecated usage.
>Nevertheless, it's optional in the schema, which is confusing.
> 
>Discovery needs to check which of the parameters the command accepts.
>Less awkward.
> 
> 4. Add a new command, deprecate the old one
> 
>Quick search for commands to deprecate: block_passwd, block_resize,
>blockdev-snapshot-sync.  Not really bad.
> 
>Discovery needs to check query-commands for the new command.  Easy.
> 
> Any objections to #4?

The first two don't match the wanted pattern for block layer
commands anyway, so blockdev-passwd and blockdev-resize seems like an
improvement.

blockdev-snapshot-sync should probably be replaced by a "real blockdev"
command that adds a snapshot into the tree without actually creating the
BDS, i.e. you need to use blockdev-add first. This solves the problem
that currently you can't specify any bdrv_open() options when taking a
snapshot.

Kevin

[Qemu-devel] [PULL 0/4] vga: cirrus hwcursor fixes.

[Gerd Hoffmann]

  Hi,

vga patches lingering way too long in my queue.

Finally merging the cirrus hwcursor fixes (for NT4 guests!)
discussed months ago ...

Also minor tweaks picked up during freeze.

please pull,
  Gerd

The following changes since commit dfa9c2a0f4d0a0c8b2c1449ecdbb1297427e1560:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging 
(2014-12-15 16:43:42 +)

are available in the git repository at:


  git://git.kraxel.org/qemu tags/pull-vga-20141216-1

for you to fetch changes up to 46817e86fc1d97af0a7d9e4060730f7b00183083:

  vga: set catagory bit for secondary vga device (2014-12-16 15:14:42 +0100)


cirrus hwcursor fixes.
set secondary-vga category.


Benjamin Herrenschmidt (2):
  vga: Add mechanism to force the use of a shadow surface
  cirrus: Force use of shadow pixmap when HW cursor is enabled

Gerd Hoffmann (1):
  move hw cursor pos from cirrus to vga

Gonglei (1):
  vga: set catagory bit for secondary vga device

 hw/display/cirrus_vga.c | 40 +++-
 hw/display/vga-pci.c|  1 +
 hw/display/vga.c| 17 +++--
 hw/display/vga_int.h|  3 +++
 4 files changed, 42 insertions(+), 19 deletions(-)

[Qemu-devel] [PULL 1/4] vga: Add mechanism to force the use of a shadow surface

[Gerd Hoffmann]

From: Benjamin Herrenschmidt 

This prevents surface sharing which will be necessary to
fix cirrus HW cursor support.

Signed-off-by: Benjamin Herrenschmidt 
Signed-off-by: Gerd Hoffmann 
---
 hw/display/vga.c | 17 +++--
 hw/display/vga_int.h |  1 +
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/hw/display/vga.c b/hw/display/vga.c
index 52eaf05..a620c07 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -1436,6 +1436,7 @@ static void vga_draw_graphic(VGACommonState *s, int 
full_update)
 uint8_t *d;
 uint32_t v, addr1, addr;
 vga_draw_line_func *vga_draw_line = NULL;
+bool share_surface;
 #ifdef HOST_WORDS_BIGENDIAN
 bool byteswap = !s->big_endian_fb;
 #else
@@ -1479,21 +1480,33 @@ static void vga_draw_graphic(VGACommonState *s, int 
full_update)
 }
 
 depth = s->get_bpp(s);
+
+share_surface = (!s->force_shadow) &&
+( depth == 32 || (depth == 16 && !byteswap) );
 if (s->line_offset != s->last_line_offset ||
 disp_width != s->last_width ||
 height != s->last_height ||
 s->last_depth != depth ||
-s->last_byteswap != byteswap) {
-if (depth == 32 || (depth == 16 && !byteswap)) {
+s->last_byteswap != byteswap ||
+share_surface != is_buffer_shared(surface)) {
+if (share_surface) {
 pixman_format_code_t format =
 qemu_default_pixman_format(depth, !byteswap);
 surface = qemu_create_displaysurface_from(disp_width,
 height, format, s->line_offset,
 s->vram_ptr + (s->start_addr * 4));
 dpy_gfx_replace_surface(s->con, surface);
+#ifdef DEBUG_VGA
+printf("VGA: Using shared surface for depth=%d swap=%d\n",
+   depth, byteswap);
+#endif
 } else {
 qemu_console_resize(s->con, disp_width, height);
 surface = qemu_console_surface(s->con);
+#ifdef DEBUG_VGA
+printf("VGA: Using shadow surface for depth=%d swap=%d\n",
+   depth, byteswap);
+#endif
 }
 s->last_scr_width = disp_width;
 s->last_scr_height = height;
diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h
index ed69e06..72e00f2 100644
--- a/hw/display/vga_int.h
+++ b/hw/display/vga_int.h
@@ -151,6 +151,7 @@ typedef struct VGACommonState {
 uint32_t last_scr_width, last_scr_height; /* in pixels */
 uint32_t last_depth; /* in bits */
 bool last_byteswap;
+bool force_shadow;
 uint8_t cursor_start, cursor_end;
 bool cursor_visible_phase;
 int64_t cursor_blink_time;
-- 
1.8.3.1

[Qemu-devel] [PULL 2/4] cirrus: Force use of shadow pixmap when HW cursor is enabled

[Gerd Hoffmann]

From: Benjamin Herrenschmidt 

The HW cursor cannot be painted on a shared surface. This fixes HW
cursor display in Windows NT 4.0 and Windows 98.

Signed-off-by: Benjamin Herrenschmidt 
Signed-off-by: Gerd Hoffmann 
---
 hw/display/cirrus_vga.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index 2725264..686b062 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -1351,7 +1351,6 @@ static void cirrus_vga_write_sr(CirrusVGAState * s, 
uint32_t val)
 case 0x0d: // VCLK 2
 case 0x0e: // VCLK 3
 case 0x0f: // DRAM Control
-case 0x12: // Graphics Cursor Attribute
 case 0x13: // Graphics Cursor Pattern Address
 case 0x14: // Scratch Register 2
 case 0x15: // Scratch Register 3
@@ -1370,6 +1369,14 @@ static void cirrus_vga_write_sr(CirrusVGAState * s, 
uint32_t val)
   s->vga.sr_index, val);
 #endif
break;
+case 0x12: // Graphics Cursor Attribute
+   s->vga.sr[0x12] = val;
+s->vga.force_shadow = !!(val & CIRRUS_CURSOR_SHOW);
+#ifdef DEBUG_CIRRUS
+printf("cirrus: cursor ctl SR12=%02x (force shadow: %d)\n",
+   val, s->vga.force_shadow);
+#endif
+break;
 case 0x17: // Configuration Readback and Extended Control
s->vga.sr[s->vga.sr_index] = (s->vga.sr[s->vga.sr_index] & 0x38)
| (val & 0xc7);
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 5/8] target-tricore: Add instructions of RR opcode format, that have 0x4b as the first opcode

[Bastian Koppelmann]

On 12/12/2014 08:45 PM, Richard Henderson wrote:


Probably doesn't matter much, but

   ret = (ctpop8(r1) & 1)
 | ((ctpop8(r1 >> 8) & 1) << 8)
 | ((ctpop8(r1 >> 16) & 1) << 16)
 | ((ctpop8(r1 >> 24) & 1) << 24);

This looks much more compact to me. Thanks!

One could also make a case for adding new helpers that
use __builtin_parity rather than __builtin_popcount.

I usually like to look at things like this and see how
the general infrastructure can be improved...

This is a good idea. I'll send another patch, that adds the infrastructure.

Cheers,
Bastian

[Qemu-devel] [PULL 3/4] move hw cursor pos from cirrus to vga

[Gerd Hoffmann]

---
 hw/display/cirrus_vga.c | 31 +++
 hw/display/vga_int.h|  2 ++
 2 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index 686b062..3a53f20 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -202,8 +202,6 @@ typedef struct CirrusVGAState {
 uint32_t cirrus_bank_base[2];
 uint32_t cirrus_bank_limit[2];
 uint8_t cirrus_hidden_palette[48];
-uint32_t hw_cursor_x;
-uint32_t hw_cursor_y;
 int cirrus_blt_pixelwidth;
 int cirrus_blt_width;
 int cirrus_blt_height;
@@ -1328,7 +1326,7 @@ static void cirrus_vga_write_sr(CirrusVGAState * s, 
uint32_t val)
 case 0xd0:
 case 0xf0: // Graphics Cursor X
s->vga.sr[0x10] = val;
-   s->hw_cursor_x = (val << 3) | (s->vga.sr_index >> 5);
+s->vga.hw_cursor_x = (val << 3) | (s->vga.sr_index >> 5);
break;
 case 0x11:
 case 0x31:
@@ -1339,7 +1337,7 @@ static void cirrus_vga_write_sr(CirrusVGAState * s, 
uint32_t val)
 case 0xd1:
 case 0xf1: // Graphics Cursor Y
s->vga.sr[0x11] = val;
-   s->hw_cursor_y = (val << 3) | (s->vga.sr_index >> 5);
+s->vga.hw_cursor_y = (val << 3) | (s->vga.sr_index >> 5);
break;
 case 0x07: // Extended Sequencer Mode
 cirrus_update_memory_access(s);
@@ -2195,14 +2193,14 @@ static void cirrus_cursor_invalidate(VGACommonState *s1)
 }
 /* invalidate last cursor and new cursor if any change */
 if (s->last_hw_cursor_size != size ||
-s->last_hw_cursor_x != s->hw_cursor_x ||
-s->last_hw_cursor_y != s->hw_cursor_y) {
+s->last_hw_cursor_x != s->vga.hw_cursor_x ||
+s->last_hw_cursor_y != s->vga.hw_cursor_y) {
 
 invalidate_cursor1(s);
 
 s->last_hw_cursor_size = size;
-s->last_hw_cursor_x = s->hw_cursor_x;
-s->last_hw_cursor_y = s->hw_cursor_y;
+s->last_hw_cursor_x = s->vga.hw_cursor_x;
+s->last_hw_cursor_y = s->vga.hw_cursor_y;
 /* compute the real cursor min and max y */
 cirrus_cursor_compute_yrange(s);
 invalidate_cursor1(s);
@@ -2259,14 +2257,15 @@ static void cirrus_cursor_draw_line(VGACommonState *s1, 
uint8_t *d1, int scr_y)
 } else {
 h = 32;
 }
-if (scr_y < s->hw_cursor_y ||
-scr_y >= (s->hw_cursor_y + h))
+if (scr_y < s->vga.hw_cursor_y ||
+scr_y >= (s->vga.hw_cursor_y + h)) {
 return;
+}
 
 src = s->vga.vram_ptr + s->real_vram_size - 16 * 1024;
 if (s->vga.sr[0x12] & CIRRUS_CURSOR_LARGE) {
 src += (s->vga.sr[0x13] & 0x3c) * 256;
-src += (scr_y - s->hw_cursor_y) * 16;
+src += (scr_y - s->vga.hw_cursor_y) * 16;
 poffset = 8;
 content = ((uint32_t *)src)[0] |
 ((uint32_t *)src)[1] |
@@ -2274,7 +2273,7 @@ static void cirrus_cursor_draw_line(VGACommonState *s1, 
uint8_t *d1, int scr_y)
 ((uint32_t *)src)[3];
 } else {
 src += (s->vga.sr[0x13] & 0x3f) * 256;
-src += (scr_y - s->hw_cursor_y) * 4;
+src += (scr_y - s->vga.hw_cursor_y) * 4;
 
 
 poffset = 128;
@@ -2286,10 +2285,10 @@ static void cirrus_cursor_draw_line(VGACommonState *s1, 
uint8_t *d1, int scr_y)
 return;
 w = h;
 
-x1 = s->hw_cursor_x;
+x1 = s->vga.hw_cursor_x;
 if (x1 >= s->vga.last_scr_width)
 return;
-x2 = s->hw_cursor_x + w;
+x2 = s->vga.hw_cursor_x + w;
 if (x2 > s->vga.last_scr_width)
 x2 = s->vga.last_scr_width;
 w = x2 - x1;
@@ -2778,8 +2777,8 @@ static const VMStateDescription vmstate_cirrus_vga = {
 VMSTATE_INT32(vga.bank_offset, CirrusVGAState),
 VMSTATE_UINT8(cirrus_hidden_dac_lockindex, CirrusVGAState),
 VMSTATE_UINT8(cirrus_hidden_dac_data, CirrusVGAState),
-VMSTATE_UINT32(hw_cursor_x, CirrusVGAState),
-VMSTATE_UINT32(hw_cursor_y, CirrusVGAState),
+VMSTATE_UINT32(vga.hw_cursor_x, CirrusVGAState),
+VMSTATE_UINT32(vga.hw_cursor_y, CirrusVGAState),
 /* XXX: we do not save the bitblt state - we assume we do not save
the state when the blitter is active */
 VMSTATE_END_OF_LIST()
diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h
index 72e00f2..fcfcc5f 100644
--- a/hw/display/vga_int.h
+++ b/hw/display/vga_int.h
@@ -163,6 +163,8 @@ typedef struct VGACommonState {
 bool default_endian_fb;
 /* hardware mouse cursor support */
 uint32_t invalidated_y_table[VGA_MAX_HEIGHT / 32];
+uint32_t hw_cursor_x;
+uint32_t hw_cursor_y;
 void (*cursor_invalidate)(struct VGACommonState *s);
 void (*cursor_draw_line)(struct VGACommonState *s, uint8_t *d, int y);
 /* tell for each page if it has been updated since the last time */
-- 
1.8.3.1

[Qemu-devel] [PULL 4/4] vga: set catagory bit for secondary vga device

[Gerd Hoffmann]

From: Gonglei 

Signed-off-by: Gonglei 
Signed-off-by: Gerd Hoffmann 
---
 hw/display/vga-pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c
index db922f1..53739e4 100644
--- a/hw/display/vga-pci.c
+++ b/hw/display/vga-pci.c
@@ -321,6 +321,7 @@ static void secondary_class_init(ObjectClass *klass, void 
*data)
 dc->vmsd = &vmstate_vga_pci;
 dc->props = secondary_pci_properties;
 dc->reset = pci_secondary_vga_reset;
+set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
 }
 
 static const TypeInfo vga_info = {
-- 
1.8.3.1

[Qemu-devel] [PULL 0/1] update ipxe from 69313ed to 35c5379

[Gerd Hoffmann]

  Hi,

Gonglei asked for a ipxe update to pick up a bugfix, so here we go!

please pull,
  Gerd

The following changes since commit dfa9c2a0f4d0a0c8b2c1449ecdbb1297427e1560:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging 
(2014-12-15 16:43:42 +)

are available in the git repository at:


  git://git.kraxel.org/qemu tags/pull-roms-20141217-1

for you to fetch changes up to c246cee4eedb17ae3932d699e009a8b63240235f:

  update ipxe from 69313ed to 35c5379 (2014-12-17 14:11:39 +0100)


update ipxe from 69313ed to 35c5379


Gerd Hoffmann (1):
  update ipxe from 69313ed to 35c5379

 pc-bios/efi-e1000.rom| Bin 194560 -> 197120 bytes
 pc-bios/efi-eepro100.rom | Bin 196096 -> 197632 bytes
 pc-bios/efi-ne2k_pci.rom | Bin 194560 -> 195584 bytes
 pc-bios/efi-pcnet.rom| Bin 194560 -> 195584 bytes
 pc-bios/efi-rtl8139.rom  | Bin 198144 -> 200192 bytes
 pc-bios/efi-virtio.rom   | Bin 192000 -> 194048 bytes
 roms/ipxe|   2 +-
 7 files changed, 1 insertion(+), 1 deletion(-)

[Qemu-devel] [PULL 1/1] update ipxe from 69313ed to 35c5379

[Gerd Hoffmann]

Anton D. Kachalov (1):
  [intel] Add 8086:1557 card (Intel 82599 10G ethernet mezz)

Christian Hesse (1):
  [build] Merge util/geniso and util/genliso

Curtis Larsen (3):
  [efi] Use EFI_CONSOLE_CONTROL_PROTOCOL to set text mode if available
  [efi] Report errors from attempting to disconnect existing drivers
  [efi] Try various possible SNP receive filters

Dale Hamel (1):
  [smbios] Expose board serial number as ${board-serial}

Florian Schmaus (1):
  [build] Set GITVERSION only if there is a git repository

Hannes Reinecke (3):
  [ethernet] Provide eth_random_addr() to generate random Ethernet addresses
  [igbvf] Assign random MAC address if none is set
  [igbvf] Allow changing of MAC address

Jan Kiszka (1):
  [intel] Add I217-LM PCI ID

Marin Hannache (4):
  [nfs] Fix an invalid free() when loading a symlink
  [nfs] Fix an invalid free() when loading a regular (non-symlink) file
  [nfs] Rewrite NFS URI handling
  [readline] Add CTRL-W shortcut to remove a word

Michael Brown (144):
  [profile] Allow interrupts to be excluded from profiling results
  [intel] Exclude time spent in hypervisor from profiling
  [build] Fix version.o dependency upon git index
  [tcp] Defer sending ACKs until all received packets have been processed
  [lkrnprefix] Function as a bzImage kernel
  [build] Avoid errors when build directory is mounted via NFS
  [undi] Apply quota only to number of complete received packets
  [lkrnprefix] Make real-mode setup code relocatable
  [intel] Increase receive ring fill level
  [syslog] Strip invalid characters from hostname
  [test] Add self-tests for strdup()
  [libc] Prevent strndup() from reading beyond the end of the string
  [efi] Allow for optional protocols
  [efi] Make EFI_DEVICE_PATH_TO_TEXT_PROTOCOL optional
  [efi] Make EFI_HII_DATABASE_PROTOCOL optional
  [efi] Do not try to fetch loaded image device path protocol
  [ipv6] Fix definition of IN6_IS_ADDR_LINKLOCAL()
  [dhcpv6] Do not set sin6_scope_id on the unspecified client socket address
  [ipv6] Do not set sin6_scope_id on source address
  [ipv6] Include network device when transcribing multicast addresses
  [ipv6] Avoid potentially copying from a NULL pointer in ipv6_tx()
  [librm] Allow for the PIC interrupt vector offset to be changed
  [ifmgmt] Do not sleep CPU while configuring network devices
  [scsi] Improve sense code parsing
  [iscsi] Read IPv4 settings only from the relevant network device
  [iscsi] Include IP address origin in iBFT
  [debug] Allow debug message colours to be customised via DBGCOL=...
  [build] Expose build timestamp, build name, and product names
  [efi] Allow device paths to be easily included in debug messages
  [efi] Provide a meaningful EFI SNP device name
  [efi] Restructure EFI driver model
  [build] Fix erroneous object name in version object
  [build] Add yet another potential location for isolinux.bin
  [efi] Allow network devices to be created on top of arbitrary SNP devices
  [autoboot] Allow autoboot device to be identified by link-layer address
  [efi] Identify autoboot device by MAC address when chainloading
  [efi] Attempt to start only drivers claiming support for a device
  [efi] Rewrite SNP NIC driver
  [efi] Include SNP NIC driver within the all-drivers target
  [crypto] Add support for iPAddress subject alternative names
  [crypto] Fix debug message
  [netdevice] Reset network device index when last device is unregistered
  [efi] Update EDK2 headers
  [efi] Install our own disk I/O protocol and claim exclusive use of it
  [efi] Allow for interception of boot services calls by loaded image
  [efi] Print well-known GUIDs by name in debug messages
  [efi] Include EFI_CONSOLE_CONTROL_PROTOCOL header
  [ioapi] Fail ioremap() when attempting to map a zero bus address
  [intel] Check for ioremap() failures
  [realtek] Check for ioremap() failures
  [vmxnet3] Check for ioremap() failures
  [skel] Check for ioremap() failures
  [myson] Check for ioremap() failures
  [natsemi] Check for ioremap() failures
  [i386] Add functions to read and write model-specific registers
  [x86_64] Add functions to read and write model-specific registers
  [efi] Show more diagnostic information when building with DEBUG=efi_wrap
  [ioapi] Centralise notion of PAGE_SIZE
  [lotest] Discard packets arriving on the incorrect network device
  [xen] Import selected public headers
  [xen] Add basic support for PV-HVM domains
  [xen] Add support for Xen netfront virtual NICs
  [efi] Default to releasing network devices for use via SNP
  [efi] Unload started images only on failure
  [efi] Fill in loaded image's DeviceHandle if firmware fails to do so
  [efi] Fix incorrect debug message level whe

Re: [Qemu-devel] [PATCH v4 21/47] Add wrappers and handlers for sending/receiving the postcopy-ram migration messages.

[Dr. David Alan Gilbert]

* David Gibson (da...@gibson.dropbear.id.au) wrote:
> On Fri, Oct 03, 2014 at 06:47:27PM +0100, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" 
> > 
> > Add state variable showing current incoming postcopy state.
> 
> This appears to implement a lot more than just adding a state variable...

It's clearer with the title line;  I've reworded it to:

Add wrappers and handlers for sending/receiving the postcopy-ram migration 
messages.

The state of the postcopy process is managed via a series of messages;
   * Add wrappers and handlers for sending/receiving these messages
   * Add state variable that track the current state of postcopy

> > Signed-off-by: Dr. David Alan Gilbert 
> > ---
> >  include/migration/migration.h |   8 +
> >  include/sysemu/sysemu.h   |  20 +++
> >  savevm.c  | 335 
> > ++
> >  3 files changed, 363 insertions(+)
> > 
> > diff --git a/include/migration/migration.h b/include/migration/migration.h
> > index 0d9f62d..2c078c4 100644
> > --- a/include/migration/migration.h
> > +++ b/include/migration/migration.h
> > @@ -61,6 +61,14 @@ typedef struct MigrationState MigrationState;
> >  struct MigrationIncomingState {
> >  QEMUFile *file;
> >  
> > +volatile enum {
> 
> What's the reason for the volatile?  I think that really needs a comment.

I've removed it and replaced it by atomic_ functions to access it;
the state is accessed from multiple threads.

> > +POSTCOPY_RAM_INCOMING_NONE = 0,  /* Initial state - no postcopy */
> > +POSTCOPY_RAM_INCOMING_ADVISE,
> > +POSTCOPY_RAM_INCOMING_LISTENING,
> > +POSTCOPY_RAM_INCOMING_RUNNING,
> > +POSTCOPY_RAM_INCOMING_END
> > +} postcopy_ram_state;
> > +
> >  QEMUFile *return_path;
> >  QemuMutex  rp_mutex;/* We send replies from multiple threads */
> >  };



> > diff --git a/savevm.c b/savevm.c
> > index 7236232..b942e8c 100644
> > --- a/savevm.c
> > +++ b/savevm.c
> > @@ -39,6 +39,7 @@
> >  #include "exec/memory.h"
> >  #include "qmp-commands.h"
> >  #include "trace.h"
> > +#include "qemu/bitops.h"
> >  #include "qemu/iov.h"
> >  #include "block/snapshot.h"
> >  #include "block/qapi.h"
> > @@ -624,6 +625,92 @@ void qemu_savevm_send_openrp(QEMUFile *f)
> >  {
> >  qemu_savevm_command_send(f, QEMU_VM_CMD_OPENRP, 0, NULL);
> >  }
> > +
> > +/* Send prior to any RAM transfer */
> > +void qemu_savevm_send_postcopy_ram_advise(QEMUFile *f)
> > +{
> > +DPRINTF("send postcopy-ram-advise");
> > +uint64_t tmp[2];
> > +tmp[0] = cpu_to_be64(sysconf(_SC_PAGESIZE));
> > +tmp[1] = cpu_to_be64(1ul << qemu_target_page_bits());
> > +
> > +qemu_savevm_command_send(f, QEMU_VM_CMD_POSTCOPY_RAM_ADVISE, 16,
> > + (uint8_t *)tmp);
> > +}
> > +
> > +/* Prior to running, to cause pages that have been dirtied after precopy
> > + * started to be discarded on the destination.
> > + * CMD_POSTCOPY_RAM_DISCARD consist of:
> > + *  3 byte header (filled in by qemu_savevm_send_postcopy_ram_discard)
> > + *  byte   version (0)
> > + *  byte   offset into the 1st data word containing 1st page of 
> > RAMBlock
> 
> I'm not able to follow what that description means.

I've reworded it as:
 *  byte   offset to be subtracted from each page address to deal with
 * RAMBlocks that don't start on a mask word boundary.

(I've tried reworking this protocol a few times, the offset still seems
to work out easiest, otherwise you end up having to treat the address entries
as potentially signed).



> > +/*
> > + * Postcopy will be sending lots of small messages along the return 
> > path
> > + * that it needs quick answers to.
> > + */
> > +socket_set_nodelay(qemu_get_fd(mis->return_path));
> 
> So, here you break the QEMUFile abstraction and assume you have a
> socket.

Ah yes; I put that in to see if it would help.  Hmm; I've taken it out for now,
I need to see if there's a sensible way to add the abstration.

> > +while (len) {
> > +uint64_t startaddr;
> > +uint32_t mask;
> > +/*
> > + * We now have pairs of address, mask
> > + *   The mask is 32 bits of bitmask starting at 'startaddr'-offset
> > + *   RAMBlock; e.g. if the RAMBlock started at 8k where TPS=4k
> > + *   then first_bit_offset=2 and the 1st 2 bits of the mask
> > + *   aren't relevant to this RAMBlock, and bit 2 corresponds
> > + *   to the 1st page of this RAMBlock
> 
> Um.. yeah.. can't make much snse of this comment either.

Well, at least it's the one that corresponds to the comment above you couldn't
understand either.
I've reworded it to:
 * We now have pairs of address, mask
 *   Each word of mask is 32 bits, where each bit corresponds to one
 *   target page.
 *   RAMBlocks don't necessarily start on word boundaries, 
 *   and the offset in th

[Qemu-devel] [PATCH v2 8/8] target-tricore: Add instructions of RR1 opcode format, that have 0xb3 as first opcode

[Bastian Koppelmann]

Add instructions of RR1 opcode format, that have 0xb3 as first opcode.
Add helper functions mulh, mulmh and mulrh, that compute multiplication,
with multiprecision (mulmh) or rounding (mulrh) of 4 halfwords, being either 
low or high parts
of two 32 bit regs.

Signed-off-by: Bastian Koppelmann 
---
v1 -> v2:
- mul_h/mulm_h/mulr_h: * move arg extraction to tcg-ops.
   * compute psw flags in tcg-ops.
   * helper now use TCG_CALL_NO_RWG_SE flag.

 target-tricore/helper.h|   4 +
 target-tricore/op_helper.c |  72 +
 target-tricore/translate.c | 196 +
 3 files changed, 272 insertions(+)

diff --git a/target-tricore/helper.h b/target-tricore/helper.h
index f45600d..e74d8f6 100644
--- a/target-tricore/helper.h
+++ b/target-tricore/helper.h
@@ -87,6 +87,10 @@ DEF_HELPER_3(dvinit_b_13, i64, env, i32, i32)
 DEF_HELPER_3(dvinit_b_131, i64, env, i32, i32)
 DEF_HELPER_3(dvinit_h_13, i64, env, i32, i32)
 DEF_HELPER_3(dvinit_h_131, i64, env, i32, i32)
+/* mulh */
+DEF_HELPER_FLAGS_5(mul_h, TCG_CALL_NO_RWG_SE, i64, i32, i32, i32, i32, i32)
+DEF_HELPER_FLAGS_5(mulm_h, TCG_CALL_NO_RWG_SE, i64, i32, i32, i32, i32, i32)
+DEF_HELPER_FLAGS_5(mulr_h, TCG_CALL_NO_RWG_SE, i32, i32, i32, i32, i32, i32)
 /* CSA */
 DEF_HELPER_2(call, void, env, i32)
 DEF_HELPER_1(ret, void, env)
diff --git a/target-tricore/op_helper.c b/target-tricore/op_helper.c
index 63d2d56..13e2729 100644
--- a/target-tricore/op_helper.c
+++ b/target-tricore/op_helper.c
@@ -1228,6 +1228,78 @@ uint64_t helper_dvinit_h_131(CPUTriCoreState *env, 
uint32_t r1, uint32_t r2)
 return ret;
 }

+uint64_t helper_mul_h(uint32_t arg00, uint32_t arg01,
+  uint32_t arg10, uint32_t arg11, uint32_t n)
+{
+uint64_t ret;
+uint32_t result0, result1;
+
+int32_t sc1 = ((arg00 & 0x) == 0x8000) &&
+  ((arg10 & 0x) == 0x8000) && (n == 1);
+int32_t sc0 = ((arg01 & 0x) == 0x8000) &&
+  ((arg11 & 0x) == 0x8000) && (n == 1);
+if (sc1) {
+result1 = 0x7fff;
+} else {
+result1 = (((uint32_t)(arg00 * arg10)) << n);
+}
+if (sc0) {
+result0 = 0x7fff;
+} else {
+result0 = (((uint32_t)(arg01 * arg11)) << n);
+}
+ret = (((uint64_t)result1 << 32)) | result0;
+return ret;
+}
+
+uint64_t helper_mulm_h(uint32_t arg00, uint32_t arg01,
+   uint32_t arg10, uint32_t arg11, uint32_t n)
+{
+uint64_t ret;
+int64_t result0, result1;
+
+int32_t sc1 = ((arg00 & 0x) == 0x8000) &&
+  ((arg10 & 0x) == 0x8000) && (n == 1);
+int32_t sc0 = ((arg01 & 0x) == 0x8000) &&
+  ((arg11 & 0x) == 0x8000) && (n == 1);
+
+if (sc1) {
+result1 = 0x7fff;
+} else {
+result1 = (((int32_t)arg00 * (int32_t)arg10) << n);
+}
+if (sc0) {
+result0 = 0x7fff;
+} else {
+result0 = (((int32_t)arg01 * (int32_t)arg11) << n);
+}
+ret = (result1 + result0);
+ret = ret << 16;
+return ret;
+}
+uint32_t helper_mulr_h(uint32_t arg00, uint32_t arg01,
+   uint32_t arg10, uint32_t arg11, uint32_t n)
+{
+uint32_t result0, result1;
+
+int32_t sc1 = ((arg00 & 0x) == 0x8000) &&
+  ((arg10 & 0x) == 0x8000) && (n == 1);
+int32_t sc0 = ((arg01 & 0x) == 0x8000) &&
+  ((arg11 & 0x) == 0x8000) && (n == 1);
+
+if (sc1) {
+result1 = 0x7fff;
+} else {
+result1 = ((arg00 * arg10) << n) + 0x8000;
+}
+if (sc0) {
+result0 = 0x7fff;
+} else {
+result0 = ((arg01 * arg11) << n) + 0x8000;
+}
+return (result1 & 0x) | (result0 >> 16);
+}
+
 /* context save area (CSA) related helpers */

 static int cdc_increment(target_ulong *psw)
diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index 8940a23..50bb70a 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -115,6 +115,63 @@ void tricore_cpu_dump_state(CPUState *cs, FILE *f,
 tcg_temp_free_i32(helper_tmp);\
 } while (0)

+#define GEN_HELPER_LL(name, ret, arg0, arg1, n) do { \
+TCGv arg00 = tcg_temp_new(); \
+TCGv arg01 = tcg_temp_new(); \
+TCGv arg11 = tcg_temp_new(); \
+tcg_gen_sari_tl(arg00, arg0, 16);\
+tcg_gen_ext16s_tl(arg01, arg0);  \
+tcg_gen_ext16s_tl(arg11, arg1);  \
+gen_helper_##name(ret, arg00, arg01, arg11, arg11, n);   \
+tcg_temp_free(arg00);\
+tcg_temp_free(arg01);\
+tcg_temp_free(arg11);\
+} while (0)
+
+#define GEN_HELPER_LU(name, ret, arg0, arg1,

[Qemu-devel] [PATCH v2 1/8] target-tricore: Change SSOV/SUOV makro name to SSOV32/SUOV32

[Bastian Koppelmann]

Those makros are exclusively used for 32 bit arithmetics and won't work for
16 bit with two halfwords. So lets get rid of the len parameter and make them
always use 32 bit. Now no token pasting is needed anymore and they can be
regular functions.

Signed-off-by: Bastian Koppelmann 
---
v1 -> v2:
- SSOV32/SUOV32 are now regular functions.

 target-tricore/op_helper.c | 134 -
 1 file changed, 58 insertions(+), 76 deletions(-)

diff --git a/target-tricore/op_helper.c b/target-tricore/op_helper.c
index 4da76ff..f1a8d16 100644
--- a/target-tricore/op_helper.c
+++ b/target-tricore/op_helper.c
@@ -56,118 +56,111 @@ uint32_t helper_circ_update(uint32_t reg, uint32_t off)
 return reg - index + new_index;
 }

-#define SSOV(env, ret, arg, len) do {   \
-int64_t max_pos = INT##len ##_MAX;  \
-int64_t max_neg = INT##len ##_MIN;  \
-if (arg > max_pos) {\
-env->PSW_USB_V = (1 << 31); \
-env->PSW_USB_SV = (1 << 31);\
-ret = (target_ulong)max_pos;\
-} else {\
-if (arg < max_neg) {\
-env->PSW_USB_V = (1 << 31); \
-env->PSW_USB_SV = (1 << 31);\
-ret = (target_ulong)max_neg;\
-} else {\
-env->PSW_USB_V = 0; \
-ret = (target_ulong)arg;\
-}   \
-}   \
-env->PSW_USB_AV = arg ^ arg * 2u;   \
-env->PSW_USB_SAV |= env->PSW_USB_AV;\
-} while (0)
-
-#define SUOV(env, ret, arg, len) do {   \
-int64_t max_pos = UINT##len ##_MAX; \
-if (arg > max_pos) {\
-env->PSW_USB_V = (1 << 31); \
-env->PSW_USB_SV = (1 << 31);\
-ret = (target_ulong)max_pos;\
-} else {\
-if (arg < 0) {  \
-env->PSW_USB_V = (1 << 31); \
-env->PSW_USB_SV = (1 << 31);\
-ret = 0;\
-} else {\
-env->PSW_USB_V = 0; \
-ret = (target_ulong)arg;\
-}   \
- }  \
-env->PSW_USB_AV = arg ^ arg * 2u;   \
-env->PSW_USB_SAV |= env->PSW_USB_AV;\
-} while (0)
+static uint32_t ssov32(CPUTriCoreState *env, int64_t arg)
+{
+uint32_t ret;
+int64_t max_pos = INT32_MAX;
+int64_t max_neg = INT32_MIN;
+if (arg > max_pos) {
+env->PSW_USB_V = (1 << 31);
+env->PSW_USB_SV = (1 << 31);
+ret = (target_ulong)max_pos;
+} else {
+if (arg < max_neg) {
+env->PSW_USB_V = (1 << 31);
+env->PSW_USB_SV = (1 << 31);
+ret = (target_ulong)max_neg;
+} else {
+env->PSW_USB_V = 0;
+ret = (target_ulong)arg;
+}
+}
+env->PSW_USB_AV = arg ^ arg * 2u;
+env->PSW_USB_SAV |= env->PSW_USB_AV;
+return ret;
+}
+
+static uint32_t suov32(CPUTriCoreState *env, int64_t arg)
+{
+uint32_t ret;
+int64_t max_pos = UINT32_MAX;
+if (arg > max_pos) {
+env->PSW_USB_V = (1 << 31);
+env->PSW_USB_SV = (1 << 31);
+ret = (target_ulong)max_pos;
+} else {
+if (arg < 0) {
+env->PSW_USB_V = (1 << 31);
+env->PSW_USB_SV = (1 << 31);
+ret = 0;
+} else {
+env->PSW_USB_V = 0;
+ret = (target_ulong)arg;
+}
+ }
+env->PSW_USB_AV = arg ^ arg * 2u;
+env->PSW_USB_SAV |= env->PSW_USB_AV;
+return ret;
+}


 target_ulong helper_add_ssov(CPUTriCoreState *env, target_ulong r1,
  target_ulong r2)
 {
-target_ulong ret;
 int64_t t1 = sextract64(r1, 0, 32);
 int64_t t2 = sextract64(r2, 0, 32);
 int64_t result = t1 + t2;
-SSOV(env, ret, result, 32);
-return ret;
+return ssov32(env, result);
 }

 target_ulong helper_add_suov(CPUTriCoreState *env, target_ulong r1,
  target_ulong r2)
 {
-target_ulong ret;
 int64_t t1 = extract64(r1, 0, 32);
 int64_t t2 = extract64(r2, 0, 32);
 int64_t result = t1 + t2;
-SUOV(env, ret, result, 32);
-return ret;
+return suov32(env, result);
 }

 target_ulong helper_sub_ssov(CPUTriCoreState *env, target_ulong r1,
  target_ulong r2)
 {
-target_ulong ret;
 int64_t t1 = sextract64(r1, 0, 32);
 int64_t t2 = sextract64(r2, 0

[Qemu-devel] [PATCH v2 2/8] target-tricore: Add instructions of RR opcode format, that have 0xb as the first opcode

[Bastian Koppelmann]

Add instructions of RR opcode format, that have 0xb as the first opcode.
Add helper functions, for hword and byte arithmetics:
* add_h_ssov/suov: Add two halfword and saturate on overflow.
* sub_h_ssov/suov: Sub two halfword and saturate on overflow.
* absdif_h_ssov: Compute absolute difference for halfwords and saturate on 
overflow.
* abs_h_ssov/suov: Compute absolute value for two halfwords and saturate on 
overflow.
* abs_b/h: Compute absolute value for four/two bytes/halfwords
* absdif_b/h: Compute absolute difference for four/two bytes/halfwords
* add_b/h: Add four/two bytes/halfwords.
* sub_b/h: Sub four/two bytes/halfwords.
* eq_b/h: Compare four/two bytes/halfwords with four/two bytes/halfwords on
  equality and set all bits of to either one ore zero.
* eqany_b/h: Compare four/two bytes/halfwords with four/two bytes/halfwords 
on equality.
* lt_b/bu/h/hu: Compare four/two bytes/halfwords with four/two 
bytes/halfwords
on less than signed and unsigned.
* max_b/bu/h/hu: Calculate max for four/two bytes/halfwords signed and 
unsigned.
* min_b/bu/h/hu: Calculate min for four/two bytes/halfwords signed and 
unsigned.
Add helper function abs_ssov, that computes the absolute value for a 32 bit 
integer and saturates on overflow.
Add microcode generator functions:
* gen_sub_CC: Caluclates sub and sets the carry bit.
* gen_subc_CC: Caluclates sub and carry and sets the carry bit
* gen_abs: Compute absolute value for a 32 bit integer.
* gen_cond_w: Compares two 32 bit values on cond and sets result either 
zero or all bits one.

OPC2_32_RR_MIN switched with OPC2_32_RR_MIN_U.

Signed-off-by: Bastian Koppelmann 
---
v1 -> v2:
- gen_cond_w now uses neg and saves a temp.
- SSOV16/SUOV16 are now regular functions.
- Use TCG_CALL_NO_RWG_SE for all helpers not using globals.

 target-tricore/helper.h  |  32 +++
 target-tricore/op_helper.c   | 525 +++
 target-tricore/translate.c   | 383 
 target-tricore/tricore-opcodes.h |   4 +-
 4 files changed, 942 insertions(+), 2 deletions(-)

diff --git a/target-tricore/helper.h b/target-tricore/helper.h
index 6c07bd7..df87c0d 100644
--- a/target-tricore/helper.h
+++ b/target-tricore/helper.h
@@ -18,8 +18,12 @@
 /* Arithmetic */
 DEF_HELPER_3(add_ssov, i32, env, i32, i32)
 DEF_HELPER_3(add_suov, i32, env, i32, i32)
+DEF_HELPER_3(add_h_ssov, i32, env, i32, i32)
+DEF_HELPER_3(add_h_suov, i32, env, i32, i32)
 DEF_HELPER_3(sub_ssov, i32, env, i32, i32)
 DEF_HELPER_3(sub_suov, i32, env, i32, i32)
+DEF_HELPER_3(sub_h_ssov, i32, env, i32, i32)
+DEF_HELPER_3(sub_h_suov, i32, env, i32, i32)
 DEF_HELPER_3(mul_ssov, i32, env, i32, i32)
 DEF_HELPER_3(mul_suov, i32, env, i32, i32)
 DEF_HELPER_3(sha_ssov, i32, env, i32, i32)
@@ -32,6 +36,34 @@ DEF_HELPER_4(msub32_ssov, i32, env, i32, i32, i32)
 DEF_HELPER_4(msub32_suov, i32, env, i32, i32, i32)
 DEF_HELPER_4(msub64_ssov, i64, env, i32, i64, i32)
 DEF_HELPER_4(msub64_suov, i64, env, i32, i64, i32)
+DEF_HELPER_3(absdif_h_ssov, i32, env, i32, i32)
+DEF_HELPER_2(abs_ssov, i32, env, i32)
+DEF_HELPER_2(abs_h_ssov, i32, env, i32)
+/* hword/byte arithmetic */
+DEF_HELPER_2(abs_b, i32, env, i32)
+DEF_HELPER_2(abs_h, i32, env, i32)
+DEF_HELPER_3(absdif_b, i32, env, i32, i32)
+DEF_HELPER_3(absdif_h, i32, env, i32, i32)
+DEF_HELPER_3(add_b, i32, env, i32, i32)
+DEF_HELPER_3(add_h, i32, env, i32, i32)
+DEF_HELPER_3(sub_b, i32, env, i32, i32)
+DEF_HELPER_3(sub_h, i32, env, i32, i32)
+DEF_HELPER_FLAGS_2(eq_b, TCG_CALL_NO_RWG_SE, i32, i32, i32)
+DEF_HELPER_FLAGS_2(eq_h, TCG_CALL_NO_RWG_SE, i32, i32, i32)
+DEF_HELPER_FLAGS_2(eqany_b, TCG_CALL_NO_RWG_SE, i32, i32, i32)
+DEF_HELPER_FLAGS_2(eqany_h, TCG_CALL_NO_RWG_SE, i32, i32, i32)
+DEF_HELPER_FLAGS_2(lt_b, TCG_CALL_NO_RWG_SE, i32, i32, i32)
+DEF_HELPER_FLAGS_2(lt_bu, TCG_CALL_NO_RWG_SE, i32, i32, i32)
+DEF_HELPER_FLAGS_2(lt_h, TCG_CALL_NO_RWG_SE, i32, i32, i32)
+DEF_HELPER_FLAGS_2(lt_hu, TCG_CALL_NO_RWG_SE, i32, i32, i32)
+DEF_HELPER_FLAGS_2(max_b, TCG_CALL_NO_RWG_SE, i32, i32, i32)
+DEF_HELPER_FLAGS_2(max_bu, TCG_CALL_NO_RWG_SE, i32, i32, i32)
+DEF_HELPER_FLAGS_2(max_h, TCG_CALL_NO_RWG_SE, i32, i32, i32)
+DEF_HELPER_FLAGS_2(max_hu, TCG_CALL_NO_RWG_SE, i32, i32, i32)
+DEF_HELPER_FLAGS_2(min_b, TCG_CALL_NO_RWG_SE, i32, i32, i32)
+DEF_HELPER_FLAGS_2(min_bu, TCG_CALL_NO_RWG_SE, i32, i32, i32)
+DEF_HELPER_FLAGS_2(min_h, TCG_CALL_NO_RWG_SE, i32, i32, i32)
+DEF_HELPER_FLAGS_2(min_hu, TCG_CALL_NO_RWG_SE, i32, i32, i32)
 /* CSA */
 DEF_HELPER_2(call, void, env, i32)
 DEF_HELPER_1(ret, void, env)
diff --git a/target-tricore/op_helper.c b/target-tricore/op_helper.c
index f1a8d16..26e01ed 100644
--- a/target-tricore/op_helper.c
+++ b/target-tricore/op_helper.c
@@ -103,6 +103,66 @@ static uint32_t suov32(CPUTriCoreState *env, int64_t arg)
 return ret;
 }

+static uint32_t ssov16(CPUTriCoreState *env, int32_t hw0, int32_t hw1)

[Qemu-devel] [PATCH v2 7/8] target-tricore: Fix MFCR/MTCR insn and B format offset.

[Bastian Koppelmann]

Fix gen_mtcr using wrong register.
Fix gen_mtcr/mfcr using sign extended offsets.
Fix B format insn using not sign extendend offsets.

Signed-off-by: Bastian Koppelmann 
Reviewed-by: Richard Henderson 
---
 target-tricore/translate.c   | 6 --
 target-tricore/tricore-opcodes.h | 2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index 5691267..8940a23 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -3930,6 +3930,7 @@ static void decode_rlc_opc(CPUTriCoreState *env, 
DisasContext *ctx,
 tcg_gen_addi_tl(cpu_gpr_a[r2], cpu_gpr_a[r1], const16 << 16);
 break;
 case OPC1_32_RLC_MFCR:
+const16 = MASK_OP_RLC_CONST16(ctx->opcode);
 gen_mfcr(env, cpu_gpr_d[r2], const16);
 break;
 case OPC1_32_RLC_MOV:
@@ -3946,7 +3947,8 @@ static void decode_rlc_opc(CPUTriCoreState *env, 
DisasContext *ctx,
 tcg_gen_movi_tl(cpu_gpr_a[r2], const16 << 16);
 break;
 case OPC1_32_RLC_MTCR:
-gen_mtcr(env, ctx, cpu_gpr_d[r2], const16);
+const16 = MASK_OP_RLC_CONST16(ctx->opcode);
+gen_mtcr(env, ctx, cpu_gpr_d[r1], const16);
 break;
 }
 }
@@ -4650,7 +4652,7 @@ static void decode_32Bit_opc(CPUTriCoreState *env, 
DisasContext *ctx)
 case OPC1_32_B_JA:
 case OPC1_32_B_JL:
 case OPC1_32_B_JLA:
-address = MASK_OP_B_DISP24(ctx->opcode);
+address = MASK_OP_B_DISP24_SEXT(ctx->opcode);
 gen_compute_branch(ctx, op1, 0, 0, 0, address);
 break;
 /* Bit-format */
diff --git a/target-tricore/tricore-opcodes.h b/target-tricore/tricore-opcodes.h
index 5274765..1273f3d 100644
--- a/target-tricore/tricore-opcodes.h
+++ b/target-tricore/tricore-opcodes.h
@@ -94,6 +94,8 @@
 /* B Format   */
 #define MASK_OP_B_DISP24(op)   (MASK_BITS_SHIFT(op, 16, 31) + \
(MASK_BITS_SHIFT(op, 8, 15) << 16))
+#define MASK_OP_B_DISP24_SEXT(op)   (MASK_BITS_SHIFT(op, 16, 31) + \
+(MASK_BITS_SHIFT_SEXT(op, 8, 15) << 16))
 /* BIT Format */
 #define MASK_OP_BIT_D(op)  MASK_BITS_SHIFT(op, 28, 31)
 #define MASK_OP_BIT_POS2(op)   MASK_BITS_SHIFT(op, 23, 27)
-- 
2.1.3

[Qemu-devel] [PATCH v2 0/8] TriCore add instructions of RR and RR1 opcode format

[Bastian Koppelmann]

Hi,

here is the next patchset for the TriCore ISA, which steadily moves towards 
being a usable qemu guest.
This patchset first cleans up the SSOV/SUOV makros, which were only suitable 
for 32 bit arithmetic,
to make room for 16bit SSOV/SUOV arithmetic used for the RR insn. These are 
splitted into four patches,
seperated by the first opcode all insn of one patch have. I also added missed 
1.6 insns and fixed some
minor errors. The last patch adds the first half of the RR1 insn.

Cheers,
Bastian

v1 -> v2:
- SSOV32/SUOV32 are now regular functions.
- gen_cond_w now uses neg and saves a temp.
- SSOV16/SUOV16 are now regular functions.
- Use TCG_CALL_NO_RWG_SE for all helpers not using globals.
- Use more compact code for helper_parity. (Thanks Richard!)
- Remove redundant temp creation.
- mul_h/mulm_h/mulr_h: * move arg extraction to tcg.
   * compute psw flags in tcg-op now.
   * helper now use TCG_CALL_NO_RWG_SE flag.

Bastian Koppelmann (8):
  target-tricore: Change SSOV/SUOV makro name to SSOV32/SUOV32
  target-tricore: Add instructions of RR opcode format, that have 0xb as
the first opcode
  target-tricore: Add instructions of RR opcode format, that have 0xf as
the first opcode
  target-tricore: Add instructions of RR opcode format, that have 0x1 as
the first opcode
  target-tricore: Add instructions of RR opcode format, that have 0x4b
as the first opcode
  target-tricore: Add missing 1.6 insn of BOL opcode format
  target-tricore: Fix MFCR/MTCR insn and B format offset.
  target-tricore: Add instructions of RR1 opcode format, that have 0xb3
as first opcode

 target-tricore/helper.h  |   59 +++
 target-tricore/op_helper.c   | 1086 +++---
 target-tricore/translate.c   |  992 +-
 target-tricore/tricore-opcodes.h |   14 +-
 4 files changed, 2069 insertions(+), 82 deletions(-)

--
2.1.3

[Qemu-devel] [PATCH v2 6/8] target-tricore: Add missing 1.6 insn of BOL opcode format

[Bastian Koppelmann]

Some of the 1.6 ISA instructions were still missing. So let's add them.

Signed-off-by: Bastian Koppelmann 
Reviewed-by: Richard Henderson 
---
 target-tricore/translate.c   | 49 +++-
 target-tricore/tricore-opcodes.h |  6 +
 2 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index a2107b5..5691267 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -3335,8 +3335,49 @@ static void decode_bol_opc(CPUTriCoreState *env, 
DisasContext *ctx, int32_t op1)
 case OPC1_32_BOL_ST_W_LONGOFF:
 gen_offset_st(ctx, cpu_gpr_d[r1], cpu_gpr_a[r2], address, MO_LEUL);
 break;
+case OPC1_32_BOL_LD_B_LONGOFF:
+if (tricore_feature(env, TRICORE_FEATURE_16)) {
+gen_offset_ld(ctx, cpu_gpr_d[r1], cpu_gpr_a[r2], address, MO_SB);
+} else {
+/* raise illegal opcode trap */
+}
+break;
+case OPC1_32_BOL_LD_BU_LONGOFF:
+if (tricore_feature(env, TRICORE_FEATURE_16)) {
+gen_offset_ld(ctx, cpu_gpr_d[r1], cpu_gpr_a[r2], address, MO_UB);
+} else {
+/* raise illegal opcode trap */
+}
+break;
+case OPC1_32_BOL_LD_H_LONGOFF:
+if (tricore_feature(env, TRICORE_FEATURE_16)) {
+gen_offset_ld(ctx, cpu_gpr_d[r1], cpu_gpr_a[r2], address, MO_LESW);
+} else {
+/* raise illegal opcode trap */
+}
+break;
+case OPC1_32_BOL_LD_HU_LONGOFF:
+if (tricore_feature(env, TRICORE_FEATURE_16)) {
+gen_offset_ld(ctx, cpu_gpr_d[r1], cpu_gpr_a[r2], address, MO_LEUW);
+} else {
+/* raise illegal opcode trap */
+}
+break;
+case OPC1_32_BOL_ST_B_LONGOFF:
+if (tricore_feature(env, TRICORE_FEATURE_16)) {
+gen_offset_st(ctx, cpu_gpr_d[r1], cpu_gpr_a[r2], address, MO_SB);
+} else {
+/* raise illegal opcode trap */
+}
+break;
+case OPC1_32_BOL_ST_H_LONGOFF:
+if (tricore_feature(env, TRICORE_FEATURE_16)) {
+gen_offset_ld(ctx, cpu_gpr_d[r1], cpu_gpr_a[r2], address, MO_LESW);
+} else {
+/* raise illegal opcode trap */
+}
+break;
 }
-
 }
 
 /* RC format */
@@ -4659,6 +4700,12 @@ static void decode_32Bit_opc(CPUTriCoreState *env, 
DisasContext *ctx)
 case OPC1_32_BOL_LEA_LONGOFF:
 case OPC1_32_BOL_ST_W_LONGOFF:
 case OPC1_32_BOL_ST_A_LONGOFF:
+case OPC1_32_BOL_LD_B_LONGOFF:
+case OPC1_32_BOL_LD_BU_LONGOFF:
+case OPC1_32_BOL_LD_H_LONGOFF:
+case OPC1_32_BOL_LD_HU_LONGOFF:
+case OPC1_32_BOL_ST_B_LONGOFF:
+case OPC1_32_BOL_ST_H_LONGOFF:
 decode_bol_opc(env, ctx, op1);
 break;
 /* BRC Format */
diff --git a/target-tricore/tricore-opcodes.h b/target-tricore/tricore-opcodes.h
index 0badb28..5274765 100644
--- a/target-tricore/tricore-opcodes.h
+++ b/target-tricore/tricore-opcodes.h
@@ -451,6 +451,12 @@ enum {
 OPC1_32_BOL_LEA_LONGOFF  = 0xd9,
 OPC1_32_BOL_ST_W_LONGOFF = 0x59,
 OPC1_32_BOL_ST_A_LONGOFF = 0xb5, /* 1.6 only */
+OPC1_32_BOL_LD_B_LONGOFF = 0x79, /* 1.6 only */
+OPC1_32_BOL_LD_BU_LONGOFF= 0x39, /* 1.6 only */
+OPC1_32_BOL_LD_H_LONGOFF = 0xc9, /* 1.6 only */
+OPC1_32_BOL_LD_HU_LONGOFF= 0xb9, /* 1.6 only */
+OPC1_32_BOL_ST_B_LONGOFF = 0xe9, /* 1.6 only */
+OPC1_32_BOL_ST_H_LONGOFF = 0xf9, /* 1.6 only */
 /* BRC Format */
 OPCM_32_BRC_EQ_NEQ   = 0xdf,
 OPCM_32_BRC_GE   = 0xff,
-- 
2.1.3

[Qemu-devel] [PATCH v2 3/8] target-tricore: Add instructions of RR opcode format, that have 0xf as the first opcode

[Bastian Koppelmann]

Add instructions of RR opcode format, that have 0xf as the first opcode.
Add helper functions:
* clo/z/s: Counts leading ones/zeros/signs.
* clo/z/s_h: Count leading ones/zeros/signs in two haflwords.
* sh/_h: Shifts one/two word/hwords.
* sha/_h: Shifts one/two word/hwords arithmeticly.

Signed-off-by: Bastian Koppelmann 
---
v1 -> v2:
- Use TCG_CALL_NO_RWG_SE for all helpers not using globals.

 target-tricore/helper.h|  12 
 target-tricore/op_helper.c | 160 +
 target-tricore/translate.c |  78 ++
 3 files changed, 250 insertions(+)

diff --git a/target-tricore/helper.h b/target-tricore/helper.h
index df87c0d..3b85c1a 100644
--- a/target-tricore/helper.h
+++ b/target-tricore/helper.h
@@ -64,6 +64,18 @@ DEF_HELPER_FLAGS_2(min_b, TCG_CALL_NO_RWG_SE, i32, i32, i32)
 DEF_HELPER_FLAGS_2(min_bu, TCG_CALL_NO_RWG_SE, i32, i32, i32)
 DEF_HELPER_FLAGS_2(min_h, TCG_CALL_NO_RWG_SE, i32, i32, i32)
 DEF_HELPER_FLAGS_2(min_hu, TCG_CALL_NO_RWG_SE, i32, i32, i32)
+/* count leading ... */
+DEF_HELPER_FLAGS_1(clo, TCG_CALL_NO_RWG_SE, i32, i32)
+DEF_HELPER_FLAGS_1(clo_h, TCG_CALL_NO_RWG_SE, i32, i32)
+DEF_HELPER_FLAGS_1(clz, TCG_CALL_NO_RWG_SE, i32, i32)
+DEF_HELPER_FLAGS_1(clz_h, TCG_CALL_NO_RWG_SE, i32, i32)
+DEF_HELPER_FLAGS_1(cls, TCG_CALL_NO_RWG_SE, i32, i32)
+DEF_HELPER_FLAGS_1(cls_h, TCG_CALL_NO_RWG_SE, i32, i32)
+/* sh */
+DEF_HELPER_FLAGS_2(sh, TCG_CALL_NO_RWG_SE, i32, i32, i32)
+DEF_HELPER_FLAGS_2(sh_h, TCG_CALL_NO_RWG_SE, i32, i32, i32)
+DEF_HELPER_3(sha, i32, env, i32, i32)
+DEF_HELPER_2(sha_h, i32, i32, i32)
 /* CSA */
 DEF_HELPER_2(call, void, env, i32)
 DEF_HELPER_1(ret, void, env)
diff --git a/target-tricore/op_helper.c b/target-tricore/op_helper.c
index 26e01ed..98f28d5 100644
--- a/target-tricore/op_helper.c
+++ b/target-tricore/op_helper.c
@@ -873,6 +873,166 @@ EXTREMA_H_B(min, <)

 #undef EXTREMA_H_B

+uint32_t helper_clo(target_ulong r1)
+{
+return clo32(r1);
+}
+
+uint32_t helper_clo_h(target_ulong r1)
+{
+uint32_t ret_hw0 = extract32(r1, 0, 16);
+uint32_t ret_hw1 = extract32(r1, 16, 16);
+
+ret_hw0 = clo32(ret_hw0 << 16);
+ret_hw1 = clo32(ret_hw1 << 16);
+
+if (ret_hw0 > 16) {
+ret_hw0 = 16;
+}
+if (ret_hw1 > 16) {
+ret_hw1 = 16;
+}
+
+return ret_hw0 | (ret_hw1 << 16);
+}
+
+uint32_t helper_clz(target_ulong r1)
+{
+return clz32(r1);
+}
+
+uint32_t helper_clz_h(target_ulong r1)
+{
+uint32_t ret_hw0 = extract32(r1, 0, 16);
+uint32_t ret_hw1 = extract32(r1, 16, 16);
+
+ret_hw0 = clz32(ret_hw0 << 16);
+ret_hw1 = clz32(ret_hw1 << 16);
+
+if (ret_hw0 > 16) {
+ret_hw0 = 16;
+}
+if (ret_hw1 > 16) {
+ret_hw1 = 16;
+}
+
+return ret_hw0 | (ret_hw1 << 16);
+}
+
+uint32_t helper_cls(target_ulong r1)
+{
+return clrsb32(r1);
+}
+
+uint32_t helper_cls_h(target_ulong r1)
+{
+uint32_t ret_hw0 = extract32(r1, 0, 16);
+uint32_t ret_hw1 = extract32(r1, 16, 16);
+
+ret_hw0 = clrsb32(ret_hw0 << 16);
+ret_hw1 = clrsb32(ret_hw1 << 16);
+
+if (ret_hw0 > 15) {
+ret_hw0 = 15;
+}
+if (ret_hw1 > 15) {
+ret_hw1 = 15;
+}
+
+return ret_hw0 | (ret_hw1 << 16);
+}
+
+uint32_t helper_sh(target_ulong r1, target_ulong r2)
+{
+int32_t shift_count = sextract32(r2, 0, 6);
+
+if (shift_count == -32) {
+return 0;
+} else if (shift_count < 0) {
+return r1 >> -shift_count;
+} else {
+return r1 << shift_count;
+}
+}
+
+uint32_t helper_sh_h(target_ulong r1, target_ulong r2)
+{
+int32_t ret_hw0, ret_hw1;
+int32_t shift_count;
+
+shift_count = sextract32(r2, 0, 5);
+
+if (shift_count == -16) {
+return 0;
+} else if (shift_count < 0) {
+ret_hw0 = extract32(r1, 0, 16) >> -shift_count;
+ret_hw1 = extract32(r1, 16, 16) >> -shift_count;
+return (ret_hw0 & 0x) | (ret_hw1 << 16);
+} else {
+ret_hw0 = extract32(r1, 0, 16) << shift_count;
+ret_hw1 = extract32(r1, 16, 16) << shift_count;
+return (ret_hw0 & 0x) | (ret_hw1 << 16);
+}
+}
+
+uint32_t helper_sha(CPUTriCoreState *env, target_ulong r1, target_ulong r2)
+{
+int32_t shift_count;
+int64_t result, t1;
+uint32_t ret;
+
+shift_count = sextract32(r2, 0, 6);
+t1 = sextract32(r1, 0, 32);
+
+if (shift_count == 0) {
+env->PSW_USB_C = env->PSW_USB_V = 0;
+ret = r1;
+} else if (shift_count == -32) {
+env->PSW_USB_C = r1;
+env->PSW_USB_V = 0;
+ret = t1 >> 31;
+} else if (shift_count > 0) {
+result = t1 << shift_count;
+/* calc carry */
+env->PSW_USB_C = ((result & 0x) != 0);
+/* calc v */
+env->PSW_USB_V = (((result > 0x7fffLL) ||
+   (result < -0x8000LL)) << 31);
+/* calc sv */
+env->PSW_USB_SV |= env->PSW_USB_V;
+ret = (uint32_t)result;
+} 

[Qemu-devel] [PATCH v2 5/8] target-tricore: Add instructions of RR opcode format, that have 0x4b as the first opcode

[Bastian Koppelmann]

Add instructions of RR opcode format, that have 0x4b as the first opcode.
Add helper functions:
* parity: Calculates the parity bits for every byte of a 32 int.
* bmerge/bsplit: Merges two regs into one bitwise/Splits one reg into two 
bitwise.
* unpack: unpack a IEEE 754 single precision floating point number as 
exponent and mantissa.
* dvinit_b_13/131: (ISA v1.3/v1.31)Prepare operands for a divide operation,
   where the quotient result is guaranteed to fit into 8 
bit.
* dvinit_h_13/131: (ISA v1.3/v1.31)Prepare operands for a divide operation,
   where the quotient result is guaranteed to fit into 16 
bit.
OPCM_32_RR_FLOAT -> OPCM_32_RR_DIVIDE.

Signed-off-by: Bastian Koppelmann 
---
v1 -> v2:
- Use more compact code for helper_parity. (Thanks Richard!)
- decode_rr_divide: remove redundant temp creation.

 target-tricore/helper.h  |  11 +++
 target-tricore/op_helper.c   | 195 +++
 target-tricore/translate.c   | 183 
 target-tricore/tricore-opcodes.h |   2 +-
 4 files changed, 390 insertions(+), 1 deletion(-)

diff --git a/target-tricore/helper.h b/target-tricore/helper.h
index 3b85c1a..f45600d 100644
--- a/target-tricore/helper.h
+++ b/target-tricore/helper.h
@@ -76,6 +76,17 @@ DEF_HELPER_FLAGS_2(sh, TCG_CALL_NO_RWG_SE, i32, i32, i32)
 DEF_HELPER_FLAGS_2(sh_h, TCG_CALL_NO_RWG_SE, i32, i32, i32)
 DEF_HELPER_3(sha, i32, env, i32, i32)
 DEF_HELPER_2(sha_h, i32, i32, i32)
+/* merge/split/parity */
+DEF_HELPER_2(bmerge, i32, i32, i32)
+DEF_HELPER_1(bsplit, i64, i32)
+DEF_HELPER_1(parity, i32, i32)
+/* float */
+DEF_HELPER_1(unpack, i64, i32)
+/* dvinit */
+DEF_HELPER_3(dvinit_b_13, i64, env, i32, i32)
+DEF_HELPER_3(dvinit_b_131, i64, env, i32, i32)
+DEF_HELPER_3(dvinit_h_13, i64, env, i32, i32)
+DEF_HELPER_3(dvinit_h_131, i64, env, i32, i32)
 /* CSA */
 DEF_HELPER_2(call, void, env, i32)
 DEF_HELPER_1(ret, void, env)
diff --git a/target-tricore/op_helper.c b/target-tricore/op_helper.c
index 98f28d5..63d2d56 100644
--- a/target-tricore/op_helper.c
+++ b/target-tricore/op_helper.c
@@ -1033,6 +1033,201 @@ uint32_t helper_sha_h(target_ulong r1, target_ulong r2)
 }
 }

+uint32_t helper_bmerge(target_ulong r1, target_ulong r2)
+{
+uint32_t i, ret;
+
+ret = 0;
+for (i = 0; i < 16; i++) {
+ret |= (r1 & 1) << (2 * i + 1);
+ret |= (r2 & 1) << (2 * i);
+r1 = r1 >> 1;
+r2 = r2 >> 1;
+}
+return ret;
+}
+
+uint64_t helper_bsplit(uint32_t r1)
+{
+int32_t i;
+uint64_t ret;
+
+ret = 0;
+for (i = 0; i < 32; i = i + 2) {
+/* even */
+ret |= (r1 & 1) << (i/2);
+r1 = r1 >> 1;
+/* odd */
+ret |= (uint64_t)(r1 & 1) << (i/2 + 32);
+r1 = r1 >> 1;
+}
+return ret;
+}
+
+uint32_t helper_parity(target_ulong r1)
+{
+uint32_t ret;
+uint32_t nOnes, i;
+
+ret = 0;
+nOnes = 0;
+for (i = 0; i < 8; i++) {
+ret ^= (r1 & 1);
+r1 = r1 >> 1;
+}
+/* second byte */
+nOnes = 0;
+for (i = 0; i < 8; i++) {
+nOnes ^= (r1 & 1);
+r1 = r1 >> 1;
+}
+ret |= nOnes << 8;
+/* third byte */
+nOnes = 0;
+for (i = 0; i < 8; i++) {
+nOnes ^= (r1 & 1);
+r1 = r1 >> 1;
+}
+ret |= nOnes << 16;
+/* fourth byte */
+nOnes = 0;
+for (i = 0; i < 8; i++) {
+nOnes ^= (r1 & 1);
+r1 = r1 >> 1;
+}
+ret |= nOnes << 24;
+
+return ret;
+}
+
+uint64_t helper_unpack(target_ulong arg1)
+{
+int32_t fp_exp  = extract32(arg1, 23, 8);
+int32_t fp_frac = extract32(arg1, 0, 23);
+uint64_t ret;
+int32_t int_exp, int_mant;
+
+if (fp_exp == 255) {
+int_exp = 255;
+int_mant = (fp_frac << 7);
+} else if ((fp_exp == 0) && (fp_frac == 0)) {
+int_exp  = -127;
+int_mant = 0;
+} else if ((fp_exp == 0) && (fp_frac != 0)) {
+int_exp  = -126;
+int_mant = (fp_frac << 7);
+} else {
+int_exp  = fp_exp - 127;
+int_mant = (fp_frac << 7);
+int_mant |= (1 << 30);
+}
+ret = int_exp;
+ret = ret << 32;
+ret |= int_mant;
+
+return ret;
+}
+
+uint64_t helper_dvinit_b_13(CPUTriCoreState *env, uint32_t r1, uint32_t r2)
+{
+uint64_t ret;
+int32_t abs_sig_dividend, abs_base_dividend, abs_divisor;
+int32_t quotient_sign;
+
+ret = sextract32(r1, 0, 32);
+ret = ret << 24;
+quotient_sign = 0;
+if (!((r1 & 0x8000) == (r2 & 0x8000))) {
+ret |= 0xff;
+quotient_sign = 1;
+}
+
+abs_sig_dividend = abs(r1) >> 7;
+abs_base_dividend = abs(r1) & 0x7f;
+abs_divisor = abs(r1);
+/* calc overflow */
+env->PSW_USB_V = 0;
+if ((quotient_sign) && (abs_divisor)) {
+env->PSW_USB_V = (((abs_sig_dividend == abs_divisor) &&
+ (abs_base_dividend >= abs_divisor)) ||
+   

Re: [Qemu-devel] [PATCH v2 1/5] linux-aio: queue requests that cannot be submitted

[Paolo Bonzini]

On 17/12/2014 13:34, Paolo Bonzini wrote:
> 
> 
> On 16/12/2014 21:26, Paolo Bonzini wrote:
>>
>>> I could reproduce this very well on a random OS image that I had around.
>>>  This is raw over XFS over dm-crypt, and the image is about 75% sparse
>>> (8.2G used over 35G).  I only get 1-2%, but still it's visible.
>>>
>>> However I can hardly reproduce it when using a partition directly:
>>>
>>>  oldnew
>>> mean 9.9565 9.9636  (+0.07%)
>>> stddev   0.0405 0.0537
>>> min  9.871  9.867
>>> median   9.973  9.971
>>> max  10.01  10.053
>>> count20 20
>>>
>>> I haven't tried removing layers (e.g. fully-allocated XFS image without
>>> dm-crypt).
>>
>> Could not reproduce it with a fully-allocated XFS image, on the contrary
>> the patched QEMU is a bit faster:
>>
>> old  new
>> mean14.83325 14.82660
>> stddev  0.016930 0.010328
>> min 14.819   14.818
>> max 14.854   14.883
>> median  14.8225  14.8255
>> count   20   20
> 
> Same for 50% sparse XFS image, patched QEMU a bit faster:
> 
> old new
> Mean  8.31285 8.30625
> stddev0.03690 0.0
> min 8.277   8.276
> max 8.378   8.382
> median8.292   8.2905
> count   20  20

Today I cannot reproduce it even on the original testcase:

   old new
mean   9.64175 9.60935
stddev 0.15211 0.7
min9.445   9.454
max10.102  9.835
median 9.649.6205
count  20  20

Some notes:

1) do you have Fam's io_get_events patch?  I don't, and the tests
probably should be done with it.

2) "qemu-img bench" probably could also be changed to use AioContext
directly instead of going through GMainLoop.  That said, there is some
low-hanging fruit for GMainLoop that I'll shortly send a patch for.

3) bench_cb does this:

b->sector += b->bufsize;

Should it shift right by >> BDRV_SECTOR_BITS?  Or is it intended to do
the equivalent of a random read benchmark?

Paolo

[Qemu-devel] [PATCH v2 4/8] target-tricore: Add instructions of RR opcode format, that have 0x1 as the first opcode

[Bastian Koppelmann]

Add instructions of RR opcode format, that have 0x1 as the first opcode.

Signed-off-by: Bastian Koppelmann 
Reviewed-by: Richard Henderson 
---
 target-tricore/translate.c | 97 ++
 1 file changed, 97 insertions(+)

diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index ba434c2..d853039 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -4251,6 +4251,97 @@ static void decode_rr_logical_shift(CPUTriCoreState 
*env, DisasContext *ctx)
 tcg_temp_free(temp);
 }

+static void decode_rr_address(CPUTriCoreState *env, DisasContext *ctx)
+{
+uint32_t op2, n;
+int r1, r2, r3;
+TCGv temp;
+
+op2 = MASK_OP_RR_OP2(ctx->opcode);
+r3 = MASK_OP_RR_D(ctx->opcode);
+r2 = MASK_OP_RR_S2(ctx->opcode);
+r1 = MASK_OP_RR_S1(ctx->opcode);
+n = MASK_OP_RR_N(ctx->opcode);
+
+switch (op2) {
+case OPC2_32_RR_ADD_A:
+tcg_gen_add_tl(cpu_gpr_a[r3], cpu_gpr_a[r1], cpu_gpr_a[r2]);
+break;
+case OPC2_32_RR_ADDSC_A:
+temp = tcg_temp_new();
+tcg_gen_shli_tl(temp, cpu_gpr_d[r1], n);
+tcg_gen_add_tl(cpu_gpr_a[r3], cpu_gpr_a[r2], temp);
+tcg_temp_free(temp);
+break;
+case OPC2_32_RR_ADDSC_AT:
+temp = tcg_temp_new();
+tcg_gen_sari_tl(temp, cpu_gpr_d[r1], 3);
+tcg_gen_add_tl(temp, cpu_gpr_a[r2], temp);
+tcg_gen_andi_tl(cpu_gpr_a[r3], temp, 0xFFFC);
+tcg_temp_free(temp);
+break;
+case OPC2_32_RR_EQ_A:
+tcg_gen_setcond_tl(TCG_COND_EQ, cpu_gpr_d[r3], cpu_gpr_a[r1],
+   cpu_gpr_a[r2]);
+break;
+case OPC2_32_RR_EQZ:
+tcg_gen_setcondi_tl(TCG_COND_EQ, cpu_gpr_d[r3], cpu_gpr_a[r1], 0);
+break;
+case OPC2_32_RR_GE_A:
+tcg_gen_setcond_tl(TCG_COND_GEU, cpu_gpr_d[r3], cpu_gpr_a[r1],
+   cpu_gpr_a[r2]);
+break;
+case OPC2_32_RR_LT_A:
+tcg_gen_setcond_tl(TCG_COND_LTU, cpu_gpr_d[r3], cpu_gpr_a[r1],
+   cpu_gpr_a[r2]);
+break;
+case OPC2_32_RR_MOV_A:
+tcg_gen_mov_tl(cpu_gpr_a[r3], cpu_gpr_d[r2]);
+break;
+case OPC2_32_RR_MOV_AA:
+tcg_gen_mov_tl(cpu_gpr_a[r3], cpu_gpr_a[r2]);
+break;
+case OPC2_32_RR_MOV_D:
+tcg_gen_mov_tl(cpu_gpr_d[r3], cpu_gpr_a[r2]);
+break;
+case OPC2_32_RR_NE_A:
+tcg_gen_setcond_tl(TCG_COND_NE, cpu_gpr_d[r3], cpu_gpr_a[r1],
+   cpu_gpr_a[r2]);
+break;
+case OPC2_32_RR_NEZ_A:
+tcg_gen_setcondi_tl(TCG_COND_NE, cpu_gpr_d[r3], cpu_gpr_a[r1], 0);
+break;
+case OPC2_32_RR_SUB_A:
+tcg_gen_sub_tl(cpu_gpr_a[r3], cpu_gpr_a[r1], cpu_gpr_a[r2]);
+break;
+}
+}
+
+static void decode_rr_idirect(CPUTriCoreState *env, DisasContext *ctx)
+{
+uint32_t op2;
+int r1;
+
+op2 = MASK_OP_RR_OP2(ctx->opcode);
+r1 = MASK_OP_RR_S1(ctx->opcode);
+
+switch (op2) {
+case OPC2_32_RR_JI:
+tcg_gen_andi_tl(cpu_PC, cpu_gpr_a[r1], ~0x1);
+break;
+case OPC2_32_RR_JLI:
+tcg_gen_movi_tl(cpu_gpr_a[11], ctx->next_pc);
+tcg_gen_andi_tl(cpu_PC, cpu_gpr_a[r1], ~0x1);
+break;
+case OPC2_32_RR_CALLI:
+gen_helper_1arg(call, ctx->next_pc);
+tcg_gen_andi_tl(cpu_PC, cpu_gpr_a[r1], ~0x1);
+break;
+}
+tcg_gen_exit_tb(0);
+ctx->bstate = BS_BRANCH;
+}
+
 static void decode_32Bit_opc(CPUTriCoreState *env, DisasContext *ctx)
 {
 int op1;
@@ -4488,6 +4579,12 @@ static void decode_32Bit_opc(CPUTriCoreState *env, 
DisasContext *ctx)
 case OPCM_32_RR_LOGICAL_SHIFT:
 decode_rr_logical_shift(env, ctx);
 break;
+case OPCM_32_RR_ADRESS:
+decode_rr_address(env, ctx);
+break;
+case OPCM_32_RR_IDIRECT:
+decode_rr_idirect(env, ctx);
+break;
 }
 }

--
2.1.3

[Qemu-devel] [PATCH 1/3] block: mark AioContext as recursive

[Paolo Bonzini]

AioContext can be accessed recursively, in fact that's what we do with
aio_poll.  Marking the GSource as recursive avoids that GLib blocks it
and unblocks it around every call to aio_dispatch, which is a pretty
expensive operation.

Signed-off-by: Paolo Bonzini 
---
 async.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/async.c b/async.c
index 3939b79..572f239 100644
--- a/async.c
+++ b/async.c
@@ -300,6 +300,7 @@ AioContext *aio_context_new(Error **errp)
 error_setg_errno(errp, -ret, "Failed to initialize event notifier");
 return NULL;
 }
+g_source_set_can_recurse(&ctx->source, true);
 aio_set_event_notifier(ctx, &ctx->notifier,
(EventNotifierHandler *)
event_notifier_test_and_clear);
-- 
2.1.0

[Qemu-devel] [PATCH 0/3] block: three optimizations

[Paolo Bonzini]

These are three unrelated micro-optimizations of the block layer.
The first is only visible on non-dataplane operation, the others
are generic.

Paolo Bonzini (3):
  block: mark AioContext as recursive
  block: do not allocate an iovec per read of a growable/zero_after_eof BDS
  block: replace g_new0 with g_new for bottom half allocation.

 async.c | 11 +++
 block.c | 12 +---
 2 files changed, 12 insertions(+), 11 deletions(-)

-- 
2.1.0

[Qemu-devel] [RFC PATCH 2/3] hw/vfio: amba device support

[Baptiste Reynal]

Add VFIO_DEVICE_TYPE_AMBA.
Differentiate amba and platform devices according to compatible string.
If the device is amba, clocks description is added in the fd.

Signed-off-by: Baptiste Reynal 
---
 hw/arm/sysbus-fdt.c   | 11 +++
 hw/vfio/platform.c| 15 ---
 include/hw/vfio/vfio-common.h |  1 +
 3 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
index 125ba37..41b4c87 100644
--- a/hw/arm/sysbus-fdt.c
+++ b/hw/arm/sysbus-fdt.c
@@ -62,6 +62,8 @@ typedef struct NodeCreationPair {
  *
  * Generates a very simple node with following properties:
  * compatible string, regs, interrupts
+ * And the following properties if it is on an AMBA bus :
+ * clocks, clock-names
  */
 static int add_generic_fdt_node(SysBusDevice *sbdev, void *opaque)
 {
@@ -103,6 +105,15 @@ static int add_generic_fdt_node(SysBusDevice *sbdev, void 
*opaque)
 qemu_fdt_setprop(fdt, nodename, "compatible",
   compat, compat_str_len);
 
+if (vbasedev->type == VFIO_DEVICE_TYPE_AMBA) {
+/* Setup clocks for AMBA device */
+qemu_fdt_setprop_cells(fdt, nodename, "clocks",
+qemu_fdt_getprop_cell(fdt, "/apb-pclk", "phandle"));
+char clock_names[] = "apb_pclk";
+qemu_fdt_setprop(fdt, nodename, "clock-names", clock_names,
+sizeof(clock_names));
+}
+
 reg_attr = g_new(uint64_t, vbasedev->num_regions*4);
 
 for (i = 0; i < vbasedev->num_regions; i++) {
diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 39eef08..0a96178 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -563,8 +563,13 @@ static int vfio_base_device_init(VFIODevice *vbasedev)
 }
 
 /* Check that the host device exists */
-snprintf(path, sizeof(path), "/sys/bus/platform/devices/%s/",
- vbasedev->name);
+if (vbasedev->type == VFIO_DEVICE_TYPE_AMBA) {
+snprintf(path, sizeof(path), "/sys/bus/amba/devices/%s/",
+vbasedev->name);
+} else {
+snprintf(path, sizeof(path), "/sys/bus/platform/devices/%s/",
+vbasedev->name);
+}
 
 if (stat(path, &st) < 0) {
 error_report("vfio: error: no such host device: %s", path);
@@ -661,7 +666,11 @@ static void vfio_platform_realize(DeviceState *dev, Error 
**errp)
 VFIODevice *vbasedev = &vdev->vbasedev;
 int i, ret;
 
-vbasedev->type = VFIO_DEVICE_TYPE_PLATFORM;
+if (strstr(vdev->compat, "arm,primecell")) {
+vbasedev->type = VFIO_DEVICE_TYPE_AMBA;
+} else {
+vbasedev->type = VFIO_DEVICE_TYPE_PLATFORM;
+}
 vbasedev->ops = &vfio_platform_ops;
 
 #ifdef CONFIG_KVM
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 58fd786..2f1b09c 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -49,6 +49,7 @@ extern int vfio_kvm_device_fd;
 enum {
 VFIO_DEVICE_TYPE_PCI = 0,
 VFIO_DEVICE_TYPE_PLATFORM = 1,
+VFIO_DEVICE_TYPE_AMBA = 2,
 };
 
 typedef struct VFIORegion {
-- 
2.1.3

[Qemu-devel] [RFC PATCH 1/3] hw/vfio/sysbus-fdt: generic add_generic_fdt_node

[Baptiste Reynal]

Modify add_calxeda_midway_xgmac_fdt_node to make it more generic.
Add multiple compatible strings support.

Signed-off-by: Baptiste Reynal 
---
 hw/arm/sysbus-fdt.c | 21 -
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
index 15bb50c..125ba37 100644
--- a/hw/arm/sysbus-fdt.c
+++ b/hw/arm/sysbus-fdt.c
@@ -58,12 +58,12 @@ typedef struct NodeCreationPair {
 /* Device Specific Code */
 
 /**
- * add_calxeda_midway_xgmac_fdt_node
+ * add_device_fdt_node
  *
  * Generates a very simple node with following properties:
  * compatible string, regs, interrupts
  */
-static int add_calxeda_midway_xgmac_fdt_node(SysBusDevice *sbdev, void *opaque)
+static int add_generic_fdt_node(SysBusDevice *sbdev, void *opaque)
 {
 PlatformBusFdtData *data = opaque;
 PlatformBusDevice *pbus = data->pbus;
@@ -80,6 +80,18 @@ static int add_calxeda_midway_xgmac_fdt_node(SysBusDevice 
*sbdev, void *opaque)
 VFIODevice *vbasedev = &vdev->vbasedev;
 Object *obj = OBJECT(sbdev);
 
+/*
+ * Process compatible string to deal with multiple strings
+ * (; is replaced by \0)
+ */
+char *compat = g_strdup(vdev->compat);
+compat_str_len = strlen(compat) + 1;
+
+char *semicolon = compat;
+while ((semicolon = strchr(semicolon, ';')) != NULL) {
+*semicolon = '\0';
+}
+
 mmio_base = object_property_get_int(obj, "mmio[0]", NULL);
 
 nodename = g_strdup_printf("%s/%s@%" PRIx64, parent_node,
@@ -88,9 +100,8 @@ static int add_calxeda_midway_xgmac_fdt_node(SysBusDevice 
*sbdev, void *opaque)
 
 qemu_fdt_add_subnode(fdt, nodename);
 
-compat_str_len = strlen(vdev->compat) + 1;
 qemu_fdt_setprop(fdt, nodename, "compatible",
-  vdev->compat, compat_str_len);
+  compat, compat_str_len);
 
 reg_attr = g_new(uint64_t, vbasedev->num_regions*4);
 
@@ -140,7 +151,7 @@ fail:
 
 /* list of supported dynamic sysbus devices */
 static const NodeCreationPair add_fdt_node_functions[] = {
-{TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node},
+{TYPE_VFIO_CALXEDA_XGMAC, add_generic_fdt_node},
 {"", NULL}, /*last element*/
 };
 
-- 
2.1.3

[Qemu-devel] [RFC PATCH 0/3] VFIO support for AMBA devices

[Baptiste Reynal]

The following series add VFIO support for AMBA devices.

It introduces multiple compatible string support to deal with arm,primecell
compatible string.

The VFIOPlatformDevice now checks for this string and performs amba specific
operations if it is present (change path of the device, add clock in the fd).

This patch applies on top of http://git.linaro.org/people/eric.auger/qemu.git
on branch vfio_integ_v9_rc6_official_dynsysbus_v6 for irqfd support.

TODO: The IRQ forwarding doesn't work.

The series has been tested using PL330 device, irq forwarding doesn't work.
(The test hangs on the first IRQ).
The test performed relies on a special guest kernel, which can be found here
http://github.com/virtualopensystems/linux-kvm-arm.git on branch
vfio-platform-v6-guide, with CONFIG_KVM, CONFIG_ARM_SMMU, CONFIG_PL330_DMA
and CONFIG_VOSYS_DMATEST enabled.

Run qemu using this command:
./qemu-system-arm -M virt -m 512M -enable-kvm \
-device 
vfio-pl330,host="2c0a.dma",id=dma,x-irqfd=false,x-forward=false \
-append "earlyprintk console=ttyAMA0" \
-kernel zImage -nographic

Then run DMA test:
mount -t debugfs none /sys/kernel/debug
echo 1 > /sys/kernel/debug/vosys_dmatest/start

Results can be checked using dmesg.

Baptiste Reynal (3):
  hw/vfio/sysbus-fdt: generic add_generic_fdt_node
  hw/vfio: amba device support
  hw/vfio: add pl330 device support

 hw/arm/sysbus-fdt.c   | 34 +-
 hw/vfio/Makefile.objs |  1 +
 hw/vfio/pl330.c   | 41 +
 hw/vfio/platform.c| 15 ---
 include/hw/vfio/vfio-common.h |  1 +
 include/hw/vfio/vfio-pl330.h  | 26 ++
 6 files changed, 110 insertions(+), 8 deletions(-)
 create mode 100644 hw/vfio/pl330.c
 create mode 100644 include/hw/vfio/vfio-pl330.h

-- 
2.1.3

Re: [Qemu-devel] [PATCH] qmp: implement a qmp command get_link_status

[Wolfgang]

Tanks for your tips.

2014-12-17 10:02 GMT+01:00 Markus Armbruster :

> > +if (queues == 0) {
> > +error_set(errp, QERR_DEVICE_NOT_FOUND, name);
> > +return (int64_t) -1;
> > +}
> > +
> > +nc = ncs[0];
> > +ret = ncs[0]->link_down;
> > +
> > +if (nc->peer->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
> > +ret = ncs[0]->peer->link_down;
> > +}
>
> Can you explain why examining only the first queue suffices?
>
>
Normally there should be only one nic with the name of @name.
I could check the number of queues tho verify this.

> +
> > +return (int64_t) ret ? 0 : 1;
>
> Superfluous cast of ret.
>
> > +}
> > +
> >  void qmp_set_link(const char *name, bool up, Error **errp)
> >  {
> >  NetClientState *ncs[MAX_QUEUE_NUM];
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 9ffdcf8..c5321e6 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -1200,6 +1200,21 @@
> >  { 'command': 'inject-nmi' }
> >
> >  ##
> > +# @get_link_status
>
> We have two informational commands named like "get":
> trace-event-set-state and qom-get.  We have many named query-FOO.  But
> they usually don't take an argument.  Hmm.
>

I thought get is more precise the only link_status;


>
> > +#
> > +# Get the current link state of the nics or nic.
> > +#
> > +# @name: name of the nic you get the state of
>
> "nics or nic" (one or more) or "the nic" (just one)?
>
>
It should only nic (just one).
First concept was to get both and the I forgot to change it.


> Doesn't your code above support any network client, not just NICs?
>

I didn't test it with network clients!


>
> > +#
> > +# Return: If link is up 1
> > +# If link is down 0
> > +# If an error occure an empty string.
>
> A QMP command makes the server send either a success response, of the
> type declared in the schema (here: 'returns': 'int'), or an error
> response.
>
> If we have nothing special to say about the error response in the
> schema's command documentation, we say nothing at all.
>
> Bool is a more natural type then int for the answer to "is the link up?"
>

Yes.


>
> We usually make return value objects to facilitate future extensions.
> Perhaps not an issue for a command that answers the "is the link up?"
> question.  Begs the question whether we should we have a more general
> command to query NIC properties.
>
> Should link status be visible in HMP's "info network"?
>

I thought in fact of completeness.


>
> Stefan, what's the QMP equivalent to "info network"?
>
> > +#
> > +# Notes: this is an Proxmox VE extension and not offical part of Qemu.
>
> Really?
>

I pushed it first to this Open Source Project and forgot to erase it.


>
> > +##
> > +{ 'command': 'get_link_status', 'data': {'name': 'str'}, 'returns':
> 'int'}
> > +
> > +##
> >  # @set_link:
> >  #
> >  # Sets the link status of a virtual network adapter.
> > diff --git a/qmp-commands.hx b/qmp-commands.hx
> > index 718dd92..ecc501a 100644
> > --- a/qmp-commands.hx
> > +++ b/qmp-commands.hx
> > @@ -1431,6 +1431,28 @@ Example:
> >  EQMP
> >
> >  {
> > + .name   = "get_link_status",
> > + .args_type  = "name:s",
> > + .mhandler.cmd_new = qmp_marshal_input_get_link_status,
> > +},
> > +
> > +SQMP
> > +get_link_status
> > +
> > +
> > +Get the link status of a network adapter.
> > +
> > +Arguments:
> > +
> > +- "name": network device name (json-string)
> > +
> > +Example:
> > +
> > +-> { "execute": "set_link", "arguments": { "name": "e1000.0" } }
> > +<- { "return": {1} }
> > +EQMP
> > +
> > +{
> >  .name   = "set_link",
> >  .args_type  = "name:s,up:b",
> >  .mhandler.cmd_new = qmp_marshal_input_set_link,
>

[Qemu-devel] [RFC PATCH 3/3] hw/vfio: add pl330 device support

[Baptiste Reynal]

Create a meta-device for PL330 DMA.

Signed-off-by: Baptiste Reynal 
---
 hw/arm/sysbus-fdt.c  |  2 ++
 hw/vfio/Makefile.objs|  1 +
 hw/vfio/pl330.c  | 41 +
 include/hw/vfio/vfio-pl330.h | 26 ++
 4 files changed, 70 insertions(+)
 create mode 100644 hw/vfio/pl330.c
 create mode 100644 include/hw/vfio/vfio-pl330.h

diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
index 41b4c87..1141185 100644
--- a/hw/arm/sysbus-fdt.c
+++ b/hw/arm/sysbus-fdt.c
@@ -28,6 +28,7 @@
 #include "sysemu/sysemu.h"
 #include "hw/vfio/vfio-platform.h"
 #include "hw/vfio/vfio-calxeda-xgmac.h"
+#include "hw/vfio/vfio-pl330.h"
 
 /*
  * internal struct that contains the information to create dynamic
@@ -163,6 +164,7 @@ fail:
 /* list of supported dynamic sysbus devices */
 static const NodeCreationPair add_fdt_node_functions[] = {
 {TYPE_VFIO_CALXEDA_XGMAC, add_generic_fdt_node},
+{TYPE_VFIO_PL330, add_generic_fdt_node},
 {"", NULL}, /*last element*/
 };
 
diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
index 913ab14..be3023b 100644
--- a/hw/vfio/Makefile.objs
+++ b/hw/vfio/Makefile.objs
@@ -3,4 +3,5 @@ obj-$(CONFIG_SOFTMMU) += common.o
 obj-$(CONFIG_PCI) += pci.o
 obj-$(CONFIG_SOFTMMU) += platform.o
 obj-$(CONFIG_SOFTMMU) += calxeda_xgmac.o
+obj-$(CONFIG_SOFTMMU) += pl330.o
 endif
diff --git a/hw/vfio/pl330.c b/hw/vfio/pl330.c
new file mode 100644
index 000..a409024
--- /dev/null
+++ b/hw/vfio/pl330.c
@@ -0,0 +1,41 @@
+#include "hw/vfio/vfio-pl330.h"
+
+static void pl330_realize(DeviceState *dev, Error **errp)
+{
+VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(dev);
+VFIOPl330DeviceClass *k = VFIO_PL330_DEVICE_GET_CLASS(dev);
+
+vdev->compat = g_strdup("arm,pl330;arm,primecell");
+
+k->parent_realize(dev, errp);
+}
+
+static const VMStateDescription vfio_platform_vmstate = {
+.name = TYPE_VFIO_PL330,
+.unmigratable = 1,
+};
+
+static void vfio_pl330_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+VFIOPl330DeviceClass *vpc =
+VFIO_PL330_DEVICE_CLASS(klass);
+vpc->parent_realize = dc->realize;
+dc->realize = pl330_realize;
+dc->desc = "VFIO PL330";
+}
+
+static const TypeInfo vfio_pl330_dev_info = {
+.name = TYPE_VFIO_PL330,
+.parent = TYPE_VFIO_PLATFORM,
+.instance_size = sizeof(VFIOPl330Device),
+.class_init = vfio_pl330_class_init,
+.class_size = sizeof(VFIOPl330DeviceClass),
+};
+
+static void register_pl330_dev_type(void)
+{
+type_register_static(&vfio_pl330_dev_info);
+}
+
+type_init(register_pl330_dev_type)
diff --git a/include/hw/vfio/vfio-pl330.h b/include/hw/vfio/vfio-pl330.h
new file mode 100644
index 000..1cdf039
--- /dev/null
+++ b/include/hw/vfio/vfio-pl330.h
@@ -0,0 +1,26 @@
+#ifndef HW_VFIO_VFIO_PL330
+#define HW_VFIO_VFIO_PL330
+
+#include "hw/vfio/vfio-platform.h"
+
+#define TYPE_VFIO_PL330 "vfio-pl330"
+
+typedef struct VFIOPl330Device {
+VFIOPlatformDevice vdev;
+} VFIOPl330Device;
+
+typedef struct VFIOPl330DeviceClass {
+VFIOPlatformDeviceClass parent_class;
+DeviceRealize parent_realize;
+} VFIOPl330DeviceClass;
+
+#define VFIO_PL330_DEVICE(obj) \
+ OBJECT_CHECK(VFIOPl330Device, (obj), TYPE_VFIO_PL330)
+#define VFIO_PL330_DEVICE_CLASS(klass) \
+ OBJECT_CLASS_CHECK(VFIOPl330DeviceClass, (klass), \
+TYPE_VFIO_PL330)
+#define VFIO_PL330_DEVICE_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(VFIOPl330DeviceClass, (obj), \
+TYPE_VFIO_PL330)
+
+#endif
-- 
2.1.3

Re: [Qemu-devel] [RFC PATCH 1/3] hw/vfio/sysbus-fdt: generic add_generic_fdt_node

[Baptiste Reynal]

On Wed, Dec 17, 2014 at 2:32 PM, Eric Auger  wrote:
>
> On 12/17/2014 11:09 AM, Baptiste Reynal wrote:
> > Modify add_calxeda_midway_xgmac_fdt_node to make it more generic.
> > Add multiple compatible strings support.
> Hi Baptiste,
>
> Although I understand you may be tempted to transform the Calxeda basic
> node creation function into something more generic, we came to the
> current result after long discussions with Alex Graf. This generic
> function was something released in the past but transformed into
> something specific at the end.
>
> Typically the fact the IRQ is edge sensitive or level sensitive may
> depend on the device and it would be arbitrary here to choose either in
> a "generic" node creation.
>
> So the current trend for new devices is to add another entry in the
> add_fdt_node_functions array; genericity target was argued in the past
> and rejected.
>
> If you want to do something going in the direction of genericity I would
> advise you to investigate using Antonios API ([RFC 0/4] VFIO: PLATFORM:
> Return device tree info for a platform device node). This could
> typically enable to get the egde/level sensitive characteritics.
>

I'm not sure I totally understand your point, but creating a new
add_pl330_fdt_node function which will call to
add_calxeda_midway_xgmac_fdt_node and perform amba specific process
(multiple compatible string and clocks) seems to be a good solution for you
?

>
> >
> > Signed-off-by: Baptiste Reynal 
> > ---
> >  hw/arm/sysbus-fdt.c | 21 -
> >  1 file changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
> > index 15bb50c..125ba37 100644
> > --- a/hw/arm/sysbus-fdt.c
> > +++ b/hw/arm/sysbus-fdt.c
> > @@ -58,12 +58,12 @@ typedef struct NodeCreationPair {
> >  /* Device Specific Code */
> >
> >  /**
> > - * add_calxeda_midway_xgmac_fdt_node
> > + * add_device_fdt_node
> >   *
> >   * Generates a very simple node with following properties:
> >   * compatible string, regs, interrupts
> >   */
> > -static int add_calxeda_midway_xgmac_fdt_node(SysBusDevice *sbdev, void
> *opaque)
> > +static int add_generic_fdt_node(SysBusDevice *sbdev, void *opaque)
> >  {
> >  PlatformBusFdtData *data = opaque;
> >  PlatformBusDevice *pbus = data->pbus;
> > @@ -80,6 +80,18 @@ static int
> add_calxeda_midway_xgmac_fdt_node(SysBusDevice *sbdev, void *opaque)
> >  VFIODevice *vbasedev = &vdev->vbasedev;
> >  Object *obj = OBJECT(sbdev);
> >
> > +/*
> > + * Process compatible string to deal with multiple strings
> > + * (; is replaced by \0)
> > + */
> > +char *compat = g_strdup(vdev->compat);
> > +compat_str_len = strlen(compat) + 1;
> > +
> > +char *semicolon = compat;
> > +while ((semicolon = strchr(semicolon, ';')) != NULL) {
> > +*semicolon = '\0';
> > +}
> why can't you format the string directly in the corresponding VFIO AMBA
> device? Formerly we had this problem of conversion since we passed the
> format string in qemu command line.
>
> Best Regards
>
> Eric
>

>From my point of view, formatting the string directly in VFIO AMBA device
will mess a lot with string function, but I can use an array of string in
VFIOPlatformDevice to avoid it.

Best Regards,

Baptiste

> +
> >  mmio_base = object_property_get_int(obj, "mmio[0]", NULL);
> >
> >  nodename = g_strdup_printf("%s/%s@%" PRIx64, parent_node,
> > @@ -88,9 +100,8 @@ static int
> add_calxeda_midway_xgmac_fdt_node(SysBusDevice *sbdev, void *opaque)
> >
> >  qemu_fdt_add_subnode(fdt, nodename);
> >
> > -compat_str_len = strlen(vdev->compat) + 1;
> >  qemu_fdt_setprop(fdt, nodename, "compatible",
> > -  vdev->compat, compat_str_len);
> > +  compat, compat_str_len);
> >
> >  reg_attr = g_new(uint64_t, vbasedev->num_regions*4);
> >
> > @@ -140,7 +151,7 @@ fail:
> >
> >  /* list of supported dynamic sysbus devices */
> >  static const NodeCreationPair add_fdt_node_functions[] = {
> > -{TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node},
> > +{TYPE_VFIO_CALXEDA_XGMAC, add_generic_fdt_node},
> >  {"", NULL}, /*last element*/
> >  };
> >
> >
>
>

[Qemu-devel] [PATCH 3/3] block: replace g_new0 with g_new for bottom half allocation.

[Paolo Bonzini]

This saves about 15% of the clock cycles spent on allocation.  Using the
slice allocator does not add a visible improvement; allocation is faster
than malloc, while freeing seems to be slower.

Signed-off-by: Paolo Bonzini 
---
 async.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/async.c b/async.c
index 572f239..2be88cc 100644
--- a/async.c
+++ b/async.c
@@ -44,10 +44,12 @@ struct QEMUBH {
 QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
 {
 QEMUBH *bh;
-bh = g_new0(QEMUBH, 1);
-bh->ctx = ctx;
-bh->cb = cb;
-bh->opaque = opaque;
+bh = g_new(QEMUBH, 1);
+*bh = (QEMUBH){
+.ctx = ctx,
+.cb = cb,
+.opaque = opaque,
+};
 qemu_mutex_lock(&ctx->bh_lock);
 bh->next = ctx->first_bh;
 /* Make sure that the members are ready before putting bh into list */
-- 
2.1.0

[Qemu-devel] [PATCH 2/3] block: do not allocate an iovec per read of a growable/zero_after_eof BDS

[Paolo Bonzini]

Most reads do not go past the end of the file, and they can use the
input QEMUIOVector instead of creating one.  This removes the
qemu_iovec_* functions from the profile.

Signed-off-by: Paolo Bonzini 
---
 block.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/block.c b/block.c
index 4165d42..58f8042 100644
--- a/block.c
+++ b/block.c
@@ -3034,18 +3034,16 @@ static int coroutine_fn 
bdrv_aligned_preadv(BlockDriverState *bs,
 
 max_nb_sectors = ROUND_UP(MAX(0, total_sectors - sector_num),
   align >> BDRV_SECTOR_BITS);
-if (max_nb_sectors > 0) {
+if (nb_sectors < max_nb_sectors) {
+ret = drv->bdrv_co_readv(bs, sector_num, nb_sectors, qiov);
+} else if (max_nb_sectors > 0) {
 QEMUIOVector local_qiov;
-size_t local_sectors;
-
-max_nb_sectors = MIN(max_nb_sectors, SIZE_MAX / BDRV_SECTOR_BITS);
-local_sectors = MIN(max_nb_sectors, nb_sectors);
 
 qemu_iovec_init(&local_qiov, qiov->niov);
 qemu_iovec_concat(&local_qiov, qiov, 0,
-  local_sectors * BDRV_SECTOR_SIZE);
+  max_nb_sectors * BDRV_SECTOR_SIZE);
 
-ret = drv->bdrv_co_readv(bs, sector_num, local_sectors,
+ret = drv->bdrv_co_readv(bs, sector_num, max_nb_sectors,
  &local_qiov);
 
 qemu_iovec_destroy(&local_qiov);
-- 
2.1.0