All of lore.kernel.org
 help / color / mirror / Atom feed
* [PULL 00/30] Misc changes for 2021-10-03
@ 2021-10-03  7:42 Paolo Bonzini
  2021-10-03  7:42 ` [PULL 01/30] qapi/machine: Fix an incorrect comment of SMPConfiguration Paolo Bonzini
                   ` (31 more replies)
  0 siblings, 32 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel

The following changes since commit bb4aa8f59e18412cff0d69f14aee7abba153161a:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20210930' into staging (2021-09-30 21:16:54 +0100)

are available in the Git repository at:

  https://gitlab.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to cb83ba8c1ab856b4327e7e869c410bbfd4152c2c:

  softmmu/memory_mapping: optimize for RamDiscardManager sections (2021-10-02 08:43:22 +0200)

----------------------------------------------------------------
* -smp cleanpus (Yanan)
* Hyper-V enlightenment functionality (Vitaly)
* virtio-mem support in dump, tpm and QMP (David)
* NetBSD GCC 7.4 compiler support (Nia)

----------------------------------------------------------------
David Hildenbrand (7):
      virtio-mem-pci: Fix memory leak when creating MEMORY_DEVICE_SIZE_CHANGE event
      qapi: Include qom-path in MEMORY_DEVICE_SIZE_CHANGE qapi events
      monitor: Rate-limit MEMORY_DEVICE_SIZE_CHANGE qapi events per device
      tpm: mark correct memory region range dirty when clearing RAM
      softmmu/memory_mapping: never merge ranges accross memory regions
      softmmu/memory_mapping: factor out adding physical memory ranges
      softmmu/memory_mapping: optimize for RamDiscardManager sections

Paolo Bonzini (1):
      machine: Use g_autoptr in machine_set_smp

Vitaly Kuznetsov (6):
      i386: Support KVM_CAP_ENFORCE_PV_FEATURE_CPUID
      i386: Support KVM_CAP_HYPERV_ENFORCE_CPUID
      i386: Move HV_APIC_ACCESS_RECOMMENDED bit setting to hyperv_fill_cpuids()
      i386: Implement pseudo 'hv-avic' ('hv-apicv') enlightenment
      i386: Make Hyper-V version id configurable
      i386: Change the default Hyper-V version to match WS2016

Yanan Wang (15):
      qapi/machine: Fix an incorrect comment of SMPConfiguration
      machine: Deprecate "parameter=0" SMP configurations
      machine: Minor refactor/fix for the smp parsers
      machine: Uniformly use maxcpus to calculate the omitted parameters
      machine: Set the value of cpus to match maxcpus if it's omitted
      machine: Improve the error reporting of smp parsing
      qtest/numa-test: Use detailed -smp CLIs in pc_dynamic_cpu_cfg
      qtest/numa-test: Use detailed -smp CLIs in test_def_cpu_split
      machine: Prefer cores over sockets in smp parsing since 6.2
      machine: Use ms instead of global current_machine in sanity-check
      machine: Tweak the order of topology members in struct CpuTopology
      machine: Make smp_parse generic enough for all arches
      machine: Remove smp_parse callback from MachineClass
      machine: Move smp_prefer_sockets to struct SMPCompatProps
      machine: Put all sanity-check in the generic SMP parser

nia (1):
      configure: Loosen GCC requirement from 7.5.0 to 7.4.0

 configure                      |   6 +-
 docs/about/deprecated.rst      |  15 +++
 docs/hyperv.txt                |  41 +++++++-
 hw/arm/virt.c                  |   1 +
 hw/core/machine.c              | 209 ++++++++++++++++++++++++++++-------------
 hw/i386/pc.c                   |  69 ++------------
 hw/i386/pc_piix.c              |   1 +
 hw/i386/pc_q35.c               |   1 +
 hw/ppc/spapr.c                 |   1 +
 hw/s390x/s390-virtio-ccw.c     |   1 +
 hw/tpm/tpm_ppi.c               |   5 +-
 hw/virtio/virtio-mem-pci.c     |  10 +-
 include/hw/boards.h            |  23 +++--
 monitor/monitor.c              |   9 ++
 qapi/machine.json              |   7 +-
 qemu-options.hx                |  24 +++--
 softmmu/memory_mapping.c       |  64 ++++++++-----
 target/i386/cpu.c              |  22 ++++-
 target/i386/cpu.h              |  12 ++-
 target/i386/kvm/hyperv-proto.h |   1 +
 target/i386/kvm/kvm.c          |  62 +++++++++---
 tests/qtest/numa-test.c        |   6 +-
 22 files changed, 387 insertions(+), 203 deletions(-)
-- 
2.31.1



^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PULL 01/30] qapi/machine: Fix an incorrect comment of SMPConfiguration
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 02/30] machine: Deprecate "parameter=0" SMP configurations Paolo Bonzini
                   ` (30 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Yanan Wang, Daniel P . Berrangé

From: Yanan Wang <wangyanan55@huawei.com>

The explanation of @cores should be "number of cores per die" but
not "number of cores per thread". Let's fix it.

Fixes: 1e63fe685804 ("machine: pass QAPI struct to mc->smp_parse")
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20210929025816.21076-2-wangyanan55@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qapi/machine.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qapi/machine.json b/qapi/machine.json
index f1c4983b64..0e91a57a76 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1393,7 +1393,7 @@
 #
 # @dies: number of dies per socket in the CPU topology
 #
-# @cores: number of cores per thread in the CPU topology
+# @cores: number of cores per die in the CPU topology
 #
 # @threads: number of threads per core in the CPU topology
 #
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 02/30] machine: Deprecate "parameter=0" SMP configurations
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
  2021-10-03  7:42 ` [PULL 01/30] qapi/machine: Fix an incorrect comment of SMPConfiguration Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 03/30] machine: Minor refactor/fix for the smp parsers Paolo Bonzini
                   ` (29 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Yanan Wang, Cornelia Huck, Daniel P . Berrangé

From: Yanan Wang <wangyanan55@huawei.com>

In the SMP configuration, we should either provide a topology
parameter with a reasonable value (greater than zero) or just
omit it and QEMU will compute the missing value.

The users shouldn't provide a configuration with any parameter
of it specified as zero (e.g. -smp 8,sockets=0) which could
possibly cause unexpected results in the -smp parsing. So we
deprecate this kind of configurations since 6.2 by adding the
explicit sanity check.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20210929025816.21076-3-wangyanan55@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 docs/about/deprecated.rst | 15 +++++++++++++++
 hw/core/machine.c         | 14 ++++++++++++++
 qemu-options.hx           | 12 +++++++-----
 3 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 2f7db9a98d..0bed6ecb1d 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -160,6 +160,21 @@ Use ``-display sdl`` instead.
 
 Use ``-display curses`` instead.
 
+``-smp`` ("parameter=0" SMP configurations) (since 6.2)
+'''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Specified CPU topology parameters must be greater than zero.
+
+In the SMP configuration, users should either provide a CPU topology
+parameter with a reasonable value (greater than zero) or just omit it
+and QEMU will compute the missing value.
+
+However, historically it was implicitly allowed for users to provide
+a parameter with zero value, which is meaningless and could also possibly
+cause unexpected results in the -smp parsing. So support for this kind of
+configurations (e.g. -smp 8,sockets=0) is deprecated since 6.2 and will
+be removed in the near future, users have to ensure that all the topology
+members described with -smp are greater than zero.
 
 Plugin argument passing through ``arg=<string>`` (since 6.1)
 ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 067f42b528..4e409261c9 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -835,6 +835,20 @@ static void machine_set_smp(Object *obj, Visitor *v, const char *name,
         return;
     }
 
+    /*
+     * Specified CPU topology parameters must be greater than zero,
+     * explicit configuration like "cpus=0" is not allowed.
+     */
+    if ((config->has_cpus && config->cpus == 0) ||
+        (config->has_sockets && config->sockets == 0) ||
+        (config->has_dies && config->dies == 0) ||
+        (config->has_cores && config->cores == 0) ||
+        (config->has_threads && config->threads == 0) ||
+        (config->has_maxcpus && config->maxcpus == 0)) {
+        warn_report("Deprecated CPU topology (considered invalid): "
+                    "CPU topology parameters must be greater than zero");
+    }
+
     mc->smp_parse(ms, config, errp);
     if (*errp) {
         goto out_free;
diff --git a/qemu-options.hx b/qemu-options.hx
index ceca52818a..4a092a092a 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -233,11 +233,13 @@ SRST
     of computing the CPU maximum count.
 
     Either the initial CPU count, or at least one of the topology parameters
-    must be specified. Values for any omitted parameters will be computed
-    from those which are given. Historically preference was given to the
-    coarsest topology parameters when computing missing values (ie sockets
-    preferred over cores, which were preferred over threads), however, this
-    behaviour is considered liable to change.
+    must be specified. The specified parameters must be greater than zero,
+    explicit configuration like "cpus=0" is not allowed. Values for any
+    omitted parameters will be computed from those which are given.
+    Historically preference was given to the coarsest topology parameters
+    when computing missing values (ie sockets preferred over cores, which
+    were preferred over threads), however, this behaviour is considered
+    liable to change.
 ERST
 
 DEF("numa", HAS_ARG, QEMU_OPTION_numa,
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 03/30] machine: Minor refactor/fix for the smp parsers
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
  2021-10-03  7:42 ` [PULL 01/30] qapi/machine: Fix an incorrect comment of SMPConfiguration Paolo Bonzini
  2021-10-03  7:42 ` [PULL 02/30] machine: Deprecate "parameter=0" SMP configurations Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 04/30] machine: Uniformly use maxcpus to calculate the omitted parameters Paolo Bonzini
                   ` (28 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Yanan Wang, Andrew Jones, Daniel P . Berrangé

From: Yanan Wang <wangyanan55@huawei.com>

To pave the way for the functional improvement in later patches,
make some refactor/cleanup for the smp parsers, including using
local maxcpus instead of ms->smp.max_cpus in the calculation,
defaulting dies to 0 initially like other members, cleanup the
sanity check for dies.

We actually also fix a hidden defect by avoiding directly using
the provided *zero value* in the calculation, which could cause
a segment fault (e.g. using dies=0 in the calculation).

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20210929025816.21076-4-wangyanan55@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/machine.c | 18 ++++++++++--------
 hw/i386/pc.c      | 23 ++++++++++++++---------
 2 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 4e409261c9..8e719e2932 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -752,8 +752,9 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
     unsigned sockets = config->has_sockets ? config->sockets : 0;
     unsigned cores   = config->has_cores ? config->cores : 0;
     unsigned threads = config->has_threads ? config->threads : 0;
+    unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0;
 
-    if (config->has_dies && config->dies != 0 && config->dies != 1) {
+    if (config->has_dies && config->dies > 1) {
         error_setg(errp, "dies not supported by this machine's CPU topology");
         return;
     }
@@ -766,8 +767,8 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
             sockets = sockets > 0 ? sockets : 1;
             cpus = cores * threads * sockets;
         } else {
-            ms->smp.max_cpus = config->has_maxcpus ? config->maxcpus : cpus;
-            sockets = ms->smp.max_cpus / (cores * threads);
+            maxcpus = maxcpus > 0 ? maxcpus : cpus;
+            sockets = maxcpus / (cores * threads);
         }
     } else if (cores == 0) {
         threads = threads > 0 ? threads : 1;
@@ -784,26 +785,27 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
         return;
     }
 
-    ms->smp.max_cpus = config->has_maxcpus ? config->maxcpus : cpus;
+    maxcpus = maxcpus > 0 ? maxcpus : cpus;
 
-    if (ms->smp.max_cpus < cpus) {
+    if (maxcpus < cpus) {
         error_setg(errp, "maxcpus must be equal to or greater than smp");
         return;
     }
 
-    if (sockets * cores * threads != ms->smp.max_cpus) {
+    if (sockets * cores * threads != maxcpus) {
         error_setg(errp, "Invalid CPU topology: "
                    "sockets (%u) * cores (%u) * threads (%u) "
                    "!= maxcpus (%u)",
                    sockets, cores, threads,
-                   ms->smp.max_cpus);
+                   maxcpus);
         return;
     }
 
     ms->smp.cpus = cpus;
+    ms->smp.sockets = sockets;
     ms->smp.cores = cores;
     ms->smp.threads = threads;
-    ms->smp.sockets = sockets;
+    ms->smp.max_cpus = maxcpus;
 }
 
 static void machine_get_smp(Object *obj, Visitor *v, const char *name,
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index df457eceba..92c78d9933 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -718,9 +718,13 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
 {
     unsigned cpus    = config->has_cpus ? config->cpus : 0;
     unsigned sockets = config->has_sockets ? config->sockets : 0;
-    unsigned dies    = config->has_dies ? config->dies : 1;
+    unsigned dies    = config->has_dies ? config->dies : 0;
     unsigned cores   = config->has_cores ? config->cores : 0;
     unsigned threads = config->has_threads ? config->threads : 0;
+    unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0;
+
+    /* directly default dies to 1 if it's omitted */
+    dies = dies > 0 ? dies : 1;
 
     /* compute missing values, prefer sockets over cores over threads */
     if (cpus == 0 || sockets == 0) {
@@ -730,8 +734,8 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
             sockets = sockets > 0 ? sockets : 1;
             cpus = cores * threads * dies * sockets;
         } else {
-            ms->smp.max_cpus = config->has_maxcpus ? config->maxcpus : cpus;
-            sockets = ms->smp.max_cpus / (cores * threads * dies);
+            maxcpus = maxcpus > 0 ? maxcpus : cpus;
+            sockets = maxcpus / (dies * cores * threads);
         }
     } else if (cores == 0) {
         threads = threads > 0 ? threads : 1;
@@ -748,27 +752,28 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
         return;
     }
 
-    ms->smp.max_cpus = config->has_maxcpus ? config->maxcpus : cpus;
+    maxcpus = maxcpus > 0 ? maxcpus : cpus;
 
-    if (ms->smp.max_cpus < cpus) {
+    if (maxcpus < cpus) {
         error_setg(errp, "maxcpus must be equal to or greater than smp");
         return;
     }
 
-    if (sockets * dies * cores * threads != ms->smp.max_cpus) {
+    if (sockets * dies * cores * threads != maxcpus) {
         error_setg(errp, "Invalid CPU topology deprecated: "
                    "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
                    "!= maxcpus (%u)",
                    sockets, dies, cores, threads,
-                   ms->smp.max_cpus);
+                   maxcpus);
         return;
     }
 
     ms->smp.cpus = cpus;
-    ms->smp.cores = cores;
-    ms->smp.threads = threads;
     ms->smp.sockets = sockets;
     ms->smp.dies = dies;
+    ms->smp.cores = cores;
+    ms->smp.threads = threads;
+    ms->smp.max_cpus = maxcpus;
 }
 
 static
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 04/30] machine: Uniformly use maxcpus to calculate the omitted parameters
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (2 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 03/30] machine: Minor refactor/fix for the smp parsers Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 05/30] machine: Set the value of cpus to match maxcpus if it's omitted Paolo Bonzini
                   ` (27 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Yanan Wang, Andrew Jones, Daniel P . Berrangé, Pankaj Gupta

From: Yanan Wang <wangyanan55@huawei.com>

We are currently using maxcpus to calculate the omitted sockets
but using cpus to calculate the omitted cores/threads. This makes
cmdlines like:
  -smp cpus=8,maxcpus=16
  -smp cpus=8,cores=4,maxcpus=16
  -smp cpus=8,threads=2,maxcpus=16
work fine but the ones like:
  -smp cpus=8,sockets=2,maxcpus=16
  -smp cpus=8,sockets=2,cores=4,maxcpus=16
  -smp cpus=8,sockets=2,threads=2,maxcpus=16
break the sanity check.

Since we require for a valid config that the product of "sockets * cores
* threads" should equal to the maxcpus, we should uniformly use maxcpus
to calculate their omitted values.

Also the if-branch of "cpus == 0 || sockets == 0" was split into two
branches of "cpus == 0" and "sockets == 0" so that we can clearly read
that we are parsing the configuration with a preference on cpus over
sockets over cores over threads.

Note: change in this patch won't affect any existing working cmdlines
but improves consistency and allows more incomplete configs to be valid.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20210929025816.21076-5-wangyanan55@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/machine.c | 30 +++++++++++++++---------------
 hw/i386/pc.c      | 30 +++++++++++++++---------------
 2 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 8e719e2932..596e758133 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -760,24 +760,26 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
     }
 
     /* compute missing values, prefer sockets over cores over threads */
-    if (cpus == 0 || sockets == 0) {
+    maxcpus = maxcpus > 0 ? maxcpus : cpus;
+
+    if (cpus == 0) {
+        sockets = sockets > 0 ? sockets : 1;
         cores = cores > 0 ? cores : 1;
         threads = threads > 0 ? threads : 1;
-        if (cpus == 0) {
-            sockets = sockets > 0 ? sockets : 1;
-            cpus = cores * threads * sockets;
-        } else {
-            maxcpus = maxcpus > 0 ? maxcpus : cpus;
-            sockets = maxcpus / (cores * threads);
-        }
+        cpus = sockets * cores * threads;
+        maxcpus = maxcpus > 0 ? maxcpus : cpus;
+    } else if (sockets == 0) {
+        cores = cores > 0 ? cores : 1;
+        threads = threads > 0 ? threads : 1;
+        sockets = maxcpus / (cores * threads);
     } else if (cores == 0) {
         threads = threads > 0 ? threads : 1;
-        cores = cpus / (sockets * threads);
-        cores = cores > 0 ? cores : 1;
+        cores = maxcpus / (sockets * threads);
     } else if (threads == 0) {
-        threads = cpus / (cores * sockets);
-        threads = threads > 0 ? threads : 1;
-    } else if (sockets * cores * threads < cpus) {
+        threads = maxcpus / (sockets * cores);
+    }
+
+    if (sockets * cores * threads < cpus) {
         error_setg(errp, "cpu topology: "
                    "sockets (%u) * cores (%u) * threads (%u) < "
                    "smp_cpus (%u)",
@@ -785,8 +787,6 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
         return;
     }
 
-    maxcpus = maxcpus > 0 ? maxcpus : cpus;
-
     if (maxcpus < cpus) {
         error_setg(errp, "maxcpus must be equal to or greater than smp");
         return;
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 92c78d9933..e37e84cc7b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -727,24 +727,26 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
     dies = dies > 0 ? dies : 1;
 
     /* compute missing values, prefer sockets over cores over threads */
-    if (cpus == 0 || sockets == 0) {
+    maxcpus = maxcpus > 0 ? maxcpus : cpus;
+
+    if (cpus == 0) {
+        sockets = sockets > 0 ? sockets : 1;
         cores = cores > 0 ? cores : 1;
         threads = threads > 0 ? threads : 1;
-        if (cpus == 0) {
-            sockets = sockets > 0 ? sockets : 1;
-            cpus = cores * threads * dies * sockets;
-        } else {
-            maxcpus = maxcpus > 0 ? maxcpus : cpus;
-            sockets = maxcpus / (dies * cores * threads);
-        }
+        cpus = sockets * dies * cores * threads;
+        maxcpus = maxcpus > 0 ? maxcpus : cpus;
+    } else if (sockets == 0) {
+        cores = cores > 0 ? cores : 1;
+        threads = threads > 0 ? threads : 1;
+        sockets = maxcpus / (dies * cores * threads);
     } else if (cores == 0) {
         threads = threads > 0 ? threads : 1;
-        cores = cpus / (sockets * dies * threads);
-        cores = cores > 0 ? cores : 1;
+        cores = maxcpus / (sockets * dies * threads);
     } else if (threads == 0) {
-        threads = cpus / (cores * dies * sockets);
-        threads = threads > 0 ? threads : 1;
-    } else if (sockets * dies * cores * threads < cpus) {
+        threads = maxcpus / (sockets * dies * cores);
+    }
+
+    if (sockets * dies * cores * threads < cpus) {
         error_setg(errp, "cpu topology: "
                    "sockets (%u) * dies (%u) * cores (%u) * threads (%u) < "
                    "smp_cpus (%u)",
@@ -752,8 +754,6 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
         return;
     }
 
-    maxcpus = maxcpus > 0 ? maxcpus : cpus;
-
     if (maxcpus < cpus) {
         error_setg(errp, "maxcpus must be equal to or greater than smp");
         return;
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 05/30] machine: Set the value of cpus to match maxcpus if it's omitted
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (3 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 04/30] machine: Uniformly use maxcpus to calculate the omitted parameters Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 06/30] machine: Improve the error reporting of smp parsing Paolo Bonzini
                   ` (26 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Yanan Wang, Andrew Jones, Daniel P . Berrangé

From: Yanan Wang <wangyanan55@huawei.com>

Currently we directly calculate the omitted cpus based on the given
incomplete collection of parameters. This makes some cmdlines like:
  -smp maxcpus=16
  -smp sockets=2,maxcpus=16
  -smp sockets=2,dies=2,maxcpus=16
  -smp sockets=2,cores=4,maxcpus=16
not work. We should probably set the value of cpus to match maxcpus
if it's omitted, which will make above configs start to work.

So the calculation logic of cpus/maxcpus after this patch will be:
When both maxcpus and cpus are omitted, maxcpus will be calculated
from the given parameters and cpus will be set equal to maxcpus.
When only one of maxcpus and cpus is given then the omitted one
will be set to its counterpart's value. Both maxcpus and cpus may
be specified, but maxcpus must be equal to or greater than cpus.

Note: change in this patch won't affect any existing working cmdlines
but allows more incomplete configs to be valid.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20210929025816.21076-6-wangyanan55@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/machine.c | 29 ++++++++++++++++-------------
 hw/i386/pc.c      | 29 ++++++++++++++++-------------
 qemu-options.hx   | 11 ++++++++---
 3 files changed, 40 insertions(+), 29 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 596e758133..d8f458db60 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -760,25 +760,28 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
     }
 
     /* compute missing values, prefer sockets over cores over threads */
