Re: [RFC PATCH 03/13] ppc/spapr: Disable FWNMI capability with TCG

[David Gibson]

On Wed, Mar 04, 2020 at 03:28:40PM +1000, Nicholas Piggin wrote:
> Succeeding "ibm,nmi-register" RTAS calls but failing to actually
> operate to specification is unhelpful at best. Disable the capability
> when running on TCG.

Nack.

Providing (obviously) guest visibly different environments for TCG and
KVM is something we strenously avoid (there are some legacy ones, but
we're gradually fixing them).

In what way does TCG not do this correctly that's actually plausibly
important?  AFAICT TCG should basically be equivalent to a correct
implementation that never happens to deliver an NMI.

I explicitly asked Aravinda to make this only a warning, because it's
a real pain if you have to explicitly override a setting in order to
boot TCG at all.

> Signed-off-by: Nicholas Piggin <address@hidden>
> ---
>  hw/ppc/spapr_caps.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index f626d769a0..10864c7eed 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -517,12 +517,12 @@ static void cap_fwnmi_apply(SpaprMachineState *spapr, 
> uint8_t val,
>      }
>  
>      if (tcg_enabled()) {
> -        warn_report("Firmware Assisted Non-Maskable Interrupts(FWNMI) not "
> -                    "supported in TCG");
> +        error_setg(errp, "Firmware Assisted Non-Maskable Interrupts(FWNMI) 
> not "
> +                         "supported in TCG");
>      } else if (kvm_enabled()) {
>          if (kvmppc_set_fwnmi() < 0) {
>              error_setg(errp, "Firmware Assisted Non-Maskable 
> Interrupts(FWNMI) "
> -                             "not supported by KVM");
> +                             "not supported by KVM.");
>          }
>      }
>  }
> @@ -645,6 +645,10 @@ static SpaprCapabilities 
> default_caps_with_cpu(SpaprMachineState *spapr,
>  
>      caps = smc->default_caps;
>  
> +    if (tcg_enabled()) {
> +        caps.caps[SPAPR_CAP_FWNMI] = SPAPR_CAP_OFF;
> +    }
> +
>      if (!ppc_type_check_compat(cputype, CPU_POWERPC_LOGICAL_3_00,
>                                 0, spapr->max_compat_pvr)) {
>          caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_OFF;

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature