lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <5fa0ca0bd4024a5bdb6654fc9b3900377938ed8d.camel@mediatek.com>
Date:   Tue, 24 Oct 2023 16:59:13 +0000
From:   Jason-JH Lin (林睿祥) 
        <Jason-JH.Lin@...iatek.com>
To:     "fshao@...omium.org" <fshao@...omium.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        Singo Chang (張興國) 
        <Singo.Chang@...iatek.com>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        Johnson Wang (王聖鑫) 
        <Johnson.Wang@...iatek.com>,
        "chunkuang.hu@...nel.org" <chunkuang.hu@...nel.org>,
        Jason-ch Chen (陳建豪) 
        <Jason-ch.Chen@...iatek.com>,
        Shawn Sung (宋孝謙) 
        <Shawn.Sung@...iatek.com>,
        Nancy Lin (林欣螢) <Nancy.Lin@...iatek.com>,
        "jkardatzke@...gle.com" <jkardatzke@...gle.com>,
        "conor+dt@...nel.org" <conor+dt@...nel.org>,
        Project_Global_Chrome_Upstream_Group 
        <Project_Global_Chrome_Upstream_Group@...iatek.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "krzysztof.kozlowski+dt@...aro.org" 
        <krzysztof.kozlowski+dt@...aro.org>,
        "matthias.bgg@...il.com" <matthias.bgg@...il.com>,
        "jassisinghbrar@...il.com" <jassisinghbrar@...il.com>,
        "angelogioacchino.delregno@...labora.com" 
        <angelogioacchino.delregno@...labora.com>
Subject: Re: [PATCH v2 3/9] soc: mailbox: Add cmdq_pkt_logic_command to
 support math operation

Hi Fei,

Thanks for the reviews.

On Mon, 2023-10-23 at 16:26 +0800, Fei Shao wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  Hi Jason,
> 
> On Mon, Oct 23, 2023 at 12:39 PM Jason-JH.Lin <
> jason-jh.lin@...iatek.com> wrote:
> >
> > Add cmdq_pkt_logic_command to support match operation.
> s/match/math/
> 
I'll fix this typo. Thanks.

> >
> > cmdq_pkt_logic_command can append logic command to the CMDQ packet,
> > ask GCE to execute a arithematic calculate instruction,
> s/arithematic/arithmetic/
> 
I'll fix this typo. Thanks.

> > such as add, subtract, multiply, AND, OR and NOT, etc.
> >
> > Note that all instructions just accept unsigned calculate.
> s/calculate/calculation/
> 
> 
> Or I'd rephrase it as:
>   Note that all arithmetic instructions are unsigned calculations.
> 
OK, I'll rephrase as yours. Thanks!

> > If there are any overflows, GCE will sent the invalid IRQ to notify
> > CMDQ driver.
> >
> > Signed-off-by: Jason-JH.Lin <jason-jh.lin@...iatek.com>
> > ---
> >  drivers/soc/mediatek/mtk-cmdq-helper.c | 36 ++++++++++++++++++++++
> >  include/linux/soc/mediatek/mtk-cmdq.h  | 41
> ++++++++++++++++++++++++++
> >  2 files changed, 77 insertions(+)
> >
> > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
> b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > index b0cd071c4719..5194d66dfc0a 100644
> > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > @@ -13,9 +13,18 @@
> >  #define CMDQ_WRITE_ENABLE_MASK BIT(0)
> >  #define CMDQ_POLL_ENABLE_MASK  BIT(0)
> >  #define CMDQ_EOC_IRQ_EN                BIT(0)
> > +#define CMDQ_IMMEDIATE_VALUE   0
> >  #define CMDQ_REG_TYPE          1
> >  #define CMDQ_JUMP_RELATIVE     1
> >
> > +#define CMDQ_OPERAND_GET_IDX_VALUE(operand) \
> > +       ({ \
> > +               struct cmdq_operand *op = operand; \
> > +               op->reg ? op->idx : op->value; \
> > +       })
> `((operand)->reg ? (operand)->idx : (operand)->value)` fits in one
> line.
> 

I had use that way, but it will get this CHECK warning:
CHECK: Macro argument reuse '_stat' - possible side-effects?

So I change to current way.

> > +#define CMDQ_OPERAND_TYPE(operand) \
> > +       ((operand)->reg ? CMDQ_REG_TYPE : CMDQ_IMMEDIATE_VALUE)
> > +
> >  struct cmdq_instruction {
> >         union {
> >                 u32 value;
> > @@ -380,6 +389,33 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt,
> u8 subsys,
> >  }
> >  EXPORT_SYMBOL(cmdq_pkt_poll_mask);
> >
> > +int cmdq_pkt_logic_command(struct cmdq_pkt *pkt, enum
> CMDQ_LOGIC_ENUM s_op,
> > +                          u16 result_reg_idx,
> > +                          struct cmdq_operand *left_operand,
> > +                          struct cmdq_operand *right_operand)
> > +{
> > +       struct cmdq_instruction inst = { {0} };
> > +       u32 left_idx_value;
> > +       u32 right_idx_value;
> > +
> > +       if (!left_operand || !right_operand)
> > +               return -EINVAL;
> > +
> > +       left_idx_value = CMDQ_OPERAND_GET_IDX_VALUE(left_operand);
> > +       right_idx_value =
> CMDQ_OPERAND_GET_IDX_VALUE(right_operand);
> > +       inst.op = CMDQ_CODE_LOGIC;
> > +       inst.dst_t = CMDQ_REG_TYPE;
> > +       inst.src_t = CMDQ_OPERAND_TYPE(left_operand);
> > +       inst.arg_c_t = CMDQ_OPERAND_TYPE(right_operand);
> > +       inst.sop = s_op;
> > +       inst.arg_c = right_idx_value;
> > +       inst.src_reg = left_idx_value;
> > +       inst.reg_dst = result_reg_idx;
> > +
> > +       return cmdq_pkt_append_command(pkt, inst);
> > +}
> > +EXPORT_SYMBOL(cmdq_pkt_logic_command);
> > +
> >  int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value)
> >  {
> >         struct cmdq_instruction inst = {};
> > diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
> b/include/linux/soc/mediatek/mtk-cmdq.h
> > index a253c001c861..ea4fadfb5443 100644
> > --- a/include/linux/soc/mediatek/mtk-cmdq.h
> > +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> > @@ -26,6 +26,30 @@
> >
> >  struct cmdq_pkt;
> >
> > +enum CMDQ_LOGIC_ENUM {
> Use lower case, and perhaps use `cmdq_logic_type` or
> `cmdq_operator_type`.
> 
Thanks! I think I would change to `cmdq_logic_op`.

Regards,
Jason-JH.Lin

> Regards,
> Fei

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