Re: [PATCH v2 04/10] cxl/pci: Use synchronous API for DOE

[Lukas Wunner <lukas@wunner.de>]

On Fri, Feb 03, 2023 at 04:53:34PM +0800, Li, Ming wrote:
> On 1/24/2023 8:52 AM, Ira Weiny wrote:
> > Lukas Wunner wrote:
> > >  static int cxl_cdat_get_length(struct device *dev,
> > >  			       struct pci_doe_mb *cdat_doe,
> > >  			       size_t *length)
> > >  {
> > > -	DECLARE_CDAT_DOE_TASK(CDAT_DOE_REQ(0), t);
> > > +	u32 request = CDAT_DOE_REQ(0);
> > > +	u32 response[32];
> > >  	int rc;
> > >  
> > > -	rc = pci_doe_submit_task(cdat_doe, &t.task);
> > > +	rc = pci_doe(cdat_doe, PCI_DVSEC_VENDOR_ID_CXL,
> > > +		     CXL_DOE_PROTOCOL_TABLE_ACCESS,
> > > +		     &request, sizeof(request),
> > > +		     &response, sizeof(response));
> > >  	if (rc < 0) {
> > > -		dev_err(dev, "DOE submit failed: %d", rc);
> > > +		dev_err(dev, "DOE failed: %d", rc);
> > >  		return rc;
> > >  	}
> > > -	wait_for_completion(&t.c);
> > > -	if (t.task.rv < sizeof(u32))
> > > +	if (rc < sizeof(u32))
> > >  		return -EIO;
> > >  
> 
> Sorry, I didn't find the original patchset email, only can reply here.
> Should this "if (rc < sizeof(u32))" be "if (rc < 2 * sizeof(u32))"?
> Because below code used response[1] directly, that means we need unless
> two u32 in response payload.

Yes I spotted that as well, there's already a fixup on my
development branch:

  https://github.com/l1k/linux/commits/doe

It's in commit "cxl/pci: Handle truncated CDAT header" which is:

  https://github.com/l1k/linux/commit/208f256b319b

...but that URL may stop working as soon as I rebase the next time.

Actually there's a lot more broken here, there are 3 other new fixup
patches at the beginning of that development branch.

Thanks,

Lukas