Re: [PATCH 4/9] gpu: nova-core: read vGPU mode from FSP via PRC protocol

[Zhi Wang <zhiw@nvidia.com>]

On Tue, 16 Jun 2026 17:35:27 +0900
"Alexandre Courbot" <acourbot@nvidia.com> wrote:

> On Thu Jun 4, 2026 at 8:43 PM JST, Zhi Wang wrote:

snip

> > +}  
> 
> These 3 should be defined as their own enum types, to make sure we
> cannot mix them. For instance:
> 
>     enum PrcMessageSubcmd {
>       Read = 0x0c,
>     }
> 
>     enum PrcObjectId {
>       VgpuMode = 0x29,
>     }
> 
>     (for the flags I guess you will want to use
> `kernel::impl_flags!`?)
> 
> Then, `NvdmPayloadPrc` can have a constructor that takes these 3 as
> parameters and returns the constructed value - that way no risk of,
> say, using the subcmd as an object identifier.
> 

Great idea. Actually, this is what I am looking for - how to
organized those values.

> Btw, is there a public source for these values?

Production reconfiguration knobs are public knowledge where folks
can find from [1]. vGPU mode is a newly added one.

[1] https://github.com/NVIDIA/gpu-admin-tools

> 
> I like the idea of using a `prc` sub-module - to the point where I'd
> suggest moving all PRC-related types to it, and all CoT messages to a
> `cot` sub-module. But doing it in this series would distract from the
> goal, and the `fsp` module is not that large, so let's keep everything
> in it for now and do this as a follow-up.
> 

I see. I can send another series to address above after this one is
done.

