[PATCH 1/2] powerpc: Define the hcall opcodes & return values we need

[PATCH 1/2] powerpc: Define the hcall opcodes & return values we need

[Michael Ellerman]

Now that we don't have the kernel header on hand, just define the
minimum set of hcall opcodes and return values we need in order to
build.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 powerpc/spapr.h | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/powerpc/spapr.h b/powerpc/spapr.h
index 0537f881c0e4..7a377d093ef4 100644
--- a/powerpc/spapr.h
+++ b/powerpc/spapr.h
@@ -16,17 +16,32 @@
 
 #include <inttypes.h>
 
-/* We need some of the H_ hcall defs, but they're __KERNEL__ only. */
-#define __KERNEL__
-#include <asm/hvcall.h>
-#undef __KERNEL__
-
 #include "kvm/kvm.h"
 #include "kvm/kvm-cpu.h"
 
 typedef unsigned long target_ulong;
 typedef uintptr_t target_phys_addr_t;
 
+#define H_SUCCESS	0
+#define H_HARDWARE	-1	/* Hardware error */
+#define H_FUNCTION	-2	/* Function not supported */
+#define H_PARAMETER	-4	/* Parameter invalid, out-of-range or conflicting */
+
+#define H_SET_DABR		0x28
+#define H_LOGICAL_CI_LOAD	0x3c
+#define H_LOGICAL_CI_STORE	0x40
+#define H_LOGICAL_CACHE_LOAD	0x44
+#define H_LOGICAL_CACHE_STORE	0x48
+#define H_LOGICAL_ICBI		0x4c
+#define H_LOGICAL_DCBF		0x50
+#define H_GET_TERM_CHAR		0x54
+#define H_PUT_TERM_CHAR		0x58
+#define H_CPPR			0x68
+#define H_EOI			0x64
+#define H_IPI			0x6c
+#define H_XIRR			0x74
+#define MAX_HCALL_OPCODE	H_XIRR
+
 /*
  * The hcalls above are standardized in PAPR and implemented by pHyp
  * as well.
-- 
2.1.0

[PATCH 2/2] powerpc: Enable 32-bit build

[Michael Ellerman]

We have always built kvmtool as 64-bit on powerpc, but mainly just out
of habit. There's not AFAIK any reason we *can't* build 32-bit.

So fix up a few places where we were assuming 64-bit, and drop the
Makefile logic that forces 64-bit.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 Makefile                           |  2 --
 powerpc/include/kvm/kvm-cpu-arch.h | 36 ++++++++++++++++++------------------
 powerpc/kvm-cpu.c                  |  2 +-
 powerpc/kvm.c                      |  4 ++--
 4 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/Makefile b/Makefile
index 6110b8e11611..151fa9de07cb 100644
--- a/Makefile
+++ b/Makefile
@@ -148,8 +148,6 @@ ifeq ($(ARCH), powerpc)
 	OBJS	+= powerpc/spapr_pci.o
 	OBJS	+= powerpc/xics.o
 	ARCH_INCLUDE := powerpc/include
-	CFLAGS 	+= -m64
-	LDFLAGS += -m elf64ppc
 
 	ARCH_WANT_LIBFDT := y
 endif
diff --git a/powerpc/include/kvm/kvm-cpu-arch.h b/powerpc/include/kvm/kvm-cpu-arch.h
index e256f5d0733e..01eafdf09785 100644
--- a/powerpc/include/kvm/kvm-cpu-arch.h
+++ b/powerpc/include/kvm/kvm-cpu-arch.h
@@ -17,24 +17,24 @@
 #include <stdbool.h>
 #include <pthread.h>
 
-#define MSR_SF		(1UL<<63)
-#define MSR_HV		(1UL<<60)
-#define MSR_VEC		(1UL<<25)
-#define MSR_VSX		(1UL<<23)
-#define MSR_POW		(1UL<<18)
-#define MSR_EE		(1UL<<15)
-#define MSR_PR		(1UL<<14)
-#define MSR_FP		(1UL<<13)
-#define MSR_ME		(1UL<<12)
-#define MSR_FE0		(1UL<<11)
-#define MSR_SE		(1UL<<10)
-#define MSR_BE		(1UL<<9)
-#define MSR_FE1		(1UL<<8)
-#define MSR_IR		(1UL<<5)
-#define MSR_DR		(1UL<<4)
-#define MSR_PMM		(1UL<<2)
-#define MSR_RI		(1UL<<1)
-#define MSR_LE		(1UL<<0)
+#define MSR_SF		(1ULL<<63)
+#define MSR_HV		(1ULL<<60)
+#define MSR_VEC		(1ULL<<25)
+#define MSR_VSX		(1ULL<<23)
+#define MSR_POW		(1ULL<<18)
+#define MSR_EE		(1ULL<<15)
+#define MSR_PR		(1ULL<<14)
+#define MSR_FP		(1ULL<<13)
+#define MSR_ME		(1ULL<<12)
+#define MSR_FE0		(1ULL<<11)
+#define MSR_SE		(1ULL<<10)
+#define MSR_BE		(1ULL<<9)
+#define MSR_FE1		(1ULL<<8)
+#define MSR_IR		(1ULL<<5)
+#define MSR_DR		(1ULL<<4)
+#define MSR_PMM		(1ULL<<2)
+#define MSR_RI		(1ULL<<1)
+#define MSR_LE		(1ULL<<0)
 
 #define POWER7_EXT_IRQ	0
 
diff --git a/powerpc/kvm-cpu.c b/powerpc/kvm-cpu.c
index 2f5cfc691f42..461e0a907f6d 100644
--- a/powerpc/kvm-cpu.c
+++ b/powerpc/kvm-cpu.c
@@ -147,7 +147,7 @@ static void kvm_cpu__setup_sregs(struct kvm_cpu *vcpu)
 
 	reg.id = KVM_REG_PPC_HIOR;
 	value = 0;
-	reg.addr = (u64)&value;
+	reg.addr = (u64)(unsigned long)&value;
 	if (ioctl(vcpu->vcpu_fd, KVM_SET_ONE_REG, &reg) < 0)
 		die("KVM_SET_ONE_REG failed");
 }
diff --git a/powerpc/kvm.c b/powerpc/kvm.c
index 2b03a1213159..b4c331068374 100644
--- a/powerpc/kvm.c
+++ b/powerpc/kvm.c
@@ -172,7 +172,7 @@ int load_flat_binary(struct kvm *kvm, int fd_kernel, int fd_initrd, const char *
 	while ((nr = read(fd_kernel, p, 65536)) > 0)
 		p += nr;
 
-	pr_info("Loaded kernel to 0x%x (%ld bytes)", KERNEL_LOAD_ADDR, p-k_start);
+	pr_info("Loaded kernel to 0x%x (%ld bytes)", KERNEL_LOAD_ADDR, (long)(p-k_start));
 
 	if (fd_initrd != -1) {
 		if (lseek(fd_initrd, 0, SEEK_SET) < 0)
@@ -192,7 +192,7 @@ int load_flat_binary(struct kvm *kvm, int fd_kernel, int fd_initrd, const char *
 			die("initrd too big to contain in guest RAM.\n");
 
 		pr_info("Loaded initrd to 0x%x (%ld bytes)",
-			INITRD_LOAD_ADDR, p-i_start);
+			INITRD_LOAD_ADDR, (long)(p-i_start));
 		kvm->arch.initrd_gra = INITRD_LOAD_ADDR;
 		kvm->arch.initrd_size = p-i_start;
 	} else {
-- 
2.1.0

Re: [PATCH 1/2] powerpc: Define the hcall opcodes & return values we need

[Michael Ellerman]

On Fri, 2015-06-19 at 17:21 +1000, Michael Ellerman wrote:
> Now that we don't have the kernel header on hand, just define the
> minimum set of hcall opcodes and return values we need in order to
> build.

Hi Andre,

I hope you don't mind me jumping in here but I thought I should help out rather
than just sending feedback :)

I also should have said that these are on top of e96a27a10732 from Will's tree.

cheers

Re: [PATCH 1/2] powerpc: Define the hcall opcodes & return values we need

[Will Deacon]

On Fri, Jun 19, 2015 at 08:21:00AM +0100, Michael Ellerman wrote:
> Now that we don't have the kernel header on hand, just define the
> minimum set of hcall opcodes and return values we need in order to
> build.

Thanks Michael!

I pushed both of these out.

Will

Re: [PATCH 1/2] powerpc: Define the hcall opcodes & return values we need

[Paolo Bonzini]

On 19/06/2015 09:21, Michael Ellerman wrote:
> Now that we don't have the kernel header on hand, just define the
> minimum set of hcall opcodes and return values we need in order to
> build.
> 
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>  powerpc/spapr.h | 25 ++++++++++++++++++++-----
>  1 file changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/powerpc/spapr.h b/powerpc/spapr.h
> index 0537f881c0e4..7a377d093ef4 100644
> --- a/powerpc/spapr.h
> +++ b/powerpc/spapr.h
> @@ -16,17 +16,32 @@
>  
>  #include <inttypes.h>
>  
> -/* We need some of the H_ hcall defs, but they're __KERNEL__ only. */
> -#define __KERNEL__
> -#include <asm/hvcall.h>
> -#undef __KERNEL__
> -

