Re: [Qemu-devel] [PATCH v5 0/5] fw_cfg: add DMA operations & etc/vmcoreinfo support

[Hatayama, Daisuke]

Marc-Andre,

It looks to me that the 4th and 5th patches somehow has not been sent.
Could you send them, too?
I'd like them to actually build and run the kernel for testing.

> -Original Message-
> From: linux-kernel-ow...@vger.kernel.org
> [mailto:linux-kernel-ow...@vger.kernel.org] On Behalf Of Marc-Andre Lureau
> Sent: Wednesday, November 8, 2017 1:24 AM
> To: linux-ker...@vger.kernel.org
> Cc: so...@cmu.edu; qemu-devel@nongnu.org; m...@redhat.com; Marc-André Lureau
> 
> Subject: [PATCH v5 0/5] fw_cfg: add DMA operations & etc/vmcoreinfo support
> 
> Hi,
> 
> This series adds DMA operations support to the qemu fw_cfg kernel
> module and populates "etc/vmcoreinfo" with vmcoreinfo location
> details.
> 
> Note: the support for this entry handling has been merged for next
> qemu release (2.11)
> 
> v5:
> - resent to CC kdump people on the paddr_vmcoreinfo_note() export patch
> 
> v4:
> - export paddr_vmcoreinfo_note() to fix fw_cfg.ko build
> - fix build with !CONFIG_CRASH_CORE
> - replace the unbounded yield() loop with a usleep_range() loop and a
>   200ms timeout
> - do not write vmcoreinfo entry when running the kdump kernel (D. Hatayama)
> - drop the experimental sysfs write support patch from this series
> 
> v3: (thanks kbuild)
> - add "fw_cfg: fix the command line module name" patch
> - fix build of "fw_cfg: add DMA register" with CONFIG_FW_CFG_SYSFS_CMDLINE=y
> - fix 'Wshift-count-overflow'
> 
> v2:
> - use platform device for dma mapping
> - add etc/vmcoreinfo patch
> - some code cleanups
> 
> Marc-André Lureau (5):
>   fw_cfg: fix the command line module name
>   fw_cfg: add DMA register
>   fw_cfg: do DMA read operation
>   crash: export paddr_vmcoreinfo_note()
>   fw_cfg: write vmcoreinfo details
> 
>  drivers/firmware/qemu_fw_cfg.c | 292
> +
>  kernel/crash_core.c|   1 +
>  2 files changed, 264 insertions(+), 29 deletions(-)
> 
> --
> 2.15.0.125.g8f49766d64
> 
> 

Re: [Qemu-devel] [v21 RESEND 1/2] virtio-crypto: Add virtio crypto device specification

[Longpeng (Mike)]

Hi Stefan,

On 2017/11/11 1:02, Stefan Hajnoczi wrote:

> On Mon, Nov 06, 2017 at 02:58:47PM +0800, Longpeng(Mike) wrote:
>> From: Gonglei 
>>
>> The virtio crypto device is a virtual crypto device (ie. hardware
>> crypto accelerator card). Currently, the virtio crypto device provides
>> the following crypto services: CIPHER, MAC, HASH, and AEAD.
>>
>> In this patch, CIPHER, MAC, HASH, AEAD services are introduced.
>>
>> VIRTIO-153
>>
>> Signed-off-by: Gonglei 
>> Signed-off-by: Longpeng(Mike) 
> 
> This doesn't build cleanly for me on commit
> 2faf9b0666886abc0ca99f6813446a4e2a04f3ca
> (https://github.com/oasis-tcs/virtio-spec master):
> 
>   $ 
>   $ ./makeall.sh
>   ! LaTeX Error: Lonely \item--perhaps a missing list environment.
> 
> See the LaTeX manual or LaTeX Companion for explanation.
> Type  H   for immediate help.
>  ...
> 
> l.384 \item I
>  f the VIRTIO_CRYPTO_F_MUX_MODE feature bit is NOT negotiated, the
> ?
> 

Ah yes, I'll fix it.

>> diff --git a/acknowledgements.tex b/acknowledgements.tex
>> index 6c86d12..c4b6844 100644
>> --- a/acknowledgements.tex
>> +++ b/acknowledgements.tex
>> @@ -26,6 +26,8 @@ Sasha Levin,   Oracle  \newline
>>  Sergey Tverdyshev,  Thales e-Security   \newline
>>  Stefan Hajnoczi,Red Hat \newline
>>  Tom Lyon,   Samya Systems, Inc. \newline
>> +Lei Gong,   Huawei  \newline
>> +Peng Long,  Huawei  \newline
>>  \end{oasistitlesection}
>>  
>>  The following non-members have provided valuable feedback on this
>> @@ -43,4 +45,5 @@ Laura Novich, Red Hat  \newline
>>  Patrick Durusau,Technical Advisory Board, OASIS \newline
>>  Thomas Huth,IBM \newline
>>  Yan Vugenfirer, Red Hat / Daynix\newline
>> +Halil Pasic,IBM \newline
>>  \end{oasistitlesection}
> 
> These lists of names are in alphabetical order.  Please insert names at
> the appropriate place.
> 

OK.

>> +If VIRTIO_CRYPTO_F_MUX_MODE is negotioated the device may support both 
>> session mode
> 
> s/negotioated/negotiated/

OK. I'll fix some other typos together, thx.

-- 
Regards,
Longpeng(Mike)

Re: [Qemu-devel] [PATCH for 2.12] disas/hppa: Fix compiler errors

[no-reply]

Hi,

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

Subject: [Qemu-devel] [PATCH for 2.12] disas/hppa: Fix compiler errors
Type: series
Message-id: 20171113072715.3443-1...@weilnetz.de

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

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

git config --local diff.renamelimit 0
git config --local diff.renames True

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

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]   patchew/20171113072715.3443-1...@weilnetz.de -> 
patchew/20171113072715.3443-1...@weilnetz.de
Switched to a new branch 'test'
e0bbd4d71b disas/hppa: Fix compiler errors

=== OUTPUT BEGIN ===
Checking PATCH 1/1: disas/hppa: Fix compiler errors...
ERROR: suspect code indent for conditional statements (20, 22)
#23: FILE: disas/hppa.c:2701:
if (sign)
+ disp = (UINT_MAX << 10) | imm10;

ERROR: code indent should never use tabs
#25: FILE: disas/hppa.c:2702:
+^I^I  disp = (UINT_MAX << 10) | imm10;$

ERROR: suspect code indent for conditional statements (20, 22)
#32: FILE: disas/hppa.c:2717:
if (sign)
+ disp = (UINT_MAX << 11) | imm11;

ERROR: code indent should never use tabs
#34: FILE: disas/hppa.c:2718:
+^I^I  disp = (UINT_MAX << 11) | imm11;$

total: 4 errors, 0 warnings, 16 lines checked

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

=== OUTPUT END ===

Test command exited with code: 1


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

[Qemu-devel] [PATCH for 2.12] disas/hppa: Fix compiler errors

[Stefan Weil]

gcc (Debian 6.3.0-18) 6.3.0 20170516 reports these errors:

disas/hppa.c:2702:20: error:
 left shift of negative value [-Werror=shift-negative-value]
disas/hppa.c:2718:20: error:
 left shift of negative value [-Werror=shift-negative-value]

Signed-off-by: Stefan Weil 
---

This patch fails with checkpatch.pl because the code uses
tab indention.

 disas/hppa.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/disas/hppa.c b/disas/hppa.c
index a2d371fdb1..0d1e99eab3 100644
--- a/disas/hppa.c
+++ b/disas/hppa.c
@@ -2699,7 +2699,7 @@ print_insn_hppa (bfd_vma memaddr, disassemble_info *info)
int disp;
 
if (sign)
- disp = (-1 << 10) | imm10;
+ disp = (UINT_MAX << 10) | imm10;
else
  disp = imm10;
 
@@ -2715,7 +2715,7 @@ print_insn_hppa (bfd_vma memaddr, disassemble_info *info)
int disp;
 
if (sign)
- disp = (-1 << 11) | imm11;
+ disp = (UINT_MAX << 11) | imm11;
else
  disp = imm11;
 
-- 
2.11.0

Re: [Qemu-devel] [virtio-dev] Re: [v21 1/2] virtio-crypto: Add virtio crypto device specification

[Gonglei (Arei)]

Hi,

>
> > +The controlq request is composed of two parts:
> > +\begin{lstlisting}
> > +struct virtio_crypto_op_ctrl_req {
> > +struct virtio_crypto_ctrl_header header;
> > +
> > +/* additional paramenter */
> > +u8 additional_para[addl_para_len];
> 
> What does additional paramenter mean? Even if I s/paramenter/parameter
> id doesn't sit well. To me and in this context additional is kind
> of like optional: because each member of a struct is trivially additional
> in respect to the previous members, and there is no point in pointing
> out additional. I would much rather go with something like:
> u8 op_specific[]
> 
> I also don't find the addl_para_len used anywhere. Then IMHO we don't
> need to introduce a name.
> 

I'd like to say that the additional_para[addl_para_len] is just a placeholder,
which is explained by the below content. I'm fine with op_specific[] too.

> > +};
> > +\end{lstlisting}
> > +
> > +The first is a general header (see above). And the second one, additional
> > +paramenter, contains an crypto-service-specific structure, which could be
> one
> 
> s/paramenter/parameter
> 
> It's actually opcode specific, or? Or is there a destroy service?
> 
We can choose the specific request (structure) as the *op_specific* according 
to opcode.