-    maxcpus = maxcpus > 0 ? maxcpus : cpus;
-
-    if (cpus == 0) {
+    if (cpus == 0 && maxcpus == 0) {
         sockets = sockets > 0 ? sockets : 1;
         cores = cores > 0 ? cores : 1;
         threads = threads > 0 ? threads : 1;
-        cpus = sockets * cores * threads;
+    } else {
         maxcpus = maxcpus > 0 ? maxcpus : cpus;
-    } else if (sockets == 0) {
-        cores = cores > 0 ? cores : 1;
-        threads = threads > 0 ? threads : 1;
-        sockets = maxcpus / (cores * threads);
-    } else if (cores == 0) {
-        threads = threads > 0 ? threads : 1;
-        cores = maxcpus / (sockets * threads);
-    } else if (threads == 0) {
-        threads = maxcpus / (sockets * cores);
+
+        if (sockets == 0) {
+            cores = cores > 0 ? cores : 1;
+            threads = threads > 0 ? threads : 1;
+            sockets = maxcpus / (cores * threads);
+        } else if (cores == 0) {
+            threads = threads > 0 ? threads : 1;
+            cores = maxcpus / (sockets * threads);
+        } else if (threads == 0) {
+            threads = maxcpus / (sockets * cores);
+        }
     }
 
+    maxcpus = maxcpus > 0 ? maxcpus : sockets * cores * threads;
+    cpus = cpus > 0 ? cpus : maxcpus;
+
     if (sockets * cores * threads < cpus) {
         error_setg(errp, "cpu topology: "
                    "sockets (%u) * cores (%u) * threads (%u) < "
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e37e84cc7b..f24a1d72ad 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -727,25 +727,28 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
     dies = dies > 0 ? dies : 1;
 
     /* compute missing values, prefer sockets over cores over threads */
-    maxcpus = maxcpus > 0 ? maxcpus : cpus;
-
-    if (cpus == 0) {
+    if (cpus == 0 && maxcpus == 0) {
         sockets = sockets > 0 ? sockets : 1;
         cores = cores > 0 ? cores : 1;
         threads = threads > 0 ? threads : 1;
-        cpus = sockets * dies * cores * threads;
+    } else {
         maxcpus = maxcpus > 0 ? maxcpus : cpus;
-    } else if (sockets == 0) {
-        cores = cores > 0 ? cores : 1;
-        threads = threads > 0 ? threads : 1;
-        sockets = maxcpus / (dies * cores * threads);
-    } else if (cores == 0) {
-        threads = threads > 0 ? threads : 1;
-        cores = maxcpus / (sockets * dies * threads);
-    } else if (threads == 0) {
-        threads = maxcpus / (sockets * dies * cores);
+
+        if (sockets == 0) {
+            cores = cores > 0 ? cores : 1;
+            threads = threads > 0 ? threads : 1;
+            sockets = maxcpus / (dies * cores * threads);
+        } else if (cores == 0) {
+            threads = threads > 0 ? threads : 1;
+            cores = maxcpus / (sockets * dies * threads);
+        } else if (threads == 0) {
+            threads = maxcpus / (sockets * dies * cores);
+        }
     }
 
+    maxcpus = maxcpus > 0 ? maxcpus : sockets * dies * cores * threads;
+    cpus = cpus > 0 ? cpus : maxcpus;
+
     if (sockets * dies * cores * threads < cpus) {
         error_setg(errp, "cpu topology: "
                    "sockets (%u) * dies (%u) * cores (%u) * threads (%u) < "
diff --git a/qemu-options.hx b/qemu-options.hx
index 4a092a092a..2eac830fdb 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -220,9 +220,14 @@ SRST
     Simulate a SMP system with '\ ``n``\ ' CPUs initially present on
     the machine type board. On boards supporting CPU hotplug, the optional
     '\ ``maxcpus``\ ' parameter can be set to enable further CPUs to be
-    added at runtime. If omitted the maximum number of CPUs will be
-    set to match the initial CPU count. Both parameters are subject to
-    an upper limit that is determined by the specific machine type chosen.
+    added at runtime. When both parameters are omitted, the maximum number
+    of CPUs will be calculated from the provided topology members and the
+    initial CPU count will match the maximum number. When only one of them
+    is given then the omitted one will be set to its counterpart's value.
+    Both parameters may be specified, but the maximum number of CPUs must
+    be equal to or greater than the initial CPU count. Both parameters are
+    subject to an upper limit that is determined by the specific machine
+    type chosen.
 
     To control reporting of CPU topology information, the number of sockets,
     dies per socket, cores per die, and threads per core can be specified.
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 06/30] machine: Improve the error reporting of smp parsing
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (4 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 05/30] machine: Set the value of cpus to match maxcpus if it's omitted Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 07/30] qtest/numa-test: Use detailed -smp CLIs in pc_dynamic_cpu_cfg Paolo Bonzini
                   ` (25 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Yanan Wang, Philippe Mathieu-Daudé,
	Andrew Jones, Daniel P . Berrangé,
	Pankaj Gupta

From: Yanan Wang <wangyanan55@huawei.com>

We have two requirements for a valid SMP configuration:
the product of "sockets * cores * threads" must represent all the
possible cpus, i.e., max_cpus, and then must include the initially
present cpus, i.e., smp_cpus.

So we only need to ensure 1) "sockets * cores * threads == maxcpus"
at first and then ensure 2) "maxcpus >= cpus". With a reasonable
order of the sanity check, we can simplify the error reporting code.
When reporting an error message we also report the exact value of
each topology member to make users easily see what's going on.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210929025816.21076-7-wangyanan55@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/machine.c | 22 +++++++++-------------
 hw/i386/pc.c      | 24 ++++++++++--------------
 2 files changed, 19 insertions(+), 27 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index d8f458db60..e38ab760e6 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -782,25 +782,21 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
     maxcpus = maxcpus > 0 ? maxcpus : sockets * cores * threads;
     cpus = cpus > 0 ? cpus : maxcpus;
 
-    if (sockets * cores * threads < cpus) {
-        error_setg(errp, "cpu topology: "
-                   "sockets (%u) * cores (%u) * threads (%u) < "
-                   "smp_cpus (%u)",
-                   sockets, cores, threads, cpus);
+    if (sockets * cores * threads != maxcpus) {
+        error_setg(errp, "Invalid CPU topology: "
+                   "product of the hierarchy must match maxcpus: "
+                   "sockets (%u) * cores (%u) * threads (%u) "
+                   "!= maxcpus (%u)",
+                   sockets, cores, threads, maxcpus);
         return;
     }
 
     if (maxcpus < cpus) {
-        error_setg(errp, "maxcpus must be equal to or greater than smp");
-        return;
-    }
-
-    if (sockets * cores * threads != maxcpus) {
         error_setg(errp, "Invalid CPU topology: "
+                   "maxcpus must be equal to or greater than smp: "
                    "sockets (%u) * cores (%u) * threads (%u) "
-                   "!= maxcpus (%u)",
-                   sockets, cores, threads,
-                   maxcpus);
+                   "== maxcpus (%u) < smp_cpus (%u)",
+                   sockets, cores, threads, maxcpus, cpus);
         return;
     }
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f24a1d72ad..9216ad163d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -749,25 +749,21 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
     maxcpus = maxcpus > 0 ? maxcpus : sockets * dies * cores * threads;
     cpus = cpus > 0 ? cpus : maxcpus;
 
-    if (sockets * dies * cores * threads < cpus) {
-        error_setg(errp, "cpu topology: "
-                   "sockets (%u) * dies (%u) * cores (%u) * threads (%u) < "
-                   "smp_cpus (%u)",
-                   sockets, dies, cores, threads, cpus);
+    if (sockets * dies * cores * threads != maxcpus) {
+        error_setg(errp, "Invalid CPU topology: "
+                   "product of the hierarchy must match maxcpus: "
+                   "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
+                   "!= maxcpus (%u)",
+                   sockets, dies, cores, threads, maxcpus);
         return;
     }
 
     if (maxcpus < cpus) {
-        error_setg(errp, "maxcpus must be equal to or greater than smp");
-        return;
-    }
-
-    if (sockets * dies * cores * threads != maxcpus) {
-        error_setg(errp, "Invalid CPU topology deprecated: "
+        error_setg(errp, "Invalid CPU topology: "
+                   "maxcpus must be equal to or greater than smp: "
                    "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
-                   "!= maxcpus (%u)",
-                   sockets, dies, cores, threads,
-                   maxcpus);
+                   "== maxcpus (%u) < smp_cpus (%u)",
+                   sockets, dies, cores, threads, maxcpus, cpus);
         return;
     }
 
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 07/30] qtest/numa-test: Use detailed -smp CLIs in pc_dynamic_cpu_cfg
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (5 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 06/30] machine: Improve the error reporting of smp parsing Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 08/30] qtest/numa-test: Use detailed -smp CLIs in test_def_cpu_split Paolo Bonzini
                   ` (24 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Yanan Wang, Philippe Mathieu-Daudé,
	Andrew Jones, Daniel P . Berrangé

From: Yanan Wang <wangyanan55@huawei.com>

Since commit 80d7835749 (qemu-options: rewrite help for -smp options),
the preference of sockets/cores in -smp parsing is considered liable
to change, and actually we are going to change it in a coming commit.
So it'll be more stable to use detailed -smp CLIs in testing if we
have strong dependency on the parsing results.

pc_dynamic_cpu_cfg currently assumes/needs that there will be 2 CPU
sockets with "-smp 2". To avoid breaking the test because of parsing
logic change, now explicitly use "-smp 2,sockets=2".

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210929025816.21076-8-wangyanan55@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qtest/numa-test.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/numa-test.c b/tests/qtest/numa-test.c
index c677cd63c4..fd7a2e80a0 100644
--- a/tests/qtest/numa-test.c
+++ b/tests/qtest/numa-test.c
@@ -265,7 +265,8 @@ static void pc_dynamic_cpu_cfg(const void *data)
     QTestState *qs;
     g_autofree char *cli = NULL;
 
-    cli = make_cli(data, "-nodefaults --preconfig -machine smp.cpus=2");
+    cli = make_cli(data, "-nodefaults --preconfig "
+                         "-machine smp.cpus=2,smp.sockets=2");
     qs = qtest_init(cli);
 
     /* create 2 numa nodes */
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 08/30] qtest/numa-test: Use detailed -smp CLIs in test_def_cpu_split
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (6 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 07/30] qtest/numa-test: Use detailed -smp CLIs in pc_dynamic_cpu_cfg Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 09/30] machine: Prefer cores over sockets in smp parsing since 6.2 Paolo Bonzini
                   ` (23 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Yanan Wang, Philippe Mathieu-Daudé,
	Andrew Jones, Daniel P . Berrangé

From: Yanan Wang <wangyanan55@huawei.com>

Since commit 80d7835749 (qemu-options: rewrite help for -smp options),
the preference of sockets/cores in -smp parsing is considered liable
to change, and actually we are going to change it in a coming commit.
So it'll be more stable to use detailed -smp CLIs in the testcases
that have strong dependency on the parsing results.

Currently, test_def_cpu_split use "-smp 8" and will get 8 CPU sockets
based on current parsing rule. But if we change to prefer cores over
sockets we will get one CPU socket with 8 cores, and this testcase
will not get expected numa set by default on x86_64 (Ok on aarch64).

So now explicitly use "-smp 8,sockets=8" to avoid affect from parsing
logic change.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210929025816.21076-9-wangyanan55@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qtest/numa-test.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/numa-test.c b/tests/qtest/numa-test.c
index fd7a2e80a0..90bf68a5b3 100644
--- a/tests/qtest/numa-test.c
+++ b/tests/qtest/numa-test.c
@@ -42,7 +42,8 @@ static void test_def_cpu_split(const void *data)
     g_autofree char *s = NULL;
     g_autofree char *cli = NULL;
 
-    cli = make_cli(data, "-machine smp.cpus=8 -numa node,memdev=ram -numa node");
+    cli = make_cli(data, "-machine smp.cpus=8,smp.sockets=8 "
+                         "-numa node,memdev=ram -numa node");
     qts = qtest_init(cli);
 
     s = qtest_hmp(qts, "info numa");
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 09/30] machine: Prefer cores over sockets in smp parsing since 6.2
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (7 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 08/30] qtest/numa-test: Use detailed -smp CLIs in test_def_cpu_split Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 10/30] machine: Use ms instead of global current_machine in sanity-check Paolo Bonzini
                   ` (22 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Andrew Jones, Daniel P . Berrange, Cornelia Huck, Pankaj Gupta,
	Yanan Wang, David Gibson

From: Yanan Wang <wangyanan55@huawei.com>

In the real SMP hardware topology world, it's much more likely that
we have high cores-per-socket counts and few sockets totally. While
the current preference of sockets over cores in smp parsing results
in a virtual cpu topology with low cores-per-sockets counts and a
large number of sockets, which is just contrary to the real world.

Given that it is better to make the virtual cpu topology be more
reflective of the real world and also for the sake of compatibility,
we start to prefer cores over sockets over threads in smp parsing
since machine type 6.2 for different arches.

In this patch, a boolean "smp_prefer_sockets" is added, and we only
enable the old preference on older machines and enable the new one
since type 6.2 for all arches by using the machine compat mechanism.

Suggested-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Acked-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20210929025816.21076-10-wangyanan55@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/arm/virt.c              |  1 +
 hw/core/machine.c          | 35 ++++++++++++++++++++++++++---------
 hw/i386/pc.c               | 35 ++++++++++++++++++++++++++---------
 hw/i386/pc_piix.c          |  1 +
 hw/i386/pc_q35.c           |  1 +
 hw/ppc/spapr.c             |  1 +
 hw/s390x/s390-virtio-ccw.c |  1 +
 include/hw/boards.h        |  1 +
 qemu-options.hx            |  3 ++-
 9 files changed, 60 insertions(+), 19 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 1d59f0e59f..8c13deb5db 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2815,6 +2815,7 @@ static void virt_machine_6_1_options(MachineClass *mc)
 
     virt_machine_6_2_options(mc);
     compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
+    mc->smp_prefer_sockets = true;
 
     /* qemu ITS was introduced with 6.2 */
     vmc->no_tcg_its = true;
diff --git a/hw/core/machine.c b/hw/core/machine.c
index e38ab760e6..f2a34d98c0 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -748,6 +748,7 @@ void machine_set_cpu_numa_node(MachineState *machine,
 
 static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
 {
+    MachineClass *mc = MACHINE_GET_CLASS(ms);
     unsigned cpus    = config->has_cpus ? config->cpus : 0;
     unsigned sockets = config->has_sockets ? config->sockets : 0;
     unsigned cores   = config->has_cores ? config->cores : 0;
@@ -759,7 +760,7 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
         return;
     }
 
-    /* compute missing values, prefer sockets over cores over threads */
+    /* compute missing values based on the provided ones */
     if (cpus == 0 && maxcpus == 0) {
         sockets = sockets > 0 ? sockets : 1;
         cores = cores > 0 ? cores : 1;
@@ -767,14 +768,30 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
     } else {
         maxcpus = maxcpus > 0 ? maxcpus : cpus;
 
-        if (sockets == 0) {
-            cores = cores > 0 ? cores : 1;
-            threads = threads > 0 ? threads : 1;
-            sockets = maxcpus / (cores * threads);
-        } else if (cores == 0) {
-            threads = threads > 0 ? threads : 1;
-            cores = maxcpus / (sockets * threads);
-        } else if (threads == 0) {
+        if (mc->smp_prefer_sockets) {
+            /* prefer sockets over cores before 6.2 */
+            if (sockets == 0) {
+                cores = cores > 0 ? cores : 1;
+                threads = threads > 0 ? threads : 1;
+                sockets = maxcpus / (cores * threads);
+            } else if (cores == 0) {
+                threads = threads > 0 ? threads : 1;
+                cores = maxcpus / (sockets * threads);
+            }
+        } else {
+            /* prefer cores over sockets since 6.2 */
+            if (cores == 0) {
+                sockets = sockets > 0 ? sockets : 1;
+                threads = threads > 0 ? threads : 1;
+                cores = maxcpus / (sockets * threads);
+            } else if (sockets == 0) {
+                threads = threads > 0 ? threads : 1;
+                sockets = maxcpus / (cores * threads);
+            }
+        }
+
+        /* try to calculate omitted threads at last */
+        if (threads == 0) {
             threads = maxcpus / (sockets * cores);
         }
     }
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 9216ad163d..6cc32f4048 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -716,6 +716,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
  */
 static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
 {
+    MachineClass *mc = MACHINE_GET_CLASS(ms);
     unsigned cpus    = config->has_cpus ? config->cpus : 0;
     unsigned sockets = config->has_sockets ? config->sockets : 0;
     unsigned dies    = config->has_dies ? config->dies : 0;
@@ -726,7 +727,7 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
     /* directly default dies to 1 if it's omitted */
     dies = dies > 0 ? dies : 1;
 
-    /* compute missing values, prefer sockets over cores over threads */
+    /* compute missing values based on the provided ones */
     if (cpus == 0 && maxcpus == 0) {
         sockets = sockets > 0 ? sockets : 1;
         cores = cores > 0 ? cores : 1;
@@ -734,14 +735,30 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
     } else {
         maxcpus = maxcpus > 0 ? maxcpus : cpus;
 
-        if (sockets == 0) {
-            cores = cores > 0 ? cores : 1;
-            threads = threads > 0 ? threads : 1;
-            sockets = maxcpus / (dies * cores * threads);
-        } else if (cores == 0) {
-            threads = threads > 0 ? threads : 1;
-            cores = maxcpus / (sockets * dies * threads);
-        } else if (threads == 0) {
+        if (mc->smp_prefer_sockets) {
+            /* prefer sockets over cores before 6.2 */
+            if (sockets == 0) {
+                cores = cores > 0 ? cores : 1;
+                threads = threads > 0 ? threads : 1;
+                sockets = maxcpus / (dies * cores * threads);
+            } else if (cores == 0) {
+                threads = threads > 0 ? threads : 1;
+                cores = maxcpus / (sockets * dies * threads);
+            }
+        } else {
+            /* prefer cores over sockets since 6.2 */
+            if (cores == 0) {
+                sockets = sockets > 0 ? sockets : 1;
+                threads = threads > 0 ? threads : 1;
+                cores = maxcpus / (sockets * dies * threads);
+            } else if (sockets == 0) {
+                threads = threads > 0 ? threads : 1;
+                sockets = maxcpus / (dies * cores * threads);
+            }
+        }
+
+        /* try to calculate omitted threads at last */
+        if (threads == 0) {
             threads = maxcpus / (sockets * dies * cores);
         }
     }
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 6cc834aff6..932e6e2cfe 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -432,6 +432,7 @@ static void pc_i440fx_6_1_machine_options(MachineClass *m)
     m->is_default = false;
     compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
     compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
+    m->smp_prefer_sockets = true;
 }
 
 DEFINE_I440FX_MACHINE(v6_1, "pc-i440fx-6.1", NULL,
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 5481d5c965..57ab07e5b7 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -372,6 +372,7 @@ static void pc_q35_6_1_machine_options(MachineClass *m)
     m->alias = NULL;
     compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
     compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
+    m->smp_prefer_sockets = true;
 }
 
 DEFINE_Q35_MACHINE(v6_1, "pc-q35-6.1", NULL,
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index b7bee5f4ff..2986108b5a 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4685,6 +4685,7 @@ static void spapr_machine_6_1_class_options(MachineClass *mc)
     spapr_machine_6_2_class_options(mc);
     compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
     smc->pre_6_2_numa_affinity = true;
+    mc->smp_prefer_sockets = true;
 }
 
 DEFINE_SPAPR_MACHINE(6_1, "6.1", false);
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 61aeccb163..5401c985cf 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -814,6 +814,7 @@ static void ccw_machine_6_1_class_options(MachineClass *mc)
 {
     ccw_machine_6_2_class_options(mc);
     compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
+    mc->smp_prefer_sockets = true;
 }
 DEFINE_CCW_MACHINE(6_1, "6.1", false);
 
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 463a5514f9..2ae039b74f 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -247,6 +247,7 @@ struct MachineClass {
     bool nvdimm_supported;
     bool numa_mem_supported;
     bool auto_enable_numa;
+    bool smp_prefer_sockets;
     const char *default_ram_id;
 
     HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
diff --git a/qemu-options.hx b/qemu-options.hx
index 2eac830fdb..8ef178180d 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -244,7 +244,8 @@ SRST
     Historically preference was given to the coarsest topology parameters
     when computing missing values (ie sockets preferred over cores, which
     were preferred over threads), however, this behaviour is considered
-    liable to change.
+    liable to change. Prior to 6.2 the preference was sockets over cores
+    over threads. Since 6.2 the preference is cores over sockets over threads.
 ERST
 
 DEF("numa", HAS_ARG, QEMU_OPTION_numa,
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 10/30] machine: Use ms instead of global current_machine in sanity-check
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (8 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 09/30] machine: Prefer cores over sockets in smp parsing since 6.2 Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 11/30] machine: Tweak the order of topology members in struct CpuTopology Paolo Bonzini
                   ` (21 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Andrew Jones, Daniel P . Berrangé,
	Cornelia Huck, Pankaj Gupta, Yanan Wang,
	Philippe Mathieu-Daudé

From: Yanan Wang <wangyanan55@huawei.com>

In the sanity-check of smp_cpus and max_cpus against mc in function
machine_set_smp(), we are now using ms->smp.max_cpus for the check
but using current_machine->smp.max_cpus in the error message.
Tweak this by uniformly using the local ms.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210929025816.21076-11-wangyanan55@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/machine.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index f2a34d98c0..12d7416053 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -881,7 +881,7 @@ static void machine_set_smp(Object *obj, Visitor *v, const char *name,
     } else if (ms->smp.max_cpus > mc->max_cpus) {
         error_setg(errp, "Invalid SMP CPUs %d. The max CPUs "
                    "supported by machine '%s' is %d",
-                   current_machine->smp.max_cpus,
+                   ms->smp.max_cpus,
                    mc->name, mc->max_cpus);
     }
 
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 11/30] machine: Tweak the order of topology members in struct CpuTopology
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (9 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 10/30] machine: Use ms instead of global current_machine in sanity-check Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 12/30] machine: Make smp_parse generic enough for all arches Paolo Bonzini
                   ` (20 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Yanan Wang, Philippe Mathieu-Daudé,
	Andrew Jones, Daniel P . Berrangé,
	Pankaj Gupta

From: Yanan Wang <wangyanan55@huawei.com>

Now that all the possible topology parameters are integrated in struct
CpuTopology, tweak the order of topology members to be "cpus/sockets/
dies/cores/threads/maxcpus" for readability and consistency. We also
tweak the comment by adding explanation of dies parameter.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210929025816.21076-12-wangyanan55@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/machine.c   | 10 +++++-----
 include/hw/boards.h |  7 ++++---
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 12d7416053..83cbdcce47 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -829,11 +829,11 @@ static void machine_get_smp(Object *obj, Visitor *v, const char *name,
 {
     MachineState *ms = MACHINE(obj);
     SMPConfiguration *config = &(SMPConfiguration){
-        .has_cores = true, .cores = ms->smp.cores,
+        .has_cpus = true, .cpus = ms->smp.cpus,
         .has_sockets = true, .sockets = ms->smp.sockets,
         .has_dies = true, .dies = ms->smp.dies,
+        .has_cores = true, .cores = ms->smp.cores,
         .has_threads = true, .threads = ms->smp.threads,
-        .has_cpus = true, .cpus = ms->smp.cpus,
         .has_maxcpus = true, .maxcpus = ms->smp.max_cpus,
     };
     if (!visit_type_SMPConfiguration(v, name, &config, &error_abort)) {
@@ -1060,10 +1060,10 @@ static void machine_initfn(Object *obj)
     /* default to mc->default_cpus */
     ms->smp.cpus = mc->default_cpus;
     ms->smp.max_cpus = mc->default_cpus;
-    ms->smp.cores = 1;
-    ms->smp.dies = 1;
-    ms->smp.threads = 1;
     ms->smp.sockets = 1;
+    ms->smp.dies = 1;
+    ms->smp.cores = 1;
+    ms->smp.threads = 1;
 }
 
 static void machine_finalize(Object *obj)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 2ae039b74f..2a1bba86c0 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -275,17 +275,18 @@ typedef struct DeviceMemoryState {
 /**
  * CpuTopology:
  * @cpus: the number of present logical processors on the machine
- * @cores: the number of cores in one package
- * @threads: the number of threads in one core
  * @sockets: the number of sockets on the machine
+ * @dies: the number of dies in one socket
+ * @cores: the number of cores in one die
+ * @threads: the number of threads in one core
  * @max_cpus: the maximum number of logical processors on the machine
  */
 typedef struct CpuTopology {
     unsigned int cpus;
+    unsigned int sockets;
     unsigned int dies;
     unsigned int cores;
     unsigned int threads;
-    unsigned int sockets;
     unsigned int max_cpus;
 } CpuTopology;
 
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 12/30] machine: Make smp_parse generic enough for all arches
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (10 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 11/30] machine: Tweak the order of topology members in struct CpuTopology Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 13/30] machine: Remove smp_parse callback from MachineClass Paolo Bonzini
                   ` (19 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Yanan Wang, Andrew Jones, Daniel P . Berrangé

From: Yanan Wang <wangyanan55@huawei.com>

Currently the only difference between smp_parse and pc_smp_parse
is the support of dies parameter and the related error reporting.
With some arch compat variables like "bool dies_supported", we can
make smp_parse generic enough for all arches and the PC specific
one can be removed.

Making smp_parse() generic enough can reduce code duplication and
ease the code maintenance, and also allows extending the topology
with more arch specific members (e.g., clusters) in the future.

Suggested-by: Andrew Jones <drjones@redhat.com>
Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20210929025816.21076-13-wangyanan55@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/machine.c   | 91 +++++++++++++++++++++++++++++++++++----------
 hw/i386/pc.c        | 84 +----------------------------------------
 include/hw/boards.h |  9 +++++
 3 files changed, 81 insertions(+), 103 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 83cbdcce47..12872d7715 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -746,20 +746,69 @@ void machine_set_cpu_numa_node(MachineState *machine,
     }
 }
 
+/*
+ * Report information of a machine's supported CPU topology hierarchy.
+ * Topology members will be ordered from the largest to the smallest
+ * in the string.
+ */
+static char *cpu_hierarchy_to_string(MachineState *ms)
+{
+    MachineClass *mc = MACHINE_GET_CLASS(ms);
+    GString *s = g_string_new(NULL);
+
+    g_string_append_printf(s, "sockets (%u)", ms->smp.sockets);
+
+    if (mc->smp_props.dies_supported) {
+        g_string_append_printf(s, " * dies (%u)", ms->smp.dies);
+    }
+
+    g_string_append_printf(s, " * cores (%u)", ms->smp.cores);
+    g_string_append_printf(s, " * threads (%u)", ms->smp.threads);
+
+    return g_string_free(s, false);
+}
+
+/*
+ * smp_parse - Generic function used to parse the given SMP configuration
+ *
+ * Any missing parameter in "cpus/maxcpus/sockets/cores/threads" will be
+ * automatically computed based on the provided ones.
+ *
+ * In the calculation of omitted sockets/cores/threads: we prefer sockets
+ * over cores over threads before 6.2, while preferring cores over sockets
+ * over threads since 6.2.
+ *
+ * In the calculation of cpus/maxcpus: When both maxcpus and cpus are omitted,
+ * maxcpus will be computed from the given parameters and cpus will be set
+ * equal to maxcpus. When only one of maxcpus and cpus is given then the
+ * omitted one will be set to its given counterpart's value. Both maxcpus and
+ * cpus may be specified, but maxcpus must be equal to or greater than cpus.
+ *
+ * For compatibility, apart from the parameters that will be computed, newly
+ * introduced topology members which are likely to be target specific should
+ * be directly set as 1 if they are omitted (e.g. dies for PC since 4.1).
+ */
 static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
 {
     MachineClass *mc = MACHINE_GET_CLASS(ms);
     unsigned cpus    = config->has_cpus ? config->cpus : 0;
     unsigned sockets = config->has_sockets ? config->sockets : 0;
+    unsigned dies    = config->has_dies ? config->dies : 0;
     unsigned cores   = config->has_cores ? config->cores : 0;
     unsigned threads = config->has_threads ? config->threads : 0;
     unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0;
 
-    if (config->has_dies && config->dies > 1) {
+    /*
+     * If not supported by the machine, a topology parameter must be
+     * omitted or specified equal to 1.
+     */
+    if (!mc->smp_props.dies_supported && dies > 1) {
         error_setg(errp, "dies not supported by this machine's CPU topology");
         return;
     }
 
+    dies = dies > 0 ? dies : 1;
+
     /* compute missing values based on the provided ones */
     if (cpus == 0 && maxcpus == 0) {
         sockets = sockets > 0 ? sockets : 1;
@@ -773,55 +822,57 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
             if (sockets == 0) {
                 cores = cores > 0 ? cores : 1;
                 threads = threads > 0 ? threads : 1;
-                sockets = maxcpus / (cores * threads);
+                sockets = maxcpus / (dies * cores * threads);
             } else if (cores == 0) {
                 threads = threads > 0 ? threads : 1;
-                cores = maxcpus / (sockets * threads);
+                cores = maxcpus / (sockets * dies * threads);
             }
         } else {
             /* prefer cores over sockets since 6.2 */
             if (cores == 0) {
                 sockets = sockets > 0 ? sockets : 1;
                 threads = threads > 0 ? threads : 1;
-                cores = maxcpus / (sockets * threads);
+                cores = maxcpus / (sockets * dies * threads);
             } else if (sockets == 0) {
                 threads = threads > 0 ? threads : 1;
-                sockets = maxcpus / (cores * threads);
+                sockets = maxcpus / (dies * cores * threads);
             }
         }
 
         /* try to calculate omitted threads at last */
         if (threads == 0) {
-            threads = maxcpus / (sockets * cores);
+            threads = maxcpus / (sockets * dies * cores);
         }
     }
 
-    maxcpus = maxcpus > 0 ? maxcpus : sockets * cores * threads;
+    maxcpus = maxcpus > 0 ? maxcpus : sockets * dies * cores * threads;
     cpus = cpus > 0 ? cpus : maxcpus;
 
-    if (sockets * cores * threads != maxcpus) {
+    ms->smp.cpus = cpus;
+    ms->smp.sockets = sockets;
+    ms->smp.dies = dies;
+    ms->smp.cores = cores;
+    ms->smp.threads = threads;
+    ms->smp.max_cpus = maxcpus;
+
+    /* sanity-check of the computed topology */
+    if (sockets * dies * cores * threads != maxcpus) {
+        g_autofree char *topo_msg = cpu_hierarchy_to_string(ms);
         error_setg(errp, "Invalid CPU topology: "
                    "product of the hierarchy must match maxcpus: "
-                   "sockets (%u) * cores (%u) * threads (%u) "
-                   "!= maxcpus (%u)",
-                   sockets, cores, threads, maxcpus);
+                   "%s != maxcpus (%u)",
+                   topo_msg, maxcpus);
         return;
     }
 
     if (maxcpus < cpus) {
+        g_autofree char *topo_msg = cpu_hierarchy_to_string(ms);
         error_setg(errp, "Invalid CPU topology: "
                    "maxcpus must be equal to or greater than smp: "
-                   "sockets (%u) * cores (%u) * threads (%u) "
-                   "== maxcpus (%u) < smp_cpus (%u)",
-                   sockets, cores, threads, maxcpus, cpus);
+                   "%s == maxcpus (%u) < smp_cpus (%u)",
+                   topo_msg, maxcpus, cpus);
         return;
     }
-
-    ms->smp.cpus = cpus;
-    ms->smp.sockets = sockets;
-    ms->smp.cores = cores;
-    ms->smp.threads = threads;
-    ms->smp.max_cpus = maxcpus;
 }
 
 static void machine_get_smp(Object *obj, Visitor *v, const char *name,
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 6cc32f4048..28e1b83b9d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -710,88 +710,6 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
     }
 }
 
-/*
- * This function is very similar to smp_parse()
- * in hw/core/machine.c but includes CPU die support.
- */
-static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
-{
-    MachineClass *mc = MACHINE_GET_CLASS(ms);
-    unsigned cpus    = config->has_cpus ? config->cpus : 0;
-    unsigned sockets = config->has_sockets ? config->sockets : 0;
-    unsigned dies    = config->has_dies ? config->dies : 0;
-    unsigned cores   = config->has_cores ? config->cores : 0;
-    unsigned threads = config->has_threads ? config->threads : 0;
-    unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0;
-
-    /* directly default dies to 1 if it's omitted */
-    dies = dies > 0 ? dies : 1;
-
-    /* compute missing values based on the provided ones */
-    if (cpus == 0 && maxcpus == 0) {
-        sockets = sockets > 0 ? sockets : 1;
-        cores = cores > 0 ? cores : 1;
-        threads = threads > 0 ? threads : 1;
-    } else {
-        maxcpus = maxcpus > 0 ? maxcpus : cpus;
-
-        if (mc->smp_prefer_sockets) {
-            /* prefer sockets over cores before 6.2 */
-            if (sockets == 0) {
-                cores = cores > 0 ? cores : 1;
-                threads = threads > 0 ? threads : 1;
-                sockets = maxcpus / (dies * cores * threads);
-            } else if (cores == 0) {
-                threads = threads > 0 ? threads : 1;
-                cores = maxcpus / (sockets * dies * threads);
-            }
-        } else {
-            /* prefer cores over sockets since 6.2 */
-            if (cores == 0) {
-                sockets = sockets > 0 ? sockets : 1;
-                threads = threads > 0 ? threads : 1;
-                cores = maxcpus / (sockets * dies * threads);
-            } else if (sockets == 0) {
-                threads = threads > 0 ? threads : 1;
-                sockets = maxcpus / (dies * cores * threads);
-            }
-        }
-
-        /* try to calculate omitted threads at last */
-        if (threads == 0) {
-            threads = maxcpus / (sockets * dies * cores);
-        }
-    }
-
-    maxcpus = maxcpus > 0 ? maxcpus : sockets * dies * cores * threads;
-    cpus = cpus > 0 ? cpus : maxcpus;
-
-    if (sockets * dies * cores * threads != maxcpus) {
-        error_setg(errp, "Invalid CPU topology: "
-                   "product of the hierarchy must match maxcpus: "
-                   "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
-                   "!= maxcpus (%u)",
-                   sockets, dies, cores, threads, maxcpus);
-        return;
-    }
-
-    if (maxcpus < cpus) {
-        error_setg(errp, "Invalid CPU topology: "
-                   "maxcpus must be equal to or greater than smp: "
-                   "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
-                   "== maxcpus (%u) < smp_cpus (%u)",
-                   sockets, dies, cores, threads, maxcpus, cpus);
-        return;
-    }
-
-    ms->smp.cpus = cpus;
-    ms->smp.sockets = sockets;
-    ms->smp.dies = dies;
-    ms->smp.cores = cores;
-    ms->smp.threads = threads;
-    ms->smp.max_cpus = maxcpus;
-}
-
 static
 void pc_machine_done(Notifier *notifier, void *data)
 {
@@ -1755,7 +1673,6 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
     mc->auto_enable_numa_with_memdev = true;
     mc->has_hotpluggable_cpus = true;
     mc->default_boot_order = "cad";
-    mc->smp_parse = pc_smp_parse;
     mc->block_default_type = IF_IDE;
     mc->max_cpus = 255;
     mc->reset = pc_machine_reset;
@@ -1766,6 +1683,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
     hc->unplug = pc_machine_device_unplug_cb;
     mc->default_cpu_type = TARGET_DEFAULT_CPU_TYPE;
     mc->nvdimm_supported = true;
+    mc->smp_props.dies_supported = true;
     mc->default_ram_id = "pc.ram";
 
     object_class_property_add(oc, PC_MACHINE_MAX_RAM_BELOW_4G, "size",
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 2a1bba86c0..72a23e4e0f 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -108,6 +108,14 @@ typedef struct {
     CPUArchId cpus[];
 } CPUArchIdList;
 
+/**
+ * SMPCompatProps:
+ * @dies_supported - whether dies are supported by the machine
+ */
+typedef struct {
+    bool dies_supported;
+} SMPCompatProps;
+
 /**
  * MachineClass:
  * @deprecation_reason: If set, the machine is marked as deprecated. The
@@ -248,6 +256,7 @@ struct MachineClass {
     bool numa_mem_supported;
     bool auto_enable_numa;
     bool smp_prefer_sockets;
+    SMPCompatProps smp_props;
     const char *default_ram_id;
 
     HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 13/30] machine: Remove smp_parse callback from MachineClass
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (11 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 12/30] machine: Make smp_parse generic enough for all arches Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 14/30] machine: Move smp_prefer_sockets to struct SMPCompatProps Paolo Bonzini
                   ` (18 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Yanan Wang, Andrew Jones, Daniel P . Berrangé

From: Yanan Wang <wangyanan55@huawei.com>

Now we have a generic smp parser for all arches, and there will
not be any other arch specific ones, so let's remove the callback
from MachineClass and call the parser directly.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20210929025816.21076-14-wangyanan55@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/machine.c   | 3 +--
 include/hw/boards.h | 5 -----
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 12872d7715..8b0f1aed83 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -918,7 +918,7 @@ static void machine_set_smp(Object *obj, Visitor *v, const char *name,
                     "CPU topology parameters must be greater than zero");
     }
 
-    mc->smp_parse(ms, config, errp);
+    smp_parse(ms, config, errp);
     if (*errp) {
         goto out_free;
     }
@@ -947,7 +947,6 @@ static void machine_class_init(ObjectClass *oc, void *data)
     /* Default 128 MB as guest ram size */
     mc->default_ram_size = 128 * MiB;
     mc->rom_file_has_mr = true;
-    mc->smp_parse = smp_parse;
 
     /* numa node memory size aligned on 8MB by default.
      * On Linux, each node's border has to be 8MB aligned
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 72a23e4e0f..fa284e01e9 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -177,10 +177,6 @@ typedef struct {
  *    kvm-type may be NULL if it is not needed.
  * @numa_mem_supported:
  *    true if '--numa node.mem' option is supported and false otherwise
- * @smp_parse:
- *    The function pointer to hook different machine specific functions for
- *    parsing "smp-opts" from QemuOpts to MachineState::CpuTopology and more
- *    machine specific topology fields, such as smp_dies for PCMachine.
  * @hotplug_allowed:
  *    If the hook is provided, then it'll be called for each device
  *    hotplug to check whether the device hotplug is allowed.  Return
@@ -217,7 +213,6 @@ struct MachineClass {
     void (*reset)(MachineState *state);
     void (*wakeup)(MachineState *state);
     int (*kvm_type)(MachineState *machine, const char *arg);
-    void (*smp_parse)(MachineState *ms, SMPConfiguration *config, Error **errp);
 
     BlockInterfaceType block_default_type;
     int units_per_default_bus;
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 14/30] machine: Move smp_prefer_sockets to struct SMPCompatProps
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (12 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 13/30] machine: Remove smp_parse callback from MachineClass Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 15/30] machine: Use g_autoptr in machine_set_smp Paolo Bonzini
                   ` (17 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Yanan Wang, Andrew Jones, Daniel P . Berrangé, David Gibson

From: Yanan Wang <wangyanan55@huawei.com>

Now we have a common structure SMPCompatProps used to store information
about SMP compatibility stuff, so we can also move smp_prefer_sockets
there for cleaner code.

No functional change intended.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20210929025816.21076-15-wangyanan55@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/arm/virt.c              | 2 +-
 hw/core/machine.c          | 2 +-
 hw/i386/pc_piix.c          | 2 +-
 hw/i386/pc_q35.c           | 2 +-
 hw/ppc/spapr.c             | 2 +-
 hw/s390x/s390-virtio-ccw.c | 2 +-
 include/hw/boards.h        | 3 ++-
 7 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 8c13deb5db..7170aaacd5 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2815,7 +2815,7 @@ static void virt_machine_6_1_options(MachineClass *mc)
 
     virt_machine_6_2_options(mc);
     compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
-    mc->smp_prefer_sockets = true;
+    mc->smp_props.prefer_sockets = true;
 
     /* qemu ITS was introduced with 6.2 */
     vmc->no_tcg_its = true;
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 8b0f1aed83..54f04a5ac6 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -817,7 +817,7 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
     } else {
         maxcpus = maxcpus > 0 ? maxcpus : cpus;
 
-        if (mc->smp_prefer_sockets) {
+        if (mc->smp_props.prefer_sockets) {
             /* prefer sockets over cores before 6.2 */
             if (sockets == 0) {
                 cores = cores > 0 ? cores : 1;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 932e6e2cfe..6ad0d763c5 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -432,7 +432,7 @@ static void pc_i440fx_6_1_machine_options(MachineClass *m)
     m->is_default = false;
     compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
     compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
-    m->smp_prefer_sockets = true;
+    m->smp_props.prefer_sockets = true;
 }
 
 DEFINE_I440FX_MACHINE(v6_1, "pc-i440fx-6.1", NULL,
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 57ab07e5b7..fcc6e4eb2b 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -372,7 +372,7 @@ static void pc_q35_6_1_machine_options(MachineClass *m)
     m->alias = NULL;
     compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
     compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
-    m->smp_prefer_sockets = true;
+    m->smp_props.prefer_sockets = true;
 }
 
 DEFINE_Q35_MACHINE(v6_1, "pc-q35-6.1", NULL,
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 2986108b5a..163c90388a 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4685,7 +4685,7 @@ static void spapr_machine_6_1_class_options(MachineClass *mc)
     spapr_machine_6_2_class_options(mc);
     compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
     smc->pre_6_2_numa_affinity = true;
-    mc->smp_prefer_sockets = true;
+    mc->smp_props.prefer_sockets = true;
 }
 
 DEFINE_SPAPR_MACHINE(6_1, "6.1", false);
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 5401c985cf..653587ea62 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -814,7 +814,7 @@ static void ccw_machine_6_1_class_options(MachineClass *mc)
 {
     ccw_machine_6_2_class_options(mc);
     compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
-    mc->smp_prefer_sockets = true;
+    mc->smp_props.prefer_sockets = true;
 }
 DEFINE_CCW_MACHINE(6_1, "6.1", false);
 
diff --git a/include/hw/boards.h b/include/hw/boards.h
index fa284e01e9..5adbcbb99b 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -110,9 +110,11 @@ typedef struct {
 
 /**
  * SMPCompatProps:
+ * @prefer_sockets - whether sockets are preferred over cores in smp parsing
  * @dies_supported - whether dies are supported by the machine
  */
 typedef struct {
+    bool prefer_sockets;
     bool dies_supported;
 } SMPCompatProps;
 
@@ -250,7 +252,6 @@ struct MachineClass {
     bool nvdimm_supported;
     bool numa_mem_supported;
     bool auto_enable_numa;
-    bool smp_prefer_sockets;
     SMPCompatProps smp_props;
     const char *default_ram_id;
 
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 15/30] machine: Use g_autoptr in machine_set_smp
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (13 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 14/30] machine: Move smp_prefer_sockets to struct SMPCompatProps Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 16/30] machine: Put all sanity-check in the generic SMP parser Paolo Bonzini
                   ` (16 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/machine.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 54f04a5ac6..d49ebc24e2 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -897,7 +897,7 @@ static void machine_set_smp(Object *obj, Visitor *v, const char *name,
 {
     MachineClass *mc = MACHINE_GET_CLASS(obj);
     MachineState *ms = MACHINE(obj);
-    SMPConfiguration *config;
+    g_autoptr(SMPConfiguration) config = NULL;
     ERRP_GUARD();
 
     if (!visit_type_SMPConfiguration(v, name, &config, errp)) {
@@ -920,7 +920,7 @@ static void machine_set_smp(Object *obj, Visitor *v, const char *name,
 
     smp_parse(ms, config, errp);
     if (*errp) {
-        goto out_free;
+        return;
     }
 
     /* sanity-check smp_cpus and max_cpus against mc */
@@ -935,9 +935,6 @@ static void machine_set_smp(Object *obj, Visitor *v, const char *name,
                    ms->smp.max_cpus,
                    mc->name, mc->max_cpus);
     }
-
-out_free:
-    qapi_free_SMPConfiguration(config);
 }
 
 static void machine_class_init(ObjectClass *oc, void *data)
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 16/30] machine: Put all sanity-check in the generic SMP parser
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (14 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 15/30] machine: Use g_autoptr in machine_set_smp Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 17/30] i386: Support KVM_CAP_ENFORCE_PV_FEATURE_CPUID Paolo Bonzini
                   ` (15 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Yanan Wang, Andrew Jones, Daniel P . Berrangé, Pankaj Gupta

From: Yanan Wang <wangyanan55@huawei.com>

Put both sanity-check of the input SMP configuration and sanity-check
of the output SMP configuration uniformly in the generic parser. Then
machine_set_smp() will become cleaner, also all the invalid scenarios
can be tested only by calling the parser.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20210929025816.21076-16-wangyanan55@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/machine.c | 62 +++++++++++++++++++++++------------------------
 1 file changed, 30 insertions(+), 32 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index d49ebc24e2..3920a2f2af 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -798,6 +798,20 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
     unsigned threads = config->has_threads ? config->threads : 0;
     unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0;
 
+    /*
+     * Specified CPU topology parameters must be greater than zero,
+     * explicit configuration like "cpus=0" is not allowed.
+     */
+    if ((config->has_cpus && config->cpus == 0) ||
+        (config->has_sockets && config->sockets == 0) ||
+        (config->has_dies && config->dies == 0) ||
+        (config->has_cores && config->cores == 0) ||
+        (config->has_threads && config->threads == 0) ||
+        (config->has_maxcpus && config->maxcpus == 0)) {
+        warn_report("Deprecated CPU topology (considered invalid): "
+                    "CPU topology parameters must be greater than zero");
+    }
+
     /*
      * If not supported by the machine, a topology parameter must be
      * omitted or specified equal to 1.
@@ -873,6 +887,22 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
                    topo_msg, maxcpus, cpus);
         return;
     }
+
+    if (ms->smp.cpus < mc->min_cpus) {
+        error_setg(errp, "Invalid SMP CPUs %d. The min CPUs "
+                   "supported by machine '%s' is %d",
+                   ms->smp.cpus,
+                   mc->name, mc->min_cpus);
+        return;
+    }
+
+    if (ms->smp.max_cpus > mc->max_cpus) {
+        error_setg(errp, "Invalid SMP CPUs %d. The max CPUs "
+                   "supported by machine '%s' is %d",
+                   ms->smp.max_cpus,
+                   mc->name, mc->max_cpus);
+        return;
+    }
 }
 
 static void machine_get_smp(Object *obj, Visitor *v, const char *name,
@@ -895,46 +925,14 @@ static void machine_get_smp(Object *obj, Visitor *v, const char *name,
 static void machine_set_smp(Object *obj, Visitor *v, const char *name,
                             void *opaque, Error **errp)
 {
-    MachineClass *mc = MACHINE_GET_CLASS(obj);
     MachineState *ms = MACHINE(obj);
     g_autoptr(SMPConfiguration) config = NULL;
-    ERRP_GUARD();
 
     if (!visit_type_SMPConfiguration(v, name, &config, errp)) {
         return;
     }
 
-    /*
-     * Specified CPU topology parameters must be greater than zero,
-     * explicit configuration like "cpus=0" is not allowed.
-     */
-    if ((config->has_cpus && config->cpus == 0) ||
-        (config->has_sockets && config->sockets == 0) ||
-        (config->has_dies && config->dies == 0) ||
-        (config->has_cores && config->cores == 0) ||
-        (config->has_threads && config->threads == 0) ||
-        (config->has_maxcpus && config->maxcpus == 0)) {
-        warn_report("Deprecated CPU topology (considered invalid): "
-                    "CPU topology parameters must be greater than zero");
-    }
-
     smp_parse(ms, config, errp);
-    if (*errp) {
-        return;
-    }
-
-    /* sanity-check smp_cpus and max_cpus against mc */
-    if (ms->smp.cpus < mc->min_cpus) {
-        error_setg(errp, "Invalid SMP CPUs %d. The min CPUs "
-                   "supported by machine '%s' is %d",
-                   ms->smp.cpus,
-                   mc->name, mc->min_cpus);
-    } else if (ms->smp.max_cpus > mc->max_cpus) {
-        error_setg(errp, "Invalid SMP CPUs %d. The max CPUs "
-                   "supported by machine '%s' is %d",
-                   ms->smp.max_cpus,
-                   mc->name, mc->max_cpus);
-    }
 }
 
 static void machine_class_init(ObjectClass *oc, void *data)
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 17/30] i386: Support KVM_CAP_ENFORCE_PV_FEATURE_CPUID
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (15 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 16/30] machine: Put all sanity-check in the generic SMP parser Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 18/30] i386: Support KVM_CAP_HYPERV_ENFORCE_CPUID Paolo Bonzini
                   ` (14 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Vitaly Kuznetsov

From: Vitaly Kuznetsov <vkuznets@redhat.com>

By default, KVM allows the guest to use all currently supported PV features
even when they were not announced in guest visible CPUIDs. Introduce a new
"kvm-pv-enforce-cpuid" flag to limit the supported feature set to the
exposed features. The feature is supported by Linux >= 5.10 and is not
enabled by default in QEMU.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Message-Id: <20210902093530.345756-4-vkuznets@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/cpu.c     |  2 ++
 target/i386/cpu.h     |  3 +++
 target/i386/kvm/kvm.c | 10 ++++++++++
 3 files changed, 15 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index cacec605bf..598019de12 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6860,6 +6860,8 @@ static Property x86_cpu_properties[] = {
     DEFINE_PROP_BOOL("l3-cache", X86CPU, enable_l3_cache, true),
     DEFINE_PROP_BOOL("kvm-no-smi-migration", X86CPU, kvm_no_smi_migration,
                      false),
+    DEFINE_PROP_BOOL("kvm-pv-enforce-cpuid", X86CPU, kvm_pv_enforce_cpuid,
+                     false),
     DEFINE_PROP_BOOL("vmware-cpuid-freq", X86CPU, vmware_cpuid_freq, true),
     DEFINE_PROP_BOOL("tcg-cpuid", X86CPU, expose_tcg, true),
     DEFINE_PROP_BOOL("x-migrate-smi-count", X86CPU, migrate_smi_count,
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 29552dc2a7..c990150373 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1802,6 +1802,9 @@ struct X86CPU {
     /* Stop SMI delivery for migration compatibility with old machines */
     bool kvm_no_smi_migration;
 
+    /* Forcefully disable KVM PV features not exposed in guest CPUIDs */
+    bool kvm_pv_enforce_cpuid;
+
     /* Number of physical address bits supported */
     uint32_t phys_bits;
 
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 7f1b060e6d..d6a70c27e5 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1629,6 +1629,16 @@ int kvm_arch_init_vcpu(CPUState *cs)
 
     cpu_x86_cpuid(env, 0, 0, &limit, &unused, &unused, &unused);
 
+    if (cpu->kvm_pv_enforce_cpuid) {
+        r = kvm_vcpu_enable_cap(cs, KVM_CAP_ENFORCE_PV_FEATURE_CPUID, 0, 1);
+        if (r < 0) {
+            fprintf(stderr,
+                    "failed to enable KVM_CAP_ENFORCE_PV_FEATURE_CPUID: %s",
+                    strerror(-r));
+            abort();
+        }
+    }
+
     for (i = 0; i <= limit; i++) {
         if (cpuid_i == KVM_MAX_CPUID_ENTRIES) {
             fprintf(stderr, "unsupported level value: 0x%x\n", limit);
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 18/30] i386: Support KVM_CAP_HYPERV_ENFORCE_CPUID
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (16 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 17/30] i386: Support KVM_CAP_ENFORCE_PV_FEATURE_CPUID Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 19/30] i386: Move HV_APIC_ACCESS_RECOMMENDED bit setting to hyperv_fill_cpuids() Paolo Bonzini
                   ` (13 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Vitaly Kuznetsov

From: Vitaly Kuznetsov <vkuznets@redhat.com>

By default, KVM allows the guest to use all currently supported Hyper-V
enlightenments when Hyper-V CPUID interface was exposed, regardless of if
some features were not announced in guest visible CPUIDs. hv-enforce-cpuid
feature alters this behavior and only allows the guest to use exposed
Hyper-V enlightenments. The feature is supported by Linux >= 5.14 and is
not enabled by default in QEMU.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Message-Id: <20210902093530.345756-5-vkuznets@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 docs/hyperv.txt       | 17 ++++++++++++++---
 target/i386/cpu.c     |  1 +
 target/i386/cpu.h     |  1 +
 target/i386/kvm/kvm.c |  9 +++++++++
 4 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/docs/hyperv.txt b/docs/hyperv.txt
index 000638a2fd..072709a68f 100644
--- a/docs/hyperv.txt
+++ b/docs/hyperv.txt
@@ -203,8 +203,11 @@ When the option is set to 'on' QEMU will always enable the feature, regardless
 of host setup. To keep guests secure, this can only be used in conjunction with
 exposing correct vCPU topology and vCPU pinning.
 
-4. Development features
-========================
+4. Supplementary features
+=========================
+
+4.1. hv-passthrough
+===================
 In some cases (e.g. during development) it may make sense to use QEMU in
 'pass-through' mode and give Windows guests all enlightenments currently
 supported by KVM. This pass-through mode is enabled by "hv-passthrough" CPU
@@ -215,8 +218,16 @@ values from KVM to QEMU. "hv-passthrough" overrides all other "hv-*" settings on
 the command line. Also, enabling this flag effectively prevents migration as the
 list of enabled enlightenments may differ between target and destination hosts.
 
+4.2. hv-enforce-cpuid
+=====================
+By default, KVM allows the guest to use all currently supported Hyper-V
+enlightenments when Hyper-V CPUID interface was exposed, regardless of if
+some features were not announced in guest visible CPUIDs. 'hv-enforce-cpuid'
+feature alters this behavior and only allows the guest to use exposed Hyper-V
+enlightenments.
 
-4. Useful links
+
+5. Useful links
 ================
 Hyper-V Top Level Functional specification and other information:
 https://github.com/MicrosoftDocs/Virtualization-Documentation
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 598019de12..2a19eba56d 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6834,6 +6834,7 @@ static Property x86_cpu_properties[] = {
     DEFINE_PROP_ON_OFF_AUTO("hv-no-nonarch-coresharing", X86CPU,
                             hyperv_no_nonarch_cs, ON_OFF_AUTO_OFF),
     DEFINE_PROP_BOOL("hv-passthrough", X86CPU, hyperv_passthrough, false),
+    DEFINE_PROP_BOOL("hv-enforce-cpuid", X86CPU, hyperv_enforce_cpuid, false),
 
     DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
     DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index c990150373..8a7209bbf2 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1719,6 +1719,7 @@ struct X86CPU {
     uint32_t hyperv_version_id[4];
     uint32_t hyperv_limits[3];
     uint32_t hyperv_nested[4];
+    bool hyperv_enforce_cpuid;
 
     bool check_cpuid;
     bool enforce_cpuid;
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index d6a70c27e5..fbe6b7ac72 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1531,6 +1531,15 @@ static int hyperv_init_vcpu(X86CPU *cpu)
         cpu->hyperv_nested[0] = evmcs_version;
     }
 
+    if (cpu->hyperv_enforce_cpuid) {
+        ret = kvm_vcpu_enable_cap(cs, KVM_CAP_HYPERV_ENFORCE_CPUID, 0, 1);
+        if (ret < 0) {
+            error_report("failed to enable KVM_CAP_HYPERV_ENFORCE_CPUID: %s",
+                         strerror(-ret));
+            return ret;
+        }
+    }
+
     return 0;
 }
 
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 19/30] i386: Move HV_APIC_ACCESS_RECOMMENDED bit setting to hyperv_fill_cpuids()
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (17 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 18/30] i386: Support KVM_CAP_HYPERV_ENFORCE_CPUID Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 20/30] i386: Implement pseudo 'hv-avic' ('hv-apicv') enlightenment Paolo Bonzini
                   ` (12 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Vitaly Kuznetsov

From: Vitaly Kuznetsov <vkuznets@redhat.com>

In preparation to enabling Hyper-V + APICv/AVIC move
HV_APIC_ACCESS_RECOMMENDED setting out of kvm_hyperv_properties[]: the
'real' feature bit for the vAPIC features is HV_APIC_ACCESS_AVAILABLE,
HV_APIC_ACCESS_RECOMMENDED is a recommendation to use the feature which
we may not always want to give.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Message-Id: <20210902093530.345756-6-vkuznets@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/kvm/kvm.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index fbe6b7ac72..a9a8f77df3 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -821,9 +821,7 @@ static struct {
         .desc = "virtual APIC (hv-vapic)",
         .flags = {
             {.func = HV_CPUID_FEATURES, .reg = R_EAX,
-             .bits = HV_APIC_ACCESS_AVAILABLE},
-            {.func = HV_CPUID_ENLIGHTMENT_INFO, .reg = R_EAX,
-             .bits = HV_APIC_ACCESS_RECOMMENDED}
+             .bits = HV_APIC_ACCESS_AVAILABLE}
         }
     },
     [HYPERV_FEAT_TIME] = {
@@ -1366,6 +1364,7 @@ static int hyperv_fill_cpuids(CPUState *cs,
         c->ebx |= HV_POST_MESSAGES | HV_SIGNAL_EVENTS;
     }
 
+
     /* Not exposed by KVM but needed to make CPU hotplug in Windows work */
     c->edx |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
 
@@ -1374,6 +1373,10 @@ static int hyperv_fill_cpuids(CPUState *cs,
     c->eax = hv_build_cpuid_leaf(cs, HV_CPUID_ENLIGHTMENT_INFO, R_EAX);
     c->ebx = cpu->hyperv_spinlock_attempts;
 
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VAPIC)) {
+        c->eax |= HV_APIC_ACCESS_RECOMMENDED;
+    }
+
     if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_ON) {
         c->eax |= HV_NO_NONARCH_CORESHARING;
     } else if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_AUTO) {
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 20/30] i386: Implement pseudo 'hv-avic' ('hv-apicv') enlightenment
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (18 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 19/30] i386: Move HV_APIC_ACCESS_RECOMMENDED bit setting to hyperv_fill_cpuids() Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 21/30] i386: Make Hyper-V version id configurable Paolo Bonzini
                   ` (11 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Vitaly Kuznetsov

From: Vitaly Kuznetsov <vkuznets@redhat.com>

The enlightenment allows to use Hyper-V SynIC with hardware APICv/AVIC
enabled. Normally, Hyper-V SynIC disables these hardware features and
suggests the guest to use paravirtualized AutoEOI feature. Linux-4.15
gains support for conditional APICv/AVIC disablement, the feature
stays on until the guest tries to use AutoEOI feature with SynIC. With
'HV_DEPRECATING_AEOI_RECOMMENDED' bit exposed, modern enough Windows/
Hyper-V versions should follow the recommendation and not use the
(unwanted) feature.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Message-Id: <20210902093530.345756-7-vkuznets@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 docs/hyperv.txt                | 10 +++++++++-
 target/i386/cpu.c              |  4 ++++
 target/i386/cpu.h              |  1 +
 target/i386/kvm/hyperv-proto.h |  1 +
 target/i386/kvm/kvm.c          | 10 +++++++++-
 5 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/docs/hyperv.txt b/docs/hyperv.txt
index 072709a68f..cd1ea3bbe9 100644
--- a/docs/hyperv.txt
+++ b/docs/hyperv.txt
@@ -189,7 +189,15 @@ enabled.
 
 Requires: hv-vpindex, hv-synic, hv-time, hv-stimer
 
-3.17. hv-no-nonarch-coresharing=on/off/auto
+3.18. hv-avic (hv-apicv)
+=======================
+The enlightenment allows to use Hyper-V SynIC with hardware APICv/AVIC enabled.
+Normally, Hyper-V SynIC disables these hardware feature and suggests the guest
+to use paravirtualized AutoEOI feature.
+Note: enabling this feature on old hardware (without APICv/AVIC support) may
+have negative effect on guest's performace.
+
+3.19. hv-no-nonarch-coresharing=on/off/auto
 ===========================================
 This enlightenment tells guest OS that virtual processors will never share a
 physical core unless they are reported as sibling SMT threads. This information
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 2a19eba56d..8154343cc4 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6644,6 +6644,8 @@ static void x86_cpu_initfn(Object *obj)
     object_property_add_alias(obj, "sse4_1", obj, "sse4.1");
     object_property_add_alias(obj, "sse4_2", obj, "sse4.2");
 
+    object_property_add_alias(obj, "hv-apicv", obj, "hv-avic");
+
     if (xcc->model) {
         x86_cpu_load_model(cpu, xcc->model);
     }
@@ -6831,6 +6833,8 @@ static Property x86_cpu_properties[] = {
                       HYPERV_FEAT_IPI, 0),
     DEFINE_PROP_BIT64("hv-stimer-direct", X86CPU, hyperv_features,
                       HYPERV_FEAT_STIMER_DIRECT, 0),
+    DEFINE_PROP_BIT64("hv-avic", X86CPU, hyperv_features,
+                      HYPERV_FEAT_AVIC, 0),
     DEFINE_PROP_ON_OFF_AUTO("hv-no-nonarch-coresharing", X86CPU,
                             hyperv_no_nonarch_cs, ON_OFF_AUTO_OFF),
     DEFINE_PROP_BOOL("hv-passthrough", X86CPU, hyperv_passthrough, false),
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 8a7209bbf2..65f0ee2caf 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1056,6 +1056,7 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS];
 #define HYPERV_FEAT_EVMCS               12
 #define HYPERV_FEAT_IPI                 13
 #define HYPERV_FEAT_STIMER_DIRECT       14
+#define HYPERV_FEAT_AVIC                15
 
 #ifndef HYPERV_SPINLOCK_NEVER_NOTIFY
 #define HYPERV_SPINLOCK_NEVER_NOTIFY             0xFFFFFFFF
diff --git a/target/i386/kvm/hyperv-proto.h b/target/i386/kvm/hyperv-proto.h
index 5fbb385cc1..89f81afda7 100644
--- a/target/i386/kvm/hyperv-proto.h
+++ b/target/i386/kvm/hyperv-proto.h
@@ -66,6 +66,7 @@
 #define HV_APIC_ACCESS_RECOMMENDED          (1u << 3)
 #define HV_SYSTEM_RESET_RECOMMENDED         (1u << 4)
 #define HV_RELAXED_TIMING_RECOMMENDED       (1u << 5)
+#define HV_DEPRECATING_AEOI_RECOMMENDED     (1u << 9)
 #define HV_CLUSTER_IPI_RECOMMENDED          (1u << 10)
 #define HV_EX_PROCESSOR_MASKS_RECOMMENDED   (1u << 11)
 #define HV_ENLIGHTENED_VMCS_RECOMMENDED     (1u << 14)
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index a9a8f77df3..68faf72e34 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -924,6 +924,13 @@ static struct {
         },
         .dependencies = BIT(HYPERV_FEAT_STIMER)
     },
+    [HYPERV_FEAT_AVIC] = {
+        .desc = "AVIC/APICv support (hv-avic/hv-apicv)",
+        .flags = {
+            {.func = HV_CPUID_ENLIGHTMENT_INFO, .reg = R_EAX,
+             .bits = HV_DEPRECATING_AEOI_RECOMMENDED}
+        }
+    },
 };
 
 static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max,
@@ -1373,7 +1380,8 @@ static int hyperv_fill_cpuids(CPUState *cs,
     c->eax = hv_build_cpuid_leaf(cs, HV_CPUID_ENLIGHTMENT_INFO, R_EAX);
     c->ebx = cpu->hyperv_spinlock_attempts;
 
-    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VAPIC)) {
+    if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VAPIC) &&
+        !hyperv_feat_enabled(cpu, HYPERV_FEAT_AVIC)) {
         c->eax |= HV_APIC_ACCESS_RECOMMENDED;
     }
 
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 21/30] i386: Make Hyper-V version id configurable
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (19 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 20/30] i386: Implement pseudo 'hv-avic' ('hv-apicv') enlightenment Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 22/30] i386: Change the default Hyper-V version to match WS2016 Paolo Bonzini
                   ` (10 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Vitaly Kuznetsov

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Currently, we hardcode Hyper-V version id (CPUID 0x40000002) to
WS2008R2 and it is known that certain tools in Windows check this. It
seems useful to provide some flexibility by making it possible to change
this info at will. CPUID information is defined in TLFS as:

EAX: Build Number
EBX Bits 31-16: Major Version
    Bits 15-0: Minor Version
ECX Service Pack
EDX Bits 31-24: Service Branch
    Bits 23-0: Service Number

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Message-Id: <20210902093530.345756-8-vkuznets@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 docs/hyperv.txt       | 14 ++++++++++++++
 target/i386/cpu.c     | 15 +++++++++++----
 target/i386/cpu.h     |  7 ++++++-
 target/i386/kvm/kvm.c | 26 ++++++++++++++++----------
 4 files changed, 47 insertions(+), 15 deletions(-)

diff --git a/docs/hyperv.txt b/docs/hyperv.txt
index cd1ea3bbe9..7803495468 100644
--- a/docs/hyperv.txt
+++ b/docs/hyperv.txt
@@ -211,6 +211,20 @@ When the option is set to 'on' QEMU will always enable the feature, regardless
 of host setup. To keep guests secure, this can only be used in conjunction with
 exposing correct vCPU topology and vCPU pinning.
 
+3.20. hv-version-id-{build,major,minor,spack,sbranch,snumber}
+=============================================================
+This changes Hyper-V version identification in CPUID 0x40000002.EAX-EDX from the
+default (WS2008R2).
+- hv-version-id-build sets 'Build Number' (32 bits)
+- hv-version-id-major sets 'Major Version' (16 bits)
+- hv-version-id-minor sets 'Minor Version' (16 bits)
+- hv-version-id-spack sets 'Service Pack' (32 bits)
+- hv-version-id-sbranch sets 'Service Branch' (8 bits)
+- hv-version-id-snumber sets 'Service Number' (24 bits)
+
+Note: hv-version-id-* are not enlightenments and thus don't enable Hyper-V
+identification when specified without any other enlightenments.
+
 4. Supplementary features
 =========================
 
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 8154343cc4..d1d057fabe 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6258,10 +6258,6 @@ static void x86_cpu_hyperv_realize(X86CPU *cpu)
     cpu->hyperv_interface_id[2] = 0;
     cpu->hyperv_interface_id[3] = 0;
 
-    /* Hypervisor system identity */
-    cpu->hyperv_version_id[0] = 0x00001bbc;
-    cpu->hyperv_version_id[1] = 0x00060001;
-
     /* Hypervisor implementation limits */
     cpu->hyperv_limits[0] = 64;
     cpu->hyperv_limits[1] = 0;
@@ -6840,6 +6836,17 @@ static Property x86_cpu_properties[] = {
     DEFINE_PROP_BOOL("hv-passthrough", X86CPU, hyperv_passthrough, false),
     DEFINE_PROP_BOOL("hv-enforce-cpuid", X86CPU, hyperv_enforce_cpuid, false),
 
+    /* WS2008R2 identify by default */
+    DEFINE_PROP_UINT32("hv-version-id-build", X86CPU, hyperv_ver_id_build,
+                       0x1bbc),
+    DEFINE_PROP_UINT16("hv-version-id-major", X86CPU, hyperv_ver_id_major,
+                       0x0006),
+    DEFINE_PROP_UINT16("hv-version-id-minor", X86CPU, hyperv_ver_id_minor,
+                       0x0001),
+    DEFINE_PROP_UINT32("hv-version-id-spack", X86CPU, hyperv_ver_id_sp, 0),
+    DEFINE_PROP_UINT8("hv-version-id-sbranch", X86CPU, hyperv_ver_id_sb, 0),
+    DEFINE_PROP_UINT32("hv-version-id-snumber", X86CPU, hyperv_ver_id_sn, 0),
+
     DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
     DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
     DEFINE_PROP_BOOL("x-force-features", X86CPU, force_features, false),
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 65f0ee2caf..3edaad7688 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1717,10 +1717,15 @@ struct X86CPU {
     OnOffAuto hyperv_no_nonarch_cs;
     uint32_t hyperv_vendor_id[3];
     uint32_t hyperv_interface_id[4];
-    uint32_t hyperv_version_id[4];
     uint32_t hyperv_limits[3];
     uint32_t hyperv_nested[4];
     bool hyperv_enforce_cpuid;
+    uint32_t hyperv_ver_id_build;
+    uint16_t hyperv_ver_id_major;
+    uint16_t hyperv_ver_id_minor;
+    uint32_t hyperv_ver_id_sp;
+    uint8_t hyperv_ver_id_sb;
+    uint32_t hyperv_ver_id_sn;
 
     bool check_cpuid;
     bool enforce_cpuid;
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 68faf72e34..f25837f63f 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1258,14 +1258,18 @@ bool kvm_hyperv_expand_features(X86CPU *cpu, Error **errp)
         cpu->hyperv_interface_id[3] =
             hv_cpuid_get_host(cs, HV_CPUID_INTERFACE, R_EDX);
 
-        cpu->hyperv_version_id[0] =
+        cpu->hyperv_ver_id_build =
             hv_cpuid_get_host(cs, HV_CPUID_VERSION, R_EAX);
-        cpu->hyperv_version_id[1] =
-            hv_cpuid_get_host(cs, HV_CPUID_VERSION, R_EBX);
-        cpu->hyperv_version_id[2] =
+        cpu->hyperv_ver_id_major =
+            hv_cpuid_get_host(cs, HV_CPUID_VERSION, R_EBX) >> 16;
+        cpu->hyperv_ver_id_minor =
+            hv_cpuid_get_host(cs, HV_CPUID_VERSION, R_EBX) & 0xffff;
+        cpu->hyperv_ver_id_sp =
             hv_cpuid_get_host(cs, HV_CPUID_VERSION, R_ECX);
-        cpu->hyperv_version_id[3] =
-            hv_cpuid_get_host(cs, HV_CPUID_VERSION, R_EDX);
+        cpu->hyperv_ver_id_sb =
+            hv_cpuid_get_host(cs, HV_CPUID_VERSION, R_EDX) >> 24;
+        cpu->hyperv_ver_id_sn =
+            hv_cpuid_get_host(cs, HV_CPUID_VERSION, R_EDX) & 0xffffff;
 
         cpu->hv_max_vps = hv_cpuid_get_host(cs, HV_CPUID_IMPLEMENT_LIMITS,
                                             R_EAX);
@@ -1351,10 +1355,12 @@ static int hyperv_fill_cpuids(CPUState *cs,
 
     c = &cpuid_ent[cpuid_i++];
     c->function = HV_CPUID_VERSION;
-    c->eax = cpu->hyperv_version_id[0];
-    c->ebx = cpu->hyperv_version_id[1];
-    c->ecx = cpu->hyperv_version_id[2];
-    c->edx = cpu->hyperv_version_id[3];
+    c->eax = cpu->hyperv_ver_id_build;
+    c->ebx = (uint32_t)cpu->hyperv_ver_id_major << 16 |
+        cpu->hyperv_ver_id_minor;
+    c->ecx = cpu->hyperv_ver_id_sp;
+    c->edx = (uint32_t)cpu->hyperv_ver_id_sb << 24 |
+        (cpu->hyperv_ver_id_sn & 0xffffff);
 
     c = &cpuid_ent[cpuid_i++];
     c->function = HV_CPUID_FEATURES;
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 22/30] i386: Change the default Hyper-V version to match WS2016
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (20 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 21/30] i386: Make Hyper-V version id configurable Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 23/30] configure: Loosen GCC requirement from 7.5.0 to 7.4.0 Paolo Bonzini
                   ` (9 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Vitaly Kuznetsov

From: Vitaly Kuznetsov <vkuznets@redhat.com>

KVM implements some Hyper-V 2016 functions so providing WS2008R2 version
is somewhat incorrect. While generally guests shouldn't care about it
and always check feature bits, it is known that some tools in Windows
actually check version info.

For compatibility reasons make the change for 6.2 machine types only.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Message-Id: <20210902093530.345756-9-vkuznets@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 docs/hyperv.txt   | 2 +-
 hw/i386/pc.c      | 6 +++++-
 target/i386/cpu.c | 6 +++---
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/docs/hyperv.txt b/docs/hyperv.txt
index 7803495468..5d99fd9a72 100644
--- a/docs/hyperv.txt
+++ b/docs/hyperv.txt
@@ -214,7 +214,7 @@ exposing correct vCPU topology and vCPU pinning.
 3.20. hv-version-id-{build,major,minor,spack,sbranch,snumber}
 =============================================================
 This changes Hyper-V version identification in CPUID 0x40000002.EAX-EDX from the
-default (WS2008R2).
+default (WS2016).
 - hv-version-id-build sets 'Build Number' (32 bits)
 - hv-version-id-major sets 'Major Version' (16 bits)
 - hv-version-id-minor sets 'Minor Version' (16 bits)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 28e1b83b9d..86223acfd3 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -93,7 +93,11 @@
 #include "trace.h"
 #include CONFIG_DEVICES
 
-GlobalProperty pc_compat_6_1[] = {};
+GlobalProperty pc_compat_6_1[] = {
+    { TYPE_X86_CPU, "hv-version-id-build", "0x1bbc" },
+    { TYPE_X86_CPU, "hv-version-id-major", "0x0006" },
+    { TYPE_X86_CPU, "hv-version-id-minor", "0x0001" },
+};
 const size_t pc_compat_6_1_len = G_N_ELEMENTS(pc_compat_6_1);
 
 GlobalProperty pc_compat_6_0[] = {
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index d1d057fabe..a7b1b6aa93 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6838,11 +6838,11 @@ static Property x86_cpu_properties[] = {
 
     /* WS2008R2 identify by default */
     DEFINE_PROP_UINT32("hv-version-id-build", X86CPU, hyperv_ver_id_build,
-                       0x1bbc),
+                       0x3839),
     DEFINE_PROP_UINT16("hv-version-id-major", X86CPU, hyperv_ver_id_major,
-                       0x0006),
+                       0x000A),
     DEFINE_PROP_UINT16("hv-version-id-minor", X86CPU, hyperv_ver_id_minor,
-                       0x0001),
+                       0x0000),
     DEFINE_PROP_UINT32("hv-version-id-spack", X86CPU, hyperv_ver_id_sp, 0),
     DEFINE_PROP_UINT8("hv-version-id-sbranch", X86CPU, hyperv_ver_id_sb, 0),
     DEFINE_PROP_UINT32("hv-version-id-snumber", X86CPU, hyperv_ver_id_sn, 0),
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 23/30] configure: Loosen GCC requirement from 7.5.0 to 7.4.0
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (21 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 22/30] i386: Change the default Hyper-V version to match WS2016 Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 24/30] virtio-mem-pci: Fix memory leak when creating MEMORY_DEVICE_SIZE_CHANGE event Paolo Bonzini
                   ` (8 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: nia, Richard Henderson

From: nia <nia@NetBSD.org>

As discussed in issue 614, we're shipping GCC 7.4.0 as the
system compiler in NetBSD 9, the most recent stable branch,
and are still actively interested in QEMU on this platform.

The differences between GCC 7.5.0 and 7.4.0 are trivial.

Signed-off-by: Nia Alarie <nia@NetBSD.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <YVcpe79I0rly1HJh@homeworld.netbsd.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 1043ccce4f..b0b1a1cc25 100755
--- a/configure
+++ b/configure
@@ -2094,8 +2094,8 @@ cat > $TMPC << EOF
 #  endif
 # endif
 #elif defined(__GNUC__) && defined(__GNUC_MINOR__)
-# if __GNUC__ < 7 || (__GNUC__ == 7 && __GNUC_MINOR__ < 5)
-#  error You need at least GCC v7.5.0 to compile QEMU
+# if __GNUC__ < 7 || (__GNUC__ == 7 && __GNUC_MINOR__ < 4)
+#  error You need at least GCC v7.4.0 to compile QEMU
 # endif
 #else
 # error You either need GCC or Clang to compiler QEMU
@@ -2103,7 +2103,7 @@ cat > $TMPC << EOF
 int main (void) { return 0; }
 EOF
 if ! compile_prog "" "" ; then
-    error_exit "You need at least GCC v7.5 or Clang v6.0 (or XCode Clang v10.0)"
+    error_exit "You need at least GCC v7.4 or Clang v6.0 (or XCode Clang v10.0)"
 fi
 
 # Accumulate -Wfoo and -Wno-bar separately.
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 24/30] virtio-mem-pci: Fix memory leak when creating MEMORY_DEVICE_SIZE_CHANGE event
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (22 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 23/30] configure: Loosen GCC requirement from 7.5.0 to 7.4.0 Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 25/30] qapi: Include qom-path in MEMORY_DEVICE_SIZE_CHANGE qapi events Paolo Bonzini
                   ` (7 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Markus Armbruster, qemu-stable, David Hildenbrand

From: David Hildenbrand <david@redhat.com>

Apparently, we don't have to duplicate the string.

Fixes: 722a3c783ef4 ("virtio-pci: Send qapi events when the virtio-mem size changes")
Cc: qemu-stable@nongnu.org
Signed-off-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210929162445.64060-2-david@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio/virtio-mem-pci.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/hw/virtio/virtio-mem-pci.c b/hw/virtio/virtio-mem-pci.c
index fa5395cd88..7e384b7397 100644
--- a/hw/virtio/virtio-mem-pci.c
+++ b/hw/virtio/virtio-mem-pci.c
@@ -88,13 +88,8 @@ static void virtio_mem_pci_size_change_notify(Notifier *notifier, void *data)
                                          size_change_notifier);
     DeviceState *dev = DEVICE(pci_mem);
     const uint64_t * const size_p = data;
-    const char *id = NULL;
 
-    if (dev->id) {
-        id = g_strdup(dev->id);
-    }
-
-    qapi_event_send_memory_device_size_change(!!id, id, *size_p);
+    qapi_event_send_memory_device_size_change(!!dev->id, dev->id, *size_p);
 }
 
 static void virtio_mem_pci_class_init(ObjectClass *klass, void *data)
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 25/30] qapi: Include qom-path in MEMORY_DEVICE_SIZE_CHANGE qapi events
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (23 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 24/30] virtio-mem-pci: Fix memory leak when creating MEMORY_DEVICE_SIZE_CHANGE event Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 26/30] monitor: Rate-limit MEMORY_DEVICE_SIZE_CHANGE qapi events per device Paolo Bonzini
                   ` (6 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Markus Armbruster, David Hildenbrand

From: David Hildenbrand <david@redhat.com>

As we might not always have a device id, it is impossible to always
match MEMORY_DEVICE_SIZE_CHANGE events to an actual device. Let's
include the qom-path in the event, which allows for reliable mapping of
events to devices.

Fixes: 722a3c783ef4 ("virtio-pci: Send qapi events when the virtio-mem size changes")
Suggested-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20210929162445.64060-3-david@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio/virtio-mem-pci.c | 5 ++++-
 qapi/machine.json          | 5 ++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-mem-pci.c b/hw/virtio/virtio-mem-pci.c
index 7e384b7397..be2383b0c5 100644
--- a/hw/virtio/virtio-mem-pci.c
+++ b/hw/virtio/virtio-mem-pci.c
@@ -87,9 +87,12 @@ static void virtio_mem_pci_size_change_notify(Notifier *notifier, void *data)
     VirtIOMEMPCI *pci_mem = container_of(notifier, VirtIOMEMPCI,
                                          size_change_notifier);
     DeviceState *dev = DEVICE(pci_mem);
+    char *qom_path = object_get_canonical_path(OBJECT(dev));
     const uint64_t * const size_p = data;
 
-    qapi_event_send_memory_device_size_change(!!dev->id, dev->id, *size_p);
+    qapi_event_send_memory_device_size_change(!!dev->id, dev->id, *size_p,
+                                              qom_path);
+    g_free(qom_path);
 }
 
 static void virtio_mem_pci_class_init(ObjectClass *klass, void *data)
diff --git a/qapi/machine.json b/qapi/machine.json
index 0e91a57a76..5db54df298 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1336,8 +1336,11 @@
 # action).
 #
 # @id: device's ID
+#
 # @size: the new size of memory that the device provides
 #
+# @qom-path: path to the device object in the QOM tree (since 6.2)
+#
 # Note: this event is rate-limited.
 #
 # Since: 5.1
@@ -1350,7 +1353,7 @@
 #
 ##
 { 'event': 'MEMORY_DEVICE_SIZE_CHANGE',
-  'data': { '*id': 'str', 'size': 'size' } }
+  'data': { '*id': 'str', 'size': 'size', 'qom-path' : 'str'} }
 
 
 ##
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 26/30] monitor: Rate-limit MEMORY_DEVICE_SIZE_CHANGE qapi events per device
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (24 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 25/30] qapi: Include qom-path in MEMORY_DEVICE_SIZE_CHANGE qapi events Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 27/30] tpm: mark correct memory region range dirty when clearing RAM Paolo Bonzini
                   ` (5 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: Markus Armbruster, David Hildenbrand

From: David Hildenbrand <david@redhat.com>

We want to rate-limit MEMORY_DEVICE_SIZE_CHANGE events per device,
otherwise we can lose some events for devices. We can now use the
qom-path to reliably map an event to a device and make rate-limiting
device-aware.

This was noticed by starting a VM with two virtio-mem devices that each
have a requested size > 0. The Linux guest will initialize both devices
in parallel, resulting in losing MEMORY_DEVICE_SIZE_CHANGE events for
one of the devices.

Fixes: 722a3c783ef4 ("virtio-pci: Send qapi events when the virtio-mem size changes")
Suggested-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20210929162445.64060-4-david@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 monitor/monitor.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/monitor/monitor.c b/monitor/monitor.c
index 46a171bca6..21c7a68758 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -474,6 +474,10 @@ static unsigned int qapi_event_throttle_hash(const void *key)
         hash += g_str_hash(qdict_get_str(evstate->data, "node-name"));
     }
 