> > +
> > +/// vGPU operating mode as reported by FSP via the PRC protocol.
> > +#[derive(Debug, Clone, Copy, PartialEq, Eq)]
> > +pub(crate) enum VgpuMode {
> > +    /// vGPU support is disabled on this GPU.
> > +    Disabled = 0,
> > +    /// vGPU support is enabled on this GPU.
> > +    Enabled = 1,
> > +}
> > +
> > +impl TryFrom<u16> for VgpuMode {
> > +    type Error = kernel::error::Error;
> > +
> > +    fn try_from(value: u16) -> Result<Self> {
> > +        match value {
> > +            0 => Ok(VgpuMode::Disabled),
> > +            1 => Ok(VgpuMode::Enabled),
> > +            _ => Err(EINVAL),
> > +        }
> > +    }
> > +}  
> 
> You can even do `TryFrom<NvdmPayloadPrcResponse> for VgpuMode` for a
> better fit.
> 
> > +
> >  /// FSP command response payload (`NVDM_PAYLOAD_COMMAND_RESPONSE`).
> >  #[repr(C, packed)]
> >  #[derive(Clone, Copy)]
> > @@ -57,6 +95,39 @@ struct NvdmPayloadCommandResponse {
> >      error_code: u32,
> >  }
> >  
> > +// SAFETY: NvdmPayloadCommandResponse is a packed C struct with
> > only integral fields. +unsafe impl FromBytes for
> > NvdmPayloadCommandResponse {} +
> > +/// PRC message payload.
> > +///
> > +/// Sent to FSP to query or modify a device configuration knob.
> > +/// The response includes the common FSP response header followed
> > by +/// a [`NvdmPayloadPrcResponse`] with the knob's current state
> > value. +#[repr(C, packed)]
> > +#[derive(Clone, Copy)]
> > +struct NvdmPayloadPrc {
> > +    sub_message_id: u8,
> > +    flags: u8,
> > +    object_id: u8,
> > +    reserved: u8,
> > +}
> > +
> > +// SAFETY: NvdmPayloadPrc is a packed C struct with only integral
> > fields. +unsafe impl AsBytes for NvdmPayloadPrc {}
> > +
> > +/// PRC response payload containing the knob state value.
> > +#[repr(C, packed)]
> > +#[derive(Clone, Copy)]
> > +struct NvdmPayloadPrcResponse {
> > +    value_low: u8,
> > +    value_high: u8,
> > +    reserved1: u8,
> > +    reserved2: u8,
> > +}
> > +
> > +// SAFETY: NvdmPayloadPrcResponse is a packed C struct with only
> > integral fields. +unsafe impl FromBytes for NvdmPayloadPrcResponse
> > {} +
> >  /// Common MCTP and NVDM headers shared by all FSP messages.
> >  #[repr(C, packed)]
> >  #[derive(Clone, Copy)]
> > @@ -92,6 +163,18 @@ struct FspResponse {
> >  // SAFETY: FspResponse is a packed C struct with only integral
> > fields. unsafe impl FromBytes for FspResponse {}
> >  
> > +/// Complete FSP PRC response including the knob state payload.
> > +#[repr(C, packed)]
> > +#[derive(Clone, Copy)]
> > +struct FspPrcResponse {
> > +    header: FspMessageHeader,
> > +    response: NvdmPayloadCommandResponse,  
> 
> Since you introduced `FspMessageHeader` to factor out `mctp_header`
> and `nvdm_header`, can you do the same for the reponse? I guess in our
> case this is as simple as renaming `FspResponse` to
> `FspResponseHeader` and using it in `FspPrcResponse`.
> 
> > +    prc_data: NvdmPayloadPrcResponse,
> > +}
> > +
> > +// SAFETY: FspPrcResponse is a packed C struct with only integral
> > fields. +unsafe impl FromBytes for FspPrcResponse {}
> > +
> >  /// Trait implemented by types representing a message to send to
> > FSP. ///
> >  /// This provides [`Fsp::send_sync_fsp`] with the information it
> > needs to send @@ -178,10 +261,25 @@ fn new<'a>(
> >  // bytes are initialized.
> >  unsafe impl AsBytes for FspCotMessage {}
> >  
> > +/// Complete FSP PRC message.
> > +#[repr(C, packed)]
> > +#[derive(Clone, Copy)]
> > +struct FspPrcMessage {
> > +    header: FspMessageHeader,
> > +    prc: NvdmPayloadPrc,
> > +}
> > +
> > +// SAFETY: FspPrcMessage is a packed C struct with only integral
> > fields. +unsafe impl AsBytes for FspPrcMessage {}
> > +
> >  impl MessageToFsp for FspCotMessage {
> >      const NVDM_TYPE: NvdmType = NvdmType::Cot;
> >  }
> >  
> > +impl MessageToFsp for FspPrcMessage {
> > +    const NVDM_TYPE: NvdmType = NvdmType::Prc;
> > +}
> > +
> >  /// Bundled arguments for FMC boot via FSP Chain of Trust.
> >  pub(crate) struct FmcBootArgs {
> >      chipset: Chipset,
> > @@ -226,6 +324,53 @@ pub(crate) struct Fsp {
> >  }
> >  
> >  impl Fsp {
> > +    /// Read vGPU mode from FSP using the PRC protocol.
> > +    ///
> > +    /// Queries FSP's Management Partition for the active vGPU
> > mode knob value.
> > +    /// Returns [`VgpuMode::Enabled`] if vGPU support is active on
> > this GPU,
> > +    /// [`VgpuMode::Disabled`] otherwise.
> > +    #[expect(dead_code)]
> > +    pub(crate) fn read_vgpu_mode(
> > +        &mut self,
> > +        dev: &device::Device<device::Bound>,
> > +        bar: Bar0<'_>,
> > +    ) -> Result<VgpuMode> {
> > +        let msg = KBox::new(
> > +            FspPrcMessage {
> > +                header: FspMessageHeader::new(NvdmType::Prc),
> > +                prc: NvdmPayloadPrc {
> > +                    sub_message_id: prc::SUBCMD_READ,
> > +                    flags: prc::FLAG_ACTIVE,
> > +                    object_id: prc::OBJECT_VGPU_MODE,
> > +                    reserved: 0,
> > +                },
> > +            },
> > +            GFP_KERNEL,
> > +        )?;
> > +
> > +        let response_buf = self.send_sync_fsp(dev, bar, &*msg)?;
> > +
> > +        let prc_resp_size = core::mem::size_of::<FspPrcResponse>();
> > +        if response_buf.len() < prc_resp_size {
> > +            dev_err!(
> > +                dev,
> > +                "PRC response too small: {} bytes (expected {})\n",
> > +                response_buf.len(),
> > +                prc_resp_size
> > +            );
> > +            return Err(EIO);
> > +        }  
> 
> IIUC `from_bytes_prefix` takes care of checking the size, so this
> check looks redundant.