Re: [Qemu-devel] [Qemu-ppc] [PATCH 07/12] ppc: Better figure out if processor has HV mode

["Cédric Le Goater" <clg@kaod.org>]

On 05/28/2016 11:52 AM, David Gibson wrote:
> On Fri, May 27, 2016 at 09:56:32AM +0200, Cédric Le Goater wrote:
>> On 05/27/2016 05:38 AM, David Gibson wrote:
>>> On Tue, May 03, 2016 at 06:03:29PM +0200, Cédric Le Goater wrote:
>>>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>>>
>>>> We use an env. flag which is set to the initial value of MSR_HVB in
>>>> the msr_mask. We also adjust the POWER8 mask to set SHV.
>>>>
>>>> Also use this to adjust ctx.hv so that it is *set* when the processor
>>>> doesn't have an HV mode (970 with Apple mode for example), thus enabling
>>>> hypervisor instructions/SPRs.
>>>>
>>>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>>> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
>>>> ---
>>>>  target-ppc/cpu.h            |  4 ++++
>>>>  target-ppc/translate.c      |  4 +++-
>>>>  target-ppc/translate_init.c | 21 ++++++++++++++++-----
>>>>  3 files changed, 23 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>>>> index 2a96efcbf813..02f2e72e6d14 100644
>>>> --- a/target-ppc/cpu.h
>>>> +++ b/target-ppc/cpu.h
>>>> @@ -1161,6 +1161,10 @@ struct CPUPPCState {
>>>>      hwaddr mpic_iack;
>>>>      /* true when the external proxy facility mode is enabled */
>>>>      bool mpic_proxy;
>>>> +    /* set when the processor has an HV mode, thus HV priv
>>>> +     * instructions and SPRs are diallowed if MSR:HV is 0
>>>> +     */
>>>> +    bool has_hv_mode;
>>>>  #endif
>>>>  
>>>>      /* Those resources are used only during code translation */
>>>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>>>> index 7a672cba796d..6f55bcd34a74 100644
>>>> --- a/target-ppc/translate.c
>>>> +++ b/target-ppc/translate.c
>>>> @@ -11495,8 +11495,10 @@ void gen_intermediate_code(CPUPPCState *env, struct TranslationBlock *tb)
>>>>      ctx.exception = POWERPC_EXCP_NONE;
>>>>      ctx.spr_cb = env->spr_cb;
>>>>      ctx.pr = msr_pr;
>>>> -    ctx.hv = !msr_pr && msr_hv;
>>>
>>> The test for msr_pr has been removed in the new version.  Maybe that's
>>> safe, but I think it needs some justification.
>>
>> I don't know if it is safe to run with (MSR_HV|MSR_PR) = 11
> 
> Um.. I believe host userland runs routinely in that state.

oui oui oui ... of course. hmm, I need to dig more the consequences of 
the *HV* patches. 

Thanks,

C. 

>> There is a note in the Power ISA book describing such a case but I am not 
>> sure qemu-ppc supports that. So I will keep the msr_pr check in the next 
>> version :
>>
>> 	ctx.hv = !msr_pr && (msr_hv || !env->has_hv_mode);
> 
> Ok.
>