qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/1] spapr_caps.c: check user input before warning about TCG only caps
@ 2021-01-20 10:54 Daniel Henrique Barboza
  2021-01-20 10:54 ` [PATCH v2 1/1] " Daniel Henrique Barboza
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Henrique Barboza @ 2021-01-20 10:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel Henrique Barboza, qemu-ppc, groug, david

changes from v1:
- check command line input to decide if the warning will be shown
  instead of setting capabilities based on running with TCG or not.
- v1 link: https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg04890.html


Daniel Henrique Barboza (1):
  spapr_caps.c: check user input before warning about TCG only caps

 hw/ppc/spapr_caps.c | 47 ++++++++++++++++++++++++++++++++++-----------
 1 file changed, 36 insertions(+), 11 deletions(-)

-- 
2.26.2



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 1/1] spapr_caps.c: check user input before warning about TCG only caps
  2021-01-20 10:54 [PATCH v2 0/1] spapr_caps.c: check user input before warning about TCG only caps Daniel Henrique Barboza
@ 2021-01-20 10:54 ` Daniel Henrique Barboza
  2021-01-20 11:16   ` Greg Kurz
  2021-01-23  1:46   ` David Gibson
  0 siblings, 2 replies; 5+ messages in thread
From: Daniel Henrique Barboza @ 2021-01-20 10:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel Henrique Barboza, qemu-ppc, groug, david

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 these caps were set in the command line before sending an user
warning.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/ppc/spapr_caps.c | 47 ++++++++++++++++++++++++++++++++++-----------
 1 file changed, 36 insertions(+), 11 deletions(-)

diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 9341e9782a..629c24a96d 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -244,9 +244,15 @@ static void cap_safe_cache_apply(SpaprMachineState *spapr, uint8_t val,
     uint8_t kvm_val =  kvmppc_get_cap_safe_cache();
 
     if (tcg_enabled() && val) {
-        /* TCG only supports broken, allow other values and print a warning */
-        warn_report("TCG doesn't support requested feature, cap-cfpc=%s",
-                    cap_cfpc_possible.vals[val]);
+        /*
+         * TCG only supports broken, allow other values and print a warning
+         * in case the user attempted to set a different value in the command
+         * line.
+         */
+        if (spapr->cmd_line_caps[SPAPR_CAP_CFPC] != SPAPR_CAP_BROKEN) {
+            warn_report("TCG doesn't support requested feature, cap-cfpc=%s",
+                        cap_cfpc_possible.vals[val]);
+        }
     } else if (kvm_enabled() && (val > kvm_val)) {
         error_setg(errp,
                    "Requested safe cache capability level not supported by KVM");
@@ -269,9 +275,15 @@ static void cap_safe_bounds_check_apply(SpaprMachineState *spapr, uint8_t val,
     uint8_t kvm_val =  kvmppc_get_cap_safe_bounds_check();
 
     if (tcg_enabled() && val) {
-        /* TCG only supports broken, allow other values and print a warning */
-        warn_report("TCG doesn't support requested feature, cap-sbbc=%s",
-                    cap_sbbc_possible.vals[val]);
+        /*
+         * TCG only supports broken, allow other values and print a warning
+         * in case the user attempted to set a different value in the command
+         * line.
+         */
+        if (spapr->cmd_line_caps[SPAPR_CAP_SBBC] != SPAPR_CAP_BROKEN) {
+            warn_report("TCG doesn't support requested feature, cap-sbbc=%s",
+                        cap_sbbc_possible.vals[val]);
+        }
     } else if (kvm_enabled() && (val > kvm_val)) {
         error_setg(errp,
 "Requested safe bounds check capability level not supported by KVM");
@@ -297,9 +309,15 @@ static void cap_safe_indirect_branch_apply(SpaprMachineState *spapr,
     uint8_t kvm_val = kvmppc_get_cap_safe_indirect_branch();
 
     if (tcg_enabled() && val) {
-        /* TCG only supports broken, allow other values and print a warning */
-        warn_report("TCG doesn't support requested feature, cap-ibs=%s",
-                    cap_ibs_possible.vals[val]);
+        /*
+         * TCG only supports broken, allow other values and print a warning
+         * in case the user attempted to set a different value in the command
+         * line.
+         */
+        if (spapr->cmd_line_caps[SPAPR_CAP_IBS] != SPAPR_CAP_BROKEN) {
+            warn_report("TCG doesn't support requested feature, cap-ibs=%s",
+                        cap_ibs_possible.vals[val]);
+        }
     } else if (kvm_enabled() && (val > kvm_val)) {
         error_setg(errp,
 "Requested safe indirect branch capability level not supported by KVM");
@@ -483,8 +501,15 @@ static void cap_ccf_assist_apply(SpaprMachineState *spapr, uint8_t val,
     uint8_t kvm_val = kvmppc_get_cap_count_cache_flush_assist();
 
     if (tcg_enabled() && val) {
-        /* TCG doesn't implement anything here, but allow with a warning */
-        warn_report("TCG doesn't support requested feature, cap-ccf-assist=on");
+        /*
+         * TCG doesn't implement anything here, but allow with a warning
+         * in case the user attempted to set a different value in the command
+         * line.
+         */
+        if (spapr->cmd_line_caps[SPAPR_CAP_CCF_ASSIST] != SPAPR_CAP_OFF) {
+            warn_report("TCG doesn't support requested feature, "
+                        "cap-ccf-assist=on");
+        }
     } else if (kvm_enabled() && (val > kvm_val)) {
         uint8_t kvm_ibs = kvmppc_get_cap_safe_indirect_branch();
 
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/1] spapr_caps.c: check user input before warning about TCG only caps
  2021-01-20 10:54 ` [PATCH v2 1/1] " Daniel Henrique Barboza
@ 2021-01-20 11:16   ` Greg Kurz
  2021-01-23  1:46   ` David Gibson
  1 sibling, 0 replies; 5+ messages in thread
From: Greg Kurz @ 2021-01-20 11:16 UTC (permalink / raw)
  To: Daniel Henrique Barboza; +Cc: qemu-ppc, qemu-devel, david

On Wed, 20 Jan 2021 07:54:06 -0300
Daniel Henrique Barboza <danielhb413@gmail.com> 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 these caps were set in the command line before sending an user
> warning.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---

Heh I've just posted a mail suggesting you to do something like that :)

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/ppc/spapr_caps.c | 47 ++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 36 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index 9341e9782a..629c24a96d 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -244,9 +244,15 @@ static void cap_safe_cache_apply(SpaprMachineState *spapr, uint8_t val,
>      uint8_t kvm_val =  kvmppc_get_cap_safe_cache();
>  
>      if (tcg_enabled() && val) {
> -        /* TCG only supports broken, allow other values and print a warning */
> -        warn_report("TCG doesn't support requested feature, cap-cfpc=%s",
> -                    cap_cfpc_possible.vals[val]);
> +        /*
> +         * TCG only supports broken, allow other values and print a warning
> +         * in case the user attempted to set a different value in the command
> +         * line.
> +         */
> +        if (spapr->cmd_line_caps[SPAPR_CAP_CFPC] != SPAPR_CAP_BROKEN) {
> +            warn_report("TCG doesn't support requested feature, cap-cfpc=%s",
> +                        cap_cfpc_possible.vals[val]);
> +        }
>      } else if (kvm_enabled() && (val > kvm_val)) {
>          error_setg(errp,
>                     "Requested safe cache capability level not supported by KVM");
> @@ -269,9 +275,15 @@ static void cap_safe_bounds_check_apply(SpaprMachineState *spapr, uint8_t val,
>      uint8_t kvm_val =  kvmppc_get_cap_safe_bounds_check();
>  
>      if (tcg_enabled() && val) {
> -        /* TCG only supports broken, allow other values and print a warning */
> -        warn_report("TCG doesn't support requested feature, cap-sbbc=%s",
> -                    cap_sbbc_possible.vals[val]);
> +        /*
> +         * TCG only supports broken, allow other values and print a warning
> +         * in case the user attempted to set a different value in the command
> +         * line.
> +         */
> +        if (spapr->cmd_line_caps[SPAPR_CAP_SBBC] != SPAPR_CAP_BROKEN) {
> +            warn_report("TCG doesn't support requested feature, cap-sbbc=%s",
> +                        cap_sbbc_possible.vals[val]);
> +        }
>      } else if (kvm_enabled() && (val > kvm_val)) {
>          error_setg(errp,
>  "Requested safe bounds check capability level not supported by KVM");
> @@ -297,9 +309,15 @@ static void cap_safe_indirect_branch_apply(SpaprMachineState *spapr,
>      uint8_t kvm_val = kvmppc_get_cap_safe_indirect_branch();
>  
>      if (tcg_enabled() && val) {
> -        /* TCG only supports broken, allow other values and print a warning */
> -        warn_report("TCG doesn't support requested feature, cap-ibs=%s",
> -                    cap_ibs_possible.vals[val]);
> +        /*
> +         * TCG only supports broken, allow other values and print a warning
> +         * in case the user attempted to set a different value in the command
> +         * line.
> +         */
> +        if (spapr->cmd_line_caps[SPAPR_CAP_IBS] != SPAPR_CAP_BROKEN) {
> +            warn_report("TCG doesn't support requested feature, cap-ibs=%s",
> +                        cap_ibs_possible.vals[val]);
> +        }
>      } else if (kvm_enabled() && (val > kvm_val)) {
>          error_setg(errp,
>  "Requested safe indirect branch capability level not supported by KVM");
> @@ -483,8 +501,15 @@ static void cap_ccf_assist_apply(SpaprMachineState *spapr, uint8_t val,
>      uint8_t kvm_val = kvmppc_get_cap_count_cache_flush_assist();
>  
>      if (tcg_enabled() && val) {
> -        /* TCG doesn't implement anything here, but allow with a warning */
> -        warn_report("TCG doesn't support requested feature, cap-ccf-assist=on");
> +        /*
> +         * TCG doesn't implement anything here, but allow with a warning
> +         * in case the user attempted to set a different value in the command
> +         * line.
> +         */
> +        if (spapr->cmd_line_caps[SPAPR_CAP_CCF_ASSIST] != SPAPR_CAP_OFF) {
> +            warn_report("TCG doesn't support requested feature, "
> +                        "cap-ccf-assist=on");
> +        }
>      } else if (kvm_enabled() && (val > kvm_val)) {
>          uint8_t kvm_ibs = kvmppc_get_cap_safe_indirect_branch();
>  



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/1] spapr_caps.c: check user input before warning about TCG only caps
  2021-01-20 10:54 ` [PATCH v2 1/1] " Daniel Henrique Barboza
  2021-01-20 11:16   ` Greg Kurz
@ 2021-01-23  1:46   ` David Gibson
  2021-01-25 11:27     ` Daniel Henrique Barboza
  1 sibling, 1 reply; 5+ messages in thread
From: David Gibson @ 2021-01-23  1:46 UTC (permalink / raw)
  To: Daniel Henrique Barboza; +Cc: qemu-ppc, qemu-devel, groug

[-- Attachment #1: Type: text/plain, Size: 6337 bytes --]

On Wed, Jan 20, 2021 at 07:54:06AM -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 these caps were set in the command line before sending an user
> warning.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Oof.  I have real mixed feelings about this.

So, yes, the warnings are annoying, but they're not meaningless.  They
are really indicating that the guest environment is different from the
one you requested (implicitly, via the machine version). The fact that
they are only warnings, not hard errors, is already a compromise
because otherwise there would be no real way to use TCG at all with
current machines.

In short, the warnings are scary because they're *meant* to be scary.
TCG will not, and cannot, supply the Spectre mitigations that are
expected on a current machine type.

I agree that the current behaviour is pretty irritating, but I don't
know that silently pretending TCG can do what's normally expected of
that command line is a great option either.


> ---
>  hw/ppc/spapr_caps.c | 47 ++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 36 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index 9341e9782a..629c24a96d 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -244,9 +244,15 @@ static void cap_safe_cache_apply(SpaprMachineState *spapr, uint8_t val,
>      uint8_t kvm_val =  kvmppc_get_cap_safe_cache();
>  
>      if (tcg_enabled() && val) {
> -        /* TCG only supports broken, allow other values and print a warning */
> -        warn_report("TCG doesn't support requested feature, cap-cfpc=%s",
> -                    cap_cfpc_possible.vals[val]);
> +        /*
> +         * TCG only supports broken, allow other values and print a warning
> +         * in case the user attempted to set a different value in the command
> +         * line.
> +         */
> +        if (spapr->cmd_line_caps[SPAPR_CAP_CFPC] != SPAPR_CAP_BROKEN) {
> +            warn_report("TCG doesn't support requested feature, cap-cfpc=%s",
> +                        cap_cfpc_possible.vals[val]);
> +        }
>      } else if (kvm_enabled() && (val > kvm_val)) {
>          error_setg(errp,
>                     "Requested safe cache capability level not supported by KVM");
> @@ -269,9 +275,15 @@ static void cap_safe_bounds_check_apply(SpaprMachineState *spapr, uint8_t val,
>      uint8_t kvm_val =  kvmppc_get_cap_safe_bounds_check();
>  
>      if (tcg_enabled() && val) {
> -        /* TCG only supports broken, allow other values and print a warning */
> -        warn_report("TCG doesn't support requested feature, cap-sbbc=%s",
> -                    cap_sbbc_possible.vals[val]);
> +        /*
> +         * TCG only supports broken, allow other values and print a warning
> +         * in case the user attempted to set a different value in the command
> +         * line.
> +         */
> +        if (spapr->cmd_line_caps[SPAPR_CAP_SBBC] != SPAPR_CAP_BROKEN) {
> +            warn_report("TCG doesn't support requested feature, cap-sbbc=%s",
> +                        cap_sbbc_possible.vals[val]);
> +        }
>      } else if (kvm_enabled() && (val > kvm_val)) {
>          error_setg(errp,
>  "Requested safe bounds check capability level not supported by KVM");
> @@ -297,9 +309,15 @@ static void cap_safe_indirect_branch_apply(SpaprMachineState *spapr,
>      uint8_t kvm_val = kvmppc_get_cap_safe_indirect_branch();
>  
>      if (tcg_enabled() && val) {
> -        /* TCG only supports broken, allow other values and print a warning */
> -        warn_report("TCG doesn't support requested feature, cap-ibs=%s",
> -                    cap_ibs_possible.vals[val]);
> +        /*
> +         * TCG only supports broken, allow other values and print a warning
> +         * in case the user attempted to set a different value in the command
> +         * line.
> +         */
> +        if (spapr->cmd_line_caps[SPAPR_CAP_IBS] != SPAPR_CAP_BROKEN) {
> +            warn_report("TCG doesn't support requested feature, cap-ibs=%s",
> +                        cap_ibs_possible.vals[val]);
> +        }
>      } else if (kvm_enabled() && (val > kvm_val)) {
>          error_setg(errp,
>  "Requested safe indirect branch capability level not supported by KVM");
> @@ -483,8 +501,15 @@ static void cap_ccf_assist_apply(SpaprMachineState *spapr, uint8_t val,
>      uint8_t kvm_val = kvmppc_get_cap_count_cache_flush_assist();
>  
>      if (tcg_enabled() && val) {
> -        /* TCG doesn't implement anything here, but allow with a warning */
> -        warn_report("TCG doesn't support requested feature, cap-ccf-assist=on");
> +        /*
> +         * TCG doesn't implement anything here, but allow with a warning
> +         * in case the user attempted to set a different value in the command
> +         * line.
> +         */
> +        if (spapr->cmd_line_caps[SPAPR_CAP_CCF_ASSIST] != SPAPR_CAP_OFF) {
> +            warn_report("TCG doesn't support requested feature, "
> +                        "cap-ccf-assist=on");
> +        }
>      } else if (kvm_enabled() && (val > kvm_val)) {
>          uint8_t kvm_ibs = kvmppc_get_cap_safe_indirect_branch();
>  

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/1] spapr_caps.c: check user input before warning about TCG only caps
  2021-01-23  1:46   ` David Gibson
@ 2021-01-25 11:27     ` Daniel Henrique Barboza
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel Henrique Barboza @ 2021-01-25 11:27 UTC (permalink / raw)
  To: David Gibson; +Cc: qemu-ppc, qemu-devel, groug



On 1/22/21 10:46 PM, David Gibson wrote:
> On Wed, Jan 20, 2021 at 07:54:06AM -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 these caps were set in the command line before sending an user
>> warning.
>>
>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> 
> Oof.  I have real mixed feelings about this.
> 
> So, yes, the warnings are annoying, but they're not meaningless.  They
> are really indicating that the guest environment is different from the
> one you requested (implicitly, via the machine version). The fact that
> they are only warnings, not hard errors, is already a compromise
> because otherwise there would be no real way to use TCG at all with
> current machines.
> 
> In short, the warnings are scary because they're *meant* to be scary.
> TCG will not, and cannot, supply the Spectre mitigations that are
> expected on a current machine type.

Quick story: I'm involved in helping folks in a local Brazilian college working
with QEMU in Power ([1] for more info). One user is trying to run a pseries TCG
guest in Windows 10, and he is having problems with the current pseries machine,
while we is still able to do it with the pseries-2.8 one (I'm actually surprised
that it works at all in Windows 10 TBH).

So this user ask me for help with this scenario because he didn't know how to
fix these TCG warnings, because he was thinking that they had something to do
with the problem he is having with his use case. I said that these warnings
could be safely ignored for TCG.

These warnings are indeed scary, as you said. But not in a helpful way. Consider
that most QEMU warnings are a call for action for the user to fix something, e.g.
an option that's about to be deprecated/no longer supported. In this case we're
warning the user of something that the user has no fault on, and more important,
can do nothing about it but to ignore. And this user interaction I had made me
realize that it's not trivial to ignore warnings when your use case is not
working as intended. You will attempt to fix the warnings before trying to open
a developer bug and so on.

What we're doing here I can call a 'developer warning', something to remind us,
developers, that TCG does not implement Spectre caps that are default in the
pseries machine. Well, I'd rather document somewhere (in tcg/README, or perhaps
create a hw/ppc/README since this is a pseries exclusive behavior) that TCG is
ignoring default Spectre caps of the pseries machine, than to issue warnings about
it.


> 
> I agree that the current behaviour is pretty irritating, but I don't
> know that silently pretending TCG can do what's normally expected of
> that command line is a great option either.


I can send a patch to change the messages to say something like "this can be safely
ignored. Use -machine cap-X=broken to hid it". At least we will inform TCG users
that these warning are not their fault and they shouldn't spend their time trying to
figure them out. But then, why issue a warning and tell the user "this is warning,
please ignore me"?



Thanks,


DHB



[1] https://openpower.ic.unicamp.br/minicloud/

> 
> 
>> ---
>>   hw/ppc/spapr_caps.c | 47 ++++++++++++++++++++++++++++++++++-----------
>>   1 file changed, 36 insertions(+), 11 deletions(-)
>>
>> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
>> index 9341e9782a..629c24a96d 100644
>> --- a/hw/ppc/spapr_caps.c
>> +++ b/hw/ppc/spapr_caps.c
>> @@ -244,9 +244,15 @@ static void cap_safe_cache_apply(SpaprMachineState *spapr, uint8_t val,
>>       uint8_t kvm_val =  kvmppc_get_cap_safe_cache();
>>   
>>       if (tcg_enabled() && val) {
>> -        /* TCG only supports broken, allow other values and print a warning */
>> -        warn_report("TCG doesn't support requested feature, cap-cfpc=%s",
>> -                    cap_cfpc_possible.vals[val]);
>> +        /*
>> +         * TCG only supports broken, allow other values and print a warning
>> +         * in case the user attempted to set a different value in the command
>> +         * line.
>> +         */
>> +        if (spapr->cmd_line_caps[SPAPR_CAP_CFPC] != SPAPR_CAP_BROKEN) {
>> +            warn_report("TCG doesn't support requested feature, cap-cfpc=%s",
>> +                        cap_cfpc_possible.vals[val]);
>> +        }
>>       } else if (kvm_enabled() && (val > kvm_val)) {
>>           error_setg(errp,
>>                      "Requested safe cache capability level not supported by KVM");
>> @@ -269,9 +275,15 @@ static void cap_safe_bounds_check_apply(SpaprMachineState *spapr, uint8_t val,
>>       uint8_t kvm_val =  kvmppc_get_cap_safe_bounds_check();
>>   
>>       if (tcg_enabled() && val) {
>> -        /* TCG only supports broken, allow other values and print a warning */
>> -        warn_report("TCG doesn't support requested feature, cap-sbbc=%s",
>> -                    cap_sbbc_possible.vals[val]);
>> +        /*
>> +         * TCG only supports broken, allow other values and print a warning
>> +         * in case the user attempted to set a different value in the command
>> +         * line.
>> +         */
>> +        if (spapr->cmd_line_caps[SPAPR_CAP_SBBC] != SPAPR_CAP_BROKEN) {
>> +            warn_report("TCG doesn't support requested feature, cap-sbbc=%s",
>> +                        cap_sbbc_possible.vals[val]);
>> +        }
>>       } else if (kvm_enabled() && (val > kvm_val)) {
>>           error_setg(errp,
>>   "Requested safe bounds check capability level not supported by KVM");
>> @@ -297,9 +309,15 @@ static void cap_safe_indirect_branch_apply(SpaprMachineState *spapr,
>>       uint8_t kvm_val = kvmppc_get_cap_safe_indirect_branch();
>>   
>>       if (tcg_enabled() && val) {
>> -        /* TCG only supports broken, allow other values and print a warning */
>> -        warn_report("TCG doesn't support requested feature, cap-ibs=%s",
>> -                    cap_ibs_possible.vals[val]);
>> +        /*
>> +         * TCG only supports broken, allow other values and print a warning
>> +         * in case the user attempted to set a different value in the command
>> +         * line.
>> +         */
>> +        if (spapr->cmd_line_caps[SPAPR_CAP_IBS] != SPAPR_CAP_BROKEN) {
>> +            warn_report("TCG doesn't support requested feature, cap-ibs=%s",
>> +                        cap_ibs_possible.vals[val]);
>> +        }
>>       } else if (kvm_enabled() && (val > kvm_val)) {
>>           error_setg(errp,
>>   "Requested safe indirect branch capability level not supported by KVM");
>> @@ -483,8 +501,15 @@ static void cap_ccf_assist_apply(SpaprMachineState *spapr, uint8_t val,
>>       uint8_t kvm_val = kvmppc_get_cap_count_cache_flush_assist();
>>   
>>       if (tcg_enabled() && val) {
>> -        /* TCG doesn't implement anything here, but allow with a warning */
>> -        warn_report("TCG doesn't support requested feature, cap-ccf-assist=on");
>> +        /*
>> +         * TCG doesn't implement anything here, but allow with a warning
>> +         * in case the user attempted to set a different value in the command
>> +         * line.
>> +         */
>> +        if (spapr->cmd_line_caps[SPAPR_CAP_CCF_ASSIST] != SPAPR_CAP_OFF) {
>> +            warn_report("TCG doesn't support requested feature, "
>> +                        "cap-ccf-assist=on");
>> +        }
>>       } else if (kvm_enabled() && (val > kvm_val)) {
>>           uint8_t kvm_ibs = kvmppc_get_cap_safe_indirect_branch();
>>   
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-01-25 11:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-20 10:54 [PATCH v2 0/1] spapr_caps.c: check user input before warning about TCG only caps Daniel Henrique Barboza
2021-01-20 10:54 ` [PATCH v2 1/1] " Daniel Henrique Barboza
2021-01-20 11:16   ` Greg Kurz
2021-01-23  1:46   ` David Gibson
2021-01-25 11:27     ` Daniel Henrique Barboza

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).