Re: [PATCH] cxl/port: Enable the HDM decoder capability for switch ports

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Thu, 1 Jun 2023 10:25:13 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> Jonathan Cameron wrote:
> > On Wed, 17 May 2023 20:19:43 -0700
> > Dan Williams <dan.j.williams@intel.com> wrote:
> >   
> > > Derick noticed, when testing hot plug, that hot-add behaves nominally
> > > after a removal. However, if the hot-add is done without a prior
> > > removal, CXL.mem accesses fail. It turns out that the original
> > > implementation of the port driver and region programming wrongly assumed
> > > that platform-firmware always enables the host-bridge HDM decoder
> > > capability. Add support turning on switch-level HDM decoders in the case
> > > where platform-firmware has not.
> > > 
> > > The implementation is careful to only arrange for the enable to be
> > > undone if the current instance of the driver was the one that did the
> > > enable. This is to interoperate with platform-firmware that may expect
> > > CXL.mem to remain active after the driver is shutdown. This comes at the
> > > cost of potentially not shutting down the enable on kexec flows, but it
> > > is mitigated by the fact that the related HDM decoders still need to be
> > > enabled on an individual basis.
> > > 
> > > Cc: <stable@vger.kernel.org>
> > > Reported-by: Derick Marks <derick.w.marks@intel.com>
> > > Fixes: 54cdbf845cf7 ("cxl/port: Add a driver for 'struct cxl_port' objects")
> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com>  
> > 
> > I doubt anyone will be surprised to hear that the QEMU emulation wasn't
> > checking this bit when doing the topology walk.
> > Oops - I'll put together a fix.
> > 
> > I'm guessing this is already queued or upstream but if not
> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>  
> 
> So I took it on faith from the reporter who said "the kernel is failing
> to enable the HDM decoder capability in the root port", but now that I
> go back and read the specification this bit is only applicable to
> *endpoints*. So something else is wrong and this needs to be reverted.

Glad you checked! :)

I'll drop the qemu patch (not posted yet)

Thanks,

Jonathan