Re: [Qemu-devel] [edk2] Could not add PCI device with big memory to aarch64 VMs

[Laszlo Ersek]

On 11/30/15 19:45, liang yan wrote:
> 
> 
> On 11/04/2015 05:53 PM, Laszlo Ersek wrote:
>> On 11/04/15 23:22, liang yan wrote:
>>> Hello, Laszlo,
>>>
>>>
>>> (2)It also has a problem that once I use a memory bigger than 256M for
>>> ivshmem, it could not get through UEFI,
>>> the error message is
>>>
>>> PciBus: Discovered PCI @ [00|01|00]
>>> BAR[0]: Type =  Mem32; Alignment = 0xFFF;Length = 0x100;
>>> Offset =
>>> 0x10
>>> BAR[1]: Type =  Mem32; Alignment = 0xFFF;Length = 0x1000; Offset
>>> = 0x14
>>> BAR[2]: Type = PMem64; Alignment = 0x3FFF;Length =
>>> 0x4000;Offset = 0x18
>>>
>>> PciBus: HostBridge->SubmitResources() - Success
>>> ASSERT
>>> /home/liang/studio/edk2/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c(449):
>>> ((BOOLEAN)(0==1))
>>>
>>>
>>> I am wandering if there are memory limitation for pcie devices under
>>> Qemu environment?
>>>
>>>
>>> Just thank you in advance and any information would be appreciated.
>> (CC'ing Ard.)
>>
>> "Apparently", the firmware-side counterpart of QEMU commit 5125f9cd2532
>> has never been contributed to edk2.
>>
>> Therefore the the ProcessPciHost() function in
>> "ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c" ignores the
>> DTB_PCI_HOST_RANGE_MMIO64 type range from the DTB. (Thus only
>> DTB_PCI_HOST_RANGE_MMIO32 is recognized as PCI MMIO aperture.)
>>
>> However, even if said driver was extended to parse the new 64-bit
>> aperture into PCDs (which wouldn't be hard), the
>> ArmVirtPkg/PciHostBridgeDxe driver would still have to be taught to look
>> at that aperture (from the PCDs) and to serve MMIO BAR allocation
>> requests from it. That could be hard.
>>
>> Please check edk2 commits e48f1f15b0e2^..e5ceb6c9d390, approximately,
>> for the background on the current code. See also chapter 13 "Protocols -
>> PCI Bus Support" in the UEFI spec.
>>
>> Patches welcome. :)
>>
>> (A separate note on ACPI vs. DT: the firmware forwards *both* from QEMU
>> to the runtime guest OS. However, the firmware parses only the DT for
>> its own purposes.)
> Hello, Laszlo,
> 
> Thanks for your advices above, it's very helpful.
> 
> When debugging, I also found some problems for 32 bit PCI devices.
> Hope could get some clues from you.
> 
> I checked on 512M, 1G, and 2G devices.(4G return invalid parameter
> error, so I think it may be taken as a 64bit devices, is this right?).

I guess so.

> 
> 
> First,
> 
> All devices start from base address 3EFE.

According to the below:

> ProcessPciHost: Config[0x3F00+0x100) Bus[0x0..0xF]
> Io[0x0+0x1)@0x3EFF Mem[0x1000+0x2EFF)@0x0

the address you mention (0x3EFE) is the *highest* inclusive
guest-phys address that an MMIO BAR can take. Not sure if that's what
you meant.

The size of the MMIO aperture for the entire PCI host is 0x2EFF
bytes: a little less than 752 MB. So devices that need 1G and 2G MMIO
BARs have no chance.

> PcdPciMmio32Base is  1000=
> PcdPciMmio32Size is  2EFF=
> 
> 
> Second,
> 
> It could not get new base address when searching memory space in GCD map.
> 
> For 512M devices,
> 
> *BaseAddress = (*BaseAddress + 1 - Length) & (~AlignmentMask);

This seems to be from CoreAllocateSpace()
[MdeModulePkg/Core/Dxe/Gcd/Gcd.c]. AlignmentMask is computed from the
Alignment input parameter.

Which in turn seems to come from the BitsOfAlignment parameter computed
in NotifyPhase(), "ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c".

> 
> BaseAddress is 3EFE==

So this is the highest address (inclusive) where the 512MB BAR could end.

> new BaseAddress is 1EEF==

This is the highest address (inclusive) where the 512MB BAR could start.

This should be rounded down to an 512MB alignment (I believe), and then
checked if that is still in the MMIO aperture.

512MB is 0x2000_.

Rounding 0x1EEF_ down to an integral multiple of 0x2000_ results
in zero:

> ~AlignmentMask is E000==
> Final BaseAddress is 

And that address does not fall into the MMIO aperture.

In other words, although your size requirement of 512MB could be
theoretically satisfied from the aperture (which extends from exactly
256 MB to a little lower than 1008 MB), if you *also* require the base
address to be aligned at 512MB, then that cannot be satisfied.

> 
> Status = CoreSearchGcdMapEntry (*BaseAddress, Length, ,
> , Map);
> 
> 
> 
> For bigger devices:
> 
> all stops when searching memory space because below code, Length will
> bigger than MaxAddress(3EFE)
> 
> if ((Entry->BaseAddress + Length) > MaxAddress) {
>  continue;
> }
> 
> 
> I also checked on ArmVirtQemu.dsc which all set to 0.
> 
>   gArmPlatformTokenSpaceGuid.PcdPciBusMin|0x0
>   gArmPlatformTokenSpaceGuid.PcdPciBusMax|0x0
>   gArmPlatformTokenSpaceGuid.PcdPciIoBase|0x0
>   gArmPlatformTokenSpaceGuid.PcdPciIoSize|0x0
>   gArmPlatformTokenSpaceGuid.PcdPciIoTranslation|0x0
>   

Re: [Qemu-devel] [PATCH] m25p80.c Added support for N25Q256 and N25Q512

[Krzeminski, Marcin (Nokia - PL/Wroclaw)]

> -Original Message-
> From: EXT Peter Crosthwaite [mailto:crosthwaitepe...@gmail.com]
> Sent: Monday, November 30, 2015 9:55 PM
> To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
> Cc: qemu-devel@nongnu.org; g...@xilinx.com; Sai Pavan Boddu
> Subject: Re: [Qemu-devel] [PATCH] m25p80.c Added support for N25Q256
> and N25Q512
> 
> On Mon, Nov 30, 2015 at 11:51 AM, Krzeminski, Marcin (Nokia -
> PL/Wroclaw)  wrote:
> >
> >
> >> -Original Message-
> >> From: EXT Peter Crosthwaite [mailto:crosthwaitepe...@gmail.com]
> >> Sent: Sunday, November 29, 2015 8:19 PM
> >> To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
> >> Cc: qemu-devel@nongnu.org; g...@xilinx.com; Sai Pavan Boddu
> >> Subject: Re: [Qemu-devel] [PATCH] m25p80.c Added support for
> N25Q256
> >> and N25Q512
> >>
> >> On Sun, Nov 29, 2015 at 5:12 AM, Krzeminski, Marcin (Nokia -
> >> PL/Wroclaw)  wrote:
> >> >
> >> >
> >> >> -Original Message-
> >> >> From: EXT Peter Crosthwaite [mailto:crosthwaitepe...@gmail.com]
> >> >> Sent: Saturday, November 28, 2015 7:50 PM
> >> >> To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
> >> >> Cc: qemu-devel@nongnu.org; g...@xilinx.com; Sai Pavan Boddu
> >> >> Subject: Re: [Qemu-devel] [PATCH] m25p80.c Added support for
> >> N25Q256
> >> >> and N25Q512
> >> >>
> >> >> These features are also available in Xilinx QEMU if you want to
> >> >> compare
> >> >> implementation:
> >> >>
> >> >>
> >>
> https://github.com/Xilinx/qemu/blob/pub/2015.2.plnx/hw/block/m25p80.c
> >> >>
> >> >> That work also handles the larger and newer Spansion flash parts,
> >> >> as well as the quad and dual mode commands for QSPI (also features
> >> >> of
> >> n25qXXX).
> >> >>
> >> > Too bad I did not checked xilix fork, so V2 of this patch does not
> >> > make
> >> sense since all is already implemented.
> >>
> >> V2 still makes sense. My V1 comments are pretty minor and that Xilinx
> >> work will need cleanup before upstreaming. It is not a competing
> >> implementaiton and the diff there will go down when it is rebased on
> yours.
> >>
> > Since this is start with open source, making this feature at least
> > ready to pull seem to be worth to try. I'll prepare v2 when I got some free
> time.
> 
> There is a learning curve for some people on getting into patch respinning.
> You need some features of git that let you rewrite history. Specifically you
> want to look at:
> 
> git rebase -i
> git add -p
> git commit --amend
> 
> If you are unfamiliar.
> 
For me it is much more complicated. I added support for those flash devices in 
one commit,
then made one or two fixes and that's it. It is really hard to get patch set 
from that..
The way of working is the main issue here...

> >> > Why didn't xilinks merge it with mainline?
> >>
> >> There's a lot in that tree and Xilinx hasn't gotten around to it yet.
> >>
> > Yes, I noticed one interesting feature.
> 
> Are you able to share more? :) If there is known community interest in
> specific features it helps upstreaming.
> 
Yes, most interesting thing is the I2C eeproms (m24cxx.c file). There is smbus 
eeprom,
but this is not exactly I2C. At least from my point of view it is interesting 
to have it in upstream :)

Regards,
Marcin

> Regards,
> Peter

Re: [Qemu-devel] [PATCH v3 10/12] Dump: add qmp command "query-dump"

[Eric Blake]

On 11/30/2015 04:32 AM, Peter Xu wrote:
> When dump-guest-memory is requested with detach flag, after its
> return, user could query its status using "query-dump" command (with
> no argument). The result for now contains:
> 
> - status: current dump status
> - written_bytes: bytes written in latest dump
> - total_bytes: bytes to write in latest dump
> 
>>From written_bytes and total_bytes, we could see how much work
> finished by calculating:
> 
>   100.0 * written_bytes / total_bytes (%)
> 
> Signed-off-by: Peter Xu 
> ---
>  dump.c   | 10 ++
>  qapi-schema.json | 29 +
>  qmp-commands.hx  | 26 +-
>  3 files changed, 64 insertions(+), 1 deletion(-)
> 
> diff --git a/dump.c b/dump.c
> index 56a2d7e..6596bc8 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -1675,6 +1675,16 @@ static void *dump_thread(void *data)
>  return NULL;
>  }
>  
> +DumpQueryResult *qmp_query_dump(Error **errp)
> +{
> +DumpQueryResult *result = g_malloc0(sizeof(*result));

Might be nicer as g_new0(DumpQueryResult, 1). Markus has been switching
to g_new0 where a type name was already mentioned, although here you
used *result rather than a typename.


> +++ b/qapi-schema.json
> @@ -2157,6 +2157,35 @@
>'data': [ 'none', 'active', 'completed', 'failed' ] }
>  
>  ##
> +# @DumpQueryResult
> +#
> +# The result format for 'query-dump'.
> +#
> +# @status: enum of @DumpStatus, which shows current dump status
> +#
> +# @written_bytes: bytes written in latest dump (uncompressed)
> +#
> +# @total_bytes: total bytes to be write in latest dump (uncompressed)

s/be write/written/

> +#
> +# Since 2.6
> +##
> +{ 'struct': 'DumpQueryResult',
> +  'data': { 'status': 'DumpStatus',
> +'written_bytes': 'int',
> +'total_bytes': 'int' } }

Prefer '-' over '_' in new QMP (as in 'total-bytes' rather than
'total_bytes').  Furthermore, QMP already defaults to bytes, so it would
be sufficient to name these merely 'written'/'total' or even
'complete'/'total'.

> +++ b/qmp-commands.hx
> @@ -881,7 +881,7 @@ EQMP
>  {
>  .name   = "query-dump-guest-memory-capability",
>  .args_type  = "",
> -.mhandler.cmd_new = qmp_marshal_query_dump_guest_memory_capability,
> +.mhandler.cmd_new = qmp_marshal_query_dump_guest_memory_capability,
>  },

Unrelated hunk.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 02/12] dump-guest-memory: add "detach" flag for QMP/HMP interfaces.

[Eric Blake]

On 11/30/2015 04:32 AM, Peter Xu wrote:
> This patch only adds the interfaces, but not implements them.
> "detach" parameter is made optional, to make sure that all the old
> dump-guest-memory requests will still be able to work.
> 
> Signed-off-by: Peter Xu 
> ---
>  dump.c   | 5 +++--
>  hmp-commands.hx  | 5 +++--
>  hmp.c| 9 +++--
>  qapi-schema.json | 8 ++--
>  qmp-commands.hx  | 6 --
>  5 files changed, 23 insertions(+), 10 deletions(-)

> +++ b/hmp.c
> @@ -1586,8 +1586,10 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict 
> *qdict)
>  const char *file = qdict_get_str(qdict, "filename");
>  bool has_begin = qdict_haskey(qdict, "begin");
>  bool has_length = qdict_haskey(qdict, "length");
> +bool has_detach = qdict_haskey(qdict, "detach");

Here, you probe whether 'detach' is present...

>  int64_t begin = 0;
>  int64_t length = 0;
> +bool detach = false;

...here, you default 'detach' to false,..

>  enum DumpGuestMemoryFormat dump_format = DUMP_GUEST_MEMORY_FORMAT_ELF;
>  char *prot;
>  
> @@ -1615,11 +1617,14 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict 
> *qdict)
>  if (has_length) {
>  length = qdict_get_int(qdict, "length");
>  }
> +if (has_detach) {
> +detach = qdict_get_try_bool(qdict, "detach", false);

...therefore, this line is only reachable if 'detach' is present, which
means the default will never be needed.

> +}
>  
>  prot = g_strconcat("file:", file, NULL);
>  
> -qmp_dump_guest_memory(paging, prot, has_begin, begin, has_length, length,
> -  true, dump_format, );
> +qmp_dump_guest_memory(paging, prot, has_detach, detach, has_begin, begin,
> +  has_length, length, true, dump_format, );

There are two competing ways to simplify this; I don't care which one
you choose, although you can't do both:

1. Note that since the second assignment to detach only happens if
has_detach is true, you can use the simpler:

if (has_detach) {
detach = qdict_get_bool(qdict, "detach");
}

If you go with this approach, you could even get away without
initializing 'detach' outside of the 'if (has_detach)' conditional
(although it's still probably wiser to avoid uninitialized memory than
to rely on qmp_dump_guest_memory() correctly ignoring 'detach' when
'has_detach' is false).

2. Note that since qdict_get_try_bool() lets you specify a default, you
can eliminate the has_detach variable and instead just do:

bool detach = qdict_get_try_bool(qdict, "detach", false);
...
qmp_dump_guest_memory(paging, prot, true, detach, ...);

> +++ b/qapi-schema.json
> @@ -2115,6 +2115,9 @@
>  #2. fd: the protocol starts with "fd:", and the following string
>  #   is the fd's name.
>  #
> +# @detach: #optional if true, QMP will return immediately rather than
> +#  waiting dump to be finished (since 2.6).

s/dump/for the dump/
s/be finished/finish/

> @@ -857,6 +857,8 @@ Arguments:
>  - "paging": do paging to get guest's memory mapping (json-bool)
>  - "protocol": destination file(started with "file:") or destination file
>descriptor (started with "fd:") (json-string)
> +- "detach": if specificed, command will return immediately, without waiting

s/specificed/specified/

> +for dump to be finished (json-bool)

s/dump/the dump/
s/be finished/finish/

>  - "begin": the starting physical address. It's optional, and should be 
> specified
> with length together (json-int)
>  - "length": the memory size, in bytes. It's optional, and should be specified
> 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 08/12] dump-guest-memory: add qmp event DUMP_COMPLETED

[Eric Blake]

On 11/30/2015 04:32 AM, Peter Xu wrote:
> One new QMP event DUMP_COMPLETED is added. It is used when user
> specified "detach" in dump, and triggered when the dump finishes
> (either succeeded or failed). If failed, one "err" data will be
> passed with specific error message.
> 
> Signed-off-by: Peter Xu 
> ---
>  docs/qmp-events.txt | 14 ++
>  dump.c  |  6 +-
>  qapi/event.json | 10 ++
>  3 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/qmp-events.txt b/docs/qmp-events.txt
> index d2f1ce4..7b9f835 100644
> --- a/docs/qmp-events.txt
> +++ b/docs/qmp-events.txt
> @@ -674,3 +674,17 @@ Note: If action is "reset", "shutdown", or "pause" the 
> WATCHDOG event is
>  followed respectively by the RESET, SHUTDOWN, or STOP events.
>  
>  Note: this event is rate-limited.
> +
> +DUMP_COMPLETED
> +--
> +
> +Emitted when the guest has finished one memory dump.
> +
> +Data:
> +
> +- "error": Error message when dump failed (json-string, optional)
> +
> +Example:
> +
> +{ "event": "DUMP_COMPLETED",
> +  "data": {} }

Please keep this file sorted.  The insertion should be between
DEVICE_TRAY_MOVED and GUEST_PANICKED.

> diff --git a/dump.c b/dump.c
> index 14fd41f..43f565d 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -25,6 +25,7 @@
>  #include "qapi/error.h"
>  #include "qapi/qmp/qerror.h"
>  #include "qmp-commands.h"
> +#include "qapi-event.h"
>  
>  #include 
>  #ifdef CONFIG_LZO
> @@ -1633,8 +1634,11 @@ static void *dump_thread(void *data)
>  dump_process(s, );
>  
>  if (err) {
> -/* TODO: notify user the error */
> +qapi_event_send_dump_completed(true, error_get_pretty(err),
> +   _abort);
>  error_free(err);
> +} else {
> +qapi_event_send_dump_completed(false, NULL, _abort);
>  }

Hmmm. I wonder if error_get_pretty() should be improved to return NULL
when there is no error.  Then we could write:

qapi_event_send_dump_completed(!!err, error_get_pretty(err),
   _abort);
error_free(err);

But that doesn't affect the correctness of your patch.

>  return NULL;
>  }
> diff --git a/qapi/event.json b/qapi/event.json
> index f0cef01..c46214b 100644
> --- a/qapi/event.json
> +++ b/qapi/event.json
> @@ -356,3 +356,13 @@
>  ##
>  { 'event': 'MEM_UNPLUG_ERROR',
>'data': { 'device': 'str', 'msg': 'str' } }
> +
> +##
> +# @DUMP_COMPLETED
> +#
> +# Emitted when background dump has completed
> +#
> +# Since: 2.6

Missing documentation of 'error'.

> +##
> +{ 'event': 'DUMP_COMPLETED' ,
> +  'data': { '*error': 'str' } }
> 

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] Reminder: QEMU 2.5rc3 (final RC) planned for Thurs 1st Dec, please get bugfixes in before then

[Peter Maydell]

Hi; just a reminder that QEMU 2.5 rc3 is planned for this Thursday,
the 1st December. (http://wiki.qemu.org/Planning/2.5#Release_Schedule)
This is our last scheduled release candidate, so please make sure you
get all outstanding bugfixes for 2.5 in before then. As usual I intend
to only roll an extra rc4 for any critical bugfixes that show up after
rc3.

If there's anything you aren't sure you're going to be able to get
in a pull request by Thursday please let me know so we can decide
whether it's better to slip rc3 by a day or so.

Listing bugs/patches that need to go into 2.5 on the Planning page
on the wiki is a good way to ensure I don't miss them by mistake.

Thanks for your help in getting the release out the door more or
less on time.

thanks
-- PMM

Re: [Qemu-devel] [Qemu-ppc] [PATCH 00/77] ppc: Add "native" POWER8 platform

[Cédric Le Goater]

On 11/30/2015 09:09 PM, Benjamin Herrenschmidt wrote:
> On Mon, 2015-11-30 at 19:15 +0100, Cédric Le Goater wrote:
>> The pnor file is compiled from github. The patch is below (without the dirty
>> cut and paste I did in loader.c). The offset for the PAYLOAD and BOOTKERNEL
>> partitions are hard coded but I guess we don't need to read the flash 
>> partition
>> table in qemu, not yet.
> 
> In practice we should read the partition tables, I don't like hard
> coded offsets... But we should probably create a proper "flash driver"
> that does a bunch of this, and also adds the BMC style flash interface
> so OPAL can write to nvram.

yes that would be better but I don't measure what it takes to implement 
the LPC/AHB bridge to access the PNOR.

C.

Re: [Qemu-devel] [PULL 0/5] ppc-for-2.5 queue 20151130

[Peter Maydell]

On 30 November 2015 at 08:44, David Gibson <da...@gibson.dropbear.id.au> wrote:
> The following changes since commit 714487515dbe0c65d5904251e796cd3a5b3579fb:
>
>   Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into 
> staging (2015-11-27 10:44:42 +)
>
> are available in the git repository at:
>
>   git://github.com/dgibson/qemu.git tags/ppc-for-2.5-20151130
>
> for you to fetch changes up to 7624789234cd63b671bce1b49b93b0b1c00ea407:
>
>   target-ppc/fpu_helper: fix FPSCR_FX bit shift operation (2015-11-30 
> 19:39:01 +1100)
>
> ----
> ppc patch queue for qemu-2.5 20151130
>
> target-ppc and related bugfix patches for qemu-2.5
>
> I don't have the facilities to test the Macintosh and BookE related
> patches.  I've sanity checked them (inspection + make check), but I'm
> otherwise relying on the submitters.
>
> 
> Hervé Poussineau (1):
>   mac_dbdma: always initialize channel field in DBDMA_channel
>
> Madhavan Srinivasan (2):
>   target-ppc: Move the FPSCR bit update macros to cpu.h
>   target-ppc/fpu_helper: fix FPSCR_FX bit shift operation
>
> Peter Maydell (1):
>   hw/ppc/ppc405_boards: Fix infinite recursion by converting taihu_cpld 
> from old_mmio
>
> Thomas Huth (1):
>   hw/ppc/spapr: Remove duplicated "pseries" alias

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v3 03/12] dump-guest-memory: using static DumpState, add DumpStatus

[Eric Blake]

On 11/30/2015 04:32 AM, Peter Xu wrote:
> Instead of malloc/free each time for DumpState, make it
> static. Added DumpStatus to show status for dump.
> 
> This is to be used for detach dump.

s/detach/detached/

> 
> Signed-off-by: Peter Xu 
> ---
>  dump.c| 30 +++---
>  include/sysemu/dump.h |  2 ++
>  qapi-schema.json  | 18 ++
>  3 files changed, 47 insertions(+), 3 deletions(-)
> 

In addition to Paolo's review,

> +++ b/qapi-schema.json
> @@ -2139,6 +2139,24 @@
>  '*format': 'DumpGuestMemoryFormat'} }
>  
>  ##
> +# @DumpStatus
> +#
> +# Define the status for dump guest memory.

Reads awkwardly.  Maybe:

Describe the status of a long-running background guest memory dump.

> +#
> +# @none: not started any dump-guest-memory yet.

@none: no dump-guest-memory has started yet

> +#
> +# @active: there is one dump running in background.
> +#
> +# @completed: the last dump has finished sucessfully

s/sucessfully/successfully/

Inconsistent on whether your lines end in '.'

> +#
> +# @failed: the last dump has failed.
> +#
> +# Since 2.6
> +##
> +{ 'enum': 'DumpStatus',
> +  'data': [ 'none', 'active', 'completed', 'failed' ] }
> +
> +##
>  # @DumpGuestMemoryCapability:
>  #
>  # A list of the available formats for dump-guest-memory
> 

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH] exec: Stop using memory after free

[Don Slutz]

memory_region_unref(mr) can free memory.

