| Message ID | 20210615013309.2833323-4-danielhb413@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | pSeries base FORM2 NUMA affinity support | expand |
On Mon, Jun 14, 2021 at 10:33:04PM -0300, Daniel Henrique Barboza wrote: > spapr_numa_write_rtas_dt() is called from spapr_dt_rtas(), which in > turned is called by spapr_build_fdt(). spapr_build_fdt() is called in > two places: spapr_machine_reset() and do_client_architecture_support(). > When called in machine_reset() we're writing RTAS nodes with NUMA > artifacts without going through CAS first. > > This is not an issue because we always write the same thing in DT, since > we support just FORM1 NUMA affinity. With the upcoming FORM2 support, > we're now reliant on guest choice to decide what to write. > > Instead of taking a guess (e.g. default to FORM1, switch to FORM2 if > guest chooses it), postpone the writing of > ibm,associativity-reference-points and ibm,max-associativity-domains > until we're sure what was negotiated with the guest. > > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> I think it makes sense to fold this in with 1/8 moving the calculation itself until after CAS. This does make a (theoretical) functional change - it means that NUMA information is not available before CAS, which it was before. I think that's very unlikely to break anything, but I wonder if we should make it dependent on the machine version just to be safe. > --- > hw/ppc/spapr_numa.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c > index 04a86f9b5b..e1a7f80076 100644 > --- a/hw/ppc/spapr_numa.c > +++ b/hw/ppc/spapr_numa.c > @@ -379,6 +379,10 @@ static void spapr_numa_FORM1_write_rtas_dt(SpaprMachineState *spapr, > */ > void spapr_numa_write_rtas_dt(SpaprMachineState *spapr, void *fdt, int rtas) > { > + if (spapr_ovec_empty(spapr->ov5_cas)) { > + return; > + } > + > spapr_numa_FORM1_write_rtas_dt(spapr, fdt, rtas); > } >
On 6/15/21 1:02 AM, David Gibson wrote: > On Mon, Jun 14, 2021 at 10:33:04PM -0300, Daniel Henrique Barboza wrote: >> spapr_numa_write_rtas_dt() is called from spapr_dt_rtas(), which in >> turned is called by spapr_build_fdt(). spapr_build_fdt() is called in >> two places: spapr_machine_reset() and do_client_architecture_support(). >> When called in machine_reset() we're writing RTAS nodes with NUMA >> artifacts without going through CAS first. >> >> This is not an issue because we always write the same thing in DT, since >> we support just FORM1 NUMA affinity. With the upcoming FORM2 support, >> we're now reliant on guest choice to decide what to write. >> >> Instead of taking a guess (e.g. default to FORM1, switch to FORM2 if >> guest chooses it), postpone the writing of >> ibm,associativity-reference-points and ibm,max-associativity-domains >> until we're sure what was negotiated with the guest. >> >> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > > I think it makes sense to fold this in with 1/8 moving the calculation > itself until after CAS. Ok. > > This does make a (theoretical) functional change - it means that NUMA > information is not available before CAS, which it was before. I think > that's very unlikely to break anything, but I wonder if we should make > it dependent on the machine version just to be safe. I don't mind making it dependent on the default machine. I'll wrap this CAS change (and as result, all FORM2 support) to be available only for the default machine type. Thanks, Daniel > >> --- >> hw/ppc/spapr_numa.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c >> index 04a86f9b5b..e1a7f80076 100644 >> --- a/hw/ppc/spapr_numa.c >> +++ b/hw/ppc/spapr_numa.c >> @@ -379,6 +379,10 @@ static void spapr_numa_FORM1_write_rtas_dt(SpaprMachineState *spapr, >> */ >> void spapr_numa_write_rtas_dt(SpaprMachineState *spapr, void *fdt, int rtas) >> { >> + if (spapr_ovec_empty(spapr->ov5_cas)) { >> + return; >> + } >> + >> spapr_numa_FORM1_write_rtas_dt(spapr, fdt, rtas); >> } >> >
diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c index 04a86f9b5b..e1a7f80076 100644 --- a/hw/ppc/spapr_numa.c +++ b/hw/ppc/spapr_numa.c @@ -379,6 +379,10 @@ static void spapr_numa_FORM1_write_rtas_dt(SpaprMachineState *spapr, */ void spapr_numa_write_rtas_dt(SpaprMachineState *spapr, void *fdt, int rtas) { + if (spapr_ovec_empty(spapr->ov5_cas)) { + return; + } + spapr_numa_FORM1_write_rtas_dt(spapr, fdt, rtas); }
spapr_numa_write_rtas_dt() is called from spapr_dt_rtas(), which in turned is called by spapr_build_fdt(). spapr_build_fdt() is called in two places: spapr_machine_reset() and do_client_architecture_support(). When called in machine_reset() we're writing RTAS nodes with NUMA artifacts without going through CAS first. This is not an issue because we always write the same thing in DT, since we support just FORM1 NUMA affinity. With the upcoming FORM2 support, we're now reliant on guest choice to decide what to write. Instead of taking a guess (e.g. default to FORM1, switch to FORM2 if guest chooses it), postpone the writing of ibm,associativity-reference-points and ibm,max-associativity-domains until we're sure what was negotiated with the guest. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- hw/ppc/spapr_numa.c | 4 ++++ 1 file changed, 4 insertions(+)