* [Qemu-devel] [RFC PATCH 0/2] ppc/spapr: Fix migration of radix guests @ 2017-05-08 9:06 Bharata B Rao 2017-05-08 9:06 ` [Qemu-devel] [RFC PATCH 1/2] spapr: Make h_register_process_table hcall flags global Bharata B Rao 2017-05-08 9:06 ` [Qemu-devel] [RFC PATCH 2/2] spapr: Fix migration of Radix guests Bharata B Rao 0 siblings, 2 replies; 6+ messages in thread From: Bharata B Rao @ 2017-05-08 9:06 UTC (permalink / raw) To: qemu-devel; +Cc: qemu-ppc, david, sam.bobroff, rnsastry, Bharata B Rao Migrating radix guests fails with the following error on the target side: qemu-system-ppc64: htab_load() bad index 2113929216 (5378+0 entries) in htab stream (htab_shift=0) qemu-system-ppc64: error while loading state for instance 0x0 of device 'spapr/htab' qemu-system-ppc64: load of migration failed: Invalid argument Fix this by not registering htab savevm handler for radix case. Patches apply against ppc-for-2.10 branch of dwg's tree. Bharata B Rao (2): spapr: Make h_register_process_table hcall flags global spapr: Fix migration of Radix guests hw/ppc/spapr.c | 18 +++++++++++++++--- hw/ppc/spapr_hcall.c | 36 +++++++++++++++++++----------------- include/hw/ppc/spapr.h | 13 +++++++++++++ 3 files changed, 47 insertions(+), 20 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [RFC PATCH 1/2] spapr: Make h_register_process_table hcall flags global 2017-05-08 9:06 [Qemu-devel] [RFC PATCH 0/2] ppc/spapr: Fix migration of radix guests Bharata B Rao @ 2017-05-08 9:06 ` Bharata B Rao 2017-05-11 0:14 ` David Gibson 2017-05-08 9:06 ` [Qemu-devel] [RFC PATCH 2/2] spapr: Fix migration of Radix guests Bharata B Rao 1 sibling, 1 reply; 6+ messages in thread From: Bharata B Rao @ 2017-05-08 9:06 UTC (permalink / raw) To: qemu-devel; +Cc: qemu-ppc, david, sam.bobroff, rnsastry, Bharata B Rao The flags used in h_register_process_table hcall are needed in spapr.c and hence move them to a header file. While doing so, give them slightly specific names. Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> --- hw/ppc/spapr_hcall.c | 31 ++++++++++++++----------------- include/hw/ppc/spapr.h | 10 ++++++++++ 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c index 0d608d6..3600b0e 100644 --- a/hw/ppc/spapr_hcall.c +++ b/hw/ppc/spapr_hcall.c @@ -924,13 +924,6 @@ static void spapr_check_setup_free_hpt(sPAPRMachineState *spapr, return; } -#define FLAGS_MASK 0x01FULL -#define FLAG_MODIFY 0x10 -#define FLAG_REGISTER 0x08 -#define FLAG_RADIX 0x04 -#define FLAG_HASH_PROC_TBL 0x02 -#define FLAG_GTSE 0x01 - static target_ulong h_register_process_table(PowerPCCPU *cpu, sPAPRMachineState *spapr, target_ulong opcode, @@ -943,12 +936,13 @@ static target_ulong h_register_process_table(PowerPCCPU *cpu, target_ulong table_size = args[3]; uint64_t cproc; - if (flags & ~FLAGS_MASK) { /* Check no reserved bits are set */ + if (flags & ~SPAPR_PROC_TABLE_MASK) { /* Check no reserved bits are set */ return H_PARAMETER; } - if (flags & FLAG_MODIFY) { - if (flags & FLAG_REGISTER) { - if (flags & FLAG_RADIX) { /* Register new RADIX process table */ + if (flags & SPAPR_PROC_TABLE_MODIFY) { + if (flags & SPAPR_PROC_TABLE_REGISTER) { + if (flags & SPAPR_PROC_TABLE_RADIX) { + /* Register new RADIX process table */ if (proc_tbl & 0xfff || proc_tbl >> 60) { return H_P2; } else if (page_size) { @@ -958,7 +952,8 @@ static target_ulong h_register_process_table(PowerPCCPU *cpu, } cproc = PATBE1_GR | proc_tbl | table_size; } else { /* Register new HPT process table */ - if (flags & FLAG_HASH_PROC_TBL) { /* Hash with Segment Tables */ + if (flags & SPAPR_PROC_TABLE_HPT_PT) { + /* Hash with Segment Tables */ /* TODO - Not Supported */ /* Technically caused by flag bits => H_PARAMETER */ return H_PARAMETER; @@ -981,7 +976,8 @@ static target_ulong h_register_process_table(PowerPCCPU *cpu, cproc = spapr->patb_entry & PATBE1_GR; } } else { /* Maintain current registration */ - if (!(flags & FLAG_RADIX) != !(spapr->patb_entry & PATBE1_GR)) { + if (!(flags & SPAPR_PROC_TABLE_RADIX) != + !(spapr->patb_entry & PATBE1_GR)) { /* Technically caused by flag bits => H_PARAMETER */ return H_PARAMETER; /* Existing Process Table Mismatch */ } @@ -996,13 +992,14 @@ static target_ulong h_register_process_table(PowerPCCPU *cpu, /* Update the UPRT and GTSE bits in the LPCR for all cpus */ CPU_FOREACH(cs) { set_spr(cs, SPR_LPCR, LPCR_UPRT | LPCR_GTSE, - ((flags & (FLAG_RADIX | FLAG_HASH_PROC_TBL)) ? LPCR_UPRT : 0) | - ((flags & FLAG_GTSE) ? LPCR_GTSE : 0)); + ((flags & (SPAPR_PROC_TABLE_RADIX | SPAPR_PROC_TABLE_HPT_PT)) ? + LPCR_UPRT : 0) | ((flags & SPAPR_PROC_TABLE_GTSE) ? + SPAPR_PROC_TABLE_GTSE : 0)); } if (kvm_enabled()) { - return kvmppc_configure_v3_mmu(cpu, flags & FLAG_RADIX, - flags & FLAG_GTSE, cproc); + return kvmppc_configure_v3_mmu(cpu, flags & SPAPR_PROC_TABLE_RADIX, + flags & SPAPR_PROC_TABLE_GTSE, cproc); } return H_SUCCESS; } diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index 5802f88..a692e63 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -681,4 +681,14 @@ int spapr_rng_populate_dt(void *fdt); void spapr_do_system_reset_on_cpu(CPUState *cs, run_on_cpu_data arg); +/* + * Defines for flag value used in H_REGISTER_PROC_TBL hcall. + */ +#define SPAPR_PROC_TABLE_MASK 0x01FULL +#define SPAPR_PROC_TABLE_MODIFY 0x10 +#define SPAPR_PROC_TABLE_REGISTER 0x08 +#define SPAPR_PROC_TABLE_RADIX 0x04 +#define SPAPR_PROC_TABLE_HPT_PT 0x02 +#define SPAPR_PROC_TABLE_GTSE 0x01 + #endif /* HW_SPAPR_H */ -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 1/2] spapr: Make h_register_process_table hcall flags global 2017-05-08 9:06 ` [Qemu-devel] [RFC PATCH 1/2] spapr: Make h_register_process_table hcall flags global Bharata B Rao @ 2017-05-11 0:14 ` David Gibson 0 siblings, 0 replies; 6+ messages in thread From: David Gibson @ 2017-05-11 0:14 UTC (permalink / raw) To: Bharata B Rao; +Cc: qemu-devel, qemu-ppc, sam.bobroff, rnsastry [-- Attachment #1: Type: text/plain, Size: 5155 bytes --] On Mon, May 08, 2017 at 02:36:16PM +0530, Bharata B Rao wrote: > The flags used in h_register_process_table hcall are needed in spapr.c > and hence move them to a header file. While doing so, give them > slightly specific names. > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > --- > hw/ppc/spapr_hcall.c | 31 ++++++++++++++----------------- > include/hw/ppc/spapr.h | 10 ++++++++++ > 2 files changed, 24 insertions(+), 17 deletions(-) > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > index 0d608d6..3600b0e 100644 > --- a/hw/ppc/spapr_hcall.c > +++ b/hw/ppc/spapr_hcall.c > @@ -924,13 +924,6 @@ static void spapr_check_setup_free_hpt(sPAPRMachineState *spapr, > return; > } > > -#define FLAGS_MASK 0x01FULL > -#define FLAG_MODIFY 0x10 > -#define FLAG_REGISTER 0x08 > -#define FLAG_RADIX 0x04 > -#define FLAG_HASH_PROC_TBL 0x02 > -#define FLAG_GTSE 0x01 > - > static target_ulong h_register_process_table(PowerPCCPU *cpu, > sPAPRMachineState *spapr, > target_ulong opcode, > @@ -943,12 +936,13 @@ static target_ulong h_register_process_table(PowerPCCPU *cpu, > target_ulong table_size = args[3]; > uint64_t cproc; > > - if (flags & ~FLAGS_MASK) { /* Check no reserved bits are set */ > + if (flags & ~SPAPR_PROC_TABLE_MASK) { /* Check no reserved bits are set */ > return H_PARAMETER; > } > - if (flags & FLAG_MODIFY) { > - if (flags & FLAG_REGISTER) { > - if (flags & FLAG_RADIX) { /* Register new RADIX process table */ > + if (flags & SPAPR_PROC_TABLE_MODIFY) { > + if (flags & SPAPR_PROC_TABLE_REGISTER) { > + if (flags & SPAPR_PROC_TABLE_RADIX) { > + /* Register new RADIX process table */ > if (proc_tbl & 0xfff || proc_tbl >> 60) { > return H_P2; > } else if (page_size) { > @@ -958,7 +952,8 @@ static target_ulong h_register_process_table(PowerPCCPU *cpu, > } > cproc = PATBE1_GR | proc_tbl | table_size; > } else { /* Register new HPT process table */ > - if (flags & FLAG_HASH_PROC_TBL) { /* Hash with Segment Tables */ > + if (flags & SPAPR_PROC_TABLE_HPT_PT) { > + /* Hash with Segment Tables */ > /* TODO - Not Supported */ > /* Technically caused by flag bits => H_PARAMETER */ > return H_PARAMETER; > @@ -981,7 +976,8 @@ static target_ulong h_register_process_table(PowerPCCPU *cpu, > cproc = spapr->patb_entry & PATBE1_GR; > } > } else { /* Maintain current registration */ > - if (!(flags & FLAG_RADIX) != !(spapr->patb_entry & PATBE1_GR)) { > + if (!(flags & SPAPR_PROC_TABLE_RADIX) != > + !(spapr->patb_entry & PATBE1_GR)) { > /* Technically caused by flag bits => H_PARAMETER */ > return H_PARAMETER; /* Existing Process Table Mismatch */ > } > @@ -996,13 +992,14 @@ static target_ulong h_register_process_table(PowerPCCPU *cpu, > /* Update the UPRT and GTSE bits in the LPCR for all cpus */ > CPU_FOREACH(cs) { > set_spr(cs, SPR_LPCR, LPCR_UPRT | LPCR_GTSE, > - ((flags & (FLAG_RADIX | FLAG_HASH_PROC_TBL)) ? LPCR_UPRT : 0) | > - ((flags & FLAG_GTSE) ? LPCR_GTSE : 0)); > + ((flags & (SPAPR_PROC_TABLE_RADIX | SPAPR_PROC_TABLE_HPT_PT)) ? > + LPCR_UPRT : 0) | ((flags & SPAPR_PROC_TABLE_GTSE) ? > + SPAPR_PROC_TABLE_GTSE : 0)); > } > > if (kvm_enabled()) { > - return kvmppc_configure_v3_mmu(cpu, flags & FLAG_RADIX, > - flags & FLAG_GTSE, cproc); > + return kvmppc_configure_v3_mmu(cpu, flags & SPAPR_PROC_TABLE_RADIX, > + flags & SPAPR_PROC_TABLE_GTSE, cproc); > } > return H_SUCCESS; > } > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 5802f88..a692e63 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -681,4 +681,14 @@ int spapr_rng_populate_dt(void *fdt); > > void spapr_do_system_reset_on_cpu(CPUState *cs, run_on_cpu_data arg); > > +/* > + * Defines for flag value used in H_REGISTER_PROC_TBL hcall. > + */ > +#define SPAPR_PROC_TABLE_MASK 0x01FULL > +#define SPAPR_PROC_TABLE_MODIFY 0x10 > +#define SPAPR_PROC_TABLE_REGISTER 0x08 > +#define SPAPR_PROC_TABLE_RADIX 0x04 > +#define SPAPR_PROC_TABLE_HPT_PT 0x02 > +#define SPAPR_PROC_TABLE_GTSE 0x01 > + > #endif /* HW_SPAPR_H */ -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [RFC PATCH 2/2] spapr: Fix migration of Radix guests 2017-05-08 9:06 [Qemu-devel] [RFC PATCH 0/2] ppc/spapr: Fix migration of radix guests Bharata B Rao 2017-05-08 9:06 ` [Qemu-devel] [RFC PATCH 1/2] spapr: Make h_register_process_table hcall flags global Bharata B Rao @ 2017-05-08 9:06 ` Bharata B Rao 2017-05-11 1:02 ` David Gibson 1 sibling, 1 reply; 6+ messages in thread From: Bharata B Rao @ 2017-05-08 9:06 UTC (permalink / raw) To: qemu-devel; +Cc: qemu-ppc, david, sam.bobroff, rnsastry, Bharata B Rao Currently HTAB savevm handlers get registered by default and migration of radix guest will fail. - Ensure that HTAB savevm handlers are not registered for radix case. - Ensure that we issue KVM_PPC_CONFIGURE_V3_MMU for radix case post migration. TODO: Right now I have delayed the HTAB savevm handler registration to CAS call where we know if the guest is radix or hash. Another approach is to let the HTAB handlers to be registered by default (as it is being done currently, but unregister them from CAS when we discover radix capability). Reported-by: Nageswara R Sastry <rnsastry@linux.vnet.ibm.com> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> --- hw/ppc/spapr.c | 18 +++++++++++++++--- hw/ppc/spapr_hcall.c | 5 +++++ include/hw/ppc/spapr.h | 3 +++ 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index e2dc77c..e14f55c 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1436,6 +1436,14 @@ static int spapr_post_load(void *opaque, int version_id) err = spapr_rtc_import_offset(&spapr->rtc, spapr->rtc_offset); } + if (spapr->patb_entry && (spapr->patb_flags & SPAPR_PROC_TABLE_RADIX) && + kvmppc_has_cap_mmu_radix() && kvm_enabled()) { + err = kvmppc_configure_v3_mmu(POWERPC_CPU(first_cpu), + spapr->patb_flags & + SPAPR_PROC_TABLE_RADIX, + spapr->patb_flags & SPAPR_PROC_TABLE_GTSE, + spapr->patb_entry); + } return err; } @@ -1520,6 +1528,7 @@ static const VMStateDescription vmstate_spapr_patb_entry = { .needed = spapr_patb_entry_needed, .fields = (VMStateField[]) { VMSTATE_UINT64(patb_entry, sPAPRMachineState), + VMSTATE_UINT64(patb_flags, sPAPRMachineState), VMSTATE_END_OF_LIST() }, }; @@ -1869,6 +1878,12 @@ static SaveVMHandlers savevm_htab_handlers = { .load_state = htab_load, }; +void spapr_htab_savevm_register(sPAPRMachineState *spapr) +{ + register_savevm_live(NULL, "spapr/htab", -1, 1, + &savevm_htab_handlers, spapr); +} + static void spapr_boot_set(void *opaque, const char *boot_device, Error **errp) { @@ -2331,9 +2346,6 @@ static void ppc_spapr_init(MachineState *machine) * interface, this is a legacy from the sPAPREnvironment structure * which predated MachineState but had a similar function */ vmstate_register(NULL, 0, &vmstate_spapr, spapr); - register_savevm_live(NULL, "spapr/htab", -1, 1, - &savevm_htab_handlers, spapr); - /* used by RTAS */ QTAILQ_INIT(&spapr->ccs_list); qemu_register_reset(spapr_ccs_reset_hook, spapr); diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c index 3600b0e..fa03ca0 100644 --- a/hw/ppc/spapr_hcall.c +++ b/hw/ppc/spapr_hcall.c @@ -988,6 +988,7 @@ static target_ulong h_register_process_table(PowerPCCPU *cpu, spapr_check_setup_free_hpt(spapr, spapr->patb_entry, cproc); spapr->patb_entry = cproc; /* Save new process table */ + spapr->patb_flags = flags; /* Save the flags */ /* Update the UPRT and GTSE bits in the LPCR for all cpus */ CPU_FOREACH(cs) { @@ -1169,6 +1170,10 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu, } } + /* Register savevm handlers for HTAB case */ + if (!guest_radix || !kvmppc_has_cap_mmu_radix()) { + spapr_htab_savevm_register(spapr); + } return H_SUCCESS; } diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index a692e63..0d0e9e4 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -75,6 +75,7 @@ struct sPAPRMachineState { void *htab; uint32_t htab_shift; uint64_t patb_entry; /* Process tbl registed in H_REGISTER_PROCESS_TABLE */ + uint64_t patb_flags; hwaddr rma_size; int vrma_adjust; ssize_t rtas_size; @@ -691,4 +692,6 @@ void spapr_do_system_reset_on_cpu(CPUState *cs, run_on_cpu_data arg); #define SPAPR_PROC_TABLE_HPT_PT 0x02 #define SPAPR_PROC_TABLE_GTSE 0x01 +void spapr_htab_savevm_register(sPAPRMachineState *spapr); + #endif /* HW_SPAPR_H */ -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 2/2] spapr: Fix migration of Radix guests 2017-05-08 9:06 ` [Qemu-devel] [RFC PATCH 2/2] spapr: Fix migration of Radix guests Bharata B Rao @ 2017-05-11 1:02 ` David Gibson 2017-05-17 3:42 ` Bharata B Rao 0 siblings, 1 reply; 6+ messages in thread From: David Gibson @ 2017-05-11 1:02 UTC (permalink / raw) To: Bharata B Rao; +Cc: qemu-devel, qemu-ppc, sam.bobroff, rnsastry [-- Attachment #1: Type: text/plain, Size: 5737 bytes --] On Mon, May 08, 2017 at 02:36:17PM +0530, Bharata B Rao wrote: > Currently HTAB savevm handlers get registered by default and migration > of radix guest will fail. > > - Ensure that HTAB savevm handlers are not registered for radix case. > - Ensure that we issue KVM_PPC_CONFIGURE_V3_MMU for radix case post > migration. > > TODO: Right now I have delayed the HTAB savevm handler registration > to CAS call where we know if the guest is radix or hash. Another approach > is to let the HTAB handlers to be registered by default (as it is being > done currently, but unregister them from CAS when we discover radix > capability). Option 2 there sounds messy. I also suspect it could break if you try to migrate an (eventually) radix guest before it's done CAS. Strictly speaking only registering at CAS time will break old hash guests that don't do CAS at all. However such guests are really, really ancient, and I suspect we don't work with them already. You do, however, need to deregister (and allow the choice to be made again) on guest reset. On KVM we can only (for now) support either hash or radix guests. Under TCG, however, we could run a radix guest then reboot to a hash guest or vice versa. > > Reported-by: Nageswara R Sastry <rnsastry@linux.vnet.ibm.com> > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> > --- > hw/ppc/spapr.c | 18 +++++++++++++++--- > hw/ppc/spapr_hcall.c | 5 +++++ > include/hw/ppc/spapr.h | 3 +++ > 3 files changed, 23 insertions(+), 3 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index e2dc77c..e14f55c 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1436,6 +1436,14 @@ static int spapr_post_load(void *opaque, int version_id) > err = spapr_rtc_import_offset(&spapr->rtc, spapr->rtc_offset); > } > > + if (spapr->patb_entry && (spapr->patb_flags & SPAPR_PROC_TABLE_RADIX) && patb_entry already tells you whether the guest is radix or not (PATBE1_GR), you shouldn't need extra flags. > + kvmppc_has_cap_mmu_radix() && kvm_enabled()) { You should also fail the migration if you have an incoming radix guest, but the your new KVM host can't do radix. Or the reverse, for that matter. > + err = kvmppc_configure_v3_mmu(POWERPC_CPU(first_cpu), > + spapr->patb_flags & > + SPAPR_PROC_TABLE_RADIX, > + spapr->patb_flags & SPAPR_PROC_TABLE_GTSE, > + spapr->patb_entry); > + } > return err; > } > > @@ -1520,6 +1528,7 @@ static const VMStateDescription vmstate_spapr_patb_entry = { > .needed = spapr_patb_entry_needed, > .fields = (VMStateField[]) { > VMSTATE_UINT64(patb_entry, sPAPRMachineState), > + VMSTATE_UINT64(patb_flags, sPAPRMachineState), > VMSTATE_END_OF_LIST() > }, > }; > @@ -1869,6 +1878,12 @@ static SaveVMHandlers savevm_htab_handlers = { > .load_state = htab_load, > }; > > +void spapr_htab_savevm_register(sPAPRMachineState *spapr) > +{ > + register_savevm_live(NULL, "spapr/htab", -1, 1, > + &savevm_htab_handlers, spapr); > +} > + > static void spapr_boot_set(void *opaque, const char *boot_device, > Error **errp) > { > @@ -2331,9 +2346,6 @@ static void ppc_spapr_init(MachineState *machine) > * interface, this is a legacy from the sPAPREnvironment structure > * which predated MachineState but had a similar function */ > vmstate_register(NULL, 0, &vmstate_spapr, spapr); > - register_savevm_live(NULL, "spapr/htab", -1, 1, > - &savevm_htab_handlers, spapr); > - > /* used by RTAS */ > QTAILQ_INIT(&spapr->ccs_list); > qemu_register_reset(spapr_ccs_reset_hook, spapr); > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > index 3600b0e..fa03ca0 100644 > --- a/hw/ppc/spapr_hcall.c > +++ b/hw/ppc/spapr_hcall.c > @@ -988,6 +988,7 @@ static target_ulong h_register_process_table(PowerPCCPU *cpu, > spapr_check_setup_free_hpt(spapr, spapr->patb_entry, cproc); > > spapr->patb_entry = cproc; /* Save new process table */ > + spapr->patb_flags = flags; /* Save the flags */ > > /* Update the UPRT and GTSE bits in the LPCR for all cpus */ > CPU_FOREACH(cs) { > @@ -1169,6 +1170,10 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu, > } > } > > + /* Register savevm handlers for HTAB case */ > + if (!guest_radix || !kvmppc_has_cap_mmu_radix()) { > + spapr_htab_savevm_register(spapr); > + } > return H_SUCCESS; > } > > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index a692e63..0d0e9e4 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -75,6 +75,7 @@ struct sPAPRMachineState { > void *htab; > uint32_t htab_shift; > uint64_t patb_entry; /* Process tbl registed in H_REGISTER_PROCESS_TABLE */ > + uint64_t patb_flags; > hwaddr rma_size; > int vrma_adjust; > ssize_t rtas_size; > @@ -691,4 +692,6 @@ void spapr_do_system_reset_on_cpu(CPUState *cs, run_on_cpu_data arg); > #define SPAPR_PROC_TABLE_HPT_PT 0x02 > #define SPAPR_PROC_TABLE_GTSE 0x01 > > +void spapr_htab_savevm_register(sPAPRMachineState *spapr); > + > #endif /* HW_SPAPR_H */ -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 2/2] spapr: Fix migration of Radix guests 2017-05-11 1:02 ` David Gibson @ 2017-05-17 3:42 ` Bharata B Rao 0 siblings, 0 replies; 6+ messages in thread From: Bharata B Rao @ 2017-05-17 3:42 UTC (permalink / raw) To: David Gibson; +Cc: qemu-devel, qemu-ppc, sam.bobroff, rnsastry On Thu, May 11, 2017 at 11:02:20AM +1000, David Gibson wrote: > On Mon, May 08, 2017 at 02:36:17PM +0530, Bharata B Rao wrote: > > Currently HTAB savevm handlers get registered by default and migration > > of radix guest will fail. > > > > - Ensure that HTAB savevm handlers are not registered for radix case. > > - Ensure that we issue KVM_PPC_CONFIGURE_V3_MMU for radix case post > > migration. > > > > TODO: Right now I have delayed the HTAB savevm handler registration > > to CAS call where we know if the guest is radix or hash. Another approach > > is to let the HTAB handlers to be registered by default (as it is being > > done currently, but unregister them from CAS when we discover radix > > capability). > > Option 2 there sounds messy. I also suspect it could break if you try > to migrate an (eventually) radix guest before it's done CAS. > > Strictly speaking only registering at CAS time will break old hash > guests that don't do CAS at all. However such guests are really, > really ancient, and I suspect we don't work with them already. > > You do, however, need to deregister (and allow the choice to be made > again) on guest reset. On KVM we can only (for now) support either > hash or radix guests. Under TCG, however, we could run a radix guest > then reboot to a hash guest or vice versa. Took care of this in v1. > > > > > > Reported-by: Nageswara R Sastry <rnsastry@linux.vnet.ibm.com> > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> > > --- > > hw/ppc/spapr.c | 18 +++++++++++++++--- > > hw/ppc/spapr_hcall.c | 5 +++++ > > include/hw/ppc/spapr.h | 3 +++ > > 3 files changed, 23 insertions(+), 3 deletions(-) > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index e2dc77c..e14f55c 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -1436,6 +1436,14 @@ static int spapr_post_load(void *opaque, int version_id) > > err = spapr_rtc_import_offset(&spapr->rtc, spapr->rtc_offset); > > } > > > > + if (spapr->patb_entry && (spapr->patb_flags & SPAPR_PROC_TABLE_RADIX) && > > patb_entry already tells you whether the guest is radix or not > (PATBE1_GR), you shouldn't need extra flags. > > > + kvmppc_has_cap_mmu_radix() && kvm_enabled()) { > > You should also fail the migration if you have an incoming radix > guest, but the your new KVM host can't do radix. Or the reverse, for > that matter. I have checks in v1 to fail migration of radix guest to a host that doesn't support radix. But I don't see how we can detect and fail the migration of hash guests into hosts that don't support hash from here (i,e., from spapr_post_load). The hash guest's migration stream would have htab savevm entries and the target will fail as it knows not about htab savevm entries. Regards, Bharata. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-05-17 3:43 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-05-08 9:06 [Qemu-devel] [RFC PATCH 0/2] ppc/spapr: Fix migration of radix guests Bharata B Rao 2017-05-08 9:06 ` [Qemu-devel] [RFC PATCH 1/2] spapr: Make h_register_process_table hcall flags global Bharata B Rao 2017-05-11 0:14 ` David Gibson 2017-05-08 9:06 ` [Qemu-devel] [RFC PATCH 2/2] spapr: Fix migration of Radix guests Bharata B Rao 2017-05-11 1:02 ` David Gibson 2017-05-17 3:42 ` Bharata B Rao
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).