mbox series

[v4,0/8] s390: Extended-Length SCCB & DIAGNOSE 0x318

Message ID 20200624202312.28349-1-walling@linux.ibm.com (mailing list archive)
Headers show
Series s390: Extended-Length SCCB & DIAGNOSE 0x318 | expand

Message

Collin Walling June 24, 2020, 8:23 p.m. UTC
Changelog:

    v4
    
    • added r-b's and ack's (thanks, everyone!)

    • renamed boundary and length function

    • updated header sync to reflect a change discussed in the respective
        KVM patches

    • s/data_len/offset_cpu

    • added /* fallthrough */ comment in boundary check

    v3

    • Device IOCTLs removed
        - diag 318 info is now communicated via sync_regs

    • Reset code removed
        - this is now handled in KVM
        - diag318_info is stored within the CPU reset portion of the
            S390CPUState

    • Various cleanups for ELS preliminary patches

    v2

    • QEMU now handles the instruction call
        - as such, the "enable diag 318" IOCTL has been removed

    • patch #1 now changes the read scp/cpu info functions to
      retrieve the machine state once
        - as such, I have not added any ack's or r-bs since this
          patch differs from the previous version

    • patch #3 introduces a new "get_read_scp_info_data_len"
      function in order clean-up the variable data length assignment
      in patch #7
        - a comment above this function should help clarify what's
          going on to make things a bit easier to read

    • other misc clean ups and fixes
        - s/diag318/diag_318 in order to keep the naming scheme
          consistent with Linux and other diag-related code
        - s/byte_134/fac134 to align naming scheme with Linux

-----------------------------------------------------------------------

This patch series introduces two features for an s390 KVM quest:
    - Extended-Length SCCB (els) for the Read SCP/CPU Info SCLP 
        commands
    - DIAGNOSE 0x318 (diag_318) enabling / migration handling

The diag 318 feature depends on els and KVM support.

The els feature is handled entirely with QEMU, and does not require 
KVM support.

Both features are made available starting with the zEC12-full model.

These patches are introduced together for two main reasons:
    - els allows diag 318 to exist while retaining the original 248 
        VCPU max
    - diag 318 is presented to show how els is useful

Full els support is dependant on the Linux kernel, which must react
to the SCLP response code and set an appropriate-length SCCB. 

A user should take care when tuning the CPU model for a VM.
If a user defines a VM with els support and specifies 248 CPUs, but
the guest Linux kernel cannot react to the SCLP response code, then
the guest will crash immediately upon kernel startup.


Collin L. Walling (8):
  s390/sclp: get machine once during read scp/cpu info
  s390/sclp: check sccb len before filling in data
  s390/sclp: rework sclp boundary and length checks
  s390/sclp: read sccb from mem based on sccb length
  s390/sclp: use cpu offset to locate cpu entries
  s390/sclp: add extended-length sccb support for kvm guest
  s390/kvm: header sync for diag318
  s390: guest support for diagnose 0x318

 hw/s390x/sclp.c                     | 115 ++++++++++++++++++++++------
 include/hw/s390x/sclp.h             |   4 +
 linux-headers/asm-s390/kvm.h        |   7 +-
 linux-headers/linux/kvm.h           |   1 +
 target/s390x/cpu.h                  |   2 +
 target/s390x/cpu_features.h         |   1 +
 target/s390x/cpu_features_def.inc.h |   4 +
 target/s390x/cpu_models.c           |   1 +
 target/s390x/gen-features.c         |   2 +
 target/s390x/kvm.c                  |  39 ++++++++++
 target/s390x/machine.c              |  17 ++++
 11 files changed, 166 insertions(+), 27 deletions(-)

Comments

Collin Walling July 15, 2020, 3:36 p.m. UTC | #1
Polite ping. Patches have been sitting on the list for a few weeks now,
and it doesn't look like any further changes are requested (hopefully I
didn't miss something).

Thanks for everyone's time and patience. Stay safe out there.

 - Collin
Cornelia Huck July 15, 2020, 4:04 p.m. UTC | #2
On Wed, 15 Jul 2020 11:36:35 -0400
Collin Walling <walling@linux.ibm.com> wrote:

> Polite ping. Patches have been sitting on the list for a few weeks now,
> and it doesn't look like any further changes are requested (hopefully I
> didn't miss something).

The only thing I had was (I think) the logging of the length you just
replied to. We can still tweak things like that later, of course.

As these patches depend on a headers sync, I could not yet queue them.
I can keep a preliminary version on a branch. I assume that the header
changes will go in during the next kernel merge window? (If I missed
something, apologies for that.)

