Avoid invalid bits in kvm_valid_regs and kvm_dirty_regs on s390x.
v2:
- Split the single patch from v1 into two separate patches
(I've kept the Reviewed-bys from v1, but if you don't agree with the
patch description of the 2nd patch, please complain)
Thomas Huth (2):
KVM: s390: Disallow invalid bits in kvm_valid_regs and kvm_dirty_regs
KVM: selftests: Test invalid bits in kvm_valid_regs and kvm_dirty_regs
on s390x
arch/s390/include/uapi/asm/kvm.h | 6 ++++
arch/s390/kvm/kvm-s390.c | 4 +++
.../selftests/kvm/s390x/sync_regs_test.c | 30 +++++++++++++++++++
3 files changed, 40 insertions(+)
--
2.18.1
If unknown bits are set in kvm_valid_regs or kvm_dirty_regs, this
clearly indicates that something went wrong in the KVM userspace
application. The x86 variant of KVM already contains a check for
bad bits, so let's do the same on s390x now, too.
Reviewed-by: Janosch Frank <[email protected]>
Reviewed-by: Christian Borntraeger <[email protected]>
Reviewed-by: Cornelia Huck <[email protected]>
Signed-off-by: Thomas Huth <[email protected]>
---
arch/s390/include/uapi/asm/kvm.h | 6 ++++++
arch/s390/kvm/kvm-s390.c | 4 ++++
2 files changed, 10 insertions(+)
diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
index 47104e5b47fd..436ec7636927 100644
--- a/arch/s390/include/uapi/asm/kvm.h
+++ b/arch/s390/include/uapi/asm/kvm.h
@@ -231,6 +231,12 @@ struct kvm_guest_debug_arch {
#define KVM_SYNC_GSCB (1UL << 9)
#define KVM_SYNC_BPBC (1UL << 10)
#define KVM_SYNC_ETOKEN (1UL << 11)
+
+#define KVM_SYNC_S390_VALID_FIELDS \
+ (KVM_SYNC_PREFIX | KVM_SYNC_GPRS | KVM_SYNC_ACRS | KVM_SYNC_CRS | \
+ KVM_SYNC_ARCH0 | KVM_SYNC_PFAULT | KVM_SYNC_VRS | KVM_SYNC_RICCB | \
+ KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN)
+
/* length and alignment of the sdnx as a power of two */
#define SDNXC 8
#define SDNXL (1UL << SDNXC)
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 49d7722229ae..a7d7dedfe527 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -3998,6 +3998,10 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
if (kvm_run->immediate_exit)
return -EINTR;
+ if (kvm_run->kvm_valid_regs & ~KVM_SYNC_S390_VALID_FIELDS ||
+ kvm_run->kvm_dirty_regs & ~KVM_SYNC_S390_VALID_FIELDS)
+ return -EINVAL;
+
vcpu_load(vcpu);
if (guestdbg_exit_pending(vcpu)) {
--
2.18.1
Now that we disallow invalid bits in kvm_valid_regs and kvm_dirty_regs
on s390x, too, we should also check this condition in the selftests.
The code has been taken from the x86-version of the sync_regs_test.
Reviewed-by: Janosch Frank <[email protected]>
Reviewed-by: Christian Borntraeger <[email protected]>
Reviewed-by: Cornelia Huck <[email protected]>
Signed-off-by: Thomas Huth <[email protected]>
---
.../selftests/kvm/s390x/sync_regs_test.c | 30 +++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/tools/testing/selftests/kvm/s390x/sync_regs_test.c b/tools/testing/selftests/kvm/s390x/sync_regs_test.c
index bbc93094519b..d5290b4ad636 100644
--- a/tools/testing/selftests/kvm/s390x/sync_regs_test.c
+++ b/tools/testing/selftests/kvm/s390x/sync_regs_test.c
@@ -85,6 +85,36 @@ int main(int argc, char *argv[])
run = vcpu_state(vm, VCPU_ID);
+ /* Request reading invalid register set from VCPU. */
+ run->kvm_valid_regs = INVALID_SYNC_FIELD;
+ rv = _vcpu_run(vm, VCPU_ID);
+ TEST_ASSERT(rv < 0 && errno == EINVAL,
+ "Invalid kvm_valid_regs did not cause expected KVM_RUN error: %d\n",
+ rv);
+ vcpu_state(vm, VCPU_ID)->kvm_valid_regs = 0;
+
+ run->kvm_valid_regs = INVALID_SYNC_FIELD | TEST_SYNC_FIELDS;
+ rv = _vcpu_run(vm, VCPU_ID);
+ TEST_ASSERT(rv < 0 && errno == EINVAL,
+ "Invalid kvm_valid_regs did not cause expected KVM_RUN error: %d\n",
+ rv);
+ vcpu_state(vm, VCPU_ID)->kvm_valid_regs = 0;
+
+ /* Request setting invalid register set into VCPU. */
+ run->kvm_dirty_regs = INVALID_SYNC_FIELD;
+ rv = _vcpu_run(vm, VCPU_ID);
+ TEST_ASSERT(rv < 0 && errno == EINVAL,
+ "Invalid kvm_dirty_regs did not cause expected KVM_RUN error: %d\n",
+ rv);
+ vcpu_state(vm, VCPU_ID)->kvm_dirty_regs = 0;
+
+ run->kvm_dirty_regs = INVALID_SYNC_FIELD | TEST_SYNC_FIELDS;
+ rv = _vcpu_run(vm, VCPU_ID);
+ TEST_ASSERT(rv < 0 && errno == EINVAL,
+ "Invalid kvm_dirty_regs did not cause expected KVM_RUN error: %d\n",
+ rv);
+ vcpu_state(vm, VCPU_ID)->kvm_dirty_regs = 0;
+
/* Request and verify all valid register sets. */
run->kvm_valid_regs = TEST_SYNC_FIELDS;
rv = _vcpu_run(vm, VCPU_ID);
--
2.18.1
On 04.09.19 10:52, Thomas Huth wrote:
> Now that we disallow invalid bits in kvm_valid_regs and kvm_dirty_regs
> on s390x, too, we should also check this condition in the selftests.
> The code has been taken from the x86-version of the sync_regs_test.
>
> Reviewed-by: Janosch Frank <[email protected]>
> Reviewed-by: Christian Borntraeger <[email protected]>
> Reviewed-by: Cornelia Huck <[email protected]>
> Signed-off-by: Thomas Huth <[email protected]>
> ---
> .../selftests/kvm/s390x/sync_regs_test.c | 30 +++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/tools/testing/selftests/kvm/s390x/sync_regs_test.c b/tools/testing/selftests/kvm/s390x/sync_regs_test.c
> index bbc93094519b..d5290b4ad636 100644
> --- a/tools/testing/selftests/kvm/s390x/sync_regs_test.c
> +++ b/tools/testing/selftests/kvm/s390x/sync_regs_test.c
> @@ -85,6 +85,36 @@ int main(int argc, char *argv[])
>
> run = vcpu_state(vm, VCPU_ID);
>
> + /* Request reading invalid register set from VCPU. */
> + run->kvm_valid_regs = INVALID_SYNC_FIELD;
> + rv = _vcpu_run(vm, VCPU_ID);
> + TEST_ASSERT(rv < 0 && errno == EINVAL,
> + "Invalid kvm_valid_regs did not cause expected KVM_RUN error: %d\n",
> + rv);
> + vcpu_state(vm, VCPU_ID)->kvm_valid_regs = 0;
> +
> + run->kvm_valid_regs = INVALID_SYNC_FIELD | TEST_SYNC_FIELDS;
> + rv = _vcpu_run(vm, VCPU_ID);
> + TEST_ASSERT(rv < 0 && errno == EINVAL,
> + "Invalid kvm_valid_regs did not cause expected KVM_RUN error: %d\n",
> + rv);
> + vcpu_state(vm, VCPU_ID)->kvm_valid_regs = 0;
> +
> + /* Request setting invalid register set into VCPU. */
> + run->kvm_dirty_regs = INVALID_SYNC_FIELD;
> + rv = _vcpu_run(vm, VCPU_ID);
> + TEST_ASSERT(rv < 0 && errno == EINVAL,
> + "Invalid kvm_dirty_regs did not cause expected KVM_RUN error: %d\n",
> + rv);
> + vcpu_state(vm, VCPU_ID)->kvm_dirty_regs = 0;
> +
> + run->kvm_dirty_regs = INVALID_SYNC_FIELD | TEST_SYNC_FIELDS;
> + rv = _vcpu_run(vm, VCPU_ID);
> + TEST_ASSERT(rv < 0 && errno == EINVAL,
> + "Invalid kvm_dirty_regs did not cause expected KVM_RUN error: %d\n",
> + rv);
> + vcpu_state(vm, VCPU_ID)->kvm_dirty_regs = 0;
> +
> /* Request and verify all valid register sets. */
> run->kvm_valid_regs = TEST_SYNC_FIELDS;
> rv = _vcpu_run(vm, VCPU_ID);
>
Looks sane to me.
--
Thanks,
David / dhildenb
On 04.09.19 10:51, Thomas Huth wrote:
> If unknown bits are set in kvm_valid_regs or kvm_dirty_regs, this
> clearly indicates that something went wrong in the KVM userspace
> application. The x86 variant of KVM already contains a check for
> bad bits, so let's do the same on s390x now, too.
>
> Reviewed-by: Janosch Frank <[email protected]>
> Reviewed-by: Christian Borntraeger <[email protected]>
> Reviewed-by: Cornelia Huck <[email protected]>
> Signed-off-by: Thomas Huth <[email protected]>
> ---
> arch/s390/include/uapi/asm/kvm.h | 6 ++++++
> arch/s390/kvm/kvm-s390.c | 4 ++++
> 2 files changed, 10 insertions(+)
>
> diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
> index 47104e5b47fd..436ec7636927 100644
> --- a/arch/s390/include/uapi/asm/kvm.h
> +++ b/arch/s390/include/uapi/asm/kvm.h
> @@ -231,6 +231,12 @@ struct kvm_guest_debug_arch {
> #define KVM_SYNC_GSCB (1UL << 9)
> #define KVM_SYNC_BPBC (1UL << 10)
> #define KVM_SYNC_ETOKEN (1UL << 11)
> +
> +#define KVM_SYNC_S390_VALID_FIELDS \
> + (KVM_SYNC_PREFIX | KVM_SYNC_GPRS | KVM_SYNC_ACRS | KVM_SYNC_CRS | \
> + KVM_SYNC_ARCH0 | KVM_SYNC_PFAULT | KVM_SYNC_VRS | KVM_SYNC_RICCB | \
> + KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN)
> +
We didn't care about the S390 for the actual flags, why care now?
> /* length and alignment of the sdnx as a power of two */
> #define SDNXC 8
> #define SDNXL (1UL << SDNXC)
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 49d7722229ae..a7d7dedfe527 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -3998,6 +3998,10 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
> if (kvm_run->immediate_exit)
> return -EINTR;
>
> + if (kvm_run->kvm_valid_regs & ~KVM_SYNC_S390_VALID_FIELDS ||
> + kvm_run->kvm_dirty_regs & ~KVM_SYNC_S390_VALID_FIELDS)
> + return -EINVAL;
> +
> vcpu_load(vcpu);
>
> if (guestdbg_exit_pending(vcpu)) {
>
Reviewed-by: David Hildenbrand <[email protected]>
--
Thanks,
David / dhildenb
On 04.09.19 11:05, David Hildenbrand wrote:
> On 04.09.19 10:51, Thomas Huth wrote:
>> If unknown bits are set in kvm_valid_regs or kvm_dirty_regs, this
>> clearly indicates that something went wrong in the KVM userspace
>> application. The x86 variant of KVM already contains a check for
>> bad bits, so let's do the same on s390x now, too.
>>
>> Reviewed-by: Janosch Frank <[email protected]>
>> Reviewed-by: Christian Borntraeger <[email protected]>
>> Reviewed-by: Cornelia Huck <[email protected]>
>> Signed-off-by: Thomas Huth <[email protected]>
>> ---
>> arch/s390/include/uapi/asm/kvm.h | 6 ++++++
>> arch/s390/kvm/kvm-s390.c | 4 ++++
>> 2 files changed, 10 insertions(+)
>>
>> diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
>> index 47104e5b47fd..436ec7636927 100644
>> --- a/arch/s390/include/uapi/asm/kvm.h
>> +++ b/arch/s390/include/uapi/asm/kvm.h
>> @@ -231,6 +231,12 @@ struct kvm_guest_debug_arch {
>> #define KVM_SYNC_GSCB (1UL << 9)
>> #define KVM_SYNC_BPBC (1UL << 10)
>> #define KVM_SYNC_ETOKEN (1UL << 11)
>> +
>> +#define KVM_SYNC_S390_VALID_FIELDS \
>> + (KVM_SYNC_PREFIX | KVM_SYNC_GPRS | KVM_SYNC_ACRS | KVM_SYNC_CRS | \
>> + KVM_SYNC_ARCH0 | KVM_SYNC_PFAULT | KVM_SYNC_VRS | KVM_SYNC_RICCB | \
>> + KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN)
>> +
>
> We didn't care about the S390 for the actual flags, why care now?
I think it makes sense to have the interface as defined as possible. If for some
reason userspace sets a wrong bit this would be undetected. If we at a later point
in time use that bit this would resultin strange problems.
>
>
>> /* length and alignment of the sdnx as a power of two */
>> #define SDNXC 8
>> #define SDNXL (1UL << SDNXC)
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 49d7722229ae..a7d7dedfe527 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -3998,6 +3998,10 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>> if (kvm_run->immediate_exit)
>> return -EINTR;
>>
>> + if (kvm_run->kvm_valid_regs & ~KVM_SYNC_S390_VALID_FIELDS ||
>> + kvm_run->kvm_dirty_regs & ~KVM_SYNC_S390_VALID_FIELDS)
>> + return -EINVAL;
>> +
>> vcpu_load(vcpu);
>>
>> if (guestdbg_exit_pending(vcpu)) {
>>
>
> Reviewed-by: David Hildenbrand <[email protected]>
>
On 04/09/2019 11.05, David Hildenbrand wrote:
> On 04.09.19 10:51, Thomas Huth wrote:
>> If unknown bits are set in kvm_valid_regs or kvm_dirty_regs, this
>> clearly indicates that something went wrong in the KVM userspace
>> application. The x86 variant of KVM already contains a check for
>> bad bits, so let's do the same on s390x now, too.
>>
>> Reviewed-by: Janosch Frank <[email protected]>
>> Reviewed-by: Christian Borntraeger <[email protected]>
>> Reviewed-by: Cornelia Huck <[email protected]>
>> Signed-off-by: Thomas Huth <[email protected]>
>> ---
>> arch/s390/include/uapi/asm/kvm.h | 6 ++++++
>> arch/s390/kvm/kvm-s390.c | 4 ++++
>> 2 files changed, 10 insertions(+)
>>
>> diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
>> index 47104e5b47fd..436ec7636927 100644
>> --- a/arch/s390/include/uapi/asm/kvm.h
>> +++ b/arch/s390/include/uapi/asm/kvm.h
>> @@ -231,6 +231,12 @@ struct kvm_guest_debug_arch {
>> #define KVM_SYNC_GSCB (1UL << 9)
>> #define KVM_SYNC_BPBC (1UL << 10)
>> #define KVM_SYNC_ETOKEN (1UL << 11)
>> +
>> +#define KVM_SYNC_S390_VALID_FIELDS \
>> + (KVM_SYNC_PREFIX | KVM_SYNC_GPRS | KVM_SYNC_ACRS | KVM_SYNC_CRS | \
>> + KVM_SYNC_ARCH0 | KVM_SYNC_PFAULT | KVM_SYNC_VRS | KVM_SYNC_RICCB | \
>> + KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN)
>> +
>
> We didn't care about the S390 for the actual flags, why care now?
x86 does the same, and we don't want to be worse than x86, do we? ;-)
Honestly, this was just one of the differences that I noticed while
porting the sync_regs_test from x86 to s390x.
Thomas
On 04.09.19 11:10, Thomas Huth wrote:
> On 04/09/2019 11.05, David Hildenbrand wrote:
>> On 04.09.19 10:51, Thomas Huth wrote:
>>> If unknown bits are set in kvm_valid_regs or kvm_dirty_regs, this
>>> clearly indicates that something went wrong in the KVM userspace
>>> application. The x86 variant of KVM already contains a check for
>>> bad bits, so let's do the same on s390x now, too.
>>>
>>> Reviewed-by: Janosch Frank <[email protected]>
>>> Reviewed-by: Christian Borntraeger <[email protected]>
>>> Reviewed-by: Cornelia Huck <[email protected]>
>>> Signed-off-by: Thomas Huth <[email protected]>
>>> ---
>>> arch/s390/include/uapi/asm/kvm.h | 6 ++++++
>>> arch/s390/kvm/kvm-s390.c | 4 ++++
>>> 2 files changed, 10 insertions(+)
>>>
>>> diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
>>> index 47104e5b47fd..436ec7636927 100644
>>> --- a/arch/s390/include/uapi/asm/kvm.h
>>> +++ b/arch/s390/include/uapi/asm/kvm.h
>>> @@ -231,6 +231,12 @@ struct kvm_guest_debug_arch {
>>> #define KVM_SYNC_GSCB (1UL << 9)
>>> #define KVM_SYNC_BPBC (1UL << 10)
>>> #define KVM_SYNC_ETOKEN (1UL << 11)
>>> +
>>> +#define KVM_SYNC_S390_VALID_FIELDS \
>>> + (KVM_SYNC_PREFIX | KVM_SYNC_GPRS | KVM_SYNC_ACRS | KVM_SYNC_CRS | \
>>> + KVM_SYNC_ARCH0 | KVM_SYNC_PFAULT | KVM_SYNC_VRS | KVM_SYNC_RICCB | \
>>> + KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN)
>>> +
>>
>> We didn't care about the S390 for the actual flags, why care now?
>
> x86 does the same, and we don't want to be worse than x86, do we? ;-)
Yeah, but they do have X86 in every flag - in contrast to us.
>
> Honestly, this was just one of the differences that I noticed while
> porting the sync_regs_test from x86 to s390x.
>
> Thomas
>
--
Thanks,
David / dhildenb
On 04.09.19 11:11, Christian Borntraeger wrote:
>
>
> On 04.09.19 11:05, David Hildenbrand wrote:
>> On 04.09.19 10:51, Thomas Huth wrote:
>>> If unknown bits are set in kvm_valid_regs or kvm_dirty_regs, this
>>> clearly indicates that something went wrong in the KVM userspace
>>> application. The x86 variant of KVM already contains a check for
>>> bad bits, so let's do the same on s390x now, too.
>>>
>>> Reviewed-by: Janosch Frank <[email protected]>
>>> Reviewed-by: Christian Borntraeger <[email protected]>
>>> Reviewed-by: Cornelia Huck <[email protected]>
>>> Signed-off-by: Thomas Huth <[email protected]>
>>> ---
>>> arch/s390/include/uapi/asm/kvm.h | 6 ++++++
>>> arch/s390/kvm/kvm-s390.c | 4 ++++
>>> 2 files changed, 10 insertions(+)
>>>
>>> diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
>>> index 47104e5b47fd..436ec7636927 100644
>>> --- a/arch/s390/include/uapi/asm/kvm.h
>>> +++ b/arch/s390/include/uapi/asm/kvm.h
>>> @@ -231,6 +231,12 @@ struct kvm_guest_debug_arch {
>>> #define KVM_SYNC_GSCB (1UL << 9)
>>> #define KVM_SYNC_BPBC (1UL << 10)
>>> #define KVM_SYNC_ETOKEN (1UL << 11)
>>> +
>>> +#define KVM_SYNC_S390_VALID_FIELDS \
>>> + (KVM_SYNC_PREFIX | KVM_SYNC_GPRS | KVM_SYNC_ACRS | KVM_SYNC_CRS | \
>>> + KVM_SYNC_ARCH0 | KVM_SYNC_PFAULT | KVM_SYNC_VRS | KVM_SYNC_RICCB | \
>>> + KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN)
>>> +
>>
>> We didn't care about the S390 for the actual flags, why care now?
>
> I think it makes sense to have the interface as defined as possible. If for some
> reason userspace sets a wrong bit this would be undetected. If we at a later point
> in time use that bit this would resultin strange problems.
Not arguing about the concept of checking for valid bits. Was just
wondering if the "S390" part in the name makes sense at all. But you
guys seem to have a consent here.
--
Thanks,
David / dhildenb
On 04/09/2019 11.15, David Hildenbrand wrote:
> On 04.09.19 11:11, Christian Borntraeger wrote:
>>
>>
>> On 04.09.19 11:05, David Hildenbrand wrote:
>>> On 04.09.19 10:51, Thomas Huth wrote:
>>>> If unknown bits are set in kvm_valid_regs or kvm_dirty_regs, this
>>>> clearly indicates that something went wrong in the KVM userspace
>>>> application. The x86 variant of KVM already contains a check for
>>>> bad bits, so let's do the same on s390x now, too.
>>>>
>>>> Reviewed-by: Janosch Frank <[email protected]>
>>>> Reviewed-by: Christian Borntraeger <[email protected]>
>>>> Reviewed-by: Cornelia Huck <[email protected]>
>>>> Signed-off-by: Thomas Huth <[email protected]>
>>>> ---
>>>> arch/s390/include/uapi/asm/kvm.h | 6 ++++++
>>>> arch/s390/kvm/kvm-s390.c | 4 ++++
>>>> 2 files changed, 10 insertions(+)
>>>>
>>>> diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
>>>> index 47104e5b47fd..436ec7636927 100644
>>>> --- a/arch/s390/include/uapi/asm/kvm.h
>>>> +++ b/arch/s390/include/uapi/asm/kvm.h
>>>> @@ -231,6 +231,12 @@ struct kvm_guest_debug_arch {
>>>> #define KVM_SYNC_GSCB (1UL << 9)
>>>> #define KVM_SYNC_BPBC (1UL << 10)
>>>> #define KVM_SYNC_ETOKEN (1UL << 11)
>>>> +
>>>> +#define KVM_SYNC_S390_VALID_FIELDS \
>>>> + (KVM_SYNC_PREFIX | KVM_SYNC_GPRS | KVM_SYNC_ACRS | KVM_SYNC_CRS | \
>>>> + KVM_SYNC_ARCH0 | KVM_SYNC_PFAULT | KVM_SYNC_VRS | KVM_SYNC_RICCB | \
>>>> + KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN)
>>>> +
>>>
>>> We didn't care about the S390 for the actual flags, why care now?
>>
>> I think it makes sense to have the interface as defined as possible. If for some
>> reason userspace sets a wrong bit this would be undetected. If we at a later point
>> in time use that bit this would resultin strange problems.
>
> Not arguing about the concept of checking for valid bits. Was just
> wondering if the "S390" part in the name makes sense at all. But you
> guys seem to have a consent here.
Oh, I guess we both got your question wrong... Well, I don't care too
much whether we've got an "S390" in there or not ... so unless there is
a real good reason to remove it, let's simply keep it this way.
Thomas
On 9/4/19 10:51 AM, Thomas Huth wrote:
> Avoid invalid bits in kvm_valid_regs and kvm_dirty_regs on s390x.
>
> v2:
> - Split the single patch from v1 into two separate patches
> (I've kept the Reviewed-bys from v1, but if you don't agree with the
> patch description of the 2nd patch, please complain)
>
> Thomas Huth (2):
> KVM: s390: Disallow invalid bits in kvm_valid_regs and kvm_dirty_regs
> KVM: selftests: Test invalid bits in kvm_valid_regs and kvm_dirty_regs
> on s390x
>
> arch/s390/include/uapi/asm/kvm.h | 6 ++++
> arch/s390/kvm/kvm-s390.c | 4 +++
> .../selftests/kvm/s390x/sync_regs_test.c | 30 +++++++++++++++++++
> 3 files changed, 40 insertions(+)
>
Thanks, both are picked