All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Toshi Kani <toshi.kani@hp.com>
Cc: akpm@linux-foundation.org, hpa@zytor.com, tglx@linutronix.de,
	mingo@redhat.com, linux-mm@kvack.org, x86@kernel.org,
	linux-kernel@vger.kernel.org, dave.hansen@intel.com,
	Elliott@hp.com, pebolle@tiscali.nl
Subject: Re: [PATCH v3 3/5] mtrr, x86: Fix MTRR state checks in mtrr_type_lookup()
Date: Mon, 16 Mar 2015 08:51:39 +0100	[thread overview]
Message-ID: <20150316075139.GB15955@gmail.com> (raw)
In-Reply-To: <1426282421-25385-4-git-send-email-toshi.kani@hp.com>


* Toshi Kani <toshi.kani@hp.com> wrote:

> 'mtrr_state.enabled' contains FE (fixed MTRRs enabled) and
> E (MTRRs enabled) flags in MSR_MTRRdefType.  Intel SDM,
> section 11.11.2.1, defines these flags as follows:
>  - All MTRRs are disabled when the E flag is clear.
>    The FE flag has no affect when the E flag is clear.
>  - The default type is enabled when the E flag is set.
>  - MTRR variable ranges are enabled when the E flag is set.
>  - MTRR fixed ranges are enabled when both E and FE flags
>    are set.
> 
> MTRR state checks in __mtrr_type_lookup() do not follow the
> SDM definitions.  Therefore, this patch fixes the MTRR state
> checks according to the SDM.  This patch defines the flags
> in mtrr_state.enabled as follows.  print_mtrr_state() is also
> updated.
>  - FE flag: MTRR_STATE_MTRR_FIXED_ENABLED
>  - E  flag: MTRR_STATE_MTRR_ENABLED
> 
> Lastly, this patch fixes the 'else if (start < 0x1000000)',
> which checks a fixed range but has an extra-zero in the
> address, to 'else' with no condition.

Firstly, this does multiple bug fixes in a single patch, which is a 
no-no: please split it up into separate patches.

Secondly, please also outline the differences between the old code and 
the new code - don't just list the SDM logic and state that we are 
updating to it.

Thanks,

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Ingo Molnar <mingo@kernel.org>
To: Toshi Kani <toshi.kani@hp.com>
Cc: akpm@linux-foundation.org, hpa@zytor.com, tglx@linutronix.de,
	mingo@redhat.com, linux-mm@kvack.org, x86@kernel.org,
	linux-kernel@vger.kernel.org, dave.hansen@intel.com,
	Elliott@hp.com, pebolle@tiscali.nl
Subject: Re: [PATCH v3 3/5] mtrr, x86: Fix MTRR state checks in mtrr_type_lookup()
Date: Mon, 16 Mar 2015 08:51:39 +0100	[thread overview]
Message-ID: <20150316075139.GB15955@gmail.com> (raw)
In-Reply-To: <1426282421-25385-4-git-send-email-toshi.kani@hp.com>


* Toshi Kani <toshi.kani@hp.com> wrote:

> 'mtrr_state.enabled' contains FE (fixed MTRRs enabled) and
> E (MTRRs enabled) flags in MSR_MTRRdefType.  Intel SDM,
> section 11.11.2.1, defines these flags as follows:
>  - All MTRRs are disabled when the E flag is clear.
>    The FE flag has no affect when the E flag is clear.
>  - The default type is enabled when the E flag is set.
>  - MTRR variable ranges are enabled when the E flag is set.
>  - MTRR fixed ranges are enabled when both E and FE flags
>    are set.
> 
> MTRR state checks in __mtrr_type_lookup() do not follow the
> SDM definitions.  Therefore, this patch fixes the MTRR state
> checks according to the SDM.  This patch defines the flags
> in mtrr_state.enabled as follows.  print_mtrr_state() is also
> updated.
>  - FE flag: MTRR_STATE_MTRR_FIXED_ENABLED
>  - E  flag: MTRR_STATE_MTRR_ENABLED
> 
> Lastly, this patch fixes the 'else if (start < 0x1000000)',
> which checks a fixed range but has an extra-zero in the
> address, to 'else' with no condition.

Firstly, this does multiple bug fixes in a single patch, which is a 
no-no: please split it up into separate patches.

Secondly, please also outline the differences between the old code and 
the new code - don't just list the SDM logic and state that we are 
updating to it.

Thanks,

	Ingo

  reply	other threads:[~2015-03-16  7:51 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-13 21:33 [PATCH v3 0/5] mtrr, mm, x86: Enhance MTRR checks for huge I/O mapping Toshi Kani
2015-03-13 21:33 ` Toshi Kani
2015-03-13 21:33 ` [PATCH v3 1/5] mm, x86: Document return values of mapping funcs Toshi Kani
2015-03-13 21:33   ` Toshi Kani
2015-03-13 21:33 ` [PATCH v3 2/5] mtrr, x86: Fix MTRR lookup to handle inclusive entry Toshi Kani
2015-03-13 21:33   ` Toshi Kani
2015-03-16  7:49   ` Ingo Molnar
2015-03-16  7:49     ` Ingo Molnar
2015-03-16 21:03     ` Kani, Toshimitsu
2015-03-16 21:03       ` Kani, Toshimitsu
2015-03-13 21:33 ` [PATCH v3 3/5] mtrr, x86: Fix MTRR state checks in mtrr_type_lookup() Toshi Kani
2015-03-13 21:33   ` Toshi Kani
2015-03-16  7:51   ` Ingo Molnar [this message]
2015-03-16  7:51     ` Ingo Molnar
2015-03-16 21:08     ` Kani, Toshimitsu
2015-03-16 21:08       ` Kani, Toshimitsu
2015-03-13 21:33 ` [PATCH v3 4/5] mtrr, x86: Clean up mtrr_type_lookup() Toshi Kani
2015-03-13 21:33   ` Toshi Kani
2015-03-16  7:58   ` Ingo Molnar
2015-03-16  7:58     ` Ingo Molnar
2015-03-16 21:24     ` Kani, Toshimitsu
2015-03-16 21:24       ` Kani, Toshimitsu
2015-03-23 19:27       ` Toshi Kani
2015-03-23 19:27         ` Toshi Kani
2015-03-13 21:33 ` [PATCH v3 5/5] mtrr, mm, x86: Enhance MTRR checks for KVA huge page mapping Toshi Kani
2015-03-13 21:33   ` Toshi Kani

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150316075139.GB15955@gmail.com \
    --to=mingo@kernel.org \
    --cc=Elliott@hp.com \
    --cc=akpm@linux-foundation.org \
    --cc=dave.hansen@intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mingo@redhat.com \
    --cc=pebolle@tiscali.nl \
    --cc=tglx@linutronix.de \
    --cc=toshi.kani@hp.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.