+    if (evstate->event == QAPI_EVENT_MEMORY_DEVICE_SIZE_CHANGE) {
+        hash += g_str_hash(qdict_get_str(evstate->data, "qom-path"));
+    }
+
     return hash;
 }
 
@@ -496,6 +500,11 @@ static gboolean qapi_event_throttle_equal(const void *a, const void *b)
                        qdict_get_str(evb->data, "node-name"));
     }
 
+    if (eva->event == QAPI_EVENT_MEMORY_DEVICE_SIZE_CHANGE) {
+        return !strcmp(qdict_get_str(eva->data, "qom-path"),
+                       qdict_get_str(evb->data, "qom-path"));
+    }
+
     return TRUE;
 }
 
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 27/30] tpm: mark correct memory region range dirty when clearing RAM
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (25 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 26/30] monitor: Rate-limit MEMORY_DEVICE_SIZE_CHANGE qapi events per device Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 28/30] softmmu/memory_mapping: never merge ranges accross memory regions Paolo Bonzini
                   ` (4 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Thomas Huth, Stefan Berger, Eduardo Habkost,
	Michael S. Tsirkin, David Hildenbrand, Dr . David Alan Gilbert,
	Peter Xu, Alex Williamson, Claudio Fontana, Igor Mammedov,
	Marc-André Lureau, Alex Bennée, Stefan Berger

From: David Hildenbrand <david@redhat.com>

We might not start at the beginning of the memory region. Let's
calculate the offset into the memory region via the difference in the
host addresses.

Acked-by: Stefan Berger <stefanb@linux.ibm.com>
Fixes: ffab1be70692 ("tpm: clear RAM when "memory overwrite" requested")
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Claudio Fontana <cfontana@suse.de>
Cc: Thomas Huth <thuth@redhat.com>
Cc: "Alex Bennée" <alex.bennee@linaro.org>
Cc: Peter Xu <peterx@redhat.com>
Cc: Laurent Vivier <lvivier@redhat.com>
Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Message-Id: <20210727082545.17934-2-david@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/tpm/tpm_ppi.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
index 362edcc5c9..274e9aa4b0 100644
--- a/hw/tpm/tpm_ppi.c
+++ b/hw/tpm/tpm_ppi.c
@@ -30,11 +30,14 @@ void tpm_ppi_reset(TPMPPI *tpmppi)
         guest_phys_blocks_init(&guest_phys_blocks);
         guest_phys_blocks_append(&guest_phys_blocks);
         QTAILQ_FOREACH(block, &guest_phys_blocks.head, next) {
+            hwaddr mr_offs = block->host_addr -
+                             (uint8_t *)memory_region_get_ram_ptr(block->mr);
+
             trace_tpm_ppi_memset(block->host_addr,
                                  block->target_end - block->target_start);
             memset(block->host_addr, 0,
                    block->target_end - block->target_start);
-            memory_region_set_dirty(block->mr, 0,
+            memory_region_set_dirty(block->mr, mr_offs,
                                     block->target_end - block->target_start);
         }
         guest_phys_blocks_free(&guest_phys_blocks);
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 28/30] softmmu/memory_mapping: never merge ranges accross memory regions
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (26 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 27/30] tpm: mark correct memory region range dirty when clearing RAM Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 29/30] softmmu/memory_mapping: factor out adding physical memory ranges Paolo Bonzini
                   ` (3 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Thomas Huth, Eduardo Habkost, Michael S. Tsirkin,
	David Hildenbrand, Dr . David Alan Gilbert, Peter Xu,
	Alex Williamson, Claudio Fontana, Igor Mammedov,
	Marc-André Lureau, Alex Bennée, Stefan Berger

From: David Hildenbrand <david@redhat.com>

Let's make sure to not merge when different memory regions are involved.
Unlikely, but theoretically possible.

Acked-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Claudio Fontana <cfontana@suse.de>
Cc: Thomas Huth <thuth@redhat.com>
Cc: "Alex Bennée" <alex.bennee@linaro.org>
Cc: Peter Xu <peterx@redhat.com>
Cc: Laurent Vivier <lvivier@redhat.com>
Cc: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20210727082545.17934-3-david@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 softmmu/memory_mapping.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/softmmu/memory_mapping.c b/softmmu/memory_mapping.c
index e7af276546..d401ca7e31 100644
--- a/softmmu/memory_mapping.c
+++ b/softmmu/memory_mapping.c
@@ -229,7 +229,8 @@ static void guest_phys_blocks_region_add(MemoryListener *listener,
 
         /* we want continuity in both guest-physical and host-virtual memory */
         if (predecessor->target_end < target_start ||
-            predecessor->host_addr + predecessor_size != host_addr) {
+            predecessor->host_addr + predecessor_size != host_addr ||
+            predecessor->mr != section->mr) {
             predecessor = NULL;
         }
     }
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 29/30] softmmu/memory_mapping: factor out adding physical memory ranges
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (27 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 28/30] softmmu/memory_mapping: never merge ranges accross memory regions Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03  7:42 ` [PULL 30/30] softmmu/memory_mapping: optimize for RamDiscardManager sections Paolo Bonzini
                   ` (2 subsequent siblings)
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Thomas Huth, Eduardo Habkost, Michael S. Tsirkin,
	David Hildenbrand, Dr . David Alan Gilbert, Peter Xu,
	Alex Williamson, Claudio Fontana, Igor Mammedov,
	Marc-André Lureau, Alex Bennée, Stefan Berger

