Re: [PATCH v1] KVM: s390: store DXC/VXC in fpc on DATA/Vector-processing exceptions

[David Hildenbrand <david@redhat.com>]

On 24.08.2018 08:34, Christian Borntraeger wrote:
> 
> 
> On 08/23/2018 07:44 PM, David Hildenbrand wrote:
>> On 23.08.2018 17:43, Christian Borntraeger wrote:
>>>
>>>
>>> On 08/22/2018 11:53 AM, David Hildenbrand wrote:
>>>> When DATA exceptions and vector-processing exceptions (program interrupts)
>>>> are injected, the DXC/VXC is also to be stored in the fpc, if AFP is
>>>> enabled in CR0.
>>>>
>>>> This can happen inside KVM when reinjecting an interrupt during program
>>>> interrupt intercepts. These are triggered for example when debugging the
>>>> guest (concurrent PER events result in an intercept instead of an
>>>> injection of such interrupts).
>>>>
>>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>>> ---
>>>>
>>>> Only compile-tested.
>>>
>>> I checked the Linux code (arch/s390/kernel/traps.c) and Linux uses the FPC (and
>>> not the lowcore field) to decide about the signal (SIGFPE) and si_code. So we want
>>> to have the correct DXC/VXC value.
>>>
>>> Now, I wrote a short test program that does
>>> feenableexcept(FE_DIVBYZERO);
>>> and a division by zero.
>>> and attached gdb to that guest together with a breakpoint on the divide (and the instruction
>>> after).
>>> I get the pint exit for the instruction after (as it is suppressing) and at this point in
>>> time the guest fpc already contains the correct DXC value. So you patch will certainly not
>>> hurt, but it seems not necessary.
>>
>> Thanks for trying. Wonder if that is documented behavior or just works
>> by pure luck.
> 
> 
> My guess is, that this is works as designed. There is the interruption
> parameter block that is used instead of the guest lowcore for program
> interrupt exits. To me it looks like that everything is "prepared" except
> for the psw swap itself and the data in the lowcore. The data is written 
> to the interruption parameter block instead. So that the hypervisor then
> just has to move the data and do the psw swap. 
> 
>>
>> E.g. it would be interesting to see what other instructions do that
>> usually don't touch the DXC, except when injecting an exception. E.g. CRT.
>>
>> But if you believe this is not needed, we can also drop it. (if ever
>> somebody would want to inject from QEMU, he could also just set the fpc
>> directly)
> 
> The (unlikely to ever happen) inject from QEMU is indeed a thing where this
> patch would simplify things.
> 
> I will talk to some hardware folks to verify my assumption but for the time
> being, lets drop this patch.

Just tested CRTG, and it also seems to work fine when single-stepping
over it, landing in the PGM handler.

-- 

Thanks,

David / dhildenb