Re: [PATCHv3 08/14] Add get_fw_dev_path callback for pci bus.
On Thu, Nov 11, 2010 at 06:07:53PM +0200, Gleb Natapov wrote: > On Thu, Nov 11, 2010 at 05:05:11PM +0200, Michael S. Tsirkin wrote: > > On Thu, Nov 11, 2010 at 11:07:01AM +0100, Gerd Hoffmann wrote: > > > On 11/10/10 18:34, Michael S. Tsirkin wrote: > > > >On Wed, Nov 10, 2010 at 07:14:15PM +0200, Gleb Natapov wrote: > > > >> > > > >>Signed-off-by: Gleb Natapov > > > > > > > >Good stuff. We should also consider using this for > > > >CLI and monitor. Some comments below. > > > > > > Oh, we already have a table to map pci classes to descriptions for > > > 'info pci'. > > > > You remind me, that one must be fixed to use names from > > pci_ids.h > > > For now I used numbers like other elements of the table. Fair enough. > If I > would do the conversion I would use convenience marcros to not have > names like PCI_CLASS_NETWORK_TOKEN_RING all over the table :) > > -- > Gleb. I'd prefer to keep it simple :) -- MST -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 08/14] Add get_fw_dev_path callback for pci bus.
On Thu, Nov 11, 2010 at 05:05:11PM +0200, Michael S. Tsirkin wrote: > On Thu, Nov 11, 2010 at 11:07:01AM +0100, Gerd Hoffmann wrote: > > On 11/10/10 18:34, Michael S. Tsirkin wrote: > > >On Wed, Nov 10, 2010 at 07:14:15PM +0200, Gleb Natapov wrote: > > >> > > >>Signed-off-by: Gleb Natapov > > > > > >Good stuff. We should also consider using this for > > >CLI and monitor. Some comments below. > > > > Oh, we already have a table to map pci classes to descriptions for > > 'info pci'. > > You remind me, that one must be fixed to use names from > pci_ids.h > For now I used numbers like other elements of the table. If I would do the conversion I would use convenience marcros to not have names like PCI_CLASS_NETWORK_TOKEN_RING all over the table :) -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 08/14] Add get_fw_dev_path callback for pci bus.
On Thu, Nov 11, 2010 at 11:07:01AM +0100, Gerd Hoffmann wrote: > On 11/10/10 18:34, Michael S. Tsirkin wrote: > >On Wed, Nov 10, 2010 at 07:14:15PM +0200, Gleb Natapov wrote: > >> > >>Signed-off-by: Gleb Natapov > > > >Good stuff. We should also consider using this for > >CLI and monitor. Some comments below. > > Oh, we already have a table to map pci classes to descriptions for > 'info pci'. You remind me, that one must be fixed to use names from pci_ids.h > I'd strongly suggest to just add the fw names to that > table instead of creating a second one ... > > cheers, > Gerd No, I mean the path. We are currently passing domain:bus:slot.function to point at a device and we know it's broken for nested bridges and kind of broken for multiple domains. It's a bug that we must fix. Class names aren't that interesting to me, and I'd be just as happy with packing these into 2 tables as a single one personally, it all seems to be a matter of style. -- MST -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 08/14] Add get_fw_dev_path callback for pci bus.
Oh, we already have a table to map pci classes to descriptions for
'info pci'. I'd strongly suggest to just add the fw names to that
table instead of creating a second one ...
Do you mean pci_class_descriptions?
Exactly.
For some classes open firmware spec
defines single name for all subclasses. For instance all 0Axx should be
called "dock" no matter what xx is. This can't be encoded in a simple
table.
Well, actually it is a list not a table. Should be easy to extend, like
adding a mask, so you can have { .class = 0xa000, .mask = 0xff00,
.fw_name = "dock", .desc = "whatever" } to match 0xA0xx ...
cheers,
Gerd
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 08/14] Add get_fw_dev_path callback for pci bus.
On Thu, Nov 11, 2010 at 11:07:01AM +0100, Gerd Hoffmann wrote: > On 11/10/10 18:34, Michael S. Tsirkin wrote: > >On Wed, Nov 10, 2010 at 07:14:15PM +0200, Gleb Natapov wrote: > >> > >>Signed-off-by: Gleb Natapov > > > >Good stuff. We should also consider using this for > >CLI and monitor. Some comments below. > > Oh, we already have a table to map pci classes to descriptions for > 'info pci'. I'd strongly suggest to just add the fw names to that > table instead of creating a second one ... > Do you mean pci_class_descriptions? For some classes open firmware spec defines single name for all subclasses. For instance all 0Axx should be called "dock" no matter what xx is. This can't be encoded in a simple table. -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 08/14] Add get_fw_dev_path callback for pci bus.
On 11/10/10 18:34, Michael S. Tsirkin wrote: On Wed, Nov 10, 2010 at 07:14:15PM +0200, Gleb Natapov wrote: Signed-off-by: Gleb Natapov Good stuff. We should also consider using this for CLI and monitor. Some comments below. Oh, we already have a table to map pci classes to descriptions for 'info pci'. I'd strongly suggest to just add the fw names to that table instead of creating a second one ... cheers, Gerd -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 08/14] Add get_fw_dev_path callback for pci bus.
On Wed, Nov 10, 2010 at 08:21:55PM +0200, Michael S. Tsirkin wrote:
> On Wed, Nov 10, 2010 at 08:02:12PM +0200, Gleb Natapov wrote:
> > On Wed, Nov 10, 2010 at 07:34:12PM +0200, Michael S. Tsirkin wrote:
> > > On Wed, Nov 10, 2010 at 07:14:15PM +0200, Gleb Natapov wrote:
> > > >
> > > > Signed-off-by: Gleb Natapov
> > >
> > > Good stuff. We should also consider using this for
> > > CLI and monitor. Some comments below.
> > >
> > > > ---
> > > > hw/pci.c | 54 ++
> > > > 1 files changed, 54 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/hw/pci.c b/hw/pci.c
> > > > index 92aaa85..ab0399c 100644
> > > > --- a/hw/pci.c
> > > > +++ b/hw/pci.c
> > > > @@ -63,12 +63,14 @@ struct PCIBus {
> > > >
> > > > static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int
> > > > indent);
> > > > static char *pcibus_get_dev_path(DeviceState *dev);
> > > > +static char *pcibus_get_fw_dev_path(DeviceState *dev);
> > > >
> > > > static struct BusInfo pci_bus_info = {
> > > > .name = "PCI",
> > > > .size = sizeof(PCIBus),
> > > > .print_dev = pcibus_dev_print,
> > > > .get_dev_path = pcibus_get_dev_path,
> > > > +.get_fw_dev_path = pcibus_get_fw_dev_path,
> > > > .props = (Property[]) {
> > > > DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
> > > > DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
> > > > @@ -2135,6 +2137,58 @@ static void pcibus_dev_print(Monitor *mon,
> > > > DeviceState *dev, int indent)
> > > > }
> > > > }
> > > >
> > > > +static char *pci_dev_fw_name(DeviceState *dev, char *buf, int len)
> > > > +{
> > > > +PCIDevice *d = (PCIDevice *)dev;
> > > > +const char *name = NULL;
> > > > +
> > > > +#define E(R, N) case R: name = N; break;
> > >
> > > Straight from underhanded C contest :).
> > > Does this macro really buy us all that much?
> > Easy typing.
>
> I am serious about underhanded C. Macros like this one are their tool
> of choice.
>
Oh, come on. It is not even try to obfuscate anything. There are plenty
of places in kernel where we define macro to simplify table creation.
This is not a new trick.
> > > Split this switch to an inline function, we'll get
> > > case 0x0600: return "host";
> > > and then we won't need the macro.
> > >
> > > > +
> > > > +/* names taken from pci binding for open firmware */
> > > > +switch (pci_get_word(d->config + PCI_CLASS_DEVICE)) {
> > > > +E(0x0001, "display"); E(0x0100, "scsi"); E(0x0101, "ide");
> > > > +E(0x0102, "fdc"); E(0x0103, "ipi"); E(0x0104, "raid");
> > > > +E(0x0200, "ethernet"); E(0x0201, "token-ring"); E(0x0202,
> > > > "fddi");
> > > > +E(0x0203, "atm"); E(0x0300 ... 0x03ff, "display"); E(0x0400,
> > > > "video");
> > > > +E(0x0401, "sound"); E(0x0500, "memory"); E(0x0501, "flash");
> > > > +E(0x0600, "host"); E(0x0601, "isa"); E(0x0602, "eisa");
> > > > +E(0x0603, "mca"); E(0x0604, "pci"); E(0x0605, "pcmcia");
> > > > +E(0x0606, "nubus"); E(0x0607, "cardbus"); E(0x0700, "serial");
> > > > +E(0x0701, "parallel"); E(0x0800, "interrupt-controller");
> > > > +E(0x0801, "dma-controller"); E(0x0802, "timer");
> > > > +E(0x0803, "rtc"); E(0x0900, "keyboard"); E(0x0901, "pen");
> > > > +E(0x0902, "mouse"); E(0x0a00 ... 0x0aff, "dock");
> > > > +E(0x0b00 ... 0x0bff, "cpu"); E(0x0c00, "fireware");
> > > > +E(0x0c01, "access-bus"); E(0x0c02, "ssa"); E(0x0c03, "usb");
> > > > +E(0x0c04, "fibre-channel");
> > >
> > > Please use constants from
> > > hw/pci_ids.h
> > OK. But this will make macro even more handy :) Are all classes defined
> > there?
>
> Not in qemu, but the linux pci_ids has them all I think.
> So just add stuff from there to qemu.
>
OK.
> > >
> > >
> > > > +}
> > > > +#undef E
> > > > +
> > > > +if (name) {
> > > > +pstrcpy(buf, len, name);
> > > > +} else {
> > > > +snprintf(buf, len, "pci%04x,%04x",
> > > > + pci_get_word(d->config + PCI_VENDOR_ID),
> > > > + pci_get_word(d->config + PCI_DEVICE_ID));
> > > > +}
> > > > +
> > > > +return buf;
> > > > +}
> > > > +
> > > > +static char *pcibus_get_fw_dev_path(DeviceState *dev)
> > > > +{
> > > > +PCIDevice *d = (PCIDevice *)dev;
> > > > +char path[50], name[33];
> > > > +int off;
> > > > +
> > > > +off = snprintf(path, sizeof(path), "%...@%x",
> > > > + 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));
> > > > +return strdup(path);
> > > > +}
> > > > +
> > > > static char *pcibus_get_dev_path(DeviceState *dev)
> > > > {
> > > > PCIDevice *d = (PCIDevice *)dev;
> > > > --
> > > > 1.7.1
> >
> > --
> >
Re: [PATCHv3 08/14] Add get_fw_dev_path callback for pci bus.
On Wed, Nov 10, 2010 at 6:21 PM, Michael S. Tsirkin wrote:
> On Wed, Nov 10, 2010 at 08:02:12PM +0200, Gleb Natapov wrote:
>> On Wed, Nov 10, 2010 at 07:34:12PM +0200, Michael S. Tsirkin wrote:
>> > On Wed, Nov 10, 2010 at 07:14:15PM +0200, Gleb Natapov wrote:
>> > >
>> > > Signed-off-by: Gleb Natapov
>> >
>> > Good stuff. We should also consider using this for
>> > CLI and monitor. Some comments below.
>> >
>> > > ---
>> > > hw/pci.c | 54 ++
>> > > 1 files changed, 54 insertions(+), 0 deletions(-)
>> > >
>> > > diff --git a/hw/pci.c b/hw/pci.c
>> > > index 92aaa85..ab0399c 100644
>> > > --- a/hw/pci.c
>> > > +++ b/hw/pci.c
>> > > @@ -63,12 +63,14 @@ struct PCIBus {
>> > >
>> > > static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int
>> > > indent);
>> > > static char *pcibus_get_dev_path(DeviceState *dev);
>> > > +static char *pcibus_get_fw_dev_path(DeviceState *dev);
>> > >
>> > > static struct BusInfo pci_bus_info = {
>> > > .name = "PCI",
>> > > .size = sizeof(PCIBus),
>> > > .print_dev = pcibus_dev_print,
>> > > .get_dev_path = pcibus_get_dev_path,
>> > > + .get_fw_dev_path = pcibus_get_fw_dev_path,
>> > > .props = (Property[]) {
>> > > DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
>> > > DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
>> > > @@ -2135,6 +2137,58 @@ static void pcibus_dev_print(Monitor *mon,
>> > > DeviceState *dev, int indent)
>> > > }
>> > > }
>> > >
>> > > +static char *pci_dev_fw_name(DeviceState *dev, char *buf, int len)
>> > > +{
>> > > + PCIDevice *d = (PCIDevice *)dev;
>> > > + const char *name = NULL;
>> > > +
>> > > +#define E(R, N) case R: name = N; break;
>> >
>> > Straight from underhanded C contest :).
>> > Does this macro really buy us all that much?
>> Easy typing.
>
> I am serious about underhanded C. Macros like this one are their tool
> of choice.
>
>> > Split this switch to an inline function, we'll get
>> > case 0x0600: return "host";
>> > and then we won't need the macro.
>> >
>> > > +
>> > > + /* names taken from pci binding for open firmware */
>> > > + switch (pci_get_word(d->config + PCI_CLASS_DEVICE)) {
>> > > + E(0x0001, "display"); E(0x0100, "scsi"); E(0x0101, "ide");
>> > > + E(0x0102, "fdc"); E(0x0103, "ipi"); E(0x0104, "raid");
>> > > + E(0x0200, "ethernet"); E(0x0201, "token-ring"); E(0x0202,
>> > > "fddi");
>> > > + E(0x0203, "atm"); E(0x0300 ... 0x03ff, "display"); E(0x0400,
>> > > "video");
>> > > + E(0x0401, "sound"); E(0x0500, "memory"); E(0x0501, "flash");
>> > > + E(0x0600, "host"); E(0x0601, "isa"); E(0x0602, "eisa");
>> > > + E(0x0603, "mca"); E(0x0604, "pci"); E(0x0605, "pcmcia");
>> > > + E(0x0606, "nubus"); E(0x0607, "cardbus"); E(0x0700, "serial");
>> > > + E(0x0701, "parallel"); E(0x0800, "interrupt-controller");
>> > > + E(0x0801, "dma-controller"); E(0x0802, "timer");
>> > > + E(0x0803, "rtc"); E(0x0900, "keyboard"); E(0x0901, "pen");
>> > > + E(0x0902, "mouse"); E(0x0a00 ... 0x0aff, "dock");
>> > > + E(0x0b00 ... 0x0bff, "cpu"); E(0x0c00, "fireware");
>> > > + E(0x0c01, "access-bus"); E(0x0c02, "ssa"); E(0x0c03, "usb");
>> > > + E(0x0c04, "fibre-channel");
>> >
>> > Please use constants from
>> > hw/pci_ids.h
>> OK. But this will make macro even more handy :) Are all classes defined
>> there?
>
> Not in qemu, but the linux pci_ids has them all I think.
> So just add stuff from there to qemu.
The PCI database in OpenBIOS may also be handy:
http://tracker.coreboot.org/trac/openbios/browser/trunk/openbios-devel/drivers/pci_database.c
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 08/14] Add get_fw_dev_path callback for pci bus.
On Wed, Nov 10, 2010 at 08:02:12PM +0200, Gleb Natapov wrote:
> On Wed, Nov 10, 2010 at 07:34:12PM +0200, Michael S. Tsirkin wrote:
> > On Wed, Nov 10, 2010 at 07:14:15PM +0200, Gleb Natapov wrote:
> > >
> > > Signed-off-by: Gleb Natapov
> >
> > Good stuff. We should also consider using this for
> > CLI and monitor. Some comments below.
> >
> > > ---
> > > hw/pci.c | 54 ++
> > > 1 files changed, 54 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/hw/pci.c b/hw/pci.c
> > > index 92aaa85..ab0399c 100644
> > > --- a/hw/pci.c
> > > +++ b/hw/pci.c
> > > @@ -63,12 +63,14 @@ struct PCIBus {
> > >
> > > static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent);
> > > static char *pcibus_get_dev_path(DeviceState *dev);
> > > +static char *pcibus_get_fw_dev_path(DeviceState *dev);
> > >
> > > static struct BusInfo pci_bus_info = {
> > > .name = "PCI",
> > > .size = sizeof(PCIBus),
> > > .print_dev = pcibus_dev_print,
> > > .get_dev_path = pcibus_get_dev_path,
> > > +.get_fw_dev_path = pcibus_get_fw_dev_path,
> > > .props = (Property[]) {
> > > DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
> > > DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
> > > @@ -2135,6 +2137,58 @@ static void pcibus_dev_print(Monitor *mon,
> > > DeviceState *dev, int indent)
> > > }
> > > }
> > >
> > > +static char *pci_dev_fw_name(DeviceState *dev, char *buf, int len)
> > > +{
> > > +PCIDevice *d = (PCIDevice *)dev;
> > > +const char *name = NULL;
> > > +
> > > +#define E(R, N) case R: name = N; break;
> >
> > Straight from underhanded C contest :).
> > Does this macro really buy us all that much?
> Easy typing.
I am serious about underhanded C. Macros like this one are their tool
of choice.
> > Split this switch to an inline function, we'll get
> > case 0x0600: return "host";
> > and then we won't need the macro.
> >
> > > +
> > > +/* names taken from pci binding for open firmware */
> > > +switch (pci_get_word(d->config + PCI_CLASS_DEVICE)) {
> > > +E(0x0001, "display"); E(0x0100, "scsi"); E(0x0101, "ide");
> > > +E(0x0102, "fdc"); E(0x0103, "ipi"); E(0x0104, "raid");
> > > +E(0x0200, "ethernet"); E(0x0201, "token-ring"); E(0x0202,
> > > "fddi");
> > > +E(0x0203, "atm"); E(0x0300 ... 0x03ff, "display"); E(0x0400,
> > > "video");
> > > +E(0x0401, "sound"); E(0x0500, "memory"); E(0x0501, "flash");
> > > +E(0x0600, "host"); E(0x0601, "isa"); E(0x0602, "eisa");
> > > +E(0x0603, "mca"); E(0x0604, "pci"); E(0x0605, "pcmcia");
> > > +E(0x0606, "nubus"); E(0x0607, "cardbus"); E(0x0700, "serial");
> > > +E(0x0701, "parallel"); E(0x0800, "interrupt-controller");
> > > +E(0x0801, "dma-controller"); E(0x0802, "timer");
> > > +E(0x0803, "rtc"); E(0x0900, "keyboard"); E(0x0901, "pen");
> > > +E(0x0902, "mouse"); E(0x0a00 ... 0x0aff, "dock");
> > > +E(0x0b00 ... 0x0bff, "cpu"); E(0x0c00, "fireware");
> > > +E(0x0c01, "access-bus"); E(0x0c02, "ssa"); E(0x0c03, "usb");
> > > +E(0x0c04, "fibre-channel");
> >
> > Please use constants from
> > hw/pci_ids.h
> OK. But this will make macro even more handy :) Are all classes defined
> there?
Not in qemu, but the linux pci_ids has them all I think.
So just add stuff from there to qemu.
> >
> >
> > > +}
> > > +#undef E
> > > +
> > > +if (name) {
> > > +pstrcpy(buf, len, name);
> > > +} else {
> > > +snprintf(buf, len, "pci%04x,%04x",
> > > + pci_get_word(d->config + PCI_VENDOR_ID),
> > > + pci_get_word(d->config + PCI_DEVICE_ID));
> > > +}
> > > +
> > > +return buf;
> > > +}
> > > +
> > > +static char *pcibus_get_fw_dev_path(DeviceState *dev)
> > > +{
> > > +PCIDevice *d = (PCIDevice *)dev;
> > > +char path[50], name[33];
> > > +int off;
> > > +
> > > +off = snprintf(path, sizeof(path), "%...@%x",
> > > + 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));
> > > +return strdup(path);
> > > +}
> > > +
> > > static char *pcibus_get_dev_path(DeviceState *dev)
> > > {
> > > PCIDevice *d = (PCIDevice *)dev;
> > > --
> > > 1.7.1
>
> --
> Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 08/14] Add get_fw_dev_path callback for pci bus.
On Wed, Nov 10, 2010 at 07:34:12PM +0200, Michael S. Tsirkin wrote:
> On Wed, Nov 10, 2010 at 07:14:15PM +0200, Gleb Natapov wrote:
> >
> > Signed-off-by: Gleb Natapov
>
> Good stuff. We should also consider using this for
> CLI and monitor. Some comments below.
>
> > ---
> > hw/pci.c | 54 ++
> > 1 files changed, 54 insertions(+), 0 deletions(-)
> >
> > diff --git a/hw/pci.c b/hw/pci.c
> > index 92aaa85..ab0399c 100644
> > --- a/hw/pci.c
> > +++ b/hw/pci.c
> > @@ -63,12 +63,14 @@ struct PCIBus {
> >
> > static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent);
> > static char *pcibus_get_dev_path(DeviceState *dev);
> > +static char *pcibus_get_fw_dev_path(DeviceState *dev);
> >
> > static struct BusInfo pci_bus_info = {
> > .name = "PCI",
> > .size = sizeof(PCIBus),
> > .print_dev = pcibus_dev_print,
> > .get_dev_path = pcibus_get_dev_path,
> > +.get_fw_dev_path = pcibus_get_fw_dev_path,
> > .props = (Property[]) {
> > DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
> > DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
> > @@ -2135,6 +2137,58 @@ static void pcibus_dev_print(Monitor *mon,
> > DeviceState *dev, int indent)
> > }
> > }
> >
> > +static char *pci_dev_fw_name(DeviceState *dev, char *buf, int len)
> > +{
> > +PCIDevice *d = (PCIDevice *)dev;
> > +const char *name = NULL;
> > +
> > +#define E(R, N) case R: name = N; break;
>
> Straight from underhanded C contest :).
> Does this macro really buy us all that much?
Easy typing.
> Split this switch to an inline function, we'll get
> case 0x0600: return "host";
> and then we won't need the macro.
>
> > +
> > +/* names taken from pci binding for open firmware */
> > +switch (pci_get_word(d->config + PCI_CLASS_DEVICE)) {
> > +E(0x0001, "display"); E(0x0100, "scsi"); E(0x0101, "ide");
> > +E(0x0102, "fdc"); E(0x0103, "ipi"); E(0x0104, "raid");
> > +E(0x0200, "ethernet"); E(0x0201, "token-ring"); E(0x0202, "fddi");
> > +E(0x0203, "atm"); E(0x0300 ... 0x03ff, "display"); E(0x0400,
> > "video");
> > +E(0x0401, "sound"); E(0x0500, "memory"); E(0x0501, "flash");
> > +E(0x0600, "host"); E(0x0601, "isa"); E(0x0602, "eisa");
> > +E(0x0603, "mca"); E(0x0604, "pci"); E(0x0605, "pcmcia");
> > +E(0x0606, "nubus"); E(0x0607, "cardbus"); E(0x0700, "serial");
> > +E(0x0701, "parallel"); E(0x0800, "interrupt-controller");
> > +E(0x0801, "dma-controller"); E(0x0802, "timer");
> > +E(0x0803, "rtc"); E(0x0900, "keyboard"); E(0x0901, "pen");
> > +E(0x0902, "mouse"); E(0x0a00 ... 0x0aff, "dock");
> > +E(0x0b00 ... 0x0bff, "cpu"); E(0x0c00, "fireware");
> > +E(0x0c01, "access-bus"); E(0x0c02, "ssa"); E(0x0c03, "usb");
> > +E(0x0c04, "fibre-channel");
>
> Please use constants from
> hw/pci_ids.h
OK. But this will make macro even more handy :) Are all classes defined there?
>
>
> > +}
> > +#undef E
> > +
> > +if (name) {
> > +pstrcpy(buf, len, name);
> > +} else {
> > +snprintf(buf, len, "pci%04x,%04x",
> > + pci_get_word(d->config + PCI_VENDOR_ID),
> > + pci_get_word(d->config + PCI_DEVICE_ID));
> > +}
> > +
> > +return buf;
> > +}
> > +
> > +static char *pcibus_get_fw_dev_path(DeviceState *dev)
> > +{
> > +PCIDevice *d = (PCIDevice *)dev;
> > +char path[50], name[33];
> > +int off;
> > +
> > +off = snprintf(path, sizeof(path), "%...@%x",
> > + 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));
> > +return strdup(path);
> > +}
> > +
> > static char *pcibus_get_dev_path(DeviceState *dev)
> > {
> > PCIDevice *d = (PCIDevice *)dev;
> > --
> > 1.7.1
--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 08/14] Add get_fw_dev_path callback for pci bus.
On Wed, Nov 10, 2010 at 07:14:15PM +0200, Gleb Natapov wrote:
>
> Signed-off-by: Gleb Natapov
Good stuff. We should also consider using this for
CLI and monitor. Some comments below.
> ---
> hw/pci.c | 54 ++
> 1 files changed, 54 insertions(+), 0 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index 92aaa85..ab0399c 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -63,12 +63,14 @@ struct PCIBus {
>
> static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent);
> static char *pcibus_get_dev_path(DeviceState *dev);
> +static char *pcibus_get_fw_dev_path(DeviceState *dev);
>
> static struct BusInfo pci_bus_info = {
> .name = "PCI",
> .size = sizeof(PCIBus),
> .print_dev = pcibus_dev_print,
> .get_dev_path = pcibus_get_dev_path,
> +.get_fw_dev_path = pcibus_get_fw_dev_path,
> .props = (Property[]) {
> DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
> DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
> @@ -2135,6 +2137,58 @@ static void pcibus_dev_print(Monitor *mon, DeviceState
> *dev, int indent)
> }
> }
>
> +static char *pci_dev_fw_name(DeviceState *dev, char *buf, int len)
> +{
> +PCIDevice *d = (PCIDevice *)dev;
> +const char *name = NULL;
> +
> +#define E(R, N) case R: name = N; break;
Straight from underhanded C contest :).
Does this macro really buy us all that much?
Split this switch to an inline function, we'll get
case 0x0600: return "host";
and then we won't need the macro.
> +
> +/* names taken from pci binding for open firmware */
> +switch (pci_get_word(d->config + PCI_CLASS_DEVICE)) {
> +E(0x0001, "display"); E(0x0100, "scsi"); E(0x0101, "ide");
> +E(0x0102, "fdc"); E(0x0103, "ipi"); E(0x0104, "raid");
> +E(0x0200, "ethernet"); E(0x0201, "token-ring"); E(0x0202, "fddi");
> +E(0x0203, "atm"); E(0x0300 ... 0x03ff, "display"); E(0x0400,
> "video");
> +E(0x0401, "sound"); E(0x0500, "memory"); E(0x0501, "flash");
> +E(0x0600, "host"); E(0x0601, "isa"); E(0x0602, "eisa");
> +E(0x0603, "mca"); E(0x0604, "pci"); E(0x0605, "pcmcia");
> +E(0x0606, "nubus"); E(0x0607, "cardbus"); E(0x0700, "serial");
> +E(0x0701, "parallel"); E(0x0800, "interrupt-controller");
> +E(0x0801, "dma-controller"); E(0x0802, "timer");
> +E(0x0803, "rtc"); E(0x0900, "keyboard"); E(0x0901, "pen");
> +E(0x0902, "mouse"); E(0x0a00 ... 0x0aff, "dock");
> +E(0x0b00 ... 0x0bff, "cpu"); E(0x0c00, "fireware");
> +E(0x0c01, "access-bus"); E(0x0c02, "ssa"); E(0x0c03, "usb");
> +E(0x0c04, "fibre-channel");
Please use constants from
hw/pci_ids.h
> +}
> +#undef E
> +
> +if (name) {
> +pstrcpy(buf, len, name);
> +} else {
> +snprintf(buf, len, "pci%04x,%04x",
> + pci_get_word(d->config + PCI_VENDOR_ID),
> + pci_get_word(d->config + PCI_DEVICE_ID));
> +}
> +
> +return buf;
> +}
> +
> +static char *pcibus_get_fw_dev_path(DeviceState *dev)
> +{
> +PCIDevice *d = (PCIDevice *)dev;
> +char path[50], name[33];
> +int off;
> +
> +off = snprintf(path, sizeof(path), "%...@%x",
> + 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));
> +return strdup(path);
> +}
> +
> static char *pcibus_get_dev_path(DeviceState *dev)
> {
> PCIDevice *d = (PCIDevice *)dev;
> --
> 1.7.1
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
