[PATCH 0/4] spapr: Get rid of CAS reboot flag

[PATCH 0/4] spapr: Get rid of CAS reboot flag

[Greg Kurz]

The CAS reboot flag was introduced in QEMU 2.10 to allow the guest
to be presented a new boot-time device tree after CAS negotiation.
CAS-generated resets rely on qemu_system_reset_request() which has
the particularity of dropping the main loop lock at some point. This
opens a window where migration can happen, hence promotting the CAS
reboot flag to actual state that we should also migrate. In practice,
this can't happen anymore since we have eliminated the scenario of
the XICS/XIVE switch and the much less frequent scenario of device
plug/unplug before CAS.

We still have much of the CAS reboot bits around though. The full FDT
rendering we do at CAS is enough to get rid of them once and far all.

Some preliminary cleanup is made before going for the full removal,
for easier reviewing. At some point I had the need to move some code
around in CAS, and Alexey's patch from the "spapr: kill SLOF" (v8)
series proved to be helpful so I've reused it in this patchset.

This series applies cleanly on both ppc-for-5.0 and ppc-for-5.1.
Since it doesn't fix any actual bug, I think this can be delayed
to 5.1.

--
Greg

---

Alexey Kardashevskiy (1):
      spapr/cas: Separate CAS handling from rebuilding the FDT

Greg Kurz (3):
      spapr: Don't check capabilities removed between CAS calls
      spapr: Simplify selection of radix/hash during CAS
      spapr: Drop CAS reboot flag


 hw/ppc/spapr.c         |   20 ++-------
 hw/ppc/spapr_hcall.c   |  108 +++++++++++++++++++++++-------------------------
 include/hw/ppc/spapr.h |    8 +++-
 3 files changed, 63 insertions(+), 73 deletions(-)

[PATCH 1/4] spapr: Don't check capabilities removed between CAS calls

[Greg Kurz]

We currently check if some capability in OV5 was removed by the guest
since the previous CAS, and we trigger a CAS reboot in that case. This
was required because it could call for a device-tree property or node
removal, that we didn't support until recently (see commit 6787d27b04a7
"spapr: add option vector handling in CAS-generated resets" for details).

Now that we render a full FDT at CAS and that SLOF is able to handle
node removal, we don't need to do a CAS reset in this case anymore.
Also, this check can only return true if the guest has already called
CAS since the last full system reset (otherwise spapr->ov5_cas is
empty). Linux doesn't do that so this can be considered as dead code
for the vast majority of existing setups.

Drop the check. Since the only use of the ov5_cas_old variable is
precisely the check itself, drop the variable as well.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr_hcall.c |   14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 40c86e91eb89..1f123a68e46c 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1676,7 +1676,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
     target_ulong fdt_bufsize = args[2];
     target_ulong ov_table;
     uint32_t cas_pvr;
-    SpaprOptionVector *ov1_guest, *ov5_guest, *ov5_cas_old;
+    SpaprOptionVector *ov1_guest, *ov5_guest;
     bool guest_radix;
     Error *local_err = NULL;
     bool raw_mode_supported = false;
@@ -1781,22 +1781,10 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
      * by LoPAPR 1.1, 14.5.4.8, which QEMU doesn't implement, we don't need
      * to worry about this for now.
      */
-    ov5_cas_old = spapr_ovec_clone(spapr->ov5_cas);
-
-    /* also clear the radix/hash bit from the current ov5_cas bits to
-     * be in sync with the newly ov5 bits. Else the radix bit will be
-     * seen as being removed and this will generate a reset loop
-     */
-    spapr_ovec_clear(ov5_cas_old, OV5_MMU_RADIX_300);
 
     /* full range of negotiated ov5 capabilities */
     spapr_ovec_intersect(spapr->ov5_cas, spapr->ov5, ov5_guest);
     spapr_ovec_cleanup(ov5_guest);
