From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ej1-f73.google.com (mail-ej1-f73.google.com [209.85.218.73]) (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 7ED7B3A63F1 for ; Tue, 20 Jan 2026 08:12:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.218.73 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768896749; cv=none; b=ihKYKphayST5ppjrorU/vCGF+oZoMJ6QDGv2QjUbH/8MKpUfeRTPOjWL2QAxWMmuv6JIQPc2+GMdM1F7IWoaQAwvo1tBazK1KflqoUF3CjoM1K6TXPUqZKfkI0UYN2zxze2i0U72dd4jXMTHLfjEJ4L2MrXW+yYOQWgDyX2ma48= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768896749; c=relaxed/simple; bh=P+SC1Lv8GtKFMmDmSP1puIAFS8fxxtphwLdKaMU0tBM=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=AtvQO4cHCCl3Mntm8HH9MB4RxloE5fjOmY1nZx+anCIlCwAFWWNrkQQId8yI7KXTp+aJwh3FGALYlMG18dCbZFcMQsERiiJ7YPwlzpMElJUZsDQtDfQ3szGQKAWjt05gG0EuQi3QeA/jcPrNRB40c0Zsv2edIvOQIUMqAScwKME= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--aliceryhl.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=atdIbhTf; arc=none smtp.client-ip=209.85.218.73 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--aliceryhl.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="atdIbhTf" Received: by mail-ej1-f73.google.com with SMTP id a640c23a62f3a-b8720608e53so813083366b.1 for ; Tue, 20 Jan 2026 00:12:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1768896746; x=1769501546; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=m4fFtv+H37c+9JHNHcFH2S2lHNgEGMW4y4wNCwUNdFE=; b=atdIbhTfpCa/ygLlJXzb7H2byzytYhaVpduqmUEfXodfcXYbZ85S6QY67VbJfUHmi7 Y6oHAErgP94LgtppglE0hYi3HGWPHhBhn68fpgGyIrKcO83w7Lz+TpcDLe0sBEiXUylX LbgU52TR5tsgHD5JLOVHVCREskD+OBBU5JoY2IN3DjgWmgdSVqkykyCyydgFCPFQbRxI aIeO80ct9vHhnmid07saTWcRgTT2wzuIl+HHu/vpjbrwHVbgjwFW9KwellZ3kx2OYSo7 oqR2I+z1qpsy8SycErm8cr2RLtmzUO/y+JUxL9lfKv1vFSY+JDvCwPuPgE646nfc+dZy ue4A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1768896746; x=1769501546; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=m4fFtv+H37c+9JHNHcFH2S2lHNgEGMW4y4wNCwUNdFE=; b=BW7RdvoIsAudpNoi5c2XqSN9aNDDfogmV7aub5N5kfsXArp1nmHWeCpTN2zdo0jIAd MMAilbNOHMSkgAX4HHJ/eyvgh8vpfqi+QLretyOWJyk1nfqlVJ8H7Zqh5xeg0/l74mAI cgwpY4/cazhX8Ocf7IDhHc1yvpgXPczay0HgMB0QKAlqs2jlOs6m4rpCOrJjPBwzZJ3n t2oU3Aim2XmEGpEuK2/oXdfmoMnDGHuu/zd3dpbd5qWGG4fDRd4O9CvoKboaBNtRFhuw 5cD7XARxtafVFpmhqcqHlwCmhX6CUSNrQyD89yPY2OkK/umYYVuDml2Move+F7su88i9 kJpg== X-Forwarded-Encrypted: i=1; AJvYcCXw4ukYsmKoBNo5wfSxy3Pr9Ldrv7byEpMRh020dafdjIm5cLbzuRJaJeN3UnVEhi6adT/TtzAs16E93gc=@vger.kernel.org X-Gm-Message-State: AOJu0YzPs4jK6rLBGAWfV0fvEcP0iYFze7R8Kuei7hi8f/gOir1kS/Ql LZyRq1pjcGARhlh2YYX7ZsdtEWrw4M6nMWd2vYe6O5m8CPFq4nFleS4QhGjb4Ca5d9NvWvD3DRC +PESD5AXoB+p5e8x9tA== X-Received: from wmgb2.prod.google.com ([2002:a05:600c:1502:b0:477:9801:6a64]) (user=aliceryhl job=prod-delivery.src-stubby-dispatcher) by 2002:a05:600c:3e0d:b0:479:3a86:dc1c with SMTP id 5b1f17b1804b1-4803e803c1amr13091405e9.36.1768896286451; Tue, 20 Jan 2026 00:04:46 -0800 (PST) Date: Tue, 20 Jan 2026 08:04:45 +0000 In-Reply-To: <20260119202250.870588-3-zhiw@nvidia.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20260119202250.870588-1-zhiw@nvidia.com> <20260119202250.870588-3-zhiw@nvidia.com> Message-ID: Subject: Re: [PATCH v10 2/5] rust: io: separate generic I/O helpers from MMIO implementation From: Alice Ryhl To: Zhi Wang Cc: rust-for-linux@vger.kernel.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, dakr@kernel.org, bhelgaas@google.com, kwilczynski@kernel.org, ojeda@kernel.org, alex.gaynor@gmail.com, boqun.feng@gmail.com, gary@garyguo.net, bjorn3_gh@protonmail.com, lossin@kernel.org, a.hindborg@kernel.org, tmgross@umich.edu, markus.probst@posteo.de, helgaas@kernel.org, cjia@nvidia.com, smitra@nvidia.com, ankita@nvidia.com, aniketa@nvidia.com, kwankhede@nvidia.com, targupta@nvidia.com, acourbot@nvidia.com, joelagnelf@nvidia.com, jhubbard@nvidia.com, zhiwang@kernel.org, daniel.almeida@collabora.com Content-Type: text/plain; charset="utf-8" On Mon, Jan 19, 2026 at 10:22:44PM +0200, Zhi Wang wrote: > The previous Io type combined both the generic I/O access helpers > and MMIO implementation details in a single struct. This coupling prevented > reusing the I/O helpers for other backends, such as PCI configuration > space. > > Establish a clean separation between the I/O interface and concrete backends > by separating generic I/O helpers from MMIO implementation. > > Introduce two traits to handle different access capabilities: > - IoCapable trait provides infallible I/O operations (read/write) > with compile-time bounds checking. > - IoTryCapable trait provides fallible I/O operations > (try_read/try_write) with runtime bounds checking. > - The Io trait defines convenience accessors (read8/write8, try_read8/ > try_write8, etc.) that forward to the corresponding IoCapable or > IoTryCapable implementations. > > This separation allows backends to selectively implement only the operations > they support. For example, PCI configuration space can implement IoCapable > for infallible operations while MMIO regions can implement both IoCapable > and IoTryCapable. > > Move the MMIO-specific logic into a dedicated Mmio type that > implements Io and the corresponding `IoCapable` and `IoTryCapable` traits. > Rename IoRaw to MmioRaw and update consumers to use the new types. > > Cc: Alexandre Courbot > Cc: Alice Ryhl > Cc: Bjorn Helgaas > Cc: Gary Guo > Cc: Danilo Krummrich > Cc: John Hubbard > Signed-off-by: Zhi Wang Overall looks good to me. Some comments below: > +/// Trait representing infallible I/O operations of a certain type. > +/// > +/// This trait is used to provide compile-time bounds-checked I/O operations. > +/// Different I/O backends can implement this trait to expose only the operations they support. > +/// > +/// For example, a PCI configuration space may implement `IoCapable`, `IoCapable`, > +/// and `IoCapable`, but not `IoCapable`, while an MMIO region on a 64-bit > +/// system might implement all four. > +pub trait IoCapable { > + /// Infallible read with compile-time bounds check. > + fn read(&self, offset: usize) -> T; > + > + /// Infallible write with compile-time bounds check. > + fn write(&self, value: T, offset: usize); > +} > + > +/// Trait representing fallible I/O operations of a certain type. > +/// > +/// This trait is used to provide runtime bounds-checked I/O operations. > +/// Backends that do not support fallible operations (e.g., PCI configuration space) > +/// do not need to implement this trait. > +pub trait IoTryCapable { > + /// Fallible read with runtime bounds check. > + fn try_read(&self, offset: usize) -> Result; > + > + /// Fallible write with runtime bounds check. > + fn try_write(&self, value: T, offset: usize) -> Result; > +} I still think it would make sense to have `IoCapable: IoTryCapable`, but it's not a big deal. > + /// Infallible 64-bit read with compile-time bounds check. > + #[cfg(CONFIG_64BIT)] > + fn read64(&self, offset: usize) -> u64 > + #[cfg(CONFIG_64BIT)] > + fn try_read64(&self, offset: usize) -> Result These don't really need cfg(CONFIG_64BIT). You can place that cfg on impl blocks of IoCapable. e.g., remove above but keep here: > +// MMIO regions on 64-bit systems also support 64-bit accesses. > +#[cfg(CONFIG_64BIT)] > +impl IoCapable for Mmio { > + define_read!(infallible, read, readq -> u64); > + define_write!(infallible, write, writeq <- u64); > +} > +#[cfg(CONFIG_64BIT)] > +impl IoTryCapable for Mmio { > + define_read!(fallible, try_read, readq -> u64); > + define_write!(fallible, try_write, writeq <- u64); > +} Alice