[Qemu-devel] [PATCH] exec.c: Clarify comment about tlb_flush() flush_global parameter

[Qemu-devel] [PATCH] exec.c: Clarify comment about tlb_flush() flush_global parameter

[Peter Maydell]

Clarify the comment about tlb_flush()'s flush_global parameter,
so it is clearer what it does and why it is OK that the implementation
currently ignores it.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Minor clarification following a conversation on IRC...

 exec.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/exec.c b/exec.c
index 7f9f730..f667cf0 100644
--- a/exec.c
+++ b/exec.c
@@ -1876,8 +1876,18 @@ static CPUTLBEntry s_cputlb_empty_entry = {
     .addend     = -1,
 };
 
-/* NOTE: if flush_global is true, also flush global entries (not
-   implemented yet) */
+/* NOTE:
+ * If flush_global is true (the usual case), flush all tlb entries.
+ * If flush_global is false, flush (at least) all tlb entries not
+ * marked global.
+ *
+ * Since QEMU doesn't currently implement a global/not-global flag
+ * for tlb entries, at the moment tlb_flush() will also flush all
+ * tlb entries in the flush_global == false case. This is OK because
+ * CPU architectures generally permit an implementation to drop
+ * entries from the TLB at any time, so flushing more entries than
+ * required is only an efficiency issue, not a correctness issue.
+ */
 void tlb_flush(CPUState *env, int flush_global)
 {
     int i;
-- 
1.7.1

Re: [Qemu-devel] [PATCH] exec.c: Clarify comment about tlb_flush() flush_global parameter

[Peter Maydell]

Ping?

-- PMM

On 17 January 2012 13:23, Peter Maydell <peter.maydell@linaro.org> wrote:
> Clarify the comment about tlb_flush()'s flush_global parameter,
> so it is clearer what it does and why it is OK that the implementation
> currently ignores it.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Minor clarification following a conversation on IRC...
>
>  exec.c |   14 ++++++++++++--
>  1 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 7f9f730..f667cf0 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1876,8 +1876,18 @@ static CPUTLBEntry s_cputlb_empty_entry = {
>     .addend     = -1,
>  };
>
> -/* NOTE: if flush_global is true, also flush global entries (not
> -   implemented yet) */
> +/* NOTE:
> + * If flush_global is true (the usual case), flush all tlb entries.
> + * If flush_global is false, flush (at least) all tlb entries not
> + * marked global.
> + *
> + * Since QEMU doesn't currently implement a global/not-global flag
> + * for tlb entries, at the moment tlb_flush() will also flush all
> + * tlb entries in the flush_global == false case. This is OK because
> + * CPU architectures generally permit an implementation to drop
> + * entries from the TLB at any time, so flushing more entries than
> + * required is only an efficiency issue, not a correctness issue.
> + */
>  void tlb_flush(CPUState *env, int flush_global)
>  {
>     int i;
> --
> 1.7.1
>
>

Re: [Qemu-devel] [PATCH] exec.c: Clarify comment about tlb_flush() flush_global parameter

[Andreas Färber]

Am 17.01.2012 14:23, schrieb Peter Maydell:
> Clarify the comment about tlb_flush()'s flush_global parameter,
> so it is clearer what it does and why it is OK that the implementation
> currently ignores it.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Andreas Färber <afaerber@suse.de>

CC'ing qemu-trivial.

Andreas

> ---
> Minor clarification following a conversation on IRC...
> 
>  exec.c |   14 ++++++++++++--
>  1 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 7f9f730..f667cf0 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1876,8 +1876,18 @@ static CPUTLBEntry s_cputlb_empty_entry = {
>      .addend     = -1,
>  };
>  
> -/* NOTE: if flush_global is true, also flush global entries (not
> -   implemented yet) */
> +/* NOTE:
> + * If flush_global is true (the usual case), flush all tlb entries.
> + * If flush_global is false, flush (at least) all tlb entries not
> + * marked global.
> + *
> + * Since QEMU doesn't currently implement a global/not-global flag
> + * for tlb entries, at the moment tlb_flush() will also flush all
> + * tlb entries in the flush_global == false case. This is OK because
> + * CPU architectures generally permit an implementation to drop
> + * entries from the TLB at any time, so flushing more entries than
> + * required is only an efficiency issue, not a correctness issue.
> + */
>  void tlb_flush(CPUState *env, int flush_global)
>  {
>      int i;

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] exec.c: Clarify comment about tlb_flush() flush_global parameter

[Anthony Liguori]

On 01/17/2012 07:23 AM, Peter Maydell wrote:
> Clarify the comment about tlb_flush()'s flush_global parameter,
> so it is clearer what it does and why it is OK that the implementation
> currently ignores it.
>
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>

Applied.  Thanks.

Regards,

Anthony Liguori

> ---
> Minor clarification following a conversation on IRC...
>
>   exec.c |   14 ++++++++++++--
>   1 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 7f9f730..f667cf0 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1876,8 +1876,18 @@ static CPUTLBEntry s_cputlb_empty_entry = {
>       .addend     = -1,
>   };
>
> -/* NOTE: if flush_global is true, also flush global entries (not
> -   implemented yet) */
> +/* NOTE:
> + * If flush_global is true (the usual case), flush all tlb entries.
> + * If flush_global is false, flush (at least) all tlb entries not
> + * marked global.
> + *
> + * Since QEMU doesn't currently implement a global/not-global flag
> + * for tlb entries, at the moment tlb_flush() will also flush all
> + * tlb entries in the flush_global == false case. This is OK because
> + * CPU architectures generally permit an implementation to drop
> + * entries from the TLB at any time, so flushing more entries than
> + * required is only an efficiency issue, not a correctness issue.
> + */
>   void tlb_flush(CPUState *env, int flush_global)
>   {
>       int i;