As there is an inclusive rte_io_wmb within the following rte_write32() API who rings the doorbell, this makes the above rte_wmb unecessary and remove it. The doorbell ringing operation requires a rte_io_wmb immediately to make it complete and visible to the device. To read the doorbell response, which is held in the host CIO memory, rte_cio_rmb is sufficient. Fixes: 804e746c7b73 ("net/bnxt: add hardware resource manager init code") Fixes: ca241d9a0952 ("net/bnxt: use I/O device memory read/write API") Cc: stable@dpdk.org Signed-off-by: Gavin Hu <gavin.hu@arm.com> Reviewed-by: Steve Capper <steve.capper@arm.com> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> Reviewed-by: Phil Yang <phil.yang@arm.com> --- drivers/net/bnxt/bnxt_hwrm.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c index 9883fb5..ee07db9 100644 --- a/drivers/net/bnxt/bnxt_hwrm.c +++ b/drivers/net/bnxt/bnxt_hwrm.c @@ -115,9 +115,6 @@ static int bnxt_hwrm_send_message(struct bnxt *bp, void *msg, data = (uint32_t *)&short_input; msg_len = sizeof(short_input); - /* Sync memory write before updating doorbell */ - rte_wmb(); - max_req_len = BNXT_HWRM_SHORT_REQ_LEN; } @@ -137,11 +134,12 @@ static int bnxt_hwrm_send_message(struct bnxt *bp, void *msg, /* Ring channel doorbell */ bar = (uint8_t *)bp->bar0 + mb_trigger_offset; rte_write32(1, bar); + rte_io_wmb(); /* Poll for the valid bit */ for (i = 0; i < HWRM_CMD_TIMEOUT; i++) { /* Sanity check on the resp->resp_len */ - rte_rmb(); + rte_cio_rmb(); if (resp->resp_len && resp->resp_len <= bp->max_resp_len) { /* Last byte of resp contains the valid key */ valid = (uint8_t *)resp + resp->resp_len - 1; -- 2.7.4
As there is an inclusive rte_io_wmb within the following rte_write32() API who rings the doorbell, this makes the above rte_wmb unnecessary and remove it. Fixes: 1cd45aeb3270 ("net/bnxt: support Stratus VF device") Cc: stable@dpdk.org Signed-off-by: Gavin Hu <gavin.hu@arm.com> Reviewed-by: Steve Capper <steve.capper@arm.com> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> Reviewed-by: Phil Yang <phil.yang@arm.com> --- drivers/net/bnxt/bnxt_hwrm.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c index 9883fb5..1d9fb17 100644 --- a/drivers/net/bnxt/bnxt_hwrm.c +++ b/drivers/net/bnxt/bnxt_hwrm.c @@ -115,9 +115,6 @@ static int bnxt_hwrm_send_message(struct bnxt *bp, void *msg, data = (uint32_t *)&short_input; msg_len = sizeof(short_input); - /* Sync memory write before updating doorbell */ - rte_wmb(); - max_req_len = BNXT_HWRM_SHORT_REQ_LEN; } -- 2.7.4
To read the doorbell response, which is held in the host CIO memory, rte_cio_rmb is sufficient. Fixes: 804e746c7b73 ("net/bnxt: add hardware resource manager init code") Cc: stable@dpdk.org Signed-off-by: Gavin Hu <gavin.hu@arm.com> --- drivers/net/bnxt/bnxt_hwrm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c index 1d9fb17..2d5dc00 100644 --- a/drivers/net/bnxt/bnxt_hwrm.c +++ b/drivers/net/bnxt/bnxt_hwrm.c @@ -138,7 +138,7 @@ static int bnxt_hwrm_send_message(struct bnxt *bp, void *msg, /* Poll for the valid bit */ for (i = 0; i < HWRM_CMD_TIMEOUT; i++) { /* Sanity check on the resp->resp_len */ - rte_rmb(); + rte_cio_rmb(); if (resp->resp_len && resp->resp_len <= bp->max_resp_len) { /* Last byte of resp contains the valid key */ valid = (uint8_t *)resp + resp->resp_len - 1; -- 2.7.4
The doorbell ringing operation requires a rte_io_mb immediately to make the command complete and visible to the device before reading the response, otherwise it may read stale or invalid responses. Fixes: ca241d9a0952 ("net/bnxt: use I/O device memory read/write API") Cc: stable@dpdk.org Signed-off-by: Gavin Hu <gavin.hu@arm.com> --- drivers/net/bnxt/bnxt_hwrm.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c index 2d5dc00..7d192e3 100644 --- a/drivers/net/bnxt/bnxt_hwrm.c +++ b/drivers/net/bnxt/bnxt_hwrm.c @@ -134,6 +134,12 @@ static int bnxt_hwrm_send_message(struct bnxt *bp, void *msg, /* Ring channel doorbell */ bar = (uint8_t *)bp->bar0 + mb_trigger_offset; rte_write32(1, bar); + /* + * Make sure the channel doorbell ring command complete before + * reading the response to avoid getting stale or invalid + * responses. + */ + rte_io_mb(); /* Poll for the valid bit */ for (i = 0; i < HWRM_CMD_TIMEOUT; i++) { -- 2.7.4
On Mon, Sep 16, 2019 at 4:27 AM Gavin Hu <gavin.hu@arm.com> wrote: > The doorbell ringing operation requires a rte_io_mb immediately to make > the command complete and visible to the device before reading the > response, otherwise it may read stale or invalid responses. > > Fixes: ca241d9a0952 ("net/bnxt: use I/O device memory read/write API") > Cc: stable@dpdk.org > > Signed-off-by: Gavin Hu <gavin.hu@arm.com> > Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com> > --- > drivers/net/bnxt/bnxt_hwrm.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c > index 2d5dc00..7d192e3 100644 > --- a/drivers/net/bnxt/bnxt_hwrm.c > +++ b/drivers/net/bnxt/bnxt_hwrm.c > @@ -134,6 +134,12 @@ static int bnxt_hwrm_send_message(struct bnxt *bp, > void *msg, > /* Ring channel doorbell */ > bar = (uint8_t *)bp->bar0 + mb_trigger_offset; > rte_write32(1, bar); > + /* > + * Make sure the channel doorbell ring command complete before > + * reading the response to avoid getting stale or invalid > + * responses. > + */ > + rte_io_mb(); > > /* Poll for the valid bit */ > for (i = 0; i < HWRM_CMD_TIMEOUT; i++) { > -- > 2.7.4 > >
On Mon, Sep 16, 2019 at 4:27 AM Gavin Hu <gavin.hu@arm.com> wrote: > As there is an inclusive rte_io_wmb within the following rte_write32() > API who rings the doorbell, this makes the above rte_wmb unnecessary and > remove it. > > Fixes: 1cd45aeb3270 ("net/bnxt: support Stratus VF device") > Cc: stable@dpdk.org > > Signed-off-by: Gavin Hu <gavin.hu@arm.com> > Reviewed-by: Steve Capper <steve.capper@arm.com> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> > Reviewed-by: Phil Yang <phil.yang@arm.com> > Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com> > --- > drivers/net/bnxt/bnxt_hwrm.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c > index 9883fb5..1d9fb17 100644 > --- a/drivers/net/bnxt/bnxt_hwrm.c > +++ b/drivers/net/bnxt/bnxt_hwrm.c > @@ -115,9 +115,6 @@ static int bnxt_hwrm_send_message(struct bnxt *bp, > void *msg, > data = (uint32_t *)&short_input; > msg_len = sizeof(short_input); > > - /* Sync memory write before updating doorbell */ > - rte_wmb(); > - > max_req_len = BNXT_HWRM_SHORT_REQ_LEN; > } > > -- > 2.7.4 > >
On Mon, Sep 16, 2019 at 4:27 AM Gavin Hu <gavin.hu@arm.com> wrote: > To read the doorbell response, which is held in the host CIO memory, > rte_cio_rmb is sufficient. > > Fixes: 804e746c7b73 ("net/bnxt: add hardware resource manager init code") > Cc: stable@dpdk.org > > Signed-off-by: Gavin Hu <gavin.hu@arm.com> > Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com> > --- > drivers/net/bnxt/bnxt_hwrm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c > index 1d9fb17..2d5dc00 100644 > --- a/drivers/net/bnxt/bnxt_hwrm.c > +++ b/drivers/net/bnxt/bnxt_hwrm.c > @@ -138,7 +138,7 @@ static int bnxt_hwrm_send_message(struct bnxt *bp, > void *msg, > /* Poll for the valid bit */ > for (i = 0; i < HWRM_CMD_TIMEOUT; i++) { > /* Sanity check on the resp->resp_len */ > - rte_rmb(); > + rte_cio_rmb(); > if (resp->resp_len && resp->resp_len <= bp->max_resp_len) { > /* Last byte of resp contains the valid key */ > valid = (uint8_t *)resp + resp->resp_len - 1; > -- > 2.7.4 > >