From: Joel Fernandes <joelagnelf@nvidia.com>
To: Daniel Almeida <daniel.almeida@collabora.com>
Cc: Guangbo Cui <2407018371@qq.com>, Miguel Ojeda <ojeda@kernel.org>,
a.hindborg@kernel.org, alex.gaynor@gmail.com,
aliceryhl@google.com, benno.lossin@proton.me,
bjorn3_gh@protonmail.mco, boqun.feng@gmail.com,
boris.brezillon@collabora.com, dakr@kernel.org, gary@garyguo.net,
gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
rafael@kernel.org, robh@kernel.org,
rust-for-linux@vger.kernel.org, tmgross@umich.edu,
John Hubbard <jhubbard@nvidia.com>,
Alexandre Courbot <acourbot@nvidia.com>,
Alistair Popple <apopple@nvidia.com>
Subject: Re: [PATCH v6 2/3] rust: io: mem: add a generic iomem abstraction
Date: Tue, 1 Apr 2025 11:57:35 -0400 [thread overview]
Message-ID: <20250401155735.GA804471@joelnvbox> (raw)
In-Reply-To: <67AB9311-3EDC-44A8-9C7C-ABF2ED6B632C@collabora.com>
On Thu, Feb 06, 2025 at 12:57:30PM -0300, Daniel Almeida wrote:
> Btw, Miguel & others,
>
> IMHO, I think we should write a comment about this somewhere in the docs.
>
> When I first came across this issue myself, it took me a while to understand that
> the build_error was actually triggering.
>
> That’s because the result is:
>
> ```
> ERROR: modpost: "rust_build_error" [rust_platform_uio_driver.ko] undefined!
> ```
>
> When a symbol is undefined, someone would be within their rights to assume that
> something is broken in some KConfig somewhere, like this person did. It specifically
> doesn’t tell them that the problem is their own code triggering a build_error because
> they are misusing an API.
>
> I know that we can’t really provide a message through build_error itself, hence my
> suggestion about the docs.
>
> I can send a patch if you agree, it will prevent this confusion from coming up in the
> future.
Interesting, I just ran into this. I am writing function as follows that
reads bar0 in the nova driver, however not having the "if current_len + i"
causes the same issue:
ERROR: modpost: "rust_build_error" [nova_core.ko] undefined!
which did not help much about what the issue really is. I had to figure it
out through tedious trial and error. Also what is the reason for this, the
compiler is doing some checks in with_bar? Looking at with_bar
implementation, I could not see any. Also enabling
CONFIG_RUST_BUILD_ASSERT_ALLOW did not show more menaingful messages. Thanks
for taking a look:
pub(crate) fn read_more(&mut self, bytes: u32) -> Result {
with_bar!(self.bar0, |bar0| {
// Get current length
let current_len = self.data.len();
// Read ROM data bytes push directly to vector
for i in 0..bytes as usize {
// This check fixes:
// ERROR: modpost: "rust_build_error" [nova_core.ko] undefined!
if current_len + i >= 10000000 {
return Err(EINVAL);
}
// Read a byte from the VBIOS ROM and push it to the data vector
let rom_addr = ROM_OFFSET + current_len + i;
let byte = bar0.readb(rom_addr);
self.data.push(byte, GFP_KERNEL)?;
}
Ok(())
})?
}
thanks,
- Joel
next prev parent reply other threads:[~2025-04-01 15:57 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-30 22:05 [PATCH v6 0/3] rust: platform: add Io support Daniel Almeida
2025-01-30 22:05 ` [PATCH v6 1/3] rust: io: add resource abstraction Daniel Almeida
2025-01-31 10:02 ` Daniel Sedlak
2025-02-09 11:45 ` Guangbo Cui
2025-01-30 22:05 ` [PATCH v6 2/3] rust: io: mem: add a generic iomem abstraction Daniel Almeida
2025-01-31 10:09 ` Daniel Sedlak
2025-02-02 22:45 ` Asahi Lina
2025-02-03 9:26 ` Alice Ryhl
2025-02-03 14:14 ` Asahi Lina
2025-02-03 9:32 ` Alice Ryhl
2025-02-05 14:56 ` Guangbo Cui
2025-02-06 15:43 ` Alice Ryhl
2025-02-06 15:58 ` Miguel Ojeda
2025-02-06 15:58 ` Guangbo Cui
2025-02-06 16:11 ` Miguel Ojeda
[not found] ` <tencent_E1DC219DB45DC03A8454E2124D238DCEC705@qq.com>
2025-02-06 17:13 ` Danilo Krummrich
2025-02-07 13:25 ` Daniel Almeida
2025-02-06 15:57 ` Daniel Almeida
2025-02-06 16:05 ` Miguel Ojeda
2025-04-01 15:57 ` Joel Fernandes [this message]
2025-04-01 16:44 ` Danilo Krummrich
2025-04-01 17:07 ` Joel Fernandes
2025-01-30 22:05 ` [PATCH v6 3/3] rust: platform: allow ioremap of platform resources Daniel Almeida
2025-01-31 10:19 ` Daniel Sedlak
2025-01-31 11:36 ` Alice Ryhl
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250401155735.GA804471@joelnvbox \
--to=joelagnelf@nvidia.com \
--cc=2407018371@qq.com \
--cc=a.hindborg@kernel.org \
--cc=acourbot@nvidia.com \
--cc=alex.gaynor@gmail.com \
--cc=aliceryhl@google.com \
--cc=apopple@nvidia.com \
--cc=benno.lossin@proton.me \
--cc=bjorn3_gh@protonmail.mco \
--cc=boqun.feng@gmail.com \
--cc=boris.brezillon@collabora.com \
--cc=dakr@kernel.org \
--cc=daniel.almeida@collabora.com \
--cc=gary@garyguo.net \
--cc=gregkh@linuxfoundation.org \
--cc=jhubbard@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=ojeda@kernel.org \
--cc=rafael@kernel.org \
--cc=robh@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=tmgross@umich.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).