| Message ID | 20230906043333.448244-5-harshpb@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Nested PAPR API (KVM on PowerVM) | expand |
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)) {
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 --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)) {