Re: [PATCH v4 15/15] spapr: nested: Set the PCR when logical PVR is set

[Harsh Prateek Bora]

On 2/27/24 15:53, Nicholas Piggin wrote:
> On Tue Feb 20, 2024 at 6:36 PM AEST, Harsh Prateek Bora wrote:
> > From: Amit Machhiwal <amachhiw@linux.vnet.ibm.com>
> >
> > In APIv1, KVM L0 sets the PCR, while in the nested papr APIv2, this
> > doesn't work as the PCR can't be set via the guest state buffer; the
> > logical PVR is set via the GSB though.
> >
> > This change sets the PCR whenever the logical PVR is set via the GSB.
> > Also, unlike the other registers, the value 1 in a defined bit in the
> > PCR makes the affected resources unavailable and the value 0 makes
> > them available. Hence, the PCR is set accordingly.
>
> Should this be squashed in as a fix?

Yeh, it can be squashed with 10/15 GSB initialization patch, will update 
as suggested in v5.

regards,
Harsh
> Thanks,
> Nick
>
> > Signed-off-by: Amit Machhiwal <amachhiw@linux.vnet.ibm.com>
> > Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> > ---
> >   include/hw/ppc/spapr_nested.h |  9 +++++++++
> >   hw/ppc/spapr_nested.c         | 24 ++++++++++++++++++++++++
> >   2 files changed, 33 insertions(+)
> >
> > diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h
> > index da918d2dd0..f67c721f53 100644
> > --- a/include/hw/ppc/spapr_nested.h
> > +++ b/include/hw/ppc/spapr_nested.h
> > @@ -229,6 +229,15 @@ typedef struct SpaprMachineStateNestedGuest {
> >   #define GUEST_STATE_REQUEST_GUEST_WIDE       0x1
> >   #define GUEST_STATE_REQUEST_SET              0x2
> >   
> > +/* As per ISA v3.1B, following bits are reserved:
> > + *      0:2
> > + *      4:57  (ISA mentions bit 58 as well but it should be used for P10)
> > + *      61:63 (hence, haven't included PCR bits for v2.06 and v2.05
> > + *             in LOW BITS)
> > + */
> > +#define PCR_LOW_BITS   (PCR_COMPAT_3_10 | PCR_COMPAT_3_00)
> > +#define HVMASK_PCR     ~PCR_LOW_BITS
> > +
> >   #define GUEST_STATE_ELEMENT(i, sz, s, f, ptr, c) { \
> >       .id = (i),                                     \
> >       .size = (sz),                                  \
> > diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c
> > index 6e6a90616e..af8a482337 100644
> > --- a/hw/ppc/spapr_nested.c
> > +++ b/hw/ppc/spapr_nested.c
> > @@ -740,9 +740,11 @@ static void out_buf_min_size(void *a, void *b, bool set)
> >   
> >   static void copy_logical_pvr(void *a, void *b, bool set)
> >   {
> > +    SpaprMachineStateNestedGuest *guest;
> >       uint32_t *buf; /* 1 word */
> >       uint32_t *pvr_logical_ptr;
> >       uint32_t pvr_logical;
> > +    target_ulong pcr = 0;
> >   
> >       pvr_logical_ptr = a;
> >       buf = b;
> > @@ -755,6 +757,28 @@ static void copy_logical_pvr(void *a, void *b, bool set)
> >       pvr_logical = be32_to_cpu(buf[0]);
> >   
> >       *pvr_logical_ptr = pvr_logical;
> > +
> > +    if (*pvr_logical_ptr) {
> > +        switch (*pvr_logical_ptr) {
> > +            case CPU_POWERPC_LOGICAL_3_10:
> > +                pcr = PCR_COMPAT_3_10 | PCR_COMPAT_3_00;
> > +                break;
> > +            case CPU_POWERPC_LOGICAL_3_00:
> > +                pcr = PCR_COMPAT_3_00;
> > +                break;
> > +            default:
> > +                qemu_log_mask(LOG_GUEST_ERROR,
> > +                    "Could not set PCR for LPVR=0x%08x\n", *pvr_logical_ptr);
> > +                return;
> > +        }
> > +    }
> > +
> > +    guest = container_of(pvr_logical_ptr,
> > +                         struct SpaprMachineStateNestedGuest,
> > +                         pvr_logical);
> > +    for (int i = 0; i < guest->vcpus; i++) {
> > +        guest->vcpu[i].state.pcr = ~pcr | HVMASK_PCR;
> > +    }
> >   }
> >   
> >   static void copy_tb_offset(void *a, void *b, bool set)