On 7/3/21 11:19 AM, Borislav Petkov wrote:

On Thu, Jul 01, 2021 at 04:32:25PM -0500, Brijesh Singh wrote:

quoted

The spec definition is present in include/linux/psp-sev.h but sometime
we don't expose the spec defs as-is to userspace.

Why?

Having such undocumented and maybe unwarranted differences - I still
don't see a clear reason why - is calling for additional and unnecessary
confusion.

Because some of fields don't make any sense for the userspace interfaces. 

quoted

Several SEV/SEV-SNP does not need to be exposed to the userspace,
those which need to be expose we provide a bit modified Linux uapi for
it, and for SEV drivers we choose "_user" prefix.

Is that documented somewhere?

Because "user" doesn't tell me it is a modified structure which is
different from the spec.

We have good documentation for the SEV ioctl and structure provided
through the KVM interface.

Unfortunately the the documentation for the ioctl and structure provided
through /dev/sev does not exist. We could look into adding this
documentation outside this series. The structure provided through
/dev/sev is identical to the structure documented in the spec with minor
changes such as not exposing reserved fields or rename from paddr to
uaddr etc.

quoted

e.g
a spec definition for the PEK import in include/linux/psp-sev.h is:
struct sev_data_pek_cert_import {
	u64 pdh_cert_address;  /* system physical address */
	u32 pdh_cert_len;
	u32 reserved;
	...
};

But its corresponding userspace structure def in include/uapi/linux/psp-sev.h is:
struct sev_user_data_pek_cert_import {
	__u64 pek_cert_uaddr; /* userspace address */
	__u32 pek_cert_len;
	...
};

And the difference is a single "u32 reserved"?

Mostly yes.

Dunno, from where I'm standing this looks like unnecessary confusion to
me.

quoted

The ioctl handling takes care of mapping from uaddr to pa and other
things as required. So, I took similar approach for the SEV-SNP guest
ioctl. In this particular case the guest request structure defined in
the spec contains multiple field but many of those fields are managed
internally by the kernel (e.g seqno, IV, etc etc).

Ok, multiple fields sounds like you wanna save on the data that is
shovelled between kernel and user space and then some of the fields
don't mean a thing for the user API. Ok.

But again, where is this documented and stated clear so that people are
aware?

Or are you assuming that since the user counterparts are in

include/uapi/linux/psp-sev.h
	^^^^

and it being an uapi header, then that should state that?

Yes, the assumption is user wanting to communicate to PSP through
/dev/sev will need to include include psp-sev.h from
uapi/linux/psp-sev.h. The header file itself document the fields
definition, and then user need to refer to SEV SPEC for the further
details. I could start documenting the SNP specific ioctl in
Documentation/virt/coco/sevguest.rst and it can be later expanded to
cover SEV and SEV-ES.

-Brijesh