* Re: kvm PCI assignment & VFIO ramblings
From: Alex Williamson @ 2011-08-02 18:14 UTC (permalink / raw)
To: David Gibson
Cc: aafabbri, Alexey Kardashevskiy, kvm, Paul Mackerras, qemu-devel,
chrisw, iommu, Anthony Liguori, linux-pci@vger.kernel.org,
linuxppc-dev, benve
In-Reply-To: <20110802082848.GD29719@yookeroo.fritz.box>
On Tue, 2011-08-02 at 18:28 +1000, David Gibson wrote:
> On Sat, Jul 30, 2011 at 12:20:08PM -0600, Alex Williamson wrote:
> > On Sat, 2011-07-30 at 09:58 +1000, Benjamin Herrenschmidt wrote:
> [snip]
> > On x86, the USB controllers don't typically live behind a PCIe-to-PCI
> > bridge, so don't suffer the source identifier problem, but they do often
> > share an interrupt. But even then, we can count on most modern devices
> > supporting PCI2.3, and thus the DisINTx feature, which allows us to
> > share interrupts. In any case, yes, it's more rare but we need to know
> > how to handle devices behind PCI bridges. However I disagree that we
> > need to assign all the devices behind such a bridge to the guest.
> > There's a difference between removing the device from the host and
> > exposing the device to the guest.
>
> I think you're arguing only over details of what words to use for
> what, rather than anything of substance here. The point is that an
> entire partitionable group must be assigned to "host" (in which case
> kernel drivers may bind to it) or to a particular guest partition (or
> at least to a single UID on the host). Which of the assigned devices
> the partition actually uses is another matter of course, as is at
> exactly which level they become "de-exposed" if you don't want to use
> all of then.
Well first we need to define what a partitionable group is, whether it's
based on hardware requirements or user policy. And while I agree that
we need unique ownership of a partition, I disagree that qemu is
necessarily the owner of the entire partition vs individual devices.
But feel free to dismiss it as unsubstantial.
> [snip]
> > > Maybe something like /sys/devgroups ? This probably warrants involving
> > > more kernel people into the discussion.
> >
> > I don't yet buy into passing groups to qemu since I don't buy into the
> > idea of always exposing all of those devices to qemu. Would it be
> > sufficient to expose iommu nodes in sysfs that link to the devices
> > behind them and describe properties and capabilities of the iommu
> > itself? More on this at the end.
>
> Again, I don't think you're making a distinction of any substance.
> Ben is saying the group as a whole must be set to allow partition
> access, whether or not you call that "assigning". There's no reason
> that passing a sysfs descriptor to qemu couldn't be the qemu
> developer's quick-and-dirty method of putting the devices in, while
> also allowing full assignment of the devices within the groups by
> libvirt.
Well, there is a reason for not passing a sysfs descriptor to qemu if
qemu isn't the one defining the policy about how the members of that
group are exposed. I tend to envision a userspace entity defining
policy and granting devices to qemu. Do we really want separate
developer vs production interfaces?
> [snip]
> > > Now some of this can be fixed with tweaks, and we've started doing it
> > > (we have a working pass-through using VFIO, forgot to mention that, it's
> > > just that we don't like what we had to do to get there).
> >
> > This is a result of wanting to support *unmodified* x86 guests. We
> > don't have the luxury of having a predefined pvDMA spec that all x86
> > OSes adhere to. The 32bit problem is unfortunate, but the priority use
> > case for assigning devices to guests is high performance I/O, which
> > usually entails modern, 64bit hardware. I'd like to see us get to the
> > point of having emulated IOMMU hardware on x86, which could then be
> > backed by VFIO, but for now guest pinning is the most practical and
> > useful.
>
> No-one's suggesting that this isn't a valid mode of operation. It's
> just that right now conditionally disabling it for us is fairly ugly
> because of the way the qemu code is structured.
It really shouldn't be any more than skipping the
cpu_register_phys_memory_client() and calling the map/unmap routines
elsewhere.
> [snip]
> > > - I don't like too much the fact that VFIO provides yet another
> > > different API to do what we already have at least 2 kernel APIs for, ie,
> > > BAR mapping and config space access. At least it should be better at
> > > using the backend infrastructure of the 2 others (sysfs & procfs). I
> > > understand it wants to filter in some case (config space) and -maybe-
> > > yet another API is the right way to go but allow me to have my doubts.
> >
> > The use of PCI sysfs is actually one of my complaints about current
> > device assignment. To do assignment with an unprivileged guest we need
> > to open the PCI sysfs config file for it, then change ownership on a
> > handful of other PCI sysfs files, then there's this other pci-stub thing
> > to maintain ownership, but the kvm ioctls don't actually require it and
> > can grab onto any free device... We are duplicating some of that in
> > VFIO, but we also put the ownership of the device behind a single device
> > file. We do have the uiommu problem that we can't give an unprivileged
> > user ownership of that, but your usage model may actually make that
> > easier. More below...
>
> Hrm. I was assuming that a sysfs groups interface would provide a
> single place to set the ownership of the whole group. Whether that's
> a echoing a uid to a magic file or doing or chown on the directory or
> whatever is a matter of details.
Except one of those details is whether we manage the group in sysfs or
just expose enough information in sysfs for another userspace entity to
manage the devices. Where do we manage enforcement of hardware policy
vs userspace policy?
> [snip]
> > I spent a lot of time looking for an architecture neutral solution here,
> > but I don't think it exists. Please prove me wrong. The problem is
> > that we have to disable INTx on an assigned device after it fires (VFIO
> > does this automatically). If we don't do this, a non-responsive or
> > malicious guest could sit on the interrupt, causing it to fire
> > repeatedly as a DoS on the host. The only indication that we can rely
> > on to re-enable INTx is when the guest CPU writes an EOI to the APIC.
> > We can't just wait for device accesses because a) the device CSRs are
> > (hopefully) direct mapped and we'd have to slow map them or attempt to
> > do some kind of dirty logging to detect when they're accesses b) what
> > constitutes an interrupt service is device specific.
> >
> > That means we need to figure out how PCI interrupt 'A' (or B...)
> > translates to a GSI (Global System Interrupt - ACPI definition, but
> > hopefully a generic concept). That GSI identifies a pin on an IOAPIC,
> > which will also see the APIC EOI. And just to spice things up, the
> > guest can change the PCI to GSI mappings via ACPI. I think the set of
> > callbacks I've added are generic (maybe I left ioapic in the name), but
> > yes they do need to be implemented for other architectures. Patches
> > appreciated from those with knowledge of the systems and/or access to
> > device specs. This is the only reason that I make QEMU VFIO only build
> > for x86.
>
> There will certainly need to be some arch hooks here, but it can be
> made less intrusively x86 specific without too much difficulty.
> e.g. Create an EOF notifier chain in qemu - the master PICs (APIC for
> x86, XICS for pSeries) for all vfio capable machines need to kick it,
> and vfio subscribes.
Am I the only one that sees ioapic_add/remove_gsi_eoi_notifier() in the
qemu/vfio patch series? Shoot me for using ioapic in the name, but it's
exactly what you ask for. It just needs to be made a common service and
implemented for power.
> [snip]
> > Rather than your "groups" idea, I've been mulling over whether we can
> > just expose the dependencies, configuration, and capabilities in sysfs
> > and build qemu commandlines to describe it. For instance, if we simply
> > start with creating iommu nodes in sysfs, we could create links under
> > each iommu directory to the devices behind them. Some kind of
> > capability file could define properties like whether it's page table
> > based or fixed iova window or the granularity of mapping the devices
> > behind it. Once we have that, we could probably make uiommu attach to
> > each of those nodes.
>
> Well, that would address our chief concern that inherently tying the
> lifetime of a domain to an fd is problematic. In fact, I don't really
> see how this differs from the groups proposal except in the details of
> how you inform qemu of the group^H^H^H^H^Hiommu domain.
One implies group policy, configuration and management in sysfs, the
other exposes the hardware dependencies in sysfs and leaves the rest for
someone else (libvirt). Thanks,
Alex
^ permalink raw reply
* Re: kvm PCI assignment & VFIO ramblings
From: Alex Williamson @ 2011-08-02 18:35 UTC (permalink / raw)
To: David Gibson
Cc: aafabbri, Alexey Kardashevskiy, kvm, Paul Mackerras, qemu-devel,
chrisw, iommu, Anthony Liguori, linux-pci@vger.kernel.org,
linuxppc-dev, benve
In-Reply-To: <1312308847.2653.467.camel@bling.home>
On Tue, 2011-08-02 at 12:14 -0600, Alex Williamson wrote:
> On Tue, 2011-08-02 at 18:28 +1000, David Gibson wrote:
> > On Sat, Jul 30, 2011 at 12:20:08PM -0600, Alex Williamson wrote:
> > > On Sat, 2011-07-30 at 09:58 +1000, Benjamin Herrenschmidt wrote:
> > [snip]
> > > On x86, the USB controllers don't typically live behind a PCIe-to-PCI
> > > bridge, so don't suffer the source identifier problem, but they do often
> > > share an interrupt. But even then, we can count on most modern devices
> > > supporting PCI2.3, and thus the DisINTx feature, which allows us to
> > > share interrupts. In any case, yes, it's more rare but we need to know
> > > how to handle devices behind PCI bridges. However I disagree that we
> > > need to assign all the devices behind such a bridge to the guest.
> > > There's a difference between removing the device from the host and
> > > exposing the device to the guest.
> >
> > I think you're arguing only over details of what words to use for
> > what, rather than anything of substance here. The point is that an
> > entire partitionable group must be assigned to "host" (in which case
> > kernel drivers may bind to it) or to a particular guest partition (or
> > at least to a single UID on the host). Which of the assigned devices
> > the partition actually uses is another matter of course, as is at
> > exactly which level they become "de-exposed" if you don't want to use
> > all of then.
>
> Well first we need to define what a partitionable group is, whether it's
> based on hardware requirements or user policy. And while I agree that
> we need unique ownership of a partition, I disagree that qemu is
> necessarily the owner of the entire partition vs individual devices.
Sorry, I didn't intend to have such circular logic. "... I disagree
that qemu is necessarily the owner of the entire partition vs granted
access to devices within the partition". Thanks,
Alex
^ permalink raw reply
* [PATCH] powerpc/64e: External Proxy interrupt support
From: Scott Wood @ 2011-08-02 19:44 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev
Adds support for External Proxy (a.k.a. CoreInt) interrupts on 64-bit
kernels. External Proxy combines interrupt delivery and
acknowledgement, so simply returning from the interrupt without EOI
or other action will not result in the interrupt being reasserted.
When an external interrupt is deferred in this manner (whether
external proxy is used or not), we set a flag in the PACA. When we
re-enable interrupts, either explicitly or as part of an exception
return, we check the flag and branch to the interrupt exception
vector as if hardware had delivered the interrupt.
Another approach I considered was to use doorbells to replay the
interrupt. There are some problems with this:
- The timing of the actual delivery of the doorbell is undefined.
This means we can't be sure in an architected way that the
doorbell will happen before interrupts are again soft-disabled, at
which point (barring interrupt-controller specific actions such as
raising CTPR) we could take a higher priority interrupt and
overwrite the saved EPR.
- Doorbells have a lower priority than true external interrupt. This
means you could have a lower priority interrupt appear to preempt
a higher prio interrupt, once the higher priority interrupt
enables EE and the doorbell comes in.
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
arch/powerpc/include/asm/irq.h | 2 +
arch/powerpc/include/asm/paca.h | 4 ++
arch/powerpc/kernel/asm-offsets.c | 3 +
arch/powerpc/kernel/entry_64.S | 4 ++
arch/powerpc/kernel/exceptions-64e.S | 94 +++++++++++++++++++++++++++-----
arch/powerpc/kernel/irq.c | 11 ++++
arch/powerpc/platforms/85xx/p5020_ds.c | 5 --
7 files changed, 103 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/include/asm/irq.h b/arch/powerpc/include/asm/irq.h
index c57a28e..c0a45e7 100644
--- a/arch/powerpc/include/asm/irq.h
+++ b/arch/powerpc/include/asm/irq.h
@@ -332,5 +332,7 @@ extern void do_IRQ(struct pt_regs *regs);
int irq_choose_cpu(const struct cpumask *mask);
+void deliver_pending_irq(void);
+
#endif /* _ASM_IRQ_H */
#endif /* __KERNEL__ */
diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index c1f65f5..e5af3e3 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
@@ -134,6 +134,10 @@ struct paca_struct {
u8 hard_enabled; /* set if irqs are enabled in MSR */
u8 io_sync; /* writel() needs spin_unlock sync */
u8 irq_work_pending; /* IRQ_WORK interrupt while soft-disable */
+#ifdef CONFIG_PPC_BOOK3E
+ /* an irq is pending while soft-disabled */
+ u8 irq_pending;
+#endif
/* Stuff for accurate time accounting */
u64 user_time; /* accumulated usermode TB ticks */
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index c98144f..5082ee7 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -206,6 +206,9 @@ int main(void)
DEFINE(SVCPU_SLB, offsetof(struct kvmppc_book3s_shadow_vcpu, slb));
DEFINE(SVCPU_SLB_MAX, offsetof(struct kvmppc_book3s_shadow_vcpu, slb_max));
#endif
+#ifdef CONFIG_PPC_BOOK3E
+ DEFINE(PACA_IRQ_PENDING, offsetof(struct paca_struct, irq_pending));
+#endif
#endif /* CONFIG_PPC64 */
/* RTAS */
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index d834425..c1d8eea 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -596,6 +596,9 @@ _GLOBAL(ret_from_except_lite)
restore:
BEGIN_FW_FTR_SECTION
ld r5,SOFTE(r1)
+#ifdef CONFIG_PPC_BOOK3E
+ lbz r6,PACA_IRQ_PENDING(r13)
+#endif
FW_FTR_SECTION_ELSE
b .Liseries_check_pending_irqs
ALT_FW_FTR_SECTION_END_IFCLR(FW_FEATURE_ISERIES)
@@ -608,6 +611,7 @@ ALT_FW_FTR_SECTION_END_IFCLR(FW_FEATURE_ISERIES)
stb r4,PACAHARDIRQEN(r13)
#ifdef CONFIG_PPC_BOOK3E
+ /* consumes r3-r6 */
b .exception_return_book3e
#else
ld r4,_CTR(r1)
diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index 429983c..9886be9 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -2,6 +2,7 @@
* Boot code and exception vectors for Book3E processors
*
* Copyright (C) 2007 Ben. Herrenschmidt (benh@kernel.crashing.org), IBM Corp.
+ * Copyright 2011 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
@@ -125,6 +126,10 @@
cmpwi cr0,r11,0; /* yes -> go out of line */ \
beq masked_doorbell_book3e
+#define PROLOG_ADDITION_EXTIRQ_GEN \
+ lbz r11,PACASOFTIRQEN(r13); /* are irqs soft-disabled ? */ \
+ cmpwi cr0,r11,0; /* yes -> go out of line */ \
+ beq masked_extirq_book3e
/* Core exception code for all exceptions except TLB misses.
* XXX: Needs to make SPRN_SPRG_GEN depend on exception type
@@ -325,7 +330,13 @@ interrupt_end_book3e:
b storage_fault_common
/* External Input Interrupt */
- MASKABLE_EXCEPTION(0x500, external_input, .do_IRQ, ACK_NONE)
+ START_EXCEPTION(external_input)
+ NORMAL_EXCEPTION_PROLOG(0x500, PROLOG_ADDITION_EXTIRQ)
+ EXCEPTION_COMMON(0x500, PACA_EXGEN, INTS_DISABLE_ALL)
+ CHECK_NAPPING()
+ addi r3,r1,STACK_FRAME_OVERHEAD
+ bl .do_IRQ
+ b .ret_from_except_lite
/* Alignment */
START_EXCEPTION(alignment);
@@ -557,6 +568,12 @@ kernel_dbg_exc:
* An interrupt came in while soft-disabled; clear EE in SRR1,
* clear paca->hard_enabled and return.
*/
+masked_extirq_book3e:
+ mtcr r10
+ li r10,1
+ stb r10,PACA_IRQ_PENDING(r13)
+ b masked_interrupt_book3e_common
+
masked_doorbell_book3e:
mtcr r10
/* Resend the doorbell to fire again when ints enabled */
@@ -618,20 +635,8 @@ alignment_more:
bl .alignment_exception
b .ret_from_except
-/*
- * We branch here from entry_64.S for the last stage of the exception
- * return code path. MSR:EE is expected to be off at that point
- */
-_GLOBAL(exception_return_book3e)
- b 1f
-
-/* This is the return from load_up_fpu fast path which could do with
- * less GPR restores in fact, but for now we have a single return path
- */
- .globl fast_exception_return
-fast_exception_return:
- wrteei 0
-1: mr r0,r13
+.macro exception_restore
+ mr r0,r13
ld r10,_MSR(r1)
REST_4GPRS(2, r1)
andi. r6,r10,MSR_PR
@@ -667,8 +672,67 @@ fast_exception_return:
ld r10,PACA_EXGEN+EX_R10(r13)
ld r11,PACA_EXGEN+EX_R11(r13)
mfspr r13,SPRN_SPRG_GEN_SCRATCH
+.endm
+
+/*
+ * We branch here from entry_64.S for the last stage of the exception
+ * return code path. MSR:EE is expected to be off at that point
+ * r3 = MSR for return context
+ * r4 = hard irq-enable status for return context
+ * r5 = soft irq-enable status for return context
+ * r6 = irq pending flag
+ */
+_GLOBAL(exception_return_book3e)
+ cmpwi r6,0
+ beq common_exception_return
+
+/*
+ * There's an interrupt pending. If we're returning to a context that
+ * is soft-irq-enabled, we need to deliver the interrupt now.
+ *
+ * We should never get here with soft IRQs enabled but hard IRQs disabled,
+ * but just to be sure, check that too.
+ */
+ cmpwi r5,0
+ beq common_exception_return
+ cmpwi r4,0
+ beq common_exception_return
+
+ lis r5,(MSR_CE | MSR_ME | MSR_DE)@h
+ li r4,0
+ ori r5,r5,(MSR_CE | MSR_ME | MSR_DE)@l
+ stb r4,PACA_IRQ_PENDING(r13)
+ and r5,r5,r3
+ oris r5,r5,MSR_CM@h
+ mtmsr r5
+
+ exception_restore
+ b exc_external_input_book3e
+
+/* This is the return from load_up_fpu fast path which could do with
+ * less GPR restores in fact, but for now we have a single return path
+ */
+ .globl fast_exception_return
+fast_exception_return:
+ wrteei 0
+common_exception_return:
+ exception_restore
rfi
+/* Called from arch_local_irq_restore() prior to hard-enabling interrupts */
+_GLOBAL(deliver_pending_irq)
+ mflr r3
+ mfmsr r4
+ lis r5,(MSR_CM | MSR_CE | MSR_ME | MSR_DE)@h
+ ori r5,r5,(MSR_CM | MSR_CE | MSR_ME | MSR_DE)@l
+ and r5,r5,r4
+ ori r4,r4,MSR_EE
+
+ mtspr SPRN_SRR0,r3
+ mtspr SPRN_SRR1,r4
+ mtmsr r5
+ b exc_external_input_book3e
+
/*
* Trampolines used when spotting a bad kernel stack pointer in
* the exception entry code.
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index d281fb6..44a23d0 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -184,6 +184,17 @@ notrace void arch_local_irq_restore(unsigned long en)
lv1_get_version_info(&tmp);
}
+#ifdef CONFIG_PPC_BOOK3E
+ /*
+ * If there's a pending IRQ, deliver it now. Interrupts
+ * will be hard-enabled on return.
+ */
+ if (get_paca()->irq_pending) {
+ get_paca()->irq_pending = 0;
+ deliver_pending_irq();
+ }
+#endif
+
__hard_irq_enable();
}
EXPORT_SYMBOL(arch_local_irq_restore);
diff --git a/arch/powerpc/platforms/85xx/p5020_ds.c b/arch/powerpc/platforms/85xx/p5020_ds.c
index e8cba50..87e7d29 100644
--- a/arch/powerpc/platforms/85xx/p5020_ds.c
+++ b/arch/powerpc/platforms/85xx/p5020_ds.c
@@ -76,12 +76,7 @@ define_machine(p5020_ds) {
#ifdef CONFIG_PCI
.pcibios_fixup_bus = fsl_pcibios_fixup_bus,
#endif
-/* coreint doesn't play nice with lazy EE, use legacy mpic for now */
-#ifdef CONFIG_PPC64
- .get_irq = mpic_get_irq,
-#else
.get_irq = mpic_get_coreint_irq,
-#endif
.restart = fsl_rstcr_restart,
.calibrate_decr = generic_calibrate_decr,
.progress = udbg_progress,
--
1.7.4.1
^ permalink raw reply related
* RE: [Cbe-oss-dev] [PATCH 09/15] ps3: Limit theb number of regions per storage device
From: dm @ 2011-08-02 19:55 UTC (permalink / raw)
To: 'Duait'
Cc: 'Geoff Levand', cbe-oss-dev, 'Hector Martin',
linuxppc-dev
Sent from my HTC Touch HD
-----Original Message-----
From: linuxppc-dev-bounces+dm=3Dmdtech.ru@lists.ozlabs.org =
<linuxppc-dev-bounces+dm=3Dmdtech.ru@lists.ozlabs.org>
Sent: 01 =D0=B0=D0=B2=D0=B3=D1=83=D1=81=D1=82=D0=B0 2011 23:58
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Geoff Levand <geoff@infradead.org>; cbe-oss-dev@lists.ozlabs.org =
<cbe-oss-dev@lists.ozlabs.org>; Hector Martin <hector@marcansoft.com>; =
linuxppc-dev@lists.ozlabs.org <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [Cbe-oss-dev] [PATCH 09/15] ps3: Limit the number of =
regions per storage device
On Mon, Aug 1, 2011 at 10:30 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Mon, Aug 1, 2011 at 22:03, Andre Heider <a.heider@gmail.com> wrote:
>> There can be only 8 regions, add a sanity check
>
> Why can there be only 8 regions?
I believe lv1 limits it to 8? I might be mistaken here, it mostly is a
check for the patches after this one
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-d
^ permalink raw reply
* Re: kvm PCI assignment & VFIO ramblings
From: Konrad Rzeszutek Wilk @ 2011-08-02 21:29 UTC (permalink / raw)
To: Alex Williamson
Cc: Alexey Kardashevskiy, kvm, Paul Mackerras, David Gibson,
Avi Kivity, Anthony Liguori, linux-pci@vger.kernel.org,
linuxppc-dev
In-Reply-To: <1312299299.2653.429.camel@bling.home>
On Tue, Aug 02, 2011 at 09:34:58AM -0600, Alex Williamson wrote:
> On Tue, 2011-08-02 at 22:58 +1000, Benjamin Herrenschmidt wrote:
> >
> > Don't worry, it took me a while to get my head around the HW :-) SR-IOV
> > VFs will generally not have limitations like that no, but on the other
> > hand, they -will- still require 1 VF = 1 group, ie, you won't be able to
> > take a bunch of VFs and put them in the same 'domain'.
> >
> > I think the main deal is that VFIO/qemu sees "domains" as "guests" and
> > tries to put all devices for a given guest into a "domain".
>
> Actually, that's only a recent optimization, before that each device got
> it's own iommu domain. It's actually completely configurable on the
> qemu command line which devices get their own iommu and which share.
> The default optimizes the number of domains (one) and thus the number of
> mapping callbacks since we pin the entire guest.
>
> > On POWER, we have a different view of things were domains/groups are
> > defined to be the smallest granularity we can (down to a single VF) and
> > we give several groups to a guest (ie we avoid sharing the iommu in most
> > cases)
> >
> > This is driven by the HW design but that design is itself driven by the
> > idea that the domains/group are also error isolation groups and we don't
> > want to take all of the IOs of a guest down if one adapter in that guest
> > is having an error.
> >
> > The x86 domains are conceptually different as they are about sharing the
> > iommu page tables with the clear long term intent of then sharing those
> > page tables with the guest CPU own. We aren't going in that direction
> > (at this point at least) on POWER..
>
> Yes and no. The x86 domains are pretty flexible and used a few
> different ways. On the host we do dynamic DMA with a domain per device,
> mapping only the inflight DMA ranges. In order to achieve the
> transparent device assignment model, we have to flip that around and map
> the entire guest. As noted, we can continue to use separate domains for
> this, but since each maps the entire guest, it doesn't add a lot of
> value and uses more resources and requires more mapping callbacks (and
> x86 doesn't have the best error containment anyway). If we had a well
> supported IOMMU model that we could adapt for pvDMA, then it would make
> sense to keep each device in it's own domain again. Thanks,
Could you have an PV IOMMU (in the guest) that would set up those
maps?
^ permalink raw reply
* Re: kvm PCI assignment & VFIO ramblings
From: Alex Williamson @ 2011-08-03 1:02 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Alexey Kardashevskiy, kvm, Paul Mackerras, David Gibson,
Avi Kivity, Anthony Liguori, linux-pci@vger.kernel.org,
linuxppc-dev
In-Reply-To: <20110802212949.GB18496@dumpdata.com>
On Tue, 2011-08-02 at 17:29 -0400, Konrad Rzeszutek Wilk wrote:
> On Tue, Aug 02, 2011 at 09:34:58AM -0600, Alex Williamson wrote:
> > On Tue, 2011-08-02 at 22:58 +1000, Benjamin Herrenschmidt wrote:
> > >
> > > Don't worry, it took me a while to get my head around the HW :-) SR-IOV
> > > VFs will generally not have limitations like that no, but on the other
> > > hand, they -will- still require 1 VF = 1 group, ie, you won't be able to
> > > take a bunch of VFs and put them in the same 'domain'.
> > >
> > > I think the main deal is that VFIO/qemu sees "domains" as "guests" and
> > > tries to put all devices for a given guest into a "domain".
> >
> > Actually, that's only a recent optimization, before that each device got
> > it's own iommu domain. It's actually completely configurable on the
> > qemu command line which devices get their own iommu and which share.
> > The default optimizes the number of domains (one) and thus the number of
> > mapping callbacks since we pin the entire guest.
> >
> > > On POWER, we have a different view of things were domains/groups are
> > > defined to be the smallest granularity we can (down to a single VF) and
> > > we give several groups to a guest (ie we avoid sharing the iommu in most
> > > cases)
> > >
> > > This is driven by the HW design but that design is itself driven by the
> > > idea that the domains/group are also error isolation groups and we don't
> > > want to take all of the IOs of a guest down if one adapter in that guest
> > > is having an error.
> > >
> > > The x86 domains are conceptually different as they are about sharing the
> > > iommu page tables with the clear long term intent of then sharing those
> > > page tables with the guest CPU own. We aren't going in that direction
> > > (at this point at least) on POWER..
> >
> > Yes and no. The x86 domains are pretty flexible and used a few
> > different ways. On the host we do dynamic DMA with a domain per device,
> > mapping only the inflight DMA ranges. In order to achieve the
> > transparent device assignment model, we have to flip that around and map
> > the entire guest. As noted, we can continue to use separate domains for
> > this, but since each maps the entire guest, it doesn't add a lot of
> > value and uses more resources and requires more mapping callbacks (and
> > x86 doesn't have the best error containment anyway). If we had a well
> > supported IOMMU model that we could adapt for pvDMA, then it would make
> > sense to keep each device in it's own domain again. Thanks,
>
> Could you have an PV IOMMU (in the guest) that would set up those
> maps?
Yep, definitely. That's effectively what power wants to do. We could
do it on x86, but as others have noted, the map/unmap interface isn't
tuned to do this at that granularity and our target guest OS audience is
effectively reduced to Linux. Thanks,
Alex
^ permalink raw reply
* Re: kvm PCI assignment & VFIO ramblings
From: David Gibson @ 2011-08-03 2:04 UTC (permalink / raw)
To: Alex Williamson
Cc: chrisw, Alexey Kardashevskiy, kvm, Paul Mackerras,
linux-pci@vger.kernel.org, qemu-devel, aafabbri, iommu,
Anthony Liguori, linuxppc-dev, benve
In-Reply-To: <1312310121.2653.470.camel@bling.home>
On Tue, Aug 02, 2011 at 12:35:19PM -0600, Alex Williamson wrote:
> On Tue, 2011-08-02 at 12:14 -0600, Alex Williamson wrote:
> > On Tue, 2011-08-02 at 18:28 +1000, David Gibson wrote:
> > > On Sat, Jul 30, 2011 at 12:20:08PM -0600, Alex Williamson wrote:
> > > > On Sat, 2011-07-30 at 09:58 +1000, Benjamin Herrenschmidt wrote:
> > > [snip]
> > > > On x86, the USB controllers don't typically live behind a PCIe-to-PCI
> > > > bridge, so don't suffer the source identifier problem, but they do often
> > > > share an interrupt. But even then, we can count on most modern devices
> > > > supporting PCI2.3, and thus the DisINTx feature, which allows us to
> > > > share interrupts. In any case, yes, it's more rare but we need to know
> > > > how to handle devices behind PCI bridges. However I disagree that we
> > > > need to assign all the devices behind such a bridge to the guest.
> > > > There's a difference between removing the device from the host and
> > > > exposing the device to the guest.
> > >
> > > I think you're arguing only over details of what words to use for
> > > what, rather than anything of substance here. The point is that an
> > > entire partitionable group must be assigned to "host" (in which case
> > > kernel drivers may bind to it) or to a particular guest partition (or
> > > at least to a single UID on the host). Which of the assigned devices
> > > the partition actually uses is another matter of course, as is at
> > > exactly which level they become "de-exposed" if you don't want to use
> > > all of then.
> >
> > Well first we need to define what a partitionable group is, whether it's
> > based on hardware requirements or user policy. And while I agree that
> > we need unique ownership of a partition, I disagree that qemu is
> > necessarily the owner of the entire partition vs individual devices.
>
> Sorry, I didn't intend to have such circular logic. "... I disagree
> that qemu is necessarily the owner of the entire partition vs granted
> access to devices within the partition". Thanks,
I still don't understand the distinction you're making. We're saying
the group is "owned" by a given user or guest in the sense that no-one
else may use anything in the group (including host drivers). At that
point none, some or all of the devices in the group may actually be
used by the guest.
You seem to be making a distinction between "owned by" and "assigned
to" and "used by" and I really don't see what it is.
--
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/3] KVM: PPC: Implement H_CEDE hcall for book3s_hv in real-mode code
From: Paul Mackerras @ 2011-08-03 3:31 UTC (permalink / raw)
To: Alexander Graf; +Cc: linuxppc-dev, kvm-ppc
In-Reply-To: <4E380DEC.8030803@suse.de>
On Tue, Aug 02, 2011 at 04:47:08PM +0200, Alexander Graf wrote:
> > int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu, struct kvm_interrupt *irq)
> > {
> >- if (irq->irq == KVM_INTERRUPT_UNSET)
> >+ if (irq->irq == KVM_INTERRUPT_UNSET) {
> > kvmppc_core_dequeue_external(vcpu, irq);
> >- else
> >- kvmppc_core_queue_external(vcpu, irq);
> >+ return 0;
> >+ }
>
> Not sure I understand this part. Mind to explain?
It's a micro-optimization - we don't really need to wake up or
interrupt the vcpu thread when we're clearing the interrupt.
Unless of course I'm missing something... :)
>
> Alex
>
> >+
> >+ kvmppc_core_queue_external(vcpu, irq);
> >
> >- if (waitqueue_active(&vcpu->wq)) {
> >- wake_up_interruptible(&vcpu->wq);
> >+ if (waitqueue_active(vcpu->arch.wqp)) {
> >+ wake_up_interruptible(vcpu->arch.wqp);
> > vcpu->stat.halt_wakeup++;
> > } else if (vcpu->cpu != -1) {
> > smp_send_reschedule(vcpu->cpu);
Paul.
^ permalink raw reply
* Re: kvm PCI assignment & VFIO ramblings
From: Alex Williamson @ 2011-08-03 3:44 UTC (permalink / raw)
To: David Gibson
Cc: chrisw, Alexey Kardashevskiy, kvm, Paul Mackerras,
linux-pci@vger.kernel.org, qemu-devel, aafabbri, iommu,
Anthony Liguori, linuxppc-dev, benve
In-Reply-To: <20110803020422.GF29719@yookeroo.fritz.box>
On Wed, 2011-08-03 at 12:04 +1000, David Gibson wrote:
> On Tue, Aug 02, 2011 at 12:35:19PM -0600, Alex Williamson wrote:
> > On Tue, 2011-08-02 at 12:14 -0600, Alex Williamson wrote:
> > > On Tue, 2011-08-02 at 18:28 +1000, David Gibson wrote:
> > > > On Sat, Jul 30, 2011 at 12:20:08PM -0600, Alex Williamson wrote:
> > > > > On Sat, 2011-07-30 at 09:58 +1000, Benjamin Herrenschmidt wrote:
> > > > [snip]
> > > > > On x86, the USB controllers don't typically live behind a PCIe-to-PCI
> > > > > bridge, so don't suffer the source identifier problem, but they do often
> > > > > share an interrupt. But even then, we can count on most modern devices
> > > > > supporting PCI2.3, and thus the DisINTx feature, which allows us to
> > > > > share interrupts. In any case, yes, it's more rare but we need to know
> > > > > how to handle devices behind PCI bridges. However I disagree that we
> > > > > need to assign all the devices behind such a bridge to the guest.
> > > > > There's a difference between removing the device from the host and
> > > > > exposing the device to the guest.
> > > >
> > > > I think you're arguing only over details of what words to use for
> > > > what, rather than anything of substance here. The point is that an
> > > > entire partitionable group must be assigned to "host" (in which case
> > > > kernel drivers may bind to it) or to a particular guest partition (or
> > > > at least to a single UID on the host). Which of the assigned devices
> > > > the partition actually uses is another matter of course, as is at
> > > > exactly which level they become "de-exposed" if you don't want to use
> > > > all of then.
> > >
> > > Well first we need to define what a partitionable group is, whether it's
> > > based on hardware requirements or user policy. And while I agree that
> > > we need unique ownership of a partition, I disagree that qemu is
> > > necessarily the owner of the entire partition vs individual devices.
> >
> > Sorry, I didn't intend to have such circular logic. "... I disagree
> > that qemu is necessarily the owner of the entire partition vs granted
> > access to devices within the partition". Thanks,
>
> I still don't understand the distinction you're making. We're saying
> the group is "owned" by a given user or guest in the sense that no-one
> else may use anything in the group (including host drivers). At that
> point none, some or all of the devices in the group may actually be
> used by the guest.
>
> You seem to be making a distinction between "owned by" and "assigned
> to" and "used by" and I really don't see what it is.
How does a qemu instance that uses none of the devices in a group still
own that group? Aren't we at that point free to move the group to a
different qemu instance or return ownership to the host? Who does that?
In my mental model, there's an intermediary that "owns" the group and
just as kernel drivers bind to devices when the host owns the group,
qemu is a userspace device driver that binds to sets of devices when the
intermediary owns it. Obviously I'm thinking libvirt, but it doesn't
have to be. Thanks,
Alex
^ permalink raw reply
* RE: [PATCH v2] mtd/nand : workaround for Freescale FCM to support large-page Nand chip
From: Li Yang-R58472 @ 2011-08-03 6:15 UTC (permalink / raw)
To: Liu Shuo-B35362, dwmw2@infradead.org, dedekind1@gmail.com
Cc: linuxppc-dev@ozlabs.org, linux-mtd@lists.infradead.org
In-Reply-To: <1310446122-18050-1-git-send-email-b35362@freescale.com>
>-----Original Message-----
>From: Liu Shuo-B35362
>Sent: Tuesday, July 12, 2011 12:49 PM
>To: dwmw2@infradead.org
>Cc: linux-mtd@lists.infradead.org; linuxppc-dev@ozlabs.org; Liu Shuo-
>B35362; Li Yang-R58472
>Subject: [PATCH v2] mtd/nand : workaround for Freescale FCM to support
>large-page Nand chip
>
>From: Liu Shuo <b35362@freescale.com>
>
>Freescale FCM controller has a 2K size limitation of buffer RAM. In order
>to support the Nand flash chip whose page size is larger than 2K bytes, we
>divide a page into multi-2K pages for MTD layer driver. In that case, we
>force to set the page size to 2K bytes. We convert the page address of MTD
>layer driver to a real page address in flash chips and a column index in
>fsl_elbc driver. We can issue any column address by UA instruction of elbc
>controller.
>
>Signed-off-by: Liu Shuo <b35362@freescale.com>
>Signed-off-by: Li Yang <leoli@freescale.com>
>---
Hi David and Artem,
We have fixed the multi-line comment style problem. Could you help to pick=
the patch?
- Leo
^ permalink raw reply
* Re: Fwd: MPC7410 Linux Kernel
From: tiejun.chen @ 2011-08-03 6:42 UTC (permalink / raw)
To: Vineeth; +Cc: linuxppc-dev
In-Reply-To: <CAFbQSaDhEpSqX4GQHjSTsmpwv3a36RE7WrXbn7ni=NVcTtVyiw@mail.gmail.com>
Vineeth wrote:
> Thanks for the reply.
>
> We were referring kuroboxHG.dts which uses Sandpoint architecture; which is
> almost same as ours.
>
> 1. one doubt in kuroboxHG is the ranges property in SOC node says EUMB is at
> 0xFC00_0000; and as per the datasheet of mpc107, the open pic address will
And for powerpc looks u-boot should initialize MPC107 and EUMB_ADDR should be
configured as well. And kernel doesn't reconfigure EUMB again. This is different
from original ppc implementation.
So I think you should check how u-boot did. Note not all targets use 0xfc000000
as EUMB_ADDR. At least for linkstation 0x8000000 is set as EUMB_ADDR. And so
maybe this value, 0xFC000000, should be one typo since kernel don't use this
node property as I previously said.
> be EUMB_BASE + 0x40000; but in kurobox its given as 0x80040000;
>
> 2. We know that our UART is mapped at address 0xDB00_0100; which is
> connected in a PCI-LOCAL bridge whose base is at 0xDB00_0000
> How can i represent these things in dts ? Can the RANGES property of PCI
> node can mention this ?
How do u-boot resided on your target set EUMB_ADDR? Then you can migrate
kuroboxHG.dts with this base address.
Tiejun
>
>
>
>
>
>
>
> On Tue, Aug 2, 2011 at 1:15 PM, tiejun.chen <tiejun.chen@windriver.com>wrote:
>
>> Vineeth wrote:
>>> Hi,
>>>
>>> We are trying to port linux 2.6.38 on MPC7410 based board (This is a
>>> preparatory design by our customer)
>>>
>>> System architecture is as follows,
>>>
>>> MPC7410 <=> MPC107 <=> PCI_to_LOCAL(plx9052) <=> UART
>> MPCXXX should be compatible with TSIXXX. So you can refer to mpc7448_hpc2.
>>
>>> Previously we were using ppc architecture and we had some issues with
>>> page_init() functions; which may be because of our configuration.As we
>> didnt
>>> get much support on ppc architecture we moved to powerpc.
>>>
>>> Now we moved to powerpc architecture. We have some doubts on writing the
>> dts
>>> file. Please find the dts file attached.
>>>
>>> when we checked the legacy_serial.c file, we found that
>>> legacy_serial_parents not expecting a pci-local or a pci bridge as
>> parent.
>>> is our understanding correct ? should we introduce a new pci parent in
>> that
>>> structure ?
>> So you can understand this after refer to the file,
>> arch/powerpc/boot/dts/mpc7448hpc2.dts.
>>
>> Tiejun
>>
>>> We are confused about writing the ranges property of PCI node.we were
>>> referring booting_without_of doc but didnt get much info. Is there any
>> file
>>> which gives better idea about the ranges property ?
>>>
>>> Thanks
>>> Vineeth
>>
>
^ permalink raw reply
* [PATCH] mtd-utils: fix corrupt cleanmarker with flash_erase -j command
From: b35362 @ 2011-08-03 5:50 UTC (permalink / raw)
To: dwmw2, linuxppc-dev; +Cc: Liu Shuo, linuxppc-dev, linux-mtd
From: Liu Shuo <b35362@freescale.com>
Flash_erase -j should fill discrete freeoob areas with required bytes
of JFFS2 cleanmarker in jffs2_check_nand_cleanmarker(). Not just fill
the first freeoob area.
Signed-off-by: Liu Shuo <b35362@freescale.com>
Signed-off-by: Li Yang <leoli@freescale.com>
---
flash_erase.c | 41 +++++++++++++++++++++++++++++++++++------
1 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/flash_erase.c b/flash_erase.c
index fe2eaca..e6747fc 100644
--- a/flash_erase.c
+++ b/flash_erase.c
@@ -98,6 +98,7 @@ int main(int argc, char *argv[])
int isNAND;
int error = 0;
uint64_t offset = 0;
+ void *oob_data = NULL;
/*
* Process user arguments
@@ -197,15 +198,40 @@ int main(int argc, char *argv[])
if (ioctl(fd, MEMGETOOBSEL, &oobinfo) != 0)
return sys_errmsg("%s: unable to get NAND oobinfo", mtd_device);
+ cleanmarker.totlen = cpu_to_je32(8);
/* Check for autoplacement */
if (oobinfo.useecc == MTD_NANDECC_AUTOPLACE) {
+ struct nand_ecclayout_user ecclayout;
/* Get the position of the free bytes */
- if (!oobinfo.oobfree[0][1])
+ if (ioctl(fd, ECCGETLAYOUT, &ecclayout) != 0)
+ return sys_errmsg("%s: unable to get NAND ecclayout", mtd_device);
+
+ if (!ecclayout.oobavail)
return errmsg(" Eeep. Autoplacement selected and no empty space in oob");
clmpos = oobinfo.oobfree[0][0];
- clmlen = oobinfo.oobfree[0][1];
- if (clmlen > 8)
- clmlen = 8;
+ clmlen = MIN(ecclayout.oobavail, 8);
+
+ if (oobinfo.oobfree[0][1] < 8 && ecclayout.oobavail >= 8) {
+ int i, left, n, last = 0;
+ void *cm;
+
+ oob_data = malloc(mtd.oob_size);
+ if (!oob_data)
+ return -ENOMEM;
+
+ memset(oob_data, 0xff, mtd.oob_size);
+ cm = &cleanmarker;
+ for (i = 0, left = clmlen; left ; i++) {
+ n = MIN(left, oobinfo.oobfree[i][1]);
+ memcpy(oob_data + oobinfo.oobfree[i][0],
+ cm, n);
+ left -= n;
+ cm += n;
+ last = oobinfo.oobfree[i][0] + n;
+ }
+
+ clmlen = last - clmpos;
+ }
} else {
/* Legacy mode */
switch (mtd.oob_size) {
@@ -223,7 +249,6 @@ int main(int argc, char *argv[])
break;
}
}
- cleanmarker.totlen = cpu_to_je32(8);
}
cleanmarker.hdr_crc = cpu_to_je32(mtd_crc32(0, &cleanmarker, sizeof(cleanmarker) - 4));
}
@@ -272,7 +297,8 @@ int main(int argc, char *argv[])
/* write cleanmarker */
if (isNAND) {
- if (mtd_write_oob(mtd_desc, &mtd, fd, offset + clmpos, clmlen, &cleanmarker) != 0) {
+ void *data = oob_data ? oob_data + clmpos : &cleanmarker;
+ if (mtd_write_oob(mtd_desc, &mtd, fd, offset + clmpos, clmlen, data) != 0) {
sys_errmsg("%s: MTD writeoob failure", mtd_device);
continue;
}
@@ -291,5 +317,8 @@ int main(int argc, char *argv[])
show_progress(&mtd, offset, eb, eb_start, eb_cnt);
bareverbose(!quiet, "\n");
+ if (oob_data)
+ free(oob_data);
+
return 0;
}
--
1.7.1
^ permalink raw reply related
* RE: [PATCH 3/4] powerpc/85xx: Merge PCI/PCI Express error management registers
From: Xie Shaohui-B21989 @ 2011-08-03 9:57 UTC (permalink / raw)
To: linuxppc-dev@lists.ozlabs.org; +Cc: Jiang Kai-B18973, Xie Shaohui-B21989
In-Reply-To: <1311244195-4418-1-git-send-email-Shaohui.Xie@freescale.com>
Hi all,
Any concerns of this patch?
Best Regards,=20
Shaohui Xie=20
>-----Original Message-----
>From: Xie Shaohui-B21989
>Sent: Tuesday, July 26, 2011 2:50 PM
>To: Kumar Gala
>Cc: Jiang Kai-B18973; linuxppc-dev@lists.ozlabs.org
>Subject: RE: [PATCH 3/4] powerpc/85xx: Merge PCI/PCI Express error
>management registers
>
>
>
>>-----Original Message-----
>>From: Xie Shaohui-B21989
>>Sent: Thursday, July 21, 2011 6:30 PM
>>To: linuxppc-dev@lists.ozlabs.org
>>Cc: Gala Kumar-B11780; Jiang Kai-B18973; Kumar Gala; Xie Shaohui-B21989
>>Subject: [PATCH 3/4] powerpc/85xx: Merge PCI/PCI Express error
>>management registers
>>
>>From: Kai.Jiang <Kai.Jiang@freescale.com>
>>
>>There are some differences of register offset and definition between
>>pci and pcie error management registers. While, some other pci/pcie
>>error management registers are nearly the same.
>>
>>Signed-off-by: Kai.Jiang <Kai.Jiang@freescale.com>
>>Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
>>Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
>>---
>> arch/powerpc/sysdev/fsl_pci.h | 31 +++++++++++++++++++++++++------
>> 1 files changed, 25 insertions(+), 6 deletions(-)
>>
>>difg --gite a/arch/powerpc/sysdev/fsl_pci.h
>>b/arch/powerpc/sysdev/fsl_pci.h index a39ed5c..60a76e9 100644
>>--- a/arch/powerpc/sysdev/fsl_pci.h
>>+++ b/arch/powerpc/sysdev/fsl_pci.h
>>@@ -74,13 +74,32 @@ struct ccsr_pci {
>> */
>> struct pci_inbound_window_regs piw[4];
>>
>>+/* Merge PCI/PCI Express error management registers */
>> __be32 pex_err_dr; /* 0x.e00 - PCI/PCIE error detect
>>register */
>>- u8 res21[4];
>>- __be32 pex_err_en; /* 0x.e08 - PCI/PCIE error interrupt
>>enable register */
>>- u8 res22[4];
>>- __be32 pex_err_disr; /* 0x.e10 - PCI/PCIE error
>>disable register */
>>- u8 res23[12];
>>- __be32 pex_err_cap_stat; /* 0x.e20 - PCI/PCIE error capture
>>status register */
>>+ __be32 pex_err_cap_dr; /* 0x.e04 */
>>+ /* - PCI error capture disabled register */
>>+ /* - PCIE has no this register */
>>+ __be32 pex_err_en; /* 0x.e08 */
>>+ /* - PCI/PCIE error interrupt enable
>>register*/
>>+ __be32 pex_err_attrib; /* 0x.e0c */
>>+ /* - PCI error attributes capture register
>>*/
>>+ /* - PCIE has no this register */
>>+ __be32 pex_err_disr; /* 0x.e10 */
>>+ /* - PCI error address capture register */
>>+ /* - PCIE error disable register */
>>+ __be32 pex_err_ext_addr; /* 0x.e14 */
>>+ /* - PCI error extended addr capture
>>register*/
>>+ /* - PCIE has no this register */
>>+ __be32 pex_err_dl; /* 0x.e18 */
>>+ /* - PCI error data low capture register */
>>+ /* - PCIE has no this register */
>>+ __be32 pex_err_dh; /* 0x.e1c */
>>+ /* - PCI error data high capture register */
>>+ /* - PCIE has no this register */
>>+ __be32 pex_err_cap_stat; /* 0x.e20 */
>>+ /* - PCI gasket timer register */
>>+ /* - PCIE error capture status register */
>>+
>> u8 res24[4];
>> __be32 pex_err_cap_r0; /* 0x.e28 - PCIE error capture
>>register 0 */
>> __be32 pex_err_cap_r1; /* 0x.e2c - PCIE error capture
>>register 0 */
>>--
>>1.6.4
>[Xie Shaohui] I've verified this patch can apply for galak/powerpc.git
>'next' branch with no change.
>
>
>Best Regards,
>Shaohui Xie
^ permalink raw reply
* RE: [PATCH 4/4] edac/85xx: PCI/PCIE error interrupt edac support.
From: Xie Shaohui-B21989 @ 2011-08-03 9:58 UTC (permalink / raw)
To: Xie Shaohui-B21989, linuxppc-dev@lists.ozlabs.org, Kumar Gala
Cc: mm-commits@vger.kernel.org, avorontsov@mvista.com,
Jiang Kai-B18973, akpm@linux-foundation.org, davem@davemloft.net
In-Reply-To: <1311244404-4463-1-git-send-email-Shaohui.Xie@freescale.com>
Hi all,
Any concerns of this patch?
Best Regards,=20
Shaohui Xie=20
>-----Original Message-----
>From: Xie Shaohui-B21989
>Sent: Tuesday, July 26, 2011 2:52 PM
>To: linuxppc-dev@lists.ozlabs.org; Kumar Gala
>Cc: mm-commits@vger.kernel.org; avorontsov@mvista.com; davem@davemloft.net=
;
>grant.likely@secretlab.ca; akpm@linux-foundation.org; Jiang Kai-B18973
>Subject: RE: [PATCH 4/4] edac/85xx: PCI/PCIE error interrupt edac support.
>
>I've verified this patch can apply for galak/powerpc.git 'next' branch
>with no change.
>
>
>Best Regards,
>Shaohui Xie
>
>
>>-----Original Message-----
>>From: Xie Shaohui-B21989
>>Sent: Thursday, July 21, 2011 6:33 PM
>>To: linuxppc-dev@lists.ozlabs.org
>>Cc: Gala Kumar-B11780; mm-commits@vger.kernel.org; avorontsov@mvista.com;
>>davem@davemloft.net; grant.likely@secretlab.ca; akpm@linux-foundation.org=
;
>>Jiang Kai-B18973; Kumar Gala; Xie Shaohui-B21989
>>Subject: [PATCH 4/4] edac/85xx: PCI/PCIE error interrupt edac support.
>>
>>From: Kai.Jiang <Kai.Jiang@freescale.com>
>>
>>Add pcie error interrupt edac support for mpc85xx and p4080.
>>mpc85xx uses the legacy interrupt report mechanism - the error interrupts
>>are reported directly to mpic. While, p4080 attaches most of error
>>interrupts to interrupt 0. And report error interrupt to mpic via
>>interrupt 0. This patch can handle both of them.
>>
>>
>>Due to the error management register offset and definition
>>
>>difference between pci and pcie, use ccsr_pci structure to merge pci and
>>pcie edac code into one.
>>
>>Signed-off-by: Kai.Jiang <Kai.Jiang@freescale.com>
>>Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
>>Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
>>---
>> drivers/edac/mpc85xx_edac.c | 239 ++++++++++++++++++++++++++++++++-----
>-
>>----
>> drivers/edac/mpc85xx_edac.h | 17 +--
>> 2 files changed, 188 insertions(+), 68 deletions(-)
>>
>>diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/mpc85xx_edac.c
>>index b048a5f..dde156f 100644
>>--- a/drivers/edac/mpc85xx_edac.c
>>+++ b/drivers/edac/mpc85xx_edac.c
>>@@ -1,5 +1,6 @@
>> /*
>> * Freescale MPC85xx Memory Controller kenel module
>>+ * Copyright (c) 2011 Freescale Semiconductor, Inc.
>> *
>> * Author: Dave Jiang <djiang@mvista.com>
>> *
>>@@ -21,6 +22,8 @@
>>
>> #include <linux/of_platform.h>
>> #include <linux/of_device.h>
>>+#include <include/asm/pci.h>
>>+#include <sysdev/fsl_pci.h>
>> #include "edac_module.h"
>> #include "edac_core.h"
>> #include "mpc85xx_edac.h"
>>@@ -34,14 +37,6 @@ static int edac_mc_idx; static u32
>>orig_ddr_err_disable; static u32 orig_ddr_err_sbe;
>>
>>-/*
>>- * PCI Err defines
>>- */
>>-#ifdef CONFIG_PCI
>>-static u32 orig_pci_err_cap_dr;
>>-static u32 orig_pci_err_en;
>>-#endif
>>-
>> static u32 orig_l2_err_disable;
>> #ifdef CONFIG_FSL_SOC_BOOKE
>> static u32 orig_hid1[2];
>>@@ -151,37 +146,52 @@ static void mpc85xx_pci_check(struct
>>edac_pci_ctl_info *pci) {
>> struct mpc85xx_pci_pdata *pdata =3D pci->pvt_info;
>> u32 err_detect;
>>+ struct ccsr_pci *reg =3D pdata->pci_reg;
>>+
>>+ err_detect =3D in_be32(&pdata->pci_reg->pex_err_dr);
>>+
>>+ if (pdata->pcie_flag) {
>>+ printk(KERN_ERR "PCIE error(s) detected\n");
>>+ printk(KERN_ERR "PCIE ERR_DR register: 0x%08x\n", err_detect);
>>+ printk(KERN_ERR "PCIE ERR_CAP_STAT register: 0x%08x\n",
>>+ in_be32(®->pex_err_cap_stat));
>>+ printk(KERN_ERR "PCIE ERR_CAP_R0 register: 0x%08x\n",
>>+ in_be32(®->pex_err_cap_r0));
>>+ printk(KERN_ERR "PCIE ERR_CAP_R1 register: 0x%08x\n",
>>+ in_be32(®->pex_err_cap_r1));
>>+ printk(KERN_ERR "PCIE ERR_CAP_R2 register: 0x%08x\n",
>>+ in_be32(®->pex_err_cap_r2));
>>+ printk(KERN_ERR "PCIE ERR_CAP_R3 register: 0x%08x\n",
>>+ in_be32(®->pex_err_cap_r3));
>>+ } else {
>>+ /* master aborts can happen during PCI config cycles */
>>+ if (!(err_detect & ~(PCI_EDE_MULTI_ERR | PCI_EDE_MST_ABRT))) {
>>+ out_be32(®->pex_err_dr, err_detect);
>>+ return;
>>+ }
>>
>>- err_detect =3D in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DR);
>>-
>>- /* master aborts can happen during PCI config cycles */
>>- if (!(err_detect & ~(PCI_EDE_MULTI_ERR | PCI_EDE_MST_ABRT))) {
>>- out_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DR, err_detect);
>>- return;
>>+ printk(KERN_ERR "PCI error(s) detected\n");
>>+ printk(KERN_ERR "PCI/X ERR_DR register: 0x%08x\n", err_detect);
>>+ printk(KERN_ERR "PCI/X ERR_ATTRIB register: 0x%08x\n",
>>+ in_be32(®->pex_err_attrib));
>>+ printk(KERN_ERR "PCI/X ERR_ADDR register: 0x%08x\n",
>>+ in_be32(®->pex_err_disr));
>>+ printk(KERN_ERR "PCI/X ERR_EXT_ADDR register: 0x%08x\n",
>>+ in_be32(®->pex_err_ext_addr));
>>+ printk(KERN_ERR "PCI/X ERR_DL register: 0x%08x\n",
>>+ in_be32(®->pex_err_dl));
>>+ printk(KERN_ERR "PCI/X ERR_DH register: 0x%08x\n",
>>+ in_be32(®->pex_err_dh));
>>+
>>+ if (err_detect & PCI_EDE_PERR_MASK)
>>+ edac_pci_handle_pe(pci, pci->ctl_name);
>>+
>>+ if ((err_detect & ~PCI_EDE_MULTI_ERR) & ~PCI_EDE_PERR_MASK)
>>+ edac_pci_handle_npe(pci, pci->ctl_name);
>> }
>>
>>- printk(KERN_ERR "PCI error(s) detected\n");
>>- printk(KERN_ERR "PCI/X ERR_DR register: %#08x\n", err_detect);
>>-
>>- printk(KERN_ERR "PCI/X ERR_ATTRIB register: %#08x\n",
>>- in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_ATTRIB));
>>- printk(KERN_ERR "PCI/X ERR_ADDR register: %#08x\n",
>>- in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_ADDR));
>>- printk(KERN_ERR "PCI/X ERR_EXT_ADDR register: %#08x\n",
>>- in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_EXT_ADDR));
>>- printk(KERN_ERR "PCI/X ERR_DL register: %#08x\n",
>>- in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DL));
>>- printk(KERN_ERR "PCI/X ERR_DH register: %#08x\n",
>>- in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DH));
>>-
>> /* clear error bits */
>>- out_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DR, err_detect);
>>-
>>- if (err_detect & PCI_EDE_PERR_MASK)
>>- edac_pci_handle_pe(pci, pci->ctl_name);
>>-
>>- if ((err_detect & ~PCI_EDE_MULTI_ERR) & ~PCI_EDE_PERR_MASK)
>>- edac_pci_handle_npe(pci, pci->ctl_name);
>>+ out_be32(®->pex_err_dr, err_detect);
>> }
>>
>> static irqreturn_t mpc85xx_pci_isr(int irq, void *dev_id) @@ -190,7
>>+200,7 @@ static irqreturn_t mpc85xx_pci_isr(int irq, void *dev_id)
>> struct mpc85xx_pci_pdata *pdata =3D pci->pvt_info;
>> u32 err_detect;
>>
>>- err_detect =3D in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DR);
>>+ err_detect =3D in_be32(&pdata->pci_reg->pex_err_dr);
>>
>> if (!err_detect)
>> return IRQ_NONE;
>>@@ -200,11 +210,99 @@ static irqreturn_t mpc85xx_pci_isr(int irq, void
>>*dev_id)
>> return IRQ_HANDLED;
>> }
>>
>>+#define MPC85XX_MPIC_EIMR0 0x3910
>>+/*
>>+ * This function is for error interrupt ORed mechanism.
>>+ * This mechanism attaches most functions' error interrupts to interrupt
>>0.
>>+ * And report error interrupt to mpic via interrupt 0.
>>+ * EIMR0 - Error Interrupt Mask Register 0.
>>+ *
>>+ * This function check whether the device support error interrupt ORed
>>+ * mechanism via device tree. If supported, umask pcie error interrupt
>>+ * bit in EIMR0.
>>+ */
>>+static int mpc85xx_err_int_en(struct device *op) {
>>+ u32 *int_cell =3D NULL;
>>+ struct device_node *np =3D NULL;
>>+ void __iomem *mpic_base =3D NULL;
>>+ u32 reg_tmp =3D 0;
>>+ u32 int_len =3D 0;
>>+ struct resource r;
>>+ int res =3D 0;
>>+
>>+ if (!op->of_node)
>>+ return -EINVAL;
>>+ /*
>>+ * Unmask pcie error interrupt bit in EIMR0
>>+ * extend interrupt specifier has 4 cells. For the 3rd cell:
>>+ * 0 -- normal interrupt; 1 -- error interrupt.
>>+ */
>>+ int_cell =3D (u32 *)of_get_property(op->of_node, "interrupts",
>>&int_len);
>>+ if ((int_len/sizeof(u32)) =3D=3D 4) {
>>+ /* soc has error interrupt integration handling mechanism */
>>+ if (*(int_cell + 2) =3D=3D 1) {
>>+ np =3D of_find_node_by_type(NULL, "open-pic");
>>+
>>+ if (of_address_to_resource(np, 0, &r)) {
>>+ printk(KERN_ERR
>>+ "%s:Failed to map mpic regs\n", __func__);
>>+ of_node_put(np);
>>+ res =3D -ENOMEM;
>>+ goto err;
>>+ }
>>+
>>+ if (!request_mem_region(r.start,
>>+ r.end - r.start + 1, "mpic")) {
>>+ printk(KERN_ERR
>>+ "%s:Error while requesting mem region\n",
>>+ __func__);
>>+ res =3D -EBUSY;
>>+ goto err;
>>+ }
>>+
>>+ mpic_base =3D ioremap(r.start, r.end - r.start + 1);
>>+ if (!mpic_base) {
>>+ printk(KERN_ERR
>>+ "%s:Unable to map mpic regs\n", __func__);
>>+ res =3D -ENOMEM;
>>+ goto err_ioremap;
>>+ }
>>+
>>+ reg_tmp =3D in_be32(mpic_base + MPC85XX_MPIC_EIMR0);
>>+ out_be32(mpic_base + MPC85XX_MPIC_EIMR0,
>>+ reg_tmp & ~(1 << (31 - *(int_cell + 3))));
>>+ iounmap(mpic_base);
>>+ release_mem_region(r.start, r.end - r.start + 1);
>>+ of_node_put(np);
>>+ }
>>+ }
>>+
>>+ return 0;
>>+err_ioremap:
>>+ release_mem_region(r.start, r.end - r.start + 1);
>>+err:
>>+
>>+ return res;
>>+}
>>+
>>+static int mpc85xx_pcie_find_capability(struct device_node *np) {
>>+ struct pci_controller *hose;
>>+ if (!np)
>>+ return -EINVAL;
>>+
>>+ hose =3D pci_find_hose_for_OF_device(np);
>>+ return early_find_capability(hose, hose->bus->number,
>>+ 0, PCI_CAP_ID_EXP);
>>+}
>>+
>> static int __devinit mpc85xx_pci_err_probe(struct platform_device *op)
>{
>> struct edac_pci_ctl_info *pci;
>> struct mpc85xx_pci_pdata *pdata;
>> struct resource r;
>>+ struct ccsr_pci *reg =3D NULL;
>> int res =3D 0;
>>
>> if (!devres_open_group(&op->dev, mpc85xx_pci_err_probe, GFP_KERNEL))
>>@@ -217,6 +315,10 @@ static int __devinit mpc85xx_pci_err_probe(struct
>>platform_device *op)
>> pdata =3D pci->pvt_info;
>> pdata->name =3D "mpc85xx_pci_err";
>> pdata->irq =3D NO_IRQ;
>>+
>>+ if (mpc85xx_pcie_find_capability(op->dev.of_node) > 0)
>>+ pdata->pcie_flag =3D 1;
>>+
>> dev_set_drvdata(&op->dev, pci);
>> pci->dev =3D &op->dev;
>> pci->mod_name =3D EDAC_MOD_STR;
>>@@ -235,37 +337,40 @@ static int __devinit mpc85xx_pci_err_probe(struct
>>platform_device *op)
>> goto err;
>> }
>>
>>- /* we only need the error registers */
>>- r.start +=3D 0xe00;
>>-
>> if (!devm_request_mem_region(&op->dev, r.start, resource_size(&r),
>> pdata->name)) {
>>- printk(KERN_ERR "%s: Error while requesting mem region\n",
>>- __func__);
>>+ printk(KERN_ERR
>>+ "%s:Error while requesting mem region\n", __func__);
>> res =3D -EBUSY;
>> goto err;
>> }
>>
>>- pdata->pci_vbase =3D devm_ioremap(&op->dev, r.start,
>>resource_size(&r));
>>- if (!pdata->pci_vbase) {
>>+ pdata->pci_reg =3D devm_ioremap(&op->dev, r.start, resource_size(&r));
>>+ if (!pdata->pci_reg) {
>> printk(KERN_ERR "%s: Unable to setup PCI err regs\n",
>>__func__);
>> res =3D -ENOMEM;
>> goto err;
>> }
>>
>>- orig_pci_err_cap_dr =3D
>>- in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_CAP_DR);
>>-
>>- /* PCI master abort is expected during config cycles */
>>- out_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_CAP_DR, 0x40);
>>+ if (mpc85xx_err_int_en(&op->dev) < 0)
>>+ goto err;
>>
>>- orig_pci_err_en =3D in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_EN);
>>+ reg =3D pdata->pci_reg;
>>+ /* disable pci/pcie error detect */
>>+ if (pdata->pcie_flag) {
>>+ pdata->orig_pci_err_dr =3D in_be32(®->pex_err_disr);
>>+ out_be32(®->pex_err_disr, ~0);
>>+ } else {
>>+ pdata->orig_pci_err_dr =3D in_be32(®->pex_err_cap_dr);
>>+ out_be32(®->pex_err_cap_dr, ~0);
>>+ }
>>
>>- /* disable master abort reporting */
>>- out_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_EN, ~0x40);
>>+ /* disable all pcie error interrupt */
>>+ pdata->orig_pci_err_en =3D in_be32(®->pex_err_en);
>>+ out_be32(®->pex_err_en, 0);
>>
>>- /* clear error bits */
>>- out_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DR, ~0);
>>+ /* clear all error bits */
>>+ out_be32(®->pex_err_dr, ~0);
>>
>> if (edac_pci_add_device(pci, pdata->edac_idx) > 0) {
>> debugf3("%s(): failed edac_pci_add_device()\n", __func__); @@
>>-275,7 +380,7 @@ static int __devinit mpc85xx_pci_err_probe(struct
>>platform_device *op)
>> if (edac_op_state =3D=3D EDAC_OPSTATE_INT) {
>> pdata->irq =3D irq_of_parse_and_map(op->dev.of_node, 0);
>> res =3D devm_request_irq(&op->dev, pdata->irq,
>>- mpc85xx_pci_isr, IRQF_DISABLED,
>>+ mpc85xx_pci_isr, IRQF_SHARED,
>> "[EDAC] PCI err", pci);
>> if (res < 0) {
>> printk(KERN_ERR
>>@@ -290,6 +395,17 @@ static int __devinit mpc85xx_pci_err_probe(struct
>>platform_device *op)
>> pdata->irq);
>> }
>>
>>+ if (pdata->pcie_flag) {
>>+ /* enable all pcie error interrupt & error detect */
>>+ out_be32(®->pex_err_en, ~0);
>>+ out_be32(®->pex_err_disr, 0);
>>+ } else {
>>+ /* PCI master abort is expected during config cycles */
>>+ out_be32(®->pex_err_cap_dr, PCI_ERR_CAP_DR_DIS_MST);
>>+ /* disable master abort reporting */
>>+ out_be32(®->pex_err_en, PCI_ERR_EN_DIS_MST);
>>+ }
>>+
>> devres_remove_group(&op->dev, mpc85xx_pci_err_probe);
>> debugf3("%s(): success\n", __func__);
>> printk(KERN_INFO EDAC_MOD_STR " PCI err registered\n"); @@ -311,10
>>+427,13 @@ static int mpc85xx_pci_err_remove(struct platform_device *op)
>>
>> debugf0("%s()\n", __func__);
>>
>>- out_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_CAP_DR,
>>- orig_pci_err_cap_dr);
>>+ if (pdata->pcie_flag)
>>+ out_be32(&pdata->pci_reg->pex_err_disr, pdata-
>>>orig_pci_err_dr);
>>+ else
>>+ out_be32(&pdata->pci_reg->pex_err_cap_dr,
>>+ pdata->orig_pci_err_dr);
>>
>>- out_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_EN, orig_pci_err_en);
>>+ out_be32(&pdata->pci_reg->pex_err_en, pdata->orig_pci_err_en);
>>
>> edac_pci_del_device(pci->dev);
>>
>>@@ -333,6 +452,12 @@ static struct of_device_id mpc85xx_pci_err_of_match[=
]
>>=3D {
>> {
>> .compatible =3D "fsl,mpc8540-pci",
>> },
>>+ {
>>+ .compatible =3D "fsl,mpc8548-pcie",
>>+ },
>>+ {
>>+ .compatible =3D "fsl,p4080-pcie",
>>+ },
>> {},
>> };
>> MODULE_DEVICE_TABLE(of, mpc85xx_pci_err_of_match); diff --git
>>a/drivers/edac/mpc85xx_edac.h b/drivers/edac/mpc85xx_edac.h index
>>932016f..d0e7b11 100644
>>--- a/drivers/edac/mpc85xx_edac.h
>>+++ b/drivers/edac/mpc85xx_edac.h
>>@@ -131,16 +131,8 @@
>> #define PCI_EDE_PERR_MASK (PCI_EDE_TGT_PERR | PCI_EDE_MST_PERR | \
>> PCI_EDE_ADDR_PERR)
>>
>>-#define MPC85XX_PCI_ERR_DR 0x0000
>>-#define MPC85XX_PCI_ERR_CAP_DR 0x0004
>>-#define MPC85XX_PCI_ERR_EN 0x0008
>>-#define MPC85XX_PCI_ERR_ATTRIB 0x000c
>>-#define MPC85XX_PCI_ERR_ADDR 0x0010
>>-#define MPC85XX_PCI_ERR_EXT_ADDR 0x0014
>>-#define MPC85XX_PCI_ERR_DL 0x0018
>>-#define MPC85XX_PCI_ERR_DH 0x001c
>>-#define MPC85XX_PCI_GAS_TIMR 0x0020
>>-#define MPC85XX_PCI_PCIX_TIMR 0x0024
>>+#define PCI_ERR_CAP_DR_DIS_MST 0x40
>>+#define PCI_ERR_EN_DIS_MST (~0x40)
>>
>> struct mpc85xx_mc_pdata {
>> char *name;
>>@@ -159,8 +151,11 @@ struct mpc85xx_l2_pdata { struct mpc85xx_pci_pdata
>{
>> char *name;
>> int edac_idx;
>>- void __iomem *pci_vbase;
>> int irq;
>>+ struct ccsr_pci *pci_reg;
>>+ u8 pcie_flag;
>>+ u32 orig_pci_err_dr;
>>+ u32 orig_pci_err_en;
>> };
>>
>> #endif
>>--
>>1.6.4
^ permalink raw reply
* Re: [RFC PATCH V1 3/7] cpuidle: stop using pm_idle
From: Len Brown @ 2011-08-03 17:45 UTC (permalink / raw)
To: Trinabh Gupta; +Cc: linuxppc-dev, linux-pm, linux-kernel
In-Reply-To: <20110607162947.6848.79430.stgit@tringupt.in.ibm.com>
On Tue, 7 Jun 2011, Trinabh Gupta wrote:
> From: Len Brown <len.brown@intel.com>
>
> pm_idle does not scale as an idle handler registration mechanism.
> Don't use it for cpuidle. Instead, call cpuidle directly, and
> allow architectures to use pm_idle as an arch-specific default
> if they need it. ie.
>
> cpu_idle()
> ...
> if(cpuidle_call_idle())
Looks like you forgot to correct my typo that you pointed out earlier,
s/cpuidle_call_idle/cpuidle_idle_call/
both in the comment here and for arm and sh below.
Thanks for including the From: above, that is correct form.
But note in the future that when you modify somebody else's patch,
you should append a note about what you changed,
and also add your signed-off-by, so we can
track the changes.
thanks,
-Len
> pm_idle();
>
> cc: x86@kernel.org
> cc: Kevin Hilman <khilman@deeprootsystems.com>
> cc: Paul Mundt <lethal@linux-sh.org>
> Signed-off-by: Len Brown <len.brown@intel.com>
>
> ---
>
> arch/arm/kernel/process.c | 4 +++-
> arch/sh/kernel/idle.c | 6 ++++--
> arch/x86/kernel/process_32.c | 4 +++-
> arch/x86/kernel/process_64.c | 4 +++-
> drivers/cpuidle/cpuidle.c | 39 ++++++++++++++++++---------------------
> include/linux/cpuidle.h | 2 ++
> 6 files changed, 33 insertions(+), 26 deletions(-)
>
> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> index 5e1e541..d7ee0d4 100644
> --- a/arch/arm/kernel/process.c
> +++ b/arch/arm/kernel/process.c
> @@ -30,6 +30,7 @@
> #include <linux/uaccess.h>
> #include <linux/random.h>
> #include <linux/hw_breakpoint.h>
> +#include <linux/cpuidle.h>
>
> #include <asm/cacheflush.h>
> #include <asm/leds.h>
> @@ -196,7 +197,8 @@ void cpu_idle(void)
> cpu_relax();
> } else {
> stop_critical_timings();
> - pm_idle();
> + if (cpuidle_call_idle())
> + pm_idle();
> start_critical_timings();
> /*
> * This will eventually be removed - pm_idle
> diff --git a/arch/sh/kernel/idle.c b/arch/sh/kernel/idle.c
> index 425d604..9c7099e 100644
> --- a/arch/sh/kernel/idle.c
> +++ b/arch/sh/kernel/idle.c
> @@ -16,12 +16,13 @@
> #include <linux/thread_info.h>
> #include <linux/irqflags.h>
> #include <linux/smp.h>
> +#include <linux/cpuidle.h>
> #include <asm/pgalloc.h>
> #include <asm/system.h>
> #include <asm/atomic.h>
> #include <asm/smp.h>
>
> -void (*pm_idle)(void) = NULL;
> +static void (*pm_idle)(void);
>
> static int hlt_counter;
>
> @@ -100,7 +101,8 @@ void cpu_idle(void)
> local_irq_disable();
> /* Don't trace irqs off for idle */
> stop_critical_timings();
> - pm_idle();
> + if (cpuidle_call_idle())
> + pm_idle();
> /*
> * Sanity check to ensure that pm_idle() returns
> * with IRQs enabled
> diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c
> index 8d12878..61fadbe 100644
> --- a/arch/x86/kernel/process_32.c
> +++ b/arch/x86/kernel/process_32.c
> @@ -38,6 +38,7 @@
> #include <linux/uaccess.h>
> #include <linux/io.h>
> #include <linux/kdebug.h>
> +#include <linux/cpuidle.h>
>
> #include <asm/pgtable.h>
> #include <asm/system.h>
> @@ -109,7 +110,8 @@ void cpu_idle(void)
> local_irq_disable();
> /* Don't trace irqs off for idle */
> stop_critical_timings();
> - pm_idle();
> + if (cpuidle_idle_call())
> + pm_idle();
> start_critical_timings();
> }
> tick_nohz_restart_sched_tick();
> diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
> index 6c9dd92..62c219a 100644
> --- a/arch/x86/kernel/process_64.c
> +++ b/arch/x86/kernel/process_64.c
> @@ -37,6 +37,7 @@
> #include <linux/uaccess.h>
> #include <linux/io.h>
> #include <linux/ftrace.h>
> +#include <linux/cpuidle.h>
>
> #include <asm/pgtable.h>
> #include <asm/system.h>
> @@ -136,7 +137,8 @@ void cpu_idle(void)
> enter_idle();
> /* Don't trace irqs off for idle */
> stop_critical_timings();
> - pm_idle();
> + if (cpuidle_idle_call())
> + pm_idle();
> start_critical_timings();
>
> /* In many cases the interrupt that ended idle
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 8d7303b..304e378 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -25,10 +25,10 @@ DEFINE_PER_CPU(struct cpuidle_device *, cpuidle_devices);
>
> DEFINE_MUTEX(cpuidle_lock);
> LIST_HEAD(cpuidle_detected_devices);
> -static void (*pm_idle_old)(void);
>
> static int enabled_devices;
> static int off __read_mostly;
> +static int initialized __read_mostly;
>
> int cpuidle_disabled(void)
> {
> @@ -56,27 +56,24 @@ static int __cpuidle_register_device(struct cpuidle_device *dev);
> * cpuidle_idle_call - the main idle loop
> *
> * NOTE: no locks or semaphores should be used here
> + * return non-zero on failure
> */
> -static void cpuidle_idle_call(void)
> +int cpuidle_idle_call(void)
> {
> struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices);
> struct cpuidle_driver *drv = cpuidle_get_driver();
> struct cpuidle_state *target_state;
> int next_state, entered_state;
>
> - /* check if the device is ready */
> - if (!dev || !dev->enabled) {
> - if (pm_idle_old)
> - pm_idle_old();
> - else
> -#if defined(CONFIG_ARCH_HAS_DEFAULT_IDLE)
> - default_idle();
> -#else
> - local_irq_enable();
> -#endif
> - return;
> - }
> + if (off)
> + return -ENODEV;
> +
> + if (!initialized)
> + return -ENODEV;
>
> + /* check if the device is ready */
> + if (!dev || !dev->enabled)
> + return -EBUSY;
> #if 0
> /* shows regressions, re-enable for 2.6.29 */
> /*
> @@ -90,7 +87,7 @@ static void cpuidle_idle_call(void)
> next_state = cpuidle_curr_governor->select(drv, dev);
> if (need_resched()) {
> local_irq_enable();
> - return;
> + return 0;
> }
>
> target_state = &drv->states[next_state];
> @@ -116,6 +113,8 @@ static void cpuidle_idle_call(void)
> /* give the governor an opportunity to reflect on the outcome */
> if (cpuidle_curr_governor->reflect)
> cpuidle_curr_governor->reflect(dev, entered_state);
> +
> + return 0;
> }
>
> /**
> @@ -123,10 +122,10 @@ static void cpuidle_idle_call(void)
> */
> void cpuidle_install_idle_handler(void)
> {
> - if (enabled_devices && (pm_idle != cpuidle_idle_call)) {
> + if (enabled_devices) {
> /* Make sure all changes finished before we switch to new idle */
> smp_wmb();
> - pm_idle = cpuidle_idle_call;
> + initialized = 1;
> }
> }
>
> @@ -135,8 +134,8 @@ void cpuidle_install_idle_handler(void)
> */
> void cpuidle_uninstall_idle_handler(void)
> {
> - if (enabled_devices && pm_idle_old && (pm_idle != pm_idle_old)) {
> - pm_idle = pm_idle_old;
> + if (enabled_devices) {
> + initialized = 0;
> cpuidle_kick_cpus();
> }
> }
> @@ -410,8 +409,6 @@ static int __init cpuidle_init(void)
> if (cpuidle_disabled())
> return -ENODEV;
>
> - pm_idle_old = pm_idle;
> -
> ret = cpuidle_add_class_sysfs(&cpu_sysdev_class);
> if (ret)
> return ret;
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index 2786787..c904188 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -128,6 +128,7 @@ struct cpuidle_driver {
>
> #ifdef CONFIG_CPU_IDLE
> extern void disable_cpuidle(void);
> +extern int cpuidle_idle_call(void);
>
> extern int cpuidle_register_driver(struct cpuidle_driver *drv);
> struct cpuidle_driver *cpuidle_get_driver(void);
> @@ -142,6 +143,7 @@ extern void cpuidle_disable_device(struct cpuidle_device *dev);
>
> #else
> static inline void disable_cpuidle(void) { }
> +static inline int cpuidle_idle_call(void) { return -ENODEV; }
>
> static inline int cpuidle_register_driver(struct cpuidle_driver *drv)
> {return -ENODEV; }
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
^ permalink raw reply
* [PATCH] powerpc/kvm: fix build errors with older toolchains
From: Nishanth Aravamudan @ 2011-08-03 18:55 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: kvm, Marcelo Tosatti, Alexander Graf, kvm-ppc, linux-kernel,
Paul Mackerras, Avi Kivity, linuxppc-dev
On a box with gcc 4.3.2, I see errors like:
arch/powerpc/kvm/book3s_hv_rmhandlers.S:1254: Error: Unrecognized opcode: stxvd2x
arch/powerpc/kvm/book3s_hv_rmhandlers.S:1316: Error: Unrecognized opcode: lxvd2x
Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
---
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 6dd3358..de29501 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -1251,7 +1251,7 @@ BEGIN_FTR_SECTION
reg = 0
.rept 32
li r6,reg*16+VCPU_VSRS
- stxvd2x reg,r6,r3
+ STXVD2X(reg,r6,r3)
reg = reg + 1
.endr
FTR_SECTION_ELSE
@@ -1313,7 +1313,7 @@ BEGIN_FTR_SECTION
reg = 0
.rept 32
li r7,reg*16+VCPU_VSRS
- lxvd2x reg,r7,r4
+ LXVD2X(reg,r7,r4)
reg = reg + 1
.endr
FTR_SECTION_ELSE
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH 00/15] ps3: Support more than the OtherOS lpar
From: Geoff Levand @ 2011-08-03 22:23 UTC (permalink / raw)
To: Andre Heider; +Cc: cbe-oss-dev, Hector Martin, linuxppc-dev
In-Reply-To: <1312228986-32307-1-git-send-email-a.heider@gmail.com>
Hi Andre,
On 08/01/2011 01:02 PM, Andre Heider wrote:
> This series addresses various issues and extends support when running
> in lpars like GameOS. Included are some patches from Hector Martin, which
> I found useful.
Much of this is just general fixups and improvements to the existing PS3
support. I think you should separate those changes out and work to get
them included, then consider others. If I give some comment, then
I consider that part worth pursuing at the present time.
I have limited time to review the patches, so it will take me a while to
get through them.
> Patches are based on 2.6.39 since master doesn't boot with smp on my
> console. I wasn't able to pinpoint the cause so far (not that I tried
> too hard).
I'm looking into this problem, but it will take some time.
-Geoff
^ permalink raw reply
* Re: [PATCH 02/15] [PS3] Get lv1 high memory region from devtree
From: Geoff Levand @ 2011-08-03 22:30 UTC (permalink / raw)
To: Andre Heider; +Cc: cbe-oss-dev, Hector Martin, linuxppc-dev
In-Reply-To: <1312228986-32307-3-git-send-email-a.heider@gmail.com>
On 08/01/2011 01:02 PM, Andre Heider wrote:
>
> This lets the bootloader preallocate the high lv1 region and pass its
> location to the kernel through the devtree. Thus, it can be used to hold
> the initrd. If the property doesn't exist, the kernel retains the old
> behavior and attempts to allocate the region itself.
With this mechanism how is the address of the initrd passed to the
new kernel, in the DT?
How would a kexec based bootloader work? If it's kernel were to allocate
high mem and the bootloader program uses the high mem, how could it tell
that kernel not to destroy the region on shutdown?
If arch/powerpc/boot/ps3.c allocated the mem and added a DT entry
then other OSes that don't know about the Linux device tree won't
be able to use that allocated memory. Other OSes could do a
test to see if the allocation was already done. Another option
that might work is to write info into the LV1 repository then
have boot code look there for allocated hig mem.
> Signed-off-by: Hector Martin <hector@marcansoft.com>
> [a.heider: Various cleanups to make checkpatch.pl happy]
> Signed-off-by: Andre Heider <a.heider@gmail.com>
> ---
> arch/powerpc/platforms/ps3/mm.c | 61 +++++++++++++++++++++++++++++++++++++-
> 1 files changed, 59 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/platforms/ps3/mm.c b/arch/powerpc/platforms/ps3/mm.c
> index c204588..30bb096 100644
> --- a/arch/powerpc/platforms/ps3/mm.c
> +++ b/arch/powerpc/platforms/ps3/mm.c
> @@ -110,6 +110,7 @@ struct map {
> u64 htab_size;
> struct mem_region rm;
> struct mem_region r1;
> + int destroy_r1;
In the general case we could have multiple high mem
regions, and each could need to be destroyed, so I
think struct mem_region should have a destroy flag.
> };
>
> #define debug_dump_map(x) _debug_dump_map(x, __func__, __LINE__)
> @@ -287,6 +288,49 @@ static void ps3_mm_region_destroy(struct mem_region *r)
> }
> }
>
> +static int ps3_mm_scan_memory(unsigned long node, const char *uname,
> + int depth, void *data)
> +{
Something like 'ps3_mm_dt_scan_highmem() is more descriptive.
> + struct mem_region *r = data;
> + void *p;
> + u64 prop[2];
> + unsigned long l;
> + char *type = of_get_flat_dt_prop(node, "device_type", NULL);
> +
> + if (type == NULL)
> + return 0;
> + if (strcmp(type, "memory") != 0)
Should this be 'if (strcmp(type, "memory"))'?
> + return 0;
> +
> + p = of_get_flat_dt_prop(node, "sony,lv1-highmem", &l);
> + if (p == NULL)
> + return 0;
> +
> + BUG_ON(l != sizeof(prop));
> + memcpy(prop, p, sizeof(prop));
> +
> + r->base = prop[0];
> + r->size = prop[1];
> + r->offset = r->base - map.rm.size;
> +
> + return -1;
> +}
> +
> +static int ps3_mm_get_devtree_highmem(struct mem_region *r)
> +{
> + r->size = r->base = r->offset = 0;
> + of_scan_flat_dt(ps3_mm_scan_memory, r);
> +
> + if (r->base && r->size) {
> + DBG("%s:%d got high region from devtree: %llxh %llxh\n",
> + __func__, __LINE__, r->base, r->size);
> + return 0;
> + } else {
> + DBG("%s:%d no high region in devtree...\n", __func__, __LINE__);
> + return -1;
> + }
> +}
> +
> /**
> * ps3_mm_add_memory - hot add memory
> */
> @@ -303,6 +347,12 @@ static int __init ps3_mm_add_memory(void)
>
> BUG_ON(!mem_init_done);
>
> + if (!map.r1.size) {
> + DBG("%s:%d: no region 1, not adding memory\n",
> + __func__, __LINE__);
> + return 0;
> + }
Did you find this to be hit? Also, in the general case,
there could be more than one high mem region, but I don't
know of any current systems that do.
> +
> start_addr = map.rm.size;
> start_pfn = start_addr >> PAGE_SHIFT;
> nr_pages = (map.r1.size + PAGE_SIZE - 1) >> PAGE_SHIFT;
> @@ -1219,7 +1269,13 @@ void __init ps3_mm_init(void)
>
>
> /* arrange to do this in ps3_mm_add_memory */
> - ps3_mm_region_create(&map.r1, map.total - map.rm.size);
> +
> + if (ps3_mm_get_devtree_highmem(&map.r1) == 0) {
> + map.destroy_r1 = 0;
> + } else {
This should be
if (!ps3_mm_get_devtree_highmem(&map.r1))
map.destroy_r1 = 0;
else {
> + ps3_mm_region_create(&map.r1, map.total - map.rm.size);
> + map.destroy_r1 = 1;
> + }
>
> /* correct map.total for the real total amount of memory we use */
> map.total = map.rm.size + map.r1.size;
> @@ -1233,5 +1289,6 @@ void __init ps3_mm_init(void)
>
> void ps3_mm_shutdown(void)
> {
> - ps3_mm_region_destroy(&map.r1);
> + if (map.destroy_r1)
> + ps3_mm_region_destroy(&map.r1);
> }
-Geoff
^ permalink raw reply
* Re: [PATCH 05/15] ps3: Detect the current lpar environment
From: Geoff Levand @ 2011-08-03 22:31 UTC (permalink / raw)
To: Andre Heider; +Cc: cbe-oss-dev, Hector Martin, linuxppc-dev
In-Reply-To: <1312228986-32307-6-git-send-email-a.heider@gmail.com>
On 08/01/2011 01:02 PM, Andre Heider wrote:
> ---
> arch/powerpc/include/asm/ps3.h | 7 +++++++
> arch/powerpc/platforms/ps3/platform.h | 4 ++++
> arch/powerpc/platforms/ps3/repository.c | 19 +++++++++++++++++++
> arch/powerpc/platforms/ps3/setup.c | 22 ++++++++++++++++++++++
> 4 files changed, 52 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ps3.h b/arch/powerpc/include/asm/ps3.h
> index 7f065e1..136354a 100644
> --- a/arch/powerpc/include/asm/ps3.h
> +++ b/arch/powerpc/include/asm/ps3.h
> @@ -39,6 +39,13 @@ union ps3_firmware_version {
> void ps3_get_firmware_version(union ps3_firmware_version *v);
> int ps3_compare_firmware_version(u16 major, u16 minor, u16 rev);
>
> +enum ps3_ss_laid {
> + PS3_SS_LAID_GAMEOS = 0x1070000002000001UL,
> + PS3_SS_LAID_OTHEROS = 0x1080000004000001UL,
Only PS3_SS_LAID_OTHEROS is used for anything outside ps3_setup_arch(),
so I think it makes sense to split this into two patches with one adding
just PS3_SS_LAID_OTHEROS and ps3_get_ss_laid() with a comment that
it adds the ps3_get_ss_laid routine.
> +};
> +
> +enum ps3_ss_laid ps3_get_ss_laid(void);
> +
> /* 'Other OS' area */
>
> enum ps3_param_av_multi_out {
> diff --git a/arch/powerpc/platforms/ps3/platform.h b/arch/powerpc/platforms/ps3/platform.h
> index 9a196a8..1ba15b8 100644
> --- a/arch/powerpc/platforms/ps3/platform.h
> +++ b/arch/powerpc/platforms/ps3/platform.h
> @@ -232,4 +232,8 @@ int ps3_repository_read_spu_resource_id(unsigned int res_index,
> int ps3_repository_read_vuart_av_port(unsigned int *port);
> int ps3_repository_read_vuart_sysmgr_port(unsigned int *port);
>
> +/* repository ss info */
> +
> +int ps3_repository_read_ss_laid(enum ps3_ss_laid *laid);
> +
> #endif
> diff --git a/arch/powerpc/platforms/ps3/repository.c b/arch/powerpc/platforms/ps3/repository.c
> index 5e304c2..6fa3e96 100644
> --- a/arch/powerpc/platforms/ps3/repository.c
> +++ b/arch/powerpc/platforms/ps3/repository.c
> @@ -1002,6 +1002,25 @@ int ps3_repository_read_lpm_privileges(unsigned int be_index, u64 *lpar,
> lpar, rights);
> }
>
> +/**
> + * ps3_repository_read_ss_laid - Read the lpar auth id
> + */
> +
> +int ps3_repository_read_ss_laid(enum ps3_ss_laid *laid)
> +{
> + int result;
> + u64 id, v1;
> +
> + lv1_get_logical_partition_id(&id);
> + result = read_node(PS3_LPAR_ID_PME,
> + make_first_field("ss", 0),
> + make_field("laid", 0),
> + id, 0,
> + &v1, NULL);
> + *laid = v1;
> + return result;
> +}
> +
> #if defined(DEBUG)
>
> int ps3_repository_dump_resource_info(const struct ps3_repository_device *repo)
> diff --git a/arch/powerpc/platforms/ps3/setup.c b/arch/powerpc/platforms/ps3/setup.c
> index 149bea2..f430279 100644
> --- a/arch/powerpc/platforms/ps3/setup.c
> +++ b/arch/powerpc/platforms/ps3/setup.c
> @@ -47,6 +47,7 @@ DEFINE_MUTEX(ps3_gpu_mutex);
> EXPORT_SYMBOL_GPL(ps3_gpu_mutex);
>
> static union ps3_firmware_version ps3_firmware_version;
> +static enum ps3_ss_laid ps3_ss_laid;
>
> void ps3_get_firmware_version(union ps3_firmware_version *v)
> {
> @@ -68,6 +69,12 @@ int ps3_compare_firmware_version(u16 major, u16 minor, u16 rev)
> }
> EXPORT_SYMBOL_GPL(ps3_compare_firmware_version);
>
> +enum ps3_ss_laid ps3_get_ss_laid(void)
> +{
> + return ps3_ss_laid;
> +}
> +EXPORT_SYMBOL_GPL(ps3_get_ss_laid);
> +
> static void ps3_power_save(void)
> {
> /*
> @@ -192,6 +199,7 @@ static int ps3_set_dabr(unsigned long dabr)
>
> static void __init ps3_setup_arch(void)
> {
> + const char *laid_str;
>
> DBG(" -> %s:%d\n", __func__, __LINE__);
>
> @@ -200,6 +208,20 @@ static void __init ps3_setup_arch(void)
> ps3_firmware_version.major, ps3_firmware_version.minor,
> ps3_firmware_version.rev);
>
> + ps3_repository_read_ss_laid(&ps3_ss_laid);
> + switch (ps3_ss_laid) {
> + case PS3_SS_LAID_GAMEOS:
> + laid_str = "GameOS";
> + break;
> + case PS3_SS_LAID_OTHEROS:
> + laid_str = "OtherOS";
> + break;
> + default:
> + laid_str = "unknown";
> + break;
> + }
> + printk(KERN_INFO "Running in %s lpar\n", laid_str);
> +
> ps3_spu_set_platform();
>
> #ifdef CONFIG_SMP
^ permalink raw reply
* Re: [PATCH 01/15] [PS3] Add udbg driver using the PS3 gelic Ethernet device
From: Geoff Levand @ 2011-08-03 22:32 UTC (permalink / raw)
To: Andre Heider; +Cc: cbe-oss-dev, Hector Martin, linuxppc-dev
In-Reply-To: <1312228986-32307-2-git-send-email-a.heider@gmail.com>
On 08/01/2011 01:02 PM, Andre Heider wrote:
> --- /dev/null
> +++ b/arch/powerpc/platforms/ps3/gelic_udbg.c
> @@ -0,0 +1,272 @@
> +/*
> + * arch/powerpc/platforms/ps3/gelic_udbg.c
Don't put file names in files. When the file gets moved, then this will
no longer be correct.
> + *
> + * udbg debug output routine via GELIC UDP broadcasts
> + * Copyright (C) 2010 Hector Martin <hector@marcansoft.com>
> + * Copyright (C) 2011 Andre Heider <a.heider@gmail.com>
Some of this seems to be taken from the gelic driver, so shouldn't
the copyright info from there be included here?
> + * 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 <asm/io.h>
> +#include <asm/udbg.h>
> +#include <asm/lv1call.h>
> +
> +#define GELIC_BUS_ID 1
> +#define GELIC_DEVICE_ID 0
> +#define GELIC_DEBUG_PORT 18194
> +#define GELIC_MAX_MESSAGE_SIZE 1000
> +
> +#define GELIC_LV1_GET_MAC_ADDRESS 1
> +#define GELIC_LV1_GET_VLAN_ID 4
> +#define GELIC_LV1_VLAN_TX_ETHERNET_0 2
> +
> +#define GELIC_DESCR_DMA_STAT_MASK 0xf0000000
> +#define GELIC_DESCR_DMA_CARDOWNED 0xa0000000
> +
> +#define GELIC_DESCR_TX_DMA_IKE 0x00080000
> +#define GELIC_DESCR_TX_DMA_NO_CHKSUM 0x00000000
> +#define GELIC_DESCR_TX_DMA_FRAME_TAIL 0x00040000
> +
> +#define GELIC_DESCR_DMA_CMD_NO_CHKSUM (GELIC_DESCR_DMA_CARDOWNED | \
> + GELIC_DESCR_TX_DMA_IKE | \
> + GELIC_DESCR_TX_DMA_NO_CHKSUM)
> +
> +static u64 bus_addr;
> +
> +struct gelic_descr {
> + /* as defined by the hardware */
These are BE from the hardware, so should be __beXX types.
> + u32 buf_addr;
> + u32 buf_size;
> + u32 next_descr_addr;
> + u32 dmac_cmd_status;
> + u32 result_size;
> + u32 valid_size; /* all zeroes for tx */
> + u32 data_status;
> + u32 data_error; /* all zeroes for tx */
> +} __attribute__((aligned(32)));
...
> +static void gelic_debug_init(void)
> +{
...
> + result = lv1_net_control(GELIC_BUS_ID, GELIC_DEVICE_ID,
> + GELIC_LV1_GET_VLAN_ID,
> + GELIC_LV1_VLAN_TX_ETHERNET_0, 0, 0,
> + &vlan_id, &v2);
> + if (result == 0) {
This should be 'if (!result)'
-Geoff
^ permalink raw reply
* Re: [PATCH 03/15] [PS3] Add region 1 memory early
From: Geoff Levand @ 2011-08-03 22:32 UTC (permalink / raw)
To: Andre Heider; +Cc: cbe-oss-dev, Hector Martin, linuxppc-dev
In-Reply-To: <1312228986-32307-4-git-send-email-a.heider@gmail.com>
On 08/01/2011 01:02 PM, Andre Heider wrote:
> From: Hector Martin <hector@marcansoft.com>
We need an explanation of this change.
> Signed-off-by: Hector Martin <hector@marcansoft.com>
> [a.heider: Various cleanups to make checkpatch.pl happy]
> Signed-off-by: Andre Heider <a.heider@gmail.com>
> ---
> arch/powerpc/platforms/ps3/mm.c | 62 +++++++--------------------------------
> 1 files changed, 11 insertions(+), 51 deletions(-)
^ permalink raw reply
* Re: [PATCH 07/15] ps3flash: Refuse to work in lpars other than OtherOS
From: Geoff Levand @ 2011-08-03 22:34 UTC (permalink / raw)
To: Andre Heider; +Cc: cbe-oss-dev, Hector Martin, linuxppc-dev
In-Reply-To: <1312228986-32307-8-git-send-email-a.heider@gmail.com>
On 08/01/2011 01:02 PM, Andre Heider wrote:
> The driver implements a character and misc device, meant for the
> axed OtherOS to exchange various settings with GameOS.
> Since Firmware 3.21 there is no GameOS support anymore to write these
> settings, so limit the driver to the OtherOS environment.
This is really a test if running on the PS3 OtherOS, so this
comment should state that.
>
> Signed-off-by: Andre Heider <a.heider@gmail.com>
> ---
> arch/powerpc/platforms/ps3/Kconfig | 1 +
> drivers/char/ps3flash.c | 7 +++++++
> 2 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/platforms/ps3/Kconfig b/arch/powerpc/platforms/ps3/Kconfig
> index 84df5c8..5eb956a 100644
> --- a/arch/powerpc/platforms/ps3/Kconfig
> +++ b/arch/powerpc/platforms/ps3/Kconfig
> @@ -121,6 +121,7 @@ config PS3_FLASH
>
> This support is required to access the PS3 FLASH ROM, which
> contains the boot loader and some boot options.
> + This driver only supports the deprecated OtherOS LPAR.
This will be confusing for OtherOS users, so should be removed.
> In general, all users will say Y or M.
This could be changed to: 'In general, all PS3 OtherOS users will say Y or M.'
>
> As this driver needs a fixed buffer of 256 KiB of memory, it can
> diff --git a/drivers/char/ps3flash.c b/drivers/char/ps3flash.c
> index 69c734a..b1e8659 100644
> --- a/drivers/char/ps3flash.c
> +++ b/drivers/char/ps3flash.c
> @@ -25,6 +25,7 @@
>
> #include <asm/lv1call.h>
> #include <asm/ps3stor.h>
> +#include <asm/firmware.h>
>
>
> #define DEVICE_NAME "ps3flash"
> @@ -455,6 +456,12 @@ static struct ps3_system_bus_driver ps3flash = {
>
> static int __init ps3flash_init(void)
> {
> + if (!firmware_has_feature(FW_FEATURE_PS3_LV1))
> + return -ENODEV;
Is this needed? Won't this driver only be loaded on PS3 hardware?
> +
> + if (ps3_get_ss_laid() != PS3_SS_LAID_OTHEROS)
> + return -ENODEV;
> +
> return ps3_system_bus_driver_register(&ps3flash);
> }
>
-Geoff
^ permalink raw reply
* Re: [PATCH 03/15] [PS3] Add region 1 memory early
From: Hector Martin @ 2011-08-04 0:08 UTC (permalink / raw)
To: Geoff Levand; +Cc: cbe-oss-dev, Andre Heider, linuxppc-dev
In-Reply-To: <4E39CC79.3050208@infradead.org>
On 08/04/2011 12:32 AM, Geoff Levand wrote:
> We need an explanation of this change.
I actually have a hard time understanding the reason for the existing
behavior of hot-adding memory halfway through the boot process. Maybe
you can shed some light on this?
The reason for the change is that under the default GameOS LPAR, real
mode memory is 16MB which is already tight for a kernel (under certain
conditions) and runs out quickly as memory is allocated during kernel
startup. Having region1 available sooner fixes this.
Though, reviewing the code, I think I found a bug (that should already
have a chance of happening as things stand now, though this patch might
make it more likely): if storage bounce buffers or the ps3fb xdr happen
to straddle the boundary between the regions, bad things will happen
since they're not actually contiguous in LPAR space. This won't happen
right now for ps3flash or ps3fb since those are allocated early out of
bootmem, but it can currently happen for the other buffers (ps3disk,
ps3vram, etc.) AFAICT.
Maybe we should introduce a reserved or nonexistent page gap at the
beginning of region1 to ensure that nothing will ever allocate
contiguous memory across the boundary. That will probably prevent
bootmem from grabbing region1 due to the gap, so early on memory will be
tight. Can we get rid of the ps3flash and ps3fb preallocations to save
bootmem and just allocate them during device init like the other drivers
do? What is the reason for preallocating these?
--
Hector Martin (hector@marcansoft.com)
Public Key: http://www.marcansoft.com/marcan.asc
^ permalink raw reply
* Re: kvm PCI assignment & VFIO ramblings
From: David Gibson @ 2011-08-04 0:39 UTC (permalink / raw)
To: Alex Williamson
Cc: aafabbri, Alexey Kardashevskiy, kvm, Paul Mackerras,
linux-pci@vger.kernel.org, qemu-devel, chrisw, iommu,
Anthony Liguori, linuxppc-dev, benve
In-Reply-To: <1312343090.2653.564.camel@bling.home>
On Tue, Aug 02, 2011 at 09:44:49PM -0600, Alex Williamson wrote:
> On Wed, 2011-08-03 at 12:04 +1000, David Gibson wrote:
> > On Tue, Aug 02, 2011 at 12:35:19PM -0600, Alex Williamson wrote:
> > > On Tue, 2011-08-02 at 12:14 -0600, Alex Williamson wrote:
> > > > On Tue, 2011-08-02 at 18:28 +1000, David Gibson wrote:
> > > > > On Sat, Jul 30, 2011 at 12:20:08PM -0600, Alex Williamson wrote:
> > > > > > On Sat, 2011-07-30 at 09:58 +1000, Benjamin Herrenschmidt wrote:
> > > > > [snip]
> > > > > > On x86, the USB controllers don't typically live behind a PCIe-to-PCI
> > > > > > bridge, so don't suffer the source identifier problem, but they do often
> > > > > > share an interrupt. But even then, we can count on most modern devices
> > > > > > supporting PCI2.3, and thus the DisINTx feature, which allows us to
> > > > > > share interrupts. In any case, yes, it's more rare but we need to know
> > > > > > how to handle devices behind PCI bridges. However I disagree that we
> > > > > > need to assign all the devices behind such a bridge to the guest.
> > > > > > There's a difference between removing the device from the host and
> > > > > > exposing the device to the guest.
> > > > >
> > > > > I think you're arguing only over details of what words to use for
> > > > > what, rather than anything of substance here. The point is that an
> > > > > entire partitionable group must be assigned to "host" (in which case
> > > > > kernel drivers may bind to it) or to a particular guest partition (or
> > > > > at least to a single UID on the host). Which of the assigned devices
> > > > > the partition actually uses is another matter of course, as is at
> > > > > exactly which level they become "de-exposed" if you don't want to use
> > > > > all of then.
> > > >
> > > > Well first we need to define what a partitionable group is, whether it's
> > > > based on hardware requirements or user policy. And while I agree that
> > > > we need unique ownership of a partition, I disagree that qemu is
> > > > necessarily the owner of the entire partition vs individual devices.
> > >
> > > Sorry, I didn't intend to have such circular logic. "... I disagree
> > > that qemu is necessarily the owner of the entire partition vs granted
> > > access to devices within the partition". Thanks,
> >
> > I still don't understand the distinction you're making. We're saying
> > the group is "owned" by a given user or guest in the sense that no-one
> > else may use anything in the group (including host drivers). At that
> > point none, some or all of the devices in the group may actually be
> > used by the guest.
> >
> > You seem to be making a distinction between "owned by" and "assigned
> > to" and "used by" and I really don't see what it is.
>
> How does a qemu instance that uses none of the devices in a group still
> own that group?
?? In the same way that you still own a file you don't have open..?
> Aren't we at that point free to move the group to a
> different qemu instance or return ownership to the host?
Of course. But until you actually do that, the group is still
notionally owned by the guest.
> Who does that?
The admin. Possily by poking sysfs, or possibly by frobbing some
character device, or maybe something else. Naturally libvirt or
whatever could also do this.
> In my mental model, there's an intermediary that "owns" the group and
> just as kernel drivers bind to devices when the host owns the group,
> qemu is a userspace device driver that binds to sets of devices when the
> intermediary owns it. Obviously I'm thinking libvirt, but it doesn't
> have to be. Thanks,
Well sure, but I really don't see how such an intermediary fits into
the kernel's model of ownership.
So, first, take a step back and look at what sort of entities can
"own" a group (or device or whatever). I notice that when I've said
"owned by the guest" you seem to have read this as "owned by qemu"
which is not necessarily the same thing.
What I had in mind is that each group is either owned by "host", in
which case host kernel drivers can bind to it, or it's in "guest mode"
in which case it has a user, group and mode and can be bound by user
drivers (and therefore guests) with the right permission. From the
kernel's perspective there is therefore no distinction between "owned
by qemu" and "owned by libvirt".
--
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 02/15] [PS3] Get lv1 high memory region from devtree
From: Hector Martin @ 2011-08-04 1:19 UTC (permalink / raw)
To: Geoff Levand; +Cc: cbe-oss-dev, Andre Heider, linuxppc-dev
In-Reply-To: <4E39CBFC.2010501@infradead.org>
On 08/04/2011 12:30 AM, Geoff Levand wrote:
> With this mechanism how is the address of the initrd passed to the
> new kernel, in the DT?
Using the /chosen linux,initrd-{start,end} properties. The bootloader
knows about the Linux trick of sticking together bootmem and highmem and
precalculates the linux "physical" address. Yeah, that's a hack, it
should probably be done in the kernel so the bootloader doesn't have to
know or care about how Linux decides to lay out its physical address
space. Do you have any suggestion as to how we would do this sanely?
Right now early_init_dt_setup_initrd_arch in arch/powerpc/kernel/prom.c
is generic and doesn't know anything about platform specifics.
> How would a kexec based bootloader work? If it's kernel were to allocate
> high mem and the bootloader program uses the high mem, how could it tell
> that kernel not to destroy the region on shutdown?
The current code contemplates the case where a non-kexec based
bootloader is the first stage and allocates highmem (and knows how to
tell the kernel about it), possibly followed by kexec stages that just
keep that allocation. To support a kexec bootloader as the first
bootloader using this mechanism would indeed require extra support to
tell that kernel to retain its allocation, preferably something that can
be decided from userland. Of course the current kexec bootloader
behavior where highmem isn't handed over to the child kernel will still
work.
> If arch/powerpc/boot/ps3.c allocated the mem and added a DT entry
> then other OSes that don't know about the Linux device tree won't
> be able to use that allocated memory. Other OSes could do a
> test to see if the allocation was already done. Another option
> that might work is to write info into the LV1 repository then
> have boot code look there for allocated hig mem.
If you're booting another OS that isn't Linux then it also has no use
for a Linux-specific ramdisk (linux,initrd-start) and thus no use for
preallocated highmem and should be booted as such (maybe make the
userland tools tell the kernel to release highmem if there's no initrd
defined).
Using the lv1 repo is an option, but does it make sense? It's even less
standard than a FDT and we'd have to put both the region1 location and
the initrd location in there (there's no point to maintaining highmem if
you aren't going to use it).
FWIW, the lv1 repo writing hypercalls are unused and undocumented.
>> + if (!map.r1.size) {
>> + DBG("%s:%d: no region 1, not adding memory\n",
>> + __func__, __LINE__);
>> + return 0;
>> + }
>
> Did you find this to be hit? Also, in the general case,
> there could be more than one high mem region, but I don't
> know of any current systems that do.
Probably only during debugging, but it doesn't sound like a bad idea
anyway (e.g. bootloader allocated highmem but didn't tell the kernel so
the kernel couldn't allocate it).
As for multiple regions, well, currently it only supports one and that
is hardcoded in the phys->lpar translation, so I see no point in
worrying about that now.
ACK on the other code comments.
--
Hector Martin (hector@marcansoft.com)
Public Key: http://www.marcansoft.com/marcan.asc
^ 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