Re: [PATCH v6 3/3] hw/cxl: Add a performant (and correct) path for the non interleaved cases

[Jonathan Cameron via qemu development <qemu-devel@nongnu.org>]

On Thu, 5 Mar 2026 16:13:24 -0500
Gregory Price <gourry@gourry.net> wrote:

> On Thu, Feb 26, 2026 at 03:20:34PM +0000, Alireza Sanaee wrote:
> > +        /*
> > +         * Optimization: Looking for a fully committed path; if the type 3 HDM
> > +         * decoder is not commmitted, it cannot lie on such a path.  
>                                ^^^ committed
> 
> ... snip ...
> > +                object_child_foreach_recursive(object_get_root(),
> > +                                               cxl_fmws_direct_passthrough,
> > +                                               &state);
> > +            }
> > +        }
> > +        dpa_base += decoder_size / cxl_interleave_ways_dec(iw, &error_fatal);
> > +    }
> > +  
> 
> Div-by-0 here for invalid values?
> 
> int cxl_interleave_ways_dec(uint8_t iw_enc, Error **errp)
> {
>     switch (iw_enc) {
>     ... snip ...
>     default:
>         error_setg(errp, "Encoded interleave ways: %d not supported", iw_enc);
>         return 0;

It will abort because error_fatal is passed in. I believe we've always sanity
checked the value at write so 
That's not particularly elegant so I think we should do a check on it being
zero and just return 0 if it happens to be so. 
We have similar checks in the other uses of cxl_interleave_ways_dec()

On the todo list for a long time has been adding more sanity checking
to the HDM commit flows.  As we are getting closer to this stuff actually
being usable for virtualization as well as emulation / testing, that will
need cleaning up.

>     }
> }
> 
> Or does this abort before return?
> 
> > +    return false;
> > +}  
> 
> You're returning false as an 'int'
Good spot. That should be return 0.

> 
> ~Gregory