| Message ID | 20151216060847.10636.76722.stgit@aravindap |
|---|---|
| State | New |
| Headers | show |
On Wed, Dec 16, 2015 at 11:38:47AM +0530, Aravinda Prasad wrote: > Introduce a new KVM capability to control how KVM > behaves on machine check exception. > > Without this capability, KVM redirects machine check > exceptions to guest's 0x200 vector if the address in > error belongs to guest. With this capability KVM > causes a guest exit with NMI exit reason. > > This is required to avoid problem if a new kernel/KVM > is used with an old QEMU. As old QEMU might not > understand the new NMI exit type and treat it as a > fatal error, even though the guest could have actually > handled the error if the exception was delivered to > guest's 0x200 interrupt vector. > > PS: KVM_CAP_PPC_FWNMI is set to 121 as 119 and 120 are > used by KVM_CAP_GUEST_DEBUG_HW_BPS and KVM_CAP_GUEST_DEBUG_HW_WPS > in KVM code, but still not reflected in QEMU code. The commit message seems to be written as if this were the kernel patch adding the capability there, rather than the qemu patch using it. > Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com> > --- > hw/ppc/spapr_rtas.c | 17 +++++++++++++++++ > linux-headers/linux/kvm.h | 1 + > target-ppc/kvm.c | 2 ++ > 3 files changed, 20 insertions(+) > > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c > index 17c4672..53319da 100644 > --- a/hw/ppc/spapr_rtas.c > +++ b/hw/ppc/spapr_rtas.c > @@ -38,6 +38,8 @@ > #include <libfdt.h> > #include "hw/ppc/spapr_drc.h" > > +extern int cap_fwnmi; > + > /* #define DEBUG_SPAPR */ > > #ifdef DEBUG_SPAPR > @@ -603,9 +605,24 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu, > target_ulong args, > uint32_t nret, target_ulong rets) > { > + int ret; > + CPUState *cs = CPU(cpu); > + > + if (!cap_fwnmi) { > + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED); > + return; > + } > + > spapr->mc_in_progress = false; > qemu_cond_init(&spapr->mc_delivery_cond); > spapr->guest_machine_check_addr = rtas_ld(args, 1); > + > + ret = kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0); > + if (ret < 0) { > + rtas_st(rets, 0, RTAS_OUT_HW_ERROR); > + return; > + } > + > rtas_st(rets, 0, RTAS_OUT_SUCCESS); > } > > diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h > index 683f713..2db1fba 100644 > --- a/linux-headers/linux/kvm.h > +++ b/linux-headers/linux/kvm.h > @@ -819,6 +819,7 @@ struct kvm_ppc_smmu_info { > #define KVM_CAP_DISABLE_QUIRKS 116 > #define KVM_CAP_X86_SMM 117 > #define KVM_CAP_MULTI_ADDRESS_SPACE 118 > +#define KVM_CAP_PPC_FWNMI 121 > > #ifdef KVM_CAP_IRQ_ROUTING > > diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c > index 2bbb46d..5339c04 100644 > --- a/target-ppc/kvm.c > +++ b/target-ppc/kvm.c > @@ -74,6 +74,7 @@ static int cap_ppc_watchdog; > static int cap_papr; > static int cap_htab_fd; > static int cap_fixup_hcalls; > +int cap_fwnmi; > > static uint32_t debug_inst_opcode; > > @@ -116,6 +117,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s) > * only activated after this by kvmppc_set_papr() */ > cap_htab_fd = kvm_check_extension(s, KVM_CAP_PPC_HTAB_FD); > cap_fixup_hcalls = kvm_check_extension(s, KVM_CAP_PPC_FIXUP_HCALL); > + cap_fwnmi = kvm_check_extension(s, KVM_CAP_PPC_FWNMI); > > if (!cap_interrupt_level) { > fprintf(stderr, "KVM: Couldn't find level irq capability. Expect the " >
On Thursday 17 December 2015 09:32 AM, David Gibson wrote: > On Wed, Dec 16, 2015 at 11:38:47AM +0530, Aravinda Prasad wrote: >> Introduce a new KVM capability to control how KVM >> behaves on machine check exception. >> >> Without this capability, KVM redirects machine check >> exceptions to guest's 0x200 vector if the address in >> error belongs to guest. With this capability KVM >> causes a guest exit with NMI exit reason. >> >> This is required to avoid problem if a new kernel/KVM >> is used with an old QEMU. As old QEMU might not >> understand the new NMI exit type and treat it as a >> fatal error, even though the guest could have actually >> handled the error if the exception was delivered to >> guest's 0x200 interrupt vector. >> >> PS: KVM_CAP_PPC_FWNMI is set to 121 as 119 and 120 are >> used by KVM_CAP_GUEST_DEBUG_HW_BPS and KVM_CAP_GUEST_DEBUG_HW_WPS >> in KVM code, but still not reflected in QEMU code. > > The commit message seems to be written as if this were the kernel > patch adding the capability there, rather than the qemu patch using > it. > I will reword it. Regards, Aravinda >> Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com> >> --- >> hw/ppc/spapr_rtas.c | 17 +++++++++++++++++ >> linux-headers/linux/kvm.h | 1 + >> target-ppc/kvm.c | 2 ++ >> 3 files changed, 20 insertions(+) >> >> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c >> index 17c4672..53319da 100644 >> --- a/hw/ppc/spapr_rtas.c >> +++ b/hw/ppc/spapr_rtas.c >> @@ -38,6 +38,8 @@ >> #include <libfdt.h> >> #include "hw/ppc/spapr_drc.h" >> >> +extern int cap_fwnmi; >> + >> /* #define DEBUG_SPAPR */ >> >> #ifdef DEBUG_SPAPR >> @@ -603,9 +605,24 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu, >> target_ulong args, >> uint32_t nret, target_ulong rets) >> { >> + int ret; >> + CPUState *cs = CPU(cpu); >> + >> + if (!cap_fwnmi) { >> + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED); >> + return; >> + } >> + >> spapr->mc_in_progress = false; >> qemu_cond_init(&spapr->mc_delivery_cond); >> spapr->guest_machine_check_addr = rtas_ld(args, 1); >> + >> + ret = kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0); >> + if (ret < 0) { >> + rtas_st(rets, 0, RTAS_OUT_HW_ERROR); >> + return; >> + } >> + >> rtas_st(rets, 0, RTAS_OUT_SUCCESS); >> } >> >> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h >> index 683f713..2db1fba 100644 >> --- a/linux-headers/linux/kvm.h >> +++ b/linux-headers/linux/kvm.h >> @@ -819,6 +819,7 @@ struct kvm_ppc_smmu_info { >> #define KVM_CAP_DISABLE_QUIRKS 116 >> #define KVM_CAP_X86_SMM 117 >> #define KVM_CAP_MULTI_ADDRESS_SPACE 118 >> +#define KVM_CAP_PPC_FWNMI 121 >> >> #ifdef KVM_CAP_IRQ_ROUTING >> >> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c >> index 2bbb46d..5339c04 100644 >> --- a/target-ppc/kvm.c >> +++ b/target-ppc/kvm.c >> @@ -74,6 +74,7 @@ static int cap_ppc_watchdog; >> static int cap_papr; >> static int cap_htab_fd; >> static int cap_fixup_hcalls; >> +int cap_fwnmi; >> >> static uint32_t debug_inst_opcode; >> >> @@ -116,6 +117,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s) >> * only activated after this by kvmppc_set_papr() */ >> cap_htab_fd = kvm_check_extension(s, KVM_CAP_PPC_HTAB_FD); >> cap_fixup_hcalls = kvm_check_extension(s, KVM_CAP_PPC_FIXUP_HCALL); >> + cap_fwnmi = kvm_check_extension(s, KVM_CAP_PPC_FWNMI); >> >> if (!cap_interrupt_level) { >> fprintf(stderr, "KVM: Couldn't find level irq capability. Expect the " >> >
On 16/12/15 07:08, Aravinda Prasad wrote: > Introduce a new KVM capability to control how KVM > behaves on machine check exception. > > Without this capability, KVM redirects machine check > exceptions to guest's 0x200 vector if the address in > error belongs to guest. With this capability KVM > causes a guest exit with NMI exit reason. > > This is required to avoid problem if a new kernel/KVM > is used with an old QEMU. As old QEMU might not > understand the new NMI exit type and treat it as a > fatal error, even though the guest could have actually > handled the error if the exception was delivered to > guest's 0x200 interrupt vector. > > PS: KVM_CAP_PPC_FWNMI is set to 121 as 119 and 120 are > used by KVM_CAP_GUEST_DEBUG_HW_BPS and KVM_CAP_GUEST_DEBUG_HW_WPS > in KVM code, but still not reflected in QEMU code. > > Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com> > --- > hw/ppc/spapr_rtas.c | 17 +++++++++++++++++ > linux-headers/linux/kvm.h | 1 + > target-ppc/kvm.c | 2 ++ > 3 files changed, 20 insertions(+) > > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c > index 17c4672..53319da 100644 > --- a/hw/ppc/spapr_rtas.c > +++ b/hw/ppc/spapr_rtas.c > @@ -38,6 +38,8 @@ > #include <libfdt.h> > #include "hw/ppc/spapr_drc.h" > > +extern int cap_fwnmi; > + > /* #define DEBUG_SPAPR */ > > #ifdef DEBUG_SPAPR > @@ -603,9 +605,24 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu, > target_ulong args, > uint32_t nret, target_ulong rets) > { > + int ret; > + CPUState *cs = CPU(cpu); > + > + if (!cap_fwnmi) { > + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED); > + return; > + } > + > spapr->mc_in_progress = false; > qemu_cond_init(&spapr->mc_delivery_cond); > spapr->guest_machine_check_addr = rtas_ld(args, 1); > + > + ret = kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0); > + if (ret < 0) { > + rtas_st(rets, 0, RTAS_OUT_HW_ERROR); > + return; > + } > + > rtas_st(rets, 0, RTAS_OUT_SUCCESS); > } > > diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h > index 683f713..2db1fba 100644 > --- a/linux-headers/linux/kvm.h > +++ b/linux-headers/linux/kvm.h > @@ -819,6 +819,7 @@ struct kvm_ppc_smmu_info { > #define KVM_CAP_DISABLE_QUIRKS 116 > #define KVM_CAP_X86_SMM 117 > #define KVM_CAP_MULTI_ADDRESS_SPACE 118 > +#define KVM_CAP_PPC_FWNMI 121 > > #ifdef KVM_CAP_IRQ_ROUTING > As far as I know, the proper way to update the files in the linux-headers directory, is to use the scripts/update-linux-headers.sh script to refresh them. So you also have to make sure that the line with KVM_CAP_PPC_FWNMI has been accepted first by the upstream kernel folks, i.e. it should be first in Paolo's KVM tree for example before you can update the QEMU sources. Please also do that update in a separate patch (since it can get bigger sometimes; and that way it can also be ACKed separately from people who are not directly involved in the ppc stuff). Thomas
On 17/12/15 15:38, Aravinda Prasad wrote: > > > On Thursday 17 December 2015 09:32 AM, David Gibson wrote: >> On Wed, Dec 16, 2015 at 11:38:47AM +0530, Aravinda Prasad wrote: >>> Introduce a new KVM capability to control how KVM >>> behaves on machine check exception. >>> >>> Without this capability, KVM redirects machine check >>> exceptions to guest's 0x200 vector if the address in >>> error belongs to guest. With this capability KVM >>> causes a guest exit with NMI exit reason. >>> >>> This is required to avoid problem if a new kernel/KVM >>> is used with an old QEMU. As old QEMU might not >>> understand the new NMI exit type and treat it as a >>> fatal error, even though the guest could have actually >>> handled the error if the exception was delivered to >>> guest's 0x200 interrupt vector. >>> >>> PS: KVM_CAP_PPC_FWNMI is set to 121 as 119 and 120 are >>> used by KVM_CAP_GUEST_DEBUG_HW_BPS and KVM_CAP_GUEST_DEBUG_HW_WPS >>> in KVM code, but still not reflected in QEMU code. >> >> The commit message seems to be written as if this were the kernel >> patch adding the capability there, rather than the qemu patch using >> it. >> > > I will reword it. Was there any progress in regard of FWNMI? > > Regards, > Aravinda > >>> Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com> >>> --- >>> hw/ppc/spapr_rtas.c | 17 +++++++++++++++++ >>> linux-headers/linux/kvm.h | 1 + >>> target-ppc/kvm.c | 2 ++ >>> 3 files changed, 20 insertions(+) >>> >>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c >>> index 17c4672..53319da 100644 >>> --- a/hw/ppc/spapr_rtas.c >>> +++ b/hw/ppc/spapr_rtas.c >>> @@ -38,6 +38,8 @@ >>> #include <libfdt.h> >>> #include "hw/ppc/spapr_drc.h" >>> >>> +extern int cap_fwnmi; >>> + >>> /* #define DEBUG_SPAPR */ >>> >>> #ifdef DEBUG_SPAPR >>> @@ -603,9 +605,24 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu, >>> target_ulong args, >>> uint32_t nret, target_ulong rets) >>> { >>> + int ret; >>> + CPUState *cs = CPU(cpu); >>> + >>> + if (!cap_fwnmi) { >>> + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED); >>> + return; >>> + } >>> + >>> spapr->mc_in_progress = false; >>> qemu_cond_init(&spapr->mc_delivery_cond); >>> spapr->guest_machine_check_addr = rtas_ld(args, 1); >>> + >>> + ret = kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0); >>> + if (ret < 0) { >>> + rtas_st(rets, 0, RTAS_OUT_HW_ERROR); >>> + return; >>> + } >>> + >>> rtas_st(rets, 0, RTAS_OUT_SUCCESS); >>> } >>> >>> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h >>> index 683f713..2db1fba 100644 >>> --- a/linux-headers/linux/kvm.h >>> +++ b/linux-headers/linux/kvm.h >>> @@ -819,6 +819,7 @@ struct kvm_ppc_smmu_info { >>> #define KVM_CAP_DISABLE_QUIRKS 116 >>> #define KVM_CAP_X86_SMM 117 >>> #define KVM_CAP_MULTI_ADDRESS_SPACE 118 >>> +#define KVM_CAP_PPC_FWNMI 121 >>> >>> #ifdef KVM_CAP_IRQ_ROUTING >>> >>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c >>> index 2bbb46d..5339c04 100644 >>> --- a/target-ppc/kvm.c >>> +++ b/target-ppc/kvm.c >>> @@ -74,6 +74,7 @@ static int cap_ppc_watchdog; >>> static int cap_papr; >>> static int cap_htab_fd; >>> static int cap_fixup_hcalls; >>> +int cap_fwnmi; >>> >>> static uint32_t debug_inst_opcode; >>> >>> @@ -116,6 +117,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s) >>> * only activated after this by kvmppc_set_papr() */ >>> cap_htab_fd = kvm_check_extension(s, KVM_CAP_PPC_HTAB_FD); >>> cap_fixup_hcalls = kvm_check_extension(s, KVM_CAP_PPC_FIXUP_HCALL); >>> + cap_fwnmi = kvm_check_extension(s, KVM_CAP_PPC_FWNMI); >>> >>> if (!cap_interrupt_level) { >>> fprintf(stderr, "KVM: Couldn't find level irq capability. Expect the " >>> >> >
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c index 17c4672..53319da 100644 --- a/hw/ppc/spapr_rtas.c +++ b/hw/ppc/spapr_rtas.c @@ -38,6 +38,8 @@ #include <libfdt.h> #include "hw/ppc/spapr_drc.h" +extern int cap_fwnmi; + /* #define DEBUG_SPAPR */ #ifdef DEBUG_SPAPR @@ -603,9 +605,24 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu, target_ulong args, uint32_t nret, target_ulong rets) { + int ret; + CPUState *cs = CPU(cpu); + + if (!cap_fwnmi) { + rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED); + return; + } + spapr->mc_in_progress = false; qemu_cond_init(&spapr->mc_delivery_cond); spapr->guest_machine_check_addr = rtas_ld(args, 1); + + ret = kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0); + if (ret < 0) { + rtas_st(rets, 0, RTAS_OUT_HW_ERROR); + return; + } + rtas_st(rets, 0, RTAS_OUT_SUCCESS); } diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h index 683f713..2db1fba 100644 --- a/linux-headers/linux/kvm.h +++ b/linux-headers/linux/kvm.h @@ -819,6 +819,7 @@ struct kvm_ppc_smmu_info { #define KVM_CAP_DISABLE_QUIRKS 116 #define KVM_CAP_X86_SMM 117 #define KVM_CAP_MULTI_ADDRESS_SPACE 118 +#define KVM_CAP_PPC_FWNMI 121 #ifdef KVM_CAP_IRQ_ROUTING diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c index 2bbb46d..5339c04 100644 --- a/target-ppc/kvm.c +++ b/target-ppc/kvm.c @@ -74,6 +74,7 @@ static int cap_ppc_watchdog; static int cap_papr; static int cap_htab_fd; static int cap_fixup_hcalls; +int cap_fwnmi; static uint32_t debug_inst_opcode; @@ -116,6 +117,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s) * only activated after this by kvmppc_set_papr() */ cap_htab_fd = kvm_check_extension(s, KVM_CAP_PPC_HTAB_FD); cap_fixup_hcalls = kvm_check_extension(s, KVM_CAP_PPC_FIXUP_HCALL); + cap_fwnmi = kvm_check_extension(s, KVM_CAP_PPC_FWNMI); if (!cap_interrupt_level) { fprintf(stderr, "KVM: Couldn't find level irq capability. Expect the "
Introduce a new KVM capability to control how KVM behaves on machine check exception. Without this capability, KVM redirects machine check exceptions to guest's 0x200 vector if the address in error belongs to guest. With this capability KVM causes a guest exit with NMI exit reason. This is required to avoid problem if a new kernel/KVM is used with an old QEMU. As old QEMU might not understand the new NMI exit type and treat it as a fatal error, even though the guest could have actually handled the error if the exception was delivered to guest's 0x200 interrupt vector. PS: KVM_CAP_PPC_FWNMI is set to 121 as 119 and 120 are used by KVM_CAP_GUEST_DEBUG_HW_BPS and KVM_CAP_GUEST_DEBUG_HW_WPS in KVM code, but still not reflected in QEMU code. Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com> --- hw/ppc/spapr_rtas.c | 17 +++++++++++++++++ linux-headers/linux/kvm.h | 1 + target-ppc/kvm.c | 2 ++ 3 files changed, 20 insertions(+)