From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33045) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SDBOE-0003WI-2C for qemu-devel@nongnu.org; Thu, 29 Mar 2012 05:11:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SDBO7-0006W0-SU for qemu-devel@nongnu.org; Thu, 29 Mar 2012 05:11:29 -0400 Message-ID: <4F742739.30008@ilande.co.uk> Date: Thu, 29 Mar 2012 10:11:21 +0100 From: Mark Cave-Ayland MIME-Version: 1.0 References: <1332862915-27501-1-git-send-email-mark.cave-ayland@ilande.co.uk> <1332862915-27501-2-git-send-email-mark.cave-ayland@ilande.co.uk> <4F71FD34.2040200@freescale.com> <20120328004653.GE9582@truffala.fritz.box> In-Reply-To: <20120328004653.GE9582@truffala.fritz.box> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/2] PPC: Fix interrupt MSR value within the PPC interrupt handler. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Scott Wood , qemu-ppc@nongnu.org, qemu-devel@nongnu.org On 28/03/12 01:46, David Gibson wrote: Hi David, >> If we're going to make this specific to MSRs, might as well cut down on >> the user's verbosity: >> >> #define MSR_BIT(x) ((target_ulong)1<< MSR_##x) >> >> ...and move it to a header file. >> >> Or possibly have the header file define a set of MSRBIT_IR, MSRBIT_DR, etc. I think I prefer your macro above and move it to a relevant part of target-ppc/cpu.h with the other MSR defines. >>> static inline void powerpc_excp(CPUPPCState *env, int excp_model, int excp) >>> { >>> target_ulong msr, new_msr, vector; >>> @@ -2478,11 +2480,26 @@ static inline void powerpc_excp(CPUPPCState *env, int excp_model, int excp) >>> qemu_log_mask(CPU_LOG_INT, "Raise exception at " TARGET_FMT_lx >>> " => %08x (%02x)\n", env->nip, excp, env->error_code); >>> >>> - /* new srr1 value excluding must-be-zero bits */ >>> + /* new srr1 value with interrupt-specific bits defaulting to zero */ >>> msr = env->msr& ~0x783f0000ULL; >>> >>> - /* new interrupt handler msr */ >>> - new_msr = env->msr& ((target_ulong)1<< MSR_ME); >>> + switch (excp_model) { >>> + case POWERPC_EXCP_BOOKE: >>> + /* new interrupt handler msr */ >>> + new_msr = env->msr& ((target_ulong)1<< MSR_ME); >>> + break; >>> + >>> + default: >>> + /* new interrupt handler msr (as per PowerISA 2.06B p.811 and p.814): >>> + 1) force the following bits to zero >>> + IR, DR, FE0, FE1, EE, BE, FP, PMM, PR, SE >>> + 2) default the following bits to zero (can be overidden later on) >>> + RI */ >>> + new_msr = env->msr& ~(MSR_BIT(MSR_IR) | MSR_BIT(MSR_DR) >>> + | MSR_BIT(MSR_FE0)| MSR_BIT(MSR_FE1) | MSR_BIT(MSR_EE) >>> + | MSR_BIT(MSR_BE) | MSR_BIT(MSR_FP) | MSR_BIT(MSR_PMM) >>> + | MSR_BIT(MSR_PR) | MSR_BIT(MSR_SE) | MSR_BIT(MSR_RI)); >>> + } >> >> What about POWERPC_EXCP_40x? And are all the classic chips OK with the >> 2.06B implementation? > > Hrm, yeah. I think what you ought to do is to use the new logic just > for the "classic" exception models. Have the default branch remain > the one that just masks ME. That's wrong, but it's the same wrong as > we have already, and we can fix it later once we've verified what the > right thing to do is for 40x and BookE. I'm actually coming at this from a fixing what was potentially an OpenBIOS bug rather than a PPC angle, so I have to admit I have no I idea which ones are the "classic" exception models. Would you consider this to be just EXCP_STD, EXCP_6* and EXCP_7*? Many thanks, Mark.