Re: [PATCH] cxl/mbox: Do not allow immediate mode in SET_PARTITION_INFO

[Alison Schofield <alison.schofield@intel.com>]

Thanks for the review...
On Tue, Jan 04, 2022 at 11:02:19AM -0800, Dan Williams wrote:
> On Mon, Jan 3, 2022 at 12:15 PM <alison.schofield@intel.com> wrote:
> >
> > From: Alison Schofield <alison.schofield@intel.com>
> >
> > User space may send the SET_PARTITION_INFO mailbox command using
> > the IOCTL interface. Inspect the input payload and fail if the
> > immediate flag is set.
> >
> > This is the first instance of the driver inspecting an input payload
> > from user space. Assume there will be more such cases and implement
> > with an extensible helper.
> >
> > Note: At this time immediate partitioning is not allowed because the
> > kernel will need to react immediately to this configuration change
> > and that support is not yet implemented.
> 
> I would say:
> 
> In order for the kernel to react to an immediate partition change it
> needs to assert that the change will not affect any active decode. At
> a minimum this requires validating that the device is using HDM
> decoders instead of the CXL DVSEC for decode, and that none of the
> active HDM decoders are affected by the partition change. For now,
> just fail until that support arrives.
> 
Got it! 
> >

snip

> > +static bool cxl_payload_from_user_allowed(u16 opcode, void *payload_in)
> > +{
> > +       switch (opcode) {
> > +       case CXL_MBOX_OP_SET_PARTITION_INFO: {
> > +               struct cxl_mbox_set_partition_info *pi;
> > +
> > +               pi = (struct cxl_mbox_set_partition_info *)payload_in;
> 
> No need for a cast since @payload_in is a 'void *'
> 
Got it!

> > +               if (pi->flags && CXL_SET_PARTITION_IMMEDIATE_FLAG)
> > +                       return false;
> > +               break;
> > +       }
> > +       default:
> > +               break;
> > +       }
> > +       return true;
> > +}
> > +

snip

> > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> > index 8d96d009ad90..e10b86f06c75 100644
> > --- a/drivers/cxl/cxlmem.h
> > +++ b/drivers/cxl/cxlmem.h
> > @@ -231,6 +231,13 @@ struct cxl_mbox_set_lsa {
> >         u8 data[];
> >  } __packed;
> >
> > +struct cxl_mbox_set_partition_info {
> > +       u64 volatile_capacity;
> 
> This should be __le64, and it looks like 'struct cxl_mbox_get_lsa' and
> 'struct cxl_mbox_set_lsa' need similar fixups.
> 

Got it and will do!


> > +       u8 flags;
> > +} __packed;
> > +
> > +#define  CXL_SET_PARTITION_IMMEDIATE_FLAG      BIT(0)
> > +
> >  /**
> >   * struct cxl_mem_command - Driver representation of a memory device command
> >   * @info: Command information as it exists for the UAPI
> >
> > base-commit: 53989fad1286e652ea3655ae3367ba698da8d2ff
> > --
> > 2.31.1
> >