Re: [RFC PATCH 02/15] cxl/core/hdm: Bail on endpoint init fail

[Ben Widawsky <ben.widawsky@intel.com>]

On 22-04-18 16:37:12, Adam Manzanares wrote:
> On Wed, Apr 13, 2022 at 02:31:42PM -0700, Dan Williams wrote:
> > On Wed, Apr 13, 2022 at 11:38 AM Ben Widawsky <ben.widawsky@intel.com> wrote:
> > >
> > > Endpoint decoder enumeration is the only way in which we can determine
> > > Device Physical Address (DPA) -> Host Physical Address (HPA) mappings.
> > > Information is obtained only when the register state can be read
> > > sequentially. If when enumerating the decoders a failure occurs, all
> > > other decoders must also fail since the decoders can no longer be
> > > accurately managed (unless it's the last decoder in which case it can
> > > still work).
> > 
> > I think this should be expanded to fail if any decoder fails to
> > allocate anywhere in the topology otherwise it leaves a mess for
> > future address translation code to work through cases where decoder
> > information is missing.
> > 
> > The current approach is based around the current expectation that
> > nothing is enumerating pre-existing regions, and nothing is performing
> > address translation.
> 
> Does the qemu support currently allow testing of this patch? If so, it would 
> be good to reference qemu configurations. Any other alternatives would be 
> welcome as well. 
> 
> +Luis on cc.
> 

No. This type of error injection would be cool to have, but I'm not sure of a
good way to support that in a scalable way. Maybe Jonathan has some ideas?

> > 
> > >
> > > Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
> > > ---
> > >  drivers/cxl/core/hdm.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> > > index bfc8ee876278..c3c021b54079 100644
> > > --- a/drivers/cxl/core/hdm.c
> > > +++ b/drivers/cxl/core/hdm.c
> > > @@ -255,6 +255,8 @@ int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm)
> > >                                       cxlhdm->regs.hdm_decoder, i);
> > >                 if (rc) {
> > >                         put_device(&cxld->dev);
> > > +                       if (is_endpoint_decoder(&cxld->dev))
> > > +                               return rc;
> > >                         failed++;
> > >                         continue;
> > >                 }
> > > --
> > > 2.35.1
> > >
> >