diff mbox series

[RESEND,14/15] ppc: spapr: Implement nested PAPR hcall - H_GUEST_DELETE

Message ID 20230906043333.448244-15-harshpb@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series Nested PAPR API (KVM on PowerVM) | expand

Commit Message

Harsh Prateek Bora Sept. 6, 2023, 4:33 a.m. UTC 
This hcall is used by L1 to delete a guest entry in L0 or can also be
used to delete all guests if needed (usually in shutdown scenarios).

Signed-off-by: Michael Neuling <mikey@neuling.org>
Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
---
 hw/ppc/spapr_nested.c         | 32 ++++++++++++++++++++++++++++++++
 include/hw/ppc/spapr_nested.h |  1 +
 2 files changed, 33 insertions(+)

Comments

Nicholas Piggin Sept. 7, 2023, 2:31 a.m. UTC | #1 
On Wed Sep 6, 2023 at 2:33 PM AEST, Harsh Prateek Bora wrote:
> This hcall is used by L1 to delete a guest entry in L0 or can also be
> used to delete all guests if needed (usually in shutdown scenarios).

I'd squash with at least the create hcall.

>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> ---
>  hw/ppc/spapr_nested.c         | 32 ++++++++++++++++++++++++++++++++
>  include/hw/ppc/spapr_nested.h |  1 +
>  2 files changed, 33 insertions(+)
>
> diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c
> index 3605f27115..5afdad4990 100644
> --- a/hw/ppc/spapr_nested.c
> +++ b/hw/ppc/spapr_nested.c
> @@ -1692,6 +1692,37 @@ static void exit_process_output_buffer(PowerPCCPU *cpu,
>      return;
>  }
>  
> +static target_ulong h_guest_delete(PowerPCCPU *cpu,
> +                                   SpaprMachineState *spapr,
> +                                   target_ulong opcode,
> +                                   target_ulong *args)
> +{
> +    target_ulong flags = args[0];
> +    target_ulong lpid = args[1];
> +    struct SpaprMachineStateNestedGuest *guest;
> +
> +    if (!spapr_get_cap(spapr, SPAPR_CAP_NESTED_PAPR)) {
> +        return H_FUNCTION;
> +    }

If you only register these hcalls when you apply the cap, then you
don't need to test it, right?

Open question as to whether it's better to register hcalls when
enabling such caps, or do the tests for them here. I guess the
former makes sense.

> +
> +    /* handle flag deleteAllGuests, remaining bits reserved */

This comment is confusing. What is flag deleteAllGuests?

H_GUEST_DELETE_ALL_MASK? Is that a mask, or a flag?

> +    if (flags & ~H_GUEST_DELETE_ALL_MASK) {
> +        return H_UNSUPPORTED_FLAG;
> +    } else if (flags & H_GUEST_DELETE_ALL_MASK) {
> +        g_hash_table_destroy(spapr->nested.guests);
> +        return H_SUCCESS;
> +    }
> +
> +    guest = g_hash_table_lookup(spapr->nested.guests, GINT_TO_POINTER(lpid));
> +    if (!guest) {
> +        return H_P2;
> +    }
> +
> +    g_hash_table_remove(spapr->nested.guests, GINT_TO_POINTER(lpid));
> +
> +    return H_SUCCESS;
> +}
> +
>  void spapr_register_nested(void)
>  {
>      spapr_register_hypercall(KVMPPC_H_SET_PARTITION_TABLE, h_set_ptbl);
> @@ -1709,6 +1740,7 @@ void spapr_register_nested_phyp(void)
>      spapr_register_hypercall(H_GUEST_SET_STATE       , h_guest_set_state);
>      spapr_register_hypercall(H_GUEST_GET_STATE       , h_guest_get_state);
>      spapr_register_hypercall(H_GUEST_RUN_VCPU        , h_guest_run_vcpu);
> +    spapr_register_hypercall(H_GUEST_DELETE          , h_guest_delete);
>  }
>  
>  #else
> diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h
> index ca5d28c06e..9eb43778ad 100644
> --- a/include/hw/ppc/spapr_nested.h
> +++ b/include/hw/ppc/spapr_nested.h
> @@ -209,6 +209,7 @@
>  #define H_GUEST_GETSET_STATE_FLAG_GUEST_WIDE 0x8000000000000000 /* BE in GSB */
>  #define GUEST_STATE_REQUEST_GUEST_WIDE       0x1
>  #define GUEST_STATE_REQUEST_SET              0x2
> +#define H_GUEST_DELETE_ALL_MASK              0x8000000000000000ULL
>  
>  #define GUEST_STATE_ELEMENT(i, sz, s, f, ptr, c) { \
>      .id = (i),                                     \
Harsh Prateek Bora Oct. 3, 2023, 8:01 a.m. UTC | #2 
On 9/7/23 08:01, Nicholas Piggin wrote:
> On Wed Sep 6, 2023 at 2:33 PM AEST, Harsh Prateek Bora wrote:
>> This hcall is used by L1 to delete a guest entry in L0 or can also be
>> used to delete all guests if needed (usually in shutdown scenarios).
> 
> I'd squash with at least the create hcall.

