On 04.04.2023 18:06, Sergey Dyasli wrote:
> Currently it's impossible to get CPU's microcode revision from Xen after
> late loading without looking into Xen logs which is not always convenient.
>
> Add a new platform op in order to get the required data from Xen and
> provide a wrapper for libxenctrl.
>
> Signed-off-by: Sergey Dyasli <sergey.dya...@citrix.com>
Reviewed-by: Jan Beulich <jbeul...@suse.com>
with two remarks:
> --- a/tools/libs/ctrl/xc_misc.c
> +++ b/tools/libs/ctrl/xc_misc.c
> @@ -243,6 +243,24 @@ int xc_get_cpu_version(xc_interface *xch, struct
> xenpf_pcpu_version *cpu_ver)
> return 0;
> }
>
> +int xc_get_ucode_revision(xc_interface *xch,
> + struct xenpf_ucode_revision *ucode_rev)
> +{
> + int ret;
> + struct xen_platform_op op = {
> + .cmd = XENPF_get_ucode_revision,
> + .u.ucode_revision.cpu = ucode_rev->cpu,
> + };
> +
> + ret = do_platform_op(xch, &op);
> + if ( ret != 0 )
> + return ret;
Is there anything wrong with omitting this if() and ...
> + *ucode_rev = op.u.ucode_revision;
> +
> + return 0;
... using "return ret" here?
> --- a/xen/arch/x86/platform_hypercall.c
> +++ b/xen/arch/x86/platform_hypercall.c
> @@ -640,6 +640,35 @@ ret_t do_platform_op(
> }
> break;
>
> + case XENPF_get_ucode_revision:
> + {
> + struct xenpf_ucode_revision *rev = &op->u.ucode_revision;
> +
> + if ( !get_cpu_maps() )
> + {
> + ret = -EBUSY;
> + break;
> + }
> +
> + /* TODO: make it possible to know ucode revisions for parked CPUs */
> + if ( (rev->cpu >= nr_cpu_ids) || !cpu_online(rev->cpu) )
> + ret = -ENOENT;
While the cpu_online() check needs to be done under lock, it's kind of
misleading for the caller to tell it to try again later when it has
passed an out-of-range CPU number.
Jan