-    /* capabilities that have been added since CAS-generated guest reset.
-     * if capabilities have since been removed, generate another reset
-     */
-    spapr->cas_reboot = !spapr_ovec_subset(ov5_cas_old, spapr->ov5_cas);
-    spapr_ovec_cleanup(ov5_cas_old);
     /* Now that processing is finished, set the radix/hash bit for the
      * guest if it requested a valid mode; otherwise terminate the boot. */
     if (guest_radix) {

[PATCH 2/4] spapr: Simplify selection of radix/hash during CAS

[Greg Kurz]

The guest can select the MMU mode by setting bits 0-1 of byte 24
in OV5 to to 0b00 for hash or 0b01 for radix. As required by the
architecture, we terminate the boot process if any other value
is found there.

The usual way to negotiate features in OV5 is basically ANDing
the bitfield provided by the guest and the bitfield of features
supported by QEMU, previously populated at machine init.

For some not documented reason, MMU is treated differently : bit 1
of byte 24 (the radix/hash bit) is cleared from the guest OV5 and
explicitely set in the final negotiated OV5 if radix was requested.

Since the only expected input from the guest is the radix/hash bit
being set or not, it seems more appropriate to handle this like we
do for XIVE.

Set the radix bit in spapr->ov5 at machine init if it has a chance
to work (ie. power9, either TCG or a radix capable KVM) and rely
exclusively on spapr_ovec_intersect() to set the radix bit in
spapr->ov5_cas.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr.c       |    1 +
 hw/ppc/spapr_hcall.c |    6 +-----
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 9a2bd501aa6e..91f8cfe7a8b1 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2837,6 +2837,7 @@ static void spapr_machine_init(MachineState *machine)
     if ((!kvm_enabled() || kvmppc_has_cap_mmu_radix()) &&
         ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00, 0,
                               spapr->max_compat_pvr)) {
+        spapr_ovec_set(spapr->ov5, OV5_MMU_RADIX_300);
         /* KVM and TCG always allow GTSE with radix... */
         spapr_ovec_set(spapr->ov5, OV5_MMU_RADIX_GTSE);
     }
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 1f123a68e46c..9bed659b1a94 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1738,9 +1738,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
         exit(EXIT_FAILURE);
     }
 
-    /* The radix/hash bit in byte 24 requires special handling: */
     guest_radix = spapr_ovec_test(ov5_guest, OV5_MMU_RADIX_300);
-    spapr_ovec_clear(ov5_guest, OV5_MMU_RADIX_300);
 
     guest_xive = spapr_ovec_test(ov5_guest, OV5_XIVE_EXPLOIT);
 
@@ -1785,14 +1783,12 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
     /* full range of negotiated ov5 capabilities */
     spapr_ovec_intersect(spapr->ov5_cas, spapr->ov5, ov5_guest);
     spapr_ovec_cleanup(ov5_guest);
-    /* Now that processing is finished, set the radix/hash bit for the
-     * guest if it requested a valid mode; otherwise terminate the boot. */
+
     if (guest_radix) {
         if (kvm_enabled() && !kvmppc_has_cap_mmu_radix()) {
             error_report("Guest requested unavailable MMU mode (radix).");
             exit(EXIT_FAILURE);
         }
-        spapr_ovec_set(spapr->ov5_cas, OV5_MMU_RADIX_300);
     } else {
         if (kvm_enabled() && kvmppc_has_cap_mmu_radix()
             && !kvmppc_has_cap_mmu_hash_v3()) {

[PATCH 3/4] spapr/cas: Separate CAS handling from rebuilding the FDT

[Greg Kurz]

From: Alexey Kardashevskiy <aik@ozlabs.ru>

At the moment "ibm,client-architecture-support" ("CAS") is implemented
in SLOF and QEMU assists via the custom H_CAS hypercall which copies
an updated flatten device tree (FDT) blob to the SLOF memory which
it then uses to update its internal tree.

When we enable the OpenFirmware client interface in QEMU, we won't need
to copy the FDT to the guest as the client is expected to fetch
the device tree using the client interface.

This moves FDT rebuild out to a separate helper which is going to be
called from the "ibm,client-architecture-support" handler and leaves
writing FDT to the guest in the H_CAS handler.

This should not cause any behavioral change.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Message-Id: <20200310050733.29805-3-aik@ozlabs.ru>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr.c         |    1 -
 hw/ppc/spapr_hcall.c   |   67 +++++++++++++++++++++++++++++-------------------
 include/hw/ppc/spapr.h |    7 +++++
 3 files changed, 48 insertions(+), 27 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 91f8cfe7a8b1..c522cd205fee 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -96,7 +96,6 @@
  *
  * We load our kernel at 4M, leaving space for SLOF initial image
  */
-#define FDT_MAX_SIZE            0x100000
 #define RTAS_MAX_ADDR           0x80000000 /* RTAS must stay below that */
 #define FW_MAX_SIZE             0x400000
 #define FW_FILE_NAME            "slof.bin"
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 9bed659b1a94..b424bca4d2c0 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1665,16 +1665,12 @@ static void spapr_handle_transient_dev_before_cas(SpaprMachineState *spapr)
     spapr_clear_pending_hotplug_events(spapr);
 }
 
-static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
-                                                  SpaprMachineState *spapr,
-                                                  target_ulong opcode,
-                                                  target_ulong *args)
+target_ulong do_client_architecture_support(PowerPCCPU *cpu,
+                                            SpaprMachineState *spapr,
+                                            target_ulong vec,
+                                            target_ulong fdt_bufsize)
 {
-    /* Working address in data buffer */
-    target_ulong addr = ppc64_phys_to_real(args[0]);
-    target_ulong fdt_buf = args[1];
-    target_ulong fdt_bufsize = args[2];
-    target_ulong ov_table;
+    target_ulong ov_table; /* Working address in data buffer */
     uint32_t cas_pvr;
     SpaprOptionVector *ov1_guest, *ov5_guest;
     bool guest_radix;
@@ -1694,7 +1690,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
         }
     }
 
-    cas_pvr = cas_check_pvr(spapr, cpu, &addr, &raw_mode_supported, &local_err);
+    cas_pvr = cas_check_pvr(spapr, cpu, &vec, &raw_mode_supported, &local_err);
     if (local_err) {
         error_report_err(local_err);
         return H_HARDWARE;
@@ -1717,7 +1713,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
     }
 
     /* For the future use: here @ov_table points to the first option vector */
-    ov_table = addr;
+    ov_table = vec;
 
     ov1_guest = spapr_ovec_parse_vector(ov_table, 1);
     if (!ov1_guest) {
@@ -1823,7 +1819,6 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
 
     if (!spapr->cas_reboot) {
         void *fdt;
-        SpaprDeviceTreeUpdateHeader hdr = { .version_id = 1 };
 
         /* If spapr_machine_reset() did not set up a HPT but one is necessary
          * (because the guest isn't going to use radix) then set it up here. */
@@ -1832,21 +1827,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
             spapr_setup_hpt(spapr);
         }
 
-        if (fdt_bufsize < sizeof(hdr)) {
-            error_report("SLOF provided insufficient CAS buffer "
-                         TARGET_FMT_lu " (min: %zu)", fdt_bufsize, sizeof(hdr));
-            exit(EXIT_FAILURE);
-        }
-
-        fdt_bufsize -= sizeof(hdr);
-
         fdt = spapr_build_fdt(spapr, false, fdt_bufsize);
-        _FDT((fdt_pack(fdt)));
-
-        cpu_physical_memory_write(fdt_buf, &hdr, sizeof(hdr));
-        cpu_physical_memory_write(fdt_buf + sizeof(hdr), fdt,
-                                  fdt_totalsize(fdt));
-        trace_spapr_cas_continue(fdt_totalsize(fdt) + sizeof(hdr));
 
         g_free(spapr->fdt_blob);
         spapr->fdt_size = fdt_totalsize(fdt);
@@ -1861,6 +1842,40 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
     return H_SUCCESS;
 }
 