> 
> Thanks for everyone's time and patience. Stay safe out there.
> 
>  - Collin
>
Collin Walling July 15, 2020, 4:26 p.m. UTC | #3
On 7/15/20 12:04 PM, Cornelia Huck wrote:
> On Wed, 15 Jul 2020 11:36:35 -0400
> Collin Walling <walling@linux.ibm.com> wrote:
> 
>> Polite ping. Patches have been sitting on the list for a few weeks now,
>> and it doesn't look like any further changes are requested (hopefully I
>> didn't miss something).
> 
> The only thing I had was (I think) the logging of the length you just
> replied to. We can still tweak things like that later, of course.
> 
> As these patches depend on a headers sync, I could not yet queue them.
> I can keep a preliminary version on a branch. I assume that the header
> changes will go in during the next kernel merge window? (If I missed
> something, apologies for that.)
> 

Gotcha. Thanks for the update :)

There was an email on the KVM list a couple of days that made one change
to the Linux header. Just changed the integer used for the DIAG cap,
which should be reflected in QEMU as well.

https://www.spinics.net/lists/kvm/msg220548.html

Should I respin this patch series to include the new ack's and account
for the header sync?

>>
>> Thanks for everyone's time and patience. Stay safe out there.
>>
>>  - Collin
>>
> 
>
Cornelia Huck July 16, 2020, 12:02 p.m. UTC | #4
On Wed, 15 Jul 2020 12:26:20 -0400
Collin Walling <walling@linux.ibm.com> wrote:

> On 7/15/20 12:04 PM, Cornelia Huck wrote:
> > On Wed, 15 Jul 2020 11:36:35 -0400
> > Collin Walling <walling@linux.ibm.com> wrote:
> >   
> >> Polite ping. Patches have been sitting on the list for a few weeks now,
> >> and it doesn't look like any further changes are requested (hopefully I
> >> didn't miss something).  
> > 
> > The only thing I had was (I think) the logging of the length you just
> > replied to. We can still tweak things like that later, of course.
> > 
> > As these patches depend on a headers sync, I could not yet queue them.
> > I can keep a preliminary version on a branch. I assume that the header
> > changes will go in during the next kernel merge window? (If I missed
> > something, apologies for that.)
> >   
> 
> Gotcha. Thanks for the update :)
> 
> There was an email on the KVM list a couple of days that made one change
> to the Linux header. Just changed the integer used for the DIAG cap,
> which should be reflected in QEMU as well.
> 
> https://www.spinics.net/lists/kvm/msg220548.html
> 
> Should I respin this patch series to include the new ack's and account
> for the header sync?

No need for that, my tooling picks up acks and the headers update needs
to be replaced with a sync against a proper Linux version anyway.

I've queued the patches on a local branch, and the only patch that did
not apply cleanly was the headers patch, which will get replaced later
anyway :) Just ping me when the kernel patches hit upstream, then I'll
do a header sync against the next -rc and queue the patches on
s390-next.
Christian Borntraeger Sept. 9, 2020, 7:54 a.m. UTC | #5
On 16.07.20 14:02, Cornelia Huck wrote:
> On Wed, 15 Jul 2020 12:26:20 -0400
> Collin Walling <walling@linux.ibm.com> wrote:
> 
>> On 7/15/20 12:04 PM, Cornelia Huck wrote:
>>> On Wed, 15 Jul 2020 11:36:35 -0400
>>> Collin Walling <walling@linux.ibm.com> wrote:
>>>   
>>>> Polite ping. Patches have been sitting on the list for a few weeks now,
>>>> and it doesn't look like any further changes are requested (hopefully I
>>>> didn't miss something).  
>>>
>>> The only thing I had was (I think) the logging of the length you just
>>> replied to. We can still tweak things like that later, of course.
>>>
>>> As these patches depend on a headers sync, I could not yet queue them.
>>> I can keep a preliminary version on a branch. I assume that the header
>>> changes will go in during the next kernel merge window? (If I missed
>>> something, apologies for that.)
>>>   
>>
>> Gotcha. Thanks for the update :)
>>
>> There was an email on the KVM list a couple of days that made one change
>> to the Linux header. Just changed the integer used for the DIAG cap,
>> which should be reflected in QEMU as well.
>>
>> https://www.spinics.net/lists/kvm/msg220548.html
>>
>> Should I respin this patch series to include the new ack's and account
>> for the header sync?
> 
> No need for that, my tooling picks up acks and the headers update needs
> to be replaced with a sync against a proper Linux version anyway.
> 
> I've queued the patches on a local branch, and the only patch that did
> not apply cleanly was the headers patch, which will get replaced later
> anyway :) Just ping me when the kernel patches hit upstream, then I'll
> do a header sync against the next -rc and queue the patches on
> s390-next.

