Re: [PATCH qemu v11] spapr: Implement Open Firmware client interface

[Alexey Kardashevskiy]

On 07/12/2020 22:48, BALATON Zoltan wrote:

> > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> > index 2e89e36cfbdc..048bf49592aa 100644
> > --- a/include/hw/ppc/spapr.h
> > +++ b/include/hw/ppc/spapr.h
> > @@ -175,6 +175,13 @@ struct SpaprMachineState {
> >     long kernel_size;
> >     bool kernel_le;
> >     uint64_t kernel_addr;
> > +    bool vof; /* Virtual Open Firmware */
> > +    uint32_t rtas_base;
> > +    GArray *claimed; /* array of SpaprOfClaimed */
> > +    uint64_t claimed_base;
> > +    GHashTable *of_instances; /* ihandle -> SpaprOfInstance */
> > +    uint32_t of_instance_last;
> > +    char *bootargs;
>
> Are these really state for vof so is it better to place them in a 
> separate of_state struct instead of adding to the machine state? I'm not 
> interested in spapr but interested in using vof as a replacement 
> firmware for other machines so clear separation of what is spapr 
> specific and what is vof specific would help me (and maybe also other 
> reviewers to tell how much impact this really has on spapr which seems 
> to be a concern of Greg).

This is a very good point, I'll separate VOF from the rest, may be even 
at QOM level. I was also thinking of adding a pseries-vof machine type 
but this is probably an overkill.

Out of curiosity - how are you going to use this VOF anyway, for what 
machine type?


ps: I'll keep the braces ;)


--
Alexey