On Mon, Aug 29, 2022, Kyle Huey wrote:
> @@ -1246,6 +1246,21 @@ static int copy_uabi_to_xstate(struct fpstate *fpstate, const void *kbuf,
> }
> }
>
> + /*
> + * Update the user protection key storage. Allow KVM to
> + * pass in a NULL pkru pointer if the mask bit is unset
> + * for its legacy ABI behavior.
> + */
> + if (pkru)
> + *pkru = 0;
> +
> + if (hdr.xfeatures & XFEATURE_MASK_PKRU) {
> + struct pkru_state *xpkru;
> +
> + xpkru = __raw_xsave_addr(xsave, XFEATURE_PKRU);
> + *pkru = xpkru->pkru;
> + }
What about writing this as:
if (hdr.xfeatures & XFEATURE_MASK_PKRU) {
...
*pkru = xpkru->pkru;
} else if (pkru) {
*pkru = 0;
}
to make it slightly more obvious that @pkru must be non-NULL if the feature flag
is enabled?
Or we could be paranoid, though I'm not sure this is worthwhile.
if ((hdr.xfeatures & XFEATURE_MASK_PKRU) &&
!WARN_ON_ONCE(!pkru)) {
...
*pkru = xpkru->pkru;
} else if (pkru) {
*pkru = 0;
}
Otherwise, looks good from a KVM perspective. Thanks!
