| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20130709172456.GC9213@intel.com>
Date: Tue, 9 Jul 2013 10:24:57 -0700
From: Jubin Mehta <jubin.mehta@...el.com>
To: Dan Williams <djbw@...com>
Cc: Vinod Koul <vinod.koul@...el.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Dave Jiang <dave.jiang@...el.com>,
Jon Mason <jon.mason@...el.com>
Subject: Re: [PATCH v2] dmatest: masking tests for channel capabilities
On Wed, Jun 26, 2013 at 01:47:07PM -0700, Dan Williams wrote:
> On Fri, Jun 21, 2013 at 4:46 PM, Jubin Mehta <jubin.mehta@...el.com> wrote:
> > The current dmatest module tests all the hardware capabilities (MEMCPY, XOR
> > and PQ) supported by a particular DMA channel and these tests are performed
> > concurrently by default. This patch allows the user to enable or disable the
> > test performed for any particular capability. The mask bits for enabling the
> > tests are set using the debugfs.
> >
> > Signed-off-by: Jubin Mehta <jubin.mehta@...el.com>
> [..]
> > diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
> > index d8ce4ec..c152755 100644
> > --- a/drivers/dma/dmatest.c
> > +++ b/drivers/dma/dmatest.c
> > @@ -23,6 +23,7 @@
> > #include <linux/debugfs.h>
> > #include <linux/uaccess.h>
> > #include <linux/seq_file.h>
> > +#include <linux/string.h>
> >
> > static unsigned int test_buf_size = 16384;
> > module_param(test_buf_size, uint, S_IRUGO);
> > @@ -66,6 +67,11 @@ module_param(timeout, uint, S_IRUGO);
> > MODULE_PARM_DESC(timeout, "Transfer Timeout in msec (default: 3000), "
> > "Pass -1 for infinite timeout");
> >
> > +static char test_cap_mask[20] = "copy,xor,pq";
> > +module_param_string(cap_mask, test_cap_mask, sizeof(test_cap_mask), S_IRUGO);
>
> Let's just make this module parameter writable so we don't need to add
> a new debugfs file.
>
I shall do that. But, instead of having just the cap_mask parameter as
a writable module parameter, it would be great if we could have all
the module parameters writable and just have run file (to trigger the
test) and results file (to display the results of the test) in debugfs.
> The 'get' routine can also display the available testable operation
> types.
>
The testable operation types depend on the dma channel under test.
Thus, a 'get' routine cannot be used to obtain a default value for
the module parameter 'cap_mask'.
--
Jubin
> > +MODULE_PARM_DESC(cap_mask, "Mask bits for different capability test "
> > + "(default: copy,xor,pq)");
> > +
> > /* Maximum amount of mismatched bytes in buffer to print */
> > #define MAX_ERROR_COUNT 32
> >
> > @@ -164,6 +170,7 @@ struct dmatest_chan {
> > * @xor_sources: number of xor source buffers
> > * @pq_sources: number of p+q source buffers
> > * @timeout: transfer timeout in msec, -1 for infinite timeout
> > + * @cap_mask: mask for different hardware capabilities tests
> > */
> > struct dmatest_params {
> > unsigned int buf_size;
> > @@ -175,6 +182,7 @@ struct dmatest_params {
> > unsigned int xor_sources;
> > unsigned int pq_sources;
> > int timeout;
> > + char cap_mask[128];
>
> This can be the capability mask directly... see below:
>
> > };
> >
> > /**
> > @@ -883,11 +891,44 @@ static int dmatest_add_threads(struct dmatest_info *info,
> > return i;
> > }
> >
> > +static int dmatest_cap_check(struct dmatest_info *info,
> > + dma_cap_mask_t *test_cap_mask)
> > +{
> > + struct dmatest_params *params;
> > + char *cap;
> > + unsigned char c;
> > + int i;
> > +
> > + params = &info->params;
> > + cap = (params->cap_mask);
> > +
> > + for (i = 0; i < 3; i++) {
> > + if (!strncmp(cap, "copy", 4)) {
> > + dma_cap_set(DMA_MEMCPY, *test_cap_mask);
> > + cap += 4;
> > + } else if (!strncmp(cap, "xor", 3)) {
> > + dma_cap_set(DMA_XOR, *test_cap_mask);
> > + cap += 3;
> > + } else if (!strncmp(cap, "pq", 2)) {
> > + dma_cap_set(DMA_PQ, *test_cap_mask);
> > + cap += 2;
> > + } else {
> > + return -1;
> > + }
> > + c = *cap++;
> > + if (c != ',')
> > + break;
> > + }
> > + return 0;
> > +}
>
> We can run this routine when the capabilities are set to return error
> on invalid capability masks directly. Then we can set the
> test_cap_mask before dmatest_add_channel().
>
> > +
> > static int dmatest_add_channel(struct dmatest_info *info,
> > struct dma_chan *chan)
> > {
> > struct dmatest_chan *dtc;
> > struct dma_device *dma_dev = chan->device;
> > + struct dmatest_params *params;
> > + dma_cap_mask_t test_cap_mask;
> > unsigned int thread_count = 0;
> > int cnt;
> >
> > @@ -900,15 +941,28 @@ static int dmatest_add_channel(struct dmatest_info *info,
> > dtc->chan = chan;
> > INIT_LIST_HEAD(&dtc->threads);
> >
> > - if (dma_has_cap(DMA_MEMCPY, dma_dev->cap_mask)) {
> > + params = &info->params;
> > +
> > + dma_cap_zero(test_cap_mask);
> > + cnt = dmatest_cap_check(info, &test_cap_mask);
> > + if (cnt) {
> > + pr_warn("dmatest: Invalid Capability Mask Paramters\n");
> > + return -EINVAL;
> > + }
>
> As above I think this too late to be validating the parameters.
>
> > +
> > + /* Run dmatest threads for the supported and enabled capabilities */
> > + if (dma_has_cap(DMA_MEMCPY, dma_dev->cap_mask)
> > + && dma_has_cap(DMA_MEMCPY, test_cap_mask)) {
> > cnt = dmatest_add_threads(info, dtc, DMA_MEMCPY);
> > thread_count += cnt > 0 ? cnt : 0;
> > }
> > - if (dma_has_cap(DMA_XOR, dma_dev->cap_mask)) {
> > + if (dma_has_cap(DMA_XOR, dma_dev->cap_mask)
> > + && dma_has_cap(DMA_XOR, test_cap_mask)) {
> > cnt = dmatest_add_threads(info, dtc, DMA_XOR);
> > thread_count += cnt > 0 ? cnt : 0;
> > }
> > - if (dma_has_cap(DMA_PQ, dma_dev->cap_mask)) {
> > + if (dma_has_cap(DMA_PQ, dma_dev->cap_mask)
> > + && dma_has_cap(DMA_PQ, test_cap_mask)) {
> > cnt = dmatest_add_threads(info, dtc, DMA_PQ);
> > thread_count += cnt > 0 ? cnt : 0;
> > }
>
> Could above be written simpler with one bitmap_and(test_cap_mask,
> dma_dev->cap_mask, test_cap_mask) at the start and then
> for_each_set_bit(test_cap_mask)?
>
>
> --
> Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists