Re: [Qemu-devel] [PATCH v2 01/10] ppc: Fix rfi/rfid/hrfi/... emulation

2016-09-08 Thread Cédric Le Goater 
On 09/07/2016 11:48 PM, Benjamin Herrenschmidt wrote:
> On Wed, 2016-09-07 at 14:13 +0200, Cédric Le Goater wrote:
>> On 09/07/2016 01:08 PM, Benjamin Herrenschmidt wrote:
>>>
>>> On Wed, 2016-09-07 at 12:50 +0200, Cédric Le Goater wrote:

 This is a bit broader than Ben's patch which used
 PPC_SEGMENT_64B. 
 it's basically !PPC_64B which includes the e5500.

 If so, here is a proposal below adding a new PPC_RFI in the 
 "PowerPC Instructions types definitions" enum for that purpose. 
 Not much bits left there.
>>>
>>> Why not stick to PPC_SEGMENT_64B ?
>>
>> I am trying to remove the rfi instruction from the set of the CPU 
>> and I think we need to introduce a new PPC_* bit for GEN_HANDLER to :
> 
> What does it buy you instead of just having the test in the handler ?

not much. I will send the remaining bit of the original patch.

Thanks,
C.


>> +GEN_HANDLER(rfi, 0x13, 0x12, 0x01, 0x03FF8001, PPC_RFI),
>>
>> we can also keep the test on PPC_SEGMENT_64B in the handler which 
>> works perfectly fine. 
>>
>>>
>>> rfi exists on all 32-bit processors and all non-Book3S (aka server
>>> aka
>>> segment/hash) 64-bit. So PPC_SEGMENT_64B is the test we want.
>>>
>>> IE. rfi does exist on e5500
>>
>> ok.
>>
>> Cheers,
>> C.

Re: [Qemu-devel] [PATCH v2 01/10] ppc: Fix rfi/rfid/hrfi/... emulation

2016-09-07 Thread Benjamin Herrenschmidt 
On Wed, 2016-09-07 at 14:13 +0200, Cédric Le Goater wrote:
> On 09/07/2016 01:08 PM, Benjamin Herrenschmidt wrote:
> > 
> > On Wed, 2016-09-07 at 12:50 +0200, Cédric Le Goater wrote:
> > > 
> > > This is a bit broader than Ben's patch which used
> > > PPC_SEGMENT_64B. 
> > > it's basically !PPC_64B which includes the e5500.
> > > 
> > > If so, here is a proposal below adding a new PPC_RFI in the 
> > > "PowerPC Instructions types definitions" enum for that purpose. 
> > > Not much bits left there.
> > 
> > Why not stick to PPC_SEGMENT_64B ?
> 
> I am trying to remove the rfi instruction from the set of the CPU 
> and I think we need to introduce a new PPC_* bit for GEN_HANDLER to :

What does it buy you instead of just having the test in the handler ?

> +GEN_HANDLER(rfi, 0x13, 0x12, 0x01, 0x03FF8001, PPC_RFI),
> 
> we can also keep the test on PPC_SEGMENT_64B in the handler which 
> works perfectly fine. 
> 
> > 
> > rfi exists on all 32-bit processors and all non-Book3S (aka server
> > aka
> > segment/hash) 64-bit. So PPC_SEGMENT_64B is the test we want.
> > 
> > IE. rfi does exist on e5500
> 
> ok.
> 
> Cheers,
> C.

Re: [Qemu-devel] [PATCH v2 01/10] ppc: Fix rfi/rfid/hrfi/... emulation

2016-09-07 Thread Cédric Le Goater 
On 09/07/2016 01:08 PM, Benjamin Herrenschmidt wrote:
> On Wed, 2016-09-07 at 12:50 +0200, Cédric Le Goater wrote:
>> This is a bit broader than Ben's patch which used PPC_SEGMENT_64B. 
>> it's basically !PPC_64B which includes the e5500.
>>
>> If so, here is a proposal below adding a new PPC_RFI in the 
>> "PowerPC Instructions types definitions" enum for that purpose. 
>> Not much bits left there.
> 
> Why not stick to PPC_SEGMENT_64B ?

