Alexandre Courbot
2025-Oct-22 13:35 UTC
[PATCH 1/2] nova-core: Solve mentions of `CoherentAllocation` improvements [COHA]
On Sun Oct 19, 2025 at 8:57 PM JST, Daniel del Castillo wrote:>>> @@ -296,24 +287,19 @@ fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd: FwsecCommand) -> Re >>> size: 0, >>> flags: 2, >>> }; >>> - >>> - dmem_mapper.init_cmd = match cmd { >>> - FwsecCommand::Frts { >>> - frts_addr, >>> - frts_size, >>> - } => { >>> - frts_cmd.frts_region = FrtsRegion { >>> - ver: 1, >>> - hdr: size_of::<FrtsRegion>() as u32, >>> - addr: (frts_addr >> 12) as u32, >>> - size: (frts_size >> 12) as u32, >>> - ftype: NVFW_FRTS_CMD_REGION_TYPE_FB, >>> - }; >>> - >>> - NVFW_FALCON_APPIF_DMEMMAPPER_CMD_FRTS >>> - } >>> - FwsecCommand::Sb => NVFW_FALCON_APPIF_DMEMMAPPER_CMD_SB, >>> - }; >>> + if let FwsecCommand::Frts { >>> + frts_addr, >>> + frts_size, >>> + } = cmd >>> + { >>> + frts_cmd.frts_region = FrtsRegion { >>> + ver: 1, >>> + hdr: size_of::<FrtsRegion>() as u32, >>> + addr: (frts_addr >> 12) as u32, >>> + size: (frts_size >> 12) as u32, >>> + ftype: NVFW_FRTS_CMD_REGION_TYPE_FB, >>> + }; >>> + } >> >> I liked that the original code updated both `init_cmd` and `frts_region` >> in the same match block. I understand it might be difficult to preserve >> due to the borrowing rules, but can you try to preserve it if that's >> possible at all? > > I agree it was nicer. I tried to preserve it, but I don't see a way to > do it cleanly, as I can't keep both mutable references at the same time. > What I could do is only check `cmd` once, set `init_cmd` and store an > `Option<FrtsRegion>` that I will later use to set `frts_region` if it's > not `None`. Let me know if you prefer that.Yeah, I agree the borrow checker will get in the way now that we have fixed the lifetimes. What I wanted to avoid is performing the same match operation on `cmd` twice, but if that's difficult then I guess we can live with it. Using a temporary `Option` comes down to the same in the end, except that the second test is indirect.