| 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
| ||
|
Message-ID: <ccbade77-5060-2c2c-ab40-80c6456599e2@amd.com>
Date: Tue, 10 Dec 2024 21:32:14 -0800
From: Lizhi Hou <lizhi.hou@....com>
To: Mario Limonciello <mario.limonciello@....com>, <ogabbay@...nel.org>,
<quic_jhugo@...cinc.com>, <dri-devel@...ts.freedesktop.org>
CC: <linux-kernel@...r.kernel.org>, <min.ma@....com>, <max.zhen@....com>,
<sonal.santan@....com>, <king.tam@....com>
Subject: Re: [PATCH V2 7/8] accel/amdxdna: Read firmware interface version
from registers
On 12/10/24 16:20, Mario Limonciello wrote:
> On 12/6/2024 16:00, Lizhi Hou wrote:
>> The latest released firmware supports reading firmware interface version
>> from registers directly. The driver's probe routine reads the major and
>> minor version numbers. If the firmware interface does not compatible
>> with
> s/does/is/
Thanks. I will fix this.
>> the driver, the driver's probe routine returns failure.
>>
>> Co-developed-by: Min Ma <min.ma@....com>
>> Signed-off-by: Min Ma <min.ma@....com>
>> Signed-off-by: Lizhi Hou <lizhi.hou@....com>
>
> Just to confirm you're not backing yourself into a corner the plan is
> not to bump this major version any time soon for anything already
> supported by the driver; right?
That is correct.
Thanks,
Lizhi
>
>
> Because once you do that this is going to get messy quickly.
>
> Reviewed-by: Mario Limonciello <mario.limonciello@....com>
>> ---
>> drivers/accel/amdxdna/aie2_message.c | 26 ----------
>> drivers/accel/amdxdna/aie2_pci.c | 74 ++++++++++++++++++++++------
>> drivers/accel/amdxdna/aie2_pci.h | 6 +--
>> drivers/accel/amdxdna/npu1_regs.c | 2 +-
>> drivers/accel/amdxdna/npu2_regs.c | 2 +-
>> drivers/accel/amdxdna/npu4_regs.c | 2 +-
>> drivers/accel/amdxdna/npu5_regs.c | 2 +-
>> 7 files changed, 64 insertions(+), 50 deletions(-)
>>
>> diff --git a/drivers/accel/amdxdna/aie2_message.c
>> b/drivers/accel/amdxdna/aie2_message.c
>> index 13b5a96f8d25..f6d46e1e5086 100644
>> --- a/drivers/accel/amdxdna/aie2_message.c
>> +++ b/drivers/accel/amdxdna/aie2_message.c
>> @@ -100,32 +100,6 @@ int aie2_get_runtime_cfg(struct amdxdna_dev_hdl
>> *ndev, u32 type, u64 *value)
>> return 0;
>> }
>> -int aie2_check_protocol_version(struct amdxdna_dev_hdl *ndev)
>> -{
>> - DECLARE_AIE2_MSG(protocol_version, MSG_OP_GET_PROTOCOL_VERSION);
>> - struct amdxdna_dev *xdna = ndev->xdna;
>> - int ret;
>> -
>> - ret = aie2_send_mgmt_msg_wait(ndev, &msg);
>> - if (ret) {
>> - XDNA_ERR(xdna, "Failed to get protocol version, ret %d", ret);
>> - return ret;
>> - }
>> -
>> - if (resp.major != ndev->priv->protocol_major) {
>> - XDNA_ERR(xdna, "Incompatible firmware protocol version major
>> %d minor %d",
>> - resp.major, resp.minor);
>> - return -EINVAL;
>> - }
>> -
>> - if (resp.minor < ndev->priv->protocol_minor) {
>> - XDNA_ERR(xdna, "Firmware minor version smaller than
>> supported");
>> - return -EINVAL;
>> - }
>> -
>> - return 0;
>> -}
>> -
>> int aie2_assign_mgmt_pasid(struct amdxdna_dev_hdl *ndev, u16 pasid)
>> {
>> DECLARE_AIE2_MSG(assign_mgmt_pasid, MSG_OP_ASSIGN_MGMT_PASID);
>> diff --git a/drivers/accel/amdxdna/aie2_pci.c
>> b/drivers/accel/amdxdna/aie2_pci.c
>> index 489744a2e226..2d2b6b66617a 100644
>> --- a/drivers/accel/amdxdna/aie2_pci.c
>> +++ b/drivers/accel/amdxdna/aie2_pci.c
>> @@ -33,17 +33,51 @@ MODULE_PARM_DESC(aie2_max_col, "Maximum column
>> could be used");
>> * The related register and ring buffer information is on SRAM BAR.
>> * This struct is the register layout.
>> */
>> +#define MGMT_MBOX_MAGIC 0x55504e5f /* _NPU */
>> struct mgmt_mbox_chann_info {
>> - u32 x2i_tail;
>> - u32 x2i_head;
>> - u32 x2i_buf;
>> - u32 x2i_buf_sz;
>> - u32 i2x_tail;
>> - u32 i2x_head;
>> - u32 i2x_buf;
>> - u32 i2x_buf_sz;
>> + __u32 x2i_tail;
>> + __u32 x2i_head;
>> + __u32 x2i_buf;
>> + __u32 x2i_buf_sz;
>> + __u32 i2x_tail;
>> + __u32 i2x_head;
>> + __u32 i2x_buf;
>> + __u32 i2x_buf_sz;
>> + __u32 magic;
>> + __u32 msi_id;
>> + __u32 prot_major;
>> + __u32 prot_minor;
>> + __u32 rsvd[4];
>> };
>> +static int aie2_check_protocol(struct amdxdna_dev_hdl *ndev, u32
>> fw_major, u32 fw_minor)
>> +{
>> + struct amdxdna_dev *xdna = ndev->xdna;
>> +
>> + /*
>> + * The driver supported mailbox behavior is defined by
>> + * ndev->priv->protocol_major and protocol_minor.
>> + *
>> + * When protocol_major and fw_major are different, it means driver
>> + * and firmware are incompatible.
>> + */
>> + if (ndev->priv->protocol_major != fw_major) {
>> + XDNA_ERR(xdna, "Incompatible firmware protocol major %d
>> minor %d",
>> + fw_major, fw_minor);
>> + return -EINVAL;
>> + }
>> +
>> + /*
>> + * When protocol_minor is greater then fw_minor, that means driver
>> + * relies on operation the installed firmware does not support.
>> + */
>> + if (ndev->priv->protocol_minor > fw_minor) {
>> + XDNA_ERR(xdna, "Firmware minor version smaller than
>> supported");
>> + return -EINVAL;
>> + }
>> + return 0;
>> +}
>> +
>> static void aie2_dump_chann_info_debug(struct amdxdna_dev_hdl *ndev)
>> {
>> struct amdxdna_dev *xdna = ndev->xdna;
>> @@ -57,6 +91,8 @@ static void aie2_dump_chann_info_debug(struct
>> amdxdna_dev_hdl *ndev)
>> XDNA_DBG(xdna, "x2i ringbuf 0x%x", ndev->mgmt_x2i.rb_start_addr);
>> XDNA_DBG(xdna, "x2i rsize 0x%x", ndev->mgmt_x2i.rb_size);
>> XDNA_DBG(xdna, "x2i chann index 0x%x", ndev->mgmt_chan_idx);
>> + XDNA_DBG(xdna, "mailbox protocol major 0x%x",
>> ndev->mgmt_prot_major);
>> + XDNA_DBG(xdna, "mailbox protocol minor 0x%x",
>> ndev->mgmt_prot_minor);
>> }
>> static int aie2_get_mgmt_chann_info(struct amdxdna_dev_hdl *ndev)
>> @@ -87,6 +123,12 @@ static int aie2_get_mgmt_chann_info(struct
>> amdxdna_dev_hdl *ndev)
>> for (i = 0; i < sizeof(info_regs) / sizeof(u32); i++)
>> reg[i] = readl(ndev->sram_base + off + i * sizeof(u32));
>> + if (info_regs.magic != MGMT_MBOX_MAGIC) {
>> + XDNA_ERR(ndev->xdna, "Invalid mbox magic 0x%x",
>> info_regs.magic);
>> + ret = -EINVAL;
>> + goto done;
>> + }
>> +
>> i2x = &ndev->mgmt_i2x;
>> x2i = &ndev->mgmt_x2i;
>> @@ -99,14 +141,20 @@ static int aie2_get_mgmt_chann_info(struct
>> amdxdna_dev_hdl *ndev)
>> x2i->mb_tail_ptr_reg = AIE2_MBOX_OFF(ndev, info_regs.x2i_tail);
>> x2i->rb_start_addr = AIE2_SRAM_OFF(ndev, info_regs.x2i_buf);
>> x2i->rb_size = info_regs.x2i_buf_sz;
>> - ndev->mgmt_chan_idx = CHANN_INDEX(ndev, x2i->rb_start_addr);
>> + ndev->mgmt_chan_idx = info_regs.msi_id;
>> + ndev->mgmt_prot_major = info_regs.prot_major;
>> + ndev->mgmt_prot_minor = info_regs.prot_minor;
>> +
>> + ret = aie2_check_protocol(ndev, ndev->mgmt_prot_major,
>> ndev->mgmt_prot_minor);
>> +
>> +done:
>> aie2_dump_chann_info_debug(ndev);
>> /* Must clear address at FW_ALIVE_OFF */
>> writel(0, SRAM_GET_ADDR(ndev, FW_ALIVE_OFF));
>> - return 0;
>> + return ret;
>> }
>> int aie2_runtime_cfg(struct amdxdna_dev_hdl *ndev,
>> @@ -155,12 +203,6 @@ static int aie2_mgmt_fw_init(struct
>> amdxdna_dev_hdl *ndev)
>> {
>> int ret;
>> - ret = aie2_check_protocol_version(ndev);
>> - if (ret) {
>> - XDNA_ERR(ndev->xdna, "Check header hash failed");
>> - return ret;
>> - }
>> -
>> ret = aie2_runtime_cfg(ndev, AIE2_RT_CFG_INIT, NULL);
>> if (ret) {
>> XDNA_ERR(ndev->xdna, "Runtime config failed");
>> diff --git a/drivers/accel/amdxdna/aie2_pci.h
>> b/drivers/accel/amdxdna/aie2_pci.h
>> index 8c17b74654ce..cc159cadff9f 100644
>> --- a/drivers/accel/amdxdna/aie2_pci.h
>> +++ b/drivers/accel/amdxdna/aie2_pci.h
>> @@ -39,9 +39,6 @@
>> })
>> #define CHAN_SLOT_SZ SZ_8K
>> -#define CHANN_INDEX(ndev, rbuf_off) \
>> - (((rbuf_off) - SRAM_REG_OFF((ndev), MBOX_CHANN_OFF)) /
>> CHAN_SLOT_SZ)
>> -
>> #define MBOX_SIZE(ndev) \
>> ({ \
>> typeof(ndev) _ndev = (ndev); \
>> @@ -170,6 +167,8 @@ struct amdxdna_dev_hdl {
>> struct xdna_mailbox_chann_res mgmt_x2i;
>> struct xdna_mailbox_chann_res mgmt_i2x;
>> u32 mgmt_chan_idx;
>> + u32 mgmt_prot_major;
>> + u32 mgmt_prot_minor;
>> u32 total_col;
>> struct aie_version version;
>> @@ -262,7 +261,6 @@ int aie2_suspend_fw(struct amdxdna_dev_hdl *ndev);
>> int aie2_resume_fw(struct amdxdna_dev_hdl *ndev);
>> int aie2_set_runtime_cfg(struct amdxdna_dev_hdl *ndev, u32 type,
>> u64 value);
>> int aie2_get_runtime_cfg(struct amdxdna_dev_hdl *ndev, u32 type,
>> u64 *value);
>> -int aie2_check_protocol_version(struct amdxdna_dev_hdl *ndev);
>> int aie2_assign_mgmt_pasid(struct amdxdna_dev_hdl *ndev, u16 pasid);
>> int aie2_query_aie_version(struct amdxdna_dev_hdl *ndev, struct
>> aie_version *version);
>> int aie2_query_aie_metadata(struct amdxdna_dev_hdl *ndev, struct
>> aie_metadata *metadata);
>> diff --git a/drivers/accel/amdxdna/npu1_regs.c
>> b/drivers/accel/amdxdna/npu1_regs.c
>> index c8f4d1cac65d..e408af57e378 100644
>> --- a/drivers/accel/amdxdna/npu1_regs.c
>> +++ b/drivers/accel/amdxdna/npu1_regs.c
>> @@ -65,7 +65,7 @@ const struct dpm_clk_freq npu1_dpm_clk_table[] = {
>> const struct amdxdna_dev_priv npu1_dev_priv = {
>> .fw_path = "amdnpu/1502_00/npu.sbin",
>> .protocol_major = 0x5,
>> - .protocol_minor = 0x1,
>> + .protocol_minor = 0x7,
>> .rt_config = npu1_default_rt_cfg,
>> .dpm_clk_tbl = npu1_dpm_clk_table,
>> .col_align = COL_ALIGN_NONE,
>> diff --git a/drivers/accel/amdxdna/npu2_regs.c
>> b/drivers/accel/amdxdna/npu2_regs.c
>> index ac63131f9c7c..286bd0d475e2 100644
>> --- a/drivers/accel/amdxdna/npu2_regs.c
>> +++ b/drivers/accel/amdxdna/npu2_regs.c
>> @@ -64,7 +64,7 @@
>> const struct amdxdna_dev_priv npu2_dev_priv = {
>> .fw_path = "amdnpu/17f0_00/npu.sbin",
>> .protocol_major = 0x6,
>> - .protocol_minor = 0x1,
>> + .protocol_minor = 0x6,
>> .rt_config = npu4_default_rt_cfg,
>> .dpm_clk_tbl = npu4_dpm_clk_table,
>> .col_align = COL_ALIGN_NATURE,
>> diff --git a/drivers/accel/amdxdna/npu4_regs.c
>> b/drivers/accel/amdxdna/npu4_regs.c
>> index a713ac18adfc..00c52833ce89 100644
>> --- a/drivers/accel/amdxdna/npu4_regs.c
>> +++ b/drivers/accel/amdxdna/npu4_regs.c
>> @@ -85,7 +85,7 @@ const struct dpm_clk_freq npu4_dpm_clk_table[] = {
>> const struct amdxdna_dev_priv npu4_dev_priv = {
>> .fw_path = "amdnpu/17f0_10/npu.sbin",
>> .protocol_major = 0x6,
>> - .protocol_minor = 0x1,
>> + .protocol_minor = 12,
>> .rt_config = npu4_default_rt_cfg,
>> .dpm_clk_tbl = npu4_dpm_clk_table,
>> .col_align = COL_ALIGN_NATURE,
>> diff --git a/drivers/accel/amdxdna/npu5_regs.c
>> b/drivers/accel/amdxdna/npu5_regs.c
>> index 67a5d5bc8a49..118849272f27 100644
>> --- a/drivers/accel/amdxdna/npu5_regs.c
>> +++ b/drivers/accel/amdxdna/npu5_regs.c
>> @@ -64,7 +64,7 @@
>> const struct amdxdna_dev_priv npu5_dev_priv = {
>> .fw_path = "amdnpu/17f0_11/npu.sbin",
>> .protocol_major = 0x6,
>> - .protocol_minor = 0x1,
>> + .protocol_minor = 12,
>> .rt_config = npu4_default_rt_cfg,
>> .dpm_clk_tbl = npu4_dpm_clk_table,
>> .col_align = COL_ALIGN_NATURE,
>
Powered by blists - more mailing lists