| Message ID | af7536c3234bc7a9d048b4cb1d45f084b4343bdb.1744657012.git.andriy.sultanov@vates.tech (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drivers: Simplify handling of pci_sbdf_t in passthrough/amd | expand |
On Mon, Apr 14, 2025 at 08:19:18PM +0100, Andrii Sultanov wrote: > From: Andrii Sultanov <sultanovandriy@gmail.com> > > Following a similar change to amd_iommu struct, make two more structs > take pci_sbdf_t directly instead of seg and bdf separately. This lets us > drop several conversions from the latter to the former and simplifies > several comparisons and assignments. > > Bloat-o-meter reports: > add/remove: 0/0 grow/shrink: 1/10 up/down: 256/-320 (-64) > Function old new delta > _einittext 22092 22348 +256 > parse_ivrs_hpet 248 245 -3 > amd_iommu_detect_one_acpi 876 868 -8 > iov_supports_xt 275 264 -11 > amd_iommu_read_ioapic_from_ire 344 332 -12 > amd_setup_hpet_msi 237 224 -13 > amd_iommu_ioapic_update_ire 575 555 -20 > reserve_unity_map_for_device 453 424 -29 > _hvm_dpci_msi_eoi 160 128 -32 > amd_iommu_get_supported_ivhd_type 86 30 -56 > parse_ivrs_table 3966 3830 -136 > > Signed-off-by: Andrii Sultanov <sultanovandriy@gmail.com> > > --- > Changes in V4: > * Folded several separate seg+bdf comparisons and assignments into one > with sbdf_t > * With reshuffling in the prior commits, this commit is no longer > neutral in terms of code size > > Changes in V3: > * Dropped aliasing of seg and bdf, renamed users. > > Changes in V2: > * Split single commit into several patches > * Change the format specifier to %pp in amd_iommu_ioapic_update_ire > --- > xen/drivers/passthrough/amd/iommu.h | 5 +-- > xen/drivers/passthrough/amd/iommu_acpi.c | 30 +++++++--------- > xen/drivers/passthrough/amd/iommu_intr.c | 44 +++++++++++------------- > 3 files changed, 37 insertions(+), 42 deletions(-) > > diff --git a/xen/drivers/passthrough/amd/iommu.h b/xen/drivers/passthrough/amd/iommu.h > index 2599800e6a..52f748310b 100644 > --- a/xen/drivers/passthrough/amd/iommu.h > +++ b/xen/drivers/passthrough/amd/iommu.h > @@ -262,7 +262,7 @@ int cf_check amd_setup_hpet_msi(struct msi_desc *msi_desc); > void cf_check amd_iommu_dump_intremap_tables(unsigned char key); > > extern struct ioapic_sbdf { > - u16 bdf, seg; > + pci_sbdf_t sbdf; > u8 id; > bool cmdline; > u16 *pin_2_idx; > @@ -273,7 +273,8 @@ unsigned int ioapic_id_to_index(unsigned int apic_id); > unsigned int get_next_ioapic_sbdf_index(void); > > extern struct hpet_sbdf { > - u16 bdf, seg, id; > + pci_sbdf_t sbdf; > + uint16_t id; > enum { > HPET_NONE, > HPET_CMDL, > diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c b/xen/drivers/passthrough/amd/iommu_acpi.c > index 9e4fbee953..14845766e6 100644 > --- a/xen/drivers/passthrough/amd/iommu_acpi.c > +++ b/xen/drivers/passthrough/amd/iommu_acpi.c > @@ -707,8 +707,7 @@ static int __init cf_check parse_ivrs_ioapic(const char *str) > } > } > > - ioapic_sbdf[idx].bdf = PCI_BDF(bus, dev, func); > - ioapic_sbdf[idx].seg = seg; > + ioapic_sbdf[idx].sbdf = PCI_SBDF( seg, PCI_BDF(bus, dev, func) ); PCI_SBDF() is a variadic macro, so, IMO, the line above can be simplified as: ioapic_sbdf[idx].sbdf = PCI_SBDF( seg, bus, dev, func ); Ex: pdev_type() in xen/drivers/passthrough/pci.c Can you please double check in the modified code? > ioapic_sbdf[idx].id = id; > ioapic_sbdf[idx].cmdline = true; > > @@ -734,8 +733,7 @@ static int __init cf_check parse_ivrs_hpet(const char *str) > return -EINVAL; > > hpet_sbdf.id = id; > - hpet_sbdf.bdf = PCI_BDF(bus, dev, func); > - hpet_sbdf.seg = seg; > + hpet_sbdf.sbdf = PCI_SBDF( seg, PCI_BDF(bus, dev, func) ); ^^ e.g. here it can be simplified too. > hpet_sbdf.init = HPET_CMDL; > > return 0; > @@ -748,6 +746,7 @@ static u16 __init parse_ivhd_device_special( > { > u16 dev_length, bdf; > unsigned int apic, idx; > + pci_sbdf_t sbdf; > > dev_length = sizeof(*special); > if ( header_length < (block_length + dev_length) ) > @@ -757,6 +756,7 @@ static u16 __init parse_ivhd_device_special( > } > > bdf = special->used_id; > + sbdf = PCI_SBDF(seg, bdf); > if ( bdf >= ivrs_bdf_entries ) > { > AMD_IOMMU_ERROR("IVHD: invalid Device_Entry Dev_Id %#x\n", bdf); > @@ -764,7 +764,7 @@ static u16 __init parse_ivhd_device_special( > } > > AMD_IOMMU_DEBUG("IVHD Special: %pp variety %#x handle %#x\n", > - &PCI_SBDF(seg, bdf), special->variety, special->handle); > + &sbdf, special->variety, special->handle); > add_ivrs_mapping_entry(bdf, bdf, special->header.data_setting, 0, true, > iommu); > > @@ -780,8 +780,7 @@ static u16 __init parse_ivhd_device_special( > */ > for ( idx = 0; idx < nr_ioapic_sbdf; idx++ ) > { > - if ( ioapic_sbdf[idx].bdf == bdf && > - ioapic_sbdf[idx].seg == seg && > + if ( ioapic_sbdf[idx].sbdf.sbdf == sbdf.sbdf && > ioapic_sbdf[idx].cmdline ) > break; > } > @@ -790,7 +789,7 @@ static u16 __init parse_ivhd_device_special( > AMD_IOMMU_DEBUG("IVHD: Command line override present for IO-APIC %#x" > "(IVRS: %#x devID %pp)\n", > ioapic_sbdf[idx].id, special->handle, > - &PCI_SBDF(seg, bdf)); > + &sbdf); > break; > } > > @@ -805,8 +804,7 @@ static u16 __init parse_ivhd_device_special( > special->handle); > else if ( idx != MAX_IO_APICS && ioapic_sbdf[idx].pin_2_idx ) > { > - if ( ioapic_sbdf[idx].bdf == bdf && > - ioapic_sbdf[idx].seg == seg ) > + if ( ioapic_sbdf[idx].sbdf.sbdf == sbdf.sbdf ) > AMD_IOMMU_WARN("IVHD: duplicate IO-APIC %#x entries\n", > special->handle); > else > @@ -827,8 +825,7 @@ static u16 __init parse_ivhd_device_special( > } > > /* set device id of ioapic */ > - ioapic_sbdf[idx].bdf = bdf; > - ioapic_sbdf[idx].seg = seg; > + ioapic_sbdf[idx].sbdf = sbdf; > ioapic_sbdf[idx].id = special->handle; > > ioapic_sbdf[idx].pin_2_idx = xmalloc_array( > @@ -862,13 +859,12 @@ static u16 __init parse_ivhd_device_special( > AMD_IOMMU_DEBUG("IVHD: Command line override present for HPET %#x " > "(IVRS: %#x devID %pp)\n", > hpet_sbdf.id, special->handle, > - &PCI_SBDF(seg, bdf)); > + &sbdf); > break; > case HPET_NONE: > /* set device id of hpet */ > hpet_sbdf.id = special->handle; > - hpet_sbdf.bdf = bdf; > - hpet_sbdf.seg = seg; > + hpet_sbdf.sbdf = sbdf; > hpet_sbdf.init = HPET_IVHD; > break; > default: > @@ -1139,9 +1135,9 @@ static int __init cf_check parse_ivrs_table(struct acpi_table_header *table) > return -ENXIO; > } > > - if ( !ioapic_sbdf[idx].seg && > + if ( !ioapic_sbdf[idx].sbdf.seg && > /* SB IO-APIC is always on this device in AMD systems. */ > - ioapic_sbdf[idx].bdf == PCI_BDF(0, 0x14, 0) ) > + ioapic_sbdf[idx].sbdf.bdf == PCI_BDF(0, 0x14, 0) ) Looks like something like the following should work: if ( ioapic_sbdf[idx].sbdf.sbdf == PCI_SBDF(0, 0, 0x14, 0).sbdf ) What do you think? > sb_ioapic = 1; > > if ( ioapic_sbdf[idx].pin_2_idx ) > diff --git a/xen/drivers/passthrough/amd/iommu_intr.c b/xen/drivers/passthrough/amd/iommu_intr.c > index 16075cd5a1..b788675be2 100644 > --- a/xen/drivers/passthrough/amd/iommu_intr.c > +++ b/xen/drivers/passthrough/amd/iommu_intr.c > @@ -323,7 +323,8 @@ void cf_check amd_iommu_ioapic_update_ire( > unsigned int apic, unsigned int pin, uint64_t rte) > { > struct IO_APIC_route_entry new_rte; > - int seg, bdf, rc; > + pci_sbdf_t sbdf; > + int rc; > struct amd_iommu *iommu; > unsigned int idx; > > @@ -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); > __ioapic_write_entry(apic, pin, true, new_rte); > return; > } > > /* Update interrupt remapping entry */ > rc = update_intremap_entry_from_ioapic( > - bdf, iommu, &new_rte, > + sbdf.bdf, iommu, &new_rte, > &ioapic_sbdf[idx].pin_2_idx[pin]); > > if ( rc ) > @@ -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) ) > @@ -515,15 +514,15 @@ int cf_check amd_iommu_msi_msg_update_ire( > struct msi_desc *msi_desc, struct msi_msg *msg) > { > struct pci_dev *pdev = msi_desc->dev; > - int bdf, seg, rc; > + pci_sbdf_t sbdf; > + int rc; > struct amd_iommu *iommu; > unsigned int i, nr = 1; > u32 data; > > - bdf = pdev ? pdev->sbdf.bdf : hpet_sbdf.bdf; > - seg = pdev ? pdev->seg : hpet_sbdf.seg; > + sbdf = pdev ? pdev->sbdf : hpet_sbdf.sbdf; > > - iommu = _find_iommu_for_device(PCI_SBDF(seg, bdf)); > + iommu = _find_iommu_for_device(sbdf); > if ( IS_ERR_OR_NULL(iommu) ) > return PTR_ERR(iommu); > > @@ -532,7 +531,7 @@ int cf_check amd_iommu_msi_msg_update_ire( > > if ( msi_desc->remap_index >= 0 && !msg ) > { > - update_intremap_entry_from_msi_msg(iommu, bdf, nr, > + update_intremap_entry_from_msi_msg(iommu, sbdf.bdf, nr, > &msi_desc->remap_index, > NULL, NULL); > > @@ -543,7 +542,7 @@ int cf_check amd_iommu_msi_msg_update_ire( > if ( !msg ) > return 0; > > - rc = update_intremap_entry_from_msi_msg(iommu, bdf, nr, > + rc = update_intremap_entry_from_msi_msg(iommu, sbdf.bdf, nr, > &msi_desc->remap_index, > msg, &data); > if ( rc > 0 ) > @@ -660,8 +659,7 @@ bool __init cf_check iov_supports_xt(void) > if ( idx == MAX_IO_APICS ) > return false; > > - if ( !find_iommu_for_device(PCI_SBDF(ioapic_sbdf[idx].seg, > - ioapic_sbdf[idx].bdf)) ) > + if ( !find_iommu_for_device(ioapic_sbdf[idx].sbdf) ) > { > AMD_IOMMU_WARN("no IOMMU for IO-APIC %#x (ID %x)\n", > apic, IO_APIC_ID(apic)); > @@ -690,14 +688,14 @@ int __init cf_check amd_setup_hpet_msi(struct msi_desc *msi_desc) > return -ENODEV; > } > > - iommu = find_iommu_for_device(PCI_SBDF(hpet_sbdf.seg, hpet_sbdf.bdf)); > + iommu = find_iommu_for_device(hpet_sbdf.sbdf); > if ( !iommu ) > return -ENXIO; > > - lock = get_intremap_lock(hpet_sbdf.seg, hpet_sbdf.bdf); > + lock = get_intremap_lock(hpet_sbdf.sbdf.seg, hpet_sbdf.sbdf.bdf); > spin_lock_irqsave(lock, flags); > > - msi_desc->remap_index = alloc_intremap_entry(iommu, hpet_sbdf.bdf, 1); > + msi_desc->remap_index = alloc_intremap_entry(iommu, hpet_sbdf.sbdf.bdf, 1); > if ( msi_desc->remap_index >= INTREMAP_MAX_ENTRIES ) > { > msi_desc->remap_index = -1; > -- > 2.49.0 > > Thanks, Denis
On 4/15/25 8:05 AM, dmkhn@proton.me wrote: >> --- a/xen/drivers/passthrough/amd/iommu_acpi.c >> +++ b/xen/drivers/passthrough/amd/iommu_acpi.c >> @@ -707,8 +707,7 @@ static int __init cf_check parse_ivrs_ioapic(const char *str) >> } >> } >> >> - ioapic_sbdf[idx].bdf = PCI_BDF(bus, dev, func); >> - ioapic_sbdf[idx].seg = seg; >> + ioapic_sbdf[idx].sbdf = PCI_SBDF( seg, PCI_BDF(bus, dev, func) ); > PCI_SBDF() is a variadic macro, so, IMO, the line above can be simplified as: > > ioapic_sbdf[idx].sbdf = PCI_SBDF( seg, bus, dev, func ); > > Ex: pdev_type() in xen/drivers/passthrough/pci.c > > Can you please double check in the modified code? > >> ioapic_sbdf[idx].id = id; >> ioapic_sbdf[idx].cmdline = true; >> >> @@ -734,8 +733,7 @@ static int __init cf_check parse_ivrs_hpet(const char *str) >> return -EINVAL; >> >> hpet_sbdf.id = id; >> - hpet_sbdf.bdf = PCI_BDF(bus, dev, func); >> - hpet_sbdf.seg = seg; >> + hpet_sbdf.sbdf = PCI_SBDF( seg, PCI_BDF(bus, dev, func) ); > ^^ > e.g. here it can be simplified too. You are right, PCI_SBDF(sef, bus, dev, func) works here and above. Will resend. >> @@ -1139,9 +1135,9 @@ static int __init cf_check parse_ivrs_table(struct acpi_table_header *table) >> return -ENXIO; >> } >> >> - if ( !ioapic_sbdf[idx].seg && >> + if ( !ioapic_sbdf[idx].sbdf.seg && >> /* SB IO-APIC is always on this device in AMD systems. */ >> - ioapic_sbdf[idx].bdf == PCI_BDF(0, 0x14, 0) ) >> + ioapic_sbdf[idx].sbdf.bdf == PCI_BDF(0, 0x14, 0) ) > Looks like something like the following should work: > > if ( ioapic_sbdf[idx].sbdf.sbdf == PCI_SBDF(0, 0, 0x14, 0).sbdf ) > > What do you think? Will replace this one as well. Thank you!
On 15.04.2025 09:15, Andriy Sultanov wrote: > On 4/15/25 8:05 AM, dmkhn@proton.me wrote: >>> --- a/xen/drivers/passthrough/amd/iommu_acpi.c >>> +++ b/xen/drivers/passthrough/amd/iommu_acpi.c >>> @@ -707,8 +707,7 @@ static int __init cf_check parse_ivrs_ioapic(const char *str) >>> } >>> } >>> >>> - ioapic_sbdf[idx].bdf = PCI_BDF(bus, dev, func); >>> - ioapic_sbdf[idx].seg = seg; >>> + ioapic_sbdf[idx].sbdf = PCI_SBDF( seg, PCI_BDF(bus, dev, func) ); >> PCI_SBDF() is a variadic macro, so, IMO, the line above can be simplified as: >> >> ioapic_sbdf[idx].sbdf = PCI_SBDF( seg, bus, dev, func ); >> >> Ex: pdev_type() in xen/drivers/passthrough/pci.c >> >> Can you please double check in the modified code? >> >>> ioapic_sbdf[idx].id = id; >>> ioapic_sbdf[idx].cmdline = true; >>> >>> @@ -734,8 +733,7 @@ static int __init cf_check parse_ivrs_hpet(const char *str) >>> return -EINVAL; >>> >>> hpet_sbdf.id = id; >>> - hpet_sbdf.bdf = PCI_BDF(bus, dev, func); >>> - hpet_sbdf.seg = seg; >>> + hpet_sbdf.sbdf = PCI_SBDF( seg, PCI_BDF(bus, dev, func) ); >> ^^ >> e.g. here it can be simplified too. > You are right, PCI_SBDF(sef, bus, dev, func) works here and above. Will > resend. And please also drop the stray blanks immediately next to the parentheses. Jan
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
diff --git a/xen/drivers/passthrough/amd/iommu.h b/xen/drivers/passthrough/amd/iommu.h index 2599800e6a..52f748310b 100644 --- a/xen/drivers/passthrough/amd/iommu.h +++ b/xen/drivers/passthrough/amd/iommu.h @@ -262,7 +262,7 @@ int cf_check amd_setup_hpet_msi(struct msi_desc *msi_desc); void cf_check amd_iommu_dump_intremap_tables(unsigned char key); extern struct ioapic_sbdf { - u16 bdf, seg; + pci_sbdf_t sbdf; u8 id; bool cmdline; u16 *pin_2_idx; @@ -273,7 +273,8 @@ unsigned int ioapic_id_to_index(unsigned int apic_id); unsigned int get_next_ioapic_sbdf_index(void); extern struct hpet_sbdf { - u16 bdf, seg, id; + pci_sbdf_t sbdf; + uint16_t id; enum { HPET_NONE, HPET_CMDL, diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c b/xen/drivers/passthrough/amd/iommu_acpi.c index 9e4fbee953..14845766e6 100644 --- a/xen/drivers/passthrough/amd/iommu_acpi.c +++ b/xen/drivers/passthrough/amd/iommu_acpi.c @@ -707,8 +707,7 @@ static int __init cf_check parse_ivrs_ioapic(const char *str) } } - ioapic_sbdf[idx].bdf = PCI_BDF(bus, dev, func); - ioapic_sbdf[idx].seg = seg; + ioapic_sbdf[idx].sbdf = PCI_SBDF( seg, PCI_BDF(bus, dev, func) ); ioapic_sbdf[idx].id = id; ioapic_sbdf[idx].cmdline = true; @@ -734,8 +733,7 @@ static int __init cf_check parse_ivrs_hpet(const char *str) return -EINVAL; hpet_sbdf.id = id; - hpet_sbdf.bdf = PCI_BDF(bus, dev, func); - hpet_sbdf.seg = seg; + hpet_sbdf.sbdf = PCI_SBDF( seg, PCI_BDF(bus, dev, func) ); hpet_sbdf.init = HPET_CMDL; return 0; @@ -748,6 +746,7 @@ static u16 __init parse_ivhd_device_special( { u16 dev_length, bdf; unsigned int apic, idx; + pci_sbdf_t sbdf; dev_length = sizeof(*special); if ( header_length < (block_length + dev_length) ) @@ -757,6 +756,7 @@ static u16 __init parse_ivhd_device_special( } bdf = special->used_id; + sbdf = PCI_SBDF(seg, bdf); if ( bdf >= ivrs_bdf_entries ) { AMD_IOMMU_ERROR("IVHD: invalid Device_Entry Dev_Id %#x\n", bdf); @@ -764,7 +764,7 @@ static u16 __init parse_ivhd_device_special( } AMD_IOMMU_DEBUG("IVHD Special: %pp variety %#x handle %#x\n", - &PCI_SBDF(seg, bdf), special->variety, special->handle); + &sbdf, special->variety, special->handle); add_ivrs_mapping_entry(bdf, bdf, special->header.data_setting, 0, true, iommu); @@ -780,8 +780,7 @@ static u16 __init parse_ivhd_device_special( */ for ( idx = 0; idx < nr_ioapic_sbdf; idx++ ) { - if ( ioapic_sbdf[idx].bdf == bdf && - ioapic_sbdf[idx].seg == seg && + if ( ioapic_sbdf[idx].sbdf.sbdf == sbdf.sbdf && ioapic_sbdf[idx].cmdline ) break; } @@ -790,7 +789,7 @@ static u16 __init parse_ivhd_device_special( AMD_IOMMU_DEBUG("IVHD: Command line override present for IO-APIC %#x" "(IVRS: %#x devID %pp)\n", ioapic_sbdf[idx].id, special->handle, - &PCI_SBDF(seg, bdf)); + &sbdf); break; } @@ -805,8 +804,7 @@ static u16 __init parse_ivhd_device_special( special->handle); else if ( idx != MAX_IO_APICS && ioapic_sbdf[idx].pin_2_idx ) { - if ( ioapic_sbdf[idx].bdf == bdf && - ioapic_sbdf[idx].seg == seg ) + if ( ioapic_sbdf[idx].sbdf.sbdf == sbdf.sbdf ) AMD_IOMMU_WARN("IVHD: duplicate IO-APIC %#x entries\n", special->handle); else @@ -827,8 +825,7 @@ static u16 __init parse_ivhd_device_special( } /* set device id of ioapic */ - ioapic_sbdf[idx].bdf = bdf; - ioapic_sbdf[idx].seg = seg; + ioapic_sbdf[idx].sbdf = sbdf; ioapic_sbdf[idx].id = special->handle; ioapic_sbdf[idx].pin_2_idx = xmalloc_array( @@ -862,13 +859,12 @@ static u16 __init parse_ivhd_device_special( AMD_IOMMU_DEBUG("IVHD: Command line override present for HPET %#x " "(IVRS: %#x devID %pp)\n", hpet_sbdf.id, special->handle, - &PCI_SBDF(seg, bdf)); + &sbdf); break; case HPET_NONE: /* set device id of hpet */ hpet_sbdf.id = special->handle; - hpet_sbdf.bdf = bdf; - hpet_sbdf.seg = seg; + hpet_sbdf.sbdf = sbdf; hpet_sbdf.init = HPET_IVHD; break; default: @@ -1139,9 +1135,9 @@ static int __init cf_check parse_ivrs_table(struct acpi_table_header *table) return -ENXIO; } - if ( !ioapic_sbdf[idx].seg && + if ( !ioapic_sbdf[idx].sbdf.seg && /* SB IO-APIC is always on this device in AMD systems. */ - ioapic_sbdf[idx].bdf == PCI_BDF(0, 0x14, 0) ) + ioapic_sbdf[idx].sbdf.bdf == PCI_BDF(0, 0x14, 0) ) sb_ioapic = 1; if ( ioapic_sbdf[idx].pin_2_idx ) diff --git a/xen/drivers/passthrough/amd/iommu_intr.c b/xen/drivers/passthrough/amd/iommu_intr.c index 16075cd5a1..b788675be2 100644 --- a/xen/drivers/passthrough/amd/iommu_intr.c +++ b/xen/drivers/passthrough/amd/iommu_intr.c @@ -323,7 +323,8 @@ void cf_check amd_iommu_ioapic_update_ire( unsigned int apic, unsigned int pin, uint64_t rte) { struct IO_APIC_route_entry new_rte; - int seg, bdf, rc; + pci_sbdf_t sbdf; + int rc; struct amd_iommu *iommu; unsigned int idx; @@ -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); __ioapic_write_entry(apic, pin, true, new_rte); return; } /* Update interrupt remapping entry */ rc = update_intremap_entry_from_ioapic( - bdf, iommu, &new_rte, + sbdf.bdf, iommu, &new_rte, &ioapic_sbdf[idx].pin_2_idx[pin]); if ( rc ) @@ -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) ) @@ -515,15 +514,15 @@ int cf_check amd_iommu_msi_msg_update_ire( struct msi_desc *msi_desc, struct msi_msg *msg) { struct pci_dev *pdev = msi_desc->dev; - int bdf, seg, rc; + pci_sbdf_t sbdf; + int rc; struct amd_iommu *iommu; unsigned int i, nr = 1; u32 data; - bdf = pdev ? pdev->sbdf.bdf : hpet_sbdf.bdf; - seg = pdev ? pdev->seg : hpet_sbdf.seg; + sbdf = pdev ? pdev->sbdf : hpet_sbdf.sbdf; - iommu = _find_iommu_for_device(PCI_SBDF(seg, bdf)); + iommu = _find_iommu_for_device(sbdf); if ( IS_ERR_OR_NULL(iommu) ) return PTR_ERR(iommu); @@ -532,7 +531,7 @@ int cf_check amd_iommu_msi_msg_update_ire( if ( msi_desc->remap_index >= 0 && !msg ) { - update_intremap_entry_from_msi_msg(iommu, bdf, nr, + update_intremap_entry_from_msi_msg(iommu, sbdf.bdf, nr, &msi_desc->remap_index, NULL, NULL); @@ -543,7 +542,7 @@ int cf_check amd_iommu_msi_msg_update_ire( if ( !msg ) return 0; - rc = update_intremap_entry_from_msi_msg(iommu, bdf, nr, + rc = update_intremap_entry_from_msi_msg(iommu, sbdf.bdf, nr, &msi_desc->remap_index, msg, &data); if ( rc > 0 ) @@ -660,8 +659,7 @@ bool __init cf_check iov_supports_xt(void) if ( idx == MAX_IO_APICS ) return false; - if ( !find_iommu_for_device(PCI_SBDF(ioapic_sbdf[idx].seg, - ioapic_sbdf[idx].bdf)) ) + if ( !find_iommu_for_device(ioapic_sbdf[idx].sbdf) ) { AMD_IOMMU_WARN("no IOMMU for IO-APIC %#x (ID %x)\n", apic, IO_APIC_ID(apic)); @@ -690,14 +688,14 @@ int __init cf_check amd_setup_hpet_msi(struct msi_desc *msi_desc) return -ENODEV; } - iommu = find_iommu_for_device(PCI_SBDF(hpet_sbdf.seg, hpet_sbdf.bdf)); + iommu = find_iommu_for_device(hpet_sbdf.sbdf); if ( !iommu ) return -ENXIO; - lock = get_intremap_lock(hpet_sbdf.seg, hpet_sbdf.bdf); + lock = get_intremap_lock(hpet_sbdf.sbdf.seg, hpet_sbdf.sbdf.bdf); spin_lock_irqsave(lock, flags); - msi_desc->remap_index = alloc_intremap_entry(iommu, hpet_sbdf.bdf, 1); + msi_desc->remap_index = alloc_intremap_entry(iommu, hpet_sbdf.sbdf.bdf, 1); if ( msi_desc->remap_index >= INTREMAP_MAX_ENTRIES ) { msi_desc->remap_index = -1;