Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

x86/sev: Change snp_guest_issue_request()'s fw_err argument

The GHCB specification declares that the firmware error value for
a guest request will be stored in the lower 32 bits of EXIT_INFO_2. The
upper 32 bits are for the VMM's own error code. The fw_err argument to
snp_guest_issue_request() is thus a misnomer, and callers will need
access to all 64 bits.

The type of unsigned long also causes problems, since sw_exit_info2 is
u64 (unsigned long long) vs the argument's unsigned long*. Change this
type for issuing the guest request. Pass the ioctl command struct's error
field directly instead of in a local variable, since an incomplete guest
request may not set the error code, and uninitialized stack memory would
be written back to user space.

The firmware might not even be called, so bookend the call with the no
firmware call error and clear the error.

Since the "fw_err" field is really exitinfo2 split into the upper bits'
vmm error code and lower bits' firmware error code, convert the 64 bit
value to a union.

[ bp:
- Massage commit message
- adjust code
- Fix a build issue as
Reported-by: kernel test robot <lkp@intel.com>
Link: https://lore.kernel.org/oe-kbuild-all/202303070609.vX6wp2Af-lkp@intel.com
- print exitinfo2 in hex
Tom:
- Correct -EIO exit case. ]

Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Link: https://lore.kernel.org/r/20230214164638.1189804-5-dionnaglaze@google.com
Link: https://lore.kernel.org/r/20230307192449.24732-12-bp@alien8.de

authored by

Dionna Glaze and committed by
Borislav Petkov (AMD)
0144e3b8 96500610

