On 1/10/20 8:03 AM, Thomas Huth wrote:
> On 09/01/2020 18.51, Janosch Frank wrote:
>> On 1/9/20 6:08 PM, Cornelia Huck wrote:
>>> On Thu, 9 Jan 2020 10:56:01 -0500
>>> Janosch Frank <frankja@xxxxxxxxxxxxx> 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 on a normal reset.
>>>>
>>>> Let's implement an interface for the missing normal and clear resets
>>>> and reset all local IRQs, registers and control structures as stated
>>>> in the architecture.
>>>>
>>>> Userspace might already reset the registers via the vcpu run struct,
>>>> but as we need the interface for the interrupt clearing part anyway,
>>>> we implement the resets fully and don't rely on userspace to reset the
>>>> rest.
>>>>
>>>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
>>>> ---
>>>>
>>>> I dropped the reviews, as I changed quite a lot.
>>>>
>>>> Keep in mind, that now we'll need a new parameter in normal and
>>>> initial reset for protected virtualization to indicate that we need to
>>>> do the reset via the UV call. The Ultravisor does only accept the
>>>> needed reset, not any subset resets.
>>>
>>> In the interface, or externally?
>>
>> ?
>>
>>>
>>> [Apologies, but the details of the protected virt stuff are no longer
>>> in my cache.
>> Reworded explanation:
>> I can't use a fallthrough, because the UV will reject the normal reset
>> if we do an initial reset (same goes for the clear reset). To address
>> this issue, I added a boolean to the normal and initial reset functions
>> which tells the function if it was called directly or was called because
>> of the fallthrough.
>>
>> Only if called directly a UV call for the reset is done, that way we can
>> keep the fallthrough.
>
> Sounds complicated. And do we need the fallthrough stuff here at all?
> What about doing something like:
That would work and I thought about it, it just comes down to taste :-)
I don't have any strong feelings for a specific implementation.
>
> static int kvm_arch_vcpu_ioctl_normal_reset(struct kvm_vcpu *vcpu)
> {
> ...
> }
>
> static int kvm_arch_vcpu_ioctl_initial_reset(struct kvm_vcpu *vcpu)
> {
> kvm_arch_vcpu_ioctl_normal_reset(vcpu);
> ...
> }
>
> static int kvm_arch_vcpu_ioctl_clear_reset(struct kvm_vcpu *vcpu)
> {
> kvm_arch_vcpu_ioctl_initial_reset(vcpu);
> ...
> }
>
> ...
>
> case KVM_S390_CLEAR_RESET:
> r = kvm_arch_vcpu_ioctl_clear_reset(vcpu);
> if (!r && protected) {
> r = uv_cmd_nodata(...,
> UVC_CMD_CPU_RESET_CLEAR, ...);
> }
> break;
> case KVM_S390_INITIAL_RESET:
> r = kvm_arch_vcpu_ioctl_initial_reset(vcpu);
> if (!r && protected) {
> r = uv_cmd_nodata(...,
> UVC_CMD_CPU_RESET_INITIAL, ...);
> }
> case KVM_S390_NORMAL_RESET:
> r = kvm_arch_vcpu_ioctl_normal_reset(vcpu);
> if (!r && protected) {
> r = uv_cmd_nodata(...,
> UVC_CMD_CPU_RESET, ...);
> }
> break;
>
> ... or does that not work due to some other constraints that I've missed?
>
> Thomas
>
Attachment:
signature.asc
Description: OpenPGP digital signature
