| Message ID | 20220224185817.2207228-3-farosas@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ppc: nested TCG migration (KVM-on-TCG) | expand |
On Thu, Feb 24, 2022 at 03:58:15PM -0300, Fabiano Rosas wrote: > I'm seeing some stack traces in the migrated guest going through cede > and some hangs at the plpar_hcall_norets so let's make sure everything > related to cede/prod is being migrated just in case. This is a poor approach in general. Migration becomes even harder to maintain than it already is if you don't pare down the set of migrated data to something minimal and non-redundant. If you want to migrate prod, you have to give a case for why you *need* it, not "just in case". Also, you have to put this in a subsection with a needed function in order not to break compatibility. > > Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> > --- > hw/ppc/spapr_cpu_core.c | 1 + > include/hw/ppc/spapr_cpu_core.h | 2 +- > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > index ed84713960..efda7730f1 100644 > --- a/hw/ppc/spapr_cpu_core.c > +++ b/hw/ppc/spapr_cpu_core.c > @@ -179,6 +179,7 @@ static const VMStateDescription vmstate_spapr_cpu_state = { > .version_id = 1, > .minimum_version_id = 1, > .fields = (VMStateField[]) { > + VMSTATE_BOOL(prod, SpaprCpuState), > VMSTATE_END_OF_LIST() > }, > .subsections = (const VMStateDescription * []) { > diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h > index b560514560..2772689c84 100644 > --- a/include/hw/ppc/spapr_cpu_core.h > +++ b/include/hw/ppc/spapr_cpu_core.h > @@ -45,7 +45,7 @@ typedef struct SpaprCpuState { > uint64_t vpa_addr; > uint64_t slb_shadow_addr, slb_shadow_size; > uint64_t dtl_addr, dtl_size; > - bool prod; /* not migrated, only used to improve dispatch latencies */ > + bool prod; > struct ICPState *icp; > struct XiveTCTX *tctx; >
David Gibson <david@gibson.dropbear.id.au> writes: > On Thu, Feb 24, 2022 at 03:58:15PM -0300, Fabiano Rosas wrote: >> I'm seeing some stack traces in the migrated guest going through cede >> and some hangs at the plpar_hcall_norets so let's make sure everything >> related to cede/prod is being migrated just in case. > > This is a poor approach in general. Migration becomes even harder to > maintain than it already is if you don't pare down the set of migrated > data to something minimal and non-redundant. > > If you want to migrate prod, you have to give a case for why you > *need* it, not "just in case". Ah yes, I'm not actually trying to merge stuff without a good explanation. I haven't even delineated the problem properly. But I know little about migration so I need to do some probing, bear with me please. > Also, you have to put this in a subsection with a needed function in > order not to break compatibility. > >> >> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> >> --- >> hw/ppc/spapr_cpu_core.c | 1 + >> include/hw/ppc/spapr_cpu_core.h | 2 +- >> 2 files changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c >> index ed84713960..efda7730f1 100644 >> --- a/hw/ppc/spapr_cpu_core.c >> +++ b/hw/ppc/spapr_cpu_core.c >> @@ -179,6 +179,7 @@ static const VMStateDescription vmstate_spapr_cpu_state = { >> .version_id = 1, >> .minimum_version_id = 1, >> .fields = (VMStateField[]) { >> + VMSTATE_BOOL(prod, SpaprCpuState), >> VMSTATE_END_OF_LIST() >> }, >> .subsections = (const VMStateDescription * []) { >> diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h >> index b560514560..2772689c84 100644 >> --- a/include/hw/ppc/spapr_cpu_core.h >> +++ b/include/hw/ppc/spapr_cpu_core.h >> @@ -45,7 +45,7 @@ typedef struct SpaprCpuState { >> uint64_t vpa_addr; >> uint64_t slb_shadow_addr, slb_shadow_size; >> uint64_t dtl_addr, dtl_size; >> - bool prod; /* not migrated, only used to improve dispatch latencies */ >> + bool prod; >> struct ICPState *icp; >> struct XiveTCTX *tctx; >>
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index ed84713960..efda7730f1 100644 --- a/hw/ppc/spapr_cpu_core.c +++ b/hw/ppc/spapr_cpu_core.c @@ -179,6 +179,7 @@ static const VMStateDescription vmstate_spapr_cpu_state = { .version_id = 1, .minimum_version_id = 1, .fields = (VMStateField[]) { + VMSTATE_BOOL(prod, SpaprCpuState), VMSTATE_END_OF_LIST() }, .subsections = (const VMStateDescription * []) { diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h index b560514560..2772689c84 100644 --- a/include/hw/ppc/spapr_cpu_core.h +++ b/include/hw/ppc/spapr_cpu_core.h @@ -45,7 +45,7 @@ typedef struct SpaprCpuState { uint64_t vpa_addr; uint64_t slb_shadow_addr, slb_shadow_size; uint64_t dtl_addr, dtl_size; - bool prod; /* not migrated, only used to improve dispatch latencies */ + bool prod; struct ICPState *icp; struct XiveTCTX *tctx;
I'm seeing some stack traces in the migrated guest going through cede and some hangs at the plpar_hcall_norets so let's make sure everything related to cede/prod is being migrated just in case. Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> --- hw/ppc/spapr_cpu_core.c | 1 + include/hw/ppc/spapr_cpu_core.h | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)