[Qemu-devel] [PULL for-2.0 0/2] QOM CPUState patch queue 2014-03-31

[Qemu-devel] [PULL for-2.0 0/2] QOM CPUState patch queue 2014-03-31

[Andreas Färber]

Hello Peter,

This is my current QOM CPU patch queue. Please pull.

Regards,
Andreas

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Anthony Liguori <anthony@codemonkey.ws>

The following changes since commit 8648fcd52a9bcc2aa415cbe87b7c636e545acb38:

  make-release: Record SeaBIOS version (2014-03-31 15:02:04 +0100)

are available in the git repository at:

  git://github.com/afaerber/qemu-cpu.git tags/qom-cpu-for-2.0

for you to fetch changes up to 0d6d1ab4990b6e8c6f24e9b1308801d657d411ad:

  cpu: Avoid QOM casts for CPU() (2014-03-31 19:28:38 +0200)

----------------------------------------------------------------
QOM CPUState refactorings / X86CPU

* X86CPU IA32e 1GB paging support
* Performance quickfix for CPU() cast macro

----------------------------------------------------------------
Andreas Färber (1):
      cpu: Avoid QOM casts for CPU()

Luiz Capitulino (1):
      target-i386: x86_cpu_get_phys_page_debug(): support 1GB page translation

 include/qom/cpu.h    |  7 ++++++-
 target-i386/helper.c | 11 +++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

[Qemu-devel] [PULL for-2.0 1/2] target-i386: x86_cpu_get_phys_page_debug(): support 1GB page translation

[Andreas Färber]

From: Luiz Capitulino <lcapitulino@redhat.com>

Linux guests, when using more than 4GB of RAM, may end up using 1GB pages
to store (kernel) data. When this happens, we're unable to debug a running
Linux kernel with GDB:

(gdb) p node_data[0]->node_id
Cannot access memory at address 0xffff88013fffd3a0
(gdb)

GDB returns this error because x86_cpu_get_phys_page_debug() doesn't support
translating 1GB pages in IA-32e paging mode and returns an error to GDB.

This commit adds support for 1GB page translation for IA32e paging.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-i386/helper.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index 4f447b8..372f0e3 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -941,6 +941,14 @@ hwaddr x86_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
             pdpe = ldq_phys(cs->as, pdpe_addr);
             if (!(pdpe & PG_PRESENT_MASK))
                 return -1;
+
+            if (pdpe & PG_PSE_MASK) {
+                page_size = 1024 * 1024 * 1024;
+                pte = pdpe & ~( (page_size - 1) & ~0xfff);
+                pte &= ~(PG_NX_MASK | PG_HI_USER_MASK);
+                goto out;
+            }
+
         } else
 #endif
         {
@@ -993,6 +1001,9 @@ hwaddr x86_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
         pte = pte & env->a20_mask;
     }
 
+#ifdef TARGET_X86_64
+out:
+#endif
     page_offset = (addr & TARGET_PAGE_MASK) & (page_size - 1);
     paddr = (pte & TARGET_PAGE_MASK) + page_offset;
     return paddr;
-- 
1.8.4.5

[Qemu-devel] [PULL for-2.0 2/2] cpu: Avoid QOM casts for CPU()

[Andreas Färber]

CPU address spaces touching load and store helpers as well as the
movement of (almost) all fields from CPU_COMMON to CPUState have led to
a noticeable increase of CPU() usage in "hot" paths for both TCG and KVM.

While CPU()'s OBJECT_CHECK() might help detect development errors, i.e.
in form of crashes due to QOM vs. non-QOM mismatches rather than QOM
type mismatches, it is not really needed at runtime since mostly used in
CPU-specific paths, coming from a target-specific CPU subtype. If that
pointer is damaged, other errors are highly likely to occur elsewhere
anyway.

Keep the CPU() macro for a consistent developer experience and for
flexibility to exchange its implementation, but turn it into a pure,
unchecked C cast for now.

Compare commit 6e42be7cd10260fd3a006d94f6c870692bf7a2c0.

Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 include/qom/cpu.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index f99885a..df977c8 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -53,7 +53,12 @@ typedef uint64_t vaddr;
 
 #define TYPE_CPU "cpu"
 
-#define CPU(obj) OBJECT_CHECK(CPUState, (obj), TYPE_CPU)
+/* Since this macro is used a lot in hot code paths and in conjunction with
+ * FooCPU *foo_env_get_cpu(), we deviate from usual QOM practice by using
+ * an unchecked cast.
+ */
+#define CPU(obj) ((CPUState *)(obj))
+
 #define CPU_CLASS(class) OBJECT_CLASS_CHECK(CPUClass, (class), TYPE_CPU)
 #define CPU_GET_CLASS(obj) OBJECT_GET_CLASS(CPUClass, (obj), TYPE_CPU)
 
-- 
1.8.4.5

Re: [Qemu-devel] [PULL for-2.0 0/2] QOM CPUState patch queue 2014-03-31

[Peter Maydell]

