On Thu, 09 Nov 2023, Peter Maydell wrote:
On Tue, 7 Nov 2023 at 10:13, Michael S. Tsirkin <m...@redhat.com> wrote:
From: Davidlohr Bueso <d...@stgolabs.net>
Support background commands in the mailbox, and update
cmd_infostat_bg_op_sts() accordingly. This patch does not implement mbox
interrupts upon completion, so the kernel driver must rely on polling to
know when the operation is done.
Signed-off-by: Davidlohr Bueso <d...@stgolabs.net>
Signed-off-by: Jonathan Cameron <jonathan.came...@huawei.com>
Message-Id: <20231023160806.13206-12-jonathan.came...@huawei.com>
Reviewed-by: Michael S. Tsirkin <m...@redhat.com>
Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
Hi; Coverity points out dead code in this function (CID 1523907):
+static void bg_timercb(void *opaque)
+{
+ CXLCCI *cci = opaque;
+ CXLDeviceState *cxl_dstate = &CXL_TYPE3(cci->d)->cxl_dstate;
+ uint64_t bg_status_reg = 0;
+ uint64_t now = qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL);
+ uint64_t total_time = cci->bg.starttime + cci->bg.runtime;
+
+ assert(cci->bg.runtime > 0);
+ bg_status_reg = FIELD_DP64(bg_status_reg, CXL_DEV_BG_CMD_STS,
+ OP, cci->bg.opcode);
+
+ if (now >= total_time) { /* we are done */
+ uint64_t status_reg;
+ uint16_t ret = CXL_MBOX_SUCCESS;
Here we set 'ret' to CXL_MBOX_SUCCESS...
+
+ cci->bg.complete_pct = 100;
+ /* Clear bg */
+ status_reg = FIELD_DP64(0, CXL_DEV_MAILBOX_STS, BG_OP, 0);
+ cxl_dstate->mbox_reg_state64[R_CXL_DEV_MAILBOX_STS] = status_reg;
+
+ bg_status_reg = FIELD_DP64(bg_status_reg, CXL_DEV_BG_CMD_STS,
+ RET_CODE, ret);
...and nothing here changes 'ret'...
+
+ /* TODO add ad-hoc cmd succesful completion handling */
+
+ qemu_log("Background command %04xh finished: %s\n",
+ cci->bg.opcode,
+ ret == CXL_MBOX_SUCCESS ? "success" : "aborted");
...but here we check whether ret is CXL_MBOX_SUCCESS or not:
the "aborted" half of this condition is dead code.
Yeah this had also been removed in a later version:
https://lore.kernel.org/linux-cxl/20230418172337.19207-2-d...@stgolabs.net/
A later commit adds an "if (ret == CXL_MBOX_SUCCESS) {" block
where the TODO currently is, and that is an unnecessary check,
because ret cannot be anything else at that point.
I'll send a patch along with the other dead code report.
Thanks,
Davidlohr