> > +of the following types:
> > +\begin{itemize*}
> > +\item struct virtio_crypto_sym_create_session_req
> > +\item struct virtio_crypto_hash_create_session_req
> > +\item struct virtio_crypto_mac_create_session_req
> > +\item struct virtio_crypto_aead_create_session_req
> > +\item virtio_crypto_destroy_session_req
> > +\end{itemize*}
> > +
> > +The size of the additional paramenter depends on the
> VIRTIO_CRYPTO_F_MUX_MODE
> 
> s/paramenter/parameter
> 
> > +feature bit:
> > +\item If the VIRTIO_CRYPTO_F_MUX_MODE feature bit is NOT negotiated,
> the
> > +size of additional paramenter is fixed to 56 bytes, the data of the
> unused
> 
> s/paramenter/parameter
> 
> > +part (if has) will be ingored.
> 
> s/ingored/ignored
> 
> 
> > +\item If the VIRTIO_CRYPTO_F_MUX_MODE feature bit is negotiated, the
> size of
> > +additional paramenter is flexible, which is the same as the
> crypto-service-specific
> 
> s/paramenter/parameter
> 
> > +structure used.
> > +
> > +\paragraph{Session operation}\label{sec:Device Types / Crypto Device /
> Device Operation / Control Virtqueue / Session operation}
> > +
> > +The session is a handle which describes the cryptographic parameters to be
> > +applied to a number of buffers.
> > +
> > +The following structure stores the result of session creation set by the
> device:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_session_input {
> > +/* Device-writable part */
> > +le64 session_id;
> > +le32 status;
> > +le32 padding;
> > +};
> > +\end{lstlisting}
> > +
> > +A request to destroy a session includes the following information:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_destroy_session_req {
> > +/* Device-readable part */
> > +le64  session_id;
> > +/* Device-writable part */
> > +le32  status;
> > +le32  padding;
> > +};
> > +\end{lstlisting}
> > +
> > +\subparagraph{Session operation: HASH session}\label{sec:Device Types /
> Crypto Device / Device
> > +Operation / Control Virtqueue / Session operation / Session operation: HASH
> session}
> > +
> 
> Let me skip to the one actually implemented.
> 
> > +
> > +The request of symmetric session includes two parts, CIPHER algorithms
> > +and chain algorithms (chaining CIPHER and HASH/MAC).
> 
> This sounds like concatenation and not either-or.
> > +
> > +CIPHER session requests are as follows:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_cipher_session_para {
> > +/* See VIRTIO_CRYPTO_CIPHER* above */
> > +le32 algo;
> > +/* length of key */
> > +le32 keylen;
> > +#define VIRTIO_CRYPTO_OP_ENCRYPT  1
> > +#define VIRTIO_CRYPTO_OP_DECRYPT  2
> > +/* encryption or decryption */
> > +le32 op;
> > +le32 padding;
> > +};
> > +
> > +struct virtio_crypto_cipher_session_req {
> > +/* Device-readable part */
> > +struct virtio_crypto_cipher_session_para para;
> > +/* The cipher key */
> > +u8 cipher_key[keylen];
> > +
> 
> Is there a limit to the size of chiper_key. I don't see one in your
> kernel code. OTOH given that virtio_crypto_sym_create_session_req
> is one flavor of virtio_crypto_op_ctrl_req.additional_para and that
> the later is 56 bytes in case no mux mode is supported, I think
> there must be a limit to the size of cipher_key!
> 
Of course the size of cipher_key is limited, firstly the max length is defined
in virtio crypto's configuration, see

struct virtio_crypto_config {
  ... ...
/* Maximum length of cipher key */
uint32_t max_cipher_key_len;
  ... ...
};

Secondly the real cipher_key size for a specific request, is in struct 
virtio_crypto_cipher_session_para,

struct virtio_crypto_cipher_session_para {
   ... ...
/* 

[Qemu-devel] QEMU 3.0 ? (was: [PATCH for-2.12 v3 01/11] spapr: add pseries 2.12 machine type)

[Thomas Huth]

On 10.11.2017 16:20, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater 
> ---
>  hw/ppc/spapr.c | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d682f013d422..a2dcbee07214 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3687,6 +3687,20 @@ static const TypeInfo spapr_machine_info = {
>  type_init(spapr_machine_register_##suffix)
>  
>  /*
> + * pseries-2.12
> + */
> +static void spapr_machine_2_12_instance_options(MachineState *machine)
> +{
> +}
> +
> +static void spapr_machine_2_12_class_options(MachineClass *mc)
> +{
> +/* Defaults for the latest behaviour inherited from the base class */
> +}
> +
> +DEFINE_SPAPR_MACHINE(2_12, "2.12", true);

By the way, before everybody now introduces "2.12" machine types ... is
there already a consensus that the next version will be "2.12" ?

A couple of months ago, we discussed that we could maybe do a 3.0 after
2.11, e.g. here:

 https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg05056.html

I'd still like to see that happen... Peter, any thoughts on this?

 Thomas

Re: [Qemu-devel] [PATCH] net/socket: fix coverity issue

[Jason Wang]

On 2017年11月06日 21:28, Jens Freimann wrote:

This fixes coverity issue CID1005339.

Make sure that saddr is not used uninitialized if the
mcast parameter is NULL.

Cc: qemu-sta...@nongnu.org
Reported-by: Peter Maydell 
Signed-off-by: Jens Freimann 
---
  net/socket.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index e6b471c63d..51eaea67a0 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -332,7 +332,7 @@ static NetSocketState 
*net_socket_fd_init_dgram(NetClientState *peer,
  const char *mcast,
  Error **errp)
  {
-struct sockaddr_in saddr;
+struct sockaddr_in saddr = { 0 };
  int newfd;
  NetClientState *nc;
  NetSocketState *s;
@@ -373,7 +373,7 @@ static NetSocketState 
*net_socket_fd_init_dgram(NetClientState *peer,
  net_socket_read_poll(s, true);
  
  /* mcast: save bound address as dst */

-if (is_connected) {
+if (is_connected && mcast != NULL) {
  s->dgram_dst = saddr;
  snprintf(nc->info_str, sizeof(nc->info_str),
   "socket: fd=%d (cloned mcast=%s:%d)",


Applied, thanks.

[Qemu-devel] [PATCH for 2.12] target/i386: Fix compiler warnings

[Stefan Weil]

These gcc warnings are fixed:

target/i386/translate.c:4461:12: warning:
 variable ‘prefixes’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Wclobbered]
target/i386/translate.c:4466:9: warning:
 variable ‘rex_w’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Wclobbered]
target/i386/translate.c:4466:16: warning:
 variable ‘rex_r’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Wclobbered]

Tested with x86_64-w64-mingw32-gcc from Debian stretch.

Signed-off-by: Stefan Weil 
---
 target/i386/translate.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/target/i386/translate.c b/target/i386/translate.c
index 088a9d9766..f410938244 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -4467,10 +4467,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState 
*cpu)
 target_ulong pc_start = s->base.pc_next;
 
 s->pc_start = s->pc = pc_start;
-prefixes = 0;
 s->override = -1;
-rex_w = -1;
-rex_r = 0;
 #ifdef TARGET_X86_64
 s->rex_x = 0;
 s->rex_b = 0;
@@ -4484,6 +4481,10 @@ static target_ulong disas_insn(DisasContext *s, CPUState 
*cpu)
 return s->pc;
 }
 
+prefixes = 0;
+rex_w = -1;
+rex_r = 0;
+
  next_byte:
 b = x86_ldub_code(env, s);
 /* Collect prefixes.  */
-- 
2.11.0

Re: [Qemu-devel] [PATCH] hw/vfio: improve error message when cannot init vfio event notifiers

[Markus Armbruster]

Jim Quigley  writes:

> On 16/10/2017 19:07, Michael Tokarev wrote:
>> 10.10.2017 13:22, Jim Quigley wrote:
>>> More information is required to assist trouble-shooting when
>>> QEMU fails to initialise the event notifications for devices
>>> assigned with VFIO-PCI. Instead of supplying the user with a cryptic
>>> error number only, print out a proper error message with strerror()
>>> so that the user has a better way to figure out what the problem is.
>>>
>>> Reviewed-by: Liam Merwick 
>>> Signed-off-by: Jim Quigley 
>>> ---
>>> Cc: qemu-triv...@nongnu.org
>>> Cc: m...@tls.msk.ru
>>> Cc: laur...@vivier.eu
>>> Cc: alex.william...@redhat.com
>>> ---
>>>   hw/vfio/pci.c | 35 ---
>>>   1 file changed, 24 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>>> index 31e1edf..3bffb93 100644
>>> --- a/hw/vfio/pci.c
>>> +++ b/hw/vfio/pci.c
>>> @@ -430,13 +430,16 @@ static int vfio_enable_vectors(VFIOPCIDevice *vdev, 
>>> bool msix)
>>>   static void vfio_add_kvm_msi_virq(VFIOPCIDevice *vdev, VFIOMSIVector 
>>> *vector,
>>> int vector_n, bool msix)
>>>   {
>>> -int virq;
>>> +int virq, ret;
>>> if ((msix && vdev->no_kvm_msix) || (!msix &&
>>> vdev->no_kvm_msi)) {
>>>   return;
>>>   }
>>>   -if (event_notifier_init(>kvm_interrupt, 0)) {
>>> +ret = event_notifier_init(>kvm_interrupt, 0);
>>> +if (ret) {
>>> +error_report("vfio (%s): Error: unable to init event notifier: %s 
>>> (%d)",
>>> + __func__, strerror(-ret), -ret);
>> Since this pattern gets repeated again and again, maybe we can either

Parts of it are anti-patterns:

* Error messages are by definition meant for the user.  If you feel you
  need to include __func__ so it makes sense, you're obviously failing
  to address the end user.

* Likewise for the numeric encoding of errno.

* Printing "Error: " or similar is error_report()'s job.

That leaves printing strerror().

>> use a common wrapper or move that eror reporting into event_notifier_init()?
>> Note there are other users of this function, besides hw/vfio, and maybe
>> these, too, can benefit from better error reporting?
>
>     Ideally the strerror() would be included in the error_report()
> function,
>     (as per the error_setg() function), which obviously would involve
> a more
>     extensive change to the code base. Would that be an acceptable
> solution ?

All existing uses error_report() that print strerror() would have to be
converted to the new function.  Same for error_vreport(), warn_report(),
...  Whether that's worthwhile is not obvious until we see patches.

>     Or I can move the reporting into theevent_notifier_init() function
> if that is
>     the preferred approach ?

Moving error_report() into event_notifier_init() makes it unusable in
contexts where error_report() is unwanted or wrong.

[Qemu-devel] [PATCH v2] highbank: validate register offset before access

[P J P]

From: Prasad J Pandit 

An 'offset' parameter sent to highbank register r/w functions
could be greater than number(NUM_REGS=0x200) of hb registers,
leading to an OOB access issue. Add check to avoid it.

Reported-by: Moguofang (Dennis mo) 
Signed-off-by: Prasad J Pandit 
---
 hw/arm/highbank.c | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

Update: use HWADDR_PRIx to print offset
  -> https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg02116.html

diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index 354c6b25a8..287392bbdc 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -34,6 +34,7 @@
 #include "hw/ide/ahci.h"
 #include "hw/cpu/a9mpcore.h"
 #include "hw/cpu/a15mpcore.h"
+#include "qemu/log.h"
 
 #define SMP_BOOT_ADDR   0x100
 #define SMP_BOOT_REG0x40
@@ -117,14 +118,26 @@ static void hb_regs_write(void *opaque, hwaddr offset,
 }
 }
 
-regs[offset/4] = value;
+if (offset / 4 >= NUM_REGS) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "highbank: bad write offset 0x%" HWADDR_PRIx "\n", offset);
+return;
+}
+regs[offset / 4] = value;
 }
 
 static uint64_t hb_regs_read(void *opaque, hwaddr offset,
  unsigned size)
 {
+uint32_t value;
 uint32_t *regs = opaque;
-uint32_t value = regs[offset/4];
+
+if (offset / 4 >= NUM_REGS) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "highbank: bad read offset 0x%" HWADDR_PRIx "\n", offset);
+return 0;
+}
+value = regs[offset / 4];
 
 if ((offset == 0x100) || (offset == 0x108) || (offset == 0x10C)) {
 value |= 0x3000;
-- 
2.13.6

Re: [Qemu-devel] [RESEND PATCH 2/6] memory: introduce AddressSpaceOps and IOMMUObject

[David Gibson]

On Fri, Nov 03, 2017 at 08:01:52PM +0800, Liu, Yi L wrote:
> From: Peter Xu 
> 
> AddressSpaceOps is similar to MemoryRegionOps, it's just for address
> spaces to store arch-specific hooks.
> 
> The first hook I would like to introduce is iommu_get(). Return an
> IOMMUObject behind the AddressSpace.
> 
> For systems that have IOMMUs, we will create a special address
> space per device which is different from system default address
> space for it (please refer to pci_device_iommu_address_space()).
> Normally when that happens, there will be one specific IOMMU (or
> say, translation unit) stands right behind that new address space.
> 
> This iommu_get() fetches that guy behind the address space. Here,
> the guy is defined as IOMMUObject, which includes a notifier_list
> so far, may extend in future. Along with IOMMUObject, a new iommu
> notifier mechanism is introduced. It would be used for virt-svm.
> Also IOMMUObject can further have a IOMMUObjectOps which is similar
> to MemoryRegionOps. The difference is IOMMUObjectOps is not relied
> on MemoryRegion.
> 
> Signed-off-by: Peter Xu 
> Signed-off-by: Liu, Yi L 

Hi, sorry I didn't reply to the earlier postings of this after our
discussion in China.  I've been sick several times and very busy.

I still don't feel like there's an adequate explanation of exactly
what an IOMMUObject represents.   Obviously it can represent more than
a single translation window - since that's represented by the
IOMMUMR.  But what exactly do all the MRs - or whatever else - that
are represented by the IOMMUObject have in common, from a functional
point of view.

Even understanding the SVM stuff better than I did, I don't really see
why an AddressSpace is an obvious unit to have an IOMMUObject
associated with it.

> ---
>  hw/core/Makefile.objs   |  1 +
>  hw/core/iommu.c | 58 +++
>  include/exec/memory.h   | 22 +++
>  include/hw/core/iommu.h | 73 
> +
>  memory.c| 10 +--
>  5 files changed, 162 insertions(+), 2 deletions(-)
>  create mode 100644 hw/core/iommu.c
>  create mode 100644 include/hw/core/iommu.h
> 
> diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
> index f8d7a4a..d688412 100644
> --- a/hw/core/Makefile.objs
> +++ b/hw/core/Makefile.objs
> @@ -5,6 +5,7 @@ common-obj-y += fw-path-provider.o
>  # irq.o needed for qdev GPIO handling:
>  common-obj-y += irq.o
>  common-obj-y += hotplug.o
> +common-obj-y += iommu.o
>  common-obj-y += nmi.o
>  
>  common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
> diff --git a/hw/core/iommu.c b/hw/core/iommu.c
> new file mode 100644
> index 000..7c4fcfe
> --- /dev/null
> +++ b/hw/core/iommu.c
> @@ -0,0 +1,58 @@
> +/*
> + * QEMU emulation of IOMMU logic
> + *
> + * Copyright (C) 2017 Red Hat Inc.
> + *
> + * Authors: Peter Xu ,
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> +
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> +
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see .
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/core/iommu.h"
> +
> +void iommu_notifier_register(IOMMUObject *iommu,
> + IOMMUNotifier *n,
> + IOMMUNotifyFn fn,
> + IOMMUEvent event)
> +{
> +n->event = event;
> +n->iommu_notify = fn;
> +QLIST_INSERT_HEAD(>iommu_notifiers, n, node);
> +return;
> +}
> +
> +void iommu_notifier_unregister(IOMMUObject *iommu,
> +   IOMMUNotifier *notifier)
> +{
> +IOMMUNotifier *cur, *next;
> +
> +QLIST_FOREACH_SAFE(cur, >iommu_notifiers, node, next) {
> +if (cur == notifier) {
> +QLIST_REMOVE(cur, node);
> +break;
> +}
> +}
> +}
> +
> +void iommu_notify(IOMMUObject *iommu, IOMMUEventData *event_data)
> +{
> +IOMMUNotifier *cur;
> +
> +QLIST_FOREACH(cur, >iommu_notifiers, node) {
> +if ((cur->event == event_data->event) && cur->iommu_notify) {
> +cur->iommu_notify(cur, event_data);
> +}
> +}
> +}
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 03595e3..8350973 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -26,6 +26,7 @@
>  #include "qom/object.h"
>  #include "qemu/rcu.h"
>  #include "hw/qdev-core.h"
> +#include "hw/core/iommu.h"
>  
> 

Re: [Qemu-devel] [PATCH for-2.12 v3 01/11] spapr: add pseries 2.12 machine type

[David Gibson]

On Fri, Nov 10, 2017 at 03:20:07PM +, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater 
> ---
>  hw/ppc/spapr.c | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d682f013d422..a2dcbee07214 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3687,6 +3687,20 @@ static const TypeInfo spapr_machine_info = {
>  type_init(spapr_machine_register_##suffix)
>  
>  /*
> + * pseries-2.12
> + */
> +static void spapr_machine_2_12_instance_options(MachineState *machine)
> +{
> +}
> +
> +static void spapr_machine_2_12_class_options(MachineClass *mc)
> +{
> +/* Defaults for the latest behaviour inherited from the base class */
> +}
> +
> +DEFINE_SPAPR_MACHINE(2_12, "2.12", true);
> +
> +/*
>   * pseries-2.11
>   */
>  static void spapr_machine_2_11_instance_options(MachineState *machine)
> @@ -3698,7 +3712,7 @@ static void 
> spapr_machine_2_11_class_options(MachineClass *mc)
>  /* Defaults for the latest behaviour inherited from the base class */
>  }
>  
> -DEFINE_SPAPR_MACHINE(2_11, "2.11", true);
> +DEFINE_SPAPR_MACHINE(2_11, "2.11", false);
>  
>  /*
>   * pseries-2.10

Uh.. not quite right.  You also need to chain the 2.11 hooks onto the
new 2.12 ones.  Never mind, we'll need it sooner or later, so I've put
my own version into ppc-for-2.12.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [Qemu devel PATCH] MAINTAINERS: Add entries for Smartfusion2

[sundeep subbaraya]

Hi,

On Fri, Nov 10, 2017 at 7:10 PM, Philippe Mathieu-Daudé 
wrote:

> On 11/10/2017 09:56 AM, Peter Maydell wrote:
> > On 10 November 2017 at 00:22, Philippe Mathieu-Daudé 
> wrote:
> >> On 11/09/2017 08:55 PM, Peter Maydell wrote:
> >>> I don't in general expect to take pull requests from
> >>> everybody listed as a maintainer in the MAINTAINERS file.
> >>> That just means "I'm going to be reviewing and should
> >>> be cc'd on patches". Pull requests are sent by people
> >>> who are maintainers for a subsystem. Rule of thumb:
> >>> unless somebody asks you to send a pull request, you
> >>> don't need to do it.
> >>
> >> Ok, please apologize my misunderstanding. I still think the M: entry
> >> stand for 'Maintainer' instead of 'Mail', and still don't understand the
> >> difference with a "Designated reviewer" (R: entry):
> >>
> >> M: Mail patches to: FullName 
> >> R: Designated reviewer: FullName 
> >>These reviewers should be CCed on patches.
> >>
> >> "Designated reviewer" seems to duplicate the M: entry and is therefore
> >> confusing. Can we simply remove it instead?
> >
> > I hadn't realized we had an 'R:' tag in MAINTAINERS...
> >
> >> --
> >> Some people are not content with the amount of mail they get, and would
> >> like to be CCed on patches for areas they do not maintain.  Let them
> >> satisfy their own appetite for qemu-devel messages.
> >>
> >> Seriously: the purpose here is a bit different from the Linux kernel.
> >> While Linux uses "R" to designate non-maintainers for reviewing patches
> >> in a given area, in QEMU I would also like to use "R" so that people can
> >> delegate sending pull requests while keeping some degree of oversight.
> >> --
> >
> > So, my view, based on what happens in practice:
> >  * "maintainer" means you are in effect accepting some responsibility
> >for the continued maintenance of some bit of the codebase, ie
> >you actually will review stuff
> >  * "reviewer" is a bit weird but I guess is just asking for cc:
> >without promising to actually do anything
> >  * somebody who sends me pull requests is effectively somebody we've
> >given the ability to make direct more-or-less unchecked commits
> >to master, so that is given out more sparingly and for larger
> >subsystems
>
> Thanks for clarifying this!
>

Thanks Peter, Philippe and Alistair,
Sundeep


> This is more understandable (to me) than the "QEMU Maintainers" section
> entries description.
>
> > But MAINTAINERS is mostly about what submitters need to do (ie
> > who to send patchmails to), so it doesn't particularly document
> > how patches flow onward into master, which varies. (For instance
> > the block layer folks have a two-level setup where some trees
> > get merged into others before they go to master. ARM devboards
> > go through me, and "maintainer" just means I let somebody else
> > deal with the device specifics if possible but am still the
> > reviewer of last resort.)
>
> Ok :)
>
> Regards,
>
> Phil.
>

[Qemu-devel] [Qemu devel PATCH v2] MAINTAINERS: Add entries for Smartfusion2

[Subbaraya Sundeep]

Voluntarily add myself as maintainer for Smartfusion2

Signed-off-by: Subbaraya Sundeep 
Reviewed-by: Alistair Francis 
Reviewed-by: Philippe Mathieu-Daudé 
---
v2:
reframed commit message as per Alistair's comment.
Renamed MSF2 SoC -> SmartFusion2
MSF2 SOM -> Emcraft M2S-FG484
as per Philippe's comments.

 MAINTAINERS | 17 +
 1 file changed, 17 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 0cd4d02..ffd77b4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -564,6 +564,23 @@ M: Alistair Francis 
 S: Maintained
 F: hw/arm/netduino2.c
 
+SmartFusion2
+M: Subbaraya Sundeep 
+S: Maintained
+F: hw/arm/msf2-soc.c
+F: hw/misc/msf2-sysreg.c
+F: hw/timer/mss-timer.c
+F: hw/ssi/mss-spi.c
+F: include/hw/arm/msf2-soc.h
+F: include/hw/misc/msf2-sysreg.h
+F: include/hw/timer/mss-timer.h
+F: include/hw/ssi/mss-spi.h
+
+Emcraft M2S-FG484
+M: Subbaraya Sundeep 
+S: Maintained
+F: hw/arm/msf2-som.c
+
 CRIS Machines
 -
 Axis Dev88
-- 
2.5.0

Re: [Qemu-devel] [PATCH for-2.12 v3 02/11] ppc/xics: remove useless if condition

[David Gibson]

On Fri, Nov 10, 2017 at 03:20:08PM +, Cédric Le Goater wrote:
> The previous code section uses a 'first < 0' test and returns. Therefore,
> there is no need to test the 'first' variable against '>= 0' afterwards.
> 
> Signed-off-by: Cédric Le Goater 
> ---
>  hw/intc/xics_spapr.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> index d98ea8b13068..e8c0a1b3e903 100644
> --- a/hw/intc/xics_spapr.c
> +++ b/hw/intc/xics_spapr.c
> @@ -329,10 +329,8 @@ int spapr_ics_alloc_block(ICSState *ics, int num, bool 
> lsi,
>  return -1;
>  }
>  
> -if (first >= 0) {
> -for (i = first; i < first + num; ++i) {
> -ics_set_irq_type(ics, i, lsi);
> -}
> +for (i = first; i < first + num; ++i) {
> +ics_set_irq_type(ics, i, lsi);
>  }
>  first += ics->offset;
>  

Applied to ppc-for-2.12.

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


signature.asc
Description: PGP signature

[Qemu-devel] [Bug 888016] Re: RHEL 6.1 guest fails to boot with vhost

[Launchpad Bug Tracker]

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

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

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

Title:
  RHEL 6.1 guest fails to boot with vhost

Status in QEMU:
  Expired

Bug description:
  Tried to boot 6.1 guest on hs22 blade with/without  vhost enabled.

  With vhost enabled,  guest aborted with core dump.

  installed guest with autotest.
  Command : 
  /usr/local/bin/qemu-system-x86_64 -name 'vm1' -nodefaults -vga std -monitor 
unix:'/tmp/monitor-humanmonitor1-2008-193209-fc6O',server,nowait -serial 
unix:'/tmp/serial-2008-193209-fc6O',server,nowait -drive 
file='/home/pradeep/autotest/client/tests/kvm/images/rhel6.1-64.qed',index=0,if=virtio,cache=none
 -device virtio-net-pci,netdev=idQhUaOc,mac='9a:b7:ea:c9:0e:0d',id='idVR6XQz' 
-netdev tap,id=idQhUaOc,vhost=on,script=/home/pradeep/qemu-ifup-latest -m 1024 
-smp 8 -vnc :0 -monitor stdio
  QEMU 0.15.91 monitor - type 'help' for more information
  (qemu) Aborted (core dumped)

  
  host:

  2.6.32-214
  m/c: hs22
  vhost modules are loaded.

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

[Qemu-devel] [Bug 1254940] Re: qemu-KVM guest OS occurs many ext3-fs errors after multiple forced shutdown

[Launchpad Bug Tracker]

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

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

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

Title:
  qemu-KVM guest OS occurs many ext3-fs errors after multiple forced
  shutdown

Status in QEMU:
  Expired

Bug description:
  Hi:
  I met some filesysterm errors in a sles guest on KVM. My system environment 
is:
  HOST:
     suse 10, the kernel version is 2.6.32.43
     Qemu-KVM 1.2
     Libvirt 1.0
  guest OS:
     suse 10, the kernel version is 2.6.32.43
  VMs use a qcow2 disk.

  Description of problem:
  I have 20+ VMs with qcow2 disk, these VMs have been forced to shut down by
  "virsh destroy" many times during and after VM installation.
  When these vm reboot,dmesg show a ext3-fs mount error occurred on /usr/local
  partion /dev/vda3:
  EXT3-fs warning: mounting fs with errors, running e2fsck is recommendedand
  when I wrote into partion /dev/vda3,some errors occurred in dmesg:
  1.error (device vda3): ext3_free_blocks: Freeing blocks not in datazone - 
block
  = 1869619311, count = 1error (device vda3): ext3_free_blocks_sb: bit already
  cleared for block 2178152error (device vda3): ext3_readdir: bad entry in
  directory #1083501:
  2.[347470.661893] attempt to access beyond end of device[347470.661896] vda3:
  rw=0, want=6870892952, limit=41945715[347470.661897] EXT3-fs error (device
  vda3): ext3_free_branches: Read failure, inode=1083508, block=858861618
  3.EXT3-fs error (device vda3): ext3_new_block: block(4295028581) >= blocks
  count(-1) - block_group = 1, es == 88021b6c7400

  I suspect this fs-error is caused by multiple forced shutdown, but I can't
  reproduce this bug now.

  So if someone else encountered the same bugs(or fs errors) with me,
  and what the reasons leads to these fs errors, is the qemu or KVM or
  just my "rough" cmd.

  Thanks in Advance
  Regards
  Ben

  Steps to Reproduce:
  I can't reproduce this bug now.

  additional:
  1.multiple forced shutdown during and after the vm installing
  2.vm with qcow2 disk
  3.different vm dmesg different errors in above error list(1/2/3)

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

[Qemu-devel] [Bug 996798] Re: Incorrect order of task switching

[Launchpad Bug Tracker]

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

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

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

Title:
  Incorrect order of task switching

Status in QEMU:
  Expired

Bug description:
  In Intel  specifications
  (http://download.intel.com/design/processor/manuals/253668.pdf 7.3),
  we can see:

  8. Saves the state of the current (old) task in the current task’s
  TSS.

  …

     12. The TSS state is loaded into the processor

  But, in QEMU code
  (https://raw.github.com/qemu/QEMU/v1.0/target-i386/op_helper.c :375),
  the order is reversed: TSS registers & segments loads BEFORE save old
  task state.

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

Re: [Qemu-devel] [PATCH 1/1] target/ppc: correct htab shift for hash on radix

[David Gibson]

On Mon, Nov 06, 2017 at 02:14:35PM +1100, Sam Bobroff wrote:
> KVM HV will soon support running a guest in hash mode on a POWER9 host
> running in radix mode (see [1]), however the guest currently fails to
> boot.
> 
> This is because the "htab_shift" value (the size of the MMU's hash
> table) is added to the device tree before KVM has had a chance to
> change it. If the host is in hash mode, KVM does not need to change it
> and so the problem is not seen, but when the host is in radix mode a
> change is required and we see a problem.
> 
> To fix this, move the call spapr_setup_hpt_and_vrma() (where
> htab_shift could be changed) up a little so that it's called before
> spapr_h_cas_compose_response() (where htab_shift is added to the
> device tree).
> 
> Signed-off-by: Sam Bobroff 

Applied to ppc-for-2.11.

> 
> [1] See http://www.spinics.net/lists/kvm-ppc/msg13057.html
> ---
> I tested this patch using a kernel based on Paul's kvm-ppc-next branch from 
> his
> powerpc tree:
> https://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git/
> 
>  hw/ppc/spapr_hcall.c | 13 ++---
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 0d59d1534d..be22a6b289 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1636,6 +1636,12 @@ static target_ulong 
> h_client_architecture_support(PowerPCCPU *cpu,
>  spapr->cas_legacy_guest_workaround = !spapr_ovec_test(ov1_guest,
>OV1_PPC_3_00);
>  if (!spapr->cas_reboot) {
> +/* If ppc_spapr_reset() did not set up a HPT but one is necessary
> + * (because the guest isn't going to use radix) then set it up here. 
> */
> +if ((spapr->patb_entry & PATBE1_GR) && !guest_radix) {
> +/* legacy hash or new hash: */
> +spapr_setup_hpt_and_vrma(spapr);
> +}
>  spapr->cas_reboot =
>  (spapr_h_cas_compose_response(spapr, args[1], args[2],
>ov5_updates) != 0);
> @@ -1644,13 +1650,6 @@ static target_ulong 
> h_client_architecture_support(PowerPCCPU *cpu,
>  
>  if (spapr->cas_reboot) {
>  qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> -} else {
> -/* If ppc_spapr_reset() did not set up a HPT but one is necessary
> - * (because the guest isn't going to use radix) then set it up here. 
> */
> -if ((spapr->patb_entry & PATBE1_GR) && !guest_radix) {
> -/* legacy hash or new hash: */
> -spapr_setup_hpt_and_vrma(spapr);
> -}
>  }
>  
>  return H_SUCCESS;

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v1] migration/ram.c: do not set 'postcopy_running' in POSTCOPY_INCOMING_END

