Re: [PATCH 1/3] x86/msr: Split MSR_SPEC_CTRL handling

[Andrew Cooper <amc96@xxxxxxxx>]

On 17/01/2022 11:07, Jan Beulich wrote:
> On 13.01.2022 17:38, Andrew Cooper wrote:
>> In order to fix a VT-x bug, and support MSR_SPEC_CTRL on AMD, there will need
>> to be three different access methods for where the guest's value lives.
>> However, it would be better not to duplicate the #GP checking logic.
>>
>> guest_{rd,wr}msr() are always called first in the PV and HVM MSR paths, so we
>> can repurpose X86EMUL_UNHANDLEABLE slightly for this.  This is going to be a
>> common pattern for other MSRs too in the future.
> I consider this repurposing risky. Did you consider using e.g.
> X86EMUL_DONE or X86EMUL_RETRY instead? Neither of the two is
> presently used by the MSR machinery afaics.

RETRY is used for the MSRs which can cause a p2m allocation and hit the
paging path.  DONE is not remotely appropriate for this purpose.

I also don't want to introduce a new PARTIAL because that is a recipe
for backport bugs.

> What's worse, aren't you making it possible to hit the
> ASSERT_UNREACHABLE() in hvm_save_cpu_msrs() as well as in
> XEN_DOMCTL_get_vcpu_msrs handling? And have hvm_load_cpu_msrs()
> produce -ENXIO and XEN_DOMCTL_set_vcpu_msrs return -EINVAL?

I found this bug after backporting the patches and getting them some
wider testing.  I'm doing a pre-req patch to rework the migration
machinery to call into the pv/hvm paths rather than into
guest_{rd,wr}msr() directly.

~Andrew