Re: [PATCH 3/5] cxl/acpi: Minimize granularity for x1 interleaves

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

On Wed, 3 Aug 2022 10:18:27 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> Jonathan Cameron wrote:
> > On Tue, 2 Aug 2022 10:33:19 -0700
> > Dan Williams <dan.j.williams@intel.com> wrote:
> >   
> > > Jonathan Cameron wrote:  
> > > > On Fri, 22 Jul 2022 17:56:09 -0700
> > > > Dan Williams <dan.j.williams@intel.com> wrote:
> > > >     
> > > > > The kernel enforces that region granularity is >= to the top-level
> > > > > interleave-granularity for the given CXL window. However, when the CXL
> > > > > window interleave is x1, i.e. non-interleaved at the host bridge level,
> > > > > then the specified granularity does not matter. Override the window
> > > > > specified granularity to the CXL minimum so that any valid region
> > > > > granularity is >= to the root granularity.
> > > > > 
> > > > > Reported-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > > > Signed-off-by: Dan Williams <dan.j.williams@intel.com>    
> > > > 
> > > > Hi Dan,
> > > > 
> > > > Debugging exactly why this is failing (from cxl.git/preview) for my test setup...
> > > > (1 hb, 8 rp, 8 direct connected devices)
> > > > 
> > > > If I set the interleave granularity of a region to 256, I end
> > > > up with 256 for the CFMWS which is fine, then 512 for the HB which
> > > > is not - EP interleave granularity is expected 256.
> > > > 
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl.git/tree/drivers/cxl/core/region.c?h=preview#n1070
> > > > 
> > > > Calculates the eig as address_bit - eiw + 1
> > > > 
> > > > iw = 8
> > > > eiw = 3
> > > > peig = 0 (pig = 256)
> > > > peiw = 0 (piw = 1)
> > > > (all as expected I think...)
> > > > 
> > > > So address_bit = s max(peig + peiw, eiw + peig) = max(0, 3)
> > > > and eig = 3 - 3 + 1 = 1 (ig = 512) which is wrong.
> > > > 
> > > > I'm not 100% sure on the logic behind this maths, but would expect eig = 0 as the output for this
> > > > setup..    
> > > 
> > > Yeah, the "+ 1" is not required when routing from the x1 HB level. I can
> > > reproduce this config with cxl_test to validate.  
> > 
> > Other than this off by one, with the other fixes you posted everything now works for me
> > with the particular test case above.  
> 
> Oh, you are hitting this path in your test? I misread the condition when
> this triggers. It should be skipping the granularity increment when the
> port's parent interleave is x1. I.e. in this case with devices directly
> connected to a host-bridge where the host-bridge is not interleaved then
> there is no need for an extra address bit to route the decode.
> 

I haven't chased down why, but definitely hitting it with resulting routing
to wrong device. 
Current test when setting interleave gran for the decoder1.0 (the HB in this case)
is on the ways of decoder 1.0 which is 8.
Maybe the test should be on the parent_rr nr_targets? (or parent_iw should work I think)
That makes things 'work' for this particular test, but need to test on a wider range
of setups.

Jonathan