I am trying to remove the rfi instruction from the set of the CPU 
and I think we need to introduce a new PPC_* bit for GEN_HANDLER to :

+GEN_HANDLER(rfi, 0x13, 0x12, 0x01, 0x03FF8001, PPC_RFI),

we can also keep the test on PPC_SEGMENT_64B in the handler which 
works perfectly fine. 

> rfi exists on all 32-bit processors and all non-Book3S (aka server aka
> segment/hash) 64-bit. So PPC_SEGMENT_64B is the test we want.
> 
> IE. rfi does exist on e5500

ok.

Cheers,
C.

Re: [Qemu-devel] [PATCH v2 01/10] ppc: Fix rfi/rfid/hrfi/... emulation

2016-09-07 Thread Benjamin Herrenschmidt 
On Wed, 2016-09-07 at 12:50 +0200, Cédric Le Goater wrote:
> This is a bit broader than Ben's patch which used PPC_SEGMENT_64B. 
> it's basically !PPC_64B which includes the e5500.
> 
> If so, here is a proposal below adding a new PPC_RFI in the 
> "PowerPC Instructions types definitions" enum for that purpose. 
> Not much bits left there.

Why not stick to PPC_SEGMENT_64B ?

rfi exists on all 32-bit processors and all non-Book3S (aka server aka
segment/hash) 64-bit. So PPC_SEGMENT_64B is the test we want.

IE. rfi does exist on e5500

Cheers,
Ben.

Re: [Qemu-devel] [PATCH v2 01/10] ppc: Fix rfi/rfid/hrfi/... emulation

2016-09-07 Thread Cédric Le Goater 
On 09/06/2016 09:07 AM, Mark Cave-Ayland wrote:
> On 06/09/16 01:16, David Gibson wrote:
> 
>> On Mon, Sep 05, 2016 at 09:51:09PM +0100, Mark Cave-Ayland wrote:
>>> On 05/09/16 21:30, Cédric Le Goater wrote:
>>>
> Shall we disable rfi now for QEMU 2.8 ? Cédric, could you maybe send a
> patch with that hunk again?

 Sure. I have kept it in a warm place here : 
  

 https://github.com/legoater/qemu/commit/492a631e4e817863be312c1a34957cd8d679a56c

 Mark, is openbios at the right level now ? I have lost track of the 
 recent changes.
>>>
>>> The following patch is already in the current OpenBIOS binaries:
>>> https://github.com/openbios/openbios/commit/b747b6acc272f6ab839728193042455c9b36e26a.
>>> Is that the one you're looking for?
>>
>> Right, the relevant question is whether the updated openbios is in the
>> qemu submodule and canned binary.
> 
> Yes, it was included in last merge for 2.7.

make check-qtest-ppc now runs fine with original patch :


https://github.com/legoater/qemu/commit/492a631e4e817863be312c1a34957cd8d679a56c

but we could also just remove the rfi instruction from the 
instruction set of the processors not supporting it. This is 
where it gets complicated :)

I suppose the impact is on 60x, 75x, 74xx, embedded (all the 
32bits) but not the 64bits  :  

POWERPC_FAMILY(e5500)(ObjectClass *oc, void *data)
POWERPC_FAMILY(970)(ObjectClass *oc, void *data)
POWERPC_FAMILY(POWER5P)(ObjectClass *oc, void *data)
POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)

This is a bit broader than Ben's patch which used PPC_SEGMENT_64B. 
it's basically !PPC_64B which includes the e5500.

If so, here is a proposal below adding a new PPC_RFI in the 
"PowerPC Instructions types definitions" enum for that purpose. 
Not much bits left there.

Thanks,

C. 


From: Cédric Le Goater 
Subject: [PATCH] ppc: enable rfi support for 32bits CPUs only
Date: Wed, 07 Sep 2016 11:55:43 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Add a new PPC_RFI bit in the PowerPC Instructions types definitions
for this purpose and update the insns_flags of the target CPUs.

Signed-off-by: Cédric Le Goater 
---
 target-ppc/cpu.h|5 +-
 target-ppc/translate.c  |2 
 target-ppc/translate_init.c |   92 ++--
 3 files changed, 51 insertions(+), 48 deletions(-)

Index: qemu-dgibson-for-2.8.git/target-ppc/translate.c
===
--- qemu-dgibson-for-2.8.git.orig/target-ppc/translate.c
+++ qemu-dgibson-for-2.8.git/target-ppc/translate.c
@@ -6297,7 +6297,7 @@ GEN_HANDLER(bcctr, 0x13, 0x10, 0x10, 0x0
 GEN_HANDLER(bclr, 0x13, 0x10, 0x00, 0x, PPC_FLOW),
 GEN_HANDLER_E(bctar, 0x13, 0x10, 0x11, 0, PPC_NONE, PPC2_BCTAR_ISA207),
 GEN_HANDLER(mcrf, 0x13, 0x00, 0xFF, 0x0001, PPC_INTEGER),
-GEN_HANDLER(rfi, 0x13, 0x12, 0x01, 0x03FF8001, PPC_FLOW),
+GEN_HANDLER(rfi, 0x13, 0x12, 0x01, 0x03FF8001, PPC_RFI),
 #if defined(TARGET_PPC64)
 GEN_HANDLER(rfid, 0x13, 0x12, 0x00, 0x03FF8001, PPC_64B),
 GEN_HANDLER_E(doze, 0x13, 0x12, 0x0c, 0x03FFF801, PPC_NONE, PPC2_PM_ISA206),
Index: qemu-dgibson-for-2.8.git/target-ppc/cpu.h
===
--- qemu-dgibson-for-2.8.git.orig/target-ppc/cpu.h
+++ qemu-dgibson-for-2.8.git/target-ppc/cpu.h
@@ -1997,6 +1997,9 @@ enum {
 /*   SLB management  */
 PPC_SLBI   = 0x4000ULL,
 
+/*   rfi support  
*/
+PPC_RFI   = 0x8000ULL,
+
 /* Embedded PowerPC dedicated instructions   */
 PPC_WRTEE  = 0x0001ULL,
 /* PowerPC 40x exception model   */
@@ -2047,7 +2050,7 @@ enum {
 | PPC_CACHE_DCBA | PPC_CACHE_LOCK \
 | PPC_EXTERN | PPC_SEGMENT | PPC_6xx_TLB \
 | PPC_74xx_TLB | PPC_40x_TLB | PPC_SEGMENT_64B \
-| PPC_SLBI | PPC_WRTEE | PPC_40x_EXCP \
+| PPC_SLBI | PPC_RFI | PPC_WRTEE | PPC_40x_EXCP \
 | PPC_405_MAC | PPC_440_SPEC | PPC_BOOKE \
 | PPC_MFAPIDI | PPC_TLBIVA | PPC_TLBIVAX \
 | PPC_4xx_COMMON | PPC_40x_ICBT | PPC_RFMCI \
Index: qemu-dgibson-for-2.8.git/target-ppc/translate_init.c
===
--- qemu-dgibson-for-2.8.git.orig/target-ppc/translate_init.c
+++ qemu-dgibson-for-2.8.git/target-ppc/translate_init.c
@@ -3325,7 +3325,7 @@ POWERPC_FAMILY(401)(ObjectClass *oc, voi

Re: [Qemu-devel] [PATCH v2 01/10] ppc: Fix rfi/rfid/hrfi/... emulation

2016-09-06 Thread Mark Cave-Ayland 
On 06/09/16 01:16, David Gibson wrote:

> On Mon, Sep 05, 2016 at 09:51:09PM +0100, Mark Cave-Ayland wrote:
>> On 05/09/16 21:30, Cédric Le Goater wrote:
>>
 Shall we disable rfi now for QEMU 2.8 ? Cédric, could you maybe send a
 patch with that hunk again?
>>>
>>> Sure. I have kept it in a warm place here : 
>>>  
>>> 
>>> https://github.com/legoater/qemu/commit/492a631e4e817863be312c1a34957cd8d679a56c
>>>
>>> Mark, is openbios at the right level now ? I have lost track of the 
>>> recent changes.
>>
>> The following patch is already in the current OpenBIOS binaries:
>> https://github.com/openbios/openbios/commit/b747b6acc272f6ab839728193042455c9b36e26a.
>> Is that the one you're looking for?
> 
> Right, the relevant question is whether the updated openbios is in the
> qemu submodule and canned binary.

Yes, it was included in last merge for 2.7.


ATB,

Mark.

Re: [Qemu-devel] [PATCH v2 01/10] ppc: Fix rfi/rfid/hrfi/... emulation

2016-09-05 Thread David Gibson 
On Mon, Sep 05, 2016 at 09:51:09PM +0100, Mark Cave-Ayland wrote:
> On 05/09/16 21:30, Cédric Le Goater wrote:
> 
> >> Shall we disable rfi now for QEMU 2.8 ? Cédric, could you maybe send a
> >> patch with that hunk again?
> > 
> > Sure. I have kept it in a warm place here : 
> >  
> > 
> > https://github.com/legoater/qemu/commit/492a631e4e817863be312c1a34957cd8d679a56c
> > 
> > Mark, is openbios at the right level now ? I have lost track of the 
> > recent changes.
> 
> The following patch is already in the current OpenBIOS binaries:
> https://github.com/openbios/openbios/commit/b747b6acc272f6ab839728193042455c9b36e26a.
> Is that the one you're looking for?

Right, the relevant question is whether the updated openbios is in the
qemu submodule and canned binary.

-- 
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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 01/10] ppc: Fix rfi/rfid/hrfi/... emulation

2016-09-05 Thread Mark Cave-Ayland 
On 05/09/16 21:30, Cédric Le Goater wrote:

>> Shall we disable rfi now for QEMU 2.8 ? Cédric, could you maybe send a
>> patch with that hunk again?
> 
> Sure. I have kept it in a warm place here : 
>  
>   
> https://github.com/legoater/qemu/commit/492a631e4e817863be312c1a34957cd8d679a56c
> 
> Mark, is openbios at the right level now ? I have lost track of the 
> recent changes.

The following patch is already in the current OpenBIOS binaries:
https://github.com/openbios/openbios/commit/b747b6acc272f6ab839728193042455c9b36e26a.
Is that the one you're looking for?


ATB,

Mark.

Re: [Qemu-devel] [PATCH v2 01/10] ppc: Fix rfi/rfid/hrfi/... emulation

2016-09-05 Thread Cédric Le Goater 
On 09/05/2016 10:25 PM, Thomas Huth wrote:
> On 23.06.2016 07:50, David Gibson wrote:
>> On Wed, Jun 22, 2016 at 08:48:14AM +0200, Cédric Le Goater wrote:
>>> On 06/22/2016 04:46 AM, David Gibson wrote:
 I'm not comfortable merging this until the openbios change is pulled
 back into the qemu tree (submodule and pre-built binary).

 Again - sure you don't want to apply this with rfi still enabled for
 64-bit for now, letting the rest of this series go in as well, then
 clean up the rfi/64 behaviour later?
>>>
>>> Sure. I don't think it has an impact on the serie anyway so we can keep
>>> it for later.
>>
>> Ok, I modified 1/10 to leave rfi in for 64-bit cpus for now, and
>> applied the full series to ppc-for-2.7.  I've also now sent a pull
>> request including this.
> 
> Shall we disable rfi now for QEMU 2.8 ? Cédric, could you maybe send a
> patch with that hunk again?

Sure. I have kept it in a warm place here : 
 

https://github.com/legoater/qemu/commit/492a631e4e817863be312c1a34957cd8d679a56c

Mark, is openbios at the right level now ? I have lost track of the 
recent changes.

Thanks,

C.

Re: [Qemu-devel] [PATCH v2 01/10] ppc: Fix rfi/rfid/hrfi/... emulation

2016-09-05 Thread Thomas Huth 
On 23.06.2016 07:50, David Gibson wrote:
> On Wed, Jun 22, 2016 at 08:48:14AM +0200, Cédric Le Goater wrote:
>> On 06/22/2016 04:46 AM, David Gibson wrote:
>>> I'm not comfortable merging this until the openbios change is pulled
>>> back into the qemu tree (submodule and pre-built binary).
>>>
>>> Again - sure you don't want to apply this with rfi still enabled for
>>> 64-bit for now, letting the rest of this series go in as well, then
>>> clean up the rfi/64 behaviour later?
>>
>> Sure. I don't think it has an impact on the serie anyway so we can keep
>> it for later.
> 
> Ok, I modified 1/10 to leave rfi in for 64-bit cpus for now, and
> applied the full series to ppc-for-2.7.  I've also now sent a pull
> request including this.

Shall we disable rfi now for QEMU 2.8 ? Cédric, could you maybe send a
patch with that hunk again?

 Thomas




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 01/10] ppc: Fix rfi/rfid/hrfi/... emulation

2016-06-23 Thread David Gibson 
On Thu, Jun 23, 2016 at 08:08:38AM +0200, Cédric Le Goater wrote:
> On 06/23/2016 07:50 AM, David Gibson wrote:
> > On Wed, Jun 22, 2016 at 08:48:14AM +0200, Cédric Le Goater wrote:
> >> On 06/22/2016 04:46 AM, David Gibson wrote:
> >>> I'm not comfortable merging this until the openbios change is pulled
> >>> back into the qemu tree (submodule and pre-built binary).
> >>>
> >>> Again - sure you don't want to apply this with rfi still enabled for
> >>> 64-bit for now, letting the rest of this series go in as well, then
> >>> clean up the rfi/64 behaviour later?
> >>
> >> Sure. I don't think it has an impact on the serie anyway so we can keep
> >> it for later.
> > 
> > Ok, I modified 1/10 to leave rfi in for 64-bit cpus for now, and
> > applied the full series to ppc-for-2.7.  I've also now sent a pull
> > request including this.
> 
> Perfect. I was wondering if we should not just remove the instruction from 
> the 64bit set.

We should, but I don't want to do so until OpenBIOS is updated to
cope.

-- 
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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 01/10] ppc: Fix rfi/rfid/hrfi/... emulation

2016-06-23 Thread Cédric Le Goater 
On 06/23/2016 07:50 AM, David Gibson wrote:
> On Wed, Jun 22, 2016 at 08:48:14AM +0200, Cédric Le Goater wrote:
>> On 06/22/2016 04:46 AM, David Gibson wrote:
>>> I'm not comfortable merging this until the openbios change is pulled
>>> back into the qemu tree (submodule and pre-built binary).
>>>
>>> Again - sure you don't want to apply this with rfi still enabled for
>>> 64-bit for now, letting the rest of this series go in as well, then
>>> clean up the rfi/64 behaviour later?
>>
>> Sure. I don't think it has an impact on the serie anyway so we can keep
>> it for later.
> 
> Ok, I modified 1/10 to leave rfi in for 64-bit cpus for now, and
> applied the full series to ppc-for-2.7.  I've also now sent a pull
> request including this.

Perfect. I was wondering if we should not just remove the instruction from 
the 64bit set.

Thanks,

C.

Re: [Qemu-devel] [PATCH v2 01/10] ppc: Fix rfi/rfid/hrfi/... emulation

2016-06-23 Thread David Gibson 
On Wed, Jun 22, 2016 at 08:48:14AM +0200, Cédric Le Goater wrote:
> On 06/22/2016 04:46 AM, David Gibson wrote:
> > I'm not comfortable merging this until the openbios change is pulled
> > back into the qemu tree (submodule and pre-built binary).
> > 
> > Again - sure you don't want to apply this with rfi still enabled for
> > 64-bit for now, letting the rest of this series go in as well, then
> > clean up the rfi/64 behaviour later?
> 
> Sure. I don't think it has an impact on the serie anyway so we can keep
> it for later.

Ok, I modified 1/10 to leave rfi in for 64-bit cpus for now, and
applied the full series to ppc-for-2.7.  I've also now sent a pull
request including this.

-- 
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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 01/10] ppc: Fix rfi/rfid/hrfi/... emulation

2016-06-22 Thread Cédric Le Goater 
On 06/22/2016 04:46 AM, David Gibson wrote:
> I'm not comfortable merging this until the openbios change is pulled
> back into the qemu tree (submodule and pre-built binary).
> 
> Again - sure you don't want to apply this with rfi still enabled for
> 64-bit for now, letting the rest of this series go in as well, then
> clean up the rfi/64 behaviour later?

Sure. I don't think it has an impact on the serie anyway so we can keep
it for later.

Thanks,

C.

Re: [Qemu-devel] [PATCH v2 01/10] ppc: Fix rfi/rfid/hrfi/... emulation

2016-06-21 Thread David Gibson 
On Tue, Jun 21, 2016 at 11:48:46PM +0200, Cédric Le Goater wrote:
> From: Benjamin Herrenschmidt 
> 
> This reworks emulation of the various "rfi" variants. I removed
> some masking bits that I couldn't make sense of, the only bit that
> I am aware we should mask here is POW, the CPU's MSR mask should
> take care of the rest.
> 
> This also fixes some problems when running 32-bit userspace under
> a 64-bit kernel.
> 
> This patch broke 32bit OpenBIOS when run under a 970 cpu. A fix was
> proposed here :
> 
> https://www.coreboot.org/pipermail/openbios/2016-June/009452.html
> 
> Signed-off-by: Benjamin Herrenschmidt 
> Reviewed-by: David Gibson 
> [clg: updated the commit log with the reference of the openbios fix ]
> Signed-off-by: Cédric Le Goater 

