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.
