The SCCB must be checked for a sufficient length before it is filled
with any data. If the length is insufficient, then the SCLP command
is suppressed and the proper response code is set in the SCCB header.
Fixes: 832be0d8a3bb ("s390x: sclp: Report insufficient SCCB length")
Signed-off-
Changelog:
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
On 6/11/20 8:01 AM, Thomas Huth wrote:
> On 16/05/2020 00.20, Collin Walling wrote:
>> The SCCB must be checked for a sufficient length before it is filled
>> with any data. If the length is insufficient, then the SCLP command
>> is suppressed and the proper response code is s
On 6/11/20 8:56 AM, Thomas Huth wrote:
> On 16/05/2020 00.20, Collin Walling wrote:
>> Rework the SCLP boundary check to account for different SCLP commands
>> (eventually) allowing different boundary sizes.
>>
>> Move the length check code into a separate function, and
On 5/25/20 6:50 AM, Janosch Frank wrote:
> On 5/18/20 4:31 PM, Collin Walling wrote:
>> On 5/18/20 4:55 AM, Janosch Frank wrote:
>>> On 5/16/20 12:20 AM, Collin Walling wrote:
>>>> As more features and facilities are added to the Read SCP Info (RSCPI)
>>>>
On 5/20/20 7:30 AM, Cornelia Huck wrote:
> On Fri, 15 May 2020 18:20:32 -0400
> Collin Walling wrote:
>
>> DIAGNOSE 0x318 (diag 318) allows the storage of diagnostic data that
>> is collected by the firmware in the case of hardware/firmware service
>> events.
>&
On 5/16/20 2:41 AM, no-re...@patchew.org wrote:
> Patchew URL:
> https://patchew.org/QEMU/20200515222032.18838-1-wall...@linux.ibm.com/
>
>
>
> Hi,
>
> This series seems to have some coding style problems. See output below for
> more information:
>
> Message-id:
On 5/18/20 11:43 AM, David Hildenbrand wrote:
> On 18.05.20 16:32, Collin Walling wrote:
>> On 5/18/20 7:46 AM, David Hildenbrand wrote:
>>> On 16.05.20 00:20, Collin Walling wrote:
>>>> The SCCB must be checked for a sufficient length before it is filled
>
On 5/18/20 4:38 AM, David Hildenbrand wrote:
> On 16.05.20 00:20, Collin Walling wrote:
>> Functions within read scp/cpu info will need access to the machine
>> state. Let's make a call to retrieve the machine state once and
>> pass the appropriate data to the respective fu
On 5/18/20 4:50 AM, Janosch Frank wrote:
> On 5/16/20 12:20 AM, Collin Walling wrote:
>> Rework the SCLP boundary check to account for different SCLP commands
>> (eventually) allowing different boundary sizes.
>>
>> Move the length check code into a separate funct
On 5/18/20 7:46 AM, David Hildenbrand wrote:
> On 16.05.20 00:20, Collin Walling wrote:
>> The SCCB must be checked for a sufficient length before it is filled
>> with any data. If the length is insufficient, then the SCLP command
>> is suppressed and the proper response co
On 5/18/20 4:55 AM, Janosch Frank wrote:
> On 5/16/20 12:20 AM, Collin Walling wrote:
>> As more features and facilities are added to the Read SCP Info (RSCPI)
>> response, more space is required to store them. The space used to store
>> these new features intrudes on the
Functions within read scp/cpu info will need access to the machine
state. Let's make a call to retrieve the machine state once and
pass the appropriate data to the respective functions.
Signed-off-by: Collin Walling
---
hw/s390x/sclp.c | 8
1 file changed, 4 insertions(+), 4 deletions
any explicitly-set boundaries.
Signed-off-by: Collin Walling
---
hw/s390x/sclp.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 987699e3c4..5d6e98ae64 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -256,9 +256,8
the CPU entries should begin).
Signed-off-by: Collin Walling
---
hw/s390x/sclp.c | 57 ++---
1 file changed, 49 insertions(+), 8 deletions(-)
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 2bd618515e..987699e3c4 100644
--- a/hw/s390x/sclp.c
ilable CPU entries after migration (such as during re-ipl).
Signed-off-by: Collin Walling
---
hw/s390x/sclp.c | 21 -
include/hw/s390x/sclp.h | 1 +
target/s390x/cpu_features_def.inc.h | 1 +
target/s390x/gen-features.c | 1 +
target/s
of when Read SCP Info inevitably introduces new
bytes that push the start of the CPUEntry field further away.
Read CPU Info is unlikely to ever change, so let's not bother
accounting for the offset there.
Signed-off-by: Collin Walling
---
hw/s390x/sclp.c | 3 ++-
1 file changed, 2 insertions(+), 1
Signed-off-by: Collin Walling
---
linux-headers/asm-s390/kvm.h | 4
1 file changed, 4 insertions(+)
diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
index 0138ccb0d8..3d85817ec2 100644
--- a/linux-headers/asm-s390/kvm.h
+++ b/linux-headers/asm-s390/kvm.h
@@ -74,6
Changelog:
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
The SCCB must be checked for a sufficient length before it is filled
with any data. If the length is insufficient, then the SCLP command
is suppressed and the proper response code is set in the SCCB header.
Fixes: 832be0d8a3bb ("s390x: sclp: Report insufficient SCCB length")
Signed-off-
is not supported in protected virtualization mode.
Signed-off-by: Collin Walling
---
hw/s390x/s390-virtio-ccw.c | 45 +
hw/s390x/sclp.c | 5
include/hw/s390x/s390-virtio-ccw.h | 1 +
include/hw/s390x/sclp.h | 3
16:50, Janosch Frank wrote:
>>>>> On 5/11/20 4:44 PM, David Hildenbrand wrote:
>>>>>> On 11.05.20 16:36, Janosch Frank wrote:
>>>>>>> On 5/9/20 1:08 AM, Collin Walling wrote:
>>>>>>>> The SCCB must
On 5/13/20 3:00 AM, Cornelia Huck wrote:
> On Tue, 12 May 2020 10:55:56 -0400
> Collin Walling wrote:
>
>> On 5/12/20 3:21 AM, David Hildenbrand wrote:
>>> On 09.05.20 01:08, Collin Walling wrote:
>
>>>> +static bool check_s
On 5/13/20 3:05 AM, Cornelia Huck wrote:
> On Fri, 8 May 2020 19:08:22 -0400
> Collin Walling wrote:
>
>> Signed-off-by: Collin Walling
>> ---
>> linux-headers/asm-s390/kvm.h | 5 +
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/linux-he
On 5/12/20 12:24 PM, Cornelia Huck wrote:
On Tue, 12 May 2020 12:16:45 -0400
Collin Walling wrote:
On 5/12/20 12:01 PM, Cornelia Huck wrote:
On Mon, 11 May 2020 17:02:06 +0200
David Hildenbrand wrote:
On 11.05.20 16:50, Janosch Frank wrote:
On 5/11/20 4:44 PM, David Hildenbrand wrote
On 5/12/20 12:08 PM, Cornelia Huck wrote:
On Fri, 8 May 2020 19:08:15 -0400
Collin Walling wrote:
Collin L. Walling (8):
s390/sclp: remove SCLPDevice param from prepare_cpu_entries
This looks like a simple cleanup...
s390/sclp: check sccb len before filling in data
On 5/12/20 12:01 PM, Cornelia Huck wrote:
On Mon, 11 May 2020 17:02:06 +0200
David Hildenbrand wrote:
On 11.05.20 16:50, Janosch Frank wrote:
On 5/11/20 4:44 PM, David Hildenbrand wrote:
On 11.05.20 16:36, Janosch Frank wrote:
On 5/9/20 1:08 AM, Collin Walling wrote:
The SCCB must
On 5/12/20 3:21 AM, David Hildenbrand wrote:
On 09.05.20 01:08, Collin Walling wrote:
Let's factor out the SCLP boundary and length checks
into separate functions.
Signed-off-by: Collin Walling
---
hw/s390x/sclp.c | 41 +++--
1 file changed, 35
On 5/12/20 3:26 AM, David Hildenbrand wrote:
On 09.05.20 01:08, Collin Walling wrote:
The header of the SCCB contains the actual length of the
SCCB. Instead of using a static 4K size, let's allow
for a variable size determined by the value set in the
header. The proper checks are already
On 5/11/20 10:44 AM, David Hildenbrand wrote:
On 11.05.20 16:36, Janosch Frank wrote:
On 5/9/20 1:08 AM, Collin Walling wrote:
The SCCB must be checked for a sufficient length before it is filled
with any data. If the length is insufficient, then the SCLP command
is suppressed and the proper
Signed-off-by: Collin Walling
---
linux-headers/asm-s390/kvm.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
index 0138ccb0d8..b661feafdc 100644
--- a/linux-headers/asm-s390/kvm.h
+++ b/linux-headers/asm-s390/kvm.h
@@ -74,6
Let's factor out the SCLP boundary and length checks
into separate functions.
Signed-off-by: Collin Walling
---
hw/s390x/sclp.c | 41 +++--
1 file changed, 35 insertions(+), 6 deletions(-)
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index d08a291e40
The SCCB must be checked for a sufficient length before it is filled
with any data. If the length is insufficient, then the SCLP command
is suppressed and the proper response code is set in the SCCB header.
Signed-off-by: Collin Walling
---
hw/s390x/sclp.c | 22
CPUs.
This feature depends on the Extended-Length SCCB (els) feature.
Diag318 is set to 0 during modified clear and load normal resets.
This feature is not supported in protected virtualization mode.
Signed-off-by: Collin Walling
---
hw/s390x/s390-virtio-ccw.c | 45
away.
Read CPU Info is unlikely to ever change, so let's not
bother accounting for the offset there.
Signed-off-by: Collin Walling
---
hw/s390x/sclp.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 748d04a0e2..c47bd3b5ab 100644
migration.
Signed-off-by: Collin Walling
---
hw/s390x/sclp.c | 23 +--
include/hw/s390x/sclp.h | 1 +
target/s390x/cpu_features_def.inc.h | 1 +
target/s390x/gen-features.c | 1 +
target/s390x/kvm.c | 4
5 files c
It was never used in this function, so let's remove it.
Signed-off-by: Collin Walling
---
hw/s390x/sclp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index ede056b3ef..156ffe3223 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
does not cross any explicitly-set
boundaries.
Signed-off-by: Collin Walling
---
hw/s390x/sclp.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 470d5da7a2..748d04a0e2 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
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 (diag318) enabling / migration handling
The diag318 feature depends on els and KVM support.
The els feature is handled entirely
and understanding,
- Collin
On 1/24/20 5:14 PM, Collin Walling wrote:
Changes from v5 -> v6
Migration and DeviceObject Code:
- load/save/needed functions now check if kvm_enabled before calling
kvm_get/set and has_feat (respectively)
QEMU->KVM Code:
-
On 1/27/20 12:35 PM, Cornelia Huck wrote:
> On Mon, 27 Jan 2020 11:39:02 -0500
> Collin Walling wrote:
>
>> On 1/27/20 6:47 AM, Cornelia Huck wrote:
>>> On Fri, 24 Jan 2020 17:14:04 -0500
>>> Collin Walling wrote:
>>>
>>>> DIAGNOSE 0x318
On 1/28/20 6:24 AM, Cornelia Huck wrote:
> On Mon, 27 Jan 2020 18:05:36 -0500
> Collin Walling wrote:
>
>> On 1/27/20 12:35 PM, Cornelia Huck wrote:
>>> On Mon, 27 Jan 2020 11:39:02 -0500
>>> Collin Walling wrote:
>>>
>>>> On 1/27/20 6:4
On 1/27/20 12:35 PM, Cornelia Huck wrote:
> On Mon, 27 Jan 2020 11:39:02 -0500
> Collin Walling wrote:
>
>> On 1/27/20 6:47 AM, Cornelia Huck wrote:
>>> On Fri, 24 Jan 2020 17:14:04 -0500
>>> Collin Walling wrote:
>>>
[...]
>>>>
&
On 1/27/20 1:21 PM, Collin Walling wrote:
> On 1/27/20 12:55 PM, David Hildenbrand wrote:
>> On 27.01.20 18:29, Cornelia Huck wrote:
>>> On Mon, 27 Jan 2020 18:09:11 +0100
>>> David Hildenbrand wrote:
>>>
>>>>>
hat needs to be stored once and migrated
>>>> once?
>>>
>>> ... I actually thought we have something like this already. Personally,
>>> I think that would make sense. At least spapr seems to have something
>>> like this already (hw/ppc/spapr.c:spapr_machine_init().
>>>
>>> @Conny?
>>
>> What are you referring to? I only see the one with the FIXME in front
>> of it...
>
> That's the one I mean. The fixme states something about qdev ... but
> AFAIK that's only applicable if TYPE_DEVICE is involved. So maybe right
> now there is no other way than registering the vmstate directly.
>
Hmm okay. I'll take a look at how spapr does it. I think I've registered a
vmstate via register_savevm_live() in an earlier version, but had difficulties
figuring out where to store the data. I'll revisit this approach.
Thanks for the feedback!
--
Respectfully,
- Collin Walling
On 1/27/20 6:47 AM, Cornelia Huck wrote:
> On Fri, 24 Jan 2020 17:14:04 -0500
> Collin Walling wrote:
>
>> DIAGNOSE 0x318 (diag318) is a privileged s390x instruction that must
>> be intercepted by SIE and handled via KVM. Let's introduce some
>> functions to communica
On 1/27/20 6:36 AM, Thomas Huth wrote:
> On 24/01/2020 23.14, Collin Walling wrote:
>> DIAGNOSE 0x318 (diag318) is a privileged s390x instruction that must
>> be intercepted by SIE and handled via KVM. Let's introduce some
>> functions to communicate between QE
type_init(s390_diag318_register_types)
>> diff --git a/hw/s390x/diag318.h b/hw/s390x/diag318.h
>> new file mode 100644
>> index 000..06d9f67
>> --- /dev/null
>> +++ b/hw/s390x/diag318.h
>> @@ -0,0 +1,40 @@
>> +/*
>> + * DIAGNOSE
Signed-off-by: Collin Walling
Acked-by: David Hildenbrand
---
linux-headers/asm-s390/kvm.h | 4
1 file changed, 4 insertions(+)
diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
index 0138ccb..4633090 100644
--- a/linux-headers/asm-s390/kvm.h
+++ b/linux-headers/asm
Changes from v5 -> v6
Migration and DeviceObject Code:
- load/save/needed functions now check if kvm_enabled before calling
kvm_get/set and has_feat (respectively)
QEMU->KVM Code:
- added kvm_s390_* stubs for get/set functions for TCG compilation
VCPU
still be
ran with 248 CPUs.
In order to simplify the migration and system reset requirements of
the diag318 data, let's introduce it as a device class and include
a VMStateDescription.
Diag318 is set to 0 during modified clear and load normal resets.
Signed-off-by: Collin Walling
---
hw/s390x
Acked-by: Collin Walling
On 9/27/19 9:21 AM, Matthew Rosato wrote:
As discussed previously with Collin, I will take over maintaining
s390 pci.
Signed-off-by: Matthew Rosato
---
MAINTAINERS | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index
On 7/17/19 9:52 AM, Paolo Bonzini wrote:
On 17/07/19 14:59, Collin Walling wrote:
On 7/16/19 11:04 AM, Thomas Huth wrote:
On 16/07/2019 15.06, Markus Armbruster wrote:
Paolo Bonzini writes:
On 15/07/19 18:12, Cornelia Huck wrote:
Is it INTx vs. MSI vs. MSI-X?
I think for s390x we need
On 7/16/19 11:04 AM, Thomas Huth wrote:
On 16/07/2019 15.06, Markus Armbruster wrote:
Paolo Bonzini writes:
On 15/07/19 18:12, Cornelia Huck wrote:
Is it INTx vs. MSI vs. MSI-X?
I think for s390x we need (INTx || MSI) vs MSI-X...
I think MSI vs MSI-X is just how it's configured, not the
On 7/17/19 5:27 AM, Christian Borntraeger wrote:
On 17.07.19 10:54, Cornelia Huck wrote:
On Tue, 16 Jul 2019 14:34:22 -0400
Collin Walling wrote:
On 7/16/19 11:20 AM, Cornelia Huck wrote:
On Wed, 10 Jul 2019 10:20:41 +0200
Cornelia Huck wrote:
On Tue, 9 Jul 2019 18:55:34 -0400
On 7/16/19 11:20 AM, Cornelia Huck wrote:
On Wed, 10 Jul 2019 10:20:41 +0200
Cornelia Huck wrote:
On Tue, 9 Jul 2019 18:55:34 -0400
Collin Walling wrote:
On 7/8/19 9:23 AM, Christian Borntraeger wrote:
On 08.07.19 14:54, Cornelia Huck wrote:
According to the comment, the bits
ommu->enabled) {
Acked-by: Collin Walling
Side note: is there somewhere that I could access this bug report? :)
On 6/26/19 8:14 AM, Cornelia Huck wrote:
On Wed, 26 Jun 2019 11:12:04 +0200
Christian Borntraeger wrote:
On 25.06.19 17:17, Collin Walling wrote:
index a606547..4c26754 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -39,7 +39,13 @@
#define MMU_USER_IDX 0
-#define
On 6/26/19 8:33 AM, Cornelia Huck wrote:
On Tue, 25 Jun 2019 11:17:09 -0400
Collin Walling wrote:
DIAGNOSE 0x318 (diag318) is a privileged s390x instruction that must
be intercepted by SIE and handled via KVM. Let's introduce some
functions to communicate between QEMU and KVM via ioctls
Signed-off-by: Collin Walling
---
linux-headers/asm-s390/kvm.h | 4
1 file changed, 4 insertions(+)
diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
index 03ab596..4a857bb 100644
--- a/linux-headers/asm-s390/kvm.h
+++ b/linux-headers/asm-s390/kvm.h
@@ -74,6 +74,7
the
maximum possible CPUs from 248 to 247. Hopefully this drawback does not affect
many
VMs.
Collin Walling (2):
s390/kvm: header sync for diag318
s390: diagnose 318 info reset and migration support
hw/s390x/Makefile.objs | 1 +
hw/s390x/diag318.c | 80
reset requirements of
the diag318 data, let's introduce it as a device class and include
a VMStateDescription.
Diag318 is set to 0 during modified clear and load normal resets.
Signed-off-by: Collin Walling
---
hw/s390x/Makefile.objs | 1 +
hw/s390x/diag318.c | 80
On 5/14/19 5:30 AM, Cornelia Huck wrote:
On Tue, 14 May 2019 11:27:32 +0200
David Hildenbrand wrote:
On 14.05.19 11:25, Christian Borntraeger wrote:
On 14.05.19 11:23, Christian Borntraeger wrote:
On 14.05.19 11:20, David Hildenbrand wrote:
On 14.05.19 11:10, Christian Borntraeger
On 5/14/19 5:04 AM, Christian Borntraeger wrote:
On 14.05.19 11:00, David Hildenbrand wrote:
On 14.05.19 10:56, Christian Borntraeger wrote:
On 14.05.19 10:50, David Hildenbrand wrote:
On 14.05.19 10:37, Christian Borntraeger wrote:
On 14.05.19 09:28, David Hildenbrand wrote:
But that
On 5/9/19 5:58 AM, Christian Borntraeger wrote:
On 02.05.19 00:31, Collin Walling wrote:
DIAGNOSE 0x318 (diag318) is a privileged s390x instruction that must
be intercepted by SIE and handled via KVM. Let's introduce some
functions to communicate between QEMU and KVM via ioctls
uring modified clear and load normal.
Signed-off-by: Collin Walling
---
hw/s390x/Makefile.objs | 1 +
hw/s390x/diag318.c | 100 +++
hw/s390x/diag318.h | 39 +
hw/s390x/s390-virtio-ccw.c | 23 +++
On 4/3/19 10:16 AM, Collin Walling wrote:
On 4/3/19 8:30 AM, Cornelia Huck wrote:
On Wed, 3 Apr 2019 13:46:07 +0200
David Hildenbrand wrote:
On 01.04.19 23:48, Collin Walling wrote:
DIAGNOSE 0x318 (diag318) is a privileged s390x instruction that must
be intercepted by SIE and handled via
On 4/3/19 8:30 AM, Cornelia Huck wrote:
On Wed, 3 Apr 2019 13:46:07 +0200
David Hildenbrand wrote:
On 01.04.19 23:48, Collin Walling wrote:
DIAGNOSE 0x318 (diag318) is a privileged s390x instruction that must
be intercepted by SIE and handled via KVM. Let's introduce some
functions
On 4/1/19 5:48 PM, Collin Walling wrote:
[...]
---
This version is posted in tandem with a new kernel patch that changes
how the execution of the diag 0x318 instruction is handled. A link to
this series will be attached as a reply to this series for convenience.
https://www.spinics.net
o ensure the diag318 info is migrated. The diag318
info migration is handled via a VMStateDescription. This feature is
only supported on QEMU machines 4.0 and later.
Signed-off-by: Collin Walling
---
This version is posted in tandem with a new kernel patch that changes
how the execution of the di
.
Reviewed-by: Collin Walling
---
tests/Makefile.include | 4 ++
tests/device-plug-test.c | 93
2 files changed, 97 insertions(+)
create mode 100644 tests/device-plug-test.c
diff --git a/tests/Makefile.include b/tests/Makefile.include
index
On 2/12/19 4:04 AM, David Hildenbrand wrote:
> On 12.02.19 02:16, Collin Walling wrote:
>> Latest systems and host kernels support mepoch, which is a
>> feature that was meant to be supported for z14 GA1 from the
>> get-go. Let's copy it to the z14 GA1 default CPU model.
&g
On 2/12/19 5:04 AM, Cornelia Huck wrote:
> On Mon, 11 Feb 2019 20:16:55 -0500
> Collin Walling wrote:
>
>> The extended PTFF features (qsie, qtoue, stoe, stoue) are dependent
>> on the multiple-epoch facility (mepoch). Let's print a warning if these
>> features
On 2/12/19 5:09 AM, Christian Borntraeger wrote:
>
>
> On 12.02.2019 02:16, Collin Walling wrote:
>> The extended PTFF features (qsie, qtoue, stoe, stoue) are dependent
>> on the multiple-epoch facility (mepoch). Let's print a warning if these
>> features are enabled
functions in the default model.
Signed-off-by: Collin Walling
---
@Christian, @David: I elected to not add your r-b's to this
patch since I've added some code to disable feature groups.
I'd like to know what you both think :)
---
hw/s390x/s390-virtio-ccw.c | 2 ++
target/s390x/cpu_models.c
indexed with its
generated S390FeatGroup enum.
Signed-off-by: Collin Walling
---
target/s390x/cpu_features.c | 2 +-
target/s390x/cpu_models.c | 4
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
index 60cfeba48f
Introduce the z14 GA2 cpu model for QEMU. There are no new features
introduced with this model, and will inherit the same feature set as
z14 GA1.
Signed-off-by: Collin Walling
Acked-by: Christian Borntraeger
Reviewed-by: David Hildenbrand
---
target/s390x/cpu_models.c | 1 +
target/s390x
Introduce the z14 GA2 cpu model for QEMU. There are no new features
introduced with this model, and will inherit the same feature set as
z14 GA1.
Signed-off-by: Collin Walling
---
target/s390x/cpu_models.c | 1 +
target/s390x/gen-features.c | 7 +++
2 files changed, 8 insertions(+)
diff
.
Signed-off-by: Collin Walling
---
hw/s390x/s390-virtio-ccw.c | 1 +
target/s390x/gen-features.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index fd9d0b0542..32c5027345 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390
Signed-off-by: David Hildenbrand
---
Reviewed-by: Collin Walling
On 2/4/19 4:54 PM, David Hildenbrand wrote:
On 04.02.19 21:19, Collin Walling wrote:
On 1/30/19 10:57 AM, David Hildenbrand wrote:
We decided to always create the PCI host bridge, even if 'zpci' is not
enabled (due to migration compatibility). This however right now allows
to add zPCI/PCI
} while (pci_get_bus(pdev) && pci_dev_bus_num(pdev));
+
+s390_pci_update_subordinate(pdev, s->bus_no);
}
} else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
pdev = PCI_DEVICE(dev);
Looks good to me...
Reviewed-by: Collin Walling
Side note: unrelated to th
On 1/30/19 10:57 AM, David Hildenbrand wrote:
We decided to always create the PCI host bridge, even if 'zpci' is not
enabled (due to migration compatibility). This however right now allows
to add zPCI/PCI devices to a VM although the guest will never actually see
them, confusing people that are
On 2/1/19 4:18 AM, David Hildenbrand wrote:
On 01.02.19 09:35, Cornelia Huck wrote:
On Thu, 31 Jan 2019 15:43:16 -0500
Collin Walling wrote:
On 1/30/19 10:57 AM, David Hildenbrand wrote:
These are all the patches that are not yet upstream (@Conny you might
already picked some, including
= {
Good band-aid patch.!
Reviewed-by: Collin Walling
, 5, and 6 make the code a lot easier to understand
what is going on here.
I assume you were able to squeeze in an easy test? At least making sure
we can still hot unplug a device from a guest successfully?
From the code perspective:
Reviewed-by: Collin Walling
On 1/30/19 10:57 AM, David Hildenbrand wrote:
These are all the patches that are not yet upstream (@Conny you might
already picked some, including them for the full picture) and after
a good discussion yesterday, including a patch t get rid of the release
timer. I ran a couple of sanity tests on
l right now overcomplicates
nit: "over-complicates" or "over complicates"
things and doesn't really.
"...and doesn't really." sounds odd to me :)
Let's allow to send multiple requests.
Signed-off-by: David Hildenbrand
---
Just a quick clean up of the commit message, and all is good.
Reviewed-by: Collin Walling
versation regarding PCI states, we can skip this event because we're
removing the device from the guest during a reset here (there are a few
reasons why we'd do this, but no sense in listing them here). Also makes
sense to me.
Reviewed-by: Collin Walling
296 100644
--- a/hw/s390x/s390-pci-bus.h
+++ b/hw/s390x/s390-pci-bus.h
@@ -336,6 +336,7 @@ struct S390PCIBusDevice {
IndAddr *summary_ind;
IndAddr *indicator;
QEMUTimer *release_timer;
+bool pci_unplug_request_processed;
QTAILQ_ENTRY(S390PCIBusDevice) link;
};
Patch looks good so far. I only have a few questions above.
--
Respectfully,
- Collin Walling
t to have relinquish the device.
>>>> Or do they?
>>>> How long do they wait?
>>>
>>> They wait for ever. And I guess we should do the same thing. If the
>>> guest driver is broken (and this is really a rare scenario!) we would
>>> not get the device back. Which is perfectly fine in my point of view. In
>>> all other scenarios I guess the guest will simply respond in a timely
>>> manner. And ripping out stuff from the guest always feels wrong. (yes
>>> the channel subsystem works like that, but here we actually have a choice)
>>>
>>> If we reboot, we can unplug the device. Otherwise, let's keep it simply
>>> and don't use a timer.
>>>
>>> Thanks!
>>>
This makes more sense to me. If something goes wrong with unplugging a device,
and we use a timeout for forcefully unplug the device... it will never be
apparent to the guest or user that something went wrong in the first place!
>>
>> AFAIK We have no plan to operate on pools of PCI devices so for me I
>> have no objection to keep it simple:
>
> Especially one note:
>
> There seems to be demand for a so called "forced PCI removal" also on
> other architectures. However, this would than rather most probably be
> modeled on top of what we have right now.
>
> E.g. instead of "device_del XXX" which would request the guest to let go
> of the device, there could be something like "device_del XXX,forced=true".
>
> E.g. ask the guest. If it does not respond after some time, force remove
> it. This is basically the timer, but managed by a different level, of
> software. And you can than actually decide if you want to do eventually
> harm to the guest OS.
>
> Are there any other objects of getting rid of the timer?
>
> Conny could pick up patch #1 once you get an ACK. I would send more
> patches to drop the timer and rework this patch.
>
> Thanks Pierre!
>
>>
>> Regards,
>> Pierre
>>
>>
>>
>
>
David, Pierre: good discussion.
I'm in favor of dropping the timer, especially with the notes and examples
above. I think the PCI code will look a lot cleaner / easier-to-maintain
without it as well.
--
Respectfully,
- Collin Walling
On 1/28/19 6:28 AM, Cornelia Huck wrote:
On Wed, 23 Jan 2019 12:05:39 +0100
Cornelia Huck wrote:
On Mon, 21 Jan 2019 14:42:49 +0100
David Hildenbrand wrote:
When resetting the guest we should unplug and remove all devices that
are still pending. Otherwise the fresh guest will see devices
So is it a ack from you?
Acked-by: David Hildenbrand
:)
Great!
Still waiting for an ack from Collin (nudge, nudge :) before applying
this.
Whoops, thought I already acked this one -- sorry!
I give this my full Reviewed-by: Collin Walling
return;
+}
s390_pci_generate_plug_event(HP_EVENT_DECONFIGURE_REQUEST,
pbdev->fh, pbdev->fid);
pbdev->release_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
Looks good to me.
Reviewed-by: Collin Walling
when the device is already
in standby (i.e. not configured). So this makes sense to me.
Reviewed-by: Collin Walling
akes sense to one day replace all function calls with the
macro.
Reviewed-by: Collin Walling
RY_REGION,
OBJECT(>mr),
- name, iommu->pal + 1);
+ name, iommu->pal - iommu->pba + 1);
iommu->enabled = true;
memory_region_add_subregion(>mr, 0,
MEMORY_REGION(>iommu_mr));
g_free(name);
Acked-by: Collin Walling
rely on that this works :)
>>>
>>> Yep, let's wait for feedback from folks with architecture access.
>>>
>>
>> Works fine on the architecture too.
>>
>> Seems the logical thing to do for me.
>>
>> Reviewed-by: Pierre Morel
>
> Than
>
> Spotted by Coverity: CID 1398593
>
> Signed-off-by: Li Qiang
> ---
>
[...]
Reviewed-by: Collin Walling
sponse to Thomas.
>> Otherwise have you any remark?
>
> I don't remember anything beyond the endianess issue.
>
This patch looks sane to me (I've lost the parent email on my
client, else I would've replied directly to that).
I'm currently awaiting getting my system up-and-running to test
this thoroughly. Shall we do one more round with the endianess
addressed in the mean time?
--
Respectfully,
- Collin Walling
101 - 200 of 256 matches
Mail list logo