Re: [RFC PATCH v2 23/28] cxl/region: Implement XHB verification

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
To: Ben Widawsky <ben.widawsky@intel.com>
Cc: <linux-cxl@vger.kernel.org>,
	Chet Douglas <chet.r.douglas@intel.com>,
	Alison Schofield <alison.schofield@intel.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Ira Weiny <ira.weiny@intel.com>,
	Vishal Verma <vishal.l.verma@intel.com>
Subject: Re: [RFC PATCH v2 23/28] cxl/region: Implement XHB verification
Date: Thu, 6 Jan 2022 18:10:33 +0000	[thread overview]
Message-ID: <20220106181033.00000f4c@huawei.com> (raw)
In-Reply-To: <20220106173346.00002171@huawei.com>

On Thu, 6 Jan 2022 17:33:46 +0000
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:

> On Thu, 6 Jan 2022 08:58:15 -0800
> Ben Widawsky <ben.widawsky@intel.com> wrote:
> 
> > On 22-01-06 16:55:47, Jonathan Cameron wrote:  
> > > On Fri, 22 Oct 2021 11:37:04 -0700
> > > Ben Widawsky <ben.widawsky@intel.com> wrote:
> > >     
> > > > Cross host bridge verification primarily determines if the requested
> > > > interleave ordering can be achieved by the root decoder, which isn't as
> > > > programmable as other decoders.
> > > > 
> > > > The algorithm implemented here is based on the CXL Type 3 Memory Device
> > > > Software Guide, chapter 2.13.14
> > > > 
> > > > Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
> > > > ---
> > > >  .clang-format        |  1 +
> > > >  drivers/cxl/region.c | 81 +++++++++++++++++++++++++++++++++++++++++++-
> > > >  drivers/cxl/trace.h  |  3 ++
> > > >  3 files changed, 84 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/.clang-format b/.clang-format
> > > > index cb7c46371465..55f628f21722 100644
> > > > --- a/.clang-format
> > > > +++ b/.clang-format
> > > > @@ -169,6 +169,7 @@ ForEachMacros:
> > > >    - 'for_each_cpu_and'
> > > >    - 'for_each_cpu_not'
> > > >    - 'for_each_cpu_wrap'
> > > > +  - 'for_each_cxl_decoder_target'
> > > >    - 'for_each_cxl_endpoint'
> > > >    - 'for_each_dapm_widgets'
> > > >    - 'for_each_dev_addr'
> > > > diff --git a/drivers/cxl/region.c b/drivers/cxl/region.c
> > > > index d127c9c69eef..53442de33d11 100644
> > > > --- a/drivers/cxl/region.c
> > > > +++ b/drivers/cxl/region.c
> > > > @@ -30,6 +30,11 @@
> > > >  	for (idx = 0, ep = (region)->targets[idx]; idx < region_ways(region);  \
> > > >  	     idx++, ep = (region)->targets[idx])
> > > >  
> > > > +#define for_each_cxl_decoder_target(target, decoder, idx)                      \
> > > > +	for (idx = 0, target = (decoder)->target[idx];                         \
> > > > +	     idx < (decoder)->nr_targets;                                      \
> > > > +	     idx++, target = (decoder)->target[idx])
> > > > +    
> > > target used for too many things in this macro.
> > > 
> > > I'm messing around with this to poke some of the Qemu stuff and noticed
> > > this in passing...
> > > 
> > > Jonathan    
> > 
> > Thanks.
> > 
> > BTW, I have some rather large changes in flight. Might be good to check this
> > branch (I'm in force push mode):
> > https://gitlab.com/bwidawsk/linux/-/commits/cxl_region
> > 
> > Also, I have a minor QEMU change (HACK) to support multiple root ports.
> > https://gitlab.com/bwidawsk/qemu/-/commit/7c76849f9a4d2bc5fc9c355ed06ea926fc7ab494  

If we were feeling lazy that could (I think) just be set to the maximum allowed and
be 'correct' in all cases.

> Thanks. Will take a look at both.
> 
> Mostly I'm interested in the QEMU side of things and trying to get a cleaner
> command line working but good to have a way to poke it an check the
> CFMWS is correct etc.

FYI. I'll leave feedback for where I'm hitting bugs on your gitlab branches.
My test setup that I'm trying to build regions on is
2 host bridge, 2 ports on each, 1 device directly connected to both.
The qemu code will unfortunately take a bit of extracting from company internals
so I want to get a bit further with it before going the effort of doing that
and I have a few other things on my todo list.

Jonathan

> 
> Jonathan
> 
> 