From: Daisuke Matsuda <dskmtsd@gmail.com>
To: yilun.xu@intel.com, mdf@kernel.org, trix@redhat.com
Cc: linux-fpga@vger.kernel.org, linux-kernel@vger.kernel.org,
Daisuke Matsuda <dskmtsd@gmail.com>,
Daisuke Matsuda <matsuda@preferred.jp>
Subject: [PATCH 2/2] fpga: altera-cvp: Propagate PCI config access errors
Date: Wed, 1 Jul 2026 09:26:33 +0000 [thread overview]
Message-ID: <20260701092633.5146-2-dskmtsd@gmail.com> (raw)
In-Reply-To: <20260701092633.5146-1-dskmtsd@gmail.com>
From: Daisuke Matsuda <matsuda@preferred.jp>
The CvP control path accesses the VSEC registers through PCI config space,
but several reads and writes ignore the return value. If a config read
fails, the driver can make decisions from an undefined register value and
continue programming with bogus status or control bits.
Fixes: 34d1dc17ce97 ("fpga manager: Add Altera CvP driver")
Signed-off-by: Daisuke Matsuda <matsuda@preferred.jp>
---
drivers/fpga/altera-cvp.c | 137 +++++++++++++++++++++++++++++---------
1 file changed, 105 insertions(+), 32 deletions(-)
diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c
index c00292370a8e..8a896a93eba8 100644
--- a/drivers/fpga/altera-cvp.c
+++ b/drivers/fpga/altera-cvp.c
@@ -82,7 +82,7 @@ struct altera_cvp_conf {
};
struct cvp_priv {
- void (*switch_clk)(struct altera_cvp_conf *conf);
+ int (*switch_clk)(struct altera_cvp_conf *conf);
int (*clear_state)(struct altera_cvp_conf *conf);
int (*wait_credit)(struct fpga_manager *mgr, u32 blocks);
size_t block_size;
@@ -116,7 +116,8 @@ static enum fpga_mgr_states altera_cvp_state(struct fpga_manager *mgr)
struct altera_cvp_conf *conf = mgr->priv;
u32 status;
- altera_read_config_dword(conf, VSE_CVP_STATUS, &status);
+ if (altera_read_config_dword(conf, VSE_CVP_STATUS, &status))
+ return FPGA_MGR_STATE_UNKNOWN;
if (status & VSE_CVP_STATUS_CFG_DONE)
return FPGA_MGR_STATE_OPERATING;
@@ -139,19 +140,27 @@ static void altera_cvp_write_data_config(struct altera_cvp_conf *conf, u32 val)
}
/* switches between CvP clock and internal clock */
-static void altera_cvp_dummy_write(struct altera_cvp_conf *conf)
+static int altera_cvp_dummy_write(struct altera_cvp_conf *conf)
{
unsigned int i;
+ int ret;
u32 val;
/* set 1 CVP clock cycle for every CVP Data Register Write */
- altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
+ ret = altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
+ if (ret)
+ return ret;
+
val &= ~VSE_CVP_MODE_CTRL_NUMCLKS_MASK;
val |= 1 << VSE_CVP_MODE_CTRL_NUMCLKS_OFF;
- altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
+ ret = altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
+ if (ret)
+ return ret;
for (i = 0; i < CVP_DUMMY_WR; i++)
conf->write_data(conf, 0); /* dummy data, could be any value */
+
+ return 0;
}
static int altera_cvp_wait_status(struct altera_cvp_conf *conf, u32 status_mask,
@@ -159,13 +168,17 @@ static int altera_cvp_wait_status(struct altera_cvp_conf *conf, u32 status_mask,
{
unsigned int retries;
u32 val;
+ int ret;
retries = timeout_us / 10;
if (timeout_us % 10)
retries++;
do {
- altera_read_config_dword(conf, VSE_CVP_STATUS, &val);
+ ret = altera_read_config_dword(conf, VSE_CVP_STATUS, &val);
+ if (ret)
+ return ret;
+
if ((val & status_mask) == status_val)
return 0;
@@ -287,21 +300,31 @@ static int altera_cvp_teardown(struct fpga_manager *mgr,
u32 val;
/* STEP 12 - reset START_XFER bit */
- altera_read_config_dword(conf, VSE_CVP_PROG_CTRL, &val);
+ ret = altera_read_config_dword(conf, VSE_CVP_PROG_CTRL, &val);
+ if (ret)
+ return ret;
+
val &= ~VSE_CVP_PROG_CTRL_START_XFER;
- altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
+ ret = altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
+ if (ret)
+ return ret;
/* STEP 13 - reset CVP_CONFIG bit */
val &= ~VSE_CVP_PROG_CTRL_CONFIG;
- altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
+ ret = altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
+ if (ret)
+ return ret;
/*
* STEP 14
* - set CVP_NUMCLKS to 1 and then issue CVP_DUMMY_WR dummy
* writes to the HIP
*/
- if (conf->priv->switch_clk)
- conf->priv->switch_clk(conf);
+ if (conf->priv->switch_clk) {
+ ret = conf->priv->switch_clk(conf);
+ if (ret)
+ return ret;
+ }
/* STEP 15 - poll CVP_CONFIG_READY bit for 0 with 10us timeout */
ret = altera_cvp_wait_status(conf, VSE_CVP_STATUS_CFG_RDY, 0,
@@ -336,7 +359,10 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
conf->numclks = 1; /* for uncompressed and unencrypted images */
/* STEP 1 - read CVP status and check CVP_EN flag */
- altera_read_config_dword(conf, VSE_CVP_STATUS, &val);
+ ret = altera_read_config_dword(conf, VSE_CVP_STATUS, &val);
+ if (ret)
+ return ret;
+
if (!(val & VSE_CVP_STATUS_CVP_EN)) {
dev_err(&mgr->dev, "CVP mode off: 0x%04x\n", val);
return -ENODEV;
@@ -354,21 +380,34 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
* - set HIP_CLK_SEL and CVP_MODE (must be set in the order mentioned)
*/
/* switch from fabric to PMA clock */
- altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
+ ret = altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
+ if (ret)
+ return ret;
+
val |= VSE_CVP_MODE_CTRL_HIP_CLK_SEL;
- altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
+ ret = altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
+ if (ret)
+ return ret;
/* set CVP mode */
- altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
+ ret = altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
+ if (ret)
+ return ret;
+
val |= VSE_CVP_MODE_CTRL_CVP_MODE;
- altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
+ ret = altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
+ if (ret)
+ return ret;
/*
* STEP 3
* - set CVP_NUMCLKS to 1 and issue CVP_DUMMY_WR dummy writes to the HIP
*/
- if (conf->priv->switch_clk)
- conf->priv->switch_clk(conf);
+ if (conf->priv->switch_clk) {
+ ret = conf->priv->switch_clk(conf);
+ if (ret)
+ return ret;
+ }
if (conf->priv->clear_state) {
ret = conf->priv->clear_state(conf);
@@ -381,10 +420,15 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
conf->sent_packets = 0;
/* STEP 4 - set CVP_CONFIG bit */
- altera_read_config_dword(conf, VSE_CVP_PROG_CTRL, &val);
+ ret = altera_read_config_dword(conf, VSE_CVP_PROG_CTRL, &val);
+ if (ret)
+ return ret;
+
/* request control block to begin transfer using CVP */
val |= VSE_CVP_PROG_CTRL_CONFIG;
- altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
+ ret = altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
+ if (ret)
+ return ret;
/* STEP 5 - poll CVP_CONFIG READY for 1 with timeout */
ret = altera_cvp_wait_status(conf, VSE_CVP_STATUS_CFG_RDY,
@@ -399,8 +443,11 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
* STEP 6
* - set CVP_NUMCLKS to 1 and issue CVP_DUMMY_WR dummy writes to the HIP
*/
- if (conf->priv->switch_clk)
- conf->priv->switch_clk(conf);
+ if (conf->priv->switch_clk) {
+ ret = conf->priv->switch_clk(conf);
+ if (ret)
+ return ret;
+ }
if (altera_cvp_chkcfg) {
ret = altera_cvp_chk_error(mgr, 0);
@@ -411,16 +458,26 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
}
/* STEP 7 - set START_XFER */
- altera_read_config_dword(conf, VSE_CVP_PROG_CTRL, &val);
+ ret = altera_read_config_dword(conf, VSE_CVP_PROG_CTRL, &val);
+ if (ret)
+ return ret;
+
val |= VSE_CVP_PROG_CTRL_START_XFER;
- altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
+ ret = altera_write_config_dword(conf, VSE_CVP_PROG_CTRL, val);
+ if (ret)
+ return ret;
/* STEP 8 - start transfer (set CVP_NUMCLKS for bitstream) */
if (conf->priv->switch_clk) {
- altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
+ ret = altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
+ if (ret)
+ return ret;
+
val &= ~VSE_CVP_MODE_CTRL_NUMCLKS_MASK;
val |= conf->numclks << VSE_CVP_MODE_CTRL_NUMCLKS_OFF;
- altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
+ ret = altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
+ if (ret)
+ return ret;
}
return 0;
}
@@ -490,17 +547,25 @@ static int altera_cvp_write_complete(struct fpga_manager *mgr,
return ret;
/* STEP 16 - check CVP_CONFIG_ERROR_LATCHED bit */
- altera_read_config_dword(conf, VSE_UNCOR_ERR_STATUS, &val);
+ ret = altera_read_config_dword(conf, VSE_UNCOR_ERR_STATUS, &val);
+ if (ret)
+ return ret;
+
if (val & VSE_UNCOR_ERR_CVP_CFG_ERR) {
dev_err(&mgr->dev, "detected CVP_CONFIG_ERROR_LATCHED!\n");
return -EPROTO;
}
/* STEP 17 - reset CVP_MODE and HIP_CLK_SEL bit */
- altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
+ ret = altera_read_config_dword(conf, VSE_CVP_MODE_CTRL, &val);
+ if (ret)
+ return ret;
+
val &= ~VSE_CVP_MODE_CTRL_HIP_CLK_SEL;
val &= ~VSE_CVP_MODE_CTRL_CVP_MODE;
- altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
+ ret = altera_write_config_dword(conf, VSE_CVP_MODE_CTRL, val);
+ if (ret)
+ return ret;
/* STEP 18 - poll PLD_CLK_IN_USE and USER_MODE bits */
mask = VSE_CVP_STATUS_PLD_CLK_IN_USE | VSE_CVP_STATUS_USERMODE;
@@ -590,7 +655,10 @@ static int altera_cvp_probe(struct pci_dev *pdev,
return -ENODEV;
}
- pci_read_config_dword(pdev, offset + VSE_CVP_STATUS, ®val);
+ ret = pci_read_config_dword(pdev, offset + VSE_CVP_STATUS, ®val);
+ if (ret)
+ return ret;
+
if (!(regval & VSE_CVP_STATUS_CVP_EN)) {
dev_err(&pdev->dev,
"CVP is disabled for this device: CVP_STATUS Reg 0x%x\n",
@@ -613,10 +681,15 @@ static int altera_cvp_probe(struct pci_dev *pdev,
* even if the concerned BAR is not needed for FPGA configuration
* at all. Thus, enable the device via PCI config space command.
*/
- pci_read_config_word(pdev, PCI_COMMAND, &cmd);
+ ret = pci_read_config_word(pdev, PCI_COMMAND, &cmd);
+ if (ret)
+ return ret;
+
if (!(cmd & PCI_COMMAND_MEMORY)) {
cmd |= PCI_COMMAND_MEMORY;
- pci_write_config_word(pdev, PCI_COMMAND, cmd);
+ ret = pci_write_config_word(pdev, PCI_COMMAND, cmd);
+ if (ret)
+ return ret;
}
ret = pci_request_region(pdev, CVP_BAR, "CVP");
--
2.47.3
prev parent reply other threads:[~2026-07-01 9:27 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-01 9:26 [PATCH 1/2] fpga: altera-cvp: Avoid out-of-bounds read in trailing byte write Daisuke Matsuda
2026-07-01 9:26 ` Daisuke Matsuda [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260701092633.5146-2-dskmtsd@gmail.com \
--to=dskmtsd@gmail.com \
--cc=linux-fpga@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=matsuda@preferred.jp \
--cc=mdf@kernel.org \
--cc=trix@redhat.com \
--cc=yilun.xu@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.