RE: [PATCH v4 1/3] KVM: PPC: epapr: Factor out the epapr init

2012-02-17 Thread Liu Yu-B13201


 -Original Message-
 From: Wood Scott-B07421
 Sent: Friday, February 17, 2012 1:13 AM
 To: Liu Yu-B13201
 Cc: ag...@suse.de; kvm-...@vger.kernel.org; kvm@vger.kernel.org;
 linuxppc-...@ozlabs.org; Wood Scott-B07421
 Subject: Re: [PATCH v4 1/3] KVM: PPC: epapr: Factor out the epapr init
 
 On 02/16/2012 03:26 AM, Liu Yu wrote:
  from the kvm guest paravirt init code.
 
  Signed-off-by: Liu Yu yu@freescale.com
  ---
  v4:
  1. code cleanup
  2. move kvm_hypercall_start() to epapr_hypercall_start()
 
   arch/powerpc/Kconfig|4 ++
   arch/powerpc/include/asm/epapr_hcalls.h |2 +
   arch/powerpc/kernel/Makefile|1 +
   arch/powerpc/kernel/epapr.S |   25 
   arch/powerpc/kernel/epapr_para.c|   49
 +++
   arch/powerpc/kernel/kvm.c   |   28 ++
   arch/powerpc/kernel/kvm_emul.S  |   10 --
   arch/powerpc/kvm/Kconfig|1 +
   8 files changed, 85 insertions(+), 35 deletions(-)  create mode
  100644 arch/powerpc/kernel/epapr.S  create mode 100644
  arch/powerpc/kernel/epapr_para.c
 
 The comment about spelling out paravirt wasnn't meant to be restricted
 to the kconfig symbol.  There are lots of words that begin with para,
 and ePAPR isn't just about virtualization.

What do you mean? Do you suggest that we should name it epapr_paravirt.c?

Thanks,
Yu


Re: [PATCH v4 1/3] KVM: PPC: epapr: Factor out the epapr init

2012-02-17 Thread Scott Wood
On 02/17/2012 04:03 AM, Liu Yu-B13201 wrote:
 
 
 -Original Message-
 From: Wood Scott-B07421
 Sent: Friday, February 17, 2012 1:13 AM
 To: Liu Yu-B13201
 Cc: ag...@suse.de; kvm-...@vger.kernel.org; kvm@vger.kernel.org;
 linuxppc-...@ozlabs.org; Wood Scott-B07421
 Subject: Re: [PATCH v4 1/3] KVM: PPC: epapr: Factor out the epapr init

 On 02/16/2012 03:26 AM, Liu Yu wrote:
 from the kvm guest paravirt init code.

 Signed-off-by: Liu Yu yu@freescale.com
 ---
 v4:
 1. code cleanup
 2. move kvm_hypercall_start() to epapr_hypercall_start()

  arch/powerpc/Kconfig|4 ++
  arch/powerpc/include/asm/epapr_hcalls.h |2 +
  arch/powerpc/kernel/Makefile|1 +
  arch/powerpc/kernel/epapr.S |   25 
  arch/powerpc/kernel/epapr_para.c|   49
 +++
  arch/powerpc/kernel/kvm.c   |   28 ++
  arch/powerpc/kernel/kvm_emul.S  |   10 --
  arch/powerpc/kvm/Kconfig|1 +
  8 files changed, 85 insertions(+), 35 deletions(-)  create mode
 100644 arch/powerpc/kernel/epapr.S  create mode 100644
 arch/powerpc/kernel/epapr_para.c

 The comment about spelling out paravirt wasnn't meant to be restricted
 to the kconfig symbol.  There are lots of words that begin with para,
 and ePAPR isn't just about virtualization.
 
 What do you mean? Do you suggest that we should name it epapr_paravirt.c?

Yes, and likewise with variables and functions and such (at least
anything that is exposed outside a single file).

-Scott

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH v4 1/3] KVM: PPC: epapr: Factor out the epapr init

