On Fri Mar 6, 2026 at 10:41 AM JST, Eliot Courtney wrote:
<snip>
>> diff --git a/drivers/gpu/nova-core/falcon.rs
>> b/drivers/gpu/nova-core/falcon.rs
>> index 37bfee1d0949..8d444cf9d55c 100644
>> --- a/drivers/gpu/nova-core/falcon.rs
>> +++ b/drivers/gpu/nova-core/falcon.rs
>> @@ -2,12 +2,13 @@
>>
>> //! Falcon microprocessor base support
>>
>> -use core::ops::Deref;
>> -
>> use hal::FalconHal;
>>
>> use kernel::{
>> - device,
>> + device::{
>> + self,
>> + Device, //
>> + },
>> dma::{
>> DmaAddress,
>> DmaMask, //
>> @@ -15,9 +16,7 @@
>> io::poll::read_poll_timeout,
>> prelude::*,
>> sync::aref::ARef,
>> - time::{
>> - Delta, //
>> - },
>> + time::Delta,
>
> nit: Missing // guard here.
IIUC the guard is only required to prevent rustfmt from reformatting,
which does not happen here.
<snip>
>> -impl FirmwareDmaObject<FwsecFirmware, Unsigned> {
>> - fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd:
>> FwsecCommand) -> Result<Self> {
>> +impl FirmwareObject<FwsecFirmware, Unsigned> {
>> + fn new_fwsec(bios: &Vbios, cmd: FwsecCommand) -> Result<Self> {
>> let desc = bios.fwsec_image().header()?;
>> - let ucode = bios.fwsec_image().ucode(&desc)?;
>> - let mut dma_object = DmaObject::from_data(dev, ucode)?;
>> + let mut ucode = KVVec::new();
>> + ucode.extend_from_slice(bios.fwsec_image().ucode(&desc)?,
>> GFP_KERNEL)?;
>>
>> let hdr_offset = desc
>> .imem_load_size()
>> @@ -269,8 +226,9 @@ fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios,
>> cmd: FwsecCommand) -> Re
>> .map(usize::from_safe_cast)
>> .ok_or(EINVAL)?;
>>
>> - // SAFETY: we have exclusive access to `dma_object`.
>> - let hdr: &FalconAppifHdrV1 = unsafe { transmute(&dma_object,
>> hdr_offset) }?;
>> + let hdr = FalconAppifHdrV1::from_bytes_prefix(&ucode[hdr_offset..])
>> + .ok_or(EINVAL)?
>> + .0;
>
> Is it worth adding // PANIC: comments like we have in some other areas
> of the codebase for each of these indexes into ucode?
Even better, we can do
let hdr = ucode
.get(hdr_offset..)
.and_then(FalconAppifHdrV1::from_bytes_prefix)
.ok_or(EINVAL)?
.0;
and have the bounds checked at runtime, as they should be.
>
> Other than those two optional nits,
> Reviewed-by: Eliot Courtney <[email protected]>
Thanks!
