| Message ID | 1369414987-8839-13-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
Am 24.05.2013 19:03, schrieb Paolo Bonzini: > With the next patch, the memory API will complain if the > TARGET_PHYS_ADDR_SPACE_BITS gets dangerously close to an > overflow. s390x can handle up to 64 bit of physical address > space from its page tables, but we never use that much. Just > decrease the value. > > Cc: Alexander Graf <agraf@suse.de> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Didn't Avi introduce 128-bit arithmetic into QEMU to avoid 64-bit values overflowing? Why are you limiting Memory API to 62-bit now? Andreas > --- > target-s390x/cpu.h | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h > index 0ce82cf..6304c4d 100644 > --- a/target-s390x/cpu.h > +++ b/target-s390x/cpu.h > @@ -34,7 +34,10 @@ > #include "exec/cpu-defs.h" > #define TARGET_PAGE_BITS 12 > > -#define TARGET_PHYS_ADDR_SPACE_BITS 64 > +/* Actually 64-bits, limited by the memory API to 62 bits. We > + * never use that much. > + */ > +#define TARGET_PHYS_ADDR_SPACE_BITS 62 > #define TARGET_VIRT_ADDR_SPACE_BITS 64 > > #include "exec/cpu-all.h" >
Il 26/05/2013 16:14, Andreas Färber ha scritto: > > With the next patch, the memory API will complain if the > > TARGET_PHYS_ADDR_SPACE_BITS gets dangerously close to an > > overflow. s390x can handle up to 64 bit of physical address > > space from its page tables, but we never use that much. Just > > decrease the value. > > > > Cc: Alexander Graf <agraf@suse.de> > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > Didn't Avi introduce 128-bit arithmetic into QEMU to avoid 64-bit values > overflowing? Why are you limiting Memory API to 62-bit now? The next patch makes a difference between artificial memory regions (containers and aliases) which can have arbitrary placement and width, and the final view of the address space which cannot have a full 64-bit size. 63 bits probably would work, but I preferred to be safe since 62 is the largest used by other targets. It should be fixable, but if it is not a problem I wouldn't worry much about it. Paolo
On 26/05/13 21:07, Paolo Bonzini wrote: > Il 26/05/2013 16:14, Andreas Färber ha scritto: >>> With the next patch, the memory API will complain if the >>> TARGET_PHYS_ADDR_SPACE_BITS gets dangerously close to an >>> overflow. s390x can handle up to 64 bit of physical address >>> space from its page tables, but we never use that much. Just >>> decrease the value. >>> >>> Cc: Alexander Graf <agraf@suse.de> >>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> >> Didn't Avi introduce 128-bit arithmetic into QEMU to avoid 64-bit values >> overflowing? Why are you limiting Memory API to 62-bit now? > > The next patch makes a difference between artificial memory regions > (containers and aliases) which can have arbitrary placement and width, > and the final view of the address space which cannot have a full 64-bit > size. > > 63 bits probably would work, but I preferred to be safe since 62 is the > largest used by other targets. > > It should be fixable, but if it is not a problem I wouldn't worry much > about it. I would prefer to allow 64bit of address space. Memory on s390x can be discontiguous. It is currently not used under KVM and it might not make a lot of sense, but the current KVM code would allow a guest that has a layout of lets say 0...1GB + 16EB-1GB...16EB. Furthermore, I know of some (prototype only) hw memory devices that actually populated the upper memory addresses. If such a thing becomes reality in the future we cannot provide virtualization of those. Christian
Il 26/05/2013 23:08, Christian Borntraeger ha scritto: > On 26/05/13 21:07, Paolo Bonzini wrote: >> Il 26/05/2013 16:14, Andreas Färber ha scritto: >>>> With the next patch, the memory API will complain if the >>>> TARGET_PHYS_ADDR_SPACE_BITS gets dangerously close to an >>>> overflow. s390x can handle up to 64 bit of physical address >>>> space from its page tables, but we never use that much. Just >>>> decrease the value. >>>> >>>> Cc: Alexander Graf <agraf@suse.de> >>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >>> >>> Didn't Avi introduce 128-bit arithmetic into QEMU to avoid 64-bit values >>> overflowing? Why are you limiting Memory API to 62-bit now? >> >> The next patch makes a difference between artificial memory regions >> (containers and aliases) which can have arbitrary placement and width, >> and the final view of the address space which cannot have a full 64-bit >> size. >> >> 63 bits probably would work, but I preferred to be safe since 62 is the >> largest used by other targets. >> >> It should be fixable, but if it is not a problem I wouldn't worry much >> about it. > > I would prefer to allow 64bit of address space. Memory on s390x can be > discontiguous. It is currently not used under KVM and it might not make > a lot of sense, but the current KVM code would allow a guest that has a > layout of lets say 0...1GB + 16EB-1GB...16EB. > > Furthermore, I know of some (prototype only) hw memory devices that actually > populated the upper memory addresses. If such a thing becomes reality in the > future we cannot provide virtualization of those. Ok, I'll drop this patch and the next one from the pull request. Paolo
Am 27.05.2013 09:28, schrieb Paolo Bonzini: > Il 26/05/2013 23:08, Christian Borntraeger ha scritto: >> On 26/05/13 21:07, Paolo Bonzini wrote: >>> Il 26/05/2013 16:14, Andreas Färber ha scritto: >>>>> With the next patch, the memory API will complain if the >>>>> TARGET_PHYS_ADDR_SPACE_BITS gets dangerously close to an >>>>> overflow. s390x can handle up to 64 bit of physical address >>>>> space from its page tables, but we never use that much. Just >>>>> decrease the value. >>>>> >>>>> Cc: Alexander Graf <agraf@suse.de> >>>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> [...] >> I would prefer to allow 64bit of address space. Memory on s390x can be >> discontiguous. It is currently not used under KVM and it might not make >> a lot of sense, but the current KVM code would allow a guest that has a >> layout of lets say 0...1GB + 16EB-1GB...16EB. >> >> Furthermore, I know of some (prototype only) hw memory devices that actually >> populated the upper memory addresses. If such a thing becomes reality in the >> future we cannot provide virtualization of those. > > Ok, I'll drop this patch and the next one from the pull request. It has already been merged, that's how I became aware of it: http://git.qemu.org/?p=qemu.git;a=commit;h=fd469df97ab4277411ecdd4032a2f045a3a87b2a Note that Alex is currently absent, so please CC some of the other s390x folks or wait for an ack if you plan alternative changes. Cheers, Andreas
Il 27/05/2013 14:52, Andreas Färber ha scritto: >> > Ok, I'll drop this patch and the next one from the pull request. > It has already been merged, that's how I became aware of it: > > http://git.qemu.org/?p=qemu.git;a=commit;h=fd469df97ab4277411ecdd4032a2f045a3a87b2a > > Note that Alex is currently absent, so please CC some of the other s390x > folks or wait for an ack if you plan alternative changes. I just plan to revert these two patches. The fix will be in the memory API, not in s390. Paolo
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h index 0ce82cf..6304c4d 100644 --- a/target-s390x/cpu.h +++ b/target-s390x/cpu.h @@ -34,7 +34,10 @@ #include "exec/cpu-defs.h" #define TARGET_PAGE_BITS 12 -#define TARGET_PHYS_ADDR_SPACE_BITS 64 +/* Actually 64-bits, limited by the memory API to 62 bits. We + * never use that much. + */ +#define TARGET_PHYS_ADDR_SPACE_BITS 62 #define TARGET_VIRT_ADDR_SPACE_BITS 64 #include "exec/cpu-all.h"
With the next patch, the memory API will complain if the TARGET_PHYS_ADDR_SPACE_BITS gets dangerously close to an overflow. s390x can handle up to 64 bit of physical address space from its page tables, but we never use that much. Just decrease the value. Cc: Alexander Graf <agraf@suse.de> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- target-s390x/cpu.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)