I'm not comfortable merging this until the openbios change is pulled
back into the qemu tree (submodule and pre-built binary).

Again - sure you don't want to apply this with rfi still enabled for
64-bit for now, letting the rest of this series go in as well, then
clean up the rfi/64 behaviour later?

> ---
> 
>  target-ppc/excp_helper.c | 51 
> +++-
>  target-ppc/translate.c   |  8 
>  2 files changed, 28 insertions(+), 31 deletions(-)
> 
> diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
> index 30e960e30b63..aa0b63f4b0de 100644
> --- a/target-ppc/excp_helper.c
> +++ b/target-ppc/excp_helper.c
> @@ -922,25 +922,20 @@ void helper_store_msr(CPUPPCState *env, target_ulong 
> val)
>  }
>  }
>  
> -static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong 
> msr,
> -  target_ulong msrm, int keep_msrh)
> +static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong 
> msr)
>  {
>  CPUState *cs = CPU(ppc_env_get_cpu(env));
>  
> +/* MSR:POW cannot be set by any form of rfi */
> +msr &= ~(1ULL << MSR_POW);
> +
>  #if defined(TARGET_PPC64)
> -if (msr_is_64bit(env, msr)) {
> -nip = (uint64_t)nip;
> -msr &= (uint64_t)msrm;
> -} else {
> +/* Switching to 32-bit ? Crop the nip */
> +if (!msr_is_64bit(env, msr)) {
>  nip = (uint32_t)nip;
> -msr = (uint32_t)(msr & msrm);
> -if (keep_msrh) {
> -msr |= env->msr & ~((uint64_t)0x);
> -}
>  }
>  #else
>  nip = (uint32_t)nip;
> -msr &= (uint32_t)msrm;
>  #endif
>  /* XXX: beware: this is false if VLE is supported */
>  env->nip = nip & ~((target_ulong)0x0003);
> @@ -959,26 +954,24 @@ static inline void do_rfi(CPUPPCState *env, 
> target_ulong nip, target_ulong msr,
>  
>  void helper_rfi(CPUPPCState *env)
>  {
> -if (env->excp_model == POWERPC_EXCP_BOOKE) {
> -do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1],
> -   ~((target_ulong)0), 0);
> -} else {
> -do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1],
> -   ~((target_ulong)0x783F), 1);
> -}
> +do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1] & 0xul);
>  }
>  
> +#define MSR_BOOK3S_MASK
>  #if defined(TARGET_PPC64)
>  void helper_rfid(CPUPPCState *env)
>  {
> -do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1],
> -   ~((target_ulong)0x783F), 0);
> +/* The architeture defines a number of rules for which bits
> + * can change but in practice, we handle this in hreg_store_msr()
> + * which will be called by do_rfi(), so there is no need to filter
> + * here
> + */
> +do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1]);
>  }
>  
>  void helper_hrfid(CPUPPCState *env)
>  {
> -do_rfi(env, env->spr[SPR_HSRR0], env->spr[SPR_HSRR1],
> -   ~((target_ulong)0x783F), 0);
> +do_rfi(env, env->spr[SPR_HSRR0], env->spr[SPR_HSRR1]);
>  }
>  #endif
>  
> @@ -986,28 +979,24 @@ void helper_hrfid(CPUPPCState *env)
>  /* Embedded PowerPC specific helpers */
>  void helper_40x_rfci(CPUPPCState *env)
>  {
> -do_rfi(env, env->spr[SPR_40x_SRR2], env->spr[SPR_40x_SRR3],
> -   ~((target_ulong)0x), 0);
> +do_rfi(env, env->spr[SPR_40x_SRR2], env->spr[SPR_40x_SRR3]);
>  }
>  
>  void helper_rfci(CPUPPCState *env)
>  {
> -do_rfi(env, env->spr[SPR_BOOKE_CSRR0], env->spr[SPR_BOOKE_CSRR1],
> -   ~((target_ulong)0), 0);
> +do_rfi(env, env->spr[SPR_BOOKE_CSRR0], env->spr[SPR_BOOKE_CSRR1]);
>  }
>  
>  void helper_rfdi(CPUPPCState *env)
>  {
>  /* FIXME: choose CSRR1 or DSRR1 based on cpu type */
> -do_rfi(env, env->spr[SPR_BOOKE_DSRR0], env->spr[SPR_BOOKE_DSRR1],
> -   ~((target_ulong)0), 0);
> +do_rfi(env, env->spr[SPR_BOOKE_DSRR0], env->spr[SPR_BOOKE_DSRR1]);
>  }
>  
>  void helper_rfmci(CPUPPCState *env)
>  {
>  /* FIXME: choose CSRR1 or MCSRR1 based on cpu type */
> -do_rfi(env, env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSRR1],
> -   ~((target_ulong)0), 0);

