On 15.04.2025 09:34, dm...@proton.me wrote:
> On Mon, Apr 14, 2025 at 08:19:16PM +0100, Andrii Sultanov wrote:
>> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
>> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
>> @@ -43,7 +43,7 @@ struct amd_iommu *find_iommu_for_device(int seg, int bdf)
>> {
>> unsigned int bd0 = bdf & ~PCI_FUNC(~0);
>>
>> - if ( ivrs_mappings[bd0].iommu && ivrs_mappings[bd0].iommu->bdf !=
>> bdf )
>> + if ( ivrs_mappings[bd0].iommu && ivrs_mappings[bd0].iommu->sbdf.bdf
>> != bdf )
>> {
>> struct ivrs_mappings tmp = ivrs_mappings[bd0];
>>
>> @@ -121,7 +121,7 @@ static bool use_ats(
>> {
>> return !ivrs_dev->block_ats &&
>> iommu_has_cap(iommu, PCI_CAP_IOTLB_SHIFT) &&
>> - pci_ats_device(iommu->seg, pdev->bus, pdev->devfn);
>> + pci_ats_device(iommu->sbdf.seg, pdev->bus, pdev->devfn);
>
> Same idea about updating signature to take pci_sbdf_t.
Perhaps both this and ...
>> @@ -147,17 +147,17 @@ static int __must_check amd_iommu_setup_domain_device(
>> if ( rc )
>> return rc;
>>
>> - req_id = get_dma_requestor_id(iommu->seg, pdev->sbdf.bdf);
>> - ivrs_dev = &get_ivrs_mappings(iommu->seg)[req_id];
>> + req_id = get_dma_requestor_id(iommu->sbdf.seg, pdev->sbdf.bdf);
>> + ivrs_dev = &get_ivrs_mappings(iommu->sbdf.seg)[req_id];
>> sr_flags = (iommu_hwdom_passthrough && is_hardware_domain(domain)
>> ? 0 : SET_ROOT_VALID)
>> | (ivrs_dev->unity_map ? SET_ROOT_WITH_UNITY_MAP : 0);
>>
>> /* get device-table entry */
>> - req_id = get_dma_requestor_id(iommu->seg, PCI_BDF(bus, devfn));
>> + req_id = get_dma_requestor_id(iommu->sbdf.seg, PCI_BDF(bus, devfn));
>> table = iommu->dev_table.buffer;
>> dte = &table[req_id];
>> - ivrs_dev = &get_ivrs_mappings(iommu->seg)[req_id];
>> + ivrs_dev = &get_ivrs_mappings(iommu->sbdf.seg)[req_id];
>>
>> if ( domain != dom_io )
>> {
>> @@ -275,7 +275,7 @@ static int __must_check amd_iommu_setup_domain_device(
>> ASSERT(pcidevs_locked());
>>
>> if ( use_ats(pdev, iommu, ivrs_dev) &&
>> - !pci_ats_enabled(iommu->seg, bus, pdev->devfn) )
>> + !pci_ats_enabled(iommu->sbdf.seg, bus, pdev->devfn) )
>
> ... and same for pci_ats_enabled()
... this would best take a pointer to a const pdev (if that works out).
And yes, there's a lot of similar cleanup to be done in this area. I don't
think we can demand Andrii to do it all in one go.
Jan
