Messages in this thread Patches in this message | | | Date | Mon, 23 Jan 2023 21:24:23 +0100 | From | Borislav Petkov <> | Subject | Re: [PATCH v12 2/3] x86/sev: Change snp_guest_issue_request's fw_err |
| |
On Fri, Jan 20, 2023 at 09:48:55PM +0000, Dionna Glaze wrote: > Since the "fw_err" field is really exitinfo2 split into the upper bits' > vmm error code and lower bits' firmware error code, sev-guest.h is > updated to represent the 64 bit value as a union.
Yah, documentation needs update too:
diff --git a/Documentation/virt/coco/sev-guest.rst b/Documentation/virt/coco/sev-guest.rst index 4f0dcc1d16e8..4f9df24b829f 100644 --- a/Documentation/virt/coco/sev-guest.rst +++ b/Documentation/virt/coco/sev-guest.rst @@ -57,9 +57,16 @@ counter (e.g. counter overflow), then -EIO will be returned. __u64 req_data; __u64 resp_data; - /* firmware error code on failure (see psp-sev.h) */ - __u64 fw_err; - }; + /* bits[63:32]: VMM error code, bits[31:0] firmware error code (see psp-sev.h) */ + union { + __u64 exitinfo2; + struct { + __u32 fw_error; + __u32 vmm_error; + }; + }; + }; + 2.1 SNP_GET_REPORT ------------------ /me goes and looks at patch 3...
Yap, that change adding the union should all belong together in a single patch.
> @@ -366,24 +367,22 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in > * of the VMPCK and the error code being propagated back to the > * user as an ioctl() return code. > */ > - rc = snp_issue_guest_request(exit_code, &snp_dev->input, &err); > + rc = snp_issue_guest_request(exit_code, &snp_dev->input, > + &arg->exitinfo2); > > /* > * Override the error to inform callers the given extended > * request buffer size was too small and give the caller the > * required buffer size. > */ > - err = SNP_GUEST_REQ_INVALID_LEN; > + arg->vmm_error = SNP_GUEST_VMM_ERR_INVALID_LEN; > snp_dev->input.data_npages = certs_npages; > } > > - if (fw_err) > - *fw_err = err; > - > if (rc) { > dev_alert(snp_dev->dev, > - "Detected error from ASP request. rc: %d, fw_err: %llu\n", > - rc, *fw_err); > + "Detected error from ASP request. rc: %d, exitinfo2: %llu\n", > + rc, arg->exitinfo2);
I guess that's better off dumped in binary now:
diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c index d4973d2dbc24..a5d6ea3eebe9 100644 --- a/drivers/virt/coco/sev-guest/sev-guest.c +++ b/drivers/virt/coco/sev-guest/sev-guest.c @@ -380,7 +380,7 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, if (rc) { dev_alert(snp_dev->dev, - "Detected error from ASP request. rc: %d, exitinfo2: %llu\n", + "Detected error from ASP request. rc: %d, exitinfo2: 0x%llx\n", rc, arg->exitinfo2); goto disable_vmpck; } Anyway, that's a lot of changes for a fix which needs to go to stable. I don't mind them but not in a minimal fix.
So how is this for a minimal fix to go in now, ontop of your first patch? The cleanups can then go later...
--- diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c index 4ec4174e05a3..20b560a45bc1 100644 --- a/drivers/virt/coco/sev-guest/sev-guest.c +++ b/drivers/virt/coco/sev-guest/sev-guest.c @@ -322,7 +322,7 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in u8 type, void *req_buf, size_t req_sz, void *resp_buf, u32 resp_sz, __u64 *fw_err) { - unsigned long err; + unsigned long err = SEV_RET_NO_FW_CALL; u64 seqno; int rc; -- Regards/Gruss, Boris.
https://people.kernel.org/tglx/notes-about-netiquette
| |