| Message ID | 1509466547-761-1-git-send-email-anju@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2] powerpc/kernel/sysfs: Export ldbar spr to sysfs | expand |
Anju T Sudhakar <anju@linux.vnet.ibm.com> writes: > Add ldbar spr to sysfs. The spr will hold thread level In-Memory Collection (IMC) > counter configuration data. This is missing any justification for why we would want to expose this, and in particular why we would make it *writable*. cheers > diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c > index 4437c70..f8caee0 100644 > --- a/arch/powerpc/kernel/sysfs.c > +++ b/arch/powerpc/kernel/sysfs.c > @@ -485,6 +485,7 @@ SYSFS_PMCSETUP(mmcra, SPRN_MMCRA); > SYSFS_SPRSETUP(purr, SPRN_PURR); > SYSFS_SPRSETUP(spurr, SPRN_SPURR); > SYSFS_SPRSETUP(pir, SPRN_PIR); > +SYSFS_SPRSETUP(ldbar, SPRN_LDBAR); > > /* > Lets only enable read for phyp resources and > @@ -492,6 +493,7 @@ SYSFS_SPRSETUP(pir, SPRN_PIR); > Lets be conservative and default to pseries. > */ > static DEVICE_ATTR(mmcra, 0600, show_mmcra, store_mmcra); > +static DEVICE_ATTR(ldbar, 0600, show_ldbar, store_ldbar); > static DEVICE_ATTR(spurr, 0400, show_spurr, NULL); > static DEVICE_ATTR(purr, 0400, show_purr, store_purr); > static DEVICE_ATTR(pir, 0400, show_pir, NULL); > @@ -757,6 +759,9 @@ static int register_cpu_online(unsigned int cpu) > device_create_file(s, &pmc_attrs[i]); > > #ifdef CONFIG_PPC64 > + if (cpu_has_feature(CPU_FTR_ARCH_300)) > + device_create_file(s, &dev_attr_ldbar); > + > if (cpu_has_feature(CPU_FTR_MMCRA)) > device_create_file(s, &dev_attr_mmcra); > > @@ -842,6 +847,9 @@ static int unregister_cpu_online(unsigned int cpu) > device_remove_file(s, &pmc_attrs[i]); > > #ifdef CONFIG_PPC64 > + if (cpu_has_feature(CPU_FTR_ARCH_300)) > + device_remove_file(s, &dev_attr_ldbar); > + > if (cpu_has_feature(CPU_FTR_MMCRA)) > device_remove_file(s, &dev_attr_mmcra); > > -- > 2.7.4
Hi mpe, On Wednesday 01 November 2017 06:20 AM, Michael Ellerman wrote: > Anju T Sudhakar <anju@linux.vnet.ibm.com> writes: > >> Add ldbar spr to sysfs. The spr will hold thread level In-Memory Collection (IMC) >> counter configuration data. > This is missing any justification for why we would want to expose this, > and in particular why we would make it *writable*. > > cheers Thank you for reviewing the patch. LDBAR, holds the thread-level counter configuration. Exposing this will help us to understand the current status of thread-level counters in the system. Primarily, Bit 0 of ldbar tells whether the counters are enabled or not. And bit 1 tells the mode (if 0-Accumulation Mode/if 1-Trace Mode). But regarding the permission, you are right. On a reassessment I think that the permission should be read only, because it is possible that we may write an incorrect value to the ldbar, that is wrong. So I will change the permission here. Thanks, Anju > >> diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c >> index 4437c70..f8caee0 100644 >> --- a/arch/powerpc/kernel/sysfs.c >> +++ b/arch/powerpc/kernel/sysfs.c >> @@ -485,6 +485,7 @@ SYSFS_PMCSETUP(mmcra, SPRN_MMCRA); >> SYSFS_SPRSETUP(purr, SPRN_PURR); >> SYSFS_SPRSETUP(spurr, SPRN_SPURR); >> SYSFS_SPRSETUP(pir, SPRN_PIR); >> +SYSFS_SPRSETUP(ldbar, SPRN_LDBAR); >> >> /* >> Lets only enable read for phyp resources and >> @@ -492,6 +493,7 @@ SYSFS_SPRSETUP(pir, SPRN_PIR); >> Lets be conservative and default to pseries. >> */ >> static DEVICE_ATTR(mmcra, 0600, show_mmcra, store_mmcra); >> +static DEVICE_ATTR(ldbar, 0600, show_ldbar, store_ldbar); >> static DEVICE_ATTR(spurr, 0400, show_spurr, NULL); >> static DEVICE_ATTR(purr, 0400, show_purr, store_purr); >> static DEVICE_ATTR(pir, 0400, show_pir, NULL); >> @@ -757,6 +759,9 @@ static int register_cpu_online(unsigned int cpu) >> device_create_file(s, &pmc_attrs[i]); >> >> #ifdef CONFIG_PPC64 >> + if (cpu_has_feature(CPU_FTR_ARCH_300)) >> + device_create_file(s, &dev_attr_ldbar); >> + >> if (cpu_has_feature(CPU_FTR_MMCRA)) >> device_create_file(s, &dev_attr_mmcra); >> >> @@ -842,6 +847,9 @@ static int unregister_cpu_online(unsigned int cpu) >> device_remove_file(s, &pmc_attrs[i]); >> >> #ifdef CONFIG_PPC64 >> + if (cpu_has_feature(CPU_FTR_ARCH_300)) >> + device_remove_file(s, &dev_attr_ldbar); >> + >> if (cpu_has_feature(CPU_FTR_MMCRA)) >> device_remove_file(s, &dev_attr_mmcra); >> >> -- >> 2.7.4
diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c index 4437c70..f8caee0 100644 --- a/arch/powerpc/kernel/sysfs.c +++ b/arch/powerpc/kernel/sysfs.c @@ -485,6 +485,7 @@ SYSFS_PMCSETUP(mmcra, SPRN_MMCRA); SYSFS_SPRSETUP(purr, SPRN_PURR); SYSFS_SPRSETUP(spurr, SPRN_SPURR); SYSFS_SPRSETUP(pir, SPRN_PIR); +SYSFS_SPRSETUP(ldbar, SPRN_LDBAR); /* Lets only enable read for phyp resources and @@ -492,6 +493,7 @@ SYSFS_SPRSETUP(pir, SPRN_PIR); Lets be conservative and default to pseries. */ static DEVICE_ATTR(mmcra, 0600, show_mmcra, store_mmcra); +static DEVICE_ATTR(ldbar, 0600, show_ldbar, store_ldbar); static DEVICE_ATTR(spurr, 0400, show_spurr, NULL); static DEVICE_ATTR(purr, 0400, show_purr, store_purr); static DEVICE_ATTR(pir, 0400, show_pir, NULL); @@ -757,6 +759,9 @@ static int register_cpu_online(unsigned int cpu) device_create_file(s, &pmc_attrs[i]); #ifdef CONFIG_PPC64 + if (cpu_has_feature(CPU_FTR_ARCH_300)) + device_create_file(s, &dev_attr_ldbar); + if (cpu_has_feature(CPU_FTR_MMCRA)) device_create_file(s, &dev_attr_mmcra); @@ -842,6 +847,9 @@ static int unregister_cpu_online(unsigned int cpu) device_remove_file(s, &pmc_attrs[i]); #ifdef CONFIG_PPC64 + if (cpu_has_feature(CPU_FTR_ARCH_300)) + device_remove_file(s, &dev_attr_ldbar); + if (cpu_has_feature(CPU_FTR_MMCRA)) device_remove_file(s, &dev_attr_mmcra);
Add ldbar spr to sysfs. The spr will hold thread level In-Memory Collection (IMC) counter configuration data. Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com> --- arch/powerpc/kernel/sysfs.c | 8 ++++++++ 1 file changed, 8 insertions(+)