2012-02-17 Thread Liu Yu-B13201


 -Original Message-
 From: Wood Scott-B07421
 Sent: Friday, February 17, 2012 1:13 AM
 To: Liu Yu-B13201
 Cc: ag...@suse.de; kvm-ppc@vger.kernel.org; k...@vger.kernel.org;
 linuxppc-...@ozlabs.org; Wood Scott-B07421
 Subject: Re: [PATCH v4 1/3] KVM: PPC: epapr: Factor out the epapr init
 
 On 02/16/2012 03:26 AM, Liu Yu wrote:
  from the kvm guest paravirt init code.
 
  Signed-off-by: Liu Yu yu@freescale.com
  ---
  v4:
  1. code cleanup
  2. move kvm_hypercall_start() to epapr_hypercall_start()
 
   arch/powerpc/Kconfig|4 ++
   arch/powerpc/include/asm/epapr_hcalls.h |2 +
   arch/powerpc/kernel/Makefile|1 +
   arch/powerpc/kernel/epapr.S |   25 
   arch/powerpc/kernel/epapr_para.c|   49
 +++
   arch/powerpc/kernel/kvm.c   |   28 ++
   arch/powerpc/kernel/kvm_emul.S  |   10 --
   arch/powerpc/kvm/Kconfig|1 +
   8 files changed, 85 insertions(+), 35 deletions(-)  create mode
  100644 arch/powerpc/kernel/epapr.S  create mode 100644
  arch/powerpc/kernel/epapr_para.c
 
 The comment about spelling out paravirt wasnn't meant to be restricted
 to the kconfig symbol.  There are lots of words that begin with para,
 and ePAPR isn't just about virtualization.

What do you mean? Do you suggest that we should name it epapr_paravirt.c?

Thanks,
Yu


Re: [PATCH v4 1/3] KVM: PPC: epapr: Factor out the epapr init

2012-02-16 Thread Alexander Graf


On 16.02.2012, at 10:26, Liu Yu yu@freescale.com wrote:

 from the kvm guest paravirt init code.
 
 Signed-off-by: Liu Yu yu@freescale.com
 ---
 v4:
 1. code cleanup
 2. move kvm_hypercall_start() to epapr_hypercall_start()
 
 arch/powerpc/Kconfig|4 ++
 arch/powerpc/include/asm/epapr_hcalls.h |2 +
 arch/powerpc/kernel/Makefile|1 +
 arch/powerpc/kernel/epapr.S |   25 
 arch/powerpc/kernel/epapr_para.c|   49 +++
 arch/powerpc/kernel/kvm.c   |   28 ++
 arch/powerpc/kernel/kvm_emul.S  |   10 --
 arch/powerpc/kvm/Kconfig|1 +
 8 files changed, 85 insertions(+), 35 deletions(-)
 create mode 100644 arch/powerpc/kernel/epapr.S
 create mode 100644 arch/powerpc/kernel/epapr_para.c
 
 diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
 index 1919634..1262b43 100644
 --- a/arch/powerpc/Kconfig
 +++ b/arch/powerpc/Kconfig
 @@ -202,6 +202,10 @@ config EPAPR_BOOT
  Used to allow a board to specify it wants an ePAPR compliant wrapper.
default n
 
 +config EPAPR_PARAVIRT
 +bool
 +default n
 +
 config DEFAULT_UIMAGE
bool
help
 diff --git a/arch/powerpc/include/asm/epapr_hcalls.h 
 b/arch/powerpc/include/asm/epapr_hcalls.h
 index f3b0c2c..0ff3f24 100644
 --- a/arch/powerpc/include/asm/epapr_hcalls.h
 +++ b/arch/powerpc/include/asm/epapr_hcalls.h
 @@ -148,6 +148,8 @@
 #define EV_HCALL_CLOBBERS2 EV_HCALL_CLOBBERS3, r5
 #define EV_HCALL_CLOBBERS1 EV_HCALL_CLOBBERS2, r4
 
 +extern bool epapr_para_enabled;
 +extern u32 epapr_hypercall_start[];
 
 /*
  * We use uintptr_t to define a register because it's guaranteed to be a
 diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
 index ee728e4..9e807f3 100644
 --- a/arch/powerpc/kernel/Makefile
 +++ b/arch/powerpc/kernel/Makefile
 @@ -136,6 +136,7 @@ ifneq ($(CONFIG_XMON)$(CONFIG_KEXEC),)
 obj-y+= ppc_save_regs.o
 endif
 
 +obj-$(CONFIG_EPAPR_PARAVIRT)+= epapr_para.o epapr.o
 obj-$(CONFIG_KVM_GUEST)+= kvm.o kvm_emul.o
 
 # Disable GCOV in odd or sensitive code
 diff --git a/arch/powerpc/kernel/epapr.S b/arch/powerpc/kernel/epapr.S
 new file mode 100644
 index 000..697b390
 --- /dev/null
 +++ b/arch/powerpc/kernel/epapr.S
 @@ -0,0 +1,25 @@
 +/*
 + * Copyright (C) 2012 Freescale Semiconductor, Inc.
 + *
 + * This program is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU General Public License
 + * as published by the Free Software Foundation; either version
 + * 2 of the License, or (at your option) any later version.
 + */
 +
 +#include linux/threads.h
 +#include asm/reg.h
 +#include asm/page.h
 +#include asm/cputable.h
 +#include asm/thread_info.h
 +#include asm/ppc_asm.h
 +#include asm/asm-offsets.h
 +
 +/* Hypercall entry point. Will be patched with device tree instructions. */
 +.global epapr_hypercall_start
 +epapr_hypercall_start:
 +lir3, -1
 +nop
 +nop
 +nop
 +blr
 diff --git a/arch/powerpc/kernel/epapr_para.c 
 b/arch/powerpc/kernel/epapr_para.c
 new file mode 100644
 index 000..ea13cac
 --- /dev/null
 +++ b/arch/powerpc/kernel/epapr_para.c
 @@ -0,0 +1,49 @@
 +/*
 + * ePAPR para-virtualization support.
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License, version 2, as
 + * published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program; if not, write to the Free Software
 + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 + *
 + * Copyright (C) 2012 Freescale Semiconductor, Inc.
 + */
 +
 +#include linux/of.h
 +#include asm/epapr_hcalls.h
 +#include asm/cacheflush.h
 +
 +bool epapr_para_enabled = false;
 +
 +static int __init epapr_para_init(void)
 +{
 +struct device_node *hyper_node;
 +const u32 *insts;
 +int len, i;
 +
 +hyper_node = of_find_node_by_path(/hypervisor);
 +if (!hyper_node)
 +return -ENODEV;
 +
 +insts = of_get_property(hyper_node, hcall-instructions, len);
 +if (!(len % 4)  (len = (4 * 4))) {
 +for (i = 0; i  (len / 4); i++)

So if the dt passes in less than 4 ops, you bail out, and for more you 
overwrite the buffer? Not good :)

 +epapr_hypercall_start[i] = insts[i];
 +flush_icache_range((ulong)epapr_hypercall_start,
 +   (ulong)epapr_hypercall_start + len);
 +
 +epapr_para_enabled = true;
 +}
 +
 +return 0;
 +}
 +
 +early_initcall(epapr_para_init);
 diff --git 

Re: [PATCH v4 1/3] KVM: PPC: epapr: Factor out the epapr init

2012-02-16 Thread Scott Wood
On 02/16/2012 03:26 AM, Liu Yu wrote:
 from the kvm guest paravirt init code.
 
 Signed-off-by: Liu Yu yu@freescale.com
 ---
 v4:
 1. code cleanup
 2. move kvm_hypercall_start() to epapr_hypercall_start()
 
  arch/powerpc/Kconfig|4 ++
  arch/powerpc/include/asm/epapr_hcalls.h |2 +
  arch/powerpc/kernel/Makefile|1 +
  arch/powerpc/kernel/epapr.S |   25 
  arch/powerpc/kernel/epapr_para.c|   49 
 +++
  arch/powerpc/kernel/kvm.c   |   28 ++
  arch/powerpc/kernel/kvm_emul.S  |   10 --
  arch/powerpc/kvm/Kconfig|1 +
  8 files changed, 85 insertions(+), 35 deletions(-)
  create mode 100644 arch/powerpc/kernel/epapr.S
  create mode 100644 arch/powerpc/kernel/epapr_para.c

The comment about spelling out paravirt wasnn't meant to be restricted
to the kconfig symbol.  There are lots of words that begin with para,
and ePAPR isn't just about virtualization.

 diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
 index 1919634..1262b43 100644
 --- a/arch/powerpc/Kconfig
 +++ b/arch/powerpc/Kconfig
 @@ -202,6 +202,10 @@ config EPAPR_BOOT
 Used to allow a board to specify it wants an ePAPR compliant wrapper.
   default n
  
 +config EPAPR_PARAVIRT
 + bool
 + default n

Why isn't this user-selectable?  It's useful on its own.

 diff --git a/arch/powerpc/kernel/epapr.S b/arch/powerpc/kernel/epapr.S
 new file mode 100644
 index 000..697b390
 --- /dev/null
 +++ b/arch/powerpc/kernel/epapr.S

epapr-hcall.S

 +bool epapr_para_enabled = false;
 +
 +static int __init epapr_para_init(void)
 +{
 + struct device_node *hyper_node;
 + const u32 *insts;
 + int len, i;
 +
 + hyper_node = of_find_node_by_path(/hypervisor);
 + if (!hyper_node)
 + return -ENODEV;
 +
 + insts = of_get_property(hyper_node, hcall-instructions, len);
 + if (!(len % 4)  (len = (4 * 4))) {
 + for (i = 0; i  (len / 4); i++)
 + epapr_hypercall_start[i] = insts[i];
 + flush_icache_range((ulong)epapr_hypercall_start,
 +(ulong)epapr_hypercall_start + len);
 +
 + epapr_para_enabled = true;
 + }

Use patch_instruction(), fix the if test, and remove unnecessary
parentheses.  Print an error if the if test fails, but return silently
if the property is absent.

Please make asm/epapr_hcalls.h and asm/fsl_hcalls.h work with this as well.

-Scott

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html