[Qemu-devel] [PATCH v2 01/10] ppc: Fix rfi/rfid/hrfi/... emulation

2016-06-21 Thread Cédric Le Goater 
From: Benjamin Herrenschmidt 

This reworks emulation of the various "rfi" variants. I removed
some masking bits that I couldn't make sense of, the only bit that
I am aware we should mask here is POW, the CPU's MSR mask should
take care of the rest.

This also fixes some problems when running 32-bit userspace under
a 64-bit kernel.

This patch broke 32bit OpenBIOS when run under a 970 cpu. A fix was
proposed here :

https://www.coreboot.org/pipermail/openbios/2016-June/009452.html

Signed-off-by: Benjamin Herrenschmidt 
Reviewed-by: David Gibson 
[clg: updated the commit log with the reference of the openbios fix ]
Signed-off-by: Cédric Le Goater 
---

 target-ppc/excp_helper.c | 51 +++-
 target-ppc/translate.c   |  8 
 2 files changed, 28 insertions(+), 31 deletions(-)

diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
index 30e960e30b63..aa0b63f4b0de 100644
--- a/target-ppc/excp_helper.c
+++ b/target-ppc/excp_helper.c
@@ -922,25 +922,20 @@ void helper_store_msr(CPUPPCState *env, target_ulong val)
 }
 }
 
