| Message ID | 1398845480-9945-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> 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? > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> > --- > 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? Mikey > }; > > static void __init scan_features(unsigned long node, unsigned char *ftrs, > -- > 1.9.1 > > _______________________________________________ > Linuxppc-dev mailing list > Linuxppc-dev@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/linuxppc-dev
Michael Neuling <mikey@neuling.org> writes: > Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> 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 <aneesh.kumar@linux.vnet.ibm.com> >> --- >> 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 ? -aneesh
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> writes: > Michael Neuling <mikey@neuling.org> writes: > >> Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> 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 <aneesh.kumar@linux.vnet.ibm.com> >>> --- >>> 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
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> writes: > "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> writes: > >> Michael Neuling <mikey@neuling.org> writes: >> >>> Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> 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 <aneesh.kumar@linux.vnet.ibm.com> >>>> --- >>>> 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. This will actually result in illegal instruction. -aneesh
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}, }; static void __init scan_features(unsigned long node, unsigned char *ftrs,
Runtime disable transactional memory feature looking at pa-features device tree entry. This provides a mechanism to disable TM on P8 systems. Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> --- arch/powerpc/kernel/prom.c | 5 +++++ 1 file changed, 5 insertions(+)