| Message ID | 1459352314-12552-1-git-send-email-clg@fr.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 30 Mar 2016 17:38:34 +0200 Cédric Le Goater <clg@fr.ibm.com> wrote: > This address is changed by the linux kernel using the H_SET_MODE hcall > and needs to be restored when migrating a spapr VM running in > TCG. This can be done using the AIL bits from the LPCR register. > > The patch introduces a helper routine cpu_ppc_get_excp_prefix() which > returns the effective address offset of the interrupt handler > depending on the LPCR_AIL bits. The same helper can be used in the > H_SET_MODE hcall, which lets us remove the H_SET_MODE_ADDR_TRANS_* > defines. > <clarification> ... because these H_SET_MODE_ADDR_TRANS_* defines, which refer to the values 0, 1, 2 and 3 for the mflag argument to the H_SET_MODE hypercall as described in section 14.5.4.3.8 of PAPR, are actually values for bits 39:40 (AIL) of the LPCR register, as described in section 2.2 of PowerISA. </clarification> > Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> > --- > Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > Changes since v1: > > - moved helper routine under target-ppc/ > - moved the restore of excp_prefix under cpu_post_load() > > hw/ppc/spapr_hcall.c | 13 ++----------- > include/hw/ppc/spapr.h | 5 ----- > target-ppc/cpu.h | 9 +++++++++ > target-ppc/machine.c | 20 +++++++++++++++++++- > target-ppc/translate_init.c | 14 ++++++++++++++ > 5 files changed, 44 insertions(+), 17 deletions(-) > > Index: qemu-dgibson-for-2.6.git/hw/ppc/spapr_hcall.c > =================================================================== > --- qemu-dgibson-for-2.6.git.orig/hw/ppc/spapr_hcall.c > +++ qemu-dgibson-for-2.6.git/hw/ppc/spapr_hcall.c > @@ -835,17 +835,8 @@ static target_ulong h_set_mode_resource_ > return H_P4; > } > > - switch (mflags) { > - case H_SET_MODE_ADDR_TRANS_NONE: > - prefix = 0; > - break; > - case H_SET_MODE_ADDR_TRANS_0001_8000: > - prefix = 0x18000; > - break; > - case H_SET_MODE_ADDR_TRANS_C000_0000_0000_4000: > - prefix = 0xC000000000004000ULL; > - break; > - default: > + prefix = cpu_ppc_get_excp_prefix(mflags); > + if (prefix == (target_ulong) -1ULL) { > return H_UNSUPPORTED_FLAG; > } > > Index: qemu-dgibson-for-2.6.git/target-ppc/machine.c > =================================================================== > --- qemu-dgibson-for-2.6.git.orig/target-ppc/machine.c > +++ qemu-dgibson-for-2.6.git/target-ppc/machine.c > @@ -156,12 +156,26 @@ static void cpu_pre_save(void *opaque) > } > } > > + > +static int cpu_post_load_excp_prefix(CPUPPCState *env) > +{ > + int ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT; > + target_ulong prefix = cpu_ppc_get_excp_prefix(ail); > + > + if (prefix == (target_ulong) -1ULL) { > + return -EINVAL; > + } > + env->excp_prefix = prefix; > + return 0; > +} > + > static int cpu_post_load(void *opaque, int version_id) > { > PowerPCCPU *cpu = opaque; > CPUPPCState *env = &cpu->env; > int i; > target_ulong msr; > + int ret = 0; > > /* > * We always ignore the source PVR. The user or management > @@ -201,7 +215,11 @@ static int cpu_post_load(void *opaque, i > > hreg_compute_mem_idx(env); > > - return 0; > + if (env->spr[SPR_LPCR] & LPCR_AIL) { > + ret = cpu_post_load_excp_prefix(env); > + } > + > + return ret; > } > > static bool fpu_needed(void *opaque) > Index: qemu-dgibson-for-2.6.git/target-ppc/translate_init.c > =================================================================== > --- qemu-dgibson-for-2.6.git.orig/target-ppc/translate_init.c > +++ qemu-dgibson-for-2.6.git/target-ppc/translate_init.c > @@ -8522,6 +8522,20 @@ void cpu_ppc_set_papr(PowerPCCPU *cpu) > } > } > > +target_ulong cpu_ppc_get_excp_prefix(target_ulong ail) > +{ > + switch (ail) { > + case AIL_NONE: > + return 0; > + case AIL_0001_8000: > + return 0x18000; > + case AIL_C000_0000_0000_4000: > + return 0xC000000000004000ULL; > + default: > + return (target_ulong) -1ULL; > + } > +} > + > #endif /* !defined(CONFIG_USER_ONLY) */ > > #endif /* defined (TARGET_PPC64) */ > Index: qemu-dgibson-for-2.6.git/target-ppc/cpu.h > =================================================================== > --- qemu-dgibson-for-2.6.git.orig/target-ppc/cpu.h > +++ qemu-dgibson-for-2.6.git/target-ppc/cpu.h > @@ -1269,6 +1269,7 @@ void store_booke_tsr (CPUPPCState *env, > void ppc_tlb_invalidate_all (CPUPPCState *env); > void ppc_tlb_invalidate_one (CPUPPCState *env, target_ulong addr); > void cpu_ppc_set_papr(PowerPCCPU *cpu); > +target_ulong cpu_ppc_get_excp_prefix(target_ulong ail); > #endif > #endif > > @@ -2277,6 +2278,14 @@ enum { > HMER_XSCOM_STATUS_LSH = (63 - 23), > }; > > +/* Alternate Interrupt Location (AIL) */ > +enum { > + AIL_NONE = 0, > + AIL_RESERVED = 1, > + AIL_0001_8000 = 2, > + AIL_C000_0000_0000_4000 = 3, > +}; > + > /*****************************************************************************/ > > static inline target_ulong cpu_read_xer(CPUPPCState *env) > Index: qemu-dgibson-for-2.6.git/include/hw/ppc/spapr.h > =================================================================== > --- qemu-dgibson-for-2.6.git.orig/include/hw/ppc/spapr.h > +++ qemu-dgibson-for-2.6.git/include/hw/ppc/spapr.h > @@ -204,11 +204,6 @@ struct sPAPRMachineState { > #define H_SET_MODE_ENDIAN_BIG 0 > #define H_SET_MODE_ENDIAN_LITTLE 1 > > -/* Flags for H_SET_MODE_RESOURCE_ADDR_TRANS_MODE */ > -#define H_SET_MODE_ADDR_TRANS_NONE 0 > -#define H_SET_MODE_ADDR_TRANS_0001_8000 2 > -#define H_SET_MODE_ADDR_TRANS_C000_0000_0000_4000 3 > - > /* VASI States */ > #define H_VASI_INVALID 0 > #define H_VASI_ENABLED 1 > >
On Wed, 30 Mar 2016 17:38:34 +0200 Cédric Le Goater <clg@fr.ibm.com> wrote: > This address is changed by the linux kernel using the H_SET_MODE hcall > and needs to be restored when migrating a spapr VM running in > TCG. This can be done using the AIL bits from the LPCR register. > > The patch introduces a helper routine cpu_ppc_get_excp_prefix() which > returns the effective address offset of the interrupt handler > depending on the LPCR_AIL bits. The same helper can be used in the > H_SET_MODE hcall, which lets us remove the H_SET_MODE_ADDR_TRANS_* > defines. > > Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> > --- > Sorry I hit the send button too quickly... Just a nit but my Reviewed-by stands. > Changes since v1: > > - moved helper routine under target-ppc/ > - moved the restore of excp_prefix under cpu_post_load() > > hw/ppc/spapr_hcall.c | 13 ++----------- > include/hw/ppc/spapr.h | 5 ----- > target-ppc/cpu.h | 9 +++++++++ > target-ppc/machine.c | 20 +++++++++++++++++++- > target-ppc/translate_init.c | 14 ++++++++++++++ > 5 files changed, 44 insertions(+), 17 deletions(-) > > Index: qemu-dgibson-for-2.6.git/hw/ppc/spapr_hcall.c > =================================================================== > --- qemu-dgibson-for-2.6.git.orig/hw/ppc/spapr_hcall.c > +++ qemu-dgibson-for-2.6.git/hw/ppc/spapr_hcall.c > @@ -835,17 +835,8 @@ static target_ulong h_set_mode_resource_ > return H_P4; > } > > - switch (mflags) { > - case H_SET_MODE_ADDR_TRANS_NONE: > - prefix = 0; > - break; > - case H_SET_MODE_ADDR_TRANS_0001_8000: > - prefix = 0x18000; > - break; > - case H_SET_MODE_ADDR_TRANS_C000_0000_0000_4000: > - prefix = 0xC000000000004000ULL; > - break; > - default: > + prefix = cpu_ppc_get_excp_prefix(mflags); > + if (prefix == (target_ulong) -1ULL) { + if (prefix == (target_ulong) (-1ULL)) { to make ./scripts/checkpatch.pl happy :) > return H_UNSUPPORTED_FLAG; > } > > Index: qemu-dgibson-for-2.6.git/target-ppc/machine.c > =================================================================== > --- qemu-dgibson-for-2.6.git.orig/target-ppc/machine.c > +++ qemu-dgibson-for-2.6.git/target-ppc/machine.c > @@ -156,12 +156,26 @@ static void cpu_pre_save(void *opaque) > } > } > > + > +static int cpu_post_load_excp_prefix(CPUPPCState *env) > +{ > + int ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT; > + target_ulong prefix = cpu_ppc_get_excp_prefix(ail); > + > + if (prefix == (target_ulong) -1ULL) { > + return -EINVAL; > + } > + env->excp_prefix = prefix; > + return 0; > +} > + > static int cpu_post_load(void *opaque, int version_id) > { > PowerPCCPU *cpu = opaque; > CPUPPCState *env = &cpu->env; > int i; > target_ulong msr; > + int ret = 0; > > /* > * We always ignore the source PVR. The user or management > @@ -201,7 +215,11 @@ static int cpu_post_load(void *opaque, i > > hreg_compute_mem_idx(env); > > - return 0; > + if (env->spr[SPR_LPCR] & LPCR_AIL) { > + ret = cpu_post_load_excp_prefix(env); > + } > + > + return ret; > } > > static bool fpu_needed(void *opaque) > Index: qemu-dgibson-for-2.6.git/target-ppc/translate_init.c > =================================================================== > --- qemu-dgibson-for-2.6.git.orig/target-ppc/translate_init.c > +++ qemu-dgibson-for-2.6.git/target-ppc/translate_init.c > @@ -8522,6 +8522,20 @@ void cpu_ppc_set_papr(PowerPCCPU *cpu) > } > } > > +target_ulong cpu_ppc_get_excp_prefix(target_ulong ail) > +{ > + switch (ail) { > + case AIL_NONE: > + return 0; > + case AIL_0001_8000: > + return 0x18000; > + case AIL_C000_0000_0000_4000: > + return 0xC000000000004000ULL; > + default: > + return (target_ulong) -1ULL; > + } > +} > + > #endif /* !defined(CONFIG_USER_ONLY) */ > > #endif /* defined (TARGET_PPC64) */ > Index: qemu-dgibson-for-2.6.git/target-ppc/cpu.h > =================================================================== > --- qemu-dgibson-for-2.6.git.orig/target-ppc/cpu.h > +++ qemu-dgibson-for-2.6.git/target-ppc/cpu.h > @@ -1269,6 +1269,7 @@ void store_booke_tsr (CPUPPCState *env, > void ppc_tlb_invalidate_all (CPUPPCState *env); > void ppc_tlb_invalidate_one (CPUPPCState *env, target_ulong addr); > void cpu_ppc_set_papr(PowerPCCPU *cpu); > +target_ulong cpu_ppc_get_excp_prefix(target_ulong ail); > #endif > #endif > > @@ -2277,6 +2278,14 @@ enum { > HMER_XSCOM_STATUS_LSH = (63 - 23), > }; > > +/* Alternate Interrupt Location (AIL) */ > +enum { > + AIL_NONE = 0, > + AIL_RESERVED = 1, > + AIL_0001_8000 = 2, > + AIL_C000_0000_0000_4000 = 3, > +}; > + > /*****************************************************************************/ > > static inline target_ulong cpu_read_xer(CPUPPCState *env) > Index: qemu-dgibson-for-2.6.git/include/hw/ppc/spapr.h > =================================================================== > --- qemu-dgibson-for-2.6.git.orig/include/hw/ppc/spapr.h > +++ qemu-dgibson-for-2.6.git/include/hw/ppc/spapr.h > @@ -204,11 +204,6 @@ struct sPAPRMachineState { > #define H_SET_MODE_ENDIAN_BIG 0 > #define H_SET_MODE_ENDIAN_LITTLE 1 > > -/* Flags for H_SET_MODE_RESOURCE_ADDR_TRANS_MODE */ > -#define H_SET_MODE_ADDR_TRANS_NONE 0 > -#define H_SET_MODE_ADDR_TRANS_0001_8000 2 > -#define H_SET_MODE_ADDR_TRANS_C000_0000_0000_4000 3 > - > /* VASI States */ > #define H_VASI_INVALID 0 > #define H_VASI_ENABLED 1
On Wed, 30 Mar 2016 19:01:31 +0200 Greg Kurz <gkurz@linux.vnet.ibm.com> wrote: > On Wed, 30 Mar 2016 17:38:34 +0200 > Cédric Le Goater <clg@fr.ibm.com> wrote: > > > This address is changed by the linux kernel using the H_SET_MODE hcall > > and needs to be restored when migrating a spapr VM running in > > TCG. This can be done using the AIL bits from the LPCR register. > > > > The patch introduces a helper routine cpu_ppc_get_excp_prefix() which > > returns the effective address offset of the interrupt handler > > depending on the LPCR_AIL bits. The same helper can be used in the > > H_SET_MODE hcall, which lets us remove the H_SET_MODE_ADDR_TRANS_* > > defines. > > > > <clarification> > ... because these H_SET_MODE_ADDR_TRANS_* defines, which refer to the values > 0, 1, 2 and 3 for the mflag argument to the H_SET_MODE hypercall as described > in section 14.5.4.3.8 of PAPR, are actually values for bits 39:40 (AIL) of the > LPCR register, as described in section 2.2 of PowerISA. read "section section 2.2 of chapter 2 of book-III S in PowerISA" :) > </clarification> > > > Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> > > --- > > > > Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > > > Changes since v1: > > > > - moved helper routine under target-ppc/ > > - moved the restore of excp_prefix under cpu_post_load() > > > > hw/ppc/spapr_hcall.c | 13 ++----------- > > include/hw/ppc/spapr.h | 5 ----- > > target-ppc/cpu.h | 9 +++++++++ > > target-ppc/machine.c | 20 +++++++++++++++++++- > > target-ppc/translate_init.c | 14 ++++++++++++++ > > 5 files changed, 44 insertions(+), 17 deletions(-) > > > > Index: qemu-dgibson-for-2.6.git/hw/ppc/spapr_hcall.c > > =================================================================== > > --- qemu-dgibson-for-2.6.git.orig/hw/ppc/spapr_hcall.c > > +++ qemu-dgibson-for-2.6.git/hw/ppc/spapr_hcall.c > > @@ -835,17 +835,8 @@ static target_ulong h_set_mode_resource_ > > return H_P4; > > } > > > > - switch (mflags) { > > - case H_SET_MODE_ADDR_TRANS_NONE: > > - prefix = 0; > > - break; > > - case H_SET_MODE_ADDR_TRANS_0001_8000: > > - prefix = 0x18000; > > - break; > > - case H_SET_MODE_ADDR_TRANS_C000_0000_0000_4000: > > - prefix = 0xC000000000004000ULL; > > - break; > > - default: > > + prefix = cpu_ppc_get_excp_prefix(mflags); > > + if (prefix == (target_ulong) -1ULL) { > > return H_UNSUPPORTED_FLAG; > > } > > > > Index: qemu-dgibson-for-2.6.git/target-ppc/machine.c > > =================================================================== > > --- qemu-dgibson-for-2.6.git.orig/target-ppc/machine.c > > +++ qemu-dgibson-for-2.6.git/target-ppc/machine.c > > @@ -156,12 +156,26 @@ static void cpu_pre_save(void *opaque) > > } > > } > > > > + > > +static int cpu_post_load_excp_prefix(CPUPPCState *env) > > +{ > > + int ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT; > > + target_ulong prefix = cpu_ppc_get_excp_prefix(ail); > > + > > + if (prefix == (target_ulong) -1ULL) { > > + return -EINVAL; > > + } > > + env->excp_prefix = prefix; > > + return 0; > > +} > > + > > static int cpu_post_load(void *opaque, int version_id) > > { > > PowerPCCPU *cpu = opaque; > > CPUPPCState *env = &cpu->env; > > int i; > > target_ulong msr; > > + int ret = 0; > > > > /* > > * We always ignore the source PVR. The user or management > > @@ -201,7 +215,11 @@ static int cpu_post_load(void *opaque, i > > > > hreg_compute_mem_idx(env); > > > > - return 0; > > + if (env->spr[SPR_LPCR] & LPCR_AIL) { > > + ret = cpu_post_load_excp_prefix(env); > > + } > > + > > + return ret; > > } > > > > static bool fpu_needed(void *opaque) > > Index: qemu-dgibson-for-2.6.git/target-ppc/translate_init.c > > =================================================================== > > --- qemu-dgibson-for-2.6.git.orig/target-ppc/translate_init.c > > +++ qemu-dgibson-for-2.6.git/target-ppc/translate_init.c > > @@ -8522,6 +8522,20 @@ void cpu_ppc_set_papr(PowerPCCPU *cpu) > > } > > } > > > > +target_ulong cpu_ppc_get_excp_prefix(target_ulong ail) > > +{ > > + switch (ail) { > > + case AIL_NONE: > > + return 0; > > + case AIL_0001_8000: > > + return 0x18000; > > + case AIL_C000_0000_0000_4000: > > + return 0xC000000000004000ULL; > > + default: > > + return (target_ulong) -1ULL; > > + } > > +} > > + > > #endif /* !defined(CONFIG_USER_ONLY) */ > > > > #endif /* defined (TARGET_PPC64) */ > > Index: qemu-dgibson-for-2.6.git/target-ppc/cpu.h > > =================================================================== > > --- qemu-dgibson-for-2.6.git.orig/target-ppc/cpu.h > > +++ qemu-dgibson-for-2.6.git/target-ppc/cpu.h > > @@ -1269,6 +1269,7 @@ void store_booke_tsr (CPUPPCState *env, > > void ppc_tlb_invalidate_all (CPUPPCState *env); > > void ppc_tlb_invalidate_one (CPUPPCState *env, target_ulong addr); > > void cpu_ppc_set_papr(PowerPCCPU *cpu); > > +target_ulong cpu_ppc_get_excp_prefix(target_ulong ail); > > #endif > > #endif > > > > @@ -2277,6 +2278,14 @@ enum { > > HMER_XSCOM_STATUS_LSH = (63 - 23), > > }; > > > > +/* Alternate Interrupt Location (AIL) */ > > +enum { > > + AIL_NONE = 0, > > + AIL_RESERVED = 1, > > + AIL_0001_8000 = 2, > > + AIL_C000_0000_0000_4000 = 3, > > +}; > > + > > /*****************************************************************************/ > > > > static inline target_ulong cpu_read_xer(CPUPPCState *env) > > Index: qemu-dgibson-for-2.6.git/include/hw/ppc/spapr.h > > =================================================================== > > --- qemu-dgibson-for-2.6.git.orig/include/hw/ppc/spapr.h > > +++ qemu-dgibson-for-2.6.git/include/hw/ppc/spapr.h > > @@ -204,11 +204,6 @@ struct sPAPRMachineState { > > #define H_SET_MODE_ENDIAN_BIG 0 > > #define H_SET_MODE_ENDIAN_LITTLE 1 > > > > -/* Flags for H_SET_MODE_RESOURCE_ADDR_TRANS_MODE */ > > -#define H_SET_MODE_ADDR_TRANS_NONE 0 > > -#define H_SET_MODE_ADDR_TRANS_0001_8000 2 > > -#define H_SET_MODE_ADDR_TRANS_C000_0000_0000_4000 3 > > - > > /* VASI States */ > > #define H_VASI_INVALID 0 > > #define H_VASI_ENABLED 1 > > > > > >
On Wed, Mar 30, 2016 at 05:38:34PM +0200, Cédric Le Goater wrote: > This address is changed by the linux kernel using the H_SET_MODE hcall > and needs to be restored when migrating a spapr VM running in > TCG. This can be done using the AIL bits from the LPCR register. > > The patch introduces a helper routine cpu_ppc_get_excp_prefix() which > returns the effective address offset of the interrupt handler > depending on the LPCR_AIL bits. The same helper can be used in the > H_SET_MODE hcall, which lets us remove the H_SET_MODE_ADDR_TRANS_* > defines. > > Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> I've applied this (with Greg's minor amendments) to ppc-for-2.6), since it certainly improves behaviour, although I have a couple of queries: > --- > > Changes since v1: > > - moved helper routine under target-ppc/ > - moved the restore of excp_prefix under cpu_post_load() > > hw/ppc/spapr_hcall.c | 13 ++----------- > include/hw/ppc/spapr.h | 5 ----- > target-ppc/cpu.h | 9 +++++++++ > target-ppc/machine.c | 20 +++++++++++++++++++- > target-ppc/translate_init.c | 14 ++++++++++++++ > 5 files changed, 44 insertions(+), 17 deletions(-) > > Index: qemu-dgibson-for-2.6.git/hw/ppc/spapr_hcall.c > =================================================================== > --- qemu-dgibson-for-2.6.git.orig/hw/ppc/spapr_hcall.c > +++ qemu-dgibson-for-2.6.git/hw/ppc/spapr_hcall.c > @@ -835,17 +835,8 @@ static target_ulong h_set_mode_resource_ > return H_P4; > } > > - switch (mflags) { > - case H_SET_MODE_ADDR_TRANS_NONE: > - prefix = 0; > - break; > - case H_SET_MODE_ADDR_TRANS_0001_8000: > - prefix = 0x18000; > - break; > - case H_SET_MODE_ADDR_TRANS_C000_0000_0000_4000: > - prefix = 0xC000000000004000ULL; > - break; > - default: > + prefix = cpu_ppc_get_excp_prefix(mflags); > + if (prefix == (target_ulong) -1ULL) { > return H_UNSUPPORTED_FLAG; > } > > Index: qemu-dgibson-for-2.6.git/target-ppc/machine.c > =================================================================== > --- qemu-dgibson-for-2.6.git.orig/target-ppc/machine.c > +++ qemu-dgibson-for-2.6.git/target-ppc/machine.c > @@ -156,12 +156,26 @@ static void cpu_pre_save(void *opaque) > } > } > > + > +static int cpu_post_load_excp_prefix(CPUPPCState *env) > +{ > + int ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT; > + target_ulong prefix = cpu_ppc_get_excp_prefix(ail); > + > + if (prefix == (target_ulong) -1ULL) { > + return -EINVAL; > + } > + env->excp_prefix = prefix; > + return 0; > +} > + > static int cpu_post_load(void *opaque, int version_id) > { > PowerPCCPU *cpu = opaque; > CPUPPCState *env = &cpu->env; > int i; > target_ulong msr; > + int ret = 0; > > /* > * We always ignore the source PVR. The user or management > @@ -201,7 +215,11 @@ static int cpu_post_load(void *opaque, i > > hreg_compute_mem_idx(env); > > - return 0; > + if (env->spr[SPR_LPCR] & LPCR_AIL) { > + ret = cpu_post_load_excp_prefix(env); > + } Why not call this unconditionally? If AIL == 0 it will still do the right thing. Aren't there also circumstances where the exception prefix can depend on the MSR? Do those need to be handled somewhere? > + > + return ret; > } > > static bool fpu_needed(void *opaque) > Index: qemu-dgibson-for-2.6.git/target-ppc/translate_init.c > =================================================================== > --- qemu-dgibson-for-2.6.git.orig/target-ppc/translate_init.c > +++ qemu-dgibson-for-2.6.git/target-ppc/translate_init.c > @@ -8522,6 +8522,20 @@ void cpu_ppc_set_papr(PowerPCCPU *cpu) > } > } > > +target_ulong cpu_ppc_get_excp_prefix(target_ulong ail) > +{ > + switch (ail) { > + case AIL_NONE: > + return 0; > + case AIL_0001_8000: > + return 0x18000; > + case AIL_C000_0000_0000_4000: > + return 0xC000000000004000ULL; > + default: > + return (target_ulong) -1ULL; > + } > +} > + > #endif /* !defined(CONFIG_USER_ONLY) */ > > #endif /* defined (TARGET_PPC64) */ > Index: qemu-dgibson-for-2.6.git/target-ppc/cpu.h > =================================================================== > --- qemu-dgibson-for-2.6.git.orig/target-ppc/cpu.h > +++ qemu-dgibson-for-2.6.git/target-ppc/cpu.h > @@ -1269,6 +1269,7 @@ void store_booke_tsr (CPUPPCState *env, > void ppc_tlb_invalidate_all (CPUPPCState *env); > void ppc_tlb_invalidate_one (CPUPPCState *env, target_ulong addr); > void cpu_ppc_set_papr(PowerPCCPU *cpu); > +target_ulong cpu_ppc_get_excp_prefix(target_ulong ail); > #endif > #endif > > @@ -2277,6 +2278,14 @@ enum { > HMER_XSCOM_STATUS_LSH = (63 - 23), > }; > > +/* Alternate Interrupt Location (AIL) */ > +enum { > + AIL_NONE = 0, > + AIL_RESERVED = 1, > + AIL_0001_8000 = 2, > + AIL_C000_0000_0000_4000 = 3, > +}; > + > /*****************************************************************************/ > > static inline target_ulong cpu_read_xer(CPUPPCState *env) > Index: qemu-dgibson-for-2.6.git/include/hw/ppc/spapr.h > =================================================================== > --- qemu-dgibson-for-2.6.git.orig/include/hw/ppc/spapr.h > +++ qemu-dgibson-for-2.6.git/include/hw/ppc/spapr.h > @@ -204,11 +204,6 @@ struct sPAPRMachineState { > #define H_SET_MODE_ENDIAN_BIG 0 > #define H_SET_MODE_ENDIAN_LITTLE 1 > > -/* Flags for H_SET_MODE_RESOURCE_ADDR_TRANS_MODE */ > -#define H_SET_MODE_ADDR_TRANS_NONE 0 > -#define H_SET_MODE_ADDR_TRANS_0001_8000 2 > -#define H_SET_MODE_ADDR_TRANS_C000_0000_0000_4000 3 > - > /* VASI States */ > #define H_VASI_INVALID 0 > #define H_VASI_ENABLED 1 >
On 31/03/16 05:55, David Gibson wrote: > On Wed, Mar 30, 2016 at 05:38:34PM +0200, Cédric Le Goater wrote: >> This address is changed by the linux kernel using the H_SET_MODE hcall >> and needs to be restored when migrating a spapr VM running in >> TCG. This can be done using the AIL bits from the LPCR register. >> >> The patch introduces a helper routine cpu_ppc_get_excp_prefix() which >> returns the effective address offset of the interrupt handler >> depending on the LPCR_AIL bits. The same helper can be used in the >> H_SET_MODE hcall, which lets us remove the H_SET_MODE_ADDR_TRANS_* >> defines. >> >> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> > > I've applied this (with Greg's minor amendments) to ppc-for-2.6), > since it certainly improves behaviour, although I have a couple of > queries: > >> --- >> >> Changes since v1: >> >> - moved helper routine under target-ppc/ >> - moved the restore of excp_prefix under cpu_post_load() >> >> hw/ppc/spapr_hcall.c | 13 ++----------- >> include/hw/ppc/spapr.h | 5 ----- >> target-ppc/cpu.h | 9 +++++++++ >> target-ppc/machine.c | 20 +++++++++++++++++++- >> target-ppc/translate_init.c | 14 ++++++++++++++ >> 5 files changed, 44 insertions(+), 17 deletions(-) >> >> Index: qemu-dgibson-for-2.6.git/hw/ppc/spapr_hcall.c >> =================================================================== >> --- qemu-dgibson-for-2.6.git.orig/hw/ppc/spapr_hcall.c >> +++ qemu-dgibson-for-2.6.git/hw/ppc/spapr_hcall.c >> @@ -835,17 +835,8 @@ static target_ulong h_set_mode_resource_ >> return H_P4; >> } >> >> - switch (mflags) { >> - case H_SET_MODE_ADDR_TRANS_NONE: >> - prefix = 0; >> - break; >> - case H_SET_MODE_ADDR_TRANS_0001_8000: >> - prefix = 0x18000; >> - break; >> - case H_SET_MODE_ADDR_TRANS_C000_0000_0000_4000: >> - prefix = 0xC000000000004000ULL; >> - break; >> - default: >> + prefix = cpu_ppc_get_excp_prefix(mflags); >> + if (prefix == (target_ulong) -1ULL) { >> return H_UNSUPPORTED_FLAG; >> } >> >> Index: qemu-dgibson-for-2.6.git/target-ppc/machine.c >> =================================================================== >> --- qemu-dgibson-for-2.6.git.orig/target-ppc/machine.c >> +++ qemu-dgibson-for-2.6.git/target-ppc/machine.c >> @@ -156,12 +156,26 @@ static void cpu_pre_save(void *opaque) >> } >> } >> >> + >> +static int cpu_post_load_excp_prefix(CPUPPCState *env) >> +{ >> + int ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT; >> + target_ulong prefix = cpu_ppc_get_excp_prefix(ail); >> + >> + if (prefix == (target_ulong) -1ULL) { >> + return -EINVAL; >> + } >> + env->excp_prefix = prefix; >> + return 0; >> +} >> + >> static int cpu_post_load(void *opaque, int version_id) >> { >> PowerPCCPU *cpu = opaque; >> CPUPPCState *env = &cpu->env; >> int i; >> target_ulong msr; >> + int ret = 0; >> >> /* >> * We always ignore the source PVR. The user or management >> @@ -201,7 +215,11 @@ static int cpu_post_load(void *opaque, i >> >> hreg_compute_mem_idx(env); >> >> - return 0; >> + if (env->spr[SPR_LPCR] & LPCR_AIL) { >> + ret = cpu_post_load_excp_prefix(env); >> + } > > Why not call this unconditionally? If AIL == 0 it will still do the > right thing. > > Aren't there also circumstances where the exception prefix can depend > on the MSR? Do those need to be handled somewhere? Yes indeed - this was part of my patchset last year to fix up various migration issues for the Mac PPC machines (see commit 2360b6e84f78d41fa0f76555a947148b73645259). I agree that having the env->excp_prefix logic split like this isn't a particularly great idea. Let's just have a single helper function as in the patch above and use that in both places (and in fact with recent changes I have a feeling env->excp_prefix is one of the few remaining reasons that the above change is still required, but please check this). ATB, Mark.
On 03/31/2016 06:55 AM, David Gibson wrote: > On Wed, Mar 30, 2016 at 05:38:34PM +0200, Cédric Le Goater wrote: >> This address is changed by the linux kernel using the H_SET_MODE hcall >> and needs to be restored when migrating a spapr VM running in >> TCG. This can be done using the AIL bits from the LPCR register. >> >> The patch introduces a helper routine cpu_ppc_get_excp_prefix() which >> returns the effective address offset of the interrupt handler >> depending on the LPCR_AIL bits. The same helper can be used in the >> H_SET_MODE hcall, which lets us remove the H_SET_MODE_ADDR_TRANS_* >> defines. >> >> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> > > I've applied this (with Greg's minor amendments) to ppc-for-2.6), > since it certainly improves behaviour, although I have a couple of > queries: > >> --- >> >> Changes since v1: >> >> - moved helper routine under target-ppc/ >> - moved the restore of excp_prefix under cpu_post_load() >> >> hw/ppc/spapr_hcall.c | 13 ++----------- >> include/hw/ppc/spapr.h | 5 ----- >> target-ppc/cpu.h | 9 +++++++++ >> target-ppc/machine.c | 20 +++++++++++++++++++- >> target-ppc/translate_init.c | 14 ++++++++++++++ >> 5 files changed, 44 insertions(+), 17 deletions(-) >> >> Index: qemu-dgibson-for-2.6.git/hw/ppc/spapr_hcall.c >> =================================================================== >> --- qemu-dgibson-for-2.6.git.orig/hw/ppc/spapr_hcall.c >> +++ qemu-dgibson-for-2.6.git/hw/ppc/spapr_hcall.c >> @@ -835,17 +835,8 @@ static target_ulong h_set_mode_resource_ >> return H_P4; >> } >> >> - switch (mflags) { >> - case H_SET_MODE_ADDR_TRANS_NONE: >> - prefix = 0; >> - break; >> - case H_SET_MODE_ADDR_TRANS_0001_8000: >> - prefix = 0x18000; >> - break; >> - case H_SET_MODE_ADDR_TRANS_C000_0000_0000_4000: >> - prefix = 0xC000000000004000ULL; >> - break; >> - default: >> + prefix = cpu_ppc_get_excp_prefix(mflags); >> + if (prefix == (target_ulong) -1ULL) { >> return H_UNSUPPORTED_FLAG; >> } >> >> Index: qemu-dgibson-for-2.6.git/target-ppc/machine.c >> =================================================================== >> --- qemu-dgibson-for-2.6.git.orig/target-ppc/machine.c >> +++ qemu-dgibson-for-2.6.git/target-ppc/machine.c >> @@ -156,12 +156,26 @@ static void cpu_pre_save(void *opaque) >> } >> } >> >> + >> +static int cpu_post_load_excp_prefix(CPUPPCState *env) >> +{ >> + int ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT; >> + target_ulong prefix = cpu_ppc_get_excp_prefix(ail); >> + >> + if (prefix == (target_ulong) -1ULL) { >> + return -EINVAL; >> + } >> + env->excp_prefix = prefix; >> + return 0; >> +} >> + >> static int cpu_post_load(void *opaque, int version_id) >> { >> PowerPCCPU *cpu = opaque; >> CPUPPCState *env = &cpu->env; >> int i; >> target_ulong msr; >> + int ret = 0; >> >> /* >> * We always ignore the source PVR. The user or management >> @@ -201,7 +215,11 @@ static int cpu_post_load(void *opaque, i >> >> hreg_compute_mem_idx(env); >> >> - return 0; >> + if (env->spr[SPR_LPCR] & LPCR_AIL) { >> + ret = cpu_post_load_excp_prefix(env); >> + } > > Why not call this unconditionally? If AIL == 0 it will still do the > right thing. yes, indeed. the test is not required. > Aren't there also circumstances where the exception prefix can depend > on the MSR? Do those need to be handled somewhere? Well, if translation is not enabled, LPCR_AIL should be 0, and so should be excp_prefix. Is that something we should check/enforce at restart ? Looking closer, we could probably get rid of excp_prefix (for spapr) and compute the value directly from LPCR in powerpc_excp() where we really need it. C. >> + >> + return ret; >> } >> >> static bool fpu_needed(void *opaque) >> Index: qemu-dgibson-for-2.6.git/target-ppc/translate_init.c >> =================================================================== >> --- qemu-dgibson-for-2.6.git.orig/target-ppc/translate_init.c >> +++ qemu-dgibson-for-2.6.git/target-ppc/translate_init.c >> @@ -8522,6 +8522,20 @@ void cpu_ppc_set_papr(PowerPCCPU *cpu) >> } >> } >> >> +target_ulong cpu_ppc_get_excp_prefix(target_ulong ail) >> +{ >> + switch (ail) { >> + case AIL_NONE: >> + return 0; >> + case AIL_0001_8000: >> + return 0x18000; >> + case AIL_C000_0000_0000_4000: >> + return 0xC000000000004000ULL; >> + default: >> + return (target_ulong) -1ULL; >> + } >> +} >> + >> #endif /* !defined(CONFIG_USER_ONLY) */ >> >> #endif /* defined (TARGET_PPC64) */ >> Index: qemu-dgibson-for-2.6.git/target-ppc/cpu.h >> =================================================================== >> --- qemu-dgibson-for-2.6.git.orig/target-ppc/cpu.h >> +++ qemu-dgibson-for-2.6.git/target-ppc/cpu.h >> @@ -1269,6 +1269,7 @@ void store_booke_tsr (CPUPPCState *env, >> void ppc_tlb_invalidate_all (CPUPPCState *env); >> void ppc_tlb_invalidate_one (CPUPPCState *env, target_ulong addr); >> void cpu_ppc_set_papr(PowerPCCPU *cpu); >> +target_ulong cpu_ppc_get_excp_prefix(target_ulong ail); >> #endif >> #endif >> >> @@ -2277,6 +2278,14 @@ enum { >> HMER_XSCOM_STATUS_LSH = (63 - 23), >> }; >> >> +/* Alternate Interrupt Location (AIL) */ >> +enum { >> + AIL_NONE = 0, >> + AIL_RESERVED = 1, >> + AIL_0001_8000 = 2, >> + AIL_C000_0000_0000_4000 = 3, >> +}; >> + >> /*****************************************************************************/ >> >> static inline target_ulong cpu_read_xer(CPUPPCState *env) >> Index: qemu-dgibson-for-2.6.git/include/hw/ppc/spapr.h >> =================================================================== >> --- qemu-dgibson-for-2.6.git.orig/include/hw/ppc/spapr.h >> +++ qemu-dgibson-for-2.6.git/include/hw/ppc/spapr.h >> @@ -204,11 +204,6 @@ struct sPAPRMachineState { >> #define H_SET_MODE_ENDIAN_BIG 0 >> #define H_SET_MODE_ENDIAN_LITTLE 1 >> >> -/* Flags for H_SET_MODE_RESOURCE_ADDR_TRANS_MODE */ >> -#define H_SET_MODE_ADDR_TRANS_NONE 0 >> -#define H_SET_MODE_ADDR_TRANS_0001_8000 2 >> -#define H_SET_MODE_ADDR_TRANS_C000_0000_0000_4000 3 >> - >> /* VASI States */ >> #define H_VASI_INVALID 0 >> #define H_VASI_ENABLED 1 >> >
On 03/30/2016 07:12 PM, Greg Kurz wrote: > On Wed, 30 Mar 2016 17:38:34 +0200 > Cédric Le Goater <clg@fr.ibm.com> wrote: > >> This address is changed by the linux kernel using the H_SET_MODE hcall >> and needs to be restored when migrating a spapr VM running in >> TCG. This can be done using the AIL bits from the LPCR register. >> >> The patch introduces a helper routine cpu_ppc_get_excp_prefix() which >> returns the effective address offset of the interrupt handler >> depending on the LPCR_AIL bits. The same helper can be used in the >> H_SET_MODE hcall, which lets us remove the H_SET_MODE_ADDR_TRANS_* >> defines. >> >> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> >> --- >> > > Sorry I hit the send button too quickly... Just a nit but my Reviewed-by stands. > >> Changes since v1: >> >> - moved helper routine under target-ppc/ >> - moved the restore of excp_prefix under cpu_post_load() >> >> hw/ppc/spapr_hcall.c | 13 ++----------- >> include/hw/ppc/spapr.h | 5 ----- >> target-ppc/cpu.h | 9 +++++++++ >> target-ppc/machine.c | 20 +++++++++++++++++++- >> target-ppc/translate_init.c | 14 ++++++++++++++ >> 5 files changed, 44 insertions(+), 17 deletions(-) >> >> Index: qemu-dgibson-for-2.6.git/hw/ppc/spapr_hcall.c >> =================================================================== >> --- qemu-dgibson-for-2.6.git.orig/hw/ppc/spapr_hcall.c >> +++ qemu-dgibson-for-2.6.git/hw/ppc/spapr_hcall.c >> @@ -835,17 +835,8 @@ static target_ulong h_set_mode_resource_ >> return H_P4; >> } >> >> - switch (mflags) { >> - case H_SET_MODE_ADDR_TRANS_NONE: >> - prefix = 0; >> - break; >> - case H_SET_MODE_ADDR_TRANS_0001_8000: >> - prefix = 0x18000; >> - break; >> - case H_SET_MODE_ADDR_TRANS_C000_0000_0000_4000: >> - prefix = 0xC000000000004000ULL; >> - break; >> - default: >> + prefix = cpu_ppc_get_excp_prefix(mflags); >> + if (prefix == (target_ulong) -1ULL) { > > + if (prefix == (target_ulong) (-1ULL)) { > > to make ./scripts/checkpatch.pl happy :) yes. The funny thing is that checkpatch.pl does not complain for the exact same line below. Looks like a checkpatch.pl bug to me. C. > >> return H_UNSUPPORTED_FLAG; >> } >> >> Index: qemu-dgibson-for-2.6.git/target-ppc/machine.c >> =================================================================== >> --- qemu-dgibson-for-2.6.git.orig/target-ppc/machine.c >> +++ qemu-dgibson-for-2.6.git/target-ppc/machine.c >> @@ -156,12 +156,26 @@ static void cpu_pre_save(void *opaque) >> } >> } >> >> + >> +static int cpu_post_load_excp_prefix(CPUPPCState *env) >> +{ >> + int ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT; >> + target_ulong prefix = cpu_ppc_get_excp_prefix(ail); >> + >> + if (prefix == (target_ulong) -1ULL) { >> + return -EINVAL; >> + } >> + env->excp_prefix = prefix; >> + return 0; >> +} >> + >> static int cpu_post_load(void *opaque, int version_id) >> { >> PowerPCCPU *cpu = opaque; >> CPUPPCState *env = &cpu->env; >> int i; >> target_ulong msr; >> + int ret = 0; >> >> /* >> * We always ignore the source PVR. The user or management >> @@ -201,7 +215,11 @@ static int cpu_post_load(void *opaque, i >> >> hreg_compute_mem_idx(env); >> >> - return 0; >> + if (env->spr[SPR_LPCR] & LPCR_AIL) { >> + ret = cpu_post_load_excp_prefix(env); >> + } >> + >> + return ret; >> } >> >> static bool fpu_needed(void *opaque) >> Index: qemu-dgibson-for-2.6.git/target-ppc/translate_init.c >> =================================================================== >> --- qemu-dgibson-for-2.6.git.orig/target-ppc/translate_init.c >> +++ qemu-dgibson-for-2.6.git/target-ppc/translate_init.c >> @@ -8522,6 +8522,20 @@ void cpu_ppc_set_papr(PowerPCCPU *cpu) >> } >> } >> >> +target_ulong cpu_ppc_get_excp_prefix(target_ulong ail) >> +{ >> + switch (ail) { >> + case AIL_NONE: >> + return 0; >> + case AIL_0001_8000: >> + return 0x18000; >> + case AIL_C000_0000_0000_4000: >> + return 0xC000000000004000ULL; >> + default: >> + return (target_ulong) -1ULL; >> + } >> +} >> + >> #endif /* !defined(CONFIG_USER_ONLY) */ >> >> #endif /* defined (TARGET_PPC64) */ >> Index: qemu-dgibson-for-2.6.git/target-ppc/cpu.h >> =================================================================== >> --- qemu-dgibson-for-2.6.git.orig/target-ppc/cpu.h >> +++ qemu-dgibson-for-2.6.git/target-ppc/cpu.h >> @@ -1269,6 +1269,7 @@ void store_booke_tsr (CPUPPCState *env, >> void ppc_tlb_invalidate_all (CPUPPCState *env); >> void ppc_tlb_invalidate_one (CPUPPCState *env, target_ulong addr); >> void cpu_ppc_set_papr(PowerPCCPU *cpu); >> +target_ulong cpu_ppc_get_excp_prefix(target_ulong ail); >> #endif >> #endif >> >> @@ -2277,6 +2278,14 @@ enum { >> HMER_XSCOM_STATUS_LSH = (63 - 23), >> }; >> >> +/* Alternate Interrupt Location (AIL) */ >> +enum { >> + AIL_NONE = 0, >> + AIL_RESERVED = 1, >> + AIL_0001_8000 = 2, >> + AIL_C000_0000_0000_4000 = 3, >> +}; >> + >> /*****************************************************************************/ >> >> static inline target_ulong cpu_read_xer(CPUPPCState *env) >> Index: qemu-dgibson-for-2.6.git/include/hw/ppc/spapr.h >> =================================================================== >> --- qemu-dgibson-for-2.6.git.orig/include/hw/ppc/spapr.h >> +++ qemu-dgibson-for-2.6.git/include/hw/ppc/spapr.h >> @@ -204,11 +204,6 @@ struct sPAPRMachineState { >> #define H_SET_MODE_ENDIAN_BIG 0 >> #define H_SET_MODE_ENDIAN_LITTLE 1 >> >> -/* Flags for H_SET_MODE_RESOURCE_ADDR_TRANS_MODE */ >> -#define H_SET_MODE_ADDR_TRANS_NONE 0 >> -#define H_SET_MODE_ADDR_TRANS_0001_8000 2 >> -#define H_SET_MODE_ADDR_TRANS_C000_0000_0000_4000 3 >> - >> /* VASI States */ >> #define H_VASI_INVALID 0 >> #define H_VASI_ENABLED 1 >
>>> Index: qemu-dgibson-for-2.6.git/hw/ppc/spapr_hcall.c >>> =================================================================== >>> --- qemu-dgibson-for-2.6.git.orig/hw/ppc/spapr_hcall.c >>> +++ qemu-dgibson-for-2.6.git/hw/ppc/spapr_hcall.c >>> @@ -835,17 +835,8 @@ static target_ulong h_set_mode_resource_ >>> return H_P4; >>> } >>> >>> - switch (mflags) { >>> - case H_SET_MODE_ADDR_TRANS_NONE: >>> - prefix = 0; >>> - break; >>> - case H_SET_MODE_ADDR_TRANS_0001_8000: >>> - prefix = 0x18000; >>> - break; >>> - case H_SET_MODE_ADDR_TRANS_C000_0000_0000_4000: >>> - prefix = 0xC000000000004000ULL; >>> - break; >>> - default: >>> + prefix = cpu_ppc_get_excp_prefix(mflags); >>> + if (prefix == (target_ulong) -1ULL) { >> >> + if (prefix == (target_ulong) (-1ULL)) { >> >> to make ./scripts/checkpatch.pl happy :) > > yes. > > The funny thing is that checkpatch.pl does not complain for the exact > same line below. Looks like a checkpatch.pl bug to me. Well, the reason is that a new possible type 'target_ulong' is caught a little late in the checkpatch process and this is why the script complains at the first occurrence. Pointer casts are caught early but not casts, probably because this is too complex to catch with a regex. So the easy fix would be to add 'target_ulong' in the @typeList array. This seems reasonable enough as 'target_ulong' is a common qemu type. C.
On Thu, Mar 31, 2016 at 08:13:09AM +0100, Mark Cave-Ayland wrote: > On 31/03/16 05:55, David Gibson wrote: > > > On Wed, Mar 30, 2016 at 05:38:34PM +0200, Cédric Le Goater wrote: > >> This address is changed by the linux kernel using the H_SET_MODE hcall > >> and needs to be restored when migrating a spapr VM running in > >> TCG. This can be done using the AIL bits from the LPCR register. > >> > >> The patch introduces a helper routine cpu_ppc_get_excp_prefix() which > >> returns the effective address offset of the interrupt handler > >> depending on the LPCR_AIL bits. The same helper can be used in the > >> H_SET_MODE hcall, which lets us remove the H_SET_MODE_ADDR_TRANS_* > >> defines. > >> > >> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> > > > > I've applied this (with Greg's minor amendments) to ppc-for-2.6), > > since it certainly improves behaviour, although I have a couple of > > queries: > > > >> --- > >> > >> Changes since v1: > >> > >> - moved helper routine under target-ppc/ > >> - moved the restore of excp_prefix under cpu_post_load() > >> > >> hw/ppc/spapr_hcall.c | 13 ++----------- > >> include/hw/ppc/spapr.h | 5 ----- > >> target-ppc/cpu.h | 9 +++++++++ > >> target-ppc/machine.c | 20 +++++++++++++++++++- > >> target-ppc/translate_init.c | 14 ++++++++++++++ > >> 5 files changed, 44 insertions(+), 17 deletions(-) > >> > >> Index: qemu-dgibson-for-2.6.git/hw/ppc/spapr_hcall.c > >> =================================================================== > >> --- qemu-dgibson-for-2.6.git.orig/hw/ppc/spapr_hcall.c > >> +++ qemu-dgibson-for-2.6.git/hw/ppc/spapr_hcall.c > >> @@ -835,17 +835,8 @@ static target_ulong h_set_mode_resource_ > >> return H_P4; > >> } > >> > >> - switch (mflags) { > >> - case H_SET_MODE_ADDR_TRANS_NONE: > >> - prefix = 0; > >> - break; > >> - case H_SET_MODE_ADDR_TRANS_0001_8000: > >> - prefix = 0x18000; > >> - break; > >> - case H_SET_MODE_ADDR_TRANS_C000_0000_0000_4000: > >> - prefix = 0xC000000000004000ULL; > >> - break; > >> - default: > >> + prefix = cpu_ppc_get_excp_prefix(mflags); > >> + if (prefix == (target_ulong) -1ULL) { > >> return H_UNSUPPORTED_FLAG; > >> } > >> > >> Index: qemu-dgibson-for-2.6.git/target-ppc/machine.c > >> =================================================================== > >> --- qemu-dgibson-for-2.6.git.orig/target-ppc/machine.c > >> +++ qemu-dgibson-for-2.6.git/target-ppc/machine.c > >> @@ -156,12 +156,26 @@ static void cpu_pre_save(void *opaque) > >> } > >> } > >> > >> + > >> +static int cpu_post_load_excp_prefix(CPUPPCState *env) > >> +{ > >> + int ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT; > >> + target_ulong prefix = cpu_ppc_get_excp_prefix(ail); > >> + > >> + if (prefix == (target_ulong) -1ULL) { > >> + return -EINVAL; > >> + } > >> + env->excp_prefix = prefix; > >> + return 0; > >> +} > >> + > >> static int cpu_post_load(void *opaque, int version_id) > >> { > >> PowerPCCPU *cpu = opaque; > >> CPUPPCState *env = &cpu->env; > >> int i; > >> target_ulong msr; > >> + int ret = 0; > >> > >> /* > >> * We always ignore the source PVR. The user or management > >> @@ -201,7 +215,11 @@ static int cpu_post_load(void *opaque, i > >> > >> hreg_compute_mem_idx(env); > >> > >> - return 0; > >> + if (env->spr[SPR_LPCR] & LPCR_AIL) { > >> + ret = cpu_post_load_excp_prefix(env); > >> + } > > > > Why not call this unconditionally? If AIL == 0 it will still do the > > right thing. > > > > Aren't there also circumstances where the exception prefix can depend > > on the MSR? Do those need to be handled somewhere? > > Yes indeed - this was part of my patchset last year to fix up various > migration issues for the Mac PPC machines (see commit > 2360b6e84f78d41fa0f76555a947148b73645259). > > I agree that having the env->excp_prefix logic split like this isn't a > particularly great idea. Let's just have a single helper function as in > the patch above and use that in both places (and in fact with recent > changes I have a feeling env->excp_prefix is one of the few remaining > reasons that the above change is still required, but please check this). Right. So, what I'd really prefer to see here is a single update_excp_prefix() helper which will set env->excp_prefix based on whatever registers are relevant (LPCR, MSR and probably the cpu model). That would be called on incoming migration, and after updating any of the relevant registers (so from the mtmsr and mtlpcr emulations). H_SET_MODE should be handled by first updating the LPCR value, then calling the update helper. Cédric, I'm ok if you provide a version of that which only handles POWER7 and POWER8 (i.e. spapr compatible models for now). Mark - if you can supply corrections to that outline for Macintosh era models, that would be great. I do want to get the basic migration problem fixed before 2.6 is release.
On Thu, Mar 31, 2016 at 03:55:42PM +1100, David Gibson wrote: > On Wed, Mar 30, 2016 at 05:38:34PM +0200, Cédric Le Goater wrote: > > This address is changed by the linux kernel using the H_SET_MODE hcall > > and needs to be restored when migrating a spapr VM running in > > TCG. This can be done using the AIL bits from the LPCR register. > > > > The patch introduces a helper routine cpu_ppc_get_excp_prefix() which > > returns the effective address offset of the interrupt handler > > depending on the LPCR_AIL bits. The same helper can be used in the > > H_SET_MODE hcall, which lets us remove the H_SET_MODE_ADDR_TRANS_* > > defines. > > > > Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> > > I've applied this (with Greg's minor amendments) to ppc-for-2.6), > since it certainly improves behaviour, although I have a couple of > queries: And.. I've take it out again now. In addition to the fact that I'd like some rework suggested elsewhere, it breaks compile for 32-bit ppc targets.
Index: qemu-dgibson-for-2.6.git/hw/ppc/spapr_hcall.c =================================================================== --- qemu-dgibson-for-2.6.git.orig/hw/ppc/spapr_hcall.c +++ qemu-dgibson-for-2.6.git/hw/ppc/spapr_hcall.c @@ -835,17 +835,8 @@ static target_ulong h_set_mode_resource_ return H_P4; } - switch (mflags) { - case H_SET_MODE_ADDR_TRANS_NONE: - prefix = 0; - break; - case H_SET_MODE_ADDR_TRANS_0001_8000: - prefix = 0x18000; - break; - case H_SET_MODE_ADDR_TRANS_C000_0000_0000_4000: - prefix = 0xC000000000004000ULL; - break; - default: + prefix = cpu_ppc_get_excp_prefix(mflags); + if (prefix == (target_ulong) -1ULL) { return H_UNSUPPORTED_FLAG; } Index: qemu-dgibson-for-2.6.git/target-ppc/machine.c =================================================================== --- qemu-dgibson-for-2.6.git.orig/target-ppc/machine.c +++ qemu-dgibson-for-2.6.git/target-ppc/machine.c @@ -156,12 +156,26 @@ static void cpu_pre_save(void *opaque) } } + +static int cpu_post_load_excp_prefix(CPUPPCState *env) +{ + int ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT; + target_ulong prefix = cpu_ppc_get_excp_prefix(ail); + + if (prefix == (target_ulong) -1ULL) { + return -EINVAL; + } + env->excp_prefix = prefix; + return 0; +} + static int cpu_post_load(void *opaque, int version_id) { PowerPCCPU *cpu = opaque; CPUPPCState *env = &cpu->env; int i; target_ulong msr; + int ret = 0; /* * We always ignore the source PVR. The user or management @@ -201,7 +215,11 @@ static int cpu_post_load(void *opaque, i hreg_compute_mem_idx(env); - return 0; + if (env->spr[SPR_LPCR] & LPCR_AIL) { + ret = cpu_post_load_excp_prefix(env); + } + + return ret; } static bool fpu_needed(void *opaque) Index: qemu-dgibson-for-2.6.git/target-ppc/translate_init.c =================================================================== --- qemu-dgibson-for-2.6.git.orig/target-ppc/translate_init.c +++ qemu-dgibson-for-2.6.git/target-ppc/translate_init.c @@ -8522,6 +8522,20 @@ void cpu_ppc_set_papr(PowerPCCPU *cpu) } } +target_ulong cpu_ppc_get_excp_prefix(target_ulong ail) +{ + switch (ail) { + case AIL_NONE: + return 0; + case AIL_0001_8000: + return 0x18000; + case AIL_C000_0000_0000_4000: + return 0xC000000000004000ULL; + default: + return (target_ulong) -1ULL; + } +} + #endif /* !defined(CONFIG_USER_ONLY) */ #endif /* defined (TARGET_PPC64) */ Index: qemu-dgibson-for-2.6.git/target-ppc/cpu.h =================================================================== --- qemu-dgibson-for-2.6.git.orig/target-ppc/cpu.h +++ qemu-dgibson-for-2.6.git/target-ppc/cpu.h @@ -1269,6 +1269,7 @@ void store_booke_tsr (CPUPPCState *env, void ppc_tlb_invalidate_all (CPUPPCState *env); void ppc_tlb_invalidate_one (CPUPPCState *env, target_ulong addr); void cpu_ppc_set_papr(PowerPCCPU *cpu); +target_ulong cpu_ppc_get_excp_prefix(target_ulong ail); #endif #endif @@ -2277,6 +2278,14 @@ enum { HMER_XSCOM_STATUS_LSH = (63 - 23), }; +/* Alternate Interrupt Location (AIL) */ +enum { + AIL_NONE = 0, + AIL_RESERVED = 1, + AIL_0001_8000 = 2, + AIL_C000_0000_0000_4000 = 3, +}; + /*****************************************************************************/ static inline target_ulong cpu_read_xer(CPUPPCState *env) Index: qemu-dgibson-for-2.6.git/include/hw/ppc/spapr.h =================================================================== --- qemu-dgibson-for-2.6.git.orig/include/hw/ppc/spapr.h +++ qemu-dgibson-for-2.6.git/include/hw/ppc/spapr.h @@ -204,11 +204,6 @@ struct sPAPRMachineState { #define H_SET_MODE_ENDIAN_BIG 0 #define H_SET_MODE_ENDIAN_LITTLE 1 -/* Flags for H_SET_MODE_RESOURCE_ADDR_TRANS_MODE */ -#define H_SET_MODE_ADDR_TRANS_NONE 0 -#define H_SET_MODE_ADDR_TRANS_0001_8000 2 -#define H_SET_MODE_ADDR_TRANS_C000_0000_0000_4000 3 - /* VASI States */ #define H_VASI_INVALID 0 #define H_VASI_ENABLED 1
This address is changed by the linux kernel using the H_SET_MODE hcall and needs to be restored when migrating a spapr VM running in TCG. This can be done using the AIL bits from the LPCR register. The patch introduces a helper routine cpu_ppc_get_excp_prefix() which returns the effective address offset of the interrupt handler depending on the LPCR_AIL bits. The same helper can be used in the H_SET_MODE hcall, which lets us remove the H_SET_MODE_ADDR_TRANS_* defines. Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> --- Changes since v1: - moved helper routine under target-ppc/ - moved the restore of excp_prefix under cpu_post_load() hw/ppc/spapr_hcall.c | 13 ++----------- include/hw/ppc/spapr.h | 5 ----- target-ppc/cpu.h | 9 +++++++++ target-ppc/machine.c | 20 +++++++++++++++++++- target-ppc/translate_init.c | 14 ++++++++++++++ 5 files changed, 44 insertions(+), 17 deletions(-)