On Thu, May 21, 2020 at 01:34:37AM +0200, Greg Kurz wrote:
On Mon, 18 May 2020 16:44:17 -0500
Reza Arbab <ar...@linux.ibm.com> wrote:
@@ -944,8 +946,9 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void 
*fdt)
                      qemu_hypertas->str, qemu_hypertas->len));
     g_string_free(qemu_hypertas, TRUE);

+    nr_refpoints = MIN(smc->nr_assoc_refpoints, ARRAY_SIZE(refpoints));

Having the machine requesting more reference points than available
would clearly be a bug. I'd rather add an assert() than silently
clipping to the size of refpoints[].

I'll rework nr_assoc_refpoints into a bool as David suggested. Struggling for a name at the moment, but I'll think on it.

     _FDT(fdt_setprop(fdt, rtas, "ibm,associativity-reference-points",
-                     refpoints, sizeof(refpoints)));
+                     refpoints, nr_refpoints * sizeof(uint32_t)));


Size can be expressed without yet another explicit reference to the
uint32_t type:

nr_refpoints * sizeof(refpoints[0])

Will do.

@@ -4541,6 +4544,7 @@ static void spapr_machine_class_init(ObjectClass *oc, 
void *data)
     smc->linux_pci_probe = true;
     smc->smp_threads_vsmt = true;
     smc->nr_xirqs = SPAPR_NR_XIRQS;
+    smc->nr_assoc_refpoints = 2;

When adding a new setting for the default machine type, we usually
take care of older machine types at the same time, ie. folding this
patch into the next one. Both patches are simple enough that it should
be okay and this would avoid this line to be touched again.

No problem. I'll squash the patches together and work in that PHB compat property as well.
Thanks for your feedback!

--
Reza Arbab

Reply via email to