diff mbox series

ARC: Add a knob to control usage of dual-issue

Message ID 20180118134831.41489-1-abrodkin@synopsys.com
State New
Headers show
Series ARC: Add a knob to control usage of dual-issue | expand

Commit Message

Alexey Brodkin Jan. 18, 2018, 1:48 p.m. UTC
HS48 core starts with dual-issue enabled but in some cases like
debugging as well as benchmarking it might be useful to disable
dual-issue for a particular run.

Note:
  1. To disable dual-issue user has to change a value of a global variable
     in target's memory right before start of Linu kernel execution
     (most probably via JTAG)

  2. Disabling happens very early on boot and to get it back enabled it's
     required to restart Linux kernel. I.e. with this change we don't allow
     toggling dual-issue state in random moments of run-time

Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
---
 arch/arc/kernel/setup.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Vineet Gupta Jan. 18, 2018, 6:54 p.m. UTC | #1
On 01/18/2018 05:48 AM, Alexey Brodkin wrote:
> HS48 core starts with dual-issue enabled but in some cases like
> debugging as well as benchmarking it might be useful to disable
> dual-issue for a particular run.
>
> Note:
>    1. To disable dual-issue user has to change a value of a global variable
>       in target's memory right before start of Linu kernel execution
>       (most probably via JTAG)
>
>    2. Disabling happens very early on boot and to get it back enabled it's
>       required to restart Linux kernel. I.e. with this change we don't allow
>       toggling dual-issue state in random moments of run-time

But we need access to a debugger anyways to change this global variable.
If you already have that won't it be better to change the aux register itself from 
the debugger itself.
I don't really see how the global variable way of toggle adds any value here ?

