Re: [PATCH net v2 1/2] bnge: fix initial HWRM sequence

[Vikas Gupta]

On Fri, Apr 17, 2026 at 8:12 PM Jakub Kicinski <kuba@xxxxxxxxxx> wrote:
>
> On Fri, 17 Apr 2026 11:46:08 +0530 Vikas Gupta wrote:
> > > > -err_func_unrgtr:
> > > > -     bnge_fw_unregister_dev(bd);
> > > > +err_free_ctx_mem:
> > > > +     bnge_free_ctx_mem(bd);
> > > >       return rc;
> > > >  }
> > >
> > > This error path appears to have the same regression. If
> > > bnge_hwrm_func_drv_rgtr() fails after bnge_func_qcaps() has already
> > > configured the backing store, freeing the context memory directly without
> > > unregistering might allow the hardware to access freed memory.
> >
> > Even if bnge_hwrm_func_drv_rgtr() fails, it is still safe to free the context
> > memory at the host because the driver unloads from this point.
>
> Looking closer, indeed, the way bnge_hwrm_func_drv_unrgtr() is written
> the AI suggestion is pointless. Hopefully you're right cause debugging
> FW corrupting host memory after reboot on bnxt is not fun.
>
> > AI reviews appear to ignore logic related to handling context memory
> > in the patch.
> > I see no valid comments on the patch.
>
> Why is bnge_func_qcaps() allocating context mem? It may be the case
> that context mem has to be allocated but bnge_func_qcaps() doesn't
> sound like a function that'd perform such key part of init.
> Why not just move the alloc earlier in bnge_fw_register_dev() ?

I agree that bnge_func_qcaps(), which appears to be a query function,
should not allocate memory. I can refactor bnge_func_qcaps() and
move bnge_alloc_ctx_mem() to bnge_fw_register_dev() so that
bnge_func_qcaps() remains solely a query function.
I`ll make changes in v3.

Thanks,
Vikas

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature