| Message ID | 161161231309.406594.6061304765472136401.stgit@djiang5-desk3.ch.intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | dmaengine: idxd: check device state before issue command | expand |
On 25-01-21, 15:05, Dave Jiang wrote: > Add device state check before executing command. Without the check the > command can be issued while device is in halt state and causes the driver to > block while waiting for the completion of the command. > > Reported-by: Sanjay Kumar <sanjay.k.kumar@intel.com> > Signed-off-by: Dave Jiang <dave.jiang@intel.com> > Tested-by: Sanjay Kumar <sanjay.k.kumar@intel.com> > Fixes: 0d5c10b4c84d ("dmaengine: idxd: add work queue drain support") > --- > drivers/dma/idxd/device.c | 25 ++++++++++++++++++++++++- > drivers/dma/idxd/idxd.h | 2 +- > drivers/dma/idxd/init.c | 5 ++++- > 3 files changed, 29 insertions(+), 3 deletions(-) > > diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c > index 95f94a3ed6be..45077727ce5b 100644 > --- a/drivers/dma/idxd/device.c > +++ b/drivers/dma/idxd/device.c > @@ -398,17 +398,33 @@ static inline bool idxd_is_enabled(struct idxd_device *idxd) > return false; > } > > +static inline bool idxd_device_is_halted(struct idxd_device *idxd) > +{ > + union gensts_reg gensts; > + > + gensts.bits = ioread32(idxd->reg_base + IDXD_GENSTATS_OFFSET); > + > + if (gensts.state == IDXD_DEVICE_STATE_HALT) > + return true; > + return false; return (gensts.state == IDXD_DEVICE_STATE_HALT) ??
On 1/31/2021 11:11 PM, Vinod Koul wrote: > On 25-01-21, 15:05, Dave Jiang wrote: >> Add device state check before executing command. Without the check the >> command can be issued while device is in halt state and causes the driver to >> block while waiting for the completion of the command. >> >> Reported-by: Sanjay Kumar <sanjay.k.kumar@intel.com> >> Signed-off-by: Dave Jiang <dave.jiang@intel.com> >> Tested-by: Sanjay Kumar <sanjay.k.kumar@intel.com> >> Fixes: 0d5c10b4c84d ("dmaengine: idxd: add work queue drain support") >> --- >> drivers/dma/idxd/device.c | 25 ++++++++++++++++++++++++- >> drivers/dma/idxd/idxd.h | 2 +- >> drivers/dma/idxd/init.c | 5 ++++- >> 3 files changed, 29 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c >> index 95f94a3ed6be..45077727ce5b 100644 >> --- a/drivers/dma/idxd/device.c >> +++ b/drivers/dma/idxd/device.c >> @@ -398,17 +398,33 @@ static inline bool idxd_is_enabled(struct idxd_device *idxd) >> return false; >> } >> >> +static inline bool idxd_device_is_halted(struct idxd_device *idxd) >> +{ >> + union gensts_reg gensts; >> + >> + gensts.bits = ioread32(idxd->reg_base + IDXD_GENSTATS_OFFSET); >> + >> + if (gensts.state == IDXD_DEVICE_STATE_HALT) >> + return true; >> + return false; > return (gensts.state == IDXD_DEVICE_STATE_HALT) ?? Ok I'll send v2.
diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c index 95f94a3ed6be..45077727ce5b 100644 --- a/drivers/dma/idxd/device.c +++ b/drivers/dma/idxd/device.c @@ -398,17 +398,33 @@ static inline bool idxd_is_enabled(struct idxd_device *idxd) return false; } +static inline bool idxd_device_is_halted(struct idxd_device *idxd) +{ + union gensts_reg gensts; + + gensts.bits = ioread32(idxd->reg_base + IDXD_GENSTATS_OFFSET); + + if (gensts.state == IDXD_DEVICE_STATE_HALT) + return true; + return false; +} + /* * This is function is only used for reset during probe and will * poll for completion. Once the device is setup with interrupts, * all commands will be done via interrupt completion. */ -void idxd_device_init_reset(struct idxd_device *idxd) +int idxd_device_init_reset(struct idxd_device *idxd) { struct device *dev = &idxd->pdev->dev; union idxd_command_reg cmd; unsigned long flags; + if (idxd_device_is_halted(idxd)) { + dev_warn(&idxd->pdev->dev, "Device is HALTED!\n"); + return -ENXIO; + } + memset(&cmd, 0, sizeof(cmd)); cmd.cmd = IDXD_CMD_RESET_DEVICE; dev_dbg(dev, "%s: sending reset for init.\n", __func__); @@ -419,6 +435,7 @@ void idxd_device_init_reset(struct idxd_device *idxd) IDXD_CMDSTS_ACTIVE) cpu_relax(); spin_unlock_irqrestore(&idxd->dev_lock, flags); + return 0; } static void idxd_cmd_exec(struct idxd_device *idxd, int cmd_code, u32 operand, @@ -428,6 +445,12 @@ static void idxd_cmd_exec(struct idxd_device *idxd, int cmd_code, u32 operand, DECLARE_COMPLETION_ONSTACK(done); unsigned long flags; + if (idxd_device_is_halted(idxd)) { + dev_warn(&idxd->pdev->dev, "Device is HALTED!\n"); + *status = IDXD_CMDSTS_HW_ERR; + return; + } + memset(&cmd, 0, sizeof(cmd)); cmd.cmd = cmd_code; cmd.operand = operand; diff --git a/drivers/dma/idxd/idxd.h b/drivers/dma/idxd/idxd.h index 5a50e91c71bf..81a0e65fd316 100644 --- a/drivers/dma/idxd/idxd.h +++ b/drivers/dma/idxd/idxd.h @@ -326,7 +326,7 @@ void idxd_mask_msix_vector(struct idxd_device *idxd, int vec_id); void idxd_unmask_msix_vector(struct idxd_device *idxd, int vec_id); /* device control */ -void idxd_device_init_reset(struct idxd_device *idxd); +int idxd_device_init_reset(struct idxd_device *idxd); int idxd_device_enable(struct idxd_device *idxd); int idxd_device_disable(struct idxd_device *idxd); void idxd_device_reset(struct idxd_device *idxd); diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c index 2c051e07c34c..fa04acd5582a 100644 --- a/drivers/dma/idxd/init.c +++ b/drivers/dma/idxd/init.c @@ -335,7 +335,10 @@ static int idxd_probe(struct idxd_device *idxd) int rc; dev_dbg(dev, "%s entered and resetting device\n", __func__); - idxd_device_init_reset(idxd); + rc = idxd_device_init_reset(idxd); + if (rc < 0) + return rc; + dev_dbg(dev, "IDXD reset complete\n"); if (IS_ENABLED(CONFIG_INTEL_IDXD_SVM)) {