+static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
+                                                  SpaprMachineState *spapr,
+                                                  target_ulong opcode,
+                                                  target_ulong *args)
+{
+    target_ulong vec = ppc64_phys_to_real(args[0]);
+    target_ulong fdt_buf = args[1];
+    target_ulong fdt_bufsize = args[2];
+    target_ulong ret;
+    SpaprDeviceTreeUpdateHeader hdr = { .version_id = 1 };
+
+    if (fdt_bufsize < sizeof(hdr)) {
+        error_report("SLOF provided insufficient CAS buffer "
+                     TARGET_FMT_lu " (min: %zu)", fdt_bufsize, sizeof(hdr));
+        exit(EXIT_FAILURE);
+    }
+
+    fdt_bufsize -= sizeof(hdr);
+
+    ret = do_client_architecture_support(cpu, spapr, vec, fdt_bufsize);
+    if (ret == H_SUCCESS) {
+        _FDT((fdt_pack(spapr->fdt_blob)));
+        spapr->fdt_size = fdt_totalsize(spapr->fdt_blob);
+        spapr->fdt_initial_size = spapr->fdt_size;
+
+        cpu_physical_memory_write(fdt_buf, &hdr, sizeof(hdr));
+        cpu_physical_memory_write(fdt_buf + sizeof(hdr), spapr->fdt_blob,
+                                  spapr->fdt_size);
+        trace_spapr_cas_continue(spapr->fdt_size + sizeof(hdr));
+    }
+
+    return ret;
+}
+
 static target_ulong h_home_node_associativity(PowerPCCPU *cpu,
                                               SpaprMachineState *spapr,
                                               target_ulong opcode,
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 42d64a036821..b7e13e5aafb7 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -102,6 +102,8 @@ typedef enum {
 #define SPAPR_CAP_FIXED_CCD             0x03
 #define SPAPR_CAP_FIXED_NA              0x10 /* Lets leave a bit of a gap... */
 
+#define FDT_MAX_SIZE                    0x100000
+
 typedef struct SpaprCapabilities SpaprCapabilities;
 struct SpaprCapabilities {
     uint8_t caps[SPAPR_CAP_NUM];
@@ -566,6 +568,11 @@ void spapr_register_hypercall(target_ulong opcode, spapr_hcall_fn fn);
 target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode,
                              target_ulong *args);
 
+target_ulong do_client_architecture_support(PowerPCCPU *cpu,
+                                            SpaprMachineState *spapr,
+                                            target_ulong addr,
+                                            target_ulong fdt_bufsize);
+
 /* Virtual Processor Area structure constants */
 #define VPA_MIN_SIZE           640
 #define VPA_SIZE_OFFSET        0x4

[PATCH 4/4] spapr: Drop CAS reboot flag

[Greg Kurz]

The CAS reboot flag is false by default and all the locations that
could set it to true have been dropped. This means that all code
blocks depending on the flag being set is dead code and the other
code blocks should be executed always.

Just do that and drop the now uneeded CAS reboot flag. Fix a
comment on the way to make checkpatch happy.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr.c         |   18 ++++--------------
 hw/ppc/spapr_hcall.c   |   33 ++++++++++++++-------------------
 include/hw/ppc/spapr.h |    1 -
 3 files changed, 18 insertions(+), 34 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index c522cd205fee..4c46578b1387 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1579,9 +1579,7 @@ void spapr_setup_hpt(SpaprMachineState *spapr)
 {
     int hpt_shift;
 
-    if ((spapr->resize_hpt == SPAPR_RESIZE_HPT_DISABLED)
-        || (spapr->cas_reboot
-            && !spapr_ovec_test(spapr->ov5_cas, OV5_HPT_RESIZE))) {
+    if (spapr->resize_hpt == SPAPR_RESIZE_HPT_DISABLED) {
         hpt_shift = spapr_hpt_shift_for_ramsize(MACHINE(spapr)->maxram_size);
     } else {
         uint64_t current_ram_size;
@@ -1645,16 +1643,10 @@ static void spapr_machine_reset(MachineState *machine)
 
     qemu_devices_reset();
 
-    /*
-     * If this reset wasn't generated by CAS, we should reset our
-     * negotiated options and start from scratch
-     */
-    if (!spapr->cas_reboot) {
-        spapr_ovec_cleanup(spapr->ov5_cas);
-        spapr->ov5_cas = spapr_ovec_new();
+    spapr_ovec_cleanup(spapr->ov5_cas);
+    spapr->ov5_cas = spapr_ovec_new();
 
-        ppc_set_compat_all(spapr->max_compat_pvr, &error_fatal);
-    }
+    ppc_set_compat_all(spapr->max_compat_pvr, &error_fatal);
 
     /*
      * This is fixing some of the default configuration of the XIVE
@@ -1707,8 +1699,6 @@ static void spapr_machine_reset(MachineState *machine)
     spapr_cpu_set_entry_state(first_ppc_cpu, SPAPR_ENTRY_POINT, 0, fdt_addr, 0);
     first_ppc_cpu->env.gpr[5] = 0;
 
-    spapr->cas_reboot = false;
-
     spapr->fwnmi_system_reset_addr = -1;
     spapr->fwnmi_machine_check_addr = -1;
     spapr->fwnmi_machine_check_interlock = -1;
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index b424bca4d2c0..0fe8da3c9359 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1678,6 +1678,7 @@ target_ulong do_client_architecture_support(PowerPCCPU *cpu,
     bool raw_mode_supported = false;
     bool guest_xive;
     CPUState *cs;
+    void *fdt;
 
     /* CAS is supposed to be called early when only the boot vCPU is active. */
     CPU_FOREACH(cs) {
@@ -1817,27 +1818,21 @@ target_ulong do_client_architecture_support(PowerPCCPU *cpu,
 
     spapr_handle_transient_dev_before_cas(spapr);
 
-    if (!spapr->cas_reboot) {
-        void *fdt;
-
-        /* If spapr_machine_reset() did not set up a HPT but one is necessary
-         * (because the guest isn't going to use radix) then set it up here. */
-        if ((spapr->patb_entry & PATE1_GR) && !guest_radix) {
-            /* legacy hash or new hash: */
-            spapr_setup_hpt(spapr);
-        }
-
-        fdt = spapr_build_fdt(spapr, false, fdt_bufsize);
-
-        g_free(spapr->fdt_blob);
-        spapr->fdt_size = fdt_totalsize(fdt);
-        spapr->fdt_initial_size = spapr->fdt_size;
-        spapr->fdt_blob = fdt;
+    /*
+     * If spapr_machine_reset() did not set up a HPT but one is necessary
+     * (because the guest isn't going to use radix) then set it up here.
+     */
+    if ((spapr->patb_entry & PATE1_GR) && !guest_radix) {
+        /* legacy hash or new hash: */
+        spapr_setup_hpt(spapr);
     }
 
-    if (spapr->cas_reboot) {
-        qemu_system_reset_request(SHUTDOWN_CAUSE_SUBSYSTEM_RESET);
-    }
+    fdt = spapr_build_fdt(spapr, false, fdt_bufsize);
+
+    g_free(spapr->fdt_blob);
+    spapr->fdt_size = fdt_totalsize(fdt);
+    spapr->fdt_initial_size = spapr->fdt_size;
+    spapr->fdt_blob = fdt;
 
     return H_SUCCESS;
 }
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index b7e13e5aafb7..e579eaf28c05 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -178,7 +178,6 @@ struct SpaprMachineState {
     SpaprEventSource *event_sources;
 
     /* ibm,client-architecture-support option negotiation */
-    bool cas_reboot;
     bool cas_pre_isa3_guest;
     SpaprOptionVector *ov5;         /* QEMU-supported option vectors */
     SpaprOptionVector *ov5_cas;     /* negotiated (via CAS) option vectors */

Re: [PATCH 1/4] spapr: Don't check capabilities removed between CAS calls

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3135 bytes --]

On Wed, Mar 25, 2020 at 04:25:30PM +0100, Greg Kurz wrote:
> We currently check if some capability in OV5 was removed by the guest
> since the previous CAS, and we trigger a CAS reboot in that case. This
> was required because it could call for a device-tree property or node
> removal, that we didn't support until recently (see commit 6787d27b04a7
> "spapr: add option vector handling in CAS-generated resets" for details).
> 
> Now that we render a full FDT at CAS and that SLOF is able to handle
> node removal, we don't need to do a CAS reset in this case anymore.
> Also, this check can only return true if the guest has already called
> CAS since the last full system reset (otherwise spapr->ov5_cas is
> empty). Linux doesn't do that so this can be considered as dead code
> for the vast majority of existing setups.
> 
> Drop the check. Since the only use of the ov5_cas_old variable is
> precisely the check itself, drop the variable as well.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Applied to ppc-for-5.1.

> ---
>  hw/ppc/spapr_hcall.c |   14 +-------------
>  1 file changed, 1 insertion(+), 13 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 40c86e91eb89..1f123a68e46c 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1676,7 +1676,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>      target_ulong fdt_bufsize = args[2];
>      target_ulong ov_table;
>      uint32_t cas_pvr;
> -    SpaprOptionVector *ov1_guest, *ov5_guest, *ov5_cas_old;
> +    SpaprOptionVector *ov1_guest, *ov5_guest;
>      bool guest_radix;
>      Error *local_err = NULL;
>      bool raw_mode_supported = false;
> @@ -1781,22 +1781,10 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>       * by LoPAPR 1.1, 14.5.4.8, which QEMU doesn't implement, we don't need
>       * to worry about this for now.
>       */
> -    ov5_cas_old = spapr_ovec_clone(spapr->ov5_cas);
> -
> -    /* also clear the radix/hash bit from the current ov5_cas bits to
> -     * be in sync with the newly ov5 bits. Else the radix bit will be
> -     * seen as being removed and this will generate a reset loop
> -     */
> -    spapr_ovec_clear(ov5_cas_old, OV5_MMU_RADIX_300);
>  
>      /* full range of negotiated ov5 capabilities */
>      spapr_ovec_intersect(spapr->ov5_cas, spapr->ov5, ov5_guest);
>      spapr_ovec_cleanup(ov5_guest);
> -    /* capabilities that have been added since CAS-generated guest reset.
> -     * if capabilities have since been removed, generate another reset
> -     */
> -    spapr->cas_reboot = !spapr_ovec_subset(ov5_cas_old, spapr->ov5_cas);
> -    spapr_ovec_cleanup(ov5_cas_old);
>      /* Now that processing is finished, set the radix/hash bit for the
>       * guest if it requested a valid mode; otherwise terminate the boot. */
>      if (guest_radix) {
> 

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/4] spapr: Get rid of CAS reboot flag

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1467 bytes --]

On Wed, Mar 25, 2020 at 04:25:24PM +0100, Greg Kurz wrote:
> The CAS reboot flag was introduced in QEMU 2.10 to allow the guest
> to be presented a new boot-time device tree after CAS negotiation.
> CAS-generated resets rely on qemu_system_reset_request() which has
> the particularity of dropping the main loop lock at some point. This
> opens a window where migration can happen, hence promotting the CAS
> reboot flag to actual state that we should also migrate. In practice,
> this can't happen anymore since we have eliminated the scenario of
> the XICS/XIVE switch and the much less frequent scenario of device
> plug/unplug before CAS.
> 
> We still have much of the CAS reboot bits around though. The full FDT
> rendering we do at CAS is enough to get rid of them once and far all.
> 
> Some preliminary cleanup is made before going for the full removal,
> for easier reviewing. At some point I had the need to move some code
> around in CAS, and Alexey's patch from the "spapr: kill SLOF" (v8)
> series proved to be helpful so I've reused it in this patchset.
> 
> This series applies cleanly on both ppc-for-5.0 and ppc-for-5.1.
> Since it doesn't fix any actual bug, I think this can be delayed
> to 5.1.

Applied to ppc-for-5.1.

> 

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/4] spapr: Get rid of CAS reboot flag

[Alexey Kardashevskiy]

On 31/03/2020 11:44, David Gibson wrote:
> On Wed, Mar 25, 2020 at 04:25:24PM +0100, Greg Kurz wrote:
>> The CAS reboot flag was introduced in QEMU 2.10 to allow the guest
>> to be presented a new boot-time device tree after CAS negotiation.
>> CAS-generated resets rely on qemu_system_reset_request() which has
>> the particularity of dropping the main loop lock at some point. This
>> opens a window where migration can happen, hence promotting the CAS
>> reboot flag to actual state that we should also migrate. In practice,
>> this can't happen anymore since we have eliminated the scenario of
>> the XICS/XIVE switch and the much less frequent scenario of device
>> plug/unplug before CAS.
>>
>> We still have much of the CAS reboot bits around though. The full FDT
>> rendering we do at CAS is enough to get rid of them once and far all.
>>
>> Some preliminary cleanup is made before going for the full removal,
>> for easier reviewing. At some point I had the need to move some code
>> around in CAS, and Alexey's patch from the "spapr: kill SLOF" (v8)
>> series proved to be helpful so I've reused it in this patchset.
>>
>> This series applies cleanly on both ppc-for-5.0 and ppc-for-5.1.
>> Since it doesn't fix any actual bug, I think this can be delayed
>> to 5.1.
> 
> Applied to ppc-for-5.1.



Can you push it out please? The existing ppc-for-5.1 corrupts stack on
my machine in qemu_init().

-- 
Alexey

Re: [PATCH 0/4] spapr: Get rid of CAS reboot flag

[Greg Kurz]

On Tue, 31 Mar 2020 13:59:01 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> 
> 
> On 31/03/2020 11:44, David Gibson wrote:
> > On Wed, Mar 25, 2020 at 04:25:24PM +0100, Greg Kurz wrote:
> >> The CAS reboot flag was introduced in QEMU 2.10 to allow the guest
> >> to be presented a new boot-time device tree after CAS negotiation.
> >> CAS-generated resets rely on qemu_system_reset_request() which has
> >> the particularity of dropping the main loop lock at some point. This
> >> opens a window where migration can happen, hence promotting the CAS
> >> reboot flag to actual state that we should also migrate. In practice,
> >> this can't happen anymore since we have eliminated the scenario of
> >> the XICS/XIVE switch and the much less frequent scenario of device
> >> plug/unplug before CAS.
> >>
> >> We still have much of the CAS reboot bits around though. The full FDT
> >> rendering we do at CAS is enough to get rid of them once and far all.
> >>
> >> Some preliminary cleanup is made before going for the full removal,
> >> for easier reviewing. At some point I had the need to move some code
> >> around in CAS, and Alexey's patch from the "spapr: kill SLOF" (v8)
> >> series proved to be helpful so I've reused it in this patchset.
> >>
> >> This series applies cleanly on both ppc-for-5.0 and ppc-for-5.1.
> >> Since it doesn't fix any actual bug, I think this can be delayed
> >> to 5.1.
> > 
> > Applied to ppc-for-5.1.
> 
> 
> 
> Can you push it out please? The existing ppc-for-5.1 corrupts stack on
> my machine in qemu_init().
> 

Can you provide more details ?

Re: [PATCH 0/4] spapr: Get rid of CAS reboot flag

[Alexey Kardashevskiy]

On 31/03/2020 18:08, Greg Kurz wrote:
> On Tue, 31 Mar 2020 13:59:01 +1100
> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> 
>>
>>
>> On 31/03/2020 11:44, David Gibson wrote:
>>> On Wed, Mar 25, 2020 at 04:25:24PM +0100, Greg Kurz wrote:
>>>> The CAS reboot flag was introduced in QEMU 2.10 to allow the guest
>>>> to be presented a new boot-time device tree after CAS negotiation.
>>>> CAS-generated resets rely on qemu_system_reset_request() which has
>>>> the particularity of dropping the main loop lock at some point. This
>>>> opens a window where migration can happen, hence promotting the CAS
>>>> reboot flag to actual state that we should also migrate. In practice,
>>>> this can't happen anymore since we have eliminated the scenario of
>>>> the XICS/XIVE switch and the much less frequent scenario of device
>>>> plug/unplug before CAS.
>>>>
>>>> We still have much of the CAS reboot bits around though. The full FDT
>>>> rendering we do at CAS is enough to get rid of them once and far all.
>>>>
>>>> Some preliminary cleanup is made before going for the full removal,
>>>> for easier reviewing. At some point I had the need to move some code
>>>> around in CAS, and Alexey's patch from the "spapr: kill SLOF" (v8)
>>>> series proved to be helpful so I've reused it in this patchset.
>>>>
>>>> This series applies cleanly on both ppc-for-5.0 and ppc-for-5.1.
>>>> Since it doesn't fix any actual bug, I think this can be delayed
>>>> to 5.1.
>>>
>>> Applied to ppc-for-5.1.
>>
>>
>>
>> Can you push it out please? The existing ppc-for-5.1 corrupts stack on
>> my machine in qemu_init().
>>
> 
> Can you provide more details ?

This one:
6d15081ee3ca (HEAD, dwg/ppc-for-5.1) 4 hours ago Nicholas Piggin
ppc/pnv: Add support for NMI interface

removed {} in:


diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 671535ebe6a6..ac83b8698b8e 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -2067,6 +2067,7 @@ static const TypeInfo types[] = {
         .interfaces = (InterfaceInfo[]) {
             { TYPE_INTERRUPT_STATS_PROVIDER },
             { TYPE_NMI },
+            { },
         },


David fixed that up, pushed out (forced, as usual), all good, sorry for
the noise :) Cheers.


-- 
Alexey