| Message ID | 20240220083609.748325-16-harshpb@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Nested PAPR API (KVM on PowerVM) | expand |
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? 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)
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) >
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)