Re: [PATCH v2 1/3] rust: pci: provide access to PCI Class, subclass, implementation values

[John Hubbard <jhubbard@nvidia.com>]

On 8/18/25 7:57 AM, Danilo Krummrich wrote:
> On Mon Aug 18, 2025 at 3:33 AM CEST, John Hubbard wrote:
>> +            /// Create a `Class` from the raw class code value, or `None` if the value doesn't
>> +            /// match any known class.
>> +            pub fn from_u32(value: u32) -> Option<Self> {
>> +                match value {
>> +                    $(x if x == Self::$variant.0 => Some(Self::$variant),)+
>> +                    _ => None,
>> +                }
>> +            }
> 
> Additional to the comments from Alex, I think one should be
> `impl TryFrom<u32> for Class`.

Will do.

> 
>> +
>> +    /// Create a new `pci::DeviceId` from a class number, mask, and specific vendor.
>> +    ///
>> +    /// This is more targeted than [`DeviceId::from_class`]: in addition to matching by Vendor, it
>> +    /// also matches the PCI Class (up to the entire 24 bits, depending on the mask).
>> +    pub const fn from_class_and_vendor(class: Class, class_mask: u32, vendor: u32) -> Self {
>> +        Self(bindings::pci_device_id {
>> +            vendor,
>> +            device: DeviceId::PCI_ANY_ID,
>> +            subvendor: DeviceId::PCI_ANY_ID,
>> +            subdevice: DeviceId::PCI_ANY_ID,
>> +            class: class.as_u32(),
>> +            class_mask,
>> +            driver_data: 0,
>> +            override_only: 0,
>> +        })
>> +    }
>>  }
>>  
>>  // SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `pci_device_id` and does not add
>> @@ -410,6 +600,18 @@ pub fn resource_len(&self, bar: u32) -> Result<bindings::resource_size_t> {
>>          // - by its type invariant `self.as_raw` is always a valid pointer to a `struct pci_dev`.
>>          Ok(unsafe { bindings::pci_resource_len(self.as_raw(), bar.try_into()?) })
>>      }
>> +
>> +    /// Returns the full 24-bit PCI class code as stored in hardware.
>> +    /// This includes base class, subclass, and programming interface.
>> +    pub fn class_code_raw(&self) -> u32 {
>> +        // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
>> +        unsafe { (*self.as_raw()).class }
>> +    }
>> +
>> +    /// Returns the PCI class as a `Class` struct, or `None` if the class code is invalid.
>> +    pub fn class_enum(&self) -> Option<Class> {
>> +        Class::from_u32(self.class_code_raw())
>> +    }
> 
> I don't think we have struct pci_dev instances without a valid class code,
> can we? Maybe we should convert infallibly here.

Interesting. Let me un-dizzy myself about the lifetimes here, and verify if 
that claim is true. :)


thanks,
-- 
John Hubbard