| Message ID | 4EDD8E4D.5000309@ozlabs.org |
|---|---|
| State | New, archived |
| Headers | show |
Ingo actually got us to remove all the PRI* specifiers, but that was back when we only did x86 :) Ingo, does it make sense to use them now when we support different architectures? On Tue, 2011-12-06 at 14:38 +1100, Matt Evans wrote: > On LP64 systems our u64s are just longs; remove the %llx'es in favour of PRIx64 > etc. > > This patch also adds CFLAGS to the final link, so that any -m64 is obeyed when > linking, too. > > Signed-off-by: Matt Evans <matt@ozlabs.org> > --- > tools/kvm/Makefile | 2 +- > tools/kvm/builtin-run.c | 14 ++++++++------ > tools/kvm/builtin-stat.c | 4 +++- > tools/kvm/disk/core.c | 4 +++- > tools/kvm/mmio.c | 4 +++- > 5 files changed, 18 insertions(+), 10 deletions(-) > > diff --git a/tools/kvm/Makefile b/tools/kvm/Makefile > index 009a6ba..57dc521 100644 > --- a/tools/kvm/Makefile > +++ b/tools/kvm/Makefile > @@ -218,7 +218,7 @@ KVMTOOLS-VERSION-FILE: > > $(PROGRAM): $(DEPS) $(OBJS) > $(E) " LINK " $@ > - $(Q) $(CC) $(OBJS) $(LIBS) -o $@ > + $(Q) $(CC) $(CFLAGS) $(OBJS) $(LIBS) -o $@ > > $(GUEST_INIT): guest/init.c > $(E) " LINK " $@ > diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c > index e4aa87e..7cf208d 100644 > --- a/tools/kvm/builtin-run.c > +++ b/tools/kvm/builtin-run.c > @@ -42,6 +42,8 @@ > #include <stdlib.h> > #include <string.h> > #include <unistd.h> > +#define __STDC_FORMAT_MACROS > +#include <inttypes.h> > #include <ctype.h> > #include <stdio.h> > > @@ -383,8 +385,8 @@ static int shmem_parser(const struct option *opt, const char *arg, int unset) > strcpy(handle, default_handle); > } > if (verbose) { > - pr_info("shmem: phys_addr = %llx", phys_addr); > - pr_info("shmem: size = %llx", size); > + pr_info("shmem: phys_addr = %"PRIx64, phys_addr); > + pr_info("shmem: size = %"PRIx64, size); > pr_info("shmem: handle = %s", handle); > pr_info("shmem: create = %d", create); > } > @@ -545,7 +547,7 @@ panic_kvm: > current_kvm_cpu->kvm_run->exit_reason, > kvm_exit_reasons[current_kvm_cpu->kvm_run->exit_reason]); > if (current_kvm_cpu->kvm_run->exit_reason == KVM_EXIT_UNKNOWN) > - fprintf(stderr, "KVM exit code: 0x%Lu\n", > + fprintf(stderr, "KVM exit code: 0x%"PRIx64"\n", > current_kvm_cpu->kvm_run->hw.hardware_exit_reason); > > kvm_cpu__set_debug_fd(STDOUT_FILENO); > @@ -760,10 +762,10 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix) > ram_size = get_ram_size(nrcpus); > > if (ram_size < MIN_RAM_SIZE_MB) > - die("Not enough memory specified: %lluMB (min %lluMB)", ram_size, MIN_RAM_SIZE_MB); > + die("Not enough memory specified: %"PRIu64"MB (min %lluMB)", ram_size, MIN_RAM_SIZE_MB); > > if (ram_size > host_ram_size()) > - pr_warning("Guest memory size %lluMB exceeds host physical RAM size %lluMB", ram_size, host_ram_size()); > + pr_warning("Guest memory size %"PRIu64"MB exceeds host physical RAM size %"PRIu64"MB", ram_size, host_ram_size()); > > ram_size <<= MB_SHIFT; > > @@ -878,7 +880,7 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix) > virtio_blk__init_all(kvm); > } > > - printf(" # kvm run -k %s -m %Lu -c %d --name %s\n", kernel_filename, ram_size / 1024 / 1024, nrcpus, guest_name); > + printf(" # kvm run -k %s -m %"PRId64" -c %d --name %s\n", kernel_filename, ram_size / 1024 / 1024, nrcpus, guest_name); > > if (!kvm__load_kernel(kvm, kernel_filename, initrd_filename, > real_cmdline, vidmode)) > diff --git a/tools/kvm/builtin-stat.c b/tools/kvm/builtin-stat.c > index e28eb5b..c1f2605 100644 > --- a/tools/kvm/builtin-stat.c > +++ b/tools/kvm/builtin-stat.c > @@ -9,6 +9,8 @@ > #include <stdio.h> > #include <string.h> > #include <signal.h> > +#define __STDC_FORMAT_MACROS > +#include <inttypes.h> > > #include <linux/virtio_balloon.h> > > @@ -97,7 +99,7 @@ static int do_memstat(const char *name, int sock) > printf("The total amount of memory available (in bytes):"); > break; > } > - printf("%llu\n", stats[i].val); > + printf("%"PRId64"\n", stats[i].val); > } > printf("\n"); > > diff --git a/tools/kvm/disk/core.c b/tools/kvm/disk/core.c > index 4915efd..a135851 100644 > --- a/tools/kvm/disk/core.c > +++ b/tools/kvm/disk/core.c > @@ -4,6 +4,8 @@ > > #include <sys/eventfd.h> > #include <sys/poll.h> > +#define __STDC_FORMAT_MACROS > +#include <inttypes.h> > > #define AIO_MAX 32 > > @@ -232,7 +234,7 @@ ssize_t disk_image__get_serial(struct disk_image *disk, void *buffer, ssize_t *l > if (fstat(disk->fd, &st) != 0) > return 0; > > - *len = snprintf(buffer, *len, "%llu%llu%llu", (u64)st.st_dev, (u64)st.st_rdev, (u64)st.st_ino); > + *len = snprintf(buffer, *len, "%"PRId64"%"PRId64"%"PRId64, (u64)st.st_dev, (u64)st.st_rdev, (u64)st.st_ino); > return *len; > } > > diff --git a/tools/kvm/mmio.c b/tools/kvm/mmio.c > index de7320f..1158bff 100644 > --- a/tools/kvm/mmio.c > +++ b/tools/kvm/mmio.c > @@ -9,6 +9,8 @@ > #include <linux/kvm.h> > #include <linux/types.h> > #include <linux/rbtree.h> > +#define __STDC_FORMAT_MACROS > +#include <inttypes.h> > > #define mmio_node(n) rb_entry(n, struct mmio_mapping, node) > > @@ -124,7 +126,7 @@ bool kvm__emulate_mmio(struct kvm *kvm, u64 phys_addr, u8 *data, u32 len, u8 is_ > if (mmio) > mmio->mmio_fn(phys_addr, data, len, is_write, mmio->ptr); > else > - fprintf(stderr, "Warning: Ignoring MMIO %s at %016llx (length %u)\n", > + fprintf(stderr, "Warning: Ignoring MMIO %s at %016"PRIx64" (length %u)\n", > to_direction(is_write), phys_addr, len); > br_read_unlock(); > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
* Sasha Levin <levinsasha928@gmail.com> wrote: > Ingo actually got us to remove all the PRI* specifiers, but > that was back when we only did x86 :) > > Ingo, does it make sense to use them now when we support > different architectures? Not at all - ppc should use a sane u64/s64 definition, i.e. int-ll64.h instead of the int-l64.h crap. The powerpc maintainers indicated that they'd fix that, a couple of years ago, but nothing appears to have come out of that. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 06, 2011 at 09:28:27AM +0100, Ingo Molnar wrote: > > * Sasha Levin <levinsasha928@gmail.com> wrote: > > > Ingo actually got us to remove all the PRI* specifiers, but > > that was back when we only did x86 :) > > > > Ingo, does it make sense to use them now when we support > > different architectures? > > Not at all - ppc should use a sane u64/s64 definition, i.e. > int-ll64.h instead of the int-l64.h crap. > > The powerpc maintainers indicated that they'd fix that, a couple > of years ago, but nothing appears to have come out of that. We changed it for the kernel, but not for userspace (i.e. glibc) because of concerns about possibly breaking existing userspace programs. I haven't looked closely at Matt's patches, but it should be possible to use [un]signed long long for the u64/s64 types, I would think. Paul. -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Paul Mackerras <paulus@samba.org> wrote: > On Tue, Dec 06, 2011 at 09:28:27AM +0100, Ingo Molnar wrote: > > > > * Sasha Levin <levinsasha928@gmail.com> wrote: > > > > > Ingo actually got us to remove all the PRI* specifiers, but > > > that was back when we only did x86 :) > > > > > > Ingo, does it make sense to use them now when we support > > > different architectures? > > > > Not at all - ppc should use a sane u64/s64 definition, i.e. > > int-ll64.h instead of the int-l64.h crap. > > > > The powerpc maintainers indicated that they'd fix that, a couple > > of years ago, but nothing appears to have come out of that. > > We changed it for the kernel, but not for userspace (i.e. > glibc) because of concerns about possibly breaking existing > userspace programs. [...] Indeed, you do: #if defined(__powerpc64__) && !defined(__KERNEL__) # include <asm-generic/int-l64.h> #else # include <asm-generic/int-ll64.h> #endif which correctly uses int-ll64.h everywhere except 64-bit userspace inclusions. So i take back my 'nothing appears to have come out of that' comment - it's all nicely fixed! > [...] I haven't looked closely at Matt's > patches, but it should be possible to use [un]signed long long > for the u64/s64 types, I would think. In tools/kvm/ we are using our own u64/s64 definitions, not glibc's, so i think it should be fine - as long as we don't pick up int-l64.h accidentally via the arch/powerpc/include/asm/types.h exception for user-space. Stray uses of int64 should be converted to u64 (i see some in tools/kvm/virtio/9p-pdu.c) but otherwise we should be ok - unless i'm missing some complication. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ingo, On 06/12/11 21:24, Ingo Molnar wrote: > > * Paul Mackerras <paulus@samba.org> wrote: > >> On Tue, Dec 06, 2011 at 09:28:27AM +0100, Ingo Molnar wrote: >>> >>> * Sasha Levin <levinsasha928@gmail.com> wrote: >>> >>>> Ingo actually got us to remove all the PRI* specifiers, but >>>> that was back when we only did x86 :) >>>> >>>> Ingo, does it make sense to use them now when we support >>>> different architectures? >>> >>> Not at all - ppc should use a sane u64/s64 definition, i.e. >>> int-ll64.h instead of the int-l64.h crap. >>> >>> The powerpc maintainers indicated that they'd fix that, a couple >>> of years ago, but nothing appears to have come out of that. >> >> We changed it for the kernel, but not for userspace (i.e. >> glibc) because of concerns about possibly breaking existing >> userspace programs. [...] > > Indeed, you do: > > #if defined(__powerpc64__) && !defined(__KERNEL__) > # include <asm-generic/int-l64.h> > #else > # include <asm-generic/int-ll64.h> > #endif > > which correctly uses int-ll64.h everywhere except 64-bit > userspace inclusions. So i take back my 'nothing appears to have > come out of that' comment - it's all nicely fixed! > >> [...] I haven't looked closely at Matt's >> patches, but it should be possible to use [un]signed long long >> for the u64/s64 types, I would think. > > In tools/kvm/ we are using our own u64/s64 definitions, not > glibc's, so i think it should be fine - as long as we don't pick > up int-l64.h accidentally via the > arch/powerpc/include/asm/types.h exception for user-space. That's what's happening here; we're __powerpc64__ and !__KERNEL__, tools/kvm/include/linux/types.h includes asm/types.h so gets the int-l64.h definition of __u64, as above. :/ builtin-run.c:389: error: format `%llx' expects type `long long unsigned int', but argument 2 has type `u64' etc. etc. Cheers, Matt -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Matt Evans <matt@ozlabs.org> wrote: > >> [...] I haven't looked closely at Matt's > >> patches, but it should be possible to use [un]signed long long > >> for the u64/s64 types, I would think. > > > > In tools/kvm/ we are using our own u64/s64 definitions, not > > glibc's, so i think it should be fine - as long as we don't pick > > up int-l64.h accidentally via the > > arch/powerpc/include/asm/types.h exception for user-space. > > That's what's happening here; we're __powerpc64__ and > !__KERNEL__, tools/kvm/include/linux/types.h includes > asm/types.h so gets the int-l64.h definition of __u64, as > above. :/ > > builtin-run.c:389: error: format `%llx' expects type `long > long unsigned int', but argument 2 has type `u64' So either define __KERNEL__ or patch a __NEW_USERSPACE__ define into power/asm/types.h and use it - if the PowerPC folks agree with that approach. Sane userspace should not be prevented from using the same sane types the kernel is already using :-) Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/07/2011 09:16 AM, Ingo Molnar wrote: >> > That's what's happening here; we're __powerpc64__ and >> > !__KERNEL__, tools/kvm/include/linux/types.h includes >> > asm/types.h so gets the int-l64.h definition of __u64, as >> > above. :/ >> > >> > builtin-run.c:389: error: format `%llx' expects type `long >> > long unsigned int', but argument 2 has type `u64' > So either define __KERNEL__ or patch a __NEW_USERSPACE__ define > into power/asm/types.h and use it - if the PowerPC folks agree > with that approach. > > Sane userspace should not be prevented from using the same sane > types the kernel is already using:-) I should dig up that patent of mine for "apparatus for conversion of circular motion to rectilinear ('wheel')". Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 7 Dec 2011, Ingo Molnar wrote: > > * Matt Evans <matt@ozlabs.org> wrote: > >>>> [...] I haven't looked closely at Matt's >>>> patches, but it should be possible to use [un]signed long long >>>> for the u64/s64 types, I would think. >>> >>> In tools/kvm/ we are using our own u64/s64 definitions, not >>> glibc's, so i think it should be fine - as long as we don't pick >>> up int-l64.h accidentally via the >>> arch/powerpc/include/asm/types.h exception for user-space. >> >> That's what's happening here; we're __powerpc64__ and >> !__KERNEL__, tools/kvm/include/linux/types.h includes >> asm/types.h so gets the int-l64.h definition of __u64, as >> above. :/ >> >> builtin-run.c:389: error: format `%llx' expects type `long >> long unsigned int', but argument 2 has type `u64' > > So either define __KERNEL__ or patch a __NEW_USERSPACE__ define > into power/asm/types.h and use it - if the PowerPC folks agree > with that approach. > > Sane userspace should not be prevented from using the same sane > types the kernel is already using :-) How does perf handle this? I'm sure it has the exact same issue, doesn't it? Pekka -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/07/2011 09:16 AM, Ingo Molnar wrote: >>> > That's what's happening here; we're __powerpc64__ and >>> > !__KERNEL__, tools/kvm/include/linux/types.h includes >>> > asm/types.h so gets the int-l64.h definition of __u64, as >>> > above. :/ >>> > >>> > builtin-run.c:389: error: format `%llx' expects type `long >>> > long unsigned int', but argument 2 has type `u64' >> So either define __KERNEL__ or patch a __NEW_USERSPACE__ define >> into power/asm/types.h and use it - if the PowerPC folks agree >> with that approach. >> >> Sane userspace should not be prevented from using the same sane >> types the kernel is already using:-) On Wed, 7 Dec 2011, Paolo Bonzini wrote: > I should dig up that patent of mine for "apparatus for conversion of circular > motion to rectilinear ('wheel')". What's your point? We use kernel types for obvious reasons and it trying to use <stdint.h> for the PPC port is just begging for trouble. Pekka -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/12/11 04:14, Pekka Enberg wrote: > On Wed, 7 Dec 2011, Ingo Molnar wrote: > >> >> * Matt Evans <matt@ozlabs.org> wrote: >> >>>>> [...] I haven't looked closely at Matt's >>>>> patches, but it should be possible to use [un]signed long long >>>>> for the u64/s64 types, I would think. >>>> >>>> In tools/kvm/ we are using our own u64/s64 definitions, not >>>> glibc's, so i think it should be fine - as long as we don't pick >>>> up int-l64.h accidentally via the >>>> arch/powerpc/include/asm/types.h exception for user-space. >>> >>> That's what's happening here; we're __powerpc64__ and >>> !__KERNEL__, tools/kvm/include/linux/types.h includes >>> asm/types.h so gets the int-l64.h definition of __u64, as >>> above. :/ >>> >>> builtin-run.c:389: error: format `%llx' expects type `long >>> long unsigned int', but argument 2 has type `u64' >> >> So either define __KERNEL__ or patch a __NEW_USERSPACE__ define >> into power/asm/types.h and use it - if the PowerPC folks agree >> with that approach. >> >> Sane userspace should not be prevented from using the same sane >> types the kernel is already using :-) > > How does perf handle this? I'm sure it has the exact same issue, doesn't > it? It does; ironically it uses PRIblah, so I had followed its example. Cheers, Matt -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Matt Evans <matt@ozlabs.org> wrote: > On 08/12/11 04:14, Pekka Enberg wrote: > > On Wed, 7 Dec 2011, Ingo Molnar wrote: > > > >> > >> * Matt Evans <matt@ozlabs.org> wrote: > >> > >>>>> [...] I haven't looked closely at Matt's > >>>>> patches, but it should be possible to use [un]signed long long > >>>>> for the u64/s64 types, I would think. > >>>> > >>>> In tools/kvm/ we are using our own u64/s64 definitions, not > >>>> glibc's, so i think it should be fine - as long as we don't pick > >>>> up int-l64.h accidentally via the > >>>> arch/powerpc/include/asm/types.h exception for user-space. > >>> > >>> That's what's happening here; we're __powerpc64__ and > >>> !__KERNEL__, tools/kvm/include/linux/types.h includes > >>> asm/types.h so gets the int-l64.h definition of __u64, as > >>> above. :/ > >>> > >>> builtin-run.c:389: error: format `%llx' expects type `long > >>> long unsigned int', but argument 2 has type `u64' > >> > >> So either define __KERNEL__ or patch a __NEW_USERSPACE__ define > >> into power/asm/types.h and use it - if the PowerPC folks agree > >> with that approach. > >> > >> Sane userspace should not be prevented from using the same sane > >> types the kernel is already using :-) > > > > How does perf handle this? I'm sure it has the exact same > > issue, doesn't it? > > It does; ironically it uses PRIblah, so I had followed its > example. Sadly it regressed lately in that area - it was certainly PRIblah-less in the early stages :-) Pekka, how do the headers react if we define __KERNEL__? Alternatively, allowing an override in powerpc/types.h beyond __KERNEL__ looks sensible as well. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/12/11 15:49, Ingo Molnar wrote: > > * Matt Evans <matt@ozlabs.org> wrote: > >> On 08/12/11 04:14, Pekka Enberg wrote: >>> On Wed, 7 Dec 2011, Ingo Molnar wrote: >>> >>>> >>>> * Matt Evans <matt@ozlabs.org> wrote: >>>> >>>>>>> [...] I haven't looked closely at Matt's >>>>>>> patches, but it should be possible to use [un]signed long long >>>>>>> for the u64/s64 types, I would think. >>>>>> >>>>>> In tools/kvm/ we are using our own u64/s64 definitions, not >>>>>> glibc's, so i think it should be fine - as long as we don't pick >>>>>> up int-l64.h accidentally via the >>>>>> arch/powerpc/include/asm/types.h exception for user-space. >>>>> >>>>> That's what's happening here; we're __powerpc64__ and >>>>> !__KERNEL__, tools/kvm/include/linux/types.h includes >>>>> asm/types.h so gets the int-l64.h definition of __u64, as >>>>> above. :/ >>>>> >>>>> builtin-run.c:389: error: format `%llx' expects type `long >>>>> long unsigned int', but argument 2 has type `u64' >>>> >>>> So either define __KERNEL__ or patch a __NEW_USERSPACE__ define >>>> into power/asm/types.h and use it - if the PowerPC folks agree >>>> with that approach. >>>> >>>> Sane userspace should not be prevented from using the same sane >>>> types the kernel is already using :-) >>> >>> How does perf handle this? I'm sure it has the exact same >>> issue, doesn't it? >> >> It does; ironically it uses PRIblah, so I had followed its >> example. > > Sadly it regressed lately in that area - it was certainly > PRIblah-less in the early stages :-) Oh dear :-) > Pekka, how do the headers react if we define __KERNEL__? > Alternatively, allowing an override in powerpc/types.h beyond > __KERNEL__ looks sensible as well. Well, I just tried it and it ended in tears; various things bring in tons more (ppc) stuff from asm/, quite a few conflicts. I've resorted to defining __KERNEL__ in linux/types.h *only* around #include <asm/types.h> (i.e. undefining it afterwards). This picks up the correct int-ll64.h on PPC64 and doesn't break everything else. Cheers, Matt -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/12/11 15:56, Matt Evans wrote: > On 08/12/11 15:49, Ingo Molnar wrote: >> >> * Matt Evans <matt@ozlabs.org> wrote: >> >>> On 08/12/11 04:14, Pekka Enberg wrote: >>>> On Wed, 7 Dec 2011, Ingo Molnar wrote: >>>> >>>>> >>>>> * Matt Evans <matt@ozlabs.org> wrote: >>>>> >>>>>>>> [...] I haven't looked closely at Matt's >>>>>>>> patches, but it should be possible to use [un]signed long long >>>>>>>> for the u64/s64 types, I would think. >>>>>>> >>>>>>> In tools/kvm/ we are using our own u64/s64 definitions, not >>>>>>> glibc's, so i think it should be fine - as long as we don't pick >>>>>>> up int-l64.h accidentally via the >>>>>>> arch/powerpc/include/asm/types.h exception for user-space. >>>>>> >>>>>> That's what's happening here; we're __powerpc64__ and >>>>>> !__KERNEL__, tools/kvm/include/linux/types.h includes >>>>>> asm/types.h so gets the int-l64.h definition of __u64, as >>>>>> above. :/ >>>>>> >>>>>> builtin-run.c:389: error: format `%llx' expects type `long >>>>>> long unsigned int', but argument 2 has type `u64' >>>>> >>>>> So either define __KERNEL__ or patch a __NEW_USERSPACE__ define >>>>> into power/asm/types.h and use it - if the PowerPC folks agree >>>>> with that approach. >>>>> >>>>> Sane userspace should not be prevented from using the same sane >>>>> types the kernel is already using :-) >>>> >>>> How does perf handle this? I'm sure it has the exact same >>>> issue, doesn't it? >>> >>> It does; ironically it uses PRIblah, so I had followed its >>> example. >> >> Sadly it regressed lately in that area - it was certainly >> PRIblah-less in the early stages :-) > > Oh dear :-) > >> Pekka, how do the headers react if we define __KERNEL__? >> Alternatively, allowing an override in powerpc/types.h beyond >> __KERNEL__ looks sensible as well. > > Well, I just tried it and it ended in tears; various things bring in tons more > (ppc) stuff from asm/, quite a few conflicts. > > I've resorted to defining __KERNEL__ in linux/types.h *only* around #include > <asm/types.h> (i.e. undefining it afterwards). This picks up the correct > int-ll64.h on PPC64 and doesn't break everything else. I spoke too soon; this screws x86 (who?), in that BITS_PER_LONG's redefined as ../../include/asm-generic/bitsperlong.h now kicks in if __KERNEL__'s defined. Defining __KERNEL__ feels a bit nasty, esp. considering these knock-on effects. Since tools/kvm/include/linux/types.h only requires __u32, __u64 et al from <asm/types.h>, wouldn't it be most straightforward to just #include <asm-generic/int-ll64.h>? This avoids #define __KERNEL__ breaking other includes brought into userland, avoids changing system headers/distros, and includes the file we're really interested in on both x86 & PPC. Cheers, Matt -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Matt Evans <matt@ozlabs.org> wrote: > Since tools/kvm/include/linux/types.h only requires __u32, > __u64 et al from <asm/types.h>, wouldn't it be most > straightforward to just #include <asm-generic/int-ll64.h>? > This avoids #define __KERNEL__ breaking other includes brought > into userland, avoids changing system headers/distros, and > includes the file we're really interested in on both x86 & > PPC. No system headers need to be changed: you can just patch powerpc/types.h with the extra __SANE_USERSPACE__ define (or any suitable name) and the KVM tool should work as-is. The beauty of tools/kvm/ being integrated into the kernel tree. Really, if that works and if the PowerPC folks agree then we have an immediate, fully adequate, instantly applicable solution. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/12/11 16:49, Ingo Molnar wrote: > > * Matt Evans <matt@ozlabs.org> wrote: > >> Since tools/kvm/include/linux/types.h only requires __u32, >> __u64 et al from <asm/types.h>, wouldn't it be most >> straightforward to just #include <asm-generic/int-ll64.h>? >> This avoids #define __KERNEL__ breaking other includes brought >> into userland, avoids changing system headers/distros, and >> includes the file we're really interested in on both x86 & >> PPC. > > No system headers need to be changed: you can just patch > powerpc/types.h with the extra __SANE_USERSPACE__ define (or any > suitable name) and the KVM tool should work as-is. > > The beauty of tools/kvm/ being integrated into the kernel tree. > > Really, if that works and if the PowerPC folks agree then we > have an immediate, fully adequate, instantly applicable > solution. Ahh, I finally understand you-- I'd earlier thought you were referring to the system headers' asm/types.h, etc. Thanks for your patience on this very trivial point :-) I'll add __SANE_USERSPACE_TYPES__ and we'll see how the others like it. >:) (Paul, that sound OK?) Thanks, Matt -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/tools/kvm/Makefile b/tools/kvm/Makefile index 009a6ba..57dc521 100644 --- a/tools/kvm/Makefile +++ b/tools/kvm/Makefile @@ -218,7 +218,7 @@ KVMTOOLS-VERSION-FILE: $(PROGRAM): $(DEPS) $(OBJS) $(E) " LINK " $@ - $(Q) $(CC) $(OBJS) $(LIBS) -o $@ + $(Q) $(CC) $(CFLAGS) $(OBJS) $(LIBS) -o $@ $(GUEST_INIT): guest/init.c $(E) " LINK " $@ diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c index e4aa87e..7cf208d 100644 --- a/tools/kvm/builtin-run.c +++ b/tools/kvm/builtin-run.c @@ -42,6 +42,8 @@ #include <stdlib.h> #include <string.h> #include <unistd.h> +#define __STDC_FORMAT_MACROS +#include <inttypes.h> #include <ctype.h> #include <stdio.h> @@ -383,8 +385,8 @@ static int shmem_parser(const struct option *opt, const char *arg, int unset) strcpy(handle, default_handle); } if (verbose) { - pr_info("shmem: phys_addr = %llx", phys_addr); - pr_info("shmem: size = %llx", size); + pr_info("shmem: phys_addr = %"PRIx64, phys_addr); + pr_info("shmem: size = %"PRIx64, size); pr_info("shmem: handle = %s", handle); pr_info("shmem: create = %d", create); } @@ -545,7 +547,7 @@ panic_kvm: current_kvm_cpu->kvm_run->exit_reason, kvm_exit_reasons[current_kvm_cpu->kvm_run->exit_reason]); if (current_kvm_cpu->kvm_run->exit_reason == KVM_EXIT_UNKNOWN) - fprintf(stderr, "KVM exit code: 0x%Lu\n", + fprintf(stderr, "KVM exit code: 0x%"PRIx64"\n", current_kvm_cpu->kvm_run->hw.hardware_exit_reason); kvm_cpu__set_debug_fd(STDOUT_FILENO); @@ -760,10 +762,10 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix) ram_size = get_ram_size(nrcpus); if (ram_size < MIN_RAM_SIZE_MB) - die("Not enough memory specified: %lluMB (min %lluMB)", ram_size, MIN_RAM_SIZE_MB); + die("Not enough memory specified: %"PRIu64"MB (min %lluMB)", ram_size, MIN_RAM_SIZE_MB); if (ram_size > host_ram_size()) - pr_warning("Guest memory size %lluMB exceeds host physical RAM size %lluMB", ram_size, host_ram_size()); + pr_warning("Guest memory size %"PRIu64"MB exceeds host physical RAM size %"PRIu64"MB", ram_size, host_ram_size()); ram_size <<= MB_SHIFT; @@ -878,7 +880,7 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix) virtio_blk__init_all(kvm); } - printf(" # kvm run -k %s -m %Lu -c %d --name %s\n", kernel_filename, ram_size / 1024 / 1024, nrcpus, guest_name); + printf(" # kvm run -k %s -m %"PRId64" -c %d --name %s\n", kernel_filename, ram_size / 1024 / 1024, nrcpus, guest_name); if (!kvm__load_kernel(kvm, kernel_filename, initrd_filename, real_cmdline, vidmode)) diff --git a/tools/kvm/builtin-stat.c b/tools/kvm/builtin-stat.c index e28eb5b..c1f2605 100644 --- a/tools/kvm/builtin-stat.c +++ b/tools/kvm/builtin-stat.c @@ -9,6 +9,8 @@ #include <stdio.h> #include <string.h> #include <signal.h> +#define __STDC_FORMAT_MACROS +#include <inttypes.h> #include <linux/virtio_balloon.h> @@ -97,7 +99,7 @@ static int do_memstat(const char *name, int sock) printf("The total amount of memory available (in bytes):"); break; } - printf("%llu\n", stats[i].val); + printf("%"PRId64"\n", stats[i].val); } printf("\n"); diff --git a/tools/kvm/disk/core.c b/tools/kvm/disk/core.c index 4915efd..a135851 100644 --- a/tools/kvm/disk/core.c +++ b/tools/kvm/disk/core.c @@ -4,6 +4,8 @@ #include <sys/eventfd.h> #include <sys/poll.h> +#define __STDC_FORMAT_MACROS +#include <inttypes.h> #define AIO_MAX 32 @@ -232,7 +234,7 @@ ssize_t disk_image__get_serial(struct disk_image *disk, void *buffer, ssize_t *l if (fstat(disk->fd, &st) != 0) return 0; - *len = snprintf(buffer, *len, "%llu%llu%llu", (u64)st.st_dev, (u64)st.st_rdev, (u64)st.st_ino); + *len = snprintf(buffer, *len, "%"PRId64"%"PRId64"%"PRId64, (u64)st.st_dev, (u64)st.st_rdev, (u64)st.st_ino); return *len; } diff --git a/tools/kvm/mmio.c b/tools/kvm/mmio.c index de7320f..1158bff 100644 --- a/tools/kvm/mmio.c +++ b/tools/kvm/mmio.c @@ -9,6 +9,8 @@ #include <linux/kvm.h> #include <linux/types.h> #include <linux/rbtree.h> +#define __STDC_FORMAT_MACROS +#include <inttypes.h> #define mmio_node(n) rb_entry(n, struct mmio_mapping, node) @@ -124,7 +126,7 @@ bool kvm__emulate_mmio(struct kvm *kvm, u64 phys_addr, u8 *data, u32 len, u8 is_ if (mmio) mmio->mmio_fn(phys_addr, data, len, is_write, mmio->ptr); else - fprintf(stderr, "Warning: Ignoring MMIO %s at %016llx (length %u)\n", + fprintf(stderr, "Warning: Ignoring MMIO %s at %016"PRIx64" (length %u)\n", to_direction(is_write), phys_addr, len); br_read_unlock();
On LP64 systems our u64s are just longs; remove the %llx'es in favour of PRIx64 etc. This patch also adds CFLAGS to the final link, so that any -m64 is obeyed when linking, too. Signed-off-by: Matt Evans <matt@ozlabs.org> --- tools/kvm/Makefile | 2 +- tools/kvm/builtin-run.c | 14 ++++++++------ tools/kvm/builtin-stat.c | 4 +++- tools/kvm/disk/core.c | 4 +++- tools/kvm/mmio.c | 4 +++- 5 files changed, 18 insertions(+), 10 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html