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

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

On 22-01-06 18:10:33, Jonathan Cameron wrote:
> 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.
> 

Yeah. For validation standpoint, having it be a prop is nice, but I think
default as max rather than 1 is smart.

> > 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.

Okay, thanks.

> 
> Jonathan
> 
> > 
> > Jonathan
> > 
> > 
>