From: David Hildenbrand <david@redhat.com>

Let's factor out adding a MemoryRegionSection to the list, to be reused in
RamDiscardManager context next.

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Claudio Fontana <cfontana@suse.de>
Cc: Thomas Huth <thuth@redhat.com>
Cc: "Alex Bennée" <alex.bennee@linaro.org>
Cc: Peter Xu <peterx@redhat.com>
Cc: Laurent Vivier <lvivier@redhat.com>
Cc: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20210727082545.17934-4-david@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 softmmu/memory_mapping.c | 41 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/softmmu/memory_mapping.c b/softmmu/memory_mapping.c
index d401ca7e31..a2af02c41c 100644
--- a/softmmu/memory_mapping.c
+++ b/softmmu/memory_mapping.c
@@ -193,29 +193,14 @@ typedef struct GuestPhysListener {
     MemoryListener listener;
 } GuestPhysListener;
 
-static void guest_phys_blocks_region_add(MemoryListener *listener,
+static void guest_phys_block_add_section(GuestPhysListener *g,
                                          MemoryRegionSection *section)
 {
-    GuestPhysListener *g;
-    uint64_t section_size;
-    hwaddr target_start, target_end;
-    uint8_t *host_addr;
-    GuestPhysBlock *predecessor;
-
-    /* we only care about RAM */
-    if (!memory_region_is_ram(section->mr) ||
-        memory_region_is_ram_device(section->mr) ||
-        memory_region_is_nonvolatile(section->mr)) {
-        return;
-    }
-
-    g            = container_of(listener, GuestPhysListener, listener);
-    section_size = int128_get64(section->size);
-    target_start = section->offset_within_address_space;
-    target_end   = target_start + section_size;
-    host_addr    = memory_region_get_ram_ptr(section->mr) +
-                   section->offset_within_region;
-    predecessor  = NULL;
+    const hwaddr target_start = section->offset_within_address_space;
+    const hwaddr target_end = target_start + int128_get64(section->size);
+    uint8_t *host_addr = memory_region_get_ram_ptr(section->mr) +
+                         section->offset_within_region;
+    GuestPhysBlock *predecessor = NULL;
 
     /* find continuity in guest physical address space */
     if (!QTAILQ_EMPTY(&g->list->head)) {
@@ -261,6 +246,20 @@ static void guest_phys_blocks_region_add(MemoryListener *listener,
 #endif
 }
 
+static void guest_phys_blocks_region_add(MemoryListener *listener,
+                                         MemoryRegionSection *section)
+{
+    GuestPhysListener *g = container_of(listener, GuestPhysListener, listener);
+
+    /* we only care about RAM */
+    if (!memory_region_is_ram(section->mr) ||
+        memory_region_is_ram_device(section->mr) ||
+        memory_region_is_nonvolatile(section->mr)) {
+        return;
+    }
+    guest_phys_block_add_section(g, section);
+}
+
 void guest_phys_blocks_append(GuestPhysBlockList *list)
 {
     GuestPhysListener g = { 0 };
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PULL 30/30] softmmu/memory_mapping: optimize for RamDiscardManager sections
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (28 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 29/30] softmmu/memory_mapping: factor out adding physical memory ranges Paolo Bonzini
@ 2021-10-03  7:42 ` Paolo Bonzini
  2021-10-03 14:44 ` [PULL 00/30] Misc changes for 2021-10-03 Philippe Mathieu-Daudé
  2021-10-03 15:12 ` Richard Henderson
  31 siblings, 0 replies; 33+ messages in thread
From: Paolo Bonzini @ 2021-10-03  7:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Thomas Huth, Eduardo Habkost, Michael S. Tsirkin,
	David Hildenbrand, Dr . David Alan Gilbert, Peter Xu,
	Alex Williamson, Claudio Fontana, Igor Mammedov,
	Marc-André Lureau, Alex Bennée, Stefan Berger

From: David Hildenbrand <david@redhat.com>

virtio-mem logically plugs/unplugs memory within a sparse memory region
and notifies via the RamDiscardManager interface when parts become
plugged (populated) or unplugged (discarded).

Currently, we end up (via the two users)
1) zeroing all logically unplugged/discarded memory during TPM resets.
2) reading all logically unplugged/discarded memory when dumping, to
   figure out the content is zero.

1) is always bad, because we assume unplugged memory stays discarded
   (and is already implicitly zero).
2) isn't that bad with anonymous memory, we end up reading the zero
   page (slow and unnecessary, though). However, once we use some
   file-backed memory (future use case), even reading will populate memory.

Let's cut out all parts marked as not-populated (discarded) via the
RamDiscardManager. As virtio-mem is the single user, this now means that
logically unplugged memory ranges will no longer be included in the
dump, which results in smaller dump files and faster dumping.

virtio-mem has a minimum granularity of 1 MiB (and the default is usually
2 MiB). Theoretically, we can see quite some fragmentation, in practice
we won't have it completely fragmented in 1 MiB pieces. Still, we might
end up with many physical ranges.

Both, the ELF format and kdump seem to be ready to support many
individual ranges (e.g., for ELF it seems to be UINT32_MAX, kdump has a
linear bitmap).

Reviewed-by: Peter Xu <peterx@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Claudio Fontana <cfontana@suse.de>
Cc: Thomas Huth <thuth@redhat.com>
Cc: "Alex Bennée" <alex.bennee@linaro.org>
Cc: Peter Xu <peterx@redhat.com>
Cc: Laurent Vivier <lvivier@redhat.com>
Cc: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20210727082545.17934-5-david@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 softmmu/memory_mapping.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/softmmu/memory_mapping.c b/softmmu/memory_mapping.c
index a2af02c41c..a62eaa49cc 100644
--- a/softmmu/memory_mapping.c
+++ b/softmmu/memory_mapping.c
@@ -246,6 +246,15 @@ static void guest_phys_block_add_section(GuestPhysListener *g,
 #endif
 }
 
+static int guest_phys_ram_populate_cb(MemoryRegionSection *section,
+                                      void *opaque)
+{
+    GuestPhysListener *g = opaque;
+
+    guest_phys_block_add_section(g, section);
+    return 0;
+}
+
 static void guest_phys_blocks_region_add(MemoryListener *listener,
                                          MemoryRegionSection *section)
 {
@@ -257,6 +266,17 @@ static void guest_phys_blocks_region_add(MemoryListener *listener,
         memory_region_is_nonvolatile(section->mr)) {
         return;
     }
+
+    /* for special sparse regions, only add populated parts */
+    if (memory_region_has_ram_discard_manager(section->mr)) {
+        RamDiscardManager *rdm;
+
+        rdm = memory_region_get_ram_discard_manager(section->mr);
+        ram_discard_manager_replay_populated(rdm, section,
+                                             guest_phys_ram_populate_cb, g);
+        return;
+    }
+
     guest_phys_block_add_section(g, section);
 }
 
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PULL 00/30] Misc changes for 2021-10-03
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (29 preceding siblings ...)
  2021-10-03  7:42 ` [PULL 30/30] softmmu/memory_mapping: optimize for RamDiscardManager sections Paolo Bonzini
@ 2021-10-03 14:44 ` Philippe Mathieu-Daudé
  2021-10-03 15:12 ` Richard Henderson
  31 siblings, 0 replies; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-10-03 14:44 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: Nia Alarie, Richard Henderson

On 10/3/21 09:42, Paolo Bonzini wrote:
> The following changes since commit bb4aa8f59e18412cff0d69f14aee7abba153161a:
> 
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20210930' into staging (2021-09-30 21:16:54 +0100)
> 
> are available in the Git repository at:
> 
>   https://gitlab.com/bonzini/qemu.git tags/for-upstream
> 
> for you to fetch changes up to cb83ba8c1ab856b4327e7e869c410bbfd4152c2c:
> 
>   softmmu/memory_mapping: optimize for RamDiscardManager sections (2021-10-02 08:43:22 +0200)
> 
> ----------------------------------------------------------------

> nia (1):
>       configure: Loosen GCC requirement from 7.5.0 to 7.4.0

The patch is 'Signed-off-by: Nia Alarie <nia@NetBSD.org>' so I suppose
there is a git-config problem somewhere.

I don't want to delay this pull request, but it would be nice to fix
this in a follow-up .mailmap patch, to comply with our guidelines:

  If you wrote the patch, make sure your "From:" and
  "Signed-off-by:" lines use the same spelling."

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



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PULL 00/30] Misc changes for 2021-10-03
  2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
                   ` (30 preceding siblings ...)
  2021-10-03 14:44 ` [PULL 00/30] Misc changes for 2021-10-03 Philippe Mathieu-Daudé
@ 2021-10-03 15:12 ` Richard Henderson
  31 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2021-10-03 15:12 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel

On 10/3/21 3:42 AM, Paolo Bonzini wrote:
> The following changes since commit bb4aa8f59e18412cff0d69f14aee7abba153161a:
> 
>    Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20210930' into staging (2021-09-30 21:16:54 +0100)
> 
> are available in the Git repository at:
> 
>    https://gitlab.com/bonzini/qemu.git  tags/for-upstream
> 
> for you to fetch changes up to cb83ba8c1ab856b4327e7e869c410bbfd4152c2c:
> 
>    softmmu/memory_mapping: optimize for RamDiscardManager sections (2021-10-02 08:43:22 +0200)
> 
> ----------------------------------------------------------------
> * -smp cleanpus (Yanan)
> * Hyper-V enlightenment functionality (Vitaly)
> * virtio-mem support in dump, tpm and QMP (David)
> * NetBSD GCC 7.4 compiler support (Nia)

Applied, thanks.

r~


^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2021-10-03 15:14 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-03  7:42 [PULL 00/30] Misc changes for 2021-10-03 Paolo Bonzini
2021-10-03  7:42 ` [PULL 01/30] qapi/machine: Fix an incorrect comment of SMPConfiguration Paolo Bonzini
2021-10-03  7:42 ` [PULL 02/30] machine: Deprecate "parameter=0" SMP configurations Paolo Bonzini
2021-10-03  7:42 ` [PULL 03/30] machine: Minor refactor/fix for the smp parsers Paolo Bonzini
2021-10-03  7:42 ` [PULL 04/30] machine: Uniformly use maxcpus to calculate the omitted parameters Paolo Bonzini
2021-10-03  7:42 ` [PULL 05/30] machine: Set the value of cpus to match maxcpus if it's omitted Paolo Bonzini
2021-10-03  7:42 ` [PULL 06/30] machine: Improve the error reporting of smp parsing Paolo Bonzini
2021-10-03  7:42 ` [PULL 07/30] qtest/numa-test: Use detailed -smp CLIs in pc_dynamic_cpu_cfg Paolo Bonzini
2021-10-03  7:42 ` [PULL 08/30] qtest/numa-test: Use detailed -smp CLIs in test_def_cpu_split Paolo Bonzini
2021-10-03  7:42 ` [PULL 09/30] machine: Prefer cores over sockets in smp parsing since 6.2 Paolo Bonzini
2021-10-03  7:42 ` [PULL 10/30] machine: Use ms instead of global current_machine in sanity-check Paolo Bonzini
2021-10-03  7:42 ` [PULL 11/30] machine: Tweak the order of topology members in struct CpuTopology Paolo Bonzini
2021-10-03  7:42 ` [PULL 12/30] machine: Make smp_parse generic enough for all arches Paolo Bonzini
2021-10-03  7:42 ` [PULL 13/30] machine: Remove smp_parse callback from MachineClass Paolo Bonzini
2021-10-03  7:42 ` [PULL 14/30] machine: Move smp_prefer_sockets to struct SMPCompatProps Paolo Bonzini
2021-10-03  7:42 ` [PULL 15/30] machine: Use g_autoptr in machine_set_smp Paolo Bonzini
2021-10-03  7:42 ` [PULL 16/30] machine: Put all sanity-check in the generic SMP parser Paolo Bonzini
2021-10-03  7:42 ` [PULL 17/30] i386: Support KVM_CAP_ENFORCE_PV_FEATURE_CPUID Paolo Bonzini
2021-10-03  7:42 ` [PULL 18/30] i386: Support KVM_CAP_HYPERV_ENFORCE_CPUID Paolo Bonzini
2021-10-03  7:42 ` [PULL 19/30] i386: Move HV_APIC_ACCESS_RECOMMENDED bit setting to hyperv_fill_cpuids() Paolo Bonzini
2021-10-03  7:42 ` [PULL 20/30] i386: Implement pseudo 'hv-avic' ('hv-apicv') enlightenment Paolo Bonzini
2021-10-03  7:42 ` [PULL 21/30] i386: Make Hyper-V version id configurable Paolo Bonzini
2021-10-03  7:42 ` [PULL 22/30] i386: Change the default Hyper-V version to match WS2016 Paolo Bonzini
2021-10-03  7:42 ` [PULL 23/30] configure: Loosen GCC requirement from 7.5.0 to 7.4.0 Paolo Bonzini
2021-10-03  7:42 ` [PULL 24/30] virtio-mem-pci: Fix memory leak when creating MEMORY_DEVICE_SIZE_CHANGE event Paolo Bonzini
2021-10-03  7:42 ` [PULL 25/30] qapi: Include qom-path in MEMORY_DEVICE_SIZE_CHANGE qapi events Paolo Bonzini
2021-10-03  7:42 ` [PULL 26/30] monitor: Rate-limit MEMORY_DEVICE_SIZE_CHANGE qapi events per device Paolo Bonzini
2021-10-03  7:42 ` [PULL 27/30] tpm: mark correct memory region range dirty when clearing RAM Paolo Bonzini
2021-10-03  7:42 ` [PULL 28/30] softmmu/memory_mapping: never merge ranges accross memory regions Paolo Bonzini
2021-10-03  7:42 ` [PULL 29/30] softmmu/memory_mapping: factor out adding physical memory ranges Paolo Bonzini
2021-10-03  7:42 ` [PULL 30/30] softmmu/memory_mapping: optimize for RamDiscardManager sections Paolo Bonzini
2021-10-03 14:44 ` [PULL 00/30] Misc changes for 2021-10-03 Philippe Mathieu-Daudé
2021-10-03 15:12 ` Richard Henderson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.