Message ID | 20180629135616.16570-3-mark.cave-ayland@ilande.co.uk |
---|---|
State | New |
Headers | show |
Series | sysbus/pci: allow better customisation of firmware device paths | expand |
On Fri, Jun 29, 2018 at 02:56:16PM +0100, Mark Cave-Ayland wrote: > The current implementation of pci_dev_fw_name() scans through a fixed > set of PCI class descriptions to determine the firmware device name > but in some cases this isn't always appropriate, for example with > the macio device which uses a class code of 0xff (unassigned). > > Rather than add a new entry for the macio device and risk a potential > clash with another unassigned device later, add a check to > pcibus_get_fw_dev_path() that will use DeviceClass fw_name if set in > preference to pci_dev_fw_name(). > > This enables PCI devices such as macio to set dc->fw_name as required > to match the name specified in the firmware. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > hw/pci/pci.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 80bc45930d..126dd683dc 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -2411,12 +2411,14 @@ static char *pci_dev_fw_name(DeviceState *dev, char *buf, int len) > > static char *pcibus_get_fw_dev_path(DeviceState *dev) > { > + DeviceClass *dc = DEVICE_GET_CLASS(dev); > PCIDevice *d = (PCIDevice *)dev; > char path[50], name[33]; > int off; > > off = snprintf(path, sizeof(path), "%s@%x", > - pci_dev_fw_name(dev, name, sizeof name), > + dc->fw_name ? dc->fw_name : > + pci_dev_fw_name(dev, name, sizeof name), > PCI_SLOT(d->devfn)); So why not put the logic into pci_dev_fw_name then? > if (PCI_FUNC(d->devfn)) > snprintf(path + off, sizeof(path) + off, ",%x", PCI_FUNC(d->devfn)); > -- > 2.11.0
On 29/06/18 18:04, Michael S. Tsirkin wrote: > On Fri, Jun 29, 2018 at 02:56:16PM +0100, Mark Cave-Ayland wrote: >> The current implementation of pci_dev_fw_name() scans through a fixed >> set of PCI class descriptions to determine the firmware device name >> but in some cases this isn't always appropriate, for example with >> the macio device which uses a class code of 0xff (unassigned). >> >> Rather than add a new entry for the macio device and risk a potential >> clash with another unassigned device later, add a check to >> pcibus_get_fw_dev_path() that will use DeviceClass fw_name if set in >> preference to pci_dev_fw_name(). >> >> This enables PCI devices such as macio to set dc->fw_name as required >> to match the name specified in the firmware. >> >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> --- >> hw/pci/pci.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c >> index 80bc45930d..126dd683dc 100644 >> --- a/hw/pci/pci.c >> +++ b/hw/pci/pci.c >> @@ -2411,12 +2411,14 @@ static char *pci_dev_fw_name(DeviceState *dev, char *buf, int len) >> >> static char *pcibus_get_fw_dev_path(DeviceState *dev) >> { >> + DeviceClass *dc = DEVICE_GET_CLASS(dev); >> PCIDevice *d = (PCIDevice *)dev; >> char path[50], name[33]; >> int off; >> >> off = snprintf(path, sizeof(path), "%s@%x", >> - pci_dev_fw_name(dev, name, sizeof name), >> + dc->fw_name ? dc->fw_name : >> + pci_dev_fw_name(dev, name, sizeof name), >> PCI_SLOT(d->devfn)); > > So why not put the logic into pci_dev_fw_name then? To me it felt like pci_dev_fw_name() had already made the assumption that it's going to start interrogating PCI config space to determine the fw_name (the initialisers at the top of the function have already started pulling information from out of there) so it seemed a little odd to bury the logic in there. However if you feel it should still belong in pci_dev_fw_name() then I'm happy to resubmit. ATB, Mark.
diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 80bc45930d..126dd683dc 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -2411,12 +2411,14 @@ static char *pci_dev_fw_name(DeviceState *dev, char *buf, int len) static char *pcibus_get_fw_dev_path(DeviceState *dev) { + DeviceClass *dc = DEVICE_GET_CLASS(dev); PCIDevice *d = (PCIDevice *)dev; char path[50], name[33]; int off; off = snprintf(path, sizeof(path), "%s@%x", - pci_dev_fw_name(dev, name, sizeof name), + dc->fw_name ? dc->fw_name : + pci_dev_fw_name(dev, name, sizeof name), PCI_SLOT(d->devfn)); if (PCI_FUNC(d->devfn)) snprintf(path + off, sizeof(path) + off, ",%x", PCI_FUNC(d->devfn));