Re: [PATCH v3 6/7] ALSA: hda: Add support for Hygon family 18h model 5h HD-Audio

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The new Hygon HD-Audio controller support misses critical AMD/Zen architectural quirks, specifically 40-bit DMA mask clamping and the AZX_DCAPS_NO_TCSEL capability flag.
--

commit b159eff305dbdd80947b5dde567df2ff7e67de70
Author: Fu Hao <fuhao@open-hieco.net>

ALSA: hda: Add support for Hygon family 18h model 5h HD-Audio

This commit adds the PCI ID 0x1d94 0x14a9 to support the Hygon family 18h
model 5h HDA controller.

> diff --git a/sound/hda/controllers/intel.c b/sound/hda/controllers/intel.c
> index 1e8e3d61291a1..e706d38d7fd6c 100644
> --- a/sound/hda/controllers/intel.c
> +++ b/sound/hda/controllers/intel.c
[ ... ]
> @@ -2815,6 +2819,9 @@ static const struct pci_device_id azx_ids[] = {
>  	  .driver_data = AZX_DRIVER_LOONGSON | AZX_DCAPS_NO_TCSEL },
>  	{ PCI_VDEVICE(LOONGSON, PCI_DEVICE_ID_LOONGSON_HDMI),
>  	  .driver_data = AZX_DRIVER_LOONGSON | AZX_DCAPS_NO_TCSEL },
> +	/* Hygon HDAudio */
> +	{ PCI_VDEVICE(HYGON, PCI_DEVICE_ID_HYGON_18H_M05H_HDA),
> +	  .driver_data = AZX_DRIVER_HYGON | AZX_DCAPS_POSFIX_LPIB | AZX_DCAPS_NO_MSI },

[Severity: High]
Should this capabilities bitmask include the AZX_DCAPS_NO_TCSEL flag?

Hygon processors are based on the AMD Zen architecture. Without this flag,
azx_init_pci() will inadvertently write to the Intel-specific Traffic Class
Select register at PCI offset 0x44:

sound/hda/controllers/intel.c:azx_init_pci() {
    ...
    if (!(chip->driver_caps & AZX_DCAPS_NO_TCSEL)) {
        dev_dbg(chip->card->dev, "Clearing TCSEL\n");
        update_pci_byte(chip->pci, AZX_PCIREG_TCSEL, 0x07, 0);
    }
    ...
}

On AMD/Hygon hardware, offset 0x44 is not TCSEL and modifying it could cause
unintended behavior or system instability.

[Severity: High]
Does this new Hygon device also need the 40-bit DMA mask clamping that is
applied to AMD devices?

In azx_first_init(), AMD devices have their DMA bits explicitly restricted:

sound/hda/controllers/intel.c:azx_first_init() {
    ...
    if (chip->pci->vendor == PCI_VENDOR_ID_AMD)
        dma_bits = 40;
    ...
}

Since Hygon shares hardware constraints with AMD Zen, leaving the DMA bits
at 64 could lead to physical address truncation or IOMMU DMAR faults on
systems with sufficient memory.

>  	{ 0, }
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1781523812.git.fuhao@open-hieco.net?part=6