This thing is exactly why I think kvmtool's life in the kernel tree was
harmful.

Why wasn't instead H_* just moved to a uapi/ header?!?!?  Can you do
that now?

Thanks,

Paolo


>  #include "kvm/kvm.h"
>  #include "kvm/kvm-cpu.h"
>  
>  typedef unsigned long target_ulong;
>  typedef uintptr_t target_phys_addr_t;
>  
> +#define H_SUCCESS	0
> +#define H_HARDWARE	-1	/* Hardware error */
> +#define H_FUNCTION	-2	/* Function not supported */
> +#define H_PARAMETER	-4	/* Parameter invalid, out-of-range or conflicting */
> +
> +#define H_SET_DABR		0x28
> +#define H_LOGICAL_CI_LOAD	0x3c
> +#define H_LOGICAL_CI_STORE	0x40
> +#define H_LOGICAL_CACHE_LOAD	0x44
> +#define H_LOGICAL_CACHE_STORE	0x48
> +#define H_LOGICAL_ICBI		0x4c
> +#define H_LOGICAL_DCBF		0x50
> +#define H_GET_TERM_CHAR		0x54
> +#define H_PUT_TERM_CHAR		0x58
> +#define H_CPPR			0x68
> +#define H_EOI			0x64
> +#define H_IPI			0x6c
> +#define H_XIRR			0x74
> +#define MAX_HCALL_OPCODE	H_XIRR
> +
>  /*
>   * The hcalls above are standardized in PAPR and implemented by pHyp
>   * as well.
> 

Re: [PATCH 1/2] powerpc: Define the hcall opcodes & return values we need

[Michael Ellerman]

On Tue, 2015-06-23 at 11:33 +0200, Paolo Bonzini wrote:
> On 19/06/2015 09:21, Michael Ellerman wrote:
> > diff --git a/powerpc/spapr.h b/powerpc/spapr.h
> > index 0537f881c0e4..7a377d093ef4 100644
> > --- a/powerpc/spapr.h
> > +++ b/powerpc/spapr.h
> > @@ -16,17 +16,32 @@
> >  
> >  #include <inttypes.h>
> >  
> > -/* We need some of the H_ hcall defs, but they're __KERNEL__ only. */
> > -#define __KERNEL__
> > -#include <asm/hvcall.h>
> > -#undef __KERNEL__
> > -
> 
> This thing is exactly why I think kvmtool's life in the kernel tree was
> harmful.