-static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr,
-  target_ulong msrm, int keep_msrh)
+static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr)
 {
 CPUState *cs = CPU(ppc_env_get_cpu(env));
 
+/* MSR:POW cannot be set by any form of rfi */
+msr &= ~(1ULL << MSR_POW);
+
 #if defined(TARGET_PPC64)
-if (msr_is_64bit(env, msr)) {
-nip = (uint64_t)nip;
-msr &= (uint64_t)msrm;
-} else {
+/* Switching to 32-bit ? Crop the nip */
+if (!msr_is_64bit(env, msr)) {
 nip = (uint32_t)nip;
-msr = (uint32_t)(msr & msrm);
-if (keep_msrh) {
-msr |= env->msr & ~((uint64_t)0x);
-}
 }
 #else
 nip = (uint32_t)nip;
-msr &= (uint32_t)msrm;
 #endif
 /* XXX: beware: this is false if VLE is supported */
 env->nip = nip & ~((target_ulong)0x0003);
@@ -959,26 +954,24 @@ static inline void do_rfi(CPUPPCState *env, target_ulong 
nip, target_ulong msr,
 
 void helper_rfi(CPUPPCState *env)
 {
-if (env->excp_model == POWERPC_EXCP_BOOKE) {
-do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1],
-   ~((target_ulong)0), 0);
-} else {
-do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1],
-   ~((target_ulong)0x783F), 1);
-}
+do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1] & 0xul);
 }
 
