[v1,for-2.12,5/5] s390x/tcg: wire up STORE CHANNEL REPORT WORD

Message ID 20171204125505.29203-6-david@redhat.com
State New
Headers show
Series
  • s390x/tcg: CCW hotplug support
Related show

Commit Message

David Hildenbrand Dec. 4, 2017, 12:55 p.m.
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(+)

Comments

Cornelia Huck Dec. 4, 2017, 5:22 p.m. | #1
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(+)
David Hildenbrand Dec. 4, 2017, 5:34 p.m. | #2
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(+)
Cornelia Huck Dec. 4, 2017, 5:53 p.m. | #3
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".)
David Hildenbrand Dec. 4, 2017, 5:56 p.m. | #4
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)
Cornelia Huck Dec. 4, 2017, 5:58 p.m. | #5
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)
David Hildenbrand Dec. 4, 2017, 6:37 p.m. | #6
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."
Cornelia Huck Dec. 5, 2017, 10:04 a.m. | #7
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.
Thomas Huth Dec. 6, 2017, 3:40 p.m. | #8
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>

Patch

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);