Re: [PATCH RESEND 08/15] ppc: spapr: Implement nested PAPR hcall - H_GUEST_CREATE
On 9/7/23 07:58, Nicholas Piggin wrote:
On Wed Sep 6, 2023 at 2:33 PM AEST, Harsh Prateek Bora wrote:
This hcall is used by L1 to indicate to L0 that a new nested guest needs
to be created and therefore necessary resource allocation shall be made.
The L0 uses a hash table for nested guest specific resource management.
This data structure is further utilized by other hcalls to operate on
related members during entire life cycle of the nested guest.
Similar comment for changelog re detail. Detailed specification of API
and implementation could go in comments or documentation if useful.
Sure, squashing guest create/delete together and updating commit log to
be abstract as needed.
Signed-off-by: Michael Neuling
Signed-off-by: Shivaprasad G Bhat
Signed-off-by: Harsh Prateek Bora
---
hw/ppc/spapr_nested.c | 75 +++
include/hw/ppc/spapr_nested.h | 3 ++
2 files changed, 78 insertions(+)
diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c
index 9af65f257f..09bbbfb341 100644
--- a/hw/ppc/spapr_nested.c
+++ b/hw/ppc/spapr_nested.c
@@ -444,6 +444,80 @@ static target_ulong h_guest_set_capabilities(PowerPCCPU
*cpu,
return H_SUCCESS;
}
+static void
+destroy_guest_helper(gpointer value)
+{
+struct SpaprMachineStateNestedGuest *guest = value;
+g_free(guest);
+}
+
+static target_ulong h_guest_create(PowerPCCPU *cpu,
+ SpaprMachineState *spapr,
+ target_ulong opcode,
+ target_ulong *args)
+{
+CPUPPCState *env = &cpu->env;
+target_ulong flags = args[0];
+target_ulong continue_token = args[1];
+uint64_t lpid;
+int nguests = 0;
+struct SpaprMachineStateNestedGuest *guest;
+
+if (flags) { /* don't handle any flags for now */
+return H_UNSUPPORTED_FLAG;
+}
+
+if (continue_token != -1) {
+return H_P2;
+}
+
+if (!spapr_get_cap(spapr, SPAPR_CAP_NESTED_PAPR)) {
+return H_FUNCTION;
+}
+
+if (!spapr->nested.capabilities_set) {
+return H_STATE;
+}
+
+if (!spapr->nested.guests) {
+spapr->nested.lpid_max = NESTED_GUEST_MAX;
+spapr->nested.guests = g_hash_table_new_full(NULL,
+ NULL,
+ NULL,
+ destroy_guest_helper);
Is lpid_max only used by create? Probably no need to have it in spapr
then->nested then. Also, do we even need to have a limit?
Yes, as of now, it is being used only by create and doesnt need to part
of spapr->nested. We can simply use the macro for max guests. Keeping it
to emulate a finite resource model.
For all practical purposes, nested guests in an TCG emulated L0
shouldn't reach that limit.
+}
+
+nguests = g_hash_table_size(spapr->nested.guests);
+
+if (nguests == spapr->nested.lpid_max) {
+return H_NO_MEM;
+}
+
+/* Lookup for available lpid */
+for (lpid = 1; lpid < spapr->nested.lpid_max; lpid++) {
PAPR API calls it "guest ID" I think. Should change all references to
lpid to that.
Changing it to "guestid".
+if (!(g_hash_table_lookup(spapr->nested.guests,
+ GINT_TO_POINTER(lpid {
+break;
+}
+}
+if (lpid == spapr->nested.lpid_max) {
+return H_NO_MEM;
+}
+
+guest = g_try_new0(struct SpaprMachineStateNestedGuest, 1);
+if (!guest) {
+return H_NO_MEM;
+}
+
+guest->pvr_logical = spapr->nested.pvr_base;
+
+g_hash_table_insert(spapr->nested.guests, GINT_TO_POINTER(lpid), guest);
+printf("%s: lpid: %lu (MAX: %i)\n", __func__, lpid,
spapr->nested.lpid_max);
Remove printf.
Done.
+
+env->gpr[4] = lpid;
+return H_SUCCESS;
+}
+
void spapr_register_nested(void)
{
spapr_register_hypercall(KVMPPC_H_SET_PARTITION_TABLE, h_set_ptbl);
@@ -456,6 +530,7 @@ 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);
+spapr_register_hypercall(H_GUEST_CREATE , h_guest_create);
}
#else
diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h
index a7996251cb..7841027df8 100644
--- a/include/hw/ppc/spapr_nested.h
+++ b/include/hw/ppc/spapr_nested.h
@@ -197,6 +197,9 @@
#define H_GUEST_CAP_P9_MODE_BMAP1
#define H_GUEST_CAP_P10_MODE_BMAP 2
+/* Nested PAPR API macros */
+#define NESTED_GUEST_MAX 4096
Prefix with PAPR_?
Done.
Thanks
Harsh
Thanks,
Nick
+
typedef struct SpaprMachineStateNestedGuest {
unsigned long vcpus;
struct SpaprMachineStateNestedGuestVcpu *vcpu;
Re: [PATCH RESEND 08/15] ppc: spapr: Implement nested PAPR hcall - H_GUEST_CREATE
On Wed Sep 6, 2023 at 2:33 PM AEST, Harsh Prateek Bora wrote:
> This hcall is used by L1 to indicate to L0 that a new nested guest needs
> to be created and therefore necessary resource allocation shall be made.
> The L0 uses a hash table for nested guest specific resource management.
> This data structure is further utilized by other hcalls to operate on
> related members during entire life cycle of the nested guest.
Similar comment for changelog re detail. Detailed specification of API
and implementation could go in comments or documentation if useful.
>
> Signed-off-by: Michael Neuling
> Signed-off-by: Shivaprasad G Bhat
> Signed-off-by: Harsh Prateek Bora
> ---
> hw/ppc/spapr_nested.c | 75 +++
> include/hw/ppc/spapr_nested.h | 3 ++
> 2 files changed, 78 insertions(+)
>
> diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c
> index 9af65f257f..09bbbfb341 100644
> --- a/hw/ppc/spapr_nested.c
> +++ b/hw/ppc/spapr_nested.c
> @@ -444,6 +444,80 @@ static target_ulong h_guest_set_capabilities(PowerPCCPU
> *cpu,
> return H_SUCCESS;
> }
>
> +static void
> +destroy_guest_helper(gpointer value)
> +{
> +struct SpaprMachineStateNestedGuest *guest = value;
> +g_free(guest);
> +}
> +
> +static target_ulong h_guest_create(PowerPCCPU *cpu,
> + SpaprMachineState *spapr,
> + target_ulong opcode,
> + target_ulong *args)
> +{
> +CPUPPCState *env = &cpu->env;
> +target_ulong flags = args[0];
> +target_ulong continue_token = args[1];
> +uint64_t lpid;
> +int nguests = 0;
> +struct SpaprMachineStateNestedGuest *guest;
> +
> +if (flags) { /* don't handle any flags for now */
> +return H_UNSUPPORTED_FLAG;
> +}
> +
> +if (continue_token != -1) {
> +return H_P2;
> +}
> +
> +if (!spapr_get_cap(spapr, SPAPR_CAP_NESTED_PAPR)) {
> +return H_FUNCTION;
> +}
> +
> +if (!spapr->nested.capabilities_set) {
> +return H_STATE;
> +}
> +
> +if (!spapr->nested.guests) {
> +spapr->nested.lpid_max = NESTED_GUEST_MAX;
> +spapr->nested.guests = g_hash_table_new_full(NULL,
> + NULL,
> + NULL,
> + destroy_guest_helper);
Is lpid_max only used by create? Probably no need to have it in spapr
then->nested then. Also, do we even need to have a limit?
> +}
> +
> +nguests = g_hash_table_size(spapr->nested.guests);
> +
> +if (nguests == spapr->nested.lpid_max) {
> +return H_NO_MEM;
> +}
> +
> +/* Lookup for available lpid */
> +for (lpid = 1; lpid < spapr->nested.lpid_max; lpid++) {
PAPR API calls it "guest ID" I think. Should change all references to
lpid to that.
> +if (!(g_hash_table_lookup(spapr->nested.guests,
> + GINT_TO_POINTER(lpid {
> +break;
> +}
> +}
> +if (lpid == spapr->nested.lpid_max) {
> +return H_NO_MEM;
> +}
> +
> +guest = g_try_new0(struct SpaprMachineStateNestedGuest, 1);
> +if (!guest) {
> +return H_NO_MEM;
> +}
> +
> +guest->pvr_logical = spapr->nested.pvr_base;
> +
> +g_hash_table_insert(spapr->nested.guests, GINT_TO_POINTER(lpid), guest);
> +printf("%s: lpid: %lu (MAX: %i)\n", __func__, lpid,
> spapr->nested.lpid_max);
Remove printf.
> +
> +env->gpr[4] = lpid;
> +return H_SUCCESS;
> +}
> +
> void spapr_register_nested(void)
> {
> spapr_register_hypercall(KVMPPC_H_SET_PARTITION_TABLE, h_set_ptbl);
> @@ -456,6 +530,7 @@ 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);
> +spapr_register_hypercall(H_GUEST_CREATE , h_guest_create);
> }
>
> #else
> diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h
> index a7996251cb..7841027df8 100644
> --- a/include/hw/ppc/spapr_nested.h
> +++ b/include/hw/ppc/spapr_nested.h
> @@ -197,6 +197,9 @@
> #define H_GUEST_CAP_P9_MODE_BMAP1
> #define H_GUEST_CAP_P10_MODE_BMAP 2
>
> +/* Nested PAPR API macros */
> +#define NESTED_GUEST_MAX 4096
Prefix with PAPR_?
Thanks,
Nick
> +
> typedef struct SpaprMachineStateNestedGuest {
> unsigned long vcpus;
> struct SpaprMachineStateNestedGuestVcpu *vcpu;
