* RE: [PATCH 1/4] crypto: powerpc - Factor out the core CRC vpmsum algorithm
From: Daniel Axtens @ 2017-03-15 22:30 UTC (permalink / raw)
To: David Laight, linuxppc-dev@lists.ozlabs.org,
linux-crypto@vger.kernel.org
Cc: anton@samba.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DCFFB1A81@AcuExch.aculab.com>
Hi David,
> While not part of this change, the unrolled loops look as though
> they just destroy the cpu cache.
> I'd like be convinced that anything does CRC over long enough buffers
> to make it a gain at all.
>
> With modern (not that modern now) superscalar cpus you can often
> get the loop instructions 'for free'.
> Sometimes pipelining the loop is needed to get full throughput.
> Unlike the IP checksum, you don't even have to 'loop carry' the
> cpu carry flag.
Internal testing on a NVMe device with T10DIF enabled on 4k blocks
shows a 20x - 30x improvement. Without these patches, crc_t10dif_generic
uses over 60% of CPU time - with these patches CRC drops to single
digits.
I should probably have lead with that, sorry.
FWIW, the original patch showed a 3.7x gain on btrfs as well -
6dd7a82cc54e ("crypto: powerpc - Add POWER8 optimised crc32c")
When Anton wrote the original code he had access to IBM's internal
tooling for looking at how instructions flow through the various stages
of the CPU, so I trust it's pretty much optimal from that point of view.
Regards,
Daniel
^ permalink raw reply
* [PATCH] crypto, x86: aesni - fix token pasting for clang
From: Michael Davidson @ 2017-03-15 22:36 UTC (permalink / raw)
To: Herbert Xu, David S . Miller, Thomas Gleixner, Ingo Molnar,
H . Peter Anvin
Cc: Alexander Potapenko, Dmitry Vyukov, x86, linux-crypto,
linux-kernel, Michael Davidson
aes_ctrby8_avx-x86_64.S uses the C preprocessor for token pasting
of character sequences that are not valid preprocessor tokens.
While this is allowed when preprocessing assembler files it exposes
an incompatibilty between the clang and gcc preprocessors where
clang does not strip leading white space from macro parameters,
leading to the CONCAT(%xmm, i) macro expansion on line 96 resulting
in a token with a space character embedded in it.
While this could be resolved by deleting the offending space character,
the assembler is perfectly capable of doing the token pasting correctly
for itself so we can just get rid of the preprocessor macros.
Signed-off-by: Michael Davidson <md@google.com>
---
arch/x86/crypto/aes_ctrby8_avx-x86_64.S | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/arch/x86/crypto/aes_ctrby8_avx-x86_64.S b/arch/x86/crypto/aes_ctrby8_avx-x86_64.S
index a916c4a61165..5f6a5af9c489 100644
--- a/arch/x86/crypto/aes_ctrby8_avx-x86_64.S
+++ b/arch/x86/crypto/aes_ctrby8_avx-x86_64.S
@@ -65,7 +65,6 @@
#include <linux/linkage.h>
#include <asm/inst.h>
-#define CONCAT(a,b) a##b
#define VMOVDQ vmovdqu
#define xdata0 %xmm0
@@ -92,8 +91,6 @@
#define num_bytes %r8
#define tmp %r10
-#define DDQ(i) CONCAT(ddq_add_,i)
-#define XMM(i) CONCAT(%xmm, i)
#define DDQ_DATA 0
#define XDATA 1
#define KEY_128 1
@@ -131,12 +128,12 @@ ddq_add_8:
/* generate a unique variable for ddq_add_x */
.macro setddq n
- var_ddq_add = DDQ(\n)
+ var_ddq_add = ddq_add_\n
.endm
/* generate a unique variable for xmm register */
.macro setxdata n
- var_xdata = XMM(\n)
+ var_xdata = %xmm\n
.endm
/* club the numeric 'id' to the symbol 'name' */
--
2.12.0.367.g23dc2f6d3c-goog
^ permalink raw reply related
* Re: [PATCH v5 1/2] crypto: skcipher AF_ALG - overhaul memory management
From: Herbert Xu @ 2017-03-16 8:39 UTC (permalink / raw)
To: Stephan Müller; +Cc: linux-crypto
In-Reply-To: <16908830.B9gcyeAhPX@positron.chronox.de>
On Fri, Feb 17, 2017 at 11:31:41PM +0100, Stephan Müller wrote:
>
> + } else {
> + /* Synchronous operation */
> + skcipher_request_set_callback(&areq->req,
> + CRYPTO_TFM_REQ_MAY_SLEEP |
> + CRYPTO_TFM_REQ_MAY_BACKLOG,
> + af_alg_complete,
> + &ctx->completion);
> + err = af_alg_wait_for_completion(ctx->enc ?
> + crypto_skcipher_encrypt(&areq->req) :
> + crypto_skcipher_decrypt(&areq->req),
> + &ctx->completion);
> + }
This is now outside of the loop for the sync case. The purpose
of the loop in the sync case was to segment the data when we get
a very large SG list that does not fit inside a single call.
Or did I miss something?
Overall I like this patch.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [RFC 0/7] crypto: caam - add Queue Interface (QI) support
From: Herbert Xu @ 2017-03-16 8:42 UTC (permalink / raw)
To: Horia Geantă
Cc: Scott Wood, Roy Pledge, linux-crypto, linux-arm-kernel,
Dan Douglass, Alexandru Porosanu, Vakul Garg, Cristian Stoica,
Claudiu Manoil
In-Reply-To: <1488552733-20806-1-git-send-email-horia.geanta@nxp.com>
On Fri, Mar 03, 2017 at 04:52:06PM +0200, Horia Geantă wrote:
> The patchset adds support for CAAM Queue Interface (QI), the additional
> interface (besides job ring) available for submitting jobs to the engine
> on platforms having DPAA (Datapath Acceleration Architecture).
>
> Patches 1-4 are QMan dependencies.
> I would prefer to take them through the crypto tree,
> but I am open to suggestions.
>
> Patch 5 adds a missing double inclusion guard in desc_constr.h.
>
> Patch 6 adds the caam/qi job submission backend.
>
> Patch 7 adds algorithms (ablkcipher and authenc) that run on top
> of caam/qi. For now, their priority is set lower than caam/jr.
I'm fine with the crypto bits.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v5 1/2] crypto: skcipher AF_ALG - overhaul memory management
From: Stephan Müller @ 2017-03-16 8:55 UTC (permalink / raw)
To: Herbert Xu; +Cc: linux-crypto
In-Reply-To: <20170316083922.GA11653@gondor.apana.org.au>
Am Donnerstag, 16. März 2017, 09:39:23 CET schrieb Herbert Xu:
Hi Herbert,
> On Fri, Feb 17, 2017 at 11:31:41PM +0100, Stephan Müller wrote:
> > + } else {
> > + /* Synchronous operation */
> > + skcipher_request_set_callback(&areq->req,
> > + CRYPTO_TFM_REQ_MAY_SLEEP |
> > + CRYPTO_TFM_REQ_MAY_BACKLOG,
> > + af_alg_complete,
> > + &ctx->completion);
> > + err = af_alg_wait_for_completion(ctx->enc ?
> > + crypto_skcipher_encrypt(&areq->req) :
> > + crypto_skcipher_decrypt(&areq->req),
> > + &ctx->completion);
> > + }
>
> This is now outside of the loop for the sync case. The purpose
> of the loop in the sync case was to segment the data when we get
> a very large SG list that does not fit inside a single call.
>
> Or did I miss something?
The while loop present in the skcipher_recvmsg_sync present in the current
upstream kernel operates on ctx->rsgl. That data structure is defined as:
struct af_alg_sgl {
struct scatterlist sg[ALG_MAX_PAGES + 1];
struct page *pages[ALG_MAX_PAGES];
unsigned int npages;
};
I.e. recvmsg operates on at most 16 SGs where each can take one page of data.
Thus, if a user provides more data than 16 pages, such while loop would make
much sense.
The patch proposed here is not limited on a fixed set of 16 SGs in the RX-SGL.
The recvmsg code path allocates the required amount of SGLs space:
/* convert iovecs of output buffers into RX SGL */
while (len < ctx->used && iov_iter_count(&msg->msg_iter)) {
struct skcipher_rsgl *rsgl;
...
rsgl = sock_kmalloc(sk, sizeof(*rsgl), GFP_KERNEL);
...
struct skcipher_rsgl uses the af_alg_sgl data structure with its 16 SG
entries. But now you can have arbitrary numbers of af_alg_sgl instances up to
the memory provided by sock_kmalloc. I.e. recvmsg can now operate on multiples
of af_alg_sgl in one go.
With this approach I thought that the while loop could be a thing of the past,
considering that this is also the approach taken in skcipher_recvmsg_async
that is present in the current upstream code base.
>
> Overall I like this patch.
Thank you very much.
>
> Thanks,
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH v5 1/2] crypto: skcipher AF_ALG - overhaul memory management
From: Herbert Xu @ 2017-03-16 9:08 UTC (permalink / raw)
To: Stephan Müller; +Cc: linux-crypto
In-Reply-To: <13069331.7r3bo5pPcW@positron.chronox.de>
On Thu, Mar 16, 2017 at 09:55:17AM +0100, Stephan Müller wrote:
>
> With this approach I thought that the while loop could be a thing of the past,
> considering that this is also the approach taken in skcipher_recvmsg_async
> that is present in the current upstream code base.
The reason there is a limit is so that user-space doesn't pin down
unlimited amounts of memory. How is this addressed under your
scheme?
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v5 1/2] crypto: skcipher AF_ALG - overhaul memory management
From: Stephan Müller @ 2017-03-16 9:23 UTC (permalink / raw)
To: Herbert Xu; +Cc: linux-crypto
In-Reply-To: <20170316090823.GA11924@gondor.apana.org.au>
Am Donnerstag, 16. März 2017, 10:08:23 CET schrieb Herbert Xu:
Hi Herbert,
> On Thu, Mar 16, 2017 at 09:55:17AM +0100, Stephan Müller wrote:
> > With this approach I thought that the while loop could be a thing of the
> > past, considering that this is also the approach taken in
> > skcipher_recvmsg_async that is present in the current upstream code base.
>
> The reason there is a limit is so that user-space doesn't pin down
> unlimited amounts of memory. How is this addressed under your
> scheme?
sock_kmalloc limits the number of SG tables that we can manage. On my system,
sock_kmalloc has 20480 bytes at its disposal as the limiting factor.
As this concept for limiting the impact of user space on kernel memory is used
in the current upstream skcipher_recvmsg_async, I simply re-used that
approach:
while (iov_iter_count(&msg->msg_iter)) {
struct skcipher_async_rsgl *rsgl;
...
rsgl = kmalloc(sizeof(*rsgl), GFP_KERNEL);
...
Note I use sock_kmalloc instead of the currently existing kmalloc to ensure
such limitation.
Also, please note that this approach is identical to the currently used code
in algif_aead:aead_recvmsg_sync. This implementation led be to belief that
this my new code would be appropriate in this case, too.
Ciao
Stephan
^ permalink raw reply
* RE: [PATCH 1/4] crypto: powerpc - Factor out the core CRC vpmsum algorithm
From: David Laight @ 2017-03-16 9:50 UTC (permalink / raw)
To: 'Daniel Axtens', linuxppc-dev@lists.ozlabs.org,
linux-crypto@vger.kernel.org
Cc: anton@samba.org
In-Reply-To: <87efxy41hi.fsf@possimpible.ozlabs.ibm.com>
From: Daniel Axtens
> Sent: 15 March 2017 22:30
> Hi David,
>
> > While not part of this change, the unrolled loops look as though
> > they just destroy the cpu cache.
> > I'd like be convinced that anything does CRC over long enough buffers
> > to make it a gain at all.
> >
> > With modern (not that modern now) superscalar cpus you can often
> > get the loop instructions 'for free'.
> > Sometimes pipelining the loop is needed to get full throughput.
> > Unlike the IP checksum, you don't even have to 'loop carry' the
> > cpu carry flag.
>
> Internal testing on a NVMe device with T10DIF enabled on 4k blocks
> shows a 20x - 30x improvement. Without these patches, crc_t10dif_generic
> uses over 60% of CPU time - with these patches CRC drops to single
> digits.
>
> I should probably have lead with that, sorry.
I'm not doubting that using the cpu instruction for crcs gives a
massive performance boost.
Just that the heavily unrolled loop is unlikely to help overall.
Some 'cold cache' tests on shorter buffers might be illuminating.
> FWIW, the original patch showed a 3.7x gain on btrfs as well -
> 6dd7a82cc54e ("crypto: powerpc - Add POWER8 optimised crc32c")
>
> When Anton wrote the original code he had access to IBM's internal
> tooling for looking at how instructions flow through the various stages
> of the CPU, so I trust it's pretty much optimal from that point of view.
Doesn't mean he used it :-)
David
^ permalink raw reply
* Re: [PATCH v5 1/2] crypto: skcipher AF_ALG - overhaul memory management
From: Herbert Xu @ 2017-03-16 9:52 UTC (permalink / raw)
To: Stephan Müller; +Cc: linux-crypto
In-Reply-To: <1797510.JnIj36BbdJ@tauon.atsec.com>
On Thu, Mar 16, 2017 at 10:23:49AM +0100, Stephan Müller wrote:
> Am Donnerstag, 16. März 2017, 10:08:23 CET schrieb Herbert Xu:
>
> Hi Herbert,
>
> > On Thu, Mar 16, 2017 at 09:55:17AM +0100, Stephan Müller wrote:
> > > With this approach I thought that the while loop could be a thing of the
> > > past, considering that this is also the approach taken in
> > > skcipher_recvmsg_async that is present in the current upstream code base.
> >
> > The reason there is a limit is so that user-space doesn't pin down
> > unlimited amounts of memory. How is this addressed under your
> > scheme?
>
> sock_kmalloc limits the number of SG tables that we can manage. On my system,
> sock_kmalloc has 20480 bytes at its disposal as the limiting factor.
>
> As this concept for limiting the impact of user space on kernel memory is used
> in the current upstream skcipher_recvmsg_async, I simply re-used that
> approach:
>
> while (iov_iter_count(&msg->msg_iter)) {
> struct skcipher_async_rsgl *rsgl;
> ...
> rsgl = kmalloc(sizeof(*rsgl), GFP_KERNEL);
> ...
>
> Note I use sock_kmalloc instead of the currently existing kmalloc to ensure
> such limitation.
First of all you're only limiting the amount of memory occupied
by the SG list which is not the same thing as the memory pinned
down by the actual recvmsg.
More importantly, with the current code, a very large recvmsg
would still work by doing it piecemeal. With your patch, won't
it fail because sock_kmalloc could fail to allocate memory for
the whole thing?
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] crypto: ccp - Assign DMA commands to the channel's CCP
From: Herbert Xu @ 2017-03-16 9:53 UTC (permalink / raw)
To: Gary R Hook; +Cc: linux-crypto, thomas.lendacky, davem
In-Reply-To: <20170310182818.21186.92711.stgit@taos>
On Fri, Mar 10, 2017 at 12:28:18PM -0600, Gary R Hook wrote:
> From: Gary R Hook <ghook@amd.com>
>
> The CCP driver generally uses a round-robin approach when
> assigning operations to available CCPs. For the DMA engine,
> however, the DMA mappings of the SGs are associated with a
> specific CCP. When an IOMMU is enabled, the IOMMU is
> programmed based on this specific device.
>
> If the DMA operations are not performed by that specific
> CCP then addressing errors and I/O page faults will occur.
>
> Update the CCP driver to allow a specific CCP device to be
> requested for an operation and use this in the DMA engine
> support.
>
> Cc: <stable@vger.kernel.org> # 4.9.x-
> Signed-off-by: Gary R Hook <gary.hook@amd.com>
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH 0/2] hwrng: revert managed API changes for amd and geode
From: Herbert Xu @ 2017-03-16 9:53 UTC (permalink / raw)
To: Prarit Bhargava
Cc: linux-kernel, Matt Mackall, Corentin LABBE,
PrasannaKumar Muralidharan, Wei Yongjun, linux-crypto,
linux-geode
In-Reply-To: <1489491362-26339-1-git-send-email-prarit@redhat.com>
On Tue, Mar 14, 2017 at 07:36:00AM -0400, Prarit Bhargava wrote:
> When booting top-of-tree the following WARN_ON triggers in the kernel on
> a 15h AMD system.
>
> WARNING: CPU: 2 PID: 621 at drivers/base/dd.c:349 driver_probe_device+0x38c
> Modules linked in: i2c_amd756(+) amd_rng sg pcspkr parport_pc(+) parport k8
> CPU: 2 PID: 621 Comm: systemd-udevd Not tainted 4.11.0-0.rc1.git0.1.el7_UNS
> Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./TYAN High-End
> Call Trace:
> dump_stack+0x63/0x8e
> __warn+0xd1/0xf0
> warn_slowpath_null+0x1d/0x20
> driver_probe_device+0x38c/0x470
> __driver_attach+0xc9/0xf0
> ? driver_probe_device+0x470/0x470
> bus_for_each_dev+0x5d/0x90
> driver_attach+0x1e/0x20
> bus_add_driver+0x1d0/0x290
> driver_register+0x60/0xe0
> ? 0xffffffffa0037000
> __pci_register_driver+0x4c/0x50
> amd756_driver_init+0x1e/0x1000 [i2c_amd756]
> do_one_initcall+0x51/0x1b0
> ? __vunmap+0x85/0xd0
> ? do_init_module+0x27/0x1fa
> do_init_module+0x60/0x1fa
> load_module+0x15d1/0x1ad0
> ? m_show+0x1c0/0x1c0
> SYSC_finit_module+0xa9/0xd0
>
> There are PCI devices that contain both a RNG and SMBUS device. The
> RNG device is initialized by the amd-rng driver but the driver does not
> register against the device. The SMBUS device is initialized by the
> i2c-amd756 driver and registers against the device and hits the WARN_ON()
> because the amd-rng driver has already allocated resources against the
> device.
>
> The amd-rng driver was incorrectly migrated to the device resource model
> (devres), and after code inspection I found that the geode-rng driver was also
> incorrectly migrated. These drivers are using devres but do not register a
> driver against the device, and both drivers are expecting a memory cleanup on
> a driver detach that will never happen. This results in a memory leak when the
> driver is unloaded and the inability to reload the driver.
>
> Revert 31b2a73c9c5f ("hwrng: amd - Migrate to managed API"), and 6e9b5e76882c
> ("hwrng: geode - Migrate to managed API").
>
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Fixes: 31b2a73c9c5f ("hwrng: amd - Migrate to managed API").
> Fixes: 6e9b5e76882c ("hwrng: geode - Migrate to managed API")
> Cc: Matt Mackall <mpm@selenic.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: Corentin LABBE <clabbe.montjoie@gmail.com>
> Cc: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
> Cc: Wei Yongjun <weiyongjun1@huawei.com>
> Cc: linux-crypto@vger.kernel.org
> Cc: linux-geode@lists.infradead.org
Both patches applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v3 1/3] clk: meson-gxbb: expose clock CLKID_RNG0
From: Herbert Xu @ 2017-03-16 10:07 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Jerome Brunet, Kevin Hilman, Neil Armstrong, linux-amlogic,
linux-crypto, Stephen Boyd, Michael Turquette, linux-clk,
devicetree
In-Reply-To: <4e2d12e5-f75b-3ac6-a3f7-18e8a8b7283b@gmail.com>
On Wed, Feb 22, 2017 at 07:55:24AM +0100, Heiner Kallweit wrote:
> Expose clock CLKID_RNG0 which is needed for the HW random number generator.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
All patches applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH 0/4] hwrng: omap - fixes and improvements
From: Herbert Xu @ 2017-03-16 10:08 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: Deepak Saxena, Matt Mackall, linux-crypto, romain.perier,
Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
Nadav Haklai, Hanna Hawa, Yehuda Yitschak
In-Reply-To: <1488896089-17586-1-git-send-email-thomas.petazzoni@free-electrons.com>
On Tue, Mar 07, 2017 at 03:14:45PM +0100, Thomas Petazzoni wrote:
> Hello,
>
> This small patch series brings a few fixes and improvements to the
> omap_rng driver. The first fix is particularly important, as it fixes
> using the driver built as a module on SoCs that require a clock for
> the IP to work properly.
>
> Thanks,
>
> Thomas
>
> Thomas Petazzoni (4):
> hwrng: omap - write registers after enabling the clock
> hwrng: omap - use devm_clk_get() instead of of_clk_get()
> hwrng: omap - Do not access INTMASK_REG on EIP76
> hwrng: omap - move clock related code to omap_rng_probe()
Patches 1-3 applied to crypto and patch 4 applied to cryptodev.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v1 0/8] improve performances on mediatek crypto driver
From: Herbert Xu @ 2017-03-16 10:08 UTC (permalink / raw)
To: Ryder Lee; +Cc: linux-mediatek, linux-crypto
In-Reply-To: <1489025479-48036-1-git-send-email-ryder.lee@mediatek.com>
On Thu, Mar 09, 2017 at 10:11:11AM +0800, Ryder Lee wrote:
> Hi all,
>
> Some patches of this series improve the performances whereas others
> clean up code and refine data structure to make it more efficient
>
> Changes since v1:
> - drop OFB and CFB patch
>
> Ryder Lee (8):
> crypto: mediatek - rework interrupt handler
> crypto: mediatek - add MTK_* prefix and correct annotations.
> crypto: mediatek - make mtk_sha_xmit() more generic
> crypto: mediatek - simplify descriptor ring management
> crypto: mediatek - add queue_task tasklet
> crypto: mediatek - fix error handling in mtk_aes_complete()
> crypto: mediatek - add mtk_aes_gcm_tag_verify()
> crypto: mediatek - make hardware operation flow more efficient
All applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v2] MAINTAINERS: Add maintianer entry for crypto/s5p-sss
From: Herbert Xu @ 2017-03-16 10:09 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: David S. Miller, linux-crypto, linux-kernel, Vladimir Zapolskiy
In-Reply-To: <20170311061100.4659-1-krzk@kernel.org>
On Sat, Mar 11, 2017 at 08:11:00AM +0200, Krzysztof Kozlowski wrote:
> Add Krzysztof Kozlowski and Vladimir Zapolskiy as maintainers of s5p-sss
> driver for handling reviews, testing and getting bug reports from the
> users.
>
> Cc: Vladimir Zapolskiy <vz@mleia.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] crypto: doc - fix typo (struct sdesc)
From: Herbert Xu @ 2017-03-16 10:09 UTC (permalink / raw)
To: Fabien Dessenne; +Cc: linux-crypto
In-Reply-To: <1489052405-405-1-git-send-email-fabien.dessenne@st.com>
Fabien Dessenne <fabien.dessenne@st.com> wrote:
> Add missing " " in api-samples.rst
>
> Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [RFC PATCH v2 12/32] x86: Add early boot support when running with SEV active
From: Borislav Petkov @ 2017-03-16 10:16 UTC (permalink / raw)
To: Brijesh Singh
Cc: Paolo Bonzini, simon.guinot, linux-efi, kvm, rkrcmar, matt,
linux-pci, linus.walleij, gary.hook, linux-mm, paul.gortmaker,
hpa, cl, dan.j.williams, aarcange, sfr, andriy.shevchenko,
herbert, bhe, xemul, joro, x86, peterz, piotr.luc, mingo, msalter,
ross.zwisler, dyoung, thomas.lendacky, jroedel, keescook, arnd,
toshi.kani, mathieu.desnoyers, luto, d
In-Reply-To: <1fe1e177-f588-fe5a-dc13-e9fde00e8958@amd.com>
On Fri, Mar 10, 2017 at 10:35:30AM -0600, Brijesh Singh wrote:
> We could update this patch to use the below logic:
>
> * CPUID(0) - Check for AuthenticAMD
> * CPID(1) - Check if under hypervisor
> * CPUID(0x80000000) - Check for highest supported leaf
> * CPUID(0x8000001F).EAX - Check for SME and SEV support
> * rdmsr (MSR_K8_SYSCFG)[MemEncryptionModeEnc] - Check if SMEE is set
Actually, it is still not clear to me *why* we need to do anything
special wrt SEV in the guest.
Lemme clarify: why can't the guest boot just like a normal Linux on
baremetal and use the SME(!) detection code to set sme_enable and so
on? IOW, I'd like to avoid all those checks whether we're running under
hypervisor and handle all that like we're running on baremetal.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
^ permalink raw reply
* Re: [PATCH v5 1/2] crypto: skcipher AF_ALG - overhaul memory management
From: Stephan Müller @ 2017-03-16 10:18 UTC (permalink / raw)
To: Herbert Xu; +Cc: linux-crypto
In-Reply-To: <20170316095248.GA11996@gondor.apana.org.au>
Am Donnerstag, 16. März 2017, 10:52:48 CET schrieb Herbert Xu:
Hi Herbert,
> First of all you're only limiting the amount of memory occupied
> by the SG list which is not the same thing as the memory pinned
> down by the actual recvmsg.
I am fully aware of that. As this was present in the code, I thought I could
reuse that approach.
Are you saying that you want to stop this approach?
>
> More importantly, with the current code, a very large recvmsg
> would still work by doing it piecemeal.
This piecemeal would be done in the sync case. In the current AIO code path,
such piecemeal would not be achieved.
> With your patch, won't
> it fail because sock_kmalloc could fail to allocate memory for
> the whole thing?
I this case yes. To handle AIO and sync in a common code path (and thus make
both behave identically), this would be the result.
If very large recvmsg requests are to be handled, the socket option could be
increased or the caller would piecemeal that request.
Otherwise, if do a piecemeal handling like it is done now, I see the following
drawbacks:
- no common sync/AIO code and no common behavior
- no common approach between skcipher and other implementations (algif_aead
and the algif_akcipher that I have in the pipe which looks identically to the
proposed patchset)
Ciao
Stephan
^ permalink raw reply
* Re: [RFC PATCH v2 23/32] kvm: introduce KVM_MEMORY_ENCRYPT_OP ioctl
From: Paolo Bonzini @ 2017-03-16 10:25 UTC (permalink / raw)
To: Brijesh Singh, simon.guinot, linux-efi, kvm, rkrcmar, matt,
linux-pci, linus.walleij, gary.hook, linux-mm, paul.gortmaker,
hpa, cl, dan.j.williams, aarcange, sfr, andriy.shevchenko,
herbert, bhe, xemul, joro, x86, peterz, piotr.luc, mingo, msalter,
ross.zwisler, bp, dyoung, thomas.lendacky, jroedel, keescook,
arnd, toshi.kani, mathieu.desnoyers, luto
In-Reply-To: <148846783136.2349.9362218518503742320.stgit@brijesh-build-machine>
On 02/03/2017 16:17, Brijesh Singh wrote:
> If hardware supports encrypting then KVM_MEMORY_ENCRYPT_OP ioctl can
> be used by qemu to issue platform specific memory encryption commands.
>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
> arch/x86/include/asm/kvm_host.h | 2 ++
> arch/x86/kvm/x86.c | 12 ++++++++++++
> include/uapi/linux/kvm.h | 2 ++
> 3 files changed, 16 insertions(+)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index bff1f15..62651ad 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1033,6 +1033,8 @@ struct kvm_x86_ops {
> void (*cancel_hv_timer)(struct kvm_vcpu *vcpu);
>
> void (*setup_mce)(struct kvm_vcpu *vcpu);
> +
> + int (*memory_encryption_op)(struct kvm *kvm, void __user *argp);
> };
>
> struct kvm_arch_async_pf {
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 2099df8..6a737e9 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3926,6 +3926,14 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
> return r;
> }
>
> +static int kvm_vm_ioctl_memory_encryption_op(struct kvm *kvm, void __user *argp)
> +{
> + if (kvm_x86_ops->memory_encryption_op)
> + return kvm_x86_ops->memory_encryption_op(kvm, argp);
> +
> + return -ENOTTY;
> +}
> +
> long kvm_arch_vm_ioctl(struct file *filp,
> unsigned int ioctl, unsigned long arg)
> {
> @@ -4189,6 +4197,10 @@ long kvm_arch_vm_ioctl(struct file *filp,
> r = kvm_vm_ioctl_enable_cap(kvm, &cap);
> break;
> }
> + case KVM_MEMORY_ENCRYPT_OP: {
> + r = kvm_vm_ioctl_memory_encryption_op(kvm, argp);
> + break;
> + }
> default:
> r = kvm_vm_ioctl_assigned_device(kvm, ioctl, arg);
> }
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index cac48ed..fef7d83 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1281,6 +1281,8 @@ struct kvm_s390_ucas_mapping {
> #define KVM_S390_GET_IRQ_STATE _IOW(KVMIO, 0xb6, struct kvm_s390_irq_state)
> /* Available with KVM_CAP_X86_SMM */
> #define KVM_SMI _IO(KVMIO, 0xb7)
> +/* Memory Encryption Commands */
> +#define KVM_MEMORY_ENCRYPT_OP _IOWR(KVMIO, 0xb8, unsigned long)
>
> #define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0)
> #define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1)
>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply
* Re: [RFC PATCH v2 24/32] kvm: x86: prepare for SEV guest management API support
From: Paolo Bonzini @ 2017-03-16 10:33 UTC (permalink / raw)
To: Brijesh Singh, simon.guinot, linux-efi, kvm, rkrcmar, matt,
linux-pci, linus.walleij, gary.hook, linux-mm, paul.gortmaker,
hpa, cl, dan.j.williams, aarcange, sfr, andriy.shevchenko,
herbert, bhe, xemul, joro, x86, peterz, piotr.luc, mingo, msalter,
ross.zwisler, bp, dyoung, thomas.lendacky, jroedel, keescook,
arnd, toshi.kani, mathieu.desnoyers, luto
In-Reply-To: <148846784278.2349.17771314083820274411.stgit@brijesh-build-machine>
On 02/03/2017 16:17, Brijesh Singh wrote:
> ASID management:
> - Reserve asid range for SEV guest, SEV asid range is obtained through
> CPUID Fn8000_001f[ECX]. A non-SEV guest can use any asid outside the SEV
> asid range.
How is backwards compatibility handled?
> - SEV guest must have asid value within asid range obtained through CPUID.
> - SEV guest must have the same asid for all vcpu's. A TLB flush is required
> if different vcpu for the same ASID is to be run on the same host CPU.
[...]
> +
> + /* which host cpu was used for running this vcpu */
> + bool last_cpuid;
Should be unsigned int.
>
> + /* Assign the asid allocated for this SEV guest */
> + svm->vmcb->control.asid = asid;
> +
> + /* Flush guest TLB:
> + * - when different VMCB for the same ASID is to be run on the
> + * same host CPU
> + * or
> + * - this VMCB was executed on different host cpu in previous VMRUNs.
> + */
> + if (sd->sev_vmcbs[asid] != (void *)svm->vmcb ||
Why the cast?
> + svm->last_cpuid != cpu)
> + svm->vmcb->control.tlb_ctl = TLB_CONTROL_FLUSH_ALL_ASID;
If there is a match, you don't need to do anything else (neither reset
the asid, nor mark it as dirty, nor update the fields), so:
if (sd->sev_vmcbs[asid] == svm->vmcb &&
svm->last_cpuid == cpu)
return;
svm->last_cpuid = cpu;
sd->sev_vmcbs[asid] = svm->vmcb;
svm->vmcb->control.tlb_ctl = TLB_CONTROL_FLUSH_ALL_ASID;
svm->vmcb->control.asid = asid;
mark_dirty(svm->vmcb, VMCB_ASID);
(plus comments ;)).
Also, why not TLB_CONTROL_FLUSH_ASID if possible?
> + svm->last_cpuid = cpu;
> + sd->sev_vmcbs[asid] = (void *)svm->vmcb;
> +
> + mark_dirty(svm->vmcb, VMCB_ASID);
[...]
>
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index fef7d83..9df37a2 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1284,6 +1284,104 @@ struct kvm_s390_ucas_mapping {
> /* Memory Encryption Commands */
> #define KVM_MEMORY_ENCRYPT_OP _IOWR(KVMIO, 0xb8, unsigned long)
>
> +/* Secure Encrypted Virtualization mode */
> +enum sev_cmd_id {
Please add documentation in Documentation/virtual/kvm/memory_encrypt.txt.
Paolo
^ permalink raw reply
* Re: [RFC PATCH v2 32/32] x86: kvm: Pin the guest memory when SEV is active
From: Paolo Bonzini @ 2017-03-16 10:38 UTC (permalink / raw)
To: Brijesh Singh, simon.guinot, linux-efi, kvm, rkrcmar, matt,
linux-pci, linus.walleij, gary.hook, linux-mm, paul.gortmaker,
hpa, cl, dan.j.williams, aarcange, sfr, andriy.shevchenko,
herbert, bhe, xemul, joro, x86, peterz, piotr.luc, mingo, msalter,
ross.zwisler, bp, dyoung, thomas.lendacky, jroedel, keescook,
arnd, toshi.kani, mathieu.desnoyers, luto
In-Reply-To: <148846793743.2349.8478208161427437950.stgit@brijesh-build-machine>
On 02/03/2017 16:18, Brijesh Singh wrote:
> The SEV memory encryption engine uses a tweak such that two identical
> plaintexts at different location will have a different ciphertexts.
> So swapping or moving ciphertexts of two pages will not result in
> plaintexts being swapped. Relocating (or migrating) a physical backing pages
> for SEV guest will require some additional steps. The current SEV key
> management spec [1] does not provide commands to swap or migrate (move)
> ciphertexts. For now we pin the memory allocated for the SEV guest. In
> future when SEV key management spec provides the commands to support the
> page migration we can update the KVM code to remove the pinning logical
> without making any changes into userspace (qemu).
>
> The patch pins userspace memory when a new slot is created and unpin the
> memory when slot is removed.
>
> [1] http://support.amd.com/TechDocs/55766_SEV-KM%20API_Spec.pdf
This is not enough, because memory can be hidden temporarily from the
guest and remapped later. Think of a PCI BAR that is backed by RAM, or
also SMRAM. The pinning must be kept even in that case.
You need to add a pair of KVM_MEMORY_ENCRYPT_OPs (one that doesn't map
to a PSP operation), such as KVM_REGISTER/UNREGISTER_ENCRYPTED_RAM. In
QEMU you can use a RAMBlockNotifier to invoke the ioctls.
Paolo
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
> arch/x86/include/asm/kvm_host.h | 6 +++
> arch/x86/kvm/svm.c | 93 +++++++++++++++++++++++++++++++++++++++
> arch/x86/kvm/x86.c | 3 +
> 3 files changed, 102 insertions(+)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index fcc4710..9dc59f0 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -723,6 +723,7 @@ struct kvm_sev_info {
> unsigned int handle; /* firmware handle */
> unsigned int asid; /* asid for this guest */
> int sev_fd; /* SEV device fd */
> + struct list_head pinned_memory_slot;
> };
>
> struct kvm_arch {
> @@ -1043,6 +1044,11 @@ struct kvm_x86_ops {
> void (*setup_mce)(struct kvm_vcpu *vcpu);
>
> int (*memory_encryption_op)(struct kvm *kvm, void __user *argp);
> +
> + void (*prepare_memory_region)(struct kvm *kvm,
> + struct kvm_memory_slot *memslot,
> + const struct kvm_userspace_memory_region *mem,
> + enum kvm_mr_change change);
> };
>
> struct kvm_arch_async_pf {
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 13996d6..ab973f9 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -498,12 +498,21 @@ static inline bool gif_set(struct vcpu_svm *svm)
> }
>
> /* Secure Encrypted Virtualization */
> +struct kvm_sev_pinned_memory_slot {
> + struct list_head list;
> + unsigned long npages;
> + struct page **pages;
> + unsigned long userspace_addr;
> + short id;
> +};
> +
> static unsigned int max_sev_asid;
> static unsigned long *sev_asid_bitmap;
> static void sev_deactivate_handle(struct kvm *kvm);
> static void sev_decommission_handle(struct kvm *kvm);
> static int sev_asid_new(void);
> static void sev_asid_free(int asid);
> +static void sev_unpin_memory(struct page **pages, unsigned long npages);
> #define __sev_page_pa(x) ((page_to_pfn(x) << PAGE_SHIFT) | sme_me_mask)
>
> static bool kvm_sev_enabled(void)
> @@ -1544,9 +1553,25 @@ static inline int avic_free_vm_id(int id)
>
> static void sev_vm_destroy(struct kvm *kvm)
> {
> + struct list_head *pos, *q;
> + struct kvm_sev_pinned_memory_slot *pinned_slot;
> + struct list_head *head = &kvm->arch.sev_info.pinned_memory_slot;
> +
> if (!sev_guest(kvm))
> return;
>
> + /* if guest memory is pinned then unpin it now */
> + if (!list_empty(head)) {
> + list_for_each_safe(pos, q, head) {
> + pinned_slot = list_entry(pos,
> + struct kvm_sev_pinned_memory_slot, list);
> + sev_unpin_memory(pinned_slot->pages,
> + pinned_slot->npages);
> + list_del(pos);
> + kfree(pinned_slot);
> + }
> + }
> +
> /* release the firmware resources */
> sev_deactivate_handle(kvm);
> sev_decommission_handle(kvm);
> @@ -5663,6 +5688,8 @@ static int sev_pre_start(struct kvm *kvm, int *asid)
> }
> *asid = ret;
> ret = 0;
> +
> + INIT_LIST_HEAD(&kvm->arch.sev_info.pinned_memory_slot);
> }
>
> return ret;
> @@ -6189,6 +6216,71 @@ static int sev_launch_measure(struct kvm *kvm, struct kvm_sev_cmd *argp)
> return ret;
> }
>
> +static struct kvm_sev_pinned_memory_slot *sev_find_pinned_memory_slot(
> + struct kvm *kvm, struct kvm_memory_slot *slot)
> +{
> + struct kvm_sev_pinned_memory_slot *i;
> + struct list_head *head = &kvm->arch.sev_info.pinned_memory_slot;
> +
> + list_for_each_entry(i, head, list) {
> + if (i->userspace_addr == slot->userspace_addr &&
> + i->id == slot->id)
> + return i;
> + }
> +
> + return NULL;
> +}
> +
> +static void amd_prepare_memory_region(struct kvm *kvm,
> + struct kvm_memory_slot *memslot,
> + const struct kvm_userspace_memory_region *mem,
> + enum kvm_mr_change change)
> +{
> + struct kvm_sev_pinned_memory_slot *pinned_slot;
> + struct list_head *head = &kvm->arch.sev_info.pinned_memory_slot;
> +
> + mutex_lock(&kvm->lock);
> +
> + if (!sev_guest(kvm))
> + goto unlock;
> +
> + if (change == KVM_MR_CREATE) {
> +
> + if (!mem->memory_size)
> + goto unlock;
> +
> + pinned_slot = kmalloc(sizeof(*pinned_slot), GFP_KERNEL);
> + if (pinned_slot == NULL)
> + goto unlock;
> +
> + pinned_slot->pages = sev_pin_memory(mem->userspace_addr,
> + mem->memory_size, &pinned_slot->npages);
> + if (pinned_slot->pages == NULL) {
> + kfree(pinned_slot);
> + goto unlock;
> + }
> +
> + sev_clflush_pages(pinned_slot->pages, pinned_slot->npages);
> +
> + pinned_slot->id = memslot->id;
> + pinned_slot->userspace_addr = mem->userspace_addr;
> + list_add_tail(&pinned_slot->list, head);
> +
> + } else if (change == KVM_MR_DELETE) {
> +
> + pinned_slot = sev_find_pinned_memory_slot(kvm, memslot);
> + if (!pinned_slot)
> + goto unlock;
> +
> + sev_unpin_memory(pinned_slot->pages, pinned_slot->npages);
> + list_del(&pinned_slot->list);
> + kfree(pinned_slot);
> + }
> +
> +unlock:
> + mutex_unlock(&kvm->lock);
> +}
> +
> static int amd_memory_encryption_cmd(struct kvm *kvm, void __user *argp)
> {
> int r = -ENOTTY;
> @@ -6355,6 +6447,7 @@ static struct kvm_x86_ops svm_x86_ops __ro_after_init = {
> .update_pi_irte = svm_update_pi_irte,
>
> .memory_encryption_op = amd_memory_encryption_cmd,
> + .prepare_memory_region = amd_prepare_memory_region,
> };
>
> static int __init svm_init(void)
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 6a737e9..e05069d 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -8195,6 +8195,9 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,
> const struct kvm_userspace_memory_region *mem,
> enum kvm_mr_change change)
> {
> + if (kvm_x86_ops->prepare_memory_region)
> + kvm_x86_ops->prepare_memory_region(kvm, memslot, mem, change);
> +
> return 0;
> }
>
>
^ permalink raw reply
* Re: [PATCH 2/4] crypto: powerpc - Re-enable non-REFLECTed CRCs
From: Michael Ellerman @ 2017-03-16 10:44 UTC (permalink / raw)
To: Daniel Axtens, linuxppc-dev, linux-crypto; +Cc: anton, Daniel Axtens
In-Reply-To: <20170315123737.20234-2-dja@axtens.net>
Daniel Axtens <dja@axtens.net> writes:
> When CRC32c was included in the kernel, Anton ripped out
> the #ifdefs around reflected polynomials, because CRC32c
> is always reflected. However, not all CRCs use reflection
> so we'd like to make it optional.
>
> Restore the REFLECT parts from Anton's original CRC32
> implementation (https://github.com/antonblanchard/crc32-vpmsum)
>
> That implementation is available under GPLv2+, so we're OK
> from a licensing point of view:
> https://github.com/antonblanchard/crc32-vpmsum/blob/master/LICENSE.TXT
It's also written by Anton and copyright IBM, so you (we (IBM)) could
always just relicense it anyway.
So doubly OK IMO.
cheers
^ permalink raw reply
* Re: [PATCH 1/4] crypto: powerpc - Factor out the core CRC vpmsum algorithm
From: Michael Ellerman @ 2017-03-16 10:45 UTC (permalink / raw)
To: Daniel Axtens, linuxppc-dev, linux-crypto; +Cc: anton, Daniel Axtens
In-Reply-To: <20170315123737.20234-1-dja@axtens.net>
Daniel Axtens <dja@axtens.net> writes:
> The core nuts and bolts of the crc32c vpmsum algorithm will
> also work for a number of other CRC algorithms with different
> polynomials. Factor out the function into a new asm file.
>
> To handle multiple users of the function, a user simply
> provides constants, defines the name of their CRC function,
> and then #includes the core algorithm file.
>
> Cc: Anton Blanchard <anton@samba.org>
> Signed-off-by: Daniel Axtens <dja@axtens.net>
>
> --
>
> It's possible at this point to argue that the address
> of the constant tables should be passed in to the function,
> rather than doing this somewhat unconventional #include.
>
> However, we're about to add further #ifdef's back into the core
> that will be provided by the encapsulaing code, and which couldn't
> be done as a variable without performance loss.
> ---
> arch/powerpc/crypto/crc32-vpmsum_core.S | 726 ++++++++++++++++++++++++++++++++
> arch/powerpc/crypto/crc32c-vpmsum_asm.S | 714 +------------------------------
> 2 files changed, 729 insertions(+), 711 deletions(-)
> create mode 100644 arch/powerpc/crypto/crc32-vpmsum_core.S
So although this sits in arch/powerpc, it's heavy on the crypto which is
not my area of expertise (to say the least!), so I think it should
probably go via Herbert and the crypto tree?
cheers
^ permalink raw reply
* Re: [RFC PATCH v2 26/32] kvm: svm: Add support for SEV LAUNCH_UPDATE_DATA command
From: Paolo Bonzini @ 2017-03-16 10:48 UTC (permalink / raw)
To: Brijesh Singh, simon.guinot, linux-efi, kvm, rkrcmar, matt,
linux-pci, linus.walleij, gary.hook, linux-mm, paul.gortmaker,
hpa, cl, dan.j.williams, aarcange, sfr, andriy.shevchenko,
herbert, bhe, xemul, joro, x86, peterz, piotr.luc, mingo, msalter,
ross.zwisler, bp, dyoung, thomas.lendacky, jroedel, keescook,
arnd, toshi.kani, mathieu.desnoyers, luto
In-Reply-To: <148846786714.2349.17724971671841396908.stgit@brijesh-build-machine>
On 02/03/2017 16:17, Brijesh Singh wrote:
> +static struct page **sev_pin_memory(unsigned long uaddr, unsigned long ulen,
> + unsigned long *n)
> +{
> + struct page **pages;
> + int first, last;
> + unsigned long npages, pinned;
> +
> + /* Get number of pages */
> + first = (uaddr & PAGE_MASK) >> PAGE_SHIFT;
> + last = ((uaddr + ulen - 1) & PAGE_MASK) >> PAGE_SHIFT;
> + npages = (last - first + 1);
> +
> + pages = kzalloc(npages * sizeof(struct page *), GFP_KERNEL);
> + if (!pages)
> + return NULL;
> +
> + /* pin the user virtual address */
> + down_read(¤t->mm->mmap_sem);
> + pinned = get_user_pages_fast(uaddr, npages, 1, pages);
> + up_read(¤t->mm->mmap_sem);
get_user_pages_fast, like get_user_pages_unlocked, must be called
without mmap_sem held.
> + if (pinned != npages) {
> + printk(KERN_ERR "SEV: failed to pin %ld pages (got %ld)\n",
> + npages, pinned);
> + goto err;
> + }
> +
> + *n = npages;
> + return pages;
> +err:
> + if (pinned > 0)
> + release_pages(pages, pinned, 0);
> + kfree(pages);
> +
> + return NULL;
> +}
>
> + /* the array of pages returned by get_user_pages() is a page-aligned
> + * memory. Since the user buffer is probably not page-aligned, we need
> + * to calculate the offset within a page for first update entry.
> + */
> + offset = uaddr & (PAGE_SIZE - 1);
> + len = min_t(size_t, (PAGE_SIZE - offset), ulen);
> + ulen -= len;
> +
> + /* update first page -
> + * special care need to be taken for the first page because we might
> + * be dealing with offset within the page
> + */
No need to special case the first page; just set "offset = 0" inside the
loop after the first iteration.
Paolo
> + data->handle = sev_get_handle(kvm);
> + data->length = len;
> + data->address = __sev_page_pa(inpages[0]) + offset;
> + ret = sev_issue_cmd(kvm, SEV_CMD_LAUNCH_UPDATE_DATA,
> + data, &argp->error);
> + if (ret)
> + goto err_3;
> +
> + /* update remaining pages */
> + for (i = 1; i < nr_pages; i++) {
> +
> + len = min_t(size_t, PAGE_SIZE, ulen);
> + ulen -= len;
> + data->length = len;
> + data->address = __sev_page_pa(inpages[i]);
> + ret = sev_issue_cmd(kvm, SEV_CMD_LAUNCH_UPDATE_DATA,
> + data, &argp->error);
> + if (ret)
> + goto err_3;
> + }
^ permalink raw reply
* Re: [RFC PATCH v2 29/32] kvm: svm: Add support for SEV DEBUG_DECRYPT command
From: Paolo Bonzini @ 2017-03-16 10:54 UTC (permalink / raw)
To: Brijesh Singh, simon.guinot, linux-efi, kvm, rkrcmar, matt,
linux-pci, linus.walleij, gary.hook, linux-mm, paul.gortmaker,
hpa, cl, dan.j.williams, aarcange, sfr, andriy.shevchenko,
herbert, bhe, xemul, joro, x86, peterz, piotr.luc, mingo, msalter,
ross.zwisler, bp, dyoung, thomas.lendacky, jroedel, keescook,
arnd, toshi.kani, mathieu.desnoyers, luto
In-Reply-To: <148846789744.2349.167641684941925238.stgit@brijesh-build-machine>
On 02/03/2017 16:18, Brijesh Singh wrote:
> +static int __sev_dbg_decrypt_page(struct kvm *kvm, unsigned long src,
> + void *dst, int *error)
> +{
> + inpages = sev_pin_memory(src, PAGE_SIZE, &npages);
> + if (!inpages) {
> + ret = -ENOMEM;
> + goto err_1;
> + }
> +
> + data->handle = sev_get_handle(kvm);
> + data->dst_addr = __psp_pa(dst);
> + data->src_addr = __sev_page_pa(inpages[0]);
> + data->length = PAGE_SIZE;
> +
> + ret = sev_issue_cmd(kvm, SEV_CMD_DBG_DECRYPT, data, error);
> + if (ret)
> + printk(KERN_ERR "SEV: DEBUG_DECRYPT %d (%#010x)\n",
> + ret, *error);
> + sev_unpin_memory(inpages, npages);
> +err_1:
> + kfree(data);
> + return ret;
> +}
> +
> +static int sev_dbg_decrypt(struct kvm *kvm, struct kvm_sev_cmd *argp)
> +{
> + void *data;
> + int ret, offset, len;
> + struct kvm_sev_dbg debug;
> +
> + if (!sev_guest(kvm))
> + return -ENOTTY;
> +
> + if (copy_from_user(&debug, (void *)argp->data,
> + sizeof(struct kvm_sev_dbg)))
> + return -EFAULT;
> + /*
> + * TODO: add support for decrypting length which crosses the
> + * page boundary.
> + */
> + offset = debug.src_addr & (PAGE_SIZE - 1);
> + if (offset + debug.length > PAGE_SIZE)
> + return -EINVAL;
> +
Please do add it, it doesn't seem very different from what you're doing
in LAUNCH_UPDATE_DATA. There's no need for a separate
__sev_dbg_decrypt_page function, you can just pin/unpin here and do a
per-page loop as in LAUNCH_UPDATE_DATA.
Paolo
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox