diff mbox series

[v4,12/15] spapr: nested: Use correct source for parttbl info for nested PAPR API.

Message ID 20240220083609.748325-13-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 Feb. 20, 2024, 8:36 a.m. UTC 
For nested PAPR API, we use SpaprMachineStateNestedGuest struct to store
partition table info, use the same in spapr_get_pate_nested() via
helper.

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

Comments

Nicholas Piggin Feb. 27, 2024, 10:16 a.m. UTC | #1 
On Tue Feb 20, 2024 at 6:36 PM AEST, Harsh Prateek Bora wrote:
> For nested PAPR API, we use SpaprMachineStateNestedGuest struct to store
> partition table info, use the same in spapr_get_pate_nested() via
> helper.
>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> ---
>  include/hw/ppc/spapr_nested.h |  4 ++++
>  hw/ppc/spapr.c                |  2 ++
>  hw/ppc/spapr_nested.c         | 20 +++++++++++++++++++-
>  3 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h
> index 1b7e55f12a..da918d2dd0 100644
> --- a/include/hw/ppc/spapr_nested.h
> +++ b/include/hw/ppc/spapr_nested.h
> @@ -511,4 +511,8 @@ bool spapr_get_pate_nested_hv(SpaprMachineState *spapr, PowerPCCPU *cpu,
>  void spapr_nested_init(SpaprMachineState *spapr);
>  uint8_t spapr_nested_api(SpaprMachineState *spapr);
>  void spapr_nested_gsb_init(void);
> +bool spapr_get_pate_nested_papr(SpaprMachineState *spapr, PowerPCCPU *cpu,
> +                                target_ulong lpid, ppc_v3_pate_t *entry);
> +SpaprMachineStateNestedGuest *spapr_get_nested_guest(SpaprMachineState *spapr,
> +                                                     target_ulong lpid);
>  #endif /* HW_SPAPR_NESTED_H */
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 51a1be027a..3453b30a57 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1379,6 +1379,8 @@ static bool spapr_get_pate(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu,
>          assert(spapr_nested_api(spapr));
>          if (spapr_nested_api(spapr) == NESTED_API_KVM_HV) {
>              return spapr_get_pate_nested_hv(spapr, cpu, lpid, entry);
> +        } else if (spapr_nested_api(spapr) == NESTED_API_PAPR) {
> +            return spapr_get_pate_nested_papr(spapr, cpu, lpid, entry);
>          }
>          return false;
>      }

BTW. I would change these asserts to } else { g_assert_not_reached(); }

> diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c
> index aba4b25da6..0edb362709 100644
> --- a/hw/ppc/spapr_nested.c
> +++ b/hw/ppc/spapr_nested.c
> @@ -52,6 +52,19 @@ bool spapr_get_pate_nested_hv(SpaprMachineState *spapr, PowerPCCPU *cpu,
>      return true;
>  }
>  
> +bool spapr_get_pate_nested_papr(SpaprMachineState *spapr, PowerPCCPU *cpu,
> +                                target_ulong lpid, ppc_v3_pate_t *entry)
> +{
> +    SpaprMachineStateNestedGuest *guest;
> +    assert(lpid != 0);
> +    guest = spapr_get_nested_guest(spapr, lpid);
> +    assert(guest != NULL);
> +
> +    entry->dw0 = guest->parttbl[0];
> +    entry->dw1 = guest->parttbl[1];
> +    return true;
> +}

Asserts should not need to be changed to proper error handling, right?

Thanks,
Nick
Harsh Prateek Bora March 5, 2024, 8:21 a.m. UTC | #2 
Hi Nick,

On 2/27/24 15:46, Nicholas Piggin wrote:
> On Tue Feb 20, 2024 at 6:36 PM AEST, Harsh Prateek Bora wrote:
>> For nested PAPR API, we use SpaprMachineStateNestedGuest struct to store
>> partition table info, use the same in spapr_get_pate_nested() via
>> helper.
>>
>> Signed-off-by: Michael Neuling <mikey@neuling.org>
>> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
>> ---
>>   include/hw/ppc/spapr_nested.h |  4 ++++
>>   hw/ppc/spapr.c                |  2 ++
>>   hw/ppc/spapr_nested.c         | 20 +++++++++++++++++++-
>>   3 files changed, 25 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h
>> index 1b7e55f12a..da918d2dd0 100644
>> --- a/include/hw/ppc/spapr_nested.h
>> +++ b/include/hw/ppc/spapr_nested.h
>> @@ -511,4 +511,8 @@ bool spapr_get_pate_nested_hv(SpaprMachineState *spapr, PowerPCCPU *cpu,
>>   void spapr_nested_init(SpaprMachineState *spapr);
>>   uint8_t spapr_nested_api(SpaprMachineState *spapr);
>>   void spapr_nested_gsb_init(void);
>> +bool spapr_get_pate_nested_papr(SpaprMachineState *spapr, PowerPCCPU *cpu,
>> +                                target_ulong lpid, ppc_v3_pate_t *entry);
>> +SpaprMachineStateNestedGuest *spapr_get_nested_guest(SpaprMachineState *spapr,
>> +                                                     target_ulong lpid);
>>   #endif /* HW_SPAPR_NESTED_H */
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 51a1be027a..3453b30a57 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1379,6 +1379,8 @@ static bool spapr_get_pate(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu,
>>           assert(spapr_nested_api(spapr));
>>           if (spapr_nested_api(spapr) == NESTED_API_KVM_HV) {
>>               return spapr_get_pate_nested_hv(spapr, cpu, lpid, entry);
>> +        } else if (spapr_nested_api(spapr) == NESTED_API_PAPR) {
>> +            return spapr_get_pate_nested_papr(spapr, cpu, lpid, entry);
>>           }
>>           return false;
>>       }
> 
> BTW. I would change these asserts to } else { g_assert_not_reached(); }

Sure, updating as suggested.

> 
>> diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c
>> index aba4b25da6..0edb362709 100644
>> --- a/hw/ppc/spapr_nested.c
>> +++ b/hw/ppc/spapr_nested.c
>> @@ -52,6 +52,19 @@ bool spapr_get_pate_nested_hv(SpaprMachineState *spapr, PowerPCCPU *cpu,
>>       return true;
>>   }
>>   
>> +bool spapr_get_pate_nested_papr(SpaprMachineState *spapr, PowerPCCPU *cpu,
>> +                                target_ulong lpid, ppc_v3_pate_t *entry)
>> +{
>> +    SpaprMachineStateNestedGuest *guest;
>> +    assert(lpid != 0);
>> +    guest = spapr_get_nested_guest(spapr, lpid);
>> +    assert(guest != NULL);
>> +
>> +    entry->dw0 = guest->parttbl[0];
>> +    entry->dw1 = guest->parttbl[1];
>> +    return true;
>> +}
> 
> Asserts should not need to be changed to proper error handling, right?

Hmm, changing !guest check to return false as more appropriate.
lpid check shall remain an assert.

regards,
Harsh

> 
> Thanks,
> Nick
diff mbox series

Patch

diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h
index 1b7e55f12a..da918d2dd0 100644
--- a/include/hw/ppc/spapr_nested.h
+++ b/include/hw/ppc/spapr_nested.h
@@ -511,4 +511,8 @@  bool spapr_get_pate_nested_hv(SpaprMachineState *spapr, PowerPCCPU *cpu,
 void spapr_nested_init(SpaprMachineState *spapr);
 uint8_t spapr_nested_api(SpaprMachineState *spapr);
 void spapr_nested_gsb_init(void);
+bool spapr_get_pate_nested_papr(SpaprMachineState *spapr, PowerPCCPU *cpu,
+                                target_ulong lpid, ppc_v3_pate_t *entry);
+SpaprMachineStateNestedGuest *spapr_get_nested_guest(SpaprMachineState *spapr,
+                                                     target_ulong lpid);
 #endif /* HW_SPAPR_NESTED_H */
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 51a1be027a..3453b30a57 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1379,6 +1379,8 @@  static bool spapr_get_pate(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu,
         assert(spapr_nested_api(spapr));
         if (spapr_nested_api(spapr) == NESTED_API_KVM_HV) {
             return spapr_get_pate_nested_hv(spapr, cpu, lpid, entry);
+        } else if (spapr_nested_api(spapr) == NESTED_API_PAPR) {
+            return spapr_get_pate_nested_papr(spapr, cpu, lpid, entry);
         }
         return false;
     }
diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c
index aba4b25da6..0edb362709 100644
--- a/hw/ppc/spapr_nested.c
+++ b/hw/ppc/spapr_nested.c
@@ -52,6 +52,19 @@  bool spapr_get_pate_nested_hv(SpaprMachineState *spapr, PowerPCCPU *cpu,
     return true;
 }
 
+bool spapr_get_pate_nested_papr(SpaprMachineState *spapr, PowerPCCPU *cpu,
+                                target_ulong lpid, ppc_v3_pate_t *entry)
+{
+    SpaprMachineStateNestedGuest *guest;
+    assert(lpid != 0);
+    guest = spapr_get_nested_guest(spapr, lpid);
+    assert(guest != NULL);
+
+    entry->dw0 = guest->parttbl[0];
+    entry->dw1 = guest->parttbl[1];
+    return true;
+}
+
 #define PRTS_MASK      0x1f
 
 static target_ulong h_set_ptbl(PowerPCCPU *cpu,
@@ -536,7 +549,6 @@  void spapr_exit_nested(PowerPCCPU *cpu, int excp)
     }
 }
 
-static
 SpaprMachineStateNestedGuest *spapr_get_nested_guest(SpaprMachineState *spapr,
                                                      target_ulong guestid)
 {
@@ -1558,6 +1570,12 @@  bool spapr_get_pate_nested_hv(SpaprMachineState *spapr, PowerPCCPU *cpu,
     return false;
 }
 
+bool spapr_get_pate_nested_papr(SpaprMachineState *spapr, PowerPCCPU *cpu,
+                                target_ulong lpid, ppc_v3_pate_t *entry)
+{
+    return false;
+}
+
 void spapr_register_nested_papr(void)
 {
     /* DO NOTHING */