Re: [PULL 0/2] 2021-03-18 COLO proxy patches

[Jason Wang]

在 2021/3/18 下午12:11, Zhang Chen 写道:

Hi Jason, please merge this series to net queue.

Lukas Straub (2):
   net/colo-compare.c: Fix memory leak for non-tcp packet
   net/colo-compare.c: Optimize removal of secondary packet

  net/colo-compare.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)



Applied.

Thanks

[Bug 1880518] Re: issue while installing docker inside s390x container

[Thomas Huth]

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

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

Title:
  issue while installing docker inside s390x container

Status in QEMU:
  New

Bug description:
  This is in reference to 
https://github.com/multiarch/qemu-user-static/issues/108.
  I am facing issue while installing docker inside s390x container under qemu 
on Ubuntu 18.04 host running on amd64.
  Following are the contents of /proc/sys/fs/binfmt_misc/qemu-s390x on Intel 
host after running 
  docker run --rm --privileged multiarch/qemu-user-static --reset -p yes
  enabled
  interpreter /usr/bin/qemu-s390x-static
  flags: F
  offset 0
  magic 7f454c46020201020016
  mask ff00fffe
  I could get docker service up with the following trick. 
  printf '{"iptables": false,"ip-masq": false,"bridge": "none" }' > 
/etc/docker/daemon.json
  But even though docker service comes up, images are not getting pulled, 
docker pull fails with the following error.
  failed to register layer: Error processing tar file(exit status 1):

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

Re: [PATCH v1] MAINTAINERS: Fix tests/migration maintainers

[Thomas Huth]

On 19/03/2021 03.25, huang...@chinatelecom.cn wrote:

From: Hyman Huang(黄勇) 

Signed-off-by: Hyman Huang(黄勇) 
---
  MAINTAINERS | 1 +
  1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 25fc49d1dc..20e2387c66 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2525,6 +2525,7 @@ M: Cleber Rosa 
  S: Odd Fixes
  F: scripts/*.py
  F: tests/*.py
+F: tests/migration/


 Hi,

that looks like you've added it to the "Python" section ... but I think this 
should rather be be added to the "Migration" section instead?


 Thomas

Re: [PATCH 1/1] iotests: fix 051.out expected output after error text touchups

[Christian Borntraeger]

On 18.03.21 21:09, Connor Kuehl wrote:

A patch was recently applied that touched up some error messages that
pertained to key names like 'node-name'. The trouble is it only updated
tests/qemu-iotests/051.pc.out and not tests/qemu-iotests/051.out as
well.

Do that now.

Fixes: 785ec4b1b9 ("block: Clarify error messages pertaining to
'node-name'")
Signed-off-by: Connor Kuehl 


Tested-by: Christian Borntraeger 

Thanks for the quick response.


---
  tests/qemu-iotests/051.out | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index de4771bcb3..db8c14b903 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -61,13 +61,13 @@ QEMU X.Y.Z monitor - type 'help' for more information
  (qemu) quit
  
  Testing: -drive file=TEST_DIR/t.qcow2,node-name=123foo

-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,node-name=123foo: Invalid node name
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,node-name=123foo: Invalid node-name: 
'123foo'
  
  Testing: -drive file=TEST_DIR/t.qcow2,node-name=_foo

-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,node-name=_foo: Invalid node name
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,node-name=_foo: Invalid node-name: 
'_foo'
  
  Testing: -drive file=TEST_DIR/t.qcow2,node-name=foo#12

-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,node-name=foo#12: Invalid node name
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,node-name=foo#12: Invalid node-name: 
'foo#12'
  
  
  === Device without drive ===

Re: [RFC] adding a generic QAPI event for failed device hotunplug

[Markus Armbruster]

Markus Armbruster  writes:

> David Gibson  writes:
>
>> On Thu, Mar 11, 2021 at 05:50:42PM -0300, Daniel Henrique Barboza wrote:
>>> 
>>> 
>>> On 3/9/21 3:22 AM, Markus Armbruster wrote:
>>> > Cc: Paolo and Julia in addition to Igor, because the thread is wandering
>>> > towards DeviceState member pending_deleted_event.
>>> > 
>>> > Cc: Laine for libvirt expertise.  Laine, if you're not the right person,
>>> > please loop in the right person.
>>> > 
>>> > David Gibson  writes:
>>> > 
>>> > > On Mon, Mar 08, 2021 at 03:01:53PM -0300, Daniel Henrique Barboza wrote:
>>> > > > 
>>> > > > 
>>> > > > On 3/8/21 2:04 PM, Markus Armbruster wrote:
>>> > > > > Daniel Henrique Barboza  writes:
>>> > > > > 
>>> > > > > > On 3/6/21 3:57 AM, Markus Armbruster wrote:
>>> > [...]
>>> > > > > > > We should document the event's reliability.  Are there unplug 
>>> > > > > > > operations
>>> > > > > > > where we can't detect failure?  Are there unplug operations 
>>> > > > > > > where we
>>> > > > > > > could, but haven't implemented the event?
>>> > > > > > 
>>> > > > > > The current version of the PowerPC spec that the pseries machine 
>>> > > > > > implements
>>> > > > > > (LOPAR) does not predict a way for the virtual machine to report 
>>> > > > > > a hotunplug
>>> > > > > > error back to the hypervisor. If something fails, the hypervisor 
>>> > > > > > is left
>>> > > > > > in the dark.
>>> > > > > > 
>>> > > > > > What happened in the 6.0.0 dev cycle is that we faced a reliable 
>>> > > > > > way of
>>> > > > > 
>>> > > > > Do you mean "unreliable way"?
>>> > > > 
>>> > > > I guess a better word would be 'reproducible', as in we discovered a 
>>> > > > reproducible
>>> > > > way of getting the guest kernel to refuse the CPU hotunplug.
>>> > > 
>>> > > Right.  It's worth noting here that in the PAPR model, there are no
>>> > > "forced" unplugs.  Unplugs always consist of a request to the guest,
>>> > > which is then resposible for offlining the device and signalling back
>>> > > to the hypervisor that it's done with it.
>>> > > 
>>> > > > > > making CPU hotunplug fail in the guest (trying to hotunplug the 
>>> > > > > > last online
>>> > > > > > CPU) and the pseries machine was unprepared for dealing with it. 
>>> > > > > > We ended up
>>> > > > > > implementing a hotunplug timeout and, if the timeout kicks in, 
>>> > > > > > we're assuming
>>> > > > > > that the CPU hotunplug failed in the guest. This is the first 
>>> > > > > > scenario we have
>>> > > > > > today where we want to send a QAPI event informing the CPU 
>>> > > > > > hotunplug error,
>>> > > > > > but this event does not exist in QEMU ATM.
>>> > > > > 
>>> > > > > When the timeout kicks in, how can you know the operation failed?  
>>> > > > > You
>>> > > > > better be sure when you send an error event.  In other words: what
>>> > > > > prevents the scenario where the operation is much slower than you
>>> > > > > expect, the timeout expires, the error event is sent, and then the
>>> > > > > operation completes successfully?
>>> > > > 
>>> > > > A CPU hotunplug in a pseries guest takes no more than a couple of 
>>> > > > seconds, even
>>> > > > if the guest is under heavy load. The timeout is set to 15 seconds.
>>> > > 
>>> > > Right.  We're well aware that a timeout is an ugly hack, since it's
>>> > > not really possible to distinguish it from a guest that's just being
>>> > > really slow.
>>> > 
>>> > As long as unplug failure cannot be detected reliably, we need a timeout
>>> > *somewhere*.  I vaguely recall libvirt has one.  Laine?
>>> 
>>> Yeah, Libvirt has a timeout for hotunplug operations. I agree that QEMU 
>>> doing
>>> the timeout makes more sense since it has more information about the
>>> conditions/mechanics involved.
>>
>> Right.  In particular, I can't really see how libvirt can fully
>> implement that timeout.  AFAIK qemu has no way of listing or
>> cancelling "in flight" unplug requests, so it's entirely possible that
>> the unplug could still complete after libvirt's has "timed out".
>
> QEMU doesn't really keep track of "in flight" unplug requests, and as
> long as that's the case, its timeout even will have the same issue.

If we change QEMU to keep track of "in flight" unplug requests, then we
likely want QMP commands to query and cancel them.

Instead of inventing ad hoc commands, we should look into using the job
framework: query-jobs, job-cancel, ...  See qapi/job.json.

Bonus: we don't need new events, existing JOB_STATUS_CHANGE can do the
job (pardon the pun).

[...]

[PATCH v2] MAINTAINERS: Fix tests/migration maintainers

[huangy81]

From: Hyman Huang(黄勇) 

when executing the following scripts, it throw error message:
$ ./scripts/get_maintainer.pl -f tests/migration/guestperf.py
get_maintainer.pl: No maintainers found, printing recent contributors.
get_maintainer.pl: Do not blindly cc: them on patches!  Use common sense.

add the tests/migration to the "Migration" section of MAINTAINERS

Signed-off-by: Hyman Huang(黄勇) 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 25fc49d1dc..d3f3edb47d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2724,6 +2724,7 @@ F: tests/vmstate-static-checker-data/
 F: tests/qtest/migration-test.c
 F: docs/devel/migration.rst
 F: qapi/migration.json
+F: tests/migration/
 
 D-Bus
 M: Marc-André Lureau 
-- 
2.24.3

Re: arm_cpu_post_init (Was: Re: arm: "max" CPU class hierarchy changes possible?)

[Claudio Fontana]

On 3/18/21 2:10 PM, Eduardo Habkost wrote:
> On Thu, Mar 18, 2021 at 01:59:08PM +0100, Andrew Jones wrote:
>> On Thu, Mar 18, 2021 at 01:42:36PM +0100, Claudio Fontana wrote:
>>> On 3/18/21 1:08 PM, Andrew Jones wrote:
 On Thu, Mar 18, 2021 at 12:32:30PM +0100, Claudio Fontana wrote:
> And why do we have a separate arm_cpu_finalize_features()?

 Separate, because it's not just called from arm_cpu_realizefn().
>>>
>>> In particular it is also called by the monitor.c in 
>>> qmp_query_cpu_model_expansion(),
>>>
>>> which basically creates an object of the cpu subclass,
>>> and then calls arm_cpu_finalize_[features]() explicitly on the object.
>>>
>>> Is the qdev realize() method not called in this case? Should instead it be 
>>> triggered, rather than initializing/realizing an incomplete object?
>>
>> Can you elaborate on what you mean by "triggered"? The QMP query does the
>> least that it can get away with while still reusing the CPU model's
>> feature initialization code. Any suggestions for improving that,
>> preferably in the form of a patch, would be welcome. If it works well for
>> Arm, then it could probably be applied to other architectures. The Arm QMP
>> query is modeled off the others.
> 
> This sound very similar to x86_cpu_expand_features(), so the
> approach makes sense to me.

Interesting, to me it sounds like a CPUClass method is hiding here, 
cc->cpu_expand_features(),
I could help kickstart the implementation but would need a good description / 
comment of exactly which features are supposed to be expanded there. 

> 
> It wouldn't make sense to call realize() inside
> qmp_query_cpu_model_expansion().  Realizing the CPU means
> plugging it into the guest, and we would never want to do that
> when executing a query command.
> 

Makes sense, thanks for the explanation.

Ciao,

Claudio

Re: arm_cpu_post_init (Was: Re: arm: "max" CPU class hierarchy changes possible?)

[Claudio Fontana]

Hi Markus,

could you help me untangle the arm_cpu_post_init question?

I am trying to cleanup a bit the initialization path for ARM,
and it seems that arm_cpu_post_init is called numerous times for AArch64 in 
particular,

while for "tcg cpus", 32bit it is called only once.

Any reason for the multiple calls in the hierarchy?
Was the intention to actually call this just once from the final leaf classes?

The ability to execute code after the initialization would come in handy in an 
ARM CPU class refactoring I am doing,
but I stopped short of adding anything to arm_cpu_post_init since I noticed the 
inconsistencies.

Thanks,

Claudio


On 3/18/21 12:06 PM, Claudio Fontana wrote:
> On 3/11/21 8:10 PM, Andrew Jones wrote:
>> On Thu, Mar 11, 2021 at 06:33:15PM +, Peter Maydell wrote:
>>> On Thu, 11 Mar 2021 at 17:16, Claudio Fontana  wrote:
 Maybe Peter you could clarify similarly what the intended meaning of "max" 
 is on ARM?
>>>
>>> "max" is "best we can do, whatever that is". (On KVM this is "same as
>>> the host".)
>>> "host" is "whatever the host is (KVM only)".
>>>
 KVM: (aarch64-only): aarch64_max_initfn():

 The following comment in the code seems wrong to me:

 /* -cpu max: if KVM is enabled, like -cpu host (best possible with this 
 host); */

 This is not exactly true:

 "-cpu max" calls kvm_arm_set_cpu_features_from_host(), (which checks 
 "dtb_compatible", and if not set gets the features from the host, if set 
 ...?)
 After that, calls aarch64_add_sve_properties() and then adds also 
 "svw-max-vq". This code is common with TCG.
> 
> 
> As part of this research I noticed that arm_cpu_post_init() is quite 
> confusing, seems really inconsistent to me.
> 
> Apparently the intention was to call it from the leaf classes:
> 
> commit 51e5ef459eca045d7e8afe880ee60190f0b75b26
> Author: Marc-André Lureau 
> Date:   Tue Nov 27 12:55:59 2018 +0400
> 
> arm: replace instance_post_init()
> 
> Replace arm_cpu_post_init() instance callback by calling it from leaf
> classes, to avoid potential ordering issue with other post_init callbacks.
> 
> Signed-off-by: Marc-André Lureau 
> Suggested-by: Igor Mammedov 
> Reviewed-by: Igor Mammedov 
> Acked-by: Eduardo Habkost 
> 
> 
> but then we end up calling it multiple times in the class hierarch, which is 
> a recipe for bugs, and makes it difficult to understand what 
> arm_cpu_post_init()
> even means, what calling this function is supposed to do.
> 
> For a "max" or "host" cpu on AArch64, this function is called:
> 
> for the ARM CPU base class, TYPE_ARM_CPU, in
> 
> cpu.c::arm_cpu_instance_init,
> 
> then later again for the TYPE_AARCH64_CPU class, child of TYPE_ARM_CPU, in
> 
> cpu64.c::aarch64_cpu_instance_init,
> 
> then later again for the TYPE_ARM_HOST_CPU class, child of TYPE_AARCH64_CPU, 
> in
> 
> cpu.c::arm_host_initfn.
> 
> Same for "max".
> 
> When looking at 32bit CPUs instead, only the ARM CPU base class ends up 
> calling arm_cpu_post_init.
> "Leaf" classes do not do it (see cpu_tcg.c).
> 
> What is then arm_cpu_post_init even supposed to mean?
> 
> Thanks,
> 
> Claudio
> 
> 

 In the case of cpu host instead,

 "-cpu host" calls kvm_arm_set_cpu_features_from_host(), same as max, then 
 calls aarch64_add_sve_properties() but does NOT add "svw-max-vq".

 Is this a bug?
>>
>> It was left out intentionally. More below.
>>
>>>
>>> Maybe; that's a question for Richard or Drew...
>>>
 Are "max" and "host" for KVM supposed to be the same like with x86?
>>
>> Yes, but my understanding of "max" == "host" for KVM is that that only
>> applies to the perspective of the guest. What CPU and what CPU features
>> the guest can see should be exactly the same with either "max" or "host",
>> depending on the enabling/disabling of any optional CPU properties.
>>
>> The question here seems to be that, if one has a CPU property, does that
>> imply the other should have the same? Which would effectively allow the
>> two to be aliases (when KVM is enabled). I don't know, does x86 ensure
>> 100% property compatibility?
>>
>> I opted not to support sve-max-vq for "host" because I consider it a
>> legacy CPU property, one I didn't want to propagate. Indeed it may
>> make more sense to depreciate sve-max-vq than to "fix" this issue
>> by adding it to "host". Note, we can already create equivalent SVE
>> CPUs. The following are the same from the perspective of the guest
>>
>>  -accel kvm -cpu host,sve512=on
>>  -accel kvm -cpu max,sve512=on
>>
>> And, for TCG, these are the same from the perspective of the guest
>>  
>>  -accel tcg -cpu max,sve512=on
>>  -accel tcg -cpu max,sve-max-vq=4
>>
>> So we already don't need sve-max-vq.
>>
>> Thanks,
>> drew
>>
> 
> 

Re: [PATCH 2/3] migration: Inhibit virtio-balloon for the duration of background snapshot

[Andrey Gruzdev]

On 18.03.2021 21:16, David Hildenbrand wrote:

On 18.03.21 18:46, Andrey Gruzdev wrote:

The same thing as for incoming postcopy - we cannot deal with concurrent
RAM discards when using background snapshot feature in outgoing 
migration.


Signed-off-by: Andrey Gruzdev 
---
  hw/virtio/virtio-balloon.c | 8 ++--
  include/migration/misc.h   | 2 ++
  migration/migration.c  | 8 
  3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index e770955176..d120bf8f43 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -66,8 +66,12 @@ static bool 
