linux-kernel - Re: [PATCH 1/4] ASoC: soc-card: Add storage for PCI SSID

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <92b2b69e-3175-651f-45bb-d9ae11eaec82@linux.intel.com>
Date:   Wed, 13 Sep 2023 12:56:03 +0200
From:   Amadeusz Sławiński 
        <amadeuszx.slawinski@...ux.intel.com>
To:     Richard Fitzgerald <rf@...nsource.cirrus.com>, broonie@...nel.org,
        pierre-louis.bossart@...ux.intel.com,
        yung-chuan.liao@...ux.intel.com, kai.vehmanen@...ux.intel.com,
        peter.ujfalusi@...ux.intel.com
Cc:     alsa-devel@...a-project.org, patches@...nsource.cirrus.com,
        linux-kernel@...r.kernel.org,
        Cezary Rojewski <cezary.rojewski@...el.com>
Subject: Re: [PATCH 1/4] ASoC: soc-card: Add storage for PCI SSID

On 9/12/2023 6:32 PM, Richard Fitzgerald wrote:
> Add members to struct snd_soc_card to store the PCI subsystem ID (SSID)
> of the soundcard.
> 
> The PCI specification provides two registers to store a vendor-specific
> SSID that can be read by drivers to uniquely identify a particular
> "soundcard". This is defined in the PCI specification to distinguish
> products that use the same silicon (and therefore have the same silicon
> ID) so that product-specific differences can be applied.
> 
> PCI only defines 0xFFFF as an invalid value. 0x0000 is not defined as
> invalid. So the usual pattern of zero-filling the struct and then
> assuming a zero value unset will not work. A flag is included to
> indicate when the SSID information has been filled in.
> 
> Unlike DMI information, which has a free-format entirely up to the vendor,
> the PCI SSID has a strictly defined format and a registry of vendor IDs.
> 
> It is usual in Windows drivers that the SSID is used as the sole identifier
> of the specific end-product and the Windows driver contains tables mapping
> that to information about the hardware setup, rather than using ACPI
> properties.
> 
> This SSID is important information for ASoC components that need to apply
> hardware-specific configuration on PCI-based systems.
> 
> As the SSID is a generic part of the PCI specification and is treated as
> identifying the "soundcard", it is reasonable to include this information
> in struct snd_soc_card, instead of components inventing their own custom
> ways to pass this information around.
> 
> Signed-off-by: Richard Fitzgerald <rf@...nsource.cirrus.com>
> ---
>   include/sound/soc-card.h | 37 +++++++++++++++++++++++++++++++++++++
>   include/sound/soc.h      | 11 +++++++++++
>   2 files changed, 48 insertions(+)
> 
> diff --git a/include/sound/soc-card.h b/include/sound/soc-card.h
> index fc94dfb0021f..e8ff2e089cd0 100644
> --- a/include/sound/soc-card.h
> +++ b/include/sound/soc-card.h

...

> diff --git a/include/sound/soc.h b/include/sound/soc.h
> index 509386ff5212..81ed08c5c67d 100644
> --- a/include/sound/soc.h
> +++ b/include/sound/soc.h
> @@ -929,6 +929,17 @@ struct snd_soc_card {
>   #ifdef CONFIG_DMI
>   	char dmi_longname[80];
>   #endif /* CONFIG_DMI */
> +
> +#ifdef CONFIG_PCI
> +	/*
> +	 * PCI does not define 0 as invalid, so pci_subsystem_set indicates
> +	 * whether a value has been written to these fields.
> +	 */
> +	unsigned short pci_subsystem_vendor;
> +	unsigned short pci_subsystem_device;
> +	bool pci_subsystem_set;
> +#endif /* CONFIG_PCI */
> +
>   	char topology_shortname[32];
>   
>   	struct device *dev;

This looks bit weird to me, snd_soc_card is _generic_ struct which is 
not device specific in any way, and now you add fields for PCI, can't 
this somehow be done using drvdata or something?