* Re: [PATCH 3/5] powerpc/dts: update MSI bindings doc for MPIC v4.3
From: Scott Wood @ 2013-06-18 0:28 UTC (permalink / raw)
To: Lian Minghuan-b31939; +Cc: Minghuan Lian, linuxppc-dev, Zang Roy-R61911
In-Reply-To: <51BE999D.2080207@freescale.com>
On 06/17/2013 12:07:41 AM, Lian Minghuan-b31939 wrote:
> Hi Soctt,
>=20
> please see my comments.
>=20
> On 06/15/2013 06:06 AM, Scott Wood wrote:
>> On 06/14/2013 02:15:57 AM, Minghuan Lian wrote:
>>> Add compatible "fsl,mpic-msi-v4.3" for MPIC v4.3. MPIC v4.3 contains
>>> MSIIR and MSIIR1. MSIIR supports 8 MSI registers and MSIIR1 supports
>>> 16 MSI registers, but uses different IBS and SRS shift. When using
>>> MSIR1, the interrupt number is not consecutive. It is hard to use
>>> 'msi-available-ranges' to describe the ranges of the available
>>> interrupt and the ranges are related to the application, rather than
>>> the description of the hardware. this patch also removes
>>> 'msi-available-ranges' property.
>>>=20
>>> Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
>>> ---
>>> .../devicetree/bindings/powerpc/fsl/msi-pic.txt | 49 =20
>>> ++++++++++------------
>>> 1 file changed, 22 insertions(+), 27 deletions(-)
>>>=20
>>> diff --git =20
>>> a/Documentation/devicetree/bindings/powerpc/fsl/msi-pic.txt =20
>>> b/Documentation/devicetree/bindings/powerpc/fsl/msi-pic.txt
>>> index 5693877..e851e93 100644
>>> --- a/Documentation/devicetree/bindings/powerpc/fsl/msi-pic.txt
>>> +++ b/Documentation/devicetree/bindings/powerpc/fsl/msi-pic.txt
>>> @@ -1,26 +1,23 @@
>>> * Freescale MSI interrupt controller
>>>=20
>>> Required properties:
>>> -- compatible : compatible list, contains 2 entries,
>>> +- compatible : compatible list, may contains one or two entries,
>>> first is "fsl,CHIP-msi", where CHIP is the processor(mpc8610, =20
>>> mpc8572,
>>> - etc.) and the second is "fsl,mpic-msi" or "fsl,ipic-msi" =20
>>> depending on
>>> - the parent type.
>>> + etc.) and the second is "fsl,mpic-msi" or "fsl,ipic-msi" or
>>> + "fsl,mpic-msi-v4.3" depending on the parent type and version. If =20
>>> mpic
>>> + version is 4.3, the number of MSI registers is increased to 16, =20
>>> MSIIR1 is
>>> + provided to access these 16 registers, compatible =20
>>> "fsl,mpic-msi-v4.3"
>>> + should be used.
>>=20
>> Why "one or two"? What does it look like in the case where there's =20
>> just one?
>>=20
> [Minghuan] The original doc said 'contains 2 entries', but I notcie =20
> pq3-mpic.dtsi and qoriq-mpic.dtsi have only one entry "fsl,mpic-msi", =20
> do not have "fsl,CHIP-msi".
> for example:
> mpc8610_hpcd.dts: compatible =3D "fsl,mpc8610-msi", "fsl,mpic-msi";
> fsl/qoriq-mpic.dtsi: compatible =3D "fsl,mpic-msi"
>=20
> Maybe I should say " For some platforms, "fsl,CHIP-msi' is optional."
Well, this is more a matter of some device trees not complying with the =20
binding, rather than an update for MPIC v4.3.
In any case, if the plan is to update the binding to match what we've =20
been doing in the actual trees, at least word it so that it's clear =20
which one of the two is optional.
>> Why are you removing msi-available-ranges? It's not valid for MPIC =20
>> v4.3, but it's still valid for older MPICs. It should move to the =20
>> optional section, though.
> [Minghuan] Because I would like to add kernel parameter 'msiregs' =20
> instead of "msi-available-ranges", for all the MPICs, we will have a =20
> uniform way to configure
I've responded elsewhere to this, but I'd also like to add that we =20
don't break compatibility with older device tree bindings just for "a =20
uniform way".
>>> Example:
>>> msi@41600 {
>>> - compatible =3D "fsl,mpc8610-msi", "fsl,mpic-msi";
>>> - reg =3D <0x41600 0x80>;
>>> - msi-available-ranges =3D <0 0x100>;
>>> - interrupts =3D <
>>> - 0xe0 0
>>> - 0xe1 0
>>> - 0xe2 0
>>> - 0xe3 0
>>> - 0xe4 0
>>> - 0xe5 0
>>> - 0xe6 0
>>> - 0xe7 0>;
>>> - interrupt-parent =3D <&mpic>;
>>> - };
>>> + compatible =3D "fsl,mpic-msi";
>>> + reg =3D <0x41600 0x200 0x44140 4>;
>>=20
>> Why 0x200?
>>=20
> [Minghuan] The offsets of the MSIA registers are from 0x41600 to =20
> 0x417ff, and the size is 0x200.
> offset 0x41600-0x4170 are MSIIRA1-7.
> 0x41720 is MSISRA,
> 0x41750 is MSIIR.
> The others are reserved.
There is no MSIIRA on fsl,mpic-msi.
If you want to show an fsl,mpic-msi-v4.3 example, update the compatible =20
and add the extra 8 interrupts. We should probably show an example of =20
each.
BTW, why are you changing/breaking the whitespace in the example?
-Scott=
^ permalink raw reply
* Re: [PATCH 1/2] powerpc: add Book E support to 64-bit hibernation
From: Scott Wood @ 2013-06-18 0:22 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Wood Scott-B07421, anton@enomsg.org, Wang Dongsheng-B40534,
johannes@sipsolutions.net, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1371514650.21896.160.camel@pasglop>
On 06/17/2013 07:17:30 PM, Benjamin Herrenschmidt wrote:
> On Mon, 2013-06-17 at 19:01 -0500, Scott Wood wrote:
> > I really doubt the exception scratch registers need to be saved --
> > we're not trying to restore into the middle of an exception
> > prolog/epilog.
> >
> > book3s has the PACA as well and they don't save it. Don't we rely =20
> on
> > things like boot-time memory allocations happening in the same place
> > when we resume? extlb is part of the PACA, so the same applies.
>=20
> I doubt we seriously tested hibernation :-) The PACA SPR should
> definitely be saved/restored.
OK. It's not obvious to me how much the entire mechanism depends on =20
things like boot time allocations being the same each time -- if we do =20
depend on that in general, then the PACA shouldn't change on a =20
particular CPU, right?
Is it possible to restore on a different CPU than we saved on? If so, =20
could restoring the PACA leave us pointing to a different CPU's PACA?
-Scott=
^ permalink raw reply
* Re: [PATCH 1/5] powerpc/dts: add MPIC v4.3 dts node
From: Scott Wood @ 2013-06-18 0:18 UTC (permalink / raw)
To: Lian Minghuan-b31939; +Cc: Minghuan Lian, linuxppc-dev, Zang Roy-R61911
In-Reply-To: <51BE7329.8070304@freescale.com>
On 06/16/2013 09:23:37 PM, Lian Minghuan-b31939 wrote:
> Hi Scott,
>=20
> please see my comments.
>=20
> On 06/15/2013 05:53 AM, Scott Wood wrote:
>> On 06/14/2013 03:39:26 PM, Scott Wood wrote:
>>> On 06/14/2013 02:15:55 AM, Minghuan Lian wrote:
>>>> +msi0: msi@41600 {
>>>> + compatible =3D "fsl,mpic-msi", "fsl,mpic-msi-v4.3";
>>>=20
>>> More specific compatibles come first -- and I don't think this is =20
>>> 100% backwards compatible with "fsl,mpic-msi" anyway.
>>=20
>> Also please update the binding.
> [Minghuan] Yes, maybe I should remove "fsl,mpic-msi". What do you =20
> think?
Yes.
-Scott=
^ permalink raw reply
* Re: [PATCH 5/5] powerpc/fsl_msi: add 'msiregs' kernel parameter
From: Scott Wood @ 2013-06-18 0:18 UTC (permalink / raw)
To: Lian Minghuan-b31939; +Cc: Minghuan Lian, linuxppc-dev, Zang Roy-R61911
In-Reply-To: <51BEA072.1040902@freescale.com>
On 06/17/2013 12:36:50 AM, Lian Minghuan-b31939 wrote:
> Hi Scott,
>=20
> please see my comments inline.
>=20
> On 06/15/2013 06:13 AM, Scott Wood wrote:
>> On 06/14/2013 02:15:59 AM, Minghuan Lian wrote:
>>> 1. Only MSIIR1 can index 16 MSI registers, but when using MSIIR1
>>> the IRQs of a register are not continuous. for example, the first
>>> register irq values are 0x0, 0x10, 0x20, 0x30 ... 0x1f0. So it
>>> is hard to use 'msi-available-ranges' property to indicate the
>>> available ranges and 'msi-available-ranges' property has been
>>> removed from dts node, so this patch removes the related code.
>>>=20
>>> 2. Add 'msiregs' kernel parameter instead of 'msi-available-ranges'
>>> functionality.
>>=20
>> The reason we used a device tree property was because this is for =20
>> virtualization and AMP scenarios where this instance of Linux does =20
>> not own all of the MSI registers.
>>=20
>> I don't see any reasonable way to partition an MPIC v4.3 MSI group =20
>> -- but there are more groups, so it's not that bad. What's the use =20
>> case for this patch?
>>=20
> [Minghuan] I do not known any case about this patch. I add 'msiregs' =20
> just for achieving "msi-available-ranges" functionality. I do not =20
> want to remove partition functionality when updating to mpic4.3, =20
> although I do not see virtualization and AMP cases on T4(KVM does not =20
> need this functionality).
Such functionality does not work on mpic v4.3. There are conflicting =20
requirements of contiguous MSIs (because PCI devices can use them that =20
way) and the inability to partition a single register (because they all =20
go to the same MPIC interrupt).
Keep msi-available-ranges as is for older hardware, and just ignore it =20
(with a warning printed) if it's present on MPIC v4.3.
-Scott=
^ permalink raw reply
* Re: [PATCH 1/2] powerpc: add Book E support to 64-bit hibernation
From: Benjamin Herrenschmidt @ 2013-06-18 0:17 UTC (permalink / raw)
To: Scott Wood
Cc: Wood Scott-B07421, anton@enomsg.org, Wang Dongsheng-B40534,
johannes@sipsolutions.net, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1371513683.9073.6@snotra>
On Mon, 2013-06-17 at 19:01 -0500, Scott Wood wrote:
> I really doubt the exception scratch registers need to be saved --
> we're not trying to restore into the middle of an exception
> prolog/epilog.
>
> book3s has the PACA as well and they don't save it. Don't we rely on
> things like boot-time memory allocations happening in the same place
> when we resume? extlb is part of the PACA, so the same applies.
I doubt we seriously tested hibernation :-) The PACA SPR should
definitely be saved/restored.
> Granted, this isn't performance critical so it may seem better to
> save/restore just in case, but there's value in not unnecessarily
> deviating from what book3s does.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 2/5] powerpc/fsl_msi: add MSIIR1 support for MPIC v4.3
From: Scott Wood @ 2013-06-18 0:15 UTC (permalink / raw)
To: Lian Minghuan-b31939; +Cc: Minghuan Lian, linuxppc-dev, Zang Roy-R61911
In-Reply-To: <51BE7BB1.1070309@freescale.com>
On 06/16/2013 10:00:01 PM, Lian Minghuan-b31939 wrote:
> Hi Scott,
>=20
> please see my comments inline.
>=20
> On 06/15/2013 06:09 AM, Scott Wood wrote:
>> On 06/14/2013 02:15:56 AM, Minghuan Lian wrote:
>>> diff --git a/arch/powerpc/sysdev/fsl_msi.h =20
>>> b/arch/powerpc/sysdev/fsl_msi.h
>>> index 8225f86..43a9d99 100644
>>> --- a/arch/powerpc/sysdev/fsl_msi.h
>>> +++ b/arch/powerpc/sysdev/fsl_msi.h
>>> @@ -16,7 +16,7 @@
>>> #include <linux/of.h>
>>> #include <asm/msi_bitmap.h>
>>>=20
>>> -#define NR_MSI_REG 8
>>> +#define NR_MSI_REG 16
>>> #define IRQS_PER_MSI_REG 32
>>> #define NR_MSI_IRQS (NR_MSI_REG * IRQS_PER_MSI_REG)
>>=20
>> I don't see where you update all_avail in fsl_of_msi_probe.
>>=20
>> We should also be bounds-checking the contents of =20
>> msi-available-ranges.
>> Currently it looks like we just silently overrun the bitmap if we =20
>> get bad
>> input from the device tree.
>>=20
> [Minghuan] all_avail definition: static const u32 all_avail[] =3D { 0, =20
> NR_MSI_IRQS };
> When changing NR_MSI_REG to 16, NR_MSI_IRQS has been changed to =20
> 16*32, and all_avail also is updated.
That's my point. It shouldn't change for older hardware.
> Before calling fsl_msi_setup_hwirq(), the code has checked =20
> 'msi-available-ranges', only the interrupts lied in =20
> 'msi-available-ranges' will be initialized by call =20
> fsl_msi_setup_hwirq() , and the corresponding bitmap will be freed. I =20
> moved msi_bitmap_free_hwirqs() to fsl_msi_setup_hwirq(), because the =20
> code would generate different bitmap when using MSIIR or MSIIR1.
And what happens if msi-available-ranges is bad, and refers to =20
non-existent MSIs past the end of the bitmap?
-Scott=
^ permalink raw reply
* Re: [PATCH 4/5] powerpc/dts: remove msi-available-ranges property
From: Scott Wood @ 2013-06-18 0:13 UTC (permalink / raw)
To: Lian Minghuan-b31939; +Cc: Minghuan Lian, linuxppc-dev, Zang Roy-R61911
In-Reply-To: <51BE9B78.9080302@freescale.com>
On 06/17/2013 12:15:36 AM, Lian Minghuan-b31939 wrote:
> On 06/15/2013 06:10 AM, Scott Wood wrote:
>> On 06/14/2013 02:15:58 AM, Minghuan Lian wrote:
>>> For MPIC v4.3 MSIIR supports 8 MSI registers and MSIIR1 supports
>>> 16 MSI registers, but uses different IBS and SRS shift. For the
>>> first register, when using MSIIR we will get the irqs 0x0 0x1 0x2
>>> ...0x1f, but when using MSIIR1, the irqs are 0x0 0x10 0x20 ... 0x1f0
>>> It is hard to describe the available irqs using property
>>> 'msi-available-ranges'. The patch removes this property.
>>=20
>> Only remove it from mpic 4.3. And since you introduced =20
>> qoriq-mpic4.3.dtsi earlier in the patchset, why didn't you just =20
>> avoid adding it then?
>>=20
> [Minghuan] If adding it in qoriq-mpic4.3, and the 3-5 patches are not =20
> accepted, mpic4.3 can also work.
mpic 4.3 cannot work with msi-available-ranges, at all. The hardware =20
just doesn't work that way.
-Scott=
^ permalink raw reply
* Re: [PATCH v2] powerpc/pci: Fix setup of Freescale PCI / PCIe controllers
From: Scott Wood @ 2013-06-18 0:10 UTC (permalink / raw)
To: Rojhalat Ibrahim; +Cc: linuxppc-dev, linux-kernel, Michael Guntsche
In-Reply-To: <2435509.QyTW5GJC53@pcimr>
On 06/17/2013 08:15:33 AM, Rojhalat Ibrahim wrote:
> On Friday 14 June 2013 15:18:03 Scott Wood wrote:
> > On 83xx:
> > cc1: warnings being treated as errors
> > =20
> /home/scott/fsl/git/linux/upstream/arch/powerpc/sysdev/fsl_pci.c:100:23:
> > error: 'fsl_indirect_pcie_ops' defined but not used
> > make[2]: *** [arch/powerpc/sysdev/fsl_pci.o] Error 1
> > make[2]: *** Waiting for unfinished jobs....
> >
> > I can fix this when applying, but this makes me wonder how you =20
> tested
> > it, given that the whole point is to fix 83xx... Did you fix this =20
> and
> > then accidentally sent a stale version?
> >
> > Also, please be careful that the patch doesn't get line wrapped -- I
> > had to manually unwrap a couple places. Use git send-email if you
> > can't get KMail to cooperate.
> >
> > -Scott
>=20
> Sorry about the mess. I'll send a v3.
> Please note: I don't have an 83xx system. So I can only test if it =20
> compiles,
> which I obviously did not do with the right config.
OK, so it looks like it was Michael who ran into the problem on 83xx. =20
Michael, could you test the v3 patch that Rojhalat posted?
-Scott=
^ permalink raw reply
* Re: [PATCH 1/2] powerpc: add Book E support to 64-bit hibernation
From: Scott Wood @ 2013-06-18 0:01 UTC (permalink / raw)
To: Wang Dongsheng-B40534
Cc: Wood Scott-B07421, anton@enomsg.org, johannes@sipsolutions.net,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <ABB05CD9C9F68C46A5CEDC7F15439259F85C34@039-SN2MPN1-022.039d.mgd.msft.net>
On 06/17/2013 12:54:32 AM, Wang Dongsheng-B40534 wrote:
>=20
>=20
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Friday, June 14, 2013 12:51 AM
> > To: Wang Dongsheng-B40534
> > Cc: Wood Scott-B07421; benh@kernel.crashing.org;
> > johannes@sipsolutions.net; anton@enomsg.org; =20
> galak@kernel.crashing.org;
> > linuxppc-dev@lists.ozlabs.org
> > Subject: Re: [PATCH 1/2] powerpc: add Book E support to 64-bit
> > hibernation
> >
> > On 06/13/2013 04:55:43 AM, Wang Dongsheng-B40534 wrote:
> > > > > +#else
> > > > > + /* Save SPRGs */
> > > > > + RESTORE_SPRG(0)
> > > > > + RESTORE_SPRG(1)
> > > > > + RESTORE_SPRG(2)
> > > > > + RESTORE_SPRG(3)
> > > > > + RESTORE_SPRG(4)
> > > > > + RESTORE_SPRG(5)
> > > > > + RESTORE_SPRG(6)
> > > > > + RESTORE_SPRG(7)
> > > >
> > > > Why do we need this on book3e and not on book3s?
> > > >
> > > Book3e: SPRG1 used save paca, SPRG2 be defined
> > > SPRN_SPRG_TLB_EXFRAME,...
> > > I think those register should be save, even now some SPRG =20
> register not
> > > be use.
> >
> > Are those expected/allowed to change as a result of the restore?
> >
> Those registers are used by software, some allowed to change.
> Exception handling is used in some registers, see exception-64e.h
> These registers can be modified and saved.
I really doubt the exception scratch registers need to be saved -- =20
we're not trying to restore into the middle of an exception =20
prolog/epilog.
book3s has the PACA as well and they don't save it. Don't we rely on =20
things like boot-time memory allocations happening in the same place =20
when we resume? extlb is part of the PACA, so the same applies.
Granted, this isn't performance critical so it may seem better to =20
save/restore just in case, but there's value in not unnecessarily =20
deviating from what book3s does.
-Scott=
^ permalink raw reply
* [PATCH] powerpc: delete __cpuinit usage from all users
From: Paul Gortmaker @ 2013-06-17 20:10 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Josh Boyer, Kumar Gala
Cc: Paul Gortmaker, linuxppc-dev
The __cpuinit type of throwaway sections might have made sense
some time ago when RAM was more constrained, but now the savings
do not offset the cost and complications. For example, the fix in
commit 5e427ec2d0 ("x86: Fix bit corruption at CPU resume time")
is a good example of the nasty type of bugs that can be created
with improper use of the various __init prefixes.
After a discussion on LKML[1] it was decided that cpuinit should go
the way of devinit and be phased out. Once all the users are gone,
we can then finally remove the macros themselves from linux/init.h.
This removes all the powerpc uses of the __cpuinit macros.
[1] https://lkml.org/lkml/2013/5/20/589
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
[This was generated against today's linux-next tree ; I'm assuming all
pending powerpc changes are in there currently.]
arch/powerpc/include/asm/rtas.h | 4 ++--
arch/powerpc/include/asm/vdso.h | 2 +-
arch/powerpc/kernel/cacheinfo.c | 19 +++++++++++--------
arch/powerpc/kernel/rtas.c | 4 ++--
arch/powerpc/kernel/smp.c | 2 +-
arch/powerpc/kernel/sysfs.c | 6 +++---
arch/powerpc/kernel/time.c | 1 -
arch/powerpc/kernel/vdso.c | 2 +-
arch/powerpc/mm/44x_mmu.c | 6 +++---
arch/powerpc/mm/hash_utils_64.c | 2 +-
arch/powerpc/mm/mmu_context_nohash.c | 6 +++---
arch/powerpc/mm/numa.c | 7 +++----
arch/powerpc/mm/tlb_nohash.c | 2 +-
arch/powerpc/perf/core-book3s.c | 4 ++--
arch/powerpc/platforms/44x/currituck.c | 4 ++--
arch/powerpc/platforms/44x/iss4xx.c | 4 ++--
arch/powerpc/platforms/85xx/smp.c | 6 +++---
arch/powerpc/platforms/powermac/smp.c | 2 +-
arch/powerpc/platforms/powernv/smp.c | 2 +-
19 files changed, 43 insertions(+), 42 deletions(-)
diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index 34fd704..c7a8bfc 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -350,8 +350,8 @@ static inline u32 rtas_config_addr(int busno, int devfn, int reg)
(devfn << 8) | (reg & 0xff);
}
-extern void __cpuinit rtas_give_timebase(void);
-extern void __cpuinit rtas_take_timebase(void);
+extern void rtas_give_timebase(void);
+extern void rtas_take_timebase(void);
#ifdef CONFIG_PPC_RTAS
static inline int page_is_rtas_user_buf(unsigned long pfn)
diff --git a/arch/powerpc/include/asm/vdso.h b/arch/powerpc/include/asm/vdso.h
index 50f261b..0d9cecd 100644
--- a/arch/powerpc/include/asm/vdso.h
+++ b/arch/powerpc/include/asm/vdso.h
@@ -22,7 +22,7 @@ extern unsigned long vdso64_rt_sigtramp;
extern unsigned long vdso32_sigtramp;
extern unsigned long vdso32_rt_sigtramp;
-int __cpuinit vdso_getcpu_init(void);
+int vdso_getcpu_init(void);
#else /* __ASSEMBLY__ */
diff --git a/arch/powerpc/kernel/cacheinfo.c b/arch/powerpc/kernel/cacheinfo.c
index 92c6b00..ddaecb1 100644
--- a/arch/powerpc/kernel/cacheinfo.c
+++ b/arch/powerpc/kernel/cacheinfo.c
@@ -131,7 +131,8 @@ static const char *cache_type_string(const struct cache *cache)
return cache_type_info[cache->type].name;
}
-static void __cpuinit cache_init(struct cache *cache, int type, int level, struct device_node *ofnode)
+static void cache_init(struct cache *cache, int type, int level,
+ struct device_node *ofnode)
{
cache->type = type;
cache->level = level;
@@ -385,7 +386,7 @@ static struct cache *__cpuinit cache_lookup_or_instantiate(struct device_node *n
return cache;
}
-static void __cpuinit link_cache_lists(struct cache *smaller, struct cache *bigger)
+static void link_cache_lists(struct cache *smaller, struct cache *bigger)
{
while (smaller->next_local) {
if (smaller->next_local == bigger)
@@ -396,13 +397,13 @@ static void __cpuinit link_cache_lists(struct cache *smaller, struct cache *bigg
smaller->next_local = bigger;
}
-static void __cpuinit do_subsidiary_caches_debugcheck(struct cache *cache)
+static void do_subsidiary_caches_debugcheck(struct cache *cache)
{
WARN_ON_ONCE(cache->level != 1);
WARN_ON_ONCE(strcmp(cache->ofnode->type, "cpu"));
}
-static void __cpuinit do_subsidiary_caches(struct cache *cache)
+static void do_subsidiary_caches(struct cache *cache)
{
struct device_node *subcache_node;
int level = cache->level;
@@ -653,7 +654,7 @@ static struct kobj_type cache_index_type = {
.default_attrs = cache_index_default_attrs,
};
-static void __cpuinit cacheinfo_create_index_opt_attrs(struct cache_index_dir *dir)
+static void cacheinfo_create_index_opt_attrs(struct cache_index_dir *dir)
{
const char *cache_name;
const char *cache_type;
@@ -696,7 +697,8 @@ static void __cpuinit cacheinfo_create_index_opt_attrs(struct cache_index_dir *d
kfree(buf);
}
-static void __cpuinit cacheinfo_create_index_dir(struct cache *cache, int index, struct cache_dir *cache_dir)
+static void cacheinfo_create_index_dir(struct cache *cache, int index,
+ struct cache_dir *cache_dir)
{
struct cache_index_dir *index_dir;
int rc;
@@ -722,7 +724,8 @@ err:
kfree(index_dir);
}
-static void __cpuinit cacheinfo_sysfs_populate(unsigned int cpu_id, struct cache *cache_list)
+static void cacheinfo_sysfs_populate(unsigned int cpu_id,
+ struct cache *cache_list)
{
struct cache_dir *cache_dir;
struct cache *cache;
@@ -740,7 +743,7 @@ static void __cpuinit cacheinfo_sysfs_populate(unsigned int cpu_id, struct cache
}
}
-void __cpuinit cacheinfo_cpu_online(unsigned int cpu_id)
+void cacheinfo_cpu_online(unsigned int cpu_id)
{
struct cache *cache;
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 52add6f..80b5ef4 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -1172,7 +1172,7 @@ int __init early_init_dt_scan_rtas(unsigned long node,
static arch_spinlock_t timebase_lock;
static u64 timebase = 0;
-void __cpuinit rtas_give_timebase(void)
+void rtas_give_timebase(void)
{
unsigned long flags;
@@ -1189,7 +1189,7 @@ void __cpuinit rtas_give_timebase(void)
local_irq_restore(flags);
}
-void __cpuinit rtas_take_timebase(void)
+void rtas_take_timebase(void)
{
while (!timebase)
barrier();
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index ee7ac5e..d6f42dc 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -480,7 +480,7 @@ static void cpu_idle_thread_init(unsigned int cpu, struct task_struct *idle)
secondary_ti = current_set[cpu] = ti;
}
-int __cpuinit __cpu_up(unsigned int cpu, struct task_struct *tidle)
+int __cpu_up(unsigned int cpu, struct task_struct *tidle)
{
int rc, c;
diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
index e68a845..27a90b9 100644
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -341,7 +341,7 @@ static struct device_attribute pa6t_attrs[] = {
#endif /* HAS_PPC_PMC_PA6T */
#endif /* HAS_PPC_PMC_CLASSIC */
-static void __cpuinit register_cpu_online(unsigned int cpu)
+static void register_cpu_online(unsigned int cpu)
{
struct cpu *c = &per_cpu(cpu_devices, cpu);
struct device *s = &c->dev;
@@ -502,7 +502,7 @@ ssize_t arch_cpu_release(const char *buf, size_t count)
#endif /* CONFIG_HOTPLUG_CPU */
-static int __cpuinit sysfs_cpu_notify(struct notifier_block *self,
+static int sysfs_cpu_notify(struct notifier_block *self,
unsigned long action, void *hcpu)
{
unsigned int cpu = (unsigned int)(long)hcpu;
@@ -522,7 +522,7 @@ static int __cpuinit sysfs_cpu_notify(struct notifier_block *self,
return NOTIFY_OK;
}
-static struct notifier_block __cpuinitdata sysfs_cpu_nb = {
+static struct notifier_block sysfs_cpu_nb = {
.notifier_call = sysfs_cpu_notify,
};
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 5fc29ad..65ab9e9 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -631,7 +631,6 @@ static int __init get_freq(char *name, int cells, unsigned long *val)
return found;
}
-/* should become __cpuinit when secondary_cpu_time_init also is */
void start_cpu_decrementer(void)
{
#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
index d4f463a..1d9c926 100644
--- a/arch/powerpc/kernel/vdso.c
+++ b/arch/powerpc/kernel/vdso.c
@@ -711,7 +711,7 @@ static void __init vdso_setup_syscall_map(void)
}
#ifdef CONFIG_PPC64
-int __cpuinit vdso_getcpu_init(void)
+int vdso_getcpu_init(void)
{
unsigned long cpu, node, val;
diff --git a/arch/powerpc/mm/44x_mmu.c b/arch/powerpc/mm/44x_mmu.c
index 2c9441e..82b1ff7 100644
--- a/arch/powerpc/mm/44x_mmu.c
+++ b/arch/powerpc/mm/44x_mmu.c
@@ -41,7 +41,7 @@ int icache_44x_need_flush;
unsigned long tlb_47x_boltmap[1024/8];
-static void __cpuinit ppc44x_update_tlb_hwater(void)
+static void ppc44x_update_tlb_hwater(void)
{
extern unsigned int tlb_44x_patch_hwater_D[];
extern unsigned int tlb_44x_patch_hwater_I[];
@@ -134,7 +134,7 @@ static void __init ppc47x_update_boltmap(void)
/*
* "Pins" a 256MB TLB entry in AS0 for kernel lowmem for 47x type MMU
*/
-static void __cpuinit ppc47x_pin_tlb(unsigned int virt, unsigned int phys)
+static void ppc47x_pin_tlb(unsigned int virt, unsigned int phys)
{
unsigned int rA;
int bolted;
@@ -229,7 +229,7 @@ void setup_initial_memory_limit(phys_addr_t first_memblock_base,
}
#ifdef CONFIG_SMP
-void __cpuinit mmu_init_secondary(int cpu)
+void mmu_init_secondary(int cpu)
{
unsigned long addr;
unsigned long memstart = memstart_addr & ~(PPC_PIN_SIZE - 1);
diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
index e303a6d..4481172 100644
--- a/arch/powerpc/mm/hash_utils_64.c
+++ b/arch/powerpc/mm/hash_utils_64.c
@@ -807,7 +807,7 @@ void __init early_init_mmu(void)
}
#ifdef CONFIG_SMP
-void __cpuinit early_init_mmu_secondary(void)
+void early_init_mmu_secondary(void)
{
/* Initialize hash table for that CPU */
if (!firmware_has_feature(FW_FEATURE_LPAR))
diff --git a/arch/powerpc/mm/mmu_context_nohash.c b/arch/powerpc/mm/mmu_context_nohash.c
index e779642..0f149b9 100644
--- a/arch/powerpc/mm/mmu_context_nohash.c
+++ b/arch/powerpc/mm/mmu_context_nohash.c
@@ -329,8 +329,8 @@ void destroy_context(struct mm_struct *mm)
#ifdef CONFIG_SMP
-static int __cpuinit mmu_context_cpu_notify(struct notifier_block *self,
- unsigned long action, void *hcpu)
+static int mmu_context_cpu_notify(struct notifier_block *self,
+ unsigned long action, void *hcpu)
{
unsigned int cpu = (unsigned int)(long)hcpu;
@@ -363,7 +363,7 @@ static int __cpuinit mmu_context_cpu_notify(struct notifier_block *self,
return NOTIFY_OK;
}
-static struct notifier_block __cpuinitdata mmu_context_cpu_nb = {
+static struct notifier_block mmu_context_cpu_nb = {
.notifier_call = mmu_context_cpu_notify,
};
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 88c0425..c792cd9 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -516,7 +516,7 @@ static int of_drconf_to_nid_single(struct of_drconf_cell *drmem,
* Figure out to which domain a cpu belongs and stick it there.
* Return the id of the domain used.
*/
-static int __cpuinit numa_setup_cpu(unsigned long lcpu)
+static int numa_setup_cpu(unsigned long lcpu)
{
int nid = 0;
struct device_node *cpu = of_get_cpu_node(lcpu, NULL);
@@ -538,8 +538,7 @@ out:
return nid;
}
-static int __cpuinit cpu_numa_callback(struct notifier_block *nfb,
- unsigned long action,
+static int cpu_numa_callback(struct notifier_block *nfb, unsigned long action,
void *hcpu)
{
unsigned long lcpu = (unsigned long)hcpu;
@@ -919,7 +918,7 @@ static void __init *careful_zallocation(int nid, unsigned long size,
return ret;
}
-static struct notifier_block __cpuinitdata ppc64_numa_nb = {
+static struct notifier_block ppc64_numa_nb = {
.notifier_call = cpu_numa_callback,
.priority = 1 /* Must run before sched domains notifier. */
};
diff --git a/arch/powerpc/mm/tlb_nohash.c b/arch/powerpc/mm/tlb_nohash.c
index 6888cad..41cd68d 100644
--- a/arch/powerpc/mm/tlb_nohash.c
+++ b/arch/powerpc/mm/tlb_nohash.c
@@ -648,7 +648,7 @@ void __init early_init_mmu(void)
__early_init_mmu(1);
}
-void __cpuinit early_init_mmu_secondary(void)
+void early_init_mmu_secondary(void)
{
__early_init_mmu(0);
}
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 29c6482..af94a71 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -1786,7 +1786,7 @@ static void power_pmu_setup(int cpu)
cpuhw->mmcr[0] = MMCR0_FC;
}
-static int __cpuinit
+static int
power_pmu_notifier(struct notifier_block *self, unsigned long action, void *hcpu)
{
unsigned int cpu = (long)hcpu;
@@ -1803,7 +1803,7 @@ power_pmu_notifier(struct notifier_block *self, unsigned long action, void *hcpu
return NOTIFY_OK;
}
-int __cpuinit register_power_pmu(struct power_pmu *pmu)
+int register_power_pmu(struct power_pmu *pmu)
{
if (ppmu)
return -EBUSY; /* something's already registered */
diff --git a/arch/powerpc/platforms/44x/currituck.c b/arch/powerpc/platforms/44x/currituck.c
index ecd3890..9cef9d3 100644
--- a/arch/powerpc/platforms/44x/currituck.c
+++ b/arch/powerpc/platforms/44x/currituck.c
@@ -91,12 +91,12 @@ static void __init ppc47x_init_irq(void)
}
#ifdef CONFIG_SMP
-static void __cpuinit smp_ppc47x_setup_cpu(int cpu)
+static void smp_ppc47x_setup_cpu(int cpu)
{
mpic_setup_this_cpu();
}
-static int __cpuinit smp_ppc47x_kick_cpu(int cpu)
+static int smp_ppc47x_kick_cpu(int cpu)
{
struct device_node *cpunode = of_get_cpu_node(cpu, NULL);
const u64 *spin_table_addr_prop;
diff --git a/arch/powerpc/platforms/44x/iss4xx.c b/arch/powerpc/platforms/44x/iss4xx.c
index a28a862..4241bc8 100644
--- a/arch/powerpc/platforms/44x/iss4xx.c
+++ b/arch/powerpc/platforms/44x/iss4xx.c
@@ -81,12 +81,12 @@ static void __init iss4xx_init_irq(void)
}
#ifdef CONFIG_SMP
-static void __cpuinit smp_iss4xx_setup_cpu(int cpu)
+static void smp_iss4xx_setup_cpu(int cpu)
{
mpic_setup_this_cpu();
}
-static int __cpuinit smp_iss4xx_kick_cpu(int cpu)
+static int smp_iss4xx_kick_cpu(int cpu)
{
struct device_node *cpunode = of_get_cpu_node(cpu, NULL);
const u64 *spin_table_addr_prop;
diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c
index 6a17599..5ced4f5 100644
--- a/arch/powerpc/platforms/85xx/smp.c
+++ b/arch/powerpc/platforms/85xx/smp.c
@@ -99,7 +99,7 @@ static void mpc85xx_take_timebase(void)
}
#ifdef CONFIG_HOTPLUG_CPU
-static void __cpuinit smp_85xx_mach_cpu_die(void)
+static void smp_85xx_mach_cpu_die(void)
{
unsigned int cpu = smp_processor_id();
u32 tmp;
@@ -141,7 +141,7 @@ static inline u32 read_spin_table_addr_l(void *spin_table)
return in_be32(&((struct epapr_spin_table *)spin_table)->addr_l);
}
-static int __cpuinit smp_85xx_kick_cpu(int nr)
+static int smp_85xx_kick_cpu(int nr)
{
unsigned long flags;
const u64 *cpu_rel_addr;
@@ -362,7 +362,7 @@ static void mpc85xx_smp_machine_kexec(struct kimage *image)
}
#endif /* CONFIG_KEXEC */
-static void __cpuinit smp_85xx_setup_cpu(int cpu_nr)
+static void smp_85xx_setup_cpu(int cpu_nr)
{
if (smp_85xx_ops.probe == smp_mpic_probe)
mpic_setup_this_cpu();
diff --git a/arch/powerpc/platforms/powermac/smp.c b/arch/powerpc/platforms/powermac/smp.c
index f921067..5cbd4d6 100644
--- a/arch/powerpc/platforms/powermac/smp.c
+++ b/arch/powerpc/platforms/powermac/smp.c
@@ -885,7 +885,7 @@ static int smp_core99_cpu_notify(struct notifier_block *self,
return NOTIFY_OK;
}
-static struct notifier_block __cpuinitdata smp_core99_cpu_nb = {
+static struct notifier_block smp_core99_cpu_nb = {
.notifier_call = smp_core99_cpu_notify,
};
#endif /* CONFIG_HOTPLUG_CPU */
diff --git a/arch/powerpc/platforms/powernv/smp.c b/arch/powerpc/platforms/powernv/smp.c
index 88c9459..c22b2b3 100644
--- a/arch/powerpc/platforms/powernv/smp.c
+++ b/arch/powerpc/platforms/powernv/smp.c
@@ -40,7 +40,7 @@
#define DBG(fmt...)
#endif
-static void __cpuinit pnv_smp_setup_cpu(int cpu)
+static void pnv_smp_setup_cpu(int cpu)
{
if (cpu != boot_cpuid)
xics_setup_cpu();
--
1.8.1.2
^ permalink raw reply related
* Re: Build regressions/improvements in v3.10-rc6
From: Geert Uytterhoeven @ 2013-06-17 19:19 UTC (permalink / raw)
To: Linux Kernel Development; +Cc: Linux/PPC Development
In-Reply-To: <alpine.DEB.2.00.1306172114580.17108@ayla.of.borg>
On Mon, 17 Jun 2013, Geert Uytterhoeven wrote:
> JFYI, when comparing v3.10-rc6 to v3.10-rc5[3], the summaries are:
> - build errors: +16/-36
+ arch/mn10300/include/asm/irqflags.h: error: implicit declaration of function 'raw_smp_processor_id' [-Werror=implicit-function-declaration]: => 71:2
+ include/linux/smp.h: error: implicit declaration of function 'local_irq_restore' [-Werror=implicit-function-declaration]: => 150:2
+ include/linux/smp.h: error: implicit declaration of function 'local_irq_save' [-Werror=implicit-function-declaration]: => 148:2
am33_2.0/asb2303_defconfig and am33_2.0/asb2364_defconfig
Known, fix available
+ arch/powerpc/include/asm/mmu-hash64.h: error: control reaches end of non-void function [-Werror=return-type]: => 180:1
+ arch/powerpc/kvm/book3s_hv.c: error: 'vcpus_to_update[need_vpa_update]' may be used uninitialized in this function [-Werror=uninitialized]: => 1187:22
+ arch/powerpc/platforms/cell/beat_iommu.c: error: 'dma_base' may be used uninitialized in this function [-Werror=uninitialized]: => 69:11
+ arch/powerpc/platforms/cell/beat_iommu.c: error: 'dma_size' may be used uninitialized in this function [-Werror=uninitialized]: => 68:2
+ arch/powerpc/platforms/cell/beat_iommu.c: error: 'io_page_size' may be used uninitialized in this function [-Werror=uninitialized]: => 68:54
+ arch/powerpc/platforms/cell/beat_wrapper.h: error: 'io_space_id' may be used uninitialized in this function [-Werror=uninitialized]: => 249:2
+ arch/powerpc/platforms/cell/beat_wrapper.h: error: 'ioid' may be used uninitialized in this function [-Werror=uninitialized]: => 249:2
powerpc-randconfig
We need more randconfig builds to divert attention from powerpc ;-)
> [1] http://kisskb.ellerman.id.au/kisskb/head/6325/ (86 out of 120 configs)
> [3] http://kisskb.ellerman.id.au/kisskb/head/6308/ (all 120 configs)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 4/4] KVM: PPC: Add hugepage support for IOMMU in-kernel handling
From: Paolo Bonzini @ 2013-06-17 16:35 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: kvm, Alexander Graf, kvm-ppc, linux-kernel, Paul Mackerras,
linuxppc-dev, David Gibson
In-Reply-To: <1370412673-1345-5-git-send-email-aik@ozlabs.ru>
Il 05/06/2013 08:11, Alexey Kardashevskiy ha scritto:
> +/*
> + * The KVM guest can be backed with 16MB pages (qemu switch
> + * -mem-path /var/lib/hugetlbfs/global/pagesize-16MB/).
Nitpick: we try to avoid references to QEMU, so perhaps
s/qemu switch/for example, with QEMU you can use the command-line option/
Paolo
^ permalink raw reply
* [PATCH v3] powerpc/pci: Fix setup of Freescale PCI / PCIe controllers
From: Rojhalat Ibrahim @ 2013-06-17 14:02 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, linux-kernel, Michael Guntsche
Commit 50d8f87d2b3 (powerpc/fsl-pci Make PCIe hotplug work with Freescale
PCIe controllers) does not handle non-PCIe controllers properly, which causes
a panic during boot for certain configurations.
This patch fixes the issue by calling setup_indirect_pci for all device types.
fsl_indirect_read_config is now only used for booke/86xx PCIe controllers.
Reported-by: Michael Guntsche <mike@it-loops.com>
Cc: Scott Wood <scottwood@freescale.com>
Signed-off-by: Rojhalat Ibrahim <imr@rtschenk.de>
---
v3: Fix compile error
arch/powerpc/sysdev/fsl_pci.c | 24 +++++++++---------------
1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index 028ac1f..46ac1dd 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -97,22 +97,14 @@ static int fsl_indirect_read_config(struct pci_bus *bus, unsigned int devfn,
return indirect_read_config(bus, devfn, offset, len, val);
}
-static struct pci_ops fsl_indirect_pci_ops =
+#if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx)
+
+static struct pci_ops fsl_indirect_pcie_ops =
{
.read = fsl_indirect_read_config,
.write = indirect_write_config,
};
-static void __init fsl_setup_indirect_pci(struct pci_controller* hose,
- resource_size_t cfg_addr,
- resource_size_t cfg_data, u32 flags)
-{
- setup_indirect_pci(hose, cfg_addr, cfg_data, flags);
- hose->ops = &fsl_indirect_pci_ops;
-}
-
-#if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx)
-
#define MAX_PHYS_ADDR_BITS 40
static u64 pci64_dma_offset = 1ull << MAX_PHYS_ADDR_BITS;
@@ -504,13 +496,15 @@ int __init fsl_add_bridge(struct platform_device *pdev, int is_primary)
if (!hose->private_data)
goto no_bridge;
- fsl_setup_indirect_pci(hose, rsrc.start, rsrc.start + 0x4,
- PPC_INDIRECT_TYPE_BIG_ENDIAN);
+ setup_indirect_pci(hose, rsrc.start, rsrc.start + 0x4,
+ PPC_INDIRECT_TYPE_BIG_ENDIAN);
if (in_be32(&pci->block_rev1) < PCIE_IP_REV_3_0)
hose->indirect_type |= PPC_INDIRECT_TYPE_FSL_CFG_REG_LINK;
if (early_find_capability(hose, 0, 0, PCI_CAP_ID_EXP)) {
+ /* use fsl_indirect_read_config for PCIe */
+ hose->ops = &fsl_indirect_pcie_ops;
/* For PCIE read HEADER_TYPE to identify controler mode */
early_read_config_byte(hose, 0, 0, PCI_HEADER_TYPE, &hdr_type);
if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE)
@@ -814,8 +808,8 @@ int __init mpc83xx_add_bridge(struct device_node *dev)
if (ret)
goto err0;
} else {
- fsl_setup_indirect_pci(hose, rsrc_cfg.start,
- rsrc_cfg.start + 4, 0);
+ setup_indirect_pci(hose, rsrc_cfg.start,
+ rsrc_cfg.start + 4, 0);
}
printk(KERN_INFO "Found FSL PCI host bridge at 0x%016llx. "
--
1.8.1.5
^ permalink raw reply related
* Re: Regression in RCU subsystem in latest mainline kernel
From: Steven Rostedt @ 2013-06-17 13:51 UTC (permalink / raw)
To: Rojhalat Ibrahim; +Cc: paulmck, linuxppc-dev, linux-kernel
In-Reply-To: <1558776.vQIgbXvFW8@pcimr>
On Mon, 2013-06-17 at 15:21 +0200, Rojhalat Ibrahim wrote:
> On Friday 14 June 2013 22:17:34 Steven Rostedt wrote:
> > On Sat, 2013-06-15 at 12:02 +1000, Benjamin Herrenschmidt wrote:
> > > On Fri, 2013-06-14 at 17:06 -0400, Steven Rostedt wrote:
> > > > I was pretty much able to reproduce this on my PA Semi PPC box. Funny
> > > > thing is, when I type on the console, it makes progress. Anyway, it
> > > > seems that powerpc has an issue with irq_work(). I'll try to get some
> > > > time either tonight or next week to figure it out.
> > >
> > > Does this help ?
> >
> > It did for me. Rojhalat, did this fix your issue too?
> >
>
> FWIW, since the fix is already applied, but it fixes my problem too.
>
Thanks for the update. Yeah, Ben was about to send a queue to Linus, and
as this proved to fix it for me, he felt that it needed to go in that
queue instead of waiting for an update.
But I do appreciate the update as I wasn't 100% that this would fix the
problem for you too.
-- Steve
^ permalink raw reply
* Re: Regression in RCU subsystem in latest mainline kernel
From: Rojhalat Ibrahim @ 2013-06-17 13:21 UTC (permalink / raw)
To: Steven Rostedt; +Cc: paulmck, linuxppc-dev, linux-kernel
In-Reply-To: <1371262654.9844.340.camel@gandalf.local.home>
On Friday 14 June 2013 22:17:34 Steven Rostedt wrote:
> On Sat, 2013-06-15 at 12:02 +1000, Benjamin Herrenschmidt wrote:
> > On Fri, 2013-06-14 at 17:06 -0400, Steven Rostedt wrote:
> > > I was pretty much able to reproduce this on my PA Semi PPC box. Funny
> > > thing is, when I type on the console, it makes progress. Anyway, it
> > > seems that powerpc has an issue with irq_work(). I'll try to get some
> > > time either tonight or next week to figure it out.
> >
> > Does this help ?
>
> It did for me. Rojhalat, did this fix your issue too?
>
FWIW, since the fix is already applied, but it fixes my problem too.
Rojhalat
> -- Steve
>
> > diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
> > index 5cbcf4d..ea185e0 100644
> > --- a/arch/powerpc/kernel/irq.c
> > +++ b/arch/powerpc/kernel/irq.c
> > @@ -162,7 +162,7 @@ notrace unsigned int __check_irq_replay(void)
> >
> > * in case we also had a rollover while hard disabled
> > */
> >
> > local_paca->irq_happened &= ~PACA_IRQ_DEC;
> >
> > - if (decrementer_check_overflow())
> > + if ((happened & PACA_IRQ_DEC) || decrementer_check_overflow())
> >
> > return 0x900;
> >
> > /* Finally check if an external interrupt happened */
> >
> > Cheers,
> > Ben.
^ permalink raw reply
* Re: [PATCH v2] powerpc/pci: Fix setup of Freescale PCI / PCIe controllers
From: Rojhalat Ibrahim @ 2013-06-17 13:15 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, linux-kernel, Michael Guntsche
In-Reply-To: <1371241083.2996.7@snotra>
On Friday 14 June 2013 15:18:03 Scott Wood wrote:
> On 06/14/2013 04:05:34 AM, Rojhalat Ibrahim wrote:
> > Commit 50d8f87d2b3 (powerpc/fsl-pci Make PCIe hotplug work with
> > Freescale
> > PCIe controllers) does not handle non-PCIe controllers properly,
> > which causes
> > a panic during boot for certain configurations.
> > This patch fixes the issue by calling setup_indirect_pci for all
> > device types.
> > fsl_indirect_read_config is now only used for booke/86xx PCIe
> > controllers.
> >
> > Reported-by: Michael Guntsche <mike@it-loops.com>
> > Cc: Scott Wood <scottwood@freescale.com>
> > Signed-off-by: Rojhalat Ibrahim <imr@rtschenk.de>
> > ---
> > v2: Make it more consistent.
> >
> > arch/powerpc/sysdev/fsl_pci.c | 20 +++++++-------------
> > 1 file changed, 7 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/powerpc/sysdev/fsl_pci.c
> > b/arch/powerpc/sysdev/fsl_pci.c
> > index 028ac1f..5682c8a 100644
> > --- a/arch/powerpc/sysdev/fsl_pci.c
> > +++ b/arch/powerpc/sysdev/fsl_pci.c
> > @@ -97,20 +97,12 @@ static int fsl_indirect_read_config(struct
> > pci_bus *bus,
> > unsigned int devfn,
> >
> > return indirect_read_config(bus, devfn, offset, len, val);
> >
> > }
> >
> > -static struct pci_ops fsl_indirect_pci_ops =
> > +static struct pci_ops fsl_indirect_pcie_ops =
> >
> > {
> >
> > .read = fsl_indirect_read_config,
> > .write = indirect_write_config,
> >
> > };
>
> On 83xx:
> cc1: warnings being treated as errors
> /home/scott/fsl/git/linux/upstream/arch/powerpc/sysdev/fsl_pci.c:100:23:
> error: 'fsl_indirect_pcie_ops' defined but not used
> make[2]: *** [arch/powerpc/sysdev/fsl_pci.o] Error 1
> make[2]: *** Waiting for unfinished jobs....
>
> I can fix this when applying, but this makes me wonder how you tested
> it, given that the whole point is to fix 83xx... Did you fix this and
> then accidentally sent a stale version?
>
> Also, please be careful that the patch doesn't get line wrapped -- I
> had to manually unwrap a couple places. Use git send-email if you
> can't get KMail to cooperate.
>
> -Scott
Sorry about the mess. I'll send a v3.
Please note: I don't have an 83xx system. So I can only test if it compiles,
which I obviously did not do with the right config.
Rojhalat
^ permalink raw reply
* Re: [PATCH 1/4] KVM: PPC: Add support for multiple-TCE hcalls
From: Alexander Graf @ 2013-06-17 10:48 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: kvm, linux-kernel, kvm-ppc, Paul Mackerras, linuxppc-dev,
David Gibson
In-Reply-To: <51BEE906.5010306@suse.de>
On 06/17/2013 12:46 PM, Alexander Graf wrote:
> On 06/17/2013 10:51 AM, Alexey Kardashevskiy wrote:
>> On 06/17/2013 06:40 PM, Alexander Graf wrote:
>>> On 17.06.2013, at 10:34, Alexey Kardashevskiy wrote:
>>>
>>>> On 06/17/2013 06:02 PM, Alexander Graf wrote:
>>>>> On 17.06.2013, at 09:55, Alexey Kardashevskiy wrote:
>>>>>
>>>>>> On 06/17/2013 08:06 AM, Alexander Graf wrote:
>>>>>>> On 05.06.2013, at 08:11, Alexey Kardashevskiy wrote:
>>>>>>>
>>>>>>>> This adds real mode handlers for the H_PUT_TCE_INDIRECT and
>>>>>>>> H_STUFF_TCE hypercalls for QEMU emulated devices such as
>>>>>>>> IBMVIO devices or emulated PCI. These calls allow adding
>>>>>>>> multiple entries (up to 512) into the TCE table in one call
>>>>>>>> which saves time on transition to/from real mode.
>>>>>>>>
>>>>>>>> This adds a tce_tmp cache to kvm_vcpu_arch to save valid TCEs
>>>>>>>> (copied from user and verified) before writing the whole list
>>>>>>>> into the TCE table. This cache will be utilized more in the
>>>>>>>> upcoming VFIO/IOMMU support to continue TCE list processing in
>>>>>>>> the virtual mode in the case if the real mode handler failed
>>>>>>>> for some reason.
>>>>>>>>
>>>>>>>> This adds a guest physical to host real address converter and
>>>>>>>> calls the existing H_PUT_TCE handler. The converting function
>>>>>>>> is going to be fully utilized by upcoming VFIO supporting
>>>>>>>> patches.
>>>>>>>>
>>>>>>>> This also implements the KVM_CAP_PPC_MULTITCE capability, so
>>>>>>>> in order to support the functionality of this patch, QEMU
>>>>>>>> needs to query for this capability and set the
>>>>>>>> "hcall-multi-tce" hypertas property only if the capability is
>>>>>>>> present, otherwise there will be serious performance
>>>>>>>> degradation.
>>>>>>>>
>>>>>>>> Cc: David Gibson<david@gibson.dropbear.id.au> Signed-off-by:
>>>>>>>> Alexey Kardashevskiy<aik@ozlabs.ru> Signed-off-by: Paul
>>>>>>>> Mackerras<paulus@samba.org>
>>>>>>> Only a few minor nits. Ben already commented on implementation
>>>>>>> details.
>>>>>>>
>>>>>>>> --- Changelog: 2013/06/05: * fixed mistype about IBMVIO in the
>>>>>>>> commit message * updated doc and moved it to another section *
>>>>>>>> changed capability number
>>>>>>>>
>>>>>>>> 2013/05/21: * added kvm_vcpu_arch::tce_tmp * removed cleanup
>>>>>>>> if put_indirect failed, instead we do not even start writing
>>>>>>>> to TCE table if we cannot get TCEs from the user and they are
>>>>>>>> invalid * kvmppc_emulated_h_put_tce is split to
>>>>>>>> kvmppc_emulated_put_tce and kvmppc_emulated_validate_tce (for
>>>>>>>> the previous item) * fixed bug with failthrough for H_IPI *
>>>>>>>> removed all get_user() from real mode handlers *
>>>>>>>> kvmppc_lookup_pte() added (instead of making lookup_linux_pte
>>>>>>>> public) --- Documentation/virtual/kvm/api.txt | 17 ++
>>>>>>>> arch/powerpc/include/asm/kvm_host.h | 2 +
>>>>>>>> arch/powerpc/include/asm/kvm_ppc.h | 16 +-
>>>>>>>> arch/powerpc/kvm/book3s_64_vio.c | 118 ++++++++++++++
>>>>>>>> arch/powerpc/kvm/book3s_64_vio_hv.c | 266
>>>>>>>> +++++++++++++++++++++++++++---- arch/powerpc/kvm/book3s_hv.c
>>>>>>>> | 39 +++++ arch/powerpc/kvm/book3s_hv_rmhandlers.S | 6 +
>>>>>>>> arch/powerpc/kvm/book3s_pr_papr.c | 37 ++++-
>>>>>>>> arch/powerpc/kvm/powerpc.c | 3 +
>>>>>>>> include/uapi/linux/kvm.h | 1 + 10 files
>>>>>>>> changed, 473 insertions(+), 32 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/Documentation/virtual/kvm/api.txt
>>>>>>>> b/Documentation/virtual/kvm/api.txt index 5f91eda..6c082ff
>>>>>>>> 100644 --- a/Documentation/virtual/kvm/api.txt +++
>>>>>>>> b/Documentation/virtual/kvm/api.txt @@ -2362,6 +2362,23 @@
>>>>>>>> calls by the guest for that service will be passed to
>>>>>>>> userspace to be handled.
>>>>>>>>
>>>>>>>>
>>>>>>>> +4.83 KVM_CAP_PPC_MULTITCE + +Capability:
>>>>>>>> KVM_CAP_PPC_MULTITCE +Architectures: ppc +Type: vm + +This
>>>>>>>> capability tells the guest that multiple TCE entry add/remove
>>>>>>>> hypercalls +handling is supported by the kernel. This
>>>>>>>> significanly accelerates DMA +operations for PPC KVM guests.
>>>>>>>> + +Unlike other capabilities in this section, this one does
>>>>>>>> not have an ioctl. +Instead, when the capability is present,
>>>>>>>> the H_PUT_TCE_INDIRECT and +H_STUFF_TCE hypercalls are to be
>>>>>>>> handled in the host kernel and not passed to +the guest.
>>>>>>>> Othwerwise it might be better for the guest to continue using
>>>>>>>> H_PUT_TCE +hypercall (if KVM_CAP_SPAPR_TCE or
>>>>>>>> KVM_CAP_SPAPR_TCE_IOMMU are present).
>>>>>>> While this describes perfectly well what the consequences are of
>>>>>>> the patches, it does not describe properly what the CAP actually
>>>>>>> expresses. The CAP only says "this kernel is able to handle
>>>>>>> H_PUT_TCE_INDIRECT and H_STUFF_TCE hypercalls directly". All
>>>>>>> other consequences are nice to document, but the semantics of
>>>>>>> the CAP are missing.
>>>>>>
>>>>>> ? It expresses ability to handle 2 hcalls. What is missing?
>>>>> You don't describe the kvm<-> qemu interface. You describe some
>>>>> decisions qemu can take from this cap.
>>>>
>>>> This file does not mention qemu at all. And the interface is - qemu
>>>> (or kvmtool could do that) just adds "hcall-multi-tce" to
>>>> "ibm,hypertas-functions" but this is for pseries linux and AIX could
>>>> always do it (no idea about it). Does it really have to be in this
>>>> file?
>>> Ok, let's go back a step. What does this CAP describe? Don't look at
>>> the
>>> description you wrote above. Just write a new one.
>> The CAP means the kernel is capable of handling hcalls A and B without
>> passing those into the user space. That accelerates DMA.
>>
>>
>>> What exactly can user space expect when it finds this CAP?
>> The user space can expect that its handlers for A and B are not going
>> to be
>> called if it configures the guest appropriately.
Actually a nitpick here too. User space can expect that its handlers for
A and B are going to already be processed by KVM. Regardless of how user
space configures the guest.
Alex
^ permalink raw reply
* Re: [PATCH 1/4] KVM: PPC: Add support for multiple-TCE hcalls
From: Alexander Graf @ 2013-06-17 10:46 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: kvm, linux-kernel, kvm-ppc, Paul Mackerras, linuxppc-dev,
David Gibson
In-Reply-To: <51BECDF9.2020701@ozlabs.ru>
On 06/17/2013 10:51 AM, Alexey Kardashevskiy wrote:
> On 06/17/2013 06:40 PM, Alexander Graf wrote:
>> On 17.06.2013, at 10:34, Alexey Kardashevskiy wrote:
>>
>>> On 06/17/2013 06:02 PM, Alexander Graf wrote:
>>>> On 17.06.2013, at 09:55, Alexey Kardashevskiy wrote:
>>>>
>>>>> On 06/17/2013 08:06 AM, Alexander Graf wrote:
>>>>>> On 05.06.2013, at 08:11, Alexey Kardashevskiy wrote:
>>>>>>
>>>>>>> This adds real mode handlers for the H_PUT_TCE_INDIRECT and
>>>>>>> H_STUFF_TCE hypercalls for QEMU emulated devices such as
>>>>>>> IBMVIO devices or emulated PCI. These calls allow adding
>>>>>>> multiple entries (up to 512) into the TCE table in one call
>>>>>>> which saves time on transition to/from real mode.
>>>>>>>
>>>>>>> This adds a tce_tmp cache to kvm_vcpu_arch to save valid TCEs
>>>>>>> (copied from user and verified) before writing the whole list
>>>>>>> into the TCE table. This cache will be utilized more in the
>>>>>>> upcoming VFIO/IOMMU support to continue TCE list processing in
>>>>>>> the virtual mode in the case if the real mode handler failed
>>>>>>> for some reason.
>>>>>>>
>>>>>>> This adds a guest physical to host real address converter and
>>>>>>> calls the existing H_PUT_TCE handler. The converting function
>>>>>>> is going to be fully utilized by upcoming VFIO supporting
>>>>>>> patches.
>>>>>>>
>>>>>>> This also implements the KVM_CAP_PPC_MULTITCE capability, so
>>>>>>> in order to support the functionality of this patch, QEMU
>>>>>>> needs to query for this capability and set the
>>>>>>> "hcall-multi-tce" hypertas property only if the capability is
>>>>>>> present, otherwise there will be serious performance
>>>>>>> degradation.
>>>>>>>
>>>>>>> Cc: David Gibson<david@gibson.dropbear.id.au> Signed-off-by:
>>>>>>> Alexey Kardashevskiy<aik@ozlabs.ru> Signed-off-by: Paul
>>>>>>> Mackerras<paulus@samba.org>
>>>>>> Only a few minor nits. Ben already commented on implementation
>>>>>> details.
>>>>>>
>>>>>>> --- Changelog: 2013/06/05: * fixed mistype about IBMVIO in the
>>>>>>> commit message * updated doc and moved it to another section *
>>>>>>> changed capability number
>>>>>>>
>>>>>>> 2013/05/21: * added kvm_vcpu_arch::tce_tmp * removed cleanup
>>>>>>> if put_indirect failed, instead we do not even start writing
>>>>>>> to TCE table if we cannot get TCEs from the user and they are
>>>>>>> invalid * kvmppc_emulated_h_put_tce is split to
>>>>>>> kvmppc_emulated_put_tce and kvmppc_emulated_validate_tce (for
>>>>>>> the previous item) * fixed bug with failthrough for H_IPI *
>>>>>>> removed all get_user() from real mode handlers *
>>>>>>> kvmppc_lookup_pte() added (instead of making lookup_linux_pte
>>>>>>> public) --- Documentation/virtual/kvm/api.txt | 17 ++
>>>>>>> arch/powerpc/include/asm/kvm_host.h | 2 +
>>>>>>> arch/powerpc/include/asm/kvm_ppc.h | 16 +-
>>>>>>> arch/powerpc/kvm/book3s_64_vio.c | 118 ++++++++++++++
>>>>>>> arch/powerpc/kvm/book3s_64_vio_hv.c | 266
>>>>>>> +++++++++++++++++++++++++++---- arch/powerpc/kvm/book3s_hv.c
>>>>>>> | 39 +++++ arch/powerpc/kvm/book3s_hv_rmhandlers.S | 6 +
>>>>>>> arch/powerpc/kvm/book3s_pr_papr.c | 37 ++++-
>>>>>>> arch/powerpc/kvm/powerpc.c | 3 +
>>>>>>> include/uapi/linux/kvm.h | 1 + 10 files
>>>>>>> changed, 473 insertions(+), 32 deletions(-)
>>>>>>>
>>>>>>> diff --git a/Documentation/virtual/kvm/api.txt
>>>>>>> b/Documentation/virtual/kvm/api.txt index 5f91eda..6c082ff
>>>>>>> 100644 --- a/Documentation/virtual/kvm/api.txt +++
>>>>>>> b/Documentation/virtual/kvm/api.txt @@ -2362,6 +2362,23 @@
>>>>>>> calls by the guest for that service will be passed to
>>>>>>> userspace to be handled.
>>>>>>>
>>>>>>>
>>>>>>> +4.83 KVM_CAP_PPC_MULTITCE + +Capability:
>>>>>>> KVM_CAP_PPC_MULTITCE +Architectures: ppc +Type: vm + +This
>>>>>>> capability tells the guest that multiple TCE entry add/remove
>>>>>>> hypercalls +handling is supported by the kernel. This
>>>>>>> significanly accelerates DMA +operations for PPC KVM guests.
>>>>>>> + +Unlike other capabilities in this section, this one does
>>>>>>> not have an ioctl. +Instead, when the capability is present,
>>>>>>> the H_PUT_TCE_INDIRECT and +H_STUFF_TCE hypercalls are to be
>>>>>>> handled in the host kernel and not passed to +the guest.
>>>>>>> Othwerwise it might be better for the guest to continue using
>>>>>>> H_PUT_TCE +hypercall (if KVM_CAP_SPAPR_TCE or
>>>>>>> KVM_CAP_SPAPR_TCE_IOMMU are present).
>>>>>> While this describes perfectly well what the consequences are of
>>>>>> the patches, it does not describe properly what the CAP actually
>>>>>> expresses. The CAP only says "this kernel is able to handle
>>>>>> H_PUT_TCE_INDIRECT and H_STUFF_TCE hypercalls directly". All
>>>>>> other consequences are nice to document, but the semantics of
>>>>>> the CAP are missing.
>>>>>
>>>>> ? It expresses ability to handle 2 hcalls. What is missing?
>>>> You don't describe the kvm<-> qemu interface. You describe some
>>>> decisions qemu can take from this cap.
>>>
>>> This file does not mention qemu at all. And the interface is - qemu
>>> (or kvmtool could do that) just adds "hcall-multi-tce" to
>>> "ibm,hypertas-functions" but this is for pseries linux and AIX could
>>> always do it (no idea about it). Does it really have to be in this
>>> file?
>> Ok, let's go back a step. What does this CAP describe? Don't look at the
>> description you wrote above. Just write a new one.
> The CAP means the kernel is capable of handling hcalls A and B without
> passing those into the user space. That accelerates DMA.
>
>
>> What exactly can user space expect when it finds this CAP?
> The user space can expect that its handlers for A and B are not going to be
> called if it configures the guest appropriately.
>
> Any better? :)
A lot, yes. This is what the CAP actually means.
It's nice to give some guidance in the documentation of implications
(should expose "ibm,hypertas-functions" to enable the guest to actually
use these for example) but the first paragraph should only indicate what
the CAP changes.
Alex
^ permalink raw reply
* Re: [PATCH] powerpc: Fix emulation of illegal instructions on PowerNV platform
From: Anshuman Khandual @ 2013-06-17 9:32 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <20130614100741.GA612@iris.ozlabs.ibm.com>
On 06/14/2013 03:37 PM, Paul Mackerras wrote:
> Normally, the kernel emulates a few instructions that are unimplemented
> on some processors (e.g. the old dcba instruction), or privileged (e.g.
> mfpvr). The emulation of unimplemented instructions is currently not
> working on the PowerNV platform. The reason is that on these machines,
> unimplemented and illegal instructions cause a hypervisor emulation
> assist interrupt, rather than a program interrupt as on older CPUs.
> Our vector for the emulation assist interrupt just calls
> program_check_exception() directly, without setting the bit in SRR1
> that indicates an illegal instruction interrupt.
Hey Paul,
The only difference between (1) Program interrupt and (2) Hypervisor
emulation assist interrupt is that in the earlier, HW sets "illegal
instruction" as the reason code in the SRR1 registers implicitly.
We fix it in the later interrupt vector by doing the same explicitly
in SW ? The cause of the problem before was not having the appropriate
reason code ("illegal instruction") in the SRR1 register for program_
check_exception() function to take it forward ?
Regards
Anshuman
^ permalink raw reply
* Re: [PATCH 2/4] powerpc: Prepare to support kernel handling of IOMMU map/unmap
From: Alexey Kardashevskiy @ 2013-06-17 9:17 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: kvm, linux-kernel, kvm-ppc, Alexander Graf, linux-mm@kvack.org,
Paul Mackerras, linuxppc-dev, David Gibson
In-Reply-To: <1371356818.21896.114.camel@pasglop>
On 06/16/2013 02:26 PM, Benjamin Herrenschmidt wrote:
>> +#if defined(CONFIG_SPARSEMEM_VMEMMAP) || defined(CONFIG_FLATMEM)
>> +int realmode_get_page(struct page *page)
>> +{
>> + if (PageCompound(page))
>> + return -EAGAIN;
>> +
>> + get_page(page);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(realmode_get_page);
>> +
>> +int realmode_put_page(struct page *page)
>> +{
>> + if (PageCompound(page))
>> + return -EAGAIN;
>> +
>> + if (!atomic_add_unless(&page->_count, -1, 1))
>> + return -EAGAIN;
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(realmode_put_page);
>> +#endif
>
> Several worries here, mostly that if the generic code ever changes
> (something gets added to get_page() that makes it no-longer safe for use
> in real mode for example, or some other condition gets added to
> put_page()), we go out of sync and potentially end up with very hard and
> very subtle bugs.
>
> It might be worth making sure that:
>
> - This is reviewed by some generic VM people (and make sure they
> understand why we need to do that)
>
> - A comment is added to get_page() and put_page() to make sure that if
> they are changed in any way, dbl check the impact on our
> realmode_get_page() (or "ping" us to make sure things are still ok).
After changing get_page() to get_page_unless_zero(), the get_page API I use is:
get_page_unless_zero() - basically atomic_inc_not_zero()
atomic_add_unless() - just operated with the counter
PageCompound() - check if it is a huge page.
No usage of get_page or put_page.
If any of those changes, I would expect it to hit us immediately, no?
So it may only make sense to add a comment to PageCompound(). But the
comment says "PageCompound is generally not used in hot code paths", and
our path is hot. Heh.
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 6d53675..c70a654 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -329,7 +329,8 @@ static inline void set_page_writeback(struct page *page)
* System with lots of page flags available. This allows separate
* flags for PageHead() and PageTail() checks of compound pages so that bit
* tests can be used in performance sensitive paths. PageCompound is
- * generally not used in hot code paths.
+ * generally not used in hot code paths except arch/powerpc/mm/init_64.c
+ * which uses it to detect huge pages and avoid handling those in real mode.
*/
__PAGEFLAG(Head, head) CLEARPAGEFLAG(Head, head)
__PAGEFLAG(Tail, tail)
So?
--
Alexey
^ permalink raw reply related
* Re: [PATCH 1/4] KVM: PPC: Add support for multiple-TCE hcalls
From: Alexey Kardashevskiy @ 2013-06-17 8:51 UTC (permalink / raw)
To: Alexander Graf
Cc: kvm, linux-kernel, kvm-ppc, Paul Mackerras, linuxppc-dev,
David Gibson
In-Reply-To: <5BDDB985-B0A2-476C-B6E5-76E88E3FB182@suse.de>
On 06/17/2013 06:40 PM, Alexander Graf wrote:
>
> On 17.06.2013, at 10:34, Alexey Kardashevskiy wrote:
>
>> On 06/17/2013 06:02 PM, Alexander Graf wrote:
>>>
>>> On 17.06.2013, at 09:55, Alexey Kardashevskiy wrote:
>>>
>>>> On 06/17/2013 08:06 AM, Alexander Graf wrote:
>>>>>
>>>>> On 05.06.2013, at 08:11, Alexey Kardashevskiy wrote:
>>>>>
>>>>>> This adds real mode handlers for the H_PUT_TCE_INDIRECT and
>>>>>> H_STUFF_TCE hypercalls for QEMU emulated devices such as
>>>>>> IBMVIO devices or emulated PCI. These calls allow adding
>>>>>> multiple entries (up to 512) into the TCE table in one call
>>>>>> which saves time on transition to/from real mode.
>>>>>>
>>>>>> This adds a tce_tmp cache to kvm_vcpu_arch to save valid TCEs
>>>>>> (copied from user and verified) before writing the whole list
>>>>>> into the TCE table. This cache will be utilized more in the
>>>>>> upcoming VFIO/IOMMU support to continue TCE list processing in
>>>>>> the virtual mode in the case if the real mode handler failed
>>>>>> for some reason.
>>>>>>
>>>>>> This adds a guest physical to host real address converter and
>>>>>> calls the existing H_PUT_TCE handler. The converting function
>>>>>> is going to be fully utilized by upcoming VFIO supporting
>>>>>> patches.
>>>>>>
>>>>>> This also implements the KVM_CAP_PPC_MULTITCE capability, so
>>>>>> in order to support the functionality of this patch, QEMU
>>>>>> needs to query for this capability and set the
>>>>>> "hcall-multi-tce" hypertas property only if the capability is
>>>>>> present, otherwise there will be serious performance
>>>>>> degradation.
>>>>>>
>>>>>> Cc: David Gibson <david@gibson.dropbear.id.au> Signed-off-by:
>>>>>> Alexey Kardashevskiy <aik@ozlabs.ru> Signed-off-by: Paul
>>>>>> Mackerras <paulus@samba.org>
>>>>>
>>>>> Only a few minor nits. Ben already commented on implementation
>>>>> details.
>>>>>
>>>>>>
>>>>>> --- Changelog: 2013/06/05: * fixed mistype about IBMVIO in the
>>>>>> commit message * updated doc and moved it to another section *
>>>>>> changed capability number
>>>>>>
>>>>>> 2013/05/21: * added kvm_vcpu_arch::tce_tmp * removed cleanup
>>>>>> if put_indirect failed, instead we do not even start writing
>>>>>> to TCE table if we cannot get TCEs from the user and they are
>>>>>> invalid * kvmppc_emulated_h_put_tce is split to
>>>>>> kvmppc_emulated_put_tce and kvmppc_emulated_validate_tce (for
>>>>>> the previous item) * fixed bug with failthrough for H_IPI *
>>>>>> removed all get_user() from real mode handlers *
>>>>>> kvmppc_lookup_pte() added (instead of making lookup_linux_pte
>>>>>> public) --- Documentation/virtual/kvm/api.txt | 17 ++
>>>>>> arch/powerpc/include/asm/kvm_host.h | 2 +
>>>>>> arch/powerpc/include/asm/kvm_ppc.h | 16 +-
>>>>>> arch/powerpc/kvm/book3s_64_vio.c | 118 ++++++++++++++
>>>>>> arch/powerpc/kvm/book3s_64_vio_hv.c | 266
>>>>>> +++++++++++++++++++++++++++---- arch/powerpc/kvm/book3s_hv.c
>>>>>> | 39 +++++ arch/powerpc/kvm/book3s_hv_rmhandlers.S | 6 +
>>>>>> arch/powerpc/kvm/book3s_pr_papr.c | 37 ++++-
>>>>>> arch/powerpc/kvm/powerpc.c | 3 +
>>>>>> include/uapi/linux/kvm.h | 1 + 10 files
>>>>>> changed, 473 insertions(+), 32 deletions(-)
>>>>>>
>>>>>> diff --git a/Documentation/virtual/kvm/api.txt
>>>>>> b/Documentation/virtual/kvm/api.txt index 5f91eda..6c082ff
>>>>>> 100644 --- a/Documentation/virtual/kvm/api.txt +++
>>>>>> b/Documentation/virtual/kvm/api.txt @@ -2362,6 +2362,23 @@
>>>>>> calls by the guest for that service will be passed to
>>>>>> userspace to be handled.
>>>>>>
>>>>>>
>>>>>> +4.83 KVM_CAP_PPC_MULTITCE + +Capability:
>>>>>> KVM_CAP_PPC_MULTITCE +Architectures: ppc +Type: vm + +This
>>>>>> capability tells the guest that multiple TCE entry add/remove
>>>>>> hypercalls +handling is supported by the kernel. This
>>>>>> significanly accelerates DMA +operations for PPC KVM guests.
>>>>>> + +Unlike other capabilities in this section, this one does
>>>>>> not have an ioctl. +Instead, when the capability is present,
>>>>>> the H_PUT_TCE_INDIRECT and +H_STUFF_TCE hypercalls are to be
>>>>>> handled in the host kernel and not passed to +the guest.
>>>>>> Othwerwise it might be better for the guest to continue using
>>>>>> H_PUT_TCE +hypercall (if KVM_CAP_SPAPR_TCE or
>>>>>> KVM_CAP_SPAPR_TCE_IOMMU are present).
>>>>>
>>>>
>>>>> While this describes perfectly well what the consequences are of
>>>>> the patches, it does not describe properly what the CAP actually
>>>>> expresses. The CAP only says "this kernel is able to handle
>>>>> H_PUT_TCE_INDIRECT and H_STUFF_TCE hypercalls directly". All
>>>>> other consequences are nice to document, but the semantics of
>>>>> the CAP are missing.
>>>>
>>>>
>>>> ? It expresses ability to handle 2 hcalls. What is missing?
>>>
>>> You don't describe the kvm <-> qemu interface. You describe some
>>> decisions qemu can take from this cap.
>>
>>
>> This file does not mention qemu at all. And the interface is - qemu
>> (or kvmtool could do that) just adds "hcall-multi-tce" to
>> "ibm,hypertas-functions" but this is for pseries linux and AIX could
>> always do it (no idea about it). Does it really have to be in this
>> file?
>
> Ok, let's go back a step. What does this CAP describe? Don't look at the
> description you wrote above. Just write a new one.
The CAP means the kernel is capable of handling hcalls A and B without
passing those into the user space. That accelerates DMA.
> What exactly can user space expect when it finds this CAP?
The user space can expect that its handlers for A and B are not going to be
called if it configures the guest appropriately.
Any better? :)
--
Alexey
^ permalink raw reply
* Re: [PATCH 1/4] KVM: PPC: Add support for multiple-TCE hcalls
From: Alexander Graf @ 2013-06-17 8:42 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: kvm, Alexey Kardashevskiy, linux-kernel, kvm-ppc, Paul Mackerras,
linuxppc-dev, David Gibson
In-Reply-To: <1371458225.21896.153.camel@pasglop>
On 17.06.2013, at 10:37, Benjamin Herrenschmidt wrote:
> On Mon, 2013-06-17 at 17:55 +1000, Alexey Kardashevskiy wrote:
>> David:
>> =3D=3D=3D
>> So, in the case of MULTITCE, that's not quite right. PR KVM can
>> emulate a PAPR system on a BookE machine, and there's no reason not =
to
>> allow TCE acceleration as well. We can't make it dependent on PAPR
>> mode being selected, because that's enabled per-vcpu, whereas these
>> capabilities are queried on the VM before the vcpus are created.
>> =3D=3D=3D
>>=20
>> Wrong?
>=20
> The capability just tells qemu the kernel supports it, it doesn't have
> to depend on PAPR mode, qemu can sort things out no ?
Yes, this goes hand-in-hand with the documentation bit I'm trying to get =
through to Alexey atm. The CAP merely says that if in PAPR mode the =
kernel can handle hypercalls X and Y itself.
This is true for all book3s implementations as the patches stand. It is =
not true for BookE as the patches stand. Hence the CAP should be limited =
to book3s, regardless of its mode :).
Alex
^ permalink raw reply
* Re: [PATCH 1/4] KVM: PPC: Add support for multiple-TCE hcalls
From: Alexander Graf @ 2013-06-17 8:40 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: kvm, linux-kernel, kvm-ppc, Paul Mackerras, linuxppc-dev,
David Gibson
In-Reply-To: <51BECA2B.60401@ozlabs.ru>
On 17.06.2013, at 10:34, Alexey Kardashevskiy wrote:
> On 06/17/2013 06:02 PM, Alexander Graf wrote:
>>=20
>> On 17.06.2013, at 09:55, Alexey Kardashevskiy wrote:
>>=20
>>> On 06/17/2013 08:06 AM, Alexander Graf wrote:
>>>>=20
>>>> On 05.06.2013, at 08:11, Alexey Kardashevskiy wrote:
>>>>=20
>>>>> This adds real mode handlers for the H_PUT_TCE_INDIRECT and
>>>>> H_STUFF_TCE hypercalls for QEMU emulated devices such as IBMVIO
>>>>> devices or emulated PCI. These calls allow adding multiple =
entries
>>>>> (up to 512) into the TCE table in one call which saves time on
>>>>> transition to/from real mode.
>>>>>=20
>>>>> This adds a tce_tmp cache to kvm_vcpu_arch to save valid TCEs
>>>>> (copied from user and verified) before writing the whole list into
>>>>> the TCE table. This cache will be utilized more in the upcoming
>>>>> VFIO/IOMMU support to continue TCE list processing in the virtual
>>>>> mode in the case if the real mode handler failed for some reason.
>>>>>=20
>>>>> This adds a guest physical to host real address converter
>>>>> and calls the existing H_PUT_TCE handler. The converting function
>>>>> is going to be fully utilized by upcoming VFIO supporting patches.
>>>>>=20
>>>>> This also implements the KVM_CAP_PPC_MULTITCE capability,
>>>>> so in order to support the functionality of this patch, QEMU
>>>>> needs to query for this capability and set the "hcall-multi-tce"
>>>>> hypertas property only if the capability is present, otherwise
>>>>> there will be serious performance degradation.
>>>>>=20
>>>>> Cc: David Gibson <david@gibson.dropbear.id.au>
>>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>>> Signed-off-by: Paul Mackerras <paulus@samba.org>
>>>>=20
>>>> Only a few minor nits. Ben already commented on implementation =
details.
>>>>=20
>>>>>=20
>>>>> ---
>>>>> Changelog:
>>>>> 2013/06/05:
>>>>> * fixed mistype about IBMVIO in the commit message
>>>>> * updated doc and moved it to another section
>>>>> * changed capability number
>>>>>=20
>>>>> 2013/05/21:
>>>>> * added kvm_vcpu_arch::tce_tmp
>>>>> * removed cleanup if put_indirect failed, instead we do not even =
start
>>>>> writing to TCE table if we cannot get TCEs from the user and they =
are
>>>>> invalid
>>>>> * kvmppc_emulated_h_put_tce is split to kvmppc_emulated_put_tce
>>>>> and kvmppc_emulated_validate_tce (for the previous item)
>>>>> * fixed bug with failthrough for H_IPI
>>>>> * removed all get_user() from real mode handlers
>>>>> * kvmppc_lookup_pte() added (instead of making lookup_linux_pte =
public)
>>>>> ---
>>>>> Documentation/virtual/kvm/api.txt | 17 ++
>>>>> arch/powerpc/include/asm/kvm_host.h | 2 +
>>>>> arch/powerpc/include/asm/kvm_ppc.h | 16 +-
>>>>> arch/powerpc/kvm/book3s_64_vio.c | 118 ++++++++++++++
>>>>> arch/powerpc/kvm/book3s_64_vio_hv.c | 266 =
+++++++++++++++++++++++++++----
>>>>> arch/powerpc/kvm/book3s_hv.c | 39 +++++
>>>>> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 6 +
>>>>> arch/powerpc/kvm/book3s_pr_papr.c | 37 ++++-
>>>>> arch/powerpc/kvm/powerpc.c | 3 +
>>>>> include/uapi/linux/kvm.h | 1 +
>>>>> 10 files changed, 473 insertions(+), 32 deletions(-)
>>>>>=20
>>>>> diff --git a/Documentation/virtual/kvm/api.txt =
b/Documentation/virtual/kvm/api.txt
>>>>> index 5f91eda..6c082ff 100644
>>>>> --- a/Documentation/virtual/kvm/api.txt
>>>>> +++ b/Documentation/virtual/kvm/api.txt
>>>>> @@ -2362,6 +2362,23 @@ calls by the guest for that service will be =
passed to userspace to be
>>>>> handled.
>>>>>=20
>>>>>=20
>>>>> +4.83 KVM_CAP_PPC_MULTITCE
>>>>> +
>>>>> +Capability: KVM_CAP_PPC_MULTITCE
>>>>> +Architectures: ppc
>>>>> +Type: vm
>>>>> +
>>>>> +This capability tells the guest that multiple TCE entry =
add/remove hypercalls
>>>>> +handling is supported by the kernel. This significanly =
accelerates DMA
>>>>> +operations for PPC KVM guests.
>>>>> +
>>>>> +Unlike other capabilities in this section, this one does not have =
an ioctl.
>>>>> +Instead, when the capability is present, the H_PUT_TCE_INDIRECT =
and
>>>>> +H_STUFF_TCE hypercalls are to be handled in the host kernel and =
not passed to
>>>>> +the guest. Othwerwise it might be better for the guest to =
continue using H_PUT_TCE
>>>>> +hypercall (if KVM_CAP_SPAPR_TCE or KVM_CAP_SPAPR_TCE_IOMMU are =
present).
>>>>=20
>>>=20
>>>> While this describes perfectly well what the consequences are of =
the
>>>> patches, it does not describe properly what the CAP actually =
expresses.
>>>> The CAP only says "this kernel is able to handle H_PUT_TCE_INDIRECT =
and
>>>> H_STUFF_TCE hypercalls directly". All other consequences are nice =
to
>>>> document, but the semantics of the CAP are missing.
>>>=20
>>>=20
>>> ? It expresses ability to handle 2 hcalls. What is missing?
>>=20
>> You don't describe the kvm <-> qemu interface. You describe some =
decisions qemu can take from this cap.
>=20
>=20
> This file does not mention qemu at all. And the interface is - qemu =
(or
> kvmtool could do that) just adds "hcall-multi-tce" to
> "ibm,hypertas-functions" but this is for pseries linux and AIX could =
always
> do it (no idea about it). Does it really have to be in this file?
Ok, let's go back a step. What does this CAP describe? Don't look at the =
description you wrote above. Just write a new one. What exactly can user =
space expect when it finds this CAP?
>=20
>=20
>=20
>>>> We also usually try to keep KVM behavior unchanged with regards to =
older
>>>> versions until a CAP is enabled. In this case I don't think it =
matters
>>>> all that much, so I'm fine with declaring it as enabled by default.
>>>> Please document that this is a change in behavior versus older KVM
>>>> versions though.
>>>=20
>>>=20
>>> Ok!
>>>=20
>>>=20
>>>>> +
>>>>> +
>>>>> 5. The kvm_run structure
>>>>> ------------------------
>>>>>=20
>>>>> diff --git a/arch/powerpc/include/asm/kvm_host.h =
b/arch/powerpc/include/asm/kvm_host.h
>>>>> index af326cd..85d8f26 100644
>>>>> --- a/arch/powerpc/include/asm/kvm_host.h
>>>>> +++ b/arch/powerpc/include/asm/kvm_host.h
>>>>> @@ -609,6 +609,8 @@ struct kvm_vcpu_arch {
>>>>> spinlock_t tbacct_lock;
>>>>> u64 busy_stolen;
>>>>> u64 busy_preempt;
>>>>> +
>>>>> + unsigned long *tce_tmp; /* TCE cache for TCE_PUT_INDIRECT =
hall */
>>>>> #endif
>>>>> };
>>>>=20
>>>> [...]
>>>>>=20
>>>>>=20
>>>>=20
>>>> [...]
>>>>=20
>>>>> diff --git a/arch/powerpc/kvm/book3s_hv.c =
b/arch/powerpc/kvm/book3s_hv.c
>>>>> index 550f592..a39039a 100644
>>>>> --- a/arch/powerpc/kvm/book3s_hv.c
>>>>> +++ b/arch/powerpc/kvm/book3s_hv.c
>>>>> @@ -568,6 +568,30 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu =
*vcpu)
>>>>> ret =3D kvmppc_xics_hcall(vcpu, req);
>>>>> break;
>>>>> } /* fallthrough */
>>>>=20
>>>> The fallthrough comment isn't accurate anymore.
>>>>=20
>>>>> + return RESUME_HOST;
>>>>> + case H_PUT_TCE:
>>>>> + ret =3D kvmppc_virtmode_h_put_tce(vcpu, =
kvmppc_get_gpr(vcpu, 4),
>>>>> + kvmppc_get_gpr(vcpu, 5),
>>>>> + kvmppc_get_gpr(vcpu, =
6));
>>>>> + if (ret =3D=3D H_TOO_HARD)
>>>>> + return RESUME_HOST;
>>>>> + break;
>>>>> + case H_PUT_TCE_INDIRECT:
>>>>> + ret =3D kvmppc_virtmode_h_put_tce_indirect(vcpu, =
kvmppc_get_gpr(vcpu, 4),
>>>>> + kvmppc_get_gpr(vcpu, 5),
>>>>> + kvmppc_get_gpr(vcpu, 6),
>>>>> + kvmppc_get_gpr(vcpu, =
7));
>>>>> + if (ret =3D=3D H_TOO_HARD)
>>>>> + return RESUME_HOST;
>>>>> + break;
>>>>> + case H_STUFF_TCE:
>>>>> + ret =3D kvmppc_virtmode_h_stuff_tce(vcpu, =
kvmppc_get_gpr(vcpu, 4),
>>>>> + kvmppc_get_gpr(vcpu, 5),
>>>>> + kvmppc_get_gpr(vcpu, 6),
>>>>> + kvmppc_get_gpr(vcpu, =
7));
>>>>> + if (ret =3D=3D H_TOO_HARD)
>>>>> + return RESUME_HOST;
>>>>> + break;
>>>>> default:
>>>>> return RESUME_HOST;
>>>>> }
>>>>> @@ -958,6 +982,20 @@ struct kvm_vcpu =
*kvmppc_core_vcpu_create(struct kvm *kvm, unsigned int id)
>>>>> vcpu->arch.cpu_type =3D KVM_CPU_3S_64;
>>>>> kvmppc_sanity_check(vcpu);
>>>>>=20
>>>>> + /*
>>>>> + * As we want to minimize the chance of having =
H_PUT_TCE_INDIRECT
>>>>> + * half executed, we first read TCEs from the user, check them =
and
>>>>> + * return error if something went wrong and only then put TCEs =
into
>>>>> + * the TCE table.
>>>>> + *
>>>>> + * tce_tmp is a cache for TCEs to avoid stack allocation or
>>>>> + * kmalloc as the whole TCE list can take up to 512 items 8 =
bytes
>>>>> + * each (4096 bytes).
>>>>> + */
>>>>> + vcpu->arch.tce_tmp =3D kmalloc(4096, GFP_KERNEL);
>>>>> + if (!vcpu->arch.tce_tmp)
>>>>> + goto free_vcpu;
>>>>> +
>>>>> return vcpu;
>>>>>=20
>>>>> free_vcpu:
>>>>> @@ -980,6 +1018,7 @@ void kvmppc_core_vcpu_free(struct kvm_vcpu =
*vcpu)
>>>>> unpin_vpa(vcpu->kvm, &vcpu->arch.slb_shadow);
>>>>> unpin_vpa(vcpu->kvm, &vcpu->arch.vpa);
>>>>> spin_unlock(&vcpu->arch.vpa_update_lock);
>>>>> + kfree(vcpu->arch.tce_tmp);
>>>>> kvm_vcpu_uninit(vcpu);
>>>>> kmem_cache_free(kvm_vcpu_cache, vcpu);
>>>>> }
>>>>> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S =
b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
>>>>> index b02f91e..d35554e 100644
>>>>> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
>>>>> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
>>>>> @@ -1490,6 +1490,12 @@ hcall_real_table:
>>>>> .long 0 /* 0x11c */
>>>>> .long 0 /* 0x120 */
>>>>> .long .kvmppc_h_bulk_remove - hcall_real_table
>>>>> + .long 0 /* 0x128 */
>>>>> + .long 0 /* 0x12c */
>>>>> + .long 0 /* 0x130 */
>>>>> + .long 0 /* 0x134 */
>>>>> + .long .kvmppc_h_stuff_tce - hcall_real_table
>>>>> + .long .kvmppc_h_put_tce_indirect - hcall_real_table
>>>>> hcall_real_table_end:
>>>>>=20
>>>>> ignore_hdec:
>>>>> diff --git a/arch/powerpc/kvm/book3s_pr_papr.c =
b/arch/powerpc/kvm/book3s_pr_papr.c
>>>>> index da0e0bc..91d4b45 100644
>>>>> --- a/arch/powerpc/kvm/book3s_pr_papr.c
>>>>> +++ b/arch/powerpc/kvm/book3s_pr_papr.c
>>>>> @@ -220,7 +220,38 @@ static int kvmppc_h_pr_put_tce(struct =
kvm_vcpu *vcpu)
>>>>> unsigned long tce =3D kvmppc_get_gpr(vcpu, 6);
>>>>> long rc;
>>>>>=20
>>>>> - rc =3D kvmppc_h_put_tce(vcpu, liobn, ioba, tce);
>>>>> + rc =3D kvmppc_virtmode_h_put_tce(vcpu, liobn, ioba, tce);
>>>>> + if (rc =3D=3D H_TOO_HARD)
>>>>> + return EMULATE_FAIL;
>>>>> + kvmppc_set_gpr(vcpu, 3, rc);
>>>>> + return EMULATE_DONE;
>>>>> +}
>>>>> +
>>>>> +static int kvmppc_h_pr_put_tce_indirect(struct kvm_vcpu *vcpu)
>>>>> +{
>>>>> + unsigned long liobn =3D kvmppc_get_gpr(vcpu, 4);
>>>>> + unsigned long ioba =3D kvmppc_get_gpr(vcpu, 5);
>>>>> + unsigned long tce =3D kvmppc_get_gpr(vcpu, 6);
>>>>> + unsigned long npages =3D kvmppc_get_gpr(vcpu, 7);
>>>>> + long rc;
>>>>> +
>>>>> + rc =3D kvmppc_virtmode_h_put_tce_indirect(vcpu, liobn, ioba,
>>>>> + tce, npages);
>>>>> + if (rc =3D=3D H_TOO_HARD)
>>>>> + return EMULATE_FAIL;
>>>>> + kvmppc_set_gpr(vcpu, 3, rc);
>>>>> + return EMULATE_DONE;
>>>>> +}
>>>>> +
>>>>> +static int kvmppc_h_pr_stuff_tce(struct kvm_vcpu *vcpu)
>>>>> +{
>>>>> + unsigned long liobn =3D kvmppc_get_gpr(vcpu, 4);
>>>>> + unsigned long ioba =3D kvmppc_get_gpr(vcpu, 5);
>>>>> + unsigned long tce_value =3D kvmppc_get_gpr(vcpu, 6);
>>>>> + unsigned long npages =3D kvmppc_get_gpr(vcpu, 7);
>>>>> + long rc;
>>>>> +
>>>>> + rc =3D kvmppc_virtmode_h_stuff_tce(vcpu, liobn, ioba, tce_value, =
npages);
>>>>> if (rc =3D=3D H_TOO_HARD)
>>>>> return EMULATE_FAIL;
>>>>> kvmppc_set_gpr(vcpu, 3, rc);
>>>>> @@ -247,6 +278,10 @@ int kvmppc_h_pr(struct kvm_vcpu *vcpu, =
unsigned long cmd)
>>>>> return kvmppc_h_pr_bulk_remove(vcpu);
>>>>> case H_PUT_TCE:
>>>>> return kvmppc_h_pr_put_tce(vcpu);
>>>>> + case H_PUT_TCE_INDIRECT:
>>>>> + return kvmppc_h_pr_put_tce_indirect(vcpu);
>>>>> + case H_STUFF_TCE:
>>>>> + return kvmppc_h_pr_stuff_tce(vcpu);
>>>>> case H_CEDE:
>>>>> vcpu->arch.shared->msr |=3D MSR_EE;
>>>>> kvm_vcpu_block(vcpu);
>>>>> diff --git a/arch/powerpc/kvm/powerpc.c =
b/arch/powerpc/kvm/powerpc.c
>>>>> index 6316ee3..8465c2a 100644
>>>>> --- a/arch/powerpc/kvm/powerpc.c
>>>>> +++ b/arch/powerpc/kvm/powerpc.c
>>>>> @@ -395,6 +395,9 @@ int kvm_dev_ioctl_check_extension(long ext)
>>>>> r =3D 1;
>>>>> break;
>>>>> #endif
>>>>> + case KVM_CAP_SPAPR_MULTITCE:
>>>>> + r =3D 1;
>>>>=20
>>>> This should only be true for book3s.
>>>=20
>>>=20
>>> We had this discussion with v2.
>>>=20
>>> David:
>>> =3D=3D=3D
>>> So, in the case of MULTITCE, that's not quite right. PR KVM can
>>> emulate a PAPR system on a BookE machine, and there's no reason not =
to
>>> allow TCE acceleration as well. We can't make it dependent on PAPR
>>> mode being selected, because that's enabled per-vcpu, whereas these
>>> capabilities are queried on the VM before the vcpus are created.
>>> =3D=3D=3D
>>>=20
>>> Wrong?
>=20
>> Partially. BookE can not emulate a PAPR system as it stands today.
>=20
> Oh.
> Ok.
> So - #ifdef CONFIG_PPC_BOOK3S_64 ? Or run-time check for book3s =
(how...)?
The former.
Alex
^ permalink raw reply
* Re: [PATCH 1/4] KVM: PPC: Add support for multiple-TCE hcalls
From: Benjamin Herrenschmidt @ 2013-06-17 8:37 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: kvm, linux-kernel, kvm-ppc, Alexander Graf, Paul Mackerras,
linuxppc-dev, David Gibson
In-Reply-To: <51BEC0FE.4020805@ozlabs.ru>
On Mon, 2013-06-17 at 17:55 +1000, Alexey Kardashevskiy wrote:
> David:
> ===
> So, in the case of MULTITCE, that's not quite right. PR KVM can
> emulate a PAPR system on a BookE machine, and there's no reason not to
> allow TCE acceleration as well. We can't make it dependent on PAPR
> mode being selected, because that's enabled per-vcpu, whereas these
> capabilities are queried on the VM before the vcpus are created.
> ===
>
> Wrong?
The capability just tells qemu the kernel supports it, it doesn't have
to depend on PAPR mode, qemu can sort things out no ?
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 1/4] KVM: PPC: Add support for multiple-TCE hcalls
From: Alexey Kardashevskiy @ 2013-06-17 8:34 UTC (permalink / raw)
To: Alexander Graf
Cc: kvm, linux-kernel, kvm-ppc, Paul Mackerras, linuxppc-dev,
David Gibson
In-Reply-To: <F3AAE42F-F325-462B-AD22-F3E568C39A8C@suse.de>
On 06/17/2013 06:02 PM, Alexander Graf wrote:
>
> On 17.06.2013, at 09:55, Alexey Kardashevskiy wrote:
>
>> On 06/17/2013 08:06 AM, Alexander Graf wrote:
>>>
>>> On 05.06.2013, at 08:11, Alexey Kardashevskiy wrote:
>>>
>>>> This adds real mode handlers for the H_PUT_TCE_INDIRECT and
>>>> H_STUFF_TCE hypercalls for QEMU emulated devices such as IBMVIO
>>>> devices or emulated PCI. These calls allow adding multiple entries
>>>> (up to 512) into the TCE table in one call which saves time on
>>>> transition to/from real mode.
>>>>
>>>> This adds a tce_tmp cache to kvm_vcpu_arch to save valid TCEs
>>>> (copied from user and verified) before writing the whole list into
>>>> the TCE table. This cache will be utilized more in the upcoming
>>>> VFIO/IOMMU support to continue TCE list processing in the virtual
>>>> mode in the case if the real mode handler failed for some reason.
>>>>
>>>> This adds a guest physical to host real address converter
>>>> and calls the existing H_PUT_TCE handler. The converting function
>>>> is going to be fully utilized by upcoming VFIO supporting patches.
>>>>
>>>> This also implements the KVM_CAP_PPC_MULTITCE capability,
>>>> so in order to support the functionality of this patch, QEMU
>>>> needs to query for this capability and set the "hcall-multi-tce"
>>>> hypertas property only if the capability is present, otherwise
>>>> there will be serious performance degradation.
>>>>
>>>> Cc: David Gibson <david@gibson.dropbear.id.au>
>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>> Signed-off-by: Paul Mackerras <paulus@samba.org>
>>>
>>> Only a few minor nits. Ben already commented on implementation details.
>>>
>>>>
>>>> ---
>>>> Changelog:
>>>> 2013/06/05:
>>>> * fixed mistype about IBMVIO in the commit message
>>>> * updated doc and moved it to another section
>>>> * changed capability number
>>>>
>>>> 2013/05/21:
>>>> * added kvm_vcpu_arch::tce_tmp
>>>> * removed cleanup if put_indirect failed, instead we do not even start
>>>> writing to TCE table if we cannot get TCEs from the user and they are
>>>> invalid
>>>> * kvmppc_emulated_h_put_tce is split to kvmppc_emulated_put_tce
>>>> and kvmppc_emulated_validate_tce (for the previous item)
>>>> * fixed bug with failthrough for H_IPI
>>>> * removed all get_user() from real mode handlers
>>>> * kvmppc_lookup_pte() added (instead of making lookup_linux_pte public)
>>>> ---
>>>> Documentation/virtual/kvm/api.txt | 17 ++
>>>> arch/powerpc/include/asm/kvm_host.h | 2 +
>>>> arch/powerpc/include/asm/kvm_ppc.h | 16 +-
>>>> arch/powerpc/kvm/book3s_64_vio.c | 118 ++++++++++++++
>>>> arch/powerpc/kvm/book3s_64_vio_hv.c | 266 +++++++++++++++++++++++++++----
>>>> arch/powerpc/kvm/book3s_hv.c | 39 +++++
>>>> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 6 +
>>>> arch/powerpc/kvm/book3s_pr_papr.c | 37 ++++-
>>>> arch/powerpc/kvm/powerpc.c | 3 +
>>>> include/uapi/linux/kvm.h | 1 +
>>>> 10 files changed, 473 insertions(+), 32 deletions(-)
>>>>
>>>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>>>> index 5f91eda..6c082ff 100644
>>>> --- a/Documentation/virtual/kvm/api.txt
>>>> +++ b/Documentation/virtual/kvm/api.txt
>>>> @@ -2362,6 +2362,23 @@ calls by the guest for that service will be passed to userspace to be
>>>> handled.
>>>>
>>>>
>>>> +4.83 KVM_CAP_PPC_MULTITCE
>>>> +
>>>> +Capability: KVM_CAP_PPC_MULTITCE
>>>> +Architectures: ppc
>>>> +Type: vm
>>>> +
>>>> +This capability tells the guest that multiple TCE entry add/remove hypercalls
>>>> +handling is supported by the kernel. This significanly accelerates DMA
>>>> +operations for PPC KVM guests.
>>>> +
>>>> +Unlike other capabilities in this section, this one does not have an ioctl.
>>>> +Instead, when the capability is present, the H_PUT_TCE_INDIRECT and
>>>> +H_STUFF_TCE hypercalls are to be handled in the host kernel and not passed to
>>>> +the guest. Othwerwise it might be better for the guest to continue using H_PUT_TCE
>>>> +hypercall (if KVM_CAP_SPAPR_TCE or KVM_CAP_SPAPR_TCE_IOMMU are present).
>>>
>>
>>> While this describes perfectly well what the consequences are of the
>>> patches, it does not describe properly what the CAP actually expresses.
>>> The CAP only says "this kernel is able to handle H_PUT_TCE_INDIRECT and
>>> H_STUFF_TCE hypercalls directly". All other consequences are nice to
>>> document, but the semantics of the CAP are missing.
>>
>>
>> ? It expresses ability to handle 2 hcalls. What is missing?
>
> You don't describe the kvm <-> qemu interface. You describe some decisions qemu can take from this cap.
This file does not mention qemu at all. And the interface is - qemu (or
kvmtool could do that) just adds "hcall-multi-tce" to
"ibm,hypertas-functions" but this is for pseries linux and AIX could always
do it (no idea about it). Does it really have to be in this file?
>>> We also usually try to keep KVM behavior unchanged with regards to older
>>> versions until a CAP is enabled. In this case I don't think it matters
>>> all that much, so I'm fine with declaring it as enabled by default.
>>> Please document that this is a change in behavior versus older KVM
>>> versions though.
>>
>>
>> Ok!
>>
>>
>>>> +
>>>> +
>>>> 5. The kvm_run structure
>>>> ------------------------
>>>>
>>>> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
>>>> index af326cd..85d8f26 100644
>>>> --- a/arch/powerpc/include/asm/kvm_host.h
>>>> +++ b/arch/powerpc/include/asm/kvm_host.h
>>>> @@ -609,6 +609,8 @@ struct kvm_vcpu_arch {
>>>> spinlock_t tbacct_lock;
>>>> u64 busy_stolen;
>>>> u64 busy_preempt;
>>>> +
>>>> + unsigned long *tce_tmp; /* TCE cache for TCE_PUT_INDIRECT hall */
>>>> #endif
>>>> };
>>>
>>> [...]
>>>>
>>>>
>>>
>>> [...]
>>>
>>>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>>>> index 550f592..a39039a 100644
>>>> --- a/arch/powerpc/kvm/book3s_hv.c
>>>> +++ b/arch/powerpc/kvm/book3s_hv.c
>>>> @@ -568,6 +568,30 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
>>>> ret = kvmppc_xics_hcall(vcpu, req);
>>>> break;
>>>> } /* fallthrough */
>>>
>>> The fallthrough comment isn't accurate anymore.
>>>
>>>> + return RESUME_HOST;
>>>> + case H_PUT_TCE:
>>>> + ret = kvmppc_virtmode_h_put_tce(vcpu, kvmppc_get_gpr(vcpu, 4),
>>>> + kvmppc_get_gpr(vcpu, 5),
>>>> + kvmppc_get_gpr(vcpu, 6));
>>>> + if (ret == H_TOO_HARD)
>>>> + return RESUME_HOST;
>>>> + break;
>>>> + case H_PUT_TCE_INDIRECT:
>>>> + ret = kvmppc_virtmode_h_put_tce_indirect(vcpu, kvmppc_get_gpr(vcpu, 4),
>>>> + kvmppc_get_gpr(vcpu, 5),
>>>> + kvmppc_get_gpr(vcpu, 6),
>>>> + kvmppc_get_gpr(vcpu, 7));
>>>> + if (ret == H_TOO_HARD)
>>>> + return RESUME_HOST;
>>>> + break;
>>>> + case H_STUFF_TCE:
>>>> + ret = kvmppc_virtmode_h_stuff_tce(vcpu, kvmppc_get_gpr(vcpu, 4),
>>>> + kvmppc_get_gpr(vcpu, 5),
>>>> + kvmppc_get_gpr(vcpu, 6),
>>>> + kvmppc_get_gpr(vcpu, 7));
>>>> + if (ret == H_TOO_HARD)
>>>> + return RESUME_HOST;
>>>> + break;
>>>> default:
>>>> return RESUME_HOST;
>>>> }
>>>> @@ -958,6 +982,20 @@ struct kvm_vcpu *kvmppc_core_vcpu_create(struct kvm *kvm, unsigned int id)
>>>> vcpu->arch.cpu_type = KVM_CPU_3S_64;
>>>> kvmppc_sanity_check(vcpu);
>>>>
>>>> + /*
>>>> + * As we want to minimize the chance of having H_PUT_TCE_INDIRECT
>>>> + * half executed, we first read TCEs from the user, check them and
>>>> + * return error if something went wrong and only then put TCEs into
>>>> + * the TCE table.
>>>> + *
>>>> + * tce_tmp is a cache for TCEs to avoid stack allocation or
>>>> + * kmalloc as the whole TCE list can take up to 512 items 8 bytes
>>>> + * each (4096 bytes).
>>>> + */
>>>> + vcpu->arch.tce_tmp = kmalloc(4096, GFP_KERNEL);
>>>> + if (!vcpu->arch.tce_tmp)
>>>> + goto free_vcpu;
>>>> +
>>>> return vcpu;
>>>>
>>>> free_vcpu:
>>>> @@ -980,6 +1018,7 @@ void kvmppc_core_vcpu_free(struct kvm_vcpu *vcpu)
>>>> unpin_vpa(vcpu->kvm, &vcpu->arch.slb_shadow);
>>>> unpin_vpa(vcpu->kvm, &vcpu->arch.vpa);
>>>> spin_unlock(&vcpu->arch.vpa_update_lock);
>>>> + kfree(vcpu->arch.tce_tmp);
>>>> kvm_vcpu_uninit(vcpu);
>>>> kmem_cache_free(kvm_vcpu_cache, vcpu);
>>>> }
>>>> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
>>>> index b02f91e..d35554e 100644
>>>> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
>>>> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
>>>> @@ -1490,6 +1490,12 @@ hcall_real_table:
>>>> .long 0 /* 0x11c */
>>>> .long 0 /* 0x120 */
>>>> .long .kvmppc_h_bulk_remove - hcall_real_table
>>>> + .long 0 /* 0x128 */
>>>> + .long 0 /* 0x12c */
>>>> + .long 0 /* 0x130 */
>>>> + .long 0 /* 0x134 */
>>>> + .long .kvmppc_h_stuff_tce - hcall_real_table
>>>> + .long .kvmppc_h_put_tce_indirect - hcall_real_table
>>>> hcall_real_table_end:
>>>>
>>>> ignore_hdec:
>>>> diff --git a/arch/powerpc/kvm/book3s_pr_papr.c b/arch/powerpc/kvm/book3s_pr_papr.c
>>>> index da0e0bc..91d4b45 100644
>>>> --- a/arch/powerpc/kvm/book3s_pr_papr.c
>>>> +++ b/arch/powerpc/kvm/book3s_pr_papr.c
>>>> @@ -220,7 +220,38 @@ static int kvmppc_h_pr_put_tce(struct kvm_vcpu *vcpu)
>>>> unsigned long tce = kvmppc_get_gpr(vcpu, 6);
>>>> long rc;
>>>>
>>>> - rc = kvmppc_h_put_tce(vcpu, liobn, ioba, tce);
>>>> + rc = kvmppc_virtmode_h_put_tce(vcpu, liobn, ioba, tce);
>>>> + if (rc == H_TOO_HARD)
>>>> + return EMULATE_FAIL;
>>>> + kvmppc_set_gpr(vcpu, 3, rc);
>>>> + return EMULATE_DONE;
>>>> +}
>>>> +
>>>> +static int kvmppc_h_pr_put_tce_indirect(struct kvm_vcpu *vcpu)
>>>> +{
>>>> + unsigned long liobn = kvmppc_get_gpr(vcpu, 4);
>>>> + unsigned long ioba = kvmppc_get_gpr(vcpu, 5);
>>>> + unsigned long tce = kvmppc_get_gpr(vcpu, 6);
>>>> + unsigned long npages = kvmppc_get_gpr(vcpu, 7);
>>>> + long rc;
>>>> +
>>>> + rc = kvmppc_virtmode_h_put_tce_indirect(vcpu, liobn, ioba,
>>>> + tce, npages);
>>>> + if (rc == H_TOO_HARD)
>>>> + return EMULATE_FAIL;
>>>> + kvmppc_set_gpr(vcpu, 3, rc);
>>>> + return EMULATE_DONE;
>>>> +}
>>>> +
>>>> +static int kvmppc_h_pr_stuff_tce(struct kvm_vcpu *vcpu)
>>>> +{
>>>> + unsigned long liobn = kvmppc_get_gpr(vcpu, 4);
>>>> + unsigned long ioba = kvmppc_get_gpr(vcpu, 5);
>>>> + unsigned long tce_value = kvmppc_get_gpr(vcpu, 6);
>>>> + unsigned long npages = kvmppc_get_gpr(vcpu, 7);
>>>> + long rc;
>>>> +
>>>> + rc = kvmppc_virtmode_h_stuff_tce(vcpu, liobn, ioba, tce_value, npages);
>>>> if (rc == H_TOO_HARD)
>>>> return EMULATE_FAIL;
>>>> kvmppc_set_gpr(vcpu, 3, rc);
>>>> @@ -247,6 +278,10 @@ int kvmppc_h_pr(struct kvm_vcpu *vcpu, unsigned long cmd)
>>>> return kvmppc_h_pr_bulk_remove(vcpu);
>>>> case H_PUT_TCE:
>>>> return kvmppc_h_pr_put_tce(vcpu);
>>>> + case H_PUT_TCE_INDIRECT:
>>>> + return kvmppc_h_pr_put_tce_indirect(vcpu);
>>>> + case H_STUFF_TCE:
>>>> + return kvmppc_h_pr_stuff_tce(vcpu);
>>>> case H_CEDE:
>>>> vcpu->arch.shared->msr |= MSR_EE;
>>>> kvm_vcpu_block(vcpu);
>>>> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
>>>> index 6316ee3..8465c2a 100644
>>>> --- a/arch/powerpc/kvm/powerpc.c
>>>> +++ b/arch/powerpc/kvm/powerpc.c
>>>> @@ -395,6 +395,9 @@ int kvm_dev_ioctl_check_extension(long ext)
>>>> r = 1;
>>>> break;
>>>> #endif
>>>> + case KVM_CAP_SPAPR_MULTITCE:
>>>> + r = 1;
>>>
>>> This should only be true for book3s.
>>
>>
>> We had this discussion with v2.
>>
>> David:
>> ===
>> So, in the case of MULTITCE, that's not quite right. PR KVM can
>> emulate a PAPR system on a BookE machine, and there's no reason not to
>> allow TCE acceleration as well. We can't make it dependent on PAPR
>> mode being selected, because that's enabled per-vcpu, whereas these
>> capabilities are queried on the VM before the vcpus are created.
>> ===
>>
>> Wrong?
> Partially. BookE can not emulate a PAPR system as it stands today.
Oh.
Ok.
So - #ifdef CONFIG_PPC_BOOK3S_64 ? Or run-time check for book3s (how...)?
--
Alexey
^ 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