Message ID | 1429257161-29597-4-git-send-email-cornelia.huck@de.ibm.com |
---|---|
State | New |
Headers | show |
On 04/17/2015 09:52 AM, Cornelia Huck wrote: > From: Xu Wang <gesaint@linux.vnet.ibm.com> > > Intercept the diag288 requests from kvm guests, and hand the > requested command to the diag288 watchdog device for further > handling. > > Signed-off-by: Xu Wang <gesaint@linux.vnet.ibm.com> > Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com> > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com> We're getting a lot of random devices allocating diag intercepts. Can't we make this an actual interface, similar to the hypercall registration on sPAPR? Alex > --- > target-s390x/cpu.h | 1 + > target-s390x/kvm.c | 18 ++++++++++++++++++ > target-s390x/misc_helper.c | 33 +++++++++++++++++++++++++++++++++ > 3 files changed, 52 insertions(+) > > diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h > index ba7d250..f5cafc3 100644 > --- a/target-s390x/cpu.h > +++ b/target-s390x/cpu.h > @@ -1059,6 +1059,7 @@ uint32_t set_cc_nz_f128(float128 v); > > /* misc_helper.c */ > #ifndef CONFIG_USER_ONLY > +int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3); > void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3); > #endif > void program_interrupt(CPUS390XState *env, uint32_t code, int ilen); > diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c > index b8703b8..32ed4ab 100644 > --- a/target-s390x/kvm.c > +++ b/target-s390x/kvm.c > @@ -97,6 +97,7 @@ > #define PRIV_E3_MPCIFC 0xd0 > #define PRIV_E3_STPCIFC 0xd4 > > +#define DIAG_TIMEREVENT 0x288 > #define DIAG_IPL 0x308 > #define DIAG_KVM_HYPERCALL 0x500 > #define DIAG_KVM_BREAKPOINT 0x501 > @@ -1215,6 +1216,20 @@ static int handle_hypercall(S390CPU *cpu, struct kvm_run *run) > return ret; > } > > +static void kvm_handle_diag_288(S390CPU *cpu, struct kvm_run *run) > +{ > + uint64_t r1, r3; > + int rc; > + > + cpu_synchronize_state(CPU(cpu)); > + r1 = (run->s390_sieic.ipa & 0x00f0) >> 4; > + r3 = run->s390_sieic.ipa & 0x000f; > + rc = handle_diag_288(&cpu->env, r1, r3); > + if (rc == -1) { > + enter_pgmcheck(cpu, PGM_SPECIFICATION); > + } > +} > + > static void kvm_handle_diag_308(S390CPU *cpu, struct kvm_run *run) > { > uint64_t r1, r3; > @@ -1254,6 +1269,9 @@ static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb) > */ > func_code = decode_basedisp_rs(&cpu->env, ipb, NULL) & DIAG_KVM_CODE_MASK; > switch (func_code) { > + case DIAG_TIMEREVENT: > + kvm_handle_diag_288(cpu, run); > + break; > case DIAG_IPL: > kvm_handle_diag_308(cpu, run); > break; > diff --git a/target-s390x/misc_helper.c b/target-s390x/misc_helper.c > index e1007fa..88d96bf 100644 > --- a/target-s390x/misc_helper.c > +++ b/target-s390x/misc_helper.c > @@ -30,6 +30,7 @@ > #include <linux/kvm.h> > #endif > #include "exec/cpu_ldst.h" > +#include "hw/watchdog/wdt_diag288.h" > > #if !defined(CONFIG_USER_ONLY) > #include "sysemu/cpus.h" > @@ -153,6 +154,38 @@ static int load_normal_reset(S390CPU *cpu) > return 0; > } > > +int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3) > +{ > + uint64_t func = env->regs[r1]; > + uint64_t timeout = env->regs[r1 + 1]; > + uint64_t action = env->regs[r3]; > + Object *obj; > + DIAG288State *diag288; > + DIAG288Class *diag288_class; > + > + if (r1 % 2) { > + return -1; > + } > + > + if (action != 0) { > + return -1; > + } > + > + /* Timeout must be more than 15 seconds except for timer deletion */ > + if (func != WDT_DIAG288_CANCEL && timeout < 15) { > + return -1; > + } > + > + obj = object_resolve_path_type("", TYPE_WDT_DIAG288, NULL); > + if (!obj) { > + return -1; > + } > + > + diag288 = DIAG288(obj); > + diag288_class = DIAG288_GET_CLASS(diag288); > + return diag288_class->handle_timer(diag288, func, timeout); > +} > + > #define DIAG_308_RC_OK 0x0001 > #define DIAG_308_RC_NO_CONF 0x0102 > #define DIAG_308_RC_INVALID 0x0402
On Tue, 21 Apr 2015 21:09:05 +0200 Alexander Graf <agraf@suse.de> wrote: > On 04/17/2015 09:52 AM, Cornelia Huck wrote: > > From: Xu Wang <gesaint@linux.vnet.ibm.com> > > > > Intercept the diag288 requests from kvm guests, and hand the > > requested command to the diag288 watchdog device for further > > handling. > > > > Signed-off-by: Xu Wang <gesaint@linux.vnet.ibm.com> > > Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com> > > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com> > > We're getting a lot of random devices allocating diag intercepts. Can't > we make this an actual interface, similar to the hypercall registration > on sPAPR? I've looked at the sPAPR hcall code, and it seems to basically provide a table with a nice registration interface (we already use something similar for the diagnose 500 virtio subcodes, btw.) While we could move our basic diagnose handling over to a table-like approach and registering new diagnoses, I think this is orthogonal to introducing a diag288 watchdog device: It just makes sense to model the watchdog as a device that just happens to be poked via a diagnose. If we introduce any further diagnoses to manipulate timing etc., I agree we don't want to add a device for each of these.
On 04/22/2015 10:32 AM, Cornelia Huck wrote: > On Tue, 21 Apr 2015 21:09:05 +0200 > Alexander Graf <agraf@suse.de> wrote: > >> On 04/17/2015 09:52 AM, Cornelia Huck wrote: >>> From: Xu Wang <gesaint@linux.vnet.ibm.com> >>> >>> Intercept the diag288 requests from kvm guests, and hand the >>> requested command to the diag288 watchdog device for further >>> handling. >>> >>> Signed-off-by: Xu Wang <gesaint@linux.vnet.ibm.com> >>> Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com> >>> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com> >> We're getting a lot of random devices allocating diag intercepts. Can't >> we make this an actual interface, similar to the hypercall registration >> on sPAPR? > I've looked at the sPAPR hcall code, and it seems to basically provide > a table with a nice registration interface (we already use something > similar for the diagnose 500 virtio subcodes, btw.) > > While we could move our basic diagnose handling over to a table-like > approach and registering new diagnoses, I think this is orthogonal to > introducing a diag288 watchdog device: It just makes sense to model the > watchdog as a device that just happens to be poked via a diagnose. If > we introduce any further diagnoses to manipulate timing etc., I agree > we don't want to add a device for each of these. My thinking was in the opposite level. I really like the idea of having separate devices for each function. But I think that they should be completely self-contained with well defined interfaces. Alex
On Wed, 22 Apr 2015 11:16:00 +0200 Alexander Graf <agraf@suse.de> wrote: > On 04/22/2015 10:32 AM, Cornelia Huck wrote: > > On Tue, 21 Apr 2015 21:09:05 +0200 > > Alexander Graf <agraf@suse.de> wrote: > > > >> On 04/17/2015 09:52 AM, Cornelia Huck wrote: > >>> From: Xu Wang <gesaint@linux.vnet.ibm.com> > >>> > >>> Intercept the diag288 requests from kvm guests, and hand the > >>> requested command to the diag288 watchdog device for further > >>> handling. > >>> > >>> Signed-off-by: Xu Wang <gesaint@linux.vnet.ibm.com> > >>> Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com> > >>> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com> > >> We're getting a lot of random devices allocating diag intercepts. Can't > >> we make this an actual interface, similar to the hypercall registration > >> on sPAPR? > > I've looked at the sPAPR hcall code, and it seems to basically provide > > a table with a nice registration interface (we already use something > > similar for the diagnose 500 virtio subcodes, btw.) > > > > While we could move our basic diagnose handling over to a table-like > > approach and registering new diagnoses, I think this is orthogonal to > > introducing a diag288 watchdog device: It just makes sense to model the > > watchdog as a device that just happens to be poked via a diagnose. If > > we introduce any further diagnoses to manipulate timing etc., I agree > > we don't want to add a device for each of these. > > My thinking was in the opposite level. I really like the idea of having > separate devices for each function. But I think that they should be > completely self-contained with well defined interfaces. I don't know... having a device for e.g. a hypercall that changes some timing characteristics feels a bit artificial to me.
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h index ba7d250..f5cafc3 100644 --- a/target-s390x/cpu.h +++ b/target-s390x/cpu.h @@ -1059,6 +1059,7 @@ uint32_t set_cc_nz_f128(float128 v); /* misc_helper.c */ #ifndef CONFIG_USER_ONLY +int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3); void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3); #endif void program_interrupt(CPUS390XState *env, uint32_t code, int ilen); diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c index b8703b8..32ed4ab 100644 --- a/target-s390x/kvm.c +++ b/target-s390x/kvm.c @@ -97,6 +97,7 @@ #define PRIV_E3_MPCIFC 0xd0 #define PRIV_E3_STPCIFC 0xd4 +#define DIAG_TIMEREVENT 0x288 #define DIAG_IPL 0x308 #define DIAG_KVM_HYPERCALL 0x500 #define DIAG_KVM_BREAKPOINT 0x501 @@ -1215,6 +1216,20 @@ static int handle_hypercall(S390CPU *cpu, struct kvm_run *run) return ret; } +static void kvm_handle_diag_288(S390CPU *cpu, struct kvm_run *run) +{ + uint64_t r1, r3; + int rc; + + cpu_synchronize_state(CPU(cpu)); + r1 = (run->s390_sieic.ipa & 0x00f0) >> 4; + r3 = run->s390_sieic.ipa & 0x000f; + rc = handle_diag_288(&cpu->env, r1, r3); + if (rc == -1) { + enter_pgmcheck(cpu, PGM_SPECIFICATION); + } +} + static void kvm_handle_diag_308(S390CPU *cpu, struct kvm_run *run) { uint64_t r1, r3; @@ -1254,6 +1269,9 @@ static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb) */ func_code = decode_basedisp_rs(&cpu->env, ipb, NULL) & DIAG_KVM_CODE_MASK; switch (func_code) { + case DIAG_TIMEREVENT: + kvm_handle_diag_288(cpu, run); + break; case DIAG_IPL: kvm_handle_diag_308(cpu, run); break; diff --git a/target-s390x/misc_helper.c b/target-s390x/misc_helper.c index e1007fa..88d96bf 100644 --- a/target-s390x/misc_helper.c +++ b/target-s390x/misc_helper.c @@ -30,6 +30,7 @@ #include <linux/kvm.h> #endif #include "exec/cpu_ldst.h" +#include "hw/watchdog/wdt_diag288.h" #if !defined(CONFIG_USER_ONLY) #include "sysemu/cpus.h" @@ -153,6 +154,38 @@ static int load_normal_reset(S390CPU *cpu) return 0; } +int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3) +{ + uint64_t func = env->regs[r1]; + uint64_t timeout = env->regs[r1 + 1]; + uint64_t action = env->regs[r3]; + Object *obj; + DIAG288State *diag288; + DIAG288Class *diag288_class; + + if (r1 % 2) { + return -1; + } + + if (action != 0) { + return -1; + } + + /* Timeout must be more than 15 seconds except for timer deletion */ + if (func != WDT_DIAG288_CANCEL && timeout < 15) { + return -1; + } + + obj = object_resolve_path_type("", TYPE_WDT_DIAG288, NULL); + if (!obj) { + return -1; + } + + diag288 = DIAG288(obj); + diag288_class = DIAG288_GET_CLASS(diag288); + return diag288_class->handle_timer(diag288, func, timeout); +} + #define DIAG_308_RC_OK 0x0001 #define DIAG_308_RC_NO_CONF 0x0102 #define DIAG_308_RC_INVALID 0x0402