Re: [PATCH 1/1] spapr_caps.c: disable KVM specific caps when running with TCG

[David Gibson]

On Tue, Jan 19, 2021 at 05:58:24PM -0300, Daniel Henrique Barboza wrote:
> Commit 006e9d361869 added warning messages for cap-cfpc, cap-ibs and
> cap-sbbc when enabled under TCG. Commit 8ff43ee404d3 did the same thing
> when introducing cap-ccf-assist.
> 
> These warning messages, although benign to the machine launch, can make
> users a bit confused. E.g:
> 
> $ sudo ./ppc64-softmmu/qemu-system-ppc64
> qemu-system-ppc64: warning: TCG doesn't support requested feature, 
> cap-cfpc=workaround
> qemu-system-ppc64: warning: TCG doesn't support requested feature, 
> cap-sbbc=workaround
> qemu-system-ppc64: warning: TCG doesn't support requested feature, 
> cap-ibs=workaround
> qemu-system-ppc64: warning: TCG doesn't support requested feature, 
> cap-ccf-assist=on
> 
> We're complaining about "TCG doesn't support requested feature" when the
> user didn't request any of those caps in the command line.
> 
> Check if we're running with TCG and change the defaults in spapr_caps_init().
> Note that this change doesn't impact backward compatibility or migration
> to older QEMU versions because we never activated these caps with TCG
> in the first place.

Nack.  Changing those capabilities changes guest visible properties of
the guest environment.  Silently altering guest visible
characteristics based on whether or not we're running with KVM is not
acceptable (we did it in the past and it caused a lot of grief).

> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>  hw/ppc/spapr_caps.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index 9341e9782a..53eea2b11e 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -781,6 +781,21 @@ void spapr_caps_init(SpaprMachineState *spapr)
>      /* Compute the actual set of caps we should run with */
>      default_caps = default_caps_with_cpu(spapr, MACHINE(spapr)->cpu_type);
>  
> +   /*
> +    * These are KVM specific caps that TCG doesn't support, but will
> +    * throw an warning if enabled by default (see 006e9d361869 and
> +    * 8ff43ee404d3). This behavior can make the user wonder why a warning
> +    * is being shown for caps that the user didn't enable in the
> +    * command line.
> +    *
> +    * Disable them for TCG. */
> +    if (tcg_enabled()) {
> +        default_caps.caps[SPAPR_CAP_CFPC] = SPAPR_CAP_BROKEN;
> +        default_caps.caps[SPAPR_CAP_SBBC] = SPAPR_CAP_BROKEN;
> +        default_caps.caps[SPAPR_CAP_IBS] = SPAPR_CAP_BROKEN;
> +        default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_BROKEN;
> +    }
> +
>      for (i = 0; i < SPAPR_CAP_NUM; i++) {
>          /* Store the defaults */
>          spapr->def.caps[i] = default_caps.caps[i];

-- 
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