Harmful is a bit of an overstatement. Though I agree that was a bit of a hack.

> Why wasn't instead H_* just moved to a uapi/ header?!?!?  Can you do
> that now?

Because it's not part of the kernel API, never was, still isn't.

It *is* part of the hypervisor API, but it's not Linux's job to export that and
deal with the fallout if it was ever changed.

cheers

Re: [PATCH 1/2] powerpc: Define the hcall opcodes & return values we need

[Andre Przywara]

Hi,

On 23/06/15 11:05, Michael Ellerman wrote:
> On Tue, 2015-06-23 at 11:33 +0200, Paolo Bonzini wrote:
>> On 19/06/2015 09:21, Michael Ellerman wrote:
>>> diff --git a/powerpc/spapr.h b/powerpc/spapr.h
>>> index 0537f881c0e4..7a377d093ef4 100644
>>> --- a/powerpc/spapr.h
>>> +++ b/powerpc/spapr.h
>>> @@ -16,17 +16,32 @@
>>>  
>>>  #include <inttypes.h>
>>>  
>>> -/* We need some of the H_ hcall defs, but they're __KERNEL__ only. */
>>> -#define __KERNEL__
>>> -#include <asm/hvcall.h>
>>> -#undef __KERNEL__
>>> -
>>
>> This thing is exactly why I think kvmtool's life in the kernel tree was
>> harmful.
> 
> Harmful is a bit of an overstatement. Though I agree that was a bit of a hack.
> 
>> Why wasn't instead H_* just moved to a uapi/ header?!?!?  Can you do
>> that now?
> 
> Because it's not part of the kernel API, never was, still isn't.

Technically it may not - if I get this correctly it is more a platform
API defined by the architecture, like ACPI or PSCI on ARM.
But if I get this correctly Linux re-uses those definitions in the KVM
API, by piggy-backing on the existing hypercalls. Please correct me if I
am wrong here, I am looking at arch/powerpc/kvm/book3s_hv.c, where it
seems to trap those hypercalls.

So I think that - though not originally invented or defined by Linux -
it should export those definitions that KVM (re-)uses.
QEMU also has a header file duplicating those definitions, so I support
the idea of an uapi header.

> It *is* part of the hypervisor API, but it's not Linux's job to export that and
> deal with the fallout if it was ever changed.

Regardless of what happens if that phyp API changes, KVM would still
need to support those hypercalls, so I think it should export them as well.

Cheers,
Andre.

Re: [PATCH 1/2] powerpc: Define the hcall opcodes & return values we need

[Paolo Bonzini]

On 23/06/2015 12:31, Andre Przywara wrote:
>> > 
>> > Because it's not part of the kernel API, never was, still isn't.
> Technically it may not - if I get this correctly it is more a platform
> API defined by the architecture, like ACPI or PSCI on ARM.
> But if I get this correctly Linux re-uses those definitions in the KVM
> API, by piggy-backing on the existing hypercalls. Please correct me if I
> am wrong here, I am looking at arch/powerpc/kvm/book3s_hv.c, where it
> seems to trap those hypercalls.
> 
> So I think that - though not originally invented or defined by Linux -
> it should export those definitions that KVM (re-)uses.
> QEMU also has a header file duplicating those definitions, so I support
> the idea of an uapi header.

Right, the same is true for other headers: include/scsi/scsi.h and
include/uapi/linux/virtio_*.h come to mind.

Paolo