[Peter Xu]

On Fri, Nov 10, 2017 at 06:35:16PM -0200, Daniel Henrique Barboza wrote:
> When migrating a VM with 'migrate_set_capability postcopy-ram on'
> a postcopy_state is set during the process, ending up with the
> state POSTCOPY_INCOMING_END when the migration is over. This
> postcopy_state is taken into account inside ram_load to check
> how it will load the memory pages. This same ram_load is called when
> in a loadvm command.
> 
> Inside ram_load, the logic to see if we're at postcopy_running state
> is:
> 
> postcopy_running = postcopy_state_get() >= POSTCOPY_INCOMING_LISTENING
> 
> postcopy_state_get() returns this enum type:
> 
> typedef enum {
> POSTCOPY_INCOMING_NONE = 0,
> POSTCOPY_INCOMING_ADVISE,
> POSTCOPY_INCOMING_DISCARD,
> POSTCOPY_INCOMING_LISTENING,
> POSTCOPY_INCOMING_RUNNING,
> POSTCOPY_INCOMING_END
> } PostcopyState;
> 
> In the case where ram_load is executed and postcopy_state is
> POSTCOPY_INCOMING_END, postcopy_running will be set to 'true' and
> ram_load will behave like a postcopy is in progress. This scenario isn't
> achievable in a migration but it is reproducible when executing
> savevm/loadvm after migrating with 'postcopy-ram on', causing loadvm
> to fail with Error -22:
> 
> Source:
> 
> (qemu) migrate_set_capability postcopy-ram on
> (qemu) migrate tcp:127.0.0.1:
> 
> Dest:
> 
> (qemu) migrate_set_capability postcopy-ram on
> (qemu)
> ubuntu1704-intel login:
> Ubuntu 17.04 ubuntu1704-intel ttyS0
> 
> ubuntu1704-intel login: (qemu)
> (qemu) savevm test1
> (qemu) loadvm test1
> Unknown combination of migration flags: 0x4 (postcopy mode)
> error while loading state for instance 0x0 of device 'ram'
> Error -22 while loading VM state
> (qemu)
> 
> This patch fixes this problem by changing a bit the semantics
> of postcopy_running inside ram_load, verifying first if
> we're not in the POSTCOPY_INCOMING_END state. In this case,
> postcopy_running is set to 'false'.
> 
> Signed-off-by: Daniel Henrique Barboza 
> ---
>  migration/ram.c | 22 +++---
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 8620aa400a..43ed719668 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2803,13 +2803,21 @@ static int ram_load(QEMUFile *f, void *opaque, int 
> version_id)
>  int flags = 0, ret = 0, invalid_flags = 0;
>  static uint64_t seq_iter;
>  int len = 0;
> -/*
> - * If system is running in postcopy mode, page inserts to host memory 
> must
> - * be atomic
> - */
> -bool postcopy_running = postcopy_state_get() >= 
> POSTCOPY_INCOMING_LISTENING;
> -/* ADVISE is earlier, it shows the source has the postcopy capability on 
> */
> -bool postcopy_advised = postcopy_state_get() >= POSTCOPY_INCOMING_ADVISE;
> +bool postcopy_advised = false, postcopy_running = false;
> +uint8_t postcopy_state = postcopy_state_get();
> +
> +if (postcopy_state != POSTCOPY_INCOMING_END) {
> +/*
> + * If system is running in postcopy mode, page inserts to host memory
> + * must be atomic
> + */
> +postcopy_running = postcopy_state >= POSTCOPY_INCOMING_LISTENING;
> +
> +/* ADVISE is earlier, it shows the source has the postcopy
> + * capability on
> + */
> +postcopy_advised = postcopy_state >= POSTCOPY_INCOMING_ADVISE;
> +}

For me, this is not that clear.  I would prefer something simpler like:

  PostcopyState state = postcopy_state_get();
  bool postcopy_running = state >= POSTCOPY_INCOMING_LISTENING && \
  state < POSTCOPY_INCOMING_END;

Or we can introduce postcopy_is_running() helper.  Same thing to
postcopy_advised.  Thanks,

>  
>  seq_iter++;
>  
> -- 
> 2.13.6
> 
> 

-- 
Peter Xu

Re: [Qemu-devel] [U-Boot] Support of latest qemux86-64

[Bin Meng]

Hi Anton,

On Sat, Nov 11, 2017 at 1:34 AM, Anton Gerasimov
 wrote:
> Hooray, changing SYS_CAR_ADDR to 0x1 in arch/x86/cpu/qemu/Kconfig
> does the trick. Bin, what do you think about it?
>

Great! Would you please create a patch against U-Boot QEMU?

> Best regards,
> Anton Gerasimov
>
> On 11/10/2017 06:25 PM, Anton Gerasimov wrote:
>> Yes, apparently 0xdfffc is in ROM area for QEMU (0xc -- 0xe,
>> defined in include/hw/loader.h). The next thing to figure out is why
>> u-boot uses it as a stack area.
>>
>> Best regards,
>> Anton Gerasimov
>>
>> On 11/10/2017 06:04 PM, Anton Gerasimov wrote:
>>> New guess:
>>>
>>> in the most safe configuration of u-boot (CONFIG_SMP=n, lacpi disabled)
>>> with Igor's patch applied `qemu-system-i386 -bios /path/to/uboot.rom`
>>> fails on the first 'ret' instruction. GDB shows that memory at $esp
>>> (0xdfffc at the entrance to board_init_f_mem) and everything around it
>>> is zero despite 'call' and 'push' instructions executed. If you go one
>>> commit before the breaking one it works fine, stuff gets put onto stack.
>>> Could it that be that stack itself is in this 'readonly' area?
>>>
>>> Thanks,
>>> Anton Gerasimov
>>>
>>> On 11/09/2017 02:58 AM, Bin Meng wrote:
 On Wed, Nov 8, 2017 at 9:05 PM, Anton Gerasimov
  wrote:
> Adding Igor Mammedov to the loop.
>
 Really add Igor Mammedov.

 Igor, can you help look at this?