+#define MSR_BOOK3S_MASK
 #if defined(TARGET_PPC64)
 void helper_rfid(CPUPPCState *env)
 {
-do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1],
-   ~((target_ulong)0x783F), 0);
+/* The architeture defines a number of rules for which bits
+ * can change but in practice, we handle this in hreg_store_msr()
+ * which will be called by do_rfi(), so there is no need to filter
+ * here
+ */
+do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1]);
 }
 
 void helper_hrfid(CPUPPCState *env)
 {
-do_rfi(env, env->spr[SPR_HSRR0], env->spr[SPR_HSRR1],
-   ~((target_ulong)0x783F), 0);
+do_rfi(env, env->spr[SPR_HSRR0], env->spr[SPR_HSRR1]);
 }
 #endif
 
@@ -986,28 +979,24 @@ void helper_hrfid(CPUPPCState *env)
 /* Embedded PowerPC specific helpers */
 void helper_40x_rfci(CPUPPCState *env)
 {
-do_rfi(env, env->spr[SPR_40x_SRR2], env->spr[SPR_40x_SRR3],
-   ~((target_ulong)0x), 0);
+do_rfi(env, env->spr[SPR_40x_SRR2], env->spr[SPR_40x_SRR3]);
 }
 
 void helper_rfci(CPUPPCState *env)
 {
-do_rfi(env, env->spr[SPR_BOOKE_CSRR0], env->spr[SPR_BOOKE_CSRR1],
-   ~((target_ulong)0), 0);
+do_rfi(env, env->spr[SPR_BOOKE_CSRR0], env->spr[SPR_BOOKE_CSRR1]);
 }
 
 void helper_rfdi(CPUPPCState *env)
 {
 /* FIXME: choose CSRR1 or DSRR1 based on cpu type */
-do_rfi(env, env->spr[SPR_BOOKE_DSRR0], env->spr[SPR_BOOKE_DSRR1],
-   ~((target_ulong)0), 0);
+do_rfi(env, env->spr[SPR_BOOKE_DSRR0], env->spr[SPR_BOOKE_DSRR1]);
 }
 
 void helper_rfmci(CPUPPCState *env)
 {
 /* FIXME: choose CSRR1 or MCSRR1 based on cpu type */
-do_rfi(env, env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSRR1],
-   ~((target_ulong)0), 0);
+do_rfi(env, env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSRR1]);
 }
 #endif
 
@@ -1045,7 +1034,7 @@ void helper_td(CPUPPCState *env, target_ulong arg1, 
target_ulong arg2,
 
 void helper_rfsvc(CPUPPCState *env)
 {
-do_rfi(env, env->lr, env->ctr, 0x, 0);
+do_rfi(env, env->lr, env->ctr & 0x);
 }
 
 /* Embedded.Processor Control */
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index b6894751e8df..81481955a589 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -4087,6 +4087,14 @@ static void gen_rfi(DisasContext *ctx)
 #if defined(CONFIG_USER_ONLY)