* Re: [PATCH 0/8] crypto: omap-sham: convert to sg based data engine
From: Herbert Xu @ 2016-09-22 10:46 UTC (permalink / raw)
To: Tero Kristo
Cc: lokeshvutla, davem, linux-crypto, linux-omap, linux-arm-kernel
In-Reply-To: <1474298539-23897-1-git-send-email-t-kristo@ti.com>
On Mon, Sep 19, 2016 at 06:22:11PM +0300, Tero Kristo wrote:
> Hi,
>
> This series converts the omap-sham buffer handling towards a scatterlist
> approach. This avoids the need to have a huge internal buffer within the
> driver, and also allows us to properly implement export/import for the
> driver. I tried splitting up the changes to some sane patches, but this
> was rather difficult due to the fact that this is largely a complete
> rewrite of portions of the driver. Patch #6 is a prime example of this
> being pretty large, but splitting this up would break bisectability.
> Hopefully the patch is still understandable though.
>
> Crypto manager tests work fine at least on omap3/am3/am4/dra7 SoC:s.
> (Something is broken with test farm again and could not try omap2/omap4.)
>
> Also tested tcrypt SHA performance on DRA7 and it seems to be working
> fine with different buffer sizes.
>
> My test branch is also available here for interested parties:
> tree: https://github.com/t-kristo/linux-pm.git
> breanch: 4.8-rc1-crypto
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] hwrng: omap - Only fail if pm_runtime_get_sync returns < 0
From: Herbert Xu @ 2016-09-22 10:46 UTC (permalink / raw)
To: Dave Gerlach
Cc: Nishanth Menon, Deepak Saxena, linux-kernel, linux-crypto,
Matt Mackall, linux-omap, linux-arm-kernel
In-Reply-To: <20160920152540.3004-1-d-gerlach@ti.com>
On Tue, Sep 20, 2016 at 10:25:40AM -0500, Dave Gerlach wrote:
> Currently omap-rng checks the return value of pm_runtime_get_sync and
> reports failure if anything is returned, however it should be checking
> if ret < 0 as pm_runtime_get_sync return 0 on success but also can return
> 1 if the device was already active which is not a failure case. Only
> values < 0 are actual failures.
>
> Fixes: 61dc0a446e5d ("hwrng: omap - Fix assumption that runtime_get_sync will always succeed")
> Signed-off-by: Dave Gerlach <d-gerlach@ti.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 v2] crypto: caam - fix sg dump
From: Herbert Xu @ 2016-09-22 10:47 UTC (permalink / raw)
To: Catalin Vasile
Cc: linux-crypto, linux-crypto-owner, horia.geanta, cata.vasile
In-Reply-To: <1474534678-10118-1-git-send-email-cata.vasile@nxp.com>
Catalin Vasile <cata.vasile@nxp.com> wrote:
> Ensure scatterlists have a virtual memory mapping before dumping.
>
> Signed-off-by: Catalin Vasile <cata.vasile@nxp.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 v2] crypto: caam - fix sg dump
From: Herbert Xu @ 2016-09-22 10:55 UTC (permalink / raw)
To: Horia Geanta Neag; +Cc: cata.vasile, linux-crypto, linux-crypto-owner
In-Reply-To: <DB4PR04MB0847584C19565F8B3FBE6ED698C90@DB4PR04MB0847.eurprd04.prod.outlook.com>
Horia Geanta Neag <horia.geanta@nxp.com> wrote:
>
>> +
>> +static void dbg_dump_sg(const char *level, const char *prefix_str,
>> + int prefix_type, int rowsize, int groupsize,
>> + struct scatterlist *sg, size_t tlen, bool ascii,
>> + bool may_sleep)
> may_sleep is no longer needed.
As I've already applied this patch, please send any improvements
as incremental patches.
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
* [PATCH] crypto: caam - fix dbg_dump_sg() style issues
From: Catalin Vasile @ 2016-09-22 10:58 UTC (permalink / raw)
To: linux-crypto; +Cc: linux-crypto-owner, horia.geanta, Catalin Vasile
Signed-off-by: Catalin Vasile <cata.vasile@nxp.com>
---
drivers/crypto/caam/caamalg.c | 43 ++++++++++++++++++++-----------------------
1 file changed, 20 insertions(+), 23 deletions(-)
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index eb97562..c8ebddb 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -117,8 +117,7 @@
static void dbg_dump_sg(const char *level, const char *prefix_str,
int prefix_type, int rowsize, int groupsize,
- struct scatterlist *sg, size_t tlen, bool ascii,
- bool may_sleep)
+ struct scatterlist *sg, size_t tlen, bool ascii)
{
struct scatterlist *it;
void *it_page;
@@ -132,7 +131,7 @@ static void dbg_dump_sg(const char *level, const char *prefix_str,
*/
it_page = kmap_atomic(sg_page(it));
if (unlikely(!it_page)) {
- printk(KERN_ERR "dbg_dump_sg: kmap failed\n");
+ pr_err("dbg_dump_sg: kmap failed\n");
return;
}
@@ -145,6 +144,12 @@ static void dbg_dump_sg(const char *level, const char *prefix_str,
kunmap_atomic(it_page);
}
}
+#else
+static void dbg_dump_sg(const char *level, const char *prefix_str,
+ int prefix_type, int rowsize, int groupsize,
+ struct scatterlist *sg, size_t tlen, bool ascii)
+{
+}
#endif
static struct list_head alg_list;
@@ -2018,10 +2023,11 @@ static void ablkcipher_encrypt_done(struct device *jrdev, u32 *desc, u32 err,
print_hex_dump(KERN_ERR, "dstiv @"__stringify(__LINE__)": ",
DUMP_PREFIX_ADDRESS, 16, 4, req->info,
edesc->src_nents > 1 ? 100 : ivsize, 1);
+#endif
+
dbg_dump_sg(KERN_ERR, "dst @"__stringify(__LINE__)": ",
DUMP_PREFIX_ADDRESS, 16, 4, req->dst,
- edesc->dst_nents > 1 ? 100 : req->nbytes, 1, true);
-#endif
+ edesc->dst_nents > 1 ? 100 : req->nbytes, 1);
ablkcipher_unmap(jrdev, edesc, req);
kfree(edesc);
@@ -2050,10 +2056,10 @@ static void ablkcipher_decrypt_done(struct device *jrdev, u32 *desc, u32 err,
print_hex_dump(KERN_ERR, "dstiv @"__stringify(__LINE__)": ",
DUMP_PREFIX_ADDRESS, 16, 4, req->info,
ivsize, 1);
+#endif
dbg_dump_sg(KERN_ERR, "dst @"__stringify(__LINE__)": ",
DUMP_PREFIX_ADDRESS, 16, 4, req->dst,
- edesc->dst_nents > 1 ? 100 : req->nbytes, 1, true);
-#endif
+ edesc->dst_nents > 1 ? 100 : req->nbytes, 1);
ablkcipher_unmap(jrdev, edesc, req);
kfree(edesc);
@@ -2207,16 +2213,14 @@ static void init_ablkcipher_job(u32 *sh_desc, dma_addr_t ptr,
int len, sec4_sg_index = 0;
#ifdef DEBUG
- bool may_sleep = ((req->base.flags & (CRYPTO_TFM_REQ_MAY_BACKLOG |
- CRYPTO_TFM_REQ_MAY_SLEEP)) != 0);
print_hex_dump(KERN_ERR, "presciv@"__stringify(__LINE__)": ",
DUMP_PREFIX_ADDRESS, 16, 4, req->info,
ivsize, 1);
printk(KERN_ERR "asked=%d, nbytes%d\n", (int)edesc->src_nents ? 100 : req->nbytes, req->nbytes);
+#endif
dbg_dump_sg(KERN_ERR, "src @"__stringify(__LINE__)": ",
DUMP_PREFIX_ADDRESS, 16, 4, req->src,
- edesc->src_nents ? 100 : req->nbytes, 1, may_sleep);
-#endif
+ edesc->src_nents ? 100 : req->nbytes, 1);
len = desc_len(sh_desc);
init_job_desc_shared(desc, ptr, len, HDR_SHARE_DEFER | HDR_REVERSE);
@@ -2267,15 +2271,13 @@ static void init_ablkcipher_giv_job(u32 *sh_desc, dma_addr_t ptr,
int len, sec4_sg_index = 0;
#ifdef DEBUG
- bool may_sleep = ((req->base.flags & (CRYPTO_TFM_REQ_MAY_BACKLOG |
- CRYPTO_TFM_REQ_MAY_SLEEP)) != 0);
print_hex_dump(KERN_ERR, "presciv@" __stringify(__LINE__) ": ",
DUMP_PREFIX_ADDRESS, 16, 4, req->info,
ivsize, 1);
+#endif
dbg_dump_sg(KERN_ERR, "src @" __stringify(__LINE__) ": ",
DUMP_PREFIX_ADDRESS, 16, 4, req->src,
- edesc->src_nents ? 100 : req->nbytes, 1, may_sleep);
-#endif
+ edesc->src_nents ? 100 : req->nbytes, 1);
len = desc_len(sh_desc);
init_job_desc_shared(desc, ptr, len, HDR_SHARE_DEFER | HDR_REVERSE);
@@ -2544,14 +2546,6 @@ static int aead_decrypt(struct aead_request *req)
u32 *desc;
int ret = 0;
-#ifdef DEBUG
- bool may_sleep = ((req->base.flags & (CRYPTO_TFM_REQ_MAY_BACKLOG |
- CRYPTO_TFM_REQ_MAY_SLEEP)) != 0);
- dbg_dump_sg(KERN_ERR, "dec src@"__stringify(__LINE__)": ",
- DUMP_PREFIX_ADDRESS, 16, 4, req->src,
- req->assoclen + req->cryptlen, 1, may_sleep);
-#endif
-
/* allocate extended descriptor */
edesc = aead_edesc_alloc(req, AUTHENC_DESC_JOB_IO_LEN,
&all_contig, false);
@@ -2565,6 +2559,9 @@ static int aead_decrypt(struct aead_request *req)
DUMP_PREFIX_ADDRESS, 16, 4, edesc->hw_desc,
desc_bytes(edesc->hw_desc), 1);
#endif
+ dbg_dump_sg(KERN_ERR, "dec src@"__stringify(__LINE__)": ",
+ DUMP_PREFIX_ADDRESS, 16, 4, req->src,
+ req->assoclen + req->cryptlen, 1);
desc = edesc->hw_desc;
ret = caam_jr_enqueue(jrdev, desc, aead_decrypt_done, req);
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v2] crypto: caam - fix sg dump
From: Horia Geanta Neag @ 2016-09-22 11:47 UTC (permalink / raw)
To: Herbert Xu
Cc: Cata Vasile, linux-crypto@vger.kernel.org,
linux-crypto-owner@vger.kernel.org
In-Reply-To: <20160922105500.GA6306@gondor.apana.org.au>
On 9/22/2016 1:55 PM, Herbert Xu wrote:
> Horia Geanta Neag <horia.geanta@nxp.com> wrote:
>>
>>> +
>>> +static void dbg_dump_sg(const char *level, const char *prefix_str,
>>> + int prefix_type, int rowsize, int groupsize,
>>> + struct scatterlist *sg, size_t tlen, bool ascii,
>>> + bool may_sleep)
>> may_sleep is no longer needed.
>
> As I've already applied this patch, please send any improvements
> as incremental patches.
I am trying to understand the process here...
Would adding an official maintainer for the driver avoid applying
patches without an ack?
(This is not the first time changes are applied without getting a chance
to comment. OTOH I admit there were also cases when there was absolute
silence.)
Thanks,
Horia
^ permalink raw reply
* Re: [BUG] crypto: atmel-aes - erro when compiling with VERBOSE_DEBUG enable
From: levent demir @ 2016-09-22 12:45 UTC (permalink / raw)
To: Herbert Xu; +Cc: linux-crypto, cyrille.pitchen
In-Reply-To: <20160922092629.GA5508@gondor.apana.org.au>
Fix debug function call in atmel_aes_write
Signed-off-by: Levent DEMIR <levent.demir@inria.fr>
---
drivers/crypto/atmel-aes.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
index e3d40a8..2b0f926 100644
--- a/drivers/crypto/atmel-aes.c
+++ b/drivers/crypto/atmel-aes.c
@@ -317,7 +317,7 @@ static inline void atmel_aes_write(struct
atmel_aes_dev *dd,
char tmp[16];
dev_vdbg(dd->dev, "write 0x%08x into %s\n", value,
- atmel_aes_reg_name(offset, tmp));
+ atmel_aes_reg_name(offset, tmp, sizeof(tmp)));
}
#endif /* VERBOSE_DEBUG */
--
2.5.5
Le jeudi 22 septembre 2016 à 17:26 +0800, Herbert Xu a écrit :
> levent demir <levent.demir@inria.fr> wrote:
> > Hello,
> >
> > if you enable VERBOSE_DEBUG and compile you will have the following
> > error :
> >
> > drivers/crypto/atmel-aes.c:323:5: error: too few arguments to function
> > 'atmel_aes_reg_name'
> > atmel_aes_reg_name(offset, tmp));
> > ^
> > include/linux/device.h:1306:41: note: in definition of macro 'dev_vdbg'
> > dev_printk(KERN_DEBUG, dev, format, ##arg); \
> > ^
> > drivers/crypto/atmel-aes.c:205:20: note: declared here
> > static const char *atmel_aes_reg_name(u32 offset, char *tmp, size_t sz)
> >
> > Indeed, in atmel_aes_write function the call to atmel_aes_reg_name
> > contains only two arguments instead of 3 :
> >
> > atmel_aes_reg_name(offset, tmp));
> >
> > To fix it, one has to only add the size of tmp as third argument :
> >
> > atmel_aes_reg_name(offset, tmp, sizeof(tmp)));
>
> Thanks for the patch. In order to apply it, you need to fix the
> white-space damage as well as add a sign-off. For details please
> refer to Documentation/SubmittingPatches.
>
> Cheers,
^ permalink raw reply related
* Re: [PATCH v2] crypto: caam - fix sg dump
From: Herbert Xu @ 2016-09-22 13:08 UTC (permalink / raw)
To: Horia Geanta Neag
Cc: Cata Vasile, linux-crypto@vger.kernel.org,
linux-crypto-owner@vger.kernel.org
In-Reply-To: <DB4PR04MB0847AAA0EE82226E33D8155D98C90@DB4PR04MB0847.eurprd04.prod.outlook.com>
On Thu, Sep 22, 2016 at 11:47:49AM +0000, Horia Geanta Neag wrote:
>
> I am trying to understand the process here...
> Would adding an official maintainer for the driver avoid applying
> patches without an ack?
> (This is not the first time changes are applied without getting a chance
> to comment. OTOH I admit there were also cases when there was absolute
> silence.)
Sure, it wouldn't hurt.
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
* [cryptodev:master 132/132] drivers/crypto/caam/caamalg.c:140:9: note: in expansion of macro 'min'
From: kbuild test robot @ 2016-09-22 14:02 UTC (permalink / raw)
To: Catalin Vasile; +Cc: kbuild-all, linux-crypto, Herbert Xu
[-- Attachment #1: Type: text/plain, Size: 5233 bytes --]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
head: 5ecf8ef9103cb018cbd82b6eace529ff4c5b5c66
commit: 5ecf8ef9103cb018cbd82b6eace529ff4c5b5c66 [132/132] crypto: caam - fix sg dump
config: arm64-allmodconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 5ecf8ef9103cb018cbd82b6eace529ff4c5b5c66
# save the attached .config to linux build tree
make.cross ARCH=arm64
All warnings (new ones prefixed by >>):
In file included from drivers/crypto/caam/compat.h:8:0,
from drivers/crypto/caam/caamalg.c:47:
drivers/crypto/caam/caamalg.c: In function 'dbg_dump_sg':
include/linux/kernel.h:742:17: warning: comparison of distinct pointer types lacks a cast
(void) (&_min1 == &_min2); \
^
>> drivers/crypto/caam/caamalg.c:140:9: note: in expansion of macro 'min'
len = min(tlen, it->length);
^~~
vim +/min +140 drivers/crypto/caam/caamalg.c
41 * | SEQ_IN_PTR |
42 * | (input buffer) |
43 * | (input length) |
44 * ---------------------
45 */
46
> 47 #include "compat.h"
48
49 #include "regs.h"
50 #include "intern.h"
51 #include "desc_constr.h"
52 #include "jr.h"
53 #include "error.h"
54 #include "sg_sw_sec4.h"
55 #include "key_gen.h"
56
57 /*
58 * crypto alg
59 */
60 #define CAAM_CRA_PRIORITY 3000
61 /* max key is sum of AES_MAX_KEY_SIZE, max split key size */
62 #define CAAM_MAX_KEY_SIZE (AES_MAX_KEY_SIZE + \
63 CTR_RFC3686_NONCE_SIZE + \
64 SHA512_DIGEST_SIZE * 2)
65 /* max IV is max of AES_BLOCK_SIZE, DES3_EDE_BLOCK_SIZE */
66 #define CAAM_MAX_IV_LENGTH 16
67
68 #define AEAD_DESC_JOB_IO_LEN (DESC_JOB_IO_LEN + CAAM_CMD_SZ * 2)
69 #define GCM_DESC_JOB_IO_LEN (AEAD_DESC_JOB_IO_LEN + \
70 CAAM_CMD_SZ * 4)
71 #define AUTHENC_DESC_JOB_IO_LEN (AEAD_DESC_JOB_IO_LEN + \
72 CAAM_CMD_SZ * 5)
73
74 /* length of descriptors text */
75 #define DESC_AEAD_BASE (4 * CAAM_CMD_SZ)
76 #define DESC_AEAD_ENC_LEN (DESC_AEAD_BASE + 11 * CAAM_CMD_SZ)
77 #define DESC_AEAD_DEC_LEN (DESC_AEAD_BASE + 15 * CAAM_CMD_SZ)
78 #define DESC_AEAD_GIVENC_LEN (DESC_AEAD_ENC_LEN + 9 * CAAM_CMD_SZ)
79
80 /* Note: Nonce is counted in enckeylen */
81 #define DESC_AEAD_CTR_RFC3686_LEN (4 * CAAM_CMD_SZ)
82
83 #define DESC_AEAD_NULL_BASE (3 * CAAM_CMD_SZ)
84 #define DESC_AEAD_NULL_ENC_LEN (DESC_AEAD_NULL_BASE + 11 * CAAM_CMD_SZ)
85 #define DESC_AEAD_NULL_DEC_LEN (DESC_AEAD_NULL_BASE + 13 * CAAM_CMD_SZ)
86
87 #define DESC_GCM_BASE (3 * CAAM_CMD_SZ)
88 #define DESC_GCM_ENC_LEN (DESC_GCM_BASE + 16 * CAAM_CMD_SZ)
89 #define DESC_GCM_DEC_LEN (DESC_GCM_BASE + 12 * CAAM_CMD_SZ)
90
91 #define DESC_RFC4106_BASE (3 * CAAM_CMD_SZ)
92 #define DESC_RFC4106_ENC_LEN (DESC_RFC4106_BASE + 13 * CAAM_CMD_SZ)
93 #define DESC_RFC4106_DEC_LEN (DESC_RFC4106_BASE + 13 * CAAM_CMD_SZ)
94
95 #define DESC_RFC4543_BASE (3 * CAAM_CMD_SZ)
96 #define DESC_RFC4543_ENC_LEN (DESC_RFC4543_BASE + 11 * CAAM_CMD_SZ)
97 #define DESC_RFC4543_DEC_LEN (DESC_RFC4543_BASE + 12 * CAAM_CMD_SZ)
98
99 #define DESC_ABLKCIPHER_BASE (3 * CAAM_CMD_SZ)
100 #define DESC_ABLKCIPHER_ENC_LEN (DESC_ABLKCIPHER_BASE + \
101 20 * CAAM_CMD_SZ)
102 #define DESC_ABLKCIPHER_DEC_LEN (DESC_ABLKCIPHER_BASE + \
103 15 * CAAM_CMD_SZ)
104
105 #define DESC_MAX_USED_BYTES (CAAM_DESC_BYTES_MAX - DESC_JOB_IO_LEN)
106 #define DESC_MAX_USED_LEN (DESC_MAX_USED_BYTES / CAAM_CMD_SZ)
107
108 #ifdef DEBUG
109 /* for print_hex_dumps with line references */
110 #define debug(format, arg...) printk(format, arg)
111 #else
112 #define debug(format, arg...)
113 #endif
114
115 #ifdef DEBUG
116 #include <linux/highmem.h>
117
118 static void dbg_dump_sg(const char *level, const char *prefix_str,
119 int prefix_type, int rowsize, int groupsize,
120 struct scatterlist *sg, size_t tlen, bool ascii,
121 bool may_sleep)
122 {
123 struct scatterlist *it;
124 void *it_page;
125 size_t len;
126 void *buf;
127
128 for (it = sg; it != NULL && tlen > 0 ; it = sg_next(sg)) {
129 /*
130 * make sure the scatterlist's page
131 * has a valid virtual memory mapping
132 */
133 it_page = kmap_atomic(sg_page(it));
134 if (unlikely(!it_page)) {
135 printk(KERN_ERR "dbg_dump_sg: kmap failed\n");
136 return;
137 }
138
139 buf = it_page + it->offset;
> 140 len = min(tlen, it->length);
141 print_hex_dump(level, prefix_str, prefix_type, rowsize,
142 groupsize, buf, len, ascii);
143 tlen -= len;
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 51326 bytes --]
^ permalink raw reply
* Re: [RFC PATCH v1 05/28] KVM: SVM: prepare for new bit definition in nested_ctl
From: Borislav Petkov @ 2016-09-22 14:17 UTC (permalink / raw)
To: Brijesh Singh
Cc: simon.guinot, linux-efi, kvm, rkrcmar, matt, linus.walleij,
linux-mm, paul.gortmaker, hpa, dan.j.williams, aarcange, sfr,
andriy.shevchenko, herbert, bhe, xemul, joro, x86, mingo, msalter,
ross.zwisler, dyoung, thomas.lendacky, jroedel, keescook,
toshi.kani, mathieu.desnoyers, devel, tglx, mchehab,
iamjoonsoo.kim, labbott, tony.luck, alexandre.bounine
In-Reply-To: <147190827232.9523.18200019204059963746.stgit@brijesh-build-machine>
On Mon, Aug 22, 2016 at 07:24:32PM -0400, Brijesh Singh wrote:
> From: Tom Lendacky <thomas.lendacky@amd.com>
>
> Currently the nested_ctl variable in the vmcb_control_area structure is
> used to indicate nested paging support. The nested paging support field
> is actually defined as bit 0 of the this field. In order to support a new
> feature flag the usage of the nested_ctl and nested paging support must
> be converted to operate on a single bit.
>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
> arch/x86/include/asm/svm.h | 2 ++
> arch/x86/kvm/svm.c | 7 ++++---
> 2 files changed, 6 insertions(+), 3 deletions(-)
Reviewed-by: Borislav Petkov <bp@suse.de>
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
^ permalink raw reply
* Re: [RFC PATCH v1 09/28] x86/efi: Access EFI data as encrypted when SEV is active
From: Borislav Petkov @ 2016-09-22 14:35 UTC (permalink / raw)
To: Brijesh Singh, thomas.lendacky
Cc: simon.guinot, linux-efi, kvm, rkrcmar, matt, linus.walleij,
linux-mm, paul.gortmaker, hpa, dan.j.williams, aarcange, sfr,
andriy.shevchenko, herbert, bhe, xemul, joro, x86, mingo, msalter,
ross.zwisler, dyoung, thomas.lendacky, jroedel, keescook,
toshi.kani, mathieu.desnoyers, devel, tglx, mchehab,
iamjoonsoo.kim, labbott, tony.luck, alexandre.bounine,
kuleshovmail, linux-kernel, mcgrof, linux-crypto
In-Reply-To: <147190832511.9523.10850626471583956499.stgit@brijesh-build-machine>
On Mon, Aug 22, 2016 at 07:25:25PM -0400, Brijesh Singh wrote:
> From: Tom Lendacky <thomas.lendacky@amd.com>
>
> EFI data is encrypted when the kernel is run under SEV. Update the
> page table references to be sure the EFI memory areas are accessed
> encrypted.
>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
> arch/x86/platform/efi/efi_64.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
> index 0871ea4..98363f3 100644
> --- a/arch/x86/platform/efi/efi_64.c
> +++ b/arch/x86/platform/efi/efi_64.c
> @@ -213,7 +213,7 @@ void efi_sync_low_kernel_mappings(void)
>
> int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
> {
> - unsigned long pfn, text;
> + unsigned long pfn, text, flags;
> efi_memory_desc_t *md;
> struct page *page;
> unsigned npages;
> @@ -230,6 +230,10 @@ int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
> efi_scratch.efi_pgt = (pgd_t *)__sme_pa(efi_pgd);
> pgd = efi_pgd;
>
> + flags = _PAGE_NX | _PAGE_RW;
> + if (sev_active)
> + flags |= _PAGE_ENC;
So this is confusing me. There's this patch which says EFI data is
accessed in the clear:
https://lkml.kernel.org/r/20160822223738.29880.6909.stgit@tlendack-t1.amdoffice.net
but now here it is encrypted when SEV is enabled.
Do you mean, it is encrypted here because we're in the guest kernel?
Thanks.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC PATCH v1 09/28] x86/efi: Access EFI data as encrypted when SEV is active
From: Paolo Bonzini @ 2016-09-22 14:45 UTC (permalink / raw)
To: Borislav Petkov, Brijesh Singh, thomas.lendacky
Cc: simon.guinot, linux-efi, kvm, rkrcmar, matt, linus.walleij,
linux-mm, paul.gortmaker, hpa, dan.j.williams, aarcange, sfr,
andriy.shevchenko, herbert, bhe, xemul, joro, x86, mingo, msalter,
ross.zwisler, dyoung, jroedel, keescook, toshi.kani,
mathieu.desnoyers, devel, tglx, mchehab, iamjoonsoo.kim, labbott,
tony.luck, alexandre.bounine, kuleshovmail, linux-kernel@
In-Reply-To: <20160922143545.3kl7khff6vqk7b2t@pd.tnic>
On 22/09/2016 16:35, Borislav Petkov wrote:
>> > @@ -230,6 +230,10 @@ int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
>> > efi_scratch.efi_pgt = (pgd_t *)__sme_pa(efi_pgd);
>> > pgd = efi_pgd;
>> >
>> > + flags = _PAGE_NX | _PAGE_RW;
>> > + if (sev_active)
>> > + flags |= _PAGE_ENC;
> So this is confusing me. There's this patch which says EFI data is
> accessed in the clear:
>
> https://lkml.kernel.org/r/20160822223738.29880.6909.stgit@tlendack-t1.amdoffice.net
>
> but now here it is encrypted when SEV is enabled.
>
> Do you mean, it is encrypted here because we're in the guest kernel?
I suspect this patch is untested, and also wrong. :)
The main difference between the SME and SEV encryption, from the point
of view of the kernel, is that real-mode always writes unencrypted in
SME and always writes encrypted in SEV. But UEFI can run in 64-bit mode
and learn about the C bit, so EFI boot data should be unprotected in SEV
guests.
Because the firmware volume is written to high memory in encrypted form,
and because the PEI phase runs in 32-bit mode, the firmware code will be
encrypted; on the other hand, data that is placed in low memory for the
kernel can be unencrypted, thus limiting differences between SME and SEV.
Important: I don't know what you guys are doing for SEV and
Windows guests, but if you are doing something I would really
appreciate doing things in the open. If Linux and Windows end
up doing different things with EFI boot data, ACPI tables, etc.
it will be a huge pain. On the other hand, if we can enjoy
being first, that's great.
In fact, I have suggested in the QEMU list that SEV guests should always
use UEFI; because BIOS runs in real-mode or 32-bit non-paging protected
mode, BIOS must always write encrypted data, which becomes painful in
the kernel.
And regarding the above "important" point, all I know is that Microsoft
for sure will be happy to restrict SEV to UEFI guests. :)
There are still some differences, mostly around the real mode trampoline
executed by the kernel, but they should be much smaller.
Paolo
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC PATCH v1 09/28] x86/efi: Access EFI data as encrypted when SEV is active
From: Borislav Petkov @ 2016-09-22 14:59 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Brijesh Singh, thomas.lendacky, simon.guinot, linux-efi, kvm,
rkrcmar, matt, linus.walleij, linux-mm, paul.gortmaker, hpa,
dan.j.williams, aarcange, sfr, andriy.shevchenko, herbert, bhe,
xemul, joro, x86, mingo, msalter, ross.zwisler, dyoung, jroedel,
keescook, toshi.kani, mathieu.desnoyers, devel, tglx, mchehab,
iamjoonsoo.kim, labbott, tony.luck, alexandre.bounine,
kuleshovmail, linux-kernel
In-Reply-To: <464461b7-1efb-0af1-dd3e-eb919a2578e9@redhat.com>
On Thu, Sep 22, 2016 at 04:45:51PM +0200, Paolo Bonzini wrote:
> The main difference between the SME and SEV encryption, from the point
> of view of the kernel, is that real-mode always writes unencrypted in
> SME and always writes encrypted in SEV. But UEFI can run in 64-bit mode
> and learn about the C bit, so EFI boot data should be unprotected in SEV
> guests.
Actually, it is different: you can start fully encrypted in SME, see:
https://lkml.kernel.org/r/20160822223539.29880.96739.stgit@tlendack-t1.amdoffice.net
The last paragraph alludes to a certain transparent mode where you're
already encrypted and only certain pieces like EFI is not encrypted. I
think the aim is to have the transparent mode be the default one, which
makes most sense anyway.
The EFI regions are unencrypted for obvious reasons and you need to
access them as such.
> Because the firmware volume is written to high memory in encrypted
> form, and because the PEI phase runs in 32-bit mode, the firmware
> code will be encrypted; on the other hand, data that is placed in low
> memory for the kernel can be unencrypted, thus limiting differences
> between SME and SEV.
When you run fully encrypted, you still need to access EFI tables in the
clear. That's why I'm confused about this patch here.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC PATCH v1 04/28] x86: Secure Encrypted Virtualization (SEV) support
From: Borislav Petkov @ 2016-09-22 15:00 UTC (permalink / raw)
To: Brijesh Singh
Cc: linux-efi, kvm, rkrcmar, matt, linus.walleij, paul.gortmaker, hpa,
tglx, aarcange, sfr, mchehab, simon.guinot, bhe, xemul, joro, x86,
mingo, msalter, ross.zwisler, labbott, dyoung, thomas.lendacky,
jroedel, keescook, toshi.kani, mathieu.desnoyers, pbonzini,
dan.j.williams, andriy.shevchenko, akpm, herbert, tony.luck,
linux-mm, kuleshovmail, linux-kernel, mcgrof, linux-crypto, devel
In-Reply-To: <147190825949.9523.11406635622434950066.stgit@brijesh-build-machine>
On Mon, Aug 22, 2016 at 07:24:19PM -0400, Brijesh Singh wrote:
> From: Tom Lendacky <thomas.lendacky@amd.com>
Subject: [RFC PATCH v1 04/28] x86: Secure Encrypted Virtualization (SEV) support
Please start patch commit heading with a verb, i.e.:
"x86: Add AMD Secure Encrypted Virtualization (SEV) support"
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply
* Re: [RFC PATCH v1 09/28] x86/efi: Access EFI data as encrypted when SEV is active
From: Paolo Bonzini @ 2016-09-22 15:05 UTC (permalink / raw)
To: Borislav Petkov
Cc: linux-efi, Brijesh Singh, kvm, rkrcmar, matt, linus.walleij,
paul.gortmaker, hpa, tglx, aarcange, sfr, mchehab, simon.guinot,
bhe, xemul, joro, x86, mingo, msalter, ross.zwisler, labbott,
dyoung, thomas.lendacky, jroedel, keescook, toshi.kani,
mathieu.desnoyers, dan.j.williams, andriy.shevchenko, akpm,
herbert, tony.luck, linux-mm, kuleshovmail, linux-kernel, mcgrof,
linux-crypto, devel
In-Reply-To: <20160922145947.52v42l7p7dl7u3r4@pd.tnic>
On 22/09/2016 16:59, Borislav Petkov wrote:
> On Thu, Sep 22, 2016 at 04:45:51PM +0200, Paolo Bonzini wrote:
>> The main difference between the SME and SEV encryption, from the point
>> of view of the kernel, is that real-mode always writes unencrypted in
>> SME and always writes encrypted in SEV. But UEFI can run in 64-bit mode
>> and learn about the C bit, so EFI boot data should be unprotected in SEV
>> guests.
>
> Actually, it is different: you can start fully encrypted in SME, see:
>
> https://lkml.kernel.org/r/20160822223539.29880.96739.stgit@tlendack-t1.amdoffice.net
>
> The last paragraph alludes to a certain transparent mode where you're
> already encrypted and only certain pieces like EFI is not encrypted.
Which paragraph?
>> Because the firmware volume is written to high memory in encrypted
>> form, and because the PEI phase runs in 32-bit mode, the firmware
>> code will be encrypted; on the other hand, data that is placed in low
>> memory for the kernel can be unencrypted, thus limiting differences
>> between SME and SEV.
>
> When you run fully encrypted, you still need to access EFI tables in the
> clear. That's why I'm confused about this patch here.
I might be wrong, but I don't think this patch was tested with OVMF or Duet.
Paolo
^ permalink raw reply
* Re: BUG: rsa-pkcs1pad decrypt regression in 4.8
From: Mat Martineau @ 2016-09-22 15:55 UTC (permalink / raw)
To: Herbert Xu; +Cc: linux-crypto, smueller
In-Reply-To: <20160922090456.GA5272@gondor.apana.org.au>
Herbert -
On Thu, 22 Sep 2016, Herbert Xu wrote:
> On Wed, Sep 21, 2016 at 04:39:30PM -0700, Mat Martineau wrote:
>>
>> There was a regression in pkcs1pad signature verification, related
>> to signature verification, that you fixed in commit 27710b8ea3defcb:
>>
>> https://git.kernel.org/cgit/linux/kernel/git/herbert/crypto-2.6.git/commit/?id=27710b8ea3defcbd7d340dbd0423d911b4eb7c4f
>>
>> There is a very similar problem in the decrypt operation, which was
>> not adjusted for the leading zero changes. See
>> pkcs1pad_decrypt_complete().
>>
>> I haven't had a chance to test a fix yet, but with the final 4.8
>> release coming up very soon I wanted to report the issue.
>
> Thanks. This patch should fix the problem.
>
> ---8<---
> crypto: rsa-pkcs1pad - Handle leading zero for decryption
>
> As the software RSA implementation now produces fixed-length
> output, we need to eliminate leading zeros in the calling code
> instead.
>
> This patch does just that for pkcs1pad decryption while signature
> verification was fixed in an earlier patch.
>
> Fixes: 9b45b7bba3d2 ("crypto: rsa - Generate fixed-length output")
> Reported-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
This patch does fix the decrypt problem, my tests are now passing. Thank
you.
--
Mat Martineau
Intel OTC
^ permalink raw reply
* Re: [RFC PATCH v1 09/28] x86/efi: Access EFI data as encrypted when SEV is active
From: Borislav Petkov @ 2016-09-22 17:07 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Brijesh Singh, thomas.lendacky, simon.guinot, linux-efi, kvm,
rkrcmar, matt, linus.walleij, linux-mm, paul.gortmaker, hpa,
dan.j.williams, aarcange, sfr, andriy.shevchenko, herbert, bhe,
xemul, joro, x86, mingo, msalter, ross.zwisler, dyoung, jroedel,
keescook, toshi.kani, mathieu.desnoyers, devel, tglx, mchehab,
iamjoonsoo.kim, labbott, tony.luck, alexandre.bounine,
kuleshovmail, linux-kernel
In-Reply-To: <938ee0cf-85e6-eefa-7df9-9d5e09ed7a9d@redhat.com>
On Thu, Sep 22, 2016 at 05:05:54PM +0200, Paolo Bonzini wrote:
> Which paragraph?
"Linux relies on BIOS to set this bit if BIOS has determined that the
reduction in the physical address space as a result of enabling memory
encryption..."
Basically, you can enable SME in the BIOS and you're all set.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC PATCH v1 09/28] x86/efi: Access EFI data as encrypted when SEV is active
From: Paolo Bonzini @ 2016-09-22 17:08 UTC (permalink / raw)
To: Borislav Petkov
Cc: Brijesh Singh, thomas.lendacky, simon.guinot, linux-efi, kvm,
rkrcmar, matt, linus.walleij, linux-mm, paul.gortmaker, hpa,
dan.j.williams, aarcange, sfr, andriy.shevchenko, herbert, bhe,
xemul, joro, x86, mingo, msalter, ross.zwisler, dyoung, jroedel,
keescook, toshi.kani, mathieu.desnoyers, devel, tglx, mchehab,
iamjoonsoo.kim, labbott, tony.luck
In-Reply-To: <20160922170718.34d4ppockeurrg25@pd.tnic>
On 22/09/2016 19:07, Borislav Petkov wrote:
>> Which paragraph?
> "Linux relies on BIOS to set this bit if BIOS has determined that the
> reduction in the physical address space as a result of enabling memory
> encryption..."
>
> Basically, you can enable SME in the BIOS and you're all set.
That's not how I read it. I just figured that the BIOS has some magic
things high in the physical address space and if you reduce the physical
address space the BIOS (which is called from e.g. EFI runtime services)
would have problems with that.
Paolo
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC PATCH v1 09/28] x86/efi: Access EFI data as encrypted when SEV is active
From: Borislav Petkov @ 2016-09-22 17:27 UTC (permalink / raw)
To: Paolo Bonzini
Cc: linux-efi, Brijesh Singh, kvm, rkrcmar, matt, linus.walleij,
paul.gortmaker, hpa, tglx, aarcange, sfr, mchehab, simon.guinot,
bhe, xemul, joro, x86, mingo, msalter, ross.zwisler, labbott,
dyoung, thomas.lendacky, jroedel, keescook, toshi.kani,
mathieu.desnoyers, dan.j.williams, andriy.shevchenko, akpm,
herbert, tony.luck, linux-mm, kuleshovmail, linux-kernel, mcgrof,
linux-crypto, devel
In-Reply-To: <c1a609f0-307e-9c6c-ce33-b562ca5c0624@redhat.com>
On Thu, Sep 22, 2016 at 07:08:50PM +0200, Paolo Bonzini wrote:
> That's not how I read it. I just figured that the BIOS has some magic
> things high in the physical address space and if you reduce the physical
> address space the BIOS (which is called from e.g. EFI runtime services)
> would have problems with that.
Yeah, I had to ask about that myself and Tom will have it explained
better in the next version.
The reduction in physical address space happens when SME enabled because
you need a couple of bits in the PTE with which to say which key has
encrypted that page. So it is an indelible part of the SME machinery.
Btw, section "7.10 Secure Memory Encryption" has an initial writeup:
http://support.amd.com/TechDocs/24593.pdf
Now that I skim over it, it doesn't mention the BIOS thing but that'll
be updated.
HTH.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply
* Re: [RFC PATCH v1 09/28] x86/efi: Access EFI data as encrypted when SEV is active
From: Tom Lendacky @ 2016-09-22 17:46 UTC (permalink / raw)
To: Borislav Petkov, Brijesh Singh
Cc: simon.guinot, linux-efi, kvm, rkrcmar, matt, linus.walleij,
linux-mm, paul.gortmaker, hpa, dan.j.williams, aarcange, sfr,
andriy.shevchenko, herbert, bhe, xemul, joro, x86, mingo, msalter,
ross.zwisler, dyoung, jroedel, keescook, toshi.kani,
mathieu.desnoyers, devel, tglx, mchehab, iamjoonsoo.kim, labbott,
tony.luck, alexandre.bounine, kuleshovmail, linux-kernel
In-Reply-To: <20160922143545.3kl7khff6vqk7b2t@pd.tnic>
On 09/22/2016 09:35 AM, Borislav Petkov wrote:
> On Mon, Aug 22, 2016 at 07:25:25PM -0400, Brijesh Singh wrote:
>> From: Tom Lendacky <thomas.lendacky@amd.com>
>>
>> EFI data is encrypted when the kernel is run under SEV. Update the
>> page table references to be sure the EFI memory areas are accessed
>> encrypted.
>>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>> ---
>> arch/x86/platform/efi/efi_64.c | 14 ++++++++++++--
>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
>> index 0871ea4..98363f3 100644
>> --- a/arch/x86/platform/efi/efi_64.c
>> +++ b/arch/x86/platform/efi/efi_64.c
>> @@ -213,7 +213,7 @@ void efi_sync_low_kernel_mappings(void)
>>
>> int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
>> {
>> - unsigned long pfn, text;
>> + unsigned long pfn, text, flags;
>> efi_memory_desc_t *md;
>> struct page *page;
>> unsigned npages;
>> @@ -230,6 +230,10 @@ int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
>> efi_scratch.efi_pgt = (pgd_t *)__sme_pa(efi_pgd);
>> pgd = efi_pgd;
>>
>> + flags = _PAGE_NX | _PAGE_RW;
>> + if (sev_active)
>> + flags |= _PAGE_ENC;
>
> So this is confusing me. There's this patch which says EFI data is
> accessed in the clear:
>
> https://lkml.kernel.org/r/20160822223738.29880.6909.stgit@tlendack-t1.amdoffice.net
>
> but now here it is encrypted when SEV is enabled.
>
> Do you mean, it is encrypted here because we're in the guest kernel?
Yes, the idea is that the SEV guest will be running encrypted from the
start, including the BIOS/UEFI, and so all of the EFI related data will
be encrypted.
Thanks,
Tom
>
> Thanks.
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC PATCH v1 09/28] x86/efi: Access EFI data as encrypted when SEV is active
From: Paolo Bonzini @ 2016-09-22 18:23 UTC (permalink / raw)
To: Tom Lendacky, Borislav Petkov, Brijesh Singh
Cc: linux-efi, kvm, rkrcmar, matt, linus.walleij, paul.gortmaker, hpa,
tglx, aarcange, sfr, mchehab, simon.guinot, bhe, xemul, joro, x86,
mingo, msalter, ross.zwisler, labbott, dyoung, jroedel, keescook,
toshi.kani, mathieu.desnoyers, dan.j.williams, andriy.shevchenko,
akpm, herbert, tony.luck, linux-mm, kuleshovmail, linux-kernel,
mcgrof, linux-crypto, devel, iamjoonsoo.kim, alexandre.bounine
In-Reply-To: <443d06f5-2db5-5107-296f-94fabd209407@amd.com>
On 22/09/2016 19:46, Tom Lendacky wrote:
>> > Do you mean, it is encrypted here because we're in the guest kernel?
> Yes, the idea is that the SEV guest will be running encrypted from the
> start, including the BIOS/UEFI, and so all of the EFI related data will
> be encrypted.
Unless this is part of some spec, it's easier if things are the same in
SME and SEV.
Paolo
^ permalink raw reply
* Re: [RFC PATCH v1 09/28] x86/efi: Access EFI data as encrypted when SEV is active
From: Borislav Petkov @ 2016-09-22 18:37 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Tom Lendacky, Brijesh Singh, simon.guinot, linux-efi, kvm,
rkrcmar, matt, linus.walleij, linux-mm, paul.gortmaker, hpa,
dan.j.williams, aarcange, sfr, andriy.shevchenko, herbert, bhe,
xemul, joro, x86, mingo, msalter, ross.zwisler, dyoung, jroedel,
keescook, toshi.kani, mathieu.desnoyers, devel, tglx, mchehab,
iamjoonsoo.kim, labbott, tony.luck, alexandre.bounine,
kuleshovmail, linux-kernel
In-Reply-To: <45a56110-95e9-e1f3-83ab-e777b48bf79a@redhat.com>
On Thu, Sep 22, 2016 at 08:23:36PM +0200, Paolo Bonzini wrote:
> Unless this is part of some spec, it's easier if things are the same in
> SME and SEV.
Yeah, I was pondering over how sprinkling sev_active checks might not be
so clean.
I'm wondering if we could make the EFI regions presented to the guest
unencrypted too, as part of some SEV-specific init routine so that the
guest kernel doesn't need to do anything different.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC PATCH v1 09/28] x86/efi: Access EFI data as encrypted when SEV is active
From: Tom Lendacky @ 2016-09-22 18:47 UTC (permalink / raw)
To: Paolo Bonzini, Borislav Petkov, Brijesh Singh
Cc: simon.guinot, linux-efi, kvm, rkrcmar, matt, linus.walleij,
linux-mm, paul.gortmaker, hpa, dan.j.williams, aarcange, sfr,
andriy.shevchenko, herbert, bhe, xemul, joro, x86, mingo, msalter,
ross.zwisler, dyoung, jroedel, keescook, toshi.kani,
mathieu.desnoyers, devel, tglx, mchehab, iamjoonsoo.kim, labbott,
<tony.l
In-Reply-To: <464461b7-1efb-0af1-dd3e-eb919a2578e9@redhat.com>
On 09/22/2016 09:45 AM, Paolo Bonzini wrote:
>
>
> On 22/09/2016 16:35, Borislav Petkov wrote:
>>>> @@ -230,6 +230,10 @@ int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
>>>> efi_scratch.efi_pgt = (pgd_t *)__sme_pa(efi_pgd);
>>>> pgd = efi_pgd;
>>>>
>>>> + flags = _PAGE_NX | _PAGE_RW;
>>>> + if (sev_active)
>>>> + flags |= _PAGE_ENC;
>> So this is confusing me. There's this patch which says EFI data is
>> accessed in the clear:
>>
>> https://lkml.kernel.org/r/20160822223738.29880.6909.stgit@tlendack-t1.amdoffice.net
>>
>> but now here it is encrypted when SEV is enabled.
>>
>> Do you mean, it is encrypted here because we're in the guest kernel?
>
> I suspect this patch is untested, and also wrong. :)
Yes, it is untested but not sure that it is wrong... It all depends on
how we add SEV support to the guest UEFI BIOS. My take would be to have
the EFI data and ACPI tables encrypted.
>
> The main difference between the SME and SEV encryption, from the point
> of view of the kernel, is that real-mode always writes unencrypted in
> SME and always writes encrypted in SEV. But UEFI can run in 64-bit mode
> and learn about the C bit, so EFI boot data should be unprotected in SEV
> guests.
>
> Because the firmware volume is written to high memory in encrypted form,
> and because the PEI phase runs in 32-bit mode, the firmware code will be
> encrypted; on the other hand, data that is placed in low memory for the
> kernel can be unencrypted, thus limiting differences between SME and SEV.
I like the idea of limiting the differences but it would leave the EFI
data and ACPI tables exposed and able to be manipulated.
>
> Important: I don't know what you guys are doing for SEV and
> Windows guests, but if you are doing something I would really
> appreciate doing things in the open. If Linux and Windows end
> up doing different things with EFI boot data, ACPI tables, etc.
> it will be a huge pain. On the other hand, if we can enjoy
> being first, that's great.
We haven't discussed Windows guests under SEV yet, but as you say, we
need to do things the same.
Thanks,
Tom
>
> In fact, I have suggested in the QEMU list that SEV guests should always
> use UEFI; because BIOS runs in real-mode or 32-bit non-paging protected
> mode, BIOS must always write encrypted data, which becomes painful in
> the kernel.
>
> And regarding the above "important" point, all I know is that Microsoft
> for sure will be happy to restrict SEV to UEFI guests. :)
>
> There are still some differences, mostly around the real mode trampoline
> executed by the kernel, but they should be much smaller.
>
> Paolo
>
^ permalink raw reply
* Re: [RFC PATCH v1 09/28] x86/efi: Access EFI data as encrypted when SEV is active
From: Paolo Bonzini @ 2016-09-22 18:50 UTC (permalink / raw)
To: Tom Lendacky, Borislav Petkov, Brijesh Singh
Cc: simon.guinot, linux-efi, kvm, rkrcmar, matt, linus.walleij,
linux-mm, paul.gortmaker, hpa, dan.j.williams, aarcange, sfr,
andriy.shevchenko, herbert, bhe, xemul, joro, x86, mingo, msalter,
ross.zwisler, dyoung, jroedel, keescook, toshi.kani,
mathieu.desnoyers, devel, tglx, mchehab, iamjoonsoo.kim, labbott,
tony.luck, alexandre.bounine, kuleshovmail, linux-kernel@
In-Reply-To: <0012cd4d-d432-ae14-1041-5e8b62973b37@amd.com>
On 22/09/2016 20:47, Tom Lendacky wrote:
> > Because the firmware volume is written to high memory in encrypted form,
> > and because the PEI phase runs in 32-bit mode, the firmware code will be
> > encrypted; on the other hand, data that is placed in low memory for the
> > kernel can be unencrypted, thus limiting differences between SME and SEV.
>
> I like the idea of limiting the differences but it would leave the EFI
> data and ACPI tables exposed and able to be manipulated.
Hmm, that makes sense. So I guess this has to stay, and Borislav's
proposal doesn't fly either.
Paolo
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC PATCH v1 09/28] x86/efi: Access EFI data as encrypted when SEV is active
From: Tom Lendacky @ 2016-09-22 18:59 UTC (permalink / raw)
To: Borislav Petkov, Paolo Bonzini
Cc: Brijesh Singh, simon.guinot, linux-efi, kvm, rkrcmar, matt,
linus.walleij, linux-mm, paul.gortmaker, hpa, dan.j.williams,
aarcange, sfr, andriy.shevchenko, herbert, bhe, xemul, joro, x86,
mingo, msalter, ross.zwisler, dyoung, jroedel, keescook,
toshi.kani, mathieu.desnoyers, devel, tglx, mchehab,
iamjoonsoo.kim
In-Reply-To: <20160922145947.52v42l7p7dl7u3r4@pd.tnic>
On 09/22/2016 09:59 AM, Borislav Petkov wrote:
> On Thu, Sep 22, 2016 at 04:45:51PM +0200, Paolo Bonzini wrote:
>> The main difference between the SME and SEV encryption, from the point
>> of view of the kernel, is that real-mode always writes unencrypted in
>> SME and always writes encrypted in SEV. But UEFI can run in 64-bit mode
>> and learn about the C bit, so EFI boot data should be unprotected in SEV
>> guests.
>
> Actually, it is different: you can start fully encrypted in SME, see:
>
> https://lkml.kernel.org/r/20160822223539.29880.96739.stgit@tlendack-t1.amdoffice.net
>
> The last paragraph alludes to a certain transparent mode where you're
> already encrypted and only certain pieces like EFI is not encrypted. I
> think the aim is to have the transparent mode be the default one, which
> makes most sense anyway.
There is a new Transparent SME mode that is now part of the overall
SME support, but I'm not alluding to that in the documentation at all.
In TSME mode, everything that goes through the memory controller would
be encrypted and that would include EFI data, etc. TSME would be
enabled through a BIOS option, thus allowing legacy OSes to benefit.
>
> The EFI regions are unencrypted for obvious reasons and you need to
> access them as such.
>
>> Because the firmware volume is written to high memory in encrypted
>> form, and because the PEI phase runs in 32-bit mode, the firmware
>> code will be encrypted; on the other hand, data that is placed in low
>> memory for the kernel can be unencrypted, thus limiting differences
>> between SME and SEV.
>
> When you run fully encrypted, you still need to access EFI tables in the
> clear. That's why I'm confused about this patch here.
This patch assumes that the EFI regions of a guest would be encrypted.
Thanks,
Tom
>
^ 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