Re: [PATCH 08/12] hwrng: bcm2835-rng: Abstract I/O accessors
On 11/03/2017 01:19 PM, Eric Anholt wrote:
> Florian Fainelli writes:
>
>> In preparation for allowing BCM63xx to use this driver, we abstract I/O
>> accessors such that we can easily change those later on.
>>
>> Signed-off-by: Florian Fainelli
>> ---
>> drivers/char/hw_random/bcm2835-rng.c | 27 +++
>> 1 file changed, 19 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/char/hw_random/bcm2835-rng.c
>> b/drivers/char/hw_random/bcm2835-rng.c
>> index 35928efb52e7..500275d55044 100644
>> --- a/drivers/char/hw_random/bcm2835-rng.c
>> +++ b/drivers/char/hw_random/bcm2835-rng.c
>> @@ -42,6 +42,17 @@ static inline struct bcm2835_rng_priv *to_rng_priv(struct
>> hwrng *rng)
>> return container_of(rng, struct bcm2835_rng_priv, rng);
>> }
>>
>> +static inline u32 rng_readl(struct bcm2835_rng_priv *priv, u32 offset)
>> +{
>> +return readl(priv->base + offset);
>> +}
>> +
>> +static inline void rng_writel(struct bcm2835_rng_priv *priv, u32 val,
>> + u32 offset)
>> +{
>> +writel(val, priv->base + offset);
>> +}
>> +
>> static int bcm2835_rng_read(struct hwrng *rng, void *buf, size_t max,
>> bool wait)
>> {
>> @@ -49,18 +60,18 @@ static int bcm2835_rng_read(struct hwrng *rng, void
>> *buf, size_t max,
>> u32 max_words = max / sizeof(u32);
>> u32 num_words, count;
>>
>> -while ((__raw_readl(priv->base + RNG_STATUS) >> 24) == 0) {
>> +while ((rng_readl(priv, RNG_STATUS) >> 24) == 0) {
>> if (!wait)
>> return 0;
>> cpu_relax();
>> }
>
> What was the difference between the __raw_readl and readl that's now
> being done in the new call? Is it important?
readl() on ARM contains a memory barrier, which has therefore stronger
ordering guarantees than __raw_readl() which does not.
In practice I don't think this makes a whole lot of difference in that
the above loop does not even have a barrier outside of it to try to have
any sort of ordering guarantee so it seems to me like this may be an
oversight.
I took the liberty to use the stronger operation here because it seems
to me like this is what is desired, or at least won't cause functional
problems, and because I am not intimately familiar with the 2835 busing
architecture. I know for a thing that the Broadcom STB and DSL busses
(named GISB and UBUS respectively) do not require such barriers since
they do not re-order transactions and are non-posted.
>
>> /* set warm-up count & enable */
>> -__raw_writel(RNG_WARMUP_COUNT, priv->base + RNG_STATUS);
>> -__raw_writel(RNG_RBGEN, priv->base + RNG_CTRL);
>> +rng_writel(priv, RNG_WARMUP_COUNT, RNG_STATUS);
>> +rng_writel(priv, RNG_RBGEN, RNG_CTRL);
>
> Similar question.
And here we definitively are not in a hot-path so the more "ordered"
variant is acceptable it seems.
--
Florian
Re: [PATCH 12/12] hwrng: bcm63xx-rng: Remove since bcm2835-rng takes over
On 11/03/2017 01:18 PM, Eric Anholt wrote:
> Florian Fainelli writes:
>
>> bcm2835-rng is now capable of supporting the BCM63xx hardware, so remove
>> the driver which duplicates the same functionality.
>>
>> Signed-off-by: Florian Fainelli
>> ---
>> drivers/char/hw_random/Kconfig | 13 ---
>> drivers/char/hw_random/Makefile | 1 -
>> drivers/char/hw_random/bcm63xx-rng.c | 154
>> ---
>> 3 files changed, 168 deletions(-)
>> delete mode 100644 drivers/char/hw_random/bcm63xx-rng.c
>>
>
>> diff --git a/drivers/char/hw_random/bcm63xx-rng.c
>> b/drivers/char/hw_random/bcm63xx-rng.c
>> deleted file mode 100644
>> index 5132c9cde50d..
>> --- a/drivers/char/hw_random/bcm63xx-rng.c
>> +++ /dev/null
>
>> -static int bcm63xx_rng_data_present(struct hwrng *rng, int wait)
>> -{
>> -struct bcm63xx_rng_priv *priv = to_rng_priv(rng);
>> -
>> -return __raw_readl(priv->regs + RNG_STAT) & RNG_AVAIL_MASK;
>> -}
>
> It looks like this method isn't in the 2835 implementation. Should it
> get ported over?
The read method already has something similar in that it busy loops
until it gets some random words available, which is why I did not port
it over.
--
Florian
Re: [PATCH 08/12] hwrng: bcm2835-rng: Abstract I/O accessors
Florian Fainelli writes:
> In preparation for allowing BCM63xx to use this driver, we abstract I/O
> accessors such that we can easily change those later on.
>
> Signed-off-by: Florian Fainelli
> ---
> drivers/char/hw_random/bcm2835-rng.c | 27 +++
> 1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/char/hw_random/bcm2835-rng.c
> b/drivers/char/hw_random/bcm2835-rng.c
> index 35928efb52e7..500275d55044 100644
> --- a/drivers/char/hw_random/bcm2835-rng.c
> +++ b/drivers/char/hw_random/bcm2835-rng.c
> @@ -42,6 +42,17 @@ static inline struct bcm2835_rng_priv *to_rng_priv(struct
> hwrng *rng)
> return container_of(rng, struct bcm2835_rng_priv, rng);
> }
>
> +static inline u32 rng_readl(struct bcm2835_rng_priv *priv, u32 offset)
> +{
> + return readl(priv->base + offset);
> +}
> +
> +static inline void rng_writel(struct bcm2835_rng_priv *priv, u32 val,
> + u32 offset)
> +{
> + writel(val, priv->base + offset);
> +}
> +
> static int bcm2835_rng_read(struct hwrng *rng, void *buf, size_t max,
> bool wait)
> {
> @@ -49,18 +60,18 @@ static int bcm2835_rng_read(struct hwrng *rng, void *buf,
> size_t max,
> u32 max_words = max / sizeof(u32);
> u32 num_words, count;
>
> - while ((__raw_readl(priv->base + RNG_STATUS) >> 24) == 0) {
> + while ((rng_readl(priv, RNG_STATUS) >> 24) == 0) {
> if (!wait)
> return 0;
> cpu_relax();
> }
What was the difference between the __raw_readl and readl that's now
being done in the new call? Is it important?
> /* set warm-up count & enable */
> - __raw_writel(RNG_WARMUP_COUNT, priv->base + RNG_STATUS);
> - __raw_writel(RNG_RBGEN, priv->base + RNG_CTRL);
> + rng_writel(priv, RNG_WARMUP_COUNT, RNG_STATUS);
> + rng_writel(priv, RNG_RBGEN, RNG_CTRL);
Similar question.
signature.asc
Description: PGP signature
Re: [PATCH 12/12] hwrng: bcm63xx-rng: Remove since bcm2835-rng takes over
Florian Fainelli writes:
> bcm2835-rng is now capable of supporting the BCM63xx hardware, so remove
> the driver which duplicates the same functionality.
>
> Signed-off-by: Florian Fainelli
> ---
> drivers/char/hw_random/Kconfig | 13 ---
> drivers/char/hw_random/Makefile | 1 -
> drivers/char/hw_random/bcm63xx-rng.c | 154
> ---
> 3 files changed, 168 deletions(-)
> delete mode 100644 drivers/char/hw_random/bcm63xx-rng.c
>
> diff --git a/drivers/char/hw_random/bcm63xx-rng.c
> b/drivers/char/hw_random/bcm63xx-rng.c
> deleted file mode 100644
> index 5132c9cde50d..
> --- a/drivers/char/hw_random/bcm63xx-rng.c
> +++ /dev/null
> -static int bcm63xx_rng_data_present(struct hwrng *rng, int wait)
> -{
> - struct bcm63xx_rng_priv *priv = to_rng_priv(rng);
> -
> - return __raw_readl(priv->regs + RNG_STAT) & RNG_AVAIL_MASK;
> -}
It looks like this method isn't in the 2835 implementation. Should it
get ported over?
signature.asc
Description: PGP signature
Re: [Part2 PATCH v7 18/38] crypto: ccp: Implement SEV_PEK_CSR ioctl command
On 11/3/17 2:42 PM, Borislav Petkov wrote:
...
>> +if (psp_master->sev_state == SEV_STATE_UNINIT) {
>> +ret = __sev_platform_init_locked(psp_master->sev_init,
>> &argp->error);
> Right, you're passing psp_master->sev_init (or whatever you're going to
> end up calling it) down but then in __sev_platform_init_locked() you end
> up doing
>
> if (!data)
> data = &psp->cmd_buf;
>
> which looks silly.
>
> IOW, if not really required, __sev_platform_init_locked() could have
> only one argument instead:
>
> static int __sev_platform_init_locked(int *error)
The cmd_buf argument can be remove. I am have not looked at SEV-ES but
it may be possible that during SEV-ES patches kvm driver may need to
call the sev_platform_init() with different cmd_buf to initialize SEV-ES
states. I can removed it in next version.
-Brijesh
Re: [Part2 PATCH v7 18/38] crypto: ccp: Implement SEV_PEK_CSR ioctl command
On Wed, Nov 01, 2017 at 04:16:03PM -0500, Brijesh Singh wrote:
> The SEV_PEK_CSR command can be used to generate a PEK certificate
> signing request. The command is defined in SEV spec section 5.7.
>
> Cc: Paolo Bonzini
> Cc: "Radim Krčmář"
> Cc: Borislav Petkov
> Cc: Herbert Xu
> Cc: Gary Hook
> Cc: Tom Lendacky
> Cc: linux-crypto@vger.kernel.org
> Cc: k...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Improvements-by: Borislav Petkov
> Signed-off-by: Brijesh Singh
> Acked-by: Gary R Hook
> ---
> drivers/crypto/ccp/psp-dev.c | 68
>
> 1 file changed, 68 insertions(+)
>
> diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
> index 42991c2e9085..4e2f9d037f0a 100644
> --- a/drivers/crypto/ccp/psp-dev.c
> +++ b/drivers/crypto/ccp/psp-dev.c
> @@ -298,6 +298,71 @@ static int sev_ioctl_do_pek_pdh_gen(int cmd, struct
> sev_issue_cmd *argp)
> return __sev_do_cmd_locked(cmd, 0, &argp->error);
> }
>
> +static int sev_ioctl_do_pek_csr(struct sev_issue_cmd *argp)
> +{
> + struct sev_user_data_pek_csr input;
> + struct sev_data_pek_csr *data;
> + void *blob = NULL;
> + int ret;
> +
> + if (copy_from_user(&input, (void __user *)argp->data, sizeof(input)))
> + return -EFAULT;
> +
> + data = kzalloc(sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + /* userspace wants to query CSR length */
> + if (!input.address || !input.length)
> + goto cmd;
> +
> + /* allocate a physically contiguous buffer to store the CSR blob */
> + if (!access_ok(VERIFY_WRITE, input.address, input.length) ||
> + input.length > SEV_FW_BLOB_MAX_SIZE) {
> + ret = -EFAULT;
> + goto e_free;
> + }
> +
> + blob = kmalloc(input.length, GFP_KERNEL);
> + if (!blob) {
> + ret = -ENOMEM;
> + goto e_free;
> + }
> +
> + data->address = __psp_pa(blob);
> + data->len = input.length;
> +
> +cmd:
> + if (psp_master->sev_state == SEV_STATE_UNINIT) {
> + ret = __sev_platform_init_locked(psp_master->sev_init,
> &argp->error);
Right, you're passing psp_master->sev_init (or whatever you're going to
end up calling it) down but then in __sev_platform_init_locked() you end
up doing
if (!data)
data = &psp->cmd_buf;
which looks silly.
IOW, if not really required, __sev_platform_init_locked() could have
only one argument instead:
static int __sev_platform_init_locked(int *error)
unless some later patch needs to pass in a different command buffer. I
guess I need to keep on reviewing...
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
[PATCH] crypto: chcr - Replace _manual_ swap with swap macro
Make use of the swap macro and remove unnecessary variable temp.
This makes the code easier to read and maintain.
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva
---
drivers/crypto/chelsio/chcr_algo.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/crypto/chelsio/chcr_algo.c
b/drivers/crypto/chelsio/chcr_algo.c
index 936bdd8..4b508cb 100644
--- a/drivers/crypto/chelsio/chcr_algo.c
+++ b/drivers/crypto/chelsio/chcr_algo.c
@@ -1469,11 +1469,8 @@ static int chcr_ahash_update(struct ahash_request *req)
return -ENOMEM;
if (remainder) {
- u8 *temp;
/* Swap buffers */
- temp = req_ctx->reqbfr;
- req_ctx->reqbfr = req_ctx->skbfr;
- req_ctx->skbfr = temp;
+ swap(req_ctx->reqbfr, req_ctx->skbfr);
sg_pcopy_to_buffer(req->src, sg_nents(req->src),
req_ctx->reqbfr, remainder, req->nbytes -
remainder);
--
2.7.4
Business
I have a rather important business proposal for you, I shall provide you with details upon your response to my email.
Re: [cryptodev:master 125/166] FATAL: drivers/crypto/marvell/marvell-cesa: struct platform_device_id is not terminated with a NULL entry!
Hi Herbert, On Sat, 4 Nov 2017 00:38:26 +0800 kbuild test robot wrote: > tree: > https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git > master > head: 7a373fd74a8d1c4882e0236cc38345cec1393505 > commit: 7b0c3d693ce65900dd3c79766185f539fa37a29a [125/166] crypto: marvell - > Add a platform_device_id table > config: arm-multi_v7_defconfig (attached as .config) > compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705 > reproduce: > wget > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O > ~/bin/make.cross > chmod +x ~/bin/make.cross > git checkout 7b0c3d693ce65900dd3c79766185f539fa37a29a > # save the attached .config to linux build tree > make.cross ARCH=arm > > All errors (new ones prefixed by >>): > >drivers/crypto/marvell/marvell-cesa: struct platform_device_id is 24 > bytes. The last of 1 is: >0x6d 0x76 0x5f 0x63 0x72 0x79 0x70 0x74 0x6f 0x00 0x00 0x00 0x00 0x00 0x00 > 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 > >> FATAL: drivers/crypto/marvell/marvell-cesa: struct platform_device_id is > >> not terminated with a NULL entry! > Just sent a patch fixing the problem. Sorry for the mess. Regards, Boris > --- > 0-DAY kernel test infrastructureOpen Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation
[PATCH] crypto: marvell - Add a NULL entry at the end of mv_cesa_plat_id_table[]
struct platform_device_id should be NULL terminated to let the core detect
where the last entry is.
Fixes: 07c50a8be41a ("crypto: marvell - Add a platform_device_id table")
Signed-off-by: Boris Brezillon
---
drivers/crypto/marvell/cesa.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/crypto/marvell/cesa.c b/drivers/crypto/marvell/cesa.c
index 577dc6773fa0..6dabb261bac4 100644
--- a/drivers/crypto/marvell/cesa.c
+++ b/drivers/crypto/marvell/cesa.c
@@ -594,6 +594,7 @@ static int mv_cesa_remove(struct platform_device *pdev)
static const struct platform_device_id mv_cesa_plat_id_table[] = {
{ .name = "mv_crypto" },
+ { /* sentinel */ },
};
MODULE_DEVICE_TABLE(platform, mv_cesa_plat_id_table);
--
2.11.0
[cryptodev:master 125/166] FATAL: drivers/crypto/marvell/marvell-cesa: struct platform_device_id is not terminated with a NULL entry!
tree: https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master head: 7a373fd74a8d1c4882e0236cc38345cec1393505 commit: 7b0c3d693ce65900dd3c79766185f539fa37a29a [125/166] crypto: marvell - Add a platform_device_id table config: arm-multi_v7_defconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout 7b0c3d693ce65900dd3c79766185f539fa37a29a # save the attached .config to linux build tree make.cross ARCH=arm All errors (new ones prefixed by >>): drivers/crypto/marvell/marvell-cesa: struct platform_device_id is 24 bytes. The last of 1 is: 0x6d 0x76 0x5f 0x63 0x72 0x79 0x70 0x74 0x6f 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 >> FATAL: drivers/crypto/marvell/marvell-cesa: struct platform_device_id is not >> terminated with a NULL entry! --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: virtio:rng: Virtio RNG devices need to be re-registered after suspend/resume
Jim,
Minor comment below.
On 3 November 2017 at 15:27, Jim Quigley wrote:
> The patch for
>
> commit: 5c06273401f2eb7b290cadbae18ee00f8f65e893
> Author: Amit Shah
> Date: Sun Jul 27 07:34:01 2014 +0930
>
> virtio: rng: delay hwrng_register() till driver is ready
>
> moved the call to hwrng_register() out of the probe routine into the scan
> routine. We need to call hwrng_register() after a suspend/restore cycle
> to re-register the device, but the scan function is not invoked for the
> restore. Add the call to hwrng_register() to virtio_restore().
>
> Reviewed-by: Liam Merwick
> Signed-off-by: Jim Quigley
> ---
> drivers/char/hw_random/virtio-rng.c | 21 -
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/hw_random/virtio-rng.c
> b/drivers/char/hw_random/virtio-rng.c
> index 3fa2f8a..b89df66 100644
> --- a/drivers/char/hw_random/virtio-rng.c
> +++ b/drivers/char/hw_random/virtio-rng.c
> @@ -184,7 +184,26 @@ static int virtrng_freeze(struct virtio_device *vdev)
>
> static int virtrng_restore(struct virtio_device *vdev)
> {
> - return probe_common(vdev);
> + int err;
> +
> + err = probe_common(vdev);
> + if (!err) {
> + struct virtrng_info *vi = vdev->priv;
> +
> + /*
> +* Set hwrng_removed to ensure that virtio_read()
> +* does not block waiting for data before the
> +* registration is complete.
> +*/
> + vi->hwrng_removed = true;
Hwrng core does not call read method till hwrng_register has finished.
Is this really required? I think it has no effect.
> + err = hwrng_register(&vi->hwrng);
> + if (!err) {
> + vi->hwrng_register_done = true;
> + vi->hwrng_removed = false;
> + }
> + }
> +
> + return err;
> }
> #endif
>
> --
> 1.8.3.1
>
I am fine with this change as is. Reviewed-by: PrasannaKumar
Muralidharan
Regards,
PrasannaKumar
Re: virtio:rng: Virtio RNG devices need to be re-registered after suspend/resume
Hi Jim, Have second thoughts on this. On 3 November 2017 at 20:55, PrasannaKumar Muralidharan wrote: >> >> It would be cleaner to just get rid of probe_common() altogether in that >> case, and do whatever >> needs to be done in virtrng_probe()/virtrng_restore() respectively, but > > That's correct. > >> I didn't want to change code >> affecting the normal probe path as well as suspend/resume. Is it OK to >> leave it that way to avoid >> the more extensive changes ? > > Personally I would prefer to do the cleaner way. It is up to the > virtio and hwrng maintainer. You are trying to restore the status quo of the driver that was before the commit 5c06273401. It is really important and valid. The driver's suspend/resume code is not in best shape but that is not what you are trying to solve. I am fine with this change. Regards, PrasannaKumar
Fwd: virtio:rng: Virtio RNG devices need to be re-registered after suspend/resume
Did reply instead of reply all. Forwarding my previous message.
-- Forwarded message --
From: PrasannaKumar Muralidharan
Date: 3 November 2017 at 20:19
Subject: Re: virtio:rng: Virtio RNG devices need to be re-registered
after suspend/resume
To: Jim Quigley
Hi Jim,
On 3 November 2017 at 20:11, Jim Quigley wrote:
>
>
> On 03/11/2017 13:06, PrasannaKumar Muralidharan wrote:
>>
>> Hi Jim,
>>
>> On 3 November 2017 at 15:27, Jim Quigley wrote:
>>>
>>> The patch for
>>>
>>> commit: 5c06273401f2eb7b290cadbae18ee00f8f65e893
>>> Author: Amit Shah
>>> Date: Sun Jul 27 07:34:01 2014 +0930
>>>
>>> virtio: rng: delay hwrng_register() till driver is ready
>>>
>>> moved the call to hwrng_register() out of the probe routine into the scan
>>> routine. We need to call hwrng_register() after a suspend/restore cycle
>>> to re-register the device, but the scan function is not invoked for the
>>> restore. Add the call to hwrng_register() to virtio_restore().
>>>
>>> Reviewed-by: Liam Merwick
>>> Signed-off-by: Jim Quigley
>>> ---
>>> drivers/char/hw_random/virtio-rng.c | 21 -
>>> 1 file changed, 20 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/char/hw_random/virtio-rng.c
>>> b/drivers/char/hw_random/virtio-rng.c
>>> index 3fa2f8a..b89df66 100644
>>> --- a/drivers/char/hw_random/virtio-rng.c
>>> +++ b/drivers/char/hw_random/virtio-rng.c
>>> @@ -184,7 +184,26 @@ static int virtrng_freeze(struct virtio_device
>>> *vdev)
>>>
>>> static int virtrng_restore(struct virtio_device *vdev)
>>> {
>>> - return probe_common(vdev);
>>> + int err;
>>> +
>>> + err = probe_common(vdev);
>>> + if (!err) {
>>> + struct virtrng_info *vi = vdev->priv;
>>> +
>>> + /*
>>> +* Set hwrng_removed to ensure that virtio_read()
>>> +* does not block waiting for data before the
>>> +* registration is complete.
>>> +*/
>>> + vi->hwrng_removed = true;
>>> + err = hwrng_register(&vi->hwrng);
>>> + if (!err) {
>>> + vi->hwrng_register_done = true;
>>> + vi->hwrng_removed = false;
>>> + }
>>> + }
>>> +
>>> + return err;
>>> }
>>> #endif
>>>
>>> --
>>> 1.8.3.1
>>>
>> This patch makes me wonder why hwrng_unregister is required in
>> virtrng_freeze. Looks strange and unusual. May be that is not required
>> and it can be removed. If it is required can you please add a comment
>> on why it is required?
>
>
> The reason it's required is because the virtrng_restore() uses
> probe_common() which allocates
> a new virtrng_info struct, changing the devices private pointer . This
> virtrng struct is used in
> hwrng_register() to set the current RNG etc. If we don't
> unregister/re-register then we would
> need to split probe_common() to avoid
>
> vi = kzalloc(sizeof(struct virtrng_info), GFP_KERNEL);
>
> overwriting vdev->priv on a restore.
True. As restore uses probe_common calling hwrng_register is required.
But I think it is not correct way to do.
>
> It would be cleaner to just get rid of probe_common() altogether in that
> case, and do whatever
> needs to be done in virtrng_probe()/virtrng_restore() respectively, but
That's correct.
> I didn't want to change code
> affecting the normal probe path as well as suspend/resume. Is it OK to
> leave it that way to avoid
> the more extensive changes ?
Personally I would prefer to do the cleaner way. It is up to the
virtio and hwrng maintainer.
Regards,
PrasannaKumar
Re: virtio:rng: Virtio RNG devices need to be re-registered after suspend/resume
On 03/11/2017 13:06, PrasannaKumar Muralidharan wrote:
Hi Jim,
On 3 November 2017 at 15:27, Jim Quigley wrote:
The patch for
commit: 5c06273401f2eb7b290cadbae18ee00f8f65e893
Author: Amit Shah
Date: Sun Jul 27 07:34:01 2014 +0930
virtio: rng: delay hwrng_register() till driver is ready
moved the call to hwrng_register() out of the probe routine into the scan
routine. We need to call hwrng_register() after a suspend/restore cycle
to re-register the device, but the scan function is not invoked for the
restore. Add the call to hwrng_register() to virtio_restore().
Reviewed-by: Liam Merwick
Signed-off-by: Jim Quigley
---
drivers/char/hw_random/virtio-rng.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/char/hw_random/virtio-rng.c
b/drivers/char/hw_random/virtio-rng.c
index 3fa2f8a..b89df66 100644
--- a/drivers/char/hw_random/virtio-rng.c
+++ b/drivers/char/hw_random/virtio-rng.c
@@ -184,7 +184,26 @@ static int virtrng_freeze(struct virtio_device *vdev)
static int virtrng_restore(struct virtio_device *vdev)
{
- return probe_common(vdev);
+ int err;
+
+ err = probe_common(vdev);
+ if (!err) {
+ struct virtrng_info *vi = vdev->priv;
+
+ /*
+* Set hwrng_removed to ensure that virtio_read()
+* does not block waiting for data before the
+* registration is complete.
+*/
+ vi->hwrng_removed = true;
+ err = hwrng_register(&vi->hwrng);
+ if (!err) {
+ vi->hwrng_register_done = true;
+ vi->hwrng_removed = false;
+ }
+ }
+
+ return err;
}
#endif
--
1.8.3.1
This patch makes me wonder why hwrng_unregister is required in
virtrng_freeze. Looks strange and unusual. May be that is not required
and it can be removed. If it is required can you please add a comment
on why it is required?
The reason it's required is because the virtrng_restore() uses
probe_common() which allocates
a new virtrng_info struct, changing the devices private pointer .
This virtrng struct is used in
hwrng_register() to set the current RNG etc. If we don't
unregister/re-register then we would
need to split probe_common() to avoid
vi = kzalloc(sizeof(struct virtrng_info), GFP_KERNEL);
overwriting vdev->priv on a restore.
It would be cleaner to just get rid of probe_common() altogether in
that case, and do whatever
needs to be done in virtrng_probe()/virtrng_restore() respectively,
but I didn't want to change code
affecting the normal probe path as well as suspend/resume. Is it OK
to leave it that way to avoid
the more extensive changes ?
thanks
regards
Jim Q.
Thanks,
PrasannaKumar
Re: [PATCH] crypto: qat: qat_common: qat_uclo - mark expected switch fall-throughs
Quoting Herbert Xu : On Thu, Oct 12, 2017 at 05:55:29PM -0500, Gustavo A. R. Silva wrote: In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva Patch applied. Thanks. Thank you, Herbert. -- Gustavo A. R. Silva
Re: [PATCH] crypto: cavium: clean up clang warning on unread variable offset
On Thu, Oct 12, 2017 at 05:44:06PM +0100, Colin King wrote:
> From: Colin Ian King
>
> The variable offset is being assigned and not being used; it should
> be passed as the 2nd argument to call to function nitrox_write_csr
> but has been omitted. Fix this.
>
> Cleans up clang warning: Value stored to 'offset' is never read
>
> Fixes: 14fa93cdcd9b ("crypto: cavium - Add support for CNN55XX adapters.")
> Signed-off-by: Colin Ian King
Patch applied. Thanks.
--
Email: Herbert Xu
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH 0/3] crypto: marvell - Remove the old CESA driver
On Wed, Oct 11, 2017 at 03:16:16PM +0200, Boris Brezillon wrote: > Hello, > > It's been several releases since we added a new driver to support the > CESA IP (the new driver was introduced in 4.2). It seems most major > bugs have been discovered and fixed and now is probably a good time to > kill the old driver and force remaining users to switch to the new one. > > This series first add a platform_device_id table to the marvell CESA > driver so that it can work on non-DT platforms. Then we patch all > defconfigs to select the new driver instead of the old. And finally we > remove the old driver. > These patches have to be applied in this order to make things > bisectable, so, if this series is accepted it will have to go through > a single tree (either ARM or crypto). > > Regards, > > Boris > > Boris Brezillon (3): > crypto: marvell - Add a platform_device_id table > ARM: configs: Stop selecting the old CESA driver > crypto: marvell - Remove the old mv_cesa driver > > arch/arm/configs/dove_defconfig |2 +- > arch/arm/configs/multi_v5_defconfig |2 +- > arch/arm/configs/orion5x_defconfig |2 +- > drivers/crypto/Kconfig | 22 +- > drivers/crypto/Makefile |1 - > drivers/crypto/marvell/cesa.c | 13 +- > drivers/crypto/mv_cesa.c| 1216 > --- > drivers/crypto/mv_cesa.h| 150 - > 8 files changed, 12 insertions(+), 1396 deletions(-) > delete mode 100644 drivers/crypto/mv_cesa.c > delete mode 100644 drivers/crypto/mv_cesa.h All applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: qat: qat_common: qat_uclo - mark expected switch fall-throughs
On Thu, Oct 12, 2017 at 05:55:29PM -0500, Gustavo A. R. Silva wrote: > In preparation to enabling -Wimplicit-fallthrough, mark switch cases > where we are expecting to fall through. > > Signed-off-by: Gustavo A. R. Silva Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: qat: remove unused and redundant pointer vf_info
On Thu, Oct 12, 2017 at 06:04:56PM +0100, Colin King wrote:
> From: Colin Ian King
>
> The pointer vf_info is being assigned but never read, it is redundant
> and therefore can be removed.
>
> Cleans up clang warning: Value stored to 'vf_info' is never read
>
> Fixes: ed8ccaef52fa ("crypto: qat - Add support for SRIOV")
> Signed-off-by: Colin Ian King
Patch applied. Thanks.
--
Email: Herbert Xu
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] MAINTAINERS: update caam crypto driver maintainers list
On Fri, Oct 13, 2017 at 03:01:23PM +0300, Horia Geantă wrote: > Dan steps down as caam maintainer, being replaced by Aymen. > > Signed-off-by: Horia Geantă Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH 0/2] Fixes for the Atmel AES crypto module
On Tue, Oct 31, 2017 at 04:25:22PM +0100, Romain Izard wrote:
> After encountering an issue with cts(cbc(aes)) in the Atmel AES module,
> I have used tcrypt and libkcapi's test suite to validate my fix. This led
> me to observe some other issues.
>
> This series includes the IV issue correction for the Atmel AES crypto
> engine, as well as a secondary issue observed when running
> 'insmod tcrypt.ko mode=10' and 'insmod tcrypt.ko mode=152' on a SAMA5D2
> board.
>
> The libkcapi test suite still reports some problems, for example when the
> input data is too large to fit into an intermediate buffer in unaligned
> cases. And it seems that with the v4.14 updates, new asynchronous tests
> are enabled and report new issues.
>
> Romain Izard (2):
> crypto: atmel-aes - properly set IV after {en,de}crypt
> crypto: atmel-aes - Reset the controller before each use
>
> drivers/crypto/atmel-aes.c | 50
> --
> 1 file changed, 40 insertions(+), 10 deletions(-)
All applied. Thanks.
--
Email: Herbert Xu
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v8 0/2] crypto: s5p-sss: Add HASH support for Exynos
On Wed, Oct 25, 2017 at 05:27:33PM +0200, Kamil Konieczny wrote: > First patch change spaces to tabs, second adds HASH support for Exynos. > Changes: > > version 8: > - fixes suggested by Vladimir Zapolskiy: drop first condition check in > s5p_hash_import, delete unused include delay.h, fix typo in commit > message, fix descriptions of struct s5p_hash_reqctx and function > s5p_hash_final() All applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] drivers/crypto: Convert timers to use timer_setup()
On Wed, Oct 25, 2017 at 03:18:42AM -0700, Kees Cook wrote: > In preparation for unconditionally passing the struct timer_list pointer to > all timer callbacks, switch to using the new timer_setup() and from_timer() > to pass the timer pointer explicitly. > > Cc: Herbert Xu > Cc: Jesper Nilsson > Cc: Lars Persson > Cc: Niklas Cassel > Cc: "David S. Miller" > Cc: Jamie Iles > Cc: linux-arm-ker...@axis.com > Cc: linux-crypto@vger.kernel.org > Cc: linux-arm-ker...@lists.infradead.org > Signed-off-by: Kees Cook Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] hw_random: core: Reset user selected rng by writing "" to rng_current
On Fri, Oct 27, 2017 at 10:34:04PM +0530, PrasannaKumar Muralidharan wrote: > User is able to select a chosen rng by writing its name to rng_current > but there is no way to reset it without unbinding the rng. Let user > write "" to rng_current and delesect the chosen rng. > > Signed-off-by: PrasannaKumar Muralidharan Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: caam/qi - abort algorithm setup on DPAA2 parts
On Tue, Oct 24, 2017 at 09:27:31AM +0300, Horia Geantă wrote:
> caam/qi frontend (i.e. caamalg_qi) mustn't be used in case it runs on a
> DPAA2 part (this could happen when using a multiplatform kernel).
>
> Fixes: 297b9cebd2fc ("crypto: caam/jr - add support for DPAA2 parts")
> Signed-off-by: Horia Geantă
Patch applied. Thanks.
--
Email: Herbert Xu
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v10 00/20] simplify crypto wait for async op
On Wed, Oct 18, 2017 at 08:00:32AM +0100, Gilad Ben-Yossef wrote: > Many users of kernel async. crypto services have a pattern of > starting an async. crypto op and than using a completion > to wait for it to end. > > This patch set simplifies this common use case in two ways: > > First, by separating the return codes of the case where a > request is queued to a backlog due to the provider being > busy (-EBUSY) from the case the request has failed due > to the provider being busy and backlogging is not enabled > (-ENOSPC). > > Next, this change is than built on to create a generic API > to wait for a async. crypto operation to complete. > > The end result is a smaller code base and an API that is > easier to use and more difficult to get wrong. > > The patch set was boot tested on x86_64 and arm64 which > at the very least tests the crypto users via testmgr and > tcrypt but I do note that I do not have access to some > of the HW whose drivers are modified nor do I claim I was > able to test all of the corner cases. > > The patch set is based upon linux-next release tagged > next-20171017. All applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH 1/3] crypto: atmel-aes/tdes/sha - return appropriate error code
On Mon, Oct 23, 2017 at 06:34:39PM +0300, Tudor Ambarus wrote: > Return -ENODEV when dma_request_slave_channel_compat() fails. > > Signed-off-by: Tudor Ambarus Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: caam - fix incorrect define
On Tue, Oct 24, 2017 at 09:27:30AM +0300, Horia Geantă wrote:
> From: Radu Alexe
>
> Fixes: 3ebfa92f49a6 ("crypto: caam - Add new macros for building extended SEC
> descriptors (> 64 words)")
> Signed-off-by: Radu Alexe
> Signed-off-by: Horia Geantă
Patch applied. Thanks.
--
Email: Herbert Xu
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH 3/3] crypto: atmel-aes/tdes/sha - remove useless irq init
On Mon, Oct 23, 2017 at 06:34:41PM +0300, Tudor Ambarus wrote: > irq would be set to -1 and then unused, if we failed to get IORESOURCE_MEM. > > Signed-off-by: Tudor Ambarus Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: vmx - Use skcipher for ctr fallback
On Mon, Oct 16, 2017 at 08:54:19PM -0200, Paulo Flabiano Smorigo wrote: > Signed-off-by: Paulo Flabiano Smorigo Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: marvell - Switch cipher algs to the skcipher interface
On Fri, Oct 13, 2017 at 03:30:32PM +0200, Boris Brezillon wrote: > crypto_alg is not supposed to be directly implemented by crypto engine > driver. Drivers should instead implement specialized interfaces like > ahash_alg or skcipher_alg. > > Migrate to all cipher algorithms to the skcipher_alg interface. While at > it, get rid of all references to ablkcipher including in internal struct > or function names. > > Signed-off-by: Boris Brezillon Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: ccp: remove unused variable qim
On Thu, Oct 12, 2017 at 05:55:41PM +0100, Colin King wrote:
> From: Colin Ian King
>
> Variable qim is assigned but never read, it is redundant and can
> be removed.
>
> Cleans up clang warning: Value stored to 'qim' is never read
>
> Fixes: 4b394a232df7 ("crypto: ccp - Let a v5 CCP provide the same function as
> v3")
> Signed-off-by: Colin Ian King
Patch applied. Thanks.
--
Email: Herbert Xu
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH 2/2] crypto: tcrypt - fix buffer lengths in test_aead_speed()
On Tue, Oct 10, 2017 at 01:22:00PM +0300, Robert Baronescu wrote: > Fix the way the length of the buffers used for > encryption / decryption are computed. > For e.g. in case of encryption, input buffer does not contain > an authentication tag. > > Signed-off-by: Robert Baronescu Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: tcrypt: mark expected switch fall-throughs in do_test()
On Mon, Oct 09, 2017 at 02:43:21PM -0500, Gustavo A. R. Silva wrote: > In preparation to enabling -Wimplicit-fallthrough, mark switch cases > where we are expecting to fall through. > > Cc: Herbert Xu > Cc: "David S. Miller" > Cc: linux-crypto@vger.kernel.org > Signed-off-by: Gustavo A. R. Silva Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v2 1/7] crypto:chelsio: Remove unused parameter
On Sun, Oct 08, 2017 at 01:37:18PM +0530, Harsh Jain wrote: > From: Yeshaswi M R Gowda > > Remove unused parameter sent to latest fw. > > Signed-off-by: Harsh Jain All applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: ccm - preserve the IV buffer
On Tue, Oct 31, 2017 at 03:42:35PM +0100, Romain Izard wrote: > The IV buffer used during CCM operations is used twice, during both the > hashing step and the ciphering step. > > When using a hardware accelerator that updates the contents of the IV > buffer at the end of ciphering operations, the value will be modified. > In the decryption case, the subsequent setup of the hashing algorithm > will interpret the updated IV instead of the original value, which can > lead to out-of-bounds writes. > > Reuse the idata buffer, only used in the hashing step, to preserve the > IV's value during the ciphering step in the decryption case. > > Signed-off-by: Romain Izard Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH 1/2] x86/crypto/sha256-mb: fix panic due to unaligned access
On Mon, Oct 16, 2017 at 06:51:30PM +0300, Andrey Ryabinin wrote:
> struct sha256_ctx_mgr allocated in sha256_mb_mod_init() via kzalloc()
> and later passed in sha256_mb_flusher_mgr_flush_avx2() function where
> instructions vmovdqa used to access the struct. vmovdqa requires
> 16-bytes aligned argument, but nothing guarantees that struct
> sha256_ctx_mgr will have that alignment. Unaligned vmovdqa will
> generate GP fault.
>
> Fix this by replacing vmovdqa with vmovdqu which doesn't have alignment
> requirements.
>
> Fixes: a377c6b1876e ("crypto: sha256-mb - submit/flush routines for AVX2")
> Reported-by: Josh Poimboeuf
> Signed-off-by: Andrey Ryabinin
> Cc:
Both patches applied. Thanks.
--
Email: Herbert Xu
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: AF_ALG - remove locking in async callback
Am Freitag, 3. November 2017, 14:20:16 CET schrieb Herbert Xu: Hi Herbert, > On Sun, Oct 29, 2017 at 09:39:30PM +0100, Stephan Müller wrote: > > Am Mittwoch, 25. Oktober 2017, 17:26:31 CET schrieb Romain Izard: > > > > Hi Romain, > > > > the patch below should cover the issue you see. Would you mind testing it? > > > > Thanks > > Stephan > > > > ---8<--- > > > > The code paths protected by the socket-lock do not use or modify the > > socket in a non-atomic fashion. The actions pertaining the socket do not > > even need to be handled as an atomic operation. Thus, the socket-lock > > can be safely ignored. > > Are you sure about that? In particular is the callback function still > sane without the socket lock if a concurrent recvmsg/sendmsg call is > made? resultlen receives its data from the async_request -> no socket af_alg_free_areq_sgls(areq) does not require a socket, but it uses the socket to find the data structures -> I do not see that the socket is operated on though. The socket will always be alive as the sk_refcnt is not yet decremented by __sock_put. sock_kfree_s uses an atomic operation __sock_put uses an atomic operation iocb->ki_complete does not use the socket Where would you think that the lock is needed? > > Your fixes header is wrong too as the locks weren't introduced in that > commit, they just got moved around. Neither the skcipher_async_cb nor aead_async_cb up to and including 4.13 contain any lock. Ciao Stephan
Re: [PATCH] crypto: AF_ALG - remove locking in async callback
On Sun, Oct 29, 2017 at 09:39:30PM +0100, Stephan Müller wrote: > Am Mittwoch, 25. Oktober 2017, 17:26:31 CET schrieb Romain Izard: > > Hi Romain, > > the patch below should cover the issue you see. Would you mind testing it? > > Thanks > Stephan > > ---8<--- > > The code paths protected by the socket-lock do not use or modify the > socket in a non-atomic fashion. The actions pertaining the socket do not > even need to be handled as an atomic operation. Thus, the socket-lock > can be safely ignored. Are you sure about that? In particular is the callback function still sane without the socket lock if a concurrent recvmsg/sendmsg call is made? Your fixes header is wrong too as the locks weren't introduced in that commit, they just got moved around. Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: virtio:rng: Virtio RNG devices need to be re-registered after suspend/resume
Hi Jim,
On 3 November 2017 at 15:27, Jim Quigley wrote:
> The patch for
>
> commit: 5c06273401f2eb7b290cadbae18ee00f8f65e893
> Author: Amit Shah
> Date: Sun Jul 27 07:34:01 2014 +0930
>
> virtio: rng: delay hwrng_register() till driver is ready
>
> moved the call to hwrng_register() out of the probe routine into the scan
> routine. We need to call hwrng_register() after a suspend/restore cycle
> to re-register the device, but the scan function is not invoked for the
> restore. Add the call to hwrng_register() to virtio_restore().
>
> Reviewed-by: Liam Merwick
> Signed-off-by: Jim Quigley
> ---
> drivers/char/hw_random/virtio-rng.c | 21 -
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/hw_random/virtio-rng.c
> b/drivers/char/hw_random/virtio-rng.c
> index 3fa2f8a..b89df66 100644
> --- a/drivers/char/hw_random/virtio-rng.c
> +++ b/drivers/char/hw_random/virtio-rng.c
> @@ -184,7 +184,26 @@ static int virtrng_freeze(struct virtio_device *vdev)
>
> static int virtrng_restore(struct virtio_device *vdev)
> {
> - return probe_common(vdev);
> + int err;
> +
> + err = probe_common(vdev);
> + if (!err) {
> + struct virtrng_info *vi = vdev->priv;
> +
> + /*
> +* Set hwrng_removed to ensure that virtio_read()
> +* does not block waiting for data before the
> +* registration is complete.
> +*/
> + vi->hwrng_removed = true;
> + err = hwrng_register(&vi->hwrng);
> + if (!err) {
> + vi->hwrng_register_done = true;
> + vi->hwrng_removed = false;
> + }
> + }
> +
> + return err;
> }
> #endif
>
> --
> 1.8.3.1
>
This patch makes me wonder why hwrng_unregister is required in
virtrng_freeze. Looks strange and unusual. May be that is not required
and it can be removed. If it is required can you please add a comment
on why it is required?
Thanks,
PrasannaKumar
Re: [PATCH 1/2] crypto: tcrypt - fix S/G table for test_aead_speed()
On Tue, Oct 10, 2017 at 01:21:59PM +0300, Robert Baronescu wrote: > In case buffer length is a multiple of PAGE_SIZE, > the S/G table is incorrectly generated. > Fix this by handling buflen = k * PAGE_SIZE separately. > > Signed-off-by: Robert Baronescu > --- > crypto/tcrypt.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c > index 0022a18..bd9b66c 100644 > --- a/crypto/tcrypt.c > +++ b/crypto/tcrypt.c > @@ -221,11 +221,13 @@ static void sg_init_aead(struct scatterlist *sg, char > *xbuf[XBUFSIZE], > } > > sg_init_table(sg, np + 1); > - np--; > + if (rem) > + np--; > for (k = 0; k < np; k++) > sg_set_buf(&sg[k + 1], xbuf[k], PAGE_SIZE); > > - sg_set_buf(&sg[k + 1], xbuf[k], rem); > + if (rem) > + sg_set_buf(&sg[k + 1], xbuf[k], rem); Sorry but I think this is still buggy because you have not moved the end-of-table marking in the rem == 0 case. Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [Part2 PATCH v7 14/38] crypto: ccp: Implement SEV_FACTORY_RESET ioctl command
On Wed, Nov 01, 2017 at 04:15:59PM -0500, Brijesh Singh wrote:
> The SEV_FACTORY_RESET command can be used by the platform owner to
> reset the non-volatile SEV related data. The command is defined in
> SEV spec section 5.4
>
> Cc: Paolo Bonzini
> Cc: "Radim Krčmář"
> Cc: Borislav Petkov
> Cc: Herbert Xu
> Cc: Gary Hook
> Cc: Tom Lendacky
> Cc: linux-crypto@vger.kernel.org
> Cc: k...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Improvements-by: Borislav Petkov
> Signed-off-by: Brijesh Singh
> Acked-by: Gary R Hook
Acked-by: needs to go away too if you send a new revision with
non-trivial changes. Same as with Reviewed-by: tags.
> ---
> drivers/crypto/ccp/psp-dev.c | 70
> +++-
> 1 file changed, 69 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
> index c61ca16096ca..a757bd1c34e8 100644
> --- a/drivers/crypto/ccp/psp-dev.c
> +++ b/drivers/crypto/ccp/psp-dev.c
> @@ -235,9 +235,77 @@ static int sev_platform_shutdown(int *error)
> return rc;
> }
>
> +static int sev_platform_state(int *state, int *error)
It needs a verb in the name: sev_get_platform_state()
> +{
> + int rc;
> +
> + rc = __sev_do_cmd_locked(SEV_CMD_PLATFORM_STATUS,
> + psp_master->sev_status, error);
> + if (rc)
> + return rc;
> +
> + *state = psp_master->sev_status->state;
> + return rc;
> +}
> +
> +static int sev_ioctl_do_reset(struct sev_issue_cmd *argp)
> +{
> + int state, rc;
> +
> + rc = sev_platform_state(&state, &argp->error);
> + if (rc)
> + return rc;
> +
> + if (state == SEV_STATE_WORKING) {
You could write a short blurb somewhere in a comment around here, what
the logic now is going to be for the SEV device: If in working state,
reset is denied. All other states accept it, because... .
> + argp->error = SEV_RET_INVALID_PLATFORM_STATE;
If you're going to write enum psp_ret_code types into argp->error, then
it needs to be of enum psp_ret_code type and not an int.
> + return -EBUSY;
> + }
> +
> + if (state == SEV_STATE_INIT) {
> + rc = __sev_platform_shutdown_locked(&argp->error);
> + if (rc)
> + return rc;
> + }
> +
> + return __sev_do_cmd_locked(SEV_CMD_FACTORY_RESET, 0, &argp->error);
> +}
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
virtio:rng: Virtio RNG devices need to be re-registered after suspend/resume
The patch for
commit: 5c06273401f2eb7b290cadbae18ee00f8f65e893
Author: Amit Shah
Date: Sun Jul 27 07:34:01 2014 +0930
virtio: rng: delay hwrng_register() till driver is ready
moved the call to hwrng_register() out of the probe routine into the scan
routine. We need to call hwrng_register() after a suspend/restore cycle
to re-register the device, but the scan function is not invoked for the
restore. Add the call to hwrng_register() to virtio_restore().
Reviewed-by: Liam Merwick
Signed-off-by: Jim Quigley
---
drivers/char/hw_random/virtio-rng.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/char/hw_random/virtio-rng.c
b/drivers/char/hw_random/virtio-rng.c
index 3fa2f8a..b89df66 100644
--- a/drivers/char/hw_random/virtio-rng.c
+++ b/drivers/char/hw_random/virtio-rng.c
@@ -184,7 +184,26 @@ static int virtrng_freeze(struct virtio_device *vdev)
static int virtrng_restore(struct virtio_device *vdev)
{
- return probe_common(vdev);
+ int err;
+
+ err = probe_common(vdev);
+ if (!err) {
+ struct virtrng_info *vi = vdev->priv;
+
+ /*
+* Set hwrng_removed to ensure that virtio_read()
+* does not block waiting for data before the
+* registration is complete.
+*/
+ vi->hwrng_removed = true;
+ err = hwrng_register(&vi->hwrng);
+ if (!err) {
+ vi->hwrng_register_done = true;
+ vi->hwrng_removed = false;
+ }
+ }
+
+ return err;
}
#endif
--
1.8.3.1
