Re: [PATCH v3 4/4] hw/cxl: Support 4 HDM decoders at all levels of topology

[Jonathan Cameron]

On Tue, 12 Sep 2023 18:08:44 +0000
Fan Ni <fan.ni@samsung.com> wrote:

> On Mon, Sep 11, 2023 at 12:43:13PM +0100, Jonathan Cameron wrote:
> 
> > Support these decoders in CXL host bridges (pxb-cxl), CXL Switch USP
> > and CXL Type 3 end points.
> > 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > ---  
> 
> One comment inline, other than that, looks good to me.

I think we are fine, but also possible I'm missing something :)

> >  
> >  /* 8.2.5.13 - CXL Extended Security Capability Structure (Root complex 
> > only) */
> >  #define EXTSEC_ENTRY_MAX        256
> > diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
> > index aa011a8f34..3ecdad4a5e 100644
> > --- a/hw/cxl/cxl-component-utils.c
> > +++ b/hw/cxl/cxl-component-utils.c
> > @@ -90,6 +90,9 @@ static void dumb_hdm_handler(CXLComponentState 
> > *cxl_cstate, hwaddr offset,
> >  
> >      switch (offset) {
> >      case A_CXL_HDM_DECODER0_CTRL:
> > +    case A_CXL_HDM_DECODER1_CTRL:
> > +    case A_CXL_HDM_DECODER2_CTRL:
> > +    case A_CXL_HDM_DECODER3_CTRL:
> >          should_commit = FIELD_EX32(value, CXL_HDM_DECODER0_CTRL, COMMIT);
> >          should_uncommit = !should_commit;  
> 
> So for the commit/uncommit flag, we always check decoder 0 control
> register? Or i read it wrong? I thought the commit bit is per control register
> thing?

This is in the write handler and the value passed in that we are looking at is
for whichever of the _CTRL registers is being written.

I could have coded this as separate entries for each register as
FIELD_EX32(value, CXL_HDM_DECODER[X]_CTRL, COMMIT)
but as this only figures out the field offset and mask, it is the same for 
X=0,1,2,3

Jonathan