Re: [Qemu-devel] [PATCH v2 04/13] spapr/xive: add state synchronization with KVM

[Cédric Le Goater]

On 3/14/19 3:10 AM, David Gibson wrote:
> On Mon, Mar 11, 2019 at 09:41:12PM +0100, Cédric Le Goater wrote:
>> On 2/26/19 1:01 AM, David Gibson wrote:
>>> On Fri, Feb 22, 2019 at 02:13:13PM +0100, Cédric Le Goater wrote:
 This extends the KVM XIVE device backend with 'synchronize_state'
 methods used to retrieve the state from KVM. The HW state of the
 sources, the KVM device and the thread interrupt contexts are
 collected for the monitor usage and also migration.

 These get operations rely on their KVM counterpart in the host kernel
 which acts as a proxy for OPAL, the host firmware. The set operations
 will be added for migration support later.

 Signed-off-by: Cédric Le Goater 
 ---
  include/hw/ppc/spapr_xive.h |  8 
  include/hw/ppc/xive.h   |  1 +
  hw/intc/spapr_xive.c| 17 ---
  hw/intc/spapr_xive_kvm.c| 89 +
  hw/intc/xive.c  | 10 +
  5 files changed, 118 insertions(+), 7 deletions(-)

 diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
 index 749c6cbc2c56..ebd65e7fe36b 100644
 --- a/include/hw/ppc/spapr_xive.h
 +++ b/include/hw/ppc/spapr_xive.h
 @@ -44,6 +44,13 @@ typedef struct sPAPRXive {
  void  *tm_mmap;
  } sPAPRXive;
  
 +/*
 + * The sPAPR machine has a unique XIVE IC device. Assign a fixed value
 + * to the controller block id value. It can nevertheless be changed
 + * for testing purpose.
 + */
 +#define SPAPR_XIVE_BLOCK_ID 0x0
 +
  bool spapr_xive_irq_claim(sPAPRXive *xive, uint32_t lisn, bool lsi);
  bool spapr_xive_irq_free(sPAPRXive *xive, uint32_t lisn);
  void spapr_xive_pic_print_info(sPAPRXive *xive, Monitor *mon);
 @@ -74,5 +81,6 @@ void kvmppc_xive_set_queue_config(sPAPRXive *xive, 
 uint8_t end_blk,
  void kvmppc_xive_get_queue_config(sPAPRXive *xive, uint8_t end_blk,
   uint32_t end_idx, XiveEND *end,
   Error **errp);
 +void kvmppc_xive_synchronize_state(sPAPRXive *xive, Error **errp);
  
  #endif /* PPC_SPAPR_XIVE_H */
 diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
 index 061d43fea24d..f3766fd881a2 100644
 --- a/include/hw/ppc/xive.h
 +++ b/include/hw/ppc/xive.h
 @@ -431,5 +431,6 @@ void kvmppc_xive_source_reset_one(XiveSource *xsrc, 
 int srcno, Error **errp);
  void kvmppc_xive_source_reset(XiveSource *xsrc, Error **errp);
  void kvmppc_xive_source_set_irq(void *opaque, int srcno, int val);
  void kvmppc_xive_cpu_connect(XiveTCTX *tctx, Error **errp);
 +void kvmppc_xive_cpu_synchronize_state(XiveTCTX *tctx, Error **errp);
  
  #endif /* PPC_XIVE_H */
 diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
 index 3db24391e31c..9f07567f4d78 100644
 --- a/hw/intc/spapr_xive.c
 +++ b/hw/intc/spapr_xive.c
 @@ -40,13 +40,6 @@
  
  #define SPAPR_XIVE_NVT_BASE 0x400
  
 -/*
 - * The sPAPR machine has a unique XIVE IC device. Assign a fixed value
 - * to the controller block id value. It can nevertheless be changed
 - * for testing purpose.
 - */
 -#define SPAPR_XIVE_BLOCK_ID 0x0
 -
  /*
   * sPAPR NVT and END indexing helpers
   */
 @@ -153,6 +146,16 @@ void spapr_xive_pic_print_info(sPAPRXive *xive, 
 Monitor *mon)
  XiveSource *xsrc = &xive->source;
  int i;
  
 +if (kvm_irqchip_in_kernel()) {
 +Error *local_err = NULL;
 +
 +kvmppc_xive_synchronize_state(xive, &local_err);
 +if (local_err) {
 +error_report_err(local_err);
 +return;
 +}
 +}
 +
  monitor_printf(mon, "  LSIN PQEISN CPU/PRIO EQ\n");
  
  for (i = 0; i < xive->nr_irqs; i++) {
 diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
 index 6b50451b4f85..4b1ffb9835f9 100644
 --- a/hw/intc/spapr_xive_kvm.c
 +++ b/hw/intc/spapr_xive_kvm.c
 @@ -60,6 +60,57 @@ static void kvm_cpu_enable(CPUState *cs)
  /*
   * XIVE Thread Interrupt Management context (KVM)
   */
 +static void kvmppc_xive_cpu_get_state(XiveTCTX *tctx, Error **errp)
 +{
 +uint64_t state[4] = { 0 };
 +int ret;
 +
 +ret = kvm_get_one_reg(tctx->cs, KVM_REG_PPC_NVT_STATE, state);
 +if (ret != 0) {
 +error_setg_errno(errp, errno,
 + "XIVE: could not capture KVM state of CPU %ld",
 + kvm_arch_vcpu_id(tctx->cs));
 +return;
 +}
 +
 +/* word0 and word1 of the OS ring. */
 +*((uint64_t *) &tctx->regs[TM_QW1_OS]) = state[0];
 +
 +/*
 + * KVM also returns word2 containing the OS CAM line which is
 + * interesting 

Re: [Qemu-devel] [PATCH] ppc/xics/spapr: Fix H_IPOLL implementation

[Greg Kurz]

On Thu, 14 Mar 2019 07:38:55 +0100
Cédric Le Goater  wrote:

> From: Benjamin Herrenschmidt 
> 
> H_IPOLL takes the CPU# of the processor to poll as an argument,
> it doesn't operate on self.
> 

True.

> Signed-off-by: Benjamin Herrenschmidt 
> Signed-off-by: Cédric Le Goater 
> ---

Reviewed-by: Greg Kurz 

>  hw/intc/xics_spapr.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> index 607e1c167ba2..9d2b8adef7c5 100644
> --- a/hw/intc/xics_spapr.c
> +++ b/hw/intc/xics_spapr.c
> @@ -95,8 +95,15 @@ static target_ulong h_eoi(PowerPCCPU *cpu, 
> SpaprMachineState *spapr,
>  static target_ulong h_ipoll(PowerPCCPU *cpu, SpaprMachineState *spapr,
>  target_ulong opcode, target_ulong *args)
>  {
> +ICPState *icp = xics_icp_get(XICS_FABRIC(spapr), args[0]);
>  uint32_t mfrr;
> -uint32_t xirr = icp_ipoll(spapr_cpu_state(cpu)->icp, &mfrr);
> +uint32_t xirr;
> +
> +if (!icp) {
> +return H_PARAMETER;
> +}
> +
> +xirr = icp_ipoll(icp, &mfrr);
>  
>  args[0] = xirr;
>  args[1] = mfrr;

Re: [Qemu-devel] [PATCH RESEND v4] drive-mirror: add incremental mode

[Vladimir Sementsov-Ogievskiy]

On 13.03.2019 19:52, John Snow wrote:
> 
> 
> On 3/12/19 12:32 PM, Vladimir Sementsov-Ogievskiy wrote:
>> 12.03.2019 18:58, John Snow wrote:
>>>
>>>
>>> On 3/12/19 9:43 AM, Eric Blake wrote:
 On 3/12/19 3:19 AM, Vladimir Sementsov-Ogievskiy wrote:

>> So one important point about incremental backups is that you can
>> actually do them incrementally: The bitmap is effectively cleared at the
>> beginning of the backup process (a successor bitmap is installed that is
>> cleared and receives all changes; at the end of the backup, it either
>> replaces the old bitmap (on success) or is merged into it (on failure)).
>> Therefore, you can do the next incremental backup by using the same 
>> bitmap.
>
> Hmm, I heard in some other thread that Eric finally decided to always 
> start
> backup on copied bitmap in libvirt, so this logic is skipped. Do we need 
> it
> for mirror? Sorry if I'm wrong, Eric, am I?

 You are correct that my current libvirt patches do incremental backup
 mode with a temporary bitmap (so regardless of whether the temporary
 bitmap is cleared on success or left alone is irrelevant, the temporary
 bitmap goes away afterwards). But just because libvirt isn't using that
 particular feature of qemu's incremental backups does not excuse qemu
 from not thinking about other clients that might be impacted by doing
 incremental backup with a live bitmap, where qemu management of the
 bitmap matters.

>>>
>>> I'd prefer adding SYNC=DIFFERENTIAL to intuit that the bitmap isn't
>>> modified after use, if there's no reason to add an actual "incremental"
>>> mode to mirror.
>>>
>>> Then it would be fine for mirror to implement differential and not
>>> incremental, and still use the bitmap.
>>>
>>
>> Agree, this is better.
>>
>> But I have an idea: could we just add parameter @x-bitmap, independent of 
>> @sync ?
>>
> 
> It's a thought. Fam's original sketch for bitmaps included a separate
> parameter for how to clean up the bitmap at the end, but it was removed
> because we didn't have use cases for it at the time.
> 
>> In this case, we can get NONE mode reduced by bitmap, which may be usable 
>> too.
> 
> What do you have in mind for the use case?

I think about backup(sync=none) which lacks bitmap support for the idea 
described below. For mirror - I don't know) To imagine this, I need to 
know original reason for incremental-mirror and original reason for
none-mode-mirror :)

> 
>> And FULL mode with bitmap would be what you called DIFFERENTIAL. Not sure 
>> about
>> could TOP mode with bitmap be meaningful.
>>
>>
>> Also it may be useful at some point to share bitmap between jobs, so we could
>>
>> start backup(sync=none) from active disk to local temp node, and then mirror 
>> from temp
>> to target. Which gives backup which (1) don't slow down guest writes if 
>> target is far and slow
>> and (2) fast as it is mostly mirror, keeping in mind that mirror is faster 
>> than backup as it uses
>> async pattern and block_status.
>>
> 
> In effect, a disk-backed buffer so we don't lose performance on slow
> network links, right?

Yes

> 
>> And to make such a backup incremental we need to share dirty bitmap between 
>> backup and mirror,
>> and this bitmap should be cleared when something is copied out from source 
>> (it may be cleared
>> both by backup(sync=none) and mirror)... But I'm not sure that shared bitmap 
>> should be the same
>> as bitmap which initializes differential/incremental mode. And this is why I 
>> propose new parameter
>> to be experimental.
>>
> 
> This could be very cool; how does this vision fit in with the vision for
> a unified block-copy job? (i.e. unifying mirror, stream, backup and commit.)

My current plan is to finish backup top filter first. Then may be 
implement disk-backed buffer inside backup, to don't start two jobs. And 
then make backup as fast as mirror. Or, may be it would be backup-top 
filter, who will do most of work, so it should be fast too..

I often think about unifying jobs code, but when I try to dig in, it 
becomes too difficult. I think, we should start from separating common 
code to some kind of library, separate file. And first thing to separate
seems to be an async copying loop backed by coroutines and 
block_status.. To share it between all jobs and qemu-img convert. But 
even here, there are differences, for example, mirror is the only job
which needs several iterations of the loop. Backup needs to share
block_status results with backup_top filter, to make it useful for
copy-before-write operations.. Mirror needs block_status of active disk
and backup actually need block_status of point-in-time..

And mirror code is tooo difficult, it's a problem too;)

But there must be some generic part which we may separate to start from.
And the idea is to start from unifying code-path, not interface.

> 
> Worth thinking about, because we also want the abi

[Qemu-devel] [PATCH] ppc/xics/spapr: Fix H_IPOLL implementation

[Cédric Le Goater]

From: Benjamin Herrenschmidt 

H_IPOLL takes the CPU# of the processor to poll as an argument,
it doesn't operate on self.

Signed-off-by: Benjamin Herrenschmidt 
Signed-off-by: Cédric Le Goater 
---
 hw/intc/xics_spapr.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
index 607e1c167ba2..9d2b8adef7c5 100644
--- a/hw/intc/xics_spapr.c
+++ b/hw/intc/xics_spapr.c
@@ -95,8 +95,15 @@ static target_ulong h_eoi(PowerPCCPU *cpu, SpaprMachineState 
*spapr,
 static target_ulong h_ipoll(PowerPCCPU *cpu, SpaprMachineState *spapr,
 target_ulong opcode, target_ulong *args)
 {
+ICPState *icp = xics_icp_get(XICS_FABRIC(spapr), args[0]);
 uint32_t mfrr;
-uint32_t xirr = icp_ipoll(spapr_cpu_state(cpu)->icp, &mfrr);
+uint32_t xirr;
+
+if (!icp) {
+return H_PARAMETER;
+}
+
+xirr = icp_ipoll(icp, &mfrr);
 
 args[0] = xirr;
 args[1] = mfrr;
-- 
2.20.1

Re: [Qemu-devel] Maintainers, please tell us how to boot your machines!

[Mark Cave-Ayland]

On 12/03/2019 17:36, Markus Armbruster wrote:

> = hw/ppc/prep.c =
> "Hervé Poussineau"  (maintainer:PReP)
> David Gibson  (maintainer:PowerPC)
> qemu-...@nongnu.org (open list:PReP)

Note that -M prep has been formally deprecated and so we're really only 
interested in
-M 40p going forward. The test for this is to boot the sandalfoot zImage at
http://www.juneau-lug.org/zImage.initrd.sandalfoot booted as below:

./qemu-system-ppc -M 40p -cdrom zImage.initrd.sandalfoot -boot d

> = hw/sparc/leon3.c =
> Fabien Chouteau  (maintainer:Leon3)
> Mark Cave-Ayland  (maintainer:SPARC)
> Artyom Tarasenko  (maintainer:SPARC)

I don't really do anything with the leon3 so I'll leave this to Fabien :)

> = hw/sparc/sun4m.c =
> Mark Cave-Ayland  (maintainer:Sun4m)
> Artyom Tarasenko  (maintainer:SPARC)

The easiest test here is using Aurelien's image from
https://people.debian.org/~aurel32/qemu/sparc/ which is really simple:

./qemu-system-sparc -hda debian_etch_sparc_small.qcow2 -snapshot

> = hw/sparc64/niagara.c =
> Artyom Tarasenko  (maintainer:Sun4v)
> Mark Cave-Ayland  (maintainer:SPARC)

I don't really touch the niagara parts at the moment, however you can find a 
couple
of examples in both the QEMU documentation at
https://qemu.weilnetz.de/doc/qemu-doc.html#Sparc64-System-emulator and also the 
wiki
homepage at https://wiki.qemu.org/Documentation/Platforms/SPARC.

> = hw/sparc64/sun4u.c =
> Mark Cave-Ayland  (maintainer:Sun4u)
> Artyom Tarasenko  (maintainer:SPARC)

For this I tend to use Debian ports CD at
https://cdimage.debian.org/cdimage/ports/10.0/sparc64/iso-cd/ with a simple boot
command line:

./qemu-system-sparc64 -cdrom debian-10.0-sparc64-NETINST-1.iso -nographic -m 
256 -boot d

> = hw/ppc/mac_newworld.c =
> Mark Cave-Ayland  (odd fixer:New World 
> (mac99))
> David Gibson  (reviewer:New World (mac99))
> qemu-...@nongnu.org (open list:New World (mac99))
> 
> = hw/ppc/mac_oldworld.c =
> Mark Cave-Ayland  (odd fixer:Old World 
> (g3beige))
> David Gibson  (reviewer:Old World (g3beige))
> qemu-...@nongnu.org (open list:Old World (g3beige))

Again for both of these a Debian ports CD from
https://cdimage.debian.org/cdimage/ports/10.0/powerpc/iso-cd/ is enough for a 
simple
boot test here:

./qemu-system-ppc -cdrom debian-10.0-powerpc-NETINST-1.iso -boot d


ATB,

Mark.

Re: [Qemu-devel] [PATCH] Clean up includes

[Markus Armbruster]

Eric Blake  writes:

> On 3/13/19 11:28 AM, Markus Armbruster wrote:
>> Clean up includes so that osdep.h is included first and headers
>> which it implies are not included manually.
>> 
>> This commit was created with scripts/clean-includes, with the changes
>> to the following files manually reverted:
>
> Is it worth tweaking scripts/clean-includes as a pre-req patch so that
> some of these files become listed exceptions and don't need manual
> reversion here?

If we ran it more often than we do, probably yes.

>> contrib/libvhost-user/libvhost-user-glib.h
>> contrib/libvhost-user/libvhost-user.c
>> contrib/libvhost-user/libvhost-user.h
>> linux-user/mips64/cpu_loop.c
>> linux-user/mips64/signal.c
>> linux-user/sparc64/cpu_loop.c
>> linux-user/sparc64/signal.c
>> linux-user/x86_64/cpu_loop.c
>> linux-user/x86_64/signal.c
>> slirp/src/*
>> target/s390x/gen-features.c
>> tests/migration/s390x/a-b-bios.c
>> tests/test-rcu-simpleq.c
>> tests/test-rcu-tailq.c
>> tests/uefi-test-tools/UefiTestToolsPkg/BiosTablesTest/BiosTablesTest.c
>> 
>> We're in the process of spinning out slirp/.  tests/uefi-test-tools/
>> is guest software.  The remaining reverts are the same as in commit
>> b7d89466dde.
>> 
>> Signed-off-by: Markus Armbruster 
>> ---
>
>> +++ b/contrib/elf2dmp/pdb.c
>> @@ -18,9 +18,8 @@
>>   * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
>
> Unrelated, but if someone is looking for a project, cleaning up stale
> address references in older GPL copy-and-paste is a possibility.

Would be nice.

> I think these changes are okay for 4.0 soft freeze; but it also doesn't
> hurt if this delays into 4.1.
>
> Reviewed-by: Eric Blake 

Thanks!

Re: [Qemu-devel] Maintainers, please tell us how to boot your machines!

[Gerd Hoffmann]

> I could not yet boot Linux on it but I'm interested in this machine because
> I want to use it to cross-check my patches so don't drop it just yet.
> Currently it seems to panic because of my newly added radeon emulation that
> I need to fix:

Doesn't look like an issue in ati-vga.  It doesn't come that far that
the ati emulation code actually runs, it seems to struggle at pci
initialization.

> Data bus error, epc == 80418f88, ra == 80418f68

Where does that come from?  Seems to be the root cause.

HTH,
  Gerd

Re: [Qemu-devel] [PATCH] Clean up includes

[Markus Armbruster]

BALATON Zoltan  writes:

> On Wed, 13 Mar 2019, Markus Armbruster wrote:
>> BALATON Zoltan  writes:
>>> On Wed, 13 Mar 2019, Markus Armbruster wrote:
 Clean up includes so that osdep.h is included first and headers
 which it implies are not included manually.

 This commit was created with scripts/clean-includes, with the changes
 to the following files manually reverted:

contrib/libvhost-user/libvhost-user-glib.h
contrib/libvhost-user/libvhost-user.c
contrib/libvhost-user/libvhost-user.h
linux-user/mips64/cpu_loop.c
linux-user/mips64/signal.c
linux-user/sparc64/cpu_loop.c
linux-user/sparc64/signal.c
linux-user/x86_64/cpu_loop.c
linux-user/x86_64/signal.c
slirp/src/*
target/s390x/gen-features.c
tests/migration/s390x/a-b-bios.c
tests/test-rcu-simpleq.c
tests/test-rcu-tailq.c
tests/uefi-test-tools/UefiTestToolsPkg/BiosTablesTest/BiosTablesTest.c

 We're in the process of spinning out slirp/.  tests/uefi-test-tools/
 is guest software.  The remaining reverts are the same as in commit
 b7d89466dde.

 Signed-off-by: Markus Armbruster 
>> [...]
 diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
 index bc98ba6eeb..f31b3c27c7 100644
 --- a/hw/display/ati_2d.c
 +++ b/hw/display/ati_2d.c
 @@ -7,6 +7,7 @@
  * This work is licensed under the GNU GPL license version 2 or later.
  */

 +#include "qemu/osdep.h"
 #include "ati_int.h"
 #include "ati_regs.h"
 #include "qemu/log.h"
 diff --git a/hw/display/ati_dbg.c b/hw/display/ati_dbg.c
 index 1e6c32624e..b045f81d06 100644
 --- a/hw/display/ati_dbg.c
 +++ b/hw/display/ati_dbg.c
 @@ -1,3 +1,4 @@
 +#include "qemu/osdep.h"
 #include "ati_int.h"

 #ifdef DEBUG_ATI
 diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h
 index a6f3e20e63..2f426064cf 100644
 --- a/hw/display/ati_int.h
 +++ b/hw/display/ati_int.h
 @@ -9,7 +9,6 @@
 #ifndef ATI_INT_H
 #define ATI_INT_H

 -#include "qemu/osdep.h"
>>>
>>> What's wrong with ati_int.h including osdep.h first and everything
>>> else including ati_int.h first? I think it was OK that way so unless
>>> there's a good reason to explicitely include osdep in all files that
>>> also include ati_int.h I think these should not be changed. For the
>>> ati model we need ati_int.h included first so it's OK to include
>>> osdep.h from there.
>>
>> ./HACKING explains:
>>
>>1.2. Include directives
>>
>>Order include directives as follows:
>>
>>#include "qemu/osdep.h"  /* Always first... */
>>#include <...>   /* then system headers... */
>>#include "..."   /* and finally QEMU headers. */
>>
>>The "qemu/osdep.h" header contains preprocessor macros that affect the 
>> behavior
>>of core system headers like .  It must be the first include so 
>> that
>>core system headers included by external libraries get the preprocessor 
>> macros
>>that QEMU depends on.
>>
>>Do not include "qemu/osdep.h" from header files since the .c file will 
>> have
>>already included it.
>
> I'm not convinced. The rule is to include osdep.h first and it's
> currently satisified without this change as well but if it makes
> clean-includes script happy then I don't mind.

The rule is actually to include osdep.h first in .c, and never in .h.

We chose this rule because it makes conformance locally obvious.  You
don't have to chase down a chain of first includes to verify it ends in
"osdep.h" and none of the intermediate headers have anything before
their first #include.

The obviousness translates into robustness here.  If you hide #include
"osdep.h" in a header, you can easily lose "firstness" in a
harmless-looking change to any of the sources involved.  Pray this
breaks the build, because if it breaks only at run-time, it'll be nasty
to debug.

[Qemu-devel] [PATCH 2/2] virtio-gpu: clear command queue on reset

[Gerd Hoffmann]

It was never correct to not clear them.  Due to commit "3912e66a3feb
virtio-vga: fix reset." this became more obvious though.  The virtio
rings get properly reset now, and trying to process the stale commands
will trigger an assert in the virtio core.

Signed-off-by: Gerd Hoffmann 
---
 hw/display/virtio-gpu.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index fbd8d908ad32..cdab327658d2 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1356,6 +1356,7 @@ static void virtio_gpu_reset(VirtIODevice *vdev)
 {
 VirtIOGPU *g = VIRTIO_GPU(vdev);
 struct virtio_gpu_simple_resource *res, *tmp;
+struct virtio_gpu_ctrl_command *cmd;
 int i;
 
 g->enable = 0;
@@ -1372,6 +1373,12 @@ static void virtio_gpu_reset(VirtIODevice *vdev)
 g->scanout[i].ds = NULL;
 }
 
+while (!QTAILQ_EMPTY(&g->cmdq)) {
+cmd = QTAILQ_FIRST(&g->cmdq);
+QTAILQ_REMOVE(&g->cmdq, cmd, next);
+g_free(cmd);
+}
+
 #ifdef CONFIG_VIRGL
 if (g->use_virgl_renderer) {
 if (g->renderer_blocked) {
-- 
2.18.1

[Qemu-devel] [PATCH 1/2] virtio-gpu: delay virglrenderer reset when blocked.

[Gerd Hoffmann]

If renderer_blocked is set do not call virtio_gpu_virgl_reset().
Instead set a flag indicating that virglrenderer needs a reset.
When renderer_blocked gets cleared do the actual reset call.

Without this we can trigger an assert in spice due to calling
spice_qxl_gl_scanout() while another operation is still running:

spice_qxl_gl_scanout: condition `qxl_state->gl_draw_cookie == 
GL_DRAW_COOKIE_INVALID' failed

Signed-off-by: Gerd Hoffmann 
---
 include/hw/virtio/virtio-gpu.h |  1 +
 hw/display/virtio-gpu.c| 12 +++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index ce0ca7217175..60425c5d58dc 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -113,6 +113,7 @@ typedef struct VirtIOGPU {
 bool use_virgl_renderer;
 bool renderer_inited;
 int renderer_blocked;
+bool renderer_reset;
 QEMUTimer *fence_poll;
 QEMUTimer *print_stats;
 
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 4dbf48e42482..fbd8d908ad32 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1084,6 +1084,12 @@ static void virtio_gpu_gl_block(void *opaque, bool block)
 assert(g->renderer_blocked >= 0);
 
 if (g->renderer_blocked == 0) {
+#ifdef CONFIG_VIRGL
+if (g->renderer_reset) {
+g->renderer_reset = false;
+virtio_gpu_virgl_reset(g);
+}
+#endif
 virtio_gpu_process_cmdq(g);
 }
 }
@@ -1368,7 +1374,11 @@ static void virtio_gpu_reset(VirtIODevice *vdev)
 
 #ifdef CONFIG_VIRGL
 if (g->use_virgl_renderer) {
-virtio_gpu_virgl_reset(g);
+if (g->renderer_blocked) {
+g->renderer_reset = true;
+} else {
+virtio_gpu_virgl_reset(g);
+}
 g->use_virgl_renderer = 0;
 }
 #endif
-- 
2.18.1

[Qemu-devel] [PATCH 0/2] virtio-gpu: more reset fixes.

[Gerd Hoffmann]

Gerd Hoffmann (2):
  virtio-gpu: delay virglrenderer reset when blocked.
  virtio-gpu: clear command queue on reset

 include/hw/virtio/virtio-gpu.h |  1 +
 hw/display/virtio-gpu.c| 19 ++-
 2 files changed, 19 insertions(+), 1 deletion(-)

-- 
2.18.1

[Qemu-devel] [PATCH v2 14/14] target/arm: Implement ARMv8.5-RNG

[Richard Henderson]

Cc: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 target/arm/cpu.h|  5 +
 target/arm/cpu64.c  |  1 +
 target/arm/helper.c | 32 
 3 files changed, 38 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 5f23c62132..aaa9e02e78 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3441,6 +3441,11 @@ static inline bool isar_feature_aa64_condm_5(const 
ARMISARegisters *id)
 return FIELD_EX64(id->id_aa64isar0, ID_AA64ISAR0, TS) >= 2;
 }
 
+static inline bool isar_feature_aa64_rndr(const ARMISARegisters *id)
+{
+return FIELD_EX64(id->id_aa64isar0, ID_AA64ISAR0, RNDR) != 0;
+}
+
 static inline bool isar_feature_aa64_jscvt(const ARMISARegisters *id)
 {
 return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, JSCVT) != 0;
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 228906f267..835f73cceb 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -310,6 +310,7 @@ static void aarch64_max_initfn(Object *obj)
 t = FIELD_DP64(t, ID_AA64ISAR0, DP, 1);
 t = FIELD_DP64(t, ID_AA64ISAR0, FHM, 1);
 t = FIELD_DP64(t, ID_AA64ISAR0, TS, 2); /* v8.5-CondM */
+t = FIELD_DP64(t, ID_AA64ISAR0, RNDR, 1);
 cpu->isar.id_aa64isar0 = t;
 
 t = cpu->isar.id_aa64isar1;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 2607d39ad1..3fe7dc8719 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -20,6 +20,7 @@
 #include "fpu/softfloat.h"
 #include "qemu/range.h"
 #include "qapi/qapi-commands-target.h"
+#include "qemu/random.h"
 
 #define ARM_CPU_FREQ 10 /* FIXME: 1 GHz, should be configurable */
 
@@ -5717,6 +5718,34 @@ static const ARMCPRegInfo pauth_reginfo[] = {
   .fieldoffset = offsetof(CPUARMState, apib_key.hi) },
 REGINFO_SENTINEL
 };
+
+static uint64_t rndr_readfn(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+uint64_t ret;
+
+/* Success sets NZCV = .  */
+env->NF = env->CF = env->VF = 0, env->ZF = 1;
+if (likely(qemu_getrandom(&ret, sizeof(ret), true))) {
+return ret;
+}
+
+/* Failure sets Z = 1 and returns 0.  */
+env->ZF = 0;
+return 0;
+}
+
+/* We do not support re-seeding, so the two registers operate the same.  */
+static const ARMCPRegInfo rndr_reginfo[] = {
+{ .name = "RNDR", .state = ARM_CP_STATE_AA64,
+  .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END,
+  .opc0 = 3, .opc1 = 3, .crn = 2, .crm = 4, .opc2 = 0,
+  .access = PL0_R, .readfn = rndr_readfn },
+{ .name = "RNDRRS", .state = ARM_CP_STATE_AA64,
+  .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END,
+  .opc0 = 3, .opc1 = 3, .crn = 2, .crm = 4, .opc2 = 1,
+  .access = PL0_R, .readfn = rndr_readfn },
+REGINFO_SENTINEL
+};
 #endif
 
 static CPAccessResult access_predinv(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -6661,6 +6690,9 @@ void register_cp_regs_for_features(ARMCPU *cpu)
 if (cpu_isar_feature(aa64_pauth, cpu)) {
 define_arm_cp_regs(cpu, pauth_reginfo);
 }
+if (cpu_isar_feature(aa64_rndr, cpu)) {
+define_arm_cp_regs(cpu, rndr_reginfo);
+}
 #endif
 
 /*
-- 
2.17.1

[Qemu-devel] [PATCH v2 09/14] util: Use qcrypto_random_bytes for qemu_getrandom

[Richard Henderson]

When not requesting deterministic bytes, use our existing crypto.

Signed-off-by: Richard Henderson 
---
 util/random.c | 61 ---
 1 file changed, 48 insertions(+), 13 deletions(-)

diff --git a/util/random.c b/util/random.c
index 467c987a66..c2628ace80 100644
--- a/util/random.c
+++ b/util/random.c
@@ -14,19 +14,20 @@
 #include "qemu/cutils.h"
 #include "qapi/error.h"
 #include "qemu/random.h"
+#include "crypto/random.h"
+
 
 static __thread GRand *thread_rand;
+static bool deterministic;
+
 
 /* Deterministic implementation using Glib's Mersenne Twister.  */
-bool qemu_getrandom(void *buf, size_t len, bool nonblock)
+static bool do_glib(void *buf, size_t len, bool nonblock)
 {
-GRand *rand;
+GRand *rand = thread_rand;
 size_t i;
 uint32_t x;
 
-g_assert(len <= 256);
-
-rand = thread_rand;
 if (unlikely(rand == NULL)) {
 /* Thread not initialized for a cpu, or main w/o -seed.  */
 thread_rand = rand = g_rand_new();
@@ -44,18 +45,53 @@ bool qemu_getrandom(void *buf, size_t len, bool nonblock)
 return true;
 }
 
+/* Non-deterministic implementation using crypto routines.  */
+static bool do_qcrypt(void *buf, size_t len, bool nonblock)
+{
+if (nonblock) {
+/*
+ * ??? This is not non-blocking; report failure as "would block".
+ * That said, what does "failure" really mean, and can we in fact
+ * reasonably recover from it?
+ */
+if (qcrypto_random_bytes(buf, len, NULL) < 0) {
+return false;
+}
+} else {
+int ret = qcrypto_random_bytes(buf, len, &error_fatal);
+g_assert(ret == 0);
+}
+return true;
+}
+
+bool qemu_getrandom(void *buf, size_t len, bool nonblock)
+{
+g_assert(len <= 256);
+if (unlikely(deterministic)) {
+return do_glib(buf, len, nonblock);
+} else {
+return do_qcrypt(buf, len, nonblock);
+}
+}
+
 uint64_t qemu_seedrandom_thread_part1(void)
 {
-uint64_t ret;
-qemu_getrandom(&ret, sizeof(ret), false);
-return ret;
+if (deterministic) {
+uint64_t ret;
+do_glib(&ret, sizeof(ret), false);
+return ret;
+}
+return 0;
 }
 
 void qemu_seedrandom_thread_part2(uint64_t seed)
 {
 g_assert(thread_rand == NULL);
-thread_rand = g_rand_new_with_seed_array((const guint32 *)&seed,
- sizeof(seed) / sizeof(guint32));
+if (deterministic) {
+thread_rand =
+g_rand_new_with_seed_array((const guint32 *)&seed,
+   sizeof(seed) / sizeof(guint32));
+}
 }
 
 void qemu_seedrandom_main(const char *optarg, Error **errp)
@@ -64,8 +100,7 @@ void qemu_seedrandom_main(const char *optarg, Error **errp)
 if (parse_uint_full(optarg, &seed, 0)) {
 error_setg(errp, "Invalid seed number: %s", optarg);
 } else {
-g_assert(thread_rand != NULL);
-g_rand_set_seed_array(thread_rand, (const guint32 *)&seed,
-  sizeof(seed) / sizeof(guint32));
+deterministic = true;
+qemu_seedrandom_thread_part2(seed);
 }
 }
-- 
2.17.1

[Qemu-devel] [PATCH v2 06/14] crypto: Change the qcrypto_random_bytes buffer type to void*

[Richard Henderson]

Using uint8_t* merely requires useless casts for use with
other types to be filled with randomness.

Cc: Daniel P. Berrangé 
Signed-off-by: Richard Henderson 
---
 include/crypto/random.h  | 2 +-
 crypto/random-gcrypt.c   | 2 +-
 crypto/random-gnutls.c   | 2 +-
 crypto/random-platform.c | 4 ++--
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/crypto/random.h b/include/crypto/random.h
index 8764ca0562..fde592904e 100644
--- a/include/crypto/random.h
+++ b/include/crypto/random.h
@@ -34,7 +34,7 @@
  *
  * Returns 0 on success, -1 on error
  */
-int qcrypto_random_bytes(uint8_t *buf,
+int qcrypto_random_bytes(void *buf,
  size_t buflen,
  Error **errp);
 
diff --git a/crypto/random-gcrypt.c b/crypto/random-gcrypt.c
index 9f1c9ee60e..7aea4ac81f 100644
--- a/crypto/random-gcrypt.c
+++ b/crypto/random-gcrypt.c
@@ -24,7 +24,7 @@
 
 #include 
 
-int qcrypto_random_bytes(uint8_t *buf,
+int qcrypto_random_bytes(void *buf,
  size_t buflen,
  Error **errp G_GNUC_UNUSED)
 {
diff --git a/crypto/random-gnutls.c b/crypto/random-gnutls.c
index 445fd6a30b..ed6c9ca12f 100644
--- a/crypto/random-gnutls.c
+++ b/crypto/random-gnutls.c
@@ -26,7 +26,7 @@
 #include 
 #include 
 
-int qcrypto_random_bytes(uint8_t *buf,
+int qcrypto_random_bytes(void *buf,
  size_t buflen,
  Error **errp)
 {
diff --git a/crypto/random-platform.c b/crypto/random-platform.c
index bdaa8fbbfb..51c219701d 100644
--- a/crypto/random-platform.c
+++ b/crypto/random-platform.c
@@ -63,8 +63,8 @@ int qcrypto_random_init(Error **errp)
 return 0;
 }
 
-int qcrypto_random_bytes(uint8_t *buf G_GNUC_UNUSED,
- size_t buflen G_GNUC_UNUSED,
+int qcrypto_random_bytes(void *buf,
+ size_t buflen,
  Error **errp)
 {
 #ifdef _WIN32
-- 
2.17.1

[Qemu-devel] [PATCH v2 12/14] linux-user/aarch64: Use qemu_getrandom for arm_init_pauth_key

[Richard Henderson]

Use a better interface for random numbers than rand * 3.

Cc: Laurent Vivier 
Cc: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 linux-user/aarch64/cpu_loop.c | 16 ++--
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
index d75fd9d3e2..ad30cab52d 100644
--- a/linux-user/aarch64/cpu_loop.c
+++ b/linux-user/aarch64/cpu_loop.c
@@ -20,6 +20,7 @@
 #include "qemu/osdep.h"
 #include "qemu.h"
 #include "cpu_loop-common.h"
+#include "qemu/random.h"
 
 #define get_user_code_u32(x, gaddr, env)\
 ({ abi_long __r = get_user_u32((x), (gaddr));   \
@@ -147,22 +148,9 @@ void cpu_loop(CPUARMState *env)
 }
 }
 
-static uint64_t arm_rand64(void)
-{
-int shift = 64 - clz64(RAND_MAX);
-int i, n = 64 / shift + (64 % shift != 0);
-uint64_t ret = 0;
-
-for (i = 0; i < n; i++) {
-ret = (ret << shift) | rand();
-}
-return ret;
-}
-
 void arm_init_pauth_key(ARMPACKey *key)
 {
-key->lo = arm_rand64();
-key->hi = arm_rand64();
+qemu_getrandom(key, sizeof(*key), false);
 }
 
 void target_cpu_copy_regs(CPUArchState *env, struct target_pt_regs *regs)
-- 
2.17.1

[Qemu-devel] [PATCH v2 08/14] util: Add qemu_getrandom and support functions

[Richard Henderson]

Initialize the system from system and linux-user arguments.
Propagate deterministic seeds when creating new cpu threads.

Signed-off-by: Richard Henderson 
---
 include/qemu/random.h | 58 +++
 include/qom/cpu.h |  1 +
 cpus.c|  9 ++
 linux-user/main.c | 21 ++---
 linux-user/syscall.c  |  3 ++
 util/random.c | 71 +++
 vl.c  |  4 +++
 qemu-options.hx   | 10 ++
 util/Makefile.objs|  1 +
 9 files changed, 167 insertions(+), 11 deletions(-)
 create mode 100644 include/qemu/random.h
 create mode 100644 util/random.c

diff --git a/include/qemu/random.h b/include/qemu/random.h
new file mode 100644
index 00..9d88008288
--- /dev/null
+++ b/include/qemu/random.h
@@ -0,0 +1,58 @@
+/*
+ * QEMU random functions
+ *
+ * Copyright 2019 Linaro, Ltd.
+ *
+ * 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.
+ */
+
+#ifndef QEMU_RANDOM_H
+#define QEMU_RANDOM_H
+
+/**
+ * qemu_seedrandom_main(const char *optarg, Error **errp)
+ * @optarg: a non-NULL pointer to a C string
+ * @errp: an Error handler
+ *
+ * The @optarg value is that which accompanies the -seed argument.
+ * This forces qemu_getrandom into deterministic mode.
+ */
+void qemu_seedrandom_main(const char *optarg, Error **errp);
+
+/**
+ * qemu_seedrandom_thread_part1(void)
+ *
+ * If qemu_getrandom is in deterministic mode, returns an
+ * independant seed for the new thread.  Otherwise returns 0.
+ */
+uint64_t qemu_seedrandom_thread_part1(void);
+
+/**
+ * qemu_seedrandom_thread_part2(uint64_t seed)
+ * @seed: a value for the new thread.
+ *
+ * If qemu_getrandom is in deterministic mode, this stores an
+ * independant seed for the new thread.  Otherwise a no-op.
+ */
+void qemu_seedrandom_thread_part2(uint64_t seed);
+
+/**
+ * qemu_getrandom(void *buf, size_t len, bool nonblock)
+ * @buf: a buffer of bytes to be written
+ * @len: the number of bytes in @buf
+ * @nonblock: do not delay if the entropy pool is low
+ *
+ * Fills len bytes in buf with random data.  If nonblock is false,
+ * this may require a delay while the entropy pool fills.  Returns
+ * true if the call is successful, but the only non-successful case
+ * is when nonblock is true.
+ *
+ * The value of len must be <= 256, so that the BSD getentropy(3)
+ * function can be used to implement this.
+ */
+bool qemu_getrandom(void *buf, size_t len, bool nonblock);
+
+#endif /* QEMU_RANDOM_H */
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 1d6099e5d4..343cc6d51e 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -372,6 +372,7 @@ struct CPUState {
 int singlestep_enabled;
 int64_t icount_budget;
 int64_t icount_extra;
+uint64_t random_seed;
 sigjmp_buf jmp_env;
 
 QemuMutex work_mutex;
diff --git a/cpus.c b/cpus.c
index e83f72b48b..b5d3f46220 100644
--- a/cpus.c
+++ b/cpus.c
@@ -49,6 +49,7 @@
 #include "qemu/option.h"
 #include "qemu/bitmap.h"
 #include "qemu/seqlock.h"
+#include "qemu/random.h"
 #include "tcg.h"
 #include "hw/nmi.h"
 #include "sysemu/replay.h"
@@ -1275,6 +1276,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
 /* signal CPU creation */
 cpu->created = true;
 qemu_cond_signal(&qemu_cpu_cond);
+qemu_seedrandom_thread_part2(cpu->random_seed);
 
 do {
 if (cpu_can_run(cpu)) {
@@ -1318,6 +1320,7 @@ static void *qemu_dummy_cpu_thread_fn(void *arg)
 /* signal CPU creation */
 cpu->created = true;
 qemu_cond_signal(&qemu_cpu_cond);
+qemu_seedrandom_thread_part2(cpu->random_seed);
 
 do {
 qemu_mutex_unlock_iothread();
@@ -1477,6 +1480,7 @@ static void *qemu_tcg_rr_cpu_thread_fn(void *arg)
 cpu->created = true;
 cpu->can_do_io = 1;
 qemu_cond_signal(&qemu_cpu_cond);
+qemu_seedrandom_thread_part2(cpu->random_seed);
 
 /* wait for initial kick-off after machine start */
 while (first_cpu->stopped) {
@@ -1591,6 +1595,7 @@ static void *qemu_hax_cpu_thread_fn(void *arg)
 
 hax_init_vcpu(cpu);
 qemu_cond_signal(&qemu_cpu_cond);
+qemu_seedrandom_thread_part2(cpu->random_seed);
 
 do {
 if (cpu_can_run(cpu)) {
@@ -1630,6 +1635,7 @@ static void *qemu_hvf_cpu_thread_fn(void *arg)
 /* signal CPU creation */
 cpu->created = true;
 qemu_cond_signal(&qemu_cpu_cond);
+qemu_seedrandom_thread_part2(cpu->random_seed);
 
 do {
 if (cpu_can_run(cpu)) {
@@ -1670,6 +1676,7 @@ static void *qemu_whpx_cpu_thread_fn(void *arg)
 /* signal CPU creation */
 cpu->created = true;
 qemu_cond_signal(&qemu_cpu_cond);
+qemu_seedrandom_thread_part2(cpu->random_seed);
 
 do {
 if (cpu_can_run(cpu)) {
@@ -1723,6 +1730,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
 cpu->can_do_

[Qemu-devel] [PATCH v2 05/14] crypto: Use getrandom for qcrypto_random_bytes

[Richard Henderson]

Prefer it to direct use of /dev/urandom.

Cc: Daniel P. Berrangé 
Signed-off-by: Richard Henderson 
---
 crypto/random-platform.c | 21 +
 configure| 18 +-
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/crypto/random-platform.c b/crypto/random-platform.c
index 8bfce99a65..bdaa8fbbfb 100644
--- a/crypto/random-platform.c
+++ b/crypto/random-platform.c
@@ -26,6 +26,8 @@
 #ifdef _WIN32
 #include 
 static HCRYPTPROV hCryptProv;
+#elif defined(CONFIG_GETRANDOM)
+#include 
 #else
 static int fd; /* a file handle to either /dev/urandom or /dev/random */
 #endif
@@ -39,6 +41,12 @@ int qcrypto_random_init(Error **errp)
  "Unable to create cryptographic provider");
 return -1;
 }
+#elif defined(CONFIG_GETRANDOM)
+errno = 0;
+if (getrandom(NULL, 0, 0) < 0 && errno == ENOSYS) {
+error_setg_errno(errp, errno, "getrandom");
+return -1;
+}
 #else
 /* TBD perhaps also add support for BSD getentropy / Linux
  * getrandom syscalls directly */
@@ -65,6 +73,19 @@ int qcrypto_random_bytes(uint8_t *buf G_GNUC_UNUSED,
  "Unable to read random bytes");
 return -1;
 }
+#elif defined(CONFIG_GETRANDOM)
+while (buflen > 0) {
+ssize_t got = getrandom(buf, buflen, 0);
+if (unlikely(got < 0)) {
+if (errno != EINTR) {
+error_setg_errno(errp, errno, "getrandom");
+return -1;
+}
+} else {
+buflen -= got;
+buf += got;
+}
+}
 #else
 while (buflen > 0) {
 ssize_t got = read(fd, buf, buflen);
diff --git a/configure b/configure
index 8992b3aade..6a32284d26 100755
--- a/configure
+++ b/configure
@@ -5783,6 +5783,20 @@ if compile_prog "" "" ; then
 have_utmpx=yes
 fi
 
+##
+# check for getrandom()
+
+have_getrandom=no
+cat > $TMPC << EOF
+#include 
+int main(void) {
+return getrandom(0, 0, GRND_NONBLOCK);
+}
+EOF
+if compile_prog "" "" ; then
+have_getrandom=yes
+fi
+
 ##
 # checks for sanitizers
 
@@ -7170,7 +7184,9 @@ fi
 if test "$have_utmpx" = "yes" ; then
   echo "HAVE_UTMPX=y" >> $config_host_mak
 fi
-
+if test "$have_getrandom" = "yes" ; then
+  echo "CONFIG_GETRANDOM=y" >> $config_host_mak
+fi
 if test "$ivshmem" = "yes" ; then
   echo "CONFIG_IVSHMEM=y" >> $config_host_mak
 fi
-- 
2.17.1

[Qemu-devel] [PATCH v2 04/14] crypto: Use O_CLOEXEC in qcrypto_random_init

[Richard Henderson]

Avoids leaking the /dev/urandom fd into any child processes.

Cc: Daniel P. Berrangé 
Signed-off-by: Richard Henderson 
---
 crypto/random-platform.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/crypto/random-platform.c b/crypto/random-platform.c
index 0866f216dc..8bfce99a65 100644
--- a/crypto/random-platform.c
+++ b/crypto/random-platform.c
@@ -42,9 +42,9 @@ int qcrypto_random_init(Error **errp)
 #else
 /* TBD perhaps also add support for BSD getentropy / Linux
  * getrandom syscalls directly */
-fd = open("/dev/urandom", O_RDONLY);
+fd = open("/dev/urandom", O_RDONLY | O_CLOEXEC);
 if (fd == -1 && errno == ENOENT) {
-fd = open("/dev/random", O_RDONLY);
+fd = open("/dev/random", O_RDONLY | O_CLOEXEC);
 }
 
 if (fd < 0) {
-- 
2.17.1

[Qemu-devel] [PATCH v2 13/14] linux-user: Remove srand call

[Richard Henderson]

We no longer use rand() within linux-user.

Cc: Laurent Vivier 
Signed-off-by: Richard Henderson 
---
 linux-user/main.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 192bf759b8..11a37f7f9b 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -621,8 +621,6 @@ int main(int argc, char **argv, char **envp)
 
 cpu_model = NULL;
 
-srand(time(NULL));
-
 qemu_add_opts(&qemu_trace_opts);
 
 optind = parse_args(argc, argv);
-- 
2.17.1

[Qemu-devel] [PATCH v2 02/14] crypto: Reverse code blocks in random-platform.c

[Richard Henderson]

Use #ifdef _WIN32 instead of #ifndef _WIN32.
This will make other tests easier to sequence.

Cc: Daniel P. Berrangé 
Signed-off-by: Richard Henderson 
---
 crypto/random-platform.c | 35 +--
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/crypto/random-platform.c b/crypto/random-platform.c
index 7541b4cae7..f995fc0ef1 100644
--- a/crypto/random-platform.c
+++ b/crypto/random-platform.c
@@ -32,7 +32,14 @@ static int fd; /* a file handle to either /dev/urandom or 
/dev/random */
 
 int qcrypto_random_init(Error **errp)
 {
-#ifndef _WIN32
+#ifdef _WIN32
+if (!CryptAcquireContext(&hCryptProv, NULL, NULL, PROV_RSA_FULL,
+ CRYPT_SILENT | CRYPT_VERIFYCONTEXT)) {
+error_setg_win32(errp, GetLastError(),
+ "Unable to create cryptographic provider");
+return -1;
+}
+#else
 /* TBD perhaps also add support for BSD getentropy / Linux
  * getrandom syscalls directly */
 fd = open("/dev/urandom", O_RDONLY);
@@ -44,15 +51,7 @@ int qcrypto_random_init(Error **errp)
 error_setg(errp, "No /dev/urandom or /dev/random found");
 return -1;
 }
-#else
-if (!CryptAcquireContext(&hCryptProv, NULL, NULL, PROV_RSA_FULL,
- CRYPT_SILENT | CRYPT_VERIFYCONTEXT)) {
-error_setg_win32(errp, GetLastError(),
- "Unable to create cryptographic provider");
-return -1;
-}
 #endif
-
 return 0;
 }
 
@@ -60,7 +59,15 @@ int qcrypto_random_bytes(uint8_t *buf G_GNUC_UNUSED,
  size_t buflen G_GNUC_UNUSED,
  Error **errp)
 {
-#ifndef _WIN32
+#ifdef _WIN32
+if (!CryptGenRandom(hCryptProv, buflen, buf)) {
+error_setg_win32(errp, GetLastError(),
+ "Unable to read random bytes");
+return -1;
+}
+
+return 0;
+#else
 int ret = -1;
 int got;
 
@@ -82,13 +89,5 @@ int qcrypto_random_bytes(uint8_t *buf G_GNUC_UNUSED,
 ret = 0;
  cleanup:
 return ret;
-#else
-if (!CryptGenRandom(hCryptProv, buflen, buf)) {
-error_setg_win32(errp, GetLastError(),
- "Unable to read random bytes");
-return -1;
-}
-
-return 0;
 #endif
 }
-- 
2.17.1

[Qemu-devel] [PATCH v2 03/14] crypto: Do not fail for EINTR during qcrypto_random_bytes

[Richard Henderson]

We can always get EINTR for read; /dev/urandom is no exception.
Clean up return paths and avoid unnecessary goto.

Cc: Daniel P. Berrangé 
Signed-off-by: Richard Henderson 
---
 crypto/random-platform.c | 33 +
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/crypto/random-platform.c b/crypto/random-platform.c
index f995fc0ef1..0866f216dc 100644
--- a/crypto/random-platform.c
+++ b/crypto/random-platform.c
@@ -65,29 +65,22 @@ int qcrypto_random_bytes(uint8_t *buf G_GNUC_UNUSED,
  "Unable to read random bytes");
 return -1;
 }
-
-return 0;
 #else
-int ret = -1;
-int got;
-
 while (buflen > 0) {
-got = read(fd, buf, buflen);
-if (got < 0) {
-error_setg_errno(errp, errno,
- "Unable to read random bytes");
-goto cleanup;
-} else if (!got) {
-error_setg(errp,
-   "Unexpected EOF reading random bytes");
-goto cleanup;
+ssize_t got = read(fd, buf, buflen);
+if (unlikely(got <= 0)) {
+if (got == 0) {
+error_setg(errp, "Unexpected EOF reading random bytes");
+return -1;
+} else if (errno != EINTR) {
+error_setg_errno(errp, errno, "Unable to read random bytes");
+return -1;
+}
+} else {
+buflen -= got;
+buf += got;
 }
-buflen -= got;
-buf += got;
 }
-
-ret = 0;
- cleanup:
-return ret;
 #endif
+return 0;
 }
-- 
2.17.1

[Qemu-devel] [PATCH v2 07/14] ui/vnc: Use qcrypto_random_bytes for make_challenge

[Richard Henderson]

Use a better interface for random numbers than rand,
plus some useless floating point arithmetic.

Cc: Gerd Hoffmann 
Signed-off-by: Richard Henderson 
---
v2: Use qcrypto_random_bytes, not qemu_getrandom, as there is
no need for deterministic results for this interface.
---
 ui/vnc.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 1871422e1d..9fa586dfa0 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -43,6 +43,7 @@
 #include "crypto/hash.h"
 #include "crypto/tlscredsanon.h"
 #include "crypto/tlscredsx509.h"
+#include "crypto/random.h"
 #include "qom/object_interfaces.h"
 #include "qemu/cutils.h"
 #include "io/dns-resolver.h"
@@ -2537,12 +2538,7 @@ void start_client_init(VncState *vs)
 
 static void make_challenge(VncState *vs)
 {
-int i;
-
-srand(time(NULL)+getpid()+getpid()*987654+rand());
-
-for (i = 0 ; i < sizeof(vs->challenge) ; i++)
-vs->challenge[i] = (int) (256.0*rand()/(RAND_MAX+1.0));
+qcrypto_random_bytes(vs->challenge, sizeof(vs->challenge), &error_fatal);
 }
 
 static int protocol_client_auth_vnc(VncState *vs, uint8_t *data, size_t len)
-- 
2.17.1

[Qemu-devel] [PATCH v2 11/14] linux-user: Use qemu_getrandom for AT_RANDOM

[Richard Henderson]

Use a better interface for random numbers than rand * 16.

Cc: Laurent Vivier 
Signed-off-by: Richard Henderson 
---
 linux-user/elfload.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index c1a26021f8..f33bf4fb85 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -7,6 +7,7 @@
 #include "qemu.h"
 #include "disas/disas.h"
 #include "qemu/path.h"
+#include "qemu/random.h"
 
 #ifdef _ARCH_PPC64
 #undef ARCH_DLINFO
@@ -1883,12 +1884,9 @@ static abi_ulong create_elf_tables(abi_ulong p, int 
argc, int envc,
 }
 
 /*
- * Generate 16 random bytes for userspace PRNG seeding (not
- * cryptically secure but it's not the aim of QEMU).
+ * Generate 16 random bytes for userspace PRNG seeding.
  */
-for (i = 0; i < 16; i++) {
-k_rand_bytes[i] = rand();
-}
+qemu_getrandom(k_rand_bytes, sizeof(k_rand_bytes), false);
 if (STACK_GROWS_DOWN) {
 sp -= 16;
 u_rand_bytes = sp;
-- 
2.17.1

[Qemu-devel] [PATCH v2 01/14] crypto: Merge crypto-obj-y into libqemuutil.a

[Richard Henderson]

We will shortly need this in the user-only binaries, so drop the split
into system and tools binaries.  This also means that crypto-aes-obj-y
can be merged back into crypto-obj-y.

Cc: Daniel P. Berrangé 
Signed-off-by: Richard Henderson 
---
 Makefile | 12 +---
 Makefile.objs|  8 ++--
 Makefile.target  |  4 
 configure|  9 +++--
 crypto/Makefile.objs |  5 +
 5 files changed, 11 insertions(+), 27 deletions(-)

diff --git a/Makefile b/Makefile
index abd78a9826..a85f33f47e 100644
--- a/Makefile
+++ b/Makefile
@@ -405,7 +405,6 @@ dummy := $(call unnest-vars,, \
 block-obj-y \
 block-obj-m \
 crypto-obj-y \
-crypto-aes-obj-y \
 qom-obj-y \
 io-obj-y \
 common-obj-y \
@@ -441,7 +440,6 @@ SOFTMMU_SUBDIR_RULES=$(filter %-softmmu,$(SUBDIR_RULES))
 
 $(SOFTMMU_SUBDIR_RULES): $(authz-obj-y)
 $(SOFTMMU_SUBDIR_RULES): $(block-obj-y)
-$(SOFTMMU_SUBDIR_RULES): $(crypto-obj-y)
 $(SOFTMMU_SUBDIR_RULES): $(io-obj-y)
 $(SOFTMMU_SUBDIR_RULES): config-all-devices.mak
 
@@ -496,7 +494,7 @@ Makefile: $(version-obj-y)
 ##
 # Build libraries
 
-libqemuutil.a: $(util-obj-y) $(trace-obj-y) $(stub-obj-y)
+libqemuutil.a: $(util-obj-y) $(trace-obj-y) $(stub-obj-y) $(crypto-obj-y)
 libvhost-user.a: $(libvhost-user-obj-y) $(util-obj-y) $(stub-obj-y)
 
 ##
@@ -505,9 +503,9 @@ COMMON_LDADDS = libqemuutil.a
 
 qemu-img.o: qemu-img-cmds.h
 
-qemu-img$(EXESUF): qemu-img.o $(authz-obj-y) $(block-obj-y) $(crypto-obj-y) 
$(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS)
-qemu-nbd$(EXESUF): qemu-nbd.o $(authz-obj-y) $(block-obj-y) $(crypto-obj-y) 
$(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS)
-qemu-io$(EXESUF): qemu-io.o $(authz-obj-y) $(block-obj-y) $(crypto-obj-y) 
$(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS)
+qemu-img$(EXESUF): qemu-img.o $(authz-obj-y) $(block-obj-y) $(io-obj-y) 
$(qom-obj-y) $(COMMON_LDADDS)
+qemu-nbd$(EXESUF): qemu-nbd.o $(authz-obj-y) $(block-obj-y) $(io-obj-y) 
$(qom-obj-y) $(COMMON_LDADDS)
+qemu-io$(EXESUF): qemu-io.o $(authz-obj-y) $(block-obj-y) $(io-obj-y) 
$(qom-obj-y) $(COMMON_LDADDS)
 
 qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o $(COMMON_LDADDS)
 
@@ -518,7 +516,7 @@ qemu-edid$(EXESUF): qemu-edid.o hw/display/edid-generate.o 
$(COMMON_LDADDS)
 fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o 
fsdev/9p-marshal.o fsdev/9p-iov-marshal.o $(COMMON_LDADDS)
 fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap
 
-scsi/qemu-pr-helper$(EXESUF): scsi/qemu-pr-helper.o scsi/utils.o 
$(authz-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS)
+scsi/qemu-pr-helper$(EXESUF): scsi/qemu-pr-helper.o scsi/utils.o 
$(authz-obj-y) $(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS)
 ifdef CONFIG_MPATH
 scsi/qemu-pr-helper$(EXESUF): LIBS += -ludev -lmultipath -lmpathpersist
 endif
diff --git a/Makefile.objs b/Makefile.objs
index 72debbf5c5..23950a37af 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -3,6 +3,8 @@
 stub-obj-y = stubs/ util/ crypto/
 util-obj-y = util/ qobject/ qapi/
 
+crypto-obj-y = crypto/
+
 chardev-obj-y = chardev/
 
 ###
@@ -21,12 +23,6 @@ block-obj-$(CONFIG_REPLICATION) += replication.o
 
 block-obj-m = block/
 
-###
-# crypto-obj-y is code used by both qemu system emulation and qemu-img
-
-crypto-obj-y = crypto/
-crypto-aes-obj-y = crypto/
-
 ###
 # qom-obj-y is code used by both qemu system emulation and qemu-img
 
diff --git a/Makefile.target b/Makefile.target
index d8048aab8f..8840d70b57 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -181,8 +181,6 @@ dummy := $(call unnest-vars,.., \
block-obj-y \
block-obj-m \
chardev-obj-y \
-   crypto-obj-y \
-   crypto-aes-obj-y \
qom-obj-y \
io-obj-y \
common-obj-y \
@@ -191,8 +189,6 @@ all-obj-y += $(common-obj-y)
 all-obj-y += $(qom-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(authz-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y) $(chardev-obj-y)
-all-obj-$(CONFIG_USER_ONLY) += $(crypto-aes-obj-y)
-all-obj-$(CONFIG_SOFTMMU) += $(crypto-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(io-obj-y)
 
 ifdef CONFIG_SOFTMMU
diff --git a/configure b/configure
index abe453f8fd..8992b3aade 100755
--- a/configure
+++ b/configure
@@ -2771,8 +2771,7 @@ if test "$gnutls" != "no"; then
 if $pkg_config --exists "gnutls >= 3.1.18"; then
 gnutls_cflags=$($pkg_config --cflags gnutls)
 gnutls_libs=$($pkg_config --libs gnutls)
-libs_softmmu="$gnutls_libs $libs_softmmu"
-libs_tools="$gnutls_libs $libs_tools"
+LIBS="$gnutls_lib

[Qemu-devel] [PATCH v2 10/14] linux-user: Call qcrypto_init if not using -seed

[Richard Henderson]

This is required before using qcrypto_random_bytes
by way of qemu_getrandom.

Cc: Laurent Vivier 
Signed-off-by: Richard Henderson 
---
 linux-user/main.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/linux-user/main.c b/linux-user/main.c
index 9682e81610..192bf759b8 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -38,6 +38,7 @@
 #include "trace/control.h"
 #include "target_elf.h"
 #include "cpu_loop-common.h"
+#include "crypto/init.h"
 
 char *exec_path;
 
@@ -688,6 +689,13 @@ int main(int argc, char **argv, char **envp)
 }
 if (seed_optarg != NULL) {
 qemu_seedrandom_main(seed_optarg, &error_fatal);
+} else {
+/* ??? This assumes qcrypto is only used by qemu_getrandom.  */
+Error *err = NULL;
+if (qcrypto_init(&err) < 0) {
+error_reportf_err(err, "cannot initialize crypto: ");
+exit(1);
+}
 }
 
 target_environ = envlist_to_environ(envlist, NULL);
-- 
2.17.1

[Qemu-devel] [PATCH v2 00/14] Add qemu_getrandom and ARMv8.5-RNG

[Richard Henderson]

Changes since v1:
  * Build crypto-obj-y for linux-user as well.
  * Several patches to tidy crypto/random-platform.c.
  * Use getrandom(2) in crypto/random-platform.c.
  * Use qcrypto_random_bytes in ui/vnc.c.
  * In qemu_getrandom:
- Use g_rand_int instead of srand48.
- Use qcrypto_random_bytes instead of getrandom directly.


r~


Richard Henderson (14):
  crypto: Merge crypto-obj-y into libqemuutil.a
  crypto: Reverse code blocks in random-platform.c
  crypto: Do not fail for EINTR during qcrypto_random_bytes
  crypto: Use O_CLOEXEC in qcrypto_random_init
  crypto: Use getrandom for qcrypto_random_bytes
  crypto: Change the qcrypto_random_bytes buffer type to void*
  ui/vnc: Use qcrypto_random_bytes for make_challenge
  util: Add qemu_getrandom and support functions
  util: Use qcrypto_random_bytes for qemu_getrandom
  linux-user: Call qcrypto_init if not using -seed
  linux-user: Use qemu_getrandom for AT_RANDOM
  linux-user/aarch64: Use qemu_getrandom for arm_init_pauth_key
  linux-user: Remove srand call
  target/arm: Implement ARMv8.5-RNG

 Makefile  |  12 ++--
 Makefile.objs |   8 +--
 Makefile.target   |   4 --
 include/crypto/random.h   |   2 +-
 include/qemu/random.h |  58 +++
 include/qom/cpu.h |   1 +
 target/arm/cpu.h  |   5 ++
 cpus.c|   9 +++
 crypto/random-gcrypt.c|   2 +-
 crypto/random-gnutls.c|   2 +-
 crypto/random-platform.c  |  95 +-
 linux-user/aarch64/cpu_loop.c |  16 +
 linux-user/elfload.c  |   8 +--
 linux-user/main.c |  31 +-
 linux-user/syscall.c  |   3 +
 target/arm/cpu64.c|   1 +
 target/arm/helper.c   |  32 ++
 ui/vnc.c  |   8 +--
 util/random.c | 106 ++
 vl.c  |   4 ++
 configure |  27 ++---
 crypto/Makefile.objs  |   5 +-
 qemu-options.hx   |  10 
 util/Makefile.objs|   1 +
 24 files changed, 340 insertions(+), 110 deletions(-)
 create mode 100644 include/qemu/random.h
 create mode 100644 util/random.c

-- 
2.17.1

Re: [Qemu-devel] [PATCH v2 04/13] spapr/xive: add state synchronization with KVM

[David Gibson]

On Mon, Mar 11, 2019 at 09:41:12PM +0100, Cédric Le Goater wrote:
> On 2/26/19 1:01 AM, David Gibson wrote:
> > On Fri, Feb 22, 2019 at 02:13:13PM +0100, Cédric Le Goater wrote:
> >> This extends the KVM XIVE device backend with 'synchronize_state'
> >> methods used to retrieve the state from KVM. The HW state of the
> >> sources, the KVM device and the thread interrupt contexts are
> >> collected for the monitor usage and also migration.
> >>
> >> These get operations rely on their KVM counterpart in the host kernel
> >> which acts as a proxy for OPAL, the host firmware. The set operations
> >> will be added for migration support later.
> >>
> >> Signed-off-by: Cédric Le Goater 
> >> ---
> >>  include/hw/ppc/spapr_xive.h |  8 
> >>  include/hw/ppc/xive.h   |  1 +
> >>  hw/intc/spapr_xive.c| 17 ---
> >>  hw/intc/spapr_xive_kvm.c| 89 +
> >>  hw/intc/xive.c  | 10 +
> >>  5 files changed, 118 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> >> index 749c6cbc2c56..ebd65e7fe36b 100644
> >> --- a/include/hw/ppc/spapr_xive.h
> >> +++ b/include/hw/ppc/spapr_xive.h
> >> @@ -44,6 +44,13 @@ typedef struct sPAPRXive {
> >>  void  *tm_mmap;
> >>  } sPAPRXive;
> >>  
> >> +/*
> >> + * The sPAPR machine has a unique XIVE IC device. Assign a fixed value
> >> + * to the controller block id value. It can nevertheless be changed
> >> + * for testing purpose.
> >> + */
> >> +#define SPAPR_XIVE_BLOCK_ID 0x0
> >> +
> >>  bool spapr_xive_irq_claim(sPAPRXive *xive, uint32_t lisn, bool lsi);
> >>  bool spapr_xive_irq_free(sPAPRXive *xive, uint32_t lisn);
> >>  void spapr_xive_pic_print_info(sPAPRXive *xive, Monitor *mon);
> >> @@ -74,5 +81,6 @@ void kvmppc_xive_set_queue_config(sPAPRXive *xive, 
> >> uint8_t end_blk,
> >>  void kvmppc_xive_get_queue_config(sPAPRXive *xive, uint8_t end_blk,
> >>   uint32_t end_idx, XiveEND *end,
> >>   Error **errp);
> >> +void kvmppc_xive_synchronize_state(sPAPRXive *xive, Error **errp);
> >>  
> >>  #endif /* PPC_SPAPR_XIVE_H */
> >> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> >> index 061d43fea24d..f3766fd881a2 100644
> >> --- a/include/hw/ppc/xive.h
> >> +++ b/include/hw/ppc/xive.h
> >> @@ -431,5 +431,6 @@ void kvmppc_xive_source_reset_one(XiveSource *xsrc, 
> >> int srcno, Error **errp);
> >>  void kvmppc_xive_source_reset(XiveSource *xsrc, Error **errp);
> >>  void kvmppc_xive_source_set_irq(void *opaque, int srcno, int val);
> >>  void kvmppc_xive_cpu_connect(XiveTCTX *tctx, Error **errp);
> >> +void kvmppc_xive_cpu_synchronize_state(XiveTCTX *tctx, Error **errp);
> >>  
> >>  #endif /* PPC_XIVE_H */
> >> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> >> index 3db24391e31c..9f07567f4d78 100644
> >> --- a/hw/intc/spapr_xive.c
> >> +++ b/hw/intc/spapr_xive.c
> >> @@ -40,13 +40,6 @@
> >>  
> >>  #define SPAPR_XIVE_NVT_BASE 0x400
> >>  
> >> -/*
> >> - * The sPAPR machine has a unique XIVE IC device. Assign a fixed value
> >> - * to the controller block id value. It can nevertheless be changed
> >> - * for testing purpose.
> >> - */
> >> -#define SPAPR_XIVE_BLOCK_ID 0x0
> >> -
> >>  /*
> >>   * sPAPR NVT and END indexing helpers
> >>   */
> >> @@ -153,6 +146,16 @@ void spapr_xive_pic_print_info(sPAPRXive *xive, 
> >> Monitor *mon)
> >>  XiveSource *xsrc = &xive->source;
> >>  int i;
> >>  
> >> +if (kvm_irqchip_in_kernel()) {
> >> +Error *local_err = NULL;
> >> +
> >> +kvmppc_xive_synchronize_state(xive, &local_err);
> >> +if (local_err) {
> >> +error_report_err(local_err);
> >> +return;
> >> +}
> >> +}
> >> +
> >>  monitor_printf(mon, "  LSIN PQEISN CPU/PRIO EQ\n");
> >>  
> >>  for (i = 0; i < xive->nr_irqs; i++) {
> >> diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
> >> index 6b50451b4f85..4b1ffb9835f9 100644
> >> --- a/hw/intc/spapr_xive_kvm.c
> >> +++ b/hw/intc/spapr_xive_kvm.c
> >> @@ -60,6 +60,57 @@ static void kvm_cpu_enable(CPUState *cs)
> >>  /*
> >>   * XIVE Thread Interrupt Management context (KVM)
> >>   */
> >> +static void kvmppc_xive_cpu_get_state(XiveTCTX *tctx, Error **errp)
> >> +{
> >> +uint64_t state[4] = { 0 };
> >> +int ret;
> >> +
> >> +ret = kvm_get_one_reg(tctx->cs, KVM_REG_PPC_NVT_STATE, state);
> >> +if (ret != 0) {
> >> +error_setg_errno(errp, errno,
> >> + "XIVE: could not capture KVM state of CPU %ld",
> >> + kvm_arch_vcpu_id(tctx->cs));
> >> +return;
> >> +}
> >> +
> >> +/* word0 and word1 of the OS ring. */
> >> +*((uint64_t *) &tctx->regs[TM_QW1_OS]) = state[0];
> >> +
> >> +/*
> >> + * KVM also returns word2 containing the OS CAM line which is
> >> + * interesting to print out in the QEMU monitor.
> >> +   

Re: [Qemu-devel] [PATCH v2 02/13] spapr/xive: add hcall support when under KVM

[David Gibson]

On Wed, Mar 13, 2019 at 11:43:54AM +0100, Cédric Le Goater wrote:
> On 3/12/19 11:26 AM, David Gibson wrote:
> > On Mon, Mar 11, 2019 at 06:32:05PM +0100, Cédric Le Goater wrote:
> >> On 2/26/19 12:22 AM, David Gibson wrote:
> >>> On Fri, Feb 22, 2019 at 02:13:11PM +0100, Cédric Le Goater wrote:
> > [snip]
>  +void kvmppc_xive_set_source_config(sPAPRXive *xive, uint32_t lisn, 
>  XiveEAS *eas,
>  +   Error **errp)
>  +{
>  +uint32_t end_idx;
>  +uint32_t end_blk;
>  +uint32_t eisn;
>  +uint8_t priority;
>  +uint32_t server;
>  +uint64_t kvm_src;
>  +Error *local_err = NULL;
>  +
>  +/*
>  + * No need to set a MASKED source, this is the default state after
>  + * reset.
> >>>
> >>> I don't quite follow this comment, why is there no need to call a
> >>> MASKED source?
> >>
> >> because MASKED is the default state in which KVM initializes the IRQ. I 
> >> will
> >> clarify.
> > 
> > I believe it's possible - though rare - to process an incoming
> > migration on an established VM which isn't in fresh reset state.  So
> > it's best not to rely on that.
> > 
>  +static void xive_esb_trigger(XiveSource *xsrc, int srcno)
>  +{
>  +unsigned long addr = (unsigned long) xsrc->esb_mmap +
>  +xive_source_esb_page(xsrc, srcno);
>  +
>  +*((uint64_t *) addr) = 0x0;
>  +}
> >>>
> >>> Also.. aren't some of these register accesses likely to need memory
> >>> barriers?
> >>
> >> AIUI, these are CI pages. So we shouldn't need barriers.
> > 
> > CI doesn't negate the need for barriers, althugh it might change the
> > type you need.  At the very least you need a compiler barrier to stop
> > it re-ordering the access, but you can also have in-cpu store and load
> > queues.
> > 
> 
> ok. So I will need to add some smp_r/wmb() 

No, smp_[rw]mb() is for cases where it's strictly about cpu vs. cpu
ordering.  Here it's cpu vs. IO ordering so you need plain [rw]mb().

-- 
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 v2] MAINTAINERS: PPC: add a PowerNV machine entry

[David Gibson]

On Wed, Mar 13, 2019 at 05:24:23PM +0100, Cédric Le Goater wrote:
> and declare David and myself as maintainers of the PPC PowerNV
> (Non-Virtualized) machine using the OPAL (skiboot) firmware.

Applied to ppc-for-4.0, thanks.

> 
> Signed-off-by: Cédric Le Goater 
> Reviewed-by: Greg Kurz 
> ---
>  MAINTAINERS | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d3267560799b..e771810cfb73 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1059,7 +1059,6 @@ F: include/hw/*/xics*
>  F: pc-bios/spapr-rtas/*
>  F: pc-bios/spapr-rtas.bin
>  F: pc-bios/slof.bin
> -F: pc-bios/skiboot.lid
>  F: docs/specs/ppc-spapr-hcalls.txt
>  F: docs/specs/ppc-spapr-hotplug.txt
>  F: tests/spapr*
> @@ -1067,6 +1066,18 @@ F: tests/libqos/*spapr*
>  F: tests/rtas*
>  F: tests/libqos/rtas*
>  
> +PowerNV (Non-Virtualized)
> +M: Cédric Le Goater 
> +M: David Gibson 
> +L: qemu-...@nongnu.org
> +S: Maintained
> +F: hw/ppc/pnv*
> +F: hw/intc/pnv*
> +F: hw/intc/xics_pnv.c
> +F: include/hw/ppc/pnv*
> +F: pc-bios/skiboot.lid
> +F: tests/pnv*
> +
>  virtex_ml507
>  M: Edgar E. Iglesias 
>  L: qemu-...@nongnu.org

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] spapr: Correctly set LPCR[GTSE] in H_REGISTER_PROCESS_TABLE

[David Gibson]

On Wed, Mar 13, 2019 at 08:17:42AM +0100, Cédric Le Goater wrote:
> On 3/13/19 4:20 AM, David Gibson wrote:
> > 176dccee "target/ppc/spapr: Clear partition table entry when allocating
> > hash table" reworked the H_REGISTER_PROCESS_TABLE hypercall, but
> > unfortunately due to a small error no longer correctly sets the LPCR[GTSE]
> > bit which allows the guest to directly execute (some types of) tlbie (TLB
> > flush) instructions without involving the hypervisor.
> > 
> > We got away with this, initially, because POWER9 did not have hypervisor
> > mode enabled in its msr_mask, which meant we didn't actually run hypervisor
> > privilege checks in TCG at all.  However, da874d90 "target/ppc: add HV
> > support for POWER9" turned on HV support on POWER9 for the benefit of the
> > powernv machine type.
> > 
> > This exposed the earlier bug in H_REGISTER_PROCESS_TABLE, and causes guests
> > which rely on LPCR[GTSE] (i.e. basically all of them) to crash during early
> > boot when their first tlbie instruction causes an unexpected trap.
> > 
> > Fixes: 176dccee target/ppc/spapr: Clear partition table entry when 
> > allocating hash table
> > Signed-off-by: David Gibson 
> 
> Nice catch. 
> 
> May be we should rename the flags from FLAG_ to PROC_TBL__FLAG ? 

Meh, the current names are theoretically ambiguous, but they're
short.  If they were truly global I'd be concerned, but given they're
#defined right above I don't think there's any urgency.

> Reviewed-by: Cédric Le Goater 
> 
> Thanks,
> 
> C.
> 
> > ---
> >  hw/ppc/spapr_hcall.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > index 0761e10142..8a736797b9 100644
> > --- a/hw/ppc/spapr_hcall.c
> > +++ b/hw/ppc/spapr_hcall.c
> > @@ -1400,7 +1400,8 @@ static target_ulong 
> > h_register_process_table(PowerPCCPU *cpu,
> >  else if (flags & FLAG_HASH_PROC_TBL) /* Hash with process tables */
> >  update_lpcr |= LPCR_UPRT;
> >  if (flags & FLAG_GTSE)  /* Guest translation shootdown enable */
> > -update_lpcr |= FLAG_GTSE;
> > +update_lpcr |= LPCR_GTSE;
> > +
> >  spapr_set_all_lpcrs(update_lpcr, LPCR_UPRT | LPCR_HR | LPCR_GTSE);
> >  
> >  if (kvm_enabled()) {
> > 
> 

-- 
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] [PULL 00/60] ppc-for-4.0 queue 20190310

[David Gibson]

On Wed, Mar 13, 2019 at 09:30:54AM +, Alex Bennée wrote:
> 
> David Gibson  writes:
> 
> > On Tue, Mar 12, 2019 at 07:20:22PM +, Alex Bennée wrote:
> >>
> >> David Gibson  writes:
> >>
> >> > On Mon, Mar 11, 2019 at 10:40:54AM +, Alex Bennée wrote:
> >> >>
> >
> > [snip]
> >> >> > MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
> >> >> > QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64
> >> >> > QTEST_QEMU_IMG=qemu-img tests/
> >> >> > boot-serial-test -m=quick -k --tap < /dev/null |
> >> >> > ./scripts/tap-driver.pl --test-name="boot-serial-test"
> >> >> > PASS 1 boot-serial-test /ppc64/boot-serial/ppce500
> >> >> > PASS 2 boot-serial-test /ppc64/boot-serial/40p
> >> >> > PASS 3 boot-serial-test /ppc64/boot-serial/mac99
> >> >> > qemu-system-ppc64: warning: TCG doesn't support requested feature,
> >> >> > cap-cfpc=workaround
> >> >> > qemu-system-ppc64: warning: TCG doesn't support requested feature,
> >> >> > cap-sbbc=workaround
> >> >> > qemu-system-ppc64: warning: TCG doesn't support requested feature,
> >> >> > cap-ibs=workaround
> >> >> > PASS 4 boot-serial-test /ppc64/boot-serial/pseries
> >> >> > PASS 5 boot-serial-test /ppc64/boot-serial/powernv
> >> >> > PASS 6 boot-serial-test /ppc64/boot-serial/sam460ex
> >> >> >
> >> >> > and similarly during the boot-pxe-test.
> >> >> >
> >> >> > Could you silence these, please?
> >> >>
> >> >> FWIW this PR contains fixes that will finally get the gitlab CI green so
> >> >> I look forward to v2 getting merged ;-)
> >> >
> >> > Huh.  I knew about the travis CI and the shippable CI, but not the
> >> > gitlab one.  How do I see that?  It was surprisingly non-obvious from
> >> > the gitlab qemu project page.
> >>
> >> The various badges should link to the relevant bits:
> >>
> >>   https://wiki.qemu.org/Testing
> >
> > Um.. not that I can see.  The "GitLab CI" link on the left of the
> > table takes me to a GitLab mirror of qemu, but I can't spot any links
> > to CI status or information.
> >
> > The link on the right, next to the badge gives me a 404.
> 
> Ahh looks like a permission thing. I assume you also asked for access to
> the project as @dgibson?

I did.  It wasn't very clear to me if that was needed to see anything,
or was actually requesting some sort of write or admin permission.

> I'll dig through and see if I can make the view public, it should be.

Sounds good.

-- 
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 1/2] ppc/pnv: Use local_err variable in pnv_chip_power9_intc_create()

[David Gibson]

On Wed, Mar 13, 2019 at 03:54:01PM +0100, Greg Kurz wrote:
> Detected by Coverity: CID 1399702

Series applied to ppc-for-4.0, thanks.

> 
> Signed-off-by: Greg Kurz 
> ---
>  hw/ppc/pnv.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 8be4d4cbf785..dfb4ea5742c1 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -755,7 +755,7 @@ static void pnv_chip_power9_intc_create(PnvChip *chip, 
> PowerPCCPU *cpu,
>   * controller object is initialized afterwards. Hopefully, it's
>   * only used at runtime.
>   */
> -obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(&chip9->xive), errp);
> +obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(&chip9->xive), 
> &local_err);
>  if (local_err) {
>  error_propagate(errp, local_err);
>  return;
> 

-- 
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] ppc/pnv: update skiboot to commit 261ca8e779e5.

[David Gibson]

On Wed, Mar 13, 2019 at 08:20:40AM +0100, Cédric Le Goater wrote:
> On 3/13/19 5:23 AM, David Gibson wrote:
> > On Sun, Mar 10, 2019 at 06:53:37PM +0100, Cédric Le Goater wrote:
> >> It includes better support for POWER9 processor and the QEMU platform.
> >> DD1.0 workarounds have been removed which simplifies a bit the XIVE
> >> PowerNV model.
> >>
> >> Built from submodule.
> >>
> >> Signed-off-by: Cédric Le Goater 
> >> ---
> >>
> >>  David,
> >>
> >>  If you resend your QEMU 4.0 PR, that should be a nice to have for
> >>  PowerNV.
> > 
> > Sorry, although I did respin the PR I missed this one.  I had a long
> > frustrating day yesterday, I'm afraid.
> > 
> > Since this only affects powernv, which isn't really fully stable to
> > begin with, and is essential to actully use the other powernv POWER9
> > changes which were merged, I've put this into ppc-for-4.0.
> > 
> > I'll make the case to put it in during the soft freeze, but it's
> > possible it will have to be punted to 4.1.
> 
> That's fine. I also waited the last minute to send it. 
> 
> I received a couple of messages like below which I don't understand.
> May be the patch is too big ?

Hm, I don't know.  It's possible.

> 
> Thanks,
> 
> C.
> 
> 
> This message was created automatically by mail delivery software.
> A message that you sent has not yet been delivered to one or more of its
> recipients after more than 48 hours on the queue on eggs.gnu.org.
> 
> The message identifier is: 1h32eQ-00027L-6z
> The subject of the message is: [PATCH] ppc/pnv: update skiboot to commit 
> 261ca8e779e5.
> The date of the message is:Sun, 10 Mar 2019 18:53:37 +0100
> 
> The address to which the message has not yet been delivered is:
> 
>   qemu-devel@nongnu.org
> 
> No action is required on your part. Delivery attempts will continue for
> some time, and this warning may be repeated at intervals if the message
> remains undelivered. Eventually the mail delivery software will give up,
> and when that happens, the message will be returned to you.
> 

-- 
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 5/7] tcg/i386: add support for IBT

[Richard Henderson]

On 3/13/19 5:40 AM, Paolo Bonzini wrote:
> +static void tcg_out_endbr(TCGContext *s)
> +{
> +#if defined __CET__ && (__CET__ & 1)
> +#ifdef __x86_64__
> +tcg_out32(s, 0xfa1e0ff3);
> +#else
> +tcg_out32(s, 0xfb1e0ff3);
> +#endif
> +#endif
> +}

Normally we'd use a runtime test for the feature.

Just because we compiled with CET does not mean we're running on CET-enabled
hardware, since IIRC this is a nop otherwise.  I assume there's a cpuid/xgetbv
bit that indicates when IBT is present and/or active?


r~

Re: [Qemu-devel] [PATCH 0/5] QEMU VFIO live migration

[Zhao Yan]

On Thu, Mar 14, 2019 at 03:14:54AM +0800, Alex Williamson wrote:
> On Tue, 12 Mar 2019 21:13:01 -0400
> Zhao Yan  wrote:
> 
> > hi Alex
> > Any comments to the sequence below?
> > 
> > Actaully we have some concerns and suggestions to userspace-opaque migration
> > data.
> > 
> > 1. if data is opaque to userspace, kernel interface must be tightly bound to
> > migration. 
> >e.g. vendor driver has to know state (running + not logging) should not
> >return any data, and state (running + logging) should return whole
> >snapshot first and dirty later. it also has to know qemu migration will
> >not call GET_BUFFER in state (running + not logging), otherwise, it has
> >to adjust its behavior.
> 
> This all just sounds like defining the protocol we expect with the
> interface.  For instance if we define a session as beginning when
> logging is enabled and ending when the device is stopped and the
> interface reports no more data is available, then we can state that any
> partial accumulation of data is incomplete relative to migration.  If
> userspace wants to initiate a new migration stream, they can simply
> toggle logging.  How the vendor driver provides the data during the
> session is not defined, but beginning the session with a snapshot
> followed by repeated iterations of dirtied data is certainly a valid
> approach.
> 
> > 2. vendor driver cannot ensure userspace get all the data it intends to
> > save in pre-copy phase.
> >   e.g. in stop-and-copy phase, vendor driver has to first check and send
> >   data in previous phase.
> 
> First, I don't think the device has control of when QEMU switches from
> pre-copy to stop-and-copy, the protocol needs to support that
> transition at any point.  However, it seems a simply data available
> counter provides an indication of when it might be optimal to make such
> a transition.  If a vendor driver follows a scheme as above, the
> available data counter would indicate a large value, the entire initial
> snapshot of the device.  As the migration continues and pages are
> dirtied, the device would reach a steady state amount of data
> available, depending on the guest activity.  This could indicate to the
> user to stop the device.  The migration stream would not be considered
> completed until the available data counter reaches zero while the
> device is in the stopped|logging state.
> 
> > 3. if all the sequence is tightly bound to live migration, can we remove the
> > logging state? what about adding two states migrate-in and migrate-out?
> > so there are four states: running, stopped, migrate-in, migrate-out.
> >migrate-out is for source side when migration starts. together with
> >state running and stopped, it can substitute state logging.
> >migrate-in is for target side.
> 
> In fact, Kirti's implementation specifies a data direction, but I think
> we still need logging to indicate sessions.  I'd also assume that
> logging implies some overhead for the vendor driver.
>
ok. If you prefer logging, I'm ok with it. just found migrate-in and
migrate-out are more universal againt hardware requirement changes.

> > On Tue, Mar 12, 2019 at 10:57:47AM +0800, Zhao Yan wrote:
> > > hi Alex
> > > thanks for your reply.
> > > 
> > > So, if we choose migration data to be userspace opaque, do you think below
> > > sequence is the right behavior for vendor driver to follow:
> > > 
> > > 1. initially LOGGING state is not set. If userspace calls GET_BUFFER to
> > > vendor driver,  vendor driver should reject and return 0.
> 
> What would this state mean otherwise?  If we're not logging then it
> should not be expected that we can construct dirtied data from a
> previous read of the state before logging was enabled (it would be
> outside of the "session").  So at best this is an incomplete segment of
> the initial snapshot of the device, but that presumes how the vendor
> driver constructs the data.  I wouldn't necessarily mandate the vendor
> driver reject it, but I think we should consider it undefined and
> vendor specific relative to the migration interface.
> 
> > > 2. then LOGGING state is set, if userspace calls GET_BUFFER to vendor
> > > driver,
> > >a. vendor driver shoud first query a whole snapshot of device memory
> > >(let's use this term to represent device's standalone memory for now),
> > >b. vendor driver returns a chunk of data just queried to userspace,
> > >while recording current pos in data.
> > >c. vendor driver finds all data just queried is finished transmitting 
> > > to
> > >userspace, and queries only dirty data in device memory now.
> > >d. vendor driver returns a chunk of data just quered (this time is 
> > > dirty
> > >data )to userspace while recording current pos in data
> > >e. if all data is transmited to usespace and still GET_BUFFERs come 
> > > from
> > >userspace, vendor driver starts another round of dirty data query.
> 
> This is a valid vendor driver approach, but 

Re: [Qemu-devel] [PATCH] ati-vga: Implement DDC and EDID info from monitor

[BALATON Zoltan]

On Mon, 11 Mar 2019, BALATON Zoltan wrote:
> On Mon, 11 Mar 2019, Gerd Hoffmann wrote:
>> On Sun, Mar 10, 2019 at 12:22:17AM +0100, BALATON Zoltan wrote:
>>> This adds DDC support to ati-vga and connects i2c-ddc to provide EDID
>>> info that is read by guests to find available screen modes. Not sure
>>> if this is 100% correct yet but at least MorphOS is happy with it and
>>> starts in a high resolution mode instead of 640x480 (although its
>>> splash screen is still not correct).
>> 
>>> Linux needs support from VESA
>>> vgabios, it seems to be missing INT10 0x4F15 function (see
>>> https://gitlab.freedesktop.org/xorg/xserver/blob/master/hw/xfree86/vbe/vbe.c)
>>> without which no DDC is available that also prevents loading the
>>> accelerated X driver.
>> 
>> radeonfb doesn't pick up the edid either.
>> There seem to be four i2c busses though:
>> 
>> void radeon_create_i2c_busses(struct radeonfb_info *rinfo)
>> {
>>  rinfo->i2c[0].rinfo = rinfo;
>>  rinfo->i2c[0].ddc_reg   = GPIO_MONID;
>> #ifndef CONFIG_PPC
>>  rinfo->i2c[0].adapter.class = I2C_CLASS_HWMON;
>> #endif
>>  radeon_setup_i2c_bus(&rinfo->i2c[0], "monid");
>>
>>  rinfo->i2c[1].rinfo = rinfo;
>>  rinfo->i2c[1].ddc_reg   = GPIO_DVI_DDC;
>>  radeon_setup_i2c_bus(&rinfo->i2c[1], "dvi");
>>
>>  rinfo->i2c[2].rinfo = rinfo;
>>  rinfo->i2c[2].ddc_reg   = GPIO_VGA_DDC;
>>  radeon_setup_i2c_bus(&rinfo->i2c[2], "vga");
>>
>>  rinfo->i2c[3].rinfo = rinfo;
>>  rinfo->i2c[3].ddc_reg   = GPIO_CRT2_DDC;
>>  radeon_setup_i2c_bus(&rinfo->i2c[3], "crt2");
>> }
>> 
>> aty128fb has no i2c support.
>
> On rage128p I think there's only GPIO_MONID where there are 4 pins that can 
> have 2 i2c busses. I think they are DDC for CRT and flat-panel ports but not 
> sure about that. So what it's missing is i2c for hw monitoring which the 
> GPIO_MONID may be used for on radeon according to the above.
>
> The r128 xorg driver complains about missing support in VESA VBE as quoted 
> above and unloads itself. I think it may work if that support is implemented 
> in vgabios which I hope you can look at as I have no experience with vgabios.
>
> For radeon the DDC may need to be added on the GPIO_VGA_PORT as well based on 
> the above code excerpt but I haven't checked that.

I was thinking about something like the patch below but it does not seem 
to change anything for me. Does it work for you with radeonfb? Maybe it 
does not get to reading the EDID as it's missing something before that?

Regards,
BALATON Zoltan

-- >8 --

diff --git a/hw/display/ati.c b/hw/display/ati.c
index 260cd803d8..e2efc6f222 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -269,6 +269,9 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, 
unsigned int size)
 case DAC_CNTL:
 val = s->regs.dac_cntl;
 break;
+case GPIO_VGA_DDC:
+val = s->regs.gpio_vga_ddc;
+break;
 case GPIO_MONID:
 val = s->regs.gpio_monid;
 break;
@@ -505,7 +508,24 @@ static void ati_mm_write(void *opaque, hwaddr addr,
 s->regs.dac_cntl = data & 0xe3ff;
 s->vga.dac_8bit = !!(data & DAC_8BIT_EN);
 break;
+case GPIO_VGA_DDC:
+if (s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF) {
+break;
+}
+s->regs.gpio_vga_ddc = data & 0xf000f;
+if (data & BIT(17)) {
+s->regs.gpio_monid |= !!(data & BIT(1)) << 9;
+bitbang_i2c_set(s->bbi2c, BITBANG_I2C_SCL, (data & BIT(1)) != 0);
+}
+if (data & BIT(16)) {
+s->regs.gpio_monid |= bitbang_i2c_set(s->bbi2c, BITBANG_I2C_SDA,
+  data & BIT(0)) << 8;
+}
+break;
 case GPIO_MONID:
+if (s->dev_id != PCI_DEVICE_ID_ATI_RAGE128_PF) {
+break; /* FIXME What does Radeon have here? */
+}
 s->regs.gpio_monid = data & 0x0f0f000f;
 if (data & BIT(2) << 24) {
 s->regs.gpio_monid |= !!(data & BIT(2)) << 10;
diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h
index 8df00efd93..5cf1f3269e 100644
--- a/hw/display/ati_int.h
+++ b/hw/display/ati_int.h
@@ -37,6 +37,7 @@ typedef struct ATIVGARegs {
 uint32_t crtc_gen_cntl;
 uint32_t crtc_ext_cntl;
 uint32_t dac_cntl;
+uint32_t gpio_vga_ddc;
 uint32_t gpio_monid;
 uint32_t crtc_h_total_disp;
 uint32_t crtc_h_sync_strt_wid;
diff --git a/hw/display/ati_regs.h b/hw/display/ati_regs.h
index 923bfd33ce..90384c886e 100644
--- a/hw/display/ati_regs.h
+++ b/hw/display/ati_regs.h
@@ -37,6 +37,7 @@
 #define CRTC_GEN_CNTL   0x0050
 #define CRTC_EXT_CNTL   0x0054
 #define DAC_CNTL0x0058
+#define GPIO_VGA_DDC0x0060
 #define GPIO_MONID  0x0068
 #define I2C_CNTL_1  0x0094
 #define PALETTE_INDEX   0x0

Re: [Qemu-devel] [PATCH 3/7] configure: add CET support

[Richard Henderson]

On 3/13/19 5:40 AM, Paolo Bonzini wrote:
> +##
> +# detect CET support in the toolchain
> +
> +if test "$cet" != no; then
> +  write_c_skeleton;
> +  if ! compile_prog "-fcf-protection" "" ; then
> +if test "$cet" = yes; then
> +  feature_not_found "cet" 'CET is not supported by your toolchain'
> +fi
> +cet=no
> +  fi
> +fi
> +if test "$cet" = ""; then
> +  cet=yes
> +  QEMU_CFLAGS="-fcf-protection $QEMU_CFLAGS"
> +fi

Hmm.  The gcc for aarch64 names the similar feature -mbranch-protection.  I'm
rather annoyed that the i386 gcc folk appropriated a generic -f name without
actually making the feature generic at the same time.

Thankfully the aarch64 version does not include shadow stacks, and so is less
invasive into the normal abi -- ARM uses pointer authentication instead.


r~

[Qemu-devel] Data bus error with redeonfb on mips_fulong2e

[BALATON Zoltan]

Hello,

Trying to debug the Linux kernel oops with radeonfb I've added some more 
debug logs and got this:


radeonfb_pci_register BEGIN
pci_host_data: pci_data_read: ati-vga: addr=04 val= len=4
PCI: Enabling device :00:06.0 ( -> 0003)
pci_host_data: pci_data_read: ati-vga: addr=04 val= len=4
pci_host_data: pci_data_write: ati-vga: addr=04 val=0003 len=4
pci_update_mappings: adding bar 0 to pci.mem @ 0x1400
pci_update_mappings: adding bar 1 to io @ 0x4000
pci_update_mappings: adding bar 2 to pci.mem @ 0x1505
pci_host_data: pci_data_read: ati-vga: addr=3c val= len=4
radeonfb_pci_register fb_base_phys=1400
radeonfb_pci_register mmio_base_phys=1505
radeonfb_pci_register request mem regions
radeonfb_pci_register map regions
radeonfb_pci_register mmio_base=90001505
Data bus error, epc == 80418e4c, ra == 80418e4c
(full oops dump in previous message but I couldn't find anything useful in 
there so omitted here)


I think the problem is around here (I have to guess because Linux prints 
(ptrval) instead of offsets in oops message as a security measure that I 
don't know how to turn off and the mips64 objdump that comes with my 
distro can't interleave source with disassembly so even if I have the 
offset can't find the source line by it so I had to go by debug printfs):


linux-4.15.10/drivers/video/fbdev/aty/radeon_base.c:
   2311 /* Set base addrs */
   2312 rinfo->fb_base_phys = pci_resource_start (pdev, 0);
   2313 rinfo->mmio_base_phys = pci_resource_start (pdev, 2);
   2314 pr_debug("radeonfb_pci_register fb_base_phys=%lx\n", 
rinfo->fb_base_phys);
   2315 pr_debug("radeonfb_pci_register mmio_base_phys=%lx\n", 
rinfo->mmio_base_phys);
   2316
   2317 /* request the mem regions */
   2318 pr_debug("radeonfb_pci_register request mem regions\n");
   2319 ret = pci_request_region(pdev, 0, "radeonfb framebuffer");
   2320 if (ret < 0) {
   2321 printk( KERN_ERR "radeonfb (%s): cannot request region 
0.\n",
   2322 pci_name(rinfo->pdev));
   2323 goto err_release_fb;
   2324 }
   2325
   2326 ret = pci_request_region(pdev, 2, "radeonfb mmio");
   2327 if (ret < 0) {
   2328 printk( KERN_ERR "radeonfb (%s): cannot request region 
2.\n",
   2329 pci_name(rinfo->pdev));
   2330 goto err_release_pci0;
   2331 }
   2332
   2333 /* map the regions */
   2334 pr_debug("radeonfb_pci_register map regions\n");
   2335 rinfo->mmio_base = ioremap(rinfo->mmio_base_phys, 
RADEON_REGSIZE);
   2336 if (!rinfo->mmio_base) {
   2337 printk(KERN_ERR "radeonfb (%s): cannot map MMIO\n",
   2338pci_name(rinfo->pdev));
   2339 ret = -EIO;
   2340 goto err_release_pci2;
   2341 }
   2342 pr_debug("radeonfb_pci_register mmio_base=%px\n", 
rinfo->mmio_base);
   2343
   2344 rinfo->fb_local_base = INREG(MC_FB_LOCATION) << 16;
   2345 pr_debug("radeonfb_pci_register fb_local_base=%lx\n", 
rinfo->fb_local_base);

So it fails in INREG on line 2344 which is in radeonfb.h:380

#define INREG(addr) readl((rinfo->mmio_base)+addr)

This reg read never reaches QEMU so either something's wrong with the 
mmio_base or readl can't handle it correctly. This seems to be something 
Linux MIPS specific that I have no idea about. Does this make sense to 
anyone?


Regards,
BALATON Zoltan

Re: [Qemu-devel] [PATCH 7/7] coroutine-x86: add CET shadow stack support

[Richard Henderson]

On 3/13/19 5:40 AM, Paolo Bonzini wrote:
> +static bool have_cet(void)
> +{
> +#if defined CONFIG_CET
> +uint64_t ssp;
> +asm ("xor %0, %0; rdsspq %0\n" : "=rm" (ssp));

The xor is incompatible with a memory output.
I don't think you really wanted that in the first place.
Just use "=r".

The rest is hard to review because of ARCH_X86_CET_ALLOC_SHSTK.
I'm surprised that a prctl actually allocates memory...


r~

Re: [Qemu-devel] [PATCH 6/7] linux-user: add IBT support to x86 safe-syscall.S

[Richard Henderson]

On 3/13/19 5:40 AM, Paolo Bonzini wrote:
> Because safe-syscall.S does not go through the C compiler, the
> .note.gnu.property note has to be added manually.  Safe syscalls do not
> involve any indirect branch or stack unwinding, so they are trivially
> safe for IBT or shadow stacks.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  linux-user/host/i386/safe-syscall.inc.S   | 19 +++
>  linux-user/host/x86_64/safe-syscall.inc.S | 19 +++
>  2 files changed, 38 insertions(+)

I suppose it's not worth trying to share these 19 lines...

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 5/7] tcg/i386: add support for IBT

[Richard Henderson]

On 3/13/19 5:40 AM, Paolo Bonzini wrote:
> Add endbr annotations before indirect branch targets.  This lets QEMU enable
> IBT even for TCG-enabled builds.

> @@ -3514,6 +3526,7 @@ static void tcg_target_qemu_prologue(TCGContext *s)
>CPU_TEMP_BUF_NLONGS * sizeof(long));
>  
>  /* Save all callee saved registers.  */
> +tcg_out_endbr(s);
>  for (i = 0; i < ARRAY_SIZE(tcg_target_callee_save_regs); i++) {
>  tcg_out_push(s, tcg_target_callee_save_regs[i]);
>  }

Nit: Keep the comment with the code to which it applies.

I'll note that there's one latent but currently unused indirect branch:

> } else {
> /* indirect jump method */
> tcg_out_modrm_offset(s, OPC_GRP5, EXT5_JMPN_Ev, -1,
>  (intptr_t)(s->tb_jmp_target_addr + a0));
> }
> set_jmp_reset_offset(s, a0);

We can probably just delete that condition, and assert it instead.

Otherwise,
Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] Maintainers, please tell us how to boot your machines!

[Peter.Chubb]

> "Markus" == Markus Armbruster  writes:

Markus> = hw/arm/kzm.c = Peter Chubb 
Markus> (odd fixer:i.MX31 (kzm)) Peter Maydell
Markus>  (odd fixer:i.MX31 (kzm))
Markus> qemu-...@nongnu.org (open list:i.MX31 (kzm))

qemu-system-arm -m kzm -nographic -kernel zImage -initrd rootfs.cpio.gz

Kernel and rootfs from buildroot; but any armhf kernel and rootfs that
supports the imx_v6_v7 kernel defconfig should work.

Peter C
-- 
Dr Peter Chubb Tel: +61 2 9490 5852  http://ts.data61.csiro.au/
Trustworthy Systems Group Data61, CSIRO (formerly NICTA)

Re: [Qemu-devel] [PATCH 4/7] tcg: add tcg_out_start

[Richard Henderson]

On 3/13/19 5:40 AM, Paolo Bonzini wrote:
> This function is called at the beginning of any translation block.  We will
> use it to emit ENDBR32 or ENDBR64 annotations for x86 CET.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  tcg/aarch64/tcg-target.inc.c | 4 
>  tcg/arm/tcg-target.inc.c | 4 
>  tcg/i386/tcg-target.inc.c| 4 
>  tcg/mips/tcg-target.inc.c| 4 
>  tcg/ppc/tcg-target.inc.c | 4 
>  tcg/riscv/tcg-target.inc.c   | 4 
>  tcg/s390/tcg-target.inc.c| 4 
>  tcg/sparc/tcg-target.inc.c   | 4 
>  tcg/tcg.c| 2 ++
>  tcg/tci/tcg-target.inc.c | 4 
>  10 files changed, 38 insertions(+)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PULL 0/2] Ui 20190313 patches

[Samuel Thibault]

Peter Maydell, le mer. 13 mars 2019 22:20:03 +, a ecrit:
> On Wed, 13 Mar 2019 at 07:41, Gerd Hoffmann  wrote:
> >
> > The following changes since commit 377b155bde451d5ac545fbdcdfbf6ca17a4228f5:
> >
> >   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into 
> > staging (2019-03-11 18:26:37 +)
> >
> > are available in the Git repository at:
> >
> >   git://git.kraxel.org/qemu tags/ui-20190313-pull-request
> >
> > for you to fetch changes up to 2f8b7cd587558944532f587abb5203ce54badba9:
> >
> >   curses: add option to specify VGA font encoding (2019-03-13 08:29:06 
> > +0100)
> >
> > 
> > ui: better unicode support for curses, v2.
> >
> > 
> 
> Applied, thanks.
> 
> Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0
> for any user-visible changes.

Done so, thanks for the merge!

Samuel

Re: [Qemu-devel] [PULL 0/2] Ui 20190313 patches

[Peter Maydell]

On Wed, 13 Mar 2019 at 07:41, Gerd Hoffmann  wrote:
>
> The following changes since commit 377b155bde451d5ac545fbdcdfbf6ca17a4228f5:
>
>   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into 
> staging (2019-03-11 18:26:37 +)
>
> are available in the Git repository at:
>
>   git://git.kraxel.org/qemu tags/ui-20190313-pull-request
>
> for you to fetch changes up to 2f8b7cd587558944532f587abb5203ce54badba9:
>
>   curses: add option to specify VGA font encoding (2019-03-13 08:29:06 +0100)
>
> 
> ui: better unicode support for curses, v2.
>
> 

Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0
for any user-visible changes.

-- PMM

Re: [Qemu-devel] Maintainers, please tell us how to boot your machines!

[Alistair Francis]

On Tue, Mar 12, 2019 at 11:22 AM Markus Armbruster  wrote:
>
> Dear board code maintainers,

...

>
>
> Machines with at least one maintainer:
>

...

>
> = hw/arm/netduino2.c =
> Alistair Francis  (maintainer:Netduino 2)
> Peter Maydell  (maintainer:Netduino 2)
> qemu-...@nongnu.org (open list:ARM)

No Linux support here (at least that I ever tried), just baremetal applications.

There are steps on booting here:
https://github.com/alistair23/qemu/wiki/Getting-Started

The QEMU command line ends up being:
./arm-softmmu/qemu-system-arm -M netduino2 -nographic -kernel main.elf
-chardev stdio,mux=on,id=terminal -serial /dev/null -serial
chardev:terminal -serial /dev/null -serial chardev:terminal -monitor
chardev:terminal

The images are avalible here (in source):
https://github.com/alistair23/CSSE3010-QEMU-Examples

I can look at uploading binaries somewhere if required.

>

...

>
> = hw/arm/xilinx_zynq.c =
> "Edgar E. Iglesias"  (maintainer:Xilinx Zynq)
> Alistair Francis  (maintainer:Xilinx Zynq)
> Peter Maydell  (maintainer:Xilinx Zynq)
> qemu-...@nongnu.org (open list:Xilinx Zynq)
>
> = hw/arm/xlnx-versal-virt.c =
> Alistair Francis  (maintainer:Xilinx ZynqMP)
> "Edgar E. Iglesias"  (maintainer:Xilinx ZynqMP)
> Peter Maydell  (maintainer:Xilinx ZynqMP)
> qemu-...@nongnu.org (open list:Xilinx ZynqMP)
>
> = hw/arm/xlnx-zcu102.c =
> Alistair Francis  (maintainer:Xilinx ZynqMP)
> "Edgar E. Iglesias"  (maintainer:Xilinx ZynqMP)
> Peter Maydell  (maintainer:Xilinx ZynqMP)
> qemu-...@nongnu.org (open list:Xilinx ZynqMP)

...

>
> = hw/microblaze/xlnx-zynqmp-pmu.c =
> Alistair Francis  (maintainer:Xilinx ZynqMP)
> "Edgar E. Iglesias"  (maintainer:Xilinx ZynqMP)
> Peter Maydell  (maintainer:Xilinx ZynqMP)
> qemu-...@nongnu.org (open list:Xilinx ZynqMP)

Edgar has these covered.

>
...
>
> = hw/riscv/sifive_e.c =
> Palmer Dabbelt  (supporter:RISC-V)
> Alistair Francis  (supporter:RISC-V)
> Sagar Karandikar  (supporter:RISC-V)
> Bastian Koppelmann  (supporter:RISC-V)
> qemu-ri...@nongnu.org (open list:RISC-V)
>
> = hw/riscv/sifive_u.c =
> Palmer Dabbelt  (supporter:RISC-V)
> Alistair Francis  (supporter:RISC-V)
> Sagar Karandikar  (supporter:RISC-V)
> Bastian Koppelmann  (supporter:RISC-V)
> qemu-ri...@nongnu.org (open list:RISC-V)
>
> = hw/riscv/spike.c =
> Palmer Dabbelt  (supporter:RISC-V)
> Alistair Francis  (supporter:RISC-V)
> Sagar Karandikar  (supporter:RISC-V)
> Bastian Koppelmann  (supporter:RISC-V)
> qemu-ri...@nongnu.org (open list:RISC-V)
>
> = hw/riscv/virt.c =
> Palmer Dabbelt  (supporter:RISC-V)
> Alistair Francis  (supporter:RISC-V)
> Sagar Karandikar  (supporter:RISC-V)
> Bastian Koppelmann  (supporter:RISC-V)
> qemu-ri...@nongnu.org (open list:RISC-V)
>

I use OpenEmbedded to build images and run Linux on these. The
meta-riscv layer is a good starting point for that:
https://github.com/riscv/meta-riscv

Fedora images also work and there is information on that here:
https://fedoraproject.org/wiki/Architectures/RISC-V/Installing

The Fedora command line ends up looking like this:

qemu-system-riscv64 \
-nographic \
-machine virt \
-smp 4 \
-m 2G \
-kernel bbl \
-object rng-random,filename=/dev/urandom,id=rng0 \
-device virtio-rng-device,rng=rng0 \
-append "console=ttyS0 ro root=/dev/vda1" \
-device virtio-blk-device,drive=hd0 \
-drive file=Fedora-Developer-Rawhide-.n.0-sda.raw,format=raw,id=hd0 \
-device virtio-net-device,netdev=usernet \
-netdev user,id=usernet,hostfwd=tcp::1-:22

and the Fedora images can be downloaded from: http://185.97.32.145/koji/builds

Alistair

Re: [Qemu-devel] [PATCH v2 1/4] slirp: relicense GPL files to BSD-3

[Samuel Thibault]

Hello,

Marc-André Lureau, le mer. 13 mars 2019 18:35:09 +0100, a ecrit:
> On Wed, Mar 13, 2019 at 6:32 PM Eric Blake  wrote:
> >
> > On 3/13/19 11:42 AM, Marc-André Lureau wrote:
> > > In order to make slirp a standalone project, the project must have a
> > > clear license, and be compatible with the GPL or LGPL.
> > >
> > > Since commit 2f5f89963186d42a7ded253bc6cf5b32abb45cec ("Remove the
> > > advertising clause from the slirp license"), slirp is BSD-3. But new
> > > files have been added under slirp/ with QEMU GPL license since then.
> > >
> > > The copyright holders have been asked to relicense files to BSD-3 and
> > > gave their permission:
> >
> > Reviewed-by: Eric Blake 
> >
> >
> > >
> > > Signed-off-by: Marc-André Lureau 
> > > ---
> > >  slirp/src/dhcpv6.h   | 31 +--
> > >  slirp/src/ncsi-pkt.h | 33 +
> > >  slirp/src/vmstate.h  | 42 +++---
> > >  slirp/src/dhcpv6.c   | 37 +++--
> > >  slirp/src/ncsi.c | 31 +--
> > >  slirp/src/vmstate.c  | 31 +--
> > >  6 files changed, 170 insertions(+), 35 deletions(-)
> >
> > Are we trying to split slirp off in time for 4.0, or are we at the point
> > where it remains embedded until 4.1?
> 
> I don't know what Samuel, Peter and others think about it.

I'd say since the source seems ready we can try for 4.0, but I let
general qemu maintainers consider what they prefer.

> If we don't have code change, but only move slirp/ to a submodule, is
> that acceptable during soft-freeze?
> 
> Fwiw, we are mostly ready to do that, we mostly need the repo creation
> now (https://gitlab.freedesktop.org/freedesktop/freedesktop/issues/119)

Re: [Qemu-devel] [RFC PATCH 3/3] hw/acpi: Extract build_mcfg

[Wei Yang]

On Wed, Mar 13, 2019 at 05:09:43PM +0100, Igor Mammedov wrote:
>On Wed, 13 Mar 2019 13:33:59 +
>Wei Yang  wrote:
>
>> On Wed, Mar 13, 2019 at 01:23:00PM +0100, Igor Mammedov wrote:
>> >On Wed, 13 Mar 2019 12:42:53 +0800
>> >Wei Yang  wrote:
>> >  
>> >> Now we have two identical build_mcfg function.
>> >> 
>> >> Extract them to aml-build.c.
>> >> 
>> >> Signed-off-by: Wei Yang 
>> >> ---
>> >>  hw/acpi/aml-build.c | 30 ++
>> >>  hw/arm/virt-acpi-build.c| 16 
>> >>  hw/i386/acpi-build.c| 31 +--
>> >>  include/hw/acpi/aml-build.h |  1 +
>> >>  4 files changed, 32 insertions(+), 46 deletions(-)
>> >> 
>> >> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>> >> index 555c24f21d..58d3b8f31d 100644
>> >> --- a/hw/acpi/aml-build.c
>> >> +++ b/hw/acpi/aml-build.c  
>> >
>> >I don't like polluting aml-build.c with PCI stuff,
>> >we have a lot of PCI related code that needs generalizing
>> >lets create a new file for that, something like
>> >hw/acpi/pci.c + include/hw/acpi/pci.h
>> >  
>> >> @@ -25,6 +25,7 @@
>> >>  #include "qemu/bswap.h"
>> >>  #include "qemu/bitops.h"
>> >>  #include "sysemu/numa.h"
>> >> +#include "hw/pci/pcie_host.h"
>> >>  
>> >>  static GArray *build_alloc_array(void)
>> >>  {
>> >> @@ -1870,3 +1871,32 @@ build_hdr:
>> >>  build_header(linker, tbl, (void *)(tbl->data + fadt_start),
>> >>   "FACP", tbl->len - fadt_start, f->rev, oem_id, 
>> >> oem_table_id);
>> >>  }
>> >> +
>> >> +void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo 
>> >> *info)
>> >> +{
>> >> +AcpiTableMcfg *mcfg;
>> >> +const char *sig;
>> >> +int len = sizeof(*mcfg) + sizeof(mcfg->allocation[0]);
>> >> +
>> >> +mcfg = acpi_data_push(table_data, len);
>> >> +mcfg->allocation[0].address = cpu_to_le64(info->mcfg_base);
>> >> +/* Only a single allocation so no need to play with segments */
>> >> +mcfg->allocation[0].pci_segment = cpu_to_le16(0);
>> >> +mcfg->allocation[0].start_bus_number = 0;
>> >> +mcfg->allocation[0].end_bus_number = PCIE_MMCFG_BUS(info->mcfg_size 
>> >> - 1);  
>> >  
>> >> +/*
>> >> + * MCFG is used for ECAM which can be enabled or disabled by guest.
>> >> + * To avoid table size changes (which create migration issues),
>> >> + * always create the table even if there are no allocations,
>> >> + * but set the signature to a reserved value in this case.
>> >> + * ACPI spec requires OSPMs to ignore such tables.
>> >> + */
>> >> +if (info->mcfg_base == PCIE_BASE_ADDR_UNMAPPED) {
>> >> +/* Reserved signature: ignored by OSPM */
>> >> +sig = "QEMU";
>> >> +} else {
>> >> +sig = "MCFG";
>> >> +}  
>> >I'd leave these hack at acpi-build.c, just push it up call chain.  
>> 
>> Assign sig in acpi-build.c and pass it to build_mcfg()?
>nope, see more below
>
> 
>> >More over we don't really need it since resizeable memory region was 
>> >introduced.
>> >
>> >So we need to keep table_blob size only for legacy usecase (pre resizable)
>> >and for that just padding table_blob on required size would be sufficient,
>> >there is no need to create dummy QEMU table.
>> >As for newer machines (since resizeable memory region) we don't need to
>> >do even that i.e. just skip table generation altogether if guest disabled 
>> >it.
>> >  
>> 
>> I am lost at this place.
>> 
>> sig is a part of ACPI table header, you mean the sig is not necessary to
>> be set in ACPI table header?
>> 
>> "skip table generation" means remove build_header() in build_mcfg()?
>I mean do not call build_mcfg() at all when you don't have to.
>
>And when you need to keep table_blob the same size (for old machines)
>using acpi_data_push() to reserve space instead of build_mcfg(sig="QEMU")
>might just work as well. it's still hack but it can live in x86 specific
>acpi_build() keeping build_mcfg() generic.
>

Seems got your idea.

>As for defining what to use as criteria to decide when we need to keep
>table_blob size the same, I don't remember history of it, so I'd suggest
>to look at commit a1666142, study history of acpi_ram_update() and
>legacy_acpi_table_size to figure out since which machine type one doesn't
>have to keep table_blob size the same.
>

OK, let me study the history first.

BTW, the legacy here is hardware specification level or qemu software
design level?

-- 
Wei Yang
Help you, Help me

[Qemu-devel] [PULL 2/2] configure: remove slirp submodule support that doesn't exist yet

[Samuel Thibault]

From: Daniel P. Berrangé 

The slirp code is not yet split off into a separate repository, so
configuring QEMU to use slirp as a submodule is premature. This
causes the non-existant "slirp" to be requested from git when syncing
submodules. This in turn appears to be cause of non-deterministic
failures some developers are seeing with QEMU's submodule sync process.

Signed-off-by: Daniel P. Berrangé 
Message-Id: <20190313173157.30504-1-berra...@redhat.com>
Reviewed-by: Marc-André Lureau 
Signed-off-by: Samuel Thibault 
---
 configure | 11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/configure b/configure
index cab830a4c9..355eebfc5d 100755
--- a/configure
+++ b/configure
@@ -1109,8 +1109,6 @@ for opt do
   ;;
   --disable-slirp) slirp="no"
   ;;
-  --enable-slirp=git) slirp="git"
-  ;;
   --enable-slirp=system) slirp="system"
   ;;
   --disable-vde) vde="no"
@@ -5780,8 +5778,6 @@ case "$slirp" in
   "" | yes)
 if $pkg_config slirp; then
   slirp=system
-elif test -e "${source_path}/.git" && test $git_update = 'yes' ; then
-  slirp=git
 elif test -e "${source_path}/slirp/Makefile" ; then
   slirp=internal
 elif test -z "$slirp" ; then
@@ -5799,10 +5795,7 @@ case "$slirp" in
 esac
 
 case "$slirp" in
-  git | internal)
-if test "$slirp" = git; then
-  git_submodules="${git_submodules} slirp"
-fi
+  internal)
 mkdir -p slirp
 slirp_cflags="-I\$(SRC_PATH)/slirp/src -I\$(BUILD_DIR)/slirp/src"
 slirp_libs="-L\$(BUILD_DIR)/slirp -lslirp"
@@ -6464,7 +6457,7 @@ if test "$slirp" != "no"; then
   echo "SLIRP_CFLAGS=$slirp_cflags" >> $config_host_mak
   echo "SLIRP_LIBS=$slirp_libs" >> $config_host_mak
 fi
-if [ "$slirp" = "git" -o "$slirp" = "internal" ]; then
+if [ "$slirp" = "internal" ]; then
 echo "config-host.h: subdir-slirp" >> $config_host_mak
 fi
 if test "$vde" = "yes" ; then
-- 
2.20.1

[Qemu-devel] [PULL 1/1] configure: remove slirp submodule support that doesn't exist yet

[Samuel Thibault]

From: Daniel P. Berrangé 

The slirp code is not yet split off into a separate repository, so
configuring QEMU to use slirp as a submodule is premature. This
causes the non-existant "slirp" to be requested from git when syncing
submodules. This in turn appears to be cause of non-deterministic
failures some developers are seeing with QEMU's submodule sync process.

Signed-off-by: Daniel P. Berrangé 
Message-Id: <20190313173157.30504-1-berra...@redhat.com>
Reviewed-by: Marc-André Lureau 
Signed-off-by: Samuel Thibault 
---
 configure | 11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/configure b/configure
index cab830a4c9..355eebfc5d 100755
--- a/configure
+++ b/configure
@@ -1109,8 +1109,6 @@ for opt do
   ;;
   --disable-slirp) slirp="no"
   ;;
-  --enable-slirp=git) slirp="git"
-  ;;
   --enable-slirp=system) slirp="system"
   ;;
   --disable-vde) vde="no"
@@ -5780,8 +5778,6 @@ case "$slirp" in
   "" | yes)
 if $pkg_config slirp; then
   slirp=system
-elif test -e "${source_path}/.git" && test $git_update = 'yes' ; then
-  slirp=git
 elif test -e "${source_path}/slirp/Makefile" ; then
   slirp=internal
 elif test -z "$slirp" ; then
@@ -5799,10 +5795,7 @@ case "$slirp" in
 esac
 
 case "$slirp" in
-  git | internal)
-if test "$slirp" = git; then
-  git_submodules="${git_submodules} slirp"
-fi
+  internal)
 mkdir -p slirp
 slirp_cflags="-I\$(SRC_PATH)/slirp/src -I\$(BUILD_DIR)/slirp/src"
 slirp_libs="-L\$(BUILD_DIR)/slirp -lslirp"
@@ -6464,7 +6457,7 @@ if test "$slirp" != "no"; then
   echo "SLIRP_CFLAGS=$slirp_cflags" >> $config_host_mak
   echo "SLIRP_LIBS=$slirp_libs" >> $config_host_mak
 fi
-if [ "$slirp" = "git" -o "$slirp" = "internal" ]; then
+if [ "$slirp" = "internal" ]; then
 echo "config-host.h: subdir-slirp" >> $config_host_mak
 fi
 if test "$vde" = "yes" ; then
-- 
2.20.1

[Qemu-devel] [PULL 1/2] slirp: remove empty state.h

[Samuel Thibault]

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
Message-Id: <20190313173949.2369-1-marcandre.lur...@redhat.com>
Reviewed-by: Eric Blake 
Signed-off-by: Samuel Thibault 
---
 slirp/src/state.c | 1 -
 slirp/src/state.h | 0
 2 files changed, 1 deletion(-)
 delete mode 100644 slirp/src/state.h

diff --git a/slirp/src/state.c b/slirp/src/state.c
index f5dd80cdc8..c3e3f0b671 100644
--- a/slirp/src/state.c
+++ b/slirp/src/state.c
@@ -23,7 +23,6 @@
  */
 #include "slirp.h"
 #include "vmstate.h"
-#include "state.h"
 #include "stream.h"
 
 static int slirp_tcp_post_load(void *opaque, int version)
diff --git a/slirp/src/state.h b/slirp/src/state.h
deleted file mode 100644
index e69de29bb2..00
-- 
2.20.1

Re: [Qemu-devel] [PATCH] slirp: remove empty state.h

[Samuel Thibault]

Eric Blake, le mer. 13 mars 2019 13:21:23 -0500, a ecrit:
> On 3/13/19 12:39 PM, Marc-André Lureau wrote:
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  slirp/src/state.h | 0
> >  slirp/src/state.c | 1 -
> >  2 files changed, 1 deletion(-)
> >  delete mode 100644 slirp/src/state.h
> 
> Made empty in commit d890344, before moving to its current location in
> c2d63650.
> 
> Reviewed-by: Eric Blake 

Applied and sent pull request, thanks!

[Qemu-devel] [PULL 0/2] Slirp updates

[Samuel Thibault]

The following changes since commit cd82b1e170019c4b722ed53116ee9346315d7791:

  slirp: remove empty state.h (2019-03-13 22:12:23 +0100)

are available in the Git repository at:

  https://people.debian.org/~sthibault/qemu.git tags/samuel-thibault

for you to fetch changes up to 1f773d9dd3187281b29ceae38541fa2d945b1d0f:

  configure: remove slirp submodule support that doesn't exist yet (2019-03-13 
22:15:30 +0100)


Slirp updates

Daniel P. Berrangé (1):
  configure: remove slirp submodule support that doesn't exist yet

Marc-André Lureau (1):
  slirp: remove empty state.h


Daniel P. Berrangé (1):
  configure: remove slirp submodule support that doesn't exist yet

 configure | 11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

[Qemu-devel] [PATCH v2 01/12] roms: lift "edk2-funcs.sh" from "tests/uefi-test-tools/build.sh"

[Laszlo Ersek]

Extract the dense logic for architecture and toolchain massaging from
"tests/uefi-test-tools/build.sh", to a set of small functions. We'll reuse
these functions for building full platform firmware images.

Signed-off-by: Laszlo Ersek 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Reviewed-by: Michal Privoznik 
Reviewed-by: Michael S. Tsirkin 
---

Notes:
v2:

- Pick up Phil's R-b and T-b. (I hope that's okay, after the discussion
  about cross-building on Ubuntu and/or with Linaro toolchains!)

- drop comma after copyright year, in the new file [Eric]

- clarify in the leading comment of "edk2-funcs.sh" that the file
  requires bash [Phil, Eric]

- pick up Michal's and Michael's R-b's

 roms/edk2-funcs.sh | 240 
 tests/uefi-test-tools/build.sh |  97 +---
 2 files changed, 246 insertions(+), 91 deletions(-)

diff --git a/roms/edk2-funcs.sh b/roms/edk2-funcs.sh
new file mode 100644
index ..8930479dcb44
--- /dev/null
+++ b/roms/edk2-funcs.sh
@@ -0,0 +1,240 @@
+# Shell script that defines functions for determining some environmental
+# characteristics for the edk2 "build" utility.
+#
+# This script is meant to be sourced, in a bash environment.
+#
+# Copyright (C) 2019 Red Hat, Inc.
+#
+# This program and the accompanying materials are licensed and made available
+# under the terms and conditions of the BSD License that accompanies this
+# distribution. The full text of the license may be found at
+# .
+#
+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
+# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+
+# Verify whether the QEMU system emulation target is supported by the UEFI spec
+# and edk2. Print a message to the standard error, and return with nonzero
+# status, if verification fails.
+#
+# Parameters:
+#   $1: QEMU system emulation target
+qemu_edk2_verify_arch()
+{
+  local emulation_target="$1"
+  local program_name=$(basename -- "$0")
+
+  case "$emulation_target" in
+(arm|aarch64|i386|x86_64)
+  ;;
+(*)
+  printf '%s: unknown/unsupported QEMU system emulation target "%s"\n' \
+"$program_name" "$emulation_target" >&2
+  return 1
+  ;;
+  esac
+}
+
+
+# Translate the QEMU system emulation target to the edk2 architecture
+# identifier. Print the result to the standard output.
+#
+# Parameters:
+#   $1: QEMU system emulation target
+qemu_edk2_get_arch()
+{
+  local emulation_target="$1"
+
+  if ! qemu_edk2_verify_arch "$emulation_target"; then
+return 1
+  fi
+
+  case "$emulation_target" in
+(arm)
+  printf 'ARM\n'
+  ;;
+(aarch64)
+  printf 'AARCH64\n'
+  ;;
+(i386)
+  printf 'IA32\n'
+  ;;
+(x86_64)
+  printf 'X64\n'
+  ;;
+  esac
+}
+
+
+# Translate the QEMU system emulation target to the gcc cross-compilation
+# architecture identifier. Print the result to the standard output.
+#
+# Parameters:
+#   $1: QEMU system emulation target
+qemu_edk2_get_gcc_arch()
+{
+  local emulation_target="$1"
+
+  if ! qemu_edk2_verify_arch "$emulation_target"; then
+return 1
+  fi
+
+  case "$emulation_target" in
+(arm|aarch64|x86_64)
+  printf '%s\n' "$emulation_target"
+  ;;
+(i386)
+  printf 'i686\n'
+  ;;
+  esac
+}
+
+
+# Determine the gcc cross-compiler prefix (if any) for use with the edk2
+# toolchain. Print the result to the standard output.
+#
+# Parameters:
+#   $1: QEMU system emulation target
+qemu_edk2_get_cross_prefix()
+{
+  local emulation_target="$1"
+  local gcc_arch
+  local host_arch
+
+  if ! gcc_arch=$(qemu_edk2_get_gcc_arch "$emulation_target"); then
+return 1
+  fi
+
+  host_arch=$(uname -m)
+
+  if [ "$gcc_arch" == "$host_arch" ] ||
+ ( [ "$gcc_arch" == i686 ] && [ "$host_arch" == x86_64 ] ); then
+# no cross-compiler needed
+:
+  else
+printf '%s-linux-gnu-\n' "$gcc_arch"
+  fi
+}
+
+
+# Determine the edk2 toolchain tag for the QEMU system emulation target. Print
+# the result to the standard output. Print a message to the standard error, and
+# return with nonzero status, if the (conditional) gcc version check fails.
+#
+# Parameters:
+#   $1: QEMU system emulation target
+qemu_edk2_get_toolchain()
+{
+  local emulation_target="$1"
+  local program_name=$(basename -- "$0")
+  local cross_prefix
+  local gcc_version
+
+  if ! qemu_edk2_verify_arch "$emulation_target"; then
+return 1
+  fi
+
+  case "$emulation_target" in
+(arm|aarch64)
+  printf 'GCC5\n'
+  ;;
+
+(i386|x86_64)
+  if ! cross_prefix=$(qemu_edk2_get_cross_prefix "$emulation_target"); then
+return 1
+  fi
+
+  gcc_version=$("${cross_prefix}gcc" -v 2>&1 | tail -1 | awk '{print $3}')
+  # Run "git-blame" on "OvmfPkg/build.sh" in edk2 for more information on
+  # the mapping below.
+  case "$gcc_version" in
+([1-3].*|4.[0

Re: [Qemu-devel] [PATCH v5 00/15] s390: vfio-ccw dasd ipl support

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/1552494682-16788-1-git-send-email-jjhe...@linux.ibm.com/



Hi,

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

Type: series
Message-id: 1552494682-16788-1-git-send-email-jjhe...@linux.ibm.com
Subject: [Qemu-devel] [PATCH v5 00/15] s390: vfio-ccw dasd ipl support

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
   36fe770966..523a2a42c3  master -> master
 t [tag update]
patchew/1552494682-16788-1-git-send-email-jjhe...@linux.ibm.com -> 
patchew/1552494682-16788-1-git-send-email-jjhe...@linux.ibm.com
Switched to a new branch 'test'
ea5b2d5dbb s390-bios: Support booting from real dasd device
9af807232e s390-bios: Add channel command codes/structs needed for dasd-ipl
07592e8719 s390-bios: Use control unit type to determine boot method
6951c3ac21 s390-bios: Refactor virtio to run channel programs via cio
019a22ce2b s390-bios: cio error handling
233dcaa131 s390-bios: Support for running format-0/1 channel programs
071334b15c s390-bios: ptr2u32 and u32toptr
8fda549cc9 s390-bios: Map low core memory
9000c16854 s390-bios: Decouple channel i/o logic from virtio
d8c254d212 s390-bios: Clean up cio.h
52f48bcf08 s390-bios: Factor finding boot device out of virtio code path
141b34b683 s390-bios: Extend find_dev() for non-virtio devices
ca9d1e9638 s390-bios: decouple common boot logic from virtio
e528af5cbb s390-bios: decouple cio setup from virtio
edfff47b44 s390 vfio-ccw: Add bootindex property and IPLB data

=== OUTPUT BEGIN ===
1/15 Checking commit edfff47b4417 (s390 vfio-ccw: Add bootindex property and 
IPLB data)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#221: 
new file mode 100644

total: 0 errors, 1 warnings, 199 lines checked

Patch 1/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/15 Checking commit e528af5cbbfd (s390-bios: decouple cio setup from virtio)
3/15 Checking commit ca9d1e9638d2 (s390-bios: decouple common boot logic from 
virtio)
ERROR: externs should be avoided in .c files
#31: FILE: pc-bios/s390-ccw/main.c:19:
+IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));

total: 1 errors, 0 warnings, 65 lines checked

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

4/15 Checking commit 141b34b683e5 (s390-bios: Extend find_dev() for non-virtio 
devices)
5/15 Checking commit 52f48bcf08d6 (s390-bios: Factor finding boot device out of 
virtio code path)
6/15 Checking commit d8c254d212f1 (s390-bios: Clean up cio.h)
7/15 Checking commit 9000c16854b7 (s390-bios: Decouple channel i/o logic from 
virtio)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#29: 
new file mode 100644

total: 0 errors, 1 warnings, 123 lines checked

Patch 7/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
8/15 Checking commit 8fda549cc9fa (s390-bios: Map low core memory)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#36: 
new file mode 100644

total: 0 errors, 1 warnings, 104 lines checked

Patch 8/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/15 Checking commit 071334b15c8c (s390-bios: ptr2u32 and u32toptr)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100644

total: 0 errors, 1 warnings, 31 lines checked

Patch 9/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/15 Checking commit 233dcaa13117 (s390-bios: Support for running format-0/1 
channel programs)
ERROR: trailing whitespace
#138: FILE: pc-bios/s390-ccw/cio.c:145:
+ * active (generating i/o interrupts). $

total: 1 errors, 0 warnings, 375 lines checked

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

11/15 Checking commit 019a22ce2be3 (s390-bios: cio error handling)
12/15 Checking commit 6951c3ac21d2 (s390-bios: Refactor virtio to run channel 
programs via cio)
WARNING: line over 80 characters
#104: FILE: pc-bios/s390-ccw/virtio.c:298:
+run_ccw(vdev, CCW_CMD_READ_VQ_CONF, &config, sizeof(config), 
false) == 0,

WARNING: line over 80 characters
#117: FILE: pc-bios/s390-ccw/virtio.c:308:
+run_ccw(vdev, CCW_CMD_WRITE_STATUS, &

[Qemu-devel] [PULL 0/1] slirp update

[Samuel Thibault]

The following changes since commit cd82b1e170019c4b722ed53116ee9346315d7791:

  slirp: remove empty state.h (2019-03-13 22:12:23 +0100)

are available in the Git repository at:

  https://people.debian.org/~sthibault/qemu.git tags/samuel-thibault

for you to fetch changes up to cd82b1e170019c4b722ed53116ee9346315d7791:

  slirp: remove empty state.h (2019-03-13 22:12:23 +0100)


Slirp updates

Marc-André Lureau (1):
  slirp: remove empty state.h

Re: [Qemu-devel] [PULL 0/1] slirp update

[Samuel Thibault]

Samuel Thibault, le mer. 13 mars 2019 22:14:42 +0100, a ecrit:
> The following changes since commit cd82b1e170019c4b722ed53116ee9346315d7791:
> 
>   slirp: remove empty state.h (2019-03-13 22:12:23 +0100)

Oops, sorry, there was another patch to commit, you can ignore this pull
request.

Samuel

[Qemu-devel] [PATCH v2 03/12] tests/uefi-test-tools/build.sh: work around TianoCore#1607

[Laszlo Ersek]

The edk2-stabe201903 release introduced Python3 support to edk2's
BaseTools; however the Python3 enablement breaks in a corner case (which
is nevertheless supported by the edk2 community), namely the in-module
parallelization that we utilize.

This is tracked under
. For now, work
around the issue (in advance) by forcing Python2. (The workaround is a
no-op before we move to edk2-stabe201903 in the roms/edk2 submodule.)

Signed-off-by: Laszlo Ersek 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Reviewed-by: Michal Privoznik 
Reviewed-by: Michael S. Tsirkin 
---

Notes:
v2:

- pick up Phil's R-b / T-b

- pick up Michal's and Michael's R-b's

 tests/uefi-test-tools/build.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/uefi-test-tools/build.sh b/tests/uefi-test-tools/build.sh
index e2b52c855c39..8aa7935c43bb 100755
--- a/tests/uefi-test-tools/build.sh
+++ b/tests/uefi-test-tools/build.sh
@@ -29,6 +29,9 @@ export PACKAGES_PATH=$(realpath -- "$edk2_dir")
 export WORKSPACE=$PWD
 mkdir -p Conf
 
+# Work around .
+export PYTHON_COMMAND=python2
+
 # Source "edksetup.sh" carefully.
 set +e +u +C
 source "$PACKAGES_PATH/edksetup.sh"
-- 
2.19.1.3.g30247aa5d201

[Qemu-devel] [PATCH v2 02/12] roms/edk2-funcs.sh: require gcc-4.8+ for building i386 and x86_64

[Laszlo Ersek]

Adapt the qemu_edk2_get_toolchain() function in "roms/edk2-funcs.sh" in
advance to edk2 commit 8d7cdfae8cb8 ("OvmfPkg: require GCC48 or later",
2019-01-08), which is part of the "edk2-stable201903" tag.

Signed-off-by: Laszlo Ersek 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Reviewed-by: Michal Privoznik 
Reviewed-by: Michael S. Tsirkin 
---

Notes:
v2:

- pick up Phil's R-b / T-b

- pick up Michal's and Michael's R-b's

 roms/edk2-funcs.sh | 14 +-
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/roms/edk2-funcs.sh b/roms/edk2-funcs.sh
index 8930479dcb44..d1cb1e4a111e 100644
--- a/roms/edk2-funcs.sh
+++ b/roms/edk2-funcs.sh
@@ -149,23 +149,11 @@ qemu_edk2_get_toolchain()
   # Run "git-blame" on "OvmfPkg/build.sh" in edk2 for more information on
   # the mapping below.
   case "$gcc_version" in
-([1-3].*|4.[0-3].*)
+([1-3].*|4.[0-7].*)
   printf '%s: unsupported gcc version "%s"\n' \
 "$program_name" "$gcc_version" >&2
   return 1
   ;;
-(4.4.*)
-  printf 'GCC44\n'
-  ;;
-(4.5.*)
-  printf 'GCC45\n'
-  ;;
-(4.6.*)
-  printf 'GCC46\n'
-  ;;
-(4.7.*)
-  printf 'GCC47\n'
-  ;;
 (4.8.*)
   printf 'GCC48\n'
   ;;
-- 
2.19.1.3.g30247aa5d201

[Qemu-devel] [PULL 1/1] slirp: remove empty state.h

[Samuel Thibault]

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
Message-Id: <20190313173949.2369-1-marcandre.lur...@redhat.com>
Reviewed-by: Eric Blake 
Signed-off-by: Samuel Thibault 
---
 slirp/src/state.c | 1 -
 slirp/src/state.h | 0
 2 files changed, 1 deletion(-)
 delete mode 100644 slirp/src/state.h

diff --git a/slirp/src/state.c b/slirp/src/state.c
index f5dd80cdc8..c3e3f0b671 100644
--- a/slirp/src/state.c
+++ b/slirp/src/state.c
@@ -23,7 +23,6 @@
  */
 #include "slirp.h"
 #include "vmstate.h"
-#include "state.h"
 #include "stream.h"
 
 static int slirp_tcp_post_load(void *opaque, int version)
diff --git a/slirp/src/state.h b/slirp/src/state.h
deleted file mode 100644
index e69de29bb2..00
-- 
2.20.1

[Qemu-devel] [PATCH v2 07/12] roms: build edk2 firmware binaries and variable store templates

[Laszlo Ersek]

Add the "efi" target to "Makefile".

Introduce "Makefile.edk2" for building and cleaning the firmware images
and varstore templates.

Collect the common bits from the recipes in the helper script
"edk2-build.sh".

Signed-off-by: Laszlo Ersek 
---

Notes:
v2:

- drop comma after copyright year, in both new files [Eric]

- define the SHELL macro as /bin/bash near the top of "Makefile.edk2",
  so that various bashisms (e.g. the "source" built-in, and the ==
  operator of "[") work even if /bin/sh isn't bash [Phil, Eric]

- rework "Makefile.edk2" to produce xz-compressed flash device files
  [Dan, Michael, Phil]:

  - add implicit rule for compression,

  - mark uncompressed FD files as intermediate,

  - factor out the "flashdevs" macro for sharing between the "all" and
".INTERMEDIATE" targets

- due to said rework above, do not pick up Phil's R-b / T-b, and
  Michal's and Michael's R-b's

 roms/Makefile  |   5 +
 roms/Makefile.edk2 | 148 
 roms/edk2-build.sh |  55 
 3 files changed, 208 insertions(+)

diff --git a/roms/Makefile b/roms/Makefile
index 2e83ececa25a..054b432834ba 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -61,6 +61,7 @@ default:
@echo "  skiboot-- update skiboot.lid"
@echo "  u-boot.e500-- update u-boot.e500"
@echo "  u-boot.sam460  -- update u-boot.sam460"
+   @echo "  efi-- update UEFI (edk2) platform firmware"
 
 bios: build-seabios-config-seabios-128k build-seabios-config-seabios-256k
cp seabios/builds/seabios-128k/bios.bin ../pc-bios/bios.bin
@@ -143,6 +144,9 @@ skiboot:
$(MAKE) -C skiboot CROSS=$(powerpc64_cross_prefix)
cp skiboot/skiboot.lid ../pc-bios/skiboot.lid
 
+efi: edk2-basetools
+   $(MAKE) -f Makefile.edk2
+
 clean:
rm -rf seabios/.config seabios/out seabios/builds
$(MAKE) -C sgabios clean
@@ -153,3 +157,4 @@ clean:
rm -rf u-boot/build.e500
$(MAKE) -C u-boot-sam460ex distclean
$(MAKE) -C skiboot clean
+   $(MAKE) -f Makefile.edk2 clean
diff --git a/roms/Makefile.edk2 b/roms/Makefile.edk2
new file mode 100644
index ..e5c3036949c8
--- /dev/null
+++ b/roms/Makefile.edk2
@@ -0,0 +1,148 @@
+# Makefile for building firmware binaries and variable store templates for a
+# number of virtual platforms in edk2.
+#
+# Copyright (C) 2019 Red Hat, Inc.
+#
+# This program and the accompanying materials are licensed and made available
+# under the terms and conditions of the BSD License that accompanies this
+# distribution. The full text of the license may be found at
+# .
+#
+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
+# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+SHELL = /bin/bash
+
+toolchain = $(shell source ./edk2-funcs.sh && qemu_edk2_get_toolchain $(1))
+
+licenses := \
+   edk2/License.txt \
+   edk2/OvmfPkg/License.txt \
+   edk2/CryptoPkg/Library/OpensslLib/openssl/LICENSE
+
+# The "edk2-arm-vars.fd" varstore template is suitable for aarch64 as well.
+# Similarly, the "edk2-i386-vars.fd" varstore template is suitable for x86_64
+# as well, independently of "secure" too.
+flashdevs := \
+   aarch64-code \
+   arm-code \
+   i386-code \
+   i386-secure-code \
+   x86_64-code \
+   x86_64-secure-code \
+   \
+   arm-vars \
+   i386-vars
+
+all: $(foreach flashdev,$(flashdevs),../pc-bios/edk2-$(flashdev).fd.xz) \
+   ../pc-bios/edk2-licenses.txt
+
+../pc-bios/edk2-%.fd.xz: ../pc-bios/edk2-%.fd
+   xz -9 -e -c $< > $@
+
+# When the build completes, we need not keep the uncompressed flash device
+# files.
+.INTERMEDIATE: $(foreach flashdev,$(flashdevs),../pc-bios/edk2-$(flashdev).fd)
+
+submodules:
+   cd edk2 && git submodule update --init --force
+
+# See notes on the ".NOTPARALLEL" target and the "+" indicator in
+# "tests/uefi-test-tools/Makefile".
+.NOTPARALLEL:
+
+../pc-bios/edk2-aarch64-code.fd: submodules
+   +./edk2-build.sh \
+   aarch64 \
+   --arch=AARCH64 \
+   --platform=ArmVirtPkg/ArmVirtQemu.dsc \
+   -D NETWORK_IP6_ENABLE \
+   -D HTTP_BOOT_ENABLE
+   cp edk2/Build/ArmVirtQemu-AARCH64/DEBUG_$(call 
toolchain,aarch64)/FV/QEMU_EFI.fd \
+   $@
+   truncate --size=64M $@
+
+../pc-bios/edk2-arm-code.fd: submodules
+   +./edk2-build.sh \
+   arm \
+   --arch=ARM \
+   --platform=ArmVirtPkg/ArmVirtQemu.dsc \
+   -D NETWORK_IP6_ENABLE \
+   -D HTTP_BOOT_ENABLE
+   cp edk2/Build/ArmVirtQemu-ARM/DEBUG_$(call 
toolchain,arm)/FV/QEMU_EFI.fd \
+   $@
+   truncate --size=64M $@
+
+../pc-bios/edk2-i386-code.fd: submodules
+   +./edk2-build.sh \
+   i386 \
+   --arch=IA32 

[Qemu-devel] [PATCH v2 04/12] roms/edk2: advance to tag edk2-stable201903

[Laszlo Ersek]

Update the roms/edk2 submodule hash from edk2-stable201811 to
edk2-stable201903. The release notes are available at
.

$ git shortlog edk2-stable201811..edk2-stable201903

Achin Gupta (9):
  ArmPkg: Add PCDs needed for MM communication driver.
  ArmPkg/Drivers: Add EFI_MM_COMMUNICATION_PROTOCOL DXE driver.
  ArmPkg/Include: Add MM interface SVC return codes.
  ArmPkg/ArmMmuLib: Add MMU Library suitable for use in S-EL0.
  StandaloneMmPkg: Add missing dependency on PL011UartClockLib
  StandaloneMmPkg: Enforce alignment check for AArch64
  StandaloneMmPkg: Zero data structure explicitly
  StandaloneMmPkg: Replace dependency on ArmMmuLib
  StandaloneMmPkg: Update dependency on PeCoffExtraActionLib

Albecki, Mateusz (1):
  MdeModulePkg/SdMmcPciHcDxe Fix eMMC HS400 switch sequence

Alex James (2):
  StdLib/sys/termios: Define cc_t as unsigned
  StdLib/Environs: Avoid infinite recursion in _Exit

Antoine Coeur (5):
  ArmVirtPkg: Fix various typos
  CryptoPkg: Fix various typos
  CorebootPayloadPkg: Fix various typos
  CorebootModulePkg: Fix various typos
  BaseTools: Various typo

Ard Biesheuvel (116):
  MdePkg/BaseIoLibIntrinsicArmVirt ARM: avoid double word loads and stores
  ArmPkg/ArmGicDxe ARM: fix encoding for GICv3 interrupt acknowledge
  ArmPlatformPkg: clear frame pointer in startup code
  ArmVirtPkg/PrePi: clear frame pointer in startup code
  ArmPkg/ArmSmcPsciResetSystemLib: add missing call to ExitBootServices()
  ArmPkg: remove now unused BsdLib.h
  ArmPlatformPkg/NorFlashDxe: prepare for devicepath format change
  ArmPlatformPkg/NorFlashDxe: use one GUID plus index to identify flash 
banks
  ArmVirtPkg/FdtClientDxe: take DT node 'status' properties into account
  ArmVirtPkg/NorFlashQemuLib: discover NOR flash banks dynamically
  ArmPlatformPkg/NorFlashPlatformLib: remove unused Guid member from struct
  ArmPkg/ArmPkg.dsc: move ArmMmuStandaloneMmLib.inf to AARCH64 section
  EmbeddedPkg/TemplateSec: remove unused module
  EmbeddedPkg/PrePiHobLib: drop CreateHobList() from library
  ArmVirtPkg/FdtPciHostBridgeLib: map ECAM and I/O spaces in GCD memory map
  ArmVirtPkg/QemuVirtMemInfoLib: remove 1:1 mapping of top of PA range
  MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits
  ArmPkg/ArmLib: add support for reading the max physical address space size
  ArmVirtPkg/XenVirtMemInfoLib: refactor reading of the PA space size
  ArmPkg/ArmMmuLib: take the CPU supported maximum PA space into account
  ArmPkg/CpuPei: base GCD memory space size on CPU's PA range
  ArmPlatformPkg/PrePi: base GCD memory space size on CPU's PA range
  ArmVirtPkg/PrePi: base GCD memory space size on CPU's PA range
  BeagleBoardPkg/PrePi: base GCD memory space size on CPU's PA range
  ArmPlatformPkg/PlatformPei: drop unused PCD references
  EmbeddedPkg/PrePiLib: drop unused PCD reference
  ArmVirtPkg: drop PcdPrePiCpuMemorySize assignments from all platforms
  EmbeddedPkg/EmbeddedPkg.dec: drop PcdPrePiCpuMemorySize declarations
  ArmPkg/ArmMmuLib ARM: handle unmapped section in GetMemoryRegion()
  ArmPkg/ArmMmuLib ARM: handle unmapped sections when updating permissions
  ArmVirtPkg/NorFlashQemuLib: disregard our primary FV
  ArmVirtPkg/QemuVirtMemInfoLib: trim the MMIO region mapping
  BaseTools/CommonLib: avoid using 'native' word size in IP address handling
  BaseTools/CommonLib: use explicit 64-bit type in Strtoi()
  BaseTools/DevicePath: use explicit 64-bit number parsing routines
  BaseTools/CommonLib: add definition of MAX_UINT32
  BaseTools/DevicePath: use MAX_UINT32 as default device path max size
  BaseTools/CommonLib: get rid of 'native' type string parsing routines
  BaseTools/CommonLib: drop definition of MAX_UINTN
  BaseTools/CommonLib: drop the use of MAX_ADDRESS
  Revert "MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits"
  MdeModulePkg/FileExplorerLib: avoid packed struct for program data
  BaseTools/tools_def AARCH64 RELEASE: move GCC49/GGC5 to 4 KB alignment
  ArmVirtPkg/ArmVirtQemuKernel ARM: make some PCD settings apply to ARM
  ArmVirtPkg/PrePiUniCoreRelocatable CLANG38: work around build issues
  BaseTools/GenFw ARM: don't permit R_ARM_GOT_PREL relocations
  MdePkg/BaseMemoryLibOptDxe ARM: add missing function annotations
  BaseTools/tools_def ARM CLANG35: work around -mno-movt option name change
  ArmVirtPkg/PrePi ARM CLANG35: drop incompatible command line option
  ArmVirtPkg/ArmVirt.dsc.inc: define TcpIoLib resolution unconditionally
  ArmPkg: remove redundant _ARM_PLATFORM_FLAGS overrides
  EmbeddedPkg: remove GdbDebugAgent library
  BaseTools/tools_def ARM: emit PIC veneers
  ArmPkg/DefaultExceptionHandlerLib ARM: avoid endless loop in RELEA

Re: [Qemu-devel] [Qemu-ppc] [PATCH v5 1/2] spapr: helper functions to get valid host fields

[Maxiwell S. Garcia]

On Tue, Mar 12, 2019 at 11:52:24AM +0100, Greg Kurz wrote:

Hi Greg,

> On Mon, 11 Mar 2019 19:57:08 -0300
> "Maxiwell S. Garcia"  wrote:
> 
> > The pseries options 'host-serial' and 'host-model' accepts
> > 'none', 'passthrough', or  content. The helper
> > functions in this commit return a valid host field based on
> > user options.
> > 
> > Signed-off-by: Maxiwell S. Garcia 
> > ---
> >  hw/ppc/spapr.c | 58 ++
> >  include/hw/ppc/spapr.h |  3 +++
> >  2 files changed, 39 insertions(+), 22 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 9e01226e18..a3078f0261 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1202,6 +1202,34 @@ static void spapr_dt_chosen(sPAPRMachineState 
> > *spapr, void *fdt)
> >  g_free(bootlist);
> >  }
> >  
> > +char *spapr_get_valid_host_serial(sPAPRMachineState *spapr)
> > +{
> > +char *host_serial = NULL;
> > +if (spapr->host_serial && !g_str_equal(spapr->host_serial, "none")) {
> > +if (g_str_equal(spapr->host_serial, "passthrough")) {
> > +/* -M host-serial=passthrough */
> > +kvmppc_get_host_serial(&host_serial);
> > +} else {
> > +host_serial = g_strdup(spapr->host_serial);
> > +}
> > +}
> > +return host_serial;
> > +}
> > +
> > +char *spapr_get_valid_host_model(sPAPRMachineState *spapr)
> > +{
> > +char *host_model = NULL;
> > +if (spapr->host_model && !g_str_equal(spapr->host_model, "none")) {
> > +if (g_str_equal(spapr->host_model, "passthrough")) {
> > +/* -M host-model=passthrough */
> > +kvmppc_get_host_model(&host_model);
> > +} else {
> > +host_model = g_strdup(spapr->host_model);
> > +}
> > +}
> > +return host_model;
> > +}
> > +
> 
> These two functions only differ because of the host or serial wording.
> Maybe consolidate the boiler plate to a macro ?
> 

Do you suggest something like that?

#define SPAPR_GET_VALID_HOST_(FIELD, buf) \
if (spapr->FIELD && !g_str_equal(spapr->FIELD, "none")) { \
if (g_str_equal(spapr->FIELD, "passthrough")) {   \
kvmppc_get_##FIELD(&buf); \
} else {  \
buf = g_strdup(spapr->FIELD); \
} \
} \

#define SPAPR_GET_VALID_HOST_SERIAL(buf) SPAPR_GET_VALID_HOST_(host_serial, buf)
#define SPAPR_GET_VALID_HOST_MODEL(buf)  SPAPR_GET_VALID_HOST_(host_model, buf)

> >  static void spapr_dt_hypervisor(sPAPRMachineState *spapr, void *fdt)
> >  {
> >  /* The /hypervisor node isn't in PAPR - this is a hack to allow PR
> > @@ -1247,30 +1275,16 @@ static void *spapr_build_fdt(sPAPRMachineState 
> > *spapr)
> >   * Add info to guest to indentify which host is it being run on
> >   * and what is the uuid of the guest
> >   */
> > -if (spapr->host_model && !g_str_equal(spapr->host_model, "none")) {
> > -if (g_str_equal(spapr->host_model, "passthrough")) {
> > -/* -M host-model=passthrough */
> > -if (kvmppc_get_host_model(&buf)) {
> > -_FDT(fdt_setprop_string(fdt, 0, "host-model", buf));
> > -g_free(buf);
> > -}
> > -} else {
> > -/* -M host-model= */
> > -_FDT(fdt_setprop_string(fdt, 0, "host-model", 
> > spapr->host_model));
> > -}
> > +buf = spapr_get_valid_host_model(spapr);
> > +if (buf) {
> > +_FDT(fdt_setprop_string(fdt, 0, "host-model", buf));
> > +g_free(buf);
> >  }
> >  
> > -if (spapr->host_serial && !g_str_equal(spapr->host_serial, "none")) {
> > -if (g_str_equal(spapr->host_serial, "passthrough")) {
> > -/* -M host-serial=passthrough */
> > -if (kvmppc_get_host_serial(&buf)) {
> > -_FDT(fdt_setprop_string(fdt, 0, "host-serial", buf));
> > -g_free(buf);
> > -}
> > -} else {
> > -/* -M host-serial= */
> > -_FDT(fdt_setprop_string(fdt, 0, "host-serial", 
> > spapr->host_serial));
> > -}
> > +buf = spapr_get_valid_host_serial(spapr);
> > +if (buf) {
> > +_FDT(fdt_setprop_string(fdt, 0, "host-serial", buf));
> > +g_free(buf);
> >  }
> >  
> >  buf = qemu_uuid_unparse_strdup(&qemu_uuid);
> > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> > index 59073a7579..f7ea99dc69 100644
> > --- a/include/hw/ppc/spapr.h
> > +++ b/include/hw/ppc/spapr.h
> > @@ -842,6 +842,9 @@ int spapr_caps_post_migration(sPAPRMachineState *spapr);
> >  
> >  void spapr_check_pagesize(sPAPRMachineState *spapr, hwaddr pagesize,
> >Error **errp);
> > +
> > +char *spapr_get_valid_host_serial(sPAPRMachineState *

[Qemu-devel] [PATCH v2 00/12] bundle edk2 platform firmware with QEMU

[Laszlo Ersek]

Repo:   https://github.com/lersek/qemu.git
Branch: edk2_build_v2

Version 1, that is:
  [Qemu-devel] [PATCH 00/10] bundle edk2 platform firmware with QEMU

was posted at:
  https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg02846.html
  20190309004826.9027-1-lersek@redhat.com">http://mid.mail-archive.com/20190309004826.9027-1-lersek@redhat.com

Updates in v2 are noted on each patch individually, in the Notes
section.

I'm pasting a cumulative diffstat, and diff, between v1 and v2 below.
Note that a normal diffstat for this v2 series is at the bottom of the
cover letter, as usual.

>  .gitignore|   1 +
>  MAINTAINERS   |  12 
>  Makefile  |  22 +++---
>  configure |   1 +
>  pc-bios/README|   2 +-
>  pc-bios/edk2-aarch64-code.fd  | Bin 67108864 -> 0 bytes
>  pc-bios/edk2-aarch64-code.fd.xz   | Bin 0 -> 1146804 bytes
>  pc-bios/edk2-arm-code.fd  | Bin 67108864 -> 0 bytes
>  pc-bios/edk2-arm-code.fd.xz   | Bin 0 -> 1147852 bytes
>  pc-bios/edk2-arm-vars.fd  | Bin 67108864 -> 0 bytes
>  pc-bios/edk2-arm-vars.fd.xz   | Bin 0 -> 10008 bytes
>  pc-bios/edk2-i386-code.fd | Bin 3653632 -> 0 bytes
>  pc-bios/edk2-i386-code.fd.xz  | Bin 0 -> 1674764 bytes
>  pc-bios/edk2-i386-secure-code.fd  | Bin 3653632 -> 0 bytes
>  pc-bios/edk2-i386-secure-code.fd.xz   | Bin 0 -> 1870024 bytes
>  pc-bios/edk2-i386-vars.fd | Bin 540672 -> 0 bytes
>  pc-bios/edk2-i386-vars.fd.xz  | Bin 0 -> 320 bytes
>  pc-bios/edk2-x86_64-code.fd   | Bin 3653632 -> 0 bytes
>  pc-bios/edk2-x86_64-code.fd.xz| Bin 0 -> 1655276 bytes
>  pc-bios/edk2-x86_64-secure-code.fd| Bin 3653632 -> 0 bytes
>  pc-bios/edk2-x86_64-secure-code.fd.xz | Bin 0 -> 1889024 bytes
>  roms/Makefile.edk2|  32 +---
>  roms/edk2-build.sh|   2 +-
>  roms/edk2-funcs.sh|   4 +--
>  tests/Makefile.include|   2 +-
>  25 files changed, 57 insertions(+), 21 deletions(-)

> diff --git a/configure b/configure
> index cab830a4c920..a8e5c14899d6 100755
> --- a/configure
> +++ b/configure
> @@ -7770,6 +7770,7 @@ for bios_file in \
>  $source_path/pc-bios/*.img \
>  $source_path/pc-bios/openbios-* \
>  $source_path/pc-bios/u-boot.* \
> +$source_path/pc-bios/edk2-*.fd.xz \
>  $source_path/pc-bios/palcode-*
>  do
>  LINKS="$LINKS pc-bios/$(basename $bios_file)"
> diff --git a/Makefile b/Makefile
> index b98567ee0f83..4b325052005e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -296,6 +296,10 @@ ui/input-keymap-%.c: $(KEYCODEMAP_GEN) $(KEYCODEMAP_CSV) 
> $(SRC_PATH)/ui/Makefile
>  $(KEYCODEMAP_GEN): .git-submodule-status
>  $(KEYCODEMAP_CSV): .git-submodule-status
>
> +edk2-decompressed = $(basename $(wildcard pc-bios/edk2-*.fd.xz))
> +pc-bios/edk2-%.fd: pc-bios/edk2-%.fd.xz
> + $(call quiet-command,xz -d -c $< > $@,"UNXZ",$<)
> +
>  # Don't try to regenerate Makefile or configure
>  # We don't generate any of them
>  Makefile: ;
> @@ -444,6 +448,7 @@ $(SOFTMMU_SUBDIR_RULES): $(block-obj-y)
>  $(SOFTMMU_SUBDIR_RULES): $(crypto-obj-y)
>  $(SOFTMMU_SUBDIR_RULES): $(io-obj-y)
>  $(SOFTMMU_SUBDIR_RULES): config-all-devices.mak
> +$(SOFTMMU_SUBDIR_RULES): $(edk2-decompressed)
>
>  subdir-%:
>   $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $* V="$(V)" 
> TARGET_DIR="$*/" all,)
> @@ -632,6 +637,7 @@ clean:
>   ! -path ./roms/edk2/ArmPkg/Library/GccLto/liblto-arm.a \
>   ! -path ./roms/edk2/BaseTools/Source/Python/UPT/Dll/sqlite3.dll 
> \
>   -exec rm {} +
> + rm -f $(edk2-decompressed)
>   rm -f $(filter-out %.tlb,$(TOOLS)) $(HELPERS-y) qemu-ga TAGS cscope.* 
> *.pod *~ */*~
>   rm -f fsdev/*.pod scsi/*.pod
>   rm -f qemu-img-cmds.h
> @@ -722,10 +728,8 @@ spapr-rtas.bin slof.bin skiboot.lid \
>  palcode-clipper \
>  u-boot.e500 u-boot-sam460-20100605.bin \
>  qemu_vga.ndrv \
> -hppa-firmware.img \
> -edk2-aarch64-code.fd edk2-arm-code.fd edk2-i386-code.fd \
> -edk2-i386-secure-code.fd edk2-x86_64-code.fd edk2-x86_64-secure-code.fd \
> -edk2-arm-vars.fd edk2-i386-vars.fd edk2-licenses.txt
> +edk2-licenses.txt \
> +hppa-firmware.img
>
>  DESCS=50-edk2-i386-secure.json 50-edk2-x86_64-secure.json \
>  60-edk2-aarch64.json 60-edk2-arm.json 60-edk2-i386.json 60-edk2-x86_64.json
> @@ -792,7 +796,8 @@ endif
>
>  ICON_SIZES=16x16 24x24 32x32 48x48 64x64 128x128 256x256 512x512
>
> -install: all $(if $(BUILD_DOCS),install-doc) install-datadir 
> install-localstatedir
> +install: all $(if $(BUILD_DOCS),install-doc) install-datadir 
> install-localstatedir \
> + $(if $(INSTALL_BLOBS),$(edk2-decompressed))
>  ifneq ($(TOOLS),)
>   $(call install-prog,$(subst 
> qemu-ga,qemu-ga$(EXESUF),$(TOOLS)),$(DESTDIR)$(bindir))
>  endif
> @@ -814,6 +819,13 @@ ifneq ($(BLOBS

[Qemu-devel] [PATCH v2 09/12] pc-bios: document the edk2 firmware images; add firmware descriptors

[Laszlo Ersek]

Update the README file with information on the images added previously,
and provide firmware descriptor documents that conform to
"docs/interop/firmware.json".

Signed-off-by: Laszlo Ersek 
Reviewed-by: Michal Privoznik 
Reviewed-by: Michael S. Tsirkin 
---

Notes:
v2:

- refer to "edk2-*.fd.xz" rather than "edk2-*.fd" in the README hunk
  [Dan, Michael, Phil]

- pick up Michal's and Michael's R-b's

 pc-bios/descriptors/50-edk2-i386-secure.json   | 34 +++
 pc-bios/descriptors/50-edk2-x86_64-secure.json | 35 
 pc-bios/descriptors/60-edk2-aarch64.json   | 31 +
 pc-bios/descriptors/60-edk2-arm.json   | 31 +
 pc-bios/descriptors/60-edk2-i386.json  | 33 ++
 pc-bios/descriptors/60-edk2-x86_64.json| 34 +++
 pc-bios/README | 11 ++
 7 files changed, 209 insertions(+)

diff --git a/pc-bios/descriptors/50-edk2-i386-secure.json 
b/pc-bios/descriptors/50-edk2-i386-secure.json
new file mode 100644
index ..d7108c1da05a
--- /dev/null
+++ b/pc-bios/descriptors/50-edk2-i386-secure.json
@@ -0,0 +1,34 @@
+{
+"description": "UEFI firmware for i386, with Secure Boot and SMM",
+"interface-types": [
+"uefi"
+],
+"mapping": {
+"device": "flash",
+"executable": {
+"filename": "@DATADIR@/edk2-i386-secure-code.fd",
+"format": "raw"
+},
+"nvram-template": {
+"filename": "@DATADIR@/edk2-i386-vars.fd",
+"format": "raw"
+}
+},
+"targets": [
+{
+"architecture": "i386",
+"machines": [
+"pc-q35-*"
+]
+}
+],
+"features": [
+"acpi-s3",
+"requires-smm",
+"secure-boot",
+"verbose-dynamic"
+],
+"tags": [
+
+]
+}
diff --git a/pc-bios/descriptors/50-edk2-x86_64-secure.json 
b/pc-bios/descriptors/50-edk2-x86_64-secure.json
new file mode 100644
index ..387eb356236b
--- /dev/null
+++ b/pc-bios/descriptors/50-edk2-x86_64-secure.json
@@ -0,0 +1,35 @@
+{
+"description": "UEFI firmware for x86_64, with Secure Boot and SMM",
+"interface-types": [
+"uefi"
+],
+"mapping": {
+"device": "flash",
+"executable": {
+"filename": "@DATADIR@/edk2-x86_64-secure-code.fd",
+"format": "raw"
+},
+"nvram-template": {
+"filename": "@DATADIR@/edk2-i386-vars.fd",
+"format": "raw"
+}
+},
+"targets": [
+{
+"architecture": "x86_64",
+"machines": [
+"pc-q35-*"
+]
+}
+],
+"features": [
+"acpi-s3",
+"amd-sev",
+"requires-smm",
+"secure-boot",
+"verbose-dynamic"
+],
+"tags": [
+
+]
+}
diff --git a/pc-bios/descriptors/60-edk2-aarch64.json 
b/pc-bios/descriptors/60-edk2-aarch64.json
new file mode 100644
index ..800a21bda691
--- /dev/null
+++ b/pc-bios/descriptors/60-edk2-aarch64.json
@@ -0,0 +1,31 @@
+{
+"description": "UEFI firmware for aarch64",
+"interface-types": [
+"uefi"
+],
+"mapping": {
+"device": "flash",
+"executable": {
+"filename": "@DATADIR@/edk2-aarch64-code.fd",
+"format": "raw"
+},
+"nvram-template": {
+"filename": "@DATADIR@/edk2-arm-vars.fd",
+"format": "raw"
+}
+},
+"targets": [
+{
+"architecture": "aarch64",
+"machines": [
+"virt-*"
+]
+}
+],
+"features": [
+"verbose-static"
+],
+"tags": [
+
+]
+}
diff --git a/pc-bios/descriptors/60-edk2-arm.json 
b/pc-bios/descriptors/60-edk2-arm.json
new file mode 100644
index ..d5f1bba6cc82
--- /dev/null
+++ b/pc-bios/descriptors/60-edk2-arm.json
@@ -0,0 +1,31 @@
+{
+"description": "UEFI firmware for arm",
+"interface-types": [
+"uefi"
+],
+"mapping": {
+"device": "flash",
+"executable": {
+"filename": "@DATADIR@/edk2-arm-code.fd",
+"format": "raw"
+},
+"nvram-template": {
+"filename": "@DATADIR@/edk2-arm-vars.fd",
+"format": "raw"
+}
+},
+"targets": [
+{
+"architecture": "arm",
+"machines": [
+"virt-*"
+]
+}
+],
+"features": [
+"verbose-static"
+],
+"tags": [
+
+]
+}
diff --git a/pc-bios/descriptors/60-edk2-i386.json 
b/pc-bios/descriptors/60-edk2-i386.json
new file mode 100644
index ..2f8dec74fecb
--- /dev/null
+++ b/pc-bios/descriptors/60-edk2-i386.json
@@ -0,0 +1,33 @@
+{
+"description": "UEFI firmware for i386",
+"interface-types": [
+"uefi"
+],
+"mapp

[Qemu-devel] [PATCH v2 05/12] roms/edk2-funcs.sh: add the qemu_edk2_get_thread_count() function

[Laszlo Ersek]

The edk2 "build" utility natively supports building modules (that is, INF
files) in parallel. The feature is not useful when building a single
module (with the "-m" option), but it is useful for platform firmware
builds (which include many modules). Add a function that determines the
"-n" option argument for "build", from the MAKEFLAGS variable (i.e. based
on the presence of a make job server).

Signed-off-by: Laszlo Ersek 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Reviewed-by: Michal Privoznik 
Reviewed-by: Michael S. Tsirkin 
---

Notes:
v2:

- pick up Phil's R-b / T-b

- pick up Michal's and Michael's R-b's

 roms/edk2-funcs.sh | 25 
 1 file changed, 25 insertions(+)

diff --git a/roms/edk2-funcs.sh b/roms/edk2-funcs.sh
index d1cb1e4a111e..a9fae7ee891b 100644
--- a/roms/edk2-funcs.sh
+++ b/roms/edk2-funcs.sh
@@ -226,3 +226,28 @@ qemu_edk2_set_cross_env()
 
   eval "export $cross_prefix_var=\$cross_prefix"
 }
+
+
+# Determine the "-n" option argument (that is, the number of modules to build
+# in parallel) for the edk2 "build" utility. Print the result to the standard
+# output.
+#
+# Parameters:
+#   $1: the value of the MAKEFLAGS variable
+qemu_edk2_get_thread_count()
+{
+  local makeflags="$1"
+
+  if [[ "$makeflags" == *--jobserver-auth=* ]] ||
+ [[ "$makeflags" == *--jobserver-fds=* ]]; then
+# If there is a job server, allow the edk2 "build" utility to parallelize
+# as many module builds as there are logical CPUs in the system. The "make"
+# instances forked by "build" are supposed to limit themselves through the
+# job server. The zero value below causes the edk2 "build" utility to fetch
+# the logical CPU count with Python's multiprocessing.cpu_count() method.
+printf '0\n'
+  else
+# Build a single module at a time.
+printf '1\n'
+  fi
+}
-- 
2.19.1.3.g30247aa5d201

[Qemu-devel] [PATCH v2 11/12] Makefile: install the edk2 firmware images and their descriptors

[Laszlo Ersek]

Decompress and install the edk2 firmware blobs as part of "make install",
unless blob installation was disabled with configure's "--disable-blobs"
option.

Additionally, decompress the blobs as a pre-requisite for building softmmu
binaries -- this is helpful for both "make check" and other ad-hoc tests
one might want to run in the build directory.

Signed-off-by: Laszlo Ersek 
---

Notes:
v2:

- adapt to tracking the edk2 flash device files in compressed form [Dan,
  Michael, Phil]

- do not pick up Michal's and Michael's R-b's due to the above change

 configure  |  1 +
 Makefile   | 29 +++-
 .gitignore |  1 +
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index cab830a4c920..a8e5c14899d6 100755
--- a/configure
+++ b/configure
@@ -7770,6 +7770,7 @@ for bios_file in \
 $source_path/pc-bios/*.img \
 $source_path/pc-bios/openbios-* \
 $source_path/pc-bios/u-boot.* \
+$source_path/pc-bios/edk2-*.fd.xz \
 $source_path/pc-bios/palcode-*
 do
 LINKS="$LINKS pc-bios/$(basename $bios_file)"
diff --git a/Makefile b/Makefile
index 6ccb8639b08a..4b325052005e 100644
--- a/Makefile
+++ b/Makefile
@@ -296,6 +296,10 @@ ui/input-keymap-%.c: $(KEYCODEMAP_GEN) $(KEYCODEMAP_CSV) 
$(SRC_PATH)/ui/Makefile
 $(KEYCODEMAP_GEN): .git-submodule-status
 $(KEYCODEMAP_CSV): .git-submodule-status
 
+edk2-decompressed = $(basename $(wildcard pc-bios/edk2-*.fd.xz))
+pc-bios/edk2-%.fd: pc-bios/edk2-%.fd.xz
+   $(call quiet-command,xz -d -c $< > $@,"UNXZ",$<)
+
 # Don't try to regenerate Makefile or configure
 # We don't generate any of them
 Makefile: ;
@@ -444,6 +448,7 @@ $(SOFTMMU_SUBDIR_RULES): $(block-obj-y)
 $(SOFTMMU_SUBDIR_RULES): $(crypto-obj-y)
 $(SOFTMMU_SUBDIR_RULES): $(io-obj-y)
 $(SOFTMMU_SUBDIR_RULES): config-all-devices.mak
+$(SOFTMMU_SUBDIR_RULES): $(edk2-decompressed)
 
 subdir-%:
$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $* V="$(V)" 
TARGET_DIR="$*/" all,)
@@ -632,6 +637,7 @@ clean:
! -path ./roms/edk2/ArmPkg/Library/GccLto/liblto-arm.a \
! -path ./roms/edk2/BaseTools/Source/Python/UPT/Dll/sqlite3.dll 
\
-exec rm {} +
+   rm -f $(edk2-decompressed)
rm -f $(filter-out %.tlb,$(TOOLS)) $(HELPERS-y) qemu-ga TAGS cscope.* 
*.pod *~ */*~
rm -f fsdev/*.pod scsi/*.pod
rm -f qemu-img-cmds.h
@@ -722,9 +728,14 @@ spapr-rtas.bin slof.bin skiboot.lid \
 palcode-clipper \
 u-boot.e500 u-boot-sam460-20100605.bin \
 qemu_vga.ndrv \
+edk2-licenses.txt \
 hppa-firmware.img
+
+DESCS=50-edk2-i386-secure.json 50-edk2-x86_64-secure.json \
+60-edk2-aarch64.json 60-edk2-arm.json 60-edk2-i386.json 60-edk2-x86_64.json
 else
 BLOBS=
+DESCS=
 endif
 
 # Note that we manually filter-out the non-Sphinx documentation which
@@ -785,7 +796,8 @@ endif
 
 ICON_SIZES=16x16 24x24 32x32 48x48 64x64 128x128 256x256 512x512
 
-install: all $(if $(BUILD_DOCS),install-doc) install-datadir 
install-localstatedir
+install: all $(if $(BUILD_DOCS),install-doc) install-datadir 
install-localstatedir \
+   $(if $(INSTALL_BLOBS),$(edk2-decompressed))
 ifneq ($(TOOLS),)
$(call install-prog,$(subst 
qemu-ga,qemu-ga$(EXESUF),$(TOOLS)),$(DESTDIR)$(bindir))
 endif
@@ -807,6 +819,21 @@ ifneq ($(BLOBS),)
set -e; for x in $(BLOBS); do \
$(INSTALL_DATA) $(SRC_PATH)/pc-bios/$$x 
"$(DESTDIR)$(qemu_datadir)"; \
done
+endif
+ifdef INSTALL_BLOBS
+   set -e; for x in $(edk2-decompressed); do \
+   $(INSTALL_DATA) $$x "$(DESTDIR)$(qemu_datadir)"; \
+   done
+endif
+ifneq ($(DESCS),)
+   $(INSTALL_DIR) "$(DESTDIR)$(qemu_datadir)/firmware"
+   set -e; tmpf=$$(mktemp); trap 'rm -f -- "$$tmpf"' EXIT; \
+   for x in $(DESCS); do \
+   sed -e 's,@DATADIR@,$(DESTDIR)$(qemu_datadir),' \
+   "$(SRC_PATH)/pc-bios/descriptors/$$x" > "$$tmpf"; \
+   $(INSTALL_DATA) "$$tmpf" \
+   "$(DESTDIR)$(qemu_datadir)/firmware/$$x"; \
+   done
 endif
for s in $(ICON_SIZES); do \
mkdir -p "$(DESTDIR)/$(qemu_icondir)/hicolor/$${s}/apps"; \
diff --git a/.gitignore b/.gitignore
index 77522561b8ea..889896c023fd 100644
--- a/.gitignore
+++ b/.gitignore
@@ -96,6 +96,7 @@
 *.gcno
 *.gcov
 /pc-bios/bios-pq/status
+/pc-bios/edk2-*.fd
 /pc-bios/vgabios-pq/status
 /pc-bios/optionrom/linuxboot.asm
 /pc-bios/optionrom/linuxboot.bin
-- 
2.19.1.3.g30247aa5d201

[Qemu-devel] [PATCH v2 08/12] pc-bios: add edk2 firmware binaries and variable store templates

[Laszlo Ersek]

Add the files built by the last patch: (compressed) binaries, and the
cumulative license text that covers them.

Signed-off-by: Laszlo Ersek 
---

Notes:
v2:

- capture the compressed build outputs of the last patch; slightly
  update the commit message [Dan, Michael, Phil]

- consequently, do not pick up Michal's and Michael's R-b's

 pc-bios/edk2-licenses.txt | 209 
 pc-bios/edk2-aarch64-code.fd.xz   | Bin 0 -> 1146804 bytes
 pc-bios/edk2-arm-code.fd.xz   | Bin 0 -> 1147852 bytes
 pc-bios/edk2-arm-vars.fd.xz   | Bin 0 -> 10008 bytes
 pc-bios/edk2-i386-code.fd.xz  | Bin 0 -> 1674764 bytes
 pc-bios/edk2-i386-secure-code.fd.xz   | Bin 0 -> 1870024 bytes
 pc-bios/edk2-i386-vars.fd.xz  | Bin 0 -> 320 bytes
 pc-bios/edk2-x86_64-code.fd.xz| Bin 0 -> 1655276 bytes
 pc-bios/edk2-x86_64-secure-code.fd.xz | Bin 0 -> 1889024 bytes
 9 files changed, 209 insertions(+)

diff --git a/pc-bios/edk2-licenses.txt b/pc-bios/edk2-licenses.txt
new file mode 100644
index ..8bdb1abc993e
--- /dev/null
+++ b/pc-bios/edk2-licenses.txt
@@ -0,0 +1,209 @@
+==> edk2/License.txt <==
+Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.
+Copyright (c) 2011 - 2015, ARM Limited. All rights reserved.
+Copyright (c) 2014 - 2015, Linaro Limited. All rights reserved.
+Copyright (c) 2013 - 2015, Red Hat, Inc.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions
+are met:
+
+* Redistributions of source code must retain the above copyright
+  notice, this list of conditions and the following disclaimer.
+* Redistributions in binary form must reproduce the above copyright
+  notice, this list of conditions and the following disclaimer in
+  the documentation and/or other materials provided with the
+  distribution.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+POSSIBILITY OF SUCH DAMAGE.
+
+==> edk2/OvmfPkg/License.txt <==
+Copyright (c) 2012, Intel Corporation. All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions
+are met:
+
+* Redistributions of source code must retain the above copyright
+  notice, this list of conditions and the following disclaimer.
+* Redistributions in binary form must reproduce the above copyright
+  notice, this list of conditions and the following disclaimer in
+  the documentation and/or other materials provided with the
+  distribution.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+POSSIBILITY OF SUCH DAMAGE.
+
+
+Some files are subject to the following license, the MIT license. Those files
+are located in:
+- OvmfPkg/Include/IndustryStandard/Xen/
+- OvmfPkg/XenBusDxe/
+- OvmfPkg/XenPvBlkDxe/
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in
+all copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES O

Re: [Qemu-devel] Maintainers, please tell us how to boot your machines!

[Stafford Horne]

On Tue, Mar 12, 2019 at 06:36:05PM +0100, Markus Armbruster wrote:
> = hw/openrisc/openrisc_sim.c =
> Jia Liu  (maintainer:or1k-sim)
> Stafford Horne  (odd fixer:OpenRISC)

For OpenRISC the main test I do is booting linux.  The steps and a link to a
system image are described here:

  https://wiki.qemu.org/Documentation/Platforms/OpenRISC

I just confirmed with the latest QEMU and it works well.

For good measure, we probably should have an SMP kernel as well and try to boot
both so we would have 2 test cases.  I will update the wiki to add SMP.

Let me know if you need me to create patches for the automated tests.

-Stafford

[Qemu-devel] [PATCH v2 06/12] roms/Makefile: replace the $(EFIROM) target with "edk2-basetools"

[Laszlo Ersek]

We don't (can't) have a recipe for building just $(EFIROM); therefore,
while we call the target $(EFIROM), we actually build all of the edk2
BaseTools. Rename the target to edk2-basetools, and update the iPXE
prerequisite accordingly. This will let other targets depend on
"edk2-basetools", where an $(EFIROM) pre-requisite would be misleading.

Signed-off-by: Laszlo Ersek 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Reviewed-by: Michal Privoznik 
Reviewed-by: Michael S. Tsirkin 
---

Notes:
v2:

- pick up Phil's R-b / T-b

- pick up Michal's and Michael's R-b's

 roms/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/roms/Makefile b/roms/Makefile
index 78d5dd18c301..2e83ececa25a 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -102,7 +102,7 @@ pxe-rom-%: build-pxe-roms
 
 efirom: $(patsubst %,efi-rom-%,$(pxerom_variants))
 
-efi-rom-%: build-pxe-roms build-efi-roms $(EFIROM)
+efi-rom-%: build-pxe-roms build-efi-roms edk2-basetools
$(EFIROM) -f "0x$(VID)" -i "0x$(DID)" -l 0x02 \
-b ipxe/src/bin/$(VID)$(DID).rom \
-ec ipxe/src/bin-i386-efi/$(VID)$(DID).efidrv \
@@ -120,7 +120,7 @@ build-efi-roms: build-pxe-roms
$(patsubst %,bin-i386-efi/%.efidrv,$(pxerom_targets)) \
$(patsubst %,bin-x86_64-efi/%.efidrv,$(pxerom_targets))
 
-$(EFIROM):
+edk2-basetools:
$(MAKE) -C edk2/BaseTools
 
 slof:
-- 
2.19.1.3.g30247aa5d201

[Qemu-devel] [PATCH v2 10/12] tests: add missing dependency to build QTEST_QEMU_BINARY, round 2

[Laszlo Ersek]

In commit b94b330e2333 ("tests: add missing dependency to build
QTEST_QEMU_BINARY", 2017-07-31), Phil fixed the dependency list of make
target "check-qtest-%". Namely, the recipe would set QTEST_QEMU_BINARY to
the softmmu emulator for the emulation target, but the prerequisites
didn't include the emulator.

The same issue affects the "check-report-qtest-%.tap" make target, which
is the other make target whose recipe sets QTEST_QEMU_BINARY:

> $ make -j4 check-report-qtest-aarch64.tap
>   TAP check-report-qtest-aarch64.tap
> sh: /.../aarch64-softmmu/qemu-system-aarch64: No such file or directory

Apply Phil's fix to this make target too.

Signed-off-by: Laszlo Ersek 
---

Notes:
v2:

- new patch, relied upon by the next patch

 tests/Makefile.include | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index a5719551dd1b..6c116addafe4 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -915,7 +915,7 @@ check-speed: $(check-speed-y)
 
 # gtester tests with TAP output
 
-$(patsubst %, check-report-qtest-%.tap, $(QTEST_TARGETS)): 
check-report-qtest-%.tap: $(check-qtest-y)
+$(patsubst %, check-report-qtest-%.tap, $(QTEST_TARGETS)): 
check-report-qtest-%.tap: subdir-%-softmmu $(check-qtest-y)
$(call do_test_tap, $(check-qtest-$*-y) $(check-qtest-generic-y), \
  QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
  QTEST_QEMU_IMG=qemu-img$(EXESUF))
-- 
2.19.1.3.g30247aa5d201

[Qemu-devel] [PATCH v2 12/12] MAINTAINERS: add the "EDK2 Firmware" subsystem

[Laszlo Ersek]

We now have the edk2 submodule, somewhat elaborate build helpers for it,
and even a UEFI application written against edk2 whose genuine home is the
QEMU repository. Add the "EDK2 Firmware" subsystem such that all relevant
pathnames be covered.

Suggested-by: Daniel P. Berrangé 
Suggested-by: Michael S. Tsirkin 
Suggested-by: Philippe Mathieu-Daudé 
Signed-off-by: Laszlo Ersek 
---

Notes:
v2:

- new patch [Dan, Michael, Phil]

- compared to the suggestions, upgrade the maintenance status ("S") from
  Maintained to Supported

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

diff --git a/MAINTAINERS b/MAINTAINERS
index d3267560799b..eefa225a234a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2207,6 +2207,18 @@ F: include/hw/i2c/smbus_master.h
 F: include/hw/i2c/smbus_slave.h
 F: include/hw/i2c/smbus_eeprom.h
 
+EDK2 Firmware
+M: Laszlo Ersek 
+M: Philippe Mathieu-Daudé 
+S: Supported
+F: pc-bios/descriptors/??-edk2-*.json
+F: pc-bios/edk2-*
+F: roms/Makefile.edk2
+F: roms/edk2
+F: roms/edk2-*
+F: tests/data/uefi-boot-images/
+F: tests/uefi-test-tools/
+
 Usermode Emulation
 --
 Overall
-- 
2.19.1.3.g30247aa5d201

Re: [Qemu-devel] [PATCH] Clean up includes

[BALATON Zoltan]

On Wed, 13 Mar 2019, Markus Armbruster wrote:

BALATON Zoltan  writes:

On Wed, 13 Mar 2019, Markus Armbruster wrote:

Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.

This commit was created with scripts/clean-includes, with the changes
to the following files manually reverted:

   contrib/libvhost-user/libvhost-user-glib.h
   contrib/libvhost-user/libvhost-user.c
   contrib/libvhost-user/libvhost-user.h
   linux-user/mips64/cpu_loop.c
   linux-user/mips64/signal.c
   linux-user/sparc64/cpu_loop.c
   linux-user/sparc64/signal.c
   linux-user/x86_64/cpu_loop.c
   linux-user/x86_64/signal.c
   slirp/src/*
   target/s390x/gen-features.c
   tests/migration/s390x/a-b-bios.c
   tests/test-rcu-simpleq.c
   tests/test-rcu-tailq.c
   tests/uefi-test-tools/UefiTestToolsPkg/BiosTablesTest/BiosTablesTest.c

We're in the process of spinning out slirp/.  tests/uefi-test-tools/
is guest software.  The remaining reverts are the same as in commit
b7d89466dde.

Signed-off-by: Markus Armbruster 

[...]

diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index bc98ba6eeb..f31b3c27c7 100644
--- a/hw/display/ati_2d.c
+++ b/hw/display/ati_2d.c
@@ -7,6 +7,7 @@
 * This work is licensed under the GNU GPL license version 2 or later.
 */

+#include "qemu/osdep.h"
#include "ati_int.h"
#include "ati_regs.h"
#include "qemu/log.h"
diff --git a/hw/display/ati_dbg.c b/hw/display/ati_dbg.c
index 1e6c32624e..b045f81d06 100644
--- a/hw/display/ati_dbg.c
+++ b/hw/display/ati_dbg.c
@@ -1,3 +1,4 @@
+#include "qemu/osdep.h"
#include "ati_int.h"

#ifdef DEBUG_ATI
diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h
index a6f3e20e63..2f426064cf 100644
--- a/hw/display/ati_int.h
+++ b/hw/display/ati_int.h
@@ -9,7 +9,6 @@
#ifndef ATI_INT_H
#define ATI_INT_H

-#include "qemu/osdep.h"


What's wrong with ati_int.h including osdep.h first and everything
else including ati_int.h first? I think it was OK that way so unless
there's a good reason to explicitely include osdep in all files that
also include ati_int.h I think these should not be changed. For the
ati model we need ati_int.h included first so it's OK to include
osdep.h from there.


./HACKING explains:

   1.2. Include directives

   Order include directives as follows:

   #include "qemu/osdep.h"  /* Always first... */
   #include <...>   /* then system headers... */
   #include "..."   /* and finally QEMU headers. */

   The "qemu/osdep.h" header contains preprocessor macros that affect the 
behavior
   of core system headers like .  It must be the first include so that
   core system headers included by external libraries get the preprocessor 
macros
   that QEMU depends on.

   Do not include "qemu/osdep.h" from header files since the .c file will have
   already included it.


I'm not convinced. The rule is to include osdep.h first and it's currently 
satisified without this change as well but if it makes clean-includes 
script happy then I don't mind.


Regards,
BALATON Zoltan

Re: [Qemu-devel] [PATCH v5 0/5] Add "boot_linux" acceptance test

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190313204611.21041-1-cr...@redhat.com/



Hi,

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

Type: series
Message-id: 20190313204611.21041-1-cr...@redhat.com
Subject: [Qemu-devel] [PATCH v5 0/5] Add "boot_linux" acceptance test

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
   523a2a42c3..3b5b6e9b51  master -> master
 * [new tag]   patchew/20190313204611.21041-1-cr...@redhat.com -> 
patchew/20190313204611.21041-1-cr...@redhat.com
Auto packing the repository in background for optimum performance.
See "git help gc" for manual housekeeping.
Switched to a new branch 'test'
711a9b5f5d Add "boot_linux" acceptance test for x86_64 and pc machine type
81e72de890 Acceptance tests: depend on qemu-img
4e7fff0941 Acceptance tests: add the build directory to the system PATH
a9c94702e7 Acceptance tests: keep a stable reference to the QEMU build dir
cdbbe8cd85 Acceptance tests: use relative location for tests

=== OUTPUT BEGIN ===
1/5 Checking commit cdbbe8cd859f (Acceptance tests: use relative location for 
tests)
2/5 Checking commit a9c94702e769 (Acceptance tests: keep a stable reference to 
the QEMU build dir)
ERROR: line over 90 characters
#48: FILE: tests/acceptance/avocado_qemu/__init__.py:17:
+SRC_ROOT_DIR = 
os.path.dirname(os.path.dirname(os.path.dirname(os.path.dirname(__file__

total: 1 errors, 0 warnings, 8 lines checked

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

3/5 Checking commit 4e7fff0941b5 (Acceptance tests: add the build directory to 
the system PATH)
4/5 Checking commit 81e72de8905f (Acceptance tests: depend on qemu-img)
5/5 Checking commit 711a9b5f5daa (Add "boot_linux" acceptance test for x86_64 
and pc machine type)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#29: 
new file mode 100644

total: 0 errors, 1 warnings, 62 lines checked

Patch 5/5 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


The full log is available at
http://patchew.org/logs/20190313204611.21041-1-cr...@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[Qemu-devel] [PATCH v5 3/5] Acceptance tests: add the build directory to the system PATH

[Cleber Rosa]

So that when binaries such as qemu-img are searched for, those in the
build tree will be favored.  As a clarification, SRC_ROOT_DIR is
dependent on the location from where tests are executed, so they are
equal to the build directory if one is being used.

The original motivation is that Avocado libraries such as
avocado.utils.vmimage.get() may use the matching binaries, but it may
also apply to any other binary that test code may eventually attempt
to execute.

Signed-off-by: Cleber Rosa 
---
 tests/acceptance/avocado_qemu/__init__.py | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/tests/acceptance/avocado_qemu/__init__.py 
b/tests/acceptance/avocado_qemu/__init__.py
index 7b5d828584..a91e1202b8 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -42,6 +42,12 @@ def pick_default_qemu_bin():
 
 class Test(avocado.Test):
 def setUp(self):
+# Some utility code uses binaries from the system's PATH.  For
+# instance, avocado.utils.vmimage.get() uses qemu-img, to
+# create a snapshot image.  This is a transparent way of
+# making sure those utilities find and use binaries on the
+# build tree by default.
+os.environ['PATH'] = '%s:%s' % (SRC_ROOT_DIR, os.environ['PATH'])
 self._vms = {}
 self.qemu_bin = self.params.get('qemu_bin',
 default=pick_default_qemu_bin())
-- 
2.20.1

[Qemu-devel] [PATCH v5 0/5] Add "boot_linux" acceptance test

[Cleber Rosa]

This adds an acceptance test that validates that a full blown Linux
guest can successfully boot in QEMU.

Changes from v4:


 * New commit "Acceptance tests: use relative location for tests"

 * New commit "Acceptance tests: keep a stable reference to the QEMU build dir"

 * Pinned the Fedora 29 image by adding a checksum.  The goal is to
   never allow more than one component to change at a time (the one
   allowed to change is QEMU itself).  Updates to the image should be
   manual. (Based on comments from Cornelia)

 * Moved the downloading of the Fedora 29 cloud image to the test
   setUp() method, canceling the test if the image can not be
   downloaded.

 * Removed the ":avocado: enable" tag, given that Avocado versions
   68.0 and later operate on a "recursive by default" manner, that
   is able to correctly identify this as an Avocado test.

Changes from v3:


 * New patch "Acceptance tests: depend on qemu-img"

Known Issues on v3 (no longer applicable):
==

 * A recent TCG performance regression[1] affects this test in a
   number of ways:
   - The test execution may timeout by itself
   - The generation of SSH host keys in the guest's first boot is also
 affected (possibly also a timeout)
   - The cloud-init "phone home" feature attempts to read the host keys
 and fails, causing the test to timeout and fail

   These are not observed anymore once the fix[2] is applied.

[1] - https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg00338.html
[2] - https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg01129.html

Changes from v2:


 * Updated the tag to include the "arch:" key, in a similar fashion as to
   the tests in the "Acceptance Tests: target architecture support":
   - https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg00369.html

 * Renamed the test method name to test_x86_64_pc, again, similarly to the
   boot_linux_console.py tests in the series mentioned before.

 * Set the machine type explicitly, again similarly to the
   boot_linux_console.py tests in the series mentioned before.

 * Added messages after the launch of the VM, to let test runners know
   the test know waits for a boot confirmation from the the guest (Eduardo).

 * Updated commit message to reflect the fact that this version does
   not allow for parameterization of the guest OS, version, etc.

 * Dropped the RFC prefix on patch "RFC: Acceptance tests: add the
   build directory to the system PATH"

 * Changed the comments on "RFC: Acceptance tests: add the build
   directory to the system PATH" to make it clear the addition of a
   the build directory to the PATH may influence other utility code.

Changes from v1:


 * The commit message was adjusted, removing the reference to the
   avocado.utils.vmimage encoding issue on previous Avocado versions
   (<= 64.0) and the fix that would (and was) included in Avocado
   version 65.0.

 * Effectively added pycdlib==1.6.0 to the requirements.txt file,
   added on a56931eef3, and adjusted the commit message was also
   to reflect that.

 * Updated the default version of the guest OS, from Fedora 28 to 29.
   Besides possible improvements in the (virtual) hardware coverage,
   it brings a performance improvement in the order of 20% to the
   test.

 * Removed all direct parameters usage.  Because some parameters and
   its default values implemented in the test would prevent it from
   running on some environments.  Example: the "accel" parameter had a
   default value of "kvm", which would prevent this test, that boots a
   x86_64 OS, from running on a host arch different than x86_64.  I
   recognize that it's desirable to make tests reusable and
   parameterized (that was the reason for the first version doing so),
   but the mechanism to be used to define the architectures that a
   given test should support is still an open issue, and has been
   discussed in other threads.  I'll follow up those discussions with
   a proposal, and until then, removing those aspects from this test
   implementation seemed to be the best option.  A caveat: this test
   currently adds the same tag (x86_64) and follows other assumptions
   made on "boot_linux_console.py", that is, that a x86_64 target
   binary will be used to run it.  If a user is in an environment that
   does not have a x86_64 target binary, it could filter those tests
   out with: "avocado run --filter-by-tags='-x86_64' tests/acceptance".

 * Removed most arguments to the QEMU command line for pretty much the
   same reasons described above, and by following the general
   perception that I could grasp from other discussions that QEMU
   defaults should preferrably be used.  This test, as well as others,
   can and should be extended later to allow for different test
   scenarios by passing well documented parameter values.  That is,
   they should respect well-known parameters such as "accel" mention

[Qemu-devel] [PATCH v5 4/5] Acceptance tests: depend on qemu-img

[Cleber Rosa]

Tests using the avocado.utils.vmimage library make use of qemu-img,
and because it makes sense to use the version matching the rest of the
source code, let's make sure it gets built.

Its selection, instead of a possible qemu-img binary installed system
wide, is already dealt with by the change that adds the build dir to
the PATH during the test execution.

This is based on the same work for qemu-iotests, and suggested by its
author:

  - https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg00951.html

CC: Philippe Mathieu-Daudé 
Signed-off-by: Cleber Rosa 
Reviewed-by: Philippe Mathieu-Daudé 
---
 tests/Makefile.include | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 546c101a14..77d71086f3 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -1159,7 +1159,7 @@ $(TESTS_RESULTS_DIR):
 
 check-venv: $(TESTS_VENV_DIR)
 
-check-acceptance: check-venv $(TESTS_RESULTS_DIR)
+check-acceptance: check-venv $(TESTS_RESULTS_DIR) qemu-img$(EXESUF)
$(call quiet-command, \
 $(TESTS_VENV_DIR)/bin/python -m avocado \
 --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \
-- 
2.20.1

[Qemu-devel] [PATCH v5 5/5] Add "boot_linux" acceptance test for x86_64 and pc machine type

[Cleber Rosa]

This acceptance test, validates that a full blown Linux guest can
successfully boot in QEMU.  In this specific case, the guest chosen is
Fedora version 29.

The method for checking the successful boot is based on "cloudinit"
and its "phone home" feature.  The guest is given an ISO image
with the location of the phone home server, and the information to
post (the instance ID).  Upon receiving the correct information,
from the guest, the test is considered to have PASSed.

This test is currently limited to user mode networking only, and
instructs the guest to connect to the "router" address that is hard
coded in QEMU.

To create the cloudinit ISO image that will be used to configure the
guest, the pycdlib library is also required and has been added as
requirement to the virtual environment created by "check-venv".

Signed-off-by: Cleber Rosa 
---
 tests/acceptance/boot_linux.py | 58 ++
 tests/requirements.txt |  1 +
 2 files changed, 59 insertions(+)
 create mode 100644 tests/acceptance/boot_linux.py

diff --git a/tests/acceptance/boot_linux.py b/tests/acceptance/boot_linux.py
new file mode 100644
index 00..639acf7795
--- /dev/null
+++ b/tests/acceptance/boot_linux.py
@@ -0,0 +1,58 @@
+# Functional test that boots a complete Linux system via a cloud image
+#
+# Copyright (c) 2018-2019 Red Hat, Inc.
+#
+# Author:
+#  Cleber Rosa 
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+import os
+
+from avocado_qemu import Test
+
+from avocado.utils import cloudinit
+from avocado.utils import network
+from avocado.utils import vmimage
+
+
+class BootLinux(Test):
+"""
+Boots a Linux system, checking for a successful initialization
+"""
+
+timeout = 600
+
+def setUp(self):
+super(BootLinux, self).setUp()
+try:
+self.log.info('Downloading and preparing boot image')
+self.boot = vmimage.get(
+'fedora', arch='x86_64', version='29',
+checksum='7109d23215a3911b260a3b9bf3a07aac3436253a',
+cache_dir=self.cache_dirs[0],
+snapshot_dir=self.workdir)
+except:
+self.cancel('Failed to download boot image')
+
+def test_x86_64_pc(self):
+"""
+:avocado: tags=arch:x86_64
+:avocado: tags=machine:pc
+"""
+self.vm.set_machine('pc')
+self.vm.add_args('-m', '1024')
+self.vm.add_args('-drive', 'file=%s' % self.boot.path)
+
+cloudinit_iso = os.path.join(self.workdir, 'cloudinit.iso')
+phone_home_port = network.find_free_port()
+cloudinit.iso(cloudinit_iso, self.name,
+  # QEMU's hard coded usermode router address
+  phone_home_host='10.0.2.2',
+  phone_home_port=phone_home_port)
+self.vm.add_args('-drive', 'file=%s' % cloudinit_iso)
+
+self.vm.launch()
+self.log.info('VM launched, waiting for boot confirmation from guest')
+cloudinit.wait_for_phone_home(('0.0.0.0', phone_home_port), self.name)
diff --git a/tests/requirements.txt b/tests/requirements.txt
index 002ded6a22..57213b8b72 100644
--- a/tests/requirements.txt
+++ b/tests/requirements.txt
@@ -2,3 +2,4 @@
 # in the tests/venv Python virtual environment. For more info,
 # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
 avocado-framework==68.0
+pycdlib==1.6.0
-- 
2.20.1

[Qemu-devel] [PATCH v5 2/5] Acceptance tests: keep a stable reference to the QEMU build dir

[Cleber Rosa]

This is related to the the differences in in-tree and out-of-tree
builds in QEMU.  For simplification,  means my build directory.

Currently, by running a `make check-acceptance` one gets (in
tests/acceptance/avocado_qemu/__init__.py):

   SRC_ROOT_DIR: /tests/acceptance/avocado_qemu/../../..

This in itself is problematic, because after the parent directories
are applied, one may be left not with a pointer to the build directory
as intended, but with the location of the source tree (assuming they
differ). Built binaries, such as qemu-img, are of course not there and
can't be found.

Given that a Python '__file__' will contain the absolute path to the
file backing the module, say:

   __file__: /tests/acceptance/avocado_qemu/__init__.py

  |  4  | 3|  2 | 1 |

A solution is to not "evaluate" the third parent dir (marked as 4
here) because that ends up following the "tests" directory symlink to
the source tree.  In fact, there's no need to keep or evaluate any of
the parent directories, we can just drop the rightmost 4 components,
and we'll keep a stable reference to the build directory (with no
symlink being followed).  This works for either a dedicated build
directory or also a combined source and build tree.

Signed-off-by: Cleber Rosa 
---
 tests/acceptance/avocado_qemu/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/acceptance/avocado_qemu/__init__.py 
b/tests/acceptance/avocado_qemu/__init__.py
index a66ec72daa..7b5d828584 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -14,7 +14,7 @@ import uuid
 
 import avocado
 
-SRC_ROOT_DIR = os.path.join(os.path.dirname(__file__), '..', '..', '..')
+SRC_ROOT_DIR = 
os.path.dirname(os.path.dirname(os.path.dirname(os.path.dirname(__file__
 sys.path.append(os.path.join(SRC_ROOT_DIR, 'python'))
 
 from qemu import QEMUMachine
-- 
2.20.1

[Qemu-devel] [PATCH v5 1/5] Acceptance tests: use relative location for tests

[Cleber Rosa]

An Avocado Test ID[1] is composed by a number of components, but it
starts with the Test Name, usually a file system location that was
given to the loader.

Because the source directory is being given as a prefix to the
"tests/acceptance" directory containing the acceptance tests, the test
names will needlessly include the directory the user is using to host
the QEMU sources (and/or build tree).

Let's remove the source dir (or a build dir) from the path given to
the test loader.  This should give more constant names, and when using
result servers and databases, it should give the same test names
across executions from different people or from different directories.

[1] - 
https://avocado-framework.readthedocs.io/en/69.0/ReferenceGuide.html#test-id

Signed-off-by: Cleber Rosa 
---
 tests/Makefile.include | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index a5719551dd..546c101a14 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -1163,7 +1163,7 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR)
$(call quiet-command, \
 $(TESTS_VENV_DIR)/bin/python -m avocado \
 --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \
---failfast=on $(SRC_PATH)/tests/acceptance, \
+--failfast=on tests/acceptance, \
 "AVOCADO", "tests/acceptance")
 
 # Consolidated targets
-- 
2.20.1

Re: [Qemu-devel] [PATCH v2 4/4] RFC: slirp: is not maintained by Kelly Price for a long time

[Kelly Price]

You have my sign-off. Take it away!

> On Mar 13, 2019, at 1:59 PM, Eric Blake  wrote:
> 
>> On 3/13/19 11:42 AM, Marc-André Lureau wrote:
>> slirp has been maintained by the QEMU maintainers and will be
>> maintained under an independent project soon.
>> 
>> Cc: Kelly Price 
>> Cc: Kelly Price 
>> Signed-off-by: Marc-André Lureau 
>> ---
>> slirp/COPYRIGHT | 2 --
>> 1 file changed, 2 deletions(-)
> 
> It would be nice to get Kelly's response, but even without, I think this
> patch is safe.
> 
> Reviewed-by: Eric Blake 
> 
>> 
>> diff --git a/slirp/COPYRIGHT b/slirp/COPYRIGHT
>> index 1bc83d497e..faba47f4ea 100644
>> --- a/slirp/COPYRIGHT
>> +++ b/slirp/COPYRIGHT
>> @@ -1,8 +1,6 @@
>> Slirp was written by Danny Gasparovski.
>> Copyright (c), 1995,1996 All Rights Reserved.
>> 
>> -Slirp is maintained by Kelly Price 
>> -
>> Slirp is free software; "free" as in you don't have to pay for it, and you
>> are free to do whatever you want with it.  I do not accept any donations,
>> monetary or otherwise, for Slirp.  Instead, I would ask you to pass this
>> 
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
> 

Re: [Qemu-devel] [PULL 00/26] pci, pc, virtio: features, fixes, cleanups

[Peter Maydell]

On Wed, 13 Mar 2019 at 02:54, Michael S. Tsirkin  wrote:
>
> The following changes since commit 9d867123659ac32edd2a0b64cabe2dca544f1d0e:
>
>   Merge remote-tracking branch 
> 'remotes/ehabkost/tags/python-next-pull-request' into staging (2019-03-12 
> 18:31:56 +)
>
> are available in the Git repository at:
>
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>
> for you to fetch changes up to 9040e6dfa8c3fed87695a3de555d2c775727bb51:
>
>   i386, acpi: check acpi_memory_hotplug capacity in pre_plug (2019-03-12 
> 22:31:21 -0400)
>
> 
> pci, pc, virtio: features, fixes, cleanups
>
> intel-iommu scalable option
> pcie acs emulation
> beginning for vhost-user-blk reconnect and of vhost-user backend work
> misc fixes and cleanups
>
> Signed-off-by: Michael S. Tsirkin 
>
> 

Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0
for any user-visible changes.

-- PMM

Re: [Qemu-devel] [PATCH 0/5] QEMU VFIO live migration

[Alex Williamson]

On Tue, 12 Mar 2019 02:48:39 +
"Tian, Kevin"  wrote:

> > From: Alex Williamson [mailto:alex.william...@redhat.com]
> > Sent: Tuesday, March 12, 2019 4:19 AM
> > On Mon, 11 Mar 2019 02:33:11 +
> > "Tian, Kevin"  wrote:
> >   
> [...]
> > 
> > I think I've fully conceded any notion of security by obscurity towards
> > opaque data already, but segregating types of device data still seems
> > to unnecessarily impose a usage model on the vendor driver that I think
> > we should try to avoid.  The migration interface should define the
> > protocol through which userspace can save and restore the device state,
> > not impose how the vendor driver exposes or manages that state.  Also, I
> > got the impression (perhaps incorrectly) that you were trying to mmap
> > live data to userspace, which would allow not only saving the state,
> > but also unchecked state modification by userspace. I think we want
> > more of a producer/consumer model of the state where consuming state
> > also involves at least some degree of sanity or consistency checking.
> > Let's not forget too that we're obviously dealing with non-open source
> > driver in the mdev universe as well.  
> 
> OK. I think for this part we are in agreement - as long as the goal of
> this interface is clearly defined as such way. :-)
> 
> [...]
> > > But I still didn't get your exact concern about security part. For
> > > version yes we still haven't worked out a sane way to represent
> > > vendor-specific compatibility requirement. But allowing user
> > > space to modify data through this interface has really no difference
> > > from allowing guest to modify data through trapped MMIO interface.
> > > mdev driver should guarantee that operations through both interfaces
> > > can modify only the state associated with the said mdev instance,
> > > w/o breaking the isolation boundary. Then the former becomes just
> > > a batch of operations to be verified in the same way as if they are
> > > done individually through the latter interface.  
> > 
> > It seems like you're assuming a working model for the vendor driver and
> > the data entering and exiting through this interface.  The vendor
> > drivers can expose data any way that they want.  All we need to do is
> > imagine that the migration data stream includes an array index count
> > somewhere which the user could modify to trigger the in-kernel vendor
> > driver to allocate an absurd array size and DoS the target.  This is
> > probably the most simplistic attack, possibly knowing the state machine
> > of the vendor driver a malicious user could trick it into providing
> > host kernel data.  We're not necessarily only conveying state that the
> > user already has access to via this interface, the vendor driver may
> > include non-visible internal state as well.  Even the state that is
> > user accessible is being pushed into the vendor driver via an alternate
> > path from the user mediation we have on the existing paths.  
> 
> Then I don't know how this concern can be effectively addressed 
> since you assume vendor drivers are not trusted here. and why do
> you trust vendor drivers on mediating existing path but not this
> alternative one? non-visible internal states just mean more stuff
> to be carefully scrutinized, which is not essentially causing a 
> conceptual difference of trust level.
> 
> Or can this concern be partially mitigated if we create some 
> test cases which poke random data through the new interface,
> and mark vendor drivers which pass such tests as trusted? Then
> there is also an open who should be in charge of such certification 
> process...

The vendor driver is necessarily trusted, it lives in the kernel, it
works in the kernel address space.  Unfortunately that's also the risk
with passing data from userspace into the vendor driver, the vendor
driver needs to take every precaution in sanitizing and validating that
data.  I wish we had a common way to perform that checking, but it
seems that each vendor driver is going to need to define their own
protocol and battle their own bugs and exploits in the code
implementing that protocol.  For open source drivers we can continue to
rely on review and openness, for closed drivers... the user has already
accepted the risk to trust the driver themselves.  Perhaps all I can do
is raise the visibility that there are potential security issues here
and vendor drivers need to own that risk.

A fuzzing test would be great, we could at least validate whether a
vendor driver implements some sort of CRC test, but I don't think we
can create a certification process around that.  Thanks,

Alex

[Qemu-devel] Seccomp profile for swtpm as default

[Stefan Berger]

Hello!

 If you have some feedback regarding a seccomp profile extension for 
swtpm for v0.2, please let me know. I created this github issue here:



https://github.com/stefanberger/swtpm/issues/115


Basically the choice is whether to make the creation of the seccomp 
profile a default behavior or have the caller explicitly pass the 
'--seccomp profile=default' on the command line, which then in turn 
would require libvirt for example to check whether this current version 
of swtpm supports the feature either by swtpm version or by strstr() the 
help page.



Regards,

   Stefan

Re: [Qemu-devel] [PATCH 0/5] QEMU VFIO live migration

[Alex Williamson]

On Tue, 12 Mar 2019 21:13:01 -0400
Zhao Yan  wrote:

> hi Alex
> Any comments to the sequence below?
> 
> Actaully we have some concerns and suggestions to userspace-opaque migration
> data.
> 
> 1. if data is opaque to userspace, kernel interface must be tightly bound to
> migration. 
>e.g. vendor driver has to know state (running + not logging) should not
>return any data, and state (running + logging) should return whole
>snapshot first and dirty later. it also has to know qemu migration will
>not call GET_BUFFER in state (running + not logging), otherwise, it has
>to adjust its behavior.

This all just sounds like defining the protocol we expect with the
interface.  For instance if we define a session as beginning when
logging is enabled and ending when the device is stopped and the
interface reports no more data is available, then we can state that any
partial accumulation of data is incomplete relative to migration.  If
userspace wants to initiate a new migration stream, they can simply
toggle logging.  How the vendor driver provides the data during the
session is not defined, but beginning the session with a snapshot
followed by repeated iterations of dirtied data is certainly a valid
approach.

> 2. vendor driver cannot ensure userspace get all the data it intends to
> save in pre-copy phase.
>   e.g. in stop-and-copy phase, vendor driver has to first check and send
>   data in previous phase.

First, I don't think the device has control of when QEMU switches from
pre-copy to stop-and-copy, the protocol needs to support that
transition at any point.  However, it seems a simply data available
counter provides an indication of when it might be optimal to make such
a transition.  If a vendor driver follows a scheme as above, the
available data counter would indicate a large value, the entire initial
snapshot of the device.  As the migration continues and pages are
dirtied, the device would reach a steady state amount of data
available, depending on the guest activity.  This could indicate to the
user to stop the device.  The migration stream would not be considered
completed until the available data counter reaches zero while the
device is in the stopped|logging state.

> 3. if all the sequence is tightly bound to live migration, can we remove the
> logging state? what about adding two states migrate-in and migrate-out?
> so there are four states: running, stopped, migrate-in, migrate-out.
>migrate-out is for source side when migration starts. together with
>state running and stopped, it can substitute state logging.
>migrate-in is for target side.

In fact, Kirti's implementation specifies a data direction, but I think
we still need logging to indicate sessions.  I'd also assume that
logging implies some overhead for the vendor driver.

> On Tue, Mar 12, 2019 at 10:57:47AM +0800, Zhao Yan wrote:
> > hi Alex
> > thanks for your reply.
> > 
> > So, if we choose migration data to be userspace opaque, do you think below
> > sequence is the right behavior for vendor driver to follow:
> > 
> > 1. initially LOGGING state is not set. If userspace calls GET_BUFFER to
> > vendor driver,  vendor driver should reject and return 0.

What would this state mean otherwise?  If we're not logging then it
should not be expected that we can construct dirtied data from a
previous read of the state before logging was enabled (it would be
outside of the "session").  So at best this is an incomplete segment of
the initial snapshot of the device, but that presumes how the vendor
driver constructs the data.  I wouldn't necessarily mandate the vendor
driver reject it, but I think we should consider it undefined and
vendor specific relative to the migration interface.

> > 2. then LOGGING state is set, if userspace calls GET_BUFFER to vendor
> > driver,
> >a. vendor driver shoud first query a whole snapshot of device memory
> >(let's use this term to represent device's standalone memory for now),
> >b. vendor driver returns a chunk of data just queried to userspace,
> >while recording current pos in data.
> >c. vendor driver finds all data just queried is finished transmitting to
> >userspace, and queries only dirty data in device memory now.
> >d. vendor driver returns a chunk of data just quered (this time is dirty
> >data )to userspace while recording current pos in data
> >e. if all data is transmited to usespace and still GET_BUFFERs come from
> >userspace, vendor driver starts another round of dirty data query.

This is a valid vendor driver approach, but it's outside the scope of
the interface definition.  A vendor driver could also decide to not
provide any data until both stopped and logging are set and then
provide a fixed, final snapshot.  The interface supports either
approach by defining the protocol to interact with it.

> > 3. if LOGGING state is unset then, and userpace calls GET_BUFFER to vendor
> > driver,
> >a. if vendor driver finds 

Re: [Qemu-devel] [PATCH for-4.1 0/7] Add qemu_getrandom and ARMv8.5-RNG

[Richard Henderson]

On 3/13/19 11:49 AM, Richard Henderson wrote:
> On 3/13/19 10:38 AM, Markus Armbruster wrote:
>> Have you considered GLib's?  Mersenne Twister under the hood.
>>
>> https://developer.gnome.org/glib/stable/glib-Random-Numbers.html
> 
> I hadn't considered glib's, but I am considering MT19337-64.

Thanks.  glib's version is just as convenient as anything else.
I'll use that for the deterministic path.


r~

Re: [Qemu-devel] [PATCH v5 00/15] s390: vfio-ccw dasd ipl support

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/1552494682-16788-1-git-send-email-jjhe...@linux.ibm.com/



Hi,

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

Type: series
Message-id: 1552494682-16788-1-git-send-email-jjhe...@linux.ibm.com
Subject: [Qemu-devel] [PATCH v5 00/15] s390: vfio-ccw dasd ipl support

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]
patchew/1552494682-16788-1-git-send-email-jjhe...@linux.ibm.com -> 
patchew/1552494682-16788-1-git-send-email-jjhe...@linux.ibm.com
Switched to a new branch 'test'
1bf5971d8e s390-bios: Support booting from real dasd device
4cc01cddd8 s390-bios: Add channel command codes/structs needed for dasd-ipl
aaf1030744 s390-bios: Use control unit type to determine boot method
fb7803ac4e s390-bios: Refactor virtio to run channel programs via cio
fff15a4260 s390-bios: cio error handling
b3cf1d3e43 s390-bios: Support for running format-0/1 channel programs
bd371cb0f7 s390-bios: ptr2u32 and u32toptr
23441a0a32 s390-bios: Map low core memory
951d0fde02 s390-bios: Decouple channel i/o logic from virtio
775a168199 s390-bios: Clean up cio.h
dcd6097827 s390-bios: Factor finding boot device out of virtio code path
dc5858492d s390-bios: Extend find_dev() for non-virtio devices
6e03bf72c8 s390-bios: decouple common boot logic from virtio
7c22a8afb6 s390-bios: decouple cio setup from virtio
814c33c58f s390 vfio-ccw: Add bootindex property and IPLB data

=== OUTPUT BEGIN ===
1/15 Checking commit 814c33c58f8b (s390 vfio-ccw: Add bootindex property and 
IPLB data)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#221: 
new file mode 100644

total: 0 errors, 1 warnings, 199 lines checked

Patch 1/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/15 Checking commit 7c22a8afb68f (s390-bios: decouple cio setup from virtio)
3/15 Checking commit 6e03bf72c85d (s390-bios: decouple common boot logic from 
virtio)
ERROR: externs should be avoided in .c files
#31: FILE: pc-bios/s390-ccw/main.c:19:
+IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));

total: 1 errors, 0 warnings, 65 lines checked

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

4/15 Checking commit dc5858492d9f (s390-bios: Extend find_dev() for non-virtio 
devices)
5/15 Checking commit dcd609782702 (s390-bios: Factor finding boot device out of 
virtio code path)
6/15 Checking commit 775a16819970 (s390-bios: Clean up cio.h)
7/15 Checking commit 951d0fde02f6 (s390-bios: Decouple channel i/o logic from 
virtio)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#29: 
new file mode 100644

total: 0 errors, 1 warnings, 123 lines checked

Patch 7/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
8/15 Checking commit 23441a0a3261 (s390-bios: Map low core memory)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#36: 
new file mode 100644

total: 0 errors, 1 warnings, 104 lines checked

Patch 8/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/15 Checking commit bd371cb0f7d1 (s390-bios: ptr2u32 and u32toptr)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100644

total: 0 errors, 1 warnings, 31 lines checked

Patch 9/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/15 Checking commit b3cf1d3e4384 (s390-bios: Support for running format-0/1 
channel programs)
ERROR: trailing whitespace
#138: FILE: pc-bios/s390-ccw/cio.c:145:
+ * active (generating i/o interrupts). $

total: 1 errors, 0 warnings, 375 lines checked

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

11/15 Checking commit fff15a42609d (s390-bios: cio error handling)
12/15 Checking commit fb7803ac4e99 (s390-bios: Refactor virtio to run channel 
programs via cio)
WARNING: line over 80 characters
#104: FILE: pc-bios/s390-ccw/virtio.c:298:
+run_ccw(vdev, CCW_CMD_READ_VQ_CONF, &config, sizeof(config), 
false) == 0,

WARNING: line over 80 characters
#117: FILE: pc-bios/s390-ccw/virtio.c:308:
+run_ccw(vdev, CCW_CMD_WRITE_STATUS, &status, sizeof(status), false) == 
0,

total: 0 

Re: [Qemu-devel] [PULL 00/22] Bitmaps patches

[Peter Maydell]

On Tue, 12 Mar 2019 at 20:23, John Snow  wrote:
>
> The following changes since commit 46316f1dfffc6be72e94e89f7b0e9162e7dcdcf1:
>
>   Merge remote-tracking branch 
> 'remotes/awilliam/tags/vfio-updates-20190311.0' into staging (2019-03-12 
> 13:37:29 +)
>
> are available in the Git repository at:
>
>   https://github.com/jnsnow/qemu.git tags/bitmaps-pull-request
>
> for you to fetch changes up to e2ec4119dc57e9d65e419b2e9071d35300aa1d92:
>
>   tests/qemu-iotests: add bitmap resize test 246 (2019-03-12 15:00:48 -0400)
>
> 
> Pull request
>

Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0
for any user-visible changes.

-- PMM

Re: [Qemu-devel] [PATCH for-4.1 0/7] Add qemu_getrandom and ARMv8.5-RNG

[Richard Henderson]

On 3/13/19 10:38 AM, Markus Armbruster wrote:
> Have you considered GLib's?  Mersenne Twister under the hood.
> 
> https://developer.gnome.org/glib/stable/glib-Random-Numbers.html

I hadn't considered glib's, but I am considering MT19337-64.


r~

Re: [Qemu-devel] [PATCH v5 00/15] s390: vfio-ccw dasd ipl support

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/1552494682-16788-1-git-send-email-jjhe...@linux.ibm.com/



Hi,

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

Type: series
Message-id: 1552494682-16788-1-git-send-email-jjhe...@linux.ibm.com
Subject: [Qemu-devel] [PATCH v5 00/15] s390: vfio-ccw dasd ipl support

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]
patchew/1552494682-16788-1-git-send-email-jjhe...@linux.ibm.com -> 
patchew/1552494682-16788-1-git-send-email-jjhe...@linux.ibm.com
Switched to a new branch 'test'
14bf2bfa7d s390-bios: Support booting from real dasd device
3eeb0e5573 s390-bios: Add channel command codes/structs needed for dasd-ipl
fda8f8e7a8 s390-bios: Use control unit type to determine boot method
9791caad3e s390-bios: Refactor virtio to run channel programs via cio
ead3fec2b1 s390-bios: cio error handling
4b90526fd3 s390-bios: Support for running format-0/1 channel programs
8d5483ff02 s390-bios: ptr2u32 and u32toptr
2f5c86181a s390-bios: Map low core memory
2a68ea5054 s390-bios: Decouple channel i/o logic from virtio
ccdb223cbb s390-bios: Clean up cio.h
368c7519c9 s390-bios: Factor finding boot device out of virtio code path
5ba43279c8 s390-bios: Extend find_dev() for non-virtio devices
900c23168c s390-bios: decouple common boot logic from virtio
74b23b191a s390-bios: decouple cio setup from virtio
8e9223d357 s390 vfio-ccw: Add bootindex property and IPLB data

=== OUTPUT BEGIN ===
1/15 Checking commit 8e9223d3577b (s390 vfio-ccw: Add bootindex property and 
IPLB data)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#221: 
new file mode 100644

total: 0 errors, 1 warnings, 199 lines checked

Patch 1/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/15 Checking commit 74b23b191aef (s390-bios: decouple cio setup from virtio)
3/15 Checking commit 900c23168c53 (s390-bios: decouple common boot logic from 
virtio)
ERROR: externs should be avoided in .c files
#31: FILE: pc-bios/s390-ccw/main.c:19:
+IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));

total: 1 errors, 0 warnings, 65 lines checked

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

4/15 Checking commit 5ba43279c892 (s390-bios: Extend find_dev() for non-virtio 
devices)
5/15 Checking commit 368c7519c9c8 (s390-bios: Factor finding boot device out of 
virtio code path)
6/15 Checking commit ccdb223cbbda (s390-bios: Clean up cio.h)
7/15 Checking commit 2a68ea50540f (s390-bios: Decouple channel i/o logic from 
virtio)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#29: 
new file mode 100644

total: 0 errors, 1 warnings, 123 lines checked

Patch 7/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
8/15 Checking commit 2f5c86181a01 (s390-bios: Map low core memory)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#36: 
new file mode 100644

total: 0 errors, 1 warnings, 104 lines checked

Patch 8/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/15 Checking commit 8d5483ff028e (s390-bios: ptr2u32 and u32toptr)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100644

total: 0 errors, 1 warnings, 31 lines checked

Patch 9/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/15 Checking commit 4b90526fd317 (s390-bios: Support for running format-0/1 
channel programs)
ERROR: trailing whitespace
#138: FILE: pc-bios/s390-ccw/cio.c:145:
+ * active (generating i/o interrupts). $

total: 1 errors, 0 warnings, 375 lines checked

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

11/15 Checking commit ead3fec2b1ca (s390-bios: cio error handling)
12/15 Checking commit 9791caad3e7c (s390-bios: Refactor virtio to run channel 
programs via cio)
WARNING: line over 80 characters
#104: FILE: pc-bios/s390-ccw/virtio.c:298:
+run_ccw(vdev, CCW_CMD_READ_VQ_CONF, &config, sizeof(config), 
false) == 0,

WARNING: line over 80 characters
#117: FILE: pc-bios/s390-ccw/virtio.c:308:
+run_ccw(vdev, CCW_CMD_WRITE_STATUS, &status, sizeof(status), false) == 
0,

total: 0 

Re: [Qemu-devel] [PATCH v5 00/15] s390: vfio-ccw dasd ipl support

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/1552494682-16788-1-git-send-email-jjhe...@linux.ibm.com/



Hi,

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

Type: series
Message-id: 1552494682-16788-1-git-send-email-jjhe...@linux.ibm.com
Subject: [Qemu-devel] [PATCH v5 00/15] s390: vfio-ccw dasd ipl support

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]
patchew/1552494682-16788-1-git-send-email-jjhe...@linux.ibm.com -> 
patchew/1552494682-16788-1-git-send-email-jjhe...@linux.ibm.com
Switched to a new branch 'test'
cd514b4e60 s390-bios: Support booting from real dasd device
27a733a38f s390-bios: Add channel command codes/structs needed for dasd-ipl
58d77c77cc s390-bios: Use control unit type to determine boot method
f35fdd07de s390-bios: Refactor virtio to run channel programs via cio
461083b940 s390-bios: cio error handling
6d5bd7c785 s390-bios: Support for running format-0/1 channel programs
8db73f2261 s390-bios: ptr2u32 and u32toptr
0054a39d4e s390-bios: Map low core memory
fd6bd00914 s390-bios: Decouple channel i/o logic from virtio
b1f529dacb s390-bios: Clean up cio.h
402c563ccd s390-bios: Factor finding boot device out of virtio code path
0916a051e0 s390-bios: Extend find_dev() for non-virtio devices
cd95f973b3 s390-bios: decouple common boot logic from virtio
424646eba2 s390-bios: decouple cio setup from virtio
29fa9c2874 s390 vfio-ccw: Add bootindex property and IPLB data

=== OUTPUT BEGIN ===
1/15 Checking commit 29fa9c287475 (s390 vfio-ccw: Add bootindex property and 
IPLB data)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#221: 
new file mode 100644

total: 0 errors, 1 warnings, 199 lines checked

Patch 1/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/15 Checking commit 424646eba20c (s390-bios: decouple cio setup from virtio)
3/15 Checking commit cd95f973b3e4 (s390-bios: decouple common boot logic from 
virtio)
ERROR: externs should be avoided in .c files
#31: FILE: pc-bios/s390-ccw/main.c:19:
+IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));

total: 1 errors, 0 warnings, 65 lines checked

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

4/15 Checking commit 0916a051e010 (s390-bios: Extend find_dev() for non-virtio 
devices)
5/15 Checking commit 402c563ccd47 (s390-bios: Factor finding boot device out of 
virtio code path)
6/15 Checking commit b1f529dacb8f (s390-bios: Clean up cio.h)
7/15 Checking commit fd6bd0091402 (s390-bios: Decouple channel i/o logic from 
virtio)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#29: 
new file mode 100644

total: 0 errors, 1 warnings, 123 lines checked

Patch 7/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
8/15 Checking commit 0054a39d4e5f (s390-bios: Map low core memory)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#36: 
new file mode 100644

total: 0 errors, 1 warnings, 104 lines checked

Patch 8/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/15 Checking commit 8db73f2261ed (s390-bios: ptr2u32 and u32toptr)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100644

total: 0 errors, 1 warnings, 31 lines checked

Patch 9/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/15 Checking commit 6d5bd7c78587 (s390-bios: Support for running format-0/1 
channel programs)
ERROR: trailing whitespace
#138: FILE: pc-bios/s390-ccw/cio.c:145:
+ * active (generating i/o interrupts). $

total: 1 errors, 0 warnings, 375 lines checked

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

11/15 Checking commit 461083b9408e (s390-bios: cio error handling)
12/15 Checking commit f35fdd07de40 (s390-bios: Refactor virtio to run channel 
programs via cio)
WARNING: line over 80 characters
#104: FILE: pc-bios/s390-ccw/virtio.c:298:
+run_ccw(vdev, CCW_CMD_READ_VQ_CONF, &config, sizeof(config), 
false) == 0,

WARNING: line over 80 characters
#117: FILE: pc-bios/s390-ccw/virtio.c:308:
+run_ccw(vdev, CCW_CMD_WRITE_STATUS, &status, sizeof(status), false) == 
0,

total: 0 

Re: [Qemu-devel] [PATCH v2 3/4] slirp: remove reference to COPYRIGHT file

[Eric Blake]

On 3/13/19 1:07 PM, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Mar 13, 2019 at 6:57 PM Eric Blake  wrote:
>>
>> On 3/13/19 11:42 AM, Marc-André Lureau wrote:
>>> The slirp COPYRIGHT file is a BSD-3 license.
>>
>> Is it? I only see 2 clauses listed there.
> 
> In commit 2f5f89963186d42a7ded253bc6cf5b32abb45cec ("Remove the
> advertising clause from the slirp license"). the 4th BSD clause was
> removed.
> 
> Daniel Gasparovski & Kelly Price gave permission to license slirp
> under 3-clause BSD.
> 
> So I think we should instead put back the 3rd clause in COPYRIGHT file.
> 
> What do you think?


Checking the history, slirp/COPYRIGHT's initial version in 2004 (commit
5fafdf24) listed only 3 clauses BUT used the poisonous advertising
clause for clause 3 which is the controversial clause of non-free
4-clause (that is, it appears that the BSD-4 license was copied, and
then the WRONG clause was deleted, when creating COPYRIGHT.  Perhaps
explained as an easy mistake to make since 3-clause was created by
removing clause 3 of the 4-clause, where you sometimes see the
three-clause version with clauses 1, 2, 4; but more commonly see a
renumbered version with clauses 1, 2, 3 to close the gap. If you pay
attention only to clause numbers instead of content, it can be easy to
confuse which clause to delete to go from 4-clause to 3-clause).  Commit
2f5f89963 removed the poisonous wrong clause on the grounds of moving
from 4-clause to 3-clause; but did not add the missing clause, which
makes it LOOK like the 2-clause version.  But I think we have a decent
enough trail showing the intent for 3-clause.  Therefore:

Yes, a pre-requisite patch that changes COPYRIGHT to use full 3-clause
wording, with full reference back to Danny's explicit mention of "I have
no objection to having Slirp code in QEMU be licensed under the 3-clause
BSD license.", seems warranted.  Feel free to copy any of my wording
into your commit message.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v5 00/15] s390: vfio-ccw dasd ipl support

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/1552494682-16788-1-git-send-email-jjhe...@linux.ibm.com/



Hi,

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

Type: series
Message-id: 1552494682-16788-1-git-send-email-jjhe...@linux.ibm.com
Subject: [Qemu-devel] [PATCH v5 00/15] s390: vfio-ccw dasd ipl support

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]
patchew/1552494682-16788-1-git-send-email-jjhe...@linux.ibm.com -> 
patchew/1552494682-16788-1-git-send-email-jjhe...@linux.ibm.com
Switched to a new branch 'test'
6ab8ad2ff9 s390-bios: Support booting from real dasd device
a9256d3427 s390-bios: Add channel command codes/structs needed for dasd-ipl
f28f52b209 s390-bios: Use control unit type to determine boot method
29fc48af3f s390-bios: Refactor virtio to run channel programs via cio
2734245df2 s390-bios: cio error handling
e7432cd8da s390-bios: Support for running format-0/1 channel programs
6c1974320c s390-bios: ptr2u32 and u32toptr
bb00e116f1 s390-bios: Map low core memory
c5d71f41f5 s390-bios: Decouple channel i/o logic from virtio
7186dda499 s390-bios: Clean up cio.h
29686c3fd2 s390-bios: Factor finding boot device out of virtio code path
8838da3b5f s390-bios: Extend find_dev() for non-virtio devices
4b35710f4e s390-bios: decouple common boot logic from virtio
6f712b8229 s390-bios: decouple cio setup from virtio
438e215747 s390 vfio-ccw: Add bootindex property and IPLB data

=== OUTPUT BEGIN ===
1/15 Checking commit 438e21574796 (s390 vfio-ccw: Add bootindex property and 
IPLB data)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#221: 
new file mode 100644

total: 0 errors, 1 warnings, 199 lines checked

Patch 1/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/15 Checking commit 6f712b8229b2 (s390-bios: decouple cio setup from virtio)
3/15 Checking commit 4b35710f4efc (s390-bios: decouple common boot logic from 
virtio)
ERROR: externs should be avoided in .c files
#31: FILE: pc-bios/s390-ccw/main.c:19:
+IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));

total: 1 errors, 0 warnings, 65 lines checked

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

4/15 Checking commit 8838da3b5f2f (s390-bios: Extend find_dev() for non-virtio 
devices)
5/15 Checking commit 29686c3fd231 (s390-bios: Factor finding boot device out of 
virtio code path)
6/15 Checking commit 7186dda4998c (s390-bios: Clean up cio.h)
7/15 Checking commit c5d71f41f58f (s390-bios: Decouple channel i/o logic from 
virtio)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#29: 
new file mode 100644

total: 0 errors, 1 warnings, 123 lines checked

Patch 7/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
8/15 Checking commit bb00e116f1c3 (s390-bios: Map low core memory)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#36: 
new file mode 100644

total: 0 errors, 1 warnings, 104 lines checked

Patch 8/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/15 Checking commit 6c1974320cad (s390-bios: ptr2u32 and u32toptr)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100644

total: 0 errors, 1 warnings, 31 lines checked

Patch 9/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/15 Checking commit e7432cd8da91 (s390-bios: Support for running format-0/1 
channel programs)
ERROR: trailing whitespace
#138: FILE: pc-bios/s390-ccw/cio.c:145:
+ * active (generating i/o interrupts). $

total: 1 errors, 0 warnings, 375 lines checked

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

11/15 Checking commit 2734245df2eb (s390-bios: cio error handling)
12/15 Checking commit 29fc48af3f06 (s390-bios: Refactor virtio to run channel 
programs via cio)
WARNING: line over 80 characters
#104: FILE: pc-bios/s390-ccw/virtio.c:298:
+run_ccw(vdev, CCW_CMD_READ_VQ_CONF, &config, sizeof(config), 
false) == 0,

WARNING: line over 80 characters
#117: FILE: pc-bios/s390-ccw/virtio.c:308:
+run_ccw(vdev, CCW_CMD_WRITE_STATUS, &status, sizeof(status), false) == 
0,

total: 0 

Re: [Qemu-devel] [PATCH for-4.1 0/7] Add qemu_getrandom and ARMv8.5-RNG

[Richard Henderson]

On 3/13/19 10:57 AM, Daniel P. Berrangé wrote:
> We already have an internal API for providing strong random bytes in
> QEMU qcrypto_random_bytes. It is preferentially backed by gnutls or
> gcrypt, but if those aren't built-in it falls back to a platform
> native API like /dev/random. I've got a todo item to make that use
> getrandom on Linux/BSD when available.
> 
> I don't think we should be adding a new APIs for getting random
> numbers that aren't backed by the qcrypto_random_bytes.

That's all very well and good for one particular use case, when we really want
random numbers.  But with -seed, we want to be able to replicate one particular
execution, with fully deterministic numbers.

But certainly I can look into making the non-deterministic execution use your
existing interface.  I simply wasn't aware of it.

Do you happen to know off-hand the maximum latency of the gnutls/gcrypt
interfaces?  I do want the interface to be able to return "timed-out" in
certain cases.  With getrandom(2) there is a handy GRND_NONBLOCK parameter that
pretty much matches exactly what I want.  With /dev/{u,}random, one would have
to use O_NONBLOCK.  With BSD getentropy, I think you'd need an alarm to time
out the operation (unless EIO covers all sorts of failures like empty entropy
pool; the manual isn't clear)?


r~

Re: [Qemu-devel] [PATCH v5 00/15] s390: vfio-ccw dasd ipl support

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/1552494682-16788-1-git-send-email-jjhe...@linux.ibm.com/



Hi,

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

Type: series
Message-id: 1552494682-16788-1-git-send-email-jjhe...@linux.ibm.com
Subject: [Qemu-devel] [PATCH v5 00/15] s390: vfio-ccw dasd ipl support

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]
patchew/1552494682-16788-1-git-send-email-jjhe...@linux.ibm.com -> 
patchew/1552494682-16788-1-git-send-email-jjhe...@linux.ibm.com
 t [tag update]
patchew/20190313164207.25166-1-marcandre.lur...@redhat.com -> 
patchew/20190313164207.25166-1-marcandre.lur...@redhat.com
Switched to a new branch 'test'
4b8f0d8ca1 s390-bios: Support booting from real dasd device
58d89a48aa s390-bios: Add channel command codes/structs needed for dasd-ipl
27184019e9 s390-bios: Use control unit type to determine boot method
e89de02f38 s390-bios: Refactor virtio to run channel programs via cio
4a9e3f3293 s390-bios: cio error handling
0e3c357024 s390-bios: Support for running format-0/1 channel programs
e026f9363d s390-bios: ptr2u32 and u32toptr
69b1199cb4 s390-bios: Map low core memory
81c99b9681 s390-bios: Decouple channel i/o logic from virtio
8cc7aeaf32 s390-bios: Clean up cio.h
eb50486bcc s390-bios: Factor finding boot device out of virtio code path
f01ef093e9 s390-bios: Extend find_dev() for non-virtio devices
560a2985c0 s390-bios: decouple common boot logic from virtio
c4b6e8b164 s390-bios: decouple cio setup from virtio
2a16b06b29 s390 vfio-ccw: Add bootindex property and IPLB data

=== OUTPUT BEGIN ===
1/15 Checking commit 2a16b06b291d (s390 vfio-ccw: Add bootindex property and 
IPLB data)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#221: 
new file mode 100644

total: 0 errors, 1 warnings, 199 lines checked

Patch 1/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/15 Checking commit c4b6e8b16493 (s390-bios: decouple cio setup from virtio)
3/15 Checking commit 560a2985c0e9 (s390-bios: decouple common boot logic from 
virtio)
ERROR: externs should be avoided in .c files
#31: FILE: pc-bios/s390-ccw/main.c:19:
+IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));

total: 1 errors, 0 warnings, 65 lines checked

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

4/15 Checking commit f01ef093e92f (s390-bios: Extend find_dev() for non-virtio 
devices)
5/15 Checking commit eb50486bcc42 (s390-bios: Factor finding boot device out of 
virtio code path)
6/15 Checking commit 8cc7aeaf32a1 (s390-bios: Clean up cio.h)
7/15 Checking commit 81c99b9681c0 (s390-bios: Decouple channel i/o logic from 
virtio)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#29: 
new file mode 100644

total: 0 errors, 1 warnings, 123 lines checked

Patch 7/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
8/15 Checking commit 69b1199cb4d5 (s390-bios: Map low core memory)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#36: 
new file mode 100644

total: 0 errors, 1 warnings, 104 lines checked

Patch 8/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/15 Checking commit e026f9363d16 (s390-bios: ptr2u32 and u32toptr)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100644

total: 0 errors, 1 warnings, 31 lines checked

Patch 9/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/15 Checking commit 0e3c3570241d (s390-bios: Support for running format-0/1 
channel programs)
ERROR: trailing whitespace
#138: FILE: pc-bios/s390-ccw/cio.c:145:
+ * active (generating i/o interrupts). $

total: 1 errors, 0 warnings, 375 lines checked

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

11/15 Checking commit 4a9e3f329374 (s390-bios: cio error handling)
12/15 Checking commit e89de02f3828 (s390-bios: Refactor virtio to run channel 
programs via cio)
WARNING: line over 80 characters
#104: FILE: pc-bios/s390-ccw/virtio.c:298:
+run_ccw(vdev, CCW_CMD_READ_VQ_CONF, &config, sizeof(config), 
false) == 0,

WARNING: line over 8

Re: [Qemu-devel] [PATCH] xen-block: Replace qdict_put_obj() by qdict_put() where appropriate

[Eric Blake]

On 3/13/19 12:44 PM, Markus Armbruster wrote:
> Patch created mechanically by rerunning:
> 
> $ spatch --sp-file scripts/coccinelle/qobject.cocci \
>  --macro-file scripts/cocci-macro-file.h \
>  --dir hw/block --in-place
> 
> Signed-off-by: Markus Armbruster 
> ---
>  hw/block/xen-block.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Reviewed-by: Eric Blake 

Safe for 4.0 softfreeze in my opinion, but also harmless if it slips to 4.1.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] slirp: remove empty state.h

[Eric Blake]

On 3/13/19 12:39 PM, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau 
> ---
>  slirp/src/state.h | 0
>  slirp/src/state.c | 1 -
>  2 files changed, 1 deletion(-)
>  delete mode 100644 slirp/src/state.h

Made empty in commit d890344, before moving to its current location in
c2d63650.

Reviewed-by: Eric Blake 

> 
> diff --git a/slirp/src/state.h b/slirp/src/state.h
> deleted file mode 100644
> index e69de29bb2..00
> diff --git a/slirp/src/state.c b/slirp/src/state.c
> index f5dd80cdc8..c3e3f0b671 100644
> --- a/slirp/src/state.c
> +++ b/slirp/src/state.c
> @@ -23,7 +23,6 @@
>   */
>  #include "slirp.h"
>  #include "vmstate.h"
> -#include "state.h"
>  #include "stream.h"
>  
>  static int slirp_tcp_post_load(void *opaque, int version)
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] Clean up includes

[Eric Blake]

On 3/13/19 11:28 AM, Markus Armbruster wrote:
> Clean up includes so that osdep.h is included first and headers
> which it implies are not included manually.
> 
> This commit was created with scripts/clean-includes, with the changes
> to the following files manually reverted:

Is it worth tweaking scripts/clean-includes as a pre-req patch so that
some of these files become listed exceptions and don't need manual
reversion here?

> 
> contrib/libvhost-user/libvhost-user-glib.h
> contrib/libvhost-user/libvhost-user.c
> contrib/libvhost-user/libvhost-user.h
> linux-user/mips64/cpu_loop.c
> linux-user/mips64/signal.c
> linux-user/sparc64/cpu_loop.c
> linux-user/sparc64/signal.c
> linux-user/x86_64/cpu_loop.c
> linux-user/x86_64/signal.c
> slirp/src/*
> target/s390x/gen-features.c
> tests/migration/s390x/a-b-bios.c
> tests/test-rcu-simpleq.c
> tests/test-rcu-tailq.c
> tests/uefi-test-tools/UefiTestToolsPkg/BiosTablesTest/BiosTablesTest.c
> 
> We're in the process of spinning out slirp/.  tests/uefi-test-tools/
> is guest software.  The remaining reverts are the same as in commit
> b7d89466dde.
> 
> Signed-off-by: Markus Armbruster 
> ---

> +++ b/contrib/elf2dmp/pdb.c
> @@ -18,9 +18,8 @@
>   * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA

Unrelated, but if someone is looking for a project, cleaning up stale
address references in older GPL copy-and-paste is a possibility.

I think these changes are okay for 4.0 soft freeze; but it also doesn't
hurt if this delays into 4.1.

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 01/17] util: add helper APIs for dealing with inotify in portable manner

[Daniel P . Berrangé]

On Wed, Mar 13, 2019 at 01:19:58PM -0400, Bandan Das wrote:
> Daniel P. Berrangé  writes:
> 
> > On Tue, Mar 12, 2019 at 07:07:42PM -0400, Bandan Das wrote:
> >> Daniel P. Berrangé  writes:
> >> ...
> >> > +
> >> > +int
> >> > +qemu_file_monitor_add_watch(QFileMonitor *mon,
> >> > +const char *dirpath,
> >> > +const char *filename,
> >> > +QFileMonitorHandler cb,
> >> > +void *opaque,
> >> > +Error **errp)
> >> > +{
> >> > +QFileMonitorDir *dir;
> >> > +QFileMonitorWatch watch;
> >> > +int ret = -1;
> >> > +
> >> > +qemu_mutex_lock(&mon->lock);
> >> > +dir = g_hash_table_lookup(mon->dirs, dirpath);
> >> > +if (!dir) {
> >> > +int rv = inotify_add_watch(mon->fd, dirpath,
> >> > +   IN_CREATE | IN_DELETE | IN_MODIFY |
> >> > +   IN_MOVED_TO | IN_MOVED_FROM | 
> >> > IN_ATTRIB);
> >> > +
> >> > +if (rv < 0) {
> >> > +error_setg_errno(errp, errno, "Unable to watch '%s'", 
> >> > dirpath);
> >> > +goto cleanup;
> >> > +}
> >> > +
> >> > +trace_qemu_file_monitor_enable_watch(mon, dirpath, rv);
> >> > +
> >> > +dir = g_new0(QFileMonitorDir, 1);
> >> > +dir->path = g_strdup(dirpath);
> >> > +dir->id = rv;
> >> > +dir->watches = g_array_new(FALSE, TRUE, 
> >> > sizeof(QFileMonitorWatch));
> >> > +
> >> > +g_hash_table_insert(mon->dirs, dir->path, dir);
> >> > +g_hash_table_insert(mon->idmap, GINT_TO_POINTER(rv), dir);
> >> > +
> >> > +if (g_hash_table_size(mon->dirs) == 1) {
> >> > +qemu_set_fd_handler(mon->fd, qemu_file_monitor_watch, NULL, 
> >> > mon);
> >> > +}
> >> > +}
> >> > +
> >> > +watch.id = dir->nextid++;
> >> > +watch.filename = g_strdup(filename);
> >> > +watch.cb = cb;
> >> > +watch.opaque = opaque;
> >> > +
> >> > +g_array_append_val(dir->watches, watch);
> >> > +
> >> > +trace_qemu_file_monitor_add_watch(mon, dirpath,
> >> > +  filename ? filename : "",
> >> > +  cb, opaque, watch.id);
> >> > +
> >> > +ret = watch.id;
> >> > +
> >> 
> >> This seems to break usb-mtp since we are returning watch.id.
> >> Why are we not returning dir->id here ? usb-mtp relies on the watch
> >> descriptor to handle events.
> >
> > dir->id is the low level inotify identifier. This is not supposed to be
> > visible to any calling code, since that should instead be using the
> > QEMU generated watch.id value.
> >
> 
> I guessed so even though I had the urge to just post a patch and return
> dir->id :)
> 
> > Can you give more info on how usb-mtp broke ?  I tested USB MTP before
> > sending this series and I saw it correctly detecting create / delete
> > of files & reflecting this in the guest.
> >
> This is what I think is happening:
> 
> Consider the mtproot to be /root/mtpshare and following directory hierarchy -
> /root/mtpshare/dir1
> 
> Initiator:
> User enters dir1
> Host:
> usb_mtp_object_readdir is called which calls qemu_file_monitor_add_watch()
> qemu_file_monitor_add_watch returns 0. 
> 
> Initiator:
> User attempts to create a directory "dir2" in dir1.
> Host:
> The callback file_monitor_event() is invoked, id is 0.
> file_monitor_event calls:
>MTPObject *parent = usb_mtp_object_lookup_id(s, 0);
> which returns the object associated with /root/mtpshare.   
> Further below, usb_mtp_add_child() will try to add the newly
> created object but it will fail because it's looking for it
> in /root/mtpshare(watchid=0) not /root/mtpshare/dir1.
> 
> I believe the problem is that adding a watch point isn't returning
> a unique identifier to identify the watch.

Yeah, I've found the problem and will send a fix tomorrow once I have
unit tests for it.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH] intel-iommu: optimize nodmar memory regions

[Paolo Bonzini]

On 13/03/19 12:45, Sergio Lopez wrote:
> 
> Peter Xu writes:
> 
>> Previously we have per-device system memory aliases when DMAR is
>> disabled by the system.  It will slow the system down if there are
>> lots of devices especially when DMAR is disabled, because each of the
>> aliased system address space will contain O(N) slots, and rendering
>> such N address spaces will be O(N^2) complexity.
>>
>> This patch introduces a shared nodmar memory region and for each
>> device we only create an alias to the shared memory region.  With the
>> aliasing, QEMU memory core API will be able to detect when devices are
>> sharing the same address space (which is the nodmar address space)
>> when rendering the FlatViews and the total number of FlatViews can be
>> dramatically reduced when there are a lot of devices.
>>
>> Suggested-by: Paolo Bonzini 
>> Signed-off-by: Peter Xu 
>> ---
>>
>> Hi, Sergio,
>>
>> This patch implements the optimization that Paolo proposed in the
>> other thread.  Would you please try this patch to see whether it could
>> help for your case?  Thanks,
> 
> Hi,
> 
> I've just gave a try and it fixes the issue here. The number of
> FlatViews goes down from 119 to 4, and the initialization time for PCI
> devices on the Guest is back to normal levels.

You must be using "iommu=pt" then.  Can you also test performance
without it?  It should be fine even if the number of FlatViews goes back
to 119.

Paolo