* [Qemu-devel] [For 2.5?? PATCH 1/1] qemu_{real_}host_page_[size|mask] change types to ram_addr_t
@ 2015-12-02 10:43 Dr. David Alan Gilbert (git)
2015-12-02 10:47 ` Juan Quintela
0 siblings, 1 reply; 4+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2015-12-02 10:43 UTC (permalink / raw)
To: qemu-devel, pbonzini, anthony.perard; +Cc: quintela, stefano.stabellini
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Anthony reported that >4GB guests on Xen with 32bit QEMU
broke after my 4ed023c (Round up RAMBlock sizes).
In that patch I mask sizes against qemu_host_page_size/mask
which are uintptr_t, and thus 32bit on a 32bit QEMU, even
though the ram space might be bigger than 4GB on Xen.
ram_addr_t is uintptr_t on KVM and uint64_t on Xen; so
there should be no change on KVM, and on Xen we always get
the larger size.
Remove the ~10 year old scary comment that the type of these
variables is probably wrong.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reported-by: Anthony PERARD <anthony.perard@citrix.com>
Fixes: 4ed023ce2a39ab5812d33cf4d819def168965a7f
---
include/exec/cpu-all.h | 9 ++++-----
translate-all.c | 4 ++--
translate-common.c | 4 ++--
3 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index f9998b9..e23b852 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -174,11 +174,10 @@ extern unsigned long reserved_va;
#define TARGET_PAGE_MASK ~(TARGET_PAGE_SIZE - 1)
#define TARGET_PAGE_ALIGN(addr) (((addr) + TARGET_PAGE_SIZE - 1) & TARGET_PAGE_MASK)
-/* ??? These should be the larger of uintptr_t and target_ulong. */
-extern uintptr_t qemu_real_host_page_size;
-extern uintptr_t qemu_real_host_page_mask;
-extern uintptr_t qemu_host_page_size;
-extern uintptr_t qemu_host_page_mask;
+extern ram_addr_t qemu_real_host_page_size;
+extern ram_addr_t qemu_real_host_page_mask;
+extern ram_addr_t qemu_host_page_size;
+extern ram_addr_t qemu_host_page_mask;
#define HOST_PAGE_ALIGN(addr) (((addr) + qemu_host_page_size - 1) & qemu_host_page_mask)
#define REAL_HOST_PAGE_ALIGN(addr) (((addr) + qemu_real_host_page_size - 1) & \
diff --git a/translate-all.c b/translate-all.c
index a940bd2..317f251 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -117,8 +117,8 @@ typedef struct PageDesc {
#define V_L1_SHIFT (L1_MAP_ADDR_SPACE_BITS - TARGET_PAGE_BITS - V_L1_BITS)
-uintptr_t qemu_host_page_size;
-uintptr_t qemu_host_page_mask;
+ram_addr_t qemu_host_page_size;
+ram_addr_t qemu_host_page_mask;
/* The bottom level has pointers to PageDesc */
static void *l1_map[V_L1_SIZE];
diff --git a/translate-common.c b/translate-common.c
index 619feb4..2fe2d1e 100644
--- a/translate-common.c
+++ b/translate-common.c
@@ -20,8 +20,8 @@
#include "qemu-common.h"
#include "qom/cpu.h"
-uintptr_t qemu_real_host_page_size;
-uintptr_t qemu_real_host_page_mask;
+ram_addr_t qemu_real_host_page_size;
+ram_addr_t qemu_real_host_page_mask;
#ifndef CONFIG_USER_ONLY
/* mask must never be zero, except for A20 change call */
--
2.5.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [For 2.5?? PATCH 1/1] qemu_{real_}host_page_[size|mask] change types to ram_addr_t
2015-12-02 10:43 [Qemu-devel] [For 2.5?? PATCH 1/1] qemu_{real_}host_page_[size|mask] change types to ram_addr_t Dr. David Alan Gilbert (git)
@ 2015-12-02 10:47 ` Juan Quintela
2015-12-02 10:54 ` Paolo Bonzini
2015-12-02 11:27 ` Dr. David Alan Gilbert
0 siblings, 2 replies; 4+ messages in thread
From: Juan Quintela @ 2015-12-02 10:47 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git)
Cc: anthony.perard, pbonzini, qemu-devel, stefano.stabellini
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Anthony reported that >4GB guests on Xen with 32bit QEMU
> broke after my 4ed023c (Round up RAMBlock sizes).
>
> In that patch I mask sizes against qemu_host_page_size/mask
> which are uintptr_t, and thus 32bit on a 32bit QEMU, even
> though the ram space might be bigger than 4GB on Xen.
>
> ram_addr_t is uintptr_t on KVM and uint64_t on Xen; so
> there should be no change on KVM, and on Xen we always get
> the larger size.
>
> Remove the ~10 year old scary comment that the type of these
> variables is probably wrong.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Reported-by: Anthony PERARD <anthony.perard@citrix.com>
> Fixes: 4ed023ce2a39ab5812d33cf4d819def168965a7f
Reviewed-by: Juan Quintela <quintela@redhat.com>
If nobody complains, I will sent this on next migration pull requset.
Paolo, I think that I preffer this that trusting the intptr_t sign
extension, but I can be convinced either way. What do you think?
> ---
> include/exec/cpu-all.h | 9 ++++-----
> translate-all.c | 4 ++--
> translate-common.c | 4 ++--
> 3 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index f9998b9..e23b852 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -174,11 +174,10 @@ extern unsigned long reserved_va;
> #define TARGET_PAGE_MASK ~(TARGET_PAGE_SIZE - 1)
> #define TARGET_PAGE_ALIGN(addr) (((addr) + TARGET_PAGE_SIZE - 1) &
> TARGET_PAGE_MASK)
>
> -/* ??? These should be the larger of uintptr_t and target_ulong. */
> -extern uintptr_t qemu_real_host_page_size;
> -extern uintptr_t qemu_real_host_page_mask;
> -extern uintptr_t qemu_host_page_size;
> -extern uintptr_t qemu_host_page_mask;
> +extern ram_addr_t qemu_real_host_page_size;
> +extern ram_addr_t qemu_real_host_page_mask;
> +extern ram_addr_t qemu_host_page_size;
> +extern ram_addr_t qemu_host_page_mask;
>
> #define HOST_PAGE_ALIGN(addr) (((addr) + qemu_host_page_size - 1) &
> qemu_host_page_mask)
> #define REAL_HOST_PAGE_ALIGN(addr) (((addr) + qemu_real_host_page_size - 1) & \
> diff --git a/translate-all.c b/translate-all.c
> index a940bd2..317f251 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -117,8 +117,8 @@ typedef struct PageDesc {
>
> #define V_L1_SHIFT (L1_MAP_ADDR_SPACE_BITS - TARGET_PAGE_BITS - V_L1_BITS)
>
> -uintptr_t qemu_host_page_size;
> -uintptr_t qemu_host_page_mask;
> +ram_addr_t qemu_host_page_size;
> +ram_addr_t qemu_host_page_mask;
>
> /* The bottom level has pointers to PageDesc */
> static void *l1_map[V_L1_SIZE];
> diff --git a/translate-common.c b/translate-common.c
> index 619feb4..2fe2d1e 100644
> --- a/translate-common.c
> +++ b/translate-common.c
> @@ -20,8 +20,8 @@
> #include "qemu-common.h"
> #include "qom/cpu.h"
>
> -uintptr_t qemu_real_host_page_size;
> -uintptr_t qemu_real_host_page_mask;
> +ram_addr_t qemu_real_host_page_size;
> +ram_addr_t qemu_real_host_page_mask;
>
> #ifndef CONFIG_USER_ONLY
> /* mask must never be zero, except for A20 change call */
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [For 2.5?? PATCH 1/1] qemu_{real_}host_page_[size|mask] change types to ram_addr_t
2015-12-02 10:47 ` Juan Quintela
@ 2015-12-02 10:54 ` Paolo Bonzini
2015-12-02 11:27 ` Dr. David Alan Gilbert
1 sibling, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2015-12-02 10:54 UTC (permalink / raw)
To: quintela, Dr. David Alan Gilbert (git)
Cc: anthony.perard, qemu-devel, stefano.stabellini
On 02/12/2015 11:47, Juan Quintela wrote:
> Reviewed-by: Juan Quintela <quintela@redhat.com>
>
> If nobody complains, I will sent this on next migration pull requset.
>
> Paolo, I think that I preffer this that trusting the intptr_t sign
> extension, but I can be convinced either way. What do you think?
It's not trusting, it's the way C works, but I'm okay either way.
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [For 2.5?? PATCH 1/1] qemu_{real_}host_page_[size|mask] change types to ram_addr_t
2015-12-02 10:47 ` Juan Quintela
2015-12-02 10:54 ` Paolo Bonzini
@ 2015-12-02 11:27 ` Dr. David Alan Gilbert
1 sibling, 0 replies; 4+ messages in thread
From: Dr. David Alan Gilbert @ 2015-12-02 11:27 UTC (permalink / raw)
To: Juan Quintela; +Cc: anthony.perard, pbonzini, qemu-devel, stefano.stabellini
* Juan Quintela (quintela@redhat.com) wrote:
> "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > Anthony reported that >4GB guests on Xen with 32bit QEMU
> > broke after my 4ed023c (Round up RAMBlock sizes).
> >
> > In that patch I mask sizes against qemu_host_page_size/mask
> > which are uintptr_t, and thus 32bit on a 32bit QEMU, even
> > though the ram space might be bigger than 4GB on Xen.
> >
> > ram_addr_t is uintptr_t on KVM and uint64_t on Xen; so
> > there should be no change on KVM, and on Xen we always get
> > the larger size.
> >
> > Remove the ~10 year old scary comment that the type of these
> > variables is probably wrong.
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > Reported-by: Anthony PERARD <anthony.perard@citrix.com>
> > Fixes: 4ed023ce2a39ab5812d33cf4d819def168965a7f
>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
>
> If nobody complains, I will sent this on next migration pull requset.
>
> Paolo, I think that I preffer this that trusting the intptr_t sign
> extension, but I can be convinced either way. What do you think?
Hang on, Juan just pointed out this breaks builds of a bunch of stuff; checking....
Dave
>
> > ---
> > include/exec/cpu-all.h | 9 ++++-----
> > translate-all.c | 4 ++--
> > translate-common.c | 4 ++--
> > 3 files changed, 8 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> > index f9998b9..e23b852 100644
> > --- a/include/exec/cpu-all.h
> > +++ b/include/exec/cpu-all.h
> > @@ -174,11 +174,10 @@ extern unsigned long reserved_va;
> > #define TARGET_PAGE_MASK ~(TARGET_PAGE_SIZE - 1)
> > #define TARGET_PAGE_ALIGN(addr) (((addr) + TARGET_PAGE_SIZE - 1) &
> > TARGET_PAGE_MASK)
> >
> > -/* ??? These should be the larger of uintptr_t and target_ulong. */
> > -extern uintptr_t qemu_real_host_page_size;
> > -extern uintptr_t qemu_real_host_page_mask;
> > -extern uintptr_t qemu_host_page_size;
> > -extern uintptr_t qemu_host_page_mask;
> > +extern ram_addr_t qemu_real_host_page_size;
> > +extern ram_addr_t qemu_real_host_page_mask;
> > +extern ram_addr_t qemu_host_page_size;
> > +extern ram_addr_t qemu_host_page_mask;
> >
> > #define HOST_PAGE_ALIGN(addr) (((addr) + qemu_host_page_size - 1) &
> > qemu_host_page_mask)
> > #define REAL_HOST_PAGE_ALIGN(addr) (((addr) + qemu_real_host_page_size - 1) & \
> > diff --git a/translate-all.c b/translate-all.c
> > index a940bd2..317f251 100644
> > --- a/translate-all.c
> > +++ b/translate-all.c
> > @@ -117,8 +117,8 @@ typedef struct PageDesc {
> >
> > #define V_L1_SHIFT (L1_MAP_ADDR_SPACE_BITS - TARGET_PAGE_BITS - V_L1_BITS)
> >
> > -uintptr_t qemu_host_page_size;
> > -uintptr_t qemu_host_page_mask;
> > +ram_addr_t qemu_host_page_size;
> > +ram_addr_t qemu_host_page_mask;
> >
> > /* The bottom level has pointers to PageDesc */
> > static void *l1_map[V_L1_SIZE];
> > diff --git a/translate-common.c b/translate-common.c
> > index 619feb4..2fe2d1e 100644
> > --- a/translate-common.c
> > +++ b/translate-common.c
> > @@ -20,8 +20,8 @@
> > #include "qemu-common.h"
> > #include "qom/cpu.h"
> >
> > -uintptr_t qemu_real_host_page_size;
> > -uintptr_t qemu_real_host_page_mask;
> > +ram_addr_t qemu_real_host_page_size;
> > +ram_addr_t qemu_real_host_page_mask;
> >
> > #ifndef CONFIG_USER_ONLY
> > /* mask must never be zero, except for A20 change call */
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-12-02 11:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-02 10:43 [Qemu-devel] [For 2.5?? PATCH 1/1] qemu_{real_}host_page_[size|mask] change types to ram_addr_t Dr. David Alan Gilbert (git)
2015-12-02 10:47 ` Juan Quintela
2015-12-02 10:54 ` Paolo Bonzini
2015-12-02 11:27 ` Dr. David Alan Gilbert
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.