Re: [Qemu-devel] [PATCH v2 3/3] x86: Clear MTRRs on vCPU reset

[Laszlo Ersek <lersek@redhat.com>]

On 08/14/14 21:24, Alex Williamson wrote:
> The SDM specifies (June 2014 Vol3 11.11.5):
> 
>     On a hardware reset, the P6 and more recent processors clear the
>     valid flags in variable-range MTRRs and clear the E flag in the
>     IA32_MTRR_DEF_TYPE MSR to disable all MTRRs. All other bits in the
>     MTRRs are undefined.
> 
> We currently do none of that, so whatever MTRR settings you had prior
> to reset is what you have after reset.  Usually this doesn't matter
> because KVM often ignores the guest mappings and uses write-back
> anyway.  However, if you have an assigned device and an IOMMU that
> allows NoSnoop for that device, KVM defers to the guest memory
> mappings which are now stale after reset.  The result is that OVMF
> rebooting on such a configuration takes a full minute to LZMA
> decompress the firmware volume, a process that is nearly instant on
> the initial boot.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: qemu-stable@nongnu.org
> ---
> 
>  target-i386/cpu.c |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 6d008ab..9768be1 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2588,6 +2588,16 @@ static void x86_cpu_reset(CPUState *s)
>  
>      env->xcr0 = 1;
>  
> +    /*
> +     * SDM 11.11.5 requires:
> +     *  - IA32_MTRR_DEF_TYPE MSR.E = 0
> +     *  - IA32_MTRR_PHYSMASKn.V = 0
> +     * All other bits are undefined.  For simplification, zero it all.
> +     */
> +    env->mtrr_deftype = 0;
> +    memset(env->mtrr_var, 0, sizeof(env->mtrr_var));
> +    memset(env->mtrr_fixed, 0, sizeof(env->mtrr_fixed));
> +
>  #if !defined(CONFIG_USER_ONLY)
>      /* We hard-wire the BSP to the first CPU. */
>      if (s->cpu_index == 0) {
> 

I like this heavy-handed approach.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>