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

Greg Kurz posted 4 patches 8 weeks ago
Maintainers: David Gibson <david@gibson.dropbear.id.au>

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

Posted by Greg Kurz 8 weeks ago
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) {


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

Posted by David Gibson 8 weeks ago
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