Re: [PATCH 03/14] cxl/mem: Find device capabilities

From: David Rientjes
Date: Mon Feb 01 2021 - 18:10:39 EST


On Mon, 1 Feb 2021, Ben Widawsky wrote:

> > I think that's what 8.2.8.4.3 says, no? And then 8.2.8.4.5 says you
> > can use up to Payload Size. That's why my recommendation was to enforce
> > this in cxl_mem_setup_mailbox() up front.
>
> Yeah. I asked our spec people to update 8.2.8.4.5 to make it clearer. I'd argue
> the intent is how you describe it, but the implementation isn't.
>
> My argument was silly anyway because if you specify greater than 1M as your
> payload, you will get EINVAL at the ioctl.
>
> The value of how it works today is the driver will at least bind and allow you
> to interact with it.
>
> How strongly do you feel about this?
>

I haven't seen the update to 8.2.8.4.5 to know yet :)

You make a good point of at least being able to interact with the driver.
I think you could argue that if the driver binds, then the payload size is
accepted, in which case it would be strange to get an EINVAL when using
the ioctl with anything >1MB.

Concern was that if we mask off the reserved bits from the command
register that we could be masking part of the payload size that is being
passed if the accepted max is >1MB. Idea was to avoid any possibility of
this inconsistency. If this is being checked for ioctl, seems like it's
checking reserved bits.

But maybe I should just wait for the spec update.