Re: [PATCH v4 10/15] spapr: nested: Initialize the GSB elements lookup table.

[Harsh Prateek Bora]

On 2/27/24 15:32, Nicholas Piggin wrote:
> On Tue Feb 20, 2024 at 6:36 PM AEST, Harsh Prateek Bora wrote:
> > Nested PAPR API provides a standard Guest State Buffer (GSB) format
> > with unique IDs for each guest state element for which get/set state is
> > supported by the API. Some of the elements are read-only and/or guest-wide.
> > Introducing helper routines for state exchange of each of the nested guest
> > state elements for which get/set state should be supported by the API.
>
> This is doing more than just adding helper routines for the GSB access.

Yes, some of the GSB elements are also introduced along with respective 
helpers.

> [snip]
>
> > +
> >   typedef struct SpaprMachineStateNested {
> >       uint64_t ptcr;
> >       uint8_t api;
> > @@ -16,6 +201,8 @@ typedef struct SpaprMachineStateNested {
> >   typedef struct SpaprMachineStateNestedGuest {
> >       uint32_t pvr_logical;
> >       unsigned long vcpus;
> > +    uint64_t parttbl[2];
> > +    uint64_t tb_offset;
> >       struct SpaprMachineStateNestedGuestVcpu *vcpu;
> >   } SpaprMachineStateNestedGuest;
> >   
> [snip]
>
> >   
> >   /*
> >    * Register state for entering a nested guest with H_ENTER_NESTED.
> > @@ -172,17 +452,40 @@ struct nested_ppc_state {
> >       uint64_t sier;
> >       uint32_t vscr;
> >       uint64_t fpscr;
> > +    int64_t dec_expiry_tb;
> > +};
> > +
> > +struct SpaprMachineStateNestedGuestVcpuRunBuf {
> > +    uint64_t addr;
> > +    uint64_t size;
> >   };
> >   
> >   typedef struct SpaprMachineStateNestedGuestVcpu {
> >       bool enabled;
> >       struct nested_ppc_state state;
> > +    struct SpaprMachineStateNestedGuestVcpuRunBuf runbufin;
> > +    struct SpaprMachineStateNestedGuestVcpuRunBuf runbufout;
> > +    int64_t tb_offset;
> > +    uint64_t hdecr_expiry_tb;
> >   } SpaprMachineStateNestedGuestVcpu;
>
> It's adding new fields in existing nested guest state
> structures. This should be explained a bit more, split into
> another patch, or moved to patches where they get used.

Yes, these new fields are actually representing GSB elements.
These elements were explained in the documentation patch which shall now
point to the documentation in the kernel docs as suggested earlier.
Let me know if we need to document additionally in this patch commit log
also.

regards,
Harsh


> Thanks,
> Nick