On Fri, Mar 18, 2022 at 10:41:19AM -0300, Fabiano Rosas wrote: > David Gibson <da...@gibson.dropbear.id.au> writes: > > > 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 <faro...@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. > > Ok. > > > 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. > > Indeed, If all parts are functioning this should never happen. I was > hacking in L0 and accidentally let some hcalls through. So I'm just > being overly cautions with this patch. If that will end up causing too > much confusion, we could drop this one.
Ok, having something check that case is reasonable - but as a "can't happen" it should abort, rather than returning something sensible to the guest. > > >> 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
signature.asc
Description: PGP signature