On 2017-08-29 20:24, Gustavo Lima Chaves wrote:
> This is a first take on the TODO-list entry
> 
>   - whitelist-based MSR access [v1.0]
> 
> *for Intel architecture*. All the architectural MSRs where given a look
> before the coding started: they were categorized, for ease of finding
> things out when one needs to refer back to them and, for the ones that
> were absolutely necessary (at least given the needs of a typical x86
> Linux build, on both root and inmate cell contexts), access without
> VM-exits was granted. Some actual model specific entries are there as
> well, noticed while testing with our baremetal hardware.
> 
> Machine-check exception, thermal event interrupts and others, at least
> on IA, can commonly have scope broader than current core only (e. g. the
> whole package). We tried the best only to give access to registers in
> that domain that would not impact other cores in any hazardous way, e.
> g. enable/disable some MCE errors. We only made such accesses possible
> because Linux relies on them. Currently we're doing nothing on writes
> for these problematic cases and everything seems to run just fine on the
> inmates.
> 
> A lot of Linux requirements regarding MSR access could be checked when
> destroying other inmates (or disabling the hypervisor altogether), when
> the CPUs go back to the root cell and it has to bring them online
> again—a lot of MSR interaction happens at those routines. The rest of
> the required MSRs could be checked running Linux as inmate.
> 
> The whitelist is structured as to be easy as possible to receive
> additions/corrections.
> 
> Signed-off-by: Gustavo Lima Chaves <[email protected]>
> ---
>  hypervisor/arch/x86/include/asm/processor.h |  13 ++
>  hypervisor/arch/x86/vcpu.c                  |  46 +++++-
>  hypervisor/arch/x86/vmx.c                   | 212 
> +++++++++++++++++++++++-----
>  3 files changed, 235 insertions(+), 36 deletions(-)
> 
> diff --git a/hypervisor/arch/x86/include/asm/processor.h 
> b/hypervisor/arch/x86/include/asm/processor.h
> index a658039..db5afa2 100644
> --- a/hypervisor/arch/x86/include/asm/processor.h
> +++ b/hypervisor/arch/x86/include/asm/processor.h
> @@ -72,12 +72,25 @@
>  
>  #define MSR_IA32_APICBASE                            0x0000001b
>  #define MSR_IA32_FEATURE_CONTROL                     0x0000003a
> +#define MSR_IA32_MCG_CTL                             0x0000017b
>  #define MSR_IA32_PAT                                 0x00000277
>  #define MSR_IA32_MTRR_DEF_TYPE                               0x000002ff
>  #define MSR_IA32_SYSENTER_CS                         0x00000174
>  #define MSR_IA32_SYSENTER_ESP                                0x00000175
>  #define MSR_IA32_SYSENTER_EIP                                0x00000176
> +#define MSR_IA32_PERF_CTL                            0x00000199
> +#define MSR_IA32_THERM_INTERRUPT                     0x0000019b
> +#define MSR_IA32_MISC_ENABLE                         0x000001a0
> +#define MSR_IA32_TEMPERATURE_TARGET                  0x000001a2
> +#define MSR_OFFCORE_RSP_0                            0x000001a6
> +#define MSR_OFFCORE_RSP_1                            0x000001a7
> +#define MSR_IA32_PACKAGE_THERM_INTERRUPT             0x000001b2
> +#define MSR_IA32_MC0_CTL2                            0x00000280
> +#define MSR_IA32_MC31_CTL2                           0x0000029f
> +#define MSR_IA32_FIXED_CTR_CTRL                      0x0000038d
>  #define MSR_IA32_PERF_GLOBAL_CTRL                    0x0000038f
> +#define MSR_IA32_MC0_CTL                             0x00000400
> +#define MSR_IA32_MC28_MISC                           0x00000473
>  #define MSR_IA32_VMX_BASIC                           0x00000480
>  #define MSR_IA32_VMX_PINBASED_CTLS                   0x00000481
>  #define MSR_IA32_VMX_PROCBASED_CTLS                  0x00000482
> diff --git a/hypervisor/arch/x86/vcpu.c b/hypervisor/arch/x86/vcpu.c
> index 638d166..afb4f72 100644
> --- a/hypervisor/arch/x86/vcpu.c
> +++ b/hypervisor/arch/x86/vcpu.c
> @@ -291,7 +291,7 @@ bool vcpu_handle_msr_write(void)
>  {
>       struct per_cpu *cpu_data = this_cpu_data();
>       unsigned int bit_pos, pa;
> -     unsigned long val;
> +     unsigned long val, tmp;
>  
>       switch (cpu_data->guest_regs.rcx) {
>       case MSR_X2APIC_BASE ... MSR_X2APIC_END:
> @@ -325,6 +325,50 @@ bool vcpu_handle_msr_write(void)
>               vcpu_vendor_set_guest_pat((val & MTRR_ENABLE) ?
>                                         cpu_data->pat : 0);
>               break;
> +     case MSR_IA32_MISC_ENABLE:
> +             /* Allow 'Fast-Strings Enable' bit (pos. 0) changes, only */
> +             val = get_wrmsr_value(&cpu_data->guest_regs);
> +             tmp = read_msr(MSR_IA32_MISC_ENABLE);
> +             if (val % 2)
> +                     cpu_data->guest_regs.rax = tmp | 1;
> +             else
> +                     cpu_data->guest_regs.rax = tmp & ~1;

Do not modify the guest registers for this. Also, use a symbolic
constant for the FSE bit.

> +             write_msr(MSR_IA32_MISC_ENABLE, cpu_data->guest_regs.rax);
> +             break;

How about something like this:

val = read_msr(MSR_IA32_MISC_ENABLE) & ~FAST_STRING_ENABLE;
val |= get_wrmsr_value(&cpu_data->guest_regs) & FAST_STRING_ENABLE;
write_msr(MSR_IA32_MISC_ENABLE, val);

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

-- 
You received this message because you are subscribed to the Google Groups 
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to