Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 9 Jun 2022 09:27:38 +0100
From:   Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To:     Ira Weiny <ira.weiny@intel.com>
CC:     Ben Widawsky <bwidawsk@kernel.org>,
        Dan Williams <dan.j.williams@intel.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        "Alison Schofield" <alison.schofield@intel.com>,
        Vishal Verma <vishal.l.verma@intel.com>,
        Dave Jiang <dave.jiang@intel.com>,
        <linux-kernel@vger.kernel.org>, <linux-cxl@vger.kernel.org>,
        <linux-pci@vger.kernel.org>
Subject: Re: [PATCH V10 6/9] cxl/port: Read CDAT table
Message-ID: <20220609092738.00007553@Huawei.com>
In-Reply-To: <YqESPug9duS6BMfQ@iweiny-desk3>
References: <20220605005049.2155874-1-ira.weiny@intel.com>
        <20220605005049.2155874-7-ira.weiny@intel.com>
        <20220606181541.ysb3zqdpe5cuk4e6@bwidawsk-mobl5>
        <YqESPug9duS6BMfQ@iweiny-desk3>
Organization: Huawei Technologies Research and Development (UK) Ltd.
MIME-Version: 1.0
Content-Type: text/plain; charset="US-ASCII"
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Wed, 8 Jun 2022 14:27:14 -0700
Ira Weiny <ira.weiny@intel.com> wrote:

> On Mon, Jun 06, 2022 at 11:15:41AM -0700, Ben Widawsky wrote:
> > On 22-06-04 17:50:46, ira.weiny@intel.com wrote:  
> > > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > >   
> 

[snip]

> >   
> > > +
> > > +		entry = cdat_response_pl + 1;
> > > +		entry_dw = task.rv / sizeof(u32);
> > > +		/* Skip Header */
> > > +		entry_dw -= 1;
> > > +		entry_dw = min(length / 4, entry_dw);
> > > +		memcpy(data, entry, entry_dw * sizeof(u32));
> > > +		length -= entry_dw * sizeof(u32);
> > > +		data += entry_dw;
> > > +		entry_handle = FIELD_GET(CXL_DOE_TABLE_ACCESS_ENTRY_HANDLE, cdat_response_pl[0]);  
> > 
> > [0] looks suspicious...  
> 
> Actually I have to claim ignorance on this one.  I've carried this from
> Jonathan's original patches.  I'm not as worried about the [0] as that is just
> the first dword.  But I'm confused as to this entry handle now.
> 
> Jonathan?  Help?

Looks right to me.  The entryhandle is a field in the upper 16 bits of the
first dword defined in Read Entry Response table in the CXL spec and also
used in the request of the next entry (which is more or less a CDAT structure)
Two magic values.
0 -  CDAT header (request only - can't be returned)
0xFFFF - No more entries.

As we are reading the whole table, we write 0 to first request and from there
on use the value returned in the response for the next request until we see
0xFFFF and stop.

Note IIRC the meaning of entry handle was clarified in a CXL 2.0 errata as
was a bit ambiguous in the original spec (we had two QEMU implementations
briefly and they did different things :)

Jonathan


> 
> >   
> > > +
> > > +	} while (entry_handle != 0xFFFF);
> > > +
> > > +	return rc;
> > > +}
> > > +