Re: [PATCH v8 2/2] lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests

[John David Anglin <dave.anglin@bell.net>]

On 2024-02-15 11:51 a.m., Charlie Jenkins wrote:
> On Thu, Feb 15, 2024 at 08:30:22AM -0800, Guenter Roeck wrote:
>> On 2/15/24 07:36, Charlie Jenkins wrote:
>>> On Thu, Feb 15, 2024 at 12:56:13AM -0800, Guenter Roeck wrote:
>>>> On 2/14/24 19:35, Charlie Jenkins wrote:
>>>>> On Wed, Feb 14, 2024 at 10:00:37PM -0500, John David Anglin wrote:
>>>>>> On 2024-02-14 8:58 p.m., Guenter Roeck wrote:
>>>>>>> Specifically: Yes, the carry/borrow bits should be restored. Question is
>>>>>>> if the Linux kernel's interrupt handler doesn't restore the carry bits
>>>>>>> or if the problem is on the qemu side.
>>>>>> The carry/borrow bits in the PSW should be saved and restored by the save_specials
>>>>>> and rest_specials macros.  They are defined in arch/parisc/include/asm/assembly.h.
>>>>> Why would they be needed to be restored in linux? The manual says "The
>>>>> PSW is set to the contents of the IPSW by the RETURN FROM INTERRUPTION
>>>>> instruction". This means that the PSW must be restored by the hardware.
>>>>>
>>>>> We can see the QEMU implementation in:
>>>>>
>>>>> rfi:
>>>>> https://github.com/qemu/qemu/blob/v8.2.1/target/hppa/sys_helper.c#L93
>>>>>
>>>>> handling interrupt:
>>>>> https://github.com/qemu/qemu/blob/v8.2.1/target/hppa/int_helper.c#L109
>>>>>
>>>>> However the implementation appears to be faulty. During an RFI, the PSW
>>>>> is always set to 0x804000e (regardless of what the PSW was before the
>>>>> interrupt).
>>>>>
>>>> Not sure if I agree. The interrupt handler in Linux is the one which needs to set
>>>> IPSW. Looking into the code, I agree with Dave that the tophys macro seems to
>>>> clobber the carry bits before psw is saved, so they can not really be restored.
>>>> The only issue with that idea is that I can only reproduce the problem with
>>>> an interrupted ldd instruction but not, for example, with ldw. This is why it
>>>> would be really important to have someone with real hardware test this.
>>>>
>>>> Thanks,
>>>> Guenter
>>> Yes, we definitely feedback from somebody with access to hardware, but I
>>> do not understand how "The PSW is set to the contents of the IPSW by the
>>> RETURN FROM INTERRUPTION" could be interpreted as anything except that
>>> the hardware is expected to over-write the contents of the PSW during
>>> the rfi.
>>>
>> Sure, I absolutely agree. But that assumes that IPSW is set correctly
>> in the Linux interrupt handler. We do know that something odd happens
> The manual defines the saving of PSW as the responsibility of the
> hardware as well: "When an interruption occurs, the current value of the
> PSW is saved in the Interruption Processor Status Word (IPSW)". I don't
> think this should be interpreted to mean that a software interrupt
> handler is required to save the IPSW.
The IPSW (cr22) is saved by save_specials to regs->gr[0].  It is modified in various
places when an interruption is handled.  In the case of emulate_ldd, we have

         /* else we handled it, let life go on. */
         regs->gr[0]|=PSW_N;

This is supposed to nullify the faulting ldd.  I've yet to find where the carry bit is getting
set in the PSW.

There is a gap between where the hardware sets IPSW and where it is saved to the stack
in save_specials.  tophys might clobber the IPSW is there is a double fault.  But it seems
more likely that regs->gr[0] is getting clobbered somehow.

Dave
>
> - Charlie
>
>> when an unaligned ldd is encountered. At least for my part I don't know
>> if the problem is in emulate_ldd() in the Linux kernel or in the ldd
>> implementation and trap handling in qemu. I do know (from my logs)
>> that qemu does see the correct PSW/IPSW values, because they do
>> show up correctly in the Linux kernel when running the qemu emulation.
>> Only it somehow gets lost when the Linux interrupt handler returns.
>>
>> Thanks.
>> Guenter
>>


-- 
John David Anglin  dave.anglin@bell.net