On Fri, 28 Feb 2020 23:54:04 -0800
Ram Pai <linuxram@xxxxxxxxxx> wrote:
> XIVE is not correctly enabled for Secure VM in the KVM Hypervisor yet.
>
What exactly is "not correctly enabled" ?
> Hence Secure VM, must always default to XICS interrupt controller.
>
So this is a temporary workaround until whatever isn't working with
XIVE and the Secure VM gets fixed. Maybe worth mentioning this in
some comment.
> If XIVE is requested through kernel command line option "xive=on",
> override and turn it off.
>
There's no such thing as requesting XIVE with "xive=on". XIVE is
on by default if the platform and CPU support it BUT it can be
disabled with "xive=off" in which case the guest wont request
XIVE except if it's the only available mode.
> If XIVE is the only supported platform interrupt controller; specified
> through qemu option "ic-mode=xive", simply abort. Otherwise default to
> XICS.
>
If XIVE is the only option and the guest requests XICS anyway, QEMU is
supposed to print an error message and terminate:
if (!spapr->irq->xics) {
error_report(
"Guest requested unavailable interrupt mode (XICS), either don't set the ic-mode machine property or try ic-mode=xics or ic-mode=dual");
exit(EXIT_FAILURE);
}
I think it would be better to end up there rather than aborting.
> Cc: kvm-ppc@xxxxxxxxxxxxxxx
> Cc: linuxppc-dev@xxxxxxxxxxxxxxxx
> Cc: Michael Ellerman <mpe@xxxxxxxxxxxxxx>
> Cc: Thiago Jung Bauermann <bauerman@xxxxxxxxxxxxx>
> Cc: Michael Anderson <andmike@xxxxxxxxxxxxx>
> Cc: Sukadev Bhattiprolu <sukadev@xxxxxxxxxxxxxxxxxx>
> Cc: Alexey Kardashevskiy <aik@xxxxxxxxx>
> Cc: Paul Mackerras <paulus@xxxxxxxxxx>
> Cc: Greg Kurz <groug@xxxxxxxx>
> Cc: Cedric Le Goater <clg@xxxxxxxxxx>
> Cc: David Gibson <david@xxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Ram Pai <linuxram@xxxxxxxxxx>
> ---
> arch/powerpc/kernel/prom_init.c | 43 ++++++++++++++++++++++++++++-------------
> 1 file changed, 30 insertions(+), 13 deletions(-)
>
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index 5773453..dd96c82 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -805,6 +805,18 @@ static void __init early_cmdline_parse(void)
> #endif
> }
>
> +#ifdef CONFIG_PPC_SVM
> + opt = prom_strstr(prom_cmd_line, "svm=");
> + if (opt) {
> + bool val;
> +
> + opt += sizeof("svm=") - 1;
> + if (!prom_strtobool(opt, &val))
> + prom_svm_enable = val;
> + prom_printf("svm =%d\n", prom_svm_enable);
> + }
> +#endif /* CONFIG_PPC_SVM */
> +
> #ifdef CONFIG_PPC_PSERIES
> prom_radix_disable = !IS_ENABLED(CONFIG_PPC_RADIX_MMU_DEFAULT);
> opt = prom_strstr(prom_cmd_line, "disable_radix");
> @@ -823,23 +835,22 @@ static void __init early_cmdline_parse(void)
> if (prom_radix_disable)
> prom_debug("Radix disabled from cmdline\n");
>
> - opt = prom_strstr(prom_cmd_line, "xive=off");
> - if (opt) {
A comment to explain why we currently need to limit ourselves to using
XICS would be appreciated.
> +#ifdef CONFIG_PPC_SVM
> + if (prom_svm_enable) {
> prom_xive_disable = true;
> - prom_debug("XIVE disabled from cmdline\n");
> + prom_debug("XIVE disabled in Secure VM\n");
> }
> -#endif /* CONFIG_PPC_PSERIES */
> -
> -#ifdef CONFIG_PPC_SVM
> - opt = prom_strstr(prom_cmd_line, "svm=");
> - if (opt) {
> - bool val;
> +#endif /* CONFIG_PPC_SVM */
>
> - opt += sizeof("svm=") - 1;
> - if (!prom_strtobool(opt, &val))
> - prom_svm_enable = val;
> + if (!prom_xive_disable) {
> + opt = prom_strstr(prom_cmd_line, "xive=off");
> + if (opt) {
> + prom_xive_disable = true;
> + prom_debug("XIVE disabled from cmdline\n");
> + }
> }
> -#endif /* CONFIG_PPC_SVM */
> +
> +#endif /* CONFIG_PPC_PSERIES */
> }
>
> #ifdef CONFIG_PPC_PSERIES
> @@ -1251,6 +1262,12 @@ static void __init prom_parse_xive_model(u8 val,
> break;
> case OV5_FEAT(OV5_XIVE_EXPLOIT): /* Only Exploitation mode */
> prom_debug("XIVE - exploitation mode supported\n");
> +
> +#ifdef CONFIG_PPC_SVM
> + if (prom_svm_enable)
> + prom_panic("WARNING: xive unsupported in Secure VM\n");
Change the prom_panic() line into a break. The guest will ask XICS and QEMU
will terminate nicely. Maybe still print out a warning since QEMU won't mention
the Secure VM aspect of things.
> +#endif /* CONFIG_PPC_SVM */
> +
> if (prom_xive_disable) {
> /*
> * If we __have__ to do XIVE, we're better off ignoring
