| 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: <72c43ee3-0b1e-42be-a936-8389f9954242@amd.com>
Date: Tue, 10 Dec 2024 18:20:10 -0600
From: Mario Limonciello <mario.limonciello@....com>
To: Lizhi Hou <lizhi.hou@....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/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/
> 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?
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