gpu: nova-core: check for overflow to DMATRFBASE1
Subsystems (4)
PATCH v2 1/2 gpu: nova-core: check for overflow to DMATRFBASE1
The NV_PFALCON_FALCON_DMATRFBASE/1 register pair supports DMA addresses up to 49 bits only, but the write to DMATRFBASE1 could exceed that. To mitigate, check first that the DMA address will fit.
---
v2: Improved comment and moved check earlier in function
drivers/gpu/nova-core/falcon.rs | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/gpu/nova-core/falcon.rs +6 -0
@@ -472,6 +472,12 @@ fn dma_wr<F: FalconFirmware<Target = E>>(return Err (EINVAL ); } +// The DMATRFBASE/1 register pair only supports a 49-bit address. +if dma_start > kernel::dma::DmaMask ::new ::<49 >().value () { +dev_err! (self.dev,"DMA address {:#x} exceeds 49 bits\n" , dma_start); +return Err (ERANGE ); + } +// DMA transfers can only be done in units of 256 bytes. Compute how many such transfers we // need to perform. let num_transfers = load_offsets.len.div_ceil (DMA_LEN );
base-commit: 2d7b4a44fb768e1887e7e4cdd8b86817ccd9c3bf
Signature · 1 line
2.52.0
PATCH v2 2/2 gpu: nova-core: add missing newlines to several print strings
Although the dev_xx!() macro calls do not technically require terminating newlines for the format strings, they should be added anyway to maintain consistency, both within Rust code and with the C versions.
---
v2: added some more strings that were missed
drivers/gpu/nova-core/falcon.rs | 6 +++--- drivers/gpu/nova-core/falcon/hal/ga102.rs | 4 ++-- drivers/gpu/nova-core/fb.rs | 2 +- drivers/gpu/nova-core/gpu.rs | 2 +- drivers/gpu/nova-core/gsp/boot.rs | 2 +- drivers/gpu/nova-core/gsp/cmdq.rs | 2 +- drivers/gpu/nova-core/gsp/sequencer.rs | 10 +++++----- drivers/gpu/nova-core/vbios.rs | 2 +- 8 files changed, 15 insertions(+), 15 deletions(-)
drivers/gpu/nova-core/falcon.rs +3 -3
@@ -466,7 +466,7 @@ fn dma_wr<F: FalconFirmware<Target = E>>(if dma_start %DmaAddress ::from (DMA_LEN ) >0 {dev_err! ( self.dev, -"DMA transfer start addresses must be a multiple of {}" , +"DMA transfer start addresses must be a multiple of {}\n" ,DMA_LEN );return Err (EINVAL );
@@ -489,11 +489,11 @@ fn dma_wr<F: FalconFirmware<Target = E>>( .and_then(|size| size.checked_add(load_offsets.src_start)) { None => { - dev_err!(self.dev, "DMA transfer length overflow"); + dev_err!(self.dev, "DMA transfer length overflow\n"); return Err(EOVERFLOW); } Some(upper_bound) if usize::from_safe_cast(upper_bound) > fw.size() => { - dev_err!(self.dev, "DMA transfer goes beyond range of DMA object"); + dev_err!(self.dev, "DMA transfer goes beyond range of DMA object\n"); return Err(EINVAL); } Some(_) => (),
drivers/gpu/nova-core/falcon/hal/ga102.rs +2 -2
@@ -52,7 +52,7 @@ fn signature_reg_fuse_version_ga102(let ucode_idx =match usize::from (ucode_id) { ucode_id @1 ..=regs::NV_FUSE_OPT_FPF_SIZE => ucode_id -1 , _ => { -dev_err! (dev,"invalid ucode id {:#x}" , ucode_id); +dev_err! (dev,"invalid ucode id {:#x}\n" , ucode_id);return Err (EINVAL ); } };
@@ -66,7 +66,7 @@ fn signature_reg_fuse_version_ga102( }else if engine_id_mask &0x0400 !=0 { regs::NV_FUSE_OPT_FPF_GSP_UCODE1_VERSION ::read (bar, ucode_idx).data () }else { -dev_err! (dev,"unexpected engine_id_mask {:#x}" , engine_id_mask); +dev_err! (dev,"unexpected engine_id_mask {:#x}\n" , engine_id_mask);return Err (EINVAL ); };
drivers/gpu/nova-core/fb.rs +1 -1
@@ -80,7 +80,7 @@ pub(crate) fn unregister(&self, bar: &Bar0) {let _ = hal.write_sysmem_flush_page (bar,0 ).inspect_err (|e| {dev_warn! ( &self.device, -"failed to unregister sysmem flush page: {:?}" , +"failed to unregister sysmem flush page: {:?}\n" , e ) });
drivers/gpu/nova-core/gpu.rs +1 -1
@@ -268,7 +268,7 @@ pub(crate) fn new<'a>(// We must wait for GFW_BOOT completion before doing any significant setup on the GPU. _: { gfw::wait_gfw_boot_completion(bar) - .inspect_err(|_| dev_err!(pdev.as_ref(), "GFW boot did not complete"))?; + .inspect_err(|_| dev_err!(pdev.as_ref(), "GFW boot did not complete\n"))?; }, sysmem_flush: SysmemFlush::register(pdev.as_ref(), bar, spec.chipset)?,
drivers/gpu/nova-core/gsp/boot.rs +1 -1
@@ -82,7 +82,7 @@ fn run_fwsec_frts(if frts_status !=0 {dev_err! ( dev, -"FWSEC-FRTS returned with error code {:#x}" , +"FWSEC-FRTS returned with error code {:#x}\n" , frts_status );
drivers/gpu/nova-core/gsp/cmdq.rs +1 -1
@@ -615,7 +615,7 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> { {dev_err! ( self.dev, -"GSP RPC: receive: Call {} - bad checksum" , +"GSP RPC: receive: Call {} - bad checksum\n" , header.sequence () );return Err (EIO );
drivers/gpu/nova-core/gsp/sequencer.rs +5 -5
@@ -121,7 +121,7 @@ pub(crate) fn new(data: &[u8], dev: &device::Device) -> Result<(Self, usize)> { };if data.len () < size { -dev_err! (dev,"Data is not enough for command" ); +dev_err! (dev,"Data is not enough for command\n" );return Err (EINVAL ); }
@@ -320,7 +320,7 @@ fn next(&mut self) -> Option<Self::Item> { cmd_result.map_or_else ( |_err| { -dev_err! (self.dev,"Error parsing command at offset {}" , offset); +dev_err! (self.dev,"Error parsing command at offset {}\n" , offset);None }, |(cmd, size)| {
@@ -382,7 +382,7 @@ pub(crate) fn run(cmdq: &mut Cmdq, params: GspSequencerParams<'a>) -> Result { dev: params.dev, }; -dev_dbg! (sequencer.dev,"Running CPU Sequencer commands" ); +dev_dbg! (sequencer.dev,"Running CPU Sequencer commands\n" );for cmd_resultin sequencer.iter () {match cmd_result {
@@ -390,7 +390,7 @@ pub(crate) fn run(cmdq: &mut Cmdq, params: GspSequencerParams<'a>) -> Result { Err(e) => {dev_err! ( sequencer.dev, -"Error running command at index {}" , +"Error running command at index {}\n" , sequencer.seq_info.cmd_index );return Err (e);
@@ -400,7 +400,7 @@ pub(crate) fn run(cmdq: &mut Cmdq, params: GspSequencerParams<'a>) -> Result {dev_dbg! ( sequencer.dev, -"CPU Sequencer commands completed successfully" +"CPU Sequencer commands completed successfully\n" );Ok (()) }
drivers/gpu/nova-core/vbios.rs +1 -1
@@ -790,7 +790,7 @@ fn falcon_data_ptr(&self) -> Result<u32> {// read the 4 bytes at the offset specified in the token let offset = usize::from (token.data_offset);let bytes: [u8 ;4 ] = self.base.data[offset..offset +4 ].try_into ().map_err (|_| { -dev_err! (self.base.dev,"Failed to convert data slice to array" ); +dev_err! (self.base.dev,"Failed to convert data slice to array\n" );EINVAL })?;
Signature · 1 line
2.52.0
Re: [PATCH v2 2/2] gpu: nova-core: add missing newlines to several print strings
Quoted 8 lines · Timur Tabi
Although the dev_xx!() macro calls do not technically require terminating newlines for the format strings, they should be added anyway to maintain consistency, both within Rust code and with the C versions.
Signed-off-by Timur Tabi <[email protected]>---
v2: added some more strings that were missed
Quoted 9 lines
drivers/gpu/nova-core/falcon.rs | 6 +++--- drivers/gpu/nova-core/falcon/hal/ga102.rs | 4 ++-- drivers/gpu/nova-core/fb.rs | 2 +- drivers/gpu/nova-core/gpu.rs | 2 +- drivers/gpu/nova-core/gsp/boot.rs | 2 +- drivers/gpu/nova-core/gsp/cmdq.rs | 2 +- drivers/gpu/nova-core/gsp/sequencer.rs | 10 +++++----- drivers/gpu/nova-core/vbios.rs | 2 +- 8 files changed, 15 insertions(+), 15 deletions(-)
OK, looks like you got them all:
Codebase WITH \n WITHOUT \n % with \n
-------------------------------------------------------------------
nova-core (dev_*!) 73 0 100%
thanks,
Signature · 1 line
John Hubbard
Quoted 164 lines
drivers/gpu/nova-core/falcon.rs +3
-3@@ -466,7 +466,7 @@ fn dma_wr<F: FalconFirmware<Target = E>>(if dma_start %DmaAddress ::from (DMA_LEN ) >0 {dev_err! ( self.dev, -"DMA transfer start addresses must be a multiple of {}" , +"DMA transfer start addresses must be a multiple of {}\n" ,DMA_LEN );return Err (EINVAL );@@ -489,11 +489,11 @@ fn dma_wr<F: FalconFirmware<Target = E>>( .and_then(|size| size.checked_add(load_offsets.src_start)) { None => { - dev_err!(self.dev, "DMA transfer length overflow"); + dev_err!(self.dev, "DMA transfer length overflow\n"); return Err(EOVERFLOW); } Some(upper_bound) if usize::from_safe_cast(upper_bound) > fw.size() => { - dev_err!(self.dev, "DMA transfer goes beyond range of DMA object"); + dev_err!(self.dev, "DMA transfer goes beyond range of DMA object\n"); return Err(EINVAL); } Some(_) => (),drivers/gpu/nova-core/falcon/hal/ga102.rs +2
-2@@ -52,7 +52,7 @@ fn signature_reg_fuse_version_ga102(let ucode_idx =match usize::from (ucode_id) { ucode_id @1 ..=regs::NV_FUSE_OPT_FPF_SIZE => ucode_id -1 , _ => { -dev_err! (dev,"invalid ucode id {:#x}" , ucode_id); +dev_err! (dev,"invalid ucode id {:#x}\n" , ucode_id);return Err (EINVAL ); } };@@ -66,7 +66,7 @@ fn signature_reg_fuse_version_ga102( }else if engine_id_mask &0x0400 !=0 { regs::NV_FUSE_OPT_FPF_GSP_UCODE1_VERSION ::read (bar, ucode_idx).data () }else { -dev_err! (dev,"unexpected engine_id_mask {:#x}" , engine_id_mask); +dev_err! (dev,"unexpected engine_id_mask {:#x}\n" , engine_id_mask);return Err (EINVAL ); };drivers/gpu/nova-core/fb.rs +1
-1@@ -80,7 +80,7 @@ pub(crate) fn unregister(&self, bar: &Bar0) {let _ = hal.write_sysmem_flush_page (bar,0 ).inspect_err (|e| {dev_warn! ( &self.device, -"failed to unregister sysmem flush page: {:?}" , +"failed to unregister sysmem flush page: {:?}\n" , e ) });drivers/gpu/nova-core/gpu.rs +1
-1@@ -268,7 +268,7 @@ pub(crate) fn new<'a>(// We must wait for GFW_BOOT completion before doing any significant setup on the GPU. _: { gfw::wait_gfw_boot_completion(bar) - .inspect_err(|_| dev_err!(pdev.as_ref(), "GFW boot did not complete"))?; + .inspect_err(|_| dev_err!(pdev.as_ref(), "GFW boot did not complete\n"))?; }, sysmem_flush: SysmemFlush::register(pdev.as_ref(), bar, spec.chipset)?,drivers/gpu/nova-core/gsp/boot.rs +1
-1@@ -82,7 +82,7 @@ fn run_fwsec_frts(if frts_status !=0 {dev_err! ( dev, -"FWSEC-FRTS returned with error code {:#x}" , +"FWSEC-FRTS returned with error code {:#x}\n" , frts_status );drivers/gpu/nova-core/gsp/cmdq.rs +1
-1@@ -615,7 +615,7 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> { {dev_err! ( self.dev, -"GSP RPC: receive: Call {} - bad checksum" , +"GSP RPC: receive: Call {} - bad checksum\n" , header.sequence () );return Err (EIO );drivers/gpu/nova-core/gsp/sequencer.rs +5
-5@@ -121,7 +121,7 @@ pub(crate) fn new(data: &[u8], dev: &device::Device) -> Result<(Self, usize)> { };if data.len () < size { -dev_err! (dev,"Data is not enough for command" ); +dev_err! (dev,"Data is not enough for command\n" );return Err (EINVAL ); }@@ -320,7 +320,7 @@ fn next(&mut self) -> Option<Self::Item> { cmd_result.map_or_else ( |_err| { -dev_err! (self.dev,"Error parsing command at offset {}" , offset); +dev_err! (self.dev,"Error parsing command at offset {}\n" , offset);None }, |(cmd, size)| {@@ -382,7 +382,7 @@ pub(crate) fn run(cmdq: &mut Cmdq, params: GspSequencerParams<'a>) -> Result { dev: params.dev, }; -dev_dbg! (sequencer.dev,"Running CPU Sequencer commands" ); +dev_dbg! (sequencer.dev,"Running CPU Sequencer commands\n" );for cmd_resultin sequencer.iter () {match cmd_result {@@ -390,7 +390,7 @@ pub(crate) fn run(cmdq: &mut Cmdq, params: GspSequencerParams<'a>) -> Result { Err(e) => {dev_err! ( sequencer.dev, -"Error running command at index {}" , +"Error running command at index {}\n" , sequencer.seq_info.cmd_index );return Err (e);@@ -400,7 +400,7 @@ pub(crate) fn run(cmdq: &mut Cmdq, params: GspSequencerParams<'a>) -> Result {dev_dbg! ( sequencer.dev, -"CPU Sequencer commands completed successfully" +"CPU Sequencer commands completed successfully\n" );Ok (()) }drivers/gpu/nova-core/vbios.rs +1
-1@@ -790,7 +790,7 @@ fn falcon_data_ptr(&self) -> Result<u32> {// read the 4 bytes at the offset specified in the token let offset = usize::from (token.data_offset);let bytes: [u8 ;4 ] = self.base.data[offset..offset +4 ].try_into ().map_err (|_| { -dev_err! (self.base.dev,"Failed to convert data slice to array" ); +dev_err! (self.base.dev,"Failed to convert data slice to array\n" );EINVAL })?;
Re: [PATCH v2 2/2] gpu: nova-core: add missing newlines to several print strings
Quoted 8 lines · John Hubbard
Quoted 7 lines · Timur Tabi
Although the dev_xx!() macro calls do not technically require terminating newlines for the format strings, they should be added anyway to maintain consistency, both within Rust code and with the C versions.
Signed-off-by Timur Tabi <[email protected]>---
v2: added some more strings that were missed
...
Quoted 2 lines
Reviewed-by John Hubbard <[email protected]>
Hi Danilo,
Are you comfortable merging these two patches to drm-rust-next? I think they are ready to go at this point.
thanks,
Signature · 1 line
John Hubbard
Danilo Krummrich
maintainerRe: [PATCH v2 2/2] gpu: nova-core: add missing newlines to several print strings
Quoted 2 lines · John Hubbard
Are you comfortable merging these two patches to drm-rust-next? I think they are ready to go at this point.
Yes, they are already on my apply list.
Re: [PATCH v2 2/2] gpu: nova-core: add missing newlines to several print strings
Quoted 5 lines · Danilo Krummrich
Quoted 2 lines · John Hubbard
Are you comfortable merging these two patches to drm-rust-next? I think they are ready to go at this point.
Yes, they are already on my apply list.
Applied to drm-rust-next, thanks!
Quoted 8 lines · Timur Tabi
The NV_PFALCON_FALCON_DMATRFBASE/1 register pair supports DMA addresses up to 49 bits only, but the write to DMATRFBASE1 could exceed that. To mitigate, check first that the DMA address will fit.
Reviewed-by John Hubbard <[email protected]>Reviewed-by Joel Fernandes <[email protected]>Fixes 69f5cd67ce41 ("gpu: nova-core: add falcon register definitions and base code")Signed-off-by Timur Tabi <[email protected]>
Danilo, is it too late to make 6.20 with this patch?
Danilo Krummrich
maintainerQuoted 11 lines · Timur Tabi
Quoted 8 lines · Timur Tabi
The NV_PFALCON_FALCON_DMATRFBASE/1 register pair supports DMA addresses up to 49 bits only, but the write to DMATRFBASE1 could exceed that. To mitigate, check first that the DMA address will fit.
Reviewed-by John Hubbard <[email protected]>Reviewed-by Joel Fernandes <[email protected]>Fixes 69f5cd67ce41 ("gpu: nova-core: add falcon register definitions and base code")Signed-off-by Timur Tabi <[email protected]>Danilo, is it too late to make 6.20 with this patch?
This patch is already in drm-rust-next.