On Thu, 7 Sept 2023 at 19:17, Stefan Hajnoczi <stefanha@redhat.com> wrote:
From: Jeuk Kim <jeuk20.kim@samsung.com>
This commit makes the UFS device support query
and nop out transfer requests.
The next patch would be support for UFS logical
unit and scsi command transfer request.
Signed-off-by: Jeuk Kim <jeuk20.kim@samsung.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id:
ff7a5f0fd26761936a553ffb89d3df0ba62844e9.1693980783.git.jeuk20.kim@gmail.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/ufs/ufs.h | 46 +++
hw/ufs/ufs.c | 988 +++++++++++++++++++++++++++++++++++++++++++-
hw/ufs/trace-events | 1 +
3 files changed, 1033 insertions(+), 2 deletions(-)
Hi; Coverity isn't happy about the code in this function
(CID 1519050). The code isn't strictly wrong, but it's
probably possible to make it a bit more clearly correct.
+static void ufs_process_db(UfsHc *u, uint32_t val)
+{
+ unsigned long doorbell;
+ uint32_t slot;
+ uint32_t nutrs = u->params.nutrs;
+ UfsRequest *req;
+
+ val &= ~u->reg.utrldbr;
+ if (!val) {
+ return;
+ }
+
+ doorbell = val;
+ slot = find_first_bit(&doorbell, nutrs);
Here we pass the address of a single 'unsigned long' to
find_first_bit(). That function operates on arrays, so
unless nutrs is guaranteed to be less than 32 this might
walk off the end of memory.
There is a check on params.nutrs in ufs_check_constraints(),
which checks for "> UFS_MAX_NUTRS" and that value is 32,
so this won't actually overflow, but Coverity can't
see that check and in any case what it really doesn't
like here is the passing of the address of a 'long'
variable to a function that is prototyped as taking
an array of longs.
You can probably make Coverity happy by defining
doorbell here as a 1 element array, and asserting
that nutrs is 32 or less. Alternatively, we have
ctz32() for working through bits in a uint32_t, though
that is a bit lower-level than find_first_bit/find_next_bit.
+
+ while (slot < nutrs) {
+ req = &u->req_list[slot];
+ if (req->state == UFS_REQUEST_ERROR) {
+ trace_ufs_err_utrl_slot_error(req->slot);
+ return;
+ }
+
+ if (req->state != UFS_REQUEST_IDLE) {
+ trace_ufs_err_utrl_slot_busy(req->slot);
+ return;
+ }
+
+ trace_ufs_process_db(slot);
+ req->state = UFS_REQUEST_READY;
+ slot = find_next_bit(&doorbell, nutrs, slot + 1);
+ }
+
+ qemu_bh_schedule(u->doorbell_bh);
+}
thanks
-- PMM