| Message ID | 20171204125505.29203-6-david@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | s390x/tcg: CCW hotplug support | expand |
On Mon, 4 Dec 2017 13:55:05 +0100 David Hildenbrand <david@redhat.com> wrote: > We somehow missed that, new kernels require it. Why _new_ kernels? I think we have unconditionally issued stcrw since back in 2.2? > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > target/s390x/helper.h | 1 + > target/s390x/insn-data.def | 1 + > target/s390x/misc_helper.c | 9 +++++++++ > target/s390x/translate.c | 8 ++++++++ > 4 files changed, 19 insertions(+)
On 04.12.2017 18:22, Cornelia Huck wrote: > On Mon, 4 Dec 2017 13:55:05 +0100 > David Hildenbrand <david@redhat.com> wrote: > >> We somehow missed that, new kernels require it. > > Why _new_ kernels? I think we have unconditionally issued stcrw since > back in 2.2? Okay, the problem is then rather related to my setup. No ccw devices -> no stcrw. How could we ever add ccw devices to a TCG guest then? > >> >> Signed-off-by: David Hildenbrand <david@redhat.com> >> --- >> target/s390x/helper.h | 1 + >> target/s390x/insn-data.def | 1 + >> target/s390x/misc_helper.c | 9 +++++++++ >> target/s390x/translate.c | 8 ++++++++ >> 4 files changed, 19 insertions(+)
On Mon, 4 Dec 2017 18:34:36 +0100 David Hildenbrand <david@redhat.com> wrote: > On 04.12.2017 18:22, Cornelia Huck wrote: > > On Mon, 4 Dec 2017 13:55:05 +0100 > > David Hildenbrand <david@redhat.com> wrote: > > > >> We somehow missed that, new kernels require it. > > > > Why _new_ kernels? I think we have unconditionally issued stcrw since > > back in 2.2? > > Okay, the problem is then rather related to my setup. No ccw devices -> > no stcrw. > > How could we ever add ccw devices to a TCG guest then? Hotplug never worked before your patches. Coldplug does not create machine checks and thus does not trigger the guest to do STCRW. (Only STSCH, in keeping with "all channel I/O acronyms look the same".)
On 04.12.2017 18:53, Cornelia Huck wrote: > On Mon, 4 Dec 2017 18:34:36 +0100 > David Hildenbrand <david@redhat.com> wrote: > >> On 04.12.2017 18:22, Cornelia Huck wrote: >>> On Mon, 4 Dec 2017 13:55:05 +0100 >>> David Hildenbrand <david@redhat.com> wrote: >>> >>>> We somehow missed that, new kernels require it. >>> >>> Why _new_ kernels? I think we have unconditionally issued stcrw since >>> back in 2.2? >> >> Okay, the problem is then rather related to my setup. No ccw devices -> >> no stcrw. >> >> How could we ever add ccw devices to a TCG guest then? > > Hotplug never worked before your patches. Coldplug does not create > machine checks and thus does not trigger the guest to do STCRW. (Only > STSCH, in keeping with "all channel I/O acronyms look the same".) > But booting Fedora 26/27 without any involved hotplugs (therefore machine checks) triggers a STCRW. That's how I originally found it. (I started playing with machine checks after I had fedora 26/27 running)
On Mon, 4 Dec 2017 18:56:00 +0100 David Hildenbrand <david@redhat.com> wrote: > On 04.12.2017 18:53, Cornelia Huck wrote: > > On Mon, 4 Dec 2017 18:34:36 +0100 > > David Hildenbrand <david@redhat.com> wrote: > > > >> On 04.12.2017 18:22, Cornelia Huck wrote: > >>> On Mon, 4 Dec 2017 13:55:05 +0100 > >>> David Hildenbrand <david@redhat.com> wrote: > >>> > >>>> We somehow missed that, new kernels require it. > >>> > >>> Why _new_ kernels? I think we have unconditionally issued stcrw since > >>> back in 2.2? > >> > >> Okay, the problem is then rather related to my setup. No ccw devices -> > >> no stcrw. > >> > >> How could we ever add ccw devices to a TCG guest then? > > > > Hotplug never worked before your patches. Coldplug does not create > > machine checks and thus does not trigger the guest to do STCRW. (Only > > STSCH, in keeping with "all channel I/O acronyms look the same".) > > > > But booting Fedora 26/27 without any involved hotplugs (therefore > machine checks) triggers a STCRW. > > That's how I originally found it. (I started playing with machine checks > after I had fedora 26/27 running) > Confused. Do you have a backtrace? (Guest/host)
On 04.12.2017 18:58, Cornelia Huck wrote: > On Mon, 4 Dec 2017 18:56:00 +0100 > David Hildenbrand <david@redhat.com> wrote: > >> On 04.12.2017 18:53, Cornelia Huck wrote: >>> On Mon, 4 Dec 2017 18:34:36 +0100 >>> David Hildenbrand <david@redhat.com> wrote: >>> >>>> On 04.12.2017 18:22, Cornelia Huck wrote: >>>>> On Mon, 4 Dec 2017 13:55:05 +0100 >>>>> David Hildenbrand <david@redhat.com> wrote: >>>>> >>>>>> We somehow missed that, new kernels require it. >>>>> >>>>> Why _new_ kernels? I think we have unconditionally issued stcrw since >>>>> back in 2.2? >>>> >>>> Okay, the problem is then rather related to my setup. No ccw devices -> >>>> no stcrw. >>>> >>>> How could we ever add ccw devices to a TCG guest then? >>> >>> Hotplug never worked before your patches. Coldplug does not create >>> machine checks and thus does not trigger the guest to do STCRW. (Only >>> STSCH, in keeping with "all channel I/O acronyms look the same".) >>> >> >> But booting Fedora 26/27 without any involved hotplugs (therefore >> machine checks) triggers a STCRW. >> >> That's how I originally found it. (I started playing with machine checks >> after I had fedora 26/27 running) >> > > Confused. Do you have a backtrace? (Guest/host) > Unfortunately not. Strange, I just tried to reproduce but can't trigger it. Maybe aside effect from the other BUGs I have been fixing :) So I'll just rephrase this to "CRW machine check handling requires STCRW."
On Mon, 4 Dec 2017 19:37:26 +0100 David Hildenbrand <david@redhat.com> wrote: > On 04.12.2017 18:58, Cornelia Huck wrote: > > On Mon, 4 Dec 2017 18:56:00 +0100 > > David Hildenbrand <david@redhat.com> wrote: > > > >> On 04.12.2017 18:53, Cornelia Huck wrote: > >>> On Mon, 4 Dec 2017 18:34:36 +0100 > >>> David Hildenbrand <david@redhat.com> wrote: > >>> > >>>> On 04.12.2017 18:22, Cornelia Huck wrote: > >>>>> On Mon, 4 Dec 2017 13:55:05 +0100 > >>>>> David Hildenbrand <david@redhat.com> wrote: > >>>>> > >>>>>> We somehow missed that, new kernels require it. > >>>>> > >>>>> Why _new_ kernels? I think we have unconditionally issued stcrw since > >>>>> back in 2.2? > >>>> > >>>> Okay, the problem is then rather related to my setup. No ccw devices -> > >>>> no stcrw. > >>>> > >>>> How could we ever add ccw devices to a TCG guest then? > >>> > >>> Hotplug never worked before your patches. Coldplug does not create > >>> machine checks and thus does not trigger the guest to do STCRW. (Only > >>> STSCH, in keeping with "all channel I/O acronyms look the same".) > >>> > >> > >> But booting Fedora 26/27 without any involved hotplugs (therefore > >> machine checks) triggers a STCRW. > >> > >> That's how I originally found it. (I started playing with machine checks > >> after I had fedora 26/27 running) > >> > > > > Confused. Do you have a backtrace? (Guest/host) > > > > Unfortunately not. Strange, I just tried to reproduce but can't trigger > it. Maybe aside effect from the other BUGs I have been fixing :) > > So I'll just rephrase this to "CRW machine check handling requires STCRW." > Yeah, let's do that.
On 04.12.2017 13:55, David Hildenbrand wrote: > We somehow missed that, new kernels require it. > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > target/s390x/helper.h | 1 + > target/s390x/insn-data.def | 1 + > target/s390x/misc_helper.c | 9 +++++++++ > target/s390x/translate.c | 8 ++++++++ > 4 files changed, 19 insertions(+) IANATE again, but the patch looks right to me, so: Reviewed-by: Thomas Huth <thuth@redhat.com>
diff --git a/target/s390x/helper.h b/target/s390x/helper.h index 0281c286b8..2ce57edc14 100644 --- a/target/s390x/helper.h +++ b/target/s390x/helper.h @@ -166,6 +166,7 @@ DEF_HELPER_3(msch, void, env, i64, i64) DEF_HELPER_2(rchp, void, env, i64) DEF_HELPER_2(rsch, void, env, i64) DEF_HELPER_3(ssch, void, env, i64, i64) +DEF_HELPER_2(stcrw, void, env, i64) DEF_HELPER_3(stsch, void, env, i64, i64) DEF_HELPER_3(tsch, void, env, i64, i64) DEF_HELPER_2(chsc, void, env, i64) diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def index 8c2541f545..43ab1963c8 100644 --- a/target/s390x/insn-data.def +++ b/target/s390x/insn-data.def @@ -1055,6 +1055,7 @@ C(0xb23b, RCHP, S, Z, 0, 0, 0, 0, rchp, 0) C(0xb238, RSCH, S, Z, 0, 0, 0, 0, rsch, 0) C(0xb233, SSCH, S, Z, 0, insn, 0, 0, ssch, 0) + C(0xb239, STCRW, S, Z, 0, insn, 0, 0, stcrw, 0) C(0xb234, STSCH, S, Z, 0, insn, 0, 0, stsch, 0) C(0xb235, TSCH, S, Z, 0, insn, 0, 0, tsch, 0) /* ??? Not listed in PoO ninth edition, but there's a linux driver that diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c index 2c6ab329fb..55e78c56d2 100644 --- a/target/s390x/misc_helper.c +++ b/target/s390x/misc_helper.c @@ -385,6 +385,15 @@ void HELPER(ssch)(CPUS390XState *env, uint64_t r1, uint64_t inst) qemu_mutex_unlock_iothread(); } +void HELPER(stcrw)(CPUS390XState *env, uint64_t inst) +{ + S390CPU *cpu = s390_env_get_cpu(env); + + qemu_mutex_lock_iothread(); + ioinst_handle_stcrw(cpu, inst >> 16, GETPC()); + qemu_mutex_unlock_iothread(); +} + void HELPER(stsch)(CPUS390XState *env, uint64_t r1, uint64_t inst) { S390CPU *cpu = s390_env_get_cpu(env); diff --git a/target/s390x/translate.c b/target/s390x/translate.c index 8da8610839..5e051fdd03 100644 --- a/target/s390x/translate.c +++ b/target/s390x/translate.c @@ -4071,6 +4071,14 @@ static ExitStatus op_stsch(DisasContext *s, DisasOps *o) return NO_EXIT; } +static ExitStatus op_stcrw(DisasContext *s, DisasOps *o) +{ + check_privileged(s); + gen_helper_stcrw(cpu_env, o->in2); + set_cc_static(s); + return NO_EXIT; +} + static ExitStatus op_tsch(DisasContext *s, DisasOps *o) { check_privileged(s);
We somehow missed that, new kernels require it. Signed-off-by: David Hildenbrand <david@redhat.com> --- target/s390x/helper.h | 1 + target/s390x/insn-data.def | 1 + target/s390x/misc_helper.c | 9 +++++++++ target/s390x/translate.c | 8 ++++++++ 4 files changed, 19 insertions(+)