Received-SPF: pass (google.com: domain of linux-kernel+bounces-67329-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Thu, 15 Feb 2024 11:51:05 -0500
From: Charlie Jenkins <charlie@rivosinc.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: John David Anglin <dave.anglin@bell.net>,
	David Laight <David.Laight@aculab.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Helge Deller <deller@gmx.de>,
	"James E.J. Bottomley" <James.Bottomley@hansenpartnership.com>,
	Parisc List <linux-parisc@vger.kernel.org>,
	Al Viro <viro@zeniv.linux.org.uk>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v8 2/2] lib: checksum: Use aligned accesses for
 ip_fast_csum and csum_ipv6_magic tests
Message-ID: <Zc5A+c0gmxtygNIa@ghost>
References: <20240214-fix_sparse_errors_checksum_tests-v8-0-36b60e673593@rivosinc.com>
 <20240214-fix_sparse_errors_checksum_tests-v8-2-36b60e673593@rivosinc.com>
 <2ec91b11-23c7-4beb-8cef-c68367c8f029@roeck-us.net>
 <Zc1pSi59aDOnqz++@ghost>
 <cb4e358b-3fd0-4ca4-bf53-9cc379087304@roeck-us.net>
 <1d5e059e-5b31-415d-ae41-593415812e94@bell.net>
 <Zc2GfgiCpevtKTtS@ghost>
 <11fff7fe-ec4c-4340-a67f-d1d54d0712d8@roeck-us.net>
 <Zc4veRSmxHHqu/bB@ghost>
 <0fa4d53a-6ce7-4f44-81b2-86282f0b9451@roeck-us.net>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <0fa4d53a-6ce7-4f44-81b2-86282f0b9451@roeck-us.net>

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.

- 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
>