[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH qemu v20] spapr: Implement Open Firmware client interface
From: |
David Gibson |
Subject: |
Re: [PATCH qemu v20] spapr: Implement Open Firmware client interface |
Date: |
Mon, 7 Jun 2021 13:05:29 +1000 |
On Fri, Jun 04, 2021 at 03:50:28PM +0200, BALATON Zoltan wrote:
> On Fri, 4 Jun 2021, David Gibson wrote:
> > On Sun, May 30, 2021 at 07:33:01PM +0200, BALATON Zoltan wrote:
[snip]
> > > MorphOS checks the name property of the root node ("/") to decide what
> > > platform it runs on so we may need to be able to set this property on /
> > > where it should return "bplan,Pegasos2", therefore the above maybe should
> > > do
> > > getprop first and only generate name property if it's not set (or at least
> > > check if we're on the root node and allow setting name property there).
> > > (On
> > > Macs the root node is named "device-tree" and this was before found to be
> > > needed for MorphOS.)
> >
> > Ah. Hrm. Have to think about what to do about that.
>
> This is easy to fix, this seems to allow setting a name property or return a
> default:
>
> > diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
> index b47bbd509d..746842593e 100644
> --- a/hw/ppc/vof.c
> +++ b/hw/ppc/vof.c
> @@ -163,14 +163,14 @@ static uint32_t vof_finddevice(const void *fdt,
> uint32_t nodeaddr)
> static const void *getprop(const void *fdt, int nodeoff, const char
> *propname,
> int *proplen, bool *write0)
> {
> - const char *unit, *prop;
> + const char *unit, *prop = fdt_getprop(fdt, nodeoff, propname, proplen);
>
> /*
> * The "name" property is not actually stored as a property in the FDT,
> * we emulate it by returning a pointer to the node's name and adjust
> * proplen to include only the name but not the unit.
> */
> - if (strcmp(propname, "name") == 0) {
> + if (!prop && strcmp(propname, "name") == 0) {
> prop = fdt_get_name(fdt, nodeoff, proplen);
> if (!prop) {
> *proplen = 0;
> @@ -196,7 +196,7 @@ static const void *getprop(const void *fdt, int nodeoff,
> const char *propname,
> if (write0) {
> *write0 = false;
> }
> - return fdt_getprop(fdt, nodeoff, propname, proplen);
> + return prop;
> }
Kind of a hack, but it'll do for now.
> This allows adding a name property to "/" different from the default but
> this does not yet fix MorphOS booting with VOF on pegasos2. I think it tries
> to query name on / and check if it's called "device-tree" in which case it
> assumes Mac hardware otherwise it goes with pegasos2 so even if we return
> nothing for name it would not matter in this case as we don't use VOF on
> Mac. If we wanted that then this would become a problem so it could be fixed
> now in advance just in case other guests may need it.
>
> > > Other than the above two problems, I've found that getting the device tree
> > > from vof returns it in reverse order compared to the board firmware if I
> > > add
> > > it the expected order. This may or may not be a problem but to avoid it I
> > > can build the tree in reverse order then it comes out right so unless
> > > there's an easy fix this should not cause a problem but may worth a
> > > comment
> > > somewhere.
> >
> > The order of things in the device tree *should* never matter. If it
> > does, that's definitely a client bug... but of course that doesn't
> > necessarily mean we won't have to work around it in practice.
>
> I don't know if it matters or not but having the device tree in the same
> order as the firmware ROM helps with comparing it for debugging but I've
> found I can solve this by building the tree in reverse order so no changes
> to VOF is needed for this, just thought adding a comment somewhere may
> clarify it but it's not really a problem.
>
> I still don't know what's MorphOS is missing, I've tried adding almost all
> misssing properties, checked what hardware is init by the firmware and tried
> to do the same in board reset code and even after that MorphOS still takes a
> different route with VOF and crashes but boots with the board firmware. I'm
> now thinking it may be either different memory organisation or the missing
> name properties that are not returned by nextprop in VOF so they are only
> appearing when explicitely queried whereas with the board firmware they are
> present as properties. With the above patch I could explicitely set it on
> nodes and test if that makes a difference.
>
> I got to this because adding more missing props or init more devices did not
> make a difference so I'm guessing it may be something else then and the only
> difference I can see compared to board firmware are the different memory
> ranges in claimed (VOF puts itself to 0 for example); and the missing name
> and additional phandle props in the device tree. MorphOS copies the whole
> device tree on startup then later it uses this copy of the device tree after
> shutting down OF with quiesce. I can imagine it may use some name props like
> that on the cpu node without checking assuming it's always there and if
> we're missing that it may cause a NULL dereference. I have no better idea
> what else could be missing so I'll test this next. If it helps I can try to
> come up with a patch to VOF to return these name props or allow setting them
> as above.
>
> Regards,
> BALATON Zoltan
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [PATCH qemu v20] spapr: Implement Open Firmware client interface, David Gibson, 2021/06/04