On 11/18/21 10:43 AM, Peter Gonda wrote:
...
+ u8 buf[89];
Could we document this magic number?
Yes, I will document from where this number came.
+
+ if (!arg->req_data || !arg->resp_data)
+ return -EINVAL;
+
+ /* Copy the request payload from userspace */
+ if (copy_from_user(&req, (void __user *)arg->req_data, sizeof(req)))
+ return -EFAULT;
+
+ /* Message version must be non-zero */
+ if (!req.msg_version)
+ return -EINVAL;
+
+ /*
+ * The intermediate response buffer is used while decrypting the
+ * response payload. Make sure that it has enough space to cover the
+ * authtag.
+ */
+ resp_len = sizeof(resp.data) + crypto->a_len;
+ if (sizeof(buf) < resp_len)
+ return -ENOMEM;
+
+ /* Issue the command to get the attestation report */
+ rc = handle_guest_request(snp_dev, SVM_VMGEXIT_GUEST_REQUEST, req.msg_version,
+ SNP_MSG_KEY_REQ, &req.data, sizeof(req.data), buf, resp_len,
+ &arg->fw_err);
+ if (rc)
+ goto e_free;
Should we check the first 32 bits of |data| here since that is a
status field? If we see 16h here we could return -EINVAL, or better to
let userspace deal with that error handling?
I was trying to avoid looking into a response structure to keep the
flexibility; e.g if SNP firmware changes a response format then we don't
need to have any changes in the driver. The userspace should be able to
deal with it and it can check the "status" or a new field etc.
thanks