On 31 March 2014 18:37, Andreas Färber <afaerber@suse.de> wrote:
> Hello Peter,
>
> This is my current QOM CPU patch queue. Please pull.
>
> Regards,
> Andreas
>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Anthony Liguori <anthony@codemonkey.ws>
>
> The following changes since commit 8648fcd52a9bcc2aa415cbe87b7c636e545acb38:
>
>   make-release: Record SeaBIOS version (2014-03-31 15:02:04 +0100)
>
> are available in the git repository at:
>
>   git://github.com/afaerber/qemu-cpu.git tags/qom-cpu-for-2.0
>
> for you to fetch changes up to 0d6d1ab4990b6e8c6f24e9b1308801d657d411ad:
>
>   cpu: Avoid QOM casts for CPU() (2014-03-31 19:28:38 +0200)
>
> ----------------------------------------------------------------
> QOM CPUState refactorings / X86CPU
>
> * X86CPU IA32e 1GB paging support
> * Performance quickfix for CPU() cast macro
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PULL for-2.0 2/2] cpu: Avoid QOM casts for CPU()

[Laurent Desnogues]

On Mon, Mar 31, 2014 at 7:37 PM, Andreas Färber <afaerber@suse.de> wrote:
> CPU address spaces touching load and store helpers as well as the
> movement of (almost) all fields from CPU_COMMON to CPUState have led to
> a noticeable increase of CPU() usage in "hot" paths for both TCG and KVM.
>
> While CPU()'s OBJECT_CHECK() might help detect development errors, i.e.
> in form of crashes due to QOM vs. non-QOM mismatches rather than QOM
> type mismatches, it is not really needed at runtime since mostly used in
> CPU-specific paths, coming from a target-specific CPU subtype. If that
> pointer is damaged, other errors are highly likely to occur elsewhere
> anyway.
>
> Keep the CPU() macro for a consistent developer experience and for
> flexibility to exchange its implementation, but turn it into a pure,
> unchecked C cast for now.
>
> Compare commit 6e42be7cd10260fd3a006d94f6c870692bf7a2c0.
>
> Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>

It works fine here.

Tested-by: Laurent Desnogues <laurent.desnogues@gmail.com>

Thanks,

Laurent

> ---
>  include/qom/cpu.h | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index f99885a..df977c8 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -53,7 +53,12 @@ typedef uint64_t vaddr;
>
>  #define TYPE_CPU "cpu"
>
> -#define CPU(obj) OBJECT_CHECK(CPUState, (obj), TYPE_CPU)
> +/* Since this macro is used a lot in hot code paths and in conjunction with
> + * FooCPU *foo_env_get_cpu(), we deviate from usual QOM practice by using
> + * an unchecked cast.
> + */
> +#define CPU(obj) ((CPUState *)(obj))
> +
>  #define CPU_CLASS(class) OBJECT_CLASS_CHECK(CPUClass, (class), TYPE_CPU)
>  #define CPU_GET_CLASS(obj) OBJECT_GET_CLASS(CPUClass, (obj), TYPE_CPU)
>
> --
> 1.8.4.5
>
>

Re: [Qemu-devel] [PULL for-2.0 2/2] cpu: Avoid QOM casts for CPU()

[Laurent Desnogues]

On Mon, Mar 31, 2014 at 7:37 PM, Andreas Färber <afaerber@suse.de> wrote:
> CPU address spaces touching load and store helpers as well as the
> movement of (almost) all fields from CPU_COMMON to CPUState have led to
> a noticeable increase of CPU() usage in "hot" paths for both TCG and KVM.
>
> While CPU()'s OBJECT_CHECK() might help detect development errors, i.e.
> in form of crashes due to QOM vs. non-QOM mismatches rather than QOM
> type mismatches, it is not really needed at runtime since mostly used in
> CPU-specific paths, coming from a target-specific CPU subtype. If that
> pointer is damaged, other errors are highly likely to occur elsewhere
> anyway.
>
> Keep the CPU() macro for a consistent developer experience and for
> flexibility to exchange its implementation, but turn it into a pure,
> unchecked C cast for now.

I re-ran my image to measure the improvements.

Intel 4770K (no HT, no OC)
Fedora Core 19
QEMU 82c6f513735297ad76acaaf2e87f0c5a0b3647a7
Image:  ARM kernel + Google V8 running Sunspider

old: without this patch
strong: default stack protection (strong in my case)
noprot: no stack protection (--disable-stack-protector)
disable: no QOM debug (--disable-qom-cast-debug)

old+strong         16.7
old+strong+disable 15.9
old+noprot         16.6
old+noprot+disable 16.0
    strong         15.8
    strong+disable 15.7
    noprot         15.6
    noprot+disable 15.5

Thanks,

Laurent

> Compare commit 6e42be7cd10260fd3a006d94f6c870692bf7a2c0.
>
> Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  include/qom/cpu.h | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index f99885a..df977c8 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -53,7 +53,12 @@ typedef uint64_t vaddr;
>
>  #define TYPE_CPU "cpu"
>
> -#define CPU(obj) OBJECT_CHECK(CPUState, (obj), TYPE_CPU)
> +/* Since this macro is used a lot in hot code paths and in conjunction with
> + * FooCPU *foo_env_get_cpu(), we deviate from usual QOM practice by using
> + * an unchecked cast.
> + */
> +#define CPU(obj) ((CPUState *)(obj))
> +
>  #define CPU_CLASS(class) OBJECT_CLASS_CHECK(CPUClass, (class), TYPE_CPU)
>  #define CPU_GET_CLASS(obj) OBJECT_GET_CLASS(CPUClass, (obj), TYPE_CPU)
>
> --
> 1.8.4.5
>
>