Re: [Qemu-devel] [PATCH] Fix phb_placement backwards compatibility

[Laurent Vivier]

On 20/05/2019 08:05, David Gibson wrote:
> When we added support for NVLink2 passthrough devices, we changed the
> phb_placement hook to handle the placement of NVLink2 bridges' specific
> resources.  For compatibility we use a version that doesn't do this
> allocation  for old machine types.
> 
> However, because of the delay between when the patch was posted and when
> it was merged, we ended up with that compatibility hook applying for
> machine versions 3.1 and earlier whereas it should apply for 4.0 and
> earlier (since the patch was applied early in the 4.1 tree).
> 
> Fixes: ec132efaa81 "spapr: Support NVIDIA V100 GPU with NVLink2"
> 
> Reported-by: Laurent Vivier <address@hidden>
> Signed-off-by: David Gibson <address@hidden>
> ---
>  hw/ppc/spapr.c | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index bcae30ad26..39e698e9b0 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -4411,18 +4411,7 @@ DEFINE_SPAPR_MACHINE(4_1, "4.1", true);
>  /*
>   * pseries-4.0
>   */
> -static void spapr_machine_4_0_class_options(MachineClass *mc)
> -{
> -    spapr_machine_4_1_class_options(mc);
> -    compat_props_add(mc->compat_props, hw_compat_4_0, hw_compat_4_0_len);
> -}
> -
> -DEFINE_SPAPR_MACHINE(4_0, "4.0", false);
> -
> -/*
> - * pseries-3.1
> - */
> -static void phb_placement_3_1(SpaprMachineState *spapr, uint32_t index,
> +static void phb_placement_4_0(SpaprMachineState *spapr, uint32_t index,
>                                uint64_t *buid, hwaddr *pio,
>                                hwaddr *mmio32, hwaddr *mmio64,
>                                unsigned n_dma, uint32_t *liobns,
> @@ -4434,6 +4423,20 @@ static void phb_placement_3_1(SpaprMachineState 
> *spapr, uint32_t index,
>      *nv2atsd = 0;
>  }
>  
> +static void spapr_machine_4_0_class_options(MachineClass *mc)
> +{
> +    SpaprMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
> +
> +    spapr_machine_4_1_class_options(mc);
> +    compat_props_add(mc->compat_props, hw_compat_4_0, hw_compat_4_0_len);
> +    smc->phb_placement = phb_placement_4_0;
> +}
> +
> +DEFINE_SPAPR_MACHINE(4_0, "4.0", false);
> +
> +/*
> + * pseries-3.1
> + */
>  static void spapr_machine_3_1_class_options(MachineClass *mc)
>  {
>      SpaprMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
> @@ -4449,7 +4452,6 @@ static void 
> spapr_machine_3_1_class_options(MachineClass *mc)
>      smc->default_caps.caps[SPAPR_CAP_SBBC] = SPAPR_CAP_BROKEN;
>      smc->default_caps.caps[SPAPR_CAP_IBS] = SPAPR_CAP_BROKEN;
>      smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_OFF;
> -    smc->phb_placement = phb_placement_3_1;
>  }
>  
>  DEFINE_SPAPR_MACHINE(3_1, "3.1", false);
> 

Reviewed-by: Laurent Vivier <address@hidden>