linux-kernel - Re: [PATCH V2 3/4] drivers/fpga/amd: Add remote queue

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3d805a67-aae1-4342-90fd-c7d0dc100b3a@wanadoo.fr>
Date: Sun, 26 Jan 2025 11:24:58 +0100
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: Yidong Zhang <yidong.zhang@....com>, linux-kernel@...r.kernel.org,
 linux-fpga@...r.kernel.org, mdf@...nel.org, hao.wu@...el.com,
 yilun.xu@...el.com
Cc: lizhi.hou@....com, Nishad Saraf <nishads@....com>,
 Prapul Krishnamurthy <prapulk@....com>
Subject: Re: [PATCH V2 3/4] drivers/fpga/amd: Add remote queue

Le 10/12/2024 à 19:37, Yidong Zhang a écrit :
> The remote queue is a hardware queue based ring buffer service between mgmt
> PF and PCIe hardware firmware for programming FPGA Program Logic, loading
> versal firmware and checking card healthy status.

...

> +static int rm_queue_submit_cmd(struct rm_cmd *cmd)
> +{
> +	struct versal_pci_device *vdev = cmd->rdev->vdev;
> +	struct rm_device *rdev = cmd->rdev;
> +	u32 offset;
> +	int ret;
> +
> +	mutex_lock(&rdev->queue);

Using guardmutex) would slightly simplify the code.

> +
> +	offset = rm_queue_get_sq_slot_offset(rdev);
> +	if (!offset) {
> +		vdev_err(vdev, "No SQ slot available");
> +		ret = -ENOSPC;
> +		goto exit;
> +	}
> +
> +	rm_queue_bulk_write(rdev, offset, (u32 *)&cmd->sq_msg,
> +			    sizeof(cmd->sq_msg));
> +
> +	ret = rm_queue_set_pidx(rdev, RM_QUEUE_SQ, ++rdev->sq.pidx);
> +	if (ret) {
> +		vdev_err(vdev, "Failed to update PIDX, ret %d", ret);
> +		goto exit;
> +	}
> +
> +	list_add_tail(&cmd->list, &rdev->submitted_cmds);
> +exit:
> +	mutex_unlock(&rdev->queue);
> +	return ret;
> +}
> +
> +void rm_queue_withdraw_cmd(struct rm_cmd *cmd)
> +{
> +	mutex_lock(&cmd->rdev->queue);

Using guardmutex) would slightly simplify the code.

> +	list_del(&cmd->list);
> +	mutex_unlock(&cmd->rdev->queue);
> +}

...

> +static void rm_check_msg(struct work_struct *w)
> +{
> +	struct rm_device *rdev = to_rdev_msg_monitor(w);
> +	int ret;
> +
> +	mutex_lock(&rdev->queue);

Using guardmutex) would slightly simplify the code.

> +
> +	rdev->sq.cidx = rm_queue_get_cidx(rdev, RM_QUEUE_SQ);
> +	rdev->cq.pidx = rm_queue_get_pidx(rdev, RM_QUEUE_CQ);
> +
> +	while (rdev->cq.cidx < rdev->cq.pidx) {
> +		ret = rm_process_msg(rdev);
> +		if (ret)
> +			break;
> +
> +		rdev->cq.cidx++;
> +
> +		rm_queue_set_cidx(rdev, RM_QUEUE_CQ, rdev->cq.cidx);
> +	}
> +
> +	mutex_unlock(&rdev->queue);
> +}

...

> +int rm_queue_init(struct rm_device *rdev)
> +{
> +	struct versal_pci_device *vdev = rdev->vdev;
> +	struct rm_queue_header header = {0};
> +	int ret;
> +
> +	INIT_LIST_HEAD(&rdev->submitted_cmds);
> +	mutex_init(&rdev->queue);

devm_mutex_init()?
In order to slightly simplify the code.

> +
> +	rm_queue_bulk_read(rdev, RM_HDR_OFF, (u32 *)&header, sizeof(header));
> +
> +	if (header.magic != RM_QUEUE_HDR_MAGIC_NUM) {
> +		vdev_err(vdev, "Invalid RM queue header");
> +		ret = -ENODEV;
> +		goto error;
> +	}

...

CJ