| 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
| ||
|
Date: Mon, 24 Jun 2024 13:39:28 +0200
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Jason-JH Lin (林睿祥) <Jason-JH.Lin@...iatek.com>,
"jassisinghbrar@...il.com" <jassisinghbrar@...il.com>,
"matthias.bgg@...il.com" <matthias.bgg@...il.com>,
"robh+dt@...nel.org" <robh+dt@...nel.org>,
"krzysztof.kozlowski+dt@...aro.org" <krzysztof.kozlowski+dt@...aro.org>,
"chunkuang.hu@...nel.org" <chunkuang.hu@...nel.org>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Singo Chang (張興國) <Singo.Chang@...iatek.com>,
"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
Jason-ch Chen (陳建豪)
<Jason-ch.Chen@...iatek.com>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
Shawn Sung (宋孝謙) <Shawn.Sung@...iatek.com>,
Nancy Lin (林欣螢) <Nancy.Lin@...iatek.com>,
"conor+dt@...nel.org" <conor+dt@...nel.org>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.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>
Subject: Re: [PATCH v6 3/8] soc: mediatek: cmdq: Add cmdq_pkt_logic_command to
support math operation
Il 28/05/24 17:52, Jason-JH Lin (林睿祥) ha scritto:
> Hi Angelo,
>
> On Tue, 2024-05-28 at 12:24 +0200, AngeloGioacchino Del Regno wrote:
>> Il 26/05/24 01:08, Jason-JH.Lin ha scritto:
>>> Add cmdq_pkt_logic_command to support math operation.
>>>
>>> cmdq_pkt_logic_command can append logic command to the CMDQ packet,
>>> ask GCE to execute a arithmetic calculate instruction,
>>> such as add, subtract, multiply, AND, OR and NOT, etc.
>>>
>>> Note that all arithmetic instructions are unsigned calculations.
>>> 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>
>>> Signed-off-by: Hsiao Chien Sung <shawn.sung@...iatek.com>
>>> ---
>>> drivers/soc/mediatek/mtk-cmdq-helper.c | 36
>>> ++++++++++++++++++++++
>>> include/linux/soc/mediatek/mtk-cmdq.h | 42
>>> ++++++++++++++++++++++++++
>>> 2 files changed, 78 insertions(+)
>>>
>>> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> b/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> index 046522664dc1..42fae05f61a8 100644
>>> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> @@ -15,10 +15,19 @@
>>> /* dedicate the last GPR_R15 to assign the register address to be
>>> poll */
>>> #define CMDQ_POLL_ADDR_GPR (15)
>>> #define CMDQ_EOC_IRQ_EN BIT(0)
>>> +#define CMDQ_IMMEDIATE_VALUE 0
>>> #define CMDQ_REG_TYPE 1
>>> #define CMDQ_JUMP_RELATIVE 0
>>> #define CMDQ_JUMP_ABSOLUTE 1
>>>
>>> +#define CMDQ_OPERAND_GET_IDX_VALUE(operand) \
>>> + ({ \
>>> + struct cmdq_operand *op = operand; \
>>> + op->reg ? op->idx : op->value; \
>>> + })
>>
>> I think this CMDQ_OPERAND_GET_IDX_VALUE would be better expressed as
>> a (inline?)
>> function instead...
>>
>
> Yes, I can change them to static inline function to pass their type
> directly.
>
>> static inline u16 cmdq_operand_get_idx_value(struct cmdq_operand *op)
>> {
>> return op->reg ? op->idx : op->value;
>> }
>>
>> and maybe the same for the other definition too..
>>
>> static inline u16 cmdq_operand_type(struct cmdq_operand *op)
>> {
>> return op->reg ? CMDQ_REG_TYPE : CMDQ_IMMEDIATE_VALUE;
>> }
>>
>> but definitely the first one is what I don't like much, the second
>> one can pass.
>>
>
> You mean '#define CMDQ_OPERAND_GET_IDX_VALUE()' is the first one or
> 'static inline u16 cmdq_operand_get_idx_value()' is the first one?
>
I'm sorry, your reply slipped through for some reason and I've read it just now.
I mean I don't like the macros (#define CMDQ_OPERAND_GET_IDX_VALUE(..) and
CMDQ_OPERAND_TYPE(..)) :-)
Please use static inline functions and resend; if you can do that this week, I
can pick the commit before I send the PR.
P.S.: And you're right, re-reading my own reply, it was a bit ambiguous, so
I'm again sorry for that, will be clearer next time.
Thanks,
Angelo
> Regards,
> Jason-JH.Lin
>
>> Cheers,
>> Angelo
>>
>>> +#define CMDQ_OPERAND_TYPE(operand) \
>>> + ((operand)->reg ? CMDQ_REG_TYPE : CMDQ_IMMEDIATE_VALUE)
>>> +
>>> struct cmdq_instruction {
>>> union {
>>> u32 value;
>>> @@ -461,6 +470,33 @@ int cmdq_pkt_poll_addr(struct cmdq_pkt *pkt,
>>> dma_addr_t addr, u32 value, u32 mas
>>> }
>>> EXPORT_SYMBOL(cmdq_pkt_poll_addr);
>>>
>>> +int cmdq_pkt_logic_command(struct cmdq_pkt *pkt, u16
>>> result_reg_idx,
>>> + struct cmdq_operand *left_operand,
>>> + enum cmdq_logic_op s_op,
>>> + struct cmdq_operand *right_operand)
>>> +{
>>> + struct cmdq_instruction inst = { {0} };
>>> + u32 left_idx_value;
>>> + u32 right_idx_value;
>>> +
>>> + if (!left_operand || !right_operand || s_op >= CMDQ_LOGIC_MAX)
>>> + 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.reg_dst = result_reg_idx;
>>> + inst.src_reg = left_idx_value;
>>> + inst.arg_c = right_idx_value;
>>> +
>>> + 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 d4a8e34505e6..5bee6f7fc400 100644
>>> --- a/include/linux/soc/mediatek/mtk-cmdq.h
>>> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
>>> @@ -25,6 +25,31 @@
>>>
>>> struct cmdq_pkt;
>>>
>>> +enum cmdq_logic_op {
>>> + CMDQ_LOGIC_ASSIGN = 0,
>>> + CMDQ_LOGIC_ADD = 1,
>>> + CMDQ_LOGIC_SUBTRACT = 2,
>>> + CMDQ_LOGIC_MULTIPLY = 3,
>>> + CMDQ_LOGIC_XOR = 8,
>>> + CMDQ_LOGIC_NOT = 9,
>>> + CMDQ_LOGIC_OR = 10,
>>> + CMDQ_LOGIC_AND = 11,
>>> + CMDQ_LOGIC_LEFT_SHIFT = 12,
>>> + CMDQ_LOGIC_RIGHT_SHIFT = 13,
>>> + CMDQ_LOGIC_MAX,
>>> +};
>>> +
>>> +struct cmdq_operand {
>>> + /* register type */
>>> + bool reg;
>>> + union {
>>> + /* index */
>>> + u16 idx;
>>> + /* value */
>>> + u16 value;
>>> + };
>>> +};
>>> +
>>> struct cmdq_client_reg {
>>> u8 subsys;
>>> u16 offset;
>>> @@ -272,6 +297,23 @@ int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8
>>> subsys,
>>> int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
>>> u16 offset, u32 value, u32 mask);
>>>
>>> +/**
>>> + * cmdq_pkt_logic_command() - Append logic command to the CMDQ
>>> packet, ask GCE to
>>> + * execute an instruction that store the result
>>> of logic operation
>>> + * with left and right operand into
>>> result_reg_idx.
>>> + * @pkt: the CMDQ packet
>>> + * @result_reg_idx: SPR index that store operation result
>>> of left_operand and right_operand
>>> + * @left_operand: left operand
>>> + * @s_op: the logic operator enum
>>> + * @right_operand: right operand
>>> + *
>>> + * Return: 0 for success; else the error code is returned
>>> + */
>>> +int cmdq_pkt_logic_command(struct cmdq_pkt *pkt, u16
>>> result_reg_idx,
>>> + struct cmdq_operand *left_operand,
>>> + enum cmdq_logic_op s_op,
>>> + struct cmdq_operand *right_operand);
>>> +
>>> /**
>>> * cmdq_pkt_assign() - Append logic assign command to the CMDQ
>>> packet, ask GCE
>>> * to execute an instruction that set a
>>> constant value into
>>
>>
>>
>>
Powered by blists - more mailing lists