* [PATCH 8/8 v3] [POWERPC] prpmc2800 needs a dtbImage
From: Dale Farnsworth @ 2008-04-07 22:13 UTC (permalink / raw)
To: paulus; +Cc: linuxppc-dev
In-Reply-To: <20080407220027.GA24510@farnsworth.org>
From: Dale Farnsworth <dale@farnsworth.org>
The prpmc2800 platform requires a zImage formatted file with an
embedded dtb file. Rename the requested boot image file to
dtbImage.prpmc2800.
Signed-off-by: Dale Farnsworth <dale@farnsworth.org>
Acked-by: Mark A. Greer <mgreer@mvista.com>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
---
arch/powerpc/boot/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -192,7 +192,7 @@ image-$(CONFIG_PPC_CHRP) += zImage.chrp
image-$(CONFIG_PPC_EFIKA) += zImage.chrp
image-$(CONFIG_PPC_PMAC) += zImage.pmac
image-$(CONFIG_PPC_HOLLY) += zImage.holly
-image-$(CONFIG_PPC_PRPMC2800) += zImage.prpmc2800
+image-$(CONFIG_PPC_PRPMC2800) += dtbImage.prpmc2800
image-$(CONFIG_PPC_ISERIES) += zImage.iseries
image-$(CONFIG_DEFAULT_UIMAGE) += uImage
^ permalink raw reply
* [PATCH 1/1 resend][PPC] replace logical by bitand in pci_process_ISA_OF_ranges(), arch/powerpc/kernel/isa-bridge.c
From: Roel Kluin @ 2008-04-07 22:26 UTC (permalink / raw)
To: jwboyer; +Cc: linuxppc-dev, lkml
in arch/powerpc/kernel/isa-bridge.c:
41:#define ISA_SPACE_MASK 0x1
42:#define ISA_SPACE_IO 0x1
...
83: if ((range->isa_addr.a_hi && ISA_SPACE_MASK) != ISA_SPACE_IO) {
...
89: if ((range->isa_addr.a_hi && ISA_SPACE_MASK) != ISA_SPACE_IO)
I think these should be single &'s, I can't test it (no hardware)
please consider the patch below.
--
replace logical by bit and for ISA_SPACE_MASK
Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
---
diff --git a/arch/powerpc/kernel/isa-bridge.c b/arch/powerpc/kernel/isa-bridge.c
index f0f49d1..406a9e6 100644
--- a/arch/powerpc/kernel/isa-bridge.c
+++ b/arch/powerpc/kernel/isa-bridge.c
@@ -80,13 +80,13 @@ static void __devinit pci_process_ISA_OF_ranges(struct device_node *isa_node,
* (size depending on dev->n_addr_cells)
* cell 5: the size of the range
*/
- if ((range->isa_addr.a_hi && ISA_SPACE_MASK) != ISA_SPACE_IO) {
+ if ((range->isa_addr.a_hi & ISA_SPACE_MASK) != ISA_SPACE_IO) {
range++;
rlen -= sizeof(struct isa_range);
if (rlen < sizeof(struct isa_range))
goto inval_range;
}
- if ((range->isa_addr.a_hi && ISA_SPACE_MASK) != ISA_SPACE_IO)
+ if ((range->isa_addr.a_hi & ISA_SPACE_MASK) != ISA_SPACE_IO)
goto inval_range;
isa_addr = range->isa_addr.a_lo;
^ permalink raw reply related
* Re: Question on mpc52xx_common.c
From: Matt Sealey @ 2008-04-07 22:31 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev, Detlev Zundel
In-Reply-To: <fa686aa40804031200n4cd7aeb4g330ab3e2620680e@mail.gmail.com>
Grant Likely wrote:
> (cc'ing the mailing list in my reply)
>
> On Thu, Apr 3, 2008 at 10:27 AM, Detlev Zundel <dzu@denx.de> wrote:
>> I wrote,
>>
>> > debugging a lite5200b kernel (linux-2.6-denx equivalent of 2.6.25-rc8) I
>> > noticed that in mpc52xx_map_common_devices (mpc52xx_common.c:161) the
>> > cdm module is tentatively mapped. It seems strange to me that there is no
>> > error checking here. Is a FDT without an cdm entry usable at all?
>
> Sure, why not? If the firmware has already set it up correctly and no
> devices using it are in use, then the kernel should be okay. :-)
> That said, I can't imagine choosing to not put the cdm node into the
> device tree.
*ahem* Efika.
--
Matt Sealey <matt@genesi-usa.com>
Genesi, Manager, Developer Relations
^ permalink raw reply
* Kernel 2.6.23 Error with make bzImage
From: Sam Karp @ 2008-04-07 22:36 UTC (permalink / raw)
To: linuxppc-embedded
[-- Attachment #1: Type: text/plain, Size: 1314 bytes --]
I've been trying to build the linux 2.6.23 kernel for the PPC405 on the
xilinx ml403. I copied over my xparameters.h and xparameters_ml40x.h files
and added the following options through menuconfig.
Enable loadable module support: DISABLE
Processor:
- Type 40x
- Math emulation: ENABLE
- IBM 40x options:
- Machine Type: "Xilinx-ML403"
Platform options:
- Kernel command line: "console=ttyUL0 root=/dev/ram"
Bus options:
- PCI support: DISABLE
- PCCARD (PCMCIA/CardBus) support:
- PCCARD (PCMCIA/CardBus) support: DISABLE
Drivers
- Block Devices:
- Xilinx SystemACE support: ENABLE
- Character devices:
- Serial drivers:
- Xilinx uartlite serial port support: ENABLE
- Support for console on Xilinx uartlite serial port:
ENABLE
I then ran make bzImage and got the following errors. I'm guessing there
are two options I chose that conflict. Any insight?
CC arch/powerpc/kernel/swsusp.o
arch/powerpc/kernel/swsusp.c 19 error: redefinition of
'save_processor_state'
include/asm/suspend.h 7 : previous definition of 'save_processor_state' was
here
arch/powerpc/kernel/swsusp.c 35 error: redefinition of
'restore_processor_state'
include/asm/suspend.h 11 : previous definition of 'restore_processor_state'
was here
Thanks,
Sam
[-- Attachment #2: Type: text/html, Size: 1734 bytes --]
^ permalink raw reply
* Re: [PATCH 1/1 resend][PPC] replace logical by bitand in pci_process_ISA_OF_ranges(), arch/powerpc/kernel/isa-bridge.c
From: Nathan Lynch @ 2008-04-07 22:59 UTC (permalink / raw)
To: Roel Kluin; +Cc: linuxppc-dev, lkml
In-Reply-To: <47FA9FA6.8090201@tiscali.nl>
Roel Kluin wrote:
> replace logical by bit and for ISA_SPACE_MASK
I think Paul has picked this up now:
http://ozlabs.org/pipermail/linuxppc-dev/2008-April/054103.html
^ permalink raw reply
* [PATCH] pseries: phyp dump: Variable size reserve space.
From: Manish Ahuja @ 2008-04-07 23:45 UTC (permalink / raw)
To: linuxppc-dev, paulus; +Cc: mahuja, linasvepstas
A small proposed change in the amount of reserve space we allocate during boot.
Currently we reserve 256MB only.
The proposed change does one of the 3 things.
A. It checks to see if there is cmdline variable set and if found sets the
value to it. OR
B. It computes 5% of total ram and rounds it down to multiples of 256MB. AND
C. Compares the rounded down value and returns larger of two values, the new
computed value or 256MB.
Again this is for large systems who have excess memory.
Signed-off-by: Manish Ahuja <mahuja@us.ibm.com>
---
arch/powerpc/kernel/prom.c | 35 +++++++++++++++++++++++++++--
arch/powerpc/platforms/pseries/phyp_dump.c | 9 +++++++
include/asm-powerpc/phyp_dump.h | 4 ++-
3 files changed, 45 insertions(+), 3 deletions(-)
Index: 2.6.25-rc1/arch/powerpc/platforms/pseries/phyp_dump.c
===================================================================
--- 2.6.25-rc1.orig/arch/powerpc/platforms/pseries/phyp_dump.c 2008-04-02 23:36:51.000000000 -0500
+++ 2.6.25-rc1/arch/powerpc/platforms/pseries/phyp_dump.c 2008-04-02 23:48:34.000000000 -0500
@@ -496,3 +496,12 @@ static int __init early_phyp_dump_enable
}
early_param("phyp_dump", early_phyp_dump_enabled);
+/* Look for phyp_dump_reserve_size= cmdline option */
+static int __init early_phyp_dump_reserve_size(char *p)
+{
+ if (p)
+ phyp_dump_info->phyp_dump_reserve_bootvar = memparse(p, &p);
+
+ return 0;
+}
+early_param("phyp_dump_reserve_size", early_phyp_dump_reserve_size);
Index: 2.6.25-rc1/include/asm-powerpc/phyp_dump.h
===================================================================
--- 2.6.25-rc1.orig/include/asm-powerpc/phyp_dump.h 2008-04-02 23:36:49.000000000 -0500
+++ 2.6.25-rc1/include/asm-powerpc/phyp_dump.h 2008-04-02 23:37:57.000000000 -0500
@@ -24,8 +24,10 @@ struct phyp_dump {
/* Memory that is reserved during very early boot. */
unsigned long init_reserve_start;
unsigned long init_reserve_size;
- /* Check status during boot if dump supported, active & present*/
+ /* cmd line options during boot */
+ unsigned long phyp_dump_reserve_bootvar;
unsigned long phyp_dump_at_boot;
+ /* Check status during boot if dump supported, active & present*/
unsigned long phyp_dump_configured;
unsigned long phyp_dump_is_active;
/* store cpu & hpte size */
Index: 2.6.25-rc1/arch/powerpc/kernel/prom.c
===================================================================
--- 2.6.25-rc1.orig/arch/powerpc/kernel/prom.c 2008-04-02 23:36:49.000000000 -0500
+++ 2.6.25-rc1/arch/powerpc/kernel/prom.c 2008-04-08 00:01:34.000000000 -0500
@@ -1042,6 +1042,33 @@ static void __init early_reserve_mem(voi
#ifdef CONFIG_PHYP_DUMP
/**
+ * phyp_dump_calculate_reserve_size() - reserve variable boot area 5% or arg
+ *
+ * Function to find the largest size we need to reserve
+ * during early boot process.
+ *
+ * It either looks for boot param and returns that OR
+ * returns larger of 256 or 5% rounded down to multiples of 256MB.
+ *
+ */
+static inline unsigned long phyp_dump_calculate_reserve_size(void)
+{
+ unsigned long tmp;
+
+ if (phyp_dump_info->phyp_dump_reserve_bootvar)
+ return phyp_dump_info->phyp_dump_reserve_bootvar;
+
+ /* divide by 20 to get 5% of value */
+ tmp = lmb_end_of_DRAM();
+ do_div(tmp, 20);
+
+ /* round it down in multiples of 256 */
+ tmp = tmp & ~0x000000001FFFFFFF;
+
+ return (tmp > PHYP_DUMP_RMR_END ? tmp : PHYP_DUMP_RMR_END);
+}
+
+/**
* phyp_dump_reserve_mem() - reserve all not-yet-dumped mmemory
*
* This routine may reserve memory regions in the kernel only
@@ -1054,6 +1081,8 @@ static void __init early_reserve_mem(voi
static void __init phyp_dump_reserve_mem(void)
{
unsigned long base, size;
+ unsigned long variable_reserve_size;
+
if (!phyp_dump_info->phyp_dump_configured) {
printk(KERN_ERR "Phyp-dump not supported on this hardware\n");
return;
@@ -1064,9 +1093,11 @@ static void __init phyp_dump_reserve_mem
return;
}
+ variable_reserve_size = phyp_dump_calculate_reserve_size();
+
if (phyp_dump_info->phyp_dump_is_active) {
/* Reserve *everything* above RMR.Area freed by userland tools*/
- base = PHYP_DUMP_RMR_END;
+ base = variable_reserve_size;
size = lmb_end_of_DRAM() - base;
/* XXX crashed_ram_end is wrong, since it may be beyond
@@ -1078,7 +1109,7 @@ static void __init phyp_dump_reserve_mem
} else {
size = phyp_dump_info->cpu_state_size +
phyp_dump_info->hpte_region_size +
- PHYP_DUMP_RMR_END;
+ variable_reserve_size;
base = lmb_end_of_DRAM() - size;
lmb_reserve(base, size);
phyp_dump_info->init_reserve_start = base;
^ permalink raw reply
* Re: [PATCH 1/1 resend][PPC] replace logical by bitand in pci_process_ISA_OF_ranges(), arch/powerpc/kernel/isa-bridge.c
From: Benjamin Herrenschmidt @ 2008-04-07 23:50 UTC (permalink / raw)
To: Roel Kluin; +Cc: linuxppc-dev, lkml
In-Reply-To: <47FA9FA6.8090201@tiscali.nl>
On Tue, 2008-04-08 at 00:26 +0200, Roel Kluin wrote:
> in arch/powerpc/kernel/isa-bridge.c:
> 41:#define ISA_SPACE_MASK 0x1
> 42:#define ISA_SPACE_IO 0x1
> ...
>
> 83: if ((range->isa_addr.a_hi && ISA_SPACE_MASK) != ISA_SPACE_IO) {
> ...
> 89: if ((range->isa_addr.a_hi && ISA_SPACE_MASK) != ISA_SPACE_IO)
>
> I think these should be single &'s, I can't test it (no hardware)
> please consider the patch below.
> --
> replace logical by bit and for ISA_SPACE_MASK
>
> Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> diff --git a/arch/powerpc/kernel/isa-bridge.c b/arch/powerpc/kernel/isa-bridge.c
> index f0f49d1..406a9e6 100644
> --- a/arch/powerpc/kernel/isa-bridge.c
> +++ b/arch/powerpc/kernel/isa-bridge.c
> @@ -80,13 +80,13 @@ static void __devinit pci_process_ISA_OF_ranges(struct device_node *isa_node,
> * (size depending on dev->n_addr_cells)
> * cell 5: the size of the range
> */
> - if ((range->isa_addr.a_hi && ISA_SPACE_MASK) != ISA_SPACE_IO) {
> + if ((range->isa_addr.a_hi & ISA_SPACE_MASK) != ISA_SPACE_IO) {
> range++;
> rlen -= sizeof(struct isa_range);
> if (rlen < sizeof(struct isa_range))
> goto inval_range;
> }
> - if ((range->isa_addr.a_hi && ISA_SPACE_MASK) != ISA_SPACE_IO)
> + if ((range->isa_addr.a_hi & ISA_SPACE_MASK) != ISA_SPACE_IO)
> goto inval_range;
>
> isa_addr = range->isa_addr.a_lo;
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
^ permalink raw reply
* Re: [RFC][PATCH] initial port of fixmap over from x86 for ppc32
From: Benjamin Herrenschmidt @ 2008-04-07 23:51 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, Paul Mackerras, hollisb
In-Reply-To: <20080407162053.GB2688@loki.buserror.net>
On Mon, 2008-04-07 at 11:20 -0500, Scott Wood wrote:
>
> Another possible use is a BAT/TLB1 mapping for SoC registers (or
> anything
> else on the board which is frequently accessed), which can be reused
> by
> ioremap() to avoid wasting normal TLB entries, and to facilitate early
> debugging.
As far as the above is concerned, I'm not too fan of fixed virtual
addresses. I'd rather dynamically assign it. But we can do it.
Ben.
^ permalink raw reply
* Re: [PATCH 2 of 3] [KVM] Add DCR access information to struct kvm_run
From: David Gibson @ 2008-04-08 1:11 UTC (permalink / raw)
To: Hollis Blanchard; +Cc: kvm-devel, linuxppc-dev, kvm-ppc-devel
In-Reply-To: <474b35009da7b7bdb58c.1207601613@localhost.localdomain>
On Mon, Apr 07, 2008 at 03:53:33PM -0500, Hollis Blanchard wrote:
> 1 file changed, 7 insertions(+)
> include/linux/kvm.h | 7 +++++++
>
>
> Device Control Registers are essentially another address space found on PowerPC
> 4xx processors, analogous to PIO on x86. DCRs are always 32 bits, and are
> identified by a 32-bit number.
Well... 10-bit, actually.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* Re: [PATCH 3 of 3] [KVM POWERPC] PowerPC 440 KVM implementation
From: Josh Boyer @ 2008-04-08 2:12 UTC (permalink / raw)
To: Hollis Blanchard; +Cc: kvm-devel, linuxppc-dev, kvm-ppc-devel
In-Reply-To: <b07f3049f16483f80322.1207601614@localhost.localdomain>
On Mon, 07 Apr 2008 15:53:34 -0500
Hollis Blanchard <hollisb@us.ibm.com> wrote:
> Currently supports only PowerPC 440 Linux guests on 440 hosts. (Only tested
> with 440EP "Bamboo" guests so far, but with appropriate userspace support other
> SoC/board combinations should work.)
I haven't reviewed the whole patch yet, but lots of it looks pretty
clean. A couple comments on some things that made me scratch my head
below.
> Interrupt handling: We use IVPR to hijack the host interrupt vectors while
> running the guest, but hand off decrementer and external interrupts for normal
> guest processing.
>
> Address spaces: We take advantage of the fact that Linux doesn't use the AS=1
> address space (in host or guest), which gives us virtual address space to use
> for guest mappings. While the guest is running, the host kernel remains mapped
> in AS=0, but the guest can only use AS=1 mappings.
>
> TLB entries: The TLB entries covering the host linear address space remain
> present while running the guest (which reduces the overhead of lightweight
> exits). We keep three copies of the TLB:
> - guest TLB: contents of the TLB as the guest sees it
> - shadow TLB: the TLB that is actually in hardware while guest is running
> - host TLB: to restore TLB state when context switching guest -> host
> When a TLB miss occurs because a mapping was not present in the shadow TLB, but
> was present in the guest TLB, KVM handles the fault without invoking the guest.
> Large guest pages are backed by multiple 4KB shadow pages through this
> mechanism.
>
> Instruction emulation: The guest kernel executes at user level, so executing
> privileged instructions trap into KVM, where we decode and emulate them. Future
> performance work will focus on reducing the overhead and frequency of these
> traps.
>
> IO: MMIO and DCR accesses are emulated by userspace. We use virtio for network
> and block IO, so those drivers must be enabled in the guest. It's possible that
> some qemu device emulation (e.g. e1000 or rtl8139) may also work with little
> effort.
>
> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
As Olof pointed out, having this in Documentation might be nice.
> diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
> --- a/arch/powerpc/Kconfig.debug
> +++ b/arch/powerpc/Kconfig.debug
> @@ -151,6 +151,7 @@
>
> config PPC_EARLY_DEBUG
> bool "Early debugging (dangerous)"
> + depends on !KVM
> help
> Say Y to enable some early debugging facilities that may be available
> for your processor/board combination. Those facilities are hacks
Might want to add a brief explanation as to why this doesn't work with
KVM due to the AS conflict.
> diff --git a/arch/powerpc/kvm/44x_tlb.c b/arch/powerpc/kvm/44x_tlb.c
> new file mode 100644
> --- /dev/null
> +++ b/arch/powerpc/kvm/44x_tlb.c
> @@ -0,0 +1,224 @@
> +/*
> + * 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 IBM Corp. 2007
> + *
> + * Authors: Hollis Blanchard <hollisb@us.ibm.com>
> + */
> +
> +#include <linux/types.h>
> +#include <linux/string.h>
> +#include <linux/kvm_host.h>
> +#include <linux/highmem.h>
> +#include <asm/mmu-44x.h>
> +#include <asm/kvm_ppc.h>
> +
> +#include "44x_tlb.h"
> +
> +#define PPC44x_TLB_USER_PERM_MASK (PPC44x_TLB_UX|PPC44x_TLB_UR|PPC44x_TLB_UW)
> +#define PPC44x_TLB_SUPER_PERM_MASK (PPC44x_TLB_SX|PPC44x_TLB_SR|PPC44x_TLB_SW)
> +
> +static unsigned int kvmppc_tlb_44x_pos;
> +
> +static u32 kvmppc_44x_tlb_shadow_attrib(u32 attrib, int usermode)
> +{
> + /* XXX remove mask when Linux is fixed */
> + attrib &= 0xf03f;
What does that mean? Is this the "44x TLB handler writes to reserved
fields" issue? If so, could you comment that a little more verbosely?
Or you could just send a patch to fix Linux... ;).
> +
> + if (!usermode) {
> + /* Guest is in supervisor mode, so we need to translate guest
> + * supervisor permissions into user permissions. */
> + attrib &= ~PPC44x_TLB_USER_PERM_MASK;
> + attrib |= (attrib & PPC44x_TLB_SUPER_PERM_MASK) << 3;
> + }
> +
> + /* Make sure host can always access this memory. */
> + attrib |= PPC44x_TLB_SX|PPC44x_TLB_SR|PPC44x_TLB_SW;
> +
> + return attrib;
> +}
> +
<snip>
> diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
> new file mode 100644
> --- /dev/null
> +++ b/arch/powerpc/kvm/emulate.c
> @@ -0,0 +1,753 @@
> +/*
> + * 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 IBM Corp. 2007
> + *
> + * Authors: Hollis Blanchard <hollisb@us.ibm.com>
> + */
> +
> +#include <linux/jiffies.h>
> +#include <linux/timer.h>
> +#include <linux/types.h>
> +#include <linux/string.h>
> +#include <linux/kvm_host.h>
> +
> +#include <asm/dcr.h>
> +#include <asm/time.h>
> +#include <asm/byteorder.h>
> +#include <asm/kvm_ppc.h>
#include <asm/dcr-regs.h>
> +
> +#include "44x_tlb.h"
> +
> +#define DCRN_CPR0_CFGADDR 0xc
> +#define DCRN_CPR0_CFGDATA 0xd
Remove these.
<snip>
> +int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
> +{
> + u32 inst = vcpu->arch.last_inst;
> + u32 ea;
> + int ra;
> + int rb;
> + int rc;
> + int rs;
> + int rt;
> + int sprn;
> + int dcrn;
> + enum emulation_result emulated = EMULATE_DONE;
> + int advance = 1;
> +
> + switch (get_op(inst)) {
<snip>
> + case 323: /* mfdcr */
> + dcrn = get_dcrn(inst);
> + rt = get_rt(inst);
> +
> + /* emulate some access in kernel */
> + switch (dcrn) {
> + case DCRN_CPR0_CFGADDR:
> + vcpu->arch.gpr[rt] = vcpu->arch.cpr0_cfgaddr;
> + break;
> + case DCRN_CPR0_CFGDATA:
> + local_irq_disable();
> + mtdcr(DCRN_CPR0_CFGADDR,
> + vcpu->arch.cpr0_cfgaddr);
> + vcpu->arch.gpr[rt] = mfdcr(DCRN_CPR0_CFGDATA);
> + local_irq_enable();
> + break;
> + default:
> + run->dcr.dcrn = dcrn;
> + run->dcr.data = 0;
> + run->dcr.is_write = 0;
> + vcpu->arch.io_gpr = rt;
> + vcpu->arch.dcr_needed = 1;
> + emulated = EMULATE_DO_DCR;
> + }
> +
> + break;
<snip>
> + case 451: /* mtdcr */
> + dcrn = get_dcrn(inst);
> + rs = get_rs(inst);
> +
> + /* emulate some access in kernel */
> + switch (dcrn) {
> + case DCRN_CPR0_CFGADDR:
> + vcpu->arch.cpr0_cfgaddr = vcpu->arch.gpr[rs];
> + break;
> + default:
> + run->dcr.dcrn = dcrn;
> + run->dcr.data = vcpu->arch.gpr[rs];
> + run->dcr.is_write = 1;
> + vcpu->arch.dcr_needed = 1;
> + emulated = EMULATE_DO_DCR;
> + }
> +
> + break;
What exactly are those doing? I'm confused as to why the CPR0 DCRs are
special cased. Also, you don't seem to actually be doing much on
storing guest DCRs, so can I assume they aren't really needed at
runtime?
josh
^ permalink raw reply
* Re: Question on mpc52xx_common.c
From: Arnd Bergmann @ 2008-04-08 2:14 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Detlev Zundel
In-Reply-To: <47FAA0A8.7050602@genesi-usa.com>
On Tuesday 08 April 2008, Matt Sealey wrote:
> Grant Likely wrote:
> >=20
> > Sure, why not? =A0If the firmware has already set it up correctly and no
> > devices using it are in use, then the kernel should be okay. =A0:-)
> > That said, I can't imagine choosing to not put the cdm node into the
> > device tree.
>=20
> *ahem* Efika.
Maybe we should just give up on making the efika boot with its regular
device tree and instead add a boot wrapper that either fixes up the
data provided by its firmware or just adds a proper dt blob?
Arnd <><
^ permalink raw reply
* Re: Question on mpc52xx_common.c
From: Grant Likely @ 2008-04-08 2:25 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linuxppc-dev, Detlev Zundel
In-Reply-To: <200804080414.42867.arnd@arndb.de>
On Mon, Apr 7, 2008 at 8:14 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 08 April 2008, Matt Sealey wrote:
>
> > Grant Likely wrote:
> > >
> > > Sure, why not? If the firmware has already set it up correctly and no
> > > devices using it are in use, then the kernel should be okay. :-)
> > > That said, I can't imagine choosing to not put the cdm node into the
> > > device tree.
> >
> > *ahem* Efika.
>
> Maybe we should just give up on making the efika boot with its regular
> device tree and instead add a boot wrapper that either fixes up the
> data provided by its firmware or just adds a proper dt blob?
Current kernels boot the Efika without any firmware scripts.
prom_init.c is able to handle the few fixups that the kernel really
wants to see. (So life is mostly happy in Efika land now. :-)
Cheers,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [kvm-ppc-devel] [PATCH] [v2] Add idle wait support for 44x platforms
From: Arnd Bergmann @ 2008-04-08 2:21 UTC (permalink / raw)
To: linuxppc-dev, jyoung5; +Cc: kvm-ppc-devel, Jimi Xenidis
In-Reply-To: <1207323761.6634.6.camel@thinkpadL>
On Friday 04 April 2008, Jerone Young wrote:
> > > +int __init ppc44x_idle_init(void)
> > > +{
> > > +=A0=A0=A0void *func =3D modes[current_mode].entry;
> > > +=A0=A0=A0struct device_node *node;
> > > +
> > > +=A0=A0=A0node =3D of_find_node_by_path("/hypervisor");
> > > +=A0=A0=A0if (node) {
> > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0/* if we find /hypervisor node is i=
n device tree,
> > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 =A0 set idle mode to wait */
> > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0func =3D &ppc44x_idle; /* wait */
> > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0of_node_put(node);
> > > +=A0=A0=A0}
> > > +
> > > +=A0=A0=A0ppc_md.power_save =3D func;
> > > +=A0=A0=A0return 0;
> > > +}
> > > +
> > > +arch_initcall(ppc44x_idle_init);
> >=20
> > IIRC, this would over-ride the idle_param() below, is that the =A0
> > intended behavior?
>=20
> Yes. At the moment if it detects a hypervisor in the kernel tree it
> overrides what the command line says.
>=20
This is unusual behavior. Normally, we try to come up with reasonable
defaults and give the user the chance to override it.
Arnd <><
^ permalink raw reply
* Re: [PATCH] Add idle wait support for 44x platforms
From: Arnd Bergmann @ 2008-04-08 2:17 UTC (permalink / raw)
To: linuxppc-dev; +Cc: kvm-ppc-devel
In-Reply-To: <20080404064741.69f8669b@zod.rchland.ibm.com>
On Friday 04 April 2008, Josh Boyer wrote:
> On Fri, 04 Apr 2008 01:12:38 -0500
> Jerone Young <jyoung5@us.ibm.com> wrote:
> > >
> > > > +static int current_mode = 0;
> > >
> > > Leave this as: static int current_mode;, so it'll end up in the bss
> >
> > The problem here is that this defines the default case. Is there really
> > a benefit having this in bss ?
>
> It's still defined to 0 if it's in the BSS, as that is all initialized
> to 0.
Actually, a static assignment to 0 has not caused the symbol to end up
in .data for many gcc versions, it always goes into .bss now unless you
assign it a value other than 0 or use explicit section attributes.
Whether or not you write the "= 0" is purely stylistic sugar and does
not have any impact the generated binary.
Arnd <><
^ permalink raw reply
* Re: [PATCH] Add idle wait support for 44x platforms
From: Josh Boyer @ 2008-04-08 2:31 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: kvm-ppc-devel, linuxppc-dev
In-Reply-To: <200804080417.37272.arnd@arndb.de>
On Tue, 8 Apr 2008 04:17:36 +0200
Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 04 April 2008, Josh Boyer wrote:
> > On Fri, 04 Apr 2008 01:12:38 -0500
> > Jerone Young <jyoung5@us.ibm.com> wrote:
> > > >
> > > > > +static int current_mode = 0;
> > > >
> > > > Leave this as: static int current_mode;, so it'll end up in the bss
> > >
> > > The problem here is that this defines the default case. Is there really
> > > a benefit having this in bss ?
> >
> > It's still defined to 0 if it's in the BSS, as that is all initialized
> > to 0.
>
> Actually, a static assignment to 0 has not caused the symbol to end up
> in .data for many gcc versions, it always goes into .bss now unless you
> assign it a value other than 0 or use explicit section attributes.
IIRC, gcc 3.2 is still supported and it didn't do that. Old toolchains
still exist.
> Whether or not you write the "= 0" is purely stylistic sugar and does
> not have any impact the generated binary.
Only if you're using a newer gcc version...
josh
^ permalink raw reply
* Re: [PATCH] pseries: phyp dump: Variable size reserve space.
From: Olof Johansson @ 2008-04-08 2:43 UTC (permalink / raw)
To: Manish Ahuja; +Cc: mahuja, linuxppc-dev, linasvepstas, paulus
In-Reply-To: <47FAB221.7050406@austin.ibm.com>
Hi,
Just a few nitpicks, no comments on the functional parts:
On Mon, Apr 07, 2008 at 06:45:37PM -0500, Manish Ahuja wrote:
> A small proposed change in the amount of reserve space we allocate during boot.
> Currently we reserve 256MB only.
> The proposed change does one of the 3 things.
>
> A. It checks to see if there is cmdline variable set and if found sets the
> value to it. OR
> B. It computes 5% of total ram and rounds it down to multiples of 256MB. AND
> C. Compares the rounded down value and returns larger of two values, the new
> computed value or 256MB.
...
> +/* Look for phyp_dump_reserve_size= cmdline option */
> +static int __init early_phyp_dump_reserve_size(char *p)
> +{
> + if (p)
> + phyp_dump_info->phyp_dump_reserve_bootvar = memparse(p, &p);
[...]
> @@ -24,8 +24,10 @@ struct phyp_dump {
> /* Memory that is reserved during very early boot. */
> unsigned long init_reserve_start;
> unsigned long init_reserve_size;
> - /* Check status during boot if dump supported, active & present*/
> + /* cmd line options during boot */
> + unsigned long phyp_dump_reserve_bootvar;
> unsigned long phyp_dump_at_boot;
> + /* Check status during boot if dump supported, active & present*/
> unsigned long phyp_dump_configured;
> unsigned long phyp_dump_is_active;
> /* store cpu & hpte size */
These make for some really long variable names and lines. I know from
experience, since I've picked unneccessary long driver names in the past
myself. :)
How about just naming the new variables reserve_bootvar, etc? The name
of the struct they're in makes it obvious what they're for.
> +static inline unsigned long phyp_dump_calculate_reserve_size(void)
> +{
> + unsigned long tmp;
> +
> + if (phyp_dump_info->phyp_dump_reserve_bootvar)
> + return phyp_dump_info->phyp_dump_reserve_bootvar;
> +
> + /* divide by 20 to get 5% of value */
> + tmp = lmb_end_of_DRAM();
> + do_div(tmp, 20);
> +
> + /* round it down in multiples of 256 */
> + tmp = tmp & ~0x000000001FFFFFFF;
That's 512MB, isn't it?
-Olof
^ permalink raw reply
* Re: [kvm-ppc-devel] [PATCH] [v2] Add idle wait support for 44x platforms
From: Josh Boyer @ 2008-04-08 2:35 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: kvm-ppc-devel, linuxppc-dev, Jimi Xenidis
In-Reply-To: <200804080421.45447.arnd@arndb.de>
On Tue, 8 Apr 2008 04:21:44 +0200
Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 04 April 2008, Jerone Young wrote:
> > > > +int __init ppc44x_idle_init(void)
> > > > +{
> > > > +=C2=A0=C2=A0=C2=A0void *func =3D modes[current_mode].entry;
> > > > +=C2=A0=C2=A0=C2=A0struct device_node *node;
> > > > +
> > > > +=C2=A0=C2=A0=C2=A0node =3D of_find_node_by_path("/hypervisor");
> > > > +=C2=A0=C2=A0=C2=A0if (node) {
> > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
/* if we find /hypervisor node is in device tree,
> > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0 set idle mode to wait */
> > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
func =3D &ppc44x_idle; /* wait */
> > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
of_node_put(node);
> > > > +=C2=A0=C2=A0=C2=A0}
> > > > +
> > > > +=C2=A0=C2=A0=C2=A0ppc_md.power_save =3D func;
> > > > +=C2=A0=C2=A0=C2=A0return 0;
> > > > +}
> > > > +
> > > > +arch_initcall(ppc44x_idle_init);
> > >=20
> > > IIRC, this would over-ride the idle_param() below, is that the =C2=A0
> > > intended behavior?
> >=20
> > Yes. At the moment if it detects a hypervisor in the kernel tree it
> > overrides what the command line says.
> >=20
>=20
> This is unusual behavior. Normally, we try to come up with reasonable
> defaults and give the user the chance to override it.
The /hypervisor check is going away completely in the next version of
the patch. I've spoken to the HW guys and am more comfortable with
wait being the default. The spin version should still be an option,
specified by the kernel parameter.
josh
^ permalink raw reply
* Re: [PATCH] Add idle wait support for 44x platforms
From: Arnd Bergmann @ 2008-04-08 2:41 UTC (permalink / raw)
To: linuxppc-dev; +Cc: kvm-ppc-devel
In-Reply-To: <20080407213127.7bb9a769@zod.rchland.ibm.com>
On Tuesday 08 April 2008, Josh Boyer wrote:
> >=20
> > Actually, a static assignment to 0 has not caused the symbol to end up
> > in .data for many gcc versions, it always goes into .bss now unless you
> > assign it a value other than 0 or use explicit section attributes.
>=20
> IIRC, gcc 3.2 is still supported and it didn't do that. =A0Old toolchains
> still exist.
Ok, I thought it was before 3.2. The oldest version I had around was
gcc-3.3 and that had the new behaviour.
Arnd <><
^ permalink raw reply
* Re: [PATCH 2/3] [POWERPC][V3] Xilinx: of_serial support for Xilinx uart 16550.
From: David Gibson @ 2008-04-08 2:44 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev, paulus, John Linn, Arnd Bergmann
In-Reply-To: <1207406756.6809.2.camel@vader.jdub.homelinux.org>
On Sat, Apr 05, 2008 at 09:45:56AM -0500, Josh Boyer wrote:
> On Thu, 2008-04-03 at 06:16 +0200, Arnd Bergmann wrote:
> > On Thursday 03 April 2008, Grant Likely wrote:
> > > >
> > > > Since it is not really compatible with ns16550, shouldn't you at least specify
> > > > a different "compatible" property? That way, the driver won't do incorrect
> > > > accesses when you try to use an old driver with a device tree that specifies
> > > > one of these.
> > >
> > > Heh; we've gone back and forth on this issue. The problem is that we
> > > have a common case of ns16550 like devices that require a little bit
> > > of register address tweaking that spans a whole range of vendors (so
> > > adding a compatible match with each of those vendor's prefixes is
> > > probably non-scalable). So, if "ns16550" is not a good idea, then
> > > what should be used? "sparse16550" has been suggested more than once.
> >
> > After another IRC discussion between Grant, Segher and myself, we concluded
> > that we don't need to invent a new "compatible" value, as only new device
> > trees with old kernels will have a problem with this, and they don't work
> > in the first place.
> >
> > The devices will still have their specific "compatible" value, e.g.
> > "xlnx,plb-uart16550-1.00.c", followed by "ns16550", and possibly
> > "ns16450" and "i8250", although the last two do not have an effect
> > on Linux.
> >
> > Josh, can you please forward all three patches in their latest version?
> >
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
>
> So is the discussion on reg-offset settled then? It seemed Paul and
> David had some issues with that, and I'd like to make sure everyone is
> agreed on that before I bring in patches 2 and 3.
I didn't like it very much, but I don't really care enough to argue
about it.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* Re: [PATCH] Add idle wait support for 44x platforms
From: Josh Boyer @ 2008-04-08 2:44 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: kvm-ppc-devel, linuxppc-dev
In-Reply-To: <200804080441.28739.arnd@arndb.de>
On Tue, 8 Apr 2008 04:41:28 +0200
Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 08 April 2008, Josh Boyer wrote:
> > >=20
> > > Actually, a static assignment to 0 has not caused the symbol to end up
> > > in .data for many gcc versions, it always goes into .bss now unless y=
ou
> > > assign it a value other than 0 or use explicit section attributes.
> >=20
> > IIRC, gcc 3.2 is still supported and it didn't do that. =C2=A0Old toolc=
hains
> > still exist.
>=20
> Ok, I thought it was before 3.2. The oldest version I had around was
> gcc-3.3 and that had the new behaviour.
http://www.gnu.org/software/gcc/gcc-3.3/changes.html
gcc 3.3.1 it seems. I thought I included that in the original reply,
but it's late and I suck :).
josh
^ permalink raw reply
* Re: [PATCH 3 of 3] [KVM POWERPC] PowerPC 440 KVM implementation
From: Arnd Bergmann @ 2008-04-08 2:58 UTC (permalink / raw)
To: linuxppc-dev; +Cc: kvm-devel, kvm-ppc-devel, Hollis Blanchard
In-Reply-To: <b07f3049f16483f80322.1207601614@localhost.localdomain>
On Monday 07 April 2008, Hollis Blanchard wrote:
> --- a/include/asm-powerpc/kvm.h
> +++ b/include/asm-powerpc/kvm.h
> @@ -1,6 +1,55 @@
> +/*
> + * 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. =A0See 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 =A002110-1301=
, USA.
> + *
> + * Copyright IBM Corp. 2007
> + *
> + * Authors: Hollis Blanchard <hollisb@us.ibm.com>
> + */
> +
> =A0#ifndef __LINUX_KVM_POWERPC_H
> =A0#define __LINUX_KVM_POWERPC_H
> =A0
> -/* powerpc does not support KVM */
> +#include <asm/types.h>
> =A0
> -#endif
> +struct kvm_regs {
> +=A0=A0=A0=A0=A0=A0=A0__u32 pc;
> +=A0=A0=A0=A0=A0=A0=A0__u32 cr;
> +=A0=A0=A0=A0=A0=A0=A0__u32 ctr;
> +=A0=A0=A0=A0=A0=A0=A0__u32 lr;
> +=A0=A0=A0=A0=A0=A0=A0__u32 xer;
> +=A0=A0=A0=A0=A0=A0=A0__u32 msr;
> +=A0=A0=A0=A0=A0=A0=A0__u32 srr0;
> +=A0=A0=A0=A0=A0=A0=A0__u32 srr1;
> +=A0=A0=A0=A0=A0=A0=A0__u32 pid;
> +
> +=A0=A0=A0=A0=A0=A0=A0__u32 sprg0;
> +=A0=A0=A0=A0=A0=A0=A0__u32 sprg1;
> +=A0=A0=A0=A0=A0=A0=A0__u32 sprg2;
> +=A0=A0=A0=A0=A0=A0=A0__u32 sprg3;
> +=A0=A0=A0=A0=A0=A0=A0__u32 sprg4;
> +=A0=A0=A0=A0=A0=A0=A0__u32 sprg5;
> +=A0=A0=A0=A0=A0=A0=A0__u32 sprg6;
> +=A0=A0=A0=A0=A0=A0=A0__u32 sprg7;
> +
> +=A0=A0=A0=A0=A0=A0=A0__u64 fpr[32];
> +=A0=A0=A0=A0=A0=A0=A0__u32 gpr[32];
> +};
> +
> +struct kvm_sregs {
> +};
> +
> +struct kvm_fpu {
> +};
> +
> +#endif /* __LINUX_KVM_POWERPC_H */
Since this defines part of the ABI, it would be nice if it's possible
to have it in a platform independent way. Most of the registers here
should probably become "unsigned long" instead of "__u32" so that
the definition can be used for a potential 64 bit port.
Also, I noticed that you lump everything into kvm_regs, instead of
using sregs for stuff like srr0 and kvm_fpu for the fprs. What is
the reason for that?
Arnd <><
^ permalink raw reply
* Re: [PATCH 2 of 3] [KVM] Add DCR access information to struct kvm_run
From: Hollis Blanchard @ 2008-04-08 3:25 UTC (permalink / raw)
To: David Gibson; +Cc: kvm-devel, linuxppc-dev, kvm-ppc-devel
In-Reply-To: <20080408011128.GA13454@localhost.localdomain>
On Monday 07 April 2008 20:11:28 David Gibson wrote:
> On Mon, Apr 07, 2008 at 03:53:33PM -0500, Hollis Blanchard wrote:
> > 1 file changed, 7 insertions(+)
> > include/linux/kvm.h | 7 +++++++
> >
> >
> > Device Control Registers are essentially another address space found on
> > PowerPC 4xx processors, analogous to PIO on x86. DCRs are always 32 bits,
> > and are identified by a 32-bit number.
>
> Well... 10-bit, actually.
The mtdcrux description in the ppc440x6 user manual says the following:
Let the contents of register RA denote a Device Control Register.
The contents of GPR[RS] are placed into the designated Device Control
Register.
I take that to mean that we must worry about 32 bits worth of DCR numbers.
Perhaps I should say "no more than" rather than "always".
--
Hollis Blanchard
IBM Linux Technology Center
^ permalink raw reply
* Re: Booting a Xilinx board
From: Grant Likely @ 2008-04-08 3:55 UTC (permalink / raw)
To: Guillaume Dargaud; +Cc: linuxppc-dev
In-Reply-To: <10a101c898c4$c183ee40$f52f9e86@LPSC0173W>
On Mon, Apr 7, 2008 at 9:33 AM, Guillaume Dargaud <dargaud@lpsc.in2p3.fr> wrote:
>
> Linux/PPC load: console=ttyS0,9600 ip=on root=/dev/sysace/disc0/part2 rw
> Uncompressing Linux...done.
> Now booting the kernel
>
> And then it stops.
> I'm sure I've overlooked something obvious...
Are you using uartlite? Try console=ttyUL0.
But Stephen is right, the first thing you should do is look at
__log_buf. You can find its address in the System.map file.
Cheers,
g.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH 2 of 3] [KVM] Add DCR access information to struct kvm_run
From: David Gibson @ 2008-04-08 3:54 UTC (permalink / raw)
To: Hollis Blanchard; +Cc: kvm-devel, linuxppc-dev, kvm-ppc-devel
In-Reply-To: <200804072225.33040.hollisb@us.ibm.com>
On Mon, Apr 07, 2008 at 10:25:32PM -0500, Hollis Blanchard wrote:
> On Monday 07 April 2008 20:11:28 David Gibson wrote:
> > On Mon, Apr 07, 2008 at 03:53:33PM -0500, Hollis Blanchard wrote:
> > > 1 file changed, 7 insertions(+)
> > > include/linux/kvm.h | 7 +++++++
> > >
> > >
> > > Device Control Registers are essentially another address space found on
> > > PowerPC 4xx processors, analogous to PIO on x86. DCRs are always 32 bits,
> > > and are identified by a 32-bit number.
> >
> > Well... 10-bit, actually.
>
> The mtdcrux description in the ppc440x6 user manual says the following:
>
> Let the contents of register RA denote a Device Control Register.
> The contents of GPR[RS] are placed into the designated Device Control
> Register.
>
> I take that to mean that we must worry about 32 bits worth of DCR numbers.
> Perhaps I should say "no more than" rather than "always".
I think that's less misleading. mtdcrux is very new, anything which
only has the mtdcr instruction certainly can't take DCR numbers above
10 bits, and I would expect that even on chips with mtdcrux the DCR
bus is probably still only 10-bits, although it could be extended.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* Re: [PATCH 3 of 3] [KVM POWERPC] PowerPC 440 KVM implementation
From: Hollis Blanchard @ 2008-04-08 4:00 UTC (permalink / raw)
To: Josh Boyer; +Cc: kvm-devel, linuxppc-dev, kvm-ppc-devel
In-Reply-To: <20080407211240.4fc5c6d9@zod.rchland.ibm.com>
On Monday 07 April 2008 21:12:40 Josh Boyer wrote:
> On Mon, 07 Apr 2008 15:53:34 -0500
>
> Hollis Blanchard <hollisb@us.ibm.com> wrote:
> > Currently supports only PowerPC 440 Linux guests on 440 hosts. (Only
> > tested with 440EP "Bamboo" guests so far, but with appropriate userspace
> > support other SoC/board combinations should work.)
>
> I haven't reviewed the whole patch yet, but lots of it looks pretty
> clean. A couple comments on some things that made me scratch my head
> below.
>
> > Interrupt handling: We use IVPR to hijack the host interrupt vectors
> > while running the guest, but hand off decrementer and external interrupts
> > for normal guest processing.
> >
> > Address spaces: We take advantage of the fact that Linux doesn't use the
> > AS=1 address space (in host or guest), which gives us virtual address
> > space to use for guest mappings. While the guest is running, the host
> > kernel remains mapped in AS=0, but the guest can only use AS=1 mappings.
> >
> > TLB entries: The TLB entries covering the host linear address space
> > remain present while running the guest (which reduces the overhead of
> > lightweight exits). We keep three copies of the TLB:
> > - guest TLB: contents of the TLB as the guest sees it
> > - shadow TLB: the TLB that is actually in hardware while guest is
> > running - host TLB: to restore TLB state when context switching guest ->
> > host When a TLB miss occurs because a mapping was not present in the
> > shadow TLB, but was present in the guest TLB, KVM handles the fault
> > without invoking the guest. Large guest pages are backed by multiple 4KB
> > shadow pages through this mechanism.
> >
> > Instruction emulation: The guest kernel executes at user level, so
> > executing privileged instructions trap into KVM, where we decode and
> > emulate them. Future performance work will focus on reducing the overhead
> > and frequency of these traps.
> >
> > IO: MMIO and DCR accesses are emulated by userspace. We use virtio for
> > network and block IO, so those drivers must be enabled in the guest. It's
> > possible that some qemu device emulation (e.g. e1000 or rtl8139) may also
> > work with little effort.
> >
> > Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
>
> As Olof pointed out, having this in Documentation might be nice.
Yeah, the trouble is that a book could be written on the subject. (OK, a short
book. At least a paper.) Anyways, I will provide something...
> > diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
> > --- a/arch/powerpc/Kconfig.debug
> > +++ b/arch/powerpc/Kconfig.debug
> > @@ -151,6 +151,7 @@
> >
> > config PPC_EARLY_DEBUG
> > bool "Early debugging (dangerous)"
> > + depends on !KVM
> > help
> > Say Y to enable some early debugging facilities that may be available
> > for your processor/board combination. Those facilities are hacks
>
> Might want to add a brief explanation as to why this doesn't work with
> KVM due to the AS conflict.
Will do.
> > diff --git a/arch/powerpc/kvm/44x_tlb.c b/arch/powerpc/kvm/44x_tlb.c
> > new file mode 100644
> > --- /dev/null
> > +++ b/arch/powerpc/kvm/44x_tlb.c
> > @@ -0,0 +1,224 @@
> > +/*
> > + * 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 IBM Corp. 2007
> > + *
> > + * Authors: Hollis Blanchard <hollisb@us.ibm.com>
> > + */
> > +
> > +#include <linux/types.h>
> > +#include <linux/string.h>
> > +#include <linux/kvm_host.h>
> > +#include <linux/highmem.h>
> > +#include <asm/mmu-44x.h>
> > +#include <asm/kvm_ppc.h>
> > +
> > +#include "44x_tlb.h"
> > +
> > +#define PPC44x_TLB_USER_PERM_MASK
> > (PPC44x_TLB_UX|PPC44x_TLB_UR|PPC44x_TLB_UW) +#define
> > PPC44x_TLB_SUPER_PERM_MASK (PPC44x_TLB_SX|PPC44x_TLB_SR|PPC44x_TLB_SW) +
> > +static unsigned int kvmppc_tlb_44x_pos;
> > +
> > +static u32 kvmppc_44x_tlb_shadow_attrib(u32 attrib, int usermode)
> > +{
> > + /* XXX remove mask when Linux is fixed */
> > + attrib &= 0xf03f;
>
> What does that mean? Is this the "44x TLB handler writes to reserved
> fields" issue? If so, could you comment that a little more verbosely?
Yup, you're right. Actually, what I should really do is this:
attrib &= PPC44x_TLB_ATTR_MASK | PPC44x_TLB_PERM_MASK;
> Or you could just send a patch to fix Linux... ;).
I had a look at it once, but didn't have time to grok the assembly bit
manipulations. I guess nobody else has either. :)
> > +
> > + if (!usermode) {
> > + /* Guest is in supervisor mode, so we need to translate guest
> > + * supervisor permissions into user permissions. */
> > + attrib &= ~PPC44x_TLB_USER_PERM_MASK;
> > + attrib |= (attrib & PPC44x_TLB_SUPER_PERM_MASK) << 3;
> > + }
> > +
> > + /* Make sure host can always access this memory. */
> > + attrib |= PPC44x_TLB_SX|PPC44x_TLB_SR|PPC44x_TLB_SW;
> > +
> > + return attrib;
> > +}
> > +
>
> <snip>
>
> > diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
> > new file mode 100644
> > --- /dev/null
> > +++ b/arch/powerpc/kvm/emulate.c
> > @@ -0,0 +1,753 @@
> > +/*
> > + * 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 IBM Corp. 2007
> > + *
> > + * Authors: Hollis Blanchard <hollisb@us.ibm.com>
> > + */
> > +
> > +#include <linux/jiffies.h>
> > +#include <linux/timer.h>
> > +#include <linux/types.h>
> > +#include <linux/string.h>
> > +#include <linux/kvm_host.h>
> > +
> > +#include <asm/dcr.h>
> > +#include <asm/time.h>
> > +#include <asm/byteorder.h>
> > +#include <asm/kvm_ppc.h>
>
> #include <asm/dcr-regs.h>
>
> > +
> > +#include "44x_tlb.h"
> > +
> > +#define DCRN_CPR0_CFGADDR 0xc
> > +#define DCRN_CPR0_CFGDATA 0xd
>
> Remove these.
OK.
> <snip>
>
> > +int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu
> > *vcpu) +{
> > + u32 inst = vcpu->arch.last_inst;
> > + u32 ea;
> > + int ra;
> > + int rb;
> > + int rc;
> > + int rs;
> > + int rt;
> > + int sprn;
> > + int dcrn;
> > + enum emulation_result emulated = EMULATE_DONE;
> > + int advance = 1;
> > +
> > + switch (get_op(inst)) {
>
> <snip>
>
> > + case 323: /* mfdcr */
> > + dcrn = get_dcrn(inst);
> > + rt = get_rt(inst);
> > +
> > + /* emulate some access in kernel */
> > + switch (dcrn) {
> > + case DCRN_CPR0_CFGADDR:
> > + vcpu->arch.gpr[rt] = vcpu->arch.cpr0_cfgaddr;
> > + break;
> > + case DCRN_CPR0_CFGDATA:
> > + local_irq_disable();
> > + mtdcr(DCRN_CPR0_CFGADDR,
> > + vcpu->arch.cpr0_cfgaddr);
> > + vcpu->arch.gpr[rt] = mfdcr(DCRN_CPR0_CFGDATA);
> > + local_irq_enable();
> > + break;
> > + default:
> > + run->dcr.dcrn = dcrn;
> > + run->dcr.data = 0;
> > + run->dcr.is_write = 0;
> > + vcpu->arch.io_gpr = rt;
> > + vcpu->arch.dcr_needed = 1;
> > + emulated = EMULATE_DO_DCR;
> > + }
> > +
> > + break;
>
> <snip>
>
> > + case 451: /* mtdcr */
> > + dcrn = get_dcrn(inst);
> > + rs = get_rs(inst);
> > +
> > + /* emulate some access in kernel */
> > + switch (dcrn) {
> > + case DCRN_CPR0_CFGADDR:
> > + vcpu->arch.cpr0_cfgaddr = vcpu->arch.gpr[rs];
> > + break;
> > + default:
> > + run->dcr.dcrn = dcrn;
> > + run->dcr.data = vcpu->arch.gpr[rs];
> > + run->dcr.is_write = 1;
> > + vcpu->arch.dcr_needed = 1;
> > + emulated = EMULATE_DO_DCR;
> > + }
> > +
> > + break;
>
> What exactly are those doing? I'm confused as to why the CPR0 DCRs are
> special cased. Also, you don't seem to actually be doing much on
> storing guest DCRs, so can I assume they aren't really needed at
> runtime?
A 440 cuImage accesses CPR0 DCRs to determine the timebase frequency.
Ordinarily, these DCR accesses would be handed to userspace (qemu) to
emulate.
To emulate this in userspace, we could extract the TB freq from the
host's /proc/device-tree (userspace can't directly access DCRs), but honestly
I didn't want to deal with the math necessary to decompose that back into the
various CPR0 register fields. So instead I special-case CPR0 reads in the
kernel.
The guest needs to know the real/host timebase frequency anyways, since it can
execute mftb without host involvement, and will get very confused if the
timebase increments at a different frequency than it expects.
--
Hollis Blanchard
IBM Linux Technology Center
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox