On 11/18/21 10:43 AM, Peter Gonda wrote:
...

quoted

+       u8 buf[89];

Could we document this magic number?

Yes, I will document from where this number came.

quoted

+
+       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