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

From: Zhi Wang

Date: Wed Jun 17 2026 - 04:08:07 EST


On Tue, 16 Jun 2026 17:35:27 +0900
"Alexandre Courbot" <acourbot@xxxxxxxxxx> 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.