Re: [PATCH v4] KVM: s390: Add new reset vcpu API

[Thomas Huth <thuth@redhat.com>]

On 05/12/2019 13.28, Janosch Frank wrote:
> The architecture states that we need to reset local IRQs for all CPU
> resets. Because the old reset interface did not support the normal CPU
> reset we never did that. Now that we have a new interface, let's
> properly clear out local IRQs.
> 
> Also we add a ioctl for the clear reset to have all resets exposed to
> userspace. Currently the clear reset falls back to the initial reset,
> but we plan to have clear reset specific code in the future.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> ---
>  Documentation/virt/kvm/api.txt | 48 ++++++++++++++++++++++++++++++++++
>  arch/s390/kvm/kvm-s390.c       | 14 ++++++++++
>  include/uapi/linux/kvm.h       |  5 ++++
>  3 files changed, 67 insertions(+)
[...]
> +4.123 KVM_S390_CLEAR_RESET
> +
> +Capability: KVM_CAP_S390_VCPU_RESETS
> +Architectures: s390
> +Type: vcpu ioctl
> +Parameters: none
> +Returns: 0
> +
> +This ioctl resets VCPU registers and control structures that userspace
> +can't access via the kvm_run structure. It is intended to be called
> +when an initial reset (which is a superset of the normal reset) is
> +performed on the vcpu and additionally clears general, access,
> +floating and vector registers.

So now you've documented that this ioctl clears the GPRs, ARs and
FRs/VRs ... but the implementation does not! That's quite ugly.
Can you please state clearly that it is the job of userspace to clear
these registers (in the default, non-protected case) and that this ioctl
should be called on top?

Same problem with the PSW bit 24 and riccb during normal reset.

Or should the kernel code maybe also clear these in addition to
userspace, just to be in line with the initial reset?

 Thomas