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

Ben Widawsky
2021-02-01T23:18:04Z

On 21-02-01 15:09:45, David Rientjes wrote:
> 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.

Well, I wouldn't hold your breath (it would be an errata in this case anyway).
My preference would be to just allow allow mailbox payload size to be 2^31 and
not deal with this.

My question was how strongly do you feel it's an error that should prevent
binding.