* Re: [RFC PATCH 16/19] powerpc: wii: hollywood interrupt controller support
From: Benjamin Herrenschmidt @ 2009-11-26 21:13 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <4B0EA1E8.6070603@yahoo.es>
On Thu, 2009-11-26 at 16:42 +0100, Albert Herranz wrote:
> Benjamin Herrenschmidt wrote:
> > On Sun, 2009-11-22 at 23:01 +0100, Albert Herranz wrote:
> >
> >> +static void hlwd_pic_mask_and_ack(unsigned int virq)
> >> +{
> >> + int irq = virq_to_hw(virq);
> >> + void __iomem *io_base = get_irq_chip_data(virq);
> >> +
> >> + clear_bit(irq, io_base + HW_BROADWAY_IMR);
> >> + set_bit(irq, io_base + HW_BROADWAY_ICR);
> >> +}
> >
> > Same comment as with flipper. BTW. It looks really similar, can't you
> > use the same driver ?
> >
>
> I'll look at it. Surely, parts of it would be mergeable.
If it's too much work, we can do that later. No big deal.
Cheers,
Ben.
> > Cheers,
> > Ben.
> >
>
> Thanks,
> Albert
>
> >
> >
^ permalink raw reply
* Re: [RFC PATCH 14/19] powerpc: allow ioremap within reserved fake ram regions
From: Benjamin Herrenschmidt @ 2009-11-26 21:13 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <4B0EA036.1090901@yahoo.es>
On Thu, 2009-11-26 at 16:35 +0100, Albert Herranz wrote:
> Benjamin Herrenschmidt wrote:
> > On Tue, 2009-11-24 at 18:09 +0100, Albert Herranz wrote:
> >> I could use ppc_md.ioremap to duplicate ioremap except for the ioremap
> >> ram check.
> >> But calling the stock ioremap without modifying it is not possible
> >> because it checks and bails out when ioremapping a region marked as
> >> ram (even if it's not real ram and it's memreserved).
> >>
> >> Is the list of memreserved areas preserved once the kernel is running?
> >> If it is preserved another option would be to unban ioremapping ram if
> >> memreserved.
> >
> > Yes, check lmb's they should still be around.
> >
>
> Good.
> So adding a kconfig option to allow ioremapping memreserved memory marked
> as ram and adding a proper check (under that kconfig) to unban this case
> in ioremap would be an acceptable solution?
Don't even make it Kconfig. Stick it under you GAMECUBE_COMMON for now
or CONFIG_WII or whatever. This is temporary until we sort out the whole
disctontig mem issue.
Cheers,
Ben.
> > Cheers,
> > Ben.
> >
>
> Thanks,
> Albert
^ permalink raw reply
* Re: [RFC PATCH 11/19] powerpc: gamecube/wii: flipper interrupt controller support
From: Benjamin Herrenschmidt @ 2009-11-26 21:12 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <4B0E9FBC.1090900@yahoo.es>
On Thu, 2009-11-26 at 16:33 +0100, Albert Herranz wrote:
> Benjamin Herrenschmidt wrote:
> > On Sun, 2009-11-22 at 16:28 -0700, Grant Likely wrote:
> >>> +unsigned int flipper_pic_get_irq(void)
> >>> +{
> >>> + void __iomem *io_base = flipper_irq_host->host_data;
> >>> + int irq;
> >>> + u32 irq_status;
> >>> +
> >>> + irq_status = in_be32(io_base + FLIPPER_ICR) &
> >>> + in_be32(io_base + FLIPPER_IMR);
> >>> + if (irq_status == 0)
> >>> + return -1; /* no more IRQs pending */
> >> NO_IRQ_IGNORE
> >
> > Why no just 0 ? (aka NO_IRQ)
> >
>
> I didn't know about 0 (I thought that was another valid interrupt).
0 is a valid hw number. It's not a valid linux "virq" number. In fact,
that's part of the reason why we generalized the whole virq thingy :-)
Because back then, Linus made the statement that 0 should never be a
valid IRQ number in linux. That also allowed us to make sure that 1..15
are never allocated to anything but a legacy 8259 to avoid all sorts of
problems with crackpot legacy x86 originated drivers who hard coded some
of these interrupts.
> I was used to -1 to tell that no IRQs were pending (at least from the
> ARCH=ppc days) :)
Yup, we changed that :-)
> > Or do you know you are getting lots of spurrious that you don't want to
> > account ?
> >
>
> No, this is not the case here.
Cheers,
Ben.
> > Cheers,
> > Ben.
> >
>
> Thanks,
> Albert
^ permalink raw reply
* Re: [RFC PATCH 11/19] powerpc: gamecube/wii: flipper interrupt controller support
From: Benjamin Herrenschmidt @ 2009-11-26 21:10 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <4B0E9F11.5000605@yahoo.es>
On Thu, 2009-11-26 at 16:30 +0100, Albert Herranz wrote:
> Benjamin Herrenschmidt wrote:
> > On Sun, 2009-11-22 at 23:01 +0100, Albert Herranz wrote:
> >
> >> +static void flipper_pic_mask_and_ack(unsigned int virq)
> >> +{
> >> + int irq = virq_to_hw(virq);
> >> + void __iomem *io_base = get_irq_chip_data(virq);
> >> +
> >> + clear_bit(irq, io_base + FLIPPER_IMR);
> >> + set_bit(irq, io_base + FLIPPER_ICR);
> >> +}
> >
> > Do not use clear_bit and set_bit on IOs. They will do lwarx/stwcx. which
> > is really not what you want. You can use __clear_bit and __set_bit but
> > it's still fishy. Those operate on unsigned long, so the size vary
> > between 32 and 64 bit etc... not something you care that much about, but
> > it's still the wrong tool for the job.
> >
> > Do those guys have more than 32 interrupts ? If not, just hand
> > code the msak & shifts. If they do, then maybe stick with __clear_bit()
> > and __set_bit() which are the non atomic variants.
> >
>
> There can be only 32 interrupt sources per pic.
> I'll build a mask and check if just a simple write works too (it should IMHO), instead of a RWM.
For the ICR just writes should be ok. For the IMR, I wouldn't be
surprised if you need to RMW. In which case you have two options:
- One is you just do RMW :-) As long as you don't do SMP, you don't
have a problem since these callbacks are generally called with local
IRQs off.
- Or you could explicitely add a spinlock_irqsave just out of paranoia
or if you envision SMP type access to these things (from the aux
processor ? But then the spinlock would have to be shared with the aux
ptiocrocessor FW... fun)
- In any case, clear_bit and set_bit are the wrong accessors since
lwarx and stwcx. are definitely the wrong instructions to use for non
cachable accesses. On other processor you probably would have blown up
here in fact. Also those accessors don't provide any memory barriers
which mean you may even have some ordering issues. Use readl_be() and
writel_be() (or the ppc specific in_be32/out_be32) or the _le variants
if you want to count bits backward :-)
- You could also keep in memory cache of what the mask is supposed to
be. You RMW the cache and do a simple store to the register. It won't
fix the concurrency problem (that you probably don't have anyways) but
it will make things faster by avoiding an MMIO load....
- However you probably need that MMIO load anyways :-) Trick is, the
store you do to ack and mask an interrupt may take some time to hit the
PIC before the said PIC actually lowers the CPU EE line. Enough time for
your code to go back, re-enable MSR:EE and start calling into the
handlers. Enough time thus to catch a "spurrious" interrupt. Old pmacs
have a similar PIC and have the same problem. I would recommend that
after a mask or a mask & ack, you also read back the register. The MMIO
accessors (readl, etc...) will use a twi;isync construct to ensure that
the load has been fully completed before execution continues which will
help.
- The construct and flow handler you use assume all interrupts are
level sensitive, this is the case right ?
Cheers,
Ben.
> > Cheers,
> > Ben.
> >
>
> Thanks,
> Albert
^ permalink raw reply
* Re: [RFC PATCH 09/19] powerpc: gamecube/wii: udbg support for usbgecko
From: Benjamin Herrenschmidt @ 2009-11-26 21:02 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <4B0E9E84.4060400@yahoo.es>
On Thu, 2009-11-26 at 16:28 +0100, Albert Herranz wrote:
> Benjamin Herrenschmidt wrote:
> > On Sun, 2009-11-22 at 23:01 +0100, Albert Herranz wrote:
> >> Add support for using the USB Gecko adapter via the udbg facility on
> >> the Nintendo GameCube and Wii video game consoles.
> >> The USB Gecko is a 3rd party memory card interface adapter that provides
> >> a EXI (External Interface) to USB serial converter.
> >
> > The main issue here is that the gecko is supposed to be something you
> > can plug or unplug no ? Is it hotswap or only at boot ? In the later
> > case, maybe the node should be created by the boot wrapper when
> > detecting the presence of the device. If it's hotplug, then we need
> > something smarter...
> >
>
> The usbgecko is hotplugable and hotswappable.
> But as this is mostly a developer feature, not normaly used by end users,
> I think that we can just let it be as it is: autodetect it on boot (now probing
> for it instead of using information from the device tree).
> If you unplug it later it causes no errors, you just miss whatever data is sent to it.
Ack. We may still went ultimately to have some kind of nice hotplug
driver for that connector and we may even have a usbgecko driver for
that things "after boot" that creates areal tty etc... but for now, just
a hard probe will do just fine.
Of course a real OF implemention is welcome to create a device node for
it if it was indeed present at boot time.
Cheers,
Ben.
> > Cheers,
> > Ben.
> >
>
> Thanks,
> Albert
>
^ permalink raw reply
* Re: [RFC PATCH 04/19] powerpc: wii: device tree
From: Benjamin Herrenschmidt @ 2009-11-26 21:01 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <4B0E9C5F.50304@yahoo.es>
> > Good point. I can't even guarantee that the kernel will work reliably
> > with nobats :-) At least you really want the kernel .text to be fully
> > covered by an IBAT.
> >
>
> It works with nobats.
But does it work -reliably- ? :-)
I haven't looked at that for years, but we used to have a subtle issue
if you happen to take a hash miss on the kernel text or data in the
wrong time, such as when SRR0/SRR1 are modified and before a subsequent
rfi. This is very very hard to trigger and maybe impossible without SMP
but to keep in mind.
Paulus added some code ages ago to close most of these by using the
MSR:RI bit so that the hash code could detect the situation and branch
to some "recovery" code, but from memory, while that dealt with missing
D-BATs, we still had a potential window of problem if the kernel text
itself wasn't entirely covered.
Any ways, not a big deal right now, as I said, we really want the BATs
for performances anyways, so we should probably just add some kind of
hack in mmu_mapin_ram() for the time being.
> I must say that all the patches posted (and the device drivers, which haven't
> been posted yet) are tested and working code.
That was my impression too, but in this case, I'm talking about a
potential very hard to hit problem that you may well have never managed
to actually trigger.
> There you can find the hardware interface that supports the IPC mechanism.
> It is made up of a pair of registers to pass data between the processors and a
> pair of control/flags registers.
> The hardware can interrupt the PowerPC side when there is data available for it.
Ok. So the right way to do that would be to have a node purely
representing the HW IPC, unrelated to whatever is running on the
secondary processor.
However, it's ok to have -below- that node, a set of device nodes or a
node with properties or whatever representing the FW in there and the
function it exposes.
That can be discussed later tho. I'm not that keen on having those info
be in the .dts coming with the kernel since those functions essentially
depend on what FW is loaded on the aux. processor.
What might do however is to have a way for that FW itself to provide you
with the nodes and properties for the functions it provides :-) Then you
can have the boot wrapper or the kernel platform init code use some well
defined (and as stable as possible) IPC API to identify the FW there and
expose all that stuff.
Of course that wouldn't work with FW we don't have control on. Can Linux
run on the wii with the original N. FW on the aux. processor ? Can we
detect what is running there ? Do we care ?
> It is what Nintendo calls the "Serial Interface" (SI) which is a proprietary
> serial hardware to drive the gamepads (and a custom keyboard too, once available
> for an RPG game).
So I would give it a different name than "serial" then. Make it gpsi
maybe ? (game pad serial interface ?) :-) Or invent something else...
Cheers,
Ben.
> > Cheers,
> > Ben.
> >
>
> Thanks,
> Albert
^ permalink raw reply
* Re: [RFC PATCH 04/19] powerpc: wii: device tree
From: Benjamin Herrenschmidt @ 2009-11-26 20:48 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <4B0E9A0F.9010203@yahoo.es>
On Thu, 2009-11-26 at 16:09 +0100, Albert Herranz wrote:
> > Are the above OHCI and EHCI "special" ? If not, there's an existing
> > binding for that sort of thing, which btw requires properties to
> > indicate the endianness of the registers and in-memory data structures
> > etc...
> >
>
> They are a bit special because registers are in reverse little endian format,
> must be written in 32-bit chunks, and (all things point to) they have hardware bugs.
Well.. first what is "reverse little endian" ? :-) Big endian ?
The OHCI driver today has separate flags to control register endianness
and memory based data structures endianness.
I think we also only use 32-bit reads and writes no ? So that should be
covered :-)
As for HW bugs, well, as long as we know them we can define a quirk bit
and add the necessary workarounds to the driver :-)
Do you have a patch somewhere that adds the OCHI and EHCI support btw ?
Cheers,
Ben.
^ permalink raw reply
* [PATCH v6 2/2] pseries: Serialize cpu hotplug operations during deactivate Vs deallocate
From: Vaidyanathan Srinivasan @ 2009-11-26 19:59 UTC (permalink / raw)
To: Nathan Fontenot, Benjamin Herrenschmidt
Cc: Peter Zijlstra, Gautham R Shenoy, linux-kernel, Arun R Bharadwaj,
Andrew Morton, linuxppc-dev, Ingo Molnar
In-Reply-To: <20091126195121.29159.66098.stgit@drishya>
From: Gautham R Shenoy <ego@in.ibm.com>
Currently the cpu-allocation/deallocation process comprises of two steps:
- Set the indicators and to update the device tree with DLPAR node
information.
- Online/offline the allocated/deallocated CPU.
This is achieved by writing to the sysfs tunables "probe" during allocation
and "release" during deallocation.
At the sametime, the userspace can independently online/offline the CPUs of
the system using the sysfs tunable "online".
It is quite possible that when a userspace tool offlines a CPU
for the purpose of deallocation and is in the process of updating the device
tree, some other userspace tool could bring the CPU back online by writing to
the "online" sysfs tunable thereby causing the deallocate process to fail.
The solution to this is to serialize writes to the "probe/release" sysfs
tunable with the writes to the "online" sysfs tunable.
This patch employs a mutex to provide this serialization, which is a no-op on
all architectures except PPC_PSERIES
Signed-off-by: Gautham R Shenoy <ego@in.ibm.com>
Acked-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>
---
arch/powerpc/platforms/pseries/dlpar.c | 45 +++++++++++++++++++++++++-------
drivers/base/cpu.c | 2 +
include/linux/cpu.h | 13 +++++++++
3 files changed, 50 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index 642e1b2..fd2f0af 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -436,6 +436,18 @@ int dlpar_release_drc(u32 drc_index)
#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
+static DEFINE_MUTEX(pseries_cpu_hotplug_mutex);
+
+void cpu_hotplug_driver_lock()
+{
+ mutex_lock(&pseries_cpu_hotplug_mutex);
+}
+
+void cpu_hotplug_driver_unlock()
+{
+ mutex_unlock(&pseries_cpu_hotplug_mutex);
+}
+
static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
{
struct device_node *dn;
@@ -443,13 +455,18 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
char *cpu_name;
int rc;
+ cpu_hotplug_driver_lock();
rc = strict_strtoul(buf, 0, &drc_index);
- if (rc)
- return -EINVAL;
+ if (rc) {
+ rc = -EINVAL;
+ goto out;
+ }
dn = dlpar_configure_connector(drc_index);
- if (!dn)
- return -EINVAL;
+ if (!dn) {
+ rc = -EINVAL;
+ goto out;
+ }
/* configure-connector reports cpus as living in the base
* directory of the device tree. CPUs actually live in the
@@ -459,7 +476,8 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
GFP_KERNEL);
if (!cpu_name) {
dlpar_free_cc_nodes(dn);
- return -ENOMEM;
+ rc = -ENOMEM;
+ goto out;
}
sprintf(cpu_name, "/cpus%s", dn->full_name);
@@ -469,7 +487,8 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
rc = dlpar_acquire_drc(drc_index);
if (rc) {
dlpar_free_cc_nodes(dn);
- return -EINVAL;
+ rc = -EINVAL;
+ goto out;
}
rc = dlpar_attach_node(dn);
@@ -479,6 +498,8 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
}
rc = online_node_cpus(dn);
+out:
+ cpu_hotplug_driver_unlock();
return rc ? rc : count;
}
@@ -499,26 +520,30 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t count)
return -EINVAL;
}
+ cpu_hotplug_driver_lock();
rc = offline_node_cpus(dn);
if (rc) {
of_node_put(dn);
- return -EINVAL;
+ rc = -EINVAL;
+ goto out;
}
rc = dlpar_release_drc(*drc_index);
if (rc) {
of_node_put(dn);
- return -EINVAL;
+ goto out;
}
rc = dlpar_detach_node(dn);
if (rc) {
dlpar_acquire_drc(*drc_index);
- return rc;
+ goto out;
}
of_node_put(dn);
- return count;
+out:
+ cpu_hotplug_driver_unlock();
+ return rc ? rc : count;
}
static int __init pseries_dlpar_init(void)
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 7c03af7..27fd775 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -35,6 +35,7 @@ static ssize_t __ref store_online(struct sys_device *dev, struct sysdev_attribut
struct cpu *cpu = container_of(dev, struct cpu, sysdev);
ssize_t ret;
+ cpu_hotplug_driver_lock();
switch (buf[0]) {
case '0':
ret = cpu_down(cpu->sysdev.id);
@@ -49,6 +50,7 @@ static ssize_t __ref store_online(struct sys_device *dev, struct sysdev_attribut
default:
ret = -EINVAL;
}
+ cpu_hotplug_driver_unlock();
if (ret >= 0)
ret = count;
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index c972f7c..e287863 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -117,6 +117,19 @@ extern void put_online_cpus(void);
#define unregister_hotcpu_notifier(nb) unregister_cpu_notifier(nb)
int cpu_down(unsigned int cpu);
+#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
+extern void cpu_hotplug_driver_lock(void);
+extern void cpu_hotplug_driver_unlock(void);
+#else
+static inline void cpu_hotplug_driver_lock(void)
+{
+}
+
+static inline void cpu_hotplug_driver_unlock(void)
+{
+}
+#endif
+
#else /* CONFIG_HOTPLUG_CPU */
#define get_online_cpus() do { } while (0)
^ permalink raw reply related
* [PATCH v6 1/2] pseries: Add code to online/offline CPUs of a DLPAR node
From: Vaidyanathan Srinivasan @ 2009-11-26 19:58 UTC (permalink / raw)
To: Nathan Fontenot, Benjamin Herrenschmidt
Cc: Peter Zijlstra, Gautham R Shenoy, linux-kernel, Arun R Bharadwaj,
Andrew Morton, linuxppc-dev, Ingo Molnar
In-Reply-To: <20091126195121.29159.66098.stgit@drishya>
From: Gautham R Shenoy <ego@in.ibm.com>
Currently the cpu-allocation/deallocation on pSeries is a
two step process from the Userspace.
- Set the indicators and update the device tree by writing to the sysfs
tunable "probe" during allocation and "release" during deallocation.
- Online / Offline the CPUs of the allocated/would_be_deallocated node by
writing to the sysfs tunable "online".
This patch adds kernel code to online/offline the CPUs soon_after/just_before
they have been allocated/would_be_deallocated. This way, the userspace tool
that performs DLPAR operations would only have to deal with one set of sysfs
tunables namely "probe" and release".
Signed-off-by: Gautham R Shenoy <ego@in.ibm.com>
Signed-off-by: Nathan Fontenot <nfont@austin.ibm.com>
Acked-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>
---
arch/powerpc/platforms/pseries/dlpar.c | 101 ++++++++++++++++++++++++++++++++
1 files changed, 101 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index fe8d4b3..642e1b2 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -16,6 +16,7 @@
#include <linux/proc_fs.h>
#include <linux/spinlock.h>
#include <linux/cpu.h>
+#include "offline_states.h"
#include <asm/prom.h>
#include <asm/machdep.h>
@@ -287,6 +288,98 @@ int dlpar_detach_node(struct device_node *dn)
return 0;
}
+int online_node_cpus(struct device_node *dn)
+{
+ int rc = 0;
+ unsigned int cpu;
+ int len, nthreads, i;
+ const u32 *intserv;
+
+ intserv = of_get_property(dn, "ibm,ppc-interrupt-server#s", &len);
+ if (!intserv)
+ return -EINVAL;
+
+ nthreads = len / sizeof(u32);
+
+ cpu_maps_update_begin();
+ for (i = 0; i < nthreads; i++) {
+ for_each_present_cpu(cpu) {
+ if (get_hard_smp_processor_id(cpu) != intserv[i])
+ continue;
+ BUG_ON(get_cpu_current_state(cpu)
+ != CPU_STATE_OFFLINE);
+ cpu_maps_update_done();
+ rc = cpu_up(cpu);
+ if (rc)
+ goto out;
+ cpu_maps_update_begin();
+
+ break;
+ }
+ if (cpu == num_possible_cpus())
+ printk(KERN_WARNING "Could not find cpu to online "
+ "with physical id 0x%x\n", intserv[i]);
+ }
+ cpu_maps_update_done();
+
+out:
+ return rc;
+
+}
+
+int offline_node_cpus(struct device_node *dn)
+{
+ int rc = 0;
+ unsigned int cpu;
+ int len, nthreads, i;
+ const u32 *intserv;
+
+ intserv = of_get_property(dn, "ibm,ppc-interrupt-server#s", &len);
+ if (!intserv)
+ return -EINVAL;
+
+ nthreads = len / sizeof(u32);
+
+ cpu_maps_update_begin();
+ for (i = 0; i < nthreads; i++) {
+ for_each_present_cpu(cpu) {
+ if (get_hard_smp_processor_id(cpu) != intserv[i])
+ continue;
+
+ if (get_cpu_current_state(cpu) == CPU_STATE_OFFLINE)
+ break;
+
+ if (get_cpu_current_state(cpu) == CPU_STATE_ONLINE) {
+ cpu_maps_update_done();
+ rc = cpu_down(cpu);
+ if (rc)
+ goto out;
+ cpu_maps_update_begin();
+ break;
+
+ }
+
+ /*
+ * The cpu is in CPU_STATE_INACTIVE.
+ * Upgrade it's state to CPU_STATE_OFFLINE.
+ */
+ set_preferred_offline_state(cpu, CPU_STATE_OFFLINE);
+ BUG_ON(plpar_hcall_norets(H_PROD, intserv[i])
+ != H_SUCCESS);
+ __cpu_die(cpu);
+ break;
+ }
+ if (cpu == num_possible_cpus())
+ printk(KERN_WARNING "Could not find cpu to offline "
+ "with physical id 0x%x\n", intserv[i]);
+ }
+ cpu_maps_update_done();
+
+out:
+ return rc;
+
+}
+
#define DR_ENTITY_SENSE 9003
#define DR_ENTITY_PRESENT 1
#define DR_ENTITY_UNUSABLE 2
@@ -385,6 +478,8 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
dlpar_free_cc_nodes(dn);
}
+ rc = online_node_cpus(dn);
+
return rc ? rc : count;
}
@@ -404,6 +499,12 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t count)
return -EINVAL;
}
+ rc = offline_node_cpus(dn);
+ if (rc) {
+ of_node_put(dn);
+ return -EINVAL;
+ }
+
rc = dlpar_release_drc(*drc_index);
if (rc) {
of_node_put(dn);
^ permalink raw reply related
* [PATCH v6 0/2] pseries: Add cede support for cpu-offline
From: Vaidyanathan Srinivasan @ 2009-11-26 19:58 UTC (permalink / raw)
To: Nathan Fontenot, Benjamin Herrenschmidt
Cc: Peter Zijlstra, Gautham R Shenoy, linux-kernel, Arun R Bharadwaj,
Andrew Morton, linuxppc-dev, Ingo Molnar
Hi,
This is version 6 of patch series that provides a framework to choose the state
a pseries CPU must be put to when it is offlined.
Previous versions can be found here:
Version 5: http://lkml.org/lkml/2009/10/30/6
Version 4: http://lkml.org/lkml/2009/10/9/59
Version 3: http://lkml.org/lkml/2009/9/15/164
Version 2: http://lkml.org/lkml/2009/8/28/102
Version 1: http://lkml.org/lkml/2009/8/6/236
Changes from the previous version include:
- Built on Nathan Fontenot's v3 of "Kernel handling of Dynamic
Logical Partitioning" http://lkml.org/lkml/2009/11/25/21
- Rebased to powerpc.git tree and hence dropped 1st and 2nd patch in
the stack since they are already in the powerpc.git tree:
With reference to previous version,
Dropped:
1/4 pSeries: extended_cede_processor() helper function
2/4 pSeries: Add hooks to put the CPU into an appropriate offline state
Posting only:
3/4 pseries: Add code to online/offline CPUs of a DLPAR node
4/4 pseries: Serialize cpu hotplug operations during deactivate Vs deallocate
Minor changes in the above patchs due to changes in Nathan's routines.
Also,
- This approach addresses Peter Z's objections regarding layering
violations. The user simply offlines the cpu and doesn't worry about what
state the CPU should be put into. That part is automatically handled by the
kernel.
Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
http://lkml.org/lkml/2009/11/11/328
- It does not add any additional sysfs interface instead uses the existing
sysfs interface to offline CPUs.
- On platforms which do not have support for ceding the vcpu with a
latency specifier value, the offlining mechanism defaults to the current
method of calling rtas_stop_self().
This patchset is based on powerpc.git + Nathan's patches and has been built and
tested on pseries platforms. This series can be applied to powerpc.git after
Nathan's patches.
Thanks,
Vaidy
---
Gautham R Shenoy (2):
pseries: Serialize cpu hotplug operations during deactivate Vs deallocate
pseries: Add code to online/offline CPUs of a DLPAR node
arch/powerpc/platforms/pseries/dlpar.c | 144 ++++++++++++++++++++++++++++++--
drivers/base/cpu.c | 2
include/linux/cpu.h | 13 +++
3 files changed, 150 insertions(+), 9 deletions(-)
--
^ permalink raw reply
* Re: [PATCH] Reserve memory for kdump kernel within RMO region
From: Bernhard Walle @ 2009-11-26 19:26 UTC (permalink / raw)
To: M. Mohan Kumar; +Cc: kexec, ppcdev
In-Reply-To: <4B0E628A.9070009@in.ibm.com>
M. Mohan Kumar schrieb:
> On 11/26/2009 12:22 AM, Bernhard Walle wrote:
>> M. Mohan Kumar schrieb:
>>> Reserve memory for kdump kernel within RMO region
>>>
>>> When the kernel size exceeds 32MB(observed with some distros), memory
>>> for kdump kernel can not be reserved as kdump kernel base is assumed to
>>> be 32MB always. When the kernel has CONFIG_RELOCATABLE option enabled,
>>> provide the feature to reserve the memory for kdump kernel anywhere in
>>> the RMO region.
>
> Hi Bernhard,
>
>> Correct me if I'm wrong, but: CONFIG_RELOCATABLE is for the kernel that
>> gets loaded as crashkernel, not for the kernel that loads the
>> crashkernel. So it would be perfectly fine that a kernel that has not
>> CONFIG_RELOCATABLE set would load another kernel that has
>> CONFIG_RELOCATABLE set on an address != 32 M.
>
> No, with relocatable option, the same kernel is used as both production
> and kdump kernel.
Can be, but it's not strictly necessary. It depends what userland does.
Especially it's possible that a non-relocatable, self-compiled kernel
loads a relocatable distribution kernel as capture kernel.
Also, it would make sense to make the behaviour symmetric across
platforms. Currently we have:
- x86 and ia64: Without offset on command line, use any offset
With offset on command line, use that offset and fail
if no memory is available at that offset.
- ppc64: Always use 32M and ignore the offset.
If your patch gets applied, we have:
- ppc64: With CONFIG_RELOCATABLE, use any offset
With offset on command
I don't see why the behaviour on ppc64 should be completely different.
Having maintained kdump for SUSE for x86, ia64 and partly ppc64 in the
past, I always felt that ppc64 is more different from x86 than ia64 is
from x86. That's one more step into that direction without a technical
reason.
Having that all said: If your patch gets in mainline kernel, than we
should change the behaviour also for x86 and ia64.
Regards,
Bernhard
^ permalink raw reply
* Re: spi_mpc8xxx.c: chip select polarity problem
From: Grant Likely @ 2009-11-26 19:17 UTC (permalink / raw)
To: avorontsov; +Cc: Torsten Fleischer, spi-devel-general, linuxppc-dev
In-Reply-To: <20091126190101.GA19404@oksana.dev.rtsoft.ru>
On Thu, Nov 26, 2009 at 12:01 PM, Anton Vorontsov
<avorontsov@ru.mvista.com> wrote:
> On Thu, Nov 26, 2009 at 11:50:05AM -0700, Grant Likely wrote:
>> On Thu, Nov 26, 2009 at 11:41 AM, Anton Vorontsov
>> <avorontsov@ru.mvista.com> wrote:
>> > On Thu, Nov 26, 2009 at 11:16:34AM -0700, Grant Likely wrote:
>> > [...]
>> >> The spi-cs-high property is defined in
>> >> Documentation/powerpc/dts-bindings/spi-bus.txt, but it definitely was
>> >> a mistake
>> >
>> > Yup.
>> >
>> >> Currently the spi-cs-high property is parsed in the
>> >> of_register_spi_devices() function, but the CS polarity needs to be
>> >> known before registering devices. =A0It needs to be factored out into
>> >> another utility function callable by spi bus drivers so that it can
>> >> get polarity data at probe time.
>> >
>> > Untill we have this, Torsten's patch is a real improvement, and
>> > works for non-broken hw/fw.
>> >
>> > So I think it should be applied.
>>
>> I disagree since it only band-aids the problem and uglifies the driver
>> in the process. =A0In the immediate term the driver needs to be changed
>> to read the spi-cs-high property out of the child nodes before
>> registering the devices.
>
> Hm. I thought we agreed that spi-cs-high is not good? Why do you
> encourage using it then? We'll have to uglify the driver with
> legacy device-tree handling code.
spi-cs-high is definitely not a complete solution, but it isn't
actively evil either. Plus it is documented and (presumably) in
active use. so support for it should not be dropped.
Regardless, there needs to be a library function for parsing all the
SPI child nodes and returning the active state for each GPIO chip
select. All the code for parsing the old spi-cs-high properties can
be contained in the same place as a new yet-to-be-defined bus node cs
polarity property. The rework to the driver itself is not ugly.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: spi_mpc8xxx.c: chip select polarity problem
From: Anton Vorontsov @ 2009-11-26 19:01 UTC (permalink / raw)
To: Grant Likely; +Cc: Torsten Fleischer, spi-devel-general, linuxppc-dev
In-Reply-To: <fa686aa40911261050g3f7ae231mc82c41f058ff8530@mail.gmail.com>
On Thu, Nov 26, 2009 at 11:50:05AM -0700, Grant Likely wrote:
> On Thu, Nov 26, 2009 at 11:41 AM, Anton Vorontsov
> <avorontsov@ru.mvista.com> wrote:
> > On Thu, Nov 26, 2009 at 11:16:34AM -0700, Grant Likely wrote:
> > [...]
> >> The spi-cs-high property is defined in
> >> Documentation/powerpc/dts-bindings/spi-bus.txt, but it definitely was
> >> a mistake
> >
> > Yup.
> >
> >> Currently the spi-cs-high property is parsed in the
> >> of_register_spi_devices() function, but the CS polarity needs to be
> >> known before registering devices. It needs to be factored out into
> >> another utility function callable by spi bus drivers so that it can
> >> get polarity data at probe time.
> >
> > Untill we have this, Torsten's patch is a real improvement, and
> > works for non-broken hw/fw.
> >
> > So I think it should be applied.
>
> I disagree since it only band-aids the problem and uglifies the driver
> in the process. In the immediate term the driver needs to be changed
> to read the spi-cs-high property out of the child nodes before
> registering the devices.
Hm. I thought we agreed that spi-cs-high is not good? Why do you
encourage using it then? We'll have to uglify the driver with
legacy device-tree handling code.
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: spi_mpc8xxx.c: chip select polarity problem
From: Grant Likely @ 2009-11-26 18:50 UTC (permalink / raw)
To: avorontsov; +Cc: Torsten Fleischer, spi-devel-general, linuxppc-dev
In-Reply-To: <20091126184157.GA16607@oksana.dev.rtsoft.ru>
On Thu, Nov 26, 2009 at 11:41 AM, Anton Vorontsov
<avorontsov@ru.mvista.com> wrote:
> On Thu, Nov 26, 2009 at 11:16:34AM -0700, Grant Likely wrote:
> [...]
>> The spi-cs-high property is defined in
>> Documentation/powerpc/dts-bindings/spi-bus.txt, but it definitely was
>> a mistake
>
> Yup.
>
>> Currently the spi-cs-high property is parsed in the
>> of_register_spi_devices() function, but the CS polarity needs to be
>> known before registering devices. =A0It needs to be factored out into
>> another utility function callable by spi bus drivers so that it can
>> get polarity data at probe time.
>
> Untill we have this, Torsten's patch is a real improvement, and
> works for non-broken hw/fw.
>
> So I think it should be applied.
I disagree since it only band-aids the problem and uglifies the driver
in the process. In the immediate term the driver needs to be changed
to read the spi-cs-high property out of the child nodes before
registering the devices. I'm not going to apply this patch.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: spi_mpc8xxx.c: chip select polarity problem
From: Anton Vorontsov @ 2009-11-26 18:41 UTC (permalink / raw)
To: Grant Likely; +Cc: Torsten Fleischer, spi-devel-general, linuxppc-dev
In-Reply-To: <fa686aa40911261016p489981a2g1fac393264c4d1c7@mail.gmail.com>
On Thu, Nov 26, 2009 at 11:16:34AM -0700, Grant Likely wrote:
[...]
> The spi-cs-high property is defined in
> Documentation/powerpc/dts-bindings/spi-bus.txt, but it definitely was
> a mistake
Yup.
> Currently the spi-cs-high property is parsed in the
> of_register_spi_devices() function, but the CS polarity needs to be
> known before registering devices. It needs to be factored out into
> another utility function callable by spi bus drivers so that it can
> get polarity data at probe time.
Untill we have this, Torsten's patch is a real improvement, and
works for non-broken hw/fw.
So I think it should be applied.
Thanks,
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: spi_mpc8xxx.c: chip select polarity problem
From: Grant Likely @ 2009-11-26 18:18 UTC (permalink / raw)
To: Torsten Fleischer; +Cc: spi-devel-general, linuxppc-dev
In-Reply-To: <200911261827.38405.to-fleischer@t-online.de>
On Thu, Nov 26, 2009 at 10:27 AM, Torsten Fleischer
<to-fleischer@t-online.de> wrote:
> On Thu, Nov 26, 2009 at 13:12:04 Anton Vorontsov wrote:
> [...]
>> > Ah. =A0I understand what you're doing now. =A0 Hmmm. =A0This approach
>> > concerns me because it relies on firmware or platform code to get CS
>> > gpios set up properly before the driver is probed.
>>
>> Yes, that was said at the very beginning of this thread.
>>
>> > =A0Firmware doesn't
>> > always get it right, and I prefer to avoid platform specific setups as
>> > much as possible. =A0Why can't the CS polarity be encoded into the
>> > device tree so the driver *does* have the polarity data at probe time?
>>
>> We have the spi-cs-high property, but it duplicates compatible
>> property. 'compatible' is enough to tell whether some device has
>> cs-high or cs-low (device's driver knows that already).
>>
>> The problem is that SPI bus master doesn't know all the devices,
>> so it can't extract that information from the compatible string.
>> To workaround that we can use 'spi-cs-high', but that's ugly
>> workaround.
>>
>> SPI modes (0,1,2,3) is another matter, some devices can work in
>> several modes, so 'spi-cpol' and 'spi-cpha' are actually useful.
>>
> To get a sane initial state the needed GPIOs can be set to be inputs duri=
ng
> the driver's initialization.
> This requires pull-up/pull-down resistors connected to the chip select
> lines. I think we can assume that they exist, because the GPIOs are all i=
nputs
> after the controller's hardware reset and the resistors are needed to hav=
e a
> well-defined voltage level on the chip select lines. Normally the level i=
s set
> so that the devices are disabled.
>
> Therefore, it doesn't matter if the firmware sets the GPIOs wrong.
No, that's just shifting responsibility from firmware to hardware.
There is just as much broken hardware out there as broken firmware.
The assumption cannot be made that the initial state of the pin is the
inactive state of the CS line. Plus, some GPIO pins are output only
and the inital state cannot be read.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: spi_mpc8xxx.c: chip select polarity problem
From: Grant Likely @ 2009-11-26 18:16 UTC (permalink / raw)
To: avorontsov; +Cc: Torsten Fleischer, spi-devel-general, linuxppc-dev
In-Reply-To: <20091126121204.GA3558@oksana.dev.rtsoft.ru>
On Thu, Nov 26, 2009 at 5:12 AM, Anton Vorontsov
<avorontsov@ru.mvista.com> wrote:
> On Wed, Nov 25, 2009 at 03:11:57PM -0700, Grant Likely wrote:
>> On Wed, Nov 25, 2009 at 1:41 PM, Torsten Fleischer
>> <to-fleischer@t-online.de> wrote:
>> > On Wen, Nov 25, 2009 at 01:33:57 Grant Likely wrote:
>> >> Thanks. =A0However, there needs to be a proper description of what th=
is
>> >> patch does to go in the commit header. =A0Can you please write one?
>> >>
>> >> Thanks,
>> >> g.
>> >>
>> > [...]
>> >
>> > The initialization of the chip selects is removed from the probe() fun=
ction of
>> > the spi_mpc8xxx driver, because the driver doesn't know the polarity o=
f the
>> > chip selects of the SPI devices at the time of its initialization.
>> >
>> > For this reason the initialization of the several chip selects is post=
poned
>> > to the point of time when the very first SPI transfer to the associate=
d device
>> > occurs.
>> >
>> >
>> > Signed-off-by: Torsten Fleischer <to-fleischer@t-online.de>
>>
>> Ah. =A0I understand what you're doing now. =A0 Hmmm. =A0This approach
>> concerns me because it relies on firmware or platform code to get CS
>> gpios set up properly before the driver is probed.
>
> Yes, that was said at the very beginning of this thread.
I also came in part way through as I wasn't an SPI maintainer when
this thread started. :-)
>> =A0Firmware doesn't
>> always get it right, and I prefer to avoid platform specific setups as
>> much as possible. =A0Why can't the CS polarity be encoded into the
>> device tree so the driver *does* have the polarity data at probe time?
>
> We have the spi-cs-high property, but it duplicates compatible
> property. 'compatible' is enough to tell whether some device has
> cs-high or cs-low (device's driver knows that already).
But the device's driver isn't controlling the CS line, the SPI bus
driver is. Besides, there is no guarantee that all drivers will
actualy be loaded before something starts using SPI. The bus driver
*must* know what the active state of each CS line is before activating
any devices.
The spi bus binding is deficient in this case. A property (can be
optional) need to be added to the spi bus node to explicitly state the
CS polarities. It's not entirely sane to look for a "spi-cs-high'
property in the spi device nodes because the SPI bus may not be fully
populated (ie. if a device happens to be sitting on the bus, but isn't
in the device tree yet). Before any SPI transactions go out, it is
the responsibility of the bus driver to ensure all CS lines are in the
correct state.
> The problem is that SPI bus master doesn't know all the devices,
> so it can't extract that information from the compatible string.
> To workaround that we can use 'spi-cs-high', but that's ugly
> workaround.
It doesn't need to know about the devices, but is must know how all of
its CS lines behave. So it isn't an really an ugly workaround, but I
do think the binding is insufficient for the SPI bus driver (see
below)
> SPI modes (0,1,2,3) is another matter, some devices can work in
> several modes, so 'spi-cpol' and 'spi-cpha' are actually useful.
yes. spi-cpol and spi-cpha are actually properties of the device, and
belong in the spi device node, not the spi bus node.
The spi-cs-high property is defined in
Documentation/powerpc/dts-bindings/spi-bus.txt, but it definitely was
a mistake for the reasons I described above. It does work in some
cases, but a property at the bus node would be better. The driver can
still fallback to looking for spi-cs-high properties in child nodes.
Currently the spi-cs-high property is parsed in the
of_register_spi_devices() function, but the CS polarity needs to be
known before registering devices. It needs to be factored out into
another utility function callable by spi bus drivers so that it can
get polarity data at probe time.
Cheers,
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: spi_mpc8xxx.c: chip select polarity problem
From: Torsten Fleischer @ 2009-11-26 17:27 UTC (permalink / raw)
To: avorontsov; +Cc: spi-devel-general, linuxppc-dev
In-Reply-To: <20091126121204.GA3558@oksana.dev.rtsoft.ru>
On Thu, Nov 26, 2009 at 13:12:04 Anton Vorontsov wrote:
[...]
> > Ah. I understand what you're doing now. Hmmm. This approach
> > concerns me because it relies on firmware or platform code to get CS
> > gpios set up properly before the driver is probed.
>
> Yes, that was said at the very beginning of this thread.
>
> > Firmware doesn't
> > always get it right, and I prefer to avoid platform specific setups as
> > much as possible. Why can't the CS polarity be encoded into the
> > device tree so the driver *does* have the polarity data at probe time?
>
> We have the spi-cs-high property, but it duplicates compatible
> property. 'compatible' is enough to tell whether some device has
> cs-high or cs-low (device's driver knows that already).
>
> The problem is that SPI bus master doesn't know all the devices,
> so it can't extract that information from the compatible string.
> To workaround that we can use 'spi-cs-high', but that's ugly
> workaround.
>
> SPI modes (0,1,2,3) is another matter, some devices can work in
> several modes, so 'spi-cpol' and 'spi-cpha' are actually useful.
>
To get a sane initial state the needed GPIOs can be set to be inputs during
the driver's initialization.
This requires pull-up/pull-down resistors connected to the chip select
lines. I think we can assume that they exist, because the GPIOs are all inputs
after the controller's hardware reset and the resistors are needed to have a
well-defined voltage level on the chip select lines. Normally the level is set
so that the devices are disabled.
Therefore, it doesn't matter if the firmware sets the GPIOs wrong.
^ permalink raw reply
* Re: [RFC PATCH 16/19] powerpc: wii: hollywood interrupt controller support
From: Albert Herranz @ 2009-11-26 15:42 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1259213022.16367.273.camel@pasglop>
Benjamin Herrenschmidt wrote:
> On Sun, 2009-11-22 at 23:01 +0100, Albert Herranz wrote:
>
>> +static void hlwd_pic_mask_and_ack(unsigned int virq)
>> +{
>> + int irq = virq_to_hw(virq);
>> + void __iomem *io_base = get_irq_chip_data(virq);
>> +
>> + clear_bit(irq, io_base + HW_BROADWAY_IMR);
>> + set_bit(irq, io_base + HW_BROADWAY_ICR);
>> +}
>
> Same comment as with flipper. BTW. It looks really similar, can't you
> use the same driver ?
>
I'll look at it. Surely, parts of it would be mergeable.
> Cheers,
> Ben.
>
Thanks,
Albert
>
>
^ permalink raw reply
* Re: [RFC PATCH 14/19] powerpc: allow ioremap within reserved fake ram regions
From: Albert Herranz @ 2009-11-26 15:35 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1259212939.16367.272.camel@pasglop>
Benjamin Herrenschmidt wrote:
> On Tue, 2009-11-24 at 18:09 +0100, Albert Herranz wrote:
>> I could use ppc_md.ioremap to duplicate ioremap except for the ioremap
>> ram check.
>> But calling the stock ioremap without modifying it is not possible
>> because it checks and bails out when ioremapping a region marked as
>> ram (even if it's not real ram and it's memreserved).
>>
>> Is the list of memreserved areas preserved once the kernel is running?
>> If it is preserved another option would be to unban ioremapping ram if
>> memreserved.
>
> Yes, check lmb's they should still be around.
>
Good.
So adding a kconfig option to allow ioremapping memreserved memory marked as ram and adding a proper check (under that kconfig) to unban this case in ioremap would be an acceptable solution?
> Cheers,
> Ben.
>
Thanks,
Albert
^ permalink raw reply
* Re: [RFC PATCH 11/19] powerpc: gamecube/wii: flipper interrupt controller support
From: Albert Herranz @ 2009-11-26 15:33 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1259212736.16367.271.camel@pasglop>
Benjamin Herrenschmidt wrote:
> On Sun, 2009-11-22 at 16:28 -0700, Grant Likely wrote:
>>> +unsigned int flipper_pic_get_irq(void)
>>> +{
>>> + void __iomem *io_base = flipper_irq_host->host_data;
>>> + int irq;
>>> + u32 irq_status;
>>> +
>>> + irq_status = in_be32(io_base + FLIPPER_ICR) &
>>> + in_be32(io_base + FLIPPER_IMR);
>>> + if (irq_status == 0)
>>> + return -1; /* no more IRQs pending */
>> NO_IRQ_IGNORE
>
> Why no just 0 ? (aka NO_IRQ)
>
I didn't know about 0 (I thought that was another valid interrupt).
I was used to -1 to tell that no IRQs were pending (at least from the ARCH=ppc days) :)
> Or do you know you are getting lots of spurrious that you don't want to
> account ?
>
No, this is not the case here.
> Cheers,
> Ben.
>
Thanks,
Albert
^ permalink raw reply
* Re: [RFC PATCH 11/19] powerpc: gamecube/wii: flipper interrupt controller support
From: Albert Herranz @ 2009-11-26 15:30 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1259212694.16367.270.camel@pasglop>
Benjamin Herrenschmidt wrote:
> On Sun, 2009-11-22 at 23:01 +0100, Albert Herranz wrote:
>
>> +static void flipper_pic_mask_and_ack(unsigned int virq)
>> +{
>> + int irq = virq_to_hw(virq);
>> + void __iomem *io_base = get_irq_chip_data(virq);
>> +
>> + clear_bit(irq, io_base + FLIPPER_IMR);
>> + set_bit(irq, io_base + FLIPPER_ICR);
>> +}
>
> Do not use clear_bit and set_bit on IOs. They will do lwarx/stwcx. which
> is really not what you want. You can use __clear_bit and __set_bit but
> it's still fishy. Those operate on unsigned long, so the size vary
> between 32 and 64 bit etc... not something you care that much about, but
> it's still the wrong tool for the job.
>
> Do those guys have more than 32 interrupts ? If not, just hand
> code the msak & shifts. If they do, then maybe stick with __clear_bit()
> and __set_bit() which are the non atomic variants.
>
There can be only 32 interrupt sources per pic.
I'll build a mask and check if just a simple write works too (it should IMHO), instead of a RWM.
> Cheers,
> Ben.
>
Thanks,
Albert
^ permalink raw reply
* Re: [RFC PATCH 09/19] powerpc: gamecube/wii: udbg support for usbgecko
From: Albert Herranz @ 2009-11-26 15:28 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1259212354.16367.265.camel@pasglop>
Benjamin Herrenschmidt wrote:
> On Sun, 2009-11-22 at 23:01 +0100, Albert Herranz wrote:
>> Add support for using the USB Gecko adapter via the udbg facility on
>> the Nintendo GameCube and Wii video game consoles.
>> The USB Gecko is a 3rd party memory card interface adapter that provides
>> a EXI (External Interface) to USB serial converter.
>
> The main issue here is that the gecko is supposed to be something you
> can plug or unplug no ? Is it hotswap or only at boot ? In the later
> case, maybe the node should be created by the boot wrapper when
> detecting the presence of the device. If it's hotplug, then we need
> something smarter...
>
The usbgecko is hotplugable and hotswappable.
But as this is mostly a developer feature, not normaly used by end users, I think that we can just let it be as it is: autodetect it on boot (now probing for it instead of using information from the device tree).
If you unplug it later it causes no errors, you just miss whatever data is sent to it.
> Cheers,
> Ben.
>
Thanks,
Albert
^ permalink raw reply
* [PATCH] PPC: Sync guest visible MMU state
From: Alexander Graf @ 2009-11-26 15:24 UTC (permalink / raw)
To: kvm; +Cc: kvm-ppc, linuxppc-dev
Currently userspace has no chance to find out which virtual address space we're
in and resolve addresses. While that is a big problem for migration, it's also
unpleasent when debugging, as gdb and the monitor don't work on virtual
addresses.
This patch exports enough of the MMU segment state to userspace to make
debugging work and thus also includes the groundwork for migration.
Signed-off-by: Alexander Graf <agraf@suse.de>
---
Ben, please take this patch in your tree.
v2 -> v3:
- don't use anonymous unions/structs
---
arch/powerpc/include/asm/kvm.h | 20 +++++++++++-
arch/powerpc/include/asm/kvm_asm.h | 1 +
arch/powerpc/include/asm/kvm_book3s.h | 3 ++
arch/powerpc/kvm/book3s.c | 53 +++++++++++++++++++++++++++++++-
arch/powerpc/kvm/book3s_64_emulate.c | 38 ++++++++++++++---------
arch/powerpc/kvm/book3s_64_mmu.c | 2 +
arch/powerpc/kvm/powerpc.c | 3 ++
include/linux/kvm.h | 3 ++
8 files changed, 104 insertions(+), 19 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm.h b/arch/powerpc/include/asm/kvm.h
index c9ca97f..cb6ad08 100644
--- a/arch/powerpc/include/asm/kvm.h
+++ b/arch/powerpc/include/asm/kvm.h
@@ -46,8 +46,24 @@ struct kvm_regs {
};
struct kvm_sregs {
- __u32 pvr;
- char pad[1020];
+ union {
+ struct {
+ __u32 pvr;
+ __u64 sdr1;
+ struct {
+ struct {
+ __u64 slbe;
+ __u64 slbv;
+ } slb[64];
+ } ppc64;
+ struct {
+ __u32 sr[16];
+ __u64 ibat[8];
+ __u64 dbat[8];
+ } ppc32;
+ } s;
+ __u8 pad[1024];
+ } u;
};
struct kvm_fpu {
diff --git a/arch/powerpc/include/asm/kvm_asm.h b/arch/powerpc/include/asm/kvm_asm.h
index 19ddb35..af2abe7 100644
--- a/arch/powerpc/include/asm/kvm_asm.h
+++ b/arch/powerpc/include/asm/kvm_asm.h
@@ -87,6 +87,7 @@
#define BOOK3S_IRQPRIO_MAX 16
#define BOOK3S_HFLAG_DCBZ32 0x1
+#define BOOK3S_HFLAG_SLB 0x2
#define RESUME_FLAG_NV (1<<0) /* Reload guest nonvolatile state? */
#define RESUME_FLAG_HOST (1<<1) /* Resume host? */
diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
index c601133..74b7369 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -46,6 +46,7 @@ struct kvmppc_sr {
};
struct kvmppc_bat {
+ u64 raw;
u32 bepi;
u32 bepi_mask;
bool vs;
@@ -113,6 +114,8 @@ extern struct kvmppc_pte *kvmppc_mmu_find_pte(struct kvm_vcpu *vcpu, u64 ea, boo
extern int kvmppc_ld(struct kvm_vcpu *vcpu, ulong eaddr, int size, void *ptr, bool data);
extern int kvmppc_st(struct kvm_vcpu *vcpu, ulong eaddr, int size, void *ptr);
extern void kvmppc_book3s_queue_irqprio(struct kvm_vcpu *vcpu, unsigned int vec);
+extern void kvmppc_set_bat(struct kvm_vcpu *vcpu, struct kvmppc_bat *bat,
+ bool upper, u32 val);
extern u32 kvmppc_trampoline_lowmem;
extern u32 kvmppc_trampoline_enter;
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index 42037d4..6666583 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -281,6 +281,7 @@ void kvmppc_core_deliver_interrupts(struct kvm_vcpu *vcpu)
void kvmppc_set_pvr(struct kvm_vcpu *vcpu, u32 pvr)
{
+ vcpu->arch.hflags &= ~BOOK3S_HFLAG_SLB;
vcpu->arch.pvr = pvr;
if ((pvr >= 0x330000) && (pvr < 0x70330000)) {
kvmppc_mmu_book3s_64_init(vcpu);
@@ -762,14 +763,62 @@ int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
int kvm_arch_vcpu_ioctl_get_sregs(struct kvm_vcpu *vcpu,
struct kvm_sregs *sregs)
{
- sregs->pvr = vcpu->arch.pvr;
+ struct kvmppc_vcpu_book3s *vcpu3s = to_book3s(vcpu);
+ int i;
+
+ sregs->u.s.pvr = vcpu->arch.pvr;
+
+ sregs->u.s.sdr1 = to_book3s(vcpu)->sdr1;
+ if (vcpu->arch.hflags & BOOK3S_HFLAG_SLB) {
+ for (i = 0; i < 64; i++) {
+ sregs->u.s.ppc64.slb[i].slbe = vcpu3s->slb[i].orige | i;
+ sregs->u.s.ppc64.slb[i].slbv = vcpu3s->slb[i].origv;
+ }
+ } else {
+ for (i = 0; i < 16; i++) {
+ sregs->u.s.ppc32.sr[i] = vcpu3s->sr[i].raw;
+ sregs->u.s.ppc32.sr[i] = vcpu3s->sr[i].raw;
+ }
+ for (i = 0; i < 8; i++) {
+ sregs->u.s.ppc32.ibat[i] = vcpu3s->ibat[i].raw;
+ sregs->u.s.ppc32.dbat[i] = vcpu3s->dbat[i].raw;
+ }
+ }
return 0;
}
int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
struct kvm_sregs *sregs)
{
- kvmppc_set_pvr(vcpu, sregs->pvr);
+ struct kvmppc_vcpu_book3s *vcpu3s = to_book3s(vcpu);
+ int i;
+
+ kvmppc_set_pvr(vcpu, sregs->u.s.pvr);
+
+ vcpu3s->sdr1 = sregs->u.s.sdr1;
+ if (vcpu->arch.hflags & BOOK3S_HFLAG_SLB) {
+ for (i = 0; i < 64; i++) {
+ vcpu->arch.mmu.slbmte(vcpu, sregs->u.s.ppc64.slb[i].slbv,
+ sregs->u.s.ppc64.slb[i].slbe);
+ }
+ } else {
+ for (i = 0; i < 16; i++) {
+ vcpu->arch.mmu.mtsrin(vcpu, i, sregs->u.s.ppc32.sr[i]);
+ }
+ for (i = 0; i < 8; i++) {
+ kvmppc_set_bat(vcpu, &(vcpu3s->ibat[i]), false,
+ (u32)sregs->u.s.ppc32.ibat[i]);
+ kvmppc_set_bat(vcpu, &(vcpu3s->ibat[i]), true,
+ (u32)(sregs->u.s.ppc32.ibat[i] >> 32));
+ kvmppc_set_bat(vcpu, &(vcpu3s->dbat[i]), false,
+ (u32)sregs->u.s.ppc32.dbat[i]);
+ kvmppc_set_bat(vcpu, &(vcpu3s->dbat[i]), true,
+ (u32)(sregs->u.s.ppc32.dbat[i] >> 32));
+ }
+ }
+
+ /* Flush the MMU after messing with the segments */
+ kvmppc_mmu_pte_flush(vcpu, 0, 0);
return 0;
}
diff --git a/arch/powerpc/kvm/book3s_64_emulate.c b/arch/powerpc/kvm/book3s_64_emulate.c
index c343e67..1027eac 100644
--- a/arch/powerpc/kvm/book3s_64_emulate.c
+++ b/arch/powerpc/kvm/book3s_64_emulate.c
@@ -185,7 +185,27 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
return emulated;
}
-static void kvmppc_write_bat(struct kvm_vcpu *vcpu, int sprn, u64 val)
+void kvmppc_set_bat(struct kvm_vcpu *vcpu, struct kvmppc_bat *bat, bool upper,
+ u32 val)
+{
+ if (upper) {
+ /* Upper BAT */
+ u32 bl = (val >> 2) & 0x7ff;
+ bat->bepi_mask = (~bl << 17);
+ bat->bepi = val & 0xfffe0000;
+ bat->vs = (val & 2) ? 1 : 0;
+ bat->vp = (val & 1) ? 1 : 0;
+ bat->raw = (bat->raw & 0xffffffff00000000ULL) | val;
+ } else {
+ /* Lower BAT */
+ bat->brpn = val & 0xfffe0000;
+ bat->wimg = (val >> 3) & 0xf;
+ bat->pp = val & 3;
+ bat->raw = (bat->raw & 0x00000000ffffffffULL) | ((u64)val << 32);
+ }
+}
+
+static void kvmppc_write_bat(struct kvm_vcpu *vcpu, int sprn, u32 val)
{
struct kvmppc_vcpu_book3s *vcpu_book3s = to_book3s(vcpu);
struct kvmppc_bat *bat;
@@ -207,19 +227,7 @@ static void kvmppc_write_bat(struct kvm_vcpu *vcpu, int sprn, u64 val)
BUG();
}
- if (!(sprn % 2)) {
- /* Upper BAT */
- u32 bl = (val >> 2) & 0x7ff;
- bat->bepi_mask = (~bl << 17);
- bat->bepi = val & 0xfffe0000;
- bat->vs = (val & 2) ? 1 : 0;
- bat->vp = (val & 1) ? 1 : 0;
- } else {
- /* Lower BAT */
- bat->brpn = val & 0xfffe0000;
- bat->wimg = (val >> 3) & 0xf;
- bat->pp = val & 3;
- }
+ kvmppc_set_bat(vcpu, bat, !(sprn % 2), val);
}
int kvmppc_core_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, int rs)
@@ -243,7 +251,7 @@ int kvmppc_core_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, int rs)
case SPRN_IBAT4U ... SPRN_IBAT7L:
case SPRN_DBAT0U ... SPRN_DBAT3L:
case SPRN_DBAT4U ... SPRN_DBAT7L:
- kvmppc_write_bat(vcpu, sprn, vcpu->arch.gpr[rs]);
+ kvmppc_write_bat(vcpu, sprn, (u32)vcpu->arch.gpr[rs]);
/* BAT writes happen so rarely that we're ok to flush
* everything here */
kvmppc_mmu_pte_flush(vcpu, 0, 0);
diff --git a/arch/powerpc/kvm/book3s_64_mmu.c b/arch/powerpc/kvm/book3s_64_mmu.c
index a31f9c6..5598f88 100644
--- a/arch/powerpc/kvm/book3s_64_mmu.c
+++ b/arch/powerpc/kvm/book3s_64_mmu.c
@@ -473,4 +473,6 @@ void kvmppc_mmu_book3s_64_init(struct kvm_vcpu *vcpu)
mmu->esid_to_vsid = kvmppc_mmu_book3s_64_esid_to_vsid;
mmu->ea_to_vp = kvmppc_mmu_book3s_64_ea_to_vp;
mmu->is_dcbz32 = kvmppc_mmu_book3s_64_is_dcbz32;
+
+ vcpu->arch.hflags |= BOOK3S_HFLAG_SLB;
}
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 692c370..d82551e 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -144,6 +144,9 @@ int kvm_dev_ioctl_check_extension(long ext)
int r;
switch (ext) {
+ case KVM_CAP_PPC_SEGSTATE:
+ r = 1;
+ break;
case KVM_CAP_COALESCED_MMIO:
r = KVM_COALESCED_MMIO_PAGE_OFFSET;
break;
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index f8f8900..caf6173 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -436,6 +436,9 @@ struct kvm_ioeventfd {
#endif
#define KVM_CAP_IOEVENTFD 36
#define KVM_CAP_SET_IDENTITY_MAP_ADDR 37
+/* KVM upstream has more features, but we synched this number.
+ Linux, please remove this comment on rebase. */
+#define KVM_CAP_PPC_SEGSTATE 43
#ifdef KVM_CAP_IRQ_ROUTING
--
1.6.0.2
^ permalink raw reply related
* Re: [RFC PATCH 06/19] powerpc: gamecube/wii: introduce GAMECUBE_COMMON
From: Albert Herranz @ 2009-11-26 15:22 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1259212129.16367.262.camel@pasglop>
Benjamin Herrenschmidt wrote:
> On Tue, 2009-11-24 at 19:03 +0100, Albert Herranz wrote:
>> Segher Boessenkool wrote:
>>>> Add a config option GAMECUBE_COMMON to be used as a dependency for all
>>>> options common to the Nintendo GameCube and Wii video game consoles.
>>> Maybe something like GAMECUBE_OR_WII instead? "COMMON" is so
>>> common it's meaningless.
>>>
>> I don't like either GAMECUBE_OR_WII.
>> It looks as if it can be used to match options for the GAMECUBE _or_ WII. But that's not the meaning of it.
>>
>> This option should be used only for options applicable to both the GAMECUBE and WII, i.e. basically those options in the WII which where "inherited" from the GAMECUBE to make it retro-compatible.
>>
>> If GAMECUBE_COMMON is unacceptable, maybe GAMECUBE_LEGACY or GAMECUBE_COMPAT?
>
> I don't have any strong opinion. Maybe NINTENDO_COMMON ?
>
We should avoid using NINTENDO just to refer to the Nintendo GameCube or Wii. They have other consoles too :)
There's even a port of Linux to the Nintendo DS...
I'd stick to GAMECUBE_COMMON or GAMECUBE_COMPAT.
> Cheers,
> Ben.
>
Thanks,
Albert
^ 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