On Thu, 2019-03-21 at 15:24 +1100, Michael Ellerman wrote:
> When I updated the spectre_v2 reporting to handle software count cache
> flush I got the logic wrong when there's no software count cache
> enabled at all.
>
> The result is that on systems with the software count cache flush
> disabled we print:
>
> Mitigation: Indirect branch cache disabled, Software count cache flush
>
> Which correctly indicates that the count cache is disabled, but
> incorrectly says the software count cache flush is enabled.
>
> The root of the problem is that we are trying to handle all
> combinations of options. But we know now that we only expect to see
> the software count cache flush enabled if the other options are false.
>
> So split the two cases, which simplifies the logic and fixes the bug.
> We were also missing a space before "(hardware accelerated)".
>
> The result is we see one of:
>
> Mitigation: Indirect branch serialisation (kernel only)
> Mitigation: Indirect branch cache disabled
> Mitigation: Software count cache flush
> Mitigation: Software count cache flush (hardware accelerated)
>
> Fixes: ee13cb249fab ("powerpc/64s: Add support for software count cache
> flush")
> Cc: sta...@vger.kernel.org # v4.19+
> Signed-off-by: Michael Ellerman <m...@ellerman.id.au>
LGTM
Reviewed-by: Michael Neuling <mi...@neuling.org>
> ---
> arch/powerpc/kernel/security.c | 23 ++++++++---------------
> 1 file changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
> index 9b8631533e02..b33bafb8fcea 100644
> --- a/arch/powerpc/kernel/security.c
> +++ b/arch/powerpc/kernel/security.c
> @@ -190,29 +190,22 @@ ssize_t cpu_show_spectre_v2(struct device *dev, struct
> device_attribute *attr, c
> bcs = security_ftr_enabled(SEC_FTR_BCCTRL_SERIALISED);
> ccd = security_ftr_enabled(SEC_FTR_COUNT_CACHE_DISABLED);
>
> - if (bcs || ccd || count_cache_flush_type != COUNT_CACHE_FLUSH_NONE) {
> - bool comma = false;
> + if (bcs || ccd) {
> seq_buf_printf(&s, "Mitigation: ");
>
> - if (bcs) {
> + if (bcs)
> seq_buf_printf(&s, "Indirect branch serialisation
> (kernel only)");
> - comma = true;
> - }
>
> - if (ccd) {
> - if (comma)
> - seq_buf_printf(&s, ", ");
> - seq_buf_printf(&s, "Indirect branch cache disabled");
> - comma = true;
> - }
> -
> - if (comma)
> + if (bcs && ccd)
> seq_buf_printf(&s, ", ");
>
> - seq_buf_printf(&s, "Software count cache flush");
> + if (ccd)
> + seq_buf_printf(&s, "Indirect branch cache disabled");
> + } else if (count_cache_flush_type != COUNT_CACHE_FLUSH_NONE) {
> + seq_buf_printf(&s, "Mitigation: Software count cache flush");
>
> if (count_cache_flush_type == COUNT_CACHE_FLUSH_HW)
> - seq_buf_printf(&s, "(hardware accelerated)");
> + seq_buf_printf(&s, " (hardware accelerated)");
> } else if (btb_flush_enabled) {
> seq_buf_printf(&s, "Mitigation: Branch predictor state flush");
> } else {