>
> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> ---
>   arch/arc/kernel/setup.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
>
> diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c
> index 9d27331fe69a..cf97f7d88934 100644
> --- a/arch/arc/kernel/setup.c
> +++ b/arch/arc/kernel/setup.c
> @@ -31,6 +31,8 @@
>   
>   #define FIX_PTR(x)  __asm__ __volatile__(";" : "+r"(x))
>   
> +int dual_issue_enable = 1;
> +
>   unsigned int intr_to_DE_cnt;
>   
>   /* Part of U-boot ABI: see head.S */
> @@ -198,6 +200,17 @@ static void read_arc_build_cfg_regs(void)
>   		if (cpu->core.family >= 0x54) {
>   			unsigned int exec_ctrl;
>   
> +			if (!dual_issue_enable) {
> +				/*
> +				 * Note:
> +				 *   1) Reset value in AUX_EXEC_CTRL is 0
> +				 *   2) Reverse logic is used,
> +				 *      i.e. by default (AUX_EXEC_CTRL=0)
> +				 *      dual-issue is enabled.
> +				 */
> +				write_aux_reg(AUX_EXEC_CTRL, 1);
> +			}
> +
>   			READ_BCR(AUX_EXEC_CTRL, exec_ctrl);
>   			cpu->extn.dual_enb = !(exec_ctrl & 1);
>
Alexey Brodkin Jan. 18, 2018, 7:11 p.m. UTC | #2
Hi Vineet,

On Thu, 2018-01-18 at 10:54 -0800, Vineet Gupta wrote:
> On 01/18/2018 05:48 AM, Alexey Brodkin wrote:
> > HS48 core starts with dual-issue enabled but in some cases like
> > debugging as well as benchmarking it might be useful to disable
> > dual-issue for a particular run.
> > 
> > Note:
> >    1. To disable dual-issue user has to change a value of a global variable
> >       in target's memory right before start of Linu kernel execution
> >       (most probably via JTAG)
> > 
> >    2. Disabling happens very early on boot and to get it back enabled it's
> >       required to restart Linux kernel. I.e. with this change we don't allow
> >       toggling dual-issue state in random moments of run-time
> 
> But we need access to a debugger anyways to change this global variable.
> If you already have that won't it be better to change the aux register itself from 
> the debugger itself.
> I don't really see how the global variable way of toggle adds any value here ?

I think there's some sense in a global variable.

1. Not sure if OpenOCD/GDB pair for ARC allows to read/write random AUX regs.
   Probably allows but I guess we'll need a new .xml description which lists EXEC_CTRL
   AUX reg. Correct me if I'm wrong here.

2. If somebody wants to disable dual-issue persistently (here I mean building an image
   which had dual-issue disabled and there's no need to alter anything after image loading)
   then it's possible just to change a simple and very obvious line in arch/arc/kernel/setup.c
   other than trying to guess what could be good place in sources to stick that AUX reg write.
   Note we don't explicitly do reads of AUX_EXEC_CTRL instead we just use READ_BCR macro so
   for newcomers it might not be immediately obvious what to do and where.
   Add the fact that logic of the flag is reversed...

-Alexey
Vineet Gupta Jan. 18, 2018, 10:50 p.m. UTC | #3
On 01/18/2018 11:11 AM, Alexey Brodkin wrote:
> Hi Vineet,
>
> On Thu, 2018-01-18 at 10:54 -0800, Vineet Gupta wrote:
>> On 01/18/2018 05:48 AM, Alexey Brodkin wrote:
>>> HS48 core starts with dual-issue enabled but in some cases like
>>> debugging as well as benchmarking it might be useful to disable
>>> dual-issue for a particular run.
>>>
>>> Note:
>>>     1. To disable dual-issue user has to change a value of a global variable
>>>        in target's memory right before start of Linu kernel execution
>>>        (most probably via JTAG)
>>>
>>>     2. Disabling happens very early on boot and to get it back enabled it's
>>>        required to restart Linux kernel. I.e. with this change we don't allow
>>>        toggling dual-issue state in random moments of run-time
>> But we need access to a debugger anyways to change this global variable.
>> If you already have that won't it be better to change the aux register itself from
>> the debugger itself.
>> I don't really see how the global variable way of toggle adds any value here ?
> I think there's some sense in a global variable.
>
> 1. Not sure if OpenOCD/GDB pair for ARC allows to read/write random AUX regs.
>     Probably allows but I guess we'll need a new .xml description which lists EXEC_CTRL
>     AUX reg. Correct me if I'm wrong here.

Not sure of this a strong argument ...

>
> 2. If somebody wants to disable dual-issue persistently (here I mean building an image
>     which had dual-issue disabled and there's no need to alter anything after image loading)

Ok this is more reasonable !
But can we do this in early boot code then. There's a fair bit of code leading up 
to where we are doing now so I'd prefer we do it just as we do for disabling L1 
caches.
Please note that SLC / IOC are special - as much as I liked to do them super early 
- we can't as there's fair bit of code bloat / mem size etc which we don't knwo 
that early.

>     then it's possible just to change a simple and very obvious line in arch/arc/kernel/setup.c
>     other than trying to guess what could be good place in sources to stick that AUX reg write.
>     Note we don't explicitly do reads of AUX_EXEC_CTRL instead we just use READ_BCR macro so
>     for newcomers it might not be immediately obvious what to do and where.
>     Add the fact that logic of the flag is reversed...
>
> -Alexey
diff mbox series

Patch

diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c
index 9d27331fe69a..cf97f7d88934 100644
--- a/arch/arc/kernel/setup.c
+++ b/arch/arc/kernel/setup.c
@@ -31,6 +31,8 @@ 
 
 #define FIX_PTR(x)  __asm__ __volatile__(";" : "+r"(x))
 
+int dual_issue_enable = 1;
+
 unsigned int intr_to_DE_cnt;
 
 /* Part of U-boot ABI: see head.S */
@@ -198,6 +200,17 @@  static void read_arc_build_cfg_regs(void)
 		if (cpu->core.family >= 0x54) {
 			unsigned int exec_ctrl;
 
+			if (!dual_issue_enable) {
+				/*
+				 * Note:
+				 *   1) Reset value in AUX_EXEC_CTRL is 0
+				 *   2) Reverse logic is used,
+				 *      i.e. by default (AUX_EXEC_CTRL=0)
+				 *      dual-issue is enabled.
+				 */
+				write_aux_reg(AUX_EXEC_CTRL, 1);
+			}
+
 			READ_BCR(AUX_EXEC_CTRL, exec_ctrl);
 			cpu->extn.dual_enb = !(exec_ctrl & 1);