What is the status of this patchset? The kernel part has been merged in 5.9-rc1.
Cornelia Huck Sept. 9, 2020, 8:46 a.m. UTC | #6
On Wed, 9 Sep 2020 09:54:40 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> On 16.07.20 14:02, Cornelia Huck wrote:
> > On Wed, 15 Jul 2020 12:26:20 -0400
> > Collin Walling <walling@linux.ibm.com> wrote:
> >   
> >> On 7/15/20 12:04 PM, Cornelia Huck wrote:  
> >>> On Wed, 15 Jul 2020 11:36:35 -0400
> >>> Collin Walling <walling@linux.ibm.com> wrote:
> >>>     
> >>>> Polite ping. Patches have been sitting on the list for a few weeks now,
> >>>> and it doesn't look like any further changes are requested (hopefully I
> >>>> didn't miss something).    
> >>>
> >>> The only thing I had was (I think) the logging of the length you just
> >>> replied to. We can still tweak things like that later, of course.
> >>>
> >>> As these patches depend on a headers sync, I could not yet queue them.
> >>> I can keep a preliminary version on a branch. I assume that the header
> >>> changes will go in during the next kernel merge window? (If I missed
> >>> something, apologies for that.)
> >>>     
> >>
> >> Gotcha. Thanks for the update :)
> >>
> >> There was an email on the KVM list a couple of days that made one change
> >> to the Linux header. Just changed the integer used for the DIAG cap,
> >> which should be reflected in QEMU as well.
> >>
> >> https://www.spinics.net/lists/kvm/msg220548.html
> >>
> >> Should I respin this patch series to include the new ack's and account
> >> for the header sync?  
> > 
> > No need for that, my tooling picks up acks and the headers update needs
> > to be replaced with a sync against a proper Linux version anyway.
> > 
> > I've queued the patches on a local branch, and the only patch that did
> > not apply cleanly was the headers patch, which will get replaced later
> > anyway :) Just ping me when the kernel patches hit upstream, then I'll
> > do a header sync against the next -rc and queue the patches on
> > s390-next.  
> 
> What is the status of this patchset? The kernel part has been merged in 5.9-rc1.
> 

Thanks for letting me know, I'll go and process this now. (Had some
busy weeks + PTO.)
Cornelia Huck Sept. 9, 2020, 9:43 a.m. UTC | #7
On Wed, 9 Sep 2020 10:46:23 +0200
Cornelia Huck <cohuck@redhat.com> wrote:

> On Wed, 9 Sep 2020 09:54:40 +0200
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
> > On 16.07.20 14:02, Cornelia Huck wrote:  
> > > On Wed, 15 Jul 2020 12:26:20 -0400
> > > Collin Walling <walling@linux.ibm.com> wrote:
> > >     
> > >> On 7/15/20 12:04 PM, Cornelia Huck wrote:    
> > >>> On Wed, 15 Jul 2020 11:36:35 -0400
> > >>> Collin Walling <walling@linux.ibm.com> wrote:
> > >>>       
> > >>>> Polite ping. Patches have been sitting on the list for a few weeks now,
> > >>>> and it doesn't look like any further changes are requested (hopefully I
> > >>>> didn't miss something).      
> > >>>
> > >>> The only thing I had was (I think) the logging of the length you just
> > >>> replied to. We can still tweak things like that later, of course.
> > >>>
> > >>> As these patches depend on a headers sync, I could not yet queue them.
> > >>> I can keep a preliminary version on a branch. I assume that the header
> > >>> changes will go in during the next kernel merge window? (If I missed
> > >>> something, apologies for that.)
> > >>>       
> > >>
> > >> Gotcha. Thanks for the update :)
> > >>
> > >> There was an email on the KVM list a couple of days that made one change
> > >> to the Linux header. Just changed the integer used for the DIAG cap,
> > >> which should be reflected in QEMU as well.
> > >>
> > >> https://www.spinics.net/lists/kvm/msg220548.html
> > >>
> > >> Should I respin this patch series to include the new ack's and account
> > >> for the header sync?    
> > > 
> > > No need for that, my tooling picks up acks and the headers update needs
> > > to be replaced with a sync against a proper Linux version anyway.
> > > 
> > > I've queued the patches on a local branch, and the only patch that did
> > > not apply cleanly was the headers patch, which will get replaced later
> > > anyway :) Just ping me when the kernel patches hit upstream, then I'll
> > > do a header sync against the next -rc and queue the patches on
> > > s390-next.    
> > 
> > What is the status of this patchset? The kernel part has been merged in 5.9-rc1.
> >   
> 
> Thanks for letting me know, I'll go and process this now. (Had some
> busy weeks + PTO.)

There were some minor conflicts, please double check that everything
looks sane.

