On 14.04.2025 21:19, Andrii Sultanov wrote:
> @@ -748,6 +746,7 @@ static u16 __init parse_ivhd_device_special(
> {
> u16 dev_length, bdf;
> unsigned int apic, idx;
> + pci_sbdf_t sbdf;
Why does bdf need keeping around here?
> @@ -335,20 +336,18 @@ void cf_check amd_iommu_ioapic_update_ire(
> new_rte.raw = rte;
>
> /* get device id of ioapic devices */
> - bdf = ioapic_sbdf[idx].bdf;
> - seg = ioapic_sbdf[idx].seg;
> - iommu = find_iommu_for_device(PCI_SBDF(seg, bdf));
> + sbdf = ioapic_sbdf[idx].sbdf;
> + iommu = find_iommu_for_device(sbdf);
> if ( !iommu )
> {
> - AMD_IOMMU_WARN("failed to find IOMMU for IO-APIC @ %04x:%04x\n",
> - seg, bdf);
> + AMD_IOMMU_WARN("failed to find IOMMU for IO-APIC @ %pp\n", &sbdf);
Hmm, this one's somewhat questionable: An IO-APIC isn't a PCI device, so
making its "coordinates" look like one can be confusing.
> @@ -369,7 +368,8 @@ unsigned int cf_check amd_iommu_read_ioapic_from_ire(
> unsigned int offset;
> unsigned int val = __io_apic_read(apic, reg);
> unsigned int pin = (reg - 0x10) / 2;
> - uint16_t seg, bdf, req_id;
> + pci_sbdf_t sbdf;
> + uint16_t req_id;
> const struct amd_iommu *iommu;
> union irte_ptr entry;
>
> @@ -381,12 +381,11 @@ unsigned int cf_check amd_iommu_read_ioapic_from_ire(
> if ( offset >= INTREMAP_MAX_ENTRIES )
> return val;
>
> - seg = ioapic_sbdf[idx].seg;
> - bdf = ioapic_sbdf[idx].bdf;
> - iommu = find_iommu_for_device(PCI_SBDF(seg, bdf));
> + sbdf = ioapic_sbdf[idx].sbdf;
> + iommu = find_iommu_for_device(sbdf);
> if ( !iommu )
> return val;
> - req_id = get_intremap_requestor_id(seg, bdf);
> + req_id = get_intremap_requestor_id(sbdf.seg, sbdf.bdf);
> entry = get_intremap_entry(iommu, req_id, offset);
>
> if ( !(reg & 1) )
Is a local variable here warranted in the first place?
Jan
