From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f41.google.com (mail-wm1-f41.google.com [209.85.128.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EA27A13B58D for ; Tue, 7 Jan 2025 08:32:32 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.41 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736238754; cv=none; b=Ui1g6z6XLqkHT/Vj1lIZsYRZoDko4orZuQ1tJPrYuIPfeTXMwF7jUbpniWN0Z0+BYUiILUt7vY+5D7L6fYDGgaevW2q1DrC8R0iJQX+FldGfdb0toYyAvulzffjQ7NkLEz2/Al+1qKBkBnDP3bB5RvCsBQMuUHLK8qi99j5mO3M= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736238754; c=relaxed/simple; bh=uhJSlEl3blxdj+gvPOntfuTRMAhUxKZptA00glyEAUA=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=hFolZC7E8pB1/WGBSk6iei2UwwWfCFg0sQBvnBBL8AFFmCsdanuDUhPInev5+lhi7px1ZsGsAtAnH5TFzNU/g/jNT2MtQdzOTBdd9xoKGibgJvIWdVKG9czseFJ/D5E53ciAYI+7pxIIPMjUSdvwQDPxsdl8K6ipSyiGqVScT18= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=rivosinc.com; spf=pass smtp.mailfrom=rivosinc.com; dkim=pass (2048-bit key) header.d=rivosinc-com.20230601.gappssmtp.com header.i=@rivosinc-com.20230601.gappssmtp.com header.b=vFP6w0+z; arc=none smtp.client-ip=209.85.128.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=rivosinc.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=rivosinc.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=rivosinc-com.20230601.gappssmtp.com header.i=@rivosinc-com.20230601.gappssmtp.com header.b="vFP6w0+z" Received: by mail-wm1-f41.google.com with SMTP id 5b1f17b1804b1-4361b6f9faeso91678095e9.1 for ; Tue, 07 Jan 2025 00:32:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20230601.gappssmtp.com; s=20230601; t=1736238751; x=1736843551; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=Bu2VZ1vDW/uI8Rfw7EAtvRn3NpKhRLd/XB967dfr6Cg=; b=vFP6w0+zOBIbmVe+nsBmxj+kQvf35VA93Ro68QbKELULu1OkBnw5CH1b1P2CD0znxo jQXnOUTwSz+m+V13AyVV0TWv0fNeX+6r81/joc6Zv2uBzq/OfJG88e++fVaGBDjqV0uL hYX1OFX2EKhzjpMETePxWRpvCyAaknxJAu5vCTpW/cMXCjsNuv7n/oJ8jRtXmuQuEvFi Q+GFVQivz75H+Rfaz6QwwD6si6dX44ecELn4k5IjAhE+e6NS7oJtFo62TtIK9eX0nmIe m5FUtJ1OCAFp4T55YzxhSkt0D5z/eWqM8t9Ts1eKt3tBzP4cE8dYwQmCETOkWkgwfcuc 06XA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736238751; x=1736843551; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Bu2VZ1vDW/uI8Rfw7EAtvRn3NpKhRLd/XB967dfr6Cg=; b=jFShXt+2U80FGdi9gfYNzQX9SXvrqZNiqsvGN562X2TRb6SihVgBJHUyQwrFB4knaC RXH6+1/q/d+YXJXIcy8ThmRPgaX15uqj3pLG29l6rPIJ2X4sLishJz2VcSxV7pmA9IfJ W3i4rhgLlYPQyh0gf1BjUeWGmyXWk1hCQXVIVf/svXRdRPYa++8XUfcxG7uZx4b8oVeP 2a27aUkqMPRRPMlsqT+qGNnqrYVT/Y9RCc4e3hRR9+xOHY/ZYWKPM4d3w4Vk8W1vkznk enegHh+9RJfRH+rGtzGO7EwJOl5fr4t8ySH8MgpSMbBV+lH2JEXvtVKjx8YBXewpl3YZ +aOQ== X-Forwarded-Encrypted: i=1; AJvYcCV/bH7XS/pEtXRqqn5wfF+w8O7FLEfAPCjnGZ50YlDyEqsdtbZa8aa5JuBjqOsuXG73PdgIsBwqkWNZMy4=@vger.kernel.org X-Gm-Message-State: AOJu0YxtSvjnvxSjVyCEBib+XQpuiyblR1WEw+D4sn7dn5+LCqUzDDjq H8kQrALKm8kdnR7V1qBct5EI1Pxlki2w1ID5qMK/vklE8S4C5U9VlOaQwUt26rQ= X-Gm-Gg: ASbGncv+1oJ09wwEjB2A5gaZbyiypo7acS8RQe+RLYD+cdJxOHyrXThmP3L2g5II07O qUk7Wg7uX7KYKf5VfEPGv65K5bHTtM914FE/Spjym8BRcCZdRaQOO4uaTg+wwpPTy4VB6PZj4Fi 8rOQpJFcLpfxPXv2cLBQQzZunVswiDb2oEIVb+NnnEQOZK/v0RKgc73EKWhOiGluZQgznOQhIZM oFMfBkq0eZD9YkK/w8W8kADNTZjiPDCM0na32XfcvfVPAJkkOG+M8zKA4bIC9tIqlZABwUoD0+D kSr4Iaef7N5yt7RcZo9GDGXhLA== X-Google-Smtp-Source: AGHT+IG6spBFNnEMfh7XosVGH+/IEd0bPSGzBdG7QLVnc35t9cj4MHCAoJXsb6lGwkDUXX3hbUj/tQ== X-Received: by 2002:a7b:c8da:0:b0:436:51ff:25de with SMTP id 5b1f17b1804b1-436dc1c7f41mr16861285e9.7.1736238750957; Tue, 07 Jan 2025 00:32:30 -0800 (PST) Received: from ?IPV6:2a01:e0a:e17:9700:16d2:7456:6634:9626? ([2a01:e0a:e17:9700:16d2:7456:6634:9626]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-38a1c832c45sm49352300f8f.32.2025.01.07.00.32.30 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 07 Jan 2025 00:32:30 -0800 (PST) Message-ID: Date: Tue, 7 Jan 2025 09:32:29 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] riscv: misaligned: disable pagefault before accessing user memory To: "Maciej W. Rozycki" Cc: Alexandre Ghiti , Paul Walmsley , Palmer Dabbelt , "open list:RISC-V ARCHITECTURE" , open list , Samuel Holland References: <20250103160214.657508-1-cleger@rivosinc.com> Content-Language: en-US From: =?UTF-8?B?Q2zDqW1lbnQgTMOpZ2Vy?= In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 07/01/2025 03:33, Maciej W. Rozycki wrote: > On Mon, 6 Jan 2025, Clément Léger wrote: > >>> I'm wondering why the irqs are disabled here? What prevents us from >>> enabling them? IIUC, if for some reason the user page is not in memory, >>> we will fail the misaligned access and kill the process, so such >>> failures could be completely random right? >> >> Hi Alex, >> >> This code is called from interrupt context as a result from an >> exception. AFAIK, interrupts are disabled during the whole exception >> handling and re-enabled when returning to user mode in >> do_notify_resume(). So that's why it triggers this warning. > > This feels outright wrong. We have analogous emulation code in Alpha and > MIPS ports and we stay away from quick sands by enabling interrupts in the > relevant exception handlers as soon as it is safe to do so; in particular > when emulating user-mode unaligned accesses. What causes the RISC-V port > to be different here? Hi Maciej, I probably did not clarify that enough. The current path for misaligned accesses does not re-enable irqs (whatever the mode we enter from). Some other path does (page_fault) as it's indeed safe. > > See e.g. the MIPS exception dispatcher in arch/mips/kernel/genex.S where > BUILD_HANDLER invocations explicitly enable interrupts for most of the > handlers (via `sti' -> `__build_clear_sti'). Then unaligned emulation is > run with the interrupt enable setting preserved from the context the fault > happened in (via `ade' -> `__build_clear_ade'), clearly because it can in > principle trigger from kernel code run with interrupts disabled. But such > code is not supposed to access user memory in the first place and traps > from the user mode will have had interrupts enabled. > > I think the RISC-V port has to do the same, i.e. run the handler with the > supervisor interrupt enable bit set according to whether interrupts were > enabled or not for the mode the fault has triggered in. It seems like as you say, it would actually be safe to re-enable interrupts in the misaligned load/store path as well as do_trap()/do_trap_break() which might also benefit from this improvement. After reviewing, the code, there is also a bunch of irqentry_nmi_enter() that seems a bit weird, maybe they could be replaced with irqentry_enter(). Thanks for the review, Clément > > Maciej