| Message ID | 165791932409.2491387.9065856569307593223.stgit@djiang5-desk3.ch.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce security commands for CXL pmem device | expand |
On Fri, 15 Jul 2022, Dave Jiang wrote: >Add context struct for mock device and move lsa under the context. This >allows additional information such as security status and other persistent >security data such as passphrase to be added for the emulated test device. > Reviewed-by: Davidlohr Bueso <dave@stgolabs.net> >Signed-off-by: Dave Jiang <dave.jiang@intel.com> >--- > tools/testing/cxl/test/mem.c | 29 +++++++++++++++++++++++------ > 1 file changed, 23 insertions(+), 6 deletions(-) > >diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c >index 6b9239b2afd4..723378248321 100644 >--- a/tools/testing/cxl/test/mem.c >+++ b/tools/testing/cxl/test/mem.c >@@ -9,6 +9,10 @@ > #include <linux/bits.h> > #include <cxlmem.h> > >+struct mock_mdev_data { >+ void *lsa; >+}; >+ > #define LSA_SIZE SZ_128K > #define EFFECT(x) (1U << x) > >@@ -140,7 +144,8 @@ static int mock_id(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd) > static int mock_get_lsa(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd) > { > struct cxl_mbox_get_lsa *get_lsa = cmd->payload_in; >- void *lsa = dev_get_drvdata(cxlds->dev); >+ struct mock_mdev_data *mdata = dev_get_drvdata(cxlds->dev); >+ void *lsa = mdata->lsa; > u32 offset, length; > > if (sizeof(*get_lsa) > cmd->size_in) >@@ -159,7 +164,8 @@ static int mock_get_lsa(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd) > static int mock_set_lsa(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd) > { > struct cxl_mbox_set_lsa *set_lsa = cmd->payload_in; >- void *lsa = dev_get_drvdata(cxlds->dev); >+ struct mock_mdev_data *mdata = dev_get_drvdata(cxlds->dev); >+ void *lsa = mdata->lsa; > u32 offset, length; > > if (sizeof(*set_lsa) > cmd->size_in) >@@ -237,9 +243,12 @@ static int cxl_mock_mbox_send(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd * > return rc; > } > >-static void label_area_release(void *lsa) >+static void cxl_mock_drvdata_release(void *data) > { >- vfree(lsa); >+ struct mock_mdev_data *mdata = data; >+ >+ vfree(mdata->lsa); >+ vfree(mdata); > } > > static int cxl_mock_mem_probe(struct platform_device *pdev) >@@ -247,13 +256,21 @@ static int cxl_mock_mem_probe(struct platform_device *pdev) > struct device *dev = &pdev->dev; > struct cxl_memdev *cxlmd; > struct cxl_dev_state *cxlds; >+ struct mock_mdev_data *mdata; > void *lsa; > int rc; > >+ mdata = vmalloc(sizeof(*mdata)); >+ if (!mdata) >+ return -ENOMEM; >+ > lsa = vmalloc(LSA_SIZE); >- if (!lsa) >+ if (!lsa) { >+ vfree(mdata); > return -ENOMEM; >- rc = devm_add_action_or_reset(dev, label_area_release, lsa); >+ } >+ >+ rc = devm_add_action_or_reset(dev, cxl_mock_drvdata_release, mdata); > if (rc) > return rc; > dev_set_drvdata(dev, lsa); >
On Fri, 15 Jul 2022 14:08:44 -0700 Dave Jiang <dave.jiang@intel.com> wrote: > Add context struct for mock device and move lsa under the context. This > allows additional information such as security status and other persistent > security data such as passphrase to be added for the emulated test device. > > Signed-off-by: Dave Jiang <dave.jiang@intel.com> > --- > tools/testing/cxl/test/mem.c | 29 +++++++++++++++++++++++------ > 1 file changed, 23 insertions(+), 6 deletions(-) > > diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c > index 6b9239b2afd4..723378248321 100644 > --- a/tools/testing/cxl/test/mem.c > +++ b/tools/testing/cxl/test/mem.c > @@ -9,6 +9,10 @@ > #include <linux/bits.h> > #include <cxlmem.h> > > +struct mock_mdev_data { > + void *lsa; > +}; > + > #define LSA_SIZE SZ_128K > #define EFFECT(x) (1U << x) > > @@ -140,7 +144,8 @@ static int mock_id(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd) > static int mock_get_lsa(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd) > { > struct cxl_mbox_get_lsa *get_lsa = cmd->payload_in; > - void *lsa = dev_get_drvdata(cxlds->dev); > + struct mock_mdev_data *mdata = dev_get_drvdata(cxlds->dev); > + void *lsa = mdata->lsa; > u32 offset, length; > > if (sizeof(*get_lsa) > cmd->size_in) > @@ -159,7 +164,8 @@ static int mock_get_lsa(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd) > static int mock_set_lsa(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd) > { > struct cxl_mbox_set_lsa *set_lsa = cmd->payload_in; > - void *lsa = dev_get_drvdata(cxlds->dev); > + struct mock_mdev_data *mdata = dev_get_drvdata(cxlds->dev); > + void *lsa = mdata->lsa; > u32 offset, length; > > if (sizeof(*set_lsa) > cmd->size_in) > @@ -237,9 +243,12 @@ static int cxl_mock_mbox_send(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd * > return rc; > } > > -static void label_area_release(void *lsa) > +static void cxl_mock_drvdata_release(void *data) > { > - vfree(lsa); > + struct mock_mdev_data *mdata = data; > + > + vfree(mdata->lsa); > + vfree(mdata); > } > > static int cxl_mock_mem_probe(struct platform_device *pdev) > @@ -247,13 +256,21 @@ static int cxl_mock_mem_probe(struct platform_device *pdev) > struct device *dev = &pdev->dev; > struct cxl_memdev *cxlmd; > struct cxl_dev_state *cxlds; > + struct mock_mdev_data *mdata; > void *lsa; > int rc; > > + mdata = vmalloc(sizeof(*mdata)); It's tiny so why vmalloc? I guess that might become apparent later. devm_kzalloc() should be fine and lead to simpler error handling. > + if (!mdata) > + return -ENOMEM; > + > lsa = vmalloc(LSA_SIZE); > - if (!lsa) > + if (!lsa) { > + vfree(mdata); In general doing this just makes things fragile in the long term. Better to register one devm_add_action_or_reset() for each thing set up (or standard allcoation). > return -ENOMEM; > - rc = devm_add_action_or_reset(dev, label_area_release, lsa); > + } > + > + rc = devm_add_action_or_reset(dev, cxl_mock_drvdata_release, mdata); > if (rc) > return rc; > dev_set_drvdata(dev, lsa); > >
On 8/3/2022 9:36 AM, Jonathan Cameron wrote: > On Fri, 15 Jul 2022 14:08:44 -0700 > Dave Jiang <dave.jiang@intel.com> wrote: > >> Add context struct for mock device and move lsa under the context. This >> allows additional information such as security status and other persistent >> security data such as passphrase to be added for the emulated test device. >> >> Signed-off-by: Dave Jiang <dave.jiang@intel.com> >> --- >> tools/testing/cxl/test/mem.c | 29 +++++++++++++++++++++++------ >> 1 file changed, 23 insertions(+), 6 deletions(-) >> >> diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c >> index 6b9239b2afd4..723378248321 100644 >> --- a/tools/testing/cxl/test/mem.c >> +++ b/tools/testing/cxl/test/mem.c >> @@ -9,6 +9,10 @@ >> #include <linux/bits.h> >> #include <cxlmem.h> >> >> +struct mock_mdev_data { >> + void *lsa; >> +}; >> + >> #define LSA_SIZE SZ_128K >> #define EFFECT(x) (1U << x) >> >> @@ -140,7 +144,8 @@ static int mock_id(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd) >> static int mock_get_lsa(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd) >> { >> struct cxl_mbox_get_lsa *get_lsa = cmd->payload_in; >> - void *lsa = dev_get_drvdata(cxlds->dev); >> + struct mock_mdev_data *mdata = dev_get_drvdata(cxlds->dev); >> + void *lsa = mdata->lsa; >> u32 offset, length; >> >> if (sizeof(*get_lsa) > cmd->size_in) >> @@ -159,7 +164,8 @@ static int mock_get_lsa(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd) >> static int mock_set_lsa(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd) >> { >> struct cxl_mbox_set_lsa *set_lsa = cmd->payload_in; >> - void *lsa = dev_get_drvdata(cxlds->dev); >> + struct mock_mdev_data *mdata = dev_get_drvdata(cxlds->dev); >> + void *lsa = mdata->lsa; >> u32 offset, length; >> >> if (sizeof(*set_lsa) > cmd->size_in) >> @@ -237,9 +243,12 @@ static int cxl_mock_mbox_send(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd * >> return rc; >> } >> >> -static void label_area_release(void *lsa) >> +static void cxl_mock_drvdata_release(void *data) >> { >> - vfree(lsa); >> + struct mock_mdev_data *mdata = data; >> + >> + vfree(mdata->lsa); >> + vfree(mdata); >> } >> >> static int cxl_mock_mem_probe(struct platform_device *pdev) >> @@ -247,13 +256,21 @@ static int cxl_mock_mem_probe(struct platform_device *pdev) >> struct device *dev = &pdev->dev; >> struct cxl_memdev *cxlmd; >> struct cxl_dev_state *cxlds; >> + struct mock_mdev_data *mdata; >> void *lsa; >> int rc; >> >> + mdata = vmalloc(sizeof(*mdata)); > It's tiny so why vmalloc? I guess that might become apparent later. > devm_kzalloc() should be fine and lead to simpler error handling. In my testing I actually realized that this needs to be part of platform data in order for the contents to be "persistent" even the driver is unloaded. So this allocation has moved to cxl_test_init() and managed via platform_device_add_data(). And the function makes a copy of the passed in data rather than taking it as is and that is managed with the platform device lifetime. > >> + if (!mdata) >> + return -ENOMEM; >> + >> lsa = vmalloc(LSA_SIZE); >> - if (!lsa) >> + if (!lsa) { >> + vfree(mdata); > In general doing this just makes things fragile in the long term. Better to > register one devm_add_action_or_reset() for each thing set up (or standard > allcoation). > >> return -ENOMEM; >> - rc = devm_add_action_or_reset(dev, label_area_release, lsa); >> + } >> + >> + rc = devm_add_action_or_reset(dev, cxl_mock_drvdata_release, mdata); >> if (rc) >> return rc; >> dev_set_drvdata(dev, lsa); >> >>
diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c index 6b9239b2afd4..723378248321 100644 --- a/tools/testing/cxl/test/mem.c +++ b/tools/testing/cxl/test/mem.c @@ -9,6 +9,10 @@ #include <linux/bits.h> #include <cxlmem.h> +struct mock_mdev_data { + void *lsa; +}; + #define LSA_SIZE SZ_128K #define EFFECT(x) (1U << x) @@ -140,7 +144,8 @@ static int mock_id(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd) static int mock_get_lsa(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd) { struct cxl_mbox_get_lsa *get_lsa = cmd->payload_in; - void *lsa = dev_get_drvdata(cxlds->dev); + struct mock_mdev_data *mdata = dev_get_drvdata(cxlds->dev); + void *lsa = mdata->lsa; u32 offset, length; if (sizeof(*get_lsa) > cmd->size_in) @@ -159,7 +164,8 @@ static int mock_get_lsa(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd) static int mock_set_lsa(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd) { struct cxl_mbox_set_lsa *set_lsa = cmd->payload_in; - void *lsa = dev_get_drvdata(cxlds->dev); + struct mock_mdev_data *mdata = dev_get_drvdata(cxlds->dev); + void *lsa = mdata->lsa; u32 offset, length; if (sizeof(*set_lsa) > cmd->size_in) @@ -237,9 +243,12 @@ static int cxl_mock_mbox_send(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd * return rc; } -static void label_area_release(void *lsa) +static void cxl_mock_drvdata_release(void *data) { - vfree(lsa); + struct mock_mdev_data *mdata = data; + + vfree(mdata->lsa); + vfree(mdata); } static int cxl_mock_mem_probe(struct platform_device *pdev) @@ -247,13 +256,21 @@ static int cxl_mock_mem_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct cxl_memdev *cxlmd; struct cxl_dev_state *cxlds; + struct mock_mdev_data *mdata; void *lsa; int rc; + mdata = vmalloc(sizeof(*mdata)); + if (!mdata) + return -ENOMEM; + lsa = vmalloc(LSA_SIZE); - if (!lsa) + if (!lsa) { + vfree(mdata); return -ENOMEM; - rc = devm_add_action_or_reset(dev, label_area_release, lsa); + } + + rc = devm_add_action_or_reset(dev, cxl_mock_drvdata_release, mdata); if (rc) return rc; dev_set_drvdata(dev, lsa);
Add context struct for mock device and move lsa under the context. This allows additional information such as security status and other persistent security data such as passphrase to be added for the emulated test device. Signed-off-by: Dave Jiang <dave.jiang@intel.com> --- tools/testing/cxl/test/mem.c | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-)