| Message ID | 20190520060550.29481-1-david@gibson.dropbear.id.au (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix phb_placement backwards compatibility | expand |
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 <lvivier@redhat.com> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > --- > 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 <lvivier@redhat.com>
On Mon, 20 May 2019 08:21:32 +0200 Laurent Vivier <lvivier@redhat.com> wrote: > 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 <lvivier@redhat.com> > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > --- Reviewed-by: Greg Kurz <groug@kaod.org> > > 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 <lvivier@redhat.com> >
On 5/20/19 8:05 AM, 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 <lvivier@redhat.com> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Cédric Le Goater <clg@kaod.org> Thanks, C. > --- > 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); >
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);
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 <lvivier@redhat.com> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> --- hw/ppc/spapr.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-)