virtio_balloon_pbp_matches(PartiallyBalloonedPage *pbp,

    static bool virtio_balloon_inhibited(void)
  {
-    /* Postcopy cannot deal with concurrent discards, so it's 
special. */
-    return ram_block_discard_is_disabled() || 
migration_in_incoming_postcopy();

+    /*
+ * Postcopy cannot deal with concurrent discards,
+ * so it's special, as well as background snapshots.
+ */
+    return ram_block_discard_is_disabled() || 
migration_in_incoming_postcopy() ||

+    migration_in_bg_snapshot();
  }
    static void balloon_inflate_page(VirtIOBalloon *balloon,
diff --git a/include/migration/misc.h b/include/migration/misc.h
index bccc1b6b44..738675ef52 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -70,6 +70,8 @@ bool 
migration_in_postcopy_after_devices(MigrationState *);

  void migration_global_dump(Monitor *mon);
  /* True if incomming migration entered POSTCOPY_INCOMING_DISCARD */
  bool migration_in_incoming_postcopy(void);
+/* True if background snapshot is active */
+bool migration_in_bg_snapshot(void);
    /* migration/block-dirty-bitmap.c */
  void dirty_bitmap_mig_init(void);
diff --git a/migration/migration.c b/migration/migration.c
index 496cf6e17b..656d6249a6 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1976,6 +1976,14 @@ bool migration_in_incoming_postcopy(void)
  return ps >= POSTCOPY_INCOMING_DISCARD && ps < 
POSTCOPY_INCOMING_END;

  }
  +bool migration_in_bg_snapshot(void)
+{
+    MigrationState *s = migrate_get_current();
+
+    return migrate_background_snapshot() &&
+    migration_is_setup_or_active(s->state);
+}
+
  bool migration_is_idle(void)
  {
  MigrationState *s = current_migration;



Reviewed-by: David Hildenbrand 


Thanks!

--
Andrey Gruzdev, Principal Engineer
Virtuozzo GmbH  +7-903-247-6397
virtuzzo.com

Re: arm_cpu_post_init (Was: Re: arm: "max" CPU class hierarchy changes possible?)

[Claudio Fontana]

On 3/19/21 9:23 AM, Claudio Fontana wrote:
> Hi Markus,
> 
> could you help me untangle the arm_cpu_post_init question?

Nevermind, I think I figured it out. The arm_cpu_post_init are indeed called 
only for the "leaf" class,
via the "instance_init" functions.

I think I can use it to do things reliably "post init" for all classes in there.

Thanks,

Claudio

> 
> I am trying to cleanup a bit the initialization path for ARM,
> and it seems that arm_cpu_post_init is called numerous times for AArch64 in 
> particular,
> 
> while for "tcg cpus", 32bit it is called only once.
> 
> Any reason for the multiple calls in the hierarchy?
> Was the intention to actually call this just once from the final leaf classes?
> 
> The ability to execute code after the initialization would come in handy in 
> an ARM CPU class refactoring I am doing,
> but I stopped short of adding anything to arm_cpu_post_init since I noticed 
> the inconsistencies.
> 
> Thanks,
> 
> Claudio
> 
> 
> On 3/18/21 12:06 PM, Claudio Fontana wrote:
>> On 3/11/21 8:10 PM, Andrew Jones wrote:
>>> On Thu, Mar 11, 2021 at 06:33:15PM +, Peter Maydell wrote:
 On Thu, 11 Mar 2021 at 17:16, Claudio Fontana  wrote:
> Maybe Peter you could clarify similarly what the intended meaning of 
> "max" is on ARM?

 "max" is "best we can do, whatever that is". (On KVM this is "same as
 the host".)
 "host" is "whatever the host is (KVM only)".

> KVM: (aarch64-only): aarch64_max_initfn():
>
> The following comment in the code seems wrong to me:
>
> /* -cpu max: if KVM is enabled, like -cpu host (best possible with this 
> host); */
>
> This is not exactly true:
>
> "-cpu max" calls kvm_arm_set_cpu_features_from_host(), (which checks 
> "dtb_compatible", and if not set gets the features from the host, if set 
> ...?)
> After that, calls aarch64_add_sve_properties() and then adds also 
> "svw-max-vq". This code is common with TCG.
>>
>>
>> As part of this research I noticed that arm_cpu_post_init() is quite 
>> confusing, seems really inconsistent to me.
>>
>> Apparently the intention was to call it from the leaf classes:
>>
>> commit 51e5ef459eca045d7e8afe880ee60190f0b75b26
>> Author: Marc-André Lureau 
>> Date:   Tue Nov 27 12:55:59 2018 +0400
>>
>> arm: replace instance_post_init()
>> 
>> Replace arm_cpu_post_init() instance callback by calling it from leaf
>> classes, to avoid potential ordering issue with other post_init 
>> callbacks.
>> 
>> Signed-off-by: Marc-André Lureau 
>> Suggested-by: Igor Mammedov 
>> Reviewed-by: Igor Mammedov 
>> Acked-by: Eduardo Habkost 
>>
>>
>> but then we end up calling it multiple times in the class hierarch, which is 
>> a recipe for bugs, and makes it difficult to understand what 
>> arm_cpu_post_init()
>> even means, what calling this function is supposed to do.
>>
>> For a "max" or "host" cpu on AArch64, this function is called:
>>
>> for the ARM CPU base class, TYPE_ARM_CPU, in
>>
>> cpu.c::arm_cpu_instance_init,
>>
>> then later again for the TYPE_AARCH64_CPU class, child of TYPE_ARM_CPU, in
>>
>> cpu64.c::aarch64_cpu_instance_init,
>>
>> then later again for the TYPE_ARM_HOST_CPU class, child of TYPE_AARCH64_CPU, 
>> in
>>
>> cpu.c::arm_host_initfn.
>>
>> Same for "max".
>>
>> When looking at 32bit CPUs instead, only the ARM CPU base class ends up 
>> calling arm_cpu_post_init.
>> "Leaf" classes do not do it (see cpu_tcg.c).
>>
>> What is then arm_cpu_post_init even supposed to mean?
>>
>> Thanks,
>>
>> Claudio
>>
>>
>
> In the case of cpu host instead,
>
> "-cpu host" calls kvm_arm_set_cpu_features_from_host(), same as max, then 
> calls aarch64_add_sve_properties() but does NOT add "svw-max-vq".
>
> Is this a bug?
>>>
>>> It was left out intentionally. More below.
>>>

 Maybe; that's a question for Richard or Drew...

> Are "max" and "host" for KVM supposed to be the same like with x86?
>>>
>>> Yes, but my understanding of "max" == "host" for KVM is that that only
>>> applies to the perspective of the guest. What CPU and what CPU features
>>> the guest can see should be exactly the same with either "max" or "host",
>>> depending on the enabling/disabling of any optional CPU properties.
>>>
>>> The question here seems to be that, if one has a CPU property, does that
>>> imply the other should have the same? Which would effectively allow the
>>> two to be aliases (when KVM is enabled). I don't know, does x86 ensure
>>> 100% property compatibility?
>>>
>>> I opted not to support sve-max-vq for "host" because I consider it a
>>> legacy CPU property, one I didn't want to propagate. Indeed it may
>>> make more sense to depreciate sve-max-vq than to "fix" this issue
>>> by adding it to "host". Note, we can already create equivalent SVE
>>> CPUs. The following are the same from the perspective of the guest
>>>
>>>  -accel kvm -cpu host,sve512=on
>>>  -accel kvm 

[PATCH] tests/qtest: cleanup the testcase for bug 1878642

[Paolo Bonzini]

Clean up the writes to the configuration space and the PM region, and
rename the test to lpc-ich9-test.

Signed-off-by: Paolo Bonzini 
---
 tests/qtest/{fuzz-test.c => lpc-ich9-test.c} | 12 +++-
 tests/qtest/meson.build  |  2 +-
 2 files changed, 8 insertions(+), 6 deletions(-)
 rename tests/qtest/{fuzz-test.c => lpc-ich9-test.c} (71%)

diff --git a/tests/qtest/fuzz-test.c b/tests/qtest/lpc-ich9-test.c
similarity index 71%
rename from tests/qtest/fuzz-test.c
rename to tests/qtest/lpc-ich9-test.c
index 00149abec7..fe0bef9980 100644
--- a/tests/qtest/fuzz-test.c
+++ b/tests/qtest/lpc-ich9-test.c
@@ -1,5 +1,5 @@
 /*
- * QTest testcase for fuzz case
+ * QTest testcases for ich9 case
  *
  * Copyright (c) 2020 Li Qiang 
  *
@@ -18,9 +18,11 @@ static void test_lp1878642_pci_bus_get_irq_level_assert(void)
 s = qtest_init("-M pc-q35-5.0 "
"-nographic -monitor none -serial none");
 
-qtest_outl(s, 0xcf8, 0x8400f841);
-qtest_outl(s, 0xcfc, 0xebed205d);
-qtest_outl(s, 0x5d02, 0xebed205d);
+qtest_outl(s, 0xcf8, 0x8000f840); /* PMBASE */
+qtest_outl(s, 0xcfc, 0x5d00);
+qtest_outl(s, 0xcf8, 0x8000f844); /* ACPI_CTRL */
+qtest_outl(s, 0xcfc, 0xeb);
+qtest_outw(s, 0x5d02, 0x205d);
 qtest_quit(s);
 }
 
@@ -31,7 +33,7 @@ int main(int argc, char **argv)
 g_test_init(&argc, &argv, NULL);
 
 if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
-qtest_add_func("fuzz/test_lp1878642_pci_bus_get_irq_level_assert",
+qtest_add_func("ich9/test_lp1878642_pci_bus_get_irq_level_assert",
test_lp1878642_pci_bus_get_irq_level_assert);
 }
 
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 5bee1acd51..72d0f83592 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -57,6 +57,7 @@ qtests_i386 = \
   (config_all_devices.has_key('CONFIG_HDA') ? ['intel-hda-test'] : []) +   
 \
   (config_all_devices.has_key('CONFIG_I82801B11') ? ['i82801b11-test'] : []) + 
\
   (config_all_devices.has_key('CONFIG_IOH3420') ? ['ioh3420-test'] : []) + 
 \
+  (config_all_devices.has_key('CONFIG_LPC_ICH9') ? ['lpc-ich9-test'] : []) +   
   \
   (config_all_devices.has_key('CONFIG_USB_UHCI') ? ['usb-hcd-uhci-test'] : []) 
+\
   (config_all_devices.has_key('CONFIG_USB_UHCI') and   
 \
config_all_devices.has_key('CONFIG_USB_EHCI') ? ['usb-hcd-ehci-test'] : []) 
+\
@@ -74,7 +75,6 @@ qtests_i386 = \
'bios-tables-test',
'rtc-test',
'i440fx-test',
-   'fuzz-test',
'fw_cfg-test',
'device-plug-test',
'drive_del-test',
-- 
2.26.2

Re: [PULL v4 00/42] Block layer patches and object-add QAPIfication

[Kevin Wolf]

Am 18.03.2021 um 20:55 hat Peter Maydell geschrieben:
> On Thu, 18 Mar 2021 at 09:48, Kevin Wolf  wrote:
> >
> > The following changes since commit 571d413b5da6bc6f1c2aaca8484717642255ddb0:
> >
> >   Merge remote-tracking branch 
> > 'remotes/mcayland/tags/qemu-openbios-20210316' into staging (2021-03-17 
> > 21:02:37 +)
> >
> > are available in the Git repository at:
> >
> >   git://repo.or.cz/qemu/kevin.git tags/for-upstream
> >
> > for you to fetch changes up to ef11a2d8972147994492c36cd3d26677e831e4d7:
> >
> >   vl: allow passing JSON to -object (2021-03-18 10:45:01 +0100)
> >
> > 
> > Block layer patches and object-add QAPIfication
> >
> > - QAPIfy object-add and --object
> > - stream: Fail gracefully if permission is denied
> > - storage-daemon: Fix crash on quit when job is still running
> > - curl: Fix use after free
> > - char: Deprecate backend aliases, fix QMP query-chardev-backends
> > - Fix image creation option defaults that exist in both the format and
> >   the protocol layer (e.g. 'cluster_size' in qcow2 and rbd; the qcow2
> >   default was incorrectly applied to the rbd layer)
> >
> 
> Auto-merging docs/system/removed-features.rst
> CONFLICT (content): Merge conflict in docs/system/removed-features.rst
> Auto-merging docs/system/deprecated.rst
> CONFLICT (content): Merge conflict in docs/system/deprecated.rst
> 
> I'm afraid your pullreq was racing with another one deprecating features :-(

I'm not sure that not resolving trivial merge conflicts on your side is
saving anyone's time. Well, v5 is coming.

Kevin

[PULL v5 00/42] Block layer patches and object-add QAPIfication

[Kevin Wolf]

The following changes since commit 8a40754bca14df63c6d2ffe473b68a270dc50679:

  Merge remote-tracking branch 'remotes/nvme/tags/nvme-next-pull-request' into 
staging (2021-03-18 19:55:37 +)

are available in the Git repository at:

  git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to 009ff89328b1da3ea8ba316bf2be2125bc9937c5:

  vl: allow passing JSON to -object (2021-03-19 10:18:17 +0100)


Block layer patches and object-add QAPIfication

- QAPIfy object-add and --object
- stream: Fail gracefully if permission is denied
- storage-daemon: Fix crash on quit when job is still running
- curl: Fix use after free
- char: Deprecate backend aliases, fix QMP query-chardev-backends
- Fix image creation option defaults that exist in both the format and
  the protocol layer (e.g. 'cluster_size' in qcow2 and rbd; the qcow2
  default was incorrectly applied to the rbd layer)


Kevin Wolf (35):
  storage-daemon: Call job_cancel_sync_all() on shutdown
  stream: Don't crash when node permission is denied
  tests: Drop 'props' from object-add calls
  qapi/qom: Drop deprecated 'props' from object-add
  qapi/qom: Add ObjectOptions for iothread
  qapi/qom: Add ObjectOptions for authz-*
  qapi/qom: Add ObjectOptions for cryptodev-*
  qapi/qom: Add ObjectOptions for dbus-vmstate
  qapi/qom: Add ObjectOptions for memory-backend-*
  qapi/qom: Add ObjectOptions for rng-*, deprecate 'opened'
  qapi/qom: Add ObjectOptions for throttle-group
  qapi/qom: Add ObjectOptions for secret*, deprecate 'loaded'
  qapi/qom: Add ObjectOptions for tls-*, deprecate 'loaded'
  qapi/qom: Add ObjectOptions for can-*
  qapi/qom: Add ObjectOptions for colo-compare
  qapi/qom: Add ObjectOptions for filter-*
  qapi/qom: Add ObjectOptions for pr-manager-helper
  qapi/qom: Add ObjectOptions for confidential-guest-support
  qapi/qom: Add ObjectOptions for input-*
  qapi/qom: Add ObjectOptions for x-remote-object
  qapi/qom: QAPIfy object-add
  qom: Make "object" QemuOptsList optional
  qemu-storage-daemon: Implement --object with qmp_object_add()
  qom: Remove user_creatable_add_dict()
  qom: Factor out user_creatable_process_cmdline()
  qemu-io: Use user_creatable_process_cmdline() for --object
  qemu-nbd: Use user_creatable_process_cmdline() for --object
  qom: Add user_creatable_add_from_str()
  qemu-img: Use user_creatable_process_cmdline() for --object
  hmp: QAPIfy object_add
  qom: Add user_creatable_parse_str()
  char: Skip CLI aliases in query-chardev-backends
  char: Deprecate backend aliases 'tty' and 'parport'
  char: Simplify chardev_name_foreach()
  qom: Support JSON in HMP object_add and tools --object

Max Reitz (2):
  curl: Store BDRVCURLState pointer in CURLSocket
  curl: Disconnect sockets from CURLState

Paolo Bonzini (3):
  tests: convert check-qom-proplist to keyval
  qom: move user_creatable_add_opts logic to vl.c and QAPIfy it
  vl: allow passing JSON to -object

Stefan Hajnoczi (1):
  block/export: disable VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD for now

Stefano Garzarella (1):
  block: remove format defaults from QemuOpts in bdrv_create_file()

 qapi/authz.json   |  61 +++-
 qapi/block-core.json  |  27 ++
 qapi/common.json  |  52 +++
 qapi/crypto.json  | 159 +
 qapi/machine.json |  22 +-
 qapi/net.json |  20 --
 qapi/qom.json | 646 +-
 qapi/ui.json  |  13 +-
 docs/system/deprecated.rst|  31 +-
 docs/system/removed-features.rst  |   5 +
 docs/tools/qemu-img.rst   |   2 +-
 include/qom/object_interfaces.h   |  98 ++
 block.c   |  36 +-
 block/curl.c  |  50 +--
 block/export/vhost-user-blk-server.c  |   3 +-
 block/stream.c|  15 +-
 chardev/char.c|  19 +-
 hw/block/xen-block.c  |  16 +-
 monitor/hmp-cmds.c|  17 +-
 monitor/misc.c|   2 -
 qemu-img.c| 251 +++--
 qemu-io.c |  33 +-
 qemu-nbd.c|  34 +-
 qom/object_interfaces.c   | 172 -
 qom/qom-qmp-cmds.c|  28 +-
 softmmu/vl.c  |  84 -
 storage-daemon/qemu-storage-daemon.c  |  28 +-
 tests/qtest/qmp-cmd-test.c|  16 +-
 tests/qtest/test-netfilter.c  |  54 ++-
 tests/unit/check-qom-proplist.c   |  77 ++--
 tests/unit/test-char.c|   6 -
 hmp-commands.hx   |

Re: [PULL 5/5] m68k: add Virtual M68k Machine

[Max Reitz]

On 19.03.21 07:32, Thomas Huth wrote:

On 18/03/2021 18.28, Max Reitz wrote:
[...]
 From that it follows that I don’t see much use in testing specific 
devices either.  Say there’s a platform that provides both virtio-pci 
and virtio-mmio, the default (say virtio-pci) is fine for the iotests. 
I see little value in testing virtio-mmio as well.  (Perhaps I’m 
short-sighted, though.)


That's a fair point. But still, if someone compiled QEMU only with a 
target that only provided virtio-mmio, the iotests should not fail when 
running "make check".
To avoid that we continue playing whack-a-mole here in the future, maybe 
it would be better to restrict the iotests to the "main" targets only, 
e.g. modify check-block.sh so that the tests only run with x86, aarch64, 
s390x and ppc64 ?


Right, that would certainly be the simplest solution.

Max

Re: [PATCH] virtio: Fix virtio_mmio_read()/virtio_mmio_write()

[Stefano Garzarella]

On Sun, Mar 14, 2021 at 09:03:00PM +0100, Laurent Vivier wrote:

Both functions don't check the personality of the interface (legacy or
modern) before accessing the configuration memory and always use
virtio_config_readX()/virtio_config_writeX().

With this patch, they now check the personality and in legacy mode
call virtio_config_readX()/virtio_config_writeX(), otherwise call
virtio_config_modern_readX()/virtio_config_modern_writeX().

This change has been tested with virtio-mmio guests (virt stretch/armhf and
virt sid/m68k) and virtio-pci guests (pseries RHEL-7.3/ppc64 and /ppc64le).

Signed-off-by: Laurent Vivier 
---
hw/virtio/virtio-mmio.c | 74 +
1 file changed, 52 insertions(+), 22 deletions(-)



Reviewed-by: Stefano Garzarella 

Re: [PATCH 1/2] floppy: add a regression test for CVE-2020-25741

[Paolo Bonzini]

On 19/03/21 06:53, Markus Armbruster wrote:

I guess this is a reproducer.  Please also describe actual and expected
result.  Same for PATCH 2.


Isn't it in the patch itself?

Alexander, I think these reproducers are self-contained enough (no 
writes to PCI configuration space to set up the device) that we can 
place them in fdc-test.c.


Paolo

Re: [PATCH 3/3] migration: Pre-fault memory before starting background snasphot

[David Hildenbrand]

+/*
+ * ram_block_populate_pages: populate memory in the RAM block by reading
+ *   an integer from the beginning of each page.
+ *
+ * Since it's solely used for userfault_fd WP feature, here we just
+ *   hardcode page size to TARGET_PAGE_SIZE.
+ *
+ * @bs: RAM block to populate
+ */
+volatile int ram_block_populate_pages__tmp;
+static void ram_block_populate_pages(RAMBlock *bs)
+{
+ram_addr_t offset = 0;
+int tmp = 0;
+
+for (char *ptr = (char *) bs->host; offset < bs->used_length;
+ptr += TARGET_PAGE_SIZE, offset += TARGET_PAGE_SIZE) {


You'll want qemu_real_host_page_size instead of TARGET_PAGE_SIZE


+/* Try to do it without memory writes */
+tmp += *(volatile int *) ptr;
+}



The following is slightly simpler and doesn't rely on volatile semantics [1].
Should work on any arch I guess.

static void ram_block_populate_pages(RAMBlock *bs)
{
char *ptr = (char *) bs->host;
ram_addr_t offset;

for (offset = 0; offset < bs->used_length;
 offset += qemu_real_host_page_size) {
char tmp = *(volatile char *)(ptr + offset)

/* Don't optimize the read out. */
asm volatile ("" : "+r" (tmp));
}

Compiles to

for (offset = 0; offset < bs->used_length;
316d:   48 8b 4b 30 mov0x30(%rbx),%rcx
char *ptr = (char *) bs->host;
3171:   48 8b 73 18 mov0x18(%rbx),%rsi
for (offset = 0; offset < bs->used_length;
3175:   48 85 c9test   %rcx,%rcx
3178:   74 ce   je 3148 

 offset += qemu_real_host_page_size) {
317a:   48 8b 05 00 00 00 00mov0x0(%rip),%rax# 3181 

3181:   48 8b 38mov(%rax),%rdi
3184:   0f 1f 40 00 nopl   0x0(%rax)
char tmp = *(volatile char *)(ptr + offset);
3188:   48 8d 04 16 lea(%rsi,%rdx,1),%rax
318c:   0f b6 00movzbl (%rax),%eax
 offset += qemu_real_host_page_size) {
318f:   48 01 faadd%rdi,%rdx
for (offset = 0; offset < bs->used_length;
3192:   48 39 cacmp%rcx,%rdx
3195:   72 f1   jb 3188 



[1] https://programfan.github.io/blog/2015/04/27/prevent-gcc-optimize-away-code/


I'll send patches soon to take care of virtio-mem via RamDiscardManager -
to skip populating the parts that are supposed to remain discarded and not 
migrated.
Unfortunately, the RamDiscardManager patches are still stuck waiting for
acks ... and now we're in soft-freeze.

--
Thanks,

David / dhildenb

Re: [PULL 5/5] m68k: add Virtual M68k Machine

[Paolo Bonzini]

On 19/03/21 10:20, Max Reitz wrote:

On 19.03.21 07:32, Thomas Huth wrote:

On 18/03/2021 18.28, Max Reitz wrote:
[...]
 From that it follows that I don’t see much use in testing specific 
devices either.  Say there’s a platform that provides both virtio-pci 
and virtio-mmio, the default (say virtio-pci) is fine for the 
iotests. I see little value in testing virtio-mmio as well.  (Perhaps 
I’m short-sighted, though.)


That's a fair point. But still, if someone compiled QEMU only with a 
target that only provided virtio-mmio, the iotests should not fail 
when running "make check".
To avoid that we continue playing whack-a-mole here in the future, 
maybe it would be better to restrict the iotests to the "main" targets 
only, e.g. modify check-block.sh so that the tests only run with x86, 
aarch64, s390x and ppc64 ?


Right, that would certainly be the simplest solution.


It would also make the patches that Laurent sent this morning 
unnecessary, and avoid the use of aliases in the tests (so that it's 
clear what is tested).


Paolo

Re: [PATCH 3/3] migration: Pre-fault memory before starting background snasphot

[David Hildenbrand]

On 19.03.21 10:28, David Hildenbrand wrote:

+/*
+ * ram_block_populate_pages: populate memory in the RAM block by reading
+ *   an integer from the beginning of each page.
+ *
+ * Since it's solely used for userfault_fd WP feature, here we just
+ *   hardcode page size to TARGET_PAGE_SIZE.
+ *
+ * @bs: RAM block to populate
+ */
+volatile int ram_block_populate_pages__tmp;
+static void ram_block_populate_pages(RAMBlock *bs)
+{
+ram_addr_t offset = 0;
+int tmp = 0;
+
+for (char *ptr = (char *) bs->host; offset < bs->used_length;
+ptr += TARGET_PAGE_SIZE, offset += TARGET_PAGE_SIZE) {


You'll want qemu_real_host_page_size instead of TARGET_PAGE_SIZE


+/* Try to do it without memory writes */
+tmp += *(volatile int *) ptr;
+}



The following is slightly simpler and doesn't rely on volatile semantics [1].
Should work on any arch I guess.

static void ram_block_populate_pages(RAMBlock *bs)
{
  char *ptr = (char *) bs->host;
  ram_addr_t offset;

  for (offset = 0; offset < bs->used_length;
   offset += qemu_real_host_page_size) {
char tmp = *(volatile char *)(ptr + offset)


I wanted to do a "= *(ptr + offset)" here.



/* Don't optimize the read out. */
asm volatile ("" : "+r" (tmp));


So this is the only volatile thing that the compiler must guarantee to 
not optimize away.



--
Thanks,

David / dhildenb

Re: Serious doubts about Gitlab CI

[Stefan Hajnoczi]

On Thu, Mar 18, 2021 at 09:30:41PM +0100, Paolo Bonzini wrote:
> On 18/03/21 20:46, Stefan Hajnoczi wrote:
> > The QEMU Project has 50,000 minutes of GitLab CI quota. Let's enable
> > GitLab Merge Requests so that anyone can submit a merge request and get
> > CI coverage.
> 
> Each merge request consumes about 2500.  That won't last long.

Yikes, that is 41 hours per CI run. I wonder if GitLab's CI minutes are
on slow machines or if we'll hit the same issue with dedicated runners.
It seems like CI optimization will be necessary...

Stefan


signature.asc
Description: PGP signature

Re: Misleading configure failure GLIB_SIZEOF_SIZE_T

[Markus Armbruster]

Peter Maydell  writes:

> On Thu, 18 Mar 2021 at 12:53, Markus Armbruster  wrote:
>>
>> I just ran into this failure:
>>
>> $ ../configure --disable-tools --disable-system --static
>>
>> ERROR: sizeof(size_t) doesn't match GLIB_SIZEOF_SIZE_T.
>>You probably need to set PKG_CONFIG_LIBDIR
>>to point to the right pkg-config files for your
>>build target
>
> The interesting question here is why the earlier configure check:
>
> write_c_skeleton;
> if compile_object ; then
>   : C compiler works ok
> else
> error_exit "\"$cc\" either does not exist or does not work"
> fi
> if ! compile_prog ; then
> error_exit "\"$cc\" cannot build an executable (is your linker broken?)"
> fi
>
> didn't fail. That is deliberately early in configure in an attempt
> to capture this kind of "the compiler can't link anything" case
> before we get into specific feature testing.

I've since installed the libraries...  uninstalling glibc-static for a
quick check...  Yep, dies in the GLIB_SIZEOF_SIZE_T test.  Let's have a
look at my config.log.  To more easily find the check you pointed out, I
stuck "exit 42" right behind it, and get:

$ ./config.status 
[Exit 42 ]
$ echo $?
42
$ tail -n 4 config.log 

funcs: do_compiler do_cc compile_prog main
lines: 145 183 2017 0
cc -std=gnu99 -Wall -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 
-D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef 
-Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv 
-o config-temp/qemu-conf.exe config-temp/qemu-conf.c -m64
$ cat config-temp/qemu-conf.c 
int main(void) { return 0; }

Run the compiler by hand to confirm:

$ cc -std=gnu99 -Wall -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 
-D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef 
-Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv 
-o config-temp/qemu-conf.exe config-temp/qemu-conf.c -m64
$ echo $?
0

The test program doesn't actually need libc, so not having glibc-static
installed doesn't bother the linker.

If it used something from libc, then I'd expect the issue to merely
shift to the next library.  Remember, the failure I reported attempts to
link with -lgthread-2.0 -pthread -lglib-2.0 -pthread -lpcre -pthread.

Re: Serious doubts about Gitlab CI

[Paolo Bonzini]

On 19/03/21 10:33, Stefan Hajnoczi wrote:

On Thu, Mar 18, 2021 at 09:30:41PM +0100, Paolo Bonzini wrote:

On 18/03/21 20:46, Stefan Hajnoczi wrote:

The QEMU Project has 50,000 minutes of GitLab CI quota. Let's enable
GitLab Merge Requests so that anyone can submit a merge request and get
CI coverage.


Each merge request consumes about 2500.  That won't last long.


Yikes, that is 41 hours per CI run. I wonder if GitLab's CI minutes are
on slow machines or if we'll hit the same issue with dedicated runners.
It seems like CI optimization will be necessary...


Shared runners are 1 vCPU, so it's really 41 CPU hours per CI run. 
That's a lot but not unheard of.


Almost every 2-socket server these days will have at least 50 CPUs; with 
some optimization we probably can get it down to half an hour of real 
time, on a single server running 3-4 runners with 16 vCPUs.


Paolo

Re: [PATCH 3/3] i386: Make sure kvm_arch_set_tsc_khz() succeeds on migration when 'hv-reenlightenment' was exposed

[Vitaly Kuznetsov]

Paolo Bonzini  writes:

> On 18/03/21 17:38, Vitaly Kuznetsov wrote:
>>> Could we instead fail to load the reenlightenment section if
>>> user_tsc_khz was not set?  This seems to be user (well, management)
>>> error really, since reenlightenment has to be enabled manually (or with
>>> hv-passthrough which blocks migration too).
>>
>> Yes, we certainly could do that but what's the added value of
>> user_tsc_khz which upper layer will have to set explicitly (probably to
>> the tsc frequency of the source host anyway)? In case we just want to
>> avoid calling KVM_SET_TSC_KHZ twice, we can probably achieve that by
>> adding a CPU flag or something.
>
> What I want to achieve is to forbid migration of VMs with 
> reenlightenment, if they don't also specify tsc-khz to the frequency of 
> the TSC on the source host.  We can't check it at the beginning of 
> migration, but at least we can check it at the end.
>
> Maybe we're talking about two different things?

No, your suggestion basically extends mine and I'm just trying to
understand the benefit. With my suggestion, it is not required to
specify tsc-khz on the source, we just take 'native' tsc frequency as a
reference. Post-migration, we require that KVM_SET_TSC_KHZ succeeds (and
not just 'try' like kvm_arch_put_registers() does so we effectively
break migration when we are unable to set the desired TSC frequency
(also at the end).

With your suggestion to require user_tsc_khz (as I see it) it'll work
the exact same way but there's an additional burden on the
user/management to explicitly specify tsc-khz on the command line and I
believe that almost always this is going to match 'native' tsc frequency
of the source host.

-- 
Vitaly

Re: [PATCH 3/3] i386: Make sure kvm_arch_set_tsc_khz() succeeds on migration when 'hv-reenlightenment' was exposed

[Vitaly Kuznetsov]

Marcelo Tosatti  writes:

> On Thu, Mar 18, 2021 at 05:38:00PM +0100, Vitaly Kuznetsov wrote:
>> Paolo Bonzini  writes:
>> 
>> > On 18/03/21 17:02, Vitaly Kuznetsov wrote:
>> >> KVM doesn't fully support Hyper-V reenlightenment notifications on
>> >> migration. In particular, it doesn't support emulating TSC frequency
>> >> of the source host by trapping all TSC accesses so unless TSC scaling
>> >> is supported on the destination host and KVM_SET_TSC_KHZ succeeds, it
>> >> is unsafe to proceed with migration.
>> >> 
>> >> Normally, we only require KVM_SET_TSC_KHZ to succeed when 'user_tsc_khz'
>> >> was set and just 'try' KVM_SET_TSC_KHZ without otherwise.
>> >> 
>> >> Introduce a new vmstate section (which is added when the guest has
>> >> reenlightenment feature enabled) and add env.tsc_khz to it. We already
>> >> have env.tsc_khz packed in 'cpu/tsc_khz' but we don't want to be dependent
>> >> on the section order.
>> >> 
>> >> Signed-off-by: Vitaly Kuznetsov 
>> >
>> > Could we instead fail to load the reenlightenment section if 
>> > user_tsc_khz was not set?  This seems to be user (well, management) 
>> > error really, since reenlightenment has to be enabled manually (or with 
>> > hv-passthrough which blocks migration too).
>
> Seems to match the strategy of the patchset...
>
>> Yes, we certainly could do that but what's the added value of
>> user_tsc_khz which upper layer will have to set explicitly (probably to
>> the tsc frequency of the source host anyway)?
>
> Yes. I think what happened was "evolution":
>
> 1) Added support to set tsc frequency (with hardware multiplier)
> in KVM, so add -tsc-khz VAL (kHz) option to KVM.
>
> 2) Scaling is enabled only if -tsc-khz VAL is supplied.
>
> 3) libvirt switches to using -tsc-khz HVAL, where HVAL it retrieves
> from KVM_GET_TSC_KHZ of newly created KVM_CREATE_VM instance.
>
> It could have been done inside qemu instead.
>
>> In case we just want to avoid calling KVM_SET_TSC_KHZ twice, we can probably 
>> achieve that by
>> adding a CPU flag or something.
>
> Avoid calling KVM_SET_TSC_KHZ twice ? Don't see why you would avoid
> that.
>

Actually, we already do KVM_SET_TSC_KHZ twice, my patch adds just
another call for KVM_SET_TSC_KHZ. We already do one call in
kvm_arch_put_registers() but we don't propagate errors from it so in case
TSC scaling is unsupported, migration still succeeds and this is
intentional unless 'tsc-khz' was explicitly specified. When 'tsc-khz' is
specified, the error is propageted from kvm_arch_init_vcpu() (second
call site). We can also achieve the goal of this patch if we follow
Paolo's suggestion: just make 'tsc-khz' a must with reenlightenment.

-- 
Vitaly

Re: [PATCH 1/2] floppy: add a regression test for CVE-2020-25741

[Markus Armbruster]

Paolo Bonzini  writes:

> On 19/03/21 06:53, Markus Armbruster wrote:
>> I guess this is a reproducer.  Please also describe actual and expected
>> result.  Same for PATCH 2.
>
> Isn't it in the patch itself?

A commit message should tell me what the patch is trying to accomplish.

This commit message's title tells me it's a test for a CVE.  Okay.  The
body additionally gives me the reproducer.  To be useful, a reproducer
needs to come with actual and expected result.  Yes, I can find those in
the patch.  But I could find the reproducer there, too.  If you're nice
enough to save me the trouble of digging through the patch for the
reproducer (thanks), please consider saving me the trouble digging for
the information I need to make use of it (thanks again).  That's all :)

[...]

[PATCH] gitlab-ci.yml: Merge the trace-backend testing into other jobs

[Thomas Huth]

Our gitlab-ci got quite slow in the past weeks, due to the immense amount
of jobs that we have, so we should try to reduce the number of jobs.
There is no real good reason for having separate jobs just to test the
trace backends, we can do this just fine in other jobs, too.

Signed-off-by: Thomas Huth 
---
 .gitlab-ci.yml | 30 +++---
 1 file changed, 3 insertions(+), 27 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 0cc6d53f7e..cbbd67f139 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -87,7 +87,7 @@ build-system-alpine:
 TARGETS: aarch64-softmmu alpha-softmmu cris-softmmu hppa-softmmu
   moxie-softmmu microblazeel-softmmu mips64el-softmmu
 MAKE_CHECK_ARGS: check-build
-CONFIGURE_ARGS: --enable-docs
+CONFIGURE_ARGS: --enable-docs --enable-trace-backends=log,simple,syslog
   artifacts:
 expire_in: 2 days
 paths:
@@ -608,7 +608,7 @@ tsan-build:
   variables:
 IMAGE: ubuntu2004
 CONFIGURE_ARGS: --enable-tsan --cc=clang-10 --cxx=clang++-10 --disable-docs
---enable-fdt=system --enable-slirp=system
+  --enable-trace-backends=ust --enable-fdt=system --enable-slirp=system
 TARGETS: x86_64-softmmu ppc64-softmmu riscv64-softmmu x86_64-linux-user
 MAKE_CHECK_ARGS: bench V=1
 
@@ -706,6 +706,7 @@ build-coroutine-sigaltstack:
   variables:
 IMAGE: ubuntu2004
 CONFIGURE_ARGS: --with-coroutine=sigaltstack --disable-tcg
+--enable-trace-backends=ftrace
 MAKE_CHECK_ARGS: check-unit
 
 # Most jobs test latest gcrypt or nettle builds
@@ -743,31 +744,6 @@ crypto-only-gnutls:
 MAKE_CHECK_ARGS: check
 
 
-# We don't need to exercise every backend with every front-end
-build-trace-multi-user:
-  <<: *native_build_job_definition
-  needs:
-job: amd64-ubuntu2004-container
-  variables:
-IMAGE: ubuntu2004
-CONFIGURE_ARGS: --enable-trace-backends=log,simple,syslog --disable-system
-
-build-trace-ftrace-system:
-  <<: *native_build_job_definition
-  needs:
-job: amd64-ubuntu2004-container
-  variables:
-IMAGE: ubuntu2004
-CONFIGURE_ARGS: --enable-trace-backends=ftrace --target-list=x86_64-softmmu
-
-build-trace-ust-system:
-  <<: *native_build_job_definition
-  needs:
-job: amd64-ubuntu2004-container
-  variables:
-IMAGE: ubuntu2004
-CONFIGURE_ARGS: --enable-trace-backends=ust --target-list=x86_64-softmmu
-
 # Check our reduced build configurations
 build-without-default-devices:
   <<: *native_build_job_definition
-- 
2.27.0

Re: [PULL v2 00/11] pflash patches for 2021-03-18

[Peter Maydell]

On Thu, 18 Mar 2021 at 15:53, Philippe Mathieu-Daudé  wrote:
>
> Since v1:
> - Fixed trace format string on 32-bit hosts (Peter)
>
> The following changes since commit 56b89f455894e4628ad7994fe5dd348145d1a9c5:
>
>   Merge remote-tracking branch 'remotes/bonzini-gitlab/tags/for-upstream' 
> into staging (2021-03-17 22:18:54 +)
>
> are available in the Git repository at:
>
>   https://github.com/philmd/qemu.git tags/pflash-20210318
>
> for you to fetch changes up to 91316cbb3830bb845c42da2d6eab06de56b889b0:
>
>   hw/block/pflash_cfi: Replace DPRINTF with trace events (2021-03-18 11:16:31 
> +0100)
>
> 
> Parallel NOR Flash patches queue
>
> - Code movement to ease maintainability
> - Tracing improvements
> 
>

Applied, thanks.

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

-- PMM

Re: [PULL v2 00/11] QAPI patches patches for 2021-03-16

[Peter Maydell]

On Thu, 18 Mar 2021 at 16:38, Markus Armbruster  wrote:
>
> The following changes since commit 1db136a29ce8594b693938ab8e788d8bcef54770:
>
>   Merge remote-tracking branch 
> 'remotes/cleber-gitlab/tags/python-next-pull-request' into staging 
> (2021-03-18 14:07:31 +)
>
> are available in the Git repository at:
>
>   git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2021-03-16-v2
>
> for you to fetch changes up to 6dbe64a7eeaf01cb2de939edb7226aff411b2816:
>
>   qapi: New -compat deprecated-input=crash (2021-03-18 16:58:29 +0100)
>
> 
> QAPI patches patches for 2021-03-16
>
> 
> Markus Armbruster (10):
>   qemu-options: New -compat to set policy for deprecated interfaces
>   qapi: Implement deprecated-output=hide for QMP command results
>   qapi: Implement deprecated-output=hide for QMP events
>   qapi: Implement deprecated-output=hide for QMP event data
>   monitor: Drop query-qmp-schema 'gen': false hack
>   qapi: Implement deprecated-output=hide for QMP introspection
>   test-util-sockets: Add stub for monitor_set_cur()
>   qapi: Implement deprecated-input=reject for QMP commands
>   qapi: Implement deprecated-input=reject for QMP command arguments
>   qapi: New -compat deprecated-input=crash
>
> Paolo Bonzini (1):
>   qemuutil: remove qemu_set_fd_handler duplicate symbol

CONFLICT (content): Merge conflict in monitor/qmp-cmds-control.c

thanks
-- PMM

[PATCH v4 0/3] qcow2: fix parallel rewrite and discard (rw-lock)

[Vladimir Sementsov-Ogievskiy]

Look at 03 for the problem and fix. 01 is preparation and 02 is the
test.

Actually previous version of this thing is 
   [PATCH v2(RFC) 0/3] qcow2: fix parallel rewrite and discard

Still
   [PATCH v3 0/6] qcow2: compressed write cache
includes another fix (more complicated) for the bug, so this is called
v4.

So, what's new:

It's still a CoRwlock based solution as suggested by Kevin.

Now I think that "writer" of the lock should be code in
update_refcount() which wants to set refcount to zero. If we consider
only guest discard request as "writer" we may miss other sources of
discarding host clusters (like rewriting compressed cluster to normal,
maybe some snapshot operations, who knows what's more).

And this means that we want to take rw-lock under qcow2 s->lock. And
this brings ordering restriction for the two locks: if we want both
locks taken, we should always take s->lock first, and never take s->lock
when rw-lock is already taken (otherwise we get classic deadlock).

This leads us to taking rd-lock for in-flight writes under s->lock in
same critical section where cluster is allocated (or just got from
metadata) and releasing after data writing completion.

This in turn leads to a bit tricky logic around transferring rd-lock to
task coroutine on normal write path (see 03).. But this is still simpler
than inflight-write-counters solution in v3..

Vladimir Sementsov-Ogievskiy (3):
  qemu-io: add aio_discard
  iotests: add qcow2-discard-during-rewrite
  block/qcow2: introduce discard_rw_lock: fix discarding host clusters

 block/qcow2.h |  20 +++
 block/qcow2-refcount.c|  22 
 block/qcow2.c |  73 +--
 qemu-io-cmds.c| 117 ++
 .../tests/qcow2-discard-during-rewrite|  99 +++
 .../tests/qcow2-discard-during-rewrite.out|  17 +++
 6 files changed, 341 insertions(+), 7 deletions(-)
 create mode 100755 tests/qemu-iotests/tests/qcow2-discard-during-rewrite
 create mode 100644 tests/qemu-iotests/tests/qcow2-discard-during-rewrite.out

-- 
2.29.2

[PATCH v4 1/3] qemu-io: add aio_discard

[Vladimir Sementsov-Ogievskiy]

Add aio_discard command like existing aio_write. It will be used in
further test.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 qemu-io-cmds.c | 117 +
 1 file changed, 117 insertions(+)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 97611969cb..28b5c3c092 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -1332,6 +1332,7 @@ struct aio_ctx {
 BlockBackend *blk;
 QEMUIOVector qiov;
 int64_t offset;
+int64_t discard_bytes;
 char *buf;
 bool qflag;
 bool vflag;
@@ -1343,6 +1344,34 @@ struct aio_ctx {
 struct timespec t1;
 };
 
+static void aio_discard_done(void *opaque, int ret)
+{
+struct aio_ctx *ctx = opaque;
+struct timespec t2;
+
+clock_gettime(CLOCK_MONOTONIC, &t2);
+
+
+if (ret < 0) {
+printf("aio_discard failed: %s\n", strerror(-ret));
+block_acct_failed(blk_get_stats(ctx->blk), &ctx->acct);
+goto out;
+}
+
+block_acct_done(blk_get_stats(ctx->blk), &ctx->acct);
+
+if (ctx->qflag) {
+goto out;
+}
+
+/* Finally, report back -- -C gives a parsable format */
+t2 = tsub(t2, ctx->t1);
+print_report("discarded", &t2, ctx->offset, ctx->discard_bytes,
+ ctx->discard_bytes, 1, ctx->Cflag);
+out:
+g_free(ctx);
+}
+
 static void aio_write_done(void *opaque, int ret)
 {
 struct aio_ctx *ctx = opaque;
@@ -1671,6 +1700,93 @@ static int aio_write_f(BlockBackend *blk, int argc, char 
**argv)
 return 0;
 }
 
+static void aio_discard_help(void)
+{
+printf(
+"\n"
+" asynchronously discards a range of bytes from the given offset\n"
+"\n"
+" Example:\n"
+" 'aio_discard 0 64k' - discards 64K at start of a disk\n"
+"\n"
+" Note that due to its asynchronous nature, this command will be\n"
+" considered successful once the request is submitted, independently\n"
+" of potential I/O errors or pattern mismatches.\n"
+" -C, -- report statistics in a machine parsable format\n"
+" -i, -- treat request as invalid, for exercising stats\n"
+" -q, -- quiet mode, do not show I/O statistics\n"
+"\n");
+}
+
+static int aio_discard_f(BlockBackend *blk, int argc, char **argv);
+
+static const cmdinfo_t aio_discard_cmd = {
+.name   = "aio_discard",
+.cfunc  = aio_discard_f,
+.perm   = BLK_PERM_WRITE,
+.argmin = 2,
+.argmax = -1,
+.args   = "[-Ciq] off len",
+.oneline= "asynchronously discards a number of bytes",
+.help   = aio_discard_help,
+};
+
+static int aio_discard_f(BlockBackend *blk, int argc, char **argv)
+{
+int ret;
+int c;
+struct aio_ctx *ctx = g_new0(struct aio_ctx, 1);
+
+ctx->blk = blk;
+while ((c = getopt(argc, argv, "Ciq")) != -1) {
+switch (c) {
+case 'C':
+ctx->Cflag = true;
+break;
+case 'q':
+ctx->qflag = true;
+break;
+case 'i':
+printf("injecting invalid discard request\n");
+block_acct_invalid(blk_get_stats(blk), BLOCK_ACCT_UNMAP);
+g_free(ctx);
+return 0;
+default:
+g_free(ctx);
+qemuio_command_usage(&aio_write_cmd);
+return -EINVAL;
+}
+}
+
+if (optind != argc - 2) {
+g_free(ctx);
+qemuio_command_usage(&aio_write_cmd);
+return -EINVAL;
+}
+
+ctx->offset = cvtnum(argv[optind]);
+if (ctx->offset < 0) {
+ret = ctx->offset;
+print_cvtnum_err(ret, argv[optind]);
+g_free(ctx);
+return ret;
+}
+optind++;
+
+ctx->discard_bytes = cvtnum(argv[optind]);
+if (ctx->discard_bytes < 0) {
+ret = ctx->discard_bytes;
+print_cvtnum_err(ret, argv[optind]);
+g_free(ctx);
+return ret;
+}
+
+blk_aio_pdiscard(blk, ctx->offset, ctx->discard_bytes,
+ aio_discard_done, ctx);
+
+return 0;
+}
+
 static int aio_flush_f(BlockBackend *blk, int argc, char **argv)
 {
 BlockAcctCookie cookie;
@@ -2494,6 +2610,7 @@ static void __attribute((constructor)) 
init_qemuio_commands(void)
 qemuio_add_command(&readv_cmd);
 qemuio_add_command(&write_cmd);
 qemuio_add_command(&writev_cmd);
+qemuio_add_command(&aio_discard_cmd);
 qemuio_add_command(&aio_read_cmd);
 qemuio_add_command(&aio_write_cmd);
 qemuio_add_command(&aio_flush_cmd);
-- 
2.29.2

[PATCH v4 2/3] iotests: add qcow2-discard-during-rewrite

[Vladimir Sementsov-Ogievskiy]

Simple test:
 - start writing to allocated cluster A
 - discard this cluster
 - write to another unallocated cluster B (it's allocated in same place
   where A was allocated)
 - continue writing to A

For now last action pollutes cluster B which is a bug fixed by the
following commit.

For now, add test to "disabled" group, so that it doesn't run
automatically.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 .../tests/qcow2-discard-during-rewrite| 99 +++
 .../tests/qcow2-discard-during-rewrite.out| 17 
 2 files changed, 116 insertions(+)
 create mode 100755 tests/qemu-iotests/tests/qcow2-discard-during-rewrite
 create mode 100644 tests/qemu-iotests/tests/qcow2-discard-during-rewrite.out

diff --git a/tests/qemu-iotests/tests/qcow2-discard-during-rewrite 
b/tests/qemu-iotests/tests/qcow2-discard-during-rewrite
new file mode 100755
index 00..dd9964ef3f
--- /dev/null
+++ b/tests/qemu-iotests/tests/qcow2-discard-during-rewrite
@@ -0,0 +1,99 @@
+#!/usr/bin/env bash
+# group: quick disabled
+#
+# Test discarding (and reusing) host cluster during writing data to it.
+#
+# Copyright (c) 2021 Virtuozzo International GmbH.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+# creator
+owner=vsement...@virtuozzo.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+status=1   # failure is the default!
+
+_cleanup()
+{
+_cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./../common.rc
+. ./../common.filter
+
+_supported_fmt qcow2
+_supported_proto file fuse
+_supported_os Linux
+
+size=1M
+_make_test_img $size
+
+(
+cat <

[PATCH v4 3/3] block/qcow2: introduce discard_rw_lock: fix discarding host clusters

[Vladimir Sementsov-Ogievskiy]

There is a bug in qcow2: host cluster can be discarded (refcount
becomes 0) and reused during data write. In this case data write may
pollute another data cluster (recently allocated) or even metadata.

To fix the issue introduce rw-lock: hold read-lock during data writing
take write-lock when refcount becomes 0.

Enable qcow2-discard-during-rewrite as it is fixed.

Suggested-by: Kevin Wolf 
Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/qcow2.h | 20 +
 block/qcow2-refcount.c| 22 ++
 block/qcow2.c | 73 +--
 .../tests/qcow2-discard-during-rewrite|  2 +-
 4 files changed, 109 insertions(+), 8 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index 0fe5f74ed3..7d387fe39d 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -420,6 +420,26 @@ typedef struct BDRVQcow2State {
  * is to convert the image with the desired compression type set.
  */
 Qcow2CompressionType compression_type;
+
+/*
+ * discard_rw_lock prevents discarding host cluster when there are 
in-flight
+ * writes into it. So, in-flight writes are "readers" and the code reducing
+ * cluster refcount to zero in update_refcount() is "writer".
+ *
+ * Data-writes should works as follows:
+ * 1. rd-lock should be acquired in the same s->lock critical section where
+ *corresponding host clusters were allocated or somehow got from the
+ *metadata. Otherwise we risk miss discarding the cluster on s->lock
+ *unlocking (unlock should only schedule another coroutine, not enter
+ *it, but better be absolutely safe).
+ *
+ * 2. rd-lock should be released after data writing finish.
+ *
+ * For reducing refcount to zero (and therefore allowing reallocating the
+ * host cluster for other needs) it's enough to take rw-lock (to wait for
+ * all in-flight writes) and immediately release it (see 
update_refcount()).
+ */
+CoRwlock discard_rw_lock;
 } BDRVQcow2State;
 
 typedef struct Qcow2COWRegion {
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 8e649b008e..bb6842cd8f 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -878,6 +878,28 @@ static int QEMU_WARN_UNUSED_RESULT 
update_refcount(BlockDriverState *bs,
 } else {
 refcount += addend;
 }
+
+if (qemu_in_coroutine() && refcount == 0) {
+/*
+ * Refcount becomes zero, but there are still may be in-fligth
+ * writes, that writing data to the cluster (that's done without
+ * qcow2 mutext held).
+ *
+ * Data writing protected by rd-locked discard_rw_lock. So here
+ * it's enough to take and immediately release wr-lock on it.
+ * We can immediately release it, because new write requests can't
+ * came to cluster which refcount becomes 0 (There should not be 
any
+ * links from L2 tables to it).
+ *
+ * We can't do it if we are not in coroutine. But if we are not in
+ * coroutine, that also means that we are modifying metadata not
+ * taking qcow2 s->lock mutex, which is wrong as well.. So, let's
+ * hope for a bright future.
+ */
+qemu_co_rwlock_wrlock(&s->discard_rw_lock);
+qemu_co_rwlock_unlock(&s->discard_rw_lock);
+}
+
 if (refcount == 0 && cluster_index < s->free_cluster_index) {
 s->free_cluster_index = cluster_index;
 }
diff --git a/block/qcow2.c b/block/qcow2.c
index 0db1227ac9..aea7aea334 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1899,6 +1899,7 @@ static int qcow2_open(BlockDriverState *bs, QDict 
*options, int flags,
 
 /* Initialise locks */
 qemu_co_mutex_init(&s->lock);
+qemu_co_rwlock_init(&s->discard_rw_lock);
 
 if (qemu_in_coroutine()) {
 /* From bdrv_co_create.  */
@@ -2182,9 +2183,17 @@ typedef struct Qcow2AioTask {
 QEMUIOVector *qiov;
 uint64_t qiov_offset;
 QCowL2Meta *l2meta; /* only for write */
+bool rdlock; /* only for write */
 } Qcow2AioTask;
 
 static coroutine_fn int qcow2_co_preadv_task_entry(AioTask *task);
+
+/*
+ * @rdlock: If true, it means that qcow2_add_task is called with 
discard_rw_lock
+ * rd-locked, and this rd-lock must be transaferred to the task. The task 
itself
+ * will release the lock. The caller expects that after qcow2_add_task() call
+ * the lock is already released.
+ */
 static coroutine_fn int qcow2_add_task(BlockDriverState *bs,
AioTaskPool *pool,
AioTaskFunc func,
@@ -2194,8 +2203,10 @@ static coroutine_fn int qcow2_add_task(BlockDriverState 
*bs,
uint64_t bytes,
QEMUIOVector *qiov,

[PATCH v4 00/14] RAM_NORESERVE, MAP_NORESERVE and hostmem "reserve" property

[David Hildenbrand]

Some fixes for shared anonymous memory, cleanups previously sent in other
context (resizeable allocations), followed by RAM_NORESERVE, implementing
it under POSIX using MAP_NORESERVE, and letting users configure it for
memory backens using the "reserve" property (default: true).

MAP_NORESERVE under Linux has in the context of QEMU an effect on
1) Private/shared anonymous memory
-> memory-backend-ram,id=mem0,size=10G
2) Private fd-based mappings
-> memory-backend-file,id=mem0,size=10G,mem-path=/dev/shm/0
-> memory-backend-memfd,id=mem0,size=10G
3) Private/shared hugetlb mappings
-> memory-backend-memfd,id=mem0,size=10G,hugetlb=on,hugetlbsize=2M

With MAP_NORESERVE/"reserve=off", we won't be reserving swap space (1/2) or
huge pages (3) for the whole memory region.

The target use case is virtio-mem, which dynamically exposes memory
inside a large, sparse memory area to the VM. MAP_NORESERVE tells the OS
"this mapping might be very sparse". This essentially allows
avoiding having to set "/proc/sys/vm/overcommit_memory == 1") when using
virtio-mem and also supporting hugetlbfs in the future.

virtio-mem currently only supports anonymous memory, in the future we want
to also support private memfd, shared file-based and shared hugetlbfs
mappings.

virtio-mem features I am currently working on that will make it all
play together with this work include:
1. Introducing a prealloc option for virtio-mem (e.g., using fallocate()
   when plugging blocks) to fail nicely when running out of
   backing storage like huge pages ("prealloc=on").
2. Handling virtio-mem requests via an iothread to not hold the BQL while
   populating/preallocating memory ("iothread=X").
3. Protecting unplugged memory e.g., using userfaultfd ("prot=uffd").
4. Dynamic reservation of swap space ("reserve=on")
5. Supporting resizable RAM block/memmory regions, such that we won't
   always expose a large, sparse memory region to the VM.
6. (resizeable allocations / optimized mmap handling when resizing RAM
blocks)

v3 -> v4:
- Minor comment/description updates
- "softmmu/physmem: Fix ram_block_discard_range() to handle shared ..."
-- Extended description
- "util/mmap-alloc: Pass flags instead of separate bools to ..."
-- Move flags to include/qemu/osdep.h and rename to "QEMU_MAP_*"
- "memory: Introduce RAM_NORESERVE and wire it up in qemu_ram_mmap()"
-- Adjust to new flags. Handle errors in mmap_activate() for now.
- "util/mmap-alloc: Support RAM_NORESERVE via MAP_NORESERVE under Linux"
-- Restrict support to Linux only for now
- "qmp: Include "reserve" property of memory backends"
-- Added
- "hmp: Print "reserve" property of memory backends with ..."
-- Added

v2 -> v3:
- Renamed "softmmu/physmem: Drop "shared" parameter from ram_block_add()"
  to "softmmu/physmem: Mark shared anonymous memory RAM_SHARED" and
  adjusted the description
- Added "softmmu/physmem: Fix ram_block_discard_range() to handle shared
  anonymous memory"
- Added "softmmu/physmem: Fix qemu_ram_remap() to handle shared anonymous
  memory"
- Added "util/mmap-alloc: Pass flags instead of separate bools to
  qemu_ram_mmap()"
- "util/mmap-alloc: Support RAM_NORESERVE via MAP_NORESERVE"
-- Further tweak code comments
-- Handle shared anonymous memory

v1 -> v2:
- Rebased to upstream and phs_mem_alloc simplifications
-- Upsteam added the "map_offset" parameter to many RAM allocation
   interfaces.
- "softmmu/physmem: Drop "shared" parameter from ram_block_add()"
-- Use local variable "shared"
- "memory: introduce RAM_NORESERVE and wire it up in qemu_ram_mmap()"
-- Simplify due to phs_mem_alloc changes
- "util/mmap-alloc: Support RAM_NORESERVE via MAP_NORESERVE"
-- Add a whole bunch of comments.
-- Exclude shared anonymous memory that QEMU doesn't use
-- Special-case readonly mappings

Cc: Peter Xu 
Cc: "Michael S. Tsirkin" 
Cc: Eduardo Habkost 
Cc: "Dr. David Alan Gilbert" 
Cc: Richard Henderson 
Cc: Paolo Bonzini 
Cc: Igor Mammedov 
Cc: "Philippe Mathieu-Daudé" 
Cc: Stefan Hajnoczi 
Cc: Murilo Opsfelder Araujo 
Cc: Greg Kurz 
Cc: Liam Merwick 
Cc: Marcel Apfelbaum 

David Hildenbrand (14):
  softmmu/physmem: Mark shared anonymous memory RAM_SHARED
  softmmu/physmem: Fix ram_block_discard_range() to handle shared
anonymous memory
  softmmu/physmem: Fix qemu_ram_remap() to handle shared anonymous
memory
  util/mmap-alloc: Factor out calculation of the pagesize for the guard
page
  util/mmap-alloc: Factor out reserving of a memory region to
mmap_reserve()
  util/mmap-alloc: Factor out activating of memory to mmap_activate()
  softmmu/memory: Pass ram_flags to qemu_ram_alloc_from_fd()
  softmmu/memory: Pass ram_flags to
memory_region_init_ram_shared_nomigrate()
  util/mmap-alloc: Pass flags instead of separate bools to
qemu_ram_mmap()
  memory: Introduce RAM_NORESERVE and wire it up in qemu_ram_mmap()
  util/mmap-alloc: Support RAM_NORESERVE via MAP_NORESERVE under Linux
  hostmem: Wire up RAM_NORESERVE via "reserve" property
  qmp: Include "reserve" proper

[PATCH v4 01/14] softmmu/physmem: Mark shared anonymous memory RAM_SHARED

[David Hildenbrand]

Let's drop the "shared" parameter from ram_block_add() and properly
store it in the flags of the ram block instead, such that
qemu_ram_is_shared() properly succeeds on all ram blocks that were mapped
MAP_SHARED.

We'll use this information next to fix some cases with shared anonymous
memory.

Reviewed-by: Igor Kotrasinski 
Reviewed-by: Richard Henderson 
Reviewed-by: Peter Xu 
Signed-off-by: David Hildenbrand 
---
 softmmu/physmem.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 85034d9c11..76bb8e324e 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -1927,8 +1927,9 @@ static void dirty_memory_extend(ram_addr_t old_ram_size,
 }
 }
 
-static void ram_block_add(RAMBlock *new_block, Error **errp, bool shared)
+static void ram_block_add(RAMBlock *new_block, Error **errp)
 {
+const bool shared = qemu_ram_is_shared(new_block);
 RAMBlock *block;
 RAMBlock *last_block = NULL;
 ram_addr_t old_ram_size, new_ram_size;
@@ -2064,7 +2065,7 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, 
MemoryRegion *mr,
 return NULL;
 }
 
-ram_block_add(new_block, &local_err, ram_flags & RAM_SHARED);
+ram_block_add(new_block, &local_err);
 if (local_err) {
 g_free(new_block);
 error_propagate(errp, local_err);
@@ -2127,10 +2128,13 @@ RAMBlock *qemu_ram_alloc_internal(ram_addr_t size, 
ram_addr_t max_size,
 if (host) {
 new_block->flags |= RAM_PREALLOC;
 }
+if (share) {
+new_block->flags |= RAM_SHARED;
+}
 if (resizeable) {
 new_block->flags |= RAM_RESIZEABLE;
 }
-ram_block_add(new_block, &local_err, share);
+ram_block_add(new_block, &local_err);
 if (local_err) {
 g_free(new_block);
 error_propagate(errp, local_err);
-- 
2.29.2

[PATCH v4 03/14] softmmu/physmem: Fix qemu_ram_remap() to handle shared anonymous memory

[David Hildenbrand]

RAM_SHARED now also properly indicates shared anonymous memory. Let's check
that flag for anonymous memory as well, to restore the proper mapping.

Fixes: 06329ccecfa0 ("mem: add share parameter to memory-backend-ram")
Signed-off-by: David Hildenbrand 
---
 softmmu/physmem.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index afff96a6dc..cc59f05593 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -,13 +,13 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
 abort();
 } else {
 flags = MAP_FIXED;
+flags |= block->flags & RAM_SHARED ?
+ MAP_SHARED : MAP_PRIVATE;
 if (block->fd >= 0) {
-flags |= (block->flags & RAM_SHARED ?
-  MAP_SHARED : MAP_PRIVATE);
 area = mmap(vaddr, length, PROT_READ | PROT_WRITE,
 flags, block->fd, offset);
 } else {
-flags |= MAP_PRIVATE | MAP_ANONYMOUS;
+flags |= MAP_ANONYMOUS;
 area = mmap(vaddr, length, PROT_READ | PROT_WRITE,
 flags, -1, 0);
 }
-- 
2.29.2

[PATCH v4 02/14] softmmu/physmem: Fix ram_block_discard_range() to handle shared anonymous memory

[David Hildenbrand]

We can create shared anonymous memory via
"-object memory-backend-ram,share=on,..."
which is, for example, required by PVRDMA for mremap() to work.

Shared anonymous memory is weird, though. Instead of MADV_DONTNEED, we
have to use MADV_REMOVE: MADV_DONTNEED will only remove / zap all
relevant page table entries of the current process, the backend storage
will not get removed, resulting in no reduced memory consumption and
a repopulation of previous content on next access.

Shared anonymous memory is internally really just shmem, but without a
fd exposed. As we cannot use fallocate() without the fd to discard the
backing storage, MADV_REMOVE gets the same job done without a fd as
documented in "man 2 madvise". Removing backing storage implicitly
invalidates all page table entries with relevant mappings - an additional
MADV_DONTNEED is not required.

Fixes: 06329ccecfa0 ("mem: add share parameter to memory-backend-ram")
Reviewed-by: Peter Xu 
Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: David Hildenbrand 
---
 softmmu/physmem.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 76bb8e324e..afff96a6dc 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -3506,6 +3506,7 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, 
size_t length)
 /* The logic here is messy;
  *madvise DONTNEED fails for hugepages
  *fallocate works on hugepages and shmem
+ *shared anonymous memory requires madvise REMOVE
  */
 need_madvise = (rb->page_size == qemu_host_page_size);
 need_fallocate = rb->fd != -1;
@@ -3539,7 +3540,11 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t 
start, size_t length)
  * fallocate'd away).
  */
 #if defined(CONFIG_MADVISE)
-ret =  madvise(host_startaddr, length, MADV_DONTNEED);
+if (qemu_ram_is_shared(rb) && rb->fd < 0) {
+ret = madvise(host_startaddr, length, MADV_REMOVE);
+} else {
+ret = madvise(host_startaddr, length, MADV_DONTNEED);
+}
 if (ret) {
 ret = -errno;
 error_report("ram_block_discard_range: Failed to discard range 
"
-- 
2.29.2

[PATCH v4 06/14] util/mmap-alloc: Factor out activating of memory to mmap_activate()

[David Hildenbrand]

We want to activate memory within a reserved memory region, to make it
accessible. Let's factor that out.

Reviewed-by: Richard Henderson 
Acked-by: Murilo Opsfelder Araujo 
Reviewed-by: Peter Xu 
Signed-off-by: David Hildenbrand 
---
 util/mmap-alloc.c | 94 +--
 1 file changed, 50 insertions(+), 44 deletions(-)

diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
index 223d66219c..0e2bd7bc0e 100644
--- a/util/mmap-alloc.c
+++ b/util/mmap-alloc.c
@@ -114,6 +114,52 @@ static void *mmap_reserve(size_t size, int fd)
 return mmap(0, size, PROT_NONE, flags, fd, 0);
 }
 
+/*
+ * Activate memory in a reserved region from the given fd (if any), to make
+ * it accessible.
+ */
+static void *mmap_activate(void *ptr, size_t size, int fd, bool readonly,
+   bool shared, bool is_pmem, off_t map_offset)
+{
+const int prot = PROT_READ | (readonly ? 0 : PROT_WRITE);
+int map_sync_flags = 0;
+int flags = MAP_FIXED;
+void *activated_ptr;
+
+flags |= fd == -1 ? MAP_ANONYMOUS : 0;
+flags |= shared ? MAP_SHARED : MAP_PRIVATE;
+if (shared && is_pmem) {
+map_sync_flags = MAP_SYNC | MAP_SHARED_VALIDATE;
+}
+
+activated_ptr = mmap(ptr, size, prot, flags | map_sync_flags, fd,
+ map_offset);
+if (activated_ptr == MAP_FAILED && map_sync_flags) {
+if (errno == ENOTSUP) {
+char *proc_link = g_strdup_printf("/proc/self/fd/%d", fd);
+char *file_name = g_malloc0(PATH_MAX);
+int len = readlink(proc_link, file_name, PATH_MAX - 1);
+
+if (len < 0) {
+len = 0;
+}
+file_name[len] = '\0';
+fprintf(stderr, "Warning: requesting persistence across crashes "
+"for backend file %s failed. Proceeding without "
+"persistence, data might become corrupted in case of host "
+"crash.\n", file_name);
+g_free(proc_link);
+g_free(file_name);
+}
+/*
+ * If mmap failed with MAP_SHARED_VALIDATE | MAP_SYNC, we will try
+ * again without these flags to handle backwards compatibility.
+ */
+activated_ptr = mmap(ptr, size, prot, flags, fd, map_offset);
+}
+return activated_ptr;
+}
+
 static inline size_t mmap_guard_pagesize(int fd)
 {
 #if defined(__powerpc64__) && defined(__linux__)
@@ -133,13 +179,8 @@ void *qemu_ram_mmap(int fd,
 off_t map_offset)
 {
 const size_t guard_pagesize = mmap_guard_pagesize(fd);
-int prot;
-int flags;
-int map_sync_flags = 0;
-size_t offset;
-size_t total;
-void *guardptr;
-void *ptr;
+size_t offset, total;
+void *ptr, *guardptr;
 
 /*
  * Note: this always allocates at least one extra page of virtual address
@@ -156,45 +197,10 @@ void *qemu_ram_mmap(int fd,
 /* Always align to host page size */
 assert(align >= guard_pagesize);
 
-flags = MAP_FIXED;
-flags |= fd == -1 ? MAP_ANONYMOUS : 0;
-flags |= shared ? MAP_SHARED : MAP_PRIVATE;
-if (shared && is_pmem) {
-map_sync_flags = MAP_SYNC | MAP_SHARED_VALIDATE;
-}
-
 offset = QEMU_ALIGN_UP((uintptr_t)guardptr, align) - (uintptr_t)guardptr;
 
-prot = PROT_READ | (readonly ? 0 : PROT_WRITE);
-
-ptr = mmap(guardptr + offset, size, prot,
-   flags | map_sync_flags, fd, map_offset);
-
-if (ptr == MAP_FAILED && map_sync_flags) {
-if (errno == ENOTSUP) {
-char *proc_link, *file_name;
-int len;
-proc_link = g_strdup_printf("/proc/self/fd/%d", fd);
-file_name = g_malloc0(PATH_MAX);
-len = readlink(proc_link, file_name, PATH_MAX - 1);
-if (len < 0) {
-len = 0;
-}
-file_name[len] = '\0';
-fprintf(stderr, "Warning: requesting persistence across crashes "
-"for backend file %s failed. Proceeding without "
-"persistence, data might become corrupted in case of host "
-"crash.\n", file_name);
-g_free(proc_link);
-g_free(file_name);
-}
-/*
- * if map failed with MAP_SHARED_VALIDATE | MAP_SYNC,
- * we will remove these flags to handle compatibility.
- */
-ptr = mmap(guardptr + offset, size, prot, flags, fd, map_offset);
-}
-
+ptr = mmap_activate(guardptr + offset, size, fd, readonly, shared, is_pmem,
+map_offset);
 if (ptr == MAP_FAILED) {
 munmap(guardptr, total);
 return MAP_FAILED;
-- 
2.29.2

[PATCH v4 04/14] util/mmap-alloc: Factor out calculation of the pagesize for the guard page

[David Hildenbrand]

Let's factor out calculating the size of the guard page and rename the
variable to make it clearer that this pagesize only applies to the
guard page.

Reviewed-by: Peter Xu 
Acked-by: Murilo Opsfelder Araujo 
Cc: Igor Kotrasinski 
Signed-off-by: David Hildenbrand 
---
 util/mmap-alloc.c | 31 ---
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
index e6fa8b598b..24854064b4 100644
--- a/util/mmap-alloc.c
+++ b/util/mmap-alloc.c
@@ -82,6 +82,16 @@ size_t qemu_mempath_getpagesize(const char *mem_path)
 return qemu_real_host_page_size;
 }
 
+static inline size_t mmap_guard_pagesize(int fd)
+{
+#if defined(__powerpc64__) && defined(__linux__)
+/* Mappings in the same segment must share the same page size */
+return qemu_fd_getpagesize(fd);
+#else
+return qemu_real_host_page_size;
+#endif
+}
+
 void *qemu_ram_mmap(int fd,
 size_t size,
 size_t align,
@@ -90,12 +100,12 @@ void *qemu_ram_mmap(int fd,
 bool is_pmem,
 off_t map_offset)
 {
+const size_t guard_pagesize = mmap_guard_pagesize(fd);
 int prot;
 int flags;
 int map_sync_flags = 0;
 int guardfd;
 size_t offset;
-size_t pagesize;
 size_t total;
 void *guardptr;
 void *ptr;
@@ -116,8 +126,7 @@ void *qemu_ram_mmap(int fd,
  * anonymous memory is OK.
  */
 flags = MAP_PRIVATE;
-pagesize = qemu_fd_getpagesize(fd);
-if (fd == -1 || pagesize == qemu_real_host_page_size) {
+if (fd == -1 || guard_pagesize == qemu_real_host_page_size) {
 guardfd = -1;
 flags |= MAP_ANONYMOUS;
 } else {
@@ -126,7 +135,6 @@ void *qemu_ram_mmap(int fd,
 }
 #else
 guardfd = -1;
-pagesize = qemu_real_host_page_size;
 flags = MAP_PRIVATE | MAP_ANONYMOUS;
 #endif
 
@@ -138,7 +146,7 @@ void *qemu_ram_mmap(int fd,
 
 assert(is_power_of_2(align));
 /* Always align to host page size */
-assert(align >= pagesize);
+assert(align >= guard_pagesize);
 
 flags = MAP_FIXED;
 flags |= fd == -1 ? MAP_ANONYMOUS : 0;
@@ -193,8 +201,8 @@ void *qemu_ram_mmap(int fd,
  * a guard page guarding against potential buffer overflows.
  */
 total -= offset;
-if (total > size + pagesize) {
-munmap(ptr + size + pagesize, total - size - pagesize);
+if (total > size + guard_pagesize) {
+munmap(ptr + size + guard_pagesize, total - size - guard_pagesize);
 }
 
 return ptr;
@@ -202,15 +210,8 @@ void *qemu_ram_mmap(int fd,
 
 void qemu_ram_munmap(int fd, void *ptr, size_t size)
 {
-size_t pagesize;
-
 if (ptr) {
 /* Unmap both the RAM block and the guard page */
-#if defined(__powerpc64__) && defined(__linux__)
-pagesize = qemu_fd_getpagesize(fd);
-#else
-pagesize = qemu_real_host_page_size;
-#endif
-munmap(ptr, size + pagesize);
+munmap(ptr, size + mmap_guard_pagesize(fd));
 }
 }
-- 
2.29.2

[PATCH v4 05/14] util/mmap-alloc: Factor out reserving of a memory region to mmap_reserve()

[David Hildenbrand]

We want to reserve a memory region without actually populating memory.
Let's factor that out.

Reviewed-by: Igor Kotrasinski 
Acked-by: Murilo Opsfelder Araujo 
Reviewed-by: Richard Henderson 
Reviewed-by: Peter Xu 
Signed-off-by: David Hildenbrand 
---
 util/mmap-alloc.c | 58 +++
 1 file changed, 33 insertions(+), 25 deletions(-)

diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
index 24854064b4..223d66219c 100644
--- a/util/mmap-alloc.c
+++ b/util/mmap-alloc.c
@@ -82,6 +82,38 @@ size_t qemu_mempath_getpagesize(const char *mem_path)
 return qemu_real_host_page_size;
 }
 
+/*
+ * Reserve a new memory region of the requested size to be used for mapping
+ * from the given fd (if any).
+ */
+static void *mmap_reserve(size_t size, int fd)
+{
+int flags = MAP_PRIVATE;
+
+#if defined(__powerpc64__) && defined(__linux__)
+/*
+ * On ppc64 mappings in the same segment (aka slice) must share the same
+ * page size. Since we will be re-allocating part of this segment
+ * from the supplied fd, we should make sure to use the same page size, to
+ * this end we mmap the supplied fd.  In this case, set MAP_NORESERVE to
+ * avoid allocating backing store memory.
+ * We do this unless we are using the system page size, in which case
+ * anonymous memory is OK.
+ */
+if (fd == -1 || qemu_fd_getpagesize(fd) == qemu_real_host_page_size) {
+fd = -1;
+flags |= MAP_ANONYMOUS;
+} else {
+flags |= MAP_NORESERVE;
+}
+#else
+fd = -1;
+flags |= MAP_ANONYMOUS;
+#endif
+
+return mmap(0, size, PROT_NONE, flags, fd, 0);
+}
+
 static inline size_t mmap_guard_pagesize(int fd)
 {
 #if defined(__powerpc64__) && defined(__linux__)
@@ -104,7 +136,6 @@ void *qemu_ram_mmap(int fd,
 int prot;
 int flags;
 int map_sync_flags = 0;
-int guardfd;
 size_t offset;
 size_t total;
 void *guardptr;
@@ -116,30 +147,7 @@ void *qemu_ram_mmap(int fd,
  */
 total = size + align;
 
-#if defined(__powerpc64__) && defined(__linux__)
-/* On ppc64 mappings in the same segment (aka slice) must share the same
- * page size. Since we will be re-allocating part of this segment
- * from the supplied fd, we should make sure to use the same page size, to
- * this end we mmap the supplied fd.  In this case, set MAP_NORESERVE to
- * avoid allocating backing store memory.
- * We do this unless we are using the system page size, in which case
- * anonymous memory is OK.
- */
-flags = MAP_PRIVATE;
-if (fd == -1 || guard_pagesize == qemu_real_host_page_size) {
-guardfd = -1;
-flags |= MAP_ANONYMOUS;
-} else {
-guardfd = fd;
-flags |= MAP_NORESERVE;
-}
-#else
-guardfd = -1;
-flags = MAP_PRIVATE | MAP_ANONYMOUS;
-#endif
-
-guardptr = mmap(0, total, PROT_NONE, flags, guardfd, 0);
-
+guardptr = mmap_reserve(total, fd);
 if (guardptr == MAP_FAILED) {
 return MAP_FAILED;
 }
-- 
2.29.2

[PATCH v4 13/14] qmp: Include "reserve" property of memory backends

[David Hildenbrand]

Let's include the new property.

Cc: Eric Blake 
Cc: Markus Armbruster 
Signed-off-by: David Hildenbrand 
---
 hw/core/machine-qmp-cmds.c | 1 +
 qapi/machine.json  | 6 ++
 2 files changed, 7 insertions(+)

diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
index 68a942595a..bd2a7f2dd0 100644
--- a/hw/core/machine-qmp-cmds.c
+++ b/hw/core/machine-qmp-cmds.c
@@ -174,6 +174,7 @@ static int query_memdev(Object *obj, void *opaque)
 m->merge = object_property_get_bool(obj, "merge", &error_abort);
 m->dump = object_property_get_bool(obj, "dump", &error_abort);
 m->prealloc = object_property_get_bool(obj, "prealloc", &error_abort);
+m->reserve = object_property_get_bool(obj, "reserve", &error_abort);
 m->policy = object_property_get_enum(obj, "policy", "HostMemPolicy",
  &error_abort);
 host_nodes = object_property_get_qobject(obj,
diff --git a/qapi/machine.json b/qapi/machine.json
index c0c52aef10..12860a1f79 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -814,6 +814,11 @@
 #
 # @prealloc: enables or disables memory preallocation
 #
+# @reserve: enables or disables reservation of swap space (or huge pages
+#   if applicable). If reservation is enabled (default), actual
+#   reservation depends on underlying OS support. In contrast,
+#   disabling reservation without OS support will bail out. (since 6.1)
+#
 # @host-nodes: host nodes for its memory policy
 #
 # @policy: memory policy of memory backend
@@ -827,6 +832,7 @@
 'merge':  'bool',
 'dump':   'bool',
 'prealloc':   'bool',
+'reserve':'bool',
 'host-nodes': ['uint16'],
 'policy': 'HostMemPolicy' }}
 
-- 
2.29.2

[PATCH v4 07/14] softmmu/memory: Pass ram_flags to qemu_ram_alloc_from_fd()

[David Hildenbrand]

Let's pass in ram flags just like we do with qemu_ram_alloc_from_file(),
to clean up and prepare for more flags.

Simplify the documentation of passed ram flags: Looking at our
documentation of RAM_SHARED and RAM_PMEM is sufficient, no need to be
repetitive.

Reviewed-by: Peter Xu 
Signed-off-by: David Hildenbrand 
---
 backends/hostmem-memfd.c | 7 ---
 hw/misc/ivshmem.c| 5 ++---
 include/exec/memory.h| 9 +++--
 include/exec/ram_addr.h  | 6 +-
 softmmu/memory.c | 7 +++
 5 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
index 69b0ae30bb..93b5d1a4cf 100644
--- a/backends/hostmem-memfd.c
+++ b/backends/hostmem-memfd.c
@@ -36,6 +36,7 @@ static void
 memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
 {
 HostMemoryBackendMemfd *m = MEMORY_BACKEND_MEMFD(backend);
+uint32_t ram_flags;
 char *name;
 int fd;
 
@@ -53,9 +54,9 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error 
**errp)
 }
 
 name = host_memory_backend_get_name(backend);
-memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend),
-   name, backend->size,
-   backend->share, fd, 0, errp);
+ram_flags = backend->share ? RAM_SHARED : 0;
+memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend), name,
+   backend->size, ram_flags, fd, 0, errp);
 g_free(name);
 }
 
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index a1fa4878be..1ba4a98377 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -493,9 +493,8 @@ static void process_msg_shmem(IVShmemState *s, int fd, 
Error **errp)
 size = buf.st_size;
 
 /* mmap the region and map into the BAR2 */
-memory_region_init_ram_from_fd(&s->server_bar2, OBJECT(s),
-   "ivshmem.bar2", size, true, fd, 0,
-   &local_err);
+memory_region_init_ram_from_fd(&s->server_bar2, OBJECT(s), "ivshmem.bar2",
+   size, RAM_SHARED, fd, 0, &local_err);
 if (local_err) {
 error_propagate(errp, local_err);
 return;
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 54ccf1a5f0..9ffd8c662c 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -967,10 +967,7 @@ void memory_region_init_resizeable_ram(MemoryRegion *mr,
  * @size: size of the region.
  * @align: alignment of the region base address; if 0, the default alignment
  * (getpagesize()) will be used.
- * @ram_flags: Memory region features:
- * - RAM_SHARED: memory must be mmaped with the MAP_SHARED flag
- * - RAM_PMEM: the memory is persistent memory
- * Other bits are ignored now.
+ * @ram_flags: RamBlock flags. Supported flags: RAM_SHARED, RAM_PMEM.
  * @path: the path in which to allocate the RAM.
  * @readonly: true to open @path for reading, false for read/write.
  * @errp: pointer to Error*, to store an error if it happens.
@@ -996,7 +993,7 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
  * @owner: the object that tracks the region's reference count
  * @name: the name of the region.
  * @size: size of the region.
- * @share: %true if memory must be mmaped with the MAP_SHARED flag
+ * @ram_flags: RamBlock flags. Supported flags: RAM_SHARED, RAM_PMEM.
  * @fd: the fd to mmap.
  * @offset: offset within the file referenced by fd
  * @errp: pointer to Error*, to store an error if it happens.
@@ -1008,7 +1005,7 @@ void memory_region_init_ram_from_fd(MemoryRegion *mr,
 Object *owner,
 const char *name,
 uint64_t size,
-bool share,
+uint32_t ram_flags,
 int fd,
 ram_addr_t offset,
 Error **errp);
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 3cb9791df3..a7e3378340 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -104,11 +104,7 @@ long qemu_maxrampagesize(void);
  * Parameters:
  *  @size: the size in bytes of the ram block
  *  @mr: the memory region where the ram block is
- *  @ram_flags: specify the properties of the ram block, which can be one
- *  or bit-or of following values
- *  - RAM_SHARED: mmap the backing file or device with MAP_SHARED
- *  - RAM_PMEM: the backend @mem_path or @fd is persistent memory
- *  Other bits are ignored.
+ *  @ram_flags: RamBlock flags. Supported flags: RAM_SHARED, RAM_PMEM.
  *  @mem_path or @fd: specify the backing file or device
  *  @readonly: true to open @path for reading, false for read/write.
  *  @errp: pointer to Error*, to store an error if it happens
diff --git a/softmmu/me

[PATCH v4 12/14] hostmem: Wire up RAM_NORESERVE via "reserve" property

[David Hildenbrand]

Let's provide a way to control the use of RAM_NORESERVE via memory
backends using the "reserve" property which defaults to true (old
behavior).

Only Linux currently supports setting the flag (and support is checked at
runtime, depending on the setting of "/proc/sys/vm/overcommit_memory").
Windows and other POSIX systems will bail out.

The target use case is virtio-mem, which dynamically exposes memory
inside a large, sparse memory area to the VM. This essentially allows
avoiding to set "/proc/sys/vm/overcommit_memory == 0") when using
virtio-mem and also supporting hugetlbfs in the future.

Reviewed-by: Peter Xu 
Reviewed-by: Eduardo Habkost 
Signed-off-by: David Hildenbrand 
---
 backends/hostmem-file.c  | 11 ++-
 backends/hostmem-memfd.c |  1 +
 backends/hostmem-ram.c   |  1 +
 backends/hostmem.c   | 33 +
 include/sysemu/hostmem.h |  2 +-
 5 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index b683da9daf..9d550e53d4 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -40,6 +40,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error 
**errp)
object_get_typename(OBJECT(backend)));
 #else
 HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
+uint32_t ram_flags;
 gchar *name;
 
 if (!backend->size) {
@@ -52,11 +53,11 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error 
**errp)
 }
 
 name = host_memory_backend_get_name(backend);
-memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
- name,
- backend->size, fb->align,
- (backend->share ? RAM_SHARED : 0) |
- (fb->is_pmem ? RAM_PMEM : 0),
+ram_flags = backend->share ? RAM_SHARED : 0;
+ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
+ram_flags |= fb->is_pmem ? RAM_PMEM : 0;
+memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), name,
+ backend->size, fb->align, ram_flags,
  fb->mem_path, fb->readonly, errp);
 g_free(name);
 #endif
diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
index 93b5d1a4cf..f3436b623d 100644
--- a/backends/hostmem-memfd.c
+++ b/backends/hostmem-memfd.c
@@ -55,6 +55,7 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error 
**errp)
 
 name = host_memory_backend_get_name(backend);
 ram_flags = backend->share ? RAM_SHARED : 0;
+ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
 memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend), name,
backend->size, ram_flags, fd, 0, errp);
 g_free(name);
diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
index 741e701062..b8e55cdbd0 100644
--- a/backends/hostmem-ram.c
+++ b/backends/hostmem-ram.c
@@ -29,6 +29,7 @@ ram_backend_memory_alloc(HostMemoryBackend *backend, Error 
**errp)
 
 name = host_memory_backend_get_name(backend);
 ram_flags = backend->share ? RAM_SHARED : 0;
+ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
 memory_region_init_ram_flags_nomigrate(&backend->mr, OBJECT(backend), name,
backend->size, ram_flags, errp);
 g_free(name);
diff --git a/backends/hostmem.c b/backends/hostmem.c
index c6c1ff5b99..9ec5141112 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -217,6 +217,11 @@ static void host_memory_backend_set_prealloc(Object *obj, 
bool value,
 Error *local_err = NULL;
 HostMemoryBackend *backend = MEMORY_BACKEND(obj);
 
+if (!backend->reserve && value) {
+error_setg(errp, "'prealloc=on' and 'reserve=off' are incompatible");
+return;
+}
+
 if (!host_memory_backend_mr_inited(backend)) {
 backend->prealloc = value;
 return;
@@ -268,6 +273,7 @@ static void host_memory_backend_init(Object *obj)
 /* TODO: convert access to globals to compat properties */
 backend->merge = machine_mem_merge(machine);
 backend->dump = machine_dump_guest_core(machine);
+backend->reserve = true;
 backend->prealloc_threads = 1;
 }
 
@@ -426,6 +432,28 @@ static void host_memory_backend_set_share(Object *o, bool 
value, Error **errp)
 backend->share = value;
 }
 
+static bool host_memory_backend_get_reserve(Object *o, Error **errp)
+{
+HostMemoryBackend *backend = MEMORY_BACKEND(o);
+
+return backend->reserve;
+}
+
+static void host_memory_backend_set_reserve(Object *o, bool value, Error 
**errp)
+{
+HostMemoryBackend *backend = MEMORY_BACKEND(o);
+
+if (host_memory_backend_mr_inited(backend)) {
+error_setg(errp, "cannot change property value");
+return;
+}
+if (backend->prealloc && !value) {
+error_setg(errp, "'prealloc=on' and 'reserve=off' are incompatible");
+re

[PATCH v2 2/2] cirrus.yml: Update the FreeBSD task to version 12.2

[Thomas Huth]

FreeBSD version 12.1 is out of service now, and the task in the
Cirrus-CI is failing. Update to 12.2 to get it working again.
Unfortunately, there is a bug in libtasn1 that triggers with the
new version of Clang that is used there (see this thread for details:
https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg00739.html ),
so we have to disable gnutls for now to make it work again. We can
enable it later again once libtasn1 has been fixed in FreeBSD.

Signed-off-by: Thomas Huth 
---
 v2: Added a TODO comment so that we remember to enable gnutls later again

 .cirrus.yml | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index bc40a0550d..f53c519447 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -3,7 +3,7 @@ env:
 
 freebsd_12_task:
   freebsd_instance:
-image_family: freebsd-12-1
+image_family: freebsd-12-2
 cpu: 8
 memory: 8G
   install_script:
@@ -13,7 +13,10 @@ freebsd_12_task:
   script:
 - mkdir build
 - cd build
-- ../configure --enable-werror || { cat config.log 
meson-logs/meson-log.txt; exit 1; }
+# TODO: Enable gnutls again once FreeBSD's libtasn1 got fixed
+# See: https://gitlab.com/gnutls/libtasn1/-/merge_requests/71
+- ../configure --enable-werror --disable-gnutls
+  || { cat config.log meson-logs/meson-log.txt; exit 1; }
 - gmake -j$(sysctl -n hw.ncpu)
 - gmake -j$(sysctl -n hw.ncpu) check V=1
 
-- 
2.27.0

[PATCH v4 08/14] softmmu/memory: Pass ram_flags to memory_region_init_ram_shared_nomigrate()

[David Hildenbrand]

Let's forward ram_flags instead, renaming
memory_region_init_ram_shared_nomigrate() into
memory_region_init_ram_flags_nomigrate(). Forward flags to
qemu_ram_alloc() and qemu_ram_alloc_internal().

Reviewed-by: Peter Xu 
Signed-off-by: David Hildenbrand 
---
 backends/hostmem-ram.c|  6 +++--
 hw/m68k/next-cube.c   |  4 ++--
 include/exec/memory.h | 24 +--
 include/exec/ram_addr.h   |  2 +-
 .../memory-region-housekeeping.cocci  |  8 +++
 softmmu/memory.c  | 20 
 softmmu/physmem.c | 24 ---
 7 files changed, 43 insertions(+), 45 deletions(-)

diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
index 5cc53e76c9..741e701062 100644
--- a/backends/hostmem-ram.c
+++ b/backends/hostmem-ram.c
@@ -19,6 +19,7 @@
 static void
 ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
 {
+uint32_t ram_flags;
 char *name;
 
 if (!backend->size) {
@@ -27,8 +28,9 @@ ram_backend_memory_alloc(HostMemoryBackend *backend, Error 
**errp)
 }
 
 name = host_memory_backend_get_name(backend);
-memory_region_init_ram_shared_nomigrate(&backend->mr, OBJECT(backend), 
name,
-   backend->size, backend->share, errp);
+ram_flags = backend->share ? RAM_SHARED : 0;
+memory_region_init_ram_flags_nomigrate(&backend->mr, OBJECT(backend), name,
+   backend->size, ram_flags, errp);
 g_free(name);
 }
 
diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c
index 92b45d760f..59ccae0d5e 100644
--- a/hw/m68k/next-cube.c
+++ b/hw/m68k/next-cube.c
@@ -986,8 +986,8 @@ static void next_cube_init(MachineState *machine)
 sysbus_mmio_map(SYS_BUS_DEVICE(pcdev), 1, 0x0210);
 
 /* BMAP memory */
-memory_region_init_ram_shared_nomigrate(bmapm1, NULL, "next.bmapmem", 64,
-true, &error_fatal);
+memory_region_init_ram_flags_nomigrate(bmapm1, NULL, "next.bmapmem", 64,
+   RAM_SHARED, &error_fatal);
 memory_region_add_subregion(sysmem, 0x020c, bmapm1);
 /* The Rev_2.5_v66.bin firmware accesses it at 0x820c0020, too */
 memory_region_init_alias(bmapm2, NULL, "next.bmapmem2", bmapm1, 0x0, 64);
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 9ffd8c662c..8879821371 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -904,27 +904,27 @@ void memory_region_init_ram_nomigrate(MemoryRegion *mr,
   Error **errp);
 
 /**
- * memory_region_init_ram_shared_nomigrate:  Initialize RAM memory region.
- *   Accesses into the region will
- *   modify memory directly.
+ * memory_region_init_ram_flags_nomigrate:  Initialize RAM memory region.
+ *  Accesses into the region will
+ *  modify memory directly.
  *
  * @mr: the #MemoryRegion to be initialized.
  * @owner: the object that tracks the region's reference count
  * @name: Region name, becomes part of RAMBlock name used in migration stream
  *must be unique within any device
  * @size: size of the region.
- * @share: allow remapping RAM to different addresses
+ * @ram_flags: RamBlock flags. Supported flags: RAM_SHARED.
  * @errp: pointer to Error*, to store an error if it happens.
  *
- * Note that this function is similar to memory_region_init_ram_nomigrate.
- * The only difference is part of the RAM region can be remapped.
+ * Note that this function does not do anything to cause the data in the
+ * RAM memory region to be migrated; that is the responsibility of the caller.
  */
-void memory_region_init_ram_shared_nomigrate(MemoryRegion *mr,
- Object *owner,
- const char *name,
- uint64_t size,
- bool share,
- Error **errp);
+void memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
+Object *owner,
+const char *name,
+uint64_t size,
+uint32_t ram_flags,
+Error **errp);
 
 /**
  * memory_region_init_resizeable_ram:  Initialize memory region with resizeable
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index a7e3378340..6d4513f8e2 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -122,7 +122,7 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, 
MemoryRegion *mr,
 
 RAMBlock *qemu_ram_allo

Re: [PATCH v7 00/13] virtio-mem: vfio support

[David Hildenbrand]

On 24.02.21 10:48, David Hildenbrand wrote:

A virtio-mem device manages a memory region in guest physical address
space, represented as a single (currently large) memory region in QEMU,
mapped into system memory address space. Before the guest is allowed to use
memory blocks, it must coordinate with the hypervisor (plug blocks). After
a reboot, all memory is usually unplugged - when the guest comes up, it
detects the virtio-mem device and selects memory blocks to plug (based on
resize requests from the hypervisor).

Memory hot(un)plug consists of (un)plugging memory blocks via a virtio-mem
device (triggered by the guest). When unplugging blocks, we discard the
memory - similar to memory balloon inflation. In contrast to memory
ballooning, we always know which memory blocks a guest may actually use -
especially during a reboot, after a crash, or after kexec (and during
hibernation as well). Guests agreed to not access unplugged memory again,
especially not via DMA.

The issue with vfio is, that it cannot deal with random discards - for this
reason, virtio-mem and vfio can currently only run mutually exclusive.
Especially, vfio would currently map the whole memory region (with possible
only little/no plugged blocks), resulting in all pages getting pinned and
therefore resulting in a higher memory consumption than expected (turning
virtio-mem basically useless in these environments).

To make vfio work nicely with virtio-mem, we have to map only the plugged
blocks, and map/unmap properly when plugging/unplugging blocks (including
discarding of RAM when unplugging). We achieve that by using a new notifier
mechanism that communicates changes.

It's important to map memory in the granularity in which we could see
unmaps again (-> virtio-mem block size) - so when e.g., plugging
consecutive 100 MB with a block size of 2 MB, we need 50 mappings. When
unmapping, we can use a single vfio_unmap call for the applicable range.
We expect that the block size of virtio-mem devices will be fairly large
in the future (to not run out of mappings and to improve hot(un)plug
performance), configured by the user, when used with vfio (e.g., 128MB,
1G, ...), but it will depend on the setup.

More info regarding virtio-mem can be found at:
 https://virtio-mem.gitlab.io/

v7 is located at:
   g...@github.com:davidhildenbrand/qemu.git virtio-mem-vfio-v7

v6 -> v7:
- s/RamDiscardMgr/RamDiscardManager/
- "memory: Introduce RamDiscardManager for RAM memory regions"
-- Make RamDiscardManager/RamDiscardListener eat MemoryRegionSections
-- Replace notify_discard_all callback by double_discard_supported
-- Reshuffle the individual hunks in memory.h
-- Provide function wrappers for RamDiscardManager calls
- "memory: Helpers to copy/free a MemoryRegionSection"
-- Added
- "virtio-mem: Implement RamDiscardManager interface"
-- Work on MemoryRegionSections instead of ranges
-- Minor optimizations
- "vfio: Support for RamDiscardManager in the !vIOMMU case"
-- Simplify based on new interfaces /  MemoryRegionSections
-- Minor cleanups and optimizations
-- Add a comment regarding dirty bitmap sync.
-- Don't store "offset_within_region" in VFIORamDiscardListener
- "vfio: Support for RamDiscardManager in the vIOMMU case"
-- Adjust to new interface
- "softmmu/physmem: Don't use atomic operations in ..."
-- Rename variables
- "softmmu/physmem: Extend ram_block_discard_(require|disable) ..."
-- Rename variables
- Rebased and retested

v5 -> v6:
- "memory: Introduce RamDiscardMgr for RAM memory regions"
-- Fix variable names in one prototype.
- "virtio-mem: Don't report errors when ram_block_discard_range() fails"
-- Added
- "virtio-mem: Implement RamDiscardMgr interface"
-- Don't report an error if discarding fails
- Rebased and retested

v4 -> v5:
- "vfio: Support for RamDiscardMgr in the !vIOMMU case"
-- Added more assertions for granularity vs. iommu supported pagesize
- "vfio: Sanity check maximum number of DMA mappings with RamDiscardMgr"
-- Fix accounting of mappings
- "vfio: Disable only uncoordinated discards for VFIO_TYPE1 iommus"
-- Fence off SPAPR and add some comments regarding future support.
-- Tweak patch description
- Rebase and retest

v3 -> v4:
- "vfio: Query and store the maximum number of DMA mappings
-- Limit the patch to querying and storing only
-- Renamed to "vfio: Query and store the maximum number of possible DMA
mappings"
- "vfio: Support for RamDiscardMgr in the !vIOMMU case"
-- Remove sanity checks / warning the user
- "vfio: Sanity check maximum number of DMA mappings with RamDiscardMgr"
-- Perform sanity checks by looking at the number of memslots and all
registered RamDiscardMgr sections
- Rebase and retest
- Reshuffled the patches slightly

v2 -> v3:
- Rebased + retested
- Fixed some typos
- Added RB's

v1 -> v2:
- "memory: Introduce RamDiscardMgr for RAM memory regions"
-- Fix some errors in the documentation
-- Make register_listener() notify about populated parts and
unregister_listener() notify about 

[PATCH v4 11/14] util/mmap-alloc: Support RAM_NORESERVE via MAP_NORESERVE under Linux

[David Hildenbrand]

Let's support RAM_NORESERVE via MAP_NORESERVE on Linux. The flag has no
effect on most shared mappings - except for hugetlbfs and anonymous memory.

Linux man page:
  "MAP_NORESERVE: Do not reserve swap space for this mapping. When swap
  space is reserved, one has the guarantee that it is possible to modify
  the mapping. When swap space is not reserved one might get SIGSEGV
  upon a write if no physical memory is available. See also the discussion
  of the file /proc/sys/vm/overcommit_memory in proc(5). In kernels before
  2.6, this flag had effect only for private writable mappings."

Note that the "guarantee" part is wrong with memory overcommit in Linux.

Also, in Linux hugetlbfs is treated differently - we configure reservation
of huge pages from the pool, not reservation of swap space (huge pages
cannot be swapped).

The rough behavior is [1]:
a) !Hugetlbfs:

  1) Without MAP_NORESERVE *or* with memory overcommit under Linux
 disabled ("/proc/sys/vm/overcommit_memory == 2"), the following
 accounting/reservation happens:
  For a file backed map
   SHARED or READ-only - 0 cost (the file is the map not swap)
   PRIVATE WRITABLE - size of mapping per instance

  For an anonymous or /dev/zero map
   SHARED   - size of mapping
   PRIVATE READ-only - 0 cost (but of little use)
   PRIVATE WRITABLE - size of mapping per instance

  2) With MAP_NORESERVE, no accounting/reservation happens.

b) Hugetlbfs:

  1) Without MAP_NORESERVE, huge pages are reserved.

  2) With MAP_NORESERVE, no huge pages are reserved.

Note: With "/proc/sys/vm/overcommit_memory == 0", we were already able
to configure it for !hugetlbfs globally; this toggle now allows
configuring it more fine-grained, not for the whole system.

The target use case is virtio-mem, which dynamically exposes memory
inside a large, sparse memory area to the VM.

[1] https://www.kernel.org/doc/Documentation/vm/overcommit-accounting

Signed-off-by: David Hildenbrand 
---
 include/qemu/osdep.h |  3 ++
 softmmu/physmem.c|  1 +
 util/mmap-alloc.c| 69 ++--
 3 files changed, 71 insertions(+), 2 deletions(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 877d9a5eff..d5c1860508 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -176,6 +176,9 @@ extern int daemon(int, int);
 #ifndef MAP_FIXED_NOREPLACE
 #define MAP_FIXED_NOREPLACE 0
 #endif
+#ifndef MAP_NORESERVE
+#define MAP_NORESERVE 0
+#endif
 #ifndef ENOMEDIUM
 #define ENOMEDIUM ENODEV
 #endif
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 608ba75d18..0d4376b04e 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -2229,6 +2229,7 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
 flags = MAP_FIXED;
 flags |= block->flags & RAM_SHARED ?
  MAP_SHARED : MAP_PRIVATE;
+flags |= block->flags & RAM_NORESERVE ? MAP_NORESERVE : 0;
 if (block->fd >= 0) {
 area = mmap(vaddr, length, PROT_READ | PROT_WRITE,
 flags, block->fd, offset);
diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
index d0cf4aaee5..838e286ce5 100644
--- a/util/mmap-alloc.c
+++ b/util/mmap-alloc.c
@@ -20,6 +20,7 @@
 #include "qemu/osdep.h"
 #include "qemu/mmap-alloc.h"
 #include "qemu/host-utils.h"
+#include "qemu/cutils.h"
 #include "qemu/error-report.h"
 
 #define HUGETLBFS_MAGIC   0x958458f6
@@ -83,6 +84,70 @@ size_t qemu_mempath_getpagesize(const char *mem_path)
 return qemu_real_host_page_size;
 }
 
+#define OVERCOMMIT_MEMORY_PATH "/proc/sys/vm/overcommit_memory"
+static bool map_noreserve_effective(int fd, uint32_t qemu_map_flags)
+{
+#if defined(__linux__)
+const bool readonly = qemu_map_flags & QEMU_MAP_READONLY;
+const bool shared = qemu_map_flags & QEMU_MAP_SHARED;
+gchar *content = NULL;
+const char *endptr;
+unsigned int tmp;
+
+/*
+ * hugeltb accounting is different than ordinary swap reservation:
+ * a) Hugetlb pages from the pool are reserved for both private and
+ *shared mappings. For shared mappings, all mappers have to specify
+ *MAP_NORESERVE.
+ * b) MAP_NORESERVE is not affected by /proc/sys/vm/overcommit_memory.
+ */
+if (qemu_fd_getpagesize(fd) != qemu_real_host_page_size) {
+return true;
+}
+
+/*
+ * Accountable mappings in the kernel that can be affected by MAP_NORESEVE
+ * are private writable mappings (see mm/mmap.c:accountable_mapping() in
+ * Linux). For all shared or readonly mappings, MAP_NORESERVE is always
+ * implicitly active -- no reservation; this includes shmem. The only
+ * exception is shared anonymous memory, it is accounted like private
+ * anonymous memory.
+ */
+if (readonly || (shared && fd >= 0)) {
+return true;
+}
+
+/*
+ * MAP_NORESERVE is globally ignored for applicable !hugetlb mappings when
+

[PATCH v4 09/14] util/mmap-alloc: Pass flags instead of separate bools to qemu_ram_mmap()

[David Hildenbrand]

Let's pass flags instead of bools to prepare for passing other flags and
update the documentation of qemu_ram_mmap(). Introduce new QEMU_MAP_
flags that abstract the mmap() PROT_ and MAP_ flag handling and simplify
it.

We expose only flags that are currently supported by qemu_ram_mmap().
Maybe, we'll see qemu_mmap() in the future as well that can implement these
flags.

Note: We don't use MAP_ flags as some flags (e.g., MAP_SYNC) are only
defined for some systems and we want to always be able to identify
these flags reliably inside qemu_ram_mmap() -- for example, to properly
warn when some future flags are not available or effective on a system.
Also, this way we can simplify PROT_ handling as well.

Signed-off-by: David Hildenbrand 
---
 include/qemu/mmap-alloc.h | 16 +---
 include/qemu/osdep.h  | 18 ++
 softmmu/physmem.c |  8 +---
 util/mmap-alloc.c | 15 ---
 util/oslib-posix.c|  3 ++-
 5 files changed, 42 insertions(+), 18 deletions(-)

diff --git a/include/qemu/mmap-alloc.h b/include/qemu/mmap-alloc.h
index 456ff87df1..a60a2085b3 100644
--- a/include/qemu/mmap-alloc.h
+++ b/include/qemu/mmap-alloc.h
@@ -7,18 +7,22 @@ size_t qemu_fd_getpagesize(int fd);
 size_t qemu_mempath_getpagesize(const char *mem_path);
 
 /**
- * qemu_ram_mmap: mmap the specified file or device.
+ * qemu_ram_mmap: mmap anonymous memory, the specified file or device.
+ *
+ * mmap() abstraction to map guest RAM, simplifying flag handling, taking
+ * care of alignment requirements and installing guard pages.
  *
  * Parameters:
  *  @fd: the file or the device to mmap
  *  @size: the number of bytes to be mmaped
  *  @align: if not zero, specify the alignment of the starting mapping address;
  *  otherwise, the alignment in use will be determined by QEMU.
- *  @readonly: true for a read-only mapping, false for read/write.
- *  @shared: map has RAM_SHARED flag.
- *  @is_pmem: map has RAM_PMEM flag.
+ *  @qemu_map_flags: QEMU_MAP_* flags
  *  @map_offset: map starts at offset of map_offset from the start of fd
  *
+ * Internally, MAP PRIVATE, MAP_ANONYMOUS and MAP_SHARED_VALIDATE are set
+ * implicitly based on other parameters.
+ *
  * Return:
  *  On success, return a pointer to the mapped area.
  *  On failure, return MAP_FAILED.
@@ -26,9 +30,7 @@ size_t qemu_mempath_getpagesize(const char *mem_path);
 void *qemu_ram_mmap(int fd,
 size_t size,
 size_t align,
-bool readonly,
-bool shared,
-bool is_pmem,
+uint32_t qemu_map_flags,
 off_t map_offset);
 
 void qemu_ram_munmap(int fd, void *ptr, size_t size);
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index ba15be9c56..5ba89f5460 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -347,6 +347,24 @@ void *qemu_anon_ram_alloc(size_t size, uint64_t *align, 
bool shared);
 void qemu_vfree(void *ptr);
 void qemu_anon_ram_free(void *ptr, size_t size);
 
+/*
+ * Abstraction of PROT_ and MAP_ flags as passed to mmap(), for example,
+ * consumed by qemu_ram_mmap().
+ */
+
+/* Map PROT_READ instead of PROT_READ | PROT_WRITE. */
+#define QEMU_MAP_READONLY   (1 << 0)
+
+/* Use MAP_SHARED instead of MAP_PRIVATE. */
+#define QEMU_MAP_SHARED (1 << 1)
+
+/*
+ * Use MAP_SYNC | MAP_SHARED_VALIDATE if supported. Ignored without
+ * QEMU_MAP_SHARED. If mapping fails, warn and fallback to !QEMU_MAP_SYNC.
+ */
+#define QEMU_MAP_SYNC   (1 << 2)
+
+
 #define QEMU_MADV_INVALID -1
 
 #if defined(CONFIG_MADVISE)
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index fdcd38ba61..d1a1a6b72e 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -1533,6 +1533,7 @@ static void *file_ram_alloc(RAMBlock *block,
 off_t offset,
 Error **errp)
 {
+uint32_t qemu_map_flags;
 void *area;
 
 block->page_size = qemu_fd_getpagesize(fd);
@@ -1580,9 +1581,10 @@ static void *file_ram_alloc(RAMBlock *block,
 perror("ftruncate");
 }
 
-area = qemu_ram_mmap(fd, memory, block->mr->align, readonly,
- block->flags & RAM_SHARED, block->flags & RAM_PMEM,
- offset);
+qemu_map_flags = readonly ? QEMU_MAP_READONLY : 0;
+qemu_map_flags |= (block->flags & RAM_SHARED) ? QEMU_MAP_SHARED : 0;
+qemu_map_flags |= (block->flags & RAM_PMEM) ? QEMU_MAP_SYNC : 0;
+area = qemu_ram_mmap(fd, memory, block->mr->align, qemu_map_flags, offset);
 if (area == MAP_FAILED) {
 error_setg_errno(errp, errno,
  "unable to map backing store for guest RAM");
diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
index 0e2bd7bc0e..1ddc0e2a1e 100644
--- a/util/mmap-alloc.c
+++ b/util/mmap-alloc.c
@@ -118,9 +118,12 @@ static void *mmap_reserve(size_t size, int fd)
  * Activate memory in a reserved region from the given fd (if any), to make
 

Re: [PATCH 1/2] floppy: add a regression test for CVE-2020-25741

[Paolo Bonzini]

On 19/03/21 10:54, Markus Armbruster wrote:

A commit message should tell me what the patch is trying to accomplish.

This commit message's title tells me it's a test for a CVE.  Okay.  The
body additionally gives me the reproducer.  To be useful, a reproducer
needs to come with actual and expected result.  Yes, I can find those in
the patch.  But I could find the reproducer there, too.  If you're nice
enough to save me the trouble of digging through the patch for the
reproducer (thanks), please consider saving me the trouble digging for
the information I need to make use of it (thanks again).  That's all:)


FWIW I don't think in this case there is an expected result other than 
not crashing, but yeah it may be worth adding in the commit message "for 
CVE-2020-25741, a {crash,undefined behavior,ASAN violation} in func_name".


Paolo

[PATCH v4 10/14] memory: Introduce RAM_NORESERVE and wire it up in qemu_ram_mmap()

[David Hildenbrand]

Let's introduce RAM_NORESERVE, allowing mmap'ing with MAP_NORESERVE. The
new flag has the following semantics:

"
RAM is mmap-ed with MAP_NORESERVE. When set, reserving swap space (or huge
pages if applicable) is skipped: will bail out if not supported. When not
set, the OS might do the reservation, depending on OS support.
"

Allow passing it into:
- memory_region_init_ram_nomigrate()
- memory_region_init_resizeable_ram()
- memory_region_init_ram_from_file()

... and teach qemu_ram_mmap() and qemu_anon_ram_alloc() about the flag.
Bail out if the flag is not supported, which is the case right now for
both, POSIX and win32. We will add Linux support next and allow specifying
RAM_NORESERVE via memory backends.

The target use case is virtio-mem, which dynamically exposes memory
inside a large, sparse memory area to the VM.

Signed-off-by: David Hildenbrand 
---
 include/exec/cpu-common.h |  1 +
 include/exec/memory.h | 15 ---
 include/exec/ram_addr.h   |  3 ++-
 include/qemu/osdep.h  |  9 -
 migration/ram.c   |  3 +--
 softmmu/physmem.c | 15 +++
 util/mmap-alloc.c |  7 +++
 util/oslib-posix.c|  6 --
 util/oslib-win32.c| 13 -
 9 files changed, 58 insertions(+), 14 deletions(-)

diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 5a0a2d93e0..38a47ad4ac 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -58,6 +58,7 @@ void *qemu_ram_get_host_addr(RAMBlock *rb);
 ram_addr_t qemu_ram_get_offset(RAMBlock *rb);
 ram_addr_t qemu_ram_get_used_length(RAMBlock *rb);
 bool qemu_ram_is_shared(RAMBlock *rb);
+bool qemu_ram_is_noreserve(RAMBlock *rb);
 bool qemu_ram_is_uf_zeroable(RAMBlock *rb);
 void qemu_ram_set_uf_zeroable(RAMBlock *rb);
 bool qemu_ram_is_migratable(RAMBlock *rb);
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 8879821371..7b700bdc33 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -155,6 +155,13 @@ typedef struct IOMMUTLBEvent {
  */
 #define RAM_UF_WRITEPROTECT (1 << 6)
 
+/*
+ * RAM is mmap-ed with MAP_NORESERVE. When set, reserving swap space (or huge
+ * pages if applicable) is skipped: will bail out if not supported. When not
+ * set, the OS might do the reservation, depending on OS support.
+ */
+#define RAM_NORESERVE (1 << 7)
+
 static inline void iommu_notifier_init(IOMMUNotifier *n, IOMMUNotify fn,
IOMMUNotifierFlag flags,
hwaddr start, hwaddr end,
@@ -913,7 +920,7 @@ void memory_region_init_ram_nomigrate(MemoryRegion *mr,
  * @name: Region name, becomes part of RAMBlock name used in migration stream
  *must be unique within any device
  * @size: size of the region.
- * @ram_flags: RamBlock flags. Supported flags: RAM_SHARED.
+ * @ram_flags: RamBlock flags. Supported flags: RAM_SHARED, RAM_NORESERVE.
  * @errp: pointer to Error*, to store an error if it happens.
  *
  * Note that this function does not do anything to cause the data in the
@@ -967,7 +974,8 @@ void memory_region_init_resizeable_ram(MemoryRegion *mr,
  * @size: size of the region.
  * @align: alignment of the region base address; if 0, the default alignment
  * (getpagesize()) will be used.
- * @ram_flags: RamBlock flags. Supported flags: RAM_SHARED, RAM_PMEM.
+ * @ram_flags: RamBlock flags. Supported flags: RAM_SHARED, RAM_PMEM,
+ * RAM_NORESERVE,
  * @path: the path in which to allocate the RAM.
  * @readonly: true to open @path for reading, false for read/write.
  * @errp: pointer to Error*, to store an error if it happens.
@@ -993,7 +1001,8 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
  * @owner: the object that tracks the region's reference count
  * @name: the name of the region.
  * @size: size of the region.
- * @ram_flags: RamBlock flags. Supported flags: RAM_SHARED, RAM_PMEM.
+ * @ram_flags: RamBlock flags. Supported flags: RAM_SHARED, RAM_PMEM,
+ * RAM_NORESERVE.
  * @fd: the fd to mmap.
  * @offset: offset within the file referenced by fd
  * @errp: pointer to Error*, to store an error if it happens.
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 6d4513f8e2..551876bed0 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -104,7 +104,8 @@ long qemu_maxrampagesize(void);
  * Parameters:
  *  @size: the size in bytes of the ram block
  *  @mr: the memory region where the ram block is
- *  @ram_flags: RamBlock flags. Supported flags: RAM_SHARED, RAM_PMEM.
+ *  @ram_flags: RamBlock flags. Supported flags: RAM_SHARED, RAM_PMEM,
+ *  RAM_NORESERVE.
  *  @mem_path or @fd: specify the backing file or device
  *  @readonly: true to open @path for reading, false for read/write.
  *  @errp: pointer to Error*, to store an error if it happens
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 5ba89f5460..877d9a5eff 100644
--- a/include/qemu/osdep.h
+++ b/include/q

Re: Serious doubts about Gitlab CI

[Andrew Jones]

On Fri, Mar 19, 2021 at 09:33:43AM +, Stefan Hajnoczi wrote:
> On Thu, Mar 18, 2021 at 09:30:41PM +0100, Paolo Bonzini wrote:
> > On 18/03/21 20:46, Stefan Hajnoczi wrote:
> > > The QEMU Project has 50,000 minutes of GitLab CI quota. Let's enable
> > > GitLab Merge Requests so that anyone can submit a merge request and get
> > > CI coverage.
> > 
> > Each merge request consumes about 2500.  That won't last long.
> 
> Yikes, that is 41 hours per CI run. I wonder if GitLab's CI minutes are
> on slow machines or if we'll hit the same issue with dedicated runners.
> It seems like CI optimization will be necessary...
>

We need to reduce the amount of CI we do, not only because we can't afford
it, but because it's wasteful. I hate to think of all the kWhs spent
testing the exact same code in the exact same way, since everyone runs
everything with a simple 'git push'. IMHO, 'git push' shouldn't trigger
anything. Starting CI should be an explicit step. Also, the default CI
should only trigger tests associated with the code changed. One should
have to explicitly trigger a complete CI when they deem it worthwhile.

Do we already have some sort of CI limiters? Or is anybody looking at
that for QEMU? Are people aware of other projects that try to limit
their CI and how they do it?

Thanks,
drew

[PATCH v4 14/14] hmp: Print "reserve" property of memory backends with "info memdev"

[David Hildenbrand]

Let's print the new property.

Cc: Markus Armbruster 
Cc: Eric Blake 
Signed-off-by: David Hildenbrand 
---
 hw/core/machine-hmp-cmds.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/core/machine-hmp-cmds.c b/hw/core/machine-hmp-cmds.c
index 58248cffa3..fb717f69b9 100644
--- a/hw/core/machine-hmp-cmds.c
+++ b/hw/core/machine-hmp-cmds.c
@@ -110,6 +110,8 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
m->value->dump ? "true" : "false");
 monitor_printf(mon, "  prealloc: %s\n",
m->value->prealloc ? "true" : "false");
+monitor_printf(mon, "  reserve: %s\n",
+   m->value->reserve ? "true" : "false");
 monitor_printf(mon, "  policy: %s\n",
HostMemPolicy_str(m->value->policy));
 visit_complete(v, &str);
-- 
2.29.2

Re: [PATCH v2] MAINTAINERS: Fix tests/migration maintainers

[Philippe Mathieu-Daudé]

On 3/19/21 9:07 AM, huang...@chinatelecom.cn wrote:
> From: Hyman Huang(黄勇) 
> 
> when executing the following scripts, it throw error message:
> $ ./scripts/get_maintainer.pl -f tests/migration/guestperf.py
> get_maintainer.pl: No maintainers found, printing recent contributors.
> get_maintainer.pl: Do not blindly cc: them on patches!  Use common sense.
> 
> add the tests/migration to the "Migration" section of MAINTAINERS
> 
> Signed-off-by: Hyman Huang(黄勇) 
> ---
>  MAINTAINERS | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Philippe Mathieu-Daudé 

ReplayKernelNormal.test_mips_malta functional test timeouting

[Philippe Mathieu-Daudé]

Hi Pavel,

The "normal" test_mips_malta timeouted on acceptance-system-fedora job:

 (23/36)
tests/acceptance/replay_kernel.py:ReplayKernelNormal.test_mips_malta:
INTERRUPTED: Test interrupted by SIGTERM\nRunner error occurred: Timeout
reached\nOriginal status: ERROR\n{'name':
'23-tests/acceptance/replay_kernel.py:ReplayKernelNormal.test_mips_malta',
'logdir':
'/builds/philmd/qemu2/build/tests/results/job-2021-03-19T09.38-e5751b5/...
(120.52 s)

artifact's debug.log:

09:43:04 DEBUG| PARAMS (key=arch, path=*, default=mips) => 'mips'
09:43:04 DEBUG| PARAMS (key=machine, path=*, default=malta) => 'malta'
09:43:04 DEBUG| PARAMS (key=qemu_bin, path=*,
default=./qemu-system-mips) => './qemu-system-mips'
09:43:04 INFO | Running 'ar t
/builds/philmd/qemu2/avocado-cache/by_location/44bac84408e676508a64ecba77e99389ac8fe10d/linux-image-2.6.32-5-4kc-malta_2.6.32-48_mips.deb'
09:43:04 DEBUG| [stdout] debian-binary
09:43:04 INFO | Command 'ar t
/builds/philmd/qemu2/avocado-cache/by_location/44bac84408e676508a64ecba77e99389ac8fe10d/linux-image-2.6.32-5-4kc-malta_2.6.32-48_mips.deb'
finished with 0 after 0.0019164085388183594s
09:43:04 DEBUG| [stdout] control.tar.gz
09:43:04 DEBUG| [stdout] data.tar.gz
09:43:04 INFO | Running 'ar x
/builds/philmd/qemu2/avocado-cache/by_location/44bac84408e676508a64ecba77e99389ac8fe10d/linux-image-2.6.32-5-4kc-malta_2.6.32-48_mips.deb
data.tar.gz'
09:43:04 INFO | Command 'ar x
/builds/philmd/qemu2/avocado-cache/by_location/44bac84408e676508a64ecba77e99389ac8fe10d/linux-image-2.6.32-5-4kc-malta_2.6.32-48_mips.deb
data.tar.gz' finished with 0 after 0.047913551330566406s
09:43:05 INFO | recording the execution...
09:43:05 DEBUG| VM launch command: './qemu-system-mips -display none
-vga none -chardev
socket,id=mon,path=/var/tmp/avo_qemu_sock_z2x1qvna/qemu-601-monitor.sock
-mon chardev=mon,mode=control -machine malta -chardev
socket,id=console,path=/var/tmp/avo_qemu_sock_z2x1qvna/qemu-601-console.sock,server=on,wait=off
-serial chardev:console -icount
shift=5,rr=record,rrfile=/var/tmp/avocado__uxji4xt/avocado_job_j3sdjxv9/23-tests_acceptance_replay_kernel.py_ReplayKernelNormal.test_mips_malta/replay.bin
-kernel
/var/tmp/avocado__uxji4xt/avocado_job_j3sdjxv9/23-tests_acceptance_replay_kernel.py_ReplayKernelNormal.test_mips_malta/boot/vmlinux-2.6.32-5-4kc-malta
-append printk.time=1 panic=-1 console=ttyS0 -net none -no-reboot'
09:43:05 DEBUG| >>> {'execute': 'qmp_capabilities'}
09:43:05 DEBUG| <<< {'return': {}}
09:43:05 DEBUG| [0.00] Initrd not found or empty - disabling initrd
09:43:05 DEBUG| [0.00] Zone PFN ranges:
09:43:05 DEBUG| [0.00]   DMA  0x -> 0x1000
09:43:05 DEBUG| [0.00]   Normal   0x1000 -> 0x7fff
09:43:05 DEBUG| [0.00] Movable zone start PFN for each node
09:43:05 DEBUG| [0.00] early_node_map[1] active PFN ranges
09:43:05 DEBUG| [0.00] 0: 0x -> 0x7fff
09:43:05 DEBUG| [0.00] Built 1 zonelists in Zone order, mobility
grouping on.  Total pages: 32511
09:43:05 DEBUG| [0.00] Kernel command line: printk.time=1
panic=-1 console=ttyS0
09:43:05 DEBUG| >>> {'execute': 'quit'}
09:43:05 DEBUG| <<< {'return': {}}
09:43:05 INFO | finished the recording with log size 21979 bytes
09:43:05 INFO | elapsed time 0.13 sec
09:43:05 INFO | replaying the execution...
09:43:05 DEBUG| VM launch command: './qemu-system-mips -display none
-vga none -chardev
socket,id=mon,path=/var/tmp/avo_qemu_sock_opalepcn/qemu-601-monitor.sock
-mon chardev=mon,mode=control -machine malta -chardev
socket,id=console,path=/var/tmp/avo_qemu_sock_opalepcn/qemu-601-console.sock,server=on,wait=off
-serial chardev:console -icount
shift=5,rr=replay,rrfile=/var/tmp/avocado__uxji4xt/avocado_job_j3sdjxv9/23-tests_acceptance_replay_kernel.py_ReplayKernelNormal.test_mips_malta/replay.bin
-kernel
/var/tmp/avocado__uxji4xt/avocado_job_j3sdjxv9/23-tests_acceptance_replay_kernel.py_ReplayKernelNormal.test_mips_malta/boot/vmlinux-2.6.32-5-4kc-malta
-append printk.time=1 panic=-1 console=ttyS0 -net none -no-reboot'
09:43:05 DEBUG| >>> {'execute': 'qmp_capabilities'}
09:43:05 DEBUG| <<< {'return': {}}
09:43:06 DEBUG| [0.00] Initrd not found or empty - disabling initrd
09:43:06 DEBUG| [0.00] Zone PFN ranges:
09:43:06 DEBUG| [0.00]   DMA  0x -> 0x1000
09:43:06 DEBUG| [0.00]   Normal   0x1000 -> 0x7fff
09:43:06 DEBUG| [0.00] Movable zone start PFN for each node
09:43:06 DEBUG| [0.00] early_node_map[1] active PFN ranges
09:43:06 DEBUG| [0.00]
09:45:05 ERROR|
09:45:05 ERROR| Reproduced traceback from:
/builds/philmd/qemu2/build/tests/venv/lib64/python3.9/site-packages/avocado/core/test.py:767
09:45:05 ERROR| Traceback (most recent call last):
09:45:05 ERROR|   File
"/builds/philmd/qemu2/build/tests/acceptance/replay_kernel.py", line
114, in test_mips_malta
09:45:05 ERROR| self.run_rr(kernel_path, kernel_command_line,
console_pattern, shift=5)
09:45:05 ERROR

Re: [PATCH] hw/display/bcm2835_fb: Remove DeviceReset() call in DeviceRealize()

[Peter Maydell]

On Sat, 13 Mar 2021 at 17:01, Philippe Mathieu-Daudé  wrote:
>
> When QDev objects have their DeviceReset handler set, they
> shouldn't worry about calling it at realization stage (it
> is handled by hw/core/qdev.c::device_set_realized).
>
> Remove the pointless/confusing bcm2835_fb_reset() call.
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/display/bcm2835_fb.c | 2 --


Applied to target-arm.next, thanks.

-- PMM

Re: [PATCH] hw/arm/virt: Disable pl011 clock migration if needed

[Peter Maydell]

On Thu, 18 Mar 2021 at 02:38, Gavin Shan  wrote:
>
> A clock is added by commit aac63e0e6ea3 ("hw/char/pl011: add a clock
> input") since v5.2.0 which corresponds to virt-5.2 machine type. It
> causes backwards migration failure from upstream to downstream (v5.1.0)
> when the machine type is specified with virt-5.1.
>
> This fixes the issue by following instructions from section "Connecting
> subsections to properties" in docs/devel/migration.rst. With this applied,
> the PL011 clock is migrated based on the machine type.
>
>virt-5.2 or newer:  migration
>virt-5.1 or older:  non-migration
>
> Cc: qemu-sta...@nongnu.org # v5.2.0+
> Fixes: aac63e0e6ea3 ("hw/char/pl011: add a clock input")
> Suggested-by: Andrew Jones 
> Signed-off-by: Gavin Shan 



Applied to target-arm.next, thanks.

-- PMM

Re: [PATCH 2/4] iotests: Revert "iotests: use -ccw on s390x for 040, 139, and 182"

[Philippe Mathieu-Daudé]

On 3/18/21 11:39 PM, Laurent Vivier wrote:
> Commit f1d5516ab583 introduces a test in some iotests to check if
> the machine is a s390-ssw-virtio and to select virtio-*-ccw rather
> than virtio-*-pci.
> 
> We don't need that because QEMU already provides aliases to use the correct
> virtio interface according to the machine type.
> 
> This patch removes all virtio-*-pci and virtio-*-ccw to use virtio-*
> instead.
> 
> Signed-off-by: Laurent Vivier 
> cc: Cornelia Huck 
> ---
>  blockdev.c|  6 +-
>  tests/qemu-iotests/040|  2 +-
>  tests/qemu-iotests/051| 12 +---
>  tests/qemu-iotests/068|  4 +---
>  tests/qemu-iotests/093|  3 +--
>  tests/qemu-iotests/139|  9 ++---
>  tests/qemu-iotests/182| 13 ++---
>  tests/qemu-iotests/238|  4 +---
>  tests/qemu-iotests/240| 10 +-
>  tests/qemu-iotests/257|  4 ++--
>  tests/qemu-iotests/307|  4 +---
>  tests/qemu-iotests/iotests.py |  5 -
>  12 files changed, 18 insertions(+), 58 deletions(-)

> diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
> index 7cbd1415ce7b..00382cc55e25 100755
> --- a/tests/qemu-iotests/051
> +++ b/tests/qemu-iotests/051
> @@ -119,17 +119,7 @@ echo
>  echo === Device without drive ===
>  echo
>  
> -case "$QEMU_DEFAULT_MACHINE" in
> -  s390-ccw-virtio)
> -  virtio_scsi=virtio-scsi-ccw
> -  ;;
> -  *)
> -  virtio_scsi=virtio-scsi-pci
> -  ;;
> -esac
> -
> -run_qemu -device $virtio_scsi -device scsi-hd |
> -sed -e "s/$virtio_scsi/VIRTIO_SCSI/"
> +run_qemu -device virtio-scsi -device scsi-hd
>  
>  echo
>  echo === Overriding backing file ===

This one failed (but the fix is trivial):

051   fail   [05:49:47] [05:50:01]   13.3soutput
mismatch (see 051.out.bad)
--- /builds/philmd/qemu2/tests/qemu-iotests/051.pc.out
+++ 051.out.bad
@@ -72,7 +72,7 @@
 === Device without drive ===
-Testing: -device VIRTIO_SCSI -device scsi-hd
+Testing: -device virtio-scsi -device scsi-hd
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) QEMU_PROG: -device scsi-hd: drive property not set

Failures: 051
Failed 1 of 56 iotests

Re: [PATCH] target/arm: Make number of counters in PMCR follow the CPU

[Peter Maydell]

Ping for review, testing, opinions on whether this should go into 6.0 ?
I think I would overall prefer it to the just-bump-PMCR_NUM_COUNTERS
patch...

thanks
-- PMM

On Thu, 11 Mar 2021 at 16:59, Peter Maydell  wrote:
>
> Currently we give all the v7-and-up CPUs a PMU with 4 counters.  This
> means that we don't provide the 6 counters that are required by the
> Arm BSA (Base System Architecture) specification if the CPU supports
> the Virtualization extensions.
>
> Instead of having a single PMCR_NUM_COUNTERS, make each CPU type
> specify the PMCR reset value (obtained from the appropriate TRM), and
> use the 'N' field of that value to define the number of counters
> provided.
>
> This means that we now supply 6 counters for Cortex-A53, A57, A72,
> A15 and A9 as well as '-cpu max'; Cortex-A7 and A8 stay at 4; and
> Cortex-R5 goes down to 3.
>
> Note that because we now use the PMCR reset value of the specific
> implementation, we no longer set the LC bit out of reset.  This has
> an UNKNOWN value out of reset for all cores with any AArch32 support,
> so guest software should be setting it anyway if it wants it.
>
> Signed-off-by: Peter Maydell 
> ---
> This is pretty much untested (I just checked Linux still boots;
> haven't tried it with KVM either). It's an alternative to
> just bumping PMCR_NUM_COUNTERS to 6.
> ---
>  target/arm/cpu.h |  1 +
>  target/arm/cpu64.c   |  3 +++
>  target/arm/cpu_tcg.c |  5 +
>  target/arm/helper.c  | 29 +
>  target/arm/kvm64.c   |  2 ++
>  5 files changed, 28 insertions(+), 12 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 193a49ec7fa..fe68f464b3a 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -942,6 +942,7 @@ struct ARMCPU {
>  uint64_t id_aa64mmfr2;
>  uint64_t id_aa64dfr0;
>  uint64_t id_aa64dfr1;
> +uint64_t reset_pmcr_el0;
>  } isar;
>  uint64_t midr;
>  uint32_t revidr;
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index f0a9e968c9c..5d9d56a33c3 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -141,6 +141,7 @@ static void aarch64_a57_initfn(Object *obj)
>  cpu->gic_num_lrs = 4;
>  cpu->gic_vpribits = 5;
>  cpu->gic_vprebits = 5;
> +cpu->isar.reset_pmcr_el0 = 0x41013000;
>  define_arm_cp_regs(cpu, cortex_a72_a57_a53_cp_reginfo);
>  }
>
> @@ -194,6 +195,7 @@ static void aarch64_a53_initfn(Object *obj)
>  cpu->gic_num_lrs = 4;
>  cpu->gic_vpribits = 5;
>  cpu->gic_vprebits = 5;
> +cpu->isar.reset_pmcr_el0 = 0x41033000;
>  define_arm_cp_regs(cpu, cortex_a72_a57_a53_cp_reginfo);
>  }
>
> @@ -245,6 +247,7 @@ static void aarch64_a72_initfn(Object *obj)
>  cpu->gic_num_lrs = 4;
>  cpu->gic_vpribits = 5;
>  cpu->gic_vprebits = 5;
> +cpu->isar.reset_pmcr_el0 = 0x41023000;
>  define_arm_cp_regs(cpu, cortex_a72_a57_a53_cp_reginfo);
>  }
>
> diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
> index 046e476f65f..8252fd29f90 100644
> --- a/target/arm/cpu_tcg.c
> +++ b/target/arm/cpu_tcg.c
> @@ -301,6 +301,7 @@ static void cortex_a8_initfn(Object *obj)
>  cpu->ccsidr[1] = 0x2007e01a; /* 16k L1 icache. */
>  cpu->ccsidr[2] = 0xf000; /* No L2 icache. */
>  cpu->reset_auxcr = 2;
> +cpu->isar.reset_pmcr_el0 = 0x41002000;
>  define_arm_cp_regs(cpu, cortexa8_cp_reginfo);
>  }
>
> @@ -373,6 +374,7 @@ static void cortex_a9_initfn(Object *obj)
>  cpu->clidr = (1 << 27) | (1 << 24) | 3;
>  cpu->ccsidr[0] = 0xe00fe019; /* 16k L1 dcache. */
>  cpu->ccsidr[1] = 0x200fe019; /* 16k L1 icache. */
> +cpu->isar.reset_pmcr_el0 = 0x41093000;
>  define_arm_cp_regs(cpu, cortexa9_cp_reginfo);
>  }
>
> @@ -443,6 +445,7 @@ static void cortex_a7_initfn(Object *obj)
>  cpu->ccsidr[0] = 0x701fe00a; /* 32K L1 dcache */
>  cpu->ccsidr[1] = 0x201fe00a; /* 32K L1 icache */
>  cpu->ccsidr[2] = 0x711fe07a; /* 4096K L2 unified cache */
> +cpu->isar.reset_pmcr_el0 = 0x41072000;
>  define_arm_cp_regs(cpu, cortexa15_cp_reginfo); /* Same as A15 */
>  }
>
> @@ -485,6 +488,7 @@ static void cortex_a15_initfn(Object *obj)
>  cpu->ccsidr[0] = 0x701fe00a; /* 32K L1 dcache */
>  cpu->ccsidr[1] = 0x201fe00a; /* 32K L1 icache */
>  cpu->ccsidr[2] = 0x711fe07a; /* 4096K L2 unified cache */
> +cpu->isar.reset_pmcr_el0 = 0x410F3000;
>  define_arm_cp_regs(cpu, cortexa15_cp_reginfo);
>  }
>
> @@ -717,6 +721,7 @@ static void cortex_r5_initfn(Object *obj)
>  cpu->isar.id_isar6 = 0x0;
>  cpu->mp_is_up = true;
>  cpu->pmsav7_dregion = 16;
> +cpu->isar.reset_pmcr_el0 = 0x41151800;
>  define_arm_cp_regs(cpu, cortexr5_cp_reginfo);
>  }
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 904b0927cd2..2f3867cad79 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -38,7 +38,6 @@
>  #endif
>
>  #define ARM_CPU_FREQ 10 /* FIXME: 1 GHz, should be configurable */
> -#define PMCR_NUM_COUNTERS 4 /

spurious error in check-dco job?

[Philippe Mathieu-Daudé]

Hi Daniel,

I'm getting this error sometimes in the check-dco job:

Using docker image
sha256:96fcfc3ceb2d65689c2918c1c501c45b2bd815d82e7fb3381048c9f252e7b046
for registry.gitlab.com/philmd/qemu2/qemu/centos8:latest with digest
registry.gitlab.com/philmd/qemu2/qemu/centos8@sha256:798eb3e5aa50eb04d60cdf1bfd08bcff5b443e933dcfd902ebda01927e2f6eb0
...
$ .gitlab-ci.d/check-dco.py
Traceback (most recent call last):
  File ".gitlab-ci.d/check-dco.py", line 25, in 
stderr=subprocess.DEVNULL)
  File "/usr/lib64/python3.6/subprocess.py", line 311, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['git', 'fetch', 'check-dco',
'master']' returned non-zero exit status 128.

https://gitlab.com/philmd/qemu2/-/jobs/1108482890

Re: [RFC 0/1] Use dmabufs for display updates instead of pixman

[Gerd Hoffmann]

> Hi,

> According to
> https://lore.kernel.org/dri-devel/20210212110140.gdpu7kapnr7ov...@sirius.home.kraxel.org/
> proposal, we made some progress on making a 'virtio-gpu (display) +
> pass-through GPU' prototype. We leverage the kmsro framework provided
> by mesa to let the virtio-gpu display work with a passed-through GPU
> in headless mode. And the MR is here:
> https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9592

Cool.

> Although our work is different from this on-going discussion which is
> about enabling a general way to share buffers between guest and host,
> we'd like to leverage this patch. So, is there any plan to refine this
> patch?

Item (1) on Vivek's new TODO list should provide that.  Once we have
shared blob resources we can create udmabufs on the host side, which
in turn allows to drop extra copies in the display path and speed up
this use case as well (both with and without opengl).

take care,
  Gerd

Re: [PULL 5/5] m68k: add Virtual M68k Machine

[Laurent Vivier]

Le 19/03/2021 à 10:20, Max Reitz a écrit :
> On 19.03.21 07:32, Thomas Huth wrote:
>> On 18/03/2021 18.28, Max Reitz wrote:
>> [...]
>>>  From that it follows that I don’t see much use in testing specific devices 
>>> either.  Say there’s
>>> a platform that provides both virtio-pci and virtio-mmio, the default (say 
>>> virtio-pci) is fine
>>> for the iotests. I see little value in testing virtio-mmio as well.  
>>> (Perhaps I’m short-sighted,
>>> though.)
>>
>> That's a fair point. But still, if someone compiled QEMU only with a target 
>> that only provided
>> virtio-mmio, the iotests should not fail when running "make check".
>> To avoid that we continue playing whack-a-mole here in the future, maybe it 
>> would be better to
>> restrict the iotests to the "main" targets only, e.g. modify check-block.sh 
>> so that the tests only
>> run with x86, aarch64, s390x and ppc64 ?
> 
> Right, that would certainly be the simplest solution.
> 

The problem with that is we can't run the tests if target-list doesn't contain 
one of these targets.

Thanks,
Laurent

Re: [PULL 5/5] m68k: add Virtual M68k Machine

[Laurent Vivier]

Le 19/03/2021 à 10:29, Paolo Bonzini a écrit :
> On 19/03/21 10:20, Max Reitz wrote:
>> On 19.03.21 07:32, Thomas Huth wrote:
>>> On 18/03/2021 18.28, Max Reitz wrote:
>>> [...]
  From that it follows that I don’t see much use in testing specific 
 devices either.  Say there’s
 a platform that provides both virtio-pci and virtio-mmio, the default (say 
 virtio-pci) is fine
 for the iotests. I see little value in testing virtio-mmio as well.  
 (Perhaps I’m short-sighted,
 though.)
>>>
>>> That's a fair point. But still, if someone compiled QEMU only with a target 
>>> that only provided
>>> virtio-mmio, the iotests should not fail when running "make check".
>>> To avoid that we continue playing whack-a-mole here in the future, maybe it 
>>> would be better to
>>> restrict the iotests to the "main" targets only, e.g. modify check-block.sh 
>>> so that the tests
>>> only run with x86, aarch64, s390x and ppc64 ?
>>
>> Right, that would certainly be the simplest solution.
> 
> It would also make the patches that Laurent sent this morning unnecessary, 
> and avoid the use of
> aliases in the tests (so that it's clear what is tested).
>

We don't test the virtio frontend, but the blockdev backend, so we don't care 
what we use here.

Aliases simplify the code...

Thanks,
Laurent

Re: [PULL 5/5] m68k: add Virtual M68k Machine

[Max Reitz]

On 19.03.21 11:50, Laurent Vivier wrote:

Le 19/03/2021 à 10:20, Max Reitz a écrit :

On 19.03.21 07:32, Thomas Huth wrote:

On 18/03/2021 18.28, Max Reitz wrote:
[...]

  From that it follows that I don’t see much use in testing specific devices 
either.  Say there’s
a platform that provides both virtio-pci and virtio-mmio, the default (say 
virtio-pci) is fine
for the iotests. I see little value in testing virtio-mmio as well.  (Perhaps 
I’m short-sighted,
though.)


That's a fair point. But still, if someone compiled QEMU only with a target 
that only provided
virtio-mmio, the iotests should not fail when running "make check".
To avoid that we continue playing whack-a-mole here in the future, maybe it 
would be better to
restrict the iotests to the "main" targets only, e.g. modify check-block.sh so 
that the tests only
run with x86, aarch64, s390x and ppc64 ?


Right, that would certainly be the simplest solution.



The problem with that is we can't run the tests if target-list doesn't contain 
one of these targets.


Yes, but is that really a problem?

Max

Re: [PATCH] target/arm: Make number of counters in PMCR follow the CPU

[Marcin Juszkiewicz]

W dniu 11.03.2021 o 17:59, Peter Maydell pisze:

Currently we give all the v7-and-up CPUs a PMU with 4 counters.  This
means that we don't provide the 6 counters that are required by the
Arm BSA (Base System Architecture) specification if the CPU supports
the Virtualization extensions.

Instead of having a single PMCR_NUM_COUNTERS, make each CPU type
specify the PMCR reset value (obtained from the appropriate TRM), and
use the 'N' field of that value to define the number of counters
provided.

This means that we now supply 6 counters for Cortex-A53, A57, A72,
A15 and A9 as well as '-cpu max'; Cortex-A7 and A8 stay at 4; and
Cortex-R5 goes down to 3.

Note that because we now use the PMCR reset value of the specific
implementation, we no longer set the LC bit out of reset.  This has
an UNKNOWN value out of reset for all cores with any AArch32 support,
so guest software should be setting it anyway if it wants it.

Signed-off-by: Peter Maydell 


I checked on A57/A72/max with sbsa-ref machine. Each of them got 7 PMU 
counters reported by both SBSA ACS and Linux 5.3 kernel.


Tested-by: Marcin Juszkiewicz 


---
This is pretty much untested (I just checked Linux still boots;
haven't tried it with KVM either). It's an alternative to
just bumping PMCR_NUM_COUNTERS to 6.
---
  target/arm/cpu.h |  1 +
  target/arm/cpu64.c   |  3 +++
  target/arm/cpu_tcg.c |  5 +
  target/arm/helper.c  | 29 +
  target/arm/kvm64.c   |  2 ++
  5 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 193a49ec7fa..fe68f464b3a 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -942,6 +942,7 @@ struct ARMCPU {
  uint64_t id_aa64mmfr2;
  uint64_t id_aa64dfr0;
  uint64_t id_aa64dfr1;
+uint64_t reset_pmcr_el0;
  } isar;
  uint64_t midr;
  uint32_t revidr;
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index f0a9e968c9c..5d9d56a33c3 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -141,6 +141,7 @@ static void aarch64_a57_initfn(Object *obj)
  cpu->gic_num_lrs = 4;
  cpu->gic_vpribits = 5;
  cpu->gic_vprebits = 5;
+cpu->isar.reset_pmcr_el0 = 0x41013000;
  define_arm_cp_regs(cpu, cortex_a72_a57_a53_cp_reginfo);
  }
  
@@ -194,6 +195,7 @@ static void aarch64_a53_initfn(Object *obj)

  cpu->gic_num_lrs = 4;
  cpu->gic_vpribits = 5;
  cpu->gic_vprebits = 5;
+cpu->isar.reset_pmcr_el0 = 0x41033000;
  define_arm_cp_regs(cpu, cortex_a72_a57_a53_cp_reginfo);
  }
  
@@ -245,6 +247,7 @@ static void aarch64_a72_initfn(Object *obj)

  cpu->gic_num_lrs = 4;
  cpu->gic_vpribits = 5;
  cpu->gic_vprebits = 5;
+cpu->isar.reset_pmcr_el0 = 0x41023000;
  define_arm_cp_regs(cpu, cortex_a72_a57_a53_cp_reginfo);
  }
  
diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c

index 046e476f65f..8252fd29f90 100644
--- a/target/arm/cpu_tcg.c
+++ b/target/arm/cpu_tcg.c
@@ -301,6 +301,7 @@ static void cortex_a8_initfn(Object *obj)
  cpu->ccsidr[1] = 0x2007e01a; /* 16k L1 icache. */
  cpu->ccsidr[2] = 0xf000; /* No L2 icache. */
  cpu->reset_auxcr = 2;
+cpu->isar.reset_pmcr_el0 = 0x41002000;
  define_arm_cp_regs(cpu, cortexa8_cp_reginfo);
  }
  
@@ -373,6 +374,7 @@ static void cortex_a9_initfn(Object *obj)

  cpu->clidr = (1 << 27) | (1 << 24) | 3;
  cpu->ccsidr[0] = 0xe00fe019; /* 16k L1 dcache. */
  cpu->ccsidr[1] = 0x200fe019; /* 16k L1 icache. */
+cpu->isar.reset_pmcr_el0 = 0x41093000;
  define_arm_cp_regs(cpu, cortexa9_cp_reginfo);
  }
  
@@ -443,6 +445,7 @@ static void cortex_a7_initfn(Object *obj)

  cpu->ccsidr[0] = 0x701fe00a; /* 32K L1 dcache */
  cpu->ccsidr[1] = 0x201fe00a; /* 32K L1 icache */
  cpu->ccsidr[2] = 0x711fe07a; /* 4096K L2 unified cache */
+cpu->isar.reset_pmcr_el0 = 0x41072000;
  define_arm_cp_regs(cpu, cortexa15_cp_reginfo); /* Same as A15 */
  }
  
@@ -485,6 +488,7 @@ static void cortex_a15_initfn(Object *obj)

  cpu->ccsidr[0] = 0x701fe00a; /* 32K L1 dcache */
  cpu->ccsidr[1] = 0x201fe00a; /* 32K L1 icache */
  cpu->ccsidr[2] = 0x711fe07a; /* 4096K L2 unified cache */
+cpu->isar.reset_pmcr_el0 = 0x410F3000;
  define_arm_cp_regs(cpu, cortexa15_cp_reginfo);
  }
  
@@ -717,6 +721,7 @@ static void cortex_r5_initfn(Object *obj)

  cpu->isar.id_isar6 = 0x0;
  cpu->mp_is_up = true;
  cpu->pmsav7_dregion = 16;
+cpu->isar.reset_pmcr_el0 = 0x41151800;
  define_arm_cp_regs(cpu, cortexr5_cp_reginfo);
  }
  
diff --git a/target/arm/helper.c b/target/arm/helper.c

index 904b0927cd2..2f3867cad79 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -38,7 +38,6 @@
  #endif
  
  #define ARM_CPU_FREQ 10 /* FIXME: 1 GHz, should be configurable */

-#define PMCR_NUM_COUNTERS 4 /* QEMU IMPDEF choice */
  
  #ifndef CONFIG_USER_ONLY
  
@@ -1149,7 +1148,9 @@ static const ARMCPRegInfo v6_cp_reginfo[] = {
  
  static inline uint32_t pmu_nu

Re: [PULL 5/5] m68k: add Virtual M68k Machine

[Thomas Huth]

On 19/03/2021 11.50, Laurent Vivier wrote:

Le 19/03/2021 à 10:20, Max Reitz a écrit :

On 19.03.21 07:32, Thomas Huth wrote:

On 18/03/2021 18.28, Max Reitz wrote:
[...]

  From that it follows that I don’t see much use in testing specific devices 
either.  Say there’s
a platform that provides both virtio-pci and virtio-mmio, the default (say 
virtio-pci) is fine
for the iotests. I see little value in testing virtio-mmio as well.  (Perhaps 
I’m short-sighted,
though.)


That's a fair point. But still, if someone compiled QEMU only with a target 
that only provided
virtio-mmio, the iotests should not fail when running "make check".
To avoid that we continue playing whack-a-mole here in the future, maybe it 
would be better to
restrict the iotests to the "main" targets only, e.g. modify check-block.sh so 
that the tests only
run with x86, aarch64, s390x and ppc64 ?


Right, that would certainly be the simplest solution.



The problem with that is we can't run the tests if target-list doesn't contain 
one of these targets.


The iotests are skipped in quite a bunch of cases already anyway, e.g. when 
GNU sed or bash are not available in the right version. So I think it would 
be also ok to skip them in builds without one of the major architectures.
Anyway, since your patches are already ready, I think we also could simply 
go with those this time, and reconsider tweaking tests/check-block.sh the 
next time we run into this issue.


 Thomas

gitlab-ci: cross-i386-system job timeouting

[Philippe Mathieu-Daudé]

Hi,

With all the recent pull requests merged, I'm now seeing
the cross-i386-system reaching the timeout limit:

[5225/5228] Linking target tests/qtest/tpm-tis-device-swtpm-test
[5226/5228] Linking target tests/unit/test-util-sockets
[5227/5228] Compiling C object
tests/unit/test-timed-average.p/test-timed-average.c.o
[5228/5228] Linking target tests/unit/test-timed-average
make: Nothing to be done for 'check-build'.
Running test qtest-aarch64/qom-test
Running test qtest-alpha/qom-test
Running test qtest-avr/qom-test
Running test qtest-hppa/qom-test
Running test qtest-m68k/qom-test
ERROR: Job failed: execution took longer than 1h20m0s seconds

Maybe it was borderline and the addition of a new machine with
new types was too much.

What to do now? Split the job yet again?

Thanks,

Phil.

Re: [PULL 5/5] m68k: add Virtual M68k Machine

[Max Reitz]

On 19.03.21 11:51, Max Reitz wrote:

On 19.03.21 11:50, Laurent Vivier wrote:

Le 19/03/2021 à 10:20, Max Reitz a écrit :

On 19.03.21 07:32, Thomas Huth wrote:

On 18/03/2021 18.28, Max Reitz wrote:
[...]
  From that it follows that I don’t see much use in testing 
specific devices either.  Say there’s
a platform that provides both virtio-pci and virtio-mmio, the 
default (say virtio-pci) is fine
for the iotests. I see little value in testing virtio-mmio as 
well.  (Perhaps I’m short-sighted,

though.)


That's a fair point. But still, if someone compiled QEMU only with a 
target that only provided

virtio-mmio, the iotests should not fail when running "make check".
To avoid that we continue playing whack-a-mole here in the future, 
maybe it would be better to
restrict the iotests to the "main" targets only, e.g. modify 
check-block.sh so that the tests only

run with x86, aarch64, s390x and ppc64 ?


Right, that would certainly be the simplest solution.



The problem with that is we can't run the tests if target-list doesn't 
contain one of these targets.


Yes, but is that really a problem?


I should add: The thing is, I wouldn’t really call it a problem.  But 
still, as I said before somewhere in this thread, in theory we want to 
allow running the tests with every configuration.  It’s just that it’s a 
tradeoff between how much it helps and how much work it is to make them 
work.  (I gave s390 as an example, where effort was undertaken to make 
the iotests work.)


You’ve sent patches, so it seems you’re willing to invest the work. 
Sounds good to me, as long as we know it won’t rot.


Max

Re: Serious doubts about Gitlab CI

[Paolo Bonzini]

On 19/03/21 11:18, Andrew Jones wrote:

Yikes, that is 41 hours per CI run. I wonder if GitLab's CI minutes are
on slow machines or if we'll hit the same issue with dedicated runners.
It seems like CI optimization will be necessary...


We need to reduce the amount of CI we do, not only because we can't afford
it, but because it's wasteful. I hate to think of all the kWhs spent
testing the exact same code in the exact same way, since everyone runs
everything with a simple 'git push'.


Yes, I thought the same.


IMHO, 'git push' shouldn't trigger
anything. Starting CI should be an explicit step.


It is possible to do that on a project that uses merge requests, for 
example like this:


workflow:
  rules:
- if: '$CI_PIPELINE_SOURCE == "merge_request_event"'
- if: '$CI_COMMIT_BRANCH
  when: never

For us it's a bit more complicated (no merge requests).

Another common feature is failing the pipeline immediately if one of the 
jobs fail, but GitLab does not support it 
(https://gitlab.com/gitlab-org/gitlab/-/issues/23605).



Also, the default CI
should only trigger tests associated with the code changed. One should
have to explicitly trigger a complete CI when they deem it worthwhile.


This is interesting.  We could add a stage that looks for changed files 
using "git diff" and sets some variables (e.g. softmmu, user, TCG, 
various targets) based on the results.  Then you use those to skip some 
jobs or some tests, for example skipping check-tcg.  See 
https://docs.gitlab.com/ee/ci/variables/#inherit-cicd-variables for more 
information.


Paolo

Re: [PATCH 3/3] i386: Make sure kvm_arch_set_tsc_khz() succeeds on migration when 'hv-reenlightenment' was exposed

[Paolo Bonzini]

On 19/03/21 10:41, Vitaly Kuznetsov wrote:

What I want to achieve is to forbid migration of VMs with
reenlightenment, if they don't also specify tsc-khz to the frequency of
the TSC on the source host.  We can't check it at the beginning of
migration, but at least we can check it at the end.

Maybe we're talking about two different things?

No, your suggestion basically extends mine and I'm just trying to
understand the benefit. With my suggestion, it is not required to
specify tsc-khz on the source, we just take 'native' tsc frequency as a
reference. Post-migration, we require that KVM_SET_TSC_KHZ succeeds (and
not just 'try' like kvm_arch_put_registers() does so we effectively
break migration when we are unable to set the desired TSC frequency
(also at the end).


Oh, okay, I understand the confusion; I was thinking of checking for 
user_tsc_khz in the post-load function for reenlightenment, not in the 
command line processing.


Paolo

Re: [PATCH 4/4] iotests: iothreads need ioeventfd

[Paolo Bonzini]

On 18/03/21 23:39, Laurent Vivier wrote:

And ioeventfd are only available with virtio-scsi-pci, so don't use the alias
and add a rule to require virtio-scsi-pci for the tests that use iothreads.

Signed-off-by: Laurent Vivier 
---
  tests/qemu-iotests/127| 4 ++--
  tests/qemu-iotests/256| 2 ++
  tests/qemu-iotests/iotests.py | 5 +
  3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/127 b/tests/qemu-iotests/127
index 98e8e82a8210..a3693533685a 100755
--- a/tests/qemu-iotests/127
+++ b/tests/qemu-iotests/127
@@ -44,7 +44,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
  _supported_fmt qcow2
  _supported_proto file fuse
  
-_require_devices virtio-scsi scsi-hd

+_require_devices virtio-scsi-pci scsi-hd


Maybe

_require_devices scsi-hd
_require_devices virtio-scsi-pci || _require_devices virtio-scsi ccw

?

Paolo

Re: [PATCH 3/3] migration: Pre-fault memory before starting background snasphot

[Andrey Gruzdev]

On 19.03.2021 12:28, David Hildenbrand wrote:

+/*
+ * ram_block_populate_pages: populate memory in the RAM block by 
reading

+ *   an integer from the beginning of each page.
+ *
+ * Since it's solely used for userfault_fd WP feature, here we just
+ *   hardcode page size to TARGET_PAGE_SIZE.
+ *
+ * @bs: RAM block to populate
+ */
+volatile int ram_block_populate_pages__tmp;
+static void ram_block_populate_pages(RAMBlock *bs)
+{
+    ram_addr_t offset = 0;
+    int tmp = 0;
+
+    for (char *ptr = (char *) bs->host; offset < bs->used_length;
+    ptr += TARGET_PAGE_SIZE, offset += TARGET_PAGE_SIZE) {


You'll want qemu_real_host_page_size instead of TARGET_PAGE_SIZE


Ok.

+    /* Try to do it without memory writes */
+    tmp += *(volatile int *) ptr;
+    }



The following is slightly simpler and doesn't rely on volatile 
semantics [1].

Should work on any arch I guess.

static void ram_block_populate_pages(RAMBlock *bs)
{
    char *ptr = (char *) bs->host;
    ram_addr_t offset;

    for (offset = 0; offset < bs->used_length;
 offset += qemu_real_host_page_size) {
char tmp = *(volatile char *)(ptr + offset)

/* Don't optimize the read out. */
asm volatile ("" : "+r" (tmp));
}


Thanks, good option, I'll change the code.


Compiles to

    for (offset = 0; offset < bs->used_length;
    316d:   48 8b 4b 30 mov    0x30(%rbx),%rcx
    char *ptr = (char *) bs->host;
    3171:   48 8b 73 18 mov    0x18(%rbx),%rsi
    for (offset = 0; offset < bs->used_length;
    3175:   48 85 c9    test   %rcx,%rcx
    3178:   74 ce   je 3148 


 offset += qemu_real_host_page_size) {
    317a:   48 8b 05 00 00 00 00    mov 0x0(%rip),%rax    # 
3181 

    3181:   48 8b 38    mov    (%rax),%rdi
    3184:   0f 1f 40 00 nopl   0x0(%rax)
    char tmp = *(volatile char *)(ptr + offset);
    3188:   48 8d 04 16 lea    (%rsi,%rdx,1),%rax
    318c:   0f b6 00    movzbl (%rax),%eax
 offset += qemu_real_host_page_size) {
    318f:   48 01 fa    add    %rdi,%rdx
    for (offset = 0; offset < bs->used_length;
    3192:   48 39 ca    cmp    %rcx,%rdx
    3195:   72 f1   jb 3188 




[1] 
https://programfan.github.io/blog/2015/04/27/prevent-gcc-optimize-away-code/



I'll send patches soon to take care of virtio-mem via RamDiscardManager -
to skip populating the parts that are supposed to remain discarded and 
not migrated.

Unfortunately, the RamDiscardManager patches are still stuck waiting for
acks ... and now we're in soft-freeze.


RamDiscardManager patches - do they also modify migration code?
I mean which part is responsible of not migrating discarded ranges.

--
Andrey Gruzdev, Principal Engineer
Virtuozzo GmbH  +7-903-247-6397
virtuzzo.com

Re: [PULL 5/5] m68k: add Virtual M68k Machine

[Paolo Bonzini]

On 19/03/21 11:51, Laurent Vivier wrote:

It would also make the patches that Laurent sent this morning unnecessary, and 
avoid the use of
aliases in the tests (so that it's clear what is tested).


We don't test the virtio frontend, but the blockdev backend, so we don't care 
what we use here.


Sort of, see for example the iothreads which, with your patch, would not 
be covered anymore on s390.



Aliases simplify the code...


Aliases are not deprecated but... let's say despised, because of the 
special casing they add.  But yes, the code simplification from your 
patch is hard to brush away.


So I agree, since you have already mostly written the patches let's just 
complete them.


Paolo

Re: [PATCH 3/3] migration: Pre-fault memory before starting background snasphot

[Andrey Gruzdev]

On 19.03.2021 12:32, David Hildenbrand wrote:

On 19.03.21 10:28, David Hildenbrand wrote:

+/*
+ * ram_block_populate_pages: populate memory in the RAM block by 
reading

+ *   an integer from the beginning of each page.
+ *
+ * Since it's solely used for userfault_fd WP feature, here we just
+ *   hardcode page size to TARGET_PAGE_SIZE.
+ *
+ * @bs: RAM block to populate
+ */
+volatile int ram_block_populate_pages__tmp;
+static void ram_block_populate_pages(RAMBlock *bs)
+{
+    ram_addr_t offset = 0;
+    int tmp = 0;
+
+    for (char *ptr = (char *) bs->host; offset < bs->used_length;
+    ptr += TARGET_PAGE_SIZE, offset += TARGET_PAGE_SIZE) {


You'll want qemu_real_host_page_size instead of TARGET_PAGE_SIZE


+    /* Try to do it without memory writes */
+    tmp += *(volatile int *) ptr;
+    }



The following is slightly simpler and doesn't rely on volatile 
semantics [1].

Should work on any arch I guess.

static void ram_block_populate_pages(RAMBlock *bs)
{
  char *ptr = (char *) bs->host;
  ram_addr_t offset;

  for (offset = 0; offset < bs->used_length;
   offset += qemu_real_host_page_size) {
char tmp = *(volatile char *)(ptr + offset)


I wanted to do a "= *(ptr + offset)" here.


Yep


/* Don't optimize the read out. */
asm volatile ("" : "+r" (tmp));


So this is the only volatile thing that the compiler must guarantee to 
not optimize away.





--
Andrey Gruzdev, Principal Engineer
Virtuozzo GmbH  +7-903-247-6397
virtuzzo.com

Re: [PATCH v2] docs/devel/testing.rst: Fix references to unit tests

[Thomas Huth]

On 18/03/2021 18.44, Wainer dos Santos Moschetta wrote:

With the recent move of the unit tests to tests/unit directory some
instructions under the "Unit tests" section became imprecise, which
are fixed by this change.

Fixes: da668aa15b99 ("tests: Move unit tests into a separate directory")
Signed-off-by: Wainer dos Santos Moschetta 
Reviewed-by: Thomas Huth 
---


Thanks, queued to my testing-next branch:

 https://gitlab.com/thuth/qemu/-/commits/testing-next

 Thomas

Re: gitlab-ci: cross-i386-system job timeouting

[Thomas Huth]

On 19/03/2021 11.56, Philippe Mathieu-Daudé wrote:

Hi,

With all the recent pull requests merged, I'm now seeing
the cross-i386-system reaching the timeout limit:


I just took 53 minutes in the main repo:

https://gitlab.com/qemu-project/qemu/-/jobs/677521

I assume you were just temporarily out of luck ... but let's keep an eye on 
it...


 Thomas

[PATCH 03/15] virtio-gpu: add virtio-gpu-gl-pci

[Gerd Hoffmann]

Add pci proxy for virtio-gpu-gl-device.

Signed-off-by: Gerd Hoffmann 
---
 hw/display/virtio-gpu-pci.c | 27 +++
 util/module.c   |  1 +
 2 files changed, 28 insertions(+)

diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c
index d742a30aecf7..f5c685862ef7 100644
--- a/hw/display/virtio-gpu-pci.c
+++ b/hw/display/virtio-gpu-pci.c
@@ -91,10 +91,37 @@ static const VirtioPCIDeviceTypeInfo virtio_gpu_pci_info = {
 .instance_init = virtio_gpu_initfn,
 };
 
+#define TYPE_VIRTIO_GPU_GL_PCI "virtio-gpu-gl-pci"
+typedef struct VirtIOGPUGLPCI VirtIOGPUGLPCI;
+DECLARE_INSTANCE_CHECKER(VirtIOGPUGLPCI, VIRTIO_GPU_GL_PCI,
+ TYPE_VIRTIO_GPU_GL_PCI)
+
+struct VirtIOGPUGLPCI {
+VirtIOGPUPCIBase parent_obj;
+VirtIOGPUGL vdev;
+};
+
+static void virtio_gpu_gl_initfn(Object *obj)
+{
+VirtIOGPUGLPCI *dev = VIRTIO_GPU_GL_PCI(obj);
+
+virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+TYPE_VIRTIO_GPU_GL);
+VIRTIO_GPU_PCI_BASE(obj)->vgpu = VIRTIO_GPU_BASE(&dev->vdev);
+}
+
+static const VirtioPCIDeviceTypeInfo virtio_gpu_gl_pci_info = {
+.generic_name = TYPE_VIRTIO_GPU_GL_PCI,
+.parent = TYPE_VIRTIO_GPU_PCI_BASE,
+.instance_size = sizeof(VirtIOGPUGLPCI),
+.instance_init = virtio_gpu_gl_initfn,
+};
+
 static void virtio_gpu_pci_register_types(void)
 {
 type_register_static(&virtio_gpu_pci_base_info);
 virtio_pci_types_register(&virtio_gpu_pci_info);
+virtio_pci_types_register(&virtio_gpu_gl_pci_info);
 }
 
 type_init(virtio_gpu_pci_register_types)
diff --git a/util/module.c b/util/module.c
index fb80b85fe606..f825b071baa2 100644
--- a/util/module.c
+++ b/util/module.c
@@ -304,6 +304,7 @@ static struct {
 { "vhost-user-gpu","hw-", "display-virtio-gpu"},
 { "virtio-gpu-pci-base",   "hw-", "display-virtio-gpu-pci" },
 { "virtio-gpu-pci","hw-", "display-virtio-gpu-pci" },
+{ "virtio-gpu-gl-pci", "hw-", "display-virtio-gpu-pci" },
 { "vhost-user-gpu-pci","hw-", "display-virtio-gpu-pci" },
 { "virtio-vga-base",   "hw-", "display-virtio-vga"},
 { "virtio-vga","hw-", "display-virtio-vga"},
-- 
2.30.2

[PATCH 00/15] virtio-gpu: split into two devices.

[Gerd Hoffmann]

Currently we have one virtio-gpu device.  Problem with this approach is
that if you compile a full-featured qemu you'll get a virtio-gpu device
which depends on opengl and virgl, so these dependencies must be
installed and the libraries will be loaded into memory even if you don't
use virgl.  Also the code is cluttered with #ifdefs and a bit messy.

This patch series splits the virtio-gpu device into two:

 (1) virtio-gpu-device becomes the non-virgl device, same as
 virtio-gpu-device,virgl=off today.
 (2) virtio-gpu-gl-device is the new virgl device, same as
 virtio-gpu-device,virgl=on today.

When compiling qemu without virglrenderer support virtio-gpu-device
behavior doesn't change.

Gerd Hoffmann (15):
  virtio-gpu: rename virgl source file.
  virtio-gpu: add virtio-gpu-gl-device
  virtio-gpu: add virtio-gpu-gl-pci
  virtio-gpu: add virtio-vga-gl
  virtio-gpu: move virgl realize + properties
  virtio-gpu: move virgl reset
  virtio-gpu: use class function for ctrl queue handlers
  virtio-gpu: move virgl handle_ctrl
  virtio-gpu: move virgl gl_flushed
  virtio-gpu: move virgl process_cmd
  virtio-gpu: move update_cursor_data
  virtio-gpu: drop VIRGL() macro
  virtio-gpu: move virtio-gpu-gl-device to separate module
  virtio-gpu: drop use_virgl_renderer
  virtio-gpu: move fields to struct VirtIOGPUGL

 include/hw/virtio/virtio-gpu.h|  31 +++-
 hw/display/virtio-gpu-base.c  |   6 +-
 hw/display/virtio-gpu-gl.c| 163 ++
 hw/display/virtio-gpu-pci.c   |  27 +++
 .../{virtio-gpu-3d.c => virtio-gpu-virgl.c}   |   0
 hw/display/virtio-gpu.c   | 142 +++
 hw/display/virtio-vga.c   |  30 
 util/module.c |   5 +
 hw/display/meson.build|  11 +-
 9 files changed, 281 insertions(+), 134 deletions(-)
 create mode 100644 hw/display/virtio-gpu-gl.c
 rename hw/display/{virtio-gpu-3d.c => virtio-gpu-virgl.c} (100%)

-- 
2.30.2

[PATCH 04/15] virtio-gpu: add virtio-vga-gl

[Gerd Hoffmann]

Add pci proxy for virtio-gpu-gl-device, with vga compatibility.

Signed-off-by: Gerd Hoffmann 
---
 hw/display/virtio-vga.c | 30 ++
 util/module.c   |  1 +
 2 files changed, 31 insertions(+)

diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c
index d3c640406152..f45ebe97d3ed 100644
--- a/hw/display/virtio-vga.c
+++ b/hw/display/virtio-vga.c
@@ -269,10 +269,40 @@ static VirtioPCIDeviceTypeInfo virtio_vga_info = {
 .instance_init = virtio_vga_inst_initfn,
 };
 
+#define TYPE_VIRTIO_VGA_GL "virtio-vga-gl"
+
+typedef struct VirtIOVGAGL VirtIOVGAGL;
+DECLARE_INSTANCE_CHECKER(VirtIOVGAGL, VIRTIO_VGA_GL,
+ TYPE_VIRTIO_VGA_GL)
+
+struct VirtIOVGAGL {
+VirtIOVGABase parent_obj;
+
+VirtIOGPUGL   vdev;
+};
+
+static void virtio_vga_gl_inst_initfn(Object *obj)
+{
+VirtIOVGAGL *dev = VIRTIO_VGA_GL(obj);
+
+virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+TYPE_VIRTIO_GPU_GL);
+VIRTIO_VGA_BASE(dev)->vgpu = VIRTIO_GPU_BASE(&dev->vdev);
+}
+
+
+static VirtioPCIDeviceTypeInfo virtio_vga_gl_info = {
+.generic_name  = TYPE_VIRTIO_VGA_GL,
+.parent= TYPE_VIRTIO_VGA_BASE,
+.instance_size = sizeof(VirtIOVGAGL),
+.instance_init = virtio_vga_gl_inst_initfn,
+};
+
 static void virtio_vga_register_types(void)
 {
 type_register_static(&virtio_vga_base_info);
 virtio_pci_types_register(&virtio_vga_info);
+virtio_pci_types_register(&virtio_vga_gl_info);
 }
 
 type_init(virtio_vga_register_types)
diff --git a/util/module.c b/util/module.c
index f825b071baa2..0bbe2b25fbec 100644
--- a/util/module.c
+++ b/util/module.c
@@ -308,6 +308,7 @@ static struct {
 { "vhost-user-gpu-pci","hw-", "display-virtio-gpu-pci" },
 { "virtio-vga-base",   "hw-", "display-virtio-vga"},
 { "virtio-vga","hw-", "display-virtio-vga"},
+{ "virtio-vga-gl", "hw-", "display-virtio-vga"},
 { "vhost-user-vga","hw-", "display-virtio-vga"},
 { "chardev-braille",   "chardev-", "baum" },
 { "chardev-spicevmc",  "chardev-", "spice"},
-- 
2.30.2

[PATCH 10/15] virtio-gpu: move virgl process_cmd

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 include/hw/virtio/virtio-gpu.h |  2 ++
 hw/display/virtio-gpu-gl.c | 11 +++
 hw/display/virtio-gpu.c|  9 +
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 380aa7dd6322..4ce39d2abb4c 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -170,6 +170,7 @@ struct VirtIOGPUClass {
 VirtIOGPUBaseClass parent;
 
 void (*handle_ctrl)(VirtIODevice *vdev, VirtQueue *vq);
+void (*process_cmd)(VirtIOGPU *g, struct virtio_gpu_ctrl_command *cmd);
 };
 
 struct VirtIOGPUGL {
@@ -228,6 +229,7 @@ void virtio_gpu_cleanup_mapping_iov(VirtIOGPU *g,
 void virtio_gpu_process_cmdq(VirtIOGPU *g);
 void virtio_gpu_device_realize(DeviceState *qdev, Error **errp);
 void virtio_gpu_reset(VirtIODevice *vdev);
+void virtio_gpu_simple_process_cmd(VirtIOGPU *g, struct 
virtio_gpu_ctrl_command *cmd);
 
 /* virtio-gpu-3d.c */
 void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index e976fb8d04c4..792cc0b41256 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -23,6 +23,16 @@
 #include "hw/virtio/virtio-gpu-pixman.h"
 #include "hw/qdev-properties.h"
 
+static void virtio_gpu_gl_process_cmd(VirtIOGPU *g,
+  struct virtio_gpu_ctrl_command *cmd)
+{
+if (g->parent_obj.use_virgl_renderer) {
+virtio_gpu_virgl_process_cmd(g, cmd);
+return;
+}
+virtio_gpu_simple_process_cmd(g, cmd);
+}
+
 static void virtio_gpu_gl_flushed(VirtIOGPUBase *b)
 {
 VirtIOGPU *g = VIRTIO_GPU(b);
@@ -116,6 +126,7 @@ static void virtio_gpu_gl_class_init(ObjectClass *klass, 
void *data)
 
 vbc->gl_flushed = virtio_gpu_gl_flushed;
 vgc->handle_ctrl = virtio_gpu_gl_handle_ctrl;
+vgc->process_cmd = virtio_gpu_gl_process_cmd;
 
 vdc->realize = virtio_gpu_gl_device_realize;
 vdc->reset = virtio_gpu_gl_reset;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index ae80519c7356..e61bfffa8019 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -747,8 +747,8 @@ virtio_gpu_resource_detach_backing(VirtIOGPU *g,
 virtio_gpu_cleanup_mapping(g, res);
 }
 
-static void virtio_gpu_simple_process_cmd(VirtIOGPU *g,
-  struct virtio_gpu_ctrl_command *cmd)
+void virtio_gpu_simple_process_cmd(VirtIOGPU *g,
+   struct virtio_gpu_ctrl_command *cmd)
 {
 VIRTIO_GPU_FILL_CMD(cmd->cmd_hdr);
 virtio_gpu_ctrl_hdr_bswap(&cmd->cmd_hdr);
@@ -806,6 +806,7 @@ static void virtio_gpu_handle_cursor_cb(VirtIODevice *vdev, 
VirtQueue *vq)
 void virtio_gpu_process_cmdq(VirtIOGPU *g)
 {
 struct virtio_gpu_ctrl_command *cmd;
+VirtIOGPUClass *vgc = VIRTIO_GPU_GET_CLASS(g);
 
 if (g->processing_cmdq) {
 return;
@@ -819,8 +820,7 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g)
 }
 
 /* process command */
-VIRGL(g, virtio_gpu_virgl_process_cmd, virtio_gpu_simple_process_cmd,
-  g, cmd);
+vgc->process_cmd(g, cmd);
 
 QTAILQ_REMOVE(&g->cmdq, cmd, next);
 if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
@@ -1189,6 +1189,7 @@ static void virtio_gpu_class_init(ObjectClass *klass, 
void *data)
 VirtIOGPUClass *vgc = VIRTIO_GPU_CLASS(klass);
 
 vgc->handle_ctrl = virtio_gpu_handle_ctrl;
+vgc->process_cmd = virtio_gpu_simple_process_cmd;
 
 vdc->realize = virtio_gpu_device_realize;
 vdc->reset = virtio_gpu_reset;
-- 
2.30.2

[PATCH 01/15] virtio-gpu: rename virgl source file.

[Gerd Hoffmann]

"3d" -> "virgl" as 3d is a rather broad term.
Hopefully a bit less confusing.

Signed-off-by: Gerd Hoffmann 
---
 hw/display/{virtio-gpu-3d.c => virtio-gpu-virgl.c} | 0
 hw/display/meson.build | 2 +-
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename hw/display/{virtio-gpu-3d.c => virtio-gpu-virgl.c} (100%)

diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-virgl.c
similarity index 100%
rename from hw/display/virtio-gpu-3d.c
rename to hw/display/virtio-gpu-virgl.c
diff --git a/hw/display/meson.build b/hw/display/meson.build
index 9d79e3951d9e..8e465731b85b 100644
--- a/hw/display/meson.build
+++ b/hw/display/meson.build
@@ -58,7 +58,7 @@ if config_all_devices.has_key('CONFIG_VIRTIO_GPU')
   virtio_gpu_ss.add(when: 'CONFIG_VIRTIO_GPU',
 if_true: [files('virtio-gpu-base.c', 'virtio-gpu.c'), 
pixman, virgl])
   virtio_gpu_ss.add(when: ['CONFIG_VIRTIO_GPU', 'CONFIG_VIRGL'],
-if_true: [files('virtio-gpu-3d.c'), pixman, virgl])
+if_true: [files('virtio-gpu-virgl.c'), pixman, virgl])
   virtio_gpu_ss.add(when: 'CONFIG_VHOST_USER_GPU', if_true: 
files('vhost-user-gpu.c'))
   hw_display_modules += {'virtio-gpu': virtio_gpu_ss}
 endif
-- 
2.30.2

[PATCH 07/15] virtio-gpu: use class function for ctrl queue handlers

[Gerd Hoffmann]

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

diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index a7b7d78310ea..380aa7dd6322 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -29,7 +29,7 @@ OBJECT_DECLARE_TYPE(VirtIOGPUBase, VirtIOGPUBaseClass,
 VIRTIO_GPU_BASE)
 
 #define TYPE_VIRTIO_GPU "virtio-gpu-device"
-OBJECT_DECLARE_SIMPLE_TYPE(VirtIOGPU, VIRTIO_GPU)
+OBJECT_DECLARE_TYPE(VirtIOGPU, VirtIOGPUClass, VIRTIO_GPU)
 
 #define TYPE_VIRTIO_GPU_GL "virtio-gpu-gl-device"
 OBJECT_DECLARE_SIMPLE_TYPE(VirtIOGPUGL, VIRTIO_GPU_GL)
@@ -166,6 +166,12 @@ struct VirtIOGPU {
 } stats;
 };
 
+struct VirtIOGPUClass {
+VirtIOGPUBaseClass parent;
+
+void (*handle_ctrl)(VirtIODevice *vdev, VirtQueue *vq);
+};
+
 struct VirtIOGPUGL {
 struct VirtIOGPU parent_obj;
 };
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 68286f75a01a..39ef22b7c08d 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -893,7 +893,9 @@ static void virtio_gpu_handle_ctrl(VirtIODevice *vdev, 
VirtQueue *vq)
 static void virtio_gpu_ctrl_bh(void *opaque)
 {
 VirtIOGPU *g = opaque;
-virtio_gpu_handle_ctrl(&g->parent_obj.parent_obj, g->ctrl_vq);
+VirtIOGPUClass *vgc = VIRTIO_GPU_GET_CLASS(g);
+
+vgc->handle_ctrl(&g->parent_obj.parent_obj, g->ctrl_vq);
 }
 
 static void virtio_gpu_handle_cursor(VirtIODevice *vdev, VirtQueue *vq)
@@ -1210,9 +1212,12 @@ static void virtio_gpu_class_init(ObjectClass *klass, 
void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
 VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-VirtIOGPUBaseClass *vgc = VIRTIO_GPU_BASE_CLASS(klass);
+VirtIOGPUBaseClass *vbc = VIRTIO_GPU_BASE_CLASS(klass);
+VirtIOGPUClass *vgc = VIRTIO_GPU_CLASS(klass);
+
+vbc->gl_flushed = virtio_gpu_gl_flushed;
+vgc->handle_ctrl = virtio_gpu_handle_ctrl;
 
-vgc->gl_flushed = virtio_gpu_gl_flushed;
 vdc->realize = virtio_gpu_device_realize;
 vdc->reset = virtio_gpu_reset;
 vdc->get_config = virtio_gpu_get_config;
@@ -1226,6 +1231,7 @@ static const TypeInfo virtio_gpu_info = {
 .name = TYPE_VIRTIO_GPU,
 .parent = TYPE_VIRTIO_GPU_BASE,
 .instance_size = sizeof(VirtIOGPU),
+.class_size = sizeof(VirtIOGPUClass),
 .class_init = virtio_gpu_class_init,
 };
 
-- 
2.30.2

[PATCH 13/15] virtio-gpu: move virtio-gpu-gl-device to separate module

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 util/module.c  |  4 +++-
 hw/display/meson.build | 11 ---
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/util/module.c b/util/module.c
index 0bbe2b25fbec..98a7e751 100644
--- a/util/module.c
+++ b/util/module.c
@@ -182,6 +182,8 @@ static const struct {
 { "ui-spice-app",   "ui-spice-core" },
 { "ui-spice-app",   "chardev-spice" },
 
+{ "hw-display-virtio-gpu-gl", "hw-display-virtio-gpu" },
+
 #ifdef CONFIG_OPENGL
 { "ui-egl-headless", "ui-opengl"},
 { "ui-gtk",  "ui-opengl"},
@@ -300,7 +302,7 @@ static struct {
 { "qxl-vga",   "hw-", "display-qxl"   },
 { "qxl",   "hw-", "display-qxl"   },
 { "virtio-gpu-device", "hw-", "display-virtio-gpu"},
-{ "virtio-gpu-gl-device",  "hw-", "display-virtio-gpu"},
+{ "virtio-gpu-gl-device",  "hw-", "display-virtio-gpu-gl" },
 { "vhost-user-gpu","hw-", "display-virtio-gpu"},
 { "virtio-gpu-pci-base",   "hw-", "display-virtio-gpu-pci" },
 { "virtio-gpu-pci","hw-", "display-virtio-gpu-pci" },
diff --git a/hw/display/meson.build b/hw/display/meson.build
index 5161efa08a6e..7cd5a1129812 100644
--- a/hw/display/meson.build
+++ b/hw/display/meson.build
@@ -56,13 +56,18 @@ softmmu_ss.add(when: [pixman, 'CONFIG_ATI_VGA'], if_true: 
files('ati.c', 'ati_2d
 if config_all_devices.has_key('CONFIG_VIRTIO_GPU')
   virtio_gpu_ss = ss.source_set()
   virtio_gpu_ss.add(when: 'CONFIG_VIRTIO_GPU',
-if_true: [files('virtio-gpu-base.c', 'virtio-gpu.c'), 
pixman, virgl])
-  virtio_gpu_ss.add(when: ['CONFIG_VIRTIO_GPU', 'CONFIG_VIRGL'],
-if_true: [files('virtio-gpu-gl.c', 'virtio-gpu-virgl.c'), 
pixman, virgl])
+if_true: [files('virtio-gpu-base.c', 'virtio-gpu.c'), 
pixman])
   virtio_gpu_ss.add(when: 'CONFIG_VHOST_USER_GPU', if_true: 
files('vhost-user-gpu.c'))
   hw_display_modules += {'virtio-gpu': virtio_gpu_ss}
 endif
 
+if config_all_devices.has_key('CONFIG_VIRTIO_GPU')
+  virtio_gpu_gl_ss = ss.source_set()
+  virtio_gpu_gl_ss.add(when: ['CONFIG_VIRTIO_GPU', 'CONFIG_VIRGL', opengl],
+   if_true: [files('virtio-gpu-gl.c', 
'virtio-gpu-virgl.c'), pixman, virgl])
+  hw_display_modules += {'virtio-gpu-gl': virtio_gpu_gl_ss}
+endif
+
 if config_all_devices.has_key('CONFIG_VIRTIO_PCI')
   virtio_gpu_pci_ss = ss.source_set()
   virtio_gpu_pci_ss.add(when: ['CONFIG_VIRTIO_GPU', 'CONFIG_VIRTIO_PCI'],
-- 
2.30.2

[PATCH 02/15] virtio-gpu: add virtio-gpu-gl-device

[Gerd Hoffmann]

Just a skeleton for starters, following patches will add more code.

Signed-off-by: Gerd Hoffmann 
---
 include/hw/virtio/virtio-gpu.h |  7 ++
 hw/display/virtio-gpu-gl.c | 41 ++
 util/module.c  |  1 +
 hw/display/meson.build |  2 +-
 4 files changed, 50 insertions(+), 1 deletion(-)
 create mode 100644 hw/display/virtio-gpu-gl.c

diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index fae149235c58..8ee6b8fe0fb8 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -31,6 +31,9 @@ OBJECT_DECLARE_TYPE(VirtIOGPUBase, VirtIOGPUBaseClass,
 #define TYPE_VIRTIO_GPU "virtio-gpu-device"
 OBJECT_DECLARE_SIMPLE_TYPE(VirtIOGPU, VIRTIO_GPU)
 
+#define TYPE_VIRTIO_GPU_GL "virtio-gpu-gl-device"
+OBJECT_DECLARE_SIMPLE_TYPE(VirtIOGPUGL, VIRTIO_GPU_GL)
+
 #define TYPE_VHOST_USER_GPU "vhost-user-gpu"
 OBJECT_DECLARE_SIMPLE_TYPE(VhostUserGPU, VHOST_USER_GPU)
 
@@ -163,6 +166,10 @@ struct VirtIOGPU {
 } stats;
 };
 
+struct VirtIOGPUGL {
+struct VirtIOGPU parent_obj;
+};
+
 struct VhostUserGPU {
 VirtIOGPUBase parent_obj;
 
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
new file mode 100644
index ..a477cbe186d3
--- /dev/null
+++ b/hw/display/virtio-gpu-gl.c
@@ -0,0 +1,41 @@
+/*
+ * Virtio GPU Device
+ *
+ * Copyright Red Hat, Inc. 2013-2014
+ *
+ * Authors:
+ * Dave Airlie 
+ * Gerd Hoffmann 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/iov.h"
+#include "qemu/module.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
+#include "hw/virtio/virtio.h"
+#include "hw/virtio/virtio-gpu.h"
+#include "hw/virtio/virtio-gpu-bswap.h"
+#include "hw/virtio/virtio-gpu-pixman.h"
+#include "hw/qdev-properties.h"
+
+static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data)
+{
+}
+
+static const TypeInfo virtio_gpu_gl_info = {
+.name = TYPE_VIRTIO_GPU_GL,
+.parent = TYPE_VIRTIO_GPU,
+.instance_size = sizeof(VirtIOGPUGL),
+.class_init = virtio_gpu_gl_class_init,
+};
+
+static void virtio_register_types(void)
+{
+type_register_static(&virtio_gpu_gl_info);
+}
+
+type_init(virtio_register_types)
diff --git a/util/module.c b/util/module.c
index c65060c167df..fb80b85fe606 100644
--- a/util/module.c
+++ b/util/module.c
@@ -300,6 +300,7 @@ static struct {
 { "qxl-vga",   "hw-", "display-qxl"   },
 { "qxl",   "hw-", "display-qxl"   },
 { "virtio-gpu-device", "hw-", "display-virtio-gpu"},
+{ "virtio-gpu-gl-device",  "hw-", "display-virtio-gpu"},
 { "vhost-user-gpu","hw-", "display-virtio-gpu"},
 { "virtio-gpu-pci-base",   "hw-", "display-virtio-gpu-pci" },
 { "virtio-gpu-pci","hw-", "display-virtio-gpu-pci" },
diff --git a/hw/display/meson.build b/hw/display/meson.build
index 8e465731b85b..5161efa08a6e 100644
--- a/hw/display/meson.build
+++ b/hw/display/meson.build
@@ -58,7 +58,7 @@ if config_all_devices.has_key('CONFIG_VIRTIO_GPU')
   virtio_gpu_ss.add(when: 'CONFIG_VIRTIO_GPU',
 if_true: [files('virtio-gpu-base.c', 'virtio-gpu.c'), 
pixman, virgl])
   virtio_gpu_ss.add(when: ['CONFIG_VIRTIO_GPU', 'CONFIG_VIRGL'],
-if_true: [files('virtio-gpu-virgl.c'), pixman, virgl])
+if_true: [files('virtio-gpu-gl.c', 'virtio-gpu-virgl.c'), 
pixman, virgl])
   virtio_gpu_ss.add(when: 'CONFIG_VHOST_USER_GPU', if_true: 
files('vhost-user-gpu.c'))
   hw_display_modules += {'virtio-gpu': virtio_gpu_ss}
 endif
-- 
2.30.2

[PATCH 05/15] virtio-gpu: move virgl realize + properties

[Gerd Hoffmann]

Move device init (realize) and properties.

Drop the virgl property, the virtio-gpu-gl-device has virgl enabled no
matter what.  Just use virtio-gpu-device instead if you don't want
enable virgl and opengl.  This simplifies the logic and reduces the test
matrix.

Signed-off-by: Gerd Hoffmann 
---
 include/hw/virtio/virtio-gpu.h |  1 +
 hw/display/virtio-gpu-gl.c | 33 +
 hw/display/virtio-gpu.c| 23 +--
 3 files changed, 35 insertions(+), 22 deletions(-)

diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 8ee6b8fe0fb8..4c1a8faebec9 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -220,6 +220,7 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
 void virtio_gpu_cleanup_mapping_iov(VirtIOGPU *g,
 struct iovec *iov, uint32_t count);
 void virtio_gpu_process_cmdq(VirtIOGPU *g);
+void virtio_gpu_device_realize(DeviceState *qdev, Error **errp);
 
 /* virtio-gpu-3d.c */
 void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index a477cbe186d3..9b7b5f00d7e6 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -16,14 +16,47 @@
 #include "qemu/module.h"
 #include "qemu/error-report.h"
 #include "qapi/error.h"
+#include "sysemu/sysemu.h"
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/virtio-gpu.h"
 #include "hw/virtio/virtio-gpu-bswap.h"
 #include "hw/virtio/virtio-gpu-pixman.h"
 #include "hw/qdev-properties.h"
 
+static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp)
+{
+VirtIOGPU *g = VIRTIO_GPU(qdev);
+
+#if defined(HOST_WORDS_BIGENDIAN)
+error_setg(errp, "virgl is not supported on bigendian platforms");
+return;
+#endif
+
+if (!display_opengl) {
+error_setg(errp, "opengl is not available");
+return;
+}
+
+g->parent_obj.conf.flags |= (1 << VIRTIO_GPU_FLAG_VIRGL_ENABLED);
+VIRTIO_GPU_BASE(g)->virtio_config.num_capsets =
+virtio_gpu_virgl_get_num_capsets(g);
+
+virtio_gpu_device_realize(qdev, errp);
+}
+
+static Property virtio_gpu_gl_properties[] = {
+DEFINE_PROP_BIT("stats", VirtIOGPU, parent_obj.conf.flags,
+VIRTIO_GPU_FLAG_STATS_ENABLED, false),
+DEFINE_PROP_END_OF_LIST(),
+};
+
 static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data)
 {
+DeviceClass *dc = DEVICE_CLASS(klass);
+VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+
+vdc->realize = virtio_gpu_gl_device_realize;
+device_class_set_props(dc, virtio_gpu_gl_properties);
 }
 
 static const TypeInfo virtio_gpu_gl_info = {
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index c9f5e36fd076..2ee6ba756aba 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1105,25 +1105,10 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, 
size_t size,
 return 0;
 }
 
-static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
+void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
 {
 VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
 VirtIOGPU *g = VIRTIO_GPU(qdev);
-bool have_virgl;
-
-#if !defined(CONFIG_VIRGL) || defined(HOST_WORDS_BIGENDIAN)
-have_virgl = false;
-#else
-have_virgl = display_opengl;
-#endif
-if (!have_virgl) {
-g->parent_obj.conf.flags &= ~(1 << VIRTIO_GPU_FLAG_VIRGL_ENABLED);
-} else {
-#if defined(CONFIG_VIRGL)
-VIRTIO_GPU_BASE(g)->virtio_config.num_capsets =
-virtio_gpu_virgl_get_num_capsets(g);
-#endif
-}
 
 if (!virtio_gpu_base_device_realize(qdev,
 virtio_gpu_handle_ctrl_cb,
@@ -1235,12 +1220,6 @@ static Property virtio_gpu_properties[] = {
 VIRTIO_GPU_BASE_PROPERTIES(VirtIOGPU, parent_obj.conf),
 DEFINE_PROP_SIZE("max_hostmem", VirtIOGPU, conf_max_hostmem,
  256 * MiB),
-#ifdef CONFIG_VIRGL
-DEFINE_PROP_BIT("virgl", VirtIOGPU, parent_obj.conf.flags,
-VIRTIO_GPU_FLAG_VIRGL_ENABLED, true),
-DEFINE_PROP_BIT("stats", VirtIOGPU, parent_obj.conf.flags,
-VIRTIO_GPU_FLAG_STATS_ENABLED, false),
-#endif
 DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.30.2

Re: [PULL v4 0/6] QOM and fdc patches patches for 2021-03-16

[Peter Maydell]

On Fri, 19 Mar 2021 at 06:45, Markus Armbruster  wrote:
>
> The following changes since commit 8a40754bca14df63c6d2ffe473b68a270dc50679:
>
>   Merge remote-tracking branch 'remotes/nvme/tags/nvme-next-pull-request' 
> into staging (2021-03-18 19:55:37 +)
>
> are available in the Git repository at:
>
>   git://repo.or.cz/qemu/armbru.git tags/pull-qom-fdc-2021-03-16-v4
>
> for you to fetch changes up to 0c727a621a646504ccec2b08e55fd48030448466:
>
>   memory: Drop "qemu:" prefix from QOM memory region type names (2021-03-19 
> 06:55:49 +0100)
>
> 
> QOM and fdc patches patches for 2021-03-16
>
> 
> Markus Armbruster (6):
>   docs/system/deprecated: Fix note on fdc drive properties
>   fdc: Drop deprecated floppy configuration
>   fdc: Inline fdctrl_connect_drives() into fdctrl_realize_common()
>   blockdev: Drop deprecated bogus -drive interface type
>   hw: Replace anti-social QOM type names
>   memory: Drop "qemu:" prefix from QOM memory region type name

Looks like this breaks the sanitizer gitlab CI job:
 https://gitlab.com/qemu-project/qemu/-/jobs/697105

Testing ./build-oss-fuzz/DEST_DIR/qemu-fuzz-i386-target-generic-fuzz-sdhci-v3
...
==8556==WARNING: ASan doesn't fully support makecontext/swapcontext
functions and may produce false positives in some cases!
==8556==WARNING: ASan is ignoring requested __asan_handle_no_return:
stack type: default top: 0x7ffcb7805000; bottom 0x7f960e9fd000; size:
0x0066a8e08000 (440919949312)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
i386: -drive if=sd,index=0,file=null-co://,format=raw,id=mydrive:
machine type does not support if=sd,bus=0,unit=0

thanks
-- PMM

[PATCH 12/15] virtio-gpu: drop VIRGL() macro

[Gerd Hoffmann]

Drops last virgl/opengl dependency from virtio-gpu-device.

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

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 2c0065277ffd..34cf35127a3d 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -39,23 +39,6 @@ virtio_gpu_find_resource(VirtIOGPU *g, uint32_t resource_id);
 static void virtio_gpu_cleanup_mapping(VirtIOGPU *g,
struct virtio_gpu_simple_resource *res);
 
-#ifdef CONFIG_VIRGL
-#include 
-#define VIRGL(_g, _virgl, _simple, ...) \
-do {\
-if (_g->parent_obj.use_virgl_renderer) {\
-_virgl(__VA_ARGS__);\
-} else {\
-_simple(__VA_ARGS__);   \
-}   \
-} while (0)
-#else
-#define VIRGL(_g, _virgl, _simple, ...) \
-do {\
-_simple(__VA_ARGS__);   \
-} while (0)
-#endif
-
 void virtio_gpu_update_cursor_data(VirtIOGPU *g,
struct virtio_gpu_scanout *s,
uint32_t resource_id)
-- 
2.30.2

[PATCH 06/15] virtio-gpu: move virgl reset

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 include/hw/virtio/virtio-gpu.h |  1 +
 hw/display/virtio-gpu-gl.c | 17 +
 hw/display/virtio-gpu.c| 19 +--
 3 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 4c1a8faebec9..a7b7d78310ea 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -221,6 +221,7 @@ void virtio_gpu_cleanup_mapping_iov(VirtIOGPU *g,
 struct iovec *iov, uint32_t count);
 void virtio_gpu_process_cmdq(VirtIOGPU *g);
 void virtio_gpu_device_realize(DeviceState *qdev, Error **errp);
+void virtio_gpu_reset(VirtIODevice *vdev);
 
 /* virtio-gpu-3d.c */
 void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 9b7b5f00d7e6..c3e562f835f7 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -23,6 +23,22 @@
 #include "hw/virtio/virtio-gpu-pixman.h"
 #include "hw/qdev-properties.h"
 
+static void virtio_gpu_gl_reset(VirtIODevice *vdev)
+{
+VirtIOGPU *g = VIRTIO_GPU(vdev);
+
+virtio_gpu_reset(vdev);
+
+if (g->parent_obj.use_virgl_renderer) {
+if (g->parent_obj.renderer_blocked) {
+g->renderer_reset = true;
+} else {
+virtio_gpu_virgl_reset(g);
+}
+g->parent_obj.use_virgl_renderer = false;
+}
+}
+
 static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp)
 {
 VirtIOGPU *g = VIRTIO_GPU(qdev);
@@ -56,6 +72,7 @@ static void virtio_gpu_gl_class_init(ObjectClass *klass, void 
*data)
 VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
 
 vdc->realize = virtio_gpu_gl_device_realize;
+vdc->reset = virtio_gpu_gl_reset;
 device_class_set_props(dc, virtio_gpu_gl_properties);
 }
 
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 2ee6ba756aba..68286f75a01a 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1126,18 +1126,12 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error 
**errp)
 QTAILQ_INIT(&g->fenceq);
 }
 
-static void virtio_gpu_reset(VirtIODevice *vdev)
+void virtio_gpu_reset(VirtIODevice *vdev)
 {
 VirtIOGPU *g = VIRTIO_GPU(vdev);
 struct virtio_gpu_simple_resource *res, *tmp;
 struct virtio_gpu_ctrl_command *cmd;
 
-#ifdef CONFIG_VIRGL
-if (g->parent_obj.use_virgl_renderer) {
-virtio_gpu_virgl_reset(g);
-}
-#endif
-
 QTAILQ_FOREACH_SAFE(res, &g->reslist, next, tmp) {
 virtio_gpu_resource_destroy(g, res);
 }
@@ -1155,17 +1149,6 @@ static void virtio_gpu_reset(VirtIODevice *vdev)
 g_free(cmd);
 }
 
-#ifdef CONFIG_VIRGL
-if (g->parent_obj.use_virgl_renderer) {
-if (g->parent_obj.renderer_blocked) {
-g->renderer_reset = true;
-} else {
-virtio_gpu_virgl_reset(g);
-}
-g->parent_obj.use_virgl_renderer = false;
-}
-#endif
-
 virtio_gpu_base_reset(VIRTIO_GPU_BASE(vdev));
 }
 
-- 
2.30.2

Re: [PATCH 3/3] migration: Pre-fault memory before starting background snasphot

[David Hildenbrand]

On 19.03.21 12:05, Andrey Gruzdev wrote:

On 19.03.2021 12:28, David Hildenbrand wrote:

+/*
+ * ram_block_populate_pages: populate memory in the RAM block by
reading
+ *   an integer from the beginning of each page.
+ *
+ * Since it's solely used for userfault_fd WP feature, here we just
+ *   hardcode page size to TARGET_PAGE_SIZE.
+ *
+ * @bs: RAM block to populate
+ */
+volatile int ram_block_populate_pages__tmp;
+static void ram_block_populate_pages(RAMBlock *bs)
+{
+    ram_addr_t offset = 0;
+    int tmp = 0;
+
+    for (char *ptr = (char *) bs->host; offset < bs->used_length;
+    ptr += TARGET_PAGE_SIZE, offset += TARGET_PAGE_SIZE) {


You'll want qemu_real_host_page_size instead of TARGET_PAGE_SIZE


Ok.

+    /* Try to do it without memory writes */
+    tmp += *(volatile int *) ptr;
+    }



The following is slightly simpler and doesn't rely on volatile
semantics [1].
Should work on any arch I guess.

static void ram_block_populate_pages(RAMBlock *bs)
{
     char *ptr = (char *) bs->host;
     ram_addr_t offset;

     for (offset = 0; offset < bs->used_length;
  offset += qemu_real_host_page_size) {
 char tmp = *(volatile char *)(ptr + offset)

 /* Don't optimize the read out. */
 asm volatile ("" : "+r" (tmp));
}


Thanks, good option, I'll change the code.


Compiles to

     for (offset = 0; offset < bs->used_length;
     316d:   48 8b 4b 30 mov    0x30(%rbx),%rcx
     char *ptr = (char *) bs->host;
     3171:   48 8b 73 18 mov    0x18(%rbx),%rsi
     for (offset = 0; offset < bs->used_length;
     3175:   48 85 c9    test   %rcx,%rcx
     3178:   74 ce   je 3148

  offset += qemu_real_host_page_size) {
     317a:   48 8b 05 00 00 00 00    mov 0x0(%rip),%rax    #
3181 
     3181:   48 8b 38    mov    (%rax),%rdi
     3184:   0f 1f 40 00 nopl   0x0(%rax)
     char tmp = *(volatile char *)(ptr + offset);
     3188:   48 8d 04 16 lea    (%rsi,%rdx,1),%rax
     318c:   0f b6 00    movzbl (%rax),%eax
  offset += qemu_real_host_page_size) {
     318f:   48 01 fa    add    %rdi,%rdx
     for (offset = 0; offset < bs->used_length;
     3192:   48 39 ca    cmp    %rcx,%rdx
     3195:   72 f1   jb 3188



[1]
https://programfan.github.io/blog/2015/04/27/prevent-gcc-optimize-away-code/


I'll send patches soon to take care of virtio-mem via RamDiscardManager -
to skip populating the parts that are supposed to remain discarded and
not migrated.
Unfortunately, the RamDiscardManager patches are still stuck waiting for
acks ... and now we're in soft-freeze.


RamDiscardManager patches - do they also modify migration code?
I mean which part is responsible of not migrating discarded ranges.


I haven't shared relevant patches yet that touch migration code. I'm 
planning on doing that once the generic RamDiscardManager has all 
relevant acks. I'll put you on cc.


--
Thanks,

David / dhildenb

[PATCH 11/15] virtio-gpu: move update_cursor_data

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 include/hw/virtio/virtio-gpu.h |  6 ++
 hw/display/virtio-gpu-gl.c | 30 +++
 hw/display/virtio-gpu.c| 38 ++
 3 files changed, 42 insertions(+), 32 deletions(-)

diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 4ce39d2abb4c..cd55c2d07090 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -171,6 +171,9 @@ struct VirtIOGPUClass {
 
 void (*handle_ctrl)(VirtIODevice *vdev, VirtQueue *vq);
 void (*process_cmd)(VirtIOGPU *g, struct virtio_gpu_ctrl_command *cmd);
+void (*update_cursor_data)(VirtIOGPU *g,
+   struct virtio_gpu_scanout *s,
+   uint32_t resource_id);
 };
 
 struct VirtIOGPUGL {
@@ -230,6 +233,9 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g);
 void virtio_gpu_device_realize(DeviceState *qdev, Error **errp);
 void virtio_gpu_reset(VirtIODevice *vdev);
 void virtio_gpu_simple_process_cmd(VirtIOGPU *g, struct 
virtio_gpu_ctrl_command *cmd);
+void virtio_gpu_update_cursor_data(VirtIOGPU *g,
+   struct virtio_gpu_scanout *s,
+   uint32_t resource_id);
 
 /* virtio-gpu-3d.c */
 void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 792cc0b41256..b4303cc5bb41 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -23,6 +23,35 @@
 #include "hw/virtio/virtio-gpu-pixman.h"
 #include "hw/qdev-properties.h"
 
+#include 
+
+static void virtio_gpu_gl_update_cursor_data(VirtIOGPU *g,
+ struct virtio_gpu_scanout *s,
+ uint32_t resource_id)
+{
+uint32_t width, height;
+uint32_t pixels, *data;
+
+if (g->parent_obj.use_virgl_renderer) {
+data = virgl_renderer_get_cursor_data(resource_id, &width, &height);
+if (!data) {
+return;
+}
+
+if (width != s->current_cursor->width ||
+height != s->current_cursor->height) {
+free(data);
+return;
+}
+
+pixels = s->current_cursor->width * s->current_cursor->height;
+memcpy(s->current_cursor->data, data, pixels * sizeof(uint32_t));
+free(data);
+return;
+}
+virtio_gpu_update_cursor_data(g, s, resource_id);
+}
+
 static void virtio_gpu_gl_process_cmd(VirtIOGPU *g,
   struct virtio_gpu_ctrl_command *cmd)
 {
@@ -127,6 +156,7 @@ static void virtio_gpu_gl_class_init(ObjectClass *klass, 
void *data)
 vbc->gl_flushed = virtio_gpu_gl_flushed;
 vgc->handle_ctrl = virtio_gpu_gl_handle_ctrl;
 vgc->process_cmd = virtio_gpu_gl_process_cmd;
+vgc->update_cursor_data = virtio_gpu_gl_update_cursor_data;
 
 vdc->realize = virtio_gpu_gl_device_realize;
 vdc->reset = virtio_gpu_gl_reset;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index e61bfffa8019..2c0065277ffd 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -56,9 +56,9 @@ static void virtio_gpu_cleanup_mapping(VirtIOGPU *g,
 } while (0)
 #endif
 
-static void update_cursor_data_simple(VirtIOGPU *g,
-  struct virtio_gpu_scanout *s,
-  uint32_t resource_id)
+void virtio_gpu_update_cursor_data(VirtIOGPU *g,
+   struct virtio_gpu_scanout *s,
+   uint32_t resource_id)
 {
 struct virtio_gpu_simple_resource *res;
 uint32_t pixels;
@@ -79,36 +79,10 @@ static void update_cursor_data_simple(VirtIOGPU *g,
pixels * sizeof(uint32_t));
 }
 
-#ifdef CONFIG_VIRGL
-
-static void update_cursor_data_virgl(VirtIOGPU *g,
- struct virtio_gpu_scanout *s,
- uint32_t resource_id)
-{
-uint32_t width, height;
-uint32_t pixels, *data;
-
-data = virgl_renderer_get_cursor_data(resource_id, &width, &height);
-if (!data) {
-return;
-}
-
-if (width != s->current_cursor->width ||
-height != s->current_cursor->height) {
-free(data);
-return;
-}
-
-pixels = s->current_cursor->width * s->current_cursor->height;
-memcpy(s->current_cursor->data, data, pixels * sizeof(uint32_t));
-free(data);
-}
-
-#endif
-
 static void update_cursor(VirtIOGPU *g, struct virtio_gpu_update_cursor 
*cursor)
 {
 struct virtio_gpu_scanout *s;
+VirtIOGPUClass *vgc = VIRTIO_GPU_GET_CLASS(g);
 bool move = cursor->hdr.type == VIRTIO_GPU_CMD_MOVE_CURSOR;
 
 if (cursor->pos.scanout_id >= g->parent_obj.conf.max_outputs) {
@@ -131,8 +105,7 @@ static void update_cursor(VirtIOGPU *g, struct 
virtio_gpu_update_cursor *cursor)
 s->current_cursor->hot_y = cursor->hot_y;
 
 if (cursor

[PATCH 15/15] virtio-gpu: move fields to struct VirtIOGPUGL

[Gerd Hoffmann]

Move two virglrenderer state variables to struct VirtIOGPUGL.

Signed-off-by: Gerd Hoffmann 
---
 include/hw/virtio/virtio-gpu.h |  5 +++--
 hw/display/virtio-gpu-gl.c | 15 +--
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 9629885c895f..0a8281aeb555 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -151,8 +151,6 @@ struct VirtIOGPU {
 uint64_t hostmem;
 
 bool processing_cmdq;
-bool renderer_inited;
-bool renderer_reset;
 QEMUTimer *fence_poll;
 QEMUTimer *print_stats;
 
@@ -177,6 +175,9 @@ struct VirtIOGPUClass {
 
 struct VirtIOGPUGL {
 struct VirtIOGPU parent_obj;
+
+bool renderer_inited;
+bool renderer_reset;
 };
 
 struct VhostUserGPU {
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 1642a973549e..d971b480806a 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -51,9 +51,10 @@ static void virtio_gpu_gl_update_cursor_data(VirtIOGPU *g,
 static void virtio_gpu_gl_flushed(VirtIOGPUBase *b)
 {
 VirtIOGPU *g = VIRTIO_GPU(b);
+VirtIOGPUGL *gl = VIRTIO_GPU_GL(b);
 
-if (g->renderer_reset) {
-g->renderer_reset = false;
+if (gl->renderer_reset) {
+gl->renderer_reset = false;
 virtio_gpu_virgl_reset(g);
 }
 virtio_gpu_process_cmdq(g);
@@ -62,15 +63,16 @@ static void virtio_gpu_gl_flushed(VirtIOGPUBase *b)
 static void virtio_gpu_gl_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 {
 VirtIOGPU *g = VIRTIO_GPU(vdev);
+VirtIOGPUGL *gl = VIRTIO_GPU_GL(vdev);
 struct virtio_gpu_ctrl_command *cmd;
 
 if (!virtio_queue_ready(vq)) {
 return;
 }
 
-if (!g->renderer_inited) {
+if (!gl->renderer_inited) {
 virtio_gpu_virgl_init(g);
-g->renderer_inited = true;
+gl->renderer_inited = true;
 }
 
 cmd = virtqueue_pop(vq, sizeof(struct virtio_gpu_ctrl_command));
@@ -89,12 +91,13 @@ static void virtio_gpu_gl_handle_ctrl(VirtIODevice *vdev, 
VirtQueue *vq)
 static void virtio_gpu_gl_reset(VirtIODevice *vdev)
 {
 VirtIOGPU *g = VIRTIO_GPU(vdev);
+VirtIOGPUGL *gl = VIRTIO_GPU_GL(vdev);
 
 virtio_gpu_reset(vdev);
 
-if (g->renderer_inited) {
+if (gl->renderer_inited) {
 if (g->parent_obj.renderer_blocked) {
-g->renderer_reset = true;
+gl->renderer_reset = true;
 } else {
 virtio_gpu_virgl_reset(g);
 }
-- 
2.30.2

[PATCH 09/15] virtio-gpu: move virgl gl_flushed

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 hw/display/virtio-gpu-gl.c | 13 +
 hw/display/virtio-gpu.c| 15 ---
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 6d0ce5bcd6f1..e976fb8d04c4 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -23,6 +23,17 @@
 #include "hw/virtio/virtio-gpu-pixman.h"
 #include "hw/qdev-properties.h"
 
+static void virtio_gpu_gl_flushed(VirtIOGPUBase *b)
+{
+VirtIOGPU *g = VIRTIO_GPU(b);
+
+if (g->renderer_reset) {
+g->renderer_reset = false;
+virtio_gpu_virgl_reset(g);
+}
+virtio_gpu_process_cmdq(g);
+}
+
 static void virtio_gpu_gl_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 {
 VirtIOGPU *g = VIRTIO_GPU(vdev);
@@ -100,8 +111,10 @@ static void virtio_gpu_gl_class_init(ObjectClass *klass, 
void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
 VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+VirtIOGPUBaseClass *vbc = VIRTIO_GPU_BASE_CLASS(klass);
 VirtIOGPUClass *vgc = VIRTIO_GPU_CLASS(klass);
 
+vbc->gl_flushed = virtio_gpu_gl_flushed;
 vgc->handle_ctrl = virtio_gpu_gl_handle_ctrl;
 
 vdc->realize = virtio_gpu_gl_device_realize;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 5901e09bcd81..ae80519c7356 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -843,19 +843,6 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g)
 g->processing_cmdq = false;
 }
 
-static void virtio_gpu_gl_flushed(VirtIOGPUBase *b)
-{
-VirtIOGPU *g = VIRTIO_GPU(b);
-
-#ifdef CONFIG_VIRGL
-if (g->renderer_reset) {
-g->renderer_reset = false;
-virtio_gpu_virgl_reset(g);
-}
-#endif
-virtio_gpu_process_cmdq(g);
-}
-
 static void virtio_gpu_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 {
 VirtIOGPU *g = VIRTIO_GPU(vdev);
@@ -1199,10 +1186,8 @@ static void virtio_gpu_class_init(ObjectClass *klass, 
void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
 VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-VirtIOGPUBaseClass *vbc = VIRTIO_GPU_BASE_CLASS(klass);
 VirtIOGPUClass *vgc = VIRTIO_GPU_CLASS(klass);
 
-vbc->gl_flushed = virtio_gpu_gl_flushed;
 vgc->handle_ctrl = virtio_gpu_handle_ctrl;
 
 vdc->realize = virtio_gpu_device_realize;
-- 
2.30.2

Re: [PATCH 4/4] iotests: iothreads need ioeventfd

[Cornelia Huck]

On Fri, 19 Mar 2021 12:06:43 +0100
Paolo Bonzini  wrote:

> On 18/03/21 23:39, Laurent Vivier wrote:
> > And ioeventfd are only available with virtio-scsi-pci, so don't use the 
> > alias
> > and add a rule to require virtio-scsi-pci for the tests that use iothreads.
> > 
> > Signed-off-by: Laurent Vivier 
> > ---
> >   tests/qemu-iotests/127| 4 ++--
> >   tests/qemu-iotests/256| 2 ++
> >   tests/qemu-iotests/iotests.py | 5 +
> >   3 files changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/qemu-iotests/127 b/tests/qemu-iotests/127
> > index 98e8e82a8210..a3693533685a 100755
> > --- a/tests/qemu-iotests/127
> > +++ b/tests/qemu-iotests/127
> > @@ -44,7 +44,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> >   _supported_fmt qcow2
> >   _supported_proto file fuse
> >   
> > -_require_devices virtio-scsi scsi-hd
> > +_require_devices virtio-scsi-pci scsi-hd  
> 
> Maybe
> 
> _require_devices scsi-hd
> _require_devices virtio-scsi-pci || _require_devices virtio-scsi ccw
> 
> ?
> 
> Paolo
> 

Yes, ioeventfds are also available for ccw; I'd expect only mmio to be
the problem here.

[PATCH 08/15] virtio-gpu: move virgl handle_ctrl

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 hw/display/virtio-gpu-gl.c | 33 +
 hw/display/virtio-gpu.c| 13 -
 2 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index c3e562f835f7..6d0ce5bcd6f1 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -23,6 +23,36 @@
 #include "hw/virtio/virtio-gpu-pixman.h"
 #include "hw/qdev-properties.h"
 
+static void virtio_gpu_gl_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
+{
+VirtIOGPU *g = VIRTIO_GPU(vdev);
+struct virtio_gpu_ctrl_command *cmd;
+
+if (!virtio_queue_ready(vq)) {
+return;
+}
+
+if (!g->renderer_inited && g->parent_obj.use_virgl_renderer) {
+virtio_gpu_virgl_init(g);
+g->renderer_inited = true;
+}
+
+cmd = virtqueue_pop(vq, sizeof(struct virtio_gpu_ctrl_command));
+while (cmd) {
+cmd->vq = vq;
+cmd->error = 0;
+cmd->finished = false;
+QTAILQ_INSERT_TAIL(&g->cmdq, cmd, next);
+cmd = virtqueue_pop(vq, sizeof(struct virtio_gpu_ctrl_command));
+}
+
+virtio_gpu_process_cmdq(g);
+
+if (g->parent_obj.use_virgl_renderer) {
+virtio_gpu_virgl_fence_poll(g);
+}
+}
+
 static void virtio_gpu_gl_reset(VirtIODevice *vdev)
 {
 VirtIOGPU *g = VIRTIO_GPU(vdev);
@@ -70,6 +100,9 @@ static void virtio_gpu_gl_class_init(ObjectClass *klass, 
void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
 VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+VirtIOGPUClass *vgc = VIRTIO_GPU_CLASS(klass);
+
+vgc->handle_ctrl = virtio_gpu_gl_handle_ctrl;
 
 vdc->realize = virtio_gpu_gl_device_realize;
 vdc->reset = virtio_gpu_gl_reset;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 39ef22b7c08d..5901e09bcd81 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -865,13 +865,6 @@ static void virtio_gpu_handle_ctrl(VirtIODevice *vdev, 
VirtQueue *vq)
 return;
 }
 
-#ifdef CONFIG_VIRGL
-if (!g->renderer_inited && g->parent_obj.use_virgl_renderer) {
-virtio_gpu_virgl_init(g);
-g->renderer_inited = true;
-}
-#endif
-
 cmd = virtqueue_pop(vq, sizeof(struct virtio_gpu_ctrl_command));
 while (cmd) {
 cmd->vq = vq;
@@ -882,12 +875,6 @@ static void virtio_gpu_handle_ctrl(VirtIODevice *vdev, 
VirtQueue *vq)
 }
 
 virtio_gpu_process_cmdq(g);
-
-#ifdef CONFIG_VIRGL
-if (g->parent_obj.use_virgl_renderer) {
-virtio_gpu_virgl_fence_poll(g);
-}
-#endif
 }
 
 static void virtio_gpu_ctrl_bh(void *opaque)
-- 
2.30.2

Re: Serious doubts about Gitlab CI

[Philippe Mathieu-Daudé]

On 3/19/21 11:59 AM, Paolo Bonzini wrote:
> On 19/03/21 11:18, Andrew Jones wrote:
>>> Yikes, that is 41 hours per CI run. I wonder if GitLab's CI minutes are
>>> on slow machines or if we'll hit the same issue with dedicated runners.
>>> It seems like CI optimization will be necessary...
>>>
>> We need to reduce the amount of CI we do, not only because we can't
>> afford
>> it, but because it's wasteful. I hate to think of all the kWhs spent
>> testing the exact same code in the exact same way, since everyone runs
>> everything with a simple 'git push'.
> 
> Yes, I thought the same.
> 
>> IMHO, 'git push' shouldn't trigger
>> anything. Starting CI should be an explicit step.

* tests/acceptance: Only run tests tagged 'gating-ci' on GitLab CI
https://www.mail-archive.com/qemu-devel@nongnu.org/msg756464.html

* gitlab-ci: Allow forks to select & restrict build jobs
https://www.mail-archive.com/qemu-devel@nongnu.org/msg758331.html

> It is possible to do that on a project that uses merge requests, for
> example like this:
> 
> workflow:
>   rules:
>     - if: '$CI_PIPELINE_SOURCE == "merge_request_event"'
>     - if: '$CI_COMMIT_BRANCH
>   when: never
> 
> For us it's a bit more complicated (no merge requests).
> 
> Another common feature is failing the pipeline immediately if one of the
> jobs fail, but GitLab does not support it
> (https://gitlab.com/gitlab-org/gitlab/-/issues/23605).
> 
>> Also, the default CI
>> should only trigger tests associated with the code changed. One should
>> have to explicitly trigger a complete CI when they deem it worthwhile.
> 
> This is interesting.  We could add a stage that looks for changed files
> using "git diff" and sets some variables (e.g. softmmu, user, TCG,
> various targets) based on the results.  Then you use those to skip some
> jobs or some tests, for example skipping check-tcg.  See
> https://docs.gitlab.com/ee/ci/variables/#inherit-cicd-variables for more
> information.
> 
> Paolo
> 
> 

[PATCH 14/15] virtio-gpu: drop use_virgl_renderer

[Gerd Hoffmann]

Now that we have separated the gl and non-gl code flows to two different
devices there is little reason turn on and off virglrenderer usage at
runtime.  The gl code can simply use virglrenderer unconditionally.

So drop use_virgl_renderer field and just do that.

Signed-off-by: Gerd Hoffmann 
---
 include/hw/virtio/virtio-gpu.h |  1 -
 hw/display/virtio-gpu-base.c   |  6 +
 hw/display/virtio-gpu-gl.c | 44 ++
 3 files changed, 14 insertions(+), 37 deletions(-)

diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index cd55c2d07090..9629885c895f 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -111,7 +111,6 @@ struct VirtIOGPUBase {
 struct virtio_gpu_config virtio_config;
 const GraphicHwOps *hw_ops;
 
-bool use_virgl_renderer;
 int renderer_blocked;
 int enable;
 
diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index 25f8920fdb67..afb3ee7d9afc 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -25,7 +25,6 @@ virtio_gpu_base_reset(VirtIOGPUBase *g)
 int i;
 
 g->enable = 0;
-g->use_virgl_renderer = false;
 
 for (i = 0; i < g->conf.max_outputs; i++) {
 g->scanout[i].resource_id = 0;
@@ -162,7 +161,6 @@ virtio_gpu_base_device_realize(DeviceState *qdev,
 return false;
 }
 
-g->use_virgl_renderer = false;
 if (virtio_gpu_virgl_enabled(g->conf)) {
 error_setg(&g->migration_blocker, "virgl is not yet migratable");
 if (migrate_add_blocker(g->migration_blocker, errp) < 0) {
@@ -218,10 +216,8 @@ static void
 virtio_gpu_base_set_features(VirtIODevice *vdev, uint64_t features)
 {
 static const uint32_t virgl = (1 << VIRTIO_GPU_F_VIRGL);
-VirtIOGPUBase *g = VIRTIO_GPU_BASE(vdev);
 
-g->use_virgl_renderer = ((features & virgl) == virgl);
-trace_virtio_gpu_features(g->use_virgl_renderer);
+trace_virtio_gpu_features(((features & virgl) == virgl));
 }
 
 static void
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index b4303cc5bb41..1642a973549e 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -32,34 +32,20 @@ static void virtio_gpu_gl_update_cursor_data(VirtIOGPU *g,
 uint32_t width, height;
 uint32_t pixels, *data;
 
-if (g->parent_obj.use_virgl_renderer) {
-data = virgl_renderer_get_cursor_data(resource_id, &width, &height);
-if (!data) {
-return;
-}
+data = virgl_renderer_get_cursor_data(resource_id, &width, &height);
+if (!data) {
+return;
+}
 
-if (width != s->current_cursor->width ||
-height != s->current_cursor->height) {
-free(data);
-return;
-}
-
-pixels = s->current_cursor->width * s->current_cursor->height;
-memcpy(s->current_cursor->data, data, pixels * sizeof(uint32_t));
+if (width != s->current_cursor->width ||
+height != s->current_cursor->height) {
 free(data);
 return;
 }
-virtio_gpu_update_cursor_data(g, s, resource_id);
-}
 
-static void virtio_gpu_gl_process_cmd(VirtIOGPU *g,
-  struct virtio_gpu_ctrl_command *cmd)
-{
-if (g->parent_obj.use_virgl_renderer) {
-virtio_gpu_virgl_process_cmd(g, cmd);
-return;
-}
-virtio_gpu_simple_process_cmd(g, cmd);
+pixels = s->current_cursor->width * s->current_cursor->height;
+memcpy(s->current_cursor->data, data, pixels * sizeof(uint32_t));
+free(data);
 }
 
 static void virtio_gpu_gl_flushed(VirtIOGPUBase *b)
@@ -82,7 +68,7 @@ static void virtio_gpu_gl_handle_ctrl(VirtIODevice *vdev, 
VirtQueue *vq)
 return;
 }
 
-if (!g->renderer_inited && g->parent_obj.use_virgl_renderer) {
+if (!g->renderer_inited) {
 virtio_gpu_virgl_init(g);
 g->renderer_inited = true;
 }
@@ -97,10 +83,7 @@ static void virtio_gpu_gl_handle_ctrl(VirtIODevice *vdev, 
VirtQueue *vq)
 }
 
 virtio_gpu_process_cmdq(g);
-
-if (g->parent_obj.use_virgl_renderer) {
-virtio_gpu_virgl_fence_poll(g);
-}
+virtio_gpu_virgl_fence_poll(g);
 }
 
 static void virtio_gpu_gl_reset(VirtIODevice *vdev)
@@ -109,13 +92,12 @@ static void virtio_gpu_gl_reset(VirtIODevice *vdev)
 
 virtio_gpu_reset(vdev);
 
-if (g->parent_obj.use_virgl_renderer) {
+if (g->renderer_inited) {
 if (g->parent_obj.renderer_blocked) {
 g->renderer_reset = true;
 } else {
 virtio_gpu_virgl_reset(g);
 }
-g->parent_obj.use_virgl_renderer = false;
 }
 }
 
@@ -155,7 +137,7 @@ static void virtio_gpu_gl_class_init(ObjectClass *klass, 
void *data)
 
 vbc->gl_flushed = virtio_gpu_gl_flushed;
 vgc->handle_ctrl = virtio_gpu_gl_handle_ctrl;
-vgc->process_cmd = virtio_gpu_gl_process_cmd;
+vgc->process_cmd = virtio_gpu_virgl_process_cmd;
 vgc->update_cursor_da

Re: [PATCH 1/4] m68k: add the virtio devices aliases

[Cornelia Huck]

On Thu, 18 Mar 2021 23:39:04 +0100
Laurent Vivier  wrote:

> Similarly to 5f629d943cb0 ("s390x: fix s390 virtio aliases"),
> define the virtio aliases.
> 
> This allows to start machines with virtio devices without
> knowledge of the implementation type.
> 
> For instance, we can use "-device virtio-scsi" on
> m68k, s390x or PC, and the device will be
> "virtio-scsi-device", "virtio-scsi-ccw" or "virtio-scsi-pci".
> 
> This already exists for s390x and -ccw interfaces, adds them
> for m68k and MMIO (-device) interfaces.
> 
> Signed-off-by: Laurent Vivier 
> ---
>  softmmu/qdev-monitor.c | 46 +++---
>  1 file changed, 30 insertions(+), 16 deletions(-)
> 
> diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
> index 8dc656becca9..262d38b8c01e 100644
> --- a/softmmu/qdev-monitor.c
> +++ b/softmmu/qdev-monitor.c
> @@ -42,6 +42,8 @@
>  #include "hw/qdev-properties.h"
>  #include "hw/clock.h"
>  
> +#define QEMU_ARCH_NO_PCI (QEMU_ARCH_S390X | QEMU_ARCH_M68K)

The name of the #define is a tad misleading (we do have virtio-pci
devices on s390x, unlike in 2012, we just don't want the aliases to
point to them.) Maybe QEMU_ARCH_NONPCI_DEFAULT?

> +
>  /*
>   * Aliases were a bad idea from the start.  Let's keep them
>   * from spreading further.

Otherwise, LGTM.

[PATCH] tests/unit/test-block-iothread: fix maybe-uninitialized error on GCC 11

[Emanuele Giuseppe Esposito]

When building qemu with GCC 11, test-block-iothread produces the following
warning:

../tests/unit/test-block-iothread.c:148:11: error: ‘buf’ may be used
uninitialized [-Werror=maybe-uninitialized]

This is caused by buf[512] left uninitialized and passed to
bdrv_save_vmstate() that expects a const uint8_t *, so the compiler
assumes it will be read and expects the parameter to be initialized.

Signed-off-by: Emanuele Giuseppe Esposito 
---
 tests/unit/test-block-iothread.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/unit/test-block-iothread.c b/tests/unit/test-block-iothread.c
index 3f866a35c6..8cf172cb7a 100644
--- a/tests/unit/test-block-iothread.c
+++ b/tests/unit/test-block-iothread.c
@@ -89,7 +89,7 @@ static void test_sync_op_pread(BdrvChild *c)
 
 static void test_sync_op_pwrite(BdrvChild *c)
 {
-uint8_t buf[512];
+uint8_t buf[512] = { 0 };
 int ret;
 
 /* Success */
@@ -117,7 +117,7 @@ static void test_sync_op_blk_pread(BlockBackend *blk)
 
 static void test_sync_op_blk_pwrite(BlockBackend *blk)
 {
-uint8_t buf[512];
+uint8_t buf[512] = { 0 };
 int ret;
 
 /* Success */
@@ -141,7 +141,7 @@ static void test_sync_op_load_vmstate(BdrvChild *c)
 
 static void test_sync_op_save_vmstate(BdrvChild *c)
 {
-uint8_t buf[512];
+uint8_t buf[512] = { 0 };
 int ret;
 
 /* Error: Driver does not support snapshots */
-- 
2.29.2

Re: [PATCH] tests/unit/test-block-iothread: fix maybe-uninitialized error on GCC 11

[Paolo Bonzini]

On 19/03/21 12:22, Emanuele Giuseppe Esposito wrote:

When building qemu with GCC 11, test-block-iothread produces the following
warning:

../tests/unit/test-block-iothread.c:148:11: error: ‘buf’ may be used
uninitialized [-Werror=maybe-uninitialized]

This is caused by buf[512] left uninitialized and passed to
bdrv_save_vmstate() that expects a const uint8_t *, so the compiler
assumes it will be read and expects the parameter to be initialized.

Signed-off-by: Emanuele Giuseppe Esposito 
---
  tests/unit/test-block-iothread.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/unit/test-block-iothread.c b/tests/unit/test-block-iothread.c
index 3f866a35c6..8cf172cb7a 100644
--- a/tests/unit/test-block-iothread.c
+++ b/tests/unit/test-block-iothread.c
@@ -89,7 +89,7 @@ static void test_sync_op_pread(BdrvChild *c)
  
  static void test_sync_op_pwrite(BdrvChild *c)

  {
-uint8_t buf[512];
+uint8_t buf[512] = { 0 };
  int ret;
  
  /* Success */

@@ -117,7 +117,7 @@ static void test_sync_op_blk_pread(BlockBackend *blk)
  
  static void test_sync_op_blk_pwrite(BlockBackend *blk)

  {
-uint8_t buf[512];
+uint8_t buf[512] = { 0 };
  int ret;
  
  /* Success */

@@ -141,7 +141,7 @@ static void test_sync_op_load_vmstate(BdrvChild *c)
  
  static void test_sync_op_save_vmstate(BdrvChild *c)

  {
-uint8_t buf[512];
+uint8_t buf[512] = { 0 };
  int ret;
  
  /* Error: Driver does not support snapshots */




Reviewed-by: Paolo Bonzini 

Re: [PATCH 00/15] virtio-gpu: split into two devices.

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20210319112147.4138943-1-kra...@redhat.com/



Hi,

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

Type: series
Message-id: 20210319112147.4138943-1-kra...@redhat.com
Subject: [PATCH 00/15] virtio-gpu: split into two devices.

=== 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
 - [tag update]  patchew/20210311165947.27470-1-peter.mayd...@linaro.org -> 
patchew/20210311165947.27470-1-peter.mayd...@linaro.org
 * [new tag] patchew/20210319112147.4138943-1-kra...@redhat.com -> 
patchew/20210319112147.4138943-1-kra...@redhat.com
Switched to a new branch 'test'
d701bd8 virtio-gpu: move fields to struct VirtIOGPUGL
3efa9d2 virtio-gpu: drop use_virgl_renderer
b347213 virtio-gpu: move virtio-gpu-gl-device to separate module
d09c1f0 virtio-gpu: drop VIRGL() macro
05d9255 virtio-gpu: move update_cursor_data
192d336 virtio-gpu: move virgl process_cmd
8eeb29f virtio-gpu: move virgl gl_flushed
9e76d10 virtio-gpu: move virgl handle_ctrl
d272555 virtio-gpu: use class function for ctrl queue handlers
c37479b virtio-gpu: move virgl reset
576cff3 virtio-gpu: move virgl realize + properties
04c9c17 virtio-gpu: add virtio-vga-gl
e6934ec virtio-gpu: add virtio-gpu-gl-pci
57c1b29 virtio-gpu: add virtio-gpu-gl-device
660100e virtio-gpu: rename virgl source file.

=== OUTPUT BEGIN ===
1/15 Checking commit 660100eb846e (virtio-gpu: rename virgl source file.)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#14: 
 hw/display/{virtio-gpu-3d.c => virtio-gpu-virgl.c} | 0

total: 0 errors, 1 warnings, 8 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 57c1b2934802 (virtio-gpu: add virtio-gpu-gl-device)
Use of uninitialized value $acpi_testexpected in string eq at 
./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#32: 
new file mode 100644

total: 0 errors, 1 warnings, 75 lines checked

Patch 2/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/15 Checking commit e6934ec37d3d (virtio-gpu: add virtio-gpu-gl-pci)
4/15 Checking commit 04c9c179a093 (virtio-gpu: add virtio-vga-gl)
5/15 Checking commit 576cff3a938e (virtio-gpu: move virgl realize + properties)
6/15 Checking commit c37479b06276 (virtio-gpu: move virgl reset)
7/15 Checking commit d2725554fce3 (virtio-gpu: use class function for ctrl 
queue handlers)
8/15 Checking commit 9e76d10e3190 (virtio-gpu: move virgl handle_ctrl)
9/15 Checking commit 8eeb29f94541 (virtio-gpu: move virgl gl_flushed)
10/15 Checking commit 192d3360b319 (virtio-gpu: move virgl process_cmd)
WARNING: line over 80 characters
#101: FILE: include/hw/virtio/virtio-gpu.h:232:
+void virtio_gpu_simple_process_cmd(VirtIOGPU *g, struct 
virtio_gpu_ctrl_command *cmd);

total: 0 errors, 1 warnings, 70 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 05d925520ac0 (virtio-gpu: move update_cursor_data)
12/15 Checking commit d09c1f0ac45a (virtio-gpu: drop VIRGL() macro)
13/15 Checking commit b347213cfde6 (virtio-gpu: move virtio-gpu-gl-device to 
separate module)
14/15 Checking commit 3efa9d26ea18 (virtio-gpu: drop use_virgl_renderer)
ERROR: "foo * bar" should be "foo *bar"
#96: FILE: hw/display/virtio-gpu-gl.c:47:
+memcpy(s->current_cursor->data, data, pixels * sizeof(uint32_t));

total: 1 errors, 0 warnings, 116 lines checked

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

15/15 Checking commit d701bd87ca8e (virtio-gpu: move fields to struct 
VirtIOGPUGL)
=== OUTPUT END ===

Test command exited with code: 1


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

Re: [PATCH 4/4] iotests: iothreads need ioeventfd

[Laurent Vivier]

Le 19/03/2021 à 12:27, Cornelia Huck a écrit :
> On Fri, 19 Mar 2021 12:06:43 +0100
> Paolo Bonzini  wrote:
> 
>> On 18/03/21 23:39, Laurent Vivier wrote:
>>> And ioeventfd are only available with virtio-scsi-pci, so don't use the 
>>> alias
>>> and add a rule to require virtio-scsi-pci for the tests that use iothreads.
>>>
>>> Signed-off-by: Laurent Vivier 
>>> ---
>>>   tests/qemu-iotests/127| 4 ++--
>>>   tests/qemu-iotests/256| 2 ++
>>>   tests/qemu-iotests/iotests.py | 5 +
>>>   3 files changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tests/qemu-iotests/127 b/tests/qemu-iotests/127
>>> index 98e8e82a8210..a3693533685a 100755
>>> --- a/tests/qemu-iotests/127
>>> +++ b/tests/qemu-iotests/127
>>> @@ -44,7 +44,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
>>>   _supported_fmt qcow2
>>>   _supported_proto file fuse
>>>   
>>> -_require_devices virtio-scsi scsi-hd
>>> +_require_devices virtio-scsi-pci scsi-hd  
>>
>> Maybe
>>
>> _require_devices scsi-hd
>> _require_devices virtio-scsi-pci || _require_devices virtio-scsi ccw
>>
>> ?
>>
>> Paolo
>>
> 
> Yes, ioeventfds are also available for ccw; I'd expect only mmio to be
> the problem here.
> 

OK, thanks.

I update my patch with the changes from Paolo.

Laurent

Re: Serious doubts about Gitlab CI

[Philippe Mathieu-Daudé]

On 3/19/21 10:41 AM, Paolo Bonzini wrote:
> On 19/03/21 10:33, Stefan Hajnoczi wrote:
>> On Thu, Mar 18, 2021 at 09:30:41PM +0100, Paolo Bonzini wrote:
>>> On 18/03/21 20:46, Stefan Hajnoczi wrote:
 The QEMU Project has 50,000 minutes of GitLab CI quota. Let's enable
 GitLab Merge Requests so that anyone can submit a merge request and get
 CI coverage.
>>>
>>> Each merge request consumes about 2500.  That won't last long.
>>
>> Yikes, that is 41 hours per CI run. I wonder if GitLab's CI minutes are
>> on slow machines or if we'll hit the same issue with dedicated runners.
>> It seems like CI optimization will be necessary...
> 
> Shared runners are 1 vCPU, so it's really 41 CPU hours per CI run.
> That's a lot but not unheard of.
> 
> Almost every 2-socket server these days will have at least 50 CPUs; with
> some optimization we probably can get it down to half an hour of real
> time, on a single server running 3-4 runners with 16 vCPUs.

Yesterday I tried to add my wife's computer she use at home to
my gitlab namespace to test Laurent latest series.

Specs:

- Intel(R) Core(TM) i7-7567U CPU @ 3.50GHz
- SSD 256GB
- 16GB RAM

So 1 runner with 4 vCPUs.

With 9 failed jobs, and 2 not run (due to previous stage failure),
the pipeline summary is:

130 jobs for m68k-iotests in 623 minutes and 49 seconds (queued for 31
seconds)

Network bandwidth/latency isn't an issue, I have a decent connection
IMO.

# du -chs /var/lib/docker
67G /var/lib/docker

^ This is a lot (fresh docker install)

This matches your "41 CPU hours per CI run." comment.

Regards,

Phil.