Re: [PATCH for-4.17 v2 1/3] hvm/msr: load VIRT_SPEC_CTRL

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.10.2022 13:49, Roger Pau Monne wrote:
> Add MSR_VIRT_SPEC_CTRL to the list of MSRs handled by
> hvm_load_cpu_msrs(), or else it would be lost.
> 
> Fixes: 8ffd5496f4 ('amd/msr: implement VIRT_SPEC_CTRL for HVM guests on top 
> of SPEC_CTRL')
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

> ---
> I'm confused as to why we have two different list of MSR to send and
> load, one in msrs_to_send[] and the other open-coded in
> hvm_load_cpu_msrs(), but given the release status it's no time to
> clean that up.

I guess this is an optimization, as looking up the MSR in msrs_to_send[]
would be an inefficient inner loop, with the result only used in a
boolean manner (entry present or not present). I do think though that
both places should have a comment referencing the respectively other one,
so both will (hopefully) be updated together. The same looks to apply to
arch_do_domctl()'s MSR handling, where I've screwed up for XFD{,_ERR}.

I'm puzzled by the ctxt->msr[i]._rsvd checking in the default: case. It
was me who added this 8.5 years ago, but I can't see the value of that
check considering the check in the next to final loop in the function.
Hmm, wait - this had an error checking purpose until commit f61685a66903
("x86: remove defunct init/load/save_msr() hvm_funcs"). I think the check
should have been removed at that point. That'll be a post-4.17 patch, I
suppose ...

Jan