[Qemu-devel] [PATCH 0/2] vfio: fix mapping of MSIX bar

[Qemu-devel] [PATCH 0/2] vfio: fix mapping of MSIX bar

[Alexey Kardashevskiy]

I tried to write my own HOST_PAGE_ALIGN to fix VFIO and found out
it is already there but does not work for KVM so here are 2 patches
instead of one.

Alexey Kardashevskiy (2):
  kvm: initialize qemu_host_page_size
  vfio: fix mapping of MSIX bar

 hw/misc/vfio.c          |  6 +++---
 include/exec/exec-all.h |  1 +
 kvm-all.c               |  1 +
 translate-all.c         | 14 ++++++++------
 4 files changed, 13 insertions(+), 9 deletions(-)

-- 
1.8.4.rc4

[Qemu-devel] [PATCH 1/2] kvm: initialize qemu_host_page_size

[Alexey Kardashevskiy]

There is a HOST_PAGE_ALIGN macro which makes sense for KVM accelerator
but it uses qemu_host_page_size/qemu_host_page_mask which initialized
for TCG only.

This moves qemu_host_page_size/qemu_host_page_mask initialization from
TCG's page_init() and adds a call for it from kvm_init().

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 include/exec/exec-all.h |  1 +
 kvm-all.c               |  1 +
 translate-all.c         | 14 ++++++++------
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index ea90b64..3b03cbf 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -81,6 +81,7 @@ void cpu_gen_init(void);
 int cpu_gen_code(CPUArchState *env, struct TranslationBlock *tb,
                  int *gen_code_size_ptr);
 bool cpu_restore_state(CPUArchState *env, uintptr_t searched_pc);
+void page_size_init(void);
 
 void QEMU_NORETURN cpu_resume_from_signal(CPUArchState *env1, void *puc);
 void QEMU_NORETURN cpu_io_recompile(CPUArchState *env, uintptr_t retaddr);
diff --git a/kvm-all.c b/kvm-all.c
index 0bfb060..edf2365 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1360,6 +1360,7 @@ int kvm_init(void)
      * page size for the system though.
      */
     assert(TARGET_PAGE_SIZE <= getpagesize());
+    page_size_init();
 
 #ifdef KVM_CAP_SET_GUEST_DEBUG
     QTAILQ_INIT(&s->kvm_sw_breakpoints);
diff --git a/translate-all.c b/translate-all.c
index 105c25a..543e1ff 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -289,17 +289,15 @@ static inline void map_exec(void *addr, long size)
 }
 #endif
 
-static void page_init(void)
+void page_size_init(void)
 {
     /* NOTE: we can always suppose that qemu_host_page_size >=
        TARGET_PAGE_SIZE */
 #ifdef _WIN32
-    {
-        SYSTEM_INFO system_info;
+    SYSTEM_INFO system_info;
 
-        GetSystemInfo(&system_info);
-        qemu_real_host_page_size = system_info.dwPageSize;
-    }
+    GetSystemInfo(&system_info);
+    qemu_real_host_page_size = system_info.dwPageSize;
 #else
     qemu_real_host_page_size = getpagesize();
 #endif
@@ -310,7 +308,11 @@ static void page_init(void)
         qemu_host_page_size = TARGET_PAGE_SIZE;
     }
     qemu_host_page_mask = ~(qemu_host_page_size - 1);
+}
 
+static void page_init(void)
+{
+    page_size_init();
 #if defined(CONFIG_BSD) && defined(CONFIG_USER_ONLY)
     {
 #ifdef HAVE_KINFO_GETVMMAP
-- 
1.8.4.rc4

Re: [Qemu-devel] [PATCH 1/2] kvm: initialize qemu_host_page_size

[Paolo Bonzini]

Il 16/01/2014 07:21, Alexey Kardashevskiy ha scritto:
> There is a HOST_PAGE_ALIGN macro which makes sense for KVM accelerator
> but it uses qemu_host_page_size/qemu_host_page_mask which initialized
> for TCG only.
> 
> This moves qemu_host_page_size/qemu_host_page_mask initialization from
> TCG's page_init() and adds a call for it from kvm_init().
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>  include/exec/exec-all.h |  1 +
>  kvm-all.c               |  1 +
>  translate-all.c         | 14 ++++++++------
>  3 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index ea90b64..3b03cbf 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -81,6 +81,7 @@ void cpu_gen_init(void);
>  int cpu_gen_code(CPUArchState *env, struct TranslationBlock *tb,
>                   int *gen_code_size_ptr);
>  bool cpu_restore_state(CPUArchState *env, uintptr_t searched_pc);
> +void page_size_init(void);
>  
>  void QEMU_NORETURN cpu_resume_from_signal(CPUArchState *env1, void *puc);
>  void QEMU_NORETURN cpu_io_recompile(CPUArchState *env, uintptr_t retaddr);
> diff --git a/kvm-all.c b/kvm-all.c
> index 0bfb060..edf2365 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1360,6 +1360,7 @@ int kvm_init(void)
>       * page size for the system though.
>       */
>      assert(TARGET_PAGE_SIZE <= getpagesize());
> +    page_size_init();
>  
>  #ifdef KVM_CAP_SET_GUEST_DEBUG
>      QTAILQ_INIT(&s->kvm_sw_breakpoints);
> diff --git a/translate-all.c b/translate-all.c
> index 105c25a..543e1ff 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -289,17 +289,15 @@ static inline void map_exec(void *addr, long size)
>  }
>  #endif
>  
> -static void page_init(void)
> +void page_size_init(void)
>  {
>      /* NOTE: we can always suppose that qemu_host_page_size >=
>         TARGET_PAGE_SIZE */
>  #ifdef _WIN32
> -    {
> -        SYSTEM_INFO system_info;
> +    SYSTEM_INFO system_info;
>  
> -        GetSystemInfo(&system_info);
> -        qemu_real_host_page_size = system_info.dwPageSize;
> -    }
> +    GetSystemInfo(&system_info);
> +    qemu_real_host_page_size = system_info.dwPageSize;
>  #else
>      qemu_real_host_page_size = getpagesize();
>  #endif
> @@ -310,7 +308,11 @@ static void page_init(void)
>          qemu_host_page_size = TARGET_PAGE_SIZE;
>      }
>      qemu_host_page_mask = ~(qemu_host_page_size - 1);
> +}
>  
> +static void page_init(void)
> +{
> +    page_size_init();
>  #if defined(CONFIG_BSD) && defined(CONFIG_USER_ONLY)
>      {
>  #ifdef HAVE_KINFO_GETVMMAP
> 

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH 1/2] kvm: initialize qemu_host_page_size

[Alex Williamson]

On Fri, 2014-01-17 at 13:55 +0100, Paolo Bonzini wrote:
> Il 16/01/2014 07:21, Alexey Kardashevskiy ha scritto:
> > There is a HOST_PAGE_ALIGN macro which makes sense for KVM accelerator
> > but it uses qemu_host_page_size/qemu_host_page_mask which initialized
> > for TCG only.
> > 
> > This moves qemu_host_page_size/qemu_host_page_mask initialization from
> > TCG's page_init() and adds a call for it from kvm_init().
> > 
> > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > ---
> >  include/exec/exec-all.h |  1 +
> >  kvm-all.c               |  1 +
> >  translate-all.c         | 14 ++++++++------
> >  3 files changed, 10 insertions(+), 6 deletions(-)
> > 
> > diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> > index ea90b64..3b03cbf 100644
> > --- a/include/exec/exec-all.h
> > +++ b/include/exec/exec-all.h
> > @@ -81,6 +81,7 @@ void cpu_gen_init(void);
> >  int cpu_gen_code(CPUArchState *env, struct TranslationBlock *tb,
> >                   int *gen_code_size_ptr);
> >  bool cpu_restore_state(CPUArchState *env, uintptr_t searched_pc);
> > +void page_size_init(void);
> >  
> >  void QEMU_NORETURN cpu_resume_from_signal(CPUArchState *env1, void *puc);
> >  void QEMU_NORETURN cpu_io_recompile(CPUArchState *env, uintptr_t retaddr);
> > diff --git a/kvm-all.c b/kvm-all.c
> > index 0bfb060..edf2365 100644
> > --- a/kvm-all.c
> > +++ b/kvm-all.c
> > @@ -1360,6 +1360,7 @@ int kvm_init(void)
> >       * page size for the system though.
> >       */
> >      assert(TARGET_PAGE_SIZE <= getpagesize());
> > +    page_size_init();
> >  
> >  #ifdef KVM_CAP_SET_GUEST_DEBUG
> >      QTAILQ_INIT(&s->kvm_sw_breakpoints);
> > diff --git a/translate-all.c b/translate-all.c
> > index 105c25a..543e1ff 100644
> > --- a/translate-all.c
> > +++ b/translate-all.c
> > @@ -289,17 +289,15 @@ static inline void map_exec(void *addr, long size)
> >  }
> >  #endif
> >  
> > -static void page_init(void)
> > +void page_size_init(void)
> >  {
> >      /* NOTE: we can always suppose that qemu_host_page_size >=
> >         TARGET_PAGE_SIZE */
> >  #ifdef _WIN32
> > -    {
> > -        SYSTEM_INFO system_info;
> > +    SYSTEM_INFO system_info;
> >  
> > -        GetSystemInfo(&system_info);
> > -        qemu_real_host_page_size = system_info.dwPageSize;
> > -    }
> > +    GetSystemInfo(&system_info);
> > +    qemu_real_host_page_size = system_info.dwPageSize;
> >  #else
> >      qemu_real_host_page_size = getpagesize();
> >  #endif
> > @@ -310,7 +308,11 @@ static void page_init(void)
> >          qemu_host_page_size = TARGET_PAGE_SIZE;
> >      }
> >      qemu_host_page_mask = ~(qemu_host_page_size - 1);
> > +}
> >  
> > +static void page_init(void)
> > +{
> > +    page_size_init();
> >  #if defined(CONFIG_BSD) && defined(CONFIG_USER_ONLY)
> >      {
> >  #ifdef HAVE_KINFO_GETVMMAP
> > 
> 
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>

How should this go in?  With your ack I could include it in my vfio tree
with patch 2/2.  Sound ok?  Thanks,

Alex

Re: [Qemu-devel] [PATCH 1/2] kvm: initialize qemu_host_page_size

[Paolo Bonzini]

Il 17/01/2014 16:34, Alex Williamson ha scritto:
>> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> How should this go in?  With your ack I could include it in my vfio tree
> with patch 2/2.  Sound ok?  Thanks,

Yup, thanks!

This is just a small enabler, the real meat is in patch 2 so it makes
sense for you to pick up both.

Paolo

[Qemu-devel] [PATCH 2/2] vfio: fix mapping of MSIX bar

[Alexey Kardashevskiy]

VFIO virtualizes MSIX table for the guest but not mapping the part of
a BAR which contains an MSIX table. Since vfio_mmap_bar() mmaps chunks
before and after the MSIX table, they have to be aligned to the host
page size which may be TARGET_PAGE_MASK (4K) or 64K in case of PPC64.

This fixes boundaries calculations to use the real host page size.

Without the patch, the chunk before MSIX table may overlap with the MSIX
table and mmap will fail in the host kernel. The result will be serious
slowdown as the whole BAR will be emulated by QEMU.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/misc/vfio.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index 9aecaa8..9989bea 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -2501,7 +2501,7 @@ static void vfio_map_bar(VFIODevice *vdev, int nr)
      * potentially insert a direct-mapped subregion before and after it.
      */
     if (vdev->msix && vdev->msix->table_bar == nr) {
-        size = vdev->msix->table_offset & TARGET_PAGE_MASK;
+        size = vdev->msix->table_offset & qemu_host_page_mask;
     }
 
     strncat(name, " mmap", sizeof(name) - strlen(name) - 1);
@@ -2513,8 +2513,8 @@ static void vfio_map_bar(VFIODevice *vdev, int nr)
     if (vdev->msix && vdev->msix->table_bar == nr) {
         unsigned start;
 
-        start = TARGET_PAGE_ALIGN(vdev->msix->table_offset +
-                                  (vdev->msix->entries * PCI_MSIX_ENTRY_SIZE));
+        start = HOST_PAGE_ALIGN(vdev->msix->table_offset +
+                                (vdev->msix->entries * PCI_MSIX_ENTRY_SIZE));
 
         size = start < bar->size ? bar->size - start : 0;
         strncat(name, " msix-hi", sizeof(name) - strlen(name) - 1);
-- 
1.8.4.rc4