* Re: [PATCH 5/5 v4] KVM: PPC: Bookehv: Get vcpu's last instruction for emulation
From: Alexander Graf @ 2014-07-03 13:53 UTC (permalink / raw)
To: Mihai Caraman, kvm-ppc; +Cc: linuxppc-dev, kvm
In-Reply-To: <1403909347-31622-6-git-send-email-mihai.caraman@freescale.com>
On 28.06.14 00:49, Mihai Caraman wrote:
> On book3e, KVM uses load external pid (lwepx) dedicated instruction to read
> guest last instruction on the exit path. lwepx exceptions (DTLB_MISS, DSI
> and LRAT), generated by loading a guest address, needs to be handled by KVM.
> These exceptions are generated in a substituted guest translation context
> (EPLC[EGS] = 1) from host context (MSR[GS] = 0).
>
> Currently, KVM hooks only interrupts generated from guest context (MSR[GS] = 1),
> doing minimal checks on the fast path to avoid host performance degradation.
> lwepx exceptions originate from host state (MSR[GS] = 0) which implies
> additional checks in DO_KVM macro (beside the current MSR[GS] = 1) by looking
> at the Exception Syndrome Register (ESR[EPID]) and the External PID Load Context
> Register (EPLC[EGS]). Doing this on each Data TLB miss exception is obvious
> too intrusive for the host.
>
> Read guest last instruction from kvmppc_load_last_inst() by searching for the
> physical address and kmap it. This address the TODO for TLB eviction and
> execute-but-not-read entries, and allow us to get rid of lwepx until we are
> able to handle failures.
>
> A simple stress benchmark shows a 1% sys performance degradation compared with
> previous approach (lwepx without failure handling):
>
> time for i in `seq 1 10000`; do /bin/echo > /dev/null; done
>
> real 0m 8.85s
> user 0m 4.34s
> sys 0m 4.48s
>
> vs
>
> real 0m 8.84s
> user 0m 4.36s
> sys 0m 4.44s
>
> An alternative solution, to handle lwepx exceptions in KVM, is to temporary
> highjack the interrupt vector from host. Some cores share host IVOR registers
> between hardware threads, which is the case of FSL e6500, which impose additional
> synchronization logic for this solution to work. The optimization can be addressed
> later on top of this patch.
>
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> ---
> v4:
> - add switch and new function when getting last inst earlier
> - use enum instead of prev semnatic
> - get rid of mas0, optimize mas7_mas3
> - give more context in visible messages
> - check storage attributes mismatch on MMUv2
> - get rid of pfn_valid check
>
> v3:
> - reworked patch description
> - use unaltered kmap addr for kunmap
> - get last instruction before beeing preempted
>
> v2:
> - reworked patch description
> - used pr_* functions
> - addressed cosmetic feedback
>
> arch/powerpc/kvm/booke.c | 44 +++++++++++++++++
> arch/powerpc/kvm/bookehv_interrupts.S | 37 ++++----------
> arch/powerpc/kvm/e500_mmu_host.c | 91 +++++++++++++++++++++++++++++++++++
> 3 files changed, 144 insertions(+), 28 deletions(-)
>
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index 34a42b9..843077b 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -869,6 +869,28 @@ static void kvmppc_restart_interrupt(struct kvm_vcpu *vcpu,
> }
> }
>
> +static int kvmppc_resume_inst_load(struct kvm_run *run, struct kvm_vcpu *vcpu,
> + enum emulation_result emulated, u32 last_inst)
> +{
> + switch (emulated) {
> + case EMULATE_AGAIN:
> + return RESUME_GUEST;
> +
> + case EMULATE_FAIL:
> + pr_debug("%s: load instruction from guest address %lx failed\n",
> + __func__, vcpu->arch.pc);
> + /* For debugging, encode the failing instruction and
> + * report it to userspace. */
> + run->hw.hardware_exit_reason = ~0ULL << 32;
> + run->hw.hardware_exit_reason |= last_inst;
> + kvmppc_core_queue_program(vcpu, ESR_PIL);
> + return RESUME_HOST;
> +
> + default:
> + BUG();
> + }
> +}
> +
> /**
> * kvmppc_handle_exit
> *
> @@ -880,6 +902,8 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
> int r = RESUME_HOST;
> int s;
> int idx;
> + u32 last_inst = KVM_INST_FETCH_FAILED;
> + enum emulation_result emulated = EMULATE_DONE;
>
> /* update before a new last_exit_type is rewritten */
> kvmppc_update_timing_stats(vcpu);
> @@ -887,6 +911,20 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
> /* restart interrupts if they were meant for the host */
> kvmppc_restart_interrupt(vcpu, exit_nr);
>
> + /*
> + * get last instruction before beeing preempted
> + * TODO: for e6500 check also BOOKE_INTERRUPT_LRAT_ERROR & ESR_DATA
> + */
> + switch (exit_nr) {
> + case BOOKE_INTERRUPT_DATA_STORAGE:
> + case BOOKE_INTERRUPT_DTLB_MISS:
> + case BOOKE_INTERRUPT_HV_PRIV:
> + emulated = kvmppc_get_last_inst(vcpu, false, &last_inst);
> + break;
> + default:
> + break;
> + }
> +
> local_irq_enable();
>
> trace_kvm_exit(exit_nr, vcpu);
> @@ -895,6 +933,11 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
> run->exit_reason = KVM_EXIT_UNKNOWN;
> run->ready_for_interrupt_injection = 1;
>
> + if (emulated != EMULATE_DONE) {
> + r = kvmppc_resume_inst_load(run, vcpu, emulated, last_inst);
> + goto out;
> + }
> +
> switch (exit_nr) {
> case BOOKE_INTERRUPT_MACHINE_CHECK:
> printk("MACHINE CHECK: %lx\n", mfspr(SPRN_MCSR));
> @@ -1184,6 +1227,7 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
> BUG();
> }
>
> +out:
> /*
> * To avoid clobbering exit_reason, only check for signals if we
> * aren't already exiting to userspace for some other reason.
> diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S
> index 6ff4480..e000b39 100644
> --- a/arch/powerpc/kvm/bookehv_interrupts.S
> +++ b/arch/powerpc/kvm/bookehv_interrupts.S
> @@ -121,38 +121,14 @@
> 1:
>
> .if \flags & NEED_EMU
> - /*
> - * This assumes you have external PID support.
> - * To support a bookehv CPU without external PID, you'll
> - * need to look up the TLB entry and create a temporary mapping.
> - *
> - * FIXME: we don't currently handle if the lwepx faults. PR-mode
> - * booke doesn't handle it either. Since Linux doesn't use
> - * broadcast tlbivax anymore, the only way this should happen is
> - * if the guest maps its memory execute-but-not-read, or if we
> - * somehow take a TLB miss in the middle of this entry code and
> - * evict the relevant entry. On e500mc, all kernel lowmem is
> - * bolted into TLB1 large page mappings, and we don't use
> - * broadcast invalidates, so we should not take a TLB miss here.
> - *
> - * Later we'll need to deal with faults here. Disallowing guest
> - * mappings that are execute-but-not-read could be an option on
> - * e500mc, but not on chips with an LRAT if it is used.
> - */
> -
> - mfspr r3, SPRN_EPLC /* will already have correct ELPID and EGS */
> PPC_STL r15, VCPU_GPR(R15)(r4)
> PPC_STL r16, VCPU_GPR(R16)(r4)
> PPC_STL r17, VCPU_GPR(R17)(r4)
> PPC_STL r18, VCPU_GPR(R18)(r4)
> PPC_STL r19, VCPU_GPR(R19)(r4)
> - mr r8, r3
> PPC_STL r20, VCPU_GPR(R20)(r4)
> - rlwimi r8, r6, EPC_EAS_SHIFT - MSR_IR_LG, EPC_EAS
> PPC_STL r21, VCPU_GPR(R21)(r4)
> - rlwimi r8, r6, EPC_EPR_SHIFT - MSR_PR_LG, EPC_EPR
> PPC_STL r22, VCPU_GPR(R22)(r4)
> - rlwimi r8, r10, EPC_EPID_SHIFT, EPC_EPID
> PPC_STL r23, VCPU_GPR(R23)(r4)
> PPC_STL r24, VCPU_GPR(R24)(r4)
> PPC_STL r25, VCPU_GPR(R25)(r4)
> @@ -162,10 +138,15 @@
> PPC_STL r29, VCPU_GPR(R29)(r4)
> PPC_STL r30, VCPU_GPR(R30)(r4)
> PPC_STL r31, VCPU_GPR(R31)(r4)
> - mtspr SPRN_EPLC, r8
> - isync
> - lwepx r9, 0, r5
> - mtspr SPRN_EPLC, r3
> +
> + /*
> + * We don't use external PID support. lwepx faults would need to be
> + * handled by KVM and this implies aditional code in DO_KVM (for
> + * DTB_MISS, DSI and LRAT) to check ESR[EPID] and EPLC[EGS] which
> + * is too intrusive for the host. Get last instuction in
> + * kvmppc_get_last_inst().
> + */
> + li r9, KVM_INST_FETCH_FAILED
> stw r9, VCPU_LAST_INST(r4)
> .endif
>
> diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
> index 4b4e8d6..57463e5 100644
> --- a/arch/powerpc/kvm/e500_mmu_host.c
> +++ b/arch/powerpc/kvm/e500_mmu_host.c
> @@ -606,11 +606,102 @@ void kvmppc_mmu_map(struct kvm_vcpu *vcpu, u64 eaddr, gpa_t gpaddr,
> }
> }
>
> +#ifdef CONFIG_KVM_BOOKE_HV
> int kvmppc_load_last_inst(struct kvm_vcpu *vcpu, enum instruction_type type,
> u32 *instr)
> {
> + gva_t geaddr;
> + hpa_t addr;
> + hfn_t pfn;
> + hva_t eaddr;
> + u32 mas1, mas2, mas3;
> + u64 mas7_mas3;
> + struct page *page;
> + unsigned int addr_space, psize_shift;
> + bool pr;
> + unsigned long flags;
> +
> + /* Search TLB for guest pc to get the real address */
> + geaddr = kvmppc_get_pc(vcpu);
> +
> + addr_space = (vcpu->arch.shared->msr & MSR_IS) >> MSR_IR_LG;
> +
> + local_irq_save(flags);
> + mtspr(SPRN_MAS6, (vcpu->arch.pid << MAS6_SPID_SHIFT) | addr_space);
> + mtspr(SPRN_MAS5, MAS5_SGS | vcpu->kvm->arch.lpid);
> + asm volatile("tlbsx 0, %[geaddr]\n" : :
> + [geaddr] "r" (geaddr));
> + mtspr(SPRN_MAS5, 0);
> + mtspr(SPRN_MAS8, 0);
> + mas1 = mfspr(SPRN_MAS1);
> + mas2 = mfspr(SPRN_MAS2);
> + mas3 = mfspr(SPRN_MAS3);
> +#ifdef CONFIG_64BIT
> + mas7_mas3 = mfspr(SPRN_MAS7_MAS3);
> +#else
> + mas7_mas3 = ((u64)mfspr(SPRN_MAS7) << 32) | mas3;
> +#endif
> + local_irq_restore(flags);
> +
> + /*
> + * If the TLB entry for guest pc was evicted, return to the guest.
> + * There are high chances to find a valid TLB entry next time.
> + */
> + if (!(mas1 & MAS1_VALID))
> + return EMULATE_AGAIN;
> +
> + /*
> + * Another thread may rewrite the TLB entry in parallel, don't
> + * execute from the address if the execute permission is not set
> + */
> + pr = vcpu->arch.shared->msr & MSR_PR;
> + if (unlikely((pr && !(mas3 & MAS3_UX)) ||
> + (!pr && !(mas3 & MAS3_SX)))) {
> + pr_debug("%s: Instuction emulation from guest addres %08lx without execute permission\n",
> + __func__, geaddr);
> + return EMULATE_FAIL;
In this case how did we ever get here? Why can't we just evict the entry
and return EMULATE_AGAIN?
> + }
> +
> + /*
> + * The real address will be mapped by a cacheable, memory coherent,
> + * write-back page. Check for mismatches when LRAT is used.
> + */
> + if (has_feature(vcpu, VCPU_FTR_MMU_V2) &&
> + unlikely((mas2 & MAS2_I) || (mas2 & MAS2_W) || !(mas2 & MAS2_M))) {
> + pr_debug("%s: Instuction emulation from guest addres %08lx mismatches storage attributes\n",
> + __func__, geaddr);
> + return EMULATE_FAIL;
Hrm - do we really want to deal with injecting faults here? I'd say it's
ok to just end up in an endless EMULATE_AGAIN loop.
> + }
> +
> + /* Get page size */
> + psize_shift = MAS1_GET_TSIZE(mas1) + 10;
> +
> + /* Map a page and get guest's instruction */
> + addr = (mas7_mas3 & (~0ULL << psize_shift)) |
> + (geaddr & ((1ULL << psize_shift) - 1ULL));
> + pfn = addr >> PAGE_SHIFT;
> +
> + /* Guard us against emulation from devices area */
> + if (unlikely(!page_is_ram(pfn))) {
> + pr_debug("%s: Instruction emulation from non-RAM host addres %08llx is not supported\n",
> + __func__, addr);
> + return EMULATE_FAIL;
Same here :).
> + }
> +
> + page = pfn_to_page(pfn);
> + eaddr = (unsigned long)kmap_atomic(page);
> + *instr = *(u32 *)(eaddr | (addr & ~PAGE_MASK));
> + kunmap_atomic((u32 *)eaddr);
Doesn't kmap_atomic() have to be guarded somehow?
Alex
^ permalink raw reply
* [RFC PATCH 1/4] powerpc/booke64: Add LRAT next and max entries to tlb_core_data structure
From: Mihai Caraman @ 2014-07-03 14:45 UTC (permalink / raw)
To: kvm-ppc; +Cc: Mihai Caraman, linuxppc-dev, kvm
In-Reply-To: <1404398727-12844-1-git-send-email-mihai.caraman@freescale.com>
LRAT (Logical to Real Address Translation) is shared between hw threads.
Add LRAT next and max entries to tlb_core_data structure and initialize them.
Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
---
arch/powerpc/include/asm/mmu-book3e.h | 7 +++++++
arch/powerpc/include/asm/reg_booke.h | 1 +
arch/powerpc/mm/fsl_booke_mmu.c | 8 ++++++++
3 files changed, 16 insertions(+)
diff --git a/arch/powerpc/include/asm/mmu-book3e.h b/arch/powerpc/include/asm/mmu-book3e.h
index 8d24f78..088fd9f 100644
--- a/arch/powerpc/include/asm/mmu-book3e.h
+++ b/arch/powerpc/include/asm/mmu-book3e.h
@@ -217,6 +217,12 @@
#define TLBILX_T_CLASS2 6
#define TLBILX_T_CLASS3 7
+/* LRATCFG bits */
+#define LRATCFG_ASSOC 0xFF000000
+#define LRATCFG_LASIZE 0x00FE0000
+#define LRATCFG_LPID 0x00002000
+#define LRATCFG_NENTRY 0x00000FFF
+
#ifndef __ASSEMBLY__
#include <asm/bug.h>
@@ -294,6 +300,7 @@ struct tlb_core_data {
/* For software way selection, as on Freescale TLB1 */
u8 esel_next, esel_max, esel_first;
+ u8 lrat_next, lrat_max;
};
#ifdef CONFIG_PPC64
diff --git a/arch/powerpc/include/asm/reg_booke.h b/arch/powerpc/include/asm/reg_booke.h
index 464f108..75bda23 100644
--- a/arch/powerpc/include/asm/reg_booke.h
+++ b/arch/powerpc/include/asm/reg_booke.h
@@ -64,6 +64,7 @@
#define SPRN_DVC2 0x13F /* Data Value Compare Register 2 */
#define SPRN_LPID 0x152 /* Logical Partition ID */
#define SPRN_MAS8 0x155 /* MMU Assist Register 8 */
+#define SPRN_LRATCFG 0x156 /* LRAT Configuration Register */
#define SPRN_TLB0PS 0x158 /* TLB 0 Page Size Register */
#define SPRN_TLB1PS 0x159 /* TLB 1 Page Size Register */
#define SPRN_MAS5_MAS6 0x15c /* MMU Assist Register 5 || 6 */
diff --git a/arch/powerpc/mm/fsl_booke_mmu.c b/arch/powerpc/mm/fsl_booke_mmu.c
index 94cd728..6492708 100644
--- a/arch/powerpc/mm/fsl_booke_mmu.c
+++ b/arch/powerpc/mm/fsl_booke_mmu.c
@@ -196,6 +196,14 @@ static unsigned long map_mem_in_cams_addr(phys_addr_t phys, unsigned long virt,
get_paca()->tcd.esel_next = i;
get_paca()->tcd.esel_max = mfspr(SPRN_TLB1CFG) & TLBnCFG_N_ENTRY;
get_paca()->tcd.esel_first = i;
+
+ get_paca()->tcd.lrat_next = 0;
+ if (((mfspr(SPRN_MMUCFG) & MMUCFG_MAVN) == MMUCFG_MAVN_V2) &&
+ (mfspr(SPRN_MMUCFG) & MMUCFG_LRAT)) {
+ get_paca()->tcd.lrat_max = mfspr(SPRN_LRATCFG) & LRATCFG_NENTRY;
+ } else {
+ get_paca()->tcd.lrat_max = 0;
+ }
#endif
return amount_mapped;
--
1.7.11.7
^ permalink raw reply related
* [RFC PATCH 3/4] KVM: PPC: e500: TLB emulation for IND entries
From: Mihai Caraman @ 2014-07-03 14:45 UTC (permalink / raw)
To: kvm-ppc; +Cc: Mihai Caraman, linuxppc-dev, kvm
In-Reply-To: <1404398727-12844-1-git-send-email-mihai.caraman@freescale.com>
Handle indirect entries (IND) in TLB emulation code. Translation size of IND
entries differ from the size of referred Page Tables (Linux guests now use IND
of 2MB for 4KB PTs) and this require careful tweak of the existing logic.
TLB search emulation requires additional search in HW TLB0 (since these entries
are directly added by HTW) and found entries shoud be presented to the guest with
RPN changed from PFN to GFN. There might be more GFNs pointing to the same PFN so
the only way to get the corresponding GFN is to search it in guest's PTE. If IND
entry for the corresponding PT is not available just invalidate guest's ea and
report a tlbsx miss. This patch only implements the invalidation and let a TODO
note for searching HW TLB0.
Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
---
arch/powerpc/include/asm/mmu-book3e.h | 2 +
arch/powerpc/kvm/e500.h | 81 ++++++++++++++++++++++++++++-------
arch/powerpc/kvm/e500_mmu.c | 78 +++++++++++++++++++++++++++------
arch/powerpc/kvm/e500_mmu_host.c | 31 ++++++++++++--
arch/powerpc/kvm/e500mc.c | 53 +++++++++++++++++++++--
5 files changed, 211 insertions(+), 34 deletions(-)
diff --git a/arch/powerpc/include/asm/mmu-book3e.h b/arch/powerpc/include/asm/mmu-book3e.h
index ac6acf7..e482ad8 100644
--- a/arch/powerpc/include/asm/mmu-book3e.h
+++ b/arch/powerpc/include/asm/mmu-book3e.h
@@ -59,6 +59,7 @@
#define MAS1_IPROT 0x40000000
#define MAS1_TID(x) (((x) << 16) & 0x3FFF0000)
#define MAS1_IND 0x00002000
+#define MAS1_IND_SHIFT 13
#define MAS1_TS 0x00001000
#define MAS1_TSIZE_MASK 0x00000f80
#define MAS1_TSIZE_SHIFT 7
@@ -94,6 +95,7 @@
#define MAS4_TLBSEL_MASK MAS0_TLBSEL_MASK
#define MAS4_TLBSELD(x) MAS0_TLBSEL(x)
#define MAS4_INDD 0x00008000 /* Default IND */
+#define MAS4_INDD_SHIFT 15
#define MAS4_TSIZED(x) MAS1_TSIZE(x)
#define MAS4_X0D 0x00000040
#define MAS4_X1D 0x00000020
diff --git a/arch/powerpc/kvm/e500.h b/arch/powerpc/kvm/e500.h
index a326178..70a556d 100644
--- a/arch/powerpc/kvm/e500.h
+++ b/arch/powerpc/kvm/e500.h
@@ -148,6 +148,22 @@ unsigned int kvmppc_e500_get_sid(struct kvmppc_vcpu_e500 *vcpu_e500,
unsigned int pr, int avoid_recursion);
#endif
+static inline bool has_feature(const struct kvm_vcpu *vcpu,
+ enum vcpu_ftr ftr)
+{
+ bool has_ftr;
+
+ switch (ftr) {
+ case VCPU_FTR_MMU_V2:
+ has_ftr = ((vcpu->arch.mmucfg & MMUCFG_MAVN) == MMUCFG_MAVN_V2);
+ break;
+
+ default:
+ return false;
+ }
+ return has_ftr;
+}
+
/* TLB helper functions */
static inline unsigned int
get_tlb_size(const struct kvm_book3e_206_tlb_entry *tlbe)
@@ -207,6 +223,16 @@ get_tlb_tsize(const struct kvm_book3e_206_tlb_entry *tlbe)
return (tlbe->mas1 & MAS1_TSIZE_MASK) >> MAS1_TSIZE_SHIFT;
}
+static inline unsigned int
+get_tlb_ind(const struct kvm_vcpu *vcpu,
+ const struct kvm_book3e_206_tlb_entry *tlbe)
+{
+ if (has_feature(vcpu, VCPU_FTR_MMU_V2))
+ return (tlbe->mas1 & MAS1_IND) >> MAS1_IND_SHIFT;
+
+ return 0;
+}
+
static inline unsigned int get_cur_pid(struct kvm_vcpu *vcpu)
{
return vcpu->arch.pid & 0xff;
@@ -232,6 +258,30 @@ static inline unsigned int get_cur_sas(const struct kvm_vcpu *vcpu)
return vcpu->arch.shared->mas6 & 0x1;
}
+static inline unsigned int get_cur_ind(const struct kvm_vcpu *vcpu)
+{
+ if (has_feature(vcpu, VCPU_FTR_MMU_V2))
+ return (vcpu->arch.shared->mas1 & MAS1_IND) >> MAS1_IND_SHIFT;
+
+ return 0;
+}
+
+static inline unsigned int get_cur_indd(const struct kvm_vcpu *vcpu)
+{
+ if (has_feature(vcpu, VCPU_FTR_MMU_V2))
+ return (vcpu->arch.shared->mas4 & MAS4_INDD) >> MAS4_INDD_SHIFT;
+
+ return 0;
+}
+
+static inline unsigned int get_cur_sind(const struct kvm_vcpu *vcpu)
+{
+ if (has_feature(vcpu, VCPU_FTR_MMU_V2))
+ return (vcpu->arch.shared->mas6 & MAS6_SIND) >> MAS6_SIND_SHIFT;
+
+ return 0;
+}
+
static inline unsigned int get_tlb_tlbsel(const struct kvm_vcpu *vcpu)
{
/*
@@ -286,6 +336,22 @@ void kvmppc_e500_tlbil_one(struct kvmppc_vcpu_e500 *vcpu_e500,
void kvmppc_e500_tlbil_all(struct kvmppc_vcpu_e500 *vcpu_e500);
#ifdef CONFIG_KVM_BOOKE_HV
+void inval_tlb_on_host(struct kvm_vcpu *vcpu, int type, int pid);
+
+void inval_ea_on_host(struct kvm_vcpu *vcpu, gva_t ea, int pid, int sas,
+ int sind);
+#else
+/* TLB is fully virtualized */
+static inline void inval_tlb_on_host(struct kvm_vcpu *vcpu,
+ int type, int pid)
+{}
+
+static inline void inval_ea_on_host(struct kvm_vcpu *vcpu, gva_t ea, int pid,
+ int sas, int sind)
+{}
+#endif
+
+#ifdef CONFIG_KVM_BOOKE_HV
#define kvmppc_e500_get_tlb_stid(vcpu, gtlbe) get_tlb_tid(gtlbe)
#define get_tlbmiss_tid(vcpu) get_cur_pid(vcpu)
#define get_tlb_sts(gtlbe) (gtlbe->mas1 & MAS1_TS)
@@ -304,19 +370,4 @@ static inline unsigned int get_tlbmiss_tid(struct kvm_vcpu *vcpu)
/* Force TS=1 for all guest mappings. */
#define get_tlb_sts(gtlbe) (MAS1_TS)
#endif /* !BOOKE_HV */
-
-static inline bool has_feature(const struct kvm_vcpu *vcpu,
- enum vcpu_ftr ftr)
-{
- bool has_ftr;
- switch (ftr) {
- case VCPU_FTR_MMU_V2:
- has_ftr = ((vcpu->arch.mmucfg & MMUCFG_MAVN) == MMUCFG_MAVN_V2);
- break;
- default:
- return false;
- }
- return has_ftr;
-}
-
#endif /* KVM_E500_H */
diff --git a/arch/powerpc/kvm/e500_mmu.c b/arch/powerpc/kvm/e500_mmu.c
index 50860e9..b775e6a 100644
--- a/arch/powerpc/kvm/e500_mmu.c
+++ b/arch/powerpc/kvm/e500_mmu.c
@@ -81,7 +81,8 @@ static unsigned int get_tlb_esel(struct kvm_vcpu *vcpu, int tlbsel)
/* Search the guest TLB for a matching entry. */
static int kvmppc_e500_tlb_index(struct kvmppc_vcpu_e500 *vcpu_e500,
- gva_t eaddr, int tlbsel, unsigned int pid, int as)
+ gva_t eaddr, int tlbsel, unsigned int pid, int as,
+ int sind)
{
int size = vcpu_e500->gtlb_params[tlbsel].entries;
unsigned int set_base, offset;
@@ -120,6 +121,9 @@ static int kvmppc_e500_tlb_index(struct kvmppc_vcpu_e500 *vcpu_e500,
if (get_tlb_ts(tlbe) != as && as != -1)
continue;
+ if (sind != -1 && get_tlb_ind(&vcpu_e500->vcpu, tlbe) != sind)
+ continue;
+
return set_base + i;
}
@@ -130,25 +134,28 @@ static inline void kvmppc_e500_deliver_tlb_miss(struct kvm_vcpu *vcpu,
gva_t eaddr, int as)
{
struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu);
- unsigned int victim, tsized;
+ unsigned int victim, tsized, indd;
int tlbsel;
/* since we only have two TLBs, only lower bit is used. */
tlbsel = (vcpu->arch.shared->mas4 >> 28) & 0x1;
victim = (tlbsel == 0) ? gtlb0_get_next_victim(vcpu_e500) : 0;
tsized = (vcpu->arch.shared->mas4 >> 7) & 0x1f;
+ indd = get_cur_indd(vcpu);
vcpu->arch.shared->mas0 = MAS0_TLBSEL(tlbsel) | MAS0_ESEL(victim)
| MAS0_NV(vcpu_e500->gtlb_nv[tlbsel]);
vcpu->arch.shared->mas1 = MAS1_VALID | (as ? MAS1_TS : 0)
| MAS1_TID(get_tlbmiss_tid(vcpu))
- | MAS1_TSIZE(tsized);
+ | MAS1_TSIZE(tsized)
+ | (indd << MAS1_IND_SHIFT);
vcpu->arch.shared->mas2 = (eaddr & MAS2_EPN)
| (vcpu->arch.shared->mas4 & MAS2_ATTRIB_MASK);
vcpu->arch.shared->mas7_3 &= MAS3_U0 | MAS3_U1 | MAS3_U2 | MAS3_U3;
vcpu->arch.shared->mas6 = (vcpu->arch.shared->mas6 & MAS6_SPID1)
| (get_cur_pid(vcpu) << 16)
- | (as ? MAS6_SAS : 0);
+ | (as ? MAS6_SAS : 0)
+ | (indd << MAS6_SIND_SHIFT);
}
static void kvmppc_recalc_tlb1map_range(struct kvmppc_vcpu_e500 *vcpu_e500)
@@ -264,12 +271,12 @@ int kvmppc_e500_emul_tlbivax(struct kvm_vcpu *vcpu, gva_t ea)
} else {
ea &= 0xfffff000;
esel = kvmppc_e500_tlb_index(vcpu_e500, ea, tlbsel,
- get_cur_pid(vcpu), -1);
+ get_cur_pid(vcpu), -1, get_cur_sind(vcpu));
if (esel >= 0)
kvmppc_e500_gtlbe_invalidate(vcpu_e500, tlbsel, esel);
}
- /* Invalidate all host shadow mappings */
+ /* Invalidate all host shadow mappings including those set by HTW */
kvmppc_core_flush_tlb(&vcpu_e500->vcpu);
return EMULATE_DONE;
@@ -280,6 +287,7 @@ static void tlbilx_all(struct kvmppc_vcpu_e500 *vcpu_e500, int tlbsel,
{
struct kvm_book3e_206_tlb_entry *tlbe;
int tid, esel;
+ int sind = get_cur_sind(&vcpu_e500->vcpu);
/* invalidate all entries */
for (esel = 0; esel < vcpu_e500->gtlb_params[tlbsel].entries; esel++) {
@@ -290,21 +298,32 @@ static void tlbilx_all(struct kvmppc_vcpu_e500 *vcpu_e500, int tlbsel,
kvmppc_e500_gtlbe_invalidate(vcpu_e500, tlbsel, esel);
}
}
+
+ /* Invalidate enties added by HTW */
+ if (has_feature(&vcpu_e500->vcpu, VCPU_FTR_MMU_V2) && (!sind))
+ inval_tlb_on_host(&vcpu_e500->vcpu, type, pid);
}
static void tlbilx_one(struct kvmppc_vcpu_e500 *vcpu_e500, int pid,
gva_t ea)
{
int tlbsel, esel;
+ int sas = get_cur_sas(&vcpu_e500->vcpu);
+ int sind = get_cur_sind(&vcpu_e500->vcpu);
for (tlbsel = 0; tlbsel < 2; tlbsel++) {
- esel = kvmppc_e500_tlb_index(vcpu_e500, ea, tlbsel, pid, -1);
+ esel = kvmppc_e500_tlb_index(vcpu_e500, ea, tlbsel, pid, -1,
+ sind);
if (esel >= 0) {
inval_gtlbe_on_host(vcpu_e500, tlbsel, esel);
kvmppc_e500_gtlbe_invalidate(vcpu_e500, tlbsel, esel);
break;
}
}
+
+ /* Invalidate enties added by HTW */
+ if (has_feature(&vcpu_e500->vcpu, VCPU_FTR_MMU_V2) && (!sind))
+ inval_ea_on_host(&vcpu_e500->vcpu, ea, pid, sas, sind);
}
int kvmppc_e500_emul_tlbilx(struct kvm_vcpu *vcpu, int type, gva_t ea)
@@ -350,7 +369,8 @@ int kvmppc_e500_emul_tlbsx(struct kvm_vcpu *vcpu, gva_t ea)
struct kvm_book3e_206_tlb_entry *gtlbe = NULL;
for (tlbsel = 0; tlbsel < 2; tlbsel++) {
- esel = kvmppc_e500_tlb_index(vcpu_e500, ea, tlbsel, pid, as);
+ esel = kvmppc_e500_tlb_index(vcpu_e500, ea, tlbsel, pid, as,
+ get_cur_sind(vcpu));
if (esel >= 0) {
gtlbe = get_entry(vcpu_e500, tlbsel, esel);
break;
@@ -368,6 +388,23 @@ int kvmppc_e500_emul_tlbsx(struct kvm_vcpu *vcpu, gva_t ea)
} else {
int victim;
+ if (has_feature(vcpu, VCPU_FTR_MMU_V2) &&
+ get_cur_sind(vcpu) != 1) {
+ /*
+ * TLB0 entries are not cached in KVM being written
+ * directly by HTW. TLB0 entry found in HW TLB0 needs
+ * to be presented to the guest with RPN changed from
+ * PFN to GFN. There might be more GFNs pointing to the
+ * same PFN so the only way to get the corresponding GFN
+ * is to search it in guest's PTE. If IND entry for the
+ * corresponding PT is not available just invalidate
+ * guest's ea and report a tlbsx miss.
+ *
+ * TODO: search ea in HW TLB0
+ */
+ inval_ea_on_host(vcpu, ea, pid, as, 0);
+ }
+
/* since we only have two TLBs, only lower bit is used. */
tlbsel = vcpu->arch.shared->mas4 >> 28 & 0x1;
victim = (tlbsel == 0) ? gtlb0_get_next_victim(vcpu_e500) : 0;
@@ -378,7 +415,8 @@ int kvmppc_e500_emul_tlbsx(struct kvm_vcpu *vcpu, gva_t ea)
vcpu->arch.shared->mas1 =
(vcpu->arch.shared->mas6 & MAS6_SPID0)
| (vcpu->arch.shared->mas6 & (MAS6_SAS ? MAS1_TS : 0))
- | (vcpu->arch.shared->mas4 & MAS4_TSIZED(~0));
+ | (vcpu->arch.shared->mas4 & MAS4_TSIZED(~0))
+ | (get_cur_indd(vcpu) << MAS1_IND_SHIFT);
vcpu->arch.shared->mas2 &= MAS2_EPN;
vcpu->arch.shared->mas2 |= vcpu->arch.shared->mas4 &
MAS2_ATTRIB_MASK;
@@ -396,7 +434,7 @@ int kvmppc_e500_emul_tlbwe(struct kvm_vcpu *vcpu)
struct kvm_book3e_206_tlb_entry *gtlbe;
int tlbsel, esel;
int recal = 0;
- int idx;
+ int idx, tsize;
tlbsel = get_tlb_tlbsel(vcpu);
esel = get_tlb_esel(vcpu, tlbsel);
@@ -411,9 +449,17 @@ int kvmppc_e500_emul_tlbwe(struct kvm_vcpu *vcpu)
}
gtlbe->mas1 = vcpu->arch.shared->mas1;
+
gtlbe->mas2 = vcpu->arch.shared->mas2;
+ /* EPN offset bits should be zero, fix early versions of Linux HTW */
+ if (get_cur_ind(vcpu)) {
+ tsize = (vcpu->arch.shared->mas1 & MAS1_TSIZE_MASK) >>
+ MAS1_TSIZE_SHIFT;
+ gtlbe->mas2 &= MAS2_EPN_MASK(tsize) | (~MAS2_EPN);
+ }
if (!(vcpu->arch.shared->msr & MSR_CM))
gtlbe->mas2 &= 0xffffffffUL;
+
gtlbe->mas7_3 = vcpu->arch.shared->mas7_3;
trace_kvm_booke206_gtlb_write(vcpu->arch.shared->mas0, gtlbe->mas1,
@@ -460,7 +506,8 @@ static int kvmppc_e500_tlb_search(struct kvm_vcpu *vcpu,
int esel, tlbsel;
for (tlbsel = 0; tlbsel < 2; tlbsel++) {
- esel = kvmppc_e500_tlb_index(vcpu_e500, eaddr, tlbsel, pid, as);
+ esel = kvmppc_e500_tlb_index(vcpu_e500, eaddr, tlbsel, pid, as,
+ -1);
if (esel >= 0)
return index_of(tlbsel, esel);
}
@@ -531,7 +578,14 @@ gpa_t kvmppc_mmu_xlate(struct kvm_vcpu *vcpu, unsigned int index,
u64 pgmask;
gtlbe = get_entry(vcpu_e500, tlbsel_of(index), esel_of(index));
- pgmask = get_tlb_bytes(gtlbe) - 1;
+ /*
+ * Use 4095 for page mask for IND enties:
+ * (1ULL << (10 + BOOK3E_PAGESZ_4K)) - 1
+ */
+ if (get_tlb_ind(vcpu, gtlbe))
+ pgmask = 4095;
+ else
+ pgmask = get_tlb_bytes(gtlbe) - 1;
return get_tlb_raddr(gtlbe) | (eaddr & pgmask);
}
diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index be1454b..60bf272 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -438,7 +438,8 @@ static void kvmppc_e500_setup_stlbe(
BUG_ON(!(ref->flags & E500_TLB_VALID));
/* Force IPROT=0 for all guest mappings. */
- stlbe->mas1 = MAS1_TSIZE(tsize) | get_tlb_sts(gtlbe) | MAS1_VALID;
+ stlbe->mas1 = MAS1_TSIZE(tsize) | get_tlb_sts(gtlbe) | MAS1_VALID |
+ (get_tlb_ind(vcpu, gtlbe) << MAS1_IND_SHIFT);
stlbe->mas2 = (gvaddr & MAS2_EPN) | (ref->flags & E500_TLB_MAS2_ATTR);
stlbe->mas7_3 = ((u64)pfn << PAGE_SHIFT) |
e500_shadow_mas3_attrib(gtlbe->mas7_3, pr);
@@ -465,6 +466,7 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
pte_t *ptep;
unsigned int wimg = 0;
pgd_t *pgdir;
+ int ind;
/* used to check for invalidations in progress */
mmu_seq = kvm->mmu_notifier_seq;
@@ -481,6 +483,15 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
slot = gfn_to_memslot(vcpu_e500->vcpu.kvm, gfn);
hva = gfn_to_hva_memslot(slot, gfn);
+ /*
+ * An IND entry refer a Page Table which have a different size
+ * then the translation size.
+ * page size bytes = (tsize bytes / 4KB) * 8 bytes
+ * so we have
+ * psize = tsize - BOOK3E_PAGESZ_4K - 7;
+ */
+ ind = get_tlb_ind(&vcpu_e500->vcpu, gtlbe);
+
if (tlbsel == 1) {
struct vm_area_struct *vma;
down_read(¤t->mm->mmap_sem);
@@ -516,12 +527,17 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
tsize = (gtlbe->mas1 & MAS1_TSIZE_MASK) >>
MAS1_TSIZE_SHIFT;
+ if (ind)
+ tsize -= BOOK3E_PAGESZ_4K + 7;
/*
* e500 doesn't implement the lowest tsize bit,
* or 1K pages.
*/
- tsize = max(BOOK3E_PAGESZ_4K, tsize & ~1);
+ if (!has_feature(&vcpu_e500->vcpu, VCPU_FTR_MMU_V2))
+ tsize &= ~1;
+
+ tsize = max(BOOK3E_PAGESZ_4K, tsize);
/*
* Now find the largest tsize (up to what the guest
@@ -555,6 +571,8 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
tsize = (gtlbe->mas1 & MAS1_TSIZE_MASK) >>
MAS1_TSIZE_SHIFT;
+ if (ind)
+ tsize -= BOOK3E_PAGESZ_4K + 7;
/*
* Take the largest page size that satisfies both host
@@ -566,7 +584,10 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
* e500 doesn't implement the lowest tsize bit,
* or 1K pages.
*/
- tsize = max(BOOK3E_PAGESZ_4K, tsize & ~1);
+ if (!has_feature(&vcpu_e500->vcpu, VCPU_FTR_MMU_V2))
+ tsize &= ~1;
+
+ tsize = max(BOOK3E_PAGESZ_4K, tsize);
}
up_read(¤t->mm->mmap_sem);
@@ -606,6 +627,10 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
}
kvmppc_e500_ref_setup(ref, gtlbe, pfn, wimg);
+ /* Restore translation size for indirect entries */
+ if (ind)
+ tsize += BOOK3E_PAGESZ_4K + 7;
+
kvmppc_e500_setup_stlbe(&vcpu_e500->vcpu, gtlbe, tsize,
ref, gvaddr, stlbe);
diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
index 5622d9a..933a4cf 100644
--- a/arch/powerpc/kvm/e500mc.c
+++ b/arch/powerpc/kvm/e500mc.c
@@ -58,7 +58,7 @@ void kvmppc_set_pending_interrupt(struct kvm_vcpu *vcpu, enum int_class type)
void kvmppc_e500_tlbil_one(struct kvmppc_vcpu_e500 *vcpu_e500,
struct kvm_book3e_206_tlb_entry *gtlbe)
{
- unsigned int tid, ts;
+ unsigned int tid, ts, ind;
gva_t eaddr;
u32 val, lpid;
unsigned long flags;
@@ -66,9 +66,10 @@ void kvmppc_e500_tlbil_one(struct kvmppc_vcpu_e500 *vcpu_e500,
ts = get_tlb_ts(gtlbe);
tid = get_tlb_tid(gtlbe);
lpid = vcpu_e500->vcpu.kvm->arch.lpid;
+ ind = get_tlb_ind(&vcpu_e500->vcpu, gtlbe);
/* We search the host TLB to invalidate its shadow TLB entry */
- val = (tid << 16) | ts;
+ val = (tid << 16) | ts | (ind << MAS6_SIND_SHIFT);
eaddr = get_tlb_eaddr(gtlbe);
local_irq_save(flags);
@@ -90,16 +91,60 @@ void kvmppc_e500_tlbil_one(struct kvmppc_vcpu_e500 *vcpu_e500,
local_irq_restore(flags);
}
-void kvmppc_e500_tlbil_all(struct kvmppc_vcpu_e500 *vcpu_e500)
+void inval_ea_on_host(struct kvm_vcpu *vcpu, gva_t ea, int pid, int sas,
+ int sind)
+{
+ unsigned long flags;
+
+ local_irq_save(flags);
+ mtspr(SPRN_MAS5, MAS5_SGS | vcpu->kvm->arch.lpid);
+ mtspr(SPRN_MAS6, (pid << MAS6_SPID_SHIFT) |
+ sas | (sind << MAS6_SIND_SHIFT));
+ asm volatile("tlbilx 3, 0, %[ea]\n" : : [ea] "r" (ea));
+ mtspr(SPRN_MAS5, 0);
+ isync();
+
+ local_irq_restore(flags);
+}
+
+void kvmppc_e500_tlbil_pid(struct kvm_vcpu *vcpu, int pid)
+{
+ unsigned long flags;
+
+ local_irq_save(flags);
+ mtspr(SPRN_MAS5, MAS5_SGS | vcpu->kvm->arch.lpid);
+ mtspr(SPRN_MAS6, pid << MAS6_SPID_SHIFT);
+ asm volatile("tlbilxpid");
+ mtspr(SPRN_MAS5, 0);
+ isync();
+
+ local_irq_restore(flags);
+}
+
+void kvmppc_e500_tlbil_lpid(struct kvm_vcpu *vcpu)
{
unsigned long flags;
local_irq_save(flags);
- mtspr(SPRN_MAS5, MAS5_SGS | vcpu_e500->vcpu.kvm->arch.lpid);
+ mtspr(SPRN_MAS5, MAS5_SGS | vcpu->kvm->arch.lpid);
asm volatile("tlbilxlpid");
mtspr(SPRN_MAS5, 0);
+ isync();
+
local_irq_restore(flags);
+}
+void inval_tlb_on_host(struct kvm_vcpu *vcpu, int type, int pid)
+{
+ if (type == 0)
+ kvmppc_e500_tlbil_lpid(vcpu);
+ else
+ kvmppc_e500_tlbil_pid(vcpu, pid);
+}
+
+void kvmppc_e500_tlbil_all(struct kvmppc_vcpu_e500 *vcpu_e500)
+{
+ kvmppc_e500_tlbil_lpid(&vcpu_e500->vcpu);
kvmppc_lrat_invalidate(&vcpu_e500->vcpu);
}
--
1.7.11.7
^ permalink raw reply related
* [RFC PATCH 2/4] KVM: PPC: Book3E: Handle LRAT error exception
From: Mihai Caraman @ 2014-07-03 14:45 UTC (permalink / raw)
To: kvm-ppc; +Cc: Mihai Caraman, linuxppc-dev, kvm
In-Reply-To: <1404398727-12844-1-git-send-email-mihai.caraman@freescale.com>
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__);
+ break;
+ }
+
+ gfn = (vcpu->arch.fault_lper & LPER_ALPN) >> LPER_ALPN_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);
+ } 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)
+{
+ int this, next;
+
+ this = local_paca->tcd.lrat_next;
+ next = (this + 1) % local_paca->tcd.lrat_max;
+ 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(); */
+
+ 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);
+}
+
+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)
--
1.7.11.7
^ permalink raw reply related
* [RFC PATCH 0/4] KVM Book3E support for HTW guests
From: Mihai Caraman @ 2014-07-03 14:45 UTC (permalink / raw)
To: kvm-ppc; +Cc: Mihai Caraman, linuxppc-dev, kvm
KVM Book3E support for Hardware Page Tablewalk enabled guests.
Mihai Caraman (4):
powerpc/booke64: Add LRAT next and max entries to tlb_core_data
structure
KVM: PPC: Book3E: Handle LRAT error exception
KVM: PPC: e500: TLB emulation for IND entries
KVM: PPC: e500mc: Advertise E.PT to support HTW guests
arch/powerpc/include/asm/kvm_host.h | 1 +
arch/powerpc/include/asm/kvm_ppc.h | 2 +
arch/powerpc/include/asm/mmu-book3e.h | 12 +++
arch/powerpc/include/asm/reg_booke.h | 14 +++
arch/powerpc/kernel/asm-offsets.c | 1 +
arch/powerpc/kvm/booke.c | 40 +++++++++
arch/powerpc/kvm/bookehv_interrupts.S | 9 +-
arch/powerpc/kvm/e500.h | 81 ++++++++++++++----
arch/powerpc/kvm/e500_mmu.c | 84 ++++++++++++++----
arch/powerpc/kvm/e500_mmu_host.c | 156 +++++++++++++++++++++++++++++++++-
arch/powerpc/kvm/e500mc.c | 55 +++++++++++-
arch/powerpc/mm/fsl_booke_mmu.c | 8 ++
12 files changed, 423 insertions(+), 40 deletions(-)
--
1.7.11.7
^ permalink raw reply
* [RFC PATCH 4/4] KVM: PPC: e500mc: Advertise E.PT to support HTW guests
From: Mihai Caraman @ 2014-07-03 14:45 UTC (permalink / raw)
To: kvm-ppc; +Cc: Mihai Caraman, linuxppc-dev, kvm
In-Reply-To: <1404398727-12844-1-git-send-email-mihai.caraman@freescale.com>
Enable E.PT for vcpus with MMU MAV 2.0 to support Hardware Page Tablewalk (HTW)
in guests.
Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
---
arch/powerpc/kvm/e500_mmu.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu.c b/arch/powerpc/kvm/e500_mmu.c
index b775e6a..1de0cd6 100644
--- a/arch/powerpc/kvm/e500_mmu.c
+++ b/arch/powerpc/kvm/e500_mmu.c
@@ -945,11 +945,7 @@ static int vcpu_mmu_init(struct kvm_vcpu *vcpu,
vcpu->arch.tlbps[1] = mfspr(SPRN_TLB1PS);
vcpu->arch.mmucfg &= ~MMUCFG_LRAT;
-
- /* Guest mmu emulation currently doesn't handle E.PT */
- vcpu->arch.eptcfg = 0;
- vcpu->arch.tlbcfg[0] &= ~TLBnCFG_PT;
- vcpu->arch.tlbcfg[1] &= ~TLBnCFG_IND;
+ vcpu->arch.eptcfg = mfspr(SPRN_EPTCFG);
}
return 0;
--
1.7.11.7
^ permalink raw reply related
* RE: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
From: mihai.caraman @ 2014-07-03 15:25 UTC (permalink / raw)
To: Alexander Graf, Scott Wood, kvm-ppc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org
In-Reply-To: <53B54AAD.4040609@suse.de>
> -----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
>=20
>=20
> 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
>=20
> 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.
>=20
> > #define BOOKE_INTERRUPT_SPE_FP_ROUND 34
> > #define BOOKE_INTERRUPT_PERFORMANCE_MONITOR 35
> > #define BOOKE_INTERRUPT_DOORBELL 36
> > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> > index ab62109..3c86d9b 100644
> > --- a/arch/powerpc/kvm/booke.c
> > +++ b/arch/powerpc/kvm/booke.c
> > @@ -388,8 +388,8 @@ static int kvmppc_booke_irqprio_deliver(struct
> kvm_vcpu *vcpu,
> > case BOOKE_IRQPRIO_ITLB_MISS:
> > case BOOKE_IRQPRIO_SYSCALL:
> > case BOOKE_IRQPRIO_FP_UNAVAIL:
> > - case BOOKE_IRQPRIO_SPE_UNAVAIL:
> > - case BOOKE_IRQPRIO_SPE_FP_DATA:
> > + case BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL:
> > + case BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST:
>=20
> #ifdef CONFIG_KVM_E500V2
> case ...SPE:
> #else
> case ..ALTIVEC:
> #endif
-Mike
^ permalink raw reply
* Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
From: Alexander Graf @ 2014-07-03 15:30 UTC (permalink / raw)
To: mihai.caraman@freescale.com, Scott Wood, kvm-ppc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org
In-Reply-To: <2399bd1f8fde430a945311ff27c3f5c3@BY2PR03MB508.namprd03.prod.outlook.com>
On 03.07.14 17:25, mihai.caraman@freescale.com 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
Uh, mind to point me to an email where I said I like the approach? :)
> different defines with same values (we specifically mapped them to the
> hardware interrupt numbers). We already upstreamed the necessary changes
Yes, I think that'd end up the most readable flow of things.
> in the kernel. Scott, please share your opinion here.
I'm not going to be religious about it, but names like
"BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST" are
1) too long
2) too ambiguous
It just means the code gets harder to read. Any way we can take to
simplify the code flow is a win IMHO. And if I don't even remotely have
to consider SPE when reading an Altivec path, I think that's a good
thing :).
Alex
^ permalink raw reply
* RE: [PATCH 3/6 v2] KVM: PPC: Book3E: Increase FPU laziness
From: mihai.caraman @ 2014-07-03 15:46 UTC (permalink / raw)
To: Alexander Graf, kvm-ppc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org
In-Reply-To: <53B54C8A.1060209@suse.de>
> -----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
>=20
>=20
> 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 vcp=
u
sched in if the kernel unloaded unit's guest state for another competing ho=
st
process we do this when we enter the guest.
-Mike
^ permalink raw reply
* RE: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
From: mihai.caraman @ 2014-07-03 15:53 UTC (permalink / raw)
To: Alexander Graf, Scott Wood, kvm-ppc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org
In-Reply-To: <53B57722.9020205@suse.de>
> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Thursday, July 03, 2014 6:31 PM
> To: Caraman Mihai Claudiu-B02008; Wood Scott-B07421; 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
>=20
>=20
> On 03.07.14 17:25, mihai.caraman@freescale.com 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
>=20
> Uh, mind to point me to an email where I said I like the approach? :)
You tacitly approved it in this thread ... I did not say you like it :)
https://lists.ozlabs.org/pipermail/linuxppc-dev/2013-July/108501.html
-Mike
^ permalink raw reply
* RE: [PATCH 4/6 v2] KVM: PPC: Book3E: Add AltiVec support
From: mihai.caraman @ 2014-07-03 15:58 UTC (permalink / raw)
To: Alexander Graf, kvm-ppc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org
In-Reply-To: <53B54D44.2010007@suse.de>
> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Thursday, July 03, 2014 3:32 PM
> To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org
> Cc: kvm@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH 4/6 v2] KVM: PPC: Book3E: Add AltiVec support
>=20
>=20
> On 30.06.14 17:34, Mihai Caraman wrote:
> > Add KVM Book3E AltiVec support. KVM Book3E FPU support gracefully reuse
> host
> > infrastructure so follow the same approach for AltiVec.
> >
> > Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>=20
> Same comment here - I fail to see how we refetch Altivec state after a
> context switch.
See previous comment. I also run my usual Altivec stress test consisting in
a guest and host process running affine to a physical core an competing for
the same unit's resources using different data sets.
-Mike
^ permalink raw reply
* RE: [PATCH 5/6 v2] KVM: PPC: Book3E: Add ONE_REG AltiVec support
From: mihai.caraman @ 2014-07-03 16:11 UTC (permalink / raw)
To: Alexander Graf, kvm-ppc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org
In-Reply-To: <53B54DB2.1000805@suse.de>
> -----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
>=20
>=20
> 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>
>=20
> 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?
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.
-Mike
^ permalink raw reply
* RE: [PATCH 3/5 v4] KVM: PPC: Book3s: Remove kvmppc_read_inst() function
From: mihai.caraman @ 2014-07-03 16:18 UTC (permalink / raw)
To: Alexander Graf, kvm-ppc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org
In-Reply-To: <53B55C81.8040300@suse.de>
> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Thursday, July 03, 2014 4:37 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/5 v4] KVM: PPC: Book3s: Remove kvmppc_read_inst()
> function
>=20
>=20
> On 28.06.14 00:49, Mihai Caraman wrote:
> > In the context of replacing kvmppc_ld() function calls with a version
> of
> > kvmppc_get_last_inst() which allow to fail, Alex Graf suggested this:
> >
> > "If we get EMULATE_AGAIN, we just have to make sure we go back into the
> guest.
> > No need to inject an ISI into the guest - it'll do that all by itself.
> > With an error returning kvmppc_get_last_inst we can just use completely
> > get rid of kvmppc_read_inst() and only use kvmppc_get_last_inst()
> instead."
> >
> > As a intermediate step get rid of kvmppc_read_inst() and only use
> kvmppc_ld()
> > instead.
> >
> > Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> > ---
> > v4:
> > - new patch
> >
> > arch/powerpc/kvm/book3s_pr.c | 85 ++++++++++++++++++-----------------
> ---------
> > 1 file changed, 35 insertions(+), 50 deletions(-)
> >
> > diff --git a/arch/powerpc/kvm/book3s_pr.c
> b/arch/powerpc/kvm/book3s_pr.c
> > index 15fd6c2..d247d88 100644
> > --- a/arch/powerpc/kvm/book3s_pr.c
> > +++ b/arch/powerpc/kvm/book3s_pr.c
> > @@ -665,42 +665,6 @@ static void kvmppc_giveup_fac(struct kvm_vcpu
> *vcpu, ulong fac)
> > #endif
> > }
> >
> > -static int kvmppc_read_inst(struct kvm_vcpu *vcpu)
> > -{
> > - ulong srr0 =3D kvmppc_get_pc(vcpu);
> > - u32 last_inst =3D kvmppc_get_last_inst(vcpu);
> > - int ret;
> > -
> > - ret =3D kvmppc_ld(vcpu, &srr0, sizeof(u32), &last_inst, false);
> > - if (ret =3D=3D -ENOENT) {
> > - ulong msr =3D kvmppc_get_msr(vcpu);
> > -
> > - msr =3D kvmppc_set_field(msr, 33, 33, 1);
> > - msr =3D kvmppc_set_field(msr, 34, 36, 0);
> > - msr =3D kvmppc_set_field(msr, 42, 47, 0);
> > - kvmppc_set_msr_fast(vcpu, msr);
> > - kvmppc_book3s_queue_irqprio(vcpu,
> BOOK3S_INTERRUPT_INST_STORAGE);
> > - return EMULATE_AGAIN;
> > - }
> > -
> > - return EMULATE_DONE;
> > -}
> > -
> > -static int kvmppc_check_ext(struct kvm_vcpu *vcpu, unsigned int
> exit_nr)
> > -{
> > -
> > - /* Need to do paired single emulation? */
> > - if (!(vcpu->arch.hflags & BOOK3S_HFLAG_PAIRED_SINGLE))
> > - return EMULATE_DONE;
> > -
> > - /* Read out the instruction */
> > - if (kvmppc_read_inst(vcpu) =3D=3D EMULATE_DONE)
> > - /* Need to emulate */
> > - return EMULATE_FAIL;
> > -
> > - return EMULATE_AGAIN;
> > -}
> > -
> > /* Handle external providers (FPU, Altivec, VSX) */
> > static int kvmppc_handle_ext(struct kvm_vcpu *vcpu, unsigned int
> exit_nr,
> > ulong msr)
> > @@ -1101,31 +1065,51 @@ program_interrupt:
> > case BOOK3S_INTERRUPT_VSX:
> > {
> > int ext_msr =3D 0;
> > + int emul;
> > + ulong pc;
> > + u32 last_inst;
> >
> > - switch (exit_nr) {
> > - case BOOK3S_INTERRUPT_FP_UNAVAIL: ext_msr =3D MSR_FP; break;
> > - case BOOK3S_INTERRUPT_ALTIVEC: ext_msr =3D MSR_VEC; break;
> > - case BOOK3S_INTERRUPT_VSX: ext_msr =3D MSR_VSX; break;
> > - }
> > + if (!(vcpu->arch.hflags & BOOK3S_HFLAG_PAIRED_SINGLE)) {
>=20
> Please make paired single emulation the unusual, if()'ed case, not the
> normal exit path :).
Huh ... do you have more Book3s specific requests, it will be strange if
it will still work after all this blind changes :)
-Mike
^ permalink raw reply
* Re: [PATCH 3/5 v4] KVM: PPC: Book3s: Remove kvmppc_read_inst() function
From: Alexander Graf @ 2014-07-03 16:25 UTC (permalink / raw)
To: mihai.caraman@freescale.com
Cc: linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org,
kvm-ppc@vger.kernel.org
In-Reply-To: <830562c87cb84ad1a02aded69f5b3e51@BY2PR03MB508.namprd03.prod.outlook.com>
Am 03.07.2014 um 18:18 schrieb "mihai.caraman@freescale.com" <mihai.caraman@=
freescale.com>:
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Thursday, July 03, 2014 4:37 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/5 v4] KVM: PPC: Book3s: Remove kvmppc_read_inst()
>> function
>>=20
>>=20
>>> On 28.06.14 00:49, Mihai Caraman wrote:
>>> In the context of replacing kvmppc_ld() function calls with a version
>> of
>>> kvmppc_get_last_inst() which allow to fail, Alex Graf suggested this:
>>>=20
>>> "If we get EMULATE_AGAIN, we just have to make sure we go back into the
>> guest.
>>> No need to inject an ISI into the guest - it'll do that all by itself.
>>> With an error returning kvmppc_get_last_inst we can just use completely
>>> get rid of kvmppc_read_inst() and only use kvmppc_get_last_inst()
>> instead."
>>>=20
>>> As a intermediate step get rid of kvmppc_read_inst() and only use
>> kvmppc_ld()
>>> instead.
>>>=20
>>> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>>> ---
>>> v4:
>>> - new patch
>>>=20
>>> arch/powerpc/kvm/book3s_pr.c | 85 ++++++++++++++++++-----------------
>> ---------
>>> 1 file changed, 35 insertions(+), 50 deletions(-)
>>>=20
>>> diff --git a/arch/powerpc/kvm/book3s_pr.c
>> b/arch/powerpc/kvm/book3s_pr.c
>>> index 15fd6c2..d247d88 100644
>>> --- a/arch/powerpc/kvm/book3s_pr.c
>>> +++ b/arch/powerpc/kvm/book3s_pr.c
>>> @@ -665,42 +665,6 @@ static void kvmppc_giveup_fac(struct kvm_vcpu
>> *vcpu, ulong fac)
>>> #endif
>>> }
>>>=20
>>> -static int kvmppc_read_inst(struct kvm_vcpu *vcpu)
>>> -{
>>> - ulong srr0 =3D kvmppc_get_pc(vcpu);
>>> - u32 last_inst =3D kvmppc_get_last_inst(vcpu);
>>> - int ret;
>>> -
>>> - ret =3D kvmppc_ld(vcpu, &srr0, sizeof(u32), &last_inst, false);
>>> - if (ret =3D=3D -ENOENT) {
>>> - ulong msr =3D kvmppc_get_msr(vcpu);
>>> -
>>> - msr =3D kvmppc_set_field(msr, 33, 33, 1);
>>> - msr =3D kvmppc_set_field(msr, 34, 36, 0);
>>> - msr =3D kvmppc_set_field(msr, 42, 47, 0);
>>> - kvmppc_set_msr_fast(vcpu, msr);
>>> - kvmppc_book3s_queue_irqprio(vcpu,
>> BOOK3S_INTERRUPT_INST_STORAGE);
>>> - return EMULATE_AGAIN;
>>> - }
>>> -
>>> - return EMULATE_DONE;
>>> -}
>>> -
>>> -static int kvmppc_check_ext(struct kvm_vcpu *vcpu, unsigned int
>> exit_nr)
>>> -{
>>> -
>>> - /* Need to do paired single emulation? */
>>> - if (!(vcpu->arch.hflags & BOOK3S_HFLAG_PAIRED_SINGLE))
>>> - return EMULATE_DONE;
>>> -
>>> - /* Read out the instruction */
>>> - if (kvmppc_read_inst(vcpu) =3D=3D EMULATE_DONE)
>>> - /* Need to emulate */
>>> - return EMULATE_FAIL;
>>> -
>>> - return EMULATE_AGAIN;
>>> -}
>>> -
>>> /* Handle external providers (FPU, Altivec, VSX) */
>>> static int kvmppc_handle_ext(struct kvm_vcpu *vcpu, unsigned int
>> exit_nr,
>>> ulong msr)
>>> @@ -1101,31 +1065,51 @@ program_interrupt:
>>> case BOOK3S_INTERRUPT_VSX:
>>> {
>>> int ext_msr =3D 0;
>>> + int emul;
>>> + ulong pc;
>>> + u32 last_inst;
>>>=20
>>> - switch (exit_nr) {
>>> - case BOOK3S_INTERRUPT_FP_UNAVAIL: ext_msr =3D MSR_FP; break;
>>> - case BOOK3S_INTERRUPT_ALTIVEC: ext_msr =3D MSR_VEC; break;
>>> - case BOOK3S_INTERRUPT_VSX: ext_msr =3D MSR_VSX; break;
>>> - }
>>> + if (!(vcpu->arch.hflags & BOOK3S_HFLAG_PAIRED_SINGLE)) {
>>=20
>> Please make paired single emulation the unusual, if()'ed case, not the
>> normal exit path :).
>=20
> Huh ... do you have more Book3s specific requests, it will be strange if
> it will still work after all this blind changes :)
Heh :).
All I'm saying is that rather than
if (no emulation) {
foo();
break;
)
ps_emulation();
break;
We should do
if (ps emulation) {
ps_emulation();
break;
}
foo();
break;
Alex
^ permalink raw reply
* Re: [PATCH] devicetree/binding/powerpc/fsl: Add binding for CPLD
From: Scott Wood @ 2014-07-03 18:07 UTC (permalink / raw)
To: Priyanka Jain; +Cc: devicetree, linuxppc-dev
In-Reply-To: <1404382185-2967-1-git-send-email-Priyanka.Jain@freescale.com>
On Thu, 2014-07-03 at 15:39 +0530, Priyanka Jain wrote:
> 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>
> ---
> .../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>;
> + };
Please keep the board name in lowercase like the rest.
-Scott
^ permalink raw reply
* Re: [PATCH] devicetree/bindings: Add binding for micron n25q512a memory
From: Scott Wood @ 2014-07-03 22:10 UTC (permalink / raw)
To: Priyanka Jain; +Cc: devicetree, linuxppc-dev, linux-mtd, linux-spi
In-Reply-To: <1404382376-3115-1-git-send-email-Priyanka.Jain@freescale.com>
On Thu, 2014-07-03 at 15:42 +0530, Priyanka Jain wrote:
> -Micron n25q512a memory is supported by m25p80 driver.
> Add compatible field required to support n25q512a in m25p80.txt
> -Add micron to the vendor-prefixes.txt file
>
> Signed-off-by: Priyanka Jain <Priyanka.Jain@freescale.com>
> ---
> Documentation/devicetree/bindings/mtd/m25p80.txt | 1 +
> .../devicetree/bindings/vendor-prefixes.txt | 1 +
> 2 files changed, 2 insertions(+), 0 deletions(-)
Why did you send this to the ppc list but not the spi or mtd lists?
I'm having a hard time following the flow of how these SPI devices get
bound -- is the compatible involved at all? I don't see this string
(with vendor prefix included) in the driver. I do see a table that
contains what looks like device IDs. If the device can report its id,
shouldn't we rely on that rather than device tree compatible?
-Scott
> diff --git a/Documentation/devicetree/bindings/mtd/m25p80.txt b/Documentation/devicetree/bindings/mtd/m25p80.txt
> index 4611aa8..ce02e81 100644
> --- a/Documentation/devicetree/bindings/mtd/m25p80.txt
> +++ b/Documentation/devicetree/bindings/mtd/m25p80.txt
> @@ -7,6 +7,7 @@ Required properties:
> the DT binding is not Linux-only, but in case of Linux, see the
> "spi_nor_ids" table in drivers/mtd/spi-nor/spi-nor.c for the list
> of supported chips.
> + example: "micron,n25q512a"
> - reg : Chip-Select number
> - spi-max-frequency : Maximum frequency of the SPI bus the chip can operate at
>
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index 4d7f375..a44cfee 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -79,6 +79,7 @@ marvell Marvell Technology Group Ltd.
> maxim Maxim Integrated Products
> micrel Micrel Inc.
> microchip Microchip Technology Inc.
> +micron Micron Technology Inc.
> mosaixtech Mosaix Technologies, Inc.
> moxa Moxa
> mpl MPL AG
^ permalink raw reply
* Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
From: Scott Wood @ 2014-07-03 22:15 UTC (permalink / raw)
To: Caraman Mihai Claudiu-B02008
Cc: linuxppc-dev@lists.ozlabs.org, Alexander Graf,
kvm-ppc@vger.kernel.org, kvm@vger.kernel.org
In-Reply-To: <2399bd1f8fde430a945311ff27c3f5c3@BY2PR03MB508.namprd03.prod.outlook.com>
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.
> > > #define BOOKE_INTERRUPT_SPE_FP_ROUND 34
> > > #define BOOKE_INTERRUPT_PERFORMANCE_MONITOR 35
> > > #define BOOKE_INTERRUPT_DOORBELL 36
> > > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> > > index ab62109..3c86d9b 100644
> > > --- a/arch/powerpc/kvm/booke.c
> > > +++ b/arch/powerpc/kvm/booke.c
> > > @@ -388,8 +388,8 @@ static int kvmppc_booke_irqprio_deliver(struct
> > kvm_vcpu *vcpu,
> > > case BOOKE_IRQPRIO_ITLB_MISS:
> > > case BOOKE_IRQPRIO_SYSCALL:
> > > case BOOKE_IRQPRIO_FP_UNAVAIL:
> > > - case BOOKE_IRQPRIO_SPE_UNAVAIL:
> > > - case BOOKE_IRQPRIO_SPE_FP_DATA:
> > > + case BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL:
> > > + case BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST:
> >
> > #ifdef CONFIG_KVM_E500V2
> > case ...SPE:
> > #else
> > case ..ALTIVEC:
> > #endif
I don't think that's an improvement.
-Scott
^ permalink raw reply
* Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
From: Alexander Graf @ 2014-07-03 22:31 UTC (permalink / raw)
To: Scott Wood, Caraman Mihai Claudiu-B02008
Cc: linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org,
kvm-ppc@vger.kernel.org
In-Reply-To: <1404425722.21434.93.camel@snotra.buserror.net>
On 04.07.14 00:15, 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.
The nice thing here is that we use almost all of these numbers in
switch() statements which give us automated duplicate checking - so we
don't accidentally go into the wrong code path without knowing it.
Alex
^ permalink raw reply
* Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
From: Alexander Graf @ 2014-07-03 22:35 UTC (permalink / raw)
To: Scott Wood, Caraman Mihai Claudiu-B02008
Cc: linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org,
kvm-ppc@vger.kernel.org
In-Reply-To: <1404426684.21434.102.camel@snotra.buserror.net>
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.
Alex
^ permalink raw reply
* Re: [PATCH 1/2] PCI: Make resetting secondary bus logic common
From: Bjorn Helgaas @ 2014-07-03 22:47 UTC (permalink / raw)
To: Gavin Shan; +Cc: linux-pci, linuxppc-dev
In-Reply-To: <1403162565-26835-1-git-send-email-gwshan@linux.vnet.ibm.com>
On Thu, Jun 19, 2014 at 05:22:44PM +1000, Gavin Shan wrote:
> Commit d92a208d086 ("powerpc/pci: Mask linkDown on resetting PCI bus")
> implemented same logic (resetting PCI secondary bus by bridge's config
> register PCI_BRIDGE_CTL_BUS_RESET) in PCI core and arch-dependent
> code. In order to avoid the duplicate implementation introduced by the
> commit, the patch puts the logic into pci_reset_secondary_bus().
>
> That commit also missed declaring newly introduced weak function
> pcibios_reset_secondary_bus() in linux/include/pci.h. The patch fixes
> it.
>
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
I applied both of these to pci/virtualization for v3.17. Ben, if you'd
rather apply the powerpc one [1], let me know and I'll drop that one.
Bjorn
[1] http://patchwork.ozlabs.org/patch/361802/
> ---
> drivers/pci/pci.c | 7 ++++++-
> include/linux/pci.h | 2 ++
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 63a54a3..758f1d8 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3193,7 +3193,7 @@ static int pci_pm_reset(struct pci_dev *dev, int probe)
> return 0;
> }
>
> -void __weak pcibios_reset_secondary_bus(struct pci_dev *dev)
> +void pci_reset_secondary_bus(struct pci_dev *dev)
> {
> u16 ctrl;
>
> @@ -3219,6 +3219,11 @@ void __weak pcibios_reset_secondary_bus(struct pci_dev *dev)
> ssleep(1);
> }
>
> +void __weak pcibios_reset_secondary_bus(struct pci_dev *dev)
> +{
> + pci_reset_secondary_bus(dev);
> +}
> +
> /**
> * pci_reset_bridge_secondary_bus - Reset the secondary bus on a PCI bridge.
> * @dev: Bridge device
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 466bcd1..340529d 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -978,6 +978,8 @@ int pci_try_reset_slot(struct pci_slot *slot);
> int pci_probe_reset_bus(struct pci_bus *bus);
> int pci_reset_bus(struct pci_bus *bus);
> int pci_try_reset_bus(struct pci_bus *bus);
> +void pci_reset_secondary_bus(struct pci_dev *dev);
> +void pcibios_reset_secondary_bus(struct pci_dev *dev);
> void pci_reset_bridge_secondary_bus(struct pci_dev *dev);
> void pci_update_resource(struct pci_dev *dev, int resno);
> int __must_check pci_assign_resource(struct pci_dev *dev, int i);
> --
> 1.8.3.2
>
^ permalink raw reply
* Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
From: Scott Wood @ 2014-07-03 22:31 UTC (permalink / raw)
To: Caraman Mihai Claudiu-B02008
Cc: linuxppc-dev@lists.ozlabs.org, Alexander Graf,
kvm-ppc@vger.kernel.org, kvm@vger.kernel.org
In-Reply-To: <1404425722.21434.93.camel@snotra.buserror.net>
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).
-Scott
^ permalink raw reply
* Re: [PATCH 1/2] PCI: Make resetting secondary bus logic common
From: Benjamin Herrenschmidt @ 2014-07-03 22:53 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: linux-pci, linuxppc-dev, Gavin Shan
In-Reply-To: <20140703224704.GA25980@google.com>
On Thu, 2014-07-03 at 16:47 -0600, Bjorn Helgaas wrote:
> On Thu, Jun 19, 2014 at 05:22:44PM +1000, Gavin Shan wrote:
> > Commit d92a208d086 ("powerpc/pci: Mask linkDown on resetting PCI bus")
> > implemented same logic (resetting PCI secondary bus by bridge's config
> > register PCI_BRIDGE_CTL_BUS_RESET) in PCI core and arch-dependent
> > code. In order to avoid the duplicate implementation introduced by the
> > commit, the patch puts the logic into pci_reset_secondary_bus().
> >
> > That commit also missed declaring newly introduced weak function
> > pcibios_reset_secondary_bus() in linux/include/pci.h. The patch fixes
> > it.
> >
> > Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>
> I applied both of these to pci/virtualization for v3.17. Ben, if you'd
> rather apply the powerpc one [1], let me know and I'll drop that one.
I'm happy for you to keep it. If for some reason I want it too we can
both carry it, it shouldn't hurt significantly at merge time.
Cheers,
Ben.
> Bjorn
>
> [1] http://patchwork.ozlabs.org/patch/361802/
>
> > ---
> > drivers/pci/pci.c | 7 ++++++-
> > include/linux/pci.h | 2 ++
> > 2 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 63a54a3..758f1d8 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -3193,7 +3193,7 @@ static int pci_pm_reset(struct pci_dev *dev, int probe)
> > return 0;
> > }
> >
> > -void __weak pcibios_reset_secondary_bus(struct pci_dev *dev)
> > +void pci_reset_secondary_bus(struct pci_dev *dev)
> > {
> > u16 ctrl;
> >
> > @@ -3219,6 +3219,11 @@ void __weak pcibios_reset_secondary_bus(struct pci_dev *dev)
> > ssleep(1);
> > }
> >
> > +void __weak pcibios_reset_secondary_bus(struct pci_dev *dev)
> > +{
> > + pci_reset_secondary_bus(dev);
> > +}
> > +
> > /**
> > * pci_reset_bridge_secondary_bus - Reset the secondary bus on a PCI bridge.
> > * @dev: Bridge device
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 466bcd1..340529d 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -978,6 +978,8 @@ int pci_try_reset_slot(struct pci_slot *slot);
> > int pci_probe_reset_bus(struct pci_bus *bus);
> > int pci_reset_bus(struct pci_bus *bus);
> > int pci_try_reset_bus(struct pci_bus *bus);
> > +void pci_reset_secondary_bus(struct pci_dev *dev);
> > +void pcibios_reset_secondary_bus(struct pci_dev *dev);
> > void pci_reset_bridge_secondary_bus(struct pci_dev *dev);
> > void pci_update_resource(struct pci_dev *dev, int resno);
> > int __must_check pci_assign_resource(struct pci_dev *dev, int i);
> > --
> > 1.8.3.2
> >
^ permalink raw reply
* Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
From: Alexander Graf @ 2014-07-03 23:02 UTC (permalink / raw)
To: Scott Wood
Cc: Caraman Mihai Claudiu-B02008, linuxppc-dev@lists.ozlabs.org,
kvm@vger.kernel.org, kvm-ppc@vger.kernel.org
In-Reply-To: <1404428423.21434.107.camel@snotra.buserror.net>
On 04.07.14 01:00, Scott Wood wrote:
> 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.
Yes, me too - but I also like to be explicit. If there's enough code to
share, factoring those into helpers certainly works well for me.
Alex
^ permalink raw reply
* Re: [PATCH 4/6 v2] KVM: PPC: Book3E: Add AltiVec support
From: Scott Wood @ 2014-07-03 23:07 UTC (permalink / raw)
To: Mihai Caraman; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1404142497-6430-5-git-send-email-mihai.caraman@freescale.com>
On Mon, 2014-06-30 at 18:34 +0300, Mihai Caraman wrote:
> Add KVM Book3E AltiVec support. KVM Book3E FPU support gracefully reuse host
> infrastructure so follow the same approach for AltiVec.
>
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> ---
> v2:
> - integrate Paul's FP/VMX/VSX changes
>
> arch/powerpc/kvm/booke.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 65 insertions(+), 2 deletions(-)
I had to apply the whole patchset to get proper context for reviewing
this, and found some nits:
> case BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST:
> if (kvmppc_supports_spe() || kvmppc_supports_altivec()) {
> kvmppc_booke_queue_irqprio(vcpu,
> BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST);
> r = RESUME_GUEST;
> } else {
> /*
> * These really should never happen without CONFIG_SPE,
> * as we should never enable the real MSR[SPE] in the
> * guest.
> */
Besides the comment not being updated for Altivec, it's not true on HV,
where the guest can enable MSR[VEC] all by itself. For HV, the reason
we shouldn't be able to get here is that we disable KVM on e6500 if
CONFIG_ALTIVEC is not enabled, and no other HV core supports either SPE
or Altivec.
> pr_crit("%s: unexpected SPE interrupt %u at %08lx\n",
> __func__, exit_nr, vcpu->arch.pc);
Error string will say SPE regardless of what sort of chip you're on.
Given that this is explicitly on the "no support for Altivec or SPE"
path, "SPE/Altivec" phrasing seems appropriate. Of course we have
bigger problems than that if we ever reach this code. :-)
-Scott
^ permalink raw reply
* 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
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