From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58657) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ag1RO-0006Nn-AF for qemu-devel@nongnu.org; Tue, 15 Mar 2016 22:44:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ag1RJ-0004zE-GW for qemu-devel@nongnu.org; Tue, 15 Mar 2016 22:44:06 -0400 Received: from mail-pf0-x22d.google.com ([2607:f8b0:400e:c00::22d]:33300) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ag1RJ-0004z5-4v for qemu-devel@nongnu.org; Tue, 15 Mar 2016 22:44:01 -0400 Received: by mail-pf0-x22d.google.com with SMTP id 124so54661268pfg.0 for ; Tue, 15 Mar 2016 19:44:00 -0700 (PDT) References: <1458021080-2145-1-git-send-email-aik@ozlabs.ru> <56E7C573.8090405@redhat.com> <56E7D8ED.4070908@ozlabs.ru> <56E7F2D5.2040201@redhat.com> From: Alexey Kardashevskiy Message-ID: <56E8C868.6090508@ozlabs.ru> Date: Wed, 16 Mar 2016 13:43:52 +1100 MIME-Version: 1.0 In-Reply-To: <56E7F2D5.2040201@redhat.com> Content-Type: text/plain; charset=koi8-r; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH qemu] spapr/target-ppc/kvm: Only add hcall-instructions if KVM supports it List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thomas Huth , qemu-devel@nongnu.org Cc: qemu-ppc@nongnu.org, Paul Mackerras , Alexander Graf , David Gibson On 03/15/2016 10:32 PM, Thomas Huth wrote: > On 15.03.2016 10:42, Alexey Kardashevskiy wrote: >> On 03/15/2016 07:18 PM, Thomas Huth wrote: >>> >>> Hi Alexey, >>> >>> On 15.03.2016 06:51, Alexey Kardashevskiy wrote: >>>> ePAPR defines "hcall-instructions" device-tree property which contains >>>> code to call hypercalls in ePAPR paravirtualized guests. However this >>>> property is also present for pseries guests where it does not make >>>> sense, >>>> even though it contains dummy code which simply fails. >>>> >>>> Instead of maintaining the property (which used to be BE only; then was >>>> fixed to be endian-agnostic) and confusing the guest (which might think >>>> there is ePAPR host while there is none), this simply does not >>>> the property to the device tree if the host kernel does not implement >>>> it. >>>> >>>> In order to tell the machine code if the host kernel supports >>>> KVM_CAP_PPC_GET_PVINFO, this changes kvmppc_get_hypercall() to return 1 >>>> if the host kernel does not implement it (which is HV KVM case). >>>> >>>> Signed-off-by: Alexey Kardashevskiy >>>> --- >>>> >>>> >>>> Alexander, >>>> >>>> We just got a bug report that LE guests would not boot under quite >>>> old QEMU >>>> and we (powerkvm) wonder if it makes sense to backport endian-agnostic >>>> hypercall code to older QEMU or it is simpler/more correct >>>> not to have epapr-hypercall property in the tree. >>>> >>>> >>>> --- >>>> hw/ppc/spapr.c | 9 +++++---- >>>> target-ppc/kvm.c | 2 +- >>>> 2 files changed, 6 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >>>> index 43708a2..8130eb4 100644 >>>> --- a/hw/ppc/spapr.c >>>> +++ b/hw/ppc/spapr.c >>>> @@ -497,10 +497,11 @@ static void *spapr_create_fdt_skel(hwaddr >>>> initrd_base, >>>> * Older KVM versions with older guest kernels were >>>> broken with the >>>> * magic page, don't allow the guest to map it. >>>> */ >>>> - kvmppc_get_hypercall(first_cpu->env_ptr, hypercall, >>>> - sizeof(hypercall)); >>>> - _FDT((fdt_property(fdt, "hcall-instructions", hypercall, >>>> - sizeof(hypercall)))); >>>> + if (!kvmppc_get_hypercall(first_cpu->env_ptr, hypercall, >>>> + sizeof(hypercall))) { >>>> + _FDT((fdt_property(fdt, "hcall-instructions", >>>> hypercall, >>>> + sizeof(hypercall)))); >>>> + } >>>> } >>>> _FDT((fdt_end_node(fdt))); >>>> } >>>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c >>>> index 776336b..e5183db 100644 >>>> --- a/target-ppc/kvm.c >>>> +++ b/target-ppc/kvm.c >>>> @@ -2001,7 +2001,7 @@ int kvmppc_get_hypercall(CPUPPCState *env, >>>> uint8_t *buf, int buf_len) >>>> hc[2] = cpu_to_be32(0x48000008); >>>> hc[3] = cpu_to_be32(bswap32(0x3860ffff)); >>>> >>>> - return 0; >>>> + return 1; >>>> } >>>> >>>> static inline int kvmppc_enable_hcall(KVMState *s, target_ulong hcall) >>> >>> Sorry, I have a hard time to understand what this is really good for. Is >>> it a patch for current QEMU or for older ones? If it is for older ones, >>> then why did you not CC: to qemu-stable? >>> If it is for current QEMU, then I've got some more questions about >>> things I do not understand: >>> >>> 1) In your patch description, you talk about ePAPR and that the property >>> does not make sense for pseries. But why is this code then available at >>> all in spapr.c? ... there must be a reason for this, I think (like using >>> a different h-call on nested KVM-PR for example?) >> >> >> No, this is from old times when there was only PR KVM fully emulating >> powermac (not pseries) which needed to interact with the hypervisor and >> epapr_hypercall was chosen for this. >> >> >>> 2) The code in spapr.c is already protected with a >>> if (kvmppc_has_cap_fixup_hcalls()) ... >>> and that CAP should only be there if the PVINFO CAP is available, too. >>> So I don't see how you could run into that problem anyway where PVINFO >>> is _not_ available but the FIXUP_HCALL CAP _is_ available? >> >> >> HV KVM guest calls (on pseries machine as well): >> >> kvm_guest_init >> kvm_para_has_feature >> kvm_arch_para_features >> kvm_para_available - this returns "1" >> epapr_hypercall0_1(KVM_HC_FEATURES) >> >> This epapr_hypercall0_1() calls a binary blob from "hcall-instructions". >> And fails if the guest is LE and the blob from BE-only times. > > What about that "if (kvmppc_has_cap_fixup_hcalls())" ? Could you please > check why this succeeds on your system , but the KVM_CAP_PPC_GET_PVINFO > call does not? KVM_CAP_PPC_FIXUP_HCALL is always enabled for CONFIG_PPC_BOOK3S_64, KVM_CAP_PPC_GET_PVINFO is only enabled for "!hv_enabled". -- Alexey