For example I got:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7f43280d4700 (LWP 4462)]
0x7f43323283c0 in phys_section_destroy (mr=0x7f43259468b0)
at /home/don/xen/tools/qemu-xen-dir/exec.c:1023
1023if (mr->subpage) {
(gdb) bt
at /home/don/xen/tools/qemu-xen-dir/exec.c:1023
at /home/don/xen/tools/qemu-xen-dir/exec.c:1034
at /home/don/xen/tools/qemu-xen-dir/exec.c:2205
(gdb) p mr
$1 = (MemoryRegion *) 0x7f43259468b0

And this change prevents this.

Signed-off-by: Don Slutz 
---
 exec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/exec.c b/exec.c
index de1cf19..0bf0a6e 100644
--- a/exec.c
+++ b/exec.c
@@ -1064,9 +1064,11 @@ static uint16_t phys_section_add(PhysPageMap *map,
 
 static void phys_section_destroy(MemoryRegion *mr)
 {
+bool have_sub_page = mr->subpage;
+
 memory_region_unref(mr);
 
-if (mr->subpage) {
+if (have_sub_page) {
 subpage_t *subpage = container_of(mr, subpage_t, iomem);
 object_unref(OBJECT(>iomem));
 g_free(subpage);
-- 
1.8.3.1

[Qemu-devel] [Bug 1054558] Re: 1366x768 resolution missing

[Bug Watch Updater]

** Changed in: debian
   Status: New => Confirmed

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

Title:
  1366x768 resolution missing

Status in QEMU:
  New
Status in Debian:
  Confirmed

Bug description:
  I use ArchLinux with QEMU 1.2.0.
  I found that 1366x768 resolution is missing, even if I use -vga std or -vga 
vmware.
  I think that it is necessary to patch it into the source.
  Also, why not add a command-line option to specify custom resolutions without 
patching the source? (I know that VirtualBox has a hidden option to add any 
resolution.)

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

[Qemu-devel] [PULL 3/5] hw/ppc/ppc405_boards: Fix infinite recursion by converting taihu_cpld from old_mmio

[David Gibson]

From: Peter Maydell 

The taihu_cpld_writel() function had an obvious typo that meant that
if it was ever called it would go into an infinite recursion. Newer
versions of clang will detect and warn about this:
  hw/ppc/ppc405_boards.c:481:1: warning: all paths through this function will 
call itself [-Winfinite-recursion]

Fix this by converting taihu_cpld from the legacy old_mmio accessors
to new-style ones, with an impl {} declaration to cause the core
memory code to do the splitting of 16 bit and 32 bit accesses into
multiple 8-bit accesses.

Signed-off-by: Peter Maydell 
Reviewed-by: Paolo Bonzini 
Signed-off-by: David Gibson 
---
 hw/ppc/ppc405_boards.c | 52 --
 1 file changed, 8 insertions(+), 44 deletions(-)

diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index ec87587..31bc186 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -408,7 +408,7 @@ struct taihu_cpld_t {
 uint8_t reg1;
 };
 
-static uint32_t taihu_cpld_readb (void *opaque, hwaddr addr)
+static uint64_t taihu_cpld_read(void *opaque, hwaddr addr, unsigned size)
 {
 taihu_cpld_t *cpld;
 uint32_t ret;
@@ -429,8 +429,8 @@ static uint32_t taihu_cpld_readb (void *opaque, hwaddr addr)
 return ret;
 }
 
-static void taihu_cpld_writeb (void *opaque,
-   hwaddr addr, uint32_t value)
+static void taihu_cpld_write(void *opaque, hwaddr addr,
+ uint64_t value, unsigned size)
 {
 taihu_cpld_t *cpld;
 
@@ -447,48 +447,12 @@ static void taihu_cpld_writeb (void *opaque,
 }
 }
 
-static uint32_t taihu_cpld_readw (void *opaque, hwaddr addr)
-{
-uint32_t ret;
-
-ret = taihu_cpld_readb(opaque, addr) << 8;
-ret |= taihu_cpld_readb(opaque, addr + 1);
-
-return ret;
-}
-
-static void taihu_cpld_writew (void *opaque,
-   hwaddr addr, uint32_t value)
-{
-taihu_cpld_writeb(opaque, addr, (value >> 8) & 0xFF);
-taihu_cpld_writeb(opaque, addr + 1, value & 0xFF);
-}
-
-static uint32_t taihu_cpld_readl (void *opaque, hwaddr addr)
-{
-uint32_t ret;
-
-ret = taihu_cpld_readb(opaque, addr) << 24;
-ret |= taihu_cpld_readb(opaque, addr + 1) << 16;
-ret |= taihu_cpld_readb(opaque, addr + 2) << 8;
-ret |= taihu_cpld_readb(opaque, addr + 3);
-
-return ret;
-}
-
-static void taihu_cpld_writel (void *opaque,
-   hwaddr addr, uint32_t value)
-{
-taihu_cpld_writel(opaque, addr, (value >> 24) & 0xFF);
-taihu_cpld_writel(opaque, addr + 1, (value >> 16) & 0xFF);
-taihu_cpld_writel(opaque, addr + 2, (value >> 8) & 0xFF);
-taihu_cpld_writeb(opaque, addr + 3, value & 0xFF);
-}
-
 static const MemoryRegionOps taihu_cpld_ops = {
-.old_mmio = {
-.read = { taihu_cpld_readb, taihu_cpld_readw, taihu_cpld_readl, },
-.write = { taihu_cpld_writeb, taihu_cpld_writew, taihu_cpld_writel, },
+.read = taihu_cpld_read,
+.write = taihu_cpld_write,
+.impl = {
+.min_access_size = 1,
+.max_access_size = 1,
 },
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
-- 
2.5.0

[Qemu-devel] [PULL 4/5] target-ppc: Move the FPSCR bit update macros to cpu.h

[David Gibson]

From: Madhavan Srinivasan 

Move the FPSCR bit update macros defined in dfp_helper
to cpu.h. This way, fpu_helper functions can also use them

Signed-off-by: Madhavan Srinivasan 
Signed-off-by: David Gibson 
---
 target-ppc/cpu.h| 21 +
 target-ppc/dfp_helper.c | 21 -
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 31c6fee..9706000 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -684,6 +684,27 @@ enum {
 #define fpscr_eex (((env->fpscr) >> FPSCR_XX) & ((env->fpscr) >> FPSCR_XE) &  \
0x1F)
 
+#define FP_FX  (1ull << FPSCR_FX)
+#define FP_FEX (1ull << FPSCR_FEX)
+#define FP_OX  (1ull << FPSCR_OX)
+#define FP_OE  (1ull << FPSCR_OE)
+#define FP_UX  (1ull << FPSCR_UX)
+#define FP_UE  (1ull << FPSCR_UE)
+#define FP_XX  (1ull << FPSCR_XX)
+#define FP_XE  (1ull << FPSCR_XE)
+#define FP_ZX  (1ull << FPSCR_ZX)
+#define FP_ZE  (1ull << FPSCR_ZE)
+#define FP_VX  (1ull << FPSCR_VX)
+#define FP_VXSNAN  (1ull << FPSCR_VXSNAN)
+#define FP_VXISI   (1ull << FPSCR_VXISI)
+#define FP_VXIMZ   (1ull << FPSCR_VXIMZ)
+#define FP_VXZDZ   (1ull << FPSCR_VXZDZ)
+#define FP_VXIDI   (1ull << FPSCR_VXIDI)
+#define FP_VXVC(1ull << FPSCR_VXVC)
+#define FP_VXCVI   (1ull << FPSCR_VXCVI)
+#define FP_VE  (1ull << FPSCR_VE)
+#define FP_FI  (1ull << FPSCR_FI)
+
 /*/
 /* Vector status and control register */
 #define VSCR_NJ16 /* Vector non-java */
diff --git a/target-ppc/dfp_helper.c b/target-ppc/dfp_helper.c
index 49820bf..451e434 100644
--- a/target-ppc/dfp_helper.c
+++ b/target-ppc/dfp_helper.c
@@ -170,27 +170,6 @@ static void dfp_prepare_decimal128(struct PPC_DFP *dfp, 
uint64_t *a,
 }
 }
 
-#define FP_FX   (1ull << FPSCR_FX)
-#define FP_FEX  (1ull << FPSCR_FEX)
-#define FP_OX   (1ull << FPSCR_OX)
-#define FP_OE   (1ull << FPSCR_OE)
-#define FP_UX   (1ull << FPSCR_UX)
-#define FP_UE   (1ull << FPSCR_UE)
-#define FP_XX   (1ull << FPSCR_XX)
-#define FP_XE   (1ull << FPSCR_XE)
-#define FP_ZX   (1ull << FPSCR_ZX)
-#define FP_ZE   (1ull << FPSCR_ZE)
-#define FP_VX   (1ull << FPSCR_VX)
-#define FP_VXSNAN   (1ull << FPSCR_VXSNAN)
-#define FP_VXISI(1ull << FPSCR_VXISI)
-#define FP_VXIMZ(1ull << FPSCR_VXIMZ)
-#define FP_VXZDZ(1ull << FPSCR_VXZDZ)
-#define FP_VXIDI(1ull << FPSCR_VXIDI)
-#define FP_VXVC (1ull << FPSCR_VXVC)
-#define FP_VXCVI(1ull << FPSCR_VXCVI)
-#define FP_VE   (1ull << FPSCR_VE)
-#define FP_FI   (1ull << FPSCR_FI)
-
 static void dfp_set_FPSCR_flag(struct PPC_DFP *dfp, uint64_t flag,
 uint64_t enabled)
 {
-- 
2.5.0

[Qemu-devel] [PULL 0/5] ppc-for-2.5 queue 20151130

[David Gibson]

The following changes since commit 714487515dbe0c65d5904251e796cd3a5b3579fb:

  Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into 
staging (2015-11-27 10:44:42 +)

are available in the git repository at:

  git://github.com/dgibson/qemu.git tags/ppc-for-2.5-20151130

for you to fetch changes up to 7624789234cd63b671bce1b49b93b0b1c00ea407:

  target-ppc/fpu_helper: fix FPSCR_FX bit shift operation (2015-11-30 19:39:01 
+1100)


ppc patch queue for qemu-2.5 20151130

target-ppc and related bugfix patches for qemu-2.5

I don't have the facilities to test the Macintosh and BookE related
patches.  I've sanity checked them (inspection + make check), but I'm
otherwise relying on the submitters.


Hervé Poussineau (1):
  mac_dbdma: always initialize channel field in DBDMA_channel

Madhavan Srinivasan (2):
  target-ppc: Move the FPSCR bit update macros to cpu.h
  target-ppc/fpu_helper: fix FPSCR_FX bit shift operation

Peter Maydell (1):
  hw/ppc/ppc405_boards: Fix infinite recursion by converting taihu_cpld 
from old_mmio

Thomas Huth (1):
  hw/ppc/spapr: Remove duplicated "pseries" alias

 hw/misc/macio/mac_dbdma.c |  2 +-
 hw/ppc/ppc405_boards.c| 52 ---
 hw/ppc/spapr.c|  2 --
 target-ppc/cpu.h  | 21 +++
 target-ppc/dfp_helper.c   | 21 ---
 target-ppc/fpu_helper.c   | 22 ++--
 6 files changed, 41 insertions(+), 79 deletions(-)

Re: [Qemu-devel] [PATCH v8 0/5] implement vNVDIMM

[Stefan Hajnoczi]

On Mon, Nov 16, 2015 at 06:50:58PM +0800, Xiao Guangrong wrote:
> This patchset can be found at:
>   https://github.com/xiaogr/qemu.git nvdimm-v8
> 
> It is based on pci branch on Michael's tree and the top commit is:
> commit e3a4e177d9 (migration/ram: fix build on 32 bit hosts).
> 
> Changelog in v8:
> We split the long patch series into the small parts, as you see now, this
> is the first part which enables NVDIMM without label data support.
> 
> The command line has been changed because some patches simplifying the
> things have not been included into this series, you should specify the
> file size exactly using the parameters as follows:
>memory-backend-file,id=mem1,share,mem-path=/tmp/nvdimm1,size=10G \
>-device nvdimm,memdev=mem1,id=nv1
> 
> Changelog in v7:
> - changes from Vladimir Sementsov-Ogievskiy's comments:
>   1) let gethugepagesize() realize if fstat is failed instead of get
>  normal page size
>   2) rename  open_file_path to open_ram_file_path
>   3) better log the error message by using error_setg_errno
>   4) update commit in the commit log to explain hugepage detection on
>  Windows
> 
> - changes from Eduardo Habkost's comments:
>   1) use 'Error**' to collect error message for qemu_file_get_page_size()
>   2) move gethugepagesize() replacement to the same patch to make it
>  better for review
>   3) introduce qemu_get_file_size to unity the code with raw_getlength()
> 
> - changes from Stefan's comments:
>   1) check the memory region is large enough to contain DSM output
>  buffer
> 
> - changes from Eric Blake's comments:
>   1) update the shell command in the commit log to generate the patch
>  which drops 'pc-dimm' prefix
>   
> - others:
>   pick up Reviewed-by from Stefan, Vladimir Sementsov-Ogievskiy, and
>   Eric Blake.
> 
> Changelog in v6:
> - changes from Stefan's comments:
>   1) fix code style of struct naming by CamelCase way
>   2) fix offset + length overflow when read/write label data
>   3) compile hw/acpi/nvdimm.c for per target so that TARGET_PAGE_SIZE can
>  be used to replace getpagesize()
> 
> Changelog in v5:
> - changes from Michael's comments:
>   1) prefix nvdimm_ to everything in NVDIMM source files
>   2) make parsing _DSM Arg3 more clear
>   3) comment style fix
>   5) drop single used definition
>   6) fix dirty dsm buffer lost due to memory write happened on host
>   7) check dsm buffer if it is big enough to contain input data
>   8) use build_append_int_noprefix to store single value to GArray
> 
> - changes from Michael's and Igor's comments:
>   1) introduce 'nvdimm-support' parameter to control nvdimm
>  enablement and it is disabled for 2.4 and its earlier versions
>  to make live migration compatible
>   2) only reserve 1 RAM page and 4 bytes IO Port for NVDIMM ACPI
>  virtualization
> 
> - changes from Stefan's comments:
>   1) do endian adjustment for the buffer length
> 
> - changes from Bharata B Rao's comments:
>   1) fix compile on ppc
> 
> - others:
>   1) the buffer length is directly got from IO read rather than got
>  from dsm memory
>   2) fix dirty label data lost due to memory write happened on host
> 
> Changelog in v4:
> - changes from Michael's comments:
>   1) show the message, "Memory is not allocated from HugeTlbfs", if file
>  based memory is not allocated from hugetlbfs.
>   2) introduce function, acpi_get_nvdimm_state(), to get NVDIMMState
>  from Machine.
>   3) statically define UUID and make its operation more clear
>   4) use GArray to build device structures to avoid potential buffer
>  overflow
>   4) improve comments in the code
>   5) improve code style
> 
> - changes from Igor's comments:
>   1) add NVDIMM ACPI spec document
>   2) use serialized method to avoid Mutex
>   3) move NVDIMM ACPI's code to hw/acpi/nvdimm.c
>   4) introduce a common ASL method used by _DSM for all devices to reduce
>  ACPI size
>   5) handle UUID in ACPI AML code. BTW, i'd keep handling revision in QEMU
>  it's better to upgrade QEMU to support Rev2 in the future
> 
> - changes from Stefan's comments:
>   1) copy input data from DSM memory to local buffer to avoid potential
>  issues as DSM memory is visible to guest. Output data is handled
>  in a similar way
> 
> - changes from Dan's comments:
>   1) drop static namespace as Linux has already supported label-less
>  nvdimm devices
> 
> - changes from Vladimir's comments:
>   1) print better message, "failed to get file size for %s, can't create
>  backend on it", if any file operation filed to obtain file size
> 
> - others:
>   create a git repo on github.com for better review/test
> 
> Also, thanks for Eric Blake's review on QAPI's side.
> 
> Thank all of you to review this patchset.
> 
> 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.
> - changes from Igor's comments:
>   

Re: [Qemu-devel] [PATCH v2 06/21] block: Exclude nested options only for children in append_open_options()

[Kevin Wolf]

