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

[Harsh Prateek Bora <harshpb@linux.ibm.com>]

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)) {
>