Thanks, applied.
Collin Walling Sept. 9, 2020, 6:13 p.m. UTC | #8
On 9/9/20 5:43 AM, Cornelia Huck wrote:
> On Wed, 9 Sep 2020 10:46:23 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
> 
>> On Wed, 9 Sep 2020 09:54:40 +0200
>> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>>
>>> On 16.07.20 14:02, Cornelia Huck wrote:  
>>>> On Wed, 15 Jul 2020 12:26:20 -0400
>>>> Collin Walling <walling@linux.ibm.com> wrote:
>>>>     
>>>>> On 7/15/20 12:04 PM, Cornelia Huck wrote:    
>>>>>> On Wed, 15 Jul 2020 11:36:35 -0400
>>>>>> Collin Walling <walling@linux.ibm.com> wrote:
>>>>>>       
>>>>>>> Polite ping. Patches have been sitting on the list for a few weeks now,
>>>>>>> and it doesn't look like any further changes are requested (hopefully I
>>>>>>> didn't miss something).      
>>>>>>
>>>>>> The only thing I had was (I think) the logging of the length you just
>>>>>> replied to. We can still tweak things like that later, of course.
>>>>>>
>>>>>> As these patches depend on a headers sync, I could not yet queue them.
>>>>>> I can keep a preliminary version on a branch. I assume that the header
>>>>>> changes will go in during the next kernel merge window? (If I missed
>>>>>> something, apologies for that.)
>>>>>>       
>>>>>
>>>>> Gotcha. Thanks for the update :)
>>>>>
>>>>> There was an email on the KVM list a couple of days that made one change
>>>>> to the Linux header. Just changed the integer used for the DIAG cap,
>>>>> which should be reflected in QEMU as well.
>>>>>
>>>>> https://www.spinics.net/lists/kvm/msg220548.html
>>>>>
>>>>> Should I respin this patch series to include the new ack's and account
>>>>> for the header sync?    
>>>>
>>>> No need for that, my tooling picks up acks and the headers update needs
>>>> to be replaced with a sync against a proper Linux version anyway.
>>>>
>>>> I've queued the patches on a local branch, and the only patch that did
>>>> not apply cleanly was the headers patch, which will get replaced later
>>>> anyway :) Just ping me when the kernel patches hit upstream, then I'll
>>>> do a header sync against the next -rc and queue the patches on
>>>> s390-next.    
>>>
>>> What is the status of this patchset? The kernel part has been merged in 5.9-rc1.
>>>   
>>
>> Thanks for letting me know, I'll go and process this now. (Had some
>> busy weeks + PTO.)
> 
> There were some minor conflicts, please double check that everything
> looks sane.
> 
> Thanks, applied.
> 
> 

Has this been merged yet? There is one patch that I neglected to include
in this series (I didn't realize it until now) that properly sets the
SCCB size in QEMU based on the length provided by the kernel (right now,
these patches allocate a static 4K size for the SCCB struct, which
causes a segfault).

I can post my set with the fix as v5, or I can wait and post the fix as
a bandaid if the patches are already in.
Cornelia Huck Sept. 10, 2020, 6:38 a.m. UTC | #9
On Wed, 9 Sep 2020 14:13:55 -0400
Collin Walling <walling@linux.ibm.com> wrote:

> Has this been merged yet? There is one patch that I neglected to include
> in this series (I didn't realize it until now) that properly sets the
> SCCB size in QEMU based on the length provided by the kernel (right now,
> these patches allocate a static 4K size for the SCCB struct, which
> causes a segfault).
> 
> I can post my set with the fix as v5, or I can wait and post the fix as
> a bandaid if the patches are already in.
> 

It's queued on s390-next right now, I can still update it.

Is that really an extra patch, or something that can be merged into the
series? If the latter, I'd prefer a v5, if the former, I'd just queue
that patch on top.
Collin Walling Sept. 10, 2020, 6:49 a.m. UTC | #10
On 9/10/20 2:38 AM, Cornelia Huck wrote:
> On Wed, 9 Sep 2020 14:13:55 -0400
> Collin Walling <walling@linux.ibm.com> wrote:
> 
>> Has this been merged yet? There is one patch that I neglected to include
>> in this series (I didn't realize it until now) that properly sets the
>> SCCB size in QEMU based on the length provided by the kernel (right now,
>> these patches allocate a static 4K size for the SCCB struct, which
>> causes a segfault).
>>
>> I can post my set with the fix as v5, or I can wait and post the fix as
>> a bandaid if the patches are already in.
>>
> 
> It's queued on s390-next right now, I can still update it.
> 
> Is that really an extra patch, or something that can be merged into the
> series? If the latter, I'd prefer a v5, if the former, I'd just queue
> that patch on top.
> 
> 

I'll post a v5. Thanks!