Am 27.11.2015 um 18:58 hat Max Reitz geschrieben:
> On 23.11.2015 16:59, Kevin Wolf wrote:
> > Some drivers have nested options (e.g. blkdebug rule arrays), which
> > don't belong to a child node and shouldn't be removed. Don't remove all
> > options with "." in their name, but check for the complete prefixes of
> > actually existing child nodes.
> > 
> > Signed-off-by: Kevin Wolf 
> > ---
> >  block.c   | 19 +++
> >  include/block/block_int.h |  1 +
> >  2 files changed, 16 insertions(+), 4 deletions(-)
> 
> Thanks, now I don't need to fix it myself. :-)
> 
> (I would have had to do that for an in-work series of mine)
> 
> > diff --git a/block.c b/block.c
> > index 23d9e10..02125e2 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -1101,11 +1101,13 @@ static int bdrv_fill_options(QDict **options, const 
> > char **pfilename,
> >  
> >  static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
> >  BlockDriverState *child_bs,
> > +const char *child_name,
> >  const BdrvChildRole *child_role)
> >  {
> >  BdrvChild *child = g_new(BdrvChild, 1);
> >  *child = (BdrvChild) {
> >  .bs = child_bs,
> > +.name   = child_name,
> >  .role   = child_role,
> >  };
> >  
> > @@ -1165,7 +1167,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs, 
> > BlockDriverState *backing_hd)
> >  bs->backing = NULL;
> >  goto out;
> >  }
> > -bs->backing = bdrv_attach_child(bs, backing_hd, _backing);
> > +bs->backing = bdrv_attach_child(bs, backing_hd, "backing", 
> > _backing);
> >  bs->open_flags &= ~BDRV_O_NO_BACKING;
> >  pstrcpy(bs->backing_file, sizeof(bs->backing_file), 
> > backing_hd->filename);
> >  pstrcpy(bs->backing_format, sizeof(bs->backing_format),
> > @@ -1322,7 +1324,7 @@ BdrvChild *bdrv_open_child(const char *filename,
> >  goto done;
> >  }
> >  
> > -c = bdrv_attach_child(parent, bs, child_role);
> > +c = bdrv_attach_child(parent, bs, bdref_key, child_role);
> >  
> >  done:
> >  qdict_del(options, bdref_key);
> > @@ -3952,13 +3954,22 @@ static bool append_open_options(QDict *d, 
> > BlockDriverState *bs)
> >  {
> >  const QDictEntry *entry;
> >  QemuOptDesc *desc;
> > +BdrvChild *child;
> >  bool found_any = false;
> > +const char *p;
> >  
> >  for (entry = qdict_first(bs->options); entry;
> >   entry = qdict_next(bs->options, entry))
> >  {
> > -/* Only take options for this level */
> > -if (strchr(qdict_entry_key(entry), '.')) {
> > +/* Exclude options for children */
> > +QLIST_FOREACH(child, >children, next) {
> > +if (strstart(qdict_entry_key(entry), child->name, )
> > +&& (!*p || *p == '.'))
> > +{
> > +break;
> > +}
> > +}
> > +if (child) {
> >  continue;
> >  }
> >  
> 
> A good general solution, but I think bdrv_refresh_filename() may be bad
> enough to break with general solutions. ;-)
> 
> bdrv_refresh_filename() only considers "file" and "backing" (actually,
> it only supports "file" for now, I'm working on "backing", though). The
> only drivers with other children are quorum, blkdebug, blkverify and
> VMDK. The former three have their own implementation of
> bdrv_refresh_filename(), so they don't use append_open_options() at all.
> The latter, however, (VMDK) does not.
> 
> This change to append_open_options results in the extent.%d options
> simply being omitted altogether because bdrv_refresh_filename() does not
> fetch them. Before, they were included in the VMDK BDS's options, which
> is not ideal but works more or less.

Are you sure? As far as I can tell, this patch should only keep options
that were previously removed, but not remove options that were
previously kept (with the exception of direct use of child names, which
I added here to address your review comments for v1).

Specifically for "extents.%d", this is a child name and is therefore
omitted. However, it contains a '.', so it was already removed without
this patch.

I'm accepting proof of the contrary in the form of a test case. ;-)

> In order to "fix" this, I see three ways right now:
> 1. Just care about "file" and "backing". bdrv_refresh_filename()
>doesn't support anything else, so that will be fine.
> 2. Implement bdrv_refresh_filename() specifically for VMDK so
>append_open_options() will never have to handle anything but "file"
>and "backing".
> 3. Fix bdrv_refresh_filename() so that it handles all children and not
>just "file" and "backing".
> 
> Since we are shooting for 2.6 anyway (I assume ;-)), I think we should
> go for option 3. This means that this patch is fine, and I'll see to
> fixing bdrv_refresh_filename() (because I'm working on that anyway).

Yes, I agree.

Re: [Qemu-devel] question: about exec/poison.h

[Paolo Bonzini]

On 30/11/2015 04:47, Peter Xu wrote:
> 
> I met one problem when trying to add a new public function in dump.h
> named "dump_state_get_global" and using it in hmp.c.

Don't do that. :)

hmp.c functions should in general use the QMP commands as the base.  In
your case, hmp_info_dump should call qmp_query_dump.

Paolo

Re: [Qemu-devel] question: about exec/poison.h

[Markus Armbruster]

Paolo Bonzini  writes:

> On 30/11/2015 04:47, Peter Xu wrote:
>> 
>> I met one problem when trying to add a new public function in dump.h
>> named "dump_state_get_global" and using it in hmp.c.
>
> Don't do that. :)
>
> hmp.c functions should in general use the QMP commands as the base.  In
> your case, hmp_info_dump should call qmp_query_dump.

Yes.  HMP commands need to be implemented on top of QMP commands, to
avoid a feature gap.  Exception: HMP commands that make no sense for
QMP, like "print" or "cpu".

Re: [Qemu-devel] [PATCH 05/40] virtio: read/write the VirtQueueElement a field at a time

[Fam Zheng]

On Tue, 11/24 19:00, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/virtio/virtio.c | 95 
> --
>  1 file changed, 93 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index fd63206..f5f8108 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -578,14 +578,105 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)
>  void *qemu_get_virtqueue_element(QEMUFile *f, size_t sz)
>  {
>  VirtQueueElement *elem = g_malloc(sz);
> -qemu_get_buffer(f, (uint8_t *)elem, sizeof(VirtQueueElement));
> +bool swap;
> +hwaddr addr[VIRTQUEUE_MAX_SIZE];
> +struct iovec iov[VIRTQUEUE_MAX_SIZE];
> +uint64_t scratch;
> +int i;
> +
> +qemu_get_be32s(f, >index);
> +qemu_get_be32s(f, >out_num);
> +qemu_get_be32s(f, >in_num);
> +
> +swap = (elem->out_num & 0x) || (elem->in_num & 0x);

This is interesting, out_num and in_num are 32 bit numbers but there max values
are both VIRTQUEUE_MAX_SIZE (thanks for explaining this on IRC), so it can be a
clue for the source using a different endianness.

Probably worth a few comments here?

It's a great patch!  Thanks!

Fam

> +if (swap) {
> +bswap32s(>index);
> +bswap32s(>out_num);
> +bswap32s(>in_num);
> +}
> +
> +for (i = 0; i < elem->in_num; i++) {
> +qemu_get_be64s(f, >in_addr[i]);
> +if (swap) {
> +bswap64s(>in_addr[i]);
> +}
> +}
> +if (i < ARRAY_SIZE(addr)) {
> +qemu_get_buffer(f, (uint8_t *)addr, sizeof(addr) - i * 
> sizeof(addr[0]));
> +}
> +
> +for (i = 0; i < elem->out_num; i++) {
> +qemu_get_be64s(f, >out_addr[i]);
> +if (swap) {
> +bswap64s(>out_addr[i]);
> +}
> +}
> +if (i < ARRAY_SIZE(addr)) {
> +qemu_get_buffer(f, (uint8_t *)addr, sizeof(addr) - i * 
> sizeof(addr[0]));
> +}
> +
> +for (i = 0; i < elem->in_num; i++) {
> +(void) qemu_get_be64(f); /* base */
> + qemu_get_be64s(f, ); /* length */
> +if (swap) {
> +bswap64s();
> +}
> + elem->in_sg[i].iov_len = scratch;
> +}
> +if (i < ARRAY_SIZE(iov)) {
> +qemu_get_buffer(f, (uint8_t *)iov, sizeof(iov) - i * sizeof(iov[0]));
> +}
> +
> +for (i = 0; i < elem->out_num; i++) {
> +(void) qemu_get_be64(f); /* base */
> +qemu_get_be64s(f, ); /* length */
> +if (swap) {
> +bswap64s();
> +}
> + elem->out_sg[i].iov_len = scratch;
> +}
> +if (i < ARRAY_SIZE(iov)) {
> +qemu_get_buffer(f, (uint8_t *)iov, sizeof(iov) - i * sizeof(iov[0]));
> +}
> +
>  virtqueue_map(elem);
>  return elem;
>  }
>  
>  void qemu_put_virtqueue_element(QEMUFile *f, VirtQueueElement *elem)
>  {
> -qemu_put_buffer(f, (uint8_t *)elem, sizeof(VirtQueueElement));
> +hwaddr addr[VIRTQUEUE_MAX_SIZE];
> +struct iovec iov[VIRTQUEUE_MAX_SIZE];
> +int i;
> +
> +memset(addr, 0, sizeof(addr));
> +memset(iov, 0, sizeof(iov));
> +
> +qemu_put_be32s(f, >index);
> +qemu_put_be32s(f, >out_num);
> +qemu_put_be32s(f, >in_num);
> +
> +for (i = 0; i < elem->in_num; i++) {
> +qemu_put_be64s(f, >in_addr[i]);
> +}
> +qemu_put_buffer(f, (uint8_t *)addr, sizeof(addr) - i * sizeof(addr[0]));
> +
> +for (i = 0; i < elem->out_num; i++) {
> +qemu_put_be64s(f, >out_addr[i]);
> +}
> +qemu_put_buffer(f, (uint8_t *)addr, sizeof(addr) - i * sizeof(addr[0]));
> +
> +for (i = 0; i < elem->in_num; i++) {
> +qemu_put_be64(f, 0);
> +qemu_put_be64(f, elem->in_sg[i].iov_len);
> +}
> +qemu_put_buffer(f, (uint8_t *)iov, sizeof(iov) - i * sizeof(iov[0]));
> +
> +for (i = 0; i < elem->out_num; i++) {
> +qemu_put_be64(f, 0);
> +qemu_put_be64(f, elem->out_sg[i].iov_len);
> +}
> +qemu_put_buffer(f, (uint8_t *)iov, sizeof(iov) - i * sizeof(iov[0]));
>  }
>  
>  /* virtio device */
> -- 
> 1.8.3.1
> 
> 
> 

Re: [Qemu-devel] question: about exec/poison.h

[Peter Xu]

On Mon, Nov 30, 2015 at 10:12:10AM +0100, Paolo Bonzini wrote:
> 
> 
> On 30/11/2015 04:47, Peter Xu wrote:
> > 
> > I met one problem when trying to add a new public function in dump.h
> > named "dump_state_get_global" and using it in hmp.c.
> 
> Don't do that. :)
> 
> hmp.c functions should in general use the QMP commands as the base.  In
> your case, hmp_info_dump should call qmp_query_dump.

Hi, Paolo,

That becomes a problem only if I do not have "percentage" (or
written/total) in QMP queries, which is suggested in the previous
review message:

https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg06088.html

"""
The percentage is not necessary as part of the QMP return value.  You
can compute it in hmp_info_dump however and print it to HMP only.
"""

So from what I understand from your reply, I could include the
percentage value into "query-dump", right? :)

Actually, I think it would be cooler if we could have them in both
QMP/HMP messages (maybe I would better prefer percentage comparing
to written/total bytes, since it is more directly human
readable).

Thanks!
Peter

> 
> Paolo

[Qemu-devel] [PATCH v5] bt: check struct sizes

[Paolo Bonzini]

See http://permalink.gmane.org/gmane.linux.bluez.kernel/36505.  For historical
reasons these do not use sizeof, and Coverity caught a mistake in
EVT_ENCRYPT_CHANGE_SIZE.

In addition:

- remove status from create_conn_cancel_cp; the "status" field is only
in rp structs.  Note that this means that the OCF_CREATE_CONN_CANCEL
could never have worked (it would have failed the LENGTH_CHECK), but
I am keeping it anyway.

- OCF_READ_LINK_QUALITY similarly could never have worked, but I am
fixing read_link_quality_cp anyway.

- fix inquiry_info which is shorter by one: the kernel has a struct that
is 14 byte long, but not counting the initial num_responses byte which
the kernel parses separately;

- remove extended_inquiry_info altogether, since it's not used and unlike
the other inquiry structs does not have the initial num_responses byte.

Signed-off-by: Paolo Bonzini 
---
 include/hw/bt.h | 21 -
 1 file changed, 4 insertions(+), 17 deletions(-)

diff --git a/include/hw/bt.h b/include/hw/bt.h
index cb2a7e6..c7c7909 100644
--- a/include/hw/bt.h
+++ b/include/hw/bt.h
@@ -504,7 +504,6 @@ typedef struct {
 
 #define OCF_CREATE_CONN_CANCEL 0x0008
 typedef struct {
-uint8_tstatus;
 bdaddr_t   bdaddr;
 } QEMU_PACKED create_conn_cancel_cp;
 #define CREATE_CONN_CANCEL_CP_SIZE 6
@@ -1266,13 +1265,13 @@ typedef struct {
 uint8_tstatus;
 uint16_t   handle;
 } QEMU_PACKED reset_failed_contact_counter_rp;
-#define RESET_FAILED_CONTACT_COUNTER_RP_SIZE 4
+#define RESET_FAILED_CONTACT_COUNTER_RP_SIZE 3
 
 #define OCF_READ_LINK_QUALITY  0x0003
 typedef struct {
 uint16_t   handle;
 } QEMU_PACKED read_link_quality_cp;
-#define READ_LINK_QUALITY_CP_SIZE 4
+#define READ_LINK_QUALITY_CP_SIZE 2
 
 typedef struct {
 uint8_tstatus;
@@ -1332,7 +1331,7 @@ typedef struct {
 uint8_tdev_class[3];
 uint16_t   clock_offset;
 } QEMU_PACKED inquiry_info;
-#define INQUIRY_INFO_SIZE 14
+#define INQUIRY_INFO_SIZE 15
 
 #define EVT_CONN_COMPLETE  0x03
 typedef struct {
@@ -1381,7 +1380,7 @@ typedef struct {
 uint16_t   handle;
 uint8_tencrypt;
 } QEMU_PACKED evt_encrypt_change;
-#define EVT_ENCRYPT_CHANGE_SIZE 5
+#define EVT_ENCRYPT_CHANGE_SIZE 4
 
 #define EVT_CHANGE_CONN_LINK_KEY_COMPLETE  0x09
 typedef struct {
@@ -1629,18 +1628,6 @@ typedef struct {
 } QEMU_PACKED evt_sniff_subrate;
 #define EVT_SNIFF_SUBRATE_SIZE 11
 
-#define EVT_EXTENDED_INQUIRY_RESULT0x2F
-typedef struct {
-bdaddr_t   bdaddr;
-uint8_tpscan_rep_mode;
-uint8_tpscan_period_mode;
-uint8_tdev_class[3];
-uint16_t   clock_offset;
-int8_t rssi;
-uint8_tdata[240];
-} QEMU_PACKED extended_inquiry_info;
-#define EXTENDED_INQUIRY_INFO_SIZE 254
-
 #define EVT_TESTING0xFE
 
 #define EVT_VENDOR 0xFF
-- 
2.5.0

Re: [Qemu-devel] question: about exec/poison.h

[Peter Xu]

On Mon, Nov 30, 2015 at 10:28:08AM +0100, Markus Armbruster wrote:
> Paolo Bonzini  writes:
> 
> > On 30/11/2015 04:47, Peter Xu wrote:
> >> 
> >> I met one problem when trying to add a new public function in dump.h
> >> named "dump_state_get_global" and using it in hmp.c.
> >
> > Don't do that. :)
> >
> > hmp.c functions should in general use the QMP commands as the base.  In
> > your case, hmp_info_dump should call qmp_query_dump.
> 
> Yes.  HMP commands need to be implemented on top of QMP commands, to
> avoid a feature gap.  Exception: HMP commands that make no sense for
> QMP, like "print" or "cpu".

I see. Thanks Markus!

Peter

Re: [Qemu-devel] [PATCH v8 3/5] nvdimm acpi: build ACPI NFIT table

[Michael S. Tsirkin]

On Mon, Nov 16, 2015 at 06:51:01PM +0800, Xiao Guangrong wrote:
> NFIT is defined in ACPI 6.0: 5.2.25 NVDIMM Firmware Interface Table (NFIT)
> 
> Currently, we only support PMEM mode. Each device has 3 structures:
> - SPA structure, defines the PMEM region info
> 
> - MEM DEV structure, it has the @handle which is used to associate specified
>   ACPI NVDIMM  device we will introduce in later patch.
>   Also we can happily ignored the memory device's interleave, the real
>   nvdimm hardware access is hidden behind host
> 
> - DCR structure, it defines vendor ID used to associate specified vendor
>   nvdimm driver. Since we only implement PMEM mode this time, Command
>   window and Data window are not needed
> 
> The NVDIMM functionality is controlled by the parameter, 'nvdimm-support',
> is introduced for PIIX4_PM and ICH9-LPC, it is true on default and it is
> false on 2.4 and its earlier version to keep compatibility

Will need to make it false on 2.5 too.

Isn't there a device that needs to be created for this
to work?  It would be cleaned to just key off
the device presence, then we don't need compat gunk,
and further, people not using it don't get a
bunch of unused AML.


> Signed-off-by: Xiao Guangrong 
> ---
>  default-configs/i386-softmmu.mak   |   1 +
>  default-configs/x86_64-softmmu.mak |   1 +
>  hw/acpi/Makefile.objs  |   1 +
>  hw/acpi/ich9.c |  19 ++
>  hw/acpi/nvdimm.c   | 382 
> +
>  hw/acpi/piix4.c|   4 +
>  hw/i386/acpi-build.c   |   6 +
>  include/hw/acpi/ich9.h |   3 +
>  include/hw/i386/pc.h   |  12 +-
>  include/hw/mem/nvdimm.h|  12 ++
>  10 files changed, 440 insertions(+), 1 deletion(-)
>  create mode 100644 hw/acpi/nvdimm.c
> 
> diff --git a/default-configs/i386-softmmu.mak 
> b/default-configs/i386-softmmu.mak
> index 4c79d3b..53fb517 100644
> --- a/default-configs/i386-softmmu.mak
> +++ b/default-configs/i386-softmmu.mak
> @@ -47,6 +47,7 @@ CONFIG_IOAPIC=y
>  CONFIG_PVPANIC=y
>  CONFIG_MEM_HOTPLUG=y
>  CONFIG_NVDIMM=y
> +CONFIG_ACPI_NVDIMM=y
>  CONFIG_XIO3130=y
>  CONFIG_IOH3420=y
>  CONFIG_I82801B11=y
> diff --git a/default-configs/x86_64-softmmu.mak 
> b/default-configs/x86_64-softmmu.mak
> index e42d2fc..766c27c 100644
> --- a/default-configs/x86_64-softmmu.mak
> +++ b/default-configs/x86_64-softmmu.mak
> @@ -47,6 +47,7 @@ CONFIG_IOAPIC=y
>  CONFIG_PVPANIC=y
>  CONFIG_MEM_HOTPLUG=y
>  CONFIG_NVDIMM=y
> +CONFIG_ACPI_NVDIMM=y
>  CONFIG_XIO3130=y
>  CONFIG_IOH3420=y
>  CONFIG_I82801B11=y
> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> index 7d3230c..095597f 100644
> --- a/hw/acpi/Makefile.objs
> +++ b/hw/acpi/Makefile.objs
> @@ -2,6 +2,7 @@ common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o pcihp.o
>  common-obj-$(CONFIG_ACPI_X86_ICH) += ich9.o tco.o
>  common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
>  common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
> +common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
>  common-obj-$(CONFIG_ACPI) += acpi_interface.o
>  common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
>  common-obj-$(CONFIG_ACPI) += aml-build.o
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index 1c7fcfa..275796f 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -307,6 +307,20 @@ static void ich9_pm_set_memory_hotplug_support(Object 
> *obj, bool value,
>  s->pm.acpi_memory_hotplug.is_enabled = value;
>  }
>  
> +static bool ich9_pm_get_nvdimm_support(Object *obj, Error **errp)
> +{
> +ICH9LPCState *s = ICH9_LPC_DEVICE(obj);
> +
> +return s->pm.nvdimm_acpi_state.is_enabled;
> +}
> +
> +static void ich9_pm_set_nvdimm_support(Object *obj, bool value, Error **errp)
> +{
> +ICH9LPCState *s = ICH9_LPC_DEVICE(obj);
> +
> +s->pm.nvdimm_acpi_state.is_enabled = value;
> +}
> +
>  static void ich9_pm_get_disable_s3(Object *obj, Visitor *v,
> void *opaque, const char *name,
> Error **errp)
> @@ -404,6 +418,7 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs 
> *pm, Error **errp)
>  {
>  static const uint32_t gpe0_len = ICH9_PMIO_GPE0_LEN;
>  pm->acpi_memory_hotplug.is_enabled = true;
> +pm->nvdimm_acpi_state.is_enabled = true;
>  pm->disable_s3 = 0;
>  pm->disable_s4 = 0;
>  pm->s4_val = 2;
> @@ -419,6 +434,10 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs 
> *pm, Error **errp)
>   ich9_pm_get_memory_hotplug_support,
>   ich9_pm_set_memory_hotplug_support,
>   NULL);
> +object_property_add_bool(obj, "nvdimm-support",
> + ich9_pm_get_nvdimm_support,
> + ich9_pm_set_nvdimm_support,
> + NULL);
>  object_property_add(obj, ACPI_PM_PROP_S3_DISABLED, "uint8",
>  

Re: [Qemu-devel] [PATCH v8 4/5] nvdimm acpi: build ACPI nvdimm devices

[Michael S. Tsirkin]

On Mon, Nov 16, 2015 at 06:51:02PM +0800, Xiao Guangrong wrote:
> NVDIMM devices is defined in ACPI 6.0 9.20 NVDIMM Devices
> 
> There is a root device under \_SB and specified NVDIMM devices are under the
> root device. Each NVDIMM device has _ADR which returns its handle used to
> associate MEMDEV structure in NFIT
> 
> Currently, we do not support any function on _DSM, that means, NVDIMM
> label data has not been supported yet
> 
> Signed-off-by: Xiao Guangrong 
> ---
>  hw/acpi/nvdimm.c | 85 
> 
>  1 file changed, 85 insertions(+)
> 
> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> index 98c004d..abe0daa 100644
> --- a/hw/acpi/nvdimm.c
> +++ b/hw/acpi/nvdimm.c
> @@ -367,6 +367,90 @@ static void nvdimm_build_nfit(GSList *device_list, 
> GArray *table_offsets,
>  g_array_free(structures, true);
>  }
>  
> +static void nvdimm_build_common_dsm(Aml *root_dev)
> +{
> +Aml *method, *ifctx, *function;
> +uint8_t byte_list[1];
> +
> +method = aml_method("NCAL", 4);

This "NCAL" needs a define as it's used
in multiple places. It's really just a DSM
implementation, right? Reflect this in the macro
name.

> +{

What's this doing?

> +function = aml_arg(2);
> +
> +/*
> + * function 0 is called to inquire what functions are supported by
> + * OSPM
> + */
> +ifctx = aml_if(aml_equal(function, aml_int(0)));
> +byte_list[0] = 0 /* No function Supported */;
> +aml_append(ifctx, aml_return(aml_buffer(1, byte_list)));
> +aml_append(method, ifctx);
> +
> +/* No function is supported yet. */
> +byte_list[0] = 1 /* Not Supported */;
> +aml_append(method, aml_return(aml_buffer(1, byte_list)));
> +}
> +aml_append(root_dev, method);
> +}
> +
> +static void nvdimm_build_nvdimm_devices(GSList *device_list, Aml *root_dev)
> +{
> +for (; device_list; device_list = device_list->next) {
> +DeviceState *dev = device_list->data;
> +int slot = object_property_get_int(OBJECT(dev), PC_DIMM_SLOT_PROP,
> +   NULL);
> +uint32_t handle = nvdimm_slot_to_handle(slot);
> +Aml *nvdimm_dev, *method;
> +
> +nvdimm_dev = aml_device("NV%02X", slot);
> +aml_append(nvdimm_dev, aml_name_decl("_ADR", aml_int(handle)));
> +
> +method = aml_method("_DSM", 4);
> +{
> +aml_append(method, aml_return(aml_call4("NCAL", aml_arg(0),
> +   aml_arg(1), aml_arg(2), aml_arg(3;
> +}
> +aml_append(nvdimm_dev, method);
> +
> +aml_append(root_dev, nvdimm_dev);
> +}
> +}
> +
> +static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets,
> +  GArray *table_data, GArray *linker)
> +{
> +Aml *ssdt, *sb_scope, *dev, *method;
> +
> +acpi_add_table(table_offsets, table_data);
> +
> +ssdt = init_aml_allocator();
> +acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
> +
> +sb_scope = aml_scope("\\_SB");
> +
> +dev = aml_device("NVDR");
> +aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0012")));

Pls add a comment explaining that ACPI0012 is NVDIMM root device.

Also - this will now appear for all users, e.g.
windows guests will prompt users for a driver.
Not nice if user didn't actually ask for nvdimm.

A simple solution is to default this functionality
to off by default.

> +
> +nvdimm_build_common_dsm(dev);
> +method = aml_method("_DSM", 4);
> +{
> +aml_append(method, aml_return(aml_call4("NCAL", aml_arg(0),
> +   aml_arg(1), aml_arg(2), aml_arg(3;
> +}

Some duplication here, move above to a sub-function please.

> +aml_append(dev, method);
> +
> +nvdimm_build_nvdimm_devices(device_list, dev);
> +
> +aml_append(sb_scope, dev);
> +
> +aml_append(ssdt, sb_scope);
> +/* copy AML table into ACPI tables blob and patch header there */
> +g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len);
> +build_header(linker, table_data,
> +(void *)(table_data->data + table_data->len - ssdt->buf->len),
> +"SSDT", ssdt->buf->len, 1, "NVDIMM");
> +free_aml_allocator();
> +}
> +
>  void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
> GArray *linker)
>  {
> @@ -378,5 +462,6 @@ void nvdimm_build_acpi(GArray *table_offsets, GArray 
> *table_data,
>  return;
>  }
>  nvdimm_build_nfit(device_list, table_offsets, table_data, linker);
> +nvdimm_build_ssdt(device_list, table_offsets, table_data, linker);
>  g_slist_free(device_list);
>  }
> -- 
> 1.8.3.1
> 
> --
> 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] [PATCH v2 0/9] KVM: Hyper-V SynIC timers

[Andrey Smetanin]

Per Hyper-V specification (and as required by Hyper-V-aware guests),
SynIC provides 4 per-vCPU timers.  Each timer is programmed via a pair
of MSRs, and signals expiration by delivering a special format message
to the configured SynIC message slot and triggering the corresponding
synthetic interrupt.

Note: as implemented by this patch, all periodic timers are "lazy"
(i.e. if the vCPU wasn't scheduled for more than the timer period the
timer events are lost), regardless of the corresponding configuration
MSR.  If deemed necessary, the "catch up" mode (the timer period is
shortened until the timer catches up) will be implemented later.

The Hyper-V SynIC timers support is required to load winhv.sys
inside Windows guest on which guest VMBus devices depends on.

This patches depends on Hyper-V SynIC patches previosly sent.

Changes v2:
* Hyper-V headers patches split and fixes
* Use remainder to calculate peridic timer expiration time

Signed-off-by: Andrey Smetanin 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Vitaly Kuznetsov 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org

Andrey Smetanin (9):
  drivers/hv: Replace enum hv_message_type by u32
  drivers/hv: Move HV_SYNIC_STIMER_COUNT into Hyper-V UAPI x86 header
  drivers/hv: Move struct hv_message into UAPI Hyper-V x86 header
  drivers/hv: Move struct hv_timer_message_payload into UAPI Hyper-V x86
header
  kvm/x86: Rearrange func's declarations inside Hyper-V header
  kvm/x86: Added Hyper-V vcpu_to_hv_vcpu()/hv_vcpu_to_vcpu() helpers
  kvm/x86: Hyper-V internal helper to read MSR HV_X64_MSR_TIME_REF_COUNT
  kvm/x86: Hyper-V SynIC message slot pending clearing at SINT ack
  kvm/x86: Hyper-V SynIC timers

 arch/x86/include/asm/kvm_host.h|  13 ++
 arch/x86/include/uapi/asm/hyperv.h |  90 ++
 arch/x86/kvm/hyperv.c  | 360 -
 arch/x86/kvm/hyperv.h  |  54 --
 arch/x86/kvm/x86.c |   9 +
 drivers/hv/hv.c|   4 +-
 drivers/hv/hyperv_vmbus.h  |  92 +-
 include/linux/kvm_host.h   |   3 +
 8 files changed, 516 insertions(+), 109 deletions(-)

-- 
2.4.3

[Qemu-devel] [PATCH v2 5/9] kvm/x86: Rearrange func's declarations inside Hyper-V header

[Andrey Smetanin]

This rearrangement places functions declarations together
according to their functionality, so future additions
will be simplier.

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Vitaly Kuznetsov 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
 arch/x86/kvm/hyperv.h | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
index 315af4b..9483d49 100644
--- a/arch/x86/kvm/hyperv.h
+++ b/arch/x86/kvm/hyperv.h
@@ -24,14 +24,6 @@
 #ifndef __ARCH_X86_KVM_HYPERV_H__
 #define __ARCH_X86_KVM_HYPERV_H__
 
-int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host);
-int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);
-bool kvm_hv_hypercall_enabled(struct kvm *kvm);
-int kvm_hv_hypercall(struct kvm_vcpu *vcpu);
-
-int kvm_hv_synic_set_irq(struct kvm *kvm, u32 vcpu_id, u32 sint);
-void kvm_hv_synic_send_eoi(struct kvm_vcpu *vcpu, int vector);
-
 static inline struct kvm_vcpu_hv_synic *vcpu_to_synic(struct kvm_vcpu *vcpu)
 {
return >arch.hyperv.synic;
@@ -46,10 +38,18 @@ static inline struct kvm_vcpu *synic_to_vcpu(struct 
kvm_vcpu_hv_synic *synic)
arch = container_of(hv, struct kvm_vcpu_arch, hyperv);
return container_of(arch, struct kvm_vcpu, arch);
 }
-void kvm_hv_irq_routing_update(struct kvm *kvm);
 
-void kvm_hv_vcpu_init(struct kvm_vcpu *vcpu);
+int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host);
+int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);
+
+bool kvm_hv_hypercall_enabled(struct kvm *kvm);
+int kvm_hv_hypercall(struct kvm_vcpu *vcpu);
 
+void kvm_hv_irq_routing_update(struct kvm *kvm);
+int kvm_hv_synic_set_irq(struct kvm *kvm, u32 vcpu_id, u32 sint);
+void kvm_hv_synic_send_eoi(struct kvm_vcpu *vcpu, int vector);
 int kvm_hv_activate_synic(struct kvm_vcpu *vcpu);
 
+void kvm_hv_vcpu_init(struct kvm_vcpu *vcpu);
+
 #endif
-- 
2.4.3

[Qemu-devel] [PATCH v2 7/9] kvm/x86: Hyper-V internal helper to read MSR HV_X64_MSR_TIME_REF_COUNT

[Andrey Smetanin]

This helper will be used also in Hyper-V SynIC timers implementation.

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Vitaly Kuznetsov 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
 arch/x86/kvm/hyperv.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 41869a9..9958926 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -335,6 +335,11 @@ static void synic_init(struct kvm_vcpu_hv_synic *synic)
}
 }
 
+static u64 get_time_ref_counter(struct kvm *kvm)
+{
+   return div_u64(get_kernel_ns() + kvm->arch.kvmclock_offset, 100);
+}
+
 void kvm_hv_vcpu_init(struct kvm_vcpu *vcpu)
 {
synic_init(vcpu_to_synic(vcpu));
@@ -576,11 +581,9 @@ static int kvm_hv_get_msr_pw(struct kvm_vcpu *vcpu, u32 
msr, u64 *pdata)
case HV_X64_MSR_HYPERCALL:
data = hv->hv_hypercall;
break;
-   case HV_X64_MSR_TIME_REF_COUNT: {
-   data =
-div_u64(get_kernel_ns() + kvm->arch.kvmclock_offset, 100);
+   case HV_X64_MSR_TIME_REF_COUNT:
+   data = get_time_ref_counter(kvm);
break;
-   }
case HV_X64_MSR_REFERENCE_TSC:
data = hv->hv_tsc_page;
break;
-- 
2.4.3

[Qemu-devel] [PATCH v3] vmxnet3: silence warning

[Michael S. Tsirkin]

vmxnet3 always produces a warning under qtest.

This is not a user error, don't warn.

Suggested-by: Paolo Bonzini 
Signed-off-by: Michael S. Tsirkin 
---

Now for real.

 hw/net/vmxnet3.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 5e3a233..37373e5 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2015,7 +2015,6 @@ static bool vmxnet3_peer_has_vnet_hdr(VMXNET3State *s)
 return true;
 }
 
-VMW_WRPRN("Peer has no virtio extension. Task offloads will be emulated.");
 return false;
 }
 
-- 
MST

Re: [Qemu-devel] [PATCH v9] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host

[Kevin Wolf]

Am 30.11.2015 um 17:19 hat Eric Blake geschrieben:
> On 11/27/2015 12:35 PM, Programmingkid wrote:
> 
> >> Unusual indentation; more typical is:
> >>
> >> | static kern_return_t FindEjectableOpticalMedia(io_iterator_t
> >> *mediaIterator,
> >> | char *mediatType)
> > 
> > I agree. I wanted the second long to be right justified with the 80 
> > character line count.
> 
> No.  We don't right-justify code to 80 columns.  That's not how it is
> done.  Trying to do it just makes you look like the proverbial 'kid' in
> your pseudonym, rather than an adult to be taken seriously.
> 
> Really, PLEASE follow the indentation patterns of the rest of the code
> base - where continued lines are left-justified to be underneath the
> character after (, and NOT right-justified to 80 columns.  Violating
> style doesn't make your code invalid, but does make your patches less
> likely to be applied.
> 
> 
> >>> +/* If you found a match, leave the loop */
> >>> +if (*mediaIterator != 0) {
> >>> +DPRINTF("Matching using %s\n", matching_array[index]);
> >>> +snprintf(mediaType, strlen(matching_array[index])+1, "%s",
> >>
> >> Spaces around binary '+'.
> > 
> > What's wrong with no spaces around the plus sign?
> 
> Again, the prevailing conventions in the code base is that you put
> spaces around every binary operator.  Yes, there is existing old code
> that does not meet the conventions, but it is not an excuse to add new
> code that is gratuitously different.
> 
> > 
> >>
> >>> +/* if a working partition on the device was not found */
> >>> +if (partition_found == false) {
> >>> +error_setg(errp, "Error: Failed to find a working partition on "
> >>> + 
> >>> "disc!\n");
> >>
> >> and I already pointed out on v8 that this is not the correct usage of
> >> error_setg().  So, here's hoping v10 addresses the comments here and
> >> elsewhere.
> > 
> > Kevin Wolf wanted it this way. What would you do instead?
> 
> Keven and I both want you to use error_setg(), but to use it correctly -
> and the correct way is to NOT supply a trailing \n.

Nor leading "Error:", for that matter.

Kevin


pgpLueWs7s0Bw.pgp
Description: PGP signature

[Qemu-devel] [PATCH v2 4/9] drivers/hv: Move struct hv_timer_message_payload into UAPI Hyper-V x86 header

[Andrey Smetanin]

This struct is required for Hyper-V SynIC timers implementation inside KVM
and for upcoming Hyper-V VMBus support by userspace(QEMU). So place it into
Hyper-V UAPI header.

Signed-off-by: Andrey Smetanin 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Vitaly Kuznetsov 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org

---
 arch/x86/include/uapi/asm/hyperv.h | 8 
 drivers/hv/hyperv_vmbus.h  | 9 -
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/uapi/asm/hyperv.h 
b/arch/x86/include/uapi/asm/hyperv.h
index 76e503d..42278f8 100644
--- a/arch/x86/include/uapi/asm/hyperv.h
+++ b/arch/x86/include/uapi/asm/hyperv.h
@@ -345,4 +345,12 @@ struct hv_message_page {
struct hv_message sint_message[HV_SYNIC_SINT_COUNT];
 };
 
+/* Define timer message payload structure. */
+struct hv_timer_message_payload {
+   __u32 timer_index;
+   __u32 reserved;
+   __u64 expiration_time;  /* When the timer expired */
+   __u64 delivery_time;/* When the message was delivered */
+};
+
 #endif
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 3f3756b..db60080 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -136,15 +136,6 @@ union hv_timer_config {
};
 };
 
-
-/* Define timer message payload structure. */
-struct hv_timer_message_payload {
-   u32 timer_index;
-   u32 reserved;
-   u64 expiration_time;/* When the timer expired */
-   u64 delivery_time;  /* When the message was delivered */
-};
-
 /* Define the number of message buffers associated with each port. */
 #define HV_PORT_MESSAGE_BUFFER_COUNT   (16)
 
-- 
2.4.3

[Qemu-devel] [PATCH v2 9/9] kvm/x86: Hyper-V SynIC timers

[Andrey Smetanin]

Per Hyper-V specification (and as required by Hyper-V-aware guests),
SynIC provides 4 per-vCPU timers.  Each timer is programmed via a pair
of MSRs, and signals expiration by delivering a special format message
to the configured SynIC message slot and triggering the corresponding
synthetic interrupt.

Note: as implemented by this patch, all periodic timers are "lazy"
(i.e. if the vCPU wasn't scheduled for more than the timer period the
timer events are lost), regardless of the corresponding configuration
MSR.  If deemed necessary, the "catch up" mode (the timer period is
shortened until the timer catches up) will be implemented later.

Changes v2:
* Use remainder to calculate periodic timer expiration time

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Vitaly Kuznetsov 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
 arch/x86/include/asm/kvm_host.h|  13 ++
 arch/x86/include/uapi/asm/hyperv.h |   6 +
 arch/x86/kvm/hyperv.c  | 318 -
 arch/x86/kvm/hyperv.h  |  24 +++
 arch/x86/kvm/x86.c |   9 ++
 include/linux/kvm_host.h   |   1 +
 6 files changed, 368 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 8140077..a7c8987 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -379,6 +379,17 @@ struct kvm_mtrr {
struct list_head head;
 };
 
+/* Hyper-V SynIC timer */
+struct kvm_vcpu_hv_stimer {
+   struct hrtimer timer;
+   int index;
+   u64 config;
+   u64 count;
+   u64 exp_time;
+   struct hv_message msg;
+   bool msg_pending;
+};
+
 /* Hyper-V synthetic interrupt controller (SynIC)*/
 struct kvm_vcpu_hv_synic {
u64 version;
@@ -398,6 +409,8 @@ struct kvm_vcpu_hv {
s64 runtime_offset;
struct kvm_vcpu_hv_synic synic;
struct kvm_hyperv_exit exit;
+   struct kvm_vcpu_hv_stimer stimer[HV_SYNIC_STIMER_COUNT];
+   DECLARE_BITMAP(stimer_pending_bitmap, HV_SYNIC_STIMER_COUNT);
 };
 
 struct kvm_vcpu_arch {
diff --git a/arch/x86/include/uapi/asm/hyperv.h 
b/arch/x86/include/uapi/asm/hyperv.h
index 42278f8..71fce3f 100644
--- a/arch/x86/include/uapi/asm/hyperv.h
+++ b/arch/x86/include/uapi/asm/hyperv.h
@@ -353,4 +353,10 @@ struct hv_timer_message_payload {
__u64 delivery_time;/* When the message was delivered */
 };
 
+#define HV_STIMER_ENABLE   (1ULL << 0)
+#define HV_STIMER_PERIODIC (1ULL << 1)
+#define HV_STIMER_LAZY (1ULL << 2)
+#define HV_STIMER_AUTOENABLE   (1ULL << 3)
+#define HV_STIMER_SINT(config) (__u8)(((config) >> 16) & 0x0F)
+
 #endif
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 6412b6b..8ff8829 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -147,15 +147,32 @@ static void kvm_hv_notify_acked_sint(struct kvm_vcpu 
*vcpu, u32 sint)
 {
struct kvm *kvm = vcpu->kvm;
struct kvm_vcpu_hv_synic *synic = vcpu_to_synic(vcpu);
-   int gsi, idx;
+   struct kvm_vcpu_hv *hv_vcpu = vcpu_to_hv_vcpu(vcpu);
+   struct kvm_vcpu_hv_stimer *stimer;
+   int gsi, idx, stimers_pending;
 
vcpu_debug(vcpu, "Hyper-V SynIC acked sint %d\n", sint);
 
if (synic->msg_page & HV_SYNIC_SIMP_ENABLE)
synic_clear_sint_msg_pending(synic, sint);
 
+   /* Try to deliver pending Hyper-V SynIC timers messages */
+   stimers_pending = 0;
+   for (idx = 0; idx < ARRAY_SIZE(hv_vcpu->stimer); idx++) {
+   stimer = _vcpu->stimer[idx];
+   if (stimer->msg_pending &&
+   (stimer->config & HV_STIMER_ENABLE) &&
+   HV_STIMER_SINT(stimer->config) == sint) {
+   set_bit(stimer->index,
+   hv_vcpu->stimer_pending_bitmap);
+   stimers_pending++;
+   }
+   }
+   if (stimers_pending)
+   kvm_make_request(KVM_REQ_HV_STIMER, vcpu);
+
idx = srcu_read_lock(>irq_srcu);
-   gsi = atomic_read(_to_synic(vcpu)->sint_to_gsi[sint]);
+   gsi = atomic_read(>sint_to_gsi[sint]);
if (gsi != -1)
kvm_notify_acked_gsi(kvm, gsi);
srcu_read_unlock(>irq_srcu, idx);
@@ -371,9 +388,268 @@ static u64 get_time_ref_counter(struct kvm *kvm)
return div_u64(get_kernel_ns() + kvm->arch.kvmclock_offset, 100);
 }
 
+static void stimer_mark_expired(struct kvm_vcpu_hv_stimer *stimer,
+   bool vcpu_kick)
+{
+   struct kvm_vcpu *vcpu = stimer_to_vcpu(stimer);
+
+   set_bit(stimer->index,
+   

Re: [Qemu-devel] [PATCH v9] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host

[Programmingkid]

On Nov 30, 2015, at 11:26 AM, Kevin Wolf wrote:

> Am 30.11.2015 um 17:19 hat Eric Blake geschrieben:
>> On 11/27/2015 12:35 PM, Programmingkid wrote:
>> 
 Unusual indentation; more typical is:
 
 | static kern_return_t FindEjectableOpticalMedia(io_iterator_t
 *mediaIterator,
 | char *mediatType)
>>> 
>>> I agree. I wanted the second long to be right justified with the 80 
>>> character line count.
>> 
>> No.  We don't right-justify code to 80 columns.  That's not how it is
>> done.  Trying to do it just makes you look like the proverbial 'kid' in
>> your pseudonym, rather than an adult to be taken seriously.
>> 
>> Really, PLEASE follow the indentation patterns of the rest of the code
>> base - where continued lines are left-justified to be underneath the
>> character after (, and NOT right-justified to 80 columns.  Violating
>> style doesn't make your code invalid, but does make your patches less
>> likely to be applied.
>> 
>> 
> +/* If you found a match, leave the loop */
> +if (*mediaIterator != 0) {
> +DPRINTF("Matching using %s\n", matching_array[index]);
> +snprintf(mediaType, strlen(matching_array[index])+1, "%s",
 
 Spaces around binary '+'.
>>> 
>>> What's wrong with no spaces around the plus sign?
>> 
>> Again, the prevailing conventions in the code base is that you put
>> spaces around every binary operator.  Yes, there is existing old code
>> that does not meet the conventions, but it is not an excuse to add new
>> code that is gratuitously different.
>> 
>>> 
 
> +/* if a working partition on the device was not found */
> +if (partition_found == false) {
> +error_setg(errp, "Error: Failed to find a working partition on "
> + 
> "disc!\n");
 
 and I already pointed out on v8 that this is not the correct usage of
 error_setg().  So, here's hoping v10 addresses the comments here and
 elsewhere.
>>> 
>>> Kevin Wolf wanted it this way. What would you do instead?
>> 
>> Keven and I both want you to use error_setg(), but to use it correctly -
>> and the correct way is to NOT supply a trailing \n.
> 
> Nor leading "Error:", for that matter.

I just think that using "Error" does communicate the fact that something is 
wrong
a lot better than just printing the message. 

[Qemu-devel] [PATCH for-2.5 v4 2/4] vhost-user-test: use unix port for migration

[marcandre . lureau]

From: Marc-André Lureau 

TCP port 1234 may be used by another process concurrently. Instead use a
temporary unix socket.

Signed-off-by: Marc-André Lureau 
---
 tests/vhost-user-test.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index 261f4b7..29205ed 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -123,6 +123,7 @@ static VhostUserMsg m __attribute__ ((unused));
 
 typedef struct TestServer {
 gchar *socket_path;
+gchar *mig_path;
 gchar *chr_name;
 CharDriverState *chr;
 int fds_num;
@@ -364,6 +365,7 @@ static TestServer *test_server_new(const gchar *name)
 gchar *chr_path;
 
 server->socket_path = g_strdup_printf("%s/%s.sock", tmpfs, name);
+server->mig_path = g_strdup_printf("%s/%s.mig", tmpfs, name);
 
 chr_path = g_strdup_printf("unix:%s,server,nowait", server->socket_path);
 server->chr_name = g_strdup_printf("chr-%s", name);
@@ -405,6 +407,9 @@ static gboolean _test_server_free(TestServer *server)
 unlink(server->socket_path);
 g_free(server->socket_path);
 
+unlink(server->mig_path);
+g_free(server->mig_path);
+
 g_free(server->chr_name);
 g_free(server);
 
@@ -512,7 +517,7 @@ static void test_migrate(void)
 {
 TestServer *s = test_server_new("src");
 TestServer *dest = test_server_new("dest");
-const char *uri = "tcp:127.0.0.1:1234";
+char *uri = g_strdup_printf("%s%s", "unix:", dest->mig_path);
 QTestState *global = global_qtest, *from, *to;
 GSource *source;
 gchar *cmd;
@@ -583,6 +588,7 @@ static void test_migrate(void)
 test_server_free(dest);
 qtest_quit(from);
 test_server_free(s);
+g_free(uri);
 
 global_qtest = global;
 }
-- 
2.5.0

[Qemu-devel] [PATCH for-2.5 v4 4/4] tests: add vhost-user-test when target is x64 only

[marcandre . lureau]

From: Marc-André Lureau 

check-qtest-x86_64-y is overwritten by the last assignment. Move
vhost-user-test addition after.

Reported-by: Alex Bennée 
Signed-off-by: Marc-André Lureau 
---
 tests/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/Makefile b/tests/Makefile
index 0ef00a1..cf1228b 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -198,13 +198,13 @@ check-qtest-i386-y += tests/pc-cpu-test$(EXESUF)
 check-qtest-i386-y += tests/q35-test$(EXESUF)
 gcov-files-i386-y += hw/pci-host/q35.c
 check-qtest-i386-$(CONFIG_VHOST_NET_TEST_i386) += 
tests/vhost-user-test$(EXESUF)
-ifeq ($(CONFIG_VHOST_NET_TEST_i386),)
-check-qtest-x86_64-$(CONFIG_VHOST_NET_TEST_x86_64) += 
tests/vhost-user-test$(EXESUF)
-endif
 check-qtest-i386-y += tests/test-netfilter$(EXESUF)
 check-qtest-x86_64-y = $(check-qtest-i386-y)
 gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
 gcov-files-x86_64-y = $(subst 
i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y))
+ifeq ($(CONFIG_VHOST_NET_TEST_i386),)
+check-qtest-x86_64-$(CONFIG_VHOST_NET_TEST_x86_64) += 
tests/vhost-user-test$(EXESUF)
+endif
 check-qtest-mips-y = tests/endianness-test$(EXESUF)
 check-qtest-mips64-y = tests/endianness-test$(EXESUF)
 check-qtest-mips64el-y = tests/endianness-test$(EXESUF)
-- 
2.5.0

Re: [Qemu-devel] [PATCH v9] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host

[Eric Blake]

On 11/30/2015 09:38 AM, Programmingkid wrote:

>> +/* if a working partition on the device was not found */
>> +if (partition_found == false) {
>> +error_setg(errp, "Error: Failed to find a working partition on "
>> + 
>> "disc!\n");
>
> and I already pointed out on v8 that this is not the correct usage of
> error_setg().  So, here's hoping v10 addresses the comments here and
> elsewhere.

 Kevin Wolf wanted it this way. What would you do instead?
>>>
>>> Keven and I both want you to use error_setg(), but to use it correctly -
>>> and the correct way is to NOT supply a trailing \n.
>>
>> Nor leading "Error:", for that matter.
> 
> I just think that using "Error" does communicate the fact that something is 
> wrong
> a lot better than just printing the message. 

But error_setg() _already_ provides the context that an error message is
being printed.  The whole point of using wrapper functions is that the
common functionality (like an 'error:' prefix, or '\n' suffix) is done
in the wrapper, not at every call site.  If you were using raw printf(),
then yes, using your own 'Error:' prefix would be appropriate.  But we
aren't using raw printf().  Your use of an 'Error:' prefix is therefore
redundant, and we are trying to convince you that you are using
error_setg() incorrectly.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] mmap-alloc: use same backend for all mappings

[Michael S. Tsirkin]

On Mon, Nov 30, 2015 at 02:46:31PM +0100, Greg Kurz wrote:
> On Mon, 30 Nov 2015 15:06:33 +0200
> "Michael S. Tsirkin"  wrote:
> 
> > On Mon, Nov 30, 2015 at 11:51:57AM +0100, Greg Kurz wrote:
> > > Since commit 8561c9244ddf1122d "exec: allocate PROT_NONE pages on top of 
> > > RAM",
> > > it is no longer possible to back guest RAM with hugepages on ppc64 hosts:
> > > 
> > > mmap(NULL, 285212672, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 
> > > 0x3fff5700
> > > mmap(0x3fff5700, 268435456, PROT_READ|PROT_WRITE, 
> > > MAP_PRIVATE|MAP_FIXED, 19, 0) = -1 EBUSY (Device or resource busy)
> > > 
> > > This is due to a limitation on ppc64 that requires MAP_FIXED mappings to 
> > > have
> > > the same page size as other mappings already present in the same "slice" 
> > > of
> > > virtual address space (Cc'ing Ben for details).
> > 
> > I'd like some details please.
> > What do you mean when you say "same page size" and "slice"?
> > 
> 
> On ppc64, the address space is divided in 256MB-sized segments where all pages
> have the same size. This is a hw limitation IIUC. I don't know if it can be
> fixed and I'll let Ben comment on it.

But it's anonymous memory with PROT_NONE.  There should be no pages there:
just a chunk of virtual memory reserved.

> Hugepage support is implemented using an abstraction of segments called
> "slices". Here's a quote from the related commit changelog in the kernel
> tree:
> 
> commit d0f13e3c20b6fb73ccb467bdca97fa7cf5a574cd
> Author: Benjamin Herrenschmidt 
> Date:   Tue May 8 16:27:27 2007 +1000
> 
> [POWERPC] Introduce address space "slices"
> 
> ...
> 
> The main issues are:
> 
>  - To maintain/keep track of the page size per "segment" (as we can
> only have one page size per segment on powerpc, which are 256MB
> divisions of the address space).
> 
>  - To make sure special mappings stay within their allotted
> "segments" (including MAP_FIXED crap)
> 
>  - To make sure everybody else doesn't mmap/brk/grow_stack into a
> "segment" that is used for a special mapping
> ...
> 
> > > This is exactly what happens
> > > when calling mmap() above: first one uses native host page size (64k) and
> > > second one uses huge page size (16M).
> > > 
> > > To be sure we always have the same page size, let's use the same backend 
> > > for
> > > both calls to mmap(): this is enough to fix the ppc64 issue.
> > > 
> > > This has no effect on RAM based mappings.
> > > 
> > > Signed-off-by: Greg Kurz 
> > > ---
> > > 
> > > This is a bug fix for 2.5
> > > 
> > >  util/mmap-alloc.c |3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
> > > index c37acbe58ede..0ff221dd94f4 100644
> > > --- a/util/mmap-alloc.c
> > > +++ b/util/mmap-alloc.c
> > > @@ -21,7 +21,8 @@ void *qemu_ram_mmap(int fd, size_t size, size_t align, 
> > > bool shared)
> > >   * space, even if size is already aligned.
> > >   */
> > >  size_t total = size + align;
> > > -void *ptr = mmap(0, total, PROT_NONE, MAP_ANONYMOUS | MAP_PRIVATE, 
> > > -1, 0);
> > > +void *ptr = mmap(0, total, PROT_NONE,
> > > + (fd == -1 ? MAP_ANONYMOUS : 0) | MAP_PRIVATE, fd, 
> > > 0);
> > >  size_t offset = QEMU_ALIGN_UP((uintptr_t)ptr, align) - 
> > > (uintptr_t)ptr;
> > >  void *ptr1;
> > >  
> > 

[Qemu-devel] [Bug 1450881] Re: qemu-system-sparc MUTEX_HELD assert and libC lock errors

[Pierre L]

Hi all,
I also have this issue with my sparcstation installation :

Emulated OS :   SunOS 5.5.1
Emulated Processor  :   sparc
Host machine OS :   Linux RED HAT

Do you manage to fix it ?

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

Title:
  qemu-system-sparc MUTEX_HELD assert and libC lock errors

Status in QEMU:
  New

Bug description:
  Here I am cross-posting a comment I made on Artyom's blog.  Atar
  responded that he "fixed these issues for some customers".  I hoped
  that opening a bug to the opensource project might help develop the
  solution for the public domain.

  I now have a mostly-working Solaris 6 emulation, with great thanks to
  the valuable information in Artyom's blog, brezular.com, and the
  QEMU/Solaris 4.14 wikibook.

  setup detail;
  QEMU (present git snapshot, reports --version 2.2.92)
  -M SS-20, openboot/proprietary prom

  # uname -a
  SunOS emu0 5.6 Generic_105181-33 sun4m sparc SUNW,SPARCstation-20

  I continue to have a problem, which I have found others posted in blog
  comments, but have not seen a resolution yet.

  # /etc/init.d/init.dmi start
  Run-time error, libC:
  Trying to release a lock that was not acquired in this thread
  (repeat above 1x)
  Abort - core dumped

  as well as:
  Assertion failed: MUTEX_HELD(_mutex), file rpc/svc_run.c, line 766

  which prints to the console periodically when "dmispd" is running.

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

[Qemu-devel] iSCSI options for IQN with colons

[Pino Toscano]

Hi,

while testing the integration of QEMU with iSCSI, I was setting up an
environment with both target and initiator IQNs with colons. Then I
tried to connect to two different targets using two different initiator
IQN, like the following:

  $ qemu ... \
  -iscsi 
id=iqn.2015-11.com.bla:suffix1,initiator-name=iqn.2015-11.com.bla:suffix1-initiator
 \
  -iscsi 
id=iqn.2015-11.com.bla:suffix2,initiator-name=iqn.2015-11.com.bla:suffix2-initiator
 \
  -drive 
file=iscsi://server/iqn.2015-11.com.bla%3Asuffix1/0,format=raw,id=hd1,if=none \
  -drive 
file=iscsi://server/iqn.2015-11.com.bla%3Asuffix2/0,format=raw,id=hd2,if=none \
  ...

which didn't work at first:

  qemu-system-x86_64: -iscsi 
id=iqn.2015-11.com.bla:suffix1,initiator-name=iqn.2015-11.com.bla:suffix1-initiator:
 Parameter 'id' expects an identifier

which, according to id_wellformed in id.c, is true. Allowing colons in
id=... like in the following patch

diff --git a/util/id.c b/util/id.c
index bcc64d8..25fca9d 100644
--- a/util/id.c
+++ b/util/id.c
@@ -20,7 +20,7 @@ bool id_wellformed(const char *id)
 return false;
 }
 for (i = 1; id[i]; i++) {
-if (!qemu_isalnum(id[i]) && !strchr("-._", id[i])) {
+if (!qemu_isalnum(id[i]) && !strchr("-._:", id[i])) {
 return false;
 }
 }

allowed me to work run QEMU with the attached disks.

The question basically boils down to whether it is right to reject
colons in id:
- if so, then there should be a way to allow them only in id of -iscsi
  (since colons can be part of IQNs)
- if not, whether allowing them could cause regressions in option
  parsing

Thanks,
-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.

[Qemu-devel] keycode error

[hjenkins]

Dear QEMU Developers,

I'm trying to use QEMU to run Debian on OS X Yosemite on a MacBookPro
(13-inch). I followed the instructions at https://wiki.debian.org/QEMU,
downloaded a standard prefab iso, and issued these commands with this
result:

qemu-img create debian.img 2G
qemu-system-x86_64  -hda debian.img -cdrom
debian-live-8.2.0-i386-lxde-desktop.iso -boot d -m 256

(process:3027): Gtk-WARNING **: Locale not supported by C library.
Using the fallback 'C' locale.
unknown keycodes `empty_aliases(qwerty)', please report to
qemu-devel@nongnu.org

Apart from that error, the Debian install screen appeared, but I could not
select anything, nor move my mouse over the top or bottom of the screen
(it jumped away from the edge if I tried).

I have no idea what might be wrong, but I have reported it as instructed
and will happily give you more information if you need it.

I installed QEMU via Fink, and can give you a step-by-step list of every
command etc. I ran to do so. I'm in Germany and using a UK keyboard and
mapping.

Regards,
Hazel

Re: [Qemu-devel] [PATCH v10] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host

[Eric Blake]

On 11/30/2015 09:51 AM, Programmingkid wrote:

>>> +++ b/block/raw-posix.c
>>> @@ -42,9 +42,9 @@
>>> #include 
>>> #include 
>>> #include 
>>> -//#include 
>>> +#include 
>>> #include 
>>> -#endif
>>> +#endif /* (__APPLE__) && (__MACH__) */
>>>
>>
>> I have now mentioned in both v8 and v9 that this hunk should be its own
>> patch (and is simple enough to cc qemu-trivial).  Disregarding reviewers
>> suggestions is not a good idea - it only serves to waste time (both
>> yours and reviewers) and earn you black marks, such that it will be even
>> less likely that anyone wants to review your patches in the first place.
>> I'm trying to help you be a better contributor, but it feels like you
>> are ignoring advice, and so my natural reaction is to ignore you.
> 
> I assure you that this change is *required* for my patch. Without it the 
> patch would
> not even compile. I need a macro from IODVDMedia.h. If removing the 
> IOCDTypes.h
> is what is bothering you, it is a very small change that no one is going to 
> miss. That
> header file was commented out but not removed for some reason. 

And I assure you that splitting the change into a separate patch, and
making this a 2-patch series, is essential for you getting your patch
applied.  It's okay for one patch to depend on another; it is not okay
to shove multiple fixes into a single patch when you have been told to
split it into multiple patches.

Okay, let me restate things a bit:

The trivial hunk of your patch (that should be applied independently,
because it is unrelated code cleanup) would be:

> #include 
> #include 
> #include 
>-//#include 
> #include 
>-#endif
>+#endif /* (__APPLE__) && (__MACH__) */

and then the part for _this_ commit (adding a new required header) would be:

> #include 
> #include 
> #include 
>+#include 
> #include 
> #endif /* (__APPLE__) && (__MACH__) */


>>> +static kern_return_t FindEjectableOpticalMedia(io_iterator_t 
>>> *mediaIterator,
>>> +char 
>>> *mediaType)
>>
>> No, your indentation is still wrong.  I tried to point out on your v8
>> that we don't right-justify to 80 columns, but rather left-justify to
>> the point just after the (.
> 
> If you feel it is that important, I will do it. I just thought it was easier 
> to read when your
> eye is already in the area. There is less time spend finding the next 
> argument that way.

When you have read THOUSANDS of lines of code indented in one style,
then your eye is already very trained to look in the same place for the
continued line.  One-off code that places the code in somewhere other
than the usual place is actually HARDER to read, because it violates the
conventions that you have already trained to read.

> I don't remember hearing about not using \n in the error_report() call, but I 
> will
> fix this in the next patch.

v8:
https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg05806.html
"Several violations of convention.  error_setg() does not need a
redundant "Error: " prefix, should not end in '!' (we aren't shouting),
and should not end in newline.  And with those fixes, you won't even
need the weird indentation."

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PULL 0/3] wxx: Last minute fixes for 2.5

[Peter Maydell]

On 30 November 2015 at 05:55, Stefan Weil <s...@weilnetz.de> wrote:
> The following changes since commit 714487515dbe0c65d5904251e796cd3a5b3579fb:
>
>   Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into 
> staging (2015-11-27 10:44:42 +)
>
> are available in the git repository at:
>
>   git://qemu.weilnetz.de/qemu.git tags/pull-wxx-20151130
>
> for you to fetch changes up to 78e9d4ad11e7116376328860a58b96765ade7b62:
>
>   w32: Use gcc option -mthreads (2015-11-30 06:47:02 +0100)
>
> 
> wxx patch queue
>
> 
> Stefan Weil (3):
>   trace/simple: Fix warning and wrong trace file name for MinGW
>   oslib-win32: Change return type of function getpagesize
>   w32: Use gcc option -mthreads
>
>  configure | 2 ++
>  include/sysemu/os-win32.h | 2 +-
>  trace/simple.c| 3 ++-
>  util/oslib-win32.c| 2 +-
>  4 files changed, 6 insertions(+), 3 deletions(-)

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v6 20/21] iotests: add incremental backup failure recovery test

[John Snow]

On 11/27/2015 12:14 PM, Kevin Wolf wrote:
> Am 18.04.2015 um 01:50 hat John Snow geschrieben:
>> Test the failure case for incremental backups.
>>
>> Signed-off-by: John Snow 
>> Reviewed-by: Max Reitz 
>> ---
>>  tests/qemu-iotests/124 | 57 
>> ++
>>  tests/qemu-iotests/124.out |  4 ++--
>>  2 files changed, 59 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
>> index 5c3b434..95f6de5 100644
>> --- a/tests/qemu-iotests/124
>> +++ b/tests/qemu-iotests/124
>> @@ -240,6 +240,63 @@ class TestIncrementalBackup(iotests.QMPTestCase):
>>  self.check_backups()
>>  
>>  
>> +def test_incremental_failure(self):
>> +'''Test: Verify backups made after a failure are correct.
>> +
>> +Simulate a failure during an incremental backup block job,
>> +emulate additional writes, then create another incremental backup
>> +afterwards and verify that the backup created is correct.
>> +'''
>> +
>> +# Create a blkdebug interface to this img as 'drive1',
>> +# but don't actually create a new image.
>> +drive1 = self.add_node('drive1', self.drives[0]['fmt'],
>> +   path=self.drives[0]['file'],
>> +   backup=self.drives[0]['backup'])
>> +result = self.vm.qmp('blockdev-add', options={
>> +'id': drive1['id'],
>> +'driver': drive1['fmt'],
>> +'file': {
>> +'driver': 'blkdebug',
>> +'image': {
>> +'driver': 'file',
>> +'filename': drive1['file']
>> +},
>> +'set-state': [{
>> +'event': 'flush_to_disk',
>> +'state': 1,
>> +'new_state': 2
>> +}],
>> +'inject-error': [{
>> +'event': 'read_aio',
>> +'errno': 5,
>> +'state': 2,
>> +'immediately': False,
>> +'once': True
>> +}],
>> +}
>> +})
>> +self.assert_qmp(result, 'return', {})
> 
> John, how naughty of you!
> 

And here I thought it was OK because nobody yelled!

The yell was just delayed.

> It's interesting how many tests break now that I tried to add some
> advisory qcow2 locking so that people don't constantly break their
> images with 'qemu-img snapshot' while the VM is running.
> 
> I think this one is a bug in the test case. I'm not completely sure how
> to fix it, though. Can we move the blkdebug layer to the top level? I
> think reusing the same qcow2 BDS (using a node name reference) would be
> okay. We just need to avoid opening the qcow2 layer twice for the same
> image.
> 

I can either do that, or just fall back to fully allocating two images
and modify the test accordingly.

> Kevin
> 

Is this for 2.5?

--js

[Qemu-devel] [PATCH v2 2/9] drivers/hv: Move HV_SYNIC_STIMER_COUNT into Hyper-V UAPI x86 header

[Andrey Smetanin]

This constant is required for Hyper-V SynIC timers MSR's
support by userspace(QEMU).

Signed-off-by: Andrey Smetanin 
Acked-by: K. Y. Srinivasan 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Vitaly Kuznetsov 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
 arch/x86/include/uapi/asm/hyperv.h | 2 ++
 drivers/hv/hyperv_vmbus.h  | 2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/uapi/asm/hyperv.h 
b/arch/x86/include/uapi/asm/hyperv.h
index 040d408..07981f0 100644
--- a/arch/x86/include/uapi/asm/hyperv.h
+++ b/arch/x86/include/uapi/asm/hyperv.h
@@ -269,4 +269,6 @@ typedef struct _HV_REFERENCE_TSC_PAGE {
 #define HV_SYNIC_SINT_AUTO_EOI (1ULL << 17)
 #define HV_SYNIC_SINT_VECTOR_MASK  (0xFF)
 
+#define HV_SYNIC_STIMER_COUNT  (4)
+
 #endif
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index e46e18c..f214e37 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -100,8 +100,6 @@ enum hv_cpuid_function {
 #define HVMSG_X64_APIC_EOI 0x80010004
 #define HVMSG_X64_LEGACY_FP_ERROR  0x80010005
 
-#define HV_SYNIC_STIMER_COUNT  (4)
-
 /* Define invalid partition identifier. */
 #define HV_PARTITION_ID_INVALID((u64)0x0)
 
-- 
2.4.3

[Qemu-devel] [PATCH v2 6/9] kvm/x86: Added Hyper-V vcpu_to_hv_vcpu()/hv_vcpu_to_vcpu() helpers

[Andrey Smetanin]

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Vitaly Kuznetsov 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
 arch/x86/kvm/hyperv.h | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
index 9483d49..d5d8217 100644
--- a/arch/x86/kvm/hyperv.h
+++ b/arch/x86/kvm/hyperv.h
@@ -24,21 +24,29 @@
 #ifndef __ARCH_X86_KVM_HYPERV_H__
 #define __ARCH_X86_KVM_HYPERV_H__
 
-static inline struct kvm_vcpu_hv_synic *vcpu_to_synic(struct kvm_vcpu *vcpu)
+static inline struct kvm_vcpu_hv *vcpu_to_hv_vcpu(struct kvm_vcpu *vcpu)
 {
-   return >arch.hyperv.synic;
+   return >arch.hyperv;
 }
 
-static inline struct kvm_vcpu *synic_to_vcpu(struct kvm_vcpu_hv_synic *synic)
+static inline struct kvm_vcpu *hv_vcpu_to_vcpu(struct kvm_vcpu_hv *hv_vcpu)
 {
-   struct kvm_vcpu_hv *hv;
struct kvm_vcpu_arch *arch;
 
-   hv = container_of(synic, struct kvm_vcpu_hv, synic);
-   arch = container_of(hv, struct kvm_vcpu_arch, hyperv);
+   arch = container_of(hv_vcpu, struct kvm_vcpu_arch, hyperv);
return container_of(arch, struct kvm_vcpu, arch);
 }
 
+static inline struct kvm_vcpu_hv_synic *vcpu_to_synic(struct kvm_vcpu *vcpu)
+{
+   return >arch.hyperv.synic;
+}
+
+static inline struct kvm_vcpu *synic_to_vcpu(struct kvm_vcpu_hv_synic *synic)
+{
+   return hv_vcpu_to_vcpu(container_of(synic, struct kvm_vcpu_hv, synic));
+}
+
 int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host);
 int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);
 
-- 
2.4.3

Re: [Qemu-devel] [PATCH v10] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host

[Eric Blake]

On 11/27/2015 02:49 PM, Programmingkid wrote:
> Mac OS X can be picky when it comes to allowing the user
> to use physical devices in QEMU. Most mounted volumes
> appear to be off limits to QEMU. If an issue is detected,
> a message is displayed showing the user how to unmount a
> volume.
> 
> Signed-off-by: John Arbuckle 
> 
> ---
> Fixed some spacing issues. 
> Removed else condition in FindEjectableOpticalMedia.
> Added continue statement to FindEjectableOpticalMedia.
> Replaced printf() with error_report() in FindEjectableOpticalMedia.
> Altered comment in FindEjectableOpticalMedia.
> If the spacing in this patch looks off, try changing the font to something
> that is mono-spaced.

Patches are best read in monospaced fonts, anyways; it's better to make
that part of your workflow, and assume that everyone else has already
done likewise, than to advertise that you are only making life harder
for yourself.

> 
>  block/raw-posix.c |  140 ++--
>  1 files changed, 102 insertions(+), 38 deletions(-)
> 
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index ccfec1c..9e7de11 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -42,9 +42,9 @@
>  #include 
>  #include 
>  #include 
> -//#include 
> +#include 
>  #include 
> -#endif
> +#endif /* (__APPLE__) && (__MACH__) */
>  

I have now mentioned in both v8 and v9 that this hunk should be its own
patch (and is simple enough to cc qemu-trivial).  Disregarding reviewers
suggestions is not a good idea - it only serves to waste time (both
yours and reviewers) and earn you black marks, such that it will be even
less likely that anyone wants to review your patches in the first place.
 I'm trying to help you be a better contributor, but it feels like you
are ignoring advice, and so my natural reaction is to ignore you.

>  #ifdef __sun__
>  #define _POSIX_PTHREAD_SEMANTICS 1
> @@ -1975,32 +1975,46 @@ BlockDriver bdrv_file = {
>  /* host device */
>  
>  #if defined(__APPLE__) && defined(__MACH__)
> -static kern_return_t FindEjectableCDMedia( io_iterator_t *mediaIterator );
>  static kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath,
>  CFIndex maxPathSize, int flags);
> -kern_return_t FindEjectableCDMedia( io_iterator_t *mediaIterator )
> +static kern_return_t FindEjectableOpticalMedia(io_iterator_t *mediaIterator,
> +char 
> *mediaType)

No, your indentation is still wrong.  I tried to point out on your v8
that we don't right-justify to 80 columns, but rather left-justify to
the point just after the (.


> +int index;
> +for (index = 0; index < ARRAY_SIZE(matching_array); index++) {
> +classesToMatch = IOServiceMatching(matching_array[index]);
> +if (classesToMatch == NULL) {
> +error_report("IOServiceMatching returned NULL for %s.\n",

No. Don't use trailing '.' or trailing '\n' in error_report() calls.
I've already mentioned this, and feel like I'm becoming a broken record.
 When you disregard my review comments, I become disinclined to review
your patches any further.

> + 
> matching_array[index]);

Indentation is still wrong.

> +continue;
> +}
> +CFDictionarySetValue(classesToMatch, CFSTR(kIOMediaEjectableKey),
> +
> kCFBooleanTrue);
> +kernResult = IOServiceGetMatchingServices(masterPort, classesToMatch,
> + 
> mediaIterator);
> +if (kernResult != KERN_SUCCESS) {
> +error_report("Note: IOServiceGetMatchingServices returned %d\n",
> +
> kernResult);

No trailing \n in error_report(), indentation is wrong.

> +}
>  
> +/* If a match was found, leave the loop */
> +if (*mediaIterator != 0) {
> +DPRINTF("Matching using %s\n", matching_array[index]);
> +snprintf(mediaType, strlen(matching_array[index])+1, "%s",
> + 
> matching_array[index]);

Spaces around binary '+', and indentation is wrong.


> +/* look for a working partition */
> +for (index = 0; index < num_of_test_partitions; index++) {
> +snprintf(test_partition, sizeof(test_partition), "%ss%d", bsd_path,
> + 
> index);

Indentation is wrong.


> +/* if a working partition on the device was not found */
> +if (partition_found == false) {
> +error_setg(errp, "Error: Failed to find a working partition on "
> + 
> "disc!\n");

Indentation is wrong, no trailing '!' or '\n' in error_setg().

I'm so 

Re: [Qemu-devel] [PATCH v3 1/4] pc: Remove redundant code from pc-*-2.3 machine classes

[Thomas Huth]

On 30/11/15 15:56, Eduardo Habkost wrote:
> Remove the redundant 'alias = NULL' and 'is_default = 0' lines
> from older machine-types. pc_*_2_4_machine_options() already
> clear those fields, so they don't need to be cleared by
> pc_*_2_3_machine_options().
> 
> Reviewed-by: Marcel Apfelbaum 
> Signed-off-by: Eduardo Habkost 
> ---
>  hw/i386/pc_piix.c | 2 --
>  hw/i386/pc_q35.c  | 1 -
>  2 files changed, 3 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 2e41efe..1a4ff01 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -499,8 +499,6 @@ static void pc_i440fx_2_3_machine_options(MachineClass *m)
>  {
>  pc_i440fx_2_4_machine_options(m);
>  m->hw_version = "2.3.0";
> -m->alias = NULL;
> -m->is_default = 0;
>  SET_MACHINE_COMPAT(m, PC_COMPAT_2_3);
>  }
>  
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 133bc68..f17acca 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -399,7 +399,6 @@ static void pc_q35_2_3_machine_options(MachineClass *m)
>  m->hw_version = "2.3.0";
>  m->no_floppy = 0;
>  m->no_tco = 1;
> -m->alias = NULL;
>  SET_MACHINE_COMPAT(m, PC_COMPAT_2_3);
>  }

Reviewed-by: Thomas Huth 

[Qemu-devel] [PATCH] coverity: Model g_memdup()

[Markus Armbruster]

We model all the non-deprecated memory allocation functions from
https://developer.gnome.org/glib/stable/glib-Memory-Allocation.html
except for g_memdup(), g_clear_pointer(), g_steal_pointer().  We don't
use the latter two.  Model the former.

Coverity now reports an OVERRUN
vl.c:2317: alloc_strlen: Allocating insufficient memory for the terminating 
null of the string.
Correct, but we omit the terminating null intentionally there.

Signed-off-by: Markus Armbruster 
---
 scripts/coverity-model.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/scripts/coverity-model.c b/scripts/coverity-model.c
index 617f67d..e1d5f45 100644
--- a/scripts/coverity-model.c
+++ b/scripts/coverity-model.c
@@ -236,6 +236,23 @@ void *g_try_realloc(void *ptr, size_t size)
 return g_try_realloc_n(ptr, 1, size);
 }
 
+/* Other memory allocation functions */
+
+void *g_memdup(const void *ptr, unsigned size)
+{
+unsigned char *dup;
+unsigned i;
+
+if (!ptr) {
+return NULL;
+}
+
+dup = g_malloc(size);
+for (i = 0; i < size; i++)
+dup[i] = ((unsigned char *)ptr)[i];
+return dup;
+}
+
 /*
  * GLib string allocation functions
  */
-- 
2.4.3

Re: [Qemu-devel] [PATCH v3 2/4] pc: Add pc-*-2.6 machine classes

[Marcel Apfelbaum]

On 11/30/2015 04:56 PM, Eduardo Habkost wrote:

Add pc-i440fx-2.6 and pc-q35-2.6 machine classes.

Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
* Add missing backslash to PC_COMPAT_2_4

Changes v2 -> v3:
* Add HW_COMPAT_2_5 to PC_COMPAT_2_5
---
  hw/i386/pc_piix.c| 16 +---
  hw/i386/pc_q35.c | 13 +++--
  include/hw/compat.h  |  3 +++
  include/hw/i386/pc.h |  4 
  4 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 1a4ff01..299c07f 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -469,13 +469,25 @@ static void pc_i440fx_machine_options(MachineClass *m)
  m->default_display = "std";
  }

-static void pc_i440fx_2_5_machine_options(MachineClass *m)
+static void pc_i440fx_2_6_machine_options(MachineClass *m)
  {
  pc_i440fx_machine_options(m);
  m->alias = "pc";
  m->is_default = 1;
  }

+DEFINE_I440FX_MACHINE(v2_6, "pc-i440fx-2.6", NULL,
+  pc_i440fx_2_6_machine_options);
+
+
+static void pc_i440fx_2_5_machine_options(MachineClass *m)
+{
+pc_i440fx_2_6_machine_options(m);
+m->alias = NULL;
+m->is_default = 0;
+SET_MACHINE_COMPAT(m, PC_COMPAT_2_5);
+}
+
  DEFINE_I440FX_MACHINE(v2_5, "pc-i440fx-2.5", NULL,
pc_i440fx_2_5_machine_options);

@@ -485,8 +497,6 @@ static void pc_i440fx_2_4_machine_options(MachineClass *m)
  PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
  pc_i440fx_2_5_machine_options(m);
  m->hw_version = "2.4.0";
-m->alias = NULL;
-m->is_default = 0;
  pcmc->broken_reserved_end = true;
  SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
  }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index f17acca..0086546 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -370,12 +370,22 @@ static void pc_q35_machine_options(MachineClass *m)
  m->no_tco = 0;
  }

-static void pc_q35_2_5_machine_options(MachineClass *m)
+static void pc_q35_2_6_machine_options(MachineClass *m)
  {
  pc_q35_machine_options(m);
  m->alias = "q35";
  }

+DEFINE_Q35_MACHINE(v2_6, "pc-q35-2.6", NULL,
+   pc_q35_2_6_machine_options);
+
+static void pc_q35_2_5_machine_options(MachineClass *m)
+{
+pc_q35_2_6_machine_options(m);
+m->alias = NULL;
+SET_MACHINE_COMPAT(m, PC_COMPAT_2_5);
+}
+
  DEFINE_Q35_MACHINE(v2_5, "pc-q35-2.5", NULL,
 pc_q35_2_5_machine_options);

@@ -384,7 +394,6 @@ static void pc_q35_2_4_machine_options(MachineClass *m)
  PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
  pc_q35_2_5_machine_options(m);
  m->hw_version = "2.4.0";
-m->alias = NULL;
  pcmc->broken_reserved_end = true;
  SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
  }
diff --git a/include/hw/compat.h b/include/hw/compat.h
index d0b1c4f..fae0d8e 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -1,6 +1,9 @@
  #ifndef HW_COMPAT_H
  #define HW_COMPAT_H

+#define HW_COMPAT_2_5 \
+/* empty */
+
  #define HW_COMPAT_2_4 \
  {\
  .driver   = "virtio-blk-device",\
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 854c330..3226291 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -296,7 +296,11 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
  int e820_get_num_entries(void);
  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);

+#define PC_COMPAT_2_5 \
+HW_COMPAT_2_5
+
  #define PC_COMPAT_2_4 \
+PC_COMPAT_2_5 \
  HW_COMPAT_2_4 \
  {\
  .driver   = "Haswell-" TYPE_X86_CPU,\



Reviewed-by: Marcel Apfelbaum 

Re: [Qemu-devel] [PATCH v3 3/4] pc: Change indentation of PC_COMPAT_* to 4 spaces

[Thomas Huth]

On 30/11/15 15:56, Eduardo Habkost wrote:
> Cosmetic change only.
> 
> Signed-off-by: Eduardo Habkost 
> ---
>  include/hw/i386/pc.h | 924 
> +--
>  1 file changed, 462 insertions(+), 462 deletions(-)

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH v3 4/4] hw/compat.h: Change indentation of HW_COMPAT_* to 4 spaces

[Thomas Huth]

On 30/11/15 15:56, Eduardo Habkost wrote:
> Cosmetic change only.
> 
> Signed-off-by: Eduardo Habkost 
> ---
>  include/hw/compat.h | 136 
> ++--
>  1 file changed, 68 insertions(+), 68 deletions(-)

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH v2 3/3] vhost-user-test: fix crash with glib < 2.36

[Markus Armbruster]

"Michael S. Tsirkin"  writes:

> On Mon, Nov 30, 2015 at 12:17:00PM +0100, marcandre.lur...@redhat.com wrote:
>> From: Marc-André Lureau 
>> 
>> The prepare callback needs to be implemented with glib < 2.36.
>> 
>> Signed-off-by: Marc-André Lureau 
>> ---
>>  tests/vhost-user-test.c | 17 +
>>  1 file changed, 13 insertions(+), 4 deletions(-)
>> 
>> diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
>> index 29205ed..27dedeb 100644
>> --- a/tests/vhost-user-test.c
>> +++ b/tests/vhost-user-test.c
>> @@ -506,11 +506,20 @@ test_migrate_source_check(GSource *source)
>>  return FALSE;
>>  }
>>  
>> +#if !GLIB_CHECK_VERSION(2,36,0)
>> +static gboolean
>> +test_migrate_source_prepare(GSource *source, gint *timeout)
>> +{
>> +*timeout = -1;
>> +return FALSE;
>> +}
>> +#endif
>> +
>>  GSourceFuncs test_migrate_source_funcs = {
>> -NULL,
>> -test_migrate_source_check,
>> -NULL,
>> -NULL
>> +#if !GLIB_CHECK_VERSION(2,36,0)
>> +.prepare = test_migrate_source_prepare,
>> +#endif
>> +.check = test_migrate_source_check,
>>  };
>>  
>>  static void test_migrate(void)
>
> I don't see why do we need the ifdefs, we can use the
> same code for all versions.
> I queued a patch that does exactly that.

The ifdefs serve as a marker that lets us drop unnecessary code when our
required version of GLib reaches 2.36.  A comment might do, too, but it
should probably contain GLIB_CHECK_VERSION() to be visible in grep.

Re: [Qemu-devel] [PATCH v10] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host

[Programmingkid]

On Nov 30, 2015, at 11:26 AM, Eric Blake wrote:

> On 11/27/2015 02:49 PM, Programmingkid wrote:
>> Mac OS X can be picky when it comes to allowing the user
>> to use physical devices in QEMU. Most mounted volumes
>> appear to be off limits to QEMU. If an issue is detected,
>> a message is displayed showing the user how to unmount a
>> volume.
>> 
>> Signed-off-by: John Arbuckle 
>> 
>> ---
>> Fixed some spacing issues. 
>> Removed else condition in FindEjectableOpticalMedia.
>> Added continue statement to FindEjectableOpticalMedia.
>> Replaced printf() with error_report() in FindEjectableOpticalMedia.
>> Altered comment in FindEjectableOpticalMedia.
>> If the spacing in this patch looks off, try changing the font to something
>> that is mono-spaced.
> 
> Patches are best read in monospaced fonts, anyways; it's better to make
> that part of your workflow, and assume that everyone else has already
> done likewise, than to advertise that you are only making life harder
> for yourself.
> 
>> 
>> block/raw-posix.c |  140 ++--
>> 1 files changed, 102 insertions(+), 38 deletions(-)
>> 
>> diff --git a/block/raw-posix.c b/block/raw-posix.c
>> index ccfec1c..9e7de11 100644
>> --- a/block/raw-posix.c
>> +++ b/block/raw-posix.c
>> @@ -42,9 +42,9 @@
>> #include 
>> #include 
>> #include 
>> -//#include 
>> +#include 
>> #include 
>> -#endif
>> +#endif /* (__APPLE__) && (__MACH__) */
>> 
> 
> I have now mentioned in both v8 and v9 that this hunk should be its own
> patch (and is simple enough to cc qemu-trivial).  Disregarding reviewers
> suggestions is not a good idea - it only serves to waste time (both
> yours and reviewers) and earn you black marks, such that it will be even
> less likely that anyone wants to review your patches in the first place.
> I'm trying to help you be a better contributor, but it feels like you
> are ignoring advice, and so my natural reaction is to ignore you.

I assure you that this change is *required* for my patch. Without it the patch 
would
not even compile. I need a macro from IODVDMedia.h. If removing the IOCDTypes.h
is what is bothering you, it is a very small change that no one is going to 
miss. That
header file was commented out but not removed for some reason. 

I do thank you for your patients. I think it might be better if instead of 
saying "this is wrong",
you talk about what should be done differently more.

> 
>> #ifdef __sun__
>> #define _POSIX_PTHREAD_SEMANTICS 1
>> @@ -1975,32 +1975,46 @@ BlockDriver bdrv_file = {
>> /* host device */
>> 
>> #if defined(__APPLE__) && defined(__MACH__)
>> -static kern_return_t FindEjectableCDMedia( io_iterator_t *mediaIterator );
>> static kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath,
>> CFIndex maxPathSize, int flags);
>> -kern_return_t FindEjectableCDMedia( io_iterator_t *mediaIterator )
>> +static kern_return_t FindEjectableOpticalMedia(io_iterator_t *mediaIterator,
>> +char 
>> *mediaType)
> 
> No, your indentation is still wrong.  I tried to point out on your v8
> that we don't right-justify to 80 columns, but rather left-justify to
> the point just after the (.

If you feel it is that important, I will do it. I just thought it was easier to 
read when your
eye is already in the area. There is less time spend finding the next argument 
that way.

> 
>> +int index;
>> +for (index = 0; index < ARRAY_SIZE(matching_array); index++) {
>> +classesToMatch = IOServiceMatching(matching_array[index]);
>> +if (classesToMatch == NULL) {
>> +error_report("IOServiceMatching returned NULL for %s.\n",
> 
> No. Don't use trailing '.' or trailing '\n' in error_report() calls.
> I've already mentioned this, and feel like I'm becoming a broken record.
> When you disregard my review comments, I become disinclined to review
> your patches any further.

I don't remember hearing about not using \n in the error_report() call, but I 
will
fix this in the next patch.

> 
>> + 
>> matching_array[index]);
> 
> Indentation is still wrong.

Will left justify with the left parenthesis. 

> 
>> +continue;
>> +}
>> +CFDictionarySetValue(classesToMatch, CFSTR(kIOMediaEjectableKey),
>> +
>> kCFBooleanTrue);
>> +kernResult = IOServiceGetMatchingServices(masterPort, 
>> classesToMatch,
>> + 
>> mediaIterator);
>> +if (kernResult != KERN_SUCCESS) {
>> +error_report("Note: IOServiceGetMatchingServices returned %d\n",
>> +
>> kernResult);
> 
> No trailing \n in error_report(), indentation is wrong.

Ok. 

> 
>> +}
>> 
>> +/* 

Re: [Qemu-devel] [PATCH v7 14/24] nbd: Switch from close to eject notifier

[Max Reitz]

On 30.11.2015 16:36, Kevin Wolf wrote:
> Am 09.11.2015 um 23:39 hat Max Reitz geschrieben:
>> The NBD code uses the BDS close notifier to determine when a medium is
>> ejected. However, now it should use the BB's BDS removal notifier for
>> that instead of the BDS's close notifier.
>>
>> Signed-off-by: Max Reitz 
>> ---
>>  blockdev-nbd.c | 37 +
>>  nbd.c  | 13 +
>>  2 files changed, 14 insertions(+), 36 deletions(-)
>>
>> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
>> index bcdd18b..b28a55b 100644
>> --- a/blockdev-nbd.c
>> +++ b/blockdev-nbd.c
>> @@ -46,37 +46,11 @@ void qmp_nbd_server_start(SocketAddress *addr, Error 
>> **errp)
>>  }
>>  }
>>  
>> -/*
>> - * Hook into the BlockBackend notifiers to close the export when the
>> - * backend is closed.
>> - */
>> -typedef struct NBDCloseNotifier {
>> -Notifier n;
>> -NBDExport *exp;
>> -QTAILQ_ENTRY(NBDCloseNotifier) next;
>> -} NBDCloseNotifier;
>> -
>> -static QTAILQ_HEAD(, NBDCloseNotifier) close_notifiers =
>> -QTAILQ_HEAD_INITIALIZER(close_notifiers);
>> -
>> -static void nbd_close_notifier(Notifier *n, void *data)
>> -{
>> -NBDCloseNotifier *cn = DO_UPCAST(NBDCloseNotifier, n, n);
>> -
>> -notifier_remove(>n);
>> -QTAILQ_REMOVE(_notifiers, cn, next);
>> -
>> -nbd_export_close(cn->exp);
>> -nbd_export_put(cn->exp);
> 
> Here you remove a close/put pair, but in the new code you only add the
> close call. Why is this correct?

Thanks for putting so much unfounded faith in me. :-)

After having put quite some time into looking into it, first I have to
say that it's a very good question.

First off, it's wrong. This is because I thought every export would have
a refcount of one. Turns out this is wrong, they have a refcount of two:
It is created with a refcount of one, and then it gains another by
giving it a name and entering it into the export list.

I did know about the second but I completely forgot about the former.
And indeed I do think it is wrong. qmp_nbd_server_add() does not keep
the reference to the export, once the function returns, it is gone.
Therefore, it should call nbd_export_put() before returning.

So in my opinion the current code is wrong and I didn't fix it enough.
*cough*

So, with the nbd_export_put() added to qmp_nbd_server_add(), every
export will have a refcount of 1 + |clients|. The eject notifier will
call nbd_close_notifier(), which first disconnects the clients, thus
reducing the refcount by |clients|, and then sets the name to NULL, thus
dropping the final refcount and destroying the export.

In the old code, we needed another nbd_export_put() because of the
superfluous reference from nbd_export_new(), which doesn't actually
exist for blockdev-nbd (it does for qemu-nbd, though).

Max



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v2 3/9] drivers/hv: Move struct hv_message into UAPI Hyper-V x86 header

[Andrey Smetanin]

This struct is required for Hyper-V SynIC timers implementation inside KVM
and for upcoming Hyper-V VMBus support by userspace(QEMU). So place it into
Hyper-V UAPI header.

Signed-off-by: Andrey Smetanin 
Acked-by: K. Y. Srinivasan 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Vitaly Kuznetsov 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
 arch/x86/include/uapi/asm/hyperv.h | 74 ++
 drivers/hv/hyperv_vmbus.h  | 73 -
 2 files changed, 74 insertions(+), 73 deletions(-)

diff --git a/arch/x86/include/uapi/asm/hyperv.h 
b/arch/x86/include/uapi/asm/hyperv.h
index 07981f0..76e503d 100644
--- a/arch/x86/include/uapi/asm/hyperv.h
+++ b/arch/x86/include/uapi/asm/hyperv.h
@@ -271,4 +271,78 @@ typedef struct _HV_REFERENCE_TSC_PAGE {
 
 #define HV_SYNIC_STIMER_COUNT  (4)
 
+/* Define synthetic interrupt controller message constants. */
+#define HV_MESSAGE_SIZE(256)
+#define HV_MESSAGE_PAYLOAD_BYTE_COUNT  (240)
+#define HV_MESSAGE_PAYLOAD_QWORD_COUNT (30)
+
+/* Define hypervisor message types. */
+#define HVMSG_NONE 0x
+
+/* Memory access messages. */
+#define HVMSG_UNMAPPED_GPA 0x8000
+#define HVMSG_GPA_INTERCEPT0x8001
+
+/* Timer notification messages. */
+#define HVMSG_TIMER_EXPIRED0x8010
+
+/* Error messages. */
+#define HVMSG_INVALID_VP_REGISTER_VALUE0x8020
+#define HVMSG_UNRECOVERABLE_EXCEPTION  0x8021
+#define HVMSG_UNSUPPORTED_FEATURE  0x8022
+
+/* Trace buffer complete messages. */
+#define HVMSG_EVENTLOG_BUFFERCOMPLETE  0x8040
+
+/* Platform-specific processor intercept messages. */
+#define HVMSG_X64_IOPORT_INTERCEPT 0x8001
+#define HVMSG_X64_MSR_INTERCEPT0x80010001
+#define HVMSG_X64_CPUID_INTERCEPT  0x80010002
+#define HVMSG_X64_EXCEPTION_INTERCEPT  0x80010003
+#define HVMSG_X64_APIC_EOI 0x80010004
+#define HVMSG_X64_LEGACY_FP_ERROR  0x80010005
+
+/* Define synthetic interrupt controller message flags. */
+union hv_message_flags {
+   __u8 asu8;
+   struct {
+   __u8 msg_pending:1;
+   __u8 reserved:7;
+   };
+};
+
+/* Define port identifier type. */
+union hv_port_id {
+   __u32 asu32;
+   struct {
+   __u32 id:24;
+   __u32 reserved:8;
+   } u;
+};
+
+/* Define synthetic interrupt controller message header. */
+struct hv_message_header {
+   __u32 message_type;
+   __u8 payload_size;
+   union hv_message_flags message_flags;
+   __u8 reserved[2];
+   union {
+   __u64 sender;
+   union hv_port_id port;
+   };
+};
+
+/* Define synthetic interrupt controller message format. */
+struct hv_message {
+   struct hv_message_header header;
+   union {
+   __u64 payload[HV_MESSAGE_PAYLOAD_QWORD_COUNT];
+   } u;
+};
+
+/* Define the synthetic interrupt message page layout. */
+struct hv_message_page {
+   struct hv_message sint_message[HV_SYNIC_SINT_COUNT];
+};
+
 #endif
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index f214e37..3f3756b 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -63,10 +63,6 @@ enum hv_cpuid_function {
 /* Define version of the synthetic interrupt controller. */
 #define HV_SYNIC_VERSION   (1)
 
-/* Define synthetic interrupt controller message constants. */
-#define HV_MESSAGE_SIZE(256)
-#define HV_MESSAGE_PAYLOAD_BYTE_COUNT  (240)
-#define HV_MESSAGE_PAYLOAD_QWORD_COUNT (30)
 #define HV_ANY_VP  (0x)
 
 /* Define synthetic interrupt controller flag constants. */
@@ -74,44 +70,9 @@ enum hv_cpuid_function {
 #define HV_EVENT_FLAGS_BYTE_COUNT  (256)
 #define HV_EVENT_FLAGS_DWORD_COUNT (256 / sizeof(u32))
 
-/* Define hypervisor message types. */
-#define HVMSG_NONE 0x
-
-/* Memory access messages. */
-#define HVMSG_UNMAPPED_GPA 0x8000
-#define HVMSG_GPA_INTERCEPT0x8001
-
-/* Timer notification messages. */
-#define HVMSG_TIMER_EXPIRED0x8010
-
-/* Error messages. */
-#define HVMSG_INVALID_VP_REGISTER_VALUE0x8020
-#define HVMSG_UNRECOVERABLE_EXCEPTION  0x8021
-#define HVMSG_UNSUPPORTED_FEATURE  0x8022
-
-/* Trace buffer complete messages. */
-#define HVMSG_EVENTLOG_BUFFERCOMPLETE  0x8040
-
-/* Platform-specific processor intercept messages. */
-#define HVMSG_X64_IOPORT_INTERCEPT 0x8001
-#define HVMSG_X64_MSR_INTERCEPT0x80010001
-#define 

[Qemu-devel] [PATCH v2 1/9] drivers/hv: replace enum hv_message_type by u32

[Andrey Smetanin]

enum hv_message_type inside struct hv_message, hv_post_message
is not size portable. Replace enum by u32.

Signed-off-by: Andrey Smetanin 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Vitaly Kuznetsov 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org

---
 drivers/hv/hv.c   |  4 ++--
 drivers/hv/hyperv_vmbus.h | 48 +++
 2 files changed, 25 insertions(+), 27 deletions(-)

diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 6341be8..dde7e1c 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -310,8 +310,8 @@ void hv_cleanup(void)
  * This involves a hypercall.
  */
 int hv_post_message(union hv_connection_id connection_id,
- enum hv_message_type message_type,
- void *payload, size_t payload_size)
+   u32 message_type,
+   void *payload, size_t payload_size)
 {
 
struct hv_input_post_message *aligned_msg;
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 3782636..e46e18c 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -75,32 +75,30 @@ enum hv_cpuid_function {
 #define HV_EVENT_FLAGS_DWORD_COUNT (256 / sizeof(u32))
 
 /* Define hypervisor message types. */
-enum hv_message_type {
-   HVMSG_NONE  = 0x,
+#define HVMSG_NONE 0x
 
-   /* Memory access messages. */
-   HVMSG_UNMAPPED_GPA  = 0x8000,
-   HVMSG_GPA_INTERCEPT = 0x8001,
+/* Memory access messages. */
+#define HVMSG_UNMAPPED_GPA 0x8000
+#define HVMSG_GPA_INTERCEPT0x8001
 
-   /* Timer notification messages. */
-   HVMSG_TIMER_EXPIRED = 0x8010,
+/* Timer notification messages. */
+#define HVMSG_TIMER_EXPIRED0x8010
 
-   /* Error messages. */
-   HVMSG_INVALID_VP_REGISTER_VALUE = 0x8020,
-   HVMSG_UNRECOVERABLE_EXCEPTION   = 0x8021,
-   HVMSG_UNSUPPORTED_FEATURE   = 0x8022,
+/* Error messages. */
+#define HVMSG_INVALID_VP_REGISTER_VALUE0x8020
+#define HVMSG_UNRECOVERABLE_EXCEPTION  0x8021
+#define HVMSG_UNSUPPORTED_FEATURE  0x8022
 
-   /* Trace buffer complete messages. */
-   HVMSG_EVENTLOG_BUFFERCOMPLETE   = 0x8040,
+/* Trace buffer complete messages. */
+#define HVMSG_EVENTLOG_BUFFERCOMPLETE  0x8040
 
-   /* Platform-specific processor intercept messages. */
-   HVMSG_X64_IOPORT_INTERCEPT  = 0x8001,
-   HVMSG_X64_MSR_INTERCEPT = 0x80010001,
-   HVMSG_X64_CPUID_INTERCEPT   = 0x80010002,
-   HVMSG_X64_EXCEPTION_INTERCEPT   = 0x80010003,
-   HVMSG_X64_APIC_EOI  = 0x80010004,
-   HVMSG_X64_LEGACY_FP_ERROR   = 0x80010005
-};
+/* Platform-specific processor intercept messages. */
+#define HVMSG_X64_IOPORT_INTERCEPT 0x8001
+#define HVMSG_X64_MSR_INTERCEPT0x80010001
+#define HVMSG_X64_CPUID_INTERCEPT  0x80010002
+#define HVMSG_X64_EXCEPTION_INTERCEPT  0x80010003
+#define HVMSG_X64_APIC_EOI 0x80010004
+#define HVMSG_X64_LEGACY_FP_ERROR  0x80010005
 
 #define HV_SYNIC_STIMER_COUNT  (4)
 
@@ -174,7 +172,7 @@ union hv_message_flags {
 
 /* Define synthetic interrupt controller message header. */
 struct hv_message_header {
-   enum hv_message_type message_type;
+   u32 message_type;
u8 payload_size;
union hv_message_flags message_flags;
u8 reserved[2];
@@ -347,7 +345,7 @@ enum hv_call_code {
 struct hv_input_post_message {
union hv_connection_id connectionid;
u32 reserved;
-   enum hv_message_type message_type;
+   u32 message_type;
u32 payload_size;
u64 payload[HV_MESSAGE_PAYLOAD_QWORD_COUNT];
 };
@@ -579,8 +577,8 @@ extern int hv_init(void);
 extern void hv_cleanup(void);
 
 extern int hv_post_message(union hv_connection_id connection_id,
-enum hv_message_type message_type,
-void *payload, size_t payload_size);
+  u32 message_type,
+  void *payload, size_t payload_size);
 
 extern u16 hv_signal_event(void *con_id);
 
-- 
2.4.3

[Qemu-devel] [PATCH v2 8/9] kvm/x86: Hyper-V SynIC message slot pending clearing at SINT ack

[Andrey Smetanin]

The SynIC message protocol mandates that the message slot is claimed
by atomically setting message type to something other than HVMSG_NONE.
If another message is to be delivered while the slot is still busy,
message pending flag is asserted to indicate to the guest that the
hypervisor wants to be notified when the slot is released.

To make sure the protocol works regardless of where the message
sources are (kernel or userspace), clear the pending flag on SINT ACK
notification, and let the message sources compete for the slot again.

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Vitaly Kuznetsov 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
 arch/x86/kvm/hyperv.c| 31 +++
 include/linux/kvm_host.h |  2 ++
 2 files changed, 33 insertions(+)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 9958926..6412b6b 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -27,6 +27,7 @@
 #include "hyperv.h"
 
 #include 
+#include 
 #include 
 #include 
 
@@ -116,13 +117,43 @@ static struct kvm_vcpu_hv_synic *synic_get(struct kvm 
*kvm, u32 vcpu_id)
return (synic->active) ? synic : NULL;
 }
 
+static void synic_clear_sint_msg_pending(struct kvm_vcpu_hv_synic *synic,
+   u32 sint)
+{
+   struct kvm_vcpu *vcpu = synic_to_vcpu(synic);
+   struct page *page;
+   gpa_t gpa;
+   struct hv_message *msg;
+   struct hv_message_page *msg_page;
+
+   gpa = synic->msg_page & PAGE_MASK;
+   page = kvm_vcpu_gfn_to_page(vcpu, gpa >> PAGE_SHIFT);
+   if (is_error_page(page)) {
+   vcpu_err(vcpu, "Hyper-V SynIC can't get msg page, gpa 0x%llx\n",
+gpa);
+   return;
+   }
+   msg_page = kmap_atomic(page);
+
+   msg = _page->sint_message[sint];
+   msg->header.message_flags.msg_pending = 0;
+
+   kunmap_atomic(msg_page);
+   kvm_release_page_dirty(page);
+   kvm_vcpu_mark_page_dirty(vcpu, gpa >> PAGE_SHIFT);
+}
+
 static void kvm_hv_notify_acked_sint(struct kvm_vcpu *vcpu, u32 sint)
 {
struct kvm *kvm = vcpu->kvm;
+   struct kvm_vcpu_hv_synic *synic = vcpu_to_synic(vcpu);
int gsi, idx;
 
vcpu_debug(vcpu, "Hyper-V SynIC acked sint %d\n", sint);
 
+   if (synic->msg_page & HV_SYNIC_SIMP_ENABLE)
+   synic_clear_sint_msg_pending(synic, sint);
+
idx = srcu_read_lock(>irq_srcu);
gsi = atomic_read(_to_synic(vcpu)->sint_to_gsi[sint]);
if (gsi != -1)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 2911919..9b64c8c 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -450,6 +450,8 @@ struct kvm {
 
 #define vcpu_debug(vcpu, fmt, ...) \
kvm_debug("vcpu%i " fmt, (vcpu)->vcpu_id, ## __VA_ARGS__)
+#define vcpu_err(vcpu, fmt, ...)   \
+   kvm_err("vcpu%i " fmt, (vcpu)->vcpu_id, ## __VA_ARGS__)
 
 static inline struct kvm_vcpu *kvm_get_vcpu(struct kvm *kvm, int i)
 {
-- 
2.4.3

Re: [Qemu-devel] [PATCH v7 24/24] iotests: Add test for block jobs and BDS ejection

[Kevin Wolf]

Am 09.11.2015 um 23:39 hat Max Reitz geschrieben:
> Suggested-by: Paolo Bonzini 
> Signed-off-by: Max Reitz 
> ---
>  tests/qemu-iotests/141 | 166 
> +
>  tests/qemu-iotests/141.out |  47 +
>  tests/qemu-iotests/group   |   1 +
>  3 files changed, 214 insertions(+)
>  create mode 100755 tests/qemu-iotests/141
>  create mode 100644 tests/qemu-iotests/141.out
> 
> diff --git a/tests/qemu-iotests/141 b/tests/qemu-iotests/141
> new file mode 100755
> index 000..6a32d56
> --- /dev/null
> +++ b/tests/qemu-iotests/141
> @@ -0,0 +1,166 @@
> +#!/bin/bash
> +#
> +# Test case for ejecting BDSs with block jobs still running on them
> +#
> +# Copyright (C) 2015 Red Hat, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see .
> +#
> +
> +# creator
> +owner=mre...@redhat.com
> +
> +seq="$(basename $0)"
> +echo "QA output created by $seq"
> +
> +here="$PWD"
> +tmp=/tmp/$$
> +status=1 # failure is the default!
> +
> +_cleanup()
> +{
> +_cleanup_test_img
> +rm -f "$TEST_DIR/{b,o}.$IMGFMT"
> +}
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.filter
> +. ./common.qemu
> +
> +# Needs backing file support
> +_supported_fmt qcow qcow2 qed

The test doesn't work for me on qcow1.

> +echo
> +echo '=== Testing block-commit ==='
> +echo
> +
> +# block-commit will send BLOCK_JOB_READY basically immediately, and 
> cancelling
> +# the job will consequently result in BLOCK_JOB_COMPLETED being emitted.
> +
> +test_blockjob \
> +"{'execute': 'block-commit',
> +  'arguments': {'device': 'drv0'}}" \
> +'BLOCK_JOB_READY' \
> +'BLOCK_JOB_COMPLETED'

This is commit of the active layer, i.e. just a mirror in disguise.
Should we test a "real" commit block job as well?

Anyway, with qcow1 removed from the list:
Reviewed-by: Kevin Wolf 

Re: [Qemu-devel] [PATCH v3 2/4] pc: Add pc-*-2.6 machine classes

[Thomas Huth]

On 30/11/15 15:56, Eduardo Habkost wrote:
> Add pc-i440fx-2.6 and pc-q35-2.6 machine classes.
> 
> Signed-off-by: Eduardo Habkost 
> ---
> Changes v1 -> v2:
> * Add missing backslash to PC_COMPAT_2_4
> 
> Changes v2 -> v3:
> * Add HW_COMPAT_2_5 to PC_COMPAT_2_5
> ---
>  hw/i386/pc_piix.c| 16 +---
>  hw/i386/pc_q35.c | 13 +++--
>  include/hw/compat.h  |  3 +++
>  include/hw/i386/pc.h |  4 
>  4 files changed, 31 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 1a4ff01..299c07f 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -469,13 +469,25 @@ static void pc_i440fx_machine_options(MachineClass *m)
>  m->default_display = "std";
>  }
>  
> -static void pc_i440fx_2_5_machine_options(MachineClass *m)
> +static void pc_i440fx_2_6_machine_options(MachineClass *m)
>  {
>  pc_i440fx_machine_options(m);
>  m->alias = "pc";
>  m->is_default = 1;
>  }
>  
> +DEFINE_I440FX_MACHINE(v2_6, "pc-i440fx-2.6", NULL,
> +  pc_i440fx_2_6_machine_options);
> +
> +
> +static void pc_i440fx_2_5_machine_options(MachineClass *m)
> +{
> +pc_i440fx_2_6_machine_options(m);
> +m->alias = NULL;
> +m->is_default = 0;
> +SET_MACHINE_COMPAT(m, PC_COMPAT_2_5);
> +}
> +
>  DEFINE_I440FX_MACHINE(v2_5, "pc-i440fx-2.5", NULL,
>pc_i440fx_2_5_machine_options);
>  
> @@ -485,8 +497,6 @@ static void pc_i440fx_2_4_machine_options(MachineClass *m)
>  PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>  pc_i440fx_2_5_machine_options(m);
>  m->hw_version = "2.4.0";
> -m->alias = NULL;
> -m->is_default = 0;
>  pcmc->broken_reserved_end = true;
>  SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
>  }
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index f17acca..0086546 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -370,12 +370,22 @@ static void pc_q35_machine_options(MachineClass *m)
>  m->no_tco = 0;
>  }
>  
> -static void pc_q35_2_5_machine_options(MachineClass *m)
> +static void pc_q35_2_6_machine_options(MachineClass *m)
>  {
>  pc_q35_machine_options(m);
>  m->alias = "q35";
>  }
>  
> +DEFINE_Q35_MACHINE(v2_6, "pc-q35-2.6", NULL,
> +   pc_q35_2_6_machine_options);
> +
> +static void pc_q35_2_5_machine_options(MachineClass *m)
> +{
> +pc_q35_2_6_machine_options(m);
> +m->alias = NULL;
> +SET_MACHINE_COMPAT(m, PC_COMPAT_2_5);
> +}
> +
>  DEFINE_Q35_MACHINE(v2_5, "pc-q35-2.5", NULL,
> pc_q35_2_5_machine_options);
>  
> @@ -384,7 +394,6 @@ static void pc_q35_2_4_machine_options(MachineClass *m)
>  PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>  pc_q35_2_5_machine_options(m);
>  m->hw_version = "2.4.0";
> -m->alias = NULL;
>  pcmc->broken_reserved_end = true;
>  SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
>  }
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index d0b1c4f..fae0d8e 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -1,6 +1,9 @@
>  #ifndef HW_COMPAT_H
>  #define HW_COMPAT_H
>  
> +#define HW_COMPAT_2_5 \
> +/* empty */
> +
>  #define HW_COMPAT_2_4 \
>  {\
>  .driver   = "virtio-blk-device",\
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 854c330..3226291 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -296,7 +296,11 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
>  int e820_get_num_entries(void);
>  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>  
> +#define PC_COMPAT_2_5 \
> +HW_COMPAT_2_5
> +
>  #define PC_COMPAT_2_4 \
> +PC_COMPAT_2_5 \
>  HW_COMPAT_2_4 \
>  {\
>  .driver   = "Haswell-" TYPE_X86_CPU,\

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH 01/40] 9pfs: allocate pdus with g_malloc/g_free

[Greg Kurz]

On Tue, 24 Nov 2015 19:00:52 +0100
Paolo Bonzini  wrote:

> Prepare for moving the allocation to virtqueue_pop.
> 
> Signed-off-by: Paolo Bonzini 
> ---

And aside from this series goal, it makes the code nicer :)

>  hw/9pfs/virtio-9p-device.c |  7 +--
>  hw/9pfs/virtio-9p.c| 10 +++---
>  hw/9pfs/virtio-9p.h|  2 --
>  3 files changed, 4 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index e3abcfa..72a93c2 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -57,7 +57,7 @@ static void virtio_9p_device_realize(DeviceState *dev, 
> Error **errp)
>  {
>  VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>  V9fsState *s = VIRTIO_9P(dev);
> -int i, len;
> +int len;
>  struct stat stat;
>  FsDriverEntry *fse;
>  V9fsPath path;
> @@ -65,12 +65,7 @@ static void virtio_9p_device_realize(DeviceState *dev, 
> Error **errp)
>  virtio_init(vdev, "virtio-9p", VIRTIO_ID_9P,
>  sizeof(struct virtio_9p_config) + MAX_TAG_LEN);
> 
> -/* initialize pdu allocator */
> -QLIST_INIT(>free_list);
>  QLIST_INIT(>active_list);
> -for (i = 0; i < (MAX_REQ - 1); i++) {
> -QLIST_INSERT_HEAD(>free_list, >pdus[i], next);
> -}
> 
>  s->vq = virtio_add_queue(vdev, MAX_REQ, handle_9p_output);
> 
> diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
> index f972731..747306b 100644
> --- a/hw/9pfs/virtio-9p.c
> +++ b/hw/9pfs/virtio-9p.c
> @@ -565,13 +565,9 @@ static int fid_to_qid(V9fsPDU *pdu, V9fsFidState *fidp, 
> V9fsQID *qidp)
> 
>  static V9fsPDU *alloc_pdu(V9fsState *s)
>  {
> -V9fsPDU *pdu = NULL;
> +V9fsPDU *pdu = g_new(V9fsPDU, 1);
> 
> -if (!QLIST_EMPTY(>free_list)) {
> -pdu = QLIST_FIRST(>free_list);
> -QLIST_REMOVE(pdu, next);
> -QLIST_INSERT_HEAD(>active_list, pdu, next);
> -}
> +QLIST_INSERT_HEAD(>active_list, pdu, next);
>  return pdu;
>  }
> 
> @@ -584,8 +580,8 @@ static void free_pdu(V9fsState *s, V9fsPDU *pdu)
>   */
>  if (!pdu->cancelled) {
>  QLIST_REMOVE(pdu, next);
> -QLIST_INSERT_HEAD(>free_list, pdu, next);
>  }
> +g_free(pdu);
>  }
>  }
> 
> diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
> index d7a4dc1..1fb4ff9 100644
> --- a/hw/9pfs/virtio-9p.h
> +++ b/hw/9pfs/virtio-9p.h
> @@ -201,8 +201,6 @@ typedef struct V9fsState
>  {
>  VirtIODevice parent_obj;
>  VirtQueue *vq;
> -V9fsPDU pdus[MAX_REQ];
> -QLIST_HEAD(, V9fsPDU) free_list;
>  QLIST_HEAD(, V9fsPDU) active_list;
>  V9fsFidState *fid_list;
>  FileOperations *ops;

[Qemu-devel] [PATCH for-2.5 v4 0/4] vhost-user-test fixes

[marcandre . lureau]

From: Marc-André Lureau 

This series fixes a number of races and crashes on glib < 2.36
(with Travis build for ex).

v3->v4:
- add patch to include vhost-user-test when target list lacks 386.
- add a code comment about <2.36 prepare callback.

v2->v3:
- quote glib documentation in commit message of last patch.

v1->v2:
- fix the last patch to not end up with check() callback instead of
  prepare(): use named initializer.

Marc-André Lureau (4):
  vhost-user-test: fix chardriver race
  vhost-user-test: use unix port for migration
  vhost-user-test: fix crash with glib < 2.36
  tests: add vhost-user-test when target is x64 only

 tests/Makefile  |  6 +++---
 tests/vhost-user-test.c | 51 -
 2 files changed, 45 insertions(+), 12 deletions(-)

-- 
2.5.0

[Qemu-devel] [PATCH for-2.5 v4 1/4] vhost-user-test: fix chardriver race

[marcandre . lureau]

From: Marc-André Lureau 

vhost-user-tests uses a helper thread to dispatch the vhost-user servers
sources. However the CharDriverState is not thread-safe. Therefore, when
it's given to the thread, it shouldn't be manipulated concurrently.

We dispatch cleaning the server in an idle source. By the end of the
test, we ensure not to leave anything behind by joining the thread and
finishing the sources dispatch.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Michael S. Tsirkin 
---
 tests/vhost-user-test.c | 26 +-
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index e4c36af..261f4b7 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -216,8 +216,7 @@ static void read_guest_mem(TestServer *s)
 
 static void *thread_function(void *data)
 {
-GMainLoop *loop;
-loop = g_main_loop_new(NULL, FALSE);
+GMainLoop *loop = data;
 g_main_loop_run(loop);
 return NULL;
 }
@@ -389,7 +388,7 @@ static TestServer *test_server_new(const gchar *name)
 g_strdup_printf(QEMU_CMD extra, (mem), (mem), (root), (s)->chr_name,   
\
 (s)->socket_path, (s)->chr_name, ##__VA_ARGS__)
 
-static void test_server_free(TestServer *server)
+static gboolean _test_server_free(TestServer *server)
 {
 int i;
 
@@ -406,9 +405,15 @@ static void test_server_free(TestServer *server)
 unlink(server->socket_path);
 g_free(server->socket_path);
 
-
 g_free(server->chr_name);
 g_free(server);
+
+return FALSE;
+}
+
+static void test_server_free(TestServer *server)
+{
+g_idle_add((GSourceFunc)_test_server_free, server);
 }
 
 static void wait_for_log_fd(TestServer *s)
@@ -590,6 +595,8 @@ int main(int argc, char **argv)
 char *qemu_cmd = NULL;
 int ret;
 char template[] = "/tmp/vhost-test-XX";
+GMainLoop *loop;
+GThread *thread;
 
 g_test_init(, , NULL);
 
@@ -612,8 +619,9 @@ int main(int argc, char **argv)
 
 server = test_server_new("test");
 
+loop = g_main_loop_new(NULL, FALSE);
 /* run the main loop thread so the chardev may operate */
-g_thread_new(NULL, thread_function, NULL);
+thread = g_thread_new(NULL, thread_function, loop);
 
 qemu_cmd = GET_QEMU_CMD(server);
 
@@ -632,6 +640,14 @@ int main(int argc, char **argv)
 /* cleanup */
 test_server_free(server);
 
+/* finish the helper thread and dispatch pending sources */
+g_main_loop_quit(loop);
+g_thread_join(thread);
+while (g_main_context_pending(NULL)) {
+g_main_context_iteration (NULL, TRUE);
+}
+g_main_loop_unref(loop);
+
 ret = rmdir(tmpfs);
 if (ret != 0) {
 g_test_message("unable to rmdir: path (%s): %s\n",
-- 
2.5.0

[Qemu-devel] [PATCH for-2.5 v4 3/4] vhost-user-test: fix crash with glib < 2.36

[marcandre . lureau]

From: Marc-André Lureau 

The prepare callback needs to be implemented with glib < 2.36,
quoting glib documentation:
"Since 2.36 this may be NULL, in which case the effect is as if the
function always returns FALSE with a timeout of -1."

Signed-off-by: Marc-André Lureau 
---
 tests/vhost-user-test.c | 19 +++
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index 29205ed..29de739 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -506,11 +506,22 @@ test_migrate_source_check(GSource *source)
 return FALSE;
 }
 
+#if !GLIB_CHECK_VERSION(2,36,0)
+/* this callback is unnecessary with glib >2.36, the default
+ * prepare for the source does the same */
+static gboolean
+test_migrate_source_prepare(GSource *source, gint *timeout)
+{
+*timeout = -1;
+return FALSE;
+}
+#endif
+
 GSourceFuncs test_migrate_source_funcs = {
-NULL,
-test_migrate_source_check,
-NULL,
-NULL
+#if !GLIB_CHECK_VERSION(2,36,0)
+.prepare = test_migrate_source_prepare,
+#endif
+.check = test_migrate_source_check,
 };
 
 static void test_migrate(void)
-- 
2.5.0

Re: [Qemu-devel] [PATCH for-2.5 v3 0/3] vhost-user-test fixes

[Alex Bennée]

marcandre.lur...@redhat.com writes:

> From: Marc-André Lureau 
>
> This series fixes a number of races and crashes on glib < 2.36
> (with Travis build for ex).

When does this test get run normally? I don't see it when I do a "make
check" with a my build:

 ./configure 
--target-list=arm-softmmu,aarch64-softmmu,i386-softmmu,x86_64-softmmu

I was struggling to find any notes on running individual tests in the
docs.

>
> v2->v3:
> - quote glib documentation in commit message of last patch.
>
> v1->v2:
> - fix the last patch to not end up with check() callback instead of
>   prepare(): use named initializer.
>
> Marc-André Lureau (3):
>   vhost-user-test: fix chardriver race
>   vhost-user-test: use unix port for migration
>   vhost-user-test: fix crash with glib < 2.36
>
>  tests/vhost-user-test.c | 49 
> -
>  1 file changed, 40 insertions(+), 9 deletions(-)


--
Alex Bennée

Re: [Qemu-devel] [PATCH for-2.5 v3 0/3] vhost-user-test fixes

[Marc-André Lureau]

On Mon, Nov 30, 2015 at 4:14 PM, Alex Bennée  wrote:
> When does this test get run normally? I don't see it when I do a "make
> check" with a my build:
>
>  ./configure 
> --target-list=arm-softmmu,aarch64-softmmu,i386-softmmu,x86_64-softmmu

That should work, as long as kvm is enabled. However, while looking
into this, I found a similar issue when x86 target is missing. I just
sent patch.

> I was struggling to find any notes on running individual tests in the
> docs.


Did you find something about it?

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v7 14/24] nbd: Switch from close to eject notifier

[Kevin Wolf]

Am 09.11.2015 um 23:39 hat Max Reitz geschrieben:
> The NBD code uses the BDS close notifier to determine when a medium is
> ejected. However, now it should use the BB's BDS removal notifier for
> that instead of the BDS's close notifier.
> 
> Signed-off-by: Max Reitz 
> ---
>  blockdev-nbd.c | 37 +
>  nbd.c  | 13 +
>  2 files changed, 14 insertions(+), 36 deletions(-)
> 
> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
> index bcdd18b..b28a55b 100644
> --- a/blockdev-nbd.c
> +++ b/blockdev-nbd.c
> @@ -46,37 +46,11 @@ void qmp_nbd_server_start(SocketAddress *addr, Error 
> **errp)
>  }
>  }
>  
> -/*
> - * Hook into the BlockBackend notifiers to close the export when the
> - * backend is closed.
> - */
> -typedef struct NBDCloseNotifier {
> -Notifier n;
> -NBDExport *exp;
> -QTAILQ_ENTRY(NBDCloseNotifier) next;
> -} NBDCloseNotifier;
> -
> -static QTAILQ_HEAD(, NBDCloseNotifier) close_notifiers =
> -QTAILQ_HEAD_INITIALIZER(close_notifiers);
> -
> -static void nbd_close_notifier(Notifier *n, void *data)
> -{
> -NBDCloseNotifier *cn = DO_UPCAST(NBDCloseNotifier, n, n);
> -
> -notifier_remove(>n);
> -QTAILQ_REMOVE(_notifiers, cn, next);
> -
> -nbd_export_close(cn->exp);
> -nbd_export_put(cn->exp);

Here you remove a close/put pair, but in the new code you only add the
close call. Why is this correct?

Kevin

Re: [Qemu-devel] [PATCH v2 06/21] block: Exclude nested options only for children in append_open_options()

[Max Reitz]

On 30.11.2015 10:01, Kevin Wolf wrote:
> Am 27.11.2015 um 18:58 hat Max Reitz geschrieben:
>> On 23.11.2015 16:59, Kevin Wolf wrote:
>>> Some drivers have nested options (e.g. blkdebug rule arrays), which
>>> don't belong to a child node and shouldn't be removed. Don't remove all
>>> options with "." in their name, but check for the complete prefixes of
>>> actually existing child nodes.
>>>
>>> Signed-off-by: Kevin Wolf 
>>> ---
>>>  block.c   | 19 +++
>>>  include/block/block_int.h |  1 +
>>>  2 files changed, 16 insertions(+), 4 deletions(-)
>>
>> Thanks, now I don't need to fix it myself. :-)
>>
>> (I would have had to do that for an in-work series of mine)
>>
>>> diff --git a/block.c b/block.c
>>> index 23d9e10..02125e2 100644
>>> --- a/block.c
>>> +++ b/block.c
>>> @@ -1101,11 +1101,13 @@ static int bdrv_fill_options(QDict **options, const 
>>> char **pfilename,
>>>  
>>>  static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
>>>  BlockDriverState *child_bs,
>>> +const char *child_name,
>>>  const BdrvChildRole *child_role)
>>>  {
>>>  BdrvChild *child = g_new(BdrvChild, 1);
>>>  *child = (BdrvChild) {
>>>  .bs = child_bs,
>>> +.name   = child_name,
>>>  .role   = child_role,
>>>  };
>>>  
>>> @@ -1165,7 +1167,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs, 
>>> BlockDriverState *backing_hd)
>>>  bs->backing = NULL;
>>>  goto out;
>>>  }
>>> -bs->backing = bdrv_attach_child(bs, backing_hd, _backing);
>>> +bs->backing = bdrv_attach_child(bs, backing_hd, "backing", 
>>> _backing);
>>>  bs->open_flags &= ~BDRV_O_NO_BACKING;
>>>  pstrcpy(bs->backing_file, sizeof(bs->backing_file), 
>>> backing_hd->filename);
>>>  pstrcpy(bs->backing_format, sizeof(bs->backing_format),
>>> @@ -1322,7 +1324,7 @@ BdrvChild *bdrv_open_child(const char *filename,
>>>  goto done;
>>>  }
>>>  
>>> -c = bdrv_attach_child(parent, bs, child_role);
>>> +c = bdrv_attach_child(parent, bs, bdref_key, child_role);
>>>  
>>>  done:
>>>  qdict_del(options, bdref_key);
>>> @@ -3952,13 +3954,22 @@ static bool append_open_options(QDict *d, 
>>> BlockDriverState *bs)
>>>  {
>>>  const QDictEntry *entry;
>>>  QemuOptDesc *desc;
>>> +BdrvChild *child;
>>>  bool found_any = false;
>>> +const char *p;
>>>  
>>>  for (entry = qdict_first(bs->options); entry;
>>>   entry = qdict_next(bs->options, entry))
>>>  {
>>> -/* Only take options for this level */
>>> -if (strchr(qdict_entry_key(entry), '.')) {
>>> +/* Exclude options for children */
>>> +QLIST_FOREACH(child, >children, next) {
>>> +if (strstart(qdict_entry_key(entry), child->name, )
>>> +&& (!*p || *p == '.'))
>>> +{
>>> +break;
>>> +}
>>> +}
>>> +if (child) {
>>>  continue;
>>>  }
>>>  
>>
>> A good general solution, but I think bdrv_refresh_filename() may be bad
>> enough to break with general solutions. ;-)
>>
>> bdrv_refresh_filename() only considers "file" and "backing" (actually,
>> it only supports "file" for now, I'm working on "backing", though). The
>> only drivers with other children are quorum, blkdebug, blkverify and
>> VMDK. The former three have their own implementation of
>> bdrv_refresh_filename(), so they don't use append_open_options() at all.
>> The latter, however, (VMDK) does not.
>>
>> This change to append_open_options results in the extent.%d options
>> simply being omitted altogether because bdrv_refresh_filename() does not
>> fetch them. Before, they were included in the VMDK BDS's options, which
>> is not ideal but works more or less.
> 
> Are you sure? As far as I can tell, this patch should only keep options
> that were previously removed, but not remove options that were
> previously kept (with the exception of direct use of child names, which
> I added here to address your review comments for v1).
> 
> Specifically for "extents.%d", this is a child name and is therefore
> omitted. However, it contains a '.', so it was already removed without
> this patch.

Right, it is broken already. The same applies to qcow2's options
containing a dot.

Max

> I'm accepting proof of the contrary in the form of a test case. ;-)
> 
>> In order to "fix" this, I see three ways right now:
>> 1. Just care about "file" and "backing". bdrv_refresh_filename()
>>doesn't support anything else, so that will be fine.
>> 2. Implement bdrv_refresh_filename() specifically for VMDK so
>>append_open_options() will never have to handle anything but "file"
>>and "backing".
>> 3. Fix bdrv_refresh_filename() so that it handles all children and not
>>just "file" and "backing".
>>
>> Since we are shooting for 2.6 anyway (I assume ;-)), I 

Re: [Qemu-devel] [PATCH v9] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host

[Eric Blake]

On 11/27/2015 12:35 PM, Programmingkid wrote:

>> Unusual indentation; more typical is:
>>
>> | static kern_return_t FindEjectableOpticalMedia(io_iterator_t
>> *mediaIterator,
>> | char *mediatType)
> 
> I agree. I wanted the second long to be right justified with the 80 character 
> line count.

No.  We don't right-justify code to 80 columns.  That's not how it is
done.  Trying to do it just makes you look like the proverbial 'kid' in
your pseudonym, rather than an adult to be taken seriously.

Really, PLEASE follow the indentation patterns of the rest of the code
base - where continued lines are left-justified to be underneath the
character after (, and NOT right-justified to 80 columns.  Violating
style doesn't make your code invalid, but does make your patches less
likely to be applied.


>>> +/* If you found a match, leave the loop */
>>> +if (*mediaIterator != 0) {
>>> +DPRINTF("Matching using %s\n", matching_array[index]);
>>> +snprintf(mediaType, strlen(matching_array[index])+1, "%s",
>>
>> Spaces around binary '+'.
> 
> What's wrong with no spaces around the plus sign?

Again, the prevailing conventions in the code base is that you put
spaces around every binary operator.  Yes, there is existing old code
that does not meet the conventions, but it is not an excuse to add new
code that is gratuitously different.

> 
>>
>>> +/* if a working partition on the device was not found */
>>> +if (partition_found == false) {
>>> +error_setg(errp, "Error: Failed to find a working partition on "
>>> + 
>>> "disc!\n");
>>
>> and I already pointed out on v8 that this is not the correct usage of
>> error_setg().  So, here's hoping v10 addresses the comments here and
>> elsewhere.
> 
> Kevin Wolf wanted it this way. What would you do instead?

Keven and I both want you to use error_setg(), but to use it correctly -
and the correct way is to NOT supply a trailing \n.

> 
> Thank you very much for reviewing my patches.

The least you can do for showing that gratitude is to actually improve
your next revisions along the lines of the comments you have been given.
 Quit making it feel like pulling teeth just to get your patches to
match the coding conventions prevalent in the project.

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v3 4/4] hw/compat.h: Change indentation of HW_COMPAT_* to 4 spaces

[Eduardo Habkost]

Cosmetic change only.

Signed-off-by: Eduardo Habkost 
---
 include/hw/compat.h | 136 ++--
 1 file changed, 68 insertions(+), 68 deletions(-)

diff --git a/include/hw/compat.h b/include/hw/compat.h
index fae0d8e..24dd2c0 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -5,79 +5,79 @@
 /* empty */
 
 #define HW_COMPAT_2_4 \
-{\
-.driver   = "virtio-blk-device",\
-.property = "scsi",\
-.value= "true",\
-},{\
-.driver   = "e1000",\
-.property = "extra_mac_registers",\
-.value= "off",\
-},{\
-.driver   = "virtio-pci",\
-.property = "x-disable-pcie",\
-.value= "on",\
-},{\
-.driver   = "virtio-pci",\
-.property = "migrate-extra",\
-.value= "off",\
-},
+{\
+.driver   = "virtio-blk-device",\
+.property = "scsi",\
+.value= "true",\
+},{\
+.driver   = "e1000",\
+.property = "extra_mac_registers",\
+.value= "off",\
+},{\
+.driver   = "virtio-pci",\
+.property = "x-disable-pcie",\
+.value= "on",\
+},{\
+.driver   = "virtio-pci",\
+.property = "migrate-extra",\
+.value= "off",\
+},
 
 #define HW_COMPAT_2_3 \
-{\
-.driver   = "virtio-blk-pci",\
-.property = "any_layout",\
-.value= "off",\
-},{\
-.driver   = "virtio-balloon-pci",\
-.property = "any_layout",\
-.value= "off",\
-},{\
-.driver   = "virtio-serial-pci",\
-.property = "any_layout",\
-.value= "off",\
-},{\
-.driver   = "virtio-9p-pci",\
-.property = "any_layout",\
-.value= "off",\
-},{\
-.driver   = "virtio-rng-pci",\
-.property = "any_layout",\
-.value= "off",\
-},
+{\
+.driver   = "virtio-blk-pci",\
+.property = "any_layout",\
+.value= "off",\
+},{\
+.driver   = "virtio-balloon-pci",\
+.property = "any_layout",\
+.value= "off",\
+},{\
+.driver   = "virtio-serial-pci",\
+.property = "any_layout",\
+.value= "off",\
+},{\
+.driver   = "virtio-9p-pci",\
+.property = "any_layout",\
+.value= "off",\
+},{\
+.driver   = "virtio-rng-pci",\
+.property = "any_layout",\
+.value= "off",\
+},
 
 #define HW_COMPAT_2_2 \
-/* empty */
+/* empty */
 
 #define HW_COMPAT_2_1 \
-{\
-.driver   = "intel-hda",\
-.property = "old_msi_addr",\
-.value= "on",\
-},{\
-.driver   = "VGA",\
-.property = "qemu-extended-regs",\
-.value= "off",\
-},{\
-.driver   = "secondary-vga",\
-.property = "qemu-extended-regs",\
-.value= "off",\
-},{\
-.driver   = "virtio-scsi-pci",\
-.property = "any_layout",\
-.value= "off",\
-},{\
-.driver   = "usb-mouse",\
-.property = "usb_version",\
-.value= stringify(1),\
-},{\
-.driver   = "usb-kbd",\
-.property = "usb_version",\
-.value= stringify(1),\
-},{\
-.driver   = "virtio-pci",\
-.property = "virtio-pci-bus-master-bug-migration",\
-.value= "on",\
-},
+{\
+.driver   = "intel-hda",\
+.property = "old_msi_addr",\
+.value= "on",\
+},{\
+.driver   = "VGA",\
+.property = "qemu-extended-regs",\
+.value= "off",\
+},{\
+.driver   = "secondary-vga",\
+.property = "qemu-extended-regs",\
+.value= "off",\
+},{\
+.driver   = "virtio-scsi-pci",\
+.property = "any_layout",\
+.value= "off",\
+},{\
+.driver   = "usb-mouse",\
+.property = "usb_version",\
+.value= stringify(1),\
+},{\
+.driver   = "usb-kbd",\
+.property = "usb_version",\
+.value= stringify(1),\
+},{\
+.driver   = "virtio-pci",\
+.property = "virtio-pci-bus-master-bug-migration",\
+.value= "on",\
+},
 
 #endif /* HW_COMPAT_H */
-- 
2.1.0

Re: [Qemu-devel] [PATCH V3 3/3] hw/i386: extend pxb query for all PC machines

[Eduardo Habkost]

On Sun, Nov 29, 2015 at 10:46:03AM +0200, Marcel Apfelbaum wrote:
> On 11/27/2015 07:28 PM, Eduardo Habkost wrote:
> >On Thu, Nov 26, 2015 at 06:00:28PM +0200, Marcel Apfelbaum wrote:
> >>Add bus property to PC machines and use it when looking
> >>for primary PCI root bus (bus 0).
> >>
> >>Signed-off-by: Marcel Apfelbaum 
> >
> >I can't pretend I have reviewed the q35 part, but the changes are
> >an improvement to the existing code that depended on
> >find_i440fx().
> >
> >Acked-by: Eduardo Habkost 
> 
> Thanks!
> 
> >
> >BTW, what's missing to allow us to change acpi_set_pci_info() to
> >use PCMachine::bus instead of find_i440fx(), too? How much of the
> >PCI hotplug stuff is different in q35?
> 
> It is pretty different.
> i440fx has acpi based hotplug while q35 has PCIe native hotplug. Since is
> "native", no acpi info is necessary.
> 
> Having said that, if we have an PCIe-PCI bridge, the pci devices behind it
> cannot be hotplugged/unplugged right now.
> 
> Once we decide to add hotplug support for this scenario, maybe we can get rid 
> of
> find_i440fx().

Thanks for the explanation. I wonder if there's a better way to
check if ACPI-based hotplug is needed by looking at
PCMachineState or PCIBus, so we don't couple the ACPI code to
piix.c.

-- 
Eduardo

Re: [Qemu-devel] [RFC PATCH V2 0/3] IXGBE/VFIO: Add live migration support for SRIOV NIC

[Alexander Duyck]

On Sun, Nov 29, 2015 at 10:53 PM, Lan, Tianyu  wrote:
> On 11/26/2015 11:56 AM, Alexander Duyck wrote:
>>
>> > I am not saying you cannot modify the drivers, however what you are
>> doing is far too invasive.  Do you seriously plan on modifying all of
>> the PCI device drivers out there in order to allow any device that
>> might be direct assigned to a port to support migration?  I certainly
>> hope not.  That is why I have said that this solution will not scale.
>
>
> Current drivers are not migration friendly. If the driver wants to
> support migration, it's necessary to be changed.

Modifying all of the drivers directly will not solve the issue though.
This is why I have suggested looking at possibly implementing
something like dma_mark_clean() which is used for ia64 architectures
to mark pages that were DMAed in as clean.  In your case though you
would want to mark such pages as dirty so that the page migration will
notice them and move them over.

> RFC PATCH V1 presented our ideas about how to deal with MMIO, ring and
> DMA tracking during migration. These are common for most drivers and
> they maybe problematic in the previous version but can be corrected later.

They can only be corrected if the underlying assumptions are correct
and they aren't.  Your solution would have never worked correctly.
The problem is you assume you can keep the device running when you are
migrating and you simply cannot.  At some point you will always have
to stop the device in order to complete the migration, and you cannot
stop it before you have stopped your page tracking mechanism.  So
unless the platform has an IOMMU that is somehow taking part in the
dirty page tracking you will not be able to stop the guest and then
the device, it will have to be the device and then the guest.

> Doing suspend and resume() may help to do migration easily but some
> devices requires low service down time. Especially network and I got
> that some cloud company promised less than 500ms network service downtime.

Honestly focusing on the downtime is getting the cart ahead of the
horse.  First you need to be able to do this without corrupting system
memory and regardless of the state of the device.  You haven't even
gotten to that state yet.  Last I knew the device had to be up in
order for your migration to even work.

Many devices are very state driven.  As such you cannot just freeze
them and restore them like you would regular device memory.  That is
where something like suspend/resume comes in because it already takes
care of getting the device ready for halt, and then resume.  Keep in
mind that those functions were meant to function on a device doing
something like a suspend to RAM or disk.  This is not too far of from
what a migration is doing since you need to halt the guest before you
move it.

As such the first step is to make it so that we can do the current
bonding approach with one change.  Specifically we want to leave the
device in the guest until the last portion of the migration instead of
having to remove it first.  To that end I would suggest focusing on
solving the DMA problem via something like a dma_mark_clean() type
solution as that would be one issue resolved and we all would see an
immediate gain instead of just those users of the ixgbevf driver.

> So I think performance effect also should be taken into account when we
> design the framework.

What you are proposing I would call premature optimization.  You need
to actually solve the problem before you can start optimizing things
and I don't see anything actually solved yet since your solution is
too unstable.

>>
>> What I am counter proposing seems like a very simple proposition.  It
>> can be implemented in two steps.
>>
>> 1.  Look at modifying dma_mark_clean().  It is a function called in
>> the sync and unmap paths of the lib/swiotlb.c.  If you could somehow
>> modify it to take care of marking the pages you unmap for Rx as being
>> dirty it will get you a good way towards your goal as it will allow
>> you to continue to do DMA while you are migrating the VM.
>>
>> 2.  Look at making use of the existing PCI suspend/resume calls that
>> are there to support PCI power management.  They have everything
>> needed to allow you to pause and resume DMA for the device before and
>> after the migration while retaining the driver state.  If you can
>> implement something that allows you to trigger these calls from the
>> PCI subsystem such as hot-plug then you would have a generic solution
>> that can be easily reproduced for multiple drivers beyond those
>> supported by ixgbevf.
>
>
> Glanced at PCI hotplug code. The hotplug events are triggered by PCI hotplug
> controller and these event are defined in the controller spec.
> It's hard to extend more events. Otherwise, we also need to add some
> specific codes in the PCI hotplug core since it's only add and remove
> PCI device when it gets events. It's also a challenge to modify Windows
> 

[Qemu-devel] [PULL 1/2] fsdev-proxy-helper: avoid TOC/TOU race

[Greg Kurz]

From: Paolo Bonzini 

There is a minor time of check/time of use race between statfs and chroot.
It can be fixed easily by stat-ing the root after it has been changed.

Signed-off-by: Paolo Bonzini 
Reviewed-by: Greg Kurz 
Signed-off-by: Greg Kurz 
---
 fsdev/virtfs-proxy-helper.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index 9097d15c989c..ad1da0d6f530 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -1128,10 +1128,19 @@ int main(int argc, char **argv)
 }
 }
 
+if (chdir("/") < 0) {
+do_perror("chdir");
+goto error;
+}
+if (chroot(rpath) < 0) {
+do_perror("chroot");
+goto error;
+}
+
 get_version = false;
 #ifdef FS_IOC_GETVERSION
 /* check whether underlying FS support IOC_GETVERSION */
-retval = statfs(rpath, _fs);
+retval = statfs("/", _fs);
 if (!retval) {
 switch (st_fs.f_type) {
 case EXT2_SUPER_MAGIC:
@@ -1144,16 +1153,7 @@ int main(int argc, char **argv)
 }
 #endif
 
-if (chdir("/") < 0) {
-do_perror("chdir");
-goto error;
-}
-if (chroot(rpath) < 0) {
-do_perror("chroot");
-goto error;
-}
 umask(0);
-
 if (init_capabilities() < 0) {
 goto error;
 }
-- 
2.4.3

[Qemu-devel] [PULL 0/2] virtio-9p fixes for 2.5

[Greg Kurz]

The following changes since commit 714487515dbe0c65d5904251e796cd3a5b3579fb:

  Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into 
staging (2015-11-27 10:44:42 +)

are available in the git repository at:

  https://github.com/gkurz/qemu.git tags/for-upstream

for you to fetch changes up to ebac1202c95a4f1b76b6ef3f0f63926fa76e753e:

  virtio-9p: use QEMU thread pool (2015-11-30 12:36:12 +0100)


Two fixes for virtfs/9p from Paolo.


Paolo Bonzini (2):
  fsdev-proxy-helper: avoid TOC/TOU race
  virtio-9p: use QEMU thread pool

 fsdev/virtfs-proxy-helper.c | 20 ++---
 hw/9pfs/virtio-9p-coth.c| 69 +
 hw/9pfs/virtio-9p-coth.h| 10 +--
 hw/9pfs/virtio-9p-device.c  |  4 ---
 4 files changed, 25 insertions(+), 78 deletions(-)
-- 
2.4.3

[Qemu-devel] [PATCH v3 3/4] pc: Change indentation of PC_COMPAT_* to 4 spaces

[Eduardo Habkost]

Cosmetic change only.

Signed-off-by: Eduardo Habkost 
---
 include/hw/i386/pc.h | 924 +--
 1 file changed, 462 insertions(+), 462 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 3226291..3a09ccd 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -300,485 +300,485 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t 
*);
 HW_COMPAT_2_5
 
 #define PC_COMPAT_2_4 \
-PC_COMPAT_2_5 \
-HW_COMPAT_2_4 \
-{\
-.driver   = "Haswell-" TYPE_X86_CPU,\
-.property = "abm",\
-.value= "off",\
-},\
-{\
-.driver   = "Haswell-noTSX-" TYPE_X86_CPU,\
-.property = "abm",\
-.value= "off",\
-},\
-{\
-.driver   = "Broadwell-" TYPE_X86_CPU,\
-.property = "abm",\
-.value= "off",\
-},\
-{\
-.driver   = "Broadwell-noTSX-" TYPE_X86_CPU,\
-.property = "abm",\
-.value= "off",\
-},\
-{\
-.driver   = "host" "-" TYPE_X86_CPU,\
-.property = "host-cache-info",\
-.value= "on",\
-},\
-{\
-.driver   = TYPE_X86_CPU,\
-.property = "check",\
-.value= "off",\
-},\
-{\
-.driver   = "qemu64" "-" TYPE_X86_CPU,\
-.property = "sse4a",\
-.value= "on",\
-},\
-{\
-.driver   = "qemu64" "-" TYPE_X86_CPU,\
-.property = "abm",\
-.value= "on",\
-},\
-{\
-.driver   = "qemu64" "-" TYPE_X86_CPU,\
-.property = "popcnt",\
-.value= "on",\
-},\
-{\
-.driver   = "qemu32" "-" TYPE_X86_CPU,\
-.property = "popcnt",\
-.value= "on",\
-},{\
-.driver   = "Opteron_G2" "-" TYPE_X86_CPU,\
-.property = "rdtscp",\
-.value= "on",\
-},{\
-.driver   = "Opteron_G3" "-" TYPE_X86_CPU,\
-.property = "rdtscp",\
-.value= "on",\
-},{\
-.driver   = "Opteron_G4" "-" TYPE_X86_CPU,\
-.property = "rdtscp",\
-.value= "on",\
-},{\
-.driver   = "Opteron_G5" "-" TYPE_X86_CPU,\
-.property = "rdtscp",\
-.value= "on",\
-},
+PC_COMPAT_2_5 \
+HW_COMPAT_2_4 \
+{\
+.driver   = "Haswell-" TYPE_X86_CPU,\
+.property = "abm",\
+.value= "off",\
+},\
+{\
+.driver   = "Haswell-noTSX-" TYPE_X86_CPU,\
+.property = "abm",\
+.value= "off",\
+},\
+{\
+.driver   = "Broadwell-" TYPE_X86_CPU,\
+.property = "abm",\
+.value= "off",\
+},\
+{\
+.driver   = "Broadwell-noTSX-" TYPE_X86_CPU,\
+.property = "abm",\
+.value= "off",\
+},\
+{\
+.driver   = "host" "-" TYPE_X86_CPU,\
+.property = "host-cache-info",\
+.value= "on",\
+},\
+{\
+.driver   = TYPE_X86_CPU,\
+.property = "check",\
+.value= "off",\
+},\
+{\
+.driver   = "qemu64" "-" TYPE_X86_CPU,\
+.property = "sse4a",\
+.value= "on",\
+},\
+{\
+.driver   = "qemu64" "-" TYPE_X86_CPU,\
+.property = "abm",\
+.value= "on",\
+},\
+{\
+.driver   = "qemu64" "-" TYPE_X86_CPU,\
+.property = "popcnt",\
+.value= "on",\
+},\
+{\
+.driver   = "qemu32" "-" TYPE_X86_CPU,\
+.property = "popcnt",\
+.value= "on",\
+},{\
+.driver   = "Opteron_G2" "-" TYPE_X86_CPU,\
+.property = "rdtscp",\
+.value= "on",\
+},{\
+.driver   = "Opteron_G3" "-" TYPE_X86_CPU,\
+.property = "rdtscp",\
+.value= "on",\
+},{\
+.driver   = "Opteron_G4" "-" TYPE_X86_CPU,\
+.property = "rdtscp",\
+.value= "on",\
+},{\
+.driver   = "Opteron_G5" "-" TYPE_X86_CPU,\
+.property = "rdtscp",\
+.value= "on",\
+},
 
 
 #define PC_COMPAT_2_3 \
-PC_COMPAT_2_4 \
-HW_COMPAT_2_3 \
-{\
-.driver   = TYPE_X86_CPU,\
-.property = "arat",\
-.value= "off",\
-},{\
-.driver   = "qemu64" "-" TYPE_X86_CPU,\
-.property = "level",\
-.value= stringify(4),\
-},{\
-.driver   = "kvm64" "-" TYPE_X86_CPU,\
-.property = "level",\
-.value= stringify(5),\
-},{\
-.driver   = "pentium3" "-" TYPE_X86_CPU,\
-.property = "level",\
-.value= stringify(2),\
-},{\
-.driver   = "n270" "-" 

[Qemu-devel] [PATCH v3 1/4] pc: Remove redundant code from pc-*-2.3 machine classes

[Eduardo Habkost]

Remove the redundant 'alias = NULL' and 'is_default = 0' lines
from older machine-types. pc_*_2_4_machine_options() already
clear those fields, so they don't need to be cleared by
pc_*_2_3_machine_options().

Reviewed-by: Marcel Apfelbaum 
Signed-off-by: Eduardo Habkost 
---
 hw/i386/pc_piix.c | 2 --
 hw/i386/pc_q35.c  | 1 -
 2 files changed, 3 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 2e41efe..1a4ff01 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -499,8 +499,6 @@ static void pc_i440fx_2_3_machine_options(MachineClass *m)
 {
 pc_i440fx_2_4_machine_options(m);
 m->hw_version = "2.3.0";
-m->alias = NULL;
-m->is_default = 0;
 SET_MACHINE_COMPAT(m, PC_COMPAT_2_3);
 }
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 133bc68..f17acca 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -399,7 +399,6 @@ static void pc_q35_2_3_machine_options(MachineClass *m)
 m->hw_version = "2.3.0";
 m->no_floppy = 0;
 m->no_tco = 1;
-m->alias = NULL;
 SET_MACHINE_COMPAT(m, PC_COMPAT_2_3);
 }
 
-- 
2.1.0

[Qemu-devel] [PATCH v3 0/4] pc: Add pc-*-2.6 machine classes

[Eduardo Habkost]

I am sending this earlier so it can be queued and used as base
for patches that need to add compat code to the pc-2.5 machine
types.

Changes v1 -> v2:
* Typo fix that I forgot to commit before submitting v1

Changes v2 -> v3:
* Add HW_COMPAT_2_5 to PC_COMPAT_2_5
* Change indentation of PC_COMPAT_* and HW_COMPAT_* to 4 spaces

Eduardo Habkost (4):
  pc: Remove redundant code from pc-*-2.3 machine classes
  pc: Add pc-*-2.6 machine classes
  pc: Change indentation of PC_COMPAT_* to 4 spaces
  hw/compat.h: Change indentation of HW_COMPAT_* to 4 spaces

 hw/i386/pc_piix.c|  18 +-
 hw/i386/pc_q35.c |  14 +-
 include/hw/compat.h  | 139 
 include/hw/i386/pc.h | 926 ++-
 4 files changed, 560 insertions(+), 537 deletions(-)

-- 
2.1.0

Re: [Qemu-devel] [Qemu-block] [PATCH v2 03/21] mirror: Error out when a BDS would get two BBs

[Alberto Garcia]

On Mon 23 Nov 2015 04:59:42 PM CET, Kevin Wolf wrote:

> @@ -370,11 +371,22 @@ static void mirror_exit(BlockJob *job, void *opaque)
>  if (s->to_replace) {
>  to_replace = s->to_replace;
>  }
> +
> +/* This was checked in mirror_start_job(), but meanwhile one of the
> + * nodes could have been newly attached to a BlockBackend. */
> +if (to_replace->blk && s->target->blk) {
> +error_report("block job: Can't create node with two 
> BlockBackends");
> +data->ret = -EINVAL;
> +goto out;
> +}

Does it make sense to even allow attaching a BDS to a Block Backend
during this block job? Is there any use case for that?

Berto

[Qemu-devel] [PATCH] MAINTAINERS: add maintainer to virtio-9p

[Greg Kurz]

As suggested by Paolo, I add myself as maintainer for virtio-9p.

Signed-off-by: Greg Kurz 
---
 MAINTAINERS |1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index bb1f3e40622b..e8cee1e2668f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -863,6 +863,7 @@ F: net/vhost-user.c
 
 virtio-9p
 M: Aneesh Kumar K.V 
+M: Greg Kurz 
 S: Supported
 F: hw/9pfs/
 F: fsdev/

[Qemu-devel] [PATCH v3 2/4] pc: Add pc-*-2.6 machine classes

[Eduardo Habkost]

Add pc-i440fx-2.6 and pc-q35-2.6 machine classes.

Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
* Add missing backslash to PC_COMPAT_2_4

Changes v2 -> v3:
* Add HW_COMPAT_2_5 to PC_COMPAT_2_5
---
 hw/i386/pc_piix.c| 16 +---
 hw/i386/pc_q35.c | 13 +++--
 include/hw/compat.h  |  3 +++
 include/hw/i386/pc.h |  4 
 4 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 1a4ff01..299c07f 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -469,13 +469,25 @@ static void pc_i440fx_machine_options(MachineClass *m)
 m->default_display = "std";
 }
 
-static void pc_i440fx_2_5_machine_options(MachineClass *m)
+static void pc_i440fx_2_6_machine_options(MachineClass *m)
 {
 pc_i440fx_machine_options(m);
 m->alias = "pc";
 m->is_default = 1;
 }
 
+DEFINE_I440FX_MACHINE(v2_6, "pc-i440fx-2.6", NULL,
+  pc_i440fx_2_6_machine_options);
+
+
+static void pc_i440fx_2_5_machine_options(MachineClass *m)
+{
+pc_i440fx_2_6_machine_options(m);
+m->alias = NULL;
+m->is_default = 0;
+SET_MACHINE_COMPAT(m, PC_COMPAT_2_5);
+}
+
 DEFINE_I440FX_MACHINE(v2_5, "pc-i440fx-2.5", NULL,
   pc_i440fx_2_5_machine_options);
 
@@ -485,8 +497,6 @@ static void pc_i440fx_2_4_machine_options(MachineClass *m)
 PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
 pc_i440fx_2_5_machine_options(m);
 m->hw_version = "2.4.0";
-m->alias = NULL;
-m->is_default = 0;
 pcmc->broken_reserved_end = true;
 SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
 }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index f17acca..0086546 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -370,12 +370,22 @@ static void pc_q35_machine_options(MachineClass *m)
 m->no_tco = 0;
 }
 
-static void pc_q35_2_5_machine_options(MachineClass *m)
+static void pc_q35_2_6_machine_options(MachineClass *m)
 {
 pc_q35_machine_options(m);
 m->alias = "q35";
 }
 
+DEFINE_Q35_MACHINE(v2_6, "pc-q35-2.6", NULL,
+   pc_q35_2_6_machine_options);
+
+static void pc_q35_2_5_machine_options(MachineClass *m)
+{
+pc_q35_2_6_machine_options(m);
+m->alias = NULL;
+SET_MACHINE_COMPAT(m, PC_COMPAT_2_5);
+}
+
 DEFINE_Q35_MACHINE(v2_5, "pc-q35-2.5", NULL,
pc_q35_2_5_machine_options);
 
@@ -384,7 +394,6 @@ static void pc_q35_2_4_machine_options(MachineClass *m)
 PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
 pc_q35_2_5_machine_options(m);
 m->hw_version = "2.4.0";
-m->alias = NULL;
 pcmc->broken_reserved_end = true;
 SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
 }
diff --git a/include/hw/compat.h b/include/hw/compat.h
index d0b1c4f..fae0d8e 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -1,6 +1,9 @@
 #ifndef HW_COMPAT_H
 #define HW_COMPAT_H
 
+#define HW_COMPAT_2_5 \
+/* empty */
+
 #define HW_COMPAT_2_4 \
 {\
 .driver   = "virtio-blk-device",\
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 854c330..3226291 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -296,7 +296,11 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
 int e820_get_num_entries(void);
 bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
 
+#define PC_COMPAT_2_5 \
+HW_COMPAT_2_5
+
 #define PC_COMPAT_2_4 \
+PC_COMPAT_2_5 \
 HW_COMPAT_2_4 \
 {\
 .driver   = "Haswell-" TYPE_X86_CPU,\
-- 
2.1.0

[Qemu-devel] [PATCH] tests: add vhost-user-test when target is x64 only

[marcandre . lureau]

From: Marc-André Lureau 

check-qtest-x86_64-y is overwritten by the last assignment. Move
vhost-user-test addition after.

Reported-by: Alex Bennée 
Signed-off-by: Marc-André Lureau 
---
 tests/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/Makefile b/tests/Makefile
index 0ef00a1..cf1228b 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -198,13 +198,13 @@ check-qtest-i386-y += tests/pc-cpu-test$(EXESUF)
 check-qtest-i386-y += tests/q35-test$(EXESUF)
 gcov-files-i386-y += hw/pci-host/q35.c
 check-qtest-i386-$(CONFIG_VHOST_NET_TEST_i386) += 
tests/vhost-user-test$(EXESUF)
-ifeq ($(CONFIG_VHOST_NET_TEST_i386),)
-check-qtest-x86_64-$(CONFIG_VHOST_NET_TEST_x86_64) += 
tests/vhost-user-test$(EXESUF)
-endif
 check-qtest-i386-y += tests/test-netfilter$(EXESUF)
 check-qtest-x86_64-y = $(check-qtest-i386-y)
 gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
 gcov-files-x86_64-y = $(subst 
i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y))
+ifeq ($(CONFIG_VHOST_NET_TEST_i386),)
+check-qtest-x86_64-$(CONFIG_VHOST_NET_TEST_x86_64) += 
tests/vhost-user-test$(EXESUF)
+endif
 check-qtest-mips-y = tests/endianness-test$(EXESUF)
 check-qtest-mips64-y = tests/endianness-test$(EXESUF)
 check-qtest-mips64el-y = tests/endianness-test$(EXESUF)
-- 
2.5.0

[Qemu-devel] [PULL 2/2] virtio-9p: use QEMU thread pool

[Greg Kurz]

From: Paolo Bonzini 

The QEMU thread pool already has a mechanism to invoke callbacks in the main
thread.  It does not need an EventNotifier and it is more efficient too.
Use it instead of GAsyncQueue + GThreadPool + glue.

As a side effect, it silences Coverity's complaint about an unchecked
return value for event_notifier_init.

Signed-off-by: Paolo Bonzini 
Reviewed-by: Greg Kurz 
(removed no more needed #include  from virtio-9p-coth.h)
Signed-off-by: Greg Kurz 
---
 hw/9pfs/virtio-9p-coth.c   | 69 ++
 hw/9pfs/virtio-9p-coth.h   | 10 +--
 hw/9pfs/virtio-9p-device.c |  4 ---
 3 files changed, 15 insertions(+), 68 deletions(-)

diff --git a/hw/9pfs/virtio-9p-coth.c b/hw/9pfs/virtio-9p-coth.c
index 5057f8d220df..fb6e8f80e0f4 100644
--- a/hw/9pfs/virtio-9p-coth.c
+++ b/hw/9pfs/virtio-9p-coth.c
@@ -12,71 +12,30 @@
  *
  */
 
-#include "fsdev/qemu-fsdev.h"
-#include "qemu/thread.h"
-#include "qemu/event_notifier.h"
+#include "qemu-common.h"
+#include "block/thread-pool.h"
 #include "qemu/coroutine.h"
+#include "qemu/main-loop.h"
 #include "virtio-9p-coth.h"
 
-/* v9fs glib thread pool */
-static V9fsThPool v9fs_pool;
-
-void co_run_in_worker_bh(void *opaque)
+/* Called from QEMU I/O thread.  */
+static void coroutine_enter_cb(void *opaque, int ret)
 {
 Coroutine *co = opaque;
-g_thread_pool_push(v9fs_pool.pool, co, NULL);
-}
-
-static void v9fs_qemu_process_req_done(EventNotifier *e)
-{
-Coroutine *co;
-
-event_notifier_test_and_clear(e);
-
-while ((co = g_async_queue_try_pop(v9fs_pool.completed)) != NULL) {
-qemu_coroutine_enter(co, NULL);
-}
+qemu_coroutine_enter(co, NULL);
 }
 
-static void v9fs_thread_routine(gpointer data, gpointer user_data)
+/* Called from worker thread.  */
+static int coroutine_enter_func(void *arg)
 {
-Coroutine *co = data;
-
+Coroutine *co = arg;
 qemu_coroutine_enter(co, NULL);
-
-g_async_queue_push(v9fs_pool.completed, co);
-
-event_notifier_set(_pool.e);
+return 0;
 }
 
-int v9fs_init_worker_threads(void)
+void co_run_in_worker_bh(void *opaque)
 {
-int ret = 0;
-V9fsThPool *p = _pool;
-sigset_t set, oldset;
-
-sigfillset();
-/* Leave signal handling to the iothread.  */
-pthread_sigmask(SIG_SETMASK, , );
-
-p->pool = g_thread_pool_new(v9fs_thread_routine, p, -1, FALSE, NULL);
-if (!p->pool) {
-ret = -1;
-goto err_out;
-}
-p->completed = g_async_queue_new();
-if (!p->completed) {
-/*
- * We are going to terminate.
- * So don't worry about cleanup
- */
-ret = -1;
-goto err_out;
-}
-event_notifier_init(>e, 0);
-
-event_notifier_set_handler(>e, v9fs_qemu_process_req_done);
-err_out:
-pthread_sigmask(SIG_SETMASK, , NULL);
-return ret;
+Coroutine *co = opaque;
+thread_pool_submit_aio(qemu_get_aio_context()->thread_pool,
+   coroutine_enter_func, co, coroutine_enter_cb, co);
 }
diff --git a/hw/9pfs/virtio-9p-coth.h b/hw/9pfs/virtio-9p-coth.h
index 0fbe49a94615..4ac1aaf90292 100644
--- a/hw/9pfs/virtio-9p-coth.h
+++ b/hw/9pfs/virtio-9p-coth.h
@@ -18,14 +18,6 @@
 #include "qemu/thread.h"
 #include "qemu/coroutine.h"
 #include "virtio-9p.h"
-#include 
-
-typedef struct V9fsThPool {
-EventNotifier e;
-
-GThreadPool *pool;
-GAsyncQueue *completed;
-} V9fsThPool;
 
 /*
  * we want to use bottom half because we want to make sure the below
@@ -45,7 +37,7 @@ typedef struct V9fsThPool {
 qemu_bh_schedule(co_bh);\
 /*  \
  * yield in qemu thread and re-enter back   \
- * in glib worker thread\
+ * in worker thread \
  */ \
 qemu_coroutine_yield(); \
 qemu_bh_delete(co_bh);  \
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index e3abcfaffb2a..944b5f5e9fcc 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -116,10 +116,6 @@ static void virtio_9p_device_realize(DeviceState *dev, 
Error **errp)
" and export path:%s", s->fsconf.fsdev_id, s->ctx.fs_root);
 goto out;
 }
-if (v9fs_init_worker_threads() < 0) {
-error_setg(errp, "worker thread initialization failed");
-goto out;
-}
 
 /*
  * Check details of export path, We need to use fs driver
-- 
2.4.3

Re: [Qemu-devel] [PATCH v7 15/24] block: Remove BDS close notifier

[Kevin Wolf]

Am 09.11.2015 um 23:39 hat Max Reitz geschrieben:
> It is unused now, so we can remove it.
> 
> Signed-off-by: Max Reitz 

Reviewed-by: Kevin Wolf 

Re: [Qemu-devel] [Qemu-block] [PATCH v2 05/21] block: Consider all block layer options in append_open_options

[Alberto Garcia]

On Mon 23 Nov 2015 04:59:44 PM CET, Kevin Wolf wrote:
> The code already special-cased "node-name", which is currently the only
> option passed in the QDict that isn't driver-specific. Generalise the
> code to take all general block layer options into consideration.
>
> Signed-off-by: Kevin Wolf 
> Reviewed-by: Eric Blake 
> Reviewed-by: Max Reitz 

Reviewed-by: Alberto Garcia 

Berto

Re: [Qemu-devel] Could not add PCI device with big memory to aarch64 VMs

[liang yan]

On 11/04/2015 05:53 PM, Laszlo Ersek wrote:

On 11/04/15 23:22, liang yan wrote:

Hello, Laszlo,


(2)It also has a problem that once I use a memory bigger than 256M for
ivshmem, it could not get through UEFI,
the error message is

PciBus: Discovered PCI @ [00|01|00]
BAR[0]: Type =  Mem32; Alignment = 0xFFF;Length = 0x100; Offset =
0x10
BAR[1]: Type =  Mem32; Alignment = 0xFFF;Length = 0x1000; Offset
= 0x14
BAR[2]: Type = PMem64; Alignment = 0x3FFF;Length =
0x4000;Offset = 0x18

PciBus: HostBridge->SubmitResources() - Success
ASSERT
/home/liang/studio/edk2/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c(449): 
((BOOLEAN)(0==1))


I am wandering if there are memory limitation for pcie devices under
Qemu environment?


Just thank you in advance and any information would be appreciated.

(CC'ing Ard.)

"Apparently", the firmware-side counterpart of QEMU commit 5125f9cd2532
has never been contributed to edk2.

Therefore the the ProcessPciHost() function in
"ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c" ignores the
DTB_PCI_HOST_RANGE_MMIO64 type range from the DTB. (Thus only
DTB_PCI_HOST_RANGE_MMIO32 is recognized as PCI MMIO aperture.)

However, even if said driver was extended to parse the new 64-bit
aperture into PCDs (which wouldn't be hard), the
ArmVirtPkg/PciHostBridgeDxe driver would still have to be taught to look
at that aperture (from the PCDs) and to serve MMIO BAR allocation
requests from it. That could be hard.

Please check edk2 commits e48f1f15b0e2^..e5ceb6c9d390, approximately,
for the background on the current code. See also chapter 13 "Protocols -
PCI Bus Support" in the UEFI spec.

Patches welcome. :)

(A separate note on ACPI vs. DT: the firmware forwards *both* from QEMU
to the runtime guest OS. However, the firmware parses only the DT for
its own purposes.)

Hello, Laszlo,

Thanks for your advices above, it's very helpful.

When debugging, I also found some problems for 32 bit PCI devices.
Hope could get some clues from you.

I checked on 512M, 1G, and 2G devices.(4G return invalid parameter 
error, so I think it may be taken as a 64bit devices, is this right?).



First,

All devices start from base address 3EFE.

ProcessPciHost: Config[0x3F00+0x100) Bus[0x0..0xF] 
Io[0x0+0x1)@0x3EFF Mem[0x1000+0x2EFF)@0x0


PcdPciMmio32Base is  1000=
PcdPciMmio32Size is  2EFF=


Second,

It could not get new base address when searching memory space in GCD map.

For 512M devices,

*BaseAddress = (*BaseAddress + 1 - Length) & (~AlignmentMask);

BaseAddress is 3EFE==
new BaseAddress is 1EEF==
~AlignmentMask is E000==
Final BaseAddress is 

Status = CoreSearchGcdMapEntry (*BaseAddress, Length, , 
, Map);




For bigger devices:

all stops when searching memory space because below code, Length will 
bigger than MaxAddress(3EFE)


if ((Entry->BaseAddress + Length) > MaxAddress) {
 continue;
}


I also checked on ArmVirtQemu.dsc which all set to 0.

  gArmPlatformTokenSpaceGuid.PcdPciBusMin|0x0
  gArmPlatformTokenSpaceGuid.PcdPciBusMax|0x0
  gArmPlatformTokenSpaceGuid.PcdPciIoBase|0x0
  gArmPlatformTokenSpaceGuid.PcdPciIoSize|0x0
  gArmPlatformTokenSpaceGuid.PcdPciIoTranslation|0x0
  gArmPlatformTokenSpaceGuid.PcdPciMmio32Base|0x0
  gArmPlatformTokenSpaceGuid.PcdPciMmio32Size|0x0
  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0x0


Do you think I should change from PcdPciMmio32Base and PcdPciMmio32Size, 
or do some change for GCD entry list, so it could allocate resources for 
PCI devices(CoreSearchGcdMapEntry)?



Looking forward to your reply.


Thanks,
Liang


Thanks
Laszlo

Re: [Qemu-devel] SPI EEPROM device model

[Peter Crosthwaite]

On Mon, Nov 30, 2015 at 12:53 AM, Krzeminski, Marcin (Nokia -
PL/Wroclaw)  wrote:
> Hello,
>
> I need to write some SPI eeprom device model (probably AT25128B or AT93C56).
> I can not see any such device in qemu, but this time I want to ask before I 
> start to implement,
> if something that can speed up work is already present somewhere ?
> Maybe do you have some preferences which eeprom device is better to have in 
> qemu?
>

So some of those EEPROM devices are nearly identical to M25P80 in
functionality, the main difference being that individual cells can be
programmed from 0 back to 1. A quick look at a AT25128B datasheet
looks like this may be the case. Read and write instructions as the
same encoding as M25P80.

Search m25p80 for WR_1, which is a flag you can set that allows write
of 1 to individual bits. I think can become a feature of M25P80 (more
table entries) if the basic instructions are just an M25P80 subset.

HTH

Regards,
Peter

> Regards,
> Marcin

Re: [Qemu-devel] [PATCH 0/7] target-i386: MMReg struct cleanup

[Richard Henderson]

On 11/30/2015 10:21 AM, Eduardo Habkost wrote:

This is an attempt to cleanup the MMXReg/XMMReg structs and make
their names, fields and usage consistent.

The last 2 patches use a bit of macro magic to generate the union
definitions and ensure type safety when using the field helper
macros, and I am sending them as RFCs.


It all looks ok, as far as it goes.

Though I wonder if it wouldn't be better to reorg everything to work akin to 
the aarch64 method -- process everything in 64-bit chunks, via TCG registers, 
rather than via pointers to structures.


That would much easier let mmx and sse (and avx2 and avx512) use the same set 
of helpers, with much less need for all this macro-ization in the first place.



r~

Re: [Qemu-devel] [Qemu-block] [PATCH for-2.5] blkdebug: silence warning under qtest

[Max Reitz]

On 30.11.2015 12:44, Michael S. Tsirkin wrote:
> make check always outputs warnings, this
> is not nice.  Disable blkdebug warnings under qtest.
> 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  block/blkdebug.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)

Thanks, applied to my block tree:

https://github.com/XanClic/qemu/commits/block

Max



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v2 2/3] target-i386: Use xsave structs for ext_save_area

[Eduardo Habkost]

This doesn't introduce any change in the code, as the offsets and
struct sizes match what was present in the table. This can be
validated by the QEMU_BUILD_BUG_ON lines on target-i386/cpu.h,
which ensures the struct sizes and offsets match the existing
values in ext_save_area.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 11e5e39..bc95437 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -458,17 +458,23 @@ typedef struct ExtSaveArea {
 
 static const ExtSaveArea ext_save_areas[] = {
 [2] = { .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX,
-.offset = 0x240, .size = 0x100 },
+.offset = offsetof(X86XSaveArea, avx_state),
+.size = sizeof(XSaveAVX) },
 [3] = { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_MPX,
-.offset = 0x3c0, .size = 0x40  },
+.offset = offsetof(X86XSaveArea, bndreg_state),
+.size = sizeof(XSaveBNDREG)  },
 [4] = { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_MPX,
-.offset = 0x400, .size = 0x40  },
+.offset = offsetof(X86XSaveArea, bndcsr_state),
+.size = sizeof(XSaveBNDCSR)  },
 [5] = { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F,
-.offset = 0x440, .size = 0x40 },
+.offset = offsetof(X86XSaveArea, opmask_state),
+.size = sizeof(XSaveOpmask) },
 [6] = { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F,
-.offset = 0x480, .size = 0x200 },
+.offset = offsetof(X86XSaveArea, zmm_hi256_state),
+.size = sizeof(XSaveZMM_Hi256) },
 [7] = { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F,
-.offset = 0x680, .size = 0x400 },
+.offset = offsetof(X86XSaveArea, hi16_zmm_state),
+.size = sizeof(XSaveHi16_ZMM) },
 };
 
 const char *get_register_name_32(unsigned int reg)
-- 
2.1.0

[Qemu-devel] [PATCH v2 3/3] target-i386: kvm: Use X86XSaveArea struct for xsave save/load

[Eduardo Habkost]

Instead of using offset macros and bit operations in a uint32_t
array, use the X86XSaveArea struct to perform the loading/saving
operations in kvm_put_xsave() and kvm_get_xsave().

Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
* Use uint8_t pointers when loading/saving xmm, ymmh, zmmh,
  keeping the same load/save logic from previous code
* Keep the QEMU_BUILD_BUG_ON lines
---
 target-i386/kvm.c | 74 +++
 1 file changed, 36 insertions(+), 38 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index b8b336b..98249e4 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1243,9 +1243,8 @@ ASSERT_OFFSET(XSAVE_Hi16_ZMM, hi16_zmm_state);
 static int kvm_put_xsave(X86CPU *cpu)
 {
 CPUX86State *env = >env;
-struct kvm_xsave* xsave = env->kvm_xsave_buf;
+X86XSaveArea *xsave = env->kvm_xsave_buf;
 uint16_t cwd, swd, twd;
-uint8_t *xmm, *ymmh, *zmmh;
 int i, r;
 
 if (!has_xsave) {
@@ -1260,25 +1259,26 @@ static int kvm_put_xsave(X86CPU *cpu)
 for (i = 0; i < 8; ++i) {
 twd |= (!env->fptags[i]) << i;
 }
-xsave->region[XSAVE_FCW_FSW] = (uint32_t)(swd << 16) + cwd;
-xsave->region[XSAVE_FTW_FOP] = (uint32_t)(env->fpop << 16) + twd;
-memcpy(>region[XSAVE_CWD_RIP], >fpip, sizeof(env->fpip));
-memcpy(>region[XSAVE_CWD_RDP], >fpdp, sizeof(env->fpdp));
-memcpy(>region[XSAVE_ST_SPACE], env->fpregs,
+xsave->legacy.fcw = cwd;
+xsave->legacy.fsw = swd;
+xsave->legacy.ftw = twd;
+xsave->legacy.fpop = env->fpop;
+xsave->legacy.fpip = env->fpip;
+xsave->legacy.fpdp = env->fpdp;
+memcpy(>legacy.fpregs, env->fpregs,
 sizeof env->fpregs);
-xsave->region[XSAVE_MXCSR] = env->mxcsr;
-*(uint64_t *)>region[XSAVE_XSTATE_BV] = env->xstate_bv;
-memcpy(>region[XSAVE_BNDREGS], env->bnd_regs,
+xsave->legacy.mxcsr = env->mxcsr;
+xsave->header.xstate_bv = env->xstate_bv;
+memcpy(>bndreg_state.bnd_regs, env->bnd_regs,
 sizeof env->bnd_regs);
-memcpy(>region[XSAVE_BNDCSR], >bndcs_regs,
-sizeof(env->bndcs_regs));
-memcpy(>region[XSAVE_OPMASK], env->opmask_regs,
+xsave->bndcsr_state.bndcsr = env->bndcs_regs;
+memcpy(>opmask_state.opmask_regs, env->opmask_regs,
 sizeof env->opmask_regs);
 
-xmm = (uint8_t *)>region[XSAVE_XMM_SPACE];
-ymmh = (uint8_t *)>region[XSAVE_YMMH_SPACE];
-zmmh = (uint8_t *)>region[XSAVE_ZMM_Hi256];
-for (i = 0; i < CPU_NB_REGS; i++, xmm += 16, ymmh += 16, zmmh += 32) {
+for (i = 0; i < CPU_NB_REGS; i++) {
+uint8_t *xmm = xsave->legacy.xmm_regs[i];
+uint8_t *ymmh = xsave->avx_state.ymmh[i];
+uint8_t *zmmh = xsave->zmm_hi256_state.zmm_hi256[i];
 stq_p(xmm, env->xmm_regs[i].XMM_Q(0));
 stq_p(xmm+8,   env->xmm_regs[i].XMM_Q(1));
 stq_p(ymmh,env->xmm_regs[i].XMM_Q(2));
@@ -1290,7 +1290,7 @@ static int kvm_put_xsave(X86CPU *cpu)
 }
 
 #ifdef TARGET_X86_64
-memcpy(>region[XSAVE_Hi16_ZMM], >xmm_regs[16],
+memcpy(>hi16_zmm_state.hi16_zmm, >xmm_regs[16],
 16 * sizeof env->xmm_regs[16]);
 #endif
 r = kvm_vcpu_ioctl(CPU(cpu), KVM_SET_XSAVE, xsave);
@@ -1626,9 +1626,8 @@ static int kvm_get_fpu(X86CPU *cpu)
 static int kvm_get_xsave(X86CPU *cpu)
 {
 CPUX86State *env = >env;
-struct kvm_xsave* xsave = env->kvm_xsave_buf;
+X86XSaveArea *xsave = env->kvm_xsave_buf;
 int ret, i;
-const uint8_t *xmm, *ymmh, *zmmh;
 uint16_t cwd, swd, twd;
 
 if (!has_xsave) {
@@ -1640,33 +1639,32 @@ static int kvm_get_xsave(X86CPU *cpu)
 return ret;
 }
 
-cwd = (uint16_t)xsave->region[XSAVE_FCW_FSW];
-swd = (uint16_t)(xsave->region[XSAVE_FCW_FSW] >> 16);
-twd = (uint16_t)xsave->region[XSAVE_FTW_FOP];
-env->fpop = (uint16_t)(xsave->region[XSAVE_FTW_FOP] >> 16);
+cwd = xsave->legacy.fcw;
+swd = xsave->legacy.fsw;
+twd = xsave->legacy.ftw;
+env->fpop = xsave->legacy.fpop;
 env->fpstt = (swd >> 11) & 7;
 env->fpus = swd;
 env->fpuc = cwd;
 for (i = 0; i < 8; ++i) {
 env->fptags[i] = !((twd >> i) & 1);
 }
-memcpy(>fpip, >region[XSAVE_CWD_RIP], sizeof(env->fpip));
-memcpy(>fpdp, >region[XSAVE_CWD_RDP], sizeof(env->fpdp));
-env->mxcsr = xsave->region[XSAVE_MXCSR];
-memcpy(env->fpregs, >region[XSAVE_ST_SPACE],
+env->fpip = xsave->legacy.fpip;
+env->fpdp = xsave->legacy.fpdp;
+env->mxcsr = xsave->legacy.mxcsr;
+memcpy(env->fpregs, >legacy.fpregs,
 sizeof env->fpregs);
-env->xstate_bv = *(uint64_t *)>region[XSAVE_XSTATE_BV];
-memcpy(env->bnd_regs, >region[XSAVE_BNDREGS],
+env->xstate_bv = xsave->header.xstate_bv;
+memcpy(env->bnd_regs, >bndreg_state.bnd_regs,
 sizeof env->bnd_regs);
-memcpy(>bndcs_regs, >region[XSAVE_BNDCSR],
-sizeof(env->bndcs_regs));
-memcpy(env->opmask_regs, >region[XSAVE_OPMASK],
+

Re: [Qemu-devel] [PATCH v7 24/24] iotests: Add test for block jobs and BDS ejection

[Max Reitz]

On 30.11.2015 17:23, Kevin Wolf wrote:
> Am 09.11.2015 um 23:39 hat Max Reitz geschrieben:
>> Suggested-by: Paolo Bonzini 
>> Signed-off-by: Max Reitz 
>> ---
>>  tests/qemu-iotests/141 | 166 
>> +
>>  tests/qemu-iotests/141.out |  47 +
>>  tests/qemu-iotests/group   |   1 +
>>  3 files changed, 214 insertions(+)
>>  create mode 100755 tests/qemu-iotests/141
>>  create mode 100644 tests/qemu-iotests/141.out
>>
>> diff --git a/tests/qemu-iotests/141 b/tests/qemu-iotests/141
>> new file mode 100755
>> index 000..6a32d56
>> --- /dev/null
>> +++ b/tests/qemu-iotests/141
>> @@ -0,0 +1,166 @@
>> +#!/bin/bash
>> +#
>> +# Test case for ejecting BDSs with block jobs still running on them
>> +#
>> +# Copyright (C) 2015 Red Hat, Inc.
>> +#
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 2 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see .
>> +#
>> +
>> +# creator
>> +owner=mre...@redhat.com
>> +
>> +seq="$(basename $0)"
>> +echo "QA output created by $seq"
>> +
>> +here="$PWD"
>> +tmp=/tmp/$$
>> +status=1# failure is the default!
>> +
>> +_cleanup()
>> +{
>> +_cleanup_test_img
>> +rm -f "$TEST_DIR/{b,o}.$IMGFMT"
>> +}
>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>> +
>> +# get standard environment, filters and checks
>> +. ./common.rc
>> +. ./common.filter
>> +. ./common.qemu
>> +
>> +# Needs backing file support
>> +_supported_fmt qcow qcow2 qed
> 
> The test doesn't work for me on qcow1.

Hm, and I thought I had tested it. Well, block jobs creating an overlay
file not being supported on qcow1 is probably all right.

>> +echo
>> +echo '=== Testing block-commit ==='
>> +echo
>> +
>> +# block-commit will send BLOCK_JOB_READY basically immediately, and 
>> cancelling
>> +# the job will consequently result in BLOCK_JOB_COMPLETED being emitted.
>> +
>> +test_blockjob \
>> +"{'execute': 'block-commit',
>> +  'arguments': {'device': 'drv0'}}" \
>> +'BLOCK_JOB_READY' \
>> +'BLOCK_JOB_COMPLETED'
> 
> This is commit of the active layer, i.e. just a mirror in disguise.
> Should we test a "real" commit block job as well?

Well, the op blocker we are testing is set by block_job_create(), so a
single block job would have sufficed. But now that I'm trying to test
them all, there's no reason not to test the real commit job, too.

> Anyway, with qcow1 removed from the list:
> Reviewed-by: Kevin Wolf 

Thanks!

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 2/2] hmp: add support for system_suspend

[Eric Blake]

On 11/27/2015 08:01 PM, Chen Hanxiao wrote:
> From: Chen Hanxiao 
> 
> This patch add support for system_suspend hmp command.
> 
> Signed-off-by: Chen Hanxiao 
> ---
>  hmp-commands.hx | 14 ++
>  hmp.c   |  5 +
>  hmp.h   |  1 +
>  3 files changed, 20 insertions(+)
> 

> +++ b/hmp.c
> @@ -885,6 +885,11 @@ void hmp_system_powerdown(Monitor *mon, const QDict 
> *qdict)
>  qmp_system_powerdown(NULL);
>  }
>  
> +void hmp_system_suspend(Monitor *mon, const QDict *qdict)
> +{
> +qmp_system_suspend(NULL);

Why are you ignoring any potential errors?  Wouldn't it be better to
report an error, if one occurs?  Or at least pass _abort to assert
that no error can occur (matching the current implementation in patch
1/2 that never fails).

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH 6/7] [RFC] target-i386: Define MMREG_UNION macro

[Eduardo Habkost]

This will simplify the definitions of ZMMReg and MMXReg.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.h | 27 +++
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 7519023..b189748 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -725,23 +725,18 @@ typedef struct SegmentCache {
 uint32_t flags;
 } SegmentCache;
 
-typedef union {
-uint8_t _b[64];
-uint16_t _w[32];
-uint32_t _l[16];
-uint64_t _q[8];
-float32 _s[16];
-float64 _d[8];
-} ZMMReg;
+#define MMREG_UNION(q)  \
+union { \
+uint8_t _b[(q)*8];  \
+uint16_t _w[(q)*4]; \
+uint32_t _l[(q)*2]; \
+uint64_t _q[(q)];   \
+float32 _s[(q)*2];  \
+float64 _d[(q)];\
+}
 
-typedef union {
-uint8_t _b[8];
-uint16_t _w[4];
-uint32_t _l[2];
-uint64_t _q[1];
-float32 _s[2];
-float64 _d[1];
-} MMXReg;
+typedef MMREG_UNION(8) ZMMReg;
+typedef MMREG_UNION(1) MMXReg;
 
 typedef struct BNDReg {
 uint64_t lb;
-- 
2.1.0

Re: [Qemu-devel] [PATCH v2 5/8] dump-query: add "dump-query" command to query dump status

[Eric Blake]

On 11/26/2015 07:48 PM, Peter Xu wrote:
> This patch is only adding the QMP/HMP interface for "dump-query"
> command, but not implementing them. This command could be used to
> query background dump status. Please refer to the next patch to see
> how dump status are defined.
> 
> Currently, only fake results are returned.

Feels a bit awkward to return fake results; maybe you should squash some
patches together or fail with an error instead of returning fake results.

> 
> Signed-off-by: Peter Xu 
> ---
>  dump.c   |  9 +
>  hmp-commands.hx  | 15 +++
>  hmp.c|  6 ++
>  hmp.h|  1 +
>  qapi-schema.json | 21 +
>  qmp-commands.hx  | 29 -
>  6 files changed, 80 insertions(+), 1 deletion(-)
> 

> +++ b/qapi-schema.json
> @@ -2139,6 +2139,27 @@
>  '*format': 'DumpGuestMemoryFormat'} }
>  
>  ##
> +# @DumpStatus
> +#
> +# Status for the last guest memory dump.
> +#

Missing documentation for 'status' and 'percentage' fields.

> +# Since: 2.6
> +##
> +{ 'struct': 'DumpStatus',
> +  'data': { 'status': 'str', 'percentage': 'str' } }

What values will 'status' contain?  If it is a finite set of status
strings, then it should be an enum type.

'percentage' should NOT be a string.  It should probably be numeric;
'number' if you intend to return a floating point value between 0 and 1.
 Or, like other interfaces, you should probably return two numbers
(current and total, both 'int'), and let the caller compute percentage
themselves.

> +
> +##
> +# @dump-query

Most query commands are named 'query-FOO', not 'FOO-query'.  Unless you
have a compelling reason otherwise, this should be 'query-dump'.

> +#
> +# Query latest dump status.
> +#
> +# Returns: A @DumpStatus object showing the dump status.
> +#
> +# Since: 2.6
> +##
> +{ 'command': 'dump-query', 'returns': 'DumpStatus' }
> +

> +SQMP
> +dump-query
> +--
> +
> +Query background dump status.
> +
> +Arguments: None.
> +
> +Example:
> +
> +-> { "execute": "dump-query" }
> +<- { "return": {"status": "IN_PROGRESS", "percentage": "85%" } }

ALL_CAPS status is annoying to read; if you add an enum type, it should
be 'lower-case' values.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 1/2] qmp: add support for system_suspend

[Eric Blake]

On 11/27/2015 08:01 PM, Chen Hanxiao wrote:
> From: Chen Hanxiao 
> 
> This patch add support for system_suspend qmp command.
> 
> Signed-off-by: Chen Hanxiao 
> ---
>  qapi-schema.json |  9 +
>  qmp-commands.hx  | 21 +
>  qmp.c|  5 +
>  3 files changed, 35 insertions(+)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 8b1a423..78bbb29 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3971,3 +3971,12 @@
>  ##
>  { 'enum': 'ReplayMode',
>'data': [ 'none', 'record', 'play' ] }
> +
> +##
> +# @system_suspend:
> +#
> +# Performs suspend operation of a guest.
> +#
> +# Since: 2.5
> +##
> +{ 'command': 'system_suspend' }

You've missed 2.5; this should be since 2.6.  Also, new QMP commands
should be named with '-' rather than '_'; so this should be
'system-suspend'.  (Yes, I know 'system_wakeup' already exists with the
older spelling).

How does this command differ from the existing ability to use
qemu-guest-agent to request the guest put itself into suspend state?

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [Qemu-ppc] [PATCH 00/77] ppc: Add "native" POWER8 platform

[Cédric Le Goater]

On 11/28/2015 08:59 AM, Benjamin Herrenschmidt wrote:
> On Fri, 2015-11-27 at 11:21 +0100, Alexander Graf wrote:
>>
>> How does real hardware store petitboot? If it's flash, you could pass it
>> in using -pflash and thus model things even more closely and allow users
>> to just take the ROM image as is.
> 
> It is a flash image, we could use an Open Power machine flash image "as-is"
> provided we taught qemu to extract skiboot (aka OPAL) from it.

Couldn't we add an offset argument to load_image_targphys() or make that 
an extra routine ? If so, we could then load directly from an openpower 
pnor file. 

I gave it a quick (and dirty) try and a powernv guest runs fine up to 
petitboot with just :

qemu-system-ppc64 -m 2G -M powernv -bios  
~/work/open-power/images/palmetto.pnor -nographic -nodefaults -serial stdio

The pnor file is compiled from github. The patch is below (without the dirty
cut and paste I did in loader.c). The offset for the PAYLOAD and BOOTKERNEL
partitions are hard coded but I guess we don't need to read the flash partition
table in qemu, not yet.
 

Cheers,

C. 


Index: qemu-powernv.git/hw/ppc/pnv.c
===
--- qemu-powernv.git.orig/hw/ppc/pnv.c
+++ qemu-powernv.git/hw/ppc/pnv.c
@@ -69,7 +69,7 @@
 
 #define FDT_ADDR0x0100
 #define FDT_MAX_SIZE0x0010
-#define FW_MAX_SIZE 0x0040
+#define FW_MAX_SIZE 0x0400
 #define FW_FILE_NAME"skiboot.lid"
 #define KERNEL_FILE_NAME"skiroot.lid"
 #define KERNEL_LOAD_ADDR0x2000
@@ -902,7 +902,9 @@ static void ppc_powernv_init(MachineStat
 {
 ram_addr_t ram_size = machine->ram_size;
 const char *cpu_model = machine->cpu_model;
+#if 0
 const char *kernel_filename = machine->kernel_filename;
+#endif
 const char *initrd_filename = machine->initrd_filename;
 uint32_t initrd_base = 0;
 long initrd_size = 0;
@@ -998,19 +1000,20 @@ static void ppc_powernv_init(MachineStat
 bios_name = FW_FILE_NAME;
 }
 filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
-fw_size = load_image_targphys(filename, 0, FW_MAX_SIZE);
+fw_size = load_image_targphys_offset(filename, 0, FW_MAX_SIZE, 0x961000);
 if (fw_size < 0) {
 hw_error("qemu: could not load OPAL '%s'\n", filename);
 exit(1);
 }
+#if 0
 g_free(filename);
 
 if (kernel_filename == NULL) {
 kernel_filename = KERNEL_FILE_NAME;
 }
 filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, kernel_filename);
-fw_size = load_image_targphys(filename, 0x2000, 0x200);
+#endif
+fw_size = load_image_targphys_offset(filename, 0x2000, 0x200, 
0xa61000);
 if (fw_size < 0) {
 hw_error("qemu: could not load kernel'%s'\n", filename);
 exit(1);

Re: [Qemu-devel] [PATCH v2 4/8] dump-guest-memory: add qmp event DUMP_COMPLETED

[Eric Blake]

On 11/26/2015 07:48 PM, Peter Xu wrote:
> To get aligned with QMP interface, one new QMP event DUMP_COMPLETED
> is added. It is used when user specified "detach" in dump, and
> triggered when the dump finishes. Error message will be appended to
> this event if the dump has failed.

Why not emit the new event unconditionally, instead of only when detach
was specified?

> 
> Signed-off-by: Peter Xu 
> ---
>  docs/qmp-events.txt | 12 
>  dump.c  | 12 +++-
>  qapi/event.json | 10 ++
>  3 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/qmp-events.txt b/docs/qmp-events.txt
> index d2f1ce4..fe494f9 100644
> --- a/docs/qmp-events.txt
> +++ b/docs/qmp-events.txt
> @@ -674,3 +674,15 @@ Note: If action is "reset", "shutdown", or "pause" the 
> WATCHDOG event is
>  followed respectively by the RESET, SHUTDOWN, or STOP events.
>  
>  Note: this event is rate-limited.
> +
> +DUMP_COMPLETED
> +--
> +
> +Emitted when the guest has finished one memory dump.
> +
> +Data: None.

Wrong - you have 'msg' as data.  Except that Paolo is right, 'msg'
should be optional, and only present on error.  You should also document
that the contents of 'msg' are for human consumption and should not be
machine-parsed (basically, only the presence of absence of 'msg' is
useful for machines).

> +
> +Example:
> +
> +{ "event": "DUMP_COMPLETED",
> +  "data": { "msg": "Dump completed successfully" } }
> diff --git a/dump.c b/dump.c

> +++ b/qapi/event.json
> @@ -356,3 +356,13 @@
>  ##
>  { 'event': 'MEM_UNPLUG_ERROR',
>'data': { 'device': 'str', 'msg': 'str' } }
> +
> +##
> +# @DUMP_COMPLETED
> +#
> +# Emitted when background dump has completed
> +#

Missing documentation of 'msg', which should be optional.

> +# Since: 2.6
> +##
> +{ 'event': 'DUMP_COMPLETED' ,
> +  'data': { 'msg': 'str' } }
> 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 2/8] dump-guest-memory: add "detach" flag for QMP/HMP interfaces.

[Eric Blake]

On 11/26/2015 07:48 PM, Peter Xu wrote:
> This patch only adds the interfaces, but not implements them.

s/not implements/does not implement/

> "detach" parameter is made optional, to make sure that all the old
> dump-guest-memory requests will still be able to work.
> 
> Signed-off-by: Peter Xu 
> ---

In addition to Fam's comments,

> +++ b/qapi-schema.json
> @@ -2115,6 +2115,9 @@
>  #2. fd: the protocol starts with "fd:", and the following string
>  #   is the fd's name.
>  #
> +# @detach: #optional if true, QMP will return immediately rather than
> +#  waiting dump to be finished (since 2.6).

s/waiting/waiting for the/
s/be finished/finish/

> +++ b/qmp-commands.hx
> @@ -840,8 +840,8 @@ EQMP
>  
>  {
>  .name   = "dump-guest-memory",
> -.args_type  = "paging:b,protocol:s,begin:i?,end:i?,format:s?",
> -.params = "-p protocol [begin] [length] [format]",
> +.args_type  = 
> "paging:b,protocol:s,detach:b?,begin:i?,end:i?,format:s?",
> +.params = "-p protocol [-d] [begin] [length] [format]",
>  .help   = "dump guest memory to file",
>  .mhandler.cmd_new = qmp_marshal_dump_guest_memory,
>  },
> @@ -857,6 +857,8 @@ Arguments:
>  - "paging": do paging to get guest's memory mapping (json-bool)
>  - "protocol": destination file(started with "file:") or destination file
>descriptor (started with "fd:") (json-string)
> +- "detach": if specificed, command will return immediately, without waiting

s/specificed/specified/

> +for dump to be finished (json-bool)

s/dump to be finished/the dump to finish/

>  - "begin": the starting physical address. It's optional, and should be 
> specified
> with length together (json-int)
>  - "length": the memory size, in bytes. It's optional, and should be specified
> 

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH 1/7] target-i386/ops_sse.h: Use MMX_Q macro

[Eduardo Habkost]

We have a MMX_Q macro in addition to MMX_{B,W,L}. Use it.

Signed-off-by: Eduardo Habkost 
---
 target-i386/ops_sse.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-i386/ops_sse.h b/target-i386/ops_sse.h
index 1780d1d..52ec0b0 100644
--- a/target-i386/ops_sse.h
+++ b/target-i386/ops_sse.h
@@ -26,7 +26,7 @@
 #define B(n) MMX_B(n)
 #define W(n) MMX_W(n)
 #define L(n) MMX_L(n)
-#define Q(n) q
+#define Q(n) MMX_Q(n)
 #define SUFFIX _mmx
 #else
 #define Reg XMMReg
-- 
2.1.0

[Qemu-devel] [PATCH 2/7] target-i386: Use a _q array on MMXReg too

[Eduardo Habkost]

Make MMXReg use the same field names used on XMMReg, so we can
try to reuse macros and other code later.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 84edfd0..154891e 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -739,7 +739,7 @@ typedef union {
 uint16_t _w[4];
 uint32_t _l[2];
 float32 _s[2];
-uint64_t q;
+uint64_t _q[1];
 } MMXReg;
 
 typedef struct BNDReg {
@@ -777,7 +777,7 @@ typedef struct BNDCSReg {
 #define MMX_L(n) _l[n]
 #define MMX_S(n) _s[n]
 #endif
-#define MMX_Q(n) q
+#define MMX_Q(n) _q[n]
 
 typedef union {
 floatx80 d __attribute__((aligned(16)));
-- 
2.1.0

[Qemu-devel] [PATCH 5/7] target-i386: Define MMXReg._d field

[Eduardo Habkost]

Add a new field and reorder MMXReg fields, to make MMXReg and
ZMMReg field lists look the same (except for the array sizes).

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 614197f..7519023 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -738,8 +738,9 @@ typedef union {
 uint8_t _b[8];
 uint16_t _w[4];
 uint32_t _l[2];
-float32 _s[2];
 uint64_t _q[1];
+float32 _s[2];
+float64 _d[1];
 } MMXReg;
 
 typedef struct BNDReg {
-- 
2.1.0

[Qemu-devel] [PATCH 7/7] [RFC] target-i386: Add suffixes to MMReg struct fields

[Eduardo Habkost]

This will ensure we never use the MMX_* and ZMM_* macros with the
wrong struct type.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.h | 66 +++
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index b189748..6eeac16 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -725,18 +725,18 @@ typedef struct SegmentCache {
 uint32_t flags;
 } SegmentCache;
 
-#define MMREG_UNION(q)  \
-union { \
-uint8_t _b[(q)*8];  \
-uint16_t _w[(q)*4]; \
-uint32_t _l[(q)*2]; \
-uint64_t _q[(q)];   \
-float32 _s[(q)*2];  \
-float64 _d[(q)];\
+#define MMREG_UNION(n, q)   \
+union n {   \
+uint8_t _b_##n[(q)*8];  \
+uint16_t _w_##n[(q)*4]; \
+uint32_t _l_##n[(q)*2]; \
+uint64_t _q_##n[(q)];   \
+float32 _s_##n[(q)*2];  \
+float64 _d_##n[(q)];\
 }
 
-typedef MMREG_UNION(8) ZMMReg;
-typedef MMREG_UNION(1) MMXReg;
+typedef MMREG_UNION(ZMMReg, 8) ZMMReg;
+typedef MMREG_UNION(MMXReg, 1) MMXReg;
 
 typedef struct BNDReg {
 uint64_t lb;
@@ -749,31 +749,31 @@ typedef struct BNDCSReg {
 } BNDCSReg;
 
 #ifdef HOST_WORDS_BIGENDIAN
-#define ZMM_B(n) _b[63 - (n)]
-#define ZMM_W(n) _w[31 - (n)]
-#define ZMM_L(n) _l[15 - (n)]
-#define ZMM_S(n) _s[15 - (n)]
-#define ZMM_Q(n) _q[7 - (n)]
-#define ZMM_D(n) _d[7 - (n)]
-
-#define MMX_B(n) _b[7 - (n)]
-#define MMX_W(n) _w[3 - (n)]
-#define MMX_L(n) _l[1 - (n)]
-#define MMX_S(n) _s[1 - (n)]
+#define ZMM_B(n) _b_ZMMReg[63 - (n)]
+#define ZMM_W(n) _w_ZMMReg[31 - (n)]
+#define ZMM_L(n) _l_ZMMReg[15 - (n)]
+#define ZMM_S(n) _s_ZMMReg[15 - (n)]
+#define ZMM_Q(n) _q_ZMMReg[7 - (n)]
+#define ZMM_D(n) _d_ZMMReg[7 - (n)]
+
+#define MMX_B(n) _b_MMXReg[7 - (n)]
+#define MMX_W(n) _w_MMXReg[3 - (n)]
+#define MMX_L(n) _l_MMXReg[1 - (n)]
+#define MMX_S(n) _s_MMXReg[1 - (n)]
 #else
-#define ZMM_B(n) _b[n]
-#define ZMM_W(n) _w[n]
-#define ZMM_L(n) _l[n]
-#define ZMM_S(n) _s[n]
-#define ZMM_Q(n) _q[n]
-#define ZMM_D(n) _d[n]
-
-#define MMX_B(n) _b[n]
-#define MMX_W(n) _w[n]
-#define MMX_L(n) _l[n]
-#define MMX_S(n) _s[n]
+#define ZMM_B(n) _b_ZMMReg[n]
+#define ZMM_W(n) _w_ZMMReg[n]
+#define ZMM_L(n) _l_ZMMReg[n]
+#define ZMM_S(n) _s_ZMMReg[n]
+#define ZMM_Q(n) _q_ZMMReg[n]
+#define ZMM_D(n) _d_ZMMReg[n]
+
+#define MMX_B(n) _b_MMXReg[n]
+#define MMX_W(n) _w_MMXReg[n]
+#define MMX_L(n) _l_MMXReg[n]
+#define MMX_S(n) _s_MMXReg[n]
 #endif
-#define MMX_Q(n) _q[n]
+#define MMX_Q(n) _q_MMXReg[n]
 
 typedef union {
 floatx80 d __attribute__((aligned(16)));
-- 
2.1.0