* Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
From: Scott Wood @ 2014-07-03 23:00 UTC (permalink / raw)
To: Alexander Graf
Cc: Caraman Mihai Claudiu-B02008, linuxppc-dev@lists.ozlabs.org,
kvm@vger.kernel.org, kvm-ppc@vger.kernel.org
In-Reply-To: <53B5DAC6.6050403@suse.de>
On Fri, 2014-07-04 at 00:35 +0200, Alexander Graf wrote:
> On 04.07.14 00:31, Scott Wood wrote:
> > On Thu, 2014-07-03 at 17:15 -0500, Scott Wood wrote:
> >> On Thu, 2014-07-03 at 10:25 -0500, Caraman Mihai Claudiu-B02008 wrote:
> >>>> -----Original Message-----
> >>>> From: Alexander Graf [mailto:agraf@suse.de]
> >>>> Sent: Thursday, July 03, 2014 3:21 PM
> >>>> To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org
> >>>> Cc: kvm@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
> >>>> Subject: Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for
> >>>> SPE/FP/AltiVec int numbers
> >>>>
> >>>>
> >>>> On 30.06.14 17:34, Mihai Caraman wrote:
> >>>>> Use common BOOKE_IRQPRIO and BOOKE_INTERRUPT defines for SPE/FP/AltiVec
> >>>>> which share the same interrupt numbers.
> >>>>>
> >>>>> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> >>>>> ---
> >>>>> v2:
> >>>>> - remove outdated definitions
> >>>>>
> >>>>> arch/powerpc/include/asm/kvm_asm.h | 8 --------
> >>>>> arch/powerpc/kvm/booke.c | 17 +++++++++--------
> >>>>> arch/powerpc/kvm/booke.h | 4 ++--
> >>>>> arch/powerpc/kvm/booke_interrupts.S | 9 +++++----
> >>>>> arch/powerpc/kvm/bookehv_interrupts.S | 4 ++--
> >>>>> arch/powerpc/kvm/e500.c | 10 ++++++----
> >>>>> arch/powerpc/kvm/e500_emulate.c | 10 ++++++----
> >>>>> 7 files changed, 30 insertions(+), 32 deletions(-)
> >>>>>
> >>>>> diff --git a/arch/powerpc/include/asm/kvm_asm.h
> >>>> b/arch/powerpc/include/asm/kvm_asm.h
> >>>>> index 9601741..c94fd33 100644
> >>>>> --- a/arch/powerpc/include/asm/kvm_asm.h
> >>>>> +++ b/arch/powerpc/include/asm/kvm_asm.h
> >>>>> @@ -56,14 +56,6 @@
> >>>>> /* E500 */
> >>>>> #define BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL 32
> >>>>> #define BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST 33
> >>>>> -/*
> >>>>> - * TODO: Unify 32-bit and 64-bit kernel exception handlers to use same
> >>>> defines
> >>>>> - */
> >>>>> -#define BOOKE_INTERRUPT_SPE_UNAVAIL
> >>>> BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL
> >>>>> -#define BOOKE_INTERRUPT_SPE_FP_DATA
> >>>> BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST
> >>>>> -#define BOOKE_INTERRUPT_ALTIVEC_UNAVAIL
> >>>> BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL
> >>>>> -#define BOOKE_INTERRUPT_ALTIVEC_ASSIST \
> >>>>> - BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST
> >>>> I think I'd prefer to keep them separate.
> >>> What is the reason from changing your mind from ver 1? Do you want to have
> >>> different defines with same values (we specifically mapped them to the
> >>> hardware interrupt numbers). We already upstreamed the necessary changes
> >>> in the kernel. Scott, please share your opinion here.
> >> I don't like hiding the fact that they're the same number, which could
> >> lead to wrong code in the absence of ifdefs that strictly mutually
> >> exclude SPE and Altivec code -- there was an instance of this with
> >> MSR_VEC versus MSR_SPE in a previous patchset.
> > That said, if you want to enforce that mutual exclusion in a way that is
> > clear, I won't object too loudly -- but the code does look pretty
> > similar between the two (as well as between the two IVORs).
>
> Yes, I want to make sure we have 2 separate code paths for SPE and
> Altivec. No code sharing at all unless it's very generically possible.
>
> Also, which code does look pretty similar? The fact that we deflect
> interrupts back into the guest? That's mostly boilerplate.
There's also the injection of a program check (or exiting to userspace)
when CONFIG_SPE/ALTIVEC is missing. Not a big deal, but maybe it could
be factored into a helper function. I like minimizing boilerplate.
-Scott
^ permalink raw reply
* [PATCH] Use the POWER8 Micro Partition Prefetch Engine in KVM HV on POWER8
From: Stewart Smith @ 2014-07-04 1:23 UTC (permalink / raw)
To: linuxppc-dev, paulus; +Cc: Stewart Smith
The POWER8 processor has a Micro Partition Prefetch Engine, which is
a fancy way of saying "has way to store and load contents of L2 or
L2+MRU way of L3 cache". We initiate the storing of the log (list of
addresses) using the logmpp instruction and start restore by writing
to a SPR.
The logmpp instruction takes parameters in a single 64bit register:
- starting address of the table to store log of L2/L2+L3 cache contents
- 32kb for L2
- 128kb for L2+L3
- Aligned relative to maximum size of the table (32kb or 128kb)
- Log control (no-op, L2 only, L2 and L3, abort logout)
We should abort any ongoing logging before initiating one.
To initiate restore, we write to the MPPR SPR. The format of what to write
to the SPR is similar to the logmpp instruction parameter:
- starting address of the table to read from (same alignment requirements)
- table size (no data, until end of table)
- prefetch rate (from fastest possible to slower. about every 8, 16, 24 or
32 cycles)
The idea behind loading and storing the contents of L2/L3 cache is to
reduce memory latency in a system that is frequently swapping vcores on
a physical CPU.
The best case scenario for doing this is when some vcores are doing very
cache heavy workloads. The worst case is when they have about 0 cache hits,
so we just generate needless memory operations.
This implementation just does L2 store/load. In my benchmarks this proves
to be useful.
Benchmark 1:
- 16 core POWER8
- 3x Ubuntu 14.04LTS guests (LE) with 8 VCPUs each
- No split core/SMT
- two guests running sysbench memory test.
sysbench --test=memory --num-threads=8 run
- one guest running apache bench (of default HTML page)
ab -n 490000 -c 400 http://localhost/
This benchmark aims to measure performance of real world application (apache)
where other guests are cache hot with their own workloads. The sysbench memory
benchmark does pointer sized writes to a (small) memory buffer in a loop.
In this benchmark with this patch I can see an improvement both in requests
per second (~5%) and in mean and median response times (again, about 5%).
The spread of minimum and maximum response times were largely unchanged.
benchmark 2:
- Same VM config as benchmark 1
- all three guests running sysbench memory benchmark
This benchmark aims to see if there is a positive or negative affect to this
cache heavy benchmark. Although due to the nature of the benchmark (stores) we
may not see a difference in performance, but rather hopefully an improvement
in consistency of performance (when vcore switched in, don't have to wait
many times for cachelines to be pulled in)
The results of this benchmark are improvements in consistency of performance
rather than performance itself. With this patch, the few outliers in duration
go away and we get more consistent performance in each guest.
benchmark 3:
- same 3 guests and CPU configuration as benchmark 1 and 2.
- two idle guests
- 1 guest running STREAM benchmark
This scenario also saw performance improvement with this patch. On Copy and
Scale workloads from STREAM, I got 5-6% improvement with this patch. For
Add and triad, it was around 10% (or more).
benchmark 4:
- same 3 guests as previous benchmarks
- two guests running sysbench --memory, distinctly different cache heavy
workload
- one guest running STREAM benchmark.
Similar improvements to benchmark 3.
benchmark 5:
- 1 guest, 8 VCPUs, Ubuntu 14.04
- Host configured with split core (SMT8, subcores-per-core=4)
- STREAM benchmark
In this benchmark, we see a 10-20% performance improvement across the board
of STREAM benchmark results with this patch.
Based on preliminary investigation and microbenchmarks
by Prerna Saxena <prerna@linux.vnet.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/kvm_host.h | 1 +
arch/powerpc/include/asm/ppc-opcode.h | 10 +++++++
arch/powerpc/include/asm/reg.h | 1 +
arch/powerpc/kvm/book3s_hv.c | 53 ++++++++++++++++++++++++++++++++-
4 files changed, 64 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 1eaea2d..5c0e9fc 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -305,6 +305,7 @@ struct kvmppc_vcore {
u32 arch_compat;
ulong pcr;
ulong dpdes; /* doorbell state (POWER8) */
+ unsigned long mppe; /* Micro Partition Prefetch buffer */
};
#define VCORE_ENTRY_COUNT(vc) ((vc)->entry_exit_count & 0xff)
diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index 3132bb9..6201440 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -139,6 +139,7 @@
#define PPC_INST_ISEL 0x7c00001e
#define PPC_INST_ISEL_MASK 0xfc00003e
#define PPC_INST_LDARX 0x7c0000a8
+#define PPC_INST_LOGMPP 0x7c0007e4
#define PPC_INST_LSWI 0x7c0004aa
#define PPC_INST_LSWX 0x7c00042a
#define PPC_INST_LWARX 0x7c000028
@@ -275,6 +276,13 @@
#define __PPC_EH(eh) 0
#endif
+/* POWER8 Micro Partition Prefetch parameters */
+#define PPC_MPPE_ADDRESS_MASK 0xffffffffc000
+#define PPC_MPPE_WHOLE_TABLE (0x2ULL << 60)
+#define PPC_MPPE_LOG_L2 (0x02ULL << 54)
+#define PPC_MPPE_LOG_L2L3 (0x01ULL << 54)
+#define PPC_MPPE_LOG_ABORT (0x03ULL << 54)
+
/* Deal with instructions that older assemblers aren't aware of */
#define PPC_DCBAL(a, b) stringify_in_c(.long PPC_INST_DCBAL | \
__PPC_RA(a) | __PPC_RB(b))
@@ -283,6 +291,8 @@
#define PPC_LDARX(t, a, b, eh) stringify_in_c(.long PPC_INST_LDARX | \
___PPC_RT(t) | ___PPC_RA(a) | \
___PPC_RB(b) | __PPC_EH(eh))
+#define PPC_LOGMPP(b) stringify_in_c(.long PPC_INST_LOGMPP | \
+ __PPC_RB(b))
#define PPC_LWARX(t, a, b, eh) stringify_in_c(.long PPC_INST_LWARX | \
___PPC_RT(t) | ___PPC_RA(a) | \
___PPC_RB(b) | __PPC_EH(eh))
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index e5d2e0b..5164beb 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -224,6 +224,7 @@
#define CTRL_TE 0x00c00000 /* thread enable */
#define CTRL_RUNLATCH 0x1
#define SPRN_DAWR 0xB4
+#define SPRN_MPPR 0xB8 /* Micro Partition Prefetch Register */
#define SPRN_CIABR 0xBB
#define CIABR_PRIV 0x3
#define CIABR_PRIV_USER 1
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 8227dba..d19906e 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1528,6 +1528,7 @@ static void kvmppc_run_core(struct kvmppc_vcore *vc)
int i, need_vpa_update;
int srcu_idx;
struct kvm_vcpu *vcpus_to_update[threads_per_core];
+ phys_addr_t phy_addr, tmp;
/* don't start if any threads have a signal pending */
need_vpa_update = 0;
@@ -1590,9 +1591,51 @@ static void kvmppc_run_core(struct kvmppc_vcore *vc)
srcu_idx = srcu_read_lock(&vc->kvm->srcu);
+ /* If we have a saved list of L2/L3, restore it */
+ if (cpu_has_feature(CPU_FTR_ARCH_207S) && vc->mppe) {
+ phy_addr = virt_to_phys((void *)vc->mppe);
+#if defined(CONFIG_PPC_4K_PAGES)
+ phy_addr = (phy_addr + 8*4096) & ~(8*4096);
+#endif
+ tmp = phy_addr & PPC_MPPE_ADDRESS_MASK;
+ tmp = tmp | PPC_MPPE_WHOLE_TABLE;
+
+ /* For sanity, abort any 'save' requests in progress */
+ asm volatile(PPC_LOGMPP(R1) : : "r" (tmp));
+
+ /* Inititate a cache-load request */
+ mtspr(SPRN_MPPR, tmp);
+ }
+
+ /* Allocate memory before switching out of guest so we don't
+ trash L2/L3 with memory allocation stuff */
+ if (cpu_has_feature(CPU_FTR_ARCH_207S) && !vc->mppe) {
+#if defined(CONFIG_PPC_64K_PAGES)
+ vc->mppe = __get_free_pages(GFP_KERNEL|__GFP_ZERO, 0);
+#elif defined(CONFIG_PPC_4K_PAGES)
+ vc->mppe = __get_free_pages(GFP_KERNEL|__GFP_ZERO, 4);
+#endif
+ }
+
__kvmppc_vcore_entry();
spin_lock(&vc->lock);
+
+ if (cpu_has_feature(CPU_FTR_ARCH_207S) && vc->mppe) {
+ phy_addr = (phys_addr_t)virt_to_phys((void *)vc->mppe);
+#if defined(CONFIG_PPC_4K_PAGES)
+ phy_addr = (phy_addr + 8*4096) & ~(8*4096);
+#endif
+ tmp = PPC_MPPE_ADDRESS_MASK & phy_addr;
+ tmp = tmp | PPC_MPPE_LOG_L2;
+
+ /* Abort any existing 'fetch' operations for this core */
+ mtspr(SPRN_MPPR, tmp&0x0fffffffffffffff);
+
+ /* Finally, issue logmpp to save cache contents for L2 */
+ asm volatile(PPC_LOGMPP(R1) : : "r" (tmp));
+ }
+
/* disable sending of IPIs on virtual external irqs */
list_for_each_entry(vcpu, &vc->runnable_threads, arch.run_list)
vcpu->cpu = -1;
@@ -2329,8 +2372,16 @@ static void kvmppc_free_vcores(struct kvm *kvm)
{
long int i;
- for (i = 0; i < KVM_MAX_VCORES; ++i)
+ for (i = 0; i < KVM_MAX_VCORES; ++i) {
+ if (kvm->arch.vcores[i] && kvm->arch.vcores[i]->mppe) {
+#if defined(CONFIG_PPC_64K_PAGES)
+ free_pages(kvm->arch.vcores[i]->mppe, 0);
+#elif defined(CONFIG_PPC_4K_PAGES)
+ free_pages(kvm->arch.vcores[i]->mppe, 4);
+#endif
+ }
kfree(kvm->arch.vcores[i]);
+ }
kvm->arch.online_vcores = 0;
}
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH 1/3] powerpc/kvm: Remove redundant save of SIER AND MMCR2
From: Michael Ellerman @ 2014-07-04 2:16 UTC (permalink / raw)
To: Joel Stanley; +Cc: paulus, linuxppc-dev
In-Reply-To: <1404367956-19515-2-git-send-email-joel@jms.id.au>
On Thu, 2014-07-03 at 15:42 +0930, Joel Stanley wrote:
> These two registers are already saved in the block above. Aside from
> being unnecessary, by the time we get down to the second save location
> r8 no longer contains MMCR2, so we are clobbering the saved value with
> PMC5.
MMCR2 primarily consists of counter freeze bits. So restoring the value of PMC5
into MMCR2 will most likely have the effect of freezing counters.
Fixes: 72cde5a88d37 ("KVM: PPC: Book3S HV: Save/restore host PMU registers that are new in POWER8")
Cc: stable@vger.kernel.org
With the above added:
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
cheers
^ permalink raw reply
* Re: [PATCH 2/3] powerpc/perf: Add PPMU_ARCH_207S define
From: Michael Ellerman @ 2014-07-04 2:34 UTC (permalink / raw)
To: Joel Stanley; +Cc: paulus, linuxppc-dev
In-Reply-To: <1404367956-19515-3-git-send-email-joel@jms.id.au>
On Thu, 2014-07-03 at 15:42 +0930, Joel Stanley wrote:
> Instead of separate bits for every POWER8 PMU feature, have a single one
> for v2.07 of the architecture.
>
> This saves us adding a MMCR2 define for a future patch.
Cc: stable@vger.kernel.org
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
cheers
^ permalink raw reply
* Re: [PATCH 3/3] powerpc/perf: Clear MMCR2 when enabling PMU
From: Michael Ellerman @ 2014-07-04 2:36 UTC (permalink / raw)
To: Joel Stanley; +Cc: paulus, linuxppc-dev
In-Reply-To: <1404367956-19515-4-git-send-email-joel@jms.id.au>
On Thu, 2014-07-03 at 15:42 +0930, Joel Stanley wrote:
> On POWER8 when switching to a KVM guest we set bits in MMCR2 to freeze
> the PMU counters. Aside from on boot they are then never reset,
> resulting in stuck perf counters for any user in the guest or host.
Cc: stable@vger.kernel.org
Fixes: e05b9b9e5c10 ("powerpc/perf: Power8 PMU support")
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
cheers
^ permalink raw reply
* RE: [PATCH] devicetree/bindings: Add binding for micron n25q512a memory
From: Priyanka Jain @ 2014-07-04 4:08 UTC (permalink / raw)
To: Scott Wood
Cc: devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
linux-mtd@lists.infradead.org, linux-spi@vger.kernel.org
In-Reply-To: <1404425400.21434.89.camel@snotra.buserror.net>
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogV29vZCBTY290dC1CMDc0
MjENCj4gU2VudDogRnJpZGF5LCBKdWx5IDA0LCAyMDE0IDM6NDAgQU0NCj4gVG86IEphaW4gUHJp
eWFua2EtQjMyMTY3DQo+IENjOiBkZXZpY2V0cmVlQHZnZXIua2VybmVsLm9yZzsgbGludXhwcGMt
ZGV2QGxpc3RzLm96bGFicy5vcmc7IGxpbnV4LQ0KPiBzcGlAdmdlci5rZXJuZWwub3JnOyBsaW51
eC1tdGRAbGlzdHMuaW5mcmFkZWFkLm9yZw0KPiBTdWJqZWN0OiBSZTogW1BBVENIXSBkZXZpY2V0
cmVlL2JpbmRpbmdzOiBBZGQgYmluZGluZyBmb3IgbWljcm9uIG4yNXE1MTJhDQo+IG1lbW9yeQ0K
PiANCj4gT24gVGh1LCAyMDE0LTA3LTAzIGF0IDE1OjQyICswNTMwLCBQcml5YW5rYSBKYWluIHdy
b3RlOg0KPiA+IC1NaWNyb24gbjI1cTUxMmEgbWVtb3J5IGlzIHN1cHBvcnRlZCBieSBtMjVwODAg
ZHJpdmVyLg0KPiA+ICBBZGQgY29tcGF0aWJsZSBmaWVsZCByZXF1aXJlZCB0byBzdXBwb3J0IG4y
NXE1MTJhIGluIG0yNXA4MC50eHQgLUFkZA0KPiA+IG1pY3JvbiB0byB0aGUgdmVuZG9yLXByZWZp
eGVzLnR4dCBmaWxlDQo+ID4NCj4gPiBTaWduZWQtb2ZmLWJ5OiBQcml5YW5rYSBKYWluIDxQcml5
YW5rYS5KYWluQGZyZWVzY2FsZS5jb20+DQo+ID4gLS0tDQo+ID4gIERvY3VtZW50YXRpb24vZGV2
aWNldHJlZS9iaW5kaW5ncy9tdGQvbTI1cDgwLnR4dCAgIHwgICAgMSArDQo+ID4gIC4uLi9kZXZp
Y2V0cmVlL2JpbmRpbmdzL3ZlbmRvci1wcmVmaXhlcy50eHQgICAgICAgIHwgICAgMSArDQo+ID4g
IDIgZmlsZXMgY2hhbmdlZCwgMiBpbnNlcnRpb25zKCspLCAwIGRlbGV0aW9ucygtKQ0KPiANCj4g
V2h5IGRpZCB5b3Ugc2VuZCB0aGlzIHRvIHRoZSBwcGMgbGlzdCBidXQgbm90IHRoZSBzcGkgb3Ig
bXRkIGxpc3RzPw0KPiANCltKYWluIFByaXlhbmthLUIzMjE2N10gU29ycnksIEkgbWlzc2VkIHRo
YXQNCj4gSSdtIGhhdmluZyBhIGhhcmQgdGltZSBmb2xsb3dpbmcgdGhlIGZsb3cgb2YgaG93IHRo
ZXNlIFNQSSBkZXZpY2VzIGdldA0KPiBib3VuZCAtLSBpcyB0aGUgY29tcGF0aWJsZSBpbnZvbHZl
ZCBhdCBhbGw/ICBJIGRvbid0IHNlZSB0aGlzIHN0cmluZw0KPiAod2l0aCB2ZW5kb3IgcHJlZml4
IGluY2x1ZGVkKSBpbiB0aGUgZHJpdmVyLiAgSSBkbyBzZWUgYSB0YWJsZSB0aGF0DQo+IGNvbnRh
aW5zIHdoYXQgbG9va3MgbGlrZSBkZXZpY2UgSURzLiAgSWYgdGhlIGRldmljZSBjYW4gcmVwb3J0
IGl0cyBpZCwNCj4gc2hvdWxkbid0IHdlIHJlbHkgb24gdGhhdCByYXRoZXIgdGhhbiBkZXZpY2Ug
dHJlZSBjb21wYXRpYmxlPw0KPiANCj4gLVNjb3R0DQpbSmFpbiBQcml5YW5rYS1CMzIxNjddDQpT
cGkgZHJpdmVyIGhhcyBhIGNoZWNrIHRvIGRldmljZSBuYW1lIGNvcnJlc3BvbmRpbmcgdG8gZGV2
aWNlLWlkIGFuZCBjb21wYXJlIHRvIHdoYXQgaXMgcGFzc2VkIGluIGR0cyBzdHJpbmcuDQpOZXcg
Y2hlY2twYXRjaCBzY3JpcHQgc2VhcmNoIGZvciBleGFjdCBzdHJpbmcgbWF0Y2ggaW4gYmluZGlu
ZyBmb2xkZXIuIFNvLCBJIGhhdmUgYWRkZWQgdGhpcyBpbiBleGFtcGxlLg0KPiANCj4gDQo+ID4g
ZGlmZiAtLWdpdCBhL0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy9tdGQvbTI1cDgw
LnR4dA0KPiA+IGIvRG9jdW1lbnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL210ZC9tMjVwODAu
dHh0DQo+ID4gaW5kZXggNDYxMWFhOC4uY2UwMmU4MSAxMDA2NDQNCj4gPiAtLS0gYS9Eb2N1bWVu
dGF0aW9uL2RldmljZXRyZWUvYmluZGluZ3MvbXRkL20yNXA4MC50eHQNCj4gPiArKysgYi9Eb2N1
bWVudGF0aW9uL2RldmljZXRyZWUvYmluZGluZ3MvbXRkL20yNXA4MC50eHQNCj4gPiBAQCAtNyw2
ICs3LDcgQEAgUmVxdWlyZWQgcHJvcGVydGllczoNCj4gPiAgICAgICAgICAgICAgICAgdGhlIERU
IGJpbmRpbmcgaXMgbm90IExpbnV4LW9ubHksIGJ1dCBpbiBjYXNlIG9mIExpbnV4LA0KPiBzZWUg
dGhlDQo+ID4gICAgICAgICAgICAgICAgICJzcGlfbm9yX2lkcyIgdGFibGUgaW4gZHJpdmVycy9t
dGQvc3BpLW5vci9zcGktbm9yLmMNCj4gZm9yIHRoZSBsaXN0DQo+ID4gICAgICAgICAgICAgICAg
IG9mIHN1cHBvcnRlZCBjaGlwcy4NCj4gPiArICAgICAgICAgICAgICAgZXhhbXBsZTogIm1pY3Jv
bixuMjVxNTEyYSINCj4gPiAgLSByZWcgOiBDaGlwLVNlbGVjdCBudW1iZXINCj4gPiAgLSBzcGkt
bWF4LWZyZXF1ZW5jeSA6IE1heGltdW0gZnJlcXVlbmN5IG9mIHRoZSBTUEkgYnVzIHRoZSBjaGlw
IGNhbg0KPiA+IG9wZXJhdGUgYXQNCj4gPg0KPiA+IGRpZmYgLS1naXQgYS9Eb2N1bWVudGF0aW9u
L2RldmljZXRyZWUvYmluZGluZ3MvdmVuZG9yLXByZWZpeGVzLnR4dA0KPiA+IGIvRG9jdW1lbnRh
dGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL3ZlbmRvci1wcmVmaXhlcy50eHQNCj4gPiBpbmRleCA0
ZDdmMzc1Li5hNDRjZmVlIDEwMDY0NA0KPiA+IC0tLSBhL0RvY3VtZW50YXRpb24vZGV2aWNldHJl
ZS9iaW5kaW5ncy92ZW5kb3ItcHJlZml4ZXMudHh0DQo+ID4gKysrIGIvRG9jdW1lbnRhdGlvbi9k
ZXZpY2V0cmVlL2JpbmRpbmdzL3ZlbmRvci1wcmVmaXhlcy50eHQNCj4gPiBAQCAtNzksNiArNzks
NyBAQCBtYXJ2ZWxsCU1hcnZlbGwgVGVjaG5vbG9neSBHcm91cCBMdGQuDQo+ID4gIG1heGltCU1h
eGltIEludGVncmF0ZWQgUHJvZHVjdHMNCj4gPiAgbWljcmVsCU1pY3JlbCBJbmMuDQo+ID4gIG1p
Y3JvY2hpcAlNaWNyb2NoaXAgVGVjaG5vbG9neSBJbmMuDQo+ID4gK21pY3JvbglNaWNyb24gVGVj
aG5vbG9neSBJbmMuDQo+ID4gIG1vc2FpeHRlY2gJTW9zYWl4IFRlY2hub2xvZ2llcywgSW5jLg0K
PiA+ICBtb3hhCU1veGENCj4gPiAgbXBsCU1QTCBBRw0KPiANCg0K
^ permalink raw reply
* RE: [PATCH] devicetree/binding/powerpc/fsl: Add binding for CPLD
From: Priyanka Jain @ 2014-07-04 4:14 UTC (permalink / raw)
To: Scott Wood; +Cc: devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1404410878.21434.70.camel@snotra.buserror.net>
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogV29vZCBTY290dC1CMDc0
MjENCj4gU2VudDogVGh1cnNkYXksIEp1bHkgMDMsIDIwMTQgMTE6MzggUE0NCj4gVG86IEphaW4g
UHJpeWFua2EtQjMyMTY3DQo+IENjOiBkZXZpY2V0cmVlQHZnZXIua2VybmVsLm9yZzsgbGludXhw
cGMtZGV2QGxpc3RzLm96bGFicy5vcmcNCj4gU3ViamVjdDogUmU6IFtQQVRDSF0gZGV2aWNldHJl
ZS9iaW5kaW5nL3Bvd2VycGMvZnNsOiBBZGQgYmluZGluZyBmb3IgQ1BMRA0KPiANCj4gT24gVGh1
LCAyMDE0LTA3LTAzIGF0IDE1OjM5ICswNTMwLCBQcml5YW5rYSBKYWluIHdyb3RlOg0KPiA+IFNv
bWUgRnJlZXNjYWxlIGJvYXJkcyBsaWtlIFQxMDQwUkRCIGhhdmUgb24gYm9hcmQgQ1BMRCBjb25u
ZWN0ZWQgb24NCj4gPiB0aGUgSUZDIGJ1cy4gQWRkIGJpbmRpbmcgZm9yIHRoaXMgaW4gYm9hcmQu
dHh0IGZpbGUNCj4gPg0KPiA+IFNpZ25lZC1vZmYtYnk6IFByaXlhbmthIEphaW4gPFByaXlhbmth
LkphaW5AZnJlZXNjYWxlLmNvbT4NCj4gPiAtLS0NCj4gPiAgLi4uL2RldmljZXRyZWUvYmluZGlu
Z3MvcG93ZXJwYy9mc2wvYm9hcmQudHh0ICAgICAgfCAgIDE5DQo+ICsrKysrKysrKysrKysrKysr
KysNCj4gPiAgMSBmaWxlcyBjaGFuZ2VkLCAxOSBpbnNlcnRpb25zKCspLCAwIGRlbGV0aW9ucygt
KQ0KPiA+DQo+ID4gZGlmZiAtLWdpdCBhL0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5n
cy9wb3dlcnBjL2ZzbC9ib2FyZC50eHQNCj4gPiBiL0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9i
aW5kaW5ncy9wb3dlcnBjL2ZzbC9ib2FyZC50eHQNCj4gPiBpbmRleCA3MDBkZWM0Li5mMzVmMjk1
IDEwMDY0NA0KPiA+IC0tLSBhL0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy9wb3dl
cnBjL2ZzbC9ib2FyZC50eHQNCj4gPiArKysgYi9Eb2N1bWVudGF0aW9uL2RldmljZXRyZWUvYmlu
ZGluZ3MvcG93ZXJwYy9mc2wvYm9hcmQudHh0DQo+ID4gQEAgLTg0LDMgKzg0LDIyIEBAIEV4YW1w
bGU6DQo+ID4gIAkJY29tcGF0aWJsZSA9ICJmc2wsYnNjOTEzMnFkcy1mcGdhIiwgImZzbCxmcGdh
LXFpeGlzLWkyYyI7DQo+ID4gIAkJcmVnID0gPDB4NjY+Ow0KPiA+ICAJfTsNCj4gPiArDQo+ID4g
KyogRnJlZXNjYWxlIG9uLWJvYXJkIENQTEQNCj4gPiArDQo+ID4gK1NvbWUgRnJlZXNjYWxlIGJv
YXJkcyBsaWtlIFQxMDQwUkRCIGhhdmUgb24gYm9hcmQgQ1BMRCBjb25uZWN0ZWQgb24NCj4gPiAr
dGhlIElGQyBidXMuDQo+ID4gKw0KPiA+ICtSZXF1aXJlZCBwcm9wZXJ0aWVzOg0KPiA+ICstIGNv
bXBhdGlibGU6IFNob3VsZCBiZSBhIGJvYXJkLXNwZWNpZmljIHN0cmluZyBsaWtlICJmc2wsPGJv
YXJkPi0NCj4gY3BsZCINCj4gPiArICBFeGFtcGxlOg0KPiA+ICsJImZzbCxUMTA0MFJEQi1jcGxk
IiwgImZzbCxUMTA0MlJEQi1jcGxkIiwgImZzbCxUMTA0MlJEQl9QSS1jcGxkIg0KPiA+ICstIHJl
ZzogU2hvdWxkIGNvbnRhaW4gdGhlIGNoaXAgc2VsZWN0LCBhZGRyZXNzIG9mZnNldCBhbmQgbGVu
Z3RoIG9mDQo+ID4gK3RoZSBDUExEDQo+ID4gKw0KPiA+ICtFeGFtcGxlOg0KPiA+ICsJY3BsZEAz
LDAgew0KPiA+ICsJCSNhZGRyZXNzLWNlbGxzID0gPDE+Ow0KPiA+ICsJCSNzaXplLWNlbGxzID0g
PDE+Ow0KPiA+ICsJCWNvbXBhdGlibGUgPSAiZnNsLFQxMDQwUkRCLWNwbGQiOw0KPiA+ICsJCXJl
ZyA9IDwzIDAgMHgzMDA+Ow0KPiA+ICsJfTsNCj4gDQo+IFBsZWFzZSBrZWVwIHRoZSBib2FyZCBu
YW1lIGluIGxvd2VyY2FzZSBsaWtlIHRoZSByZXN0Lg0KPiANCj4gLVNjb3R0DQo+IA0KW0phaW4g
UHJpeWFua2EtQjMyMTY3XVRoYW5rcyBmb3IgdGhlIGlucHV0cy4gIEkgd2lsbCBjb3JyZWN0IHRo
aXMNCg0K
^ permalink raw reply
* [PATCH][v2] devicetree/binding/powerpc/fsl: Add binding for CPLD
From: Priyanka Jain @ 2014-07-04 4:33 UTC (permalink / raw)
To: devicetree, linuxppc-dev, scottwood; +Cc: Priyanka Jain
Some Freescale boards like T1040RDB have on board CPLD connected on
the IFC bus. Add binding for this in board.txt file
Signed-off-by: Priyanka Jain <Priyanka.Jain@freescale.com>
---
Changes for v2:
convert board name to lower-case based on Scott's suggestions
.../devicetree/bindings/powerpc/fsl/board.txt | 19 +++++++++++++++++++
1 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/board.txt b/Documentation/devicetree/bindings/powerpc/fsl/board.txt
index 700dec4..f35f295 100644
--- a/Documentation/devicetree/bindings/powerpc/fsl/board.txt
+++ b/Documentation/devicetree/bindings/powerpc/fsl/board.txt
@@ -84,3 +84,22 @@ Example:
compatible = "fsl,bsc9132qds-fpga", "fsl,fpga-qixis-i2c";
reg = <0x66>;
};
+
+* Freescale on-board CPLD
+
+Some Freescale boards like T1040RDB have on board CPLD connected on
+the IFC bus.
+
+Required properties:
+- compatible: Should be a board-specific string like "fsl,<board>-cpld"
+ Example:
+ "fsl,t1040rdb-cpld", "fsl,t1042rdb-cpld", "fsl,t1042rdb_pi-cpld"
+- reg: Should contain the chip select, address offset and length of the CPLD
+
+Example:
+ cpld@3,0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "fsl,t1040rdb-cpld";
+ reg = <3 0 0x300>;
+ };
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH v2] powerpc/kvm: support to handle sw breakpoint
From: Alexander Graf @ 2014-07-04 6:48 UTC (permalink / raw)
To: Madhavan Srinivasan, benh, paulus; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <53B62EEE.7010804@linux.vnet.ibm.com>
On 04.07.14 06:34, Madhavan Srinivasan wrote:
> On Thursday 03 July 2014 05:21 PM, Alexander Graf wrote:
>> On 01.07.14 10:41, Madhavan Srinivasan wrote:
>>> This patch adds kernel side support for software breakpoint.
>>> Design is that, by using an illegal instruction, we trap to hypervisor
>>> via Emulation Assistance interrupt, where we check for the illegal
>>> instruction
>>> and accordingly we return to Host or Guest. Patch also adds support for
>>> software breakpoint in PR KVM.
>>>
>>> Patch mandates use of "abs" instruction as sw breakpoint instruction
>>> (primary opcode 31 and extended opcode 360). Based on PowerISA v2.01,
>>> ABS instruction has been dropped from the architecture and treated an
>>> illegal instruction.
>>>
>>> Changes v1->v2:
>>>
>>> Moved the debug instruction #def to kvm_book3s.h. This way PR_KVM
>>> can also share it.
>>> Added code to use KVM get one reg infrastructure to get debug opcode.
>>> Updated emulate.c to include emulation of debug instruction incase
>>> of PR_KVM.
>>> Made changes to commit message.
>>>
>>> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
>>> ---
>>> arch/powerpc/include/asm/kvm_book3s.h | 8 ++++++++
>>> arch/powerpc/include/asm/ppc-opcode.h | 5 +++++
>>> arch/powerpc/kvm/book3s.c | 3 ++-
>>> arch/powerpc/kvm/book3s_hv.c | 9 +++++++++
>>> arch/powerpc/kvm/book3s_pr.c | 3 +++
>>> arch/powerpc/kvm/emulate.c | 10 ++++++++++
>>> 6 files changed, 37 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/kvm_book3s.h
>>> b/arch/powerpc/include/asm/kvm_book3s.h
>>> index f52f656..180d549 100644
>>> --- a/arch/powerpc/include/asm/kvm_book3s.h
>>> +++ b/arch/powerpc/include/asm/kvm_book3s.h
>>> @@ -24,6 +24,14 @@
>>> #include <linux/kvm_host.h>
>>> #include <asm/kvm_book3s_asm.h>
>>> +/*
>>> + * KVMPPC_INST_BOOK3S_DEBUG is debug Instruction for supporting
>>> Software Breakpoint.
>>> + * Instruction mnemonic is ABS, primary opcode is 31 and extended
>>> opcode is 360.
>>> + * Based on PowerISA v2.01, ABS instruction has been dropped from the
>>> architecture
>>> + * and treated an illegal instruction.
>>> + */
>>> +#define KVMPPC_INST_BOOK3S_DEBUG 0x7c0002d0
>> This will still break with LE guests.
>>
> I am told to try with all 0s opcode. So rewriting the patch.
The problem with "all 0s" is that it's reasonably likely to occur on
real world code. Hence Segher was proposing something like 0x00dddd00
which should be the same regardless of endianness, but has a certain
appeal of intentional placement ;).
>
>>> +
>>> struct kvmppc_bat {
>>> u64 raw;
>>> u32 bepi;
>>> diff --git a/arch/powerpc/include/asm/ppc-opcode.h
>>> b/arch/powerpc/include/asm/ppc-opcode.h
>>> index 3132bb9..3fbb4c1 100644
>>> --- a/arch/powerpc/include/asm/ppc-opcode.h
>>> +++ b/arch/powerpc/include/asm/ppc-opcode.h
>>> @@ -111,6 +111,11 @@
>>> #define OP_31_XOP_LHBRX 790
>>> #define OP_31_XOP_STHBRX 918
>>> +/* KVMPPC_INST_BOOK3S_DEBUG -- Software breakpoint Instruction
>>> + * Instruction mnemonic is ABS, primary opcode is 31 and extended
>>> opcode is 360.
>>> + */
>>> +#define OP_31_XOP_ABS 360
>>> +
>>> #define OP_LWZ 32
>>> #define OP_LD 58
>>> #define OP_LWZU 33
>>> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
>>> index c254c27..b40fe5d 100644
>>> --- a/arch/powerpc/kvm/book3s.c
>>> +++ b/arch/powerpc/kvm/book3s.c
>>> @@ -789,7 +789,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu
>>> *vcpu,
>>> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>>> struct kvm_guest_debug *dbg)
>>> {
>>> - return -EINVAL;
>>> + vcpu->guest_debug = dbg->control;
>>> + return 0;
>>> }
>>> void kvmppc_decrementer_func(unsigned long data)
>>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>>> index 7a12edb..402c1ec 100644
>>> --- a/arch/powerpc/kvm/book3s_hv.c
>>> +++ b/arch/powerpc/kvm/book3s_hv.c
>>> @@ -725,8 +725,14 @@ static int kvmppc_handle_exit_hv(struct kvm_run
>>> *run, struct kvm_vcpu *vcpu,
>>> * we don't emulate any guest instructions at this stage.
>>> */
>>> case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
>>> + if (kvmppc_get_last_inst(vcpu) == KVMPPC_INST_BOOK3S_DEBUG ) {
>>> + run->exit_reason = KVM_EXIT_DEBUG;
>>> + run->debug.arch.address = kvmppc_get_pc(vcpu);
>>> + r = RESUME_HOST;
>> Phew - why can't we just go into the normal instruction emulator for
>> EMUL_ASSIST?
>>
> IIUC, using the emulation_assist_interrupt function (kernel/trap.c) ?
I was more thinking of kvmppc_emulate_instruction() :).
Alex
^ permalink raw reply
* Re: [PATCH v2] powerpc/kvm: support to handle sw breakpoint
From: Madhavan Srinivasan @ 2014-07-04 7:32 UTC (permalink / raw)
To: Alexander Graf, benh, paulus; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <53B64E4D.1010802@suse.de>
On Friday 04 July 2014 12:18 PM, Alexander Graf wrote:
>
> On 04.07.14 06:34, Madhavan Srinivasan wrote:
>> On Thursday 03 July 2014 05:21 PM, Alexander Graf wrote:
>>> On 01.07.14 10:41, Madhavan Srinivasan wrote:
>>>> This patch adds kernel side support for software breakpoint.
>>>> Design is that, by using an illegal instruction, we trap to hypervisor
>>>> via Emulation Assistance interrupt, where we check for the illegal
>>>> instruction
>>>> and accordingly we return to Host or Guest. Patch also adds support for
>>>> software breakpoint in PR KVM.
>>>>
>>>> Patch mandates use of "abs" instruction as sw breakpoint instruction
>>>> (primary opcode 31 and extended opcode 360). Based on PowerISA v2.01,
>>>> ABS instruction has been dropped from the architecture and treated an
>>>> illegal instruction.
>>>>
>>>> Changes v1->v2:
>>>>
>>>> Moved the debug instruction #def to kvm_book3s.h. This way PR_KVM
>>>> can also share it.
>>>> Added code to use KVM get one reg infrastructure to get debug
>>>> opcode.
>>>> Updated emulate.c to include emulation of debug instruction incase
>>>> of PR_KVM.
>>>> Made changes to commit message.
>>>>
>>>> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
>>>> ---
>>>> arch/powerpc/include/asm/kvm_book3s.h | 8 ++++++++
>>>> arch/powerpc/include/asm/ppc-opcode.h | 5 +++++
>>>> arch/powerpc/kvm/book3s.c | 3 ++-
>>>> arch/powerpc/kvm/book3s_hv.c | 9 +++++++++
>>>> arch/powerpc/kvm/book3s_pr.c | 3 +++
>>>> arch/powerpc/kvm/emulate.c | 10 ++++++++++
>>>> 6 files changed, 37 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/powerpc/include/asm/kvm_book3s.h
>>>> b/arch/powerpc/include/asm/kvm_book3s.h
>>>> index f52f656..180d549 100644
>>>> --- a/arch/powerpc/include/asm/kvm_book3s.h
>>>> +++ b/arch/powerpc/include/asm/kvm_book3s.h
>>>> @@ -24,6 +24,14 @@
>>>> #include <linux/kvm_host.h>
>>>> #include <asm/kvm_book3s_asm.h>
>>>> +/*
>>>> + * KVMPPC_INST_BOOK3S_DEBUG is debug Instruction for supporting
>>>> Software Breakpoint.
>>>> + * Instruction mnemonic is ABS, primary opcode is 31 and extended
>>>> opcode is 360.
>>>> + * Based on PowerISA v2.01, ABS instruction has been dropped from the
>>>> architecture
>>>> + * and treated an illegal instruction.
>>>> + */
>>>> +#define KVMPPC_INST_BOOK3S_DEBUG 0x7c0002d0
>>> This will still break with LE guests.
>>>
>> I am told to try with all 0s opcode. So rewriting the patch.
>
> The problem with "all 0s" is that it's reasonably likely to occur on
> real world code. Hence Segher was proposing something like 0x00dddd00
> which should be the same regardless of endianness, but has a certain
> appeal of intentional placement ;).
>
Ok Sure.
>>
>>>> +
>>>> struct kvmppc_bat {
>>>> u64 raw;
>>>> u32 bepi;
>>>> diff --git a/arch/powerpc/include/asm/ppc-opcode.h
>>>> b/arch/powerpc/include/asm/ppc-opcode.h
>>>> index 3132bb9..3fbb4c1 100644
>>>> --- a/arch/powerpc/include/asm/ppc-opcode.h
>>>> +++ b/arch/powerpc/include/asm/ppc-opcode.h
>>>> @@ -111,6 +111,11 @@
>>>> #define OP_31_XOP_LHBRX 790
>>>> #define OP_31_XOP_STHBRX 918
>>>> +/* KVMPPC_INST_BOOK3S_DEBUG -- Software breakpoint Instruction
>>>> + * Instruction mnemonic is ABS, primary opcode is 31 and extended
>>>> opcode is 360.
>>>> + */
>>>> +#define OP_31_XOP_ABS 360
>>>> +
>>>> #define OP_LWZ 32
>>>> #define OP_LD 58
>>>> #define OP_LWZU 33
>>>> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
>>>> index c254c27..b40fe5d 100644
>>>> --- a/arch/powerpc/kvm/book3s.c
>>>> +++ b/arch/powerpc/kvm/book3s.c
>>>> @@ -789,7 +789,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu
>>>> *vcpu,
>>>> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>>>> struct kvm_guest_debug *dbg)
>>>> {
>>>> - return -EINVAL;
>>>> + vcpu->guest_debug = dbg->control;
>>>> + return 0;
>>>> }
>>>> void kvmppc_decrementer_func(unsigned long data)
>>>> diff --git a/arch/powerpc/kvm/book3s_hv.c
>>>> b/arch/powerpc/kvm/book3s_hv.c
>>>> index 7a12edb..402c1ec 100644
>>>> --- a/arch/powerpc/kvm/book3s_hv.c
>>>> +++ b/arch/powerpc/kvm/book3s_hv.c
>>>> @@ -725,8 +725,14 @@ static int kvmppc_handle_exit_hv(struct kvm_run
>>>> *run, struct kvm_vcpu *vcpu,
>>>> * we don't emulate any guest instructions at this stage.
>>>> */
>>>> case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
>>>> + if (kvmppc_get_last_inst(vcpu) == KVMPPC_INST_BOOK3S_DEBUG ) {
>>>> + run->exit_reason = KVM_EXIT_DEBUG;
>>>> + run->debug.arch.address = kvmppc_get_pc(vcpu);
>>>> + r = RESUME_HOST;
>>> Phew - why can't we just go into the normal instruction emulator for
>>> EMUL_ASSIST?
>>>
>> IIUC, using the emulation_assist_interrupt function (kernel/trap.c) ?
>
> I was more thinking of kvmppc_emulate_instruction() :).
>
This makes sense. Can use the same call for pr kvm also. awesome :)
>
> Alex
>
^ permalink raw reply
* Re: [PATCH 3/6 v2] KVM: PPC: Book3E: Increase FPU laziness
From: Alexander Graf @ 2014-07-04 7:46 UTC (permalink / raw)
To: mihai.caraman@freescale.com, kvm-ppc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org
In-Reply-To: <bf24ca20f84f449a8781107acaec33d1@BY2PR03MB508.namprd03.prod.outlook.com>
On 03.07.14 17:46, mihai.caraman@freescale.com wrote:
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Thursday, July 03, 2014 3:29 PM
>> To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org
>> Cc: kvm@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
>> Subject: Re: [PATCH 3/6 v2] KVM: PPC: Book3E: Increase FPU laziness
>>
>>
>> On 30.06.14 17:34, Mihai Caraman wrote:
>>> Increase FPU laziness by calling kvmppc_load_guest_fp() just before
>>> returning to guest instead of each sched in. Without this improvement
>>> an interrupt may also claim floting point corrupting guest state.
>> How do you handle context switching with this patch applied? During most
>> of the guest's lifetime we never exit kvmppc_vcpu_run(), so when the
>> guest gets switched out all FPU state gets lost?
> No, we had this discussion in ver 1. The FP/VMX/VSX is implemented lazy in
> the kernel i.e. the unit state is not saved/restored until another thread
> that once claimed the unit is sched in.
>
> Since FP/VMX/VSX can be activated by the guest independent of the host, the
> vcpu thread is always using the unit (even if it did not claimed it once).
>
> Now, this patch optimize the sched in flow. Instead of checking on each vcpu
> sched in if the kernel unloaded unit's guest state for another competing host
> process we do this when we enter the guest.
But we only do it when we enter the guest from QEMU, not when we enter
the guest after a context switch on cond_resched(), no?
Alex
^ permalink raw reply
* Re: [PATCH 1/3] powerpc/kvm: Remove redundant save of SIER AND MMCR2
From: Paul Mackerras @ 2014-07-04 7:46 UTC (permalink / raw)
To: Joel Stanley; +Cc: linuxppc-dev
In-Reply-To: <1404367956-19515-2-git-send-email-joel@jms.id.au>
On Thu, Jul 03, 2014 at 03:42:34PM +0930, Joel Stanley wrote:
> These two registers are already saved in the block above. Aside from
> being unnecessary, by the time we get down to the second save location
> r8 no longer contains MMCR2, so we are clobbering the saved value with
> PMC5.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Eek!
Acked-by: Paul Mackerras <paulus@samba.org>
^ permalink raw reply
* Re: [PATCH 3/6 v2] KVM: PPC: Book3E: Increase FPU laziness
From: Alexander Graf @ 2014-07-04 7:52 UTC (permalink / raw)
To: mihai.caraman@freescale.com, kvm-ppc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org
In-Reply-To: <53B65BCA.6040200@suse.de>
On 04.07.14 09:46, Alexander Graf wrote:
>
> On 03.07.14 17:46, mihai.caraman@freescale.com wrote:
>>> -----Original Message-----
>>> From: Alexander Graf [mailto:agraf@suse.de]
>>> Sent: Thursday, July 03, 2014 3:29 PM
>>> To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org
>>> Cc: kvm@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
>>> Subject: Re: [PATCH 3/6 v2] KVM: PPC: Book3E: Increase FPU laziness
>>>
>>>
>>> On 30.06.14 17:34, Mihai Caraman wrote:
>>>> Increase FPU laziness by calling kvmppc_load_guest_fp() just before
>>>> returning to guest instead of each sched in. Without this improvement
>>>> an interrupt may also claim floting point corrupting guest state.
>>> How do you handle context switching with this patch applied? During
>>> most
>>> of the guest's lifetime we never exit kvmppc_vcpu_run(), so when the
>>> guest gets switched out all FPU state gets lost?
>> No, we had this discussion in ver 1. The FP/VMX/VSX is implemented
>> lazy in
>> the kernel i.e. the unit state is not saved/restored until another
>> thread
>> that once claimed the unit is sched in.
>>
>> Since FP/VMX/VSX can be activated by the guest independent of the
>> host, the
>> vcpu thread is always using the unit (even if it did not claimed it
>> once).
>>
>> Now, this patch optimize the sched in flow. Instead of checking on
>> each vcpu
>> sched in if the kernel unloaded unit's guest state for another
>> competing host
>> process we do this when we enter the guest.
>
> But we only do it when we enter the guest from QEMU, not when we enter
> the guest after a context switch on cond_resched(), no?
Ah, I missed the call to the load function in handle_exit(). Ok, I think
that approach should work.
Alex
^ permalink raw reply
* Re: [PATCH 5/6 v2] KVM: PPC: Book3E: Add ONE_REG AltiVec support
From: Alexander Graf @ 2014-07-04 7:54 UTC (permalink / raw)
To: mihai.caraman@freescale.com, kvm-ppc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org
In-Reply-To: <92e31868b99a421ab578640527b41ccf@BY2PR03MB508.namprd03.prod.outlook.com>
On 03.07.14 18:11, mihai.caraman@freescale.com wrote:
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Thursday, July 03, 2014 3:34 PM
>> To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org
>> Cc: kvm@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
>> Subject: Re: [PATCH 5/6 v2] KVM: PPC: Book3E: Add ONE_REG AltiVec support
>>
>>
>> On 30.06.14 17:34, Mihai Caraman wrote:
>>> Add ONE_REG support for AltiVec on Book3E.
>>>
>>> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>> Any chance we can handle these in generic code?
> I expected this request :) Can we let this for a second phase to have
> e6500 enabled first?
I don't see the value of duplicating code in e500 specific code only to
remove and combine it in common code in a follow-up patch after that.
> Can you share with us a Book3S setup so I can validate the requested
> changes? I already fell anxious touching strange hardware specific
> Book3S code without running it.
Until a few weeks ago I had an externally reachable G5 machine that we
could've used for this. Unfortunately I had to replace the box with
another one that's not quite as stable. I'll try and see if I can fix or
replace it soon.
Alex
^ permalink raw reply
* Re: [PATCH 1/3] powerpc/kvm: Remove redundant save of SIER AND MMCR2
From: Alexander Graf @ 2014-07-04 7:57 UTC (permalink / raw)
To: Joel Stanley, paulus, benh, mpe; +Cc: linuxppc-dev, KVM, kvm-ppc
In-Reply-To: <1404367956-19515-2-git-send-email-joel@jms.id.au>
On 03.07.14 08:12, Joel Stanley wrote:
> These two registers are already saved in the block above. Aside from
> being unnecessary, by the time we get down to the second save location
> r8 no longer contains MMCR2, so we are clobbering the saved value with
> PMC5.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Alexander Graf <agraf@suse.de>
Please CC kvm-ppc@vger and kvm@vger when you send kvm related patches :).
Ben, I think this patch makes sense to go via your tree. Want to take it?
Alex
> ---
> arch/powerpc/kvm/book3s_hv_interrupts.S | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv_interrupts.S b/arch/powerpc/kvm/book3s_hv_interrupts.S
> index 8c86422..731be74 100644
> --- a/arch/powerpc/kvm/book3s_hv_interrupts.S
> +++ b/arch/powerpc/kvm/book3s_hv_interrupts.S
> @@ -127,11 +127,6 @@ BEGIN_FTR_SECTION
> stw r10, HSTATE_PMC + 24(r13)
> stw r11, HSTATE_PMC + 28(r13)
> END_FTR_SECTION_IFSET(CPU_FTR_ARCH_201)
> -BEGIN_FTR_SECTION
> - mfspr r9, SPRN_SIER
> - std r8, HSTATE_MMCR + 40(r13)
> - std r9, HSTATE_MMCR + 48(r13)
> -END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
> 31:
>
> /*
^ permalink raw reply
* Re: [RFC PATCH 2/4] KVM: PPC: Book3E: Handle LRAT error exception
From: Alexander Graf @ 2014-07-04 8:15 UTC (permalink / raw)
To: Mihai Caraman, kvm-ppc; +Cc: linuxppc-dev, kvm
In-Reply-To: <1404398727-12844-3-git-send-email-mihai.caraman@freescale.com>
On 03.07.14 16:45, Mihai Caraman wrote:
> Handle LRAT error exception with support for lrat mapping and invalidation.
>
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> ---
> arch/powerpc/include/asm/kvm_host.h | 1 +
> arch/powerpc/include/asm/kvm_ppc.h | 2 +
> arch/powerpc/include/asm/mmu-book3e.h | 3 +
> arch/powerpc/include/asm/reg_booke.h | 13 ++++
> arch/powerpc/kernel/asm-offsets.c | 1 +
> arch/powerpc/kvm/booke.c | 40 +++++++++++
> arch/powerpc/kvm/bookehv_interrupts.S | 9 ++-
> arch/powerpc/kvm/e500_mmu_host.c | 125 ++++++++++++++++++++++++++++++++++
> arch/powerpc/kvm/e500mc.c | 2 +
> 9 files changed, 195 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
> index bb66d8b..7b6b2ec 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -433,6 +433,7 @@ struct kvm_vcpu_arch {
> u32 eplc;
> u32 epsc;
> u32 oldpir;
> + u64 fault_lper;
> #endif
>
> #if defined(CONFIG_BOOKE)
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
> index 9c89cdd..2730a29 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -86,6 +86,8 @@ extern gpa_t kvmppc_mmu_xlate(struct kvm_vcpu *vcpu, unsigned int gtlb_index,
> gva_t eaddr);
> extern void kvmppc_mmu_dtlb_miss(struct kvm_vcpu *vcpu);
> extern void kvmppc_mmu_itlb_miss(struct kvm_vcpu *vcpu);
> +extern void kvmppc_lrat_map(struct kvm_vcpu *vcpu, gfn_t gfn);
> +extern void kvmppc_lrat_invalidate(struct kvm_vcpu *vcpu);
>
> extern struct kvm_vcpu *kvmppc_core_vcpu_create(struct kvm *kvm,
> unsigned int id);
> diff --git a/arch/powerpc/include/asm/mmu-book3e.h b/arch/powerpc/include/asm/mmu-book3e.h
> index 088fd9f..ac6acf7 100644
> --- a/arch/powerpc/include/asm/mmu-book3e.h
> +++ b/arch/powerpc/include/asm/mmu-book3e.h
> @@ -40,6 +40,8 @@
>
> /* MAS registers bit definitions */
>
> +#define MAS0_ATSEL 0x80000000
> +#define MAS0_ATSEL_SHIFT 31
> #define MAS0_TLBSEL_MASK 0x30000000
> #define MAS0_TLBSEL_SHIFT 28
> #define MAS0_TLBSEL(x) (((x) << MAS0_TLBSEL_SHIFT) & MAS0_TLBSEL_MASK)
> @@ -53,6 +55,7 @@
> #define MAS0_WQ_CLR_RSRV 0x00002000
>
> #define MAS1_VALID 0x80000000
> +#define MAS1_VALID_SHIFT 31
> #define MAS1_IPROT 0x40000000
> #define MAS1_TID(x) (((x) << 16) & 0x3FFF0000)
> #define MAS1_IND 0x00002000
> diff --git a/arch/powerpc/include/asm/reg_booke.h b/arch/powerpc/include/asm/reg_booke.h
> index 75bda23..783d617 100644
> --- a/arch/powerpc/include/asm/reg_booke.h
> +++ b/arch/powerpc/include/asm/reg_booke.h
> @@ -43,6 +43,8 @@
>
> /* Special Purpose Registers (SPRNs)*/
> #define SPRN_DECAR 0x036 /* Decrementer Auto Reload Register */
> +#define SPRN_LPER 0x038 /* Logical Page Exception Register */
> +#define SPRN_LPERU 0x039 /* Logical Page Exception Register Upper */
> #define SPRN_IVPR 0x03F /* Interrupt Vector Prefix Register */
> #define SPRN_USPRG0 0x100 /* User Special Purpose Register General 0 */
> #define SPRN_SPRG3R 0x103 /* Special Purpose Register General 3 Read */
> @@ -358,6 +360,9 @@
> #define ESR_ILK 0x00100000 /* Instr. Cache Locking */
> #define ESR_PUO 0x00040000 /* Unimplemented Operation exception */
> #define ESR_BO 0x00020000 /* Byte Ordering */
> +#define ESR_DATA 0x00000400 /* Page Table Data Access */
> +#define ESR_TLBI 0x00000200 /* Page Table TLB Ineligible */
> +#define ESR_PT 0x00000100 /* Page Table Translation */
> #define ESR_SPV 0x00000080 /* Signal Processing operation */
>
> /* Bit definitions related to the DBCR0. */
> @@ -649,6 +654,14 @@
> #define EPC_EPID 0x00003fff
> #define EPC_EPID_SHIFT 0
>
> +/* Bit definitions for LPER */
> +#define LPER_ALPN 0x000FFFFFFFFFF000ULL
> +#define LPER_ALPN_SHIFT 12
> +#define LPER_WIMGE 0x00000F80
> +#define LPER_WIMGE_SHIFT 7
> +#define LPER_LPS 0x0000000F
> +#define LPER_LPS_SHIFT 0
> +
> /*
> * The IBM-403 is an even more odd special case, as it is much
> * older than the IBM-405 series. We put these down here incase someone
> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
> index f5995a9..be6e329 100644
> --- a/arch/powerpc/kernel/asm-offsets.c
> +++ b/arch/powerpc/kernel/asm-offsets.c
> @@ -713,6 +713,7 @@ int main(void)
> DEFINE(VCPU_HOST_MAS4, offsetof(struct kvm_vcpu, arch.host_mas4));
> DEFINE(VCPU_HOST_MAS6, offsetof(struct kvm_vcpu, arch.host_mas6));
> DEFINE(VCPU_EPLC, offsetof(struct kvm_vcpu, arch.eplc));
> + DEFINE(VCPU_FAULT_LPER, offsetof(struct kvm_vcpu, arch.fault_lper));
> #endif
>
> #ifdef CONFIG_KVM_EXIT_TIMING
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index a192975..ab1077f 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -1286,6 +1286,46 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
> break;
> }
>
> +#ifdef CONFIG_KVM_BOOKE_HV
> + case BOOKE_INTERRUPT_LRAT_ERROR:
> + {
> + gfn_t gfn;
> +
> + /*
> + * Guest TLB management instructions (EPCR.DGTMI == 0) is not
> + * supported for now
> + */
> + if (!(vcpu->arch.fault_esr & ESR_PT)) {
> + WARN(1, "%s: Guest TLB management instructions not supported!\n", __func__);
Wouldn't this allow a guest to flood the host's kernel log?
> + break;
> + }
> +
> + gfn = (vcpu->arch.fault_lper & LPER_ALPN) >> LPER_ALPN_SHIFT;
Maybe add an #ifdef and #error check to make sure that LPER_ALPN_SHIFT
== PAGE_SHIFT?
> +
> + idx = srcu_read_lock(&vcpu->kvm->srcu);
> +
> + if (kvm_is_visible_gfn(vcpu->kvm, gfn)) {
> + kvmppc_lrat_map(vcpu, gfn);
> + r = RESUME_GUEST;
> + } else if (vcpu->arch.fault_esr & ESR_DATA) {
> + vcpu->arch.paddr_accessed = (gfn << PAGE_SHIFT)
> + | (vcpu->arch.fault_dear & (PAGE_SIZE - 1));
> + vcpu->arch.vaddr_accessed =
> + vcpu->arch.fault_dear;
> +
> + r = kvmppc_emulate_mmio(run, vcpu);
> + kvmppc_account_exit(vcpu, MMIO_EXITS);
It's a shame we have to duplicate that logic from the normal TLB miss
path, but I can't see any good way to combine them either.
> + } else {
> + kvmppc_booke_queue_irqprio(vcpu,
> + BOOKE_IRQPRIO_MACHINE_CHECK);
> + r = RESUME_GUEST;
> + }
> +
> + srcu_read_unlock(&vcpu->kvm->srcu, idx);
> + break;
> + }
> +#endif
> +
> case BOOKE_INTERRUPT_DEBUG: {
> r = kvmppc_handle_debug(run, vcpu);
> if (r == RESUME_HOST)
> diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S
> index b3ecdd6..341c3a8 100644
> --- a/arch/powerpc/kvm/bookehv_interrupts.S
> +++ b/arch/powerpc/kvm/bookehv_interrupts.S
> @@ -64,6 +64,7 @@
> #define NEED_EMU 0x00000001 /* emulation -- save nv regs */
> #define NEED_DEAR 0x00000002 /* save faulting DEAR */
> #define NEED_ESR 0x00000004 /* save faulting ESR */
> +#define NEED_LPER 0x00000008 /* save faulting LPER */
>
> /*
> * On entry:
> @@ -203,6 +204,12 @@
> PPC_STL r9, VCPU_FAULT_DEAR(r4)
> .endif
>
> + /* Only suppported on 64-bit cores for now */
> + .if \flags & NEED_LPER
> + mfspr r7, SPRN_LPER
> + std r7, VCPU_FAULT_LPER(r4)
> + .endif
> +
> b kvmppc_resume_host
> .endm
>
> @@ -325,7 +332,7 @@ kvm_handler BOOKE_INTERRUPT_DEBUG, EX_PARAMS(DBG), \
> kvm_handler BOOKE_INTERRUPT_DEBUG, EX_PARAMS(CRIT), \
> SPRN_CSRR0, SPRN_CSRR1, 0
> kvm_handler BOOKE_INTERRUPT_LRAT_ERROR, EX_PARAMS(GEN), \
> - SPRN_SRR0, SPRN_SRR1, (NEED_EMU | NEED_DEAR | NEED_ESR)
> + SPRN_SRR0, SPRN_SRR1, (NEED_EMU | NEED_DEAR | NEED_ESR | NEED_LPER)
> #else
> /*
> * For input register values, see arch/powerpc/include/asm/kvm_booke_hv_asm.h
> diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
> index 79677d7..be1454b 100644
> --- a/arch/powerpc/kvm/e500_mmu_host.c
> +++ b/arch/powerpc/kvm/e500_mmu_host.c
> @@ -95,6 +95,131 @@ static inline void __write_host_tlbe(struct kvm_book3e_206_tlb_entry *stlbe,
> stlbe->mas2, stlbe->mas7_3);
> }
>
> +#ifdef CONFIG_KVM_BOOKE_HV
> +#ifdef CONFIG_64BIT
> +static inline int lrat_next(void)
No inline in .c files please. Just only make them "static".
> +{
> + int this, next;
> +
> + this = local_paca->tcd.lrat_next;
> + next = (this + 1) % local_paca->tcd.lrat_max;
Can we assume that lrat_max is always a power of 2? IIRC modulo
functions with variables can be quite expensive. So if we can instead do
next = (this + 1) & local_paca->tcd.lrat_mask;
we should be faster and not rely on division helpers.
> + local_paca->tcd.lrat_next = next;
> +
> + return this;
> +}
> +
> +static inline int lrat_size(void)
> +{
> + return local_paca->tcd.lrat_max;
> +}
> +#else
> +/* LRAT is only supported in 64-bit kernel for now */
> +static inline int lrat_next(void)
> +{
> + BUG();
> +}
> +
> +static inline int lrat_size(void)
> +{
> + return 0;
> +}
> +#endif
> +
> +void write_host_lrate(int tsize, gfn_t gfn, unsigned long pfn, uint32_t lpid,
> + int valid, int lrat_entry)
> +{
> + struct kvm_book3e_206_tlb_entry stlbe;
> + int esel = lrat_entry;
> + unsigned long flags;
> +
> + stlbe.mas1 = (valid ? MAS1_VALID : 0) | MAS1_TSIZE(tsize);
> + stlbe.mas2 = ((u64)gfn << PAGE_SHIFT);
> + stlbe.mas7_3 = ((u64)pfn << PAGE_SHIFT);
> + stlbe.mas8 = MAS8_TGS | lpid;
> +
> + local_irq_save(flags);
> + /* book3e_tlb_lock(); */
Hm?
> +
> + if (esel == -1)
> + esel = lrat_next();
> + __write_host_tlbe(&stlbe, MAS0_ATSEL | MAS0_ESEL(esel));
> +
> + /* book3e_tlb_unlock(); */
> + local_irq_restore(flags);
> +}
> +
> +void kvmppc_lrat_map(struct kvm_vcpu *vcpu, gfn_t gfn)
> +{
> + struct kvm_memory_slot *slot;
> + unsigned long pfn;
> + unsigned long hva;
> + struct vm_area_struct *vma;
> + unsigned long psize;
> + int tsize;
> + unsigned long tsize_pages;
> +
> + slot = gfn_to_memslot(vcpu->kvm, gfn);
> + if (!slot) {
> + pr_err_ratelimited("%s: couldn't find memslot for gfn %lx!\n",
> + __func__, (long)gfn);
> + return;
> + }
> +
> + hva = slot->userspace_addr;
> +
> + down_read(¤t->mm->mmap_sem);
> + vma = find_vma(current->mm, hva);
> + if (vma && (hva >= vma->vm_start)) {
> + psize = vma_kernel_pagesize(vma);
> + } else {
> + pr_err_ratelimited("%s: couldn't find virtual memory address for gfn %lx!\n", __func__, (long)gfn);
> + return;
> + }
> + up_read(¤t->mm->mmap_sem);
> +
> + pfn = gfn_to_pfn_memslot(slot, gfn);
> + if (is_error_noslot_pfn(pfn)) {
> + pr_err_ratelimited("%s: couldn't get real page for gfn %lx!\n",
> + __func__, (long)gfn);
> + return;
> + }
> +
> + tsize = __ilog2(psize) - 10;
> + tsize_pages = 1 << (tsize + 10 - PAGE_SHIFT);
> + gfn &= ~(tsize_pages - 1);
> + pfn &= ~(tsize_pages - 1);
> +
> + write_host_lrate(tsize, gfn, pfn, vcpu->kvm->arch.lpid, 1, -1);
> + kvm_release_pfn_clean(pfn);
Don't we have to keep the page locked so it doesn't get swapped away?
Alex
> +}
> +
> +void kvmppc_lrat_invalidate(struct kvm_vcpu *vcpu)
> +{
> + uint32_t mas0, mas1 = 0;
> + int esel;
> + unsigned long flags;
> +
> + local_irq_save(flags);
> + /* book3e_tlb_lock(); */
> +
> + /* LRAT does not have a dedicated instruction for invalidation */
> + for (esel = 0; esel < lrat_size(); esel++) {
> + mas0 = MAS0_ATSEL | MAS0_ESEL(esel);
> + mtspr(SPRN_MAS0, mas0);
> + asm volatile("isync; tlbre" : : : "memory");
> + mas1 = mfspr(SPRN_MAS1) & ~MAS1_VALID;
> + mtspr(SPRN_MAS1, mas1);
> + asm volatile("isync; tlbwe" : : : "memory");
> + }
> + /* Must clear mas8 for other host tlbwe's */
> + mtspr(SPRN_MAS8, 0);
> + isync();
> +
> + /* book3e_tlb_unlock(); */
> + local_irq_restore(flags);
> +}
> +#endif
> +
> /*
> * Acquire a mas0 with victim hint, as if we just took a TLB miss.
> *
> diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
> index b1d9939..5622d9a 100644
> --- a/arch/powerpc/kvm/e500mc.c
> +++ b/arch/powerpc/kvm/e500mc.c
> @@ -99,6 +99,8 @@ void kvmppc_e500_tlbil_all(struct kvmppc_vcpu_e500 *vcpu_e500)
> asm volatile("tlbilxlpid");
> mtspr(SPRN_MAS5, 0);
> local_irq_restore(flags);
> +
> + kvmppc_lrat_invalidate(&vcpu_e500->vcpu);
> }
>
> void kvmppc_set_pid(struct kvm_vcpu *vcpu, u32 pid)
^ permalink raw reply
* [PATCH v2] KVM: PPC: e500: Emulate power management control SPR
From: Mihai Caraman @ 2014-07-04 8:17 UTC (permalink / raw)
To: kvm-ppc; +Cc: Mihai Caraman, linuxppc-dev, kvm
For FSL e6500 core the kernel uses power management SPR register (PWRMGTCR0)
to enable idle power down for cores and devices by setting up the idle count
period at boot time. With the host already controlling the power management
configuration the guest could simply benefit from it, so emulate guest request
as a general store.
Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
---
v2:
- treat the operation as a general store
arch/powerpc/include/asm/kvm_host.h | 1 +
arch/powerpc/kvm/e500_emulate.c | 12 ++++++++++++
2 files changed, 13 insertions(+)
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 62b2cee..faf2f0e 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -584,6 +584,7 @@ struct kvm_vcpu_arch {
u32 mmucfg;
u32 eptcfg;
u32 epr;
+ u32 pwrmgtcr0;
u32 crit_save;
/* guest debug registers*/
struct debug_reg dbg_reg;
diff --git a/arch/powerpc/kvm/e500_emulate.c b/arch/powerpc/kvm/e500_emulate.c
index 002d517..c99c40e 100644
--- a/arch/powerpc/kvm/e500_emulate.c
+++ b/arch/powerpc/kvm/e500_emulate.c
@@ -250,6 +250,14 @@ int kvmppc_core_emulate_mtspr_e500(struct kvm_vcpu *vcpu, int sprn, ulong spr_va
spr_val);
break;
+ case SPRN_PWRMGTCR0:
+ /*
+ * Guest relies on host power management configurations
+ * Treat the request as a general store
+ */
+ vcpu->arch.pwrmgtcr0 = spr_val;
+ break;
+
/* extra exceptions */
case SPRN_IVOR32:
vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_UNAVAIL] = spr_val;
@@ -368,6 +376,10 @@ int kvmppc_core_emulate_mfspr_e500(struct kvm_vcpu *vcpu, int sprn, ulong *spr_v
*spr_val = vcpu->arch.eptcfg;
break;
+ case SPRN_PWRMGTCR0:
+ *spr_val = vcpu->arch.pwrmgtcr0;
+ break;
+
/* extra exceptions */
case SPRN_IVOR32:
*spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_UNAVAIL];
--
1.7.11.7
^ permalink raw reply related
* Re: [RFC PATCH 0/4] KVM Book3E support for HTW guests
From: Alexander Graf @ 2014-07-04 8:29 UTC (permalink / raw)
To: Mihai Caraman, kvm-ppc; +Cc: Scott Wood, linuxppc-dev, kvm
In-Reply-To: <1404398727-12844-1-git-send-email-mihai.caraman@freescale.com>
On 03.07.14 16:45, Mihai Caraman wrote:
> KVM Book3E support for Hardware Page Tablewalk enabled guests.
It looks reasonably straight forward to me, though I have to admit that
I find the sind conditions pretty confusing.
Scott, would you mind to have a look at this set too? :)
Thanks a lot!
Alex
^ permalink raw reply
* Re: [PATCH v2] KVM: PPC: e500: Emulate power management control SPR
From: Alexander Graf @ 2014-07-04 8:30 UTC (permalink / raw)
To: Mihai Caraman, kvm-ppc; +Cc: linuxppc-dev, kvm
In-Reply-To: <1404461848-31999-1-git-send-email-mihai.caraman@freescale.com>
On 04.07.14 10:17, Mihai Caraman wrote:
> For FSL e6500 core the kernel uses power management SPR register (PWRMGTCR0)
> to enable idle power down for cores and devices by setting up the idle count
> period at boot time. With the host already controlling the power management
> configuration the guest could simply benefit from it, so emulate guest request
> as a general store.
>
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
Thanks, applied to kvm-ppc-queue.
Alex
^ permalink raw reply
* Re: [PATCH 1/3] PCI/MSI: Add pci_enable_msi_partial()
From: Alexander Gordeev @ 2014-07-04 8:57 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-mips, linux-s390, linux-pci, x86, linux-doc, linux-kernel,
linux-ide, iommu, xen-devel, linuxppc-dev
In-Reply-To: <20140702202201.GA28852@google.com>
On Wed, Jul 02, 2014 at 02:22:01PM -0600, Bjorn Helgaas wrote:
> On Tue, Jun 10, 2014 at 03:10:30PM +0200, Alexander Gordeev wrote:
> > There are PCI devices that require a particular value written
> > to the Multiple Message Enable (MME) register while aligned on
> > power of 2 boundary value of actually used MSI vectors 'nvec'
> > is a lesser of that MME value:
> >
> > roundup_pow_of_two(nvec) < 'Multiple Message Enable'
> >
> > However the existing pci_enable_msi_block() interface is not
> > able to configure such devices, since the value written to the
> > MME register is calculated from the number of requested MSIs
> > 'nvec':
> >
> > 'Multiple Message Enable' = roundup_pow_of_two(nvec)
>
> For MSI, software learns how many vectors a device requests by reading
> the Multiple Message Capable (MMC) field. This field is encoded, so a
> device can only request 1, 2, 4, 8, etc., vectors. It's impossible
> for a device to request 3 vectors; it would have to round up that up
> to a power of two and request 4 vectors.
>
> Software writes similarly encoded values to MME to tell the device how
> many vectors have been allocated for its use. For example, it's
> impossible to tell the device that it can use 3 vectors; the OS has to
> round that up and tell the device it can use 4 vectors.
Nod.
> So if I understand correctly, the point of this series is to take
> advantage of device-specific knowledge, e.g., the device requests 4
> vectors via MMC, but we "know" the device is only capable of using 3.
> Moreover, we tell the device via MME that 4 vectors are available, but
> we've only actually set up 3 of them.
Exactly.
> This makes me uneasy because we're lying to the device, and the device
> is perfectly within spec to use all 4 of those vectors. If anything
> changes the number of vectors the device uses (new device revision,
> firmware upgrade, etc.), this is liable to break.
If a device committed via non-MSI specific means to send only 3 vectors
out of 4 available why should we expect it to send 4? The probability of
a firmware sending 4/4 vectors in this case is equal to the probability
of sending 5/4 or 16/4, with the very same reason - a bug in the firmware.
Moreover, even vector 4/4 would be unexpected by the device driver, though
it is perfectly within the spec.
As of new device revision or firmware update etc. - it is just yet another
case of device driver vs the firmware match/mismatch. Not including this
change does not help here at all IMHO.
> Can you quantify the benefit of this? Can't a device already use
> MSI-X to request exactly the number of vectors it can use? (I know
A Intel AHCI chipset requires 16 vectors written to MME while advertises
(via AHCI registers) and uses only 6. Even attempt to init 8 vectors results
in device's fallback to 1 (!).
> not all devices support MSI-X, but maybe we should just accept MSI for
> what it is and encourage the HW guys to use MSI-X if MSI isn't good
> enough.)
>
> > In this case the result written to the MME register may not
> > satisfy the aforementioned PCI devices requirement and therefore
> > the PCI functions will not operate in a desired mode.
>
> I'm not sure what you mean by "will not operate in a desired mode."
> I thought this was an optimization to save vectors and that these
> changes would be completely invisible to the hardware.
Yes, this should be invisible to the hardware. The above is an attempt
to describe the Intel AHCI weirdness in general terms :) I think it
could be omitted.
> Bjorn
>
> > This update introduces pci_enable_msi_partial() extension to
> > pci_enable_msi_block() interface that accepts extra 'nvec_mme'
> > argument which is then written to MME register while the value
> > of 'nvec' is still used to setup as many interrupts as requested.
> >
> > As result of this change, architecture-specific callbacks
> > arch_msi_check_device() and arch_setup_msi_irqs() get an extra
> > 'nvec_mme' parameter as well, but it is ignored for now.
> > Therefore, this update is a placeholder for architectures that
> > wish to support pci_enable_msi_partial() function in the future.
> >
> > Cc: linux-doc@vger.kernel.org
> > Cc: linux-mips@linux-mips.org
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Cc: linux-s390@vger.kernel.org
> > Cc: x86@kernel.org
> > Cc: xen-devel@lists.xenproject.org
> > Cc: iommu@lists.linux-foundation.org
> > Cc: linux-ide@vger.kernel.org
> > Cc: linux-pci@vger.kernel.org
> > Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> > ---
> > Documentation/PCI/MSI-HOWTO.txt | 36 ++++++++++++++--
> > arch/mips/pci/msi-octeon.c | 2 +-
> > arch/powerpc/kernel/msi.c | 4 +-
> > arch/s390/pci/pci.c | 2 +-
> > arch/x86/kernel/x86_init.c | 2 +-
> > drivers/pci/msi.c | 83 ++++++++++++++++++++++++++++++++++-----
> > include/linux/msi.h | 5 +-
> > include/linux/pci.h | 3 +
> > 8 files changed, 115 insertions(+), 22 deletions(-)
> >
> > diff --git a/Documentation/PCI/MSI-HOWTO.txt b/Documentation/PCI/MSI-HOWTO.txt
> > index 10a9369..c8a8503 100644
> > --- a/Documentation/PCI/MSI-HOWTO.txt
> > +++ b/Documentation/PCI/MSI-HOWTO.txt
> > @@ -195,14 +195,40 @@ By contrast with pci_enable_msi_range() function, pci_enable_msi_exact()
> > returns zero in case of success, which indicates MSI interrupts have been
> > successfully allocated.
> >
> > -4.2.4 pci_disable_msi
> > +4.2.4 pci_enable_msi_partial
> > +
> > +int pci_enable_msi_partial(struct pci_dev *dev, int nvec, int nvec_mme)
> > +
> > +This variation on pci_enable_msi_exact() call allows a device driver to
> > +setup 'nvec_mme' number of multiple MSIs with the PCI function, while
> > +setup only 'nvec' (which could be a lesser of 'nvec_mme') number of MSIs
> > +in operating system. The MSI specification only allows 'nvec_mme' to be
> > +allocated in powers of two, up to a maximum of 2^5 (32).
> > +
> > +This function could be used when a PCI function is known to send 'nvec'
> > +MSIs, but still requires a particular number of MSIs 'nvec_mme' to be
> > +initialized with. As result, 'nvec_mme' - 'nvec' number of unused MSIs
> > +do not waste system resources.
> > +
> > +If this function returns 0, it has succeeded in allocating 'nvec_mme'
> > +interrupts and setting up 'nvec' interrupts. In this case, the function
> > +enables MSI on this device and updates dev->irq to be the lowest of the
> > +new interrupts assigned to it. The other interrupts assigned to the
> > +device are in the range dev->irq to dev->irq + nvec - 1.
> > +
> > +If this function returns a negative number, it indicates an error and
> > +the driver should not attempt to request any more MSI interrupts for
> > +this device.
> > +
> > +4.2.5 pci_disable_msi
> >
> > void pci_disable_msi(struct pci_dev *dev)
> >
> > -This function should be used to undo the effect of pci_enable_msi_range().
> > -Calling it restores dev->irq to the pin-based interrupt number and frees
> > -the previously allocated MSIs. The interrupts may subsequently be assigned
> > -to another device, so drivers should not cache the value of dev->irq.
> > +This function should be used to undo the effect of pci_enable_msi_range()
> > +or pci_enable_msi_partial(). Calling it restores dev->irq to the pin-based
> > +interrupt number and frees the previously allocated MSIs. The interrupts
> > +may subsequently be assigned to another device, so drivers should not cache
> > +the value of dev->irq.
> >
> > Before calling this function, a device driver must always call free_irq()
> > on any interrupt for which it previously called request_irq().
> > diff --git a/arch/mips/pci/msi-octeon.c b/arch/mips/pci/msi-octeon.c
> > index 2b91b0e..2be7979 100644
> > --- a/arch/mips/pci/msi-octeon.c
> > +++ b/arch/mips/pci/msi-octeon.c
> > @@ -178,7 +178,7 @@ msi_irq_allocated:
> > return 0;
> > }
> >
> > -int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
> > +int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int nvec_mme, int type)
> > {
> > struct msi_desc *entry;
> > int ret;
> > diff --git a/arch/powerpc/kernel/msi.c b/arch/powerpc/kernel/msi.c
> > index 8bbc12d..c60aee3 100644
> > --- a/arch/powerpc/kernel/msi.c
> > +++ b/arch/powerpc/kernel/msi.c
> > @@ -13,7 +13,7 @@
> >
> > #include <asm/machdep.h>
> >
> > -int arch_msi_check_device(struct pci_dev* dev, int nvec, int type)
> > +int arch_msi_check_device(struct pci_dev *dev, int nvec, int nvec_mme, int type)
> > {
> > if (!ppc_md.setup_msi_irqs || !ppc_md.teardown_msi_irqs) {
> > pr_debug("msi: Platform doesn't provide MSI callbacks.\n");
> > @@ -32,7 +32,7 @@ int arch_msi_check_device(struct pci_dev* dev, int nvec, int type)
> > return 0;
> > }
> >
> > -int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
> > +int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int nvec_mme, int type)
> > {
> > return ppc_md.setup_msi_irqs(dev, nvec, type);
> > }
> > diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> > index 9ddc51e..3cf38a8 100644
> > --- a/arch/s390/pci/pci.c
> > +++ b/arch/s390/pci/pci.c
> > @@ -398,7 +398,7 @@ static void zpci_irq_handler(struct airq_struct *airq)
> > }
> > }
> >
> > -int arch_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
> > +int arch_setup_msi_irqs(struct pci_dev *pdev, int nvec, int nvec_mme, int type)
> > {
> > struct zpci_dev *zdev = get_zdev(pdev);
> > unsigned int hwirq, msi_vecs;
> > diff --git a/arch/x86/kernel/x86_init.c b/arch/x86/kernel/x86_init.c
> > index e48b674..b65bf95 100644
> > --- a/arch/x86/kernel/x86_init.c
> > +++ b/arch/x86/kernel/x86_init.c
> > @@ -121,7 +121,7 @@ struct x86_msi_ops x86_msi = {
> > };
> >
> > /* MSI arch specific hooks */
> > -int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
> > +int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int nvec_mme, int type)
> > {
> > return x86_msi.setup_msi_irqs(dev, nvec, type);
> > }
> > diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> > index 27a7e67..0410d9b 100644
> > --- a/drivers/pci/msi.c
> > +++ b/drivers/pci/msi.c
> > @@ -56,7 +56,8 @@ void __weak arch_teardown_msi_irq(unsigned int irq)
> > chip->teardown_irq(chip, irq);
> > }
> >
> > -int __weak arch_msi_check_device(struct pci_dev *dev, int nvec, int type)
> > +int __weak arch_msi_check_device(struct pci_dev *dev,
> > + int nvec, int nvec_mme, int type)
> > {
> > struct msi_chip *chip = dev->bus->msi;
> >
> > @@ -66,7 +67,8 @@ int __weak arch_msi_check_device(struct pci_dev *dev, int nvec, int type)
> > return chip->check_device(chip, dev, nvec, type);
> > }
> >
> > -int __weak arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
> > +int __weak arch_setup_msi_irqs(struct pci_dev *dev,
> > + int nvec, int nvec_mme, int type)
> > {
> > struct msi_desc *entry;
> > int ret;
> > @@ -598,6 +600,7 @@ error_attrs:
> > * msi_capability_init - configure device's MSI capability structure
> > * @dev: pointer to the pci_dev data structure of MSI device function
> > * @nvec: number of interrupts to allocate
> > + * @nvec_mme: number of interrupts to write to Multiple Message Enable register
> > *
> > * Setup the MSI capability structure of the device with the requested
> > * number of interrupts. A return value of zero indicates the successful
> > @@ -605,7 +608,7 @@ error_attrs:
> > * an error, and a positive return value indicates the number of interrupts
> > * which could have been allocated.
> > */
> > -static int msi_capability_init(struct pci_dev *dev, int nvec)
> > +static int msi_capability_init(struct pci_dev *dev, int nvec, int nvec_mme)
> > {
> > struct msi_desc *entry;
> > int ret;
> > @@ -640,7 +643,7 @@ static int msi_capability_init(struct pci_dev *dev, int nvec)
> > list_add_tail(&entry->list, &dev->msi_list);
> >
> > /* Configure MSI capability structure */
> > - ret = arch_setup_msi_irqs(dev, nvec, PCI_CAP_ID_MSI);
> > + ret = arch_setup_msi_irqs(dev, nvec, nvec_mme, PCI_CAP_ID_MSI);
> > if (ret) {
> > msi_mask_irq(entry, mask, ~mask);
> > free_msi_irqs(dev);
> > @@ -758,7 +761,8 @@ static int msix_capability_init(struct pci_dev *dev,
> > if (ret)
> > return ret;
> >
> > - ret = arch_setup_msi_irqs(dev, nvec, PCI_CAP_ID_MSIX);
> > + /* Parameter 'nvec_mme' does not make sense in case of MSI-X */
> > + ret = arch_setup_msi_irqs(dev, nvec, 0, PCI_CAP_ID_MSIX);
> > if (ret)
> > goto out_avail;
> >
> > @@ -812,13 +816,15 @@ out_free:
> > * pci_msi_check_device - check whether MSI may be enabled on a device
> > * @dev: pointer to the pci_dev data structure of MSI device function
> > * @nvec: how many MSIs have been requested ?
> > + * @nvec_mme: how many MSIs write to Multiple Message Enable register ?
> > * @type: are we checking for MSI or MSI-X ?
> > *
> > * Look at global flags, the device itself, and its parent buses
> > * to determine if MSI/-X are supported for the device. If MSI/-X is
> > * supported return 0, else return an error code.
> > **/
> > -static int pci_msi_check_device(struct pci_dev *dev, int nvec, int type)
> > +static int pci_msi_check_device(struct pci_dev *dev,
> > + int nvec, int nvec_mme, int type)
> > {
> > struct pci_bus *bus;
> > int ret;
> > @@ -846,7 +852,7 @@ static int pci_msi_check_device(struct pci_dev *dev, int nvec, int type)
> > if (bus->bus_flags & PCI_BUS_FLAGS_NO_MSI)
> > return -EINVAL;
> >
> > - ret = arch_msi_check_device(dev, nvec, type);
> > + ret = arch_msi_check_device(dev, nvec, nvec_mme, type);
> > if (ret)
> > return ret;
> >
> > @@ -878,6 +884,62 @@ int pci_msi_vec_count(struct pci_dev *dev)
> > }
> > EXPORT_SYMBOL(pci_msi_vec_count);
> >
> > +/**
> > + * pci_enable_msi_partial - configure device's MSI capability structure
> > + * @dev: device to configure
> > + * @nvec: number of interrupts to configure
> > + * @nvec_mme: number of interrupts to write to Multiple Message Enable register
> > + *
> > + * This function tries to allocate @nvec number of interrupts while setup
> > + * device's Multiple Message Enable register with @nvec_mme interrupts.
> > + * It returns a negative errno if an error occurs. If it succeeds, it returns
> > + * zero and updates the @dev's irq member to the lowest new interrupt number;
> > + * the other interrupt numbers allocated to this device are consecutive.
> > + */
> > +int pci_enable_msi_partial(struct pci_dev *dev, int nvec, int nvec_mme)
> > +{
> > + int maxvec;
> > + int rc;
> > +
> > + if (dev->current_state != PCI_D0)
> > + return -EINVAL;
> > +
> > + WARN_ON(!!dev->msi_enabled);
> > +
> > + /* Check whether driver already requested MSI-X irqs */
> > + if (dev->msix_enabled) {
> > + dev_info(&dev->dev, "can't enable MSI "
> > + "(MSI-X already enabled)\n");
> > + return -EINVAL;
> > + }
> > +
> > + if (!is_power_of_2(nvec_mme))
> > + return -EINVAL;
> > + if (nvec > nvec_mme)
> > + return -EINVAL;
> > +
> > + maxvec = pci_msi_vec_count(dev);
> > + if (maxvec < 0)
> > + return maxvec;
> > + else if (nvec_mme > maxvec)
> > + return -EINVAL;
> > +
> > + rc = pci_msi_check_device(dev, nvec, nvec_mme, PCI_CAP_ID_MSI);
> > + if (rc < 0)
> > + return rc;
> > + else if (rc > 0)
> > + return -ENOSPC;
> > +
> > + rc = msi_capability_init(dev, nvec, nvec_mme);
> > + if (rc < 0)
> > + return rc;
> > + else if (rc > 0)
> > + return -ENOSPC;
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(pci_enable_msi_partial);
> > +
> > void pci_msi_shutdown(struct pci_dev *dev)
> > {
> > struct msi_desc *desc;
> > @@ -957,7 +1019,7 @@ int pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries, int nvec)
> > if (!entries || !dev->msix_cap || dev->current_state != PCI_D0)
> > return -EINVAL;
> >
> > - status = pci_msi_check_device(dev, nvec, PCI_CAP_ID_MSIX);
> > + status = pci_msi_check_device(dev, nvec, 0, PCI_CAP_ID_MSIX);
> > if (status)
> > return status;
> >
> > @@ -1110,7 +1172,8 @@ int pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec)
> > nvec = maxvec;
> >
> > do {
> > - rc = pci_msi_check_device(dev, nvec, PCI_CAP_ID_MSI);
> > + rc = pci_msi_check_device(dev, nvec, roundup_pow_of_two(nvec),
> > + PCI_CAP_ID_MSI);
> > if (rc < 0) {
> > return rc;
> > } else if (rc > 0) {
> > @@ -1121,7 +1184,7 @@ int pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec)
> > } while (rc);
> >
> > do {
> > - rc = msi_capability_init(dev, nvec);
> > + rc = msi_capability_init(dev, nvec, roundup_pow_of_two(nvec));
> > if (rc < 0) {
> > return rc;
> > } else if (rc > 0) {
> > diff --git a/include/linux/msi.h b/include/linux/msi.h
> > index 92a2f99..b9f89ee 100644
> > --- a/include/linux/msi.h
> > +++ b/include/linux/msi.h
> > @@ -57,9 +57,10 @@ struct msi_desc {
> > */
> > int arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc);
> > void arch_teardown_msi_irq(unsigned int irq);
> > -int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
> > +int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int nvec_mme, int type);
> > void arch_teardown_msi_irqs(struct pci_dev *dev);
> > -int arch_msi_check_device(struct pci_dev* dev, int nvec, int type);
> > +int arch_msi_check_device(struct pci_dev *dev,
> > + int nvec, int nvec_mme, int type);
> > void arch_restore_msi_irqs(struct pci_dev *dev);
> >
> > void default_teardown_msi_irqs(struct pci_dev *dev);
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 71d9673..7360bd2 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -1184,6 +1184,7 @@ void pci_disable_msix(struct pci_dev *dev);
> > void msi_remove_pci_irq_vectors(struct pci_dev *dev);
> > void pci_restore_msi_state(struct pci_dev *dev);
> > int pci_msi_enabled(void);
> > +int pci_enable_msi_partial(struct pci_dev *dev, int nvec, int nvec_mme);
> > int pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec);
> > static inline int pci_enable_msi_exact(struct pci_dev *dev, int nvec)
> > {
> > @@ -1215,6 +1216,8 @@ static inline void pci_disable_msix(struct pci_dev *dev) { }
> > static inline void msi_remove_pci_irq_vectors(struct pci_dev *dev) { }
> > static inline void pci_restore_msi_state(struct pci_dev *dev) { }
> > static inline int pci_msi_enabled(void) { return 0; }
> > +static int pci_enable_msi_partial(struct pci_dev *dev, int nvec, int nvec_mme)
> > +{ return -ENOSYS; }
> > static inline int pci_enable_msi_range(struct pci_dev *dev, int minvec,
> > int maxvec)
> > { return -ENOSYS; }
> > --
> > 1.7.7.6
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Regards,
Alexander Gordeev
agordeev@redhat.com
^ permalink raw reply
* Re: [PATCH 1/3] PCI/MSI: Add pci_enable_msi_partial()
From: Alexander Gordeev @ 2014-07-04 8:58 UTC (permalink / raw)
To: David Laight
Cc: linux-mips@linux-mips.org, linux-s390@vger.kernel.org,
linux-doc@vger.kernel.org, linux-pci@vger.kernel.org,
x86@kernel.org, linux-kernel@vger.kernel.org,
linux-ide@vger.kernel.org, iommu@lists.linux-foundation.org,
'Bjorn Helgaas', xen-devel@lists.xenproject.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1726BF4E@AcuExch.aculab.com>
On Thu, Jul 03, 2014 at 09:20:52AM +0000, David Laight wrote:
> From: Bjorn Helgaas
> > On Tue, Jun 10, 2014 at 03:10:30PM +0200, Alexander Gordeev wrote:
> > > There are PCI devices that require a particular value written
> > > to the Multiple Message Enable (MME) register while aligned on
> > > power of 2 boundary value of actually used MSI vectors 'nvec'
> > > is a lesser of that MME value:
> > >
> > > roundup_pow_of_two(nvec) < 'Multiple Message Enable'
> > >
> > > However the existing pci_enable_msi_block() interface is not
> > > able to configure such devices, since the value written to the
> > > MME register is calculated from the number of requested MSIs
> > > 'nvec':
> > >
> > > 'Multiple Message Enable' = roundup_pow_of_two(nvec)
> >
> > For MSI, software learns how many vectors a device requests by reading
> > the Multiple Message Capable (MMC) field. This field is encoded, so a
> > device can only request 1, 2, 4, 8, etc., vectors. It's impossible
> > for a device to request 3 vectors; it would have to round up that up
> > to a power of two and request 4 vectors.
> >
> > Software writes similarly encoded values to MME to tell the device how
> > many vectors have been allocated for its use. For example, it's
> > impossible to tell the device that it can use 3 vectors; the OS has to
> > round that up and tell the device it can use 4 vectors.
> >
> > So if I understand correctly, the point of this series is to take
> > advantage of device-specific knowledge, e.g., the device requests 4
> > vectors via MMC, but we "know" the device is only capable of using 3.
> > Moreover, we tell the device via MME that 4 vectors are available, but
> > we've only actually set up 3 of them.
> ...
>
> Even if you do that, you ought to write valid interrupt information
> into the 4th slot (maybe replicating one of the earlier interrupts).
> Then, if the device does raise the 'unexpected' interrupt you don't
> get a write to a random kernel location.
I might be missing something, but we are talking of MSI address space
here, aren't we? I am not getting how we could end up with a 'write'
to a random kernel location when a unclaimed MSI vector sent. We could
only expect a spurious interrupt at worst, which is handled and reported.
Anyway, as I described in my reply to Bjorn, this is not a concern IMO.
> Plausibly something similar should be done when a smaller number of
> interrupts is assigned.
>
> David
>
--
Regards,
Alexander Gordeev
agordeev@redhat.com
^ permalink raw reply
* RE: [PATCH 1/3] PCI/MSI: Add pci_enable_msi_partial()
From: David Laight @ 2014-07-04 9:11 UTC (permalink / raw)
To: 'Alexander Gordeev'
Cc: linux-mips@linux-mips.org, linux-s390@vger.kernel.org,
linux-doc@vger.kernel.org, linux-pci@vger.kernel.org,
x86@kernel.org, linux-kernel@vger.kernel.org,
linux-ide@vger.kernel.org, iommu@lists.linux-foundation.org,
'Bjorn Helgaas', xen-devel@lists.xenproject.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20140704085816.GB12247@dhcp-26-207.brq.redhat.com>
From: Alexander Gordeev
...
> > Even if you do that, you ought to write valid interrupt information
> > into the 4th slot (maybe replicating one of the earlier interrupts).
> > Then, if the device does raise the 'unexpected' interrupt you don't
> > get a write to a random kernel location.
>=20
> I might be missing something, but we are talking of MSI address space
> here, aren't we? I am not getting how we could end up with a 'write'
> to a random kernel location when a unclaimed MSI vector sent. We could
> only expect a spurious interrupt at worst, which is handled and reported.
>=20
> Anyway, as I described in my reply to Bjorn, this is not a concern IMO.
I'm thinking of the following - which might be MSI-X ?
1) Hardware requests some interrupts and tells the host the BAR (and offset=
)
where the 'vectors' should be written.
2) To raise an interrupt the hardware uses the 'vector' as the address
of a normal PCIe write cycle.
So if the hardware requests 4 interrupts, but the driver (believing it
will only use 3) only write 3 vectors, and then the hardware uses the
4th vector it can write to a random location.
Debugging that would be hard!
David
^ permalink raw reply
* Re: [PATCH 1/3] PCI/MSI: Add pci_enable_msi_partial()
From: Alexander Gordeev @ 2014-07-04 9:54 UTC (permalink / raw)
To: David Laight
Cc: linux-mips@linux-mips.org, linux-s390@vger.kernel.org,
linux-doc@vger.kernel.org, linux-pci@vger.kernel.org,
x86@kernel.org, linux-kernel@vger.kernel.org,
linux-ide@vger.kernel.org, iommu@lists.linux-foundation.org,
'Bjorn Helgaas', xen-devel@lists.xenproject.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1726C717@AcuExch.aculab.com>
On Fri, Jul 04, 2014 at 09:11:50AM +0000, David Laight wrote:
> > I might be missing something, but we are talking of MSI address space
> > here, aren't we? I am not getting how we could end up with a 'write'
> > to a random kernel location when a unclaimed MSI vector sent. We could
> > only expect a spurious interrupt at worst, which is handled and reported.
> >
> > Anyway, as I described in my reply to Bjorn, this is not a concern IMO.
>
> I'm thinking of the following - which might be MSI-X ?
> 1) Hardware requests some interrupts and tells the host the BAR (and offset)
> where the 'vectors' should be written.
> 2) To raise an interrupt the hardware uses the 'vector' as the address
> of a normal PCIe write cycle.
>
> So if the hardware requests 4 interrupts, but the driver (believing it
> will only use 3) only write 3 vectors, and then the hardware uses the
> 4th vector it can write to a random location.
>
> Debugging that would be hard!
MSI base address is kind of hardcoded for a platform. A combination of
MSI base address, PCI function number and MSI vector makes a PCI host to
raise interrupt on a CPU. I might be inaccurate in details, but the scenario
you described is impossible AFAICT.
> David
>
>
>
--
Regards,
Alexander Gordeev
agordeev@redhat.com
^ permalink raw reply
* Re: [PATCH 1/3] iommu/fsl: Fix PAMU window size check.
From: Joerg Roedel @ 2014-07-04 10:44 UTC (permalink / raw)
To: Varun Sethi; +Cc: alex.williamson, iommu, linuxppc-dev, linux-kernel
In-Reply-To: <1403618237-26248-2-git-send-email-Varun.Sethi@freescale.com>
On Tue, Jun 24, 2014 at 07:27:15PM +0530, Varun Sethi wrote:
> /* window size is 2^(WSE+1) bytes */
> - return __ffs(addrspace_size) - 1;
> + return fls64(addrspace_size) - 2;
This looks bogus, why do you replace ffs (find-first-bit) by fls
(find-last-bit)?
Joerg
^ permalink raw reply
* Re: [PATCH 2/3] iommu/fsl: Fix the device domain attach condition.
From: Joerg Roedel @ 2014-07-04 10:54 UTC (permalink / raw)
To: Varun Sethi; +Cc: alex.williamson, iommu, linuxppc-dev, linux-kernel
In-Reply-To: <1403618237-26248-3-git-send-email-Varun.Sethi@freescale.com>
Hmm,
On Tue, Jun 24, 2014 at 07:27:16PM +0530, Varun Sethi wrote:
> - old_domain_info = find_domain(dev);
> + old_domain_info = dev->archdata.iommu_domain;
> if (old_domain_info && old_domain_info->domain != dma_domain) {
> spin_unlock_irqrestore(&device_domain_lock, flags);
> detach_device(dev, old_domain_info->domain);
Wouldn't this set dev->archdata.iommu_domain to NULL anyway, so that ...
> @@ -399,7 +394,7 @@ static void attach_device(struct fsl_dma_domain *dma_domain, int liodn, struct d
> * the info for the first LIODN as all
> * LIODNs share the same domain
> */
> - if (!old_domain_info)
> + if (!dev->archdata.iommu_domain)
> dev->archdata.iommu_domain = info;
We already know that it _must_ be NULL here?
> spin_unlock_irqrestore(&device_domain_lock, flags);
This would shrink down the patch to:
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 93072ba..d21b554 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -399,8 +399,7 @@ static void attach_device(struct fsl_dma_domain *dma_domain, int liodn, struct d
* the info for the first LIODN as all
* LIODNs share the same domain
*/
- if (!old_domain_info)
- dev->archdata.iommu_domain = info;
+ dev->archdata.iommu_domain = info;
spin_unlock_irqrestore(&device_domain_lock, flags);
}
^ permalink raw reply related
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