Re: [PATCH 1/3] spapr: Ignore nested KVM hypercalls when not running TCG

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

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

On Thu, Mar 17, 2022 at 02:20:47PM -0300, Fabiano Rosas wrote:
> It is possible that nested KVM hypercalls reach QEMU while we're
> running KVM. The spapr virtual hypervisor implementation of the nested
> KVM API only works when the L1 is running under TCG. So return
> H_FUNCTION if we are under KVM.
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
>  hw/ppc/spapr_hcall.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index f008290787..119baa1d2d 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1508,7 +1508,7 @@ static target_ulong h_set_ptbl(PowerPCCPU *cpu,
>  {
>      target_ulong ptcr = args[0];
>  
> -    if (!spapr_get_cap(spapr, SPAPR_CAP_NESTED_KVM_HV)) {
> +    if (!spapr_get_cap(spapr, SPAPR_CAP_NESTED_KVM_HV) || !tcg_enabled()) {

I was about to nack this on the grounds that it changes guest visible
behaviour based on host properties.  Then I realized that's not the
case, because in the KVM + SPAPR_CAP_NESTED_KVM_HV case the hypercall
should be caught by KVM first and never reach here.

So at the very least I think this needs a comment explaining that.

However, I'm still kind of confused how we would get here in the first
place.  If SPAPR_CAP_NESTED_KVM_HV is set, but KVM doesn't support it,
we should fail outright in cap_nested_kvm_hv_apply().  So how *do* we
get here?  Is the kernel not doing what we expect of it?  If so, we
should probably abort, rather than just returning H_FUNCTION.


>          return H_FUNCTION;
>      }
>  
> @@ -1532,6 +1532,10 @@ static target_ulong h_tlb_invalidate(PowerPCCPU *cpu,
>       * across L1<->L2 transitions, so nothing is required here.
>       */
>  
> +    if (!tcg_enabled()) {
> +        return H_FUNCTION;
> +    }
> +
>      return H_SUCCESS;
>  }
>  
> @@ -1572,6 +1576,10 @@ static target_ulong h_enter_nested(PowerPCCPU *cpu,
>      uint64_t cr;
>      int i;
>  
> +    if (!tcg_enabled()) {
> +        return H_FUNCTION;
> +    }
> +
>      if (spapr->nested_ptcr == 0) {
>          return H_NOT_AVAILABLE;
>      }

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