On 30.04.2024 14:47, Fouad Hilly wrote:
> @@ -633,12 +637,12 @@ static long cf_check microcode_update_helper(void *data)
> microcode_cache);
>
> if ( result != NEW_UCODE &&
> - !(opt_ucode_allow_same && result == SAME_UCODE) )
> + !((opt_ucode_allow_same || ucode_force_flag) && result ==
> SAME_UCODE) )
Why would "force" not also allow a downgrade?
> @@ -708,11 +712,15 @@ static long cf_check microcode_update_helper(void *data)
> return ret;
> }
>
> -int microcode_update(XEN_GUEST_HANDLE(const_void) buf, unsigned long len)
> +int microcode_update(XEN_GUEST_HANDLE(const_void) buf,
> + unsigned long len, unsigned int flags)
> {
> int ret;
> struct ucode_buf *buffer;
>
> + if ( flags > 1 )
How is one to connect this literal 1 with what is really meant here? Also
would be nice if this check fit with other similar checks we do, i.e.
if ( flags & ~XENPF_UCODE_FLAG_FORCE_SET )
> + return -EINVAL;
> +
> if ( len != (uint32_t)len )
> return -E2BIG;
As an aside: Isn't this dead code, with the respective hypercall interface
struct fields (now) both being uint32_t?
> --- a/xen/arch/x86/platform_hypercall.c
> +++ b/xen/arch/x86/platform_hypercall.c
> @@ -311,7 +311,17 @@ ret_t do_platform_op(
>
> guest_from_compat_handle(data, op->u.microcode.data);
>
> - ret = microcode_update(data, op->u.microcode.length);
> + ret = microcode_update(data, op->u.microcode.length, 0);
> + break;
> + }
> +
> + case XENPF_microcode_update2:
> + {
> + XEN_GUEST_HANDLE(const_void) data;
> +
> + guest_from_compat_handle(data, op->u.microcode2.data);
> +
> + ret = microcode_update(data, op->u.microcode2.length,
> op->u.microcode2.flags);
Nit (style): Overlong line.
> --- a/xen/include/public/platform.h
> +++ b/xen/include/public/platform.h
> @@ -624,6 +624,19 @@ struct xenpf_ucode_revision {
> typedef struct xenpf_ucode_revision xenpf_ucode_revision_t;
> DEFINE_XEN_GUEST_HANDLE(xenpf_ucode_revision_t);
>
> +/* Hypercall to microcode_update with flags */
> +#define XENPF_microcode_update2 66
> +struct xenpf_microcode_update2 {
> + /* IN variables. */
> + uint32_t flags; /* Flags to be passed with ucode. */
> +/* Force to skip microcode version check when set */
> +#define XENPF_UCODE_FLAG_FORCE_SET 1
Nit: What is "SET" in the identifier intended to mean?
Jan
