| Message ID | 20230906043333.448244-8-harshpb@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Nested PAPR API (KVM on PowerVM) | expand |
On Wed Sep 6, 2023 at 2:33 PM AEST, Harsh Prateek Bora wrote: > This patch implements nested PAPR hcall H_GUEST_SET_CAPABILITIES. > This is used by L1 to set capabilities of the nested guest being > created. The capabilities being set are subset of the capabilities > returned from the previous call to H_GUEST_GET_CAPABILITIES hcall. > Currently, it only supports P9/P10 capability check through PVR. > > Signed-off-by: Michael Neuling <mikey@neuling.org> > Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com> > --- > hw/ppc/spapr.c | 1 + > hw/ppc/spapr_nested.c | 46 +++++++++++++++++++++++++++++++++++ > include/hw/ppc/spapr_nested.h | 3 +++ > 3 files changed, 50 insertions(+) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index cbab7a825f..7c6f6ee25d 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -3443,6 +3443,7 @@ static void spapr_instance_init(Object *obj) > "Host serial number to advertise in guest device tree"); > /* Nested */ > spapr->nested.api = 0; > + spapr->nested.capabilities_set = false; I would actually think about moving spapr->nested init into spapr_nested.c. > } > > static void spapr_machine_finalizefn(Object *obj) > diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c > index 37f3a49be2..9af65f257f 100644 > --- a/hw/ppc/spapr_nested.c > +++ b/hw/ppc/spapr_nested.c > @@ -399,6 +399,51 @@ static target_ulong h_guest_get_capabilities(PowerPCCPU *cpu, > return H_SUCCESS; > } > > +static target_ulong h_guest_set_capabilities(PowerPCCPU *cpu, > + SpaprMachineState *spapr, > + target_ulong opcode, > + target_ulong *args) > +{ > + CPUPPCState *env = &cpu->env; > + target_ulong flags = args[0]; > + target_ulong capabilities = args[1]; > + > + if (flags) { /* don't handle any flags capabilities for now */ > + return H_PARAMETER; > + } > + > + May need to do a pass over whitespace. > + /* isn't supported */ > + if (capabilities & H_GUEST_CAPABILITIES_COPY_MEM) { > + env->gpr[4] = 0; > + return H_P2; > + } > + > + if ((env->spr[SPR_PVR] & CPU_POWERPC_POWER_SERVER_MASK) == > + (CPU_POWERPC_POWER9_BASE)) { > + /* We are a P9 */ > + if (!(capabilities & H_GUEST_CAPABILITIES_P9_MODE)) { > + env->gpr[4] = 1; > + return H_P2; > + } > + } > + > + if ((env->spr[SPR_PVR] & CPU_POWERPC_POWER_SERVER_MASK) == > + (CPU_POWERPC_POWER10_BASE)) { > + /* We are a P10 */ The 2 comments above aren't helpful. Just remove them. > + if (!(capabilities & H_GUEST_CAPABILITIES_P10_MODE)) { > + env->gpr[4] = 2; > + return H_P2; > + } > + } > + > + spapr->nested.capabilities_set = true; Is it okay to set twice? If not, add a check. If yes, remove capabilities_set until it's needed. > + > + spapr->nested.pvr_base = env->spr[SPR_PVR]; > + > + return H_SUCCESS; > +} > + > void spapr_register_nested(void) > { > spapr_register_hypercall(KVMPPC_H_SET_PARTITION_TABLE, h_set_ptbl); > @@ -410,6 +455,7 @@ void spapr_register_nested(void) > void spapr_register_nested_phyp(void) > { > spapr_register_hypercall(H_GUEST_GET_CAPABILITIES, h_guest_get_capabilities); > + spapr_register_hypercall(H_GUEST_SET_CAPABILITIES, h_guest_set_capabilities); > } > > #else > diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h > index ce198e9f70..a7996251cb 100644 > --- a/include/hw/ppc/spapr_nested.h > +++ b/include/hw/ppc/spapr_nested.h > @@ -193,6 +193,9 @@ > #define H_GUEST_CAPABILITIES_COPY_MEM 0x8000000000000000 > #define H_GUEST_CAPABILITIES_P9_MODE 0x4000000000000000 > #define H_GUEST_CAPABILITIES_P10_MODE 0x2000000000000000 > +#define H_GUEST_CAP_COPY_MEM_BMAP 0 > +#define H_GUEST_CAP_P9_MODE_BMAP 1 > +#define H_GUEST_CAP_P10_MODE_BMAP 2 > > typedef struct SpaprMachineStateNestedGuest { > unsigned long vcpus;
On 9/7/23 07:39, Nicholas Piggin wrote: > On Wed Sep 6, 2023 at 2:33 PM AEST, Harsh Prateek Bora wrote: >> This patch implements nested PAPR hcall H_GUEST_SET_CAPABILITIES. >> This is used by L1 to set capabilities of the nested guest being >> created. The capabilities being set are subset of the capabilities >> returned from the previous call to H_GUEST_GET_CAPABILITIES hcall. >> Currently, it only supports P9/P10 capability check through PVR. >> >> Signed-off-by: Michael Neuling <mikey@neuling.org> >> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com> >> --- >> hw/ppc/spapr.c | 1 + >> hw/ppc/spapr_nested.c | 46 +++++++++++++++++++++++++++++++++++ >> include/hw/ppc/spapr_nested.h | 3 +++ >> 3 files changed, 50 insertions(+) >> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index cbab7a825f..7c6f6ee25d 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -3443,6 +3443,7 @@ static void spapr_instance_init(Object *obj) >> "Host serial number to advertise in guest device tree"); >> /* Nested */ >> spapr->nested.api = 0; >> + spapr->nested.capabilities_set = false; > > I would actually think about moving spapr->nested init into > spapr_nested.c. > Agree, moved. >> } >> >> static void spapr_machine_finalizefn(Object *obj) >> diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c >> index 37f3a49be2..9af65f257f 100644 >> --- a/hw/ppc/spapr_nested.c >> +++ b/hw/ppc/spapr_nested.c >> @@ -399,6 +399,51 @@ static target_ulong h_guest_get_capabilities(PowerPCCPU *cpu, >> return H_SUCCESS; >> } >> >> +static target_ulong h_guest_set_capabilities(PowerPCCPU *cpu, >> + SpaprMachineState *spapr, >> + target_ulong opcode, >> + target_ulong *args) >> +{ >> + CPUPPCState *env = &cpu->env; >> + target_ulong flags = args[0]; >> + target_ulong capabilities = args[1]; >> + >> + if (flags) { /* don't handle any flags capabilities for now */ >> + return H_PARAMETER; >> + } >> + >> + > > May need to do a pass over whitespace. > Sure, done. >> + /* isn't supported */ >> + if (capabilities & H_GUEST_CAPABILITIES_COPY_MEM) { >> + env->gpr[4] = 0; >> + return H_P2; >> + } >> + >> + if ((env->spr[SPR_PVR] & CPU_POWERPC_POWER_SERVER_MASK) == >> + (CPU_POWERPC_POWER9_BASE)) { >> + /* We are a P9 */ >> + if (!(capabilities & H_GUEST_CAPABILITIES_P9_MODE)) { >> + env->gpr[4] = 1; >> + return H_P2; >> + } >> + } >> + >> + if ((env->spr[SPR_PVR] & CPU_POWERPC_POWER_SERVER_MASK) == >> + (CPU_POWERPC_POWER10_BASE)) { >> + /* We are a P10 */ > > The 2 comments above aren't helpful. Just remove them. > Sure, done. >> + if (!(capabilities & H_GUEST_CAPABILITIES_P10_MODE)) { >> + env->gpr[4] = 2; >> + return H_P2; >> + } >> + } >> + >> + spapr->nested.capabilities_set = true; > > Is it okay to set twice? If not, add a check. If yes, remove > capabilities_set until it's needed. > Thanks for pointing it out, adding a check as appropriate. Thanks Harsh >> + >> + spapr->nested.pvr_base = env->spr[SPR_PVR]; >> + >> + return H_SUCCESS; >> +} >> + >> void spapr_register_nested(void) >> { >> spapr_register_hypercall(KVMPPC_H_SET_PARTITION_TABLE, h_set_ptbl); >> @@ -410,6 +455,7 @@ void spapr_register_nested(void) >> void spapr_register_nested_phyp(void) >> { >> spapr_register_hypercall(H_GUEST_GET_CAPABILITIES, h_guest_get_capabilities); >> + spapr_register_hypercall(H_GUEST_SET_CAPABILITIES, h_guest_set_capabilities); >> } >> >> #else >> diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h >> index ce198e9f70..a7996251cb 100644 >> --- a/include/hw/ppc/spapr_nested.h >> +++ b/include/hw/ppc/spapr_nested.h >> @@ -193,6 +193,9 @@ >> #define H_GUEST_CAPABILITIES_COPY_MEM 0x8000000000000000 >> #define H_GUEST_CAPABILITIES_P9_MODE 0x4000000000000000 >> #define H_GUEST_CAPABILITIES_P10_MODE 0x2000000000000000 >> +#define H_GUEST_CAP_COPY_MEM_BMAP 0 >> +#define H_GUEST_CAP_P9_MODE_BMAP 1 >> +#define H_GUEST_CAP_P10_MODE_BMAP 2 >> >> typedef struct SpaprMachineStateNestedGuest { >> unsigned long vcpus; >
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index cbab7a825f..7c6f6ee25d 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3443,6 +3443,7 @@ static void spapr_instance_init(Object *obj) "Host serial number to advertise in guest device tree"); /* Nested */ spapr->nested.api = 0; + spapr->nested.capabilities_set = false; } static void spapr_machine_finalizefn(Object *obj) diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c index 37f3a49be2..9af65f257f 100644 --- a/hw/ppc/spapr_nested.c +++ b/hw/ppc/spapr_nested.c @@ -399,6 +399,51 @@ static target_ulong h_guest_get_capabilities(PowerPCCPU *cpu, return H_SUCCESS; } +static target_ulong h_guest_set_capabilities(PowerPCCPU *cpu, + SpaprMachineState *spapr, + target_ulong opcode, + target_ulong *args) +{ + CPUPPCState *env = &cpu->env; + target_ulong flags = args[0]; + target_ulong capabilities = args[1]; + + if (flags) { /* don't handle any flags capabilities for now */ + return H_PARAMETER; + } + + + /* isn't supported */ + if (capabilities & H_GUEST_CAPABILITIES_COPY_MEM) { + env->gpr[4] = 0; + return H_P2; + } + + if ((env->spr[SPR_PVR] & CPU_POWERPC_POWER_SERVER_MASK) == + (CPU_POWERPC_POWER9_BASE)) { + /* We are a P9 */ + if (!(capabilities & H_GUEST_CAPABILITIES_P9_MODE)) { + env->gpr[4] = 1; + return H_P2; + } + } + + if ((env->spr[SPR_PVR] & CPU_POWERPC_POWER_SERVER_MASK) == + (CPU_POWERPC_POWER10_BASE)) { + /* We are a P10 */ + if (!(capabilities & H_GUEST_CAPABILITIES_P10_MODE)) { + env->gpr[4] = 2; + return H_P2; + } + } + + spapr->nested.capabilities_set = true; + + spapr->nested.pvr_base = env->spr[SPR_PVR]; + + return H_SUCCESS; +} + void spapr_register_nested(void) { spapr_register_hypercall(KVMPPC_H_SET_PARTITION_TABLE, h_set_ptbl); @@ -410,6 +455,7 @@ void spapr_register_nested(void) void spapr_register_nested_phyp(void) { spapr_register_hypercall(H_GUEST_GET_CAPABILITIES, h_guest_get_capabilities); + spapr_register_hypercall(H_GUEST_SET_CAPABILITIES, h_guest_set_capabilities); } #else diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h index ce198e9f70..a7996251cb 100644 --- a/include/hw/ppc/spapr_nested.h +++ b/include/hw/ppc/spapr_nested.h @@ -193,6 +193,9 @@ #define H_GUEST_CAPABILITIES_COPY_MEM 0x8000000000000000 #define H_GUEST_CAPABILITIES_P9_MODE 0x4000000000000000 #define H_GUEST_CAPABILITIES_P10_MODE 0x2000000000000000 +#define H_GUEST_CAP_COPY_MEM_BMAP 0 +#define H_GUEST_CAP_P9_MODE_BMAP 1 +#define H_GUEST_CAP_P10_MODE_BMAP 2 typedef struct SpaprMachineStateNestedGuest { unsigned long vcpus;