Date: Tue, 17 Jun 2014 20:58:27 +0200 From: Michael Tuexen <tuexen@freebsd.org> To: Mark R V Murray <markm@FreeBSD.org> Cc: Hans Petter Selasky <hps@selasky.org>, svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r266083 - in head/sys/arm: arm include Message-ID: <CDC9896A-FE7A-47EA-B8D0-EE34EFA0D6F4@freebsd.org> In-Reply-To: <7124948D-6F58-4E01-AA61-E381AC223F39@FreeBSD.org> References: <201405141911.s4EJBFZZ097826@svn.freebsd.org> <537D0952.2040001@selasky.org> <7610C8E6-3F01-4317-BC1A-67645A162CD7@FreeBSD.org> <53871493.2010502@selasky.org> <EE7E19AA-BE18-4740-A636-4DA3023A5392@FreeBSD.org> <9412A358-EBCB-4A5A-B728-2A15C50FC217@fh-muenster.de> <D118F546-0F32-429F-86F2-69BEE0EC2D2F@FreeBSD.org> <CC36082E-1DFD-4576-9653-970F93A3699C@fh-muenster.de> <FD5DF228-1B2B-48F9-8445-54BF5959FEDA@FreeBSD.org> <9B438991-C970-46A7-8116-A490E02D7139@fh-muenster.de> <3841D090-5973-40B0-B61C-F15E8C1978C9@FreeBSD.org> <3664DBD3-7CEA-43E8-9757-E835BB21FE9E@freebsd.org> <5F36DFA5-C5B1-4E5D-88EE-9D60240B40FD@FreeBSD.org> <494BE7DA-26BC-43A7-8BC0-ECAE8A75E1EF@freebsd.org> <6268E444-F6C7-4B8B-86B7-244A2BB3C4DE@FreeBSD.org> <44F7EEB4-46D1-49C7-A4CA-4EA5D41BDF31@freebsd.org> <C2B493ED-DF51-4EC4-9655-EB14499E44C2@FreeBSD.org> <1F3FE585-4C04-499E-86CE-BA4DC3F05D97@freebsd.org> <7124948D-6F58-4E01-AA61-E381AC223F39@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 17 Jun 2014, at 20:33, Mark R V Murray <markm@FreeBSD.org> wrote:
> Hi Michael,
>
> If that works, then Brilliant! :-) :-)
>
> Could you please fix this so the _PMC_USER_READ_WRITE_ is all in one place (it’s too dangerous to split up), and put an architecture-specific #ifdef around just the MCR instruction we care about.
So you want something like:
Index: cpufunc.c
===================================================================
--- cpufunc.c (revision 267575)
+++ cpufunc.c (working copy)
@@ -1410,12 +1410,27 @@
* you want!
*/
#ifdef _PMC_USER_READ_WRITE_
+#if defined(CPU_ARM1136) || defined(CPU_ARM1176)
+ /* Use the Secure User and Non-secure Access Validation Control Register
+ * to allow userland access
+ */
+ __asm volatile ("mcr p15, 0, %0, c15, c9, 0\n\t"
+ :
+ : "r"(0x00000001));
+#else
/* Set PMUSERENR[0] to allow userland access */
__asm volatile ("mcr p15, 0, %0, c9, c14, 0\n\t"
:
: "r"(0x00000001));
#endif
- /* Set up the PMCCNTR register as a cyclecounter:
+#endif
+#if defined(CPU_ARM1136) || defined(CPU_ARM1176)
+ /* Set PMCR[2,0] to enable counters and reset CCNT */
+ __asm volatile ("mcr p15, 0, %0, c15, c12, 0\n\t"
+ :
+ : "r"(0x00000005));
+#else
+ /* Set up the PMCCNTR register as a cyclecounter:
* Set PMINTENCLR to 0xFFFFFFFF to block interrupts
* Set PMCR[2,0] to enable counters and reset CCNT
* Set PMCNTENSET to 0x80000000 to enable CCNT */
@@ -1426,6 +1441,7 @@
: "r"(0xFFFFFFFF),
"r"(0x00000005),
"r"(0x80000000));
+#endif
}
#endif
>
> With that, its good to commit, I’d say. I’ll likely follow up and exclude the architectures that are unproven.
You can commit this and your patch to include/cpu.h. I think it would be good to
have both fixes in one commit...
Best regards
Michael
>
> M
>
> On 17 Jun 2014, at 10:52, Michael Tuexen <tuexen@freebsd.org> wrote:
>
>> On 16 Jun 2014, at 22:13, Mark R V Murray <markm@FreeBSD.org> wrote:
>> Hi Mark,
>>
>> I just adopted the comments to the code change. So here is the improved patch:
>>
>> Index: cpufunc.c
>> ===================================================================
>> --- cpufunc.c (revision 267575)
>> +++ cpufunc.c (working copy)
>> @@ -1404,18 +1404,36 @@
>> static __inline void
>> cpu_scc_setup_ccnt(void)
>> {
>> -/* This is how you give userland access to the CCNT and PMCn
>> - * registers.
>> - * BEWARE! This gives write access also, which may not be what
>> - * you want!
>> - */
>> +#if defined(CPU_ARM1136) || defined(CPU_ARM1176)
>> #ifdef _PMC_USER_READ_WRITE_
>> - /* Set PMUSERENR[0] to allow userland access */
>> + /* This is how you give userland access to the CCNT and PMCn
>> + * registers.
>> + * BEWARE! This gives write access also, which may not be what
>> + * you want!
>> + * Use the Secure User and Non-secure Access Validation Control Register
>> + * to allow userland access
>> + */
>> + __asm volatile ("mcr p15, 0, %0, c15, c9, 0\n\t"
>> + :
>> + : "r"(0x00000001));
>> +#endif
>> + /* Set PMCR[2,0] to enable counters and reset CCNT */
>> + __asm volatile ("mcr p15, 0, %0, c15, c12, 0\n\t"
>> + :
>> + : "r"(0x00000005));
>> +#else
>> +#ifdef _PMC_USER_READ_WRITE_
>> + /* This is how you give userland access to the CCNT and PMCn
>> + * registers.
>> + * BEWARE! This gives write access also, which may not be what
>> + * you want!
>> + * Set PMUSERENR[0] to allow userland access
>> + */
>> __asm volatile ("mcr p15, 0, %0, c9, c14, 0\n\t"
>> :
>> : "r"(0x00000001));
>> #endif
>> - /* Set up the PMCCNTR register as a cyclecounter:
>> + /* Set up the PMCCNTR register as a cyclecounter:
>> * Set PMINTENCLR to 0xFFFFFFFF to block interrupts
>> * Set PMCR[2,0] to enable counters and reset CCNT
>> * Set PMCNTENSET to 0x80000000 to enable CCNT */
>> @@ -1426,6 +1444,7 @@
>> : "r"(0xFFFFFFFF),
>> "r"(0x00000005),
>> "r"(0x80000000));
>> +#endif
>> }
>> #endif
>>
>> Let me know if I can help.
>>
>> Best regards
>> Michael
>>>
>>> On 16 Jun 2014, at 20:38, Michael Tuexen <tuexen@freebsd.org> wrote:
>>>> Hmm, the documentation reads
>>>
>>> Which docs are you using?
>>>
>>> I’m using DDI0360F. (And that could easily be a wrong choice).
>>>
>>> M
>>> --
>>> Mark R V Murray
>>>
>>>
>>
>
> --
> Mark R V Murray
>
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CDC9896A-FE7A-47EA-B8D0-EE34EFA0D6F4>