Done.

> 
>>
>> Signed-off-by: Michael Neuling <mikey@neuling.org>
>> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
>> ---
>>   hw/ppc/spapr_nested.c         | 32 ++++++++++++++++++++++++++++++++
>>   include/hw/ppc/spapr_nested.h |  1 +
>>   2 files changed, 33 insertions(+)
>>
>> diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c
>> index 3605f27115..5afdad4990 100644
>> --- a/hw/ppc/spapr_nested.c
>> +++ b/hw/ppc/spapr_nested.c
>> @@ -1692,6 +1692,37 @@ static void exit_process_output_buffer(PowerPCCPU *cpu,
>>       return;
>>   }
>>   
>> +static target_ulong h_guest_delete(PowerPCCPU *cpu,
>> +                                   SpaprMachineState *spapr,
>> +                                   target_ulong opcode,
>> +                                   target_ulong *args)
>> +{
>> +    target_ulong flags = args[0];
>> +    target_ulong lpid = args[1];
>> +    struct SpaprMachineStateNestedGuest *guest;
>> +
>> +    if (!spapr_get_cap(spapr, SPAPR_CAP_NESTED_PAPR)) {
>> +        return H_FUNCTION;
>> +    }
> 
> If you only register these hcalls when you apply the cap, then you
> don't need to test it, right?
> 
Yes, cleaned up now.

> Open question as to whether it's better to register hcalls when
> enabling such caps, or do the tests for them here. I guess the
> former makes sense.

Yeh, I am inclined towards former as well.

> 
>> +
>> +    /* handle flag deleteAllGuests, remaining bits reserved */
> 
> This comment is confusing. What is flag deleteAllGuests?
> 
This flag, as per spec, if set, should delete all guests and the
provided guestID is ignored. Updating comment to mention the same.

> H_GUEST_DELETE_ALL_MASK? Is that a mask, or a flag?

Flag, Updating it to H_GUEST_DELETE_ALL_FLAG.

> 
>> +    if (flags & ~H_GUEST_DELETE_ALL_MASK) {
>> +        return H_UNSUPPORTED_FLAG;
>> +    } else if (flags & H_GUEST_DELETE_ALL_MASK) {
>> +        g_hash_table_destroy(spapr->nested.guests);
>> +        return H_SUCCESS;
>> +    }
>> +
>> +    guest = g_hash_table_lookup(spapr->nested.guests, GINT_TO_POINTER(lpid));
>> +    if (!guest) {
>> +        return H_P2;
>> +    }
>> +
>> +    g_hash_table_remove(spapr->nested.guests, GINT_TO_POINTER(lpid));
>> +
>> +    return H_SUCCESS;
>> +}
>> +
>>   void spapr_register_nested(void)
>>   {
>>       spapr_register_hypercall(KVMPPC_H_SET_PARTITION_TABLE, h_set_ptbl);
>> @@ -1709,6 +1740,7 @@ void spapr_register_nested_phyp(void)
>>       spapr_register_hypercall(H_GUEST_SET_STATE       , h_guest_set_state);
>>       spapr_register_hypercall(H_GUEST_GET_STATE       , h_guest_get_state);
>>       spapr_register_hypercall(H_GUEST_RUN_VCPU        , h_guest_run_vcpu);
>> +    spapr_register_hypercall(H_GUEST_DELETE          , h_guest_delete);
>>   }
>>   
>>   #else
>> diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h
>> index ca5d28c06e..9eb43778ad 100644
>> --- a/include/hw/ppc/spapr_nested.h
>> +++ b/include/hw/ppc/spapr_nested.h
>> @@ -209,6 +209,7 @@
>>   #define H_GUEST_GETSET_STATE_FLAG_GUEST_WIDE 0x8000000000000000 /* BE in GSB */
>>   #define GUEST_STATE_REQUEST_GUEST_WIDE       0x1
>>   #define GUEST_STATE_REQUEST_SET              0x2
>> +#define H_GUEST_DELETE_ALL_MASK              0x8000000000000000ULL
>>   
>>   #define GUEST_STATE_ELEMENT(i, sz, s, f, ptr, c) { \
>>       .id = (i),                                     \
>
diff mbox series

