Re: [Qemu-devel] [QEMU-PPC] [PATCH V4 1/2] ppc/spapr-caps: Disallow setting workaround for spapr-cap-ibs

[David Gibson <david@gibson.dropbear.id.au>]

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

On Fri, Feb 16, 2018 at 01:33:27PM +1100, Suraj Jitindar Singh wrote:
> The spapr-cap cap-ibs can only have values broken or fixed as there is
> no explicit workaround required. Currently setting the value workaround
> for this cap will hit an assert if the guest makes the hcall
> h_get_cpu_characteristics.
> 
> Report an error when attempting to apply the setting with a more helpful
> error message.
> 
> Reported-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>

Applied, thanks.

> 
> ---
> 
> V3 -> V4:
>  - Add this patch back from V1 to replace
>    "ppc/spapr-caps: Convert spapr-cap-ibs to be a boolean"
>    as this was deemed to be a better solution
> ---
>  hw/ppc/spapr_caps.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index e69d308560..99a4b71d19 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -205,7 +205,9 @@ static void cap_safe_bounds_check_apply(sPAPRMachineState *spapr, uint8_t val,
>  static void cap_safe_indirect_branch_apply(sPAPRMachineState *spapr,
>                                             uint8_t val, Error **errp)
>  {
> -    if (tcg_enabled() && val) {
> +    if (val == SPAPR_CAP_WORKAROUND) { /* Can only be Broken or Fixed */
> +        error_setg(errp, "Requested safe indirect branch capability level \"workaround\" not valid, try cap-ibs=fixed");
> +    } else if (tcg_enabled() && val) {
>          /* TODO - for now only allow broken for TCG */
>          error_setg(errp, "Requested safe indirect branch capability level not supported by tcg, try a different value for cap-ibs");
>      } else if (kvm_enabled() && (val > kvmppc_get_cap_safe_indirect_branch())) {
> @@ -263,7 +265,7 @@ sPAPRCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
>      },
>      [SPAPR_CAP_IBS] = {
>          .name = "ibs",
> -        .description = "Indirect Branch Serialisation" VALUE_DESC_TRISTATE,
> +        .description = "Indirect Branch Serialisation (broken, fixed)",
>          .index = SPAPR_CAP_IBS,
>          .get = spapr_cap_get_tristate,
>          .set = spapr_cap_set_tristate,

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