diff mbox series

[RESEND,04/15] ppc: spapr: Start using nested.api for nested kvm-hv api

Message ID 20230906043333.448244-5-harshpb@linux.ibm.com
State New
Headers show
Series Nested PAPR API (KVM on PowerVM) | expand

Commit Message

Harsh Prateek Bora Sept. 6, 2023, 4:33 a.m. UTC 
With this patch, isolating kvm-hv nested api code to be executed only
when cap-nested-hv is set. This helps keeping api specific logic
mutually exclusive.

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

Comments

Nicholas Piggin Sept. 7, 2023, 1:35 a.m. UTC | #1 
On Wed Sep 6, 2023 at 2:33 PM AEST, Harsh Prateek Bora wrote:
> With this patch, isolating kvm-hv nested api code to be executed only
> when cap-nested-hv is set. This helps keeping api specific logic
> mutually exclusive.

Changelog needs a bit of improvement. Emphasis on "why" for changelogs.
If you take a changeset that makes a single logical change to the code,
you should be able to understand why that is done. You could make some
assumptions about the bigger series when it comes to details so don't
have to explain from first principles. But if it's easy to explain why
the high level, you could.

Why are we adding this fundamentally? So that the spapr nested code can
be extended to support a second API.

This patch should add the api field to the struct, and also the
NESTED_API_KVM_HV definition.

Thanks,
Nick

>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> ---
>  hw/ppc/spapr.c      | 7 ++++++-
>  hw/ppc/spapr_caps.c | 1 +
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index e44686b04d..0aa9f21516 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1334,8 +1334,11 @@ static bool spapr_get_pate(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu,
>          /* Copy PATE1:GR into PATE0:HR */
>          entry->dw0 = spapr->patb_entry & PATE0_HR;
>          entry->dw1 = spapr->patb_entry;
> +        return true;
> +    }
> +    assert(spapr->nested.api);
>  
> -    } else {
> +    if (spapr->nested.api == NESTED_API_KVM_HV) {
>          uint64_t patb, pats;
>  
>          assert(lpid != 0);
> @@ -3437,6 +3440,8 @@ static void spapr_instance_init(Object *obj)
>          spapr_get_host_serial, spapr_set_host_serial);
>      object_property_set_description(obj, "host-serial",
>          "Host serial number to advertise in guest device tree");
> +    /* Nested */
> +    spapr->nested.api = 0;
>  }
>  
>  static void spapr_machine_finalizefn(Object *obj)
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index 5a0755d34f..a3a790b026 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -454,6 +454,7 @@ static void cap_nested_kvm_hv_apply(SpaprMachineState *spapr,
>          return;
>      }
>  
> +    spapr->nested.api = NESTED_API_KVM_HV;
>      if (kvm_enabled()) {
>          if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_3_00, 0,
>                                spapr->max_compat_pvr)) {
Harsh Prateek Bora Sept. 11, 2023, 8:18 a.m. UTC | #2 
On 9/7/23 07:05, Nicholas Piggin wrote:
> On Wed Sep 6, 2023 at 2:33 PM AEST, Harsh Prateek Bora wrote:
>> With this patch, isolating kvm-hv nested api code to be executed only
>> when cap-nested-hv is set. This helps keeping api specific logic
>> mutually exclusive.
> 
> Changelog needs a bit of improvement. Emphasis on "why" for changelogs.
> If you take a changeset that makes a single logical change to the code,
> you should be able to understand why that is done. You could make some
> assumptions about the bigger series when it comes to details so don't
> have to explain from first principles. But if it's easy to explain why
> the high level, you could.
> 
> Why are we adding this fundamentally? So that the spapr nested code can
> be extended to support a second API.
> 
> This patch should add the api field to the struct, and also the
> NESTED_API_KVM_HV definition.

Sure, folding related changes (struct member, macros) into this patch
and updating changelog as suggested sounds more meaningful to me too.

regards,
Harsh

> 
> Thanks,
> Nick
> 
>>
>> Signed-off-by: Michael Neuling <mikey@neuling.org>
>> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
>> ---
>>   hw/ppc/spapr.c      | 7 ++++++-
>>   hw/ppc/spapr_caps.c | 1 +
>>   2 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index e44686b04d..0aa9f21516 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1334,8 +1334,11 @@ static bool spapr_get_pate(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu,
>>           /* Copy PATE1:GR into PATE0:HR */
>>           entry->dw0 = spapr->patb_entry & PATE0_HR;
>>           entry->dw1 = spapr->patb_entry;
>> +        return true;
>> +    }
>> +    assert(spapr->nested.api);
>>   
>> -    } else {
>> +    if (spapr->nested.api == NESTED_API_KVM_HV) {
>>           uint64_t patb, pats;
>>   
>>           assert(lpid != 0);
>> @@ -3437,6 +3440,8 @@ static void spapr_instance_init(Object *obj)
>>           spapr_get_host_serial, spapr_set_host_serial);
>>       object_property_set_description(obj, "host-serial",
>>           "Host serial number to advertise in guest device tree");
>> +    /* Nested */
>> +    spapr->nested.api = 0;
>>   }
>>   
>>   static void spapr_machine_finalizefn(Object *obj)
>> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
>> index 5a0755d34f..a3a790b026 100644
>> --- a/hw/ppc/spapr_caps.c
>> +++ b/hw/ppc/spapr_caps.c
>> @@ -454,6 +454,7 @@ static void cap_nested_kvm_hv_apply(SpaprMachineState *spapr,
>>           return;
>>       }
>>   
>> +    spapr->nested.api = NESTED_API_KVM_HV;
>>       if (kvm_enabled()) {
>>           if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_3_00, 0,
>>                                 spapr->max_compat_pvr)) {
>
diff mbox series

Patch

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index e44686b04d..0aa9f21516 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1334,8 +1334,11 @@  static bool spapr_get_pate(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu,
         /* Copy PATE1:GR into PATE0:HR */
         entry->dw0 = spapr->patb_entry & PATE0_HR;
         entry->dw1 = spapr->patb_entry;
+        return true;
+    }
+    assert(spapr->nested.api);
 
-    } else {
+    if (spapr->nested.api == NESTED_API_KVM_HV) {
         uint64_t patb, pats;
 
         assert(lpid != 0);
@@ -3437,6 +3440,8 @@  static void spapr_instance_init(Object *obj)
         spapr_get_host_serial, spapr_set_host_serial);
     object_property_set_description(obj, "host-serial",
         "Host serial number to advertise in guest device tree");
+    /* Nested */
+    spapr->nested.api = 0;
 }
 
 static void spapr_machine_finalizefn(Object *obj)
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 5a0755d34f..a3a790b026 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -454,6 +454,7 @@  static void cap_nested_kvm_hv_apply(SpaprMachineState *spapr,
         return;
     }
 
+    spapr->nested.api = NESTED_API_KVM_HV;
     if (kvm_enabled()) {
         if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_3_00, 0,
                               spapr->max_compat_pvr)) {