Patch

diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c
index 3605f27115..5afdad4990 100644
--- a/hw/ppc/spapr_nested.c
+++ b/hw/ppc/spapr_nested.c
@@ -1692,6 +1692,37 @@  static void exit_process_output_buffer(PowerPCCPU *cpu,
     return;
 }
 
+static target_ulong h_guest_delete(PowerPCCPU *cpu,
+                                   SpaprMachineState *spapr,
+                                   target_ulong opcode,
+                                   target_ulong *args)
+{
+    target_ulong flags = args[0];
+    target_ulong lpid = args[1];
+    struct SpaprMachineStateNestedGuest *guest;
+
+    if (!spapr_get_cap(spapr, SPAPR_CAP_NESTED_PAPR)) {
+        return H_FUNCTION;
+    }
+
+    /* handle flag deleteAllGuests, remaining bits reserved */
+    if (flags & ~H_GUEST_DELETE_ALL_MASK) {
+        return H_UNSUPPORTED_FLAG;
+    } else if (flags & H_GUEST_DELETE_ALL_MASK) {
+        g_hash_table_destroy(spapr->nested.guests);
+        return H_SUCCESS;
+    }
+
+    guest = g_hash_table_lookup(spapr->nested.guests, GINT_TO_POINTER(lpid));
+    if (!guest) {
+        return H_P2;
+    }
+
+    g_hash_table_remove(spapr->nested.guests, GINT_TO_POINTER(lpid));
+
+    return H_SUCCESS;
+}
+
 void spapr_register_nested(void)
 {
     spapr_register_hypercall(KVMPPC_H_SET_PARTITION_TABLE, h_set_ptbl);
@@ -1709,6 +1740,7 @@  void spapr_register_nested_phyp(void)
     spapr_register_hypercall(H_GUEST_SET_STATE       , h_guest_set_state);
     spapr_register_hypercall(H_GUEST_GET_STATE       , h_guest_get_state);
     spapr_register_hypercall(H_GUEST_RUN_VCPU        , h_guest_run_vcpu);
+    spapr_register_hypercall(H_GUEST_DELETE          , h_guest_delete);
 }
 
 #else
diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h
index ca5d28c06e..9eb43778ad 100644
--- a/include/hw/ppc/spapr_nested.h
+++ b/include/hw/ppc/spapr_nested.h
@@ -209,6 +209,7 @@ 
 #define H_GUEST_GETSET_STATE_FLAG_GUEST_WIDE 0x8000000000000000 /* BE in GSB */
 #define GUEST_STATE_REQUEST_GUEST_WIDE       0x1
 #define GUEST_STATE_REQUEST_SET              0x2
+#define H_GUEST_DELETE_ALL_MASK              0x8000000000000000ULL
 
 #define GUEST_STATE_ELEMENT(i, sz, s, f, ptr, c) { \
     .id = (i),                                     \