+83 -56
+13 -7
Documentation/virt/coco/sev-guest.rst
··· 37 37 the return value. General error numbers (-ENOMEM, -EINVAL) 38 38 are not detailed, but errors with specific meanings are. 39 39 40 - The guest ioctl should be issued on a file descriptor of the /dev/sev-guest device. 41 - The ioctl accepts struct snp_user_guest_request. The input and output structure is 42 - specified through the req_data and resp_data field respectively. If the ioctl fails 43 - to execute due to a firmware error, then fw_err code will be set. Otherwise, fw_err 44 - will be set to 0x00000000ffffffff, i.e., the lower 32-bits are -1. 40 + The guest ioctl should be issued on a file descriptor of the /dev/sev-guest 41 + device. The ioctl accepts struct snp_user_guest_request. The input and 42 + output structure is specified through the req_data and resp_data field 43 + respectively. If the ioctl fails to execute due to a firmware error, then 44 + the fw_error code will be set, otherwise fw_error will be set to -1. 45 45 46 46 The firmware checks that the message sequence counter is one greater than 47 47 the guests message sequence counter. If guest driver fails to increment message ··· 57 57 __u64 req_data; 58 58 __u64 resp_data; 59 59 60 - /* firmware error code on failure (see psp-sev.h) */ 61 - __u64 fw_err; 60 + /* bits[63:32]: VMM error code, bits[31:0] firmware error code (see psp-sev.h) */ 61 + union { 62 + __u64 exitinfo2; 63 + struct { 64 + __u32 fw_error; 65 + __u32 vmm_error; 66 + }; 67 + }; 62 68 }; 63 69 64 70 2.1 SNP_GET_REPORT
-4
arch/x86/include/asm/sev-common.h
··· 128 128 struct psc_entry entries[VMGEXIT_PSC_MAX_ENTRY]; 129 129 } __packed; 130 130 131 - /* Guest message request error codes */ 132 - #define SNP_GUEST_REQ_INVALID_LEN BIT_ULL(32) 133 - #define SNP_GUEST_REQ_ERR_BUSY BIT_ULL(33) 134 - 135 131 #define GHCB_MSR_TERM_REQ 0x100 136 132 #define GHCB_MSR_TERM_REASON_SET_POS 12 137 133 #define GHCB_MSR_TERM_REASON_SET_MASK 0xf
+7 -3
arch/x86/include/asm/sev.h
··· 9 9 #define __ASM_ENCRYPTED_STATE_H 10 10 11 11 #include <linux/types.h> 12 + #include <linux/sev-guest.h> 13 + 12 14 #include <asm/insn.h> 13 15 #include <asm/sev-common.h> 14 16 #include <asm/bootparam.h> ··· 187 185 188 186 return rc; 189 187 } 188 + 189 + struct snp_guest_request_ioctl; 190 + 190 191 void setup_ghcb(void); 191 192 void __init early_snp_set_memory_private(unsigned long vaddr, unsigned long paddr, 192 193 unsigned int npages); ··· 201 196 void snp_set_wakeup_secondary_cpu(void); 202 197 bool snp_init(struct boot_params *bp); 203 198 void __init __noreturn snp_abort(void); 204 - int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, unsigned long *fw_err); 199 + int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct snp_guest_request_ioctl *rio); 205 200 #else 206 201 static inline void sev_es_ist_enter(struct pt_regs *regs) { } 207 202 static inline void sev_es_ist_exit(void) { } ··· 221 216 static inline void snp_set_wakeup_secondary_cpu(void) { } 222 217 static inline bool snp_init(struct boot_params *bp) { return false; } 223 218 static inline void snp_abort(void) { } 224 - static inline int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, 225 - unsigned long *fw_err) 219 + static inline int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct snp_guest_request_ioctl *rio) 226 220 { 227 221 return -ENOTTY; 228 222 }
+8 -7
arch/x86/kernel/sev.c
··· 22 22 #include <linux/efi.h> 23 23 #include <linux/platform_device.h> 24 24 #include <linux/io.h> 25 + #include <linux/psp-sev.h> 26 + #include <uapi/linux/sev-guest.h> 25 27 26 28 #include <asm/cpu_entry_area.h> 27 29 #include <asm/stacktrace.h> ··· 2177 2175 } 2178 2176 __setup("sev=", init_sev_config); 2179 2177 2180 - int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, unsigned long *fw_err) 2178 + int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct snp_guest_request_ioctl *rio) 2181 2179 { 2182 2180 struct ghcb_state state; 2183 2181 struct es_em_ctxt ctxt; ··· 2185 2183 struct ghcb *ghcb; 2186 2184 int ret; 2187 2185 2188 - if (!fw_err) 2189 - return -EINVAL; 2186 + rio->exitinfo2 = SEV_RET_NO_FW_CALL; 2190 2187 2191 2188 /* 2192 2189 * __sev_get_ghcb() needs to run with IRQs disabled because it is using ··· 2210 2209 if (ret) 2211 2210 goto e_put; 2212 2211 2213 - *fw_err = ghcb->save.sw_exit_info_2; 2214 - switch (*fw_err) { 2212 + rio->exitinfo2 = ghcb->save.sw_exit_info_2; 2213 + switch (rio->exitinfo2) { 2215 2214 case 0: 2216 2215 break; 2217 2216 2218 - case SNP_GUEST_REQ_ERR_BUSY: 2217 + case SNP_GUEST_VMM_ERR(SNP_GUEST_VMM_ERR_BUSY): 2219 2218 ret = -EAGAIN; 2220 2219 break; 2221 2220 2222 - case SNP_GUEST_REQ_INVALID_LEN: 2221 + case SNP_GUEST_VMM_ERR(SNP_GUEST_VMM_ERR_INVALID_LEN): 2223 2222 /* Number of expected pages are returned in RBX */ 2224 2223 if (exit_code == SVM_VMGEXIT_EXT_GUEST_REQUEST) { 2225 2224 input->data_npages = ghcb_get_rbx(ghcb);
+39 -33
drivers/virt/coco/sev-guest/sev-guest.c
··· 332 332 return __enc_payload(snp_dev, req, payload, sz); 333 333 } 334 334 335 - static int __handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, __u64 *fw_err) 335 + static int __handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, 336 + struct snp_guest_request_ioctl *rio) 336 337 { 337 - unsigned long err = 0xff, override_err = 0; 338 338 unsigned long req_start = jiffies; 339 339 unsigned int override_npages = 0; 340 + u64 override_err = 0; 340 341 int rc; 341 342 342 343 retry_request: ··· 347 346 * sequence number must be incremented or the VMPCK must be deleted to 348 347 * prevent reuse of the IV. 349 348 */ 350 - rc = snp_issue_guest_request(exit_code, &snp_dev->input, &err); 349 + rc = snp_issue_guest_request(exit_code, &snp_dev->input, rio); 351 350 switch (rc) { 352 351 case -ENOSPC: 353 352 /* ··· 365 364 * request buffer size was too small and give the caller the 366 365 * required buffer size. 367 366 */ 368 - override_err = SNP_GUEST_REQ_INVALID_LEN; 367 + override_err = SNP_GUEST_VMM_ERR(SNP_GUEST_VMM_ERR_INVALID_LEN); 369 368 370 369 /* 371 370 * If this call to the firmware succeeds, the sequence number can ··· 378 377 goto retry_request; 379 378 380 379 /* 381 - * The host may return SNP_GUEST_REQ_ERR_EBUSY if the request has been 380 + * The host may return SNP_GUEST_VMM_ERR_BUSY if the request has been 382 381 * throttled. Retry in the driver to avoid returning and reusing the 383 382 * message sequence number on a different message. 384 383 */ ··· 399 398 */ 400 399 snp_inc_msg_seqno(snp_dev); 401 400 402 - if (fw_err) 403 - *fw_err = override_err ?: err; 401 + if (override_err) { 402 + rio->exitinfo2 = override_err; 403 + 404 + /* 405 + * If an extended guest request was issued and the supplied certificate 406 + * buffer was not large enough, a standard guest request was issued to 407 + * prevent IV reuse. If the standard request was successful, return -EIO 408 + * back to the caller as would have originally been returned. 409 + */ 410 + if (!rc && override_err == SNP_GUEST_VMM_ERR(SNP_GUEST_VMM_ERR_INVALID_LEN)) 411 + rc = -EIO; 412 + } 404 413 405 414 if (override_npages) 406 415 snp_dev->input.data_npages = override_npages; 407 416 408 - /* 409 - * If an extended guest request was issued and the supplied certificate 410 - * buffer was not large enough, a standard guest request was issued to 411 - * prevent IV reuse. If the standard request was successful, return -EIO 412 - * back to the caller as would have originally been returned. 413 - */ 414 - if (!rc && override_err == SNP_GUEST_REQ_INVALID_LEN) 415 - return -EIO; 416 - 417 417 return rc; 418 418 } 419 419 420 - static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, int msg_ver, 421 - u8 type, void *req_buf, size_t req_sz, void *resp_buf, 422 - u32 resp_sz, __u64 *fw_err) 420 + static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, 421 + struct snp_guest_request_ioctl *rio, u8 type, 422 + void *req_buf, size_t req_sz, void *resp_buf, 423 + u32 resp_sz) 423 424 { 424 425 u64 seqno; 425 426 int rc; ··· 435 432 memset(snp_dev->response, 0, sizeof(struct snp_guest_msg)); 436 433 437 434 /* Encrypt the userspace provided payload in snp_dev->secret_request. */ 438 - rc = enc_payload(snp_dev, seqno, msg_ver, type, req_buf, req_sz); 435 + rc = enc_payload(snp_dev, seqno, rio->msg_version, type, req_buf, req_sz); 439 436 if (rc) 440 437 return rc; 441 438 ··· 446 443 memcpy(snp_dev->request, &snp_dev->secret_request, 447 444 sizeof(snp_dev->secret_request)); 448 445 449 - rc = __handle_guest_request(snp_dev, exit_code, fw_err); 446 + rc = __handle_guest_request(snp_dev, exit_code, rio); 450 447 if (rc) { 451 - if (rc == -EIO && *fw_err == SNP_GUEST_REQ_INVALID_LEN) 448 + if (rc == -EIO && 449 + rio->exitinfo2 == SNP_GUEST_VMM_ERR(SNP_GUEST_VMM_ERR_INVALID_LEN)) 452 450 return rc; 453 451 454 - dev_alert(snp_dev->dev, "Detected error from ASP request. rc: %d, fw_err: %llu\n", rc, *fw_err); 452 + dev_alert(snp_dev->dev, 453 + "Detected error from ASP request. rc: %d, exitinfo2: 0x%llx\n", 454 + rc, rio->exitinfo2); 455 + 455 456 snp_disable_vmpck(snp_dev); 456 457 return rc; 457 458 } ··· 495 488 if (!resp) 496 489 return -ENOMEM; 497 490 498 - rc = handle_guest_request(snp_dev, SVM_VMGEXIT_GUEST_REQUEST, arg->msg_version, 491 + rc = handle_guest_request(snp_dev, SVM_VMGEXIT_GUEST_REQUEST, arg, 499 492 SNP_MSG_REPORT_REQ, &req, sizeof(req), resp->data, 500 - resp_len, &arg->fw_err); 493 + resp_len); 501 494 if (rc) 502 495 goto e_free; 503 496 ··· 535 528 if (copy_from_user(&req, (void __user *)arg->req_data, sizeof(req))) 536 529 return -EFAULT; 537 530 538 - rc = handle_guest_request(snp_dev, SVM_VMGEXIT_GUEST_REQUEST, arg->msg_version, 539 - SNP_MSG_KEY_REQ, &req, sizeof(req), buf, resp_len, 540 - &arg->fw_err); 531 + rc = handle_guest_request(snp_dev, SVM_VMGEXIT_GUEST_REQUEST, arg, 532 + SNP_MSG_KEY_REQ, &req, sizeof(req), buf, resp_len); 541 533 if (rc) 542 534 return rc; 543 535 ··· 596 590 return -ENOMEM; 597 591 598 592 snp_dev->input.data_npages = npages; 599 - ret = handle_guest_request(snp_dev, SVM_VMGEXIT_EXT_GUEST_REQUEST, arg->msg_version, 593 + ret = handle_guest_request(snp_dev, SVM_VMGEXIT_EXT_GUEST_REQUEST, arg, 600 594 SNP_MSG_REPORT_REQ, &req.data, 601 - sizeof(req.data), resp->data, resp_len, &arg->fw_err); 595 + sizeof(req.data), resp->data, resp_len); 602 596 603 597 /* If certs length is invalid then copy the returned length */ 604 - if (arg->fw_err == SNP_GUEST_REQ_INVALID_LEN) { 598 + if (arg->vmm_error == SNP_GUEST_VMM_ERR_INVALID_LEN) { 605 599 req.certs_len = snp_dev->input.data_npages << PAGE_SHIFT; 606 600 607 601 if (copy_to_user((void __user *)arg->req_data, &req, sizeof(req))) ··· 636 630 if (copy_from_user(&input, argp, sizeof(input))) 637 631 return -EFAULT; 638 632 639 - input.fw_err = 0xff; 633 + input.exitinfo2 = 0xff; 640 634 641 635 /* Message version must be non-zero */ 642 636 if (!input.msg_version) ··· 667 661 668 662 mutex_unlock(&snp_cmd_mutex); 669 663 670 - if (input.fw_err && copy_to_user(argp, &input, sizeof(input))) 664 + if (input.exitinfo2 && copy_to_user(argp, &input, sizeof(input))) 671 665 return -EFAULT; 672 666 673 667 return ret;
+16 -2
include/uapi/linux/sev-guest.h
··· 52 52 __u64 req_data; 53 53 __u64 resp_data; 54 54 55 - /* firmware error code on failure (see psp-sev.h) */ 56 - __u64 fw_err; 55 + /* bits[63:32]: VMM error code, bits[31:0] firmware error code (see psp-sev.h) */ 56 + union { 57 + __u64 exitinfo2; 58 + struct { 59 + __u32 fw_error; 60 + __u32 vmm_error; 61 + }; 62 + }; 57 63 }; 58 64 59 65 struct snp_ext_report_req { ··· 82 76 83 77 /* Get SNP extended report as defined in the GHCB specification version 2. */ 84 78 #define SNP_GET_EXT_REPORT _IOWR(SNP_GUEST_REQ_IOC_TYPE, 0x2, struct snp_guest_request_ioctl) 79 + 80 + /* Guest message request EXIT_INFO_2 constants */ 81 + #define SNP_GUEST_FW_ERR_MASK GENMASK_ULL(31, 0) 82 + #define SNP_GUEST_VMM_ERR_SHIFT 32 83 + #define SNP_GUEST_VMM_ERR(x) (((u64)x) << SNP_GUEST_VMM_ERR_SHIFT) 84 + 85 + #define SNP_GUEST_VMM_ERR_INVALID_LEN 1 86 + #define SNP_GUEST_VMM_ERR_BUSY 2 85 87 86 88 #endif /* __UAPI_LINUX_SEV_GUEST_H_ */