Alexandre Courbot
2025-Oct-02 13:49 UTC
[PATCH v3 08/13] gpu: nova-core: Add bindings and accessors for GspSystemInfo
Hi Alistair, (+Benno as this concerns the `init!` macros) On Tue Sep 30, 2025 at 10:16 PM JST, Alistair Popple wrote:> Adds bindings and an in-place initialiser for the GspSystemInfo struct. > > Signed-off-by: Alistair Popple <apopple at nvidia.com> > > --- > > It would be good to move to using the `init!` macros at some point, but > I couldn't figure out how to make that work to initialise an enum rather > than a struct as is required for the transparent representation.Indeed we have to jump through a few (minor) hoops. First the `init!` macros do not seem to support tuple structs. They match a `{` after the type name, which is not present in `GspSystemInfo`. By turning it into a regular struct with a single field, we can overcome this, and it doesn't affect the layout the `#[repr(transparent)]` can still be used. Then, due to a limitation with declarative macros, `init!` interprets `::` as a separator for generic arguments, so `bindings::GspSystemInfo` also doesn't parse. Here the trick is to use a local type alias. After overcoming these two, I have been able to make `GspSystemInfo::init` return an in-place initializer. It is then a matter of changing `send_gsp_command` to accept it - please apply the following patch on top of your series for an illustration of how it can be done. Note that I have added a new generic argument for the error returned by the `Init` - this is to let us also use infallible initializers transparently. The cool thing is that callers don't need to specify any generic argument since they can be inferred automatically. diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs index 5580eaf52f7b..0709153f9dc9 100644 --- a/drivers/gpu/nova-core/gsp/cmdq.rs +++ b/drivers/gpu/nova-core/gsp/cmdq.rs @@ -247,12 +247,20 @@ fn notify_gsp(bar: &Bar0) { NV_PGSP_QUEUE_HEAD::default().set_address(0).write(bar); } - pub(crate) fn send_gsp_command<M: CommandToGsp>( + pub(crate) fn send_gsp_command<M, E>( &mut self, bar: &Bar0, payload_size: usize, - init: impl FnOnce(&mut M, SBuffer<core::array::IntoIter<&mut [u8], 2>>) -> Result, - ) -> Result { + init: impl Init<M, E>, + init_sbuffer: impl FnOnce(SBuffer<core::array::IntoIter<&mut [u8], 2>>) -> Result, + ) -> Result + where + M: CommandToGsp, + // This allows all error types, including `Infallible`, to be used with `init`. Without + // this we cannot use regular stack objects as `init` since their `Init` implementation + // does not return any error. + Error: From<E>, + { // TODO: a method that extracts the regions for a given command? // ... and another that reduces the region to a given number of bytes! let driver_area = self.gsp_mem.driver_write_area(); @@ -264,7 +272,7 @@ pub(crate) fn send_gsp_command<M: CommandToGsp>( return Err(EAGAIN); } - let (msg_header, cmd, payload_1, payload_2) = { + let (msg_header, cmd_ptr, payload_1, payload_2) = { #[allow(clippy::incompatible_msrv)] let (msg_header_slice, slice_1) = driver_area .0 @@ -272,7 +280,6 @@ pub(crate) fn send_gsp_command<M: CommandToGsp>( .split_at_mut(size_of::<GspMsgElement>()); let msg_header = GspMsgElement::from_bytes_mut(msg_header_slice).ok_or(EINVAL)?; let (cmd_slice, payload_1) = slice_1.split_at_mut(size_of::<M>()); - let cmd = M::from_bytes_mut(cmd_slice).ok_or(EINVAL)?; #[allow(clippy::incompatible_msrv)] let payload_2 = driver_area.1.as_flattened_mut(); // TODO: Replace this workaround to cut the payload size. @@ -283,11 +290,22 @@ pub(crate) fn send_gsp_command<M: CommandToGsp>( None => (&mut payload_1[..payload_size], payload_2), }; - (msg_header, cmd, payload_1, payload_2) + ( + msg_header, + cmd_slice.as_mut_ptr().cast(), + payload_1, + payload_2, + ) + }; + + let cmd = unsafe { + init.__init(cmd_ptr)?; + // Convert the pointer backto a reference for checksum. + &mut *cmd_ptr }; let sbuffer = SBuffer::new_writer([&mut payload_1[..], &mut payload_2[..]]); - init(cmd, sbuffer)?; + init_sbuffer(sbuffer)?; *msg_header GspMsgElement::new(self.seq, size_of::<M>() + payload_size, M::FUNCTION as u32); diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs index 69df8d4be353..6f1be9078853 100644 --- a/drivers/gpu/nova-core/gsp/commands.rs +++ b/drivers/gpu/nova-core/gsp/commands.rs @@ -79,10 +79,12 @@ pub(crate) fn build_registry(cmdq: &mut Cmdq, bar: &Bar0) -> Result { ], }; - cmdq.send_gsp_command::<PackedRegistryTable>(bar, registry.size(), |table, sbuffer| { - *table = PackedRegistryTable::new(GSP_REGISTRY_NUM_ENTRIES as u32, registry.size() as u32); - registry.write_payload(sbuffer) - }) + cmdq.send_gsp_command( + bar, + registry.size(), + PackedRegistryTable::new(GSP_REGISTRY_NUM_ENTRIES as u32, registry.size() as u32), + |sbuffer| registry.write_payload(sbuffer), + ) } impl CommandToGsp for GspSystemInfo { @@ -95,7 +97,7 @@ pub(crate) fn set_system_info( bar: &Bar0, ) -> Result { build_assert!(size_of::<GspSystemInfo>() < GSP_PAGE_SIZE); - cmdq.send_gsp_command::<GspSystemInfo>(bar, 0, |info, _| GspSystemInfo::init(info, dev))?; + cmdq.send_gsp_command(bar, 0, GspSystemInfo::init(dev), |_| Ok(()))?; Ok(()) } diff --git a/drivers/gpu/nova-core/gsp/fw/commands.rs b/drivers/gpu/nova-core/gsp/fw/commands.rs index 83c2b017c4cb..e69be2f422f2 100644 --- a/drivers/gpu/nova-core/gsp/fw/commands.rs +++ b/drivers/gpu/nova-core/gsp/fw/commands.rs @@ -4,31 +4,50 @@ use kernel::transmute::{AsBytes, FromBytes}; use kernel::{device, pci}; +// Ideally we would derive this for all our bindings, using the same technique as +// https://lore.kernel.org/rust-for-linux/20250814093046.2071971-3-lossin at kernel.org/ +unsafe impl Zeroable for bindings::GspSystemInfo {} + #[repr(transparent)] -pub(crate) struct GspSystemInfo(bindings::GspSystemInfo); +pub(crate) struct GspSystemInfo { + // `try_init!` doesn't seem to work with tuple structs. Work around this by declaring a regular + // field, which comes down to exactly the same. + inner: bindings::GspSystemInfo, +} impl GspSystemInfo { - pub(crate) fn init(&mut self, dev: &pci::Device<device::Bound>) -> Result { - self.0.gpuPhysAddr = dev.resource_start(0)?; - self.0.gpuPhysFbAddr = dev.resource_start(1)?; - self.0.gpuPhysInstAddr = dev.resource_start(3)?; - self.0.nvDomainBusDeviceFunc = u64::from(dev.dev_id()); + #[allow(non_snake_case)] + pub(crate) fn init<'a>(dev: &'a pci::Device<device::Bound>) -> impl Init<Self, Error> + 'a { + // `try_init!` interprets `::` as a separator for generics, use a type alias to remove + // them. + type InnerGspSystemInfo = bindings::GspSystemInfo; - // Using TASK_SIZE in r535_gsp_rpc_set_system_info() seems wrong because - // TASK_SIZE is per-task. That's probably a design issue in GSP-RM though. - self.0.maxUserVa = (1 << 47) - 4096; - self.0.pciConfigMirrorBase = 0x088000; - self.0.pciConfigMirrorSize = 0x001000; + // Initializer for the bindings type. + let init_inner = try_init!(InnerGspSystemInfo { + gpuPhysAddr: dev.resource_start(0)?, + gpuPhysFbAddr: dev.resource_start(1)?, + gpuPhysInstAddr: dev.resource_start(3)?, + nvDomainBusDeviceFunc: u64::from(dev.dev_id()), - self.0.PCIDeviceID - (u32::from(dev.device_id()) << 16) | u32::from(dev.vendor_id().as_raw()); - self.0.PCISubDeviceID - (u32::from(dev.subsystem_device_id()) << 16) | u32::from(dev.subsystem_vendor_id()); - self.0.PCIRevisionID = u32::from(dev.revision_id()); - self.0.bIsPrimary = 0; - self.0.bPreserveVideoMemoryAllocations = 0; + // Using TASK_SIZE in r535_gsp_rpc_set_system_info() seems wrong because + // TASK_SIZE is per-task. That's probably a design issue in GSP-RM though. + maxUserVa: (1 << 47) - 4096, + pciConfigMirrorBase: 0x088000, + pciConfigMirrorSize: 0x001000, - Ok(()) + PCIDeviceID: (u32::from(dev.device_id()) << 16) | u32::from(dev.vendor_id().as_raw()), + PCISubDeviceID: (u32::from(dev.subsystem_device_id()) << 16) + | u32::from(dev.subsystem_vendor_id()), + PCIRevisionID: u32::from(dev.revision_id()), + bIsPrimary: 0, + bPreserveVideoMemoryAllocations: 0, + ..Zeroable::init_zeroed() + }); + + // Final initializer for our type. + try_init!(GspSystemInfo { + inner <- init_inner, + }) } }
Alistair Popple
2025-Oct-02 23:38 UTC
[PATCH v3 08/13] gpu: nova-core: Add bindings and accessors for GspSystemInfo
On 2025-10-02 at 23:49 +1000, Alexandre Courbot <acourbot at nvidia.com> wrote...> Hi Alistair, (+Benno as this concerns the `init!` macros) > > On Tue Sep 30, 2025 at 10:16 PM JST, Alistair Popple wrote: > > Adds bindings and an in-place initialiser for the GspSystemInfo struct. > > > > Signed-off-by: Alistair Popple <apopple at nvidia.com> > > > > --- > > > > It would be good to move to using the `init!` macros at some point, but > > I couldn't figure out how to make that work to initialise an enum rather > > than a struct as is required for the transparent representation. > > Indeed we have to jump through a few (minor) hoops. > > First the `init!` macros do not seem to support tuple structs. They > match a `{` after the type name, which is not present in > `GspSystemInfo`. By turning it into a regular struct with a single > field, we can overcome this, and it doesn't affect the layout the > `#[repr(transparent)]` can still be used.I was thinking we should fix the `init!` macro to support tuple structs. Is there some fundamental reason `init!` couldn't be modified to support tuple structs? It seems like it would be nicer to fix that limitation rather than work around it here.> Then, due to a limitation with declarative macros, `init!` interprets > `::` as a separator for generic arguments, so `bindings::GspSystemInfo` > also doesn't parse. Here the trick is to use a local type alias. > > After overcoming these two, I have been able to make > `GspSystemInfo::init` return an in-place initializer. It is then a > matter of changing `send_gsp_command` to accept it - please apply the > following patch on top of your series for an illustration of how it can > be done. > > Note that I have added a new generic argument for the error returned by > the `Init` - this is to let us also use infallible initializers > transparently. The cool thing is that callers don't need to specify any > generic argument since they can be inferred automatically. > > diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs > index 5580eaf52f7b..0709153f9dc9 100644 > --- a/drivers/gpu/nova-core/gsp/cmdq.rs > +++ b/drivers/gpu/nova-core/gsp/cmdq.rs > @@ -247,12 +247,20 @@ fn notify_gsp(bar: &Bar0) { > NV_PGSP_QUEUE_HEAD::default().set_address(0).write(bar); > } > > - pub(crate) fn send_gsp_command<M: CommandToGsp>( > + pub(crate) fn send_gsp_command<M, E>( > &mut self, > bar: &Bar0, > payload_size: usize, > - init: impl FnOnce(&mut M, SBuffer<core::array::IntoIter<&mut [u8], 2>>) -> Result, > - ) -> Result { > + init: impl Init<M, E>, > + init_sbuffer: impl FnOnce(SBuffer<core::array::IntoIter<&mut [u8], 2>>) -> Result, > + ) -> Result > + where > + M: CommandToGsp, > + // This allows all error types, including `Infallible`, to be used with `init`. Without > + // this we cannot use regular stack objects as `init` since their `Init` implementation > + // does not return any error. > + Error: From<E>, > + { > // TODO: a method that extracts the regions for a given command? > // ... and another that reduces the region to a given number of bytes! > let driver_area = self.gsp_mem.driver_write_area(); > @@ -264,7 +272,7 @@ pub(crate) fn send_gsp_command<M: CommandToGsp>( > return Err(EAGAIN); > } > > - let (msg_header, cmd, payload_1, payload_2) = { > + let (msg_header, cmd_ptr, payload_1, payload_2) = { > #[allow(clippy::incompatible_msrv)] > let (msg_header_slice, slice_1) = driver_area > .0 > @@ -272,7 +280,6 @@ pub(crate) fn send_gsp_command<M: CommandToGsp>( > .split_at_mut(size_of::<GspMsgElement>()); > let msg_header = GspMsgElement::from_bytes_mut(msg_header_slice).ok_or(EINVAL)?; > let (cmd_slice, payload_1) = slice_1.split_at_mut(size_of::<M>()); > - let cmd = M::from_bytes_mut(cmd_slice).ok_or(EINVAL)?; > #[allow(clippy::incompatible_msrv)] > let payload_2 = driver_area.1.as_flattened_mut(); > // TODO: Replace this workaround to cut the payload size. > @@ -283,11 +290,22 @@ pub(crate) fn send_gsp_command<M: CommandToGsp>( > None => (&mut payload_1[..payload_size], payload_2), > }; > > - (msg_header, cmd, payload_1, payload_2) > + ( > + msg_header, > + cmd_slice.as_mut_ptr().cast(), > + payload_1, > + payload_2, > + ) > + }; > + > + let cmd = unsafe { > + init.__init(cmd_ptr)?; > + // Convert the pointer backto a reference for checksum. > + &mut *cmd_ptr > }; > > let sbuffer = SBuffer::new_writer([&mut payload_1[..], &mut payload_2[..]]); > - init(cmd, sbuffer)?; > + init_sbuffer(sbuffer)?; > > *msg_header > GspMsgElement::new(self.seq, size_of::<M>() + payload_size, M::FUNCTION as u32); > diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs > index 69df8d4be353..6f1be9078853 100644 > --- a/drivers/gpu/nova-core/gsp/commands.rs > +++ b/drivers/gpu/nova-core/gsp/commands.rs > @@ -79,10 +79,12 @@ pub(crate) fn build_registry(cmdq: &mut Cmdq, bar: &Bar0) -> Result { > ], > }; > > - cmdq.send_gsp_command::<PackedRegistryTable>(bar, registry.size(), |table, sbuffer| { > - *table = PackedRegistryTable::new(GSP_REGISTRY_NUM_ENTRIES as u32, registry.size() as u32); > - registry.write_payload(sbuffer) > - }) > + cmdq.send_gsp_command( > + bar, > + registry.size(), > + PackedRegistryTable::new(GSP_REGISTRY_NUM_ENTRIES as u32, registry.size() as u32), > + |sbuffer| registry.write_payload(sbuffer), > + ) > } > > impl CommandToGsp for GspSystemInfo { > @@ -95,7 +97,7 @@ pub(crate) fn set_system_info( > bar: &Bar0, > ) -> Result { > build_assert!(size_of::<GspSystemInfo>() < GSP_PAGE_SIZE); > - cmdq.send_gsp_command::<GspSystemInfo>(bar, 0, |info, _| GspSystemInfo::init(info, dev))?; > + cmdq.send_gsp_command(bar, 0, GspSystemInfo::init(dev), |_| Ok(()))?; > > Ok(()) > } > diff --git a/drivers/gpu/nova-core/gsp/fw/commands.rs b/drivers/gpu/nova-core/gsp/fw/commands.rs > index 83c2b017c4cb..e69be2f422f2 100644 > --- a/drivers/gpu/nova-core/gsp/fw/commands.rs > +++ b/drivers/gpu/nova-core/gsp/fw/commands.rs > @@ -4,31 +4,50 @@ > use kernel::transmute::{AsBytes, FromBytes}; > use kernel::{device, pci}; > > +// Ideally we would derive this for all our bindings, using the same technique as > +// https://lore.kernel.org/rust-for-linux/20250814093046.2071971-3-lossin at kernel.org/ > +unsafe impl Zeroable for bindings::GspSystemInfo {} > + > #[repr(transparent)] > -pub(crate) struct GspSystemInfo(bindings::GspSystemInfo); > +pub(crate) struct GspSystemInfo { > + // `try_init!` doesn't seem to work with tuple structs. Work around this by declaring a regular > + // field, which comes down to exactly the same. > + inner: bindings::GspSystemInfo, > +} > > impl GspSystemInfo { > - pub(crate) fn init(&mut self, dev: &pci::Device<device::Bound>) -> Result { > - self.0.gpuPhysAddr = dev.resource_start(0)?; > - self.0.gpuPhysFbAddr = dev.resource_start(1)?; > - self.0.gpuPhysInstAddr = dev.resource_start(3)?; > - self.0.nvDomainBusDeviceFunc = u64::from(dev.dev_id()); > + #[allow(non_snake_case)] > + pub(crate) fn init<'a>(dev: &'a pci::Device<device::Bound>) -> impl Init<Self, Error> + 'a { > + // `try_init!` interprets `::` as a separator for generics, use a type alias to remove > + // them. > + type InnerGspSystemInfo = bindings::GspSystemInfo; > > - // Using TASK_SIZE in r535_gsp_rpc_set_system_info() seems wrong because > - // TASK_SIZE is per-task. That's probably a design issue in GSP-RM though. > - self.0.maxUserVa = (1 << 47) - 4096; > - self.0.pciConfigMirrorBase = 0x088000; > - self.0.pciConfigMirrorSize = 0x001000; > + // Initializer for the bindings type. > + let init_inner = try_init!(InnerGspSystemInfo { > + gpuPhysAddr: dev.resource_start(0)?, > + gpuPhysFbAddr: dev.resource_start(1)?, > + gpuPhysInstAddr: dev.resource_start(3)?, > + nvDomainBusDeviceFunc: u64::from(dev.dev_id()), > > - self.0.PCIDeviceID > - (u32::from(dev.device_id()) << 16) | u32::from(dev.vendor_id().as_raw()); > - self.0.PCISubDeviceID > - (u32::from(dev.subsystem_device_id()) << 16) | u32::from(dev.subsystem_vendor_id()); > - self.0.PCIRevisionID = u32::from(dev.revision_id()); > - self.0.bIsPrimary = 0; > - self.0.bPreserveVideoMemoryAllocations = 0; > + // Using TASK_SIZE in r535_gsp_rpc_set_system_info() seems wrong because > + // TASK_SIZE is per-task. That's probably a design issue in GSP-RM though. > + maxUserVa: (1 << 47) - 4096, > + pciConfigMirrorBase: 0x088000, > + pciConfigMirrorSize: 0x001000, > > - Ok(()) > + PCIDeviceID: (u32::from(dev.device_id()) << 16) | u32::from(dev.vendor_id().as_raw()), > + PCISubDeviceID: (u32::from(dev.subsystem_device_id()) << 16) > + | u32::from(dev.subsystem_vendor_id()), > + PCIRevisionID: u32::from(dev.revision_id()), > + bIsPrimary: 0, > + bPreserveVideoMemoryAllocations: 0, > + ..Zeroable::init_zeroed() > + }); > + > + // Final initializer for our type. > + try_init!(GspSystemInfo { > + inner <- init_inner, > + }) > } > }
Benno Lossin
2025-Oct-03 16:34 UTC
[PATCH v3 08/13] gpu: nova-core: Add bindings and accessors for GspSystemInfo
On Thu Oct 2, 2025 at 3:49 PM CEST, Alexandre Courbot wrote:> Hi Alistair, (+Benno as this concerns the `init!` macros) > > On Tue Sep 30, 2025 at 10:16 PM JST, Alistair Popple wrote: >> Adds bindings and an in-place initialiser for the GspSystemInfo struct. >> >> Signed-off-by: Alistair Popple <apopple at nvidia.com> >> >> --- >> >> It would be good to move to using the `init!` macros at some point, but >> I couldn't figure out how to make that work to initialise an enum rather >> than a struct as is required for the transparent representation. > > Indeed we have to jump through a few (minor) hoops. > > First the `init!` macros do not seem to support tuple structs. They > match a `{` after the type name, which is not present in > `GspSystemInfo`. By turning it into a regular struct with a single > field, we can overcome this, and it doesn't affect the layout the > `#[repr(transparent)]` can still be used.Yeah that's the correct workaround at the moment. I'm tracking support for tuple structs in [1]. Essentially the problem is that it requires lots of effort to parse tuple structs using declarative macros. We will get `syn` this cycle, which will enable me to support several things, including tuple structs. [1]: https://github.com/Rust-for-Linux/pin-init/issues/85> Then, due to a limitation with declarative macros, `init!` interprets > `::` as a separator for generic arguments, so `bindings::GspSystemInfo` > also doesn't parse. Here the trick is to use a local type alias.This one will also be solved when we switch to syn.> After overcoming these two, I have been able to make > `GspSystemInfo::init` return an in-place initializer. It is then a > matter of changing `send_gsp_command` to accept it - please apply the > following patch on top of your series for an illustration of how it can > be done. > > Note that I have added a new generic argument for the error returned by > the `Init` - this is to let us also use infallible initializers > transparently. The cool thing is that callers don't need to specify any > generic argument since they can be inferred automatically. > > diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs > index 5580eaf52f7b..0709153f9dc9 100644 > --- a/drivers/gpu/nova-core/gsp/cmdq.rs > +++ b/drivers/gpu/nova-core/gsp/cmdq.rs > @@ -247,12 +247,20 @@ fn notify_gsp(bar: &Bar0) { > NV_PGSP_QUEUE_HEAD::default().set_address(0).write(bar); > } > > - pub(crate) fn send_gsp_command<M: CommandToGsp>( > + pub(crate) fn send_gsp_command<M, E>( > &mut self, > bar: &Bar0, > payload_size: usize, > - init: impl FnOnce(&mut M, SBuffer<core::array::IntoIter<&mut [u8], 2>>) -> Result, > - ) -> Result { > + init: impl Init<M, E>, > + init_sbuffer: impl FnOnce(SBuffer<core::array::IntoIter<&mut [u8], 2>>) -> Result, > + ) -> Result > + where > + M: CommandToGsp, > + // This allows all error types, including `Infallible`, to be used with `init`. Without > + // this we cannot use regular stack objects as `init` since their `Init` implementation > + // does not return any error. > + Error: From<E>, > + { > // TODO: a method that extracts the regions for a given command? > // ... and another that reduces the region to a given number of bytes! > let driver_area = self.gsp_mem.driver_write_area(); > @@ -264,7 +272,7 @@ pub(crate) fn send_gsp_command<M: CommandToGsp>( > return Err(EAGAIN); > } > > - let (msg_header, cmd, payload_1, payload_2) = { > + let (msg_header, cmd_ptr, payload_1, payload_2) = { > #[allow(clippy::incompatible_msrv)] > let (msg_header_slice, slice_1) = driver_area > .0 > @@ -272,7 +280,6 @@ pub(crate) fn send_gsp_command<M: CommandToGsp>( > .split_at_mut(size_of::<GspMsgElement>()); > let msg_header = GspMsgElement::from_bytes_mut(msg_header_slice).ok_or(EINVAL)?; > let (cmd_slice, payload_1) = slice_1.split_at_mut(size_of::<M>()); > - let cmd = M::from_bytes_mut(cmd_slice).ok_or(EINVAL)?; > #[allow(clippy::incompatible_msrv)] > let payload_2 = driver_area.1.as_flattened_mut(); > // TODO: Replace this workaround to cut the payload size. > @@ -283,11 +290,22 @@ pub(crate) fn send_gsp_command<M: CommandToGsp>( > None => (&mut payload_1[..payload_size], payload_2), > }; > > - (msg_header, cmd, payload_1, payload_2) > + ( > + msg_header, > + cmd_slice.as_mut_ptr().cast(), > + payload_1, > + payload_2, > + ) > + }; > + > + let cmd = unsafe { > + init.__init(cmd_ptr)?;This is missing a safety comment. I haven't looked at this locally, so I don't know what is happening in the 10-20 lines that aren't shown, so I don't know if this is correct (if you're only assuming its initialized after this line completes then it's fine). The rest of the patch looks normal. Hope this helps! --- Cheers, Benno> + // Convert the pointer backto a reference for checksum. > + &mut *cmd_ptr > };