Re: [PATCH] cxl/hdm: Fix hdm decoder init by adding COMMIT field check

From: Dan Williams
Date: Fri Mar 03 2023 - 17:39:29 EST


Fan Ni wrote:
[..]
> > I think a separate fix for that crash is needed, can you send the
> > backtrace? I.e. I worry that crash can be triggered by other means.
> Hi Dan,
> See backtrace below.

Thanks, I'll take a look.

[..]
> > > @@ -710,10 +711,11 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
> > > base = ioread64_hi_lo(hdm + CXL_HDM_DECODER0_BASE_LOW_OFFSET(which));
> > > size = ioread64_hi_lo(hdm + CXL_HDM_DECODER0_SIZE_LOW_OFFSET(which));
> > > committed = !!(ctrl & CXL_HDM_DECODER0_CTRL_COMMITTED);
> > > + should_commit = !!(ctrl & CXL_HDM_DECODER0_CTRL_COMMIT);
> >
> > This change looks like a good idea in general given the ambiguity of
> > 'committed'. However just combine the two checks into the @committed
> > variable with something like this:
> >
> > commit_mask = CXL_HDM_DECODER0_CTRL_COMMITTED|CXL_HDM_DECODER0_CTRL_COMMIT;
> > committed = (ctrl & commit_mask) == commit_mask;

Did you also notice this ^^^ request for a fixed up version of the
current patch?