Subject: Re: [PATCH] PPC: BOOK3S: Disable/Enable TM looking at the ibm, pa-features device tree entry

From: "Aneesh Kumar K.V" <[email-masked]>
Date: Fri, 02 May 2014 10:47:36 +0530

"Aneesh Kumar K.V" <[email-masked]> writes:

> Michael Neuling <[email-masked]> writes:
>
>> Aneesh Kumar K.V <[email-masked]> wrote:
>>
>>> Runtime disable transactional memory feature looking at pa-features
>>> device tree entry. This provides a mechanism to disable TM on P8
>>> systems.
>>
>> What are we actually achieving with this?
>
> PAPR compliance  :) ? Also I wanted to disable guest kernel from doing
> TM related save restore. Guest kernel already look at the cpu feature
> before doing that. Hence needed a mechanism to disable the feature. 
>
> Things like
>
> static inline void __switch_to_tm(struct task_struct *prev)
> {
> 	if (cpu_has_feature(CPU_FTR_TM)) {
> 		tm_enable();
> 		tm_reclaim_task(prev);
> 	}
> }
>
>
>>
>>> Signed-off-by: Aneesh Kumar K.V <[email-masked]>
>>> ---
>>>  arch/powerpc/kernel/prom.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>> 
>>> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
>>> index 668aa4791fd7..537bd7e7db0b 100644
>>> --- a/arch/powerpc/kernel/prom.c
>>> +++ b/arch/powerpc/kernel/prom.c
>>> @@ -161,6 +161,11 @@ static struct ibm_pa_feature {
>>>  	{CPU_FTR_NODSISRALIGN, 0, 0,	1, 1, 1},
>>>  	{0, MMU_FTR_CI_LARGE_PAGE, 0,	1, 2, 0},
>>>  	{CPU_FTR_REAL_LE, PPC_FEATURE_TRUE_LE, 5, 0, 0},
>>> +	/*
>>> +	 * We should use CPU_FTR_TM_COMP so that if we disable TM, it won't get
>>> +	 * enabled via device tree
>>> +	 */
>>> +	{CPU_FTR_TM_COMP, 0, 0,		22, 0, 0},
>>
>> What does this do to guests?  Will it turn TM unavailable into an
>> illegal instruction?
>>
>
> Good suggestion. I guess it should be facility unavailable interrupt ?
> I should also make the sure __init_HFSCR only set HFSCR_TM only if the
> cpu feature is enabled ?

I looked at this and I guess we don't need to update HFSCR considering
that the guest kernel (privileged) access to TM always happen within
if (cpu_has_feature(CPU_FTR_TM)) conditional block. Also we want to
disable this per guest and there is no easy way to suggest hypervisor
that disable TM in HFSCR.

BTW we already do this for guest problme state. We do in guest kernel

	if (cpu_has_feature(CPU_FTR_TM))
		regs->msr |= MSR_TM;

IIUC that should result in facility unavailable interrupt when problem
state try to access TM ?

I will try to run some test with the patch and update here.

-aneesh