Re: [PATCH 4/4] ppc/spapr: Move spapr nested HV to a new file

["Nicholas Piggin" <npiggin@gmail.com>]

On Thu Jun 15, 2023 at 4:30 PM AEST, Harsh Prateek Bora wrote:
>
>
> On 6/8/23 14:43, Nicholas Piggin wrote:
> > Create spapr_nested.c for most of the nested HV implementation.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   hw/ppc/meson.build     |   1 +
> >   hw/ppc/spapr_hcall.c   | 415 +---------------------------------
> >   hw/ppc/spapr_nested.c  | 496 +++++++++++++++++++++++++++++++++++++++++
> >   include/hw/ppc/spapr.h |  61 +----
> >   4 files changed, 499 insertions(+), 474 deletions(-)
> >   create mode 100644 hw/ppc/spapr_nested.c

[snip]

> > diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c
> > new file mode 100644
> > index 0000000000..c06dd8903c
> > --- /dev/null
> > +++ b/hw/ppc/spapr_nested.c
> > @@ -0,0 +1,496 @@
> > +#include "qemu/osdep.h"
> > +#include "qemu/cutils.h"
> > +#include "exec/exec-all.h"
> > +#include "helper_regs.h"
> > +#include "hw/ppc/ppc.h"
> > +#include "hw/ppc/spapr.h"
> > +#include "hw/ppc/spapr_cpu_core.h"
> > +
> > +/*
> > + * Register state for entering a nested guest with H_ENTER_NESTED.
> > + * New member must be added at the end.
> > + */
> > +struct kvmppc_hv_guest_state {
> > +    uint64_t version;      /* version of this structure layout, must be first */
> > +    uint32_t lpid;
> > +    uint32_t vcpu_token;
> > +    /* These registers are hypervisor privileged (at least for writing) */
> > +    uint64_t lpcr;
> > +    uint64_t pcr;
> > +    uint64_t amor;
> > +    uint64_t dpdes;
> > +    uint64_t hfscr;
> > +    int64_t tb_offset;
> > +    uint64_t dawr0;
> > +    uint64_t dawrx0;
> > +    uint64_t ciabr;
> > +    uint64_t hdec_expiry;
> > +    uint64_t purr;
> > +    uint64_t spurr;
> > +    uint64_t ic;
> > +    uint64_t vtb;
> > +    uint64_t hdar;
> > +    uint64_t hdsisr;
> > +    uint64_t heir;
> > +    uint64_t asdr;
> > +    /* These are OS privileged but need to be set late in guest entry */
> > +    uint64_t srr0;
> > +    uint64_t srr1;
> > +    uint64_t sprg[4];
> > +    uint64_t pidr;
> > +    uint64_t cfar;
> > +    uint64_t ppr;
> > +    /* Version 1 ends here */
> > +    uint64_t dawr1;
> > +    uint64_t dawrx1;
> > +    /* Version 2 ends here */
> > +};
> > +
> > +/* Latest version of hv_guest_state structure */
> > +#define HV_GUEST_STATE_VERSION  2
> > +
> > +/* Linux 64-bit powerpc pt_regs struct, used by nested HV */
> > +struct kvmppc_pt_regs {
> > +    uint64_t gpr[32];
> > +    uint64_t nip;
> > +    uint64_t msr;
> > +    uint64_t orig_gpr3;    /* Used for restarting system calls */
> > +    uint64_t ctr;
> > +    uint64_t link;
> > +    uint64_t xer;
> > +    uint64_t ccr;
> > +    uint64_t softe;        /* Soft enabled/disabled */
> > +    uint64_t trap;         /* Reason for being here */
> > +    uint64_t dar;          /* Fault registers */
> > +    uint64_t dsisr;        /* on 4xx/Book-E used for ESR */
> > +    uint64_t result;       /* Result of a system call */
> > +};
>
> Now that we have a separated spapr_nested.c for nested related code,
> Can above definitions and other nested related defines (like struct 
> nested_ppc_state below) be moved to a new spapr_nested.h as well?
> Otherwise, looks good to me.

They're private to this file, so do we need to? I'm on the fence about
it, maybe because they're hcall ABI. I don't object to adding a new
spapr_nested.h for nested related things in general though.

Thanks,
Nick