MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date:   Mon, 15 Nov 2021 12:17:23 +0100
From:   Michael Walle <michael@walle.cc>
To:     ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>
Cc:     horia.geanta@nxp.com, pankaj.gupta@nxp.com,
        herbert@gondor.apana.org.au, davem@davemloft.net,
        iuliana.prodan@nxp.com, linux-crypto@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] crypto: caam - check jr permissions before probing
In-Reply-To: <AM6PR06MB46914ECA12815ABFCE8C1A59A6989@AM6PR06MB4691.eurprd06.prod.outlook.com>
References: <20211104162114.2019509-1-andrey.zhizhikin@leica-geosystems.com>
 <20211111164601.13135-1-andrey.zhizhikin@leica-geosystems.com>
 <20211111164601.13135-3-andrey.zhizhikin@leica-geosystems.com>
 <e8456cc0bb9e4fc306e8188c5bd666d0@walle.cc>
 <AM6PR06MB46914ECA12815ABFCE8C1A59A6989@AM6PR06MB4691.eurprd06.prod.outlook.com>
User-Agent: Roundcube Webmail/1.4.11
Message-ID: <2070c2062606821fbf6ed052e6864255@walle.cc>
Precedence: bulk

>> > +EXPORT_SYMBOL(caam_ctrl_check_jr_perm);
>> 
>> no need for exporting this, no?
> 
> Unfortunately, both CONFIG_CRYPTO_DEV_FSL_CAAM and
> CONFIG_CRYPTO_DEV_FSL_CAAM_JR are tristate. Setting both
> config options to "=m" fails to resolve caam_ctrl_check_jr_perm,
> therefore I had to export it.
> 
> It strikes me odd however that CAAM can be compiled as module
> without CAAM_JR module at all. This would imply that DECO is used
> directly, which according to SRM is used for pure descriptor debug
> purposes and should never be used in production.
> 
> I guess CRYPTO_DEV_FSL_CAAM _JR should be merged into
> CRYPTO_DEV_FSL_CAAM, so they at least comes together. In that
> case the export would not be necessary at all.
> 
> I must admit I didn't find this a good solution, therefore any advise
> on a better solution here is highly appreciated.

I see, and I'm too lazy at the moment to figure that out ;) but afaik
new exports should be only EXPORT_SYMBOL_GPL().

>> >       if ((rd_reg32(&jrp->rregs->jrintstatus) & JRINT_ERR_HALT_MASK) !=
>> >           JRINT_ERR_HALT_COMPLETE || timeout == 0) {
>> > -             dev_err(dev, "failed to flush job ring %d\n", jrp->ridx);
>> > +             dev_err(dev, "failed to flush job ring %x\n",
>> > + jrp->ridx);
>> 
>> mh? why changing this?
> 
> After the change, jrp->ridx would contain JR hex address instead of 
> index,
> therefore I had to replace the debug output.

ahh then, ridx should renamed accordingly, I suppose.

-michael