[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH v5 5/6] ppc: spapr: Enable FWNMI capability
From: |
Aravinda Prasad |
Subject: |
Re: [Qemu-ppc] [PATCH v5 5/6] ppc: spapr: Enable FWNMI capability |
Date: |
Sun, 8 Oct 2017 13:56:06 +0530 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 |
On Wednesday 04 October 2017 07:04 AM, David Gibson wrote:
> On Thu, Sep 28, 2017 at 04:08:21PM +0530, Aravinda Prasad wrote:
>> Enable the KVM capability KVM_CAP_PPC_FWNMI so that
>> the KVM causes guest exit with NMI as exit reason
>> when it encounters a machine check exception on the
>> address belonging to a guest. Without this capability
>> enabled, KVM redirects machine check exceptions to
>> guest's 0x200 vector.
>>
>> Signed-off-by: Aravinda Prasad <address@hidden>
>> ---
>> hw/ppc/spapr_rtas.c | 15 +++++++++++++++
>> target/ppc/kvm.c | 13 +++++++++++++
>> target/ppc/kvm_ppc.h | 6 ++++++
>> 3 files changed, 34 insertions(+)
>>
>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>> index 08e9a5e..d017a67 100644
>> --- a/hw/ppc/spapr_rtas.c
>> +++ b/hw/ppc/spapr_rtas.c
>> @@ -46,6 +46,7 @@
>> #include "qemu/cutils.h"
>> #include "trace.h"
>> #include "hw/ppc/fdt.h"
>> +#include "kvm_ppc.h"
>>
>> static void rtas_display_character(PowerPCCPU *cpu, sPAPRMachineState
>> *spapr,
>> uint32_t token, uint32_t nargs,
>> @@ -354,6 +355,20 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
>> target_ulong args,
>> uint32_t nret, target_ulong rets)
>> {
>> + int ret;
>> +
>> + ret = kvmppc_fwnmi_enable(cpu);
>
> If you're enabling it here, doesn't that mean you need to disable it
> on reset?
We don't have a way in KVM to disable it once enabled.
>
>> + if (ret == 1) {
>> + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
>> + return;
>> + }
>> +
>> + if (ret < 0) {
>> + rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
>> + return;
>> + }
>> +
>> spapr->guest_machine_check_addr = rtas_ld(args, 1);
>> rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>> }
>> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
>> index 7e4ce02..59b3322 100644
>> --- a/target/ppc/kvm.c
>> +++ b/target/ppc/kvm.c
>> @@ -92,6 +92,7 @@ static int cap_mmu_radix;
>> static int cap_mmu_hash_v3;
>> static int cap_resize_hpt;
>> static int cap_ppc_pvr_compat;
>> +static int cap_fwnmi;
>>
>> static uint32_t debug_inst_opcode;
>>
>> @@ -150,6 +151,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>> cap_mmu_radix = kvm_vm_check_extension(s, KVM_CAP_PPC_MMU_RADIX);
>> cap_mmu_hash_v3 = kvm_vm_check_extension(s, KVM_CAP_PPC_MMU_HASH_V3);
>> cap_resize_hpt = kvm_vm_check_extension(s, KVM_CAP_SPAPR_RESIZE_HPT);
>> + cap_fwnmi = kvm_check_extension(s, KVM_CAP_PPC_FWNMI);
>> /*
>> * Note: setting it to false because there is not such capability
>> * in KVM at this moment.
>> @@ -2142,6 +2144,17 @@ void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int
>> mpic_proxy)
>> }
>> }
>>
>> +int kvmppc_fwnmi_enable(PowerPCCPU *cpu)
>> +{
>> + CPUState *cs = CPU(cpu);
>> +
>> + if (!cap_fwnmi) {
>> + return 1;
>> + }
>> +
>> + return kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);
>
> Yeah, this is no good. It means migration from a host that's fwnmi
> capable to one that isn't will be subtly broken. Instead you need to
> make fwnmi capability a machine property. If the property is
> requested and the host kernel doesn't support it, you need to outright
> fail, rather than try to fall back.
Sure.
>
>> +}
>> +
>> int kvmppc_smt_threads(void)
>> {
>> return cap_ppc_smt ? cap_ppc_smt : 1;
>> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
>> index 0139dae..55b6df2 100644
>> --- a/target/ppc/kvm_ppc.h
>> +++ b/target/ppc/kvm_ppc.h
>> @@ -28,6 +28,7 @@ void kvmppc_enable_clear_ref_mod_hcalls(void);
>> void kvmppc_set_papr(PowerPCCPU *cpu);
>> int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr);
>> void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
>> +int kvmppc_fwnmi_enable(PowerPCCPU *cpu);
>> int kvmppc_smt_threads(void);
>> void kvmppc_hint_smt_possible(Error **errp);
>> int kvmppc_set_smt_threads(int smt);
>> @@ -157,6 +158,11 @@ static inline void kvmppc_set_mpic_proxy(PowerPCCPU
>> *cpu, int mpic_proxy)
>> {
>> }
>>
>> +int kvmppc_fwnmi_enable(PowerPCCPU *cpu)
>> +{
>> + return 1;
>
> Likewise, this should be available, not banned, on TCG. I think there
> are existing problems with TCG<->KVM migration, but there's no
> inherent reason they shouldn't work, so we don't want to introduce
> extra reasons they don't.
>
> Even if TCG will never generate fwnmis (for now), it should allow the
> guest to register for them.
Should this be then changed later only when TCG generates fwnmis?
Because we don't want to set spapr->guest_machine_check_addr for TCG in
rtas_ibm_nmi_register(). If set then all machine checks are redirected
to this address which is not desirable for TCG as we still want machine
checks to be passed via 0x200.
Regards,
Aravinda
>
>> +}
>> +
>> static inline int kvmppc_smt_threads(void)
>> {
>> return 1;
>>
>
--
Regards,
Aravinda