On Tue, May 22, 2018 at 07:35:59PM +0200, Mark Kettenis wrote:
> > Date: Fri, 4 May 2018 20:14:38 +0200 (CEST)
> > From: Mark Kettenis <mark.kette...@xs4all.nl>
> >
> > > Date: Tue, 1 May 2018 11:55:00 +0200 (CEST)
> > > From: Mark Kettenis <mark.kette...@xs4all.nl>
> > >
> > > So after adding a quick hack to mitigate Spectre variant 2 to ARM
> > > Trusted Firmware (ATF), ARM actually designed a proper solution that
> > > minimizes the performance loss and makes the presence of the
> > > workaround detectable. This is all documented in an update of the SMC
> > > Calling Convention (SMCCC) standard.
> > >
> > > The diff below implements support for this solution while keeping
> > > support for the hack. While ARM strongly suggests vendors to update
> > > to a version of ATF that implements SMCCC 1.1 the current ATF for the
> > > Marvell ARMADA 8040 hasn't been updated yet (but does include the
> > > initial hack).
> >
> > So it turns out that while I wasn't paying attention, the Marvell
> > folks backported SMCCC 1.1 support to their ATF fork. This means we
> > can drop support for the PSCI_VERSION hack without leaving
> > MACCHIATOBin owener out in the cold. I think the code simplification
> > is worth it. It also means we can tell whether a machine is safe or
> > not from a dmesg.
> >
> > ok?
>
> And in light of Spectre variant 4, I think there is even less reason
> to continue support for firmware that doesn't implement SMCCC 1.1.
>
> I'd like to commit this diff such that I can improve the way we signal
> that mitigations are present in the firmware.
>
> ok?
>
Diff and reasoning make sense to me. ok mlarkin
> > Index: dev/fdt/psci.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/fdt/psci.c,v
> > retrieving revision 1.7
> > diff -u -p -r1.7 psci.c
> > --- dev/fdt/psci.c 3 May 2018 09:45:57 -0000 1.7
> > +++ dev/fdt/psci.c 4 May 2018 18:01:04 -0000
> > @@ -54,7 +54,6 @@ struct psci_softc {
> > uint32_t sc_system_reset;
> > uint32_t sc_cpu_on;
> >
> > - uint32_t sc_version;
> > uint32_t sc_smccc_version;
> > };
> >
> > @@ -98,6 +97,7 @@ psci_attach(struct device *parent, struc
> > struct psci_softc *sc = (struct psci_softc *)self;
> > struct fdt_attach_args *faa = aux;
> > char method[128];
> > + uint32_t version;
> >
> > if (OF_getprop(faa->fa_node, "method", method, sizeof(method))) {
> > if (strcmp(method, "hvc") == 0)
> > @@ -127,10 +127,10 @@ psci_attach(struct device *parent, struc
> >
> > psci_sc = sc;
> >
> > - sc->sc_version = psci_version();
> > - printf(": PSCI %d.%d", sc->sc_version >> 16, sc->sc_version & 0xffff);
> > + version = psci_version();
> > + printf(": PSCI %d.%d", version >> 16, version & 0xffff);
> >
> > - if (sc->sc_version >= 0x10000) {
> > + if (version >= 0x10000) {
> > if (psci_features(SMCCC_VERSION) == PSCI_SUCCESS) {
> > sc->sc_smccc_version = smccc_version();
> > printf(", SMCCC %d.%d", sc->sc_smccc_version >> 16,
> > @@ -165,11 +165,9 @@ psci_powerdown(void)
> > }
> >
> > /*
> > - * Firmware-based workaround for CVE-2017-5715. We pick the
> > - * appropriate mechanism based on the PSCI and SMCCC versions. We
> > - * determine whether the workaround is actually needed the first time
> > - * we are invoked such that we only make the firmware call if we
> > - * really need to.
> > + * Firmware-based workaround for CVE-2017-5715. We determine whether
> > + * the workaround is actually implemented and needed the first time we
> > + * are invoked such that we only make the firmware call when appropriate.
> > */
> >
> > void
> > @@ -178,14 +176,6 @@ psci_flush_bp_none(void)
> > }
> >
> > void
> > -psci_flush_bp_psci_version(void)
> > -{
> > - struct psci_softc *sc = psci_sc;
> > -
> > - (*sc->sc_callfn)(PSCI_VERSION, 0, 0, 0);
> > -}
> > -
> > -void
> > psci_flush_bp_smccc_arch_workaround_1(void)
> > {
> > struct psci_softc *sc = psci_sc;
> > @@ -199,32 +189,17 @@ psci_flush_bp(void)
> > struct psci_softc *sc = psci_sc;
> > struct cpu_info *ci = curcpu();
> >
> > - /* No PSCI or an old version of PSCI; nothing we can do. */
> > - if (sc == NULL || sc->sc_version < 0x10000) {
> > - ci->ci_flush_bp = psci_flush_bp_none;
> > - return;
> > - }
> > -
> > - /*
> > - * PSCI 1.0 or later with SMCCC 1.0; invoke PSCI_VERSION and
> > - * hope for the best.
> > - */
> > - if (sc->sc_smccc_version < 0x10001) {
> > - ci->ci_flush_bp = psci_flush_bp_psci_version;
> > - ci->ci_flush_bp();
> > - return;
> > - }
> > -
> > /*
> > - * SMCCC 1.1 or later; we can actually detect if the
> > - * workaround is implemented and needed.
> > + * SMCCC 1.1 allows us to detect if the workaround is
> > + * implemented and needed.
> > */
> > - if (smccc_arch_features(SMCCC_ARCH_WORKAROUND_1) == 0) {
> > + if (sc && sc->sc_smccc_version >= 0x10001 &&
> > + smccc_arch_features(SMCCC_ARCH_WORKAROUND_1) == 0) {
> > /* Workaround implemented and needed. */
> > ci->ci_flush_bp = psci_flush_bp_smccc_arch_workaround_1;
> > ci->ci_flush_bp();
> > } else {
> > - /* No workaround needed. */
> > + /* Workaround isn't implemented or isn't needed. */
> > ci->ci_flush_bp = psci_flush_bp_none;
> > }
> > }
> >
> >
>
