[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-stable] [PATCH v2 3/3] x86: Clear MTRRs on vCPU reset
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-stable] [PATCH v2 3/3] x86: Clear MTRRs on vCPU reset |
Date: |
Thu, 14 Aug 2014 23:23:49 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 |
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 <address@hidden>
> Cc: Laszlo Ersek <address@hidden>
> Cc: address@hidden
> ---
>
> 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 <address@hidden>