On 04/01/2016 04:43 AM, David Gibson wrote:
> 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 <c...@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).
Sure.
But first, could you please take a look at a reworked patch of Ben who
had forseen the issue ? I kept the AIL fix, added some extra for ILE
and cleanup H_SET_MODE. Tests under KVM,TCG on LE,BE guests went fine.
If you think this is too risky for 2.6, I will work on what you asked for.
> 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.
Thanks,
C.
>From bb9d1e06a0ea2132902db724f61c8dc655aa1a96 Mon Sep 17 00:00:00 2001
From: Benjamin Herrenschmidt <b...@kernel.crashing.org>
Date: Thu, 4 Jun 2015 03:39:19 +1000
Subject: [PATCH 18/77] ppc: Rework POWER7 & POWER8 exception model
This patch fixes the current AIL implementation for POWER8. The
interrupt vector address can be calculated directly from LPCR when the
exception is handled. The excp_prefix update becomes useless and we
can cleanup the H_SET_MODE hcall.
Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
[clg: Removed LPES0/1 handling for HV vs. !HV
Fixed LPCR_ILE case for POWERPC_EXCP_POWER8 ]
Signed-off-by: Cédric Le Goater <c...@fr.ibm.com>
---
hw/ppc/spapr_hcall.c | 16 --------------
include/hw/ppc/spapr.h | 5 ----
target-ppc/cpu.h | 10 ++++++++
target-ppc/excp_helper.c | 49 ++++++++++++++++++++++++++++++++++++++++++--
target-ppc/translate_init.c | 2 -
5 files changed, 59 insertions(+), 23 deletions(-)
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
@@ -167,6 +167,8 @@ enum powerpc_excp_t {
POWERPC_EXCP_970,
/* POWER7 exception model */
POWERPC_EXCP_POWER7,
+ /* POWER8 exception model */
+ POWERPC_EXCP_POWER8,
#endif /* defined(TARGET_PPC64) */
};
@@ -2277,6 +2279,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/target-ppc/excp_helper.c
===================================================================
--- qemu-dgibson-for-2.6.git.orig/target-ppc/excp_helper.c
+++ qemu-dgibson-for-2.6.git/target-ppc/excp_helper.c
@@ -77,7 +77,7 @@ static inline void powerpc_excp(PowerPCC
CPUPPCState *env = &cpu->env;
target_ulong msr, new_msr, vector;
int srr0, srr1, asrr0, asrr1;
- int lpes0, lpes1, lev;
+ int lpes0, lpes1, lev, ail;
if (0) {
/* XXX: find a suitable condition to enable the hypervisor mode */
@@ -108,6 +108,25 @@ static inline void powerpc_excp(PowerPCC
asrr0 = -1;
asrr1 = -1;
+ /* Exception targetting modifiers
+ *
+ * AIL is initialized here but can be cleared by
+ * selected exceptions
+ */
+#if defined(TARGET_PPC64)
+ if (excp_model == POWERPC_EXCP_POWER7 ||
+ excp_model == POWERPC_EXCP_POWER8) {
+ if (excp_model == POWERPC_EXCP_POWER8) {
+ ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT;
+ } else {
+ ail = 0;
+ }
+ } else
+#endif /* defined(TARGET_PPC64) */
+ {
+ ail = 0;
+ }
+
switch (excp) {
case POWERPC_EXCP_NONE:
/* Should never happen */
@@ -146,6 +165,7 @@ static inline void powerpc_excp(PowerPCC
/* XXX: find a suitable condition to enable the hypervisor mode */
new_msr |= (target_ulong)MSR_HVB;
}
+ ail = 0;
/* machine check exceptions don't have ME set */
new_msr &= ~((target_ulong)1 << MSR_ME);
@@ -344,6 +364,7 @@ static inline void powerpc_excp(PowerPCC
/* XXX: find a suitable condition to enable the hypervisor mode */
new_msr |= (target_ulong)MSR_HVB;
}
+ ail = 0;
goto store_next;
case POWERPC_EXCP_DSEG: /* Data segment exception */
if (lpes1 == 0) {
@@ -630,7 +651,8 @@ static inline void powerpc_excp(PowerPCC
}
#ifdef TARGET_PPC64
- if (excp_model == POWERPC_EXCP_POWER7) {
+ if (excp_model == POWERPC_EXCP_POWER7 ||
+ excp_model == POWERPC_EXCP_POWER8) {
if (env->spr[SPR_LPCR] & LPCR_ILE) {
new_msr |= (target_ulong)1 << MSR_LE;
}
@@ -650,6 +672,29 @@ static inline void powerpc_excp(PowerPCC
excp);
}
vector |= env->excp_prefix;
+
+ /* AIL only works if there is no HV transition and we are running with
+ * translations enabled
+ */
+ if (!((msr >> MSR_IR) & 1) || !((msr >> MSR_DR) & 1)) {
+ ail = 0;
+ }
+ /* Handle AIL */
+ if (ail) {
+ new_msr |= (1 << MSR_IR) | (1 << MSR_DR);
+ switch(ail) {
+ case AIL_0001_8000:
+ vector |= 0x18000;
+ break;
+ case AIL_C000_0000_0000_4000:
+ vector |= 0xc000000000004000ull;
+ break;
+ default:
+ cpu_abort(cs, "Invalid AIL combination %d\n", ail);
+ break;
+ }
+ }
+
#if defined(TARGET_PPC64)
if (excp_model == POWERPC_EXCP_BOOKE) {
if (env->spr[SPR_BOOKE_EPCR] & EPCR_ICM) {
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
@@ -8487,7 +8487,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc,
pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
pcc->sps = &POWER7_POWER8_sps;
#endif
- pcc->excp_model = POWERPC_EXCP_POWER7;
+ pcc->excp_model = POWERPC_EXCP_POWER8;
pcc->bus_model = PPC_FLAGS_INPUT_POWER7;
pcc->bfd_mach = bfd_mach_ppc64;
pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
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
@@ -823,7 +823,6 @@ static target_ulong h_set_mode_resource_
{
CPUState *cs;
PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
- target_ulong prefix;
if (!(pcc->insns_flags2 & PPC2_ISA207S)) {
return H_P2;
@@ -835,25 +834,12 @@ 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:
+ if (mflags == AIL_RESERVED) {
return H_UNSUPPORTED_FLAG;
}
CPU_FOREACH(cs) {
- CPUPPCState *env = &POWERPC_CPU(cpu)->env;
-
set_spr(cs, SPR_LPCR, mflags << LPCR_AIL_SHIFT, LPCR_AIL);
- env->excp_prefix = prefix;
}
return H_SUCCESS;
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
