* [kvm-unit-tests PATCH v1 0/2] Fix some compilation issues on 32bit @ 2020-07-14 11:09 Claudio Imbrenda 2020-07-14 11:09 ` [kvm-unit-tests PATCH v1 1/2] x86/cstart: Fix compilation issue in 32 bit mode Claudio Imbrenda 2020-07-14 11:09 ` [kvm-unit-tests PATCH v1 2/2] lib/alloc_page: Fix compilation issue on 32bit archs Claudio Imbrenda 0 siblings, 2 replies; 7+ messages in thread From: Claudio Imbrenda @ 2020-07-14 11:09 UTC (permalink / raw) To: kvm, pbonzini Cc: frankja, thuth, david, drjones, jmattson, sean.j.christopherson Two small patches to fix compilation issues on 32bit: one for a typo in x86/cstart one for a thinko in lib/alloc_page notice that there is another patch for the lib/alloc_page issue floating around, this patch is an alternative to that one Claudio Imbrenda (2): x86/cstart: Fix compilation issue in 32 bit mode lib/alloc_page: Fix compilation issue on 32bit archs lib/alloc_page.c | 5 +++-- x86/cstart.S | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) -- 2.26.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [kvm-unit-tests PATCH v1 1/2] x86/cstart: Fix compilation issue in 32 bit mode 2020-07-14 11:09 [kvm-unit-tests PATCH v1 0/2] Fix some compilation issues on 32bit Claudio Imbrenda @ 2020-07-14 11:09 ` Claudio Imbrenda 2020-07-14 11:09 ` [kvm-unit-tests PATCH v1 2/2] lib/alloc_page: Fix compilation issue on 32bit archs Claudio Imbrenda 1 sibling, 0 replies; 7+ messages in thread From: Claudio Imbrenda @ 2020-07-14 11:09 UTC (permalink / raw) To: kvm, pbonzini Cc: frankja, thuth, david, drjones, jmattson, sean.j.christopherson Fix a typo in x86/cstart.S so that 32bit code can be compiled again on x86. Fixes: d86ef58519645 ("cstart: do not assume CR4 starts as zero") Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> --- x86/cstart.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x86/cstart.S b/x86/cstart.S index e63e4e2..c0efc5f 100644 --- a/x86/cstart.S +++ b/x86/cstart.S @@ -125,7 +125,7 @@ start: jmpl $8, $start32 prepare_32: - mov %(1 << 4), %eax // pse + mov $(1 << 4), %eax // pse mov %eax, %cr4 mov $pt, %eax -- 2.26.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [kvm-unit-tests PATCH v1 2/2] lib/alloc_page: Fix compilation issue on 32bit archs 2020-07-14 11:09 [kvm-unit-tests PATCH v1 0/2] Fix some compilation issues on 32bit Claudio Imbrenda 2020-07-14 11:09 ` [kvm-unit-tests PATCH v1 1/2] x86/cstart: Fix compilation issue in 32 bit mode Claudio Imbrenda @ 2020-07-14 11:09 ` Claudio Imbrenda 2020-07-14 11:20 ` Thomas Huth 1 sibling, 1 reply; 7+ messages in thread From: Claudio Imbrenda @ 2020-07-14 11:09 UTC (permalink / raw) To: kvm, pbonzini Cc: frankja, thuth, david, drjones, jmattson, sean.j.christopherson The assert in lib/alloc_page is hardcoded to long, and size_t is just an int on 32 bit architectures. Adding a cast makes the compiler happy. Fixes: 73f4b202beb39 ("lib/alloc_page: change some parameter types") Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> --- lib/alloc_page.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/alloc_page.c b/lib/alloc_page.c index fa3c527..617b003 100644 --- a/lib/alloc_page.c +++ b/lib/alloc_page.c @@ -29,11 +29,12 @@ void free_pages(void *mem, size_t size) assert_msg((unsigned long) mem % PAGE_SIZE == 0, "mem not page aligned: %p", mem); - assert_msg(size % PAGE_SIZE == 0, "size not page aligned: %#lx", size); + assert_msg(size % PAGE_SIZE == 0, "size not page aligned: %#lx", + (unsigned long)size); assert_msg(size == 0 || (uintptr_t)mem == -size || (uintptr_t)mem + size > (uintptr_t)mem, - "mem + size overflow: %p + %#lx", mem, size); + "mem + size overflow: %p + %#lx", mem, (unsigned long)size); if (size == 0) { freelist = NULL; -- 2.26.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [kvm-unit-tests PATCH v1 2/2] lib/alloc_page: Fix compilation issue on 32bit archs 2020-07-14 11:09 ` [kvm-unit-tests PATCH v1 2/2] lib/alloc_page: Fix compilation issue on 32bit archs Claudio Imbrenda @ 2020-07-14 11:20 ` Thomas Huth 2020-07-14 11:41 ` Claudio Imbrenda 0 siblings, 1 reply; 7+ messages in thread From: Thomas Huth @ 2020-07-14 11:20 UTC (permalink / raw) To: Claudio Imbrenda, kvm, pbonzini Cc: frankja, david, drjones, jmattson, sean.j.christopherson On 14/07/2020 13.09, Claudio Imbrenda wrote: > The assert in lib/alloc_page is hardcoded to long, and size_t is just > an int on 32 bit architectures. > > Adding a cast makes the compiler happy. > > Fixes: 73f4b202beb39 ("lib/alloc_page: change some parameter types") > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > --- > lib/alloc_page.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/lib/alloc_page.c b/lib/alloc_page.c > index fa3c527..617b003 100644 > --- a/lib/alloc_page.c > +++ b/lib/alloc_page.c > @@ -29,11 +29,12 @@ void free_pages(void *mem, size_t size) > assert_msg((unsigned long) mem % PAGE_SIZE == 0, > "mem not page aligned: %p", mem); > > - assert_msg(size % PAGE_SIZE == 0, "size not page aligned: %#lx", size); > + assert_msg(size % PAGE_SIZE == 0, "size not page aligned: %#lx", > + (unsigned long)size); > > assert_msg(size == 0 || (uintptr_t)mem == -size || > (uintptr_t)mem + size > (uintptr_t)mem, > - "mem + size overflow: %p + %#lx", mem, size); > + "mem + size overflow: %p + %#lx", mem, (unsigned long)size); Looking at lib/printf.c, it seems like it also supports %z ... have you tried? Thomas ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [kvm-unit-tests PATCH v1 2/2] lib/alloc_page: Fix compilation issue on 32bit archs 2020-07-14 11:20 ` Thomas Huth @ 2020-07-14 11:41 ` Claudio Imbrenda 2020-07-14 14:05 ` Sean Christopherson 0 siblings, 1 reply; 7+ messages in thread From: Claudio Imbrenda @ 2020-07-14 11:41 UTC (permalink / raw) To: Thomas Huth Cc: kvm, pbonzini, frankja, david, drjones, jmattson, sean.j.christopherson On Tue, 14 Jul 2020 13:20:16 +0200 Thomas Huth <thuth@redhat.com> wrote: > On 14/07/2020 13.09, Claudio Imbrenda wrote: > > The assert in lib/alloc_page is hardcoded to long, and size_t is > > just an int on 32 bit architectures. > > > > Adding a cast makes the compiler happy. > > > > Fixes: 73f4b202beb39 ("lib/alloc_page: change some parameter types") > > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > > --- > > lib/alloc_page.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/lib/alloc_page.c b/lib/alloc_page.c > > index fa3c527..617b003 100644 > > --- a/lib/alloc_page.c > > +++ b/lib/alloc_page.c > > @@ -29,11 +29,12 @@ void free_pages(void *mem, size_t size) > > assert_msg((unsigned long) mem % PAGE_SIZE == 0, > > "mem not page aligned: %p", mem); > > > > - assert_msg(size % PAGE_SIZE == 0, "size not page aligned: > > %#lx", size); > > + assert_msg(size % PAGE_SIZE == 0, "size not page aligned: > > %#lx", > > + (unsigned long)size); > > > > assert_msg(size == 0 || (uintptr_t)mem == -size || > > (uintptr_t)mem + size > (uintptr_t)mem, > > - "mem + size overflow: %p + %#lx", mem, size); > > + "mem + size overflow: %p + %#lx", mem, > > (unsigned long)size); > > Looking at lib/printf.c, it seems like it also supports %z ... have > you tried? no, but in hindsight I should have. It's probably a much cleaner solution. I'll try and respin. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [kvm-unit-tests PATCH v1 2/2] lib/alloc_page: Fix compilation issue on 32bit archs 2020-07-14 11:41 ` Claudio Imbrenda @ 2020-07-14 14:05 ` Sean Christopherson 2020-07-14 14:11 ` Claudio Imbrenda 0 siblings, 1 reply; 7+ messages in thread From: Sean Christopherson @ 2020-07-14 14:05 UTC (permalink / raw) To: Claudio Imbrenda Cc: Thomas Huth, kvm, pbonzini, frankja, david, drjones, jmattson On Tue, Jul 14, 2020 at 01:41:23PM +0200, Claudio Imbrenda wrote: > On Tue, 14 Jul 2020 13:20:16 +0200 > Thomas Huth <thuth@redhat.com> wrote: > > > On 14/07/2020 13.09, Claudio Imbrenda wrote: > > > The assert in lib/alloc_page is hardcoded to long, and size_t is > > > just an int on 32 bit architectures. > > > > > > Adding a cast makes the compiler happy. > > > > > > Fixes: 73f4b202beb39 ("lib/alloc_page: change some parameter types") > > > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > > > --- > > > lib/alloc_page.c | 5 +++-- > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > diff --git a/lib/alloc_page.c b/lib/alloc_page.c > > > index fa3c527..617b003 100644 > > > --- a/lib/alloc_page.c > > > +++ b/lib/alloc_page.c > > > @@ -29,11 +29,12 @@ void free_pages(void *mem, size_t size) > > > assert_msg((unsigned long) mem % PAGE_SIZE == 0, > > > "mem not page aligned: %p", mem); > > > > > > - assert_msg(size % PAGE_SIZE == 0, "size not page aligned: > > > %#lx", size); > > > + assert_msg(size % PAGE_SIZE == 0, "size not page aligned: > > > %#lx", > > > + (unsigned long)size); > > > > > > assert_msg(size == 0 || (uintptr_t)mem == -size || > > > (uintptr_t)mem + size > (uintptr_t)mem, > > > - "mem + size overflow: %p + %#lx", mem, size); > > > + "mem + size overflow: %p + %#lx", mem, > > > (unsigned long)size); > > > > Looking at lib/printf.c, it seems like it also supports %z ... have > > you tried? > > no, but in hindsight I should have. It's probably a much cleaner > solution. I'll try and respin. I'm not opposed to using size_t, but if we go that route then the entirety of alloc_page.c should be converted to size_t. As is, there is code like: void free_pages_by_order(void *mem, unsigned int order) { free_pages(mem, 1ul << (order + PAGE_SHIFT)); } and void *alloc_pages(unsigned int order) { ... /* Looking for a run of length (1 << order). */ unsigned long run = 0; const unsigned long n = 1ul << order; const unsigned long align_mask = (n << PAGE_SHIFT) - 1; void *run_start = NULL; void *run_prev = NULL; unsigned long run_next_pa = 0; unsigned long pa; assert(order < sizeof(unsigned long) * 8); ... } that very explicitly uses 'unsigned long' for the size. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [kvm-unit-tests PATCH v1 2/2] lib/alloc_page: Fix compilation issue on 32bit archs 2020-07-14 14:05 ` Sean Christopherson @ 2020-07-14 14:11 ` Claudio Imbrenda 0 siblings, 0 replies; 7+ messages in thread From: Claudio Imbrenda @ 2020-07-14 14:11 UTC (permalink / raw) To: Sean Christopherson Cc: Thomas Huth, kvm, pbonzini, frankja, david, drjones, jmattson On Tue, 14 Jul 2020 07:05:34 -0700 Sean Christopherson <sean.j.christopherson@intel.com> wrote: > On Tue, Jul 14, 2020 at 01:41:23PM +0200, Claudio Imbrenda wrote: > > On Tue, 14 Jul 2020 13:20:16 +0200 > > Thomas Huth <thuth@redhat.com> wrote: > > > > > On 14/07/2020 13.09, Claudio Imbrenda wrote: > > > > The assert in lib/alloc_page is hardcoded to long, and size_t is > > > > just an int on 32 bit architectures. > > > > > > > > Adding a cast makes the compiler happy. > > > > > > > > Fixes: 73f4b202beb39 ("lib/alloc_page: change some parameter > > > > types") Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > > > > --- > > > > lib/alloc_page.c | 5 +++-- > > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/lib/alloc_page.c b/lib/alloc_page.c > > > > index fa3c527..617b003 100644 > > > > --- a/lib/alloc_page.c > > > > +++ b/lib/alloc_page.c > > > > @@ -29,11 +29,12 @@ void free_pages(void *mem, size_t size) > > > > assert_msg((unsigned long) mem % PAGE_SIZE == 0, > > > > "mem not page aligned: %p", mem); > > > > > > > > - assert_msg(size % PAGE_SIZE == 0, "size not page > > > > aligned: %#lx", size); > > > > + assert_msg(size % PAGE_SIZE == 0, "size not page > > > > aligned: %#lx", > > > > + (unsigned long)size); > > > > > > > > assert_msg(size == 0 || (uintptr_t)mem == -size || > > > > (uintptr_t)mem + size > (uintptr_t)mem, > > > > - "mem + size overflow: %p + %#lx", mem, > > > > size); > > > > + "mem + size overflow: %p + %#lx", mem, > > > > (unsigned long)size); > > > > > > Looking at lib/printf.c, it seems like it also supports %z ... > > > have you tried? > > > > no, but in hindsight I should have. It's probably a much cleaner > > solution. I'll try and respin. > > I'm not opposed to using size_t, but if we go that route then the > entirety of alloc_page.c should be converted to size_t. As is, there > is code like: > > void free_pages_by_order(void *mem, unsigned int order) > { > free_pages(mem, 1ul << (order + PAGE_SHIFT)); > } > > and > > void *alloc_pages(unsigned int order) > { > ... > > /* Looking for a run of length (1 << order). */ > unsigned long run = 0; > const unsigned long n = 1ul << order; > const unsigned long align_mask = (n << PAGE_SHIFT) - > 1; void *run_start = NULL; > void *run_prev = NULL; > unsigned long run_next_pa = 0; > unsigned long pa; > > assert(order < sizeof(unsigned long) * 8); > > ... > } > > that very explicitly uses 'unsigned long' for the size. don't worry, those won't stay there for long :) once this patch series has stabilized, I'm going to send a more radical rewrite of the allocators ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-07-14 14:11 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-07-14 11:09 [kvm-unit-tests PATCH v1 0/2] Fix some compilation issues on 32bit Claudio Imbrenda 2020-07-14 11:09 ` [kvm-unit-tests PATCH v1 1/2] x86/cstart: Fix compilation issue in 32 bit mode Claudio Imbrenda 2020-07-14 11:09 ` [kvm-unit-tests PATCH v1 2/2] lib/alloc_page: Fix compilation issue on 32bit archs Claudio Imbrenda 2020-07-14 11:20 ` Thomas Huth 2020-07-14 11:41 ` Claudio Imbrenda 2020-07-14 14:05 ` Sean Christopherson 2020-07-14 14:11 ` Claudio Imbrenda
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox