Re: [Qemu-devel] [PATCH v4 27/33] nvdimm acpi: save arg3 for NVDIMM device _DSM method

[Michael S. Tsirkin]

On Mon, Oct 19, 2015 at 12:04:48PM +0800, Xiao Guangrong wrote:
> 
> 
> On 10/19/2015 01:16 AM, Michael S. Tsirkin wrote:
> >On Mon, Oct 19, 2015 at 08:54:13AM +0800, Xiao Guangrong wrote:
> >>Check if the input Arg3 is valid then store it into dsm_in if needed
> >>
> >>Signed-off-by: Xiao Guangrong 
> >>---
> >>  hw/acpi/nvdimm.c | 19 +++
> >>  1 file changed, 19 insertions(+)
> >>
> >>diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> >>index 7e99889..b211b8b 100644
> >>--- a/hw/acpi/nvdimm.c
> >>+++ b/hw/acpi/nvdimm.c
> >>@@ -624,10 +624,29 @@ static void nvdimm_build_acpi_devices(NVDIMMState 
> >>*state, GSList *device_list,
> >>
> >>  method = aml_method_serialized("NCAL", 4);
> >>  {
> >>+Aml *ifctx;
> >>+
> >>  aml_append(method, aml_store(aml_arg(0), aml_name("HDLE")));
> >>  aml_append(method, aml_store(aml_arg(1), aml_name("REVS")));
> >>  aml_append(method, aml_store(aml_arg(2), aml_name("FUNC")));
> >>
> >>+/* Arg3 is passed as Package and it has one element? */
> >>+ifctx = aml_if(aml_and(aml_equal(aml_object_type(aml_arg(3)),
> >>+ aml_int(4)),
> >>+   aml_equal(aml_sizeof(aml_arg(3)),
> >>+ aml_int(1;
> >>+{
> >>+/* Local0 = Index(Arg3, 0) */
> >>+aml_append(ifctx, aml_store(aml_index(aml_arg(3), aml_int(0)),
> >>+aml_local(0)));
> >>+/* Local3 = DeRefOf(Local0) */
> >>+aml_append(ifctx, aml_store(aml_derefof(aml_local(0)),
> >>+aml_local(3)));
> >>+/* ARG3 = Local3 */
> >>+aml_append(ifctx, aml_store(aml_local(3), aml_name("ARG3")));
> >>+}
> >>+aml_append(method, ifctx);
> >>+
> >>  aml_append(method, aml_store(aml_int(NOTIFY_VALUE), 
> >> aml_name("NOTI")));
> >>
> >>  aml_append(method, aml_store(aml_name("RLEN"), aml_local(6)));
> >
> >I commented on this patch on v3.
> >It doesn't look like this was addressed.
> >
> 
> Ah... I see no one commented this patch ([PATCH v3 26/32] nvdimm: save arg3 
> for NVDIMM
> device_DSM method) on v3.
> 
> Do you mean we need more and better comment to explain arg3? Or anything else?

Interesting. I have it in my sent mail file, but it doesn't seem to
be on list. I've just resent it, and a couple of other messages
that seem to have disappeared into the ether.

These are the messages I have for v3:

33587   F To Xiao Guangro Re: [PATCH v3 26/32] nvdimm: save arg3 for NVDIMM 
device _DSM method
33588   F To Xiao Guangro Re: [PATCH v3 22/32] nvdimm: init the address region 
used by NVDIMM ACPI
33589   F To Xiao Guangro Re: [PATCH v3 23/32] nvdimm: build ACPI NFIT table
33590   F To Xiao Guangro Re: [PATCH v3 00/32] implement vNVDIMM
33597   F To Xiao Guangro Re: [PATCH v3 23/32] nvdimm: build ACPI NFIT table
33598   F To Xiao Guangro Re: [PATCH v3 00/32] implement vNVDIMM
33599   F To Xiao Guangro Re: [PATCH v3 23/32] nvdimm: build ACPI NFIT table

-- 
MST

Re: [Qemu-devel] [PATCH v3 00/32] implement vNVDIMM

[Michael S. Tsirkin]

On Tue, Oct 13, 2015 at 01:29:48PM +0800, Xiao Guangrong wrote:
> 
> 
> On 10/12/2015 07:55 PM, Michael S. Tsirkin wrote:
> >On Sun, Oct 11, 2015 at 11:52:32AM +0800, Xiao Guangrong wrote:
> >>Changelog in v3:
> >>There is huge change in this version, thank Igor, Stefan, Paolo, Eduardo,
> >>Michael for their valuable comments, the patchset finally gets better shape.
> >
> >Thanks!
> >This needs some changes in coding style, and more comments, to
> >make it easier to maintain going forward.
> 
> Thanks for your review, Michael. I have learned lots of thing from
> your comments.
> 
> >
> >High level comments - I didn't point out all instances,
> >please go over code and locate them yourself.
> >I focused on acpi code in this review.
> 
> Okay, will do.
> 
> >
> > - fix coding style violations, prefix eveything with nvdimm_ etc
> 
> Actually i did not pay attention on naming the stuff which is only internally
> used. Thank you for pointing it out and will fix it in next version.
> 
> > - in apci code, avoid manual memory management/complex pointer math
> 
> I am not very good at ACPI ASL/AML, could you please more detail?

It's about C.

For example:
Foo *foo = acpi_data_push(table, sizeof *foo);
Bar *foo = acpi_data_push(table, sizeof *bar);
is pretty obviously safe, and it doesn't require you to do any
calculations.
char *buf = acpi_data_push(table, sizeof *foo + sizeof *bar);
is worse, now you need:
Bar *bar = (Bar *)(buf + sizeof *foo);
which will corrupt memory if you get the size wrong in push.

> > - comments are needed to document apis & explain what's going on
> > - constants need comments too, refer to text that
> >   can be looked up in acpi spec verbatim
> 
> Indeed, will document carefully.

Re: [Qemu-devel] [Qemu-block] [RFC] transactions: add transaction-wide property

[Markus Armbruster]

John Snow  writes:

> On 10/16/2015 08:23 AM, Stefan Hajnoczi wrote:
>> On Mon, Oct 12, 2015 at 12:50:20PM -0400, John Snow wrote:
>>> Ping -- any consensus on how we should implement the "do-or-die"
>>> argument for transactions that start block jobs? :)
>>>
>>> This patch may look a little hokey in how it boxes arguments, but I can
>>> re-do it on top of Eric Blake's very official way of boxing arguments,
>>> when the QAPI dust settles.
>> 
>> I don't understand what you are trying to do after staring at the email
>> for 5 minutes.  Maybe the other reviewers hit the same problem and
>> haven't responded.
>> 
>> What is the problem you're trying to solve?
>> 
>> Stefan
>> 
>
> Sorry...
>
> What I am trying to do is to add the transactional blocker property to
> the *transaction* command and not as an argument to each individual action.
>
> There was some discussion on this so I wanted to just send an RFC to
> show what I had in mind.

Was it the discussion on @transactional-cancel?  I'm on record
supporting it per transaction rather than per action:
Message-ID: <87mvwd8k9q@blackfin.pond.sub.org>
http://lists.gnu.org/archive/html/qemu-devel/2015-09/msg05948.html

> This series applies on top of Fam's latest series and moves the
> arguments from each action to a transaction-wide property.

Re: [Qemu-devel] [PATCH v3 22/32] nvdimm: init the address region used by NVDIMM ACPI

[Xiao Guangrong]

On 10/19/2015 02:56 PM, Michael S. Tsirkin wrote:

On Sun, Oct 11, 2015 at 11:52:54AM +0800, Xiao Guangrong wrote:

We reserve the memory region 0xFF0 ~ 0xFFF0 for NVDIMM ACPI
which is used as:
- the first page is mapped as MMIO, ACPI write data to this page to
   transfer the control to QEMU

- the second page is RAM-based which used to save the input info of
   _DSM method and QEMU reuse it store output info

- the left is mapped as RAM, it's the buffer returned by _FIT method,
   this is needed by NVDIMM hotplug



Isn't there some way to document this in code, e.g. with
macros?



Yes. It's also documented when DSM memory is created, see
nvdimm_build_dsm_memory() introduced in
[PATCH v4 25/33] nvdimm acpi: init the address region used by DSM


Adding text under docs/specs would also be a good idea.



Yes. A doc has been added in v4.




Signed-off-by: Xiao Guangrong 
---
  hw/i386/pc.c|   3 ++
  hw/mem/Makefile.objs|   2 +-
  hw/mem/nvdimm/acpi.c| 120 
  include/hw/i386/pc.h|   2 +
  include/hw/mem/nvdimm.h |  19 
  5 files changed, 145 insertions(+), 1 deletion(-)
  create mode 100644 hw/mem/nvdimm/acpi.c

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 6694b18..8fea4c3 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1360,6 +1360,9 @@ FWCfgState *pc_memory_init(PCMachineState *pcms,
  exit(EXIT_FAILURE);
  }

+nvdimm_init_memory_state(>nvdimm_memory, system_memory, machine,
+ TARGET_PAGE_SIZE);
+


Shouldn't this be conditional on presence of the nvdimm device?



We will enable hotplug on nvdimm devices in the near future once Linux driver is
ready. I'd keep it here for future development.




  pcms->hotplug_memory.base =
  ROUND_UP(0x1ULL + pcms->above_4g_mem_size, 1ULL << 30);

diff --git a/hw/mem/Makefile.objs b/hw/mem/Makefile.objs
index e0ff328..7310bac 100644
--- a/hw/mem/Makefile.objs
+++ b/hw/mem/Makefile.objs
@@ -1,3 +1,3 @@
  common-obj-$(CONFIG_DIMM) += dimm.o
  common-obj-$(CONFIG_MEM_HOTPLUG) += pc-dimm.o
-common-obj-$(CONFIG_NVDIMM) += nvdimm/nvdimm.o
+common-obj-$(CONFIG_NVDIMM) += nvdimm/nvdimm.o nvdimm/acpi.o
diff --git a/hw/mem/nvdimm/acpi.c b/hw/mem/nvdimm/acpi.c
new file mode 100644
index 000..b640874
--- /dev/null
+++ b/hw/mem/nvdimm/acpi.c
@@ -0,0 +1,120 @@
+/*
+ * NVDIMM ACPI Implementation
+ *
+ * Copyright(C) 2015 Intel Corporation.
+ *
+ * Author:
+ *  Xiao Guangrong 
+ *
+ * NFIT is defined in ACPI 6.0: 5.2.25 NVDIMM Firmware Interface Table (NFIT)
+ * and the DSM specfication can be found at:
+ *   http://pmem.io/documents/NVDIMM_DSM_Interface_Example.pdf
+ *
+ * Currently, it only supports PMEM Virtualization.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see 
+ */
+
+#include "qemu-common.h"
+#include "hw/acpi/acpi.h"
+#include "hw/acpi/aml-build.h"
+#include "hw/mem/nvdimm.h"
+#include "internal.h"
+
+/* System Physical Address Range Structure */
+struct nfit_spa {
+uint16_t type;
+uint16_t length;
+uint16_t spa_index;
+uint16_t flags;
+uint32_t reserved;
+uint32_t proximity_domain;
+uint8_t type_guid[16];
+uint64_t spa_base;
+uint64_t spa_length;
+uint64_t mem_attr;
+} QEMU_PACKED;
+typedef struct nfit_spa nfit_spa;
+
+/* Memory Device to System Physical Address Range Mapping Structure */
+struct nfit_memdev {
+uint16_t type;
+uint16_t length;
+uint32_t nfit_handle;
+uint16_t phys_id;
+uint16_t region_id;
+uint16_t spa_index;
+uint16_t dcr_index;
+uint64_t region_len;
+uint64_t region_offset;
+uint64_t region_dpa;
+uint16_t interleave_index;
+uint16_t interleave_ways;
+uint16_t flags;
+uint16_t reserved;
+} QEMU_PACKED;
+typedef struct nfit_memdev nfit_memdev;
+
+/* NVDIMM Control Region Structure */
+struct nfit_dcr {
+uint16_t type;
+uint16_t length;
+uint16_t dcr_index;
+uint16_t vendor_id;
+uint16_t device_id;
+uint16_t revision_id;
+uint16_t sub_vendor_id;
+uint16_t sub_device_id;
+uint16_t sub_revision_id;
+uint8_t reserved[6];
+uint32_t serial_number;
+uint16_t fic;
+uint16_t num_bcw;
+uint64_t bcw_size;
+uint64_t cmd_offset;
+uint64_t 

Re: [Qemu-devel] QEMU patch to allow VM introspection via libvmi

[Markus Armbruster]

Valerio Aimale  writes:

> On 10/16/15 2:15 AM, Markus Armbruster wrote:
>> vale...@aimale.com writes:
>>
>>> All-
>>>
>>> I've produced a patch for the current QEMU HEAD, for libvmi to
>>> introspect QEMU/KVM VMs.
>>>
>>> Libvmi has patches for the old qeum-kvm fork, inside its source tree:
>>> https://github.com/libvmi/libvmi/tree/master/tools/qemu-kvm-patch
>>>
>>> This patch adds a hmp and a qmp command, "pmemaccess". When the
>>> commands is invoked with a string arguments (a filename), it will open
>>> a UNIX socket and spawn a listening thread.
>>>
>>> The client writes binary commands to the socket, in the form of a c
>>> structure:
>>>
>>> struct request {
>>>   uint8_t type;   // 0 quit, 1 read, 2 write, ... rest reserved
>>>   uint64_t address;   // address to read from OR write to
>>>   uint64_t length;// number of bytes to read OR write
>>> };
>>>
>>> The client receives as a response, either (length+1) bytes, if it is a
>>> read operation, or 1 byte ifit is a write operation.
>>>
>>> The last bytes of a read operation response indicates success (1
>>> success, 0 failure). The single byte returned for a write operation
>>> indicates same (1 success, 0 failure).
>> So, if you ask to read 1 MiB, and it fails, you get back 1 MiB of
>> garbage followed by the "it failed" byte?
> Markus, that appear to be the case. However, I did not write the
> communication protocol between libvmi and qemu. I'm assuming that the
> person that wrote the protocol, did not want to bother with over
> complicating things.
>
> https://github.com/libvmi/libvmi/blob/master/libvmi/driver/kvm/kvm.c
>
> I'm thinking he assumed reads would be small in size and the price of
> reading garbage was less than the price of writing a more complicated
> protocol. I can see his point, confronted with the same problem, I
> might have done the same.

All right, the interface is designed for *small* memory blocks then.

Makes me wonder why he needs a separate binary protocol on a separate
socket.  Small blocks could be done just fine in QMP.

>>> The socket API was written by the libvmi author and it works the with
>>> current libvmi version. The libvmi client-side implementation is at:
>>>
>>> https://github.com/libvmi/libvmi/blob/master/libvmi/driver/kvm/kvm.c
>>>
>>> As many use kvm VM's for introspection, malware and security analysis,
>>> it might be worth thinking about making the pmemaccess a permanent
>>> hmp/qmp command, as opposed to having to produce a patch at each QEMU
>>> point release.
>> Related existing commands: memsave, pmemsave, dump-guest-memory.
>>
>> Can you explain why these won't do for your use case?
> For people who do security analysis there are two use cases, static
> and dynamic analysis. With memsave, pmemsave and dum-guest-memory one
> can do static analysis. I.e. snapshotting a VM and see what was
> happening at that point in time.
> Dynamic analysis require to be able to 'introspect' a VM while it's running.
>
> If you take a snapshot of two people exchanging a glass of water, and
> you happen to take it at the very moment both persons have their hands
> on the glass, it's hard to tell who passed the glass to whom. If you
> have a movie of the same scene, it's obvious who's the giver and who's
> the receiver. Same use case.

I understand the need for introspecting a running guest.  What exactly
makes the existing commands unsuitable for that?

> More to the point, there's a host of C and python frameworks to
> dynamically analyze VMs: volatility, rekal, "drakvuf", etc. They all
> build on top of libvmi. I did not want to reinvent the wheel.

Fair enough.

Front page http://libvmi.com/ claims "Works with Xen, KVM, Qemu, and Raw
memory files."  What exactly is missing for KVM?

> Mind you, 99.9% of people that do dynamic VM analysis use xen. They
> contend that xen has better introspection support. In my case, I did
> not want to bother with dedicating a full server to be a xen domain
> 0. I just wanted to do a quick test by standing up a QEMU/kvm VM, in
> an otherwise purposed server.

I'm not at all against better introspection support in QEMU.  I'm just
trying to understand the problem you're trying to solve with your
patches.

>>> Also, the pmemsave commands QAPI should be changed to be usable with
>>> 64bit VM's
>>>
>>> in qapi-schema.json
>>>
>>> from
>>>
>>> ---
>>> { 'command': 'pmemsave',
>>>'data': {'val': 'int', 'size': 'int', 'filename': 'str'} }
>>> ---
>>>
>>> to
>>>
>>> ---
>>> { 'command': 'pmemsave',
>>>'data': {'val': 'int64', 'size': 'int64', 'filename': 'str'} }
>>> ---
>> In the QAPI schema, 'int' is actually an alias for 'int64'.  Yes, that's
>> confusing.
> I think it's confusing for the HMP parser too. If you have a VM with
> 8Gb of RAM and want to snapshot the whole physical memory, via HMP
> over telnet this is what happens:
>
> $ telnet localhost 1234
> Trying 127.0.0.1...
> Connected to localhost.
> Escape character is '^]'.
> QEMU 

Re: [Qemu-devel] [PATCH V5 0/8] Xilinx DisplayPort.

[Frederic Konrad]

On 16/10/2015 23:57, Alistair Francis wrote:
> On Fri, Oct 16, 2015 at 6:41 AM,   wrote:
>> From: KONRAD Frederic 
>>
>> This is the fifth version of this patch-set of the implementation of the 
>> Xilinx
>> DisplayPort and DPDMA.
>>
>> This fifth version moves some headers files to the right directory.
>>
>> Second patch introduces an AUX bus needed by the DP to read the DPCD.
>> It's also possible to connect an I2C device on it to to I2C through AUX
>> commands. The drivers requires I2C broadcast write to be modeled as well 
>> which
>> seems to be missing currently upstream.
>>
>> The tree can be cloned at:
>> g...@git.greensocs.com:fkonrad/xilinx_dp.git branch xilinx_dp_v5_release
> I can't seem to access this, is it public?
>
> Thanks,
>
> Alistair
oops, sorry for that.
Should be ok now.

Thanks,
Fred
>> Details of the DPDMA part:
>>  * DPDMA is implemented as a QEMU SYSBUS device.
>>  * Interrupts are implemented except the axi error and fifo.
>>
>> Details of the XILINX-DP:
>>  * DP is also implemented as a QEMU SYSBUS. Multiple memory regions are used 
>> to
>>avoid having a single big region as there are holes in the DP memory map.
>>  * An aux-bus has been implemented, it creates a memory map for aux slaves 
>> and
>>has an i2c bus (which is already implemented in QEMU).
>>  * The normal programmable i2c clock and controller implementation is missing
>>from the QEMU tree so the easiest way for us was to implement a dummy-clk
>>driver in the kernel. It's a clock which does nothing but fakes a clock 
>> such
>>that the DPDMA driver works. The patch will be send separately.
>>  * The graphic plane works on channel 3, video on channel 0 and audios on
>>channel 4 and 5.
>>
>> Thanks,
>> Fred
>>
>> V4 -> V5 changes:
>>   * aux:
>> * Move the header include/hw => include/hw/misc
>>   * dpcd:
>> * Move the header hw/display => include/hw/display
>>   * i2c-ddc:
>> * Move the header hw/i2c => include/hw/i2c
>>   * xlnx_dpdma:
>> * Move the header hw/dma => include/hw/dma
>> * Fix some styles issues.
>>   * xlnx_dp:
>> * Move the header hw/display => include/hw/display
>>   * globally:
>> * Rebased on current master (c49d3411faae8ffaab8f7e5db47405a008411c10).
>>
>> V3 -> V4 changes:
>>   * xlnx_dpdma:
>> * Initialize operation_finished during reset.
>> * Add a function to trigger a VSYNC interrupt from the xlnx_dp.
>>   * xlnx_dp:
>> * Fix the default pixman format for video buffer.
>> * Remove unused buffer.
>>   * dpcd:
>> * Add the missing DPCD_LANE_X_STATUS.
>> * Set status field for all ports to avoid driver error.
>> * Use 4 lines by default.
>> * Use guest error in case of an outbound access.
>>   * i2c broadcast:
>> * Use a list of device instead of relying on broadcast field to remove 
>> duped
>>   code.
>>   * other:
>> * rebased on current master (774ee4772b6838b78741ea52d4bf26b8922244c5)
>>
>> V2 -> V3 changes:
>>   * dpcd:
>> * Add a CONFIG_DPCD.
>>   * i2c-ddc:
>> * Fill in VMSD.
>>   * aux:
>> * Remove address field.
>> * Add a CONFIG_AUX.
>>   * dpdma:
>> * Fill in VMSD.
>> * Some coding style changes.
>>   * dp:
>> * Fill in VMSD.
>> * Coding style changes.
>>
>> V1 -> V2 changes:
>>   * xlnx-zynqmp:
>> * Remove the dummy object_property_add_child(..).
>>   * dpcd:
>> * Compile only when the ZYNQMP platform is compiled.
>> * Use qemu_log instead of printf.
>> * Compile test debug traces.
>> * Remove the unused current_reg.
>> * Remove the blank realize.
>> * Use dpcd_ prefixes instead of aux_ prefixes.
>> * Add a reset callback.
>> * Add the VMSD.
>> * Add size constraint in the MemoryRegionOps structure instead of 
>> asserting.
>> * Style fixes.
>>   * aux:
>> * Compile only when the ZYNQMP platform is compiled.
>> * Remove the class init and the class for aux-slave.
>>   * dpdma:
>> * Compile only when the ZYNQMP platform is compiled.
>> * Unify per channel macro in one, simplify the switch case.
>> * Use extractXX.
>> * Make DPDMA_GBL an or'ed register.
>>   * dp:
>> * Compile only when the ZYNQMP platform is compiled.
>> * Don't look at the audio channel count.
>> * Use a third pixman plane when we do blending.
>>   * other:
>> * Drop the useless "console: add qemu_alloc_display_format." patch as
>>   suggested by Gerd.
>> * Rebase on current master (f3e3b083d4c266ea864ae3c83da49d4086857679).
>>
>> KONRAD Frederic (7):
>>   i2cbus: remove unused dev field
>>   introduce aux-bus
>>   i2c: implement broadcast write
>>   introduce dpcd module
>>   introduce xlnx-dpdma
>>   introduce xlnx-dp
>>   arm: xlnx-zynqmp: Add xlnx-dp and xlnx-dpdma
>>
>> Peter Maydell (1):
>>   hw/i2c-ddc.c: Implement DDC I2C slave
>>
>>  default-configs/aarch64-softmmu.mak |3 +
>>  hw/arm/xlnx-zynqmp.c|   20 +
>>  

Re: [Qemu-devel] [PATCH] hw/display/tcx: Remove superfluous OBJECT() typecasts

[Markus Armbruster]

Mark Cave-Ayland  writes:

> On 15/10/15 09:54, Thomas Huth wrote:
>
>> The tcx_initfn() function is already supplied with an
>> Object *obj pointer, so there is no need to cast the
>> state pointer back to an Object pointer all over the
>> place. And while we're at it, also remove the superfluous
>> "return;" statement in this function.
>> 
>> Signed-off-by: Thomas Huth 
[...]
> Acked-by: Mark Cave-Ayland 
>
> Note that I'm in the process of moving and therefore connectivity is
> going to be poor for the next week and a bit - happy for this to go via
> -trivial if that works for everyone?

Sure!

Re: [Qemu-devel] [PATCH v4 28/33] nvdimm acpi: support DSM_FUN_IMPLEMENTED function

[Xiao Guangrong]

On 10/19/2015 03:06 PM, Michael S. Tsirkin wrote:

On Mon, Oct 19, 2015 at 12:39:24PM +0800, Xiao Guangrong wrote:



On 10/19/2015 02:05 AM, Michael S. Tsirkin wrote:

On Mon, Oct 19, 2015 at 08:54:14AM +0800, Xiao Guangrong wrote:

__DSM is defined in ACPI 6.0: 9.14.1 _DSM (Device Specific Method)

Function 0 is a query function. We do not support any function on root
device and only 3 functions are support for NVDIMM device,
DSM_DEV_FUN_NAMESPACE_LABEL_SIZE, DSM_DEV_FUN_GET_NAMESPACE_LABEL_DATA and
DSM_DEV_FUN_SET_NAMESPACE_LABEL_DATA, that means we currently only allow to
access device's Label Namespace

Signed-off-by: Xiao Guangrong 
---
  hw/acpi/nvdimm.c | 184 ++-
  1 file changed, 182 insertions(+), 2 deletions(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index b211b8b..37fea1c 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -260,6 +260,22 @@ static uint32_t nvdimm_slot_to_dcr_index(int slot)
  return nvdimm_slot_to_spa_index(slot) + 1;
  }

+static NVDIMMDevice
+*nvdimm_get_device_by_handle(GSList *list, uint32_t handle)
+{
+for (; list; list = list->next) {
+NVDIMMDevice *nvdimm = list->data;
+int slot = object_property_get_int(OBJECT(nvdimm), DIMM_SLOT_PROP,
+   NULL);
+
+if (nvdimm_slot_to_handle(slot) == handle) {
+return nvdimm;
+}
+}
+
+return NULL;
+}
+
  /*
   * Please refer to ACPI 6.0: 5.2.25.1 System Physical Address Range
   * Structure
@@ -411,6 +427,60 @@ static void nvdimm_build_nfit(GArray *structures, GArray 
*table_offsets,
  /* detailed _DSM design please refer to docs/specs/acpi_nvdimm.txt */
  #define NOTIFY_VALUE  0x99


Again, please prefix everything consistently.


Okay, will do. Sorry for i missed it.





+enum {
+DSM_FUN_IMPLEMENTED = 0,
+
+/* NVDIMM Root Device Functions */
+DSM_ROOT_DEV_FUN_ARS_CAP = 1,
+DSM_ROOT_DEV_FUN_ARS_START = 2,
+DSM_ROOT_DEV_FUN_ARS_QUERY = 3,
+
+/* NVDIMM Device (non-root) Functions */
+DSM_DEV_FUN_SMART = 1,
+DSM_DEV_FUN_SMART_THRESHOLD = 2,
+DSM_DEV_FUN_BLOCK_NVDIMM_FLAGS = 3,
+DSM_DEV_FUN_NAMESPACE_LABEL_SIZE = 4,
+DSM_DEV_FUN_GET_NAMESPACE_LABEL_DATA = 5,
+DSM_DEV_FUN_SET_NAMESPACE_LABEL_DATA = 6,
+DSM_DEV_FUN_VENDOR_EFFECT_LOG_SIZE = 7,
+DSM_DEV_FUN_GET_VENDOR_EFFECT_LOG = 8,
+DSM_DEV_FUN_VENDOR_SPECIFIC = 9,
+};


Does FUN stand for "function"? FUNC or FN is probably better.



Yes.


Please list exact names as they appear in spec so
they can be searched for.


The spec reference was at where this _FUN_ is used, eg:

/*
  * Please refer to DSM specification 4.4.1 Get Namespace Label Size
  * (Function Index 4).
  *
  * It gets the size of Namespace Label data area and the max data size
  * that Get/Set Namespace Label Data functions can transfer.
  */
static void nvdimm_dsm_func_label_size(NVDIMMDevice *nvdimm, GArray *out)

I will follow your ‘single use’ comments below, these definitions will
be dropped, the code will be like this:

switch (function) {
case 4 /* DSM Spec 4.4.1 Get Namespace Label Size Get Namespace Label Size. */:


If it's the same spec, you don't have to repeat it:

/* Encode function according to DSM Spec rev 1.0 */

switch (function) {
 case 4 /* 4.4.1 Get Namespace Label Size Get Namespace Label Size. */:


same for chapter etc.


Okay.




nvdimm_dsm_func_label_size();
case ...
...
};






+
+enum {
+/* Common return status codes. */
+DSM_STATUS_SUCCESS = 0,   /* Success */
+DSM_STATUS_NOT_SUPPORTED = 1, /* Not Supported */
+
+/* NVDIMM Root Device _DSM function return status codes*/
+DSM_ROOT_DEV_STATUS_INVALID_PARAS = 2,/* Invalid Input Parameters */
+DSM_ROOT_DEV_STATUS_FUNCTION_SPECIFIC_ERROR = 3, /* Function-Specific
+Error */
+
+/* NVDIMM Device (non-root) _DSM function return status codes*/
+DSM_DEV_STATUS_NON_EXISTING_MEM_DEV = 2,  /* Non-Existing Memory Device */
+DSM_DEV_STATUS_INVALID_PARAS = 3, /* Invalid Input Parameters */
+DSM_DEV_STATUS_VENDOR_SPECIFIC_ERROR = 4, /* Vendor Specific Error */
+};
+
+/* Current revision supported by DSM specification is 1. */
+#define DSM_REVISION(1)
+
+/*
+ * please refer to ACPI 6.0: 9.14.1 _DSM (Device Specific Method): Return
+ * Value Information:


Drop "please refer to".


Okay.




+ *   if set to zero, no functions are supported (other than function zero)
+ *   for the specified UUID and Revision ID. If set to one, at least one
+ *   additional function is supported.
+ */
+
+/* do not support any function on root. */
+#define ROOT_SUPPORT_FUN (0ULL)


Needs a name that implies the comment somehow.


+#define DIMM_SUPPORT_FUN((1 << DSM_FUN_IMPLEMENTED)   \
+   | (1 << DSM_DEV_FUN_NAMESPACE_LABEL_SIZE)  

Re: [Qemu-devel] [PATCH v3 26/32] nvdimm: save arg3 for NVDIMM device _DSM method

[Xiao Guangrong]

On 10/19/2015 02:50 PM, Michael S. Tsirkin wrote:

On Sun, Oct 11, 2015 at 11:52:58AM +0800, Xiao Guangrong wrote:

Check if the input Arg3 is valid then store it into dsm_in if needed

We only do the save on NVDIMM device since we are not going to support any
function on root device

Signed-off-by: Xiao Guangrong 
---
  hw/mem/nvdimm/acpi.c | 21 -
  1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/hw/mem/nvdimm/acpi.c b/hw/mem/nvdimm/acpi.c
index d9fa0fd..3b9399c 100644
--- a/hw/mem/nvdimm/acpi.c
+++ b/hw/mem/nvdimm/acpi.c
@@ -442,7 +442,7 @@ static void build_nvdimm_devices(NVDIMMState *state, GSList 
*device_list,
  int slot = object_property_get_int(OBJECT(nvdimm), DIMM_SLOT_PROP,
 NULL);
  uint32_t handle = nvdimm_slot_to_handle(slot);
-Aml *dev, *method;
+Aml *dev, *method, *ifctx;

  dev = aml_device("NV%02X", slot);
  aml_append(dev, aml_name_decl("_ADR", aml_int(handle)));
@@ -452,6 +452,24 @@ static void build_nvdimm_devices(NVDIMMState *state, 
GSList *device_list,
  method = aml_method("_DSM", 4);
  {
  SAVE_ARG012_HANDLE_LOCK(method, aml_int(handle));
+
+/* Arg3 is passed as Package and it has one element? */
+ifctx = aml_if(aml_and(aml_equal(aml_object_type(aml_arg(3)),
+ aml_int(4)),
+   aml_equal(aml_sizeof(aml_arg(3)),


aml_arg(3) is used many times below.
Pls give it a name that makes sense (not arg3! what is it for?)



Er. aml_arg(3) is just the fourth parameter of _DSM method. Will add some
comments:

/*
 * The fourth parameter (Arg3) of _DMS is a package which contains a buffer, the
 * layout of the buffer is specified by UUID (Arg0), Revision ID (Arg1) and
 * Function Index (Arg2) which are documented in the DSM specification.
 */


+ aml_int(1;


Pls document AML constants used.
Like this:

 ifctx = aml_if(aml_and(aml_equal(aml_object_type(aml_arg(3)),
  aml_int(4 /* 4 - Package */) ),
aml_equal(aml_sizeof(aml_arg(3)),
  aml_int(1;


+{
+/* Local0 = Index(Arg3, 0) */
+aml_append(ifctx, aml_store(aml_index(aml_arg(3), aml_int(0)),
+aml_local(0)));
+/* Local3 = DeRefOf(Local0) */
+aml_append(ifctx, aml_store(aml_derefof(aml_local(0)),
+aml_local(3)));
+/* ARG3 = Local3 */
+aml_append(ifctx, aml_store(aml_local(3), aml_name("ARG3")));


This isn't a good way to comment things: you are
just adding ASL before the equivalent C.
Pls document what's going on.



Okay... i just thought C is little readable than AML. Will change the comment
to:

/* fetch buffer from the package (Arg3) and store it to DSM memory. */

Thanks.

Re: [Qemu-devel] [PATCH v3 26/32] nvdimm: save arg3 for NVDIMM device _DSM method

[Michael S. Tsirkin]

On Mon, Oct 19, 2015 at 03:14:28PM +0800, Xiao Guangrong wrote:
> 
> 
> On 10/19/2015 02:50 PM, Michael S. Tsirkin wrote:
> >On Sun, Oct 11, 2015 at 11:52:58AM +0800, Xiao Guangrong wrote:
> >>Check if the input Arg3 is valid then store it into dsm_in if needed
> >>
> >>We only do the save on NVDIMM device since we are not going to support any
> >>function on root device
> >>
> >>Signed-off-by: Xiao Guangrong 
> >>---
> >>  hw/mem/nvdimm/acpi.c | 21 -
> >>  1 file changed, 20 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/hw/mem/nvdimm/acpi.c b/hw/mem/nvdimm/acpi.c
> >>index d9fa0fd..3b9399c 100644
> >>--- a/hw/mem/nvdimm/acpi.c
> >>+++ b/hw/mem/nvdimm/acpi.c
> >>@@ -442,7 +442,7 @@ static void build_nvdimm_devices(NVDIMMState *state, 
> >>GSList *device_list,
> >>  int slot = object_property_get_int(OBJECT(nvdimm), DIMM_SLOT_PROP,
> >> NULL);
> >>  uint32_t handle = nvdimm_slot_to_handle(slot);
> >>-Aml *dev, *method;
> >>+Aml *dev, *method, *ifctx;
> >>
> >>  dev = aml_device("NV%02X", slot);
> >>  aml_append(dev, aml_name_decl("_ADR", aml_int(handle)));
> >>@@ -452,6 +452,24 @@ static void build_nvdimm_devices(NVDIMMState *state, 
> >>GSList *device_list,
> >>  method = aml_method("_DSM", 4);
> >>  {
> >>  SAVE_ARG012_HANDLE_LOCK(method, aml_int(handle));
> >>+
> >>+/* Arg3 is passed as Package and it has one element? */
> >>+ifctx = aml_if(aml_and(aml_equal(aml_object_type(aml_arg(3)),
> >>+ aml_int(4)),
> >>+   aml_equal(aml_sizeof(aml_arg(3)),
> >
> >aml_arg(3) is used many times below.
> >Pls give it a name that makes sense (not arg3! what is it for?)
> >
> 
> Er. aml_arg(3) is just the fourth parameter of _DSM method. Will add some
> comments:
> 
> /*
>  * The fourth parameter (Arg3) of _DMS is a package which contains a buffer, 
> the
>  * layout of the buffer is specified by UUID (Arg0), Revision ID (Arg1) and
>  * Function Index (Arg2) which are documented in the DSM specification.
>  */
> 
> >>+ aml_int(1;
> >
> >Pls document AML constants used.
> >Like this:
> >
> > ifctx = aml_if(aml_and(aml_equal(aml_object_type(aml_arg(3)),
> >  aml_int(4 /* 4 - Package */) ),
> >aml_equal(aml_sizeof(aml_arg(3)),
> >  aml_int(1;
> >
> >>+{
> >>+/* Local0 = Index(Arg3, 0) */
> >>+aml_append(ifctx, aml_store(aml_index(aml_arg(3), 
> >>aml_int(0)),
> >>+aml_local(0)));
> >>+/* Local3 = DeRefOf(Local0) */
> >>+aml_append(ifctx, aml_store(aml_derefof(aml_local(0)),
> >>+aml_local(3)));
> >>+/* ARG3 = Local3 */
> >>+aml_append(ifctx, aml_store(aml_local(3), 
> >>aml_name("ARG3")));
> >
> >This isn't a good way to comment things: you are
> >just adding ASL before the equivalent C.
> >Pls document what's going on.
> >
> 
> Okay... i just thought C is little readable than AML. Will change the comment
> to:
> 
> /* fetch buffer from the package (Arg3) and store it to DSM memory. */
> 
> Thanks.

You can use variables to make the logic clear. E.g.:

Aml *pckg = aml_arg(3);
Aml *pckg_idx = aml_local(0);
Aml *pckg_buf = aml_local(3);

aml_append(ifctx, aml_store(aml_index(pckg, aml_int(0)), pckg_idx);
aml_append(ifctx, aml_store(aml_derefof(pckg_idx), pckg_buf));


This is also better than repeating aml_arg(3) many times.

-- 
MST

Re: [Qemu-devel] [PATCH v3 22/32] nvdimm: init the address region used by NVDIMM ACPI

[Michael S. Tsirkin]

On Mon, Oct 19, 2015 at 03:27:21PM +0800, Xiao Guangrong wrote:
> >>+nvdimm_init_memory_state(>nvdimm_memory, system_memory, 
> >>machine,
> >>+ TARGET_PAGE_SIZE);
> >>+
> >
> >Shouldn't this be conditional on presence of the nvdimm device?
> >
> 
> We will enable hotplug on nvdimm devices in the near future once Linux driver 
> is
> ready. I'd keep it here for future development.

No, I don't think we should add stuff unconditionally. If not nvdimm,
some other flag should indicate user intends to hotplug things.

-- 
MST

Re: [Qemu-devel] [PATCH v2] kvm: Allow the Hyper-V vendor ID to be specified

[Paolo Bonzini]

On 16/10/2015 17:38, Alex Williamson wrote:
> According to Microsoft documentation, the signature in the standard
> hypervisor CPUID leaf at 0x4000 identifies the Vendor ID and is
> for reporting and diagnostic purposes only.  We can therefore allow
> the user to change it to whatever they want, within the 12 character
> limit.  Add a new hyperv-vendor-id option to the -cpu flag to allow
> for this, ex:
> 
>  -cpu host,hv_time,hv_vendor_id=KeenlyKVM
> 
> Link: http://msdn.microsoft.com/library/windows/hardware/hh975392
> Signed-off-by: Alex Williamson 
> ---
> 
> v2: Replace abort() with truncating the string, error report updated
> 
> Igor also had the idea of creating a DEFINE_PROP_STRING_LEN property
> where we could enforce the length earlier in the parameter checking.
> If we like that idea, we probably need to do it first since we don't
> want to switch from truncating to erroring between releases.  I can
> work on that if preferred.  Thanks,

I applied this one, because the truncation matches what is done in other
places (ACPI tables, SCSI product/vendor, etc.)

Paolo

Re: [Qemu-devel] [PATCH v4 27/33] nvdimm acpi: save arg3 for NVDIMM device _DSM method

[Michael S. Tsirkin]

On Mon, Oct 19, 2015 at 12:04:48PM +0800, Xiao Guangrong wrote:
> 
> 
> On 10/19/2015 01:16 AM, Michael S. Tsirkin wrote:
> >On Mon, Oct 19, 2015 at 08:54:13AM +0800, Xiao Guangrong wrote:
> >>Check if the input Arg3 is valid then store it into dsm_in if needed
> >>
> >>Signed-off-by: Xiao Guangrong 
> >>---
> >>  hw/acpi/nvdimm.c | 19 +++
> >>  1 file changed, 19 insertions(+)
> >>
> >>diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> >>index 7e99889..b211b8b 100644
> >>--- a/hw/acpi/nvdimm.c
> >>+++ b/hw/acpi/nvdimm.c
> >>@@ -624,10 +624,29 @@ static void nvdimm_build_acpi_devices(NVDIMMState 
> >>*state, GSList *device_list,
> >>
> >>  method = aml_method_serialized("NCAL", 4);
> >>  {
> >>+Aml *ifctx;
> >>+
> >>  aml_append(method, aml_store(aml_arg(0), aml_name("HDLE")));
> >>  aml_append(method, aml_store(aml_arg(1), aml_name("REVS")));
> >>  aml_append(method, aml_store(aml_arg(2), aml_name("FUNC")));
> >>
> >>+/* Arg3 is passed as Package and it has one element? */
> >>+ifctx = aml_if(aml_and(aml_equal(aml_object_type(aml_arg(3)),
> >>+ aml_int(4)),
> >>+   aml_equal(aml_sizeof(aml_arg(3)),
> >>+ aml_int(1;
> >>+{
> >>+/* Local0 = Index(Arg3, 0) */
> >>+aml_append(ifctx, aml_store(aml_index(aml_arg(3), aml_int(0)),
> >>+aml_local(0)));
> >>+/* Local3 = DeRefOf(Local0) */
> >>+aml_append(ifctx, aml_store(aml_derefof(aml_local(0)),
> >>+aml_local(3)));
> >>+/* ARG3 = Local3 */
> >>+aml_append(ifctx, aml_store(aml_local(3), aml_name("ARG3")));
> >>+}
> >>+aml_append(method, ifctx);
> >>+
> >>  aml_append(method, aml_store(aml_int(NOTIFY_VALUE), 
> >> aml_name("NOTI")));
> >>
> >>  aml_append(method, aml_store(aml_name("RLEN"), aml_local(6)));
> >
> >I commented on this patch on v3.
> >It doesn't look like this was addressed.
> >
> 
> Ah... I see no one commented this patch ([PATCH v3 26/32] nvdimm: save arg3 
> for NVDIMM
> device_DSM method) on v3.
> 
> Do you mean we need more and better comment to explain arg3? Or anything else?
> 


I mean don't use ASL to comment C. It's not more readable.
Describe why the code is the way it is. Use variables by preference,
C does not have weird limitations like ASL so you don't need to call
your variables "arg3". What does it hold?

Re: [Qemu-devel] [PATCH v3 00/32] implement vNVDIMM

[Michael S. Tsirkin]

On Sun, Oct 11, 2015 at 11:52:32AM +0800, Xiao Guangrong wrote:
> Changelog in v3:
> There is huge change in this version, thank Igor, Stefan, Paolo, Eduardo,
> Michael for their valuable comments, the patchset finally gets better shape.

Thanks!
This needs some changes in coding style, and more comments, to
make it easier to maintain going forward.

High level comments - I didn't point out all instances,
please go over code and locate them yourself.
I focused on acpi code in this review.

- fix coding style violations, prefix eveything with nvdimm_ etc
- in apci code, avoid manual memory management/complex pointer math
- comments are needed to document apis & explain what's going on
- constants need comments too, refer to text that
  can be looked up in acpi spec verbatim


> - changes from Igor's comments:
>   1) abstract dimm device type from pc-dimm and create nvdimm device based on
>  dimm, then it uses memory backend device as nvdimm's memory and NUMA has
>  easily been implemented.
>   2) let file-backend device support any kind of filesystem not only for
>  hugetlbfs and let it work on file not only for directory which is
>  achieved by extending 'mem-path' - if it's a directory then it works as
>  current behavior, otherwise if it's file then directly allocates memory
>  from it.
>   3) we figure out a unused memory hole below 4G that is 0xFF0 ~ 
>  0xFFF0, this range is large enough for NVDIMM ACPI as build 64-bit
>  ACPI SSDT/DSDT table will break windows XP.
>  BTW, only make SSDT.rev = 2 can not work since the width is only depended
>  on DSDT.rev based on 19.6.28 DefinitionBlock (Declare Definition Block)
>  in ACPI spec:
> | Note: For compatibility with ACPI versions before ACPI 2.0, the bit 
> | width of Integer objects is dependent on the ComplianceRevision of the DSDT.
> | If the ComplianceRevision is less than 2, all integers are restricted to 32 
> | bits. Otherwise, full 64-bit integers are used. The version of the DSDT 
> sets 
> | the global integer width for all integers, including integers in SSDTs.
>   4) use the lowest ACPI spec version to document AML terms.
>   5) use "nvdimm" as nvdimm device name instead of "pc-nvdimm"
> 
> - changes from Stefan's comments:
>   1) do not do endian adjustment in-place since _DSM memory is visible to 
> guest
>   2) use target platform's target page size instead of fixed PAGE_SIZE
>  definition
>   3) lots of code style improvement and typo fixes.
>   4) live migration fix
> - changes from Paolo's comments:
>   1) improve the name of memory region
>   
> - other changes:
>   1) return exact buffer size for _DSM method instead of the page size.
>   2) introduce mutex in NVDIMM ACPI as the _DSM memory is shared by all nvdimm
>  devices.
>   3) NUMA support
>   4) implement _FIT method
>   5) rename "configdata" to "reserve-label-data"
>   6) simplify _DSM arg3 determination
>   7) main changelog update to let it reflect v3.
> 
> Changlog in v2:
> - Use litten endian for DSM method, thanks for Stefan's suggestion
> 
> - introduce a new parameter, @configdata, if it's false, Qemu will
>   build a static and readonly namespace in memory and use it serveing
>   for DSM GET_CONFIG_SIZE/GET_CONFIG_DATA requests. In this case, no
>   reserved region is needed at the end of the @file, it is good for
>   the user who want to pass whole nvdimm device and make its data
>   completely be visible to guest
> 
> - divide the source code into separated files and add maintain info
> 
> BTW, PCOMMIT virtualization on KVM side is work in progress, hopefully will
> be posted on next week
> 
> == Background ==
> NVDIMM (A Non-Volatile Dual In-line Memory Module) is going to be supported
> on Intel's platform. They are discovered via ACPI and configured by _DSM
> method of NVDIMM device in ACPI. There has some supporting documents which
> can be found at:
> ACPI 6: http://www.uefi.org/sites/default/files/resources/ACPI_6.0.pdf
> NVDIMM Namespace: http://pmem.io/documents/NVDIMM_Namespace_Spec.pdf
> DSM Interface Example: 
> http://pmem.io/documents/NVDIMM_DSM_Interface_Example.pdf
> Driver Writer's Guide: 
> http://pmem.io/documents/NVDIMM_Driver_Writers_Guide.pdf
> 
> Currently, the NVDIMM driver has been merged into upstream Linux Kernel and
> this patchset tries to enable it in virtualization field
> 
> == Design ==
> NVDIMM supports two mode accesses, one is PMEM which maps NVDIMM into CPU's
> address space then CPU can directly access it as normal memory, another is
> BLK which is used as block device to reduce the occupying of CPU address
> space
> 
> BLK mode accesses NVDIMM via Command Register window and Data Register window.
> BLK virtualization has high workload since each sector access will cause at
> least two VM-EXIT. So we currently only imperilment vPMEM in this patchset
> 
> --- vPMEM design ---
> We introduce a new device named "nvdimm", it uses memory 

Re: [Qemu-devel] [PATCH v3 22/32] nvdimm: init the address region used by NVDIMM ACPI

[Michael S. Tsirkin]

On Sun, Oct 11, 2015 at 11:52:54AM +0800, Xiao Guangrong wrote:
> We reserve the memory region 0xFF0 ~ 0xFFF0 for NVDIMM ACPI
> which is used as:
> - the first page is mapped as MMIO, ACPI write data to this page to
>   transfer the control to QEMU
> 
> - the second page is RAM-based which used to save the input info of
>   _DSM method and QEMU reuse it store output info
> 
> - the left is mapped as RAM, it's the buffer returned by _FIT method,
>   this is needed by NVDIMM hotplug
> 

Isn't there some way to document this in code, e.g. with
macros?

Adding text under docs/specs would also be a good idea.


> Signed-off-by: Xiao Guangrong 
> ---
>  hw/i386/pc.c|   3 ++
>  hw/mem/Makefile.objs|   2 +-
>  hw/mem/nvdimm/acpi.c| 120 
> 
>  include/hw/i386/pc.h|   2 +
>  include/hw/mem/nvdimm.h |  19 
>  5 files changed, 145 insertions(+), 1 deletion(-)
>  create mode 100644 hw/mem/nvdimm/acpi.c
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 6694b18..8fea4c3 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1360,6 +1360,9 @@ FWCfgState *pc_memory_init(PCMachineState *pcms,
>  exit(EXIT_FAILURE);
>  }
>  
> +nvdimm_init_memory_state(>nvdimm_memory, system_memory, 
> machine,
> + TARGET_PAGE_SIZE);
> +

Shouldn't this be conditional on presence of the nvdimm device?


>  pcms->hotplug_memory.base =
>  ROUND_UP(0x1ULL + pcms->above_4g_mem_size, 1ULL << 30);
>  
> diff --git a/hw/mem/Makefile.objs b/hw/mem/Makefile.objs
> index e0ff328..7310bac 100644
> --- a/hw/mem/Makefile.objs
> +++ b/hw/mem/Makefile.objs
> @@ -1,3 +1,3 @@
>  common-obj-$(CONFIG_DIMM) += dimm.o
>  common-obj-$(CONFIG_MEM_HOTPLUG) += pc-dimm.o
> -common-obj-$(CONFIG_NVDIMM) += nvdimm/nvdimm.o
> +common-obj-$(CONFIG_NVDIMM) += nvdimm/nvdimm.o nvdimm/acpi.o
> diff --git a/hw/mem/nvdimm/acpi.c b/hw/mem/nvdimm/acpi.c
> new file mode 100644
> index 000..b640874
> --- /dev/null
> +++ b/hw/mem/nvdimm/acpi.c
> @@ -0,0 +1,120 @@
> +/*
> + * NVDIMM ACPI Implementation
> + *
> + * Copyright(C) 2015 Intel Corporation.
> + *
> + * Author:
> + *  Xiao Guangrong 
> + *
> + * NFIT is defined in ACPI 6.0: 5.2.25 NVDIMM Firmware Interface Table (NFIT)
> + * and the DSM specfication can be found at:
> + *   http://pmem.io/documents/NVDIMM_DSM_Interface_Example.pdf
> + *
> + * Currently, it only supports PMEM Virtualization.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see 
> 
> + */
> +
> +#include "qemu-common.h"
> +#include "hw/acpi/acpi.h"
> +#include "hw/acpi/aml-build.h"
> +#include "hw/mem/nvdimm.h"
> +#include "internal.h"
> +
> +/* System Physical Address Range Structure */
> +struct nfit_spa {
> +uint16_t type;
> +uint16_t length;
> +uint16_t spa_index;
> +uint16_t flags;
> +uint32_t reserved;
> +uint32_t proximity_domain;
> +uint8_t type_guid[16];
> +uint64_t spa_base;
> +uint64_t spa_length;
> +uint64_t mem_attr;
> +} QEMU_PACKED;
> +typedef struct nfit_spa nfit_spa;
> +
> +/* Memory Device to System Physical Address Range Mapping Structure */
> +struct nfit_memdev {
> +uint16_t type;
> +uint16_t length;
> +uint32_t nfit_handle;
> +uint16_t phys_id;
> +uint16_t region_id;
> +uint16_t spa_index;
> +uint16_t dcr_index;
> +uint64_t region_len;
> +uint64_t region_offset;
> +uint64_t region_dpa;
> +uint16_t interleave_index;
> +uint16_t interleave_ways;
> +uint16_t flags;
> +uint16_t reserved;
> +} QEMU_PACKED;
> +typedef struct nfit_memdev nfit_memdev;
> +
> +/* NVDIMM Control Region Structure */
> +struct nfit_dcr {
> +uint16_t type;
> +uint16_t length;
> +uint16_t dcr_index;
> +uint16_t vendor_id;
> +uint16_t device_id;
> +uint16_t revision_id;
> +uint16_t sub_vendor_id;
> +uint16_t sub_device_id;
> +uint16_t sub_revision_id;
> +uint8_t reserved[6];
> +uint32_t serial_number;
> +uint16_t fic;
> +uint16_t num_bcw;
> +uint64_t bcw_size;
> +uint64_t cmd_offset;
> +uint64_t cmd_size;
> +uint64_t status_offset;
> +uint64_t status_size;
> +uint16_t flags;
> +uint8_t reserved2[6];
> +} 

Re: [Qemu-devel] [PATCH v3 26/32] nvdimm: save arg3 for NVDIMM device _DSM method

[Michael S. Tsirkin]

On Sun, Oct 11, 2015 at 11:52:58AM +0800, Xiao Guangrong wrote:
> Check if the input Arg3 is valid then store it into dsm_in if needed
> 
> We only do the save on NVDIMM device since we are not going to support any
> function on root device
> 
> Signed-off-by: Xiao Guangrong 
> ---
>  hw/mem/nvdimm/acpi.c | 21 -
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/mem/nvdimm/acpi.c b/hw/mem/nvdimm/acpi.c
> index d9fa0fd..3b9399c 100644
> --- a/hw/mem/nvdimm/acpi.c
> +++ b/hw/mem/nvdimm/acpi.c
> @@ -442,7 +442,7 @@ static void build_nvdimm_devices(NVDIMMState *state, 
> GSList *device_list,
>  int slot = object_property_get_int(OBJECT(nvdimm), DIMM_SLOT_PROP,
> NULL);
>  uint32_t handle = nvdimm_slot_to_handle(slot);
> -Aml *dev, *method;
> +Aml *dev, *method, *ifctx;
>  
>  dev = aml_device("NV%02X", slot);
>  aml_append(dev, aml_name_decl("_ADR", aml_int(handle)));
> @@ -452,6 +452,24 @@ static void build_nvdimm_devices(NVDIMMState *state, 
> GSList *device_list,
>  method = aml_method("_DSM", 4);
>  {
>  SAVE_ARG012_HANDLE_LOCK(method, aml_int(handle));
> +
> +/* Arg3 is passed as Package and it has one element? */
> +ifctx = aml_if(aml_and(aml_equal(aml_object_type(aml_arg(3)),
> + aml_int(4)),
> +   aml_equal(aml_sizeof(aml_arg(3)),

aml_arg(3) is used many times below.
Pls give it a name that makes sense (not arg3! what is it for?)

> + aml_int(1;

Pls document AML constants used.
Like this:

ifctx = aml_if(aml_and(aml_equal(aml_object_type(aml_arg(3)),
 aml_int(4 /* 4 - Package */) ),
   aml_equal(aml_sizeof(aml_arg(3)),
 aml_int(1;

> +{
> +/* Local0 = Index(Arg3, 0) */
> +aml_append(ifctx, aml_store(aml_index(aml_arg(3), 
> aml_int(0)),
> +aml_local(0)));
> +/* Local3 = DeRefOf(Local0) */
> +aml_append(ifctx, aml_store(aml_derefof(aml_local(0)),
> +aml_local(3)));
> +/* ARG3 = Local3 */
> +aml_append(ifctx, aml_store(aml_local(3), aml_name("ARG3")));

This isn't a good way to comment things: you are
just adding ASL before the equivalent C.
Pls document what's going on.




> +}
> +aml_append(method, ifctx);
> +
>  NOTIFY_AND_RETURN_UNLOCK(method);
>  }
>  aml_append(dev, method);
> @@ -534,6 +552,7 @@ static void nvdimm_build_acpi_devices(NVDIMMState *state, 
> GSList *device_list,
>  method = aml_method("_DSM", 4);
>  {
>  SAVE_ARG012_HANDLE_LOCK(method, aml_int(0));
> +/* no command we support on ROOT device has Arg3. */
>  NOTIFY_AND_RETURN_UNLOCK(method);
>  }
>  aml_append(dev, method);
> -- 
> 1.8.3.1

Re: [Qemu-devel] [PATCH v4 28/33] nvdimm acpi: support DSM_FUN_IMPLEMENTED function

[Michael S. Tsirkin]

On Mon, Oct 19, 2015 at 12:39:24PM +0800, Xiao Guangrong wrote:
> 
> 
> On 10/19/2015 02:05 AM, Michael S. Tsirkin wrote:
> >On Mon, Oct 19, 2015 at 08:54:14AM +0800, Xiao Guangrong wrote:
> >>__DSM is defined in ACPI 6.0: 9.14.1 _DSM (Device Specific Method)
> >>
> >>Function 0 is a query function. We do not support any function on root
> >>device and only 3 functions are support for NVDIMM device,
> >>DSM_DEV_FUN_NAMESPACE_LABEL_SIZE, DSM_DEV_FUN_GET_NAMESPACE_LABEL_DATA and
> >>DSM_DEV_FUN_SET_NAMESPACE_LABEL_DATA, that means we currently only allow to
> >>access device's Label Namespace
> >>
> >>Signed-off-by: Xiao Guangrong 
> >>---
> >>  hw/acpi/nvdimm.c | 184 
> >> ++-
> >>  1 file changed, 182 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> >>index b211b8b..37fea1c 100644
> >>--- a/hw/acpi/nvdimm.c
> >>+++ b/hw/acpi/nvdimm.c
> >>@@ -260,6 +260,22 @@ static uint32_t nvdimm_slot_to_dcr_index(int slot)
> >>  return nvdimm_slot_to_spa_index(slot) + 1;
> >>  }
> >>
> >>+static NVDIMMDevice
> >>+*nvdimm_get_device_by_handle(GSList *list, uint32_t handle)
> >>+{
> >>+for (; list; list = list->next) {
> >>+NVDIMMDevice *nvdimm = list->data;
> >>+int slot = object_property_get_int(OBJECT(nvdimm), DIMM_SLOT_PROP,
> >>+   NULL);
> >>+
> >>+if (nvdimm_slot_to_handle(slot) == handle) {
> >>+return nvdimm;
> >>+}
> >>+}
> >>+
> >>+return NULL;
> >>+}
> >>+
> >>  /*
> >>   * Please refer to ACPI 6.0: 5.2.25.1 System Physical Address Range
> >>   * Structure
> >>@@ -411,6 +427,60 @@ static void nvdimm_build_nfit(GArray *structures, 
> >>GArray *table_offsets,
> >>  /* detailed _DSM design please refer to docs/specs/acpi_nvdimm.txt */
> >>  #define NOTIFY_VALUE  0x99
> >
> >Again, please prefix everything consistently.
> 
> Okay, will do. Sorry for i missed it.
> 
> >
> >>
> >>+enum {
> >>+DSM_FUN_IMPLEMENTED = 0,
> >>+
> >>+/* NVDIMM Root Device Functions */
> >>+DSM_ROOT_DEV_FUN_ARS_CAP = 1,
> >>+DSM_ROOT_DEV_FUN_ARS_START = 2,
> >>+DSM_ROOT_DEV_FUN_ARS_QUERY = 3,
> >>+
> >>+/* NVDIMM Device (non-root) Functions */
> >>+DSM_DEV_FUN_SMART = 1,
> >>+DSM_DEV_FUN_SMART_THRESHOLD = 2,
> >>+DSM_DEV_FUN_BLOCK_NVDIMM_FLAGS = 3,
> >>+DSM_DEV_FUN_NAMESPACE_LABEL_SIZE = 4,
> >>+DSM_DEV_FUN_GET_NAMESPACE_LABEL_DATA = 5,
> >>+DSM_DEV_FUN_SET_NAMESPACE_LABEL_DATA = 6,
> >>+DSM_DEV_FUN_VENDOR_EFFECT_LOG_SIZE = 7,
> >>+DSM_DEV_FUN_GET_VENDOR_EFFECT_LOG = 8,
> >>+DSM_DEV_FUN_VENDOR_SPECIFIC = 9,
> >>+};
> >
> >Does FUN stand for "function"? FUNC or FN is probably better.
> >
> 
> Yes.
> 
> >Please list exact names as they appear in spec so
> >they can be searched for.
> 
> The spec reference was at where this _FUN_ is used, eg:
> 
> /*
>  * Please refer to DSM specification 4.4.1 Get Namespace Label Size
>  * (Function Index 4).
>  *
>  * It gets the size of Namespace Label data area and the max data size
>  * that Get/Set Namespace Label Data functions can transfer.
>  */
> static void nvdimm_dsm_func_label_size(NVDIMMDevice *nvdimm, GArray *out)
> 
> I will follow your ‘single use’ comments below, these definitions will
> be dropped, the code will be like this:
> 
> switch (function) {
> case 4 /* DSM Spec 4.4.1 Get Namespace Label Size Get Namespace Label Size. 
> */:

If it's the same spec, you don't have to repeat it:

/* Encode function according to DSM Spec rev 1.0 */
> switch (function) {
> case 4 /* 4.4.1 Get Namespace Label Size Get Namespace Label Size. */:

same for chapter etc.

> nvdimm_dsm_func_label_size();
> case ...
> ...
> };
> 
> >
> >
> >
> >>+
> >>+enum {
> >>+/* Common return status codes. */
> >>+DSM_STATUS_SUCCESS = 0,   /* Success */
> >>+DSM_STATUS_NOT_SUPPORTED = 1, /* Not Supported */
> >>+
> >>+/* NVDIMM Root Device _DSM function return status codes*/
> >>+DSM_ROOT_DEV_STATUS_INVALID_PARAS = 2,/* Invalid Input Parameters 
> >>*/
> >>+DSM_ROOT_DEV_STATUS_FUNCTION_SPECIFIC_ERROR = 3, /* Function-Specific
> >>+Error */
> >>+
> >>+/* NVDIMM Device (non-root) _DSM function return status codes*/
> >>+DSM_DEV_STATUS_NON_EXISTING_MEM_DEV = 2,  /* Non-Existing Memory 
> >>Device */
> >>+DSM_DEV_STATUS_INVALID_PARAS = 3, /* Invalid Input Parameters 
> >>*/
> >>+DSM_DEV_STATUS_VENDOR_SPECIFIC_ERROR = 4, /* Vendor Specific Error */
> >>+};
> >>+
> >>+/* Current revision supported by DSM specification is 1. */
> >>+#define DSM_REVISION(1)
> >>+
> >>+/*
> >>+ * please refer to ACPI 6.0: 9.14.1 _DSM (Device Specific Method): Return
> >>+ * Value Information:
> >
> >Drop "please refer to".
> 
> Okay.
> 
> >
> >>+ *   if set to zero, no functions are supported (other than function zero)

Re: [Qemu-devel] [PATCH v3 22/32] nvdimm: init the address region used by NVDIMM ACPI

[Xiao Guangrong]

On 10/19/2015 03:39 PM, Michael S. Tsirkin wrote:

On Mon, Oct 19, 2015 at 03:27:21PM +0800, Xiao Guangrong wrote:

+nvdimm_init_memory_state(>nvdimm_memory, system_memory, machine,
+ TARGET_PAGE_SIZE);
+


Shouldn't this be conditional on presence of the nvdimm device?



We will enable hotplug on nvdimm devices in the near future once Linux driver is
ready. I'd keep it here for future development.


No, I don't think we should add stuff unconditionally. If not nvdimm,
some other flag should indicate user intends to hotplug things.



Actually, it is not unconditionally which is called if parameter "-m aaa, 
maxmem=bbb"
(aaa < bbb) is used. It is on the some path of memoy-hotplug initiation.

Re: [Qemu-devel] [PATCH v3 26/32] nvdimm: save arg3 for NVDIMM device _DSM method

[Xiao Guangrong]

On 10/19/2015 03:47 PM, Michael S. Tsirkin wrote:


aml_arg(3) is used many times below.
Pls give it a name that makes sense (not arg3! what is it for?)



Er. aml_arg(3) is just the fourth parameter of _DSM method. Will add some
comments:

/*
  * The fourth parameter (Arg3) of _DMS is a package which contains a buffer, 
the
  * layout of the buffer is specified by UUID (Arg0), Revision ID (Arg1) and
  * Function Index (Arg2) which are documented in the DSM specification.
  */


+ aml_int(1;


Pls document AML constants used.
Like this:

 ifctx = aml_if(aml_and(aml_equal(aml_object_type(aml_arg(3)),
  aml_int(4 /* 4 - Package */) ),
aml_equal(aml_sizeof(aml_arg(3)),
  aml_int(1;


+{
+/* Local0 = Index(Arg3, 0) */
+aml_append(ifctx, aml_store(aml_index(aml_arg(3), aml_int(0)),
+aml_local(0)));
+/* Local3 = DeRefOf(Local0) */
+aml_append(ifctx, aml_store(aml_derefof(aml_local(0)),
+aml_local(3)));
+/* ARG3 = Local3 */
+aml_append(ifctx, aml_store(aml_local(3), aml_name("ARG3")));


This isn't a good way to comment things: you are
just adding ASL before the equivalent C.
Pls document what's going on.



Okay... i just thought C is little readable than AML. Will change the comment
to:

/* fetch buffer from the package (Arg3) and store it to DSM memory. */

Thanks.


You can use variables to make the logic clear. E.g.:

Aml *pckg = aml_arg(3);
Aml *pckg_idx = aml_local(0);
Aml *pckg_buf = aml_local(3);

aml_append(ifctx, aml_store(aml_index(pckg, aml_int(0)), pckg_idx);
aml_append(ifctx, aml_store(aml_derefof(pckg_idx), pckg_buf));


This is also better than repeating aml_arg(3) many times.



Indeed, it's more clearer now.

Thanks for your review and really appreciate for your patience, Michael!

Re: [Qemu-devel] [PATCH v2 3/3] virtio-blk: switch off scsi-passthrough by default

[Cornelia Huck]

On Mon, 19 Oct 2015 14:57:31 +0300
"Michael S. Tsirkin"  wrote:

> On Mon, Oct 19, 2015 at 01:53:50PM +0200, Cornelia Huck wrote:
> > On Sun, 18 Oct 2015 10:59:59 +0300
> > "Michael S. Tsirkin"  wrote:
> > 
> > > On Fri, Oct 16, 2015 at 01:07:28PM +0200, Christian Borntraeger wrote:
> > 
> > > > Lets keep this patch as is to have scsi=off as default for virtio 1.0
> > > > 
> > > > (some iotests do fail because of this)
> > > > 
> > > > Christian
> > > 
> > > What fails, exactly?
> > 
> > For example, testcase 068 (on s390x, since we default virtio-1 to on):
> 
> I see, thanks.
> So it's just the assertion that we have in code that fires.
> Sure, scsi must be off by default before virtio 1 is on.

And -hda always assumes defaults...

[Qemu-devel] [PATCHv3] fw_cfg: Define a static signature to be returned on DMA port reads

[Kevin O'Connor]

Return a static signature ("QEMU CFG") if the guest does a read to the
DMA address io register.

Signed-off-by: Kevin O'Connor 
Reviewed-by: Laszlo Ersek 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Gerd Hoffmann 
---

Gerd/Marc - if you respin the fw_cfg series, I believe this patch will
avoid the Windows compile error.

Changes since v2:
 - use /* */ comments instead of C++ comments
 - Use ULL on 64bit constants

Changes since v1:
 - use extract64()
 - reword documentation to be more clear

---
 docs/specs/fw_cfg.txt |  3 +++
 hw/nvram/fw_cfg.c | 14 --
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
index 536909a..b8c794f 100644
--- a/docs/specs/fw_cfg.txt
+++ b/docs/specs/fw_cfg.txt
@@ -93,6 +93,9 @@ by selecting the "signature" item using key 0x 
(FW_CFG_SIGNATURE),
 and reading four bytes from the data register. If the fw_cfg device is
 present, the four bytes read will contain the characters "QEMU".
 
+If the DMA interface is available, then reading the DMA Address
+Register returns 0x51454d5520434647 ("QEMU CFG" in big-endian format).
+
 === Revision / feature bitmap (Key 0x0001, FW_CFG_ID) ===
 
 A 32-bit little-endian unsigned int, this item is used to check for enabled
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 91829d4..73b0a81 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -53,6 +53,8 @@
 #define FW_CFG_DMA_CTL_SKIP0x04
 #define FW_CFG_DMA_CTL_SELECT  0x08
 
+#define FW_CFG_DMA_SIGNATURE 0x51454d5520434647ULL /* "QEMU CFG" */
+
 typedef struct FWCfgEntry {
 uint32_t len;
 uint8_t *data;
@@ -397,6 +399,13 @@ static void fw_cfg_dma_transfer(FWCfgState *s)
 trace_fw_cfg_read(s, 0);
 }
 
+static uint64_t fw_cfg_dma_mem_read(void *opaque, hwaddr addr,
+unsigned size)
+{
+/* Return a signature value (and handle various read sizes) */
+return extract64(FW_CFG_DMA_SIGNATURE, (8 - addr - size) * 8, size * 8);
+}
+
 static void fw_cfg_dma_mem_write(void *opaque, hwaddr addr,
  uint64_t value, unsigned size)
 {
@@ -420,8 +429,8 @@ static void fw_cfg_dma_mem_write(void *opaque, hwaddr addr,
 static bool fw_cfg_dma_mem_valid(void *opaque, hwaddr addr,
   unsigned size, bool is_write)
 {
-return is_write && ((size == 4 && (addr == 0 || addr == 4)) ||
-(size == 8 && addr == 0));
+return !is_write || ((size == 4 && (addr == 0 || addr == 4)) ||
+ (size == 8 && addr == 0));
 }
 
 static bool fw_cfg_data_mem_valid(void *opaque, hwaddr addr,
@@ -492,6 +501,7 @@ static const MemoryRegionOps fw_cfg_comb_mem_ops = {
 };
 
 static const MemoryRegionOps fw_cfg_dma_mem_ops = {
+.read = fw_cfg_dma_mem_read,
 .write = fw_cfg_dma_mem_write,
 .endianness = DEVICE_BIG_ENDIAN,
 .valid.accepts = fw_cfg_dma_mem_valid,
-- 
2.4.3

[Qemu-devel] [PATCH v2 3/3] ui/curses: Fix pageup/pagedown on -curses

[OGAWA Hirofumi]

Current KEY_NPAGE/KEY_PPAGE handling is broken on -curses. Those uses
"GREY", but "KEY_MASK" masked out "GREY".

To fix, we have to use correct mask value - SCANCODE_KEYMASK.

Then, this adds support of "shift + pageup/pagedown". With this,
-curses mode can use scroll-up/down as usual like other display modes.

Signed-off-by: OGAWA Hirofumi 
---

 ui/curses_keys.h |8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff -puN ui/curses_keys.h~support-curses-pageup ui/curses_keys.h
--- qemu/ui/curses_keys.h~support-curses-pageup 2015-10-19 21:18:09.107732122 
+0900
+++ qemu-hirofumi/ui/curses_keys.h  2015-10-19 21:18:09.111732112 +0900
@@ -29,8 +29,7 @@
 #include "keymaps.h"
 
 
-#define KEY_RELEASE 0x80
-#define KEY_MASK0x7f
+#define KEY_MASKSCANCODE_KEYMASK
 #define GREY_CODE   0xe0
 #define GREYSCANCODE_GREY
 #define SHIFT_CODE  0x2a
@@ -60,6 +59,8 @@ static const int curses2keysym[CURSES_KE
 ['\n'] = KEY_ENTER,
 [27] = 27,
 [KEY_BTAB] = '\t' | KEYSYM_SHIFT,
+[KEY_SPREVIOUS] = KEY_PPAGE | KEYSYM_SHIFT,
+[KEY_SNEXT] = KEY_NPAGE | KEYSYM_SHIFT,
 };
 
 static const int curses2keycode[CURSES_KEYS] = {
@@ -149,6 +150,9 @@ static const int curses2keycode[CURSES_K
 [KEY_IC] = 82 | GREY, /* Insert */
 [KEY_DC] = 83 | GREY, /* Delete */
 
+[KEY_SPREVIOUS] = 73 | GREY | SHIFT, /* Shift + Page Up */
+[KEY_SNEXT] = 81 | GREY | SHIFT, /* Shift + Page Down */
+
 ['!'] = 2 | SHIFT,
 ['@'] = 3 | SHIFT,
 ['#'] = 4 | SHIFT,
_

Re: [Qemu-devel] [PULL 0/7] fw_cfg: add dma interface, add strings via cmdline.

[Kevin O'Connor]

On Mon, Oct 19, 2015 at 12:12:34PM +0100, Peter Maydell wrote:
> On 19 October 2015 at 10:17, Gerd Hoffmann  wrote:
> > 
> > fw_cfg: add dma interface, add strings via cmdline.
> >
> > 
> 
> Hi. I'm afraid this fails 'make check':
> 
> TEST: tests/fw_cfg-test... (pid=17533)
>   /i386/fw_cfg/signature:  OK
>   /i386/fw_cfg/id: **
> ERROR:/home/petmay01/qemu/tests/fw_cfg-test.c:40:test_fw_cfg_id:
> assertion failed (qfw_cfg_get_u32(fw_cfg, FW_CFG_ID) == 1): (3 == 1)
> FAIL
> 
> (same failure on 64-bit ARM, ppc64be, OSX, 32-bit ARM).
> 
> Windows fails to compile:
> /home/petmay01/linaro/qemu-for-merges/hw/nvram/fw_cfg.c: In function
> ‘fw_cfg_dma_mem_read’:
> /home/petmay01/linaro/qemu-for-merges/hw/nvram/fw_cfg.c:406: warning:
> integer constant is too large for ‘long’ type

I don't have a Windows test environment, but I suspect the following:

#define FW_CFG_DMA_SIGNATURE 0x51454d5520434647 /* "QEMU CFG" */

should be changed to:

#define FW_CFG_DMA_SIGNATURE 0x51454d5520434647ULL /* "QEMU CFG" */

If that sounds right, I'll respin the last patch.

-Kevin

Re: [Qemu-devel] [PATCH 2/3] ui/curses: Support line graphics chars on -curses mode

[OGAWA Hirofumi]

Gerd Hoffmann  writes:

> Lets go with v2 of this patch.
>
> There are some codestyle issues in the patch (and the others too),
> please fix them them (scripts/checkpatch.pl helps), then resend the
> whole series.

OK. I'll send with new thread, not this thread.
-- 
OGAWA Hirofumi 

Re: [Qemu-devel] [PATCH] target-*: Advance pc after recognizing a breakpoint

[Sergey Fedorov]

On 19.10.2015 01:46, Richard Henderson wrote:
> On 10/16/2015 04:08 AM, Sergey Fedorov wrote:
>> On 16.10.2015 04:14, Richard Henderson wrote:
>>> On 10/16/2015 03:36 AM, Peter Maydell wrote:
 On 14 October 2015 at 22:02, Richard Henderson 
 wrote:
> On 10/15/2015 06:34 AM, Peter Maydell wrote:
>>
>> This is still the same cryptic comment we have in the
>> targets which do do this. Can we have something
>> that is a bit more explanatory about what is going on and
>> why we need to do this, please?
>
>
> Suggestions?

 ...well, I don't entirely understand the problem it's
 fixing, which is why I'm asking for a better comment :-)
>>>
>>> Heh.  Fair enough.  How about
>>>
>>>/* The address covered by the breakpoint must be included in
>>>   [tb->pc, tb->pc + tb->size) in order to for it to be
>>>   properly cleared -- thus we increment the PC here so that
>>>   the logic setting tb->size below does the right thing.  */
>>>
>>> There are two edge cases that cause the problem with clearing that
>>> could be described, but I think that the comment becomes too bulky, as
>>> well as confuses the situation for someone cutting-and-pasting the
>>> logic to a new port.
>>
>> Maybe we could rather fix that condition in
>> tb_invalidate_phys_page_range()? It seems weird that it can't handle a
>> zero-sized TB.
>
> We also need to be able to handle a TB which crosses a page.  E.g. the
> breakpoint is at the page boundary, and we fall through into it from
> the top. This will be true on e.g. x86.  This is not simply true for
> breakpoint insertion/removal, but also page invalidation.
>
> The same fix, adding a byte to the size, handles this as well.

It's clear except that instructions crossing a page boundary can be
different in size. AFAIK, x86 instructions can be up to 15-byte long.
What if only the very last byte of instruction crosses a page boundary?

Best regards,
Sergey

Re: [Qemu-devel] [PATCH v6 1/4] Add new block driver interface to add/delete a BDS's child

[Alberto Garcia]

On Fri 16 Oct 2015 10:57:43 AM CEST, Wen Congyang  wrote:
> In some cases, we want to take a quorum child offline, and take
> another child online.
>
> Signed-off-by: Wen Congyang 
> Signed-off-by: zhanghailiang 
> Signed-off-by: Gonglei 
> Reviewed-by: Eric Blake 
> ---

Reviewed-by: Alberto Garcia 

Berto

[Qemu-devel] [PATCH] qdev: free qemu-opts when the QOM path goes away

[Paolo Bonzini]

Otherwise there is a race where the DEVICE_DELETED event has been sent but
attempts to reuse the ID will fail.

Reported-by: Michael S. Tsirkin 
Signed-off-by: Paolo Bonzini 
---
 hw/core/qdev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 4ab04aa..92bd8bb 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -1203,7 +1203,6 @@ static void device_finalize(Object *obj)
 NamedGPIOList *ngl, *next;
 
 DeviceState *dev = DEVICE(obj);
-qemu_opts_del(dev->opts);
 
 QLIST_FOREACH_SAFE(ngl, >gpios, node, next) {
 QLIST_REMOVE(ngl, node);
@@ -1251,6 +1250,9 @@ static void device_unparent(Object *obj)
 qapi_event_send_device_deleted(!!dev->id, dev->id, path, _abort);
 g_free(path);
 }
+
+qemu_opts_del(dev->opts);
+dev->opts = NULL;
 }
 
 static void device_class_init(ObjectClass *class, void *data)
-- 
2.5.0

Re: [Qemu-devel] [PULL 0/7] fw_cfg: add dma interface, add strings via cmdline.

[Peter Maydell]

On 19 October 2015 at 12:52, Kevin O'Connor  wrote:
> On Mon, Oct 19, 2015 at 12:12:34PM +0100, Peter Maydell wrote:
>> Windows fails to compile:
>> /home/petmay01/linaro/qemu-for-merges/hw/nvram/fw_cfg.c: In function
>> ‘fw_cfg_dma_mem_read’:
>> /home/petmay01/linaro/qemu-for-merges/hw/nvram/fw_cfg.c:406: warning:
>> integer constant is too large for ‘long’ type
>
> I don't have a Windows test environment, but I suspect the following:
>
> #define FW_CFG_DMA_SIGNATURE 0x51454d5520434647 /* "QEMU CFG" */
>
> should be changed to:
>
> #define FW_CFG_DMA_SIGNATURE 0x51454d5520434647ULL /* "QEMU CFG" */

Yes. All 64 bit constants should have an LL or ULL suffix.

(The test failure is presumably something different.)

thanks
-- PMM

[Qemu-devel] [PATCH v2] qemu-options: add documentation for using UDP unicast network backend.

[Victor Kaplansky]

For the long time QEMU has support for UDP unicast network
backend added by commit 0e0e7facc775e9, but manual was missing
description and usage examples.

Changes from v1:

Address comments by Michael S. Tsirkin:
  - remove superfluous empty lines.
  - place the note about two QEMU instances after the example.
  - English and others cleanups.

Signed-off-by: Victor Kaplansky 
---
 qemu-options.hx | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/qemu-options.hx b/qemu-options.hx
index 2485b94..6bdb884 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1907,6 +1907,28 @@ qemu-system-i386 linux.img \
  -net socket,mcast=239.192.168.1:1102,localaddr=1.2.3.4
 @end example
 
+@item -netdev 
socket,id=@var{id}[,fd=@var{h}][,udp=@var{rhost}:@var{rport}[,localaddr=@var{lhost}:@var{lport}]]
+@itemx -net 
socket[,vlan=@var{n}][,name=@var{name}][,fd=@var{h}][,udp=@var{rhost}:@var{rport}[,localaddr=@var{rhost}:@var{lport}]]
+
+Connect a VLAN @var{n} to a remote VLAN in another QEMU virtual
+machine using a UDP tunnel. Use lhost:lport as the local host and
+port, rhost:rport as the remote host and port of the tunnel.  Use
+@option{fd=h} to specify an already opened UDP socket.
+
+Example:
+@example
+# launch one QEMU instance
+qemu-system-i386 linux.img \
+ -device virtio-net-pci,netdev=net0 \
+ -netdev 
socket,id=net0,udp=localhost:,localaddr=localhost:
+# launch a second QEMU instance sharing the network with the first one
+qemu-system-i386 linux.img \
+ -device virtio-net-pci,netdev=net0 \
+ -netdev 
socket,id=net0,udp=localhost:,localaddr=localhost:
+
+NOTE: The two QEMU instances can be running on different hosts.
+@end example
+
 @item -netdev 
l2tpv3,id=@var{id},src=@var{srcaddr},dst=@var{dstaddr}[,srcport=@var{srcport}][,dstport=@var{dstport}],txsession=@var{txsession}[,rxsession=@var{rxsession}][,ipv6][,udp][,cookie64][,counter][,pincounter][,txcookie=@var{txcookie}][,rxcookie=@var{rxcookie}][,offset=@var{offset}]
 @itemx -net 
l2tpv3[,vlan=@var{n}][,name=@var{name}],src=@var{srcaddr},dst=@var{dstaddr}[,srcport=@var{srcport}][,dstport=@var{dstport}],txsession=@var{txsession}[,rxsession=@var{rxsession}][,ipv6][,udp][,cookie64][,counter][,pincounter][,txcookie=@var{txcookie}][,rxcookie=@var{rxcookie}][,offset=@var{offset}]
 Connect VLAN @var{n} to L2TPv3 pseudowire. L2TPv3 (RFC3391) is a popular
-- 
--Victor

[Qemu-devel] [PATCH 3/3] block/gluster: code cleanup

[Prasanna Kumar Kalever]

unified coding styles of multiline function arguments and other error functions
moved random declarations of structures and other list variables

Signed-off-by: Prasanna Kumar Kalever 
---
 block/gluster.c | 109 ++--
 1 file changed, 59 insertions(+), 50 deletions(-)

diff --git a/block/gluster.c b/block/gluster.c
index e1f9b21..1293a13 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -21,6 +21,7 @@
 
 #define GLUSTER_DEFAULT_PORT   24007
 
+
 typedef struct GlusterAIOCB {
 int64_t size;
 int ret;
@@ -34,6 +35,11 @@ typedef struct BDRVGlusterState {
 struct glfs_fd *fd;
 } BDRVGlusterState;
 
+typedef struct BDRVGlusterReopenState {
+struct glfs *glfs;
+struct glfs_fd *fd;
+} BDRVGlusterReopenState;
+
 typedef struct GlusterServerConf {
 char *host;
 int port;
@@ -46,6 +52,38 @@ typedef struct GlusterConf {
 GlusterServerConf *gsconf;
 } GlusterConf;
 
+
+static QemuOptsList qemu_gluster_create_opts = {
+.name = "qemu-gluster-create-opts",
+.head = QTAILQ_HEAD_INITIALIZER(qemu_gluster_create_opts.head),
+.desc = {
+{
+.name = BLOCK_OPT_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = "Virtual disk size"
+},
+{
+.name = BLOCK_OPT_PREALLOC,
+.type = QEMU_OPT_STRING,
+.help = "Preallocation mode (allowed values: off, full)"
+},
+{ /* end of list */ }
+}
+};
+
+static QemuOptsList runtime_opts = {
+.name = "gluster",
+.head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
+.desc = {
+{
+.name = GLUSTER_OPT_FILENAME,
+.type = QEMU_OPT_STRING,
+.help = "URL to the gluster image",
+},
+{ /* end of list */ }
+},
+};
+
 static QemuOptsList runtime_json_opts = {
 .name = "gluster_json",
 .head = QTAILQ_HEAD_INITIALIZER(runtime_json_opts.head),
@@ -87,6 +125,7 @@ static QemuOptsList runtime_tuple_opts = {
 },
 };
 
+
 static void qemu_gluster_gconf_free(GlusterConf *gconf)
 {
 if (gconf) {
@@ -586,19 +625,6 @@ static void gluster_finish_aiocb(struct glfs_fd *fd, 
ssize_t ret, void *arg)
 qemu_bh_schedule(acb->bh);
 }
 
-static QemuOptsList runtime_opts = {
-.name = "gluster",
-.head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
-.desc = {
-{
-.name = GLUSTER_OPT_FILENAME,
-.type = QEMU_OPT_STRING,
-.help = "URL to the gluster image",
-},
-{ /* end of list */ }
-},
-};
-
 static void qemu_gluster_parse_flags(int bdrv_flags, int *open_flags)
 {
 assert(open_flags != NULL);
@@ -664,12 +690,6 @@ out:
 return ret;
 }
 
-typedef struct BDRVGlusterReopenState {
-struct glfs *glfs;
-struct glfs_fd *fd;
-} BDRVGlusterReopenState;
-
-
 static int qemu_gluster_reopen_prepare(BDRVReopenState *state,
BlockReopenQueue *queue, Error **errp)
 {
@@ -754,7 +774,9 @@ static void qemu_gluster_reopen_abort(BDRVReopenState 
*state)
 
 #ifdef CONFIG_GLUSTERFS_ZEROFILL
 static coroutine_fn int qemu_gluster_co_write_zeroes(BlockDriverState *bs,
-int64_t sector_num, int nb_sectors, BdrvRequestFlags flags)
+ int64_t sector_num,
+ int nb_sectors,
+ BdrvRequestFlags flags)
 {
 int ret;
 GlusterAIOCB *acb = g_slice_new(GlusterAIOCB);
@@ -787,7 +809,7 @@ static inline bool gluster_supports_zerofill(void)
 }
 
 static inline int qemu_gluster_zerofill(struct glfs_fd *fd, int64_t offset,
-int64_t size)
+int64_t size)
 {
 return glfs_zerofill(fd, offset, size);
 }
@@ -799,7 +821,7 @@ static inline bool gluster_supports_zerofill(void)
 }
 
 static inline int qemu_gluster_zerofill(struct glfs_fd *fd, int64_t offset,
-int64_t size)
+int64_t size)
 {
 return 0;
 }
@@ -832,15 +854,14 @@ static int qemu_gluster_create(const char *filename,
gluster_supports_zerofill()) {
 prealloc = 1;
 } else {
-error_setg(errp, "Invalid preallocation mode: '%s'"
-" or GlusterFS doesn't support zerofill API",
-tmp);
+error_setg(errp, "Error: Invalid preallocation mode: '%s'"
+ " or GlusterFS doesn't support zerofill API", tmp);
 ret = -EINVAL;
 goto out;
 }
 
 fd = glfs_creat(glfs, gconf->path,
-O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR | S_IWUSR);
+O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR | 
S_IWUSR);
 if (!fd) {
 ret = -errno;
 } else {
@@ -866,7 +887,8 @@ out:
 }
 
 static coroutine_fn int qemu_gluster_co_rw(BlockDriverState *bs,
-int64_t sector_num, int 

Re: [Qemu-devel] [PULL v3 00/13] qemu-ga patch queue

[Michael Roth]

Quoting Peter Maydell (2015-10-18 16:21:30)
> On 17 October 2015 at 17:59, Michael Roth  wrote:
> > Hi Peter,
> >
> > Please note that 'glib-compat: add 2.38/2.40/2.46 asserts' is also in
> > Marc-André's recent ivshmem PULL. The 2 versions of the patches are 
> > identical,
> > but let me know if you'd prefer a re-send/re-base later.
> >
> > The following changes since commit 6d57410a79d51d92673c54f26624b44f27fa6214:
> >
> >   Merge remote-tracking branch 
> > 'remotes/pmaydell/tags/pull-target-arm-20151016' into staging (2015-10-17 
> > 12:31:33 +0100)
> >
> > are available in the git repository at:
> >
> >
> >   git://github.com/mdroth/qemu.git tags/qga-pull-2015-10-14-v3-tag
> >
> > for you to fetch changes up to ed9f1986d19c2d21667a14b875b2ac8b8a19b8a5:
> >
> >   qga: fix uninitialized value warning for win32 (2015-10-17 11:24:27 -0500)
> >
> > 
> > qemu-ga patch queue
> >
> > * add unit tests for qemu-ga
> > * add guest-exec support for posix/w32 guests
> > * added 'qemu-ga' target for w32. this allows us to do full MSI build,
> >   without overloading 'qemu-ga.exe' target with uneeded dependencies.
> > * number of s/g_new/g_malloc/ conversions for qga
> >
> > v2:
> > * commit message and qapi documentation spelling fixes
> > * rename 'inp-data' guest-exec param to 'input-data'
> >
> > v3:
> > * fix OSX build errors for test-qga by using PRId64
> >   format in place of glib's, and dropping use of G_SPAWN_DEFAULT
> >   macro for glib 2.22 compat
> > * fix win32 build warnings for 32-bit builds by avoid int casts
> >   of process HANDLEs
> 
> Still seeing failures, I'm afraid.
> 
> OSX assertion failures:
> 
> GTESTER tests/test-qga
> **
> ERROR:/Users/pm215/src/qemu-for-merges/tests/libqtest.c:238:void
> socket_send(int, const char *, size_t): assertion failed (len != -1):
> (-1 != -1)
> GTester: last random seed: R02S96655a200709f74b72ea42792cd65e8e

Argh, sorry for all this noise.

I suspect what's happening is qemu-ga itself might be failing to load or
bind to the socket, and there appears to be a bug in the test case where
we don't check for an fd == -1 return value in connect_qga(), so probably
end up trying to write to it later.

As far as why qemu-ga doesn't appear to be loading properly under OSX,
I think I need to get an environment set up somewhere to debug. I tried
to get an OSX guest going but apparently you need to own an OSX box to
get a AppleSMC OSK key so I'll have to hold off on that for now.

This isn't really something I think has been tested or deployed very
often outside of linux/w32 guests (and w32 isn't enabled since
interacting with a qemu-ga under wine/binfmt requires a specialized
wine environment), so for now I think the best option is to run test-qga
under CONFIG_LINUX instead of CONFIG_POSIX.

> 
> (repeated about 10 times).
> 
> Test failures on 64-bit ARM:
> TEST: tests/test-qga... (pid=22454)
>   /qga/sync-delimited: OK
>   /qga/sync:   OK
>   /qga/ping:   OK
>   /qga/info:   OK
>   /qga/network-get-interfaces: OK
>   /qga/get-vcpus:  OK
>   /qga/get-fsinfo: OK
>   /qga/get-memory-block-info:  **
> ERROR:/home/petmay01/qemu/tests/test-qga.c:294:test_qga_get_memory_block_info:
> assertion failed ret: GenericError
> open("/sys/devices/system/memory/"): No such file or directory
> FAIL
> GTester: last random seed: R02S6aa3e1f8b691a9900d2ea9945e79869d
> (pid=22458)
>   /qga/get-memory-blocks:  **
> ERROR:/home/petmay01/qemu/tests/test-qga.c:313:test_qga_get_memory_blocks:
> assertion failed ret: GenericError Can't open
> directory"/sys/devices/system/memory/"
> : No such file or directory
> FAIL
> GTester: last random seed: R02S978afb04187410dc690a8b7d6d236793
> (pid=22461)
>   /qga/file-ops:   OK
>   /qga/get-time:   OK
>   /qga/invalid-cmd:OK
>   /qga/fsfreeze-status:OK
>   /qga/blacklist:  OK
>   /qga/config: OK
> FAIL: tests/test-qga
> make: *** [check-tests/test-qga] Error 1
> 
> Not sure why it's complaining, /sys/devices/system/memory/ does
> exist on this box.
> 
> Ditto on 32-bit ARM, though that's not surprising as it's just
> a chroot on an equivalently-configured machine to the 64-bit build.

Hmm, I'm have FC22 running via 

Re: [Qemu-devel] [PULL 2/2] xen-platform: Ensure xen is enabled when initializing

[Paolo Bonzini]

On 19/10/2015 12:23, Stefano Stabellini wrote:
> The xen-platform code crashes on reset if the xen backend is not
> initialized, because it calls xc_hvm_set_mem_type(). Ensure xen-platform
> won't be created without initializing the xen backend.
> 
> The assert can't be triggered by the user because the device is not
> hotpluggable, and the only code creating it (at pc_xen_hvm_init())
> already checks xen_enabled().

The device is not hotpluggable, but it is accessible with -device.  This
is by design because IIUC, we want -M xenfv to be phased out in favor of
the PC machines plus -device xen-platform.

Thus, k->init should be changed to k->realize instead.  I guess it can
be done on top of this pull request, so I'm not blocking it.

Paolo

> Signed-off-by: Eduardo Habkost 
> Reviewed-by: Stefano Stabellini 
> Signed-off-by: Stefano Stabellini 
> ---
>  hw/i386/xen/xen_platform.c |3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
> index ee45f03..8682c42 100644
> --- a/hw/i386/xen/xen_platform.c
> +++ b/hw/i386/xen/xen_platform.c
> @@ -387,6 +387,9 @@ static int xen_platform_initfn(PCIDevice *dev)
>  PCIXenPlatformState *d = XEN_PLATFORM(dev);
>  uint8_t *pci_conf;
>  
> +/* Device will crash on reset if xen is not initialized */
> +assert(xen_enabled());
> +

Re: [Qemu-devel] [PULL 0/2] Xen 2015-10-19-tag

[Peter Maydell]

On 19 October 2015 at 11:23, Stefano Stabellini
<stefano.stabell...@eu.citrix.com> wrote:
> The following changes since commit aedc8806172dd1ae904f04169ee3b19fce1d7893:
>
>   Merge remote-tracking branch 'remotes/kraxel/tags/pull-audio-20151019-1' 
> into staging (2015-10-19 10:06:56 +0100)
>
> are available in the git repository at:
>
>
>   git://xenbits.xen.org/people/sstabellini/qemu-dm.git tags/2015-10-19-tag
>
> for you to fetch changes up to dbb7405d8caad0814ceddd568cb49f163a847561:
>
>   xen-platform: Ensure xen is enabled when initializing (2015-10-19 10:16:01 
> +)
>
> 
> Xen 2015-10-19
>
> 

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v2 3/3] virtio-blk: switch off scsi-passthrough by default

[Cornelia Huck]

On Sun, 18 Oct 2015 10:59:59 +0300
"Michael S. Tsirkin"  wrote:

> On Fri, Oct 16, 2015 at 01:07:28PM +0200, Christian Borntraeger wrote:

> > Lets keep this patch as is to have scsi=off as default for virtio 1.0
> > 
> > (some iotests do fail because of this)
> > 
> > Christian
> 
> What fails, exactly?

For example, testcase 068 (on s390x, since we default virtio-1 to on):

--- /data/git/yyy/qemu/tests/qemu-iotests/068.out   2015-03-09 
12:32:35.245444052 +0100
+++ 068.out.bad 2015-10-19 13:48:00.023772655 +0200
@@ -3,9 +3,8 @@
 === Saving and reloading a VM state to/from a qcow2 image ===
 
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072
+qemu-system-s390x: -hda 
/data/git/yyy/qemu/build/tests/qemu-iotests/scratch/t.qcow2: Please set 
scsi=off for virtio-blk devices in order to use virtio 1.0
 QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) savevm 0
-(qemu) quit
+(qemu) qemu-system-s390x: -hda 
/data/git/yyy/qemu/build/tests/qemu-iotests/scratch/t.qcow2: Please set 
scsi=off for virtio-blk devices in order to use virtio 1.0
 QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) quit
-*** done
+(qemu) *** done

Re: [Qemu-devel] [PULL 0/7] fw_cfg: add dma interface, add strings via cmdline.

[Kevin O'Connor]

On Mon, Oct 19, 2015 at 01:02:41PM +0100, Peter Maydell wrote:
> On 19 October 2015 at 12:52, Kevin O'Connor  wrote:
> > On Mon, Oct 19, 2015 at 12:12:34PM +0100, Peter Maydell wrote:
> >> Windows fails to compile:
> >> /home/petmay01/linaro/qemu-for-merges/hw/nvram/fw_cfg.c: In function
> >> ‘fw_cfg_dma_mem_read’:
> >> /home/petmay01/linaro/qemu-for-merges/hw/nvram/fw_cfg.c:406: warning:
> >> integer constant is too large for ‘long’ type
> >
> > I don't have a Windows test environment, but I suspect the following:
> >
> > #define FW_CFG_DMA_SIGNATURE 0x51454d5520434647 /* "QEMU CFG" */
> >
> > should be changed to:
> >
> > #define FW_CFG_DMA_SIGNATURE 0x51454d5520434647ULL /* "QEMU CFG" */
> 
> Yes. All 64 bit constants should have an LL or ULL suffix.

Okay - I'll respin patch 7.

> (The test failure is presumably something different.)

I think the fix for the test failure is to make the test less picky
(it should accept a feature bitmap of either 1 or 3).  But I'll leave
that to Marc or Gerd.

-Kevin

[Qemu-devel] [PATCH 2/3] block/gluster: rename [server, volname, image] -> [host, volume, path]

[Prasanna Kumar Kalever]

for example in 'servers' tuple values we use 'server' variable for key 'host'
in the code, it will be quite messy to have colliding names for variables,
so to maintain better readability and makes it consistent with other existing
code as well as the input keys/options, this patch renames the following
variables
'server'  -> 'host'
'image'   -> 'path'
'volname' -> 'volume'

Signed-off-by: Prasanna Kumar Kalever 
---
 block/gluster.c | 54 +++---
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/block/gluster.c b/block/gluster.c
index dd076fe..e1f9b21 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -35,14 +35,14 @@ typedef struct BDRVGlusterState {
 } BDRVGlusterState;
 
 typedef struct GlusterServerConf {
-char *server;
+char *host;
 int port;
 char *transport;
 } GlusterServerConf;
 
 typedef struct GlusterConf {
-char *volname;
-char *image;
+char *volume;
+char *path;
 GlusterServerConf *gsconf;
 } GlusterConf;
 
@@ -90,10 +90,10 @@ static QemuOptsList runtime_tuple_opts = {
 static void qemu_gluster_gconf_free(GlusterConf *gconf)
 {
 if (gconf) {
-g_free(gconf->volname);
-g_free(gconf->image);
+g_free(gconf->volume);
+g_free(gconf->path);
 if (gconf->gsconf) {
-g_free(gconf->gsconf[0].server);
+g_free(gconf->gsconf[0].host);
 g_free(gconf->gsconf[0].transport);
 g_free(gconf->gsconf);
 gconf->gsconf = NULL;
@@ -117,19 +117,19 @@ static int parse_volume_options(GlusterConf *gconf, char 
*path)
 if (*p == '\0') {
 return -EINVAL;
 }
-gconf->volname = g_strndup(q, p - q);
+gconf->volume = g_strndup(q, p - q);
 
-/* image */
+/* path */
 p += strspn(p, "/");
 if (*p == '\0') {
 return -EINVAL;
 }
-gconf->image = g_strdup(p);
+gconf->path = g_strdup(p);
 return 0;
 }
 
 /*
- * file=gluster[+transport]://[server[:port]]/volname/image[?socket=...]
+ * file=gluster[+transport]://[host[:port]]/volume/path[?socket=...]
  *
  * 'gluster' is the protocol.
  *
@@ -138,10 +138,10 @@ static int parse_volume_options(GlusterConf *gconf, char 
*path)
  * tcp, unix and rdma. If a transport type isn't specified, then tcp
  * type is assumed.
  *
- * 'server' specifies the server where the volume file specification for
+ * 'host' specifies the host where the volume file specification for
  * the given volume resides. This can be either hostname, ipv4 address
  * or ipv6 address. ipv6 address needs to be within square brackets [ ].
- * If transport type is 'unix', then 'server' field should not be specified.
+ * If transport type is 'unix', then 'host' field should not be specified.
  * The 'socket' field needs to be populated with the path to unix domain
  * socket.
  *
@@ -150,9 +150,9 @@ static int parse_volume_options(GlusterConf *gconf, char 
*path)
  * default port. If the transport type is unix, then 'port' should not be
  * specified.
  *
- * 'volname' is the name of the gluster volume which contains the VM image.
+ * 'volume' is the name of the gluster volume which contains the VM image.
  *
- * 'image' is the path to the actual VM image that resides on gluster volume.
+ * 'path' is the path to the actual VM image that resides on gluster volume.
  *
  * Examples:
  *
@@ -161,7 +161,7 @@ static int parse_volume_options(GlusterConf *gconf, char 
*path)
  * file=gluster+tcp://1.2.3.4:24007/testvol/dir/a.img
  * file=gluster+tcp://[1:2:3:4:5:6:7:8]/testvol/dir/a.img
  * file=gluster+tcp://[1:2:3:4:5:6:7:8]:24007/testvol/dir/a.img
- * file=gluster+tcp://server.domain.com:24007/testvol/dir/a.img
+ * file=gluster+tcp://host.domain.com:24007/testvol/dir/a.img
  * file=gluster+unix:///testvol/dir/a.img?socket=/tmp/glusterd.socket
  * file=gluster+rdma://1.2.3.4:24007/testvol/a.img
  */
@@ -216,9 +216,9 @@ static int qemu_gluster_parseuri(GlusterConf **pgconf, 
const char *filename)
 ret = -EINVAL;
 goto out;
 }
-gconf->gsconf[0].server = g_strdup(qp->p[0].value);
+gconf->gsconf[0].host = g_strdup(qp->p[0].value);
 } else {
-gconf->gsconf[0].server = g_strdup(uri->server ? uri->server : 
"localhost");
+gconf->gsconf[0].host = g_strdup(uri->server ? uri->server : 
"localhost");
 if (uri->port) {
 gconf->gsconf[0].port = uri->port;
 } else {
@@ -247,14 +247,14 @@ static struct glfs *qemu_gluster_glfs_init(GlusterConf 
*gconf, int num_servers,
 int old_errno;
 int i;
 
-glfs = glfs_new(gconf->volname);
+glfs = glfs_new(gconf->volume);
 if (!glfs) {
 goto out;
 }
 
 for (i = 0; i < num_servers; i++) {
 ret = glfs_set_volfile_server(glfs, gconf->gsconf[i].transport,
-  gconf->gsconf[i].server,
+  gconf->gsconf[i].host,
   

Re: [Qemu-devel] Question about odd snapshot behaviour

[Kevin Wolf]

[ CC: qemu-block ]

Am 19.10.2015 um 07:27 hat Sam Bobroff geschrieben:
> Hi all,
> 
> While working through the QEMU code that saves and loads snapshots, I've
> noticed some confusing behaviour when using named VM snapshots that may need 
> to
> be fixed somehow. This is what I've noticed:
> 
> * If I create two empty qcow2 filesystems: fs1.qcow2 and fs2.qcow2:
>   * e.g. qemu-image create -f qcow2 fs1.qcow2 10G
>   * e.g. qemu-image create -f qcow2 fs2.qcow2 10G
> * Then boot the guest with the above filesystems as hda and hdb (my guest's 
> rootfs
>   is an initramfs, not either of these):
>   * e.g. qemu-system-ppc64 ... -hda fs1.qcow2 -hdb fs2.qcow2
> * In the QEMU monitor, create a blockdev snapshot on hd2 called "foo":
>   e.g. snapshot_blkdev_internal scsi0-hd1 foo
> * Check fs2 on the host command line:
>   e.g. qemu-img snapshot -l fs2.qcow2
>   Snapshot list:
>   IDTAG VM SIZEDATE   VM CLOCK
>   1 foo   0 2015-10-19 15:56:29   00:00:20.969
> * In the QEMU monitor, save the vm state as "bar":
>   e.g. savevm bar
> * Check the snapshot on fs1:
>   e.g. info snapshots
>   IDTAG VM SIZEDATE   VM CLOCK
>   1 bar215M 2015-10-19 04:57:13   00:01:05.589
> * Check fs2 again on the host command line:
>   e.g. qemu-img snapshot -l fs2.qcow2
>   Snapshot list:
>   IDTAG VM SIZEDATE   VM CLOCK
>   1 foo   0 2015-10-19 15:56:29   00:00:20.969
>   2 bar   0 2015-10-19 15:57:13   00:01:05.589
> 
> * Now the fun part: overwrite the bar snapshot on fs1 by ID:
>   * savevm 1
> * Examine the results from the monitor:
>   * e.g. info snapshots
>   There is no suitable snapshot available
> * Examine fs1 and fs2 from the command line:
>   * e.g. qemu-img snapshot -l fs1.qcow2 
>   Snapshot list:
>   IDTAG VM SIZEDATE   VM CLOCK
>   1 bar215M 2015-10-19 16:00:45   00:04:36.065
>   * e.g. qemu-img snapshot -l fs2.qcow2 
>   Snapshot list:
>   IDTAG VM SIZEDATE   VM CLOCK
>   2 bar   0 2015-10-19 15:57:13   00:01:05.589
>   3 bar   0 2015-10-19 16:00:45   00:04:36.065

I must admit that I wouldn't have been able to predict this result. :-)

Generally, internal snapshots with multiple disks are somewhat tricky
because we have individual names and IDs in every image file, and each
file can individually be snapshotted, but need to create a consistent
view from it.

> So what seems odd is:
> 
> * QEMU has lost the bar snapshot on fs1, although qemu-img still shows
>   it and it re-appears in QEMU if the guest is started with only
>   hd1.qcow attached.

This one I think is a feature: A snapshot can only be loaded if it's
present on all image files. Loading the snapshot only on some disks and
leaving the other disks alone is likely to result in an inconsistent
state.

> * QEMU has deleted the snapshot named "foo" on hd2, even though we issued
>   "savevm 1" and the old snapshot (on hd1) with ID 1 was named "bar". This
>   surprised me.

You mean we should check whether a snapshot identified by an ID has the
same name on all image files before it's considered valid? That might be
a reasonable constraint.

What I would have expected is that the old snapshot with ID 1 indeed
gets deleted, but a new one with ID 1 is created again. I'm not sure if
I would have expected "foo" or "bar" as its name, I could argue either
way.

> * QEMU has created a snapshot on hd2 with a duplicate name ("bar") and if we
>   issue a "loadvm bar" QEMU loads the snapshot with ID 1 on hd1 and the first
>   snapshot with the name "bar" on hd2 which is the one with ID 2. That is not
>   the snapshot that matches the vm state (which is the one with ID 3) and
>   presumably can lead to filesystem corruption on hd2.

Yup, this doesn't seem very nice. I'm not sure whether we can forbid
duplicate names, though, without breaking compatibility in an
unacceptable way.

On the other hand, IDs and names don't really mix well and make it too
easy to shoot yourself in the foot. Maybe it's bad enough to justify
even some incompatibilities.

> Do these seem like bugs? Should I try to find a way to fix some or all of 
> them?
> 
> (Note: I checked the launchpad bug list but didn't see these listed.)

I'm not sure whether to call them bugs, but there's definitely a safety
check or two missing. Let's discuss what the exact behaviour should be,
and then you can go ahead and write the fixes to get that behaviour.

Kevin

Re: [Qemu-devel] [PULL 0/7] fw_cfg: add dma interface, add strings via cmdline.

[Peter Maydell]

On 19 October 2015 at 10:17, Gerd Hoffmann <kra...@redhat.com> wrote:
>   Hi,
>
> As promised last week I've picked up the fw_cfg patches which are ready
> to go, so here comes the pull request for them.  Headline feature is
> the fw_cfg dam support fo course, enabled for arm and x86.  Also small
> fixes and an option to add string fw_cfg entries via command line.
>
> please pull,
>   Gerd
>
> The following changes since commit 40fe17bea478793fc9106a630fa3610dad51f939:
>
>   hw/ide/ahci.c: Fix shift left into sign bit (2015-10-18 11:00:40 +0100)
>
> are available in the git repository at:
>
>   git://git.kraxel.org/qemu tags/pull-fw_cfg-20151019-1
>
> for you to fetch changes up to 7b0eec285d447057405df73beae78c8e8aeb9793:
>
>   fw_cfg: Define a static signature to be returned on DMA port reads 
> (2015-10-19 11:00:50 +0200)
>
> 
> fw_cfg: add dma interface, add strings via cmdline.
>
> 

Hi. I'm afraid this fails 'make check':

TEST: tests/fw_cfg-test... (pid=17533)
  /i386/fw_cfg/signature:  OK
  /i386/fw_cfg/id: **
ERROR:/home/petmay01/qemu/tests/fw_cfg-test.c:40:test_fw_cfg_id:
assertion failed (qfw_cfg_get_u32(fw_cfg, FW_CFG_ID) == 1): (3 == 1)
FAIL

(same failure on 64-bit ARM, ppc64be, OSX, 32-bit ARM).

Windows fails to compile:
/home/petmay01/linaro/qemu-for-merges/hw/nvram/fw_cfg.c: In function
‘fw_cfg_dma_mem_read’:
/home/petmay01/linaro/qemu-for-merges/hw/nvram/fw_cfg.c:406: warning:
integer constant is too large for ‘long’ type

thanks
-- PMM

Re: [Qemu-devel] Question about xen disk unplug support for ahci missed in qemu

[Gerd Hoffmann]

  Hi,

> > I'm trying to follow this discussion as best as I am able, but my lack
> > of experience with Xen prevents me from really participating in a
> > meaningful way.
> > 
> > (I see that Laszlo is still discussing some CD-ROM issues with Fabio
> > which may be of interest to me...)
> > 
> > At any rate, I won't be authoring any Xen-specific hacks to the AHCI
> > device, but I do have plans to implement hot-plugging emulation as per
> > the AHCI spec. Perhaps this is sufficient for the Xen layer, but someone
> > else will need to author the appropriate glue code.
> > 
> > If "real" hot-plugging is not sufficient, we'll need to discuss further,
> > preferably over some RFC patches.
> 
> That's fine. AHCI hot-plugging would go a long way and once we have
> that, the rest is easy.

Can we get some more background on this?

IIRC the IDE bits are needed to boot hvm guests, which goes like this:

  (1) boot disk is hooked up using both xenbus and ide.
  (2) seabios boots using ide.
  (3) linux kernel activates xenbus, at which point qemu zaps the ide
  disks to avoid the disk being present twice in the system.

Correct?

Do we really want repeat this exercise for AHCI?  Alot has changed since
this boot hack for ide was added ...

As far I know OVMF has xenbus drivers, so OVMF should already boot xen
guests just fine without this, correct?

Can we just have xenbus drivers for seabios too?  seabios can run disk
drivers in 32bit mode meanwhile, so this should not be as difficult any
more as it used to be.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 0/3] Add 'blockdev-del' command

[Kevin Wolf]

Am 13.10.2015 um 15:48 hat Alberto Garcia geschrieben:
> Here's my first attempt at the 'blockdev-del' command.
> 
> This series goes on top of Max's "BlockBackend and media" v6:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg02810.html
> 
> With Max's code, 'blockdev-add' can now create a BlockDriverState with
> or without a BlockBackend (depending on whether the 'id' parameter is
> passed).
> 
> Therefore, 'blockdev-del' can be used to delete a backend and/or a
> BDS.
> 
> The way it works is simple: it receives a single 'device' parameter
> which can refer to a block backend or a node name.
> 
>  - If it's a block backend, it will delete it along with its attached
>BDS (if any).
> 
>  - If it's a node name, it will delete the BDS along with the backend
>it is attached to (if any).
> 
>  - If you're wondering whether it's possible to delete the BDS but not
>the backend it is attached to, there is already eject or
>blockdev-remove-medium for that.
> 
> If either the backend or the BDS are being used (refcount > 1, or if
> the BDS has any parents) the command fails.

I've been thinking a bit about the creation and deletion of
BlockBackends a bit last week, and honestly it still feels a bit messy
(or maybe I just don't fully understand it yet). Anyway, the cover
letter of this series seems to be a good place for trying to write it
down.

So we seem to have two criteria to distinguish BDSes:

1. Does/Doesn't have a BlockBackend on top.
   In the future, multiple BlockBackends are possible, too.

2. Has/Hasn't been created explicitly by the user.
   Only if it has, the monitor owns a reference.

In addition, we also have BlockBackends without a BDS attached. All BBs
are explicitly created by the user. This leads us to the following
cases:

1. BB without BDS
2. BB attached to BDS without monitor reference
3. BB attached to BDS with monitor reference
4. BDS without BB and without monitor reference
5. BDS without BB and with monitor reference

The question I've been thinking about for each of the cases is: Can we
create this with blockdev-add today, and what should blockdev-del do?
Another interesting question is: Can we move between the cases, e.g. by
adding a monitor reference retroactively, or do we even want to do that?

Let me start with the first two questions for all cases:

1. As far as I know, it's currently not possible to directly create a BB
   without a BDS (or before Max' series with an empty BDS). You have to
   create a BB with an opened BDS and then eject that. Oops.

   blockdev-del is easy: It deletes the BB and nothing else.

2. This is the normal case, easy to produce with blockdev-add.
   The two options for deleting something are removing the BDS while
   keeping the BB (this is already covered by 'eject') and dropping the
   BB (probably makes sense to use 'blockdev-del' here). There is no
   monitor reference to the BDS anyway, so blockdev-dev can't delete
   that.

   Dropping the BB automatically also drops the BDS in simple cases. In
   more complicated cases where the BDS has got more references later,
   it might still exist. Then the blockdev-del wasn't the exact opposite
   operation of blockdev-add.

   Is this a problem for a user/management tool that wants to keep track
   of which image files are still in use?

3. A BDS with a monitor reference can be created (with some patches) by
   using blockdev-add with a node-name, but no id. Directly attaching it
   to a BB is tricky today, even though I'm sure you could do it with
   some clever use of block jobs... With 1. fixed (i.e. you can create
   an empty BB), insert-medium should do the job.

   Here we have even more choices for deleting something: Delete the BB,
   but leave the BDS alone; delete the BDS and leave an empty BB behind
   (this is different from eject because eject would drop the connection
   between BB and BDS, but both would continue to exist!); delete both
   at once.

   We had two separate blockdev-add commands for the BDS and the BB, so
   it makes sense to require two separate blockdev-del commands as well.
   In order to keep blockdev-add/del symmetrical, we would have to make
   blockdev-del of a node-name delete the BDS and blockdev-del of an id
   delete the BB.

4. That's the typical bs->file. Created by nested blockdev-add
   descriptions. blockdev-del errors out, there is no monitor reference
   to drop, and there is also no BB that the request could be used for.

5. Created by a top-level blockdev-add with node-name and no id (like 3.
   but without attaching it to a BB afterwards). blockdev-del can only
   mean deleting the BDS.

If I compare this with the semantics described in the cover letter, I
must say that I see some problems, especially with case 3.

I haven't thought about it enough yet, but it seems to me that we can't
do the BDS/BB aliasing with blockdev-del, but need to interpret
node-name as BDS and id as BB. Perhaps we also shouldn't use a single

Re: [Qemu-devel] Question about xen disk unplug support for ahci missed in qemu

[Stefano Stabellini]

On Mon, 19 Oct 2015, Gerd Hoffmann wrote:
>   Hi,
> 
> > > I'm trying to follow this discussion as best as I am able, but my lack
> > > of experience with Xen prevents me from really participating in a
> > > meaningful way.
> > > 
> > > (I see that Laszlo is still discussing some CD-ROM issues with Fabio
> > > which may be of interest to me...)
> > > 
> > > At any rate, I won't be authoring any Xen-specific hacks to the AHCI
> > > device, but I do have plans to implement hot-plugging emulation as per
> > > the AHCI spec. Perhaps this is sufficient for the Xen layer, but someone
> > > else will need to author the appropriate glue code.
> > > 
> > > If "real" hot-plugging is not sufficient, we'll need to discuss further,
> > > preferably over some RFC patches.
> > 
> > That's fine. AHCI hot-plugging would go a long way and once we have
> > that, the rest is easy.
> 
> Can we get some more background on this?
> 
> IIRC the IDE bits are needed to boot hvm guests, which goes like this:
> 
>   (1) boot disk is hooked up using both xenbus and ide.
>   (2) seabios boots using ide.
>   (3) linux kernel activates xenbus, at which point qemu zaps the ide
>   disks to avoid the disk being present twice in the system.
> 
> Correct?
> 
> Do we really want repeat this exercise for AHCI?  Alot has changed since
> this boot hack for ide was added ...
>
> As far I know OVMF has xenbus drivers, so OVMF should already boot xen
> guests just fine without this, correct?

I agree with you that the current unplug in nasty. Also I don't care
much about AHCI, in fact I don't think we should be spending efforts
into making that scenario work better. I think we should be working on
OVMF instead and fix the bug about empty cdrom drives reported by Fabio.


> Can we just have xenbus drivers for seabios too?  seabios can run disk
> drivers in 32bit mode meanwhile, so this should not be as difficult any
> more as it used to be.

Sure, I would be happy to see that happen, but I won't be working on it.

Re: [Qemu-devel] [PATCH v2 3/3] virtio-blk: switch off scsi-passthrough by default

[Michael S. Tsirkin]

On Mon, Oct 19, 2015 at 01:53:50PM +0200, Cornelia Huck wrote:
> On Sun, 18 Oct 2015 10:59:59 +0300
> "Michael S. Tsirkin"  wrote:
> 
> > On Fri, Oct 16, 2015 at 01:07:28PM +0200, Christian Borntraeger wrote:
> 
> > > Lets keep this patch as is to have scsi=off as default for virtio 1.0
> > > 
> > > (some iotests do fail because of this)
> > > 
> > > Christian
> > 
> > What fails, exactly?
> 
> For example, testcase 068 (on s390x, since we default virtio-1 to on):

I see, thanks.
So it's just the assertion that we have in code that fires.
Sure, scsi must be off by default before virtio 1 is on.


> --- /data/git/yyy/qemu/tests/qemu-iotests/068.out 2015-03-09 
> 12:32:35.245444052 +0100
> +++ 068.out.bad   2015-10-19 13:48:00.023772655 +0200
> @@ -3,9 +3,8 @@
>  === Saving and reloading a VM state to/from a qcow2 image ===
>  
>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072
> +qemu-system-s390x: -hda 
> /data/git/yyy/qemu/build/tests/qemu-iotests/scratch/t.qcow2: Please set 
> scsi=off for virtio-blk devices in order to use virtio 1.0
>  QEMU X.Y.Z monitor - type 'help' for more information
> -(qemu) savevm 0
> -(qemu) quit
> +(qemu) qemu-system-s390x: -hda 
> /data/git/yyy/qemu/build/tests/qemu-iotests/scratch/t.qcow2: Please set 
> scsi=off for virtio-blk devices in order to use virtio 1.0
>  QEMU X.Y.Z monitor - type 'help' for more information
> -(qemu) quit
> -*** done
> +(qemu) *** done

[Qemu-devel] [PATCH v2] target-arm/translate.c: Handle non-executable page-straddling Thumb insns

[Peter Maydell]

When the memory we're trying to translate code from is not executable we have
to turn this into a guest fault. In order to report the correct PC for this
fault, and to make sure it is not reported until after any other possible
faults for instructions earlier in execution, we must terminate TBs at
the end of a page, in case the next instruction is in a non-executable page.
This is simple for T16, A32 and A64 instructions, which are always aligned
to their size. However T32 instructions may be 32-bits but only 16-aligned,
so they can straddle a page boundary.

Correct the condition that checks whether the next instruction will touch
the following page, to ensure that if we're 2 bytes before the boundary
and this insn is T32 then we end the TB.

Reported-by: Pavel Dovgalyuk 
Reviewed-by: Laurent Desnogues 
Signed-off-by: Peter Maydell 
---
Changes v1->v2:
 * rebase
 * check '(insn >> 11) >= 0x1d' rather than switching on insn >> 11

 target-arm/translate.c | 45 -
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 9f1d740..6be2c72 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -11179,6 +11179,35 @@ undef:
default_exception_el(s));
 }
 
+static bool insn_crosses_page(CPUARMState *env, DisasContext *s)
+{
+/* Return true if the insn at dc->pc might cross a page boundary.
+ * (False positives are OK, false negatives are not.)
+ */
+uint16_t insn;
+
+if ((s->pc & 3) == 0) {
+/* At a 4-aligned address we can't be crossing a page */
+return false;
+}
+
+/* This must be a Thumb insn */
+insn = arm_lduw_code(env, s->pc, s->bswap_code);
+
+if ((insn >> 11) >= 0x1d) {
+/* Top five bits 0b11101 / 0b0 / 0b1 : this is the
+ * First half of a 32-bit Thumb insn. Thumb-1 cores might
+ * end up actually treating this as two 16-bit insns (see the
+ * code at the start of disas_thumb2_insn()) but we don't bother
+ * to check for that as it is unlikely, and false positives here
+ * are harmless.
+ */
+return true;
+}
+/* Definitely a 16-bit insn, can't be crossing a page. */
+return false;
+}
+
 /* generate intermediate code in gen_opc_buf and gen_opparam_buf for
basic block 'tb'.  */
 void gen_intermediate_code(CPUARMState *env, TranslationBlock *tb)
@@ -11190,6 +11219,7 @@ void gen_intermediate_code(CPUARMState *env, 
TranslationBlock *tb)
 target_ulong next_page_start;
 int num_insns;
 int max_insns;
+bool end_of_page;
 
 /* generate intermediate code */
 
@@ -11411,11 +11441,24 @@ void gen_intermediate_code(CPUARMState *env, 
TranslationBlock *tb)
  * Otherwise the subsequent code could get translated several times.
  * Also stop translation when a page boundary is reached.  This
  * ensures prefetch aborts occur at the right place.  */
+
+/* We want to stop the TB if the next insn starts in a new page,
+ * or if it spans between this page and the next. This means that
+ * if we're looking at the last halfword in the page we need to
+ * see if it's a 16-bit Thumb insn (which will fit in this TB)
+ * or a 32-bit Thumb insn (which won't).
+ * This is to avoid generating a silly TB with a single 16-bit insn
+ * in it at the end of this page (which would execute correctly
+ * but isn't very efficient).
+ */
+end_of_page = (dc->pc >= next_page_start) ||
+((dc->pc >= next_page_start - 3) && insn_crosses_page(env, dc));
+
 } while (!dc->is_jmp && !tcg_op_buf_full() &&
  !cs->singlestep_enabled &&
  !singlestep &&
  !dc->ss_active &&
- dc->pc < next_page_start &&
+ !end_of_page &&
  num_insns < max_insns);
 
 if (tb->cflags & CF_LAST_IO) {
-- 
1.9.1

[Qemu-devel] [PATCH v2 2/3] ui/curses: Support line graphics chars on -curses mode

[OGAWA Hirofumi]

This converts vga code to curses code in console_write_bh().

With this changes, we can see line graphics (for example, dialog uses)
correctly.

Signed-off-by: OGAWA Hirofumi 
---

 include/ui/console.h |   12 +++-
 ui/curses.c  |   44 
 2 files changed, 55 insertions(+), 1 deletion(-)

diff -puN include/ui/console.h~support-curses-border include/ui/console.h
--- qemu/include/ui/console.h~support-curses-border 2015-10-19 
21:16:50.439929458 +0900
+++ qemu-hirofumi/include/ui/console.h  2015-10-19 21:17:57.939760088 +0900
@@ -284,13 +284,23 @@ static inline pixman_format_code_t surfa
 #ifdef CONFIG_CURSES
 #include 
 typedef chtype console_ch_t;
+extern chtype vga_to_curses[];
 #else
 typedef unsigned long console_ch_t;
 #endif
 static inline void console_write_ch(console_ch_t *dest, uint32_t ch)
 {
-if (!(ch & 0xff))
+uint8_t c = ch;
+#ifdef CONFIG_CURSES
+if (vga_to_curses[c]) {
+ch &= ~(console_ch_t)0xff;
+ch |= vga_to_curses[c];
+}
+#else
+if (c == '\0') {
 ch |= ' ';
+}
+#endif
 *dest = ch;
 }
 
diff -puN ui/curses.c~support-curses-border ui/curses.c
--- qemu/ui/curses.c~support-curses-border  2015-10-19 21:16:50.439929458 
+0900
+++ qemu-hirofumi/ui/curses.c   2015-10-19 21:17:09.587881354 +0900
@@ -42,6 +42,8 @@ static WINDOW *screenpad = NULL;
 static int width, height, gwidth, gheight, invalidate;
 static int px, py, sminx, sminy, smaxx, smaxy;
 
+chtype vga_to_curses[256];
+
 static void curses_update(DisplayChangeListener *dcl,
   int x, int y, int w, int h)
 {
@@ -348,6 +350,48 @@ static void curses_setup(void)
 for (i = 64; i < COLOR_PAIRS; i++) {
 init_pair(i, COLOR_WHITE, COLOR_BLACK);
 }
+
+/*
+ * Setup mapping for vga to curses line graphics.
+ * FIXME: for better font, have to use ncursesw and setlocale()
+ */
+#if 0
+/* FIXME: map from where? */
+ACS_S1;
+ACS_S3;
+ACS_S7;
+ACS_S9;
+#endif
+/* ACS_* is not constant. So, we can't initialize statically. */
+vga_to_curses['\0'] = ' ';
+vga_to_curses[0x04] = ACS_DIAMOND;
+vga_to_curses[0x0a] = ACS_RARROW;
+vga_to_curses[0x0b] = ACS_LARROW;
+vga_to_curses[0x18] = ACS_UARROW;
+vga_to_curses[0x19] = ACS_DARROW;
+vga_to_curses[0x9c] = ACS_STERLING;
+vga_to_curses[0xb0] = ACS_BOARD;
+vga_to_curses[0xb1] = ACS_CKBOARD;
+vga_to_curses[0xb3] = ACS_VLINE;
+vga_to_curses[0xb4] = ACS_RTEE;
+vga_to_curses[0xbf] = ACS_URCORNER;
+vga_to_curses[0xc0] = ACS_LLCORNER;
+vga_to_curses[0xc1] = ACS_BTEE;
+vga_to_curses[0xc2] = ACS_TTEE;
+vga_to_curses[0xc3] = ACS_LTEE;
+vga_to_curses[0xc4] = ACS_HLINE;
+vga_to_curses[0xc5] = ACS_PLUS;
+vga_to_curses[0xce] = ACS_LANTERN;
+vga_to_curses[0xd8] = ACS_NEQUAL;
+vga_to_curses[0xd9] = ACS_LRCORNER;
+vga_to_curses[0xda] = ACS_ULCORNER;
+vga_to_curses[0xdb] = ACS_BLOCK;
+vga_to_curses[0xe3] = ACS_PI;
+vga_to_curses[0xf1] = ACS_PLMINUS;
+vga_to_curses[0xf2] = ACS_GEQUAL;
+vga_to_curses[0xf3] = ACS_LEQUAL;
+vga_to_curses[0xf8] = ACS_DEGREE;
+vga_to_curses[0xfe] = ACS_BULLET;
 }
 
 static void curses_keyboard_setup(void)
_

Re: [Qemu-devel] [PATCH v6 2/4] quorum: implement bdrv_add_child() and bdrv_del_child()

[Alberto Garcia]

On Fri 16 Oct 2015 10:57:44 AM CEST, Wen Congyang wrote:
> +static void quorum_add_child(BlockDriverState *bs, BlockDriverState 
> *child_bs,
> + Error **errp)
> +{
> +BDRVQuorumState *s = bs->opaque;
> +BdrvChild *child;
> +
> +bdrv_drain(bs);
> +
> +assert(s->num_children <= INT_MAX / sizeof(BdrvChild *));
> +if (s->num_children == INT_MAX / sizeof(BdrvChild *)) {
> +error_setg(errp, "Too many children");
> +return;
> +}

This limit guarantees that s->num_children <= INT_MAX and that
s->num_children * sizeof(BdrvChild *) <= SIZE_MAX, right?

> +s->children = g_renew(BdrvChild *, s->children, s->num_children + 1);
> +
> +bdrv_ref(child_bs);
> +child = bdrv_attach_child(bs, child_bs, _format);
> +s->children[s->num_children++] = child;
> +}

Maybe you want to use ++s->num_children in the g_renew() call to make it
symmetric to the one in quorum_del_child()...

> +/* We can safely remove this child now */
> +memmove(>children[i], >children[i + 1],
> +(s->num_children - i - 1) * sizeof(void *));
> +s->children = g_renew(BdrvChild *, s->children, --s->num_children);
> +bdrv_unref_child(bs, child);
> +}

But it's not really important, so you can leave it as it is now if you
prefer.

Reviewed-by: Alberto Garcia 

Berto

[Qemu-devel] [PATCH v2 1/3] ui/curses: Fix monitor color with -curses when 256 colors

[OGAWA Hirofumi]

If TERM=xterm-256color, COLOR_PAIRS==256 and monitor passes chtype
like 0x74xx. Then, the code uses uninitialized color pair. As result,
monitor uses black for both of fg and bg color, i.e. terminal is
filled by black.

To fix, this initialize above than 64 with default color (fg=white,bg=black).

FIXME: on 256 color, curses may be possible better vga color emulation.

Signed-off-by: OGAWA Hirofumi 
---

 ui/curses.c |7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff -puN ui/curses.c~support-curses-256color ui/curses.c
--- qemu/ui/curses.c~support-curses-256color2015-10-14 20:12:06.311051365 
+0900
+++ qemu-hirofumi/ui/curses.c   2015-10-19 21:16:24.595994457 +0900
@@ -341,8 +341,13 @@ static void curses_setup(void)
 nodelay(stdscr, TRUE); nonl(); keypad(stdscr, TRUE);
 start_color(); raw(); scrollok(stdscr, FALSE);
 
-for (i = 0; i < 64; i ++)
+for (i = 0; i < 64; i++) {
 init_pair(i, colour_default[i & 7], colour_default[i >> 3]);
+}
+/* Set default color for more than 64. (monitor uses 0x74xx for example) */
+for (i = 64; i < COLOR_PAIRS; i++) {
+init_pair(i, COLOR_WHITE, COLOR_BLACK);
+}
 }
 
 static void curses_keyboard_setup(void)
_

Re: [Qemu-devel] [PULL 0/7] fw_cfg: add dma interface, add strings via cmdline.

[Marc Marí]

On Mon, 19 Oct 2015 08:12:22 -0400
"Kevin O'Connor"  wrote:

> On Mon, Oct 19, 2015 at 01:02:41PM +0100, Peter Maydell wrote:
> > On 19 October 2015 at 12:52, Kevin O'Connor 
> > wrote:
> > > On Mon, Oct 19, 2015 at 12:12:34PM +0100, Peter Maydell wrote:
> > >> Windows fails to compile:
> > >> /home/petmay01/linaro/qemu-for-merges/hw/nvram/fw_cfg.c: In
> > >> function ‘fw_cfg_dma_mem_read’:
> > >> /home/petmay01/linaro/qemu-for-merges/hw/nvram/fw_cfg.c:406:
> > >> warning: integer constant is too large for ‘long’ type
> > >
> > > I don't have a Windows test environment, but I suspect the
> > > following:
> > >
> > > #define FW_CFG_DMA_SIGNATURE 0x51454d5520434647 /* "QEMU CFG" */
> > >
> > > should be changed to:
> > >
> > > #define FW_CFG_DMA_SIGNATURE 0x51454d5520434647ULL /* "QEMU CFG"
> > > */
> > 
> > Yes. All 64 bit constants should have an LL or ULL suffix.
> 
> Okay - I'll respin patch 7.

Thanks
 
> > (The test failure is presumably something different.)
> 
> I think the fix for the test failure is to make the test less picky
> (it should accept a feature bitmap of either 1 or 3).  But I'll leave
> that to Marc or Gerd.

Yes, that's the solution. We updated the version number, and the test
expects exclusively version 1. And it would also be nice to update the
test to test also fw_cfg DMA

I can send a patch for the test (at least the version) tomorrow.

Thanks
Marc

[Qemu-devel] [PULL 28/49] qemu-char: convert parallel backend to data-driven creation

[Paolo Bonzini]

Conversion to Error * brings better error messages; before:

qemu-system-x86_64: -chardev id=serial,backend=parallel,path=vl.c: Failed 
to create chardev

After:

qemu-system-x86_64: -chardev id=serial,backend=parallel,path=vl.c: not a 
parallel port: Inappropriate ioctl for device

Reviewed-by: Eric Blake 
Signed-off-by: Paolo Bonzini 
---
 qemu-char.c | 31 +++
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 8567580..ee6381b 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1753,12 +1753,13 @@ static void pp_close(CharDriverState *chr)
 qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
 }
 
-static CharDriverState *qemu_chr_open_pp_fd(int fd)
+static CharDriverState *qemu_chr_open_pp_fd(int fd, Error **errp)
 {
 CharDriverState *chr;
 ParallelCharDriver *drv;
 
 if (ioctl(fd, PPCLAIM) < 0) {
+error_setg_errno(errp, errno, "not a parallel port");
 close(fd);
 return NULL;
 }
@@ -1818,7 +1819,7 @@ static int pp_ioctl(CharDriverState *chr, int cmd, void 
*arg)
 return 0;
 }
 
-static CharDriverState *qemu_chr_open_pp_fd(int fd)
+static CharDriverState *qemu_chr_open_pp_fd(int fd, Error **errp)
 {
 CharDriverState *chr;
 
@@ -3481,6 +3482,7 @@ static void qemu_chr_parse_serial(QemuOpts *opts, 
ChardevBackend *backend,
 }
 #endif
 
+#ifdef HAVE_CHARDEV_PARPORT
 static void qemu_chr_parse_parallel(QemuOpts *opts, ChardevBackend *backend,
 Error **errp)
 {
@@ -3493,6 +3495,7 @@ static void qemu_chr_parse_parallel(QemuOpts *opts, 
ChardevBackend *backend,
 backend->parallel = g_new0(ChardevHostdev, 1);
 backend->parallel->device = g_strdup(device);
 }
+#endif
 
 static void qemu_chr_parse_pipe(QemuOpts *opts, ChardevBackend *backend,
 Error **errp)
@@ -4044,13 +4047,6 @@ static CharDriverState *qmp_chardev_open_serial(const 
char *id,
 return qemu_chr_open_win_path(serial->device, errp);
 }
 
-static CharDriverState *qmp_chardev_open_parallel(ChardevHostdev *parallel,
-  Error **errp)
-{
-error_setg(errp, "character device backend type 'parallel' not supported");
-return NULL;
-}
-
 #else /* WIN32 */
 
 static int qmp_chardev_open_file_source(char *src, int flags,
@@ -4110,16 +4106,19 @@ static CharDriverState *qmp_chardev_open_serial(const 
char *id,
 #endif
 
 #ifdef HAVE_CHARDEV_PARPORT
-static CharDriverState *qmp_chardev_open_parallel(ChardevHostdev *parallel,
+static CharDriverState *qmp_chardev_open_parallel(const char *id,
+  ChardevBackend *backend,
+  ChardevReturn *ret,
   Error **errp)
 {
+ChardevHostdev *parallel = backend->parallel;
 int fd;
 
 fd = qmp_chardev_open_file_source(parallel->device, O_RDWR, errp);
 if (fd < 0) {
 return NULL;
 }
-return qemu_chr_open_pp_fd(fd);
+return qemu_chr_open_pp_fd(fd, errp);
 }
 #endif
 
@@ -4265,11 +4264,9 @@ ChardevReturn *qmp_chardev_add(const char *id, 
ChardevBackend *backend,
 case CHARDEV_BACKEND_KIND_SERIAL:
 abort();
 break;
-#ifdef HAVE_CHARDEV_PARPORT
 case CHARDEV_BACKEND_KIND_PARALLEL:
-chr = qmp_chardev_open_parallel(backend->parallel, _err);
+abort();
 break;
-#endif
 case CHARDEV_BACKEND_KIND_PIPE:
 chr = qemu_chr_open_pipe(backend->pipe);
 break;
@@ -4405,10 +4402,12 @@ static void register_types(void)
 register_char_driver("tty", CHARDEV_BACKEND_KIND_SERIAL,
  qemu_chr_parse_serial, qmp_chardev_open_serial);
 #endif
+#ifdef HAVE_CHARDEV_PARPORT
 register_char_driver("parallel", CHARDEV_BACKEND_KIND_PARALLEL,
- qemu_chr_parse_parallel, NULL);
+ qemu_chr_parse_parallel, qmp_chardev_open_parallel);
 register_char_driver("parport", CHARDEV_BACKEND_KIND_PARALLEL,
- qemu_chr_parse_parallel, NULL);
+ qemu_chr_parse_parallel, qmp_chardev_open_parallel);
+#endif
 register_char_driver("pty", CHARDEV_BACKEND_KIND_PTY, NULL,
  NULL);
 register_char_driver("console", CHARDEV_BACKEND_KIND_CONSOLE, NULL,
-- 
2.5.0

Re: [Qemu-devel] [PATCH v3 22/32] nvdimm: init the address region used by NVDIMM ACPI

[Michael S. Tsirkin]

On Mon, Oct 19, 2015 at 03:44:13PM +0800, Xiao Guangrong wrote:
> 
> 
> On 10/19/2015 03:39 PM, Michael S. Tsirkin wrote:
> >On Mon, Oct 19, 2015 at 03:27:21PM +0800, Xiao Guangrong wrote:
> +nvdimm_init_memory_state(>nvdimm_memory, system_memory, 
> machine,
> + TARGET_PAGE_SIZE);
> +
> >>>
> >>>Shouldn't this be conditional on presence of the nvdimm device?
> >>>
> >>
> >>We will enable hotplug on nvdimm devices in the near future once Linux 
> >>driver is
> >>ready. I'd keep it here for future development.
> >
> >No, I don't think we should add stuff unconditionally. If not nvdimm,
> >some other flag should indicate user intends to hotplug things.
> >
> 
> Actually, it is not unconditionally which is called if parameter "-m aaa, 
> maxmem=bbb"
> (aaa < bbb) is used. It is on the some path of memoy-hotplug initiation.
> 

Right, but that's not the same as nvdimm.

Re: [Qemu-devel] [PATCH v3 22/32] nvdimm: init the address region used by NVDIMM ACPI

[Igor Mammedov]

On Mon, 19 Oct 2015 12:17:22 +0300
"Michael S. Tsirkin"  wrote:

> On Mon, Oct 19, 2015 at 03:44:13PM +0800, Xiao Guangrong wrote:
> > 
> > 
> > On 10/19/2015 03:39 PM, Michael S. Tsirkin wrote:
> > >On Mon, Oct 19, 2015 at 03:27:21PM +0800, Xiao Guangrong wrote:
> > +nvdimm_init_memory_state(>nvdimm_memory,
> > system_memory, machine,
> > + TARGET_PAGE_SIZE);
> > +
> > >>>
> > >>>Shouldn't this be conditional on presence of the nvdimm device?
> > >>>
> > >>
> > >>We will enable hotplug on nvdimm devices in the near future once
> > >>Linux driver is ready. I'd keep it here for future development.
> > >
> > >No, I don't think we should add stuff unconditionally. If not
> > >nvdimm, some other flag should indicate user intends to hotplug
> > >things.
> > >
> > 
> > Actually, it is not unconditionally which is called if parameter
> > "-m aaa, maxmem=bbb" (aaa < bbb) is used. It is on the some path of
> > memoy-hotplug initiation.
> > 
> 
> Right, but that's not the same as nvdimm.
> 

it could be pc-machine property, then it could be turned on like this:
 -machine nvdimm_support=on

Re: [Qemu-devel] [PATCH v3 22/32] nvdimm: init the address region used by NVDIMM ACPI

[Igor Mammedov]

On Mon, 19 Oct 2015 09:56:12 +0300
"Michael S. Tsirkin"  wrote:

> On Sun, Oct 11, 2015 at 11:52:54AM +0800, Xiao Guangrong wrote:
[...]
> > diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
> > index f6bd2c4..aa95961 100644
> > --- a/include/hw/mem/nvdimm.h
> > +++ b/include/hw/mem/nvdimm.h
> > @@ -15,6 +15,10 @@
> >  
> >  #include "hw/mem/dimm.h"
> >  
> > +/* Memory region 0xFF0 ~ 0xFFF0 is reserved for NVDIMM
> > ACPI. */ +#define NVDIMM_ACPI_MEM_BASE   0xFF00ULL
Michael,

If it's ok to map control RAM region directly from QEMU at arbitrary
location let's do the same for VMGENID too (i.e. use v16
implementation which does exactly the same thing as this series).

> > +#define NVDIMM_ACPI_MEM_SIZE   0xF0ULL
[...]

[Qemu-devel] [PULL 6/7] Enable fw_cfg DMA interface for x86

[Gerd Hoffmann]

From: Marc Marí 

Enable the fw_cfg DMA interface for all the x86 platforms.

Based on Gerd Hoffman's initial implementation.

Signed-off-by: Marc Marí 
Reviewed-by: Laszlo Ersek 
Signed-off-by: Gerd Hoffmann 
---
 hw/i386/pc.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 682867a..b25a872 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -752,14 +752,15 @@ static void pc_build_smbios(FWCfgState *fw_cfg)
 }
 }
 
-static FWCfgState *bochs_bios_init(void)
+static FWCfgState *bochs_bios_init(AddressSpace *as)
 {
 FWCfgState *fw_cfg;
 uint64_t *numa_fw_cfg;
 int i, j;
 unsigned int apic_id_limit = pc_apic_id_limit(max_cpus);
 
-fw_cfg = fw_cfg_init_io(BIOS_CFG_IOPORT);
+fw_cfg = fw_cfg_init_io_dma(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 4, as);
+
 /* FW_CFG_MAX_CPUS is a bit confusing/problematic on x86:
  *
  * SeaBIOS needs FW_CFG_MAX_CPUS for CPU hotplug, but the CPU hotplug
@@ -1393,7 +1394,8 @@ FWCfgState *pc_memory_init(PCMachineState *pcms,
 option_rom_mr,
 1);
 
-fw_cfg = bochs_bios_init();
+fw_cfg = bochs_bios_init(_space_memory);
+
 rom_set_fw(fw_cfg);
 
 if (guest_info->has_reserved_memory && pcms->hotplug_memory.base) {
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH COLO-Frame v9 08/32] COLO/migration: establish a new communication path from destination to source

[Dr. David Alan Gilbert]

* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
> Add a new member 'to_src_file' to MigrationIncomingState and a
> new member 'from_dst_file' to MigrationState.
> They will be used for returning messages from destination to source.
> It will also be used by post-copy migration.
> 
> Signed-off-by: zhanghailiang 
> Signed-off-by: Li Zhijian 
> Cc: Dr. David Alan Gilbert 
> ---
>  include/migration/migration.h |  3 ++-
>  migration/colo.c  | 43 
> +++
>  2 files changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index 6488e03..0c94103 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -50,7 +50,7 @@ typedef QLIST_HEAD(, LoadStateEntry) LoadStateEntry_Head;
>  /* State for the incoming migration */
>  struct MigrationIncomingState {
>  QEMUFile *from_src_file;
> -
> +QEMUFile *to_src_file;
>  int state;
>  
>  bool have_colo_incoming_thread;
> @@ -74,6 +74,7 @@ struct MigrationState
>  QemuThread thread;
>  QEMUBH *cleanup_bh;
>  QEMUFile *to_dst_file;
> +QEMUFile  *from_dst_file;
>  int parameters[MIGRATION_PARAMETER_MAX];
>  
>  int state;
> diff --git a/migration/colo.c b/migration/colo.c
> index a341eee..5f4fb20 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -39,6 +39,20 @@ bool migration_incoming_in_colo_state(void)
>  static void *colo_thread(void *opaque)
>  {
>  MigrationState *s = opaque;
> +int fd, ret = 0;
> +
> +/* Dup the fd of to_dst_file */
> +fd = dup(qemu_get_fd(s->to_dst_file));
> +if (fd == -1) {
> +ret = -errno;
> +goto out;
> +}
> +s->from_dst_file = qemu_fopen_socket(fd, "rb");

In my postcopy code I add the return-path opening as a new
method on QEMUFile, that way if we get a return path working
on another transport (RDMA which I hope to do) then it
works;  have a look at 'Return path: Open a return path on QEMUFile for sockets'

> +if (!s->from_dst_file) {
> +ret = -EINVAL;
> +error_report("Open QEMUFile failed!");

In errors, try to give detail of where a problem was;
e.g. 'colo_thread: Open QEMUFile from_dst failed'.

> +goto out;
> +}
>  
>  qemu_mutex_lock_iothread();
>  vm_start();
> @@ -47,9 +61,17 @@ static void *colo_thread(void *opaque)
>  
>  /*TODO: COLO checkpoint savevm loop*/
>  
> +out:
> +if (ret < 0) {
> +error_report("Detect some error: %s", strerror(-ret));
> +}

Again, best to say where the error happened.


>  migrate_set_state(>state, MIGRATION_STATUS_COLO,
>MIGRATION_STATUS_COMPLETED);
>  
> +if (s->from_dst_file) {
> +qemu_fclose(s->from_dst_file);
> +}
> +
>  qemu_mutex_lock_iothread();
>  qemu_bh_schedule(s->cleanup_bh);
>  qemu_mutex_unlock_iothread();
> @@ -86,12 +108,33 @@ void colo_init_checkpointer(MigrationState *s)
>  void *colo_process_incoming_thread(void *opaque)
>  {
>  MigrationIncomingState *mis = opaque;
> +int fd, ret = 0;
>  
>  migrate_set_state(>state, MIGRATION_STATUS_ACTIVE,
>MIGRATION_STATUS_COLO);
>  
> +fd = dup(qemu_get_fd(mis->from_src_file));
> +if (fd < 0) {
> +ret = -errno;
> +goto out;
> +}
> +mis->to_src_file = qemu_fopen_socket(fd, "wb");
> +if (!mis->to_src_file) {
> +ret = -EINVAL;
> +error_report("Can't open incoming channel!");
> +goto out;
> +}

Same as above.

Dave

>  /* TODO: COLO checkpoint restore loop */
>  
> +out:
> +if (ret < 0) {

> +error_report("colo incoming thread will exit, detect error: %s",
> + strerror(-ret));
> +}
> +
> +if (mis->to_src_file) {
> +qemu_fclose(mis->to_src_file);
> +}
>  migration_incoming_exit_colo();
>  
>  return NULL;
> -- 
> 1.8.3.1
> 
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v3 22/32] nvdimm: init the address region used by NVDIMM ACPI

[Xiao Guangrong]

On 10/19/2015 05:46 PM, Igor Mammedov wrote:

On Mon, 19 Oct 2015 12:17:22 +0300
"Michael S. Tsirkin"  wrote:


On Mon, Oct 19, 2015 at 03:44:13PM +0800, Xiao Guangrong wrote:



On 10/19/2015 03:39 PM, Michael S. Tsirkin wrote:

On Mon, Oct 19, 2015 at 03:27:21PM +0800, Xiao Guangrong wrote:

+nvdimm_init_memory_state(>nvdimm_memory,
system_memory, machine,
+ TARGET_PAGE_SIZE);
+


Shouldn't this be conditional on presence of the nvdimm device?



We will enable hotplug on nvdimm devices in the near future once
Linux driver is ready. I'd keep it here for future development.


No, I don't think we should add stuff unconditionally. If not
nvdimm, some other flag should indicate user intends to hotplug
things.



Actually, it is not unconditionally which is called if parameter
"-m aaa, maxmem=bbb" (aaa < bbb) is used. It is on the some path of
memoy-hotplug initiation.



Right, but that's not the same as nvdimm.



it could be pc-machine property, then it could be turned on like this:
  -machine nvdimm_support=on


Er, I do not understand why this separate switch is needed and why nvdimm
and pc-dimm is different. :(

NVDIMM reuses memory-hotplug's framework, such as maxmem, slot, and dimm
device, even some of ACPI logic to do hotplug things, etc. Both nvdimm
and pc-dimm are built on the same infrastructure.

Re: [Qemu-devel] [PATCH COLO-Frame v9 06/32] migration: Integrate COLO checkpoint process into loadvm

[Dr. David Alan Gilbert]

* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
> Switch from normal migration loadvm process into COLO checkpoint process if
> COLO mode is enabled.
> We add three new members to struct MigrationIncomingState, 
> 'have_colo_incoming_thread'
> and 'colo_incoming_thread' record the colo related threads for secondary VM,
> 'migration_incoming_co' records the original migration incoming coroutine.
> 
> Signed-off-by: zhanghailiang 
> Signed-off-by: Li Zhijian 
> Signed-off-by: Yang Hongyang 

Mostly OK, some mostly minor comments, and one question below:

> ---
>  include/migration/colo.h  |  7 +++
>  include/migration/migration.h |  7 +++
>  migration/colo-comm.c | 10 ++
>  migration/colo.c  | 22 ++
>  migration/migration.c | 33 +++--
>  stubs/migration-colo.c| 10 ++
>  trace-events  |  1 +
>  7 files changed, 80 insertions(+), 10 deletions(-)
> 
> diff --git a/include/migration/colo.h b/include/migration/colo.h
> index dface19..58849f7 100644
> --- a/include/migration/colo.h
> +++ b/include/migration/colo.h
> @@ -15,6 +15,8 @@
>  
>  #include "qemu-common.h"
>  #include "migration/migration.h"
> +#include "block/coroutine.h"
> +#include "qemu/thread.h"
>  
>  bool colo_supported(void);
>  void colo_info_mig_init(void);
> @@ -22,4 +24,9 @@ void colo_info_mig_init(void);
>  void colo_init_checkpointer(MigrationState *s);
>  bool migration_in_colo_state(void);
>  
> +/* loadvm */
> +bool migration_incoming_enable_colo(void);
> +void migration_incoming_exit_colo(void);
> +void *colo_process_incoming_thread(void *opaque);
> +bool migration_incoming_in_colo_state(void);
>  #endif
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index a62068f..9cdd6b6 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -22,6 +22,7 @@
>  #include "migration/vmstate.h"
>  #include "qapi-types.h"
>  #include "exec/cpu-common.h"
> +#include "block/coroutine.h"
>  
>  #define QEMU_VM_FILE_MAGIC   0x5145564d
>  #define QEMU_VM_FILE_VERSION_COMPAT  0x0002
> @@ -51,6 +52,12 @@ struct MigrationIncomingState {
>  QEMUFile *file;
>  
>  int state;
> +
> +bool have_colo_incoming_thread;
> +QemuThread colo_incoming_thread;
> +/* The coroutine we should enter (back) after failover */
> +Coroutine *migration_incoming_co;
> +
>  /* See savevm.c */
>  LoadStateEntry_Head loadvm_handlers;
>  };
> diff --git a/migration/colo-comm.c b/migration/colo-comm.c
> index 4330bd8..0808d6c 100644
> --- a/migration/colo-comm.c
> +++ b/migration/colo-comm.c
> @@ -52,3 +52,13 @@ void colo_info_mig_init(void)
>  {
>  vmstate_register(NULL, 0, _state, _info);
>  }
> +
> +bool migration_incoming_enable_colo(void)
> +{
> +return colo_info.colo_requested;
> +}
> +
> +void migration_incoming_exit_colo(void)
> +{
> +colo_info.colo_requested = 0;
> +}
> diff --git a/migration/colo.c b/migration/colo.c
> index 97e64a3..a341eee 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -13,6 +13,7 @@
>  #include "sysemu/sysemu.h"
>  #include "migration/colo.h"
>  #include "trace.h"
> +#include "qemu/error-report.h"
>  
>  static QEMUBH *colo_bh;
>  
> @@ -28,6 +29,13 @@ bool migration_in_colo_state(void)
>  return (s->state == MIGRATION_STATUS_COLO);
>  }
>  
> +bool migration_incoming_in_colo_state(void)
> +{
> +MigrationIncomingState *mis = migration_incoming_get_current();
> +
> +return (mis && (mis->state == MIGRATION_STATUS_COLO));

Can remove outer brackets.

> +}
> +
>  static void *colo_thread(void *opaque)
>  {
>  MigrationState *s = opaque;
> @@ -74,3 +82,17 @@ void colo_init_checkpointer(MigrationState *s)
>  colo_bh = qemu_bh_new(colo_start_checkpointer, s);
>  qemu_bh_schedule(colo_bh);
>  }
> +
> +void *colo_process_incoming_thread(void *opaque)
> +{
> +MigrationIncomingState *mis = opaque;
> +
> +migrate_set_state(>state, MIGRATION_STATUS_ACTIVE,
> +  MIGRATION_STATUS_COLO);
> +
> +/* TODO: COLO checkpoint restore loop */
> +
> +migration_incoming_exit_colo();
> +
> +return NULL;
> +}
> diff --git a/migration/migration.c b/migration/migration.c
> index bee61aa..241689f 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -280,7 +280,28 @@ static void process_incoming_migration_co(void *opaque)
>MIGRATION_STATUS_ACTIVE);
>  ret = qemu_loadvm_state(f);
>  
> -qemu_fclose(f);
> +if (!ret) {
> +/* Make sure all file formats flush their mutable metadata */
> +bdrv_invalidate_cache_all(_err);
> +if (local_err) {
> +error_report_err(local_err);
> +migrate_decompress_threads_join();
> +exit(EXIT_FAILURE);
> +}
> +}
> +/* we get colo 

[Qemu-devel] [PULL 0/7] fw_cfg: add dma interface, add strings via cmdline.

[Gerd Hoffmann]

  Hi,

As promised last week I've picked up the fw_cfg patches which are ready
to go, so here comes the pull request for them.  Headline feature is
the fw_cfg dam support fo course, enabled for arm and x86.  Also small
fixes and an option to add string fw_cfg entries via command line.

please pull,
  Gerd

The following changes since commit 40fe17bea478793fc9106a630fa3610dad51f939:

  hw/ide/ahci.c: Fix shift left into sign bit (2015-10-18 11:00:40 +0100)

are available in the git repository at:

  git://git.kraxel.org/qemu tags/pull-fw_cfg-20151019-1

for you to fetch changes up to 7b0eec285d447057405df73beae78c8e8aeb9793:

  fw_cfg: Define a static signature to be returned on DMA port reads 
(2015-10-19 11:00:50 +0200)


fw_cfg: add dma interface, add strings via cmdline.


Gabriel L. Somlo (2):
  fw_cfg: insert string blobs via qemu cmdline
  fw_cfg: document fw_cfg_modify_iXX() update functions

Kevin O'Connor (1):
  fw_cfg: Define a static signature to be returned on DMA port reads

Marc Marí (4):
  fw_cfg DMA interface documentation
  Implement fw_cfg DMA interface
  Enable fw_cfg DMA interface for ARM
  Enable fw_cfg DMA interface for x86

 docs/specs/fw_cfg.txt |  94 -
 hw/arm/virt.c |   8 +-
 hw/i386/pc.c  |   8 +-
 hw/nvram/fw_cfg.c | 250 --
 include/hw/nvram/fw_cfg.h |  16 ++-
 qemu-options.hx   |   7 +-
 vl.c  |  33 --
 7 files changed, 385 insertions(+), 31 deletions(-)

[Qemu-devel] [PULL 7/7] fw_cfg: Define a static signature to be returned on DMA port reads

[Gerd Hoffmann]

From: Kevin O'Connor 

Return a static signature ("QEMU CFG") if the guest does a read to the
DMA address io register.

Signed-off-by: Kevin O'Connor 
Reviewed-by: Laszlo Ersek 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Gerd Hoffmann 
---
 docs/specs/fw_cfg.txt |  3 +++
 hw/nvram/fw_cfg.c | 14 --
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
index 536909a..b8c794f 100644
--- a/docs/specs/fw_cfg.txt
+++ b/docs/specs/fw_cfg.txt
@@ -93,6 +93,9 @@ by selecting the "signature" item using key 0x 
(FW_CFG_SIGNATURE),
 and reading four bytes from the data register. If the fw_cfg device is
 present, the four bytes read will contain the characters "QEMU".
 
+If the DMA interface is available, then reading the DMA Address
+Register returns 0x51454d5520434647 ("QEMU CFG" in big-endian format).
+
 === Revision / feature bitmap (Key 0x0001, FW_CFG_ID) ===
 
 A 32-bit little-endian unsigned int, this item is used to check for enabled
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 91829d4..69d674f 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -53,6 +53,8 @@
 #define FW_CFG_DMA_CTL_SKIP0x04
 #define FW_CFG_DMA_CTL_SELECT  0x08
 
+#define FW_CFG_DMA_SIGNATURE 0x51454d5520434647 /* "QEMU CFG" */
+
 typedef struct FWCfgEntry {
 uint32_t len;
 uint8_t *data;
@@ -397,6 +399,13 @@ static void fw_cfg_dma_transfer(FWCfgState *s)
 trace_fw_cfg_read(s, 0);
 }
 
+static uint64_t fw_cfg_dma_mem_read(void *opaque, hwaddr addr,
+unsigned size)
+{
+/* Return a signature value (and handle various read sizes) */
+return extract64(FW_CFG_DMA_SIGNATURE, (8 - addr - size) * 8, size * 8);
+}
+
 static void fw_cfg_dma_mem_write(void *opaque, hwaddr addr,
  uint64_t value, unsigned size)
 {
@@ -420,8 +429,8 @@ static void fw_cfg_dma_mem_write(void *opaque, hwaddr addr,
 static bool fw_cfg_dma_mem_valid(void *opaque, hwaddr addr,
   unsigned size, bool is_write)
 {
-return is_write && ((size == 4 && (addr == 0 || addr == 4)) ||
-(size == 8 && addr == 0));
+return !is_write || ((size == 4 && (addr == 0 || addr == 4)) ||
+ (size == 8 && addr == 0));
 }
 
 static bool fw_cfg_data_mem_valid(void *opaque, hwaddr addr,
@@ -492,6 +501,7 @@ static const MemoryRegionOps fw_cfg_comb_mem_ops = {
 };
 
 static const MemoryRegionOps fw_cfg_dma_mem_ops = {
+.read = fw_cfg_dma_mem_read,
 .write = fw_cfg_dma_mem_write,
 .endianness = DEVICE_BIG_ENDIAN,
 .valid.accepts = fw_cfg_dma_mem_valid,
-- 
1.8.3.1

[Qemu-devel] [PULL 1/2] pc: Require xen when initializing xenfv machine

[Stefano Stabellini]

From: Eduardo Habkost 

Without this check, the xen-platform device will crash on reset
if using the accel option with anything other than xen (e.g.
"-machine xenfv,accel=kvm").

Signed-off-by: Eduardo Habkost 
Reviewed-by: Stefano Stabellini 
Signed-off-by: Stefano Stabellini 
---
 hw/i386/pc_piix.c |5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index ae7bbeb..a91cc3d 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -434,6 +434,11 @@ static void pc_xen_hvm_init(MachineState *machine)
 {
 PCIBus *bus;
 
+if (!xen_enabled()) {
+error_report("xenfv machine requires the xen accelerator");
+exit(1);
+}
+
 pc_xen_hvm_init_pci(machine);
 
 bus = pci_find_primary_bus();
-- 
1.7.10.4

[Qemu-devel] [PULL 2/2] xen-platform: Ensure xen is enabled when initializing

[Stefano Stabellini]

From: Eduardo Habkost 

The xen-platform code crashes on reset if the xen backend is not
initialized, because it calls xc_hvm_set_mem_type(). Ensure xen-platform
won't be created without initializing the xen backend.

The assert can't be triggered by the user because the device is not
hotpluggable, and the only code creating it (at pc_xen_hvm_init())
already checks xen_enabled().

Signed-off-by: Eduardo Habkost 
Reviewed-by: Stefano Stabellini 
Signed-off-by: Stefano Stabellini 
---
 hw/i386/xen/xen_platform.c |3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
index ee45f03..8682c42 100644
--- a/hw/i386/xen/xen_platform.c
+++ b/hw/i386/xen/xen_platform.c
@@ -387,6 +387,9 @@ static int xen_platform_initfn(PCIDevice *dev)
 PCIXenPlatformState *d = XEN_PLATFORM(dev);
 uint8_t *pci_conf;
 
+/* Device will crash on reset if xen is not initialized */
+assert(xen_enabled());
+
 pci_conf = dev->config;
 
 pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
-- 
1.7.10.4

Re: [Qemu-devel] [PULL v2 00/49] Misc patches for 2015-10-16

[Peter Maydell]

On 19 October 2015 at 09:38, Paolo Bonzini  wrote:
> The following changes since commit 5451316ed07b758a187dedf21047bed8f843f7f1:
>
>   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' 
> into staging (2015-10-12 15:52:54 +0100)
>
> are available in the git repository at:
>
>   git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 1c4a55dbed9a47fde9294f7de6c8bb060d874c88:
>
>   kvm: Allow the Hyper-V vendor ID to be specified (2015-10-19 10:13:07 +0200)
>
> 
> * KVM page size fix for PPC
> * Support for Linux 4.4's new Hyper-V features
> * Eliminate g_slice from areas I maintain
> * checkpatch fix
> * Peter's cpu_reload_memory_map() cleanups
> * More changes to MAINTAINERS
> * Require Python 2.6
> * chardev creation fixes
> * PCI requester id for ARM KVM
> * cleanups and doc fixes
> * Allow customization of the Hyper-V vendor id
>

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH] qapi: Adding websocket information inside VncInfo structure.

[Gerd Hoffmann]

  Hi,

> diff --git a/qapi-schema.json b/qapi-schema.json
> index a386605..e97f78f 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -916,7 +916,8 @@
>  { 'struct': 'VncInfo',
>'data': {'enabled': 'bool', '*host': 'str',
> '*family': 'NetworkAddressFamily',
> -   '*service': 'str', '*auth': 'str', '*clients': ['VncClientInfo']} 
> }
> +   '*service': 'str', '*auth': 'str', '*clients': ['VncClientInfo'],
> +   '*webservice': 'str', 'has_websocket': 'bool'} }
>  
>  ##
>  # @VncPriAuth:

Please leave VncInfo alone.  There is VncInfo2 which should fill all
your needs.

You might consider switching hmp_info_vnc() to use the new
qmp_query_vnc_servers() instead of the old qmp_query_vnc().

cheers,
  Gerd

Re: [Qemu-devel] [PULL 0/1] audio: Remove macros IO_READ_PROTO and IO_WRITE_PROTO

[Peter Maydell]

On 19 October 2015 at 09:24, Gerd Hoffmann <kra...@redhat.com> wrote:
>   Hi,
>
> Release is coming, time to start flushing my patch queues even if there
> isn't much in there.  Starting with the one-patch audio patch queue ;)
>
> please pull,
>   Gerd
>
> PS: The gsoc audio backend work is not forgotten.  I'm just waiting for
> the dust of the qapi changes to settle ;)
>
> The following changes since commit 40fe17bea478793fc9106a630fa3610dad51f939:
>
>   hw/ide/ahci.c: Fix shift left into sign bit (2015-10-18 11:00:40 +0100)
>
> are available in the git repository at:
>
>   git://git.kraxel.org/qemu tags/pull-audio-20151019-1
>
> for you to fetch changes up to 8307c294a355bbf3c5352e00877365b0cda66d52:
>
>   Remove macros IO_READ_PROTO and IO_WRITE_PROTO (2015-10-19 09:03:53 +0200)
>
> 
> Remove macros IO_READ_PROTO and IO_WRITE_PROTO
>
> 
> Nutan Shinde (1):
>   Remove macros IO_READ_PROTO and IO_WRITE_PROTO

Applied to master, thanks.

-- PMM

[Qemu-devel] How to log DRAM access address in QEMU

[Yu-Cheng Liu]

Dear all:
I want to log out DRAM access information( address,read,write ) in QEMU
when guest OS or application use the DRAM, which function can I add the log
?

thanks for your help~

Re: [Qemu-devel] [PATCH v3 22/32] nvdimm: init the address region used by NVDIMM ACPI

[Michael S. Tsirkin]

On Mon, Oct 19, 2015 at 11:18:36AM +0200, Igor Mammedov wrote:
> On Mon, 19 Oct 2015 09:56:12 +0300
> "Michael S. Tsirkin"  wrote:
> 
> > On Sun, Oct 11, 2015 at 11:52:54AM +0800, Xiao Guangrong wrote:
> [...]
> > > diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
> > > index f6bd2c4..aa95961 100644
> > > --- a/include/hw/mem/nvdimm.h
> > > +++ b/include/hw/mem/nvdimm.h
> > > @@ -15,6 +15,10 @@
> > >  
> > >  #include "hw/mem/dimm.h"
> > >  
> > > +/* Memory region 0xFF0 ~ 0xFFF0 is reserved for NVDIMM
> > > ACPI. */ +#define NVDIMM_ACPI_MEM_BASE   0xFF00ULL
> Michael,
> 
> If it's ok to map control RAM region directly from QEMU at arbitrary
> location

It's a fair question. Where is it reserved? In which spec?

-- 
MST

[Qemu-devel] [PULL 0/2] Xen 2015-10-19-tag

[Stefano Stabellini]

The following changes since commit aedc8806172dd1ae904f04169ee3b19fce1d7893:

  Merge remote-tracking branch 'remotes/kraxel/tags/pull-audio-20151019-1' into 
staging (2015-10-19 10:06:56 +0100)

are available in the git repository at:


  git://xenbits.xen.org/people/sstabellini/qemu-dm.git tags/2015-10-19-tag

for you to fetch changes up to dbb7405d8caad0814ceddd568cb49f163a847561:

  xen-platform: Ensure xen is enabled when initializing (2015-10-19 10:16:01 
+)


Xen 2015-10-19


Eduardo Habkost (2):
  pc: Require xen when initializing xenfv machine
  xen-platform: Ensure xen is enabled when initializing

 hw/i386/pc_piix.c  |5 +
 hw/i386/xen/xen_platform.c |3 +++
 2 files changed, 8 insertions(+)

Re: [Qemu-devel] [PATCH v3 22/32] nvdimm: init the address region used by NVDIMM ACPI

[Michael S. Tsirkin]

On Mon, Oct 19, 2015 at 06:01:17PM +0800, Xiao Guangrong wrote:
> 
> 
> On 10/19/2015 05:46 PM, Igor Mammedov wrote:
> >On Mon, 19 Oct 2015 12:17:22 +0300
> >"Michael S. Tsirkin"  wrote:
> >
> >>On Mon, Oct 19, 2015 at 03:44:13PM +0800, Xiao Guangrong wrote:
> >>>
> >>>
> >>>On 10/19/2015 03:39 PM, Michael S. Tsirkin wrote:
> On Mon, Oct 19, 2015 at 03:27:21PM +0800, Xiao Guangrong wrote:
> >>>+nvdimm_init_memory_state(>nvdimm_memory,
> >>>system_memory, machine,
> >>>+ TARGET_PAGE_SIZE);
> >>>+
> >>
> >>Shouldn't this be conditional on presence of the nvdimm device?
> >>
> >
> >We will enable hotplug on nvdimm devices in the near future once
> >Linux driver is ready. I'd keep it here for future development.
> 
> No, I don't think we should add stuff unconditionally. If not
> nvdimm, some other flag should indicate user intends to hotplug
> things.
> 
> >>>
> >>>Actually, it is not unconditionally which is called if parameter
> >>>"-m aaa, maxmem=bbb" (aaa < bbb) is used. It is on the some path of
> >>>memoy-hotplug initiation.
> >>>
> >>
> >>Right, but that's not the same as nvdimm.
> >>
> >
> >it could be pc-machine property, then it could be turned on like this:
> >  -machine nvdimm_support=on
> 
> Er, I do not understand why this separate switch is needed and why nvdimm
> and pc-dimm is different. :(
> 
> NVDIMM reuses memory-hotplug's framework, such as maxmem, slot, and dimm
> device, even some of ACPI logic to do hotplug things, etc. Both nvdimm
> and pc-dimm are built on the same infrastructure.
> 
> 
> 
> 

It does seem to add a bunch of devices in ACPI and memory regions in
memory space.
Whatever your device is, it's generally safe to assume that most
people don't need it.

-- 
MST

[Qemu-devel] [PULL 0/1] audio: Remove macros IO_READ_PROTO and IO_WRITE_PROTO

[Gerd Hoffmann]

  Hi,

Release is coming, time to start flushing my patch queues even if there
isn't much in there.  Starting with the one-patch audio patch queue ;)

please pull,
  Gerd

PS: The gsoc audio backend work is not forgotten.  I'm just waiting for
the dust of the qapi changes to settle ;)

The following changes since commit 40fe17bea478793fc9106a630fa3610dad51f939:

  hw/ide/ahci.c: Fix shift left into sign bit (2015-10-18 11:00:40 +0100)

are available in the git repository at:

  git://git.kraxel.org/qemu tags/pull-audio-20151019-1

for you to fetch changes up to 8307c294a355bbf3c5352e00877365b0cda66d52:

  Remove macros IO_READ_PROTO and IO_WRITE_PROTO (2015-10-19 09:03:53 +0200)


Remove macros IO_READ_PROTO and IO_WRITE_PROTO


Nutan Shinde (1):
  Remove macros IO_READ_PROTO and IO_WRITE_PROTO

 hw/audio/adlib.c  |  9 ++---
 hw/audio/es1370.c | 17 ++---
 hw/audio/gus.c|  9 ++---
 hw/audio/sb16.c   | 15 +--
 4 files changed, 15 insertions(+), 35 deletions(-)

[Qemu-devel] [PULL 1/1] Remove macros IO_READ_PROTO and IO_WRITE_PROTO

[Gerd Hoffmann]

From: Nutan Shinde 

Signed-off-by: Nutan Shinde 
Reviewed-by: Peter Maydell 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Gerd Hoffmann 
---
 hw/audio/adlib.c  |  9 ++---
 hw/audio/es1370.c | 17 ++---
 hw/audio/gus.c|  9 ++---
 hw/audio/sb16.c   | 15 +--
 4 files changed, 15 insertions(+), 35 deletions(-)

diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c
index 656eb37..334935f 100644
--- a/hw/audio/adlib.c
+++ b/hw/audio/adlib.c
@@ -57,11 +57,6 @@ void YMF262UpdateOneQEMU (int which, INT16 *dst, int length);
 #define SHIFT 1
 #endif
 
-#define IO_READ_PROTO(name) \
-uint32_t name (void *opaque, uint32_t nport)
-#define IO_WRITE_PROTO(name) \
-void name (void *opaque, uint32_t nport, uint32_t val)
-
 #define TYPE_ADLIB "adlib"
 #define ADLIB(obj) OBJECT_CHECK(AdlibState, (obj), TYPE_ADLIB)
 
@@ -124,7 +119,7 @@ static void adlib_kill_timers (AdlibState *s)
 }
 }
 
-static IO_WRITE_PROTO (adlib_write)
+static void adlib_write(void *opaque, uint32_t nport, uint32_t val)
 {
 AdlibState *s = opaque;
 int a = nport & 3;
@@ -141,7 +136,7 @@ static IO_WRITE_PROTO (adlib_write)
 #endif
 }
 
-static IO_READ_PROTO (adlib_read)
+static uint32_t adlib_read(void *opaque, uint32_t nport)
 {
 AdlibState *s = opaque;
 uint8_t data;
diff --git a/hw/audio/es1370.c b/hw/audio/es1370.c
index 8e7bcf5..592578b 100644
--- a/hw/audio/es1370.c
+++ b/hw/audio/es1370.c
@@ -157,11 +157,6 @@ static const unsigned dac1_samplerate[] = { 5512, 11025, 
22050, 44100 };
 #define DAC2_CHANNEL 1
 #define ADC_CHANNEL 2
 
-#define IO_READ_PROTO(n) \
-static uint32_t n (void *opaque, uint32_t addr)
-#define IO_WRITE_PROTO(n) \
-static void n (void *opaque, uint32_t addr, uint32_t val)
-
 static void es1370_dac1_callback (void *opaque, int free);
 static void es1370_dac2_callback (void *opaque, int free);
 static void es1370_adc_callback (void *opaque, int avail);
@@ -474,7 +469,7 @@ static inline uint32_t es1370_fixup (ES1370State *s, 
uint32_t addr)
 return addr;
 }
 
-IO_WRITE_PROTO (es1370_writeb)
+static void es1370_writeb(void *opaque, uint32_t addr, uint32_t val)
 {
 ES1370State *s = opaque;
 uint32_t shift, mask;
@@ -512,7 +507,7 @@ IO_WRITE_PROTO (es1370_writeb)
 }
 }
 
-IO_WRITE_PROTO (es1370_writew)
+static void es1370_writew(void *opaque, uint32_t addr, uint32_t val)
 {
 ES1370State *s = opaque;
 addr = es1370_fixup (s, addr);
@@ -549,7 +544,7 @@ IO_WRITE_PROTO (es1370_writew)
 }
 }
 
-IO_WRITE_PROTO (es1370_writel)
+static void es1370_writel(void *opaque, uint32_t addr, uint32_t val)
 {
 ES1370State *s = opaque;
 struct chan *d = >chan[0];
@@ -615,7 +610,7 @@ IO_WRITE_PROTO (es1370_writel)
 }
 }
 
-IO_READ_PROTO (es1370_readb)
+static uint32_t es1370_readb(void *opaque, uint32_t addr)
 {
 ES1370State *s = opaque;
 uint32_t val;
@@ -650,7 +645,7 @@ IO_READ_PROTO (es1370_readb)
 return val;
 }
 
-IO_READ_PROTO (es1370_readw)
+static uint32_t es1370_readw(void *opaque, uint32_t addr)
 {
 ES1370State *s = opaque;
 struct chan *d = >chan[0];
@@ -692,7 +687,7 @@ IO_READ_PROTO (es1370_readw)
 return val;
 }
 
-IO_READ_PROTO (es1370_readl)
+static uint32_t es1370_readl(void *opaque, uint32_t addr)
 {
 ES1370State *s = opaque;
 uint32_t val;
diff --git a/hw/audio/gus.c b/hw/audio/gus.c
index 86223a9..e0c8a4e 100644
--- a/hw/audio/gus.c
+++ b/hw/audio/gus.c
@@ -41,11 +41,6 @@
 #define GUS_ENDIANNESS 0
 #endif
 
-#define IO_READ_PROTO(name) \
-static uint32_t name (void *opaque, uint32_t nport)
-#define IO_WRITE_PROTO(name) \
-static void name (void *opaque, uint32_t nport, uint32_t val)
-
 #define TYPE_GUS "gus"
 #define GUS(obj) OBJECT_CHECK (GUSState, (obj), TYPE_GUS)
 
@@ -64,14 +59,14 @@ typedef struct GUSState {
 qemu_irq pic;
 } GUSState;
 
-IO_READ_PROTO (gus_readb)
+static uint32_t gus_readb(void *opaque, uint32_t nport)
 {
 GUSState *s = opaque;
 
 return gus_read (>emu, nport, 1);
 }
 
-IO_WRITE_PROTO (gus_writeb)
+static void gus_writeb(void *opaque, uint32_t nport, uint32_t val)
 {
 GUSState *s = opaque;
 
diff --git a/hw/audio/sb16.c b/hw/audio/sb16.c
index b052de5..995435f 100644
--- a/hw/audio/sb16.c
+++ b/hw/audio/sb16.c
@@ -40,11 +40,6 @@
 #define ldebug(...)
 #endif
 
-#define IO_READ_PROTO(name) \
-uint32_t name (void *opaque, uint32_t nport)
-#define IO_WRITE_PROTO(name)\
-void name (void *opaque, uint32_t nport, uint32_t val)
-
 static const char e3[] = "COPYRIGHT (C) CREATIVE TECHNOLOGY LTD, 1992.";
 
 #define TYPE_SB16 "sb16"
@@ -881,7 +876,7 @@ static void reset (SB16State *s)
 legacy_reset (s);
 }
 
-static IO_WRITE_PROTO (dsp_write)
+static void dsp_write(void *opaque, uint32_t nport, uint32_t val)
 {
 SB16State *s = opaque;
 int iport;
@@ -959,7 +954,7 @@ 

Re: [Qemu-devel] [PATCH 2/3] ui/curses: Support line graphics chars on -curses mode

[Gerd Hoffmann]

  Hi,

> OK. For cleaner (purpose is separation of curses code), I introduced
> hw_text_update_cb() interface (looks like possible to use text data for
> other than curses interface too). But you may feel this is overkill.

Waded through the code a bit.  The whole text interface isn't cleanly
abstracted.  There are curses-specific assumptions all over the place
and thats why there is a curses include in console.h (ideally there
shouldn't be any outside ui/curses.c).

So I've changed my mind and I think adding this interface doesn't help
much.  It pretends we have a clean text mode interface, which we clearly
don't have.  Tex mode support needs some major work anyway should we add
a second text mode interface at some point in the future.  Using
vga_to_curses in console.c makes at least pretty clear what is going on
here.

Lets go with v2 of this patch.

There are some codestyle issues in the patch (and the others too),
please fix them them (scripts/checkpatch.pl helps), then resend the
whole series.

thanks,
  Gerd

Re: [Qemu-devel] Question about xen disk unplug support for ahci missed in qemu

[Stefano Stabellini]

On Fri, 16 Oct 2015, John Snow wrote:
> On 10/13/2015 01:10 PM, Stefano Stabellini wrote:
> > On Tue, 13 Oct 2015, John Snow wrote:
> >> On 10/13/2015 11:55 AM, Fabio Fantoni wrote:
> >>> I added ahci disk support in libxl and using it for week seems that was
> >>> ok, after a reply of Stefano Stabellini seems that xen disk unplug
> >>> support only ide disks:
> >>> http://git.qemu.org/?p=qemu.git;a=commitdiff;h=679f4f8b178e7c66fbc2f39c905374ee8663d5d8
> >>>
> >>> Today Paul Durrant told me that even if pv disk is ok also with ahci and
> >>> the emulated one is offline can be a risk:
> >>> http://lists.xenproject.org/archives/html/win-pv-devel/2015-10/msg00021.html
> >>>
> >>>
> >>> I tried to take a fast look in qemu code but I not understand the needed
> >>> thing for add the xen disk unplug support also for ahci, can someone do
> >>> it or tell me useful information for do it please?
> >>>
> >>> Thanks for any reply and sorry for my bad english.
> >>>
> >>
> >> I'm not entirely sure what features you need AHCI to support in order
> >> for Xen to be happy.
> >>
> >> I'd guess hotplugging, but where I get confused is that IDE disks don't
> >> support hotplugging either, so I guess I'm not sure sure what you need.
> >>
> >> Stefano, can you help bridge my Xen knowledge gap?
> >  
> > Hi John,
> > 
> > we need something like hw/i386/xen/xen_platform.c:unplug_disks but that
> > can unplug AHCI disk. And by unplug, I mean "make disappear" like
> > pci_piix3_xen_ide_unplug does for ide.
> > 
> 
> I'm trying to follow this discussion as best as I am able, but my lack
> of experience with Xen prevents me from really participating in a
> meaningful way.
> 
> (I see that Laszlo is still discussing some CD-ROM issues with Fabio
> which may be of interest to me...)
> 
> At any rate, I won't be authoring any Xen-specific hacks to the AHCI
> device, but I do have plans to implement hot-plugging emulation as per
> the AHCI spec. Perhaps this is sufficient for the Xen layer, but someone
> else will need to author the appropriate glue code.
> 
> If "real" hot-plugging is not sufficient, we'll need to discuss further,
> preferably over some RFC patches.

That's fine. AHCI hot-plugging would go a long way and once we have
that, the rest is easy.

Thanks,

Stefano

Re: [Qemu-devel] [PATCH] qemu-options: add documentation for using UDP unicast network backend.

[Michael S. Tsirkin]

On Mon, Oct 19, 2015 at 12:49:15PM +0300, Victor Kaplansky wrote:
> For a long time QEMU has had support for UDP unicast network
> backend, but manual was missing description and usage examples.
> 
> This patch adds some more documentation and an example.
> 
> Signed-off-by: Victor Kaplansky 


Good point, thanks!
Minor comments:

> ---
>  qemu-options.hx | 23 +++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 2485b94..342bb14 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1855,6 +1855,8 @@ qemu-system-i386 linux.img \
>   -net socket,connect=127.0.0.1:1234
>  @end example
>  
> +
> +
>  @item -netdev 
> socket,id=@var{id}[,fd=@var{h}][,mcast=@var{maddr}:@var{port}[,localaddr=@var{addr}]]
>  @itemx -net 
> socket[,vlan=@var{n}][,name=@var{name}][,fd=@var{h}][,mcast=@var{maddr}:@var{port}[,localaddr=@var{addr}]]
>  

Why add this here? Probably safe to drop.

> @@ -1907,6 +1909,27 @@ qemu-system-i386 linux.img \
>   -net socket,mcast=239.192.168.1:1102,localaddr=1.2.3.4
>  @end example
>  
> +@item -netdev 
> socket,id=@var{id}[,fd=@var{h}][,udp=@var{rhost}:@var{rport}[,localaddr=@var{lhost}:@var{lport}]]
> +@itemx -net 
> socket[,vlan=@var{n}][,name=@var{name}][,fd=@var{h}][,udp=@var{host}:@var{rport}[,localaddr=@var{host}:@var{lport}]]

-net variant says "host" -netdev - lhost/rhost.

> +
> +Connect a VLAN @var{n} to a remote VLAN in another QEMU virtual
> +machine using a UDP tunnel. Use lport as local port and rport as
> +remote port of the tunnel.

the local port/the remote port

Pls document lhost/rhost as well.
> The two QEMU 

QEMU instances


>can be running on
> +different hosts.

There are no "the two QEMU" so far.
Should this come after the examples?

> Use @option{fd=h} to specify an already opened
> +UDP socket.
> +
> +Example:
> +@example
> +# launch one QEMU instance
> +qemu-system-i386 linux.img \
> + -device virtio-net-pci,netdev=net0 \
> + -netdev 
> socket,id=net0,udp=127.0.0.1:,localaddr=127.0.0.1:
> +# launch second QEMU instance sharing the network with the first one

"a second QEMU instance"

> +qemu-system-i386 linux.img \
> + -device virtio-net-pci,netdev=net0 \
> + -netdev 
> socket,id=net0,udp=127.0.0.1:,localaddr=127.0.0.1:

localhost might be a bit clearer, this way people know names are OK to use, too.
It does work, doesn't it?

> +@end example
> +
>  @item -netdev 
> l2tpv3,id=@var{id},src=@var{srcaddr},dst=@var{dstaddr}[,srcport=@var{srcport}][,dstport=@var{dstport}],txsession=@var{txsession}[,rxsession=@var{rxsession}][,ipv6][,udp][,cookie64][,counter][,pincounter][,txcookie=@var{txcookie}][,rxcookie=@var{rxcookie}][,offset=@var{offset}]
>  @itemx -net 
> l2tpv3[,vlan=@var{n}][,name=@var{name}],src=@var{srcaddr},dst=@var{dstaddr}[,srcport=@var{srcport}][,dstport=@var{dstport}],txsession=@var{txsession}[,rxsession=@var{rxsession}][,ipv6][,udp][,cookie64][,counter][,pincounter][,txcookie=@var{txcookie}][,rxcookie=@var{rxcookie}][,offset=@var{offset}]
>  Connect VLAN @var{n} to L2TPv3 pseudowire. L2TPv3 (RFC3391) is a popular
> -- 
> --Victor

Re: [Qemu-devel] [PATCH v3 22/32] nvdimm: init the address region used by NVDIMM ACPI

[Igor Mammedov]

On Mon, 19 Oct 2015 18:01:17 +0800
Xiao Guangrong  wrote:

> 
> 
> On 10/19/2015 05:46 PM, Igor Mammedov wrote:
> > On Mon, 19 Oct 2015 12:17:22 +0300
> > "Michael S. Tsirkin"  wrote:
> >
> >> On Mon, Oct 19, 2015 at 03:44:13PM +0800, Xiao Guangrong wrote:
> >>>
> >>>
> >>> On 10/19/2015 03:39 PM, Michael S. Tsirkin wrote:
>  On Mon, Oct 19, 2015 at 03:27:21PM +0800, Xiao Guangrong wrote:
> >>> +nvdimm_init_memory_state(>nvdimm_memory,
> >>> system_memory, machine,
> >>> + TARGET_PAGE_SIZE);
> >>> +
> >>
> >> Shouldn't this be conditional on presence of the nvdimm device?
> >>
> >
> > We will enable hotplug on nvdimm devices in the near future once
> > Linux driver is ready. I'd keep it here for future development.
> 
>  No, I don't think we should add stuff unconditionally. If not
>  nvdimm, some other flag should indicate user intends to hotplug
>  things.
> 
> >>>
> >>> Actually, it is not unconditionally which is called if parameter
> >>> "-m aaa, maxmem=bbb" (aaa < bbb) is used. It is on the some path
> >>> of memoy-hotplug initiation.
> >>>
> >>
> >> Right, but that's not the same as nvdimm.
> >>
> >
> > it could be pc-machine property, then it could be turned on like
> > this: -machine nvdimm_support=on
> 
> Er, I do not understand why this separate switch is needed and why
> nvdimm and pc-dimm is different. :(
> 
> NVDIMM reuses memory-hotplug's framework, such as maxmem, slot, and
> dimm device, even some of ACPI logic to do hotplug things, etc. Both
> nvdimm and pc-dimm are built on the same infrastructure.
NVDIMM support consumes precious low RAM  and MMIO resources and
not small amount at that. So turning it on unconditionally with
memory hotplug even if NVDIMM wouldn't ever be used isn't nice.

However that concern could be dropped if instead of allocating it's
own control MMIO/RAM regions, NVDIMM would reuse memory hotplug's MMIO
region and replace RAM region with serializing/marshaling label data
over the same MMIO interface (yes, it's slower but it's not
performance critical path).  

> 
> 
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Qemu-devel] [PULL v2 00/49] Misc patches for 2015-10-16

[Paolo Bonzini]

The following changes since commit 5451316ed07b758a187dedf21047bed8f843f7f1:

  Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into 
staging (2015-10-12 15:52:54 +0100)

are available in the git repository at:

  git://github.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to 1c4a55dbed9a47fde9294f7de6c8bb060d874c88:

  kvm: Allow the Hyper-V vendor ID to be specified (2015-10-19 10:13:07 +0200)


* KVM page size fix for PPC
* Support for Linux 4.4's new Hyper-V features
* Eliminate g_slice from areas I maintain
* checkpatch fix
* Peter's cpu_reload_memory_map() cleanups
* More changes to MAINTAINERS
* Require Python 2.6
* chardev creation fixes
* PCI requester id for ARM KVM
* cleanups and doc fixes
* Allow customization of the Hyper-V vendor id


Alex Williamson (1):
  kvm: Allow the Hyper-V vendor ID to be specified

Alexey Kardashevskiy (1):
  kvm-all: Align to qemu_real_host_page_size in kvm_set_phys_mem

Andrey Smetanin (3):
  target-i386/kvm: Hyper-V HV_X64_MSR_RESET support
  target-i386/kvm: set Hyper-V features cpuid bit 
HV_X64_MSR_VP_INDEX_AVAILABLE
  target-i386/kvm: Hyper-V HV_X64_MSR_VP_RUNTIME support

Andy Whitcroft (1):
  checkpatch: port fix from kernel "## is not a valid modifier"

Daniel P. Berrange (1):
  README: fill out some useful quickstart information

Markus Armbruster (1):
  configure: Require Python 2.6

Paolo Bonzini (33):
  nbd: switch from g_slice allocator to malloc
  scsi: switch from g_slice allocator to malloc
  megasas: fix megasas_get_sata_addr
  checkpatch: allow open braces on typedef lines
  linux-headers: update from kvm/next
  exec: remove non-TCG stuff from exec-all.h header.
  MAINTAINERS: add two devices to the e500 section
  MAINTAINERS: Add more Xen files
  MAINTAINERS: Add more pxa2xx files and boards
  MAINTAINERS: Add maintainer for ARM PrimeCell and integrated devices
  MAINTAINERS: Add more devices to realview board
  qemu-sockets: fix conversion of ipv4/ipv6 JSON to QemuOpts
  qemu-char: cleanup qmp_chardev_add
  qemu-char: cleanup HAVE_CHARDEV_*
  qemu-char: add create to register_char_driver
  qemu-char: convert file backend to data-driven creation
  qemu-char: convert serial backend to data-driven creation
  qemu-char: convert parallel backend to data-driven creation
  qemu-char: convert pipe backend to data-driven creation
  qemu-char: convert socket backend to data-driven creation
  qemu-char: convert UDP backend to data-driven creation
  qemu-char: convert pty backend to data-driven creation
  qemu-char: convert null backend to data-driven creation
  qemu-char: convert mux backend to data-driven creation
  qemu-char: convert msmouse backend to data-driven creation
  qemu-char: convert braille backend to data-driven creation
  qemu-char: convert testdev backend to data-driven creation
  qemu-char: convert stdio backend to data-driven creation
  qemu-char: convert console backend to data-driven creation
  qemu-char: convert spice backend to data-driven creation
  qemu-char: convert vc backend to data-driven creation
  qemu-char: convert ringbuf backend to data-driven creation
  qemu-char: cleanup after completed conversion to cd->create

Pavel Fedin (3):
  kvm: Make KVM_CAP_SIGNAL_MSI globally available
  hw/pci: Introduce pci_requester_id()
  kvm: Pass PCI device pointer to MSI routing functions

Peter Maydell (3):
  exec.c: Don't call cpu_reload_memory_map() from cpu_exec_init()
  cpu-exec-common.c: Clarify comment about cpu_reload_memory_map()'s RCU 
operations
  exec.c: Collect AddressSpace related fields into a CPUAddressSpace struct

Sergey Fedorov (1):
  doc/rcu: fix g_free_rcu() usage example

Thomas Huth (1):
  kvm: Move x86-specific functions into target-i386/kvm.c

 MAINTAINERS   |  51 +++-
 README| 108 +++-
 backends/baum.c   |  17 +-
 backends/msmouse.c|   8 +-
 backends/testdev.c|   8 +-
 configure |  12 +-
 cpu-exec-common.c |  33 +--
 docs/rcu.txt  |   2 +-
 exec.c|  57 +++--
 hw/i386/kvm/pci-assign.c  |  11 +-
 hw/pci/msi.c  |   2 +-
 hw/pci/pcie_aer.c |   2 +-
 hw/scsi/megasas.c |   2 +-
 hw/scsi/scsi-bus.c|   4 +-
 hw/scsi/virtio-scsi-dataplane.c   |  10 +-
 hw/scsi/virtio-scsi.c |  12 +-
 hw/vfio/pci.c |  11 +-
 hw/virtio/virtio-pci.c 

[Qemu-devel] [PULL 37/49] qemu-char: convert testdev backend to data-driven creation

[Paolo Bonzini]

Reviewed-by: Eric Blake 
Signed-off-by: Paolo Bonzini 
---
 backends/testdev.c| 7 +--
 include/sysemu/char.h | 3 ---
 qemu-char.c   | 2 +-
 stubs/Makefile.objs   | 1 -
 stubs/chr-testdev.c   | 7 ---
 5 files changed, 6 insertions(+), 14 deletions(-)
 delete mode 100644 stubs/chr-testdev.c

diff --git a/backends/testdev.c b/backends/testdev.c
index 43787f6..26d5c73 100644
--- a/backends/testdev.c
+++ b/backends/testdev.c
@@ -108,7 +108,10 @@ static void testdev_close(struct CharDriverState *chr)
 g_free(testdev);
 }
 
-CharDriverState *chr_testdev_init(void)
+static CharDriverState *chr_testdev_init(const char *id,
+ ChardevBackend *backend,
+ ChardevReturn *ret,
+ Error **errp)
 {
 TestdevCharState *testdev;
 CharDriverState *chr;
@@ -126,7 +129,7 @@ CharDriverState *chr_testdev_init(void)
 static void register_types(void)
 {
 register_char_driver("testdev", CHARDEV_BACKEND_KIND_TESTDEV, NULL,
- NULL);
+ chr_testdev_init);
 }
 
 type_init(register_types);
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 77415ec..5c28c16 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -356,9 +356,6 @@ extern int term_escape_char;
 
 CharDriverState *qemu_char_get_next_serial(void);
 
-/* testdev.c */
-CharDriverState *chr_testdev_init(void);
-
 /* console.c */
 typedef CharDriverState *(VcHandler)(ChardevVC *vc);
 
diff --git a/qemu-char.c b/qemu-char.c
index f2e3a35..56bc7ed 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4318,7 +4318,7 @@ ChardevReturn *qmp_chardev_add(const char *id, 
ChardevBackend *backend,
 abort();
 break;
 case CHARDEV_BACKEND_KIND_TESTDEV:
-chr = chr_testdev_init();
+abort();
 break;
 case CHARDEV_BACKEND_KIND_STDIO:
 chr = qemu_chr_open_stdio(backend->stdio);
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 8cfa5a2..b5322a2 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -1,6 +1,5 @@
 stub-obj-y += arch-query-cpu-def.o
 stub-obj-y += bdrv-commit-all.o
-stub-obj-y += chr-testdev.o
 stub-obj-y += clock-warp.o
 stub-obj-y += cpu-get-clock.o
 stub-obj-y += cpu-get-icount.o
diff --git a/stubs/chr-testdev.c b/stubs/chr-testdev.c
deleted file mode 100644
index 23112a2..000
--- a/stubs/chr-testdev.c
+++ /dev/null
@@ -1,7 +0,0 @@
-#include "qemu-common.h"
-#include "sysemu/char.h"
-
-CharDriverState *chr_testdev_init(void)
-{
-return 0;
-}
-- 
2.5.0

[Qemu-devel] [PULL 49/49] kvm: Allow the Hyper-V vendor ID to be specified

[Paolo Bonzini]

From: Alex Williamson 

According to Microsoft documentation, the signature in the standard
hypervisor CPUID leaf at 0x4000 identifies the Vendor ID and is
for reporting and diagnostic purposes only.  We can therefore allow
the user to change it to whatever they want, within the 12 character
limit.  Add a new hv-vendor-id option to the -cpu flag to allow
for this, ex:

 -cpu host,hv_time,hv-vendor-id=KeenlyKVM

Link: http://msdn.microsoft.com/library/windows/hardware/hh975392
Signed-off-by: Alex Williamson 
Message-Id: <20151016153356.28104.48612.st...@gimli.home>
[Adjust error message to match the property name, use error_report. - Paolo]
Signed-off-by: Paolo Bonzini 
---
 target-i386/cpu-qom.h |  1 +
 target-i386/cpu.c |  1 +
 target-i386/kvm.c | 14 +-
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 9eab41b..e3bfe9d 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -88,6 +88,7 @@ typedef struct X86CPU {
 bool hyperv_vapic;
 bool hyperv_relaxed_timing;
 int hyperv_spinlock_attempts;
+char *hyperv_vendor_id;
 bool hyperv_time;
 bool hyperv_crash;
 bool hyperv_reset;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index d2b0619..5f53af2 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -3149,6 +3149,7 @@ static Property x86_cpu_properties[] = {
 DEFINE_PROP_UINT32("level", X86CPU, env.cpuid_level, 0),
 DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0),
 DEFINE_PROP_UINT32("xlevel2", X86CPU, env.cpuid_xlevel2, 0),
+DEFINE_PROP_STRING("hv-vendor-id", X86CPU, hyperv_vendor_id),
 DEFINE_PROP_END_OF_LIST()
 };
 
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 010ac51..64046cb 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -28,6 +28,7 @@
 #include "exec/gdbstub.h"
 #include "qemu/host-utils.h"
 #include "qemu/config-file.h"
+#include "qemu/error-report.h"
 #include "hw/i386/pc.h"
 #include "hw/i386/apic.h"
 #include "hw/i386/apic_internal.h"
@@ -505,7 +506,18 @@ int kvm_arch_init_vcpu(CPUState *cs)
 if (hyperv_enabled(cpu)) {
 c = _data.entries[cpuid_i++];
 c->function = HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS;
-memcpy(signature, "Microsoft Hv", 12);
+if (!cpu->hyperv_vendor_id) {
+memcpy(signature, "Microsoft Hv", 12);
+} else {
+size_t len = strlen(cpu->hyperv_vendor_id);
+
+if (len > 12) {
+error_report("hv-vendor-id truncated to 12 characters");
+len = 12;
+}
+memset(signature, 0, 12);
+memcpy(signature, cpu->hyperv_vendor_id, len);
+}
 c->eax = HYPERV_CPUID_MIN;
 c->ebx = signature[0];
 c->ecx = signature[1];
-- 
2.5.0

[Qemu-devel] [PULL 3/7] fw_cfg DMA interface documentation

[Gerd Hoffmann]

From: Marc Marí 

Add fw_cfg DMA interface specification in the documentation.

Based on Gerd Hoffman's initial implementation.

Signed-off-by: Marc Marí 
Reviewed-by: Peter Maydell 
Reviewed-by: Laszlo Ersek 
Signed-off-by: Gerd Hoffmann 
---
 docs/specs/fw_cfg.txt | 65 +++
 1 file changed, 61 insertions(+), 4 deletions(-)

diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
index d5dee4b..536909a 100644
--- a/docs/specs/fw_cfg.txt
+++ b/docs/specs/fw_cfg.txt
@@ -76,6 +76,13 @@ increasing address order, similar to memcpy().
 
 Selector Register IOport: 0x510
 Data Register IOport: 0x511
+DMA Address IOport:   0x514
+
+=== ARM Register Locations ===
+
+Selector Register address: Base + 8 (2 bytes)
+Data Register address: Base + 0 (8 bytes)
+DMA Address address:   Base + 16 (8 bytes)
 
 == Firmware Configuration Items ==
 
@@ -86,11 +93,12 @@ by selecting the "signature" item using key 0x 
(FW_CFG_SIGNATURE),
 and reading four bytes from the data register. If the fw_cfg device is
 present, the four bytes read will contain the characters "QEMU".
 
-=== Revision (Key 0x0001, FW_CFG_ID) ===
+=== Revision / feature bitmap (Key 0x0001, FW_CFG_ID) ===
 
-A 32-bit little-endian unsigned int, this item is used as an interface
-revision number, and is currently set to 1 by QEMU when fw_cfg is
-initialized.
+A 32-bit little-endian unsigned int, this item is used to check for enabled
+features.
+ - Bit 0: traditional interface. Always set.
+ - Bit 1: DMA interface.
 
 === File Directory (Key 0x0019, FW_CFG_FILE_DIR) ===
 
@@ -132,6 +140,55 @@ Selector Reg.Range Usage
 In practice, the number of allowed firmware configuration items is given
 by the value of FW_CFG_MAX_ENTRY (see fw_cfg.h).
 
+= Guest-side DMA Interface =
+
+If bit 1 of the feature bitmap is set, the DMA interface is present. This does
+not replace the existing fw_cfg interface, it is an add-on. This interface
+can be used through the 64-bit wide address register.
+
+The address register is in big-endian format. The value for the register is 0
+at startup and after an operation. A write to the least significant half (at
+offset 4) triggers an operation. This means that operations with 32-bit
+addresses can be triggered with just one write, whereas operations with
+64-bit addresses can be triggered with one 64-bit write or two 32-bit writes,
+starting with the most significant half (at offset 0).
+
+In this register, the physical address of a FWCfgDmaAccess structure in RAM
+should be written. This is the format of the FWCfgDmaAccess structure:
+
+typedef struct FWCfgDmaAccess {
+uint32_t control;
+uint32_t length;
+uint64_t address;
+} FWCfgDmaAccess;
+
+The fields of the structure are in big endian mode, and the field at the lowest
+address is the "control" field.
+
+The "control" field has the following bits:
+ - Bit 0: Error
+ - Bit 1: Read
+ - Bit 2: Skip
+ - Bit 3: Select. The upper 16 bits are the selected index.
+
+When an operation is triggered, if the "control" field has bit 3 set, the
+upper 16 bits are interpreted as an index of a firmware configuration item.
+This has the same effect as writing the selector register.
+
+If the "control" field has bit 1 set, a read operation will be performed.
+"length" bytes for the current selector and offset will be copied into the
+physical RAM address specified by the "address" field.
+
+If the "control" field has bit 2 set (and not bit 1), a skip operation will be
+performed. The offset for the current selector will be advanced "length" bytes.
+
+To check the result, read the "control" field:
+   error bit set->  something went wrong.
+   all bits cleared ->  transfer finished successfully.
+   otherwise->  transfer still in progress (doesn't happen
+today due to implementation not being async,
+but may in the future).
+
 = Host-side API =
 
 The following functions are available to the QEMU programmer for adding
-- 
1.8.3.1

[Qemu-devel] [PATCH] qemu-options: add documentation for using UDP unicast network backend.

[Victor Kaplansky]

For a long time QEMU has had support for UDP unicast network
backend, but manual was missing description and usage examples.

This patch adds some more documentation and an example.

Signed-off-by: Victor Kaplansky 
---
 qemu-options.hx | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/qemu-options.hx b/qemu-options.hx
index 2485b94..342bb14 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1855,6 +1855,8 @@ qemu-system-i386 linux.img \
  -net socket,connect=127.0.0.1:1234
 @end example
 
+
+
 @item -netdev 
socket,id=@var{id}[,fd=@var{h}][,mcast=@var{maddr}:@var{port}[,localaddr=@var{addr}]]
 @itemx -net 
socket[,vlan=@var{n}][,name=@var{name}][,fd=@var{h}][,mcast=@var{maddr}:@var{port}[,localaddr=@var{addr}]]
 
@@ -1907,6 +1909,27 @@ qemu-system-i386 linux.img \
  -net socket,mcast=239.192.168.1:1102,localaddr=1.2.3.4
 @end example
 
+@item -netdev 
socket,id=@var{id}[,fd=@var{h}][,udp=@var{rhost}:@var{rport}[,localaddr=@var{lhost}:@var{lport}]]
+@itemx -net 
socket[,vlan=@var{n}][,name=@var{name}][,fd=@var{h}][,udp=@var{host}:@var{rport}[,localaddr=@var{host}:@var{lport}]]
+
+Connect a VLAN @var{n} to a remote VLAN in another QEMU virtual
+machine using a UDP tunnel. Use lport as local port and rport as
+remote port of the tunnel. The two QEMU can be running on
+different hosts. Use @option{fd=h} to specify an already opened
+UDP socket.
+
+Example:
+@example
+# launch one QEMU instance
+qemu-system-i386 linux.img \
+ -device virtio-net-pci,netdev=net0 \
+ -netdev 
socket,id=net0,udp=127.0.0.1:,localaddr=127.0.0.1:
+# launch second QEMU instance sharing the network with the first one
+qemu-system-i386 linux.img \
+ -device virtio-net-pci,netdev=net0 \
+ -netdev 
socket,id=net0,udp=127.0.0.1:,localaddr=127.0.0.1:
+@end example
+
 @item -netdev 
l2tpv3,id=@var{id},src=@var{srcaddr},dst=@var{dstaddr}[,srcport=@var{srcport}][,dstport=@var{dstport}],txsession=@var{txsession}[,rxsession=@var{rxsession}][,ipv6][,udp][,cookie64][,counter][,pincounter][,txcookie=@var{txcookie}][,rxcookie=@var{rxcookie}][,offset=@var{offset}]
 @itemx -net 
l2tpv3[,vlan=@var{n}][,name=@var{name}],src=@var{srcaddr},dst=@var{dstaddr}[,srcport=@var{srcport}][,dstport=@var{dstport}],txsession=@var{txsession}[,rxsession=@var{rxsession}][,ipv6][,udp][,cookie64][,counter][,pincounter][,txcookie=@var{txcookie}][,rxcookie=@var{rxcookie}][,offset=@var{offset}]
 Connect VLAN @var{n} to L2TPv3 pseudowire. L2TPv3 (RFC3391) is a popular
-- 
--Victor

Re: [Qemu-devel] [PATCH v5 2/4] pcie: Add support for Single Root I/O Virtualization (SR/IOV)

[Paolo Bonzini]

On 18/10/2015 13:02, Marcel Apfelbaum wrote:
> 
> +pci_device_deassert_intx(dev);
> +assert(dev->irq_state == 0);
> +
> +/* Clear all writable bits */
> +pci_word_test_and_clear_mask(dev->config + PCI_COMMAND,
> + pci_get_word(dev->wmask + PCI_COMMAND) |
> + pci_get_word(dev->w1cmask +
> PCI_COMMAND));
> +pci_word_test_and_clear_mask(dev->config + PCI_STATUS,
> + pci_get_word(dev->wmask + PCI_STATUS) |
> + pci_get_word(dev->w1cmask + PCI_STATUS));
> +dev->config[PCI_CACHE_LINE_SIZE] = 0x0;
> +dev->config[PCI_INTERRUPT_LINE] = 0x0;
> +pci_reset_regions(dev);
> +pci_update_mappings(dev);
> +
> +msi_reset(dev);
> +msix_reset(dev);

All this should stay in pci_do_device_reset.  Of course it's okay to
split the PF-specific parts to pci_reset_regions.

Paolo

[Qemu-devel] [PULL 4/7] Implement fw_cfg DMA interface

[Gerd Hoffmann]

From: Marc Marí 

Based on the specifications on docs/specs/fw_cfg.txt

This interface is an addon. The old interface can still be used as usual.

Based on Gerd Hoffman's initial implementation.

Signed-off-by: Marc Marí 
Reviewed-by: Laszlo Ersek 
Signed-off-by: Gerd Hoffmann 
---
 hw/arm/virt.c |   2 +-
 hw/nvram/fw_cfg.c | 240 +++---
 include/hw/nvram/fw_cfg.h |  16 +++-
 3 files changed, 244 insertions(+), 14 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 4e7160c..b670bc6 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -683,7 +683,7 @@ static void create_fw_cfg(const VirtBoardInfo *vbi)
 hwaddr size = vbi->memmap[VIRT_FW_CFG].size;
 char *nodename;
 
-fw_cfg_init_mem_wide(base + 8, base, 8);
+fw_cfg_init_mem_wide(base + 8, base, 8, 0, NULL);
 
 nodename = g_strdup_printf("/fw-cfg@%" PRIx64, base);
 qemu_fdt_add_subnode(vbi->fdt, nodename);
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 658f8c4..91829d4 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -23,6 +23,7 @@
  */
 #include "hw/hw.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/dma.h"
 #include "hw/isa/isa.h"
 #include "hw/nvram/fw_cfg.h"
 #include "hw/sysbus.h"
@@ -30,7 +31,7 @@
 #include "qemu/error-report.h"
 #include "qemu/config-file.h"
 
-#define FW_CFG_SIZE 2
+#define FW_CFG_CTL_SIZE 2
 #define FW_CFG_NAME "fw_cfg"
 #define FW_CFG_PATH "/machine/" FW_CFG_NAME
 
@@ -42,6 +43,16 @@
 #define FW_CFG_IO(obj)  OBJECT_CHECK(FWCfgIoState,  (obj), TYPE_FW_CFG_IO)
 #define FW_CFG_MEM(obj) OBJECT_CHECK(FWCfgMemState, (obj), TYPE_FW_CFG_MEM)
 
+/* FW_CFG_VERSION bits */
+#define FW_CFG_VERSION  0x01
+#define FW_CFG_VERSION_DMA  0x02
+
+/* FW_CFG_DMA_CONTROL bits */
+#define FW_CFG_DMA_CTL_ERROR   0x01
+#define FW_CFG_DMA_CTL_READ0x02
+#define FW_CFG_DMA_CTL_SKIP0x04
+#define FW_CFG_DMA_CTL_SELECT  0x08
+
 typedef struct FWCfgEntry {
 uint32_t len;
 uint8_t *data;
@@ -59,6 +70,11 @@ struct FWCfgState {
 uint16_t cur_entry;
 uint32_t cur_offset;
 Notifier machine_ready;
+
+bool dma_enabled;
+dma_addr_t dma_addr;
+AddressSpace *dma_as;
+MemoryRegion dma_iomem;
 };
 
 struct FWCfgIoState {
@@ -67,7 +83,7 @@ struct FWCfgIoState {
 /*< public >*/
 
 MemoryRegion comb_iomem;
-uint32_t iobase;
+uint32_t iobase, dma_iobase;
 };
 
 struct FWCfgMemState {
@@ -292,6 +308,122 @@ static void fw_cfg_data_mem_write(void *opaque, hwaddr 
addr,
 } while (i);
 }
 
+static void fw_cfg_dma_transfer(FWCfgState *s)
+{
+dma_addr_t len;
+FWCfgDmaAccess dma;
+int arch;
+FWCfgEntry *e;
+int read;
+dma_addr_t dma_addr;
+
+/* Reset the address before the next access */
+dma_addr = s->dma_addr;
+s->dma_addr = 0;
+
+if (dma_memory_read(s->dma_as, dma_addr, , sizeof(dma))) {
+stl_be_dma(s->dma_as, dma_addr + offsetof(FWCfgDmaAccess, control),
+   FW_CFG_DMA_CTL_ERROR);
+return;
+}
+
+dma.address = be64_to_cpu(dma.address);
+dma.length = be32_to_cpu(dma.length);
+dma.control = be32_to_cpu(dma.control);
+
+if (dma.control & FW_CFG_DMA_CTL_SELECT) {
+fw_cfg_select(s, dma.control >> 16);
+}
+
+arch = !!(s->cur_entry & FW_CFG_ARCH_LOCAL);
+e = >entries[arch][s->cur_entry & FW_CFG_ENTRY_MASK];
+
+if (dma.control & FW_CFG_DMA_CTL_READ) {
+read = 1;
+} else if (dma.control & FW_CFG_DMA_CTL_SKIP) {
+read = 0;
+} else {
+dma.length = 0;
+}
+
+dma.control = 0;
+
+while (dma.length > 0 && !(dma.control & FW_CFG_DMA_CTL_ERROR)) {
+if (s->cur_entry == FW_CFG_INVALID || !e->data ||
+s->cur_offset >= e->len) {
+len = dma.length;
+
+/* If the access is not a read access, it will be a skip access,
+ * tested before.
+ */
+if (read) {
+if (dma_memory_set(s->dma_as, dma.address, 0, len)) {
+dma.control |= FW_CFG_DMA_CTL_ERROR;
+}
+}
+
+} else {
+if (dma.length <= (e->len - s->cur_offset)) {
+len = dma.length;
+} else {
+len = (e->len - s->cur_offset);
+}
+
+if (e->read_callback) {
+e->read_callback(e->callback_opaque, s->cur_offset);
+}
+
+/* If the access is not a read access, it will be a skip access,
+ * tested before.
+ */
+if (read) {
+if (dma_memory_write(s->dma_as, dma.address,
+>data[s->cur_offset], len)) {
+dma.control |= FW_CFG_DMA_CTL_ERROR;
+}
+}
+
+s->cur_offset += len;
+}
+
+dma.address += len;
+dma.length  -= len;

[Qemu-devel] [PULL 1/7] fw_cfg: insert string blobs via qemu cmdline

[Gerd Hoffmann]

From: "Gabriel L. Somlo" 

Allow users to provide custom fw_cfg blobs with ascii string
payloads specified directly on the qemu command line.

Suggested-by: Jordan Justen 
Suggested-by: Laszlo Ersek 
Signed-off-by: Gabriel Somlo 
Message-id: 1443544141-26568-1-git-send-email-so...@cmu.edu
Reviewd-by: Laszlo Ersek 
Signed-off-by: Gerd Hoffmann 
---
 docs/specs/fw_cfg.txt | 15 +++
 qemu-options.hx   |  7 ++-
 vl.c  | 33 +++--
 3 files changed, 48 insertions(+), 7 deletions(-)

diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
index 74351dd..c0e76aa 100644
--- a/docs/specs/fw_cfg.txt
+++ b/docs/specs/fw_cfg.txt
@@ -216,6 +216,21 @@ the following syntax:
 where  is the fw_cfg item name, and  is the location
 on the host file system of a file containing the data to be inserted.
 
+Small enough items may be provided directly as strings on the command
+line, using the syntax:
+
+-fw_cfg [name=],string=
+
+The terminating NUL character of the content  will NOT be
+included as part of the fw_cfg item data, which is consistent with
+the absence of a NUL terminator for items inserted via the file option.
+
+Both  and, if applicable, the content  are passed
+through by QEMU without any interpretation, expansion, or further
+processing. Any such processing (potentially performed e.g., by the shell)
+is outside of QEMU's responsibility; as such, using plain ASCII characters
+is recommended.
+
 NOTE: Users *SHOULD* choose item names beginning with the prefix "opt/"
 when using the "-fw_cfg" command line option, to avoid conflicting with
 item names used internally by QEMU. For instance:
diff --git a/qemu-options.hx b/qemu-options.hx
index 2485b94..edee5f4 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2724,13 +2724,18 @@ ETEXI
 
 DEF("fw_cfg", HAS_ARG, QEMU_OPTION_fwcfg,
 "-fw_cfg [name=],file=\n"
-"add named fw_cfg entry from file\n",
+"add named fw_cfg entry from file\n"
+"-fw_cfg [name=],string=\n"
+"add named fw_cfg entry from string\n",
 QEMU_ARCH_ALL)
 STEXI
 @item -fw_cfg [name=]@var{name},file=@var{file}
 @findex -fw_cfg
 Add named fw_cfg entry from file. @var{name} determines the name of
 the entry in the fw_cfg file directory exposed to the guest.
+
+@item -fw_cfg [name=]@var{name},string=@var{str}
+Add named fw_cfg entry from string.
 ETEXI
 
 DEF("serial", HAS_ARG, QEMU_OPTION_serial, \
diff --git a/vl.c b/vl.c
index 7c806a2..332d828 100644
--- a/vl.c
+++ b/vl.c
@@ -512,6 +512,10 @@ static QemuOptsList qemu_fw_cfg_opts = {
 .type = QEMU_OPT_STRING,
 .help = "Sets the name of the file from which\n"
 "the fw_cfg blob will be loaded",
+}, {
+.name = "string",
+.type = QEMU_OPT_STRING,
+.help = "Sets content of the blob to be inserted from a string",
 },
 { /* end of list */ }
 },
@@ -2239,11 +2243,16 @@ char *qemu_find_file(int type, const char *name)
 return NULL;
 }
 
+static inline bool nonempty_str(const char *str)
+{
+return str && *str;
+}
+
 static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp)
 {
 gchar *buf;
 size_t size;
-const char *name, *file;
+const char *name, *file, *str;
 
 if (opaque == NULL) {
 error_report("fw_cfg device not available");
@@ -2251,8 +2260,15 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, 
Error **errp)
 }
 name = qemu_opt_get(opts, "name");
 file = qemu_opt_get(opts, "file");
-if (name == NULL || *name == '\0' || file == NULL || *file == '\0') {
-error_report("invalid argument value");
+str = qemu_opt_get(opts, "string");
+
+/* we need name and either a file or the content string */
+if (!(nonempty_str(name) && (nonempty_str(file) || nonempty_str(str {
+error_report("invalid argument(s)");
+return -1;
+}
+if (nonempty_str(file) && nonempty_str(str)) {
+error_report("file and string are mutually exclusive");
 return -1;
 }
 if (strlen(name) > FW_CFG_MAX_FILE_PATH - 1) {
@@ -2263,9 +2279,14 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, 
Error **errp)
 error_report("WARNING: externally provided fw_cfg item names "
  "should be prefixed with \"opt/\"!");
 }
-if (!g_file_get_contents(file, , , NULL)) {
-error_report("can't load %s", file);
-return -1;
+if (nonempty_str(str)) {
+size = strlen(str); /* NUL terminator NOT included in fw_cfg blob */
+buf = g_memdup(str, size);
+} else {
+if (!g_file_get_contents(file, , , NULL)) {
+error_report("can't load %s", file);
+return -1;
+}
 }
 fw_cfg_add_file((FWCfgState *)opaque, name, buf, 

[Qemu-devel] [PULL 5/7] Enable fw_cfg DMA interface for ARM

[Gerd Hoffmann]

From: Marc Marí 

Enable the fw_cfg DMA interface for the ARM virt machine.

Based on Gerd Hoffman's initial implementation.

Signed-off-by: Marc Marí 
Reviewed-by: Peter Maydell 
Reviewed-by: Laszlo Ersek 
Signed-off-by: Gerd Hoffmann 
---
 hw/arm/virt.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index b670bc6..5d38c47 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -119,7 +119,7 @@ static const MemMapEntry a15memmap[] = {
 [VIRT_GIC_REDIST] = { 0x080A, 0x00F6 },
 [VIRT_UART] =   { 0x0900, 0x1000 },
 [VIRT_RTC] ={ 0x0901, 0x1000 },
-[VIRT_FW_CFG] = { 0x0902, 0x000a },
+[VIRT_FW_CFG] = { 0x0902, 0x0018 },
 [VIRT_MMIO] =   { 0x0a00, 0x0200 },
 /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
 [VIRT_PLATFORM_BUS] =   { 0x0c00, 0x0200 },
@@ -677,13 +677,13 @@ static void create_flash(const VirtBoardInfo *vbi)
 g_free(nodename);
 }
 
-static void create_fw_cfg(const VirtBoardInfo *vbi)
+static void create_fw_cfg(const VirtBoardInfo *vbi, AddressSpace *as)
 {
 hwaddr base = vbi->memmap[VIRT_FW_CFG].base;
 hwaddr size = vbi->memmap[VIRT_FW_CFG].size;
 char *nodename;
 
-fw_cfg_init_mem_wide(base + 8, base, 8, 0, NULL);
+fw_cfg_init_mem_wide(base + 8, base, 8, base + 16, as);
 
 nodename = g_strdup_printf("/fw-cfg@%" PRIx64, base);
 qemu_fdt_add_subnode(vbi->fdt, nodename);
@@ -1031,7 +1031,7 @@ static void machvirt_init(MachineState *machine)
  */
 create_virtio_devices(vbi, pic);
 
-create_fw_cfg(vbi);
+create_fw_cfg(vbi, _space_memory);
 rom_set_fw(fw_cfg_find());
 
 guest_info->smp_cpus = smp_cpus;
-- 
1.8.3.1

[Qemu-devel] [PULL 2/7] fw_cfg: document fw_cfg_modify_iXX() update functions

[Gerd Hoffmann]

From: "Gabriel L. Somlo" 

Document the behavior of fw_cfg_modify_iXX() for leak-less updating
of integer-type blobs.

Currently only fw_cfg_modify_i16() is coded, but 32- and 64-bit versions
may be added later if necessary..

Signed-off-by: Gabriel Somlo 
Reviewed-by: Laszlo Ersek 
Signed-off-by: Gerd Hoffmann 
---
 docs/specs/fw_cfg.txt | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
index c0e76aa..d5dee4b 100644
--- a/docs/specs/fw_cfg.txt
+++ b/docs/specs/fw_cfg.txt
@@ -159,6 +159,17 @@ will convert a 16-, 32-, or 64-bit integer to 
little-endian, then add
 a dynamically allocated copy of the appropriately sized item to fw_cfg
 under the given selector key value.
 
+== fw_cfg_modify_iXX() ==
+
+Modify the value of an XX-bit item (where XX may be 16, 32, or 64).
+Similarly to the corresponding fw_cfg_add_iXX() function set, convert
+a 16-, 32-, or 64-bit integer to little endian, create a dynamically
+allocated copy of the required size, and replace the existing item at
+the given selector key value with the newly allocated one. The previous
+item, assumed to have been allocated during an earlier call to
+fw_cfg_add_iXX() or fw_cfg_modify_iXX() (of the same width XX), is freed
+before the function returns.
+
 == fw_cfg_add_file() ==
 
 Given a filename (i.e., fw_cfg item name), starting pointer, and size,
-- 
1.8.3.1

[Qemu-devel] [PATCH 0/1] vhost-user: support of live migration with multiqueue

[Thibaut Collet]

Patches serie for vhost-user live migration from Marc-Andre Lureau
[PATCH v8 00/27] vhost-user: add migration support
(http://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg02452.html) does not
work if multiqueue is set.
This patch correct an issue of queue index when a migration is started with
multiqueue

Thibaut Collet (1):
  vhost: set the correct queue index in case of migration with
multiqueue

 hw/virtio/vhost.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH] bt: fix use of uninitialized variable seqlen

[Paolo Bonzini]

sdp_svc_match, sdp_attr_match and sdp_svc_attr_match read the last
argument.  The only sensible way to change the code is to make that last
argument "len" instead of "seqlen" which is the length of a subsequence
in the previous "if" branch.

To make the structure of the code clearer, use "else" instead of
"else if".

Reported by Coverity.

Signed-off-by: Paolo Bonzini 
---
 hw/bt/sdp.c | 29 -
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/hw/bt/sdp.c b/hw/bt/sdp.c
index c903747..b9bcdcc 100644
--- a/hw/bt/sdp.c
+++ b/hw/bt/sdp.c
@@ -150,12 +150,14 @@ static ssize_t sdp_svc_search(struct bt_l2cap_sdp_state_s 
*sdp,
 if (seqlen < 3 || len < seqlen)
 return -SDP_INVALID_SYNTAX;
 len -= seqlen;
-
 while (seqlen)
 if (sdp_svc_match(sdp, , ))
 return -SDP_INVALID_SYNTAX;
-} else if (sdp_svc_match(sdp, , ))
-return -SDP_INVALID_SYNTAX;
+} else {
+if (sdp_svc_match(sdp, , )) {
+return -SDP_INVALID_SYNTAX;
+}
+}
 
 if (len < 3)
 return -SDP_INVALID_SYNTAX;
@@ -278,8 +280,11 @@ static ssize_t sdp_attr_get(struct bt_l2cap_sdp_state_s 
*sdp,
 while (seqlen)
 if (sdp_attr_match(record, , ))
 return -SDP_INVALID_SYNTAX;
-} else if (sdp_attr_match(record, , ))
-return -SDP_INVALID_SYNTAX;
+} else {
+if (sdp_attr_match(record, , )) {
+return -SDP_INVALID_SYNTAX;
+}
+}
 
 if (len < 1)
 return -SDP_INVALID_SYNTAX;
@@ -393,8 +398,11 @@ static ssize_t sdp_svc_search_attr_get(struct 
bt_l2cap_sdp_state_s *sdp,
 while (seqlen)
 if (sdp_svc_match(sdp, , ))
 return -SDP_INVALID_SYNTAX;
-} else if (sdp_svc_match(sdp, , ))
-return -SDP_INVALID_SYNTAX;
+} else {
+if (sdp_svc_match(sdp, , )) {
+return -SDP_INVALID_SYNTAX;
+}
+}
 
 if (len < 3)
 return -SDP_INVALID_SYNTAX;
@@ -413,8 +421,11 @@ static ssize_t sdp_svc_search_attr_get(struct 
bt_l2cap_sdp_state_s *sdp,
 while (seqlen)
 if (sdp_svc_attr_match(sdp, , ))
 return -SDP_INVALID_SYNTAX;
-} else if (sdp_svc_attr_match(sdp, , ))
-return -SDP_INVALID_SYNTAX;
+} else {
+if (sdp_svc_attr_match(sdp, , )) {
+return -SDP_INVALID_SYNTAX;
+}
+}
 
 if (len < 1)
 return -SDP_INVALID_SYNTAX;
-- 
2.5.0

Re: [Qemu-devel] [PATCH 2/3] block: Add 'blockdev-del' QMP command

[Alberto Garcia]

On Sat 17 Oct 2015 08:06:19 PM CEST, Max Reitz wrote:
>> +if (blk_get_refcnt(blk) > 1 ||
>> +(bs && (bs->refcnt > 1 || !QLIST_EMPTY(>parents {
>> +error_setg(errp, "Block device %s is being used", device);
>> +goto out;
>> +}
>
> I can't think of a way off the top of my head that a BB with a refcount
> of one or a BDS with a refcount of one without any parents would not be
> monitor-owned, but I don't quite like assuming it just from the refcount
> and the parent list anyway.

I agree, I would also like to have a clearer way to do it.

I'll try to go over all possible ways to create a BDS and see other
cases that I might have missed. At the very list I would like to extend
the iotest and make it as comprehensive as possible.

> The two patches which are significant for this patch are:
> - "blockdev: Keep track of monitor-owned BDS" from the first series
> - "blockdev: Add list of monitor-owned BlockBackends" from the second
>
> Explaining what they do and why they are relevant to this patch doesn't
> really seem necessary, but anyway: They add one list for the
> monitor-owned BDSs and one for the monitor-owned BBs.

Yeah, those would be handy indeed.

> It's up to you. It probably works just how you implemented it, and
> considering the pace of the "BB and media" series (and other series of
> mine in the past year), getting those two series merged may be a
> matter of decades. So it's probably best to do it like this for now
> and I'll fix it up then...

I think I'll first try to go over all possible cases and see what I
find.

Thanks,

Berto

Re: [Qemu-devel] What's the intended use of log.h logging?

[Paolo Bonzini]

On 19/10/2015 15:17, Markus Armbruster wrote:
> This is at least as much an argument for use of tracing as against it.
> Tracing is a lot more flexible than log.h, and with the right backend,
> it's much more efficient, too.
> 
> If the appropriate trace patterns turn out to be too hard to remember,
> we can provide canned trace patterns with names that are easy to
> remember.
> 
> -d could become sugar for a suitable trace patterns.

I think that's putting the cart a bit before the horse, since "-d" is
currently not related to tracing at all. :)

Paolo

Re: [Qemu-devel] [PATCH QEMU] target-arm: Add support for SPSR_(ABT|UND|IRQ|FIQ)

[Peter Maydell]

On 15 October 2015 at 05:41, Soren Brinkmann  wrote:
> Signed-off-by: Soren Brinkmann 
> ---
> Hi,
>
> I recently came across some code that caused undefined instruction exceptions
> when executing instructions 'mrs x11, spsr_abt' and the like. I'm not 
> sure I
> get the full picture, but it seems QEMU already keeps the state for those SPSR
> registers and all that might be missing is exposing those registers to the
> guest.

Well, these are only accessible from EL2 or EL3, and we don't implement
those yet for 64-bit...

> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 83679970b432..0c64c0588115 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3281,6 +3281,22 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
>.type = ARM_CP_ALIAS,
>.opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 0,
>.access = PL2_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[6]) 
> },
> +{ .name = "SPSR_IRQ", .state = ARM_CP_STATE_AA64,
> +  .type = ARM_CP_ALIAS,
> +  .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 3, .opc2 = 0,
> +  .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[4]) 
> },
> +{ .name = "SPSR_ABT", .state = ARM_CP_STATE_AA64,
> +  .type = ARM_CP_ALIAS,
> +  .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 3, .opc2 = 1,
> +  .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[2]) 
> },
> +{ .name = "SPSR_UND", .state = ARM_CP_STATE_AA64,
> +  .type = ARM_CP_ALIAS,
> +  .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 3, .opc2 = 2,
> +  .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[3]) 
> },
> +{ .name = "SPSR_FIQ", .state = ARM_CP_STATE_AA64,
> +  .type = ARM_CP_ALIAS,
> +  .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 3, .opc2 = 3,
> +  .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[5]) 
> },
>  { .name = "VBAR_EL2", .state = ARM_CP_STATE_AA64,
>.opc0 = 3, .opc1 = 4, .crn = 12, .crm = 0, .opc2 = 0,
>.access = PL2_RW, .writefn = vbar_write,

These look OK, so
Reviewed-by: Peter Maydell 

I wonder if we should define some ARM_BANK_* constants for the
indexes into the banked_* arrays?

Providing the 32-bit versions of these would be nice, but they aren't
implemented as system registers and they're kind of complicated, so
we'll leave that for now.

thanks
-- PMM

Re: [Qemu-devel] [PULL 11/14] netfilter: print filter info associate with the netdev

[Paolo Bonzini]

On 12/10/2015 10:17, Jason Wang wrote:
> +}
> +QTAILQ_FOREACH(nf, >filters, next) {
> +monitor_printf(mon, "  - %s: type=%s%s\n",
> +   object_get_canonical_path_component(OBJECT(nf)),

The value returned from object_get_canonical_path_component must be freed.

Paolo

> +   object_get_typename(OBJECT(nf)),
> +   nf->info_str);
> +}

Re: [Qemu-devel] [PULL 08/14] net: merge qemu_deliver_packet and qemu_deliver_packet_iov

[Paolo Bonzini]

On 12/10/2015 10:17, Jason Wang wrote:
>  static ssize_t nc_sendv_compat(NetClientState *nc, const struct iovec *iov,
> -   int iovcnt)
> +   int iovcnt, unsigned flags)
>  {
> -uint8_t buffer[NET_BUFSIZE];
> +uint8_t buf[NET_BUFSIZE];
> +uint8_t *buffer;
>  size_t offset;
>  
> -offset = iov_to_buf(iov, iovcnt, 0, buffer, sizeof(buffer));
> +if (iovcnt == 1) {
> +buffer = iov[0].iov_base;
> +offset = iov[0].iov_len;
> +} else {
> +buffer = buf;
> +offset = iov_to_buf(iov, iovcnt, 0, buffer, sizeof(buffer));

You want "buf, sizeof(buf)" here.  sizeof(buffer) is the size of a
pointer, which is wrong.

Jason, can you please fix this?

Paolo

> +}

Re: [Qemu-devel] [PULL 0/7] fw_cfg: add dma interface, add strings via cmdline.

[Peter Maydell]

On 19 October 2015 at 14:29, Gerd Hoffmann  wrote:
>   Hi
>
>> > Windows fails to compile:
>> > /home/petmay01/linaro/qemu-for-merges/hw/nvram/fw_cfg.c: In function
>> > ‘fw_cfg_dma_mem_read’:
>> > /home/petmay01/linaro/qemu-for-merges/hw/nvram/fw_cfg.c:406: warning:
>> > integer constant is too large for ‘long’ type
>>
>> I don't have a Windows test environment, but I suspect the following:
>
> Builds fine here for some strange reason ...

It's really an "older version of gcc" kind of build warning, it's just
that my w32 setup happens to be the oldest gcc I have in the test set.

thanks
-- PMM

[Qemu-devel] [PATCH] target-alpha: fix uninitialized variable

[Paolo Bonzini]

I am not sure why the compiler does not catch it.  There is no
semantic change since gen_excp returns EXIT_NORETURN, but the
old code is wrong.

Reported by Coverity.

Signed-off-by: Paolo Bonzini 
---
 target-alpha/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index f936d1b..53f6833 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -2916,7 +2916,7 @@ void gen_intermediate_code(CPUAlphaState *env, struct 
TranslationBlock *tb)
 num_insns++;
 
 if (unlikely(cpu_breakpoint_test(cs, ctx.pc, BP_ANY))) {
-gen_excp(, EXCP_DEBUG, 0);
+ret = gen_excp(, EXCP_DEBUG, 0);
 break;
 }
 if (num_insns == max_insns && (tb->cflags & CF_LAST_IO)) {
-- 
2.5.0

Re: [Qemu-devel] [PATCH v2 00/16] Introduce I/O channels framework

[Paolo Bonzini]

On 19/10/2015 15:47, Daniel P. Berrange wrote:
> Ping, does anyone feel like doing a review of this series, if not I'll
> just do a pull request as-is...

You can certainly do a pull request of patches 1-7.  For 8-16, I haven't
reviewed it because I'm not sure how much it makes sense to commit
QIOChannel without a user.

Thanks,

Paolo

[Qemu-devel] [PATCH] net: Remove duplicate data from query-rx-filter on multiqueue net devices

[Vladislav Yasevich]

When responding to a query-rx-filter command on a multiqueue
netdev, qemu reports the data for each queue.  The data, however,
is not per-queue, but per device and the same data is reported
multiple times.  This causes confusion and may also cause extra
unnecessary processing when looking at the data.

Commit 638fb14169 (net: Make qmp_query_rx_filter() with name argument
more obvious) partially addresses this issue, by limiting the output
when the name is specified.  However, when the name is not specified,
the issue still persists.

Signed-off-by: Vladislav Yasevich 
---
 net/net.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/net/net.c b/net/net.c
index 39af893..a8cfeba 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1219,6 +1219,12 @@ RxFilterInfoList *qmp_query_rx_filter(bool has_name, 
const char *name,
 continue;
 }
 
+/* only query information on queue 0 since the info is per nic,
+ * not per queue
+ */
+if (nc->queue_index != 0)
+continue;
+
 if (nc->info->query_rx_filter) {
 info = nc->info->query_rx_filter(nc);
 entry = g_malloc0(sizeof(*entry));
-- 
1.9.3

[Qemu-devel] [PATCH 1/1] vhost: set the correct queue index in case of migration with multiqueue

[Thibaut Collet]

When a live migration is started the log address to mark dirty pages is provided
to the vhost backend through the vhost_dev_set_log function.
This function is called for each queue pairs but the queue index is wrongly set:
always set to the first queue pair. Then vhost backend lost descriptor addresses
of the queue pairs greater than 1 and behaviour of the vhost backend is
unpredictable.

The queue index is computed by taking account of the vq_index (to retrieve the
queue pair index) and calling the vhost_get_vq_index method of the backend.

Signed-off-by: Thibaut Collet 
---
 hw/virtio/vhost.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index feeaaa4..de29968 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -656,13 +656,14 @@ static int vhost_dev_set_features(struct vhost_dev *dev, 
bool enable_log)
 
 static int vhost_dev_set_log(struct vhost_dev *dev, bool enable_log)
 {
-int r, t, i;
+int r, t, i, idx;
 r = vhost_dev_set_features(dev, enable_log);
 if (r < 0) {
 goto err_features;
 }
 for (i = 0; i < dev->nvqs; ++i) {
-r = vhost_virtqueue_set_addr(dev, dev->vqs + i, i,
+idx = dev->vhost_ops->vhost_get_vq_index(dev, dev->vq_index + i);
+r = vhost_virtqueue_set_addr(dev, dev->vqs + i, idx,
  enable_log);
 if (r < 0) {
 goto err_vq;
@@ -671,7 +672,8 @@ static int vhost_dev_set_log(struct vhost_dev *dev, bool 
enable_log)
 return 0;
 err_vq:
 for (; i >= 0; --i) {
-t = vhost_virtqueue_set_addr(dev, dev->vqs + i, i,
+idx = dev->vhost_ops->vhost_get_vq_index(dev, dev->vq_index + i);
+t = vhost_virtqueue_set_addr(dev, dev->vqs + i, idx,
  dev->log_enabled);
 assert(t >= 0);
 }
-- 
2.1.4

Re: [Qemu-devel] [PULL 0/7] fw_cfg: add dma interface, add strings via cmdline.

[Gerd Hoffmann]

  Hi

> > Windows fails to compile:
> > /home/petmay01/linaro/qemu-for-merges/hw/nvram/fw_cfg.c: In function
> > ‘fw_cfg_dma_mem_read’:
> > /home/petmay01/linaro/qemu-for-merges/hw/nvram/fw_cfg.c:406: warning:
> > integer constant is too large for ‘long’ type
> 
> I don't have a Windows test environment, but I suspect the following:

Builds fine here for some strange reason ...

> #define FW_CFG_DMA_SIGNATURE 0x51454d5520434647 /* "QEMU CFG" */
> 
> should be changed to:
> 
> #define FW_CFG_DMA_SIGNATURE 0x51454d5520434647ULL /* "QEMU CFG" */

But that is correct.  Fixed.

cheers,
  Gerd

Re: [Qemu-devel] Question about xen disk unplug support for ahci missed in qemu

[Fabio Fantoni]

Il 16/10/2015 18:53, Paul Durrant ha scritto:

-Original Message-
From: Kevin Wolf [mailto:kw...@redhat.com]
Sent: 16 October 2015 17:43
To: Paul Durrant
Cc: Fabio Fantoni; Stefano Stabellini; John Snow; Anthony Perard; qemu-
de...@nongnu.org; xen-de...@lists.xen.org; qemu-bl...@nongnu.org
Subject: Re: [Qemu-devel] Question about xen disk unplug support for ahci
missed in qemu

Am 16.10.2015 um 18:20 hat Paul Durrant geschrieben:

-Original Message-
From: Kevin Wolf [mailto:kw...@redhat.com]
Sent: 16 October 2015 17:12
To: Paul Durrant
Cc: Fabio Fantoni; Stefano Stabellini; John Snow; Anthony Perard; qemu-
de...@nongnu.org; xen-de...@lists.xen.org; qemu-bl...@nongnu.org
Subject: Re: [Qemu-devel] Question about xen disk unplug support for

ahci

missed in qemu

Am 16.10.2015 um 17:10 hat Paul Durrant geschrieben:

-Original Message-
From: Kevin Wolf [mailto:kw...@redhat.com]
Sent: 16 October 2015 16:02
To: Paul Durrant
Cc: Fabio Fantoni; Stefano Stabellini; John Snow; Anthony Perard;

qemu-

de...@nongnu.org; xen-de...@lists.xen.org; qemu-

bl...@nongnu.org

Subject: Re: [Qemu-devel] Question about xen disk unplug support

for

ahci

missed in qemu

Am 16.10.2015 um 16:24 hat Paul Durrant geschrieben:

-Original Message-
From: Kevin Wolf [mailto:kw...@redhat.com]
Sent: 16 October 2015 15:04
To: Paul Durrant
Cc: Fabio Fantoni; Stefano Stabellini; John Snow; Anthony Perard;

qemu-

de...@nongnu.org; xen-de...@lists.xen.org; qemu-

bl...@nongnu.org

Subject: Re: [Qemu-devel] Question about xen disk unplug

support

for

ahci

missed in qemu

Am 14.10.2015 um 14:48 hat Paul Durrant geschrieben:

-Original Message-
From: Fabio Fantoni [mailto:fabio.fant...@m2r.biz]
Sent: 14 October 2015 12:12
To: Kevin Wolf; Stefano Stabellini
Cc: John Snow; Anthony Perard; qemu-devel@nongnu.org;

xen-

de...@lists.xen.org; qemu-bl...@nongnu.org; Paul Durrant
Subject: Re: [Qemu-devel] Question about xen disk unplug

support

for

ahci

missed in qemu



Il 14/10/2015 11:47, Kevin Wolf ha scritto:

[ CC qemu-block ]

Am 13.10.2015 um 19:10 hat Stefano Stabellini geschrieben:

On Tue, 13 Oct 2015, John Snow wrote:

On 10/13/2015 11:55 AM, Fabio Fantoni wrote:

I added ahci disk support in libxl and using it for week

seems

that

was

ok, after a reply of Stefano Stabellini seems that xen disk

unplug

support only ide disks:


http://git.qemu.org/?p=qemu.git;a=commitdiff;h=679f4f8b178e7c66fbc2f39

c905374ee8663d5d8

Today Paul Durrant told me that even if pv disk is ok also

with

ahci

and

the emulated one is offline can be a risk:
http://lists.xenproject.org/archives/html/win-pv-

devel/2015-

10/msg00021.html


I tried to take a fast look in qemu code but I not

understand

the

needed

thing for add the xen disk unplug support also for ahci,

can

someone do

it or tell me useful information for do it please?

Thanks for any reply and sorry for my bad english.


I'm not entirely sure what features you need AHCI to

support

in

order

for Xen to be happy.

I'd guess hotplugging, but where I get confused is that IDE

disks

don't

support hotplugging either, so I guess I'm not sure sure

what

you

need.

Stefano, can you help bridge my Xen knowledge gap?

Hi John,

we need something like

hw/i386/xen/xen_platform.c:unplug_disks

but

that

can unplug AHCI disk. And by unplug, I mean "make

disappear"

like

pci_piix3_xen_ide_unplug does for ide.

Maybe this would be the right time to stop the craziness

with

your

hybrid IDE/xendisk setup. It's a horrible thing that would

never

happen

on real hardware.

Unfortunately, it's going to be difficult to remove such 'craziness'

when

you

don't know a priori whether the VM has PV drivers or not.

Why wouldn't you know that beforehand? I mean, even on real

hardware

you
can have different disk interfaces (IDE, AHCI, SCSI) and you install
the exact driver that your hardware needs. You just do the same

thing on

VM: If your hardware is PV, you install a PV driver. If your

hardware is

IDE, you install an IDE driver. Whether it's PV or IDE is something

that

you, the user, decided when configuring the VM, so you definitely

know.

That's not necessarily true. The host admin that provisions the VM

does

not

necessarily know what OS the user of that VM will install. The admin

may

just

be providing a generic VM with an emulated CD drive that the user

can

point

at any ISO they want.

So, as a host admin, if you provide a VM with only PV backends and

your

user is trying to boot an OS with no PV drivers they are not going to be
happy, so you provide emulated devices. Then, at some point later,

when

the user installs PV drivers, there really should be some way for those

drivers

to start up without any need to contact the host admin and have the

VM

reconfigured.

Why only IDE and xendisk then? Maybe I have an OS that works great

with

AHCI, or virtio-blk, or an LSI SCSI controller, or a Megasas SCSI
controller, or USB sticks, 

Re: [Qemu-devel] What's the intended use of log.h logging?

[Peter Maydell]

On 19 October 2015 at 14:17, Markus Armbruster  wrote:
> Peter Maydell  writes:
>> In a lot of cases, especially with the TCG logging, not enabling
>> voluminous tracing is really important because if you turn it all
>> on then the system is way too slow (and can behave differently
>> as a result), and generates gigabytes of trace output. (-d exec
>> and -d cpu will do this, for instance.)
>
> This is at least as much an argument for use of tracing as against it.
> Tracing is a lot more flexible than log.h, and with the right backend,
> it's much more efficient, too.
>
> If the appropriate trace patterns turn out to be too hard to remember,
> we can provide canned trace patterns with names that are easy to
> remember.
>
> -d could become sugar for a suitable trace patterns.

I don't object to the use of tracing under the hood, as long as
the user-facing experience remains as good as what we have for -d
at the moment (in terms of it being always present, working the
same for everybody, easily discoverable and simple to use).

thanks
-- PMM

[Qemu-devel] [PATCH 1/3] block/gluster: add support for multiple gluster servers

[Prasanna Kumar Kalever]

This patch adds a way to specify multiple volfile servers to the gluster
block backend of QEMU with tcp|rdma transport types and their port numbers.

Problem:

Currenly VM Image on gluster volume is specified like this:

file=gluster[+tcp]://host[:port]/testvol/a.img

Assuming we have three hosts in trustred pool with replica 3 volume
in action and unfortunately host (mentioned in the command above) went down
for some reason, since the volume is replica 3 we now have other 2 hosts
active from which we can boot the VM.

But currently there is no mechanism to pass the other 2 gluster host
addresses to qemu.

Solution:

New way of specifying VM Image on gluster volume with volfile servers:
(We still support old syntax to maintain backward compatibility)

Basic command line syntax looks like:

Pattern I:
 -drive driver=gluster,
volume=testvol,path=/path/a.raw,
servers.0.host=1.2.3.4,
   [servers.0.port=24007,]
   [servers.0.transport=tcp,]
servers.1.host=5.6.7.8,
   [servers.1.port=24008,]
   [servers.1.transport=rdma,] ...

Pattern II:
 'json:{"driver":"qcow2","file":{"driver":"gluster",
   "volume":"testvol","path":"/path/a.qcow2",
   "servers":[{tuple0},{tuple1}, ...{tupleN}]}}'

   driver  => 'gluster' (protocol name)
   volume  => name of gluster volume where our VM image resides
   path=> absolute path of image in gluster volume

  {tuple}  => {"host":"1.2.3.4"[,"port":"24007","transport":"tcp"]}

   host=> host address (hostname/ipv4/ipv6 addresses)
   port=> port number on which glusterd is listening. (default 24007)
   transport   => transport type used to connect to gluster management daemon,
   it can be tcp|rdma (default 'tcp')

Examples:
1.
 -drive driver=qcow2,file.driver=gluster,
file.volume=testvol,file.path=/path/a.qcow2,
file.servers.0.host=1.2.3.4,
file.servers.0.port=24007,
file.servers.0.transport=tcp,
file.servers.1.host=5.6.7.8,
file.servers.1.port=24008,
file.servers.1.transport=rdma
2.
 'json:{"driver":"qcow2","file":{"driver":"gluster","volume":"testvol",
 "path":"/path/a.qcow2","servers":
 [{"host":"1.2.3.4","port":"24007","transport":"tcp"},
  {"host":"4.5.6.7","port":"24008","transport":"rdma"}] } }'

This patch gives a mechanism to provide all the server addresses, which are in
replica set, so in case host1 is down VM can still boot from any of the
active hosts.

This is equivalent to the backup-volfile-servers option supported by
mount.glusterfs (FUSE way of mounting gluster volume)

This patch depends on a recent fix in libgfapi raised as part of this work:
http://review.gluster.org/#/c/12114/

Credits: Sincere thanks to Kevin Wolf  and
"Deepak C Shetty"  for inputs and all their support

Signed-off-by: Prasanna Kumar Kalever 
---
This series of patches are sent based on "Peter Krempa" 
review comments on v8 sent earlier.
---
 block/gluster.c  | 414 +--
 qapi/block-core.json |  60 +++-
 2 files changed, 426 insertions(+), 48 deletions(-)

diff --git a/block/gluster.c b/block/gluster.c
index 1eb3a8c..dd076fe 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -11,6 +11,16 @@
 #include "block/block_int.h"
 #include "qemu/uri.h"
 
+#define GLUSTER_OPT_FILENAME   "filename"
+#define GLUSTER_OPT_VOLUME "volume"
+#define GLUSTER_OPT_PATH   "path"
+#define GLUSTER_OPT_HOST   "host"
+#define GLUSTER_OPT_PORT   "port"
+#define GLUSTER_OPT_TRANSPORT  "transport"
+#define GLUSTER_OPT_SERVER_PATTERN "servers."
+
+#define GLUSTER_DEFAULT_PORT   24007
+
 typedef struct GlusterAIOCB {
 int64_t size;
 int ret;
@@ -24,22 +34,72 @@ typedef struct BDRVGlusterState {
 struct glfs_fd *fd;
 } BDRVGlusterState;
 
-typedef struct GlusterConf {
+typedef struct GlusterServerConf {
 char *server;
 int port;
+char *transport;
+} GlusterServerConf;
+
+typedef struct GlusterConf {
 char *volname;
 char *image;
-char *transport;
+GlusterServerConf *gsconf;
 } GlusterConf;
 
+static QemuOptsList runtime_json_opts = {
+.name = "gluster_json",
+.head = QTAILQ_HEAD_INITIALIZER(runtime_json_opts.head),
+.desc = {
+{
+.name = GLUSTER_OPT_VOLUME,
+.type = QEMU_OPT_STRING,
+.help = "name of gluster volume where our VM image resides",
+},
+{
+.name = GLUSTER_OPT_PATH,
+.type = QEMU_OPT_STRING,
+.help = "absolute path to image file in gluster volume",
+},
+{ /* end of list */ }
+},
+};
+
+static QemuOptsList runtime_tuple_opts = {
+.name = "gluster_tuple",
+.head = QTAILQ_HEAD_INITIALIZER(runtime_tuple_opts.head),
+.desc = {
+{
+.name = GLUSTER_OPT_HOST,
+

Re: [Qemu-devel] What's the intended use of log.h logging?

[Markus Armbruster]

Peter Maydell  writes:

> On 16 October 2015 at 15:17, Paolo Bonzini  wrote:
>> On 16/10/2015 15:36, Peter Maydell wrote:
 We could add "-d trace:help" to list valid tracepoint names.
>>>
>>> I think people are mostly going to care about categories
>>> (eg "enable tracing for device X", or "enable tracing for
>>> all unimplemented features", or "enable a medium level of
>>> debug tracing for device Y".
>>
>> I hadn't thought about levels, but honestly when I use KVM tracing (which
>> is often measured in tens of megabytes per run) I don't care.
>
> In a lot of cases, especially with the TCG logging, not enabling
> voluminous tracing is really important because if you turn it all
> on then the system is way too slow (and can behave differently
> as a result), and generates gigabytes of trace output. (-d exec
> and -d cpu will do this, for instance.)

This is at least as much an argument for use of tracing as against it.
Tracing is a lot more flexible than log.h, and with the right backend,
it's much more efficient, too.

If the appropriate trace patterns turn out to be too hard to remember,
we can provide canned trace patterns with names that are easy to
remember.

-d could become sugar for a suitable trace patterns.

Re: [Qemu-devel] [PATCH v8 00/27] vhost-user: add migration support

[Thibaut Collet]

On Sun, Oct 18, 2015 at 10:21 AM, Michael S. Tsirkin  wrote:
>
> On Tue, Oct 13, 2015 at 02:19:41PM +0200, Thibaut Collet wrote:
> > Hi,
> >
> > I have still a comment on this serie. During rebase operation with 
> > multiqueue a
> > modification has been lost.
> > This lost impact only guest without GUEST_ANNOUNCE capabilities: the 
> > backend is
> > not notified to send a fake RARP to reduce VM outage.
> >
> > Sorry for the late notice but I have tested only recent guest to give my ack
> > yesterday.
> >
> > Marc Andre and Michael could you apply the attached fixup to the patch 
> > "vhost
> > user: add support of live migration" on the pull request ?
> >
> > Thanks
> >
> > Best regards.
>
> The way to post fixups is just like a regular patch, but prepend
> fixup! on the subject line.
> Comments can come after the --- line.
>
>
>
> > On Mon, Oct 12, 2015 at 5:56 PM, Thibaut Collet 
> > wrote:
> >
> >
> >
> > On Fri, Oct 9, 2015 at 5:17 PM,  wrote:
> >
> > From: Marc-André Lureau 
> >
> > Hi,
> >
> > The following series implement shareable log for vhost-user to 
> > support
> > memory tracking during live migration. On qemu-side, the solution is
> > fairly straightfoward since vhost already supports the dirty log, 
> > only
> > vhost-user couldn't access the log memory until then.
> >
> > The series includes "vhost user: Add live migration" patches from
> > Thibaut Collet.
> >
> > v7->v8:
> > - rebased
> > - fix build on osx & aarch64
> > - add seccomp patch from Eduardo
> > - fix enum usage and MQ (squashed Thibaut fix)
> > - fixed vhost_net_notify_migration_done() fallback
> > - split util-obj- on multi-lines in seperate patch
> >
> > v6->v7:
> > - add migration blocker if memfd failed
> > - add doc about the qemu_memfd_alloc() helper
> > - (rebase on top of Michael pci branch)
> >
> > v5->v6:
> > - rebased
> > - fix protocol feature mask update
> > - add a patch from Thibault Collet using enum for features, and
> >   compute mask
> > - add unistd.h linux headers to help building memfd if missing on
> >   build host. Hopefully will be useful for other syscalls some day
> > - reorder/merge patches to share-allocate the log only if needed
> > - split the memfd helper to allow to drop the fallback code
> > - allow to call qemu_memfd_free() even if alloc failed
> > - add some missing spaces
> >
> > v4->v5:
> > - rebase on top of last Michael S. Tsirkin PULL request
> > - block live migration if !PROTOCOL_F_LOG_SHMFD
> > - wait for a reply after SET_LOG_BASE
> > - split vhost_set_log_base from the rest of vhost_call refactoring
> > - use a seperate global vhost_log_shm
> >
> > v3->v4:
> > - add the proto negotiation & the migration series
> > - replace the varargs vhost_call() approach for callbacks
> > - only share-allocate when the backend needs it
> >
> > v2->v3:
> > - changed some patch summary
> > - added migration tests
> > - added a patch to replace error message with a trace
> >
> > The development branch I used is:
> > https://github.com/elmarco/qemu branch "vhost-user"
> >
> > Eduardo Otubo (1):
> >   seccomp: add memfd_create to whitelist
> >
> > Marc-André Lureau (22):
> >   configure: probe for memfd
> >   linux-headers: add unistd.h
> >   build-sys: split util-obj- on multi-lines
> >   util: add linux-only memfd fallback
> >   util: add memfd helpers
> >   util: add fallback for qemu_memfd_alloc()
> >   vhost: document log resizing
> >   vhost: add vhost_set_log_base op
> >   vhost-user: add vhost_user_requires_shm_log()
> >   vhost: alloc shareable log
> >   vhost-user: send log shm fd along with log_base
> >   vhost-user: add a migration blocker
> >   vhost: use a function for each call
> >   vhost-user: document migration log
> >   net: add trace_vhost_user_event
> >   vhost: add migration block if memfd failed
> >   vhost-user-test: move wait_for_fds() out
> >   vhost-user-test: remove useless static check
> >   vhost-user-test: wrap server in TestServer struct
> >   vhost-user-test: learn to tweak various qemu arguments
> >   vhost-user-test: add live-migration test
> >   vhost-user-test: check ownership during migration
> >
> > Michael S. Tsirkin (1):
> >   exec: factor out duplicate mmap code
> >
> > Thibaut Collet (3):
> >   vhost user: add support of live migration
> >   vhost user: