[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 2/2] VT-d: have set_msi_source_id() return a success indicator
On 13.03.2025 16:01, Andrew Cooper wrote: > On 13/03/2025 1:34 pm, Jan Beulich wrote: >> Handling possible internal errors by just emitting a (debug-build-only) >> log message can't be quite enough. Return error codes in those cases, >> and have the caller propagate those up. >> >> Drop a pointless return path, rather than "inventing" an error code for >> it. >> >> While touching the function declarator anyway also constify its first >> parameter. >> >> Fixes: 476bbccc811c ("VT-d: fix MSI source-id of interrupt remapping") >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> > > Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Thanks. > although... > >> --- a/xen/drivers/passthrough/vtd/intremap.c >> +++ b/xen/drivers/passthrough/vtd/intremap.c >> @@ -436,15 +436,13 @@ void cf_check io_apic_write_remap_rte( >> __ioapic_write_entry(apic, pin, true, old_rte); >> } >> >> -static void set_msi_source_id(struct pci_dev *pdev, struct iremap_entry >> *ire) >> +static int set_msi_source_id(const struct pci_dev *pdev, >> + struct iremap_entry *ire) >> { >> u16 seg; >> u8 bus, devfn, secbus; >> int ret; >> >> - if ( !pdev || !ire ) >> - return; >> - >> seg = pdev->seg; >> bus = pdev->bus; >> devfn = pdev->devfn; >> @@ -485,16 +483,21 @@ static void set_msi_source_id(struct pci >> PCI_BDF(bus, devfn)); >> } >> else >> + { >> dprintk(XENLOG_WARNING VTDPREFIX, >> "d%d: no upstream bridge for %pp\n", >> pdev->domain->domain_id, &pdev->sbdf); > > as you're doing cleanup, %pd here? Given DOM_IO for quarantine, I think > it's more likely now than it used to be. > >> + return -ENXIO; >> + } >> break; >> >> default: >> dprintk(XENLOG_WARNING VTDPREFIX, "d%d: unknown(%u): %pp\n", >> pdev->domain->domain_id, pdev->type, &pdev->sbdf); > > Here too. Also, the "unknown(%u)" is less than ideal. "%pd: %pp > unknown type %u\n" would be better. To be honest this feels like going too far with unrelated cleanup here, even if it's all in patch context. It would be different if at least I touched those dprintk() invocations. Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |