[Skiboot] [PATCH 1/1] Disable protected execution facility
Alistair Popple
alistair at popple.id.au
Tue Feb 18 16:29:35 AEDT 2020
We've run into this issue with Simics this works there as well so:
Tested-by: Alistair Popple <alistair at popple.id.au>
Couple of questions though:
> +.global exit_uv_mode
> +exit_uv_mode:
> + mfmsr %r4
> + LOAD_IMM64(%r5, ~MSR_S)
> + and %r4,%r4,%r5
> + mtspr SPR_USRR1,%r4
> +
> + mfspr %r4,SPR_HSRR1
> + and %r4,%r4,%r5
> + mtspr SPR_HSRR1,%r3
> +
> + mfspr %r4,SPR_SRR1
> + and %r4,%r4,%r5
> + mtspr SPR_SRR1,%r4
Is there a reason we need to update [H]SRR1 as well? I doubt we'd be running
this in the context of an exception and other uses of SRR1 tend to set it
explicitly rather than relying on existing values, although I may be missing
something.
> + cmpdi %r3,1
> + bne 1f
> + mfspr %r4, SPR_SMFCTRL
> + LOAD_IMM64(%r5, ~PPC_BIT(0))
> + and %r4,%r4,%r5
> + mtspr SPR_SMFCTRL,%r4
> +1:
> + isync
> +
> + mflr %r4
> + mtspr SPR_USRR0,%r4
> +
> + urfid
> diff --git a/core/cpu.c b/core/cpu.c
> index d5b7d623..1adf16cc 100644
> --- a/core/cpu.c
> +++ b/core/cpu.c
> @@ -1644,3 +1644,62 @@ static int64_t opal_nmmu_set_ptcr(uint64_t chip_id,
> uint64_t ptcr) return rc;
> }
> opal_call(OPAL_NMMU_SET_PTCR, opal_nmmu_set_ptcr, 2);
> +
> +static void _exit_uv_mode(void *data __unused)
> +{
> + prlog(PR_DEBUG, "Exit uv mode on cpu pir 0x%04x\n", this_cpu()->pir);
> + /* HW has smfctrl shared between threads but on Mambo it is per-thread */
> + if (chip_quirk(QUIRK_MAMBO_CALLOUTS))
> + exit_uv_mode(1);
> + else
> + exit_uv_mode(cpu_is_thread0(this_cpu()));
> +}
> +
> +void cpu_disable_pef(void)
> +{
> + struct cpu_thread *cpu;
> + struct cpu_job **jobs;
> +
> + if (!(mfmsr() & MSR_S)) {
> + prlog(PR_DEBUG, "UV mode off on cpu pir 0x%04x\n", this_cpu()->pir);
> + return;
> + }
> +
> + jobs = zalloc(sizeof(struct cpu_job *) * (cpu_max_pir + 1));
> + assert(jobs);
> +
> + /* Exit uv mode on all secondary threads before touching
> + * smfctrl on thread 0 */
Do we need to separate things out this way though? It seems like it should
have much the same affect to clear SMFCTRL and UV mode on every thread. Doing
so might simplify the code a bit as you could just call exit_uv_mode()
directly from main/secondary_cpu_init().
> + for_each_available_cpu(cpu) {
> + if (cpu == this_cpu())
> + continue;
> +
> + if (!cpu_is_thread0(cpu))
> + jobs[cpu->pir] = cpu_queue_job(cpu, "exit_uv_mode",
> + _exit_uv_mode, NULL);
> + }
> +
> + for_each_available_cpu(cpu)
> + if (jobs[cpu->pir]) {
> + cpu_wait_job(jobs[cpu->pir], true);
> + jobs[cpu->pir] = NULL;
> + }
> +
> + /* Exit uv mode and disable smfctrl on primary threads */
> + for_each_available_cpu(cpu) {
Bit of a nit-pick but you could use for_each_available_core_in_chip() instead.
- Alistair
> + if (cpu == this_cpu())
> + continue;
> +
> + if (cpu_is_thread0(cpu))
> + jobs[cpu->pir] = cpu_queue_job(cpu, "exit_uv_mode",
> + _exit_uv_mode, NULL);
> + }
> +
> + for_each_available_cpu(cpu)
> + if (jobs[cpu->pir])
> + cpu_wait_job(jobs[cpu->pir], true);
> +
> + free(jobs);
> +
> + _exit_uv_mode(NULL);
> +}
> diff --git a/core/init.c b/core/init.c
> index 339462e5..0d993abb 100644
> --- a/core/init.c
> +++ b/core/init.c
> @@ -1354,6 +1354,9 @@ void __noreturn __nomcount main_cpu_entry(const void
> *fdt) /* Add the list of interrupts going to OPAL */
> add_opal_interrupts();
>
> + /* Disable protected execution facility in BML */
> + cpu_disable_pef();
> +
> /* Now release parts of memory nodes we haven't used ourselves... */
> mem_region_release_unused();
>
> diff --git a/include/cpu.h b/include/cpu.h
> index 686310d7..cab63360 100644
> --- a/include/cpu.h
> +++ b/include/cpu.h
> @@ -309,4 +309,7 @@ int dctl_set_special_wakeup(struct cpu_thread *t);
> int dctl_clear_special_wakeup(struct cpu_thread *t);
> int dctl_core_is_gated(struct cpu_thread *t);
>
> +extern void exit_uv_mode(int);
> +void cpu_disable_pef(void);
> +
> #endif /* __CPU_H */
> diff --git a/include/processor.h b/include/processor.h
> index a0c2864a..1fdcc02b 100644
> --- a/include/processor.h
> +++ b/include/processor.h
> @@ -11,6 +11,7 @@
> #define MSR_HV PPC_BIT(3) /* Hypervisor mode */
> #define MSR_VEC PPC_BIT(38) /* VMX enable */
> #define MSR_VSX PPC_BIT(40) /* VSX enable */
> +#define MSR_S PPC_BIT(41) /* Secure mode */
> #define MSR_EE PPC_BIT(48) /* External Int. Enable */
> #define MSR_PR PPC_BIT(49) /* Problem state */
> #define MSR_FP PPC_BIT(50) /* Floating Point Enable */
> @@ -65,6 +66,9 @@
> #define SPR_HMEER 0x151 /* HMER interrupt enable mask */
> #define SPR_PCR 0x152
> #define SPR_AMOR 0x15d
> +#define SPR_USRR0 0x1fa /* RW: Ultravisor Save/Restore Register 0 */
> +#define SPR_USRR1 0x1fb /* RW: Ultravisor Save/Restore Register 1 */
> +#define SPR_SMFCTRL 0x1ff /* RW: Secure Memory Facility Control */
> #define SPR_PSSCR 0x357 /* RW: Stop status and control (ISA 3) */
> #define SPR_TSCR 0x399
> #define SPR_HID0 0x3f0
More information about the Skiboot
mailing list