From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: gpu: nova-core: vbios: compute FWSEC-relative Falcon data offset Date: Tue, 26 May 2026 07:03:42 +1000 Message-ID: In-Reply-To: <20260525-fix-vbios-v5-10-e5e455251537@nvidia.com> References: <20260525-fix-vbios-v5-0-e5e455251537@nvidia.com> <20260525-fix-vbios-v5-10-e5e455251537@nvidia.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review Good refactoring. Merging the PCI-AT size subtraction into the helper function `falcon_data_offset` (renamed from `falcon_data_ptr`) eliminates the confusing comment about EFI images sitting in between. The `checked_sub` replaces the old incorrect `<` check (the original checked `data_ptr < data.len()` but this was wrong -- it should have been `>=` since the pointer should be *beyond* the PCI-AT image). **Observation:** The old code had: ```rust if (usize::from_safe_cast(data_ptr)) < self.base.data.len() { dev_err!(self.base.dev, "Falcon data pointer out of bounds\n"); return Err(EINVAL); } ``` This check is inverted -- it errors when the pointer is *within* the PCI-AT data, which actually means it's correct as a sanity check (the falcon data should be in the FWSEC region, not in the PCI-AT image). The new `checked_sub` achieves the same semantics more clearly. --- Generated by Claude Code Patch Reviewer