[PATCH for-6.0 3/9] spapr: Fix pre-2.10 dummy ICP hack

Greg Kurz posted 9 patches 5 years, 2 months ago
Maintainers: David Gibson <david@gibson.dropbear.id.au>
There is a newer version of this series
[PATCH for-6.0 3/9] spapr: Fix pre-2.10 dummy ICP hack
Posted by Greg Kurz 5 years, 2 months ago
This hack registers dummy VMState entries of ICPs in order to
support migration of old pseries machine types that used to
create all smp.max_cpus possible ICPs at machine init.

Part of the work is to unregister the dummy entries when plugging
an actual vCPU core, and to register them back when unplugging the
core. The code that unregisters the dummy ICPs in spapr_core_plug()
is misplaced: if ppc_set_compat() fails afterwards, the hotplug
operation will be cancelled and the dummy ICPs won't be registered
back since the unplug handler isn't called.

Unregister the dummy ICPs at the end of spapr_core_plug().

Signed-off-by: Greg Kurz <groug@kaod.org>
---

The next patch makes this patch a bit useless. I post it
anyway for the records because it is a programming error.
---
 hw/ppc/spapr.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 394d28d9e081..f58f77389e8e 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3785,13 +3785,6 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
 
     core_slot->cpu = OBJECT(dev);
 
-    if (smc->pre_2_10_has_unused_icps) {
-        for (i = 0; i < cc->nr_threads; i++) {
-            cs = CPU(core->threads[i]);
-            pre_2_10_vmstate_unregister_dummy_icp(cs->cpu_index);
-        }
-    }
-
     /*
      * Set compatibility mode to match the boot CPU, which was either set
      * by the machine reset code or by CAS.
@@ -3805,6 +3798,13 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
             }
         }
     }
+
+    if (smc->pre_2_10_has_unused_icps) {
+        for (i = 0; i < cc->nr_threads; i++) {
+            cs = CPU(core->threads[i]);
+            pre_2_10_vmstate_unregister_dummy_icp(cs->cpu_index);
+        }
+    }
 }
 
 static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
-- 
2.26.2


Re: [PATCH for-6.0 3/9] spapr: Fix pre-2.10 dummy ICP hack
Posted by David Gibson 5 years, 2 months ago
On Sat, Nov 21, 2020 at 12:42:02AM +0100, Greg Kurz wrote:
> This hack registers dummy VMState entries of ICPs in order to
> support migration of old pseries machine types that used to
> create all smp.max_cpus possible ICPs at machine init.
> 
> Part of the work is to unregister the dummy entries when plugging
> an actual vCPU core, and to register them back when unplugging the
> core. The code that unregisters the dummy ICPs in spapr_core_plug()
> is misplaced: if ppc_set_compat() fails afterwards, the hotplug
> operation will be cancelled and the dummy ICPs won't be registered
> back since the unplug handler isn't called.
> 
> Unregister the dummy ICPs at the end of spapr_core_plug().
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Applied to ppc-for-6.0, thanks.

> ---
> 
> The next patch makes this patch a bit useless. I post it
> anyway for the records because it is a programming error.
> ---
>  hw/ppc/spapr.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 394d28d9e081..f58f77389e8e 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3785,13 +3785,6 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>  
>      core_slot->cpu = OBJECT(dev);
>  
> -    if (smc->pre_2_10_has_unused_icps) {
> -        for (i = 0; i < cc->nr_threads; i++) {
> -            cs = CPU(core->threads[i]);
> -            pre_2_10_vmstate_unregister_dummy_icp(cs->cpu_index);
> -        }
> -    }
> -
>      /*
>       * Set compatibility mode to match the boot CPU, which was either set
>       * by the machine reset code or by CAS.
> @@ -3805,6 +3798,13 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>              }
>          }
>      }
> +
> +    if (smc->pre_2_10_has_unused_icps) {
> +        for (i = 0; i < cc->nr_threads; i++) {
> +            cs = CPU(core->threads[i]);
> +            pre_2_10_vmstate_unregister_dummy_icp(cs->cpu_index);
> +        }
> +    }
>  }
>  
>  static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,

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