> On 11/08/2017 01:59 PM, Anton Gerasimov wrote:
>> To whoever might be interested: I've bisected qemu and the breaking
>> commit is 208fa0e43645edd0b0d8f838857dfc79daff40a8 (pc: make 'pc.rom'
>> readonly when machine has PCI enabled). It's just three lines added,
>> I'll paste the whole patch here. Not quite sure what can we do here 
>> though.
>>
>>
>>   diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>>   index 22e16031b0..59435390ba 100644
>>   --- a/hw/i386/pc.c
>>   +++ b/hw/i386/pc.c
>>   @@ -1443,6 +1443,9 @@ void pc_memory_init(PCMachineState *pcms,
>>option_rom_mr = g_malloc(sizeof(*option_rom_mr));
>>memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE,
>>   _fatal);
>>   +if (pcmc->pci_enabled) {
>>   +memory_region_set_readonly(option_rom_mr, true);
>>   +}
>>memory_region_add_subregion_overlap(rom_memory,
>>PC_ROM_MIN_VGA,
>>option_rom_mr,
>>
>>
 Regards,
 Bin
>

Regards,
Bin

Re: [Qemu-devel] [PATCH v12 09/12] Move related hwpoison page function to accel/kvm/ folder

[gengdongjiu]

On 2017/11/10 19:32, Paolo Bonzini wrote:
> On 10/11/2017 20:19, Dongjiu Geng wrote:
>> +typedef struct HWPoisonPage {
>> +ram_addr_t ram_addr;
>> +QLIST_ENTRY(HWPoisonPage) list;
>> +} HWPoisonPage;
>> +
> 
> Is this actually needed outside accel/kvm/kvm-all.c?
Paolo,
   Thanks for the comments, this structure is added in the accel/kvm/kvm-all.c 
is also OK.
My previous thought is that this is structure definition, so I move it to a 
header file.
If you think this structure should be added in accel/kvm/kvm-all.c, I will move 
it.
thanks!

> 
> Thanks,
> 
> Paolo
> 
> .
> 

[Qemu-devel] [PATCH 5/8] sdl2 uses surface relative coordinates

[Jindrich Makovicka]

This patch fixes mouse positioning with -device usb-tablet and fullscreen
or resized window.

Signed-off-by: Jindrich Makovicka 
---
 ui/sdl2.c | 28 ++--
 1 file changed, 2 insertions(+), 26 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 677c22282d..51721d764e 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -276,32 +276,8 @@ static void sdl_send_mouse_event(struct sdl2_console 
*scon, int dx, int dy,
 }
 
 if (qemu_input_is_absolute()) {
-int scr_w, scr_h;
-int max_w = 0, max_h = 0;
-int off_x = 0, off_y = 0;
-int cur_off_x = 0, cur_off_y = 0;
-int i;
-
-for (i = 0; i < sdl2_num_outputs; i++) {
-struct sdl2_console *thiscon = _console[i];
-if (thiscon->real_window && thiscon->surface) {
-SDL_GetWindowSize(thiscon->real_window, _w, _h);
-cur_off_x = thiscon->x;
-cur_off_y = thiscon->y;
-if (scr_w + cur_off_x > max_w) {
-max_w = scr_w + cur_off_x;
-}
-if (scr_h + cur_off_y > max_h) {
-max_h = scr_h + cur_off_y;
-}
-if (i == scon->idx) {
-off_x = cur_off_x;
-off_y = cur_off_y;
-}
-}
-}
-qemu_input_queue_abs(scon->dcl.con, INPUT_AXIS_X, off_x + x, 0, max_w);
-qemu_input_queue_abs(scon->dcl.con, INPUT_AXIS_Y, off_y + y, 0, max_h);
+qemu_input_queue_abs(scon->dcl.con, INPUT_AXIS_X, x, 0, 
surface_width(scon->surface));
+qemu_input_queue_abs(scon->dcl.con, INPUT_AXIS_Y, y, 0, 
surface_height(scon->surface));
 } else {
 if (guest_cursor) {
 x -= guest_x;
-- 
2.15.0

[Qemu-devel] [PATCH 7/8] sdl2: Do not leave grab when fullscreen

[Jindrich Makovicka]

Prevents displaying of a doubled mouse pointer when moving the pointer
to the screen edges when fullscreen.

Signed-off-by: Jindrich Makovicka 
---
 ui/sdl2.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index cdc599be48..6feb637739 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -451,8 +451,9 @@ static void handle_mousemotion(SDL_Event *ev)
 SDL_GetWindowSize(scon->real_window, _w, _h);
 max_x = scr_w - 1;
 max_y = scr_h - 1;
-if (gui_grab && (ev->motion.x == 0 || ev->motion.y == 0 ||
- ev->motion.x == max_x || ev->motion.y == max_y)) {
+if (gui_grab && !gui_fullscreen
+&& (ev->motion.x == 0 || ev->motion.y == 0 ||
+ev->motion.x == max_x || ev->motion.y == max_y)) {
 sdl_grab_end(scon);
 }
 if (!gui_grab &&
-- 
2.15.0

[Qemu-devel] [PATCH 8/8] sdl2: Ignore UI hotkeys after a focus change when GUI modifier is held

[Jindrich Makovicka]

When SDL2 windows change focus while a key is held, the window that
receives the focus also receives a new KeyDown event, without an
autorepeat flag. This means that if a WM places the qemu console
over the main window after Ctrl-Alt-2, the console closes immediately
after opening. Then, the main window receives the KeyDown event again
and the whole process repeats.

This patch makes the SDL2 UI ignore the KeyDown events on a window that
just received the focus, if the GUI modifier was held. The ignore flag
is reset on a first KeyUp event. This effectively works around the issue
above.

Signed-off-by: Jindrich Makovicka 
---
 include/ui/sdl2.h |  1 +
 ui/sdl2.c | 25 -
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
index b29cf803c9..51084e6320 100644
--- a/include/ui/sdl2.h
+++ b/include/ui/sdl2.h
@@ -24,6 +24,7 @@ struct sdl2_console {
 int opengl;
 int updates;
 int idle_counter;
+int ignore_hotkeys;
 SDL_GLContext winctx;
 #ifdef CONFIG_OPENGL
 QemuGLShader *gls;
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 6feb637739..2d48686b83 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -310,22 +310,26 @@ static void toggle_full_screen(struct sdl2_console *scon)
 sdl2_redraw(scon);
 }
 
-static void handle_keydown(SDL_Event *ev)
+static int get_mod_state(void)
 {
-int mod_state, win;
-struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
-
 if (alt_grab) {
-mod_state = (SDL_GetModState() & (gui_grab_code | KMOD_LSHIFT)) ==
+return (SDL_GetModState() & (gui_grab_code | KMOD_LSHIFT)) ==
 (gui_grab_code | KMOD_LSHIFT);
 } else if (ctrl_grab) {
-mod_state = (SDL_GetModState() & KMOD_RCTRL) == KMOD_RCTRL;
+return (SDL_GetModState() & KMOD_RCTRL) == KMOD_RCTRL;
 } else {
-mod_state = (SDL_GetModState() & gui_grab_code) == gui_grab_code;
+return (SDL_GetModState() & gui_grab_code) == gui_grab_code;
 }
-gui_key_modifier_pressed = mod_state;
+}
+
+static void handle_keydown(SDL_Event *ev)
+{
+int win;
+struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
 
-if (gui_key_modifier_pressed) {
+gui_key_modifier_pressed = get_mod_state();
+
+if (!scon->ignore_hotkeys && gui_key_modifier_pressed && !ev->key.repeat) {
 switch (ev->key.keysym.scancode) {
 case SDL_SCANCODE_2:
 case SDL_SCANCODE_3:
@@ -399,6 +403,8 @@ static void handle_keyup(SDL_Event *ev)
 int mod_state;
 struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
 
+scon->ignore_hotkeys = false;
+
 if (!alt_grab) {
 mod_state = (ev->key.keysym.mod & gui_grab_code);
 } else {
@@ -545,6 +551,7 @@ static void handle_windowevent(SDL_Event *ev)
 if (!gui_grab && (qemu_input_is_absolute() || absolute_enabled)) {
 absolute_mouse_grab(scon);
 }
+scon->ignore_hotkeys = get_mod_state();
 break;
 case SDL_WINDOWEVENT_FOCUS_LOST:
 if (gui_grab && !gui_fullscreen) {
-- 
2.15.0

[Qemu-devel] [PATCH 6/8] sdl2: Fix dead keyboard after fullsceen

[Jindrich Makovicka]

Signed-off-by: Jindrich Makovicka 
---
 ui/sdl2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 51721d764e..cdc599be48 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -418,6 +418,7 @@ static void handle_keyup(SDL_Event *ev)
 sdl2_reset_keys(scon);
 return;
 }
+sdl2_reset_keys(scon);
 gui_keysym = 0;
 }
 if (!gui_keysym) {
-- 
2.15.0

[Qemu-devel] [PATCH 4/8] sdl2: Do not hide the cursor on auxilliary windows

[Jindrich Makovicka]

Signed-off-by: Jindrich Makovicka 
---
 ui/sdl2.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 9cf4b1772b..677c22282d 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -465,6 +465,10 @@ static void handle_mousemotion(SDL_Event *ev)
 int max_x, max_y;
 struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
 
+if (!qemu_console_is_graphic(scon->dcl.con)) {
+return;
+}
+
 if (qemu_input_is_absolute() || absolute_enabled) {
 int scr_w, scr_h;
 SDL_GetWindowSize(scon->real_window, _w, _h);
@@ -492,6 +496,10 @@ static void handle_mousebutton(SDL_Event *ev)
 SDL_MouseButtonEvent *bev;
 struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
 
+if (!qemu_console_is_graphic(scon->dcl.con)) {
+return;
+}
+
 bev = >button;
 if (!gui_grab && !qemu_input_is_absolute()) {
 if (ev->type == SDL_MOUSEBUTTONUP && bev->button == SDL_BUTTON_LEFT) {
@@ -514,6 +522,10 @@ static void handle_mousewheel(SDL_Event *ev)
 SDL_MouseWheelEvent *wev = >wheel;
 InputButton btn;
 
+if (!qemu_console_is_graphic(scon->dcl.con)) {
+return;
+}
+
 if (wev->y > 0) {
 btn = INPUT_BUTTON_WHEEL_UP;
 } else if (wev->y < 0) {
@@ -655,6 +667,11 @@ static void sdl_mouse_warp(DisplayChangeListener *dcl,
int x, int y, int on)
 {
 struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
+
+if (!qemu_console_is_graphic(scon->dcl.con)) {
+return;
+}
+
 if (on) {
 if (!guest_cursor) {
 sdl_show_cursor();
-- 
2.15.0

[Qemu-devel] [PATCH 3/8] sdl2: Use the same pointer show/hide logic for absolute and relative mode

[Jindrich Makovicka]

Also use a proper enum parameter for SDL_ShowCursor

Signed-off-by: Jindrich Makovicka 
---
 ui/sdl2.c | 24 +---
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index dfbd0de791..9cf4b1772b 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -169,10 +169,10 @@ static void sdl_hide_cursor(void)
 return;
 }
 
-if (qemu_input_is_absolute()) {
-SDL_ShowCursor(1);
-SDL_SetCursor(sdl_cursor_hidden);
-} else {
+SDL_ShowCursor(SDL_DISABLE);
+SDL_SetCursor(sdl_cursor_hidden);
+
+if (!qemu_input_is_absolute()) {
 SDL_SetRelativeMouseMode(SDL_TRUE);
 }
 }
@@ -185,14 +185,16 @@ static void sdl_show_cursor(void)
 
 if (!qemu_input_is_absolute()) {
 SDL_SetRelativeMouseMode(SDL_FALSE);
-SDL_ShowCursor(1);
-if (guest_cursor &&
-(gui_grab || qemu_input_is_absolute() || absolute_enabled)) {
-SDL_SetCursor(guest_sprite);
-} else {
-SDL_SetCursor(sdl_cursor_normal);
-}
 }
+
+if (guest_cursor &&
+(gui_grab || qemu_input_is_absolute() || absolute_enabled)) {
+SDL_SetCursor(guest_sprite);
+} else {
+SDL_SetCursor(sdl_cursor_normal);
+}
+
+SDL_ShowCursor(SDL_ENABLE);
 }
 
 static void sdl_grab_start(struct sdl2_console *scon)
-- 
2.15.0

[Qemu-devel] [PATCH 1/8] sdl2: Fix broken display updating after the window is hidden

[Jindrich Makovicka]

With SDL 2.0.6, calling SDL_ShowWindow during SDL_WINDOWEVENT_HIDDEN
blocks all subsequent display updates.

Instead of trying to override the change, just update the scon->hidden
flag.

Signed-off-by: Jindrich Makovicka 
---
 ui/sdl2.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 53dd447fd2..774904cbf2 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -572,14 +572,10 @@ static void handle_windowevent(SDL_Event *ev)
 }
 break;
 case SDL_WINDOWEVENT_SHOWN:
-if (scon->hidden) {
-SDL_HideWindow(scon->real_window);
-}
+scon->hidden = false;
 break;
 case SDL_WINDOWEVENT_HIDDEN:
-if (!scon->hidden) {
-SDL_ShowWindow(scon->real_window);
-}
+scon->hidden = true;
 break;
 }
 }
-- 
2.15.0

[Qemu-devel] [PATCH v3] SDL2 various fixes

[Jindrich Makovicka]

Hi,

here is an identical patchset with Signed-off-by.

Regards, Jindrich

[Qemu-devel] [PATCH 2/8] sdl2: Do not quit the emulator when an auxilliary window is closed

[Jindrich Makovicka]

Signed-off-by: Jindrich Makovicka 
---
 ui/sdl2.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 774904cbf2..dfbd0de791 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -566,9 +566,14 @@ static void handle_windowevent(SDL_Event *ev)
 update_displaychangelistener(>dcl, 500);
 break;
 case SDL_WINDOWEVENT_CLOSE:
-if (!no_quit) {
-no_shutdown = 0;
-qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
+if (qemu_console_is_graphic(scon->dcl.con)) {
+if (!no_quit) {
+no_shutdown = 0;
+qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
+}
+} else {
+SDL_HideWindow(scon->real_window);
+scon->hidden = true;
 }
 break;
 case SDL_WINDOWEVENT_SHOWN:
-- 
2.15.0

Re: [Qemu-devel] [PATCH v4 3/5] fw_cfg: do DMA read operation

[Gabriel L. Somlo]

On Sun, Nov 12, 2017 at 09:36:33AM -0500, Marc-André Lureau wrote:
> 
> 
> - Original Message -
> > On Tue, Oct 31, 2017 at 04:19:36PM +0100, Marc-André Lureau wrote:
> > > Modify fw_cfg_read_blob() to use DMA if the device supports it.
> > > Return errors, because the operation may fail.
> > > 
> > > To avoid polling with unbound amount of time, the DMA operation is
> > > expected to complete within 200ms, or will return ETIME error.
> > > 
> > > We may want to switch all the *buf addresses to use only kmalloc'ed
> > > buffers (instead of using stack/image addresses with dma=false).
> > > 
> > > Signed-off-by: Marc-André Lureau 
> > 
> > A couple of silly questions, inline, below.
> > 
> > Thanks,
> > --G
> > 
> > > ---
> > >  drivers/firmware/qemu_fw_cfg.c | 154
> > >  -
> > >  1 file changed, 137 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/drivers/firmware/qemu_fw_cfg.c
> > > b/drivers/firmware/qemu_fw_cfg.c
> > > index 1f3e8545dab7..8ce5e49b7c62 100644
> > > --- a/drivers/firmware/qemu_fw_cfg.c
> > > +++ b/drivers/firmware/qemu_fw_cfg.c
> > > @@ -33,6 +33,8 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> > > +#include 
> > >  
> > >  MODULE_AUTHOR("Gabriel L. Somlo ");
> > >  MODULE_DESCRIPTION("QEMU fw_cfg sysfs support");
> > > @@ -43,12 +45,26 @@ MODULE_LICENSE("GPL");
> > >  #define FW_CFG_ID 0x01
> > >  #define FW_CFG_FILE_DIR   0x19
> > >  
> > > +#define FW_CFG_VERSION_DMA 0x02
> > > +#define FW_CFG_DMA_CTL_ERROR   0x01
> > > +#define FW_CFG_DMA_CTL_READ0x02
> > > +#define FW_CFG_DMA_CTL_SKIP0x04
> > > +#define FW_CFG_DMA_CTL_SELECT  0x08
> > > +#define FW_CFG_DMA_CTL_WRITE   0x10
> > > +#define FW_CFG_DMA_TIMEOUT 200 /* ms */
> > > +
> > >  /* size in bytes of fw_cfg signature */
> > >  #define FW_CFG_SIG_SIZE 4
> > >  
> > >  /* fw_cfg "file name" is up to 56 characters (including terminating nul)
> > >  */
> > >  #define FW_CFG_MAX_FILE_PATH 56
> > >  
> > > +/* platform device for dma mapping */
> > > +static struct device *dev;
> > > +
> > > +/* fw_cfg revision attribute, in /sys/firmware/qemu_fw_cfg top-level dir.
> > > */
> > > +static u32 fw_cfg_rev;
> > > +
> > >  /* fw_cfg file directory entry type */
> > >  struct fw_cfg_file {
> > >   u32 size;
> > > @@ -57,6 +73,12 @@ struct fw_cfg_file {
> > >   char name[FW_CFG_MAX_FILE_PATH];
> > >  };
> > >  
> > > +struct fw_cfg_dma {
> > > + u32 control;
> > > + u32 length;
> > > + u64 address;
> > > +} __packed;
> > > +
> > >  /* fw_cfg device i/o register addresses */
> > >  static bool fw_cfg_is_mmio;
> > >  static phys_addr_t fw_cfg_p_base;
> > > @@ -75,12 +97,93 @@ static inline u16 fw_cfg_sel_endianness(u16 key)
> > >   return fw_cfg_is_mmio ? cpu_to_be16(key) : cpu_to_le16(key);
> > >  }
> > >  
> > > +static inline bool fw_cfg_dma_enabled(void)
> > > +{
> > > + return fw_cfg_rev & FW_CFG_VERSION_DMA && fw_cfg_reg_dma;
> > > +}
> > > +
> > > +static bool fw_cfg_wait_for_control(struct fw_cfg_dma *d, unsigned long
> > > timeout)
> > > +{
> > > + ktime_t start;
> > > + ktime_t stop;
> > > +
> > > + start = ktime_get();
> > > + stop = ktime_add(start, ms_to_ktime(timeout));
> > > +
> > > + do {
> > > + if ((be32_to_cpu(d->control) & ~FW_CFG_DMA_CTL_ERROR) == 0)
> > > + return true;
> > > +
> > > + usleep_range(50, 100);
> > > + } while (ktime_before(ktime_get(), stop));
> > > +
> > > + return false;
> > > +}
> > > +
> > > +static ssize_t fw_cfg_dma_transfer(void *address, u32 length, u32 
> > > control)
> > > +{
> > > + dma_addr_t dma_addr = 0;
> > > + struct fw_cfg_dma *d;
> > > + dma_addr_t dma;
> > > + ssize_t ret = length;
> > > + enum dma_data_direction dir =
> > > + (control & FW_CFG_DMA_CTL_READ ? DMA_FROM_DEVICE : 0);
> > > +
> > > + if (address && length) {
> > > + dma_addr = dma_map_single(dev, address, length, dir);
> > > + if (dma_mapping_error(NULL, dma_addr)) {
> > > + WARN(1, "%s: failed to map address\n", __func__);
> > > + return -EFAULT;
> > > + }
> > > + }
> > > +
> > > + d = kmalloc(sizeof(*d), GFP_KERNEL | GFP_DMA);
> > > + if (!d) {
> > > + ret = -ENOMEM;
> > > + goto end;
> > > + }
> > > +
> > > + dma = dma_map_single(dev, d, sizeof(*d), DMA_BIDIRECTIONAL);
> > > + if (dma_mapping_error(NULL, dma)) {
> > > + WARN(1, "%s: failed to map fw_cfg_dma\n", __func__);
> > > + ret = -EFAULT;
> > > + goto end;
> > > + }
> > > +
> > > + *d = (struct fw_cfg_dma) {
> > > + .address = cpu_to_be64(dma_addr),
> > > + .length = cpu_to_be32(length),
> > > + .control = cpu_to_be32(control)
> > > + };
> > > +
> > > + iowrite32be((u64)dma >> 32, fw_cfg_reg_dma);
> > > + iowrite32be(dma, fw_cfg_reg_dma + 4);
> > > +
> > > + if (!fw_cfg_wait_for_control(d, FW_CFG_DMA_TIMEOUT)) {
> > > + WARN(1, "%s: timeout", __func__);
> 

Re: [Qemu-devel] [PATCH v1] highbank: validate register offset before access

[Philippe Mathieu-Daudé]

Hi Prasad,

Thanks for updating this patch so quickly :)

On 11/11/2017 05:11 AM, P J P wrote:
> From: Prasad J Pandit 
> 
> An 'offset' parameter sent to highbank register r/w functions
> could be greater than number(NUM_REGS=0x200) of hb registers,
> leading to an OOB access issue. Add check to avoid it.
> 
> Reported-by: Moguofang (Dennis mo) 
> Signed-off-by: Prasad J Pandit 
> ---
>  hw/arm/highbank.c | 15 ++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> Update: log error message(via qemu_log_mask) before returning
>   -> https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg01914.html
> 
> diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
> index 354c6b25a8..8494dc6a48 100644
> --- a/hw/arm/highbank.c
> +++ b/hw/arm/highbank.c
> @@ -34,6 +34,7 @@
>  #include "hw/ide/ahci.h"
>  #include "hw/cpu/a9mpcore.h"
>  #include "hw/cpu/a15mpcore.h"
> +#include "qemu/log.h"
>  
>  #define SMP_BOOT_ADDR   0x100
>  #define SMP_BOOT_REG0x40
> @@ -117,14 +118,26 @@ static void hb_regs_write(void *opaque, hwaddr offset,
>  }
>  }
>  
> +if (offset / 4 >= NUM_REGS) {
> +qemu_log_mask(LOG_GUEST_ERROR,
> +  "highbank: bad write offset 0x%x\n", (uint32_t)offset);

I'd rather use:

   "highbank: bad write offset 0x%" HWADDR_PRIx "\n", offset);

> +return;
> +}
>  regs[offset/4] = value;
>  }
>  
>  static uint64_t hb_regs_read(void *opaque, hwaddr offset,
>   unsigned size)
>  {
> +uint32_t value;
>  uint32_t *regs = opaque;
> -uint32_t value = regs[offset/4];
> +
> +if (offset / 4 >= NUM_REGS) {
> +qemu_log_mask(LOG_GUEST_ERROR,
> +  "highbank: bad read offset 0x%x\n", (uint32_t)offset);

Ditto HWADDR_PRIx.

> +return 0;
> +}
> +value = regs[offset/4];

  +value = regs[offset / 4];

With checkpatch fixed:
Reviewed-by: Philippe Mathieu-Daudé 

>  
>  if ((offset == 0x100) || (offset == 0x108) || (offset == 0x10C)) {
>  value |= 0x3000;
> 

Re: [Qemu-devel] [PATCH] linux-user, s390x: ignore OS ABI value in ELF header

[Philippe Mathieu-Daudé]

On 11/12/2017 06:23 AM, Laurent Vivier wrote:
> Le 12/11/2017 à 07:56, Thomas Huth a écrit :
>> Am Fri, 10 Nov 2017 20:49:35 +0100
>> schrieb Laurent Vivier :
>>
>>> I have this error:
>>> bash: /sbin/ldconfig: cannot execute binary file: Exec format error
>>>
>>> because /sbin/ldconfig is:
>>> ELF 64-bit MSB executable, IBM S/390, version 1 (GNU/Linux),
>>> statically linked, for GNU/Linux 3.2.0,
>>> BuildID[sha1]=90b64604014aafac9c1a0623b1cf447281d1a382, stripped
>>>
>>> OS ABI is GNU/linux
>>>
>>> "/bin/ls" works well:
>>>
>>> ELF 64-bit MSB shared object, IBM S/390, version 1 (SYSV),
>>> dynamically linked, interpreter /lib/ld64.so.1, for GNU/Linux 3.2.0,
>>> BuildID[sha1]=be9b19143d4657678846f6e5277383071fc1059a, stripped
>>>
>>> OS ABI is SYSV
>>>
>>> To be able to execute ldconfig, this patch modifies s390x binfmt mask
>>> to ignore the OS ABI value (EI_OSABI, byte 7).
>>>
>>> Signed-off-by: Laurent Vivier 
>>> ---
>>>  scripts/qemu-binfmt-conf.sh | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
>>> index 8afc3eb5bb..e2e1b7544d 100755
>>> --- a/scripts/qemu-binfmt-conf.sh
>>> +++ b/scripts/qemu-binfmt-conf.sh
>>> @@ -85,7 +85,7 @@
>>> sh4eb_mask='\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff
>>> sh4eb_family=sh4 
>>>  
>>> s390x_magic='\x7fELF\x02\x02\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x16'
>>> -s390x_mask='\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff'
>>> +s390x_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff'
>>
>> If I've got that right, the OSABI field should either be 0 for
>> "No extensions or unspecified" (which is then printed  by "file"
>> as SYSV) or 3 for "GNU/Linux".
>> Thus wouldn't it be better to use a mask of 0xfc here instead, so
>> that we refuse at least everything with a value > 3 here?
> No, because with this mask you allow HPUX and NetBSD (See
> /usr/include/elf.h), I don't think it's really better. The simplest way
> to manage this is to ignore the field. A better way should be to have
> two binfmt entries, one for SYSV and one for GNU/Linux, but as our goal
> is to run linux binaries on linux system, is it worth it?

This was my first thought when I reviewed your patch, this indeed looks
cleaner and would be self-explanatory.

Regards,

Phil.

Re: [Qemu-devel] [PATCH] SDL2 various fixes

[Philippe Mathieu-Daudé]

Hi Jindrich,

On 11/12/2017 07:42 AM, Jindrich Makovicka wrote:
> Hi,
> 
> here is a revised patchset:

Please increment the patchset version when you respin (this is v2).

> 
> - sdl2: Do not hide the cursor on auxilliary windows 
> split into two
> 
> - sdl2: Only accept the hotkeys on the main window 
> reworked, also with a more descriptive commit message
> 
> - sdl2: Do not quit the emulator when an auxilliary window is closed
> added missing scon->hidden update
> 
> - sdl2: Fix broken display updating after the window is hidden
> added scon->hidden update instead + the SHOWN part changed
> the same way

You missed to add your Signed-off-by tag in the whole series.

See:
https://wiki.qemu.org/Contribute/SubmitAPatch#Patch_emails_must_include_a_Signed-off-by:_line

This can be added automatically using the "-s" option of the "git
format-patch" command, also the resend version using the "-v" option.

(https://wiki.qemu.org/Contribute/SubmitAPatch#Use_git_format-patch)

Regards,

Phil.

Re: [Qemu-devel] [PATCH v4 3/5] fw_cfg: do DMA read operation

[Marc-André Lureau]

- Original Message -
> On Tue, Oct 31, 2017 at 04:19:36PM +0100, Marc-André Lureau wrote:
> > Modify fw_cfg_read_blob() to use DMA if the device supports it.
> > Return errors, because the operation may fail.
> > 
> > To avoid polling with unbound amount of time, the DMA operation is
> > expected to complete within 200ms, or will return ETIME error.
> > 
> > We may want to switch all the *buf addresses to use only kmalloc'ed
> > buffers (instead of using stack/image addresses with dma=false).
> > 
> > Signed-off-by: Marc-André Lureau 
> 
> A couple of silly questions, inline, below.
> 
> Thanks,
> --G
> 
> > ---
> >  drivers/firmware/qemu_fw_cfg.c | 154
> >  -
> >  1 file changed, 137 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/firmware/qemu_fw_cfg.c
> > b/drivers/firmware/qemu_fw_cfg.c
> > index 1f3e8545dab7..8ce5e49b7c62 100644
> > --- a/drivers/firmware/qemu_fw_cfg.c
> > +++ b/drivers/firmware/qemu_fw_cfg.c
> > @@ -33,6 +33,8 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> > +#include 
> >  
> >  MODULE_AUTHOR("Gabriel L. Somlo ");
> >  MODULE_DESCRIPTION("QEMU fw_cfg sysfs support");
> > @@ -43,12 +45,26 @@ MODULE_LICENSE("GPL");
> >  #define FW_CFG_ID 0x01
> >  #define FW_CFG_FILE_DIR   0x19
> >  
> > +#define FW_CFG_VERSION_DMA 0x02
> > +#define FW_CFG_DMA_CTL_ERROR   0x01
> > +#define FW_CFG_DMA_CTL_READ0x02
> > +#define FW_CFG_DMA_CTL_SKIP0x04
> > +#define FW_CFG_DMA_CTL_SELECT  0x08
> > +#define FW_CFG_DMA_CTL_WRITE   0x10
> > +#define FW_CFG_DMA_TIMEOUT 200 /* ms */
> > +
> >  /* size in bytes of fw_cfg signature */
> >  #define FW_CFG_SIG_SIZE 4
> >  
> >  /* fw_cfg "file name" is up to 56 characters (including terminating nul)
> >  */
> >  #define FW_CFG_MAX_FILE_PATH 56
> >  
> > +/* platform device for dma mapping */
> > +static struct device *dev;
> > +
> > +/* fw_cfg revision attribute, in /sys/firmware/qemu_fw_cfg top-level dir.
> > */
> > +static u32 fw_cfg_rev;
> > +
> >  /* fw_cfg file directory entry type */
> >  struct fw_cfg_file {
> > u32 size;
> > @@ -57,6 +73,12 @@ struct fw_cfg_file {
> > char name[FW_CFG_MAX_FILE_PATH];
> >  };
> >  
> > +struct fw_cfg_dma {
> > +   u32 control;
> > +   u32 length;
> > +   u64 address;
> > +} __packed;
> > +
> >  /* fw_cfg device i/o register addresses */
> >  static bool fw_cfg_is_mmio;
> >  static phys_addr_t fw_cfg_p_base;
> > @@ -75,12 +97,93 @@ static inline u16 fw_cfg_sel_endianness(u16 key)
> > return fw_cfg_is_mmio ? cpu_to_be16(key) : cpu_to_le16(key);
> >  }
> >  
> > +static inline bool fw_cfg_dma_enabled(void)
> > +{
> > +   return fw_cfg_rev & FW_CFG_VERSION_DMA && fw_cfg_reg_dma;
> > +}
> > +
> > +static bool fw_cfg_wait_for_control(struct fw_cfg_dma *d, unsigned long
> > timeout)
> > +{
> > +   ktime_t start;
> > +   ktime_t stop;
> > +
> > +   start = ktime_get();
> > +   stop = ktime_add(start, ms_to_ktime(timeout));
> > +
> > +   do {
> > +   if ((be32_to_cpu(d->control) & ~FW_CFG_DMA_CTL_ERROR) == 0)
> > +   return true;
> > +
> > +   usleep_range(50, 100);
> > +   } while (ktime_before(ktime_get(), stop));
> > +
> > +   return false;
> > +}
> > +
> > +static ssize_t fw_cfg_dma_transfer(void *address, u32 length, u32 control)
> > +{
> > +   dma_addr_t dma_addr = 0;
> > +   struct fw_cfg_dma *d;
> > +   dma_addr_t dma;
> > +   ssize_t ret = length;
> > +   enum dma_data_direction dir =
> > +   (control & FW_CFG_DMA_CTL_READ ? DMA_FROM_DEVICE : 0);
> > +
> > +   if (address && length) {
> > +   dma_addr = dma_map_single(dev, address, length, dir);
> > +   if (dma_mapping_error(NULL, dma_addr)) {
> > +   WARN(1, "%s: failed to map address\n", __func__);
> > +   return -EFAULT;
> > +   }
> > +   }
> > +
> > +   d = kmalloc(sizeof(*d), GFP_KERNEL | GFP_DMA);
> > +   if (!d) {
> > +   ret = -ENOMEM;
> > +   goto end;
> > +   }
> > +
> > +   dma = dma_map_single(dev, d, sizeof(*d), DMA_BIDIRECTIONAL);
> > +   if (dma_mapping_error(NULL, dma)) {
> > +   WARN(1, "%s: failed to map fw_cfg_dma\n", __func__);
> > +   ret = -EFAULT;
> > +   goto end;
> > +   }
> > +
> > +   *d = (struct fw_cfg_dma) {
> > +   .address = cpu_to_be64(dma_addr),
> > +   .length = cpu_to_be32(length),
> > +   .control = cpu_to_be32(control)
> > +   };
> > +
> > +   iowrite32be((u64)dma >> 32, fw_cfg_reg_dma);
> > +   iowrite32be(dma, fw_cfg_reg_dma + 4);
> > +
> > +   if (!fw_cfg_wait_for_control(d, FW_CFG_DMA_TIMEOUT)) {
> > +   WARN(1, "%s: timeout", __func__);
> > +   ret = -ETIME;
> > +   } else if (be32_to_cpu(d->control) & FW_CFG_DMA_CTL_ERROR) {
> > +   ret = -EIO;
> > +   }
> > +
> > +   dma_unmap_single(dev, dma, sizeof(*d), DMA_BIDIRECTIONAL);
> > +
> > +end:
> > +   kfree(d);
> > +   if (dma_addr)
> > +  

Re: [Qemu-devel] [PATCH v4 5/5] fw_cfg: write vmcoreinfo details

[Gabriel L. Somlo]

On Tue, Oct 31, 2017 at 04:19:38PM +0100, Marc-André Lureau wrote:
> If the "etc/vmcoreinfo" fw_cfg file is present and we are not running
> the kdump kernel, write the addr/size of the vmcoreinfo ELF note.
> 
> Signed-off-by: Marc-André Lureau 

Same two questions as for 3/5 earlier :)

Thanks,
--G

> ---
>  drivers/firmware/qemu_fw_cfg.c | 87 
> +-
>  1 file changed, 86 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
> index 8ce5e49b7c62..ebd9bb134900 100644
> --- a/drivers/firmware/qemu_fw_cfg.c
> +++ b/drivers/firmware/qemu_fw_cfg.c
> @@ -35,6 +35,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  
>  MODULE_AUTHOR("Gabriel L. Somlo ");
>  MODULE_DESCRIPTION("QEMU fw_cfg sysfs support");
> @@ -59,6 +61,8 @@ MODULE_LICENSE("GPL");
>  /* fw_cfg "file name" is up to 56 characters (including terminating nul) */
>  #define FW_CFG_MAX_FILE_PATH 56
>  
> +#define VMCOREINFO_FORMAT_ELF 0x1
> +
>  /* platform device for dma mapping */
>  static struct device *dev;
>  
> @@ -127,7 +131,8 @@ static ssize_t fw_cfg_dma_transfer(void *address, u32 
> length, u32 control)
>   dma_addr_t dma;
>   ssize_t ret = length;
>   enum dma_data_direction dir =
> - (control & FW_CFG_DMA_CTL_READ ? DMA_FROM_DEVICE : 0);
> + (control & FW_CFG_DMA_CTL_READ ? DMA_FROM_DEVICE : 0) |
> + (control & FW_CFG_DMA_CTL_WRITE ? DMA_TO_DEVICE : 0);
>  
>   if (address && length) {
>   dma_addr = dma_map_single(dev, address, length, dir);
> @@ -225,6 +230,48 @@ static ssize_t fw_cfg_read_blob(u16 key,
>   return ret;
>  }
>  
> +#ifdef CONFIG_CRASH_CORE
> +/* write chunk of given fw_cfg blob (caller responsible for sanity-check) */
> +static ssize_t fw_cfg_write_blob(u16 key,
> +  void *buf, loff_t pos, size_t count)
> +{
> + u32 glk = -1U;
> + acpi_status status;
> + ssize_t ret = count;
> +
> + /* If we have ACPI, ensure mutual exclusion against any potential
> +  * device access by the firmware, e.g. via AML methods:
> +  */
> + status = acpi_acquire_global_lock(ACPI_WAIT_FOREVER, );
> + if (ACPI_FAILURE(status) && status != AE_NOT_CONFIGURED) {
> + /* Should never get here */
> + WARN(1, "%s: Failed to lock ACPI!\n", __func__);
> + memset(buf, 0, count);
> + return -EBUSY;

Same question as before: why not e.g. EINVAL?

> + }
> +
> + mutex_lock(_cfg_dev_lock);
> + if (pos == 0) {
> + ret = fw_cfg_dma_transfer(buf, count, key << 16
> +   | FW_CFG_DMA_CTL_SELECT
> +   | FW_CFG_DMA_CTL_WRITE);
> + } else {
> + iowrite16(fw_cfg_sel_endianness(key), fw_cfg_reg_ctrl);
> + ret = fw_cfg_dma_transfer(0, pos, FW_CFG_DMA_CTL_SKIP);

NULL?

> + if (ret < 0)
> + goto end;
> + ret = fw_cfg_dma_transfer(buf, count, FW_CFG_DMA_CTL_WRITE);
> + }
> +
> +end:
> + mutex_unlock(_cfg_dev_lock);
> +
> + acpi_release_global_lock(glk);
> +
> + return ret;
> +}
> +#endif /* CONFIG_CRASH_CORE */
> +
>  /* clean up fw_cfg device i/o */
>  static void fw_cfg_io_cleanup(void)
>  {
> @@ -343,6 +390,37 @@ struct fw_cfg_sysfs_entry {
>   struct list_head list;
>  };
>  
> +#ifdef CONFIG_CRASH_CORE
> +static ssize_t write_vmcoreinfo(const struct fw_cfg_file *f)
> +{
> + struct vmci {
> + __le16 host_format;
> + __le16 guest_format;
> + __le32 size;
> + __le64 paddr;
> + } __packed;
> + struct vmci *data;
> + ssize_t ret;
> +
> + data = kmalloc(sizeof(struct vmci), GFP_KERNEL | GFP_DMA);
> + if (!data)
> + return -ENOMEM;
> +
> + /* spare ourself reading host format support for now since we
> +  * don't know what else to format - host may ignore ours
> +  */
> + *data = (struct vmci) {
> + .guest_format = cpu_to_le16(VMCOREINFO_FORMAT_ELF),
> + .size = cpu_to_le32(VMCOREINFO_NOTE_SIZE),
> + .paddr = cpu_to_le64(paddr_vmcoreinfo_note())
> + };
> + ret = fw_cfg_write_blob(f->select, data, 0, sizeof(struct vmci));
> +
> + kfree(data);
> + return ret;
> +}
> +#endif /* CONFIG_CRASH_CORE */
> +
>  /* get fw_cfg_sysfs_entry from kobject member */
>  static inline struct fw_cfg_sysfs_entry *to_entry(struct kobject *kobj)
>  {
> @@ -582,6 +660,13 @@ static int fw_cfg_register_file(const struct fw_cfg_file 
> *f)
>   int err;
>   struct fw_cfg_sysfs_entry *entry;
>  
> +#ifdef CONFIG_CRASH_CORE
> + if (strcmp(f->name, "etc/vmcoreinfo") == 0 && !is_kdump_kernel()) {
> + if (write_vmcoreinfo(f) < 0)
> + pr_warn("fw_cfg: failed to write vmcoreinfo");
> + }
> 

Re: [Qemu-devel] [PATCH v4 4/5] crash: export paddr_vmcoreinfo_note()

[Gabriel L. Somlo]

On Tue, Oct 31, 2017 at 04:19:37PM +0100, Marc-André Lureau wrote:
> The following patch is going to use the symbol from the fw_cfg module.
> 
> Signed-off-by: Marc-André Lureau 

Acked-by: Gabriel Somlo 

> ---
>  kernel/crash_core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 6db80fc0810b..47541c891810 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -375,6 +375,7 @@ phys_addr_t __weak paddr_vmcoreinfo_note(void)
>  {
>   return __pa(vmcoreinfo_note);
>  }
> +EXPORT_SYMBOL(paddr_vmcoreinfo_note);
>  
>  static int __init crash_save_vmcoreinfo_init(void)
>  {
> -- 
> 2.15.0.rc0.40.gaefcc5f6f
> 

Re: [Qemu-devel] [PATCH v4 3/5] fw_cfg: do DMA read operation

[Gabriel L. Somlo]

On Tue, Oct 31, 2017 at 04:19:36PM +0100, Marc-André Lureau wrote:
> Modify fw_cfg_read_blob() to use DMA if the device supports it.
> Return errors, because the operation may fail.
> 
> To avoid polling with unbound amount of time, the DMA operation is
> expected to complete within 200ms, or will return ETIME error.
> 
> We may want to switch all the *buf addresses to use only kmalloc'ed
> buffers (instead of using stack/image addresses with dma=false).
> 
> Signed-off-by: Marc-André Lureau 

A couple of silly questions, inline, below.

Thanks,
--G

> ---
>  drivers/firmware/qemu_fw_cfg.c | 154 
> -
>  1 file changed, 137 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
> index 1f3e8545dab7..8ce5e49b7c62 100644
> --- a/drivers/firmware/qemu_fw_cfg.c
> +++ b/drivers/firmware/qemu_fw_cfg.c
> @@ -33,6 +33,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  
>  MODULE_AUTHOR("Gabriel L. Somlo ");
>  MODULE_DESCRIPTION("QEMU fw_cfg sysfs support");
> @@ -43,12 +45,26 @@ MODULE_LICENSE("GPL");
>  #define FW_CFG_ID 0x01
>  #define FW_CFG_FILE_DIR   0x19
>  
> +#define FW_CFG_VERSION_DMA 0x02
> +#define FW_CFG_DMA_CTL_ERROR   0x01
> +#define FW_CFG_DMA_CTL_READ0x02
> +#define FW_CFG_DMA_CTL_SKIP0x04
> +#define FW_CFG_DMA_CTL_SELECT  0x08
> +#define FW_CFG_DMA_CTL_WRITE   0x10
> +#define FW_CFG_DMA_TIMEOUT 200 /* ms */
> +
>  /* size in bytes of fw_cfg signature */
>  #define FW_CFG_SIG_SIZE 4
>  
>  /* fw_cfg "file name" is up to 56 characters (including terminating nul) */
>  #define FW_CFG_MAX_FILE_PATH 56
>  
> +/* platform device for dma mapping */
> +static struct device *dev;
> +
> +/* fw_cfg revision attribute, in /sys/firmware/qemu_fw_cfg top-level dir. */
> +static u32 fw_cfg_rev;
> +
>  /* fw_cfg file directory entry type */
>  struct fw_cfg_file {
>   u32 size;
> @@ -57,6 +73,12 @@ struct fw_cfg_file {
>   char name[FW_CFG_MAX_FILE_PATH];
>  };
>  
> +struct fw_cfg_dma {
> + u32 control;
> + u32 length;
> + u64 address;
> +} __packed;
> +
>  /* fw_cfg device i/o register addresses */
>  static bool fw_cfg_is_mmio;
>  static phys_addr_t fw_cfg_p_base;
> @@ -75,12 +97,93 @@ static inline u16 fw_cfg_sel_endianness(u16 key)
>   return fw_cfg_is_mmio ? cpu_to_be16(key) : cpu_to_le16(key);
>  }
>  
> +static inline bool fw_cfg_dma_enabled(void)
> +{
> + return fw_cfg_rev & FW_CFG_VERSION_DMA && fw_cfg_reg_dma;
> +}
> +
> +static bool fw_cfg_wait_for_control(struct fw_cfg_dma *d, unsigned long 
> timeout)
> +{
> + ktime_t start;
> + ktime_t stop;
> +
> + start = ktime_get();
> + stop = ktime_add(start, ms_to_ktime(timeout));
> +
> + do {
> + if ((be32_to_cpu(d->control) & ~FW_CFG_DMA_CTL_ERROR) == 0)
> + return true;
> +
> + usleep_range(50, 100);
> + } while (ktime_before(ktime_get(), stop));
> +
> + return false;
> +}
> +
> +static ssize_t fw_cfg_dma_transfer(void *address, u32 length, u32 control)
> +{
> + dma_addr_t dma_addr = 0;
> + struct fw_cfg_dma *d;
> + dma_addr_t dma;
> + ssize_t ret = length;
> + enum dma_data_direction dir =
> + (control & FW_CFG_DMA_CTL_READ ? DMA_FROM_DEVICE : 0);
> +
> + if (address && length) {
> + dma_addr = dma_map_single(dev, address, length, dir);
> + if (dma_mapping_error(NULL, dma_addr)) {
> + WARN(1, "%s: failed to map address\n", __func__);
> + return -EFAULT;
> + }
> + }
> +
> + d = kmalloc(sizeof(*d), GFP_KERNEL | GFP_DMA);
> + if (!d) {
> + ret = -ENOMEM;
> + goto end;
> + }
> +
> + dma = dma_map_single(dev, d, sizeof(*d), DMA_BIDIRECTIONAL);
> + if (dma_mapping_error(NULL, dma)) {
> + WARN(1, "%s: failed to map fw_cfg_dma\n", __func__);
> + ret = -EFAULT;
> + goto end;
> + }
> +
> + *d = (struct fw_cfg_dma) {
> + .address = cpu_to_be64(dma_addr),
> + .length = cpu_to_be32(length),
> + .control = cpu_to_be32(control)
> + };
> +
> + iowrite32be((u64)dma >> 32, fw_cfg_reg_dma);
> + iowrite32be(dma, fw_cfg_reg_dma + 4);
> +
> + if (!fw_cfg_wait_for_control(d, FW_CFG_DMA_TIMEOUT)) {
> + WARN(1, "%s: timeout", __func__);
> + ret = -ETIME;
> + } else if (be32_to_cpu(d->control) & FW_CFG_DMA_CTL_ERROR) {
> + ret = -EIO;
> + }
> +
> + dma_unmap_single(dev, dma, sizeof(*d), DMA_BIDIRECTIONAL);
> +
> +end:
> + kfree(d);
> + if (dma_addr)
> + dma_unmap_single(dev, dma_addr, length, dir);
> +
> + return ret;
> +}
> +
>  /* read chunk of given fw_cfg blob (caller responsible for sanity-check) */
> -static inline void fw_cfg_read_blob(u16 key,
> -   

Re: [Qemu-devel] [PATCH v4 2/5] fw_cfg: add DMA register

[Gabriel L. Somlo]

On Tue, Oct 31, 2017 at 04:19:35PM +0100, Marc-André Lureau wrote:
> Add an optional  kernel module (or command line) parameter
> using the following syntax:
> 
>   [qemu_fw_cfg.]ioport=@[::[:]]
>  or
>   [qemu_fw_cfg.]mmio=@[::[:]]
> 
> and initializes the register address using given or default offset.
> 
> Signed-off-by: Marc-André Lureau 

Reviewed-by: Gabriel Somlo 

> ---
>  drivers/firmware/qemu_fw_cfg.c | 53 
> --
>  1 file changed, 41 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
> index 5cfe39f7a45f..1f3e8545dab7 100644
> --- a/drivers/firmware/qemu_fw_cfg.c
> +++ b/drivers/firmware/qemu_fw_cfg.c
> @@ -10,20 +10,21 @@
>   * and select subsets of aarch64), a Device Tree node (on arm), or using
>   * a kernel module (or command line) parameter with the following syntax:
>   *
> - *  [qemu_fw_cfg.]ioport=@[::]
> + *  
> [qemu_fw_cfg.]ioport=@[::[:]]
>   * or
> - *  [qemu_fw_cfg.]mmio=@[::]
> + *  [qemu_fw_cfg.]mmio=@[::[:]]
>   *
>   * where:
>   *   := size of ioport or mmio range
>   *   := physical base address of ioport or mmio range
>   *   := (optional) offset of control register
>   *   := (optional) offset of data register
> + *   := (optional) offset of dma register
>   *
>   * e.g.:
> - *  qemu_fw_cfg.ioport=2@0x510:0:1   (the default on x86)
> + *  qemu_fw_cfg.ioport=12@0x510:0:1:4(the default on x86)
>   * or
> - *  qemu_fw_cfg.mmio=0xA@0x902:8:0   (the default on arm)
> + *  qemu_fw_cfg.mmio=16@0x902:8:0:16 (the default on arm)
>   */
>  
>  #include 
> @@ -63,6 +64,7 @@ static resource_size_t fw_cfg_p_size;
>  static void __iomem *fw_cfg_dev_base;
>  static void __iomem *fw_cfg_reg_ctrl;
>  static void __iomem *fw_cfg_reg_data;
> +static void __iomem *fw_cfg_reg_dma;
>  
>  /* atomic access to fw_cfg device (potentially slow i/o, so using mutex) */
>  static DEFINE_MUTEX(fw_cfg_dev_lock);
> @@ -118,12 +120,14 @@ static void fw_cfg_io_cleanup(void)
>  # if (defined(CONFIG_ARM) || defined(CONFIG_ARM64))
>  #  define FW_CFG_CTRL_OFF 0x08
>  #  define FW_CFG_DATA_OFF 0x00
> +#  define FW_CFG_DMA_OFF 0x10
>  # elif (defined(CONFIG_PPC_PMAC) || defined(CONFIG_SPARC32)) /* 
> ppc/mac,sun4m */
>  #  define FW_CFG_CTRL_OFF 0x00
>  #  define FW_CFG_DATA_OFF 0x02
>  # elif (defined(CONFIG_X86) || defined(CONFIG_SPARC64)) /* x86, sun4u */
>  #  define FW_CFG_CTRL_OFF 0x00
>  #  define FW_CFG_DATA_OFF 0x01
> +#  define FW_CFG_DMA_OFF 0x04
>  # else
>  #  error "QEMU FW_CFG not available on this architecture!"
>  # endif
> @@ -133,7 +137,7 @@ static void fw_cfg_io_cleanup(void)
>  static int fw_cfg_do_platform_probe(struct platform_device *pdev)
>  {
>   char sig[FW_CFG_SIG_SIZE];
> - struct resource *range, *ctrl, *data;
> + struct resource *range, *ctrl, *data, *dma;
>  
>   /* acquire i/o range details */
>   fw_cfg_is_mmio = false;
> @@ -170,6 +174,7 @@ static int fw_cfg_do_platform_probe(struct 
> platform_device *pdev)
>   /* were custom register offsets provided (e.g. on the command line)? */
>   ctrl = platform_get_resource_byname(pdev, IORESOURCE_REG, "ctrl");
>   data = platform_get_resource_byname(pdev, IORESOURCE_REG, "data");
> + dma = platform_get_resource_byname(pdev, IORESOURCE_REG, "dma");
>   if (ctrl && data) {
>   fw_cfg_reg_ctrl = fw_cfg_dev_base + ctrl->start;
>   fw_cfg_reg_data = fw_cfg_dev_base + data->start;
> @@ -179,6 +184,13 @@ static int fw_cfg_do_platform_probe(struct 
> platform_device *pdev)
>   fw_cfg_reg_data = fw_cfg_dev_base + FW_CFG_DATA_OFF;
>   }
>  
> + if (dma)
> + fw_cfg_reg_dma = fw_cfg_dev_base + dma->start;
> +#ifdef FW_CFG_DMA_OFF
> + else
> + fw_cfg_reg_dma = fw_cfg_dev_base + FW_CFG_DMA_OFF;
> +#endif
> +
>   /* verify fw_cfg device signature */
>   fw_cfg_read_blob(FW_CFG_SIGNATURE, sig, 0, FW_CFG_SIG_SIZE);
>   if (memcmp(sig, "QEMU", FW_CFG_SIG_SIZE) != 0) {
> @@ -628,6 +640,7 @@ static struct platform_device *fw_cfg_cmdline_dev;
>  /* use special scanf/printf modifier for phys_addr_t, resource_size_t */
>  #define PH_ADDR_SCAN_FMT "@%" __PHYS_ADDR_PREFIX "i%n" \
>":%" __PHYS_ADDR_PREFIX "i" \
> +  ":%" __PHYS_ADDR_PREFIX "i%n" \
>":%" __PHYS_ADDR_PREFIX "i%n"
>  
>  #define PH_ADDR_PR_1_FMT "0x%" __PHYS_ADDR_PREFIX "x@" \
> @@ -637,12 +650,15 @@ static struct platform_device *fw_cfg_cmdline_dev;
>":%" __PHYS_ADDR_PREFIX "u" \
>":%" __PHYS_ADDR_PREFIX "u"
>  
> +#define PH_ADDR_PR_4_FMT PH_ADDR_PR_3_FMT \
> +  ":%" __PHYS_ADDR_PREFIX "u"
> +
>  static int fw_cfg_cmdline_set(const char *arg, const struct kernel_param *kp)
>  {
> - struct resource 

Re: [Qemu-devel] [PATCH v4 1/5] fw_cfg: fix the command line module name

[Gabriel L. Somlo]

On Tue, Oct 31, 2017 at 04:19:34PM +0100, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau 

Acked-by: Gabriel Somlo 

> ---
>  drivers/firmware/qemu_fw_cfg.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
> index 0e2011636fbb..5cfe39f7a45f 100644
> --- a/drivers/firmware/qemu_fw_cfg.c
> +++ b/drivers/firmware/qemu_fw_cfg.c
> @@ -10,9 +10,9 @@
>   * and select subsets of aarch64), a Device Tree node (on arm), or using
>   * a kernel module (or command line) parameter with the following syntax:
>   *
> - *  [fw_cfg.]ioport=@[::]
> + *  [qemu_fw_cfg.]ioport=@[::]
>   * or
> - *  [fw_cfg.]mmio=@[::]
> + *  [qemu_fw_cfg.]mmio=@[::]
>   *
>   * where:
>   *   := size of ioport or mmio range
> @@ -21,9 +21,9 @@
>   *   := (optional) offset of data register
>   *
>   * e.g.:
> - *  fw_cfg.ioport=2@0x510:0:1(the default on x86)
> + *  qemu_fw_cfg.ioport=2@0x510:0:1   (the default on x86)
>   * or
> - *  fw_cfg.mmio=0xA@0x902:8:0(the default on arm)
> + *  qemu_fw_cfg.mmio=0xA@0x902:8:0   (the default on arm)
>   */
>  
>  #include 
> -- 
> 2.15.0.rc0.40.gaefcc5f6f
> 

Re: [Qemu-devel] [PATCH v4 0/5] fw_cfg: add DMA operations & etc/vmcoreinfo support

[Gabriel L. Somlo]

On Tue, Oct 31, 2017 at 04:19:33PM +0100, Marc-André Lureau wrote:
> Hi,
> 
> This series adds DMA operations support to the qemu fw_cfg kernel
> module and populates "etc/vmcoreinfo" with vmcoreinfo location
> details.
> 
> Note: the support for this entry handling has been merged for next
> qemu release (2.11)
> 
> v4:
> - export paddr_vmcoreinfo_note() to fix fw_cfg.ko build
> - fix build with !CONFIG_CRASH_CORE
> - replace the unbounded yield() loop with a usleep_range() loop and a
>   200ms timeout
> - do not write vmcoreinfo entry when running the kdump kernel (D. Hatayama)
> - drop the experimental sysfs write support patch from this series

Tested it on x86_64, appears to work fine.
I have a couple of silly questions re. 3/5 and 5/5. Lost the v5 emails
with the vmcore folks cc-ed, but my questions are unrelated to vmcore
anyway :)

Thanks,
--Gabriel

> 
> v3: (thanks kbuild)
> - add "fw_cfg: fix the command line module name" patch
> - fix build of "fw_cfg: add DMA register" with CONFIG_FW_CFG_SYSFS_CMDLINE=y
> - fix 'Wshift-count-overflow'
> 
> v2:
> - use platform device for dma mapping
> - add etc/vmcoreinfo patch
> - some code cleanups
> 
> Marc-André Lureau (5):
>   fw_cfg: fix the command line module name
>   fw_cfg: add DMA register
>   fw_cfg: do DMA read operation
>   crash: export paddr_vmcoreinfo_note()
>   fw_cfg: write vmcoreinfo details
> 
>  drivers/firmware/qemu_fw_cfg.c | 292 
> +
>  kernel/crash_core.c|   1 +
>  2 files changed, 264 insertions(+), 29 deletions(-)
> 
> -- 
> 2.15.0.rc0.40.gaefcc5f6f
> 

[Qemu-devel] [PATCH v3] virtio_balloon: include disk/file caches memory statistics

[Tomáš Golembiovský]

Add a new field VIRTIO_BALLOON_S_CACHES to virtio_balloon memory
statistics protocol. The value represents all disk/file caches.

In this case it corresponds to the sum of values
Buffers+Cached+SwapCached from /proc/meminfo.

Signed-off-by: Tomáš Golembiovský 
---
 drivers/virtio/virtio_balloon.c | 4 
 include/uapi/linux/virtio_balloon.h | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index f0b3a0b9d42f..d2bd13bbaf9f 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -244,11 +244,13 @@ static unsigned int update_balloon_stats(struct 
virtio_balloon *vb)
struct sysinfo i;
unsigned int idx = 0;
long available;
+   unsigned long caches;
 
all_vm_events(events);
si_meminfo();
 
available = si_mem_available();
+   caches = global_node_page_state(NR_FILE_PAGES);
 
 #ifdef CONFIG_VM_EVENT_COUNTERS
update_stat(vb, idx++, VIRTIO_BALLOON_S_SWAP_IN,
@@ -264,6 +266,8 @@ static unsigned int update_balloon_stats(struct 
virtio_balloon *vb)
pages_to_bytes(i.totalram));
update_stat(vb, idx++, VIRTIO_BALLOON_S_AVAIL,
pages_to_bytes(available));
+   update_stat(vb, idx++, VIRTIO_BALLOON_S_CACHES,
+   pages_to_bytes(caches));
 
return idx;
 }
diff --git a/include/uapi/linux/virtio_balloon.h 
b/include/uapi/linux/virtio_balloon.h
index 343d7ddefe04..4e8b8304b793 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -52,7 +52,8 @@ struct virtio_balloon_config {
 #define VIRTIO_BALLOON_S_MEMFREE  4   /* Total amount of free memory */
 #define VIRTIO_BALLOON_S_MEMTOT   5   /* Total amount of memory */
 #define VIRTIO_BALLOON_S_AVAIL6   /* Available memory as in /proc */
-#define VIRTIO_BALLOON_S_NR   7
+#define VIRTIO_BALLOON_S_CACHES   7   /* Disk caches */
+#define VIRTIO_BALLOON_S_NR   8
 
 /*
  * Memory statistics structure.
-- 
2.15.0

[Qemu-devel] [PATCH 8/8] sdl2: Ignore UI hotkeys after a focus change when GUI modifier is held

[Jindrich Makovicka]

When SDL2 windows change focus while a key is held, the window that
receives the focus also receives a new KeyDown event, without an
autorepeat flag. This means that if a WM places the qemu console
over the main window after Ctrl-Alt-2, the console closes immediately
after opening. Then, the main window receives the KeyDown event again
and the whole process repeats.

This patch makes the SDL2 UI ignore the KeyDown events on a window that
just received the focus, if the GUI modifier was held. The ignore flag
is reset on a first KeyUp event. This effectively works around the issue
above.
---
 include/ui/sdl2.h |  1 +
 ui/sdl2.c | 25 -
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
index b29cf803c9..51084e6320 100644
--- a/include/ui/sdl2.h
+++ b/include/ui/sdl2.h
@@ -24,6 +24,7 @@ struct sdl2_console {
 int opengl;
 int updates;
 int idle_counter;
+int ignore_hotkeys;
 SDL_GLContext winctx;
 #ifdef CONFIG_OPENGL
 QemuGLShader *gls;
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 6feb637739..2d48686b83 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -310,22 +310,26 @@ static void toggle_full_screen(struct sdl2_console *scon)
 sdl2_redraw(scon);
 }
 
-static void handle_keydown(SDL_Event *ev)
+static int get_mod_state(void)
 {
-int mod_state, win;
-struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
-
 if (alt_grab) {
-mod_state = (SDL_GetModState() & (gui_grab_code | KMOD_LSHIFT)) ==
+return (SDL_GetModState() & (gui_grab_code | KMOD_LSHIFT)) ==
 (gui_grab_code | KMOD_LSHIFT);
 } else if (ctrl_grab) {
-mod_state = (SDL_GetModState() & KMOD_RCTRL) == KMOD_RCTRL;
+return (SDL_GetModState() & KMOD_RCTRL) == KMOD_RCTRL;
 } else {
-mod_state = (SDL_GetModState() & gui_grab_code) == gui_grab_code;
+return (SDL_GetModState() & gui_grab_code) == gui_grab_code;
 }
-gui_key_modifier_pressed = mod_state;
+}
+
+static void handle_keydown(SDL_Event *ev)
+{
+int win;
+struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
 
-if (gui_key_modifier_pressed) {
+gui_key_modifier_pressed = get_mod_state();
+
+if (!scon->ignore_hotkeys && gui_key_modifier_pressed && !ev->key.repeat) {
 switch (ev->key.keysym.scancode) {
 case SDL_SCANCODE_2:
 case SDL_SCANCODE_3:
@@ -399,6 +403,8 @@ static void handle_keyup(SDL_Event *ev)
 int mod_state;
 struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
 
+scon->ignore_hotkeys = false;
+
 if (!alt_grab) {
 mod_state = (ev->key.keysym.mod & gui_grab_code);
 } else {
@@ -545,6 +551,7 @@ static void handle_windowevent(SDL_Event *ev)
 if (!gui_grab && (qemu_input_is_absolute() || absolute_enabled)) {
 absolute_mouse_grab(scon);
 }
+scon->ignore_hotkeys = get_mod_state();
 break;
 case SDL_WINDOWEVENT_FOCUS_LOST:
 if (gui_grab && !gui_fullscreen) {
-- 
2.15.0

[Qemu-devel] [PATCH 7/8] sdl2: Do not leave grab when fullscreen

[Jindrich Makovicka]

Prevents displaying of a doubled mouse pointer when moving the pointer
to the screen edges when fullscreen.
---
 ui/sdl2.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index cdc599be48..6feb637739 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -451,8 +451,9 @@ static void handle_mousemotion(SDL_Event *ev)
 SDL_GetWindowSize(scon->real_window, _w, _h);
 max_x = scr_w - 1;
 max_y = scr_h - 1;
-if (gui_grab && (ev->motion.x == 0 || ev->motion.y == 0 ||
- ev->motion.x == max_x || ev->motion.y == max_y)) {
+if (gui_grab && !gui_fullscreen
+&& (ev->motion.x == 0 || ev->motion.y == 0 ||
+ev->motion.x == max_x || ev->motion.y == max_y)) {
 sdl_grab_end(scon);
 }
 if (!gui_grab &&
-- 
2.15.0

[Qemu-devel] [PATCH 6/8] sdl2: Fix dead keyboard after fullsceen

[Jindrich Makovicka]

---
 ui/sdl2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 51721d764e..cdc599be48 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -418,6 +418,7 @@ static void handle_keyup(SDL_Event *ev)
 sdl2_reset_keys(scon);
 return;
 }
+sdl2_reset_keys(scon);
 gui_keysym = 0;
 }
 if (!gui_keysym) {
-- 
2.15.0

[Qemu-devel] [PATCH 2/8] sdl2: Do not quit the emulator when an auxilliary window is closed

[Jindrich Makovicka]

---
 ui/sdl2.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 774904cbf2..dfbd0de791 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -566,9 +566,14 @@ static void handle_windowevent(SDL_Event *ev)
 update_displaychangelistener(>dcl, 500);
 break;
 case SDL_WINDOWEVENT_CLOSE:
-if (!no_quit) {
-no_shutdown = 0;
-qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
+if (qemu_console_is_graphic(scon->dcl.con)) {
+if (!no_quit) {
+no_shutdown = 0;
+qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
+}
+} else {
+SDL_HideWindow(scon->real_window);
+scon->hidden = true;
 }
 break;
 case SDL_WINDOWEVENT_SHOWN:
-- 
2.15.0

[Qemu-devel] [PATCH 4/8] sdl2: Do not hide the cursor on auxilliary windows

[Jindrich Makovicka]

---
 ui/sdl2.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 9cf4b1772b..677c22282d 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -465,6 +465,10 @@ static void handle_mousemotion(SDL_Event *ev)
 int max_x, max_y;
 struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
 
+if (!qemu_console_is_graphic(scon->dcl.con)) {
+return;
+}
+
 if (qemu_input_is_absolute() || absolute_enabled) {
 int scr_w, scr_h;
 SDL_GetWindowSize(scon->real_window, _w, _h);
@@ -492,6 +496,10 @@ static void handle_mousebutton(SDL_Event *ev)
 SDL_MouseButtonEvent *bev;
 struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
 
+if (!qemu_console_is_graphic(scon->dcl.con)) {
+return;
+}
+
 bev = >button;
 if (!gui_grab && !qemu_input_is_absolute()) {
 if (ev->type == SDL_MOUSEBUTTONUP && bev->button == SDL_BUTTON_LEFT) {
@@ -514,6 +522,10 @@ static void handle_mousewheel(SDL_Event *ev)
 SDL_MouseWheelEvent *wev = >wheel;
 InputButton btn;
 
+if (!qemu_console_is_graphic(scon->dcl.con)) {
+return;
+}
+
 if (wev->y > 0) {
 btn = INPUT_BUTTON_WHEEL_UP;
 } else if (wev->y < 0) {
@@ -655,6 +667,11 @@ static void sdl_mouse_warp(DisplayChangeListener *dcl,
int x, int y, int on)
 {
 struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
+
+if (!qemu_console_is_graphic(scon->dcl.con)) {
+return;
+}
+
 if (on) {
 if (!guest_cursor) {
 sdl_show_cursor();
-- 
2.15.0

[Qemu-devel] [PATCH 1/8] sdl2: Fix broken display updating after the window is hidden

[Jindrich Makovicka]

With SDL 2.0.6, calling SDL_ShowWindow during SDL_WINDOWEVENT_HIDDEN
blocks all subsequent display updates.

Instead of trying to override the change, just update the scon->hidden
flag.
---
 ui/sdl2.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 53dd447fd2..774904cbf2 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -572,14 +572,10 @@ static void handle_windowevent(SDL_Event *ev)
 }
 break;
 case SDL_WINDOWEVENT_SHOWN:
-if (scon->hidden) {
-SDL_HideWindow(scon->real_window);
-}
+scon->hidden = false;
 break;
 case SDL_WINDOWEVENT_HIDDEN:
-if (!scon->hidden) {
-SDL_ShowWindow(scon->real_window);
-}
+scon->hidden = true;
 break;
 }
 }
-- 
2.15.0

[Qemu-devel] [PATCH 5/8] sdl2 uses surface relative coordinates

[Jindrich Makovicka]

This patch fixes mouse positioning with -device usb-tablet and fullscreen
or resized window.
---
 ui/sdl2.c | 28 ++--
 1 file changed, 2 insertions(+), 26 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 677c22282d..51721d764e 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -276,32 +276,8 @@ static void sdl_send_mouse_event(struct sdl2_console 
*scon, int dx, int dy,
 }
 
 if (qemu_input_is_absolute()) {
-int scr_w, scr_h;
-int max_w = 0, max_h = 0;
-int off_x = 0, off_y = 0;
-int cur_off_x = 0, cur_off_y = 0;
-int i;
-
-for (i = 0; i < sdl2_num_outputs; i++) {
-struct sdl2_console *thiscon = _console[i];
-if (thiscon->real_window && thiscon->surface) {
-SDL_GetWindowSize(thiscon->real_window, _w, _h);
-cur_off_x = thiscon->x;
-cur_off_y = thiscon->y;
-if (scr_w + cur_off_x > max_w) {
-max_w = scr_w + cur_off_x;
-}
-if (scr_h + cur_off_y > max_h) {
-max_h = scr_h + cur_off_y;
-}
-if (i == scon->idx) {
-off_x = cur_off_x;
-off_y = cur_off_y;
-}
-}
-}
-qemu_input_queue_abs(scon->dcl.con, INPUT_AXIS_X, off_x + x, 0, max_w);
-qemu_input_queue_abs(scon->dcl.con, INPUT_AXIS_Y, off_y + y, 0, max_h);
+qemu_input_queue_abs(scon->dcl.con, INPUT_AXIS_X, x, 0, 
surface_width(scon->surface));
+qemu_input_queue_abs(scon->dcl.con, INPUT_AXIS_Y, y, 0, 
surface_height(scon->surface));
 } else {
 if (guest_cursor) {
 x -= guest_x;
-- 
2.15.0

[Qemu-devel] [PATCH 3/8] sdl2: Use the same pointer show/hide logic for absolute and relative mode

[Jindrich Makovicka]

Also use a proper enum parameter for SDL_ShowCursor
---
 ui/sdl2.c | 24 +---
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index dfbd0de791..9cf4b1772b 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -169,10 +169,10 @@ static void sdl_hide_cursor(void)
 return;
 }
 
-if (qemu_input_is_absolute()) {
-SDL_ShowCursor(1);
-SDL_SetCursor(sdl_cursor_hidden);
-} else {
+SDL_ShowCursor(SDL_DISABLE);
+SDL_SetCursor(sdl_cursor_hidden);
+
+if (!qemu_input_is_absolute()) {
 SDL_SetRelativeMouseMode(SDL_TRUE);
 }
 }
@@ -185,14 +185,16 @@ static void sdl_show_cursor(void)
 
 if (!qemu_input_is_absolute()) {
 SDL_SetRelativeMouseMode(SDL_FALSE);
-SDL_ShowCursor(1);
-if (guest_cursor &&
-(gui_grab || qemu_input_is_absolute() || absolute_enabled)) {
-SDL_SetCursor(guest_sprite);
-} else {
-SDL_SetCursor(sdl_cursor_normal);
-}
 }
+
+if (guest_cursor &&
+(gui_grab || qemu_input_is_absolute() || absolute_enabled)) {
+SDL_SetCursor(guest_sprite);
+} else {
+SDL_SetCursor(sdl_cursor_normal);
+}
+
+SDL_ShowCursor(SDL_ENABLE);
 }
 
 static void sdl_grab_start(struct sdl2_console *scon)
-- 
2.15.0

[Qemu-devel] [PATCH] SDL2 various fixes

[Jindrich Makovicka]

Hi,

here is a revised patchset:

- sdl2: Do not hide the cursor on auxilliary windows 
split into two

- sdl2: Only accept the hotkeys on the main window 
reworked, also with a more descriptive commit message

- sdl2: Do not quit the emulator when an auxilliary window is closed
added missing scon->hidden update

- sdl2: Fix broken display updating after the window is hidden
added scon->hidden update instead + the SHOWN part changed
the same way

Regards, Jindrich

[Qemu-devel] Abnormal observation during migration: too many "write-not-dirty" pages

[Chunguang Li]

Hi all!

I got a very abnormal observation for the VM migration. I found that many pages 
marked as dirty during migration are "not really dirty", which is, their 
content are the same as the old version.




I did the migration experiment like this:

During the setup phase of migration, first I suspended the VM. Then I copied 
all the pages within the guest physical address space to a memory buffer as 
large as the guest memory size. After that, the dirty tracking began and I 
resumed the VM. Besides, at the end
of each iteration, I also suspended the VM temporarily. During the suspension, 
I compared the content of all the pages marked as dirty in this iteration 
byte-by-byte with their former copies inside the buffer. If the content of one 
page was the same as its former copy, I recorded it as a "write-not-dirty" page 
(the page is written exactly with the same content as the old version). 
Otherwise, I replaced this page in the buffer with the new content, for the 
possible comparison in the future. After the reset of the dirty bitmap, I 
resumed the VM. Thus, I obtain the proportion of the write-not-dirty pages 
within all the pages marked as dirty for each pre-copy iteration.

I repeated this experiment with 15 workloads, which are 11 CPU2006 benchmarks, 
Memcached server, kernel compilation, playing a video, and an idle VM. The 
CPU2006 benchmarks and Memcached are write-intensive workloads. So almost all 
of them did not converge to stop-copy.




Startlingly, the proportions of the write-not-dirty pages are quite high. 
Memcached and three CPU2006 benchmarks(zeusmp, mcf and bzip2) have the most 
high proportions. Their proportions of the write-not-dirty pages within all the 
dirty pages are as high as 45%-80%. The proportions of the other workloads are 
about 5%-20%, which are also abnormal. According to my intuition, the 
proportion of write-not-dirty pages should be far less than these numbers. I 
think it should be quite a particular case that one page is written with 
exactly the same content as the former data.

Besides, the zero pages are not counted for all the results. Because I think 
codes like memset() may write large area of pages to zero pages, which are 
already zero pages before.




I excluded some possible unknown reasons with the machine hardware, because I 
repeated the experiments with two sets of different machines. Then I guessed it 
might be related with the huge page feature. However, the result was the same 
when I turned the huge page feature off in the OS.




Now there are only two possible reasons in my opinion. 

First, there is some bugs in the KVM kernel dirty tracking mechanism. It may 
mark some pages that do not receive write request as dirty.

Second, there is some bugs in the OS running inside the VM. It may issue some 
unnecessary write requests.




What do you think about this abnormal phenomenon? Any advice or possible 
reasons or even guesses? I appreciate any responses, because it has confused me 
for a long time. Thank you.


--
Chunguang Li, Ph.D. Candidate
Wuhan National Laboratory for Optoelectronics (WNLO)
Huazhong University of Science & Technology (HUST)
Wuhan, Hubei Prov., China

Re: [Qemu-devel] [PATCH] linux-user, s390x: ignore OS ABI value in ELF header

[Laurent Vivier]

Le 12/11/2017 à 07:56, Thomas Huth a écrit :
> Am Fri, 10 Nov 2017 20:49:35 +0100
> schrieb Laurent Vivier :
> 
>> I have this error:
>> bash: /sbin/ldconfig: cannot execute binary file: Exec format error
>>
>> because /sbin/ldconfig is:
>> ELF 64-bit MSB executable, IBM S/390, version 1 (GNU/Linux),
>> statically linked, for GNU/Linux 3.2.0,
>> BuildID[sha1]=90b64604014aafac9c1a0623b1cf447281d1a382, stripped
>>
>> OS ABI is GNU/linux
>>
>> "/bin/ls" works well:
>>
>> ELF 64-bit MSB shared object, IBM S/390, version 1 (SYSV),
>> dynamically linked, interpreter /lib/ld64.so.1, for GNU/Linux 3.2.0,
>> BuildID[sha1]=be9b19143d4657678846f6e5277383071fc1059a, stripped
>>
>> OS ABI is SYSV
>>
>> To be able to execute ldconfig, this patch modifies s390x binfmt mask
>> to ignore the OS ABI value (EI_OSABI, byte 7).
>>
>> Signed-off-by: Laurent Vivier 
>> ---
>>  scripts/qemu-binfmt-conf.sh | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
>> index 8afc3eb5bb..e2e1b7544d 100755
>> --- a/scripts/qemu-binfmt-conf.sh
>> +++ b/scripts/qemu-binfmt-conf.sh
>> @@ -85,7 +85,7 @@
>> sh4eb_mask='\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff
>> sh4eb_family=sh4 
>>  
>> s390x_magic='\x7fELF\x02\x02\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x16'
>> -s390x_mask='\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff'
>> +s390x_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff'
> 
> If I've got that right, the OSABI field should either be 0 for
> "No extensions or unspecified" (which is then printed  by "file"
> as SYSV) or 3 for "GNU/Linux".
> Thus wouldn't it be better to use a mask of 0xfc here instead, so
> that we refuse at least everything with a value > 3 here?
No, because with this mask you allow HPUX and NetBSD (See
/usr/include/elf.h), I don't think it's really better. The simplest way
to manage this is to ignore the field. A better way should be to have
two binfmt entries, one for SYSV and one for GNU/Linux, but as our goal
is to run linux binaries on linux system, is it worth it?

Most of the other archs use already 0.

> 
> Also I wonder whether i386 has the same problem, too?
> 

Yes, I think all targets can have the same problem (I have with m68k,
but with the gcc-7/cc1 binary).

Thanks,
Laurent

[Qemu-devel] [Bug 1086782] Re: HPET time drift windows 7 64bits guest

[Carlos-velasco]

Running from 6 days, here are my results.
There is still some time drift but not critical.
Windows 7 guest is behind around 30 seconds in 6 days. This can be corrected by 
using NTP.

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

Title:
  HPET time drift windows 7 64bits guest

Status in QEMU:
  Incomplete

Bug description:
  Using latest qemu-kvm (1.2.0), time drift (clock slow in guest) in Windows 7 
64 bits guest when HPET is enabled (default).
  Disabling HPET (-no-hpet) solves the time drift.

  UsePlatformClock enable/disable doesn't make a difference in the guest.
  bcdedit /set useplatformclock true

  Using driftfix slew doesn't make a difference too.

  
  # qemu-system-x86_64 --version
  QEMU emulator version 1.2.0 (qemu-kvm-1.2.0), Copyright (c) 2003-2008 Fabrice 
Bellard

  Kernel is 3.6.8:
  # uname -a
  Linux pulsar 3.6.8 #1 SMP Sat Dec 1 16:26:10 CET 2012 x86_64 x86_64 x86_64 
GNU/Linux

  TSC is stable in the host:
  ===
  # cat /sys/devices/system/clocksource/clocksource0/current_clocksource
  tsc

  Dmesg:
  [0.00] hpet clockevent registered
  [0.00] tsc: Fast TSC calibration using PIT
  [0.00] tsc: Detected 2660.096 MHz processor
  [0.001002] Calibrating delay loop (skipped), value calculated using timer 
frequency.. 5320.19 BogoMIPS (lpj=2660096)
  [0.001138] pid_max: default: 32768 minimum: 301
  ...
  [1.492019] tsc: Refined TSC clocksource calibration: 2659.973 MHz
  [1.492093] Switching to clocksource tsc

  
  CPUinfo, constant_tsc:
  vendor_id   : GenuineIntel
  cpu family  : 6
  model   : 23
  model name  : Intel(R) Core(TM)2 Quad CPUQ8400  @ 2.66GHz
  stepping: 10
  microcode   : 0xa0b
  cpu MHz : 2667.000
  cache size  : 2048 KB
  flags   : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca 
cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx lm 
constant_tsc arch_perfmon pebs bts rep_good nopl aperfmperf pni dtes64 monitor 
ds_cpl vmx est tm2 ssse3 cx16 xtpr pdcm sse4_1 xsave lahf_lm dtherm tpr_shadow 
vnmi flexpriority
  bogomips: 5320.19

  # grep -i hpet .config
  CONFIG_HPET_TIMER=y
  CONFIG_HPET_EMULATE_RTC=y
  CONFIG_HPET=y
  # CONFIG_HPET_MMAP is not set
  ===

  Qemu command line:
  /usr/bin/qemu-system-x86_64 -drive 
file=/dev/vol0/KVMORION01,cache=none,aio=native,if=virtio \
-drive file=/dev/vol0/KVMORION02,cache=none,aio=native,if=virtio \
-cpu host \
-m 2048 \
-smp 4,maxcpus=4,cores=4,threads=1,sockets=1 \
-rtc base=localtime,driftfix=slew \
-vnc 10.124.241.211:0,password -k es \
-monitor telnet:localhost:37200,server,nowait \
-netdev 
tap,id=kvmorion,ifname=kvmorion,script=/etc/qemu-ifup-br0,downscript=/etc/qemu-ifdown-br0
 \
-device virtio-net-pci,netdev=kvmorion,id=virtio-nic0,mac=02:85:64:02:c2:aa 
\
-device virtio-balloon-pci,id=balloon0 \
-boot menu=on \
-pidfile /var/run/kvmorion.pid \
-daemonize

  Using 1 CPU doesn't make a difference.
  Only workaround is disabling hpet (-no-hpet)

  Sample time drift in guest:
  >ntpdate -q 10.124.241.211
   5 Dec 13:36:06 ntpdate[3464]: Raised to realtime priority class
  server 10.124.241.211, stratum 2, offset 3.694184, delay 0.02551
   5 Dec 13:36:12 ntpdate[3464]: step time server 10.124.241.211 offset 
3.694184 s
  ec

  >ntpdate -q 10.124.241.211
   5 Dec 13:52:02 ntpdate[1964]: Raised to realtime priority class
  server 10.124.241.211, stratum 2, offset 4.719968, delay 0.02554
   5 Dec 13:52:08 ntpdate[1964]: step time server 10.124.241.211 offset 
4.719968 s
  ec

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