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

[David Gibson <david@gibson.dropbear.id.au>]

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