hw/ppc/vof.c | 50 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 17 deletions(-)
The FDT does not normally store name properties but reconstructs it
from path but Open Firmware specification says each node should at
least have this property. This is correctly handled in getprop but
nextprop should also return it even if not present as a property.
Explicit name properties are still allowed because they are needed
e.g. on the root node that guests expect to have specific names as
seen on real machines instead of being empty so sometimes the node
name may need to be overriden. For example on pegasos MorphOS checks
the name of "/" and expects to find bplan,Pegasos2 which is how it
identifies the machine.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
v3:
Keep phandle properties
Changed commit message
v2:
Fixed a typo in commit message
Simplified loop to get next property name
hw/ppc/vof.c | 50 +++++++++++++++++++++++++++++++++-----------------
1 file changed, 33 insertions(+), 17 deletions(-)
diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
index 09cb77de93..10bafd011e 100644
--- a/hw/ppc/vof.c
+++ b/hw/ppc/vof.c
@@ -353,34 +353,50 @@ static uint32_t vof_nextprop(const void *fdt, uint32_t phandle,
{
int offset, nodeoff = fdt_node_offset_by_phandle(fdt, phandle);
char prev[OF_PROPNAME_LEN_MAX + 1];
- const char *tmp;
+ const char *tmp = NULL;
+ bool match = false;
if (readstr(prevaddr, prev, sizeof(prev))) {
return PROM_ERROR;
}
-
- fdt_for_each_property_offset(offset, fdt, nodeoff) {
- if (!fdt_getprop_by_offset(fdt, offset, &tmp, NULL)) {
- return 0;
+ /*
+ * "name" may or may not be present in fdt but we should still return it.
+ * Do that first and then skip it if seen later.
+ */
+ if (prev[0] == '\0') {
+ tmp = "name";
+ } else {
+ if (strcmp(prev, "name") == 0) {
+ prev[0] = '\0';
}
- if (prev[0] == '\0' || strcmp(prev, tmp) == 0) {
- if (prev[0] != '\0') {
- offset = fdt_next_property_offset(fdt, offset);
- if (offset < 0) {
- return 0;
- }
- }
+ fdt_for_each_property_offset(offset, fdt, nodeoff) {
if (!fdt_getprop_by_offset(fdt, offset, &tmp, NULL)) {
return 0;
}
-
- if (VOF_MEM_WRITE(nameaddr, tmp, strlen(tmp) + 1) != MEMTX_OK) {
- return PROM_ERROR;
+ if (strcmp(tmp, "name") == 0) {
+ continue;
+ }
+ if (match) {
+ break;
}
- return 1;
+ if (strcmp(prev, tmp) == 0) {
+ match = true;
+ continue;
+ }
+ if (prev[0] == '\0') {
+ break;
+ }
+ }
+ if (offset < 0) {
+ return 0;
}
}
-
+ if (tmp) {
+ if (VOF_MEM_WRITE(nameaddr, tmp, strlen(tmp) + 1) != MEMTX_OK) {
+ return PROM_ERROR;
+ }
+ return 1;
+ }
return 0;
}
--
2.41.3On Wed, 23 Apr 2025, BALATON Zoltan wrote:
> The FDT does not normally store name properties but reconstructs it
> from path but Open Firmware specification says each node should at
> least have this property. This is correctly handled in getprop but
> nextprop should also return it even if not present as a property.
>
> Explicit name properties are still allowed because they are needed
> e.g. on the root node that guests expect to have specific names as
> seen on real machines instead of being empty so sometimes the node
> name may need to be overriden. For example on pegasos MorphOS checks
> the name of "/" and expects to find bplan,Pegasos2 which is how it
> identifies the machine.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> v3:
> Keep phandle properties
> Changed commit message
Ping?
Regards,
BALATON Zoltan
> v2:
> Fixed a typo in commit message
> Simplified loop to get next property name
>
> hw/ppc/vof.c | 50 +++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 33 insertions(+), 17 deletions(-)
>
> diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
> index 09cb77de93..10bafd011e 100644
> --- a/hw/ppc/vof.c
> +++ b/hw/ppc/vof.c
> @@ -353,34 +353,50 @@ static uint32_t vof_nextprop(const void *fdt, uint32_t phandle,
> {
> int offset, nodeoff = fdt_node_offset_by_phandle(fdt, phandle);
> char prev[OF_PROPNAME_LEN_MAX + 1];
> - const char *tmp;
> + const char *tmp = NULL;
> + bool match = false;
>
> if (readstr(prevaddr, prev, sizeof(prev))) {
> return PROM_ERROR;
> }
> -
> - fdt_for_each_property_offset(offset, fdt, nodeoff) {
> - if (!fdt_getprop_by_offset(fdt, offset, &tmp, NULL)) {
> - return 0;
> + /*
> + * "name" may or may not be present in fdt but we should still return it.
> + * Do that first and then skip it if seen later.
> + */
> + if (prev[0] == '\0') {
> + tmp = "name";
> + } else {
> + if (strcmp(prev, "name") == 0) {
> + prev[0] = '\0';
> }
> - if (prev[0] == '\0' || strcmp(prev, tmp) == 0) {
> - if (prev[0] != '\0') {
> - offset = fdt_next_property_offset(fdt, offset);
> - if (offset < 0) {
> - return 0;
> - }
> - }
> + fdt_for_each_property_offset(offset, fdt, nodeoff) {
> if (!fdt_getprop_by_offset(fdt, offset, &tmp, NULL)) {
> return 0;
> }
> -
> - if (VOF_MEM_WRITE(nameaddr, tmp, strlen(tmp) + 1) != MEMTX_OK) {
> - return PROM_ERROR;
> + if (strcmp(tmp, "name") == 0) {
> + continue;
> + }
> + if (match) {
> + break;
> }
> - return 1;
> + if (strcmp(prev, tmp) == 0) {
> + match = true;
> + continue;
> + }
> + if (prev[0] == '\0') {
> + break;
> + }
> + }
> + if (offset < 0) {
> + return 0;
> }
> }
> -
> + if (tmp) {
> + if (VOF_MEM_WRITE(nameaddr, tmp, strlen(tmp) + 1) != MEMTX_OK) {
> + return PROM_ERROR;
> + }
> + return 1;
> + }
> return 0;
> }
>
>
On Wed, 30 Apr 2025, at 21:21, BALATON Zoltan wrote:
> On Wed, 23 Apr 2025, BALATON Zoltan wrote:
> > The FDT does not normally store name properties but reconstructs it
> > from path but Open Firmware specification says each node should at
> > least have this property. This is correctly handled in getprop but
> > nextprop should also return it even if not present as a property.
> >
> > Explicit name properties are still allowed because they are needed
> > e.g. on the root node that guests expect to have specific names as
> > seen on real machines instead of being empty so sometimes the node
> > name may need to be overriden. For example on pegasos MorphOS checks
> > the name of "/" and expects to find bplan,Pegasos2 which is how it
> > identifies the machine.
> >
> > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> > ---
> > v3:
> > Keep phandle properties
> > Changed commit message
>
> Ping?
>
sorry for the delay, looks okay to me but (probably was answered but I forgot because I am slow) I still do not understand what is adding the explicit property called "name" so vof_nextprop() needs to check if it is the actual property. Thanks,
> Regards,
> BALATON Zoltan
>
> > v2:
> > Fixed a typo in commit message
> > Simplified loop to get next property name
> >
> > hw/ppc/vof.c | 50 +++++++++++++++++++++++++++++++++-----------------
> > 1 file changed, 33 insertions(+), 17 deletions(-)
> >
> > diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
> > index 09cb77de93..10bafd011e 100644
> > --- a/hw/ppc/vof.c
> > +++ b/hw/ppc/vof.c
> > @@ -353,34 +353,50 @@ static uint32_t vof_nextprop(const void *fdt, uint32_t phandle,
> > {
> > int offset, nodeoff = fdt_node_offset_by_phandle(fdt, phandle);
> > char prev[OF_PROPNAME_LEN_MAX + 1];
> > - const char *tmp;
> > + const char *tmp = NULL;
> > + bool match = false;
> >
> > if (readstr(prevaddr, prev, sizeof(prev))) {
> > return PROM_ERROR;
> > }
> > -
> > - fdt_for_each_property_offset(offset, fdt, nodeoff) {
> > - if (!fdt_getprop_by_offset(fdt, offset, &tmp, NULL)) {
> > - return 0;
> > + /*
> > + * "name" may or may not be present in fdt but we should still return it.
> > + * Do that first and then skip it if seen later.
> > + */
> > + if (prev[0] == '\0') {
> > + tmp = "name";
> > + } else {
> > + if (strcmp(prev, "name") == 0) {
> > + prev[0] = '\0';
> > }
> > - if (prev[0] == '\0' || strcmp(prev, tmp) == 0) {
> > - if (prev[0] != '\0') {
> > - offset = fdt_next_property_offset(fdt, offset);
> > - if (offset < 0) {
> > - return 0;
> > - }
> > - }
> > + fdt_for_each_property_offset(offset, fdt, nodeoff) {
> > if (!fdt_getprop_by_offset(fdt, offset, &tmp, NULL)) {
> > return 0;
> > }
> > -
> > - if (VOF_MEM_WRITE(nameaddr, tmp, strlen(tmp) + 1) != MEMTX_OK) {
> > - return PROM_ERROR;
> > + if (strcmp(tmp, "name") == 0) {
> > + continue;
> > + }
> > + if (match) {
> > + break;
> > }
> > - return 1;
> > + if (strcmp(prev, tmp) == 0) {
> > + match = true;
> > + continue;
> > + }
> > + if (prev[0] == '\0') {
> > + break;
> > + }
> > + }
> > + if (offset < 0) {
> > + return 0;
> > }
> > }
> > -
> > + if (tmp) {
> > + if (VOF_MEM_WRITE(nameaddr, tmp, strlen(tmp) + 1) != MEMTX_OK) {
> > + return PROM_ERROR;
> > + }
> > + return 1;
> > + }
> > return 0;
> > }
> >
> >
>
On Tue, 13 May 2025, Alexey Kardashevskiy wrote: > On Wed, 30 Apr 2025, at 21:21, BALATON Zoltan wrote: >> On Wed, 23 Apr 2025, BALATON Zoltan wrote: >>> The FDT does not normally store name properties but reconstructs it >>> from path but Open Firmware specification says each node should at >>> least have this property. This is correctly handled in getprop but >>> nextprop should also return it even if not present as a property. >>> >>> Explicit name properties are still allowed because they are needed >>> e.g. on the root node that guests expect to have specific names as >>> seen on real machines instead of being empty so sometimes the node >>> name may need to be overriden. For example on pegasos MorphOS checks >>> the name of "/" and expects to find bplan,Pegasos2 which is how it >>> identifies the machine. >>> >>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >>> --- >>> v3: >>> Keep phandle properties >>> Changed commit message >> >> Ping? >> > > sorry for the delay, looks okay to me but (probably was answered but I > forgot because I am slow) I still do not understand what is adding the > explicit property called "name" so vof_nextprop() needs to check if it > is the actual property. Thanks, Look at this series where this is used (I also included this patch there as patch 2 which is identical to this v3 standalone patch): https://patchew.org/QEMU/cover.1746139668.git.balaton@eik.bme.hu/ This patch allows removing explicit name properties in patch 3 and move the static parts into a dtb file in patch 4. Dtb does not allow to set name property and will use the path name for it but e.g. the root of the device tree on some machines must have a specific name. On Apple it's called device-tree and on pegasos it's bPlan,Pegasos and some guests (e.g. MorphOS) checks this to detect what machine they run on so will fail it we return / or no name for the name of the root node. Therefore in patch 4 I still have a qemu_fdt_setprop_string(fdt, "/", "name", "bplan,Pegasos2"); to make this work, and for some nodes it may still be needed although for most the path name from dtb will be correct so no need to add a name property for those. But becuase of these nodes that need a name different from their path we still need to allow explicit name properties. Regards, BALATON Zoltan
On Tue, 13 May 2025, at 22:00, BALATON Zoltan wrote: > On Tue, 13 May 2025, Alexey Kardashevskiy wrote: > > On Wed, 30 Apr 2025, at 21:21, BALATON Zoltan wrote: > >> On Wed, 23 Apr 2025, BALATON Zoltan wrote: > >>> The FDT does not normally store name properties but reconstructs it > >>> from path but Open Firmware specification says each node should at > >>> least have this property. This is correctly handled in getprop but > >>> nextprop should also return it even if not present as a property. > >>> > >>> Explicit name properties are still allowed because they are needed > >>> e.g. on the root node that guests expect to have specific names as > >>> seen on real machines instead of being empty so sometimes the node > >>> name may need to be overriden. For example on pegasos MorphOS checks > >>> the name of "/" and expects to find bplan,Pegasos2 which is how it > >>> identifies the machine. > >>> > >>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > >>> --- > >>> v3: > >>> Keep phandle properties > >>> Changed commit message > >> > >> Ping? > >> > > > > sorry for the delay, looks okay to me but (probably was answered but I > > forgot because I am slow) I still do not understand what is adding the > > explicit property called "name" so vof_nextprop() needs to check if it > > is the actual property. Thanks, > > Look at this series where this is used (I also included this patch there > as patch 2 which is identical to this v3 standalone patch): > > https://patchew.org/QEMU/cover.1746139668.git.balaton@eik.bme.hu/ > > This patch allows removing explicit name properties in patch 3 and move > the static parts into a dtb file in patch 4. Dtb does not allow to set > name property and will use the path name for it but e.g. the root of the > device tree on some machines must have a specific name. On Apple it's > called device-tree and on pegasos it's bPlan,Pegasos and some guests (e.g. > MorphOS) checks this to detect what machine they run on so will fail it we > return / or no name for the name of the root node. Therefore in patch 4 I > still have a > > qemu_fdt_setprop_string(fdt, "/", "name", "bplan,Pegasos2"); > > to make this work, and for some nodes it may still be needed although for > most the path name from dtb will be correct so no need to add a name > property for those. But becuase of these nodes that need a name different > from their path we still need to allow explicit name properties. Well I guess that ok then. Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> > > Regards, > BALATON Zoltan >
© 2016 - 2025 Red Hat, Inc.