On Mon, Feb 1, 2021 at 1:51 PM David Rientjes <rientjes@xxxxxxxxxx> wrote: > > On Mon, 1 Feb 2021, Ben Widawsky wrote: > > > On 21-01-30 15:51:49, David Rientjes wrote: > > > On Fri, 29 Jan 2021, Ben Widawsky wrote: > > > > > > > +static int cxl_mem_setup_mailbox(struct cxl_mem *cxlm) > > > > +{ > > > > + const int cap = cxl_read_mbox_reg32(cxlm, CXLDEV_MB_CAPS_OFFSET); > > > > + > > > > + cxlm->mbox.payload_size = > > > > + 1 << CXL_GET_FIELD(cap, CXLDEV_MB_CAP_PAYLOAD_SIZE); > > > > + > > > > + /* 8.2.8.4.3 */ > > > > + if (cxlm->mbox.payload_size < 256) { > > > > + dev_err(&cxlm->pdev->dev, "Mailbox is too small (%zub)", > > > > + cxlm->mbox.payload_size); > > > > + return -ENXIO; > > > > + } > > > > > > Any reason not to check cxlm->mbox.payload_size > (1 << 20) as well and > > > return ENXIO if true? > > > > If some crazy vendor wanted to ship a mailbox larger than 1M, why should the > > driver not allow it? > > > > Because the spec disallows it :) Unless it causes an operational failure in practice I'd go with the Robustness Principle and be liberal in accepting hardware geometries.