kvm, x86: unify sigbus handling, prep
PATCH 7/11 kvm, x86: unify sigbus handling, prep
There are 2 similar functions to handle SIGBUS:
sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo,
void *ctx)
kvm_on_sigbus(CPUState *env, siginfo_t *siginfo)
The former is used when main thread receives SIGBUS via signalfd, while latter is used when vcpu thread receives SIGBUS. These 2 take different siginfo, but in both case required parameters are common, the code and the addr in the info.
Restruct functions to take the code and the addr explicitly.
--- qemu-kvm.c | 41 ++++++++++++++++++++--------------------- 1 files changed, 20 insertions(+), 21 deletions(-)
qemu-kvm.c +20 -21
@@ -1219,32 +1219,28 @@ static void kvm_mce_inj_srao_broadcast(target_phys_addr_t paddr) }#endif -static void sigbus_handler (int n,struct qemu_signalfd_siginfo *siginfo, -void *ctx) +static void kvm_handle_sigbus (int code,void *vaddr) {#if defined(KVM_CAP_MCE ) && defined(TARGET_I386 ) -if ((first_cpu->mcg_cap &MCG_SER_P ) && siginfo->ssi_addr - && siginfo->ssi_code ==BUS_MCEERR_AO ) { -void *vaddr; +if ((first_cpu->mcg_cap &MCG_SER_P ) && vaddr && code ==BUS_MCEERR_AO ) {ram_addr_t ram_addr;target_phys_addr_t paddr;/* Hope we are lucky for AO MCE */ - vaddr = (void *)(intptr_t )siginfo->ssi_addr;if (do_qemu_ram_addr_from_host (vaddr, &ram_addr) || !kvm_physical_memory_addr_from_ram (kvm_state, ram_addr, &paddr)) {fprintf (stderr,"Hardware memory error for memory used by " "QEMU itself instead of guest system!: %llx\n" , - (unsigned long long )siginfo->ssi_addr); + (unsigned long long )vaddr);return ; }kvm_mce_inj_srao_broadcast (paddr); } else#endif { -if (siginfo->ssi_code ==BUS_MCEERR_AO ) { +if (code ==BUS_MCEERR_AO ) {return ; - }else if (siginfo->ssi_code ==BUS_MCEERR_AR ) { + }else if (code ==BUS_MCEERR_AR ) {hardware_memory_error (); }else {sigbus_reraise ();
@@ -1252,6 +1248,11 @@ static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo, } } +static void sigbus_handler (int n,struct qemu_signalfd_siginfo *ssi,void *ctx) +{ +kvm_handle_sigbus (ssi->ssi_code, (void *)(intptr_t )ssi->ssi_addr); +} +static void on_vcpu (CPUState *env,void (*func)(void *data),void *data) {struct qemu_work_item wi;
@@ -1377,36 +1378,34 @@ static void flush_queued_work(CPUState *env)pthread_cond_broadcast (&qemu_work_cond); } -static void kvm_on_sigbus (CPUState *env,siginfo_t *siginfo) +static void kvm_on_sigbus (CPUState *env,int code,void *vaddr) {#if defined(KVM_CAP_MCE ) && defined(TARGET_I386 ) -void *vaddr;ram_addr_t ram_addr;target_phys_addr_t paddr; -if ((env->mcg_cap &MCG_SER_P ) && siginfo->si_addr - && (siginfo->si_code ==BUS_MCEERR_AR - || siginfo->si_code ==BUS_MCEERR_AO )) { +if ((env->mcg_cap &MCG_SER_P ) && vaddr + && (code ==BUS_MCEERR_AR || code ==BUS_MCEERR_AO )) {/* * If there is an MCE excpetion being processed, ignore this SRAO MCE */ -if (siginfo->si_code ==BUS_MCEERR_AO &&kvm_mce_in_progress (env)) { + if (code == BUS_MCEERR_AO && kvm_mce_in_progress(env)) }return ; } - vaddr = (void *)siginfo->si_addr; +if (do_qemu_ram_addr_from_host (vaddr, &ram_addr) || !kvm_physical_memory_addr_from_ram (kvm_state, ram_addr, &paddr)) {fprintf (stderr,"Hardware memory error for memory used by " "QEMU itself instead of guest system!\n" );/* Hope we are lucky for AO MCE */ -if (siginfo->si_code ==BUS_MCEERR_AO ) { +if (code ==BUS_MCEERR_AO ) {return ; }else {hardware_memory_error (); } } -if (siginfo->si_code ==BUS_MCEERR_AR ) { +if (code ==BUS_MCEERR_AR ) {/* Fake an Intel architectural Data Load SRAR UCR */ kvm_mce_inj_srar_dataload (env, paddr); }else {
@@ -1416,9 +1415,9 @@ static void kvm_on_sigbus(CPUState *env, siginfo_t *siginfo) } else#endif { -if (siginfo->si_code ==BUS_MCEERR_AO ) { +if (code ==BUS_MCEERR_AO ) {return ; - }else if (siginfo->si_code ==BUS_MCEERR_AR ) { + }else if (code ==BUS_MCEERR_AR ) {hardware_memory_error (); }else {sigbus_reraise ();
@@ -1455,7 +1454,7 @@ static void kvm_main_loop_wait(CPUState *env, int timeout)switch (r) {case SIGBUS : -kvm_on_sigbus (env, &siginfo); +kvm_on_sigbus (env, siginfo.si_code, (void *)siginfo.si_addr);break ;default :break ;
Signature · 1 line
1.7.1.1
Quoted 10 lines · Jin Dongming
There are 2 similar functions to handle SIGBUS:
sigbus_handler(int n, structqemu_signalfd_siginfo*siginfo,
void *ctx)
kvm_on_sigbus(CPUState *env, siginfo_t *siginfo)The former is used when main thread receives SIGBUS via signalfd, while latter is used when vcpu thread receives SIGBUS. These 2 take different siginfo, but in both case required parameters are common, the code and the addr in the info.
Quoted 7 lines
Restruct functions to take the code and the addr explicitly.
Signed-off-by Hidetoshi Seto <[email protected]>Tested-by Jin Dongming <[email protected]>--- qemu-kvm.c | 41 ++++++++++++++++++++--------------------- 1 files changed, 20 insertions(+), 21 deletions(-)
Don't see the benefit, separate functions are cleaner.
Quoted 13 lines · Marcelo Tosatti
Quoted 12 lines · Jin Dongming
There are 2 similar functions to handle SIGBUS:
sigbus_handler(int n, structqemu_signalfd_siginfo*siginfo,
void *ctx)
kvm_on_sigbus(CPUState *env, siginfo_t *siginfo)The former is used when main thread receives SIGBUS via signalfd, while latter is used when vcpu thread receives SIGBUS. These 2 take different siginfo, but in both case required parameters are common, the code and the addr in the info.
Restruct functions to take the code and the addr explicitly.
Quoted 7 lines
Quoted 5 lines
Signed-off-by Hidetoshi Seto <[email protected]>Tested-by Jin Dongming <[email protected]>--- qemu-kvm.c | 41 ++++++++++++++++++++--------------------- 1 files changed, 20 insertions(+), 21 deletions(-)Don't see the benefit, separate functions are cleaner.
I think this is good for maintainability.
If you want to fix a bug in this area, you might have to change 2 separate functions in completely same way.
See 6c85786 and a05684e for examples.
Thanks,
H.Seto
Quoted 22 lines · Hidetoshi Seto
Quoted 13 lines · Marcelo Tosatti
Quoted 12 lines · Jin Dongming
There are 2 similar functions to handle SIGBUS:
sigbus_handler(int n, structqemu_signalfd_siginfo*siginfo,
void *ctx)
kvm_on_sigbus(CPUState *env, siginfo_t *siginfo)The former is used when main thread receives SIGBUS via signalfd, while latter is used when vcpu thread receives SIGBUS. These 2 take different siginfo, but in both case required parameters are common, the code and the addr in the info.
Restruct functions to take the code and the addr explicitly.
Quoted 7 lines
Quoted 5 lines
Signed-off-by Hidetoshi Seto <[email protected]>Tested-by Jin Dongming <[email protected]>--- qemu-kvm.c | 41 ++++++++++++++++++++--------------------- 1 files changed, 20 insertions(+), 21 deletions(-)Don't see the benefit, separate functions are cleaner.
Quoted 7 lines
I think this is good for maintainability.
If you want to fix a bug in this area, you might have to change 2 separate functions in completely same way.
See 6c85786 and a05684e for examples.Thanks,
H.Seto
OK, makes sense. Do you mind to rebase your patchset after MCE support is merged in qemu upstream?