Hi Showta, Thanks for addressing the nits — both look good. A few things before this is ready:
1. Please send v2 as a proper standalone patch submission (not inline in a reply), so it gets picked up correctly by patchwork. 2. Since the nits have been addressed, please carry over the tags in v2: Reviewed-by: Amit Machhiwal [email protected] Tested-by: Amit Machhiwal [email protected] 3. One minor nit - see below: On 2026/10/04 08:42 AM, 蟹澤翔太 wrote: > Hi Amit, > > I am sorry for the slow reply. I have just started a new job, and it > took me a while to get back to this work. > > Thank you for the review, for checking it against PAPR v10.60, and for > testing it on a nestedv2 KVM guest. Both of your suggestions make > sense; v2 below cites PAPR v10.60 first, keeps LoPAR and LoPAPR as > secondary references, and the comment on RTAS_VERSION now names PAPR > v10.60 Table 20. There is no other change to the code. > > Thanks, > Showta > > -- >8 -- > > Subject: [PATCH v2] hw/ppc/spapr: Add the "rtas-version" property to /rtas > PAPR requires every property listed in its table of /rtas device tree > properties to be present before the OS is booted (R-7.2.6-3 and Table > 20 in PAPR v10.60; the same requirement is in LoPAR 2.9 and LoPAPR > 1.1). One of them is "rtas-version", an integer encoding of the RTAS > interface version, and PAPR says it describes version 1 of that > interface. > > spapr_dt_rtas() sets "rtas-size", "rtas-error-log-max" and > "rtas-event-scan-rate" but never "rtas-version", so a client that reads > it finds no RTAS: NetBSD's RTAS driver fails to attach when the > property is absent. > > Set it to 1, the version PAPR describes. > > Signed-off-by: Showta Ishizaki <[email protected]> > --- > v2: > - Cite PAPR v10.60 first, as the rest of the spapr code does, and keep > LoPAR and LoPAPR as secondary references. > - The comment on RTAS_VERSION now names PAPR v10.60 Table 20. > > hw/ppc/spapr.c | 1 + > include/hw/ppc/spapr.h | 3 +++ > 2 files changed, 4 insertions(+) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 6d99464486..8a1b5b37b6 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1068,6 +1068,7 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, > void *fdt) > RTAS_ERROR_LOG_MAX)); > _FDT(fdt_setprop_cell(fdt, rtas, "rtas-event-scan-rate", > RTAS_EVENT_SCAN_RATE)); > + _FDT(fdt_setprop_cell(fdt, rtas, "rtas-version", RTAS_VERSION)); > > g_assert(msi_nonbroken); > _FDT(fdt_setprop(fdt, rtas, "ibm,change-msix-capable", NULL, 0)); > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 9acda15d4f..b92d25c18e 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -830,6 +830,9 @@ void spapr_load_rtas(SpaprMachineState *spapr, void *fdt, > hwaddr addr); > > #define RTAS_EVENT_SCAN_RATE 1 > > +/* The RTAS interface version PAPR v10.60 Table 20 describes */ The wording reads a bit awkwardly. How about: /* RTAS interface version, as defined by PAPR v10.60 Table 20 */ Thanks, Amit
